Skip to content

Commit f2aa4d9

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

4 files changed

Lines changed: 79 additions & 52 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: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ 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),
38-
AltitudeLower: cloneFloat32(rec.AltitudeLower),
39-
AltitudeUpper: cloneFloat32(rec.AltitudeUpper),
38+
AltitudeLower: float32Ptr(rec.AltitudeLower),
39+
AltitudeUpper: float32Ptr(rec.AltitudeUpper),
4040
Cells: cloneCells(rec.Cells),
4141
}
4242
}
@@ -99,17 +99,24 @@ 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+
if err := requireAltitudeBounds(operation.AltitudeLower, operation.AltitudeUpper); err != nil {
106+
return nil, err
107+
}
108+
102109
rec := &operationalIntentRecord{
103110
ID: operation.ID,
104111
Manager: operation.Manager,
105112
Version: operation.Version,
106113
State: operation.State,
107-
StartTime: cloneTime(operation.StartTime),
108-
EndTime: cloneTime(operation.EndTime),
114+
StartTime: *operation.StartTime,
115+
EndTime: *operation.EndTime,
109116
USSBaseURL: operation.USSBaseURL,
110117
SubscriptionID: cloneID(operation.SubscriptionID),
111-
AltitudeLower: cloneFloat32(operation.AltitudeLower),
112-
AltitudeUpper: cloneFloat32(operation.AltitudeUpper),
118+
AltitudeLower: *operation.AltitudeLower,
119+
AltitudeUpper: *operation.AltitudeUpper,
113120
Cells: cloneCells(operation.Cells),
114121
USSRequestedOVN: ussRequestedOVN,
115122
PastOVNs: clonePastOVNs(operation.PastOVNs),
@@ -144,19 +151,19 @@ func (r *repo) SearchOperationalIntents(ctx context.Context, v4d *dssmodels.Volu
144151
continue
145152
}
146153
// COALESCE(altitude_upper >= $2, true) with $2 = SpatialVolume.AltitudeLo
147-
if rec.AltitudeUpper != nil && v4d.SpatialVolume.AltitudeLo != nil && *rec.AltitudeUpper < *v4d.SpatialVolume.AltitudeLo {
154+
if v4d.SpatialVolume.AltitudeLo != nil && rec.AltitudeUpper < *v4d.SpatialVolume.AltitudeLo {
148155
continue
149156
}
150157
// COALESCE(altitude_lower <= $3, true) with $3 = SpatialVolume.AltitudeHi
151-
if rec.AltitudeLower != nil && v4d.SpatialVolume.AltitudeHi != nil && *rec.AltitudeLower > *v4d.SpatialVolume.AltitudeHi {
158+
if v4d.SpatialVolume.AltitudeHi != nil && rec.AltitudeLower > *v4d.SpatialVolume.AltitudeHi {
152159
continue
153160
}
154161
// COALESCE(ends_at >= $4, true) with $4 = v4d.StartTime
155-
if rec.EndTime != nil && v4d.StartTime != nil && rec.EndTime.Before(*v4d.StartTime) {
162+
if v4d.StartTime != nil && rec.EndTime.Before(*v4d.StartTime) {
156163
continue
157164
}
158165
// COALESCE(starts_at <= $5, true) with $5 = v4d.EndTime
159-
if rec.StartTime != nil && v4d.EndTime != nil && rec.StartTime.After(*v4d.EndTime) {
166+
if v4d.EndTime != nil && rec.StartTime.After(*v4d.EndTime) {
160167
continue
161168
}
162169
matched = append(matched, rec)
@@ -182,14 +189,8 @@ func (r *repo) GetDependentOperationalIntents(_ context.Context, subscriptionID
182189
func (r *repo) ListExpiredOperationalIntents(ctx context.Context, threshold time.Time) ([]*scdmodels.OperationalIntent, error) {
183190
var matched []*operationalIntentRecord
184191
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 {
192+
// ends_at <= threshold
193+
if rec.EndTime.After(threshold) {
193194
continue
194195
}
195196
matched = append(matched, rec)

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

Lines changed: 38 additions & 12 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,12 +69,12 @@ 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
76-
AltitudeLower *float32
77-
AltitudeUpper *float32
76+
AltitudeLower float32
77+
AltitudeUpper float32
7878
Cells s2.CellUnion
7979
USSRequestedOVN string
8080
PastOVNs []scdmodels.OVN
@@ -137,14 +137,40 @@ 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
140+
func timePtr(t time.Time) *time.Time {
141+
v := t
142+
return &v
143+
}
144+
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")
143150
}
144-
v := *t
151+
if end == nil {
152+
return stacktrace.NewError("End time must be provided")
153+
}
154+
return nil
155+
}
156+
157+
func float32Ptr(f float32) *float32 {
158+
v := f
145159
return &v
146160
}
147161

162+
// requireAltitudeBounds enforces the memstore invariant that operational intent
163+
// records always carry both altitude bounds.
164+
func requireAltitudeBounds(lower, upper *float32) error {
165+
if lower == nil {
166+
return stacktrace.NewError("Lower altitude must be provided")
167+
}
168+
if upper == nil {
169+
return stacktrace.NewError("Upper altitude must be provided")
170+
}
171+
return nil
172+
}
173+
148174
func cloneFloat32(f *float32) *float32 {
149175
if f == nil {
150176
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
}
@@ -174,14 +177,8 @@ func (r *repo) LockSubscriptionsOnCells(_ context.Context, _ s2.CellUnion, _ []d
174177
func (r *repo) ListExpiredSubscriptions(_ context.Context, threshold time.Time) ([]*scdmodels.Subscription, error) {
175178
var out []*scdmodels.Subscription
176179
for _, rec := range r.state.Subscriptions {
177-
// (ends_at IS NOT NULL AND ends_at <= threshold) OR (ends_at IS NULL AND updated_at <= threshold)
178-
var expired bool
179-
if rec.EndTime != nil {
180-
expired = !rec.EndTime.After(threshold)
181-
} else {
182-
expired = !rec.UpdatedAt.After(threshold)
183-
}
184-
if !expired {
180+
// ends_at <= threshold
181+
if rec.EndTime.After(threshold) {
185182
continue
186183
}
187184
out = append(out, rec.toModel())

0 commit comments

Comments
 (0)