Skip to content

Commit c197978

Browse files
the-gluthe-glu
committed
[memstore/rid] Don't allow nil value for start/end time
1 parent c3f668d commit c197978

4 files changed

Lines changed: 38 additions & 47 deletions

File tree

‎pkg/rid/store/memstore/identification_service_area.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ func isaRecordFromModel(isa *ridmodels.IdentificationServiceArea, updatedAt time
1818
URL: isa.URL,
1919
Owner: isa.Owner,
2020
Cells: cloneCells(isa.Cells),
21-
StartTime: cloneTime(isa.StartTime),
22-
EndTime: cloneTime(isa.EndTime),
21+
StartTime: *isa.StartTime,
22+
EndTime: *isa.EndTime,
2323
AltitudeHi: cloneFloat32(isa.AltitudeHi),
2424
AltitudeLo: cloneFloat32(isa.AltitudeLo),
2525
Writer: isa.Writer,
@@ -34,8 +34,8 @@ func (rec *isaRecord) toModel() *ridmodels.IdentificationServiceArea {
3434
URL: rec.URL,
3535
Owner: rec.Owner,
3636
Cells: cloneCells(rec.Cells),
37-
StartTime: cloneTime(rec.StartTime),
38-
EndTime: cloneTime(rec.EndTime),
37+
StartTime: timePtr(rec.StartTime),
38+
EndTime: timePtr(rec.EndTime),
3939
Version: dssmodels.VersionFromTime(rec.UpdatedAt),
4040
AltitudeHi: cloneFloat32(rec.AltitudeHi),
4141
AltitudeLo: cloneFloat32(rec.AltitudeLo),
@@ -105,11 +105,11 @@ func (r *repo) SearchISAs(_ context.Context, cells s2.CellUnion, earliest *time.
105105
var out []*ridmodels.IdentificationServiceArea
106106
for _, rec := range r.state.ISAs {
107107
// ends_at >= earliest
108-
if rec.EndTime == nil || rec.EndTime.Before(*earliest) {
108+
if rec.EndTime.Before(*earliest) {
109109
continue
110110
}
111111
// COALESCE(starts_at <= latest, true)
112-
if latest != nil && rec.StartTime != nil && rec.StartTime.After(*latest) {
112+
if latest != nil && rec.StartTime.After(*latest) {
113113
continue
114114
}
115115
if !overlaps(rec.Cells, want) {
@@ -128,7 +128,7 @@ func (r *repo) ListExpiredISAs(_ context.Context, writer string, threshold time.
128128
var out []*ridmodels.IdentificationServiceArea
129129
for _, rec := range r.state.ISAs {
130130
// ends_at <= threshold
131-
if rec.EndTime == nil || rec.EndTime.After(threshold) {
131+
if rec.EndTime.After(threshold) {
132132
continue
133133
}
134134
if writer == "" {

‎pkg/rid/store/memstore/store.go‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ type isaRecord struct {
3434
URL string
3535
Owner dssmodels.Owner
3636
Cells s2.CellUnion
37-
StartTime *time.Time
38-
EndTime *time.Time
37+
StartTime time.Time
38+
EndTime time.Time
3939
AltitudeHi *float32
4040
AltitudeLo *float32
4141
Writer string
@@ -49,8 +49,8 @@ type subscriptionRecord struct {
4949
NotificationIndex int
5050
Owner dssmodels.Owner
5151
Cells s2.CellUnion
52-
StartTime *time.Time
53-
EndTime *time.Time
52+
StartTime time.Time
53+
EndTime time.Time
5454
AltitudeHi *float32
5555
AltitudeLo *float32
5656
Writer string
@@ -86,7 +86,13 @@ func validateWriteData(cells s2.CellUnion, start, end *time.Time) error {
8686
return stacktrace.Propagate(err, "Error validating cell")
8787
}
8888
}
89-
if start != nil && end != nil && !start.Before(*end) {
89+
if start == nil {
90+
return stacktrace.NewError("Start time must be provided")
91+
}
92+
if end == nil {
93+
return stacktrace.NewError("End time must be provided")
94+
}
95+
if !start.Before(*end) {
9096
return stacktrace.NewError("Start time must be strictly before end time")
9197
}
9298
return nil
@@ -119,11 +125,8 @@ func cloneCells(cells s2.CellUnion) s2.CellUnion {
119125
return append(s2.CellUnion(nil), cells...)
120126
}
121127

122-
func cloneTime(t *time.Time) *time.Time {
123-
if t == nil {
124-
return nil
125-
}
126-
v := *t
128+
func timePtr(t time.Time) *time.Time {
129+
v := t
127130
return &v
128131
}
129132

‎pkg/rid/store/memstore/subscriptions.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ func subRecordFromModel(s *ridmodels.Subscription, updatedAt time.Time) *subscri
1919
NotificationIndex: s.NotificationIndex,
2020
Owner: s.Owner,
2121
Cells: cloneCells(s.Cells),
22-
StartTime: cloneTime(s.StartTime),
23-
EndTime: cloneTime(s.EndTime),
22+
StartTime: *s.StartTime,
23+
EndTime: *s.EndTime,
2424
AltitudeHi: cloneFloat32(s.AltitudeHi),
2525
AltitudeLo: cloneFloat32(s.AltitudeLo),
2626
Writer: s.Writer,
@@ -35,8 +35,8 @@ func (rec *subscriptionRecord) toModel() *ridmodels.Subscription {
3535
NotificationIndex: rec.NotificationIndex,
3636
Owner: rec.Owner,
3737
Cells: cloneCells(rec.Cells),
38-
StartTime: cloneTime(rec.StartTime),
39-
EndTime: cloneTime(rec.EndTime),
38+
StartTime: timePtr(rec.StartTime),
39+
EndTime: timePtr(rec.EndTime),
4040
Version: dssmodels.VersionFromTime(rec.UpdatedAt),
4141
AltitudeHi: cloneFloat32(rec.AltitudeHi),
4242
AltitudeLo: cloneFloat32(rec.AltitudeLo),
@@ -102,7 +102,7 @@ func (r *repo) SearchSubscriptions(ctx context.Context, cells s2.CellUnion) ([]*
102102
want := cellSet(cells)
103103
var out []*ridmodels.Subscription
104104
for _, rec := range r.state.Subscriptions {
105-
if rec.EndTime == nil || rec.EndTime.Before(now) {
105+
if rec.EndTime.Before(now) {
106106
continue
107107
}
108108
if !overlaps(rec.Cells, want) {
@@ -128,7 +128,7 @@ func (r *repo) SearchSubscriptionsByOwner(ctx context.Context, cells s2.CellUnio
128128
if rec.Owner != owner {
129129
continue
130130
}
131-
if rec.EndTime == nil || rec.EndTime.Before(now) {
131+
if rec.EndTime.Before(now) {
132132
continue
133133
}
134134
if !overlaps(rec.Cells, want) {
@@ -150,7 +150,7 @@ func (r *repo) UpdateNotificationIdxsInCells(ctx context.Context, cells s2.CellU
150150
want := cellSet(cells)
151151
var out []*ridmodels.Subscription
152152
for _, rec := range r.state.Subscriptions {
153-
if rec.EndTime == nil || rec.EndTime.Before(now) {
153+
if rec.EndTime.Before(now) {
154154
continue
155155
}
156156
if !overlaps(rec.Cells, want) {
@@ -170,7 +170,7 @@ func (r *repo) MaxSubscriptionCountInCellsByOwner(ctx context.Context, cells s2.
170170
if rec.Owner != owner {
171171
continue
172172
}
173-
if rec.EndTime == nil || rec.EndTime.Before(now) {
173+
if rec.EndTime.Before(now) {
174174
continue
175175
}
176176
for _, c := range rec.Cells {
@@ -192,7 +192,7 @@ func (r *repo) ListExpiredSubscriptions(_ context.Context, writer string, thresh
192192
var out []*ridmodels.Subscription
193193
for _, rec := range r.state.Subscriptions {
194194
// ends_at <= threshold
195-
if rec.EndTime == nil || rec.EndTime.After(threshold) {
195+
if rec.EndTime.After(threshold) {
196196
continue
197197
}
198198
if writer == "" {

‎pkg/rid/store/memstore/subscriptions_test.go‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,7 @@ var (
3939
},
4040
},
4141
{
42-
name: "a subscription without startTime and with endTime",
43-
input: &ridmodels.Subscription{
44-
ID: dssmodels.ID(uuid.New().String()),
45-
Owner: "myself",
46-
URL: "https://no/place/like/home",
47-
EndTime: &endTime,
48-
NotificationIndex: 42,
49-
Cells: s2.CellUnion{
50-
12494535935418957824,
51-
},
52-
},
53-
},
54-
{
55-
name: "a subscription without startTime and with endTime",
42+
name: "a subscription with a different cell",
5643
input: &ridmodels.Subscription{
5744
ID: dssmodels.ID(uuid.New().String()),
5845
Owner: "me",
@@ -174,7 +161,6 @@ func TestStoreSearchSubscription(t *testing.T) {
174161
}
175162
owners = []dssmodels.Owner{
176163
"me",
177-
"my",
178164
"self",
179165
}
180166
)
@@ -190,7 +176,7 @@ func TestStoreSearchSubscription(t *testing.T) {
190176
// Test normal search
191177
found, err := repo.SearchSubscriptions(ctx, cells)
192178
require.NoError(t, err)
193-
require.Len(t, found, 3)
179+
require.Len(t, found, 2)
194180
for _, owner := range owners {
195181
found, err := repo.SearchSubscriptionsByOwner(ctx, cells, owner)
196182
require.NoError(t, err)
@@ -205,12 +191,14 @@ func TestStoreExpiredSubscription(t *testing.T) {
205191
ctx := context.Background()
206192
repo := setUpStore(t)
207193

194+
startTime := fakeClock.Now()
208195
endTime := fakeClock.Now().Add(24 * time.Hour)
209196
sub := &ridmodels.Subscription{
210-
ID: dssmodels.ID(uuid.New().String()),
211-
Owner: dssmodels.Owner("original owner"),
212-
Cells: s2.CellUnion{s2.CellID(12494535866699481088)},
213-
EndTime: &endTime,
197+
ID: dssmodels.ID(uuid.New().String()),
198+
Owner: dssmodels.Owner("original owner"),
199+
Cells: s2.CellUnion{s2.CellID(12494535866699481088)},
200+
StartTime: &startTime,
201+
EndTime: &endTime,
214202
}
215203
_, err := repo.InsertSubscription(ctx, sub)
216204
require.NoError(t, err)
@@ -266,7 +254,7 @@ func TestMaxSubscriptionCountInCellsByOwner(t *testing.T) {
266254

267255
count, err := repo.MaxSubscriptionCountInCellsByOwner(ctx, s2.CellUnion{12494535935418957824}, "myself")
268256
require.NoError(t, err)
269-
require.Equal(t, 2, count)
257+
require.Equal(t, 1, count)
270258
}
271259

272260
func TestListExpiredSubscriptions(t *testing.T) {

0 commit comments

Comments
 (0)