Преглед изворни кода

Fix handling of string-typed object IDs

Jeremy Stretch пре 2 недеља
родитељ
комит
8aed00afdf
2 измењених фајлова са 41 додато и 2 уклоњено
  1. 8 2
      netbox/netbox/api/viewsets/mixins.py
  2. 33 0
      netbox/utilities/testing/api.py

+ 8 - 2
netbox/netbox/api/viewsets/mixins.py

@@ -505,9 +505,15 @@ class BulkUpdateModelMixin:
         if (response := get_missing_objects_response(object_ids, qs)) is not None:
             return response
 
-        # Map update data by object ID
+        # Map the attributes to be set for each object by its ID, taking the IDs from the validated
+        # data rather than from the request body: the body's values have not been coerced, so an ID
+        # submitted as a string ("123") would key this map by a value which never matches the
+        # integer PK it identifies, silently discarding that entry's attributes. Each `id` is
+        # excluded here rather than popped, leaving the request data as the client sent it. zip() is
+        # strict as the two sequences necessarily correspond, every entry having been validated.
         update_data = {
-            obj.pop('id'): obj for obj in request.data
+            object_id: {k: v for k, v in item.items() if k != 'id'}
+            for object_id, item in zip(object_ids, request.data, strict=True)
         }
 
         object_pks, errors = self.perform_bulk_update(qs, update_data, partial=partial)

+ 33 - 0
netbox/utilities/testing/api.py

@@ -572,6 +572,39 @@ class APIViewTestCases:
                     self.assertObjectChange(oc, action=ObjectChangeActionChoices.ACTION_UPDATE,
                         message=changelog_message)
 
+        def test_bulk_update_objects_string_id(self):
+            """
+            PATCH a set of objects whose IDs are given as strings rather than as numbers. The ID
+            field coerces such a value, so the object is identified and its attributes must be
+            applied -- rather than the entry being treated as though it carried no data.
+            """
+            if self.bulk_update_data is None:
+                self.skipTest("Bulk update data not set")
+
+            obj_perm = ObjectPermission(name='Test permission', actions=['change'])
+            obj_perm.save()
+            obj_perm.users.add(self.user)
+            obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model))
+
+            id_list = list(self._get_queryset().values_list('id', flat=True)[:2])
+            self.assertEqual(len(id_list), 2, "Insufficient number of objects to test bulk update")
+
+            # Quote only the second ID, so that a batch mixing the two forms is covered as well
+            data = [
+                {'id': id_list[0], **self.bulk_update_data},
+                {'id': str(id_list[1]), **self.bulk_update_data},
+            ]
+
+            response = self.client.patch(self._get_list_url(), data, format='json', **self.header)
+
+            # The attributes must have been applied to both objects. Note that the response body is
+            # deliberately not inspected: for a model whose viewset narrows its own queryset (e.g.
+            # SavedFilter, which is restricted to shared or owned objects), an update which moves an
+            # object outside that queryset succeeds but is not echoed back.
+            self.assertHttpStatus(response, status.HTTP_200_OK)
+            for instance in self._get_queryset().filter(pk__in=id_list):
+                self.assertInstanceEqual(instance, self.bulk_update_data, api=True)
+
         def test_bulk_update_objects_validation_error(self):
             """
             PATCH a set of objects where one fails validation. Verify the structured per-object error