[Patchew-devel] [PATCH 14/17] models: introduce flags into Messages
Paolo Bonzini
pbonzini at redhat.com
Thu May 2 11:18:01 UTC 2019
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
More information about the Patchew-devel
mailing list