lib/repo: Add timestamps to OstreeRepoFinderResult

Currently OstreeRepoFinderResult, a data structure used by pull code
that supports P2P operations, has a hash table mapping refs to checksums
but doesn't include timestamp information. This means that clients have
no way of knowing just from the OstreeRepoFinderResult information if a
commit being offered by a peer remote is an update or downgrade until
they start pulling it. The client could check the summary or the commit
metadata for the timestamps, but this requires adding the temporary
remotes to the repo config, and ostree is already checking timestamps
before returning the results, so I think it makes more sense for them to
be returned rather than leaving it to the client. This limitation is
especially important for offline computers, because for online computers
the latest commit available from any remote is the latest commit,
period.

This commit adds a "ref_to_timestamp" hash table to
OstreeRepoFinderResult that is symmetric to "ref_to_checksum" in that it
shares the same keys. This is an API break, but it's part of the
experimental API, and none of the current users of that (flatpak,
eos-updater, and gnome-software) are affected. See the documentation for
more details on "ref_to_timestamp". One thing to note is the data
structure currently gets initialized in find_remotes_cb(), so only users
of ostree_repo_find_remotes_async() will get them, not users of, say,
ostree_repo_finder_resolve_all_async(). This is because the individual
OstreeRepoFinder implementations don't currently access the timestamps
(but I think this could be changed in the future if there's a need).

This commit will allow P2P support to be added to
flatpak_installation_list_installed_refs_for_update, which will allow
GNOME Software to update apps from USB drives while offline (it's
already possible online).

Closes: #1518
Approved by: cgwalters

8 files changed, 189 insertions, 9 deletions

diff --git a/src/libostree/ostree-repo-finder-avahi.c b/src/libostree/ostree-repo-finder-avahi.c
index 6687a835..514351fc 100644
--- a/src/libostree/ostree-repo-finder-avahi.c
+++ b/src/libostree/ostree-repo-finder-avahi.c
@@ -844,7 +844,7 @@ ostree_avahi_service_build_repo_finder_result (OstreeAvahiService
         }
 
       g_ptr_array_add (results, ostree_repo_finder_result_new (remote, OSTREE_REPO_FINDER (finder),
-                                                               priority, supported_ref_to_checksum,
+                                                               priority, supported_ref_to_checksum, NULL,
                                                                GUINT64_FROM_BE (g_variant_get_uint64 (summary_timestamp))));
     }
 }
diff --git a/src/libostree/ostree-repo-finder-config.c b/src/libostree/ostree-repo-finder-config.c
index 76acb58e..5d1e1595 100644
--- a/src/libostree/ostree-repo-finder-config.c
+++ b/src/libostree/ostree-repo-finder-config.c
@@ -192,7 +192,7 @@ ostree_repo_finder_config_resolve_async (OstreeRepoFinder                  *find
           continue;
         }
 
-      g_ptr_array_add (results, ostree_repo_finder_result_new (remote, finder, priority, supported_ref_to_checksum, 0));
+      g_ptr_array_add (results, ostree_repo_finder_result_new (remote, finder, priority, supported_ref_to_checksum, NULL, 0));
     }
 
   g_ptr_array_sort (results, results_compare_cb);
diff --git a/src/libostree/ostree-repo-finder-mount.c b/src/libostree/ostree-repo-finder-mount.c
index 41a6bed2..7339fe52 100644
--- a/src/libostree/ostree-repo-finder-mount.c
+++ b/src/libostree/ostree-repo-finder-mount.c
@@ -545,7 +545,7 @@ G_GNUC_END_IGNORE_DEPRECATIONS
            * the code in ostree_repo_pull_from_remotes_async() will be able to
            * check it just as quickly as we can here; so don’t duplicate the
            * code. */
-          g_ptr_array_add (results, ostree_repo_finder_result_new (remote, finder, priority, supported_ref_to_checksum, 0));
+          g_ptr_array_add (results, ostree_repo_finder_result_new (remote, finder, priority, supported_ref_to_checksum, NULL, 0));
         }
     }
 
diff --git a/src/libostree/ostree-repo-finder-override.c b/src/libostree/ostree-repo-finder-override.c
index 5367708b..5bad9ace 100644
--- a/src/libostree/ostree-repo-finder-override.c
+++ b/src/libostree/ostree-repo-finder-override.c
@@ -243,7 +243,7 @@ ostree_repo_finder_override_resolve_async (OstreeRepoFinder                  *fi
   g_hash_table_iter_init (&iter, repo_remote_to_refs);
 
   while (g_hash_table_iter_next (&iter, (gpointer *) &remote, (gpointer *) &supported_ref_to_checksum))
-    g_ptr_array_add (results, ostree_repo_finder_result_new (remote, finder, priority, supported_ref_to_checksum, 0));
+    g_ptr_array_add (results, ostree_repo_finder_result_new (remote, finder, priority, supported_ref_to_checksum, NULL, 0));
 
   g_ptr_array_sort (results, results_compare_cb);
 
diff --git a/src/libostree/ostree-repo-finder.c b/src/libostree/ostree-repo-finder.c
index 4cad81f9..e7943c3e 100644
--- a/src/libostree/ostree-repo-finder.c
+++ b/src/libostree/ostree-repo-finder.c
@@ -436,6 +436,9 @@ G_DEFINE_BOXED_TYPE (OstreeRepoFinderResult, ostree_repo_finder_result,
  *    priority
  * @ref_to_checksum: (element-type OstreeCollectionRef utf8) (transfer none):
  *    map of collection–ref pairs to checksums provided by this result
+ * @ref_to_timestamp: (element-type OstreeCollectionRef guint64) (nullable)
+ *    (transfer none): map of collection–ref pairs to timestamps provided by this
+ *    result
  * @summary_last_modified: Unix timestamp (seconds since the epoch, UTC) when
  *    the summary file for the result was last modified, or `0` if this is unknown
  *
@@ -450,6 +453,7 @@ ostree_repo_finder_result_new (OstreeRemote     *remote,
                                OstreeRepoFinder *finder,
                                gint              priority,
                                GHashTable       *ref_to_checksum,
+                               GHashTable       *ref_to_timestamp,
                                guint64           summary_last_modified)
 {
   g_autoptr(OstreeRepoFinderResult) result = NULL;
@@ -463,6 +467,7 @@ ostree_repo_finder_result_new (OstreeRemote     *remote,
   result->finder = g_object_ref (finder);
   result->priority = priority;
   result->ref_to_checksum = g_hash_table_ref (ref_to_checksum);
+  result->ref_to_timestamp = ref_to_timestamp != NULL ? g_hash_table_ref (ref_to_timestamp) : NULL;
   result->summary_last_modified = summary_last_modified;
 
   return g_steal_pointer (&result);
@@ -484,7 +489,7 @@ ostree_repo_finder_result_dup (OstreeRepoFinderResult *result)
 
   return ostree_repo_finder_result_new (result->remote, result->finder,
                                         result->priority, result->ref_to_checksum,
-                                        result->summary_last_modified);
+                                        result->ref_to_timestamp, result->summary_last_modified);
 }
 
 /**
@@ -554,6 +559,7 @@ ostree_repo_finder_result_free (OstreeRepoFinderResult *result)
   /* This may be NULL iff the result is freed half-way through find_remotes_cb()
    * in ostree-repo-pull.c, and at no other time. */
   g_clear_pointer (&result->ref_to_checksum, g_hash_table_unref);
+  g_clear_pointer (&result->ref_to_timestamp, g_hash_table_unref);
   g_object_unref (result->finder);
   ostree_remote_unref (result->remote);
   g_free (result);
diff --git a/src/libostree/ostree-repo-finder.h b/src/libostree/ostree-repo-finder.h
index bb1a437e..e622c9a6 100644
--- a/src/libostree/ostree-repo-finder.h
+++ b/src/libostree/ostree-repo-finder.h
@@ -99,6 +99,8 @@ GPtrArray *ostree_repo_finder_resolve_all_finish (GAsyncResult  *result,
  * @ref_to_checksum: (element-type OstreeCollectionRef utf8): map of collection–ref
  *    pairs to checksums provided by this remote; values may be %NULL to
  *    indicate this remote doesn’t provide that ref
+ * @ref_to_timestamp: (element-type OstreeCollectionRef guint64) (nullable): map of
+ *    collection–ref pairs to timestamps; values may be 0 for various reasons
  * @summary_last_modified: Unix timestamp (seconds since the epoch, UTC) when
  *    the summary file on the remote was last modified, or `0` if unknown
  *
@@ -122,6 +124,15 @@ GPtrArray *ostree_repo_finder_resolve_all_finish (GAsyncResult  *result,
  * should be available locally, so the details for each checksum can be looked
  * up using ostree_repo_load_commit().
  *
+ * @ref_to_timestamp provides timestamps for the set of refs in
+ * @ref_to_checksum. The refs are keys (of type #OstreeCollectionRef) and the
+ * values are guint64 pointers with the timestamp associated with the checksum
+ * provided in @ref_to_checksum. @ref_to_timestamp can be %NULL, and when it's
+ * not, the timestamps are zero when any of the following conditions are met:
+ * (1) the override-commit-ids option was used on
+ * ostree_repo_find_remotes_async (2) there was an error in trying to get the
+ * commit metadata (3) the checksum for this ref is %NULL in @ref_to_checksum.
+ *
  * Since: 2017.8
  */
 typedef struct
@@ -131,9 +142,10 @@ typedef struct
   gint priority;
   GHashTable *ref_to_checksum;
   guint64 summary_last_modified;
+  GHashTable *ref_to_timestamp;
 
   /*< private >*/
-  gpointer padding[4];
+  gpointer padding[3];
 } OstreeRepoFinderResult;
 
 _OSTREE_PUBLIC
@@ -144,6 +156,7 @@ OstreeRepoFinderResult *ostree_repo_finder_result_new (OstreeRemote     *remote,
                                                        OstreeRepoFinder *finder,
                                                        gint              priority,
                                                        GHashTable       *ref_to_checksum,
+                                                       GHashTable       *ref_to_timestamp,
                                                        guint64           summary_last_modified);
 _OSTREE_PUBLIC
 OstreeRepoFinderResult *ostree_repo_finder_result_dup (OstreeRepoFinderResult *result);
diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index f4661638..be7cb228 100644
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -4862,6 +4862,7 @@ find_remotes_cb (GObject      *obj,
   g_autoptr(GHashTable) commit_metadatas = NULL;  /* (element-type commit-checksum CommitMetadata) */
   g_autoptr(OstreeFetcher) fetcher = NULL;
   g_autofree const gchar **ref_to_latest_commit = NULL;  /* indexed as @refs; (element-type commit-checksum) */
+  g_autofree guint64 *ref_to_latest_timestamp = NULL;  /* indexed as @refs; (element-type commit-timestamp) */
   gsize n_refs;
   g_autofree char **override_commit_ids = NULL;
   g_autoptr(GPtrArray) remotes_to_remove = NULL;  /* (element-type OstreeRemote) */
@@ -5017,6 +5018,7 @@ find_remotes_cb (GObject      *obj,
        * it’s been moved to @refs_and_remotes_table and is now potentially out
        * of date. */
       g_clear_pointer (&result->ref_to_checksum, g_hash_table_unref);
+      g_clear_pointer (&result->ref_to_timestamp, g_hash_table_unref);
       result->summary_last_modified = summary_last_modified;
     }
 
@@ -5153,8 +5155,12 @@ find_remotes_cb (GObject      *obj,
    *
    * @ref_to_latest_commit is indexed by @ref_index, and its values are the
    * latest checksum for each ref. If override-commit-ids was used,
-   * @ref_to_latest_commit won't be initialized or used.*/
+   * @ref_to_latest_commit won't be initialized or used.
+   *
+   * @ref_to_latest_timestamp is also indexed by @ref_index, and its values are
+   * the latest timestamp for each ref, when available.*/
   ref_to_latest_commit = g_new0 (const gchar *, n_refs);
+  ref_to_latest_timestamp = g_new0 (guint64, n_refs);
 
   for (i = 0; i < n_refs; i++)
     {
@@ -5195,6 +5201,11 @@ find_remotes_cb (GObject      *obj,
        * the summary or commit metadata files above. */
       ref_to_latest_commit[i] = latest_checksum;
 
+      if (latest_checksum != NULL && latest_commit_metadata != NULL)
+        ref_to_latest_timestamp[i] = latest_commit_metadata->timestamp;
+      else
+        ref_to_latest_timestamp[i] = 0;
+
       if (latest_commit_metadata != NULL)
         {
           latest_commit_timestamp_str = uint64_secs_to_iso8601 (latest_commit_metadata->timestamp);
@@ -5218,6 +5229,7 @@ find_remotes_cb (GObject      *obj,
     {
       OstreeRepoFinderResult *result = g_ptr_array_index (results, i);
       g_autoptr(GHashTable) validated_ref_to_checksum = NULL;  /* (element-type OstreeCollectionRef utf8) */
+      g_autoptr(GHashTable) validated_ref_to_timestamp = NULL;  /* (element-type OstreeCollectionRef guint64) */
       gsize j, n_latest_refs;
 
       /* Previous error processing this result? */
@@ -5231,11 +5243,24 @@ find_remotes_cb (GObject      *obj,
                                                          (GDestroyNotify) ostree_collection_ref_free,
                                                          g_free);
 
+      validated_ref_to_timestamp = g_hash_table_new_full (ostree_collection_ref_hash,
+                                                          ostree_collection_ref_equal,
+                                                          (GDestroyNotify) ostree_collection_ref_free,
+                                                          g_free);
       if (override_commit_ids)
         {
           for (j = 0; refs[j] != NULL; j++)
-            g_hash_table_insert (validated_ref_to_checksum, ostree_collection_ref_dup (refs[j]),
-                                 g_strdup (override_commit_ids[j]));
+            {
+              guint64 *timestamp_ptr;
+
+              g_hash_table_insert (validated_ref_to_checksum, ostree_collection_ref_dup (refs[j]),
+                                   g_strdup (override_commit_ids[j]));
+
+              timestamp_ptr = g_malloc (sizeof (guint64));
+              *timestamp_ptr = 0;
+              g_hash_table_insert (validated_ref_to_timestamp, ostree_collection_ref_dup (refs[j]),
+                                   timestamp_ptr);
+            }
         }
       else
         {
@@ -5244,6 +5269,7 @@ find_remotes_cb (GObject      *obj,
           for (j = 0; refs[j] != NULL; j++)
             {
               const gchar *latest_commit_for_ref = ref_to_latest_commit[j];
+              guint64 *timestamp_ptr;
 
               if (pointer_table_get (refs_and_remotes_table, j, i) != latest_commit_for_ref)
                 latest_commit_for_ref = NULL;
@@ -5252,6 +5278,14 @@ find_remotes_cb (GObject      *obj,
 
               g_hash_table_insert (validated_ref_to_checksum, ostree_collection_ref_dup (refs[j]),
                                    g_strdup (latest_commit_for_ref));
+
+              timestamp_ptr = g_malloc (sizeof (guint64));
+              if (latest_commit_for_ref != NULL)
+                *timestamp_ptr = GUINT64_TO_BE (ref_to_latest_timestamp[j]);
+              else
+                *timestamp_ptr = 0;
+              g_hash_table_insert (validated_ref_to_timestamp, ostree_collection_ref_dup (refs[j]),
+                                   timestamp_ptr);
             }
 
           if (n_latest_refs == 0)
@@ -5264,6 +5298,7 @@ find_remotes_cb (GObject      *obj,
         }
 
       result->ref_to_checksum = g_steal_pointer (&validated_ref_to_checksum);
+      result->ref_to_timestamp = g_steal_pointer (&validated_ref_to_timestamp);
       g_ptr_array_add (final_results, g_steal_pointer (&g_ptr_array_index (results, i)));
     }
 
diff --git a/tests/test-repo-finder-config.c b/tests/test-repo-finder-config.c
index 61d49b48..a87e3f4b 100644
--- a/tests/test-repo-finder-config.c
+++ b/tests/test-repo-finder-config.c
@@ -303,6 +303,130 @@ test_repo_finder_config_mixed_configs (Fixture       *fixture,
   g_main_context_pop_thread_default (context);
 }
 
+/* Test that using ostree_repo_find_remotes_async() works too.*/
+static void
+test_repo_finder_config_find_remotes (Fixture       *fixture,
+                                      gconstpointer  test_data)
+{
+  g_autoptr(OstreeRepoFinder) finder = NULL;
+  g_autoptr(GMainContext) context = NULL;
+  g_autoptr(GAsyncResult) result = NULL;
+  g_auto(OstreeRepoFinderResultv) results = NULL;
+  g_autoptr(GError) error = NULL;
+  gsize i;
+  const OstreeCollectionRef ref0 = { "org.example.Collection0", "exampleos/x86_64/ref0" };
+  const OstreeCollectionRef ref1 = { "org.example.Collection0", "exampleos/x86_64/ref1" };
+  const OstreeCollectionRef ref2 = { "org.example.Collection1", "exampleos/x86_64/ref1" };
+  const OstreeCollectionRef ref3 = { "org.example.Collection1", "exampleos/x86_64/ref2" };
+  const OstreeCollectionRef ref4 = { "org.example.Collection2", "exampleos/x86_64/ref3" };
+  const OstreeCollectionRef * const refs[] = { &ref0, &ref1, &ref2, &ref3, &ref4, NULL };
+  OstreeRepoFinder *finders[2] = {NULL, };
+
+  context = g_main_context_new ();
+  g_main_context_push_thread_default (context);
+
+  /* Put together various ref configuration files. */
+  g_autofree gchar *collection0_uri = assert_create_remote (fixture, "org.example.Collection0",
+                                                            "exampleos/x86_64/ref0",
+                                                            "exampleos/x86_64/ref1",
+                                                            NULL);
+  g_autofree gchar *collection1_uri = assert_create_remote (fixture, "org.example.Collection1",
+                                                            "exampleos/x86_64/ref2",
+                                                            NULL);
+  g_autofree gchar *no_collection_uri = assert_create_remote (fixture, NULL,
+                                                              "exampleos/x86_64/ref3",
+                                                              NULL);
+
+  assert_create_remote_config (fixture->parent_repo, "remote0", collection0_uri, "org.example.Collection0");
+  assert_create_remote_config (fixture->parent_repo, "remote1", collection1_uri, "org.example.Collection1");
+  assert_create_remote_config (fixture->parent_repo, "remote0-copy", collection0_uri, "org.example.Collection0");
+  assert_create_remote_config (fixture->parent_repo, "remote1-bad-copy", collection1_uri, "org.example.NotCollection1");
+  assert_create_remote_config (fixture->parent_repo, "remote2", no_collection_uri, NULL);
+
+  finders[0] = OSTREE_REPO_FINDER (ostree_repo_finder_config_new ());
+
+  /* Resolve the refs. */
+  ostree_repo_find_remotes_async (fixture->parent_repo, refs,
+                                  NULL, finders,
+                                  NULL, NULL, result_cb, &result);
+
+  while (result == NULL)
+    g_main_context_iteration (context, TRUE);
+
+  results = ostree_repo_find_remotes_finish (fixture->parent_repo,
+                                             result, &error);
+  g_assert_no_error (error);
+  g_assert_nonnull (results);
+  g_assert_cmpuint (g_strv_length ((char **) results), ==, 3);
+
+  /* Check that the results are correct: the invalid refs should have been
+   * ignored, and the valid results canonicalised and deduplicated. */
+  for (i = 0; results[i] != NULL; i++)
+    {
+      const char *ref0_checksum, *ref1_checksum, *ref2_checksum, *ref3_checksum;
+      guint64 *ref0_timestamp, *ref1_timestamp, *ref2_timestamp, *ref3_timestamp;
+
+      if (g_strcmp0 (ostree_remote_get_name (results[i]->remote), "remote0") == 0 ||
+          g_strcmp0 (ostree_remote_get_name (results[i]->remote), "remote0-copy") == 0)
+        {
+          g_assert_cmpuint (g_hash_table_size (results[i]->ref_to_checksum), ==, 5);
+
+          ref0_checksum = g_hash_table_lookup (results[i]->ref_to_checksum, &ref0);
+          g_assert_true (ostree_validate_checksum_string (ref0_checksum, NULL));
+
+          ref1_checksum = g_hash_table_lookup (results[i]->ref_to_checksum, &ref1);
+          g_assert_true (ostree_validate_checksum_string (ref1_checksum, NULL));
+
+          ref2_checksum = g_hash_table_lookup (results[i]->ref_to_checksum, &ref2);
+          g_assert (ref2_checksum == NULL);
+
+          g_assert_cmpuint (g_hash_table_size (results[i]->ref_to_timestamp), ==, 5);
+
+          ref0_timestamp = g_hash_table_lookup (results[i]->ref_to_timestamp, &ref0);
+          *ref0_timestamp = GUINT64_FROM_BE (*ref0_timestamp);
+          g_assert_cmpuint (*ref0_timestamp, >, 0);
+
+          ref1_timestamp = g_hash_table_lookup (results[i]->ref_to_timestamp, &ref1);
+          *ref1_timestamp = GUINT64_FROM_BE (*ref1_timestamp);
+          g_assert_cmpuint (*ref1_timestamp, >, 0);
+
+          ref2_timestamp = g_hash_table_lookup (results[i]->ref_to_timestamp, &ref2);
+          *ref2_timestamp = GUINT64_FROM_BE (*ref2_timestamp);
+          g_assert_cmpuint (*ref2_timestamp, ==, 0);
+
+          g_assert_cmpstr (ostree_remote_get_url (results[i]->remote), ==, collection0_uri);
+        }
+      else if (g_strcmp0 (ostree_remote_get_name (results[i]->remote), "remote1") == 0)
+        {
+          g_assert_cmpuint (g_hash_table_size (results[i]->ref_to_checksum), ==, 5);
+
+          ref3_checksum = g_hash_table_lookup (results[i]->ref_to_checksum, &ref3);
+          g_assert_true (ostree_validate_checksum_string (ref3_checksum, NULL));
+
+          ref0_checksum = g_hash_table_lookup (results[i]->ref_to_checksum, &ref0);
+          g_assert (ref0_checksum == NULL);
+
+          g_assert_cmpuint (g_hash_table_size (results[i]->ref_to_timestamp), ==, 5);
+
+          ref3_timestamp = g_hash_table_lookup (results[i]->ref_to_timestamp, &ref3);
+          *ref3_timestamp = GUINT64_FROM_BE (*ref3_timestamp);
+          g_assert_cmpuint (*ref3_timestamp, >, 0);
+
+          ref0_timestamp = g_hash_table_lookup (results[i]->ref_to_timestamp, &ref0);
+          *ref0_timestamp = GUINT64_FROM_BE (*ref0_timestamp);
+          g_assert_cmpuint (*ref0_timestamp, ==, 0);
+
+          g_assert_cmpstr (ostree_remote_get_url (results[i]->remote), ==, collection1_uri);
+        }
+      else
+        {
+          g_assert_not_reached ();
+        }
+    }
+
+  g_main_context_pop_thread_default (context);
+}
+
 int main (int argc, char **argv)
 {
   setlocale (LC_ALL, "");
@@ -313,6 +437,8 @@ int main (int argc, char **argv)
               test_repo_finder_config_no_configs, teardown);
   g_test_add ("/repo-finder-config/mixed-configs", Fixture, NULL, setup,
               test_repo_finder_config_mixed_configs, teardown);
+  g_test_add ("/repo-finder-config/find-remotes", Fixture, NULL, setup,
+              test_repo_finder_config_find_remotes, teardown);
 
   return g_test_run();
 }