Skip to content

Commit cf343f1

Browse files
heiytorheiytor
committed
refactor(api): update Device interface to use entity objects
- Change DeviceUpdate to accept *models.Device instead of separate tenant/uid/changes parameters - Change DeviceDelete to accept *models.Device instead of models.UID - Rename DeviceBulkUpdate to DeviceHeartbeat with simplified signature - Add DeviceDeleteMany method for batch device deletion operations - Update service layer to resolve devices before update/delete operations - Add proper BSON marshaling with ObjectID conversion for tag_ids - Remove DeviceChanges model usage in favor of direct entity updates - Update all service methods to use new interface signatures - Add clock mocking support for timestamp validation in tests - Update task cleanup to use batch operations for better performance - Fix merge device operations to use entity objects - Add proper error handling for device resolution in all flows - Update store implementation with transaction support for bulk operations - Cache invalidation now handles both single and batch operations BREAKING CHANGE: DeviceUpdate, DeviceDelete method signatures changed, DeviceBulkUpdate renamed to DeviceHeartbeat with new signature
1 parent 561ea17 commit cf343f1

File tree

11 files changed

+883
-464
lines changed

11 files changed

+883
-464
lines changed

‎api/services/auth.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,16 +159,18 @@ func (s *service) AuthDevice(ctx context.Context, req requests.DeviceAuth) (*mod
159159
return nil, err
160160
}
161161
} else {
162-
changes := &models.DeviceChanges{LastSeen: clock.Now(), DisconnectedAt: nil, RemovedAt: device.RemovedAt}
162+
device.LastSeen = clock.Now()
163+
device.DisconnectedAt = nil
164+
163165
if device.RemovedAt != nil {
164-
changes.Status = models.DeviceStatusPending
166+
device.Status = models.DeviceStatusPending
165167
if err := s.store.NamespaceIncrementDeviceCount(ctx, req.TenantID, models.DeviceStatusPending, 1); err != nil {
166168
return nil, err
167169
}
168170
}
169171

170172
if req.Info != nil {
171-
changes.Info = &models.DeviceInfo{
173+
device.Info = &models.DeviceInfo{
172174
ID: req.Info.ID,
173175
PrettyName: req.Info.PrettyName,
174176
Version: req.Info.Version,
@@ -177,7 +179,7 @@ func (s *service) AuthDevice(ctx context.Context, req requests.DeviceAuth) (*mod
177179
}
178180
}
179181

180-
if err := s.store.DeviceUpdate(ctx, req.TenantID, uid, changes); err != nil {
182+
if err := s.store.DeviceUpdate(ctx, device); err != nil {
181183
log.WithError(err).Error("failed to updated device to online")
182184

183185
return nil, err

‎api/services/auth_test.go‎

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ func TestAuthDevice(t *testing.T) {
173173
},
174174
requiredMocks: func(ctx context.Context) {
175175
uid := toUID("00000000-0000-4000-0000-000000000000", "hostname", "", "")
176+
device := &models.Device{UID: uid, Name: "hostname"}
176177

177178
storeMock.
178179
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
@@ -184,16 +185,22 @@ func TestAuthDevice(t *testing.T) {
184185
Once()
185186
storeMock.
186187
On("DeviceResolve", ctx, store.DeviceUIDResolver, uid).
187-
Return(&models.Device{UID: uid, Name: "hostname"}, nil).
188+
Return(device, nil).
188189
Once()
190+
191+
expectedDevice := *device
192+
expectedDevice.LastSeen = now
193+
expectedDevice.DisconnectedAt = nil
194+
expectedDevice.Info = &models.DeviceInfo{
195+
ID: "test",
196+
PrettyName: "Test",
197+
Version: "v0.20.0",
198+
Arch: "arch64",
199+
Platform: "native",
200+
}
201+
189202
storeMock.
190-
On("DeviceUpdate", ctx, "00000000-0000-4000-0000-000000000000", uid, &models.DeviceChanges{Info: &models.DeviceInfo{
191-
ID: "test",
192-
PrettyName: "Test",
193-
Version: "v0.20.0",
194-
Arch: "arch64",
195-
Platform: "native",
196-
}, LastSeen: now, DisconnectedAt: nil}).
203+
On("DeviceUpdate", ctx, &expectedDevice).
197204
Return(errors.New("error", "store", 0)).
198205
Once()
199206
},
@@ -214,6 +221,7 @@ func TestAuthDevice(t *testing.T) {
214221
},
215222
requiredMocks: func(ctx context.Context) {
216223
uid := toUID("00000000-0000-4000-0000-000000000000", "hostname", "", "")
224+
device := &models.Device{UID: uid, Name: "hostname"}
217225

218226
storeMock.
219227
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
@@ -225,10 +233,15 @@ func TestAuthDevice(t *testing.T) {
225233
Once()
226234
storeMock.
227235
On("DeviceResolve", ctx, store.DeviceUIDResolver, uid).
228-
Return(&models.Device{UID: uid, Name: "hostname"}, nil).
236+
Return(device, nil).
229237
Once()
238+
239+
expectedDevice := *device
240+
expectedDevice.LastSeen = now
241+
expectedDevice.DisconnectedAt = nil
242+
230243
storeMock.
231-
On("DeviceUpdate", ctx, "00000000-0000-4000-0000-000000000000", uid, &models.DeviceChanges{LastSeen: now, DisconnectedAt: nil}).
244+
On("DeviceUpdate", ctx, &expectedDevice).
232245
Return(errors.New("error", "store", 0)).
233246
Once()
234247
},
@@ -250,6 +263,7 @@ func TestAuthDevice(t *testing.T) {
250263
},
251264
requiredMocks: func(ctx context.Context) {
252265
uid := toUID("00000000-0000-4000-0000-000000000000", "hostname", "", "")
266+
device := &models.Device{UID: uid, Name: "hostname"}
253267

254268
storeMock.
255269
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
@@ -261,10 +275,15 @@ func TestAuthDevice(t *testing.T) {
261275
Once()
262276
storeMock.
263277
On("DeviceResolve", ctx, store.DeviceUIDResolver, uid).
264-
Return(&models.Device{UID: uid, Name: "hostname"}, nil).
278+
Return(device, nil).
265279
Once()
280+
281+
expectedDevice := *device
282+
expectedDevice.LastSeen = now
283+
expectedDevice.DisconnectedAt = nil
284+
266285
storeMock.
267-
On("DeviceUpdate", ctx, "00000000-0000-4000-0000-000000000000", uid, &models.DeviceChanges{LastSeen: now, DisconnectedAt: nil}).
286+
On("DeviceUpdate", ctx, &expectedDevice).
268287
Return(nil).
269288
Once()
270289
cacheMock.
@@ -295,6 +314,7 @@ func TestAuthDevice(t *testing.T) {
295314
},
296315
requiredMocks: func(ctx context.Context) {
297316
uid := toUID("00000000-0000-4000-0000-000000000000", "hostname", "", "")
317+
device := &models.Device{UID: uid, Name: "hostname"}
298318

299319
storeMock.
300320
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
@@ -306,10 +326,15 @@ func TestAuthDevice(t *testing.T) {
306326
Once()
307327
storeMock.
308328
On("DeviceResolve", ctx, store.DeviceUIDResolver, uid).
309-
Return(&models.Device{UID: uid, Name: "hostname"}, nil).
329+
Return(device, nil).
310330
Once()
331+
332+
expectedDevice := *device
333+
expectedDevice.LastSeen = now
334+
expectedDevice.DisconnectedAt = nil
335+
311336
storeMock.
312-
On("DeviceUpdate", ctx, "00000000-0000-4000-0000-000000000000", uid, &models.DeviceChanges{LastSeen: now, DisconnectedAt: nil}).
337+
On("DeviceUpdate", ctx, &expectedDevice).
313338
Return(nil).
314339
Once()
315340
storeMock.
@@ -354,6 +379,7 @@ func TestAuthDevice(t *testing.T) {
354379
},
355380
requiredMocks: func(ctx context.Context) {
356381
uid := toUID("00000000-0000-4000-0000-000000000000", "hostname", "", "")
382+
device := &models.Device{UID: uid, Name: "hostname"}
357383

358384
storeMock.
359385
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
@@ -365,16 +391,22 @@ func TestAuthDevice(t *testing.T) {
365391
Once()
366392
storeMock.
367393
On("DeviceResolve", ctx, store.DeviceUIDResolver, uid).
368-
Return(&models.Device{UID: uid, Name: "hostname"}, nil).
394+
Return(device, nil).
369395
Once()
396+
397+
expectedDevice := *device
398+
expectedDevice.LastSeen = now
399+
expectedDevice.DisconnectedAt = nil
400+
expectedDevice.Info = &models.DeviceInfo{
401+
ID: "test",
402+
PrettyName: "Test",
403+
Version: "v0.20.0",
404+
Arch: "arch64",
405+
Platform: "native",
406+
}
407+
370408
storeMock.
371-
On("DeviceUpdate", ctx, "00000000-0000-4000-0000-000000000000", uid, &models.DeviceChanges{Info: &models.DeviceInfo{
372-
ID: "test",
373-
PrettyName: "Test",
374-
Version: "v0.20.0",
375-
Arch: "arch64",
376-
Platform: "native",
377-
}, LastSeen: now, DisconnectedAt: nil}).
409+
On("DeviceUpdate", ctx, &expectedDevice).
378410
Return(nil).
379411
Once()
380412
cacheMock.
@@ -405,6 +437,7 @@ func TestAuthDevice(t *testing.T) {
405437
},
406438
requiredMocks: func(ctx context.Context) {
407439
uid := toUID("00000000-0000-4000-0000-000000000000", "hostname", "", "")
440+
device := &models.Device{UID: uid, Name: "hostname", RemovedAt: &now, Status: models.DeviceStatusRemoved}
408441

409442
storeMock.
410443
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
@@ -416,20 +449,21 @@ func TestAuthDevice(t *testing.T) {
416449
Once()
417450
storeMock.
418451
On("DeviceResolve", ctx, store.DeviceUIDResolver, uid).
419-
Return(&models.Device{UID: uid, Name: "hostname", RemovedAt: &now}, nil).
452+
Return(device, nil).
420453
Once()
421454
storeMock.
422455
On("NamespaceIncrementDeviceCount", ctx, "00000000-0000-4000-0000-000000000000", models.DeviceStatusPending, int64(1)).
423456
Return(nil).
424457
Once()
458+
459+
expectedDevice := *device
460+
expectedDevice.LastSeen = now
461+
expectedDevice.DisconnectedAt = nil
462+
expectedDevice.RemovedAt = &now
463+
expectedDevice.Status = models.DeviceStatusPending
464+
425465
storeMock.
426-
On(
427-
"DeviceUpdate",
428-
ctx,
429-
"00000000-0000-4000-0000-000000000000",
430-
uid,
431-
&models.DeviceChanges{Info: nil, LastSeen: now, DisconnectedAt: nil, RemovedAt: &now, Status: models.DeviceStatusPending},
432-
).
466+
On("DeviceUpdate", ctx, device).
433467
Return(nil).
434468
Once()
435469
cacheMock.

‎api/services/device.go‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -153,16 +153,19 @@ func (s *service) DeleteDevice(ctx context.Context, uid models.UID, tenant strin
153153
// namespace and void the device switching.
154154
if envs.IsCloud() && !ns.Billing.IsActive() && device.Status == models.DeviceStatusAccepted {
155155
now := clock.Now()
156-
changes := &models.DeviceChanges{DisconnectedAt: device.DisconnectedAt, RemovedAt: &now, Status: models.DeviceStatusRemoved}
157-
if err := s.store.DeviceUpdate(ctx, tenant, string(uid), changes); err != nil {
156+
157+
deviceCopy := *device
158+
deviceCopy.Status = models.DeviceStatusRemoved
159+
deviceCopy.RemovedAt = &now
160+
if err := s.store.DeviceUpdate(ctx, &deviceCopy); err != nil {
158161
return err
159162
}
160163

161164
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, models.DeviceStatusRemoved, 1); err != nil {
162165
return err
163166
}
164167
} else {
165-
if err := s.store.DeviceDelete(ctx, uid); err != nil {
168+
if err := s.store.DeviceDelete(ctx, device); err != nil {
166169
return err
167170
}
168171
}
@@ -184,8 +187,8 @@ func (s *service) RenameDevice(ctx context.Context, uid models.UID, name, tenant
184187
return nil
185188
}
186189

187-
changes := &models.DeviceChanges{DisconnectedAt: device.DisconnectedAt, RemovedAt: device.RemovedAt, Name: strings.ToLower(name)}
188-
if err := s.store.DeviceUpdate(ctx, device.TenantID, string(uid), changes); err != nil { // nolint:revive
190+
device.Name = strings.ToLower(name)
191+
if err := s.store.DeviceUpdate(ctx, device); err != nil { // nolint:revive
189192
return err
190193
}
191194

@@ -222,8 +225,8 @@ func (s *service) OfflineDevice(ctx context.Context, uid models.UID) error {
222225
}
223226

224227
now := clock.Now()
225-
changes := &models.DeviceChanges{RemovedAt: device.RemovedAt, DisconnectedAt: &now}
226-
if err := s.store.DeviceUpdate(ctx, "", string(uid), changes); err != nil {
228+
device.DisconnectedAt = &now
229+
if err := s.store.DeviceUpdate(ctx, device); err != nil { // nolint:revive
227230
if errors.Is(err, store.ErrNoDocuments) {
228231
return NewErrDeviceNotFound(uid, err)
229232
}
@@ -342,8 +345,9 @@ func (s *service) updateDeviceStatus(req *requests.DeviceUpdateStatus) store.Tra
342345
}
343346
}
344347

345-
changes := &models.DeviceChanges{RemovedAt: device.RemovedAt, DisconnectedAt: device.DisconnectedAt, Status: newStatus}
346-
if err := s.store.DeviceUpdate(ctx, namespace.TenantID, device.UID, changes); err != nil {
348+
device.Status = newStatus
349+
device.StatusUpdatedAt = clock.Now()
350+
if err := s.store.DeviceUpdate(ctx, device); err != nil {
347351
return err
348352
}
349353

@@ -369,13 +373,15 @@ func (s *service) UpdateDevice(ctx context.Context, req *requests.DeviceUpdate)
369373
return NewErrDeviceDuplicated(req.Name, err)
370374
}
371375

372-
// We pass DisconnectedAt because we don't want to update it to nil
373-
changes := &models.DeviceChanges{DisconnectedAt: device.DisconnectedAt, RemovedAt: device.RemovedAt}
374-
if req.Name != "" && strings.ToLower(req.Name) != device.Name {
375-
changes.Name = strings.ToLower(req.Name)
376+
if req.Name != "" && !strings.EqualFold(req.Name, device.Name) {
377+
device.Name = strings.ToLower(req.Name)
376378
}
377379

378-
return s.store.DeviceUpdate(ctx, req.TenantID, req.UID, changes)
380+
if err := s.store.DeviceUpdate(ctx, device); err != nil { // nolint:revive
381+
return err
382+
}
383+
384+
return nil
379385
}
380386

381387
// mergeDevice merges an old device into a new device. It transfers all sessions from the old device to the new one and
@@ -389,12 +395,12 @@ func (s *service) mergeDevice(ctx context.Context, tenantID string, oldDevice *m
389395
return err
390396
}
391397

392-
changes := &models.DeviceChanges{DisconnectedAt: newDevice.DisconnectedAt, RemovedAt: newDevice.RemovedAt, Name: oldDevice.Name}
393-
if err := s.store.DeviceUpdate(ctx, tenantID, newDevice.UID, changes); err != nil {
398+
newDevice.Name = oldDevice.Name
399+
if err := s.store.DeviceUpdate(ctx, newDevice); err != nil {
394400
return err
395401
}
396402

397-
if err := s.store.DeviceDelete(ctx, models.UID(oldDevice.UID)); err != nil {
403+
if err := s.store.DeviceDelete(ctx, oldDevice); err != nil {
398404
return err
399405
}
400406

0 commit comments

Comments
 (0)