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

[John Lenton]

part 3: setting up the bus

In which we set up the D-Bus connections and event channels.

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

49. By John Lenton on 2014-02-03

another rework to util/redialer

[John Lenton (chipaca)]

Also included for free in this branch: a rework to util/redialer that exposes it as a type.

[Samuele Pedroni (pedronis)]

needs to check somewhere that connCh is not nil?

[Samuele Pedroni (pedronis)]

takeTheBus has a lot of different styles, not sure what do do about that

265 +func (ar *AutoRetrier) AutoRetry() uint32 {

can probably just say Retry now?

[Samuele Pedroni (pedronis)]

review: Approve

[Samuele Pedroni (pedronis)]

need to double check somehow that the endpoints are taken on the right bus (system vs session)

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

50. By John Lenton on 2014-02-03

renamed AutoRetrier.AutoRetry to .Retry; checked that Client.connCh is not-nil after config.

=== modified file 'client/client.go'
--- client/client.go	2014-02-03 12:23:56 +0000
+++ client/client.go	2014-02-03 18:17:41 +0000
@@ -22,28 +22,41 @@
 	"encoding/pem"
 	"fmt"
 	"io/ioutil"
+	"launchpad.net/ubuntu-push/bus"
 	"launchpad.net/ubuntu-push/bus/connectivity"
+	"launchpad.net/ubuntu-push/bus/networkmanager"
+	"launchpad.net/ubuntu-push/bus/notifications"
+	"launchpad.net/ubuntu-push/bus/urldispatcher"
 	"launchpad.net/ubuntu-push/config"
 	"launchpad.net/ubuntu-push/logger"
+	"launchpad.net/ubuntu-push/util"
 	"launchpad.net/ubuntu-push/whoopsie/identifier"
 	"os"
 )
 
+// ClientConfig holds the client configuration
 type ClientConfig struct {
-	connectivity.ConnectivityConfig
-	// session configuration
+	connectivity.ConnectivityConfig // q.v.
+	// A reasonably large maximum ping time
 	ExchangeTimeout config.ConfigTimeDuration `json:"exchange_timeout"`
-	// server connection config
-	Addr        config.ConfigHostPort
+	// The server to connect to
+	Addr config.ConfigHostPort
+	// The PEM-encoded server certificate
 	CertPEMFile string `json:"cert_pem_file"`
 }
 
+// Client is the Ubuntu Push Notifications client-side daemon.
 type Client struct {
-	config   ClientConfig
-	log      logger.Logger
-	pem      []byte
-	idder    identifier.Id
-	deviceId string
+	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
 }
 
 // Configure loads the configuration specified in configPath, and sets it up.
@@ -61,6 +74,11 @@
 
 	// overridden for testing
 	client.idder = identifier.New()
+	client.notificationsEndp = bus.SessionBus.Endpoint(notifications.BusAddress, client.log)
+	client.urlDispatcherEndp = bus.SessionBus.Endpoint(urldispatcher.BusAddress, client.log)
+	client.connectivityEndp = bus.SystemBus.Endpoint(networkmanager.BusAddress, client.log)
+
+	client.connCh = make(chan bool)
 
 	if client.config.CertPEMFile != "" {
 		client.pem, err = ioutil.ReadFile(client.config.CertPEMFile)
@@ -86,3 +104,18 @@
 	client.deviceId = client.idder.String()
 	return nil
 }
+
+// takeTheBus starts the connection(s) to D-Bus and sets up associated event channels
+func (client *Client) takeTheBus() error {
+	go connectivity.ConnectedState(client.connectivityEndp,
+		client.config.ConnectivityConfig, client.log, client.connCh)
+	iniCh := make(chan uint32)
+	go func() { iniCh <- util.AutoRedial(client.notificationsEndp) }()
+	go func() { iniCh <- util.AutoRedial(client.urlDispatcherEndp) }()
+	<-iniCh
+	<-iniCh
+
+	actionsCh, err := notifications.Raw(client.notificationsEndp, client.log).WatchActions()
+	client.actionsCh = actionsCh
+	return err
+}

=== modified file 'client/client_test.go'
--- client/client_test.go	2014-02-03 14:19:58 +0000
+++ client/client_test.go	2014-02-03 18:17:41 +0000
@@ -20,10 +20,16 @@
 	"fmt"
 	"io/ioutil"
 	. "launchpad.net/gocheck"
+	"launchpad.net/ubuntu-push/bus/networkmanager"
+	testibus "launchpad.net/ubuntu-push/bus/testing"
 	"launchpad.net/ubuntu-push/logger"
 	helpers "launchpad.net/ubuntu-push/testing"
+	"launchpad.net/ubuntu-push/testing/condition"
+	"launchpad.net/ubuntu-push/util"
 	"launchpad.net/ubuntu-push/whoopsie/identifier"
 	idtesting "launchpad.net/ubuntu-push/whoopsie/identifier/testing"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"testing"
@@ -32,16 +38,41 @@
 
 func TestClient(t *testing.T) { TestingT(t) }
 
+// takeNext takes a value from given channel with a 5s timeout
+func takeNextBool(ch <-chan bool) bool {
+	select {
+	case <-time.After(5 * time.Second):
+		panic("channel stuck: too long waiting")
+	case v := <-ch:
+		return v
+	}
+}
+
 type clientSuite struct {
+	timeouts   []time.Duration
 	configPath string
 }
 
 var nullog = logger.NewSimpleLogger(ioutil.Discard, "error")
 var noisylog = logger.NewSimpleLogger(os.Stderr, "debug")
-var debuglog = nullog
 var _ = Suite(&clientSuite{})
 
+const (
+	staticText = "something ipsum dolor something"
+	staticHash = "6155f83b471583f47c99998a472a178f"
+)
+
+func mkHandler(text string) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		w.(http.Flusher).Flush()
+		w.Write([]byte(text))
+		w.(http.Flusher).Flush()
+	}
+}
+
 func (cs *clientSuite) SetUpTest(c *C) {
+	cs.timeouts = util.Timeouts
+	util.Timeouts = []time.Duration{0}
 	dir := c.MkDir()
 	cs.configPath = filepath.Join(dir, "config")
 	cfg := fmt.Sprintf(`
@@ -57,6 +88,10 @@
 	ioutil.WriteFile(cs.configPath, []byte(cfg), 0600)
 }
 
+func (cs *clientSuite) TearDownTest(c *C) {
+	util.Timeouts = cs.timeouts
+}
+
 /*****************************************************************
     Configure tests
 ******************************************************************/
@@ -93,6 +128,26 @@
 	c.Assert(cli.idder, DeepEquals, identifier.New())
 }
 
+func (cs *clientSuite) TestConfigureSetsUpEndpoints(c *C) {
+	cli := new(Client)
+	c.Check(cli.notificationsEndp, IsNil)
+	c.Check(cli.urlDispatcherEndp, IsNil)
+	c.Check(cli.connectivityEndp, IsNil)
+	err := cli.Configure(cs.configPath)
+	c.Assert(err, IsNil)
+	c.Assert(cli.notificationsEndp, NotNil)
+	c.Assert(cli.urlDispatcherEndp, NotNil)
+	c.Assert(cli.connectivityEndp, NotNil)
+}
+
+func (cs *clientSuite) TestConfigureSetsUpConnCh(c *C) {
+	cli := new(Client)
+	c.Check(cli.connCh, IsNil)
+	err := cli.Configure(cs.configPath)
+	c.Assert(err, IsNil)
+	c.Assert(cli.connCh, NotNil)
+}
+
 func (cs *clientSuite) TestConfigureBailsOnBadFilename(c *C) {
 	cli := new(Client)
 	err := cli.Configure("/does/not/exist")
@@ -157,3 +212,65 @@
 	c.Check(cli.deviceId, Equals, "")
 	c.Check(cli.getDeviceId(), NotNil)
 }
+
+/*****************************************************************
+    takeTheBus tests
+******************************************************************/
+
+func (cs *clientSuite) TestTakeTheBusWorks(c *C) {
+	// http server used for connectivity test
+	ts := httptest.NewServer(mkHandler(staticText))
+	defer ts.Close()
+
+	// testing endpoints
+	nCond := condition.Fail2Work(3)
+	nEndp := testibus.NewMultiValuedTestingEndpoint(nCond, condition.Work(true),
+		[]interface{}{uint32(1), "hello"})
+	uCond := condition.Fail2Work(5)
+	uEndp := testibus.NewTestingEndpoint(uCond, condition.Work(false))
+	cCond := condition.Fail2Work(7)
+	cEndp := testibus.NewTestingEndpoint(cCond, condition.Work(true),
+		uint32(networkmanager.ConnectedGlobal),
+	)
+	// ok, create the thing
+	cli := new(Client)
+	err := cli.Configure(cs.configPath)
+	c.Assert(err, IsNil)
+	// the user actions channel has not been set up
+	c.Check(cli.actionsCh, IsNil)
+
+	// and stomp on things for testing
+	cli.config.ConnectivityConfig.ConnectivityCheckURL = ts.URL
+	cli.config.ConnectivityConfig.ConnectivityCheckMD5 = staticHash
+	cli.notificationsEndp = nEndp
+	cli.urlDispatcherEndp = uEndp
+	cli.connectivityEndp = cEndp
+
+	c.Assert(cli.takeTheBus(), IsNil)
+	// the notifications and urldispatcher endpoints retried until connected
+	c.Check(nCond.OK(), Equals, true)
+	c.Check(uCond.OK(), Equals, true)
+	// the user actions channel has now been set up
+	c.Check(cli.actionsCh, NotNil)
+	c.Check(takeNextBool(cli.connCh), Equals, false)
+	c.Check(takeNextBool(cli.connCh), Equals, true)
+	// the connectivity endpoint retried until connected
+	c.Check(cCond.OK(), Equals, true)
+}
+
+// takeTheBus can, in fact, fail
+func (cs *clientSuite) TestTakeTheBusCanFail(c *C) {
+	cli := new(Client)
+	err := cli.Configure(cs.configPath)
+	c.Assert(err, IsNil)
+	// the user actions channel has not been set up
+	c.Check(cli.actionsCh, IsNil)
+
+	// and stomp on things for testing
+	cli.notificationsEndp = testibus.NewTestingEndpoint(condition.Work(true), condition.Work(false))
+	cli.urlDispatcherEndp = testibus.NewTestingEndpoint(condition.Work(true), condition.Work(false))
+	cli.connectivityEndp = testibus.NewTestingEndpoint(condition.Work(true), condition.Work(false))
+
+	c.Check(cli.takeTheBus(), NotNil)
+	c.Check(cli.actionsCh, IsNil)
+}

=== modified file 'util/redialer.go'
--- util/redialer.go	2014-01-28 23:40:52 +0000
+++ util/redialer.go	2014-02-03 18:17:41 +0000
@@ -49,14 +49,20 @@
 	return time.Duration(rand.Int63n(2*n+1) - n)
 }
 
-// AutoRetry keeps on calling f() until it stops returning an error.
-// It does exponential backoff, adding jitter at each step back.
-func AutoRetry(f func() error, jitter func(time.Duration) time.Duration) uint32 {
+type AutoRetrier struct {
+	Stop   chan bool
+	Dial   func() error
+	Jitter func(time.Duration) time.Duration
+}
+
+// AutoRetry keeps on calling Dial until it stops returning an error.  It does
+// exponential backoff, adding the output of Jitter at each step back.
+func (ar *AutoRetrier) Retry() uint32 {
 	var timeout time.Duration
 	var dialAttempts uint32 = 0 // unsigned so it can wrap safely ...
 	var numTimeouts uint32 = uint32(len(Timeouts))
 	for {
-		if f() == nil {
+		if ar.Dial() == nil {
 			return dialAttempts + 1
 		}
 		if dialAttempts < numTimeouts {
@@ -64,10 +70,10 @@
 		} else {
 			timeout = Timeouts[numTimeouts-1]
 		}
-		timeout += jitter(timeout)
+		timeout += ar.Jitter(timeout)
 		dialAttempts++
 		select {
-		case <-quitRedialing:
+		case <-ar.Stop:
 			return dialAttempts
 		case <-time.NewTimer(timeout).C:
 		}
@@ -78,7 +84,8 @@
 // stops returning an error. It does exponential (optionally
 // jitter'ed) backoff.
 func AutoRedial(dialer Dialer) uint32 {
-	return AutoRetry(dialer.Dial, dialer.Jitter)
+	ar := &AutoRetrier{quitRedialing, dialer.Dial, dialer.Jitter}
+	return ar.Retry()
 }
 
 func init() {

=== modified file 'util/redialer_test.go'
--- util/redialer_test.go	2014-01-29 11:42:27 +0000
+++ util/redialer_test.go	2014-02-03 18:17:41 +0000
@@ -76,7 +76,8 @@
 		}
 	}
 	jitter := func(time.Duration) time.Duration { return 0 }
-	c.Check(AutoRetry(f, jitter), Equals, uint32(6))
+	ar := &AutoRetrier{nil, f, jitter}
+	c.Check(ar.Retry(), Equals, uint32(6))
 }
 
 func (s *RedialerSuite) TestJitter(c *C) {