Merge lp:~jamesh/account-polld/accounts-integration into lp:~phablet-team/account-polld/trunk

[James Henstridge]

Update and refactor the online-accounts binding to support tracking multiple services and multiple instances of each service. I also split the main C logic into a separate file independent of the Go binding since it allowed me to run it independently.

There is a compiler warning due to the callback function exported from Go not matching the signature expected by the C interface: the first argument should be *C.AccountWatcher, but that doesn't seem to work right. The Go 1.3 release notes seem to indicate that they've fixed up handling of incomplete types there, but I haven't tested it yet.

The Go interface remains compatible, but perhaps it could do with some changes too.

[lp:~jamesh/account-polld/accounts-integration]

13. By James Henstridge on 2014-07-11

Remove unneeded includes.

[Sergio Schvezov (sergiusens)]

looks good to me, the only thing I'd forward fix for Poll is to use this
type AuthData struct

instead of that map type which seems to just be a complication.

review: Approve

=== added file 'accounts/account-watcher.c'
--- accounts/account-watcher.c	1970-01-01 00:00:00 +0000
+++ accounts/account-watcher.c	2014-07-11 13:44:43 +0000
@@ -0,0 +1,279 @@
+/*
+ Copyright 2014 Canonical Ltd.
+ Authors: James Henstridge <[e-mail]>
+
+ This program is free software: you can redistribute it and/or modify it
+ under the terms of the GNU General Public License version 3, as published
+ by the Free Software Foundation.
+
+ This program is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranties of
+ MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ PURPOSE.  See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along
+ with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+#include <stdio.h>
+
+#include <glib.h>
+#include <libaccounts-glib/accounts-glib.h>
+#include <libsignon-glib/signon-glib.h>
+
+#include "account-watcher.h"
+
+struct _AccountWatcher {
+    AgManager *manager;
+    /* A hash table of the service names we are interested in */
+    GHashTable *services_to_watch;
+    /* A hash table of the enabled accounts we know of.
+     * Keys are account ID integers, and AccountInfo structs as values.
+     */
+    GHashTable *services;
+
+    gulong enabled_event_signal_id;
+    gulong account_deleted_signal_id;
+
+    AccountEnabledCallback callback;
+    void *user_data;
+};
+
+typedef struct _AccountInfo AccountInfo;
+struct _AccountInfo {
+    AccountWatcher *watcher;
+    /* Manage signin session for account */
+    AgAccountService *account_service;
+    SignonAuthSession *session;
+    GVariant *auth_params;
+    GVariant *session_data;
+
+    gulong enabled_signal_id;
+    AgAccountId account_id;
+    gboolean enabled; /* the last known state of the account */
+};
+
+static void account_info_clear_login(AccountInfo *info) {
+    if (info->session_data) {
+        g_variant_unref(info->session_data);
+        info->session_data = NULL;
+    }
+    if (info->auth_params) {
+        g_variant_unref(info->auth_params);
+        info->auth_params = NULL;
+    }
+    if (info->session) {
+        signon_auth_session_cancel(info->session);
+        g_object_unref(info->session);
+        info->session = NULL;
+    }
+}
+
+static void account_info_free(AccountInfo *info) {
+    account_info_clear_login(info);
+    if (info->enabled_signal_id != 0) {
+        g_signal_handler_disconnect(
+            info->account_service, info->enabled_signal_id);
+    }
+    info->enabled_signal_id = 0;
+    if (info->account_service) {
+        g_object_unref(info->account_service);
+        info->account_service = NULL;
+    }
+    g_free(info);
+}
+
+static void account_info_notify(AccountInfo *info) {
+    AgService *service = ag_account_service_get_service(info->account_service);
+    const char *service_name = ag_service_get_name(service);
+    char *client_id = NULL;
+    char *client_secret = NULL;
+    char *access_token = NULL;
+    char *token_secret = NULL;
+
+    if (info->enabled) {
+        /* Look up OAuth 2 parameters, falling back to OAuth 1 names */
+        g_variant_lookup(info->auth_params, "ClientId", "&s", &client_id);
+        g_variant_lookup(info->auth_params, "ClientSecret", "&s", &client_secret);
+        g_variant_lookup(info->session_data, "AccessToken", "&s", &access_token);
+        g_variant_lookup(info->session_data, "TokenSecret", "&s", &token_secret);
+
+        if (client_id == NULL) {
+            g_variant_lookup(info->auth_params, "ConsumerKey", "&s", &client_id);
+        }
+        if (client_secret == NULL) {
+            g_variant_lookup(info->auth_params, "ConsumerSecret", "&s", &client_secret);
+        }
+    }
+
+    info->watcher->callback(info->watcher,
+                            info->account_id,
+                            service_name,
+                            info->enabled,
+                            client_id,
+                            client_secret,
+                            access_token,
+                            token_secret,
+                            info->watcher->user_data);
+}
+
+static void account_info_login_cb(GObject *source, GAsyncResult *result, void *user_data) {
+    SignonAuthSession *session = (SignonAuthSession *)source;
+    AccountInfo *info = (AccountInfo *)user_data;
+
+    fprintf(stderr, "Authentication for account %u complete\n", info->account_id);
+
+    GError *error = NULL;
+    info->session_data = signon_auth_session_process_finish(session, result, &error);
+
+    if (error != NULL) {
+        fprintf(stderr, "Authentication failed: %s\n", error->message);
+        g_error_free(error);
+        return;
+    }
+    account_info_notify(info);
+}
+
+static void account_info_enabled_cb(
+    AgAccountService *account_service, gboolean enabled, AccountInfo *info) {
+    fprintf(stderr, "account_info_enabled_cb for %u, enabled=%d\n", info->account_id, enabled);
+    if (info->enabled == enabled) {
+        /* no change */
+        return;
+    }
+    info->enabled = enabled;
+
+    account_info_clear_login(info);
+    if (!enabled) {
+        // Send notification that account has been disabled */
+        account_info_notify(info);
+        return;
+    }
+
+    AgAuthData *auth_data = ag_account_service_get_auth_data(account_service);
+    GError *error = NULL;
+    fprintf(stderr, "Starting authentication session for account %u\n", info->account_id);
+    info->session = signon_auth_session_new(
+        ag_auth_data_get_credentials_id(auth_data),
+        ag_auth_data_get_method(auth_data), &error);
+    if (error != NULL) {
+        fprintf(stderr, "Could not set up auth session: %s\n", error->message);
+        g_error_free(error);
+        g_object_unref(auth_data);
+        return;
+    }
+
+    GVariantBuilder builder;
+    g_variant_builder_init(&builder, G_VARIANT_TYPE_VARDICT);
+    g_variant_builder_add(
+        &builder, "{sv}",
+        SIGNON_SESSION_DATA_UI_POLICY,
+        g_variant_new_int32(SIGNON_POLICY_NO_USER_INTERACTION));
+
+    info->auth_params = g_variant_ref_sink(
+        ag_auth_data_get_login_parameters(
+            auth_data, g_variant_builder_end(&builder)));
+
+    signon_auth_session_process_async(
+        info->session,
+        info->auth_params,
+        ag_auth_data_get_mechanism(auth_data),
+        NULL, /* cancellable */
+        account_info_login_cb, info);
+    ag_auth_data_unref(auth_data);
+}
+
+static AccountInfo *account_info_new(AccountWatcher *watcher, AgAccountService *account_service) {
+    AccountInfo *info = g_new0(AccountInfo, 1);
+    info->watcher = watcher;
+    info->account_service = g_object_ref(account_service);
+
+    AgAccount *account = ag_account_service_get_account(account_service);
+    g_object_get(account, "id", &info->account_id, NULL);
+
+    info->enabled_signal_id = g_signal_connect(
+        account_service, "enabled",
+        G_CALLBACK(account_info_enabled_cb), info);
+    // Set initial state
+    account_info_enabled_cb(account_service, ag_account_service_get_enabled(account_service), info);
+
+    return info;
+}
+
+static void account_watcher_enabled_event_cb(
+    AgManager *manager, AgAccountId account_id, AccountWatcher *watcher) {
+    fprintf(stderr, "enabled-event for %u\n", account_id);
+    if (g_hash_table_contains(watcher->services, GUINT_TO_POINTER(account_id))) {
+        /* We are already tracking this account */
+        return;
+    }
+    AgAccount *account = ag_manager_get_account(manager, account_id);
+    if (account == NULL) {
+        /* There was a problem looking up the account */
+        return;
+    }
+    GList *services = ag_account_list_services(account);
+    GList *l;
+    for (l = services; l != NULL; l = l->next) {
+        AgService *service = l->data;
+
+        const char *name = ag_service_get_name(service);
+        if (g_hash_table_contains(watcher->services_to_watch, name)) {
+            AgAccountService *account_service = ag_account_service_new(
+                account, service);
+            AccountInfo *info = account_info_new(watcher, account_service);
+            g_object_unref(account_service);
+            g_hash_table_insert(watcher->services, GUINT_TO_POINTER(account_id), info);
+            break;
+        }
+    }
+    ag_service_list_free(services);
+    g_object_unref(account);
+}
+
+static void account_watcher_account_deleted_cb(
+    AgManager *manager, AgAccountId account_id, AccountWatcher *watcher) {
+    fprintf(stderr, "account-deleted for %u\n", account_id);
+    /* A disabled event should have been sent prior to this, so no
+     * need to send any notification. */
+    g_hash_table_remove(watcher->services, GUINT_TO_POINTER(account_id));
+}
+
+static gboolean account_watcher_setup(void *user_data) {
+    AccountWatcher *watcher = (AccountWatcher *)user_data;
+
+    /* Track changes to accounts */
+    watcher->enabled_event_signal_id = g_signal_connect(
+        watcher->manager, "enabled-event",
+        G_CALLBACK(account_watcher_enabled_event_cb), watcher);
+    watcher->account_deleted_signal_id = g_signal_connect(
+        watcher->manager, "account-deleted",
+        G_CALLBACK(account_watcher_account_deleted_cb), watcher);
+
+    /* Now check initial state */
+    GList *enabled_accounts = ag_manager_list(watcher->manager);
+    GList *l;
+    for (l = enabled_accounts; l != NULL; l = l->next) {
+        AgAccountId account_id = GPOINTER_TO_UINT(l->data);
+        account_watcher_enabled_event_cb(watcher->manager, account_id, watcher);
+    }
+    ag_manager_list_free(enabled_accounts);
+
+    return FALSE;
+}
+
+AccountWatcher *account_watcher_new(GHashTable *services_to_watch,
+                                    AccountEnabledCallback callback,
+                                    void *user_data) {
+    AccountWatcher *watcher = g_new0(AccountWatcher, 1);
+
+    watcher->manager = ag_manager_new();
+    watcher->services_to_watch = g_hash_table_ref(services_to_watch);
+    watcher->services = g_hash_table_new_full(
+        g_direct_hash, g_direct_equal, NULL, (GDestroyNotify)account_info_free);
+    watcher->callback = callback;
+    watcher->user_data = user_data;
+
+    /* Make sure main setup occurs within the mainloop thread */
+    g_idle_add(account_watcher_setup, watcher);
+    return watcher;
+}

=== added file 'accounts/account-watcher.h'
--- accounts/account-watcher.h	1970-01-01 00:00:00 +0000
+++ accounts/account-watcher.h	2014-07-11 13:44:43 +0000
@@ -0,0 +1,38 @@
+/*
+ Copyright 2014 Canonical Ltd.
+ Authors: James Henstridge <[e-mail]>
+
+ This program is free software: you can redistribute it and/or modify it
+ under the terms of the GNU General Public License version 3, as published
+ by the Free Software Foundation.
+
+ This program is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranties of
+ MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ PURPOSE.  See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along
+ with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+#ifndef ACCOUNT_WATCHER_H
+#define ACCOUNT_WATCHER_H
+
+#include <glib.h>
+
+typedef struct _AccountWatcher AccountWatcher;
+
+typedef void (*AccountEnabledCallback)(AccountWatcher *watcher,
+                                       unsigned int account_id,
+                                       const char *service_name,
+                                       int enabled,
+                                       const char *client_id,
+                                       const char *client_secret,
+                                       const char *access_token,
+                                       const char *token_secret,
+                                       void *user_data);
+
+AccountWatcher *account_watcher_new(GHashTable *services_to_watch,
+                                    AccountEnabledCallback callback,
+                                    void *user_data);
+
+#endif

=== modified file 'accounts/accounts.c'
--- accounts/accounts.c	2014-07-11 13:44:43 +0000
+++ accounts/accounts.c	2014-07-11 13:44:43 +0000
@@ -1,154 +1,18 @@
 #include "_cgo_export.h"
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
-
-#include <glib.h>
-#include <libaccounts-glib/accounts-glib.h>
-#include <libsignon-glib/signon-glib.h>
-
-typedef struct _AuthContext AuthContext;
-struct _AuthContext {
-    AgManager *manager;
-    char *service_name;
-    AgAccountService *account_service;
-    AgAuthData *auth_data;
-    SignonAuthSession *session;
-
-    GVariant *auth_params;
-    GVariant *session_data;
-};
-
-static void login_cb(GObject *source, GAsyncResult *result, void *user_data) {
-    SignonAuthSession *session = (SignonAuthSession *)source;
-    AuthContext *ctx = (AuthContext *)user_data;
-
-    GError *error = NULL;
-    ctx->session_data = signon_auth_session_process_finish(session, result, &error);
-
-    g_object_unref(ctx->session);
-    ctx->session = NULL;
-
-    if (error != NULL) {
-        fprintf(stderr, "Authentication failed: %s\n", error->message);
-        g_error_free(error);
-        return;
-    }
-
-    char *client_id = NULL;
-    char *client_secret = NULL;
-    char *access_token = NULL;
-    g_variant_lookup(ctx->auth_params, "ClientId", "&s", &client_id);
-    g_variant_lookup(ctx->auth_params, "ClientSecret", "&s", &client_secret);
-    g_variant_lookup(ctx->session_data, "AccessToken", "&s", &access_token);
-
-    authLogin(ctx, client_id, client_secret, access_token);
-}
-
-static void login_service(AuthContext *ctx) {
-    ctx->auth_data = ag_account_service_get_auth_data(ctx->account_service);
-
-    GError *error = NULL;
-    ctx->session = signon_auth_session_new(
-        ag_auth_data_get_credentials_id(ctx->auth_data),
-        ag_auth_data_get_method(ctx->auth_data), &error);
-    if (error != NULL) {
-        fprintf(stderr, "Could not set up auth session: %s\n", error->message);
-        g_error_free(error);
-        return;
-    }
-
-    GVariantBuilder builder;
-    g_variant_builder_init(&builder, G_VARIANT_TYPE_VARDICT);
-    g_variant_builder_add(
-        &builder, "{sv}",
-        SIGNON_SESSION_DATA_UI_POLICY,
-        g_variant_new_int32(SIGNON_POLICY_NO_USER_INTERACTION));
-
-    ctx->auth_params = g_variant_ref_sink(
-        ag_auth_data_get_login_parameters(
-            ctx->auth_data, g_variant_builder_end(&builder)));
-
-    signon_auth_session_process_async(
-        ctx->session,
-        ctx->auth_params,
-        ag_auth_data_get_mechanism(ctx->auth_data),
-        NULL, /* cancellable */
-        login_cb, ctx);
-}
-
-static void logout_service(AuthContext *ctx) {
-    if (ctx->session_data) {
-        g_variant_unref(ctx->session_data);
-        ctx->session_data = NULL;
-    }
-    if (ctx->auth_params) {
-        g_variant_unref(ctx->auth_params);
-        ctx->auth_params = NULL;
-    }
-    if (ctx->session) {
-        signon_auth_session_cancel(ctx->session);
-        g_object_unref(ctx->session);
-        ctx->session = NULL;
-    }
-    if (ctx->auth_data) {
-        g_object_unref(ctx->auth_data);
-        ctx->auth_data = NULL;
-    }
-    if (ctx->account_service) {
-        g_object_unref(ctx->account_service);
-        ctx->account_service = NULL;
-    }
-
-    authLogin(ctx, NULL, NULL, NULL);
-}
-
-static void lookup_account_service(AuthContext *ctx) {
-    GList *account_services = ag_manager_get_enabled_account_services(ctx->manager);
-    GList *tmp;
-    for (tmp = account_services; tmp != NULL; tmp = tmp->next) {
-        AgAccountService *acct_svc = AG_ACCOUNT_SERVICE(tmp->data);
-        AgService *service = ag_account_service_get_service(acct_svc);
-        if (!strcmp(ctx->service_name, ag_service_get_name(service))) {
-            ctx->account_service = g_object_ref(acct_svc);
-            break;
-        }
-    }
-    g_list_foreach(account_services, (GFunc)g_object_unref, NULL);
-    g_list_free(account_services);
-
-    if (ctx->account_service != NULL) {
-        login_service(ctx);
-    }
-}
-
-static void account_enabled_cb(AgManager *manager, guint account_id, void *user_data) {
-    AuthContext *ctx = (AuthContext *)user_data;
-
-    printf("enabled_cb account_id=%u\n", account_id);
-
-    if (ctx->account_service != NULL &&
-        !ag_account_service_get_enabled(ctx->account_service)) {
-        logout_service(ctx);
-    }
-    lookup_account_service(ctx);
-}
-
-static gboolean
-setup_context(void *user_data) {
-    AuthContext *ctx = (AuthContext *)user_data;
-
-    lookup_account_service(ctx);
-    g_signal_connect(ctx->manager, "enabled-event",
-                     G_CALLBACK(account_enabled_cb), ctx);
-    return FALSE;
-}
-
-AuthContext *watch_for_service(const char *service_name) {
-    AuthContext *ctx = g_new0(AuthContext, 1);
-    ctx->manager = ag_manager_new();
-    ctx->service_name = g_strdup(service_name);
-
-    g_idle_add(setup_context, ctx);
-    return ctx;
+#include "account-watcher.h"
+
+AccountWatcher *watch_for_services(void *array_of_service_names, int length) {
+    /* Transfer service names to hash table */
+    GoString *service_names = (GoString *)array_of_service_names;
+    GHashTable *services_to_watch = g_hash_table_new_full(
+        g_str_hash, g_str_equal, g_free, NULL);
+    int i;
+    for (i = 0; i < length; i++) {
+        g_hash_table_insert(services_to_watch, g_strdup(service_names[i].p), NULL);
+    }
+
+    AccountWatcher *watcher = account_watcher_new(
+        services_to_watch, authCallback, NULL);
+    g_hash_table_unref(services_to_watch);
+    return watcher;
 }

=== modified file 'accounts/accounts.go'
--- accounts/accounts.go	2014-07-11 13:44:43 +0000
+++ accounts/accounts.go	2014-07-11 13:44:43 +0000
@@ -22,49 +22,58 @@
 #include <stdlib.h>
 #include <glib.h>
 
-typedef struct _AuthContext AuthContext;
+typedef struct _AccountWatcher AccountWatcher;
 
-AuthContext *watch_for_service(const char *service_name);
+AccountWatcher *watch_for_services(void *array_of_service_names, int length);
 */
 import "C"
 import "unsafe"
 
 type AuthData struct {
+	AccountId   uint
+	ServiceName string
+	Enabled     bool
+
 	ClientId     string
 	ClientSecret string
 	AccessToken  string
+	TokenSecret  string
 }
 
 var (
 	mainLoop     *C.GMainLoop
-	authChannels = make(map[*C.AuthContext]chan<- AuthData)
+	authChannels = make(map[*C.AccountWatcher]chan<- AuthData)
 )
 
-func WatchForService(serviceName string) <-chan AuthData {
+func WatchForService(serviceNames... string) <-chan AuthData {
 	if mainLoop == nil {
 		mainLoop = C.g_main_loop_new(nil, C.gboolean(1))
 		go C.g_main_loop_run(mainLoop)
 	}
 
-	cService := C.CString(serviceName)
-	defer C.free(unsafe.Pointer(cService))
-	ctx := C.watch_for_service(cService)
+	watcher := C.watch_for_services(unsafe.Pointer(&serviceNames[0]), C.int(len(serviceNames)))
 
 	ch := make(chan AuthData)
-	authChannels[ctx] = ch
+	authChannels[watcher] = ch
 	return ch
 }
 
-//export authLogin
-func authLogin(user_data unsafe.Pointer, clientId *C.char, clientSecret *C.char, accessToken *C.char) {
-	ctx := (*C.AuthContext)(user_data)
-	ch := authChannels[ctx]
+//export authCallback
+func authCallback(watcher unsafe.Pointer, accountId C.uint, serviceName *C.char, enabled C.int, clientId, clientSecret, accessToken, tokenSecret *C.char, userData unsafe.Pointer) {
+	// Ideally the first argument would be of type
+	// *C.AccountWatcher, but that fails with Go 1.2.
+	ch := authChannels[(*C.AccountWatcher)(watcher)]
 	if ch == nil {
 		// Log the error
 		return
 	}
 
 	var data AuthData
+	data.AccountId = uint(accountId)
+	data.ServiceName = C.GoString(serviceName)
+	if enabled != 0 {
+		data.Enabled = true
+	}
 	if clientId != nil {
 		data.ClientId = C.GoString(clientId)
 	}
@@ -74,5 +83,8 @@
 	if accessToken != nil {
 		data.AccessToken = C.GoString(accessToken)
 	}
+	if tokenSecret != nil {
+		data.TokenSecret = C.GoString(tokenSecret)
+	}
 	ch <- data
 }

=== added directory 'cmd/account-watcher-test'
=== added file 'cmd/account-watcher-test/main.go'
--- cmd/account-watcher-test/main.go	1970-01-01 00:00:00 +0000
+++ cmd/account-watcher-test/main.go	2014-07-11 13:44:43 +0000
@@ -0,0 +1,15 @@
+package main
+
+import (
+	"fmt"
+	"os"
+
+	"launchpad.net/account-polld/accounts"
+)
+
+func main() {
+	// Expects a list of service names as command line arguments
+	for data := range accounts.WatchForService(os.Args[1:]...) {
+		fmt.Printf("%#v\n", data)
+	}
+}