Fix: Invalid columns in compact manifest for AnVIL (#6110)

src/azul/plugins/metadata/anvil/__init__.py

@@ -133,6 +133,9 @@ def _field_mapping(self) -> MetadataPlugin._FieldMapping:
             'document_id',
             'source_datarepo_row_ids'
         ]
+        # Note that there is a brittle coupling that must be maintained between
+        # the keys here and those used in the custom field name lookup in
+        # `self.manifest_config`.
         return {
             'entity_id': 'entryId',
             'bundles': {
@@ -278,10 +281,28 @@ def manifest_config(self) -> ManifestConfig:
         # Note that there is a brittle coupling that must be maintained between
         # the fields listed here and those used in `self._field_mapping`.
         fields_to_omit_from_manifest = [
+            ('contents', 'activities', 'activity_table'),
             ('contents', 'files', 'uuid'),
             ('contents', 'files', 'version'),
         ]
 
+        # This is used to modify the field names from how they are specified in
+        # the field mapping. Keys are field paths in an ES hit, and values are
+        # the desired manifest column name, or None to omit the column from the
+        # manifest.
+        #
+        # Note that there is a brittle coupling that must be maintained between
+        # the keys here and those used in `self._field_mapping`. Also, the
+        # values should match the related field's path in a response hit from
+        # the `/index/files` endpoint.
+        #
+        custom_field_names = {
+            ('bundles', 'uuid'): 'bundles.bundle_uuid',
+            ('bundles', 'version'): 'bundles.bundle_version',
+            ('sources', 'id'): 'sources.source_id',
+            ('sources', 'spec'): 'sources.source_spec',
+        }
+
         def recurse(mapping: MetadataPlugin._FieldMapping, path: FieldPath):
             for path_element, name_or_type in mapping.items():
                 new_path = (*path, path_element)
@@ -293,17 +314,20 @@ def recurse(mapping: MetadataPlugin._FieldMapping, path: FieldPath):
                     elif new_path in fields_to_omit_from_manifest:
                         result[path][path_element] = None
                         fields_to_omit_from_manifest.remove(new_path)
+                    elif new_path in custom_field_names:
+                        result[path][path_element] = custom_field_names.pop(new_path)
                     else:
                         result[path][path_element] = name_or_type
                 else:
                     assert False, (path, path_element, name_or_type)
 
         recurse(self._field_mapping, ())
         assert len(fields_to_omit_from_manifest) == 0, fields_to_omit_from_manifest
+        assert len(custom_field_names) == 0, custom_field_names
         # The file URL is synthesized from the `uuid` and `version` fields.
         # Above, we already configured these two fields to be omitted from the
         # manifest since they are not informative to the user.
-        result[('contents', 'files')]['file_url'] = 'files.file_url'
+        result[('contents', 'files')]['file_url'] = 'files.azul_file_url'
         return result
 
     def verbatim_pfb_schema(self,

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

@@ -139,6 +139,9 @@ def choose_entry(_term):
     def _make_hit(self, es_hit: JSON) -> MutableJSON:
         return {
             'entryId': es_hit['entity_id'],
+            # Note that there is a brittle coupling that must be maintained
+            # between the `sources` and `bundles` field paths and the values in
+            # the custom field name lookup in `Plugin.manifest_config`.
             'sources': list(map(self._make_source, es_hit['sources'])),
             'bundles': list(map(self._make_bundle, es_hit['bundles'])),
             **self._make_contents(es_hit['contents'])

test/integration_test.py

@@ -749,6 +749,14 @@ def _project_type(self, catalog: CatalogName) -> EntityType:
         else:
             assert False, catalog
 
+    def _uuid_column_name(self, catalog: CatalogName) -> str:
+        if config.is_hca_enabled(catalog):
+            return 'bundle_uuid'
+        elif config.is_anvil_enabled(catalog):
+            return 'bundles.bundle_uuid'
+        else:
+            assert False, catalog
+
     def _test_dos_and_drs(self, catalog: CatalogName):
         if config.is_dss_enabled(catalog) and config.dss_direct_access:
             _, file = self._get_one_inner_file(catalog)
@@ -927,8 +935,8 @@ def _assertResponseStatus(self,
             )
         )
 
-    def _check_compact_manifest(self, _catalog: CatalogName, response: bytes):
-        self.__check_csv_manifest(BytesIO(response), 'bundle_uuid')
+    def _check_compact_manifest(self, catalog: CatalogName, response: bytes):
+        self.__check_csv_manifest(BytesIO(response), self._uuid_column_name(catalog))
 
     def _check_terra_bdbag_manifest(self, catalog: CatalogName, response: bytes):
         with ZipFile(BytesIO(response)) as zip_fh:
@@ -1047,14 +1055,14 @@ def _read_csv_manifest(self, file: IO[bytes]) -> csv.DictReader:
 
     def __check_csv_manifest(self,
                              file: IO[bytes],
-                             uuid_field_name: str
+                             uuid_column_name: str
                              ) -> list[Mapping[str, str]]:
         reader = self._read_csv_manifest(file)
         rows = list(reader)
         log.info(f'Manifest contains {len(rows)} rows.')
         self.assertGreater(len(rows), 0)
-        self.assertIn(uuid_field_name, reader.fieldnames)
-        bundle_uuids = rows[0][uuid_field_name].split(ManifestGenerator.padded_joiner)
+        self.assertIn(uuid_column_name, reader.fieldnames)
+        bundle_uuids = rows[0][uuid_column_name].split(ManifestGenerator.padded_joiner)
         self.assertGreater(len(bundle_uuids), 0)
         for bundle_uuid in bundle_uuids:
             self.assertEqual(bundle_uuid, str(uuid.UUID(bundle_uuid)))
@@ -1605,9 +1613,10 @@ def bundle_uuids(hit: JSON) -> set[str]:
         def test_compact_manifest(expected_bundles):
             manifest = BytesIO(self._get_url_content(PUT, manifest_url))
             manifest_rows = self._read_csv_manifest(manifest)
+            uuid_column_name = self._uuid_column_name(catalog)
             all_found_bundles = set()
             for row in manifest_rows:
-                row_bundles = set(row['bundle_uuid'].split(ManifestGenerator.padded_joiner))
+                row_bundles = set(row[uuid_column_name].split(ManifestGenerator.padded_joiner))
                 # It's possible for one file to be present in multiple
                 # bundles (e.g. due to stitching), so each row may include
                 # additional bundles besides those included in the filters.

test/service/test_manifest.py

@@ -1716,25 +1716,25 @@ def test_compact_manifest(self):
         self.assertEqual(200, response.status_code)
         expected = [
             (
-                'bundle_uuid',
+                'bundles.bundle_uuid',
                 '6b0f6c0f-5d80-a242-accb-840921351cd5',
                 '826dea02-e274-affe-aabc-eb3db63ad068',
                 '826dea02-e274-affe-aabc-eb3db63ad068'
             ),
             (
-                'bundle_version',
+                'bundles.bundle_version',
                 '2022-06-01T00:00:00.000000Z',
                 '2022-06-01T00:00:00.000000Z',
                 '2022-06-01T00:00:00.000000Z'
             ),
             (
-                'source_id',
+                'sources.source_id',
                 '6c87f0e1-509d-46a4-b845-7584df39263b',
                 '6c87f0e1-509d-46a4-b845-7584df39263b',
                 '6c87f0e1-509d-46a4-b845-7584df39263b'
             ),
             (
-                'source_spec',
+                'sources.source_spec',
                 'tdr:bigquery:gcp:test_anvil_project:anvil_snapshot:/2',
                 'tdr:bigquery:gcp:test_anvil_project:anvil_snapshot:/2',
                 'tdr:bigquery:gcp:test_anvil_project:anvil_snapshot:/2'
@@ -1973,12 +1973,6 @@ def test_compact_manifest(self):
                 '18b3be87-e26b-4376-0d8d-c1e370e90e07',
                 'a60c5138-3749-f7cb-8714-52d389ad5231'
             ),
-            (
-                'activities.activity_table',
-                '',
-                'sequencingactivity',
-                'sequencingactivity'
-            ),
             (
                 'activities.activity_type',
                 '',
@@ -2082,7 +2076,7 @@ def test_compact_manifest(self):
                 self._drs_uri('v1_6c87f0e1-509d-46a4-b845-7584df39263b_8b722e88-8103-49c1-b351-e64fa7c6ab37')
             ),
             (
-                'files.file_url',
+                'files.azul_file_url',
                 self._file_url('6b0f6c0f-5d80-4242-accb-840921351cd5', self.version),
                 self._file_url('15b76f9c-6b46-433f-851d-34e89f1b9ba6', self.version),
                 self._file_url('3b17377b-16b1-431c-9967-e5d01fc5923f', self.version)