From 23d7817ea5b24c403e700c735c917fa0551c07b5 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Mon, 29 Dec 2025 23:58:31 +0500 Subject: [PATCH 1/6] fix: runtime services are fixed --- xmodule/modulestore/split_mongo/split.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index 07c985c21cc5..26fe3a692773 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -3283,7 +3283,7 @@ def create_runtime(self, course_entry, lazy): """ Create the proper runtime for this course """ - services = self.services + services = self.services.copy() # Only the CourseBlock can have user partitions. Therefore, creating the PartitionService with the library key # instead of the course key does not work. The XBlock validation in Studio fails with the following message: # "This component's access settings refer to deleted or invalid group configurations.". From 9bb369a513689f4e044c0e720090b731abc6f52c Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Tue, 30 Dec 2025 21:40:32 +0500 Subject: [PATCH 2/6] test: tests fixed by getting the refreshed blocks --- cms/djangoapps/contentstore/views/tests/test_block.py | 4 ++++ lms/djangoapps/courseware/tests/test_access.py | 3 ++- openedx/features/content_type_gating/tests/test_access.py | 4 ++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 846c62d2436a..39dbc0562701 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -1661,6 +1661,8 @@ def test_move_component_nonsensical_access_restriction_validation(self): self.assert_move_item(self.html_usage_key, self.vert2_usage_key) html.parent = self.vert2_usage_key html = self.store.update_item(html, self.user.id) + # get updated html from the store + html = self.store.get_item(html.location) validation = html.validate() self.assertEqual(len(validation.messages), 1) self._verify_validation_message( @@ -1673,6 +1675,8 @@ def test_move_component_nonsensical_access_restriction_validation(self): self.assert_move_item(self.html_usage_key, self.vert_usage_key) html.parent = self.vert_usage_key html = self.store.update_item(html, self.user.id) + # get updated html from the store + html = self.store.get_item(html.location) validation = html.validate() self.assertEqual(len(validation.messages), 0) diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index c763a7577acb..cfd691fb1362 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -322,9 +322,10 @@ def test_has_access_with_content_groups(self): self.course.cohort_config = {'cohorted': True} chapter = BlockFactory.create(category="chapter", parent_location=self.course.location) - chapter.group_access = {partition_id: [group_0_id]} modulestore().update_item(self.course, ModuleStoreEnum.UserID.test) + chapter = self.store.get_item(chapter.location) + chapter.group_access = {partition_id: [group_0_id]} # User should not be able to preview when masquerading as student (and not in the group above). with patch('lms.djangoapps.courseware.access.get_user_role') as mock_user_role: diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py index 3e3dae129801..c7562d437043 100644 --- a/openedx/features/content_type_gating/tests/test_access.py +++ b/openedx/features/content_type_gating/tests/test_access.py @@ -1180,6 +1180,10 @@ def test_content_type_gate_for_block(self): metadata=METADATA, ) + # get updated blocks + blocks_dict['graded_1'] = self.store.get_item(blocks_dict['graded_1'].location) + blocks_dict['not_graded_1'] = self.store.get_item(blocks_dict['not_graded_1'].location) + # The method returns a content type gate for blocks that should be gated assert 'content-paywall' in ContentTypeGatingService()._content_type_gate_for_block( # pylint: disable=protected-access self.user, blocks_dict['graded_1'], course['course'].id From 9de2ae3caa09a5f94d328193f93867855af7125f Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Mon, 5 Jan 2026 15:35:43 +0500 Subject: [PATCH 3/6] fix: deep copy services object --- xmodule/modulestore/split_mongo/split.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index 26fe3a692773..ecd6e25b3db4 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -3283,7 +3283,7 @@ def create_runtime(self, course_entry, lazy): """ Create the proper runtime for this course """ - services = self.services.copy() + services = copy.deepcopy(self.services) # Only the CourseBlock can have user partitions. Therefore, creating the PartitionService with the library key # instead of the course key does not work. The XBlock validation in Studio fails with the following message: # "This component's access settings refer to deleted or invalid group configurations.". From 674e3b34592ada22ff73c0cd9a8a6a643a62777b Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Wed, 7 Jan 2026 21:57:48 +0500 Subject: [PATCH 4/6] revert: revert back to shallow copy --- xmodule/modulestore/split_mongo/split.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index ecd6e25b3db4..26fe3a692773 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -3283,7 +3283,7 @@ def create_runtime(self, course_entry, lazy): """ Create the proper runtime for this course """ - services = copy.deepcopy(self.services) + services = self.services.copy() # Only the CourseBlock can have user partitions. Therefore, creating the PartitionService with the library key # instead of the course key does not work. The XBlock validation in Studio fails with the following message: # "This component's access settings refer to deleted or invalid group configurations.". From 7a5c1e5b8fb9b79e4d5d2a8c7cffa3a28fbc7580 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 7 Jan 2026 18:38:50 -0500 Subject: [PATCH 5/6] fix: make PartitionService behave across multiple runtimes --- .../contentstore/views/tests/test_block.py | 4 -- .../courseware/tests/test_access.py | 3 +- .../content_type_gating/tests/test_access.py | 4 -- xmodule/partitions/partitions_service.py | 43 ++++++++++++++++++- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 39dbc0562701..846c62d2436a 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -1661,8 +1661,6 @@ def test_move_component_nonsensical_access_restriction_validation(self): self.assert_move_item(self.html_usage_key, self.vert2_usage_key) html.parent = self.vert2_usage_key html = self.store.update_item(html, self.user.id) - # get updated html from the store - html = self.store.get_item(html.location) validation = html.validate() self.assertEqual(len(validation.messages), 1) self._verify_validation_message( @@ -1675,8 +1673,6 @@ def test_move_component_nonsensical_access_restriction_validation(self): self.assert_move_item(self.html_usage_key, self.vert_usage_key) html.parent = self.vert_usage_key html = self.store.update_item(html, self.user.id) - # get updated html from the store - html = self.store.get_item(html.location) validation = html.validate() self.assertEqual(len(validation.messages), 0) diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index cfd691fb1362..c763a7577acb 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -322,10 +322,9 @@ def test_has_access_with_content_groups(self): self.course.cohort_config = {'cohorted': True} chapter = BlockFactory.create(category="chapter", parent_location=self.course.location) + chapter.group_access = {partition_id: [group_0_id]} modulestore().update_item(self.course, ModuleStoreEnum.UserID.test) - chapter = self.store.get_item(chapter.location) - chapter.group_access = {partition_id: [group_0_id]} # User should not be able to preview when masquerading as student (and not in the group above). with patch('lms.djangoapps.courseware.access.get_user_role') as mock_user_role: diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py index c7562d437043..3e3dae129801 100644 --- a/openedx/features/content_type_gating/tests/test_access.py +++ b/openedx/features/content_type_gating/tests/test_access.py @@ -1180,10 +1180,6 @@ def test_content_type_gate_for_block(self): metadata=METADATA, ) - # get updated blocks - blocks_dict['graded_1'] = self.store.get_item(blocks_dict['graded_1'].location) - blocks_dict['not_graded_1'] = self.store.get_item(blocks_dict['not_graded_1'].location) - # The method returns a content type gate for blocks that should be gated assert 'content-paywall' in ContentTypeGatingService()._content_type_gate_for_block( # pylint: disable=protected-access self.user, blocks_dict['graded_1'], course['course'].id diff --git a/xmodule/partitions/partitions_service.py b/xmodule/partitions/partitions_service.py index 6cffd2c20c7b..ddd37d5212f5 100644 --- a/xmodule/partitions/partitions_service.py +++ b/xmodule/partitions/partitions_service.py @@ -99,8 +99,47 @@ class PartitionService: with a given course. """ - def __init__(self, course_id, cache=None, course=None): - self._course_id = course_id + def __init__(self, course_id: CourseKey, cache=None, course=None): + """Create a new ParititonService. This is user-specific.""" + + # There is a surprising amount of complexity in how to save the + # course_id we were passed in this constructor. + if course_id.org and course_id.course and course_id.run: + # This is the normal case, where we're instantiated with a CourseKey + # that has org, course, and run information. It will also often have + # a version_guid attached in this case, and we will want to strip + # that off in most cases. + # + # The reason for this is that the PartitionService is going to get + # recreated for every runtime (i.e. every block that's created for a + # user). Say you do the following: + # + # 1. You query the modulestore's get_item() for block A. + # 2. You update_item() for a different block B + # 3. You publish block B. + # + # When get_item() was called, a SplitModuleStoreRuntime was created + # for block A and it was given a CourseKey that had the version_guid + # encoded in it. If we persist that CourseKey with the version guid + # intact, then it will be incorrect after B is published, and any + # future access checks on A will break because it will try to query + # for a version of the course that is no longer published. + # + # Note that we still need to keep the branch information, or else + # this wouldn't work right in preview mode. + self._course_id = course_id.replace(version_guid=None) + else: + # If we're here, it means that the CourseKey we were sent doesn't + # have an org, course, and run. A much less common (but still legal) + # way to query by CourseKey involves a version_guid-only query, i.e. + # everything is None but the version_guid. In this scenario, it + # doesn't make sense to remove the one identifying piece of + # information we have, so we just assign the CourseKey without + # modification. We *could* potentially query the modulestore + # here and get the more normal form of the CourseKey, but that would + # be much more expensive and require database access. + self._course_id = course_id + self._cache = cache self.course = course From 6b3624b6449af394cd071a776fe9f0bf8aeeac75 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Thu, 8 Jan 2026 19:53:31 +0500 Subject: [PATCH 6/6] docs: brief comment added on why we added this copy() --- xmodule/modulestore/split_mongo/split.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index 26fe3a692773..74539fabed90 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -3283,6 +3283,10 @@ def create_runtime(self, course_entry, lazy): """ Create the proper runtime for this course """ + # A single SplitMongoModuleStore may create many SplitModuleStoreRuntimes, + # each of which will later modify its internal dict of services on a per-item and often per-user basis. + # Therefore, it's critical that we make a new copy of our baseline services dict here, + # so that each runtime is free to add and replace its services without impacting other runtimes. services = self.services.copy() # Only the CourseBlock can have user partitions. Therefore, creating the PartitionService with the library key # instead of the course key does not work. The XBlock validation in Studio fails with the following message: