Skip to content

Commit 9a11fcb

Browse files
RustedBonesRustedBones
committed
[scd] Validate Volume4D when parsing
1 parent b987f5e commit 9a11fcb

5 files changed

Lines changed: 234 additions & 40 deletions

File tree

‎pkg/models/scd_conversions.go‎

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,38 +7,63 @@ import (
77
"github.com/interuss/stacktrace"
88
)
99

10+
type Volume4DOpts struct {
11+
RequireAltitudeBounds bool
12+
RequireTimeBounds bool
13+
}
14+
15+
func (opts *Volume4DOpts) to3DOpts() Volume3DOpts {
16+
return Volume3DOpts{
17+
RequireAltitudeBounds: opts.RequireAltitudeBounds,
18+
}
19+
}
20+
1021
// Volume4DFromSCDRest converts vol4 SCD v1 REST model to a Volume4D
11-
func Volume4DFromSCDRest(vol4 *restapi.Volume4D) (*Volume4D, error) {
12-
vol3, err := Volume3DFromSCDRest(&vol4.Volume)
22+
func Volume4DFromSCDRest(vol4 *restapi.Volume4D, opts Volume4DOpts) (*Volume4D, error) {
23+
vol3, err := Volume3DFromSCDRest(&vol4.Volume, opts.to3DOpts())
1324
if err != nil {
1425
return nil, err // No need to Propagate this error as this stack layer does not add useful information
1526
}
1627

17-
result := &Volume4D{
18-
SpatialVolume: vol3,
19-
}
20-
28+
var startTime *time.Time
2129
if vol4.TimeStart != nil {
2230
ts, err := time.Parse(time.RFC3339Nano, vol4.TimeStart.Value)
2331
if err != nil {
2432
return nil, stacktrace.Propagate(err, "Error converting start time")
2533
}
26-
result.StartTime = &ts
34+
startTime = &ts
35+
} else if opts.RequireTimeBounds {
36+
return nil, stacktrace.NewError("Missing start time")
2737
}
2838

39+
var endTime *time.Time
2940
if vol4.TimeEnd != nil {
3041
ts, err := time.Parse(time.RFC3339Nano, vol4.TimeEnd.Value)
3142
if err != nil {
3243
return nil, stacktrace.Propagate(err, "Error converting end time")
3344
}
34-
result.EndTime = &ts
45+
endTime = &ts
46+
} else if opts.RequireTimeBounds {
47+
return nil, stacktrace.NewError("Missing end time")
3548
}
3649

37-
return result, nil
50+
if startTime != nil && endTime != nil && startTime.After(*endTime) {
51+
return nil, stacktrace.NewError("Start time cannot be after end time")
52+
}
53+
54+
return &Volume4D{
55+
SpatialVolume: vol3,
56+
StartTime: startTime,
57+
EndTime: endTime,
58+
}, nil
59+
}
60+
61+
type Volume3DOpts struct {
62+
RequireAltitudeBounds bool
3863
}
3964

4065
// Volume3DFromSCDRest converts a vol3 SCD v1 REST model to a Volume3D
41-
func Volume3DFromSCDRest(vol3 *restapi.Volume3D) (*Volume3D, error) {
66+
func Volume3DFromSCDRest(vol3 *restapi.Volume3D, opts Volume3DOpts) (*Volume3D, error) {
4267
if vol3 == nil {
4368
return nil, nil
4469
}
@@ -52,6 +77,8 @@ func Volume3DFromSCDRest(vol3 *restapi.Volume3D) (*Volume3D, error) {
5277
return nil, stacktrace.NewError("Invalid lower altitude reference")
5378
}
5479
altLo = float32p(float32(vol3.AltitudeLower.Value))
80+
} else if opts.RequireAltitudeBounds {
81+
return nil, stacktrace.NewError("Missing lower altitude")
5582
}
5683

5784
var altHi *float32
@@ -63,6 +90,12 @@ func Volume3DFromSCDRest(vol3 *restapi.Volume3D) (*Volume3D, error) {
6390
return nil, stacktrace.NewError("Invalid upper altitude reference")
6491
}
6592
altHi = float32p(float32(vol3.AltitudeUpper.Value))
93+
} else if opts.RequireAltitudeBounds {
94+
return nil, stacktrace.NewError("Missing upper altitude")
95+
}
96+
97+
if altLo != nil && altHi != nil && *altLo > *altHi {
98+
return nil, stacktrace.NewError("Lower altitude cannot be greater than upper altitude")
6699
}
67100

68101
switch {

‎pkg/models/scd_conversions_test.go‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
package models
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
restapi "github.com/interuss/dss/pkg/api/scdv1"
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestVolume4DFromSCDRest(t *testing.T) {
12+
start := time.Date(2024, time.December, 15, 15, 0, 0, 0, time.UTC)
13+
end := start.Add(time.Hour)
14+
15+
timeStart := restapi.Time{Value: start.Format(time.RFC3339), Format: TimeFormatRFC3339}
16+
timeEnd := restapi.Time{Value: end.Format(time.RFC3339), Format: TimeFormatRFC3339}
17+
invalid := restapi.Time{Value: start.Format(time.ANSIC)}
18+
testCases := []struct {
19+
name string
20+
opts Volume4DOpts
21+
rest *restapi.Volume4D
22+
want *Volume4D
23+
wantErr bool
24+
}{
25+
{
26+
name: "Empty",
27+
rest: &restapi.Volume4D{},
28+
want: &Volume4D{SpatialVolume: &Volume3D{}},
29+
},
30+
{
31+
name: "Times",
32+
opts: Volume4DOpts{RequireTimeBounds: true},
33+
rest: &restapi.Volume4D{TimeStart: &timeStart, TimeEnd: &timeEnd},
34+
want: &Volume4D{
35+
SpatialVolume: &Volume3D{},
36+
StartTime: &start,
37+
EndTime: &end,
38+
},
39+
},
40+
{
41+
name: "InvalidTimeStart",
42+
rest: &restapi.Volume4D{TimeStart: &invalid},
43+
wantErr: true,
44+
},
45+
{
46+
name: "InvalidTimeEnd",
47+
rest: &restapi.Volume4D{TimeEnd: &invalid},
48+
wantErr: true,
49+
},
50+
{
51+
name: "TimeStartAfterTimeEnd",
52+
rest: &restapi.Volume4D{TimeStart: &timeEnd, TimeEnd: &timeStart},
53+
wantErr: true,
54+
},
55+
{
56+
name: "MissingTimeStart",
57+
opts: Volume4DOpts{RequireTimeBounds: true},
58+
rest: &restapi.Volume4D{TimeEnd: &timeEnd},
59+
wantErr: true,
60+
},
61+
{
62+
name: "MissingTimeEnd",
63+
opts: Volume4DOpts{RequireTimeBounds: true},
64+
rest: &restapi.Volume4D{TimeStart: &timeStart},
65+
wantErr: true,
66+
},
67+
}
68+
69+
for _, testCase := range testCases {
70+
t.Run(testCase.name, func(t *testing.T) {
71+
actual, err := Volume4DFromSCDRest(testCase.rest, testCase.opts)
72+
if testCase.wantErr {
73+
assert.Error(t, err)
74+
} else {
75+
assert.Equal(t, testCase.want, actual)
76+
}
77+
})
78+
}
79+
}
80+
81+
func TestVolume3DFromSCDRest(t *testing.T) {
82+
lower := restapi.Altitude{
83+
Value: 100.0,
84+
Reference: ReferenceW84,
85+
Units: UnitsM,
86+
}
87+
lo := float32(100.0)
88+
upper := restapi.Altitude{
89+
Value: 200.0,
90+
Reference: ReferenceW84,
91+
Units: UnitsM,
92+
}
93+
hi := float32(200.0)
94+
invalid := restapi.Altitude{Value: 0}
95+
96+
testCases := []struct {
97+
name string
98+
opts Volume3DOpts
99+
rest *restapi.Volume3D
100+
want *Volume3D
101+
wantErr bool
102+
}{
103+
{
104+
name: "Empty",
105+
rest: &restapi.Volume3D{},
106+
want: &Volume3D{},
107+
},
108+
{
109+
name: "Polygon",
110+
rest: &restapi.Volume3D{
111+
OutlinePolygon: &restapi.Polygon{},
112+
},
113+
want: &Volume3D{
114+
Footprint: &GeoPolygon{},
115+
},
116+
},
117+
{
118+
name: "Circle",
119+
rest: &restapi.Volume3D{
120+
OutlineCircle: &restapi.Circle{
121+
Center: &restapi.LatLngPoint{},
122+
Radius: &restapi.Radius{},
123+
},
124+
},
125+
want: &Volume3D{
126+
Footprint: &GeoCircle{},
127+
},
128+
},
129+
{
130+
name: "Altitudes",
131+
rest: &restapi.Volume3D{AltitudeLower: &lower, AltitudeUpper: &upper},
132+
opts: Volume3DOpts{RequireAltitudeBounds: true},
133+
want: &Volume3D{AltitudeLo: &lo, AltitudeHi: &hi},
134+
},
135+
{
136+
name: "InvalidLowerAltitude",
137+
rest: &restapi.Volume3D{AltitudeLower: &invalid},
138+
wantErr: true,
139+
},
140+
{
141+
name: "InvalidUpperAltitude",
142+
rest: &restapi.Volume3D{AltitudeUpper: &invalid},
143+
wantErr: true,
144+
},
145+
{
146+
name: "LowerAltitudeGreaterThanUpperAltitude",
147+
rest: &restapi.Volume3D{AltitudeLower: &upper, AltitudeUpper: &lower},
148+
wantErr: true,
149+
},
150+
{
151+
name: "MissingLowerAltitude",
152+
opts: Volume3DOpts{RequireAltitudeBounds: true},
153+
rest: &restapi.Volume3D{AltitudeUpper: &upper},
154+
wantErr: true,
155+
},
156+
{
157+
name: "MissingUpperAltitude",
158+
opts: Volume3DOpts{RequireAltitudeBounds: true},
159+
rest: &restapi.Volume3D{AltitudeLower: &lower},
160+
wantErr: true,
161+
},
162+
{
163+
name: "MuiltiGeom",
164+
rest: &restapi.Volume3D{OutlineCircle: &restapi.Circle{}, OutlinePolygon: &restapi.Polygon{}},
165+
wantErr: true,
166+
},
167+
}
168+
169+
for _, testCase := range testCases {
170+
t.Run(testCase.name, func(t *testing.T) {
171+
actual, err := Volume3DFromSCDRest(testCase.rest, testCase.opts)
172+
if testCase.wantErr {
173+
assert.Error(t, err)
174+
} else {
175+
assert.Equal(t, testCase.want, actual)
176+
}
177+
})
178+
}
179+
}

‎pkg/scd/constraints_handler.go‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -416,9 +416,10 @@ func validateAndReturnConstraintUpsertParams(
416416

417417
// TODO: factor out logic below into common multi-vol4d parser and reuse with PutOperationReference
418418
valid.extents = make([]*dssmodels.Volume4D, len(params.Extents))
419-
420419
for idx, extent := range params.Extents {
421-
cExtent, err := dssmodels.Volume4DFromSCDRest(&extent)
420+
// Start and end times are required for each volume
421+
opts := dssmodels.Volume4DOpts{RequireTimeBounds: true}
422+
cExtent, err := dssmodels.Volume4DFromSCDRest(&extent, opts)
422423
if err != nil {
423424
return nil, stacktrace.Propagate(err, "Failed to parse extent %d", idx)
424425
}
@@ -429,21 +430,10 @@ func validateAndReturnConstraintUpsertParams(
429430
return nil, stacktrace.Propagate(err, "Failed to union extents")
430431
}
431432

432-
if valid.uExtent.StartTime == nil {
433-
return nil, stacktrace.NewError("Missing time_start from extents")
434-
}
435-
if valid.uExtent.EndTime == nil {
436-
return nil, stacktrace.NewError("Missing time_end from extents")
437-
}
438-
439433
if now.After(*valid.uExtent.EndTime) {
440434
return nil, stacktrace.NewError("Constraint may not end in the past")
441435
}
442436

443-
if valid.uExtent.StartTime.After(*valid.uExtent.EndTime) {
444-
return nil, stacktrace.NewError("Constraint time_end must be after time_start")
445-
}
446-
447437
valid.cells, err = valid.uExtent.CalculateSpatialCovering()
448438
if err != nil {
449439
return nil, stacktrace.Propagate(err, "Invalid area")
@@ -475,7 +465,7 @@ func (a *Server) QueryConstraintReferences(ctx context.Context, req *restapi.Que
475465
}
476466

477467
// Parse area of interest to common Volume4D
478-
vol4, err := dssmodels.Volume4DFromSCDRest(aoi)
468+
vol4, err := dssmodels.Volume4DFromSCDRest(aoi, dssmodels.Volume4DOpts{})
479469
if err != nil {
480470
return restapi.QueryConstraintReferencesResponseSet{Response400: &restapi.ErrorResponse{
481471
Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to convert to internal geometry model"))}}

‎pkg/scd/operational_intents_handler.go‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ func (a *Server) QueryOperationalIntentReferences(ctx context.Context, req *rest
267267
}
268268

269269
// Parse area of interest to common Volume4D
270-
vol4, err := dssmodels.Volume4DFromSCDRest(aoi)
270+
vol4, err := dssmodels.Volume4DFromSCDRest(aoi, dssmodels.Volume4DOpts{})
271271
if err != nil {
272272
return restapi.QueryOperationalIntentReferencesResponseSet{Response400: &restapi.ErrorResponse{
273273
Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Error parsing geometry"))}}
@@ -504,9 +504,10 @@ func validateAndReturnOIRUpsertParams(
504504
}
505505

506506
valid.extents = make([]*dssmodels.Volume4D, len(params.Extents))
507-
508507
for idx, extent := range params.Extents {
509-
cExtent, err := dssmodels.Volume4DFromSCDRest(&extent)
508+
// Start and end times, as well as lower and upper altitudes, are required for each volume
509+
opts := dssmodels.Volume4DOpts{RequireAltitudeBounds: true, RequireTimeBounds: true}
510+
cExtent, err := dssmodels.Volume4DFromSCDRest(&extent, opts)
510511
if err != nil {
511512
return nil, stacktrace.Propagate(err, "Failed to parse extent %d", idx)
512513
}
@@ -518,21 +519,10 @@ func validateAndReturnOIRUpsertParams(
518519
return nil, stacktrace.Propagate(err, "Failed to union extents")
519520
}
520521

521-
if valid.uExtent.StartTime == nil {
522-
return nil, stacktrace.NewError("Missing time_start from extents")
523-
}
524-
if valid.uExtent.EndTime == nil {
525-
return nil, stacktrace.NewError("Missing time_end from extents")
526-
}
527-
528522
if now.After(*valid.uExtent.EndTime) {
529523
return nil, stacktrace.NewError("OperationalIntents may not end in the past")
530524
}
531525

532-
if valid.uExtent.StartTime.After(*valid.uExtent.EndTime) {
533-
return nil, stacktrace.NewError("Operation time_end must be after time_start")
534-
}
535-
536526
valid.cells, err = valid.uExtent.CalculateSpatialCovering()
537527
if err != nil {
538528
return nil, stacktrace.Propagate(err, "Invalid area")

‎pkg/scd/subscriptions_handler.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,9 @@ func (a *Server) PutSubscription(ctx context.Context, manager string, subscripti
112112
}
113113

114114
// Parse extents
115-
extents, err := dssmodels.Volume4DFromSCDRest(&params.Extents)
115+
// If end time is not specified, the value will be chosen automatically by the DSS.
116+
// If start time is not specified, it will default to the time the request is processed.
117+
extents, err := dssmodels.Volume4DFromSCDRest(&params.Extents, dssmodels.Volume4DOpts{})
116118
if err != nil {
117119
return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Unable to parse extents")
118120
}
@@ -396,7 +398,7 @@ func (a *Server) QuerySubscriptions(ctx context.Context, req *restapi.QuerySubsc
396398
}
397399

398400
// Parse area of interest to common Volume4D
399-
vol4, err := dssmodels.Volume4DFromSCDRest(aoi)
401+
vol4, err := dssmodels.Volume4DFromSCDRest(aoi, dssmodels.Volume4DOpts{})
400402
if err != nil {
401403
return restapi.QuerySubscriptionsResponseSet{Response400: &restapi.ErrorResponse{
402404
Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to convert to internal geometry model"))}}

0 commit comments

Comments
 (0)