Merge lp:~pedronis/ubuntu-push/fix-1451510 into lp:ubuntu-push/automatic

[Samuele Pedroni]

skip over Gone messaging notifications to address lp:1451510, eagerly identify them in Tags, tests

* skip over Gone messaging notifications to address #1451510
* eagerly identify them in Tags
* tests

[Samuele Pedroni (pedronis)]

have +1 from Chipaca

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Samuele Pedroni (pedronis)]

review: Approve

=== modified file 'messaging/messaging.go'
--- messaging/messaging.go	2015-01-22 09:52:07 +0000
+++ messaging/messaging.go	2015-05-20 07:47:57 +0000
@@ -81,6 +81,10 @@
 func (mmu *MessagingMenu) cleanUpNotifications() {
 	mmu.lock.Lock()
 	defer mmu.lock.Unlock()
+	mmu.doCleanUpNotifications()
+}
+
+func (mmu *MessagingMenu) doCleanUpNotifications() {
 	for nid, payload := range mmu.notifications {
 		if payload.Gone {
 			// sweep
@@ -122,10 +126,11 @@
 func (mmu *MessagingMenu) Tags(app *click.AppId) []string {
 	orig := app.Original()
 	tags := []string(nil)
-	mmu.lock.RLock()
-	defer mmu.lock.RUnlock()
+	mmu.lock.Lock()
+	defer mmu.lock.Unlock()
+	mmu.doCleanUpNotifications()
 	for _, payload := range mmu.notifications {
-		if payload.App.Original() == orig {
+		if payload.App.Original() == orig && !payload.Gone {
 			tags = append(tags, payload.Tag)
 		}
 	}

=== modified file 'messaging/messaging_test.go'
--- messaging/messaging_test.go	2014-11-25 18:33:24 +0000
+++ messaging/messaging_test.go	2015-05-20 07:47:57 +0000
@@ -48,8 +48,6 @@
 	cRemoveNotification = func(a, n string) {
 		ms.log.Debugf("REMOVE: app: %s, not: %s", a, n)
 	}
-	// just in case
-	cNotificationExists = nil
 }
 
 func (ms *MessagingSuite) TearDownSuite(c *C) {
@@ -61,6 +59,8 @@
 func (ms *MessagingSuite) SetUpTest(c *C) {
 	ms.log = helpers.NewTestLogger(c, "debug")
 	ms.app = clickhelp.MustParseAppId("com.example.test_test_0")
+	// just in case
+	cNotificationExists = nil
 }
 
 func (ms *MessagingSuite) TestPresentPresents(c *C) {
@@ -129,6 +129,13 @@
 }
 
 func (ms *MessagingSuite) TestTagsListsTags(c *C) {
+	existsCount := 0
+	// patch cnotificationexists to return true
+	cNotificationExists = func(did string, nid string) bool {
+		existsCount++
+		return true
+	}
+
 	mmu := New(ms.log)
 	f := func(s string) *launch_helper.Notification {
 		card := launch_helper.Card{Summary: "tag: \"" + s + "\"", Persist: true}
@@ -138,8 +145,14 @@
 	c.Check(mmu.Tags(ms.app), IsNil)
 	c.Assert(mmu.Present(ms.app, "notif1", f("one")), Equals, true)
 	ms.checkTags(c, mmu.Tags(ms.app), []string{"one"})
+	c.Check(existsCount, Equals, 1)
+	existsCount = 0
+
 	c.Assert(mmu.Present(ms.app, "notif2", f("")), Equals, true)
 	ms.checkTags(c, mmu.Tags(ms.app), []string{"one", ""})
+	c.Check(existsCount, Equals, 2)
+	existsCount = 0
+
 	// and an empty notification doesn't count
 	c.Assert(mmu.Present(ms.app, "notif3", &launch_helper.Notification{Tag: "X"}), Equals, false)
 	ms.checkTags(c, mmu.Tags(ms.app), []string{"one", ""})
@@ -150,7 +163,32 @@
 	c.Check(mmu.Tags(ms.app), IsNil)
 }
 
+func (ms *MessagingSuite) TestTagsSkipGone(c *C) {
+	existsCount := 0
+	// patch cnotificationexists to return false
+	cNotificationExists = func(did string, nid string) bool {
+		existsCount++
+		return false
+	}
+
+	mmu := New(ms.log)
+	f := func(s string) *launch_helper.Notification {
+		card := launch_helper.Card{Summary: "tag: \"" + s + "\"", Persist: true}
+		return &launch_helper.Notification{Card: &card, Tag: s}
+	}
+
+	c.Check(mmu.Tags(ms.app), IsNil)
+	c.Assert(mmu.Present(ms.app, "notif1", f("one")), Equals, true)
+	ms.checkTags(c, mmu.Tags(ms.app), []string(nil))
+	c.Check(existsCount, Equals, 1)
+}
+
 func (ms *MessagingSuite) TestClearClears(c *C) {
+	// patch cnotificationexists to return true
+	cNotificationExists = func(did string, nid string) bool {
+		return true
+	}
+
 	app1 := ms.app
 	app2 := clickhelp.MustParseAppId("com.example.test_test-2_0")
 	app3 := clickhelp.MustParseAppId("com.example.test_test-3_0")