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

[John Lenton]

added handleConnState

Added handleConnState to deal with connectivity events.

[lp:~chipaca/ubuntu-push/client-v0-p5]

49. By John Lenton on 2014-02-03

oops, it's no longer AutoRetry

50. By John Lenton on 2014-02-04

Merged client-v0-p4 into client-v0-p5.

51. By John Lenton on 2014-02-04

fixed tests broken by changes to session

52. By John Lenton on 2014-02-04

Merged client-v0-p4 into client-v0-p5.

[Samuele Pedroni (pedronis)]

58 + panic("Don't know how to handle session creation failure.")

this panic should contain err

65 + client.sessionRetrierStopper <- true

should the autoretrier stopper be unbuffered or buffered if it's unbuffered we risk to be trying in two places to dial, no?

[Samuele Pedroni (pedronis)]

75 + // disconnected
76 + if client.sessionRetrierStopper != nil {
77 + client.sessionRetrierStopper <- true
78 + client.sessionRetrierStopper = nil
79 + } else {
80 + client.session.Close()
81 + }

it would be nice if that code was just spelled client.session.Close()

[John Lenton (chipaca)]

the panic() is changed to a return err a little further down the pipeline.

the retrierStopper needs to be buffered because it's entirely possible there's nobody listening at the other end any more. It does mean we're not waiting for one to finish before starting the next, though.

I'll be moving that logic into session in a later branch, ok?

[John Lenton (chipaca)]

bug lp:1276199 for the session retry rework.

[Samuele Pedroni (pedronis)]

ok, but see bug and as discussed

review: Approve

=== modified file 'client/client.go'
--- client/client.go	2014-02-03 16:14:16 +0000
+++ client/client.go	2014-02-04 13:40:08 +0000
@@ -27,6 +27,7 @@
 	"launchpad.net/ubuntu-push/bus/networkmanager"
 	"launchpad.net/ubuntu-push/bus/notifications"
 	"launchpad.net/ubuntu-push/bus/urldispatcher"
+	"launchpad.net/ubuntu-push/client/session"
 	"launchpad.net/ubuntu-push/config"
 	"launchpad.net/ubuntu-push/logger"
 	"launchpad.net/ubuntu-push/util"
@@ -47,16 +48,19 @@
 
 // Client is the Ubuntu Push Notifications client-side daemon.
 type Client struct {
-	config            ClientConfig
-	log               logger.Logger
-	pem               []byte
-	idder             identifier.Id
-	deviceId          string
-	notificationsEndp bus.Endpoint
-	urlDispatcherEndp bus.Endpoint
-	connectivityEndp  bus.Endpoint
-	connCh            chan bool
-	actionsCh         <-chan notifications.RawActionReply
+	config                ClientConfig
+	log                   logger.Logger
+	pem                   []byte
+	idder                 identifier.Id
+	deviceId              string
+	notificationsEndp     bus.Endpoint
+	urlDispatcherEndp     bus.Endpoint
+	connectivityEndp      bus.Endpoint
+	connCh                chan bool
+	connState             bool
+	actionsCh             <-chan notifications.RawActionReply
+	session               *session.ClientSession
+	sessionRetrierStopper chan bool
 }
 
 // Configure loads the configuration specified in configPath, and sets it up.
@@ -119,3 +123,41 @@
 	client.actionsCh = actionsCh
 	return err
 }
+
+// handleConnState deals with connectivity events
+func (client *Client) handleConnState(connState bool) {
+	if client.connState == connState {
+		// 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()
+	} else {
+		// disconnected
+		if client.sessionRetrierStopper != nil {
+			client.sessionRetrierStopper <- true
+			client.sessionRetrierStopper = nil
+		} else {
+			client.session.Close()
+		}
+	}
+}

=== modified file 'client/client_test.go'
--- client/client_test.go	2014-02-04 13:40:07 +0000
+++ client/client_test.go	2014-02-04 13:40:08 +0000
@@ -22,6 +22,7 @@
 	. "launchpad.net/gocheck"
 	"launchpad.net/ubuntu-push/bus/networkmanager"
 	testibus "launchpad.net/ubuntu-push/bus/testing"
+	"launchpad.net/ubuntu-push/client/session"
 	"launchpad.net/ubuntu-push/logger"
 	helpers "launchpad.net/ubuntu-push/testing"
 	"launchpad.net/ubuntu-push/testing/condition"
@@ -277,3 +278,57 @@
 	c.Check(cli.takeTheBus(), NotNil)
 	c.Check(cli.actionsCh, IsNil)
 }
+
+/*****************************************************************
+    handleConnState tests
+******************************************************************/
+
+func (cs *clientSuite) TestHandleConnStateD2C(c *C) {
+	cli := new(Client)
+
+	// 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)
+	cli.handleConnState(true)
+	c.Check(cli.connState, 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)
+	cli.handleConnState(false)
+	c.Check(cli.session, IsNil)
+
+	cli.connState = true
+	cli.handleConnState(true)
+	c.Check(cli.session, IsNil)
+}
+
+func (cs *clientSuite) TestHandleConnStateC2D(c *C) {
+	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.session.State() will be "Error" here, for now at least
+	c.Check(cli.session.State(), Not(Equals), session.Disconnected)
+	cli.handleConnState(false)
+	c.Check(cli.session.State(), Equals, session.Disconnected)
+}
+
+func (cs *clientSuite) TestHandleConnStateC2DPending(c *C) {
+	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.handleConnState(false)
+	c.Check(cli.session.State(), Equals, session.Disconnected)
+	c.Check(cli.sessionRetrierStopper, IsNil)
+}