Skip to content

Commit 17063bf

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

4 files changed

Lines changed: 52 additions & 45 deletions

File tree

‎pkg/scd/store/memstore/constraints.go‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ func (rec *constraintRecord) toModel() *scdmodels.Constraint {
1717
Manager: rec.Manager,
1818
Version: rec.Version,
1919
OVN: scdmodels.NewOVNFromTime(rec.UpdatedAt, rec.ID.String()),
20-
StartTime: cloneTime(rec.StartTime),
21-
EndTime: cloneTime(rec.EndTime),
20+
StartTime: timePtr(rec.StartTime),
21+
EndTime: timePtr(rec.EndTime),
2222
USSBaseURL: rec.USSBaseURL,
2323
AltitudeLower: cloneFloat32(rec.AltitudeLower),
2424
AltitudeUpper: cloneFloat32(rec.AltitudeUpper),
@@ -43,11 +43,11 @@ func (r *repo) SearchConstraints(_ context.Context, v4d *dssmodels.Volume4D) ([]
4343
continue
4444
}
4545
// COALESCE(starts_at <= $3, true) with $3 = v4d.EndTime
46-
if rec.StartTime != nil && v4d.EndTime != nil && rec.StartTime.After(*v4d.EndTime) {
46+
if v4d.EndTime != nil && rec.StartTime.After(*v4d.EndTime) {
4747
continue
4848
}
4949
// COALESCE(ends_at >= $2, true) with $2 = v4d.StartTime
50-
if rec.EndTime != nil && v4d.StartTime != nil && rec.EndTime.Before(*v4d.StartTime) {
50+
if v4d.StartTime != nil && rec.EndTime.Before(*v4d.StartTime) {
5151
continue
5252
}
5353
out = append(out, rec.toModel())
@@ -72,13 +72,16 @@ func (r *repo) UpsertConstraint(ctx context.Context, s *scdmodels.Constraint) (*
7272
if _, err := dsssql.CellUnionToCellIdsWithValidation(s.Cells); err != nil {
7373
return nil, stacktrace.Propagate(err, "Failed to convert array to jackc/pgtype")
7474
}
75+
if err := requireExtentTimes(s.StartTime, s.EndTime); err != nil {
76+
return nil, err
77+
}
7578

7679
rec := &constraintRecord{
7780
ID: s.ID,
7881
Manager: s.Manager,
7982
Version: s.Version,
80-
StartTime: cloneTime(s.StartTime),
81-
EndTime: cloneTime(s.EndTime),
83+
StartTime: *s.StartTime,
84+
EndTime: *s.EndTime,
8285
USSBaseURL: s.USSBaseURL,
8386
AltitudeLower: cloneFloat32(s.AltitudeLower),
8487
AltitudeUpper: cloneFloat32(s.AltitudeUpper),

‎pkg/scd/store/memstore/operational_intents.go‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ func (rec *operationalIntentRecord) toModel() *scdmodels.OperationalIntent {
3131
State: rec.State,
3232
OVN: ovn,
3333
PastOVNs: clonePastOVNs(rec.PastOVNs),
34-
StartTime: cloneTime(rec.StartTime),
35-
EndTime: cloneTime(rec.EndTime),
34+
StartTime: timePtr(rec.StartTime),
35+
EndTime: timePtr(rec.EndTime),
3636
USSBaseURL: rec.USSBaseURL,
3737
SubscriptionID: cloneID(rec.SubscriptionID),
3838
AltitudeLower: cloneFloat32(rec.AltitudeLower),
@@ -99,13 +99,17 @@ func (r *repo) UpsertOperationalIntent(ctx context.Context, operation *scdmodels
9999
ussRequestedOVN = operation.OVN.String()
100100
}
101101

102+
if err := requireExtentTimes(operation.StartTime, operation.EndTime); err != nil {
103+
return nil, err
104+
}
105+
102106
rec := &operationalIntentRecord{
103107
ID: operation.ID,
104108
Manager: operation.Manager,
105109
Version: operation.Version,
106110
State: operation.State,
107-
StartTime: cloneTime(operation.StartTime),
108-
EndTime: cloneTime(operation.EndTime),
111+
StartTime: *operation.StartTime,
112+
EndTime: *operation.EndTime,
109113
USSBaseURL: operation.USSBaseURL,
110114
SubscriptionID: cloneID(operation.SubscriptionID),
111115
AltitudeLower: cloneFloat32(operation.AltitudeLower),
@@ -152,11 +156,11 @@ func (r *repo) SearchOperationalIntents(ctx context.Context, v4d *dssmodels.Volu
152156
continue
153157
}
154158
// COALESCE(ends_at >= $4, true) with $4 = v4d.StartTime
155-
if rec.EndTime != nil && v4d.StartTime != nil && rec.EndTime.Before(*v4d.StartTime) {
159+
if v4d.StartTime != nil && rec.EndTime.Before(*v4d.StartTime) {
156160
continue
157161
}
158162
// COALESCE(starts_at <= $5, true) with $5 = v4d.EndTime
159-
if rec.StartTime != nil && v4d.EndTime != nil && rec.StartTime.After(*v4d.EndTime) {
163+
if v4d.EndTime != nil && rec.StartTime.After(*v4d.EndTime) {
160164
continue
161165
}
162166
matched = append(matched, rec)
@@ -182,14 +186,8 @@ func (r *repo) GetDependentOperationalIntents(_ context.Context, subscriptionID
182186
func (r *repo) ListExpiredOperationalIntents(ctx context.Context, threshold time.Time) ([]*scdmodels.OperationalIntent, error) {
183187
var matched []*operationalIntentRecord
184188
for _, rec := range r.state.OperationalIntents {
185-
// (ends_at IS NOT NULL AND ends_at <= threshold) OR (ends_at IS NULL AND updated_at <= threshold)
186-
var expired bool
187-
if rec.EndTime != nil {
188-
expired = !rec.EndTime.After(threshold)
189-
} else {
190-
expired = !rec.UpdatedAt.After(threshold)
191-
}
192-
if !expired {
189+
// ends_at <= threshold
190+
if rec.EndTime.After(threshold) {
193191
continue
194192
}
195193
matched = append(matched, rec)

‎pkg/scd/store/memstore/store.go‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ type constraintRecord struct {
3636
ID dssmodels.ID
3737
Manager dssmodels.Manager
3838
Version scdmodels.VersionNumber
39-
StartTime *time.Time
40-
EndTime *time.Time
39+
StartTime time.Time
40+
EndTime time.Time
4141
USSBaseURL string
4242
AltitudeLower *float32
4343
AltitudeUpper *float32
@@ -56,8 +56,8 @@ type subscriptionRecord struct {
5656
NotifyForOperationalIntents bool
5757
NotifyForConstraints bool
5858
ImplicitSubscription bool
59-
StartTime *time.Time
60-
EndTime *time.Time
59+
StartTime time.Time
60+
EndTime time.Time
6161
Cells s2.CellUnion
6262
UpdatedAt time.Time
6363
}
@@ -69,8 +69,8 @@ type operationalIntentRecord struct {
6969
Manager dssmodels.Manager
7070
Version scdmodels.VersionNumber
7171
State scdmodels.OperationalIntentState
72-
StartTime *time.Time
73-
EndTime *time.Time
72+
StartTime time.Time
73+
EndTime time.Time
7474
USSBaseURL string
7575
SubscriptionID *dssmodels.ID
7676
AltitudeLower *float32
@@ -137,14 +137,23 @@ func cloneCells(cells s2.CellUnion) s2.CellUnion {
137137
return append(s2.CellUnion(nil), cells...)
138138
}
139139

140-
func cloneTime(t *time.Time) *time.Time {
141-
if t == nil {
142-
return nil
143-
}
144-
v := *t
140+
func timePtr(t time.Time) *time.Time {
141+
v := t
145142
return &v
146143
}
147144

145+
// requireExtentTimes enforces the memstore invariant that records always carry
146+
// both a start and an end time.
147+
func requireExtentTimes(start, end *time.Time) error {
148+
if start == nil {
149+
return stacktrace.NewError("Start time must be provided")
150+
}
151+
if end == nil {
152+
return stacktrace.NewError("End time must be provided")
153+
}
154+
return nil
155+
}
156+
148157
func cloneFloat32(f *float32) *float32 {
149158
if f == nil {
150159
return nil

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

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ func (rec *subscriptionRecord) toModel() *scdmodels.Subscription {
1717
Version: scdmodels.NewOVNFromTime(rec.UpdatedAt, rec.ID.String()),
1818
NotificationIndex: rec.NotificationIndex,
1919
Manager: rec.Manager,
20-
StartTime: cloneTime(rec.StartTime),
21-
EndTime: cloneTime(rec.EndTime),
20+
StartTime: timePtr(rec.StartTime),
21+
EndTime: timePtr(rec.EndTime),
2222
USSBaseURL: rec.USSBaseURL,
2323
NotifyForOperationalIntents: rec.NotifyForOperationalIntents,
2424
NotifyForConstraints: rec.NotifyForConstraints,
@@ -44,11 +44,11 @@ func (r *repo) SearchSubscriptions(_ context.Context, v4d *dssmodels.Volume4D) (
4444
continue
4545
}
4646
// COALESCE(starts_at <= $3, true) with $3 = v4d.EndTime
47-
if rec.StartTime != nil && v4d.EndTime != nil && rec.StartTime.After(*v4d.EndTime) {
47+
if v4d.EndTime != nil && rec.StartTime.After(*v4d.EndTime) {
4848
continue
4949
}
5050
// COALESCE(ends_at >= $2, true) with $2 = v4d.StartTime
51-
if rec.EndTime != nil && v4d.StartTime != nil && rec.EndTime.Before(*v4d.StartTime) {
51+
if v4d.StartTime != nil && rec.EndTime.Before(*v4d.StartTime) {
5252
continue
5353
}
5454
out = append(out, rec.toModel())
@@ -70,6 +70,9 @@ func (r *repo) GetSubscription(_ context.Context, id dssmodels.ID) (*scdmodels.S
7070

7171
// UpsertSubscription implements scd.repos.Subscription.UpsertSubscription.
7272
func (r *repo) UpsertSubscription(ctx context.Context, s *scdmodels.Subscription) (*scdmodels.Subscription, error) {
73+
if err := requireExtentTimes(s.StartTime, s.EndTime); err != nil {
74+
return nil, err
75+
}
7376
rec := &subscriptionRecord{
7477
ID: s.ID,
7578
Manager: s.Manager,
@@ -78,8 +81,8 @@ func (r *repo) UpsertSubscription(ctx context.Context, s *scdmodels.Subscription
7881
NotifyForOperationalIntents: s.NotifyForOperationalIntents,
7982
NotifyForConstraints: s.NotifyForConstraints,
8083
ImplicitSubscription: s.ImplicitSubscription,
81-
StartTime: cloneTime(s.StartTime),
82-
EndTime: cloneTime(s.EndTime),
84+
StartTime: *s.StartTime,
85+
EndTime: *s.EndTime,
8386
Cells: cloneCells(s.Cells),
8487
UpdatedAt: timestamp.NowFromContext(ctx),
8588
}
@@ -134,14 +137,8 @@ func (r *repo) LockSubscriptionsOnCells(_ context.Context, _ s2.CellUnion, _ []d
134137
func (r *repo) ListExpiredSubscriptions(_ context.Context, threshold time.Time) ([]*scdmodels.Subscription, error) {
135138
var out []*scdmodels.Subscription
136139
for _, rec := range r.state.Subscriptions {
137-
// (ends_at IS NOT NULL AND ends_at <= threshold) OR (ends_at IS NULL AND updated_at <= threshold)
138-
var expired bool
139-
if rec.EndTime != nil {
140-
expired = !rec.EndTime.After(threshold)
141-
} else {
142-
expired = !rec.UpdatedAt.After(threshold)
143-
}
144-
if !expired {
140+
// ends_at <= threshold
141+
if rec.EndTime.After(threshold) {
145142
continue
146143
}
147144
out = append(out, rec.toModel())

0 commit comments

Comments
 (0)