From d833c7826a4c542212127a972a13e3fc91768070 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Tue, 21 May 2024 14:45:17 -0400 Subject: [PATCH] feat: Authoring API support for component assets This adds a number of API calls to the Authoring public API in order to support associating static assets with Components and allowing them to be downloaded. Added: * get_component_by_uuid * get_component_version_by_uuid * get_content_info_headers * get_redirect_response_for_component_asset Modified: * create_next_component_version - made title optional * create_component_version_content - annotation for learner_downloadable Most of the justification for this approach can be found in the docstring for get_redirect_response_for_component_asset. Note that there is currently no backend redis/memcached caching of the ComponentVersion and Content information. This means that every time a request is made, we will do a couple of database queries in order to do this lookup. I had actually done a version with such caching in an earlier iteration of this PR, but it added a lot of complexity for what I thought would be minimal gain, since going through the middleware will cause about 20 database queries anyway. This implements the first part of the static assets ADR for Learning Core: https://github.com/openedx/openedx-learning/pull/110 Important notes: * No view or auth is implemented. That is the responsibility of edx-platform. Learning Core only provides the storage and response generation. * The responses generated will require the use of a reverse proxy that can handle the X-Accel-Redirect header. See ADR for details. --- .../apps/authoring/components/api.py | 202 +++++++++++++++++- .../components/management/__init__.py | 0 .../management/commands/__init__.py | 0 .../commands/add_assets_to_component.py | 111 ++++++++++ .../apps/authoring/contents/api.py | 69 ++++++ .../apps/authoring/contents/models.py | 24 ++- .../apps/authoring/publishing/model_mixins.py | 10 + .../apps/authoring/components/test_assets.py | 191 +++++++++++++++++ .../apps/authoring/components/test_models.py | 3 + 9 files changed, 604 insertions(+), 6 deletions(-) create mode 100644 openedx_learning/apps/authoring/components/management/__init__.py create mode 100644 openedx_learning/apps/authoring/components/management/commands/__init__.py create mode 100644 openedx_learning/apps/authoring/components/management/commands/add_assets_to_component.py create mode 100644 tests/openedx_learning/apps/authoring/components/test_assets.py diff --git a/openedx_learning/apps/authoring/components/api.py b/openedx_learning/apps/authoring/components/api.py index d64b507b..3fd37a56 100644 --- a/openedx_learning/apps/authoring/components/api.py +++ b/openedx_learning/apps/authoring/components/api.py @@ -13,11 +13,16 @@ from __future__ import annotations from datetime import datetime +from enum import StrEnum, auto +from logging import getLogger from pathlib import Path +from uuid import UUID from django.db.models import Q, QuerySet from django.db.transaction import atomic +from django.http.response import HttpResponse, HttpResponseNotFound +from ..contents import api as contents_api from ..publishing import api as publishing_api from .models import Component, ComponentType, ComponentVersion, ComponentVersionContent @@ -34,12 +39,20 @@ "create_component_and_version", "get_component", "get_component_by_key", + "get_component_by_uuid", + "get_component_version_by_uuid", "component_exists_by_key", "get_components", "create_component_version_content", + "look_up_component_version_content", + "AssetError", + "get_redirect_response_for_component_asset", ] +logger = getLogger() + + def get_or_create_component_type(namespace: str, name: str) -> ComponentType: """ Get the ID of a ComponentType, and create if missing. @@ -112,9 +125,9 @@ def create_component_version( def create_next_component_version( component_pk: int, /, - title: str, content_to_replace: dict[str, int | None], created: datetime, + title: str | None = None, created_by: int | None = None, ) -> ComponentVersion: """ @@ -150,8 +163,11 @@ def create_next_component_version( last_version = component.versioning.latest if last_version is None: next_version_num = 1 + title = title or "" else: next_version_num = last_version.version_num + 1 + if title is None: + title = last_version.title with atomic(): publishable_entity_version = publishing_api.create_publishable_entity_version( @@ -247,6 +263,14 @@ def get_component_by_key( ) +def get_component_by_uuid(uuid: UUID) -> Component: + return Component.with_publishing_relations.get(publishable_entity__uuid=uuid) + + +def get_component_version_by_uuid(uuid: UUID) -> ComponentVersion: + return ComponentVersion.objects.get(publishable_entity_version__uuid=uuid) + + def component_exists_by_key( learning_package_id: int, /, @@ -351,7 +375,7 @@ def create_component_version_content( content_id: int, /, key: str, - learner_downloadable=False, + learner_downloadable: bool = False, ) -> ComponentVersionContent: """ Add a Content to the given ComponentVersion @@ -363,3 +387,177 @@ def create_component_version_content( learner_downloadable=learner_downloadable, ) return cvrc + + +class AssetError(StrEnum): + """Error codes related to fetching ComponentVersion assets.""" + ASSET_PATH_NOT_FOUND_FOR_COMPONENT_VERSION = auto() + ASSET_NOT_LEARNER_DOWNLOADABLE = auto() + ASSET_HAS_NO_DOWNLOAD_FILE = auto() + + +def _get_component_version_info_headers(component_version: ComponentVersion) -> dict[str, str]: + """ + These are the headers we can derive based on a valid ComponentVersion. + + These headers are intended to ease development and debugging, by showing + where this static asset is coming from. These headers will work even if + the asset path does not exist for this particular ComponentVersion. + """ + component = component_version.component + learning_package = component.learning_package + return { + # Component + "X-Open-edX-Component-Key": component.publishable_entity.key, + "X-Open-edX-Component-Uuid": component.uuid, + # Component Version + "X-Open-edX-Component-Version-Uuid": component_version.uuid, + "X-Open-edX-Component-Version-Num": component_version.version_num, + # Learning Package + "X-Open-edX-Learning-Package-Key": learning_package.key, + "X-Open-edX-Learning-Package-Uuid": learning_package.uuid, + } + + +def get_redirect_response_for_component_asset( + component_version_uuid: UUID, + asset_path: Path, + public: bool = False, + learner_downloadable_only: bool = True, +) -> HttpResponse: + """ + ``HttpResponse`` for a reverse-proxy to serve a ``ComponentVersion`` asset. + + :param component_version_uuid: ``UUID`` of the ``ComponentVersion`` that the + asset is part of. + + :param asset_path: Path to the asset being requested. + + :param public: Is this asset going to be made available without auth checks? + If ``True``, this will return an ``HttpResponse`` that can be cached in + a CDN and shared across many clients. + + :param learner_downloadable_only: Only return assets that are meant to be + downloadable by Learners, i.e. in the LMS experience. If this is + ``True``, then requests for assets that are not meant for student + download will return a ``404`` error response. + + **Response Codes** + + If the asset exists for this ``ComponentVersion``, this function will return + an ``HttpResponse`` with a status code of ``200``. + + If the specified asset does not exist for this ``ComponentVersion``, or if + the ``ComponentVersion`` itself does not exist, the response code will be + ``404``. + + Other than checking the coarse-grained ``learner_downloadable_only`` flag, + *this function does not do auth checking of any sort*–it will never return + a ``401`` or ``403`` response code. That is by design. Figuring out who is + making the request and whether they have permission to do so is the + responsiblity of whatever is calling this function. The + ``learner_downloadable_only`` flag is intended to be a filter for the entire + view. When it's True, not even staff can download component-internal assets. + This is intended to protect us from accidentally allowing sensitive grading + code to get leaked out. + + **Metadata Headers** + + The ``HttpResponse`` returned by this function will have headers describing + the asset and the ``ComponentVersion`` it belongs to (if it exists): + + * ``Content-Type`` + * ``Etag`` (this will be the asset's hash digest) + * ``X-Open-edX-Component-Key`` + * ``X-Open-edX-Component-Uuid`` + * ``X-Open-edX-Component-Version-Uuid`` + * ``X-Open-edX-Component-Version-Num`` + * ``X-Open-edX-Learning-Package-Key`` + * ``X-Open-edX-Learning-Package-Uuid`` + + **Asset Redirection** + + For performance reasons, the ``HttpResponse`` object returned by this + function does not contain the actual content data of the asset. It requires + an appropriately configured reverse proxy server that handles the + ``X-Accel-Redirect`` header (both Caddy and Nginx support this). + + .. warning:: + If you add any headers here, you may need to add them in the "media" + service container's reverse proxy configuration. In Tutor, this is a + Caddyfile. All non-standard HTTP headers should be prefixed with + ``X-Open-edX-``. + """ + # Helper to generate error header messages. + def _error_header(error: AssetError) -> dict[str, str]: + return {"X-Open-edX-Error": str(error)} + + # Check: Does the ComponentVersion exist? + try: + component_version = get_component_version_by_uuid(component_version_uuid) + except ComponentVersion.DoesNotExist: + # No need to add headers here, because no ComponentVersion was found. + logger.error(f"Asset Not Found: No ComponentVersion with UUID {component_version_uuid}") + return HttpResponseNotFound() + + # At this point we know that the ComponentVersion exists, so we can build + # those headers... + info_headers = _get_component_version_info_headers(component_version) + + # Check: Does the ComponentVersion have the requested asset (Content)? + try: + cv_content = component_version.componentversioncontent_set.get(key=asset_path) + except ComponentVersionContent.DoesNotExist: + logger.error(f"ComponentVersion {component_version_uuid} has no asset {asset_path}") + info_headers.update( + _error_header(AssetError.ASSET_PATH_NOT_FOUND_FOR_COMPONENT_VERSION) + ) + return HttpResponseNotFound(headers=info_headers) + + # Check: Does the Content have a downloadable file, instead of just inline + # text? It's easy for us to grab this content and stream it to the user + # anyway, but we're explicitly not doing so because streaming large text + # fields from the database is less scalable, and we don't want to encourage + # that usage pattern. + content = cv_content.content + if not content.has_file: + logger.error( + f"ComponentVersion {component_version_uuid} has asset {asset_path}, " + "but it is not downloadable (has_file=False)." + ) + info_headers.update( + _error_header(AssetError.ASSET_HAS_NO_DOWNLOAD_FILE) + ) + return HttpResponseNotFound(headers=info_headers) + + # Check: If we're asking only for Learner Downloadable assets, and the asset + # in question is not supposed to be downloadable by learners, then we give a + # 404 error. Even staff members are not expected to be able to download + # these assets via the LMS endpoint that serves students. Studio would be + # expected to have an entirely different view to serve these assets in that + # context (along with different timeouts, auth, and cache settings). So in + # that sense, the asset doesn't exist for that particular endpoint. + if learner_downloadable_only and (not cv_content.learner_downloadable): + logger.error( + f"ComponentVersion {component_version_uuid} has asset {asset_path}, " + "but it is not meant to be downloadable by learners " + "(ComponentVersionContent.learner_downloadable=False)." + ) + info_headers.update( + _error_header(AssetError.ASSET_NOT_LEARNER_DOWNLOADABLE) + ) + return HttpResponseNotFound(headers=info_headers) + + # At this point, we know that there is valid Content that we want to send. + # This adds Content-level headers, like the hash/etag and content type. + info_headers.update(contents_api.get_content_info_headers(content)) + stored_file_path = content.file_path() + + # Recompute redirect headers (reminder: this should never be cached). + redirect_headers = contents_api.get_redirect_headers(stored_file_path, public) + logger.info( + "Asset redirect (uncached metadata): " + f"{component_version_uuid}/{asset_path} -> {redirect_headers}" + ) + + return HttpResponse(headers={**info_headers, **redirect_headers}) diff --git a/openedx_learning/apps/authoring/components/management/__init__.py b/openedx_learning/apps/authoring/components/management/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/openedx_learning/apps/authoring/components/management/commands/__init__.py b/openedx_learning/apps/authoring/components/management/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/openedx_learning/apps/authoring/components/management/commands/add_assets_to_component.py b/openedx_learning/apps/authoring/components/management/commands/add_assets_to_component.py new file mode 100644 index 00000000..3b37eda1 --- /dev/null +++ b/openedx_learning/apps/authoring/components/management/commands/add_assets_to_component.py @@ -0,0 +1,111 @@ +""" +Management command to add files to a Component. + +This is mostly meant to be a debugging tool to let us to easily load some test +asset data into the system. +""" +import mimetypes +import pathlib +from datetime import datetime, timezone + +from django.core.management.base import BaseCommand + +from ....components.api import create_component_version_content +from ....contents.api import get_or_create_file_content, get_or_create_media_type +from ....publishing.api import get_learning_package_by_key +from ...api import create_next_component_version, get_component_by_key + + +class Command(BaseCommand): + """ + Add files to a Component, creating a new Component Version. + + This does not publish the the Component. + + Note: This is a quick debug tool meant to stuff some asset data into + Learning Core models for testing. It's not intended as a robust and + performant tool for modifying actual production content, and should not be + used for that purpose. + """ + + def add_arguments(self, parser): + parser.add_argument( + "learning_package_key", + type=str, + help="LearningPackage.key value for where the Component is located." + ) + parser.add_argument( + "component_key", + type=str, + help="Component.key that you want to add assets to." + ) + parser.add_argument( + "file_mappings", + nargs="+", + type=str, + help=( + "Mappings of desired Component asset paths to the disk paths " + "of where to upload the file from, separated by ':'. (Example: " + "static/donkey.jpg:/Users/dave/Desktop/donkey-big.jpg). A " + "blank value for upload file means to remove that from the " + "Component. You may upload/remove as many files as you want in " + "a single invocation." + ) + ) + + def handle(self, *args, **options): + """ + Add files to a Component as ComponentVersion -> Content associations. + """ + learning_package_key = options["learning_package_key"] + component_key = options["component_key"] + file_mappings = options["file_mappings"] + + learning_package = get_learning_package_by_key(learning_package_key) + # Parse something like: "xblock.v1:problem:area_of_circle_1" + namespace, type_name, local_key = component_key.split(":", 2) + component = get_component_by_key( + learning_package.id, namespace, type_name, local_key + ) + + created = datetime.now(tz=timezone.utc) + keys_to_remove = set() + local_keys_to_content = {} + + for file_mapping in file_mappings: + local_key, file_path = file_mapping.split(":", 1) + + # No file_path means to delete this entry from the next version. + if not file_path: + keys_to_remove.add(local_key) + continue + + media_type_str, _encoding = mimetypes.guess_type(file_path) + media_type = get_or_create_media_type(media_type_str) + content = get_or_create_file_content( + learning_package.id, + media_type.id, + data=pathlib.Path(file_path).read_bytes(), + created=created, + ) + local_keys_to_content[local_key] = content.id + + next_version = create_next_component_version( + component.pk, + content_to_replace={local_key: None for local_key in keys_to_remove}, + created=created, + ) + for local_key, content_id in sorted(local_keys_to_content.items()): + create_component_version_content( + next_version.pk, + content_id, + key=local_key, + learner_downloadable=True, + ) + + self.stdout.write( + f"Created v{next_version.version_num} of " + f"{next_version.component.key} ({next_version.uuid}):" + ) + for cvc in next_version.componentversioncontent_set.all(): + self.stdout.write(f"- {cvc.key} ({cvc.uuid})") diff --git a/openedx_learning/apps/authoring/contents/api.py b/openedx_learning/apps/authoring/contents/api.py index 872ebad9..31993588 100644 --- a/openedx_learning/apps/authoring/contents/api.py +++ b/openedx_learning/apps/authoring/contents/api.py @@ -7,6 +7,7 @@ from __future__ import annotations from datetime import datetime +from logging import getLogger from django.core.files.base import ContentFile from django.db.transaction import atomic @@ -22,11 +23,15 @@ __all__ = [ "get_or_create_media_type", "get_content", + "get_content_info_headers", "get_or_create_text_content", "get_or_create_file_content", ] +log = getLogger() + + def get_or_create_media_type(mime_type: str) -> MediaType: """ Return the MediaType.id for the desired mime_type string. @@ -168,3 +173,67 @@ def get_or_create_file_content( content.write_file(ContentFile(data)) return content + + +def get_content_info_headers(content: Content) -> dict[str, str]: + """ + Return HTTP headers that are specific to this Content. + + This currently only consists of the Content-Type and ETag. These values are + safe to cache. + """ + return { + "Content-Type": str(content.media_type), + "Etag": content.hash_digest, + } + + +def get_redirect_headers( + stored_file_path: str, + public: bool = False, + max_age: int | None = None, +) -> dict[str, str]: + """ + Return a dict of headers for file redirect and caching. + + This is a separate function from get_content_info_headers because the URLs + returned in these headers produced by this function should never be put into + the backend Django cache (redis/memcached). The `stored_file_path` location + *is* cacheable though–that's the actual storage location for the resource, + and not a link that could potentially expire. + + TODO: We need to add support for short-lived URL generation from the + stored_file_path. + """ + if public: + # If an asset is public, then let it be cached by the reverse-proxy and + # CDN, but do require that it be revalidated after the suggested max + # age. This would help us do things like take a URL that was mistakenly + # made public and make it require authentication. Fortunately, checking + # that the content is up to date is a cheap operation, since it just + # requires examining the Etag. + cache_directive = "must-revalidate" + + # Default to an hour of caching, to make it easier to tighten access + # later on. + max_age = max_age or (5 * 60) + else: + # If an asset is meant to be private, that means this response should + # not be cached by either the reverse-proxy or any CDN–it's only ever + # cached on the user's browser. This is what you'd use for very granular + # permissions checking, e.g. "only let them see this image if they have + # access to the Component it's associated with". Note that we're not + # doing ``Vary: Cookie`` because that would fill the reverse-proxy and + # CDN caches with a lot of redundant entries. + cache_directive = "private" + + # This only stays on the user's browser, so cache for a whole day. This + # is okay to do because Content data is typically immutable–i.e. if an + # asset actually changes, the user should be directed to a different URL + # for it. + max_age = max_age or (60 * 60 * 24) + + return { + "Cache-Control": f"max-age={max_age}, {cache_directive}", + "X-Accel-Redirect": stored_file_path, + } diff --git a/openedx_learning/apps/authoring/contents/models.py b/openedx_learning/apps/authoring/contents/models.py index c6de8bb0..7e4ff8cc 100644 --- a/openedx_learning/apps/authoring/contents/models.py +++ b/openedx_learning/apps/authoring/contents/models.py @@ -5,7 +5,7 @@ """ from __future__ import annotations -from functools import cached_property +from functools import cache, cached_property from django.core.exceptions import ValidationError from django.core.files.base import File @@ -23,6 +23,7 @@ ] +@cache def get_storage() -> Storage: """ Return the Storage instance for our Content file persistence. @@ -236,6 +237,10 @@ class Content(models.Model): hash_digest = hash_field() # Do we have file data stored for this Content in our file storage backend? + # We use has_file instead of a FileField because it's more space efficient. + # The location of a Content's file data is derivable from the Learning + # Package's UUID and the hash of the Content. There's no need to waste that + # space to encode it in every row. has_file = models.BooleanField() # The ``text`` field contains the text representation of the Content, if @@ -288,6 +293,8 @@ def file_path(self): def write_file(self, file: File) -> None: """ Write file contents to the file storage backend. + + This function does nothing if the file already exists. """ storage = get_storage() file_path = self.file_path() @@ -303,14 +310,19 @@ def write_file(self, file: File) -> None: # be two logically separate Content entries if they are different file # types. This lets other models add data to Content via 1:1 relations by # ContentType (e.g. all SRT files). This is definitely an edge case. - if not storage.exists(file_path): - storage.save(file_path, file) + # + # 3. Similar to (2), but only part of the file was written before an + # error occurred. This seems unlikely, but possible if the underlying + # storage engine writes in chunks. + if storage.exists(file_path) and storage.size(file_path) == file.size: + return + storage.save(file_path, file) def file_url(self) -> str: """ This will sometimes be a time-limited signed URL. """ - return get_storage().url(self.file_path()) + return content_file_url(self.file_path()) def clean(self): """ @@ -349,3 +361,7 @@ class Meta: ] verbose_name = "Content" verbose_name_plural = "Contents" + + +def content_file_url(file_path): + return get_storage().url(file_path) diff --git a/openedx_learning/apps/authoring/publishing/model_mixins.py b/openedx_learning/apps/authoring/publishing/model_mixins.py index c62d43a3..85ef6c96 100644 --- a/openedx_learning/apps/authoring/publishing/model_mixins.py +++ b/openedx_learning/apps/authoring/publishing/model_mixins.py @@ -272,6 +272,16 @@ def versions(self): publishable_entity_version__entity_id=pub_ent.id ) + def version_num(self, version_num): + """ + Return a specific numbered version model. + """ + pub_ent = self.content_obj.publishable_entity + return self.content_version_model_cls.objects.get( + publishable_entity_version__entity_id=pub_ent.id, + publishable_entity_version__version_num=version_num, + ) + class PublishableEntityVersionMixin(models.Model): """ diff --git a/tests/openedx_learning/apps/authoring/components/test_assets.py b/tests/openedx_learning/apps/authoring/components/test_assets.py new file mode 100644 index 00000000..6d631738 --- /dev/null +++ b/tests/openedx_learning/apps/authoring/components/test_assets.py @@ -0,0 +1,191 @@ +""" +Tests relating to serving static assets. +""" +from datetime import datetime, timezone +from pathlib import Path +from uuid import uuid4 + +from openedx_learning.apps.authoring.components import api as components_api +from openedx_learning.apps.authoring.components.api import AssetError +from openedx_learning.apps.authoring.contents import api as contents_api +from openedx_learning.apps.authoring.publishing import api as publishing_api +from openedx_learning.apps.authoring.publishing.models import LearningPackage +from openedx_learning.lib.test_utils import TestCase + + +class AssetTestCase(TestCase): + """ + Test serving static assets (Content files, via Component lookup). + """ + python_source_media_type: contents_api.MediaType + problem_block_media_type: contents_api.MediaType + html_media_type: contents_api.MediaType + + problem_type: components_api.ComponentType + component: components_api.Component + component_version: components_api.ComponentVersion + + problem_content: contents_api.Content + python_source_asset: contents_api.Content + html_asset_content: contents_api.Content + + learning_package: LearningPackage + now: datetime + + @classmethod + def setUpTestData(cls) -> None: + """ + Create all the Content and Components we need. + + The individual tests are read-only. + """ + cls.now = datetime(2024, 8, 24, tzinfo=timezone.utc) + cls.problem_type = components_api.get_or_create_component_type( + "xblock.v1", "problem" + ) + cls.python_source_media_type = contents_api.get_or_create_media_type( + "text/x-python", + ) + cls.problem_block_media_type = contents_api.get_or_create_media_type( + "application/vnd.openedx.xblock.v1.problem+xml", + ) + cls.html_media_type = contents_api.get_or_create_media_type("text/html") + + cls.learning_package = publishing_api.create_learning_package( + key="ComponentTestCase-test-key", + title="Components Test Case Learning Package", + ) + cls.component, cls.component_version = components_api.create_component_and_version( + cls.learning_package.id, + component_type=cls.problem_type, + local_key="my_problem", + title="My Problem", + created=cls.now, + created_by=None, + ) + + # ProblemBlock content that is stored as text Content, not a file. + cls.problem_content = contents_api.get_or_create_text_content( + cls.learning_package.id, + cls.problem_block_media_type.id, + text="(pretend problem OLX is here)", + created=cls.now, + ) + components_api.create_component_version_content( + cls.component_version.pk, + cls.problem_content.id, + key="block.xml", + learner_downloadable=False, + ) + + # Python source file, stored as a file. This is hypothetical, as we + # don't actually support bundling grader files like this today. + cls.python_source_asset = contents_api.get_or_create_file_content( + cls.learning_package.id, + cls.python_source_media_type.id, + data=b"print('hello world!')", + created=cls.now, + ) + components_api.create_component_version_content( + cls.component_version.pk, + cls.python_source_asset.id, + key="src/grader.py", + learner_downloadable=False, + ) + + # An HTML file that is student downloadable + cls.html_asset_content = contents_api.get_or_create_file_content( + cls.learning_package.id, + cls.html_media_type.id, + data=b"hello world!", + created=cls.now, + ) + components_api.create_component_version_content( + cls.component_version.pk, + cls.html_asset_content.id, + key="static/hello.html", + learner_downloadable=True, + ) + + def test_no_component_version(self): + """No ComponentVersion matching the UUID exists.""" + nonexistent_uuid = uuid4() + response = components_api.get_redirect_response_for_component_asset( + nonexistent_uuid, + Path("static/foo.png"), + ) + assert response.status_code == 404 + + # None of the Learning Core headers should be set... + for header_name in response.headers: + assert not header_name.startswith("X-Open-edX") + + def _assert_has_component_version_headers(self, headers): + """ + Helper to verify common headers expected of successful requests. + + Note: The request header values in an HttpResponse will all have been + serialized to strings. + """ + assert headers["X-Open-edX-Component-Key"] == self.component.key + assert headers["X-Open-edX-Component-Uuid"] == str(self.component.uuid) + assert headers["X-Open-edX-Component-Version-Uuid"] == str(self.component_version.uuid) + assert headers["X-Open-edX-Component-Version-Num"] == str(self.component_version.version_num) + assert headers["X-Open-edX-Learning-Package-Key"] == self.learning_package.key + assert headers["X-Open-edX-Learning-Package-Uuid"] == str(self.learning_package.uuid) + + def test_404s_with_component_version_info(self): + """Test 404 errors in various failure scenarios...""" + # These are all scenarios where the ComponentVersion exists, but the + # request returns a 404 Not Found error for different reasons: + paths_to_errors = { + # Asset doesn't exist for this ComponentVersion at all + "static/this-doesnt-exist.txt": AssetError.ASSET_PATH_NOT_FOUND_FOR_COMPONENT_VERSION, + + # This is testing that asset paths are case sensitive + "static/HELLO.html": AssetError.ASSET_PATH_NOT_FOUND_FOR_COMPONENT_VERSION, + + # Files that want to guarantee can never be downloaded (they're for + # backend usage only). + "src/grader.py": AssetError.ASSET_NOT_LEARNER_DOWNLOADABLE, + + # Text stored in the database directly instead of file storage. + "block.xml": AssetError.ASSET_HAS_NO_DOWNLOAD_FILE, + } + for asset_path, expected_error in paths_to_errors.items(): + response = components_api.get_redirect_response_for_component_asset( + self.component_version.uuid, + Path(asset_path), + ) + self._assert_has_component_version_headers(response.headers) + assert response.status_code == 404 + assert response.headers["X-Open-edX-Error"] == str(expected_error) + + def _assert_html_content_headers(self, response): + """Assert expected HttpResponse headers for a downloadable HTML file.""" + self._assert_has_component_version_headers(response.headers) + assert response.status_code == 200 + assert response.headers["Etag"] == self.html_asset_content.hash_digest + assert response.headers["Content-Type"] == "text/html" + assert response.headers["X-Accel-Redirect"] == self.html_asset_content.file_path() + assert "X-Open-edX-Error" not in response.headers + + def test_public_asset_response(self): + """Test an asset intended to be publicly available without auth.""" + response = components_api.get_redirect_response_for_component_asset( + self.component_version.uuid, + Path("static/hello.html"), + public=True, + ) + self._assert_html_content_headers(response) + assert "must-revalidate" in response.headers["Cache-Control"] + + def test_private_asset_response(self): + """Test an asset intended to require auth checks.""" + response = components_api.get_redirect_response_for_component_asset( + self.component_version.uuid, + Path("static/hello.html"), + public=False, + ) + self._assert_html_content_headers(response) + assert "private" in response.headers["Cache-Control"] diff --git a/tests/openedx_learning/apps/authoring/components/test_models.py b/tests/openedx_learning/apps/authoring/components/test_models.py index 99b43daf..8ac3ac43 100644 --- a/tests/openedx_learning/apps/authoring/components/test_models.py +++ b/tests/openedx_learning/apps/authoring/components/test_models.py @@ -61,6 +61,9 @@ def test_latest_version(self) -> None: # Grabbing the list of versions for this component assert list(component.versioning.versions) == [component_version] + # Grab a specific version by number + assert component.versioning.version_num(1) == component_version + def test_last_publish_log(self): """ Test last_publish_log versioning property for published Components.