original upstream content
" upstream.save() - downstream = BlockFactory.create(category='html', parent=vertical, upstream=str(upstream_key)) - downstream.sync_from_upstream(user=self.user, apply_updates=True) + downstream = BlockFactory.create(category='html', parent=vertical, upstream=str(self.upstream_key)) + self.sync_service = UpstreamSyncService(self.user) + self.sync_service.sync_from_upstream(downstream, apply_updates=True) downstream.save() self.store.update_item(downstream, self.user.id) self.downstream_key = downstream.usage_key - def test_sync_updates_content_and_settings(self): + def test_sync_to_unmodified_content(self): """ Can we sync updates from a content library block to a linked out-of-date course block? """ downstream = self.store.get_item(self.downstream_key) - assert downstream.upstream_settings["display_name"] == "original upstream title" - assert downstream.upstream_overridden == [] + assert downstream.upstream_display_name == "original upstream title" + assert downstream.downstream_customized == set() assert downstream.display_name == "original upstream title" - assert downstream.data == "original upstream content
" + assert downstream.data == "\noriginal upstream content
\n" # @@TODO newlines?? - upstream = xblock.load_block(upstream_key, self.user) + upstream = xblock.load_block(self.upstream_key, self.user) upstream.display_name = "NEW upstream title" upstream.data = "NEW upstream content
" upstream.save() - downstream.sync_from_upstream(user=self.user, apply_updates=True) - assert downstream.upstream_settings["display_name"] == "NEW upstream title" - assert downstream.upstream_overridden == [] + self.sync_service.sync_from_upstream(downstream, apply_updates=True) + assert downstream.upstream_display_name == "NEW upstream title" + assert downstream.downstream_customized == set() assert downstream.display_name == "NEW upstream title" - assert downstream.data == "NEW upstream content
" + assert downstream.data == "\nNEW upstream content
\n" # @@TODO newlines?? - def test_sync_overwrites_content_but_preserves_settings(self): + def test_sync_to_modified_contenet(self): """ - Can we sync updates from a content library block to a linked out-of-date course block? + If we sync to modified content, will it preserve customizable fields, but overwrite the rest? """ downstream = self.store.get_item(self.downstream_key) downstream.display_name = "downstream OVERRIDE of the title" downstream.data = "downstream OVERRIDE of the content
" downstream.save() - upstream = xblock.load_block(upstream_key, self.user) + upstream = xblock.load_block(self.upstream_key, self.user) upstream.display_name = "NEW upstream title" upstream.data = "NEW upstream content
" upstream.save() - downstream.sync_from_upstream(user=self.user, apply_updates=True) - assert downstream.upstream_settings["display_name"] == "NEW upstream title" - assert downstream.upstream_overridden == ["display_name"] + self.sync_service.sync_from_upstream(downstream, apply_updates=True) + assert downstream.upstream_display_name == "NEW upstream title" + assert downstream.downstream_customized == {"display_name"} assert downstream.display_name == "downstream OVERRIDE of the title" - assert downstream.data == "NEW upstream content
" + assert downstream.data == "\nNEW upstream content
\n" # @@TODO newlines?? diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py index fcf8bff2fb4d..c7f783765502 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -1,245 +1,244 @@ """ Synchronize content and settings from upstream blocks to their downstream usages. -At the time of writing, upstream blocks are assumed to come from content libraries, -and downstream blocks will generally belong to courses. However, the system is designed -to be mostly agnostic to exact type of upstream context and type of downstream context. +At the time of writing, we assume that for any upstream-downstream linkage: +* The upstream is a Component from a Learning Core-backed Content Library. +* The downstream is a block of matching type in a SplitModuleStore-backed Courses. +* They are both on the same Open edX instance. + +HOWEVER, those assumptions may loosen in the future. So, we consider these to be INTERNAL ASSUMPIONS that should not be +exposed through this module's public Python interface. """ -import json -from dataclasses import dataclass, asdict +from dataclasses import dataclass from django.contrib.auth import get_user_model -from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import LibraryUsageLocatorV2 from rest_framework.exceptions import NotFound from xblock.exceptions import XBlockNotFoundError -from xblock.fields import Scope, String, Integer, Dict, List +from xblock.fields import Scope, String, Integer, Set from xblock.core import XBlockMixin, XBlock -from webob import Request, Response import openedx.core.djangoapps.xblock.api as xblock_api -from openedx.core.djangoapps.content_libraries.api import ( - get_library_block, - LibraryXBlockMetadata, -) +from openedx.core.djangoapps.content_libraries.api import get_library_block User = get_user_model() -class UnsupportedUpstreamKeyType(Exception): - pass - - -def validate_upstream_key(usage_key: UsageKey | str) -> UsageKey: +class BadUpstream(Exception): """ - Raise an error if the provided key is not a valid upstream reference. - - Currently, only Learning-Core-backed Content Library blocks are valid upstreams, although this may - change in the future. + Reference to upstream content is malformed, invalid, and/or inaccessible. - Raises: InvalidKeyError, UnsupportedUpstreamKeyType + Should be constructed with a human-friendly, localized, PII-free message, suitable for API responses and UI display. """ - if isinstance(usage_key, str): - usage_key = UsageKey.from_string(usage_key) - if not isinstance(usage_key, LibraryUsageLocatorV2): - raise UnsupportedUpstreamKeyType( - "upstream key must be of type LibraryUsageLocatorV2; " - f"provided key '{usage_key}' is of type '{type(usage_key)}'" - ) - return usage_key @dataclass(frozen=True) class UpstreamInfo: """ - Metadata about a block's relationship with an upstream. + Metadata about a downstream's relationship with an upstream. + """ + upstream_ref: str # Reference to the upstream content, e.g., a serialized library block usage key. + current_version: int # Version of the upstream to which the downstream was last synced. + latest_version: int | None # Latest version of the upstream that's available, or None if it couldn't be loaded. + + +class UpstreamSyncService: + """ + @@TODO docstring """ - usage_key: UsageKey - current_version: int - latest_version: int | None - sync_url: str - error: str | None - - @property - def sync_available(self) -> bool: + + def __init__(self, user: User): + self.user = user + + def sync_from_upstream(self, downstream: XBlock, *, apply_updates: bool) -> None: + """ + @@TODO docstring + + Does not save `downstream` to the store. That is left up to the caller. + + Raises: BadUpstream + """ + + # No upstream -> no sync. + if not downstream.upstream: + return + + # Try to load the upstream. + upstream_info = self.inspect_upstream(downstream) # Can raise BadUpstream + try: + upstream = xblock_api.load_block(UsageKey.from_string(downstream.upstream), self.user) + except NotFound: + raise BadUpstream( + _("Linked library item could not be loaded: {}").format(downstream.upstream) + ) from NotFound + + # For every field... + customizable_fields_to_restore_fields = downstream.get_customizable_fields() + for field_name, field in upstream.__class__.fields.items(): + + # ...(ignoring fields that aren't set in the authoring environment)... + if field.scope not in [Scope.content, Scope.settings]: + continue + + # ...if the field *can be* customized (whether or not it *has been* customized), then write its latest + # upstream value to a hidden field, allowing authors to restore it as desired. + # Example: this sets `downstream.upstream_display_name = upstream.display_name`. + upstream_value = getattr(upstream, field_name) + if restore_field_name := customizable_fields_to_restore_fields.get(field.name): + setattr(downstream, restore_field_name, upstream_value) + + # ...if we're applying updates, *and* the field hasn't been customized, then update the downstream value. + # This is the part of the sync that actually pulls in updated upstream content. + # Example: this sets `downstream.display_name = upstream.display_name`. + if apply_updates and field_name not in downstream.downstream_customized: + setattr(downstream, field_name, upstream_value) + + # Done syncing. Record the latest upstream version for future reference. + downstream.upstream_version = upstream_info.latest_version + + def inspect_upstream(self, downstream: XBlock) -> UpstreamInfo | None: """ - Should the user be prompted to sync this block with upstream? + Get info on a block's relationship with its upstream without actually loading any upstream content. + + Currently, the only supported upstream are LC-backed Library Components. This may change in the future (see + module docstring). + + Raises: BadUpstream """ - return ( - self.latest_version - and self.current_version < self.latest_version - and not self.error + if not downstream.upstream: + return None + try: + upstream_key = LibraryUsageLocatorV2.from_string(downstream.upstream) + except InvalidKeyError: + raise BadUpstream( + _("Reference to linked library item is malformed") + ) from InvalidKeyError + downstream_type = downstream.usage_key.block_type + if upstream_key.block_type != downstream_type: + # Note: Currently, we strictly enforce that the downstream and upstream block_types must exactly match. + # It could be reasonable to relax this requirement in the future if there's product need for it. + # For example, there's no reason that a StaticTabBlock couldn't take updates from an HtmlBlock. + raise BadUpstream( + _("Content type mistmatch: {downstream_type} is linked to {upstream_type}").format( + downstream_type=downstream_type, upstream_type=upstream_key.block_type + ) + ) from TypeError( + f"downstream block '{downstream.usage_key}' is linked to " + f"upstream block of different type '{upstream_key}'" + ) + try: + lib_meta = get_library_block(upstream_key) + except XBlockNotFoundError: + raise BadUpstream( + _("Linked library item was not found in the system").format(downstream.upstream) + ) from XBlockNotFoundError + return UpstreamInfo( + upstream_ref=downstream.upstream, + current_version=downstream.upstream_version, + latest_version=(lib_meta.version_num if lib_meta else None), ) +@XBlockMixin.needs("upstream_sync") class UpstreamSyncMixin(XBlockMixin): """ - Mixed into CMS XBlocks so that they be associated & synced with uptsream (e.g. content library) blocks. + Allows an XBlock in the CMS to be associated & synced with an upstream. + Mixed into CMS's XBLOCK_MIXINS, but not LMS's. """ + # Upstream synchronization metadata fields upstream = String( - scope=Scope.settings, help=( "The usage key of a block (generally within a content library) which serves as a source of upstream " - "updates for this block, or None if there is no such upstream. Please note: It is valid for this field " - "to hold a usage key for an upstream block that does not exist (or does not *yet* exist) on this instance, " - "particularly if this downstream block was imported from a different instance." + "updates for this block, or None if there is no such upstream. Please note: It is valid for this " + "field to hold a usage key for an upstream block that does not exist (or does not *yet* exist) on " + "this instance, particularly if this downstream block was imported from a different instance." ), - hidden=True, - default=None, - enforce_type=True, + default=None, scope=Scope.settings, hidden=True, enforce_type=True ) upstream_version = Integer( - scope=Scope.settings, help=( "Record of the upstream block's version number at the time this block was created from it. If " - "upstream_version is smaller than the upstream block's latest version, then the user will be able to sync " - "updates into this downstream block." + "upstream_version is smaller than the upstream block's latest version, then the user will be able " + "to sync updates into this downstream block." ), - hidden=True, - default=None, - enforce_type=True, + default=None, scope=Scope.settings, hidden=True, enforce_type=True, ) - upstream_overridden = List( - scope=Scope.settings, + downstream_customized = Set( help=( - "Names of the fields which have values set on the upstream block yet have been explicitly overridden " - "on this downstream block. Unless explicitly cleared by ther user, these overrides will persist even " - "when updates are synced from the upstream." + "Names of the fields which have values set on the upstream block yet have been explicitly " + "overridden on this downstream block. Unless explicitly cleared by the user, these customizations " + "will persist even when updates are synced from the upstream." ), - hidden=True, - default=[], - enforce_type=True, + default=set(), scope=Scope.settings, hidden=True, enforce_type=True, ) - upstream_settings = Dict( - scope=Scope.settings, - help=( - "@@TODO helptext" - ), - hidden=True, - default={}, enforce_type=True, + + # Store upstream defaults for customizable fields. + upstream_display_name = String( + help=("The value of display_name on the linked upstream block."), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + upstream_max_attempts = Integer( + help=("The value of max_attempts on the linked upstream block."), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, ) - def save(self, *args, **kwargs): + @classmethod + def get_customizable_fields(cls) -> dict[str, str]: """ - Upon save, ensure that upstream_overriden tracks all upstream-provided fields which downstream has overridden. + Mapping from each customizable field to field which stores its upstream default. - @@TODO use is_dirty instead of getattr for efficiency? + XBlocks outside of edx-platform can override this in order to set up their own customizable fields. """ - for field_name, value in self.upstream_settings.items(): - if field_name not in self.upstream_overridden: - if value != getattr(self, field_name): - self.upstream_overridden.append(field_name) - super().save() - - @XBlock.handler - def sync_updates(self, request: Request, suffix=None) -> Response: - """ - POST to this handler to load and apply updates from the upstream block. + return { + "display_name": "upstream_display_name", + "max_attempts": "upstream_max_attempts", + } - @@TODO describe response codes and bodies + def save(self, *args, **kwargs): """ - if request.method != "POST": - return Response(status_code=405) - if not self.upstream: - return Response("no linked upstream", response=400) - upstream_info = self.get_upstream_info() - if upstream_info.error: - return Response(upstream_info.error, status_code=400) - # @@TOD if we must pass in a user, should we use the user service instead? - # pylint: disable=protected-access - self.sync_from_upstream(user=request._request.user, apply_updates=True) - self.save() - # @@TODO why do we need to call update_item? - self.runtime.modulestore.update_item(self, request._request.user.id) - return Response(json.dumps(self.get_upstream_info_json(), indent=4)) - - def sync_from_upstream(self, *, user: User, apply_updates: bool) -> None: + Update `downstream_customized` when a customizable field is modified. """ - @@TODO docstring - - Does NOT save the block; that is left to the caller. + # Loop through all the fields that are potentially cutomizable. + for field_name, restore_field_name in self.get_customizable_fields().items(): - Raises: InvalidKeyError, UnsupportedUpstreamKeyType, XBlockNotFoundError - """ - if not self.upstream: - self.upstream_settings = {} - self.upstream_overridden = [] - self.upstream_version = None - return - upstream_key = validate_upstream_key(self.upstream) - self.upstream_settings = {} - try: - upstream_block = xblock_api.load_block(upstream_key, user) - except NotFound as exc: - raise XBlockNotFoundError( - f"failed to load upstream ({self.upstream}) for block ({self.usage_key}) for user id={user.id}" - ) from exc - for field_name, field in upstream_block.fields.items(): - if field.scope not in [Scope.settings, Scope.content]: - continue - value = getattr(upstream_block, field_name) - if field.scope == Scope.settings: - self.upstream_settings[field_name] = value - if field_name in self.upstream_overridden: - continue - if not apply_updates: + # If the field is already marked as customized, then move on so that we don't + # unneccessarily query the block for its current value. + if field_name in self.downstream_customized: continue - setattr(self, field_name, value) - self.upstream_version = self._lib_block.version_num - - def get_upstream_info_json(self) -> dict | None: - """ - @@TODO - """ - if info := self.get_upstream_info(): - as_dict = asdict(info) - as_dict["sync_available"] = info.sync_available # @@TODO - return as_dict - else: - return None - - def get_upstream_info(self) -> UpstreamInfo | None: - """ - @@TODO - """ - if not self.upstream: - return None - latest: int | None = None - error: str | None = None - try: - latest = self._lib_block.version_num - except InvalidKeyError: - error = _("Reference to linked library item is malformed: {}").format(self.upstream) - latest = None - except XBlockNotFoundError: - error = _("Linked library item was not found in the system: {}").format(self.upstream) - latest = None - return UpstreamInfo( - usage_key=self.upstream, - current_version=self.upstream_version, - latest_version=latest, - sync_url=self.runtime.handler_url(self, 'sync_updates'), - error=error, - ) - - @cached_property - def _lib_block(self) -> LibraryXBlockMetadata | None: - """ - Internal cache of the upstream library XBlock metadata, or None if there is no upstream assigned. - - We assume, for now, that upstreams are always Learning-Core-backed Content Library blocks. - That is an INTERNAL ASSUMPTION that may change at some point; hence, this is a private - property; callers should use the public API methods which don't assume that the upstream is - a from a content library. - Raises: InvalidKeyError, XBlockNotFoundError - """ - if not self.upstream: - return None - upstream_key = validate_upstream_key(self.upstream) - return get_library_block(upstream_key) + # If this field's value doesn't match the synced upstream value, then mark the field + # as customized so that we don't clobber it later when syncing. + if getattr(self, field_name) != getattr(self, restore_field_name): + self.downstream_customized.add(field_name) + + super().save(*args, **kwargs) + + +# class todoAPI: +# +# @XBlock.handler +# def sync_updates(self, request: Request, suffix=None) -> Response: +# """ +# POST to this handler to load and apply updates from the upstream block. +# +# @@TODO describe response codes and bodies +# """ +# if request.method != "POST": +# return Response(status_code=405) +# if not self.upstream: +# return Response("no linked upstream", response=400) +# upstream_info = self.inspect_upstream() +# if upstream_info.error: +# return Response(upstream_info.error, status_code=400) +# # @@TOD if we must pass in a user, should we use the user service instead? +# # pylint: disable=protected-access +# self.sync_from_upstream(user=request._request.user, apply_updates=True) +# self.save() +# # @@TODO why do we need to call update_item? +# self.runtime.modulestore.update_item(self, request._request.user.id) +# return Response(json.dumps(self.inspect_upstream_json(), indent=4))