Fix implementation of xa.noenumerate remote option

Currently the xa.noenumerate option on a remote is documented as causing
the remote not to be used when presenting available apps/runtimes or
when searching for dependencies. The idea is that the remote is only
used for providing updates for things installed from it, and this
functionality is used when creating an origin remote for something
installed via a flatpakref file.

However, the implementation of this in flatpak_dir_list_remote_refs() is
buggy. It returns an empty set of refs even if something is both locally
installed and available from the remote. This is because it is using
hash table comparisons of FlatpkDecomposed objects (via
flatpak_decomposed_hash()) which take into account both the ref (or
refspec) and the collection ID, and the local refs' FlatpakDecomposed
objects are created from a refspec whereas the remote refs'
FlatpakDecomposed objects are created from a ref alone. We could fix
this by having them both use refspecs, but it is better to use a
collection-ref tuple for the following reasons:
(1) Changing flatpak_dir_list_all_remote_refs() to use a refspec to
create the FlatpakDecomposed objects would be a breaking change for the
other users of that function.
(2) Both the local and remote refs are from the same remote so we don't
need to use the remote name to disambiguate them, even if no collection
ID is configured.
(3) The whole point of collection IDs is to make refs uniquely
identifiable, so we're using them for the intended purpose.

In addition to fixing this bug, this commit adds a unit test in
testlibrary.c so it stays fixed.

2 files changed, 66 insertions, 2 deletions

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index 30436e45..a10e5951 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -12283,7 +12283,7 @@ flatpak_dir_list_all_remote_refs (FlatpakDir         *self,
         {
           summary = var_summary_from_gvariant (subsummary);
           ref_map = var_summary_get_ref_map (summary);
-          populate_hash_table_from_refs_map (ret_all_refs, NULL, ref_map, NULL, state);
+          populate_hash_table_from_refs_map (ret_all_refs, NULL, ref_map, state->collection_id, state);
         }
     }
   else if (state->summary != NULL)
@@ -14364,7 +14364,13 @@ flatpak_dir_list_remote_refs (FlatpakDir         *self,
       while (g_hash_table_iter_next (&hash_iter, &key, NULL))
         {
           const char *refspec = key;
-          g_autoptr(FlatpakDecomposed) d = flatpak_decomposed_new_from_refspec (refspec, NULL);
+          g_autofree char *ref = NULL;
+          g_autoptr(FlatpakDecomposed) d = NULL;
+
+          if (!ostree_parse_refspec (refspec, NULL, &ref, error))
+            return FALSE;
+
+          d = flatpak_decomposed_new_from_col_ref (ref, state->collection_id, NULL);
           if (d)
             g_hash_table_insert (decomposed_local_refs, g_steal_pointer (&d), NULL);
         }
diff --git a/tests/testlibrary.c b/tests/testlibrary.c
index 3f7912fe..e331a7fa 100644
--- a/tests/testlibrary.c
+++ b/tests/testlibrary.c
@@ -1129,6 +1129,63 @@ test_list_remote_refs (void)
     }
 }
 
+/* Test the xa.noenumerate option on a remote, which should mask non-installed refs */
+static void
+test_list_remote_refs_noenumerate (void)
+{
+  g_autoptr(FlatpakInstallation) inst = NULL;
+  g_autoptr(GError) error = NULL;
+  g_autoptr(GPtrArray) refs = NULL;
+  g_autoptr(FlatpakRemote) remote = NULL;
+  g_autoptr(FlatpakInstalledRef) runtime_ref = NULL;
+  gboolean res;
+
+  inst = flatpak_installation_new_user (NULL, &error);
+  g_assert_no_error (error);
+
+  empty_installation (inst);
+
+  refs = flatpak_installation_list_remote_refs_sync (inst, repo_name, NULL, &error);
+  g_assert_no_error (error);
+  g_assert_nonnull (refs);
+  g_assert_cmpint (refs->len, ==, 4);
+
+  /* Install a runtime */
+  G_GNUC_BEGIN_IGNORE_DEPRECATIONS
+  runtime_ref = flatpak_installation_install (inst,
+                                              repo_name,
+                                              FLATPAK_REF_KIND_RUNTIME,
+                                              "org.test.Platform",
+                                              NULL, "master", NULL, NULL, NULL,
+                                              &error);
+  G_GNUC_END_IGNORE_DEPRECATIONS
+  g_assert_no_error (error);
+  g_assert_true (FLATPAK_IS_INSTALLED_REF (runtime_ref));
+
+  /* Set xa.noenumerate=true */
+  remote = flatpak_installation_get_remote_by_name (inst, repo_name, NULL, &error);
+  g_assert_no_error (error);
+  flatpak_remote_set_noenumerate (remote, TRUE);
+  res = flatpak_installation_modify_remote (inst, remote, NULL, &error);
+  g_assert_no_error (error);
+  g_assert_true (res);
+
+  /* Only the platform should be visible */
+  g_clear_pointer (&refs, g_ptr_array_unref);
+  refs = flatpak_installation_list_remote_refs_sync (inst, repo_name, NULL, &error);
+  g_assert_no_error (error);
+  g_assert_nonnull (refs);
+  g_assert_cmpint (refs->len, ==, 1);
+
+  empty_installation (inst);
+
+  /* Set xa.noenumerate=false */
+  flatpak_remote_set_noenumerate (remote, FALSE);
+  res = flatpak_installation_modify_remote (inst, remote, NULL, &error);
+  g_assert_no_error (error);
+  g_assert_true (res);
+}
+
 static void
 test_update_installed_ref_if_missing_runtime (void)
 {
@@ -4754,6 +4811,7 @@ main (int argc, char *argv[])
   g_test_add_func ("/library/remote-new", test_remote_new);
   g_test_add_func ("/library/remote-new-from-file", test_remote_new_from_file);
   g_test_add_func ("/library/list-remote-refs", test_list_remote_refs);
+  g_test_add_func ("/library/list-remote-refs-noenumerate", test_list_remote_refs_noenumerate);
   g_test_add_func ("/library/list-remote-related-refs", test_list_remote_related_refs);
   g_test_add_func ("/library/list-remote-related-refs-for-installed", test_list_remote_related_refs_for_installed);
   g_test_add_func ("/library/list-refs", test_list_refs);