Merge lp:~chipaca/ubuntu-push/client-v0-p9 into lp:ubuntu-push

[John Lenton]

Several things:
* features:
  * bus/testing/'s Endpoint now tracks Calls; call bus/testing.GetCallCargs(endp) to get the list.
  * Client.handleErr, Client.handleNotification and Client.handleClick (and tests)
* cleanups:
  * renamed client's Client's connState to hasConnectivity
  * split out code from handleConnState into initSession/connectSession/disconnectSession

Several things:
* features:
  * bus/testing/'s Endpoint now tracks Calls; call bus/testing.GetCallCargs(endp) to get the list.
  * Client.handleErr, Client.handleNotification and Client.handleClick (and tests)
* cleanups:
  * renamed client's Client's connState to hasConnectivity
  * split out code from handleConnState into initSession/connectSession/disconnectSession

[Samuele Pedroni (pedronis)]

ok, but see bug lp:1276199 and as discussed

review: Approve

=== modified file 'bus/testing/testing_endpoint.go'
--- bus/testing/testing_endpoint.go	2014-01-31 13:46:31 +0000
+++ bus/testing/testing_endpoint.go	2014-02-04 14:21:43 +0000
@@ -26,11 +26,17 @@
 	"time"
 )
 
+type callArgs struct {
+	Member string
+	Args   []interface{}
+}
+
 type testingEndpoint struct {
 	dialCond    condition.Interface
 	callCond    condition.Interface
 	retvals     [][]interface{}
 	watchTicker chan bool
+	callArgs    []callArgs
 }
 
 // Build a bus.Endpoint that calls OK() on its condition before returning
@@ -39,7 +45,7 @@
 // NOTE: Call() always returns the first return value; Watch() will provide
 // each of them in turn, irrespective of whether Call has been called.
 func NewMultiValuedTestingEndpoint(dialCond condition.Interface, callCond condition.Interface, retvalses ...[]interface{}) bus.Endpoint {
-	return &testingEndpoint{dialCond, callCond, retvalses, nil}
+	return &testingEndpoint{dialCond, callCond, retvalses, nil, nil}
 }
 
 func NewTestingEndpoint(dialCond condition.Interface, callCond condition.Interface, retvals ...interface{}) bus.Endpoint {
@@ -47,7 +53,7 @@
 	for i, x := range retvals {
 		retvalses[i] = []interface{}{x}
 	}
-	return &testingEndpoint{dialCond, callCond, retvalses, nil}
+	return &testingEndpoint{dialCond, callCond, retvalses, nil, nil}
 }
 
 // If SetWatchTicker is called with a non-nil watchTicker, it is used
@@ -57,6 +63,11 @@
 	tc.(*testingEndpoint).watchTicker = watchTicker
 }
 
+// GetCallArgs returns a list of the arguments for each Call() invocation.
+func GetCallArgs(tc bus.Endpoint) []callArgs {
+	return tc.(*testingEndpoint).callArgs
+}
+
 // See Endpoint's WatchSignal. This WatchSignal will check its condition to
 // decide whether to return an error, or provide each of its return values
 func (tc *testingEndpoint) WatchSignal(member string, f func(...interface{}), d func()) error {
@@ -81,6 +92,7 @@
 // See Endpoint's Call. This Call will check its condition to decide whether
 // to return an error, or the first of its return values
 func (tc *testingEndpoint) Call(member string, args ...interface{}) ([]interface{}, error) {
+	tc.callArgs = append(tc.callArgs, callArgs{member, args})
 	if tc.callCond.OK() {
 		if len(tc.retvals) == 0 {
 			panic("No return values provided!")

=== modified file 'bus/testing/testing_endpoint_test.go'
--- bus/testing/testing_endpoint_test.go	2014-01-31 13:31:09 +0000
+++ bus/testing/testing_endpoint_test.go	2014-02-04 14:21:43 +0000
@@ -65,6 +65,15 @@
 	c.Check(func() { endp.Call("") }, PanicMatches, "No return values provided.*")
 }
 
+// Test that Call() updates callArgs
+func (s *TestingEndpointSuite) TestCallArgs(c *C) {
+	endp := NewTestingEndpoint(nil, condition.Work(true), 0)
+	_, err := endp.Call("what", "is", "this", "thing")
+	c.Assert(err, IsNil)
+	c.Check(GetCallArgs(endp), DeepEquals,
+		[]callArgs{{"what", []interface{}{"is", "this", "thing"}}})
+}
+
 // Test that WatchSignal() with a positive condition sends the provided return
 // values over the channel.
 func (s *TestingEndpointSuite) TestWatch(c *C) {

=== modified file 'client/client.go'
--- client/client.go	2014-02-04 14:21:43 +0000
+++ client/client.go	2014-02-04 14:21:43 +0000
@@ -22,6 +22,7 @@
 	"encoding/pem"
 	"fmt"
 	"io/ioutil"
+	"launchpad.net/go-dbus/v1"
 	"launchpad.net/ubuntu-push/bus"
 	"launchpad.net/ubuntu-push/bus/connectivity"
 	"launchpad.net/ubuntu-push/bus/networkmanager"
@@ -57,7 +58,7 @@
 	urlDispatcherEndp     bus.Endpoint
 	connectivityEndp      bus.Endpoint
 	connCh                chan bool
-	connState             bool
+	hasConnectivity       bool
 	actionsCh             <-chan notifications.RawActionReply
 	session               *session.ClientSession
 	sessionRetrierStopper chan bool
@@ -124,40 +125,90 @@
 	return err
 }
 
+// initSession creates the session object
+func (client *Client) initSession() {
+	sess, err := session.NewSession(string(client.config.Addr), client.pem,
+		client.config.ExchangeTimeout.Duration, client.deviceId, client.log)
+	if err != nil {
+		panic("Don't know how to handle session creation failure.")
+	}
+	client.session = sess
+}
+
+// connectSession kicks off the session connection dance
+func (client *Client) connectSession() {
+	if client.sessionRetrierStopper != nil {
+		client.sessionRetrierStopper <- true
+		client.sessionRetrierStopper = nil
+	}
+	ar := &util.AutoRetrier{
+		make(chan bool, 1),
+		client.session.Dial,
+		util.Jitter}
+	client.sessionRetrierStopper = ar.Stop
+	go ar.Retry()
+}
+
+// disconnectSession disconnects the session
+func (client *Client) disconnectSession() {
+	if client.sessionRetrierStopper != nil {
+		client.sessionRetrierStopper <- true
+		client.sessionRetrierStopper = nil
+	} else {
+		client.session.Close()
+	}
+}
+
 // handleConnState deals with connectivity events
-func (client *Client) handleConnState(connState bool) {
-	if client.connState == connState {
+func (client *Client) handleConnState(hasConnectivity bool) {
+	if client.hasConnectivity == hasConnectivity {
 		// nothing to do!
 		return
 	}
-	client.connState = connState
-	if client.session == nil {
-		sess, err := session.NewSession(string(client.config.Addr), client.pem,
-			client.config.ExchangeTimeout.Duration, client.deviceId, client.log)
-		if err != nil {
-			panic("Don't know how to handle session creation failure.")
-		}
-		client.session = sess
-	}
-	if connState {
-		// connected
-		if client.sessionRetrierStopper != nil {
-			client.sessionRetrierStopper <- true
-			client.sessionRetrierStopper = nil
-		}
-		ar := &util.AutoRetrier{
-			make(chan bool, 1),
-			client.session.Dial,
-			util.Jitter}
-		client.sessionRetrierStopper = ar.Stop
-		go ar.Retry()
+	client.hasConnectivity = hasConnectivity
+	if hasConnectivity {
+		client.connectSession()
 	} else {
-		// disconnected
-		if client.sessionRetrierStopper != nil {
-			client.sessionRetrierStopper <- true
-			client.sessionRetrierStopper = nil
-		} else {
-			client.session.Close()
-		}
-	}
+		client.disconnectSession()
+	}
+}
+
+// handleErr deals with the session erroring out of its loop
+func (client *Client) handleErr(err error) {
+	// if we're not connected, we don't really care
+	client.log.Errorf("session exited: %s", err)
+	if client.hasConnectivity {
+		client.connectSession()
+	}
+}
+
+// handleNotification deals with receiving a notification
+func (client *Client) handleNotification() error {
+	action_id := "dummy_id"
+	a := []string{action_id, "Go get it!"} // action value not visible on the phone
+	h := map[string]*dbus.Variant{"x-canonical-switch-to-application": &dbus.Variant{true}}
+	nots := notifications.Raw(client.notificationsEndp, client.log)
+	not_id, err := nots.Notify(
+		"ubuntu-push-client",               // app name
+		uint32(0),                          // id
+		"update_manager_icon",              // icon
+		"There's an updated system image!", // summary
+		"You've got to get it! Now! Run!",  // body
+		a,              // actions
+		h,              // hints
+		int32(10*1000), // timeout (ms)
+	)
+	if err != nil {
+		client.log.Errorf("showing notification: %s", err)
+		return err
+	}
+	client.log.Debugf("got notification id %d", not_id)
+	return nil
+}
+
+// handleClick deals with the user clicking a notification
+func (client *Client) handleClick() error {
+	// it doesn't get much simpler...
+	urld := urldispatcher.New(client.urlDispatcherEndp, client.log)
+	return urld.DispatchURL("settings:///system/system-update")
 }

=== modified file 'client/client_test.go'
--- client/client_test.go	2014-02-04 14:21:43 +0000
+++ client/client_test.go	2014-02-04 14:21:43 +0000
@@ -17,6 +17,7 @@
 package client
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	. "launchpad.net/gocheck"
@@ -280,32 +281,47 @@
 }
 
 /*****************************************************************
+    handleErr tests
+******************************************************************/
+
+func (cs *clientSuite) TestHandleErr(c *C) {
+	cli := new(Client)
+	cli.log = noisylog
+	cli.initSession()
+	cli.hasConnectivity = true
+	cli.handleErr(nil)
+	c.Assert(cli.session, NotNil)
+	// let the session connection fail
+	time.Sleep(100 * time.Millisecond)
+	c.Check(cli.session.State(), Equals, session.Error)
+}
+
+/*****************************************************************
     handleConnState tests
 ******************************************************************/
 
 func (cs *clientSuite) TestHandleConnStateD2C(c *C) {
 	cli := new(Client)
-
+	cli.initSession()
 	// let's pretend the client had a previous attempt at connecting still pending
 	// (hard to trigger in real life, but possible)
 	cli.sessionRetrierStopper = make(chan bool, 1)
 
-	c.Assert(cli.connState, Equals, false)
+	c.Assert(cli.hasConnectivity, Equals, false)
 	cli.handleConnState(true)
-	c.Check(cli.connState, Equals, true)
+	c.Check(cli.hasConnectivity, Equals, true)
 	c.Assert(cli.session, NotNil)
-	c.Check(cli.session.State(), Equals, session.Disconnected)
 }
 
 func (cs *clientSuite) TestHandleConnStateSame(c *C) {
 	cli := new(Client)
 	// here we want to check that we don't do anything
 	c.Assert(cli.session, IsNil)
-	c.Assert(cli.connState, Equals, false)
+	c.Assert(cli.hasConnectivity, Equals, false)
 	cli.handleConnState(false)
 	c.Check(cli.session, IsNil)
 
-	cli.connState = true
+	cli.hasConnectivity = true
 	cli.handleConnState(true)
 	c.Check(cli.session, IsNil)
 }
@@ -314,7 +330,7 @@
 	cli := new(Client)
 	cli.session, _ = session.NewSession(string(cli.config.Addr), cli.pem, cli.config.ExchangeTimeout.Duration, cli.deviceId, noisylog)
 	cli.session.Dial()
-	cli.connState = true
+	cli.hasConnectivity = true
 
 	// cli.session.State() will be "Error" here, for now at least
 	c.Check(cli.session.State(), Not(Equals), session.Disconnected)
@@ -326,9 +342,52 @@
 	cli := new(Client)
 	cli.session, _ = session.NewSession(string(cli.config.Addr), cli.pem, cli.config.ExchangeTimeout.Duration, cli.deviceId, noisylog)
 	cli.sessionRetrierStopper = make(chan bool, 1)
-	cli.connState = true
+	cli.hasConnectivity = true
 
 	cli.handleConnState(false)
 	c.Check(cli.session.State(), Equals, session.Disconnected)
 	c.Check(cli.sessionRetrierStopper, IsNil)
 }
+
+/*****************************************************************
+    handleNotification tests
+******************************************************************/
+
+func (cs *clientSuite) TestHandleNotification(c *C) {
+	buf := &bytes.Buffer{}
+	cli := new(Client)
+	endp := testibus.NewTestingEndpoint(nil, condition.Work(true), uint32(1))
+	cli.notificationsEndp = endp
+	cli.log = logger.NewSimpleLogger(buf, "debug")
+	c.Check(cli.handleNotification(), IsNil)
+	// check we sent the notification
+	args := testibus.GetCallArgs(endp)
+	c.Assert(args, HasLen, 1)
+	c.Check(args[0].Member, Equals, "Notify")
+	c.Check(buf.String(), Matches, `.* got notification id \d+\s*`)
+}
+
+func (cs *clientSuite) TestHandleNotificationFail(c *C) {
+	cli := new(Client)
+	endp := testibus.NewTestingEndpoint(nil, condition.Work(false))
+	cli.notificationsEndp = endp
+	cli.log = noisylog
+	c.Check(cli.handleNotification(), NotNil)
+}
+
+/*****************************************************************
+    handleClick tests
+******************************************************************/
+
+func (cs *clientSuite) TestHandleClick(c *C) {
+	cli := new(Client)
+	endp := testibus.NewTestingEndpoint(nil, condition.Work(true), nil)
+	cli.urlDispatcherEndp = endp
+	cli.log = noisylog
+	c.Check(cli.handleClick(), IsNil)
+	// check we sent the notification
+	args := testibus.GetCallArgs(endp)
+	c.Assert(args, HasLen, 1)
+	c.Check(args[0].Member, Equals, "DispatchURL")
+	c.Check(args[0].Args, DeepEquals, []interface{}{"settings:///system/system-update"})
+}