TST: Fix race condition from transaction handler

Author: thclark

tests/test_storage_fields.py

@@ -132,29 +132,12 @@ def test_change_view_loads_normally(self):
         )
 
 
-class TestBlobField(StorageOperationsMixin, TransactionTestCase):
+class TestBlobField(StorageOperationsMixin, TestCase):
     """Inherits from transaction test case, because we use an on_commit
     hook to move ingressed files once a database save has been made
 
-    TODO REMOVE TRANSACTION TEST CASE
-    as per https://code.djangoproject.com/ticket/30457
-    ```
-    with self.captureOnCommitCallbacks(execute=True) as callbacks:
-        with transaction.atomic():
-            transaction.on_commit(branch_1)
-    ```
     """
 
-    def test_validates_blob_exists_on_add(self):
-        """Ensure that a ValidationError is raised if the blob at _tmp_path does not exist"""
-
-        form = BlobForm(data={"blob": {"name": "thing", "_tmp_path": "something"}})
-        form.is_valid()
-        self.assertEqual(
-            form.errors["blob"],
-            ["Upload incomplete or failed (no blob at 'something' in bucket 'example-media-assets')"],
-        )
-
     def test_validates_name_is_present_on_add(self):
         """Ensure that a ValidationError is raised if no 'name' property is present in the blob data"""
 
@@ -220,27 +203,32 @@ def test_validates_raises_on_add_blank_none(self):
             ["This field is required."],
         )
 
-    def test_create_object_fails_with_missing_blob(self):
-        """Create an object but fail to copy the blob (because it's missing) then check that
-        no database record was created"""
-        with self.assertRaises(MissingBlobError):
-            obj = ExampleBlobFieldModel.objects.create(
-                blob={"_tmp_path": f"_tmp/{uuid4()}.txt", "name": "missing_blob.txt"}
-            )
-            obj.save()
-        count = ExampleBlobFieldModel.objects.count()
-        self.assertEqual(count, 0)
-
     @override_settings(GCP_STORAGE_OVERRIDE_BLOBFIELD_VALUE=True)
     def test_create_object_succeeds_with_overridden_path(self):
         """Through the ORM, we may need to create blobs directly at the destination"""
 
-        blob_name = self._prefix_blob_name("create_object_succeeds_with_overridden_path.txt")
-        self._create_test_blob(self.bucket, blob_name)
-        ExampleBlobFieldModel.objects.create(blob={"path": blob_name})
+        with self.captureOnCommitCallbacks(execute=False) as callbacks:
+            blob_name = self._prefix_blob_name("create_object_succeeds_with_overridden_path.txt")
+            self._create_test_blob(self.bucket, blob_name)
+            ExampleBlobFieldModel.objects.create(blob={"path": blob_name})
+
+        # There should be no callback executed when the blob field value is overwritten directly
+        self.assertEqual(len(callbacks), 0)
         count = ExampleBlobFieldModel.objects.count()
         self.assertEqual(count, 1)
 
+    def test_create_object_fails_with_missing_blob(self):
+        """Create an object but fail to copy the blob (because it's missing) then check that
+        no database record was created"""
+        with self.assertRaises(MissingBlobError):
+            with self.captureOnCommitCallbacks(execute=True) as callbacks:
+                ExampleBlobFieldModel.objects.create(
+                    blob={"_tmp_path": f"_tmp/{uuid4()}.txt", "name": "missing_blob.txt"}
+                )
+            self.assertEqual(len(callbacks), 1)
+            count = ExampleBlobFieldModel.objects.count()
+            self.assertEqual(count, 0)
+
 
 class TestBlankBlobField(BlobModelFactoryMixin, StorageOperationsMixin, TransactionTestCase):
     """Inherits from transaction test case, because we use an on_commit