[PATCH v3 5/9] signals: Add callbacks for events
Daniel Axtens
dja at axtens.net
Fri Feb 24 16:34:59 AEDT 2017
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 <dja at axtens.net>
Regards,
Daniel
> When models are modified, fire signal handlers to create the relevant
> events.
>
> Signed-off-by: Stephen Finucane <stephen at that.guru>
> ---
> 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()
> +
> +
> + at 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)
> +
> +
> + at 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)
> +
> +
> + at 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)
> +
> +
> + at 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)
> +
> +
> + at 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
> +
> +
> + at 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)
> +
> +
> + at 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)
> +
> +
> + at 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 <stephen at that.guru>
> +#
> +# 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
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list