Skip to content

Commit 57d44b5

Browse files
heiytorgustavosbarreto
authored andcommitted
feat(api, ssh): implement Bulk Writes in DeviceSetOnline
The `DeviceClaims` now includes a `Tenant` attribute. The `AuthDevice` will utilize these attribute to populate the `X-Tenant-ID`. Furthermore, the `/ssh/connection` endpoint has been updated to recognize and utilize these headers for identifying the device's tenant. Additionaly `DeviceSetOnline` method will now use bulk writes instead of single operations. This change should improve performance and reduce MongoDB I/O needs since the method is exclusively used in the `api:heartbeat` queue, which processes a list of devices to set as online. As this method's sole purpose is to set devices as online, it will no longer accept a boolean parameter to set a device as offline. Instead, a new method called `DeviceSetOffline` has been added.
1 parent f48aee0 commit 57d44b5

File tree

19 files changed

+742
-133
lines changed

19 files changed

+742
-133
lines changed

api/routes/auth.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ func (h *Handler) AuthRequest(c gateway.Context) error {
168168

169169
// Extract device UID from JWT and set it into the header.
170170
setHeader(c, client.DeviceUIDHeader, claims.UID)
171+
setHeader(c, "X-Tenant-ID", claims.Tenant)
171172

172173
return c.NoContent(http.StatusOK)
173174
default:

api/routes/device.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func (h *Handler) OfflineDevice(c gateway.Context) error {
175175
return err
176176
}
177177

178-
if err := h.service.OfflineDevice(c.Ctx(), models.UID(req.UID), false); err != nil {
178+
if err := h.service.OfflineDevice(c.Ctx(), models.UID(req.UID)); err != nil {
179179
return err
180180
}
181181

api/routes/device_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,15 +429,15 @@ func TestOfflineDevice(t *testing.T) {
429429
title: "fails when try to setting a non-existing device as offline",
430430
uid: "1234",
431431
requiredMocks: func() {
432-
mock.On("OfflineDevice", gomock.Anything, models.UID("1234"), false).Return(svc.ErrNotFound)
432+
mock.On("OfflineDevice", gomock.Anything, models.UID("1234")).Return(svc.ErrNotFound)
433433
},
434434
expectedStatus: http.StatusNotFound,
435435
},
436436
{
437437
title: "success when try to setting an existing device as offline",
438438
uid: "123",
439439
requiredMocks: func() {
440-
mock.On("OfflineDevice", gomock.Anything, models.UID("123"), false).Return(nil)
440+
mock.On("OfflineDevice", gomock.Anything, models.UID("123")).Return(nil)
441441
},
442442
expectedStatus: http.StatusOK,
443443
},

api/services/auth.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ func (s *service) AuthDevice(ctx context.Context, req requests.DeviceAuth, remot
5858
token, err := jwttoken.New().
5959
WithMethod(jwt.SigningMethodRS256).
6060
WithClaims(&models.DeviceAuthClaims{
61-
UID: key,
61+
UID: key,
62+
Tenant: req.TenantID,
6263
AuthClaims: models.AuthClaims{
6364
Claims: "device",
6465
},

api/services/device.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ package services
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"strings"
78
"time"
89

910
"github.com/shellhub-io/shellhub/api/store"
1011
req "github.com/shellhub-io/shellhub/pkg/api/internalclient"
1112
"github.com/shellhub-io/shellhub/pkg/api/query"
12-
"github.com/shellhub-io/shellhub/pkg/clock"
1313
"github.com/shellhub-io/shellhub/pkg/envs"
1414
"github.com/shellhub-io/shellhub/pkg/models"
1515
"github.com/shellhub-io/shellhub/pkg/validator"
@@ -24,7 +24,7 @@ type DeviceService interface {
2424
DeleteDevice(ctx context.Context, uid models.UID, tenant string) error
2525
RenameDevice(ctx context.Context, uid models.UID, name, tenant string) error
2626
LookupDevice(ctx context.Context, namespace, name string) (*models.Device, error)
27-
OfflineDevice(ctx context.Context, uid models.UID, online bool) error
27+
OfflineDevice(ctx context.Context, uid models.UID) error
2828
UpdateDeviceStatus(ctx context.Context, tenant string, uid models.UID, status models.DeviceStatus) error
2929
UpdateDevice(ctx context.Context, tenant string, uid models.UID, name *string, publicURL *bool) error
3030
}
@@ -176,13 +176,16 @@ func (s *service) LookupDevice(ctx context.Context, namespace, name string) (*mo
176176
return device, nil
177177
}
178178

179-
func (s *service) OfflineDevice(ctx context.Context, uid models.UID, online bool) error {
180-
err := s.store.DeviceSetOnline(ctx, uid, clock.Now(), online)
181-
if err == store.ErrNoDocuments {
182-
return NewErrDeviceNotFound(uid, err)
179+
func (s *service) OfflineDevice(ctx context.Context, uid models.UID) error {
180+
if err := s.store.DeviceSetOffline(ctx, string(uid)); err != nil {
181+
if errors.Is(err, store.ErrNoDocuments) {
182+
return NewErrDeviceNotFound(uid, err)
183+
}
184+
185+
return err
183186
}
184187

185-
return err
188+
return nil
186189
}
187190

188191
// UpdateDeviceStatus updates the device status.

api/services/device_test.go

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,52 +1613,60 @@ func TestLookupDevice(t *testing.T) {
16131613
}
16141614

16151615
func TestOfflineDevice(t *testing.T) {
1616-
mock := new(mocks.Store)
1617-
1618-
ctx := context.TODO()
1616+
storeMock := new(mocks.Store)
16191617

16201618
cases := []struct {
1621-
name string
1622-
uid models.UID
1623-
online bool
1624-
requiredMocks func()
1625-
expected error
1619+
name string
1620+
uid models.UID
1621+
mocks func(context.Context)
1622+
expected error
16261623
}{
16271624
{
1628-
name: "fails when store device online fails",
1625+
name: "fails when operation does not succeeds",
16291626
uid: models.UID("uid"),
1630-
requiredMocks: func() {
1631-
clockMock.On("Now").Return(now).Once()
1632-
mock.On("DeviceSetOnline", ctx, models.UID("uid"), now, false).
1633-
Return(errors.New("error", "", 0)).Once()
1627+
mocks: func(ctx context.Context) {
1628+
storeMock.
1629+
On("DeviceSetOffline", ctx, "uid").
1630+
Return(errors.New("error", "", 0)).
1631+
Once()
16341632
},
16351633
expected: errors.New("error", "", 0),
16361634
},
16371635
{
1638-
name: "succeeds",
1639-
uid: models.UID("uid"),
1640-
online: true,
1641-
requiredMocks: func() {
1642-
online := true
1643-
clockMock.On("Now").Return(now).Once()
1644-
mock.On("DeviceSetOnline", ctx, models.UID("uid"), now, online).
1645-
Return(errors.New("error", "", 0)).Once()
1636+
name: "fails when connected_device does not exist",
1637+
uid: models.UID("uid"),
1638+
mocks: func(ctx context.Context) {
1639+
storeMock.
1640+
On("DeviceSetOffline", ctx, "uid").
1641+
Return(store.ErrNoDocuments).
1642+
Once()
16461643
},
1647-
expected: errors.New("error", "", 0),
1644+
expected: NewErrDeviceNotFound(models.UID("uid"), store.ErrNoDocuments),
1645+
},
1646+
{
1647+
name: "succeeds",
1648+
uid: models.UID("uid"),
1649+
mocks: func(ctx context.Context) {
1650+
storeMock.
1651+
On("DeviceSetOffline", ctx, "uid").
1652+
Return(nil).
1653+
Once()
1654+
},
1655+
expected: nil,
16481656
},
16491657
}
16501658

1659+
s := NewService(store.Store(storeMock), privateKey, publicKey, storecache.NewNullCache(), clientMock, nil)
1660+
16511661
for _, tc := range cases {
16521662
t.Run(tc.name, func(t *testing.T) {
1653-
tc.requiredMocks()
1654-
1655-
service := NewService(store.Store(mock), privateKey, publicKey, storecache.NewNullCache(), clientMock, nil)
1656-
err := service.OfflineDevice(ctx, tc.uid, tc.online)
1657-
assert.Equal(t, tc.expected, err)
1663+
ctx := context.Background()
1664+
tc.mocks(ctx)
1665+
assert.Equal(t, tc.expected, s.OfflineDevice(ctx, tc.uid))
16581666
})
16591667
}
16601668

1661-
mock.AssertExpectations(t)
1669+
storeMock.AssertExpectations(t)
16621670
}
16631671

16641672
func TestUpdateDeviceStatus_same_mac(t *testing.T) {

api/services/mocks/services.go

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

api/store/device.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ type DeviceStore interface {
2828
DeviceCreate(ctx context.Context, d models.Device, hostname string) error
2929
DeviceRename(ctx context.Context, uid models.UID, hostname string) error
3030
DeviceLookup(ctx context.Context, namespace, hostname string) (*models.Device, error)
31-
DeviceSetOnline(ctx context.Context, uid models.UID, timestamp time.Time, online bool) error
3231
DeviceUpdateOnline(ctx context.Context, uid models.UID, online bool) error
3332
DeviceUpdateLastSeen(ctx context.Context, uid models.UID, ts time.Time) error
3433
DeviceUpdateStatus(ctx context.Context, uid models.UID, status models.DeviceStatus) error
@@ -45,4 +44,11 @@ type DeviceStore interface {
4544
DeviceRemovedList(ctx context.Context, tenant string, pagination query.Paginator, filters query.Filters, sorter query.Sorter) ([]models.DeviceRemoved, int, error)
4645
DeviceCreatePublicURLAddress(ctx context.Context, uid models.UID) error
4746
DeviceGetByPublicURLAddress(ctx context.Context, address string) (*models.Device, error)
47+
48+
// DeviceSetOnline receives a list of devices to mark as online. For each device in the array, it will upsert
49+
// a connected device entry; each UID must exists in the "devices" collection.
50+
DeviceSetOnline(ctx context.Context, connectedDevices []models.ConnectedDevice) error
51+
52+
// DeviceSetOffline sets a device's status to offline using its UID.
53+
DeviceSetOffline(ctx context.Context, uid string) error
4854
}

0 commit comments

Comments
 (0)