poll-daemon

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

  1. account-refresh
  2. Merge into trunk
Proposed by James Henstridge
Status: Merged
Approved by: Sergio Schvezov
Approved revision: 30
Merged at revision: 21
Proposed branch: lp:~jamesh/account-polld/account-refresh
Merge into: lp:~phablet-team/account-polld/trunk
Diff against target: 868 lines (+306/-123)
18 files modified
accounts/account-watcher.c (+80/-55)
accounts/account-watcher.h (+4/-1)
accounts/accounts.c (+16/-10)
accounts/accounts.go (+47/-14)
cmd/account-polld/account_manager.go (+30/-18)
cmd/account-polld/main.go (+8/-4)
cmd/account-watcher-test/main.go (+6/-2)
data/account-polld.application (+5/-11)
data/account-polld.service-type (+7/-0)
data/facebook-poll.service (+8/-0)
data/google-gmail-poll.service (+1/-1)
data/twitter-poll.service (+8/-0)
debian/rules (+4/-1)
plugins/facebook/facebook.go (+3/-0)
plugins/facebook/facebook_test.go (+25/-5)
plugins/plugins.go (+9/-1)
plugins/twitter/twitter.go (+12/-0)
plugins/twitter/twitter_test.go (+33/-0)
To merge this branch: bzr merge lp:~jamesh/account-polld/account-refresh
Reviewer Review Type Date Requested Status
Sergio Schvezov Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+227759@code.launchpad.net

Commit message

Add support for handling token expiry.

Description of the change

Add support for handling token expiry. The basic mechanism is that:

1. plugins convert their "expired token" errors into the plugins.ErrTokenExpired error.
2. the AccountManager handles this error by calling watcher.Refresh(accountId)
3. the new token is returned via the watcher's channel, as with other changes.

Along the way, I made a few changes:

* the accounts package now exposes a Watcher object holding the channel. This was so I'd have somewhere to hang the Refresh() method.
* I replaced the AccountManager's terminate channel with a channel for receiving the AuthData. Closing this channel is used as a terminate signal, and removed the need for a mutex.

To post a comment you must log in.
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

Looks good, just added one minor inline comment.

Another non related to this change comment is to ask if it would be ok to change the fprintf's to call go's log.Print.*

Revision history for this message
Sergio Schvezov (sergiusens) wrote :

ideally we should only print from package main, so this is really optional.

Revision history for this message
Sergio Schvezov (sergiusens) wrote :

I don't think it's catching accounts being created while running, so if I create my twitter account during a polld run it never shows up (I do go into the account and enable/disable the toggle)

if I restart polld the, the account is seen. Looking at the code again, I see only one call to ag_manager_list and no callback setup for new accounts. Should that be fixed in this merge?

Rest of the code looks fine.

review: Needs Information
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

Looking again, it should be handled by account_watcher_enabled_event_cb; not sure why I'm not seeing new accounts then... (I was playing with twitter fwiw).

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:22
http://jenkins.qa.ubuntu.com/job/account-polld-ci/2/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/2
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/2/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/2

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/2/rebuild

review: Approve (continuous-integration)
lp:~jamesh/account-polld/account-refresh updated
23. By James Henstridge

Convert account watcher to track a single service type rather than a set
of service names. This is needed to properly receive enabled event
notifications.

24. By James Henstridge

Add service files for Facebook and Twitter, and a service type file that
lets us select them along with GMail.

25. By James Henstridge

Fix up a crash when we are reporting errors to Go and no session data is
available.

26. By James Henstridge

Switch to the new prototype for accounts.NewWatcher() and the new
service names for Twitter and Facebook.

27. By James Henstridge

Disable the debug spew from account-watcher.c

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:26
http://jenkins.qa.ubuntu.com/job/account-polld-ci/3/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/3
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/3
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/3/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/3

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/3/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:27
http://jenkins.qa.ubuntu.com/job/account-polld-ci/4/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/4
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/4/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/4

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/4/rebuild

review: Approve (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

I was able to confirm the problem detecting new accounts, and believe I've fixed that in this branch.

It looks like the enabled-event isn't reliably sent out unless the account manager is limited to a single service type. So I've added an account-polld.service-type file along with services for Facebook and Twitter (as discussed during the call on Tuesday). The AccountWatcher now watches for all account services matching "account-polld", so gets notified of the three services we're interested in. This seems to do the trick and simplifies things a bit.

I've also disabled the debug spew from account-watcher.c, while still allowing it to be turned back on if I need to track down some logic bugs.

lp:~jamesh/account-polld/account-refresh updated
28. By James Henstridge

Merge from trunk

29. By James Henstridge

Install additional online accounts files.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:29
http://jenkins.qa.ubuntu.com/job/account-polld-ci/5/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/5/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/5/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/5/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/5/rebuild

review: Needs Fixing (continuous-integration)
lp:~jamesh/account-polld/account-refresh updated
30. By James Henstridge

Fix typo.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:30
http://jenkins.qa.ubuntu.com/job/account-polld-ci/6/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-amd64-ci/6
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/6
        deb: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/6/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/account-polld-utopic-i386-ci/6

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/account-polld-ci/6/rebuild

review: Approve (continuous-integration)
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

Using service types doesn't seem to work with gmail

Revision history for this message
James Henstridge (jamesh) wrote :

Can you be more specific about what doesn't work? Are you using the updated .service file? (note that I updated the service type here).

When I run "./account-watcher-test account-polld" with the extra files installed, I get auth data for all three of my accounts.

Revision history for this message
Sergio Schvezov (sergiusens) wrote :

I never get the enabled signal for gmail, nor do I see the application toggle in online accounts when testing on the phone (haven't tried on the desktop).

There's also a typo in debian/rules I've made
669 ${CURDIR}/debian/account-polld/usr/share/accounts/application
and the cp is missing a trailing 's'.

But my testing involved installing
http://jenkins.qa.ubuntu.com/job/account-polld-utopic-armhf-ci/6/artifact/work/output/*zip*/output.zip and moving the .application afterwards

Revision history for this message
Sergio Schvezov (sergiusens) wrote :

what I am seeing are most likely online accounts issues which I can reproduce with sync-monitor as well.

review: Approve
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

I'll get this in a silo in the morning; feel free to do so yourself if you want; I'll chain up some MPs similar to this one
https://code.launchpad.net/~sergiusens/account-polld/oa_fixes/+merge/228195

if you don't mind doing them here as well (at least the trailing 's')

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'accounts/account-watcher.c'
2--- accounts/account-watcher.c 2014-07-11 12:01:42 +0000
3+++ accounts/account-watcher.c 2014-07-24 08:50:37 +0000
4@@ -22,10 +22,15 @@
5
6 #include "account-watcher.h"
7
8+/* #define DEBUG */
9+#ifdef DEBUG
10+# define trace(...) fprintf(stderr, __VA_ARGS__)
11+#else
12+# define trace(...)
13+#endif
14+
15 struct _AccountWatcher {
16 AgManager *manager;
17- /* A hash table of the service names we are interested in */
18- GHashTable *services_to_watch;
19 /* A hash table of the enabled accounts we know of.
20 * Keys are account ID integers, and AccountInfo structs as values.
21 */
22@@ -82,7 +87,7 @@
23 g_free(info);
24 }
25
26-static void account_info_notify(AccountInfo *info) {
27+static void account_info_notify(AccountInfo *info, GError *error) {
28 AgService *service = ag_account_service_get_service(info->account_service);
29 const char *service_name = ag_service_get_name(service);
30 char *client_id = NULL;
31@@ -90,13 +95,10 @@
32 char *access_token = NULL;
33 char *token_secret = NULL;
34
35- if (info->enabled) {
36+ if (info->auth_params != NULL) {
37 /* Look up OAuth 2 parameters, falling back to OAuth 1 names */
38 g_variant_lookup(info->auth_params, "ClientId", "&s", &client_id);
39 g_variant_lookup(info->auth_params, "ClientSecret", "&s", &client_secret);
40- g_variant_lookup(info->session_data, "AccessToken", "&s", &access_token);
41- g_variant_lookup(info->session_data, "TokenSecret", "&s", &token_secret);
42-
43 if (client_id == NULL) {
44 g_variant_lookup(info->auth_params, "ConsumerKey", "&s", &client_id);
45 }
46@@ -104,10 +106,15 @@
47 g_variant_lookup(info->auth_params, "ConsumerSecret", "&s", &client_secret);
48 }
49 }
50+ if (info->session_data != NULL) {
51+ g_variant_lookup(info->session_data, "AccessToken", "&s", &access_token);
52+ g_variant_lookup(info->session_data, "TokenSecret", "&s", &token_secret);
53+ }
54
55 info->watcher->callback(info->watcher,
56 info->account_id,
57 service_name,
58+ error,
59 info->enabled,
60 client_id,
61 client_secret,
62@@ -120,58 +127,38 @@
63 SignonAuthSession *session = (SignonAuthSession *)source;
64 AccountInfo *info = (AccountInfo *)user_data;
65
66- fprintf(stderr, "Authentication for account %u complete\n", info->account_id);
67+ trace("Authentication for account %u complete\n", info->account_id);
68
69 GError *error = NULL;
70 info->session_data = signon_auth_session_process_finish(session, result, &error);
71+ account_info_notify(info, error);
72
73 if (error != NULL) {
74- fprintf(stderr, "Authentication failed: %s\n", error->message);
75+ trace("Authentication failed: %s\n", error->message);
76 g_error_free(error);
77- return;
78 }
79- account_info_notify(info);
80 }
81
82-static void account_info_enabled_cb(
83- AgAccountService *account_service, gboolean enabled, AccountInfo *info) {
84- fprintf(stderr, "account_info_enabled_cb for %u, enabled=%d\n", info->account_id, enabled);
85- if (info->enabled == enabled) {
86- /* no change */
87- return;
88- }
89- info->enabled = enabled;
90-
91+static void account_info_login(AccountInfo *info) {
92 account_info_clear_login(info);
93- if (!enabled) {
94- // Send notification that account has been disabled */
95- account_info_notify(info);
96- return;
97- }
98
99- AgAuthData *auth_data = ag_account_service_get_auth_data(account_service);
100+ AgAuthData *auth_data = ag_account_service_get_auth_data(info->account_service);
101 GError *error = NULL;
102- fprintf(stderr, "Starting authentication session for account %u\n", info->account_id);
103+ trace("Starting authentication session for account %u\n", info->account_id);
104 info->session = signon_auth_session_new(
105 ag_auth_data_get_credentials_id(auth_data),
106 ag_auth_data_get_method(auth_data), &error);
107 if (error != NULL) {
108- fprintf(stderr, "Could not set up auth session: %s\n", error->message);
109+ trace("Could not set up auth session: %s\n", error->message);
110+ account_info_notify(info, error);
111 g_error_free(error);
112 g_object_unref(auth_data);
113 return;
114 }
115
116- GVariantBuilder builder;
117- g_variant_builder_init(&builder, G_VARIANT_TYPE_VARDICT);
118- g_variant_builder_add(
119- &builder, "{sv}",
120- SIGNON_SESSION_DATA_UI_POLICY,
121- g_variant_new_int32(SIGNON_POLICY_NO_USER_INTERACTION));
122-
123 info->auth_params = g_variant_ref_sink(
124 ag_auth_data_get_login_parameters(
125- auth_data, g_variant_builder_end(&builder)));
126+ auth_data, NULL));
127
128 signon_auth_session_process_async(
129 info->session,
130@@ -182,6 +169,24 @@
131 ag_auth_data_unref(auth_data);
132 }
133
134+static void account_info_enabled_cb(
135+ AgAccountService *account_service, gboolean enabled, AccountInfo *info) {
136+ trace("account_info_enabled_cb for %u, enabled=%d\n", info->account_id, enabled);
137+ if (info->enabled == enabled) {
138+ /* no change */
139+ return;
140+ }
141+ info->enabled = enabled;
142+
143+ if (enabled) {
144+ account_info_login(info);
145+ } else {
146+ account_info_clear_login(info);
147+ // Send notification that account has been disabled */
148+ account_info_notify(info, NULL);
149+ }
150+}
151+
152 static AccountInfo *account_info_new(AccountWatcher *watcher, AgAccountService *account_service) {
153 AccountInfo *info = g_new0(AccountInfo, 1);
154 info->watcher = watcher;
155@@ -201,7 +206,7 @@
156
157 static void account_watcher_enabled_event_cb(
158 AgManager *manager, AgAccountId account_id, AccountWatcher *watcher) {
159- fprintf(stderr, "enabled-event for %u\n", account_id);
160+ trace("enabled-event for %u\n", account_id);
161 if (g_hash_table_contains(watcher->services, GUINT_TO_POINTER(account_id))) {
162 /* We are already tracking this account */
163 return;
164@@ -211,20 +216,16 @@
165 /* There was a problem looking up the account */
166 return;
167 }
168+ /* Since our AgManager is restricted to a particular service type,
169+ * pick the first service for the account. */
170 GList *services = ag_account_list_services(account);
171- GList *l;
172- for (l = services; l != NULL; l = l->next) {
173- AgService *service = l->data;
174-
175- const char *name = ag_service_get_name(service);
176- if (g_hash_table_contains(watcher->services_to_watch, name)) {
177- AgAccountService *account_service = ag_account_service_new(
178- account, service);
179- AccountInfo *info = account_info_new(watcher, account_service);
180- g_object_unref(account_service);
181- g_hash_table_insert(watcher->services, GUINT_TO_POINTER(account_id), info);
182- break;
183- }
184+ if (services != NULL) {
185+ AgService *service = services->data;
186+ AgAccountService *account_service = ag_account_service_new(
187+ account, service);
188+ AccountInfo *info = account_info_new(watcher, account_service);
189+ g_object_unref(account_service);
190+ g_hash_table_insert(watcher->services, GUINT_TO_POINTER(account_id), info);
191 }
192 ag_service_list_free(services);
193 g_object_unref(account);
194@@ -232,7 +233,7 @@
195
196 static void account_watcher_account_deleted_cb(
197 AgManager *manager, AgAccountId account_id, AccountWatcher *watcher) {
198- fprintf(stderr, "account-deleted for %u\n", account_id);
199+ trace("account-deleted for %u\n", account_id);
200 /* A disabled event should have been sent prior to this, so no
201 * need to send any notification. */
202 g_hash_table_remove(watcher->services, GUINT_TO_POINTER(account_id));
203@@ -258,16 +259,15 @@
204 }
205 ag_manager_list_free(enabled_accounts);
206
207- return FALSE;
208+ return G_SOURCE_REMOVE;
209 }
210
211-AccountWatcher *account_watcher_new(GHashTable *services_to_watch,
212+AccountWatcher *account_watcher_new(const char *service_type,
213 AccountEnabledCallback callback,
214 void *user_data) {
215 AccountWatcher *watcher = g_new0(AccountWatcher, 1);
216
217- watcher->manager = ag_manager_new();
218- watcher->services_to_watch = g_hash_table_ref(services_to_watch);
219+ watcher->manager = ag_manager_new_for_service_type(service_type);
220 watcher->services = g_hash_table_new_full(
221 g_direct_hash, g_direct_equal, NULL, (GDestroyNotify)account_info_free);
222 watcher->callback = callback;
223@@ -277,3 +277,28 @@
224 g_idle_add(account_watcher_setup, watcher);
225 return watcher;
226 }
227+
228+struct refresh_info {
229+ AccountWatcher *watcher;
230+ AgAccountId account_id;
231+};
232+
233+static gboolean account_watcher_refresh_cb(void *user_data) {
234+ struct refresh_info *data = (struct refresh_info *)user_data;
235+
236+ AccountInfo *info = g_hash_table_lookup(
237+ data->watcher->services, GUINT_TO_POINTER(data->account_id));
238+ if (info != NULL) {
239+ account_info_login(info);
240+ }
241+
242+ return G_SOURCE_REMOVE;
243+}
244+
245+void account_watcher_refresh(AccountWatcher *watcher, unsigned int account_id) {
246+ struct refresh_info *data = g_new(struct refresh_info, 1);
247+ data->watcher = watcher;
248+ data->account_id = account_id;
249+ g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, account_watcher_refresh_cb,
250+ data, g_free);
251+}
252
253=== modified file 'accounts/account-watcher.h'
254--- accounts/account-watcher.h 2014-07-11 12:01:42 +0000
255+++ accounts/account-watcher.h 2014-07-24 08:50:37 +0000
256@@ -24,6 +24,7 @@
257 typedef void (*AccountEnabledCallback)(AccountWatcher *watcher,
258 unsigned int account_id,
259 const char *service_name,
260+ GError *error,
261 int enabled,
262 const char *client_id,
263 const char *client_secret,
264@@ -31,8 +32,10 @@
265 const char *token_secret,
266 void *user_data);
267
268-AccountWatcher *account_watcher_new(GHashTable *services_to_watch,
269+AccountWatcher *account_watcher_new(const char *service_type,
270 AccountEnabledCallback callback,
271 void *user_data);
272
273+void account_watcher_refresh(AccountWatcher *watcher, unsigned int account_id);
274+
275 #endif
276
277=== modified file 'accounts/accounts.c'
278--- accounts/accounts.c 2014-07-11 13:44:14 +0000
279+++ accounts/accounts.c 2014-07-24 08:50:37 +0000
280@@ -1,18 +1,24 @@
281 #include "_cgo_export.h"
282-#include "account-watcher.h"
283
284-AccountWatcher *watch_for_services(void *array_of_service_names, int length) {
285+AccountWatcher *watch_for_service_type(const char *service_type) {
286 /* Transfer service names to hash table */
287- GoString *service_names = (GoString *)array_of_service_names;
288- GHashTable *services_to_watch = g_hash_table_new_full(
289- g_str_hash, g_str_equal, g_free, NULL);
290- int i;
291- for (i = 0; i < length; i++) {
292- g_hash_table_insert(services_to_watch, g_strdup(service_names[i].p), NULL);
293+ if (FALSE) {
294+ /* The Go callback doesn't quite match the
295+ * AccountEnabledCallback function prototype, so we cast the
296+ * argument in the account_watcher_new() call below.
297+ *
298+ * This is just a check to see that the function still has the
299+ * prototype we expect.
300+ */
301+ void (*unused)(void *watcher,
302+ unsigned int account_id, char *service_name,
303+ GError *error, int enabled,
304+ char *client_id, char *client_secret,
305+ char *access_token, char *token_secret,
306+ void *user_data) = authCallback;
307 }
308
309 AccountWatcher *watcher = account_watcher_new(
310- services_to_watch, authCallback, NULL);
311- g_hash_table_unref(services_to_watch);
312+ service_type, (AccountEnabledCallback)authCallback, NULL);
313 return watcher;
314 }
315
316=== modified file 'accounts/accounts.go'
317--- accounts/accounts.go 2014-07-11 12:14:35 +0000
318+++ accounts/accounts.go 2014-07-24 08:50:37 +0000
319@@ -21,17 +21,26 @@
320 #cgo pkg-config: glib-2.0 libaccounts-glib libsignon-glib
321 #include <stdlib.h>
322 #include <glib.h>
323-
324-typedef struct _AccountWatcher AccountWatcher;
325-
326-AccountWatcher *watch_for_services(void *array_of_service_names, int length);
327+#include "account-watcher.h"
328+
329+AccountWatcher *watch_for_service_type(const char *service_type);
330 */
331 import "C"
332-import "unsafe"
333+import (
334+ "errors"
335+ "sync"
336+ "unsafe"
337+)
338+
339+type Watcher struct {
340+ C <-chan AuthData
341+ watcher *C.AccountWatcher
342+}
343
344 type AuthData struct {
345 AccountId uint
346 ServiceName string
347+ Error error
348 Enabled bool
349
350 ClientId string
351@@ -41,28 +50,49 @@
352 }
353
354 var (
355- mainLoop *C.GMainLoop
356+ mainLoopOnce sync.Once
357 authChannels = make(map[*C.AccountWatcher]chan<- AuthData)
358+ authChannelsLock sync.Mutex
359 )
360
361-func WatchForService(serviceNames... string) <-chan AuthData {
362- if mainLoop == nil {
363- mainLoop = C.g_main_loop_new(nil, C.gboolean(1))
364+func startMainLoop() {
365+ mainLoopOnce.Do(func() {
366+ mainLoop := C.g_main_loop_new(nil, C.gboolean(1))
367 go C.g_main_loop_run(mainLoop)
368- }
369+ })
370+}
371
372- watcher := C.watch_for_services(unsafe.Pointer(&serviceNames[0]), C.int(len(serviceNames)))
373+// NewWatcher creates a new account watcher for the given service names
374+func NewWatcher(serviceType string) *Watcher {
375+ w := new(Watcher)
376+ cServiceType := C.CString(serviceType)
377+ defer C.free(unsafe.Pointer(cServiceType))
378+ w.watcher = C.watch_for_service_type(cServiceType)
379
380 ch := make(chan AuthData)
381- authChannels[watcher] = ch
382- return ch
383+ w.C = ch
384+ authChannelsLock.Lock()
385+ authChannels[w.watcher] = ch
386+ authChannelsLock.Unlock()
387+
388+ startMainLoop()
389+
390+ return w
391+}
392+
393+// Refresh requests that the token for the given account be refreshed.
394+// The new access token will be delivered over the watcher's channel.
395+func (w *Watcher) Refresh(accountId uint) {
396+ C.account_watcher_refresh(w.watcher, C.uint(accountId))
397 }
398
399 //export authCallback
400-func authCallback(watcher unsafe.Pointer, accountId C.uint, serviceName *C.char, enabled C.int, clientId, clientSecret, accessToken, tokenSecret *C.char, userData unsafe.Pointer) {
401+func authCallback(watcher unsafe.Pointer, accountId C.uint, serviceName *C.char, error *C.GError, enabled C.int, clientId, clientSecret, accessToken, tokenSecret *C.char, userData unsafe.Pointer) {
402 // Ideally the first argument would be of type
403 // *C.AccountWatcher, but that fails with Go 1.2.
404+ authChannelsLock.Lock()
405 ch := authChannels[(*C.AccountWatcher)(watcher)]
406+ authChannelsLock.Unlock()
407 if ch == nil {
408 // Log the error
409 return
410@@ -71,6 +101,9 @@
411 var data AuthData
412 data.AccountId = uint(accountId)
413 data.ServiceName = C.GoString(serviceName)
414+ if error != nil {
415+ data.Error = errors.New(C.GoString((*C.char)(error.message)))
416+ }
417 if enabled != 0 {
418 data.Enabled = true
419 }
420
421=== modified file 'cmd/account-polld/account_manager.go'
422--- cmd/account-polld/account_manager.go 2014-07-17 18:35:36 +0000
423+++ cmd/account-polld/account_manager.go 2014-07-24 08:50:37 +0000
424@@ -19,7 +19,6 @@
425
426 import (
427 "log"
428- "sync"
429 "time"
430
431 "launchpad.net/account-polld/accounts"
432@@ -27,59 +26,74 @@
433 )
434
435 type AccountManager struct {
436+ watcher *accounts.Watcher
437 authData accounts.AuthData
438- authMutex *sync.Mutex
439 plugin plugins.Plugin
440 interval time.Duration
441 postWatch chan *PostWatch
442- terminate chan bool
443+ authChan chan accounts.AuthData
444 }
445
446 const DEFAULT_INTERVAL = time.Duration(60 * time.Second)
447
448-func NewAccountManager(authData accounts.AuthData, postWatch chan *PostWatch, plugin plugins.Plugin) *AccountManager {
449+func NewAccountManager(watcher *accounts.Watcher, postWatch chan *PostWatch, plugin plugins.Plugin) *AccountManager {
450 return &AccountManager{
451+ watcher: watcher,
452 plugin: plugin,
453- authData: authData,
454- authMutex: &sync.Mutex{},
455+ interval: DEFAULT_INTERVAL,
456 postWatch: postWatch,
457- interval: DEFAULT_INTERVAL,
458- terminate: make(chan bool),
459+ authChan: make(chan accounts.AuthData, 1),
460 }
461 }
462
463 func (a *AccountManager) Delete() {
464- a.terminate <- true
465+ close(a.authChan)
466 }
467
468 func (a *AccountManager) Loop() {
469- defer close(a.terminate)
470+ var ok bool
471+ if a.authData, ok = <- a.authChan; !ok {
472+ return
473+ }
474 L:
475 for {
476 log.Println("Polling set to", a.interval, "for", a.authData.AccountId)
477 select {
478 case <-time.After(a.interval):
479 a.poll()
480- case <-a.terminate:
481- break L
482+ case a.authData, ok = <-a.authChan:
483+ if !ok {
484+ break L
485+ }
486+ a.poll()
487 }
488 }
489 log.Printf("Ending poll loop for account %d", a.authData.AccountId)
490 }
491
492 func (a *AccountManager) poll() {
493- a.authMutex.Lock()
494- defer a.authMutex.Unlock()
495-
496 if !a.authData.Enabled {
497 log.Println("Account", a.authData.AccountId, "no longer enabled")
498 return
499 }
500
501+ if a.authData.Error != nil {
502+ log.Println("Account", a.authData.AccountId, "failed to authenticate:", a.authData.Error)
503+ return
504+ }
505+
506 if n, err := a.plugin.Poll(&a.authData); err != nil {
507 log.Print("Error while polling ", a.authData.AccountId, ": ", err)
508 // penalizing the next poll
509 a.interval += DEFAULT_INTERVAL
510+
511+ // If the error indicates that the authentication
512+ // token has expired, request reauthentication and
513+ // mark data as disabled.
514+ if err == plugins.ErrTokenExpired {
515+ a.watcher.Refresh(a.authData.AccountId)
516+ a.authData.Enabled = false
517+ }
518 } else if len(n) > 0 {
519 // on success we reset the timeout to the default interval
520 a.interval = DEFAULT_INTERVAL
521@@ -88,7 +102,5 @@
522 }
523
524 func (a *AccountManager) updateAuthData(authData accounts.AuthData) {
525- a.authMutex.Lock()
526- defer a.authMutex.Unlock()
527- a.authData = authData
528+ a.authChan <- authData
529 }
530
531=== modified file 'cmd/account-polld/main.go'
532--- cmd/account-polld/main.go 2014-07-18 11:42:52 +0000
533+++ cmd/account-polld/main.go 2014-07-24 08:50:37 +0000
534@@ -38,9 +38,11 @@
535 }
536
537 const (
538+ SERVICETYPE_POLL = "account-polld"
539+
540 SERVICENAME_GMAIL = "google-gmail-poll"
541- SERVICENAME_TWITTER = "twitter-microblog"
542- SERVICENAME_FACEBOOK = "facebook-microblog"
543+ SERVICENAME_TWITTER = "twitter-poll"
544+ SERVICENAME_FACEBOOK = "facebook-poll"
545 )
546
547 const (
548@@ -69,9 +71,10 @@
549 }
550
551 func monitorAccounts(postWatch chan *PostWatch) {
552+ watcher := accounts.NewWatcher(SERVICETYPE_POLL)
553 mgr := make(map[uint]*AccountManager)
554 L:
555- for data := range accounts.WatchForService(SERVICENAME_GMAIL, SERVICENAME_FACEBOOK, SERVICENAME_TWITTER) {
556+ for data := range watcher.C {
557 if account, ok := mgr[data.AccountId]; ok {
558 if data.Enabled {
559 log.Printf("New account data for %d - was %#v, now is %#v", data.AccountId, account.authData, data)
560@@ -98,7 +101,8 @@
561 log.Println("Unhandled account with id", data.AccountId, "for", data.ServiceName)
562 continue L
563 }
564- mgr[data.AccountId] = NewAccountManager(data, postWatch, plugin)
565+ mgr[data.AccountId] = NewAccountManager(watcher, postWatch, plugin)
566+ mgr[data.AccountId].updateAuthData(data)
567 go mgr[data.AccountId].Loop()
568 }
569 }
570
571=== modified file 'cmd/account-watcher-test/main.go'
572--- cmd/account-watcher-test/main.go 2014-07-11 12:07:47 +0000
573+++ cmd/account-watcher-test/main.go 2014-07-24 08:50:37 +0000
574@@ -9,7 +9,11 @@
575
576 func main() {
577 // Expects a list of service names as command line arguments
578- for data := range accounts.WatchForService(os.Args[1:]...) {
579- fmt.Printf("%#v\n", data)
580+ for data := range accounts.NewWatcher(os.Args[1]).C {
581+ if data.Error != nil {
582+ fmt.Println("Failed to authenticate account", data.AccountId, ":", data.Error)
583+ } else {
584+ fmt.Printf("%#v\n", data)
585+ }
586 }
587 }
588
589=== modified file 'data/account-polld.application'
590--- data/account-polld.application 2014-07-17 19:52:56 +0000
591+++ data/account-polld.application 2014-07-24 08:50:37 +0000
592@@ -4,15 +4,9 @@
593 <desktop-entry>account-polld.desktop</desktop-entry>
594 <translations>account-polld</translations>
595
596- <services>
597- <service id="google-gmail-poll">
598- <description>Get notifications for your email</description>
599- </service>
600- <service id="facebook-microblog">
601- <description>Get notifications for facebook</description>
602- </service>
603- <service id="twitter-microblog">
604- <description>Get notifications for twitter</description>
605- </service>
606- </services>
607+ <service-types>
608+ <service-type id="account-polld">
609+ <description>Receive notifications for this account</description>
610+ </service-type>
611+ </service-types>
612 </application>
613
614=== added file 'data/account-polld.service-type'
615--- data/account-polld.service-type 1970-01-01 00:00:00 +0000
616+++ data/account-polld.service-type 2014-07-24 08:50:37 +0000
617@@ -0,0 +1,7 @@
618+<?xml version="1.0" encoding="UTF-8"?>
619+<service-type id="account-polld">
620+ <name>Account Polling</name>
621+ <description>Integrate with account-polld</description>
622+ <icon>intouch</icon>
623+ <translations>account-polld</translations>
624+</service-type>
625
626=== added file 'data/facebook-poll.service'
627--- data/facebook-poll.service 1970-01-01 00:00:00 +0000
628+++ data/facebook-poll.service 2014-07-24 08:50:37 +0000
629@@ -0,0 +1,8 @@
630+<?xml version="1.0" encoding="UTF-8"?>
631+<service id="facebook-poll">
632+ <type>account-polld</type>
633+ <name>Facebook</name>
634+ <icon>facebook</icon>
635+ <provider>facebook</provider>
636+ <translations>account-polld</translations>
637+</service>
638
639=== modified file 'data/google-gmail-poll.service'
640--- data/google-gmail-poll.service 2014-07-17 19:52:56 +0000
641+++ data/google-gmail-poll.service 2014-07-24 08:50:37 +0000
642@@ -1,6 +1,6 @@
643 <?xml version="1.0" encoding="UTF-8"?>
644 <service id="google-gmail-poll">
645- <type>mail</type>
646+ <type>account-polld</type>
647 <name>GMail</name>
648 <provider>google</provider>
649 <icon>google</icon>
650
651=== added file 'data/twitter-poll.service'
652--- data/twitter-poll.service 1970-01-01 00:00:00 +0000
653+++ data/twitter-poll.service 2014-07-24 08:50:37 +0000
654@@ -0,0 +1,8 @@
655+<?xml version="1.0" encoding="UTF-8"?>
656+<service id="twitter-poll">
657+ <type>account-polld</type>
658+ <name>Twitter</name>
659+ <icon>twitter</icon>
660+ <provider>twitter</provider>
661+ <translations>account-polld</translations>
662+</service>
663
664=== modified file 'debian/rules'
665--- debian/rules 2014-07-23 18:31:38 +0000
666+++ debian/rules 2014-07-24 08:50:37 +0000
667@@ -30,11 +30,14 @@
668 mkdir -p \
669 ${CURDIR}/debian/account-polld/usr/share/accounts/application \
670 ${CURDIR}/debian/account-polld/usr/share/accounts/services \
671+ ${CURDIR}/debian/account-polld/usr/share/accounts/service_types \
672 ${CURDIR}/debian/account-polld/usr/share/applications
673 cp ${CURDIR}/data/account-polld.application \
674 ${CURDIR}/debian/account-polld/usr/share/accounts/application/
675- cp ${CURDIR}/data/google-gmail-poll.service \
676+ cp ${CURDIR}/data/*.service \
677 ${CURDIR}/debian/account-polld/usr/share/accounts/services/
678+ cp ${CURDIR}/data/account-polld.service-type \
679+ ${CURDIR}/debian/account-polld/usr/share/accounts/service_types/
680 cp ${CURDIR}/data/account-polld.desktop \
681 ${CURDIR}/debian/account-polld/usr/share/applications/
682
683
684=== modified file 'plugins/facebook/facebook.go'
685--- plugins/facebook/facebook.go 2014-07-17 18:35:36 +0000
686+++ plugins/facebook/facebook.go 2014-07-24 08:50:37 +0000
687@@ -62,6 +62,9 @@
688 if err := decoder.Decode(&result); err != nil {
689 return nil, err
690 }
691+ if result.Error.Code == 190 {
692+ return nil, plugins.ErrTokenExpired
693+ }
694 return nil, &result.Error
695 }
696
697
698=== modified file 'plugins/facebook/facebook_test.go'
699--- plugins/facebook/facebook_test.go 2014-07-18 11:38:36 +0000
700+++ plugins/facebook/facebook_test.go 2014-07-24 08:50:37 +0000
701@@ -23,6 +23,8 @@
702 "testing"
703
704 . "launchpad.net/gocheck"
705+
706+ "launchpad.net/account-polld/plugins"
707 )
708
709 type S struct{}
710@@ -48,10 +50,18 @@
711 errorBody = `
712 {
713 "error": {
714- "message": "Message describing the error",
715+ "message": "Unknown path components: /xyz",
716+ "type": "OAuthException",
717+ "code": 2500
718+ }
719+}`
720+ tokenExpiredErrorBody = `
721+{
722+ "error": {
723+ "message": "Error validating access token: Session has expired",
724 "type": "OAuthException",
725 "code": 190 ,
726- "error_subcode": 460
727+ "error_subcode": 463
728 }
729 }`
730 notificationsBody = `
731@@ -151,7 +161,17 @@
732 c.Check(notifications, IsNil)
733 c.Assert(err, Not(IsNil))
734 graphErr := err.(*GraphError)
735- c.Check(graphErr.Message, Equals, "Message describing the error")
736- c.Check(graphErr.Code, Equals, 190)
737- c.Check(graphErr.Subcode, Equals, 460)
738+ c.Check(graphErr.Message, Equals, "Unknown path components: /xyz")
739+ c.Check(graphErr.Code, Equals, 2500)
740+}
741+
742+func (s S) TestTokenExpiredErrorResponse(c *C) {
743+ resp := &http.Response{
744+ StatusCode: http.StatusBadRequest,
745+ Body: closeWrapper{bytes.NewReader([]byte(tokenExpiredErrorBody))},
746+ }
747+ p := &fbPlugin{}
748+ notifications, err := p.parseResponse(resp)
749+ c.Check(notifications, IsNil)
750+ c.Assert(err, Equals, plugins.ErrTokenExpired)
751 }
752
753=== modified file 'plugins/plugins.go'
754--- plugins/plugins.go 2014-07-17 18:35:36 +0000
755+++ plugins/plugins.go 2014-07-24 08:50:37 +0000
756@@ -17,7 +17,11 @@
757
758 package plugins
759
760-import "launchpad.net/account-polld/accounts"
761+import (
762+ "errors"
763+
764+ "launchpad.net/account-polld/accounts"
765+)
766
767 // Plugin is an interface which the plugins will adhere to for the poll
768 // daemon to interact with.
769@@ -122,3 +126,7 @@
770 PLUGIN_EMAIL = 0
771 PLUGIN_SOCIAL
772 )
773+
774+// ErrTokenExpired is the error returned by a plugin to indicate that
775+// the web service reported that the authentication token has expired.
776+var ErrTokenExpired = errors.New("Token expired")
777
778=== modified file 'plugins/twitter/twitter.go'
779--- plugins/twitter/twitter.go 2014-07-23 10:35:13 +0000
780+++ plugins/twitter/twitter.go 2014-07-24 08:50:37 +0000
781@@ -77,6 +77,12 @@
782 if err := decoder.Decode(&result); err != nil {
783 return nil, err
784 }
785+ // Error code 89 is used for invalid or expired tokens.
786+ for _, e := range result.Errors {
787+ if e.Code == 89 {
788+ return nil, plugins.ErrTokenExpired
789+ }
790+ }
791 return nil, &result
792 }
793
794@@ -113,6 +119,12 @@
795 if err := decoder.Decode(&result); err != nil {
796 return nil, err
797 }
798+ // Error code 89 is used for invalid or expired tokens.
799+ for _, e := range result.Errors {
800+ if e.Code == 89 {
801+ return nil, plugins.ErrTokenExpired
802+ }
803+ }
804 return nil, &result
805 }
806
807
808=== modified file 'plugins/twitter/twitter_test.go'
809--- plugins/twitter/twitter_test.go 2014-07-18 11:37:04 +0000
810+++ plugins/twitter/twitter_test.go 2014-07-24 08:50:37 +0000
811@@ -23,6 +23,8 @@
812 "testing"
813
814 . "launchpad.net/gocheck"
815+
816+ "launchpad.net/account-polld/plugins"
817 )
818
819 type S struct{}
820@@ -54,6 +56,15 @@
821 }
822 ]
823 }`
824+ tokenExpiredErrorBody = `
825+{
826+ "errors": [
827+ {
828+ "message":"Invalid or expired token",
829+ "code":89
830+ }
831+ ]
832+}`
833 statusesBody = `
834 [
835 {
836@@ -425,6 +436,17 @@
837 c.Check(twErr.Errors[0].Code, Equals, 34)
838 }
839
840+func (s S) TestParseStatusesTokenExpiredError(c *C) {
841+ resp := &http.Response{
842+ StatusCode: http.StatusBadRequest,
843+ Body: closeWrapper{bytes.NewReader([]byte(tokenExpiredErrorBody))},
844+ }
845+ p := &twitterPlugin{}
846+ messages, err := p.parseStatuses(resp)
847+ c.Check(messages, IsNil)
848+ c.Assert(err, Equals, plugins.ErrTokenExpired)
849+}
850+
851 func (s S) TestParseDirectMessages(c *C) {
852 resp := &http.Response{
853 StatusCode: http.StatusOK,
854@@ -453,3 +475,14 @@
855 c.Check(twErr.Errors[0].Message, Equals, "Sorry, that page does not exist")
856 c.Check(twErr.Errors[0].Code, Equals, 34)
857 }
858+
859+func (s S) TestParseDirectMessagesTokenExpiredError(c *C) {
860+ resp := &http.Response{
861+ StatusCode: http.StatusBadRequest,
862+ Body: closeWrapper{bytes.NewReader([]byte(tokenExpiredErrorBody))},
863+ }
864+ p := &twitterPlugin{}
865+ messages, err := p.parseDirectMessages(resp)
866+ c.Check(messages, IsNil)
867+ c.Assert(err, Equals, plugins.ErrTokenExpired)
868+}

Subscribers

People subscribed via source and target branches