Fix: Redundant implementations of file URLs and mirror URIs (#8042)

src/azul/plugins/metadata/anvil/service/response.py

@@ -32,7 +32,6 @@
     json_sequence_of_mappings,
     json_str,
     json_untyped_dict,
-    optional,
 )
 from azul.plugins import (
     SpecialFields,
@@ -227,9 +226,7 @@ def _pivotal_entity(self,
                         ) -> MutableJSON:
         inner_entity = copy_json(inner_entity)
         if inner_entity_type == 'files':
-            inner_entity['azul_url'] = self._file_url(uuid=json_str(inner_entity['document_id']),
-                                                      version=json_str(inner_entity['version']),
-                                                      drs_uri=optional(json_str, inner_entity['drs_uri']))
+            inner_entity['azul_url'] = self._file_url(inner_entity)
             inner_entity['azul_mirror_uri'] = self._file_mirror_uri(source, inner_entity)
             inner_entity.pop('version', None)
         return inner_entity

src/azul/plugins/metadata/hca/service/response.py

@@ -419,9 +419,7 @@ def make_file(self, source: SourceRef, file: JSON) -> JSON:
             'version': file.get('version'),
             'matrixCellCount': file.get('matrix_cell_count'),
             'drs_uri': file.get('drs_uri'),
-            'azul_url': self._file_url(uuid=json_str(file['uuid']),
-                                       version=json_str(file['version']),
-                                       drs_uri=optional(json_str, file['drs_uri'])),
+            'azul_url': self._file_url(file),
             'azul_mirror_uri': self._file_mirror_uri(source, file),
         }
         return translated_file

src/azul/service/drs_controller.py

@@ -67,7 +67,7 @@ class DRSController(ServiceController):
 
     @cached_property
     def _service(self) -> IndexService:
-        return IndexService()
+        return IndexService(file_url_func=self._file_url)
 
     _drs_spec_description = fd('''
         This is a partial implementation of the [DRS 1.0.0 spec][1]. Not all

src/azul/service/index_controller.py

@@ -70,7 +70,7 @@ class IndexController(QueryController):
 
     @cached_property
     def _service(self) -> IndexService:
-        return IndexService()
+        return IndexService(file_url_func=self._file_url)
 
     _min_page_size = 1
 
@@ -365,7 +365,6 @@ def search(self, entity_type: str, entity_id: str | None = None) -> str | JSON:
         try:
             response = self._service.search(catalog=self.app.catalog,
                                             entity_type=entity_type,
-                                            file_url_func=self._file_url,
                                             item_id=entity_id,
                                             filters=filters,
                                             pagination=pagination)

src/azul/service/index_service.py

@@ -32,12 +32,6 @@
 from azul.filters import (
     Filters,
 )
-from azul.indexer.mirror_service import (
-    MirrorService,
-)
-from azul.lib import (
-    cache,
-)
 from azul.lib.types import (
     JSON,
     MutableJSON,
@@ -51,14 +45,13 @@
 )
 from azul.service import (
     BadArgumentException,
-    FileUrlFunc,
 )
 from azul.service.query_service import (
+    FileUrlService,
     IndexNotFoundError,
     OpenSearchStage,
     Pagination,
     PaginationStage,
-    QueryService,
     ResponseTriple,
     ToDictStage,
     _OpenSearchStage,
@@ -80,25 +73,15 @@ def __init__(self, entity_type: str, entity_id: str):
 class SearchResponseStage(_OpenSearchStage[ResponseTriple, MutableJSON],
                           metaclass=ABCMeta):
     service: IndexService
-    file_url_func: FileUrlFunc
 
     def prepare_request(self, request: Search) -> Search:
         return request
 
-    def _file_url(self, *, uuid: str, version: str, drs_uri: str | None) -> str | None:
-        if drs_uri is None:
-            # To download a file we need its DRS URI
-            return None
-        else:
-            return str(self.file_url_func(catalog=self.catalog,
-                                          fetch=False,
-                                          file_uuid=uuid,
-                                          version=version))
+    def _file_url(self, file: JSON) -> str | None:
+        return self.service.azul_file_url(self.catalog, file)
 
     def _file_mirror_uri(self, source: SourceRef, file: JSON) -> str | None:
-        file_cls = self.plugin.file_class
-        mirror_service = self.service.mirror_service(self.catalog)
-        return mirror_service.mirror_uri(source, file_cls, file)
+        return self.service.azul_mirror_uri(self.catalog, source, file)
 
 
 class SummaryResponseStage(OpenSearchStage[JSON, MutableJSON],
@@ -113,17 +96,13 @@ def prepare_request(self, request: Search) -> Search:
         return request
 
 
-class IndexService(QueryService):
-
-    @cache
-    def mirror_service(self, catalog: CatalogName) -> MirrorService:
-        return MirrorService(catalog=catalog)
+@attrs.frozen(auto_attribs=True, kw_only=True)
+class IndexService(FileUrlService):
 
     def search(self,
                *,
                catalog: CatalogName,
                entity_type: str,
-               file_url_func: FileUrlFunc,
                item_id: str | None,
                filters: Filters,
                pagination: Pagination
@@ -135,9 +114,6 @@ def search(self,
         :param pagination: A dictionary with pagination information as return from `_get_pagination()`
         :param filters: parsed JSON filters from the request
         :param item_id: If item_id is specified, only a single item is searched for
-        :param file_url_func: A function that is used only when getting a *list* of files data.
-        It creates the files URL based on info from the request. It should have the type
-        signature `(uuid: str, **params) -> str`
         :return: The OpenSearch JSON response
         """
         if item_id is not None:
@@ -148,8 +124,7 @@ def search(self,
                                 filters=filters,
                                 pagination=pagination,
                                 aggregate=item_id is None,
-                                entity_type=entity_type,
-                                file_url_func=file_url_func)
+                                entity_type=entity_type)
 
         special_fields = self.metadata_plugin(catalog).special_fields
         for hit in response['hits']:
@@ -169,7 +144,6 @@ def _search(self,
                 aggregate: bool,
                 filters: Filters,
                 pagination: Pagination,
-                file_url_func: FileUrlFunc
                 ) -> MutableJSON:
         """
         This function does the whole transformation process. It takes the path
@@ -225,8 +199,7 @@ def _search(self,
         response_stage_cls = plugin.search_response_stage
         chain = response_stage_cls(service=self,
                                    catalog=catalog,
-                                   entity_type=entity_type,
-                                   file_url_func=file_url_func).wrap(chain)
+                                   entity_type=entity_type).wrap(chain)
 
         request = self.create_request(catalog, entity_type)
         request = chain.prepare_request(request)

src/azul/service/manifest_service.py

@@ -166,17 +166,16 @@
     manifest_config_to_json,
 )
 from azul.service import (
-    FileUrlFunc,
     avro_pfb,
 )
 from azul.service.avro_pfb import (
     PFBRelation,
 )
 from azul.service.query_service import (
+    FileUrlService,
     OpenSearchChain,
     Pagination,
     PaginationStage,
-    QueryService,
     SortKey,
     ToDictStage,
     sort_key_from_json,
@@ -573,8 +572,7 @@ class CachedManifestNotFound(Exception):
 
 
 @attrs.frozen(kw_only=True)
-class ManifestService(QueryService):
-    file_url_func: FileUrlFunc
+class ManifestService(FileUrlService):
 
     @cached_property
     def storage_service(self) -> StorageService:
@@ -810,9 +808,9 @@ def format(cls) -> ManifestFormat:
     def metadata_plugin(self) -> MetadataPlugin:
         return self.service.metadata_plugin(self.catalog)
 
-    @cached_property
+    @property
     def mirror_service(self) -> MirrorService:
-        return MirrorService(catalog=self.catalog)
+        return self.service.mirror_service(self.catalog)
 
     @classmethod
     @abstractmethod
@@ -964,7 +962,6 @@ def __init__(self,
         self.service = service
         self.catalog = catalog
         self.filters = filters
-        self.file_url_func = service.file_url_func
 
     manifest_namespace = UUID('ca1df635-b42c-4671-9322-b0a7209f0235')
 
@@ -1142,20 +1139,10 @@ def _azul_file_url(self,
                        file: JSON,
                        args: Mapping = frozendict()
                        ) -> str | None:
-        if file['drs_uri'] is None:
-            # To download a file we need its DRS URI
-            return None
-        else:
-            special_fields = self.metadata_plugin.special_fields
-            return str(self.file_url_func(catalog=self.catalog,
-                                          file_uuid=json_str(file[special_fields.file_uuid.name_in_hit]),
-                                          version=json_str(file['version']),
-                                          fetch=False,
-                                          **args))
+        return self.service.azul_file_url(self.catalog, file, args)
 
     def _azul_mirror_uri(self, source: SourceRef, file: JSON) -> str | None:
-        file_cls = self.metadata_plugin.file_class
-        return self.mirror_service.mirror_uri(source, file_cls, file)
+        return self.service.azul_mirror_uri(self.catalog, source, file)
 
     @cache
     def _content_hash(self, *, by_bundle: bool) -> str: