From c3e4976d4ddd635e692c89aa3092a08bee0e3f94 Mon Sep 17 00:00:00 2001 From: Samuel Walladge Date: Wed, 25 Aug 2021 08:18:40 +0930 Subject: [PATCH] fix: crash in retrieving blockstore bundle links If the bundle is deleted, `blockstore_cache.get_bundle_version_number(bundle_uuid)` will raise an error. Catch this error in `get_bundle_links`, so the REST api doesn't crash with a 500 error when a linked bundle is deleted. --- .../core/djangoapps/content_libraries/api.py | 8 +- .../tests/test_content_libraries.py | 89 +++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index b1df97ed3d..1d5dd4bf3e 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -109,6 +109,7 @@ from openedx.core.lib.blockstore_api import ( set_draft_link, commit_draft, delete_draft, + BundleNotFound, ) from openedx.core.djangolib import blockstore_cache from openedx.core.djangolib.blockstore_cache import BundleCache @@ -1003,12 +1004,15 @@ def get_bundle_links(library_key): opaque_key = libraries_linked[link_data.bundle_uuid].library_key except KeyError: opaque_key = None - # Append the link information: + try: + latest_version = blockstore_cache.get_bundle_version_number(link_data.bundle_uuid) + except BundleNotFound: + latest_version = 0 results.append(LibraryBundleLink( id=link_name, bundle_uuid=link_data.bundle_uuid, version=link_data.version, - latest_version=blockstore_cache.get_bundle_version_number(link_data.bundle_uuid), + latest_version=latest_version, opaque_key=opaque_key, )) return results diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 3f5e2f75dd..c23679cb18 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -22,6 +22,7 @@ from openedx.core.djangoapps.content_libraries.tests.base import ( URL_BLOCK_XBLOCK_HANDLER, ) 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 common.djangoapps.student.tests.factories import UserFactory @@ -745,6 +746,94 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest): assert links_created[1]['latest_version'] == 2 assert links_created[1]['opaque_key'] == bank_lib_id + def test_library_blocks_with_deleted_links(self): + """ + Test that libraries can handle deleted links to bundles + """ + # Create a problem bank: + bank_lib = self._create_library(slug="problem_bank1X", title="Problem Bank") + bank_lib_id = bank_lib["id"] + # Add problem1 to the problem bank: + p1 = self._add_block_to_library(bank_lib_id, "problem", "problem1X") + self._set_library_block_olx(p1["id"], """ + +

What is an even number?

+ + 3 + 2 + + + """) + # Commit the changes, creating version 1: + self._commit_library_changes(bank_lib_id) + + # Create another problem bank: + bank_lib2 = self._create_library(slug="problem_bank2", title="Problem Bank 2") + bank_lib2_id = bank_lib2["id"] + # Add problem1 to the problem bank: + p2 = self._add_block_to_library(bank_lib2_id, "problem", "problem1X") + self._set_library_block_olx(p2["id"], """ + +

What is an odd number?

+ + 3 + 2 + + + """) + # Commit the changes, creating version 1: + self._commit_library_changes(bank_lib2_id) + + lib = self._create_library(slug="problem_bank2X", title="Link Test Library") + lib_id = lib["id"] + # Link to the other libraries: + self._link_to_library(lib_id, "problem_bank", bank_lib_id) + self._link_to_library(lib_id, "problem_bank_v1", bank_lib2_id) + + # check the API for retrieving links: + links_created = self._get_library_links(lib_id) + links_created.sort(key=lambda link: link["id"]) + assert len(links_created) == 2 + + assert links_created[0]['id'] == 'problem_bank' + assert links_created[0]['bundle_uuid'] == bank_lib['bundle_uuid'] + assert links_created[0]['version'] == 1 + assert links_created[0]['latest_version'] == 1 + assert links_created[0]['opaque_key'] == bank_lib_id + + assert links_created[1]['id'] == 'problem_bank_v1' + assert links_created[1]['bundle_uuid'] == bank_lib2['bundle_uuid'] + assert links_created[1]['version'] == 1 + assert links_created[1]['latest_version'] == 1 + assert links_created[1]['opaque_key'] == bank_lib2_id + + # Delete one of the linked bundles/libraries + self._delete_library(bank_lib2_id) + + # update the cache so we're not getting cached links in the next step + cache_key = 'bundle_version:{}:'.format(bank_lib['bundle_uuid']) + cache.delete(cache_key) + cache_key = 'bundle_version:{}:'.format(bank_lib2['bundle_uuid']) + cache.delete(cache_key) + + links_created = self._get_library_links(lib_id) + links_created.sort(key=lambda link: link["id"]) + assert len(links_created) == 2 + + assert links_created[0]['id'] == 'problem_bank' + assert links_created[0]['bundle_uuid'] == bank_lib['bundle_uuid'] + assert links_created[0]['version'] == 1 + assert links_created[0]['latest_version'] == 1 + assert links_created[0]['opaque_key'] == bank_lib_id + + # If a link has been deleted, the latest version will be 0, + # and the opaque key will be `None`. + assert links_created[1]['id'] == 'problem_bank_v1' + assert links_created[1]['bundle_uuid'] == bank_lib2['bundle_uuid'] + assert links_created[1]['version'] == 1 + assert links_created[1]['latest_version'] == 0 + assert links_created[1]['opaque_key'] is None + def test_library_blocks_limit(self): """ Test that libraries don't allow more than specified blocks