Skip to content

Commit cb47f92

Browse files
heiytorgustavosbarreto
authored andcommitted
refactor(api): consolidate device removal system into single collection
- Remove DeviceRemoved model and related APIs from store interface - Update device deletion to use status "removed" instead of separate collection - Add DeviceCleanup cron job to permanently delete devices after 30 days - Add migrations 103-106 to convert existing data and optimize performance: * Migration 103: Convert removed_devices to devices with status "removed" * Migration 104: Add devices_removed_count field to namespaces * Migration 105: Drop obsolete removed_devices collection * Migration 106: Add status+status_updated_at index for efficient cleanup queries - Update namespace limit checks to use devices_removed_count field - Add "lt" and "gt" operator support for time.Time filtering - Refactor DeviceList to use status-based acceptable logic instead of lookup - Update all tests to use new unified device management APIs - Remove DeviceRemoved-related error types and helper functions - Update fixtures and test data to include devices_removed_count field - DeviceRemovedGet, DeviceRemovedInsert, DeviceRemovedDelete APIs removed - DeviceRemovedList replaced with DeviceList using status filter - Error types ErrDeviceRemovedCount, ErrDeviceRemovedInsert, etc. removed Notes: - Migration 103 rollback has known limitation: doesn't preserve original device status before removal (devices will have status="removed" in rollback instead of original pre-deletion status like "accepted")
1 parent 829ee1d commit cb47f92

25 files changed

+1620
-724
lines changed

api/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ func (s *Server) Setup(ctx context.Context) error {
106106
)
107107

108108
s.worker.HandleTask(services.TaskDevicesHeartbeat, service.DevicesHeartbeat(), asynq.BatchTask())
109+
s.worker.HandleCron(services.CronDeviceCleanup, service.DeviceCleanup(), asynq.Unique())
109110

110111
log.Info("Server setup completed successfully")
111112

api/services/device.go

Lines changed: 16 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,7 @@ type DeviceService interface {
3636

3737
func (s *service) ListDevices(ctx context.Context, req *requests.DeviceList) ([]models.Device, int, error) {
3838
if req.DeviceStatus == models.DeviceStatusRemoved {
39-
// TODO: unique DeviceList
40-
removed, count, err := s.store.DeviceRemovedList(ctx, req.TenantID, req.Paginator, req.Filters, req.Sorter)
41-
if err != nil {
42-
return nil, 0, err
43-
}
44-
45-
devices := make([]models.Device, 0, len(removed))
46-
for _, device := range removed {
47-
devices = append(devices, *device.Device)
48-
}
49-
50-
return devices, count, nil
39+
return s.store.DeviceList(ctx, req.DeviceStatus, req.Paginator, req.Filters, req.Sorter, store.DeviceAcceptableFromRemoved)
5140
}
5241

5342
if req.TenantID != "" {
@@ -59,12 +48,7 @@ func (s *service) ListDevices(ctx context.Context, req *requests.DeviceList) ([]
5948
if ns.HasMaxDevices() {
6049
switch {
6150
case envs.IsCloud():
62-
removed, err := s.store.DeviceRemovedCount(ctx, ns.TenantID)
63-
if err != nil {
64-
return nil, 0, NewErrDeviceRemovedCount(err)
65-
}
66-
67-
if ns.HasLimitDevicesReached(removed) {
51+
if ns.HasLimitDevicesReached() {
6852
return s.store.DeviceList(ctx, req.DeviceStatus, req.Paginator, req.Filters, req.Sorter, store.DeviceAcceptableFromRemoved)
6953
}
7054
case envs.IsEnterprise():
@@ -130,25 +114,24 @@ func (s *service) DeleteDevice(ctx context.Context, uid models.UID, tenant strin
130114
return NewErrNamespaceNotFound(tenant, err)
131115
}
132116

133-
// This is a workaround for a current limitation: store.DeviceRemovedInsert internally updates
134-
// the device's status to models.DeviceStatusRemoved, so we need to preserve the original
135-
// device status before the update.
136-
originalStatus := device.Status
137-
138117
// NOTE: If the namespace has a limit of devices, we change the device's slot status to removed only when it is
139118
// [models.DeviceStatusAccepted]. This way, we can keep track of the number of devices that were removed from the
140119
// namespace and void the device switching.
141-
if envs.IsCloud() && envs.HasBilling() && !ns.Billing.IsActive() && originalStatus == models.DeviceStatusAccepted {
142-
if err := s.store.DeviceRemovedInsert(ctx, tenant, device); err != nil {
143-
return NewErrDeviceRemovedInsert(err)
120+
if envs.IsCloud() && envs.HasBilling() && !ns.Billing.IsActive() && device.Status == models.DeviceStatusAccepted {
121+
if err := s.store.DeviceUpdate(ctx, tenant, string(uid), &models.DeviceChanges{Status: models.DeviceStatusRemoved}); err != nil {
122+
return err
144123
}
145-
}
146124

147-
if err := s.store.DeviceDelete(ctx, uid); err != nil {
148-
return err
125+
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, models.DeviceStatusRemoved, 1); err != nil {
126+
return err
127+
}
128+
} else {
129+
if err := s.store.DeviceDelete(ctx, uid); err != nil {
130+
return err
131+
}
149132
}
150133

151-
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, originalStatus, -1); err != nil { //nolint:revive
134+
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, device.Status, -1); err != nil { //nolint:revive
152135
return err
153136
}
154137

@@ -331,39 +314,15 @@ func (s *service) UpdateDeviceStatus(ctx context.Context, tenant string, uid mod
331314
return nil
332315
}
333316

334-
// This is a workaround for a current limitation: removed devices are stored in a separate collection,
335-
// which means the namespace doesn't maintain a counter for them. Decrementing the old status count
336-
// would result in -2 (since we already subtract 1 in DeleteDevice), so we only decrement the
337-
// oldStatus count if the device has not been removed.
338-
isRemoved := false
339-
340317
switch {
341318
case envs.IsCloud() && envs.HasBilling():
342319
if namespace.Billing.IsActive() {
343320
if err := s.BillingReport(s.client, namespace.TenantID, ReportDeviceAccept); err != nil {
344321
return NewErrBillingReportNamespaceDelete(err)
345322
}
346323
} else {
347-
// TODO: this strategy that stores the removed devices in the database can be simplified.
348-
removed, err := s.store.DeviceRemovedGet(ctx, tenant, uid)
349-
if err != nil && err != store.ErrNoDocuments {
350-
return NewErrDeviceRemovedGet(err)
351-
}
352-
353-
if removed != nil {
354-
isRemoved = true
355-
if err := s.store.DeviceRemovedDelete(ctx, tenant, uid); err != nil {
356-
return NewErrDeviceRemovedDelete(err)
357-
}
358-
} else {
359-
count, err := s.store.DeviceRemovedCount(ctx, tenant)
360-
if err != nil {
361-
return NewErrDeviceRemovedCount(err)
362-
}
363-
364-
if namespace.HasMaxDevices() && namespace.HasLimitDevicesReached(count) {
365-
return NewErrDeviceRemovedFull(namespace.MaxDevices, nil)
366-
}
324+
if device.Status != models.DeviceStatusRemoved && namespace.HasMaxDevices() && namespace.HasLimitDevicesReached() {
325+
return NewErrDeviceRemovedFull(namespace.MaxDevices, nil)
367326
}
368327

369328
ok, err := s.BillingEvaluate(s.client, namespace.TenantID)
@@ -385,13 +344,7 @@ func (s *service) UpdateDeviceStatus(ctx context.Context, tenant string, uid mod
385344
return err
386345
}
387346

388-
if !isRemoved {
389-
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, originalStatus, -1); err != nil {
390-
return err
391-
}
392-
}
393-
394-
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, status, 1); err != nil { // nolint:revive
347+
if err := s.adjustDeviceCounters(ctx, tenant, originalStatus, status); err != nil { // nolint:revive
395348
return err
396349
}
397350

0 commit comments

Comments
 (0)