Merge lp:~pedronis/ubuntu-push/refactor-service-setup into lp:ubuntu-push/automatic

[Samuele Pedroni]

* let take NewPushService a setup obj, have a generic manageReg
* update config.json now that registration is a base url

* let take NewPushService a setup obj, have a generic manageReg
* update config.json now that registration is a base url

[John Lenton (chipaca)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~pedronis/ubuntu-push/refactor-service-setup into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

scripts/deps.sh ubuntu-push-client.go
scripts/deps.sh server/dev/server.go
scripts/deps.sh server/acceptance/cmd/acceptanceclient.go
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -t launchpad.net/ubuntu-push 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/notifications/app_helper launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/urldispatcher launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/service launchpad.net/ubuntu-push/client/session launchpad.net/ubuntu-push/client/session/seenstate launchpad.net/ubuntu-push/config launchpad.net/ubuntu-push/external/murmur3 launchpad.net/ubuntu-push/launch_helper launchpad.net/ubuntu-push/logger launchpad.net/ubuntu-push/messaging launchpad.net/ubuntu-push/messaging/cmessaging launchpad.net/ubuntu-push/messaging/reply launchpad.net/ubuntu-push/nih launchpad.net/ubuntu-push/nih/cnih launchpad.net/ubuntu-push/protocol launchpad.net/ubuntu-push/server launchpad.net/ubuntu-push/server/api launchpad.net/ubuntu-push/server/broker launchpad.net/ubuntu-push/server/broker/simple launchpad.net/ubuntu-push/server/broker/testing launchpad.net/ubuntu-push/server/broker/testsuite launchpad.net/ubuntu-push/server/dev launchpad.net/ubuntu-push/server/listener launchpad.net/ubuntu-push/server/session launchpad.net/ubuntu-push/server/store launchpad.net/ubuntu-push/testing launchpad.net/ubuntu-push/testing/condition launchpad.net/ubuntu-push/util launchpad.net/ubuntu-push/whoopsie launchpad.net/ubuntu-push/whoopsie/identifier launchpad.net/ubuntu-push/whoopsie/identifier/testing launchpad.net/ubuntu-push/server/acceptance/cmd/ launchpad.net/ubuntu-push/server/dev/ launchpad.net/ubuntu-push/ 2>/dev/null | cat > dependencies.tsv
rm -f -r /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 code.google.com/p/go-uuid/uuid
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -u dependencies.tsv
go install launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 code.google.com/p/go-uuid/uuid
go test launchpad.net/ubuntu-push 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/notifications/app_helper launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/urldispatcher launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/service launchpad.net/ubuntu-pu...

[lp:~pedronis/ubuntu-push/refactor-service-setup]

217. By Samuele Pedroni on 2014-07-03

wait for inc elapsed time

=== modified file 'client/client.go'
--- client/client.go	2014-07-02 20:52:56 +0000
+++ client/client.go	2014-07-03 14:25:59 +0000
@@ -26,6 +26,7 @@
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"net/url"
 	"os"
 	"os/exec"
 	"strings"
@@ -163,6 +164,19 @@
 	}
 }
 
+// derivePushServiceSetup derives the service setup from the client configuration bits.
+func (client *PushClient) derivePushServiceSetup() (*service.PushServiceSetup, error) {
+	setup := new(service.PushServiceSetup)
+	purl, err := url.Parse(client.config.RegistrationURL)
+	if err != nil {
+		return nil, fmt.Errorf("cannot parse registration url: %v", err)
+	}
+	setup.RegURL = purl
+	setup.DeviceId = client.deviceId
+	setup.AuthGetter = client.getAuthorization
+	return setup, nil
+}
+
 // getAuthorization gets the authorization blob to send to the server
 func (client *PushClient) getAuthorization(url string) string {
 	client.log.Debugf("getting authorization for %s", url)
@@ -399,6 +413,10 @@
 }
 
 func (client *PushClient) startService() error {
+	setup, err := client.derivePushServiceSetup()
+	if err != nil {
+		return err
+	}
 	if client.pushServiceEndpoint == nil {
 		client.pushServiceEndpoint = bus.SessionBus.Endpoint(service.PushServiceBusAddress, client.log)
 	}
@@ -406,10 +424,7 @@
 		client.postalServiceEndpoint = bus.SessionBus.Endpoint(service.PostalServiceBusAddress, client.log)
 	}
 
-	client.pushService = service.NewPushService(client.pushServiceEndpoint, client.log)
-	client.pushService.SetRegistrationURL(client.config.RegistrationURL)
-	client.pushService.SetAuthGetter(client.getAuthorization)
-	client.pushService.SetDeviceId(client.deviceId)
+	client.pushService = service.NewPushService(client.pushServiceEndpoint, setup, client.log)
 	if err := client.pushService.Start(); err != nil {
 		return err
 	}

=== modified file 'client/client_test.go'
--- client/client_test.go	2014-07-02 20:52:56 +0000
+++ client/client_test.go	2014-07-03 14:25:59 +0000
@@ -289,7 +289,7 @@
 	// finally compare
 	conf := cli.deriveSessionConfig(info)
 	// compare authGetter by string
-	c.Check(fmt.Sprintf("%v", conf.AuthGetter), Equals, fmt.Sprintf("%v", cli.getAuthorization))
+	c.Check(fmt.Sprintf("%#v", conf.AuthGetter), Equals, fmt.Sprintf("%#v", cli.getAuthorization))
 	// and set it to nil
 	conf.AuthGetter = nil
 	expected.AuthGetter = nil
@@ -297,6 +297,51 @@
 }
 
 /*****************************************************************
+    derivePushServiceSetup tests
+******************************************************************/
+
+func (cs *clientSuite) TestDerivePushServiceSetup(c *C) {
+	cs.writeTestConfig(map[string]interface{}{})
+	cli := NewPushClient(cs.configPath, cs.leveldbPath)
+	err := cli.configure()
+	c.Assert(err, IsNil)
+	cli.deviceId = "zoo"
+	expected := &service.PushServiceSetup{
+		DeviceId:   "zoo",
+		AuthGetter: func(string) string { return "" },
+		RegURL:     helpers.ParseURL("reg://"),
+	}
+	// sanity check that we are looking at all fields
+	vExpected := reflect.ValueOf(expected).Elem()
+	nf := vExpected.NumField()
+	for i := 0; i < nf; i++ {
+		fv := vExpected.Field(i)
+		// field isn't empty/zero
+		c.Assert(fv.Interface(), Not(DeepEquals), reflect.Zero(fv.Type()).Interface(), Commentf("forgot about: %s", vExpected.Type().Field(i).Name))
+	}
+	// finally compare
+	setup, err := cli.derivePushServiceSetup()
+	c.Assert(err, IsNil)
+	// compare authGetter by string
+	c.Check(fmt.Sprintf("%#v", setup.AuthGetter), Equals, fmt.Sprintf("%#v", cli.getAuthorization))
+	// and set it to nil
+	setup.AuthGetter = nil
+	expected.AuthGetter = nil
+	c.Check(setup, DeepEquals, expected)
+}
+
+func (cs *clientSuite) TestDerivePushServiceSetupError(c *C) {
+	cs.writeTestConfig(map[string]interface{}{
+		"registration_url": "%gh",
+	})
+	cli := NewPushClient(cs.configPath, cs.leveldbPath)
+	err := cli.configure()
+	c.Assert(err, IsNil)
+	_, err = cli.derivePushServiceSetup()
+	c.Check(err, ErrorMatches, "cannot parse registration url:.*")
+}
+
+/*****************************************************************
     startService tests
 ******************************************************************/
 
@@ -305,7 +350,8 @@
 		"auth_helper": helpers.ScriptAbsPath("dummyauth.sh"),
 	})
 	cli := NewPushClient(cs.configPath, cs.leveldbPath)
-	cli.configure()
+	err := cli.configure()
+	c.Assert(err, IsNil)
 	cli.log = cs.log
 	cli.deviceId = "fake-id"
 	cli.pushServiceEndpoint = testibus.NewTestingEndpoint(condition.Work(true), nil)
@@ -314,8 +360,6 @@
 	c.Check(cli.startService(), IsNil)
 	c.Assert(cli.pushService, NotNil)
 	c.Check(cli.pushService.IsRunning(), Equals, true)
-	c.Check(cli.pushService.GetDeviceId(), Equals, "fake-id")
-	c.Check(cli.pushService.GetRegistrationAuthorization(), Equals, "hello reg://")
 	c.Assert(cli.setupPostalService(), IsNil)
 	c.Assert(cli.startPostalService(), IsNil)
 	c.Check(cli.postalService.IsRunning(), Equals, true)
@@ -323,6 +367,17 @@
 	cli.postalService.Stop()
 }
 
+func (cs *clientSuite) TestStartServiceSetupError(c *C) {
+	cs.writeTestConfig(map[string]interface{}{
+		"registration_url": "%gh",
+	})
+	cli := NewPushClient(cs.configPath, cs.leveldbPath)
+	err := cli.configure()
+	c.Assert(err, IsNil)
+	err = cli.startService()
+	c.Check(err, ErrorMatches, "cannot parse registration url:.*")
+}
+
 func (cs *clientSuite) TestStartServiceErrorsOnNilLog(c *C) {
 	cli := NewPushClient(cs.configPath, cs.leveldbPath)
 	c.Check(cli.log, IsNil)

=== modified file 'client/service/service.go'
--- client/service/service.go	2014-07-01 11:55:30 +0000
+++ client/service/service.go	2014-07-03 14:25:59 +0000
@@ -23,6 +23,7 @@
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"net/url"
 	"os"
 	"strings"
 
@@ -32,10 +33,17 @@
 	"launchpad.net/ubuntu-push/nih"
 )
 
+// PushServiceSetup encapsulates the params for setting up a PushService.
+type PushServiceSetup struct {
+	RegURL     *url.URL
+	DeviceId   string
+	AuthGetter func(string) string
+}
+
 // PushService is the dbus api
 type PushService struct {
 	DBusService
-	regURL     string
+	regURL     *url.URL
 	deviceId   string
 	authGetter func(string) string
 	httpCli    http13.Client
@@ -50,59 +58,28 @@
 )
 
 // NewPushService() builds a new service and returns it.
-func NewPushService(bus bus.Endpoint, log logger.Logger) *PushService {
+func NewPushService(bus bus.Endpoint, setup *PushServiceSetup, log logger.Logger) *PushService {
 	var svc = &PushService{}
 	svc.Log = log
 	svc.Bus = bus
+	svc.regURL = setup.RegURL
+	svc.deviceId = setup.DeviceId
+	svc.authGetter = setup.AuthGetter
 	return svc
 }
 
-// SetRegistrationURL() sets the registration url for the service
-func (svc *PushService) SetRegistrationURL(url string) {
-	svc.lock.Lock()
-	defer svc.lock.Unlock()
-	svc.regURL = url
-}
-
-// SetAuthGetter() sets the authorization getter for the service
-func (svc *PushService) SetAuthGetter(authGetter func(string) string) {
-	svc.lock.Lock()
-	defer svc.lock.Unlock()
-	svc.authGetter = authGetter
-}
-
-// getRegistrationAuthorization() returns the authorization header for
-// POSTing to the registration HTTP endpoint
-//
-// (this is for calling with the lock held)
-func (svc *PushService) getRegistrationAuthorization() string {
-	if svc.authGetter != nil && svc.regURL != "" {
-		return svc.authGetter(svc.regURL)
-	} else {
-		return ""
-	}
-}
-
-// GetRegistrationAuthorization() returns the authorization header for
-// POSTing to the registration HTTP endpoint
-func (svc *PushService) GetRegistrationAuthorization() string {
-	svc.lock.RLock()
-	defer svc.lock.RUnlock()
-	return svc.getRegistrationAuthorization()
-}
-
-// SetDeviceId() sets the device id
-func (svc *PushService) SetDeviceId(deviceId string) {
-	svc.lock.Lock()
-	defer svc.lock.Unlock()
-	svc.deviceId = deviceId
-}
-
-// GetDeviceId() returns the device id
-func (svc *PushService) GetDeviceId() string {
-	svc.lock.RLock()
-	defer svc.lock.RUnlock()
-	return svc.deviceId
+// getAuthorization() returns the URL and the authorization header for
+// POSTing to the registration HTTP endpoint for op
+func (svc *PushService) getAuthorization(op string) (string, string) {
+	if svc.authGetter == nil || svc.regURL == nil {
+		return "", ""
+	}
+	purl, err := svc.regURL.Parse(op)
+	if err != nil {
+		panic("op to getAuthorization was invalid")
+	}
+	url := purl.String()
+	return url, svc.authGetter(url)
 }
 
 func (svc *PushService) Start() error {
@@ -130,32 +107,21 @@
 	Message string `json:"message"` //
 }
 
-func (svc *PushService) register(path string, args, _ []interface{}) ([]interface{}, error) {
-	svc.lock.RLock()
-	defer svc.lock.RUnlock()
-	if len(args) != 0 {
-		return nil, BadArgCount
-	}
-	raw_appname := path[strings.LastIndex(path, "/")+1:]
-	appname := string(nih.Unquote([]byte(raw_appname)))
-
-	rv := os.Getenv("PUSH_REG_" + raw_appname)
-	if rv != "" {
-		return []interface{}{rv}, nil
-	}
-
-	req_body, err := json.Marshal(registrationRequest{svc.deviceId, appname})
+func (svc *PushService) manageReg(op, appId string) (*registrationReply, error) {
+	req_body, err := json.Marshal(registrationRequest{svc.deviceId, appId})
 	if err != nil {
 		return nil, fmt.Errorf("unable to marshal register request body: %v", err)
 	}
-	req, err := http13.NewRequest("POST", svc.regURL, bytes.NewReader(req_body))
-	if err != nil {
-		return nil, fmt.Errorf("unable to build register request: %v", err)
-	}
-	auth := svc.getRegistrationAuthorization()
+
+	url, auth := svc.getAuthorization(op)
 	if auth == "" {
 		return nil, BadAuth
 	}
+
+	req, err := http13.NewRequest("POST", url, bytes.NewReader(req_body))
+	if err != nil {
+		panic(fmt.Errorf("unable to build register request: %v", err))
+	}
 	req.Header.Add("Authorization", auth)
 	req.Header.Add("Content-Type", "application/json")
 
@@ -188,6 +154,26 @@
 		return nil, fmt.Errorf("unable to unmarshal register response: %v", err)
 	}
 
+	return &reply, nil
+}
+
+func (svc *PushService) register(path string, args, _ []interface{}) ([]interface{}, error) {
+	if len(args) != 0 {
+		return nil, BadArgCount
+	}
+	raw_appname := path[strings.LastIndex(path, "/")+1:]
+	appname := string(nih.Unquote([]byte(raw_appname)))
+
+	rv := os.Getenv("PUSH_REG_" + raw_appname)
+	if rv != "" {
+		return []interface{}{rv}, nil
+	}
+
+	reply, err := svc.manageReg("/register", appname)
+	if err != nil {
+		return nil, err
+	}
+
 	if !reply.Ok || reply.Token == "" {
 		svc.Log.Errorf("Unexpected response: %#v", reply)
 		return nil, BadToken

=== modified file 'client/service/service_test.go'
--- client/service/service_test.go	2014-06-20 12:33:03 +0000
+++ client/service/service_test.go	2014-07-03 14:25:59 +0000
@@ -47,8 +47,25 @@
 	ss.bus = testibus.NewTestingEndpoint(condition.Work(true), nil)
 }
 
+var testSetup = &PushServiceSetup{}
+
+func (ss *serviceSuite) TestBuild(c *C) {
+	setup := &PushServiceSetup{
+		RegURL:   helpers.ParseURL("http://reg"),
+		DeviceId: "FOO",
+		AuthGetter: func(s string) string {
+			return ""
+		},
+	}
+	svc := NewPushService(ss.bus, setup, ss.log)
+	c.Check(svc.Bus, Equals, ss.bus)
+	c.Check(svc.regURL, DeepEquals, helpers.ParseURL("http://reg"))
+	c.Check(fmt.Sprintf("%#v", svc.authGetter), Equals, fmt.Sprintf("%#v", setup.AuthGetter))
+	// ...
+}
+
 func (ss *serviceSuite) TestStart(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
+	svc := NewPushService(ss.bus, testSetup, ss.log)
 	c.Check(svc.IsRunning(), Equals, false)
 	c.Check(svc.Start(), IsNil)
 	c.Check(svc.IsRunning(), Equals, true)
@@ -56,31 +73,31 @@
 }
 
 func (ss *serviceSuite) TestStartTwice(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
+	svc := NewPushService(ss.bus, testSetup, ss.log)
 	c.Check(svc.Start(), IsNil)
 	c.Check(svc.Start(), Equals, AlreadyStarted)
 	svc.Stop()
 }
 
 func (ss *serviceSuite) TestStartNoLog(c *C) {
-	svc := NewPushService(ss.bus, nil)
+	svc := NewPushService(ss.bus, testSetup, nil)
 	c.Check(svc.Start(), Equals, NotConfigured)
 }
 
 func (ss *serviceSuite) TestStartNoBus(c *C) {
-	svc := NewPushService(nil, ss.log)
+	svc := NewPushService(nil, testSetup, ss.log)
 	c.Check(svc.Start(), Equals, NotConfigured)
 }
 
 func (ss *serviceSuite) TestStartFailsOnBusDialFailure(c *C) {
 	bus := testibus.NewTestingEndpoint(condition.Work(false), nil)
-	svc := NewPushService(bus, ss.log)
+	svc := NewPushService(bus, testSetup, ss.log)
 	c.Check(svc.Start(), ErrorMatches, `.*(?i)cond said no.*`)
 	svc.Stop()
 }
 
 func (ss *serviceSuite) TestStartGrabsName(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
+	svc := NewPushService(ss.bus, testSetup, ss.log)
 	c.Assert(svc.Start(), IsNil)
 	callArgs := testibus.GetCallArgs(ss.bus)
 	defer svc.Stop()
@@ -89,7 +106,7 @@
 }
 
 func (ss *serviceSuite) TestStopClosesBus(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
+	svc := NewPushService(ss.bus, testSetup, ss.log)
 	c.Assert(svc.Start(), IsNil)
 	svc.Stop()
 	callArgs := testibus.GetCallArgs(ss.bus)
@@ -99,35 +116,27 @@
 
 // registration tests
 
-func (ss *serviceSuite) TestSetRegURLWorks(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
-	c.Check(svc.regURL, Equals, "")
-	svc.SetRegistrationURL("xyzzy://")
-	c.Check(svc.regURL, Equals, "xyzzy://")
-}
-
-func (ss *serviceSuite) TestSetAuthGetterWorks(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
-	c.Check(svc.authGetter, IsNil)
-	f := func(string) string { return "" }
-	svc.SetAuthGetter(f)
-	c.Check(fmt.Sprintf("%#v", svc.authGetter), Equals, fmt.Sprintf("%#v", f))
-}
-
 func (ss *serviceSuite) TestGetRegAuthWorks(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
-	svc.SetRegistrationURL("xyzzy://")
 	ch := make(chan string, 1)
-	f := func(s string) string { ch <- s; return "Auth " + s }
-	svc.SetAuthGetter(f)
-	c.Check(svc.getRegistrationAuthorization(), Equals, "Auth xyzzy://")
+	setup := &PushServiceSetup{
+		RegURL: helpers.ParseURL("http://foo"),
+		AuthGetter: func(s string) string {
+			ch <- s
+			return "Auth " + s
+		},
+	}
+	svc := NewPushService(ss.bus, setup, ss.log)
+	url, auth := svc.getAuthorization("/op")
+	c.Check(auth, Equals, "Auth http://foo/op")
 	c.Assert(len(ch), Equals, 1)
-	c.Check(<-ch, Equals, "xyzzy://")
+	c.Check(<-ch, Equals, "http://foo/op")
+	c.Check(url, Equals, "http://foo/op")
 }
 
 func (ss *serviceSuite) TestGetRegAuthDoesNotPanic(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
-	c.Check(svc.getRegistrationAuthorization(), Equals, "")
+	svc := NewPushService(ss.bus, testSetup, ss.log)
+	_, auth := svc.getAuthorization("/op")
+	c.Check(auth, Equals, "")
 }
 
 func (ss *serviceSuite) TestRegistrationFailsIfBadArgs(c *C) {
@@ -149,11 +158,12 @@
 		fmt.Fprintln(w, `{"ok":true,"token":"blob-of-bytes"}`)
 	}))
 	defer ts.Close()
-
-	svc := NewPushService(ss.bus, ss.log)
-	svc.SetAuthGetter(func(string) string { return "tok" })
-	svc.SetRegistrationURL(ts.URL)
-	svc.SetDeviceId("fake-device-id")
+	setup := &PushServiceSetup{
+		DeviceId:   "fake-device-id",
+		RegURL:     helpers.ParseURL(ts.URL),
+		AuthGetter: func(string) string { return "tok" },
+	}
+	svc := NewPushService(ss.bus, setup, ss.log)
 	// this'll check (un)quoting, too
 	reg, err := svc.register("/an_2dapp_2did", nil, nil)
 	c.Assert(err, IsNil)
@@ -175,60 +185,59 @@
 	c.Check(err, IsNil)
 }
 
-func (ss *serviceSuite) TestRegistrationFailsOnBadReqURL(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
-	svc.SetRegistrationURL("%gh")
-	reg, err := svc.register("thing", nil, nil)
-	c.Check(reg, IsNil)
-	c.Check(err, ErrorMatches, "unable to build register request: .*")
-}
-
-func (ss *serviceSuite) TestRegistrationFailsOnBadAuth(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
+func (ss *serviceSuite) TestManageRegFailsOnBadAuth(c *C) {
 	// ... no auth added
+	svc := NewPushService(ss.bus, testSetup, ss.log)
 	reg, err := svc.register("thing", nil, nil)
 	c.Check(reg, IsNil)
 	c.Check(err, Equals, BadAuth)
 }
 
-func (ss *serviceSuite) TestRegistrationFailsOnNoServer(c *C) {
-	svc := NewPushService(ss.bus, ss.log)
-	svc.SetRegistrationURL("xyzzy://")
-	svc.SetAuthGetter(func(string) string { return "tok" })
+func (ss *serviceSuite) TestManageRegFailsOnNoServer(c *C) {
+	setup := &PushServiceSetup{
+		DeviceId:   "fake-device-id",
+		RegURL:     helpers.ParseURL("xyzzy://"),
+		AuthGetter: func(string) string { return "tok" },
+	}
+	svc := NewPushService(ss.bus, setup, ss.log)
 	reg, err := svc.register("thing", nil, nil)
 	c.Check(reg, IsNil)
 	c.Check(err, ErrorMatches, "unable to request registration: .*")
 }
 
-func (ss *serviceSuite) TestRegistrationFailsOn40x(c *C) {
+func (ss *serviceSuite) TestManageRegFailsOn40x(c *C) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "I'm a teapot", 418)
 	}))
 	defer ts.Close()
-
-	svc := NewPushService(ss.bus, ss.log)
-	svc.SetAuthGetter(func(string) string { return "tok" })
-	svc.SetRegistrationURL(ts.URL)
+	setup := &PushServiceSetup{
+		DeviceId:   "fake-device-id",
+		RegURL:     helpers.ParseURL(ts.URL),
+		AuthGetter: func(string) string { return "tok" },
+	}
+	svc := NewPushService(ss.bus, setup, ss.log)
 	reg, err := svc.register("/thing", nil, nil)
 	c.Check(err, Equals, BadRequest)
 	c.Check(reg, IsNil)
 }
 
-func (ss *serviceSuite) TestRegistrationFailsOn50x(c *C) {
+func (ss *serviceSuite) TestManageRegFailsOn50x(c *C) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "Not implemented", 501)
 	}))
 	defer ts.Close()
-
-	svc := NewPushService(ss.bus, ss.log)
-	svc.SetAuthGetter(func(string) string { return "tok" })
-	svc.SetRegistrationURL(ts.URL)
+	setup := &PushServiceSetup{
+		DeviceId:   "fake-device-id",
+		RegURL:     helpers.ParseURL(ts.URL),
+		AuthGetter: func(string) string { return "tok" },
+	}
+	svc := NewPushService(ss.bus, setup, ss.log)
 	reg, err := svc.register("/thing", nil, nil)
 	c.Check(err, Equals, BadServer)
 	c.Check(reg, IsNil)
 }
 
-func (ss *serviceSuite) TestRegistrationFailsOnBadJSON(c *C) {
+func (ss *serviceSuite) TestManageRegFailsOnBadJSON(c *C) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		buf := make([]byte, 256)
 		n, e := r.Body.Read(buf)
@@ -241,18 +250,19 @@
 		fmt.Fprintln(w, `{`)
 	}))
 	defer ts.Close()
-
-	svc := NewPushService(ss.bus, ss.log)
-	svc.SetAuthGetter(func(string) string { return "tok" })
-	svc.SetRegistrationURL(ts.URL)
-	svc.SetDeviceId("fake-device-id")
+	setup := &PushServiceSetup{
+		DeviceId:   "fake-device-id",
+		RegURL:     helpers.ParseURL(ts.URL),
+		AuthGetter: func(string) string { return "tok" },
+	}
+	svc := NewPushService(ss.bus, setup, ss.log)
 	// this'll check (un)quoting, too
 	reg, err := svc.register("/an_2dapp_2did", nil, nil)
 	c.Check(reg, IsNil)
 	c.Check(err, ErrorMatches, "unable to unmarshal register response: .*")
 }
 
-func (ss *serviceSuite) TestRegistrationFailsOnBadJSONDocument(c *C) {
+func (ss *serviceSuite) TestManageRegFailsOnBadJSONDocument(c *C) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		buf := make([]byte, 256)
 		n, e := r.Body.Read(buf)
@@ -265,11 +275,12 @@
 		fmt.Fprintln(w, `{"bananas": "very yes"}`)
 	}))
 	defer ts.Close()
-
-	svc := NewPushService(ss.bus, ss.log)
-	svc.SetAuthGetter(func(string) string { return "tok" })
-	svc.SetRegistrationURL(ts.URL)
-	svc.SetDeviceId("fake-device-id")
+	setup := &PushServiceSetup{
+		DeviceId:   "fake-device-id",
+		RegURL:     helpers.ParseURL(ts.URL),
+		AuthGetter: func(string) string { return "tok" },
+	}
+	svc := NewPushService(ss.bus, setup, ss.log)
 	// this'll check (un)quoting, too
 	reg, err := svc.register("/an_2dapp_2did", nil, nil)
 	c.Check(reg, IsNil)

=== modified file 'debian/config.json'
--- debian/config.json	2014-06-19 21:12:04 +0000
+++ debian/config.json	2014-07-03 14:25:59 +0000
@@ -1,7 +1,7 @@
 {
     "auth_helper": "/usr/lib/ubuntu-push-client/signing-helper",
     "session_url": "https://push.ubuntu.com/",
-    "registration_url": "https://push.ubuntu.com/register",
+    "registration_url": "https://push.ubuntu.com",
     "connect_timeout": "20s",
     "exchange_timeout": "30s",
     "hosts_cache_expiry": "12h",

=== modified file 'server/session/session_test.go'
--- server/session/session_test.go	2014-06-19 13:06:52 +0000
+++ server/session/session_test.go	2014-07-03 14:25:59 +0000
@@ -373,6 +373,7 @@
 		pingTimer:     time.NewTimer(pingInterval),
 		intervalStart: now,
 	}
+	time.Sleep(10 * time.Millisecond)
 	l.pingTimer.Stop()
 	done := l.pingTimerReset(true)
 	c.Assert(done, Equals, true)

=== modified file 'testing/helpers.go'
--- testing/helpers.go	2014-06-23 12:46:28 +0000
+++ testing/helpers.go	2014-07-03 14:25:59 +0000
@@ -20,6 +20,7 @@
 import (
 	"encoding/json"
 	"fmt"
+	"net/url"
 	"os"
 	"path"
 	"path/filepath"
@@ -160,3 +161,12 @@
 	}
 	return res
 }
+
+// ParseURL parses a URL conveniently.
+func ParseURL(s string) *url.URL {
+	purl, err := url.Parse(s)
+	if err != nil {
+		panic(err)
+	}
+	return purl
+}