Merge lp:~chipaca/ubuntu-push/fixes-to-app_helper into lp:ubuntu-push

[John Lenton]

fixes to app_helper

Fixes to app_helper

[lp:~chipaca/ubuntu-push/fixes-to-app_helper]

216. By John Lenton on 2014-07-03

commented the surprising bit

Unmerged revisions

=== modified file 'bus/notifications/app_helper/app_helper_c.go'
--- bus/notifications/app_helper/app_helper_c.go	2014-06-26 17:03:34 +0000
+++ bus/notifications/app_helper/app_helper_c.go	2014-07-03 14:52:16 +0000
@@ -19,23 +19,27 @@
 
 /*
 #cgo pkg-config: gio-unix-2.0
-#cgo pkg-config: gio-2.0
-#include <stdlib.h>
 #include <glib.h>
 #include <gio/gdesktopappinfo.h>
+
+gchar* app_icon_filename_from_id (gchar* app_id) {
+    gchar* filename = NULL;
+    GAppInfo* app_info = (GAppInfo*)g_desktop_app_info_new (app_id);
+    if (app_info != NULL) {
+        GIcon* icon = g_app_info_get_icon (app_info);
+        if (icon != NULL) {
+            filename = g_icon_to_string (icon);
+        }
+        g_object_unref (app_info);
+    }
+   g_free (app_id);
+   return filename;
+}
 */
 import "C"
-import "unsafe"
 
 func AppIconFromId(appId string) string {
-	_id := C.CString(appId)
-	defer C.free(unsafe.Pointer(_id))
-	_app_info := C.g_desktop_app_info_new(_id)
-	defer C.g_app_info_delete(_app_info)
-	_app_icon := C.g_app_info_get_icon(_app_info)
-	defer C.g_object_unref((C.gpointer)(_app_icon))
-	_icon_string := C.g_icon_to_string(_app_icon)
-	defer C.free(unsafe.Pointer(_icon_string))
-	name := C.GoString((*C.char)(_icon_string))
-	return name
+	name := C.app_icon_filename_from_id((*C.gchar)(C.CString(appId)))
+	defer C.g_free((C.gpointer)(name))
+	return C.GoString((*C.char)(name))
 }
 
=== modified file 'client/client.go'
--- client/client.go	2014-07-01 11:55:30 +0000
+++ client/client.go	2014-07-03 14:52:16 +0000
@@ -320,9 +320,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 +336,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
 	}
 
=== 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:52:16 +0000
@@ -712,6 +712,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 +725,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 +881,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:52:16 +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:52:16 +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)