From daf4e31be4ab8c5abcb1cf863deba1dc3c2b43ed Mon Sep 17 00:00:00 2001
From: Julien Perrochet <julien.perro@gmail.com>
Date: Mon, 29 Jul 2024 14:50:11 +0200
Subject: [PATCH] [scd] inline cleanup of implicit subscription into CRDB call
 when removing OIR

---
 pkg/scd/operational_intents_handler.go        | 14 -------
 .../store/cockroach/operational_intents.go    | 40 +++++++++++++++++--
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go
index a09fcb74c..f1972b966 100644
--- a/pkg/scd/operational_intents_handler.go
+++ b/pkg/scd/operational_intents_handler.go
@@ -92,11 +92,6 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest
 				"OperationalIntent owned by %s, but %s attempted to delete", old.Manager, *req.Auth.ClientID)
 		}
 
-		removeImplicitSubscription, err := subscriptionCanBeRemoved(ctx, r, old.SubscriptionID)
-		if err != nil {
-			return stacktrace.Propagate(err, "Could not determine if Subscription can be removed")
-		}
-
 		// Find Subscriptions that may overlap the OperationalIntent's Volume4D
 		allsubs, err := r.SearchSubscriptions(ctx, &dssmodels.Volume4D{
 			StartTime: old.StartTime,
@@ -130,15 +125,6 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest
 			return stacktrace.Propagate(err, "Unable to delete OperationalIntent from repo")
 		}
 
-		// removeImplicitSubscription is only true if the OIR had a subscription defined
-		if removeImplicitSubscription {
-			// Automatically remove a now-unused implicit Subscription
-			err = r.DeleteSubscription(ctx, *old.SubscriptionID)
-			if err != nil {
-				return stacktrace.Propagate(err, "Unable to delete associated implicit Subscription")
-			}
-		}
-
 		// Return response to client
 		response = &restapi.ChangeOperationalIntentReferenceResponse{
 			OperationalIntentReference: *old.ToRest(),
diff --git a/pkg/scd/store/cockroach/operational_intents.go b/pkg/scd/store/cockroach/operational_intents.go
index e98d3d280..2d79cd54b 100644
--- a/pkg/scd/store/cockroach/operational_intents.go
+++ b/pkg/scd/store/cockroach/operational_intents.go
@@ -146,10 +146,34 @@ func (s *repo) GetOperationalIntent(ctx context.Context, id dssmodels.ID) (*scdm
 func (s *repo) DeleteOperationalIntent(ctx context.Context, id dssmodels.ID) error {
 	var (
 		deleteOperationQuery = `
-			DELETE FROM
+			WITH deleted_oir AS (
+            DELETE FROM
 				scd_operations
 			WHERE
 				id = $1
+            RETURNING
+                id, subscription_id
+            ),
+            exists_and_is_implicit AS (
+                SELECT subscription_id
+                FROM deleted_oir
+                JOIN scd_subscriptions ON scd_subscriptions.id = deleted_oir.subscription_id
+                WHERE scd_subscriptions.implicit = true
+            ),
+            dependent_oirs AS ( -- NOTE: this sub-query will still return the OIR being deleted (!)
+                SELECT id
+                FROM scd_operations
+                WHERE subscription_id = (SELECT subscription_id FROM deleted_oir)
+            ),
+            deleted_subscription_id AS (
+                DELETE FROM scd_subscriptions
+                WHERE id = (SELECT subscription_id FROM exists_and_is_implicit)
+                AND (SELECT COUNT(*) FROM dependent_oirs) = 1 -- NOTE: see above, the OIR being removed is still counted here, hence a value of 1
+                RETURNING id
+            )
+            SELECT -- NOTE: the following subqueries let us return details about which deletion occurred
+                (SELECT id FROM deleted_oir) AS deleted_oir_id,
+                (SELECT id FROM deleted_subscription_id) AS deleted_subscription_id
 		`
 	)
 
@@ -157,12 +181,22 @@ func (s *repo) DeleteOperationalIntent(ctx context.Context, id dssmodels.ID) err
 	if err != nil {
 		return stacktrace.Propagate(err, "Failed to convert id to PgUUID")
 	}
-	res, err := s.q.Exec(ctx, deleteOperationQuery, uid)
+	res, err := s.q.Query(ctx, deleteOperationQuery, uid)
 	if err != nil {
 		return stacktrace.Propagate(err, "Error in query: %s", deleteOperationQuery)
 	}
+	defer res.Close()
 
-	if res.RowsAffected() == 0 {
+	// Check that the deleted OIR ID was returned:
+	var opID *dssmodels.ID
+	if res.Next() {
+		err = res.Scan(&opID, nil)
+		if err != nil {
+			return stacktrace.Propagate(err, "Error scanning deleted Operation ID")
+		}
+	}
+
+	if opID == nil || *opID != id {
 		return stacktrace.NewError("Could not delete Operation that does not exist")
 	}