Rewrite NewServer to reduce cyclomatic complexity

Co-author: Steffen Vogel <post@steffenvogel.de>

Author: anastasia

server.go

@@ -28,8 +28,7 @@ type Server struct {
 	packetConnConfigs  []PacketConnConfig
 	listenerConfigs    []ListenerConfig
 	allocationManagers []*allocation.Manager
-
-	inboundMTU int
+	inboundMTU         int
 }
 
 // NewServer creates the Pion TURN server
@@ -57,7 +56,6 @@ func NewServer(config ServerConfig) (*Server, error) {
 		channelBindTimeout: config.ChannelBindTimeout,
 		packetConnConfigs:  config.PacketConnConfigs,
 		listenerConfigs:    config.ListenerConfigs,
-		allocationManagers: make([]*allocation.Manager, len(config.PacketConnConfigs)+len(config.ListenerConfigs)),
 		nonces:             &sync.Map{},
 		inboundMTU:         mtu,
 	}
@@ -66,96 +64,48 @@ func NewServer(config ServerConfig) (*Server, error) {
 		s.channelBindTimeout = proto.DefaultLifetime
 	}
 
-	for i := range s.packetConnConfigs {
-		go func(i int, p PacketConnConfig) {
-			permissionHandler := p.PermissionHandler
-			if permissionHandler == nil {
-				permissionHandler = DefaultPermissionHandler
-			}
-
-			allocationManager, err := allocation.NewManager(allocation.ManagerConfig{
-				AllocatePacketConn: p.RelayAddressGenerator.AllocatePacketConn,
-				AllocateConn:       p.RelayAddressGenerator.AllocateConn,
-				PermissionHandler:  permissionHandler,
-				LeveledLogger:      s.log,
-			})
-			if err != nil {
-				s.log.Errorf("exit read loop on error: %s", err.Error())
-				return
-			}
-			s.allocationManagers[i] = allocationManager
-			defer func() {
-				if err := allocationManager.Close(); err != nil {
-					s.log.Errorf("Failed to close AllocationManager: %s", err.Error())
-				}
-			}()
-
-			s.readLoop(p.PacketConn, allocationManager)
-		}(i, s.packetConnConfigs[i])
-	}
-
-	for i, listener := range s.listenerConfigs {
-		go func(i int, l ListenerConfig) {
-			permissionHandler := l.PermissionHandler
-			if permissionHandler == nil {
-				permissionHandler = DefaultPermissionHandler
-			}
-
-			allocationManager, err := allocation.NewManager(allocation.ManagerConfig{
-				AllocatePacketConn: l.RelayAddressGenerator.AllocatePacketConn,
-				AllocateConn:       l.RelayAddressGenerator.AllocateConn,
-				PermissionHandler:  permissionHandler,
-				LeveledLogger:      s.log,
-			})
-			if err != nil {
-				s.log.Errorf("exit read loop on error: %s", err.Error())
-				return
-			}
-			s.allocationManagers[i] = allocationManager
-			defer func() {
-				if err := allocationManager.Close(); err != nil {
-					s.log.Errorf("Failed to close AllocationManager: %s", err.Error())
-				}
-			}()
-
-			for {
-				conn, err := l.Listener.Accept()
-				if err != nil {
-					s.log.Debugf("exit accept loop on error: %s", err.Error())
-					return
-				}
-
-				go s.readLoop(NewSTUNConn(conn), allocationManager)
-			}
-		}(i+len(s.packetConnConfigs), listener)
+	for _, cfg := range s.packetConnConfigs {
+		am, err := s.createAllocationManager(cfg.RelayAddressGenerator, cfg.PermissionHandler)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create AllocationManager: %w", err)
+		}
+
+		go s.readPacketConn(cfg, am)
+	}
+
+	for _, cfg := range s.listenerConfigs {
+		am, err := s.createAllocationManager(cfg.RelayAddressGenerator, cfg.PermissionHandler)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create AllocationManager: %w", err)
+		}
+
+		go s.readListener(cfg, am)
 	}
 
 	return s, nil
 }
 
 // AllocationCount returns the number of active allocations. It can be used to drain the server before closing
 func (s *Server) AllocationCount() int {
-	allocations := 0
-	for _, manager := range s.allocationManagers {
-		if manager != nil {
-			allocations += manager.AllocationCount()
-		}
+	allocs := 0
+	for _, am := range s.allocationManagers {
+		allocs += am.AllocationCount()
 	}
-	return allocations
+	return allocs
 }
 
 // Close stops the TURN Server. It cleans up any associated state and closes all connections it is managing
 func (s *Server) Close() error {
 	var errors []error
 
-	for _, p := range s.packetConnConfigs {
-		if err := p.PacketConn.Close(); err != nil {
+	for _, cfg := range s.packetConnConfigs {
+		if err := cfg.PacketConn.Close(); err != nil {
 			errors = append(errors, err)
 		}
 	}
 
-	for _, l := range s.listenerConfigs {
-		if err := l.Listener.Close(); err != nil {
+	for _, cfg := range s.listenerConfigs {
+		if err := cfg.Listener.Close(); err != nil {
 			errors = append(errors, err)
 		}
 	}
@@ -166,12 +116,58 @@ func (s *Server) Close() error {
 
 	err := errFailedToClose
 	for _, e := range errors {
-		err = fmt.Errorf("%s; close error (%w) ", err.Error(), e)
+		err = fmt.Errorf("%s; close error (%w) ", err, e)
 	}
 
 	return err
 }
 
+func (s *Server) readPacketConn(p PacketConnConfig, am *allocation.Manager) {
+	s.readLoop(p.PacketConn, am)
+
+	if err := am.Close(); err != nil {
+		s.log.Errorf("Failed to close AllocationManager: %s", err)
+	}
+}
+
+func (s *Server) readListener(l ListenerConfig, am *allocation.Manager) {
+	defer func() {
+		if err := am.Close(); err != nil {
+			s.log.Errorf("Failed to close AllocationManager: %s", err)
+		}
+	}()
+
+	for {
+		conn, err := l.Listener.Accept()
+		if err != nil {
+			s.log.Debugf("Failed to accept: %s", err)
+			return
+		}
+
+		go s.readLoop(NewSTUNConn(conn), am)
+	}
+}
+
+func (s *Server) createAllocationManager(addrGenerator RelayAddressGenerator, handler PermissionHandler) (*allocation.Manager, error) {
+	if handler == nil {
+		handler = DefaultPermissionHandler
+	}
+
+	am, err := allocation.NewManager(allocation.ManagerConfig{
+		AllocatePacketConn: addrGenerator.AllocatePacketConn,
+		AllocateConn:       addrGenerator.AllocateConn,
+		PermissionHandler:  handler,
+		LeveledLogger:      s.log,
+	})
+	if err != nil {
+		return am, err
+	}
+
+	s.allocationManagers = append(s.allocationManagers, am)
+
+	return am, err
+}
+
 func (s *Server) readLoop(p net.PacketConn, allocationManager *allocation.Manager) {
 	buf := make([]byte, s.inboundMTU)
 	for {