Use hashtables for plugin disco features

6 files changed, 214 insertions, 28 deletions

diff --git a/Makefile.am b/Makefile.am
index 00e66081..3e4473c3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -126,6 +126,7 @@ unittest_sources = \
 	tests/unittests/test_cmd_roster.c tests/unittests/test_cmd_roster.h \
 	tests/unittests/test_cmd_disconnect.c tests/unittests/test_cmd_disconnect.h \
 	tests/unittests/test_callbacks.c tests/unittests/test_callbacks.h \
+	tests/unittests/test_plugins_disco.c tests/unittests/test_plugins_disco.h \
 	tests/unittests/unittests.c
 
 functionaltest_sources = \
diff --git a/src/plugins/disco.c b/src/plugins/disco.c
index bf42d4c0..320b7fa7 100644
--- a/src/plugins/disco.c
+++ b/src/plugins/disco.c
@@ -37,76 +37,112 @@
 
 #include <glib.h>
 
+// features to reference count map
+static GHashTable *features = NULL;
+
+// plugin to feature map
 static GHashTable *plugin_to_features = NULL;
 
 static void
-_free_features(GList *features)
+_free_features(GHashTable *features)
 {
-    g_list_free_full(features, free);
+    g_hash_table_destroy(features);
 }
 
 void
-disco_add_feature(const char *plugin_name, char* feature)
+disco_add_feature(const char *plugin_name, char *feature)
 {
     if (feature == NULL || plugin_name == NULL) {
         return;
     }
 
+    if (!features) {
+        features = g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL);
+    }
     if (!plugin_to_features) {
         plugin_to_features = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)_free_features);
     }
 
-    GList *features = g_hash_table_lookup(plugin_to_features, plugin_name);
-    if (!features) {
-        features = g_list_append(features, strdup(feature));
-        g_hash_table_insert(plugin_to_features, strdup(plugin_name), features);
+    GHashTable *plugin_features = g_hash_table_lookup(plugin_to_features, plugin_name);
+    gboolean added = FALSE;
+    if (plugin_features == NULL) {
+        plugin_features = g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL);
+        g_hash_table_add(plugin_features, strdup(feature));
+        g_hash_table_insert(plugin_to_features, strdup(plugin_name), plugin_features);
+        added = TRUE;
+    } else if (!g_hash_table_contains(plugin_features, feature)) {
+        g_hash_table_add(plugin_features, strdup(feature));
+        added = TRUE;
+    }
+
+    if (added == FALSE) {
+        return;
+    }
+
+    if (!g_hash_table_contains(features, feature)) {
+        g_hash_table_insert(features, strdup(feature), GINT_TO_POINTER(1));
     } else {
-        features = g_list_append(features, strdup(feature));
+        void *refcountp = g_hash_table_lookup(features, feature);
+        int refcount = GPOINTER_TO_INT(refcountp);
+        refcount++;
+        g_hash_table_replace(features, strdup(feature), GINT_TO_POINTER(refcount));
     }
 }
 
 void
 disco_remove_features(const char *plugin_name)
 {
+    if (!features) {
+        return;
+    }
     if (!plugin_to_features) {
         return;
     }
 
-    if (!g_hash_table_contains(plugin_to_features, plugin_name)) {
+    GHashTable *plugin_features_set = g_hash_table_lookup(plugin_to_features, plugin_name);
+    if (!plugin_features_set) {
         return;
     }
 
-    g_hash_table_remove(plugin_to_features, plugin_name);
+    GList *plugin_feature_list = g_hash_table_get_keys(plugin_features_set);
+    GList *curr = plugin_feature_list;
+    while (curr) {
+        char *feature = curr->data;
+        if (g_hash_table_contains(features, feature)) {
+            void *refcountp = g_hash_table_lookup(features, feature);
+            int refcount = GPOINTER_TO_INT(refcountp);
+            if (refcount == 1) {
+                g_hash_table_remove(features, feature);
+            } else {
+                refcount--;
+                g_hash_table_replace(features, strdup(feature), GINT_TO_POINTER(refcount));
+            }
+        }
+
+        curr = g_list_next(curr);
+    }
+    g_list_free(plugin_feature_list);
+
 }
 
 GList*
 disco_get_features(void)
 {
-    GList *result = NULL;
-    if (!plugin_to_features) {
-        return result;
+    if (features == NULL) {
+        return NULL;
     }
 
-    GList *lists = g_hash_table_get_values(plugin_to_features);
-    GList *curr_list = lists;
-    while (curr_list) {
-        GList *features = curr_list->data;
-        GList *curr = features;
-        while (curr) {
-            result = g_list_append(result, curr->data);
-            curr = g_list_next(curr);
-        }
-        curr_list = g_list_next(curr_list);
-    }
-
-    g_list_free(lists);
-
-    return result;
+    return g_hash_table_get_keys(features);
 }
 
 void
 disco_close(void)
 {
+    if (features) {
+        g_hash_table_destroy(features);
+        features = NULL;
+    }
+
     if (plugin_to_features) {
         g_hash_table_destroy(plugin_to_features);
         plugin_to_features = NULL;
diff --git a/src/plugins/disco.h b/src/plugins/disco.h
index f16e2642..d08b6537 100644
--- a/src/plugins/disco.h
+++ b/src/plugins/disco.h
@@ -35,6 +35,8 @@
 #ifndef PLUGINS_DISCO_H
 #define PLUGINS_DISCO_H
 
+#include <glib.h>
+
 void disco_add_feature(const char* plugin_name, char *feature);
 void disco_remove_features(const char *plugin_name);
 GList* disco_get_features(void);
diff --git a/tests/unittests/test_plugins_disco.c b/tests/unittests/test_plugins_disco.c
new file mode 100644
index 00000000..aac9da65
--- /dev/null
+++ b/tests/unittests/test_plugins_disco.c
@@ -0,0 +1,131 @@
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include <stdlib.h>
+
+#include "plugins/disco.h"
+
+void
+returns_empty_list_when_none(void **state)
+{
+    disco_close();
+    GList *features = disco_get_features();
+
+    assert_int_equal(g_list_length(features), 0);
+
+    g_list_free(features);
+    disco_close();
+}
+
+void
+returns_added_feature(void **state)
+{
+    disco_close();
+    disco_add_feature("my_plugin", "some:feature:example");
+
+    GList *features = disco_get_features();
+    assert_int_equal(g_list_length(features), 1);
+    char *feature = features->data;
+    assert_string_equal(feature, "some:feature:example");
+
+    g_list_free(features);
+    disco_close();
+}
+
+void
+resets_features_on_close(void **state)
+{
+    disco_close();
+    disco_add_feature("my_plugin", "some:feature:example");
+
+    GList *features = disco_get_features();
+    assert_int_equal(g_list_length(features), 1);
+    g_list_free(features);
+
+    disco_close();
+    features = disco_get_features();
+    assert_int_equal(g_list_length(features), 0);
+
+    g_list_free(features);
+    disco_close();
+}
+
+void
+returns_all_added_features(void **state)
+{
+    disco_close();
+    disco_add_feature("first_plugin", "first:feature");
+    disco_add_feature("first_plugin", "second:feature");
+    disco_add_feature("second_plugin", "third:feature");
+    disco_add_feature("third_plugin", "fourth:feature");
+    disco_add_feature("third_plugin", "fifth:feature");
+
+    GList *features = disco_get_features();
+
+    assert_int_equal(g_list_length(features), 5);
+    assert_true(g_list_find_custom(features, "first:feature", g_strcmp0));
+    assert_true(g_list_find_custom(features, "second:feature", g_strcmp0));
+    assert_true(g_list_find_custom(features, "third:feature", g_strcmp0));
+    assert_true(g_list_find_custom(features, "fourth:feature", g_strcmp0));
+    assert_true(g_list_find_custom(features, "fifth:feature", g_strcmp0));
+
+    g_list_free(features);
+    disco_close();
+}
+
+void
+does_not_add_duplicate_feature(void **state)
+{
+    disco_close();
+    disco_add_feature("my_plugin", "my:feature");
+    disco_add_feature("some_other_plugin", "my:feature");
+
+    GList *features = disco_get_features();
+    assert_int_equal(g_list_length(features), 1);
+
+    g_list_free(features);
+    disco_close();
+}
+
+void
+removes_plugin_features(void **state)
+{
+    disco_close();
+    disco_add_feature("plugin1", "plugin1:feature1");
+    disco_add_feature("plugin1", "plugin1:feature2");
+    disco_add_feature("plugin2", "plugin2:feature1");
+
+    GList *features = disco_get_features();
+    assert_int_equal(g_list_length(features), 3);
+    g_list_free(features);
+
+    disco_remove_features("plugin1");
+
+    features = disco_get_features();
+    assert_int_equal(g_list_length(features), 1);
+
+    g_list_free(features);
+    disco_close();
+}
+
+void
+does_not_remove_feature_when_more_than_one_reference(void **state)
+{
+    disco_close();
+    disco_add_feature("plugin1", "feature1");
+    disco_add_feature("plugin1", "feature2");
+    disco_add_feature("plugin2", "feature1");
+
+    GList *features = disco_get_features();
+    assert_int_equal(g_list_length(features), 2);
+    g_list_free(features);
+
+    disco_remove_features("plugin1");
+
+    features = disco_get_features();
+    assert_int_equal(g_list_length(features), 1);
+
+    g_list_free(features);
+    disco_close();
+}
diff --git a/tests/unittests/test_plugins_disco.h b/tests/unittests/test_plugins_disco.h
new file mode 100644
index 00000000..937cbfe1
--- /dev/null
+++ b/tests/unittests/test_plugins_disco.h
@@ -0,0 +1,7 @@
+void returns_empty_list_when_none(void **state);
+void returns_added_feature(void **state);
+void resets_features_on_close(void **state);
+void returns_all_added_features(void **state);
+void does_not_add_duplicate_feature(void **state);
+void removes_plugin_features(void **state);
+void does_not_remove_feature_when_more_than_one_reference(void **state);
diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c
index 9edb8b92..e427e151 100644
--- a/tests/unittests/unittests.c
+++ b/tests/unittests/unittests.c
@@ -34,6 +34,7 @@
 #include "test_cmd_disconnect.h"
 #include "test_form.h"
 #include "test_callbacks.h"
+#include "test_plugins_disco.h"
 
 int main(int argc, char* argv[]) {
     const UnitTest all_tests[] = {
@@ -587,6 +588,14 @@ int main(int argc, char* argv[]) {
 
         unit_test(returns_no_commands),
         unit_test(returns_commands),
+
+        unit_test(returns_empty_list_when_none),
+        unit_test(returns_added_feature),
+        unit_test(resets_features_on_close),
+        unit_test(returns_all_added_features),
+        unit_test(does_not_add_duplicate_feature),
+        unit_test(removes_plugin_features),
+        unit_test(does_not_remove_feature_when_more_than_one_reference),
     };
 
     return run_tests(all_tests);