[Patchew-devel] [PATCH 14/17] models: introduce flags into Messages

Paolo Bonzini posted 17 patches 2 years, 6 months ago

[Patchew-devel] [PATCH 14/17] models: introduce flags into Messages

Posted by Paolo Bonzini 2 years, 6 months ago
Flags are used as a replacement for Boolean properties that are used by
search queries.  They can be searched efficiently using a trigram index.
---
 api/migrations/0048_message_flags.py          | 20 +++++++++
 api/migrations/0049_populate_message_flags.py | 42 +++++++++++++++++++
 .../0050_message_flags_postgres_fts.py        | 22 ++++++++++
 api/models.py                                 | 13 ++++++
 api/search.py                                 | 13 +++---
 mods/tags.py                                  |  5 ++-
 mods/testing.py                               | 22 +++++-----
 tests/test_testing.py                         |  9 ++--
 8 files changed, 124 insertions(+), 22 deletions(-)
 create mode 100644 api/migrations/0048_message_flags.py
 create mode 100644 api/migrations/0049_populate_message_flags.py
 create mode 100644 api/migrations/0050_message_flags_postgres_fts.py

diff --git a/api/migrations/0048_message_flags.py b/api/migrations/0048_message_flags.py
new file mode 100644
index 0000000..00d05c4
--- /dev/null
+++ b/api/migrations/0048_message_flags.py
@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.20 on 2019-04-18 14:30
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0047_populate_project_config'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='message',
+            name='flags',
+            field=models.CharField(blank=True, default='', max_length=64),
+        ),
+    ]
diff --git a/api/migrations/0049_populate_message_flags.py b/api/migrations/0049_populate_message_flags.py
new file mode 100644
index 0000000..e8a77be
--- /dev/null
+++ b/api/migrations/0049_populate_message_flags.py
@@ -0,0 +1,42 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations
+
+from api.search import FLAG_TESTED, FLAG_REVIEWED, FLAG_OBSOLETE
+
+def property_to_flags(apps, schema_editor):
+    MessageProperty = apps.get_model('api', 'MessageProperty')
+    for p in MessageProperty.objects.filter(name='obsoleted-by'):
+        p.message.flags += FLAG_OBSOLETE
+        p.message.save()
+    for p in MessageProperty.objects.filter(name='reviewed'):
+        p.message.flags += FLAG_REVIEWED
+        p.message.save()
+    for p in MessageProperty.objects.filter(name='testing.done'):
+        p.message.flags += FLAG_TESTED
+        p.message.save()
+    MessageProperty.objects.filter(name='reviewed').delete()
+    MessageProperty.objects.filter(name='testing.done').delete()
+
+def flags_to_property(apps, schema_editor):
+    Message = apps.get_model('api', 'Message')
+    for m in Message.objects.exclude(flags=''):
+        if '[reviewed]' in m.flags:
+            new_prop = MessageProperty(message=p.message, name='reviewed', value=True)
+            new_prop.save()
+        if FLAG_TESTED in m.flags:
+            new_prop = MessageProperty(message=p.message, name='testing.done', value=True)
+            new_prop.save()
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0048_message_flags'),
+    ]
+
+    operations = [
+        migrations.RunPython(property_to_flags,
+                             reverse_code=flags_to_property),
+    ]
diff --git a/api/migrations/0050_message_flags_postgres_fts.py b/api/migrations/0050_message_flags_postgres_fts.py
new file mode 100644
index 0000000..e6137d0
--- /dev/null
+++ b/api/migrations/0050_message_flags_postgres_fts.py
@@ -0,0 +1,22 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2018-11-07 15:28
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+from django.contrib.postgres.operations import TrigramExtension
+
+from api.migrations import PostgresOnlyMigration
+
+
+class Migration(PostgresOnlyMigration):
+
+    dependencies = [
+        ('api', '0049_populate_message_flags'),
+    ]
+
+    operations = [
+        TrigramExtension(),
+        migrations.RunSQL("create index api_message_flags_gin on api_message using gin(flags gin_trgm_ops);",
+                          "drop index api_message_flags_gin"),
+    ]
diff --git a/api/models.py b/api/models.py
index 68e90e1..67d1ed5 100644
--- a/api/models.py
+++ b/api/models.py
@@ -519,6 +519,7 @@ class Message(models.Model):
     objects = MessageManager()
 
     maintainers = jsonfield.JSONField(blank=True, default=[])
+    flags = models.CharField(max_length=64, blank=True, default="")
 
     def save_mbox(self, mbox_blob):
         save_blob(mbox_blob, self.message_id)
@@ -607,6 +608,18 @@ class Message(models.Model):
             self.refresh_num_patches()
         return self.num_patches
 
+    def add_flag(self, flag):
+        assert flag[0] == '[' and flag[-1] == ']'
+        if not flag in self.flags:
+            self.flags += flag
+            self.save()
+
+    def remove_flag(self, flag):
+        assert flag[0] == '[' and flag[-1] == ']'
+        if flag in self.flags:
+            self.flags = self.flags.replace(flag, '')
+            self.save()
+
     def get_property(self, prop, default=None):
         return self.get_properties().get(prop, default)
 
diff --git a/api/search.py b/api/search.py
index baf6893..1c5aaaf 100644
--- a/api/search.py
+++ b/api/search.py
@@ -18,6 +18,10 @@ from django.contrib.postgres.search import SearchQuery, SearchVector, SearchVect
 from django.db.models import Lookup
 from django.db.models.fields import Field
 
+# These are used by migrations.  Do not change them!
+FLAG_REVIEWED = '[reviewed]'
+FLAG_OBSOLETE = '[obsolete]'
+FLAG_TESTED = '[tested]'
 
 @Field.register_lookup
 class NotEqual(Lookup):
@@ -254,16 +258,13 @@ Search text keyword in the email message. Example:
             self._add_to_keywords('PULL')
             return Q(subject__contains='[PULL') | Q(subject__contains='[GIT PULL')
         elif cond == "reviewed":
-            return self._make_filter_subquery(MessageProperty, Q(name="reviewed", value=True))
+            return Q(flags__contains=FLAG_REVIEWED)
         elif cond in ("obsoleted", "old"):
-            return self._make_filter_subquery(
-                MessageProperty,
-                Q(name="obsoleted-by", value__isnull=False) & ~Q(name="obsoleted-by", value__iexact='')
-            )
+            return Q(flags__contains=FLAG_OBSOLETE)
         elif cond == "applied":
             return self._make_filter_subquery(MessageResult, Q(name="git", status=Result.SUCCESS))
         elif cond == "tested":
-            return self._make_filter_subquery(MessageProperty, Q(name="testing.done", value=True))
+            return Q(flags__contains=FLAG_TESTED)
         elif cond == "merged":
             return Q(is_merged=True)
         return None
diff --git a/mods/tags.py b/mods/tags.py
index 02c493e..6b5bcc9 100644
--- a/mods/tags.py
+++ b/mods/tags.py
@@ -13,6 +13,7 @@ from mbox import addr_db_to_rest, parse_address
 from event import register_handler, emit_event, declare_event
 from api.models import Message
 from api.rest import PluginMethodField
+from api.search import FLAG_OBSOLETE, FLAG_REVIEWED
 import rest_framework
 
 REV_BY_PREFIX = "Reviewed-by:"
@@ -73,8 +74,10 @@ series cover letter, patch mail body and their replies.
             return m1.version > m2.version and m1.date >= m2.date
         for m in series.get_alternative_revisions():
             if newer_than(m, series):
+                series.add_flag(FLAG_OBSOLETE)
                 series.set_property("obsoleted-by", m.message_id)
             elif newer_than(series, m):
+                m.add_flag(FLAG_OBSOLETE)
                 m.set_property("obsoleted-by", series.message_id)
 
         updated = self.update_tags(series)
@@ -100,7 +103,7 @@ series cover letter, patch mail body and their replies.
         series_reviewers = _find_reviewers(series)
         reviewers = reviewers.union(series_reviewers)
         if num_reviewed == series.get_num()[1] or series_reviewers:
-            series.set_property("reviewed", True)
+            series.add_flag(FLAG_REVIEWED)
             series.set_property("reviewers", list(reviewers))
         if updated:
             emit_event("TagsUpdate", series=series)
diff --git a/mods/testing.py b/mods/testing.py
index 2c45001..969ce84 100644
--- a/mods/testing.py
+++ b/mods/testing.py
@@ -22,7 +22,7 @@ from api.views import APILoginRequiredView
 from api.models import (Message, MessageProperty, MessageResult,
         Project, ProjectResult, Result)
 from api.rest import PluginMethodField, reverse_detail
-from api.search import SearchEngine
+from api.search import SearchEngine, FLAG_TESTED
 from event import emit_event, declare_event, register_handler
 from patchew.logviewer import LogView
 from schema import *
@@ -138,9 +138,9 @@ class TestingModule(PatchewModule):
                 _instance.tester_check_in(po, result.data['tester'])
             if not self.get_testing_results(obj,
                            status__in=(Result.PENDING, Result.RUNNING)).exists():
-                obj.set_property("testing.done", True)
                 obj.set_property("testing.tested-head", result.data["head"])
                 if isinstance(obj, Message):
+                    obj.add_flag(FLAG_TESTED)
                     obj.set_property("testing.tested-base",
                                      self.get_msg_base_tags(obj))
             if isinstance(obj, Project):
@@ -188,10 +188,11 @@ class TestingModule(PatchewModule):
             for tn in all_tests:
                 if not tn in done_tests:
                     obj.create_result(name='testing.' + tn, status=Result.PENDING).save()
-            if len(done_tests) < len(all_tests):
-                obj.set_property("testing.done", None)
-                return
-        obj.set_property("testing.done", True)
+        if isinstance(obj, Message):
+            if len(all_tests) and len(done_tests) == len(all_tests):
+                obj.add_flag(FLAG_TESTED)
+            else:
+                obj.remove_flag(FLAG_TESTED)
 
     def project_recalc_pending_tests(self, project):
         self.recalc_pending_tests(project)
@@ -204,10 +205,9 @@ class TestingModule(PatchewModule):
             self.recalc_pending_tests(obj)
 
     def clear_and_start_testing(self, obj, test=""):
-        for k in list(obj.get_properties().keys()):
-            if k == "testing.done" or \
-               k == "testing.tested-head":
-                obj.set_property(k, None)
+        obj.set_property("testing.tested-head", None)
+        if isinstance(obj, Message):
+            obj.remove_flag(FLAG_TESTED)
         if test:
             r = self.get_testing_result(obj, test)
             if r:
@@ -342,7 +342,7 @@ class TestingModule(PatchewModule):
                 "type": "danger",
                 "char": "T",
                 })
-        elif message.get_property("testing.done"):
+        elif FLAG_TESTED in message.flags:
             message.status_tags.append({
                 "title": "Testing passed",
                 "url": reverse("series_detail",
diff --git a/tests/test_testing.py b/tests/test_testing.py
index 2a01b7a..efdda71 100755
--- a/tests/test_testing.py
+++ b/tests/test_testing.py
@@ -12,6 +12,7 @@ import abc
 import subprocess
 
 from api.models import Message, Result
+from api.search import FLAG_TESTED
 
 from .patchewtest import PatchewTestCase, main
 
@@ -69,7 +70,6 @@ class TestingTestCase(PatchewTestCase, metaclass=abc.ABCMeta):
         if 'status' not in kwargs:
             kwargs['status'] = Result.SUCCESS
         self.modify_test_result(obj, **kwargs)
-        obj.set_property("testing.done", True)
 
     def do_testing_report(self, **report):
         self.api_login()
@@ -180,6 +180,7 @@ class MessageTestingTest(TestingTestCase):
 
     def do_testing_done(self, **kwargs):
         self._do_testing_done(self.msg, **kwargs)
+        self.msg.add_flag(FLAG_TESTED)
 
     def do_testing_report(self, **report):
         r = super(MessageTestingTest, self).do_testing_report(**report)
@@ -364,7 +365,7 @@ class TestingResetTest(PatchewTestCase):
                                   "testing.a": Result.SUCCESS,
                                   "testing.b": Result.SUCCESS,
                                   "testing.c": Result.FAILURE})
-        self.assertTrue(msg.get_property("testing.done"))
+        self.assertIn(FLAG_TESTED, msg.flags)
 
         self.api_login()
         self.client.post('/login/', {'username': self.user, 'password': self.password})
@@ -375,7 +376,7 @@ class TestingResetTest(PatchewTestCase):
                                   "testing.a": Result.PENDING,
                                   "testing.b": Result.SUCCESS,
                                   "testing.c": Result.FAILURE})
-        self.assertFalse(msg.get_property("testing.done"))
+        self.assertNotIn(FLAG_TESTED, msg.flags)
 
         self.client.get('/testing-reset/%s/?type=message&test=b' % msg.message_id)
         self.client.get('/testing-reset/%s/?type=message&test=c' % msg.message_id)
@@ -383,7 +384,7 @@ class TestingResetTest(PatchewTestCase):
                                   "testing.a": Result.PENDING,
                                   "testing.b": Result.PENDING,
                                   "testing.c": Result.PENDING})
-        self.assertFalse(msg.get_property("testing.done"))
+        self.assertNotIn(FLAG_TESTED, msg.flags)
 
 
 class TestingDisableTest(PatchewTestCase):
-- 
2.21.0


_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel