Mercurial > pidgin3 / changeset

changeset

Propagate notify signals from the priority contact of a person

Mon, 14 Aug 2023 22:34:51 -0500

author
Gary Kramlich <grim@reaperworld.com>
date
Mon, 14 Aug 2023 22:34:51 -0500
changeset 42270
1bdd57a0c0d1
parent 42269
fa7ab9df882b
child 42271
1a7cacfd281f

Propagate notify signals from the priority contact of a person

I also ported the person unit tests to the new status api, which also required
updating purple_presence_compare. I didn't notice any breakage there, but I
didn't look too hard.

I also update the person unit tests to use a counter for the notify signal
being called instead of a boolean.

Testing Done:
Ran the unit tests which required a fair amount refactoring and connected a demo and xmpp account without issue.

Bugs closed: PIDGIN-17755

Reviewed at https://reviews.imfreedom.org/r/2557/

libpurple/purpleperson.c file | annotate | diff | comparison | revisions
libpurple/purplepresence.c file | annotate | diff | comparison | revisions
libpurple/tests/test_person.c file | annotate | diff | comparison | revisions 
--- a/libpurple/purpleperson.c	Tue Aug 08 04:47:47 2023 -0500
+++ b/libpurple/purpleperson.c	Mon Aug 14 22:34:51 2023 -0500
@@ -47,6 +47,11 @@
 };
 static GParamSpec *properties[N_PROPERTIES] = {NULL, };
 
+static void
+purple_person_priority_contact_info_notify_cb(GObject *obj,
+                                              G_GNUC_UNUSED GParamSpec *pspec,
+                                              gpointer data);
+
 /******************************************************************************
  * Helpers
  *****************************************************************************/
@@ -79,26 +84,12 @@
 }
 
 static void
-purple_person_sort_contacts(PurplePerson *person) {
-	PurpleContactInfo *original_priority = NULL;
+purple_person_sort_contacts(PurplePerson *person,
+                            PurpleContactInfo *original_priority)
+{
 	PurpleContactInfo *new_priority = NULL;
 	guint n_items = person->contacts->len;
 
-	if(n_items <= 1) {
-		GObject *obj = G_OBJECT(person);
-
-		g_object_freeze_notify(obj);
-		g_object_notify_by_pspec(obj, properties[PROP_NAME_FOR_DISPLAY]);
-		g_object_notify_by_pspec(obj, properties[PROP_PRIORITY_CONTACT_INFO]);
-		g_object_thaw_notify(obj);
-
-		g_list_model_items_changed(G_LIST_MODEL(person), 0, n_items, n_items);
-
-		return;
-	}
-
-	original_priority = purple_person_get_priority_contact_info(person);
-
 	g_ptr_array_sort(person->contacts, purple_person_contact_compare);
 
 	/* Tell the list we update our stuff. */
@@ -109,6 +100,21 @@
 	if(original_priority != new_priority) {
 		GObject *obj = G_OBJECT(person);
 
+		if(PURPLE_IS_CONTACT_INFO(original_priority)) {
+			g_signal_handlers_disconnect_by_func(original_priority,
+			                                     purple_person_priority_contact_info_notify_cb,
+			                                     person);
+		}
+
+		if(PURPLE_IS_CONTACT_INFO(new_priority)) {
+			g_signal_connect_object(new_priority, "notify::name-for-display",
+			                        G_CALLBACK(purple_person_priority_contact_info_notify_cb),
+			                        person, 0);
+			g_signal_connect_object(new_priority, "notify::avatar",
+			                        G_CALLBACK(purple_person_priority_contact_info_notify_cb),
+			                        person, 0);
+		}
+
 		g_object_freeze_notify(obj);
 		g_object_notify_by_pspec(obj, properties[PROP_NAME_FOR_DISPLAY]);
 		g_object_notify_by_pspec(obj, properties[PROP_PRIORITY_CONTACT_INFO]);
@@ -139,11 +145,34 @@
  * Callbacks
  *****************************************************************************/
 static void
+purple_person_priority_contact_info_notify_cb(G_GNUC_UNUSED GObject *obj,
+                                              GParamSpec *pspec,
+                                              gpointer data)
+{
+	PurplePerson *person = data;
+	const char *property = NULL;
+
+	property = g_param_spec_get_name(pspec);
+	if(purple_strequal(property, "name-for-display")) {
+		g_object_notify_by_pspec(G_OBJECT(person),
+		                         properties[PROP_NAME_FOR_DISPLAY]);
+	} else if(purple_strequal(property, "avatar")) {
+		g_object_notify_by_pspec(G_OBJECT(person),
+		                         properties[PROP_AVATAR_FOR_DISPLAY]);
+	}
+}
+
+static void
 purple_person_presence_notify_cb(G_GNUC_UNUSED GObject *obj,
                                  G_GNUC_UNUSED GParamSpec *pspec,
                                  gpointer data)
 {
-	purple_person_sort_contacts(data);
+	PurplePerson *person = data;
+	PurpleContactInfo *current_priority = NULL;
+
+	current_priority = purple_person_get_priority_contact_info(person);
+
+	purple_person_sort_contacts(person, current_priority);
 }
 
 /******************************************************************************
@@ -514,10 +543,13 @@
                                PurpleContactInfo *info)
 {
 	PurplePresence *presence = NULL;
+	PurpleContactInfo *current_priority = NULL;
 
 	g_return_if_fail(PURPLE_IS_PERSON(person));
 	g_return_if_fail(PURPLE_IS_CONTACT_INFO(info));
 
+	current_priority = purple_person_get_priority_contact_info(person);
+
 	g_ptr_array_add(person->contacts, g_object_ref(info));
 
 	presence = purple_contact_info_get_presence(info);
@@ -527,13 +559,14 @@
 
 	purple_contact_info_set_person(info, person);
 
-	purple_person_sort_contacts(person);
+	purple_person_sort_contacts(person, current_priority);
 }
 
 gboolean
 purple_person_remove_contact_info(PurplePerson *person,
                                   PurpleContactInfo *info)
 {
+	PurpleContactInfo *current_priority = NULL;
 	gboolean removed = FALSE;
 
 	g_return_val_if_fail(PURPLE_IS_PERSON(person), FALSE);
@@ -542,6 +575,8 @@
 	/* Ref the contact info to avoid a use-after free. */
 	g_object_ref(info);
 
+	current_priority = purple_person_get_priority_contact_info(person);
+
 	/* g_ptr_array_remove calls g_object_unref because we passed it in as a
 	 * GDestroyNotify.
 	 */
@@ -556,7 +591,7 @@
 
 		purple_contact_info_set_person(info, NULL);
 
-		purple_person_sort_contacts(person);
+		purple_person_sort_contacts(person, current_priority);
 	}
 
 	/* Remove our reference. */
--- a/libpurple/purplepresence.c	Tue Aug 08 04:47:47 2023 -0500
+++ b/libpurple/purplepresence.c	Mon Aug 14 22:34:51 2023 -0500
@@ -562,6 +562,8 @@
 
 gint
 purple_presence_compare(PurplePresence *presence1, PurplePresence *presence2) {
+	PurplePresencePrimitive primitive1 = PURPLE_PRESENCE_PRIMITIVE_OFFLINE;
+	PurplePresencePrimitive primitive2 = PURPLE_PRESENCE_PRIMITIVE_OFFLINE;
 	GDateTime *idle1 = NULL;
 	GDateTime *idle2 = NULL;
 	GDateTime *now = NULL;
@@ -576,12 +578,15 @@
 		return -1;
 	}
 
-	if(purple_presence_is_online(presence1) &&
-	   !purple_presence_is_online(presence2))
+	primitive1 = purple_presence_get_primitive(presence1);
+	primitive2 = purple_presence_get_primitive(presence2);
+
+	if(primitive1 != PURPLE_PRESENCE_PRIMITIVE_OFFLINE &&
+	   primitive2 == PURPLE_PRESENCE_PRIMITIVE_OFFLINE)
 	{
 		return -1;
-	} else if(purple_presence_is_online(presence2) &&
-	          !purple_presence_is_online(presence1))
+	} else if(primitive1 == PURPLE_PRESENCE_PRIMITIVE_OFFLINE &&
+	          primitive2 != PURPLE_PRESENCE_PRIMITIVE_OFFLINE)
 	{
 		return 1;
 	}
--- a/libpurple/tests/test_person.c	Tue Aug 08 04:47:47 2023 -0500
+++ b/libpurple/tests/test_person.c	Mon Aug 14 22:34:51 2023 -0500
@@ -42,9 +42,9 @@
                              G_GNUC_UNUSED GParamSpec *pspec,
                              gpointer data)
 {
-	gboolean *called = data;
+	guint *called = data;
 
-	*called = TRUE;
+	*called = *called + 1;
 }
 
 /******************************************************************************
@@ -120,15 +120,21 @@
 	PurpleContactInfo *info = NULL;
 	PurplePerson *person = NULL;
 	GdkPixbuf *avatar = NULL;
+	guint called = 0;
 
 	person = purple_person_new();
+	g_signal_connect(person, "notify::avatar",
+	                 G_CALLBACK(test_purple_person_notify_cb), &called);
+	g_signal_connect(person, "notify::avatar-for-display",
+	                 G_CALLBACK(test_purple_person_notify_cb), &called);
 	avatar = gdk_pixbuf_new(GDK_COLORSPACE_RGB, FALSE, 8, 1, 1);
 	purple_person_set_avatar(person, avatar);
+	g_assert_cmpuint(called, ==, 2);
 
 	info = purple_contact_info_new("id");
 	purple_person_add_contact_info(person, info);
 
-	/* Make sure the person's alias is overriding the contact info. */
+	/* Make sure the person's avatar is overriding the contact info. */
 	g_assert_true(purple_person_get_avatar_for_display(person) == avatar);
 
 	g_clear_object(&info);
@@ -141,16 +147,29 @@
 	PurpleContactInfo *info = NULL;
 	PurplePerson *person = NULL;
 	GdkPixbuf *avatar = NULL;
+	guint called = 0;
 
 	person = purple_person_new();
+	g_signal_connect(person, "notify::avatar-for-display",
+	                 G_CALLBACK(test_purple_person_notify_cb), &called);
 
 	info = purple_contact_info_new("id");
 	avatar = gdk_pixbuf_new(GDK_COLORSPACE_RGB, FALSE, 8, 1, 1);
 	purple_contact_info_set_avatar(info, avatar);
 	purple_person_add_contact_info(person, info);
+	g_assert_cmpuint(called, ==, 1);
 
 	/* Make sure the person's alias is overriding the contact info. */
 	g_assert_true(purple_person_get_avatar_for_display(person) == avatar);
+	g_clear_object(&avatar);
+
+	/* Now change the avatar on the contact info an verify that we not notified
+	 * of the property changing.
+	 */
+	called = 0;
+	avatar = gdk_pixbuf_new(GDK_COLORSPACE_RGB, FALSE, 8, 1, 1);
+	purple_contact_info_set_avatar(info, avatar);
+	g_assert_cmpuint(called, ==, 1);
 
 	g_clear_object(&info);
 	g_clear_object(&person);
@@ -180,16 +199,25 @@
 test_purple_person_name_for_display_contact(void) {
 	PurpleContactInfo *info = NULL;
 	PurplePerson *person = NULL;
+	guint called = 0;
 
 	person = purple_person_new();
+	g_signal_connect(person, "notify::name-for-display",
+	                 G_CALLBACK(test_purple_person_notify_cb), &called);
 
 	info = purple_contact_info_new("id");
 	purple_person_add_contact_info(person, info);
+	g_assert_cmpuint(called, ==, 1);
 
-	/* Make sure the contact info's name for display is called when the
-	 * person's alias is unset.
+	/* Make sure the name for display matches the id of the contact. */
+	g_assert_cmpstr(purple_person_get_name_for_display(person), ==, "id");
+
+	/* Now set a username on the contact and verify that the name for display
+	 * matches and that the notify signal was emitted for the property.
 	 */
-	g_assert_cmpstr(purple_person_get_name_for_display(person), ==, "id");
+	purple_contact_info_set_username(info, "clu");
+	g_assert_cmpstr(purple_person_get_name_for_display(person), ==, "clu");
+	g_assert_cmpuint(called, ==, 2);
 
 	g_clear_object(&info);
 	g_clear_object(&person);
@@ -306,9 +334,7 @@
 	PurpleContactInfo *priority = NULL;
 	PurplePerson *person = NULL;
 	PurplePresence *presence = NULL;
-	PurpleStatus *status = NULL;
-	PurpleStatusType *status_type = NULL;
-	gboolean called = FALSE;
+	guint called = 0;
 
 	person = purple_person_new();
 	g_signal_connect(person, "notify::priority-contact-info",
@@ -320,20 +346,16 @@
 	info = purple_contact_info_new(NULL);
 	purple_person_add_contact_info(person, info);
 
-	/* Set the status of the contact. */
+	/* Set the presence of the contact. */
 	presence = purple_contact_info_get_presence(info);
-	status_type = purple_status_type_new(PURPLE_STATUS_AVAILABLE, "available",
-	                                     "Available", FALSE);
-	status = purple_status_new(status_type, presence);
-	g_object_set(G_OBJECT(presence), "active-status", status, NULL);
-	g_clear_object(&status);
+	purple_presence_set_primitive(presence,
+	                              PURPLE_PRESENCE_PRIMITIVE_AVAILABLE);
 
-	g_assert_true(called);
+	g_assert_cmpuint(called, ==, 1);
 
 	priority = purple_person_get_priority_contact_info(person);
 	g_assert_true(priority == info);
 
-	purple_status_type_destroy(status_type);
 	g_clear_object(&person);
 	g_clear_object(&info);
 	g_clear_object(&presence);
@@ -346,10 +368,7 @@
 	PurpleContactInfo *sorted_contact = NULL;
 	PurplePerson *person = NULL;
 	PurplePresence *sorted_presence = NULL;
-	PurpleStatus *status = NULL;
-	PurpleStatusType *available = NULL;
-	PurpleStatusType *offline = NULL;
-	gboolean changed = FALSE;
+	guint called = 0;
 	gint n_infos = 5;
 	guint n_items = 0;
 
@@ -361,29 +380,24 @@
 	 * we then assert.
 	 */
 
-	/* Create our status types. */
-	available = purple_status_type_new(PURPLE_STATUS_AVAILABLE, "available",
-	                                   "Available", FALSE);
-	offline = purple_status_type_new(PURPLE_STATUS_OFFLINE, "offline",
-	                                 "Offline", FALSE);
-
 	/* Create the person and connected to the notify signal for the
 	 * priority-contact property.
 	 */
 	person = purple_person_new();
 	g_signal_connect(person, "notify::priority-contact-info",
-	                 G_CALLBACK(test_purple_person_notify_cb), &changed);
+	                 G_CALLBACK(test_purple_person_notify_cb), &called);
 	priority = purple_person_get_priority_contact_info(person);
 	g_assert_null(priority);
 
 	/* Create and add all contact infos. */
 	for(gint i = 0; i < n_infos; i++) {
 		PurpleContactInfo *info = NULL;
-		PurplePresence *presence = NULL;
 		gchar *username = NULL;
 
-		/* Set changed to false as it shouldn't be changed. */
-		changed = FALSE;
+		/* Set called to 0 as it shouldn't be called as the priority contact
+		 * info shouldn't change except for the first index.
+		 */
+		called = 0;
 
 		/* Now create a real contact. */
 		username = g_strdup_printf("username%d", i + 1);
@@ -391,23 +405,24 @@
 		purple_contact_info_set_username(info, username);
 		g_free(username);
 
-		/* Set the status for the contact. */
-		presence = purple_contact_info_get_presence(info);
-		status = purple_status_new(offline, presence);
-		g_object_set(G_OBJECT(presence), "active-status", status, NULL);
-		g_clear_object(&status);
-
 		purple_person_add_contact_info(person, info);
 
 		if(i == 0) {
 			first = g_object_ref(info);
-			g_assert_true(changed);
+			g_assert_cmpuint(called, ==, 1);
 		} else {
-			g_assert_false(changed);
+			g_assert_cmpuint(called, ==, 0);
 
 			if(i == n_infos - 2) {
+				PurplePresence *presence = NULL;
+
+				/* Add a reference to the presence of this specific contact
+				 * info, as we want to tweak it later.
+				 */
+				presence = purple_contact_info_get_presence(info);
+				sorted_presence = g_object_ref(presence);
+
 				sorted_contact = g_object_ref(info);
-				sorted_presence = g_object_ref(presence);
 			}
 		}
 
@@ -424,18 +439,14 @@
 	/* Now set the second from the last contact info's status to available, and
 	 * verify that that contact info is now the priority contact info.
 	 */
-	changed = FALSE;
-	status = purple_status_new(available, sorted_presence);
-	g_object_set(G_OBJECT(sorted_presence), "active-status", status, NULL);
-	g_clear_object(&status);
-	g_assert_true(changed);
+	called = 0;
+	purple_presence_set_primitive(sorted_presence,
+	                              PURPLE_PRESENCE_PRIMITIVE_AVAILABLE);
 	priority = purple_person_get_priority_contact_info(person);
 	g_assert_true(priority == sorted_contact);
+	g_assert_cmpuint(called, ==, 1);
 
 	/* Cleanup. */
-	purple_status_type_destroy(offline);
-	purple_status_type_destroy(available);
-
 	g_clear_object(&sorted_contact);
 	g_clear_object(&sorted_presence);

Mercurial Repository: pidgin3

mercurial