From dc4d9a8b9bf988b824615a8bcdd0da741aeb4585 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 5 Nov 2026 11:54:59 +0530 Subject: [PATCH 1/5] OpenConceptLab/ocl_issues#3010 | Version create takes an expansion choice: copy, generate or none --- core/collections/constants.py | 4 ++ core/collections/serializers.py | 13 ++++++- core/collections/tests/tests.py | 2 +- core/common/models.py | 11 ++++-- core/common/tasks.py | 6 +-- core/integration_tests/tests_collections.py | 41 +++++++++++++++++++++ core/sources/tests/tests.py | 4 +- 7 files changed, 69 insertions(+), 12 deletions(-) diff --git a/core/collections/constants.py b/core/collections/constants.py index a6f5e4206..d877cabc0 100644 --- a/core/collections/constants.py +++ b/core/collections/constants.py @@ -37,3 +37,7 @@ UNRESOLVED_RESOURCE_VERSION_TYPE = 'reference.resource_version' INTENSIONAL_REFERENCE_RESOURCE_VERSIONS_TOGGLE = 'INTENSIONAL_REFERENCE_RESOURCE_VERSIONS_TOGGLE' EXPANSION_LOCKED_REPO_VERSION_TOGGLE = 'EXPANSION_LOCKED_REPO_VERSION_TOGGLE' +VERSION_EXPANSION_COPY = 'copy' +VERSION_EXPANSION_GENERATE = 'generate' +VERSION_EXPANSION_NONE = 'none' +VERSION_EXPANSION_CHOICES = (VERSION_EXPANSION_COPY, VERSION_EXPANSION_GENERATE, VERSION_EXPANSION_NONE) diff --git a/core/collections/serializers.py b/core/collections/serializers.py index ad4da25aa..a7ddd4029 100644 --- a/core/collections/serializers.py +++ b/core/collections/serializers.py @@ -4,11 +4,13 @@ from django.core.validators import RegexValidator from pydash import get from rest_framework.fields import CharField, ListField, IntegerField, DateTimeField, JSONField, \ - BooleanField, SerializerMethodField + BooleanField, SerializerMethodField, ChoiceField from rest_framework.relations import PrimaryKeyRelatedField from rest_framework.serializers import ModelSerializer, Serializer, ValidationError from core.client_configs.serializers import ClientConfigSerializer +from core.collections.constants import VERSION_EXPANSION_CHOICES, VERSION_EXPANSION_NONE, \ + VERSION_EXPANSION_GENERATE from core.collections.models import Collection, CollectionReference, Expansion from core.common.constants import HEAD, DEFAULT_ACCESS_TYPE, NAMESPACE_REGEX, INCLUDE_SUMMARY, \ INCLUDE_CLIENT_CONFIGS, INVALID_EXPANSION_URL, INCLUDE_STATES, INCLUDE_TASKS, INCLUDE_RESOLVED_REPO_VERSIONS, \ @@ -264,6 +266,7 @@ class CollectionCreateSerializer(CollectionCreateOrUpdateSerializer): locked_date = DateTimeField(required=False, allow_null=True) autoexpand_head = BooleanField(required=False, default=True) autoexpand = BooleanField(required=False, default=True) + expansion = ChoiceField(choices=VERSION_EXPANSION_CHOICES, required=False, write_only=True) def validate(self, attrs): # create/create_version consume the raw payload (not validated_data), so DRF's boolean @@ -287,7 +290,13 @@ def create(self, validated_data): def create_version(self, validated_data): collection = self.prepare_object(validated_data) user = self.context['request'].user - errors = Collection.persist_new_version(collection, user) + expansion = validated_data.get('expansion') + if expansion == VERSION_EXPANSION_NONE: + collection.autoexpand = False + elif expansion: + collection.autoexpand = True + errors = Collection.persist_new_version( + collection, user, seed_from_head=expansion != VERSION_EXPANSION_GENERATE) self._errors.update(errors) return collection diff --git a/core/collections/tests/tests.py b/core/collections/tests/tests.py index ef33b9285..2d2e5ebb5 100644 --- a/core/collections/tests/tests.py +++ b/core/collections/tests/tests.py @@ -2546,7 +2546,7 @@ def test_persist_new_version_sync_seeds_inline(self, seed_mock): with self.captureOnCommitCallbacks(execute=False) as callbacks: Collection.persist_new_version(collection_v1, collection.created_by, sync=True) - seed_mock.assert_called_once_with('collection', collection_v1.id, True, True) + seed_mock.assert_called_once_with('collection', collection_v1.id, True, True, True) self.assertEqual(callbacks, []) @override_settings(TEST_MODE=False) diff --git a/core/common/models.py b/core/common/models.py index 527f76ca8..4fb48a73e 100644 --- a/core/common/models.py +++ b/core/common/models.py @@ -1250,6 +1250,7 @@ def persist_new_version(cls, obj, user=None, **kwargs): obj.is_active = True sync = kwargs.pop('sync', False) + seed_from_head = kwargs.pop('seed_from_head', True) if user: obj.created_by = user obj.updated_by = user @@ -1262,11 +1263,12 @@ def persist_new_version(cls, obj, user=None, **kwargs): is_test_mode = get(settings, 'TEST_MODE', False) # a synchronous seed from HEAD runs in this transaction, so it needs this transaction's snapshot is_repeatable_read = sync and not is_test_mode and not transaction.get_connection().in_atomic_block and \ - obj.can_seed_from_head_expansion() + seed_from_head and obj.can_seed_from_head_expansion() attempts = SERIALIZATION_RETRY_ATTEMPTS if is_repeatable_read else 1 for attempt in range(1, attempts + 1): try: - cls.persist_new_version_in_transaction(obj, head, user, sync, is_repeatable_read, **kwargs) + cls.persist_new_version_in_transaction( + obj, head, user, sync, is_repeatable_read, seed_from_head, **kwargs) break except OperationalError as ex: if attempt == attempts or not cls.is_serialization_failure(ex): @@ -1278,7 +1280,8 @@ def persist_new_version(cls, obj, user=None, **kwargs): return errors @classmethod - def persist_new_version_in_transaction(cls, obj, head, user, sync, is_repeatable_read, **kwargs): # pylint: disable=too-many-arguments + def persist_new_version_in_transaction( # pylint: disable=too-many-arguments + cls, obj, head, user, sync, is_repeatable_read, seed_from_head=True, **kwargs): is_test_mode = get(settings, 'TEST_MODE', False) with transaction.atomic(): if is_repeatable_read: @@ -1292,7 +1295,7 @@ def persist_new_version_in_transaction(cls, obj, head, user, sync, is_repeatable if obj.id: obj.sibling_versions.update(is_latest_version=False) - task_args = (obj.resource_type.lower(), obj.id, not is_test_mode, sync) + task_args = (obj.resource_type.lower(), obj.id, not is_test_mode, sync, seed_from_head) if is_test_mode or sync: seed_children_to_new_version(*task_args) else: diff --git a/core/common/tasks.py b/core/common/tasks.py index aacccd26e..12be650ae 100644 --- a/core/common/tasks.py +++ b/core/common/tasks.py @@ -384,7 +384,7 @@ def send_user_reset_password_email(user_id): @app.task(bind=True) -def seed_children_to_new_version(self, resource, obj_id, export=True, sync=False): # pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements +def seed_children_to_new_version(self, resource, obj_id, export=True, sync=False, seed_from_head=True): # pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements export_task = None changelog_task = None autoexpand = True @@ -418,8 +418,8 @@ def seed_children_to_new_version(self, resource, obj_id, export=True, sync=False instance.snapshot = CollectionDetailSerializer(head).data instance.save(update_fields=['snapshot']) instance.get_checksums(recalculate=True) - seed_from_head = is_collection and autoexpand and instance.can_seed_from_head_expansion() and \ - instance.seed_from_head_expansion() + seed_from_head = seed_from_head and is_collection and autoexpand and \ + instance.can_seed_from_head_expansion() and instance.seed_from_head_expansion() if not seed_from_head: instance.seed_references() if is_source: diff --git a/core/integration_tests/tests_collections.py b/core/integration_tests/tests_collections.py index 84bffb6f5..dbcc990b4 100644 --- a/core/integration_tests/tests_collections.py +++ b/core/integration_tests/tests_collections.py @@ -3311,6 +3311,47 @@ def test_post_201(self): self.assertEqual(expansion.concepts.count(), 1) self.assertEqual(expansion.mappings.count(), 0) + def test_post_201_expansion_choice(self): + seed_from_head = Collection.seed_from_head_expansion + cases = [ + (True, None, True, True), (True, 'copy', True, True), (True, 'generate', False, True), + (True, 'none', False, False), (False, None, False, True), (False, 'copy', False, True), + (False, 'generate', False, True), (False, 'none', False, False), + ] + for index, (is_locked, choice, copies, expands) in enumerate(cases): + with self.subTest(is_locked=is_locked, choice=choice), \ + patch.object(CollectionReference, 'is_expansion_locked_repo_version_on', return_value=is_locked), \ + patch.object( + Collection, 'seed_from_head_expansion', autospec=True, side_effect=seed_from_head + ) as seed_mock: + payload = {'id': f'v{index}', 'description': 'version'} + if choice: + payload['expansion'] = choice + response = self.client.post( + self.collection.uri + 'versions/', payload, + HTTP_AUTHORIZATION='Token ' + self.token, format='json') + + self.assertEqual(response.status_code, 201) + version = self.collection.versions.get(version=f'v{index}') + self.assertEqual(seed_mock.called, copies) + self.assertEqual(version.autoexpand, expands) + self.assertEqual(version.references.count(), 1) + if expands: + self.assertEqual(version.expansions.count(), 1) + self.assertEqual(version.expansion.concepts.count(), 1) + else: + self.assertEqual(version.expansions.count(), 0) + self.assertIsNone(version.expansion_uri) + + def test_post_400_expansion_choice_invalid(self): + response = self.client.post( + self.collection.uri + 'versions/', {'id': 'v1', 'description': 'version1', 'expansion': 'rebuild'}, + HTTP_AUTHORIZATION='Token ' + self.token, format='json') + + self.assertEqual(response.status_code, 400) + self.assertIn('expansion', response.data) + self.assertEqual(self.collection.versions.count(), 1) + def test_post_201_uses_head_name_when_older_version_exists(self): # Regression for the same class of bug fixed on SourceVersionListView.create(): resolving # "head_object" via get_queryset().first() (ordered by -created_at, no version filter) would diff --git a/core/sources/tests/tests.py b/core/sources/tests/tests.py index 351a06e91..80b3e59ab 100644 --- a/core/sources/tests/tests.py +++ b/core/sources/tests/tests.py @@ -942,9 +942,9 @@ def test_persist_new_version_registers_seed_task_before_enqueue(self, apply_asyn Source.persist_new_version(source_version, source.created_by) task = Task.objects.get(name='seed_children_to_new_version') - self.assertEqual(task.args, ['source', source_version.id, True, False]) + self.assertEqual(task.args, ['source', source_version.id, True, False, True]) apply_async.assert_called_once_with( - ('source', source_version.id, True, False), + ('source', source_version.id, True, False, True), task_id=task.id, queue='default', persist_args=True, From f9266a52842e2c985b17a1356e32038b32f89d0c Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 5 Nov 2026 12:00:04 +0530 Subject: [PATCH 2/5] OpenConceptLab/ocl_issues#3010 | Tests for autoexpand-only version create requests --- core/integration_tests/tests_collections.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/core/integration_tests/tests_collections.py b/core/integration_tests/tests_collections.py index dbcc990b4..5b59cb1cf 100644 --- a/core/integration_tests/tests_collections.py +++ b/core/integration_tests/tests_collections.py @@ -3314,12 +3314,15 @@ def test_post_201(self): def test_post_201_expansion_choice(self): seed_from_head = Collection.seed_from_head_expansion cases = [ - (True, None, True, True), (True, 'copy', True, True), (True, 'generate', False, True), - (True, 'none', False, False), (False, None, False, True), (False, 'copy', False, True), - (False, 'generate', False, True), (False, 'none', False, False), + (True, None, None, True, True), (True, None, True, True, True), (True, None, False, False, False), + (True, 'copy', None, True, True), (True, 'generate', None, False, True), + (True, 'none', None, False, False), (False, None, None, False, True), + (False, None, True, False, True), (False, None, False, False, False), + (False, 'copy', None, False, True), (False, 'generate', None, False, True), + (False, 'none', None, False, False), ] - for index, (is_locked, choice, copies, expands) in enumerate(cases): - with self.subTest(is_locked=is_locked, choice=choice), \ + for index, (is_locked, choice, autoexpand, copies, expands) in enumerate(cases): + with self.subTest(is_locked=is_locked, choice=choice, autoexpand=autoexpand), \ patch.object(CollectionReference, 'is_expansion_locked_repo_version_on', return_value=is_locked), \ patch.object( Collection, 'seed_from_head_expansion', autospec=True, side_effect=seed_from_head @@ -3327,6 +3330,8 @@ def test_post_201_expansion_choice(self): payload = {'id': f'v{index}', 'description': 'version'} if choice: payload['expansion'] = choice + if autoexpand is not None: + payload['autoexpand'] = autoexpand response = self.client.post( self.collection.uri + 'versions/', payload, HTTP_AUTHORIZATION='Token ' + self.token, format='json') From 7653a839d4c83f5138fd98a4290259feaabff83f Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 6 Nov 2026 11:56:02 +0530 Subject: [PATCH 3/5] OpenConceptLab/ocl_issues#3010 | New version expansion is lock, latest or none, evaluating every reference --- core/collections/constants.py | 6 +- core/collections/models.py | 94 +++++++------ core/collections/serializers.py | 6 +- core/collections/tests/tests.py | 139 ++++++++++++++++---- core/common/models.py | 18 +-- core/common/tasks.py | 9 +- core/integration_tests/tests_collections.py | 41 ++++-- core/sources/tests/tests.py | 4 +- core/tasks/models.py | 5 +- 9 files changed, 214 insertions(+), 108 deletions(-) diff --git a/core/collections/constants.py b/core/collections/constants.py index d877cabc0..d4dd9748e 100644 --- a/core/collections/constants.py +++ b/core/collections/constants.py @@ -37,7 +37,7 @@ UNRESOLVED_RESOURCE_VERSION_TYPE = 'reference.resource_version' INTENSIONAL_REFERENCE_RESOURCE_VERSIONS_TOGGLE = 'INTENSIONAL_REFERENCE_RESOURCE_VERSIONS_TOGGLE' EXPANSION_LOCKED_REPO_VERSION_TOGGLE = 'EXPANSION_LOCKED_REPO_VERSION_TOGGLE' -VERSION_EXPANSION_COPY = 'copy' -VERSION_EXPANSION_GENERATE = 'generate' +VERSION_EXPANSION_LOCK = 'lock' +VERSION_EXPANSION_LATEST = 'latest' VERSION_EXPANSION_NONE = 'none' -VERSION_EXPANSION_CHOICES = (VERSION_EXPANSION_COPY, VERSION_EXPANSION_GENERATE, VERSION_EXPANSION_NONE) +VERSION_EXPANSION_CHOICES = (VERSION_EXPANSION_LOCK, VERSION_EXPANSION_LATEST, VERSION_EXPANSION_NONE) diff --git a/core/collections/models.py b/core/collections/models.py index 1f90496f1..97cf49522 100644 --- a/core/collections/models.py +++ b/core/collections/models.py @@ -18,7 +18,8 @@ REFERENCE_TYPE_CHOICES, CONCEPT_REFERENCE_TYPE, MAPPING_REFERENCE_TYPE, SOURCE_MAPPINGS, SOURCE_TO_CONCEPTS, TRANSFORM_TO_RESOURCE_VERSIONS, COLLECTION_REFERENCE_TYPE, TRANSFORM_TO_EXTENSIONAL, RESOURCE_VERSION_REPO_RESOLUTION_TOGGLE, UNRESOLVED_RESOURCE_VERSION_TYPE, - INTENSIONAL_REFERENCE_RESOURCE_VERSIONS_TOGGLE, EXPANSION_LOCKED_REPO_VERSION_TOGGLE) + INTENSIONAL_REFERENCE_RESOURCE_VERSIONS_TOGGLE, EXPANSION_LOCKED_REPO_VERSION_TOGGLE, VERSION_EXPANSION_LOCK, + VERSION_EXPANSION_LATEST) from core.collections.parsers import CollectionReferenceParser from core.collections.translators import CollectionReferenceTranslator from core.collections.utils import is_concept, is_mapping @@ -401,19 +402,21 @@ def get_locked_repo_versions(self, relation): expansion = self.expansion if CollectionReference.is_expansion_locked_repo_version_on() else None return list(getattr(expansion, relation).filter(is_active=True).order_by('-id')) if expansion else [] - def can_seed_from_head_expansion(self): - return bool( - not self.is_head and self.should_auto_expand and CollectionReference.is_expansion_locked_repo_version_on() - and get(self.head, 'expansion') - ) + def get_expansion_choice(self, expansion=None): + """How a new version's expansion is built with the lock toggle on: lock or latest, else None (as before)""" + if self.is_head or not self.should_auto_expand or not CollectionReference.is_expansion_locked_repo_version_on(): + return None + if expansion in (VERSION_EXPANSION_LOCK, VERSION_EXPANSION_LATEST): + return expansion + return VERSION_EXPANSION_LOCK if get(self.head, 'expansion') else VERSION_EXPANSION_LATEST - def seed_from_head_expansion(self, index=True): - """False when HEAD has no expansion to copy, so the version seeds as without one""" + def seed_from_head_expansion(self, lock=True, index=True): + """References from HEAD, evaluated again with HEAD's expansion parameters (and its locks, with lock)""" is_outermost = not transaction.get_connection().in_atomic_block attempts = SERIALIZATION_RETRY_ATTEMPTS if is_outermost else 1 for attempt in range(1, attempts + 1): try: - expansion = self.seed_from_head_expansion_in_snapshot(is_outermost) + expansion = self.seed_from_head_expansion_in_snapshot(is_outermost, lock) break except OperationalError as ex: if attempt == attempts or not self.is_serialization_failure(ex): @@ -421,16 +424,14 @@ def seed_from_head_expansion(self, index=True): self.refresh_from_db() self.__dict__.pop('locked_source_versions', None) self.__dict__.pop('locked_collection_versions', None) - if not expansion: - return False if index: expansion.index_resources( list(expansion.concepts.values_list('id', flat=True)), list(expansion.mappings.values_list('id', flat=True))) return True - def seed_from_head_expansion_in_snapshot(self, is_outermost): - """References and expansion copied from HEAD in one transaction, so both come from the same HEAD state""" + def seed_from_head_expansion_in_snapshot(self, is_outermost, lock=True): + """References, parameters and locks read from HEAD in one transaction, so all come from the same HEAD state""" with transaction.atomic(): if is_outermost: with transaction.get_connection().cursor() as cursor: @@ -441,30 +442,28 @@ def seed_from_head_expansion_in_snapshot(self, is_outermost): return expansion head = self.head head_expansion = Expansion.objects.filter(uri=head.expansion_uri).first() if head.expansion_uri else None - if not head_expansion: - return None self.seed_references() + parameters = dict(get(head_expansion, 'parameters') or {}) + system_versions = head_expansion.get_locked_system_version_parameter() if lock and head_expansion else None + if system_versions: + parameters[ExpansionParameters.INCLUDE_SYSTEM] = system_versions expansion = Expansion( - mnemonic=mnemonic, collection_version=self, parameters=head_expansion.parameters, + mnemonic=mnemonic, collection_version=self, parameters=parameters, created_by_id=self.created_by_id, updated_by_id=self.created_by_id) expansion.clean() expansion.full_clean() expansion.save() - for relation in Expansion.REPO_VERSION_RELATIONS: - getattr(expansion, relation).set(getattr(head_expansion, relation).all()) + if lock and head_expansion: + # system-version doesn't reach value sets, so they start from HEAD's value set lock + expansion.evaluated_collection_versions.set( + head_expansion.evaluated_collection_versions.filter(is_active=True)) if not self.expansion_uri: self.expansion_uri = expansion.uri self.save() - if head_expansion.is_processing or head_expansion.has_pending_readd: - # HEAD's expansion is mid-build, so evaluate this version's references against HEAD's locks - references = list(self.references.all()) - for reference in references: - reference._async = True # pylint: disable=protected-access - expansion.add_references(references, index=False, force_reevaluate=True) - else: - expansion.copy_members_from(head_expansion) - expansion.unresolved_repo_versions = head_expansion.unresolved_repo_versions - expansion.save(update_fields=['unresolved_repo_versions']) + references = list(self.references.all()) + for reference in references: + reference._async = True # pylint: disable=protected-access + expansion.add_references(references, index=False, force_reevaluate=True) self.replace_head_resources(expansion) return expansion @@ -1831,7 +1830,7 @@ def add_references( # pylint: disable=too-many-locals,too-many-statements,too-m for system_version in compact(system_versions.split(',')): _system = system_version.strip() _cache_key = f"NONE-{_system}-NONE" - version, _ = ConceptContainerModel.resolve_reference_expression(_system) + version = self.resolve_system_version_entry(_system) _system_version_cache[_cache_key] = version if version.id: include_system_versions.append(version) @@ -2036,20 +2035,29 @@ def index_resources(self, concepts, mappings): if mappings: self.index_mappings(mappings) - @property - def has_pending_readd(self): - """A reference removal leaves this expansion short until its re-add succeeds""" - return Task.unfinished_readd_references_tasks().filter(args__0=self.id).exists() - - def copy_members_from(self, expansion): - for relation in ('concepts', 'mappings'): - through = getattr(Expansion, relation).through - field = f'{relation[:-1]}_id' - with transaction.get_connection().cursor() as cursor: - cursor.execute( - f'INSERT INTO {through._meta.db_table} (expansion_id, {field}) ' # pylint: disable=protected-access - f'SELECT %s, {field} FROM {through._meta.db_table} WHERE expansion_id = %s', # pylint: disable=protected-access - [self.id, expansion.id]) + @staticmethod + def resolve_system_version_entry(entry): + version, _ = ConceptContainerModel.resolve_reference_expression(entry.strip()) + return version + + def get_locked_system_version_parameter(self): + """This expansion's system-version, plus each source it locked that the parameter doesn't name yet""" + entries = [entry.strip() for entry in compact( + str(get(self.parameters, ExpansionParameters.INCLUDE_SYSTEM) or '').split(','))] + named = set() + for entry in entries: + resolved = self.resolve_system_version_entry(entry) + if resolved.id: + named.add(resolved.versioned_object_url) + for version in self.evaluated_source_versions.filter(is_active=True).order_by('-id'): + if version.versioned_object_url in named: + continue + named.add(version.versioned_object_url) + entry = f"{version.canonical_url}|{version.version}" if version.canonical_url else None + if not entry or get(self.resolve_system_version_entry(entry), 'id') != version.id: + entry = f"{version.versioned_object_url}|{version.version}" + entries.append(entry) + return ','.join(entries) @staticmethod def replace_head_rows(rows, owner, field, klass): diff --git a/core/collections/serializers.py b/core/collections/serializers.py index a7ddd4029..f246034a7 100644 --- a/core/collections/serializers.py +++ b/core/collections/serializers.py @@ -9,8 +9,7 @@ from rest_framework.serializers import ModelSerializer, Serializer, ValidationError from core.client_configs.serializers import ClientConfigSerializer -from core.collections.constants import VERSION_EXPANSION_CHOICES, VERSION_EXPANSION_NONE, \ - VERSION_EXPANSION_GENERATE +from core.collections.constants import VERSION_EXPANSION_CHOICES, VERSION_EXPANSION_NONE from core.collections.models import Collection, CollectionReference, Expansion from core.common.constants import HEAD, DEFAULT_ACCESS_TYPE, NAMESPACE_REGEX, INCLUDE_SUMMARY, \ INCLUDE_CLIENT_CONFIGS, INVALID_EXPANSION_URL, INCLUDE_STATES, INCLUDE_TASKS, INCLUDE_RESOLVED_REPO_VERSIONS, \ @@ -295,8 +294,7 @@ def create_version(self, validated_data): collection.autoexpand = False elif expansion: collection.autoexpand = True - errors = Collection.persist_new_version( - collection, user, seed_from_head=expansion != VERSION_EXPANSION_GENERATE) + errors = Collection.persist_new_version(collection, user, expansion=expansion) self._errors.update(errors) return collection diff --git a/core/collections/tests/tests.py b/core/collections/tests/tests.py index 2d2e5ebb5..7aa88f3ee 100644 --- a/core/collections/tests/tests.py +++ b/core/collections/tests/tests.py @@ -2182,28 +2182,77 @@ def test_seed_children_task(self, export_collection_task): self.assertEqual(expansion.mappings.count(), 1) export_collection_task.apply_async.assert_not_called() + @staticmethod + def head_locked_to_v1(): + """HEAD's expansion locks v1, whose concept version a later release v2 replaced""" + source, old, v1, v2 = CollectionReferenceTest.source_with_replaced_concept_version() + head = ExpansionTest.locked_auto_expansion(v1) + Expansion.objects.filter(uri=head.expansion_uri).update(parameters={'activeOnly': True}) + head.add_expressions({'expressions': [old.versioned_object.uri]}, head.created_by) + version = OrganizationCollectionFactory( + organization=head.organization, version='v1', mnemonic=head.mnemonic) + return source, old, v1, v2, head, version + @patch('core.common.tasks.export_collection') - def test_seed_children_task_copies_head_expansion(self, _): - collection = OrganizationCollectionFactory() - expansion = ExpansionFactory(collection_version=collection, parameters={'activeOnly': True}) - collection.expansion_uri = expansion.uri - collection.save() - concept = ConceptFactory() - mapping_version = MappingFactory().get_latest_version() - expansion.concepts.add(concept) - expansion.mappings.add(mapping_version) - expansion.evaluated_source_versions.add(concept.parent) - collection_v1 = OrganizationCollectionFactory( - organization=collection.organization, version='v1', mnemonic=collection.mnemonic) + def test_seed_children_task_lock_keeps_heads_versions_through_a_rebuild(self, _): + source, old, v1, _, head, version = self.head_locked_to_v1() + self.assertEqual(list(head.expansion.concepts.all()), [old]) - seed_children_to_new_version('collection', collection_v1.id, False) # pylint: disable=no-value-for-parameter + seed_children_to_new_version('collection', version.id, False, expansion='lock') # pylint: disable=no-value-for-parameter - collection_v1.refresh_from_db() - copy = collection_v1.expansion - self.assertEqual(list(copy.concepts.all()), [concept.get_latest_version()]) - self.assertEqual(list(copy.mappings.all()), [mapping_version]) - self.assertEqual(list(copy.evaluated_source_versions.all()), [concept.parent]) - self.assertEqual(copy.parameters, expansion.parameters) + version.refresh_from_db() + expansion = version.expansion + self.assertEqual(list(expansion.concepts.all()), [old]) + self.assertEqual(list(expansion.evaluated_source_versions.all()), [v1]) + self.assertEqual(expansion.parameters, {'activeOnly': True, 'system-version': f'{source.uri}|v1'}) + self.assertEqual(list(version.references.first().concepts.all()), [old]) + + expansion.seed_children(index=False, force_reevaluate=True) + + self.assertEqual(list(expansion.concepts.all()), [old]) + self.assertEqual(list(expansion.evaluated_source_versions.all()), [v1]) + + @patch('core.common.tasks.export_collection') + def test_seed_children_task_latest_evaluates_against_the_latest_release(self, _): + _, old, _, v2, _, version = self.head_locked_to_v1() + new = old.versioned_object.get_latest_version() + + seed_children_to_new_version('collection', version.id, False, expansion='latest') # pylint: disable=no-value-for-parameter + + version.refresh_from_db() + expansion = version.expansion + self.assertEqual(list(expansion.concepts.all()), [new]) + self.assertEqual(list(expansion.evaluated_source_versions.all()), [v2]) + self.assertEqual(expansion.parameters, {'activeOnly': True}) + self.assertEqual(list(version.references.first().concepts.all()), [new]) + + @patch('core.common.tasks.export_collection') + def test_seed_children_task_defaults_to_lock_with_a_head_expansion(self, _): + _, old, v1, _, _, version = self.head_locked_to_v1() + + seed_children_to_new_version('collection', version.id, False) # pylint: disable=no-value-for-parameter + + version.refresh_from_db() + self.assertEqual(list(version.expansion.concepts.all()), [old]) + self.assertEqual(list(version.expansion.evaluated_source_versions.all()), [v1]) + + def test_locked_system_version_parameter_keeps_sources_the_parameters_name(self): + source, _, v1, _ = CollectionReferenceTest.source_with_replaced_concept_version() + other = OrganizationSourceFactory() + expansion = ExpansionFactory( + collection_version=OrganizationCollectionFactory(), + parameters={'system-version': f'{source.uri}|v2'}) + expansion.evaluated_source_versions.add(v1, other) + + self.assertEqual(expansion.get_locked_system_version_parameter(), f'{source.uri}|v2,{other.uri}|HEAD') + + def test_locked_system_version_parameter_names_the_ocl_url_when_the_canonical_url_resolves_elsewhere(self): + source = OrganizationSourceFactory(canonical_url='https://example.com/cs') + expansion = ExpansionFactory(collection_version=OrganizationCollectionFactory()) + expansion.evaluated_source_versions.add(source) + + with patch.object(Expansion, 'resolve_system_version_entry', return_value=Mock(id=None)): + self.assertEqual(expansion.get_locked_system_version_parameter(), f'{source.uri}|HEAD') @staticmethod def head_with_head_concept_member(): @@ -2248,7 +2297,7 @@ def test_seed_children_task_evaluates_references_while_head_expansion_is_process latest = concept.get_latest_version() copy = collection_v1.expansion self.assertFalse(copy.is_processing) - self.assertEqual(copy.parameters, {'activeOnly': True}) + self.assertEqual(copy.parameters, {'activeOnly': True, 'system-version': f'{concept.parent.uri}|HEAD'}) self.assertEqual(list(copy.concepts.all()), [latest]) self.assertEqual(list(collection_v1.references.first().concepts.all()), [latest]) @@ -2270,7 +2319,7 @@ def test_seed_children_task_evaluates_references_while_a_removal_awaits_its_read with override_settings(TEST_MODE=False), self.captureOnCommitCallbacks(execute=True): head.delete_references([shared.uri]) self.assertTrue(readd_mock.apply_async.called) - self.assertTrue(expansion.has_pending_readd) + self.assertTrue(Task.unfinished_readd_references_tasks().filter(args__0=expansion.id).exists()) self.assertEqual(list(expansion.concepts.values_list('mnemonic', flat=True)), ['other']) collection_v1 = OrganizationCollectionFactory( @@ -2282,18 +2331,18 @@ def test_seed_children_task_evaluates_references_while_a_removal_awaits_its_read sorted(collection_v1.expansion.concepts.values_list('id', flat=True)), sorted([shared.get_latest_version().id, other.get_latest_version().id])) - def test_has_pending_readd_until_the_readd_succeeds(self): + def test_readd_stays_unfinished_until_it_succeeds(self): expansion = ExpansionFactory(collection_version=OrganizationCollectionFactory()) task = Task.new( user=expansion.collection_version.created_by, name='readd_references_to_expansion_on_references_removal', args=[expansion.id, [1]]) - self.assertTrue(expansion.has_pending_readd) + self.assertTrue(Task.unfinished_readd_references_tasks().filter(args__0=expansion.id).exists()) Task.objects.filter(id=task.id).update(state='FAILURE') - self.assertTrue(expansion.has_pending_readd) + self.assertTrue(Task.unfinished_readd_references_tasks().filter(args__0=expansion.id).exists()) Task.objects.filter(id=task.id).update(state='SUCCESS') - self.assertFalse(expansion.has_pending_readd) + self.assertFalse(Task.unfinished_readd_references_tasks().filter(args__0=expansion.id).exists()) def test_successful_readd_clears_only_older_unfinished_readds_of_that_expansion(self): expansion = ExpansionFactory(collection_version=OrganizationCollectionFactory()) @@ -2546,9 +2595,41 @@ def test_persist_new_version_sync_seeds_inline(self, seed_mock): with self.captureOnCommitCallbacks(execute=False) as callbacks: Collection.persist_new_version(collection_v1, collection.created_by, sync=True) - seed_mock.assert_called_once_with('collection', collection_v1.id, True, True, True) + seed_mock.assert_called_once_with('collection', collection_v1.id, True, True, expansion=None) self.assertEqual(callbacks, []) + @override_settings(TEST_MODE=False) + @patch('core.common.models.seed_children_to_new_version.apply_async') + def test_persist_new_version_records_the_choice_with_the_task_not_in_its_message(self, apply_async): + collection = OrganizationCollectionFactory() + collection_v1 = Collection(version='v1', mnemonic=collection.mnemonic, organization=collection.organization) + + with self.captureOnCommitCallbacks(execute=True): + Collection.persist_new_version(collection_v1, collection.created_by, expansion='latest') + + task = Task.objects.get(name='seed_children_to_new_version') + self.assertEqual(task.args, ['collection', collection_v1.id, True, False]) + self.assertEqual(task.kwargs, {'expansion': 'latest'}) + apply_async.assert_called_once_with( + ('collection', collection_v1.id, True, False), task_id=task.id, queue='default', persist_args=True) + + @patch('core.common.tasks.export_collection') + def test_seed_children_task_reads_the_choice_from_its_task(self, _): + _, _, _, v2, _, version = self.head_locked_to_v1() + task = Task.new(user=version.created_by, name='seed_children_to_new_version', kwargs={'expansion': 'latest'}) + Task.before_start(task.id, ['collection', version.id, False], {}, name='seed_children_to_new_version') + + seed_children_to_new_version.push_request(id=task.id) + try: + seed_children_to_new_version.run('collection', version.id, False) + finally: + seed_children_to_new_version.pop_request() + + version.refresh_from_db() + self.assertEqual(list(version.expansion.evaluated_source_versions.all()), [v2]) + task.refresh_from_db() + self.assertEqual(task.kwargs, {'expansion': 'latest'}) + @override_settings(TEST_MODE=False) def test_persist_new_version_retries_serialization_failures_for_overlapping_versions(self): collection = OrganizationCollectionFactory() @@ -2563,7 +2644,7 @@ def conflict_until_last_attempt(*_, **__): if len(calls) < SERIALIZATION_RETRY_ATTEMPTS: raise failure - with patch.object(Collection, 'can_seed_from_head_expansion', return_value=True), \ + with patch.object(Collection, 'get_expansion_choice', return_value='lock'), \ patch('core.common.models.transaction.get_connection', return_value=Mock(in_atomic_block=False)), \ patch.object(Collection, 'persist_new_version_in_transaction', side_effect=conflict_until_last_attempt): errors = Collection.persist_new_version(collection_v1, collection.created_by, sync=True) @@ -2579,7 +2660,7 @@ def test_persist_new_version_gives_up_after_the_serialization_retry_attempts(sel failure = OperationalError('could not serialize access') failure.__cause__ = serialization_cause() - with patch.object(Collection, 'can_seed_from_head_expansion', return_value=True), \ + with patch.object(Collection, 'get_expansion_choice', return_value='lock'), \ patch('core.common.models.transaction.get_connection', return_value=Mock(in_atomic_block=False)), \ patch.object( Collection, 'persist_new_version_in_transaction', side_effect=failure) as in_transaction: @@ -3229,7 +3310,7 @@ def test_delete_references_not_test_mode_queues_readd_task(self, batch_index_moc name='core.common.tasks.readd_references_to_expansion_on_references_removal', args__0=expansion.id) readd_task_mock.apply_async.assert_called_once_with( (expansion.id, [reference.id]), queue='default', task_id=task.id, persist_args=True) - self.assertTrue(expansion.has_pending_readd) + self.assertTrue(Task.unfinished_readd_references_tasks().filter(args__0=expansion.id).exists()) @override_settings(TEST_MODE=False) @patch('core.collections.models.readd_references_to_expansion_on_references_removal') diff --git a/core/common/models.py b/core/common/models.py index 4fb48a73e..78e0da524 100644 --- a/core/common/models.py +++ b/core/common/models.py @@ -1250,7 +1250,7 @@ def persist_new_version(cls, obj, user=None, **kwargs): obj.is_active = True sync = kwargs.pop('sync', False) - seed_from_head = kwargs.pop('seed_from_head', True) + expansion = kwargs.pop('expansion', None) if user: obj.created_by = user obj.updated_by = user @@ -1263,12 +1263,12 @@ def persist_new_version(cls, obj, user=None, **kwargs): is_test_mode = get(settings, 'TEST_MODE', False) # a synchronous seed from HEAD runs in this transaction, so it needs this transaction's snapshot is_repeatable_read = sync and not is_test_mode and not transaction.get_connection().in_atomic_block and \ - seed_from_head and obj.can_seed_from_head_expansion() + bool(obj.get_expansion_choice(expansion)) attempts = SERIALIZATION_RETRY_ATTEMPTS if is_repeatable_read else 1 for attempt in range(1, attempts + 1): try: cls.persist_new_version_in_transaction( - obj, head, user, sync, is_repeatable_read, seed_from_head, **kwargs) + obj, head, user, sync, is_repeatable_read, expansion, **kwargs) break except OperationalError as ex: if attempt == attempts or not cls.is_serialization_failure(ex): @@ -1281,7 +1281,7 @@ def persist_new_version(cls, obj, user=None, **kwargs): @classmethod def persist_new_version_in_transaction( # pylint: disable=too-many-arguments - cls, obj, head, user, sync, is_repeatable_read, seed_from_head=True, **kwargs): + cls, obj, head, user, sync, is_repeatable_read, expansion=None, **kwargs): is_test_mode = get(settings, 'TEST_MODE', False) with transaction.atomic(): if is_repeatable_read: @@ -1295,16 +1295,18 @@ def persist_new_version_in_transaction( # pylint: disable=too-many-arguments if obj.id: obj.sibling_versions.update(is_latest_version=False) - task_args = (obj.resource_type.lower(), obj.id, not is_test_mode, sync, seed_from_head) + task_args = (obj.resource_type.lower(), obj.id, not is_test_mode, sync) if is_test_mode or sync: - seed_children_to_new_version(*task_args) + seed_children_to_new_version(*task_args, expansion=expansion) else: from core.tasks.models import Task + # the choice rides on the task record, so a worker on older code still takes the message task = Task.new( queue='default', user=user, name=seed_children_to_new_version.__name__, args=task_args, + kwargs={'expansion': expansion} if expansion else None, ) # Mark the version as processing before the response goes out @@ -1328,8 +1330,8 @@ def is_serialization_failure(ex): def index_resources_for_self_as_latest_released(self, only_update=False): # pylint: disable=unused-argument pass - def can_seed_from_head_expansion(self): - return False + def get_expansion_choice(self, expansion=None): # pylint: disable=unused-argument + return None @classmethod def persist_changes(cls, obj, updated_by, original_schema, **kwargs): # pylint: disable=too-many-locals,too-many-branches diff --git a/core/common/tasks.py b/core/common/tasks.py index 12be650ae..35089a2a0 100644 --- a/core/common/tasks.py +++ b/core/common/tasks.py @@ -24,6 +24,7 @@ from core.celery import app from core.common import ERRBIT_LOGGER +from core.collections.constants import VERSION_EXPANSION_LOCK from core.common.constants import CONFIRM_EMAIL_ADDRESS_MAIL_SUBJECT, PASSWORD_RESET_MAIL_SUBJECT from core.common.exceptions import BatchIndexingError from core.common.utils import write_export_file, web_url, get_resource_class_from_resource_name, get_export_service, \ @@ -384,7 +385,7 @@ def send_user_reset_password_email(user_id): @app.task(bind=True) -def seed_children_to_new_version(self, resource, obj_id, export=True, sync=False, seed_from_head=True): # pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements +def seed_children_to_new_version(self, resource, obj_id, export=True, sync=False, expansion=None): # pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements export_task = None changelog_task = None autoexpand = True @@ -418,8 +419,10 @@ def seed_children_to_new_version(self, resource, obj_id, export=True, sync=False instance.snapshot = CollectionDetailSerializer(head).data instance.save(update_fields=['snapshot']) instance.get_checksums(recalculate=True) - seed_from_head = seed_from_head and is_collection and autoexpand and \ - instance.can_seed_from_head_expansion() and instance.seed_from_head_expansion() + if is_collection and not expansion and task_id: + expansion = get(Task.objects.filter(id=task_id).values_list('kwargs', flat=True).first(), 'expansion') + choice = instance.get_expansion_choice(expansion) if is_collection else None + seed_from_head = bool(choice) and instance.seed_from_head_expansion(lock=choice == VERSION_EXPANSION_LOCK) if not seed_from_head: instance.seed_references() if is_source: diff --git a/core/integration_tests/tests_collections.py b/core/integration_tests/tests_collections.py index 5b59cb1cf..c44e43489 100644 --- a/core/integration_tests/tests_collections.py +++ b/core/integration_tests/tests_collections.py @@ -3311,34 +3311,38 @@ def test_post_201(self): self.assertEqual(expansion.concepts.count(), 1) self.assertEqual(expansion.mappings.count(), 0) + def post_version(self, version, **payload): + return self.client.post( + self.collection.uri + 'versions/', {'id': version, 'description': 'version', **payload}, + HTTP_AUTHORIZATION='Token ' + self.token, format='json') + def test_post_201_expansion_choice(self): seed_from_head = Collection.seed_from_head_expansion cases = [ - (True, None, None, True, True), (True, None, True, True, True), (True, None, False, False, False), - (True, 'copy', None, True, True), (True, 'generate', None, False, True), - (True, 'none', None, False, False), (False, None, None, False, True), - (False, None, True, False, True), (False, None, False, False, False), - (False, 'copy', None, False, True), (False, 'generate', None, False, True), - (False, 'none', None, False, False), + (True, None, None, 'lock', True), (True, None, True, 'lock', True), (True, None, False, None, False), + (True, 'lock', None, 'lock', True), (True, 'latest', None, 'latest', True), + (True, 'none', None, None, False), (False, None, None, None, True), + (False, None, True, None, True), (False, None, False, None, False), + (False, 'lock', None, None, True), (False, 'latest', None, None, True), + (False, 'none', None, None, False), ] - for index, (is_locked, choice, autoexpand, copies, expands) in enumerate(cases): + for index, (is_locked, choice, autoexpand, built, expands) in enumerate(cases): with self.subTest(is_locked=is_locked, choice=choice, autoexpand=autoexpand), \ patch.object(CollectionReference, 'is_expansion_locked_repo_version_on', return_value=is_locked), \ patch.object( Collection, 'seed_from_head_expansion', autospec=True, side_effect=seed_from_head ) as seed_mock: - payload = {'id': f'v{index}', 'description': 'version'} - if choice: - payload['expansion'] = choice + payload = {'expansion': choice} if choice else {} if autoexpand is not None: payload['autoexpand'] = autoexpand - response = self.client.post( - self.collection.uri + 'versions/', payload, - HTTP_AUTHORIZATION='Token ' + self.token, format='json') + response = self.post_version(f'v{index}', **payload) self.assertEqual(response.status_code, 201) version = self.collection.versions.get(version=f'v{index}') - self.assertEqual(seed_mock.called, copies) + if built: + seed_mock.assert_called_once_with(version, lock=built == 'lock') + else: + seed_mock.assert_not_called() self.assertEqual(version.autoexpand, expands) self.assertEqual(version.references.count(), 1) if expands: @@ -3348,6 +3352,15 @@ def test_post_201_expansion_choice(self): self.assertEqual(version.expansions.count(), 0) self.assertIsNone(version.expansion_uri) + def test_post_201_defaults_to_latest_without_head_expansion(self): + Collection.objects.filter(id=self.collection.id).update(expansion_uri=None) + + with patch.object(Collection, 'seed_from_head_expansion', autospec=True, return_value=True) as seed_mock: + response = self.post_version('v1') + + self.assertEqual(response.status_code, 201) + seed_mock.assert_called_once_with(self.collection.versions.get(version='v1'), lock=False) + def test_post_400_expansion_choice_invalid(self): response = self.client.post( self.collection.uri + 'versions/', {'id': 'v1', 'description': 'version1', 'expansion': 'rebuild'}, diff --git a/core/sources/tests/tests.py b/core/sources/tests/tests.py index 80b3e59ab..351a06e91 100644 --- a/core/sources/tests/tests.py +++ b/core/sources/tests/tests.py @@ -942,9 +942,9 @@ def test_persist_new_version_registers_seed_task_before_enqueue(self, apply_asyn Source.persist_new_version(source_version, source.created_by) task = Task.objects.get(name='seed_children_to_new_version') - self.assertEqual(task.args, ['source', source_version.id, True, False, True]) + self.assertEqual(task.args, ['source', source_version.id, True, False]) apply_async.assert_called_once_with( - ('source', source_version.id, True, False, True), + ('source', source_version.id, True, False), task_id=task.id, queue='default', persist_args=True, diff --git a/core/tasks/models.py b/core/tasks/models.py index c3ecf021b..fb072fd66 100644 --- a/core/tasks/models.py +++ b/core/tasks/models.py @@ -121,7 +121,7 @@ def record_exception(self, exception): return self def save_common(self, args, kwargs, einfo=None): # pylint: disable=unused-argument - self.kwargs = kwargs + self.kwargs = kwargs or self.kwargs if einfo: self.record_exception(einfo) @@ -159,7 +159,8 @@ def before_start(cls, task_id, args, kwargs, name=None): # pylint: disable=unus task.name = name or task.name task.state = STARTED task.queue = kwargs.get('queue', None) or task.queue or 'default' - task.kwargs = kwargs + # a message without kwargs keeps the ones recorded with the task + task.kwargs = kwargs or task.kwargs task.started_at = timezone.now() task.save() return task From 8662e1afd5edeb7de8a9602f6a1a7782d2f54440 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 6 Nov 2026 12:18:23 +0530 Subject: [PATCH 4/5] OpenConceptLab/ocl_issues#3010 | Rebuild keeps resource versions for HEAD-resolved members; default choice settled at create --- core/collections/models.py | 18 +++++++++++-- core/collections/tests/tests.py | 45 ++++++++++++++++++++++++++++++++- core/common/models.py | 2 ++ 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/core/collections/models.py b/core/collections/models.py index 97cf49522..aeb90a659 100644 --- a/core/collections/models.py +++ b/core/collections/models.py @@ -2074,12 +2074,26 @@ def replace_head_rows(rows, owner, field, klass): rows.model.objects.filter(id__in=head_rows.values('id')).update(**{f'{field}_id': Subquery(latest)}) def seed_children(self, index=True, force_reevaluate=False): + version = self.collection_version + # a version's default expansion holds resource versions, so a rebuild swaps HEAD rows as its creation did + swap_head = force_reevaluate and not version.is_head and self.is_default and \ + CollectionReference.is_expansion_locked_repo_version_on() + previous_concept_ids = set(self.concepts.values_list('id', flat=True)) if swap_head and index else set() + previous_mapping_ids = set(self.mappings.values_list('id', flat=True)) if swap_head and index else set() self.add_references( - references=self.collection_version.references, - index=index, + references=version.references, + index=index and not swap_head, is_adding_all=True, force_reevaluate=force_reevaluate ) + if swap_head: + version.replace_head_resources(self) + if index: + concept_ids = set(self.concepts.values_list('id', flat=True)) + mapping_ids = set(self.mappings.values_list('id', flat=True)) + self.index_resources(list(concept_ids), list(mapping_ids)) + self.full_index_resources( + list(previous_concept_ids - concept_ids), list(previous_mapping_ids - mapping_ids)) def wait_until_processed(self): # pragma: no cover processing = self.is_processing diff --git a/core/collections/tests/tests.py b/core/collections/tests/tests.py index 7aa88f3ee..437157f2c 100644 --- a/core/collections/tests/tests.py +++ b/core/collections/tests/tests.py @@ -2286,6 +2286,26 @@ def test_seed_children_task_swaps_head_resources_in_references(self, _, batch_in batch_index_resources_mock.apply_async.assert_called_once_with( ('concept', {'id__in': [latest.id]}), queue='indexing', permanent=False) + @patch('core.common.tasks.export_collection') + def test_rebuild_keeps_the_resource_versions_a_head_resolved_version_was_created_with(self, _): + for choice in ('lock', 'latest'): + with self.subTest(choice=choice): + _, concept, collection_v1 = self.head_with_head_concept_member() + seed_children_to_new_version('collection', collection_v1.id, False, expansion=choice) # pylint: disable=no-value-for-parameter + collection_v1.refresh_from_db() + latest = concept.get_latest_version() + expansion = collection_v1.expansion + self.assertEqual(list(expansion.concepts.all()), [latest]) + + expansion.seed_children(index=False, force_reevaluate=True) + + self.assertEqual(list(expansion.concepts.all()), [latest]) + self.assertEqual(list(collection_v1.references.first().concepts.all()), [latest]) + + with patch.object(CollectionReference, 'is_expansion_locked_repo_version_on', return_value=False): + expansion.seed_children(index=False, force_reevaluate=True) + self.assertEqual(list(expansion.concepts.all()), [concept]) + @patch('core.common.tasks.export_collection') def test_seed_children_task_evaluates_references_while_head_expansion_is_processing(self, _): expansion, concept, collection_v1 = self.head_with_head_concept_member() @@ -2595,7 +2615,7 @@ def test_persist_new_version_sync_seeds_inline(self, seed_mock): with self.captureOnCommitCallbacks(execute=False) as callbacks: Collection.persist_new_version(collection_v1, collection.created_by, sync=True) - seed_mock.assert_called_once_with('collection', collection_v1.id, True, True, expansion=None) + seed_mock.assert_called_once_with('collection', collection_v1.id, True, True, expansion='latest') self.assertEqual(callbacks, []) @override_settings(TEST_MODE=False) @@ -2613,6 +2633,29 @@ def test_persist_new_version_records_the_choice_with_the_task_not_in_its_message apply_async.assert_called_once_with( ('collection', collection_v1.id, True, False), task_id=task.id, queue='default', persist_args=True) + @override_settings(TEST_MODE=False) + @patch('core.common.models.seed_children_to_new_version.apply_async') + def test_persist_new_version_settles_the_default_before_the_task_runs(self, _): + _, _, _, _, head, _ = self.head_locked_to_v1() + collection_v2 = Collection(version='v2', mnemonic=head.mnemonic, organization=head.organization) + + with self.captureOnCommitCallbacks(execute=True): + Collection.persist_new_version(collection_v2, head.created_by) + + task = Task.objects.get(name='seed_children_to_new_version', args__1=collection_v2.id) + self.assertEqual(task.kwargs, {'expansion': 'lock'}) + + Collection.objects.filter(id=head.id).update(expansion_uri=None) + with patch.object(Collection, 'seed_from_head_expansion', autospec=True, return_value=True) as seed_mock, \ + patch('core.common.tasks.export_collection'): + seed_children_to_new_version.push_request(id=task.id) + try: + seed_children_to_new_version.run('collection', collection_v2.id, False) + finally: + seed_children_to_new_version.pop_request() + + seed_mock.assert_called_once_with(ANY, lock=True) + @patch('core.common.tasks.export_collection') def test_seed_children_task_reads_the_choice_from_its_task(self, _): _, _, _, v2, _, version = self.head_locked_to_v1() diff --git a/core/common/models.py b/core/common/models.py index 78e0da524..159e2cc18 100644 --- a/core/common/models.py +++ b/core/common/models.py @@ -1291,6 +1291,8 @@ def persist_new_version_in_transaction( # pylint: disable=too-many-arguments head = cls.objects.select_for_update().get(id=head.id) obj.update_version_data(head) obj.save(**kwargs) + # the default is settled while HEAD is locked, so a queued task can't see another HEAD + expansion = obj.get_expansion_choice(expansion) or expansion if obj.id: obj.sibling_versions.update(is_latest_version=False) From 3bc047d7b4eee6473a21d415de639a6609963f51 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Fri, 6 Nov 2026 13:13:00 +0530 Subject: [PATCH 5/5] OpenConceptLab/ocl_issues#3010 | Mock index tasks in the task-record tests so they need no Redis --- core/collections/tests/tests.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/collections/tests/tests.py b/core/collections/tests/tests.py index 437157f2c..37397a1b6 100644 --- a/core/collections/tests/tests.py +++ b/core/collections/tests/tests.py @@ -2619,6 +2619,8 @@ def test_persist_new_version_sync_seeds_inline(self, seed_mock): self.assertEqual(callbacks, []) @override_settings(TEST_MODE=False) + @patch('core.common.models.handle_m2m_changed', Mock()) + @patch('core.common.models.handle_save', Mock()) @patch('core.common.models.seed_children_to_new_version.apply_async') def test_persist_new_version_records_the_choice_with_the_task_not_in_its_message(self, apply_async): collection = OrganizationCollectionFactory() @@ -2633,13 +2635,15 @@ def test_persist_new_version_records_the_choice_with_the_task_not_in_its_message apply_async.assert_called_once_with( ('collection', collection_v1.id, True, False), task_id=task.id, queue='default', persist_args=True) - @override_settings(TEST_MODE=False) + @patch('core.common.models.handle_m2m_changed', Mock()) + @patch('core.common.models.handle_save', Mock()) @patch('core.common.models.seed_children_to_new_version.apply_async') def test_persist_new_version_settles_the_default_before_the_task_runs(self, _): _, _, _, _, head, _ = self.head_locked_to_v1() collection_v2 = Collection(version='v2', mnemonic=head.mnemonic, organization=head.organization) - with self.captureOnCommitCallbacks(execute=True): + # only the queuing runs as outside tests, and its index tasks are mocked, so nothing reaches Redis + with override_settings(TEST_MODE=False), self.captureOnCommitCallbacks(execute=True): Collection.persist_new_version(collection_v2, head.created_by) task = Task.objects.get(name='seed_children_to_new_version', args__1=collection_v2.id)