[BD-14] fix: call blockstore APIs in atomic transactions.

openedx/core/djangoapps/content_libraries/api.py

@@ -426,6 +426,10 @@ def create_library(
     """
     assert isinstance(collection_uuid, UUID)
     assert isinstance(org, Organization)
+    assert not transaction.get_autocommit(), (
+        "Call within a django.db.transaction.atomic block so that all created objects are rolled back on error."
+    )
+
     validate_unicode_slug(slug)
     # First, create the blockstore bundle:
     bundle = create_bundle(
@@ -436,20 +440,16 @@ def create_library(
     )
     # Now create the library reference in our database:
     try:
-        # Atomic transaction required because if this fails,
-        # we need to delete the bundle in the exception handler.
-        with transaction.atomic():
-            ref = ContentLibrary.objects.create(
-                org=org,
-                slug=slug,
-                type=library_type,
-                bundle_uuid=bundle.uuid,
-                allow_public_learning=allow_public_learning,
-                allow_public_read=allow_public_read,
-                license=library_license,
-            )
+        ref = ContentLibrary.objects.create(
+            org=org,
+            slug=slug,
+            type=library_type,
+            bundle_uuid=bundle.uuid,
+            allow_public_learning=allow_public_learning,
+            allow_public_read=allow_public_read,
+            license=library_license,
+        )
     except IntegrityError:
-        delete_bundle(bundle.uuid)
         raise LibraryAlreadyExists(slug)  # lint-amnesty, pylint: disable=raise-missing-from
     CONTENT_LIBRARY_CREATED.send(sender=None, library_key=ref.library_key)
     return ContentLibraryMetadata(

openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py

@@ -30,6 +30,7 @@
 )
 from openedx.core.djangoapps.content_libraries.constants import VIDEO, COMPLEX, PROBLEM, CC_4_BY, ALL_RIGHTS_RESERVED
 from openedx.core.djangolib.blockstore_cache import cache
+from openedx.core.lib import blockstore_api
 from common.djangoapps.student.tests.factories import UserFactory
 
 
@@ -189,10 +190,22 @@ def test_library_validation(self):
         You can't create a library with the same slug as an existing library,
         or an invalid slug.
         """
+        assert 0 == len(blockstore_api.get_bundles(text_search='some-slug'))
         self._create_library(slug="some-slug", title="Existing Library")
-        self._create_library(slug="some-slug", title="Duplicate Library", expect_response=400)
+        assert 1 == len(blockstore_api.get_bundles(text_search='some-slug'))
 
-        self._create_library(slug="Invalid Slug!", title="Library with Bad Slug", expect_response=400)
+        # Try to create a library+bundle with a duplicate slug
+        response = self._create_library(slug="some-slug", title="Duplicate Library", expect_response=400)
+        assert response == {
+            'slug': 'A library with that ID already exists.',
+        }
+        # The second bundle created with that slug is removed when the transaction rolls back.
+        assert 1 == len(blockstore_api.get_bundles(text_search='some-slug'))
+
+        response = self._create_library(slug="Invalid Slug!", title="Library with Bad Slug", expect_response=400)
+        assert response == {
+            'slug': ['Enter a valid “slug” consisting of Unicode letters, numbers, underscores, or hyphens.'],
+        }
 
     @ddt.data(True, False)
     @patch("openedx.core.djangoapps.content_libraries.views.LibraryApiPagination.page_size", new=2)

openedx/core/djangoapps/content_libraries/tests/test_runtime.py

@@ -5,7 +5,7 @@
 from gettext import GNUTranslations
 
 from completion.test_utils import CompletionWaffleTestMixin
-from django.db import connections
+from django.db import connections, transaction
 from django.test import LiveServerTestCase, TestCase
 from django.utils.text import slugify
 from organizations.models import Organization
@@ -51,17 +51,18 @@ def setUp(self):
             short_name="CL-TEST",
         )
         _, slug = self.id().rsplit('.', 1)
-        self.library = library_api.create_library(
-            collection_uuid=self.collection.uuid,
-            library_type=COMPLEX,
-            org=self.organization,
-            slug=slugify(slug),
-            title=(f"{slug} Test Lib"),
-            description="",
-            allow_public_learning=True,
-            allow_public_read=False,
-            library_license=ALL_RIGHTS_RESERVED,
-        )
+        with transaction.atomic():
+            self.library = library_api.create_library(
+                collection_uuid=self.collection.uuid,
+                library_type=COMPLEX,
+                org=self.organization,
+                slug=slugify(slug),
+                title=(f"{slug} Test Lib"),
+                description="",
+                allow_public_learning=True,
+                allow_public_read=False,
+                library_license=ALL_RIGHTS_RESERVED,
+            )
 
 
 class ContentLibraryRuntimeTestMixin(ContentLibraryContentTestMixin):
@@ -83,17 +84,18 @@ def test_identical_olx(self):
         library_api.create_library_block_child(unit_block_key, "problem", "p1")
         library_api.publish_changes(self.library.key)
         # Now do the same in a different library:
-        library2 = library_api.create_library(
-            collection_uuid=self.collection.uuid,
-            org=self.organization,
-            slug="idolx",
-            title=("Identical OLX Test Lib 2"),
-            description="",
-            library_type=COMPLEX,
-            allow_public_learning=True,
-            allow_public_read=False,
-            library_license=CC_4_BY,
-        )
+        with transaction.atomic():
+            library2 = library_api.create_library(
+                collection_uuid=self.collection.uuid,
+                org=self.organization,
+                slug="idolx",
+                title=("Identical OLX Test Lib 2"),
+                description="",
+                library_type=COMPLEX,
+                allow_public_learning=True,
+                allow_public_read=False,
+                library_license=CC_4_BY,
+            )
         unit_block2_key = library_api.create_library_block(library2.key, "unit", "u1").usage_key
         library_api.create_library_block_child(unit_block2_key, "problem", "p1")
         library_api.publish_changes(library2.key)