From a205788c4d8d7c3ad4daaea19b38b5f8312dae5e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 26 Aug 2014 16:49:41 -0400 Subject: [PATCH] Work around for split not having a coherent way to work with/track updated versions of structures --- .../contentstore/tests/test_crud.py | 2 -- .../xmodule/modulestore/split_migrator.py | 17 ++++++----- .../xmodule/modulestore/split_mongo/split.py | 30 +++++++++++++++---- .../tests/test_mixed_modulestore.py | 4 +-- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py index 040268d8f8..1c09bf2f8a 100644 --- a/cms/djangoapps/contentstore/tests/test_crud.py +++ b/cms/djangoapps/contentstore/tests/test_crud.py @@ -208,8 +208,6 @@ class TemplateTests(unittest.TestCase): # The draft course root has 2 revisions: the published revision, and then the subsequent # changes to the draft revision version_history = self.split_store.get_block_generations(test_course.location) - # Base calculations on the draft revision, not the initial published revision - version_history = version_history.children[0] self.assertEqual(version_history.locator.version_guid, test_course.location.version_guid) self.assertEqual(len(version_history.children), 1) self.assertEqual(version_history.children[0].children, []) diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index be97e49d1d..452020d9a8 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -67,13 +67,16 @@ class SplitMigrator(object): **kwargs ) - with self.split_modulestore.bulk_write_operations(new_course.id): - self._copy_published_modules_to_course( - new_course, original_course.location, source_course_key, user_id, **kwargs - ) + self._copy_published_modules_to_course( + new_course, original_course.location, source_course_key, user_id, **kwargs + ) - # create a new version for the drafts - self._add_draft_modules_to_course(new_course.location, source_course_key, user_id, **kwargs) + # TODO: This should be merged back into the above transaction, but can't be until split.py + # is refactored to have more coherent access patterns + with self.split_modulestore.bulk_write_operations(new_course_key): + + # create a new version for the drafts + self._add_draft_modules_to_course(new_course.location, source_course_key, user_id, **kwargs) return new_course.id @@ -81,7 +84,7 @@ class SplitMigrator(object): """ Copy all of the modules from the 'direct' version of the course to the new split course. """ - course_version_locator = new_course.id + course_version_locator = new_course.id.replace(version_guid=None) # iterate over published course elements. Wildcarding rather than descending b/c some elements are orphaned (e.g., # course about pages, conditionals) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index e536cddb64..a0c3751cdb 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -113,10 +113,6 @@ class BulkWriteRecord(object): self.structures = {} self.structures_in_db = set() - # This stores the set of branches for whom version_structure - # has been called - self.dirty_branches = set() - @property def active(self): """ @@ -143,13 +139,37 @@ class BulkWriteRecord(object): """ return self._active_count == 1 + # TODO: This needs to track which branches have actually been modified/versioned, + # so that copying one branch to another doesn't update the original branch. + @property + def dirty_branches(self): + """ + Return a list of which branch version ids differ from what was stored + in the database at the beginning of this bulk operation. + """ + # If no course index has been set, then no branches have changed + if self.index is None: + return [] + + # If there was no index in the database to start with, then all branches + # are dirty by definition + if self.initial_index is None: + return self.index.get('versions', {}).keys() + + # Return branches whose ids differ between self.index and self.initial_index + return [ + branch + for branch, _id + in self.index.get('versions', {}).items() + if self.initial_index.get('versions', {}).get(branch) != _id + ] + def structure_for_branch(self, branch): return self.structures.get(self.index.get('versions', {}).get(branch)) def set_structure_for_branch(self, branch, structure): self.index.get('versions', {})[branch] = structure['_id'] self.structures[structure['_id']] = structure - self.dirty_branches.add(branch) def __repr__(self): return u"BulkWriteRecord<{!r}, {!r}, {!r}, {!r}, {!r}>".format( diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 4091e5adf4..a727872588 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -714,10 +714,10 @@ class TestMixedModuleStore(unittest.TestCase): # - load vertical # - load inheritance data - # TODO: LMS-11220: Document why draft send count is 5 + # TODO: LMS-11220: Document why draft send count is 6 # TODO: LMS-11220: Document why draft find count is 18 # TODO: LMS-11220: Document why split find count is 16 - @ddt.data(('draft', [19, 5], 0), ('split', [2, 2], 0)) + @ddt.data(('draft', [19, 6], 0), ('split', [2, 2], 0)) @ddt.unpack def test_path_to_location(self, default_ms, num_finds, num_sends): """