OpenConceptLab/ocl_issues#2943 | Expansion system-version resolves in the owner namespace and rejects two versions of one source

core/collections/models.py

@@ -43,6 +43,7 @@
 from core.mappings.models import Mapping
 from core.tasks.models import Task
 from core.toggles.models import Toggle
+from core.url_registry.constants import URL_REGISTRY_IN_RESOLVE_REFERENCE_TOGGLE
 
 TRUTHY = get_truthy_values()
 FALSEY = get_falsy_values()
@@ -273,11 +274,11 @@ def add_references(self, references, user=None, _async=False):
         is_locked = CollectionReference.is_expansion_locked_repo_version_on()
         for reference in references:
             reference.expression = reference.build_expression()
+            # before generating, so cascade-and-transform resolves in this collection's namespace and locks
+            reference.collection = self
             if is_locked:
-                # before generating, so cascade-and-transform resolves against this collection's locks
-                reference.collection = self
                 reference._async = _async  # pylint: disable=protected-access
-                reference.clear_resolution_cache()
+            reference.clear_resolution_cache()
             total_references += reference.generate_references()
         total_references = CollectionReference.dedupe_by_expression(total_references)
         for reference in total_references:
@@ -942,6 +943,7 @@ def __generate_references_for_type(self, queryset, resource_type, locked_version
                 created_by=self.created_by,
                 resource_version=resource_version,
                 system=self.system,
+                namespace=self.namespace,
                 version=repo_version
             ))
         return references
@@ -1138,11 +1140,18 @@ def get_locked_system_versions(self, version):
             if drop_version(locked.uri) == drop_version(version.uri)
         ]
 
+    @cached_property
+    def resolution_namespace(self):
+        """Its own namespace, else its collection owner's, as the URL registries resolve within an owner"""
+        if self.namespace or not self.collection_id:
+            return self.namespace
+        return Expansion.get_system_version_namespace(self.collection.parent_url)
+
     @cached_property
     def resolve_expression_system_version(self):
         if self.system:
             from core.sources.models import Source
-            version, _ = Source.resolve_reference_expression(self.system, self.namespace, self.version)
+            version, _ = Source.resolve_reference_expression(self.system, self.resolution_namespace, self.version)
             if version.id:
                 return version
         return None
@@ -1273,7 +1282,7 @@ def is_explicit_valueset(valueset, all_forms=True):
 
     def resolve_valueset_version(self, valueset):
         """A value set named without a version resolves to the collection's locked version of it, if any"""
-        version, _ = Collection.resolve_reference_expression(valueset, self.namespace)
+        version, _ = Collection.resolve_reference_expression(valueset, self.resolution_namespace)
         if version.id and self.collection_id and not self.is_explicit_valueset(valueset):
             return next((
                 locked for locked in self.collection.locked_collection_versions
@@ -1282,8 +1291,8 @@ def resolve_valueset_version(self, valueset):
         return version
 
     def clear_resolution_cache(self):
-        for attr in ('resolve_system_version', 'resolve_expression_system_version', 'resolve_valueset_versions',
-                     'resolve_valueset_versions_with_unresolved'):
+        for attr in ('resolution_namespace', 'resolve_system_version', 'resolve_expression_system_version',
+                     'resolve_valueset_versions', 'resolve_valueset_versions_with_unresolved'):
             self.__dict__.pop(attr, None)
 
     @cached_property
@@ -1301,7 +1310,8 @@ def resolve_valueset_versions_with_unresolved(self):
                     if version.id:
                         to_add.append(version)
                     else:
-                        unresolved.append({'url': valueset, 'namespace': self.namespace, 'type': 'reference.valueset'})
+                        unresolved.append(
+                            {'url': valueset, 'namespace': self.resolution_namespace, 'type': 'reference.valueset'})
         return explicit, evaluated, unresolved
 
     def clean(self):
@@ -1414,10 +1424,10 @@ def _get_expression_system_version(self, system_version_cache=None):
         if system_version_cache is None:
             return self.resolve_expression_system_version
 
-        cache_key = (self.namespace, self.system, self.version)
+        cache_key = (self.resolution_namespace, self.system, self.version)
         if cache_key not in system_version_cache:
             from core.sources.models import Source
-            version, _ = Source.resolve_reference_expression(self.system, self.namespace, self.version)
+            version, _ = Source.resolve_reference_expression(self.system, self.resolution_namespace, self.version)
             system_version_cache[cache_key] = version if version.id else None
         return system_version_cache[cache_key]
 
@@ -1456,9 +1466,9 @@ def _get_resolved_valueset_versions(self, valueset_version_cache=None):
         if isinstance(self.valueset, list):
             for valueset in self.valueset:  # pylint: disable=not-an-iterable
                 if valueset:
-                    cache_key = (self.namespace, valueset, None)
+                    cache_key = (self.resolution_namespace, valueset, None)
                     if cache_key not in valueset_version_cache:
-                        version, _ = Collection.resolve_reference_expression(valueset, self.namespace)
+                        version, _ = Collection.resolve_reference_expression(valueset, self.resolution_namespace)
                         valueset_version_cache[cache_key] = version if version.id else None
                     if valueset_version_cache[cache_key]:
                         versions.append(valueset_version_cache[cache_key])
@@ -1772,6 +1782,42 @@ def delete_expressions(self, expressions):  # Deprecated: Old way, must use dele
                 CollectionReference.on_commit_when_locked(lambda: batch_index_resources.apply_async(
                     ('mapping', mappings_filters), queue='indexing', permanent=False))
 
+    @staticmethod
+    def get_system_version_namespace(owner_url):
+        # without the URL registries a namespace would limit canonical URLs to the owner's repositories
+        return owner_url if Toggle.get(URL_REGISTRY_IN_RESOLVE_REFERENCE_TOGGLE) else None
+
+    @staticmethod
+    def resolve_system_version_entries(value, namespace=None):
+        """Each system-version entry with the repo version it resolves to (one without an id when unresolved)."""
+        from core.sources.models import Source
+        return [
+            (entry, Source.resolve_reference_expression(entry, namespace)[0])
+            for entry in compact([entry.strip() for entry in (value or '').split(',')])
+        ]
+
+    @classmethod
+    def get_system_version_conflicts(cls, value, namespace=None):
+        """The system-version entries naming different versions of one repository, by the repository's URL.
+        An entry that doesn't resolve still counts, under the repository its URL names and the version it asks for."""
+        from core.sources.models import Source
+        entries_by_repo = {}
+        for entry, version in cls.resolve_system_version_entries(value, namespace):
+            url, requested = canonical_url_to_url_and_version(entry)
+            if version.id:
+                repo_url, requested = drop_version(version.uri), version.version
+            elif is_canonical_uri(url):
+                repo, _ = Source.resolve_reference_expression(url, namespace)
+                repo_url = drop_version(repo.uri) if repo.id else url
+            else:
+                path = url if url.endswith('/') else url + '/'
+                repo_url = drop_version(path)
+                requested = requested or path[len(repo_url):].strip('/') or None
+            entries_by_repo.setdefault(repo_url, {}).setdefault(requested, entry)
+        return {
+            repo_url: list(entries.values()) for repo_url, entries in entries_by_repo.items() if len(entries) > 1
+        }
+
     def add_references(  # pylint: disable=too-many-locals,too-many-statements,too-many-branches,too-many-arguments
             self, references, index=True, is_adding_all=False, attempt_reevaluate=True, force_reevaluate=False):
         include_refs, exclude_refs = self.to_ref_list_separated(references)
@@ -1828,19 +1874,18 @@ def add_references(  # pylint: disable=too-many-locals,too-many-statements,too-m
         include_system_versions = []
         system_versions = self.parameters.get(ExpansionParameters.INCLUDE_SYSTEM)
         if should_reevaluate and system_versions:
-            for system_version in compact(system_versions.split(',')):
-                _system = system_version.strip()
-                _cache_key = f"NONE-{_system}-NONE"
-                version, _ = ConceptContainerModel.resolve_reference_expression(_system)
-                _system_version_cache[_cache_key] = version
+            namespace = self.get_system_version_namespace(self.collection_version.parent_url)
+            for _system, version in self.resolve_system_version_entries(system_versions, namespace):
+                # unresolved is None here too, as a reference resolving the same URL itself would get
+                _system_version_cache[f"{namespace or 'NONE'}-{_system}-NONE"] = version if version.id else None
                 if version.id:
                     include_system_versions.append(version)
                 else:
                     unresolved_repo_versions.append({'url': _system, 'type': 'expansion.parameters.system-version'})
 
         def get_ref_system_version(ref):
             if ref.system:
-                __cache_key = f"{ref.namespace or 'NONE'}-{ref.system}-{ref.version or 'NONE'}"
+                __cache_key = f"{ref.resolution_namespace or 'NONE'}-{ref.system}-{ref.version or 'NONE'}"
                 if __cache_key not in _system_version_cache:
                     _system_version_cache[__cache_key] = ref.resolve_expression_system_version
                 _version = _system_version_cache[__cache_key]
@@ -1894,7 +1939,7 @@ def get_ref_results(ref):   # pylint: disable=too-many-branches,too-many-locals
                     unresolved_repo_versions.append(
                         {
                             'url': ref.system,
-                            'namespace': ref.namespace,
+                            'namespace': ref.resolution_namespace,
                             'version': ref.version,
                             'type': 'reference.system'
                         }