[RFC 3/4] models: Migrate event fields to JSON field
Daniel Axtens
dja at axtens.net
Tue Jan 30 10:07:47 AEDT 2018
Stephen Finucane <stephen at 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 <stephen at 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 at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list