Merge pull request #22035 from open-craft/blockstore-assets

APIs for XBlock static asset files in Blockstore

openedx/core/djangoapps/content_libraries/api.py

 """
-Python API for content libraries
+Python API for content libraries.
+
+Unless otherwise specified, all APIs in this file deal with the DRAFT version
+of the content library.
 """
 from __future__ import absolute_import, division, print_function, unicode_literals
 from uuid import UUID
@@ -31,6 +34,7 @@ from openedx.core.lib.blockstore_api import (
     commit_draft,
     delete_draft,
 )
+from openedx.core.djangolib import blockstore_cache
 from openedx.core.djangolib.blockstore_cache import BundleCache
 from .models import ContentLibrary, ContentLibraryPermission
 
@@ -56,6 +60,10 @@ class LibraryBlockAlreadyExists(KeyError):
     """ An XBlock with that ID already exists in the library """
 
 
+class InvalidNameError(ValueError):
+    """ The specified name/identifier is not valid """
+
+
 # Models:
 
 @attr.s
@@ -85,6 +93,21 @@ class LibraryXBlockMetadata(object):
     has_unpublished_changes = attr.ib(False)
 
 
+@attr.s
+class LibraryXBlockStaticFile(object):
+    """
+    Class that represents a static file in a content library, associated with
+    a particular XBlock.
+    """
+    # File path e.g. "diagram.png"
+    # In some rare cases it might contain a folder part, e.g. "en/track1.srt"
+    path = attr.ib("")
+    # Publicly accessible URL where the file can be downloaded
+    url = attr.ib("")
+    # Size in bytes
+    size = attr.ib(0)
+
+
 @attr.s
 class LibraryXBlockType(object):
     """
@@ -260,13 +283,11 @@ def get_library_blocks(library_key):
     return blocks
 
 
-def get_library_block(usage_key):
+def _lookup_usage_key(usage_key):
     """
-    Get metadata (LibraryXBlockMetadata) about one specific XBlock in a library
-
-    To load the actual XBlock instance, use
-        openedx.core.djangoapps.xblock.api.load_block()
-    instead.
+    Given a LibraryUsageLocatorV2 (usage key for an XBlock in a content library)
+    return the definition key and LibraryBundle
+    or raise ContentLibraryBlockNotFound
     """
     assert isinstance(usage_key, LibraryUsageLocatorV2)
     lib_context = get_learning_context_impl(usage_key)
@@ -274,6 +295,18 @@ def get_library_block(usage_key):
     if def_key is None:
         raise ContentLibraryBlockNotFound(usage_key)
     lib_bundle = LibraryBundle(usage_key.lib_key, def_key.bundle_uuid, draft_name=DRAFT_NAME)
+    return def_key, lib_bundle
+
+
+def get_library_block(usage_key):
+    """
+    Get metadata (LibraryXBlockMetadata) about one specific XBlock in a library
+
+    To load the actual XBlock instance, use
+        openedx.core.djangoapps.xblock.api.load_block()
+    instead.
+    """
+    def_key, lib_bundle = _lookup_usage_key(usage_key)
     return LibraryXBlockMetadata(
         usage_key=usage_key,
         def_key=def_key,
@@ -371,13 +404,7 @@ def delete_library_block(usage_key, remove_from_parent=True):
         delete block. This should always be true except when this function
         calls itself recursively.
     """
-    assert isinstance(usage_key, LibraryUsageLocatorV2)
-    library_context = get_learning_context_impl(usage_key)
-    library_ref = ContentLibrary.objects.get_by_key(usage_key.context_key)
-    def_key = library_context.definition_for_usage(usage_key)
-    if def_key is None:
-        raise ContentLibraryBlockNotFound(usage_key)
-    lib_bundle = LibraryBundle(usage_key.context_key, library_ref.bundle_uuid, draft_name=DRAFT_NAME)
+    def_key, lib_bundle = _lookup_usage_key(usage_key)
     # Create a draft:
     draft_uuid = get_or_create_bundle_draft(def_key.bundle_uuid, DRAFT_NAME).uuid
     # Does this block have a parent?
@@ -395,7 +422,7 @@ def delete_library_block(usage_key, remove_from_parent=True):
             # we're going to delete this block anyways.
             delete_library_block(child_usage, remove_from_parent=False)
     # Delete the definition:
-    if def_key.bundle_uuid == library_ref.bundle_uuid:
+    if def_key.bundle_uuid == lib_bundle.bundle_uuid:
         # This definition is in the library, so delete it:
         path_prefix = lib_bundle.olx_prefix(def_key)
         for bundle_file in get_bundle_files(def_key.bundle_uuid, use_draft=DRAFT_NAME):
@@ -434,6 +461,74 @@ def create_library_block_child(parent_usage_key, block_type, definition_id):
     return metadata
 
 
+def get_library_block_static_asset_files(usage_key):
+    """
+    Given an XBlock in a content library, list all the static asset files
+    associated with that XBlock.
+
+    Returns a list of LibraryXBlockStaticFile objects.
+    """
+    def_key, lib_bundle = _lookup_usage_key(usage_key)
+    result = [
+        LibraryXBlockStaticFile(path=f.path, url=f.url, size=f.size)
+        for f in lib_bundle.get_static_files_for_definition(def_key)
+    ]
+    result.sort(key=lambda f: f.path)
+    return result
+
+
+def add_library_block_static_asset_file(usage_key, file_name, file_content):
+    """
+    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
+        like 'en/doc.pdf'
+    file_content should be a binary string.
+
+    Returns a LibraryXBlockStaticFile object.
+
+    Example:
+        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'))
+    """
+    assert isinstance(file_content, six.binary_type)
+    def_key, lib_bundle = _lookup_usage_key(usage_key)
+    if file_name != file_name.strip().strip('/'):
+        raise InvalidNameError("file name cannot start/end with / or whitespace.")
+    if '//' in file_name or '..' in file_name:
+        raise InvalidNameError("Invalid sequence (// or ..) in filename.")
+    file_path = lib_bundle.get_static_prefix_for_definition(def_key) + file_name
+    # Write the new static file into the library bundle's draft
+    draft = get_or_create_bundle_draft(def_key.bundle_uuid, DRAFT_NAME)
+    write_draft_file(draft.uuid, file_path, file_content)
+    # Clear the bundle cache so everyone sees the new file immediately:
+    lib_bundle.cache.clear()
+    file_metadata = blockstore_cache.get_bundle_file_metadata_with_cache(
+        bundle_uuid=def_key.bundle_uuid, path=file_path, draft_name=DRAFT_NAME,
+    )
+    return LibraryXBlockStaticFile(path=file_metadata.path, url=file_metadata.url, size=file_metadata.size)
+
+
+def delete_library_block_static_asset_file(usage_key, file_name):
+    """
+    Delete a static asset file from the library.
+
+    Example:
+        video_block = UsageKey.from_string("lb:VideoTeam:python-intro:video:1")
+        delete_library_block_static_asset_file(video_block, "subtitles-en.srt")
+    """
+    def_key, lib_bundle = _lookup_usage_key(usage_key)
+    if '..' in file_name:
+        raise InvalidNameError("Invalid .. in file name.")
+    file_path = lib_bundle.get_static_prefix_for_definition(def_key) + file_name
+    # Delete the file from the library bundle's draft
+    draft = get_or_create_bundle_draft(def_key.bundle_uuid, DRAFT_NAME)
+    write_draft_file(draft.uuid, file_path, contents=None)
+    # Clear the bundle cache so everyone sees the new file immediately:
+    lib_bundle.cache.clear()
+
+
 def get_allowed_block_types(library_key):  # pylint: disable=unused-argument
     """
     Get a list of XBlock types that can be added to the specified content

openedx/core/djangoapps/content_libraries/library_bundle.py

@@ -314,6 +314,34 @@ class LibraryBundle(object):
         self.cache.set(('has_changes', ), result)
         return result
 
+    def get_static_prefix_for_definition(self, definition_key):
+        """
+        Given a definition key, get the path prefix used for all (public) static
+        asset files.
+
+        Example: problem/quiz1/static/
+        """
+        return self.olx_prefix(definition_key) + 'static/'
+
+    def get_static_files_for_definition(self, definition_key):
+        """
+        Return a list of the static asset files related with a particular XBlock
+        definition.
+
+        If the bundle contains files like:
+            problem/quiz1/definition.xml
+            problem/quiz1/static/image1.png
+        Then this will return
+            [BundleFile(path="image1.png", size, url, hash_digest)]
+        """
+        path_prefix = self.get_static_prefix_for_definition(definition_key)
+        path_prefix_len = len(path_prefix)
+        return [
+            blockstore_api.BundleFile(path=f.path[path_prefix_len:], size=f.size, url=f.url, hash_digest=f.hash_digest)
+            for f in get_bundle_files_cached(self.bundle_uuid, draft_name=self.draft_name)
+            if f.path.startswith(path_prefix)
+        ]
+
     @staticmethod
     def olx_prefix(definition_key):
         """

openedx/core/djangoapps/content_libraries/serializers.py

@@ -7,6 +7,8 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 from django.core.validators import validate_unicode_slug
 from rest_framework import serializers
 
+from openedx.core.lib import blockstore_api
+
 
 class ContentLibraryMetadataSerializer(serializers.Serializer):
     """
@@ -74,3 +76,35 @@ class LibraryXBlockOlxSerializer(serializers.Serializer):
     Serializer for representing an XBlock's OLX
     """
     olx = serializers.CharField()
+
+
+class LibraryXBlockStaticFileSerializer(serializers.Serializer):
+    """
+    Serializer representing a static file associated with an XBlock
+
+    Serializes a LibraryXBlockStaticFile (or a BundleFile)
+    """
+    path = serializers.CharField()
+    # Publicly accessible URL where the file can be downloaded.
+    # Must be an absolute URL.
+    url = serializers.URLField()
+    size = serializers.IntegerField(min_value=0)
+
+    def to_representation(self, instance):
+        """
+        Generate the serialized representation of this static asset file.
+        """
+        result = super(LibraryXBlockStaticFileSerializer, self).to_representation(instance)
+        # Make sure the URL is one that will work from the user's browser,
+        # not one that only works from within a docker container:
+        result['url'] = blockstore_api.force_browser_url(result['url'])
+        return result
+
+
+class LibraryXBlockStaticFilesSerializer(serializers.Serializer):
+    """
+    Serializer representing a static file associated with an XBlock
+
+    Serializes a LibraryXBlockStaticFile (or a BundleFile)
+    """
+    files = LibraryXBlockStaticFileSerializer(many=True)

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

+# -*- coding: utf-8 -*-
+"""
+Tests for Blockstore-based Content Libraries
+"""
+from __future__ import absolute_import, division, print_function, unicode_literals
+import unittest
+
+from django.conf import settings
+from organizations.models import Organization
+from rest_framework.test import APITestCase
+import six
+
+from student.tests.factories import UserFactory
+from openedx.core.djangolib.testing.utils import skip_unless_cms
+from openedx.core.lib import blockstore_api
+
+# Define the URLs here - don't use reverse() because we want to detect
+# backwards-incompatible changes like changed URLs.
+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_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_BLOCK = URL_PREFIX + 'blocks/{block_key}/'  # Get data about a block, or delete it
+URL_LIB_BLOCK_OLX = URL_LIB_BLOCK + 'olx/'  # Get or set the OLX of the specified XBlock
+URL_LIB_BLOCK_ASSETS = URL_LIB_BLOCK + 'assets/'  # List the static asset files of the specified XBlock
+URL_LIB_BLOCK_ASSET_FILE = URL_LIB_BLOCK + 'assets/{file_name}'  # Get, delete, or upload a specific static asset file
+
+URL_BLOCK_RENDER_VIEW = '/api/xblock/v2/xblocks/{block_key}/view/{view_name}/'
+URL_BLOCK_GET_HANDLER_URL = '/api/xblock/v2/xblocks/{block_key}/handler_url/{handler_name}/'
+
+
+# Decorator for tests that require blockstore
+requires_blockstore = unittest.skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server")
+
+
+@requires_blockstore
+@skip_unless_cms  # Content Libraries REST API is only available in Studio
+class ContentLibrariesRestApiTest(APITestCase):
+    """
+    Base class for Blockstore-based Content Libraries test that use the REST API
+
+    These tests use the REST API, which in turn relies on the Python API.
+    Some tests may use the python API directly if necessary to provide
+    coverage of any code paths not accessible via the REST API.
+
+    In general, these tests should
+    (1) Use public APIs only - don't directly create data using other methods,
+        which results in a less realistic test and ties the test suite too
+        closely to specific implementation details.
+        (Exception: users can be provisioned using a user factory)
+    (2) Assert that fields are present in responses, but don't assert that the
+        entire response has some specific shape. That way, things like adding
+        new fields to an API response, which are backwards compatible, won't
+        break any tests, but backwards-incompatible API changes will.
+
+    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.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        super(ContentLibrariesRestApiTest, cls).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:
+        cls.collection = blockstore_api.create_collection("Content Library Test Collection")
+        # Create an organization
+        cls.organization = Organization.objects.create(
+            name="Content Libraries Tachyon Exploration & Survey Team",
+            short_name="CL-TEST",
+        )
+
+    def setUp(self):
+        super(ContentLibrariesRestApiTest, self).setUp()
+        self.client.login(username=self.user.username, password="edx")
+
+    # API helpers
+
+    def _api(self, method, url, data, expect_response):
+        """
+        Call a REST API
+        """
+        response = getattr(self.client, method)(url, data, format="json")
+        self.assertEqual(
+            response.status_code, expect_response,
+            "Unexpected response code {}:\n{}".format(response.status_code, getattr(response, 'data', '(no data)')),
+        )
+        return response.data
+
+    def _create_library(self, slug, title, description="", expect_response=200):
+        """ Create a library """
+        return self._api('post', URL_LIB_CREATE, {
+            "org": self.organization.short_name,
+            "slug": slug,
+            "title": title,
+            "description": description,
+            "collection_uuid": str(self.collection.uuid),
+        }, expect_response)
+
+    def _get_library(self, lib_key, expect_response=200):
+        """ Get a library """
+        return self._api('get', URL_LIB_DETAIL.format(lib_key=lib_key), None, expect_response)
+
+    def _update_library(self, lib_key, **data):
+        """ Update an existing library """
+        return self._api('patch', URL_LIB_DETAIL.format(lib_key=lib_key), data=data, expect_response=200)
+
+    def _delete_library(self, lib_key, expect_response=200):
+        """ Delete an existing library """
+        return self._api('delete', URL_LIB_DETAIL.format(lib_key=lib_key), None, expect_response)
+
+    def _commit_library_changes(self, lib_key):
+        """ Commit changes to an existing library """
+        return self._api('post', URL_LIB_COMMIT.format(lib_key=lib_key), None, expect_response=200)
+
+    def _revert_library_changes(self, lib_key):
+        """ Revert pending changes to an existing library """
+        return self._api('delete', URL_LIB_COMMIT.format(lib_key=lib_key), None, expect_response=200)
+
+    def _get_library_blocks(self, lib_key):
+        """ Get the list of XBlocks in the library """
+        return self._api('get', URL_LIB_BLOCKS.format(lib_key=lib_key), None, expect_response=200)
+
+    def _add_block_to_library(self, lib_key, block_type, slug, parent_block=None, expect_response=200):
+        """ Add a new XBlock to the library """
+        data = {"block_type": block_type, "definition_id": slug}
+        if parent_block:
+            data["parent_block"] = parent_block
+        return self._api('post', URL_LIB_BLOCKS.format(lib_key=lib_key), data, expect_response)
+
+    def _get_library_block(self, block_key, expect_response=200):
+        """ Get a specific block in the library """
+        return self._api('get', URL_LIB_BLOCK.format(block_key=block_key), None, expect_response)
+
+    def _delete_library_block(self, block_key, expect_response=200):
+        """ Delete a specific block from the library """
+        self._api('delete', URL_LIB_BLOCK.format(block_key=block_key), None, expect_response)
+
+    def _get_library_block_olx(self, block_key, expect_response=200):
+        """ Get the OLX of a specific block in the library """
+        result = self._api('get', URL_LIB_BLOCK_OLX.format(block_key=block_key), None, expect_response)
+        if expect_response == 200:
+            return result["olx"]
+        return result
+
+    def _set_library_block_olx(self, block_key, new_olx, expect_response=200):
+        """ Overwrite the OLX of a specific block in the library """
+        return self._api('post', URL_LIB_BLOCK_OLX.format(block_key=block_key), {"olx": new_olx}, expect_response)
+
+    def _get_library_block_assets(self, block_key, expect_response=200):
+        """ List the static asset files belonging to the specified XBlock """
+        url = URL_LIB_BLOCK_ASSETS.format(block_key=block_key)
+        result = self._api('get', url, None, expect_response)
+        return result["files"] if expect_response == 200 else result
+
+    def _get_library_block_asset(self, block_key, file_name, expect_response=200):
+        """
+        Get metadata about one static asset file belonging to the specified
+        XBlock.
+        """
+        url = URL_LIB_BLOCK_ASSET_FILE.format(block_key=block_key, file_name=file_name)
+        return self._api('get', url, None, expect_response)
+
+    def _set_library_block_asset(self, block_key, file_name, content, expect_response=200):
+        """
+        Set/replace a static asset file belonging to the specified XBlock.
+
+        content should be a binary string.
+        """
+        assert isinstance(content, six.binary_type)
+        file_handle = six.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(
+            response.status_code, expect_response,
+            "Unexpected response code {}:\n{}".format(response.status_code, getattr(response, 'data', '(no data)')),
+        )
+
+    def _delete_library_block_asset(self, block_key, file_name, expect_response=200):
+        """ Delete a static asset file. """
+        url = URL_LIB_BLOCK_ASSET_FILE.format(block_key=block_key, file_name=file_name)
+        return self._api('delete', url, None, expect_response)
+
+    def _render_block_view(self, block_key, view_name, expect_response=200):
+        """
+        Render an XBlock's view in the active application's runtime.
+        Note that this endpoint has different behavior in Studio (draft mode)
+        vs. the LMS (published version only).
+        """
+        url = URL_BLOCK_RENDER_VIEW.format(block_key=block_key, view_name=view_name)
+        return self._api('get', url, None, expect_response)
+
+    def _get_block_handler_url(self, block_key, handler_name):
+        """
+        Get the URL to call a specific XBlock's handler.
+        The URL itself encodes authentication information so can be called
+        without session authentication or any other kind of authentication.
+        """
+        url = URL_BLOCK_GET_HANDLER_URL.format(block_key=block_key, handler_name=handler_name)
+        return self._api('get', url, None, expect_response=200)["handler_url"]

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

@@ -6,34 +6,12 @@ from __future__ import absolute_import, division, print_function, unicode_litera
 import unittest
 from uuid import UUID
 
-from django.conf import settings
-from organizations.models import Organization
-from rest_framework.test import APITestCase
+from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest
 
-from student.tests.factories import UserFactory
-from openedx.core.djangolib.testing.utils import skip_unless_cms
-from openedx.core.lib import blockstore_api
 
-# Define the URLs here - don't use reverse() because we want to detect
-# backwards-incompatible changes like changed URLs.
-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_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_BLOCK = URL_PREFIX + 'blocks/{block_key}/'  # Get data about a block, or delete it
-URL_LIB_BLOCK_OLX = URL_LIB_BLOCK + 'olx/'  # Get or set the OLX of the specified XBlock
-
-URL_BLOCK_RENDER_VIEW = '/api/xblock/v2/xblocks/{block_key}/view/{view_name}/'
-URL_BLOCK_GET_HANDLER_URL = '/api/xblock/v2/xblocks/{block_key}/handler_url/{handler_name}/'
-
-
-@unittest.skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server")
-@skip_unless_cms  # Content Libraries REST API is only available in Studio
-class ContentLibrariesTest(APITestCase):
+class ContentLibrariesTest(ContentLibrariesRestApiTest):
     """
-    Test for Blockstore-based Content Libraries
+    General tests for Blockstore-based Content Libraries
 
     These tests use the REST API, which in turn relies on the Python API.
     Some tests may use the python API directly if necessary to provide
@@ -55,116 +33,6 @@ class ContentLibrariesTest(APITestCase):
     and cached forever.
     """
 
-    @classmethod
-    def setUpClass(cls):
-        super(ContentLibrariesTest, cls).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:
-        cls.collection = blockstore_api.create_collection("Content Library Test Collection")
-        # Create an organization
-        cls.organization = Organization.objects.create(
-            name="Content Libraries Tachyon Exploration & Survey Team",
-            short_name="CL-TEST",
-        )
-
-    def setUp(self):
-        super(ContentLibrariesTest, self).setUp()
-        self.client.login(username=self.user.username, password="edx")
-
-    # API helpers
-
-    def _api(self, method, url, data, expect_response):
-        """
-        Call a REST API
-        """
-        response = getattr(self.client, method)(url, data, format="json")
-        self.assertEqual(
-            response.status_code, expect_response,
-            "Unexpected response code {}:\n{}".format(response.status_code, getattr(response, 'data', '(no data)')),
-        )
-        return response.data
-
-    def _create_library(self, slug, title, description="", expect_response=200):
-        """ Create a library """
-        return self._api('post', URL_LIB_CREATE, {
-            "org": self.organization.short_name,
-            "slug": slug,
-            "title": title,
-            "description": description,
-            "collection_uuid": str(self.collection.uuid),
-        }, expect_response)
-
-    def _get_library(self, lib_key, expect_response=200):
-        """ Get a library """
-        return self._api('get', URL_LIB_DETAIL.format(lib_key=lib_key), None, expect_response)
-
-    def _update_library(self, lib_key, **data):
-        """ Update an existing library """
-        return self._api('patch', URL_LIB_DETAIL.format(lib_key=lib_key), data=data, expect_response=200)
-
-    def _delete_library(self, lib_key, expect_response=200):
-        """ Delete an existing library """
-        return self._api('delete', URL_LIB_DETAIL.format(lib_key=lib_key), None, expect_response)
-
-    def _commit_library_changes(self, lib_key):
-        """ Commit changes to an existing library """
-        return self._api('post', URL_LIB_COMMIT.format(lib_key=lib_key), None, expect_response=200)
-
-    def _revert_library_changes(self, lib_key):
-        """ Revert pending changes to an existing library """
-        return self._api('delete', URL_LIB_COMMIT.format(lib_key=lib_key), None, expect_response=200)
-
-    def _get_library_blocks(self, lib_key):
-        """ Get the list of XBlocks in the library """
-        return self._api('get', URL_LIB_BLOCKS.format(lib_key=lib_key), None, expect_response=200)
-
-    def _add_block_to_library(self, lib_key, block_type, slug, parent_block=None, expect_response=200):
-        """ Add a new XBlock to the library """
-        data = {"block_type": block_type, "definition_id": slug}
-        if parent_block:
-            data["parent_block"] = parent_block
-        return self._api('post', URL_LIB_BLOCKS.format(lib_key=lib_key), data, expect_response)
-
-    def _get_library_block(self, block_key, expect_response=200):
-        """ Get a specific block in the library """
-        return self._api('get', URL_LIB_BLOCK.format(block_key=block_key), None, expect_response)
-
-    def _delete_library_block(self, block_key, expect_response=200):
-        """ Delete a specific block from the library """
-        self._api('delete', URL_LIB_BLOCK.format(block_key=block_key), None, expect_response)
-
-    def _get_library_block_olx(self, block_key, expect_response=200):
-        """ Get the OLX of a specific block in the library """
-        result = self._api('get', URL_LIB_BLOCK_OLX.format(block_key=block_key), None, expect_response)
-        if expect_response == 200:
-            return result["olx"]
-        return result
-
-    def _set_library_block_olx(self, block_key, new_olx, expect_response=200):
-        """ Overwrite the OLX of a specific block in the library """
-        return self._api('post', URL_LIB_BLOCK_OLX.format(block_key=block_key), {"olx": new_olx}, expect_response)
-
-    def _render_block_view(self, block_key, view_name, expect_response=200):
-        """
-        Render an XBlock's view in the active application's runtime.
-        Note that this endpoint has different behavior in Studio (draft mode)
-        vs. the LMS (published version only).
-        """
-        url = URL_BLOCK_RENDER_VIEW.format(block_key=block_key, view_name=view_name)
-        return self._api('get', url, None, expect_response)
-
-    def _get_block_handler_url(self, block_key, handler_name):
-        """
-        Get the URL to call a specific XBlock's handler.
-        The URL itself encodes authentication information so can be called
-        without session authentication or any other kind of authentication.
-        """
-        url = URL_BLOCK_GET_HANDLER_URL.format(block_key=block_key, handler_name=handler_name)
-        return self._api('get', url, None, expect_response=200)["handler_url"]
-
-    # General Content Library tests
-
     def test_library_crud(self):
         """
         Test Create, Read, Update, and Delete of a Content Library

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

@@ -7,7 +7,6 @@ import json
 import unittest
 
 from completion.test_utils import CompletionWaffleTestMixin
-from django.conf import settings
 from django.test import TestCase
 from organizations.models import Organization
 from rest_framework.test import APIClient
@@ -16,7 +15,8 @@ from xblock import fields
 
 from lms.djangoapps.courseware.model_data import get_score
 from openedx.core.djangoapps.content_libraries import api as library_api
-from openedx.core.djangoapps.content_libraries.tests.test_content_libraries import (
+from openedx.core.djangoapps.content_libraries.tests.base import (
+    requires_blockstore,
     URL_BLOCK_RENDER_VIEW,
     URL_BLOCK_GET_HANDLER_URL,
 )
@@ -68,7 +68,7 @@ class ContentLibraryContentTestMixin(object):
         )
 
 
-@unittest.skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server")
+@requires_blockstore
 class ContentLibraryRuntimeTest(ContentLibraryContentTestMixin, TestCase):
     """
     Basic tests of the Blockstore-based XBlock runtime using XBlocks in a
@@ -92,7 +92,7 @@ class ContentLibraryRuntimeTest(ContentLibraryContentTestMixin, TestCase):
         self.assertEqual(problem_block.has_score, True)
 
 
-@unittest.skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server")
+@requires_blockstore
 # We can remove the line below to enable this in Studio once we implement a session-backed
 # field data store which we can use for both studio users and anonymous users
 @skip_unless_lms
@@ -264,7 +264,7 @@ class ContentLibraryXBlockUserStateTest(ContentLibraryContentTestMixin, TestCase
         self.assertEqual(sm.max_grade, 1)
 
 
-@unittest.skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server")
+@requires_blockstore
 @skip_unless_lms  # No completion tracking in Studio
 class ContentLibraryXBlockCompletionTest(ContentLibraryContentTestMixin, CompletionWaffleTestMixin, TestCase):
     """

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

+# -*- coding: utf-8 -*-
+"""
+Tests for static asset files in Blockstore-based Content Libraries
+"""
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+import requests
+
+from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest
+
+# Binary data representing an SVG image file
+SVG_DATA = """<svg xmlns="http://www.w3.org/2000/svg" height="30" width="100">
+  <text x="0" y="15" fill="red">SVG is 🔥</text>
+</svg>""".encode('utf-8')
+
+
+class ContentLibrariesStaticAssetsTest(ContentLibrariesRestApiTest):
+    """
+    Tests for static asset files in Blockstore-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_crud(self):
+        """
+        Test create, read, update, and write of a static asset file.
+
+        Also tests that the static asset file (an image in this case) can be
+        used in an HTML block.
+        """
+        library = self._create_library(slug="asset-lib1", title="Static Assets Test Library")
+        block = self._add_block_to_library(library["id"], "html", "html1")
+        block_id = block["id"]
+        file_name = "image.svg"
+
+        # A new block has no assets:
+        self.assertEqual(self._get_library_block_assets(block_id), [])
+        self._get_library_block_asset(block_id, file_name, expect_response=404)
+
+        # Upload an asset file
+        self._set_library_block_asset(block_id, file_name, SVG_DATA)
+
+        # Get metadata about the uploaded asset file
+        metadata = self._get_library_block_asset(block_id, file_name)
+        self.assertEqual(metadata["path"], file_name)
+        self.assertEqual(metadata["size"], len(SVG_DATA))
+        asset_list = self._get_library_block_assets(block_id)
+        # We don't just assert that 'asset_list == [metadata]' because that may
+        # break in the future if the "get asset" view returns more detail than
+        # the "list assets" view.
+        self.assertEqual(len(asset_list), 1)
+        self.assertEqual(asset_list[0]["path"], metadata["path"])
+        self.assertEqual(asset_list[0]["size"], metadata["size"])
+        self.assertEqual(asset_list[0]["url"], metadata["url"])
+
+        # Download the file and check that it matches what was uploaded.
+        # We need to download using requests since this is served by Blockstore,
+        # which the django test client can't interact with.
+        content_get_result = requests.get(metadata["url"])
+        self.assertEqual(content_get_result.content, SVG_DATA)
+
+        # Set some OLX referencing this asset:
+        self._set_library_block_olx(block_id, """
+            <html display_name="HTML with Image"><![CDATA[
+                <img src="/static/image.svg" alt="An image that says 'SVG is lit' using a fire emoji" />
+            ]]></html>
+        """)
+        # Publish the OLX and the new image file, since published data gets
+        # served differently by Blockstore and we should test that too.
+        self._commit_library_changes(library["id"])
+        metadata = self._get_library_block_asset(block_id, file_name)
+        self.assertEqual(metadata["path"], file_name)
+        self.assertEqual(metadata["size"], len(SVG_DATA))
+        # Download the file from the new URL:
+        content_get_result = requests.get(metadata["url"])
+        self.assertEqual(content_get_result.content, SVG_DATA)
+
+        # Check that the URL in the student_view gets rewritten:
+        fragment = self._render_block_view(block_id, "student_view")
+        self.assertNotIn("/static/image.svg", fragment["content"])
+        self.assertIn(metadata["url"], fragment["content"])
+
+    def test_asset_filenames(self):
+        """
+        Test various allowed and disallowed filenames
+        """
+        library = self._create_library(slug="asset-lib2", title="Static Assets Test Library")
+        block = self._add_block_to_library(library["id"], "html", "html1")
+        block_id = block["id"]
+        file_size = len(SVG_DATA)
+
+        # Unicode names are allowed
+        file_name = "🏕.svg"  # (camping).svg
+        self._set_library_block_asset(block_id, file_name, SVG_DATA)
+        self.assertEqual(self._get_library_block_asset(block_id, file_name)["path"], file_name)
+        self.assertEqual(self._get_library_block_asset(block_id, file_name)["size"], file_size)
+
+        # Subfolder names are allowed
+        file_name = "transcripts/en.srt"
+        self._set_library_block_asset(block_id, file_name, SVG_DATA)
+        self.assertEqual(self._get_library_block_asset(block_id, file_name)["path"], file_name)
+        self.assertEqual(self._get_library_block_asset(block_id, file_name)["size"], file_size)
+
+        # '../' is definitely not allowed
+        file_name = "../definition.xml"
+        self._set_library_block_asset(block_id, file_name, SVG_DATA, expect_response=400)
+
+        # 'a////////b' is not allowed
+        file_name = "a////////b"
+        self._set_library_block_asset(block_id, file_name, SVG_DATA, expect_response=400)

openedx/core/djangoapps/content_libraries/urls.py

@@ -29,9 +29,10 @@ urlpatterns = [
             url(r'^$', views.LibraryBlockView.as_view()),
             # Get the OLX source code of the specified block:
             url(r'^olx/$', views.LibraryBlockOlxView.as_view()),
-            # TODO: Publish the draft changes made to this block:
-            # url(r'^commit/$', views.LibraryBlockCommitView.as_view()),
-            # View todo: discard draft changes
+            # CRUD for static asset files associated with a block in the library:
+            url(r'^assets/$', views.LibraryBlockAssetListView.as_view()),
+            url(r'^assets/(?P<file_path>.+)$', views.LibraryBlockAssetView.as_view()),
+            # Future: publish/discard changes for just this one block
             # Future: set a block's tags (tags are stored in a Tag bundle and linked in)
         ])),
     ])),

openedx/core/djangoapps/content_libraries/views.py

@@ -7,10 +7,11 @@ import logging
 
 from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
 from organizations.models import Organization
+from rest_framework import status
 from rest_framework.exceptions import NotFound, ValidationError
-from rest_framework.views import APIView
+from rest_framework.parsers import MultiPartParser
 from rest_framework.response import Response
-#from rest_framework import authentication, permissions
+from rest_framework.views import APIView
 
 from openedx.core.lib.api.view_utils import view_auth_classes
 from . import api
@@ -21,6 +22,8 @@ from .serializers import (
     LibraryXBlockMetadataSerializer,
     LibraryXBlockTypeSerializer,
     LibraryXBlockOlxSerializer,
+    LibraryXBlockStaticFileSerializer,
+    LibraryXBlockStaticFilesSerializer,
 )
 
 log = logging.getLogger(__name__)
@@ -45,6 +48,9 @@ def convert_exceptions(fn):
         except api.LibraryBlockAlreadyExists as exc:
             log.exception(exc.message)
             raise ValidationError(exc.message)
+        except api.InvalidNameError as exc:
+            log.exception(exc.message)
+            raise ValidationError(exc.message)
     return wrapped_fn
 
 
@@ -261,3 +267,69 @@ class LibraryBlockOlxView(APIView):
         except ValueError as err:
             raise ValidationError(detail=str(err))
         return Response(LibraryXBlockOlxSerializer({"olx": new_olx_str}).data)
+
+
+@view_auth_classes()
+class LibraryBlockAssetListView(APIView):
+    """
+    Views to list an existing XBlock's static asset files
+    """
+    @convert_exceptions
+    def get(self, request, usage_key_str):
+        """
+        List the static asset files belonging to this block.
+        """
+        key = LibraryUsageLocatorV2.from_string(usage_key_str)
+        files = api.get_library_block_static_asset_files(key)
+        return Response(LibraryXBlockStaticFilesSerializer({"files": files}).data)
+
+
+@view_auth_classes()
+class LibraryBlockAssetView(APIView):
+    """
+    Views to work with an existing XBlock's static asset files
+    """
+    parser_classes = (MultiPartParser, )
+
+    @convert_exceptions
+    def get(self, request, usage_key_str, file_path):
+        """
+        Get a static asset file belonging to this block.
+        """
+        key = LibraryUsageLocatorV2.from_string(usage_key_str)
+        files = api.get_library_block_static_asset_files(key)
+        for f in files:
+            if f.path == file_path:
+                return Response(LibraryXBlockStaticFileSerializer(f).data)
+        raise NotFound
+
+    @convert_exceptions
+    def put(self, request, usage_key_str, file_path):
+        """
+        Replace a static asset file belonging to this block.
+        """
+        usage_key = LibraryUsageLocatorV2.from_string(usage_key_str)
+        file_wrapper = request.data['content']
+        if file_wrapper.size > 20 * 1024 * 1024:  # > 20 MiB
+            # In the future, we need a way to use file_wrapper.chunks() to read
+            # the file in chunks and stream that to Blockstore, but Blockstore
+            # currently lacks an API for streaming file uploads.
+            raise ValidationError("File too big")
+        file_content = file_wrapper.read()
+        try:
+            result = api.add_library_block_static_asset_file(usage_key, file_path, file_content)
+        except ValueError:
+            raise ValidationError("Invalid file path")
+        return Response(LibraryXBlockStaticFileSerializer(result).data)
+
+    @convert_exceptions
+    def delete(self, request, usage_key_str, file_path):  # pylint: disable=unused-argument
+        """
+        Delete a static asset file belonging to this block.
+        """
+        usage_key = LibraryUsageLocatorV2.from_string(usage_key_str)
+        try:
+            api.delete_library_block_static_asset_file(usage_key, file_path)
+        except ValueError:
+            raise ValidationError("Invalid file path")
+        return Response(status=status.HTTP_204_NO_CONTENT)

openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py

@@ -4,6 +4,7 @@ XBlock field data directly from Blockstore.
 """
 from __future__ import absolute_import, division, print_function, unicode_literals
 import logging
+import os.path
 
 from lxml import etree
 from opaque_keys.edx.locator import BundleDefinitionLocator
@@ -14,7 +15,11 @@ from openedx.core.djangoapps.xblock.learning_context.manager import get_learning
 from openedx.core.djangoapps.xblock.runtime.runtime import XBlockRuntime
 from openedx.core.djangoapps.xblock.runtime.olx_parsing import parse_xblock_include
 from openedx.core.djangoapps.xblock.runtime.serializer import serialize_xblock
-from openedx.core.djangolib.blockstore_cache import BundleCache, get_bundle_file_data_with_cache
+from openedx.core.djangolib.blockstore_cache import (
+    BundleCache,
+    get_bundle_file_data_with_cache,
+    get_bundle_file_metadata_with_cache,
+)
 from openedx.core.lib import blockstore_api
 
 log = logging.getLogger(__name__)
@@ -148,13 +153,45 @@ class BlockstoreXBlockRuntime(XBlockRuntime):
         olx_path = definition_key.olx_path
         blockstore_api.write_draft_file(draft_uuid, olx_path, olx_str)
         # And the other files, if any:
+        olx_static_path = os.path.dirname(olx_path) + '/static/'
         for fh in static_files:
-            raise NotImplementedError(
-                "Need to write static file {} to blockstore but that's not yet implemented yet".format(fh.name)
-            )
+            new_path = olx_static_path + fh.name
+            blockstore_api.write_draft_file(draft_uuid, new_path, fh.data)
         # Now invalidate the blockstore data cache for the bundle:
         BundleCache(definition_key.bundle_uuid, draft_name=definition_key.draft_name).clear()
 
+    def _lookup_asset_url(self, block, asset_path):
+        """
+        Return an absolute URL for the specified static asset file that may
+        belong to this XBlock.
+
+        e.g. if the XBlock settings have a field value like "/static/foo.png"
+        then this method will be called with asset_path="foo.png" and should
+        return a URL like https://cdn.none/xblock/f843u89789/static/foo.png
+
+        If the asset file is not recognized, return None
+        """
+        if '..' in asset_path:
+            return None  # Illegal path
+        definition_key = block.scope_ids.def_id
+        # Compute the full path to the static file in the bundle,
+        # e.g. "problem/prob1/static/illustration.svg"
+        expanded_path = os.path.dirname(definition_key.olx_path) + '/static/' + asset_path
+        try:
+            metadata = get_bundle_file_metadata_with_cache(
+                bundle_uuid=definition_key.bundle_uuid,
+                path=expanded_path,
+                bundle_version=definition_key.bundle_version,
+                draft_name=definition_key.draft_name,
+            )
+        except blockstore_api.BundleFileNotFound:
+            log.warning("XBlock static file not found: %s for %s", asset_path, block.scope_ids.usage_id)
+            return None
+        # Make sure the URL is one that will work from the user's browser,
+        # not one that only works from within a docker container:
+        url = blockstore_api.force_browser_url(metadata.url)
+        return url
+
 
 def xml_for_definition(definition_key):
     """

openedx/core/djangoapps/xblock/runtime/runtime.py

@@ -25,7 +25,8 @@ from lms.djangoapps.grades.api import signals as grades_signals
 from openedx.core.djangoapps.xblock.apps import get_xblock_app_config
 from openedx.core.djangoapps.xblock.runtime.blockstore_field_data import BlockstoreFieldData
 from openedx.core.djangoapps.xblock.runtime.mixin import LmsBlockMixin
-from openedx.core.lib.xblock_utils import xblock_local_resource_url
+from openedx.core.lib.xblock_utils import wrap_fragment, xblock_local_resource_url
+from static_replace import process_static_urls
 from xmodule.errortracker import make_error_tracker
 from .id_managers import OpaqueKeyReader
 from .shims import RuntimeShim, XBlockShim
@@ -289,8 +290,60 @@ class XBlockRuntime(RuntimeShim, Runtime):
                     log.debug("-> Relative resource URL: %s", resource['data'])
                     resource['data'] = get_xblock_app_config().get_site_root_url() + resource['data']
             fragment = Fragment.from_dict(frag_data)
+
+        # Apply any required transforms to the fragment.
+        # We could move to doing this in wrap_xblock() and/or use an array of
+        # wrapper methods like the ConfigurableFragmentWrapper mixin does.
+        fragment = wrap_fragment(fragment, self.transform_static_paths_to_urls(block, fragment.content))
+
         return fragment
 
+    def transform_static_paths_to_urls(self, block, html_str):
+        """
+        Given an HTML string, replace any static file paths like
+            /static/foo.png
+        (which are really pointing to block-specific assets stored in blockstore)
+        with working absolute URLs like
+            https://s3.example.com/blockstore/bundle17/this-block/assets/324.png
+        See common/djangoapps/static_replace/__init__.py
+
+        This is generally done automatically for the HTML rendered by XBlocks,
+        but if an XBlock wants to have correct URLs in data returned by its
+        handlers, the XBlock must call this API directly.
+
+        Note that the paths are only replaced if they are in "quotes" such as if
+        they are an HTML attribute or JSON data value. Thus, to transform only a
+        single path string on its own, you must pass html_str=f'"{path}"'
+        """
+
+        def replace_static_url(original, prefix, quote, rest):  # pylint: disable=unused-argument
+            """
+            Replace a single matched url.
+            """
+            original_url = prefix + rest
+            # Don't mess with things that end in '?raw'
+            if rest.endswith('?raw'):
+                new_url = original_url
+            else:
+                new_url = self._lookup_asset_url(block, rest) or original_url
+            return "".join([quote, new_url, quote])
+
+        return process_static_urls(html_str, replace_static_url)
+
+    def _lookup_asset_url(self, block, asset_path):  # pylint: disable=unused-argument
+        """
+        Return an absolute URL for the specified static asset file that may
+        belong to this XBlock.
+
+        e.g. if the XBlock settings have a field value like "/static/foo.png"
+        then this method will be called with asset_path="foo.png" and should
+        return a URL like https://cdn.none/xblock/f843u89789/static/foo.png
+
+        If the asset file is not recognized, return None
+        """
+        # Subclasses should override this
+        return None
+
 
 class XBlockRuntimeSystem(object):
     """

openedx/core/djangoapps/xblock/runtime/shims.py

@@ -11,9 +11,9 @@ from django.core.cache import cache
 from django.template import TemplateDoesNotExist
 from django.utils.functional import cached_property
 from fs.memoryfs import MemoryFS
+import six
 
 from edxmako.shortcuts import render_to_string
-import six
 
 
 class RuntimeShim(object):
@@ -184,17 +184,24 @@ class RuntimeShim(object):
 
     def replace_urls(self, html_str):
         """
-        Given an HTML string, replace any static file URLs like
+        Deprecated precursor to transform_static_paths_to_urls
+
+        Given an HTML string, replace any static file paths like
             /static/foo.png
-        with working URLs like
-            https://s3.example.com/course/this/assets/foo.png
+        (which are really pointing to block-specific assets stored in blockstore)
+        with working absolute URLs like
+            https://s3.example.com/blockstore/bundle17/this-block/assets/324.png
         See common/djangoapps/static_replace/__init__.py
+
+        This is generally done automatically for the HTML rendered by XBlocks,
+        but if an XBlock wants to have correct URLs in data returned by its
+        handlers, the XBlock must call this API directly.
+
+        Note that the paths are only replaced if they are in "quotes" such as if
+        they are an HTML attribute or JSON data value. Thus, to transform only a
+        single path string on its own, you must pass html_str=f'"{path}"'
         """
-        # TODO: implement or deprecate.
-        # Can we replace this with a filesystem service that has a .get_url
-        # method on all files? See comments in the 'resources_fs' property.
-        # See also the version in openedx/core/lib/xblock_utils/__init__.py
-        return html_str  # For now just return without changes.
+        return self.transform_static_paths_to_urls(self._active_block, html_str)
 
     def replace_course_urls(self, html_str):
         """

openedx/core/djangoapps/xblock/tests/README.rst

+XBlock App Suite Tests
+======================
+
+Where are they?
+
+As much of the runtime and XBlock API code depends on a learning context, the
+XBlock and XBlock Runtime APIs (both python and REST) in this django app are
+tested via tests found in `content_libraries/tests <../../content_libraries/tests>`_.

openedx/core/lib/blockstore_api/__init__.py

@@ -42,6 +42,8 @@ from .methods import (
     # Links:
     get_bundle_links,
     get_bundle_version_links,
+    # Misc:
+    force_browser_url,
 )
 from .exceptions import (
     BlockstoreException,

openedx/core/lib/blockstore_api/methods.py

@@ -390,3 +390,20 @@ def encode_str_for_draft(input_str):
     else:
         binary = input_str
     return base64.b64encode(binary)
+
+
+def force_browser_url(blockstore_file_url):
+    """
+    Ensure that the given URL Blockstore 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:')