Add a 'relation' parameter to Device.AddProfile() to solve some hard to fix bugs

The concept of hard and soft profiles on a device allows to us auto-add profiles based on
EDID or from a service like CUPS, with a lower priority to user-selected profiles.

This ensures that CUPS adding a profile does not overwrite the users default profile,
nor does adding a ICC profile into your home directory change the default just because
the EDID hash matches.

Of course, if a profile is marked as default, then it becomes a hard profile, no matter
on where it came from.

This is obviously an API break, but it's certain to make the logic in colord much
cleaner and more understandable.

11 files changed, 148 insertions, 13 deletions

diff --git a/client/cd-util.c b/client/cd-util.c
index 65f955d..690e9f3 100644
--- a/client/cd-util.c
+++ b/client/cd-util.c
@@ -541,7 +541,11 @@ cd_util_device_add_profile (CdUtilPrivate *priv, gchar **values, GError **error)
 					       error);
 	if (!ret)
 		goto out;
-	ret = cd_device_add_profile_sync (device, profile, NULL, error);
+	ret = cd_device_add_profile_sync (device,
+					  CD_DEVICE_RELATION_HARD,
+					  profile,
+					  NULL,
+					  error);
 	if (!ret)
 		goto out;
 out:
diff --git a/libcolord/cd-device.c b/libcolord/cd-device.c
index 482f6a5..9d32f3f 100644
--- a/libcolord/cd-device.c
+++ b/libcolord/cd-device.c
@@ -792,6 +792,7 @@ cd_device_set_mode_sync (CdDevice *device,
 /**
  * cd_device_add_profile_sync:
  * @device: a #CdDevice instance.
+ * @relation: a #CdDeviceRelation, e.g. #CD_DEVICE_RELATION_HARD
  * @profile: a #CdProfile instance
  * @cancellable: a #GCancellable or %NULL
  * @error: a #GError, or %NULL.
@@ -800,10 +801,11 @@ cd_device_set_mode_sync (CdDevice *device,
  *
  * Return value: #TRUE for success, else #FALSE and @error is used
  *
- * Since: 0.1.0
+ * Since: 0.1.3
  **/
 gboolean
 cd_device_add_profile_sync (CdDevice *device,
+			    CdDeviceRelation relation,
 			    CdProfile *profile,
 			    GCancellable *cancellable,
 			    GError **error)
@@ -814,12 +816,14 @@ cd_device_add_profile_sync (CdDevice *device,
 
 	g_return_val_if_fail (CD_IS_DEVICE (device), FALSE);
 	g_return_val_if_fail (CD_IS_PROFILE (profile), FALSE);
+	g_return_val_if_fail (relation != CD_DEVICE_RELATION_UNKNOWN, FALSE);
 	g_return_val_if_fail (device->priv->proxy != NULL, FALSE);
 
 	/* execute sync method */
 	response = g_dbus_proxy_call_sync (device->priv->proxy,
 					   "AddProfile",
-					   g_variant_new ("(o)",
+					   g_variant_new ("(so)",
+							  cd_device_relation_to_string (relation),
 							  cd_profile_get_object_path (profile)),
 					   G_DBUS_CALL_FLAGS_NONE,
 					   -1, NULL, &error_local);
diff --git a/libcolord/cd-device.h b/libcolord/cd-device.h
index 8aac26a..0f746c8 100644
--- a/libcolord/cd-device.h
+++ b/libcolord/cd-device.h
@@ -112,6 +112,7 @@ gboolean	 cd_device_set_mode_sync		(CdDevice	*device,
 							 GCancellable	*cancellable,
 							 GError		**error);
 gboolean	 cd_device_add_profile_sync		(CdDevice	*device,
+							 CdDeviceRelation relation,
 							 CdProfile	*profile,
 							 GCancellable	*cancellable,
 							 GError		**error);
diff --git a/libcolord/cd-enum.c b/libcolord/cd-enum.c
index faed7a8..c1d1fdf 100644
--- a/libcolord/cd-enum.c
+++ b/libcolord/cd-enum.c
@@ -260,3 +260,29 @@ cd_device_mode_from_string (const gchar *device_mode)
 		return CD_DEVICE_MODE_VIRTUAL;
 	return CD_DEVICE_MODE_UNKNOWN;
 }
+
+/**
+ * cd_device_relation_to_string:
+ **/
+const gchar *
+cd_device_relation_to_string (CdDeviceRelation device_relation)
+{
+	if (device_relation == CD_DEVICE_RELATION_HARD)
+		return "hard";
+	if (device_relation == CD_DEVICE_RELATION_SOFT)
+		return "soft";
+	return "unknown";
+}
+
+/**
+ * cd_device_relation_from_string:
+ **/
+CdDeviceRelation
+cd_device_relation_from_string (const gchar *device_relation)
+{
+	if (g_strcmp0 (device_relation, "hard") == 0)
+		return CD_DEVICE_RELATION_HARD;
+	if (g_strcmp0 (device_relation, "soft") == 0)
+		return CD_DEVICE_RELATION_SOFT;
+	return CD_DEVICE_MODE_UNKNOWN;
+}
diff --git a/libcolord/cd-enum.h b/libcolord/cd-enum.h
index 8a7959f..289b653 100644
--- a/libcolord/cd-enum.h
+++ b/libcolord/cd-enum.h
@@ -118,6 +118,18 @@ typedef enum {
 	CD_DEVICE_MODE_LAST
 } CdDeviceMode;
 
+/**
+ * CdDeviceRelation:
+ *
+ * The device to profile relationship.
+ **/
+typedef enum {
+	CD_DEVICE_RELATION_UNKNOWN,
+	CD_DEVICE_RELATION_SOFT,
+	CD_DEVICE_RELATION_HARD,
+	CD_DEVICE_RELATION_LAST,
+} CdDeviceRelation;
+
 #define COLORD_DBUS_SERVICE		"org.freedesktop.ColorManager"
 #define COLORD_DBUS_PATH		"/org/freedesktop/ColorManager"
 #define COLORD_DBUS_INTERFACE		"org.freedesktop.ColorManager"
@@ -134,6 +146,8 @@ const gchar	*cd_colorspace_to_string		(CdColorspace		 colorspace);
 CdColorspace	 cd_colorspace_from_string		(const gchar		*colorspace);
 const gchar	*cd_device_mode_to_string		(CdDeviceMode		 device_mode);
 CdDeviceMode	 cd_device_mode_from_string		(const gchar		*device_mode);
+const gchar	*cd_device_relation_to_string		(CdDeviceRelation	 device_relation);
+CdDeviceRelation cd_device_relation_from_string		(const gchar		*device_relation);
 
 G_END_DECLS
 
diff --git a/libcolord/cd-self-test.c b/libcolord/cd-self-test.c
index 0769e43..40ae439 100644
--- a/libcolord/cd-self-test.c
+++ b/libcolord/cd-self-test.c
@@ -134,6 +134,8 @@ colord_client_func (void)
 	const gchar *qualifier2[] = {"Lexmark.RGB.*",
 				     "Epson.RGB.*",
 				     NULL};
+	const gchar *qualifier3[] = {"*.*.*",
+				     NULL};
 
 	key = g_random_int_range (0x00, 0xffff);
 	g_debug ("using random key %04x", key);
@@ -317,6 +319,12 @@ colord_client_func (void)
 	g_assert_no_error (error);
 	g_assert (ret);
 
+	/* set profile qualifier */
+	ret = cd_profile_set_qualifier_sync (profile2, "Epson.CMYK.300dpi",
+					     NULL, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
 	/* wait for daemon */
 	_g_test_loop_run_with_timeout (50);
 
@@ -349,12 +357,12 @@ colord_client_func (void)
 	g_clear_error (&error);
 
 	/* assign profile to device */
-	ret = cd_device_add_profile_sync (device, profile, NULL, &error);
+	ret = cd_device_add_profile_sync (device, CD_DEVICE_RELATION_SOFT, profile, NULL, &error);
 	g_assert_no_error (error);
 	g_assert (ret);
 
 	/* assign extra profile to device */
-	ret = cd_device_add_profile_sync (device, profile2, NULL, &error);
+	ret = cd_device_add_profile_sync (device, CD_DEVICE_RELATION_HARD, profile2, NULL, &error);
 	g_assert_no_error (error);
 	g_assert (ret);
 
@@ -423,6 +431,17 @@ colord_client_func (void)
 			 profile_path);
 	g_object_unref (profile_tmp);
 
+	/* check hard profiles beat soft profiles */
+	profile_tmp = cd_device_get_profile_for_qualifiers_sync (device,
+								 qualifier3,
+								 NULL,
+								 &error);
+	g_assert_no_error (error);
+	g_assert (profile_tmp != NULL);
+	g_assert_cmpstr (cd_profile_get_object_path (profile_tmp), ==,
+			 profile2_path);
+	g_object_unref (profile_tmp);
+
 	/* uninhibit device (should fail) */
 	ret = cd_device_profiling_uninhibit_sync (device,
 						  NULL,
diff --git a/src/cd-device.c b/src/cd-device.c
index 1039b23..53b96eb 100644
--- a/src/cd-device.c
+++ b/src/cd-device.c
@@ -63,6 +63,8 @@ struct _CdDevicePrivate
 	gchar				*object_path;
 	GDBusConnection			*connection;
 	GPtrArray			*profiles;
+	GPtrArray			*profiles_soft;
+	GPtrArray			*profiles_hard;
 	guint				 registration_id;
 	guint				 watcher_id;
 	guint64				 created;
@@ -476,7 +478,9 @@ cd_device_remove_profile (CdDevice *device,
 		goto out;
 	}
 
-	/* remove from the array */
+	/* remove from the arrays */
+	g_ptr_array_remove (priv->profiles_soft, profile_tmp);
+	g_ptr_array_remove (priv->profiles_hard, profile_tmp);
 	ret = g_ptr_array_remove (priv->profiles, profile_tmp);
 	g_assert (ret);
 
@@ -499,6 +503,7 @@ out:
  **/
 gboolean
 cd_device_add_profile (CdDevice *device,
+		       CdDeviceRelation relation,
 		       const gchar *profile_object_path,
 		       GError **error)
 {
@@ -538,6 +543,10 @@ cd_device_add_profile (CdDevice *device,
 
 	/* add to the array */
 	g_ptr_array_add (priv->profiles, g_object_ref (profile));
+	if (relation == CD_DEVICE_RELATION_SOFT)
+		g_ptr_array_add (priv->profiles_soft, g_object_ref (profile));
+	if (relation == CD_DEVICE_RELATION_HARD)
+		g_ptr_array_add (priv->profiles_hard, g_object_ref (profile));
 
 	/* emit */
 	cd_device_dbus_emit_property_changed (device,
@@ -662,6 +671,7 @@ cd_device_dbus_method_call (GDBusConnection *connection_, const gchar *sender,
 	GVariantIter *iter = NULL;
 	GVariant *tuple = NULL;
 	GVariant *value = NULL;
+	CdDeviceRelation relation = CD_DEVICE_RELATION_UNKNOWN;
 
 	/* return '' */
 	if (g_strcmp0 (method_name, "AddProfile") == 0) {
@@ -674,13 +684,31 @@ cd_device_dbus_method_call (GDBusConnection *connection_, const gchar *sender,
 			goto out;
 
 		/* check the profile_object_path exists */
-		g_variant_get (parameters, "(o)",
+		g_variant_get (parameters, "(so)",
+			       &property_value,
 			       &profile_object_path);
 		g_debug ("CdDevice %s:AddProfile(%s)",
 			 sender, profile_object_path);
 
+		/* convert the device->profile relationship into an enum */
+		if (g_strcmp0 (property_value, "soft") == 0)
+			relation = CD_DEVICE_RELATION_SOFT;
+		else if (g_strcmp0 (property_value, "hard") == 0)
+			relation = CD_DEVICE_RELATION_HARD;
+
+		/* nothing valid */
+		if (relation == CD_DEVICE_RELATION_UNKNOWN) {
+			g_dbus_method_invocation_return_error (invocation,
+							       CD_MAIN_ERROR,
+							       CD_MAIN_ERROR_FAILED,
+							       "relation '%s' unknown, expected 'hard' or 'soft'",
+							       property_value);
+			goto out;
+		}
+
 		/* add it */
 		ret = cd_device_add_profile (device,
+					     relation,
 					     profile_object_path,
 					     &error);
 		if (!ret) {
@@ -774,11 +802,13 @@ cd_device_dbus_method_call (GDBusConnection *connection_, const gchar *sender,
 		}
 
 		/* search each regex against the profiles for this device */
-		for (i=0; regexes[i] != NULL; i++) {
+		for (i=0; profile == NULL && regexes[i] != NULL; i++) {
 			profile = cd_device_find_by_qualifier (regexes[i],
-							       priv->profiles);
-			if (profile != NULL)
-				break;
+							       priv->profiles_hard);
+		}
+		for (i=0; profile == NULL && regexes[i] != NULL; i++) {
+			profile = cd_device_find_by_qualifier (regexes[i],
+							       priv->profiles_soft);
 		}
 		if (profile == NULL) {
 			g_dbus_method_invocation_return_error (invocation,
@@ -845,6 +875,11 @@ cd_device_dbus_method_call (GDBusConnection *connection_, const gchar *sender,
 			}
 		}
 
+		/* ensure profile is in the 'hard' relation array */
+		ret = g_ptr_array_remove (priv->profiles_soft, profile);
+		if (ret)
+			g_ptr_array_add (priv->profiles_hard, g_object_ref (profile));
+
 		/* emit */
 		cd_device_dbus_emit_property_changed (device,
 						      "Profiles",
@@ -1222,6 +1257,8 @@ cd_device_init (CdDevice *device)
 {
 	device->priv = CD_DEVICE_GET_PRIVATE (device);
 	device->priv->profiles = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
+	device->priv->profiles_soft = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
+	device->priv->profiles_hard = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
 	device->priv->profile_array = cd_profile_array_new ();
 #if !GLIB_CHECK_VERSION (2, 25, 0)
 	device->priv->created = g_get_real_time ();
@@ -1269,8 +1306,9 @@ cd_device_finalize (GObject *object)
 	g_free (priv->serial);
 	g_free (priv->kind);
 	g_free (priv->object_path);
-	if (priv->profiles != NULL)
-		g_ptr_array_unref (priv->profiles);
+	g_ptr_array_unref (priv->profiles);
+	g_ptr_array_unref (priv->profiles_soft);
+	g_ptr_array_unref (priv->profiles_hard);
 	g_object_unref (priv->profile_array);
 	g_object_unref (priv->mapping_db);
 	g_object_unref (priv->device_db);
diff --git a/src/cd-device.h b/src/cd-device.h
index 27fd0ac..4d5bbe0 100644
--- a/src/cd-device.h
+++ b/src/cd-device.h
@@ -70,6 +70,7 @@ GPtrArray	*cd_device_get_profiles			(CdDevice	*device);
 void		 cd_device_set_profiles			(CdDevice	*device,
 							 GPtrArray	*profiles);
 gboolean	 cd_device_add_profile			(CdDevice	*device,
+							 CdDeviceRelation relation,
 							 const gchar	*profile_object_path,
 							 GError		**error);
 gboolean	 cd_device_remove_profile		(CdDevice	*device,
diff --git a/src/cd-main.c b/src/cd-main.c
index 246a9a0..858fdbf 100644
--- a/src/cd-main.c
+++ b/src/cd-main.c
@@ -268,6 +268,7 @@ cd_main_device_auto_add_profiles (CdDevice *device)
 				 object_path_tmp,
 				 cd_device_get_object_path (device));
 			ret = cd_device_add_profile (device,
+						     CD_DEVICE_RELATION_HARD,
 						     object_path_tmp,
 						     &error);
 			if (!ret) {
@@ -494,6 +495,7 @@ cd_main_profile_auto_add_to_device (CdProfile *profile)
 				 cd_profile_get_object_path (profile),
 				 object_path_tmp);
 			ret = cd_device_add_profile (device_tmp,
+						     CD_DEVICE_RELATION_HARD,
 						     cd_profile_get_object_path (profile),
 						     &error);
 			if (!ret) {
diff --git a/src/cd-self-test.c b/src/cd-self-test.c
index d9b71e7..56fe3b3 100644
--- a/src/cd-self-test.c
+++ b/src/cd-self-test.c
@@ -71,6 +71,7 @@ colord_device_func (void)
 
 	/* add profile */
 	ret = cd_device_add_profile (device,
+				     CD_DEVICE_RELATION_SOFT,
 				     cd_profile_get_object_path (profile),
 				     &error);
 	g_assert_no_error (error);
@@ -78,6 +79,7 @@ colord_device_func (void)
 
 	/* add profile again */
 	ret = cd_device_add_profile (device,
+				     CD_DEVICE_RELATION_HARD,
 				     cd_profile_get_object_path (profile),
 				     &error);
 	g_assert_error (error, CD_MAIN_ERROR, CD_MAIN_ERROR_FAILED);
@@ -86,6 +88,7 @@ colord_device_func (void)
 
 	/* add profile that does not exist */
 	ret = cd_device_add_profile (device,
+				     CD_DEVICE_RELATION_SOFT,
 				     "/dave",
 				     &error);
 	g_assert_error (error, CD_MAIN_ERROR, CD_MAIN_ERROR_FAILED);
diff --git a/src/org.freedesktop.ColorManager.Device.xml b/src/org.freedesktop.ColorManager.Device.xml
index cd3ea5f..05a073c 100644
--- a/src/org.freedesktop.ColorManager.Device.xml
+++ b/src/org.freedesktop.ColorManager.Device.xml
@@ -192,6 +192,29 @@
           </doc:para>
         </doc:description>
       </doc:doc>
+      <arg type='s' name='relation' direction='in'>
+        <doc:doc>
+          <doc:summary>
+            <doc:para>
+              The strength of the relationship from profile to device.
+              This can be <doc:tt>soft</doc:tt> to indicate that the
+              mapping is not important, or that the profile is assumed
+              from a device and not in response to user action.
+            <doc:para>
+            </doc:para>
+              The default option is <doc:tt>hard</doc:tt>, and this
+              means that the user has explicitly mapped a profile to
+              a device, and this should take precidence over any
+              <doc:tt>soft</doc:tt> profiles.
+            </doc:para>
+            <doc:para>
+              If the user makes a <doc:tt>soft</doc:tt> profile default,
+              then it is explicitly promoted to a <doc:tt>hard</doc:tt>
+              relationship.
+            </doc:para>
+          </doc:summary>
+        </doc:doc>
+      </arg>
       <arg type='o' name='object_path' direction='in'>
         <doc:doc>
           <doc:summary>