diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 8cf1f317283..b3a20f6c92b 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -896,14 +896,15 @@ def _create_copy_content_task(v2_library_key, v1_library_key): spin up a celery task to import the V1 Library's content into the V2 library. This utalizes the fact that course and v1 library content is stored almost identically. """ - return v2contentlib_api.import_blocks_create_task(v2_library_key, v1_library_key) + return v2contentlib_api.import_blocks_create_task( + v2_library_key, v1_library_key, + use_course_key_as_block_id_suffix=False + ) def _create_metadata(v1_library_key, collection_uuid): """instansiate an index for the V2 lib in the collection""" - print(collection_uuid) - store = modulestore() v1_library = store.get_library(v1_library_key) collection = get_collection(collection_uuid).uuid diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 76cbde89082..ebd25f54ddd 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1231,12 +1231,13 @@ class BaseEdxImportClient(abc.ABC): "video", } - def __init__(self, library_key=None, library=None): + def __init__(self, library_key=None, library=None, use_course_key_as_block_id_suffix=True): """ Initialize an import client for a library. The method accepts either a library object or a key to a library object. """ + self.use_course_key_as_block_id_suffix = use_course_key_as_block_id_suffix if bool(library_key) == bool(library): raise ValueError('Provide at least one of `library_key` or ' '`library`, but not both.') @@ -1278,8 +1279,18 @@ def import_block(self, modulestore_key): str(modulestore_key.course_key).encode() ).digest() )[:16].decode().lower() - # Prepend 'c' to allow changing hash without conflicts. - block_id = f"{modulestore_key.block_id}_c{course_key_id}" + + # add the course_key_id if use_course_key_as_suffix is enabled to increase the namespace. + # The option exists to not use the course key as a suffix because + # in order to preserve learner state in the v1 to v2 libraries migration, + # the v2 and v1 libraries' child block ids must be the same. + block_id = ( + # Prepend 'c' to allow changing hash without conflicts. + f"{modulestore_key.block_id}_c{course_key_id}" + if self.use_course_key_as_block_id_suffix + else f"{modulestore_key.block_id}" + ) + log.info('Importing to library block: id=%s', block_id) try: library_block = create_library_block( @@ -1490,7 +1501,7 @@ def _call(self, method, *args, **kwargs): return response -def import_blocks_create_task(library_key, course_key): +def import_blocks_create_task(library_key, course_key, use_course_key_as_block_id_suffix=True): """ Create a new import block task. @@ -1503,7 +1514,7 @@ def import_blocks_create_task(library_key, course_key): course_id=course_key, ) result = tasks.import_blocks_from_course.apply_async( - args=(import_task.pk, str(course_key)) + args=(import_task.pk, str(course_key), use_course_key_as_block_id_suffix) ) log.info(f"Import block task created: import_task={import_task} " f"celery_task={result.id}") diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index 3c259aaba95..13a6a32a198 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -17,7 +17,6 @@ from __future__ import annotations import logging -import hashlib from celery import shared_task from celery_utils.logged_task import LoggedTask @@ -28,7 +27,9 @@ from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import ( BlockUsageLocator, - LibraryUsageLocatorV2 + LibraryLocatorV2, + LibraryUsageLocatorV2, + LibraryLocator as LibraryLocatorV1 ) from user_tasks.tasks import UserTask, UserTaskStatus @@ -45,6 +46,8 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.mixed import MixedModuleStore +from xmodule.modulestore.split_mongo import BlockKey +from xmodule.modulestore.store_utilities import derived_key from . import api from .models import ContentLibraryBlockImportTask @@ -55,7 +58,7 @@ @shared_task(base=LoggedTask) @set_code_owner_attribute -def import_blocks_from_course(import_task_id, course_key_str): +def import_blocks_from_course(import_task_id, course_key_str, use_course_key_as_block_id_suffix=True): """ A Celery task to import blocks from a course through modulestore. """ @@ -72,7 +75,10 @@ def on_progress(block_key, block_num, block_count, exception=None): logger.info('Import block succesful: %s', block_key) import_task.save_progress(block_num / block_count) - edx_client = api.EdxModulestoreImportClient(library=import_task.library) + edx_client = api.EdxModulestoreImportClient( + library=import_task.library, + use_course_key_as_block_id_suffix=use_course_key_as_block_id_suffix + ) edx_client.import_blocks_from_course( course_key, on_progress ) @@ -84,14 +90,27 @@ def _import_block(store, user_id, source_block, dest_parent_key): """ def generate_block_key(source_key, dest_parent_key): """ - Deterministically generate an ID for the new block and return the key + Deterministically generate an ID for the new block and return the key. + Keys are generated such that they appear identical to a v1 library with + the same input block_id, library name, library organization, and parent block using derived_key """ - block_id = ( - dest_parent_key.block_id[:10] + - '-' + - hashlib.sha1(str(source_key).encode('utf-8')).hexdigest()[:10] + if not isinstance(source_key.lib_key, LibraryLocatorV2): + raise TypeError(f"Expected source library key of type LibraryLocatorV2, got {source_key.lib_key} instead.") + source_key_as_v1_course_key = LibraryLocatorV1( + org=source_key.lib_key.org, + library=source_key.lib_key.slug, + branch='library' + ) + source_key_usage_id_as_block_key = BlockKey( + type=source_key.block_type, + id=source_key.usage_id, + ) + block_id = derived_key( + source_key_as_v1_course_key, + source_key_usage_id_as_block_key, + BlockKey(dest_parent_key.block_type, dest_parent_key.block_id) ) - return dest_parent_key.context_key.make_usage_key(source_key.block_type, block_id) + return dest_parent_key.context_key.make_usage_key(source_key.block_type, block_id.id) source_key = source_block.scope_ids.usage_id new_block_key = generate_block_key(source_key, dest_parent_key)