Skip to content

Commit

Permalink
feat: versioned asset support for Learning Core XBlock runtime
Browse files Browse the repository at this point in the history 
Add support for displaying static assets in the Learing Core XBlock
runtime via "/static/asset-name" style substitutions in the OLX. This is
currently used for new Content Library components.

Static asset display is version-aware, so viewing older versions of the
XBlock content via the embed view will show the appropriate assets for
that version.
  • Loading branch information
@ormsbee
ormsbee committed Oct 17, 2024
1 parent 795d039 commit 4510124
Show file tree
Hide file tree
Showing 3 changed files with 213 additions and 22 deletions.
53 changes: 50 additions & 3 deletions openedx/core/djangoapps/content_libraries/tests/test_embed_block.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class LibrariesEmbedViewTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestM
"""

@XBlock.register_temp_plugin(FieldsTestBlock, FieldsTestBlock.BLOCK_TYPE)
def test_embed_vew_versions(self):
def test_embed_view_versions(self):
"""
Test that the embed_view renders a block and can render different versions of it.
"""
Expand Down Expand Up @@ -177,8 +177,55 @@ def check_fields(display_name, setting_field, content_field):
html = self._embed_block(block_id)
check_fields(display_name="DN-01", setting_field="SV-01", content_field="CV-01")

# TODO: test that any static assets referenced in the student_view html are loaded as the correct version, and not
# always loaded as "latest draft".
def test_embed_view_versions_static_assets(self):
"""
Test asset substitution and version-awareness.
"""
# Create a library:
lib = self._create_library(
slug="test-eb-asset-1", title="Asset Test Library", description="",
)
lib_id = lib["id"]

# Create an HTMLBlock. This will be the empty version 1:
create_response = self._add_block_to_library(lib_id, "html", "asset_block")
block_id = create_response["id"]

# Create version 2 of the block by setting its OLX. This has a reference
# to an image, but not the image itself–so it won't get auto-replaced.
olx_response = self._set_library_block_olx(block_id, """
<html display_name="Asset Test Component"><![CDATA[
<p>This is the enemy of our garden:</p>
<p><img src="/static/deer.jpg"/></p>
]]></html>
""")
assert olx_response["version_num"] == 2

# Create version 3 with some bogus file data
self._set_library_block_asset(block_id, "static/deer.jpg", b"This is not a valid JPEG file")

# Publish the library (making version 3 the published state):
self._commit_library_changes(lib_id)

# Create version 4 by deleting the asset
self._delete_library_block_asset(block_id, "static/deer.jpg")

# Grab version 2, which has the asset reference but not the asset. No
# substitution should happen.
html = self._embed_block(block_id, version=2)
assert 'src="/static/deer.jpg"' in html

# Grab the published version 3. This has the asset, so the link should
# show up.
html = self._embed_block(block_id, version='published')
# This is the pattern we're looking for:
# <img src="https://localhost:18010/library_assets/b5864c63-e1da-4d48-8c8a-cc718e2f9ad3/static/deer.jpg"/>
assert re.search(r'/library_assets/[0-9a-f-]*/static/deer.jpg', html)

# Now grab the draft version (4), which is going to once again not have
# the asset (because we deleted it).
html = self._embed_block(block_id, version='draft')
assert 'src="/static/deer.jpg"' in html

# TODO: if we are ever able to run these tests in the LMS, test that the LMS only allows accessing the published
# version.
143 changes: 137 additions & 6 deletions openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from django.core.exceptions import ObjectDoesNotExist, ValidationError
from django.db.transaction import atomic
from django.urls import reverse

from openedx_learning.api import authoring as authoring_api

Expand All @@ -19,7 +20,9 @@
from xblock.fields import Field, Scope, ScopeIds
from xblock.field_data import FieldData

from openedx.core.djangoapps.xblock.api import get_xblock_app_config
from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_for_learning_core
from openedx.core.lib.xblock_serializer.data import StaticFile
from ..data import AuthoredDataMode, LatestVersion
from ..learning_context.manager import get_learning_context_impl
from .runtime import XBlockRuntime
Expand Down Expand Up @@ -230,6 +233,33 @@ def get_block(self, usage_key, for_parent=None, *, version: int | LatestVersion

return block

def get_block_assets(self, block):
"""
Return a list of StaticFile entries.
TODO: When we want to copy a whole Section at a time, doing these
lookups one by one is going to get slow. At some point we're going to
want something to look up a bunch of blocks at once.
"""
component_version = self._get_component_version_from_block(block)

# cvc = the ComponentVersionContent through-table
cvc_list = (
component_version
.componentversioncontent_set
.filter(content__has_file=True)
.order_by('key')
)

return [
StaticFile(
name=cvc.key,
url=self._absolute_url_for_asset(component_version, cvc.key),
data=None,
)
for cvc in cvc_list
]

def save_block(self, block):
"""
Save any pending field data values to Learning Core data models.
Expand Down Expand Up @@ -300,19 +330,120 @@ def _get_component_from_usage_key(self, usage_key):

return component

def _lookup_asset_url(self, block: XBlock, asset_path: str) -> str | None: # pylint: disable=unused-argument
def _get_component_version_from_block(self, block):
"""
Given an XBlock instance, return the Learning Core ComponentVersion.
This relies on our runtime setting the _runtime_requested_version
attribute on blocks that it fetches.
"""
usage_key = block.usage_key
component = self._get_component_from_usage_key(usage_key)

block_version = block._runtime_requested_version # pylint: disable=protected-access
if block_version == LatestVersion.DRAFT:
component_version = component.versioning.draft
elif block_version == LatestVersion.PUBLISHED:
component_version = component.versioning.published
else:
component_version = component.versioning.version_num(block_version)

return component_version

def _absolute_url_for_asset(self, component_version, asset_path):
"""
The full URL for a specific library asset in a ComponentVersion.
This does not check for whether the path actually exists–it just returns
where it would be if it did exist.
"""
# This function should return absolute URLs, so we need the site root.
site_root_url = get_xblock_app_config().get_site_root_url()

return site_root_url + reverse(
'content_libraries:library-assets',
kwargs={
'component_version_uuid': component_version.uuid,
'asset_path': f"static/{asset_path}",
}
)

def _lookup_asset_url(self, block: XBlock, asset_path: str) -> str | None:
"""
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
e.g. if the XBlock settings have a field value like "/static/test.png"
then this method will be called with asset_path="test.png" and should
return a URL like:
http://studio.local.openedx.io:8001/library_assets/cd31871e-a342-4c3f-ba2f-a661bf630996/static/test.png
If the asset file is not recognized, return None
This is called by the XBlockRuntime superclass in the .runtime module.
TODO: Implement as part of larger static asset effort.
Implementation Details
----------------------
The standard XBlock "static/{asset_path}" substitution strips out the
leading "static/" part because it assumes that all files will exist in a
shared, flat namespace (i.e. a course's Files and Uploads).
Learning Core treats assets differently. Each Component has its own,
isolated namespace for asset storage. Furthermore, that namespace
contains content that are not meant to be downloadable, like the
block.xml (the OLX of the Component). There may one day be other files
that are not meant to be externally downloadable there as well, like
Markdown or LaTeX source files or grader code.
By convention, the static assets that we store in Learning Core and are
meant for download sit inside a static/ directory that is local to each
Component (and actually separate for each Component Version).
So the transformation looks like this:
1. The Learning Core ComponentVersion has an asset stored as
``static/test.png`` in the database.
2. The original OLX content we store references ``/static/test.png``,
per OLX convention. Note the leading "/".
3. The ReplaceURLService XBlock runtime service invokes
``static_replace`` and strips out ``/static/``.
4. The method we're in now is invoked with a ``static_path`` of just
``test.png``, because that's the transformation that works for
ModuleStore-based courses, where everything is stored in the root of
a shared Files and Uploads space.
5. This method then builds a URL that re-adds the "static/" prefix, and
then points to the ComponentVersion-specific location for that asset.
Note: This means that the URL for a static asset will change with every
new version of the Component that is created, i.e. with every edit
that's saved–even if the asset itself doesn't change. This was the
tradeoff we made in order to put each version's assets in an isolated
space, and to make sure that we don't break things like relative links.
On the backend, we only store the asset once.
Performance Note
----------------
This can theoretically get very expensive if there are many, many static
asset references in a Component. It's also very cacheable–we could put
it in a RequestCache with a key of (usage_key, version_num), and have
the value be the component_version.uuid and the full list of assets. I'm
not doing this yet in order to keep the code simpler, but I'm leaving
this note here in case someone needs to optimize this later.
"""
return None
component_version = self._get_component_version_from_block(block)

try:
content = (
component_version
.componentversioncontent_set
.filter(content__has_file=True)
.get(key=f"static/{asset_path}")
)
except ObjectDoesNotExist:
# This means we see a path that _looks_ like it should be a static
# asset for this Component, but that static asset doesn't really
# exist.
return None

return self._absolute_url_for_asset(component_version, asset_path)
39 changes: 26 additions & 13 deletions openedx/core/lib/xblock_serializer/block_serializer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,32 @@ def __init__(self, block):
# Search the OLX for references to files stored in the course's
# "Files & Uploads" (contentstore):
self.olx_str = utils.rewrite_absolute_static_urls(self.olx_str, course_key)
for asset in utils.collect_assets_from_text(self.olx_str, course_key):
path = asset['path']
if path not in [sf.name for sf in self.static_files]:
self.static_files.append(StaticFile(name=path, url=asset['url'], data=None))

if block.scope_ids.usage_id.block_type in ['problem', 'vertical']:
py_lib_zip_file = utils.get_python_lib_zip_if_using(self.olx_str, course_key)
if py_lib_zip_file:
self.static_files.append(py_lib_zip_file)

js_input_files = utils.get_js_input_files_if_using(self.olx_str, course_key)
for js_input_file in js_input_files:
self.static_files.append(js_input_file)

runtime_supports_explicit_assets = hasattr(block.runtime, 'get_block_assets')
if runtime_supports_explicit_assets:
# If a block supports explicitly tracked assets, things are simple.
# Learning Core backed content supports this, which currently means
# v2 Content Libraries.
self.static_files.extend(
block.runtime.get_block_assets(block)
)
else:
# Otherwise, we have to scan the content to extract associated asset
# by inference. This is what we have to do for Modulestore-backed
# courses, which store files a course-global "Files and Uploads".
for asset in utils.collect_assets_from_text(self.olx_str, course_key):
path = asset['path']
if path not in [sf.name for sf in self.static_files]:
self.static_files.append(StaticFile(name=path, url=asset['url'], data=None))

if block.scope_ids.usage_id.block_type in ['problem', 'vertical']:
py_lib_zip_file = utils.get_python_lib_zip_if_using(self.olx_str, course_key)
if py_lib_zip_file:
self.static_files.append(py_lib_zip_file)

js_input_files = utils.get_js_input_files_if_using(self.olx_str, course_key)
for js_input_file in js_input_files:
self.static_files.append(js_input_file)

def _serialize_block(self, block) -> etree.Element:
""" Serialize an XBlock to OLX/XML. """
Expand Down

0 comments on commit 4510124

Please sign in to comment.