Hi Stephen, So I was worried about the performance impact of this, but testing shows that it's unlikely to be problematic.
For this whole series: Tested-by: Daniel Axtens <[email protected]> Regards, Daniel > When models are modified, fire signal handlers to create the relevant > events. > > Signed-off-by: Stephen Finucane <[email protected]> > --- > Changes since v2: > - Rework to support additional event types > Changes since v1: > - Rework to support additional event types > Changes since RFC: > - Make use of database-provided foreign keys rather than reimplementing > them ourselves > --- > patchwork/models.py | 3 + > patchwork/signals.py | 178 +++++++++++++++++++++++++++++++++++ > patchwork/tests/test_events.py | 204 > +++++++++++++++++++++++++++++++++++++++++ > patchwork/tests/utils.py | 15 +++ > 4 files changed, 400 insertions(+) > create mode 100644 patchwork/tests/test_events.py > > diff --git a/patchwork/models.py b/patchwork/models.py > index d9f7bec..591046f 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -881,6 +881,9 @@ class Event(models.Model): > # TODO(stephenfin): Validate that the correct fields are being set by way > # of a 'clean' method > > + def __repr__(self): > + return "<Event id='%d' category='%s'" % (self.id, self.category) > + > class Meta: > ordering = ['-date'] > > diff --git a/patchwork/signals.py b/patchwork/signals.py > index 6f7f5ea..208685c 100644 > --- a/patchwork/signals.py > +++ b/patchwork/signals.py > @@ -19,11 +19,17 @@ > > from datetime import datetime as dt > > +from django.db.models.signals import post_save > from django.db.models.signals import pre_save > from django.dispatch import receiver > > +from patchwork.models import Check > +from patchwork.models import CoverLetter > +from patchwork.models import Event > from patchwork.models import Patch > from patchwork.models import PatchChangeNotification > +from patchwork.models import Series > +from patchwork.models import SeriesPatch > > > @receiver(pre_save, sender=Patch) > @@ -61,3 +67,175 @@ def patch_change_callback(sender, instance, **kwargs): > > notification.last_modified = dt.now() > notification.save() > + > + > +@receiver(post_save, sender=CoverLetter) > +def create_cover_created_event(sender, instance, created, **kwargs): > + > + def create_event(cover): > + return Event.objects.create( > + project=cover.project, > + cover=cover, > + category=Event.CATEGORY_COVER_CREATED) > + > + if not created: > + return > + > + create_event(instance) > + > + > +@receiver(post_save, sender=Patch) > +def create_patch_created_event(sender, instance, created, **kwargs): > + > + def create_event(patch): > + return Event.objects.create( > + project=patch.project, > + patch=patch, > + category=Event.CATEGORY_PATCH_CREATED) > + > + if not created: > + return > + > + create_event(instance) > + > + > +@receiver(pre_save, sender=Patch) > +def create_patch_state_changed_event(sender, instance, **kwargs): > + > + def create_event(patch, before, after): > + return Event.objects.create( > + project=patch.project, > + patch=patch, > + category=Event.CATEGORY_PATCH_STATE_CHANGED, > + previous_state=before, > + current_state=after) > + > + # only trigger on updated items > + if not instance.pk: > + return > + > + orig_patch = Patch.objects.get(pk=instance.pk) > + > + if orig_patch.state == instance.state: > + return > + > + create_event(instance, orig_patch.state, instance.state) > + > + > +@receiver(pre_save, sender=Patch) > +def create_patch_delegated_event(sender, instance, **kwargs): > + > + def create_event(patch, before, after): > + return Event.objects.create( > + project=patch.project, > + patch=patch, > + category=Event.CATEGORY_PATCH_DELEGATED, > + previous_delegate=before, > + current_delegate=after) > + > + # only trigger on updated items > + if not instance.pk: > + return > + > + orig_patch = Patch.objects.get(pk=instance.pk) > + > + if orig_patch.delegate == instance.delegate: > + return > + > + create_event(instance, orig_patch.delegate, instance.delegate) > + > + > +@receiver(post_save, sender=SeriesPatch) > +def create_patch_completed_event(sender, instance, created, **kwargs): > + """Create patch completed event for patches with series.""" > + > + def create_event(patch, series): > + return Event.objects.create( > + project=patch.project, > + patch=patch, > + series=series, > + category=Event.CATEGORY_PATCH_COMPLETED) > + > + if not created: > + return > + > + # if dependencies not met, don't raise event. There's also no point > raising > + # events for successors since they'll have the same issue > + predecessors = SeriesPatch.objects.filter( > + series=instance.series, number__lt=instance.number) > + if predecessors.count() != instance.number - 1: > + return > + > + create_event(instance.patch, instance.series) > + > + # if this satisfies dependencies for successor patch, raise events for > + # those > + count = instance.number + 1 > + for successor in SeriesPatch.objects.filter( > + series=instance.series, number__gt=instance.number): > + if successor.number != count: > + break > + > + create_event(successor.patch, successor.series) > + count += 1 > + > + > +@receiver(post_save, sender=Check) > +def create_check_created_event(sender, instance, created, **kwargs): > + > + def create_event(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( > + project=check.patch.project, > + patch=check.patch, > + created_check=check, > + category=Event.CATEGORY_CHECK_CREATED) > + > + if not created: > + return > + > + create_event(instance) > + > + > +@receiver(post_save, sender=Series) > +def create_series_created_event(sender, instance, created, **kwargs): > + > + def create_event(series): > + return Event.objects.create( > + project=series.project, > + series=series, > + category=Event.CATEGORY_SERIES_CREATED) > + > + if not created: > + return > + > + create_event(instance) > + > + > +@receiver(post_save, sender=SeriesPatch) > +def create_series_completed_event(sender, instance, created, **kwargs): > + > + # NOTE(stephenfin): We subscribe to the SeriesPatch.post_save signal > + # instead of Series.m2m_changed to minimize the amount of times this is > + # fired. The m2m_changed signal doesn't support a 'changed' parameter, > + # which we could use to quick skip the signal when a patch is merely > + # updated instead of added to the series. > + > + # NOTE(stephenfin): It's actually possible for this event to be fired > + # multiple times for a given series. To trigger this case, you would need > + # to send an additional patch to already exisiting series. This pattern > + # exists in the wild ('PATCH 5/n'), so we probably want to retest a > series > + # in that case. > + > + def create_event(series): > + return Event.objects.create( > + project=series.project, > + series=series, > + category=Event.CATEGORY_SERIES_COMPLETED) > + > + if not created: > + return > + > + if instance.series.received_all: > + create_event(instance.series) > diff --git a/patchwork/tests/test_events.py b/patchwork/tests/test_events.py > new file mode 100644 > index 0000000..487e1b5 > --- /dev/null > +++ b/patchwork/tests/test_events.py > @@ -0,0 +1,204 @@ > +# Patchwork - automated patch tracking system > +# Copyright (C) 2015 Stephen Finucane <[email protected]> > +# > +# This file is part of the Patchwork package. > +# > +# Patchwork is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# Patchwork is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with Patchwork; if not, write to the Free Software > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + > +from django.contrib.contenttypes.models import ContentType > +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): > + > + def test_patch_created(self): > + """No series, so patch dependencies implicitly exist.""" > + patch = utils.create_patch() > + > + # 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]) > + > + def test_patch_dependencies_present_series(self): > + """Patch dependencies already exist.""" > + series_patch = utils.create_series_patch() > + > + # 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]) > + > + def test_patch_dependencies_out_of_order(self): > + series = utils.create_series() > + series_patch_3 = utils.create_series_patch(series=series, number=3) > + series_patch_2 = utils.create_series_patch(series=series, number=2) > + > + # This should only raise the CATEGORY_PATCH_CREATED event for > + # both patches as they are both missing dependencies > + for series_patch in [series_patch_2, series_patch_3]: > + 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) > + > + # We should now see the CATEGORY_PATCH_COMPLETED event for all > patches > + # as the dependencies for all have been met > + for series_patch in [series_patch_1, series_patch_2, series_patch_3]: > + events = _get_events(patch=series_patch.patch) > + self.assertEqual(events.count(), 2) > + 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) > + > + # This should only raise the CATEGORY_PATCH_CREATED event as > + # there is a missing dependency (patch 1) > + 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): > + > + def test_patch_state_changed(self): > + patch = utils.create_patch() > + old_state = patch.state > + new_state = utils.create_state() > + > + patch.state = new_state > + 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) > + > + def test_patch_delegated(self): > + patch = utils.create_patch() > + delegate_a = utils.create_user() > + > + # None -> Delegate A > + > + patch.delegate = delegate_a > + 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) > + > + delegate_b = utils.create_user() > + > + # Delegate A -> Delegate B > + > + patch.delegate = delegate_b > + 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) > + > + # Delegate B -> None > + > + patch.delegate = None > + 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) > + > + > +class CheckCreateTest(_BaseTestCase): > + > + 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]) > + > + > +class CoverCreateTest(_BaseTestCase): > + > + def test_cover_created(self): > + cover = utils.create_cover() > + events = _get_events(cover=cover) > + 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]) > + > + > +class SeriesCreateTest(_BaseTestCase): > + > + def test_series_created(self): > + series = utils.create_series() > + events = _get_events(series=series) > + 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]) > diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py > index 0c6f763..3d0293c 100644 > --- a/patchwork/tests/utils.py > +++ b/patchwork/tests/utils.py > @@ -33,6 +33,7 @@ from patchwork.models import Patch > from patchwork.models import Person > from patchwork.models import Project > from patchwork.models import Series > +from patchwork.models import SeriesPatch > from patchwork.models import SeriesReference > from patchwork.models import State > from patchwork.tests import TEST_PATCH_DIR > @@ -233,6 +234,20 @@ def create_series(**kwargs): > return Series.objects.create(**values) > > > +def create_series_patch(**kwargs): > + """Create 'SeriesPatch' object.""" > + num = 1 if 'series' not in kwargs else kwargs['series'].patches.count() > + 1 > + > + values = { > + 'series': create_series() if 'series' not in kwargs else None, > + 'number': num, > + 'patch': create_patch() if 'patch' not in kwargs else None, > + } > + values.update(**kwargs) > + > + return SeriesPatch.objects.create(**values) > + > + > def create_series_reference(**kwargs): > """Create 'SeriesReference' object.""" > values = { > -- > 2.9.3 > > _______________________________________________ > Patchwork mailing list > [email protected] > https://lists.ozlabs.org/listinfo/patchwork _______________________________________________ Patchwork mailing list [email protected] https://lists.ozlabs.org/listinfo/patchwork
