Store routes as pointers / Fix GCM test failing random

gcm/gcm_connector_test.go

@@ -43,7 +43,7 @@ var errorResponseMessageJSON = `
    "results":[
       {
          "message_id":"err",
-         "registration_id":"gmcCanonicalId",
+         "registration_id":"gcmCanonicalId",
          "error":"InvalidRegistration"
       }
    ]
@@ -360,13 +360,15 @@ func TestGCMConnector_BroadcastMessage(t *testing.T) {
 	gcm.broadcastMessage(broadcastMessage)
 	//wait for the message to be processed by http server
 	<-done
+	<-time.After(100 * time.Millisecond)
+	gcm.Stop()
 
 }
 
 func TestGCMConnector_GetErrorMessageFromGcm(t *testing.T) {
 	ctrl, finish := testutil.NewMockCtrl(t)
 	defer finish()
-	//defer testutil.EnableDebugForMethod()()
+	// defer testutil.EnableDebugForMethod()()
 
 	assert := assert.New(t)
 	routerMock := NewMockRouter(ctrl)
@@ -386,7 +388,7 @@ func TestGCMConnector_GetErrorMessageFromGcm(t *testing.T) {
 	routerMock.EXPECT().Subscribe(gomock.Any()).Do(func(route *server.Route) {
 		assert.Equal("/path", string(route.Path))
 		assert.Equal("marvin", route.UserID)
-		assert.Equal("gmcCanonicalId", route.ApplicationID)
+		assert.Equal("gcmCanonicalId", route.ApplicationID)
 	})
 
 	kvStore := store.NewMemoryKVStore()
@@ -410,4 +412,6 @@ func TestGCMConnector_GetErrorMessageFromGcm(t *testing.T) {
 	gcm.channelFromRouter <- msg
 	// expect that the Http Server to give us a malformed message
 	<-done
+	<-time.After(100 * time.Millisecond)
+	gcm.Stop()
 }

server/router.go

@@ -29,7 +29,7 @@ type subRequest struct {
 
 type router struct {
 	// mapping the path to the route slice
-	routes          map[protocol.Path][]Route
+	routes          map[protocol.Path][]*Route
 	messageIn       chan *protocol.Message
 	subscribeChan   chan subRequest
 	unsubscribeChan chan subRequest
@@ -45,7 +45,7 @@ type router struct {
 // NewRouter returns a pointer to Router
 func NewRouter(accessManager auth.AccessManager, messageStore store.MessageStore, kvStore store.KVStore) Router {
 	return &router{
-		routes:          make(map[protocol.Path][]Route),
+		routes:          make(map[protocol.Path][]*Route),
 		messageIn:       make(chan *protocol.Message, 500),
 		subscribeChan:   make(chan subRequest, 10),
 		unsubscribeChan: make(chan subRequest, 10),
@@ -125,11 +125,11 @@ func (router *router) subscribe(r *Route) {
 		list = remove(list, r)
 	} else {
 		// Path not present yet. Initialize the slice
-		list = make([]Route, 0, 1)
+		list = make([]*Route, 0, 1)
 		router.routes[r.Path] = list
 	}
 
-	router.routes[r.Path] = append(list, *r)
+	router.routes[r.Path] = append(list, r)
 }
 
 func (router *router) Unsubscribe(r *Route) {
@@ -199,23 +199,23 @@ func (router *router) handleMessage(message *protocol.Message) {
 	}
 }
 
-func (router *router) deliverMessage(route Route, message *protocol.Message) {
+func (router *router) deliverMessage(route *Route, message *protocol.Message) {
 	defer protocol.PanicLogger()
 	select {
-	case route.C <- MsgAndRoute{Message: message, Route: &route}:
+	case route.C <- MsgAndRoute{Message: message, Route: route}:
 	// fine, we could send the message
 	default:
 		protocol.Info("queue was full, closing delivery for route=%v to applicationID=%v", route.Path, route.ApplicationID)
 		close(route.C)
-		router.unsubscribe(&route)
+		router.unsubscribe(route)
 	}
 }
 
 func (router *router) closeAllRoutes() {
 	for _, currentRouteList := range router.routes {
 		for _, route := range currentRouteList {
 			close(route.C)
-			router.unsubscribe(&route)
+			router.unsubscribe(route)
 		}
 	}
 }
@@ -237,7 +237,7 @@ func matchesTopic(messagePath, routePath protocol.Path) bool {
 
 // remove a route from the supplied list,
 // based on same ApplicationID id and same path
-func remove(slice []Route, route *Route) []Route {
+func remove(slice []*Route, route *Route) []*Route {
 	position := -1
 	for p, r := range slice {
 		if r.ApplicationID == route.ApplicationID && r.Path == route.Path {

server/router_test.go

@@ -35,19 +35,19 @@ func TestAddAndRemoveRoutes(t *testing.T) {
 
 	// the routes are stored
 	a.Equal(2, len(router.routes[protocol.Path("/blah")]))
-	a.True(routeBlah1.equals(router.routes[protocol.Path("/blah")][0]))
-	a.True(routeBlah2.equals(router.routes[protocol.Path("/blah")][1]))
+	a.True(routeBlah1.equals(*router.routes[protocol.Path("/blah")][0]))
+	a.True(routeBlah2.equals(*router.routes[protocol.Path("/blah")][1]))
 
 	a.Equal(1, len(router.routes[protocol.Path("/foo")]))
-	a.True(routeFoo.equals(router.routes[protocol.Path("/foo")][0]))
+	a.True(routeFoo.equals(*router.routes[protocol.Path("/foo")][0]))
 
 	// when i remove routes
 	router.Unsubscribe(routeBlah1)
 	router.Unsubscribe(routeFoo)
 
 	// then they are gone
 	a.Equal(1, len(router.routes[protocol.Path("/blah")]))
-	a.True(routeBlah2.equals(router.routes[protocol.Path("/blah")][0]))
+	a.True(routeBlah2.equals(*router.routes[protocol.Path("/blah")][0]))
 
 	a.Nil(router.routes[protocol.Path("/foo")])
 }