From 4ee990b0342800cdfa934e916f82745ae5fd5b94 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Wed, 5 Jun 2024 11:27:02 -0400 Subject: [PATCH] feat: extract save_xblock_to_clipboard api function (WIP - unlinted) --- .../core/djangoapps/content_staging/api.py | 139 +++++++++++++++++- .../core/djangoapps/content_staging/views.py | 126 +--------------- 2 files changed, 144 insertions(+), 121 deletions(-) diff --git a/openedx/core/djangoapps/content_staging/api.py b/openedx/core/djangoapps/content_staging/api.py index d2343847b58b..631d6f1c6400 100644 --- a/openedx/core/djangoapps/content_staging/api.py +++ b/openedx/core/djangoapps/content_staging/api.py @@ -3,13 +3,148 @@ """ from __future__ import annotations +import hashlib +import logging + from django.http import HttpRequest +from django.core.files.base import ContentFile +from django.db import transaction +from django.http import HttpResponse from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import AssetKey, UsageKey +from opaque_keys.edx.locator import CourseLocator +from rest_framework.exceptions import NotFound, PermissionDenied, ValidationError +from rest_framework.response import Response +from rest_framework.views import APIView + +from common.djangoapps.student.auth import has_studio_read_access +from openedx.core.lib.api.view_utils import view_auth_classes +from openedx.core.lib.xblock_serializer.api import serialize_xblock_to_olx, StaticFile +from xmodule import block_metadata_utils +from xmodule.contentstore.content import StaticContent +from xmodule.contentstore.django import contentstore +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError -from .data import StagedContentData, StagedContentFileData, StagedContentStatus, UserClipboardData -from .models import UserClipboard as _UserClipboard, StagedContent as _StagedContent +from .data import ( + CLIPBOARD_PURPOSE, + StagedContentData, StagedContentFileData, StagedContentStatus, UserClipboardData +) +from .models import ( + UserClipboard as _UserClipboard, + StagedContent as _StagedContent, + StagedContentFile as _StagedContentFile, +) from .serializers import UserClipboardSerializer as _UserClipboardSerializer +from .tasks import delete_expired_clipboards + +log = logging.getLogger(__name__) + + + +def save_xblock_to_user_clipboard(block: XBlock, user_id: int) -> UserClipboardData: + """ + TODO + """ + block_data = serialize_xblock_to_olx(block) + usage_key = block.usage_key + + expired_ids = [] + with transaction.atomic(): + # Mark all of the user's existing StagedContent rows as EXPIRED + to_expire = _StagedContent.objects.filter( + user_id=user_id, + purpose=CLIPBOARD_PURPOSE, + ).exclude( + status=StagedContentStatus.EXPIRED, + ) + for sc in to_expire: + expired_ids.append(sc.id) + sc.status = StagedContentStatus.EXPIRED + sc.save() + # Insert a new StagedContent row for this + staged_content = _StagedContent.objects.create( + user_id=user_id, + purpose=CLIPBOARD_PURPOSE, + status=StagedContentStatus.READY, + block_type=usage_key.block_type, + olx=block_data.olx_str, + display_name=block_metadata_utils.display_name_with_default(block), + suggested_url_name=usage_key.block_id, + tags=block_data.tags, + ) + (clipboard, _created) = _UserClipboard.objects.update_or_create(user_id=user_id, defaults={ + "content": staged_content, + "source_usage_key": usage_key, + }) + + # Log an event so we can analyze how this feature is used: + log.info(f"Copied {usage_key.block_type} component \"{usage_key}\" to their clipboard.") + + # Try to copy the static files. If this fails, we still consider the overall copy attempt to have succeeded, + # because intra-course pasting will still work fine, and in any case users can manually resolve the file issue. + try: + _save_static_assets_to_user_clipboard(block_data.static_files, usage_key, staged_content) + except Exception: # pylint: disable=broad-except + # Regardless of what happened, with get_asset_key_from_path or contentstore or run_filter, we don't want the + # whole "copy to clipboard" operation to fail, which would be a bad user experience. For copying and pasting + # within a single course, static assets don't even matter. So any such errors become warnings here. + log.exception(f"Unable to copy static files to clipboard for component {usage_key}") + + # Enqueue a (potentially slow) task to delete the old staged content + try: + delete_expired_clipboards.delay(expired_ids) + except Exception: # pylint: disable=broad-except + log.exception(f"Unable to enqueue cleanup task for StagedContents: {','.join(str(x) for x in expired_ids)}") + + return clipboard + +def _save_static_assets_to_user_clipboard( + static_files: list[StaticFile], usage_key: UsageKey, staged_content: _StagedContent +): + """ + Helper method for save_xblock_to_user_clipboard endpoint. This deals with copying static files into the clipboard. + """ + for f in static_files: + source_key = ( + StaticContent.get_asset_key_from_path(usage_key.context_key, f.url) + if (f.url and f.url.startswith('/')) else None + ) + # Compute the MD5 hash and get the content: + content: bytes | None = f.data + md5_hash = "" # Unknown + if content: + md5_hash = hashlib.md5(content).hexdigest() + # This asset came from the XBlock's filesystem, e.g. a video block's transcript file + source_key = usage_key + # Check if the asset file exists. It can be absent if an XBlock contains an invalid link. + elif source_key and (sc := contentstore().find(source_key, throw_on_not_found=False)): + md5_hash = sc.content_digest + content = sc.data + else: + continue # Skip this file - we don't need a reference to a non-existent file. + + # Because we store clipboard files on S3, uploading really large files will be too slow. And it's wasted if + # the copy-paste is just happening within a single course. So for files > 10MB, users must copy the files + # manually. In the future we can consider removing this or making it configurable or filterable. + limit = 10 * 1024 * 1024 + if content and len(content) > limit: + content = None + + try: + _StagedContentFile.objects.create( + for_content=staged_content, + filename=f.name, + # In some cases (e.g. really large files), we don't store the data here but we still keep track of + # the metadata. You can still use the metadata to determine if the file is already present or not, + # and then either inform the user or find another way to import the file (e.g. if the file still + # exists in the "Files & Uploads" contentstore of the source course, based on source_key_str). + data_file=ContentFile(content=content, name=f.name) if content else None, + source_key_str=str(source_key) if source_key else "", + md5_hash=md5_hash, + ) + except Exception: # pylint: disable=broad-except + log.exception(f"Unable to copy static file {f.name} to clipboard for component {usage_key}") def get_user_clipboard(user_id: int, only_ready: bool = True) -> UserClipboardData | None: diff --git a/openedx/core/djangoapps/content_staging/views.py b/openedx/core/djangoapps/content_staging/views.py index f7bc040e0668..d9680e004534 100644 --- a/openedx/core/djangoapps/content_staging/views.py +++ b/openedx/core/djangoapps/content_staging/views.py @@ -2,10 +2,7 @@ REST API views for content staging """ from __future__ import annotations -import hashlib -import logging -from django.core.files.base import ContentFile from django.db import transaction from django.http import HttpResponse from django.shortcuts import get_object_or_404 @@ -27,12 +24,9 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError -from .data import CLIPBOARD_PURPOSE, StagedContentStatus -from .models import StagedContent, StagedContentFile, UserClipboard +from . import api +from .data import StagedContentStatus from .serializers import UserClipboardSerializer, PostToClipboardSerializer -from .tasks import delete_expired_clipboards - -log = logging.getLogger(__name__) @view_auth_classes(is_authenticated=True) @@ -75,18 +69,7 @@ def get(self, request): """ Get the detailed status of the user's clipboard. This does not return the OLX. """ - try: - clipboard = UserClipboard.objects.get(user=request.user.id) - except UserClipboard.DoesNotExist: - # This user does not have any content on their clipboard. - return Response({ - "content": None, - "source_usage_key": "", - "source_context_title": "", - "source_edit_url": "", - }) - serializer = UserClipboardSerializer(clipboard, context={"request": request}) - return Response(serializer.data) + return api.get_user_clipboard_jsonr(request.user.id, request) @apidocs.schema( body=PostToClipboardSerializer, @@ -116,108 +99,13 @@ def post(self, request): if not has_studio_read_access(request.user, course_key): raise PermissionDenied("You must be a member of the course team in Studio to export OLX using this API.") - # Get the OLX of the content + # Load the block and copy it to the user's clipboard try: block = modulestore().get_item(usage_key) except ItemNotFoundError as exc: raise NotFound("The requested usage key does not exist.") from exc - block_data = serialize_xblock_to_olx(block) - - expired_ids = [] - with transaction.atomic(): - # Mark all of the user's existing StagedContent rows as EXPIRED - to_expire = StagedContent.objects.filter( - user=request.user, - purpose=CLIPBOARD_PURPOSE, - ).exclude( - status=StagedContentStatus.EXPIRED, - ) - for sc in to_expire: - expired_ids.append(sc.id) - sc.status = StagedContentStatus.EXPIRED - sc.save() - # Insert a new StagedContent row for this - staged_content = StagedContent.objects.create( - user=request.user, - purpose=CLIPBOARD_PURPOSE, - status=StagedContentStatus.READY, - block_type=usage_key.block_type, - olx=block_data.olx_str, - display_name=block_metadata_utils.display_name_with_default(block), - suggested_url_name=usage_key.block_id, - tags=block_data.tags, - ) - (clipboard, _created) = UserClipboard.objects.update_or_create(user=request.user, defaults={ - "content": staged_content, - "source_usage_key": usage_key, - }) - # Return the current clipboard exactly as if GET was called: - serializer = UserClipboardSerializer(clipboard, context={"request": request}) - # Log an event so we can analyze how this feature is used: - log.info(f"Copied {usage_key.block_type} component \"{usage_key}\" to their clipboard.") - - # Try to copy the static files. If this fails, we still consider the overall copy attempt to have succeeded, - # because intra-course pasting will still work fine, and in any case users can manually resolve the file issue. - try: - self._save_static_assets_to_clipboard(block_data.static_files, usage_key, staged_content) - except Exception: # pylint: disable=broad-except - # Regardless of what happened, with get_asset_key_from_path or contentstore or run_filter, we don't want the - # whole "copy to clipboard" operation to fail, which would be a bad user experience. For copying and pasting - # within a single course, static assets don't even matter. So any such errors become warnings here. - log.exception(f"Unable to copy static files to clipboard for component {usage_key}") + clipboard = api.save_xblock_to_user_clipboard(block=block, user_id=request.user.id) - # Enqueue a (potentially slow) task to delete the old staged content - try: - delete_expired_clipboards.delay(expired_ids) - except Exception: # pylint: disable=broad-except - log.exception(f"Unable to enqueue cleanup task for StagedContents: {','.join(str(x) for x in expired_ids)}") - # Return the response: + # Return the current clipboard exactly as if GET was called: + serializer = UserClipboardSerializer(clipboard, context={"request": request}) return Response(serializer.data) - - @staticmethod - def _save_static_assets_to_clipboard( - static_files: list[StaticFile], usage_key: UsageKey, staged_content: StagedContent - ): - """ - Helper method for "post to clipboard" API endpoint. This deals with copying static files into the clipboard. - """ - for f in static_files: - source_key = ( - StaticContent.get_asset_key_from_path(usage_key.context_key, f.url) - if (f.url and f.url.startswith('/')) else None - ) - # Compute the MD5 hash and get the content: - content: bytes | None = f.data - md5_hash = "" # Unknown - if content: - md5_hash = hashlib.md5(content).hexdigest() - # This asset came from the XBlock's filesystem, e.g. a video block's transcript file - source_key = usage_key - # Check if the asset file exists. It can be absent if an XBlock contains an invalid link. - elif source_key and (sc := contentstore().find(source_key, throw_on_not_found=False)): - md5_hash = sc.content_digest - content = sc.data - else: - continue # Skip this file - we don't need a reference to a non-existent file. - - # Because we store clipboard files on S3, uploading really large files will be too slow. And it's wasted if - # the copy-paste is just happening within a single course. So for files > 10MB, users must copy the files - # manually. In the future we can consider removing this or making it configurable or filterable. - limit = 10 * 1024 * 1024 - if content and len(content) > limit: - content = None - - try: - StagedContentFile.objects.create( - for_content=staged_content, - filename=f.name, - # In some cases (e.g. really large files), we don't store the data here but we still keep track of - # the metadata. You can still use the metadata to determine if the file is already present or not, - # and then either inform the user or find another way to import the file (e.g. if the file still - # exists in the "Files & Uploads" contentstore of the source course, based on source_key_str). - data_file=ContentFile(content=content, name=f.name) if content else None, - source_key_str=str(source_key) if source_key else "", - md5_hash=md5_hash, - ) - except Exception: # pylint: disable=broad-except - log.exception(f"Unable to copy static file {f.name} to clipboard for component {usage_key}")