Skip to content

Commit e6acabf

Browse files
TimVoschTimVosch
committed
fix(core): sensor creation race
1 parent 959e396 commit e6acabf

6 files changed

Lines changed: 198 additions & 30 deletions

File tree

‎services/core/devices/application.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ type DeviceStore interface {
2323
ListSensors(context.Context, pagination.Request) (*pagination.Page[Sensor], error)
2424
Find(ctx context.Context, id int64) (*Device, error)
2525
Save(ctx context.Context, dev *Device) error
26+
AddSensor(ctx context.Context, dev *Device, sensor *Sensor) error
2627
UpdateSensor(ctx context.Context, id int64, opts UpdateSensorOpts) error
2728
Delete(ctx context.Context, dev *Device) error
2829
GetSensor(ctx context.Context, id int64) (*Sensor, error)
@@ -167,10 +168,11 @@ func (s *Service) AddSensor(ctx context.Context, dev *Device, dto NewSensorDTO)
167168
opts.FeatureOfInterest = feature
168169
}
169170

170-
if err := dev.AddSensor(opts); err != nil {
171+
sensor, err := dev.AddSensor(opts)
172+
if err != nil {
171173
return err
172174
}
173-
if err := s.store.Save(ctx, dev); err != nil {
175+
if err := s.store.AddSensor(ctx, dev, sensor); err != nil {
174176
return err
175177
}
176178
return nil

‎services/core/devices/devices.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,31 +175,31 @@ func NewSensor(opts NewSensorOpts) (*Sensor, error) {
175175
return &sensor, nil
176176
}
177177

178-
func (d *Device) AddSensor(opts NewSensorOpts) error {
178+
func (d *Device) AddSensor(opts NewSensorOpts) (*Sensor,error) {
179179
// Check if sensor external ID already exists
180180
for _, existing := range d.Sensors {
181181
if existing.ExternalID == opts.ExternalID {
182-
return ErrDuplicateSensorExternalID
182+
return nil, ErrDuplicateSensorExternalID
183183
}
184184
if existing.Code == opts.Code {
185-
return ErrDuplicateSensorCode
185+
return nil, ErrDuplicateSensorCode
186186
}
187187
if opts.IsFallback && existing.IsFallback {
188-
return ErrDuplicateFallbackSensor
188+
return nil, ErrDuplicateFallbackSensor
189189
}
190190
}
191191

192192
sensor, err := NewSensor(opts)
193193
if err != nil {
194-
return err
194+
return nil, err
195195
}
196196
sensor.TenantID = d.TenantID
197197
sensor.DeviceID = d.ID
198198

199199
// Append sensor
200200
d.Sensors = append(d.Sensors, *sensor)
201201

202-
return nil
202+
return sensor, nil
203203
}
204204

205205
// Get the sensor with a specific code from the device

‎services/core/devices/devices_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestAddSensorToDevice(t *testing.T) {
2020
State: devices.DeviceEnabled,
2121
})
2222
require.NoError(t, err)
23-
err = device.AddSensor(devices.NewSensorOpts{
23+
_, err = device.AddSensor(devices.NewSensorOpts{
2424
Code: "existing",
2525
Description: "",
2626
ExternalID: "existing",
@@ -88,7 +88,7 @@ func TestAddSensorToDevice(t *testing.T) {
8888
for _, tC := range testCases {
8989
t.Run(tC.desc, func(t *testing.T) {
9090
device := setupDevice()
91-
err := device.AddSensor(tC.sensor)
91+
_, err := device.AddSensor(tC.sensor)
9292
if tC.expectedErr != nil {
9393
assert.Error(t, err)
9494
assert.ErrorIs(t, err, tC.expectedErr)
@@ -115,7 +115,7 @@ func TestDeviceShouldProvideSensorByEIDOrFallback(t *testing.T) {
115115
Code: "test_fallback_sensor",
116116
IsFallback: true,
117117
}
118-
err = dev.AddSensor(opt)
118+
_, err = dev.AddSensor(opt)
119119
assert.NoError(t, err)
120120

121121
// Fetch fallback
@@ -129,7 +129,7 @@ func TestDeviceShouldProvideSensorByEIDOrFallback(t *testing.T) {
129129
Code: "test_sensor",
130130
ExternalID: "sensor_eid",
131131
}
132-
err = dev.AddSensor(opt)
132+
_, err = dev.AddSensor(opt)
133133
assert.NoError(t, err)
134134

135135
// Fetch fallback

‎services/core/devices/infra/store_psql.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,31 @@ func (s *PSQLStore) Save(ctx context.Context, dev *devices.Device) error {
604604
return nil
605605
}
606606

607+
func (store *PSQLStore) AddSensor(
608+
ctx context.Context,
609+
dev *devices.Device,
610+
s *devices.Sensor,
611+
) error {
612+
var featureID sql.NullInt64
613+
if s.FeatureOfInterest != nil {
614+
featureID.Valid = true
615+
featureID.Int64 = s.FeatureOfInterest.ID
616+
}
617+
618+
err := pq.Insert("sensors").Columns(
619+
"code", "brand", "description", "archive_time", "properties", "external_id",
620+
"device_id", "created_at", "is_fallback", "tenant_id", "feature_of_interest_id",
621+
).Values(
622+
s.Code, s.Brand, s.Description, s.ArchiveTime, s.Properties, s.ExternalID,
623+
s.DeviceID, s.CreatedAt, s.IsFallback, s.TenantID, featureID,
624+
).Suffix("RETURNING id").RunWith(store.db).ScanContext(ctx, &s.ID)
625+
if err != nil {
626+
return fmt.Errorf("AddSensor Query: %w", err)
627+
}
628+
629+
return nil
630+
}
631+
607632
func (store *PSQLStore) UpdateSensor(
608633
ctx context.Context,
609634
id int64,

‎services/core/devices/infra/store_psql_test.go‎

Lines changed: 103 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,54 @@ func TestShouldCreateAndFetchDevice(t *testing.T) {
9696
readDev, err := store.Find(ctx, dev.ID)
9797
assert.NoError(t, err)
9898
assert.Equal(t, dev.ID, readDev.ID, "store.Save(insert) and store.Find result in changes")
99-
assert.Equal(t, dev.Latitude, readDev.Latitude, "store.Save(insert) and store.Find result in changes")
100-
assert.Equal(t, dev.Longitude, readDev.Longitude, "store.Save(insert) and store.Find result in changes")
101-
assert.Equal(t, dev.Altitude, readDev.Altitude, "store.Save(insert) and store.Find result in changes")
102-
assert.Equal(t, dev.LocationDescription, readDev.LocationDescription, "store.Save(insert) and store.Find result in changes")
103-
assert.Equal(t, dev.State, readDev.State, "store.Save(insert) and store.Find result in changes")
104-
assert.Equal(t, dev.Description, readDev.Description, "store.Save(insert) and store.Find result in changes")
105-
assert.Equal(t, dev.TenantID, readDev.TenantID, "store.Save(insert) and store.Find result in changes")
106-
assert.Equal(t, dev.Properties, readDev.Properties, "store.Save(insert) and store.Find result in changes")
99+
assert.Equal(
100+
t,
101+
dev.Latitude,
102+
readDev.Latitude,
103+
"store.Save(insert) and store.Find result in changes",
104+
)
105+
assert.Equal(
106+
t,
107+
dev.Longitude,
108+
readDev.Longitude,
109+
"store.Save(insert) and store.Find result in changes",
110+
)
111+
assert.Equal(
112+
t,
113+
dev.Altitude,
114+
readDev.Altitude,
115+
"store.Save(insert) and store.Find result in changes",
116+
)
117+
assert.Equal(
118+
t,
119+
dev.LocationDescription,
120+
readDev.LocationDescription,
121+
"store.Save(insert) and store.Find result in changes",
122+
)
123+
assert.Equal(
124+
t,
125+
dev.State,
126+
readDev.State,
127+
"store.Save(insert) and store.Find result in changes",
128+
)
129+
assert.Equal(
130+
t,
131+
dev.Description,
132+
readDev.Description,
133+
"store.Save(insert) and store.Find result in changes",
134+
)
135+
assert.Equal(
136+
t,
137+
dev.TenantID,
138+
readDev.TenantID,
139+
"store.Save(insert) and store.Find result in changes",
140+
)
141+
assert.Equal(
142+
t,
143+
dev.Properties,
144+
readDev.Properties,
145+
"store.Save(insert) and store.Find result in changes",
146+
)
107147
})
108148

109149
t.Run("listing created device", func(t *testing.T) {
@@ -115,7 +155,12 @@ func TestShouldCreateAndFetchDevice(t *testing.T) {
115155
assert.Equal(t, dev.Latitude, devs[0].Latitude, "store.List results in changes")
116156
assert.Equal(t, dev.Longitude, devs[0].Longitude, "store.List results in changes")
117157
assert.Equal(t, dev.Altitude, devs[0].Altitude, "store.List results in changes")
118-
assert.Equal(t, dev.LocationDescription, devs[0].LocationDescription, "store.List results in changes")
158+
assert.Equal(
159+
t,
160+
dev.LocationDescription,
161+
devs[0].LocationDescription,
162+
"store.List results in changes",
163+
)
119164
assert.Equal(t, dev.State, devs[0].State, "store.List results in changes")
120165
assert.Equal(t, dev.Description, devs[0].Description, "store.List results in changes")
121166
assert.Equal(t, dev.TenantID, devs[0].TenantID, "store.List results in changes")
@@ -136,14 +181,54 @@ func TestShouldCreateAndFetchDevice(t *testing.T) {
136181
readDev, err := store.Find(ctx, dev.ID)
137182
assert.NoError(t, err)
138183
assert.Equal(t, dev.ID, readDev.ID, "store.Save(update) and store.Find result in changes")
139-
assert.Equal(t, dev.Latitude, readDev.Latitude, "store.Save(update) and store.Find result in changes")
140-
assert.Equal(t, dev.Longitude, readDev.Longitude, "store.Save(update) and store.Find result in changes")
141-
assert.Equal(t, dev.Altitude, readDev.Altitude, "store.Save(update) and store.Find result in changes")
142-
assert.Equal(t, dev.LocationDescription, readDev.LocationDescription, "store.Save(update) and store.Find result in changes")
143-
assert.Equal(t, dev.State, readDev.State, "store.Save(update) and store.Find result in changes")
144-
assert.Equal(t, dev.Description, readDev.Description, "store.Save(update) and store.Find result in changes")
145-
assert.Equal(t, dev.TenantID, readDev.TenantID, "store.Save(update) and store.Find result in changes")
146-
assert.Equal(t, dev.Properties, readDev.Properties, "store.Save(update) and store.Find result in changes")
184+
assert.Equal(
185+
t,
186+
dev.Latitude,
187+
readDev.Latitude,
188+
"store.Save(update) and store.Find result in changes",
189+
)
190+
assert.Equal(
191+
t,
192+
dev.Longitude,
193+
readDev.Longitude,
194+
"store.Save(update) and store.Find result in changes",
195+
)
196+
assert.Equal(
197+
t,
198+
dev.Altitude,
199+
readDev.Altitude,
200+
"store.Save(update) and store.Find result in changes",
201+
)
202+
assert.Equal(
203+
t,
204+
dev.LocationDescription,
205+
readDev.LocationDescription,
206+
"store.Save(update) and store.Find result in changes",
207+
)
208+
assert.Equal(
209+
t,
210+
dev.State,
211+
readDev.State,
212+
"store.Save(update) and store.Find result in changes",
213+
)
214+
assert.Equal(
215+
t,
216+
dev.Description,
217+
readDev.Description,
218+
"store.Save(update) and store.Find result in changes",
219+
)
220+
assert.Equal(
221+
t,
222+
dev.TenantID,
223+
readDev.TenantID,
224+
"store.Save(update) and store.Find result in changes",
225+
)
226+
assert.Equal(
227+
t,
228+
dev.Properties,
229+
readDev.Properties,
230+
"store.Save(update) and store.Find result in changes",
231+
)
147232
})
148233
}
149234

@@ -178,7 +263,7 @@ func TestShouldAddSensor(t *testing.T) {
178263

179264
t.Run("should add sensor", func(t *testing.T) {
180265
// Add sensor
181-
err = dev.AddSensor(s1)
266+
_, err := dev.AddSensor(s1)
182267
require.NoError(t, err)
183268
require.Len(t, dev.Sensors, 1)
184269
err = store.Save(ctx, dev)

‎services/core/devices/mock_test.go‎

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

0 commit comments

Comments
 (0)