From 7029a60c7d5d671f2403f8a9886c6413a4bfec5d Mon Sep 17 00:00:00 2001 From: Julien Perrochet Date: Tue, 3 Sep 2024 10:44:11 +0200 Subject: [PATCH] [scd] oir upsert: factor out determination of subscriptions to notify (#1092) * [scd] oir upsert: factor out determination of subscriptions to notify * comment * cover delete --- pkg/scd/operational_intents_handler.go | 129 ++++++++++++++----------- 1 file changed, 75 insertions(+), 54 deletions(-) diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index e8e4d6e53..226ed0440 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,22 +104,11 @@ 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) + if err != nil { + return stacktrace.Propagate(err, "could not obtain relevant subscriptions") } // Delete OperationalIntent from repo @@ -139,7 +128,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,6 +506,68 @@ func validateUpsertRequestAgainstPreviousOIR( return nil } +// 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) (*dssmodels.Volume4D, error) { + + if previousOIR == nil { + return requestedExtent, nil + } + + // Compute total affected Volume4D for notification purposes + 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, notifyVolume) + if err != nil { + return nil, stacktrace.Propagate(err, "Failed to search for impacted subscriptions.") + } + + // Limit Subscription notifications to only those interested in OperationalIntents + subs := repos.Subscriptions{} + for _, sub := range allsubs { + if sub.NotifyForOperationalIntents { + subs = append(subs, sub) + } + } + + // Increment notification indices for relevant Subscriptions + if err := subs.IncrementNotificationIndices(ctx, r); err != nil { + return nil, stacktrace.Propagate(err, "Failed to increment notification indices of relevant subscriptions") + } + + return subs, nil +} + // validateKeyAndProvideConflictResponse ensures that the provided key contains all the necessary OVNs relevant for the area covered by the OperationalIntent. // - If all required keys are provided, (nil, nil) will be returned. // - If keys are missing, the conflict response to be sent back as well as an error with the dsserr.MissingOVNs code will be returned. @@ -758,57 +809,27 @@ func (a *Server) upsertOperationalIntentReference(ctx context.Context, authorize return stacktrace.Propagate(err, "Error validating time range") } - // Compute total affected Volume4D for notification purposes - var notifyVol4 *dssmodels.Volume4D - if old == nil { - notifyVol4 = validParams.uExtent - } else { - oldVol4 := &dssmodels.Volume4D{ - StartTime: old.StartTime, - EndTime: old.EndTime, - SpatialVolume: &dssmodels.Volume3D{ - AltitudeHi: old.AltitudeUpper, - AltitudeLo: old.AltitudeLower, - Footprint: dssmodels.GeometryFunc(func() (s2.CellUnion, error) { - return old.Cells, nil - }), - }} - notifyVol4, err = dssmodels.UnionVolumes4D(validParams.uExtent, oldVol4) - if err != nil { - return stacktrace.Propagate(err, "Error constructing 4D volumes union") - } - } - // Upsert the OperationalIntent op, err = r.UpsertOperationalIntent(ctx, op) if err != nil { return stacktrace.Propagate(err, "Failed to upsert OperationalIntent in repo") } - // Find Subscriptions that may need to be notified - allsubs, err := r.SearchSubscriptions(ctx, notifyVol4) + notifyVolume, err := computeNotificationVolume(old, validParams.uExtent) if err != nil { - return err - } - - // Limit Subscription notifications to only those interested in OperationalIntents - subs := repos.Subscriptions{} - for _, sub := range allsubs { - if sub.NotifyForOperationalIntents { - subs = append(subs, sub) - } + return stacktrace.Propagate(err, "Failed to compute notification volume") } - // Increment notification indices for relevant Subscriptions - err = subs.IncrementNotificationIndices(ctx, r) + // Notify relevant Subscriptions + subsToNotify, err := getRelevantSubscriptionsAndIncrementIndices(ctx, r, notifyVolume) if err != nil { - return err + return stacktrace.Propagate(err, "Failed to notify relevant Subscriptions") } // Return response to client responseOK = &restapi.ChangeOperationalIntentReferenceResponse{ OperationalIntentReference: *op.ToRest(), - Subscribers: makeSubscribersToNotify(subs), + Subscribers: makeSubscribersToNotify(subsToNotify), } return nil