From 424c0c99e9994ba590d0e2ef6bf989925b8e62d0 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Tue, 21 Nov 2023 15:45:58 -0500 Subject: [PATCH] feat!: assume & remove BLOCKSTORE_USE_BLOCKSTORE_APP_API TODO add details Ref: https://github.com/openedx/blockstore/issues/296 --- cms/envs/common.py | 4 +- cms/envs/devstack-experimental.yml | 2 - cms/envs/test.py | 6 - lms/envs/common.py | 6 - lms/envs/devstack-experimental.yml | 2 - lms/envs/test.py | 5 - .../content_libraries/tests/base.py | 11 - .../tests/test_content_libraries.py | 11 - .../tests/test_libraries_index.py | 23 - .../content_libraries/tests/test_runtime.py | 33 +- .../tests/test_static_assets.py | 10 - .../content_libraries/tests/test_views_lti.py | 12 - .../djangolib/tests/test_blockstore_cache.py | 21 +- openedx/core/lib/blockstore_api/__init__.py | 5 +- .../lib/blockstore_api/config/__init__.py | 13 - .../core/lib/blockstore_api/config/waffle.py | 20 - openedx/core/lib/blockstore_api/methods.py | 496 ------------------ openedx/core/lib/blockstore_api/tests/base.py | 8 +- .../tests/test_blockstore_api.py | 10 - 19 files changed, 8 insertions(+), 690 deletions(-) delete mode 100644 openedx/core/lib/blockstore_api/config/__init__.py delete mode 100644 openedx/core/lib/blockstore_api/config/waffle.py delete mode 100644 openedx/core/lib/blockstore_api/methods.py diff --git a/cms/envs/common.py b/cms/envs/common.py index 11afd870a016..d8219ad11ca0 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -115,7 +115,6 @@ ENTERPRISE_BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL, # Blockstore - BLOCKSTORE_USE_BLOCKSTORE_APP_API, BUNDLE_ASSET_STORAGE_SETTINGS, # Methods to derive settings @@ -2606,8 +2605,7 @@ PROCTORING_SETTINGS = {} ################## BLOCKSTORE RELATED SETTINGS ######################### -BLOCKSTORE_PUBLIC_URL_ROOT = 'http://localhost:18250' -BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1/' + # Which of django's caches to use for storing anonymous user state for XBlocks # in the blockstore-based XBlock runtime XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'default' diff --git a/cms/envs/devstack-experimental.yml b/cms/envs/devstack-experimental.yml index 8aa2c304af81..54f6edf8f9f2 100644 --- a/cms/envs/devstack-experimental.yml +++ b/cms/envs/devstack-experimental.yml @@ -40,8 +40,6 @@ AWS_SES_REGION_ENDPOINT: email.us-east-1.amazonaws.com AWS_SES_REGION_NAME: us-east-1 AWS_STORAGE_BUCKET_NAME: SET-ME-PLEASE (ex. bucket-name) BASE_COOKIE_DOMAIN: localhost -BLOCKSTORE_API_URL: http://localhost:18250/api/v1 -BLOCKSTORE_PUBLIC_URL_ROOT: http://localhost:18250 BLOCK_STRUCTURES_SETTINGS: COURSE_PUBLISH_TASK_DELAY: 30 TASK_DEFAULT_RETRY_DELAY: 30 diff --git a/cms/envs/test.py b/cms/envs/test.py index e7be26b73c0e..f63b14c16775 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -27,8 +27,6 @@ # import settings from LMS for consistent behavior with CMS from lms.envs.test import ( # pylint: disable=wrong-import-order - BLOCKSTORE_USE_BLOCKSTORE_APP_API, - BLOCKSTORE_API_URL, COMPREHENSIVE_THEME_DIRS, # unimport:skip DEFAULT_FILE_STORAGE, ECOMMERCE_API_URL, @@ -184,10 +182,6 @@ } ############################### BLOCKSTORE ##################################### -# Blockstore tests -RUN_BLOCKSTORE_TESTS = os.environ.get('EDXAPP_RUN_BLOCKSTORE_TESTS', 'no').lower() in ('true', 'yes', '1') -BLOCKSTORE_API_URL = os.environ.get('EDXAPP_BLOCKSTORE_API_URL', "http://edx.devstack.blockstore-test:18251/api/v1/") -BLOCKSTORE_API_AUTH_TOKEN = os.environ.get('EDXAPP_BLOCKSTORE_API_AUTH_TOKEN', 'edxapp-test-key') BUNDLE_ASSET_STORAGE_SETTINGS = dict( STORAGE_CLASS='django.core.files.storage.FileSystemStorage', STORAGE_KWARGS=dict( diff --git a/lms/envs/common.py b/lms/envs/common.py index 59377b67cc5d..d328174a712d 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -5104,12 +5104,6 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring MAILCHIMP_NEW_USER_LIST_ID = "" ########################## BLOCKSTORE ##################################### -BLOCKSTORE_PUBLIC_URL_ROOT = 'http://localhost:18250' -BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1/' - -# Disable the Blockstore app API by default. -# See openedx.core.lib.blockstore_api.config for details. -BLOCKSTORE_USE_BLOCKSTORE_APP_API = False # .. setting_name: XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE # .. setting_default: default diff --git a/lms/envs/devstack-experimental.yml b/lms/envs/devstack-experimental.yml index 6d24b63cf920..63812686e86a 100644 --- a/lms/envs/devstack-experimental.yml +++ b/lms/envs/devstack-experimental.yml @@ -57,8 +57,6 @@ AWS_SES_REGION_ENDPOINT: email.us-east-1.amazonaws.com AWS_SES_REGION_NAME: us-east-1 AWS_STORAGE_BUCKET_NAME: SET-ME-PLEASE (ex. bucket-name) BASE_COOKIE_DOMAIN: localhost -BLOCKSTORE_API_URL: http://localhost:18250/api/v1 -BLOCKSTORE_PUBLIC_URL_ROOT: http://localhost:18250 BLOCK_STRUCTURES_SETTINGS: COURSE_PUBLISH_TASK_DELAY: 30 TASK_DEFAULT_RETRY_DELAY: 30 diff --git a/lms/envs/test.py b/lms/envs/test.py index 9e18344aa17f..37db856fb98f 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -557,11 +557,6 @@ derive_settings(__name__) ############################### BLOCKSTORE ##################################### -# Blockstore tests -RUN_BLOCKSTORE_TESTS = os.environ.get('EDXAPP_RUN_BLOCKSTORE_TESTS', 'no').lower() in ('true', 'yes', '1') -BLOCKSTORE_USE_BLOCKSTORE_APP_API = not RUN_BLOCKSTORE_TESTS -BLOCKSTORE_API_URL = os.environ.get('EDXAPP_BLOCKSTORE_API_URL', "http://edx.devstack.blockstore-test:18251/api/v1/") -BLOCKSTORE_API_AUTH_TOKEN = os.environ.get('EDXAPP_BLOCKSTORE_API_AUTH_TOKEN', 'edxapp-test-key') XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'blockstore' # This must be set to a working cache for the tests to pass BUNDLE_ASSET_STORAGE_SETTINGS = dict( STORAGE_CLASS='django.core.files.storage.FileSystemStorage', diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index dbf76142a784..65e4e43b959c 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -20,8 +20,6 @@ from openedx.core.lib import blockstore_api from openedx.core.lib.blockstore_api.tests.base import ( BlockstoreAppTestMixin, - requires_blockstore, - requires_blockstore_app, ) # Define the URLs here - don't use reverse() because we want to detect @@ -355,15 +353,6 @@ def _get_block_handler_url(self, block_key, handler_name): return self._api('get', url, None, expect_response=200)["handler_url"] -@requires_blockstore -class ContentLibrariesRestApiBlockstoreServiceTest(_ContentLibrariesRestApiTestMixin, APITestCase): - """ - Base class for Blockstore-based Content Libraries test that use the REST API - and the standalone Blockstore service. - """ - - -@requires_blockstore_app class ContentLibrariesRestApiTest( _ContentLibrariesRestApiTestMixin, BlockstoreAppTestMixin, 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 42d5bccf3442..7744df9c23f9 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -24,7 +24,6 @@ ) from openedx.core.djangoapps.content_libraries.libraries_index import LibraryBlockIndexer, ContentLibraryIndexer from openedx.core.djangoapps.content_libraries.tests.base import ( - ContentLibrariesRestApiBlockstoreServiceTest, ContentLibrariesRestApiTest, elasticsearch_test, URL_BLOCK_METADATA_URL, @@ -1232,16 +1231,6 @@ def test_library_block_delete_event(self): ) -@elasticsearch_test -class ContentLibrariesBlockstoreServiceTest( - ContentLibrariesTestMixin, - ContentLibrariesRestApiBlockstoreServiceTest, -): - """ - General tests for Blockstore-based Content Libraries, using the standalone Blockstore service. - """ - - @elasticsearch_test class ContentLibrariesTest( ContentLibrariesTestMixin, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py b/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py index 0e46680351af..5dc333350519 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_libraries_index.py @@ -11,7 +11,6 @@ from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryBlockIndexer from openedx.core.djangoapps.content_libraries.tests.base import ( - ContentLibrariesRestApiBlockstoreServiceTest, ContentLibrariesRestApiTest, elasticsearch_test, ) @@ -181,17 +180,6 @@ def verify_uncommitted_libraries(library_key, has_unpublished_changes, has_unpub verify_uncommitted_libraries(library_key, False, False) -@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True}) -@elasticsearch_test -class ContentLibraryIndexerBlockstoreServiceTest( - ContentLibraryIndexerTestMixin, - ContentLibrariesRestApiBlockstoreServiceTest, -): - """ - Tests the operation of ContentLibraryIndexer using the standalone Blockstore service. - """ - - @override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True}) @elasticsearch_test class ContentLibraryIndexerTest( @@ -303,17 +291,6 @@ def test_crud_block(self): assert LibraryBlockIndexer.get_items([block['id']]) == [] -@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True}) -@elasticsearch_test -class LibraryBlockIndexerBlockstoreServiceTest( - LibraryBlockIndexerTestMixin, - ContentLibrariesRestApiBlockstoreServiceTest, -): - """ - Tests the operation of LibraryBlockIndexer using the standalone Blockstore service. - """ - - @override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True}) @elasticsearch_test class LibraryBlockIndexerTest( diff --git a/openedx/core/djangoapps/content_libraries/tests/test_runtime.py b/openedx/core/djangoapps/content_libraries/tests/test_runtime.py index f166cee546ca..430306d936e2 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_runtime.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_runtime.py @@ -6,7 +6,7 @@ from completion.test_utils import CompletionWaffleTestMixin from django.db import connections, transaction -from django.test import LiveServerTestCase, TestCase +from django.test import LiveServerTestCase from django.utils.text import slugify from organizations.models import Organization from rest_framework.test import APIClient @@ -16,8 +16,6 @@ from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangoapps.content_libraries.tests.base import ( BlockstoreAppTestMixin, - requires_blockstore, - requires_blockstore_app, URL_BLOCK_RENDER_VIEW, URL_BLOCK_GET_HANDLER_URL, URL_BLOCK_METADATA_URL, @@ -224,14 +222,6 @@ def test_xblock_fields(self): assert xblock_api.get_block_display_name(block_saved) == 'New Display Name' -@requires_blockstore -class ContentLibraryRuntimeBServiceTest(ContentLibraryRuntimeTestMixin, TestCase): - """ - Tests XBlock runtime using XBlocks in a content library using the standalone Blockstore service. - """ - - -@requires_blockstore_app class ContentLibraryRuntimeTest(ContentLibraryRuntimeTestMixin, BlockstoreAppTestMixin, LiveServerTestCase): """ Tests XBlock runtime using XBlocks in a content library using the installed Blockstore app. @@ -545,14 +535,6 @@ def test_i18n(self): assert 'Submit' not in dummy_public_view.data['content'] -@requires_blockstore -class ContentLibraryXBlockUserStateBServiceTest(ContentLibraryXBlockUserStateTestMixin, TestCase): # type: ignore[misc] - """ - Tests XBlock user state for XBlocks in a content library using the standalone Blockstore service. - """ - - -@requires_blockstore_app class ContentLibraryXBlockUserStateTest( # type: ignore[misc] ContentLibraryXBlockUserStateTestMixin, BlockstoreAppTestMixin, @@ -619,19 +601,6 @@ def get_block_completion_status(): assert get_block_completion_status() == 1 -@requires_blockstore -class ContentLibraryXBlockCompletionBServiceTest( - ContentLibraryXBlockCompletionTestMixin, - CompletionWaffleTestMixin, - TestCase, -): - """ - Test that the Blockstore-based XBlocks can track their completion status - using the standalone Blockstore service. - """ - - -@requires_blockstore_app class ContentLibraryXBlockCompletionTest( ContentLibraryXBlockCompletionTestMixin, CompletionWaffleTestMixin, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py b/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py index 7004860f9d4c..e330101eb3bc 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py @@ -3,7 +3,6 @@ """ from openedx.core.djangoapps.content_libraries.tests.base import ( - ContentLibrariesRestApiBlockstoreServiceTest, ContentLibrariesRestApiTest, ) @@ -109,15 +108,6 @@ def check_download(): check_download() -class ContentLibrariesStaticAssetsBlockstoreServiceTest( - ContentLibrariesStaticAssetsTestMixin, - ContentLibrariesRestApiBlockstoreServiceTest, -): - """ - Tests for static asset files in Blockstore-based Content Libraries, using the standalone Blockstore service. - """ - - class ContentLibrariesStaticAssetsTest( ContentLibrariesStaticAssetsTestMixin, ContentLibrariesRestApiTest, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py b/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py index 58dd7e9b214a..cb306ebbfe48 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py @@ -8,7 +8,6 @@ from openedx.core.djangoapps.content_libraries.constants import PROBLEM from .base import ( - ContentLibrariesRestApiBlockstoreServiceTest, ContentLibrariesRestApiTest, URL_LIB_LTI_JWKS, skip_unless_cms, @@ -81,17 +80,6 @@ def test_block_not_found(self): self._api("get", '/api/libraries/v2/blocks/lb:CL-TEST:libgg:problem:bad-block/lti/', None, expect_response=404) -@override_features(ENABLE_CONTENT_LIBRARIES=True, - ENABLE_CONTENT_LIBRARIES_LTI_TOOL=True) -class LibraryBlockLtiUrlViewBlockstoreServiceTest( - LibraryBlockLtiUrlViewTestMixin, - ContentLibrariesRestApiBlockstoreServiceTest, -): - """ - Test generating LTI URL for a block in a library, using the standalone Blockstore service. - """ - - @override_features(ENABLE_CONTENT_LIBRARIES=True, ENABLE_CONTENT_LIBRARIES_LTI_TOOL=True) class LibraryBlockLtiUrlViewTest( diff --git a/openedx/core/djangolib/tests/test_blockstore_cache.py b/openedx/core/djangolib/tests/test_blockstore_cache.py index 5c55e07f449b..5ad1993d3ecd 100644 --- a/openedx/core/djangolib/tests/test_blockstore_cache.py +++ b/openedx/core/djangolib/tests/test_blockstore_cache.py @@ -5,11 +5,6 @@ from django.test import TestCase from openedx.core.djangolib.blockstore_cache import BundleCache -from openedx.core.lib.blockstore_api.tests.base import ( - BlockstoreAppTestMixin, - requires_blockstore, - requires_blockstore_app, -) from openedx.core.lib import blockstore_api as api @@ -27,7 +22,7 @@ def setUpClass(cls): @patch('openedx.core.djangolib.blockstore_cache.MAX_BLOCKSTORE_CACHE_DELAY', 0) -class BundleCacheTestMixin(TestWithBundleMixin): +class BundleCacheTestMixin(TestWithBundleMixin, TestCase): """ Tests for BundleCache """ @@ -112,17 +107,3 @@ def test_bundle_cache_clear(self): # Now "clear" the cache, forcing the check of the new version: cache.clear() assert cache.get(key1) is None - - -@requires_blockstore -class BundleCacheBlockstoreServiceTest(BundleCacheTestMixin, TestCase): - """ - Tests BundleCache using the standalone Blockstore service. - """ - - -@requires_blockstore_app -class BundleCacheTest(BundleCacheTestMixin, BlockstoreAppTestMixin, TestCase): - """ - Tests BundleCache using the installed Blockstore app. - """ diff --git a/openedx/core/lib/blockstore_api/__init__.py b/openedx/core/lib/blockstore_api/__init__.py index 50b352578cdf..d9855ef1812f 100644 --- a/openedx/core/lib/blockstore_api/__init__.py +++ b/openedx/core/lib/blockstore_api/__init__.py @@ -4,6 +4,9 @@ This API does not do any caching; consider using BundleCache or (in openedx.core.djangolib.blockstore_cache) together with these API methods for improved performance. + +TODO: This wrapper is extraneous now that Blockstore-as-a-service isn't supported. + This whole directory tree should be removed by https://github.com/openedx/blockstore/issues/296. """ from blockstore.apps.api.data import ( BundleFileData, @@ -16,7 +19,7 @@ BundleFileNotFound, BundleStorageError, ) -from .methods import ( +from blockstore.apps.api.methods import ( # Collections: get_collection, create_collection, diff --git a/openedx/core/lib/blockstore_api/config/__init__.py b/openedx/core/lib/blockstore_api/config/__init__.py deleted file mode 100644 index 4cd2999d2e32..000000000000 --- a/openedx/core/lib/blockstore_api/config/__init__.py +++ /dev/null @@ -1,13 +0,0 @@ -""" -Helper method to indicate when the blockstore app API is enabled. -""" -from django.conf import settings -from .waffle import BLOCKSTORE_USE_BLOCKSTORE_APP_API # pylint: disable=invalid-django-waffle-import - - -def use_blockstore_app(): - """ - Use the Blockstore app API if the settings say to (e.g. in test) - or if the waffle switch is enabled. - """ - return settings.BLOCKSTORE_USE_BLOCKSTORE_APP_API or BLOCKSTORE_USE_BLOCKSTORE_APP_API.is_enabled() diff --git a/openedx/core/lib/blockstore_api/config/waffle.py b/openedx/core/lib/blockstore_api/config/waffle.py deleted file mode 100644 index 192d4d5c3ab2..000000000000 --- a/openedx/core/lib/blockstore_api/config/waffle.py +++ /dev/null @@ -1,20 +0,0 @@ -""" -Toggles for blockstore. -""" - -from edx_toggles.toggles import WaffleSwitch - -# .. toggle_name: blockstore.use_blockstore_app_api -# .. toggle_implementation: WaffleSwitch -# .. toggle_default: False -# .. toggle_description: Enable to use the installed blockstore app's Python API directly instead of the -# external blockstore service REST API. -# The blockstore REST API is used by default. -# .. toggle_use_cases: temporary, open_edx -# .. toggle_creation_date: 2022-01-13 -# .. toggle_target_removal_date: None -# .. toggle_tickets: TNL-8705, BD-14 -# .. toggle_warning: This temporary feature toggle does not have a target removal date. -BLOCKSTORE_USE_BLOCKSTORE_APP_API = WaffleSwitch( - 'blockstore.use_blockstore_app_api', __name__ -) diff --git a/openedx/core/lib/blockstore_api/methods.py b/openedx/core/lib/blockstore_api/methods.py deleted file mode 100644 index 86b4730efe1b..000000000000 --- a/openedx/core/lib/blockstore_api/methods.py +++ /dev/null @@ -1,496 +0,0 @@ -""" -API Client methods for working with Blockstore bundles and drafts -""" - -import base64 -from functools import wraps -from urllib.parse import urlencode -from uuid import UUID - -import dateutil.parser -from django.conf import settings -from django.core.exceptions import ImproperlyConfigured -import requests - -from blockstore.apps.api.data import ( - BundleData, - CollectionData, - DraftData, - BundleVersionData, - BundleFileData, - DraftFileData, - BundleLinkData, - DraftLinkData, - Dependency, -) -from blockstore.apps.api.exceptions import ( - NotFound, - CollectionNotFound, - BundleNotFound, - DraftNotFound, - BundleFileNotFound, -) -import blockstore.apps.api.methods as blockstore_api_methods - -from .config import use_blockstore_app - - -def toggle_blockstore_api(func): - """ - Decorator function to toggle usage of the Blockstore service - and the in-built Blockstore app dependency. - """ - @wraps(func) - def wrapper(*args, **kwargs): - if use_blockstore_app(): - return getattr(blockstore_api_methods, func.__name__)(*args, **kwargs) - return func(*args, **kwargs) - return wrapper - - -def api_url(*path_parts): - if not settings.BLOCKSTORE_API_URL or not settings.BLOCKSTORE_API_URL.endswith('/api/v1/'): - raise ImproperlyConfigured('BLOCKSTORE_API_URL must be set and should end with /api/v1/') - return settings.BLOCKSTORE_API_URL + '/'.join(path_parts) - - -def api_request(method, url, **kwargs): - """ - Helper method for making a request to the Blockstore REST API - """ - if not settings.BLOCKSTORE_API_AUTH_TOKEN: - raise ImproperlyConfigured("Cannot use Blockstore unless BLOCKSTORE_API_AUTH_TOKEN is set.") - kwargs.setdefault('headers', {})['Authorization'] = f"Token {settings.BLOCKSTORE_API_AUTH_TOKEN}" - response = requests.request(method, url, **kwargs) - if response.status_code == 404: - raise NotFound - response.raise_for_status() - if response.status_code == 204: - return None # No content - return response.json() - - -def _collection_from_response(data): - """ - Given data about a Collection returned by any blockstore REST API, convert it to - a CollectionData instance. - """ - return CollectionData(uuid=UUID(data['uuid']), title=data['title']) - - -def _bundle_from_response(data): - """ - Given data about a Bundle returned by any blockstore REST API, convert it to - a BundleData instance. - """ - return BundleData( - uuid=UUID(data['uuid']), - title=data['title'], - description=data['description'], - slug=data['slug'], - # drafts: Convert from a dict of URLs to a dict of UUIDs: - drafts={draft_name: UUID(url.split('/')[-1]) for (draft_name, url) in data['drafts'].items()}, - # versions field: take the last one and convert it from URL to an int - # i.e.: [..., 'https://blockstore/api/v1/bundle_versions/bundle_uuid,15'] -> 15 - latest_version=int(data['versions'][-1].split(',')[-1]) if data['versions'] else 0, - ) - - -def _bundle_version_from_response(data): - """ - Given data about a BundleVersion returned by any blockstore REST API, convert it to - a BundleVersionData instance. - """ - return BundleVersionData( - bundle_uuid=UUID(data['bundle_uuid']), - version=data.get('version', 0), - change_description=data['change_description'], - created_at=dateutil.parser.parse(data['snapshot']['created_at']), - files={ - path: BundleFileData(path=path, **filedata) - for path, filedata in data['snapshot']['files'].items() - }, - links={ - name: BundleLinkData( - name=name, - direct=Dependency(**link["direct"]), - indirect=[Dependency(**ind) for ind in link["indirect"]], - ) - for name, link in data['snapshot']['links'].items() - } - ) - - -def _draft_from_response(data): - """ - Given data about a Draft returned by any blockstore REST API, convert it to - a DraftData instance. - """ - return DraftData( - uuid=UUID(data['uuid']), - bundle_uuid=UUID(data['bundle_uuid']), - name=data['name'], - created_at=dateutil.parser.parse(data['staged_draft']['created_at']), - updated_at=dateutil.parser.parse(data['staged_draft']['updated_at']), - files={ - path: DraftFileData(path=path, **file) - for path, file in data['staged_draft']['files'].items() - }, - links={ - name: DraftLinkData( - name=name, - direct=Dependency(**link["direct"]), - indirect=[Dependency(**ind) for ind in link["indirect"]], - modified=link["modified"], - ) - for name, link in data['staged_draft']['links'].items() - } - ) - - -@toggle_blockstore_api -def get_collection(collection_uuid): - """ - Retrieve metadata about the specified collection - - Raises CollectionNotFound if the collection does not exist - """ - assert isinstance(collection_uuid, UUID) - try: - data = api_request('get', api_url('collections', str(collection_uuid))) - except NotFound: - raise CollectionNotFound(f"Collection {collection_uuid} does not exist.") # lint-amnesty, pylint: disable=raise-missing-from - return _collection_from_response(data) - - -@toggle_blockstore_api -def create_collection(title): - """ - Create a new collection. - """ - result = api_request('post', api_url('collections'), json={"title": title}) - return _collection_from_response(result) - - -@toggle_blockstore_api -def update_collection(collection_uuid, title): - """ - Update a collection's title - """ - assert isinstance(collection_uuid, UUID) - data = {"title": title} - result = api_request('patch', api_url('collections', str(collection_uuid)), json=data) - return _collection_from_response(result) - - -@toggle_blockstore_api -def delete_collection(collection_uuid): - """ - Delete a collection - """ - assert isinstance(collection_uuid, UUID) - api_request('delete', api_url('collections', str(collection_uuid))) - - -@toggle_blockstore_api -def get_bundles(uuids=None, text_search=None): - """ - Get the details of all bundles. - """ - query_params = {} - data = {} - if uuids: - # Potentially we could have a lot of libraries which will lead to 414 error (Request-URI Too Long) - # if sending uuids in the query_params. So we have to use the request data instead. - data = {'uuid': ','.join(map(str, uuids))} - if text_search: - query_params['text_search'] = text_search - version_url = api_url('bundles') + '?' + urlencode(query_params) - response = api_request('get', version_url, json=data) - # build bundle from response, convert map object to list and return - return [_bundle_from_response(item) for item in response] - - -@toggle_blockstore_api -def get_bundle(bundle_uuid): - """ - Retrieve metadata about the specified bundle - - Raises BundleNotFound if the bundle does not exist - """ - assert isinstance(bundle_uuid, UUID) - try: - data = api_request('get', api_url('bundles', str(bundle_uuid))) - except NotFound: - raise BundleNotFound(f"Bundle {bundle_uuid} does not exist.") # lint-amnesty, pylint: disable=raise-missing-from - return _bundle_from_response(data) - - -@toggle_blockstore_api -def create_bundle(collection_uuid, slug, title="New Bundle", description=""): - """ - Create a new bundle. - - Note that description is currently required. - """ - result = api_request('post', api_url('bundles'), json={ - "collection_uuid": str(collection_uuid), - "slug": slug, - "title": title, - "description": description, - }) - return _bundle_from_response(result) - - -@toggle_blockstore_api -def update_bundle(bundle_uuid, **fields): - """ - Update a bundle's title, description, slug, or collection. - """ - assert isinstance(bundle_uuid, UUID) - data = {} - # Most validation will be done by Blockstore, so we don't worry too much about data validation - for str_field in ("title", "description", "slug"): - if str_field in fields: - data[str_field] = fields.pop(str_field) - if "collection_uuid" in fields: - data["collection_uuid"] = str(fields.pop("collection_uuid")) - if fields: - raise ValueError(f"Unexpected extra fields passed " - f"to update_bundle: {fields.keys()}") - result = api_request('patch', api_url('bundles', str(bundle_uuid)), json=data) - return _bundle_from_response(result) - - -@toggle_blockstore_api -def delete_bundle(bundle_uuid): - """ - Delete a bundle - """ - assert isinstance(bundle_uuid, UUID) - api_request('delete', api_url('bundles', str(bundle_uuid))) - - -@toggle_blockstore_api -def get_draft(draft_uuid): - """ - Retrieve metadata about the specified draft. - If you don't know the draft's UUID, look it up using get_bundle() - """ - assert isinstance(draft_uuid, UUID) - try: - data = api_request('get', api_url('drafts', str(draft_uuid))) - except NotFound: - raise DraftNotFound(f"Draft does not exist: {draft_uuid}") # lint-amnesty, pylint: disable=raise-missing-from - return _draft_from_response(data) - - -@toggle_blockstore_api -def get_or_create_bundle_draft(bundle_uuid, draft_name): - """ - Retrieve metadata about the specified draft. - """ - bundle = get_bundle(bundle_uuid) - try: - return get_draft(bundle.drafts[draft_name]) # pylint: disable=unsubscriptable-object - except KeyError: - # The draft doesn't exist yet, so create it: - response = api_request('post', api_url('drafts'), json={ - "bundle_uuid": str(bundle_uuid), - "name": draft_name, - }) - # The result of creating a draft doesn't include all the fields we want, so retrieve it now: - return get_draft(UUID(response["uuid"])) - - -@toggle_blockstore_api -def commit_draft(draft_uuid): - """ - Commit all of the pending changes in the draft, creating a new version of - the associated bundle. - - Does not return any value. - """ - api_request('post', api_url('drafts', str(draft_uuid), 'commit')) - - -@toggle_blockstore_api -def delete_draft(draft_uuid): - """ - Delete the specified draft, removing any staged changes/files/deletes. - - Does not return any value. - """ - api_request('delete', api_url('drafts', str(draft_uuid))) - - -@toggle_blockstore_api -def get_bundle_version(bundle_uuid, version_number): - """ - Get the details of the specified bundle version - """ - if version_number == 0: - return None - version_url = api_url('bundle_versions', str(bundle_uuid) + ',' + str(version_number)) - return _bundle_version_from_response(api_request('get', version_url)) - - -@toggle_blockstore_api -def get_bundle_version_files(bundle_uuid, version_number): - """ - Get a list of the files in the specified bundle version - """ - if version_number == 0: - return [] - version_info = get_bundle_version(bundle_uuid, version_number) - return list(version_info.files.values()) - - -@toggle_blockstore_api -def get_bundle_version_links(bundle_uuid, version_number): - """ - Get a dictionary of the links in the specified bundle version - """ - if version_number == 0: - return {} - version_info = get_bundle_version(bundle_uuid, version_number) - return version_info.links - - -@toggle_blockstore_api -def get_bundle_files_dict(bundle_uuid, use_draft=None): - """ - Get a dict of all the files in the specified bundle. - - Returns a dict where the keys are the paths (strings) and the values are - BundleFileData or DraftFileData tuples. - """ - bundle = get_bundle(bundle_uuid) - if use_draft and use_draft in bundle.drafts: # pylint: disable=unsupported-membership-test - draft_uuid = bundle.drafts[use_draft] # pylint: disable=unsubscriptable-object - return get_draft(draft_uuid).files - elif not bundle.latest_version: - # This bundle has no versions so definitely does not contain any files - return {} - else: - return {file_meta.path: file_meta for file_meta in get_bundle_version_files(bundle_uuid, bundle.latest_version)} - - -@toggle_blockstore_api -def get_bundle_files(bundle_uuid, use_draft=None): - """ - Get an iterator over all the files in the specified bundle or draft. - """ - return get_bundle_files_dict(bundle_uuid, use_draft).values() - - -@toggle_blockstore_api -def get_bundle_links(bundle_uuid, use_draft=None): - """ - Get a dict of all the links in the specified bundle. - - Returns a dict where the keys are the link names (strings) and the values - are BundleLinkData or DraftLinkData tuples. - """ - bundle = get_bundle(bundle_uuid) - if use_draft and use_draft in bundle.drafts: # pylint: disable=unsupported-membership-test - draft_uuid = bundle.drafts[use_draft] # pylint: disable=unsubscriptable-object - return get_draft(draft_uuid).links - elif not bundle.latest_version: - # This bundle has no versions so definitely does not contain any links - return {} - else: - return get_bundle_version_links(bundle_uuid, bundle.latest_version) - - -@toggle_blockstore_api -def get_bundle_file_metadata(bundle_uuid, path, use_draft=None): - """ - Get the metadata of the specified file. - """ - assert isinstance(bundle_uuid, UUID) - files_dict = get_bundle_files_dict(bundle_uuid, use_draft=use_draft) - try: - return files_dict[path] - except KeyError: - raise BundleFileNotFound( # lint-amnesty, pylint: disable=raise-missing-from - f"Bundle {bundle_uuid} (draft: {use_draft}) does not contain a file {path}" - ) - - -@toggle_blockstore_api -def get_bundle_file_data(bundle_uuid, path, use_draft=None): - """ - Read all the data in the given bundle file and return it as a - binary string. - - Do not use this for large files! - """ - metadata = get_bundle_file_metadata(bundle_uuid, path, use_draft) - with requests.get(metadata.url, stream=True) as r: - return r.content - - -@toggle_blockstore_api -def write_draft_file(draft_uuid, path, contents): - """ - Create or overwrite the file at 'path' in the specified draft with the given - contents. To delete a file, pass contents=None. - - If you don't know the draft's UUID, look it up using - get_or_create_bundle_draft() - - Does not return anything. - """ - api_request('patch', api_url('drafts', str(draft_uuid)), json={ - 'files': { - path: _encode_str_for_draft(contents) if contents is not None else None, - }, - }) - - -@toggle_blockstore_api -def set_draft_link(draft_uuid, link_name, bundle_uuid, version): - """ - Create or replace the link with the given name in the specified draft so - that it points to the specified bundle version. To delete a link, pass - bundle_uuid=None, version=None. - - If you don't know the draft's UUID, look it up using - get_or_create_bundle_draft() - - Does not return anything. - """ - api_request('patch', api_url('drafts', str(draft_uuid)), json={ - 'links': { - link_name: {"bundle_uuid": str(bundle_uuid), "version": version} if bundle_uuid is not None else None, - }, - }) - - -def _encode_str_for_draft(input_str): - """ - Given a string, return UTF-8 representation that is then base64 encoded. - """ - if isinstance(input_str, str): - binary = input_str.encode('utf8') - else: - binary = input_str - return base64.b64encode(binary) - - -@toggle_blockstore_api -def force_browser_url(blockstore_file_url): - """ - Ensure that the given devstack URL is a URL accessible from the end user's browser. - """ - # Hack: on some devstacks, we must necessarily use different URLs for - # accessing Blockstore file data from within and outside of docker - # containers, but Blockstore has no way of knowing which case any particular - # request is for. So it always returns a URL suitable for use from within - # the container. Only this edxapp can transform the URL at the last second, - # knowing that in this case it's going to the user's browser and not being - # read by edxapp. - # In production, the same S3 URLs get used for internal and external access - # so this hack is not necessary. - return blockstore_file_url.replace('http://edx.devstack.blockstore:', 'http://localhost:') diff --git a/openedx/core/lib/blockstore_api/tests/base.py b/openedx/core/lib/blockstore_api/tests/base.py index 0888e8c81dc7..1d202d7671b5 100644 --- a/openedx/core/lib/blockstore_api/tests/base.py +++ b/openedx/core/lib/blockstore_api/tests/base.py @@ -1,17 +1,11 @@ """ Common code for tests that work with Blockstore """ -from unittest import mock, skipUnless +from unittest import mock from urllib.parse import urlparse -from django.conf import settings from django.test.client import RequestFactory -# Decorators for tests that require the blockstore service/app -requires_blockstore = skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server") - -requires_blockstore_app = skipUnless(settings.BLOCKSTORE_USE_BLOCKSTORE_APP_API, "Requires blockstore app") - class BlockstoreAppTestMixin: """ diff --git a/openedx/core/lib/blockstore_api/tests/test_blockstore_api.py b/openedx/core/lib/blockstore_api/tests/test_blockstore_api.py index 9c15e16668d0..859b24e01d03 100644 --- a/openedx/core/lib/blockstore_api/tests/test_blockstore_api.py +++ b/openedx/core/lib/blockstore_api/tests/test_blockstore_api.py @@ -10,8 +10,6 @@ from openedx.core.lib import blockstore_api as api from openedx.core.lib.blockstore_api.tests.base import ( BlockstoreAppTestMixin, - requires_blockstore, - requires_blockstore_app, ) # A fake UUID that won't represent any real bundle/draft/collection: @@ -197,14 +195,6 @@ def test_links(self): assert not api.get_bundle_links(course_bundle.uuid, use_draft=course_draft.name) -@requires_blockstore -class BlockstoreServiceApiClientTest(BlockstoreApiClientTestMixin, TestCase): - """ - Test the Blockstore API Client, using the standalone Blockstore service. - """ - - -@requires_blockstore_app class BlockstoreAppApiClientTest(BlockstoreApiClientTestMixin, BlockstoreAppTestMixin, TestCase): """ Test the Blockstore API Client, using the installed Blockstore app.