diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 9dc6570894..0366a87d46 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1,8 +1,39 @@ """ Python API for content libraries. -Unless otherwise specified, all APIs in this file deal with the DRAFT version -of the content library. +Via 'views.py', most of these API methods are also exposed as a REST API. + +The API methods in this file are focused on authoring and specific to content +libraries; they wouldn't necessarily apply or work in other learning contexts +such as courses, blogs, "pathways," etc. + +** As this is an authoring-focused API, all API methods in this file deal with +the DRAFT version of the content library. ** + +Some of these methods will work and may be used from the LMS if needed (mostly +for test setup; other use is discouraged), but some of the implementation +details rely on Studio so other methods will raise errors if called from the +LMS. (The REST API is not available at all from the LMS.) + +Any APIs that use/affect content libraries but are generic enough to work in +other learning contexts too are in the core XBlock python/REST API at + openedx.core.djangoapps.xblock.api/rest_api + +For example, to render a content library XBlock as HTML, one can use the generic + render_block_view(block, view_name, user) +API in openedx.core.djangoapps.xblock.api (use it from Studio for the draft +version, from the LMS for published version). + +There are one or two methods in this file that have some overlap with the core +XBlock API; for example, this content library API provides a get_library_block() +which returns metadata about an XBlock; it's in this API because it also returns +data about whether or not the XBlock has unpublished edits, which is an +authoring-only concern. Likewise, APIs for getting/setting an individual +XBlock's OLX directly seem more appropriate for small, reusable components in +content libraries and may not be appropriate for other learning contexts so they +are implemented here in the library API only. In the future, if we find a need +for these in most other learning contexts then those methods could be promoted +to the core XBlock API and made generic. """ from uuid import UUID import logging @@ -13,6 +44,7 @@ from django.core.exceptions import PermissionDenied from django.core.validators import validate_unicode_slug from django.db import IntegrityError from lxml import etree +from opaque_keys.edx.keys import LearningContextKey from opaque_keys.edx.locator import BundleDefinitionLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from organizations.models import Organization import six @@ -34,6 +66,7 @@ from openedx.core.lib.blockstore_api import ( update_bundle, delete_bundle, write_draft_file, + set_draft_link, commit_draft, delete_draft, ) @@ -69,7 +102,7 @@ class InvalidNameError(ValueError): # Models: @attr.s -class ContentLibraryMetadata(object): +class ContentLibraryMetadata: """ Class that represents the metadata about a content library. """ @@ -90,7 +123,7 @@ class ContentLibraryMetadata(object): allow_public_read = attr.ib(False) -class AccessLevel(object): +class AccessLevel: """ Enum defining library access levels/permissions """ ADMIN_LEVEL = ContentLibraryPermission.ADMIN_LEVEL AUTHOR_LEVEL = ContentLibraryPermission.AUTHOR_LEVEL @@ -99,7 +132,7 @@ class AccessLevel(object): @attr.s -class ContentLibraryPermissionEntry(object): +class ContentLibraryPermissionEntry: """ A user or group granted permission to use a content library. """ @@ -109,7 +142,7 @@ class ContentLibraryPermissionEntry(object): @attr.s -class LibraryXBlockMetadata(object): +class LibraryXBlockMetadata: """ Class that represents the metadata about an XBlock in a content library. """ @@ -120,7 +153,7 @@ class LibraryXBlockMetadata(object): @attr.s -class LibraryXBlockStaticFile(object): +class LibraryXBlockStaticFile: """ Class that represents a static file in a content library, associated with a particular XBlock. @@ -135,7 +168,7 @@ class LibraryXBlockStaticFile(object): @attr.s -class LibraryXBlockType(object): +class LibraryXBlockType: """ An XBlock type that can be added to a content library """ @@ -143,6 +176,33 @@ class LibraryXBlockType(object): display_name = attr.ib("") +@attr.s +class LibraryBundleLink: + """ + A link from a content library blockstore bundle to another blockstore bundle + """ + # Bundle that is linked to + bundle_uuid = attr.ib(type=UUID) + # Link name (slug) + id = attr.ib("") + # What version of this bundle we are currently linking to. + version = attr.ib(0) + # What the latest version of the linked bundle is: + # (if latest_version > version), the link can be "updated" to the latest version. + latest_version = attr.ib(0) + # Opaque key: If the linked bundle is a library or other learning context whose opaque key we can deduce, then this + # is the key. If we don't know what type of blockstore bundle this link is pointing to, then this is blank. + opaque_key = attr.ib(type=LearningContextKey, default=None) + + +class AccessLevel: + """ Enum defining library access levels/permissions """ + ADMIN_LEVEL = ContentLibraryPermission.ADMIN_LEVEL + AUTHOR_LEVEL = ContentLibraryPermission.AUTHOR_LEVEL + READ_LEVEL = ContentLibraryPermission.READ_LEVEL + NO_ACCESS = None + + def list_libraries_for_user(user): """ Lists up to 50 content libraries that the user has permission to view. @@ -643,6 +703,93 @@ def get_allowed_block_types(library_key): # pylint: disable=unused-argument return info +def get_bundle_links(library_key): + """ + Get the list of bundles/libraries linked to this content library. + + Returns LibraryBundleLink objects (defined above). + + Because every content library is a blockstore bundle, it can have "links" to + other bundles, which may or may not be content libraries. This allows using + XBlocks (or perhaps even static assets etc.) from another bundle without + needing to duplicate/copy the data. + + Links always point to a specific published version of the target bundle. + Links are identified by a slug-like ID, e.g. "link1" + """ + ref = ContentLibrary.objects.get_by_key(library_key) + links = blockstore_cache.get_bundle_draft_direct_links_cached(ref.bundle_uuid, DRAFT_NAME) + results = [] + # To be able to quickly get the library ID from the bundle ID for links which point to other libraries, build a map: + bundle_uuids = set(link_data.bundle_uuid for link_data in links.values()) + libraries_linked = { + lib.bundle_uuid: lib + for lib in ContentLibrary.objects.select_related('org').filter(bundle_uuid__in=bundle_uuids) + } + for link_name, link_data in links.items(): + # Is this linked bundle a content library? + try: + opaque_key = libraries_linked[link_data.bundle_uuid].library_key + except KeyError: + opaque_key = None + # Append the link information: + 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), + opaque_key=opaque_key, + )) + return results + + +def create_bundle_link(library_key, link_id, target_opaque_key, version=None): + """ + Create a new link to the resource with the specified opaque key. + + For now, only LibraryLocatorV2 opaque keys are supported. + """ + ref = ContentLibrary.objects.get_by_key(library_key) + # Make sure this link ID/name is not already in use: + links = blockstore_cache.get_bundle_draft_direct_links_cached(ref.bundle_uuid, DRAFT_NAME) + if link_id in links: + raise InvalidNameError("That link ID is already in use.") + # Determine the target: + if not isinstance(target_opaque_key, LibraryLocatorV2): + raise TypeError("For now, only LibraryLocatorV2 opaque keys are supported by create_bundle_link") + target_bundle_uuid = ContentLibrary.objects.get_by_key(target_opaque_key).bundle_uuid + if version is None: + version = get_bundle(target_bundle_uuid).latest_version + # Create the new link: + draft = get_or_create_bundle_draft(ref.bundle_uuid, DRAFT_NAME) + set_draft_link(draft.uuid, link_id, target_bundle_uuid, version) + # Clear the cache: + LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME).cache.clear() + + +def update_bundle_link(library_key, link_id, version=None, delete=False): + """ + Update a bundle's link to point to the specified version of its target + bundle. Use version=None to automatically point to the latest version. + Use delete=True to delete the link. + """ + ref = ContentLibrary.objects.get_by_key(library_key) + draft = get_or_create_bundle_draft(ref.bundle_uuid, DRAFT_NAME) + if delete: + set_draft_link(draft.uuid, link_id, None, None) + else: + links = blockstore_cache.get_bundle_draft_direct_links_cached(ref.bundle_uuid, DRAFT_NAME) + try: + link = links[link_id] + except KeyError: + raise InvalidNameError("That link does not exist.") + if version is None: + version = get_bundle(link.bundle_uuid).latest_version + set_draft_link(draft.uuid, link_id, link.bundle_uuid, version) + # Clear the cache: + LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME).cache.clear() + + def publish_changes(library_key): """ Publish all pending changes to the specified library. diff --git a/openedx/core/djangoapps/content_libraries/library_bundle.py b/openedx/core/djangoapps/content_libraries/library_bundle.py index 954113a08c..27570a2ff8 100644 --- a/openedx/core/djangoapps/content_libraries/library_bundle.py +++ b/openedx/core/djangoapps/content_libraries/library_bundle.py @@ -18,6 +18,7 @@ from openedx.core.djangoapps.xblock.runtime.olx_parsing import ( ) from openedx.core.djangolib.blockstore_cache import ( BundleCache, + get_bundle_direct_links_with_cache, get_bundle_files_cached, get_bundle_file_metadata_with_cache, get_bundle_version_number, @@ -299,6 +300,12 @@ class LibraryBundle(object): has_unpublished_changes = True break + if not has_unpublished_changes: + # Check if any links have changed: + old_links = set(get_bundle_direct_links_with_cache(self.bundle_uuid).items()) + new_links = set(get_bundle_direct_links_with_cache(self.bundle_uuid, draft_name=self.draft_name).items()) + has_unpublished_changes = new_links != old_links + published_file_paths = set(f.path for f in get_bundle_files_cached(self.bundle_uuid)) draft_file_paths = set(f.path for f in draft_files) for file_path in published_file_paths: diff --git a/openedx/core/djangoapps/content_libraries/serializers.py b/openedx/core/djangoapps/content_libraries/serializers.py index 998df00881..e13f57c083 100644 --- a/openedx/core/djangoapps/content_libraries/serializers.py +++ b/openedx/core/djangoapps/content_libraries/serializers.py @@ -84,6 +84,32 @@ class LibraryXBlockTypeSerializer(serializers.Serializer): display_name = serializers.CharField() +class LibraryBundleLinkSerializer(serializers.Serializer): + """ + Serializer for a link from a content library blockstore bundle to another + blockstore bundle. + """ + id = serializers.SlugField() # Link name + bundle_uuid = serializers.UUIDField(format='hex_verbose', read_only=True) + # What version of this bundle we are currently linking to. + # This is never NULL but can optionally be set to null when creating a new link, which means "use latest version." + version = serializers.IntegerField(allow_null=True) + # What the latest version of the linked bundle is: + # (if latest_version > version), the link can be "updated" to the latest version. + latest_version = serializers.IntegerField(read_only=True) + # Opaque key: If the linked bundle is a library or other learning context whose opaque key we can deduce, then this + # is the key. If we don't know what type of blockstore bundle this link is pointing to, then this is blank. + opaque_key = serializers.CharField() + + +class LibraryBundleLinkUpdateSerializer(serializers.Serializer): + """ + Serializer for updating an existing link in a content library blockstore + bundle. + """ + version = serializers.IntegerField(allow_null=True) + + class LibraryXBlockCreationSerializer(serializers.Serializer): """ Serializer for adding a new XBlock to a content library diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 68a6ff2902..490ff4052b 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -3,12 +3,12 @@ Tests for Blockstore-based Content Libraries """ from contextlib import contextmanager +from io import BytesIO import unittest from django.conf import settings from organizations.models import Organization from rest_framework.test import APITestCase, APIClient -import six from student.tests.factories import UserFactory from openedx.core.djangolib.testing.utils import skip_unless_cms @@ -20,6 +20,7 @@ URL_PREFIX = '/api/libraries/v2/' URL_LIB_CREATE = URL_PREFIX URL_LIB_DETAIL = URL_PREFIX + '{lib_key}/' # Get data about a library, update or delete library URL_LIB_BLOCK_TYPES = URL_LIB_DETAIL + 'block_types/' # Get the list of XBlock types that can be added to this library +URL_LIB_LINKS = URL_LIB_DETAIL + 'links/' # Get the list of links in this library, or add a new one URL_LIB_COMMIT = URL_LIB_DETAIL + 'commit/' # Commit (POST) or revert (DELETE) all pending changes to this library URL_LIB_BLOCKS = URL_LIB_DETAIL + 'blocks/' # Get the list of XBlocks in this library, or add a new one URL_LIB_TEAM = URL_LIB_DETAIL + 'team/' # Get the list of users/groups authorized to use this library @@ -67,7 +68,7 @@ class ContentLibrariesRestApiTest(APITestCase): @classmethod def setUpClass(cls): - super(ContentLibrariesRestApiTest, cls).setUpClass() + super().setUpClass() cls.user = UserFactory.create(username="Bob", email="bob@example.com", password="edx") # Create a collection using Blockstore API directly only because there # is not yet any Studio REST API for doing so: @@ -79,7 +80,7 @@ class ContentLibrariesRestApiTest(APITestCase): ) def setUp(self): - super(ContentLibrariesRestApiTest, self).setUp() + super().setUp() self.clients_by_user = {} self.client.login(username=self.user.username, password="edx") @@ -143,6 +144,23 @@ class ContentLibrariesRestApiTest(APITestCase): """ Delete an existing library """ return self._api('delete', URL_LIB_DETAIL.format(lib_key=lib_key), None, expect_response) + def _get_library_links(self, lib_key): + """ Get the links of the specified content library """ + return self._api('get', URL_LIB_LINKS.format(lib_key=lib_key), None, expect_response=200) + + def _link_to_library(self, lib_key, link_id, other_library_key, version=None): + """ + Modify the library identified by lib_key to create a named link to + other_library_key. This allows you to use XBlocks from other_library in + lib. Optionally specify a version to link to. + """ + data = { + "id": link_id, + "opaque_key": other_library_key, + "version": version, + } + return self._api('post', URL_LIB_LINKS.format(lib_key=lib_key), data, expect_response=200) + def _commit_library_changes(self, lib_key, expect_response=200): """ Commit changes to an existing library """ return self._api('post', URL_LIB_COMMIT.format(lib_key=lib_key), None, expect_response) @@ -221,8 +239,8 @@ class ContentLibrariesRestApiTest(APITestCase): content should be a binary string. """ - assert isinstance(content, six.binary_type) - file_handle = six.BytesIO(content) + assert isinstance(content, bytes) + file_handle = BytesIO(content) url = URL_LIB_BLOCK_ASSET_FILE.format(block_key=block_key, file_name=file_name) response = self.client.put(url, data={"content": file_handle}) self.assertEqual( 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 59ad24819c..afa54fd129 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -355,3 +355,91 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest): self._delete_library_block(block3_key) self._commit_library_changes(lib_id) self._revert_library_changes(lib_id) # This is a no-op after the commit, but should still have 200 response + + def test_library_blocks_with_links(self): + """ + Test that libraries can link to XBlocks in other content libraries + """ + # Create a problem bank: + bank_lib = self._create_library(slug="problem_bank", 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", "problem1") + 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) + # Now update problem 1 and create a new problem 2: + self._set_library_block_olx(p1["id"], """ + +

What is an odd number?

+ + 3 + 2 + + + """) + p2 = self._add_block_to_library(bank_lib_id, "problem", "problem2") + self._set_library_block_olx(p2["id"], """ + +

What holds this XBlock?

+ + A course + A problem bank + + + """) + # Commit the changes, creating version 2: + self._commit_library_changes(bank_lib_id) + # At this point, bank_lib contains two problems and has two versions. + # In version 1, problem1 is "What is an event number", and in version 2 it's "What is an odd number". + # Problem2 exists only in version 2 and asks "What holds this XBlock?" + + lib = self._create_library(slug="links_test_lib", title="Link Test Library") + lib_id = lib["id"] + # Link to the problem bank: + self._link_to_library(lib_id, "problem_bank", bank_lib_id) + self._link_to_library(lib_id, "problem_bank_v1", bank_lib_id, version=1) + + # Add a 'unit' XBlock to the library: + unit_block = self._add_block_to_library(lib_id, "unit", "unit1") + self._set_library_block_olx(unit_block["id"], """ + + + + + + + + + """) + + # The unit can see and render its children: + fragment = self._render_block_view(unit_block["id"], "student_view") + self.assertIn("What is an odd number?", fragment["content"]) + self.assertIn("What is an even number?", fragment["content"]) + self.assertIn("What holds this XBlock?", fragment["content"]) + + # Also check the API for retrieving links: + links_created = self._get_library_links(lib_id) + links_created.sort(key=lambda link: link["id"]) + self.assertEqual(len(links_created), 2) + + self.assertEqual(links_created[0]["id"], "problem_bank") + self.assertEqual(links_created[0]["bundle_uuid"], bank_lib["bundle_uuid"]) + self.assertEqual(links_created[0]["version"], 2) + self.assertEqual(links_created[0]["latest_version"], 2) + self.assertEqual(links_created[0]["opaque_key"], bank_lib_id) + + self.assertEqual(links_created[1]["id"], "problem_bank_v1") + self.assertEqual(links_created[1]["bundle_uuid"], bank_lib["bundle_uuid"]) + self.assertEqual(links_created[1]["version"], 1) + self.assertEqual(links_created[1]["latest_version"], 2) + self.assertEqual(links_created[1]["opaque_key"], bank_lib_id) diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index 2c08f87086..74019affb1 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -20,6 +20,10 @@ urlpatterns = [ url(r'^$', views.LibraryDetailsView.as_view()), # Get the list of XBlock types that can be added to this library url(r'^block_types/$', views.LibraryBlockTypesView.as_view()), + # Get the list of Blockstore Bundle Links for this library, or add a new one: + url(r'^links/$', views.LibraryLinksView.as_view()), + # Update or delete a link: + url(r'^links/(?P[^/]+)/$', views.LibraryLinkDetailView.as_view()), # Get the list of XBlocks in this library, or add a new one: url(r'^blocks/$', views.LibraryBlocksView.as_view()), # Commit (POST) or revert (DELETE) all pending changes to this library: diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index 6decb85803..7a1a41e565 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -24,6 +24,8 @@ from openedx.core.djangoapps.content_libraries.serializers import ( LibraryXBlockCreationSerializer, LibraryXBlockMetadataSerializer, LibraryXBlockTypeSerializer, + LibraryBundleLinkSerializer, + LibraryBundleLinkUpdateSerializer, LibraryXBlockOlxSerializer, LibraryXBlockStaticFileSerializer, LibraryXBlockStaticFilesSerializer, @@ -241,6 +243,80 @@ class LibraryBlockTypesView(APIView): return Response(LibraryXBlockTypeSerializer(result, many=True).data) +@view_auth_classes() +class LibraryLinksView(APIView): + """ + View to get the list of bundles/libraries linked to this content library. + + Because every content library is a blockstore bundle, it can have "links" to + other bundles, which may or may not be content libraries. This allows using + XBlocks (or perhaps even static assets etc.) from another bundle without + needing to duplicate/copy the data. + + Links always point to a specific published version of the target bundle. + Links are identified by a slug-like ID, e.g. "link1" + """ + @convert_exceptions + def get(self, request, lib_key_str): + """ + Get the list of bundles that this library links to, if any + """ + key = LibraryLocatorV2.from_string(lib_key_str) + api.require_permission_for_library_key(key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY) + result = api.get_bundle_links(key) + return Response(LibraryBundleLinkSerializer(result, many=True).data) + + @convert_exceptions + def post(self, request, lib_key_str): + """ + Create a new link in this library. + """ + key = LibraryLocatorV2.from_string(lib_key_str) + api.require_permission_for_library_key(key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY) + serializer = LibraryBundleLinkSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + target_key = LibraryLocatorV2.from_string(serializer.validated_data['opaque_key']) + api.create_bundle_link( + library_key=key, + link_id=serializer.validated_data['id'], + target_opaque_key=target_key, + version=serializer.validated_data['version'], # a number, or None for "use latest version" + ) + return Response({}) + + +@view_auth_classes() +class LibraryLinkDetailView(APIView): + """ + View to update/delete an existing library link + """ + @convert_exceptions + def patch(self, request, lib_key_str, link_id): + """ + Update the specified link to point to a different version of its + target bundle. + + Pass e.g. {"version": 40} or pass {"version": None} to update to the + latest published version. + """ + key = LibraryLocatorV2.from_string(lib_key_str) + api.require_permission_for_library_key(key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY) + serializer = LibraryBundleLinkUpdateSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + api.update_bundle_link(key, link_id, version=serializer.validated_data['version']) + return Response({}) + + @convert_exceptions + def delete(self, request, lib_key_str, link_id): # pylint: disable=unused-argument + """ + Delete a link from this library. + """ + key = LibraryLocatorV2.from_string(lib_key_str) + api.require_permission_for_library_key(key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY) + api.update_bundle_link(key, link_id, delete=True) + return Response({}) + + @view_auth_classes() class LibraryCommitView(APIView): """ diff --git a/openedx/core/lib/blockstore_api/methods.py b/openedx/core/lib/blockstore_api/methods.py index 7a3fa49ecf..a9735aace2 100644 --- a/openedx/core/lib/blockstore_api/methods.py +++ b/openedx/core/lib/blockstore_api/methods.py @@ -264,7 +264,7 @@ 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 [] + return {} version_url = api_url('bundle_versions', str(bundle_uuid) + ',' + str(version_number)) version_info = api_request('get', version_url) return {