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

[Samuele Pedroni]

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

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

217. By Samuele Pedroni on 2014-07-03

wait for inc elapsed time

218. By Samuele Pedroni on 2014-07-03

Unregister method

219. By Samuele Pedroni on 2014-07-03

Merged ubuntu-push into unregister.

220. By Samuele Pedroni on 2014-07-03

tweak test

Unmerged revisions

=== modified file 'client/client.go'
--- client/client.go	2014-07-01 11:55:30 +0000
+++ client/client.go	2014-07-03 14:00:43 +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)
@@ -320,9 +334,7 @@
 	if !client.filterBroadcastNotification(msg) {
 		return nil
 	}
-	not_id, err := client.postalService.SendNotification(service.ACTION_ID_BROADCAST,
-		"update_manager_icon", "There's an updated system image.",
-		"Tap to open the system updater.")
+	not_id, err := client.postalService.InjectBroadcast()
 	if err != nil {
 		client.log.Errorf("showing notification: %s", err)
 		return err
@@ -338,15 +350,20 @@
 }
 
 // handleClick deals with the user clicking a notification
-func (client *PushClient) handleClick(action_id string) error {
+func (client *PushClient) handleClick(actionId string) error {
 	// “The string is a stark data structure and everywhere it is passed
 	// there is much duplication of process. It is a perfect vehicle for
 	// hiding information.”
 	//
 	// From ACM's SIGPLAN publication, (September, 1982), Article
 	// "Epigrams in Programming", by Alan J. Perlis of Yale University.
-	url := strings.TrimPrefix(action_id, service.ACTION_ID_SNOWFLAKE)
-	if len(url) == len(action_id) || len(url) == 0 {
+	url := actionId
+	// XXX: branch for the broadcast notifications
+	if strings.HasPrefix(actionId, service.ACTION_ID_PREFIX) {
+		parts := strings.Split(actionId, "::")
+		url = parts[1]
+	}
+	if len(url) == len(actionId) || len(url) == 0 {
 		// it didn't start with the prefix
 		return nil
 	}
@@ -396,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)
 	}
@@ -403,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-01 11:55:30 +0000
+++ client/client_test.go	2014-07-03 14:00:43 +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)
@@ -712,6 +767,8 @@
     handleClick tests
 ******************************************************************/
 
+var ACTION_ID_BROADCAST = service.ACTION_ID_PREFIX + service.SystemUpdateUrl + service.ACTION_ID_SUFFIX
+
 func (cs *clientSuite) TestHandleClick(c *C) {
 	cli := NewPushClient(cs.configPath, cs.leveldbPath)
 	cli.log = cs.log
@@ -723,14 +780,14 @@
 	args := testibus.GetCallArgs(endp)
 	c.Assert(args, HasLen, 0)
 	// check we worked with the right action id
-	c.Check(cli.handleClick(service.ACTION_ID_BROADCAST), IsNil)
+	c.Check(cli.handleClick(ACTION_ID_BROADCAST), 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{}{service.SystemUpdateUrl})
 	// check we worked with the right action id
-	c.Check(cli.handleClick(service.ACTION_ID_SNOWFLAKE+"foo"), IsNil)
+	c.Check(cli.handleClick(service.ACTION_ID_PREFIX+"foo"), IsNil)
 	// check we sent the notification
 	args = testibus.GetCallArgs(endp)
 	c.Assert(args, HasLen, 2)
@@ -879,7 +936,7 @@
 	c.Check(cs.log.Captured(), Matches, "(?ms).*Session connected after 42 attempts$")
 
 	//  * actionsCh to the click handler/url dispatcher
-	aCh <- notifications.RawActionReply{ActionId: service.ACTION_ID_BROADCAST}
+	aCh <- notifications.RawActionReply{ActionId: ACTION_ID_BROADCAST}
 	tick()
 	uargs := testibus.GetCallArgs(cli.urlDispatcherEndp)
 	c.Assert(uargs, HasLen, 1)

=== modified file 'client/service/postal.go'
--- client/service/postal.go	2014-07-01 11:55:30 +0000
+++ client/service/postal.go	2014-07-03 14:00:43 +0000
@@ -20,7 +20,6 @@
 	"strings"
 
 	"code.google.com/p/go-uuid/uuid"
-	"launchpad.net/go-dbus/v1"
 
 	"launchpad.net/ubuntu-push/bus"
 	"launchpad.net/ubuntu-push/bus/notifications"
@@ -50,9 +49,9 @@
 )
 
 var (
-	SystemUpdateUrl     = "settings:///system/system-update"
-	ACTION_ID_SNOWFLAKE = "::ubuntu-push-client::"
-	ACTION_ID_BROADCAST = ACTION_ID_SNOWFLAKE + SystemUpdateUrl
+	SystemUpdateUrl  = "settings:///system/system-update"
+	ACTION_ID_PREFIX = "ubuntu-push-client::"
+	ACTION_ID_SUFFIX = "::0"
 )
 
 // NewPostalService() builds a new service and returns it.
@@ -90,9 +89,7 @@
 }
 
 func (svc *PostalService) TakeTheBus() (<-chan notifications.RawActionReply, error) {
-	iniCh := make(chan uint32)
-	go func() { iniCh <- util.NewAutoRedialer(svc.notificationsEndp).Redial() }()
-	<-iniCh
+	util.NewAutoRedialer(svc.notificationsEndp).Redial()
 	actionsCh, err := notifications.Raw(svc.notificationsEndp, svc.Log).WatchActions()
 	return actionsCh, err
 }
@@ -164,18 +161,14 @@
 	return err
 }
 
-func (svc *PostalService) SendNotification(action_id, icon, summary, body string) (uint32, error) {
-	a := []string{action_id, "Switch to app"} // action value not visible on the phone
-	h := map[string]*dbus.Variant{"x-canonical-switch-to-application": &dbus.Variant{true}}
-	nots := notifications.Raw(svc.notificationsEndp, svc.Log)
-	return nots.Notify(
-		"ubuntu-push-client", // app name
-		uint32(0),            // id
-		icon,                 // icon
-		summary,              // summary
-		body,                 // body
-		a,                    // actions
-		h,                    // hints
-		int32(10*1000),       // timeout (ms)
-	)
+func (svc *PostalService) InjectBroadcast() (uint32, error) {
+	// XXX: call a helper?
+	// XXX: Present force us to send the url as the notificationId
+	icon := "update_manager_icon"
+	summary := "There's an updated system image."
+	body := "Tap to open the system updater."
+	actions := []string{"Switch to app"} // action value not visible on the phone
+	card := &launch_helper.Card{Icon: icon, Summary: summary, Body: body, Actions: actions, Popup: true}
+	output := &launch_helper.HelperOutput{[]byte(""), &launch_helper.Notification{Card: card}}
+	return 0, svc.msgHandler("ubuntu-push-client", SystemUpdateUrl, output)
 }

=== modified file 'client/service/postal_test.go'
--- client/service/postal_test.go	2014-07-01 11:55:30 +0000
+++ client/service/postal_test.go	2014-07-03 14:00:43 +0000
@@ -157,6 +157,38 @@
 }
 
 //
+// Injection (Broadcast) tests
+
+func (ss *postalSuite) TestInjectBroadcast(c *C) {
+	bus := testibus.NewTestingEndpoint(nil, condition.Work(true), uint32(1))
+	svc := NewPostalService(ss.bus, bus, ss.log)
+	//svc.msgHandler = nil
+	rvs, err := svc.InjectBroadcast()
+	c.Assert(err, IsNil)
+	c.Check(rvs, Equals, uint32(0))
+	c.Assert(err, IsNil)
+	// and check it fired the right signal (twice)
+	callArgs := testibus.GetCallArgs(bus)
+	c.Assert(callArgs, HasLen, 1)
+	c.Check(callArgs[0].Member, Equals, "Notify")
+	c.Check(callArgs[0].Args[0:6], DeepEquals, []interface{}{"ubuntu-push-client", uint32(0), "update_manager_icon",
+		"There's an updated system image.", "Tap to open the system updater.",
+		[]string{"ubuntu-push-client::settings:///system/system-update::0", "Switch to app"}})
+	// TODO: check the map in callArgs?
+	// c.Check(callArgs[0].Args[7]["x-canonical-secondary-icon"], NotNil)
+	// c.Check(callArgs[0].Args[7]["x-canonical-snap-decisions"], NotNil)
+}
+
+func (ss *postalSuite) TestInjectBroadcastFails(c *C) {
+	bus := testibus.NewTestingEndpoint(condition.Work(true),
+		condition.Work(false))
+	svc := NewPostalService(ss.bus, bus, ss.log)
+	svc.SetMessageHandler(func(string, string, *launch_helper.HelperOutput) error { return errors.New("fail") })
+	_, err := svc.InjectBroadcast()
+	c.Check(err, NotNil)
+}
+
+//
 // Notifications tests
 func (ss *postalSuite) TestNotificationsWorks(c *C) {
 	svc := NewPostalService(ss.bus, ss.notifBus, ss.log)

=== 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:00:43 +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:00:43 +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:00:43 +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 'testing/helpers.go'
--- testing/helpers.go	2014-06-23 12:46:28 +0000
+++ testing/helpers.go	2014-07-03 14:00:43 +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
+}