Remove `allow_out_of_order` option for `xml_parse_main_metadata_together`

The option was causing issues when repository has duplicate packages and
it was providing little value. All repos should really have the same
order of packages in primary, filelists and other metadata.

For: https://github.com/rpm-software-management/createrepo_c/issues/305

6 files changed, 33 insertions, 78 deletions

diff --git a/examples/python/repodata_parsing.py b/examples/python/repodata_parsing.py
index 4519518..518962a 100755
--- a/examples/python/repodata_parsing.py
+++ b/examples/python/repodata_parsing.py
@@ -251,8 +251,7 @@ def third_method():
                                         other_xml_path,
                                         None,
                                         pkgcb,
-                                        warningcb,
-                                        False)
+                                        warningcb)
 
 if __name__ == "__main__":
     print('"All in one shot" method:')
diff --git a/src/python/xml_parser-py.c b/src/python/xml_parser-py.c
index 9d3d5a5..dba9eb5 100644
--- a/src/python/xml_parser-py.c
+++ b/src/python/xml_parser-py.c
@@ -715,16 +715,15 @@ py_xml_parse_main_metadata_together(G_GNUC_UNUSED PyObject *self, PyObject *args
     char *primary_filename;
     char *filelists_filename;
     char *other_filename;
-    int allow_out_of_order = 1;
     PyObject *py_newpkgcb, *py_pkgcb, *py_warningcb;
     CbData cbdata;
     GError *tmp_err = NULL;
     static char *kwlist[] = { "primary", "filelists", "other", "newpkgcb", "pkgcb",
-                              "warningcb", "allow_out_of_order", NULL };
+                              "warningcb", NULL };
 
-    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "sssOOO|p:py_xml_parse_main_metadata_together", kwlist,
+    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "sssOOO:py_xml_parse_main_metadata_together", kwlist,
                                      &primary_filename, &filelists_filename, &other_filename, &py_newpkgcb,
-                                     &py_pkgcb, &py_warningcb, &allow_out_of_order)) {
+                                     &py_pkgcb, &py_warningcb )) {
         return NULL;
     }
 
@@ -777,7 +776,6 @@ py_xml_parse_main_metadata_together(G_GNUC_UNUSED PyObject *self, PyObject *args
                                         &cbdata,
                                         ptr_c_warningcb,
                                         &cbdata,
-                                        allow_out_of_order,
                                         &tmp_err);
 
     Py_XDECREF(py_newpkgcb);
diff --git a/src/xml_parser.h b/src/xml_parser.h
index 5ce7c0e..c8909a2 100644
--- a/src/xml_parser.h
+++ b/src/xml_parser.h
@@ -297,10 +297,6 @@ cr_xml_parse_updateinfo(const char *path,
  * @param pkgcb_data         User data for the pkgcb.
  * @param warningcb          Callback for warning messages.
  * @param warningcb_data     User data for the warningcb.
- * @param allow_out_of_order Whether we should allow different order of packages
- *                           among the main metadata files. If allowed, the more
- *                           the order varies the more memory we will need to
- *                           store all the started but unfinished packages.
  * @param err                GError **
  * @return                   cr_Error code.
  */
@@ -314,7 +310,6 @@ cr_xml_parse_main_metadata_together(const char *primary_path,
                                     void *pkgcb_data,
                                     cr_XmlParserWarningCb warningcb,
                                     void *warningcb_data,
-                                    gboolean allow_out_of_order,
                                     GError **err);
 
 /** @} */
diff --git a/src/xml_parser_main_metadata_together.c b/src/xml_parser_main_metadata_together.c
index f2632a2..d8619a5 100644
--- a/src/xml_parser_main_metadata_together.c
+++ b/src/xml_parser_main_metadata_together.c
@@ -31,8 +31,7 @@
 #define ERR_DOMAIN      CREATEREPO_C_ERROR
 
 typedef struct {
-    GHashTable           *in_progress_pkgs_hash; //used only when allowing out of order pkgs
-    GSList               *in_progress_pkgs_list; // used only when not allowing out of order pkgs
+    GSList               *in_progress_pkgs_list;
     int                  in_progress_count_primary;
     int                  in_progress_count_filelists;
     int                  in_progress_count_other;
@@ -48,12 +47,8 @@ call_user_callback_if_package_finished(cr_Package *pkg, cr_CbData *cb_data, GErr
     if (pkg && (pkg->loadingflags & CR_PACKAGE_LOADED_PRI) && (pkg->loadingflags & CR_PACKAGE_LOADED_OTH) &&
         (pkg->loadingflags & CR_PACKAGE_LOADED_FIL))
     {
-        if (cb_data->in_progress_pkgs_hash) {
-            g_hash_table_remove(cb_data->in_progress_pkgs_hash, pkg->pkgId);
-        } else {
-            //remove first element in the list
-            cb_data->in_progress_pkgs_list = cb_data->in_progress_pkgs_list->next;
-        }
+        //remove first element in the list
+        cb_data->in_progress_pkgs_list = cb_data->in_progress_pkgs_list->next;
 
         // One package was fully finished
         cb_data->in_progress_count_primary--;
@@ -80,36 +75,24 @@ call_user_callback_if_package_finished(cr_Package *pkg, cr_CbData *cb_data, GErr
 static cr_Package*
 find_in_progress_pkg(cr_CbData *cb_data, const char *pkgId, int in_progress_pkg_index, GError **err)
 {
-    gpointer pval = NULL;
-    if (cb_data->in_progress_pkgs_hash) {
-        if (!g_hash_table_lookup_extended(cb_data->in_progress_pkgs_hash, pkgId, NULL, &pval)) {
-            pval = NULL;
-        }
-    } else {
-        // This is checking out of order pkgs because if we don't have in_progress_pkgs_hash -> we enforce
-        // order by using a list
-        pval = g_slist_nth_data(cb_data->in_progress_pkgs_list, in_progress_pkg_index);
-        if (pval && g_strcmp0(((cr_Package *) pval)->pkgId, pkgId)) {
-            g_set_error(err, ERR_DOMAIN, CRE_XMLPARSER,
-                        "Out of order metadata: %s vs %s.", ((cr_Package *) pval)->pkgId, pkgId);
-            pval = NULL;
-        }
+    gpointer pval = g_slist_nth_data(cb_data->in_progress_pkgs_list, in_progress_pkg_index);
+    // This is checking out of order pkgs
+    if (pval && g_strcmp0(((cr_Package *) pval)->pkgId, pkgId)) {
+        g_set_error(err, ERR_DOMAIN, CRE_XMLPARSER,
+                    "Out of order metadata: %s vs %s.", ((cr_Package *) pval)->pkgId, pkgId);
+        pval = NULL;
     }
 
     return pval;
 }
 
 static void
-store_in_progress_pkg(cr_CbData *cb_data, cr_Package *pkg, const char *pkgId)
+store_in_progress_pkg(cr_CbData *cb_data, cr_Package *pkg)
 {
     if (!pkg) {
         return;
     }
-    if (cb_data->in_progress_pkgs_hash) {
-        g_hash_table_insert(cb_data->in_progress_pkgs_hash, g_strdup(pkgId), pkg);
-    } else {
-        cb_data->in_progress_pkgs_list = g_slist_append(cb_data->in_progress_pkgs_list, pkg);
-    }
+    cb_data->in_progress_pkgs_list = g_slist_append(cb_data->in_progress_pkgs_list, pkg);
 }
 
 static int
@@ -171,7 +154,7 @@ newpkg_general(cr_Package **pkg,
             *pkg = cr_package_new();
         }
 
-        store_in_progress_pkg(cb_data, *pkg, pkgId);
+        store_in_progress_pkg(cb_data, *pkg);
     }
 
     if (*err) {
@@ -257,7 +240,7 @@ pkgcb_primary(cr_Package *pkg, void *cbdata, G_GNUC_UNUSED GError **err)
                 }
                 // user_created_pkg can be NULL if newpkgcb returns OK but
                 // not an allocated pkg -> this means we should skip it
-                store_in_progress_pkg(cb_data, user_created_pkg, pkg->pkgId);
+                store_in_progress_pkg(cb_data, user_created_pkg);
                 cr_package_free(pkg);
                 pkg = user_created_pkg;
             }
@@ -267,7 +250,7 @@ pkgcb_primary(cr_Package *pkg, void *cbdata, G_GNUC_UNUSED GError **err)
                 g_clear_error(&out_of_order_err);
             }
         } else {
-            store_in_progress_pkg(cb_data, pkg, pkg->pkgId);
+            store_in_progress_pkg(cb_data, pkg);
         }
 
     }
@@ -334,7 +317,6 @@ int cr_xml_parse_main_metadata_together(const char *primary_path,
                                         void *pkgcb_data,
                                         cr_XmlParserWarningCb warningcb,
                                         void *warningcb_data,
-                                        gboolean allow_out_of_order,
                                         GError **err)
 {
     int ret = CRE_OK;
@@ -345,16 +327,11 @@ int cr_xml_parse_main_metadata_together(const char *primary_path,
 
     cr_CbData cbdata;
     cbdata.in_progress_pkgs_list = NULL;
-    cbdata.in_progress_pkgs_hash = NULL;
     cbdata.newpkgcb = newpkgcb;
     cbdata.newpkgcb_data = newpkgcb_data;
     cbdata.pkgcb = pkgcb;
     cbdata.pkgcb_data = pkgcb_data;
 
-    if (allow_out_of_order) {
-        cbdata.in_progress_pkgs_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
-    }
-
     assert(primary_path);
     assert(filelists_path);
     assert(other_path);
@@ -459,14 +436,10 @@ out:
     cr_xml_parser_data_free(filelists_pd);
     cr_xml_parser_data_free(other_pd);
 
-    if (allow_out_of_order) {
-        g_hash_table_destroy(cbdata.in_progress_pkgs_hash);
+    if (cbdata.newpkgcb) {
+        g_slist_free(cbdata.in_progress_pkgs_list);
     } else {
-        if (cbdata.newpkgcb) {
-            g_slist_free(cbdata.in_progress_pkgs_list);
-        } else {
-            cr_slist_free_full(cbdata.in_progress_pkgs_list, (GDestroyNotify) cr_package_free);
-        }
+        cr_slist_free_full(cbdata.in_progress_pkgs_list, (GDestroyNotify) cr_package_free);
     }
 
     return ret;
diff --git a/tests/python/tests/test_xml_parser.py b/tests/python/tests/test_xml_parser.py
index ed13cac..c835710 100644
--- a/tests/python/tests/test_xml_parser.py
+++ b/tests/python/tests/test_xml_parser.py
@@ -1049,8 +1049,7 @@ class TestCaseXmlParserMainMetadataTogether(unittest.TestCase):
         def warningcb(warn_type, msg):
             userdata["warnings"].append((warn_type, msg))
 
-        cr.xml_parse_main_metadata_together(REPO_02_PRIXML, REPO_02_FILXML, REPO_02_OTHXML, newpkgcb, pkgcb, warningcb,
-                                            allow_out_of_order=True)
+        cr.xml_parse_main_metadata_together(REPO_02_PRIXML, REPO_02_FILXML, REPO_02_OTHXML, newpkgcb, pkgcb, warningcb)
 
         self.assertEqual([pkg.name for pkg in userdata["pkgs"]],
             ['fake_bash', 'super_kernel'])
diff --git a/tests/test_xml_parser_main_metadata_together.c b/tests/test_xml_parser_main_metadata_together.c
index a4e4ad7..d8038ac 100644
--- a/tests/test_xml_parser_main_metadata_together.c
+++ b/tests/test_xml_parser_main_metadata_together.c
@@ -127,7 +127,7 @@ test_cr_xml_parse_main_metadata_together_00(void)
     int parsed = 0;
     GError *tmp_err = NULL;
     int ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY, TEST_REPO_02_FILELISTS, TEST_REPO_02_OTHER,
-                                                  NULL, NULL, pkgcb, &parsed, NULL, NULL, TRUE, &tmp_err);
+                                                  NULL, NULL, pkgcb, &parsed, NULL, NULL, &tmp_err);
     g_assert(tmp_err == NULL);
     g_assert_cmpint(ret, ==, CRE_OK);
     g_assert_cmpint(parsed, ==, 2);
@@ -141,16 +141,10 @@ test_cr_xml_parse_main_metadata_together_01_out_of_order_pkgs(void)
     int ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY,
                                                   TEST_DIFF_ORDER_FILELISTS,
                                                   TEST_REPO_02_OTHER,
-                                                  NULL, NULL, pkgcb, &parsed, NULL, NULL, FALSE, &tmp_err);
+                                                  NULL, NULL, pkgcb, &parsed, NULL, NULL, &tmp_err);
     g_assert(tmp_err != NULL);
     g_assert_cmpint(ret, ==, CRE_XMLPARSER);
-
     g_clear_error(&tmp_err);
-    ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY, TEST_DIFF_ORDER_FILELISTS, TEST_REPO_02_OTHER,
-                                              NULL, NULL, pkgcb, &parsed, NULL, NULL, TRUE, &tmp_err);
-    g_assert(tmp_err == NULL);
-    g_assert_cmpint(ret, ==, CRE_OK);
-    g_assert_cmpint(parsed, ==, 2);
 }
 
 static void
@@ -159,7 +153,7 @@ test_cr_xml_parse_main_metadata_together_02_invalid_path(void)
     int parsed = 0;
     GError *tmp_err = NULL;
     int ret = cr_xml_parse_main_metadata_together("/non/existent/file", TEST_REPO_02_FILELISTS, TEST_REPO_02_OTHER,
-                                                  NULL, NULL, pkgcb, &parsed, NULL, NULL, TRUE, &tmp_err);
+                                                  NULL, NULL, pkgcb, &parsed, NULL, NULL, &tmp_err);
     g_assert(tmp_err != NULL);
     g_assert_cmpint(ret, ==, CRE_NOFILE);
 }
@@ -170,24 +164,21 @@ test_cr_xml_parse_main_metadata_together_03_newpkgcb_returns_null(void)
     int parsed = 0;
     GError *tmp_err = NULL;
     int ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY, TEST_REPO_02_FILELISTS, TEST_REPO_02_OTHER,
-                                                  newpkgcb_skip_fake_bash, NULL, pkgcb, &parsed, NULL, NULL, TRUE,
-                                                  &tmp_err);
+                                                  newpkgcb_skip_fake_bash, NULL, pkgcb, &parsed, NULL, NULL, &tmp_err);
     g_assert(tmp_err == NULL);
     g_assert_cmpint(ret, ==, CRE_OK);
     g_assert_cmpint(parsed, ==, 1);
 
     parsed = 0;
     ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY, TEST_DIFF_ORDER_FILELISTS, TEST_REPO_02_OTHER,
-                                              newpkgcb_skip_fake_bash, NULL, pkgcb, &parsed, NULL, NULL, TRUE,
-                                              &tmp_err);
+                                              newpkgcb_skip_fake_bash, NULL, pkgcb, &parsed, NULL, NULL, &tmp_err);
     g_assert(tmp_err == NULL);
     g_assert_cmpint(ret, ==, CRE_OK);
     g_assert_cmpint(parsed, ==, 1);
 
     parsed = 0;
     ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY, TEST_DIFF_ORDER_FILELISTS, TEST_REPO_02_OTHER,
-                                              newpkgcb_skip_fake_bash, NULL, pkgcb, &parsed, NULL, NULL, FALSE,
-                                              &tmp_err);
+                                              newpkgcb_skip_fake_bash, NULL, pkgcb, &parsed, NULL, NULL, &tmp_err);
     g_assert(tmp_err == NULL);
     g_assert_cmpint(ret, ==, CRE_OK);
     g_assert_cmpint(parsed, ==, 1);
@@ -199,7 +190,7 @@ test_cr_xml_parse_main_metadata_together_04_newpkgcb_interrupt(void)
     int parsed = 0;
     GError *tmp_err = NULL;
     int ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY, TEST_REPO_02_FILELISTS, TEST_REPO_02_OTHER,
-                                                  newpkgcb_interrupt, &parsed, NULL, NULL, NULL, NULL, TRUE, &tmp_err);
+                                                  newpkgcb_interrupt, &parsed, NULL, NULL, NULL, NULL, &tmp_err);
     g_assert(tmp_err != NULL);
     g_error_free(tmp_err);
     g_assert_cmpint(ret, ==, CRE_CBINTERRUPTED);
@@ -212,7 +203,7 @@ test_cr_xml_parse_main_metadata_together_05_pkgcb_interrupt(void)
     int parsed = 0;
     GError *tmp_err = NULL;
     int ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY, TEST_REPO_02_FILELISTS, TEST_REPO_02_OTHER,
-                                                  NULL, NULL, pkgcb_interrupt, &parsed, NULL, NULL, TRUE, &tmp_err);
+                                                  NULL, NULL, pkgcb_interrupt, &parsed, NULL, NULL, &tmp_err);
     g_assert(tmp_err != NULL);
     g_error_free(tmp_err);
     g_assert_cmpint(ret, ==, CRE_CBINTERRUPTED);
@@ -227,7 +218,7 @@ test_cr_xml_parse_main_metadata_together_06_warnings_bad_file_type(void)
     GError *tmp_err = NULL;
     GString *warn_strings = g_string_new(0);
     int ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY, TEST_MRF_BAD_TYPE_FIL, TEST_REPO_02_OTHER,
-                                                  NULL, NULL, pkgcb, &parsed, warningcb, warn_strings, TRUE, &tmp_err);
+                                                  NULL, NULL, pkgcb, &parsed, warningcb, warn_strings, &tmp_err);
     g_assert(tmp_err == NULL);
     g_assert_cmpint(ret, ==, CRE_OK);
     g_assert_cmpint(parsed, ==, 2);
@@ -243,7 +234,7 @@ test_cr_xml_parse_main_metadata_together_07_warningcb_interrupt(void)
     GError *tmp_err = NULL;
     int ret = cr_xml_parse_main_metadata_together(TEST_REPO_02_PRIMARY, TEST_MRF_BAD_TYPE_FIL, TEST_REPO_02_OTHER,
                                                   NULL, NULL, pkgcb, NULL, warningcb_interrupt, &numofwarnings,
-                                                  TRUE, &tmp_err);
+                                                  &tmp_err);
     g_assert(tmp_err != NULL);
     g_error_free(tmp_err);
     g_assert_cmpint(ret, ==, CRE_CBINTERRUPTED);
@@ -256,7 +247,7 @@ test_cr_xml_parse_main_metadata_together_08_long_primary(void)
     int parsed = 0;
     GError *tmp_err = NULL;
     int ret = cr_xml_parse_main_metadata_together(TEST_LONG_PRIMARY, TEST_REPO_02_FILELISTS, TEST_REPO_02_OTHER,
-                                                  NULL, NULL, pkgcb, &parsed, NULL, NULL, TRUE, &tmp_err);
+                                                  NULL, NULL, pkgcb, &parsed, NULL, NULL, &tmp_err);
     g_assert(tmp_err == NULL);
     g_assert_cmpint(ret, ==, CRE_OK);
     g_assert_cmpint(parsed, ==, 2);