fix more todos

- protect setdictionary and addroute in pitaya package

app.go

@@ -36,6 +36,7 @@ import (
 	"github.com/topfreegames/pitaya/cluster"
 	"github.com/topfreegames/pitaya/component"
 	"github.com/topfreegames/pitaya/config"
+	"github.com/topfreegames/pitaya/constants"
 	"github.com/topfreegames/pitaya/internal/codec"
 	"github.com/topfreegames/pitaya/internal/message"
 	"github.com/topfreegames/pitaya/logger"
@@ -73,6 +74,7 @@ type App struct {
 	router           *router.Router
 	rpcClient        cluster.RPCClient
 	rpcServer        cluster.RPCServer
+	running          bool
 	serializer       serialize.Serializer
 	server           *cluster.Server
 	serverMode       ServerMode
@@ -98,6 +100,7 @@ var (
 		serverMode:    Standalone,
 		serializer:    json.NewSerializer(),
 		configured:    false,
+		running:       false,
 		router:        router.New(),
 	}
 	log = logger.Log
@@ -291,7 +294,10 @@ func Start() {
 
 	listen()
 
-	defer timer.GlobalTicker.Stop()
+	defer func() {
+		timer.GlobalTicker.Stop()
+		app.running = false
+	}()
 
 	sg := make(chan os.Signal)
 	signal.Notify(sg, syscall.SIGINT, syscall.SIGQUIT, syscall.SIGKILL)
@@ -345,28 +351,37 @@ func listen() {
 		// this should be so fast that we shoudn't need concurrency
 		go remoteService.ProcessUserPush()
 	}
+
+	app.running = true
 }
 
-// SetDictionary set routes map, TODO(warning): set dictionary in runtime would be a dangerous operation!!!!!!
-func SetDictionary(dict map[string]uint16) {
+// SetDictionary set routes map
+func SetDictionary(dict map[string]uint16) error {
+	if app.running {
+		return constants.ErrChangeDictionaryWhileRunning
+	}
 	message.SetDictionary(dict)
+	return nil
 }
 
 // AddRoute adds a routing function to a server type
-// TODO calling this method with the server already running is VERY dangerous
 func AddRoute(
 	serverType string,
 	routingFunction func(
 		session *session.Session,
 		route *route.Route,
 		servers map[string]*cluster.Server,
 	) (*cluster.Server, error),
-) {
+) error {
 	if app.router != nil {
+		if app.running {
+			return constants.ErrChangeRouteWhileRunning
+		}
 		app.router.AddRoute(serverType, routingFunction)
 	} else {
-		log.Warn("ignoring route add as app router is nil")
+		return constants.ErrRouterNotInitialized
 	}
+	return nil
 }
 
 // Shutdown send a signal to let 'pitaya' shutdown itself.

constants/errors.go

@@ -46,4 +46,7 @@ var (
 	ErrNonsenseRPC                    = errors.New("you are making a rpc that may be processed locally, either specify a different server type or specify a server id")
 	ErrNoUIDBind                      = errors.New("you have to bind an UID to the session to use groups")
 	ErrOnCloseBackend                 = errors.New("onclose callbacks are not allowed on backend servers")
+	ErrChangeDictionaryWhileRunning   = errors.New("you shouldn't change the dictionary while the app is already running")
+	ErrChangeRouteWhileRunning        = errors.New("you shouldn't change routes while app is already running")
+	ErrRouterNotInitialized           = errors.New("router is not initialized")
 )

examples/demo/cluster/main.go

@@ -44,7 +44,7 @@ func configureFrontend(port int) {
 		component.WithNameFunc(strings.ToLower),
 	)
 
-	pitaya.AddRoute("room", func(
+	err := pitaya.AddRoute("room", func(
 		session *session.Session,
 		route *route.Route,
 		servers map[string]*cluster.Server,
@@ -56,14 +56,22 @@ func configureFrontend(port int) {
 		return nil, nil
 	})
 
-	pitaya.SetDictionary(map[string]uint16{
+	if err != nil {
+		fmt.Printf("error adding route %s\n", err.Error())
+	}
+
+	err = pitaya.SetDictionary(map[string]uint16{
 		"connector.getsessiondata": 1,
 		"connector.setsessiondata": 2,
 		"room.room.getsessiondata": 3,
 		"onMessage":                4,
 		"onMembers":                5,
 	})
 
+	if err != nil {
+		fmt.Printf("error setting route dictionary %s\n", err.Error())
+	}
+
 	pitaya.AddAcceptor(ws)
 }
 

examples/demo/cluster_protobuf/main.go

@@ -45,7 +45,7 @@ func configureFrontend(port int) {
 		component.WithNameFunc(strings.ToLower),
 	)
 
-	pitaya.AddRoute("room", func(
+	err := pitaya.AddRoute("room", func(
 		session *session.Session,
 		route *route.Route,
 		servers map[string]*cluster.Server,
@@ -57,6 +57,10 @@ func configureFrontend(port int) {
 		return nil, nil
 	})
 
+	if err != nil {
+		fmt.Printf("error adding route: %s\n", err.Error())
+	}
+
 	pitaya.AddAcceptor(ws)
 }
 

internal/message/message.go

@@ -24,8 +24,9 @@ import (
 	"encoding/binary"
 	"errors"
 	"fmt"
-	"log"
 	"strings"
+
+	"github.com/topfreegames/pitaya/logger"
 )
 
 // Type represents the type of message, which could be Request/Notify/Response/Push
@@ -56,6 +57,7 @@ var types = map[Type]string{
 var (
 	routes = make(map[string]uint16) // route map to code
 	codes  = make(map[uint16]string) // code map to route
+	log    = logger.Log
 )
 
 // Errors that could be occurred in message codec
@@ -214,18 +216,18 @@ func Decode(data []byte) (*Message, error) {
 }
 
 // SetDictionary set routes map which be used to compress route.
-// TODO(warning): set dictionary in runtime would be a dangerous operation!!!!!!
 func SetDictionary(dict map[string]uint16) {
+	log.Warn("SetDictionary should only be called from pitaya package")
 	for route, code := range dict {
 		r := strings.TrimSpace(route)
 
 		// duplication check
 		if _, ok := routes[r]; ok {
-			log.Printf("duplicated route(route: %s, code: %d)\n", r, code)
+			log.Warnf("duplicated route(route: %s, code: %d)\n", r, code)
 		}
 
 		if _, ok := codes[code]; ok {
-			log.Printf("duplicated route(route: %s, code: %d)\n", r, code)
+			log.Warnf("duplicated route(route: %s, code: %d)\n", r, code)
 		}
 
 		// update map, using last value when key duplicated

module.go

@@ -51,7 +51,6 @@ func startModules() {
 	for name, mod := range modules {
 		log.Debugf("initializing module: %s", name)
 		if err := mod.Init(); err != nil {
-			// TODO maybe configure max retries and timeout for starting a module
 			log.Fatalf("error starting module %s, error: %s", name, err.Error())
 		}
 	}

router/router.go

@@ -100,7 +100,6 @@ func (r *Router) Route(
 }
 
 // AddRoute adds a routing function to a server type
-// TODO calling this method with the server already running is VERY dangerous
 func (r *Router) AddRoute(
 	serverType string,
 	routingFunction func(