Merge lp:~pedronis/ubuntu-push/pluggable-tracker into lp:ubuntu-push

[Samuele Pedroni]

make the tracker used by the session for logging pluggable

make the tracker used by the session for logging pluggable

[John Lenton (chipaca)]

Nice one.

review: Approve

=== modified file 'server/dev/server.go'
--- server/dev/server.go	2014-01-13 15:21:11 +0000
+++ server/dev/server.go	2014-01-14 19:07:52 +0000
@@ -69,7 +69,8 @@
 	}
 	logger.Infof("listening for devices on %v", lst.Addr())
 	err = lst.AcceptLoop(func(conn net.Conn) error {
-		return session.Session(conn, broker, cfg, logger)
+		track := session.NewTracker(logger)
+		return session.Session(conn, broker, cfg, track)
 	}, logger)
 	if err != nil {
 		logger.Fatalf("accepting device connections: %v", err)

=== modified file 'server/session/session.go'
--- server/session/session.go	2014-01-14 19:07:52 +0000
+++ server/session/session.go	2014-01-14 19:07:52 +0000
@@ -18,7 +18,6 @@
 package session
 
 import (
-	"launchpad.net/ubuntu-push/logger"
 	"launchpad.net/ubuntu-push/protocol"
 	"launchpad.net/ubuntu-push/server/broker"
 	"net"
@@ -109,48 +108,23 @@
 	}
 }
 
-var sessionsEpoch = time.Date(2013, 1, 1, 0, 0, 0, 0, time.UTC).UnixNano()
-
-// sessionTracker logs session events.
-type sessionTracker struct {
-	logger.Logger
-	sessionId int64 // xxx use timeuuid later
-}
-
-func (trk *sessionTracker) start(conn interface {
-	RemoteAddr() net.Addr
-}) {
-	trk.sessionId = time.Now().UnixNano() - sessionsEpoch
-	trk.Debugf("session(%x) connected %v", trk.sessionId, conn.RemoteAddr())
-}
-
-func (trk *sessionTracker) registered(sess broker.BrokerSession) {
-	trk.Infof("session(%x) registered %v", trk.sessionId, sess.DeviceId())
-}
-
-func (trk *sessionTracker) end(err error) error {
-	trk.Debugf("session(%x) ended with: %v", trk.sessionId, err)
-	return err
-}
-
 // Session manages the session with a client.
-func Session(conn net.Conn, brkr broker.Broker, cfg SessionConfig, logger logger.Logger) error {
+func Session(conn net.Conn, brkr broker.Broker, cfg SessionConfig, track SessionTracker) error {
 	defer conn.Close()
-	track := sessionTracker{Logger: logger}
-	track.start(conn)
+	track.Start(conn)
 	v, err := protocol.ReadWireFormatVersion(conn, cfg.ExchangeTimeout())
 	if err != nil {
-		return track.end(err)
+		return track.End(err)
 	}
 	if v != protocol.ProtocolWireVersion {
-		return track.end(&broker.ErrAbort{"unexpected wire format version"})
+		return track.End(&broker.ErrAbort{"unexpected wire format version"})
 	}
 	proto := protocol.NewProtocol0(conn)
 	sess, err := sessionStart(proto, brkr, cfg)
 	if err != nil {
-		return track.end(err)
+		return track.End(err)
 	}
-	track.registered(sess)
+	track.Registered(sess)
 	defer brkr.Unregister(sess)
-	return track.end(sessionLoop(proto, sess, cfg))
+	return track.End(sessionLoop(proto, sess, cfg))
 }

=== modified file 'server/session/session_test.go'
--- server/session/session_test.go	2014-01-13 15:21:11 +0000
+++ server/session/session_test.go	2014-01-14 19:07:52 +0000
@@ -423,42 +423,6 @@
 	c.Check(err, Equals, io.EOF)
 }
 
-type testRemoteAddrable struct{}
-
-func (tra *testRemoteAddrable) RemoteAddr() net.Addr {
-	return &net.TCPAddr{net.IPv4(127, 0, 0, 1), 9999, ""}
-}
-
-func (s *sessionSuite) TestSessionTrackStart(c *C) {
-	buf := &bytes.Buffer{}
-	logger := logger.NewSimpleLogger(buf, "debug")
-	track := sessionTracker{Logger: logger}
-	track.start(&testRemoteAddrable{})
-	c.Check(track.sessionId, Not(Equals), 0)
-	regExpected := fmt.Sprintf(`.* DEBUG session\(%x\) connected 127\.0\.0\.1:9999\n`, track.sessionId)
-	c.Check(buf.String(), Matches, regExpected)
-}
-
-func (s *sessionSuite) TestSessionTrackRegistered(c *C) {
-	buf := &bytes.Buffer{}
-	logger := logger.NewSimpleLogger(buf, "debug")
-	track := sessionTracker{Logger: logger}
-	track.start(&testRemoteAddrable{})
-	track.registered(&testBrokerSession{deviceId: "DEV-ID"})
-	regExpected := fmt.Sprintf(`.*connected.*\n.* INFO session\(%x\) registered DEV-ID\n`, track.sessionId)
-	c.Check(buf.String(), Matches, regExpected)
-}
-
-func (s *sessionSuite) TestSessionTrackEnd(c *C) {
-	buf := &bytes.Buffer{}
-	logger := logger.NewSimpleLogger(buf, "debug")
-	track := sessionTracker{Logger: logger}
-	track.start(&testRemoteAddrable{})
-	track.end(&broker.ErrAbort{})
-	regExpected := fmt.Sprintf(`.*connected.*\n.* DEBUG session\(%x\) ended with: session aborted \(\)\n`, track.sessionId)
-	c.Check(buf.String(), Matches, regExpected)
-}
-
 func serverClientWire() (srv net.Conn, cli net.Conn, lst net.Listener) {
 	lst, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
@@ -504,14 +468,14 @@
 
 func (s *sessionSuite) TestSessionWire(c *C) {
 	buf := &bytes.Buffer{}
-	logger := logger.NewSimpleLogger(buf, "debug")
+	track := NewTracker(logger.NewSimpleLogger(buf, "debug"))
 	errCh := make(chan error, 1)
 	srv, cli, lst := serverClientWire()
 	defer lst.Close()
 	remSrv := &rememberDeadlineConn{srv, make([]string, 0, 2)}
 	brkr := newTestBroker()
 	go func() {
-		errCh <- Session(remSrv, brkr, cfg5msPingInterval, logger)
+		errCh <- Session(remSrv, brkr, cfg5msPingInterval, track)
 	}()
 	io.WriteString(cli, "\x00")
 	io.WriteString(cli, "\x00\x20{\"T\":\"connect\",\"DeviceId\":\"DEV\"}")
@@ -531,13 +495,14 @@
 }
 
 func (s *sessionSuite) TestSessionWireTimeout(c *C) {
+	nopTrack := NewTracker(nopLogger)
 	errCh := make(chan error, 1)
 	srv, cli, lst := serverClientWire()
 	defer lst.Close()
 	remSrv := &rememberDeadlineConn{srv, make([]string, 0, 2)}
 	brkr := newTestBroker()
 	go func() {
-		errCh <- Session(remSrv, brkr, cfg5msPingInterval2msExchangeTout, nopLogger)
+		errCh <- Session(remSrv, brkr, cfg5msPingInterval2msExchangeTout, nopTrack)
 	}()
 	err := <-errCh
 	c.Check(err, FitsTypeOf, &net.OpError{})
@@ -547,13 +512,13 @@
 
 func (s *sessionSuite) TestSessionWireWrongVersion(c *C) {
 	buf := &bytes.Buffer{}
-	logger := logger.NewSimpleLogger(buf, "debug")
+	track := NewTracker(logger.NewSimpleLogger(buf, "debug"))
 	errCh := make(chan error, 1)
 	srv, cli, lst := serverClientWire()
 	defer lst.Close()
 	brkr := newTestBroker()
 	go func() {
-		errCh <- Session(srv, brkr, cfg5msPingInterval, logger)
+		errCh <- Session(srv, brkr, cfg5msPingInterval, track)
 	}()
 	io.WriteString(cli, "\x10")
 	err := <-errCh
@@ -566,13 +531,13 @@
 
 func (s *sessionSuite) TestSessionWireEarlyClose(c *C) {
 	buf := &bytes.Buffer{}
-	logger := logger.NewSimpleLogger(buf, "debug")
+	track := NewTracker(logger.NewSimpleLogger(buf, "debug"))
 	errCh := make(chan error, 1)
 	srv, cli, lst := serverClientWire()
 	defer lst.Close()
 	brkr := newTestBroker()
 	go func() {
-		errCh <- Session(srv, brkr, cfg5msPingInterval, logger)
+		errCh <- Session(srv, brkr, cfg5msPingInterval, track)
 	}()
 	cli.Close()
 	err := <-errCh
@@ -584,13 +549,13 @@
 
 func (s *sessionSuite) TestSessionWireEarlyClose2(c *C) {
 	buf := &bytes.Buffer{}
-	logger := logger.NewSimpleLogger(buf, "debug")
+	track := NewTracker(logger.NewSimpleLogger(buf, "debug"))
 	errCh := make(chan error, 1)
 	srv, cli, lst := serverClientWire()
 	defer lst.Close()
 	brkr := newTestBroker()
 	go func() {
-		errCh <- Session(srv, brkr, cfg5msPingInterval, logger)
+		errCh <- Session(srv, brkr, cfg5msPingInterval, track)
 	}()
 	io.WriteString(cli, "\x00")
 	io.WriteString(cli, "\x00")
@@ -602,13 +567,14 @@
 }
 
 func (s *sessionSuite) TestSessionWireTimeout2(c *C) {
+	nopTrack := NewTracker(nopLogger)
 	errCh := make(chan error, 1)
 	srv, cli, lst := serverClientWire()
 	defer lst.Close()
 	remSrv := &rememberDeadlineConn{srv, make([]string, 0, 2)}
 	brkr := newTestBroker()
 	go func() {
-		errCh <- Session(remSrv, brkr, cfg5msPingInterval2msExchangeTout, nopLogger)
+		errCh <- Session(remSrv, brkr, cfg5msPingInterval2msExchangeTout, nopTrack)
 	}()
 	io.WriteString(cli, "\x00")
 	io.WriteString(cli, "\x00")

=== added file 'server/session/tracker.go'
--- server/session/tracker.go	1970-01-01 00:00:00 +0000
+++ server/session/tracker.go	2014-01-14 19:07:52 +0000
@@ -0,0 +1,66 @@
+/*
+ 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 session
+
+import (
+	"launchpad.net/ubuntu-push/logger"
+	"launchpad.net/ubuntu-push/server/broker"
+	"net"
+	"time"
+)
+
+// SessionTracker logs session events.
+type SessionTracker interface {
+	logger.Logger
+	// Session got started.
+	Start(WithRemoteAddr)
+	// Session got registered with broker as sess BrokerSession.
+	Registered(sess broker.BrokerSession)
+	// Session got ended with error err (can be nil).
+	End(error) error
+}
+
+// WithRemoteAddr can report a remote address.
+type WithRemoteAddr interface {
+	RemoteAddr() net.Addr
+}
+
+var sessionsEpoch = time.Date(2013, 1, 1, 0, 0, 0, 0, time.UTC).UnixNano()
+
+// Tracker implements SessionTracker simply.
+type tracker struct {
+	logger.Logger
+	sessionId int64 // xxx use timeuuid later
+}
+
+func NewTracker(logger logger.Logger) SessionTracker {
+	return &tracker{Logger: logger}
+}
+
+func (trk *tracker) Start(conn WithRemoteAddr) {
+	trk.sessionId = time.Now().UnixNano() - sessionsEpoch
+	trk.Debugf("session(%x) connected %v", trk.sessionId, conn.RemoteAddr())
+}
+
+func (trk *tracker) Registered(sess broker.BrokerSession) {
+	trk.Infof("session(%x) registered %v", trk.sessionId, sess.DeviceId())
+}
+
+func (trk *tracker) End(err error) error {
+	trk.Debugf("session(%x) ended with: %v", trk.sessionId, err)
+	return err
+}

=== added file 'server/session/tracker_test.go'
--- server/session/tracker_test.go	1970-01-01 00:00:00 +0000
+++ server/session/tracker_test.go	2014-01-14 19:07:52 +0000
@@ -0,0 +1,66 @@
+/*
+ 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 session
+
+import (
+	"bytes"
+	"fmt"
+	. "launchpad.net/gocheck"
+	"launchpad.net/ubuntu-push/logger"
+	"launchpad.net/ubuntu-push/server/broker"
+	"net"
+)
+
+type trackerSuite struct{}
+
+var _ = Suite(&trackerSuite{})
+
+type testRemoteAddrable struct{}
+
+func (tra *testRemoteAddrable) RemoteAddr() net.Addr {
+	return &net.TCPAddr{net.IPv4(127, 0, 0, 1), 9999, ""}
+}
+
+func (s *sessionSuite) TestSessionTrackStart(c *C) {
+	buf := &bytes.Buffer{}
+	logger := logger.NewSimpleLogger(buf, "debug")
+	track := NewTracker(logger)
+	track.Start(&testRemoteAddrable{})
+	c.Check(track.(*tracker).sessionId, Not(Equals), 0)
+	regExpected := fmt.Sprintf(`.* DEBUG session\(%x\) connected 127\.0\.0\.1:9999\n`, track.(*tracker).sessionId)
+	c.Check(buf.String(), Matches, regExpected)
+}
+
+func (s *sessionSuite) TestSessionTrackRegistered(c *C) {
+	buf := &bytes.Buffer{}
+	logger := logger.NewSimpleLogger(buf, "debug")
+	track := NewTracker(logger)
+	track.Start(&testRemoteAddrable{})
+	track.Registered(&testBrokerSession{deviceId: "DEV-ID"})
+	regExpected := fmt.Sprintf(`.*connected.*\n.* INFO session\(%x\) registered DEV-ID\n`, track.(*tracker).sessionId)
+	c.Check(buf.String(), Matches, regExpected)
+}
+
+func (s *sessionSuite) TestSessionTrackEnd(c *C) {
+	buf := &bytes.Buffer{}
+	logger := logger.NewSimpleLogger(buf, "debug")
+	track := NewTracker(logger)
+	track.Start(&testRemoteAddrable{})
+	track.End(&broker.ErrAbort{})
+	regExpected := fmt.Sprintf(`.*connected.*\n.* DEBUG session\(%x\) ended with: session aborted \(\)\n`, track.(*tracker).sessionId)
+	c.Check(buf.String(), Matches, regExpected)
+}