[Merge] lp:~renatofilho/account-polld/gcalendar-plugin into lp:account-polld

Jonas G. Drange jonas.drange at canonical.com
Thu Apr 14 14:49:26 UTC 2016


Review: Needs Information

Looks good! A couple of comments on the logging. There's a lot of logging already in account-pold, so I just want to double check that we need all the added log statements.

Thanks!

Diff comments:

> === modified file 'cmd/account-polld/main.go'
> --- cmd/account-polld/main.go	2015-12-19 19:54:01 +0000
> +++ cmd/account-polld/main.go	2016-04-13 13:33:08 +0000
> @@ -123,10 +128,14 @@
>  				}
>  			} else if data.Enabled {
>  				var plugin plugins.Plugin
> +				log.Println("Creat plugin for service: ", data.ServiceName)

Typo in “Creat”

>  				switch data.ServiceName {
>  				case SERVICENAME_GMAIL:
>  					log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
>  					plugin = gmail.New(data.AccountId)
> +				case SERVICENAME_GCALENDAR:
> +					log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
> +					plugin = gcalendar.New(data.AccountId)
>  				case SERVICENAME_TWITTER:
>  					// This is just stubbed until the plugin exists.
>  					log.Println("Creating account with id", data.AccountId, "for", data.ServiceName)
> 
> === added file 'plugins/gcalendar/gcalendar.go'
> --- plugins/gcalendar/gcalendar.go	1970-01-01 00:00:00 +0000
> +++ plugins/gcalendar/gcalendar.go	2016-04-13 13:33:08 +0000
> @@ -0,0 +1,146 @@
> +/*
> + Copyright 2016 Canonical Ltd.
> +
> + 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 gcalendar
> +
> +import (
> +	"encoding/json"
> +	"net/http"
> +	"net/url"
> +	"os"
> +	"log"
> +
> +	"launchpad.net/account-polld/accounts"
> +	"launchpad.net/account-polld/plugins"
> +)
> +
> +const (
> +	APP_ID      = "com.ubuntu.calendar_calendar"
> +	pluginName  = "gcalendar"
> +)
> +
> +var baseUrl, _ = url.Parse("https://www.googleapis.com/calendar/v3/calendars/primary/")
> +
> +
> +type GCalendarPlugin struct {
> +	accountId   uint
> +}
> +
> +func New(accountId uint) *GCalendarPlugin {
> +	return &GCalendarPlugin{accountId: accountId}
> +}
> +
> +func (p *GCalendarPlugin) ApplicationId() plugins.ApplicationId {
> +	return plugins.ApplicationId(APP_ID)
> +}
> +
> +func (p *GCalendarPlugin) Poll(authData *accounts.AuthData) ([]*plugins.PushMessageBatch, error) {
> +	// This envvar check is to ease testing.
> +	if token := os.Getenv("ACCOUNT_POLLD_TOKEN_GCALENDAR"); token != "" {
> +		authData.AccessToken = token
> +	}
> +
> +	syncMonitor := NewSyncMonitor()
> +	if syncMonitor == nil {
> +		log.Print("Sync monitor not available yet.")
> +		return nil, nil
> +	}
> +
> +	lastSyncDate, err := syncMonitor.LastSyncDate(p.accountId, "calendar")
> +	if err != nil {
> +		log.Print("calendar plugin ", p.accountId, ": cannot load previous sync date: ", err, ". Try next time.")
> +		return nil, nil
> +	} else {
> +		log.Print("calendar plugin ", p.accountId, ": last sync date: ", lastSyncDate)
> +	}
> +
> +	resp, err := p.requestChanges(authData.AccessToken, lastSyncDate)
> +	if err != nil {
> +		return nil, err
> +	}
> +
> +	messages, err := p.parseChangesResponse(resp)
> +	if err != nil {
> +		return nil, err
> +	}
> +
> +	if len(messages) > 0 {
> +		// Update last sync date
> +		log.Print("Request calendar sync")
> +		err = syncMonitor.SyncAccount(p.accountId, "calendar")
> +		if err != nil {
> +			log.Print("Fail to start calendar sync ", p.accountId, " error: ", err)
> +		}
> +	} else {
> +		log.Print("No sync is required.")

I found this to be a bit ambiguous reading account polld's logs. Maybe say “Found no calendar updates for account <id>”?

> +	}
> +
> +	return nil, nil
> +}
> +
> +
> +func (p *GCalendarPlugin) parseChangesResponse(resp *http.Response) ([]event, error) {
> +	defer resp.Body.Close()
> +	decoder := json.NewDecoder(resp.Body)
> +
> +	if resp.StatusCode != http.StatusOK {
> +		var errResp errorResp
> +		if err := decoder.Decode(&errResp); err != nil {
> +			return nil, err
> +		}
> +		if errResp.Err.Code == 401 {
> +			return nil, plugins.ErrTokenExpired
> +		}
> +		return nil, nil
> +	}
> +
> +	var events eventList
> +	if err := decoder.Decode(&events); err != nil {
> +		return nil, err
> +	}
> +
> +	for _, ev := range events.Events {
> +		log.Print("Found event: ", ev.Etag, ev.Summary)
> +	}
> +	
> +	return events.Events, nil
> +}
> +
> +func (p *GCalendarPlugin) requestChanges(accessToken string, lastSyncDate string) (*http.Response, error) {
> +	u, err := baseUrl.Parse("events")
> +	if err != nil {
> +		return nil, err
> +	}
> +
> +	//GET https://www.googleapis.com/calendar/v3/calendars/primary/events?showDeleted=true&singleEvents=true&updatedMin=2016-04-06T10%3A00%3A00.00Z&fields=description%2Citems(description%2Cetag%2Csummary)&key={YOUR_API_KEY}
> +	query := baseUrl.Query()
> +	query.Add("showDeleted", "true")
> +	query.Add("singleEvents", "true")
> +	query.Add("fields", "description,items(summary,etag)")
> +	query.Add("maxResults", "1")
> +	if len(lastSyncDate) > 0 {   
> +		query.Add("updatedMin", lastSyncDate)
> +	}
> +	u.RawQuery = query.Encode()
> +
> +	req, err := http.NewRequest("GET", u.String(), nil)
> +	if err != nil {
> +		return nil, err
> +	}
> +	req.Header.Set("Authorization", "Bearer "+accessToken)
> +
> +	return http.DefaultClient.Do(req)
> +}
> 
> === added file 'plugins/gcalendar/syncmonitor.go'
> --- plugins/gcalendar/syncmonitor.go	1970-01-01 00:00:00 +0000
> +++ plugins/gcalendar/syncmonitor.go	2016-04-13 13:33:08 +0000
> @@ -0,0 +1,75 @@
> +/*
> + Copyright 2016 Canonical Ltd.
> +
> + 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 gcalendar
> +
> +import (
> +	"runtime"
> +	"log"
> +
> +	"launchpad.net/go-dbus/v1"
> +)
> +
> +const (
> +	busInterface = "com.canonical.SyncMonitor"
> +	busPath      = "/com/canonical/SyncMonitor"
> +	busName      = "com.canonical.SyncMonitor"
> +)
> +
> +type SyncMonitor struct {
> +	conn     *dbus.Connection
> +	obj      *dbus.ObjectProxy
> +}
> +
> +func NewSyncMonitor() *SyncMonitor {
> +	conn, err := dbus.Connect(dbus.SessionBus)
> +	if err != nil {
> +		log.Print("Fail to connect with session bus: ", err)
> +		return nil
> +	}
> +
> +	log.Print("SynMonitor proxy created!")

Do we need to log this?

> +	p := &SyncMonitor{
> +		conn:   conn,
> +		obj:    conn.Object(busName, busPath),
> +	}
> +	runtime.SetFinalizer(p, clean)
> +	return p
> +}
> +
> +func clean(p *SyncMonitor) {
> +	log.Print("SynMonitor proxy destroyed.")

When is clean called? Do we need to log here?

> +	if p.conn != nil {
> +		p.conn.Close()
> +	}
> +}
> +
> +func (p *SyncMonitor) LastSyncDate(accountId uint, serviceName string) (lastSyncDate string, err error)  {
> +	message, err := p.obj.Call(busInterface, "lastSuccessfulSyncDate", uint32(accountId), serviceName)
> +	if err != nil {
> +		return "", err
> +	} else {
> +		var lastSyncDate string
> +		err = message.Args(&lastSyncDate)
> +		return lastSyncDate, err
> +	}
> +}
> +
> +func (p *SyncMonitor) SyncAccount(accountId uint, serviceName string) (err error) {
> +	_, err = p.obj.Call(busInterface, "syncAccount", uint32(accountId), serviceName)
> +	return err
> +}
> +


-- 
https://code.launchpad.net/~renatofilho/account-polld/gcalendar-plugin/+merge/291393
Your team Ubuntu Push Hackers is subscribed to branch lp:account-polld.



More information about the Ubuntu-reviews mailing list