feat: added warning for manifests that don't annotate their parent ar…

…chives; added error for manifests whose parents aren't COMBINE/OMEX archives; closes #76

biosimulators_utils/combine/io.py

@@ -265,15 +265,36 @@ def run(self, in_file, out_dir, include_omex_metadata_files=True, config=None):
 
         # read metadata files skipped by libCOMBINE
         content_locations = set(os.path.relpath(content.location, '.') for content in archive.contents)
+        manifest_contents = self.read_manifest(os.path.join(out_dir, 'manifest.xml'), in_file, config=config)
+
         if include_omex_metadata_files:
-            for manifest_content in self.read_manifest(os.path.join(out_dir, 'manifest.xml'), in_file, config=config):
+            for manifest_content in manifest_contents:
                 if (
                     manifest_content.format
                     and re.match(CombineArchiveContentFormatPattern.OMEX_METADATA.value, manifest_content.format)
                     and os.path.relpath(manifest_content.location, '.') not in content_locations
                 ):
                     archive.contents.append(manifest_content)
 
+        if config.VALIDATE_OMEX_MANIFESTS:
+            manifest_includes_archive = False
+            for manifest_content in manifest_contents:
+                if os.path.relpath(manifest_content.location, '.') == '.':
+                    if manifest_content.format:
+                        if re.match(CombineArchiveContentFormatPattern.OMEX, manifest_content.format):
+                            manifest_includes_archive = True
+                        else:
+                            msg = 'The format of the archive must be `{}`, not `{}`.'.format(
+                                CombineArchiveContentFormat.OMEX, manifest_content.format)
+                            self.errors.append([msg])
+
+            if not manifest_includes_archive:
+                msg = (
+                    'The manifest does not include its parent COMBINE/OMEX archive. '
+                    'Manifests should include their parent COMBINE/OMEX archives.'
+                )
+                self.warnings.append([msg])
+
         # raise warnings and errors
         if self.warnings:
             warn('COMBINE/OMEX archive may be invalid.\n  ' + flatten_nested_list_of_strings(self.warnings).replace('\n', '\n  '),
@@ -306,7 +327,9 @@ def read_manifest(self, filename, archive_filename=None, config=None):
                 return []
             else:
                 try:
-                    return CombineArchiveZipReader().run(archive_filename).contents
+                    contents = CombineArchiveZipReader().run(archive_filename).contents
+                    contents.append(CombineArchiveContent(location='.', format=CombineArchiveContentFormat.ZIP))
+                    return contents
                 except ValueError:
                     msg = "`{}` is not a valid zip file.".format(archive_filename)
                     self.errors.append([msg])
@@ -321,7 +344,9 @@ def read_manifest(self, filename, archive_filename=None, config=None):
         else:
             if errors:
                 try:
-                    return CombineArchiveZipReader().run(archive_filename).contents
+                    contents = CombineArchiveZipReader().run(archive_filename).contents
+                    contents.append(CombineArchiveContent(location='.', format=CombineArchiveContentFormat.ZIP))
+                    return contents
                 except ValueError:
                     msg = "`{}` is not a valid zip file.".format(archive_filename)
                     self.errors.append([msg])
@@ -341,6 +366,7 @@ def read_manifest(self, filename, archive_filename=None, config=None):
                 content.master = content_comb.getMaster()
 
             contents.append(content)
+
         return contents
 
     def _read_metadata(self, archive_comb, filename, obj):

biosimulators_utils/combine/validation.py

@@ -121,12 +121,6 @@ def validate(archive, archive_dirname,
     # validate files
     for content in archive.contents:
         if isinstance(content, CombineArchiveContent) and content.location and content.format:
-            if re.match(CombineArchiveContentFormatPattern.OMEX_MANIFEST, content.format) and content.location != 'manifest.xml':
-                errors.append([(
-                    'COMBINE/OMEX archives should not contain a manifest at location `{}`. '
-                    'COMBINE/OMEX archives should only contain a single manifest at location `manifest.xml`.'
-                ).format(content.location)])
-
             content_errors, content_warnings = validate_content(
                 content, archive_dirname,
                 formats_to_validate=formats_to_validate,

tests/combine/test_combine_io.py

@@ -157,6 +157,16 @@ def test_write_error_handling(self):
             with mock.patch('biosimulators_utils.combine.io.get_combine_errors_warnings', return_value=([], [['my warning']])):
                 io.CombineArchiveWriter().run(archive, self.temp_dir, archive_file)
 
+        archive_file = os.path.join(os.path.dirname(__file__), '..', 'fixtures', 'invalid-parent-format-in-manifest.omex')
+        out_dir = os.path.join(self.temp_dir, 'out-1')
+        with self.assertRaisesRegex(ValueError, 'format of the archive must be'):
+            io.CombineArchiveReader().run(archive_file, out_dir, include_omex_metadata_files=False)
+
+        archive_file = os.path.join(os.path.dirname(__file__), '..', 'fixtures', 'missing-parent-in-manifest.omex')
+        out_dir = os.path.join(self.temp_dir, 'out-2')
+        with self.assertWarnsRegex(BioSimulatorsWarning, 'Manifests should include their parent COMBINE/OMEX archives'):
+            io.CombineArchiveReader().run(archive_file, out_dir, include_omex_metadata_files=False)
+
     def test_read_from_plain_zip_archive(self):
         in_dir = os.path.join(self.temp_dir, 'in')
         os.mkdir(in_dir)
@@ -326,7 +336,7 @@ def test_read_manifest_from_plain_zip(self):
 
         config = Config(VALIDATE_OMEX_MANIFESTS=False)
         archive.contents = io.CombineArchiveReader().read_manifest(manifest_filename, archive_filename, config=config)
-        self.assertEqual(len(archive.contents), 2)
+        self.assertEqual(len(archive.contents), 3)
 
         config = Config(VALIDATE_OMEX_MANIFESTS=True)
         archive.contents = io.CombineArchiveReader().read_manifest(manifest_filename, archive_filename, config=config)

tests/combine/test_combine_validation.py

@@ -234,7 +234,7 @@ def test_manifest_in_manifest(self):
         out_dir = os.path.join(self.tmp_dir, 'out')
         archive = CombineArchiveReader().run(os.path.join(os.path.dirname(__file__), '..', 'fixtures', 'multiple-manifests.omex'), out_dir)
         errors, warnings = validate(archive, out_dir)
-        self.assertIn('should not contain a manifest at location', flatten_nested_list_of_strings(errors))
+        self.assertEqual(errors, [])
         self.assertIn('manifests should not contain content entries for themselves', flatten_nested_list_of_strings(warnings))
 
     def test_no_validation(self):