From 357a7a7b18f086272b6566ee1d03ad016662d105 Mon Sep 17 00:00:00 2001 From: Alexis Couvreur Date: Sun, 2 Feb 2025 09:08:30 -0500 Subject: [PATCH 1/3] test(api): add test theme not found --- internal/api/start_dynamic_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/api/start_dynamic_test.go b/internal/api/start_dynamic_test.go index b97d99ee..471d3987 100644 --- a/internal/api/start_dynamic_test.go +++ b/internal/api/start_dynamic_test.go @@ -32,6 +32,14 @@ func TestStartDynamic(t *testing.T) { assert.Equal(t, http.StatusBadRequest, r.Code) assert.Equal(t, rfc7807.JSONMediaType, r.Header().Get("Content-Type")) }) + t.Run("StartDynamicThemeNotFound", func(t *testing.T) { + app, router, strategy, m := NewApiTest(t) + StartDynamic(router, strategy) + m.EXPECT().RequestSessionGroup("test", gomock.Any()).Return(&sessions.SessionState{}, nil) + r := PerformRequest(app, "GET", "/api/strategies/dynamic?group=test&theme=invalid") + assert.Equal(t, http.StatusNotFound, r.Code) + assert.Equal(t, rfc7807.JSONMediaType, r.Header().Get("Content-Type")) + }) t.Run("StartDynamicByNames", func(t *testing.T) { app, router, strategy, m := NewApiTest(t) StartDynamic(router, strategy) From 6de9ed61201941d393581f2dce7709490bd784a3 Mon Sep 17 00:00:00 2001 From: Alexis Couvreur Date: Sun, 2 Feb 2025 09:40:39 -0500 Subject: [PATCH 2/3] refactor(sessions): change sync.Map to map --- app/sessions/sessions_manager.go | 36 +++++++++++---------------- app/sessions/sessions_manager_test.go | 13 +++++----- app/types/session.go | 1 - go.work.sum | 1 + internal/api/start_dynamic.go | 11 +++----- 5 files changed, 25 insertions(+), 37 deletions(-) delete mode 100644 app/types/session.go diff --git a/app/sessions/sessions_manager.go b/app/sessions/sessions_manager.go index a9ccb52a..2ea1d176 100644 --- a/app/sessions/sessions_manager.go +++ b/app/sessions/sessions_manager.go @@ -110,26 +110,21 @@ type InstanceState struct { } type SessionState struct { - Instances *sync.Map + Instances map[string]InstanceState `json:"instances"` } func (s *SessionState) IsReady() bool { - ready := true - if s.Instances == nil { - s.Instances = &sync.Map{} + s.Instances = map[string]InstanceState{} } - s.Instances.Range(func(key, value interface{}) bool { - state := value.(InstanceState) - if state.Error != nil || state.Instance.Status != instance.Ready { - ready = false + for _, v := range s.Instances { + if v.Error != nil || v.Instance.Status != instance.Ready { return false } - return true - }) + } - return ready + return true } func (s *SessionState) Status() string { @@ -148,7 +143,7 @@ func (s *SessionsManager) RequestSession(names []string, duration time.Duration) var wg sync.WaitGroup sessionState = &SessionState{ - Instances: &sync.Map{}, + Instances: map[string]InstanceState{}, } wg.Add(len(names)) @@ -158,10 +153,10 @@ func (s *SessionsManager) RequestSession(names []string, duration time.Duration) defer wg.Done() state, err := s.requestSessionInstance(name, duration) - sessionState.Instances.Store(name, InstanceState{ + sessionState.Instances[name] = InstanceState{ Instance: state, Error: err, - }) + } }(names[i]) } @@ -250,6 +245,7 @@ func (s *SessionsManager) RequestReadySession(ctx context.Context, names []strin ticker := time.NewTicker(5 * time.Second) readiness := make(chan *SessionState) + errch := make(chan error) quit := make(chan struct{}) go func() { @@ -258,6 +254,7 @@ func (s *SessionsManager) RequestReadySession(ctx context.Context, names []strin case <-ticker.C: session, err := s.RequestSession(names, duration) if err != nil { + errch <- err return } if session.IsReady() { @@ -278,6 +275,9 @@ func (s *SessionsManager) RequestReadySession(ctx context.Context, names []strin case status := <-readiness: close(quit) return status, nil + case err := <-errch: + close(quit) + return nil, err case <-time.After(timeout): close(quit) return nil, fmt.Errorf("session was not ready after %s", timeout.String()) @@ -318,13 +318,7 @@ func (s *SessionsManager) Stop() { } func (s *SessionState) MarshalJSON() ([]byte, error) { - instances := []InstanceState{} - - s.Instances.Range(func(key, value interface{}) bool { - state := value.(InstanceState) - instances = append(instances, state) - return true - }) + instances := maps.Values(s.Instances) return json.Marshal(map[string]any{ "instances": instances, diff --git a/app/sessions/sessions_manager_test.go b/app/sessions/sessions_manager_test.go index 82491548..ecf4134b 100644 --- a/app/sessions/sessions_manager_test.go +++ b/app/sessions/sessions_manager_test.go @@ -2,7 +2,6 @@ package sessions import ( "context" - "sync" "testing" "time" @@ -14,7 +13,7 @@ import ( func TestSessionState_IsReady(t *testing.T) { type fields struct { - Instances *sync.Map + Instances map[string]InstanceState Error error } tests := []struct { @@ -72,17 +71,17 @@ func TestSessionState_IsReady(t *testing.T) { } } -func createMap(instances []*instance.State) (store *sync.Map) { - store = &sync.Map{} +func createMap(instances []*instance.State) map[string]InstanceState { + states := make(map[string]InstanceState) for _, v := range instances { - store.Store(v.Name, InstanceState{ + states[v.Name] = InstanceState{ Instance: v, Error: nil, - }) + } } - return + return states } func TestNewSessionsManagerEvents(t *testing.T) { diff --git a/app/types/session.go b/app/types/session.go deleted file mode 100644 index ab1254f4..00000000 --- a/app/types/session.go +++ /dev/null @@ -1 +0,0 @@ -package types diff --git a/go.work.sum b/go.work.sum index 9ac8a0bd..9dce3667 100644 --- a/go.work.sum +++ b/go.work.sum @@ -125,6 +125,7 @@ github.com/golang-jwt/jwt/v4 v4.0.0 h1:RAqyYixv1p7uEnocuy8P1nru5wprCh/MH2BIlW5z5 github.com/golang-jwt/jwt/v4 v4.0.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg= github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= +github.com/golang/glog v1.2.0/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w= github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= diff --git a/internal/api/start_dynamic.go b/internal/api/start_dynamic.go index a04c54d7..b7d89925 100644 --- a/internal/api/start_dynamic.go +++ b/internal/api/start_dynamic.go @@ -96,15 +96,10 @@ func sessionStateToRenderOptionsInstanceState(sessionState *sessions.SessionStat log.Warnf("sessionStateToRenderOptionsInstanceState: sessionState is nil") return } - sessionState.Instances.Range(func(key, value any) bool { - if value != nil { - instances = append(instances, instanceStateToRenderOptionsRequestState(value.(sessions.InstanceState).Instance)) - } else { - log.Warnf("sessionStateToRenderOptionsInstanceState: sessionState instance is nil, key: %v", key) - } - return true - }) + for _, v := range sessionState.Instances { + instances = append(instances, instanceStateToRenderOptionsRequestState(v.Instance)) + } sort.SliceStable(instances, func(i, j int) bool { return strings.Compare(instances[i].Name, instances[j].Name) == -1 From c7682247421fc0591a964c6424c8854d7d34e405 Mon Sep 17 00:00:00 2001 From: Alexis Couvreur Date: Sun, 2 Feb 2025 09:53:04 -0500 Subject: [PATCH 3/3] test(api): add test with more theme coverage --- go.work.sum | 5 +---- internal/api/start_dynamic_test.go | 24 +++++++++++++++++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/go.work.sum b/go.work.sum index 9dce3667..1965408c 100644 --- a/go.work.sum +++ b/go.work.sum @@ -356,8 +356,6 @@ golang.org/x/crypto v0.28.0 h1:GBDwsMXVQi34v5CCYUm2jkJvu4cbtru2U4TN2PSyQnw= golang.org/x/crypto v0.28.0/go.mod h1:rmgy+3RHxRZMyY0jjAJShp2zgEdOqj2AO7U0pYmeQ7U= golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.18.0 h1:5+9lSbEzPSdWkH32vYPBwEpX8KwDbM52Ud9xBUvNlb0= -golang.org/x/mod v0.18.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/mod v0.19.0 h1:fEdghXQSo20giMthA7cd28ZC+jts4amQ3YMXiP5oMQ8= golang.org/x/mod v0.19.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/mod v0.21.0 h1:vvrHzRwRfVKSiLrG+d4FMl/Qi4ukBCE6kZlTUkDYRT0= @@ -374,6 +372,7 @@ golang.org/x/net v0.30.0 h1:AcW1SDZMkb8IpzCdQUaIq2sP4sZ4zw+55h6ynffypl4= golang.org/x/net v0.30.0/go.mod h1:2wGyMJ5iFasEhkwi13ChkO/t1ECNC4X4eBKkVFyYFlU= golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8= golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= @@ -404,8 +403,6 @@ golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58 golang.org/x/tools v0.14.0/go.mod h1:uYBEerGOWcJyEORxN+Ek8+TT266gXkNlHdJBwexUsBg= golang.org/x/tools v0.16.1/go.mod h1:kYVVN6I1mBNoB1OX+noeBjbRk4IUEPa7JJ+TJMEooJ0= golang.org/x/tools v0.21.0/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= -golang.org/x/tools v0.22.0 h1:gqSGLZqv+AI9lIQzniJ0nZDRG5GBPsSi+DRNHWNz6yA= -golang.org/x/tools v0.22.0/go.mod h1:aCwcsjqvq7Yqt6TNyX7QMU2enbQ/Gt0bo6krSeEri+c= golang.org/x/tools v0.23.0 h1:SGsXPZ+2l4JsgaCKkx+FQ9YZ5XEtA1GZYuoDjenLjvg= golang.org/x/tools v0.23.0/go.mod h1:pnu6ufv6vQkll6szChhK3C3L/ruaIv5eBeztNG8wtsI= golang.org/x/tools v0.25.0 h1:oFU9pkj/iJgs+0DT+VMHrx+oBKs/LJMV+Uvg78sl+fE= diff --git a/internal/api/start_dynamic_test.go b/internal/api/start_dynamic_test.go index 471d3987..298613bb 100644 --- a/internal/api/start_dynamic_test.go +++ b/internal/api/start_dynamic_test.go @@ -2,6 +2,7 @@ package api import ( "errors" + "github.com/sablierapp/sablier/app/instance" "github.com/sablierapp/sablier/app/sessions" "github.com/tniswong/go.rfcx/rfc7807" "go.uber.org/mock/gomock" @@ -10,6 +11,23 @@ import ( "testing" ) +func session() *sessions.SessionState { + state := instance.ReadyInstanceState("test", 1) + state2 := instance.ReadyInstanceState("test2", 1) + return &sessions.SessionState{ + Instances: map[string]sessions.InstanceState{ + "test": { + Instance: &state, + Error: nil, + }, + "test2": { + Instance: &state2, + Error: nil, + }, + }, + } +} + func TestStartDynamic(t *testing.T) { t.Run("StartDynamicInvalidBind", func(t *testing.T) { app, router, strategy, _ := NewApiTest(t) @@ -35,7 +53,7 @@ func TestStartDynamic(t *testing.T) { t.Run("StartDynamicThemeNotFound", func(t *testing.T) { app, router, strategy, m := NewApiTest(t) StartDynamic(router, strategy) - m.EXPECT().RequestSessionGroup("test", gomock.Any()).Return(&sessions.SessionState{}, nil) + m.EXPECT().RequestSessionGroup("test", gomock.Any()).Return(session(), nil) r := PerformRequest(app, "GET", "/api/strategies/dynamic?group=test&theme=invalid") assert.Equal(t, http.StatusNotFound, r.Code) assert.Equal(t, rfc7807.JSONMediaType, r.Header().Get("Content-Type")) @@ -43,7 +61,7 @@ func TestStartDynamic(t *testing.T) { t.Run("StartDynamicByNames", func(t *testing.T) { app, router, strategy, m := NewApiTest(t) StartDynamic(router, strategy) - m.EXPECT().RequestSession([]string{"test"}, gomock.Any()).Return(&sessions.SessionState{}, nil) + m.EXPECT().RequestSession([]string{"test"}, gomock.Any()).Return(session(), nil) r := PerformRequest(app, "GET", "/api/strategies/dynamic?names=test") assert.Equal(t, http.StatusOK, r.Code) assert.Equal(t, SablierStatusReady, r.Header().Get(SablierStatusHeader)) @@ -51,7 +69,7 @@ func TestStartDynamic(t *testing.T) { t.Run("StartDynamicByGroup", func(t *testing.T) { app, router, strategy, m := NewApiTest(t) StartDynamic(router, strategy) - m.EXPECT().RequestSessionGroup("test", gomock.Any()).Return(&sessions.SessionState{}, nil) + m.EXPECT().RequestSessionGroup("test", gomock.Any()).Return(session(), nil) r := PerformRequest(app, "GET", "/api/strategies/dynamic?group=test") assert.Equal(t, http.StatusOK, r.Code) assert.Equal(t, SablierStatusReady, r.Header().Get(SablierStatusHeader))