From ac5e163dc56a6b0f6b0d3d4d07ac2d07e5b6c6c6 Mon Sep 17 00:00:00 2001 From: Anler Hp Date: Wed, 11 Jun 2014 18:07:13 +0200 Subject: [PATCH] Implement tags using pg arrays --- requirements.txt | 1 + taiga/base/filters.py | 11 +-- taiga/base/tags.py | 110 +++++++++++++++++++++++++++ taiga/base/utils/db.py | 10 --- taiga/projects/issues/api.py | 5 +- taiga/projects/issues/models.py | 6 +- taiga/projects/models.py | 6 +- taiga/projects/serializers.py | 1 - taiga/projects/services/filters.py | 30 +++----- taiga/projects/tasks/models.py | 6 +- taiga/projects/userstories/models.py | 8 +- tests/factories.py | 47 +++++++++--- tests/integration/test_neighbors.py | 10 +-- tests/integration/test_tags.py | 25 ++++++ 14 files changed, 204 insertions(+), 72 deletions(-) create mode 100644 taiga/base/tags.py create mode 100644 tests/integration/test_tags.py diff --git a/requirements.txt b/requirements.txt index 96142f5b..d625be94 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,6 +11,7 @@ pytz==2014.4 six==1.6.1 djmail==0.7 django-pgjson==0.1.2 +djorm-pgarray==1.0 django-jinja==1.0.1 jinja2==2.7.2 pygments==1.6rc1 diff --git a/taiga/base/filters.py b/taiga/base/filters.py index 424f4e87..b959887c 100644 --- a/taiga/base/filters.py +++ b/taiga/base/filters.py @@ -19,7 +19,7 @@ from django.db.models import Q from rest_framework import filters -from taiga.base.utils.db import filter_by_tags +from taiga.base import tags class QueryParamsFilterMixin(object): @@ -100,14 +100,11 @@ class TagsFilter(FilterBackend): self.filter_name = filter_name def _get_tags_queryparams(self, params): - tags = params.get(self.filter_name, []) - if tags: - tags = list({tag.strip() for tag in tags.split(",")}) - return tags + return params.get(self.filter_name, "") def filter_queryset(self, request, queryset, view): - tags = self._get_tags_queryparams(request.QUERY_PARAMS) + query_tags = self._get_tags_queryparams(request.QUERY_PARAMS) if tags: - queryset = filter_by_tags(tags, queryset) + queryset = tags.filter(queryset, contains=query_tags) return queryset diff --git a/taiga/base/tags.py b/taiga/base/tags.py new file mode 100644 index 00000000..4ca2feba --- /dev/null +++ b/taiga/base/tags.py @@ -0,0 +1,110 @@ +import re +from functools import partial + +import six + +from django.db import models +from django.utils.translation import ugettext_lazy as _ + +from djorm_pgarray.fields import TextArrayField + +from picklefield.fields import PickledObjectField + + +class TaggedMixin(models.Model): + pgtags = TextArrayField(default=None, verbose_name=_("tags")) + tags = PickledObjectField(null=False, blank=True, verbose_name=_("tags")) + + class Meta: + abstract = True + + +def get_queryset_table(queryset): + """Return queryset model's table name""" + return queryset.model._meta.db_table + + +def _filter_bin(queryset, value, operator): + """tags """ + if not isinstance(value, str): + value = ",".join(value) + + sql = "{table_name}.tags {operator} string_to_array(%s, ',')" + where_clause = sql.format(table_name=get_queryset_table(queryset), operator=operator) + queryset = queryset.extra(where=[where_clause], params=[value]) + return queryset +_filter_contains = partial(_filter_bin, operator="@>") +_filter_contained_by = partial(_filter_bin, operator="<@") +_filter_overlap = partial(_filter_bin, operator="&&") + + +def _filter_index(queryset, index, value): + """tags[] == """ + sql = "{table_name}.tags[{index}] = %s" + where_clause = sql.format(table_name=get_queryset_table(queryset), index=index) + queryset = queryset.extra(where=[where_clause], params=[value]) + return queryset + + +def _filter_len(queryset, value): + """len(tags) == """ + sql = "array_length({table_name}.tags, 1) = %s" + where_clause = sql.format(table_name=get_queryset_table(queryset)) + queryset = queryset.extra(where=[where_clause], params=[value]) + return queryset + + +def _filter_len_operator(queryset, value, operator): + """len(tags) """ + operator = {"gt": ">", "lt": "<", "gte": ">=", "lte": "<="}[operator] + sql = "array_length({table_name}.tags, 1) {operator} %s" + where_clause = sql.format(table_name=get_queryset_table(queryset), operator=operator) + queryset = queryset.extra(where=[where_clause], params=[value]) + return queryset + + +def _filter_index_operator(queryset, value, operator): + """tags[] == value""" + index = int(operator) + 1 + sql = "{table_name}.tags[{index}] = %s" + where_clause = sql.format(table_name=get_queryset_table(queryset), index=index) + queryset = queryset.extra(where=[where_clause], params=[value]) + return queryset + + +def _tags_filter(**filters_map): + filter_re = re.compile(r"""(?:(len__)(gte|lte|lt|gt) + | + (index__)(\d+))""", re.VERBOSE) + + def get_filter(filter_name, strict=False): + return filters_map[filter_name] if strict else filters_map.get(filter_name) + + def get_filter_matching(filter_name): + match = filter_re.search(filter_name) + filter_name, operator = (group for group in match.groups() if group) + return partial(get_filter(filter_name, strict=True), operator=operator) + + def tags_filter(model_or_qs, **filters): + "Filter a queryset but adding support to filters that work with postgresql array fields" + if hasattr(model_or_qs, "_meta"): + qs = model_or_qs._default_manager.get_queryset() + else: + qs = model_or_qs + + for filter_name, filter_value in six.iteritems(filters): + try: + filter = get_filter(filter_name) or get_filter_matching(filter_name) + except (LookupError, AttributeError): + qs = qs.filter(**{filter_name: filter_value}) + else: + qs = filter(queryset=qs, value=filter_value) + return qs + + return tags_filter +filter = _tags_filter(contains=_filter_contains, + contained_by=_filter_contained_by, + overlap=_filter_overlap, + len=_filter_len, + len__=_filter_len_operator, + index__=_filter_index_operator) diff --git a/taiga/base/utils/db.py b/taiga/base/utils/db.py index a748c6fd..f1f2f04b 100644 --- a/taiga/base/utils/db.py +++ b/taiga/base/utils/db.py @@ -16,16 +16,6 @@ from django.contrib.contenttypes.models import ContentType -FILTER_TAGS_SQL = "unpickle({table}.tags) && %s" - - -def filter_by_tags(tags, queryset): - """Filter a queryset of a model with pickled field named tags, by tags.""" - table_name = queryset.model._meta.db_table - where_sql = FILTER_TAGS_SQL.format(table=table_name) - - return queryset.extra(where=[where_sql], params=[tags]) - def get_typename_for_model_class(model:object, for_concrete_model=True) -> str: """ diff --git a/taiga/projects/issues/api.py b/taiga/projects/issues/api.py index d8359f7d..6f250553 100644 --- a/taiga/projects/issues/api.py +++ b/taiga/projects/issues/api.py @@ -26,6 +26,7 @@ from taiga.base import filters from taiga.base import exceptions as exc from taiga.base.decorators import list_route, detail_route from taiga.base.api import ModelCrudViewSet +from taiga.base import tags from taiga.projects.notifications import WatchedResourceMixin from taiga.projects.occ import OCCResourceMixin @@ -75,9 +76,7 @@ class IssuesFilter(filters.FilterBackend): filterdata = self._prepare_filters_data(request) if "tags" in filterdata: - where_sql = ["unpickle(issues_issue.tags) @> %s"] - params = [filterdata["tags"]] - queryset = queryset.extra(where=where_sql, params=params) + queryset = tags.filter(queryset, contains=filterdata["tags"]) for name, value in filter(lambda x: x[0] != "tags", filterdata.items()): if None in value: diff --git a/taiga/projects/issues/models.py b/taiga/projects/issues/models.py index 2032986f..c289581b 100644 --- a/taiga/projects/issues/models.py +++ b/taiga/projects/issues/models.py @@ -21,15 +21,14 @@ from django.utils import timezone from django.dispatch import receiver from django.utils.translation import ugettext_lazy as _ -from picklefield.fields import PickledObjectField - +from taiga.base.tags import TaggedMixin from taiga.base.utils.slug import ref_uniquely from taiga.projects.notifications import WatchedModelMixin from taiga.projects.occ import OCCModelMixin from taiga.projects.mixins.blocked import BlockedMixin -class Issue(OCCModelMixin, WatchedModelMixin, BlockedMixin, models.Model): +class Issue(OCCModelMixin, WatchedModelMixin, BlockedMixin, TaggedMixin): ref = models.BigIntegerField(db_index=True, null=True, blank=True, default=None, verbose_name=_("ref")) owner = models.ForeignKey(settings.AUTH_USER_MODEL, null=True, blank=True, default=None, @@ -59,7 +58,6 @@ class Issue(OCCModelMixin, WatchedModelMixin, BlockedMixin, models.Model): assigned_to = models.ForeignKey(settings.AUTH_USER_MODEL, blank=True, null=True, default=None, related_name="issues_assigned_to_me", verbose_name=_("assigned to")) - tags = PickledObjectField(null=False, blank=True, verbose_name=_("tags")) attachments = generic.GenericRelation("attachments.Attachment") class Meta: diff --git a/taiga/projects/models.py b/taiga/projects/models.py index b982e562..559661a6 100644 --- a/taiga/projects/models.py +++ b/taiga/projects/models.py @@ -30,6 +30,7 @@ from django.utils import timezone from picklefield.fields import PickledObjectField from django_pgjson.fields import JsonField +from taiga.base.tags import TaggedMixin from taiga.users.models import Role from taiga.base.utils.slug import slugify_uniquely from taiga.base.utils.dicts import dict_sum @@ -109,7 +110,7 @@ class ProjectDefaults(models.Model): abstract = True -class Project(ProjectDefaults, models.Model): +class Project(ProjectDefaults, TaggedMixin): name = models.CharField(max_length=250, unique=True, null=False, blank=False, verbose_name=_("name")) slug = models.SlugField(max_length=250, unique=True, null=False, blank=True, @@ -130,7 +131,6 @@ class Project(ProjectDefaults, models.Model): verbose_name=_("total of milestones")) total_story_points = models.FloatField(default=None, null=True, blank=False, verbose_name=_("total story points")) - tags = PickledObjectField(null=False, blank=True, verbose_name=_("tags")) is_backlog_activated = models.BooleanField(default=True, null=False, blank=True, verbose_name=_("active backlog panel")) @@ -186,6 +186,8 @@ class Project(ProjectDefaults, models.Model): # Get all available roles on this project roles = self.get_roles().filter(computable=True) + if len(roles) == 0: + return # Do nothing if project does not have roles if len(roles) == 0: diff --git a/taiga/projects/serializers.py b/taiga/projects/serializers.py index b231e07e..9c34987a 100644 --- a/taiga/projects/serializers.py +++ b/taiga/projects/serializers.py @@ -83,7 +83,6 @@ class ProjectMembershipSerializer(serializers.ModelSerializer): class ProjectSerializer(serializers.ModelSerializer): - tags = PickleField(required=False) stars = serializers.SerializerMethodField("get_stars_number") class Meta: diff --git a/taiga/projects/services/filters.py b/taiga/projects/services/filters.py index 82a45e62..c779c2cb 100644 --- a/taiga/projects/services/filters.py +++ b/taiga/projects/services/filters.py @@ -19,29 +19,18 @@ from django.db import connection def _get_stories_tags(project): - extra_sql = ("select unnest(unpickle(tags)) as tagname, count(unnest(unpickle(tags))) " - "from userstories_userstory where project_id = %s " - "group by unnest(unpickle(tags)) " - "order by tagname asc") - - with closing(connection.cursor()) as cursor: - cursor.execute(extra_sql, [project.id]) - rows = cursor.fetchall() - - return set([x[0] for x in rows]) + result = set() + for tags in project.user_stories.values("tags", flat=True): + result.update(tags) + return result def _get_issues_tags(project): - extra_sql = ("select unnest(unpickle(tags)) as tagname, count(unnest(unpickle(tags))) " - "from issues_issue where project_id = %s " - "group by unnest(unpickle(tags)) " - "order by tagname asc") + result = set() + for tags in project.issues.values("tags", flat=True): + result.update(tags) + return result - with closing(connection.cursor()) as cursor: - cursor.execute(extra_sql, [project.id]) - rows = cursor.fetchall() - - return rows def _get_issues_statuses(project): extra_sql = ("select status_id, count(status_id) from issues_issue " @@ -144,9 +133,8 @@ def get_all_tags(project): Given a project, return sorted list of unique tags found on it. """ - result = set() - result.update(x[0] for x in _get_issues_tags(project)) + result.update(_get_issues_tags(project)) result.update(_get_stories_tags(project)) return sorted(result) diff --git a/taiga/projects/tasks/models.py b/taiga/projects/tasks/models.py index 8b8f4e0a..f27a0f86 100644 --- a/taiga/projects/tasks/models.py +++ b/taiga/projects/tasks/models.py @@ -21,8 +21,7 @@ from django.utils import timezone from django.dispatch import receiver from django.utils.translation import ugettext_lazy as _ -from picklefield.fields import PickledObjectField - +from taiga.base.tags import TaggedMixin from taiga.base.utils.slug import ref_uniquely from taiga.projects.notifications import WatchedModelMixin from taiga.projects.occ import OCCModelMixin @@ -31,7 +30,7 @@ from taiga.projects.milestones.models import Milestone from taiga.projects.mixins.blocked import BlockedMixin -class Task(OCCModelMixin, WatchedModelMixin, BlockedMixin, models.Model): +class Task(OCCModelMixin, WatchedModelMixin, BlockedMixin, TaggedMixin): user_story = models.ForeignKey("userstories.UserStory", null=True, blank=True, related_name="tasks", verbose_name=_("user story")) ref = models.BigIntegerField(db_index=True, null=True, blank=True, default=None, @@ -57,7 +56,6 @@ class Task(OCCModelMixin, WatchedModelMixin, BlockedMixin, models.Model): assigned_to = models.ForeignKey(settings.AUTH_USER_MODEL, blank=True, null=True, default=None, related_name="tasks_assigned_to_me", verbose_name=_("assigned to")) - tags = PickledObjectField(null=False, blank=True, verbose_name=_("tags")) attachments = generic.GenericRelation("attachments.Attachment") is_iocaine = models.BooleanField(default=False, null=False, blank=True, verbose_name=_("is iocaine")) diff --git a/taiga/projects/userstories/models.py b/taiga/projects/userstories/models.py index 300ce3ca..c071d8b6 100644 --- a/taiga/projects/userstories/models.py +++ b/taiga/projects/userstories/models.py @@ -20,8 +20,7 @@ from django.conf import settings from django.dispatch import receiver from django.utils.translation import ugettext_lazy as _ -from picklefield.fields import PickledObjectField - +from taiga.base.tags import TaggedMixin from taiga.base.utils.slug import ref_uniquely from taiga.projects.notifications import WatchedModelMixin from taiga.projects.occ import OCCModelMixin @@ -52,7 +51,8 @@ class RolePoints(models.Model): return "{}: {}".format(self.role.name, self.points.name) -class UserStory(OCCModelMixin, WatchedModelMixin, BlockedMixin, models.Model): +class UserStory(OCCModelMixin, WatchedModelMixin, BlockedMixin, TaggedMixin): + ref = models.BigIntegerField(db_index=True, null=True, blank=True, default=None, verbose_name=_("ref")) milestone = models.ForeignKey("milestones.Milestone", null=True, blank=True, @@ -90,8 +90,6 @@ class UserStory(OCCModelMixin, WatchedModelMixin, BlockedMixin, models.Model): verbose_name=_("is client requirement")) team_requirement = models.BooleanField(default=False, null=False, blank=True, verbose_name=_("is team requirement")) - tags = PickledObjectField(null=False, blank=True, - verbose_name=_("tags")) attachments = generic.GenericRelation("attachments.Attachment") generated_from_issue = models.ForeignKey("issues.Issue", null=True, blank=True, related_name="generated_user_stories", diff --git a/tests/factories.py b/tests/factories.py index 9cdf991e..90625b90 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -6,14 +6,6 @@ from django.db.models.loading import get_model import factory from django.conf import settings -# import taiga.projects.models -# import taiga.projects.userstories.models -# import taiga.projects.issues.models -# import taiga.projects.milestones.models -# import taiga.projects.stars.models -# import taiga.users.models -# import taiga.userstorage.models - class Factory(factory.DjangoModelFactory): FACTORY_STRATEGY = factory.CREATE_STRATEGY @@ -113,6 +105,25 @@ class UserStoryFactory(Factory): description = factory.Sequence(lambda n: "User Story {} description".format(n)) +class TaskFactory(Factory): + FACTORY_FOR = get_model("tasks", "Task") + + ref = factory.Sequence(lambda n: n) + owner = factory.SubFactory("tests.factories.UserFactory") + subject = factory.Sequence(lambda n: "Task {}".format(n)) + user_story = factory.SubFactory("tests.factories.UserStoryFactory") + status = factory.SubFactory("tests.factories.TaskStatusFactory") + project = factory.SubFactory("tests.factories.ProjectFactory") + milestone = factory.SubFactory("tests.factories.MilestoneFactory") + + +class TaskStatusFactory(Factory): + FACTORY_FOR = get_model("projects", "TaskStatus") + + name = factory.Sequence(lambda n: "Task status {}".format(n)) + project = factory.SubFactory("tests.factories.ProjectFactory") + + class MilestoneFactory(Factory): FACTORY_FOR = get_model("milestones", "Milestone") @@ -228,8 +239,8 @@ class ContentTypeFactory(Factory): def create_issue(**kwargs): - "Create an issue and its dependencies in an appropriate way." - owner = kwargs.pop("owner") if "owner" in kwargs else UserFactory() + "Create an issue and along with its dependencies." + owner = kwargs.pop("owner", UserFactory()) project = ProjectFactory.create(owner=owner) defaults = { "project": project, @@ -243,3 +254,19 @@ def create_issue(**kwargs): defaults.update(kwargs) return IssueFactory.create(**defaults) + + +def create_task(**kwargs): + "Create a task and along with its dependencies." + owner = kwargs.pop("owner", UserFactory()) + project = ProjectFactory.create(owner=owner) + defaults = { + "project": project, + "owner": owner, + "status": TaskStatusFactory.create(project=project), + "milestone": MilestoneFactory.create(project=project), + "user_story": UserStoryFactory.create(project=project, owner=owner), + } + defaults.update(kwargs) + + return TaskFactory.create(**defaults) diff --git a/tests/integration/test_neighbors.py b/tests/integration/test_neighbors.py index 5b91309e..f593c049 100644 --- a/tests/integration/test_neighbors.py +++ b/tests/integration/test_neighbors.py @@ -5,7 +5,7 @@ import pytest from taiga.projects.userstories.models import UserStory from taiga.projects.issues.models import Issue -from taiga.base.utils.db import filter_by_tags +from taiga.base import tags from taiga.base import neighbors as n from .. import factories as f @@ -77,14 +77,14 @@ class TestUserStories: assert neighbors.right == us3 def test_filtered_by_tags(self): - tags = ["test"] + tag_names = ["test"] project = f.ProjectFactory.create() f.UserStoryFactory.create(project=project) - us1 = f.UserStoryFactory.create(project=project, tags=tags) - us2 = f.UserStoryFactory.create(project=project, tags=tags) + us1 = f.UserStoryFactory.create(project=project, tags=tag_names) + us2 = f.UserStoryFactory.create(project=project, tags=tag_names) - test_user_stories = filter_by_tags(tags, queryset=UserStory.objects.get_queryset()) + test_user_stories = tags.filter(UserStory.objects.get_queryset(), contains=tag_names) neighbors = n.get_neighbors(us1, results_set=test_user_stories) diff --git a/tests/integration/test_tags.py b/tests/integration/test_tags.py new file mode 100644 index 00000000..3b8c0831 --- /dev/null +++ b/tests/integration/test_tags.py @@ -0,0 +1,25 @@ +import pytest + +from taiga.base import tags + +pytestmark = pytest.mark.django_db + + +class TaggedModel(tags.TaggedMixin): + class Meta: + app_label = "base" + + +def test(): + tags1 = TaggedModel.objects.create(tags=["foo", "bar"]) + tags2 = TaggedModel.objects.create(tags=["foo"]) + + assert list(tags.filter(TaggedModel, contained_by=["foo"])) == [tags2] + assert list(tags.filter(TaggedModel, overlap=["bar"])) == [tags1] + + assert list(tags.filter(TaggedModel, len=2)) == [tags1] + assert list(tags.filter(TaggedModel, len__gte=1)) == [tags1, tags2] + assert list(tags.filter(TaggedModel, len__lt=2)) == [tags2] + + assert list(tags.filter(TaggedModel, index__1="bar")) == [tags1] + assert list(tags.filter(TaggedModel, index__1="bar", id__isnull=False)) == [tags1]