From 83bc1485547976d7f5787f443752be7807b951b3 Mon Sep 17 00:00:00 2001
From: Julien Perrochet <julien.perro@gmail.com>
Date: Fri, 6 Sep 2024 12:06:09 +0200
Subject: [PATCH] [scd] oir upsert: factor out building and validating of the
 OIR (#1099)

---
 pkg/scd/models/operational_intents.go  | 21 ----------
 pkg/scd/operational_intents_handler.go | 58 ++++++++++++++------------
 2 files changed, 31 insertions(+), 48 deletions(-)

diff --git a/pkg/scd/models/operational_intents.go b/pkg/scd/models/operational_intents.go
index 6e5d607a3..182aa8a53 100644
--- a/pkg/scd/models/operational_intents.go
+++ b/pkg/scd/models/operational_intents.go
@@ -5,9 +5,7 @@ import (
 
 	"github.com/golang/geo/s2"
 	restapi "github.com/interuss/dss/pkg/api/scdv1"
-	dsserr "github.com/interuss/dss/pkg/errors"
 	dssmodels "github.com/interuss/dss/pkg/models"
-	"github.com/interuss/stacktrace"
 )
 
 // Aggregates constants for operational intents.
@@ -128,25 +126,6 @@ func (o *OperationalIntent) ToRest() *restapi.OperationalIntentReference {
 	return result
 }
 
-// ValidateTimeRange validates the time range of o.
-func (o *OperationalIntent) ValidateTimeRange() error {
-	if o.StartTime == nil {
-		return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Operation must have an time_start")
-	}
-
-	// EndTime cannot be omitted for new Operational Intents.
-	if o.EndTime == nil {
-		return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Operation must have an time_end")
-	}
-
-	// EndTime cannot be before StartTime.
-	if o.EndTime.Sub(*o.StartTime) < 0 {
-		return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Operation time_end must be after time_start")
-	}
-
-	return nil
-}
-
 // SetCells is a convenience function that accepts an int64 array and converts
 // to s2.CellUnion.
 // TODO: wrap s2.CellUnion in a custom type that embeds the struct such that
diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go
index cce204de4..edde460c2 100644
--- a/pkg/scd/operational_intents_handler.go
+++ b/pkg/scd/operational_intents_handler.go
@@ -369,6 +369,32 @@ type validOIRParams struct {
 	key map[scdmodels.OVN]bool
 }
 
+func (vp *validOIRParams) toOIR(manager dssmodels.Manager, attachedSub *scdmodels.Subscription, version int32) *scdmodels.OperationalIntent {
+	// For OIR's in the accepted state, we may not have a attachedSub available,
+	// in such cases the attachedSub ID on scdmodels.OperationalIntent will be nil
+	// and will be replaced with the 'NullV4UUID' when sent over to a client.
+	var subID *dssmodels.ID
+	if attachedSub != nil {
+		// Note: do _not_ use vp.subscriptionID here, as it may be empty
+		subID = &attachedSub.ID
+	}
+	return &scdmodels.OperationalIntent{
+		ID:      vp.id,
+		Manager: manager,
+		Version: scdmodels.VersionNumber(version),
+
+		StartTime:     vp.uExtent.StartTime,
+		EndTime:       vp.uExtent.EndTime,
+		AltitudeLower: vp.uExtent.SpatialVolume.AltitudeLo,
+		AltitudeUpper: vp.uExtent.SpatialVolume.AltitudeHi,
+		Cells:         vp.cells,
+
+		USSBaseURL:     vp.ussBaseURL,
+		SubscriptionID: subID,
+		State:          vp.state,
+	}
+}
+
 // validateAndReturnUpsertParams checks that the parameters for an Operational Intent Reference upsert are valid.
 // Note that this does NOT check for anything related to access controls: any error returned should be labeled
 // as a dsserr.BadRequest.
@@ -463,6 +489,10 @@ func validateAndReturnUpsertParams(
 		return nil, stacktrace.NewError("OperationalIntents may not end in the past")
 	}
 
+	if valid.uExtent.StartTime.After(*valid.uExtent.EndTime) {
+		return nil, stacktrace.NewError("Operation time_end must be after time_start")
+	}
+
 	valid.cells, err = valid.uExtent.CalculateSpatialCovering()
 	if err != nil {
 		return nil, stacktrace.Propagate(err, "Invalid area")
@@ -808,34 +838,8 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize
 			}
 		}
 
-		// For OIR's in the accepted state, we may not have a subscription available,
-		// in such cases the subscription ID on scdmodels.OperationalIntent will be nil
-		// and will be replaced with the 'NullV4UUID' when sent over to a client.
-		var subID *dssmodels.ID = nil
-		if sub != nil {
-			subID = &sub.ID
-		}
-
 		// Construct the new OperationalIntent
-		op := &scdmodels.OperationalIntent{
-			ID:      validParams.id,
-			Manager: manager,
-			Version: scdmodels.VersionNumber(version + 1),
-
-			StartTime:     validParams.uExtent.StartTime,
-			EndTime:       validParams.uExtent.EndTime,
-			AltitudeLower: validParams.uExtent.SpatialVolume.AltitudeLo,
-			AltitudeUpper: validParams.uExtent.SpatialVolume.AltitudeHi,
-			Cells:         validParams.cells,
-
-			USSBaseURL:     validParams.ussBaseURL,
-			SubscriptionID: subID,
-			State:          validParams.state,
-		}
-		err = op.ValidateTimeRange()
-		if err != nil {
-			return stacktrace.Propagate(err, "Error validating time range")
-		}
+		op := validParams.toOIR(manager, sub, version+1)
 
 		// Upsert the OperationalIntent
 		op, err = r.UpsertOperationalIntent(ctx, op)