diff --git a/core/collections/models.py b/core/collections/models.py index be2b8db7..f403d2b8 100644 --- a/core/collections/models.py +++ b/core/collections/models.py @@ -1472,6 +1472,19 @@ def add_references( # pylint: disable=too-many-locals,too-many-statements,too-m exclude_refs += [*existing_exclude_refs.all()] index_concepts = index_mappings = [] + previous_concept_ids = previous_mapping_ids = set() + + if force_reevaluate and is_adding_all: + # from scratch: drop members and version locks, keep the expansion id + previous_concept_ids = set(self.concepts.values_list('id', flat=True)) + previous_mapping_ids = set(self.mappings.values_list('id', flat=True)) + self.concepts.clear() + self.mappings.clear() + self.explicit_collection_versions.clear() + self.explicit_source_versions.clear() + self.evaluated_collection_versions.clear() + self.evaluated_source_versions.clear() + self.unresolved_repo_versions = [] # attempt_reevaluate is False for delete reference(s) should_reevaluate = force_reevaluate or (attempt_reevaluate and not self.is_auto_generated) @@ -1527,7 +1540,8 @@ def get_ref_results(ref): # pylint: disable=too-many-branches,too-many-locals ) if not existing_evaluated_system_version: existing_evaluated_system_version = self.evaluated_source_versions.filter( - uri__startswith=drop_version(_system_version.uri) + uri__startswith=drop_version(_system_version.uri), + released=True, is_active=True, retired=False ).order_by('-id').first() if existing_evaluated_system_version: _system_version = existing_evaluated_system_version @@ -1607,7 +1621,27 @@ def get_ref_results(ref): # pylint: disable=too-many-branches,too-many-locals self.save() self.dedupe_resources() if index: - self.index_resources(index_concepts, index_mappings) + member_concept_ids = set(self.concepts.values_list('id', flat=True)) + member_mapping_ids = set(self.mappings.values_list('id', flat=True)) + candidate_concept_ids = set(index_concepts) + candidate_mapping_ids = set(index_mappings) + self.index_resources( + list(candidate_concept_ids & member_concept_ids), list(candidate_mapping_ids & member_mapping_ids)) + # rows that left the expansion (dedupe or re-evaluate) are re-indexed from the DB to drop the tags + self.full_index_resources( + list((candidate_concept_ids | previous_concept_ids) - member_concept_ids), + list((candidate_mapping_ids | previous_mapping_ids) - member_mapping_ids) + ) + + @staticmethod + def full_index_resources(concept_ids, mapping_ids): + for resource, ids in (('concept', concept_ids), ('mapping', mapping_ids)): + if ids: + filters = {'id__in': ids} + if get(settings, 'TEST_MODE', False): + batch_index_resources(resource, filters) + else: + batch_index_resources.apply_async((resource, filters), queue='indexing', permanent=False) def dedupe_resources(self): self.__dedupe_concepts() diff --git a/core/collections/tests/tests.py b/core/collections/tests/tests.py index 280cfecd..84f24d8e 100644 --- a/core/collections/tests/tests.py +++ b/core/collections/tests/tests.py @@ -2516,6 +2516,77 @@ def test_add_references_reference_without_system_resolves_none_system_version(se self.assertEqual(expansion.mappings.count(), 0) + def _evaluated_expansion(self, source, references): + collection = OrganizationCollectionFactory() + expansion = ExpansionFactory(collection_version=collection, mnemonic='e1') + collection.expansion_uri = expansion.uri + collection.save() + for expression in references: + reference = CollectionReference( + expression=expression, collection=collection, system=source.uri, cascade='sourcetoconcepts') + reference.evaluate() + reference.save() + return collection, expansion + + def test_re_evaluate_moves_to_newer_release_and_drops_stale_members(self): + source = OrganizationSourceFactory() + concept = ConceptFactory(parent=source, mnemonic='c1') + old_version = OrganizationSourceFactory( + mnemonic=source.mnemonic, organization=source.organization, version='v1', released=True) + collection, expansion = self._evaluated_expansion(source, [source.uri + 'concepts/']) + stale = ConceptFactory(parent=source, mnemonic='stale') + expansion.concepts.add(stale) + expansion.evaluated_source_versions.add(old_version) + new_version = OrganizationSourceFactory( + mnemonic=source.mnemonic, organization=source.organization, version='v2', released=True) + + with patch('core.collections.models.batch_index_resources') as batch_index_mock, \ + patch.object(Expansion, 'index_resources'): + expansion.add_references( + collection.references.all(), index=True, is_adding_all=True, force_reevaluate=True) + + self.assertEqual(expansion.id, Expansion.objects.get(mnemonic='e1', collection_version=collection).id) + self.assertNotIn(old_version.id, expansion.evaluated_source_versions.values_list('id', flat=True)) + self.assertNotIn(stale.id, expansion.concepts.values_list('id', flat=True)) + self.assertNotIn( + old_version.id, expansion.evaluated_source_versions.values_list('id', flat=True)) + for version in expansion.evaluated_source_versions.all(): + self.assertEqual(version.id, new_version.id) + batch_index_mock.assert_any_call('concept', {'id__in': [stale.id]}) + self.assertIsNotNone(concept) + + def test_unreleased_or_retired_version_never_picked_implicitly(self): + source = OrganizationSourceFactory() + released = OrganizationSourceFactory( + mnemonic=source.mnemonic, organization=source.organization, version='v1', released=True) + OrganizationSourceFactory( + mnemonic=source.mnemonic, organization=source.organization, version='v2', released=False) + OrganizationSourceFactory( + mnemonic=source.mnemonic, organization=source.organization, version='v3', released=True, retired=True) + + self.assertEqual(source.get_latest_released_version().id, released.id) + + def test_add_references_indexes_only_post_dedupe_members(self): + source = OrganizationSourceFactory() + concept = ConceptFactory(parent=source, mnemonic='c1') + latest = concept.get_latest_version() + collection, expansion = self._evaluated_expansion(source, [concept.uri, latest.uri]) + by_expression = {concept.uri: concept, latest.uri: latest} + + def get_concepts(reference, *args, **kwargs): # pylint: disable=unused-argument + return Concept.objects.filter(id=by_expression[reference.expression].id), Mapping.objects.none() + + with patch.object(CollectionReference, 'get_concepts', get_concepts), \ + patch.object(Expansion, 'index_resources') as index_mock, \ + patch.object(Expansion, 'full_index_resources') as full_index_mock: + expansion.add_references( + collection.references.all(), index=True, is_adding_all=True, force_reevaluate=True) + + self.assertEqual(list(expansion.concepts.values_list('id', flat=True)), [latest.id]) + self.assertEqual(index_mock.call_args[0][0], [latest.id]) + full_index_mock.assert_called_once_with([concept.id], []) + + class ExpansionParametersTest(OCLTestCase): def test_apply_active_only(self): ConceptFactory(id=1, retired=False, mnemonic='active') @@ -4576,6 +4647,20 @@ def test_expansion_re_evaluate_starts_processing(self): self.assertEqual(response.status_code, 204) + def test_expansion_re_evaluate_forbidden_for_non_member(self): + collection = OrganizationCollectionFactory(created_by=self.admin, updated_by=self.admin) + expansion = ExpansionFactory(collection_version=collection, created_by=self.admin, is_processing=False) + outsider = UserProfileFactory() + + with patch('core.collections.views.seed_children_to_expansion') as seed_mock: + response = self.client.post( + f'{collection.uri}HEAD/expansions/{expansion.mnemonic}/re-evaluate/', {}, + HTTP_AUTHORIZATION=f"Token {outsider.get_token()}" + ) + + self.assertEqual(response.status_code, 403) + seed_mock.assert_not_called() + def test_expansion_children_not_found(self): collection = OrganizationCollectionFactory(created_by=self.admin, updated_by=self.admin) response = self.client.get( diff --git a/core/collections/views.py b/core/collections/views.py index d77c307e..15c616d8 100644 --- a/core/collections/views.py +++ b/core/collections/views.py @@ -953,7 +953,6 @@ def get(self, request, *args, **kwargs): # pylint: disable=unused-argument class CollectionVersionExpansionReEvaluateView(CollectionVersionExpansionBaseView, TaskMixin): serializer_class = TaskSerializer - permission_classes = (CanViewConceptDictionary, ) def post(self, request, **kwargs): # pylint: disable=unused-argument obj = self.get_object() diff --git a/core/common/models.py b/core/common/models.py index 5447ec1e..2eb025d4 100644 --- a/core/common/models.py +++ b/core/common/models.py @@ -710,14 +710,15 @@ def get_last_version(self): return self.active_versions.order_by('-created_at').first() def get_latest_released_version(self): - return self.released_versions.order_by('-created_at').first() + return self.released_versions.filter(retired=False).order_by('-created_at').first() def get_prev_released_version(self): return self.released_versions.exclude(id=self.id).order_by('-created_at').first() @property def is_latest_released(self): - return self.released and self.id == self.get_latest_released_version().id + latest_released = self.get_latest_released_version() + return bool(self.released and latest_released and self.id == latest_released.id) @classmethod def find_latest_released_version_by(cls, filters):