Merge lp:~chipaca/ubuntu-push/shawn into lp:ubuntu-push

[John Lenton]

Gave ClientSession a Shown method that the client can use to indicate when a notification has been displayed (and made Client Session only update the levelmap on Shown).

Gave ClientSession a Shown method that the client can use to indicate when a notification has been displayed (and made Client Session only update the levelmap on Shown).

[John Lenton (chipaca)]

After discussing this on IRC, I'll add Shown when I track shown vs. not shown in the session (the protocol-side ACK vs increasing levels is correct as it stands now).

review: Disapprove

Unmerged revisions

43. By John Lenton on 2014-02-02

Gave ClientSession a Shown method that the client can use to indicate when a notification has been displayed (and made Client Session only update the levelmap on Shown)

=== modified file 'client/session/session.go'
--- client/session/session.go	2014-02-01 21:04:32 +0000
+++ client/session/session.go	2014-02-02 22:07:30 +0000
@@ -33,7 +33,8 @@
 var wireVersionBytes = []byte{protocol.ProtocolWireVersion}
 
 type Notification struct {
-	// something something something
+	channel string
+	level   int64
 }
 
 type serverMsg struct {
@@ -61,6 +62,8 @@
 	MsgCh chan *Notification
 }
 
+// NewSession creates and initializes a Ubuntu Push Notifications client
+// session.
 func NewSession(serverAddr string, pem []byte, exchangeTimeout time.Duration,
 	deviceId string, log logger.Logger) (*ClientSession, error) {
 	sess := &ClientSession{
@@ -94,6 +97,7 @@
 	return nil
 }
 
+// Close disconnects the session and sets the Connection attribute to nil.
 func (sess *ClientSession) Close() {
 	if sess.Connection != nil {
 		sess.Connection.Close()
@@ -125,8 +129,7 @@
 		bcast.ChanId, bcast.AppId, bcast.TopLevel, bcast.Payloads)
 	if bcast.ChanId == protocol.SystemChannelId {
 		// the system channel id, the only one we care about for now
-		sess.Levels.Set(bcast.ChanId, bcast.TopLevel)
-		sess.MsgCh <- &Notification{}
+		sess.MsgCh <- &Notification{channel: bcast.ChanId, level: bcast.TopLevel}
 	} else {
 		sess.Log.Debugf("what is this weird channel, %#v?", bcast.ChanId)
 	}
@@ -224,3 +227,9 @@
 	}
 	return sess.run(sess.Close, sess.connect, sess.start, sess.loop)
 }
+
+// Shown should be called after the given notification was successfully
+// displayed to the user (otherwise you'll keep on getting it).
+func (sess *ClientSession) Shown(n *Notification) {
+	sess.Levels.Set(n.channel, n.level)
+}
 
=== modified file 'client/session/session_test.go'
--- client/session/session_test.go	2014-02-01 21:04:32 +0000
+++ client/session/session_test.go	2014-02-02 22:07:30 +0000
@@ -309,9 +309,9 @@
 	s.upCh <- nil // ack ok
 	c.Check(<-s.errCh, Equals, nil)
 	c.Assert(len(s.sess.MsgCh), Equals, 1)
-	c.Check(<-s.sess.MsgCh, Equals, &Notification{})
-	// and finally, the session keeps track of the levels
-	c.Check(s.sess.Levels.GetAll(), DeepEquals, map[string]int64{"0": 2})
+	c.Check(<-s.sess.MsgCh, DeepEquals, &Notification{channel: "0", level: 2})
+	// but the session doesn't update the levelmap yet
+	c.Check(s.sess.Levels.GetAll(), DeepEquals, map[string]int64{})
 }
 
 func (s *msgSuite) TestHandleBroadcastBadAckWrite(c *C) {
@@ -688,9 +688,9 @@
 	c.Check(takeNext(downCh), Equals, protocol.AckMsg{"ack"})
 	upCh <- nil
 	// ...get bubbled up,
-	c.Check(<-sess.MsgCh, NotNil)
-	// and their TopLevel remembered
-	c.Check(sess.Levels.GetAll(), DeepEquals, map[string]int64{"0": 2})
+	c.Check(<-sess.MsgCh, DeepEquals, &Notification{channel: "0", level: 2})
+	// (but their TopLevel remain unchanged)
+	c.Check(sess.Levels.GetAll(), DeepEquals, map[string]int64{})
 
 	// and ping still work even after that.
 	c.Check(takeNext(downCh), Equals, "deadline 110ms")
@@ -700,3 +700,15 @@
 	upCh <- failure
 	c.Check(<-sess.ErrCh, Equals, failure)
 }
+
+/****************************************************************
+  Shown() tests
+****************************************************************/
+
+func (cs *clientSessionSuite) TestAckWorks(c *C) {
+	sess, err := NewSession("", nil, 0, "wah", debuglog)
+	c.Assert(err, IsNil)
+	c.Check(sess.Levels.GetAll(), DeepEquals, map[string]int64{})
+	sess.Shown(&Notification{channel: "0", level: 42})
+	c.Check(sess.Levels.GetAll(), DeepEquals, map[string]int64{"0": 42})
+}