Merge lp:~chipaca/ubuntu-push/url-dispatcher into lp:ubuntu-push

[John Lenton]

URLDispatcher (and extended endpoint.Call to return []interface{})

URLDispatcher (and extended endpoint.Call to return []interface{})

[Samuele Pedroni (pedronis)]

:)

70 + return nil, errors.New("Wrong number of arguments in reply to GetProperty")

this looks like the old confusing error message

this guy is less covered than it was:

launchpad.net/ubuntu-push/bus/testing/testing_endpoint.go: GetProperty 66.7%

[lp:~chipaca/ubuntu-push/url-dispatcher]

20. By John Lenton on 2014-01-27

improved testingEndpoint's GetProperty error message

[John Lenton (chipaca)]

Improved the error message (to “Wrong number of values given to testingEndpoint -- GetProperty only returns a single value for now!”).
The coverage is taken to 100% in the “redialer” branch.

[lp:~chipaca/ubuntu-push/url-dispatcher]

21. By John Lenton on 2014-01-27

improved error message for when Notify gets the wrong message back from the bus

[Samuele Pedroni (pedronis)]

review: Approve

=== modified file 'bus/endpoint.go'
--- bus/endpoint.go	2014-01-22 10:48:52 +0000
+++ bus/endpoint.go	2014-01-27 10:47:45 +0000
@@ -31,7 +31,7 @@
 // bus.Endpoint represents the DBus connection itself.
 type Endpoint interface {
 	WatchSignal(member string, f func(...interface{}), d func()) error
-	Call(member string, args ...interface{}) (interface{}, error)
+	Call(member string, args ...interface{}) ([]interface{}, error)
 	GetProperty(property string) (interface{}, error)
 	Close()
 }
@@ -81,20 +81,13 @@
 // Call() invokes the provided member method (on the name, path and interface
 // provided when creating the endpoint). The return value is unpacked before
 // being returned.
-func (endp *endpoint) Call(member string, args ...interface{}) (interface{}, error) {
+func (endp *endpoint) Call(member string, args ...interface{}) ([]interface{}, error) {
 	msg, err := endp.proxy.Call(endp.iface, member, args...)
 	if err != nil {
-		return 0, err
+		return nil, err
 	}
 	rvs := endp.unpackOneMsg(msg, member)
-	switch len(rvs) {
-	default:
-		return 0, fmt.Errorf("Too many values in %s response: %d", member, len(rvs))
-	case 0:
-		return 0, fmt.Errorf("Not enough values in %s response: %d", member, len(rvs))
-	case 1:
-		return rvs[0], nil
-	}
+	return rvs, nil
 }
 
 // GetProperty uses the org.freedesktop.DBus.Properties interface's Get method

=== modified file 'bus/testing/testing_endpoint.go'
--- bus/testing/testing_endpoint.go	2014-01-22 10:48:52 +0000
+++ bus/testing/testing_endpoint.go	2014-01-27 10:47:45 +0000
@@ -66,23 +66,28 @@
 
 // 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) {
+func (tc *testingEndpoint) Call(member string, args ...interface{}) ([]interface{}, error) {
 	if tc.cond.OK() {
 		if len(tc.retvals) == 0 {
 			panic("No return values provided!")
 		}
-		if len(tc.retvals[0]) != 1 {
-			panic("Wrong number of values provided -- Call only returns a single value for now!")
-		}
-		return tc.retvals[0][0], nil
+		return tc.retvals[0], nil
 	} else {
-		return 0, errors.New("no way")
+		return nil, errors.New("no way")
 	}
 }
 
 // See Endpoint's GetProperty. This one is just another name for Call.
 func (tc *testingEndpoint) GetProperty(property string) (interface{}, error) {
-	return tc.Call(property)
+	rvs, err := tc.Call(property)
+	if err != nil {
+		return nil, err
+	}
+	if len(rvs) != 1 {
+		return nil, errors.New("Wrong number of values given to testingEndpoint" +
+			" -- GetProperty only returns a single value for now!")
+	}
+	return rvs[0], err
 }
 
 // see Endpoint's Close. This one does nothing.

=== modified file 'bus/testing/testing_endpoint_test.go'
--- bus/testing/testing_endpoint_test.go	2014-01-21 13:38:03 +0000
+++ bus/testing/testing_endpoint_test.go	2014-01-27 10:47:45 +0000
@@ -34,18 +34,20 @@
 func (s *TestingEndpointSuite) TestCallReturnsFirstRetval(c *C) {
 	var m, n uint32 = 42, 17
 	endp := NewTestingEndpoint(condition.Work(true), m, n)
-	v, e := endp.Call("what")
+	vs, e := endp.Call("what")
 	c.Check(e, IsNil)
-	c.Check(v, Equals, m)
+	c.Check(vs, HasLen, 1)
+	c.Check(vs[0], Equals, m)
 }
 
 // Test the same Call() but with multi-valued endpoint
 func (s *TestingEndpointSuite) TestMultiValuedCall(c *C) {
 	var m, n uint32 = 42, 17
 	endp := NewMultiValuedTestingEndpoint(condition.Work(true), []interface{}{m}, []interface{}{n})
-	v, e := endp.Call("what")
+	vs, e := endp.Call("what")
 	c.Check(e, IsNil)
-	c.Check(v, Equals, m)
+	c.Check(vs, HasLen, 1)
+	c.Check(vs[0], Equals, m)
 }
 
 // Test that Call() with a negative condition returns an error.
@@ -62,21 +64,6 @@
 	c.Check(func() { endp.Call("") }, PanicMatches, "No return values provided.*")
 }
 
-// Test that Call() with a positive condition and an empty return value panics
-// with a helpful message.
-func (s *TestingEndpointSuite) TestCallPanicsWithNiceMessage2(c *C) {
-	endp := NewMultiValuedTestingEndpoint(condition.Work(true), []interface{}{})
-	c.Check(func() { endp.Call("") }, PanicMatches, "Wrong number of values provided.*")
-}
-
-// Test Call() with positive condition and the wrong number of arguments also
-// fails with a helpful message
-func (s *TestingEndpointSuite) TestMultiValuedCallPanicsWhenWrongNumberOfValues(c *C) {
-	var m, n uint32 = 42, 17
-	endp := NewMultiValuedTestingEndpoint(condition.Work(true), []interface{}{m, n})
-	c.Check(func() { endp.Call("") }, PanicMatches, "Wrong number of values provided.*")
-}
-
 // Test that WatchSignal() with a positive condition sends the provided return
 // values over the channel.
 func (s *TestingEndpointSuite) TestWatch(c *C) {

=== modified file 'networkmanager/networkmanager_test.go'
--- networkmanager/networkmanager_test.go	2014-01-20 13:44:58 +0000
+++ networkmanager/networkmanager_test.go	2014-01-27 10:47:45 +0000
@@ -65,13 +65,20 @@
 	c.Check(state, Equals, Unknown)
 }
 
-// GetState returns the right state when dbus works but delivers rubbish
-func (s *NMSuite) TestGetStateRubbish(c *C) {
+// GetState returns the right state when dbus works but delivers rubbish values
+func (s *NMSuite) TestGetStateRubbishValues(c *C) {
 	nm := New(testingbus.NewTestingEndpoint(condition.Work(false), 42), nullog)
 	state := nm.GetState()
 	c.Check(state, Equals, Unknown)
 }
 
+// GetState returns the right state when dbus works but delivers a rubbish structure
+func (s *NMSuite) TestGetStateRubbishStructure(c *C) {
+	nm := New(testingbus.NewMultiValuedTestingEndpoint(condition.Work(true), []interface{}{}), nullog)
+	state := nm.GetState()
+	c.Check(state, Equals, Unknown)
+}
+
 // WatchState sends a stream of States over the channel
 func (s *NMSuite) TestWatchState(c *C) {
 	tc := testingbus.NewTestingEndpoint(condition.Work(true), uint32(Unknown), uint32(Asleep), uint32(ConnectedGlobal))

=== modified file 'notifications/raw.go'
--- notifications/raw.go	2014-01-22 12:15:12 +0000
+++ notifications/raw.go	2014-01-27 10:47:45 +0000
@@ -22,6 +22,7 @@
 // this is the lower-level api
 
 import (
+	"fmt"
 	"launchpad.net/go-dbus/v1"
 	"launchpad.net/ubuntu-push/bus"
 	"launchpad.net/ubuntu-push/logger"
@@ -73,12 +74,16 @@
 	timeout int32) (uint32, error) {
 	// that's a long argument list! Take a breather.
 	//
-	rv, err := raw.bus.Call("Notify", app_name, reuse_id, icon,
+	rvs, err := raw.bus.Call("Notify", app_name, reuse_id, icon,
 		summary, body, actions, hints, timeout)
 	if err != nil {
 		return 0, err
 	}
-	return rv.(uint32), nil
+	if len(rvs) != 1 {
+		return 0, fmt.Errorf("Wrong number of values in Notify response: %d",
+			len(rvs))
+	}
+	return rvs[0].(uint32), nil
 }
 
 // WatchActions listens for ActionInvoked signals from the notification daemon

=== modified file 'notifications/raw_test.go'
--- notifications/raw_test.go	2014-01-21 14:58:28 +0000
+++ notifications/raw_test.go	2014-01-27 10:47:45 +0000
@@ -65,6 +65,15 @@
 	c.Check(err, NotNil)
 }
 
+func (s *RawSuite) TestNotifiesFailsWeirdly(c *C) {
+	bus := testibus.NewMultiValuedTestingBus(condition.Work(true), condition.Work(true),
+		[]interface{}{1, 2})
+	raw, err := Raw(bus, nullog)
+	c.Assert(err, IsNil)
+	_, err = raw.Notify("", 0, "", "", "", nil, nil, 0)
+	c.Check(err, NotNil)
+}
+
 func (s *RawSuite) TestWatchActions(c *C) {
 	bus := testibus.NewMultiValuedTestingBus(condition.Work(true), condition.Work(true),
 		[]interface{}{uint32(1), "hello"})

=== added directory 'urldispatcher'
=== added file 'urldispatcher/urldispatcher.go'
--- urldispatcher/urldispatcher.go	1970-01-01 00:00:00 +0000
+++ urldispatcher/urldispatcher.go	2014-01-27 10:47:45 +0000
@@ -0,0 +1,61 @@
+/*
+ Copyright 2013-2014 Canonical Ltd.
+
+ This program is free software: you can redistribute it and/or modify it
+ under the terms of the GNU General Public License version 3, as published
+ by the Free Software Foundation.
+
+ This program is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranties of
+ MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ PURPOSE.  See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along
+ with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+// Package urldispatcher wraps the url dispatcher's dbus api point
+package urldispatcher
+
+import (
+	"launchpad.net/ubuntu-push/bus"
+	"launchpad.net/ubuntu-push/logger"
+)
+
+// UrlDispatcher lives on a well-known bus.Address
+var BusAddress bus.Address = bus.Address{
+	Interface: "com.canonical.URLDispatcher",
+	Path:      "/com/canonical/URLDispatcher",
+	Name:      "com.canonical.URLDispatcher",
+}
+
+// A URLDispatcher is a simple beast, with a single method that does what it
+// says on the box.
+type URLDispatcher interface {
+	DispatchURL(string) error
+}
+
+type urlDispatcher struct {
+	endp bus.Endpoint
+	log  logger.Logger
+}
+
+// New builds a new URL dispatcher by connecting to the provided bus.
+func New(bus bus.Bus, log logger.Logger) (URLDispatcher, error) {
+	endp, err := bus.Connect(BusAddress, log)
+	if err != nil {
+		return nil, err
+	}
+	return &urlDispatcher{endp, log}, nil
+}
+
+var _ URLDispatcher = &urlDispatcher{} // ensures it conforms
+
+func (ud *urlDispatcher) DispatchURL(url string) error {
+	ud.log.Debugf("Dispatching %s", url)
+	_, err := ud.endp.Call("DispatchURL", url)
+	if err != nil {
+		ud.log.Errorf("Dispatch to %s failed with %s", url, err)
+	}
+	return err
+}

=== added file 'urldispatcher/urldispatcher_test.go'
--- urldispatcher/urldispatcher_test.go	1970-01-01 00:00:00 +0000
+++ urldispatcher/urldispatcher_test.go	2014-01-27 10:47:45 +0000
@@ -0,0 +1,57 @@
+/*
+ Copyright 2013-2014 Canonical Ltd.
+
+ This program is free software: you can redistribute it and/or modify it
+ under the terms of the GNU General Public License version 3, as published
+ by the Free Software Foundation.
+
+ This program is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranties of
+ MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ PURPOSE.  See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along
+ with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+package urldispatcher
+
+import (
+	"io/ioutil"
+	. "launchpad.net/gocheck"
+	testibus "launchpad.net/ubuntu-push/bus/testing"
+	"launchpad.net/ubuntu-push/logger"
+	"launchpad.net/ubuntu-push/testing/condition"
+	"testing"
+)
+
+// hook up gocheck
+func TestUrldispatcher(t *testing.T) { TestingT(t) }
+
+type UDSuite struct{}
+
+var _ = Suite(&UDSuite{})
+
+var nullog = logger.NewSimpleLogger(ioutil.Discard, "error")
+
+func (s *UDSuite) TestWorks(c *C) {
+	tb := testibus.NewMultiValuedTestingBus(condition.Work(true), condition.Work(true), []interface{}{})
+	ud, err := New(tb, nullog)
+	c.Assert(err, IsNil)
+	err = ud.DispatchURL("this")
+	c.Check(err, IsNil)
+}
+
+func (s *UDSuite) TestFailsIfConnectFails(c *C) {
+	tb := testibus.NewTestingBus(condition.Work(false), condition.Work(true))
+	_, err := New(tb, nullog)
+	c.Check(err, NotNil)
+}
+
+func (s *UDSuite) TestFailsIfCallFails(c *C) {
+	tb := testibus.NewTestingBus(condition.Work(true), condition.Work(false))
+	ud, err := New(tb, nullog)
+	c.Assert(err, IsNil)
+	err = ud.DispatchURL("this")
+	c.Check(err, NotNil)
+}