From 8eed218ba311b9f5aeb0b1e302424f1787389a32 Mon Sep 17 00:00:00 2001
From: Julien Perrochet <julien.perro@gmail.com>
Date: Mon, 2 Sep 2024 20:56:43 +0200
Subject: [PATCH] cover delete

---
 pkg/scd/operational_intents_handler.go | 91 +++++++++++++-------------
 1 file changed, 47 insertions(+), 44 deletions(-)

diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go
index a3112f516..8b23404a7 100644
--- a/pkg/scd/operational_intents_handler.go
+++ b/pkg/scd/operational_intents_handler.go
@@ -94,8 +94,8 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest
 			}
 		}
 
-		// Find Subscriptions that may overlap the OperationalIntent's Volume4D
-		allsubs, err := r.SearchSubscriptions(ctx, &dssmodels.Volume4D{
+		// Gather the subscriptions that need to be notified
+		notifyVolume := &dssmodels.Volume4D{
 			StartTime: old.StartTime,
 			EndTime:   old.EndTime,
 			SpatialVolume: &dssmodels.Volume3D{
@@ -104,23 +104,9 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest
 				Footprint: dssmodels.GeometryFunc(func() (s2.CellUnion, error) {
 					return old.Cells, nil
 				}),
-			}})
-		if err != nil {
-			return stacktrace.Propagate(err, "Unable to search Subscriptions in repo")
-		}
-
-		// Limit Subscription notifications to only those interested in OperationalIntents
-		subs := repos.Subscriptions{}
-		for _, s := range allsubs {
-			if s.NotifyForOperationalIntents {
-				subs = append(subs, s)
-			}
-		}
+			}}
 
-		// Increment notification indices for Subscriptions to be notified
-		if err := subs.IncrementNotificationIndices(ctx, r); err != nil {
-			return stacktrace.Propagate(err, "Unable to increment notification indices")
-		}
+		subsToNotify, err := getRelevantSubscriptionsAndIncrementIndices(ctx, r, notifyVolume)
 
 		// Delete OperationalIntent from repo
 		if err := r.DeleteOperationalIntent(ctx, id); err != nil {
@@ -139,7 +125,7 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest
 		// Return response to client
 		response = &restapi.ChangeOperationalIntentReferenceResponse{
 			OperationalIntentReference: *old.ToRest(),
-			Subscribers:                makeSubscribersToNotify(subs),
+			Subscribers:                makeSubscribersToNotify(subsToNotify),
 		}
 
 		return nil
@@ -517,36 +503,48 @@ func validateUpsertRequestAgainstPreviousOIR(
 	return nil
 }
 
-func getRelevantSubscriptionsAndIncrementIndices(
-	ctx context.Context,
-	r repos.Repository,
+// computeNotificationVolume computes the volume that needs to be queried for subscriptions
+// given the requested extent and the (possibly nil) previous operational intent.
+// The returned volume is either the union of the requested extent and the previous OIR's extent, or just the requested extent
+// if the previous OIR is nil.
+func computeNotificationVolume(
 	previousOIR *scdmodels.OperationalIntent,
-	requestedExtent *dssmodels.Volume4D,
-) (repos.Subscriptions, error) {
+	requestedExtent *dssmodels.Volume4D) (*dssmodels.Volume4D, error) {
+
+	if previousOIR == nil {
+		return requestedExtent, nil
+	}
 
 	// Compute total affected Volume4D for notification purposes
-	notifyVol4 := requestedExtent
-	if previousOIR != nil {
-		oldVol4 := &dssmodels.Volume4D{
-			StartTime: previousOIR.StartTime,
-			EndTime:   previousOIR.EndTime,
-			SpatialVolume: &dssmodels.Volume3D{
-				AltitudeHi: previousOIR.AltitudeUpper,
-				AltitudeLo: previousOIR.AltitudeLower,
-				Footprint: dssmodels.GeometryFunc(func() (s2.CellUnion, error) {
-					return previousOIR.Cells, nil
-				}),
-			},
-		}
-		var err error
-		notifyVol4, err = dssmodels.UnionVolumes4D(requestedExtent, oldVol4)
-		if err != nil {
-			return nil, stacktrace.Propagate(err, "Error constructing 4D volumes union")
-		}
+	oldVolume := &dssmodels.Volume4D{
+		StartTime: previousOIR.StartTime,
+		EndTime:   previousOIR.EndTime,
+		SpatialVolume: &dssmodels.Volume3D{
+			AltitudeHi: previousOIR.AltitudeUpper,
+			AltitudeLo: previousOIR.AltitudeLower,
+			Footprint: dssmodels.GeometryFunc(func() (s2.CellUnion, error) {
+				return previousOIR.Cells, nil
+			}),
+		},
+	}
+	notifyVolume, err := dssmodels.UnionVolumes4D(requestedExtent, oldVolume)
+	if err != nil {
+		return nil, stacktrace.Propagate(err, "Error constructing 4D volumes union")
 	}
 
+	return notifyVolume, nil
+}
+
+// getRelevantSubscriptionsAndIncrementIndices retrieves the subscriptions relevant to the passed volume and increments their notification indices
+// before returning them.
+func getRelevantSubscriptionsAndIncrementIndices(
+	ctx context.Context,
+	r repos.Repository,
+	notifyVolume *dssmodels.Volume4D,
+) (repos.Subscriptions, error) {
+
 	// Find Subscriptions that may need to be notified
-	allsubs, err := r.SearchSubscriptions(ctx, notifyVol4)
+	allsubs, err := r.SearchSubscriptions(ctx, notifyVolume)
 	if err != nil {
 		return nil, stacktrace.Propagate(err, "Failed to search for impacted subscriptions.")
 	}
@@ -814,8 +812,13 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize
 			return stacktrace.Propagate(err, "Failed to upsert OperationalIntent in repo")
 		}
 
+		notifyVolume, err := computeNotificationVolume(old, validParams.uExtent)
+		if err != nil {
+			return stacktrace.Propagate(err, "Failed to compute notification volume")
+		}
+
 		// Notify relevant Subscriptions
-		subsToNotify, err := getRelevantSubscriptionsAndIncrementIndices(ctx, r, old, validParams.uExtent)
+		subsToNotify, err := getRelevantSubscriptionsAndIncrementIndices(ctx, r, notifyVolume)
 		if err != nil {
 			return stacktrace.Propagate(err, "Failed to notify relevant Subscriptions")
 		}