If modulemd-packager documents are found while reading directly into a module index,

report them as subdocument failures and discard them.

Signed-off-by: Merlin Mathesius <[email protected]>

Author: mmathesius

modulemd/modulemd-module-index.c

@@ -155,51 +155,12 @@ add_subdoc (ModulemdModuleIndex *self,
   switch (doctype)
     {
     case MODULEMD_YAML_DOC_PACKAGER:
-      if (modulemd_subdocument_info_get_mdversion (subdoc) <
-          MD_PACKAGER_VERSION_TWO)
-        {
-          g_set_error (error,
-                       MODULEMD_YAML_ERROR,
-                       MMD_YAML_ERROR_PARSE,
-                       "Invalid mdversion for a packager document");
-          return FALSE;
-        }
-
-      if (modulemd_subdocument_info_get_mdversion (subdoc) ==
-          MD_PACKAGER_VERSION_THREE)
-        {
-          packager = modulemd_packager_v3_parse_yaml (subdoc, error);
-
-          index =
-            modulemd_packager_v3_to_stream_v2_ext (packager, &nested_error);
-          if (!index)
-            {
-              g_propagate_error (error, g_steal_pointer (&nested_error));
-              return FALSE;
-            }
-
-          if (autogen_module_name)
-            {
-              /* nothing to do here since module/stream names were already
-               * auto-generated in order to add them to the index
-               */
-            }
-
-          /* merge index with override = FALSE and strict_default_streams = TRUE */
-          if (!modulemd_module_index_merge (
-                index, self, FALSE, TRUE, &nested_error))
-            {
-              g_propagate_error (error, g_steal_pointer (&nested_error));
-              return FALSE;
-            }
-
-          g_clear_object (&index);
-          break;
-        }
-
-      /* Fall through intentionally
-       * We will handle the v2 packager format below
-       */
+      g_set_error (error,
+                   MODULEMD_YAML_ERROR,
+                   MMD_YAML_ERROR_PARSE,
+                   "modulemd-packager document ignored while reading into a "
+                   "module index");
+      return FALSE;
 
     case MODULEMD_YAML_DOC_MODULESTREAM:
       switch (modulemd_subdocument_info_get_mdversion (subdoc))
@@ -212,7 +173,7 @@ add_subdoc (ModulemdModuleIndex *self,
         case MD_MODULESTREAM_VERSION_TWO:
           stream =
             MODULEMD_MODULE_STREAM (modulemd_module_stream_v2_parse_yaml (
-              subdoc, strict, doctype == MODULEMD_YAML_DOC_PACKAGER, error));
+              subdoc, strict, FALSE, error));
           break;
 
         default:

modulemd/tests/test-modulemd-moduleindex.c

@@ -196,6 +196,7 @@ module_index_test_read (void)
   g_autoptr (GPtrArray) failures = NULL;
   ModulemdSubdocumentInfo *subdoc = NULL;
   g_autofree gchar *yaml_path = NULL;
+  g_auto (GStrv) module_names = NULL;
 
   /* Read the specification files all in */
   index = modulemd_module_index_new ();
@@ -223,24 +224,31 @@ module_index_test_read (void)
   g_clear_pointer (&yaml_path, g_free);
   g_clear_pointer (&failures, g_ptr_array_unref);
 
-  /* The modulemd-packager definition
-   * This should fail to be read into a ModuleIndex because it provides no
-   * NSVCA information. It should only be importable via
-   * ModuleStream.read_file()
+  /* The modulemd-packager v2 definition
+   * Reading a packager document into an index can only be done by calling
+   * modulemd_read_packager_file(). Attempting to read it directly
+   * using modulemd_module_index_update_from_file() should ignore the document
+   * and report it as a failure.
    */
   yaml_path = g_strdup_printf ("%s/yaml_specs/modulemd_packager_v2.yaml",
                                g_getenv ("MESON_SOURCE_ROOT"));
   ret = modulemd_module_index_update_from_file (
     index, yaml_path, TRUE, &failures, &error);
+  modulemd_subdocument_info_debug_dump_failures (failures);
   g_assert_no_error (error);
   g_assert_cmpint (failures->len, ==, 1);
   g_assert_false (ret);
   g_assert_error (
     modulemd_subdocument_info_get_gerror (g_ptr_array_index (failures, 0)),
-    MODULEMD_ERROR,
-    MMD_ERROR_MISSING_REQUIRED);
+    MODULEMD_YAML_ERROR,
+    MMD_YAML_ERROR_PARSE);
   g_clear_pointer (&yaml_path, g_free);
   g_clear_pointer (&failures, g_ptr_array_unref);
+  /* the index must still contain only the "foo" module */
+  module_names = modulemd_module_index_get_module_names_as_strv (index);
+  g_assert_cmpint (g_strv_length (module_names), ==, 1);
+  g_assert_true (g_strv_contains ((const gchar *const *)module_names, "foo"));
+  g_clear_pointer (&module_names, g_strfreev);
 
   /* The translation definitions */
   yaml_path = g_strdup_printf ("%s/yaml_specs/modulemd_translations_v1.yaml",
@@ -373,6 +381,58 @@ module_index_test_read_mixed (void)
 }
 
 
+static void
+module_index_test_read_mixed_packager (void)
+{
+  gboolean ret;
+  g_autoptr (ModulemdModuleIndex) index = NULL;
+  g_autofree gchar *yaml_path = NULL;
+  g_autoptr (GPtrArray) failures = NULL;
+  g_autoptr (GError) error = NULL;
+  g_autofree gchar *output = NULL;
+  g_auto (GStrv) module_names = NULL;
+
+  index = modulemd_module_index_new ();
+
+  /* Reading a packager document into an index can only be done by calling
+   * modulemd_read_packager_file(). Attempting to directly read a file
+   * containing a mix of stream and packager documents using
+   * modulemd_module_index_update_from_file() should trigger errors and
+   * ignore the packager documents while successfully loading the streams.
+   */
+  yaml_path = g_strdup_printf ("%s/stream_packager_mix.yaml",
+                               g_getenv ("TEST_DATA_PATH"));
+  g_assert_nonnull (yaml_path);
+
+  ret = modulemd_module_index_update_from_file (
+    index, yaml_path, TRUE, &failures, &error);
+  modulemd_subdocument_info_debug_dump_failures (failures);
+  g_assert_no_error (error);
+  g_assert_cmpint (failures->len, ==, 2);
+  g_assert_false (ret);
+  g_assert_error (
+    modulemd_subdocument_info_get_gerror (g_ptr_array_index (failures, 0)),
+    MODULEMD_YAML_ERROR,
+    MMD_YAML_ERROR_PARSE);
+  g_assert_error (
+    modulemd_subdocument_info_get_gerror (g_ptr_array_index (failures, 1)),
+    MODULEMD_YAML_ERROR,
+    MMD_YAML_ERROR_PARSE);
+  /* Make sure the streams were successfully loaded into the index */
+  module_names = modulemd_module_index_get_module_names_as_strv (index);
+  g_assert_cmpint (g_strv_length (module_names), ==, 2);
+  g_assert_true (g_strv_contains ((const gchar *const *)module_names, "foo"));
+  g_assert_true (g_strv_contains ((const gchar *const *)module_names, "qux"));
+  g_clear_pointer (&module_names, g_strfreev);
+  g_clear_pointer (&failures, g_ptr_array_unref);
+
+  /* Verify that we can output it cleanly */
+  output = modulemd_module_index_dump_to_string (index, &error);
+  g_assert_nonnull (output);
+  g_assert_null (error);
+}
+
+
 static void
 module_index_test_read_unknown (void)
 {
@@ -1810,6 +1870,9 @@ main (int argc, char *argv[])
   g_test_add_func ("/modulemd/v2/module/index/read/mixed",
                    module_index_test_read_mixed);
 
+  g_test_add_func ("/modulemd/v2/module/index/read/mixed_packager",
+                   module_index_test_read_mixed_packager);
+
   g_test_add_func ("/modulemd/v2/module/index/read/unknown",
                    module_index_test_read_unknown);
 

modulemd/tests/test-modulemd-packager-v3.c

@@ -602,50 +602,40 @@ packager_test_read_to_index (void)
   g_autoptr (GError) error = NULL;
   g_autoptr (GPtrArray) failures = NULL;
   g_autofree gchar *yaml_path = NULL;
-  g_autofree gchar *yaml_str = NULL;
-  g_autofree gchar *expected_path = NULL;
-  g_autofree gchar *expected_str = NULL;
+  g_auto (GStrv) module_names = NULL;
 
   /* create an index */
   index = modulemd_module_index_new ();
 
-  /* The modulemd-packager v3 definition */
+  /* The modulemd-packager v3 definition
+   * Reading a packager v3 document into an index can only be done by calling
+   * modulemd_read_packager_file() and then calling
+   * modulemd_packager_v3_convert_to_index(), or internally by calling
+   * modulemd_packager_v3_to_stream_v2_ext(). Attempting to read it directly
+   * using modulemd_module_index_update_from_file() should trigger a warning
+   * and ignore the document.
+   */
   yaml_path = g_strdup_printf ("%s/yaml_specs/modulemd_packager_v3.yaml",
                                g_getenv ("MESON_SOURCE_ROOT"));
   ret = modulemd_module_index_update_from_file (
     index, yaml_path, TRUE, &failures, &error);
   g_assert_no_error (error);
   modulemd_subdocument_info_debug_dump_failures (failures);
-  g_assert_true (ret);
-  g_assert_cmpint (failures->len, ==, 0);
+  g_assert_cmpint (failures->len, ==, 1);
+  g_assert_false (ret);
+   g_assert_error (
+     modulemd_subdocument_info_get_gerror (g_ptr_array_index (failures, 0)),
+    MODULEMD_YAML_ERROR,
+    MMD_YAML_ERROR_PARSE);
   g_clear_pointer (&yaml_path, g_free);
   g_clear_pointer (&failures, g_ptr_array_unref);
 
-  g_assert_cmpint (MD_MODULESTREAM_VERSION_TWO,
-                   ==,
-                   modulemd_module_index_get_stream_mdversion (index));
-
-  yaml_str = modulemd_module_index_dump_to_string (index, &error);
-  g_assert_no_error (error);
-  g_assert_nonnull (yaml_str);
-
-  g_debug ("packager_test_read_to_index() dump of v2 index:\n%s", yaml_str);
-
-  /* the index should contain the packager v2 document converted to stream v2 */
-  expected_path = g_strdup_printf ("%s/upgrades/packager_v3_to_stream_v2.yaml",
-                                   g_getenv ("TEST_DATA_PATH"));
-  g_assert_nonnull (expected_path);
-  g_assert_true (
-    g_file_get_contents (expected_path, &expected_str, NULL, &error));
-  g_assert_no_error (error);
-  g_assert_nonnull (expected_str);
-  g_clear_pointer (&expected_path, g_free);
-
-  g_assert_cmpstr (expected_str, ==, yaml_str);
+  /* the index must still be empty */
+  module_names = modulemd_module_index_get_module_names_as_strv (index);
+  g_assert_cmpint (g_strv_length (module_names), ==, 0);
+  g_clear_pointer (&module_names, g_strfreev);
 
   g_clear_object (&index);
-  g_clear_pointer (&yaml_str, g_free);
-  g_clear_pointer (&expected_str, g_free);
 }
 
 

modulemd/tests/test_data/stream_packager_mix.yaml

@@ -0,0 +1,43 @@
+---
+document: modulemd
+version: 1
+data:
+  name: foo
+  stream: a
+  summary: Example 1
+  description: Description 1
+  license:
+    module:
+    - MIT
+...
+---
+document: modulemd-packager
+version: 2
+data:
+  summary: Example 2
+  description: Description 2
+  license:
+    module:
+    - MIT
+...
+---
+document: modulemd-packager
+version: 3
+data:
+  name: baz
+  stream: c
+  summary: Example 3
+  description: Description 3
+...
+---
+document: modulemd
+version: 2
+data:
+  name: qux
+  stream: d
+  summary: Example 4
+  description: Description 4
+  license:
+    module:
+    - MIT
+...