[Merge] lp:~jamesh/account-polld/facebook-plugin into lp:account-polld
Sergio Schvezov
sergio.schvezov at canonical.com
Wed Jul 16 20:51:07 UTC 2014
Review: Needs Fixing
I've added a couple of inline minor comments.
wrt
I'll ping alex about the extra permission (I have a similar problem for gmail) and add you to an email I sent him (he promised to look into it tomorrow morning)
If you don't mind to use the "still in" bzr gocheck, feel free to do so; it's packaged as golang-gocheck-dev and it's import path is "launchpad.net/gocheck". If you have reasons to believe the github one has things that are direly needed we should start packaging it for ubuntu now (I haven't since I didn't find an urgency for it).
I have an MP with updates to the notifications API with descriptions of what everything is; just take the "Actions" one with a grain of salt as it's subject to change on the Post side.
Diff comments:
> === modified file 'cmd/account-polld/account_manager.go'
> --- cmd/account-polld/account_manager.go 2014-07-15 02:05:10 +0000
> +++ cmd/account-polld/account_manager.go 2014-07-16 08:28:03 +0000
> @@ -59,7 +59,7 @@
> // penalizing the next poll
> a.interval += DEFAULT_INTERVAL
> continue
> - } else if n != nil {
> + } else if len(n) > 0 {
> // on success we reset the timeout to the default interval
> a.interval = DEFAULT_INTERVAL
> postWatch <- &PostWatch{notifications: n, appId: a.plugin.ApplicationId()}
>
> === modified file 'cmd/account-polld/main.go'
> --- cmd/account-polld/main.go 2014-07-15 01:57:12 +0000
> +++ cmd/account-polld/main.go 2014-07-16 08:28:03 +0000
> @@ -24,13 +24,14 @@
>
> "launchpad.net/account-polld/accounts"
> "launchpad.net/account-polld/plugins"
> + "launchpad.net/account-polld/plugins/facebook"
> "launchpad.net/account-polld/plugins/gmail"
> "launchpad.net/go-dbus/v1"
> )
>
> type PostWatch struct {
> appId plugins.ApplicationId
> - notifications *[]plugins.Notification
> + notifications []plugins.Notification
> }
>
> const (
> @@ -78,8 +79,8 @@
> plugin = gmail.New()
> case SERVICENAME_FACEBOOK:
> // This is just stubbed until the plugin exists.
> - log.Println("Unhandled account with id", data.AccountId, "for", data.ServiceName)
> - continue L
> + log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
> + plugin = facebook.New()
> case SERVICENAME_TWITTER:
> // This is just stubbed until the plugin exists.
> log.Println("Unhandled account with id", data.AccountId, "for", data.ServiceName)
> @@ -96,7 +97,7 @@
>
> func postOffice(bus *dbus.Connection, postWatch chan *PostWatch) {
> for post := range postWatch {
> - for _, n := range *post.notifications {
> + for _, n := range post.notifications {
> fmt.Println("Should be dispatching", n, "to the post office using", bus.UniqueName, "for", post.appId)
> }
> }
>
> === added directory 'plugins/facebook'
> === added file 'plugins/facebook/facebook.go'
> --- plugins/facebook/facebook.go 1970-01-01 00:00:00 +0000
> +++ plugins/facebook/facebook.go 2014-07-16 08:28:03 +0000
> @@ -0,0 +1,144 @@
> +/*
> + Copyright 2014 Canonical Ltd.
> + Authors: James Henstridge <james.henstridge at canonical.com>
> +
> + 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/>.
> +*/
> +
> +package facebook
> +
> +import (
> + "encoding/json"
> + "net/http"
> + "net/url"
> +
> + "launchpad.net/account-polld/accounts"
> + "launchpad.net/account-polld/plugins"
> +)
> +
> +var baseUrl, _ = url.Parse("https://graph.facebook.com/v2.0/")
> +
> +type fbPlugin struct {
> + lastUpdate string
> +}
> +
> +func New() plugins.Plugin {
> + return &fbPlugin{
> + lastUpdate: "",
this could be omitted, right? It initializes to an empty string anyways
> + }
> +}
> +
> +func (p *fbPlugin) ApplicationId() plugins.ApplicationId {
> + return "com.ubuntu.developer.webapps.webapp-facebook"
to be a proper application id this is missing a "_webapp-facebook" appended at the end.
> +}
> +
> +func (p *fbPlugin) request(authData *accounts.AuthData, path string) (*http.Response, error) {
> + // Resolve path relative to Graph API base URL, and add access token
> + u, err := baseUrl.Parse(path)
> + if err != nil {
> + return nil, err
> + }
> + query := u.Query()
> + query.Add("access_token", authData.AccessToken)
> + u.RawQuery = query.Encode()
> +
> + return http.Get(u.String())
> +}
> +
> +func (p *fbPlugin) parseResponse(resp *http.Response) ([]plugins.Notification, error) {
> + defer resp.Body.Close()
> + decoder := json.NewDecoder(resp.Body)
> +
> + if resp.StatusCode != http.StatusOK {
> + var result errorDoc
> + if err := decoder.Decode(&result); err != nil {
> + return nil, err
> + }
> + return nil, &result.Error
> + }
> +
> + var result notificationDoc
> + if err := decoder.Decode(&result); err != nil {
> + return nil, err
> + }
> + notifications := []plugins.Notification{}
> + latestUpdate := ""
> + for _, n := range result.Data {
> + if n.UpdatedTime <= p.lastUpdate {
> + continue
> + }
> + notifications = append(notifications, plugins.Notification{
> + Card: plugins.Card{
> + Summary: n.Title,
> + },
> + })
> + if n.UpdatedTime > latestUpdate {
> + latestUpdate = n.UpdatedTime
> + }
> + }
> + p.lastUpdate = latestUpdate
> + return notifications, nil
> +}
> +
> +func (p *fbPlugin) Poll(authData *accounts.AuthData) ([]plugins.Notification, error) {
> + resp, err := p.request(authData, "me/notifications")
> + if err != nil {
> + return nil, err
> + }
> + return p.parseResponse(resp)
> +}
> +
> +// The notifications response format is described here:
> +// https://developers.facebook.com/docs/graph-api/reference/v2.0/user/notifications/
> +type notificationDoc struct {
> + Data []notification `json:"data"`
> + Paging struct {
> + Previous string `json:"previous"`
> + Next string `json:"next"`
> + } `json:"paging"`
> +}
> +
> +type notification struct {
> + Id string `json:"id"`
> + From object `json:"from"`
> + To object `json:"to"`
> + CreatedTime string `json:"created_time"`
> + UpdatedTime string `json:"updated_time"`
> + Title string `json:"title"`
> + Link string `json:"link"`
> + Application object `json:"application"`
> + Unread int `json:"unread"`
> + Object object `json:"object"`
> +}
> +
> +type object struct {
> + Id string `json:"id"`
> + Name string `json:"name"`
> +}
> +
> +// The error response format is described here:
> +// https://developers.facebook.com/docs/graph-api/using-graph-api/v2.0#errors
> +type errorDoc struct {
> + Error GraphError `json:"error"`
> +}
> +
> +type GraphError struct {
> + Message string `json:"message"`
> + Type string `json:"type"`
> + Code int `json:"code"`
> + Subcode int `json:"error_subcode"`
> +}
> +
> +func (err *GraphError) Error() string {
> + return err.Message
> +}
>
> === added file 'plugins/facebook/facebook_test.go'
> --- plugins/facebook/facebook_test.go 1970-01-01 00:00:00 +0000
> +++ plugins/facebook/facebook_test.go 2014-07-16 08:28:03 +0000
> @@ -0,0 +1,167 @@
> +/*
> + Copyright 2014 Canonical Ltd.
> + Authors: James Henstridge <james.henstridge at canonical.com>
> +
> + 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/>.
> +*/
> +package facebook
> +
> +import (
> + "bytes"
> + "io"
> + "net/http"
> + "testing"
> +)
> +
> +// closeWraper adds a dummy Close() method to a reader
> +type closeWrapper struct {
> + io.Reader
> +}
> +
> +func (r closeWrapper) Close() error {
> + return nil
> +}
> +
> +const (
> + errorBody = `
> +{
> + "error": {
> + "message": "Message describing the error",
> + "type": "OAuthException",
> + "code": 190 ,
> + "error_subcode": 460
> + }
> +}`
> + notificationsBody = `
> +{
> + "data": [
> + {
> + "id": "notif_id",
> + "from": {
> + "id": "sender_id",
> + "name": "Sender"
> + },
> + "to": {
> + "id": "recipient_id",
> + "name": "Recipient"
> + },
> + "created_time": "2014-07-12T09:51:57+0000",
> + "updated_time": "2014-07-12T09:51:57+0000",
> + "title": "Sender posted on your timeline: \"The message...\"",
> + "link": "http://www.facebook.com/recipient/posts/id",
> + "application": {
> + "name": "Wall",
> + "namespace": "wall",
> + "id": "2719290516"
> + },
> + "unread": 1
> + },
> + {
> + "id": "notif_1105650586_80600069",
> + "from": {
> + "id": "sender2_id",
> + "name": "Sender2"
> + },
> + "to": {
> + "id": "recipient_id",
> + "name": "Recipient"
> + },
> + "created_time": "2014-07-08T06:17:52+0000",
> + "updated_time": "2014-07-08T06:17:52+0000",
> + "title": "Sender2's birthday was on July 7.",
> + "link": "http://www.facebook.com/profile.php?id=xxx&ref=brem",
> + "application": {
> + "name": "Gifts",
> + "namespace": "superkarma",
> + "id": "329122197162272"
> + },
> + "unread": 1,
> + "object": {
> + "id": "sender2_id",
> + "name": "Sender2"
> + }
> + }
> + ],
> + "paging": {
> + "previous": "https://graph.facebook.com/v2.0/recipient/notifications?limit=5000&since=1405158717&__paging_token=enc_AewDzwIQmWOwPNO-36GaZsaJAog8l93HQ7uLEO-gp1Tb6KCiolXfzMCcGY2KjrJJsDJXdDmNJObICr5dewfMZgGs",
> + "next": "https://graph.facebook.com/v2.0/recipient/notifications?limit=5000&until=1404705077&__paging_token=enc_Aewlhut5DQyhqtLNr7pLCMlYU012t4XY7FOt7cooz4wsWIWi-Jqz0a0IDnciJoeLu2vNNQkbtOpCmEmsVsN4hkM4"
> + },
> + "summary": [
> + ]
> +}
> +`
> +)
> +
> +func TestParseNotifications(t *testing.T) {
> + resp := &http.Response{
> + StatusCode: http.StatusOK,
> + Body: closeWrapper{bytes.NewReader([]byte(notificationsBody))},
> + }
> + p := &fbPlugin{}
> + notifications, err := p.parseResponse(resp)
> + if err != nil {
> + t.Fatal("Unexpected error:", err)
> + }
> + if len(notifications) != 2 {
> + t.Fatal("Expected 2 notifications, got ", len(notifications))
> + }
> + if notifications[0].Card.Summary != "Sender posted on your timeline: \"The message...\"" {
> + t.Error("Bad summary for first notification:", notifications[0].Card.Summary)
> + }
> + if notifications[1].Card.Summary != "Sender2's birthday was on July 7." {
> + t.Error("Bad summary for second notification:", notifications[0].Card.Summary)
> + }
> + if p.lastUpdate != "2014-07-12T09:51:57+0000" {
> + t.Error("Unexpected last update time:", p.lastUpdate)
> + }
> +}
> +
> +func TestIgnoreOldNotifications(t *testing.T) {
> + resp := &http.Response{
> + StatusCode: http.StatusOK,
> + Body: closeWrapper{bytes.NewReader([]byte(notificationsBody))},
> + }
> + p := &fbPlugin{lastUpdate: "2014-07-08T06:17:52+0000"}
> + notifications, err := p.parseResponse(resp)
> + if err != nil {
> + t.Fatal("Unexpected error:", err)
> + }
> + if len(notifications) != 1 {
> + t.Fatal("Expected 1 notification, got ", len(notifications))
> + }
> + if notifications[0].Card.Summary != "Sender posted on your timeline: \"The message...\"" {
> + t.Error("Bad summary for first notification:", notifications[0].Card.Summary)
> + }
> + if p.lastUpdate != "2014-07-12T09:51:57+0000" {
> + t.Error("Unexpected last update time:", p.lastUpdate)
> + }
> +}
> +
> +func TestErrorResponse(t *testing.T) {
> + resp := &http.Response{
> + StatusCode: http.StatusBadRequest,
> + Body: closeWrapper{bytes.NewReader([]byte(errorBody))},
> + }
> + p := &fbPlugin{}
> + notifications, err := p.parseResponse(resp)
> + if err == nil {
> + t.Fatal("Expected parseResponse to return an error.")
> + }
> + if notifications != nil {
> + t.Error("Expected notifications to be nil on error.")
> + }
> + graphErr := err.(*GraphError)
> + if graphErr.Message != "Message describing the error" {
> + t.Errorf("Unexpected error message: '%s'", graphErr.Message)
> + }
> +}
>
> === modified file 'plugins/gmail/plugin.go'
> --- plugins/gmail/plugin.go 2014-07-11 20:23:49 +0000
> +++ plugins/gmail/plugin.go 2014-07-16 08:28:03 +0000
> @@ -31,7 +31,7 @@
> return &GmailPlugin{}
> }
>
> -func (p *GmailPlugin) Poll(authData *accounts.AuthData) (*[]plugins.Notification, error) {
> +func (p *GmailPlugin) Poll(authData *accounts.AuthData) ([]plugins.Notification, error) {
> return nil, nil
> }
>
>
> === modified file 'plugins/plugins.go'
> --- plugins/plugins.go 2014-07-11 20:23:49 +0000
> +++ plugins/plugins.go 2014-07-16 08:28:03 +0000
> @@ -31,7 +31,7 @@
> // ApplicationId returns the APP_ID of the delivery target for Post Office.
> type Plugin interface {
> ApplicationId() ApplicationId
> - Poll(*accounts.AuthData) (*[]Notification, error)
> + Poll(*accounts.AuthData) ([]Notification, error)
> }
>
> // AuthTokens is a map with tokens the plugins are to use to make requests.
>
--
https://code.launchpad.net/~jamesh/account-polld/facebook-plugin/+merge/226966
Your team Ubuntu Phablet Team is subscribed to branch lp:account-polld.
More information about the Ubuntu-reviews
mailing list