From d17c86d0ec076438ce04bc5ebd0230cdde1391bf Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 21 Aug 2026 08:54:03 +0200 Subject: [PATCH 01/17] [raft/scd] Extract update and create opintent --- pkg/scd/actions/operational_intents.go | 232 +++++++++++++++++++++++-- pkg/scd/operational_intents_handler.go | 219 +++-------------------- pkg/scd/server.go | 23 --- pkg/store/store.go | 11 +- 4 files changed, 255 insertions(+), 230 deletions(-) diff --git a/pkg/scd/actions/operational_intents.go b/pkg/scd/actions/operational_intents.go index 51f2162ed..ab94f5a34 100644 --- a/pkg/scd/actions/operational_intents.go +++ b/pkg/scd/actions/operational_intents.go @@ -14,6 +14,7 @@ import ( scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/dss/pkg/scd/repos" dssstore "github.com/interuss/dss/pkg/store" + "github.com/interuss/dss/pkg/timestamp" "github.com/interuss/stacktrace" ) @@ -35,6 +36,16 @@ func init() { Decode: dssstore.DecodeJSON[*restapi.DeleteOperationalIntentReferenceRequest], Execute: ExecuteDeleteOperationalIntentReference, } + Registry[restapi.CreateOperationalIntentReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*restapi.CreateOperationalIntentReferenceRequest], + Execute: ExecutePutOperationalIntentReference, + } + Registry[restapi.UpdateOperationalIntentReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*restapi.UpdateOperationalIntentReferenceRequest], + Execute: ExecutePutOperationalIntentReference, + } } // SubscriptionIsImplicitAndOnlyAttachedToOIR will check if: @@ -251,11 +262,11 @@ func CheckUpsertPermissionsAndReturnManager(authorizedManager *api.Authorization return dssmodels.Manager(*authorizedManager.ClientID), nil } -// ValidateUpsertRequestAgainstPreviousOIR checks that the client requesting an OIR upsert has the necessary permissions and that the request is valid. +// validateUpsertRequestAgainstPreviousOIR checks that the client requesting an OIR upsert has the necessary permissions and that the request is valid. // On success, the version of the OIR is returned: // - upon initial creation (if no previous OIR exists), it is 0 // - otherwise, it is the version of the previous OIR -func ValidateUpsertRequestAgainstPreviousOIR( +func validateUpsertRequestAgainstPreviousOIR( requestingManager dssmodels.Manager, providedOVN scdmodels.OVN, previousOIR *scdmodels.OperationalIntent, @@ -281,11 +292,11 @@ func ValidateUpsertRequestAgainstPreviousOIR( return nil } -// ComputeNotificationVolume computes the volume that needs to be queried for subscriptions +// 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( +func computeNotificationVolume( previousOIR *scdmodels.OperationalIntent, requestedExtent *dssmodels.Volume4D) (*dssmodels.Volume4D, error) { @@ -330,7 +341,7 @@ type validOIRParams struct { Key map[scdmodels.OVN]bool } -func (vp *validOIRParams) ToOIR(manager dssmodels.Manager, attachedSub *scdmodels.Subscription, version scdmodels.VersionNumber, pastOVNs []scdmodels.OVN) *scdmodels.OperationalIntent { +func (vp *validOIRParams) toOIR(manager dssmodels.Manager, attachedSub *scdmodels.Subscription, version scdmodels.VersionNumber, pastOVNs []scdmodels.OVN) *scdmodels.OperationalIntent { // For OIR's in the accepted state, we may not have a attachedSub available, // in such cases the attachedSub ID on scdmodels.OperationalIntent will be nil // and will be replaced with the 'NullV4UUID' when sent over to a client. @@ -475,9 +486,9 @@ func ValidateAndReturnOIRUpsertParams( return valid, nil } -// CreateAndStoreNewImplicitSubscription will create a brand new implicit subscription based on the provided parameters, +// createAndStoreNewImplicitSubscription will create a brand new implicit subscription based on the provided parameters, // store it and return it. -func CreateAndStoreNewImplicitSubscription(ctx context.Context, r repos.Repository, manager dssmodels.Manager, validParams *validOIRParams) (*scdmodels.Subscription, error) { +func createAndStoreNewImplicitSubscription(ctx context.Context, r repos.Repository, manager dssmodels.Manager, validParams *validOIRParams) (*scdmodels.Subscription, error) { generator, err := random.Generator(random.MustFromContext(ctx), "implicit-subscription:"+validParams.ID.String()) if err != nil { return nil, stacktrace.Propagate(err, "Failed to derive implicit subscription ID generator") @@ -504,11 +515,11 @@ func CreateAndStoreNewImplicitSubscription(ctx context.Context, r repos.Reposito return r.UpsertSubscription(ctx, &subToUpsert) } -// ValidateKeyAndProvideConflictResponse ensures that the provided key contains all the necessary OVNs relevant for the area covered by the OperationalIntent. +// 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. // - In case of any other error, (nil, error) will be returned. -func ValidateKeyAndProvideConflictResponse( +func validateKeyAndProvideConflictResponse( ctx context.Context, r repos.Repository, requestingManager dssmodels.Manager, @@ -584,10 +595,10 @@ func ValidateKeyAndProvideConflictResponse( return nil, nil } -// EnsureSubscriptionCoversOIR ensures that the subscription covers the requested geo-temporal extent, extending it if both possible and required, +// ensureSubscriptionCoversOIR ensures that the subscription covers the requested geo-temporal extent, extending it if both possible and required, // or failing otherwise. // After this method returns successfully, the subscription will cover the requested geo-temporal extent. -func EnsureSubscriptionCoversOIR(ctx context.Context, r repos.Repository, sub *scdmodels.Subscription, params *validOIRParams) (*scdmodels.Subscription, error) { +func ensureSubscriptionCoversOIR(ctx context.Context, r repos.Repository, sub *scdmodels.Subscription, params *validOIRParams) (*scdmodels.Subscription, error) { updateSub := false if sub.StartTime != nil && sub.StartTime.After(*params.UExtent.StartTime) { @@ -624,3 +635,202 @@ func EnsureSubscriptionCoversOIR(ctx context.Context, r repos.Repository, sub *s return sub, nil } + +// PutOperationalIntentReferenceResult is the result of an Operational Intent Reference put operation. +// Exactly one of Response or Conflict is set: Conflict is set when the upsert failed because of missing OVNs (a dsserr.MissingOVNs error is returned alongside it) +// and Response is set on success. +type PutOperationalIntentReferenceResult struct { + Response *restapi.ChangeOperationalIntentReferenceResponse + Conflict *restapi.AirspaceConflictResponse +} + +// ExecutePutOperationalIntentReference inserts or updates an Operational Intent. +// If the ovn argument is empty (""), it will attempt to create a new Operational Intent. +func ExecutePutOperationalIntentReference(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { + var ( + entityid restapi.EntityID + ovn restapi.EntityOVN + params *restapi.PutOperationalIntentReferenceParameters + auth *api.AuthorizationResult + ) + + switch req := request.(type) { + case *restapi.CreateOperationalIntentReferenceRequest: + entityid, params, auth = req.Entityid, req.Body, &req.Auth + case *restapi.UpdateOperationalIntentReferenceRequest: + entityid, ovn, params, auth = req.Entityid, req.Ovn, req.Body, &req.Auth + default: + return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.CreateOperationalIntentReferenceOperationID) + } + + now := timestamp.MustGetRequestTimestamp(ctx) + + // Base URL scheme validation is a pre-flight, request-only check performed by the handler + // before this action is proposed for consensus; skip it here (allowHTTPBaseUrls: true). + validParams, err := ValidateAndReturnOIRUpsertParams(now, entityid, ovn, params, true) + if err != nil { + return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Operational Intent Reference upsert parameters") + } + if auth.ClientID == nil { + return nil, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing manager") + } + manager := dssmodels.Manager(*auth.ClientID) + + // Get existing OperationalIntent, if any + old, err := repo.GetOperationalIntent(ctx, validParams.ID) + if err != nil { + return nil, stacktrace.Propagate(err, "Could not get OperationalIntent from repo") + } + + // Lock subscriptions based on the cell and subscriptions we're going to use + // to reduce the number of retries under concurrent load. + // See issue #1002 for details. + var subscriptionIds = make([]dssmodels.ID, 0) + + if old != nil && old.SubscriptionID != nil { + subscriptionIds = append(subscriptionIds, *old.SubscriptionID) + } + + if !validParams.SubscriptionID.Empty() { + subscriptionIds = append(subscriptionIds, validParams.SubscriptionID) + } + + err = repo.LockSubscriptionsOnCells(ctx, validParams.Cells, subscriptionIds, validParams.UExtent.StartTime, validParams.UExtent.EndTime) + if err != nil { + return nil, stacktrace.Propagate(err, "Unable to acquire lock") + } + + // Validate the request against the previous OIR + if err := validateUpsertRequestAgainstPreviousOIR(manager, validParams.OVN, old); err != nil { + return nil, stacktrace.PropagateWithCode(err, stacktrace.GetCode(err), "Request validation failed") + } + + var ( + version = scdmodels.VersionNumber(1) + pastOVNs = make([]scdmodels.OVN, 0) + previousSub *scdmodels.Subscription + ) + if old != nil { + version = old.Version + 1 + pastOVNs = append(old.PastOVNs, validParams.OVN) + + // Fetch the previous OIR's subscription if it exists + if old.SubscriptionID != nil { + previousSub, err = repo.GetSubscription(ctx, *old.SubscriptionID) + if err != nil { + return nil, stacktrace.Propagate(err, "Unable to get OperationalIntent's Subscription from repo") + } + } + } + + // Determine if the previous subscription is being replaced and if it will need to be cleaned up + previousSubIsBeingReplaced := previousSub != nil && validParams.SubscriptionID != previousSub.ID + removePreviousImplicitSubscription := false + if previousSubIsBeingReplaced { + removePreviousImplicitSubscription, err = SubscriptionIsImplicitAndOnlyAttachedToOIR(ctx, repo, validParams.ID, previousSub) + if err != nil { + return nil, stacktrace.Propagate(err, "Could not determine if previous Subscription can be removed") + } + } + + // attachedSub is the subscription that will end up being attached to the OIR + // it defaults to the previous subscription (which may be nil), and may be updated if required by the parameters + attachedSub := previousSub + if validParams.SubscriptionID.Empty() { + // No subscription ID was provided: + // check if an implicit subscription should be created, otherwise do nothing + if validParams.ImplicitSubscription.Requested { + // Parameters for a new implicit subscription have been passed: we will create + // a new implicit subscription even if another subscription was attached to this OIR before, + // regardless of whether it was an implicit subscription or not. + if attachedSub, err = createAndStoreNewImplicitSubscription(ctx, repo, manager, validParams); err != nil { + return nil, stacktrace.Propagate(err, "Failed to create implicit subscription") + } + } else { + // If no subscription ID is provided and no implicit subscription is requested, + // the OIR should have no attached subscription + attachedSub = nil + } + } else { + // Attempt to rely on the specified subscription + // 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. + // We do the check below in order to avoid re-fetching the subscription if it has not changed + if attachedSub == nil || previousSubIsBeingReplaced { + attachedSub, err = repo.GetSubscription(ctx, validParams.SubscriptionID) + if err != nil { + return nil, stacktrace.Propagate(err, "Unable to get requested Subscription from store") + } + if attachedSub == nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Specified Subscription %s does not exist", validParams.SubscriptionID) + } + } + + // We need to confirm that it is owned by the calling manager + if attachedSub.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", + validParams.SubscriptionID, + attachedSub.Manager, + manager, + ) + } + + // We need to ensure the subscription covers the OIR's geo-temporal extent + attachedSub, err = ensureSubscriptionCoversOIR(ctx, repo, attachedSub, validParams) + if err != nil { + return nil, stacktrace.Propagate(err, "Failed to ensure subscription covers OIR") + } + } + + var responseConflict *restapi.AirspaceConflictResponse + if validParams.State.RequiresKey() { + responseConflict, err = validateKeyAndProvideConflictResponse(ctx, repo, manager, validParams, attachedSub) + if err != nil { + // responseConflict is non-nil here on a dsserr.MissingOVNs error: return it alongside + // the error so the handler can still send it to the client. See the doc comment above. + return &PutOperationalIntentReferenceResult{Conflict: responseConflict}, stacktrace.PropagateWithCode(err, stacktrace.GetCode(err), "Failed to validate key") + } + } + + // Construct the new OperationalIntent + op := validParams.toOIR(manager, attachedSub, version, pastOVNs) + + // Upsert the OperationalIntent + op, err = repo.UpsertOperationalIntent(ctx, op) + if err != nil { + return nil, stacktrace.Propagate(err, "Failed to upsert OperationalIntent in repo") + } + + // Check if the previously attached subscription should be removed + if removePreviousImplicitSubscription { + err = repo.DeleteSubscription(ctx, previousSub.ID) + if err != nil { + return nil, stacktrace.Propagate(err, "Unable to delete previous implicit Subscription") + } + } + + notifyVolume, err := computeNotificationVolume(old, validParams.UExtent) + if err != nil { + return nil, stacktrace.Propagate(err, "Failed to compute notification volume") + } + + // Notify relevant Subscriptions + subsToNotify, err := repo.IncrementNotificationIndicesForOperationalIntents(ctx, notifyVolume) + if err != nil { + return nil, stacktrace.Propagate(err, "Failed to notify relevant Subscriptions") + } + + // Return response to client + return &PutOperationalIntentReferenceResult{ + Response: &restapi.ChangeOperationalIntentReferenceResponse{ + OperationalIntentReference: *op.ToRest(), + Subscribers: makeSubscribersToNotify(subsToNotify), + }, + }, nil +} diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index 3e8cce828..2d42659c5 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -2,7 +2,6 @@ package scd import ( "context" - "time" "github.com/interuss/dss/pkg/api" restapi "github.com/interuss/dss/pkg/api/scdv1" @@ -12,6 +11,7 @@ import ( scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/dss/pkg/scd/repos" dssstore "github.com/interuss/dss/pkg/store" + "github.com/interuss/dss/pkg/timestamp" "github.com/interuss/stacktrace" ) @@ -138,8 +138,18 @@ func (a *Server) CreateOperationalIntentReference(ctx context.Context, req *rest return restapi.CreateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(req.BodyParseError, dsserr.BadRequest, "Malformed params"))}} } + validParams, err := actions.ValidateAndReturnOIRUpsertParams(timestamp.MustGetRequestTimestamp(ctx), req.Entityid, "", req.Body, a.AllowHTTPBaseUrls) + if err != nil { + return restapi.CreateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Operational Intent Reference upsert parameters"))}} + } + _, err = actions.CheckUpsertPermissionsAndReturnManager(&req.Auth, validParams.State) + if err != nil { + return restapi.CreateOperationalIntentReferenceResponseSet{Response403: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.PermissionDenied, "Caller is not allowed to upsert with the requested state"))}} + } - respOK, respConflict, err := a.upsertOperationalIntentReference(ctx, time.Now(), &req.Auth, req.Entityid, "", req.Body) + result, err := dssstore.TransactWithResult[repos.Repository, *actions.PutOperationalIntentReferenceResult](ctx, a.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not put Operational Intent Reference") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -152,14 +162,14 @@ func (a *Server) CreateOperationalIntentReference(ctx context.Context, req *rest return restapi.CreateOperationalIntentReferenceResponseSet{Response409: &restapi.AirspaceConflictResponse{ Message: dsserr.Handle(ctx, err)}} case dsserr.MissingOVNs: - return restapi.CreateOperationalIntentReferenceResponseSet{Response409: respConflict} + return restapi.CreateOperationalIntentReferenceResponseSet{Response409: result.Conflict} default: return restapi.CreateOperationalIntentReferenceResponseSet{Response500: &api.InternalServerErrorBody{ ErrorMessage: *dsserr.Handle(ctx, stacktrace.Propagate(err, "Got an unexpected error"))}} } } - return restapi.CreateOperationalIntentReferenceResponseSet{Response201: respOK} + return restapi.CreateOperationalIntentReferenceResponseSet{Response201: result.Response} } func (a *Server) UpdateOperationalIntentReference(ctx context.Context, req *restapi.UpdateOperationalIntentReferenceRequest, @@ -169,10 +179,20 @@ func (a *Server) UpdateOperationalIntentReference(ctx context.Context, req *rest return restapi.UpdateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(req.BodyParseError, dsserr.BadRequest, "Malformed params"))}} } + validParams, err := actions.ValidateAndReturnOIRUpsertParams(timestamp.MustGetRequestTimestamp(ctx), req.Entityid, req.Ovn, req.Body, a.AllowHTTPBaseUrls) + if err != nil { + return restapi.UpdateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Operational Intent Reference upsert parameters"))}} + } + _, err = actions.CheckUpsertPermissionsAndReturnManager(&req.Auth, validParams.State) + if err != nil { + return restapi.UpdateOperationalIntentReferenceResponseSet{Response403: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.PermissionDenied, "Caller is not allowed to upsert with the requested state"))}} + } - respOK, respConflict, err := a.upsertOperationalIntentReference(ctx, time.Now(), &req.Auth, req.Entityid, req.Ovn, req.Body) + result, err := dssstore.TransactWithResult[repos.Repository, *actions.PutOperationalIntentReferenceResult](ctx, a.Store, req) if err != nil { - err = stacktrace.Propagate(err, "Could not put subscription") + err = stacktrace.Propagate(err, "Could not put Operational Intent Reference") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} switch stacktrace.GetCode(err) { case dsserr.PermissionDenied: @@ -183,195 +203,12 @@ func (a *Server) UpdateOperationalIntentReference(ctx context.Context, req *rest return restapi.UpdateOperationalIntentReferenceResponseSet{Response409: &restapi.AirspaceConflictResponse{ Message: dsserr.Handle(ctx, err)}} case dsserr.MissingOVNs: - return restapi.UpdateOperationalIntentReferenceResponseSet{Response409: respConflict} + return restapi.UpdateOperationalIntentReferenceResponseSet{Response409: result.Conflict} default: return restapi.UpdateOperationalIntentReferenceResponseSet{Response500: &api.InternalServerErrorBody{ ErrorMessage: *dsserr.Handle(ctx, stacktrace.Propagate(err, "Got an unexpected error"))}} } } - return restapi.UpdateOperationalIntentReferenceResponseSet{Response200: respOK} -} - -// 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, now time.Time, authorizedManager *api.AuthorizationResult, entityid restapi.EntityID, ovn restapi.EntityOVN, params *restapi.PutOperationalIntentReferenceParameters, -) (*restapi.ChangeOperationalIntentReferenceResponse, *restapi.AirspaceConflictResponse, error) { - // Note: validateAndReturnOIRUpsertParams and checkUpsertPermissionsAndReturnManager could be moved out of this method and only the valid params passed, - // but this requires some changes in the caller that go beyond the immediate scope of #1088 and can be done later. - validParams, err := actions.ValidateAndReturnOIRUpsertParams(now, entityid, ovn, params, a.AllowHTTPBaseUrls) - if err != nil { - return nil, nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Operational Intent Reference upsert parameters") - } - manager, err := actions.CheckUpsertPermissionsAndReturnManager(authorizedManager, validParams.State) - if err != nil { - return nil, nil, stacktrace.PropagateWithCode(err, dsserr.PermissionDenied, "Caller is not allowed to upsert with the requested state") - } - - var responseOK *restapi.ChangeOperationalIntentReferenceResponse - var responseConflict *restapi.AirspaceConflictResponse - action := func(ctx context.Context, r repos.Repository) (err error) { - - // Get existing OperationalIntent, if any - old, err := r.GetOperationalIntent(ctx, validParams.ID) - if err != nil { - return stacktrace.Propagate(err, "Could not get OperationalIntent from repo") - } - - // Lock subscriptions based on the cell and subscriptions we're going to use - // to reduce the number of retries under concurrent load. - // See issue #1002 for details. - var subscriptionIds = make([]dssmodels.ID, 0) - - if old != nil && old.SubscriptionID != nil { - subscriptionIds = append(subscriptionIds, *old.SubscriptionID) - } - - if !validParams.SubscriptionID.Empty() { - subscriptionIds = append(subscriptionIds, validParams.SubscriptionID) - } - - err = r.LockSubscriptionsOnCells(ctx, validParams.Cells, subscriptionIds, validParams.UExtent.StartTime, validParams.UExtent.EndTime) - if err != nil { - return stacktrace.Propagate(err, "Unable to acquire lock") - } - - // Validate the request against the previous OIR - if err := actions.ValidateUpsertRequestAgainstPreviousOIR(manager, validParams.OVN, old); err != nil { - return stacktrace.PropagateWithCode(err, stacktrace.GetCode(err), "Request validation failed") - } - - var ( - version = scdmodels.VersionNumber(1) - pastOVNs = make([]scdmodels.OVN, 0) - previousSub *scdmodels.Subscription - ) - if old != nil { - version = old.Version + 1 - pastOVNs = append(old.PastOVNs, validParams.OVN) - - // Fetch the previous OIR's subscription if it exists - if old.SubscriptionID != nil { - previousSub, err = r.GetSubscription(ctx, *old.SubscriptionID) - if err != nil { - return stacktrace.Propagate(err, "Unable to get OperationalIntent's Subscription from repo") - } - } - } - - // Determine if the previous subscription is being replaced and if it will need to be cleaned up - previousSubIsBeingReplaced := previousSub != nil && validParams.SubscriptionID != previousSub.ID - removePreviousImplicitSubscription := false - if previousSubIsBeingReplaced { - removePreviousImplicitSubscription, err = actions.SubscriptionIsImplicitAndOnlyAttachedToOIR(ctx, r, validParams.ID, previousSub) - if err != nil { - return stacktrace.Propagate(err, "Could not determine if previous Subscription can be removed") - } - } - - // attachedSub is the subscription that will end up being attached to the OIR - // it defaults to the previous subscription (which may be nil), and may be updated if required by the parameters - attachedSub := previousSub - if validParams.SubscriptionID.Empty() { - // No subscription ID was provided: - // check if an implicit subscription should be created, otherwise do nothing - if validParams.ImplicitSubscription.Requested { - // Parameters for a new implicit subscription have been passed: we will create - // a new implicit subscription even if another subscription was attached to this OIR before, - // regardless of whether it was an implicit subscription or not. - if attachedSub, err = actions.CreateAndStoreNewImplicitSubscription(ctx, r, manager, validParams); err != nil { - return stacktrace.Propagate(err, "Failed to create implicit subscription") - } - } else { - // If no subscription ID is provided and no implicit subscription is requested, - // the OIR should have no attached subscription - attachedSub = nil - } - } else { - // Attempt to rely on the specified subscription - // 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. - // We do the check below in order to avoid re-fetching the subscription if it has not changed - if attachedSub == nil || previousSubIsBeingReplaced { - attachedSub, err = r.GetSubscription(ctx, validParams.SubscriptionID) - if err != nil { - return stacktrace.Propagate(err, "Unable to get requested Subscription from store") - } - if attachedSub == nil { - return stacktrace.NewErrorWithCode(dsserr.BadRequest, "Specified Subscription %s does not exist", validParams.SubscriptionID) - } - } - - // We need to confirm that it is owned by the calling manager - if attachedSub.Manager != manager { - return 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", - validParams.SubscriptionID, - attachedSub.Manager, - manager, - ) - } - - // We need to ensure the subscription covers the OIR's geo-temporal extent - attachedSub, err = actions.EnsureSubscriptionCoversOIR(ctx, r, attachedSub, validParams) - if err != nil { - return stacktrace.Propagate(err, "Failed to ensure subscription covers OIR") - } - } - - if validParams.State.RequiresKey() { - responseConflict, err = actions.ValidateKeyAndProvideConflictResponse(ctx, r, manager, validParams, attachedSub) - if err != nil { - return stacktrace.PropagateWithCode(err, stacktrace.GetCode(err), "Failed to validate key") - } - } - - // Construct the new OperationalIntent - op := validParams.ToOIR(manager, attachedSub, version, pastOVNs) - - // Upsert the OperationalIntent - op, err = r.UpsertOperationalIntent(ctx, op) - if err != nil { - return stacktrace.Propagate(err, "Failed to upsert OperationalIntent in repo") - } - - // Check if the previously attached subscription should be removed - if removePreviousImplicitSubscription { - err = r.DeleteSubscription(ctx, previousSub.ID) - if err != nil { - return stacktrace.Propagate(err, "Unable to delete previous implicit Subscription") - } - } - - notifyVolume, err := actions.ComputeNotificationVolume(old, validParams.UExtent) - if err != nil { - return stacktrace.Propagate(err, "Failed to compute notification volume") - } - - // Notify relevant Subscriptions - subsToNotify, err := r.IncrementNotificationIndicesForOperationalIntents(ctx, notifyVolume) - if err != nil { - return stacktrace.Propagate(err, "Failed to notify relevant Subscriptions") - } - - // Return response to client - responseOK = &restapi.ChangeOperationalIntentReferenceResponse{ - OperationalIntentReference: *op.ToRest(), - Subscribers: makeSubscribersToNotify(subsToNotify), - } - - return nil - } - - _, err = a.Store.Transact(ctx, dssstore.NewFuncOperation(action)) - if err != nil { - return nil, responseConflict, err // No need to Propagate this error as this is not a useful stacktrace line - } - - return responseOK, responseConflict, nil + return restapi.UpdateOperationalIntentReferenceResponseSet{Response200: result.Response} } diff --git a/pkg/scd/server.go b/pkg/scd/server.go index a8eab0aaa..9dea0d488 100644 --- a/pkg/scd/server.go +++ b/pkg/scd/server.go @@ -1,32 +1,9 @@ package scd import ( - restapi "github.com/interuss/dss/pkg/api/scdv1" - scdmodels "github.com/interuss/dss/pkg/scd/models" scdstore "github.com/interuss/dss/pkg/scd/store" ) -func makeSubscribersToNotify(subscriptions []*scdmodels.Subscription) []restapi.SubscriberToNotify { - result := []restapi.SubscriberToNotify{} - - subscriptionsByURL := map[string][]restapi.SubscriptionState{} - for _, sub := range subscriptions { - subState := restapi.SubscriptionState{ - SubscriptionId: restapi.SubscriptionID(sub.ID.String()), - NotificationIndex: restapi.SubscriptionNotificationIndex(sub.NotificationIndex), - } - subscriptionsByURL[sub.USSBaseURL] = append(subscriptionsByURL[sub.USSBaseURL], subState) - } - for url, states := range subscriptionsByURL { - result = append(result, restapi.SubscriberToNotify{ - UssBaseUrl: restapi.SubscriptionUssBaseURL(url), - Subscriptions: states, - }) - } - - return result -} - // Server implements scdv1.Implementation. type Server struct { Store scdstore.Store diff --git a/pkg/store/store.go b/pkg/store/store.go index 4b87b0a67..7102c7e95 100644 --- a/pkg/store/store.go +++ b/pkg/store/store.go @@ -54,17 +54,18 @@ func DecodeJSON[T OperationRequest](buf []byte) (OperationRequest, error) { } // TransactWithResult wraps Store.Transact and casts the result to ResultType, avoiding a cast at every call site. +// The cast is attempted even when Transact returns an error, since some operations intentionally return +// a partial result alongside an error (e.g. a conflict response). func TransactWithResult[R any, ResultType any](ctx context.Context, store Store[R], request OperationRequest) (ResultType, error) { var empty ResultType transactionResult, err := store.Transact(ctx, request) + if resultType, ok := transactionResult.(ResultType); ok { + return resultType, err + } if err != nil { return empty, err } - resultType, ok := transactionResult.(ResultType) - if !ok { - return empty, stacktrace.NewError("unexpected result type %T, want %T", transactionResult, empty) - } - return resultType, nil + return empty, stacktrace.NewError("unexpected result type %T, want %T", transactionResult, empty) } // FuncOperation wraps a closure as an OperationRequest for gradual migration. From 0c04bf8cf71b536f3de8ab60a37dd91ce2ae2ff4 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Wed, 9 Sep 2026 16:50:48 +0200 Subject: [PATCH 02/17] [scd] Return conflict without error --- pkg/errors/errors.go | 4 ---- pkg/scd/actions/operational_intents.go | 16 ++++++++-------- pkg/scd/operational_intents_handler.go | 12 ++++++++---- pkg/store/store.go | 8 +++----- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index f525e3b90..ccf10b2aa 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -16,10 +16,6 @@ const ( // larger than the max area allowed. See geo/s2.go. AreaTooLarge = stacktrace.ErrorCode(iota) - // MissingOVNs is the error to signal that an AirspaceConflictResponse should - // be returned rather than the standard error response. - MissingOVNs - // AlreadyExists is used when attempting to create a resource that already // exists. AlreadyExists diff --git a/pkg/scd/actions/operational_intents.go b/pkg/scd/actions/operational_intents.go index ab94f5a34..29272a649 100644 --- a/pkg/scd/actions/operational_intents.go +++ b/pkg/scd/actions/operational_intents.go @@ -517,7 +517,7 @@ func createAndStoreNewImplicitSubscription(ctx context.Context, r repos.Reposito // 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. +// - If keys are missing, the conflict response will be returned (conflict, nil). // - In case of any other error, (nil, error) will be returned. func validateKeyAndProvideConflictResponse( ctx context.Context, @@ -589,7 +589,7 @@ func validateKeyAndProvideConflictResponse( } } - return responseConflict, stacktrace.NewErrorWithCode(dsserr.MissingOVNs, "Missing OVNs: %v", msg) + return responseConflict, nil } return nil, nil @@ -637,7 +637,7 @@ func ensureSubscriptionCoversOIR(ctx context.Context, r repos.Repository, sub *s } // PutOperationalIntentReferenceResult is the result of an Operational Intent Reference put operation. -// Exactly one of Response or Conflict is set: Conflict is set when the upsert failed because of missing OVNs (a dsserr.MissingOVNs error is returned alongside it) +// Exactly one of Response or Conflict is set: Conflict is set when the upsert failed because of missing OVNs, // and Response is set on success. type PutOperationalIntentReferenceResult struct { Response *restapi.ChangeOperationalIntentReferenceResponse @@ -788,13 +788,13 @@ func ExecutePutOperationalIntentReference(ctx context.Context, repo repos.Reposi } } - var responseConflict *restapi.AirspaceConflictResponse if validParams.State.RequiresKey() { - responseConflict, err = validateKeyAndProvideConflictResponse(ctx, repo, manager, validParams, attachedSub) + responseConflict, err := validateKeyAndProvideConflictResponse(ctx, repo, manager, validParams, attachedSub) if err != nil { - // responseConflict is non-nil here on a dsserr.MissingOVNs error: return it alongside - // the error so the handler can still send it to the client. See the doc comment above. - return &PutOperationalIntentReferenceResult{Conflict: responseConflict}, stacktrace.PropagateWithCode(err, stacktrace.GetCode(err), "Failed to validate key") + return nil, stacktrace.Propagate(err, "Failed to validate key") + } + if responseConflict != nil { + return &PutOperationalIntentReferenceResult{Conflict: responseConflict}, nil } } diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index 2d42659c5..026969992 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -161,14 +161,16 @@ func (a *Server) CreateOperationalIntentReference(ctx context.Context, req *rest case dsserr.VersionMismatch: return restapi.CreateOperationalIntentReferenceResponseSet{Response409: &restapi.AirspaceConflictResponse{ Message: dsserr.Handle(ctx, err)}} - case dsserr.MissingOVNs: - return restapi.CreateOperationalIntentReferenceResponseSet{Response409: result.Conflict} default: return restapi.CreateOperationalIntentReferenceResponseSet{Response500: &api.InternalServerErrorBody{ ErrorMessage: *dsserr.Handle(ctx, stacktrace.Propagate(err, "Got an unexpected error"))}} } } + if result.Conflict != nil { + return restapi.CreateOperationalIntentReferenceResponseSet{Response409: result.Conflict} + } + return restapi.CreateOperationalIntentReferenceResponseSet{Response201: result.Response} } @@ -202,13 +204,15 @@ func (a *Server) UpdateOperationalIntentReference(ctx context.Context, req *rest case dsserr.VersionMismatch: return restapi.UpdateOperationalIntentReferenceResponseSet{Response409: &restapi.AirspaceConflictResponse{ Message: dsserr.Handle(ctx, err)}} - case dsserr.MissingOVNs: - return restapi.UpdateOperationalIntentReferenceResponseSet{Response409: result.Conflict} default: return restapi.UpdateOperationalIntentReferenceResponseSet{Response500: &api.InternalServerErrorBody{ ErrorMessage: *dsserr.Handle(ctx, stacktrace.Propagate(err, "Got an unexpected error"))}} } } + if result.Conflict != nil { + return restapi.UpdateOperationalIntentReferenceResponseSet{Response409: result.Conflict} + } + return restapi.UpdateOperationalIntentReferenceResponseSet{Response200: result.Response} } diff --git a/pkg/store/store.go b/pkg/store/store.go index 7102c7e95..405ae8df2 100644 --- a/pkg/store/store.go +++ b/pkg/store/store.go @@ -54,17 +54,15 @@ func DecodeJSON[T OperationRequest](buf []byte) (OperationRequest, error) { } // TransactWithResult wraps Store.Transact and casts the result to ResultType, avoiding a cast at every call site. -// The cast is attempted even when Transact returns an error, since some operations intentionally return -// a partial result alongside an error (e.g. a conflict response). func TransactWithResult[R any, ResultType any](ctx context.Context, store Store[R], request OperationRequest) (ResultType, error) { var empty ResultType transactionResult, err := store.Transact(ctx, request) - if resultType, ok := transactionResult.(ResultType); ok { - return resultType, err - } if err != nil { return empty, err } + if resultType, ok := transactionResult.(ResultType); ok { + return resultType, nil + } return empty, stacktrace.NewError("unexpected result type %T, want %T", transactionResult, empty) } From 8fe495a142f577dd8ac0a48d1be619b211893957 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 28 Aug 2026 11:10:34 +0200 Subject: [PATCH 03/17] [raftstore] Rename actions packages to operations --- pkg/rid/{actions => operations}/registry.go | 2 +- pkg/rid/{actions => operations}/subscription.go | 2 +- pkg/rid/store/raftstore/store.go | 6 +++--- pkg/rid/store/sqlstore/store.go | 4 ++-- pkg/scd/constraints_handler.go | 4 ++-- pkg/scd/operational_intents_handler.go | 14 +++++++------- pkg/scd/{actions => operations}/availability.go | 2 +- pkg/scd/{actions => operations}/constraint.go | 2 +- .../{actions => operations}/operational_intents.go | 2 +- pkg/scd/{actions => operations}/registry.go | 2 +- pkg/scd/{actions => operations}/subscription.go | 2 +- pkg/scd/store/raftstore/store.go | 6 +++--- pkg/scd/store/sqlstore/store.go | 4 ++-- 13 files changed, 26 insertions(+), 26 deletions(-) rename pkg/rid/{actions => operations}/registry.go (92%) rename pkg/rid/{actions => operations}/subscription.go (99%) rename pkg/scd/{actions => operations}/availability.go (99%) rename pkg/scd/{actions => operations}/constraint.go (99%) rename pkg/scd/{actions => operations}/operational_intents.go (99%) rename pkg/scd/{actions => operations}/registry.go (98%) rename pkg/scd/{actions => operations}/subscription.go (99%) diff --git a/pkg/rid/actions/registry.go b/pkg/rid/operations/registry.go similarity index 92% rename from pkg/rid/actions/registry.go rename to pkg/rid/operations/registry.go index 2cd22af51..989928349 100644 --- a/pkg/rid/actions/registry.go +++ b/pkg/rid/operations/registry.go @@ -1,4 +1,4 @@ -package actions +package operations import ( "github.com/interuss/dss/pkg/rid/repos" diff --git a/pkg/rid/actions/subscription.go b/pkg/rid/operations/subscription.go similarity index 99% rename from pkg/rid/actions/subscription.go rename to pkg/rid/operations/subscription.go index e3537b402..72632ff27 100644 --- a/pkg/rid/actions/subscription.go +++ b/pkg/rid/operations/subscription.go @@ -1,4 +1,4 @@ -package actions +package operations import ( "context" diff --git a/pkg/rid/store/raftstore/store.go b/pkg/rid/store/raftstore/store.go index 6dabdecf7..00b4bf0c5 100644 --- a/pkg/rid/store/raftstore/store.go +++ b/pkg/rid/store/raftstore/store.go @@ -6,7 +6,7 @@ import ( "github.com/interuss/dss/pkg/memstore" "github.com/interuss/dss/pkg/raftstore" "github.com/interuss/dss/pkg/raftstore/consensus" - "github.com/interuss/dss/pkg/rid/actions" + "github.com/interuss/dss/pkg/rid/operations" "github.com/interuss/dss/pkg/rid/repos" ridmemstore "github.com/interuss/dss/pkg/rid/store/memstore" ridraftparams "github.com/interuss/dss/pkg/rid/store/raftstore/params" @@ -33,7 +33,7 @@ func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore. } r := &repo{memStore: memStore, memRepo: memStore.GetRepo()} - store, err := raftstore.Init(ctx, logger.With(zap.String("service", "rid")), locality, params, r, actions.Registry) + store, err := raftstore.Init(ctx, logger.With(zap.String("service", "rid")), locality, params, r, operations.Registry) if err != nil { return nil, stacktrace.Propagate(err, "failed to initialize rid raftstore") } @@ -57,7 +57,7 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err switch proposal.RequestType { default: - handler, ok := actions.Registry[string(proposal.RequestType)] + handler, ok := operations.Registry[string(proposal.RequestType)] if !ok { return nil, stacktrace.NewError("unrecognized request type: %s", proposal.RequestType) } diff --git a/pkg/rid/store/sqlstore/store.go b/pkg/rid/store/sqlstore/store.go index 20bb2e026..e51eb1662 100644 --- a/pkg/rid/store/sqlstore/store.go +++ b/pkg/rid/store/sqlstore/store.go @@ -6,7 +6,7 @@ import ( dssql "github.com/interuss/dss/pkg/sql" "github.com/interuss/dss/pkg/logging" - "github.com/interuss/dss/pkg/rid/actions" + "github.com/interuss/dss/pkg/rid/operations" "github.com/interuss/dss/pkg/rid/repos" "github.com/interuss/dss/pkg/sqlstore" "github.com/interuss/dss/pkg/store/params" @@ -45,6 +45,6 @@ func Init(ctx context.Context, logger *zap.Logger, withCheckCron bool) (*sqlstor timeBasedNotificationIndex: opts.TimeBasedNotificationIndex, } }, - Registry: actions.Registry, + Registry: operations.Registry, }, withCheckCron) } diff --git a/pkg/scd/constraints_handler.go b/pkg/scd/constraints_handler.go index 055502e72..4151855fa 100644 --- a/pkg/scd/constraints_handler.go +++ b/pkg/scd/constraints_handler.go @@ -7,8 +7,8 @@ import ( restapi "github.com/interuss/dss/pkg/api/scdv1" dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" - "github.com/interuss/dss/pkg/scd/actions" scdmodels "github.com/interuss/dss/pkg/scd/models" + "github.com/interuss/dss/pkg/scd/operations" "github.com/interuss/dss/pkg/scd/repos" dssstore "github.com/interuss/dss/pkg/store" "github.com/interuss/dss/pkg/timestamp" @@ -168,7 +168,7 @@ func (a *Server) UpdateConstraintReference(ctx context.Context, req *restapi.Upd // validateConstraintUpsertRequest performs the request validation that can be done ahead of the transaction. // Note that this does NOT check for anything related to access controls: any error returned should be labeled as a dsserr.BadRequest. func validateConstraintUpsertRequest(ctx context.Context, entityid restapi.EntityID, params *restapi.PutConstraintReferenceParameters, allowHTTPBaseUrls bool) error { - _, err := actions.ValidateAndReturnConstraintUpsertParams(timestamp.MustGetRequestTimestamp(ctx), entityid, params) + _, err := operations.ValidateAndReturnConstraintUpsertParams(timestamp.MustGetRequestTimestamp(ctx), entityid, params) if err != nil { return err } diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index 026969992..eba9ff596 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -7,8 +7,8 @@ import ( restapi "github.com/interuss/dss/pkg/api/scdv1" dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" - "github.com/interuss/dss/pkg/scd/actions" scdmodels "github.com/interuss/dss/pkg/scd/models" + "github.com/interuss/dss/pkg/scd/operations" "github.com/interuss/dss/pkg/scd/repos" dssstore "github.com/interuss/dss/pkg/store" "github.com/interuss/dss/pkg/timestamp" @@ -138,18 +138,18 @@ func (a *Server) CreateOperationalIntentReference(ctx context.Context, req *rest return restapi.CreateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(req.BodyParseError, dsserr.BadRequest, "Malformed params"))}} } - validParams, err := actions.ValidateAndReturnOIRUpsertParams(timestamp.MustGetRequestTimestamp(ctx), req.Entityid, "", req.Body, a.AllowHTTPBaseUrls) + validParams, err := operations.ValidateAndReturnOIRUpsertParams(timestamp.MustGetRequestTimestamp(ctx), req.Entityid, "", req.Body, a.AllowHTTPBaseUrls) if err != nil { return restapi.CreateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Operational Intent Reference upsert parameters"))}} } - _, err = actions.CheckUpsertPermissionsAndReturnManager(&req.Auth, validParams.State) + _, err = operations.CheckUpsertPermissionsAndReturnManager(&req.Auth, validParams.State) if err != nil { return restapi.CreateOperationalIntentReferenceResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.PermissionDenied, "Caller is not allowed to upsert with the requested state"))}} } - result, err := dssstore.TransactWithResult[repos.Repository, *actions.PutOperationalIntentReferenceResult](ctx, a.Store, req) + result, err := dssstore.TransactWithResult[repos.Repository, *operations.PutOperationalIntentReferenceResult](ctx, a.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not put Operational Intent Reference") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -181,18 +181,18 @@ func (a *Server) UpdateOperationalIntentReference(ctx context.Context, req *rest return restapi.UpdateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(req.BodyParseError, dsserr.BadRequest, "Malformed params"))}} } - validParams, err := actions.ValidateAndReturnOIRUpsertParams(timestamp.MustGetRequestTimestamp(ctx), req.Entityid, req.Ovn, req.Body, a.AllowHTTPBaseUrls) + validParams, err := operations.ValidateAndReturnOIRUpsertParams(timestamp.MustGetRequestTimestamp(ctx), req.Entityid, req.Ovn, req.Body, a.AllowHTTPBaseUrls) if err != nil { return restapi.UpdateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Operational Intent Reference upsert parameters"))}} } - _, err = actions.CheckUpsertPermissionsAndReturnManager(&req.Auth, validParams.State) + _, err = operations.CheckUpsertPermissionsAndReturnManager(&req.Auth, validParams.State) if err != nil { return restapi.UpdateOperationalIntentReferenceResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.PermissionDenied, "Caller is not allowed to upsert with the requested state"))}} } - result, err := dssstore.TransactWithResult[repos.Repository, *actions.PutOperationalIntentReferenceResult](ctx, a.Store, req) + result, err := dssstore.TransactWithResult[repos.Repository, *operations.PutOperationalIntentReferenceResult](ctx, a.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not put Operational Intent Reference") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} diff --git a/pkg/scd/actions/availability.go b/pkg/scd/operations/availability.go similarity index 99% rename from pkg/scd/actions/availability.go rename to pkg/scd/operations/availability.go index 0581ddf3e..a9d2967f7 100644 --- a/pkg/scd/actions/availability.go +++ b/pkg/scd/operations/availability.go @@ -1,4 +1,4 @@ -package actions +package operations import ( "context" diff --git a/pkg/scd/actions/constraint.go b/pkg/scd/operations/constraint.go similarity index 99% rename from pkg/scd/actions/constraint.go rename to pkg/scd/operations/constraint.go index 262be05f1..2f3f3a3e1 100644 --- a/pkg/scd/actions/constraint.go +++ b/pkg/scd/operations/constraint.go @@ -1,4 +1,4 @@ -package actions +package operations import ( "context" diff --git a/pkg/scd/actions/operational_intents.go b/pkg/scd/operations/operational_intents.go similarity index 99% rename from pkg/scd/actions/operational_intents.go rename to pkg/scd/operations/operational_intents.go index 29272a649..90082834b 100644 --- a/pkg/scd/actions/operational_intents.go +++ b/pkg/scd/operations/operational_intents.go @@ -1,4 +1,4 @@ -package actions +package operations import ( "context" diff --git a/pkg/scd/actions/registry.go b/pkg/scd/operations/registry.go similarity index 98% rename from pkg/scd/actions/registry.go rename to pkg/scd/operations/registry.go index 60961f40a..c339ea535 100644 --- a/pkg/scd/actions/registry.go +++ b/pkg/scd/operations/registry.go @@ -1,4 +1,4 @@ -package actions +package operations import ( restapi "github.com/interuss/dss/pkg/api/scdv1" diff --git a/pkg/scd/actions/subscription.go b/pkg/scd/operations/subscription.go similarity index 99% rename from pkg/scd/actions/subscription.go rename to pkg/scd/operations/subscription.go index 30106ee29..909d5b378 100644 --- a/pkg/scd/actions/subscription.go +++ b/pkg/scd/operations/subscription.go @@ -1,4 +1,4 @@ -package actions +package operations import ( "context" diff --git a/pkg/scd/store/raftstore/store.go b/pkg/scd/store/raftstore/store.go index 918499c25..baa1a11ae 100644 --- a/pkg/scd/store/raftstore/store.go +++ b/pkg/scd/store/raftstore/store.go @@ -6,7 +6,7 @@ import ( "github.com/interuss/dss/pkg/memstore" "github.com/interuss/dss/pkg/raftstore" "github.com/interuss/dss/pkg/raftstore/consensus" - "github.com/interuss/dss/pkg/scd/actions" + "github.com/interuss/dss/pkg/scd/operations" "github.com/interuss/dss/pkg/scd/repos" scdmemstore "github.com/interuss/dss/pkg/scd/store/memstore" scdraftparams "github.com/interuss/dss/pkg/scd/store/raftstore/params" @@ -33,7 +33,7 @@ func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore. } r := &repo{memStore: memStore, memRepo: memStore.GetRepo()} - store, err := raftstore.Init(ctx, logger.With(zap.String("service", "scd")), locality, params, r, actions.Registry) + store, err := raftstore.Init(ctx, logger.With(zap.String("service", "scd")), locality, params, r, operations.Registry) if err != nil { return nil, stacktrace.Propagate(err, "failed to initialize scd raftstore") } @@ -57,7 +57,7 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err switch proposal.RequestType { default: - handler, ok := actions.Registry[string(proposal.RequestType)] + handler, ok := operations.Registry[string(proposal.RequestType)] if !ok { return nil, stacktrace.NewError("unrecognized request type: %s", proposal.RequestType) } diff --git a/pkg/scd/store/sqlstore/store.go b/pkg/scd/store/sqlstore/store.go index 2dd58d4c3..d4a2b510d 100644 --- a/pkg/scd/store/sqlstore/store.go +++ b/pkg/scd/store/sqlstore/store.go @@ -6,7 +6,7 @@ import ( dssql "github.com/interuss/dss/pkg/sql" "github.com/interuss/dss/pkg/logging" - "github.com/interuss/dss/pkg/scd/actions" + "github.com/interuss/dss/pkg/scd/operations" "github.com/interuss/dss/pkg/scd/repos" "github.com/interuss/dss/pkg/sqlstore" "github.com/interuss/dss/pkg/store/params" @@ -51,6 +51,6 @@ func Init(ctx context.Context, logger *zap.Logger, withCheckCron bool) (*sqlstor version: version, } }, - Registry: actions.Registry, + Registry: operations.Registry, }, withCheckCron) } From 49b4f3ac31d59053d3350eec27adb8c04c7612e0 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 28 Aug 2026 11:22:35 +0200 Subject: [PATCH 04/17] [raftstore] Unexport operations --- pkg/rid/operations/subscription.go | 6 +++--- pkg/scd/operations/availability.go | 8 ++++---- pkg/scd/operations/constraint.go | 20 ++++++++++---------- pkg/scd/operations/operational_intents.go | 22 +++++++++++----------- pkg/scd/operations/subscription.go | 18 +++++++++--------- 5 files changed, 37 insertions(+), 37 deletions(-) diff --git a/pkg/rid/operations/subscription.go b/pkg/rid/operations/subscription.go index 72632ff27..dee5fac7e 100644 --- a/pkg/rid/operations/subscription.go +++ b/pkg/rid/operations/subscription.go @@ -16,16 +16,16 @@ func init() { Registry[ridv1.DeleteSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*ridv1.DeleteSubscriptionRequest], - Execute: ExecuteDeleteSubscription, + Execute: executeDeleteSubscription, } Registry[ridv2.DeleteSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*ridv2.DeleteSubscriptionRequest], - Execute: ExecuteDeleteSubscription, + Execute: executeDeleteSubscription, } } -func ExecuteDeleteSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeDeleteSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { var ( rawID string rawVersion string diff --git a/pkg/scd/operations/availability.go b/pkg/scd/operations/availability.go index a9d2967f7..4678b90d0 100644 --- a/pkg/scd/operations/availability.go +++ b/pkg/scd/operations/availability.go @@ -17,17 +17,17 @@ func init() { Registry[restapi.GetUssAvailabilityOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.GetUssAvailabilityRequest], - Execute: ExecuteGetUssAvailability, + Execute: executeGetUssAvailability, IsReadOnly: true, } Registry[restapi.SetUssAvailabilityOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.SetUssAvailabilityRequest], - Execute: ExecuteSetUssAvailability, + Execute: executeSetUssAvailability, } } -func ExecuteGetUssAvailability(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeGetUssAvailability(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.GetUssAvailabilityRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.GetUssAvailabilityOperationID) @@ -55,7 +55,7 @@ func ExecuteGetUssAvailability(ctx context.Context, repo repos.Repository, reque }, nil } -func ExecuteSetUssAvailability(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeSetUssAvailability(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.SetUssAvailabilityRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.SetUssAvailabilityOperationID) diff --git a/pkg/scd/operations/constraint.go b/pkg/scd/operations/constraint.go index 2f3f3a3e1..e1c2202ac 100644 --- a/pkg/scd/operations/constraint.go +++ b/pkg/scd/operations/constraint.go @@ -20,33 +20,33 @@ func init() { Registry[restapi.DeleteConstraintReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.DeleteConstraintReferenceRequest], - Execute: ExecuteDeleteConstraint, + Execute: executeDeleteConstraint, } Registry[restapi.GetConstraintReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.GetConstraintReferenceRequest], - Execute: ExecuteGetConstraint, + Execute: executeGetConstraint, IsReadOnly: true, } Registry[restapi.CreateConstraintReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.CreateConstraintReferenceRequest], - Execute: ExecutePutConstraint, + Execute: executePutConstraint, } Registry[restapi.UpdateConstraintReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.UpdateConstraintReferenceRequest], - Execute: ExecutePutConstraint, + Execute: executePutConstraint, } Registry[restapi.QueryConstraintReferencesOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.QueryConstraintReferencesRequest], - Execute: ExecuteQueryConstraintReferences, + Execute: executeQueryConstraintReferences, IsReadOnly: true, } } -func ExecuteGetConstraint(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeGetConstraint(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.GetConstraintReferenceRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.GetConstraintReferenceOperationID) @@ -75,9 +75,9 @@ func ExecuteGetConstraint(ctx context.Context, repo repos.Repository, request ds }, nil } -// ExecutePutConstraint inserts or updates a Constraint. +// executePutConstraint inserts or updates a Constraint. // If ovn is empty (""), it will attempt to create a new Constraint. -func ExecutePutConstraint(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executePutConstraint(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { var ( manager string entityid restapi.EntityID @@ -230,7 +230,7 @@ func ValidateAndReturnConstraintUpsertParams( return valid, nil } -func ExecuteQueryConstraintReferences(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeQueryConstraintReferences(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.QueryConstraintReferencesRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.QueryConstraintReferencesOperationID) @@ -270,7 +270,7 @@ func ExecuteQueryConstraintReferences(ctx context.Context, repo repos.Repository return response, nil } -func ExecuteDeleteConstraint(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeDeleteConstraint(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.DeleteConstraintReferenceRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.DeleteConstraintReferenceOperationID) diff --git a/pkg/scd/operations/operational_intents.go b/pkg/scd/operations/operational_intents.go index 90082834b..f655b4868 100644 --- a/pkg/scd/operations/operational_intents.go +++ b/pkg/scd/operations/operational_intents.go @@ -22,29 +22,29 @@ func init() { Registry[restapi.GetOperationalIntentReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.GetOperationalIntentReferenceRequest], - Execute: ExecuteGetOperationalIntentReference, + Execute: executeGetOperationalIntentReference, IsReadOnly: true, } Registry[restapi.QueryOperationalIntentReferencesOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.QueryOperationalIntentReferencesRequest], - Execute: ExecuteQueryOperationalIntentReferences, + Execute: executeQueryOperationalIntentReferences, IsReadOnly: true, } Registry[restapi.DeleteOperationalIntentReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.DeleteOperationalIntentReferenceRequest], - Execute: ExecuteDeleteOperationalIntentReference, + Execute: executeDeleteOperationalIntentReference, } Registry[restapi.CreateOperationalIntentReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.CreateOperationalIntentReferenceRequest], - Execute: ExecutePutOperationalIntentReference, + Execute: executePutOperationalIntentReference, } Registry[restapi.UpdateOperationalIntentReferenceOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.UpdateOperationalIntentReferenceRequest], - Execute: ExecutePutOperationalIntentReference, + Execute: executePutOperationalIntentReference, } } @@ -79,9 +79,9 @@ func SubscriptionIsImplicitAndOnlyAttachedToOIR(ctx context.Context, r repos.Rep return false, nil } -// ExecuteDeleteOperationalIntentReference deletes a single operational intent ref for a given ID +// executeDeleteOperationalIntentReference deletes a single operational intent ref for a given ID // at the specified version. -func ExecuteDeleteOperationalIntentReference(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeDeleteOperationalIntentReference(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.DeleteOperationalIntentReferenceRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.DeleteOperationalIntentReferenceOperationID) @@ -182,7 +182,7 @@ func ExecuteDeleteOperationalIntentReference(ctx context.Context, repo repos.Rep }, nil } -func ExecuteGetOperationalIntentReference(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeGetOperationalIntentReference(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.GetOperationalIntentReferenceRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.GetOperationalIntentReferenceOperationID) @@ -210,7 +210,7 @@ func ExecuteGetOperationalIntentReference(ctx context.Context, repo repos.Reposi }, nil } -func ExecuteQueryOperationalIntentReferences(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeQueryOperationalIntentReferences(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.QueryOperationalIntentReferencesRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.QueryOperationalIntentReferencesOperationID) @@ -644,9 +644,9 @@ type PutOperationalIntentReferenceResult struct { Conflict *restapi.AirspaceConflictResponse } -// ExecutePutOperationalIntentReference inserts or updates an Operational Intent. +// executePutOperationalIntentReference inserts or updates an Operational Intent. // If the ovn argument is empty (""), it will attempt to create a new Operational Intent. -func ExecutePutOperationalIntentReference(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executePutOperationalIntentReference(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { var ( entityid restapi.EntityID ovn restapi.EntityOVN diff --git a/pkg/scd/operations/subscription.go b/pkg/scd/operations/subscription.go index 909d5b378..01585719a 100644 --- a/pkg/scd/operations/subscription.go +++ b/pkg/scd/operations/subscription.go @@ -19,33 +19,33 @@ func init() { Registry[restapi.CreateSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.CreateSubscriptionRequest], - Execute: ExecutePutSubscription, + Execute: executePutSubscription, } Registry[restapi.UpdateSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.UpdateSubscriptionRequest], - Execute: ExecutePutSubscription, + Execute: executePutSubscription, } Registry[restapi.DeleteSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.DeleteSubscriptionRequest], - Execute: ExecuteDeleteSubscription, + Execute: executeDeleteSubscription, } Registry[restapi.GetSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.GetSubscriptionRequest], - Execute: ExecuteGetSubscription, + Execute: executeGetSubscription, IsReadOnly: true, } Registry[restapi.QuerySubscriptionsOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, Decode: dssstore.DecodeJSON[*restapi.QuerySubscriptionsRequest], - Execute: ExecuteQuerySubscriptions, + Execute: executeQuerySubscriptions, IsReadOnly: true, } } -func ExecutePutSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executePutSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { var ( manager string subscriptionid restapi.SubscriptionID @@ -253,7 +253,7 @@ func getOperations(ctx context.Context, r repos.Repository, opIDs []dssmodels.ID return res, nil } -func ExecuteDeleteSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeDeleteSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.DeleteSubscriptionRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.DeleteSubscriptionOperationID) @@ -305,7 +305,7 @@ func ExecuteDeleteSubscription(ctx context.Context, repo repos.Repository, reque return &restapi.DeleteSubscriptionResponse{Subscription: *p}, nil } -func ExecuteGetSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeGetSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.GetSubscriptionRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.GetSubscriptionOperationID) @@ -349,7 +349,7 @@ func ExecuteGetSubscription(ctx context.Context, repo repos.Repository, request return &restapi.GetSubscriptionResponse{Subscription: *p}, nil } -func ExecuteQuerySubscriptions(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { +func executeQuerySubscriptions(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { req, ok := request.(*restapi.QuerySubscriptionsRequest) if !ok { return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.QuerySubscriptionsOperationID) From e724a41e4064969cab2e3f18ba2c99da575792e2 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 28 Aug 2026 13:09:19 +0200 Subject: [PATCH 05/17] [raftstore] Rename context methods --- cmds/core-service/main.go | 4 +-- pkg/aux_/pool_participants.go | 2 +- pkg/aux_/store/memstore/dss.go | 2 +- pkg/aux_/store/memstore/dss_test.go | 8 +++--- pkg/aux_/store/memstore/snapshot_test.go | 4 +-- pkg/aux_/store/memstore/store_test.go | 4 +-- pkg/locality/locality.go | 20 +++++++------- pkg/raftstore/consensus/proposal.go | 2 +- pkg/raftstore/store.go | 4 +-- .../memstore/identification_service_area.go | 4 +-- .../identification_service_area_test.go | 16 ++++++------ pkg/rid/store/memstore/snapshot_test.go | 4 +-- pkg/rid/store/memstore/store_test.go | 6 ++--- pkg/rid/store/memstore/subscriptions.go | 10 +++---- pkg/rid/store/memstore/subscriptions_test.go | 24 ++++++++--------- pkg/scd/constraints_handler.go | 2 +- pkg/scd/operational_intents_handler.go | 4 +-- pkg/scd/operations/constraint.go | 2 +- pkg/scd/operations/operational_intents.go | 2 +- pkg/scd/operations/subscription.go | 4 +-- pkg/scd/store/memstore/availability.go | 2 +- pkg/scd/store/memstore/constraints.go | 2 +- pkg/scd/store/memstore/operational_intents.go | 2 +- pkg/scd/store/memstore/store_test.go | 2 +- pkg/scd/store/memstore/subscriptions.go | 2 +- pkg/timestamp/timestamp.go | 26 +++++++++---------- 26 files changed, 82 insertions(+), 82 deletions(-) diff --git a/cmds/core-service/main.go b/cmds/core-service/main.go index 6b4026fda..91ee224b1 100644 --- a/cmds/core-service/main.go +++ b/cmds/core-service/main.go @@ -368,9 +368,9 @@ func RunHTTPServer(ctx context.Context, ctxCanceler func(), address, locality st handler = authorizer.TokenMiddleware(handler) handler = http.TimeoutHandler(handler, *timeout, "request timeout") handler = logging.HTTPMiddleware(logger, *dumpRequests, handler) - handler = timestamp.RequestTimestampMiddleware(handler) + handler = timestamp.Middleware(handler) handler = random.Middleware(handler) - handler = requestlocality.LocalityMiddleware(locality)(handler) + handler = requestlocality.Middleware(locality)(handler) if *enableMetrics || *enableTracing { // We use the default settings; the APIRouter handler will override the span value accordingly, as it has more information. diff --git a/pkg/aux_/pool_participants.go b/pkg/aux_/pool_participants.go index 8a00c9f1f..dae417936 100644 --- a/pkg/aux_/pool_participants.go +++ b/pkg/aux_/pool_participants.go @@ -96,7 +96,7 @@ func (a *Server) PutDSSInstancesHeartbeat(ctx context.Context, req *restapi.PutD } heartbeat.Timestamp = &ts } else { - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) heartbeat.Timestamp = &now } diff --git a/pkg/aux_/store/memstore/dss.go b/pkg/aux_/store/memstore/dss.go index ef90996f3..359beec9e 100644 --- a/pkg/aux_/store/memstore/dss.go +++ b/pkg/aux_/store/memstore/dss.go @@ -11,7 +11,7 @@ import ( ) func (r *repo) SaveOwnMetadata(ctx context.Context, loc string, publicEndpoint string) error { - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) r.state.Participants[locality(loc)] = &participant{ PublicEndpoint: publicEndpoint, diff --git a/pkg/aux_/store/memstore/dss_test.go b/pkg/aux_/store/memstore/dss_test.go index f39bcb248..4c9a730a4 100644 --- a/pkg/aux_/store/memstore/dss_test.go +++ b/pkg/aux_/store/memstore/dss_test.go @@ -15,7 +15,7 @@ var fakeClock = clockwork.NewFakeClock() func TestSaveOwnMetadataRoundTrip(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) r := newRepo() require.NoError(t, r.SaveOwnMetadata(ctx, "dss-1", "https://example.com")) @@ -35,7 +35,7 @@ func TestSaveOwnMetadataRoundTrip(t *testing.T) { func TestSaveOwnMetadataUpsert(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) r := newRepo() require.NoError(t, r.SaveOwnMetadata(ctx, "dss-1", "https://old.example.com")) @@ -50,7 +50,7 @@ func TestSaveOwnMetadataUpsert(t *testing.T) { func TestGetDSSMetadataPicksLatestHeartbeat(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) r := newRepo() require.NoError(t, r.SaveOwnMetadata(ctx, "dss-1", "https://example.com")) @@ -71,7 +71,7 @@ func TestGetDSSMetadataPicksLatestHeartbeat(t *testing.T) { func TestGetDSSMetadataUpdatesHeartbeatPerSource(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) r := newRepo() require.NoError(t, r.SaveOwnMetadata(ctx, "dss-1", "https://example.com")) diff --git a/pkg/aux_/store/memstore/snapshot_test.go b/pkg/aux_/store/memstore/snapshot_test.go index cd2f03a9f..1ff9015b4 100644 --- a/pkg/aux_/store/memstore/snapshot_test.go +++ b/pkg/aux_/store/memstore/snapshot_test.go @@ -17,7 +17,7 @@ import ( func TestSnapshotRoundTrip(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) src := newRepo() require.NoError(t, src.SaveOwnMetadata(ctx, "dss-1", "https://example.com")) ts := time.Now().UTC() @@ -40,7 +40,7 @@ func TestSnapshotRoundTrip(t *testing.T) { func TestRestoreFromSnapshotReplacesState(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) src := newRepo() require.NoError(t, src.SaveOwnMetadata(ctx, "dss-1", "https://example.com")) data, err := src.GetSnapshot() diff --git a/pkg/aux_/store/memstore/store_test.go b/pkg/aux_/store/memstore/store_test.go index 1526efb04..70376edb3 100644 --- a/pkg/aux_/store/memstore/store_test.go +++ b/pkg/aux_/store/memstore/store_test.go @@ -10,7 +10,7 @@ import ( func TestCheckpointRestore(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) r := newRepo() @@ -34,7 +34,7 @@ func TestCheckpointRestore(t *testing.T) { func TestCheckpointIsolatesUpsert(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) r := newRepo() require.NoError(t, r.SaveOwnMetadata(ctx, "dss-1", "https://old.example.com")) diff --git a/pkg/locality/locality.go b/pkg/locality/locality.go index 8ea51faa8..6e1454c85 100644 --- a/pkg/locality/locality.go +++ b/pkg/locality/locality.go @@ -7,12 +7,12 @@ import ( "github.com/interuss/stacktrace" ) -type localityKey struct{} +type key struct{} -// MustGetRequestLocality returns the request locality from the context and panics if it is not +// MustFromContext returns the request locality from the context and panics if it is not // present, which is a programming error. -func MustGetRequestLocality(ctx context.Context) string { - locality, ok := ctx.Value(localityKey{}).(string) +func MustFromContext(ctx context.Context) string { + locality, ok := ctx.Value(key{}).(string) if !ok { panic(stacktrace.NewError("request locality not present in context")) } @@ -20,17 +20,17 @@ func MustGetRequestLocality(ctx context.Context) string { return locality } -// WithRequestLocality returns a new context with the given locality. -func WithRequestLocality(ctx context.Context, locality string) context.Context { - return context.WithValue(ctx, localityKey{}, locality) +// NewContext returns a new context with the given locality. +func NewContext(ctx context.Context, locality string) context.Context { + return context.WithValue(ctx, key{}, locality) } -// LocalityMiddleware is an HTTP middleware that stamps each incoming request with this +// Middleware is an HTTP middleware that stamps each incoming request with this // DSS instance's locality so that locality-dependent operations execute deterministically across nodes. -func LocalityMiddleware(locality string) func(http.Handler) http.Handler { +func Middleware(locality string) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - next.ServeHTTP(w, r.WithContext(WithRequestLocality(r.Context(), locality))) + next.ServeHTTP(w, r.WithContext(NewContext(r.Context(), locality))) }) } } diff --git a/pkg/raftstore/consensus/proposal.go b/pkg/raftstore/consensus/proposal.go index bb63fda74..332a842e8 100644 --- a/pkg/raftstore/consensus/proposal.go +++ b/pkg/raftstore/consensus/proposal.go @@ -35,7 +35,7 @@ type Proposal struct { } func (c *Consensus) newProposal(ctx context.Context, requestType RequestType, value []byte, readOnly bool) Proposal { - timestamp := timestamp.MustGetRequestTimestamp(ctx) + timestamp := timestamp.MustFromContext(ctx) seed := random.MustFromContext(ctx) return Proposal{ diff --git a/pkg/raftstore/store.go b/pkg/raftstore/store.go index a17c00827..b3c6919aa 100644 --- a/pkg/raftstore/store.go +++ b/pkg/raftstore/store.go @@ -116,8 +116,8 @@ func (s *Store[R]) processCommits(ctx context.Context, commitCh <-chan consensus continue } - proposalCtx := timestamp.WithRequestTimestamp(ctx, commit.Prop.Timestamp) - proposalCtx = locality.WithRequestLocality(proposalCtx, commit.Prop.Locality) + proposalCtx := timestamp.NewContext(ctx, commit.Prop.Timestamp) + proposalCtx = locality.NewContext(proposalCtx, commit.Prop.Locality) proposalCtx = random.NewContext(proposalCtx, commit.Prop.Seed) result, err := s.raftRepo.Apply(proposalCtx, commit.Prop) commit.Done <- consensus.ProposalResult{Result: result, Error: err} diff --git a/pkg/rid/store/memstore/identification_service_area.go b/pkg/rid/store/memstore/identification_service_area.go index bf7bb042b..02976cc69 100644 --- a/pkg/rid/store/memstore/identification_service_area.go +++ b/pkg/rid/store/memstore/identification_service_area.go @@ -61,7 +61,7 @@ func (r *repo) InsertISA(ctx context.Context, isa *ridmodels.IdentificationServi return nil, stacktrace.NewError("ISA with id %s already exists", isa.ID) } - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) rec := isaRecordFromModel(isa, now) r.state.ISAs[isa.ID] = rec @@ -77,7 +77,7 @@ func (r *repo) UpdateISA(ctx context.Context, isa *ridmodels.IdentificationServi return nil, nil } - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) rec := isaRecordFromModel(isa, now) rec.Owner = prev.Owner // It's not possible to update the owner of an ISA, this ensure it's to changed to a new value. diff --git a/pkg/rid/store/memstore/identification_service_area_test.go b/pkg/rid/store/memstore/identification_service_area_test.go index f6baa8dbf..a9bb4b34f 100644 --- a/pkg/rid/store/memstore/identification_service_area_test.go +++ b/pkg/rid/store/memstore/identification_service_area_test.go @@ -34,7 +34,7 @@ var ( func TestStoreSearchISAs(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) cells := s2.CellUnion{ s2.CellID(17106221850767130624), s2.CellID(17106221885126868992), @@ -137,7 +137,7 @@ func TestStoreSearchISAs(t *testing.T) { func TestBadVersion(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) saOut1, err := repo.InsertISA(ctx, serviceArea) @@ -159,7 +159,7 @@ func TestBadVersion(t *testing.T) { func TestStoreExpiredISA(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) saOut, err := repo.InsertISA(ctx, serviceArea) @@ -194,7 +194,7 @@ func TestStoreExpiredISA(t *testing.T) { func TestStoreDeleteISAs(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) // Insert the ISA. @@ -215,7 +215,7 @@ func TestStoreDeleteISAs(t *testing.T) { func TestStoreISAWithNoGeoData(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) endTime := fakeClock.Now().Add(24 * time.Hour) @@ -230,7 +230,7 @@ func TestStoreISAWithNoGeoData(t *testing.T) { func TestListExpiredISAs(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) // Insert ISA with endtime 1 day from now @@ -261,7 +261,7 @@ func TestListExpiredISAs(t *testing.T) { func TestListExpiredISAsWithEmptyWriter(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) // Insert ISA with endtime 1 day from now @@ -294,7 +294,7 @@ func TestListExpiredISAsWithEmptyWriter(t *testing.T) { func TestStoreCountISAs(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) // Insert the ISA. diff --git a/pkg/rid/store/memstore/snapshot_test.go b/pkg/rid/store/memstore/snapshot_test.go index a795d5f5f..50fc5423d 100644 --- a/pkg/rid/store/memstore/snapshot_test.go +++ b/pkg/rid/store/memstore/snapshot_test.go @@ -15,7 +15,7 @@ import ( func TestSnapshotRoundTrip(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) src := setUpStore(t) _, err := src.InsertISA(ctx, serviceArea) require.NoError(t, err) @@ -49,7 +49,7 @@ func TestSnapshotRoundTrip(t *testing.T) { func TestRestoreFromSnapshotReplacesState(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) src := setUpStore(t) _, err := src.InsertISA(ctx, serviceArea) require.NoError(t, err) diff --git a/pkg/rid/store/memstore/store_test.go b/pkg/rid/store/memstore/store_test.go index 62752a537..55d13081f 100644 --- a/pkg/rid/store/memstore/store_test.go +++ b/pkg/rid/store/memstore/store_test.go @@ -30,7 +30,7 @@ func setUpStore(t *testing.T) *repo { func TestDatabaseEnsuresBeginsBeforeExpires(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) var ( @@ -50,7 +50,7 @@ func TestDatabaseEnsuresBeginsBeforeExpires(t *testing.T) { func TestCheckpointRestoreISA(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) _, err := repo.InsertISA(ctx, serviceArea) @@ -76,7 +76,7 @@ func TestCheckpointRestoreISA(t *testing.T) { func TestCheckpointIsolatesNotificationIndex(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) sub, err := repo.InsertSubscription(ctx, subscriptionsPool[0].input) diff --git a/pkg/rid/store/memstore/subscriptions.go b/pkg/rid/store/memstore/subscriptions.go index 63cb22c09..f5a7bffdb 100644 --- a/pkg/rid/store/memstore/subscriptions.go +++ b/pkg/rid/store/memstore/subscriptions.go @@ -67,7 +67,7 @@ func (r *repo) InsertSubscription(ctx context.Context, s *ridmodels.Subscription return nil, stacktrace.NewError("Subscription with id %s already exists", s.ID) } - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) rec := subRecordFromModel(s, now) r.state.Subscriptions[s.ID] = rec @@ -83,7 +83,7 @@ func (r *repo) UpdateSubscription(ctx context.Context, s *ridmodels.Subscription return nil, nil } - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) rec := subRecordFromModel(s, now) rec.Owner = prev.Owner // It's not possible to update the owner of a subscription, this ensure it's to changed to a new value. @@ -137,7 +137,7 @@ func (r *repo) searchSubscriptions(ctx context.Context, cells s2.CellUnion, owne return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "no location provided") } - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) var out []*ridmodels.Subscription for rec := range r.liveSubscriptionsInCells(now, cells, owner) { @@ -154,7 +154,7 @@ func (r *repo) searchSubscriptions(ctx context.Context, cells s2.CellUnion, owne // subscription in the given cells. func (r *repo) UpdateNotificationIdxsInCells(ctx context.Context, cells s2.CellUnion) ([]*ridmodels.Subscription, error) { - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) var out []*ridmodels.Subscription for rec := range r.liveSubscriptionsInCells(now, cells, nil) { @@ -166,7 +166,7 @@ func (r *repo) UpdateNotificationIdxsInCells(ctx context.Context, cells s2.CellU func (r *repo) MaxSubscriptionCountInCellsByOwner(ctx context.Context, cells s2.CellUnion, owner dssmodels.Owner) (int, error) { - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) want := cellSet(cells) counts := make(map[s2.CellID]int, len(cells)) diff --git a/pkg/rid/store/memstore/subscriptions_test.go b/pkg/rid/store/memstore/subscriptions_test.go index 1d61cf707..993aaeff8 100644 --- a/pkg/rid/store/memstore/subscriptions_test.go +++ b/pkg/rid/store/memstore/subscriptions_test.go @@ -70,7 +70,7 @@ var ( func TestStoreGetSubscription(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) for _, r := range subscriptionsPool { @@ -90,7 +90,7 @@ func TestStoreGetSubscription(t *testing.T) { func TestStoreInsertSubscription(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) for _, r := range subscriptionsPool { @@ -134,7 +134,7 @@ func TestStoreInsertSubscription(t *testing.T) { func TestStoreDeleteSubscription(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) for _, r := range subscriptionsPool { @@ -162,7 +162,7 @@ func TestStoreDeleteSubscription(t *testing.T) { func TestStoreSearchSubscription(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now().UTC()) + ctx = timestamp.NewContext(ctx, fakeClock.Now().UTC()) repo := setUpStore(t) var ( @@ -207,7 +207,7 @@ func TestStoreSearchSubscription(t *testing.T) { func TestStoreExpiredSubscription(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) endTime := fakeClock.Now().Add(24 * time.Hour) @@ -221,7 +221,7 @@ func TestStoreExpiredSubscription(t *testing.T) { require.NoError(t, err) // The subscription's endTime is 24 hours from now. - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now().Add(23*time.Hour)) + ctx = timestamp.NewContext(ctx, fakeClock.Now().Add(23*time.Hour)) // We should still be able to find the subscription by searching and by ID. subs, err := repo.SearchSubscriptionsByOwner(ctx, sub.Cells, "original owner") @@ -233,7 +233,7 @@ func TestStoreExpiredSubscription(t *testing.T) { require.NotNil(t, &ret) // But now the subscription has expired. - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now().Add(25*time.Hour)) + ctx = timestamp.NewContext(ctx, fakeClock.Now().Add(25*time.Hour)) subs, err = repo.SearchSubscriptionsByOwner(ctx, sub.Cells, "original owner") require.NoError(t, err) @@ -246,7 +246,7 @@ func TestStoreExpiredSubscription(t *testing.T) { func TestStoreSubscriptionWithNoGeoData(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) endTime := fakeClock.Now().Add(24 * time.Hour) @@ -261,7 +261,7 @@ func TestStoreSubscriptionWithNoGeoData(t *testing.T) { func TestMaxSubscriptionCountInCellsByOwner(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) for _, s := range subscriptionsPool { @@ -276,7 +276,7 @@ func TestMaxSubscriptionCountInCellsByOwner(t *testing.T) { func TestListExpiredSubscriptions(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) fakeClock := clockwork.NewFakeClockAt(time.Now()) @@ -309,7 +309,7 @@ func TestListExpiredSubscriptions(t *testing.T) { func TestListExpiredSubscriptionsWithEmptyWriter(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) // Insert Subscription with endtime 1 day from now @@ -342,7 +342,7 @@ func TestListExpiredSubscriptionsWithEmptyWriter(t *testing.T) { func TestStoreCountSubscription(t *testing.T) { ctx := context.Background() - ctx = timestamp.WithRequestTimestamp(ctx, fakeClock.Now()) + ctx = timestamp.NewContext(ctx, fakeClock.Now()) repo := setUpStore(t) for _, r := range subscriptionsPool { diff --git a/pkg/scd/constraints_handler.go b/pkg/scd/constraints_handler.go index 4151855fa..692a5405c 100644 --- a/pkg/scd/constraints_handler.go +++ b/pkg/scd/constraints_handler.go @@ -168,7 +168,7 @@ func (a *Server) UpdateConstraintReference(ctx context.Context, req *restapi.Upd // validateConstraintUpsertRequest performs the request validation that can be done ahead of the transaction. // Note that this does NOT check for anything related to access controls: any error returned should be labeled as a dsserr.BadRequest. func validateConstraintUpsertRequest(ctx context.Context, entityid restapi.EntityID, params *restapi.PutConstraintReferenceParameters, allowHTTPBaseUrls bool) error { - _, err := operations.ValidateAndReturnConstraintUpsertParams(timestamp.MustGetRequestTimestamp(ctx), entityid, params) + _, err := operations.ValidateAndReturnConstraintUpsertParams(timestamp.MustFromContext(ctx), entityid, params) if err != nil { return err } diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index eba9ff596..8e00f28f9 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -138,7 +138,7 @@ func (a *Server) CreateOperationalIntentReference(ctx context.Context, req *rest return restapi.CreateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(req.BodyParseError, dsserr.BadRequest, "Malformed params"))}} } - validParams, err := operations.ValidateAndReturnOIRUpsertParams(timestamp.MustGetRequestTimestamp(ctx), req.Entityid, "", req.Body, a.AllowHTTPBaseUrls) + validParams, err := operations.ValidateAndReturnOIRUpsertParams(timestamp.MustFromContext(ctx), req.Entityid, "", req.Body, a.AllowHTTPBaseUrls) if err != nil { return restapi.CreateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Operational Intent Reference upsert parameters"))}} @@ -181,7 +181,7 @@ func (a *Server) UpdateOperationalIntentReference(ctx context.Context, req *rest return restapi.UpdateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(req.BodyParseError, dsserr.BadRequest, "Malformed params"))}} } - validParams, err := operations.ValidateAndReturnOIRUpsertParams(timestamp.MustGetRequestTimestamp(ctx), req.Entityid, req.Ovn, req.Body, a.AllowHTTPBaseUrls) + validParams, err := operations.ValidateAndReturnOIRUpsertParams(timestamp.MustFromContext(ctx), req.Entityid, req.Ovn, req.Body, a.AllowHTTPBaseUrls) if err != nil { return restapi.UpdateOperationalIntentReferenceResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Operational Intent Reference upsert parameters"))}} diff --git a/pkg/scd/operations/constraint.go b/pkg/scd/operations/constraint.go index e1c2202ac..7b32dd7f5 100644 --- a/pkg/scd/operations/constraint.go +++ b/pkg/scd/operations/constraint.go @@ -94,7 +94,7 @@ func executePutConstraint(ctx context.Context, repo repos.Repository, request ds return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.CreateConstraintReferenceOperationID) } - validParams, err := ValidateAndReturnConstraintUpsertParams(timestamp.MustGetRequestTimestamp(ctx), entityid, params) + validParams, err := ValidateAndReturnConstraintUpsertParams(timestamp.MustFromContext(ctx), entityid, params) if err != nil { return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Constraint upsert parameters") } diff --git a/pkg/scd/operations/operational_intents.go b/pkg/scd/operations/operational_intents.go index f655b4868..76216c814 100644 --- a/pkg/scd/operations/operational_intents.go +++ b/pkg/scd/operations/operational_intents.go @@ -663,7 +663,7 @@ func executePutOperationalIntentReference(ctx context.Context, repo repos.Reposi return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, restapi.CreateOperationalIntentReferenceOperationID) } - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) // Base URL scheme validation is a pre-flight, request-only check performed by the handler // before this action is proposed for consensus; skip it here (allowHTTPBaseUrls: true). diff --git a/pkg/scd/operations/subscription.go b/pkg/scd/operations/subscription.go index 01585719a..a4cb4c989 100644 --- a/pkg/scd/operations/subscription.go +++ b/pkg/scd/operations/subscription.go @@ -119,7 +119,7 @@ func executePutSubscription(ctx context.Context, repo repos.Repository, request } // Validate and perhaps correct StartTime and EndTime. - if err := subreq.AdjustTimeRange(timestamp.MustGetRequestTimestamp(ctx), old); err != nil { + if err := subreq.AdjustTimeRange(timestamp.MustFromContext(ctx), old); err != nil { return nil, stacktrace.Propagate(err, "Error adjusting time range of Subscription") } @@ -373,7 +373,7 @@ func executeQuerySubscriptions(ctx context.Context, repo repos.Repository, reque return nil, stacktrace.Propagate(err, "Error searching Subscriptions in repo") } - nowMarker := timestamp.MustGetRequestTimestamp(ctx) + nowMarker := timestamp.MustFromContext(ctx) // Return response to client response := &restapi.QuerySubscriptionsResponse{ diff --git a/pkg/scd/store/memstore/availability.go b/pkg/scd/store/memstore/availability.go index bb750996a..8fe0dc839 100644 --- a/pkg/scd/store/memstore/availability.go +++ b/pkg/scd/store/memstore/availability.go @@ -26,7 +26,7 @@ func (r *repo) GetUssAvailability(_ context.Context, id dssmodels.Manager) (*scd } func (r *repo) UpsertUssAvailability(ctx context.Context, s *scdmodels.UssAvailabilityStatus) (*scdmodels.UssAvailabilityStatus, error) { - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) rec := &availabilityRecord{ Uss: s.Uss, diff --git a/pkg/scd/store/memstore/constraints.go b/pkg/scd/store/memstore/constraints.go index 6fb54f143..33c9a0b6a 100644 --- a/pkg/scd/store/memstore/constraints.go +++ b/pkg/scd/store/memstore/constraints.go @@ -67,7 +67,7 @@ func (r *repo) UpsertConstraint(ctx context.Context, s *scdmodels.Constraint) (* return nil, stacktrace.Propagate(err, "Failed to convert array to jackc/pgtype") } - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) rec := &constraintRecord{ ID: s.ID, diff --git a/pkg/scd/store/memstore/operational_intents.go b/pkg/scd/store/memstore/operational_intents.go index fad0deec2..0abbab93f 100644 --- a/pkg/scd/store/memstore/operational_intents.go +++ b/pkg/scd/store/memstore/operational_intents.go @@ -98,7 +98,7 @@ func (r *repo) UpsertOperationalIntent(ctx context.Context, operation *scdmodels ussRequestedOVN = operation.OVN.String() } - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) rec := &operationalIntentRecord{ ID: operation.ID, diff --git a/pkg/scd/store/memstore/store_test.go b/pkg/scd/store/memstore/store_test.go index 7f43bdf8c..caecd5bc6 100644 --- a/pkg/scd/store/memstore/store_test.go +++ b/pkg/scd/store/memstore/store_test.go @@ -40,7 +40,7 @@ func setUpStore(t *testing.T) *repo { // writeCtx returns a context carrying a deterministic write timestamp so that // updated_at is controlled in tests. func writeCtx() context.Context { - return timestamp.WithRequestTimestamp(context.Background(), writeTime) + return timestamp.NewContext(context.Background(), writeTime) } func sampleConstraint() *scdmodels.Constraint { diff --git a/pkg/scd/store/memstore/subscriptions.go b/pkg/scd/store/memstore/subscriptions.go index 02f233698..850d06125 100644 --- a/pkg/scd/store/memstore/subscriptions.go +++ b/pkg/scd/store/memstore/subscriptions.go @@ -78,7 +78,7 @@ func (r *repo) GetSubscription(_ context.Context, id dssmodels.ID) (*scdmodels.S } func (r *repo) UpsertSubscription(ctx context.Context, s *scdmodels.Subscription) (*scdmodels.Subscription, error) { - now := timestamp.MustGetRequestTimestamp(ctx) + now := timestamp.MustFromContext(ctx) rec := &subscriptionRecord{ ID: s.ID, diff --git a/pkg/timestamp/timestamp.go b/pkg/timestamp/timestamp.go index 28dbe9451..fe1329d46 100644 --- a/pkg/timestamp/timestamp.go +++ b/pkg/timestamp/timestamp.go @@ -8,13 +8,13 @@ import ( "github.com/interuss/stacktrace" ) -type timestampKey struct{} +type key struct{} -// requestTimestampFromContext returns the request timestamp from the context, or an error if the value is not present or if it is zero. +// fromContext returns the request timestamp from the context, or an error if the value is not present or if it is zero. // The timestamp is set by the Middleware when a query is received then (on the receiver side) by the Raftstore when the query is applied. // It is then used for deterministic execution of time-dependent queries. -func requestTimestampFromContext(ctx context.Context) (time.Time, error) { - timestamp, ok := ctx.Value(timestampKey{}).(time.Time) +func fromContext(ctx context.Context) (time.Time, error) { + timestamp, ok := ctx.Value(key{}).(time.Time) if !ok { return time.Time{}, stacktrace.NewError("timestamp not found in context") } @@ -26,10 +26,10 @@ func requestTimestampFromContext(ctx context.Context) (time.Time, error) { return timestamp, nil } -// MustGetRequestTimestamp returns the request timestamp from the context and panics if it is not +// MustFromContext returns the request timestamp from the context and panics if it is not // present or invalid, which is a programming error. -func MustGetRequestTimestamp(ctx context.Context) time.Time { - timestamp, err := requestTimestampFromContext(ctx) +func MustFromContext(ctx context.Context) time.Time { + timestamp, err := fromContext(ctx) if err != nil { panic(err) } @@ -37,18 +37,18 @@ func MustGetRequestTimestamp(ctx context.Context) time.Time { return timestamp } -// WithRequestTimestamp returns a new context with the given timestamp. -func WithRequestTimestamp(ctx context.Context, timestamp time.Time) context.Context { - return context.WithValue(ctx, timestampKey{}, timestamp) +// NewContext returns a new context with the given timestamp. +func NewContext(ctx context.Context, timestamp time.Time) context.Context { + return context.WithValue(ctx, key{}, timestamp) } -// RequestTimestampMiddleware is an HTTP middleware that stamps each incoming +// Middleware is an HTTP middleware that stamps each incoming // request with its received time. This timestamp is later used as the // timestamp of the Raft proposal, so that time-dependent queries // execute deterministically across nodes and contexts (catchup / restart etc.). -func RequestTimestampMiddleware(next http.Handler) http.Handler { +func Middleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := WithRequestTimestamp(r.Context(), time.Now()) + ctx := NewContext(r.Context(), time.Now()) next.ServeHTTP(w, r.WithContext(ctx)) }) } From cf95e9c6a50c532cf9f2942c2204d186ddd1dba4 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Wed, 19 Aug 2026 13:51:12 +0200 Subject: [PATCH 06/17] [raftstore] Embed memstore and use checkpoint --- pkg/aux_/store/raftstore/store.go | 19 +++++-------------- pkg/raftstore/store.go | 17 ++++++++--------- pkg/rid/store/raftstore/store.go | 15 +++------------ pkg/scd/store/raftstore/store.go | 15 +++------------ 4 files changed, 19 insertions(+), 47 deletions(-) diff --git a/pkg/aux_/store/raftstore/store.go b/pkg/aux_/store/raftstore/store.go index 8f8a4270f..7b55231ed 100644 --- a/pkg/aux_/store/raftstore/store.go +++ b/pkg/aux_/store/raftstore/store.go @@ -24,8 +24,7 @@ const ( // repo is a full implementation of aux_.repos.Repository for Raft-based storage. type repo struct { consensus *consensus.Consensus - memStore *memstore.Store[repos.Repository] - memRepo repos.Repository + *memstore.Store[repos.Repository] } func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore.Store[repos.Repository], error) { @@ -39,7 +38,7 @@ func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore. return nil, stacktrace.Propagate(err, "failed to initialize aux memstore") } - r := &repo{memStore: memStore, memRepo: memStore.GetRepo()} + r := &repo{Store: memStore} store, err := raftstore.Init(ctx, logger.With(zap.String("service", "aux_")), locality, params, r, nil) if err != nil { return nil, stacktrace.Propagate(err, "failed to initialize aux raftstore") @@ -52,14 +51,6 @@ func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore. func (r *repo) GetRepo() repos.Repository { return r } -func (r *repo) GetSnapshot() ([]byte, error) { - return r.memStore.GetSnapshot() -} - -func (r *repo) RestoreFromSnapshot(data []byte) error { - return r.memStore.RestoreFromSnapshot(data) -} - func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, error) { switch proposal.RequestType { case saveOwnMetadata: @@ -68,10 +59,10 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", saveOwnMetadata) } - return nil, r.memRepo.SaveOwnMetadata(ctx, payload.Locality, payload.PublicEndpoint) + return nil, r.Store.GetRepo().SaveOwnMetadata(ctx, payload.Locality, payload.PublicEndpoint) case getDSSMetadata: - return r.memRepo.GetDSSMetadata(ctx) + return r.Store.GetRepo().GetDSSMetadata(ctx) case recordHeartbeat: var heartbeat auxmodels.Heartbeat @@ -79,7 +70,7 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", recordHeartbeat) } - return nil, r.memRepo.RecordHeartbeat(ctx, heartbeat) + return nil, r.Store.GetRepo().RecordHeartbeat(ctx, heartbeat) default: return nil, stacktrace.NewError("unknown request type: %q", proposal.RequestType) diff --git a/pkg/raftstore/store.go b/pkg/raftstore/store.go index b3c6919aa..436f4bf8f 100644 --- a/pkg/raftstore/store.go +++ b/pkg/raftstore/store.go @@ -5,6 +5,7 @@ import ( "github.com/interuss/dss/pkg/locality" "github.com/interuss/dss/pkg/logging" + "github.com/interuss/dss/pkg/memstore" "github.com/interuss/dss/pkg/raftstore/consensus" raftparams "github.com/interuss/dss/pkg/raftstore/params" "github.com/interuss/dss/pkg/random" @@ -15,19 +16,12 @@ import ( ) type RaftRepo[R any] interface { - GetRepo() R + memstore.MemRepo[R] + // Apply is called on every committed entry. The proposal must be applied atomically. // The any return mirrors store.OperationHandler.Execute: different requests yield different // concrete result types. Callers recover the type via store.TransactWithResult. Apply(ctx context.Context, proposal consensus.Proposal) (any, error) - - // GetSnapshot returns a serialized view of current state, suitable - // for restoring via RestoreFromSnapshot. - GetSnapshot() ([]byte, error) - - // RestoreFromSnapshot replaces all state with the snapshot in data. - // data is always the output of a prior GetSnapshot. - RestoreFromSnapshot(data []byte) error } type Store[R any] struct { @@ -119,7 +113,12 @@ func (s *Store[R]) processCommits(ctx context.Context, commitCh <-chan consensus proposalCtx := timestamp.NewContext(ctx, commit.Prop.Timestamp) proposalCtx = locality.NewContext(proposalCtx, commit.Prop.Locality) proposalCtx = random.NewContext(proposalCtx, commit.Prop.Seed) + s.raftRepo.Checkpoint() result, err := s.raftRepo.Apply(proposalCtx, commit.Prop) + if err != nil { + s.logger.Warn("failed to apply proposal, rolling back", zap.String("proposal_id", commit.Prop.ID), zap.String("proposal_type", string(commit.Prop.RequestType)), zap.Error(err)) + s.raftRepo.Restore() + } commit.Done <- consensus.ProposalResult{Result: result, Error: err} } } diff --git a/pkg/rid/store/raftstore/store.go b/pkg/rid/store/raftstore/store.go index 00b4bf0c5..cfbf6dd51 100644 --- a/pkg/rid/store/raftstore/store.go +++ b/pkg/rid/store/raftstore/store.go @@ -17,8 +17,7 @@ import ( // repo is a full implementation of rid.repos.Repository for Raft-based storage. type repo struct { consensus *consensus.Consensus - memStore *memstore.Store[repos.Repository] - memRepo repos.Repository + *memstore.Store[repos.Repository] } func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore.Store[repos.Repository], error) { @@ -32,7 +31,7 @@ func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore. return nil, stacktrace.Propagate(err, "failed to initialize rid memstore") } - r := &repo{memStore: memStore, memRepo: memStore.GetRepo()} + r := &repo{Store: memStore} store, err := raftstore.Init(ctx, logger.With(zap.String("service", "rid")), locality, params, r, operations.Registry) if err != nil { return nil, stacktrace.Propagate(err, "failed to initialize rid raftstore") @@ -45,14 +44,6 @@ func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore. func (r *repo) GetRepo() repos.Repository { return r } -func (r *repo) GetSnapshot() ([]byte, error) { - return r.memStore.GetSnapshot() -} - -func (r *repo) RestoreFromSnapshot(data []byte) error { - return r.memStore.RestoreFromSnapshot(data) -} - func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, error) { switch proposal.RequestType { @@ -67,6 +58,6 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err return nil, stacktrace.Propagate(err, "failed to decode %s payload", proposal.RequestType) } - return handler.Execute(ctx, r.memRepo, request) + return handler.Execute(ctx, r.Store.GetRepo(), request) } } diff --git a/pkg/scd/store/raftstore/store.go b/pkg/scd/store/raftstore/store.go index baa1a11ae..7718c7d17 100644 --- a/pkg/scd/store/raftstore/store.go +++ b/pkg/scd/store/raftstore/store.go @@ -17,8 +17,7 @@ import ( // repo is a full implementation of scd.repos.Repository for Raft-based storage. type repo struct { consensus *consensus.Consensus - memStore *memstore.Store[repos.Repository] - memRepo repos.Repository + *memstore.Store[repos.Repository] } func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore.Store[repos.Repository], error) { @@ -32,7 +31,7 @@ func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore. return nil, stacktrace.Propagate(err, "failed to initialize scd memstore") } - r := &repo{memStore: memStore, memRepo: memStore.GetRepo()} + r := &repo{Store: memStore} store, err := raftstore.Init(ctx, logger.With(zap.String("service", "scd")), locality, params, r, operations.Registry) if err != nil { return nil, stacktrace.Propagate(err, "failed to initialize scd raftstore") @@ -45,14 +44,6 @@ func Init(ctx context.Context, logger *zap.Logger, locality string) (*raftstore. func (r *repo) GetRepo() repos.Repository { return r } -func (r *repo) GetSnapshot() ([]byte, error) { - return r.memStore.GetSnapshot() -} - -func (r *repo) RestoreFromSnapshot(data []byte) error { - return r.memStore.RestoreFromSnapshot(data) -} - func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, error) { switch proposal.RequestType { @@ -67,6 +58,6 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err return nil, stacktrace.Propagate(err, "failed to decode %s payload", proposal.RequestType) } - return handler.Execute(ctx, r.memRepo, request) + return handler.Execute(ctx, r.Store.GetRepo(), request) } } From fb30ae4eed084ba94539b332469fc1154d08d7b1 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 28 Aug 2026 15:45:13 +0200 Subject: [PATCH 07/17] [raft/scd] Implement constraints repo methods --- pkg/models/geo.go | 78 ++++++++++++++++ pkg/scd/store/raftstore/constraints.go | 118 ++++++++++++++++++++++--- pkg/scd/store/raftstore/store.go | 2 + 3 files changed, 187 insertions(+), 11 deletions(-) diff --git a/pkg/models/geo.go b/pkg/models/geo.go index f52c5ae0d..df7fc3059 100644 --- a/pkg/models/geo.go +++ b/pkg/models/geo.go @@ -1,6 +1,7 @@ package models import ( + "encoding/json" "time" "github.com/golang/geo/s2" @@ -46,6 +47,83 @@ type Volume3D struct { Footprint Geometry } +type Volume3DJSON struct { + AltitudeHi *float32 `json:"altitude_hi,omitempty"` + AltitudeLo *float32 `json:"altitude_lo,omitempty"` + Footprint *geometryJSON `json:"footprint,omitempty"` +} + +type geometryType string + +const ( + circle geometryType = "circle" + polygon geometryType = "polygon" + cells geometryType = "cells" +) + +// geometryJSON is a helper struct for marshaling and unmarshaling Geometry types to/from JSON. +type geometryJSON struct { + Type geometryType `json:"type"` + Polygon *GeoPolygon `json:"polygon,omitempty"` + Circle *GeoCircle `json:"circle,omitempty"` + Cells []s2.CellID `json:"cells,omitempty"` +} + +func (v Volume3D) MarshalJSON() ([]byte, error) { + w := Volume3DJSON{AltitudeHi: v.AltitudeHi, AltitudeLo: v.AltitudeLo} + if v.Footprint != nil { + switch f := v.Footprint.(type) { + case *GeoPolygon: + w.Footprint = &geometryJSON{Type: polygon, Polygon: f} + + case *GeoCircle: + w.Footprint = &geometryJSON{Type: circle, Circle: f} + + case precomputedCellGeometry: + cellsResult := make([]s2.CellID, 0, len(f)) + for id := range f { + cellsResult = append(cellsResult, id) + } + w.Footprint = &geometryJSON{Type: cells, Cells: cellsResult} + + default: + return nil, stacktrace.NewError("Volume3D: unsupported Footprint type %T for JSON marshaling", v.Footprint) + } + } + + return json.Marshal(w) +} + +func (v *Volume3D) UnmarshalJSON(data []byte) error { + var w Volume3DJSON + if err := json.Unmarshal(data, &w); err != nil { + return err + } + v.AltitudeHi = w.AltitudeHi + v.AltitudeLo = w.AltitudeLo + if w.Footprint != nil { + switch w.Footprint.Type { + case polygon: + v.Footprint = w.Footprint.Polygon + + case circle: + v.Footprint = w.Footprint.Circle + + case cells: + pcg := make(precomputedCellGeometry, len(w.Footprint.Cells)) + for _, id := range w.Footprint.Cells { + pcg[id] = struct{}{} + } + + v.Footprint = pcg + default: + return stacktrace.NewError("Volume3D: unknown geometry type %q", w.Footprint.Type) + } + } + + return nil +} + // Geometry models a geometry. type Geometry interface { // CalculateCovering returns an s2 cell covering for a geometry. diff --git a/pkg/scd/store/raftstore/constraints.go b/pkg/scd/store/raftstore/constraints.go index 0983add14..c2bc08626 100644 --- a/pkg/scd/store/raftstore/constraints.go +++ b/pkg/scd/store/raftstore/constraints.go @@ -2,29 +2,125 @@ package raftstore import ( "context" + "encoding/json" - dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" + "github.com/interuss/dss/pkg/raftstore/consensus" scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/stacktrace" ) -func (r *repo) SearchConstraints(_ context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.Constraint, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "SearchConstraints not implemented for raftstore") +const ( + searchConstraints consensus.RequestType = "searchConstraints" + getConstraint consensus.RequestType = "getConstraint" + upsertConstraint consensus.RequestType = "upsertConstraint" + deleteConstraint consensus.RequestType = "deleteConstraint" + countConstraints consensus.RequestType = "countConstraints" +) + +func (r *repo) SearchConstraints(ctx context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.Constraint, error) { + buf, err := json.Marshal(v4d) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, searchConstraints, buf, true) + if err != nil { + return nil, err + } + if constraints, ok := result.([]*scdmodels.Constraint); ok { + return constraints, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) GetConstraint(_ context.Context, id dssmodels.ID) (*scdmodels.Constraint, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "GetConstraint not implemented for raftstore") +func (r *repo) GetConstraint(ctx context.Context, id dssmodels.ID) (*scdmodels.Constraint, error) { + buf, err := json.Marshal(id) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, getConstraint, buf, true) + if err != nil { + return nil, err + } + if constraint, ok := result.(*scdmodels.Constraint); ok { + return constraint, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) UpsertConstraint(_ context.Context, constraint *scdmodels.Constraint) (*scdmodels.Constraint, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "UpsertConstraint not implemented for raftstore") +func (r *repo) UpsertConstraint(ctx context.Context, constraint *scdmodels.Constraint) (*scdmodels.Constraint, error) { + buf, err := json.Marshal(constraint) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, upsertConstraint, buf, false) + if err != nil { + return nil, err + } + if upserted, ok := result.(*scdmodels.Constraint); ok { + return upserted, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) DeleteConstraint(_ context.Context, id dssmodels.ID) error { - return stacktrace.NewErrorWithCode(dsserr.NotImplemented, "DeleteConstraint not implemented for raftstore") +func (r *repo) DeleteConstraint(ctx context.Context, id dssmodels.ID) error { + buf, err := json.Marshal(id) + if err != nil { + return stacktrace.Propagate(err, "failed to marshal payload") + } + + _, err = r.consensus.HandleClientRequest(ctx, deleteConstraint, buf, false) + return err } -func (r *repo) CountConstraints(_ context.Context) (int64, error) { - return 0, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "CountConstraint not implemented for raftstore") +func (r *repo) CountConstraints(ctx context.Context) (int64, error) { + result, err := r.consensus.HandleClientRequest(ctx, countConstraints, nil, true) + if err != nil { + return 0, err + } + if count, ok := result.(int64); ok { + return count, nil + } + return 0, stacktrace.NewError("unexpected result type: %T", result) +} + +func (r *repo) applyConstraint(ctx context.Context, proposal consensus.Proposal) (any, error) { + switch proposal.RequestType { + case searchConstraints: + var v4d dssmodels.Volume4D + if err := json.Unmarshal(proposal.Value, &v4d); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", searchConstraints) + } + return r.Store.GetRepo().SearchConstraints(ctx, &v4d) + + case getConstraint: + var id dssmodels.ID + if err := json.Unmarshal(proposal.Value, &id); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", getConstraint) + } + return r.Store.GetRepo().GetConstraint(ctx, id) + + case upsertConstraint: + var constraint scdmodels.Constraint + if err := json.Unmarshal(proposal.Value, &constraint); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", upsertConstraint) + } + return r.Store.GetRepo().UpsertConstraint(ctx, &constraint) + + case deleteConstraint: + var id dssmodels.ID + if err := json.Unmarshal(proposal.Value, &id); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", deleteConstraint) + } + return nil, r.Store.GetRepo().DeleteConstraint(ctx, id) + + case countConstraints: + return r.Store.GetRepo().CountConstraints(ctx) + + default: + return nil, stacktrace.NewError("unrecognized constraint request type: %s", proposal.RequestType) + } } diff --git a/pkg/scd/store/raftstore/store.go b/pkg/scd/store/raftstore/store.go index 7718c7d17..b0b7df766 100644 --- a/pkg/scd/store/raftstore/store.go +++ b/pkg/scd/store/raftstore/store.go @@ -46,6 +46,8 @@ func (r *repo) GetRepo() repos.Repository { return r } func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, error) { switch proposal.RequestType { + case searchConstraints, getConstraint, upsertConstraint, deleteConstraint, countConstraints: + return r.applyConstraint(ctx, proposal) default: handler, ok := operations.Registry[string(proposal.RequestType)] From 0bdb05bd64eaa474956626e1f44ace3ae682c02e Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 28 Aug 2026 15:45:25 +0200 Subject: [PATCH 08/17] [raft/scd] Implement subscriptions repo methods --- pkg/scd/store/raftstore/store.go | 5 + pkg/scd/store/raftstore/subscriptions.go | 187 ++++++++++++++++++++--- 2 files changed, 173 insertions(+), 19 deletions(-) diff --git a/pkg/scd/store/raftstore/store.go b/pkg/scd/store/raftstore/store.go index b0b7df766..efee3ad3c 100644 --- a/pkg/scd/store/raftstore/store.go +++ b/pkg/scd/store/raftstore/store.go @@ -49,6 +49,11 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err case searchConstraints, getConstraint, upsertConstraint, deleteConstraint, countConstraints: return r.applyConstraint(ctx, proposal) + case searchSubscriptions, getSubscription, upsertSubscription, deleteSubscription, + incrementNotificationIndicesForOperationalIntents, incrementNotificationIndicesForConstraints, + listExpiredSubscriptions, countSubscriptions: + return r.applySubscription(ctx, proposal) + default: handler, ok := operations.Registry[string(proposal.RequestType)] if !ok { diff --git a/pkg/scd/store/raftstore/subscriptions.go b/pkg/scd/store/raftstore/subscriptions.go index 2e5609335..47a5806f5 100644 --- a/pkg/scd/store/raftstore/subscriptions.go +++ b/pkg/scd/store/raftstore/subscriptions.go @@ -2,47 +2,196 @@ package raftstore import ( "context" + "encoding/json" "time" "github.com/golang/geo/s2" - dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" + "github.com/interuss/dss/pkg/raftstore/consensus" scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/stacktrace" ) -func (r *repo) SearchSubscriptions(_ context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "SearchSubscriptions not implemented for raftstore") +const ( + searchSubscriptions consensus.RequestType = "searchSubscriptions" + getSubscription consensus.RequestType = "getSubscription" + upsertSubscription consensus.RequestType = "upsertSubscription" + deleteSubscription consensus.RequestType = "deleteSubscription" + incrementNotificationIndicesForOperationalIntents consensus.RequestType = "incrementNotificationIndicesForOperationalIntents" + incrementNotificationIndicesForConstraints consensus.RequestType = "incrementNotificationIndicesForConstraints" + listExpiredSubscriptions consensus.RequestType = "listExpiredSubscriptions" + countSubscriptions consensus.RequestType = "countSubscriptions" +) + +func (r *repo) SearchSubscriptions(ctx context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.Subscription, error) { + buf, err := json.Marshal(v4d) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, searchSubscriptions, buf, true) + if err != nil { + return nil, err + } + if subscriptions, ok := result.([]*scdmodels.Subscription); ok { + return subscriptions, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) +} + +func (r *repo) GetSubscription(ctx context.Context, id dssmodels.ID) (*scdmodels.Subscription, error) { + buf, err := json.Marshal(id) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, getSubscription, buf, true) + if err != nil { + return nil, err + } + if sub, ok := result.(*scdmodels.Subscription); ok { + return sub, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) GetSubscription(_ context.Context, id dssmodels.ID) (*scdmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "GetSubscription not implemented for raftstore") +func (r *repo) UpsertSubscription(ctx context.Context, sub *scdmodels.Subscription) (*scdmodels.Subscription, error) { + buf, err := json.Marshal(sub) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, upsertSubscription, buf, false) + if err != nil { + return nil, err + } + if upserted, ok := result.(*scdmodels.Subscription); ok { + return upserted, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) +} + +func (r *repo) DeleteSubscription(ctx context.Context, id dssmodels.ID) error { + buf, err := json.Marshal(id) + if err != nil { + return stacktrace.Propagate(err, "failed to marshal payload") + } + + _, err = r.consensus.HandleClientRequest(ctx, deleteSubscription, buf, false) + return err } -func (r *repo) UpsertSubscription(_ context.Context, sub *scdmodels.Subscription) (*scdmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "UpsertSubscription not implemented for raftstore") +func (r *repo) IncrementNotificationIndicesForOperationalIntents(ctx context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.Subscription, error) { + return r.incrementNotificationIndices(ctx, incrementNotificationIndicesForOperationalIntents, v4d) } -func (r *repo) DeleteSubscription(_ context.Context, id dssmodels.ID) error { - return stacktrace.NewErrorWithCode(dsserr.NotImplemented, "DeleteSubscription not implemented for raftstore") +func (r *repo) IncrementNotificationIndicesForConstraints(ctx context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.Subscription, error) { + return r.incrementNotificationIndices(ctx, incrementNotificationIndicesForConstraints, v4d) } -func (r *repo) IncrementNotificationIndicesForOperationalIntents(_ context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "IncrementNotificationIndicesForOperationalIntents not implemented for raftstore") +func (r *repo) incrementNotificationIndices(ctx context.Context, requestType consensus.RequestType, v4d *dssmodels.Volume4D) ([]*scdmodels.Subscription, error) { + buf, err := json.Marshal(v4d) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, requestType, buf, false) + if err != nil { + return nil, err + } + if subscriptions, ok := result.([]*scdmodels.Subscription); ok { + return subscriptions, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) IncrementNotificationIndicesForConstraints(_ context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "IncrementNotificationIndicesForConstraints not implemented for raftstore") +// LockSubscriptionsOnCells is a no-op in the raftstore implementation +func (r *repo) LockSubscriptionsOnCells(_ context.Context, _ s2.CellUnion, _ []dssmodels.ID, _ *time.Time, _ *time.Time) error { + return nil } -func (r *repo) LockSubscriptionsOnCells(_ context.Context, cells s2.CellUnion, subscriptionIds []dssmodels.ID, startTime *time.Time, endTime *time.Time) error { - return stacktrace.NewErrorWithCode(dsserr.NotImplemented, "LockSubscriptionsOnCells not implemented for raftstore") +func (r *repo) ListExpiredSubscriptions(ctx context.Context, threshold time.Time) ([]*scdmodels.Subscription, error) { + buf, err := json.Marshal(threshold) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, listExpiredSubscriptions, buf, true) + if err != nil { + return nil, err + } + if subscriptions, ok := result.([]*scdmodels.Subscription); ok { + return subscriptions, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) ListExpiredSubscriptions(_ context.Context, threshold time.Time) ([]*scdmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "ListExpiredSubscriptions not implemented for raftstore") +func (r *repo) CountSubscriptions(ctx context.Context) (int64, error) { + result, err := r.consensus.HandleClientRequest(ctx, countSubscriptions, nil, true) + if err != nil { + return 0, err + } + if count, ok := result.(int64); ok { + return count, nil + } + return 0, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) CountSubscriptions(_ context.Context) (int64, error) { - return 0, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "CountSubscriptions not implemented for raftstore") +func (r *repo) applySubscription(ctx context.Context, proposal consensus.Proposal) (any, error) { + switch proposal.RequestType { + case searchSubscriptions: + var v4d dssmodels.Volume4D + if err := json.Unmarshal(proposal.Value, &v4d); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", searchSubscriptions) + } + return r.Store.GetRepo().SearchSubscriptions(ctx, &v4d) + + case getSubscription: + var id dssmodels.ID + if err := json.Unmarshal(proposal.Value, &id); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", getSubscription) + } + return r.Store.GetRepo().GetSubscription(ctx, id) + + case upsertSubscription: + var sub scdmodels.Subscription + if err := json.Unmarshal(proposal.Value, &sub); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", upsertSubscription) + } + return r.Store.GetRepo().UpsertSubscription(ctx, &sub) + + case deleteSubscription: + var id dssmodels.ID + if err := json.Unmarshal(proposal.Value, &id); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", deleteSubscription) + } + return nil, r.Store.GetRepo().DeleteSubscription(ctx, id) + + case incrementNotificationIndicesForOperationalIntents: + var v4d dssmodels.Volume4D + if err := json.Unmarshal(proposal.Value, &v4d); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", incrementNotificationIndicesForOperationalIntents) + } + return r.Store.GetRepo().IncrementNotificationIndicesForOperationalIntents(ctx, &v4d) + + case incrementNotificationIndicesForConstraints: + var v4d dssmodels.Volume4D + if err := json.Unmarshal(proposal.Value, &v4d); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", incrementNotificationIndicesForConstraints) + } + return r.Store.GetRepo().IncrementNotificationIndicesForConstraints(ctx, &v4d) + + case listExpiredSubscriptions: + var threshold time.Time + if err := json.Unmarshal(proposal.Value, &threshold); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", listExpiredSubscriptions) + } + return r.Store.GetRepo().ListExpiredSubscriptions(ctx, threshold) + + case countSubscriptions: + return r.Store.GetRepo().CountSubscriptions(ctx) + + default: + return nil, stacktrace.NewError("unrecognized subscription request type: %s", proposal.RequestType) + } } From c7ec73d4d38ace15a9a93ff7f2c9d20713dc263d Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 28 Aug 2026 15:45:40 +0200 Subject: [PATCH 09/17] [raft/scd] Implement operational intents repo methods --- .../store/raftstore/operational_intents.go | 166 ++++++++++++++++-- pkg/scd/store/raftstore/store.go | 4 + 2 files changed, 155 insertions(+), 15 deletions(-) diff --git a/pkg/scd/store/raftstore/operational_intents.go b/pkg/scd/store/raftstore/operational_intents.go index 3a6af6acd..674890513 100644 --- a/pkg/scd/store/raftstore/operational_intents.go +++ b/pkg/scd/store/raftstore/operational_intents.go @@ -2,38 +2,174 @@ package raftstore import ( "context" + "encoding/json" "time" - dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" + "github.com/interuss/dss/pkg/raftstore/consensus" scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/stacktrace" ) -func (r *repo) GetOperationalIntent(_ context.Context, id dssmodels.ID) (*scdmodels.OperationalIntent, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "GetOperationalIntent not implemented for raftstore") +const ( + getOperationalIntent consensus.RequestType = "getOperationalIntent" + deleteOperationalIntent consensus.RequestType = "deleteOperationalIntent" + upsertOperationalIntent consensus.RequestType = "upsertOperationalIntent" + searchOperationalIntents consensus.RequestType = "searchOperationalIntents" + getDependentOperationalIntents consensus.RequestType = "getDependentOperationalIntents" + listExpiredOperationalIntents consensus.RequestType = "listExpiredOperationalIntents" + countOperationalIntents consensus.RequestType = "countOperationalIntents" +) + +func (r *repo) GetOperationalIntent(ctx context.Context, id dssmodels.ID) (*scdmodels.OperationalIntent, error) { + buf, err := json.Marshal(id) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, getOperationalIntent, buf, true) + if err != nil { + return nil, err + } + if operation, ok := result.(*scdmodels.OperationalIntent); ok { + return operation, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) DeleteOperationalIntent(_ context.Context, id dssmodels.ID) error { - return stacktrace.NewErrorWithCode(dsserr.NotImplemented, "DeleteOperationalIntent not implemented for raftstore") +func (r *repo) DeleteOperationalIntent(ctx context.Context, id dssmodels.ID) error { + buf, err := json.Marshal(id) + if err != nil { + return stacktrace.Propagate(err, "failed to marshal payload") + } + + _, err = r.consensus.HandleClientRequest(ctx, deleteOperationalIntent, buf, false) + return err } -func (r *repo) UpsertOperationalIntent(_ context.Context, operation *scdmodels.OperationalIntent) (*scdmodels.OperationalIntent, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "UpsertOperationalIntent not implemented for raftstore") +func (r *repo) UpsertOperationalIntent(ctx context.Context, operation *scdmodels.OperationalIntent) (*scdmodels.OperationalIntent, error) { + buf, err := json.Marshal(operation) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, upsertOperationalIntent, buf, false) + if err != nil { + return nil, err + } + if upserted, ok := result.(*scdmodels.OperationalIntent); ok { + return upserted, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) SearchOperationalIntents(_ context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.OperationalIntent, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "SearchOperationalIntents not implemented for raftstore") +func (r *repo) SearchOperationalIntents(ctx context.Context, v4d *dssmodels.Volume4D) ([]*scdmodels.OperationalIntent, error) { + buf, err := json.Marshal(v4d) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, searchOperationalIntents, buf, true) + if err != nil { + return nil, err + } + if operations, ok := result.([]*scdmodels.OperationalIntent); ok { + return operations, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) GetDependentOperationalIntents(_ context.Context, subscriptionID dssmodels.ID) ([]dssmodels.ID, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "GetDependentOperationalIntents not implemented for raftstore") +func (r *repo) GetDependentOperationalIntents(ctx context.Context, subscriptionID dssmodels.ID) ([]dssmodels.ID, error) { + buf, err := json.Marshal(subscriptionID) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, getDependentOperationalIntents, buf, true) + if err != nil { + return nil, err + } + if ids, ok := result.([]dssmodels.ID); ok { + return ids, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) ListExpiredOperationalIntents(_ context.Context, threshold time.Time) ([]*scdmodels.OperationalIntent, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "ListExpiredOperationalIntents not implemented for raftstore") +func (r *repo) ListExpiredOperationalIntents(ctx context.Context, threshold time.Time) ([]*scdmodels.OperationalIntent, error) { + buf, err := json.Marshal(threshold) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, listExpiredOperationalIntents, buf, true) + if err != nil { + return nil, err + } + if operations, ok := result.([]*scdmodels.OperationalIntent); ok { + return operations, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) CountOperationalIntents(_ context.Context) (int64, error) { - return 0, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "CountOperationalIntents not implemented for raftstore") +func (r *repo) CountOperationalIntents(ctx context.Context) (int64, error) { + result, err := r.consensus.HandleClientRequest(ctx, countOperationalIntents, nil, true) + if err != nil { + return 0, err + } + if count, ok := result.(int64); ok { + return count, nil + } + return 0, stacktrace.NewError("unexpected result type: %T", result) +} + +func (r *repo) applyOperationalIntent(ctx context.Context, proposal consensus.Proposal) (any, error) { + switch proposal.RequestType { + case getOperationalIntent: + var id dssmodels.ID + if err := json.Unmarshal(proposal.Value, &id); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", getOperationalIntent) + } + return r.Store.GetRepo().GetOperationalIntent(ctx, id) + + case deleteOperationalIntent: + var id dssmodels.ID + if err := json.Unmarshal(proposal.Value, &id); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", deleteOperationalIntent) + } + return nil, r.Store.GetRepo().DeleteOperationalIntent(ctx, id) + + case upsertOperationalIntent: + var operation scdmodels.OperationalIntent + if err := json.Unmarshal(proposal.Value, &operation); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", upsertOperationalIntent) + } + return r.Store.GetRepo().UpsertOperationalIntent(ctx, &operation) + + case searchOperationalIntents: + var v4d dssmodels.Volume4D + if err := json.Unmarshal(proposal.Value, &v4d); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", searchOperationalIntents) + } + return r.Store.GetRepo().SearchOperationalIntents(ctx, &v4d) + + case getDependentOperationalIntents: + var subscriptionID dssmodels.ID + if err := json.Unmarshal(proposal.Value, &subscriptionID); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", getDependentOperationalIntents) + } + return r.Store.GetRepo().GetDependentOperationalIntents(ctx, subscriptionID) + + case listExpiredOperationalIntents: + var threshold time.Time + if err := json.Unmarshal(proposal.Value, &threshold); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", listExpiredOperationalIntents) + } + return r.Store.GetRepo().ListExpiredOperationalIntents(ctx, threshold) + + case countOperationalIntents: + return r.Store.GetRepo().CountOperationalIntents(ctx) + + default: + return nil, stacktrace.NewError("unrecognized operational intent request type: %s", proposal.RequestType) + } } diff --git a/pkg/scd/store/raftstore/store.go b/pkg/scd/store/raftstore/store.go index efee3ad3c..e8e326168 100644 --- a/pkg/scd/store/raftstore/store.go +++ b/pkg/scd/store/raftstore/store.go @@ -54,6 +54,10 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err listExpiredSubscriptions, countSubscriptions: return r.applySubscription(ctx, proposal) + case getOperationalIntent, deleteOperationalIntent, upsertOperationalIntent, searchOperationalIntents, + getDependentOperationalIntents, listExpiredOperationalIntents, countOperationalIntents: + return r.applyOperationalIntent(ctx, proposal) + default: handler, ok := operations.Registry[string(proposal.RequestType)] if !ok { From d12d152ff995529e310df5f64a3c2121f9224bb1 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 28 Aug 2026 15:45:53 +0200 Subject: [PATCH 10/17] [raft/scd] Implement availability repo methods --- pkg/scd/store/raftstore/availability.go | 61 +++++++++++++++++++++++-- pkg/scd/store/raftstore/store.go | 3 ++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/pkg/scd/store/raftstore/availability.go b/pkg/scd/store/raftstore/availability.go index 7e45f9c4b..7a6092ed9 100644 --- a/pkg/scd/store/raftstore/availability.go +++ b/pkg/scd/store/raftstore/availability.go @@ -2,17 +2,68 @@ package raftstore import ( "context" + "encoding/json" - dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" + "github.com/interuss/dss/pkg/raftstore/consensus" scdmodels "github.com/interuss/dss/pkg/scd/models" "github.com/interuss/stacktrace" ) -func (r *repo) GetUssAvailability(_ context.Context, id dssmodels.Manager) (*scdmodels.UssAvailabilityStatus, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "GetUssAvailability not implemented for raftstore") +const ( + getUssAvailability consensus.RequestType = "getUssAvailability" + upsertUssAvailability consensus.RequestType = "upsertUssAvailability" +) + +func (r *repo) GetUssAvailability(ctx context.Context, id dssmodels.Manager) (*scdmodels.UssAvailabilityStatus, error) { + buf, err := json.Marshal(id) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, getUssAvailability, buf, true) + if err != nil { + return nil, err + } + if ussa, ok := result.(*scdmodels.UssAvailabilityStatus); ok { + return ussa, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) +} + +func (r *repo) UpsertUssAvailability(ctx context.Context, ussa *scdmodels.UssAvailabilityStatus) (*scdmodels.UssAvailabilityStatus, error) { + buf, err := json.Marshal(ussa) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, upsertUssAvailability, buf, false) + if err != nil { + return nil, err + } + if upserted, ok := result.(*scdmodels.UssAvailabilityStatus); ok { + return upserted, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) UpsertUssAvailability(_ context.Context, ussa *scdmodels.UssAvailabilityStatus) (*scdmodels.UssAvailabilityStatus, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "UpsertUssAvailability not implemented for raftstore") +func (r *repo) applyAvailability(ctx context.Context, proposal consensus.Proposal) (any, error) { + switch proposal.RequestType { + case getUssAvailability: + var manager dssmodels.Manager + if err := json.Unmarshal(proposal.Value, &manager); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", getUssAvailability) + } + return r.Store.GetRepo().GetUssAvailability(ctx, manager) + + case upsertUssAvailability: + var ussa scdmodels.UssAvailabilityStatus + if err := json.Unmarshal(proposal.Value, &ussa); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", upsertUssAvailability) + } + return r.Store.GetRepo().UpsertUssAvailability(ctx, &ussa) + + default: + return nil, stacktrace.NewError("unrecognized availability request type: %s", proposal.RequestType) + } } diff --git a/pkg/scd/store/raftstore/store.go b/pkg/scd/store/raftstore/store.go index e8e326168..4f58487e8 100644 --- a/pkg/scd/store/raftstore/store.go +++ b/pkg/scd/store/raftstore/store.go @@ -58,6 +58,9 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err getDependentOperationalIntents, listExpiredOperationalIntents, countOperationalIntents: return r.applyOperationalIntent(ctx, proposal) + case getUssAvailability, upsertUssAvailability: + return r.applyAvailability(ctx, proposal) + default: handler, ok := operations.Registry[string(proposal.RequestType)] if !ok { From 69ba1e7539239d2b56394cc2686118e965ea39bd Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 28 Aug 2026 15:46:16 +0200 Subject: [PATCH 11/17] [raft/rid] Implement ISA repo methods --- pkg/models/models.go | 24 +++ .../raftstore/identification_service_area.go | 172 ++++++++++++++++-- pkg/rid/store/raftstore/payloads.go | 20 ++ pkg/rid/store/raftstore/store.go | 2 + 4 files changed, 203 insertions(+), 15 deletions(-) create mode 100644 pkg/rid/store/raftstore/payloads.go diff --git a/pkg/models/models.go b/pkg/models/models.go index 18bc8c29b..84be5be9e 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -1,6 +1,7 @@ package models import ( + "encoding/json" "strconv" "time" @@ -175,3 +176,26 @@ func (v *Version) ToTimestamp() *time.Time { } return &v.t } + +func (v *Version) MarshalJSON() ([]byte, error) { + return json.Marshal(v.String()) +} + +func (v *Version) UnmarshalJSON(data []byte) error { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return err + } + + if s == "" { + return nil + } + + parsed, err := VersionFromString(s) + if err != nil { + return stacktrace.Propagate(err, "failed to unmarshal version") + } + + *v = *parsed + return nil +} diff --git a/pkg/rid/store/raftstore/identification_service_area.go b/pkg/rid/store/raftstore/identification_service_area.go index b9f7222a5..55ff39b80 100644 --- a/pkg/rid/store/raftstore/identification_service_area.go +++ b/pkg/rid/store/raftstore/identification_service_area.go @@ -2,39 +2,181 @@ package raftstore import ( "context" + "encoding/json" "time" "github.com/golang/geo/s2" - dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" + "github.com/interuss/dss/pkg/raftstore/consensus" ridmodels "github.com/interuss/dss/pkg/rid/models" "github.com/interuss/stacktrace" ) -func (r *repo) GetISA(_ context.Context, id dssmodels.ID, forUpdate bool) (*ridmodels.IdentificationServiceArea, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "GetISA not implemented for raftstore") +const ( + getISA consensus.RequestType = "getISA" + deleteISA consensus.RequestType = "deleteISA" + insertISA consensus.RequestType = "insertISA" + updateISA consensus.RequestType = "updateISA" + searchISAs consensus.RequestType = "searchISAs" + listExpiredISAs consensus.RequestType = "listExpiredISAs" + countISAs consensus.RequestType = "countISAs" +) + +func (r *repo) GetISA(ctx context.Context, id dssmodels.ID, _ bool) (*ridmodels.IdentificationServiceArea, error) { + buf, err := json.Marshal(id) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, getISA, buf, true) + if err != nil { + return nil, err + } + if isa, ok := result.(*ridmodels.IdentificationServiceArea); ok { + return isa, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) DeleteISA(_ context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "DeleteISA not implemented for raftstore") +func (r *repo) DeleteISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { + buf, err := json.Marshal(isa) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, deleteISA, buf, false) + if err != nil { + return nil, err + } + if deleted, ok := result.(*ridmodels.IdentificationServiceArea); ok { + return deleted, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) InsertISA(_ context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "InsertISA not implemented for raftstore") +func (r *repo) InsertISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { + buf, err := json.Marshal(isa) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, insertISA, buf, false) + if err != nil { + return nil, err + } + if inserted, ok := result.(*ridmodels.IdentificationServiceArea); ok { + return inserted, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) UpdateISA(_ context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "UpdateISA not implemented for raftstore") +func (r *repo) UpdateISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { + buf, err := json.Marshal(isa) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, updateISA, buf, false) + if err != nil { + return nil, err + } + if updated, ok := result.(*ridmodels.IdentificationServiceArea); ok { + return updated, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) SearchISAs(_ context.Context, cells s2.CellUnion, earliest *time.Time, latest *time.Time) ([]*ridmodels.IdentificationServiceArea, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "SearchISAs not implemented for raftstore") +func (r *repo) SearchISAs(ctx context.Context, cells s2.CellUnion, earliest *time.Time, latest *time.Time) ([]*ridmodels.IdentificationServiceArea, error) { + buf, err := json.Marshal(searchISAsPayload{Cells: cells, Earliest: earliest, Latest: latest}) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, searchISAs, buf, true) + if err != nil { + return nil, err + } + if isas, ok := result.([]*ridmodels.IdentificationServiceArea); ok { + return isas, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) ListExpiredISAs(_ context.Context, writer string, threshold time.Time) ([]*ridmodels.IdentificationServiceArea, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "ListExpiredISAs not implemented for raftstore") +func (r *repo) ListExpiredISAs(ctx context.Context, writer string, threshold time.Time) ([]*ridmodels.IdentificationServiceArea, error) { + buf, err := json.Marshal(expiredPayload{Writer: writer, Threshold: threshold}) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, listExpiredISAs, buf, true) + if err != nil { + return nil, err + } + if isas, ok := result.([]*ridmodels.IdentificationServiceArea); ok { + return isas, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) CountISAs(_ context.Context) (int64, error) { - return 0, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "CountISAs not implemented for raftstore") +func (r *repo) CountISAs(ctx context.Context) (int64, error) { + result, err := r.consensus.HandleClientRequest(ctx, countISAs, nil, true) + if err != nil { + return 0, err + } + if count, ok := result.(int64); ok { + return count, nil + } + return 0, stacktrace.NewError("unexpected result type: %T", result) +} + +func (r *repo) applyISA(ctx context.Context, proposal consensus.Proposal) (any, error) { + switch proposal.RequestType { + case getISA: + var id dssmodels.ID + if err := json.Unmarshal(proposal.Value, &id); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", getISA) + } + return r.Store.GetRepo().GetISA(ctx, id, false) + + case deleteISA: + var isa ridmodels.IdentificationServiceArea + if err := json.Unmarshal(proposal.Value, &isa); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", deleteISA) + } + return r.Store.GetRepo().DeleteISA(ctx, &isa) + + case insertISA: + var isa ridmodels.IdentificationServiceArea + if err := json.Unmarshal(proposal.Value, &isa); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", insertISA) + } + return r.Store.GetRepo().InsertISA(ctx, &isa) + + case updateISA: + var isa ridmodels.IdentificationServiceArea + if err := json.Unmarshal(proposal.Value, &isa); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", updateISA) + } + return r.Store.GetRepo().UpdateISA(ctx, &isa) + + case searchISAs: + var payload searchISAsPayload + if err := json.Unmarshal(proposal.Value, &payload); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", searchISAs) + } + return r.Store.GetRepo().SearchISAs(ctx, payload.Cells, payload.Earliest, payload.Latest) + + case listExpiredISAs: + var payload expiredPayload + if err := json.Unmarshal(proposal.Value, &payload); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", listExpiredISAs) + } + return r.Store.GetRepo().ListExpiredISAs(ctx, payload.Writer, payload.Threshold) + + case countISAs: + return r.Store.GetRepo().CountISAs(ctx) + + default: + return nil, stacktrace.NewError("unrecognized ISA request type: %s", proposal.RequestType) + } } diff --git a/pkg/rid/store/raftstore/payloads.go b/pkg/rid/store/raftstore/payloads.go new file mode 100644 index 000000000..040a1788e --- /dev/null +++ b/pkg/rid/store/raftstore/payloads.go @@ -0,0 +1,20 @@ +package raftstore + +import ( + "time" + + "github.com/golang/geo/s2" +) + +// expiredPayload carries the arguments common to ListExpiredISAs/ListExpiredSubscriptions. +type expiredPayload struct { + Writer string `json:"writer"` + Threshold time.Time `json:"threshold"` +} + +// searchISAsPayload carries the arguments of SearchISAs. +type searchISAsPayload struct { + Cells s2.CellUnion `json:"cells"` + Earliest *time.Time `json:"earliest,omitempty"` + Latest *time.Time `json:"latest,omitempty"` +} diff --git a/pkg/rid/store/raftstore/store.go b/pkg/rid/store/raftstore/store.go index cfbf6dd51..2c22d87ac 100644 --- a/pkg/rid/store/raftstore/store.go +++ b/pkg/rid/store/raftstore/store.go @@ -46,6 +46,8 @@ func (r *repo) GetRepo() repos.Repository { return r } func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, error) { switch proposal.RequestType { + case getISA, deleteISA, insertISA, updateISA, searchISAs, listExpiredISAs, countISAs: + return r.applyISA(ctx, proposal) default: handler, ok := operations.Registry[string(proposal.RequestType)] From 1abf559e4cfee79eba73286573e9fc85ccbad290 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 28 Aug 2026 15:46:36 +0200 Subject: [PATCH 12/17] [raft/rid] Implement subscriptions repo methods --- pkg/rid/store/raftstore/payloads.go | 7 + pkg/rid/store/raftstore/store.go | 5 + pkg/rid/store/raftstore/subscriptions.go | 248 +++++++++++++++++++++-- 3 files changed, 239 insertions(+), 21 deletions(-) diff --git a/pkg/rid/store/raftstore/payloads.go b/pkg/rid/store/raftstore/payloads.go index 040a1788e..7db05469a 100644 --- a/pkg/rid/store/raftstore/payloads.go +++ b/pkg/rid/store/raftstore/payloads.go @@ -4,8 +4,15 @@ import ( "time" "github.com/golang/geo/s2" + dssmodels "github.com/interuss/dss/pkg/models" ) +// cellsByOwnerPayload carries the arguments common to SearchSubscriptionsByOwner/MaxSubscriptionCountInCellsByOwner. +type cellsByOwnerPayload struct { + Cells s2.CellUnion `json:"cells"` + Owner dssmodels.Owner `json:"owner"` +} + // expiredPayload carries the arguments common to ListExpiredISAs/ListExpiredSubscriptions. type expiredPayload struct { Writer string `json:"writer"` diff --git a/pkg/rid/store/raftstore/store.go b/pkg/rid/store/raftstore/store.go index 2c22d87ac..9eadfb67f 100644 --- a/pkg/rid/store/raftstore/store.go +++ b/pkg/rid/store/raftstore/store.go @@ -49,6 +49,11 @@ func (r *repo) Apply(ctx context.Context, proposal consensus.Proposal) (any, err case getISA, deleteISA, insertISA, updateISA, searchISAs, listExpiredISAs, countISAs: return r.applyISA(ctx, proposal) + case getSubscription, deleteSubscription, insertSubscription, updateSubscription, + searchSubscriptions, searchSubscriptionsByOwner, updateNotificationIdxsInCells, + maxSubscriptionCountInCellsByOwner, listExpiredSubscriptions, countSubscriptions: + return r.applySubscription(ctx, proposal) + default: handler, ok := operations.Registry[string(proposal.RequestType)] if !ok { diff --git a/pkg/rid/store/raftstore/subscriptions.go b/pkg/rid/store/raftstore/subscriptions.go index 0c9c261dd..0c7a58e23 100644 --- a/pkg/rid/store/raftstore/subscriptions.go +++ b/pkg/rid/store/raftstore/subscriptions.go @@ -2,51 +2,257 @@ package raftstore import ( "context" + "encoding/json" "time" "github.com/golang/geo/s2" - dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" + "github.com/interuss/dss/pkg/raftstore/consensus" ridmodels "github.com/interuss/dss/pkg/rid/models" "github.com/interuss/stacktrace" ) -func (r *repo) GetSubscription(_ context.Context, id dssmodels.ID) (*ridmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "GetSubscription not implemented for raftstore") +const ( + getSubscription consensus.RequestType = "getSubscription" + deleteSubscription consensus.RequestType = "deleteSubscription" + insertSubscription consensus.RequestType = "insertSubscription" + updateSubscription consensus.RequestType = "updateSubscription" + searchSubscriptions consensus.RequestType = "searchSubscriptions" + searchSubscriptionsByOwner consensus.RequestType = "searchSubscriptionsByOwner" + updateNotificationIdxsInCells consensus.RequestType = "updateNotificationIdxsInCells" + maxSubscriptionCountInCellsByOwner consensus.RequestType = "maxSubscriptionCountInCellsByOwner" + listExpiredSubscriptions consensus.RequestType = "listExpiredSubscriptions" + countSubscriptions consensus.RequestType = "countSubscriptions" +) + +func (r *repo) GetSubscription(ctx context.Context, id dssmodels.ID) (*ridmodels.Subscription, error) { + buf, err := json.Marshal(id) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, getSubscription, buf, true) + if err != nil { + return nil, err + } + if sub, ok := result.(*ridmodels.Subscription); ok { + return sub, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) DeleteSubscription(_ context.Context, sub *ridmodels.Subscription) (*ridmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "DeleteSubscription not implemented for raftstore") +func (r *repo) DeleteSubscription(ctx context.Context, sub *ridmodels.Subscription) (*ridmodels.Subscription, error) { + buf, err := json.Marshal(sub) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, deleteSubscription, buf, false) + if err != nil { + return nil, err + } + if deleted, ok := result.(*ridmodels.Subscription); ok { + return deleted, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) InsertSubscription(_ context.Context, sub *ridmodels.Subscription) (*ridmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "InsertSubscription not implemented for raftstore") +func (r *repo) InsertSubscription(ctx context.Context, sub *ridmodels.Subscription) (*ridmodels.Subscription, error) { + buf, err := json.Marshal(sub) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, insertSubscription, buf, false) + if err != nil { + return nil, err + } + if inserted, ok := result.(*ridmodels.Subscription); ok { + return inserted, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) UpdateSubscription(_ context.Context, sub *ridmodels.Subscription) (*ridmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "UpdateSubscription not implemented for raftstore") +func (r *repo) UpdateSubscription(ctx context.Context, sub *ridmodels.Subscription) (*ridmodels.Subscription, error) { + buf, err := json.Marshal(sub) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, updateSubscription, buf, false) + if err != nil { + return nil, err + } + if updated, ok := result.(*ridmodels.Subscription); ok { + return updated, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) SearchSubscriptions(_ context.Context, cells s2.CellUnion) ([]*ridmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "SearchSubscriptions not implemented for raftstore") +func (r *repo) SearchSubscriptions(ctx context.Context, cells s2.CellUnion) ([]*ridmodels.Subscription, error) { + return r.searchSubscriptions(ctx, searchSubscriptions, cells) } -func (r *repo) SearchSubscriptionsByOwner(_ context.Context, cells s2.CellUnion, owner dssmodels.Owner) ([]*ridmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "SearchSubscriptionsByOwner not implemented for raftstore") +func (r *repo) SearchSubscriptionsByOwner(ctx context.Context, cells s2.CellUnion, owner dssmodels.Owner) ([]*ridmodels.Subscription, error) { + buf, err := json.Marshal(cellsByOwnerPayload{Cells: cells, Owner: owner}) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, searchSubscriptionsByOwner, buf, true) + if err != nil { + return nil, err + } + if subscriptions, ok := result.([]*ridmodels.Subscription); ok { + return subscriptions, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) UpdateNotificationIdxsInCells(_ context.Context, cells s2.CellUnion) ([]*ridmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "UpdateNotificationIdxsInCells not implemented for raftstore") +func (r *repo) searchSubscriptions(ctx context.Context, requestType consensus.RequestType, cells s2.CellUnion) ([]*ridmodels.Subscription, error) { + buf, err := json.Marshal(cells) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, requestType, buf, true) + if err != nil { + return nil, err + } + if subscriptions, ok := result.([]*ridmodels.Subscription); ok { + return subscriptions, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) +} + +func (r *repo) UpdateNotificationIdxsInCells(ctx context.Context, cells s2.CellUnion) ([]*ridmodels.Subscription, error) { + buf, err := json.Marshal(cells) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, updateNotificationIdxsInCells, buf, false) + if err != nil { + return nil, err + } + if subscriptions, ok := result.([]*ridmodels.Subscription); ok { + return subscriptions, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) MaxSubscriptionCountInCellsByOwner(_ context.Context, cells s2.CellUnion, owner dssmodels.Owner) (int, error) { - return 0, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "MaxSubscriptionCountInCellsByOwner not implemented for raftstore") +func (r *repo) MaxSubscriptionCountInCellsByOwner(ctx context.Context, cells s2.CellUnion, owner dssmodels.Owner) (int, error) { + buf, err := json.Marshal(cellsByOwnerPayload{Cells: cells, Owner: owner}) + if err != nil { + return 0, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, maxSubscriptionCountInCellsByOwner, buf, true) + if err != nil { + return 0, err + } + if count, ok := result.(int); ok { + return count, nil + } + return 0, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) ListExpiredSubscriptions(_ context.Context, writer string, threshold time.Time) ([]*ridmodels.Subscription, error) { - return nil, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "ListExpiredSubscriptions not implemented for raftstore") +func (r *repo) ListExpiredSubscriptions(ctx context.Context, writer string, threshold time.Time) ([]*ridmodels.Subscription, error) { + buf, err := json.Marshal(expiredPayload{Writer: writer, Threshold: threshold}) + if err != nil { + return nil, stacktrace.Propagate(err, "failed to marshal payload") + } + + result, err := r.consensus.HandleClientRequest(ctx, listExpiredSubscriptions, buf, true) + if err != nil { + return nil, err + } + if subscriptions, ok := result.([]*ridmodels.Subscription); ok { + return subscriptions, nil + } + return nil, stacktrace.NewError("unexpected result type: %T", result) } -func (r *repo) CountSubscriptions(_ context.Context) (int64, error) { - return 0, stacktrace.NewErrorWithCode(dsserr.NotImplemented, "CountSubscriptions not implemented for raftstore") +func (r *repo) CountSubscriptions(ctx context.Context) (int64, error) { + result, err := r.consensus.HandleClientRequest(ctx, countSubscriptions, nil, true) + if err != nil { + return 0, err + } + if count, ok := result.(int64); ok { + return count, nil + } + return 0, stacktrace.NewError("unexpected result type: %T", result) +} + +func (r *repo) applySubscription(ctx context.Context, proposal consensus.Proposal) (any, error) { + switch proposal.RequestType { + case getSubscription: + var id dssmodels.ID + if err := json.Unmarshal(proposal.Value, &id); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", getSubscription) + } + return r.Store.GetRepo().GetSubscription(ctx, id) + + case deleteSubscription: + var sub ridmodels.Subscription + if err := json.Unmarshal(proposal.Value, &sub); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", deleteSubscription) + } + return r.Store.GetRepo().DeleteSubscription(ctx, &sub) + + case insertSubscription: + var sub ridmodels.Subscription + if err := json.Unmarshal(proposal.Value, &sub); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", insertSubscription) + } + return r.Store.GetRepo().InsertSubscription(ctx, &sub) + + case updateSubscription: + var sub ridmodels.Subscription + if err := json.Unmarshal(proposal.Value, &sub); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", updateSubscription) + } + return r.Store.GetRepo().UpdateSubscription(ctx, &sub) + + case searchSubscriptions: + var cells s2.CellUnion + if err := json.Unmarshal(proposal.Value, &cells); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", searchSubscriptions) + } + return r.Store.GetRepo().SearchSubscriptions(ctx, cells) + + case searchSubscriptionsByOwner: + var payload cellsByOwnerPayload + if err := json.Unmarshal(proposal.Value, &payload); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", searchSubscriptionsByOwner) + } + return r.Store.GetRepo().SearchSubscriptionsByOwner(ctx, payload.Cells, payload.Owner) + + case updateNotificationIdxsInCells: + var cells s2.CellUnion + if err := json.Unmarshal(proposal.Value, &cells); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", updateNotificationIdxsInCells) + } + return r.Store.GetRepo().UpdateNotificationIdxsInCells(ctx, cells) + + case maxSubscriptionCountInCellsByOwner: + var payload cellsByOwnerPayload + if err := json.Unmarshal(proposal.Value, &payload); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", maxSubscriptionCountInCellsByOwner) + } + return r.Store.GetRepo().MaxSubscriptionCountInCellsByOwner(ctx, payload.Cells, payload.Owner) + + case listExpiredSubscriptions: + var payload expiredPayload + if err := json.Unmarshal(proposal.Value, &payload); err != nil { + return nil, stacktrace.Propagate(err, "failed to unmarshal %s payload", listExpiredSubscriptions) + } + return r.Store.GetRepo().ListExpiredSubscriptions(ctx, payload.Writer, payload.Threshold) + + case countSubscriptions: + return r.Store.GetRepo().CountSubscriptions(ctx) + + default: + return nil, stacktrace.NewError("unrecognized subscription request type: %s", proposal.RequestType) + } } From af323f4f65ab936ad1bfda98e266f7e5d4650691 Mon Sep 17 00:00:00 2001 From: Maximilien Cuony Date: Fri, 11 Sep 2026 17:09:03 +0200 Subject: [PATCH 13/17] [raft] Fix start action missing required context --- cmds/core-service/main.go | 9 +++++++++ pkg/random/random.go | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/cmds/core-service/main.go b/cmds/core-service/main.go index 91ee224b1..508126932 100644 --- a/cmds/core-service/main.go +++ b/cmds/core-service/main.go @@ -112,6 +112,15 @@ func createAuxServer(ctx context.Context, locality string, publicEndpoint string return nil, stacktrace.Propagate(err, "Unable to interact with store") } + ctx = timestamp.NewContext(ctx, time.Now()) + + seed, err := random.NewSeed() + if err != nil { + return nil, stacktrace.Propagate(err, "Unable to generate seed") + } + + ctx = random.NewContext(ctx, seed) + err = repo.SaveOwnMetadata(ctx, locality, publicEndpoint) if err != nil { diff --git a/pkg/random/random.go b/pkg/random/random.go index 99b20dc81..fa8d9a8c7 100644 --- a/pkg/random/random.go +++ b/pkg/random/random.go @@ -17,7 +17,7 @@ import ( type key struct{} -func newSeed() (int64, error) { +func NewSeed() (int64, error) { var buf [8]byte _, err := rand.Read(buf[:]) if err != nil { @@ -66,7 +66,7 @@ func Generator(seed int64, label string) (*mrand.Rand, error) { // deterministically via Generator. func Middleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - seed, err := newSeed() + seed, err := NewSeed() if err != nil { http.Error(w, "failed to generate request seed", http.StatusInternalServerError) return From 1bbe0d9aac80bb80751771d6f594b16e476498e4 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Mon, 31 Aug 2026 10:44:21 +0200 Subject: [PATCH 14/17] [raft/rid] Extract InsertSubscription and route Create through the store --- pkg/rid/application/isa_test.go | 18 +- pkg/rid/application/subscription.go | 43 --- pkg/rid/application/subscription_test.go | 143 +-------- pkg/rid/operations/subscription.go | 107 +++++++ pkg/rid/operations/subscription_test.go | 336 ++++++++++++++++++++++ pkg/rid/server/v1/server_test.go | 57 +--- pkg/rid/server/v1/subscription_handler.go | 23 +- pkg/rid/server/v2/subscription_handler.go | 23 +- 8 files changed, 484 insertions(+), 266 deletions(-) create mode 100644 pkg/rid/operations/subscription_test.go diff --git a/pkg/rid/application/isa_test.go b/pkg/rid/application/isa_test.go index f620d302b..a63fc4388 100644 --- a/pkg/rid/application/isa_test.go +++ b/pkg/rid/application/isa_test.go @@ -11,6 +11,8 @@ import ( dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" + "github.com/interuss/dss/pkg/rid/operations" + "github.com/interuss/dss/pkg/timestamp" "github.com/interuss/stacktrace" "github.com/stretchr/testify/require" "go.uber.org/zap" @@ -95,7 +97,7 @@ func (store *isaStore) CountISAs(ctx context.Context) (int64, error) { } func TestISAUpdateIdxCells(t *testing.T) { - ctx := context.Background() + ctx := timestamp.NewContext(t.Context(), fakeClock.Now()) app, cleanup := setUpISAApp(ctx, t) defer cleanup() @@ -119,7 +121,10 @@ func TestISAUpdateIdxCells(t *testing.T) { // with the soon to be new version of the isa. both should increase their // notification index. - _, err = app.InsertSubscription(ctx, &ridmodels.Subscription{ + repo, err := app.store.Interact(ctx) + require.NoError(t, err) + + _, err = operations.InsertSubscription(ctx, repo, &ridmodels.Subscription{ ID: dssmodels.ID(uuid.New().String()), Owner: "owner", StartTime: &startTime, @@ -128,7 +133,7 @@ func TestISAUpdateIdxCells(t *testing.T) { }) require.NoError(t, err) - _, err = app.InsertSubscription(ctx, &ridmodels.Subscription{ + _, err = operations.InsertSubscription(ctx, repo, &ridmodels.Subscription{ ID: dssmodels.ID(uuid.New().String()), Owner: "owner", StartTime: &startTime, @@ -326,7 +331,7 @@ func TestUpdateISA(t *testing.T) { func TestAppDeleteISAs(t *testing.T) { var ( - ctx = context.Background() + ctx = timestamp.NewContext(t.Context(), fakeClock.Now()) app, cleanup = setUpISAApp(ctx, t) ) defer cleanup() @@ -334,7 +339,10 @@ func TestAppDeleteISAs(t *testing.T) { insertedSubscriptions := []*ridmodels.Subscription{} for _, r := range subscriptionsPool { sunscriptionCopy := *r.input - s1, err := app.InsertSubscription(ctx, &sunscriptionCopy) + repo, err := app.store.Interact(ctx) + require.NoError(t, err) + + s1, err := operations.InsertSubscription(ctx, repo, &sunscriptionCopy) require.NoError(t, err) require.NotNil(t, s1) require.Equal(t, 42, s1.NotificationIndex) diff --git a/pkg/rid/application/subscription.go b/pkg/rid/application/subscription.go index 706d58616..d13f6296c 100644 --- a/pkg/rid/application/subscription.go +++ b/pkg/rid/application/subscription.go @@ -25,9 +25,6 @@ const ( type SubscriptionApp interface { GetSubscription(ctx context.Context, id dssmodels.ID) (*ridmodels.Subscription, error) - // InsertSubscription inserts or updates an Subscription. - InsertSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) - // UpdateSubscription UpdateSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) @@ -51,46 +48,6 @@ func (a *app) SearchSubscriptionsByOwner(ctx context.Context, cells s2.CellUnion return repo.SearchSubscriptionsByOwner(ctx, cells, owner) } -func (a *app) InsertSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) { - // Validate and perhaps correct StartTime and EndTime. - if err := s.AdjustTimeRange(a.clock.Now(), nil); err != nil { - return nil, stacktrace.Propagate(err, "Unable to adjust time range") - } - var sub *ridmodels.Subscription - _, err := a.store.Transact(ctx, store.NewFuncOperation(func(ctx context.Context, repo repos.Repository) error { - - // ensure it doesn't exist yet - old, err := repo.GetSubscription(ctx, s.ID) - if err != nil { - return stacktrace.Propagate(err, "Error getting Subscription from repo") - } - if old != nil { - return stacktrace.NewErrorWithCode(dsserr.AlreadyExists, "Subscription %s already exists", s.ID) - } - - // Check the user hasn't created too many subscriptions in this area. - count, err := repo.MaxSubscriptionCountInCellsByOwner(ctx, s.Cells, s.Owner) - if err != nil { - a.logger.Error("Error fetching max subscription count", zap.Error(err)) - return stacktrace.Propagate(err, - "Failed to fetch subscription count, rejecting request") - } - if count >= maxSubscriptionsPerArea { - return stacktrace.Propagate( - stacktrace.NewErrorWithCode(dsserr.Exhausted, "Too many existing subscriptions in this area already"), - "%s had %d subscriptions in the area", s.Owner, count) - } - - sub, err = repo.InsertSubscription(ctx, s) - if err != nil { - return stacktrace.Propagate(err, "Error inserting Subscription into repo") - } - - return nil - })) - return sub, err -} - // InsertSubscription implements the App InsertSubscription method func (a *app) UpdateSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) { var sub *ridmodels.Subscription diff --git a/pkg/rid/application/subscription_test.go b/pkg/rid/application/subscription_test.go index 32317d5fe..57ecb1b51 100644 --- a/pkg/rid/application/subscription_test.go +++ b/pkg/rid/application/subscription_test.go @@ -172,13 +172,14 @@ func TestBadOwner(t *testing.T) { app, cleanup := setUpSubApp(ctx, t) defer cleanup() - sub := &ridmodels.Subscription{ + repo, err := app.store.Interact(ctx) + require.NoError(t, err) + + sub, err := repo.InsertSubscription(ctx, &ridmodels.Subscription{ ID: dssmodels.ID(uuid.New().String()), Owner: "orig Owner", Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, - } - - sub, err := app.InsertSubscription(ctx, sub) + }) require.NoError(t, err) // Test changing owner fails sub.Owner = "new bad owner" @@ -199,7 +200,10 @@ func TestSubscriptionUpdateCells(t *testing.T) { // library might try to Normalize (this is the name of the function) the Union // into a single cell. We don't support this currently, so let's make sure // this doesn't happen. - sub, err := app.InsertSubscription(ctx, &ridmodels.Subscription{ + repo, err := app.store.Interact(ctx) + require.NoError(t, err) + + sub, err := repo.InsertSubscription(ctx, &ridmodels.Subscription{ ID: dssmodels.ID(uuid.New().String()), Owner: owner, StartTime: &startTime, @@ -222,88 +226,6 @@ func TestSubscriptionUpdateCells(t *testing.T) { require.Len(t, subs, 1) } -func TestInsertSubscriptionsWithTimes(t *testing.T) { - ctx := context.Background() - app, cleanup := setUpSubApp(ctx, t) - defer cleanup() - - for _, r := range []struct { - name string - updateFromStartTime time.Time - updateFromEndTime time.Time - startTime time.Time - endTime time.Time - wantErr stacktrace.ErrorCode - wantStartTime time.Time - wantEndTime time.Time - }{ - { - name: "start-time-defaults-to-now", - endTime: fakeClock.Now().Add(time.Hour), - wantStartTime: fakeClock.Now(), - wantEndTime: fakeClock.Now().Add(time.Hour), - }, - { - name: "end-time-defaults-to-24h", - wantStartTime: fakeClock.Now(), - wantEndTime: fakeClock.Now().Add(24 * time.Hour), - }, - { - name: "start-time-in-the-past", - startTime: fakeClock.Now().Add(-6 * time.Minute), - endTime: fakeClock.Now().Add(time.Hour), - wantErr: dsserr.BadRequest, - }, - { - name: "start-time-slightly-in-the-past", - startTime: fakeClock.Now().Add(-4 * time.Minute), - endTime: fakeClock.Now().Add(time.Hour), - wantStartTime: fakeClock.Now().Add(-4 * time.Minute), - }, - { - name: "end-time-before-start-time", - startTime: fakeClock.Now().Add(20 * time.Minute), - endTime: fakeClock.Now().Add(10 * time.Minute), - wantErr: dsserr.BadRequest, - }, - } { - t.Run(r.name, func(t *testing.T) { - id := dssmodels.ID(uuid.New().String()) - owner := dssmodels.Owner(uuid.New().String()) - var version *dssmodels.Version - - s := &ridmodels.Subscription{ - ID: id, - Owner: owner, - Version: version, - Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, - } - if !r.startTime.IsZero() { - s.StartTime = &r.startTime - } - if !r.endTime.IsZero() { - s.EndTime = &r.endTime - } - sub, err := app.InsertSubscription(ctx, s) - - if r.wantErr == stacktrace.ErrorCode(0) { - require.NoError(t, err) - } else { - require.Equal(t, r.wantErr, stacktrace.GetCode(err)) - } - - if !r.wantStartTime.IsZero() { - require.NotNil(t, sub.StartTime) - require.Equal(t, r.wantStartTime.UTC().Truncate(time.Microsecond), (*sub.StartTime).UTC().Truncate(time.Microsecond)) - } - if !r.wantEndTime.IsZero() { - require.NotNil(t, sub.EndTime) - require.Equal(t, r.wantEndTime.UTC().Truncate(time.Microsecond), (*sub.EndTime).UTC().Truncate(time.Microsecond)) - } - }) - } -} - func TestUpdateSubscriptionsWithTimes(t *testing.T) { ctx := context.Background() app, cleanup := setUpSubApp(ctx, t) @@ -409,50 +331,3 @@ func TestUpdateSubscriptionsWithTimes(t *testing.T) { }) } } - -func TestInsertTooManySubscription(t *testing.T) { - var ( - ctx = context.Background() - app, cleanup = setUpSubApp(ctx, t) - ) - defer cleanup() - // Helper function that makes a subscription with a random ID, fixed owner, - // and provided cellIDs. - makeSubscription := func(cellIDs []uint64) *ridmodels.Subscription { - s := &ridmodels.Subscription{ - ID: dssmodels.ID(uuid.New().String()), - Owner: dssmodels.Owner("bob"), - StartTime: &startTime, - EndTime: &endTime, - Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, - } - - s.Cells = make(s2.CellUnion, len(cellIDs)) - for i, id := range cellIDs { - s.Cells[i] = s2.CellID(id) - } - return s - } - - // We should be able to insert 10 subscriptions without error. - for i := 0; i < 10; i++ { - ret, err := app.InsertSubscription(ctx, makeSubscription([]uint64{12494535901059219456, 12494535866699481088})) - require.NoError(t, err) - require.NotNil(t, &ret) - } - - // Inserting the 11th subscription will fail. - ret, err := app.InsertSubscription(ctx, makeSubscription([]uint64{12494535901059219456, 12494535866699481088})) - require.Equal(t, dsserr.Exhausted, stacktrace.GetCode(err)) - require.Nil(t, ret) - - // Inserting a subscription in a different cell will succeed. - ret, err = app.InsertSubscription(ctx, makeSubscription([]uint64{12494535832339742720})) - require.NoError(t, err) - require.NotNil(t, &ret) - - // Inserting a subscription that overlaps fail. - ret, err = app.InsertSubscription(ctx, makeSubscription([]uint64{12494535935418957824, 12494535866699481088})) - require.Equal(t, dsserr.Exhausted, stacktrace.GetCode(err)) - require.Nil(t, ret) -} diff --git a/pkg/rid/operations/subscription.go b/pkg/rid/operations/subscription.go index dee5fac7e..4c85bed75 100644 --- a/pkg/rid/operations/subscription.go +++ b/pkg/rid/operations/subscription.go @@ -6,12 +6,20 @@ import ( ridv1 "github.com/interuss/dss/pkg/api/ridv1" ridv2 "github.com/interuss/dss/pkg/api/ridv2" dsserr "github.com/interuss/dss/pkg/errors" + "github.com/interuss/dss/pkg/locality" dssmodels "github.com/interuss/dss/pkg/models" + ridmodels "github.com/interuss/dss/pkg/rid/models" + apiv1 "github.com/interuss/dss/pkg/rid/models/api/v1" + apiv2 "github.com/interuss/dss/pkg/rid/models/api/v2" "github.com/interuss/dss/pkg/rid/repos" dssstore "github.com/interuss/dss/pkg/store" + "github.com/interuss/dss/pkg/timestamp" "github.com/interuss/stacktrace" ) +// Defined in requirement DSS0030. +const maxSubscriptionsPerArea = 10 + func init() { Registry[ridv1.DeleteSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ Encode: dssstore.EncodeJSON, @@ -23,6 +31,16 @@ func init() { Decode: dssstore.DecodeJSON[*ridv2.DeleteSubscriptionRequest], Execute: executeDeleteSubscription, } + Registry[ridv1.CreateSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv1.CreateSubscriptionRequest], + Execute: executeInsertSubscription, + } + Registry[ridv2.CreateSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv2.CreateSubscriptionRequest], + Execute: executeInsertSubscription, + } } func executeDeleteSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { @@ -73,3 +91,92 @@ func executeDeleteSubscription(ctx context.Context, repo repos.Repository, reque } return ret, nil } + +func executeInsertSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { + var ( + rawID string + url string + clientID *string + extents *dssmodels.Volume4D + ) + + switch req := request.(type) { + case *ridv1.CreateSubscriptionRequest: + if req.Body.Callbacks.IdentificationServiceAreaUrl == nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required callbacks") + } + if len(req.Body.Extents.SpatialVolume.Footprint.Vertices) == 0 { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required extents") + } + e, err := apiv1.FromVolume4D(&req.Body.Extents) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)) + } + rawID, url, clientID, extents = string(req.Id), string(*req.Body.Callbacks.IdentificationServiceAreaUrl), req.Auth.ClientID, e + + case *ridv2.CreateSubscriptionRequest: + if req.Body.UssBaseUrl == "" { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required USS base URL") + } + e, err := apiv2.FromVolume4D(&req.Body.Extents) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)) + } + rawID, url, clientID, extents = string(req.Id), string(req.Body.UssBaseUrl), req.Auth.ClientID, e + + default: + return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, ridv2.CreateSubscriptionOperationID) + } + + id, err := dssmodels.IDFromString(rawID) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format") + } + + sub := &ridmodels.Subscription{ + ID: id, + Owner: dssmodels.Owner(*clientID), + URL: url, + Writer: locality.MustFromContext(ctx), + } + if err := sub.SetExtents(extents); err != nil { + return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid extents") + } + + return InsertSubscription(ctx, repo, sub) +} + +// InsertSubscription applies the business rules for inserting a new Subscription: it does not +// perform any request-format validation, which is the caller's responsibility. +func InsertSubscription(ctx context.Context, repo repos.Repository, sub *ridmodels.Subscription) (*ridmodels.Subscription, error) { + // Validate and perhaps correct StartTime and EndTime. + if err := sub.AdjustTimeRange(timestamp.MustFromContext(ctx), nil); err != nil { + return nil, stacktrace.Propagate(err, "Unable to adjust time range") + } + + // ensure it doesn't exist yet + old, err := repo.GetSubscription(ctx, sub.ID) + if err != nil { + return nil, stacktrace.Propagate(err, "Error getting Subscription from repo") + } + if old != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.AlreadyExists, "Subscription %s already exists", sub.ID) + } + + // Check the user hasn't created too many subscriptions in this area. + count, err := repo.MaxSubscriptionCountInCellsByOwner(ctx, sub.Cells, sub.Owner) + if err != nil { + return nil, stacktrace.Propagate(err, "Failed to fetch subscription count, rejecting request") + } + if count >= maxSubscriptionsPerArea { + return nil, stacktrace.Propagate( + stacktrace.NewErrorWithCode(dsserr.Exhausted, "Too many existing subscriptions in this area already"), + "%s had %d subscriptions in the area", sub.Owner, count) + } + + ret, err := repo.InsertSubscription(ctx, sub) + if err != nil { + return nil, stacktrace.Propagate(err, "Error inserting Subscription into repo") + } + return ret, nil +} diff --git a/pkg/rid/operations/subscription_test.go b/pkg/rid/operations/subscription_test.go new file mode 100644 index 000000000..21de8691d --- /dev/null +++ b/pkg/rid/operations/subscription_test.go @@ -0,0 +1,336 @@ +package operations + +import ( + "context" + "testing" + "time" + + "github.com/golang/geo/s2" + "github.com/google/uuid" + "github.com/interuss/dss/pkg/api" + restapi "github.com/interuss/dss/pkg/api/ridv1" + dsserr "github.com/interuss/dss/pkg/errors" + "github.com/interuss/dss/pkg/geo/testdata" + "github.com/interuss/dss/pkg/locality" + dssmodels "github.com/interuss/dss/pkg/models" + ridmodels "github.com/interuss/dss/pkg/rid/models" + "github.com/interuss/dss/pkg/timestamp" + "github.com/interuss/stacktrace" + "github.com/jonboulle/clockwork" + "github.com/stretchr/testify/require" +) + +var ( + fakeClock = clockwork.NewFakeClock() + startTime = fakeClock.Now().Add(-time.Minute) + endTime = fakeClock.Now().Add(time.Hour) +) + +func newTestContext() context.Context { + ctx := timestamp.NewContext(context.Background(), fakeClock.Now()) + return locality.NewContext(ctx, "test-locality") +} + +type fakeSubscriptionRepo struct { + subs map[dssmodels.ID]*ridmodels.Subscription +} + +func newFakeSubscriptionRepo() *fakeSubscriptionRepo { + return &fakeSubscriptionRepo{subs: make(map[dssmodels.ID]*ridmodels.Subscription)} +} + +func (r *fakeSubscriptionRepo) GetSubscription(_ context.Context, id dssmodels.ID) (*ridmodels.Subscription, error) { + if sub, ok := r.subs[id]; ok { + return sub, nil + } + return nil, nil +} + +func (r *fakeSubscriptionRepo) DeleteSubscription(_ context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) { + if sub, ok := r.subs[s.ID]; ok { + delete(r.subs, s.ID) + return sub, nil + } + return nil, nil +} + +func (r *fakeSubscriptionRepo) InsertSubscription(_ context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) { + storedCopy := *s + storedCopy.Version = dssmodels.VersionFromTime(time.Now()) + r.subs[s.ID] = &storedCopy + returnedCopy := storedCopy + return &returnedCopy, nil +} + +func (r *fakeSubscriptionRepo) UpdateSubscription(_ context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) { + storedCopy := *s + storedCopy.Version = dssmodels.VersionFromTime(time.Now()) + r.subs[s.ID] = &storedCopy + returnedCopy := storedCopy + return &returnedCopy, nil +} + +func (r *fakeSubscriptionRepo) SearchSubscriptions(_ context.Context, cells s2.CellUnion) ([]*ridmodels.Subscription, error) { + var subs []*ridmodels.Subscription + for _, s := range r.subs { + appended := false + for _, c1 := range s.Cells { + for _, c2 := range cells { + if c1 == c2 { + subs = append(subs, s) + appended = true + break + } + } + if appended { + break + } + } + } + return subs, nil +} + +func (r *fakeSubscriptionRepo) SearchSubscriptionsByOwner(ctx context.Context, cells s2.CellUnion, owner dssmodels.Owner) ([]*ridmodels.Subscription, error) { + var subs []*ridmodels.Subscription + res, err := r.SearchSubscriptions(ctx, cells) + if err != nil { + return nil, err + } + for _, s := range res { + if s.Owner == owner { + subs = append(subs, s) + } + } + return subs, nil +} + +func (r *fakeSubscriptionRepo) UpdateNotificationIdxsInCells(ctx context.Context, cells s2.CellUnion) ([]*ridmodels.Subscription, error) { + subs, err := r.SearchSubscriptions(ctx, cells) + if err != nil { + return nil, err + } + for i := range subs { + subs[i].NotificationIndex++ + } + return subs, nil +} + +func (r *fakeSubscriptionRepo) MaxSubscriptionCountInCellsByOwner(ctx context.Context, cells s2.CellUnion, owner dssmodels.Owner) (int, error) { + maxValue := 0 + subs, err := r.SearchSubscriptionsByOwner(ctx, cells, owner) + if err != nil { + return 0, err + } + + cellMap := make(map[s2.CellID]int) + for _, s := range subs { + for _, cid := range s.Cells { + cellMap[cid]++ + if cellMap[cid] > maxValue { + maxValue = cellMap[cid] + } + } + } + return maxValue, nil +} + +func (r *fakeSubscriptionRepo) ListExpiredSubscriptions(_ context.Context, _ string, _ time.Time) ([]*ridmodels.Subscription, error) { + return nil, nil +} + +func (r *fakeSubscriptionRepo) CountSubscriptions(_ context.Context) (int64, error) { + return int64(len(r.subs)), nil +} + +func (r *fakeSubscriptionRepo) GetISA(_ context.Context, _ dssmodels.ID, _ bool) (*ridmodels.IdentificationServiceArea, error) { + panic("not implemented") +} + +func (r *fakeSubscriptionRepo) DeleteISA(_ context.Context, _ *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { + panic("not implemented") +} + +func (r *fakeSubscriptionRepo) InsertISA(_ context.Context, _ *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { + panic("not implemented") +} + +func (r *fakeSubscriptionRepo) UpdateISA(_ context.Context, _ *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { + panic("not implemented") +} + +func (r *fakeSubscriptionRepo) SearchISAs(_ context.Context, _ s2.CellUnion, _ *time.Time, _ *time.Time) ([]*ridmodels.IdentificationServiceArea, error) { + panic("not implemented") +} + +func (r *fakeSubscriptionRepo) ListExpiredISAs(_ context.Context, _ string, _ time.Time) ([]*ridmodels.IdentificationServiceArea, error) { + panic("not implemented") +} + +func (r *fakeSubscriptionRepo) CountISAs(_ context.Context) (int64, error) { + panic("not implemented") +} + +func TestInsertSubscriptionsWithTimes(t *testing.T) { + repo := newFakeSubscriptionRepo() + + for _, r := range []struct { + name string + startTime time.Time + endTime time.Time + wantErr stacktrace.ErrorCode + wantStartTime time.Time + wantEndTime time.Time + }{ + { + name: "start-time-defaults-to-now", + endTime: fakeClock.Now().Add(time.Hour), + wantStartTime: fakeClock.Now(), + wantEndTime: fakeClock.Now().Add(time.Hour), + }, + { + name: "end-time-defaults-to-24h", + wantStartTime: fakeClock.Now(), + wantEndTime: fakeClock.Now().Add(24 * time.Hour), + }, + { + name: "start-time-in-the-past", + startTime: fakeClock.Now().Add(-6 * time.Minute), + endTime: fakeClock.Now().Add(time.Hour), + wantErr: dsserr.BadRequest, + }, + { + name: "start-time-slightly-in-the-past", + startTime: fakeClock.Now().Add(-4 * time.Minute), + endTime: fakeClock.Now().Add(time.Hour), + wantStartTime: fakeClock.Now().Add(-4 * time.Minute), + }, + { + name: "end-time-before-start-time", + startTime: fakeClock.Now().Add(20 * time.Minute), + endTime: fakeClock.Now().Add(10 * time.Minute), + wantErr: dsserr.BadRequest, + }, + } { + t.Run(r.name, func(t *testing.T) { + ctx := newTestContext() + id := dssmodels.ID(uuid.New().String()) + owner := dssmodels.Owner(uuid.New().String()) + + s := &ridmodels.Subscription{ + ID: id, + Owner: owner, + Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, + } + if !r.startTime.IsZero() { + s.StartTime = &r.startTime + } + if !r.endTime.IsZero() { + s.EndTime = &r.endTime + } + sub, err := InsertSubscription(ctx, repo, s) + + if r.wantErr == stacktrace.ErrorCode(0) { + require.NoError(t, err) + } else { + require.Equal(t, r.wantErr, stacktrace.GetCode(err)) + } + + if !r.wantStartTime.IsZero() { + require.NotNil(t, sub.StartTime) + require.Equal(t, r.wantStartTime.UTC().Truncate(time.Microsecond), (*sub.StartTime).UTC().Truncate(time.Microsecond)) + } + if !r.wantEndTime.IsZero() { + require.NotNil(t, sub.EndTime) + require.Equal(t, r.wantEndTime.UTC().Truncate(time.Microsecond), (*sub.EndTime).UTC().Truncate(time.Microsecond)) + } + }) + } +} + +func TestInsertTooManySubscription(t *testing.T) { + ctx := newTestContext() + repo := newFakeSubscriptionRepo() + + // Helper function that makes a subscription with a random ID, fixed owner, + // and provided cellIDs. + makeSubscription := func(cellIDs []uint64) *ridmodels.Subscription { + s := &ridmodels.Subscription{ + ID: dssmodels.ID(uuid.New().String()), + Owner: dssmodels.Owner("bob"), + StartTime: &startTime, + EndTime: &endTime, + } + + s.Cells = make(s2.CellUnion, len(cellIDs)) + for i, id := range cellIDs { + s.Cells[i] = s2.CellID(id) + } + return s + } + + // We should be able to insert 10 subscriptions without error. + for i := 0; i < 10; i++ { + ret, err := InsertSubscription(ctx, repo, makeSubscription([]uint64{12494535901059219456, 12494535866699481088})) + require.NoError(t, err) + require.NotNil(t, &ret) + } + + // Inserting the 11th subscription will fail. + ret, err := InsertSubscription(ctx, repo, makeSubscription([]uint64{12494535901059219456, 12494535866699481088})) + require.Equal(t, dsserr.Exhausted, stacktrace.GetCode(err)) + require.Nil(t, ret) + + // Inserting a subscription in a different cell will succeed. + ret, err = InsertSubscription(ctx, repo, makeSubscription([]uint64{12494535832339742720})) + require.NoError(t, err) + require.NotNil(t, &ret) + + // Inserting a subscription that overlaps fail. + ret, err = InsertSubscription(ctx, repo, makeSubscription([]uint64{12494535935418957824, 12494535866699481088})) + require.Equal(t, dsserr.Exhausted, stacktrace.GetCode(err)) + require.Nil(t, ret) +} + +func TestExecuteInsertSubscriptionValidatesRequest(t *testing.T) { + for _, r := range []struct { + name string + extents restapi.Volume4D + }{ + { + name: "missing-extents", + extents: restapi.Volume4D{}, + }, + { + name: "missing-extents-spatial-volume", + extents: restapi.Volume4D{ + SpatialVolume: restapi.Volume3D{}, + }, + }, + { + name: "missing-spatial-volume-footprint", + extents: restapi.Volume4D{ + SpatialVolume: restapi.Volume3D{ + Footprint: restapi.GeoPolygon{}, + }, + }, + }, + } { + t.Run(r.name, func(t *testing.T) { + ctx := newTestContext() + repo := newFakeSubscriptionRepo() + + req := &restapi.CreateSubscriptionRequest{ + Id: restapi.SubscriptionUUID(uuid.New().String()), + Body: &restapi.CreateSubscriptionParameters{ + Callbacks: restapi.SubscriptionCallbacks{IdentificationServiceAreaUrl: &testdata.CallbackURL}, + Extents: r.extents, + }, + Auth: api.AuthorizationResult{ClientID: &testdata.Owner}, + } + + ret, err := executeInsertSubscription(ctx, repo, req) + require.Equal(t, dsserr.BadRequest, stacktrace.GetCode(err)) + require.Nil(t, ret) + }) + } +} diff --git a/pkg/rid/server/v1/server_test.go b/pkg/rid/server/v1/server_test.go index 094417cba..39891147d 100644 --- a/pkg/rid/server/v1/server_test.go +++ b/pkg/rid/server/v1/server_test.go @@ -43,13 +43,6 @@ type mockApp struct { mock.Mock } -func (ma *mockApp) InsertSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) { - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - args := ma.Called(ctx, s) - return args.Get(0).(*ridmodels.Subscription), args.Error(1) -} - func (ma *mockApp) UpdateSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) { ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() @@ -202,43 +195,6 @@ func TestCreateSubscription(t *testing.T) { Cells: mustPolygonToCellIDs(&testdata.LoopPolygon), }, }, - { - name: "missing-extents", - id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), - callbacks: restapi.SubscriptionCallbacks{IdentificationServiceAreaUrl: &testdata.CallbackURL}, - appErr: dsserr.BadRequest, - wantErr: &respSet.Response400, - }, - { - name: "missing-extents-spatial-volume", - id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), - callbacks: restapi.SubscriptionCallbacks{IdentificationServiceAreaUrl: &testdata.CallbackURL}, - extents: restapi.Volume4D{}, - appErr: dsserr.BadRequest, - wantErr: &respSet.Response400, - }, - { - name: "missing-spatial-volume-footprint", - id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), - callbacks: restapi.SubscriptionCallbacks{IdentificationServiceAreaUrl: &testdata.CallbackURL}, - extents: restapi.Volume4D{ - SpatialVolume: restapi.Volume3D{}, - }, - appErr: dsserr.BadRequest, - wantErr: &respSet.Response400, - }, - { - name: "missing-spatial-volume-footprint", - id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), - callbacks: restapi.SubscriptionCallbacks{IdentificationServiceAreaUrl: &testdata.CallbackURL}, - extents: restapi.Volume4D{ - SpatialVolume: restapi.Volume3D{ - Footprint: restapi.GeoPolygon{}, - }, - }, - appErr: dsserr.BadRequest, - wantErr: &respSet.Response400, - }, { name: "missing-callbacks", id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), @@ -249,14 +205,15 @@ func TestCreateSubscription(t *testing.T) { } { t.Run(r.name, func(t *testing.T) { ma := &mockApp{} + ms := &mockStore{} if r.appErr == stacktrace.ErrorCode(0) { ma.On("SearchISAs", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return( []*ridmodels.IdentificationServiceArea(nil), nil) - ma.On("InsertSubscription", mock.Anything, r.wantSubscription).Return( + ms.On("Transact", mock.Anything, mock.Anything).Return( r.wantSubscription, nil, ) } - s := &Server{App: ma} + s := &Server{App: ma, Store: ms} respSet = s.CreateSubscription(context.Background(), &restapi.CreateSubscriptionRequest{ Id: restapi.SubscriptionUUID(r.id.String()), @@ -272,6 +229,7 @@ func TestCreateSubscription(t *testing.T) { require.NotNil(t, respSet.Response200) } require.True(t, ma.AssertExpectations(t)) + require.True(t, ms.AssertExpectations(t)) }) } } @@ -298,11 +256,13 @@ func TestCreateSubscriptionResponseIncludesISAs(t *testing.T) { } ma := &mockApp{} + ms := &mockStore{} ma.On("SearchISAs", mock.Anything, cells, mock.Anything, mock.Anything).Return(isas, nil) - ma.On("InsertSubscription", mock.Anything, sub).Return(sub, nil) + ms.On("Transact", mock.Anything, mock.Anything).Return(sub, nil) s := &Server{ - App: ma, + App: ma, + Store: ms, } respSet := s.CreateSubscription(context.Background(), &restapi.CreateSubscriptionRequest{ @@ -317,6 +277,7 @@ func TestCreateSubscriptionResponseIncludesISAs(t *testing.T) { }) require.NotNil(t, respSet.Response200) require.True(t, ma.AssertExpectations(t)) + require.True(t, ms.AssertExpectations(t)) require.Equal(t, []restapi.IdentificationServiceArea{ { diff --git a/pkg/rid/server/v1/subscription_handler.go b/pkg/rid/server/v1/subscription_handler.go index 03c931a9e..73e0158bb 100644 --- a/pkg/rid/server/v1/subscription_handler.go +++ b/pkg/rid/server/v1/subscription_handler.go @@ -140,38 +140,25 @@ func (s *Server) CreateSubscription(ctx context.Context, req *restapi.CreateSubs return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required extents"))}} } - extents, err := apiv1.FromVolume4D(&req.Body.Extents) + _, err := apiv1.FromVolume4D(&req.Body.Extents) if err != nil { return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)))}} } - id, err := dssmodels.IDFromString(string(req.Id)) + _, err = dssmodels.IDFromString(string(req.Id)) if err != nil { return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - if !s.AllowHTTPBaseUrls { - err = ridmodels.ValidateURL(string(*req.Body.Callbacks.IdentificationServiceAreaUrl)) + err := ridmodels.ValidateURL(string(*req.Body.Callbacks.IdentificationServiceAreaUrl)) if err != nil { return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate IdentificationServiceAreaUrl"))}} } } - sub := &ridmodels.Subscription{ - ID: id, - Owner: dssmodels.Owner(*req.Auth.ClientID), - URL: string(*req.Body.Callbacks.IdentificationServiceAreaUrl), - Writer: s.Locality, - } - - if err := sub.SetExtents(extents); err != nil { - return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid extents"))}} - } - - insertedSub, err := s.App.InsertSubscription(ctx, sub) + insertedSub, err := store.TransactWithResult[repos.Repository, *ridmodels.Subscription](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not insert Subscription") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -189,7 +176,7 @@ func (s *Server) CreateSubscription(ctx context.Context, req *restapi.CreateSubs } // Find ISAs that were in this subscription's area. - isas, err := s.App.SearchISAs(ctx, sub.Cells, nil, nil) + isas, err := s.App.SearchISAs(ctx, insertedSub.Cells, nil, nil) if err != nil { err = stacktrace.Propagate(err, "Could not search ISAs") if stacktrace.GetCode(err) == dsserr.BadRequest { diff --git a/pkg/rid/server/v2/subscription_handler.go b/pkg/rid/server/v2/subscription_handler.go index da7ca6431..9b598fb06 100644 --- a/pkg/rid/server/v2/subscription_handler.go +++ b/pkg/rid/server/v2/subscription_handler.go @@ -136,38 +136,25 @@ func (s *Server) CreateSubscription(ctx context.Context, req *restapi.CreateSubs return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required USS base URL"))}} } - extents, err := apiv2.FromVolume4D(&req.Body.Extents) + _, err := apiv2.FromVolume4D(&req.Body.Extents) if err != nil { return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)))}} } - id, err := dssmodels.IDFromString(string(req.Id)) + _, err = dssmodels.IDFromString(string(req.Id)) if err != nil { return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - if !s.AllowHTTPBaseUrls { - err = ridmodels.ValidateURL(string(req.Body.UssBaseUrl)) + err := ridmodels.ValidateURL(string(req.Body.UssBaseUrl)) if err != nil { return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate UssBaseUrl"))}} } } - sub := &ridmodels.Subscription{ - ID: id, - Owner: dssmodels.Owner(*req.Auth.ClientID), - URL: string(req.Body.UssBaseUrl), - Writer: s.Locality, - } - - if err := sub.SetExtents(extents); err != nil { - return restapi.CreateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid extents"))}} - } - - insertedSub, err := s.App.InsertSubscription(ctx, sub) + insertedSub, err := store.TransactWithResult[repos.Repository, *ridmodels.Subscription](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not insert Subscription") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -185,7 +172,7 @@ func (s *Server) CreateSubscription(ctx context.Context, req *restapi.CreateSubs } // Find ISAs that were in this subscription's area. - isas, err := s.App.SearchISAs(ctx, sub.Cells, nil, nil) + isas, err := s.App.SearchISAs(ctx, insertedSub.Cells, nil, nil) if err != nil { err = stacktrace.Propagate(err, "Could not search ISAs") if stacktrace.GetCode(err) == dsserr.BadRequest { From 30a2ff6583253611334d278382499a2d22247574 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Mon, 31 Aug 2026 10:45:13 +0200 Subject: [PATCH 15/17] [raft/rid] Extract UpdateSubscription and route Update through the store --- pkg/rid/application/subscription.go | 60 -------- pkg/rid/application/subscription_test.go | 176 ---------------------- pkg/rid/operations/subscription.go | 112 ++++++++++++++ pkg/rid/operations/subscription_test.go | 153 +++++++++++++++++++ pkg/rid/server/v1/subscription_handler.go | 28 ++-- pkg/rid/server/v2/subscription_handler.go | 28 ++-- 6 files changed, 285 insertions(+), 272 deletions(-) diff --git a/pkg/rid/application/subscription.go b/pkg/rid/application/subscription.go index d13f6296c..a2ef0f94c 100644 --- a/pkg/rid/application/subscription.go +++ b/pkg/rid/application/subscription.go @@ -4,18 +4,9 @@ import ( "context" "github.com/golang/geo/s2" - dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" - "github.com/interuss/dss/pkg/rid/repos" - "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" - "go.uber.org/zap" -) - -const ( - // Defined in requirement DSS0030. - maxSubscriptionsPerArea = 10 ) // SubscriptionApp provides the interface to the application logic for Subscription entities @@ -25,9 +16,6 @@ const ( type SubscriptionApp interface { GetSubscription(ctx context.Context, id dssmodels.ID) (*ridmodels.Subscription, error) - // UpdateSubscription - UpdateSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) - // SearchSubscriptionsByOwner returns all IdentificationServiceAreas ownded by "owner" in "cells". SearchSubscriptionsByOwner(ctx context.Context, cells s2.CellUnion, owner dssmodels.Owner) ([]*ridmodels.Subscription, error) } @@ -47,51 +35,3 @@ func (a *app) SearchSubscriptionsByOwner(ctx context.Context, cells s2.CellUnion } return repo.SearchSubscriptionsByOwner(ctx, cells, owner) } - -// InsertSubscription implements the App InsertSubscription method -func (a *app) UpdateSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) { - var sub *ridmodels.Subscription - - _, err := a.store.Transact(ctx, store.NewFuncOperation(func(ctx context.Context, repo repos.Repository) error { - old, err := repo.GetSubscription(ctx, s.ID) - switch { - case err != nil: - return stacktrace.Propagate(err, "Error getting Subscription from repo") - case old == nil: - // The user wants to update an existing subscription, but one wasn't found. - return stacktrace.NewErrorWithCode(dsserr.NotFound, "Subscription %s not found", s.ID.String()) - case !s.Version.Matches(old.Version): - // The user wants to update a subscription but the version doesn't match. - return stacktrace.Propagate( - stacktrace.NewErrorWithCode(dsserr.VersionMismatch, "Subscription version %s is not current", s.Version), - "Subscription currently at version %s but client specified %s", old.Version, s.Version) - case old.Owner != s.Owner: - return stacktrace.Propagate( - stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Subscription is owned by different client"), - "Subscription owned by %s, but %s attempted to update", old.Owner, s.Owner) - } - // Validate and perhaps correct StartTime and EndTime. - if err := s.AdjustTimeRange(a.clock.Now(), old); err != nil { - return stacktrace.Propagate(err, "Error adjusting time range") - } - - // Check the user hasn't created too many subscriptions in this area. - count, err := repo.MaxSubscriptionCountInCellsByOwner(ctx, s.Cells, s.Owner) - if err != nil { - a.logger.Error("Error fetching max subscription count", zap.Error(err)) - return stacktrace.Propagate(err, - "Failed to fetch subscription count, rejecting request") - } - if count >= maxSubscriptionsPerArea { - return stacktrace.Propagate( - stacktrace.NewErrorWithCode(dsserr.Exhausted, "Too many existing subscriptions in this area already"), - "%s had %d subscriptions in the area", s.Owner, count) - } - sub, err = repo.UpdateSubscription(ctx, s) - if err != nil { - return stacktrace.Propagate(err, "Error updating Subscription in repo") - } - return nil - })) - return sub, err -} diff --git a/pkg/rid/application/subscription_test.go b/pkg/rid/application/subscription_test.go index 57ecb1b51..98a138c08 100644 --- a/pkg/rid/application/subscription_test.go +++ b/pkg/rid/application/subscription_test.go @@ -2,17 +2,12 @@ package application import ( "context" - "testing" "time" "github.com/golang/geo/s2" "github.com/google/uuid" - dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" - "github.com/interuss/stacktrace" - "github.com/stretchr/testify/require" - "go.uber.org/zap" ) var ( @@ -52,12 +47,6 @@ var ( } ) -func setUpSubApp(ctx context.Context, t *testing.T) (*app, func()) { - l := zap.L() - transactor, cleanup := setUpStore(ctx, t, l) - return NewFromTransactor(transactor, l).(*app), cleanup -} - type subscriptionStore struct { subs map[dssmodels.ID]*ridmodels.Subscription } @@ -166,168 +155,3 @@ func (store *subscriptionStore) ListExpiredSubscriptions(ctx context.Context, wr func (store *subscriptionStore) CountSubscriptions(ctx context.Context) (int64, error) { return int64(len(store.subs)), nil } - -func TestBadOwner(t *testing.T) { - ctx := context.Background() - app, cleanup := setUpSubApp(ctx, t) - defer cleanup() - - repo, err := app.store.Interact(ctx) - require.NoError(t, err) - - sub, err := repo.InsertSubscription(ctx, &ridmodels.Subscription{ - ID: dssmodels.ID(uuid.New().String()), - Owner: "orig Owner", - Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, - }) - require.NoError(t, err) - // Test changing owner fails - sub.Owner = "new bad owner" - _, err = app.UpdateSubscription(ctx, sub) - require.Equal(t, dsserr.PermissionDenied, stacktrace.GetCode(err)) -} - -func TestSubscriptionUpdateCells(t *testing.T) { - ctx := context.Background() - owner := dssmodels.Owner("owner") - app, cleanup := setUpSubApp(ctx, t) - defer cleanup() - - // ensure that when we do an update, nothing in the s2 library joins multiple - // cells together at a lower level. - - // These 4 cells are fully encompassed by the parent cell, meaning the s2 - // library might try to Normalize (this is the name of the function) the Union - // into a single cell. We don't support this currently, so let's make sure - // this doesn't happen. - repo, err := app.store.Interact(ctx) - require.NoError(t, err) - - sub, err := repo.InsertSubscription(ctx, &ridmodels.Subscription{ - ID: dssmodels.ID(uuid.New().String()), - Owner: owner, - StartTime: &startTime, - EndTime: &endTime, - Cells: s2.CellUnion{17106221850767130624, 17106221885126868992, 17106221919486607360}, - }) - - require.NoError(t, err) - require.NotNil(t, sub) - - sub.Cells = s2.CellUnion{17106221953846345728} - - sub, err = app.UpdateSubscription(ctx, sub) - require.NoError(t, err) - require.NotNil(t, sub) - - subs, err := app.SearchSubscriptionsByOwner(ctx, sub.Cells, owner) - require.NoError(t, err) - require.NotNil(t, subs) - require.Len(t, subs, 1) -} - -func TestUpdateSubscriptionsWithTimes(t *testing.T) { - ctx := context.Background() - app, cleanup := setUpSubApp(ctx, t) - defer cleanup() - - for _, r := range []struct { - name string - updateFromStartTime time.Time - updateFromEndTime time.Time - startTime time.Time - endTime time.Time - wantErr stacktrace.ErrorCode - wantStartTime time.Time - wantEndTime time.Time - }{ - { - name: "updating-keeps-old-times", - updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), - updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), - wantStartTime: fakeClock.Now().Add(-6 * time.Hour), - wantEndTime: fakeClock.Now().Add(6 * time.Hour), - }, - { - name: "changing-start-time-to-past", - updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), - updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), - startTime: fakeClock.Now().Add(-3 * time.Hour), - wantErr: dsserr.BadRequest, - }, - { - name: "changing-start-time-to-future", - updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), - updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), - startTime: fakeClock.Now().Add(3 * time.Hour), - wantStartTime: fakeClock.Now().Add(3 * time.Hour), - wantEndTime: fakeClock.Now().Add(6 * time.Hour), - }, - { - name: "changing-end-time-to-future", - updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), - updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), - endTime: fakeClock.Now().Add(3 * time.Hour), - wantStartTime: fakeClock.Now().Add(-6 * time.Hour), - wantEndTime: fakeClock.Now().Add(3 * time.Hour), - }, - { - name: "changing-end-time-more-than-24h", - updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), - updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), - endTime: fakeClock.Now().Add(24 * time.Hour), - wantErr: dsserr.BadRequest, - }, - } { - t.Run(r.name, func(t *testing.T) { - var ( - id = dssmodels.ID(uuid.New().String()) - owner = dssmodels.Owner(uuid.New().String()) - version *dssmodels.Version - ) - - repo, err := app.store.Interact(ctx) - require.NoError(t, err) - - // Insert a pre-existing subscription to simulate updating from something. - existing, err := repo.InsertSubscription(ctx, &ridmodels.Subscription{ - ID: id, - Owner: owner, - StartTime: &r.updateFromStartTime, - EndTime: &r.updateFromEndTime, - Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, - }) - require.NoError(t, err) - version = existing.Version - - s := &ridmodels.Subscription{ - ID: id, - Owner: owner, - Version: version, - Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, - } - if !r.startTime.IsZero() { - s.StartTime = &r.startTime - } - if !r.endTime.IsZero() { - s.EndTime = &r.endTime - } - sub, err := app.UpdateSubscription(ctx, s) - - if r.wantErr == stacktrace.ErrorCode(0) { - require.NoError(t, err) - } else { - require.Equal(t, r.wantErr, stacktrace.GetCode(err)) - } - - if !r.wantStartTime.IsZero() { - require.NotNil(t, sub.StartTime) - require.Equal(t, r.wantStartTime.UTC().Truncate(time.Microsecond), (*sub.StartTime).UTC().Truncate(time.Microsecond)) - } - if !r.wantEndTime.IsZero() { - require.NotNil(t, sub.EndTime) - require.Equal(t, r.wantEndTime.UTC().Truncate(time.Microsecond), (*sub.EndTime).UTC().Truncate(time.Microsecond)) - } - }) - } -} diff --git a/pkg/rid/operations/subscription.go b/pkg/rid/operations/subscription.go index 4c85bed75..db1e57960 100644 --- a/pkg/rid/operations/subscription.go +++ b/pkg/rid/operations/subscription.go @@ -41,6 +41,16 @@ func init() { Decode: dssstore.DecodeJSON[*ridv2.CreateSubscriptionRequest], Execute: executeInsertSubscription, } + Registry[ridv1.UpdateSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv1.UpdateSubscriptionRequest], + Execute: executeUpdateSubscription, + } + Registry[ridv2.UpdateSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv2.UpdateSubscriptionRequest], + Execute: executeUpdateSubscription, + } } func executeDeleteSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { @@ -180,3 +190,105 @@ func InsertSubscription(ctx context.Context, repo repos.Repository, sub *ridmode } return ret, nil } + +func executeUpdateSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { + var ( + rawID string + rawVersion string + url string + clientID *string + extents *dssmodels.Volume4D + ) + + switch req := request.(type) { + case *ridv1.UpdateSubscriptionRequest: + if req.Body.Callbacks.IdentificationServiceAreaUrl == nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required callbacks") + } + if len(req.Body.Extents.SpatialVolume.Footprint.Vertices) == 0 { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required extents") + } + e, err := apiv1.FromVolume4D(&req.Body.Extents) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)) + } + rawID, rawVersion, url, clientID, extents = string(req.Id), req.Version, string(*req.Body.Callbacks.IdentificationServiceAreaUrl), req.Auth.ClientID, e + + case *ridv2.UpdateSubscriptionRequest: + if req.Body.UssBaseUrl == "" { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required USS base URL") + } + e, err := apiv2.FromVolume4D(&req.Body.Extents) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)) + } + rawID, rawVersion, url, clientID, extents = string(req.Id), req.Version, string(req.Body.UssBaseUrl), req.Auth.ClientID, e + + default: + return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, ridv2.UpdateSubscriptionOperationID) + } + + version, err := dssmodels.VersionFromString(rawVersion) + if err != nil { + return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version") + } + id, err := dssmodels.IDFromString(rawID) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format") + } + + sub := &ridmodels.Subscription{ + ID: id, + Owner: dssmodels.Owner(*clientID), + URL: url, + Version: version, + Writer: locality.MustFromContext(ctx), + } + if err := sub.SetExtents(extents); err != nil { + return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid extents") + } + + return updateSubscription(ctx, repo, sub) +} + +func updateSubscription(ctx context.Context, repo repos.Repository, sub *ridmodels.Subscription) (*ridmodels.Subscription, error) { + old, err := repo.GetSubscription(ctx, sub.ID) + switch { + case err != nil: + return nil, stacktrace.Propagate(err, "Error getting Subscription from repo") + case old == nil: + // The user wants to update an existing subscription, but one wasn't found. + return nil, stacktrace.NewErrorWithCode(dsserr.NotFound, "Subscription %s not found", sub.ID.String()) + case !sub.Version.Matches(old.Version): + // The user wants to update a subscription but the version doesn't match. + return nil, stacktrace.Propagate( + stacktrace.NewErrorWithCode(dsserr.VersionMismatch, "Subscription version %s is not current", sub.Version), + "Subscription currently at version %s but client specified %s", old.Version, sub.Version) + case old.Owner != sub.Owner: + return nil, stacktrace.Propagate( + stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Subscription is owned by different client"), + "Subscription owned by %s, but %s attempted to update", old.Owner, sub.Owner) + } + + // Validate and perhaps correct StartTime and EndTime. + if err := sub.AdjustTimeRange(timestamp.MustFromContext(ctx), old); err != nil { + return nil, stacktrace.Propagate(err, "Error adjusting time range") + } + + // Check the user hasn't created too many subscriptions in this area. + count, err := repo.MaxSubscriptionCountInCellsByOwner(ctx, sub.Cells, sub.Owner) + if err != nil { + return nil, stacktrace.Propagate(err, "Failed to fetch subscription count, rejecting request") + } + if count >= maxSubscriptionsPerArea { + return nil, stacktrace.Propagate( + stacktrace.NewErrorWithCode(dsserr.Exhausted, "Too many existing subscriptions in this area already"), + "%s had %d subscriptions in the area", sub.Owner, count) + } + + ret, err := repo.UpdateSubscription(ctx, sub) + if err != nil { + return nil, stacktrace.Propagate(err, "Error updating Subscription in repo") + } + return ret, nil +} diff --git a/pkg/rid/operations/subscription_test.go b/pkg/rid/operations/subscription_test.go index 21de8691d..9e336f607 100644 --- a/pkg/rid/operations/subscription_test.go +++ b/pkg/rid/operations/subscription_test.go @@ -170,6 +170,59 @@ func (r *fakeSubscriptionRepo) CountISAs(_ context.Context) (int64, error) { panic("not implemented") } +func TestBadOwner(t *testing.T) { + ctx := newTestContext() + repo := newFakeSubscriptionRepo() + + sub := &ridmodels.Subscription{ + ID: dssmodels.ID(uuid.New().String()), + Owner: "orig Owner", + Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, + } + + sub, err := InsertSubscription(ctx, repo, sub) + require.NoError(t, err) + // Test changing owner fails + sub.Owner = "new bad owner" + _, err = updateSubscription(ctx, repo, sub) + require.Equal(t, dsserr.PermissionDenied, stacktrace.GetCode(err)) +} + +func TestSubscriptionUpdateCells(t *testing.T) { + ctx := newTestContext() + owner := dssmodels.Owner("owner") + repo := newFakeSubscriptionRepo() + + // ensure that when we do an update, nothing in the s2 library joins multiple + // cells together at a lower level. + + // These 4 cells are fully encompassed by the parent cell, meaning the s2 + // library might try to Normalize (this is the name of the function) the Union + // into a single cell. We don't support this currently, so let's make sure + // this doesn't happen. + sub, err := InsertSubscription(ctx, repo, &ridmodels.Subscription{ + ID: dssmodels.ID(uuid.New().String()), + Owner: owner, + StartTime: &startTime, + EndTime: &endTime, + Cells: s2.CellUnion{17106221850767130624, 17106221885126868992, 17106221919486607360}, + }) + + require.NoError(t, err) + require.NotNil(t, sub) + + sub.Cells = s2.CellUnion{17106221953846345728} + + sub, err = updateSubscription(ctx, repo, sub) + require.NoError(t, err) + require.NotNil(t, sub) + + subs, err := repo.SearchSubscriptionsByOwner(ctx, sub.Cells, owner) + require.NoError(t, err) + require.NotNil(t, subs) + require.Len(t, subs, 1) +} + func TestInsertSubscriptionsWithTimes(t *testing.T) { repo := newFakeSubscriptionRepo() @@ -247,6 +300,106 @@ func TestInsertSubscriptionsWithTimes(t *testing.T) { } } +func TestUpdateSubscriptionsWithTimes(t *testing.T) { + repo := newFakeSubscriptionRepo() + + for _, r := range []struct { + name string + updateFromStartTime time.Time + updateFromEndTime time.Time + startTime time.Time + endTime time.Time + wantErr stacktrace.ErrorCode + wantStartTime time.Time + wantEndTime time.Time + }{ + { + name: "updating-keeps-old-times", + updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), + updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), + wantStartTime: fakeClock.Now().Add(-6 * time.Hour), + wantEndTime: fakeClock.Now().Add(6 * time.Hour), + }, + { + name: "changing-start-time-to-past", + updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), + updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), + startTime: fakeClock.Now().Add(-3 * time.Hour), + wantErr: dsserr.BadRequest, + }, + { + name: "changing-start-time-to-future", + updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), + updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), + startTime: fakeClock.Now().Add(3 * time.Hour), + wantStartTime: fakeClock.Now().Add(3 * time.Hour), + wantEndTime: fakeClock.Now().Add(6 * time.Hour), + }, + { + name: "changing-end-time-to-future", + updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), + updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), + endTime: fakeClock.Now().Add(3 * time.Hour), + wantStartTime: fakeClock.Now().Add(-6 * time.Hour), + wantEndTime: fakeClock.Now().Add(3 * time.Hour), + }, + { + name: "changing-end-time-more-than-24h", + updateFromStartTime: fakeClock.Now().Add(-6 * time.Hour), + updateFromEndTime: fakeClock.Now().Add(6 * time.Hour), + endTime: fakeClock.Now().Add(24 * time.Hour), + wantErr: dsserr.BadRequest, + }, + } { + t.Run(r.name, func(t *testing.T) { + ctx := newTestContext() + var ( + id = dssmodels.ID(uuid.New().String()) + owner = dssmodels.Owner(uuid.New().String()) + ) + + // Insert a pre-existing subscription to simulate updating from something. + existing, err := repo.InsertSubscription(ctx, &ridmodels.Subscription{ + ID: id, + Owner: owner, + StartTime: &r.updateFromStartTime, + EndTime: &r.updateFromEndTime, + Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, + }) + require.NoError(t, err) + + s := &ridmodels.Subscription{ + ID: id, + Owner: owner, + Version: existing.Version, + Cells: s2.CellUnion{s2.CellID(17106221850767130624)}, + } + if !r.startTime.IsZero() { + s.StartTime = &r.startTime + } + if !r.endTime.IsZero() { + s.EndTime = &r.endTime + } + sub, err := updateSubscription(ctx, repo, s) + + if r.wantErr == stacktrace.ErrorCode(0) { + require.NoError(t, err) + } else { + require.Equal(t, r.wantErr, stacktrace.GetCode(err)) + } + + if !r.wantStartTime.IsZero() { + require.NotNil(t, sub.StartTime) + require.Equal(t, r.wantStartTime.UTC().Truncate(time.Microsecond), (*sub.StartTime).UTC().Truncate(time.Microsecond)) + } + if !r.wantEndTime.IsZero() { + require.NotNil(t, sub.EndTime) + require.Equal(t, r.wantEndTime.UTC().Truncate(time.Microsecond), (*sub.EndTime).UTC().Truncate(time.Microsecond)) + } + }) + } +} + func TestInsertTooManySubscription(t *testing.T) { ctx := newTestContext() repo := newFakeSubscriptionRepo() diff --git a/pkg/rid/server/v1/subscription_handler.go b/pkg/rid/server/v1/subscription_handler.go index 73e0158bb..c75a7294a 100644 --- a/pkg/rid/server/v1/subscription_handler.go +++ b/pkg/rid/server/v1/subscription_handler.go @@ -202,13 +202,12 @@ func (s *Server) CreateSubscription(ctx context.Context, req *restapi.CreateSubs // UpdateSubscription updates a single subscription. func (s *Server) UpdateSubscription(ctx context.Context, req *restapi.UpdateSubscriptionRequest, ) restapi.UpdateSubscriptionResponseSet { - - version, err := dssmodels.VersionFromString(req.Version) + _, err := dssmodels.VersionFromString(req.Version) if err != nil { return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version"))}} } - id, err := dssmodels.IDFromString(string(req.Id)) + _, err = dssmodels.IDFromString(string(req.Id)) if err != nil { return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} @@ -230,26 +229,19 @@ func (s *Server) UpdateSubscription(ctx context.Context, req *restapi.UpdateSubs return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required extents"))}} } - extents, err := apiv1.FromVolume4D(&req.Body.Extents) + _, err = apiv1.FromVolume4D(&req.Body.Extents) if err != nil { return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)))}} } - - sub := &ridmodels.Subscription{ - ID: id, - Owner: dssmodels.Owner(*req.Auth.ClientID), - URL: string(*req.Body.Callbacks.IdentificationServiceAreaUrl), - Version: version, - Writer: s.Locality, - } - - if err := sub.SetExtents(extents); err != nil { - return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid extents"))}} + if !s.AllowHTTPBaseUrls { + if err := ridmodels.ValidateURL(string(*req.Body.Callbacks.IdentificationServiceAreaUrl)); err != nil { + return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate IdentificationServiceAreaUrl"))}} + } } - insertedSub, err := s.App.UpdateSubscription(ctx, sub) + insertedSub, err := store.TransactWithResult[repos.Repository, *ridmodels.Subscription](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not update Subscription") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -269,7 +261,7 @@ func (s *Server) UpdateSubscription(ctx context.Context, req *restapi.UpdateSubs } // Find ISAs that were in this subscription's area. - isas, err := s.App.SearchISAs(ctx, sub.Cells, nil, nil) + isas, err := s.App.SearchISAs(ctx, insertedSub.Cells, nil, nil) if err != nil { err = stacktrace.Propagate(err, "Could not search ISAs") if stacktrace.GetCode(err) == dsserr.BadRequest { diff --git a/pkg/rid/server/v2/subscription_handler.go b/pkg/rid/server/v2/subscription_handler.go index 9b598fb06..a4ba319e4 100644 --- a/pkg/rid/server/v2/subscription_handler.go +++ b/pkg/rid/server/v2/subscription_handler.go @@ -198,13 +198,12 @@ func (s *Server) CreateSubscription(ctx context.Context, req *restapi.CreateSubs // UpdateSubscription updates a single subscription. func (s *Server) UpdateSubscription(ctx context.Context, req *restapi.UpdateSubscriptionRequest, ) restapi.UpdateSubscriptionResponseSet { - - version, err := dssmodels.VersionFromString(req.Version) + _, err := dssmodels.VersionFromString(req.Version) if err != nil { return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version"))}} } - id, err := dssmodels.IDFromString(string(req.Id)) + _, err = dssmodels.IDFromString(string(req.Id)) if err != nil { return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} @@ -222,26 +221,19 @@ func (s *Server) UpdateSubscription(ctx context.Context, req *restapi.UpdateSubs return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required USS base URL"))}} } - extents, err := apiv2.FromVolume4D(&req.Body.Extents) + _, err = apiv2.FromVolume4D(&req.Body.Extents) if err != nil { return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)))}} } - - sub := &ridmodels.Subscription{ - ID: id, - Owner: dssmodels.Owner(*req.Auth.ClientID), - URL: string(req.Body.UssBaseUrl), - Version: version, - Writer: s.Locality, - } - - if err := sub.SetExtents(extents); err != nil { - return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid extents"))}} + if !s.AllowHTTPBaseUrls { + if err := ridmodels.ValidateURL(string(req.Body.UssBaseUrl)); err != nil { + return restapi.UpdateSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ + Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate UssBaseUrl"))}} + } } - insertedSub, err := s.App.UpdateSubscription(ctx, sub) + insertedSub, err := store.TransactWithResult[repos.Repository, *ridmodels.Subscription](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not update Subscription") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -261,7 +253,7 @@ func (s *Server) UpdateSubscription(ctx context.Context, req *restapi.UpdateSubs } // Find ISAs that were in this subscription's area. - isas, err := s.App.SearchISAs(ctx, sub.Cells, nil, nil) + isas, err := s.App.SearchISAs(ctx, insertedSub.Cells, nil, nil) if err != nil { err = stacktrace.Propagate(err, "Could not search ISAs") if stacktrace.GetCode(err) == dsserr.BadRequest { From 6814f24ef2edb63ab383470f7507689c003862e9 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Mon, 31 Aug 2026 11:19:24 +0200 Subject: [PATCH 16/17] [raft/rid] Extract DeleteISA and route Delete through the store --- pkg/rid/application/isa.go | 40 ----------- pkg/rid/application/isa_test.go | 63 ----------------- pkg/rid/application/subscription_test.go | 38 ---------- pkg/rid/operations/isa.go | 90 ++++++++++++++++++++++++ pkg/rid/operations/isa_test.go | 70 ++++++++++++++++++ pkg/rid/operations/subscription_test.go | 51 ++++++++++---- pkg/rid/server/v1/isa_handler.go | 13 ++-- pkg/rid/server/v1/server_test.go | 38 +++++----- pkg/rid/server/v2/isa_handler.go | 13 ++-- 9 files changed, 232 insertions(+), 184 deletions(-) create mode 100644 pkg/rid/operations/isa.go create mode 100644 pkg/rid/operations/isa_test.go diff --git a/pkg/rid/application/isa.go b/pkg/rid/application/isa.go index a84170a4f..b00a5f126 100644 --- a/pkg/rid/application/isa.go +++ b/pkg/rid/application/isa.go @@ -20,10 +20,6 @@ import ( type ISAApp interface { GetISA(ctx context.Context, id dssmodels.ID) (*ridmodels.IdentificationServiceArea, error) - // DeleteISA deletes the IdentificationServiceArea identified by "id" and owned by "owner". - // Returns the delete IdentificationServiceArea and all Subscriptions affected by the delete. - DeleteISA(ctx context.Context, id dssmodels.ID, owner dssmodels.Owner, version *dssmodels.Version) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) - // InsertISA inserts or updates an ISA. InsertISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) @@ -57,42 +53,6 @@ func (a *app) SearchISAs(ctx context.Context, cells s2.CellUnion, earliest *time return repo.SearchISAs(ctx, cells, earliest, latest) } -// DeleteISA the given ISA -func (a *app) DeleteISA(ctx context.Context, id dssmodels.ID, owner dssmodels.Owner, version *dssmodels.Version) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) { - var ( - ret *ridmodels.IdentificationServiceArea - subs []*ridmodels.Subscription - ) - // The following will automatically retry TXN retry errors. - _, err := a.store.Transact(ctx, store.NewFuncOperation(func(ctx context.Context, repo repos.Repository) error { - old, err := repo.GetISA(ctx, id, true) - switch { - case err != nil: - return stacktrace.Propagate(err, "Error getting ISA") - case old == nil: - return stacktrace.NewErrorWithCode(dsserr.NotFound, "ISA %s not found", id.String()) - case !version.Matches(old.Version): - return stacktrace.NewErrorWithCode(dsserr.VersionMismatch, - "ISA currently at version %s but client specified %s", old.Version, version) - case old.Owner != owner: - return stacktrace.NewErrorWithCode(dsserr.PermissionDenied, - "ISA owned by %s, but %s attempted to delete", old.Owner, owner) - } - - ret, err = repo.DeleteISA(ctx, old) - if err != nil { - return stacktrace.Propagate(err, "Error deleting ISA") - } - - subs, err = repo.UpdateNotificationIdxsInCells(ctx, old.Cells) - if err != nil { - return stacktrace.Propagate(err, "Error updating notification indices") - } - return nil - })) - return ret, subs, err // No need to Propagate this error as this stack layer does not add useful information -} - // InsertISA implments the AppInterface InsertISA method func (a *app) InsertISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) { // Validate and perhaps correct StartTime and EndTime. diff --git a/pkg/rid/application/isa_test.go b/pkg/rid/application/isa_test.go index a63fc4388..202862ef1 100644 --- a/pkg/rid/application/isa_test.go +++ b/pkg/rid/application/isa_test.go @@ -328,66 +328,3 @@ func TestUpdateISA(t *testing.T) { }) } } - -func TestAppDeleteISAs(t *testing.T) { - var ( - ctx = timestamp.NewContext(t.Context(), fakeClock.Now()) - app, cleanup = setUpISAApp(ctx, t) - ) - defer cleanup() - - insertedSubscriptions := []*ridmodels.Subscription{} - for _, r := range subscriptionsPool { - sunscriptionCopy := *r.input - repo, err := app.store.Interact(ctx) - require.NoError(t, err) - - s1, err := operations.InsertSubscription(ctx, repo, &sunscriptionCopy) - require.NoError(t, err) - require.NotNil(t, s1) - require.Equal(t, 42, s1.NotificationIndex) - insertedSubscriptions = append(insertedSubscriptions, s1) - } - serviceArea := &ridmodels.IdentificationServiceArea{ - ID: dssmodels.ID(uuid.New().String()), - Owner: dssmodels.Owner(uuid.New().String()), - URL: "https://no/place/like/home/for/flights", - StartTime: &startTime, - EndTime: &endTime, - Cells: s2.CellUnion{ - s2.CellID(12494535935418957824), - }, - } - - // Insert the ISA. - serviceAreaCopy := *serviceArea - isa, subscriptionsOut, err := app.InsertISA(ctx, &serviceAreaCopy) - require.NoError(t, err) - require.NotNil(t, isa) - require.Len(t, subscriptionsOut, len(insertedSubscriptions)) - - for i := range insertedSubscriptions { - require.Equal(t, 43, subscriptionsOut[i].NotificationIndex) - } - // Can't delete with different owner. - _, _, err = app.DeleteISA(ctx, isa.ID, "bad-owner", isa.Version) - require.Error(t, err) - - // Delete the ISA. - // Ensure a fresh Get, then delete still updates the subscription indexes - isa, err = app.GetISA(ctx, isa.ID) - require.NoError(t, err) - - serviceAreaOut, subscriptionsOut, err := app.DeleteISA(ctx, isa.ID, isa.Owner, isa.Version) - require.NoError(t, err) - require.Equal(t, isa, serviceAreaOut) - require.NotNil(t, subscriptionsOut) - require.Len(t, subscriptionsOut, len(subscriptionsPool)) - for i, s := range subscriptionsPool { - require.Equal(t, s.input.URL, subscriptionsOut[i].URL) - } - - for i := range insertedSubscriptions { - require.Equal(t, 44, subscriptionsOut[i].NotificationIndex) - } -} diff --git a/pkg/rid/application/subscription_test.go b/pkg/rid/application/subscription_test.go index 98a138c08..60fb826cc 100644 --- a/pkg/rid/application/subscription_test.go +++ b/pkg/rid/application/subscription_test.go @@ -5,48 +5,10 @@ import ( "time" "github.com/golang/geo/s2" - "github.com/google/uuid" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" ) -var ( - // Ensure the struct conforms to the interface - _ SubscriptionApp = &app{} - subscriptionsPool = []struct { - name string - input *ridmodels.Subscription - }{ - { - name: "a subscription with startTime and endTime", - input: &ridmodels.Subscription{ - ID: dssmodels.ID(uuid.New().String()), - Owner: dssmodels.Owner(uuid.New().String()), - URL: "https://no/place/like/home", - StartTime: &startTime, - EndTime: &endTime, - NotificationIndex: 42, - Cells: s2.CellUnion{ - 12494535935418957824, - }, - }, - }, - { - name: "a subscription without startTime and with endTime", - input: &ridmodels.Subscription{ - ID: dssmodels.ID(uuid.New().String()), - Owner: dssmodels.Owner(uuid.New().String()), - URL: "https://no/place/like/home", - EndTime: &endTime, - NotificationIndex: 42, - Cells: s2.CellUnion{ - 12494535935418957824, - }, - }, - }, - } -) - type subscriptionStore struct { subs map[dssmodels.ID]*ridmodels.Subscription } diff --git a/pkg/rid/operations/isa.go b/pkg/rid/operations/isa.go new file mode 100644 index 000000000..f6a5eb8bf --- /dev/null +++ b/pkg/rid/operations/isa.go @@ -0,0 +1,90 @@ +package operations + +import ( + "context" + + ridv1 "github.com/interuss/dss/pkg/api/ridv1" + ridv2 "github.com/interuss/dss/pkg/api/ridv2" + dsserr "github.com/interuss/dss/pkg/errors" + dssmodels "github.com/interuss/dss/pkg/models" + ridmodels "github.com/interuss/dss/pkg/rid/models" + "github.com/interuss/dss/pkg/rid/repos" + dssstore "github.com/interuss/dss/pkg/store" + "github.com/interuss/stacktrace" +) + +// ISAResult bundles the affected ISA together with the relevant Subscriptions +type ISAResult struct { + ISA *ridmodels.IdentificationServiceArea + Subscriptions []*ridmodels.Subscription +} + +func init() { + Registry[ridv1.DeleteIdentificationServiceAreaOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv1.DeleteIdentificationServiceAreaRequest], + Execute: executeDeleteISA, + } + Registry[ridv2.DeleteIdentificationServiceAreaOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv2.DeleteIdentificationServiceAreaRequest], + Execute: executeDeleteISA, + } +} + +func executeDeleteISA(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { + var ( + rawID string + rawVersion string + clientID *string + ) + + switch req := request.(type) { + case *ridv1.DeleteIdentificationServiceAreaRequest: + rawID, rawVersion, clientID = string(req.Id), req.Version, req.Auth.ClientID + case *ridv2.DeleteIdentificationServiceAreaRequest: + rawID, rawVersion, clientID = string(req.Id), req.Version, req.Auth.ClientID + default: + return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, ridv2.DeleteIdentificationServiceAreaOperationID) + } + + version, err := dssmodels.VersionFromString(rawVersion) + if err != nil { + return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version") + } + id, err := dssmodels.IDFromString(rawID) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format") + } + owner := dssmodels.Owner(*clientID) + + return deleteISA(ctx, repo, id, owner, version) +} + +func deleteISA(ctx context.Context, repo repos.Repository, id dssmodels.ID, owner dssmodels.Owner, version *dssmodels.Version) (*ISAResult, error) { + old, err := repo.GetISA(ctx, id, true) + switch { + case err != nil: + return nil, stacktrace.Propagate(err, "Error getting ISA") + case old == nil: + return nil, stacktrace.NewErrorWithCode(dsserr.NotFound, "ISA %s not found", id.String()) + case !version.Matches(old.Version): + return nil, stacktrace.NewErrorWithCode(dsserr.VersionMismatch, + "ISA currently at version %s but client specified %s", old.Version, version) + case old.Owner != owner: + return nil, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, + "ISA owned by %s, but %s attempted to delete", old.Owner, owner) + } + + ret, err := repo.DeleteISA(ctx, old) + if err != nil { + return nil, stacktrace.Propagate(err, "Error deleting ISA") + } + + subs, err := repo.UpdateNotificationIdxsInCells(ctx, old.Cells) + if err != nil { + return nil, stacktrace.Propagate(err, "Error updating notification indices") + } + + return &ISAResult{ISA: ret, Subscriptions: subs}, nil +} diff --git a/pkg/rid/operations/isa_test.go b/pkg/rid/operations/isa_test.go new file mode 100644 index 000000000..2c74ce56c --- /dev/null +++ b/pkg/rid/operations/isa_test.go @@ -0,0 +1,70 @@ +package operations + +import ( + "testing" + + "github.com/golang/geo/s2" + "github.com/google/uuid" + dsserr "github.com/interuss/dss/pkg/errors" + dssmodels "github.com/interuss/dss/pkg/models" + ridmodels "github.com/interuss/dss/pkg/rid/models" + "github.com/interuss/stacktrace" + "github.com/stretchr/testify/require" +) + +func TestDeleteISA(t *testing.T) { + ctx := newTestContext() + repo := newFakeSubscriptionRepo() + + insertedSubscriptions := make([]*ridmodels.Subscription, 0, 2) + for range 2 { + s, err := InsertSubscription(ctx, repo, &ridmodels.Subscription{ + ID: dssmodels.ID(uuid.New().String()), + Owner: "owner", + URL: "https://no/place/like/home", + StartTime: &startTime, + EndTime: &endTime, + Cells: s2.CellUnion{12494535935418957824}, + }) + require.NoError(t, err) + insertedSubscriptions = append(insertedSubscriptions, s) + } + for _, s := range insertedSubscriptions { + require.Equal(t, 0, s.NotificationIndex) + } + + // Insert the ISA. + serviceArea := &ridmodels.IdentificationServiceArea{ + ID: dssmodels.ID(uuid.New().String()), + Owner: dssmodels.Owner(uuid.New().String()), + URL: "https://no/place/like/home/for/flights", + StartTime: &startTime, + EndTime: &endTime, + Cells: s2.CellUnion{12494535935418957824}, + } + insertSubs, err := repo.UpdateNotificationIdxsInCells(ctx, serviceArea.Cells) + require.NoError(t, err) + require.Len(t, insertSubs, len(insertedSubscriptions)) + for _, s := range insertSubs { + require.Equal(t, 1, s.NotificationIndex) + } + isa, err := repo.InsertISA(ctx, serviceArea) + require.NoError(t, err) + require.NotNil(t, isa) + + // Can't delete with different owner. + _, err = deleteISA(ctx, repo, isa.ID, "bad-owner", isa.Version) + require.Equal(t, dsserr.PermissionDenied, stacktrace.GetCode(err)) + + deleteResult, err := deleteISA(ctx, repo, isa.ID, isa.Owner, isa.Version) + require.NoError(t, err) + require.Equal(t, isa, deleteResult.ISA) + require.Len(t, deleteResult.Subscriptions, len(insertedSubscriptions)) + for _, s := range deleteResult.Subscriptions { + require.Equal(t, 2, s.NotificationIndex) + } + + // Deleting again fails since it no longer exists. + _, err = deleteISA(ctx, repo, isa.ID, isa.Owner, isa.Version) + require.Equal(t, dsserr.NotFound, stacktrace.GetCode(err)) +} diff --git a/pkg/rid/operations/subscription_test.go b/pkg/rid/operations/subscription_test.go index 9e336f607..3e4384e5c 100644 --- a/pkg/rid/operations/subscription_test.go +++ b/pkg/rid/operations/subscription_test.go @@ -31,12 +31,18 @@ func newTestContext() context.Context { return locality.NewContext(ctx, "test-locality") } +// fakeSubscriptionRepo is a minimal in-memory repos.Repository backing the operations +// package's tests. type fakeSubscriptionRepo struct { subs map[dssmodels.ID]*ridmodels.Subscription + isas map[dssmodels.ID]*ridmodels.IdentificationServiceArea } func newFakeSubscriptionRepo() *fakeSubscriptionRepo { - return &fakeSubscriptionRepo{subs: make(map[dssmodels.ID]*ridmodels.Subscription)} + return &fakeSubscriptionRepo{ + subs: make(map[dssmodels.ID]*ridmodels.Subscription), + isas: make(map[dssmodels.ID]*ridmodels.IdentificationServiceArea), + } } func (r *fakeSubscriptionRepo) GetSubscription(_ context.Context, id dssmodels.ID) (*ridmodels.Subscription, error) { @@ -142,24 +148,45 @@ func (r *fakeSubscriptionRepo) CountSubscriptions(_ context.Context) (int64, err return int64(len(r.subs)), nil } -func (r *fakeSubscriptionRepo) GetISA(_ context.Context, _ dssmodels.ID, _ bool) (*ridmodels.IdentificationServiceArea, error) { - panic("not implemented") +func (r *fakeSubscriptionRepo) GetISA(_ context.Context, id dssmodels.ID, _ bool) (*ridmodels.IdentificationServiceArea, error) { + if isa, ok := r.isas[id]; ok { + return isa, nil + } + return nil, nil } -func (r *fakeSubscriptionRepo) DeleteISA(_ context.Context, _ *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { - panic("not implemented") +func (r *fakeSubscriptionRepo) DeleteISA(_ context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { + if stored, ok := r.isas[isa.ID]; ok { + delete(r.isas, isa.ID) + return stored, nil + } + return nil, nil } -func (r *fakeSubscriptionRepo) InsertISA(_ context.Context, _ *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { - panic("not implemented") +func (r *fakeSubscriptionRepo) InsertISA(_ context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { + storedCopy := *isa + storedCopy.Version = dssmodels.VersionFromTime(time.Now()) + r.isas[isa.ID] = &storedCopy + returnedCopy := storedCopy + return &returnedCopy, nil } -func (r *fakeSubscriptionRepo) UpdateISA(_ context.Context, _ *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { - panic("not implemented") +func (r *fakeSubscriptionRepo) UpdateISA(_ context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, error) { + storedCopy := *isa + storedCopy.Version = dssmodels.VersionFromTime(time.Now()) + r.isas[isa.ID] = &storedCopy + returnedCopy := storedCopy + return &returnedCopy, nil } -func (r *fakeSubscriptionRepo) SearchISAs(_ context.Context, _ s2.CellUnion, _ *time.Time, _ *time.Time) ([]*ridmodels.IdentificationServiceArea, error) { - panic("not implemented") +func (r *fakeSubscriptionRepo) SearchISAs(_ context.Context, cells s2.CellUnion, _ *time.Time, _ *time.Time) ([]*ridmodels.IdentificationServiceArea, error) { + var isas []*ridmodels.IdentificationServiceArea + for _, isa := range r.isas { + if isa.Cells.Intersects(cells) { + isas = append(isas, isa) + } + } + return isas, nil } func (r *fakeSubscriptionRepo) ListExpiredISAs(_ context.Context, _ string, _ time.Time) ([]*ridmodels.IdentificationServiceArea, error) { @@ -167,7 +194,7 @@ func (r *fakeSubscriptionRepo) ListExpiredISAs(_ context.Context, _ string, _ ti } func (r *fakeSubscriptionRepo) CountISAs(_ context.Context) (int64, error) { - panic("not implemented") + return int64(len(r.isas)), nil } func TestBadOwner(t *testing.T) { diff --git a/pkg/rid/server/v1/isa_handler.go b/pkg/rid/server/v1/isa_handler.go index a43cea391..d2e107d1c 100644 --- a/pkg/rid/server/v1/isa_handler.go +++ b/pkg/rid/server/v1/isa_handler.go @@ -11,6 +11,9 @@ import ( dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" apiv1 "github.com/interuss/dss/pkg/rid/models/api/v1" + "github.com/interuss/dss/pkg/rid/operations" + "github.com/interuss/dss/pkg/rid/repos" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "github.com/pkg/errors" ) @@ -197,17 +200,17 @@ func (s *Server) DeleteIdentificationServiceArea(ctx context.Context, req *resta return restapi.DeleteIdentificationServiceAreaResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} } - version, err := dssmodels.VersionFromString(req.Version) + _, err := dssmodels.VersionFromString(req.Version) if err != nil { return restapi.DeleteIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version"))}} } - id, err := dssmodels.IDFromString(string(req.Id)) + _, err = dssmodels.IDFromString(string(req.Id)) if err != nil { return restapi.DeleteIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - isa, subscribers, err := s.App.DeleteISA(ctx, id, dssmodels.Owner(*req.Auth.ClientID), version) + result, err := store.TransactWithResult[repos.Repository, *operations.ISAResult](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not delete ISA") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -224,10 +227,10 @@ func (s *Server) DeleteIdentificationServiceArea(ctx context.Context, req *resta } } - apiSubscribers := apiv1.MakeSubscribersToNotify(subscribers) + apiSubscribers := apiv1.MakeSubscribersToNotify(result.Subscriptions) return restapi.DeleteIdentificationServiceAreaResponseSet{Response200: &restapi.DeleteIdentificationServiceAreaResponse{ - ServiceArea: *apiv1.ToIdentificationServiceArea(isa), + ServiceArea: *apiv1.ToIdentificationServiceArea(result.ISA), Subscribers: apiSubscribers, }} } diff --git a/pkg/rid/server/v1/server_test.go b/pkg/rid/server/v1/server_test.go index 39891147d..839e1bb9b 100644 --- a/pkg/rid/server/v1/server_test.go +++ b/pkg/rid/server/v1/server_test.go @@ -14,6 +14,7 @@ import ( dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" apiv1 "github.com/interuss/dss/pkg/rid/models/api/v1" + "github.com/interuss/dss/pkg/rid/operations" "github.com/interuss/dss/pkg/rid/repos" dssstore "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" @@ -91,13 +92,6 @@ func (ma *mockApp) GetISA(ctx context.Context, id dssmodels.ID) (*ridmodels.Iden return args.Get(0).(*ridmodels.IdentificationServiceArea), args.Error(1) } -func (ma *mockApp) DeleteISA(ctx context.Context, id dssmodels.ID, owner dssmodels.Owner, version *dssmodels.Version) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) { - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - args := ma.Called(ctx, id, owner, version) - return args.Get(0).(*ridmodels.IdentificationServiceArea), args.Get(1).([]*ridmodels.Subscription), args.Error(2) -} - func (ma *mockApp) InsertISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) { args := ma.Called(ctx, isa) return args.Get(0).(*ridmodels.IdentificationServiceArea), args.Get(1).([]*ridmodels.Subscription), args.Error(2) @@ -590,26 +584,28 @@ func TestDeleteIdentificationServiceAreaRequiresOwnerInContext(t *testing.T) { func TestDeleteIdentificationServiceArea(t *testing.T) { var ( id = dssmodels.ID(uuid.New().String()) - ma = &mockApp{} + ms = &mockStore{} s = &Server{ - App: ma, + Store: ms, } ) ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() - ma.On("DeleteISA", mock.Anything, id, dssmodels.Owner(testdata.Owner), mock.Anything).Return( - &ridmodels.IdentificationServiceArea{ - ID: id, - Owner: dssmodels.Owner("me-myself-and-i"), - URL: "https://no/place/like/home", - Version: testdata.Version, - }, - []*ridmodels.Subscription{ - { - NotificationIndex: 42, - URL: "https://no/place/like/home", + ms.On("Transact", mock.Anything, mock.Anything).Return( + &operations.ISAResult{ + ISA: &ridmodels.IdentificationServiceArea{ + ID: id, + Owner: dssmodels.Owner("me-myself-and-i"), + URL: "https://no/place/like/home", + Version: testdata.Version, + }, + Subscriptions: []*ridmodels.Subscription{ + { + NotificationIndex: 42, + URL: "https://no/place/like/home", + }, }, }, error(nil), ) @@ -620,7 +616,7 @@ func TestDeleteIdentificationServiceArea(t *testing.T) { require.NotNil(t, respSet.Response200) require.Len(t, respSet.Response200.Subscribers, 1) - require.True(t, ma.AssertExpectations(t)) + require.True(t, ms.AssertExpectations(t)) } func TestSearchIdentificationServiceAreas(t *testing.T) { diff --git a/pkg/rid/server/v2/isa_handler.go b/pkg/rid/server/v2/isa_handler.go index 115665120..b7053e33a 100644 --- a/pkg/rid/server/v2/isa_handler.go +++ b/pkg/rid/server/v2/isa_handler.go @@ -11,6 +11,9 @@ import ( dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" apiv2 "github.com/interuss/dss/pkg/rid/models/api/v2" + "github.com/interuss/dss/pkg/rid/operations" + "github.com/interuss/dss/pkg/rid/repos" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "github.com/pkg/errors" ) @@ -190,17 +193,17 @@ func (s *Server) DeleteIdentificationServiceArea(ctx context.Context, req *resta Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} } - version, err := dssmodels.VersionFromString(req.Version) + _, err := dssmodels.VersionFromString(req.Version) if err != nil { return restapi.DeleteIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version"))}} } - id, err := dssmodels.IDFromString(string(req.Id)) + _, err = dssmodels.IDFromString(string(req.Id)) if err != nil { return restapi.DeleteIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - isa, subscribers, err := s.App.DeleteISA(ctx, id, dssmodels.Owner(*req.Auth.ClientID), version) + result, err := store.TransactWithResult[repos.Repository, *operations.ISAResult](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not delete ISA") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -217,10 +220,10 @@ func (s *Server) DeleteIdentificationServiceArea(ctx context.Context, req *resta } } - apiSubscribers := apiv2.MakeSubscribersToNotify(subscribers) + apiSubscribers := apiv2.MakeSubscribersToNotify(result.Subscriptions) return restapi.DeleteIdentificationServiceAreaResponseSet{Response200: &restapi.DeleteIdentificationServiceAreaResponse{ - ServiceArea: *apiv2.ToIdentificationServiceArea(isa), + ServiceArea: *apiv2.ToIdentificationServiceArea(result.ISA), Subscribers: &apiSubscribers, }} } From 17a68f384e59d2206f1125043962f2e3d319c967 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Mon, 31 Aug 2026 11:19:45 +0200 Subject: [PATCH 17/17] [raft/rid] Extract InsertISA and route Create through the store --- pkg/rid/application/application_test.go | 3 - pkg/rid/application/isa.go | 41 ------- pkg/rid/application/isa_test.go | 141 ------------------------ pkg/rid/operations/isa.go | 98 ++++++++++++++++ pkg/rid/operations/isa_test.go | 91 +++++++++++++-- pkg/rid/server/v1/isa_handler.go | 24 +--- pkg/rid/server/v1/server_test.go | 52 +-------- pkg/rid/server/v2/isa_handler.go | 24 +--- 8 files changed, 198 insertions(+), 276 deletions(-) diff --git a/pkg/rid/application/application_test.go b/pkg/rid/application/application_test.go index 7ac8db967..4dabaa765 100644 --- a/pkg/rid/application/application_test.go +++ b/pkg/rid/application/application_test.go @@ -3,7 +3,6 @@ package application import ( "context" "testing" - "time" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" @@ -23,8 +22,6 @@ import ( var ( fakeClock = clockwork.NewFakeClock() - startTime = fakeClock.Now().Add(-time.Minute) - endTime = fakeClock.Now().Add(time.Hour) ) type mockRepo struct { diff --git a/pkg/rid/application/isa.go b/pkg/rid/application/isa.go index b00a5f126..e70bb7638 100644 --- a/pkg/rid/application/isa.go +++ b/pkg/rid/application/isa.go @@ -20,9 +20,6 @@ import ( type ISAApp interface { GetISA(ctx context.Context, id dssmodels.ID) (*ridmodels.IdentificationServiceArea, error) - // InsertISA inserts or updates an ISA. - InsertISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) - // UpdateISA UpdateISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) @@ -53,44 +50,6 @@ func (a *app) SearchISAs(ctx context.Context, cells s2.CellUnion, earliest *time return repo.SearchISAs(ctx, cells, earliest, latest) } -// InsertISA implments the AppInterface InsertISA method -func (a *app) InsertISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) { - // Validate and perhaps correct StartTime and EndTime. - if err := isa.AdjustTimeRange(a.clock.Now(), nil); err != nil { - return nil, nil, stacktrace.Propagate(err, "Error adjusting time range") - } - // Update the notification index for both cells removed and added. - var ( - ret *ridmodels.IdentificationServiceArea - subs []*ridmodels.Subscription - ) - // The following will automatically retry TXN retry errors. - _, err := a.store.Transact(ctx, store.NewFuncOperation(func(ctx context.Context, repo repos.Repository) error { - // ensure it doesn't exist yet - old, err := repo.GetISA(ctx, isa.ID, false) - if err != nil { - return stacktrace.Propagate(err, "Error getting ISA") - } - if old != nil { - return stacktrace.NewErrorWithCode(dsserr.AlreadyExists, "ISA %s already exists", isa.ID) - } - - // UpdateNotificationIdxsInCells is done in a Txn along with insert since - // they are both modifying the db. Insert a susbcription alone does - // not do this, so that does not need to use a txn (in subscription.go). - subs, err = repo.UpdateNotificationIdxsInCells(ctx, isa.Cells) - if err != nil { - return stacktrace.Propagate(err, "Error updating notification indices") - } - ret, err = repo.InsertISA(ctx, isa) - if err != nil { - return stacktrace.Propagate(err, "Error inserting ISA") - } - return nil - })) - return ret, subs, err // No need to Propagate this error as this stack layer does not add useful information -} - // UpdateISA implments the AppInterface UpdateISA method func (a *app) UpdateISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) { // Update the notification index for both cells removed and added. diff --git a/pkg/rid/application/isa_test.go b/pkg/rid/application/isa_test.go index 202862ef1..030ea2bd4 100644 --- a/pkg/rid/application/isa_test.go +++ b/pkg/rid/application/isa_test.go @@ -11,8 +11,6 @@ import ( dsserr "github.com/interuss/dss/pkg/errors" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" - "github.com/interuss/dss/pkg/rid/operations" - "github.com/interuss/dss/pkg/timestamp" "github.com/interuss/stacktrace" "github.com/stretchr/testify/require" "go.uber.org/zap" @@ -96,145 +94,6 @@ func (store *isaStore) CountISAs(ctx context.Context) (int64, error) { return int64(len(store.isas)), nil } -func TestISAUpdateIdxCells(t *testing.T) { - ctx := timestamp.NewContext(t.Context(), fakeClock.Now()) - app, cleanup := setUpISAApp(ctx, t) - - defer cleanup() - // ensure that when we do an update, nothing in the s2 library joins multiple - // cells together at a lower level. - - // These 4 cells are fully encompassed by the parent cell, meaning the s2 - // library might try to Normalize (this is the name of the function) the Union - // into a single cell. We don't support this currently, so let's make sure - // this doesn't happen. - isa, _, err := app.InsertISA(ctx, &ridmodels.IdentificationServiceArea{ - ID: dssmodels.ID(uuid.New().String()), - Owner: "owner", - StartTime: &startTime, - EndTime: &endTime, - Cells: s2.CellUnion{17106221850767130624, 17106221885126868992, 17106221919486607360}, - }) - require.NoError(t, err) - require.NotNil(t, isa) - // Now insert 2 subs, one overlaps with the original isa, and the second, overlaps - // with the soon to be new version of the isa. both should increase their - // notification index. - - repo, err := app.store.Interact(ctx) - require.NoError(t, err) - - _, err = operations.InsertSubscription(ctx, repo, &ridmodels.Subscription{ - ID: dssmodels.ID(uuid.New().String()), - Owner: "owner", - StartTime: &startTime, - EndTime: &endTime, - Cells: s2.CellUnion{17106221850767130624, 17106221919486607360}, - }) - require.NoError(t, err) - - _, err = operations.InsertSubscription(ctx, repo, &ridmodels.Subscription{ - ID: dssmodels.ID(uuid.New().String()), - Owner: "owner", - StartTime: &startTime, - EndTime: &endTime, - Cells: s2.CellUnion{17106221953846345728}, - }) - require.NoError(t, err) - - isa.Cells = s2.CellUnion{17106221953846345728} - - isa, subs, err := app.UpdateISA(ctx, isa) - require.NoError(t, err) - require.NotNil(t, isa) - require.Len(t, subs, 2) - for _, sub := range subs { - require.Equal(t, 1, sub.NotificationIndex) - } - - isas, err := app.SearchISAs(ctx, isa.Cells, &startTime, nil) - require.NoError(t, err) - require.NotNil(t, isas) - require.Len(t, isas, 1) -} - -func TestInsertISA(t *testing.T) { - ctx := context.Background() - app, cleanup := setUpISAApp(ctx, t) - - defer cleanup() - - for _, r := range []struct { - name string - startTime time.Time - endTime time.Time - wantErr stacktrace.ErrorCode - wantStartTime time.Time - wantEndTime time.Time - }{ - { - name: "missing-end-time", - wantErr: dsserr.BadRequest, - }, - { - name: "start-time-defaults-to-now", - endTime: fakeClock.Now().Add(time.Hour), - wantStartTime: fakeClock.Now(), - }, - { - name: "start-time-in-the-past", - startTime: fakeClock.Now().Add(-6 * time.Minute), - endTime: fakeClock.Now().Add(time.Hour), - wantErr: dsserr.BadRequest, - }, - { - name: "start-time-slightly-in-the-past", - startTime: fakeClock.Now().Add(-4 * time.Minute), - endTime: fakeClock.Now().Add(time.Hour), - wantStartTime: fakeClock.Now().Add(-4 * time.Minute), - }, - { - name: "end-time-before-start-time", - startTime: fakeClock.Now().Add(20 * time.Minute), - endTime: fakeClock.Now().Add(10 * time.Minute), - wantErr: dsserr.BadRequest, - }, - } { - t.Run(r.name, func(t *testing.T) { - sa := &ridmodels.IdentificationServiceArea{ - ID: dssmodels.ID(uuid.New().String()), - Owner: dssmodels.Owner(uuid.New().String()), - Cells: s2.CellUnion{12494535935418957824}, - } - if !r.startTime.IsZero() { - sa.StartTime = &r.startTime - } - if !r.endTime.IsZero() { - sa.EndTime = &r.endTime - } - isa, _, err := app.InsertISA(ctx, sa) - - if r.wantErr == stacktrace.ErrorCode(0) { - require.NoError(t, err) - } else { - require.Equal(t, r.wantErr, stacktrace.GetCode(err)) - } - - if !r.wantStartTime.IsZero() { - require.NotNil(t, isa.StartTime) - // time.Time times are represented with loc==nil. The nil location means UTC. - // for test equality, it has to be explicitly converted to UTC. - // similar issue: https://github.com/golang/go/issues/19486 - require.Equal(t, r.wantStartTime.UTC().Truncate(time.Microsecond), (*isa.StartTime).UTC().Truncate(time.Microsecond)) - } - if !r.wantEndTime.IsZero() { - require.NotNil(t, isa.EndTime) - require.Equal(t, r.wantEndTime.UTC().Truncate(time.Microsecond), (*isa.EndTime).UTC().Truncate(time.Microsecond)) - } - }) - } -} - func TestUpdateISA(t *testing.T) { ctx := context.Background() app, cleanup := setUpISAApp(ctx, t) diff --git a/pkg/rid/operations/isa.go b/pkg/rid/operations/isa.go index f6a5eb8bf..91e7c0a3e 100644 --- a/pkg/rid/operations/isa.go +++ b/pkg/rid/operations/isa.go @@ -6,10 +6,14 @@ import ( ridv1 "github.com/interuss/dss/pkg/api/ridv1" ridv2 "github.com/interuss/dss/pkg/api/ridv2" dsserr "github.com/interuss/dss/pkg/errors" + "github.com/interuss/dss/pkg/locality" dssmodels "github.com/interuss/dss/pkg/models" ridmodels "github.com/interuss/dss/pkg/rid/models" + apiv1 "github.com/interuss/dss/pkg/rid/models/api/v1" + apiv2 "github.com/interuss/dss/pkg/rid/models/api/v2" "github.com/interuss/dss/pkg/rid/repos" dssstore "github.com/interuss/dss/pkg/store" + "github.com/interuss/dss/pkg/timestamp" "github.com/interuss/stacktrace" ) @@ -30,6 +34,16 @@ func init() { Decode: dssstore.DecodeJSON[*ridv2.DeleteIdentificationServiceAreaRequest], Execute: executeDeleteISA, } + Registry[ridv1.CreateIdentificationServiceAreaOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv1.CreateIdentificationServiceAreaRequest], + Execute: executeInsertISA, + } + Registry[ridv2.CreateIdentificationServiceAreaOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv2.CreateIdentificationServiceAreaRequest], + Execute: executeInsertISA, + } } func executeDeleteISA(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { @@ -88,3 +102,87 @@ func deleteISA(ctx context.Context, repo repos.Repository, id dssmodels.ID, owne return &ISAResult{ISA: ret, Subscriptions: subs}, nil } + +func executeInsertISA(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { + var ( + rawID string + url string + clientID *string + extents *dssmodels.Volume4D + ) + + switch req := request.(type) { + case *ridv1.CreateIdentificationServiceAreaRequest: + if req.Body.FlightsUrl == "" { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required flightsURL") + } + if len(req.Body.Extents.SpatialVolume.Footprint.Vertices) == 0 { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing or malformed required extents") + } + requestExtents, err := apiv1.FromVolume4D(&req.Body.Extents) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)) + } + rawID, url, clientID, extents = string(req.Id), string(req.Body.FlightsUrl), req.Auth.ClientID, requestExtents + + case *ridv2.CreateIdentificationServiceAreaRequest: + if req.Body.UssBaseUrl == "" { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required USS base URL") + } + requestExtents, err := apiv2.FromVolume4D(&req.Body.Extents) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)) + } + rawID, url, clientID, extents = string(req.Id), string(req.Body.UssBaseUrl), req.Auth.ClientID, requestExtents + + default: + return nil, stacktrace.NewError("unexpected request type %T for operation %q", request, ridv2.CreateIdentificationServiceAreaOperationID) + } + + id, err := dssmodels.IDFromString(rawID) + if err != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format") + } + + isa := &ridmodels.IdentificationServiceArea{ + ID: id, + Owner: dssmodels.Owner(*clientID), + URL: url, + Writer: locality.MustFromContext(ctx), + } + if err := isa.SetExtents(extents); err != nil { + return nil, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid extents") + } + + return insertISA(ctx, repo, isa) +} + +func insertISA(ctx context.Context, repo repos.Repository, isa *ridmodels.IdentificationServiceArea) (*ISAResult, error) { + // Validate and perhaps correct StartTime and EndTime. + if err := isa.AdjustTimeRange(timestamp.MustFromContext(ctx), nil); err != nil { + return nil, stacktrace.Propagate(err, "Error adjusting time range") + } + + // ensure it doesn't exist yet + old, err := repo.GetISA(ctx, isa.ID, false) + if err != nil { + return nil, stacktrace.Propagate(err, "Error getting ISA") + } + if old != nil { + return nil, stacktrace.NewErrorWithCode(dsserr.AlreadyExists, "ISA %s already exists", isa.ID) + } + + // UpdateNotificationIdxsInCells is done in the same transaction as the insert since they + // are both modifying the store. + subs, err := repo.UpdateNotificationIdxsInCells(ctx, isa.Cells) + if err != nil { + return nil, stacktrace.Propagate(err, "Error updating notification indices") + } + + ret, err := repo.InsertISA(ctx, isa) + if err != nil { + return nil, stacktrace.Propagate(err, "Error inserting ISA") + } + + return &ISAResult{ISA: ret, Subscriptions: subs}, nil +} diff --git a/pkg/rid/operations/isa_test.go b/pkg/rid/operations/isa_test.go index 2c74ce56c..45e9294b5 100644 --- a/pkg/rid/operations/isa_test.go +++ b/pkg/rid/operations/isa_test.go @@ -2,6 +2,7 @@ package operations import ( "testing" + "time" "github.com/golang/geo/s2" "github.com/google/uuid" @@ -12,6 +13,81 @@ import ( "github.com/stretchr/testify/require" ) +func TestInsertISA(t *testing.T) { + ctx := newTestContext() + repo := newFakeSubscriptionRepo() + + for _, r := range []struct { + name string + startTime time.Time + endTime time.Time + wantErr stacktrace.ErrorCode + wantStartTime time.Time + wantEndTime time.Time + }{ + { + name: "missing-end-time", + wantErr: dsserr.BadRequest, + }, + { + name: "start-time-defaults-to-now", + endTime: fakeClock.Now().Add(time.Hour), + wantStartTime: fakeClock.Now(), + }, + { + name: "start-time-in-the-past", + startTime: fakeClock.Now().Add(-6 * time.Minute), + endTime: fakeClock.Now().Add(time.Hour), + wantErr: dsserr.BadRequest, + }, + { + name: "start-time-slightly-in-the-past", + startTime: fakeClock.Now().Add(-4 * time.Minute), + endTime: fakeClock.Now().Add(time.Hour), + wantStartTime: fakeClock.Now().Add(-4 * time.Minute), + }, + { + name: "end-time-before-start-time", + startTime: fakeClock.Now().Add(20 * time.Minute), + endTime: fakeClock.Now().Add(10 * time.Minute), + wantErr: dsserr.BadRequest, + }, + } { + t.Run(r.name, func(t *testing.T) { + sa := &ridmodels.IdentificationServiceArea{ + ID: dssmodels.ID(uuid.New().String()), + Owner: dssmodels.Owner(uuid.New().String()), + Cells: s2.CellUnion{12494535935418957824}, + } + if !r.startTime.IsZero() { + sa.StartTime = &r.startTime + } + if !r.endTime.IsZero() { + sa.EndTime = &r.endTime + } + result, err := insertISA(ctx, repo, sa) + + if r.wantErr == stacktrace.ErrorCode(0) { + require.NoError(t, err) + } else { + require.Equal(t, r.wantErr, stacktrace.GetCode(err)) + } + + if !r.wantStartTime.IsZero() { + require.NotNil(t, result.ISA.StartTime) + // time.Time times are represented with loc==nil. The nil location means UTC. + // for test equality, it has to be explicitly converted to UTC. + // similar issue: https://github.com/golang/go/issues/19486 + require.Equal(t, r.wantStartTime.UTC().Truncate(time.Microsecond), (*result.ISA.StartTime).UTC().Truncate(time.Microsecond)) + } + if !r.wantEndTime.IsZero() { + require.NotNil(t, result.ISA.EndTime) + require.Equal(t, r.wantEndTime.UTC().Truncate(time.Microsecond), (*result.ISA.EndTime).UTC().Truncate(time.Microsecond)) + } + }) + } +} + func TestDeleteISA(t *testing.T) { ctx := newTestContext() repo := newFakeSubscriptionRepo() @@ -34,23 +110,22 @@ func TestDeleteISA(t *testing.T) { } // Insert the ISA. - serviceArea := &ridmodels.IdentificationServiceArea{ + insertResult, err := insertISA(ctx, repo, &ridmodels.IdentificationServiceArea{ ID: dssmodels.ID(uuid.New().String()), Owner: dssmodels.Owner(uuid.New().String()), URL: "https://no/place/like/home/for/flights", StartTime: &startTime, EndTime: &endTime, Cells: s2.CellUnion{12494535935418957824}, - } - insertSubs, err := repo.UpdateNotificationIdxsInCells(ctx, serviceArea.Cells) + }) require.NoError(t, err) - require.Len(t, insertSubs, len(insertedSubscriptions)) - for _, s := range insertSubs { + require.NotNil(t, insertResult) + require.Len(t, insertResult.Subscriptions, len(insertedSubscriptions)) + for _, s := range insertResult.Subscriptions { require.Equal(t, 1, s.NotificationIndex) } - isa, err := repo.InsertISA(ctx, serviceArea) - require.NoError(t, err) - require.NotNil(t, isa) + + isa := insertResult.ISA // Can't delete with different owner. _, err = deleteISA(ctx, repo, isa.ID, "bad-owner", isa.Version) diff --git a/pkg/rid/server/v1/isa_handler.go b/pkg/rid/server/v1/isa_handler.go index d2e107d1c..2685df760 100644 --- a/pkg/rid/server/v1/isa_handler.go +++ b/pkg/rid/server/v1/isa_handler.go @@ -62,38 +62,26 @@ func (s *Server) CreateIdentificationServiceArea(ctx context.Context, req *resta return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing or malformed required extents"))}} } - extents, err := apiv1.FromVolume4D(&req.Body.Extents) + _, err := apiv1.FromVolume4D(&req.Body.Extents) if err != nil { return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)))}} } - id, err := dssmodels.IDFromString(string(req.Id)) + _, err = dssmodels.IDFromString(string(req.Id)) if err != nil { return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } if !s.AllowHTTPBaseUrls { - err = ridmodels.ValidateURL(string(req.Body.FlightsUrl)) + err := ridmodels.ValidateURL(string(req.Body.FlightsUrl)) if err != nil { return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate Flight URL"))}} } } - isa := &ridmodels.IdentificationServiceArea{ - ID: id, - URL: string(req.Body.FlightsUrl), - Owner: dssmodels.Owner(*req.Auth.ClientID), - Writer: s.Locality, - } - - if err := isa.SetExtents(extents); err != nil { - return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid extents"))}} - } - - insertedISA, subscribers, err := s.App.InsertISA(ctx, isa) + result, err := store.TransactWithResult[repos.Repository, *operations.ISAResult](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not insert ISA") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -108,10 +96,10 @@ func (s *Server) CreateIdentificationServiceArea(ctx context.Context, req *resta } } - apiSubscribers := apiv1.MakeSubscribersToNotify(subscribers) + apiSubscribers := apiv1.MakeSubscribersToNotify(result.Subscriptions) return restapi.CreateIdentificationServiceAreaResponseSet{Response200: &restapi.PutIdentificationServiceAreaResponse{ - ServiceArea: *apiv1.ToIdentificationServiceArea(insertedISA), + ServiceArea: *apiv1.ToIdentificationServiceArea(result.ISA), Subscribers: apiSubscribers, }} } diff --git a/pkg/rid/server/v1/server_test.go b/pkg/rid/server/v1/server_test.go index 839e1bb9b..ae6ec9ed7 100644 --- a/pkg/rid/server/v1/server_test.go +++ b/pkg/rid/server/v1/server_test.go @@ -92,11 +92,6 @@ func (ma *mockApp) GetISA(ctx context.Context, id dssmodels.ID) (*ridmodels.Iden return args.Get(0).(*ridmodels.IdentificationServiceArea), args.Error(1) } -func (ma *mockApp) InsertISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) { - args := ma.Called(ctx, isa) - return args.Get(0).(*ridmodels.IdentificationServiceArea), args.Get(1).([]*ridmodels.Subscription), args.Error(2) -} - func (ma *mockApp) UpdateISA(ctx context.Context, isa *ridmodels.IdentificationServiceArea) (*ridmodels.IdentificationServiceArea, []*ridmodels.Subscription, error) { args := ma.Called(ctx, isa) return args.Get(0).(*ridmodels.IdentificationServiceArea), args.Get(1).([]*ridmodels.Subscription), args.Error(2) @@ -413,43 +408,6 @@ func TestCreateISA(t *testing.T) { AltitudeLo: (*float32)(testdata.LoopVolume3D.AltitudeLo), }, }, - { - name: "missing-extents", - id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), - flightsURL: "https://testdummy.interuss.org/interuss/dss/pkg/geo/testdata/testdata", - appErr: dsserr.BadRequest, - wantErr: &respSet.Response400, - }, - { - name: "missing-extents-spatial-volume", - id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), - extents: restapi.Volume4D{}, - flightsURL: "https://testdummy.interuss.org/interuss/dss/pkg/geo/testdata/testdata", - appErr: dsserr.BadRequest, - wantErr: &respSet.Response400, - }, - { - name: "missing-spatial-volume-footprint", - id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), - extents: restapi.Volume4D{ - SpatialVolume: restapi.Volume3D{}, - }, - flightsURL: "https://testdummy.interuss.org/interuss/dss/pkg/geo/testdata/testdata", - appErr: dsserr.BadRequest, - wantErr: &respSet.Response400, - }, - { - name: "missing-spatial-volume-footprint", - id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), - extents: restapi.Volume4D{ - SpatialVolume: restapi.Volume3D{ - Footprint: restapi.GeoPolygon{}, - }, - }, - flightsURL: "https://testdummy.interuss.org/interuss/dss/pkg/geo/testdata/testdata", - appErr: dsserr.BadRequest, - wantErr: &respSet.Response400, - }, { name: "missing-flights-url", id: dssmodels.ID("4348c8e5-0b1c-43cf-9114-2e67a4532765"), @@ -459,13 +417,13 @@ func TestCreateISA(t *testing.T) { }, } { t.Run(r.name, func(t *testing.T) { - ma := &mockApp{} + ms := &mockStore{} if r.wantISA != nil { - ma.On("InsertISA", mock.Anything, r.wantISA).Return( - r.wantISA, []*ridmodels.Subscription(nil), nil) + ms.On("Transact", mock.Anything, mock.Anything).Return( + &operations.ISAResult{ISA: r.wantISA}, nil) } s := &Server{ - App: ma, + Store: ms, } respSet = s.CreateIdentificationServiceArea(context.Background(), &restapi.CreateIdentificationServiceAreaRequest{ @@ -481,7 +439,7 @@ func TestCreateISA(t *testing.T) { } else { require.NotNil(t, respSet.Response200) } - require.True(t, ma.AssertExpectations(t)) + require.True(t, ms.AssertExpectations(t)) }) } } diff --git a/pkg/rid/server/v2/isa_handler.go b/pkg/rid/server/v2/isa_handler.go index b7053e33a..359ce97c1 100644 --- a/pkg/rid/server/v2/isa_handler.go +++ b/pkg/rid/server/v2/isa_handler.go @@ -58,38 +58,26 @@ func (s *Server) CreateIdentificationServiceArea(ctx context.Context, req *resta return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Missing required USS base URL"))}} } - extents, err := apiv2.FromVolume4D(&req.Body.Extents) + _, err := apiv2.FromVolume4D(&req.Body.Extents) if err != nil { return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Error parsing Volume4D: %v", stacktrace.RootCause(err)))}} } - id, err := dssmodels.IDFromString(string(req.Id)) + _, err = dssmodels.IDFromString(string(req.Id)) if err != nil { return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } if !s.AllowHTTPBaseUrls { - err = ridmodels.ValidateURL(string(req.Body.UssBaseUrl)) + err := ridmodels.ValidateURL(string(req.Body.UssBaseUrl)) if err != nil { return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Failed to validate base URL"))}} } } - isa := &ridmodels.IdentificationServiceArea{ - ID: id, - URL: string(req.Body.UssBaseUrl), - Owner: dssmodels.Owner(*req.Auth.ClientID), - Writer: s.Locality, - } - - if err := isa.SetExtents(extents); err != nil { - return restapi.CreateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid extents"))}} - } - - insertedISA, subscribers, err := s.App.InsertISA(ctx, isa) + result, err := store.TransactWithResult[repos.Repository, *operations.ISAResult](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not insert ISA") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -104,10 +92,10 @@ func (s *Server) CreateIdentificationServiceArea(ctx context.Context, req *resta } } - apiSubscribers := apiv2.MakeSubscribersToNotify(subscribers) + apiSubscribers := apiv2.MakeSubscribersToNotify(result.Subscriptions) return restapi.CreateIdentificationServiceAreaResponseSet{Response200: &restapi.PutIdentificationServiceAreaResponse{ - ServiceArea: *apiv2.ToIdentificationServiceArea(insertedISA), + ServiceArea: *apiv2.ToIdentificationServiceArea(result.ISA), Subscribers: &apiSubscribers, }} }