refactor: split libraries API & REST API up into smaller modules

Author: bradenmacdonald

openedx/core/djangoapps/content_libraries/api/__init__.py

@@ -0,0 +1,5 @@
+"""
+Python API for working with content libraries
+"""
+from .libraries import *
+from .blocks import *

openedx/core/djangoapps/content_libraries/api/blocks.py

@@ -0,0 +1,30 @@
+"""
+Content libraries API methods related to XBlocks/Components.
+
+These methods don't enforce permissions (only the REST APIs do).
+"""
+# pylint: disable=unused-import
+
+# TODO: move all the API methods related to blocks and assets in here from 'libraries.py'
+# TODO: use __all__ to limit what symbols are public.
+
+from .libraries import (
+    LibraryXBlockMetadata,
+    LibraryXBlockStaticFile,
+    LibraryXBlockType,
+    get_library_components,
+    get_library_block,
+    set_library_block_olx,
+    library_component_usage_key,
+    get_component_from_usage_key,
+    validate_can_add_block_to_library,
+    create_library_block,
+    import_staged_content_from_user_clipboard,
+    get_or_create_olx_media_type,
+    delete_library_block,
+    restore_library_block,
+    get_library_block_static_asset_files,
+    add_library_block_static_asset_file,
+    delete_library_block_static_asset_file,
+    publish_component_changes,
+)

…core/djangoapps/content_libraries/api.py …oapps/content_libraries/api/libraries.pyopenedx/core/djangoapps/content_libraries/api.py renamed to openedx/core/djangoapps/content_libraries/api/libraries.py openedx/core/djangoapps/content_libraries/api.py renamed to openedx/core/djangoapps/content_libraries/api/libraries.py

@@ -62,7 +62,7 @@
 import requests
 
 from django.conf import settings
-from django.contrib.auth.models import AbstractUser, Group
+from django.contrib.auth.models import AbstractUser, AnonymousUser, Group
 from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
 from django.core.validators import validate_unicode_slug
 from django.db import IntegrityError, transaction
@@ -116,12 +116,60 @@
 from openedx.core.types import User as UserType
 from xmodule.modulestore.django import modulestore
 
-from . import permissions, tasks
-from .constants import ALL_RIGHTS_RESERVED
-from .models import ContentLibrary, ContentLibraryPermission, ContentLibraryBlockImportTask
+from .. import permissions, tasks
+from ..constants import ALL_RIGHTS_RESERVED
+from ..models import ContentLibrary, ContentLibraryPermission, ContentLibraryBlockImportTask
 
 log = logging.getLogger(__name__)
 
+# The public API is only the following symbols:
+__all__ = [
+    # Exceptions - maybe move them to a new file?
+    "ContentLibraryNotFound",
+    "ContentLibraryCollectionNotFound",
+    "ContentLibraryBlockNotFound",
+    "LibraryAlreadyExists",
+    "LibraryCollectionAlreadyExists",
+    "LibraryBlockAlreadyExists",
+    "BlockLimitReachedError",
+    "IncompatibleTypesError",
+    "InvalidNameError",
+    "LibraryPermissionIntegrityError",
+    # Library Models
+    "ContentLibrary",  # Should this be public or not?
+    "ContentLibraryMetadata",
+    "AccessLevel",
+    "ContentLibraryPermissionEntry",
+    "CollectionMetadata",
+    # Library API methods
+    "user_can_create_library",
+    "get_libraries_for_user",
+    "get_metadata",
+    "require_permission_for_library_key",
+    "get_library",
+    "create_library",
+    "get_library_team",
+    "get_library_user_permissions",
+    "set_library_user_permissions",
+    "set_library_group_permissions",
+    "update_library",
+    "delete_library",
+    "get_allowed_block_types",
+    "publish_changes",
+    "revert_changes",
+    # Collections - TODO: move to a new file
+    "create_library_collection",
+    "update_library_collection",
+    "update_library_collection_components",
+    "set_library_component_collections",
+    "get_library_collection_usage_key",
+    "get_library_collection_from_usage_key",
+    # Import - TODO: move to a new file
+    "EdxModulestoreImportClient",
+    "EdxApiImportClient",
+    "import_blocks_create_task",
+]
+
 
 # Exceptions
 # ==========
@@ -416,6 +464,7 @@ def get_library(library_key: LibraryLocatorV2) -> ContentLibraryMetadata:
     """
     ref = ContentLibrary.objects.get_by_key(library_key)
     learning_package = ref.learning_package
+    assert learning_package is not None  # Shouldn't happen - this is just for the type checker
     num_blocks = authoring_api.get_all_drafts(learning_package.id).count()
     last_publish_log = authoring_api.get_last_publish(learning_package.id)
     last_draft_log = authoring_api.get_entities_with_unpublished_changes(learning_package.id) \
@@ -455,7 +504,7 @@ def get_library(library_key: LibraryLocatorV2) -> ContentLibraryMetadata:
     return ContentLibraryMetadata(
         key=library_key,
         title=learning_package.title,
-        description=ref.learning_package.description,
+        description=learning_package.description,
         num_blocks=num_blocks,
         version=version,
         last_published=None if last_publish_log is None else last_publish_log.published_at,
@@ -557,6 +606,8 @@ def get_library_user_permissions(library_key: LibraryLocatorV2, user: UserType)
     Fetch the specified user's access information. Will return None if no
     permissions have been granted.
     """
+    if isinstance(user, AnonymousUser):
+        return None  # Mostly here for the type checker
     ref = ContentLibrary.objects.get_by_key(library_key)
     grant = ref.permission_grants.filter(user=user).first()
     if grant is None:
@@ -574,6 +625,8 @@ def set_library_user_permissions(library_key: LibraryLocatorV2, user: UserType,
 
     access_level should be one of the AccessLevel values defined above.
     """
+    if isinstance(user, AnonymousUser):
+        raise TypeError("Invalid user type")  # Mostly here for the type checker
     ref = ContentLibrary.objects.get_by_key(library_key)
     current_grant = get_library_user_permissions(library_key, user)
     if current_grant and current_grant.access_level == AccessLevel.ADMIN_LEVEL:
@@ -633,6 +686,8 @@ def update_library(
         return
 
     content_lib = ContentLibrary.objects.get_by_key(library_key)
+    learning_package_id = content_lib.learning_package_id
+    assert learning_package_id is not None
 
     with transaction.atomic():
         # We need to make updates to both the ContentLibrary and its linked
@@ -643,12 +698,12 @@ def update_library(
             if allow_public_read is not None:
                 content_lib.allow_public_read = allow_public_read
             if library_license is not None:
-                content_lib.library_license = library_license
+                content_lib.license = library_license
             content_lib.save()
 
         if learning_pkg_changed:
             authoring_api.update_learning_package(
-                content_lib.learning_package_id,
+                learning_package_id,
                 title=title,
                 description=description,
             )
@@ -675,7 +730,8 @@ def delete_library(library_key: LibraryLocatorV2) -> None:
         # TODO: We should eventually detach the LearningPackage and delete it
         #       asynchronously, especially if we need to delete a bunch of stuff
         #       on the filesystem for it.
-        learning_package.delete()
+        if learning_package:
+            learning_package.delete()
 
     CONTENT_LIBRARY_DELETED.send_event(
         content_library=ContentLibraryData(
@@ -709,6 +765,7 @@ def get_library_components(
     """
     lib = ContentLibrary.objects.get_by_key(library_key)  # type: ignore[attr-defined]
     learning_package = lib.learning_package
+    assert learning_package is not None
     components = authoring_api.get_components(
         learning_package.id,
         draft=True,
@@ -860,7 +917,8 @@ def validate_can_add_block_to_library(
     content_library = ContentLibrary.objects.get_by_key(library_key)  # type: ignore[attr-defined]
 
     # If adding a component would take us over our max, return an error.
-    component_count = authoring_api.get_all_drafts(content_library.learning_package.id).count()
+    assert content_library.learning_package_id is not None
+    component_count = authoring_api.get_all_drafts(content_library.learning_package_id).count()
     if component_count + 1 > settings.MAX_BLOCKS_PER_CONTENT_LIBRARY:
         raise BlockLimitReachedError(
             _("Library cannot have more than {} Components").format(
@@ -1356,7 +1414,7 @@ def publish_changes(library_key: LibraryLocatorV2, user_id: int | None = None):
     Publish all pending changes to the specified library.
     """
     learning_package = ContentLibrary.objects.get_by_key(library_key).learning_package
-
+    assert learning_package is not None  # shouldn't happen but it's technically possible.
     authoring_api.publish_all_drafts(learning_package.id, published_by=user_id)
 
     CONTENT_LIBRARY_UPDATED.send_event(
@@ -1398,6 +1456,7 @@ def revert_changes(library_key: LibraryLocatorV2) -> None:
     last published version.
     """
     learning_package = ContentLibrary.objects.get_by_key(library_key).learning_package
+    assert learning_package is not None  # shouldn't happen but it's technically possible.
     authoring_api.reset_drafts_to_published(learning_package.id)
 
     CONTENT_LIBRARY_UPDATED.send_event(
@@ -1652,6 +1711,7 @@ def get_library_collection_from_usage_key(
     library_key = collection_usage_key.library_key
     collection_key = collection_usage_key.collection_id
     content_library = ContentLibrary.objects.get_by_key(library_key)  # type: ignore[attr-defined]
+    assert content_library.learning_package_id is not None  # shouldn't happen but it's technically possible.
     try:
         return authoring_api.get_collection(
             content_library.learning_package_id,

openedx/core/djangoapps/content_libraries/models.py

@@ -36,6 +36,7 @@
 
 import contextlib
 import logging
+from typing import ClassVar
 import uuid
 
 from django.contrib.auth import get_user_model
@@ -67,11 +68,11 @@
 User = get_user_model()
 
 
-class ContentLibraryManager(models.Manager):
+class ContentLibraryManager(models.Manager["ContentLibrary"]):
     """
     Custom manager for ContentLibrary class.
     """
-    def get_by_key(self, library_key):
+    def get_by_key(self, library_key) -> "ContentLibrary":
         """
         Get the ContentLibrary for the given LibraryLocatorV2 key.
         """
@@ -92,7 +93,7 @@ class ContentLibrary(models.Model):
 
     .. no_pii:
     """
-    objects: ContentLibraryManager[ContentLibrary] = ContentLibraryManager()
+    objects: ClassVar[ContentLibraryManager] = ContentLibraryManager()
 
     id = models.AutoField(primary_key=True)
     # Every Library is uniquely and permanently identified by an 'org' and a

openedx/core/djangoapps/content_libraries/rest_api/__init__.py

@@ -0,0 +1,19 @@
+"""
+Content Library REST APIs related to XBlocks/Components and their static assets
+"""
+# pylint: disable=unused-import
+
+# TODO: move the block and block asset related views from 'libraries' into this file
+from .libraries import (
+    LibraryBlockAssetListView,
+    LibraryBlockAssetView,
+    LibraryBlockCollectionsView,
+    LibraryBlockLtiUrlView,
+    LibraryBlockOlxView,
+    LibraryBlockPublishView,
+    LibraryBlockRestore,
+    LibraryBlocksView,
+    LibraryBlockView,
+    LibraryComponentAssetView,
+    LibraryComponentDraftAssetView,
+)

openedx/core/djangoapps/content_libraries/rest_api/blocks.py

@@ -1,7 +1,6 @@
 """
 Collections API Views
 """
-
 from __future__ import annotations
 
 from django.db.models import QuerySet
@@ -17,10 +16,10 @@
 from openedx_learning.api import authoring as authoring_api
 from openedx_learning.api.authoring_models import Collection
 
-from openedx.core.djangoapps.content_libraries import api, permissions
-from openedx.core.djangoapps.content_libraries.models import ContentLibrary
-from openedx.core.djangoapps.content_libraries.views import convert_exceptions
-from openedx.core.djangoapps.content_libraries.serializers import (
+from .. import api, permissions
+from ..models import ContentLibrary
+from .utils import convert_exceptions
+from .serializers import (
     ContentLibraryCollectionSerializer,
     ContentLibraryCollectionComponentsUpdateSerializer,
     ContentLibraryCollectionUpdateSerializer,

…s/content_libraries/views_collections.py …ontent_libraries/rest_api/collections.pyopenedx/core/djangoapps/content_libraries/views_collections.py renamed to openedx/core/djangoapps/content_libraries/rest_api/collections.py openedx/core/djangoapps/content_libraries/views_collections.py renamed to openedx/core/djangoapps/content_libraries/rest_api/collections.py

@@ -62,8 +62,6 @@
     to Learning Core) atomic:
         https://github.com/openedx/edx-platform/pull/30456
 """
-
-from functools import wraps
 import itertools
 import json
 import logging
@@ -86,7 +84,6 @@
 from pylti1p3.exception import LtiException, OIDCException
 
 import edx_api_doc_tools as apidocs
-from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
 from openedx_learning.api import authoring
 from organizations.api import ensure_organization
@@ -105,7 +102,7 @@
     user_can_create_organizations,
 )
 from openedx.core.djangoapps.content_libraries import api, permissions
-from openedx.core.djangoapps.content_libraries.serializers import (
+from openedx.core.djangoapps.content_libraries.rest_api.serializers import (
     ContentLibraryBlockImportTaskCreateSerializer,
     ContentLibraryBlockImportTaskSerializer,
     ContentLibraryFilterSerializer,
@@ -129,50 +126,14 @@
 from openedx.core.djangoapps.xblock import api as xblock_api
 from openedx.core.types.http import RestRequest
 
-from .models import ContentLibrary, LtiGradedResource, LtiProfile
+from .utils import convert_exceptions
+from ..models import ContentLibrary, LtiGradedResource, LtiProfile
 
 
 User = get_user_model()
 log = logging.getLogger(__name__)
 
 
-def convert_exceptions(fn):
-    """
-    Catch any Content Library API exceptions that occur and convert them to
-    DRF exceptions so DRF will return an appropriate HTTP response
-    """
-
-    @wraps(fn)
-    def wrapped_fn(*args, **kwargs):
-        try:
-            return fn(*args, **kwargs)
-        except InvalidKeyError as exc:
-            log.exception(str(exc))
-            raise NotFound  # lint-amnesty, pylint: disable=raise-missing-from
-        except api.ContentLibraryNotFound:
-            log.exception("Content library not found")
-            raise NotFound  # lint-amnesty, pylint: disable=raise-missing-from
-        except api.ContentLibraryBlockNotFound:
-            log.exception("XBlock not found in content library")
-            raise NotFound  # lint-amnesty, pylint: disable=raise-missing-from
-        except api.ContentLibraryCollectionNotFound:
-            log.exception("Collection not found in content library")
-            raise NotFound  # lint-amnesty, pylint: disable=raise-missing-from
-        except api.LibraryCollectionAlreadyExists as exc:
-            log.exception(str(exc))
-            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from
-        except api.LibraryBlockAlreadyExists as exc:
-            log.exception(str(exc))
-            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from
-        except api.InvalidNameError as exc:
-            log.exception(str(exc))
-            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from
-        except api.BlockLimitReachedError as exc:
-            log.exception(str(exc))
-            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from
-    return wrapped_fn
-
-
 class LibraryApiPaginationDocs:
     """
     API docs for query params related to paginating ContentLibraryMetadata objects.

…re/djangoapps/content_libraries/views.py …/content_libraries/rest_api/libraries.pyopenedx/core/djangoapps/content_libraries/views.py renamed to openedx/core/djangoapps/content_libraries/rest_api/libraries.py openedx/core/djangoapps/content_libraries/views.py renamed to openedx/core/djangoapps/content_libraries/rest_api/libraries.py

@@ -19,7 +19,7 @@
     ContentLibrary
 )
 from openedx.core.lib.api.serializers import CourseKeyField
-from . import permissions
+from .. import permissions
 
 
 DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%SZ'