Refactored roster add/update

8 files changed, 101 insertions, 103 deletions

diff --git a/src/profanity.c b/src/profanity.c
index 9178d056..dae91aea 100644
--- a/src/profanity.c
+++ b/src/profanity.c
@@ -118,13 +118,6 @@ prof_run(const int disable_tls, char *log_level, char *account_name)
 }
 
 void
-prof_handle_roster_add(const char * const barejid, const char * const name)
-{
-    ui_roster_add(barejid, name);
-    ui_current_page_off();
-}
-
-void
 prof_handle_idle(void)
 {
     jabber_conn_status_t status = jabber_get_connection_status();
diff --git a/src/profanity.h b/src/profanity.h
index 5e826fc4..c55e6c47 100644
--- a/src/profanity.h
+++ b/src/profanity.h
@@ -30,6 +30,5 @@ void prof_run(const int disable_tls, char *log_level, char *account_name);
 
 void prof_handle_idle(void);
 void prof_handle_activity(void);
-void prof_handle_roster_add(const char * const barejid, const char * const name);
 
 #endif
diff --git a/src/roster_list.c b/src/roster_list.c
index 7945101a..1341320f 100644
--- a/src/roster_list.c
+++ b/src/roster_list.c
@@ -30,7 +30,6 @@
 #include "contact.h"
 #include "jid.h"
 #include "tools/autocomplete.h"
-#include "profanity.h"
 
 // nicknames
 static Autocomplete name_ac;
@@ -201,60 +200,51 @@ roster_update(const char * const barejid, const char * const name,
     GSList *groups, const char * const subscription, gboolean pending_out)
 {
     PContact contact = g_hash_table_lookup(contacts, barejid);
+    assert(contact != NULL);
 
-    if (contact == NULL) {
-        roster_add(barejid, name, groups, subscription, pending_out, FALSE);
-    } else {
-        p_contact_set_subscription(contact, subscription);
-        p_contact_set_pending_out(contact, pending_out);
+    p_contact_set_subscription(contact, subscription);
+    p_contact_set_pending_out(contact, pending_out);
 
-        const char * const new_name = name;
-        const char * current_name = NULL;
-        if (p_contact_name(contact) != NULL) {
-            current_name = strdup(p_contact_name(contact));
-        }
+    const char * const new_name = name;
+    const char * current_name = NULL;
+    if (p_contact_name(contact) != NULL) {
+        current_name = strdup(p_contact_name(contact));
+    }
 
-        p_contact_set_name(contact, new_name);
-        p_contact_set_groups(contact, groups);
-        _replace_name(current_name, new_name, barejid);
+    p_contact_set_name(contact, new_name);
+    p_contact_set_groups(contact, groups);
+    _replace_name(current_name, new_name, barejid);
 
-        // add groups
-        while (groups != NULL) {
-            autocomplete_add(groups_ac, groups->data);
-            groups = g_slist_next(groups);
-        }
+    // add groups
+    while (groups != NULL) {
+        autocomplete_add(groups_ac, groups->data);
+        groups = g_slist_next(groups);
     }
 }
 
 gboolean
 roster_add(const char * const barejid, const char * const name, GSList *groups,
-    const char * const subscription, gboolean pending_out, gboolean from_initial)
+    const char * const subscription, gboolean pending_out)
 {
-    gboolean added = FALSE;
     PContact contact = g_hash_table_lookup(contacts, barejid);
+    if (contact != NULL) {
+        return FALSE;
+    }
 
-    if (contact == NULL) {
-        contact = p_contact_new(barejid, name, groups, subscription, NULL,
-            pending_out);
-
-        // add groups
-        while (groups != NULL) {
-            autocomplete_add(groups_ac, groups->data);
-            groups = g_slist_next(groups);
-        }
-
-        g_hash_table_insert(contacts, strdup(barejid), contact);
-        autocomplete_add(barejid_ac, barejid);
-        _add_name_and_barejid(name, barejid);
-
-        if (!from_initial) {
-            prof_handle_roster_add(barejid, name);
-        }
+    contact = p_contact_new(barejid, name, groups, subscription, NULL,
+        pending_out);
 
-        added = TRUE;
+    // add groups
+    while (groups != NULL) {
+        autocomplete_add(groups_ac, groups->data);
+        groups = g_slist_next(groups);
     }
 
-    return added;
+    g_hash_table_insert(contacts, strdup(barejid), contact);
+    autocomplete_add(barejid_ac, barejid);
+    _add_name_and_barejid(name, barejid);
+
+    return TRUE;
 }
 
 char *
diff --git a/src/roster_list.h b/src/roster_list.h
index a7198e5c..58490609 100644
--- a/src/roster_list.h
+++ b/src/roster_list.h
@@ -42,7 +42,7 @@ void roster_remove(const char * const name, const char * const barejid);
 void roster_update(const char * const barejid, const char * const name,
     GSList *groups, const char * const subscription, gboolean pending_out);
 gboolean roster_add(const char * const barejid, const char * const name, GSList *groups,
-    const char * const subscription, gboolean pending_out, gboolean from_initial);
+    const char * const subscription, gboolean pending_out);
 char * roster_barejid_from_name(const char * const name);
 GSList * roster_get_contacts(void);
 gboolean roster_has_pending_subscriptions(void);
diff --git a/src/server_events.c b/src/server_events.c
index cbf2de3c..ac940c86 100644
--- a/src/server_events.c
+++ b/src/server_events.c
@@ -366,3 +366,10 @@ handle_roster_remove(const char * const barejid)
     ui_roster_remove(barejid);
     ui_current_page_off();
 }
+
+void
+handle_roster_add(const char * const barejid, const char * const name)
+{
+    ui_roster_add(barejid, name);
+    ui_current_page_off();
+}
diff --git a/src/server_events.h b/src/server_events.h
index b79e9c7b..ce3c3b49 100644
--- a/src/server_events.h
+++ b/src/server_events.h
@@ -71,5 +71,6 @@ void handle_group_add(const char * const contact,
 void handle_group_remove(const char * const contact,
     const char * const group);
 void handle_roster_remove(const char * const barejid);
+void handle_roster_add(const char * const barejid, const char * const name);
 
 #endif
diff --git a/src/xmpp/roster.c b/src/xmpp/roster.c
index 3da8c7d3..a70436e7 100644
--- a/src/xmpp/roster.c
+++ b/src/xmpp/roster.c
@@ -255,7 +255,15 @@ _roster_handle_push(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza,
         GSList *groups = _get_groups_from_item(item);
 
         // update the local roster
-        roster_update(barejid, name, groups, sub, pending_out);
+        PContact contact = roster_get_contact(barejid);
+        if (contact == NULL) {
+            gboolean added = roster_add(barejid, name, groups, sub, pending_out);
+            if (added) {
+                handle_roster_add(barejid, name);
+            }
+        } else {
+            roster_update(barejid, name, groups, sub, pending_out);
+        }
     }
 
     return 1;
@@ -289,7 +297,7 @@ _roster_handle_result(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza,
 
             GSList *groups = _get_groups_from_item(item);
 
-            gboolean added = roster_add(barejid, name, groups, sub, pending_out, TRUE);
+            gboolean added = roster_add(barejid, name, groups, sub, pending_out);
 
             if (!added) {
                 log_warning("Attempt to add contact twice: %s", barejid);
diff --git a/tests/test_roster_list.c b/tests/test_roster_list.c
index 104921ba..a5612072 100644
--- a/tests/test_roster_list.c
+++ b/tests/test_roster_list.c
@@ -20,7 +20,7 @@ void empty_list_when_none_added(void **state)
 void contains_one_element(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
     GSList *list = roster_get_contacts();
     assert_int_equal(1, g_slist_length(list));
     roster_free();
@@ -29,7 +29,7 @@ void contains_one_element(void **state)
 void first_element_correct(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
     GSList *list = roster_get_contacts();
     PContact james = list->data;
 
@@ -40,8 +40,8 @@ void first_element_correct(void **state)
 void contains_two_elements(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
     GSList *list = roster_get_contacts();
 
     assert_int_equal(2, g_slist_length(list));
@@ -51,8 +51,8 @@ void contains_two_elements(void **state)
 void first_and_second_elements_correct(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
     GSList *list = roster_get_contacts();
 
     PContact first = list->data;
@@ -66,9 +66,9 @@ void first_and_second_elements_correct(void **state)
 void contains_three_elements(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
     GSList *list = roster_get_contacts();
 
     assert_int_equal(3, g_slist_length(list));
@@ -78,9 +78,9 @@ void contains_three_elements(void **state)
 void first_three_elements_correct(void **state)
 {
     roster_init();
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
     GSList *list = roster_get_contacts();
     PContact bob = list->data;
     PContact dave = (g_slist_next(list))->data;
@@ -95,10 +95,10 @@ void first_three_elements_correct(void **state)
 void add_twice_at_beginning_adds_once(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
     GSList *list = roster_get_contacts();
     PContact first = list->data;
     PContact second = (g_slist_next(list))->data;
@@ -114,10 +114,10 @@ void add_twice_at_beginning_adds_once(void **state)
 void add_twice_in_middle_adds_once(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
     GSList *list = roster_get_contacts();
     PContact first = list->data;
     PContact second = (g_slist_next(list))->data;
@@ -133,10 +133,10 @@ void add_twice_in_middle_adds_once(void **state)
 void add_twice_at_end_adds_once(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
     GSList *list = roster_get_contacts();
     PContact first = list->data;
     PContact second = (g_slist_next(list))->data;
@@ -152,9 +152,9 @@ void add_twice_at_end_adds_once(void **state)
 void find_first_exists(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
 
     char *search = (char *) malloc(2 * sizeof(char));
     strcpy(search, "B");
@@ -169,9 +169,9 @@ void find_first_exists(void **state)
 void find_second_exists(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
 
     char *result = roster_find_contact("Dav");
     assert_string_equal("Dave", result);
@@ -182,9 +182,9 @@ void find_second_exists(void **state)
 void find_third_exists(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
 
     char *result = roster_find_contact("Ja");
     assert_string_equal("James", result);
@@ -195,9 +195,9 @@ void find_third_exists(void **state)
 void find_returns_null(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
 
     char *result = roster_find_contact("Mike");
     assert_null(result);
@@ -215,9 +215,9 @@ void find_on_empty_returns_null(void **state)
 void find_twice_returns_second_when_two_match(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Jamie", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Jamie", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
 
     char *result1 = roster_find_contact("Jam");
     char *result2 = roster_find_contact(result1);
@@ -230,16 +230,16 @@ void find_twice_returns_second_when_two_match(void **state)
 void find_five_times_finds_fifth(void **state)
 {
     roster_init();
-    roster_add("Jama", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Jamb", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Mike", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Dave", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Jamm", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Jamn", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Matt", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Jamo", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Jamy", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Jamz", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("Jama", NULL, NULL, NULL, FALSE);
+    roster_add("Jamb", NULL, NULL, NULL, FALSE);
+    roster_add("Mike", NULL, NULL, NULL, FALSE);
+    roster_add("Dave", NULL, NULL, NULL, FALSE);
+    roster_add("Jamm", NULL, NULL, NULL, FALSE);
+    roster_add("Jamn", NULL, NULL, NULL, FALSE);
+    roster_add("Matt", NULL, NULL, NULL, FALSE);
+    roster_add("Jamo", NULL, NULL, NULL, FALSE);
+    roster_add("Jamy", NULL, NULL, NULL, FALSE);
+    roster_add("Jamz", NULL, NULL, NULL, FALSE);
 
     char *result1 = roster_find_contact("Jam");
     char *result2 = roster_find_contact(result1);
@@ -258,9 +258,9 @@ void find_five_times_finds_fifth(void **state)
 void find_twice_returns_first_when_two_match_and_reset(void **state)
 {
     roster_init();
-    roster_add("James", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Jamie", NULL, NULL, NULL, FALSE, TRUE);
-    roster_add("Bob", NULL, NULL, NULL, FALSE, TRUE);
+    roster_add("James", NULL, NULL, NULL, FALSE);
+    roster_add("Jamie", NULL, NULL, NULL, FALSE);
+    roster_add("Bob", NULL, NULL, NULL, FALSE);
 
     char *result1 = roster_find_contact("Jam");
     roster_reset_search_attempts();