Skip to content

Commit d18ae34

Browse files
committed
feat: new view & api calls to serve content library assets
This commit adds a new view to serve static assets for content libraries, along with Content Library API calls to add, delete, and get metadata about these assets. These assets come from Learning Core and should ONLY BE ACCESSED FROM STUDIO. Users must have read access to the library in order to see an asset in that library. This also re-implments video transcript support for content libraries and re-enables some previously disabled tests around it.
1 parent 2bbd8ec commit d18ae34

File tree

7 files changed

+373
-33
lines changed

7 files changed

+373
-33
lines changed

cms/envs/test.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,3 +333,13 @@
333333
"SECRET": "***",
334334
"URL": "***",
335335
}
336+
337+
############## openedx-learning (Learning Core) config ##############
338+
OPENEDX_LEARNING = {
339+
'MEDIA': {
340+
'BACKEND': 'django.core.files.storage.InMemoryStorage',
341+
'OPTIONS': {
342+
'location': MEDIA_ROOT + "_private"
343+
}
344+
}
345+
}

openedx/core/djangoapps/content_libraries/api.py

Lines changed: 118 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import base64
5757
import hashlib
5858
import logging
59+
import mimetypes
5960

6061
import attr
6162
import requests
@@ -68,6 +69,7 @@
6869
from django.db.models import Q, QuerySet
6970
from django.utils.translation import gettext as _
7071
from edx_rest_api_client.client import OAuthAPIClient
72+
from django.urls import reverse
7173
from lxml import etree
7274
from opaque_keys.edx.keys import BlockTypeKey, UsageKey, UsageKeyV2
7375
from opaque_keys.edx.locator import (
@@ -96,7 +98,11 @@
9698
from xblock.core import XBlock
9799
from xblock.exceptions import XBlockNotFoundError
98100

99-
from openedx.core.djangoapps.xblock.api import get_component_from_usage_key, xblock_type_display_name
101+
from openedx.core.djangoapps.xblock.api import (
102+
get_component_from_usage_key,
103+
get_xblock_app_config,
104+
xblock_type_display_name,
105+
)
100106
from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_for_learning_core
101107
from xmodule.modulestore.django import modulestore
102108

@@ -1018,18 +1024,48 @@ def get_library_block_static_asset_files(usage_key) -> list[LibraryXBlockStaticF
10181024
10191025
Returns a list of LibraryXBlockStaticFile objects, sorted by path.
10201026
1021-
TODO: This is not yet implemented for Learning Core backed libraries.
10221027
TODO: Should this be in the general XBlock API rather than the libraries API?
10231028
"""
1024-
return []
1029+
component = get_component_from_usage_key(usage_key)
1030+
component_version = component.versioning.draft
1031+
1032+
# If there is no Draft version, then this was soft-deleted
1033+
if component_version is None:
1034+
return []
1035+
1036+
# cvc = the ComponentVersionContent through table
1037+
cvc_set = (
1038+
component_version
1039+
.componentversioncontent_set
1040+
.filter(content__has_file=True)
1041+
.order_by('key')
1042+
.select_related('content')
1043+
)
1044+
1045+
site_root_url = get_xblock_app_config().get_site_root_url()
1046+
1047+
return [
1048+
LibraryXBlockStaticFile(
1049+
path=cvc.key,
1050+
size=cvc.content.size,
1051+
url=site_root_url + reverse(
1052+
'content_libraries:library-assets',
1053+
kwargs={
1054+
'component_version_uuid': component_version.uuid,
1055+
'asset_path': cvc.key,
1056+
}
1057+
),
1058+
)
1059+
for cvc in cvc_set
1060+
]
10251061

10261062

1027-
def add_library_block_static_asset_file(usage_key, file_name, file_content) -> LibraryXBlockStaticFile:
1063+
def add_library_block_static_asset_file(usage_key, file_path, file_content, user=None) -> LibraryXBlockStaticFile:
10281064
"""
10291065
Upload a static asset file into the library, to be associated with the
10301066
specified XBlock. Will silently overwrite an existing file of the same name.
10311067
1032-
file_name should be a name like "doc.pdf". It may optionally contain slashes
1068+
file_path should be a name like "doc.pdf". It may optionally contain slashes
10331069
like 'en/doc.pdf'
10341070
file_content should be a binary string.
10351071
@@ -1041,10 +1077,67 @@ def add_library_block_static_asset_file(usage_key, file_name, file_content) -> L
10411077
video_block = UsageKey.from_string("lb:VideoTeam:python-intro:video:1")
10421078
add_library_block_static_asset_file(video_block, "subtitles-en.srt", subtitles.encode('utf-8'))
10431079
"""
1044-
raise NotImplementedError("Static assets not yet implemented for Learning Core")
1080+
# File path validations copied over from v1 library logic. This can't really
1081+
# hurt us inside our system because we never use these paths in an actual
1082+
# file system–they're just string keys that point to hash-named data files
1083+
# in a common library (learning package) level directory. But it might
1084+
# become a security issue during import/export serialization.
1085+
if file_path != file_path.strip().strip('/'):
1086+
raise InvalidNameError("file_path cannot start/end with / or whitespace.")
1087+
if '//' in file_path or '..' in file_path:
1088+
raise InvalidNameError("Invalid sequence (// or ..) in file_path.")
1089+
1090+
component = get_component_from_usage_key(usage_key)
1091+
1092+
media_type_str, _encoding = mimetypes.guess_type(file_path)
1093+
# We use "application/octet-stream" as a generic fallback media type, per
1094+
# RFC 2046: https://datatracker.ietf.org/doc/html/rfc2046
1095+
# TODO: This probably makes sense to push down to openedx-learning?
1096+
media_type_str = media_type_str or "application/octet-stream"
1097+
1098+
now = datetime.now(tz=timezone.utc)
1099+
1100+
with transaction.atomic():
1101+
media_type = authoring_api.get_or_create_media_type(media_type_str)
1102+
content = authoring_api.get_or_create_file_content(
1103+
component.publishable_entity.learning_package.id,
1104+
media_type.id,
1105+
data=file_content,
1106+
created=now,
1107+
)
1108+
component_version = authoring_api.create_next_component_version(
1109+
component.pk,
1110+
content_to_replace={file_path: content.id},
1111+
created=now,
1112+
created_by=user.id if user else None,
1113+
)
1114+
transaction.on_commit(
1115+
lambda: LIBRARY_BLOCK_UPDATED.send_event(
1116+
library_block=LibraryBlockData(
1117+
library_key=usage_key.context_key,
1118+
usage_key=usage_key,
1119+
)
1120+
)
1121+
)
10451122

1123+
# Now figure out the URL for the newly created asset...
1124+
site_root_url = get_xblock_app_config().get_site_root_url()
1125+
local_path = reverse(
1126+
'content_libraries:library-assets',
1127+
kwargs={
1128+
'component_version_uuid': component_version.uuid,
1129+
'asset_path': file_path,
1130+
}
1131+
)
1132+
1133+
return LibraryXBlockStaticFile(
1134+
path=file_path,
1135+
url=site_root_url + local_path,
1136+
size=content.size,
1137+
)
10461138

1047-
def delete_library_block_static_asset_file(usage_key, file_name):
1139+
1140+
def delete_library_block_static_asset_file(usage_key, file_path, user=None):
10481141
"""
10491142
Delete a static asset file from the library.
10501143
@@ -1054,7 +1147,24 @@ def delete_library_block_static_asset_file(usage_key, file_name):
10541147
video_block = UsageKey.from_string("lb:VideoTeam:python-intro:video:1")
10551148
delete_library_block_static_asset_file(video_block, "subtitles-en.srt")
10561149
"""
1057-
raise NotImplementedError("Static assets not yet implemented for Learning Core")
1150+
component = get_component_from_usage_key(usage_key)
1151+
now = datetime.now(tz=timezone.utc)
1152+
1153+
with transaction.atomic():
1154+
component_version = authoring_api.create_next_component_version(
1155+
component.pk,
1156+
content_to_replace={file_path: None},
1157+
created=now,
1158+
created_by=user.id if user else None,
1159+
)
1160+
transaction.on_commit(
1161+
lambda: LIBRARY_BLOCK_UPDATED.send_event(
1162+
library_block=LibraryBlockData(
1163+
library_key=usage_key.context_key,
1164+
usage_key=usage_key,
1165+
)
1166+
)
1167+
)
10581168

10591169

10601170
def get_allowed_block_types(library_key): # pylint: disable=unused-argument

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -661,13 +661,13 @@ def test_library_permissions(self): # pylint: disable=too-many-statements
661661
self._get_library_block_olx(block3_key, expect_response=403)
662662
self._get_library_block_fields(block3_key, expect_response=403)
663663
self._get_library_block_assets(block3_key, expect_response=403)
664-
self._get_library_block_asset(block3_key, file_name="whatever.png", expect_response=403)
664+
self._get_library_block_asset(block3_key, file_name="static/whatever.png", expect_response=403)
665665
# Nor can they preview the block:
666666
self._render_block_view(block3_key, view_name="student_view", expect_response=403)
667667
# But if we grant allow_public_read, then they can:
668668
with self.as_user(admin):
669669
self._update_library(lib_id, allow_public_read=True)
670-
# self._set_library_block_asset(block3_key, "whatever.png", b"data")
670+
self._set_library_block_asset(block3_key, "static/whatever.png", b"data")
671671
with self.as_user(random_user):
672672
self._get_library_block_olx(block3_key)
673673
self._render_block_view(block3_key, view_name="student_view")
@@ -680,7 +680,7 @@ def test_library_permissions(self): # pylint: disable=too-many-statements
680680
with self.as_user(user):
681681
self._set_library_block_olx(block3_key, "<problem/>", expect_response=403)
682682
self._set_library_block_fields(block3_key, {"data": "<problem />", "metadata": {}}, expect_response=403)
683-
# self._set_library_block_asset(block3_key, "test.txt", b"data", expect_response=403)
683+
self._set_library_block_asset(block3_key, "static/test.txt", b"data", expect_response=403)
684684
self._delete_library_block(block3_key, expect_response=403)
685685
self._commit_library_changes(lib_id, expect_response=403)
686686
self._revert_library_changes(lib_id, expect_response=403)
@@ -690,9 +690,9 @@ def test_library_permissions(self): # pylint: disable=too-many-statements
690690
olx = self._get_library_block_olx(block3_key)
691691
self._set_library_block_olx(block3_key, olx)
692692
self._set_library_block_fields(block3_key, {"data": olx, "metadata": {}})
693-
# self._get_library_block_assets(block3_key)
694-
# self._set_library_block_asset(block3_key, "test.txt", b"data")
695-
# self._get_library_block_asset(block3_key, file_name="test.txt")
693+
self._get_library_block_assets(block3_key)
694+
self._set_library_block_asset(block3_key, "static/test.txt", b"data")
695+
self._get_library_block_asset(block3_key, file_name="static/test.txt")
696696
self._delete_library_block(block3_key)
697697
self._commit_library_changes(lib_id)
698698
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):
915915
event_receiver.call_args.kwargs
916916
)
917917

918-
@skip("We still need to re-implement static asset handling.")
919918
def test_library_block_add_asset_update_event(self):
920919
"""
921920
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):
934933

935934
block = self._add_block_to_library(lib_id, "unit", "u1")
936935
block_id = block["id"]
937-
self._set_library_block_asset(block_id, "test.txt", b"data")
936+
self._set_library_block_asset(block_id, "static/test.txt", b"data")
938937

939938
usage_key = LibraryUsageLocatorV2(
940939
lib_key=library_key,
@@ -955,7 +954,6 @@ def test_library_block_add_asset_update_event(self):
955954
event_receiver.call_args.kwargs
956955
)
957956

958-
@skip("We still need to re-implement static asset handling.")
959957
def test_library_block_del_asset_update_event(self):
960958
"""
961959
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):
974972

975973
block = self._add_block_to_library(lib_id, "unit", "u1")
976974
block_id = block["id"]
977-
self._set_library_block_asset(block_id, "test.txt", b"data")
975+
self._set_library_block_asset(block_id, "static/test.txt", b"data")
978976

979-
self._delete_library_block_asset(block_id, 'text.txt')
977+
self._delete_library_block_asset(block_id, 'static/text.txt')
980978

981979
usage_key = LibraryUsageLocatorV2(
982980
lib_key=library_key,

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

Lines changed: 84 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
"""
22
Tests for static asset files in Learning-Core-based Content Libraries
33
"""
4-
from unittest import skip
4+
from uuid import UUID
55

6+
from opaque_keys.edx.keys import UsageKey
7+
8+
from common.djangoapps.student.tests.factories import UserFactory
69
from openedx.core.djangoapps.content_libraries.tests.base import (
710
ContentLibrariesRestApiTest,
811
)
12+
from openedx.core.djangoapps.xblock.api import get_component_from_usage_key
13+
from openedx.core.djangolib.testing.utils import skip_unless_cms
914

1015
# Binary data representing an SVG image file
1116
SVG_DATA = """<svg xmlns="http://www.w3.org/2000/svg" height="30" width="100">
@@ -23,15 +28,10 @@
2328
"""
2429

2530

26-
@skip("Assets are being reimplemented in Learning Core. Disable until that's ready.")
31+
@skip_unless_cms
2732
class ContentLibrariesStaticAssetsTest(ContentLibrariesRestApiTest):
2833
"""
2934
Tests for static asset files in Learning-Core-based Content Libraries
30-
31-
WARNING: every test should have a unique library slug, because even though
32-
the django/mysql database gets reset for each test case, the lookup between
33-
library slug and bundle UUID does not because it's assumed to be immutable
34-
and cached forever.
3535
"""
3636

3737
def test_asset_filenames(self):
@@ -79,7 +79,7 @@ def test_video_transcripts(self):
7979
/>
8080
""")
8181
# Upload the transcript file
82-
self._set_library_block_asset(block_id, "3_yD_cEKoCk-en.srt", TRANSCRIPT_DATA)
82+
self._set_library_block_asset(block_id, "static/3_yD_cEKoCk-en.srt", TRANSCRIPT_DATA)
8383

8484
transcript_handler_url = self._get_block_handler_url(block_id, "transcript")
8585

@@ -108,3 +108,79 @@ def check_download():
108108
self._commit_library_changes(library["id"])
109109
check_sjson()
110110
check_download()
111+
112+
113+
@skip_unless_cms
114+
class ContentLibrariesComponentVersionAssetTest(ContentLibrariesRestApiTest):
115+
"""
116+
Tests for the view that actually delivers the Library asset in Studio.
117+
"""
118+
119+
def setUp(self):
120+
super().setUp()
121+
122+
library = self._create_library(slug="asset-lib2", title="Static Assets Test Library")
123+
block = self._add_block_to_library(library["id"], "html", "html1")
124+
self._set_library_block_asset(block["id"], "static/test.svg", SVG_DATA)
125+
usage_key = UsageKey.from_string(block["id"])
126+
self.component = get_component_from_usage_key(usage_key)
127+
self.draft_component_version = self.component.versioning.draft
128+
129+
def test_good_responses(self):
130+
get_response = self.client.get(
131+
f"/library_assets/{self.draft_component_version.uuid}/static/test.svg"
132+
)
133+
assert get_response.status_code == 200
134+
content = b''.join(chunk for chunk in get_response.streaming_content)
135+
assert content == SVG_DATA
136+
137+
good_head_response = self.client.head(
138+
f"/library_assets/{self.draft_component_version.uuid}/static/test.svg"
139+
)
140+
assert good_head_response.headers == get_response.headers
141+
142+
def test_missing(self):
143+
"""Test asset requests that should 404."""
144+
# Non-existent version...
145+
wrong_version_uuid = UUID('11111111-1111-1111-1111-111111111111')
146+
response = self.client.get(
147+
f"/library_assets/{wrong_version_uuid}/static/test.svg"
148+
)
149+
assert response.status_code == 404
150+
151+
# Non-existent file...
152+
response = self.client.get(
153+
f"/library_assets/{self.draft_component_version.uuid}/static/missing.svg"
154+
)
155+
assert response.status_code == 404
156+
157+
# File-like ComponenVersionContent entry that isn't an actually
158+
# downloadable file...
159+
response = self.client.get(
160+
f"/library_assets/{self.draft_component_version.uuid}/block.xml"
161+
)
162+
assert response.status_code == 404
163+
164+
def test_anonymous_user(self):
165+
"""Anonymous users shouldn't get access to library assets."""
166+
self.client.logout()
167+
response = self.client.get(
168+
f"/library_assets/{self.draft_component_version.uuid}/static/test.svg"
169+
)
170+
assert response.status_code == 403
171+
172+
def test_unauthorized_user(self):
173+
"""User who is not a Content Library staff should not have access."""
174+
self.client.logout()
175+
student = UserFactory.create(
176+
username="student",
177+
email="student@example.com",
178+
password="student-pass",
179+
is_staff=False,
180+
is_superuser=False,
181+
)
182+
self.client.login(username="student", password="student-pass")
183+
get_response = self.client.get(
184+
f"/library_assets/{self.draft_component_version.uuid}/static/test.svg"
185+
)
186+
assert get_response.status_code == 403

0 commit comments

Comments
 (0)