From de0da96d354c2d40ceae801bff8263973ca8f1d2 Mon Sep 17 00:00:00 2001 From: Nigel Chin Date: Thu, 1 Jul 2021 20:01:41 +1000 Subject: [PATCH 1/3] tags__icontains search return matches tagged with >=1 of each provided keyword, fixed bug with exact search --- aiarena/api/view_filters.py | 55 +++++---------------------- aiarena/core/utils.py | 76 +++++++++++++++++++++++++++++++++++++ aiarena/frontend/views.py | 23 ++++++----- 3 files changed, 96 insertions(+), 58 deletions(-) diff --git a/aiarena/api/view_filters.py b/aiarena/api/view_filters.py index 4e6d5422..4a76ea36 100644 --- a/aiarena/api/view_filters.py +++ b/aiarena/api/view_filters.py @@ -1,50 +1,7 @@ from django_filters import rest_framework as filters -from django.db.models import Q -from rest_framework.exceptions import ValidationError from aiarena.core.models import Match, Result, Bot, Map, User, Round, MatchParticipation, CompetitionParticipation, Competition - - -# Filter for items containing ALL tags in comma separated string -# If passed value contains a "|", then it will filter for items containing tags by ANY users in comma separated string on LHS of separator -class TagsFilter(filters.CharFilter): - def __init__(self, field_name2, *args, lookup_expr2='exact', **kwargs): - super().__init__(*args, **kwargs) - self.field_name2 = field_name2 - self.lookup_expr2 = lookup_expr2 - - def filter(self, qs, value): - if not value: - return qs - - # Check for pipe separator - if '|' in value: - users_str, tags_str = [s.strip() for s in value.split('|')] - else: - users_str = "" - tags_str = value - - # Build query for users - user_query = Q() - if users_str: - try: - users = [int(s) for s in users_str.split(',')] - except ValueError: - raise ValidationError({"tags":["When using pipe separator (|), Expecting user_id (int) on LHS and tag_name on RHS of separator."]}) - lookup = '%s__%s' % (self.field_name2, self.lookup_expr2) - for v in users: - user_query = user_query | Q(**{lookup: v}) - - # Build query for tags - tag_query = Q() - if tags_str: - tags = [s.strip() for s in tags_str.split(',')] - lookup = '%s__%s' % (self.field_name, self.lookup_expr) - for v in tags: - if v: - tag_query = tag_query & Q(**{lookup: v}) - - return self.get_method(qs)(user_query & tag_query) +from aiarena.core.utils import filter_tags # Filter for the API views @@ -127,8 +84,8 @@ class MatchFilter(filters.FilterSet): requested_by = filters.NumberFilter(field_name="requested_by") map = filters.NumberFilter(field_name="map") bot = filters.NumberFilter(field_name="matchparticipation__bot") - tags = TagsFilter(field_name="tags__tag__name", field_name2="tags__user", lookup_expr='iexact') - tags__icontains = TagsFilter(field_name="tags__tag__name", field_name2="tags__user", lookup_expr='icontains') + tags = filters.CharFilter(method="filter_tags__exact") + tags__icontains = filters.CharFilter(method="filter_tags__icontains") class Meta: model = Match @@ -137,3 +94,9 @@ class Meta: 'created', 'started' ] + + def filter_tags__exact(self, qs, name, value): + return filter_tags(qs, value, "tags__tag__name", "iexact", "tags__user") + + def filter_tags__icontains(self, qs, name, value): + return filter_tags(qs, value, "tags__tag__name", "icontains", "tags__user") diff --git a/aiarena/core/utils.py b/aiarena/core/utils.py index 68958b08..2e4e7c5b 100644 --- a/aiarena/core/utils.py +++ b/aiarena/core/utils.py @@ -6,12 +6,88 @@ from urllib import request from enum import Enum from zipfile import ZipFile +from django.db.models.fields import CharField +from django.db.models import Q, Aggregate, CharField +from rest_framework.exceptions import ValidationError from aiarena import settings logger = logging.getLogger(__name__) +class GroupConcat(Aggregate): + function = 'GROUP_CONCAT' + template = '%(function)s(%(distinct)s%(expressions)s%(ordering)s%(separator)s)' + + def __init__(self, expression, distinct=False, ordering=None, separator=',', **extra): + super(GroupConcat, self).__init__( + expression, + distinct='DISTINCT ' if distinct else '', + ordering=' ORDER BY %s' % ordering if ordering is not None else '', + separator=' SEPARATOR "%s"' % separator, + output_field=CharField(), + **extra + ) + + +def filter_tags(qs, value, tags_field_name, tags_lookup_expr="iexact", user_field_name="", exclude=False): + """ + Given a string value, filter the queryset for tags found in it. + qs: queryset + value: the string to be parsed. If it contains a "|", the LHS of the pipe is treated as users list, RHS as tags, else all are tags. + """ + if not value: + return qs + + # Check for pipe separator + if '|' in value: + users_str, tags_str = [s.strip() for s in value.split('|')] + else: + users_str = "" + tags_str = value + + method = lambda q: q.filter if not exclude else q.exclude + + if user_field_name and users_str: + try: + users = [int(s) for s in users_str.split(',')] + except ValueError: + raise ValidationError({"tags":["When using pipe separator (|), Expecting user_id (int) on LHS and tag_name on RHS of separator."]}) + else: + users = [] + + # Build query for users + user_query = Q() + user_lookup = '%s__%s' % (user_field_name, 'exact') + for v in users: + user_query |= Q(**{user_lookup: v}) + + # Build query for tags + tag_query = Q() + if tags_str: + tags = [s.strip() for s in tags_str.split(',')] + tags = [s for s in tags if s] + if tags_lookup_expr == 'icontains': + qs = method(qs)(user_query).distinct() + # Create string with all tag names and run icontains on the string + qs = qs.annotate(all_tags=GroupConcat(tags_field_name)) + tags_lookup = '%s__%s' % ('all_tags', tags_lookup_expr) + for v in tags: + tag_query &= Q(**{tags_lookup: v}) + return method(qs)(tag_query) + else: + user_lookup = '%s__%s' % (user_field_name, "in") + tags_lookup = '%s__%s' % (tags_field_name, tags_lookup_expr) + for v in tags: + if users: + qs = qs.filter(**{tags_lookup: v, user_lookup:users}) + else: + qs = qs.filter(**{tags_lookup: v}) + return qs.distinct() + + return method(qs)(user_query).distinct() + + def parse_tags(tags): """convert tags from single string to list if applicable, and then cleans the tags""" if tags: diff --git a/aiarena/frontend/views.py b/aiarena/frontend/views.py index 86971a00..eacab9a4 100644 --- a/aiarena/frontend/views.py +++ b/aiarena/frontend/views.py @@ -38,7 +38,7 @@ ArenaClient, News, MapPool, MatchTag, Tag from aiarena.core.models import Trophy from aiarena.core.models.relative_result import RelativeResult -from aiarena.core.utils import parse_tags +from aiarena.core.utils import parse_tags, filter_tags from aiarena.frontend.utils import restrict_page_range from aiarena.patreon.models import PatreonAccountBind @@ -363,19 +363,18 @@ def filter_match_types(self, queryset, name, value): return queryset.filter(match__requested_by__isnull=False) return queryset - def filter_tags(self, queryset, name, value): + def filter_tags(self, qs, name, value): # An unauthenticated user will have no tags if not self.user.is_authenticated and not self.tags_by_all_value: - return queryset.none() - - tag_values = parse_tags(value) - # User or All - query = Q(match__tags__user=self.user) if not self.tags_by_all_value else Q() - # Full or Partial Match - lookup = "match__tags__tag__name__" + ("icontains" if self.tags_partial_match_value else "iexact") - for v in tag_values: - query = query & Q(**{lookup:v}) - return queryset.filter(query).distinct() + return qs.none() + + if not self.tags_by_all_value: + value = str(self.user.id) + "|" + value + + if self.tags_partial_match_value: + return filter_tags(qs, value, "match__tags__tag__name", "icontains", "match__tags__user") + else: + return filter_tags(qs, value, "match__tags__tag__name", "iexact", "match__tags__user") def no_filter(self, queryset, name, value): return queryset From 0095a11914ac40c73c55c58e9521a01906fd8f3c Mon Sep 17 00:00:00 2001 From: Nigel Chin Date: Thu, 1 Jul 2021 20:35:14 +1000 Subject: [PATCH 2/3] Moved filter_tags to fix CI fails --- aiarena/api/view_filters.py | 2 +- aiarena/core/d_utils.py | 80 +++++++++++++++++++++++++++++++++++++ aiarena/core/utils.py | 76 ----------------------------------- aiarena/frontend/views.py | 3 +- 4 files changed, 83 insertions(+), 78 deletions(-) create mode 100644 aiarena/core/d_utils.py diff --git a/aiarena/api/view_filters.py b/aiarena/api/view_filters.py index 4a76ea36..e943675a 100644 --- a/aiarena/api/view_filters.py +++ b/aiarena/api/view_filters.py @@ -1,7 +1,7 @@ from django_filters import rest_framework as filters from aiarena.core.models import Match, Result, Bot, Map, User, Round, MatchParticipation, CompetitionParticipation, Competition -from aiarena.core.utils import filter_tags +from aiarena.core.d_utils import filter_tags # Filter for the API views diff --git a/aiarena/core/d_utils.py b/aiarena/core/d_utils.py new file mode 100644 index 00000000..9e3542e2 --- /dev/null +++ b/aiarena/core/d_utils.py @@ -0,0 +1,80 @@ +import logging +from django.db.models.fields import CharField +from django.db.models import Q, Aggregate, CharField +from rest_framework.exceptions import ValidationError + +# File for housing utils that require 'django' or would break CI if placed in utils.py +logger = logging.getLogger(__name__) + + +class GroupConcat(Aggregate): + function = 'GROUP_CONCAT' + template = '%(function)s(%(distinct)s%(expressions)s%(ordering)s%(separator)s)' + + def __init__(self, expression, distinct=False, ordering=None, separator=',', **extra): + super(GroupConcat, self).__init__( + expression, + distinct='DISTINCT ' if distinct else '', + ordering=' ORDER BY %s' % ordering if ordering is not None else '', + separator=' SEPARATOR "%s"' % separator, + output_field=CharField(), + **extra + ) + + +def filter_tags(qs, value, tags_field_name, tags_lookup_expr="iexact", user_field_name="", exclude=False): + """ + Given a string value, filter the queryset for tags found in it. + qs: queryset + value: the string to be parsed. If it contains a "|", the LHS of the pipe is treated as users list, RHS as tags, else all are tags. + """ + if not value: + return qs + + # Check for pipe separator + if '|' in value: + users_str, tags_str = [s.strip() for s in value.split('|')] + else: + users_str = "" + tags_str = value + + method = lambda q: q.filter if not exclude else q.exclude + + if user_field_name and users_str: + try: + users = [int(s) for s in users_str.split(',')] + except ValueError: + raise ValidationError({"tags":["When using pipe separator (|), Expecting user_id (int) on LHS and tag_name on RHS of separator."]}) + else: + users = [] + + # Build query for users + user_query = Q() + user_lookup = '%s__%s' % (user_field_name, 'exact') + for v in users: + user_query |= Q(**{user_lookup: v}) + + # Build query for tags + tag_query = Q() + if tags_str: + tags = [s.strip() for s in tags_str.split(',')] + tags = [s for s in tags if s] + if tags_lookup_expr == 'icontains': + qs = method(qs)(user_query).distinct() + # Create string with all tag names and run icontains on the string + qs = qs.annotate(all_tags=GroupConcat(tags_field_name)) + tags_lookup = '%s__%s' % ('all_tags', tags_lookup_expr) + for v in tags: + tag_query &= Q(**{tags_lookup: v}) + return method(qs)(tag_query) + else: + user_lookup = '%s__%s' % (user_field_name, "in") + tags_lookup = '%s__%s' % (tags_field_name, tags_lookup_expr) + for v in tags: + if users: + qs = qs.filter(**{tags_lookup: v, user_lookup:users}) + else: + qs = qs.filter(**{tags_lookup: v}) + return qs.distinct() + + return method(qs)(user_query).distinct() diff --git a/aiarena/core/utils.py b/aiarena/core/utils.py index 2e4e7c5b..68958b08 100644 --- a/aiarena/core/utils.py +++ b/aiarena/core/utils.py @@ -6,88 +6,12 @@ from urllib import request from enum import Enum from zipfile import ZipFile -from django.db.models.fields import CharField -from django.db.models import Q, Aggregate, CharField -from rest_framework.exceptions import ValidationError from aiarena import settings logger = logging.getLogger(__name__) -class GroupConcat(Aggregate): - function = 'GROUP_CONCAT' - template = '%(function)s(%(distinct)s%(expressions)s%(ordering)s%(separator)s)' - - def __init__(self, expression, distinct=False, ordering=None, separator=',', **extra): - super(GroupConcat, self).__init__( - expression, - distinct='DISTINCT ' if distinct else '', - ordering=' ORDER BY %s' % ordering if ordering is not None else '', - separator=' SEPARATOR "%s"' % separator, - output_field=CharField(), - **extra - ) - - -def filter_tags(qs, value, tags_field_name, tags_lookup_expr="iexact", user_field_name="", exclude=False): - """ - Given a string value, filter the queryset for tags found in it. - qs: queryset - value: the string to be parsed. If it contains a "|", the LHS of the pipe is treated as users list, RHS as tags, else all are tags. - """ - if not value: - return qs - - # Check for pipe separator - if '|' in value: - users_str, tags_str = [s.strip() for s in value.split('|')] - else: - users_str = "" - tags_str = value - - method = lambda q: q.filter if not exclude else q.exclude - - if user_field_name and users_str: - try: - users = [int(s) for s in users_str.split(',')] - except ValueError: - raise ValidationError({"tags":["When using pipe separator (|), Expecting user_id (int) on LHS and tag_name on RHS of separator."]}) - else: - users = [] - - # Build query for users - user_query = Q() - user_lookup = '%s__%s' % (user_field_name, 'exact') - for v in users: - user_query |= Q(**{user_lookup: v}) - - # Build query for tags - tag_query = Q() - if tags_str: - tags = [s.strip() for s in tags_str.split(',')] - tags = [s for s in tags if s] - if tags_lookup_expr == 'icontains': - qs = method(qs)(user_query).distinct() - # Create string with all tag names and run icontains on the string - qs = qs.annotate(all_tags=GroupConcat(tags_field_name)) - tags_lookup = '%s__%s' % ('all_tags', tags_lookup_expr) - for v in tags: - tag_query &= Q(**{tags_lookup: v}) - return method(qs)(tag_query) - else: - user_lookup = '%s__%s' % (user_field_name, "in") - tags_lookup = '%s__%s' % (tags_field_name, tags_lookup_expr) - for v in tags: - if users: - qs = qs.filter(**{tags_lookup: v, user_lookup:users}) - else: - qs = qs.filter(**{tags_lookup: v}) - return qs.distinct() - - return method(qs)(user_query).distinct() - - def parse_tags(tags): """convert tags from single string to list if applicable, and then cleans the tags""" if tags: diff --git a/aiarena/frontend/views.py b/aiarena/frontend/views.py index eacab9a4..0cbc5b60 100644 --- a/aiarena/frontend/views.py +++ b/aiarena/frontend/views.py @@ -38,7 +38,8 @@ ArenaClient, News, MapPool, MatchTag, Tag from aiarena.core.models import Trophy from aiarena.core.models.relative_result import RelativeResult -from aiarena.core.utils import parse_tags, filter_tags +from aiarena.core.utils import parse_tags +from aiarena.core.d_utils import filter_tags from aiarena.frontend.utils import restrict_page_range from aiarena.patreon.models import PatreonAccountBind From 85533626fc35fec710dc0d2a37d66717f92e2e03 Mon Sep 17 00:00:00 2001 From: Nigel Chin Date: Thu, 1 Jul 2021 21:42:29 +1000 Subject: [PATCH 3/3] Change GroupConcat to output textfield to avoid length limits --- aiarena/core/d_utils.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/aiarena/core/d_utils.py b/aiarena/core/d_utils.py index 9e3542e2..e06b6556 100644 --- a/aiarena/core/d_utils.py +++ b/aiarena/core/d_utils.py @@ -1,6 +1,6 @@ import logging -from django.db.models.fields import CharField -from django.db.models import Q, Aggregate, CharField +from django.db.models.fields import CharField, TextField +from django.db.models import Q, Aggregate from rest_framework.exceptions import ValidationError # File for housing utils that require 'django' or would break CI if placed in utils.py @@ -11,13 +11,12 @@ class GroupConcat(Aggregate): function = 'GROUP_CONCAT' template = '%(function)s(%(distinct)s%(expressions)s%(ordering)s%(separator)s)' - def __init__(self, expression, distinct=False, ordering=None, separator=',', **extra): + def __init__(self, expression, ordering=None, separator=',', **extra): super(GroupConcat, self).__init__( expression, - distinct='DISTINCT ' if distinct else '', ordering=' ORDER BY %s' % ordering if ordering is not None else '', separator=' SEPARATOR "%s"' % separator, - output_field=CharField(), + output_field=TextField(), **extra )