Skip to content

Commit 955aed1

Browse files
RustedBonesRustedBones
authored
[scd] Factorize volume4D union from rest (#1381)
1 parent 8a59aa6 commit 955aed1

File tree

4 files changed

+169
-95
lines changed

4 files changed

+169
-95
lines changed

‎pkg/models/scd_conversions.go‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@ func WithRequireTimeBounds() Volume4DValidator {
2121
}
2222
}
2323

24+
func WithRequireEndTimeAfter(now time.Time) Volume4DValidator {
25+
return func(v *Volume4D) error {
26+
if v.EndTime != nil && v.EndTime.Before(now) {
27+
return stacktrace.NewError("End time may not be in the past")
28+
}
29+
return nil
30+
}
31+
}
32+
2433
func WithRequireAltitudeBounds() Volume4DValidator {
2534
return func(v *Volume4D) error {
2635
if v.SpatialVolume.AltitudeLo == nil {
@@ -33,8 +42,33 @@ func WithRequireAltitudeBounds() Volume4DValidator {
3342
}
3443
}
3544

45+
// UnionVolumes4DFromSCDRest converts a slice of vol4 SCD v1 REST model to a single bounding Volume4D
46+
// Validation is applied on the resulting volume union
47+
func UnionVolumes4DFromSCDRest(vol4s []restapi.Volume4D, validators ...Volume4DValidator) (*Volume4D, error) {
48+
volumes := make([]*Volume4D, len(vol4s))
49+
for idx, vol4 := range vol4s {
50+
volume, err := Volume4DFromSCDRest(&vol4)
51+
if err != nil {
52+
return nil, stacktrace.Propagate(err, "Failed to parse volume %d", idx)
53+
}
54+
volumes[idx] = volume
55+
}
56+
union, err := UnionVolumes4D(volumes...)
57+
if err != nil {
58+
return nil, stacktrace.Propagate(err, "Failed to union volumes")
59+
}
60+
61+
for _, validator := range validators {
62+
if err := validator(union); err != nil {
63+
return nil, stacktrace.Propagate(err, "Invalid volume union")
64+
}
65+
}
66+
67+
return union, nil
68+
}
69+
3670
// Volume4DFromSCDRest converts vol4 SCD v1 REST model to a Volume4D
37-
func Volume4DFromSCDRest(vol4 *restapi.Volume4D, validators ...Volume4DValidator) (*Volume4D, error) {
71+
func Volume4DFromSCDRest(vol4 *restapi.Volume4D) (*Volume4D, error) {
3872
vol3, err := Volume3DFromSCDRest(&vol4.Volume)
3973
if err != nil {
4074
return nil, err // No need to Propagate this error as this stack layer does not add useful information
@@ -68,12 +102,6 @@ func Volume4DFromSCDRest(vol4 *restapi.Volume4D, validators ...Volume4DValidator
68102
EndTime: endTime,
69103
}
70104

71-
for _, validator := range validators {
72-
if err := validator(volume); err != nil {
73-
return nil, err
74-
}
75-
}
76-
77105
return volume, nil
78106
}
79107

‎pkg/models/scd_conversions_test.go‎

Lines changed: 117 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -6,106 +6,169 @@ import (
66

77
restapi "github.com/interuss/dss/pkg/api/scdv1"
88
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
910
)
1011

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)
12+
func newRestTime(t time.Time) *restapi.Time {
13+
return &restapi.Time{Value: t.Format(time.RFC3339), Format: TimeFormatRFC3339}
14+
}
15+
16+
func newRestAlt(a float32) *restapi.Altitude {
17+
return &restapi.Altitude{Value: float64(a), Reference: ReferenceW84, Units: UnitsM}
18+
}
19+
20+
func TestUnionVolumes4DFromSCDRest(t *testing.T) {
21+
timeStart := time.Date(2024, time.December, 15, 15, 0, 0, 0, time.UTC)
22+
timeMid := timeStart.Add(time.Minute)
23+
timeEnd := timeStart.Add(time.Hour)
1424

15-
timeStart := restapi.Time{Value: start.Format(time.RFC3339), Format: TimeFormatRFC3339}
16-
timeEnd := restapi.Time{Value: end.Format(time.RFC3339), Format: TimeFormatRFC3339}
17-
timeInvalid := restapi.Time{Value: start.Format(time.ANSIC)}
18-
altLower := restapi.Altitude{Value: 100.0, Reference: ReferenceW84, Units: UnitsM}
1925
altLo := float32(100.0)
20-
altUpper := restapi.Altitude{Value: 200.0, Reference: ReferenceW84, Units: UnitsM}
26+
altMid := float32(150.0)
2127
altHi := float32(200.0)
28+
2229
testCases := []struct {
2330
name string
2431
validators []Volume4DValidator
25-
rest *restapi.Volume4D
32+
rest []restapi.Volume4D
2633
want *Volume4D
2734
wantErr bool
2835
}{
2936
{
30-
name: "Empty",
31-
rest: &restapi.Volume4D{},
32-
want: &Volume4D{SpatialVolume: &Volume3D{}},
33-
},
34-
{
35-
name: "Times",
36-
validators: []Volume4DValidator{WithRequireTimeBounds()},
37-
rest: &restapi.Volume4D{TimeStart: &timeStart, TimeEnd: &timeEnd},
38-
want: &Volume4D{
39-
SpatialVolume: &Volume3D{},
40-
StartTime: &start,
41-
EndTime: &end,
37+
name: "Time",
38+
validators: []Volume4DValidator{
39+
WithRequireTimeBounds(),
40+
WithRequireEndTimeAfter(timeEnd.Add(-time.Minute)),
4241
},
42+
rest: []restapi.Volume4D{
43+
{TimeStart: newRestTime(timeStart), TimeEnd: newRestTime(timeMid)},
44+
{TimeStart: newRestTime(timeStart), TimeEnd: newRestTime(timeEnd)},
45+
},
46+
want: &Volume4D{SpatialVolume: &Volume3D{}, StartTime: &timeStart, EndTime: &timeEnd},
4347
},
4448
{
45-
name: "InvalidTimeStart",
46-
rest: &restapi.Volume4D{TimeStart: &timeInvalid},
47-
wantErr: true,
48-
},
49-
{
50-
name: "InvalidTimeEnd",
51-
rest: &restapi.Volume4D{TimeEnd: &timeInvalid},
52-
wantErr: true,
53-
},
54-
{
55-
name: "TimeStartAfterTimeEnd",
56-
rest: &restapi.Volume4D{TimeStart: &timeEnd, TimeEnd: &timeStart},
49+
name: "TimeEndExpired",
50+
validators: []Volume4DValidator{WithRequireEndTimeAfter(timeEnd.Add(time.Minute))},
51+
rest: []restapi.Volume4D{
52+
{TimeEnd: newRestTime(timeMid)},
53+
{TimeEnd: newRestTime(timeEnd)},
54+
},
5755
wantErr: true,
5856
},
5957
{
6058
name: "MissingTimeStart",
6159
validators: []Volume4DValidator{WithRequireTimeBounds()},
62-
rest: &restapi.Volume4D{TimeEnd: &timeEnd},
63-
wantErr: true,
60+
rest: []restapi.Volume4D{
61+
{TimeEnd: newRestTime(timeMid)},
62+
{TimeStart: newRestTime(timeStart), TimeEnd: newRestTime(timeEnd)},
63+
},
64+
wantErr: true,
6465
},
6566
{
6667
name: "MissingTimeEnd",
6768
validators: []Volume4DValidator{WithRequireTimeBounds()},
68-
rest: &restapi.Volume4D{TimeStart: &timeStart},
69-
wantErr: true,
69+
rest: []restapi.Volume4D{
70+
{TimeStart: newRestTime(timeStart), TimeEnd: newRestTime(timeMid)},
71+
{TimeStart: newRestTime(timeStart)},
72+
},
73+
wantErr: true,
7074
},
7175
{
7276
name: "Altitude",
7377
validators: []Volume4DValidator{WithRequireAltitudeBounds()},
74-
rest: &restapi.Volume4D{Volume: restapi.Volume3D{AltitudeLower: &altLower, AltitudeUpper: &altUpper}},
75-
want: &Volume4D{SpatialVolume: &Volume3D{AltitudeLo: &altLo, AltitudeHi: &altHi}},
78+
rest: []restapi.Volume4D{
79+
{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altLo), AltitudeUpper: newRestAlt(altMid)}},
80+
{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altMid), AltitudeUpper: newRestAlt(altHi)}},
81+
},
82+
want: &Volume4D{SpatialVolume: &Volume3D{AltitudeLo: &altLo, AltitudeHi: &altHi}},
7683
},
7784
{
7885
name: "MissingLowerAltitude",
7986
validators: []Volume4DValidator{WithRequireAltitudeBounds()},
80-
rest: &restapi.Volume4D{Volume: restapi.Volume3D{AltitudeUpper: &altUpper}},
81-
wantErr: true,
87+
rest: []restapi.Volume4D{
88+
{Volume: restapi.Volume3D{AltitudeUpper: newRestAlt(altMid)}},
89+
{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altMid), AltitudeUpper: newRestAlt(altHi)}},
90+
},
91+
wantErr: true,
8292
},
8393
{
8494
name: "MissingUpperAltitude",
8595
validators: []Volume4DValidator{WithRequireAltitudeBounds()},
86-
rest: &restapi.Volume4D{Volume: restapi.Volume3D{AltitudeLower: &altLower}},
87-
wantErr: true,
96+
rest: []restapi.Volume4D{
97+
{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altLo), AltitudeUpper: newRestAlt(altMid)}},
98+
{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altMid)}},
99+
},
100+
wantErr: true,
101+
},
102+
}
103+
104+
for _, testCase := range testCases {
105+
t.Run(testCase.name, func(t *testing.T) {
106+
actual, err := UnionVolumes4DFromSCDRest(testCase.rest, testCase.validators...)
107+
if testCase.wantErr {
108+
require.Error(t, err)
109+
} else {
110+
require.NoError(t, err)
111+
assert.Equal(t, testCase.want, actual)
112+
}
113+
})
114+
}
115+
}
116+
117+
func TestVolume4DFromSCDRest(t *testing.T) {
118+
start := time.Date(2024, time.December, 15, 15, 0, 0, 0, time.UTC)
119+
end := start.Add(time.Hour)
120+
restInvalid := &restapi.Time{Value: start.Format(time.ANSIC)}
121+
122+
testCases := []struct {
123+
name string
124+
rest *restapi.Volume4D
125+
want *Volume4D
126+
wantErr bool
127+
}{
128+
{
129+
name: "Empty",
130+
rest: &restapi.Volume4D{},
131+
want: &Volume4D{SpatialVolume: &Volume3D{}},
132+
},
133+
{
134+
name: "Times",
135+
rest: &restapi.Volume4D{TimeStart: newRestTime(start), TimeEnd: newRestTime(end)},
136+
want: &Volume4D{SpatialVolume: &Volume3D{}, StartTime: &start, EndTime: &end},
137+
},
138+
{
139+
name: "InvalidTimeStart",
140+
rest: &restapi.Volume4D{TimeStart: restInvalid},
141+
wantErr: true,
142+
},
143+
{
144+
name: "InvalidTimeEnd",
145+
rest: &restapi.Volume4D{TimeEnd: restInvalid},
146+
wantErr: true,
147+
},
148+
{
149+
name: "TimeStartAfterTimeEnd",
150+
rest: &restapi.Volume4D{TimeStart: newRestTime(end), TimeEnd: newRestTime(start)},
151+
wantErr: true,
88152
},
89153
}
90154

91155
for _, testCase := range testCases {
92156
t.Run(testCase.name, func(t *testing.T) {
93-
actual, err := Volume4DFromSCDRest(testCase.rest, testCase.validators...)
157+
actual, err := Volume4DFromSCDRest(testCase.rest)
94158
if testCase.wantErr {
95-
assert.Error(t, err)
159+
require.Error(t, err)
96160
} else {
161+
require.NoError(t, err)
97162
assert.Equal(t, testCase.want, actual)
98163
}
99164
})
100165
}
101166
}
102167

103168
func TestVolume3DFromSCDRest(t *testing.T) {
104-
lower := restapi.Altitude{Value: 100.0, Reference: ReferenceW84, Units: UnitsM}
105169
lo := float32(100.0)
106-
upper := restapi.Altitude{Value: 200.0, Reference: ReferenceW84, Units: UnitsM}
107170
hi := float32(200.0)
108-
invalid := restapi.Altitude{Value: 0}
171+
restInvalid := &restapi.Altitude{Value: 0}
109172

110173
testCases := []struct {
111174
name string
@@ -141,22 +204,22 @@ func TestVolume3DFromSCDRest(t *testing.T) {
141204
},
142205
{
143206
name: "Altitudes",
144-
rest: &restapi.Volume3D{AltitudeLower: &lower, AltitudeUpper: &upper},
207+
rest: &restapi.Volume3D{AltitudeLower: newRestAlt(lo), AltitudeUpper: newRestAlt(hi)},
145208
want: &Volume3D{AltitudeLo: &lo, AltitudeHi: &hi},
146209
},
147210
{
148211
name: "InvalidLowerAltitude",
149-
rest: &restapi.Volume3D{AltitudeLower: &invalid},
212+
rest: &restapi.Volume3D{AltitudeLower: restInvalid},
150213
wantErr: true,
151214
},
152215
{
153216
name: "InvalidUpperAltitude",
154-
rest: &restapi.Volume3D{AltitudeUpper: &invalid},
217+
rest: &restapi.Volume3D{AltitudeUpper: restInvalid},
155218
wantErr: true,
156219
},
157220
{
158221
name: "LowerAltitudeGreaterThanUpperAltitude",
159-
rest: &restapi.Volume3D{AltitudeLower: &upper, AltitudeUpper: &lower},
222+
rest: &restapi.Volume3D{AltitudeLower: newRestAlt(hi), AltitudeUpper: newRestAlt(lo)},
160223
wantErr: true,
161224
},
162225
{
@@ -170,8 +233,9 @@ func TestVolume3DFromSCDRest(t *testing.T) {
170233
t.Run(testCase.name, func(t *testing.T) {
171234
actual, err := Volume3DFromSCDRest(testCase.rest)
172235
if testCase.wantErr {
173-
assert.Error(t, err)
236+
require.Error(t, err)
174237
} else {
238+
require.NoError(t, err)
175239
assert.Equal(t, testCase.want, actual)
176240
}
177241
})

‎pkg/scd/constraints_handler.go‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,6 @@ func (a *Server) PutConstraintReference(ctx context.Context, manager string, ent
362362

363363
type validConstraintParams struct {
364364
id dssmodels.ID
365-
extents []*dssmodels.Volume4D
366365
uExtent *dssmodels.Volume4D
367366
cells s2.CellUnion
368367
ussBaseURL string
@@ -414,23 +413,15 @@ func validateAndReturnConstraintUpsertParams(
414413
}
415414
}
416415

417-
// TODO: factor out logic below into common multi-vol4d parser and reuse with PutOperationReference
418-
valid.extents = make([]*dssmodels.Volume4D, len(params.Extents))
419-
for idx, extent := range params.Extents {
420-
// Start and end times are required for each volume
421-
cExtent, err := dssmodels.Volume4DFromSCDRest(&extent, dssmodels.WithRequireTimeBounds())
422-
if err != nil {
423-
return nil, stacktrace.Propagate(err, "Failed to parse extent %d", idx)
424-
}
425-
valid.extents[idx] = cExtent
426-
}
427-
valid.uExtent, err = dssmodels.UnionVolumes4D(valid.extents...)
416+
// Start and end times are required for each volume
417+
// The end time may not be in the past
418+
valid.uExtent, err = dssmodels.UnionVolumes4DFromSCDRest(
419+
params.Extents,
420+
dssmodels.WithRequireTimeBounds(),
421+
dssmodels.WithRequireEndTimeAfter(now),
422+
)
428423
if err != nil {
429-
return nil, stacktrace.Propagate(err, "Failed to union extents")
430-
}
431-
432-
if now.After(*valid.uExtent.EndTime) {
433-
return nil, stacktrace.NewError("Constraint may not end in the past")
424+
return nil, stacktrace.Propagate(err, "Invalid extents")
434425
}
435426

436427
valid.cells, err = valid.uExtent.CalculateSpatialCovering()

0 commit comments

Comments
 (0)