Refactor tcp (#749)

* Refactor TCP server initialization

This refactor uses the functional options pattern to extract away the
optional TCP server configuration parameters. This:

- improves overall readability, by moving away from global variables
- makes it easier to configure the tcp server for tests

* Use ticker instead of for loop for room deletion

Go offers a ticker abstraction designed for performing tasks at
a regular interval, and this change uses the ticker for tcp room
deletion. It also cleans up the deletion goroutine gracefully.

* Add local relay interaction diagram

The diagram sketches out the interaction between clients and a local
relay.

* Add debug logs for room cleanup

These would be useful for future development (e.g.
adding a stopping mechanism for the TCP listener).

src/tcp/defaults.go

@@ -0,0 +1,9 @@
+package tcp
+
+import "time"
+
+const (
+	DEFAULT_LOG_LEVEL             = "debug"
+	DEFAULT_ROOM_CLEANUP_INTERVAL = 10 * time.Minute
+	DEFAULT_ROOM_TTL              = 3 * time.Hour
+)

src/tcp/options.go

@@ -0,0 +1,53 @@
+package tcp
+
+import (
+	"fmt"
+	"time"
+)
+
+// TODO: maybe export from logger library?
+var availableLogLevels = []string{"info", "error", "warn", "debug", "trace"}
+
+type serverOptsFunc func(s *server) error
+
+func WithBanner(banner ...string) serverOptsFunc {
+	return func(s *server) error {
+		if len(banner) > 0 {
+			s.banner = banner[0]
+		}
+		return nil
+	}
+}
+
+func WithLogLevel(level string) serverOptsFunc {
+	return func(s *server) error {
+		if !containsSlice(availableLogLevels, level) {
+			return fmt.Errorf("invalid log level specified: %s", level)
+		}
+		s.debugLevel = level
+		return nil
+	}
+}
+
+func WithRoomCleanupInterval(interval time.Duration) serverOptsFunc {
+	return func(s *server) error {
+		s.roomCleanupInterval = interval
+		return nil
+	}
+}
+
+func WithRoomTTL(ttl time.Duration) serverOptsFunc {
+	return func(s *server) error {
+		s.roomTTL = ttl
+		return nil
+	}
+}
+
+func containsSlice(s []string, e string) bool {
+	for _, ss := range s {
+		if e == ss {
+			return true
+		}
+	}
+	return false
+}

src/tcp/tcp.go

@@ -23,6 +23,11 @@ type server struct {
 	banner     string
 	password   string
 	rooms      roomMap
+
+	roomCleanupInterval time.Duration
+	roomTTL             time.Duration
+
+	stopRoomCleanup chan struct{}
 }
 
 type roomInfo struct {
@@ -39,21 +44,36 @@ type roomMap struct {
 
 const pingRoom = "pinglkasjdlfjsaldjf"
 
-var timeToRoomDeletion = 10 * time.Minute
+// newDefaultServer initializes a new server, with some default configuration options
-
+func newDefaultServer() *server {
-// Run starts a tcp listener, run async
-func Run(debugLevel, host, port, password string, banner ...string) (err error) {
 	s := new(server)
+	s.roomCleanupInterval = DEFAULT_ROOM_CLEANUP_INTERVAL
+	s.roomTTL = DEFAULT_ROOM_TTL
+	s.debugLevel = DEFAULT_LOG_LEVEL
+	s.stopRoomCleanup = make(chan struct{})
+	return s
+}
+
+// RunWithOptionsAsync asynchronously starts a TCP listener.
+func RunWithOptionsAsync(host, port, password string, opts ...serverOptsFunc) error {
+	s := newDefaultServer()
 	s.host = host
 	s.port = port
 	s.password = password
-	s.debugLevel = debugLevel
+	for _, opt := range opts {
-	if len(banner) > 0 {
+		err := opt(s)
-		s.banner = banner[0]
+		if err != nil {
+			return fmt.Errorf("could not apply optional configurations: %w", err)
+		}
 	}
 	return s.start()
 }
 
+// Run starts a tcp listener, run async
+func Run(debugLevel, host, port, password string, banner ...string) (err error) {
+	return RunWithOptionsAsync(host, port, password, WithBanner(banner...), WithLogLevel(debugLevel))
+}
+
 func (s *server) start() (err error) {
 	log.SetLevel(s.debugLevel)
 	log.Debugf("starting with password '%s'", s.password)
@@ -61,24 +81,8 @@ func (s *server) start() (err error) {
 	s.rooms.rooms = make(map[string]roomInfo)
 	s.rooms.Unlock()
 
-	// delete old rooms
+	go s.deleteOldRooms()
-	go func() {
+	defer s.stopRoomDeletion()
-		for {
-			time.Sleep(timeToRoomDeletion)
-			var roomsToDelete []string
-			s.rooms.Lock()
-			for room := range s.rooms.rooms {
-				if time.Since(s.rooms.rooms[room].opened) > 3*time.Hour {
-					roomsToDelete = append(roomsToDelete, room)
-				}
-			}
-			s.rooms.Unlock()
-
-			for _, room := range roomsToDelete {
-				s.deleteRoom(room)
-			}
-		}
-	}()
 
 	err = s.run()
 	if err != nil {
@@ -173,6 +177,39 @@ func (s *server) run() (err error) {
 	}
 }
 
+// deleteOldRooms checks for rooms at a regular interval and removes those that
+// have exceeded their allocated TTL.
+func (s *server) deleteOldRooms() {
+	ticker := time.NewTicker(s.roomCleanupInterval)
+	for {
+		select {
+		case <-ticker.C:
+			var roomsToDelete []string
+			s.rooms.Lock()
+			for room := range s.rooms.rooms {
+				if time.Since(s.rooms.rooms[room].opened) > s.roomTTL {
+					roomsToDelete = append(roomsToDelete, room)
+				}
+			}
+			s.rooms.Unlock()
+
+			for _, room := range roomsToDelete {
+				s.deleteRoom(room)
+				log.Debugf("room cleaned up: %s", room)
+			}
+		case <-s.stopRoomCleanup:
+			ticker.Stop()
+			log.Debug("room cleanup stopped")
+			return
+		}
+	}
+}
+
+func (s *server) stopRoomDeletion() {
+	log.Debug("stop room cleanup fired")
+	s.stopRoomCleanup <- struct{}{}
+}
+
 var weakKey = []byte{1, 2, 3}
 
 func (s *server) clientCommunication(port string, c *comm.Comm) (room string, err error) {

src/tcp/tcp_test.go

@@ -23,8 +23,8 @@ func BenchmarkConnection(b *testing.B) {
 
 func TestTCP(t *testing.T) {
 	log.SetLevel("error")
-	timeToRoomDeletion = 100 * time.Millisecond
+	timeToRoomDeletion := 100 * time.Millisecond
-	go Run("debug", "127.0.0.1", "8381", "pass123", "8382")
+	go RunWithOptionsAsync("127.0.0.1", "8381", "pass123", WithBanner("8382"), WithLogLevel("debug"), WithRoomTTL(timeToRoomDeletion))
 	time.Sleep(timeToRoomDeletion)
 	err := PingServer("127.0.0.1:8381")
 	assert.Nil(t, err)