Stephen Finucane <step...@that.guru> writes: > This allows us to remove swathes of ForeignKeys and greatly speed up the > '/events' endpoint. This comes at the disadvantage of preventing us > indexing the embedded fields and would result in different responses if > the serializers ever change; however, we don't do the former at the > moment and it's unlikely that the latter will happen regularly (and can > be addressed by a migration if absolutely necessary). > > Note that to generated the JSON, we use the same serializers we normally > use for embedded responses. This has the advantage of minimizing the > amount of new code needed and handles a lot of the ugliness of > serialization. However, this does necessitate using Django REST > Framework for events. This has been addressed in a previous patch. > > Signed-off-by: Stephen Finucane <step...@that.guru> > Closes-bug: #153 ("'/events' is slow") > --- > patchwork/api/event.py | 110 > +++++++++++++-------- > patchwork/fields.py | 32 ++++++ > .../migrations/0021_add_event_payload_field.py | 21 ++++ > ...22_migrate_data_from_event_fields_to_payload.py | 67 +++++++++++++ > .../migrations/0023_remove_old_event_fields.py | 43 ++++++++ > patchwork/models.py | 28 +----- > patchwork/signals.py | 48 +++++++++ > patchwork/tests/test_events.py | 110 > +++++++++++++-------- > 8 files changed, 354 insertions(+), 105 deletions(-) > create mode 100644 patchwork/migrations/0021_add_event_payload_field.py > create mode 100644 > patchwork/migrations/0022_migrate_data_from_event_fields_to_payload.py > create mode 100644 patchwork/migrations/0023_remove_old_event_fields.py > > diff --git a/patchwork/api/event.py b/patchwork/api/event.py > index 0d97af22..8f25eb4f 100644 > --- a/patchwork/api/event.py > +++ b/patchwork/api/event.py > @@ -19,9 +19,12 @@ > > from collections import OrderedDict > > +from rest_framework.fields import JSONField > from rest_framework.generics import ListAPIView > +from rest_framework.reverse import reverse > from rest_framework.serializers import ModelSerializer > -from rest_framework.serializers import SerializerMethodField > +from rest_framework.serializers import ReadOnlyField > +from rest_framework.serializers import Serializer > > from patchwork.api.embedded import CheckSerializer > from patchwork.api.embedded import CoverLetterSerializer > @@ -30,48 +33,79 @@ from patchwork.api.embedded import ProjectSerializer > from patchwork.api.embedded import SeriesSerializer > from patchwork.api.embedded import UserSerializer > from patchwork.api.filters import EventFilter > -from patchwork.api.patch import StateField > from patchwork.models import Event > > > -class EventSerializer(ModelSerializer): > +class StateField(ReadOnlyField): > > - project = ProjectSerializer(read_only=True) > - patch = PatchSerializer(read_only=True) > - series = SeriesSerializer(read_only=True) > - cover = CoverLetterSerializer(read_only=True) > + def to_representation(self, obj): > + return '-'.join(obj.name.lower().split()) > + > + > +class PayloadSerializer(Serializer): > + > + cover = CoverLetterSerializer(required=False) > + patch = PatchSerializer(required=False) > + series = SeriesSerializer(required=False) > previous_state = StateField() > current_state = StateField() > - previous_delegate = UserSerializer() > - current_delegate = UserSerializer() > - created_check = SerializerMethodField() > - created_check = CheckSerializer() > - > - _category_map = { > - Event.CATEGORY_COVER_CREATED: ['cover'], > - Event.CATEGORY_PATCH_CREATED: ['patch'], > - Event.CATEGORY_PATCH_COMPLETED: ['patch', 'series'], > - Event.CATEGORY_PATCH_STATE_CHANGED: ['patch', 'previous_state', > - 'current_state'], > - Event.CATEGORY_PATCH_DELEGATED: ['patch', 'previous_delegate', > - 'current_delegate'], > - Event.CATEGORY_CHECK_CREATED: ['patch', 'created_check'], > - Event.CATEGORY_SERIES_CREATED: ['series'], > - Event.CATEGORY_SERIES_COMPLETED: ['series'], > - } > + previous_delegate = UserSerializer(required=False) > + current_delegate = UserSerializer(required=False) > + check = CheckSerializer(required=False) > + > + > +class EventSerializer(ModelSerializer): > + > + project = ProjectSerializer(read_only=True) > + payload = JSONField(read_only=True) > > def to_representation(self, instance): > data = super(EventSerializer, self).to_representation(instance) > - payload = OrderedDict() > - kept_fields = self._category_map[instance.category] + [ > - 'id', 'category', 'project', 'date'] > > - for field in [x for x in data]: > - if field not in kept_fields: > - del data[field] > - elif field in self._category_map[instance.category]: > - field_name = 'check' if field == 'created_check' else field > - payload[field_name] = data.pop(field) > + # TODO(stephenfin): Remove this once we have migrations in place > + if not data['payload']: > + return data > + > + request = self.context['request'] > + payload = data['payload'] > + > + # set the 'url' and 'mbox' fields for any embedded 'cover', > + # 'patch', and 'series' fields > + for field in ('cover', 'patch', 'series'): > + if not payload.get(field): > + continue > + > + for subfield, lookup, url_name in ( > + ('url', 'pk', 'api-{}-detail'), > + ('mbox', '{}_id', '{}-mbox')): > + payload[field][subfield] = reverse( > + url_name.format(field), > + request=request, > + kwargs={ > + lookup.format(field): payload[field]['id'], > + }) > + > + # set the 'url' field for any embedded '*_delegate' fields > + for field in ('previous_delegate', 'current_delegate'): > + if not payload.get(field): > + continue > + > + payload[field]['url'] = reverse( > + 'api-user-detail', > + request=request, > + kwargs={ > + 'pk': payload[field]['id'] > + }) > + > + # set the 'url' field for any embedded 'check' fields > + if payload.get('check'): > + payload['check']['url'] = reverse( > + 'api-check-detail', > + request=request, > + kwargs={ > + 'patch_id': payload['patch']['id'], > + 'check_id': payload['check']['id'] > + }) > > data['payload'] = payload > > @@ -79,9 +113,7 @@ class EventSerializer(ModelSerializer): > > class Meta: > model = Event > - fields = ('id', 'category', 'project', 'date', 'patch', 'series', > - 'cover', 'previous_state', 'current_state', > - 'previous_delegate', 'current_delegate', 'created_check') > + fields = ('id', 'category', 'project', 'date', 'payload') > read_only_fields = fields > > > @@ -95,8 +127,4 @@ class EventList(ListAPIView): > ordering = '-date' > > def get_queryset(self): > - return Event.objects.all()\ > - .select_related('project', 'patch', 'series', 'cover', > - 'previous_state', 'current_state', > - 'previous_delegate', 'current_delegate', > - 'created_check') > + return Event.objects.all().select_related('project') > diff --git a/patchwork/fields.py b/patchwork/fields.py > index 502558be..8062e70a 100644 > --- a/patchwork/fields.py > +++ b/patchwork/fields.py > @@ -20,7 +20,9 @@ > > from __future__ import absolute_import > > +from collections import OrderedDict > import hashlib > +import json > > from django.db import models > from django.utils import six > @@ -44,3 +46,33 @@ class HashField(models.CharField): > > def db_type(self, connection=None): > return 'char(%d)' % self.n_bytes > + > + > +class JSONField(models.TextField): > + """A basic field for loading/storing JSON. > + > + We use this rather than Django's own JSONField because we don't need to > + index this field and said field does not support MySQL yet.
Speaking of lack of MySQL support, it occured to me last night that we've basically re-implemented materalised views... which we could use directly, if only mysql supported them. I know ozlabs.org uses postgres; I wonder what our other users use. More review to come. Regards, Daniel > + """ > + > + def get_prep_value(self, value): > + # This is necessary for Django 1.8.x, which called 'smart_text' from > + # 'django.utils.encoding' on all data > + return value > + > + def get_db_prep_value(self, value, connection, prepared=False): > + value = super(JSONField, self).get_db_prep_value( > + value, connection, prepared) > + return json.dumps(value) > + > + def from_db_value(self, value, *args, **kwargs): > + if value is None: > + return value > + > + return json.loads(value, object_pairs_hook=OrderedDict) > + > + def to_python(self, value): > + if value is None or isinstance(value, OrderedDict): > + return value > + > + return json.loads(value, object_pairs_hook=OrderedDict) > diff --git a/patchwork/migrations/0021_add_event_payload_field.py > b/patchwork/migrations/0021_add_event_payload_field.py > new file mode 100644 > index 00000000..e362ed03 > --- /dev/null > +++ b/patchwork/migrations/0021_add_event_payload_field.py > @@ -0,0 +1,21 @@ > +# -*- coding: utf-8 -*- > +from __future__ import unicode_literals > + > +from django.db import migrations, models > + > +import patchwork.fields > + > + > +class Migration(migrations.Migration): > + > + dependencies = [ > + ('patchwork', '0020_tag_show_column'), > + ] > + > + operations = [ > + migrations.AddField( > + model_name='event', > + name='payload', > + field=patchwork.fields.JSONField(blank=True, null=True), > + ), > + ] > diff --git > a/patchwork/migrations/0022_migrate_data_from_event_fields_to_payload.py > b/patchwork/migrations/0022_migrate_data_from_event_fields_to_payload.py > new file mode 100644 > index 00000000..760e26f2 > --- /dev/null > +++ b/patchwork/migrations/0022_migrate_data_from_event_fields_to_payload.py > @@ -0,0 +1,67 @@ > +# -*- coding: utf-8 -*- > +from __future__ import unicode_literals > + > +import json > + > +from django.db import migrations, models > + > +from patchwork.api.event import PayloadSerializer > +from patchwork.models import Event > +from patchwork.notifications import expire_events > + > + > +# TODO(stephenfin): Move this to 'api/event' and into Serializer > +class Payload(object): > + > + def __init__(self, **kwargs): > + for kwarg in kwargs: > + setattr(self, kwarg, kwargs[kwarg]) > + > + > +def generate_payload(apps, schema_editor): > + # NOTE(stephenfin): We could convert this to native SQL in the future > + # by using e.g. 'JSON_OBJECT' for MySQL > + EventBase = apps.get_model('patchwork', 'Event') > + > + expire_events() > + > + for event in EventBase.objects.all(): > + category = event.category > + # TODO(stephenfin): This logic should be moved into the > EventSerializer > + # class > + if category == Event.CATEGORY_COVER_CREATED: > + payload_obj = Payload(cover=event.cover) > + elif category == Event.CATEGORY_PATCH_CREATED: > + payload_obj = Payload(patch=event.patch) > + elif category == Event.CATEGORY_PATCH_STATE_CHANGED: > + payload_obj = Payload(patch=event.patch, > + previous_state=event.previous_state, > + current_state=event.previous_state) > + elif category == Event.CATEGORY_PATCH_DELEGATED: > + payload_obj = Payload(patch=event.patch, > + previous_delegate=event.previous_delegate, > + current_delegate=event.current_delegate) > + elif category == Event.CATEGORY_PATCH_COMPLETED: > + payload_obj = Payload(patch=event.patch) > + elif category == Event.CATEGORY_CHECK_CREATED: > + payload_obj = Payload(patch=event.patch, > check=event.created_check) > + elif category == Event.CATEGORY_SERIES_CREATED: > + payload_obj = Payload(series=event.series) > + elif category == Event.CATEGORY_SERIES_COMPLETED: > + payload_obj = Payload(series=event.series) > + > + payload = json.dumps(PayloadSerializer(payload_obj).data) > + event.payload = payload > + event.save() > + > + > +class Migration(migrations.Migration): > + > + dependencies = [ > + ('patchwork', '0021_add_event_payload_field'), > + ] > + > + operations = [ > + migrations.RunPython(generate_payload, > + atomic=True), > + ] > diff --git a/patchwork/migrations/0023_remove_old_event_fields.py > b/patchwork/migrations/0023_remove_old_event_fields.py > new file mode 100644 > index 00000000..1a3c70e1 > --- /dev/null > +++ b/patchwork/migrations/0023_remove_old_event_fields.py > @@ -0,0 +1,43 @@ > +# -*- coding: utf-8 -*- > +# Generated by Django 1.11.8 on 2018-01-10 00:16 > +from __future__ import unicode_literals > + > +from django.db import migrations > + > + > +class Migration(migrations.Migration): > + > + dependencies = [ > + ('patchwork', '0022_migrate_data_from_event_fields_to_payload'), > + ] > + > + operations = [ > + migrations.AlterModelOptions( > + name='patch', > + options={'base_manager_name': 'objects', 'verbose_name_plural': > 'Patches'}, > + ), > + migrations.AlterModelOptions( > + name='series', > + options={'ordering': ('date',), 'verbose_name_plural': 'Series'}, > + ), > + migrations.RemoveField( > + model_name='event', > + name='created_check', > + ), > + migrations.RemoveField( > + model_name='event', > + name='current_delegate', > + ), > + migrations.RemoveField( > + model_name='event', > + name='current_state', > + ), > + migrations.RemoveField( > + model_name='event', > + name='previous_delegate', > + ), > + migrations.RemoveField( > + model_name='event', > + name='previous_state', > + ), > + ] > diff --git a/patchwork/models.py b/patchwork/models.py > index 11886f1a..00674ac8 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -36,6 +36,7 @@ from django.utils.functional import cached_property > from patchwork.compat import is_authenticated > from patchwork.compat import reverse > from patchwork.fields import HashField > +from patchwork.fields import JSONField > from patchwork.hasher import hash_diff > > if settings.ENABLE_REST_API: > @@ -913,32 +914,9 @@ class Event(models.Model): > on_delete=models.CASCADE, > help_text='The cover letter that this event was created for.') > > - # fields for 'patch-state-changed' events > + # additional data > > - previous_state = models.ForeignKey( > - State, related_name='+', null=True, blank=True, > - on_delete=models.CASCADE) > - current_state = models.ForeignKey( > - State, related_name='+', null=True, blank=True, > - on_delete=models.CASCADE) > - > - # fields for 'patch-delegate-changed' events > - > - previous_delegate = models.ForeignKey( > - User, related_name='+', null=True, blank=True, > - on_delete=models.CASCADE) > - current_delegate = models.ForeignKey( > - User, related_name='+', null=True, blank=True, > - on_delete=models.CASCADE) > - > - # fields or 'patch-check-created' events > - > - created_check = models.ForeignKey( > - Check, related_name='+', null=True, blank=True, > - on_delete=models.CASCADE) > - > - # TODO(stephenfin): Validate that the correct fields are being set by way > - # of a 'clean' method > + payload = JSONField(null=True, blank=True) > > def __repr__(self): > return "<Event id='%d' category='%s'" % (self.id, self.category) > diff --git a/patchwork/signals.py b/patchwork/signals.py > index e5e7370f..c0145655 100644 > --- a/patchwork/signals.py > +++ b/patchwork/signals.py > @@ -18,11 +18,13 @@ > # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > > from datetime import datetime as dt > +import json > > from django.db.models.signals import post_save > from django.db.models.signals import pre_save > from django.dispatch import receiver > > +from patchwork.api.event import PayloadSerializer > from patchwork.models import Check > from patchwork.models import CoverLetter > from patchwork.models import Event > @@ -32,6 +34,14 @@ from patchwork.models import Series > from patchwork.models import SeriesPatch > > > +# TODO(stephenfin): Move this to 'api/event' and into Serializer > +class Payload(object): > + > + def __init__(self, **kwargs): > + for kwarg in kwargs: > + setattr(self, kwarg, kwargs[kwarg]) > + > + > @receiver(pre_save, sender=Patch) > def patch_change_callback(sender, instance, raw, **kwargs): > # we only want notification of modified patches > @@ -73,9 +83,13 @@ def patch_change_callback(sender, instance, raw, **kwargs): > def create_cover_created_event(sender, instance, created, raw, **kwargs): > > def create_event(cover): > + payload = PayloadSerializer(Payload( > + cover=cover)) > + > return Event.objects.create( > category=Event.CATEGORY_COVER_CREATED, > project=cover.project, > + payload=payload.data, > cover=cover) > > # don't trigger for items loaded from fixtures or new items > @@ -89,9 +103,13 @@ def create_cover_created_event(sender, instance, created, > raw, **kwargs): > def create_patch_created_event(sender, instance, created, raw, **kwargs): > > def create_event(patch): > + payload = PayloadSerializer(Payload( > + patch=patch)) > + > return Event.objects.create( > category=Event.CATEGORY_PATCH_CREATED, > project=patch.project, > + payload=payload.data, > patch=patch) > > # don't trigger for items loaded from fixtures or new items > @@ -105,9 +123,15 @@ def create_patch_created_event(sender, instance, > created, raw, **kwargs): > def create_patch_state_changed_event(sender, instance, raw, **kwargs): > > def create_event(patch, before, after): > + payload = PayloadSerializer(Payload( > + patch=patch, > + previous_state=before, > + current_state=after)) > + > return Event.objects.create( > category=Event.CATEGORY_PATCH_STATE_CHANGED, > project=patch.project, > + payload=payload.data, > patch=patch, > previous_state=before, > current_state=after) > @@ -128,9 +152,15 @@ def create_patch_state_changed_event(sender, instance, > raw, **kwargs): > def create_patch_delegated_event(sender, instance, raw, **kwargs): > > def create_event(patch, before, after): > + payload = PayloadSerializer(Payload( > + patch=patch, > + previous_delegate=before, > + current_delegate=after)) > + > return Event.objects.create( > category=Event.CATEGORY_PATCH_DELEGATED, > project=patch.project, > + payload=payload.data, > patch=patch, > previous_delegate=before, > current_delegate=after) > @@ -152,9 +182,14 @@ def create_patch_completed_event(sender, instance, > created, raw, **kwargs): > """Create patch completed event for patches with series.""" > > def create_event(patch, series): > + payload = PayloadSerializer(Payload( > + patch=patch, > + series=series)) > + > return Event.objects.create( > category=Event.CATEGORY_PATCH_COMPLETED, > project=patch.project, > + payload=payload.data, > patch=patch, > series=series) > > @@ -187,11 +222,16 @@ def create_patch_completed_event(sender, instance, > created, raw, **kwargs): > def create_check_created_event(sender, instance, created, raw, **kwargs): > > def create_event(check): > + payload = PayloadSerializer(Payload( > + patch=check.patch, > + check=check)) > + > # TODO(stephenfin): It might make sense to add a 'project' field to > # 'check' to prevent lookups here and in the REST API > return Event.objects.create( > category=Event.CATEGORY_CHECK_CREATED, > project=check.patch.project, > + payload=payload.data, > patch=check.patch, > created_check=check) > > @@ -206,9 +246,13 @@ def create_check_created_event(sender, instance, > created, raw, **kwargs): > def create_series_created_event(sender, instance, created, raw, **kwargs): > > def create_event(series): > + payload = PayloadSerializer(Payload( > + series=series)) > + > return Event.objects.create( > category=Event.CATEGORY_SERIES_CREATED, > project=series.project, > + payload=payload.data, > series=series) > > # don't trigger for items loaded from fixtures or existing items > @@ -234,9 +278,13 @@ def create_series_completed_event(sender, instance, > created, raw, **kwargs): > # in that case. > > def create_event(series): > + payload = PayloadSerializer(Payload( > + series=series)) > + > return Event.objects.create( > category=Event.CATEGORY_SERIES_COMPLETED, > project=series.project, > + payload=payload.data, > series=series) > > # don't trigger for items loaded from fixtures or existing items > diff --git a/patchwork/tests/test_events.py b/patchwork/tests/test_events.py > index 70d563de..39dff6f1 100644 > --- a/patchwork/tests/test_events.py > +++ b/patchwork/tests/test_events.py > @@ -17,32 +17,20 @@ > # along with Patchwork; if not, write to the Free Software > # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > > +from collections import OrderedDict > + > from django.test import TestCase > > from patchwork.models import Event > from patchwork.tests import utils > > -BASE_FIELDS = ['previous_state', 'current_state', 'previous_delegate', > - 'current_delegate'] > - > > def _get_events(**filters): > # These are sorted by reverse normally, so reverse it once again > return Event.objects.filter(**filters).order_by('date') > > > -class _BaseTestCase(TestCase): > - > - def assertEventFields(self, event, parent_type='patch', **fields): > - for field_name in [x for x in BASE_FIELDS]: > - field = getattr(event, field_name) > - if field_name in fields: > - self.assertEqual(field, fields[field_name]) > - else: > - self.assertIsNone(field) > - > - > -class PatchCreateTest(_BaseTestCase): > +class PatchCreateTest(TestCase): > > def test_patch_created(self): > """No series, so patch dependencies implicitly exist.""" > @@ -51,10 +39,15 @@ class PatchCreateTest(_BaseTestCase): > # This should raise both the CATEGORY_PATCH_CREATED and > # CATEGORY_PATCH_COMPLETED events as there are no specific > dependencies > events = _get_events(patch=patch) > + > self.assertEqual(events.count(), 1) > self.assertEqual(events[0].category, Event.CATEGORY_PATCH_CREATED) > self.assertEqual(events[0].project, patch.project) > - self.assertEventFields(events[0]) > + > + payload = events[0].payload > + self.assertIsInstance(payload, OrderedDict) > + self.assertIn('patch', payload) > + self.assertEqual(payload['patch']['id'], patch.id) > > def test_patch_dependencies_present_series(self): > """Patch dependencies already exist.""" > @@ -63,13 +56,22 @@ class PatchCreateTest(_BaseTestCase): > # This should raise both the CATEGORY_PATCH_CREATED and > # CATEGORY_PATCH_COMPLETED events > events = _get_events(patch=series_patch.patch) > + > self.assertEqual(events.count(), 2) > self.assertEqual(events[0].category, Event.CATEGORY_PATCH_CREATED) > self.assertEqual(events[0].project, series_patch.patch.project) > self.assertEqual(events[1].category, Event.CATEGORY_PATCH_COMPLETED) > self.assertEqual(events[1].project, series_patch.patch.project) > - self.assertEventFields(events[0]) > - self.assertEventFields(events[1]) > + > + payload = events[0].payload > + self.assertIn('patch', payload) > + self.assertEqual(payload['patch']['id'], series_patch.patch.id) > + > + payload = events[1].payload > + self.assertIn('patch', payload) > + self.assertEqual(payload['patch']['id'], series_patch.patch.id) > + self.assertIn('series', payload) > + self.assertEqual(payload['series']['id'], series_patch.series.id) > > def test_patch_dependencies_out_of_order(self): > series = utils.create_series() > @@ -82,7 +84,6 @@ class PatchCreateTest(_BaseTestCase): > events = _get_events(patch=series_patch.patch) > self.assertEqual(events.count(), 1) > self.assertEqual(events[0].category, > Event.CATEGORY_PATCH_CREATED) > - self.assertEventFields(events[0]) > > series_patch_1 = utils.create_series_patch(series=series, number=1) > > @@ -94,8 +95,6 @@ class PatchCreateTest(_BaseTestCase): > self.assertEqual(events[0].category, > Event.CATEGORY_PATCH_CREATED) > self.assertEqual(events[1].category, > Event.CATEGORY_PATCH_COMPLETED) > - self.assertEventFields(events[0]) > - self.assertEventFields(events[1]) > > def test_patch_dependencies_missing(self): > series_patch = utils.create_series_patch(number=2) > @@ -105,10 +104,9 @@ class PatchCreateTest(_BaseTestCase): > events = _get_events(patch=series_patch.patch) > self.assertEqual(events.count(), 1) > self.assertEqual(events[0].category, Event.CATEGORY_PATCH_CREATED) > - self.assertEventFields(events[0]) > > > -class PatchChangedTest(_BaseTestCase): > +class PatchChangedTest(TestCase): > > def test_patch_state_changed(self): > patch = utils.create_patch() > @@ -119,13 +117,18 @@ class PatchChangedTest(_BaseTestCase): > patch.save() > > events = _get_events(patch=patch) > + > self.assertEqual(events.count(), 2) > # we don't care about the CATEGORY_PATCH_CREATED event here > self.assertEqual(events[1].category, > Event.CATEGORY_PATCH_STATE_CHANGED) > self.assertEqual(events[1].project, patch.project) > - self.assertEventFields(events[1], previous_state=old_state, > - current_state=new_state) > + > + payload = events[1].payload > + self.assertIn('previous_state', payload) > + self.assertEqual(payload['previous_state'], old_state.slug) > + self.assertIn('current_state', payload) > + self.assertEqual(payload['current_state'], new_state.slug) > > def test_patch_delegated(self): > patch = utils.create_patch() > @@ -137,12 +140,18 @@ class PatchChangedTest(_BaseTestCase): > patch.save() > > events = _get_events(patch=patch) > + > self.assertEqual(events.count(), 2) > # we don't care about the CATEGORY_PATCH_CREATED event here > self.assertEqual(events[1].category, > Event.CATEGORY_PATCH_DELEGATED) > self.assertEqual(events[1].project, patch.project) > - self.assertEventFields(events[1], current_delegate=delegate_a) > + > + payload = events[1].payload > + self.assertIn('previous_delegate', payload) > + self.assertIsNone(payload['previous_delegate']) > + self.assertIn('current_delegate', payload) > + self.assertEqual(payload['current_delegate']['id'], delegate_a.id) > > delegate_b = utils.create_user() > > @@ -152,11 +161,16 @@ class PatchChangedTest(_BaseTestCase): > patch.save() > > events = _get_events(patch=patch) > + > self.assertEqual(events.count(), 3) > self.assertEqual(events[2].category, > Event.CATEGORY_PATCH_DELEGATED) > - self.assertEventFields(events[2], previous_delegate=delegate_a, > - current_delegate=delegate_b) > + > + payload = events[2].payload > + self.assertIn('previous_delegate', payload) > + self.assertEqual(payload['previous_delegate']['id'], delegate_a.id) > + self.assertIn('current_delegate', payload) > + self.assertEqual(payload['current_delegate']['id'], delegate_b.id) > > # Delegate B -> None > > @@ -164,24 +178,36 @@ class PatchChangedTest(_BaseTestCase): > patch.save() > > events = _get_events(patch=patch) > + > self.assertEqual(events.count(), 4) > self.assertEqual(events[3].category, > Event.CATEGORY_PATCH_DELEGATED) > - self.assertEventFields(events[3], previous_delegate=delegate_b) > > + payload = events[3].payload > + self.assertIn('previous_delegate', payload) > + self.assertEqual(payload['previous_delegate']['id'], delegate_b.id) > + self.assertIn('current_delegate', payload) > + self.assertIsNone(payload['current_delegate']) > > -class CheckCreateTest(_BaseTestCase): > + > +class CheckCreateTest(TestCase): > > def test_check_created(self): > check = utils.create_check() > - events = _get_events(created_check=check) > - self.assertEqual(events.count(), 1) > - self.assertEqual(events[0].category, Event.CATEGORY_CHECK_CREATED) > - self.assertEqual(events[0].project, check.patch.project) > - self.assertEventFields(events[0]) > + # we don't care about the CATEGORY_PATCH_CREATED event here > + events = _get_events(patch=check.patch) > > + self.assertEqual(events.count(), 2) > + # we don't care about the CATEGORY_PATCH_CREATED event here > + self.assertEqual(events[1].category, Event.CATEGORY_CHECK_CREATED) > + self.assertEqual(events[1].project, check.patch.project) > + > + payload = events[1].payload > + self.assertIn('check', payload) > + self.assertEqual(payload['check']['id'], check.id) > > -class CoverCreateTest(_BaseTestCase): > + > +class CoverCreateTest(TestCase): > > def test_cover_created(self): > cover = utils.create_cover() > @@ -189,10 +215,13 @@ class CoverCreateTest(_BaseTestCase): > self.assertEqual(events.count(), 1) > self.assertEqual(events[0].category, Event.CATEGORY_COVER_CREATED) > self.assertEqual(events[0].project, cover.project) > - self.assertEventFields(events[0]) > + > + payload = events[0].payload > + self.assertIn('cover', payload) > + self.assertEqual(payload['cover']['id'], cover.id) > > > -class SeriesCreateTest(_BaseTestCase): > +class SeriesCreateTest(TestCase): > > def test_series_created(self): > series = utils.create_series() > @@ -200,4 +229,7 @@ class SeriesCreateTest(_BaseTestCase): > self.assertEqual(events.count(), 1) > self.assertEqual(events[0].category, Event.CATEGORY_SERIES_CREATED) > self.assertEqual(events[0].project, series.project) > - self.assertEventFields(events[0]) > + > + payload = events[0].payload > + self.assertIn('series', payload) > + self.assertEqual(payload['series']['id'], series.id) > -- > 2.14.3 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork