From f2e16c2ffb5033fc3a53f0b9f09a87242a41667d Mon Sep 17 00:00:00 2001 From: Kirill Sysoev Date: Sat, 18 May 2024 22:37:40 +0800 Subject: [PATCH 1/4] chore: Add net.Listener to Server struct and update Run method --- server/server.go | 26 +++++++++++++++++++++--- server/server_test.go | 46 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/server/server.go b/server/server.go index 1fec328..38f1bc2 100644 --- a/server/server.go +++ b/server/server.go @@ -25,6 +25,7 @@ type Server struct { handler *http.Server addr string channels []wasabi.Channel + listener net.Listener } type Option func(*Server) @@ -33,6 +34,10 @@ type Option func(*Server) // port - port to listen on // returns new instance of Server func NewServer(addr string, opts ...Option) *Server { + if addr == "" { + addr = ":http" + } + server := &Server{ addr: addr, channels: make([]wasabi.Channel, 0, 1), @@ -64,7 +69,7 @@ func (s *Server) AddChannel(channel wasabi.Channel) { // Run starts the server // returns error if server is already running // or if server fails to start -func (s *Server) Run() error { +func (s *Server) Run() (err error) { if !s.mutex.TryLock() { return ErrServerAlreadyRunning } @@ -82,9 +87,14 @@ func (s *Server) Run() error { s.handler.Handler = mux - slog.Info("Starting app server on " + s.addr) + s.listener, err = net.Listen("tcp", s.addr) + if err != nil { + return err + } + + slog.Info("Starting app server on " + s.listener.Addr().String()) - err := s.handler.ListenAndServe() + err = s.handler.Serve(s.listener) if err != nil && err != http.ErrServerClosed { return err @@ -132,6 +142,16 @@ func (s *Server) Close(ctx ...context.Context) error { return <-done } +// Addr returns the server's network address. +// If the server is not running, it returns nil. +func (s *Server) Addr() net.Addr { + if s.listener == nil { + return nil + } + + return s.listener.Addr() +} + // BaseContext optionally specifies based context that will be used for all connections. // If not specified, context.Background() will be used. func WithBaseContext(ctx context.Context) Option { diff --git a/server/server_test.go b/server/server_test.go index 05fe6e0..cdd4c8c 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -2,6 +2,7 @@ package server import ( "context" + "fmt" "net/http" "testing" "time" @@ -218,3 +219,48 @@ func TestServer_Close_NoContext(t *testing.T) { t.Error("Expected server to stop") } } + +func TestServer_Addr(t *testing.T) { + // Create a new Server instance + server := NewServer(":0") + defer server.Close() + + // Create a mock channel + channel := mocks.NewMockChannel(t) + channel.EXPECT().Path().Return("/test") + channel.EXPECT().Handler().Return(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + channel.EXPECT().Close().Return(nil) + + server.AddChannel(channel) + + // Start the server in a separate goroutine + done := make(chan struct{}) + + // Run the server + for i := 0; i < 2; i++ { + go func() { + err := server.Run() + switch err { + case nil: + case ErrServerAlreadyRunning: + close(done) + default: + t.Errorf("Got unexpected error: %v", err) + } + }() + } + + select { + case <-done: + case <-time.After(1 * time.Second): + t.Error("Expected server to start") + } + + fmt.Println(server.listener) + + addr := server.Addr() + + if addr == nil { + t.Error("Expected non-empty address") + } +} From 7bc26456960a3b46bf350627a4ff95d8ee48bd32 Mon Sep 17 00:00:00 2001 From: Kirill Sysoev Date: Sun, 19 May 2024 16:13:31 +0800 Subject: [PATCH 2/4] Fix test logic --- server/server.go | 2 +- server/server_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/server.go b/server/server.go index 38f1bc2..f3777ad 100644 --- a/server/server.go +++ b/server/server.go @@ -21,11 +21,11 @@ var ErrServerAlreadyRunning = fmt.Errorf("server is already running") type Server struct { baseCtx context.Context + listener net.Listener mutex *sync.Mutex handler *http.Server addr string channels []wasabi.Channel - listener net.Listener } type Option func(*Server) diff --git a/server/server_test.go b/server/server_test.go index cdd4c8c..e3cd988 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -2,7 +2,6 @@ package server import ( "context" - "fmt" "net/http" "testing" "time" @@ -256,7 +255,8 @@ func TestServer_Addr(t *testing.T) { t.Error("Expected server to start") } - fmt.Println(server.listener) + // Wait for the server to fully start + time.Sleep(1 * time.Millisecond) addr := server.Addr() From 99a8b1ae963dafecb22bc44255216cd50302f0f3 Mon Sep 17 00:00:00 2001 From: Kirill Sysoev Date: Sun, 19 May 2024 16:31:36 +0800 Subject: [PATCH 3/4] Fix data race for Addr method --- server/server.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/server/server.go b/server/server.go index f3777ad..bf4d583 100644 --- a/server/server.go +++ b/server/server.go @@ -20,12 +20,13 @@ const ( var ErrServerAlreadyRunning = fmt.Errorf("server is already running") type Server struct { - baseCtx context.Context - listener net.Listener - mutex *sync.Mutex - handler *http.Server - addr string - channels []wasabi.Channel + baseCtx context.Context + listener net.Listener + listenerLock *sync.Mutex + mutex *sync.Mutex + handler *http.Server + addr string + channels []wasabi.Channel } type Option func(*Server) @@ -39,10 +40,11 @@ func NewServer(addr string, opts ...Option) *Server { } server := &Server{ - addr: addr, - channels: make([]wasabi.Channel, 0, 1), - mutex: &sync.Mutex{}, - baseCtx: context.Background(), + addr: addr, + channels: make([]wasabi.Channel, 0, 1), + mutex: &sync.Mutex{}, + listenerLock: &sync.Mutex{}, + baseCtx: context.Background(), } for _, opt := range opts { @@ -87,7 +89,10 @@ func (s *Server) Run() (err error) { s.handler.Handler = mux + s.listenerLock.Lock() s.listener, err = net.Listen("tcp", s.addr) + s.listenerLock.Unlock() + if err != nil { return err } @@ -145,6 +150,9 @@ func (s *Server) Close(ctx ...context.Context) error { // Addr returns the server's network address. // If the server is not running, it returns nil. func (s *Server) Addr() net.Addr { + s.listenerLock.Lock() + defer s.listenerLock.Unlock() + if s.listener == nil { return nil } From 619ff6b65240c88b295dbba9d05736312d308fee Mon Sep 17 00:00:00 2001 From: Kirill Sysoev Date: Sun, 19 May 2024 16:35:29 +0800 Subject: [PATCH 4/4] Improves test coverage --- server/server_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/server/server_test.go b/server/server_test.go index e3cd988..bddaf8d 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -26,6 +26,12 @@ func TestNewServer(t *testing.T) { if server.mutex == nil { t.Error("Expected non-nil mutex") } + + server = NewServer("") + + if server.addr != ":http" { + t.Errorf("Expected default port :http, but got %s", server.addr) + } } func TestServer_AddChannel(t *testing.T) { // Create a new Server instance @@ -232,6 +238,10 @@ func TestServer_Addr(t *testing.T) { server.AddChannel(channel) + if server.Addr() != nil { + t.Error("Expected nil address for server that is not running") + } + // Start the server in a separate goroutine done := make(chan struct{})