[raft/rid] Extract delete subscription

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,

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]{}

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
+}

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
-}

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

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))
 		})
 	}
 }

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"))}}

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

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"))}}