Add DeploySideloadCollectionID flatpakref/flatpakrepo key

When Flatpak's P2P updates support was replaced with the "sideloading"
implementation in 1.7.1, a new server side repo config key
"deploy-sideload-collection-id" was added which gets set when you pass
"--deploy-sideload-collection-id" to "flatpak build-update-repo", and
has the effect of setting "xa.deploy-collection-id" in the repo metadata
that is pulled by clients, which itself causes a collection id to be set
on the remote for clients using Flatpak >= 1.7.1.

This commit adds an analogous key in flatpakref and flatpakrepo files,
so the collection id can be set when the remote is configured, rather
than later on when the repo metadata is pulled and acted upon. As before
with DeployCollectionID, it has no difference in function compared to
DeployCollectionID or CollectionID and the only difference is which
Flatpak versions are affected.

It would've been better if this were added in 1.7.1 when the sideload
support was added, but alas here we are.

(Also update the docs and unit tests)

7 files changed, 85 insertions, 26 deletions

diff --git a/common/flatpak-dir-private.h b/common/flatpak-dir-private.h
index ca247274..950944b3 100644
--- a/common/flatpak-dir-private.h
+++ b/common/flatpak-dir-private.h
@@ -70,6 +70,7 @@ GType flatpak_deploy_get_type (void);
 #define FLATPAK_REF_BRANCH_KEY "Branch"
 #define FLATPAK_REF_COLLECTION_ID_KEY "CollectionID"
 #define FLATPAK_REF_DEPLOY_COLLECTION_ID_KEY "DeployCollectionID"
+#define FLATPAK_REF_DEPLOY_SIDELOAD_COLLECTION_ID_KEY "DeploySideloadCollectionID"
 
 #define FLATPAK_REPO_GROUP "Flatpak Repo"
 #define FLATPAK_REPO_VERSION_KEY "Version"
@@ -89,6 +90,7 @@ GType flatpak_deploy_get_type (void);
 
 #define FLATPAK_REPO_COLLECTION_ID_KEY "CollectionID"
 #define FLATPAK_REPO_DEPLOY_COLLECTION_ID_KEY "DeployCollectionID"
+#define FLATPAK_REPO_DEPLOY_SIDELOAD_COLLECTION_ID_KEY "DeploySideloadCollectionID"
 
 #define FLATPAK_CLI_UPDATE_INTERVAL_MS 300
 
diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index 12c749fc..6753888e 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -14395,18 +14395,27 @@ parse_ref_file (GKeyFile *keyfile,
       gpg_data = g_bytes_new_take (g_steal_pointer (&decoded), decoded_len);
     }
 
-  collection_id = g_key_file_get_string (keyfile, FLATPAK_REF_GROUP,
-                                         FLATPAK_REF_DEPLOY_COLLECTION_ID_KEY, NULL);
+  /* We have a hierarchy of keys for setting the collection ID, which all have
+   * the same effect. The only difference is which versions of Flatpak support
+   * them, and therefore what P2P implementation is enabled by them:
+   * DeploySideloadCollectionID: supported by Flatpak >= 1.12.8 (1.7.1
+   *   introduced sideload support but this key was added late)
+   * DeployCollectionID: supported by Flatpak >= 1.0.6
+   * CollectionID: supported by Flatpak >= 0.9.8
+   */
+  collection_id = flatpak_keyfile_get_string_non_empty (keyfile, FLATPAK_REF_GROUP,
+                                                        FLATPAK_REF_DEPLOY_SIDELOAD_COLLECTION_ID_KEY);
 
-  if (collection_id != NULL && *collection_id == '\0')
-    g_clear_pointer (&collection_id, g_free);
   if (collection_id == NULL)
     {
-      collection_id = g_key_file_get_string (keyfile, FLATPAK_REF_GROUP,
-                                             FLATPAK_REF_COLLECTION_ID_KEY, NULL);
+      collection_id = flatpak_keyfile_get_string_non_empty (keyfile, FLATPAK_REF_GROUP,
+                                                            FLATPAK_REF_DEPLOY_COLLECTION_ID_KEY);
+    }
+  if (collection_id == NULL)
+    {
+      collection_id = flatpak_keyfile_get_string_non_empty (keyfile, FLATPAK_REF_GROUP,
+                                                            FLATPAK_REF_COLLECTION_ID_KEY);
     }
-  if (collection_id != NULL && *collection_id == '\0')
-    g_clear_pointer (&collection_id, g_free);
 
   if (collection_id != NULL && gpg_data == NULL)
     return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA, _("Collection ID requires GPG key to be provided"));
diff --git a/common/flatpak-utils-private.h b/common/flatpak-utils-private.h
index 8333ddfa..7829d998 100644
--- a/common/flatpak-utils-private.h
+++ b/common/flatpak-utils-private.h
@@ -414,6 +414,10 @@ gboolean flatpak_switch_symlink_and_remove (const char *symlink_path,
                                             const char *target,
                                             GError    **error);
 
+char *flatpak_keyfile_get_string_non_empty (GKeyFile *keyfile,
+                                            const char *group,
+                                            const char *key);
+
 GKeyFile * flatpak_parse_repofile (const char   *remote_name,
                                    gboolean      from_ref,
                                    GKeyFile     *keyfile,
diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c
index 81a5fef1..ffc988ec 100644
--- a/common/flatpak-utils.c
+++ b/common/flatpak-utils.c
@@ -2264,6 +2264,20 @@ flatpak_summary_lookup_ref (GVariant      *summary_v,
   return TRUE;
 }
 
+char *
+flatpak_keyfile_get_string_non_empty (GKeyFile   *keyfile,
+                                      const char *group,
+                                      const char *key)
+{
+  g_autofree char *value = NULL;
+
+  value = g_key_file_get_string (keyfile, group, key, NULL);
+  if (value != NULL && *value == '\0')
+    g_clear_pointer (&value, g_free);
+
+  return g_steal_pointer (&value);
+}
+
 GKeyFile *
 flatpak_parse_repofile (const char   *remote_name,
                         gboolean      from_ref,
@@ -2370,15 +2384,23 @@ flatpak_parse_repofile (const char   *remote_name,
       g_key_file_set_boolean (config, group, "gpg-verify", FALSE);
     }
 
-  collection_id = g_key_file_get_string (keyfile, source_group,
-                                         FLATPAK_REPO_DEPLOY_COLLECTION_ID_KEY, NULL);
-  if (collection_id != NULL && *collection_id == '\0')
-    g_clear_pointer (&collection_id, g_free);
+  /* We have a hierarchy of keys for setting the collection ID, which all have
+   * the same effect. The only difference is which versions of Flatpak support
+   * them, and therefore what P2P implementation is enabled by them:
+   * DeploySideloadCollectionID: supported by Flatpak >= 1.12.8 (1.7.1
+   *   introduced sideload support but this key was added late)
+   * DeployCollectionID: supported by Flatpak >= 1.0.6 (but fully supported in
+   *   >= 1.2.0)
+   * CollectionID: supported by Flatpak >= 0.9.8
+   */
+  collection_id = flatpak_keyfile_get_string_non_empty (keyfile, source_group,
+                                                        FLATPAK_REPO_DEPLOY_SIDELOAD_COLLECTION_ID_KEY);
+  if (collection_id == NULL)
+    collection_id = flatpak_keyfile_get_string_non_empty (keyfile, source_group,
+                                                          FLATPAK_REPO_DEPLOY_COLLECTION_ID_KEY);
   if (collection_id == NULL)
-    collection_id = g_key_file_get_string (keyfile, source_group,
-                                           FLATPAK_REPO_COLLECTION_ID_KEY, NULL);
-  if (collection_id != NULL && *collection_id == '\0')
-    g_clear_pointer (&collection_id, g_free);
+    collection_id = flatpak_keyfile_get_string_non_empty (keyfile, source_group,
+                                                          FLATPAK_REPO_COLLECTION_ID_KEY);
   if (collection_id != NULL)
     {
       if (gpg_key == NULL)
diff --git a/doc/flatpak-flatpakref.xml b/doc/flatpak-flatpakref.xml
index 7e2991cf..88359701 100644
--- a/doc/flatpak-flatpakref.xml
+++ b/doc/flatpak-flatpakref.xml
@@ -108,18 +108,23 @@
                     <listitem><para>The url of a webpage describing the application or runtime.</para></listitem>
                 </varlistentry>
                 <varlistentry>
-                    <term><option>DeployCollectionID</option> (string)</term>
+                    <term><option>DeploySideloadCollectionID</option> (string)</term>
                     <listitem><para>
                         The collection ID of the remote, if it has one. This uniquely
                         identifies the collection of apps in the remote, to allow peer to peer
-                        redistribution. It is recommended to use this key over CollectionID because
-                        only newer clients pay attention to it (and older clients don't handle
+                        redistribution (see <citerefentry><refentrytitle>flatpak</refentrytitle><manvolnum>1</manvolnum></citerefentry>).
+                        It is recommended to use this key over DeployCollectionID or CollectionID because
+                        only newer clients (Flatpak 1.12.8 or later) pay attention to it (and older clients don't handle
                         collection IDs properly).
                     </para></listitem>
                 </varlistentry>
                 <varlistentry>
+                    <term><option>DeployCollectionID</option> (string)</term>
+                    <listitem><para>This is deprecated but still supported for backwards compatibility. Use DeploySideloadCollectionID instead.</para></listitem>
+                </varlistentry>
+                <varlistentry>
                     <term><option>CollectionID</option> (string)</term>
-                    <listitem><para>This is deprecated but still supported for backwards compatibility. Use DeployCollectionID instead.</para></listitem>
+                    <listitem><para>This is deprecated but still supported for backwards compatibility. Use DeploySideloadCollectionID instead.</para></listitem>
                 </varlistentry>
 
                 <varlistentry>
diff --git a/doc/flatpak-flatpakrepo.xml b/doc/flatpak-flatpakrepo.xml
index a92e1183..0e8a4d0d 100644
--- a/doc/flatpak-flatpakrepo.xml
+++ b/doc/flatpak-flatpakrepo.xml
@@ -122,18 +122,23 @@
                     </para></listitem>
                 </varlistentry>
                 <varlistentry>
-                    <term><option>DeployCollectionID</option> (string)</term>
+                    <term><option>DeploySideloadCollectionID</option> (string)</term>
                     <listitem><para>
                         The collection ID of the remote, if it has one. This uniquely
                         identifies the collection of apps in the remote, to allow peer to peer
-                        redistribution. It is recommended to use this key over CollectionID because
-                        only newer clients pay attention to it (and older clients don't handle
+                        redistribution (see <citerefentry><refentrytitle>flatpak</refentrytitle><manvolnum>1</manvolnum></citerefentry>).
+                        It is recommended to use this key over DeployCollectionID or CollectionID because
+                        only newer clients (Flatpak 1.12.8 or later) pay attention to it (and older clients don't handle
                         collection IDs properly).
                     </para></listitem>
                 </varlistentry>
                 <varlistentry>
+                    <term><option>DeployCollectionID</option> (string)</term>
+                    <listitem><para>This is deprecated but still supported for backwards compatibility. Use DeploySideloadCollectionID instead.</para></listitem>
+                </varlistentry>
+                <varlistentry>
                     <term><option>CollectionID</option> (string)</term>
-                    <listitem><para>This is deprecated but still supported for backwards compatibility. Use DeployCollectionID instead.</para></listitem>
+                    <listitem><para>This is deprecated but still supported for backwards compatibility. Use DeploySideloadCollectionID instead.</para></listitem>
                 </varlistentry>
             </variablelist>
         </refsect2>
diff --git a/tests/test-repo.sh b/tests/test-repo.sh
index 404e160f..55403259 100644
--- a/tests/test-repo.sh
+++ b/tests/test-repo.sh
@@ -24,7 +24,7 @@ set -euo pipefail
 skip_without_bwrap
 skip_revokefs_without_fuse
 
-echo "1..44"
+echo "1..45"
 
 #Regular repo
 setup_repo
@@ -208,7 +208,7 @@ GPGKey=${FL_GPG_BASE64}
 EOF
 
 if [ x${USE_COLLECTIONS_IN_CLIENT-} == xyes ]; then
-    echo "DeployCollectionID=org.test.Collection.Flatpakref" >> repos/flatpakref/flatpakref-repo.flatpakrepo
+    echo "DeploySideloadCollectionID=org.test.Collection.Flatpakref" >> repos/flatpakref/flatpakref-repo.flatpakrepo
 fi
 
 cat << EOF > org.test.Hello.flatpakref
@@ -221,6 +221,10 @@ GPGKey=${FL_GPG_BASE64}
 RuntimeRepo=http://127.0.0.1:$(cat httpd-port)/flatpakref/flatpakref-repo.flatpakrepo
 EOF
 
+if [ x${USE_COLLECTIONS_IN_CLIENT-} == xyes ]; then
+    echo "DeploySideloadCollectionID=org.test.Collection.Flatpakref" >> org.test.Hello.flatpakref
+fi
+
 ${FLATPAK} ${U} uninstall -y org.test.Platform org.test.Hello >&2
 
 # Ensure that only one remote is added even though the URL in the flatpakref
@@ -239,6 +243,14 @@ assert_remote_has_config allthegoodstuff xa.title "The Remote Title"
 
 ok "install flatpakref uses RuntimeRepo metadata for remote"
 
+if [ x${USE_COLLECTIONS_IN_CLIENT-} == xyes ]; then
+    assert_remote_has_config allthegoodstuff collection-id "org.test.Collection.Flatpakref"
+else
+    assert_remote_has_no_config allthegoodstuff collection-id
+fi
+
+ok "install flatpakref sets collection-id on remote if available"
+
 ${FLATPAK} ${U} uninstall -y org.test.Platform org.test.Hello >&2
 
 if ${FLATPAK} ${U} install -y test-missing-gpg-repo org.test.Platform &> install-error-log; then