[scd] oir upsert: factor out extension and validation of existing sub…

…scription

pkg/scd/operational_intents_handler.go

@@ -648,6 +648,81 @@ func validateKeyAndProvideConflictResponse(
 	return nil, nil
 }
 
+// dependOnExistingSubscriptionIfAllowedAndCovered ensures that:
+//   - the subscription exists
+//   - the subscription is owned by the manager
+//   - the subscription covers the requested geo-temporal extent
+//   - if the subscription is implicit, it is extended if required to cover the requested parameters
+//   - if the subscription is explicit and it is not covering the requested parameters, an error is returned
+//
+// IMPORTANT: this method currently only EXTENDS implicit subscriptions:
+//
+//	It WILL NOT shrink an implicit subscription if that subscription covers more than the area of its dependant OIRs.
+//	This is relevant for situations where clients manually specify implicit subscriptions that were previously
+//	created when creating or updating their OIRs: they may get notifications for areas they are not interested in.
+func dependOnExistingSubscriptionIfAllowedAndCovered(ctx context.Context, r repos.Repository, manager dssmodels.Manager, params *validOIRParams) (*scdmodels.Subscription, error) {
+	if params.subscriptionID.Empty() {
+		// This situation should have been caught earlier but we check again to be safe
+		return nil, stacktrace.NewError("Cannot depend on a non-specified Subscription")
+	}
+
+	sub, err := r.GetSubscription(context.Background(), params.subscriptionID)
+	if err != nil {
+		return nil, stacktrace.Propagate(err, "Unable to get depended-upon Subscription")
+	}
+	if sub == nil {
+		return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "depended-upon Subscription %s does not exist", params.subscriptionID)
+	}
+
+	if sub.Manager != manager {
+		return nil,
+			stacktrace.Propagate(
+				// We do a bit of wrapping gymnastics because the root error message will be sent in the response,
+				// and we don't want to include the effective manager in there.
+				stacktrace.NewErrorWithCode(
+					dsserr.PermissionDenied, "Specificed Subscription is owned by different client"),
+				// The propagation message will end in the logs and help with debugging.
+				"Subscription %s owned by %s, but %s attempted to use it for an OperationalIntent",
+				params.subscriptionID,
+				sub.Manager,
+				manager,
+			)
+	}
+	updateSub := false
+	if sub.StartTime != nil && sub.StartTime.After(*params.uExtent.StartTime) {
+		if sub.ImplicitSubscription {
+			sub.StartTime = params.uExtent.StartTime
+			updateSub = true
+		} else {
+			return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription does not begin until after the OperationalIntent starts")
+		}
+	}
+	if sub.EndTime != nil && sub.EndTime.Before(*params.uExtent.EndTime) {
+		if sub.ImplicitSubscription {
+			sub.EndTime = params.uExtent.EndTime
+			updateSub = true
+		} else {
+			return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription ends before the OperationalIntent ends")
+		}
+	}
+	if !sub.Cells.Contains(params.cells) {
+		if sub.ImplicitSubscription {
+			sub.Cells = s2.CellUnionFromUnion(sub.Cells, params.cells)
+			updateSub = true
+		} else {
+			return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription does not cover entire spatial area of the OperationalIntent")
+		}
+	}
+	if updateSub {
+		sub, err = r.UpsertSubscription(ctx, sub)
+		if err != nil {
+			return nil, stacktrace.Propagate(err, "Failed to update existing Subscription")
+		}
+	}
+
+	return sub, nil
+}
+
 // upsertOperationalIntentReference inserts or updates an Operational Intent.
 // If the ovn argument is empty (""), it will attempt to create a new Operational Intent.
 func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorizedManager *api.AuthorizationResult, entityid restapi.EntityID, ovn restapi.EntityOVN, params *restapi.PutOperationalIntentReferenceParameters,
@@ -727,49 +802,15 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize
 			}
 
 		} else {
-			// Use existing Subscription
-			sub, err = r.GetSubscription(ctx, validParams.subscriptionID)
+			// Use existing Subscription and adapt it if needed and possible
+			sub, err = dependOnExistingSubscriptionIfAllowedAndCovered(ctx, r, manager, validParams)
 			if err != nil {
-				return stacktrace.Propagate(err, "Unable to get Subscription")
-			}
-			if sub == nil {
-				return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Specified Subscription %s does not exist", validParams.subscriptionID)
-			}
-			if sub.Manager != manager {
-				return stacktrace.Propagate(
-					stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Specificed Subscription is owned by different client"),
-					"Subscription %s owned by %s, but %s attempted to use it for an OperationalIntent", validParams.subscriptionID, sub.Manager, manager)
-			}
-			updateSub := false
-			if sub.StartTime != nil && sub.StartTime.After(*validParams.uExtent.StartTime) {
-				if sub.ImplicitSubscription {
-					sub.StartTime = validParams.uExtent.StartTime
-					updateSub = true
-				} else {
-					return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription does not begin until after the OperationalIntent starts")
-				}
-			}
-			if sub.EndTime != nil && sub.EndTime.Before(*validParams.uExtent.EndTime) {
-				if sub.ImplicitSubscription {
-					sub.EndTime = validParams.uExtent.EndTime
-					updateSub = true
-				} else {
-					return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription ends before the OperationalIntent ends")
-				}
-			}
-			if !sub.Cells.Contains(validParams.cells) {
-				if sub.ImplicitSubscription {
-					sub.Cells = s2.CellUnionFromUnion(sub.Cells, validParams.cells)
-					updateSub = true
-				} else {
-					return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription does not cover entire spatial area of the OperationalIntent")
-				}
-			}
-			if updateSub {
-				sub, err = r.UpsertSubscription(ctx, sub)
-				if err != nil {
-					return stacktrace.Propagate(err, "Failed to update existing Subscription")
-				}
+				// Propagating the code here is important for properly handling all failure cases in the caller
+				return stacktrace.PropagateWithCode(
+					err,
+					stacktrace.GetCode(err),
+					"could not depend on specified subscription: %s",
+					validParams.subscriptionID)
 			}
 		}