Server-side Library asset support (asset view and api get/add/delete)

cms/envs/test.py

@@ -333,3 +333,13 @@
     "SECRET": "***",
     "URL": "***",
 }
+
+############## openedx-learning (Learning Core) config ##############
+OPENEDX_LEARNING = {
+    'MEDIA': {
+        'BACKEND': 'django.core.files.storage.InMemoryStorage',
+        'OPTIONS': {
+            'location': MEDIA_ROOT + "_private"
+        }
+    }
+}

openedx/core/djangoapps/content_libraries/api.py

@@ -56,6 +56,7 @@
 import base64
 import hashlib
 import logging
+import mimetypes
 
 import attr
 import requests
@@ -68,6 +69,7 @@
 from django.db.models import Q, QuerySet
 from django.utils.translation import gettext as _
 from edx_rest_api_client.client import OAuthAPIClient
+from django.urls import reverse
 from lxml import etree
 from opaque_keys.edx.keys import BlockTypeKey, UsageKey, UsageKeyV2
 from opaque_keys.edx.locator import (
@@ -96,7 +98,11 @@
 from xblock.core import XBlock
 from xblock.exceptions import XBlockNotFoundError
 
-from openedx.core.djangoapps.xblock.api import get_component_from_usage_key, xblock_type_display_name
+from openedx.core.djangoapps.xblock.api import (
+    get_component_from_usage_key,
+    get_xblock_app_config,
+    xblock_type_display_name,
+)
 from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_for_learning_core
 from xmodule.modulestore.django import modulestore
 
@@ -1018,18 +1024,48 @@ def get_library_block_static_asset_files(usage_key) -> list[LibraryXBlockStaticF
 
     Returns a list of LibraryXBlockStaticFile objects, sorted by path.
 
-    TODO: This is not yet implemented for Learning Core backed libraries.
     TODO: Should this be in the general XBlock API rather than the libraries API?
     """
-    return []
+    component = get_component_from_usage_key(usage_key)
+    component_version = component.versioning.draft
+
+    # If there is no Draft version, then this was soft-deleted
+    if component_version is None:
+        return []
+
+    # cvc = the ComponentVersionContent through table
+    cvc_set = (
+        component_version
+        .componentversioncontent_set
+        .filter(content__has_file=True)
+        .order_by('key')
+        .select_related('content')
+    )
+
+    site_root_url = get_xblock_app_config().get_site_root_url()
+
+    return [
+        LibraryXBlockStaticFile(
+            path=cvc.key,
+            size=cvc.content.size,
+            url=site_root_url + reverse(
+                'content_libraries:library-assets',
+                kwargs={
+                    'component_version_uuid': component_version.uuid,
+                    'asset_path': cvc.key,
+                }
+            ),
+        )
+        for cvc in cvc_set
+    ]
 
 
-def add_library_block_static_asset_file(usage_key, file_name, file_content) -> LibraryXBlockStaticFile:
+def add_library_block_static_asset_file(usage_key, file_path, file_content, user=None) -> LibraryXBlockStaticFile:
     """
     Upload a static asset file into the library, to be associated with the
     specified XBlock. Will silently overwrite an existing file of the same name.
 
-    file_name should be a name like "doc.pdf". It may optionally contain slashes
+    file_path should be a name like "doc.pdf". It may optionally contain slashes
         like 'en/doc.pdf'
     file_content should be a binary string.
 
@@ -1041,10 +1077,67 @@ def add_library_block_static_asset_file(usage_key, file_name, file_content) -> L
         video_block = UsageKey.from_string("lb:VideoTeam:python-intro:video:1")
         add_library_block_static_asset_file(video_block, "subtitles-en.srt", subtitles.encode('utf-8'))
     """
-    raise NotImplementedError("Static assets not yet implemented for Learning Core")
+    # File path validations copied over from v1 library logic. This can't really
+    # hurt us inside our system because we never use these paths in an actual
+    # file system–they're just string keys that point to hash-named data files
+    # in a common library (learning package) level directory. But it might
+    # become a security issue during import/export serialization.
+    if file_path != file_path.strip().strip('/'):
+        raise InvalidNameError("file_path cannot start/end with / or whitespace.")
+    if '//' in file_path or '..' in file_path:
+        raise InvalidNameError("Invalid sequence (// or ..) in file_path.")
+
+    component = get_component_from_usage_key(usage_key)
+
+    media_type_str, _encoding = mimetypes.guess_type(file_path)
+    # We use "application/octet-stream" as a generic fallback media type, per
+    # RFC 2046: https://datatracker.ietf.org/doc/html/rfc2046
+    # TODO: This probably makes sense to push down to openedx-learning?
+    media_type_str = media_type_str or "application/octet-stream"
+
+    now = datetime.now(tz=timezone.utc)
+
+    with transaction.atomic():
+        media_type = authoring_api.get_or_create_media_type(media_type_str)
+        content = authoring_api.get_or_create_file_content(
+            component.publishable_entity.learning_package.id,
+            media_type.id,
+            data=file_content,
+            created=now,
+        )
+        component_version = authoring_api.create_next_component_version(
+            component.pk,
+            content_to_replace={file_path: content.id},
+            created=now,
+            created_by=user.id if user else None,
+        )
+        transaction.on_commit(
+            lambda: LIBRARY_BLOCK_UPDATED.send_event(
+                library_block=LibraryBlockData(
+                    library_key=usage_key.context_key,
+                    usage_key=usage_key,
+                )
+            )
+        )
 
+    # Now figure out the URL for the newly created asset...
+    site_root_url = get_xblock_app_config().get_site_root_url()
+    local_path = reverse(
+        'content_libraries:library-assets',
+        kwargs={
+            'component_version_uuid': component_version.uuid,
+            'asset_path': file_path,
+        }
+    )
+
+    return LibraryXBlockStaticFile(
+        path=file_path,
+        url=site_root_url + local_path,
+        size=content.size,
+    )
 
-def delete_library_block_static_asset_file(usage_key, file_name):
+
+def delete_library_block_static_asset_file(usage_key, file_path, user=None):
     """
     Delete a static asset file from the library.
 
@@ -1054,7 +1147,24 @@ def delete_library_block_static_asset_file(usage_key, file_name):
         video_block = UsageKey.from_string("lb:VideoTeam:python-intro:video:1")
         delete_library_block_static_asset_file(video_block, "subtitles-en.srt")
     """
-    raise NotImplementedError("Static assets not yet implemented for Learning Core")
+    component = get_component_from_usage_key(usage_key)
+    now = datetime.now(tz=timezone.utc)
+
+    with transaction.atomic():
+        component_version = authoring_api.create_next_component_version(
+            component.pk,
+            content_to_replace={file_path: None},
+            created=now,
+            created_by=user.id if user else None,
+        )
+        transaction.on_commit(
+            lambda: LIBRARY_BLOCK_UPDATED.send_event(
+                library_block=LibraryBlockData(
+                    library_key=usage_key.context_key,
+                    usage_key=usage_key,
+                )
+            )
+        )
 
 
 def get_allowed_block_types(library_key):  # pylint: disable=unused-argument

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

@@ -661,13 +661,13 @@ def test_library_permissions(self):  # pylint: disable=too-many-statements
             self._get_library_block_olx(block3_key, expect_response=403)
             self._get_library_block_fields(block3_key, expect_response=403)
             self._get_library_block_assets(block3_key, expect_response=403)
-            self._get_library_block_asset(block3_key, file_name="whatever.png", expect_response=403)
+            self._get_library_block_asset(block3_key, file_name="static/whatever.png", expect_response=403)
             # Nor can they preview the block:
             self._render_block_view(block3_key, view_name="student_view", expect_response=403)
         # But if we grant allow_public_read, then they can:
         with self.as_user(admin):
             self._update_library(lib_id, allow_public_read=True)
-            # self._set_library_block_asset(block3_key, "whatever.png", b"data")
+            self._set_library_block_asset(block3_key, "static/whatever.png", b"data")
         with self.as_user(random_user):
             self._get_library_block_olx(block3_key)
             self._render_block_view(block3_key, view_name="student_view")
@@ -680,7 +680,7 @@ def test_library_permissions(self):  # pylint: disable=too-many-statements
             with self.as_user(user):
                 self._set_library_block_olx(block3_key, "<problem/>", expect_response=403)
                 self._set_library_block_fields(block3_key, {"data": "<problem />", "metadata": {}}, expect_response=403)
-                # self._set_library_block_asset(block3_key, "test.txt", b"data", expect_response=403)
+                self._set_library_block_asset(block3_key, "static/test.txt", b"data", expect_response=403)
                 self._delete_library_block(block3_key, expect_response=403)
                 self._commit_library_changes(lib_id, expect_response=403)
                 self._revert_library_changes(lib_id, expect_response=403)
@@ -690,9 +690,9 @@ def test_library_permissions(self):  # pylint: disable=too-many-statements
             olx = self._get_library_block_olx(block3_key)
             self._set_library_block_olx(block3_key, olx)
             self._set_library_block_fields(block3_key, {"data": olx, "metadata": {}})
-            # self._get_library_block_assets(block3_key)
-            # self._set_library_block_asset(block3_key, "test.txt", b"data")
-            # self._get_library_block_asset(block3_key, file_name="test.txt")
+            self._get_library_block_assets(block3_key)
+            self._set_library_block_asset(block3_key, "static/test.txt", b"data")
+            self._get_library_block_asset(block3_key, file_name="static/test.txt")
             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
@@ -915,7 +915,6 @@ def test_library_block_olx_update_event(self):
             event_receiver.call_args.kwargs
         )
 
-    @skip("We still need to re-implement static asset handling.")
     def test_library_block_add_asset_update_event(self):
         """
         Check that LIBRARY_BLOCK_CREATED event is sent when a static asset is
@@ -934,7 +933,7 @@ def test_library_block_add_asset_update_event(self):
 
         block = self._add_block_to_library(lib_id, "unit", "u1")
         block_id = block["id"]
-        self._set_library_block_asset(block_id, "test.txt", b"data")
+        self._set_library_block_asset(block_id, "static/test.txt", b"data")
 
         usage_key = LibraryUsageLocatorV2(
             lib_key=library_key,
@@ -955,7 +954,6 @@ def test_library_block_add_asset_update_event(self):
             event_receiver.call_args.kwargs
         )
 
-    @skip("We still need to re-implement static asset handling.")
     def test_library_block_del_asset_update_event(self):
         """
         Check that LIBRARY_BLOCK_CREATED event is sent when a static asset is
@@ -974,9 +972,9 @@ def test_library_block_del_asset_update_event(self):
 
         block = self._add_block_to_library(lib_id, "unit", "u1")
         block_id = block["id"]
-        self._set_library_block_asset(block_id, "test.txt", b"data")
+        self._set_library_block_asset(block_id, "static/test.txt", b"data")
 
-        self._delete_library_block_asset(block_id, 'text.txt')
+        self._delete_library_block_asset(block_id, 'static/text.txt')
 
         usage_key = LibraryUsageLocatorV2(
             lib_key=library_key,

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

@@ -1,11 +1,16 @@
 """
 Tests for static asset files in Learning-Core-based Content Libraries
 """
-from unittest import skip
+from uuid import UUID
 
+from opaque_keys.edx.keys import UsageKey
+
+from common.djangoapps.student.tests.factories import UserFactory
 from openedx.core.djangoapps.content_libraries.tests.base import (
     ContentLibrariesRestApiTest,
 )
+from openedx.core.djangoapps.xblock.api import get_component_from_usage_key
+from openedx.core.djangolib.testing.utils import skip_unless_cms
 
 # Binary data representing an SVG image file
 SVG_DATA = """<svg xmlns="http://www.w3.org/2000/svg" height="30" width="100">
@@ -23,15 +28,10 @@
 """
 
 
-@skip("Assets are being reimplemented in Learning Core. Disable until that's ready.")
+@skip_unless_cms
 class ContentLibrariesStaticAssetsTest(ContentLibrariesRestApiTest):
     """
     Tests for static asset files in Learning-Core-based Content Libraries
-
-    WARNING: every test should have a unique library slug, because even though
-    the django/mysql database gets reset for each test case, the lookup between
-    library slug and bundle UUID does not because it's assumed to be immutable
-    and cached forever.
     """
 
     def test_asset_filenames(self):
@@ -79,7 +79,7 @@ def test_video_transcripts(self):
             />
         """)
         # Upload the transcript file
-        self._set_library_block_asset(block_id, "3_yD_cEKoCk-en.srt", TRANSCRIPT_DATA)
+        self._set_library_block_asset(block_id, "static/3_yD_cEKoCk-en.srt", TRANSCRIPT_DATA)
 
         transcript_handler_url = self._get_block_handler_url(block_id, "transcript")
 
@@ -108,3 +108,79 @@ def check_download():
         self._commit_library_changes(library["id"])
         check_sjson()
         check_download()
+
+
+@skip_unless_cms
+class ContentLibrariesComponentVersionAssetTest(ContentLibrariesRestApiTest):
+    """
+    Tests for the view that actually delivers the Library asset in Studio.
+    """
+
+    def setUp(self):
+        super().setUp()
+
+        library = self._create_library(slug="asset-lib2", title="Static Assets Test Library")
+        block = self._add_block_to_library(library["id"], "html", "html1")
+        self._set_library_block_asset(block["id"], "static/test.svg", SVG_DATA)
+        usage_key = UsageKey.from_string(block["id"])
+        self.component = get_component_from_usage_key(usage_key)
+        self.draft_component_version = self.component.versioning.draft
+
+    def test_good_responses(self):
+        get_response = self.client.get(
+            f"/library_assets/{self.draft_component_version.uuid}/static/test.svg"
+        )
+        assert get_response.status_code == 200
+        content = b''.join(chunk for chunk in get_response.streaming_content)
+        assert content == SVG_DATA
+
+        good_head_response = self.client.head(
+            f"/library_assets/{self.draft_component_version.uuid}/static/test.svg"
+        )
+        assert good_head_response.headers == get_response.headers
+
+    def test_missing(self):
+        """Test asset requests that should 404."""
+        # Non-existent version...
+        wrong_version_uuid = UUID('11111111-1111-1111-1111-111111111111')
+        response = self.client.get(
+            f"/library_assets/{wrong_version_uuid}/static/test.svg"
+        )
+        assert response.status_code == 404
+
+        # Non-existent file...
+        response = self.client.get(
+            f"/library_assets/{self.draft_component_version.uuid}/static/missing.svg"
+        )
+        assert response.status_code == 404
+
+        # File-like ComponenVersionContent entry that isn't an actually
+        # downloadable file...
+        response = self.client.get(
+            f"/library_assets/{self.draft_component_version.uuid}/block.xml"
+        )
+        assert response.status_code == 404
+
+    def test_anonymous_user(self):
+        """Anonymous users shouldn't get access to library assets."""
+        self.client.logout()
+        response = self.client.get(
+            f"/library_assets/{self.draft_component_version.uuid}/static/test.svg"
+        )
+        assert response.status_code == 403
+
+    def test_unauthorized_user(self):
+        """User who is not a Content Library staff should not have access."""
+        self.client.logout()
+        student = UserFactory.create(
+            username="student",
+            email="[email protected]",
+            password="student-pass",
+            is_staff=False,
+            is_superuser=False,
+        )
+        self.client.login(username="student", password="student-pass")
+        get_response = self.client.get(
+            f"/library_assets/{self.draft_component_version.uuid}/static/test.svg"
+        )
+        assert get_response.status_code == 403