Fixes: #15194 - Check the whole queue for matching queued webhooks

netbox/extras/signals.py

@@ -55,16 +55,21 @@ def run_validators(instance, validators):
 clear_events = Signal()
 
 
-def is_same_object(instance, webhook_data, request_id):
+def last_matching_index(instance, queue, request_id):
     """
-    Compare the given instance to the most recent queued webhook object, returning True
-    if they match. This check is used to avoid creating duplicate webhook entries.
+    Find the latest queued webhook object matching the given instance, returning its index.
+    If no object is found, return None. This check is used to avoid creating duplicate webhook
+    entries.
     """
-    return (
-        ContentType.objects.get_for_model(instance) == webhook_data['content_type'] and
-        instance.pk == webhook_data['object_id'] and
-        request_id == webhook_data['request_id']
-    )
+    try:
+        return max(
+            index_ for index_, webhook_data in enumerate(queue)
+            if ContentType.objects.get_for_model(instance) == webhook_data['content_type'] and
+            instance.pk == webhook_data['object_id'] and
+            request_id == webhook_data['request_id']
+        )
+    except ValueError:
+        return None
 
 
 @receiver((post_save, m2m_changed))
@@ -112,12 +117,12 @@ def handle_changed_object(sender, instance, **kwargs):
         objectchange.request_id = request.id
         objectchange.save()
 
-    # If this is an M2M change, update the previously queued webhook (from post_save)
+    # Update the previously queued webhook from a preceeding post_save
     queue = events_queue.get()
-    if m2m_changed and queue and is_same_object(instance, queue[-1], request.id):
+    if queue and (last_index := last_matching_index(instance, queue, request.id)) is not None:
         instance.refresh_from_db()  # Ensure that we're working with fresh M2M assignments
-        queue[-1]['data'] = serialize_for_event(instance)
-        queue[-1]['snapshots']['postchange'] = get_snapshots(instance, action)['postchange']
+        queue[last_index]['data'] = serialize_for_event(instance)
+        queue[last_index]['snapshots']['postchange'] = get_snapshots(instance, action)['postchange']
     else:
         enqueue_object(queue, instance, request.user, request.id, action)
     events_queue.set(queue)

netbox/extras/tests/test_event_rules.py

@@ -16,6 +16,8 @@
 from extras.models import EventRule, Tag, Webhook
 from extras.webhooks import generate_signature, send_webhook
 from utilities.testing import APITestCase
+from netbox.tests.dummy_plugin.models import DummyModel
+from netbox.context import events_queue
 
 
 class EventRuleTest(APITestCase):
@@ -31,6 +33,7 @@ def setUp(self):
     def setUpTestData(cls):
 
         site_type = ObjectType.objects.get_for_model(Site)
+        dummy_type = ObjectType.objects.get_for_model(DummyModel)
         DUMMY_URL = 'http://localhost:9000/'
         DUMMY_SECRET = 'LOOKATMEIMASECRETSTRING'
 
@@ -65,7 +68,7 @@ def setUpTestData(cls):
             ),
         ))
         for event_rule in event_rules:
-            event_rule.object_types.set([site_type])
+            event_rule.object_types.set([site_type, dummy_type])
 
         Tag.objects.bulk_create((
             Tag(name='Foo', slug='foo'),
@@ -377,3 +380,45 @@ def dummy_send(_, request, **kwargs):
         # Patch the Session object with our dummy_send() method, then process the webhook for sending
         with patch.object(Session, 'send', dummy_send) as mock_send:
             send_webhook(**job.kwargs)
+
+    def test_webhook_double_save_create(self):
+        data = {
+            'name': 'Dummy',
+        }
+        url = reverse('plugins-api:dummy_plugin-api:dummymodel-list')
+        self.add_permissions('dummy_plugin.add_dummymodel')
+        response = self.client.post(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_201_CREATED)
+        self.assertEqual(DummyModel.objects.count(), 1)
+
+        dummy = DummyModel.objects.first()
+
+        self.assertEqual(self.queue.count, 1)
+        job = self.queue.jobs[0]
+        self.assertEqual(job.kwargs['event_rule'], EventRule.objects.get(type_create=True))
+        self.assertEqual(job.kwargs['event'], ObjectChangeActionChoices.ACTION_CREATE)
+        self.assertEqual(job.kwargs['model_name'], 'dummymodel')
+        self.assertEqual(job.kwargs['data']['id'], dummy.pk)
+        self.assertEqual(job.kwargs['snapshots']['postchange']['name'], 'Dummy')
+
+    def test_webhook_double_save_update(self):
+        dummy = DummyModel.objects.create(name='Dummy')
+
+        data = {
+            'name': 'New Dummy',
+            'number': 42,
+        }
+        url = reverse('plugins-api:dummy_plugin-api:dummymodel-detail', kwargs={'pk': dummy.pk})
+        self.add_permissions('dummy_plugin.change_dummymodel')
+        response = self.client.patch(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        self.assertEqual(DummyModel.objects.count(), 1)
+
+        self.assertEqual(self.queue.count, 1)
+        job = self.queue.jobs[0]
+        self.assertEqual(job.kwargs['event_rule'], EventRule.objects.get(type_update=True))
+        self.assertEqual(job.kwargs['event'], ObjectChangeActionChoices.ACTION_UPDATE)
+        self.assertEqual(job.kwargs['model_name'], 'dummymodel')
+        self.assertEqual(job.kwargs['data']['id'], dummy.pk)
+        self.assertEqual(job.kwargs['snapshots']['postchange']['name'], 'New Dummy')
+        self.assertEqual(job.kwargs['snapshots']['postchange']['number'], 42)

netbox/netbox/tests/dummy_plugin/api/serializers.py

@@ -2,7 +2,7 @@
 from netbox.tests.dummy_plugin.models import DummyModel
 
 
-class DummySerializer(ModelSerializer):
+class DummyModelSerializer(ModelSerializer):
 
     class Meta:
         model = DummyModel

netbox/netbox/tests/dummy_plugin/api/views.py

@@ -1,8 +1,8 @@
 from rest_framework.viewsets import ModelViewSet
 from netbox.tests.dummy_plugin.models import DummyModel
-from .serializers import DummySerializer
+from .serializers import DummyModelSerializer
 
 
 class DummyViewSet(ModelViewSet):
     queryset = DummyModel.objects.all()
-    serializer_class = DummySerializer
+    serializer_class = DummyModelSerializer

netbox/netbox/tests/dummy_plugin/migrations/0001_initial.py

@@ -15,6 +15,8 @@ class Migration(migrations.Migration):
                 ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False)),
                 ('name', models.CharField(max_length=20)),
                 ('number', models.IntegerField(default=100)),
+                ('created', models.DateTimeField(auto_now_add=True, null=True)),
+                ('last_updated', models.DateTimeField(auto_now=True, null=True)),
             ],
             options={
                 'ordering': ['name'],

netbox/netbox/tests/dummy_plugin/models.py

@@ -1,13 +1,20 @@
 from django.db import models
 
+from netbox.models import EventRulesMixin, ChangeLoggingMixin
 
-class DummyModel(models.Model):
+
+class DummyModel(EventRulesMixin, ChangeLoggingMixin, models.Model):
     name = models.CharField(
         max_length=20
     )
     number = models.IntegerField(
         default=100
     )
+    serializer_label = 'netbox.tests.dummy_plugin'
 
     class Meta:
         ordering = ['name']
+
+    def save(self, *args, **kwargs):
+        super().save()
+        super().save()

netbox/utilities/api.py

@@ -30,7 +30,12 @@ def get_serializer_for_model(model, prefix=''):
     """
     Return the appropriate REST API serializer for the given model.
     """
-    app_label, model_name = model._meta.label.split('.')
+    if hasattr(model, 'serializer_label'):
+        app_label = model.serializer_label
+        model_name = model._meta.label.split('.')[1]
+    else:
+        app_label, model_name = model._meta.label.split('.')
+
     serializer_name = f'{app_label}.api.serializers.{prefix}{model_name}Serializer'
     try:
         return import_string(serializer_name)