From a46607518d5b5f3d317cd998e514339eb55d8b61 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Fri, 10 Jul 2026 23:56:28 +0200 Subject: [PATCH] [raft/rid] Extract delete subscription --- cmds/core-service/main.go | 2 + pkg/rid/actions/registry.go | 1 - pkg/rid/actions/subscription.go | 75 +++++++++++++++++++++++ pkg/rid/application/subscription.go | 34 ---------- pkg/rid/server/v1/server.go | 2 + pkg/rid/server/v1/server_test.go | 53 ++++++++++------ pkg/rid/server/v1/subscription_handler.go | 17 ++--- pkg/rid/server/v2/server.go | 2 + pkg/rid/server/v2/subscription_handler.go | 17 ++--- 9 files changed, 127 insertions(+), 76 deletions(-) create mode 100644 pkg/rid/actions/subscription.go diff --git a/cmds/core-service/main.go b/cmds/core-service/main.go index 33e9e5922..7ca00adfc 100644 --- a/cmds/core-service/main.go +++ b/cmds/core-service/main.go @@ -141,10 +141,12 @@ func createRIDServers(ctx context.Context, locality string, logger *zap.Logger) app := application.NewFromTransactor(ridStore, logger) return &rid_v1.Server{ + Store: ridStore, App: app, Locality: locality, AllowHTTPBaseUrls: *allowHTTPBaseUrls, }, &rid_v2.Server{ + Store: ridStore, App: app, Locality: locality, AllowHTTPBaseUrls: *allowHTTPBaseUrls, diff --git a/pkg/rid/actions/registry.go b/pkg/rid/actions/registry.go index 82c125a55..2cd22af51 100644 --- a/pkg/rid/actions/registry.go +++ b/pkg/rid/actions/registry.go @@ -6,5 +6,4 @@ import ( ) // Registry maps operation IDs to their handlers. -// TODO: implement var Registry = map[string]dssstore.OperationHandler[repos.Repository]{} diff --git a/pkg/rid/actions/subscription.go b/pkg/rid/actions/subscription.go new file mode 100644 index 000000000..e3537b402 --- /dev/null +++ b/pkg/rid/actions/subscription.go @@ -0,0 +1,75 @@ +package actions + +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" + "github.com/interuss/dss/pkg/rid/repos" + dssstore "github.com/interuss/dss/pkg/store" + "github.com/interuss/stacktrace" +) + +func init() { + Registry[ridv1.DeleteSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv1.DeleteSubscriptionRequest], + Execute: ExecuteDeleteSubscription, + } + Registry[ridv2.DeleteSubscriptionOperationID] = dssstore.OperationHandler[repos.Repository]{ + Encode: dssstore.EncodeJSON, + Decode: dssstore.DecodeJSON[*ridv2.DeleteSubscriptionRequest], + Execute: ExecuteDeleteSubscription, + } +} + +func ExecuteDeleteSubscription(ctx context.Context, repo repos.Repository, request dssstore.OperationRequest) (any, error) { + var ( + rawID string + rawVersion string + clientID *string + ) + + switch req := request.(type) { + case *ridv1.DeleteSubscriptionRequest: + rawID, rawVersion, clientID = string(req.Id), req.Version, req.Auth.ClientID + case *ridv2.DeleteSubscriptionRequest: + 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.DeleteSubscriptionOperationID) + } + + 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) + + old, err := repo.GetSubscription(ctx, id) + switch { + case err != nil: + return nil, stacktrace.Propagate(err, "Error getting Subscription from repo") + case old == nil: + return nil, stacktrace.NewErrorWithCode(dsserr.NotFound, "Subscription %s not found", id.String()) + case !version.Matches(old.Version): + return nil, stacktrace.Propagate( + stacktrace.NewErrorWithCode(dsserr.VersionMismatch, "Subscription version %s is not current", version), + "Subscription currently at version %s but client specified %s", old.Version, version) + case old.Owner != owner: + return nil, stacktrace.Propagate( + stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Subscription is owned by different client"), + "Subscription owned by %s, but %s attempted to delete", old.Owner, owner) + } + + ret, err := repo.DeleteSubscription(ctx, old) + if err != nil { + return nil, stacktrace.Propagate(err, "Error deleting Subscription from repo") + } + return ret, nil +} diff --git a/pkg/rid/application/subscription.go b/pkg/rid/application/subscription.go index 2e666adeb..706d58616 100644 --- a/pkg/rid/application/subscription.go +++ b/pkg/rid/application/subscription.go @@ -25,10 +25,6 @@ const ( type SubscriptionApp interface { GetSubscription(ctx context.Context, id dssmodels.ID) (*ridmodels.Subscription, error) - // DeleteSubscription deletes the Subscription identified by "id" and owned by "owner". - // Returns the delete Subscription and all IdentificationServiceAreas affected by the delete. - DeleteSubscription(ctx context.Context, id dssmodels.ID, owner dssmodels.Owner, version *dssmodels.Version) (*ridmodels.Subscription, error) - // InsertSubscription inserts or updates an Subscription. InsertSubscription(ctx context.Context, s *ridmodels.Subscription) (*ridmodels.Subscription, error) @@ -142,33 +138,3 @@ func (a *app) UpdateSubscription(ctx context.Context, s *ridmodels.Subscription) })) return sub, err } - -// DeleteSubscription deletes the Subscription identified by "id" and owned by "owner". -func (a *app) DeleteSubscription(ctx context.Context, id dssmodels.ID, owner dssmodels.Owner, version *dssmodels.Version) (*ridmodels.Subscription, error) { - var ret *ridmodels.Subscription - _, err := a.store.Transact(ctx, store.NewFuncOperation(func(ctx context.Context, repo repos.Repository) error { - var err error - old, err := repo.GetSubscription(ctx, id) - switch { - case err != nil: - return stacktrace.Propagate(err, "Error getting Subscription from repo") - case old == nil: - return stacktrace.NewErrorWithCode(dsserr.NotFound, "Subscription %s not found", id.String()) - case !version.Matches(old.Version): - return stacktrace.Propagate( - stacktrace.NewErrorWithCode(dsserr.VersionMismatch, "Subscription version %s is not current", version), - "Subscription currently at version %s but client specified %s", old.Version, version) - case old.Owner != owner: - return stacktrace.Propagate( - stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Subscription is owned by different client"), - "Subscription owned by %s, but %s attempted to delete", old.Owner, owner) - } - - ret, err = repo.DeleteSubscription(ctx, old) - if err != nil { - return stacktrace.Propagate(err, "Error deleting Subscription from repo") - } - return nil - })) - return ret, err -} diff --git a/pkg/rid/server/v1/server.go b/pkg/rid/server/v1/server.go index be5d3f06e..684dbd62c 100644 --- a/pkg/rid/server/v1/server.go +++ b/pkg/rid/server/v1/server.go @@ -2,10 +2,12 @@ package v1 import ( "github.com/interuss/dss/pkg/rid/application" + "github.com/interuss/dss/pkg/rid/store" ) // Server implements ridv1.Implementation. type Server struct { + Store store.Store App application.App Locality string AllowHTTPBaseUrls bool diff --git a/pkg/rid/server/v1/server_test.go b/pkg/rid/server/v1/server_test.go index 08e71137a..094417cba 100644 --- a/pkg/rid/server/v1/server_test.go +++ b/pkg/rid/server/v1/server_test.go @@ -14,6 +14,8 @@ 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/repos" + dssstore "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -62,11 +64,24 @@ func (ma *mockApp) GetSubscription(ctx context.Context, id dssmodels.ID) (*ridmo return args.Get(0).(*ridmodels.Subscription), args.Error(1) } -func (ma *mockApp) DeleteSubscription(ctx context.Context, id dssmodels.ID, owner dssmodels.Owner, version *dssmodels.Version) (*ridmodels.Subscription, error) { - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - args := ma.Called(ctx, id, owner, version) - return args.Get(0).(*ridmodels.Subscription), args.Error(1) +type mockStore struct { + mock.Mock +} + +func (ms *mockStore) Interact(ctx context.Context) (repos.Repository, error) { + args := ms.Called(ctx) + repo, _ := args.Get(0).(repos.Repository) + return repo, args.Error(1) +} + +func (ms *mockStore) Transact(ctx context.Context, request dssstore.OperationRequest) (any, error) { + args := ms.Called(ctx, request) + return args.Get(0), args.Error(1) +} + +func (ms *mockStore) Close() error { + args := ms.Called() + return args.Error(0) } func (ma *mockApp) SearchSubscriptionsByOwner(ctx context.Context, cells s2.CellUnion, owner dssmodels.Owner) ([]*ridmodels.Subscription, error) { @@ -114,37 +129,37 @@ func TestDeleteSubscription(t *testing.T) { id dssmodels.ID version *dssmodels.Version subscription *ridmodels.Subscription - appErr stacktrace.ErrorCode + storeErr stacktrace.ErrorCode wantErr **restapi.ErrorResponse }{ { - name: "subscription-is-returned-if-returned-from-app", + name: "subscription-is-returned-if-returned-from-store", id: dssmodels.ID(uuid.New().String()), version: testdata.Version, subscription: &ridmodels.Subscription{}, }, { - name: "error-is-returned-if-returned-from-app", - id: dssmodels.ID(uuid.New().String()), - version: testdata.Version, - appErr: dsserr.NotFound, - wantErr: &respSet.Response404, + name: "error-is-returned-if-returned-from-store", + id: dssmodels.ID(uuid.New().String()), + version: testdata.Version, + storeErr: dsserr.NotFound, + wantErr: &respSet.Response404, }, } { t.Run(r.name, func(t *testing.T) { - ma := &mockApp{} - if r.appErr == stacktrace.ErrorCode(0) { - ma.On("DeleteSubscription", mock.Anything, r.id, mock.Anything, r.version).Return( + ms := &mockStore{} + if r.storeErr == stacktrace.ErrorCode(0) { + ms.On("Transact", mock.Anything, mock.Anything).Return( r.subscription, nil, ) } else { - ma.On("DeleteSubscription", mock.Anything, r.id, mock.Anything, r.version).Return( - (*ridmodels.Subscription)(nil), stacktrace.NewErrorWithCode(r.appErr, "Expected error"), + ms.On("Transact", mock.Anything, mock.Anything).Return( + (*ridmodels.Subscription)(nil), stacktrace.NewErrorWithCode(r.storeErr, "Expected error"), ) } s := &Server{ - App: ma, + Store: ms, } respSet = s.DeleteSubscription(context.Background(), &restapi.DeleteSubscriptionRequest{ @@ -155,7 +170,7 @@ func TestDeleteSubscription(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/v1/subscription_handler.go b/pkg/rid/server/v1/subscription_handler.go index 96a6d8659..03c931a9e 100644 --- a/pkg/rid/server/v1/subscription_handler.go +++ b/pkg/rid/server/v1/subscription_handler.go @@ -10,6 +10,8 @@ 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/repos" + "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "github.com/pkg/errors" ) @@ -22,17 +24,8 @@ func (s *Server) DeleteSubscription(ctx context.Context, req *restapi.DeleteSubs return restapi.DeleteSubscriptionResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} } - version, err := dssmodels.VersionFromString(req.Version) - if err != nil { - return restapi.DeleteSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version"))}} - } - id, err := dssmodels.IDFromString(string(req.Id)) - if err != nil { - return restapi.DeleteSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} - } - subscription, err := s.App.DeleteSubscription(ctx, id, dssmodels.Owner(*req.Auth.ClientID), version) + + subscription, err := store.TransactWithResult[repos.Repository, *ridmodels.Subscription](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not delete Subscription") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -43,6 +36,8 @@ func (s *Server) DeleteSubscription(ctx context.Context, req *restapi.DeleteSubs return restapi.DeleteSubscriptionResponseSet{Response409: errResp} case dsserr.NotFound: return restapi.DeleteSubscriptionResponseSet{Response404: errResp} + case dsserr.BadRequest: + return restapi.DeleteSubscriptionResponseSet{Response400: errResp} default: return restapi.DeleteSubscriptionResponseSet{Response500: &api.InternalServerErrorBody{ ErrorMessage: *dsserr.Handle(ctx, stacktrace.Propagate(err, "Got an unexpected error"))}} diff --git a/pkg/rid/server/v2/server.go b/pkg/rid/server/v2/server.go index 3983580ba..83a198f3b 100644 --- a/pkg/rid/server/v2/server.go +++ b/pkg/rid/server/v2/server.go @@ -4,10 +4,12 @@ import ( "github.com/robfig/cron/v3" "github.com/interuss/dss/pkg/rid/application" + "github.com/interuss/dss/pkg/rid/store" ) // Server implements ridv2.Implementation. type Server struct { + Store store.Store App application.App Locality string AllowHTTPBaseUrls bool diff --git a/pkg/rid/server/v2/subscription_handler.go b/pkg/rid/server/v2/subscription_handler.go index 978518611..da7ca6431 100644 --- a/pkg/rid/server/v2/subscription_handler.go +++ b/pkg/rid/server/v2/subscription_handler.go @@ -10,6 +10,8 @@ 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/repos" + store "github.com/interuss/dss/pkg/store" "github.com/interuss/stacktrace" "github.com/pkg/errors" ) @@ -22,17 +24,8 @@ func (s *Server) DeleteSubscription(ctx context.Context, req *restapi.DeleteSubs return restapi.DeleteSubscriptionResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} } - version, err := dssmodels.VersionFromString(req.Version) - if err != nil { - return restapi.DeleteSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version"))}} - } - id, err := dssmodels.IDFromString(string(req.Id)) - if err != nil { - return restapi.DeleteSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ - Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} - } - subscription, err := s.App.DeleteSubscription(ctx, id, dssmodels.Owner(*req.Auth.ClientID), version) + + subscription, err := store.TransactWithResult[repos.Repository, *ridmodels.Subscription](ctx, s.Store, req) if err != nil { err = stacktrace.Propagate(err, "Could not delete Subscription") errResp := &restapi.ErrorResponse{Message: dsserr.Handle(ctx, err)} @@ -43,6 +36,8 @@ func (s *Server) DeleteSubscription(ctx context.Context, req *restapi.DeleteSubs return restapi.DeleteSubscriptionResponseSet{Response409: errResp} case dsserr.NotFound: return restapi.DeleteSubscriptionResponseSet{Response404: errResp} + case dsserr.BadRequest: + return restapi.DeleteSubscriptionResponseSet{Response400: errResp} default: return restapi.DeleteSubscriptionResponseSet{Response500: &api.InternalServerErrorBody{ ErrorMessage: *dsserr.Handle(ctx, stacktrace.Propagate(err, "Got an unexpected error"))}}