[scd] Factorize volume4D union from rest

pkg/models/scd_conversions.go

@@ -21,6 +21,15 @@ func WithRequireTimeBounds() Volume4DValidator {
 	}
 }
 
+func WithRequireEndTimeAfter(now time.Time) Volume4DValidator {
+	return func(v *Volume4D) error {
+		if v.EndTime != nil && v.EndTime.Before(now) {
+			return stacktrace.NewError("End time may not be in the past")
+		}
+		return nil
+	}
+}
+
 func WithRequireAltitudeBounds() Volume4DValidator {
 	return func(v *Volume4D) error {
 		if v.SpatialVolume.AltitudeLo == nil {
@@ -33,8 +42,33 @@ func WithRequireAltitudeBounds() Volume4DValidator {
 	}
 }
 
+// UnionVolumes4DFromSCDRest converts a slice of vol4 SCD v1 REST model to a single bounding Volume4D
+// Validation is applied on the resulting volume union
+func UnionVolumes4DFromSCDRest(vol4s []restapi.Volume4D, validators ...Volume4DValidator) (*Volume4D, error) {
+	volumes := make([]*Volume4D, len(vol4s))
+	for idx, vol4 := range vol4s {
+		volume, err := Volume4DFromSCDRest(&vol4)
+		if err != nil {
+			return nil, stacktrace.Propagate(err, "Failed to parse volume %d", idx)
+		}
+		volumes[idx] = volume
+	}
+	union, err := UnionVolumes4D(volumes...)
+	if err != nil {
+		return nil, stacktrace.Propagate(err, "Failed to union volumes")
+	}
+
+	for _, validator := range validators {
+		if err := validator(union); err != nil {
+			return nil, stacktrace.Propagate(err, "Invalid volume union")
+		}
+	}
+
+	return union, nil
+}
+
 // Volume4DFromSCDRest converts vol4 SCD v1 REST model to a Volume4D
-func Volume4DFromSCDRest(vol4 *restapi.Volume4D, validators ...Volume4DValidator) (*Volume4D, error) {
+func Volume4DFromSCDRest(vol4 *restapi.Volume4D) (*Volume4D, error) {
 	vol3, err := Volume3DFromSCDRest(&vol4.Volume)
 	if err != nil {
 		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
 		EndTime:       endTime,
 	}
 
-	for _, validator := range validators {
-		if err := validator(volume); err != nil {
-			return nil, err
-		}
-	}
-
 	return volume, nil
 }
 

pkg/models/scd_conversions_test.go

@@ -6,106 +6,169 @@ import (
 
 	restapi "github.com/interuss/dss/pkg/api/scdv1"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestVolume4DFromSCDRest(t *testing.T) {
-	start := time.Date(2024, time.December, 15, 15, 0, 0, 0, time.UTC)
-	end := start.Add(time.Hour)
+func newRestTime(t time.Time) *restapi.Time {
+	return &restapi.Time{Value: t.Format(time.RFC3339), Format: TimeFormatRFC3339}
+}
+
+func newRestAlt(a float32) *restapi.Altitude {
+	return &restapi.Altitude{Value: float64(a), Reference: ReferenceW84, Units: UnitsM}
+}
+
+func TestUnionVolumes4DFromSCDRest(t *testing.T) {
+	timeStart := time.Date(2024, time.December, 15, 15, 0, 0, 0, time.UTC)
+	timeMid := timeStart.Add(time.Minute)
+	timeEnd := timeStart.Add(time.Hour)
 
-	timeStart := restapi.Time{Value: start.Format(time.RFC3339), Format: TimeFormatRFC3339}
-	timeEnd := restapi.Time{Value: end.Format(time.RFC3339), Format: TimeFormatRFC3339}
-	timeInvalid := restapi.Time{Value: start.Format(time.ANSIC)}
-	altLower := restapi.Altitude{Value: 100.0, Reference: ReferenceW84, Units: UnitsM}
 	altLo := float32(100.0)
-	altUpper := restapi.Altitude{Value: 200.0, Reference: ReferenceW84, Units: UnitsM}
+	altMid := float32(150.0)
 	altHi := float32(200.0)
+
 	testCases := []struct {
 		name       string
 		validators []Volume4DValidator
-		rest       *restapi.Volume4D
+		rest       []restapi.Volume4D
 		want       *Volume4D
 		wantErr    bool
 	}{
 		{
-			name: "Empty",
-			rest: &restapi.Volume4D{},
-			want: &Volume4D{SpatialVolume: &Volume3D{}},
-		},
-		{
-			name:       "Times",
-			validators: []Volume4DValidator{WithRequireTimeBounds()},
-			rest:       &restapi.Volume4D{TimeStart: &timeStart, TimeEnd: &timeEnd},
-			want: &Volume4D{
-				SpatialVolume: &Volume3D{},
-				StartTime:     &start,
-				EndTime:       &end,
+			name: "Time",
+			validators: []Volume4DValidator{
+				WithRequireTimeBounds(),
+				WithRequireEndTimeAfter(timeEnd.Add(-time.Minute)),
 			},
+			rest: []restapi.Volume4D{
+				{TimeStart: newRestTime(timeStart), TimeEnd: newRestTime(timeMid)},
+				{TimeStart: newRestTime(timeStart), TimeEnd: newRestTime(timeEnd)},
+			},
+			want: &Volume4D{SpatialVolume: &Volume3D{}, StartTime: &timeStart, EndTime: &timeEnd},
 		},
 		{
-			name:    "InvalidTimeStart",
-			rest:    &restapi.Volume4D{TimeStart: &timeInvalid},
-			wantErr: true,
-		},
-		{
-			name:    "InvalidTimeEnd",
-			rest:    &restapi.Volume4D{TimeEnd: &timeInvalid},
-			wantErr: true,
-		},
-		{
-			name:    "TimeStartAfterTimeEnd",
-			rest:    &restapi.Volume4D{TimeStart: &timeEnd, TimeEnd: &timeStart},
+			name:       "TimeEndExpired",
+			validators: []Volume4DValidator{WithRequireEndTimeAfter(timeEnd.Add(time.Minute))},
+			rest: []restapi.Volume4D{
+				{TimeEnd: newRestTime(timeMid)},
+				{TimeEnd: newRestTime(timeEnd)},
+			},
 			wantErr: true,
 		},
 		{
 			name:       "MissingTimeStart",
 			validators: []Volume4DValidator{WithRequireTimeBounds()},
-			rest:       &restapi.Volume4D{TimeEnd: &timeEnd},
-			wantErr:    true,
+			rest: []restapi.Volume4D{
+				{TimeEnd: newRestTime(timeMid)},
+				{TimeStart: newRestTime(timeStart), TimeEnd: newRestTime(timeEnd)},
+			},
+			wantErr: true,
 		},
 		{
 			name:       "MissingTimeEnd",
 			validators: []Volume4DValidator{WithRequireTimeBounds()},
-			rest:       &restapi.Volume4D{TimeStart: &timeStart},
-			wantErr:    true,
+			rest: []restapi.Volume4D{
+				{TimeStart: newRestTime(timeStart), TimeEnd: newRestTime(timeMid)},
+				{TimeStart: newRestTime(timeStart)},
+			},
+			wantErr: true,
 		},
 		{
 			name:       "Altitude",
 			validators: []Volume4DValidator{WithRequireAltitudeBounds()},
-			rest:       &restapi.Volume4D{Volume: restapi.Volume3D{AltitudeLower: &altLower, AltitudeUpper: &altUpper}},
-			want:       &Volume4D{SpatialVolume: &Volume3D{AltitudeLo: &altLo, AltitudeHi: &altHi}},
+			rest: []restapi.Volume4D{
+				{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altLo), AltitudeUpper: newRestAlt(altMid)}},
+				{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altMid), AltitudeUpper: newRestAlt(altHi)}},
+			},
+			want: &Volume4D{SpatialVolume: &Volume3D{AltitudeLo: &altLo, AltitudeHi: &altHi}},
 		},
 		{
 			name:       "MissingLowerAltitude",
 			validators: []Volume4DValidator{WithRequireAltitudeBounds()},
-			rest:       &restapi.Volume4D{Volume: restapi.Volume3D{AltitudeUpper: &altUpper}},
-			wantErr:    true,
+			rest: []restapi.Volume4D{
+				{Volume: restapi.Volume3D{AltitudeUpper: newRestAlt(altMid)}},
+				{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altMid), AltitudeUpper: newRestAlt(altHi)}},
+			},
+			wantErr: true,
 		},
 		{
 			name:       "MissingUpperAltitude",
 			validators: []Volume4DValidator{WithRequireAltitudeBounds()},
-			rest:       &restapi.Volume4D{Volume: restapi.Volume3D{AltitudeLower: &altLower}},
-			wantErr:    true,
+			rest: []restapi.Volume4D{
+				{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altLo), AltitudeUpper: newRestAlt(altMid)}},
+				{Volume: restapi.Volume3D{AltitudeLower: newRestAlt(altMid)}},
+			},
+			wantErr: true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			actual, err := UnionVolumes4DFromSCDRest(testCase.rest, testCase.validators...)
+			if testCase.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, testCase.want, actual)
+			}
+		})
+	}
+}
+
+func TestVolume4DFromSCDRest(t *testing.T) {
+	start := time.Date(2024, time.December, 15, 15, 0, 0, 0, time.UTC)
+	end := start.Add(time.Hour)
+	restInvalid := &restapi.Time{Value: start.Format(time.ANSIC)}
+
+	testCases := []struct {
+		name    string
+		rest    *restapi.Volume4D
+		want    *Volume4D
+		wantErr bool
+	}{
+		{
+			name: "Empty",
+			rest: &restapi.Volume4D{},
+			want: &Volume4D{SpatialVolume: &Volume3D{}},
+		},
+		{
+			name: "Times",
+			rest: &restapi.Volume4D{TimeStart: newRestTime(start), TimeEnd: newRestTime(end)},
+			want: &Volume4D{SpatialVolume: &Volume3D{}, StartTime: &start, EndTime: &end},
+		},
+		{
+			name:    "InvalidTimeStart",
+			rest:    &restapi.Volume4D{TimeStart: restInvalid},
+			wantErr: true,
+		},
+		{
+			name:    "InvalidTimeEnd",
+			rest:    &restapi.Volume4D{TimeEnd: restInvalid},
+			wantErr: true,
+		},
+		{
+			name:    "TimeStartAfterTimeEnd",
+			rest:    &restapi.Volume4D{TimeStart: newRestTime(end), TimeEnd: newRestTime(start)},
+			wantErr: true,
 		},
 	}
 
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			actual, err := Volume4DFromSCDRest(testCase.rest, testCase.validators...)
+			actual, err := Volume4DFromSCDRest(testCase.rest)
 			if testCase.wantErr {
-				assert.Error(t, err)
+				require.Error(t, err)
 			} else {
+				require.NoError(t, err)
 				assert.Equal(t, testCase.want, actual)
 			}
 		})
 	}
 }
 
 func TestVolume3DFromSCDRest(t *testing.T) {
-	lower := restapi.Altitude{Value: 100.0, Reference: ReferenceW84, Units: UnitsM}
 	lo := float32(100.0)
-	upper := restapi.Altitude{Value: 200.0, Reference: ReferenceW84, Units: UnitsM}
 	hi := float32(200.0)
-	invalid := restapi.Altitude{Value: 0}
+	restInvalid := &restapi.Altitude{Value: 0}
 
 	testCases := []struct {
 		name    string
@@ -141,22 +204,22 @@ func TestVolume3DFromSCDRest(t *testing.T) {
 		},
 		{
 			name: "Altitudes",
-			rest: &restapi.Volume3D{AltitudeLower: &lower, AltitudeUpper: &upper},
+			rest: &restapi.Volume3D{AltitudeLower: newRestAlt(lo), AltitudeUpper: newRestAlt(hi)},
 			want: &Volume3D{AltitudeLo: &lo, AltitudeHi: &hi},
 		},
 		{
 			name:    "InvalidLowerAltitude",
-			rest:    &restapi.Volume3D{AltitudeLower: &invalid},
+			rest:    &restapi.Volume3D{AltitudeLower: restInvalid},
 			wantErr: true,
 		},
 		{
 			name:    "InvalidUpperAltitude",
-			rest:    &restapi.Volume3D{AltitudeUpper: &invalid},
+			rest:    &restapi.Volume3D{AltitudeUpper: restInvalid},
 			wantErr: true,
 		},
 		{
 			name:    "LowerAltitudeGreaterThanUpperAltitude",
-			rest:    &restapi.Volume3D{AltitudeLower: &upper, AltitudeUpper: &lower},
+			rest:    &restapi.Volume3D{AltitudeLower: newRestAlt(hi), AltitudeUpper: newRestAlt(lo)},
 			wantErr: true,
 		},
 		{
@@ -170,8 +233,9 @@ func TestVolume3DFromSCDRest(t *testing.T) {
 		t.Run(testCase.name, func(t *testing.T) {
 			actual, err := Volume3DFromSCDRest(testCase.rest)
 			if testCase.wantErr {
-				assert.Error(t, err)
+				require.Error(t, err)
 			} else {
+				require.NoError(t, err)
 				assert.Equal(t, testCase.want, actual)
 			}
 		})

pkg/scd/constraints_handler.go

@@ -362,7 +362,6 @@ func (a *Server) PutConstraintReference(ctx context.Context, manager string, ent
 
 type validConstraintParams struct {
 	id         dssmodels.ID
-	extents    []*dssmodels.Volume4D
 	uExtent    *dssmodels.Volume4D
 	cells      s2.CellUnion
 	ussBaseURL string
@@ -414,23 +413,15 @@ func validateAndReturnConstraintUpsertParams(
 		}
 	}
 
-	// TODO: factor out logic below into common multi-vol4d parser and reuse with PutOperationReference
-	valid.extents = make([]*dssmodels.Volume4D, len(params.Extents))
-	for idx, extent := range params.Extents {
-		// Start and end times are required for each volume
-		cExtent, err := dssmodels.Volume4DFromSCDRest(&extent, dssmodels.WithRequireTimeBounds())
-		if err != nil {
-			return nil, stacktrace.Propagate(err, "Failed to parse extent %d", idx)
-		}
-		valid.extents[idx] = cExtent
-	}
-	valid.uExtent, err = dssmodels.UnionVolumes4D(valid.extents...)
+	// Start and end times are required for each volume
+	// The end time may not be in the past
+	valid.uExtent, err = dssmodels.UnionVolumes4DFromSCDRest(
+		params.Extents,
+		dssmodels.WithRequireTimeBounds(),
+		dssmodels.WithRequireEndTimeAfter(now),
+	)
 	if err != nil {
-		return nil, stacktrace.Propagate(err, "Failed to union extents")
-	}
-
-	if now.After(*valid.uExtent.EndTime) {
-		return nil, stacktrace.NewError("Constraint may not end in the past")
+		return nil, stacktrace.Propagate(err, "Invalid extents")
 	}
 
 	valid.cells, err = valid.uExtent.CalculateSpatialCovering()