Skip to content

Commit 5d8c11b

Browse files
heiytorgustavosbarreto
authored andcommitted
refactor(api): remove gateway dependency from GetStats store method
- Change GetStats signature to receive tenantID parameter instead of extracting from gateway context - Update service layer to extract tenantID and pass to store - Update all tests to use new tenantID parameter signature - Add comprehensive test coverage for tenant filtering scenarios - Document GetStats interface method with tenant behavior
1 parent 7750b81 commit 5d8c11b

File tree

10 files changed

+219
-42
lines changed

10 files changed

+219
-42
lines changed

api/routes/stats.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,16 @@ const (
1414
)
1515

1616
func (h *Handler) GetStats(c gateway.Context) error {
17-
stats, err := h.service.GetStats(c.Ctx())
17+
req := new(requests.GetStats)
18+
if err := c.Bind(req); err != nil {
19+
return err
20+
}
21+
22+
if err := c.Validate(req); err != nil {
23+
return err
24+
}
25+
26+
stats, err := h.service.GetStats(c.Ctx(), req)
1827
if err != nil {
1928
return err
2029
}

api/routes/stats_test.go

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,20 +75,44 @@ func TestGetStats(t *testing.T) {
7575
cases := []struct {
7676
title string
7777
reqStats *models.Stats
78+
headers map[string]string
7879
expectedStatus int
7980
requiredMocks func()
8081
}{
8182
{
82-
title: "success when try to get an stats",
83+
title: "success when try to get stats without tenantID",
8384
reqStats: &models.Stats{
8485
RegisteredDevices: 10,
8586
OnlineDevices: 5,
8687
ActiveSessions: 20,
8788
PendingDevices: 3,
8889
RejectedDevices: 2,
8990
},
91+
headers: map[string]string{
92+
"Content-Type": "application/json",
93+
"X-Role": authorizer.RoleOwner.String(),
94+
},
9095
requiredMocks: func() {
91-
mock.On("GetStats", gomock.Anything).Return(&models.Stats{}, nil)
96+
mock.On("GetStats", gomock.Anything, &requests.GetStats{TenantID: ""}).Return(&models.Stats{}, nil)
97+
},
98+
expectedStatus: http.StatusOK,
99+
},
100+
{
101+
title: "success when try to get stats with tenantID",
102+
reqStats: &models.Stats{
103+
RegisteredDevices: 5,
104+
OnlineDevices: 2,
105+
ActiveSessions: 10,
106+
PendingDevices: 1,
107+
RejectedDevices: 0,
108+
},
109+
headers: map[string]string{
110+
"Content-Type": "application/json",
111+
"X-Role": authorizer.RoleOwner.String(),
112+
"X-Tenant-ID": "00000000-0000-4000-0000-000000000000",
113+
},
114+
requiredMocks: func() {
115+
mock.On("GetStats", gomock.Anything, &requests.GetStats{TenantID: "00000000-0000-4000-0000-000000000000"}).Return(&models.Stats{}, nil)
92116
},
93117
expectedStatus: http.StatusOK,
94118
},
@@ -100,8 +124,10 @@ func TestGetStats(t *testing.T) {
100124

101125
req := httptest.NewRequest(http.MethodGet, "/api/stats", nil)
102126

103-
req.Header.Set("Content-Type", "application/json")
104-
req.Header.Set("X-Role", authorizer.RoleOwner.String())
127+
for key, value := range tc.headers {
128+
req.Header.Set(key, value)
129+
}
130+
105131
rec := httptest.NewRecorder()
106132

107133
e := NewRouter(mock)

api/services/mocks/services.go

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/services/stats.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@ package services
33
import (
44
"context"
55

6+
"github.com/shellhub-io/shellhub/pkg/api/requests"
67
"github.com/shellhub-io/shellhub/pkg/models"
78
)
89

910
type StatsService interface {
10-
GetStats(ctx context.Context) (*models.Stats, error)
11+
GetStats(ctx context.Context, req *requests.GetStats) (*models.Stats, error)
1112
}
1213

13-
func (s *service) GetStats(ctx context.Context) (*models.Stats, error) {
14-
return s.store.GetStats(ctx)
14+
func (s *service) GetStats(ctx context.Context, req *requests.GetStats) (*models.Stats, error) {
15+
return s.store.GetStats(ctx, req.TenantID)
1516
}

api/services/stats_test.go

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package services
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
8+
"github.com/shellhub-io/shellhub/api/store/mocks"
9+
"github.com/shellhub-io/shellhub/pkg/api/requests"
10+
storecache "github.com/shellhub-io/shellhub/pkg/cache"
11+
"github.com/shellhub-io/shellhub/pkg/models"
12+
"github.com/stretchr/testify/assert"
13+
gomock "github.com/stretchr/testify/mock"
14+
)
15+
16+
func TestGetStats(t *testing.T) {
17+
storeMock := &mocks.Store{}
18+
19+
ctx := context.Background()
20+
21+
cases := []struct {
22+
description string
23+
req *requests.GetStats
24+
expectedStats *models.Stats
25+
expectedError error
26+
requiredMocks func()
27+
}{
28+
{
29+
description: "fail when store returns error",
30+
req: &requests.GetStats{
31+
TenantID: "00000000-0000-4000-0000-000000000000",
32+
},
33+
expectedStats: nil,
34+
expectedError: errors.New("store error"),
35+
requiredMocks: func() {
36+
storeMock.
37+
On("GetStats", gomock.Anything, "00000000-0000-4000-0000-000000000000").
38+
Return(nil, errors.New("store error")).
39+
Once()
40+
},
41+
},
42+
{
43+
description: "success when getting stats without tenantID",
44+
req: &requests.GetStats{
45+
TenantID: "",
46+
},
47+
expectedStats: &models.Stats{
48+
RegisteredDevices: 10,
49+
OnlineDevices: 5,
50+
ActiveSessions: 15,
51+
PendingDevices: 2,
52+
RejectedDevices: 1,
53+
},
54+
expectedError: nil,
55+
requiredMocks: func() {
56+
storeMock.On("GetStats", gomock.Anything, "").
57+
Return(
58+
&models.Stats{
59+
RegisteredDevices: 10,
60+
OnlineDevices: 5,
61+
ActiveSessions: 15,
62+
PendingDevices: 2,
63+
RejectedDevices: 1,
64+
},
65+
nil,
66+
).
67+
Once()
68+
},
69+
},
70+
{
71+
description: "success when getting stats with tenantID",
72+
req: &requests.GetStats{
73+
TenantID: "00000000-0000-4000-0000-000000000000",
74+
},
75+
expectedStats: &models.Stats{
76+
RegisteredDevices: 3,
77+
OnlineDevices: 2,
78+
ActiveSessions: 5,
79+
PendingDevices: 1,
80+
RejectedDevices: 0,
81+
},
82+
expectedError: nil,
83+
requiredMocks: func() {
84+
storeMock.
85+
On("GetStats", gomock.Anything, "00000000-0000-4000-0000-000000000000").
86+
Return(
87+
&models.Stats{
88+
RegisteredDevices: 3,
89+
OnlineDevices: 2,
90+
ActiveSessions: 5,
91+
PendingDevices: 1,
92+
RejectedDevices: 0,
93+
},
94+
nil,
95+
).
96+
Once()
97+
},
98+
},
99+
}
100+
101+
s := NewService(storeMock, privateKey, publicKey, storecache.NewNullCache(), clientMock)
102+
103+
for _, tc := range cases {
104+
t.Run(tc.description, func(t *testing.T) {
105+
tc.requiredMocks()
106+
107+
stats, err := s.GetStats(ctx, tc.req)
108+
assert.Equal(t, tc.expectedStats, stats)
109+
assert.Equal(t, tc.expectedError, err)
110+
})
111+
}
112+
113+
storeMock.AssertExpectations(t)
114+
}

api/store/mocks/store.go

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/store/mongo/stats.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,12 @@ import (
44
"context"
55
"time"
66

7-
"github.com/shellhub-io/shellhub/api/pkg/gateway"
87
"github.com/shellhub-io/shellhub/pkg/models"
98
"go.mongodb.org/mongo-driver/bson"
109
"go.mongodb.org/mongo-driver/bson/primitive"
1110
)
1211

13-
func (s *Store) GetStats(ctx context.Context) (*models.Stats, error) {
14-
var tenantID string
15-
if tenant := gateway.TenantFromContext(ctx); tenant != nil {
16-
tenantID = tenant.ID
17-
}
18-
12+
func (s *Store) GetStats(ctx context.Context, tenantID string) (*models.Stats, error) {
1913
onlineDevicesQuery := buildOnlineDevicesQuery(tenantID)
2014
onlineDevices, err := CountAllMatchingDocuments(ctx, s.db.Collection("devices"), onlineDevicesQuery)
2115
if err != nil {

api/store/mongo/stats_test.go

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,28 @@ func TestGetStats(t *testing.T) {
1717
cases := []struct {
1818
description string
1919
fixtures []string
20+
tenantID string
2021
expected Expected
2122
}{
2223
{
23-
description: "succeeds",
24-
fixtures: []string{
25-
fixtureUsers,
26-
fixtureNamespaces,
27-
fixtureSessions,
28-
fixtureActiveSessions,
29-
fixtureDevices,
24+
description: "succeeds without tenantID",
25+
fixtures: []string{fixtureUsers, fixtureNamespaces, fixtureSessions, fixtureActiveSessions, fixtureDevices},
26+
tenantID: "",
27+
expected: Expected{
28+
stats: &models.Stats{
29+
RegisteredDevices: 3,
30+
OnlineDevices: 0,
31+
ActiveSessions: 1,
32+
PendingDevices: 1,
33+
RejectedDevices: 0,
34+
},
35+
err: nil,
3036
},
37+
},
38+
{
39+
description: "succeeds with specific tenantID",
40+
fixtures: []string{fixtureUsers, fixtureNamespaces, fixtureSessions, fixtureActiveSessions, fixtureDevices},
41+
tenantID: "00000000-0000-4000-0000-000000000000",
3142
expected: Expected{
3243
stats: &models.Stats{
3344
RegisteredDevices: 3,
@@ -39,6 +50,21 @@ func TestGetStats(t *testing.T) {
3950
err: nil,
4051
},
4152
},
53+
{
54+
description: "succeeds with non-existent tenantID",
55+
fixtures: []string{fixtureUsers, fixtureNamespaces, fixtureSessions, fixtureActiveSessions, fixtureDevices},
56+
tenantID: "99999999-9999-4999-9999-999999999999",
57+
expected: Expected{
58+
stats: &models.Stats{
59+
RegisteredDevices: 0,
60+
OnlineDevices: 0,
61+
ActiveSessions: 0,
62+
PendingDevices: 0,
63+
RejectedDevices: 0,
64+
},
65+
err: nil,
66+
},
67+
},
4268
}
4369

4470
for _, tc := range cases {
@@ -50,7 +76,7 @@ func TestGetStats(t *testing.T) {
5076
assert.NoError(t, srv.Reset())
5177
})
5278

53-
stats, err := s.GetStats(ctx)
79+
stats, err := s.GetStats(ctx, tc.tenantID)
5480
assert.Equal(t, tc.expected, Expected{stats: stats, err: err})
5581
})
5682
}

api/store/stats.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,7 @@ import (
77
)
88

99
type StatsStore interface {
10-
GetStats(ctx context.Context) (*models.Stats, error)
10+
// GetStats retrieves device and session statistics. If tenantID is provided,
11+
// statistics are filtered to that tenant. If empty, returns global statistics.
12+
GetStats(ctx context.Context, tenantID string) (*models.Stats, error)
1113
}

pkg/api/requests/stats.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package requests
2+
3+
type GetStats struct {
4+
TenantID string `header:"X-Tenant-ID"`
5+
}

0 commit comments

Comments
 (0)