diff --git a/.importlinter b/.importlinter index 13bbad04c..41b826753 100644 --- a/.importlinter +++ b/.importlinter @@ -58,6 +58,10 @@ layers= # The "backup_restore" applet handles the new export and import mechanism. openedx_content.applets.backup_restore + # The "courses" applet maps course runs to the Learning Packages that hold + # their content. It sits above the content applets so that it may use any of them. + openedx_content.applets.courses + # The "components" applet is responsible for storing versioned Components, # which in Open edX Studio terminology maps to things like individual # Problems, Videos, and blocks of HTML text. This is also the type we would @@ -65,7 +69,8 @@ layers= # has no child elements. # The "containers" app is built on top of publishing, and is a peer to # "components" but they do not depend on each other. - openedx_content.applets.components | openedx_content.applets.containers + # The "assets" app is a peer to "components". + openedx_content.applets.components | openedx_content.applets.containers | openedx_content.applets.assets # The "media" applet stores the simplest pieces of binary and text data, # without versioning information. These belong to a single Learning Package. diff --git a/docs/openedx_content/decisions/0011-do-not-implement-copy-on-write.rst b/docs/openedx_content/decisions/0011-do-not-implement-copy-on-write.rst index 0668414ee..710ae2319 100644 --- a/docs/openedx_content/decisions/0011-do-not-implement-copy-on-write.rst +++ b/docs/openedx_content/decisions/0011-do-not-implement-copy-on-write.rst @@ -6,7 +6,7 @@ Status ------ -Proposed. +Accepted. Context ------- diff --git a/docs/openedx_content/decisions/0012-course-learning-packages.rst b/docs/openedx_content/decisions/0012-course-learning-packages.rst index 554b4f8a9..7ab50afca 100644 --- a/docs/openedx_content/decisions/0012-course-learning-packages.rst +++ b/docs/openedx_content/decisions/0012-course-learning-packages.rst @@ -6,7 +6,7 @@ Status ------ -Proposed. +Accepted. Context ------- diff --git a/docs/openedx_content/decisions/0013-shared-static-assets.rst b/docs/openedx_content/decisions/0013-shared-static-assets.rst index a296a994b..a630e8301 100644 --- a/docs/openedx_content/decisions/0013-shared-static-assets.rst +++ b/docs/openedx_content/decisions/0013-shared-static-assets.rst @@ -6,7 +6,7 @@ Status ------ -Proposed. +Accepted. Context ------- @@ -34,14 +34,14 @@ Because Assets cannot hold more than one file, when several course files have so * By attaching all the related files to the same XBlock Component; or * By organizing the related Assets (one file per Asset) into a ``Collection`` -2. Assets are uniquely keyed by filename -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +2. Assets are uniquely keyed by filename (path) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Each Asset holds only a single file, e.g. ``solar-system.svg``, and within a given learning package (i.e. within a given course or library), each Asset's filename must be unique. (This is in contrast to asset files attached to XBlock Components, which allow multiple asset files per Component.) The filename is stored directly as a column on the ``Asset`` model, not on ``AssetVersion`` nor the ``Media`` row that any given ``AssetVersion`` points to. +Each Asset holds only a single file, e.g. ``solar-system.svg``, and within a given learning package (i.e. within a given course or library), each Asset's filename must be unique. (This is in contrast to asset files attached to XBlock Components, which allow multiple asset files per Component.) The filename is stored directly as a ``path`` column on the ``Asset`` model, not on ``AssetVersion`` nor the ``Media`` row that any given ``AssetVersion`` points to. -Filenames should be case-sensitive, such that ``fig1.png`` and ``Fig1.png`` are both allowed. Since MySQL's default collation is case-insensitive, the ``filename`` column must explicitly use a case-sensitive collation (e.g. using ``case_sensitive_char_field``, as ``ref_field`` does), including for its uniqueness constraint. For backwards compatibility, the filename is also allowed to contain ``/``, such as ``images/image1.png``. By convention, an Asset's ``entity_ref`` is ``openedx-asset:{filename}``, but nothing (including asset URLs) should rely on that; look Assets up by ``filename`` using its explicit column. +Filenames should be case-sensitive, such that ``fig1.png`` and ``Fig1.png`` are both allowed. Since MySQL's default collation is case-insensitive, the ``path`` column which stores it must explicitly use a case-sensitive collation (e.g. using ``case_sensitive_char_field``, as ``ref_field`` does), including for its uniqueness constraint. For backwards compatibility, the filename/path is also allowed to contain ``/``, such as ``images/image1.png``. By convention, an Asset's ``entity_ref`` is ``openedx-asset:{path}``, but nothing (including asset URLs) should rely on that; look Assets up by ``path`` using its explicit column. -If a file is uploaded with the same filename as an Asset that has been (soft) deleted, the deleted Asset is un-deleted: the upload becomes a new draft version of the existing Asset, rather than creating a new Asset. Its version history and any existing references to it are retained. If a specific permissions mode (``locked`` or ``private``, and separately ``allow_scripts`` or not) was specified when uploading, it will have that value; otherwise it will take on the most restrictive values between the defaults and the previous values (i.e. if the previously-deleted asset was locked, the new asset will be locked; the ``allow_scripts`` flag will be reset to ``False``). +If a file is uploaded with the same filename/path as an Asset that has been (soft) deleted, the deleted Asset is un-deleted: the upload becomes a new draft version of the existing Asset, rather than creating a new Asset. Its version history and any existing references to it are retained. If a specific permissions mode (``locked`` or ``staff``, and separately ``allow_scripts`` or not) was specified when uploading, it will have that value; otherwise it will take on the most restrictive values between the defaults and the previous values (i.e. if the previously-deleted asset was locked, the new asset will be locked; the ``allow_scripts`` flag will be reset to ``False``). 3. Assets support relative links ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -104,7 +104,7 @@ When migrating assets from MongoDB/contentstore to ``openedx_content``, each ass * **Thumbnails** generated by contentstore are dropped and not migrated. Thumbnails will be handled separately, as described in an upcoming ADR. * **Video transcripts** stored in contentstore (e.g. ``subs_*.srt.sjson``) become Assets like any other file. Video blocks reference transcripts by name rather than via ``/static/`` URLs, and during this migration they are still in modulestore with no :class:`Component`, so no dependency tracking is set up for transcript Assets at this stage. Dependencies between video components and their transcript Assets can be added when course content itself is migrated to ``openedx_content``. -* **python_lib.zip** automatically gets ``private`` mode (decision 9), so that it remains inaccessible to learners as it is today. +* **python_lib.zip** automatically gets ``staff`` mode (decision 9), so that it remains inaccessible to learners as it is today. * **HTML files** get ``allow_scripts=True`` (decision 10) if the course currently relies on them being rendered with scripts enabled, so that they keep working. Each course's Files are migrated in a single step, which also creates the run's learning package and ``CourseContent`` row (:ref:`openedx-content-adr-0012`, decision 3). A course is never in a mixed state: before the migration, all of its Files are in contentstore, and afterwards, contentstore is no longer consulted for that course. @@ -114,25 +114,25 @@ Each course's Files are migrated in a single step, which also creates the run's All Assets (in fact, all PublishableEntities) have a mutable ``title`` which can be used to store a human-readable name for the entity, such as "Illustration of the Moon's Orbit (SVG)". The title is versioned (it's defined by :class:`PublishableEntityVersion`), so changing the title creates a new version. -The title of an asset can be changed at any time, but the filename cannot be changed. (Changing the filename would break references/links to the file from the course content.) +The title of an asset can be changed at any time, but the filename/path cannot be changed. (Changing the filename would break references/links to the file from the course content.) 8. Assets are not part of the learner-facing outline ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Assets are not children of the course container or any of its descendants. Putting them there would mean every asset upload creates a new version of an outline container. -9. Asset permissions mode: "public", "locked", "private" -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +9. Asset permissions mode: "public", "locked", "staff" +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -The ``Asset`` model will have a permissions column with an enum that allows values of ``public`` (default), ``locked``, and ``private``. The permissions mode is on the unversioned model (not part of versioning), because often authors will wish to lock down all versions of an asset, not just lock the current version while still allowing access to previous versions. +The ``Asset`` model will have a permissions column with an enum that allows values of ``public`` (default), ``locked``, and ``staff``. The permissions mode is on the unversioned model (not part of versioning), because often authors will wish to lock down all versions of an asset, not just lock the current version while still allowing access to previous versions. * ``public`` is the default for now, as it was in contentstore. There are no permissions checks on such assets, and anyone who knows the filename/URL can access it. This was chosen as the default more for performance reasons (skipping expensive permissions checks) rather than product reasons, and we may change to ``locked`` as default in the future. * ``locked`` mirrors the existing contentstore flag for course files. It means that only enrolled users can access the given file. -* ``private`` means that only course staff can access the asset, e.g. for storing grading rubric PDFs. This is a new feature compared to contentstore, although it will be used to prevent learners from downloading ``python_lib.zip``, a special shared asset used by the problem XBlock that must not be accessible to learners (contentstore achieves this with a hard-coded rule). +* ``staff`` means that only course staff can access the asset, e.g. for storing grading rubric PDFs. This is a new feature compared to contentstore, although it will be used to prevent learners from downloading ``python_lib.zip``, a special shared asset used by the problem XBlock that must not be accessible to learners (contentstore achieves this with a hard-coded rule). In **libraries**, these permissions checks have little effect, but indicate what permissions an asset should have when copied into the course. -This information must be preserved round-trip on course exports/import. The attributes like ``locked``, ``private``, ``allow_scripts``, and ``title`` need to go into ``policies/assets.json``. Private shared assets other than ``python_lib.zip`` will have to be exported to a separate folder outside of ``static/`` and will be silently ignored if imported into an older version of Open edX that doesn't support private assets. +This information must be preserved round-trip on course exports/import. The attributes like ``locked``, ``staff``, ``allow_scripts``, and ``title`` need to go into ``policies/assets.json``. Staff-only shared assets other than ``python_lib.zip`` will have to be exported to a separate folder outside of ``static/`` and will be silently ignored if imported into an older version of Open edX that doesn't support private ``staff`` assets. 10. HTML Assets can be allowed to run scripts ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -156,9 +156,9 @@ Consequences **Replaced files are not deleted**, and old versions of the replaced file will still exist; this follows from the fact that the files are now versioned. -**Assets cannot be renamed.** Because the filename *is* the identifier, new files can be uploaded, but renaming an Asset would break any existing usages that refer to it by its filename identifier. +**Assets cannot be renamed.** Because the filename/path *is* the identifier, new files can be uploaded, but renaming an Asset would break any existing usages that refer to it by its filename identifier. -**There will be two different ways to use files in course content**: by attaching them directly to XBlock Components, or by uploading them as shared assets. Content libraries already support the former (attached to Components), and courses support the latter. Note that the ``locked`` and ``private`` modes will only be supported for shared assets, as any files attached to a ``Component`` that are meant to be accessible to learners will share the same permissions as the ``Component`` they're attached to. +**There will be two different ways to use files in course content**: by attaching them directly to XBlock Components, or by uploading them as shared assets. Content libraries already support the former (attached to Components), and courses support the latter. Note that the ``locked`` and ``staff`` modes will only be supported for shared assets, as any files attached to a ``Component`` that are meant to be accessible to learners will share the same permissions as the ``Component`` they're attached to. **Linking library components into courses will be simplified**, once we support XBlocks with files attached to their ``Component``, because part of the complexity in copying library components into courses involves analyzing their attached files and merging them into the course's shared Course Files. If we can instead just copy the ``Component``, including all its attached files, directly into the course's Learning Package, no analysis nor merging into shared files is necessary. diff --git a/docs/openedx_content/decisions/0014-referencing-assets.rst b/docs/openedx_content/decisions/0014-referencing-assets.rst index 8f5954b9e..78b9d2c7f 100644 --- a/docs/openedx_content/decisions/0014-referencing-assets.rst +++ b/docs/openedx_content/decisions/0014-referencing-assets.rst @@ -6,7 +6,7 @@ Status ------ -Proposed. +Accepted, but not implemented. Context ------- @@ -90,7 +90,7 @@ But, when an XBlock Component with references to shared assets is pasted/copied Each referenced Asset is handled as follows: 1. If the destination Learning Package already has an Asset that is a downstream copy of the same upstream Asset (because an author manually imported/copied it to the destination earlier), that existing copy is reused. -2. Otherwise, if the shared Asset is ``private`` or ``allow_scripts`` is True, an error occurs (private assets should not be implicitly copied, and component-local assets cannot be private nor allow scripts). +2. Otherwise, if the shared Asset is ``staff`` or ``allow_scripts`` is True, an error occurs (staff-only assets should not be implicitly copied, and component-local assets cannot be staff-only nor allow scripts). 3. Otherwise, the shared Asset gets converted to a component-local asset. (Note: this treats both ``locked`` and ``public`` assets the same, converting them to component-local and changing their permissions to match the destination component.) For example: In a library is a shared asset (an image, ``example.jpg``) and an HTML Component which references that image (``......``). When the HTML Component is copied from the library into a course (creating an upstream link), the ``example.jpg`` asset will be converted to a component-local asset attached to the HTML Component in the course, as ``/static/example.jpg``. Importantly, if the image asset (``example.jpg``) in the library is later edited and the change is published, the system must indicate that there is a newer version of the HTML Component available to sync to the course (even if the HTML component is unchanged, just because it has a dependency on the shared image asset). This will require updating the library-to-course sync code to account for ``dependencies_hash_digest`` and not just compare the ``version_num`` of each component. diff --git a/docs/openedx_content/decisions/0015-shadow-components.rst b/docs/openedx_content/decisions/0015-shadow-components.rst index fb8e2c2cd..3e5b80fb6 100644 --- a/docs/openedx_content/decisions/0015-shadow-components.rst +++ b/docs/openedx_content/decisions/0015-shadow-components.rst @@ -6,7 +6,7 @@ Status ------ -Proposed. +Accepted, but not implemented. Context ------- diff --git a/src/openedx_content/admin.py b/src/openedx_content/admin.py index ae8a88123..a5ac78bfc 100644 --- a/src/openedx_content/admin.py +++ b/src/openedx_content/admin.py @@ -3,9 +3,11 @@ """ # pylint: disable=wildcard-import +from .applets.assets.admin import * from .applets.backup_restore.admin import * from .applets.collections.admin import * from .applets.components.admin import * from .applets.containers.admin import * +from .applets.courses.admin import * from .applets.media.admin import * from .applets.publishing.admin import * diff --git a/src/openedx_content/api.py b/src/openedx_content/api.py index a0d7d0ab3..3bfb103ab 100644 --- a/src/openedx_content/api.py +++ b/src/openedx_content/api.py @@ -17,10 +17,12 @@ # `DraftChangeLogEventData` vs `DraftChangeLogRecord` is clearer if the former is `signals.DraftChangeLogEventData`) from . import signals # The rest of the public API (other than models): +from .applets.assets.api import * from .applets.backup_restore.api import * from .applets.collections.api import * from .applets.components.api import * from .applets.containers.api import * +from .applets.courses.api import * from .applets.media.api import * from .applets.publishing.api import * from .applets.sections.api import * diff --git a/src/openedx_content/applets/assets/__init__.py b/src/openedx_content/applets/assets/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/openedx_content/applets/assets/admin.py b/src/openedx_content/applets/assets/admin.py new file mode 100644 index 000000000..97c2ca0f9 --- /dev/null +++ b/src/openedx_content/applets/assets/admin.py @@ -0,0 +1,58 @@ +""" +Django admin for ``assets`` models +""" + +from django.contrib import admin +from django.template.defaultfilters import filesizeformat + +from openedx_django_lib.admin_utils import ReadOnlyModelAdmin + +from .models import Asset, AssetVersion + + +@admin.register(Asset) +class AssetAdmin(ReadOnlyModelAdmin): + """ + Django admin for Asset model + """ + + list_display = ("id", "path", "learning_package", "permissions", "allow_scripts", "created", "created_by") + list_filter = ("learning_package",) + search_fields = ("path",) + + def get_queryset(self, request): + queryset = super().get_queryset(request) + return queryset.select_related( + "learning_package", + "publishable_entity", + "publishable_entity__created_by", + ) + + +@admin.register(AssetVersion) +class AssetVersionAdmin(ReadOnlyModelAdmin): + """ + Django admin for AssetVersion model + """ + + list_display = ("asset", "version_num", "title", "media_type", "format_size", "created", "created_by") + list_filter = ("asset__learning_package",) + search_fields = ("asset__path", "publishable_entity_version__title") + + def get_queryset(self, request): + queryset = super().get_queryset(request) + return queryset.select_related( + "asset", + "publishable_entity_version", + "publishable_entity_version__created_by", + "media", + "media__media_type", + ) + + @admin.display(description="Media type") + def media_type(self, av_obj: AssetVersion) -> str: + return str(av_obj.media.media_type) + + @admin.display(description="Size") + def format_size(self, av_obj: AssetVersion) -> str: + return filesizeformat(av_obj.media.size) diff --git a/src/openedx_content/applets/assets/api.py b/src/openedx_content/applets/assets/api.py new file mode 100644 index 000000000..a25fc349f --- /dev/null +++ b/src/openedx_content/applets/assets/api.py @@ -0,0 +1,399 @@ +""" +Assets API, for assets shared by multiple entities in the Learning Package. + +Please look at the models.py file for more information about the kinds of data +stored in this app. +""" + +from __future__ import annotations + +import logging +import mimetypes +from datetime import UTC, datetime + +from django.core.exceptions import ValidationError +from django.db.models import Prefetch, QuerySet +from django.db.transaction import atomic +from django.db.utils import IntegrityError + +from ..media import api as media_api +from ..media.models import Media +from ..publishing import api as publishing_api +from ..publishing.models import LearningPackage, PublishableEntityVersion +from .models import INTERNAL_API, INVALID_PATH_MSG, PATH_MAX_LENGTH, PATH_RE, Asset, AssetPermissions, AssetVersion + +# The public API that will be re-exported by openedx_content.api +# is listed in the __all__ entries below. Internal helper functions that are +# private to this module should start with an underscore. If a function does not +# start with an underscore AND it is not in __all__, that function is considered +# to be callable only by other applets in the openedx_content package. +__all__ = [ + "AssetAlreadyExists", + "AssetPermissions", + "create_asset", + "create_asset_and_version", + "create_next_asset_version", + "get_asset", + "get_asset_by_path", + "get_assets", + "get_missing_asset_dependencies", + "update_asset", +] + + +log = logging.getLogger(__name__) + + +def _get_learning_package_id(learning_package: LearningPackage | LearningPackage.ID) -> LearningPackage.ID: + if isinstance(learning_package, LearningPackage): + return learning_package.id + assert isinstance(learning_package, int) + return learning_package + + +class AssetAlreadyExists(ValidationError): + """ + Exception raised when trying to create an asset with a path that already + exists in the given learning package. + """ + + path: str + + def __init__(self, path: str): + self.path = path + super().__init__(f'Asset with path "{path}" already exists.') + + +def create_asset( + learning_package: LearningPackage | LearningPackage.ID, + /, + path: str, + created: datetime, + created_by: int | None, + *, + permissions: AssetPermissions | None = None, + allow_scripts: bool = False, +) -> Asset: + """ + Create a new Asset (a shared file, like an image or a PDF document). + + permissions: Set permissions to restrict this asset to enrolled or staff + users. set to ``None`` to use the system default (currently ``PUBLIC`` + but may change in the future). + allow_scripts: for HTML assets only, set this ``True`` to serve the asset in + a more permissive sandbox that allows referencing JavaScript files. + """ + learning_package_id = _get_learning_package_id(learning_package) + entity_ref = f"openedx-asset:{path}" + if permissions is None: + permissions = AssetPermissions.PUBLIC + # Do some quick checks now in python. These are similar to what `Asset.validate_constraints()`` would run for us, + # but it wraps each check in a savepoint, so adds a lot of queries. The DB constraints remain as a backstop against + # things like `Asset.objects.update(...)`, which bypass all other validation. + if len(path) > PATH_MAX_LENGTH or not PATH_RE.fullmatch(path): + raise ValidationError(INVALID_PATH_MSG) + if permissions not in AssetPermissions.values: + raise RuntimeError("Invalid permissions enum value.") # This is a coding error, not a user input error + try: + with atomic(): + publishable_entity = publishing_api.create_publishable_entity( + learning_package_id, entity_ref, created, created_by, can_stand_alone=True + ) + asset = Asset( + publishable_entity=publishable_entity, + learning_package_id=learning_package_id, + path=path, + permissions=permissions, + allow_scripts=allow_scripts, + ) + asset.save(force_insert=True, saved_from=INTERNAL_API) + except IntegrityError as e: + # The only expected failure here is that an asset with this path already exists (possibly created + # concurrently; at READ COMMITTED we can see the other transaction's committed row). + if Asset.objects.filter(learning_package_id=learning_package_id, path=path).exists(): + raise AssetAlreadyExists(path=path) from e + raise + return asset + + +def create_asset_version( + asset_id: Asset | Asset.ID, + /, + version_num: int, + created: datetime, + created_by: int | None, + *, + title: str, + media: Media.ID | Media | bytes, +) -> AssetVersion: + """ + Create a new AssetVersion. Internal - most should use + ``create_next_asset_version`` or ``create_asset_and_version``. + """ + with atomic(savepoint=False): + asset = get_asset(asset_id) + publishable_entity_version = publishing_api.create_publishable_entity_version( + asset.id, + version_num=version_num, + title=title, + created=created, + created_by=created_by, + ) + media = _create_version_media(asset.learning_package_id, path=asset.path, media=media, created=created) + asset_version = AssetVersion.objects.create( + publishable_entity_version=publishable_entity_version, + asset_id=asset.id, + media=media, + ) + + return asset_version + + +def create_next_asset_version( + asset_id: Asset | Asset.ID, + /, + media: Media.ID | Media | bytes | None, + created: datetime, + created_by: int | None, + *, + title: str | None = None, + force_version_num: int | None = None, +) -> AssetVersion: + """ + Create a new AssetVersion based on the most recent version. + + Pass ``media=None`` to keep the same media, e.g. for renaming the ``title`` + only. + """ + # This needs to grab the highest version_num for this Publishable Entity. + # This will often be the Draft version, but not always. For instance, if + # an entity was soft-deleted, the draft would be None, but the version_num + # should pick up from the last edited version. Likewise, a Draft might get + # reverted to an earlier version, but we want the latest version_num when + # creating the next version. + asset = get_asset(asset_id) + last_version: AssetVersion | None = asset.versioning.latest + if last_version is None: + next_version_num = 1 + if title is None: + title = asset.path + if media is None: + raise RuntimeError("Cannot create an initial version without media.") + else: + next_version_num = last_version.version_num + 1 + if title is None: + title = last_version.title + if media is None: + media = last_version.media_id + + if force_version_num is not None: + next_version_num = force_version_num + + return create_asset_version( + asset, version_num=next_version_num, created=created, created_by=created_by, title=title, media=media + ) + + +def create_asset_and_version( + learning_package: LearningPackage | LearningPackage.ID, + /, + path: str, + *, + title: str | None = None, + created: datetime | None = None, + created_by: int | None = None, + media: Media.ID | Media | bytes, + permissions: AssetPermissions | None = None, + allow_scripts: bool = False, + overwrite_deleted: bool = True, +) -> tuple[Asset, AssetVersion]: + """ + Create an Asset and associated AssetVersion atomically. + + permissions: Set permissions to restrict this asset to enrolled or staff + users. Set to ``None`` to use the system default (currently ``PUBLIC`` + but may change in the future). If you set this to ``None`` and overwrite + a deleted asset, the deleted asset's previous permissions mode will be + restored. + allow_scripts: for HTML assets only, set this ``True`` to serve the asset in + a more permissive sandbox that allows referencing JavaScript files. + overwrite_deleted: if this asset has the same name as a deleted asset or a + "placeholder" asset (an asset with no versions at all), then un-delete + it and create a new version of it. + """ + learning_package_id = _get_learning_package_id(learning_package) + if created is None: + created = datetime.now(tz=UTC) + if title is None: + title = path + with atomic(): + try: + asset = create_asset( + learning_package_id, path, created, created_by, permissions=permissions, allow_scripts=allow_scripts + ) + asset_version = create_asset_version( + asset, version_num=1, title=title, created=created, created_by=created_by, media=media + ) + except AssetAlreadyExists: + if overwrite_deleted: + asset = Asset.objects.get(learning_package_id=learning_package_id, path=path) + if asset.versioning.draft is None: + # Reset 'allow_scripts' to False unless specified, and update permissions iff specified: + update_asset(asset, permissions=permissions, allow_scripts=allow_scripts) + asset_version = create_next_asset_version( + asset, title=title, created=created, created_by=created_by, media=media + ) + asset.refresh_from_db() + return (asset, asset_version) + raise + + return (asset, asset_version) + + +def _create_version_media( + learning_package: LearningPackage | LearningPackage.ID, + path: str, + media: Media.ID | Media | bytes, + created: datetime, +) -> Media: + """ + Internal helper to set the Media for this AssetVersion. + + Only call this when we're first initializing a AssetVersion. + + Media can be specified as ``bytes`` for testing convenience, but you will + almost always want to create a Media object first in actual app code, + because that will give you better control over the MIME type and storage + specifics (file vs. database). + """ + learning_package_id = _get_learning_package_id(learning_package) + + match media: + case int(): # MediaID + media_obj = media_api.get_media(media) + case Media(): + media_obj = media + case bytes(): + media_type_str, _encoding = mimetypes.guess_type(path) + # We use "application/octet-stream" as a generic fallback media type, per + # RFC 2046: https://datatracker.ietf.org/doc/html/rfc2046 + media_type_str = media_type_str or "application/octet-stream" + media_type = media_api.get_or_create_media_type(media_type_str) + media_obj = media_api.get_or_create_file_media( + learning_package_id, + media_type.id, + data=media, + created=created, + ) + return media_obj + case _: + raise ValueError(f"Invalid object for Asset media: {media!r}") + + if media_obj.learning_package_id != learning_package_id: + raise ValueError( + f"Media LearningPackage does not match Asset: " + f"(Media Learning Package ID {media_obj.learning_package_id!r})" + f"(Asset Learning Package ID {learning_package_id!r})" + ) + return media_obj + + +def update_asset( + asset_id: Asset | Asset.ID, /, *, permissions: AssetPermissions | None = None, allow_scripts: bool | None = None +) -> Asset: + """ + Update an asset. + + Any fields passed as ``None`` will keep their existing value. + """ + asset = get_asset(asset_id) + fields: list[str] = [] + if permissions is not None: + asset.permissions = permissions + fields.append("permissions") + if allow_scripts is not None: + asset.allow_scripts = allow_scripts + fields.append("allow_scripts") + if fields: + asset.full_clean() + asset.save(update_fields=fields, saved_from=INTERNAL_API) + return asset + + +def get_asset(asset_id: Asset | Asset.ID, /) -> Asset: + """ + Get an Asset by its primary key. + + This is the same as the PublishableEntity's ID primary key. + """ + if isinstance(asset_id, Asset): # This allows us to use get_asset throughout this API to accept either ID or Asset + return asset_id + return Asset.with_publishing_relations.get(pk=asset_id) + + +def get_asset_by_path( + learning_package: LearningPackage | LearningPackage.ID, + /, + path: str, +) -> Asset: + """ + Get an Asset by its path (filename, e.g. "image.jpg") + """ + learning_package_id = _get_learning_package_id(learning_package) + return Asset.with_publishing_relations.get(learning_package_id=learning_package_id, path=path) + + +def get_assets( + learning_package: LearningPackage | LearningPackage.ID, + /, + draft: bool | None = None, + published: bool | None = None, + used_by_draft_versions: bool = False, +) -> QuerySet[Asset]: + """ + Fetch a QuerySet of Assets for a LearningPackage using various filters. + + This method will pre-load all the relations that we need in order to get + info from the Asset's draft and published versions, since we'll be + referencing these a lot. + + ⚠️ You should always specify either ``draft=True`` or ``published=True`` or + else you might get deleted assets or "placeholder assets" that have no + active version. + + Args: + used_by_draft_versions: Set this true to prefetch a list of all the + PublishableEntityVersions of any draft entities that depend on the + returned assets. + """ + learning_package_id = _get_learning_package_id(learning_package) + qset = Asset.with_publishing_relations.filter(learning_package_id=learning_package_id).order_by("path") + if draft is not None: + qset = qset.filter(publishable_entity__draft__version__isnull=not draft) + if published is not None: + qset = qset.filter(publishable_entity__published__version__isnull=not published) + if used_by_draft_versions: + qset = qset.prefetch_related( # type: ignore[no-redef] + Prefetch( + "publishable_entity__affects", + queryset=PublishableEntityVersion.objects.filter(draft__isnull=False).select_related("entity"), + to_attr="used_by_draft_versions", + ) + ) + return qset + + +def get_missing_asset_dependencies(learning_package: LearningPackage | LearningPackage.ID) -> QuerySet[Asset]: + """ + Get a QuerySet of assets that are dependencies of some draft entity in the + learning package but which don't have a current draft version. + """ + learning_package_id = _get_learning_package_id(learning_package) + qset = ( + Asset.with_publishing_relations.filter(learning_package_id=learning_package_id) + .filter(publishable_entity__draft__version__isnull=True) + .filter(publishable_entity__publishableentityversiondependency__referring_version__draft__isnull=False) + .distinct() + .order_by("path") + ) + return qset diff --git a/src/openedx_content/applets/assets/models.py b/src/openedx_content/applets/assets/models.py new file mode 100644 index 000000000..83146237f --- /dev/null +++ b/src/openedx_content/applets/assets/models.py @@ -0,0 +1,227 @@ +""" +Models for Assets, which are files shared by multiple other entities in a +learning package. + +The model hierarchy is :class:`Asset` → :class:`AssetVersion` → :class:`Media`. +""" + +from __future__ import annotations + +import re +from typing import NewType, cast + +from django.db import models +from typing_extensions import Sentinel, deprecated + +from openedx_django_lib.fields import case_sensitive_char_field +from openedx_django_lib.managers import WithRelationsManager + +from ..media.models import Media +from ..publishing.models import ( + LearningPackage, + PublishableEntity, + PublishableEntityMixin, + PublishableEntityVersion, + PublishableEntityVersionMixin, +) + +__all__ = [ + "Asset", + "AssetVersion", +] + +# Validation pattern for Asset's filename/path field. +# Disallows starting slash, control chars, '//', '\', './' and '../'. +PATH_RE = re.compile( + r"^" # beginning of string + r"(?![^\x00-\x1f\x7f]*[\x00-\x1f\x7f])" # No control chars anywhere in the string + r"(?!.*(?:^|/)\.{1,2}(?:/|$))" # no "." or ".." path segments + r"[^/\\]+" # first segment: no slash or backslash + r"(?:/[^/\\]+)*" # more segments, each after exactly one "/" + r"$" # end of string +) +INVALID_PATH_MSG = "Asset path is invalid." +# Limit path length to 480. If not for this, on MySQL the actual limit would be 487 in practice anyways, due to +# 'entity_ref' being 'openedx-asset:...' and limited to 500 chars. +PATH_MAX_LENGTH = 480 +# Sentinel value used so only internal APIs can call `Asset.save()` +INTERNAL_API = Sentinel("INTERNAL_API") + + +class AssetPermissions(models.TextChoices): + """Access permissions for an ``Asset``""" + + # No authentication required. Default (for performance reasons, not product reasons). + PUBLIC = "public" + # Restricted to enrolled users only + LOCKED = "locked" + # Restricted to staff only + STAFF = "staff" + + +class Asset(PublishableEntityMixin): + """ + An :class:`Asset` represents a shared asset file in a learning package. + + Each file in a course run's "Files" section is an :class:`Asset`. + + :class:`Asset` is distinct from the media files that are attached to + individual :class:`Component` instances, although both use :class:`Media` to + store the actual files. The main difference is that the Component-local + Media are scoped to a single Component, whereas each :class:`Asset` + represents a file that can be used by many different Components from the + :class:`LearningPackage`, or used by no Components at all in the case of a + course-level Asset like a textbook PDF or the course card image used in the + catalog. The other significant difference is that each :class:`Asset` holds + only a single file, whereas a :class:`Component` can have many files + attached. + + .. no_pii: + """ + + AssetID = NewType("AssetID", PublishableEntity.ID) + type ID = AssetID + + @property + def id(self) -> ID: + return cast(Asset.ID, self.publishable_entity_id) + + @property + @deprecated("Use .id instead") + def pk(self): + """Mark the .pk attribute as deprecated""" + # Note: Django-Stubs forces mypy to identify the `.pk` attribute of this model as having 'Any' type (due to our + # use of a OneToOneField primary key), and this is impossible for us to override, so we prefer to use + # `.id` which we can control fully. + # Since Django uses '.pk' internally, we have to make sure it still works, however. So the best we can do is + # override this with a deprecated marker, so it shows a warning in developer's IDEs like VS Code. + return self.id + + with_publishing_relations = WithRelationsManager( + "publishable_entity", + "publishable_entity__draft__version", + "publishable_entity__draft__version__assetversion", + "publishable_entity__published__version", + "publishable_entity__published__version__assetversion", + "publishable_entity__published__publish_log_record", + "publishable_entity__published__publish_log_record__publish_log", + ) + + learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE) + """ + This foreign key is technically redundant because we're already locked + to a single :class:`LearningPackage` through our ``publishable_entity`` + relation. However, having this foreign key directly allows us to make + indexes that efficiently query by other :class:`Asset` fields + within a given :class:`LearningPackage`, which is going to be a common + use case (and we can't make a compound index using columns from + different tables). + """ + + path = case_sensitive_char_field( + max_length=PATH_MAX_LENGTH, + blank=False, + # Validation of path value is below, as a CheckConstraint. + ) + """ + The file name, such as ``image.png`` or ``images/example.jpg``. If this path + is e.g. ``image.png``, then it is referenced from OLX as "static/image.png". + But in general this ``path`` should not include ``static/``. + + The path is not versioned and cannot be changed. To "rename" a file, + create a copy with a new name and (soft) delete the instance with the old + name. + """ + + permissions = models.CharField( + max_length=12, + blank=False, + choices=AssetPermissions, + default=AssetPermissions.PUBLIC.value, + ) + + allow_scripts = models.BooleanField(default=False, null=False) + """Serve HTML file assets with a reduced sandbox that allows loading scripts.""" + + class Meta: + default_manager_name = "objects" # Use the manager from PublishableEntityMixin by default + constraints = ( + # path (filename) is unique within the learning package + models.UniqueConstraint( + fields=["learning_package", "path"], + name="oe_content_asset_uniq_path", + ), + # path (filename) has to match our path regex + models.CheckConstraint( + condition=models.lookups.Regex(models.F("path"), PATH_RE.pattern), + name="oe_content_asset_valid_path", + violation_error_message=INVALID_PATH_MSG, + ), + # Make SQLite enforce a maximum path length like MySQL does + models.CheckConstraint( + condition=models.Q(path__length__lte=PATH_MAX_LENGTH), name="oe_content_asset_path_maxlength" + ), + models.CheckConstraint( + condition=models.Q(permissions__in=AssetPermissions.values), name="oe_content_asset_permissions_enum" + ), + ) + + # These are for the Django Admin UI. + verbose_name = "Asset" + verbose_name_plural = "Assets" + + def __str__(self) -> str: + return f"Asset:{self.path}" + + def save(self, *args, **kwargs): + """Block saving changes except through authorized APIs""" + # Note: the manager's `update()` method still works as normal + if kwargs.pop("saved_from", None) != INTERNAL_API: + if self._state.adding: + raise RuntimeError("Use the ``create_asset...()`` APIs to create an Asset.") + raise RuntimeError("Use the ``update_asset()`` API to update an Asset.") + return super().save(*args, **kwargs) + + @property + def used_by_draft_versions(self) -> list[PublishableEntityVersion]: + """ + Get a list of all the draft versions of any ``PublishableEntity`` + instances whose current draft version depends on this asset. This only + works if you load a set of assets via:: + + get_assets(..., draft=True, used_by_draft_versions=True) + """ + try: + return self.publishable_entity.used_by_draft_versions # type: ignore[attr-defined] + except AttributeError: + raise RuntimeError( + "used_by_draft_versions is not loaded unless you use get_assets(..., used_by_draft_versions=True)" + ) from None + + +class AssetVersion(PublishableEntityVersionMixin): + """ + A particular version of a :class:`Asset`. + + .. no_pii: + """ + + asset = models.ForeignKey(Asset, on_delete=models.CASCADE, related_name="versions") + """ + This is technically redundant, since we can get this through + ``publishable_entity_version.publishable.asset``, but this is more + convenient. + """ + + media = models.ForeignKey(Media, null=False, on_delete=models.RESTRICT, related_name="asset_versions") + """media holds the actual file data associated with this version of the asset.""" + + class Meta: + verbose_name = "Asset Version" + verbose_name_plural = "Asset Versions" + + def save(self, *args, **kwargs): + """Block saving changes to existing rows of this model""" + if not self._state.adding: + raise RuntimeError("AssetVersion is immutable. Do not use save() to modify it.") + return super().save(*args, **kwargs) diff --git a/src/openedx_content/applets/assets/readme.rst b/src/openedx_content/applets/assets/readme.rst new file mode 100644 index 000000000..03164433e --- /dev/null +++ b/src/openedx_content/applets/assets/readme.rst @@ -0,0 +1,27 @@ +Assets Applet +============= + +The ``assets`` applet holds the core data models that store "shared assets" used in courses and content libraries: individual files, such as images or PDF documents, that can be used by many Components in a Learning Package, or by none at all. See ADRs 0013 (Shared Static Assets) and 0014 (Referencing Assets) in ``docs/openedx_content/decisions/``. + +Motivation +---------- + +Course "Files" have historically lived in the MongoDB contentstore, as a course-wide, unversioned namespace of files. We want to move them into ``openedx_content`` so that they get the same versioning, draft/publish and dependency tracking as the rest of the content in a Learning Package. + +Intended Use Cases +------------------ + +* Storing every file in a course run's "Files" page (including files migrated from contentstore) as an ``Asset`` in the run's Learning Package. +* Storing files uploaded directly to a content library, rather than attached to a particular Component. +* Answering "which Components use this file?" and "which referenced files are missing?" with queries over ``PublishableEntityVersionDependency``, instead of parsing content. + +Architecture Guidelines +----------------------- + +* An ``Asset`` is a ``PublishableEntity`` that holds exactly one file. Each ``AssetVersion`` points to a single ``Media`` row. Related files are grouped by attaching them to one Component, or by putting the Assets in a Collection, not by putting several files in one Asset. +* An Asset is identified by its ``path`` (filename), which is unique and case-sensitive within a Learning Package and can never change, because content refers to Assets by filename. Look Assets up by ``path``; don't parse ``entity_ref``. +* ``permissions`` and ``allow_scripts`` are on the unversioned ``Asset``, so that changing them applies to every version. This applet only stores them. Permission checks and the decision to honor ``allow_scripts`` belong to the platform. +* An Asset with no versions is a *placeholder* for a file that is referenced but hasn't been uploaded yet. Uploading a file with that name (or with the name of a soft-deleted Asset) creates a new version of the existing Asset, so that existing dependencies on it are kept. +* Create and modify Assets only through ``api.py``. ``Asset.save()`` refuses direct saves, and ``AssetVersion`` is immutable. + +See ADRs 0013 (Shared Static Assets) and 0014 (Referencing Assets) in ``docs/openedx_content/decisions/`` for more details. diff --git a/src/openedx_content/applets/components/api.py b/src/openedx_content/applets/components/api.py index fc4b1e843..afe977e92 100644 --- a/src/openedx_content/applets/components/api.py +++ b/src/openedx_content/applets/components/api.py @@ -10,6 +10,7 @@ Please look at the models.py file for more information about the kinds of data are stored in this app. """ + from __future__ import annotations import mimetypes @@ -27,7 +28,8 @@ from ..media import api as media_api from ..media.models import Media from ..publishing import api as publishing_api -from .models import Component, ComponentType, ComponentVersion, ComponentVersionMedia, LearningPackage +from ..publishing.models import LearningPackage, PublishableEntity +from .models import Component, ComponentType, ComponentVersion, ComponentVersionMedia # The public API that will be re-exported by openedx_content.api # is listed in the __all__ entries below. Internal helper functions that are @@ -118,6 +120,7 @@ def create_component_version( created_by: int | None, *, media: dict[str, Media.ID | Media | bytes] | None = None, + dependencies: list[PublishableEntity.ID] | None = None, ) -> ComponentVersion: """ Create a new ComponentVersion @@ -140,6 +143,7 @@ def create_component_version( title=title, created=created, created_by=created_by, + dependencies=dependencies, ) component_version = ComponentVersion.objects.create( publishable_entity_version=publishable_entity_version, @@ -161,6 +165,7 @@ def create_next_component_version( *, force_version_num: int | None = None, ignore_previous_media: bool = False, + dependencies: list[PublishableEntity.ID] | None = None, ) -> ComponentVersion: """ Create a new ComponentVersion based on the most recent version. @@ -176,6 +181,9 @@ def create_next_component_version( this version's number explicitly. Use this if you need to restore or import a version with a specific version number, such as during data migration or when synchronizing with external systems. ignore_previous_media (bool): If True, do not copy over media from the previous version. + dependencies (list, optional): The PublishableEntity IDs this version depends on. If None (the default), + the previous version's dependencies are copied over unchanged. If a list is passed, it replaces the + previous dependencies entirely; pass ``[]`` to remove all dependencies. Returns: ComponentVersion: The newly created ComponentVersion instance. @@ -235,12 +243,20 @@ def create_next_component_version( next_version_num = force_version_num with atomic(): + if dependencies is None and last_version is not None: + # Carry over the previous version's dependencies unchanged. (Pass + # an explicit list, e.g. [], to override or clear them.) + dependencies = list( + last_version.publishable_entity_version.dependencies.values_list("id", flat=True) + ) + publishable_entity_version = publishing_api.create_publishable_entity_version( component_id, version_num=next_version_num, title=title, created=created, created_by=created_by, + dependencies=dependencies, ) component_version = ComponentVersion.objects.create( publishable_entity_version=publishable_entity_version, @@ -284,6 +300,7 @@ def create_component_and_version( # pylint: disable=too-many-positional-argumen *, can_stand_alone: bool = True, media: dict[str, Media.ID | Media | bytes] | None = None, + dependencies: list[PublishableEntity.ID] | None = None, ) -> tuple[Component, ComponentVersion]: """ Create a Component and associated ComponentVersion atomically. @@ -304,6 +321,7 @@ def create_component_and_version( # pylint: disable=too-many-positional-argumen created=created, created_by=created_by, media=media or {}, + dependencies=dependencies, ) return (component, component_version) diff --git a/src/openedx_content/applets/courses/__init__.py b/src/openedx_content/applets/courses/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/openedx_content/applets/courses/admin.py b/src/openedx_content/applets/courses/admin.py new file mode 100644 index 000000000..36a296bd7 --- /dev/null +++ b/src/openedx_content/applets/courses/admin.py @@ -0,0 +1,20 @@ +""" +Django admin for ``courses`` models +""" + +from django.contrib import admin + +from openedx_django_lib.admin_utils import ReadOnlyModelAdmin + +from .models import CourseContent + + +@admin.register(CourseContent) +class CourseContentAdmin(ReadOnlyModelAdmin): + """ + Django admin for CourseContent model + """ + + list_display = ("course_run", "learning_package") + search_fields = ("course_run__course_key", "learning_package__package_ref", "learning_package__title") + list_select_related = ("course_run", "learning_package") diff --git a/src/openedx_content/applets/courses/api.py b/src/openedx_content/applets/courses/api.py new file mode 100644 index 000000000..aeb230ede --- /dev/null +++ b/src/openedx_content/applets/courses/api.py @@ -0,0 +1,171 @@ +""" +Courses API: the mapping between course runs and their Learning Packages. + +See :ref:`openedx-content-adr-0012`. Code that needs a course run's Learning +Package must look it up through this API, never by ``package_ref``. +""" + +from __future__ import annotations + +import secrets +from datetime import datetime + +from django.db import IntegrityError +from django.db.models import QuerySet +from django.db.transaction import atomic +from opaque_keys.edx.keys import CourseKey + +from openedx_catalog import api as catalog_api +from openedx_catalog.models_api import CourseRun + +from ..publishing import api as publishing_api +from ..publishing.models import LearningPackage +from .models import CourseContent + +# The public API that will be re-exported by openedx_content.api +# is listed in the __all__ entries below. Internal helper functions that are +# private to this module should start with an underscore. If a function does not +# start with an underscore AND it is not in __all__, that function is considered +# to be callable only by other applets in the openedx_content package. +__all__ = [ + "create_course_content", + "delete_course_content", + "get_course_content", + "get_course_runs_for_learning_package", + "get_learning_package_for_course_run", + "get_or_create_course_content", +] + + +def _get_course_run(course_run: CourseRun | CourseKey) -> CourseRun: + if isinstance(course_run, CourseRun): + return course_run + return catalog_api.get_course_run(course_run) + + +def _course_run_filter(course_run: CourseRun | CourseKey) -> dict: + """Get filter kwargs to find the CourseContent for a course run, without loading the CourseRun.""" + if isinstance(course_run, CourseRun): + return {"course_run": course_run} + return {"course_run__course_key__exact": course_run} + + +def _generate_package_ref(course_key: CourseKey) -> str: + """ + Generate a ``package_ref`` for a new course Learning Package. + + It includes the course key so that it is human readable, plus a random + suffix so that nothing can assume it is 1:1 with the course key (see ADR + 0012, decision 4). + """ + return f"{course_key}:{secrets.token_hex(4)}" + + +def create_course_content( + course_run: CourseRun | CourseKey, + /, + learning_package: LearningPackage | LearningPackage.ID | None = None, + *, + created: datetime | None = None, +) -> CourseContent: + """ + Associate a course run with a Learning Package that holds its content. + + If ``learning_package`` is ``None``, a new Learning Package is created for + the course run. Raises ``IntegrityError`` if the course run already has a + Learning Package. Raises ``CourseRun.DoesNotExist`` if no such course run + exists. + """ + course_run = _get_course_run(course_run) + assert course_run.course_key is not None # for type checking; the column is not nullable + with atomic(): + if learning_package is None: + learning_package = publishing_api.create_learning_package( + package_ref=_generate_package_ref(course_run.course_key), + title=course_run.title, + created=created, + ) + elif not isinstance(learning_package, LearningPackage): + learning_package = publishing_api.get_learning_package(learning_package) + return CourseContent.objects.create(course_run=course_run, learning_package=learning_package) + + +def get_or_create_course_content( + course_run: CourseRun | CourseKey, + /, + *, + created: datetime | None = None, +) -> tuple[CourseContent, bool]: + """ + Get the ``CourseContent`` for a course run, creating it (along with a new + Learning Package) if the course run doesn't have one yet. + + Returns a ``(course_content, created)`` tuple, like Django's + ``get_or_create()``. This is safe to call concurrently: if another caller + creates the ``CourseContent`` first, the Learning Package created by this + call is rolled back and the other caller's ``CourseContent`` is returned. + + ⚠️ Only call this from code that is deliberately setting up a course run to + store its content in ``openedx_content``. Never call it from code that is + just reading content, or merely looking up a course run's content could + create an empty Learning Package for it. Use ``get_course_content()`` or + ``get_learning_package_for_course_run()`` to read. + + Raises ``CourseRun.DoesNotExist`` if no such course run exists. + """ + try: + return get_course_content(course_run), False + except CourseContent.DoesNotExist: + pass + try: + # create_course_content() uses atomic(), so if this fails, its new Learning Package is rolled back too. + return create_course_content(course_run, created=created), True + except IntegrityError: + # Another caller created this course run's CourseContent concurrently. + return get_course_content(course_run), False + + +def get_course_content(course_run: CourseRun | CourseKey, /) -> CourseContent: + """ + Get the ``CourseContent`` for a course run. + + Raises ``CourseContent.DoesNotExist`` if the course run has no content in + ``openedx_content``. + """ + return CourseContent.objects.select_related("course_run", "learning_package").get(**_course_run_filter(course_run)) + + +def get_learning_package_for_course_run(course_run: CourseRun | CourseKey, /) -> LearningPackage: + """ + Get the Learning Package that holds the content of the given course run. + + Raises ``LearningPackage.DoesNotExist`` if the course run has no content in + ``openedx_content``. + """ + filter_kwargs = {f"course_contents__{k}": v for k, v in _course_run_filter(course_run).items()} + return LearningPackage.objects.get(**filter_kwargs) + + +def get_course_runs_for_learning_package( + learning_package: LearningPackage | LearningPackage.ID, + /, +) -> QuerySet[CourseRun]: + """ + Get the course run(s) whose content is held in the given Learning Package. + + By convention there is at most one, but callers must handle receiving more + than one (see ADR 0012). + """ + if isinstance(learning_package, LearningPackage): + learning_package = learning_package.id + return CourseRun.objects.filter(content__learning_package_id=learning_package).order_by("course_key") + + +def delete_course_content(course_run: CourseRun | CourseKey, /) -> None: + """ + Remove the association between a course run and its Learning Package. + + The Learning Package itself (and all of its content) is left in place. This + is a no-op if the course run has no ``CourseContent``. + """ + CourseContent.objects.filter(**_course_run_filter(course_run)).delete() diff --git a/src/openedx_content/applets/courses/models.py b/src/openedx_content/applets/courses/models.py new file mode 100644 index 000000000..35aa0f0e0 --- /dev/null +++ b/src/openedx_content/applets/courses/models.py @@ -0,0 +1,59 @@ +""" +Models that relate course runs to the Learning Packages that hold their content. + +See :ref:`openedx-content-adr-0012`. +""" + +from __future__ import annotations + +from django.db import models + +from openedx_catalog.models_api import CourseRun + +from ..publishing.models import LearningPackage + +__all__ = [ + "CourseContent", +] + + +class CourseContent(models.Model): + """ + Maps a :class:`CourseRun` to the :class:`LearningPackage` that holds its + content. + + Each course run has zero or one ``CourseContent`` rows. By convention, each + course run gets its own Learning Package, but this is deliberately *not* + enforced: several ``CourseContent`` rows may point to the same + :class:`LearningPackage`. Code must not assume that the relationship is + 1:1, and code that needs the course run(s) for a Learning Package must + handle receiving more than one. + + This is not the canonical "Course" model, and should not be used as a + foreign key target; use :class:`CourseRun` for that. + + .. no_pii: + """ + + course_run = models.OneToOneField( + CourseRun, + on_delete=models.CASCADE, + primary_key=True, + related_name="content", + ) + + learning_package = models.ForeignKey( + LearningPackage, + # Don't allow deleting a Learning Package while a course run is still using it. + on_delete=models.PROTECT, + null=False, + # Not unique: see the class docstring. + related_name="course_contents", + ) + + class Meta: + verbose_name = "Course Content" + verbose_name_plural = "Course Content" + + def __str__(self) -> str: + return f"CourseContent: {self.course_run.course_key} → {self.learning_package.package_ref}" diff --git a/src/openedx_content/applets/courses/readme.rst b/src/openedx_content/applets/courses/readme.rst new file mode 100644 index 000000000..1ef4a9415 --- /dev/null +++ b/src/openedx_content/applets/courses/readme.rst @@ -0,0 +1,12 @@ +Courses Applet +============== + +The ``courses`` applet holds the ``CourseContent`` model, which maps each course run (``openedx_catalog.CourseRun``) to the ``LearningPackage`` that holds its content. See ADR 0012 (Course Learning Packages) in ``docs/openedx_content/decisions/``. + +Architecture Guidelines +----------------------- + +* Each course run has zero or one ``CourseContent`` rows. +* By convention, each course run gets its own ``LearningPackage``, but several ``CourseContent`` rows may point to the same ``LearningPackage``. Code must never assume the relationship is 1:1. +* Code that needs a course run's ``LearningPackage`` must look it up through this applet's API, never by ``package_ref``, and never derive a course key from a ``package_ref``. +* ``CourseContent`` is not a "Course" model and should not be used as a foreign key target. Use ``CourseRun`` instead. diff --git a/src/openedx_content/apps.py b/src/openedx_content/apps.py index 4923d1d83..abfc82c59 100644 --- a/src/openedx_content/apps.py +++ b/src/openedx_content/apps.py @@ -1,6 +1,7 @@ """ App Config for our umbrella openedx_content app. """ + from django.apps import AppConfig # pylint: disable=import-outside-toplevel @@ -26,6 +27,8 @@ def register_publishable_models(self): """ from .api import register_publishable_models from .models import ( + Asset, + AssetVersion, Component, ComponentVersion, Container, @@ -37,6 +40,8 @@ def register_publishable_models(self): Unit, UnitVersion, ) + + register_publishable_models(Asset, AssetVersion) register_publishable_models(Component, ComponentVersion) register_publishable_models(Container, ContainerVersion) register_publishable_models(Section, SectionVersion) diff --git a/src/openedx_content/migrations/0015_assets.py b/src/openedx_content/migrations/0015_assets.py new file mode 100644 index 000000000..e1b0f5464 --- /dev/null +++ b/src/openedx_content/migrations/0015_assets.py @@ -0,0 +1,116 @@ +# Generated by Django 5.2.18 on 2026-11-05 03:45 + +import django.db.models.deletion +import django.db.models.lookups +from django.db import migrations, models + +import openedx_django_lib.fields + + +class Migration(migrations.Migration): + dependencies = [ + ("openedx_content", "0014_typed_media_id"), + ] + + operations = [ + migrations.CreateModel( + name="Asset", + fields=[ + ( + "publishable_entity", + models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + primary_key=True, + serialize=False, + to="openedx_content.publishableentity", + ), + ), + ( + "path", + openedx_django_lib.fields.MultiCollationCharField( + db_collations={"mysql": "utf8mb4_bin", "sqlite": "BINARY"}, max_length=480 + ), + ), + ( + "permissions", + models.CharField( + choices=[("public", "Public"), ("locked", "Locked"), ("staff", "Staff")], + default="public", + max_length=12, + ), + ), + ("allow_scripts", models.BooleanField(default=False)), + ( + "learning_package", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, to="openedx_content.learningpackage" + ), + ), + ], + options={ + "verbose_name": "Asset", + "verbose_name_plural": "Assets", + "default_manager_name": "objects", + }, + ), + migrations.CreateModel( + name="AssetVersion", + fields=[ + ( + "publishable_entity_version", + models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, + primary_key=True, + serialize=False, + to="openedx_content.publishableentityversion", + ), + ), + ( + "asset", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, related_name="versions", to="openedx_content.asset" + ), + ), + ( + "media", + models.ForeignKey( + on_delete=django.db.models.deletion.RESTRICT, + related_name="asset_versions", + to="openedx_content.media", + ), + ), + ], + options={ + "verbose_name": "Asset Version", + "verbose_name_plural": "Asset Versions", + }, + ), + migrations.AddConstraint( + model_name="asset", + constraint=models.UniqueConstraint(fields=("learning_package", "path"), name="oe_content_asset_uniq_path"), + ), + migrations.AddConstraint( + model_name="asset", + constraint=models.CheckConstraint( + condition=django.db.models.lookups.Regex( + models.F("path"), + "^(?![^\\x00-\\x1f\\x7f]*[\\x00-\\x1f\\x7f])(?!.*(?:^|/)\\.{1,2}(?:/|$))[^/\\\\]+(?:/[^/\\\\]+)*$", + ), + name="oe_content_asset_valid_path", + violation_error_message="Asset path is invalid.", + ), + ), + migrations.AddConstraint( + model_name="asset", + constraint=models.CheckConstraint( + condition=models.Q(("path__length__lte", 480)), name="oe_content_asset_path_maxlength" + ), + ), + migrations.AddConstraint( + model_name="asset", + constraint=models.CheckConstraint( + condition=models.Q(("permissions__in", ["public", "locked", "staff"])), + name="oe_content_asset_permissions_enum", + ), + ), + ] diff --git a/src/openedx_content/migrations/0016_course_content.py b/src/openedx_content/migrations/0016_course_content.py new file mode 100644 index 000000000..9dd093dd1 --- /dev/null +++ b/src/openedx_content/migrations/0016_course_content.py @@ -0,0 +1,26 @@ +# Generated by Django 5.2.18 on 2026-11-13 22:07 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('openedx_catalog', '0001_initial'), + ('openedx_content', '0015_assets'), + ] + + operations = [ + migrations.CreateModel( + name='CourseContent', + fields=[ + ('course_run', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, related_name='content', serialize=False, to='openedx_catalog.courserun')), + ('learning_package', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='course_contents', to='openedx_content.learningpackage')), + ], + options={ + 'verbose_name': 'Course Content', + 'verbose_name_plural': 'Course Content', + }, + ), + ] diff --git a/src/openedx_content/models.py b/src/openedx_content/models.py index 6a5c696b3..1c3bfe57a 100644 --- a/src/openedx_content/models.py +++ b/src/openedx_content/models.py @@ -7,10 +7,12 @@ # pylint: disable=wildcard-import +from .applets.assets.models import * from .applets.backup_restore.models import * from .applets.collections.models import * from .applets.components.models import * from .applets.containers.models import * +from .applets.courses.models import * from .applets.media.models import * from .applets.publishing.models import * from .applets.sections.models import * diff --git a/src/openedx_content/models_api.py b/src/openedx_content/models_api.py index ee62b524b..2cbbb9282 100644 --- a/src/openedx_content/models_api.py +++ b/src/openedx_content/models_api.py @@ -8,9 +8,11 @@ # These wildcard imports are okay because these modules declare __all__. # pylint: disable=wildcard-import +from .applets.assets.models import * from .applets.collections.models import * from .applets.components.models import * from .applets.containers.models import * +from .applets.courses.models import * from .applets.media.models import * from .applets.publishing.models import * from .applets.sections.models import * diff --git a/tests/openedx_content/applets/assets/__init__.py b/tests/openedx_content/applets/assets/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/openedx_content/applets/assets/test_api.py b/tests/openedx_content/applets/assets/test_api.py new file mode 100644 index 000000000..d2518110b --- /dev/null +++ b/tests/openedx_content/applets/assets/test_api.py @@ -0,0 +1,819 @@ +""" +Tests of the Asset API +""" + +from __future__ import annotations + +import base64 +import re +from datetime import UTC, datetime, timedelta +from typing import TYPE_CHECKING, assert_type, cast + +import pytest +from django.core.exceptions import ValidationError + +from openedx_content.applets.assets import api +from openedx_content.applets.assets.models import Asset, AssetVersion +from openedx_content.applets.components import api as components_api +from openedx_content.applets.media import api as media_api +from openedx_content.applets.media.models import Media +from openedx_content.applets.publishing import api as publishing_api +from openedx_content.applets.publishing.models import LearningPackage + +if TYPE_CHECKING: + from django.contrib.auth.models import User # pylint: disable=imported-auth-user + + # Test that our model manager has correct typing + assert_type(Asset.objects.create(), Asset) + assert_type(Asset.objects.get(), Asset) + assert_type(AssetVersion.objects.create(), AssetVersion) + assert_type(AssetVersion.objects.get(), AssetVersion) + +pytestmark = pytest.mark.django_db + +now = datetime.now(tz=UTC) +later = now + timedelta(seconds=10) + +BLACK_PNG_BYTES: bytes = base64.b64decode( # A valid PNG image, that is a single black pixel + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABAQAAAAA3bvkkAAAACklEQVR4AWNgAAAAAgABc3UBGAAAAABJRU5ErkJggg==" +) +GREY_PNG_BYTES: bytes = base64.b64decode( # A valid PNG image that is a single grey pixel + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVR4AWOwBQAAPwA+Eq7IEAAAAABJRU5ErkJggg==" +) + + +@pytest.fixture(name="lp") +def _lp() -> LearningPackage: + """Get a Learning Package.""" + return publishing_api.create_learning_package(package_ref="assets-test-lp", title="Assets Test LP 1") + + +@pytest.fixture(name="lp2") +def _lp2() -> LearningPackage: + return publishing_api.create_learning_package(package_ref="assets-test-lp2", title="Assets Test LP 2") + + +@pytest.fixture(name="user1") +def _user1(django_user_model) -> User: + return django_user_model.objects.create(username="user1") + + +@pytest.fixture(name="user2") +def _user2(django_user_model) -> User: + return django_user_model.objects.create(username="user2") + + +@pytest.fixture(name="black_png") +def _black_png(lp: LearningPackage) -> Media: + media_type_id = media_api.get_or_create_media_type("image/png").id + return media_api.get_or_create_file_media(lp.id, media_type_id, data=BLACK_PNG_BYTES, created=now) + + +@pytest.fixture(name="grey_png") +def _grey_png(lp: LearningPackage) -> Media: + media_type_id = media_api.get_or_create_media_type("image/png").id + return media_api.get_or_create_file_media(lp.id, media_type_id, data=GREY_PNG_BYTES, created=now) + + +@pytest.fixture(name="black") +def _black(lp: LearningPackage, black_png: Media) -> Asset: + return api.create_asset_and_version(lp.id, path="black.png", created=now, media=black_png)[0] + + +@pytest.fixture(name="grey") +def _grey(lp: LearningPackage, grey_png: Media) -> Asset: + return api.create_asset_and_version(lp.id, path="grey.png", created=now, media=grey_png)[0] + + +def make_component(lp: LearningPackage, component_code: str, dependencies: list[publishing_api.PublishableEntity.ID]): + """Helper function to create a Component and version""" + c, v = components_api.create_component_and_version( + lp.id, + component_type=components_api.get_or_create_component_type("xblock.v1", "html"), + component_code=component_code, + title=component_code, + created=now, + dependencies=dependencies, + ) + return c, v + + +# Creating Assets (without versions/data) ###################################### + + +def test_create_asset(lp: LearningPackage, user1: User) -> None: + """Basic test of ``create_asset()``: Create an asset with no version.""" + path = "instructions.txt" + + asset = api.create_asset(lp.id, path=path, created=now, created_by=user1.id) + asset.refresh_from_db() # Ensure we check what was actually saved to database, not just returned + + assert_type(asset, Asset) + assert asset.id + assert_type(asset.id, Asset.ID) + assert asset.publishable_entity.id == asset.id + assert asset.publishable_entity.learning_package_id == lp.id + assert asset.learning_package == lp + assert asset.publishable_entity.can_stand_alone + assert asset.publishable_entity.created == now + assert asset.publishable_entity.created_by + assert asset.publishable_entity.created_by.id == user1.id + assert asset.path == path + assert asset.permissions == api.AssetPermissions.PUBLIC.value + assert not asset.allow_scripts # Default must be False + assert asset.versioning.latest is None + assert asset.versioning.draft is None + assert asset.versioning.published is None + + +def test_path_unique_per_lp(lp: LearningPackage, lp2: LearningPackage) -> None: + """Test that path (filename) must be unique per learning package.""" + path = "instructions.txt" + other_path = "other.pdf" + + asset1 = api.create_asset(lp.id, path=path, created=now, created_by=None) + assert asset1.id + + with pytest.raises(api.AssetAlreadyExists, match=f'Asset with path "{path}" already exists.'): + # Cannot create an asset with the same path (filename) in the same LP: + api.create_asset(lp.id, path=path, created=now, created_by=None) + + # But creating an asset with the same path in a different LP is fine: + asset2 = api.create_asset(lp2.id, path=path, created=now, created_by=None) + assert asset2.id + assert asset1.id != asset2.id + assert asset1.path == asset2.path + + # And creating an asset with a different path is fine: + asset3 = api.create_asset(lp.id, path=other_path, created=now, created_by=None) + assert asset3.id + assert asset1.id != asset3.id + + +def test_path_case_sensitive(lp: LearningPackage) -> None: + """Test that path (filename) is case sensitive.""" + path1 = "case.txt" + path2 = "Case.txt" + + asset1 = api.create_asset(lp.id, path=path1, created=now, created_by=None) + asset2 = api.create_asset(lp.id, path=path2, created=now, created_by=None) + asset1.refresh_from_db() + asset2.refresh_from_db() + assert asset1.id != asset2.id + assert asset1.path == path1 + assert asset2.path == path2 + + +@pytest.mark.parametrize( + "path, valid", + [ + # ✅ Valid: + ("image.jpg", True), + ("image (2).jpg", True), + ("python_lib.zip", True), + ("course.elmwtqp8.tar.gz", True), + ("IMG-004.JPEG", True), + ("folder/example.pdf", True), + ("textbooks/intro textbook/ch1.pdf", True), + (".hidden.conf", True), + ("Αθήνα.md", True), + ("😎.md", True), + ("long_path_" * 48, True), # 480 characters long ('entity_ref' max is 500 including 'openedx-asset:' prefix) + # The next filename has a narrow no break space before "AM" since that's what MacOS puts in screenshot names + ("screen shots/Screenshot 2026-10-23 at 11.26.13\u202fAM.png", True), + # ❌ Invalid: + ("newline.pdf\n", False), + ("nul\x00.pdf", False), # \x7f is the DEL control character + ("del\x7f.pdf", False), # \x7f is the DEL control character + ("/image.jpg", False), + ("./image.jpg", False), + ("//image.jpg", False), + ("../image.jpg", False), + ("folder/", False), + ("backslash\\image.jpg", False), + ("folder/../image.jpg", False), + ("double//image.jpg", False), + ("../../../etc/passwd", False), + ("", False), + (".", False), + ("..", False), + ("long_path_" * 50, False), # 500 characters long + ], +) +def test_valid_invalid_paths(path: str, valid: bool, lp: LearningPackage) -> None: + """Test validation of filenames""" + + def create(): + return api.create_asset(lp.id, path=path, created=now, created_by=None) + + if valid: + asset = create() + assert asset.id + asset.refresh_from_db() + assert asset.path == path + else: + with pytest.raises(ValidationError, match="Asset path is invalid."): + create() + + +@pytest.mark.parametrize( + "permissions, valid", + [ + (api.AssetPermissions.PUBLIC, True), + (api.AssetPermissions.LOCKED, True), + (api.AssetPermissions.STAFF, True), + ("invalid", False), + ("", False), + ], +) +def test_valid_invalid_permissions(permissions: api.AssetPermissions, valid: bool, lp: LearningPackage) -> None: + """Test validation of permissions value""" + + def create(): + return api.create_asset(lp.id, path="example.txt", created=now, created_by=None, permissions=permissions) + + if valid: + asset = create() + assert asset.id + asset.refresh_from_db() + assert asset.permissions == permissions + else: + with pytest.raises(RuntimeError, match="Invalid permissions enum value."): + create() + + +def test_create_asset_manually(lp: LearningPackage) -> None: + """Manually creating an asset is forbidden""" + pe = publishing_api.create_publishable_entity( + lp.id, "entity_ref", created=now, created_by=None, can_stand_alone=True + ) + with pytest.raises(RuntimeError, match=re.escape("Use the ``create_asset...()`` APIs to create an Asset.")): + Asset.objects.create( + publishable_entity=pe, + learning_package_id=lp.id, + path="example.png", + permissions=api.AssetPermissions.PUBLIC, + ) + + +# Creating Assets (with initial version/data) ################################## + + +def test_create_asset_and_version(lp: LearningPackage, user1: User) -> None: + """Basic test of ``test_create_asset_and_version()``: Create an asset with initial version.""" + path = "black.png" + + (asset, asset_v1) = api.create_asset_and_version( + lp.id, + path=path, + created=now, + created_by=user1.id, + media=BLACK_PNG_BYTES, + ) + asset.refresh_from_db() # Ensure we check what was actually saved to database, not just returned + asset_v1.refresh_from_db() + + assert_type(asset, Asset) + assert asset.id + assert_type(asset.id, Asset.ID) + assert asset.publishable_entity.id == asset.id + assert asset.publishable_entity.learning_package_id == lp.id + assert asset.learning_package == lp + assert asset.publishable_entity.can_stand_alone + assert asset.publishable_entity.created == now + assert asset.publishable_entity.created_by + assert asset.publishable_entity.created_by.id == user1.id + assert asset.path == path + assert asset.permissions == api.AssetPermissions.PUBLIC.value + assert not asset.allow_scripts # Default must be False + assert asset.versioning.published is None # Not published by default + assert asset.versioning.draft + assert asset.versioning.draft.pk == asset_v1.pk + assert asset.versioning.latest + assert asset.versioning.latest.pk == asset_v1.pk + + assert asset_v1.asset.id == asset.id + assert_type(asset_v1.asset, Asset) + assert asset_v1.title == path # By default, title is set to path if not specified + assert asset_v1.created == now + assert asset_v1.created_by + assert asset_v1.created_by.id == user1.id + + media = asset_v1.media + assert media + assert media.learning_package.id == lp.id + assert str(media.media_type) == "image/png" + assert media.hash_digest == "8d55c179d5f1516fa5594d7c76f3ef6a971c6c8a" + assert media.size == len(BLACK_PNG_BYTES) + assert media.has_file + assert media.created == now + + +def test_path_unique_per_lp2(lp: LearningPackage, lp2: LearningPackage, black: Asset, black_png: Media) -> None: + """ + Test that path (filename) must be unique per learning package. + + Similar to ``test_path_unique_per_lp()`` but this uses + ``create_asset_and_version()`` instead of ``create_asset()``. + """ + path = black.path + other_path = "other.pdf" + + with pytest.raises(api.AssetAlreadyExists, match=f'Asset with path "{path}" already exists.'): + # Cannot create an asset with the same path (filename) in the same LP: + api.create_asset_and_version(lp, path=path, created=now, created_by=None, media=black_png) + + # But creating an asset with the same path in a different LP is fine: + asset2, _ = api.create_asset_and_version(lp2, path=path, created=now, created_by=None, media=BLACK_PNG_BYTES) + assert asset2.id + assert black.id != asset2.id + assert black.path == asset2.path + + # And creating an asset with a different path is fine: + asset3, _ = api.create_asset_and_version(lp, path=other_path, created=now, created_by=None, media=black_png) + assert asset3.id + assert black.id != asset3.id + + +def test_create_two_assets(lp: LearningPackage, user1: User, black_png: Media) -> None: + """Create two assets with the same contents""" + path1 = "black1.png" + path2 = "black2.png" + + (_asset1, asset1_v1) = api.create_asset_and_version( + lp.id, path=path1, created=now, created_by=user1.id, media=black_png + ) + (_asset2, asset2_v1) = api.create_asset_and_version( + lp.id, path=path2, created=now, created_by=user1.id, media=black_png + ) + + # V1 of both assets uses the exact same Media instance: + assert asset1_v1.media.id == black_png.id + assert asset2_v1.media.id == black_png.id + + +@pytest.mark.parametrize("permissions", [api.AssetPermissions.LOCKED, api.AssetPermissions.STAFF]) +def test_create_asset_and_version_permissions( + lp: LearningPackage, user1: User, black_png: Media, permissions: api.AssetPermissions +) -> None: + """Create an asset and version with the specified permissions""" + path = "black.png" + + (asset, _asset_v1) = api.create_asset_and_version( + lp.id, + path=path, + created=now, + created_by=user1.id, + media=black_png, + permissions=permissions, + allow_scripts=True, + ) + assert asset.permissions == permissions.value + assert asset.allow_scripts + + +def test_create_asset_and_version_queries( + lp: LearningPackage, user1: User, black_png: Media, django_assert_num_queries +) -> None: + """Create an asset and version with the specified permissions""" + path = "black.png" + + with django_assert_num_queries(25): + api.create_asset_and_version( + lp.id, + path=path, + created=now, + created_by=user1.id, + media=black_png, + ) + + +# Creating Subsequent Asset Versions ########################################### + + +def test_create_asset_two_versions(lp: LearningPackage, user1: User, black_png: Media, grey_png: Media) -> None: + """Create an asset with two versions, v1 black and v2 grey""" + path = "pixel.png" + + (asset, v1) = api.create_asset_and_version( + lp.id, path=path, created=now, created_by=user1.id, media=black_png, title="Black Pixel" + ) + v2 = api.create_next_asset_version(asset.id, media=grey_png, created=later, created_by=user1.id, title="Grey Pixel") + + asset.refresh_from_db() + assert asset.versioning.draft + assert asset.versioning.draft.pk == v2.pk + + assert v1.version_num == 1 + assert v1.title == "Black Pixel" + + assert v2.version_num == 2 + assert v2.title == "Grey Pixel" + assert v2.created == later + assert v2.media_id == grey_png.id + + +def test_create_asset_two_versions_manual(lp: LearningPackage, user1: User, black_png: Media, grey_png: Media) -> None: + """Create an asset with two versions, v1 black and v3 grey, with no v2""" + path = "pixel.png" + + (asset, v1) = api.create_asset_and_version(lp.id, path=path, created=now, created_by=user1.id, media=black_png) + v3 = api.create_next_asset_version( + asset.id, media=grey_png, created=later, created_by=user1.id, force_version_num=3 + ) + + asset.refresh_from_db() + assert asset.versioning.draft + assert asset.versioning.draft.pk == v3.pk + + assert v1.version_num == 1 + assert v3.created == later + assert v3.media_id == grey_png.id + assert v3.version_num == 3 + + +# Getting Assets ############################################################### + + +def test_get_asset_by_id(black: Asset, black_png: Media, grey: Asset, grey_png: Media): + """Test retrieving asset by ID.""" + b = api.get_asset(black.id) + assert b.id == black.id + assert b.versioning.draft.media.id == black_png.id + + g = api.get_asset(grey.id) + assert g.id == grey.id + assert g.versioning.draft.media.id == grey_png.id + + with pytest.raises(Asset.DoesNotExist): + api.get_asset(cast(Asset.ID, -400)) + + +def test_get_asset_by_filename( # pylint: disable=too-many-positional-arguments + lp: LearningPackage, lp2: LearningPackage, black: Asset, black_png: Media, grey: Asset, grey_png: Media +): + """Test retrieving asset by filename.""" + b = api.get_asset_by_path(lp.id, path="black.png") + assert b.id == black.id + assert b.versioning.draft.media.id == black_png.id + + g = api.get_asset_by_path(lp.id, path="grey.png") + assert g.id == grey.id + assert g.versioning.draft.media.id == grey_png.id + + with pytest.raises(Asset.DoesNotExist): + # Different learning package: + api.get_asset_by_path(lp2.id, path="grey.png") + + with pytest.raises(Asset.DoesNotExist): + api.get_asset_by_path(lp2.id, path="404.png") + + +def test_get_assets(lp: LearningPackage, lp2: LearningPackage, black: Asset, grey: Asset): + """ + Test getting all assets in a learning package, or a subset of assets + """ + # We start with two assets in 'lp' (one published, one not), and none in 'lp2': + publishing_api.publish_from_drafts( + lp.id, publishing_api.get_all_drafts(lp.id).filter(entity=black.publishable_entity) + ) + + assert list(api.get_assets(lp)) == [black, grey] + assert list(api.get_assets(lp, draft=True)) == [black, grey] + assert list(api.get_assets(lp, published=True)) == [black] + + assert not list(api.get_assets(lp2)) + + +# Updating Assets ############################################################## + + +def test_change_permissions(black: Asset): + """Test changing an assets permissions using ``update_asset``""" + assert black.permissions == api.AssetPermissions.PUBLIC + assert not black.allow_scripts + + api.update_asset(black, permissions=api.AssetPermissions.LOCKED) + assert black.permissions == api.AssetPermissions.LOCKED + black.refresh_from_db() + assert black.permissions == api.AssetPermissions.LOCKED + assert not black.allow_scripts + + api.update_asset(black, permissions=api.AssetPermissions.STAFF) + assert black.permissions == api.AssetPermissions.STAFF + black.refresh_from_db() + assert black.permissions == api.AssetPermissions.STAFF + assert not black.allow_scripts + + api.update_asset(black, permissions=api.AssetPermissions.PUBLIC, allow_scripts=True) + assert black.permissions == api.AssetPermissions.PUBLIC + assert black.allow_scripts + black.refresh_from_db() + assert black.permissions == api.AssetPermissions.PUBLIC + assert black.allow_scripts + + +def test_update_asset_wont_pull_in_changes(black: Asset): + """Calling ``update_asset()`` won't have unexpected side effects""" + black.path = "foo" + api.update_asset(black, allow_scripts=True) # Should only change allow_scripts, not save changes to `path` + black.refresh_from_db() + assert black.path == "black.png" + + +def test_no_changing_path(black: Asset) -> None: + """Test that an Asset's 'path' is immutable.""" + black.path = "foo" + with pytest.raises(RuntimeError, match=re.escape("Use the ``update_asset()`` API to update an Asset.")): + black.save() + black.refresh_from_db() + assert black.path == "black.png" + + +def test_no_changing_version(black: Asset, grey_png: Media) -> None: + """Test modifying an AssetVersion is forbidden.""" + v1: AssetVersion = black.versioning.latest + v1.media = grey_png + with pytest.raises(RuntimeError, match="AssetVersion is immutable"): + v1.save() + + +# Deleting Assets ############################################################## + + +def test_delete_assets(lp: LearningPackage, black: Asset, grey: Asset): + """ + Test deleting an asset + """ + # We start with two assets in 'lp' (one published, one not), and none in 'lp2': + publishing_api.publish_from_drafts( + lp.id, publishing_api.get_all_drafts(lp.id).filter(entity=black.publishable_entity) + ) + assert list(api.get_assets(lp)) == [black, grey] + # Then we delete 'black': + publishing_api.soft_delete_draft(black.id) + + # It's still in the Learning Package, and still published, but its draft is gone: + assert list(api.get_assets(lp)) == [black, grey] + assert list(api.get_assets(lp, draft=True)) == [grey] # black is gone + assert list(api.get_assets(lp, published=True)) == [black] + assert api.get_asset_by_path(lp, "black.png").versioning.draft is None + + # Publish the deletion: + publishing_api.publish_all_drafts(lp.id) + assert list(api.get_assets(lp)) == [black, grey] + assert list(api.get_assets(lp, draft=True)) == [grey] # black is gone + assert list(api.get_assets(lp, published=True)) == [grey] # black is gone + assert api.get_asset_by_path(lp, "black.png").versioning.published is None + + +def test_undelete_assets(lp: LearningPackage, black: Asset, grey_png: Media, user2: User): + """ + Test creating an asset with the same path (filename) as a deleted asset + """ + assert black.created == now + assert black.created_by is None + # Delete the 'black' asset: + publishing_api.soft_delete_draft(black.id) + + # Grey is the new black: create a new "black.png" asset with grey contents + # This should create a new version of the deleted "black.png" asset. + new_black, v2 = api.create_asset_and_version( + lp, path=black.path, media=grey_png, created=later, created_by=user2.id + ) + assert len(new_black.versioning.versions.all()) == 2 + assert new_black.versioning.draft.pk == v2.pk + assert v2.version_num == 2 + assert v2.media.id == grey_png.id + # Should the 'created' and 'created_by' be reset when un-deleting, or preserved? We'll say preserved. + assert new_black.created == now # Not 'later' + assert new_black.created_by is None # Not 'user2' + new_black.refresh_from_db() + assert new_black.created == now # Not 'later' + assert new_black.created_by is None # Not 'user2' + + +def test_undelete_assets_refused(lp: LearningPackage, black: Asset): + """ + Test creating an asset with the same path (filename) as a deleted asset is + rejected when we pass ``overwrite_deleted=False``. + """ + # Delete the 'black' asset: + publishing_api.soft_delete_draft(black.id) + + with pytest.raises(api.AssetAlreadyExists, match=f'Asset with path "{black.path}" already exists.'): + api.create_asset_and_version(lp, path=black.path, media=BLACK_PNG_BYTES, overwrite_deleted=False) + + +def test_undelete_default_title(lp: LearningPackage, black: Asset, grey_png: Media): + """ + Test creating an asset with the same path (filename) as a deleted asset does + not restore a deleted asset's custom title + """ + # Set the 'black' asset's title to something custom, then delete it: + api.create_next_asset_version( + black.id, title="OLD Title of deleted asset", created=now, created_by=None, media=None + ) + publishing_api.soft_delete_draft(black.id) + + # Grey is the new black: create a new "black.png" asset with grey contents: + _, v2 = api.create_asset_and_version(lp, path=black.path, media=grey_png, created=later, created_by=None) + # The title has been reset to the default title, the path/filename: + assert v2.title == black.path + + +def test_undelete_custom_title(lp: LearningPackage, black: Asset, grey_png: Media): + """ + Test creating an asset with the same path (filename) as a deleted asset can + specify a custom title for the new asset: + """ + publishing_api.soft_delete_draft(black.id) + + new_title = "✅ new black" + # Grey is the new black: create a new "black.png" asset with grey contents: + _, v2 = api.create_asset_and_version( + lp, path=black.path, media=grey_png, created=later, created_by=None, title=new_title + ) + assert v2.title == new_title + + +def test_undelete_permissions(lp: LearningPackage, black: Asset, grey_png: Media): + """ + Creating an asset with the same path (filename) as a deleted asset should + not change its permissions. + """ + # Mark the 'black' asset as private: + api.update_asset(black, permissions=api.AssetPermissions.STAFF) + # Publish the 'black' asset: + publishing_api.publish_all_drafts(lp.id) + v1 = black.versioning.published + # Delete the 'black' asset: + publishing_api.soft_delete_draft(black.id) + + # Grey is the new black: create a new "black.png" asset with grey contents + # This should create a new version of the deleted "black.png" asset. + _, v2 = api.create_asset_and_version(lp, path=black.path, media=grey_png) + + black.refresh_from_db() + assert black.versioning.latest == v2 + assert black.versioning.published == v1 + + # It's important that creating a new draft asset doesn't change the + # permissions of the asset unless the user specifically said to, because + # otherwise this published "staff only" document would now be accessible + # with PUBLIC perms. (Permissions are per-Asset not per AssetVersion.) + assert black.permissions == api.AssetPermissions.STAFF + + +def test_undelete_allow_scripts(lp: LearningPackage): + """ + Creating an asset with the same path (filename) as a deleted asset should + not create a new asset with `allow_scripts=True` just because the deleted + asset had that, unless the user very explicitly opts sets that. + """ + # Create an HTML asset with allow_scripts=True + path = "scripts.html" + html_media_type = media_api.get_or_create_media_type("text/html").id + html_media = media_api.get_or_create_file_media(lp.id, html_media_type, data=b"