From 673189341329ee04adf011104c5f03a65c100c70 Mon Sep 17 00:00:00 2001 From: Julien Perrochet Date: Thu, 18 Jul 2024 22:35:18 +0200 Subject: [PATCH 1/4] [scd] properly cleanup implicit sub on oir update --- pkg/scd/operational_intents_handler.go | 87 +++++++++++++++++--------- 1 file changed, 58 insertions(+), 29 deletions(-) diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index 5d8c2ac2d..96b6d33ab 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -16,6 +16,33 @@ import ( "github.com/interuss/stacktrace" ) +func subscriptionCanBeRemoved(ctx context.Context, r repos.Repository, subscriptionID *dssmodels.ID) (bool, error) { + // Get the Subscription supporting the OperationalIntent, if one is defined + if subscriptionID != nil { + sub, err := r.GetSubscription(ctx, *subscriptionID) + if err != nil { + return false, stacktrace.Propagate(err, "Unable to get OperationalIntent's Subscription from repo") + } + if sub == nil { + return false, stacktrace.NewError("OperationalIntent's Subscription missing from repo") + } + + if sub.ImplicitSubscription { + // Get the Subscription's dependent OperationalIntents + dependentOps, err := r.GetDependentOperationalIntents(ctx, sub.ID) + if err != nil { + return false, stacktrace.Propagate(err, "Could not find dependent OperationalIntents") + } + if len(dependentOps) == 0 { + return false, stacktrace.NewError("An implicit Subscription had no dependent OperationalIntents") + } else if len(dependentOps) == 1 { + return true, nil + } + } + } + return false, nil +} + // DeleteOperationalIntentReference deletes a single operational intent ref for a given ID at // the specified version. func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *restapi.DeleteOperationalIntentReferenceRequest, @@ -63,35 +90,14 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest "OperationalIntent owned by %s, but %s attempted to delete", old.Manager, *req.Auth.ClientID) } - if old.OVN != ovn { - return stacktrace.NewErrorWithCode(dsserr.VersionMismatch, - "Current version is %s but client specified version %s", old.OVN, ovn) - } + if old.OVN != ovn { + return stacktrace.NewErrorWithCode(dsserr.VersionMismatch, + "Current version is %s but client specified version %s", old.OVN, ovn) + } - // Get the Subscription supporting the OperationalIntent, if one is defined - var sub *scdmodels.Subscription - removeImplicitSubscription := false - if old.SubscriptionID != nil { - sub, err = r.GetSubscription(ctx, *old.SubscriptionID) - if err != nil { - return stacktrace.Propagate(err, "Unable to get OperationalIntent's Subscription from repo") - } - if sub == nil { - return stacktrace.NewError("OperationalIntent's Subscription missing from repo") - } - - if sub.ImplicitSubscription { - // Get the Subscription's dependent OperationalIntents - dependentOps, err := r.GetDependentOperationalIntents(ctx, sub.ID) - if err != nil { - return stacktrace.Propagate(err, "Could not find dependent OperationalIntents") - } - if len(dependentOps) == 0 { - return stacktrace.NewError("An implicit Subscription had no dependent OperationalIntents") - } else if len(dependentOps) == 1 { - removeImplicitSubscription = true - } - } + 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 @@ -130,7 +136,7 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest // removeImplicitSubscription is only true if the OIR had a subscription defined if removeImplicitSubscription { // Automatically remove a now-unused implicit Subscription - err = r.DeleteSubscription(ctx, sub.ID) + err = r.DeleteSubscription(ctx, *old.SubscriptionID) if err != nil { return stacktrace.Propagate(err, "Unable to delete associated implicit Subscription") } @@ -471,6 +477,8 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize if err != nil { return stacktrace.Propagate(err, "Could not get OperationalIntent from repo") } + + var previousSubscriptionID *dssmodels.ID if old != nil { if old.Manager != manager { return stacktrace.NewErrorWithCode(dsserr.PermissionDenied, @@ -482,6 +490,7 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize } version = int32(old.Version) + previousSubscriptionID = old.SubscriptionID } else { if ovn != "" { return stacktrace.NewErrorWithCode(dsserr.NotFound, "OperationalIntent does not exist and therefore is not version %s", ovn) @@ -491,6 +500,7 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize } var sub *scdmodels.Subscription + removePreviousImplicitSubscription := false if subscriptionID.Empty() { // Create an implicit subscription if the implicit subscription params are set: // for situations where these params are required but have not been set, @@ -504,6 +514,17 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize } } + // If an implicit subscription creation is requested, we need to check if: + // - a previous subscription was attached, + // - if so, if it was an implicit subscription + // - if so, if we can remove it after creating the new implicit subscription + removePreviousImplicitSubscription, err = subscriptionCanBeRemoved(ctx, r, previousSubscriptionID) + if err != nil { + return stacktrace.Propagate(err, "Could not determine if previous Subscription can be removed") + } + + // We _could_ reuse the previous subscription ID if it was an implicit subscription, + // but for now we chose to always create a new subscription. subToUpsert := scdmodels.Subscription{ ID: dssmodels.ID(uuid.New().String()), Manager: manager, @@ -706,6 +727,14 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize return stacktrace.Propagate(err, "Failed to upsert OperationalIntent in repo") } + // Check if the previously attached subscription should be removedre + if removePreviousImplicitSubscription { + err = r.DeleteSubscription(ctx, *previousSubscriptionID) + if err != nil { + return stacktrace.Propagate(err, "Unable to delete previous implicit Subscription") + } + } + // Find Subscriptions that may need to be notified allsubs, err := r.SearchSubscriptions(ctx, notifyVol4) if err != nil { From e2dc46647e8e09971be98b755d05e69222eb38fc Mon Sep 17 00:00:00 2001 From: Julien Perrochet Date: Thu, 25 Jul 2024 23:10:25 +0200 Subject: [PATCH 2/4] Comments --- pkg/scd/operational_intents_handler.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index 96b6d33ab..68faff3a7 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -16,6 +16,15 @@ import ( "github.com/interuss/stacktrace" ) +// subscriptionCanBeRemoved will check if: +// - a previous subscription was attached, +// - if so, if it was an implicit subscription +// - if so, if we can remove it after creating the new implicit subscription +// +// This is to be used in contexts where an implicit subscription may need to be cleaned up. +// NOTE: this should eventually be pushed down to CRDB as part of the queries being executed in the callers of this method. +// +// See https://github.com/interuss/dss/issues/1059 for more details func subscriptionCanBeRemoved(ctx context.Context, r repos.Repository, subscriptionID *dssmodels.ID) (bool, error) { // Get the Subscription supporting the OperationalIntent, if one is defined if subscriptionID != nil { @@ -514,17 +523,14 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize } } - // If an implicit subscription creation is requested, we need to check if: - // - a previous subscription was attached, - // - if so, if it was an implicit subscription - // - if so, if we can remove it after creating the new implicit subscription removePreviousImplicitSubscription, err = subscriptionCanBeRemoved(ctx, r, previousSubscriptionID) if err != nil { return stacktrace.Propagate(err, "Could not determine if previous Subscription can be removed") } - // We _could_ reuse the previous subscription ID if it was an implicit subscription, - // but for now we chose to always create a new subscription. + // Note: parameters for a new implicit subscription have been passed, so we will create + // a new implicit subscription even if another subscription was attaches to this OIR before, + // (and regardless of whether it was an implicit subscription or not). subToUpsert := scdmodels.Subscription{ ID: dssmodels.ID(uuid.New().String()), Manager: manager, @@ -727,7 +733,7 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize return stacktrace.Propagate(err, "Failed to upsert OperationalIntent in repo") } - // Check if the previously attached subscription should be removedre + // Check if the previously attached subscription should be removed if removePreviousImplicitSubscription { err = r.DeleteSubscription(ctx, *previousSubscriptionID) if err != nil { From efbb391c8a7130162de8cf8a185fced719fc29f2 Mon Sep 17 00:00:00 2001 From: Julien Perrochet Date: Tue, 13 Aug 2024 09:19:20 +0200 Subject: [PATCH 3/4] rename and make subscriptionCanBeRemoved more specific --- pkg/scd/operational_intents_handler.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index 68faff3a7..90ec23818 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -16,16 +16,18 @@ import ( "github.com/interuss/stacktrace" ) -// subscriptionCanBeRemoved will check if: -// - a previous subscription was attached, -// - if so, if it was an implicit subscription -// - if so, if we can remove it after creating the new implicit subscription +// subscriptionIsOnlyAttachedToOIR will check if: +// - the subscription exists and is implicit +// - the subscription is attached to the specified operational intent +// - the subscription is not attached to any other operational intent +// +// This is to be used in contexts where an implicit subscription may need to be cleaned up: if true is returned, +// the subscription can be safely removed after the operational intent is deleted or attached to another subscription. // -// This is to be used in contexts where an implicit subscription may need to be cleaned up. // NOTE: this should eventually be pushed down to CRDB as part of the queries being executed in the callers of this method. // // See https://github.com/interuss/dss/issues/1059 for more details -func subscriptionCanBeRemoved(ctx context.Context, r repos.Repository, subscriptionID *dssmodels.ID) (bool, error) { +func subscriptionIsOnlyAttachedToOIR(ctx context.Context, r repos.Repository, oirID, subscriptionID *dssmodels.ID) (bool, error) { // Get the Subscription supporting the OperationalIntent, if one is defined if subscriptionID != nil { sub, err := r.GetSubscription(ctx, *subscriptionID) @@ -44,7 +46,7 @@ func subscriptionCanBeRemoved(ctx context.Context, r repos.Repository, subscript } if len(dependentOps) == 0 { return false, stacktrace.NewError("An implicit Subscription had no dependent OperationalIntents") - } else if len(dependentOps) == 1 { + } else if len(dependentOps) == 1 && dependentOps[0] == *oirID { return true, nil } } @@ -99,12 +101,16 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest "OperationalIntent owned by %s, but %s attempted to delete", old.Manager, *req.Auth.ClientID) } +<<<<<<< HEAD if old.OVN != ovn { return stacktrace.NewErrorWithCode(dsserr.VersionMismatch, "Current version is %s but client specified version %s", old.OVN, ovn) } removeImplicitSubscription, err := subscriptionCanBeRemoved(ctx, r, old.SubscriptionID) +======= + removeImplicitSubscription, err := subscriptionIsOnlyAttachedToOIR(ctx, r, &id, old.SubscriptionID) +>>>>>>> fdd5e11 (rename and make subscriptionCanBeRemoved more specific) if err != nil { return stacktrace.Propagate(err, "Could not determine if Subscription can be removed") } @@ -523,7 +529,7 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize } } - removePreviousImplicitSubscription, err = subscriptionCanBeRemoved(ctx, r, previousSubscriptionID) + removePreviousImplicitSubscription, err = subscriptionIsOnlyAttachedToOIR(ctx, r, &id, previousSubscriptionID) if err != nil { return stacktrace.Propagate(err, "Could not determine if previous Subscription can be removed") } From 9ca5e54532c76c53f58902290cc059c337ec70a5 Mon Sep 17 00:00:00 2001 From: Julien Perrochet Date: Fri, 23 Aug 2024 13:08:55 +0200 Subject: [PATCH 4/4] second round of comments, handle another corner case --- pkg/scd/operational_intents_handler.go | 163 ++++++++++++++----------- 1 file changed, 89 insertions(+), 74 deletions(-) diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index 90ec23818..6d2ef8a28 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -16,8 +16,8 @@ import ( "github.com/interuss/stacktrace" ) -// subscriptionIsOnlyAttachedToOIR will check if: -// - the subscription exists and is implicit +// subscriptionIsImplicitAndOnlyAttachedToOIR will check if: +// - the subscription is defined and is implicit // - the subscription is attached to the specified operational intent // - the subscription is not attached to any other operational intent // @@ -27,29 +27,22 @@ import ( // NOTE: this should eventually be pushed down to CRDB as part of the queries being executed in the callers of this method. // // See https://github.com/interuss/dss/issues/1059 for more details -func subscriptionIsOnlyAttachedToOIR(ctx context.Context, r repos.Repository, oirID, subscriptionID *dssmodels.ID) (bool, error) { - // Get the Subscription supporting the OperationalIntent, if one is defined - if subscriptionID != nil { - sub, err := r.GetSubscription(ctx, *subscriptionID) - if err != nil { - return false, stacktrace.Propagate(err, "Unable to get OperationalIntent's Subscription from repo") - } - if sub == nil { - return false, stacktrace.NewError("OperationalIntent's Subscription missing from repo") - } - - if sub.ImplicitSubscription { - // Get the Subscription's dependent OperationalIntents - dependentOps, err := r.GetDependentOperationalIntents(ctx, sub.ID) - if err != nil { - return false, stacktrace.Propagate(err, "Could not find dependent OperationalIntents") - } - if len(dependentOps) == 0 { - return false, stacktrace.NewError("An implicit Subscription had no dependent OperationalIntents") - } else if len(dependentOps) == 1 && dependentOps[0] == *oirID { - return true, nil - } - } +func subscriptionIsImplicitAndOnlyAttachedToOIR(ctx context.Context, r repos.Repository, oirID *dssmodels.ID, subscription *scdmodels.Subscription) (bool, error) { + if subscription == nil { + return false, nil + } + if !subscription.ImplicitSubscription { + return false, nil + } + // Get the Subscription's dependent OperationalIntents + dependentOps, err := r.GetDependentOperationalIntents(ctx, subscription.ID) + if err != nil { + return false, stacktrace.Propagate(err, "Could not find dependent OperationalIntents") + } + if len(dependentOps) == 0 { + return false, stacktrace.NewError("An implicit Subscription had no dependent OperationalIntents") + } else if len(dependentOps) == 1 && dependentOps[0] == *oirID { + return true, nil } return false, nil } @@ -101,16 +94,23 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest "OperationalIntent owned by %s, but %s attempted to delete", old.Manager, *req.Auth.ClientID) } -<<<<<<< HEAD - if old.OVN != ovn { - return stacktrace.NewErrorWithCode(dsserr.VersionMismatch, - "Current version is %s but client specified version %s", old.OVN, ovn) - } + if old.OVN != ovn { + return stacktrace.NewErrorWithCode(dsserr.VersionMismatch, + "Current version is %s but client specified version %s", old.OVN, ovn) + } + + var previousSubscription *scdmodels.Subscription + if old.SubscriptionID != nil { + previousSubscription, err = r.GetSubscription(ctx, *old.SubscriptionID) + if err != nil { + return stacktrace.Propagate(err, "Unable to get OperationalIntent's Subscription from repo") + } + if previousSubscription == nil { + return stacktrace.NewError("OperationalIntent's Subscription missing from repo") + } + } - removeImplicitSubscription, err := subscriptionCanBeRemoved(ctx, r, old.SubscriptionID) -======= - removeImplicitSubscription, err := subscriptionIsOnlyAttachedToOIR(ctx, r, &id, old.SubscriptionID) ->>>>>>> fdd5e11 (rename and make subscriptionCanBeRemoved more specific) + removeImplicitSubscription, err := subscriptionIsImplicitAndOnlyAttachedToOIR(ctx, r, &id, previousSubscription) if err != nil { return stacktrace.Propagate(err, "Could not determine if Subscription can be removed") } @@ -458,9 +458,9 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize return nil, nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid state for initial version: `%s`", params.State) } - subscriptionID := dssmodels.ID("") + specifiedSubscriptionID := dssmodels.ID("") if params.SubscriptionId != nil { - subscriptionID, err = dssmodels.IDFromOptionalString(string(*params.SubscriptionId)) + specifiedSubscriptionID, err = dssmodels.IDFromOptionalString(string(*params.SubscriptionId)) if err != nil { return nil, nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format for Subscription ID: `%s`", *params.SubscriptionId) } @@ -469,7 +469,7 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize // Check if a subscription is required for this request: // OIRs in an accepted state do not need a subscription. if state.RequiresSubscription() && - subscriptionID.Empty() && + specifiedSubscriptionID.Empty() && (params.NewSubscription == nil || params.NewSubscription.UssBaseUrl == "") { return nil, nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Provided Operational Intent Reference state `%s` requires either a subscription ID or information to create an implicit subscription", state) @@ -493,7 +493,7 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize return stacktrace.Propagate(err, "Could not get OperationalIntent from repo") } - var previousSubscriptionID *dssmodels.ID + var previousSubscription *scdmodels.Subscription if old != nil { if old.Manager != manager { return stacktrace.NewErrorWithCode(dsserr.PermissionDenied, @@ -505,7 +505,13 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize } version = int32(old.Version) - previousSubscriptionID = old.SubscriptionID + + if old.SubscriptionID != nil { + previousSubscription, err = r.GetSubscription(ctx, *old.SubscriptionID) + if err != nil { + return stacktrace.Propagate(err, "Unable to get OperationalIntent's Subscription from repo") + } + } } else { if ovn != "" { return stacktrace.NewErrorWithCode(dsserr.NotFound, "OperationalIntent does not exist and therefore is not version %s", ovn) @@ -514,13 +520,23 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize version = 0 } - var sub *scdmodels.Subscription + // Determine if the previous subscription needs to be cleaned up + previousSubIsBeingReplaced := previousSubscription != nil && specifiedSubscriptionID != previousSubscription.ID removePreviousImplicitSubscription := false - if subscriptionID.Empty() { - // Create an implicit subscription if the implicit subscription params are set: - // for situations where these params are required but have not been set, - // an error will have been returned earlier. - // If they are not set at this point, continue without creating an implicit subscription. + if previousSubIsBeingReplaced { + removePreviousImplicitSubscription, err = subscriptionIsImplicitAndOnlyAttachedToOIR(ctx, r, &id, previousSubscription) + if err != nil { + return stacktrace.Propagate(err, "Could not determine if previous Subscription can be removed") + } + } + + // effectiveSubscription is the subscription that will end up being attached to the OIR + // it defaults to the previous subscription, but may be updated if required by the parameters + effectiveSubscription := previousSubscription + if specifiedSubscriptionID.Empty() { + // No subscription ID was provided: + // We will check if parameters to create a new implicit subscription were provided, + // otherwise we will do nothing. if params.NewSubscription != nil && params.NewSubscription.UssBaseUrl != "" { if !a.AllowHTTPBaseUrls { err := scdmodels.ValidateUSSBaseURL(string(params.NewSubscription.UssBaseUrl)) @@ -529,11 +545,6 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize } } - removePreviousImplicitSubscription, err = subscriptionIsOnlyAttachedToOIR(ctx, r, &id, previousSubscriptionID) - if err != nil { - return stacktrace.Propagate(err, "Could not determine if previous Subscription can be removed") - } - // Note: parameters for a new implicit subscription have been passed, so we will create // a new implicit subscription even if another subscription was attaches to this OIR before, // (and regardless of whether it was an implicit subscription or not). @@ -553,53 +564,57 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize subToUpsert.NotifyForConstraints = *params.NewSubscription.NotifyForConstraints } - sub, err = r.UpsertSubscription(ctx, &subToUpsert) + effectiveSubscription, err = r.UpsertSubscription(ctx, &subToUpsert) if err != nil { return stacktrace.Propagate(err, "Failed to create implicit subscription") } } } else { - // Use existing Subscription - sub, err = r.GetSubscription(ctx, subscriptionID) - 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", subscriptionID) + // A subscription has been specified: + // If it is different from the previous subscription, we need to fetch it from the store + // in order to ensure it correctly covers the OIR. + if effectiveSubscription == nil || previousSubIsBeingReplaced { + effectiveSubscription, err = r.GetSubscription(ctx, specifiedSubscriptionID) + if err != nil { + return stacktrace.Propagate(err, "Unable to get requested Subscription from store") + } + if effectiveSubscription == nil { + return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Specified Subscription %s does not exist", specifiedSubscriptionID) + } } - if sub.Manager != dssmodels.Manager(manager) { + if effectiveSubscription.Manager != dssmodels.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", subscriptionID, sub.Manager, manager) + "Subscription %s owned by %s, but %s attempted to use it for an OperationalIntent", specifiedSubscriptionID, effectiveSubscription.Manager, manager) } updateSub := false - if sub.StartTime != nil && sub.StartTime.After(*uExtent.StartTime) { - if sub.ImplicitSubscription { - sub.StartTime = uExtent.StartTime + if effectiveSubscription.StartTime != nil && effectiveSubscription.StartTime.After(*uExtent.StartTime) { + if effectiveSubscription.ImplicitSubscription { + effectiveSubscription.StartTime = 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(*uExtent.EndTime) { - if sub.ImplicitSubscription { - sub.EndTime = uExtent.EndTime + if effectiveSubscription.EndTime != nil && effectiveSubscription.EndTime.Before(*uExtent.EndTime) { + if effectiveSubscription.ImplicitSubscription { + effectiveSubscription.EndTime = uExtent.EndTime updateSub = true } else { return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription ends before the OperationalIntent ends") } } - if !sub.Cells.Contains(cells) { - if sub.ImplicitSubscription { - sub.Cells = s2.CellUnionFromUnion(sub.Cells, cells) + if !effectiveSubscription.Cells.Contains(cells) { + if effectiveSubscription.ImplicitSubscription { + effectiveSubscription.Cells = s2.CellUnionFromUnion(effectiveSubscription.Cells, 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) + effectiveSubscription, err = r.UpsertSubscription(ctx, effectiveSubscription) if err != nil { return stacktrace.Propagate(err, "Failed to update existing Subscription") } @@ -634,7 +649,7 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize // Identify Constraints missing from the key var missingConstraints []*scdmodels.Constraint - if sub != nil && sub.NotifyForConstraints { + if effectiveSubscription != nil && effectiveSubscription.NotifyForConstraints { constraints, err := r.SearchConstraints(ctx, uExtent) if err != nil { return stacktrace.Propagate(err, "Unable to SearchConstraints") @@ -687,8 +702,8 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize // 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 + if effectiveSubscription != nil { + subID = &effectiveSubscription.ID } // Construct the new OperationalIntent @@ -741,7 +756,7 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize // Check if the previously attached subscription should be removed if removePreviousImplicitSubscription { - err = r.DeleteSubscription(ctx, *previousSubscriptionID) + err = r.DeleteSubscription(ctx, previousSubscription.ID) if err != nil { return stacktrace.Propagate(err, "Unable to delete previous implicit Subscription") }