Skip to content

Commit bf3b988

Browse files
heiytorgustavosbarreto
authored andcommitted
fix: prevent double decrement of device counters when updating removed devices
Previously, when updating the status of a removed device, the counter for the original status would be decremented twice: once in DeleteDevice and again in UpdateDeviceStatus. This resulted in incorrect namespace device counters. - Store original device status before DeviceRemovedInsert modifies it - Track if device was removed to avoid double counter decrement - Only decrement original status counter if device wasn't previously removed - Update test expectations to reflect corrected counter logic
1 parent cf15fd9 commit bf3b988

File tree

2 files changed

+20
-6
lines changed

2 files changed

+20
-6
lines changed

api/services/device.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ func (s *service) DeleteDevice(ctx context.Context, uid models.UID, tenant strin
130130
return NewErrNamespaceNotFound(tenant, err)
131131
}
132132

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+
133138
// If the namespace has a limit of devices, we change the device's slot status to removed.
134139
// This way, we can keep track of the number of devices that were removed from the namespace and void the device
135140
// switching.
@@ -143,7 +148,7 @@ func (s *service) DeleteDevice(ctx context.Context, uid models.UID, tenant strin
143148
return err
144149
}
145150

146-
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, device.Status, -1); err != nil { //nolint:revive
151+
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, originalStatus, -1); err != nil { //nolint:revive
147152
return err
148153
}
149154

@@ -321,6 +326,12 @@ func (s *service) UpdateDeviceStatus(ctx context.Context, tenant string, uid mod
321326
return nil
322327
}
323328

329+
// This is a workaround for a current limitation: removed devices are stored in a separate collection,
330+
// which means the namespace doesn't maintain a counter for them. Decrementing the old status count
331+
// would result in -2 (since we already subtract 1 in DeleteDevice), so we only decrement the
332+
// oldStatus count if the device has not been removed.
333+
isRemoved := false
334+
324335
switch {
325336
case envs.IsCloud():
326337
if namespace.Billing.IsActive() {
@@ -335,6 +346,7 @@ func (s *service) UpdateDeviceStatus(ctx context.Context, tenant string, uid mod
335346
}
336347

337348
if removed != nil {
349+
isRemoved = true
338350
if err := s.store.DeviceRemovedDelete(ctx, tenant, uid); err != nil {
339351
return NewErrDeviceRemovedDelete(err)
340352
}
@@ -368,7 +380,13 @@ func (s *service) UpdateDeviceStatus(ctx context.Context, tenant string, uid mod
368380
return err
369381
}
370382

371-
if err := s.adjustDeviceCounters(ctx, tenant, originalStatus, status); err != nil { // nolint:revive
383+
if !isRemoved {
384+
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, originalStatus, -1); err != nil {
385+
return err
386+
}
387+
}
388+
389+
if err := s.store.NamespaceIncrementDeviceCount(ctx, tenant, status, 1); err != nil { // nolint:revive
372390
return err
373391
}
374392

api/services/device_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4111,10 +4111,6 @@ func TestUpdateDeviceStatus_cloud_subscription_inactive(t *testing.T) {
41114111
On("DeviceUpdateStatus", ctx, models.UID("uid"), models.DeviceStatus("accepted")).
41124112
Return(nil).
41134113
Once()
4114-
storeMock.
4115-
On("NamespaceIncrementDeviceCount", ctx, "00000000-0000-0000-0000-000000000000", models.DeviceStatusPending, int64(-1)).
4116-
Return(nil).
4117-
Once()
41184114
storeMock.
41194115
On("NamespaceIncrementDeviceCount", ctx, "00000000-0000-0000-0000-000000000000", models.DeviceStatusAccepted, int64(1)).
41204116
Return(nil).

0 commit comments

Comments
 (0)