diff --git a/services/skus/controllers.go b/services/skus/controllers.go index 6874a10a3..7a7d90800 100644 --- a/services/skus/controllers.go +++ b/services/skus/controllers.go @@ -78,7 +78,7 @@ func Router( r.Method( http.MethodDelete, "/{orderID}", - metricsMwr("CancelOrder", NewCORSMwr(copts, http.MethodDelete)(authMwr(CancelOrder(svc)))), + metricsMwr("CancelOrder", NewCORSMwr(copts, http.MethodDelete)(authMwr(handleCancelOrderLegacy(svc)))), ) r.Method( @@ -340,30 +340,9 @@ func handleSetOrderTrialDays(svc *Service) handlers.AppHandler { }) } -// CancelOrder handles requests for cancelling orders. -func CancelOrder(service *Service) handlers.AppHandler { +func handleCancelOrderLegacy(service *Service) handlers.AppHandler { return handlers.AppHandler(func(w http.ResponseWriter, r *http.Request) *handlers.AppError { - ctx := r.Context() - orderID := &inputs.ID{} - - if err := inputs.DecodeAndValidateString(ctx, orderID, chi.URLParam(r, "orderID")); err != nil { - return handlers.ValidationError( - "Error validating request url parameter", - map[string]interface{}{"orderID": err.Error()}, - ) - } - - oid := *orderID.UUID() - - if err := service.validateOrderMerchantAndCaveats(ctx, oid); err != nil { - return handlers.WrapError(err, "Error validating auth merchant and caveats", http.StatusForbidden) - } - - if err := service.CancelOrderLegacy(oid); err != nil { - return handlers.WrapError(err, "Error retrieving the order", http.StatusInternalServerError) - } - - return handlers.RenderContent(ctx, struct{}{}, w, http.StatusOK) + return handlers.WrapError(model.Error("not implemented"), "not implemented", http.StatusNotImplemented) }) } @@ -730,10 +709,6 @@ func deleteOrderCreds(service *Service) handlers.AppHandler { return handlers.ValidationError("orderID", map[string]interface{}{"orderID": err.Error()}) } - if err := service.validateOrderMerchantAndCaveats(ctx, orderID); err != nil { - return handlers.WrapError(err, "Error validating auth merchant and caveats", http.StatusForbidden) - } - isSigned := r.URL.Query().Get("isSigned") == "true" if err := service.DeleteOrderCreds(ctx, orderID, isSigned); err != nil { switch { diff --git a/services/skus/datastore.go b/services/skus/datastore.go index 929d72f44..5aadadcab 100644 --- a/services/skus/datastore.go +++ b/services/skus/datastore.go @@ -40,12 +40,8 @@ type Datastore interface { // GetOrder by ID GetOrder(orderID uuid.UUID) (*Order, error) - // GetOrderByExternalID by the external id from the purchase vendor - GetOrderByExternalID(externalID string) (*Order, error) // UpdateOrder updates an order when it has been paid UpdateOrder(orderID uuid.UUID, status string) error - // UpdateOrderMetadata adds a key value pair to an order's metadata - UpdateOrderMetadata(orderID uuid.UUID, key string, value string) error // CreateTransaction creates a transaction CreateTransaction(orderID uuid.UUID, externalTransactionID string, status string, currency string, kind string, amount decimal.Decimal) (*Transaction, error) // UpdateTransaction creates a transaction @@ -71,9 +67,6 @@ type Datastore interface { CommitVote(ctx context.Context, vr VoteRecord, tx *sqlx.Tx) error MarkVoteErrored(ctx context.Context, vr VoteRecord, tx *sqlx.Tx) error InsertVote(ctx context.Context, vr VoteRecord) error - CheckExpiredCheckoutSession(uuid.UUID) (bool, string, error) - IsStripeSub(uuid.UUID) (bool, string, error) - GetOrderItem(ctx context.Context, itemID uuid.UUID) (*OrderItem, error) InsertOrderCredsTx(ctx context.Context, tx *sqlx.Tx, creds *OrderCreds) error GetOrderCreds(orderID uuid.UUID, isSigned bool) ([]OrderCreds, error) SendSigningRequest(ctx context.Context, signingRequestWriter SigningRequestWriter) error @@ -89,7 +82,6 @@ type Datastore interface { GetSigningOrderRequestOutboxByOrderItem(ctx context.Context, orderID, itemID uuid.UUID) ([]SigningOrderRequestOutbox, error) DeleteSigningOrderRequestOutboxByOrderTx(ctx context.Context, tx *sqlx.Tx, orderID uuid.UUID) error UpdateSigningOrderRequestOutboxTx(ctx context.Context, tx *sqlx.Tx, requestID uuid.UUID, completedAt time.Time) error - AppendOrderMetadata(context.Context, *uuid.UUID, string, string) error GetOutboxMovAvgDurationSeconds() (int64, error) } @@ -288,30 +280,6 @@ func (pg *Postgres) CreateOrder(ctx context.Context, dbi sqlx.ExtContext, oreq * return result, nil } -// GetOrderByExternalID returns an order by the external id from the purchase vendor. -func (pg *Postgres) GetOrderByExternalID(externalID string) (*Order, error) { - ctx := context.TODO() - dbi := pg.RawDB() - - result, err := pg.orderRepo.GetByExternalID(ctx, dbi, externalID) - if err != nil { - // Preserve the legacy behaviour. - // TODO: Propagate the sentinel error, and handle in the business logic properly. - if errors.Is(err, model.ErrOrderNotFound) { - return nil, nil - } - - return nil, err - } - - result.Items, err = pg.orderItemRepo.FindByOrderID(ctx, dbi, result.ID) - if err != nil { - return nil, err - } - - return result, nil -} - // GetOutboxMovAvgDurationSeconds - get the number of seconds it takes to clear the last 20 outbox messages func (pg *Postgres) GetOutboxMovAvgDurationSeconds() (int64, error) { statement := ` @@ -357,24 +325,6 @@ func (pg *Postgres) GetOrder(orderID uuid.UUID) (*Order, error) { return result, nil } -// GetOrderItem retrieves the order item for the given identifier. -// -// It returns sql.ErrNoRows if the item is not found. -func (pg *Postgres) GetOrderItem(ctx context.Context, itemID uuid.UUID) (*OrderItem, error) { - result, err := pg.orderItemRepo.Get(ctx, pg.RawDB(), itemID) - if err != nil { - // Preserve the legacy behaviour. - // TODO: Propagate the sentinel error, and handle in the business logic properly. - if errors.Is(err, model.ErrOrderItemNotFound) { - return nil, sql.ErrNoRows - } - - return nil, err - } - - return result, nil -} - // GetPagedMerchantTransactions - get a paginated list of transactions for a merchant func (pg *Postgres) GetPagedMerchantTransactions( ctx context.Context, merchantID uuid.UUID, pagination *inputs.Pagination) (*[]Transaction, int, error) { @@ -476,50 +426,6 @@ func (pg *Postgres) GetTransaction(externalTransactionID string) (*Transaction, return &transaction, nil } -// CheckExpiredCheckoutSession indicates whether a Stripe checkout session is expired with its id for the given orderID. -// -// TODO(pavelb): The boolean return value is unnecessary, and can be removed. -// If there is experied session, the session id is present. -// If there is no session, or it has not expired, the result is the same – no session id. -// It's the caller's responsibility (the business logic layer) to interpret the result. -func (pg *Postgres) CheckExpiredCheckoutSession(orderID uuid.UUID) (bool, string, error) { - ctx := context.TODO() - - sessID, err := pg.orderRepo.GetExpiredStripeCheckoutSessionID(ctx, pg.RawDB(), orderID) - if err != nil { - if errors.Is(err, model.ErrExpiredStripeCheckoutSessionIDNotFound) { - return false, "", nil - } - - return false, "", fmt.Errorf("failed to check expired state of checkout session: %w", err) - } - - if sessID == "" { - return false, "", nil - } - - return true, sessID, nil -} - -// IsStripeSub reports whether the order is associated with a stripe subscription, if true, subscription id is returned. -// -// TODO(pavelb): This is a piece of business logic that leaked to the storage layer. -// Also, it unsuccessfully mimics the Go comma, ok idiom – bool and string should be swapped. -// But that's not necessary. -// If metadata was found, but there was no stripeSubscriptionId, it's known not to be a Stripe order. -func (pg *Postgres) IsStripeSub(orderID uuid.UUID) (bool, string, error) { - ctx := context.TODO() - - data, err := pg.orderRepo.GetMetadata(ctx, pg.RawDB(), orderID) - if err != nil { - return false, "", err - } - - sid, ok := data["stripeSubscriptionId"].(string) - - return ok, sid, nil -} - // UpdateOrder updates the orders status. // // Deprecated: This method MUST NOT be used for Premium orders. @@ -890,15 +796,6 @@ func (pg *Postgres) InsertVote(ctx context.Context, vr VoteRecord) error { return nil } -// UpdateOrderMetadata sets the order's metadata to the key and value. -// -// Deprecated: This method is no longer used and should be deleted. -// -// TODO(pavelb): Remove this method as it's dangerous and must not be used. -func (pg *Postgres) UpdateOrderMetadata(orderID uuid.UUID, key string, value string) error { - return model.Error("UpdateOrderMetadata must not be used") -} - // TimeLimitedV2Creds represent all the type TimeLimitedV2Creds struct { OrderID uuid.UUID `json:"orderId"` @@ -1292,21 +1189,6 @@ func (pg *Postgres) InsertSignedOrderCredentialsTx(ctx context.Context, tx *sqlx return nil } -// AppendOrderMetadata appends the key and string value to an order's metadata. -func (pg *Postgres) AppendOrderMetadata(ctx context.Context, orderID *uuid.UUID, key, value string) error { - _, tx, rollback, commit, err := datastore.GetTx(ctx, pg) - if err != nil { - return err - } - defer rollback() - - if err := pg.orderRepo.AppendMetadata(ctx, tx, *orderID, key, value); err != nil { - return fmt.Errorf("error updating order metadata %s: %w", orderID, err) - } - - return commit() -} - // recordOrderPayment records payments for Auto Contribute and Search Captcha. // // Deprecated: This method MUST NOT be used for Premium orders. diff --git a/services/skus/datastore_test.go b/services/skus/datastore_test.go index b49bbb6f7..65c47c3aa 100644 --- a/services/skus/datastore_test.go +++ b/services/skus/datastore_test.go @@ -133,28 +133,6 @@ func TestGetPagedMerchantTransactions(t *testing.T) { } } -func (suite *PostgresTestSuite) TestGetOrderByExternalID() { - ctx := context.Background() - defer ctx.Done() - - // create an issuer and a paid order with one order item and a time limited v2 credential type. - ctx = context.WithValue(context.Background(), appctx.WhitelistSKUsCTXKey, []string{devFreeTimeLimitedV2}) - o1, _ := createOrderAndIssuer(suite.T(), ctx, suite.storage, devFreeTimeLimitedV2) - - // add the external id to metadata - err := suite.storage.AppendOrderMetadata(ctx, &o1.ID, "externalID", "my external id") - suite.Require().NoError(err) - - // test out get by external id - o2, err := suite.storage.GetOrderByExternalID("my external id") - suite.Require().NoError(err) - suite.Assert().NotNil(o2) - - if o2 != nil { - suite.Assert().Equal(o2.ID.String(), o1.ID.String()) - } -} - func (suite *PostgresTestSuite) TestGetTimeLimitedV2OrderCredsByOrder_Success() { env := os.Getenv("ENV") ctx := context.WithValue(context.Background(), appctx.EnvironmentCTXKey, env) diff --git a/services/skus/instrumented_datastore.go b/services/skus/instrumented_datastore.go index 4fd7c2632..7a7cf0bf3 100644 --- a/services/skus/instrumented_datastore.go +++ b/services/skus/instrumented_datastore.go @@ -44,20 +44,6 @@ func NewDatastoreWithPrometheus(base Datastore, instanceName string) DatastoreWi } } -// AppendOrderMetadata implements Datastore -func (_d DatastoreWithPrometheus) AppendOrderMetadata(ctx context.Context, up1 *uuid.UUID, s1 string, s2 string) (err error) { - _since := time.Now() - defer func() { - result := "ok" - if err != nil { - result = "error" - } - - datastoreDurationSummaryVec.WithLabelValues(_d.instanceName, "AppendOrderMetadata", result).Observe(time.Since(_since).Seconds()) - }() - return _d.base.AppendOrderMetadata(ctx, up1, s1, s2) -} - // BeginTx implements Datastore func (_d DatastoreWithPrometheus) BeginTx() (tp1 *sqlx.Tx, err error) { _since := time.Now() @@ -72,20 +58,6 @@ func (_d DatastoreWithPrometheus) BeginTx() (tp1 *sqlx.Tx, err error) { return _d.base.BeginTx() } -// CheckExpiredCheckoutSession implements Datastore -func (_d DatastoreWithPrometheus) CheckExpiredCheckoutSession(u1 uuid.UUID) (b1 bool, s1 string, err error) { - _since := time.Now() - defer func() { - result := "ok" - if err != nil { - result = "error" - } - - datastoreDurationSummaryVec.WithLabelValues(_d.instanceName, "CheckExpiredCheckoutSession", result).Observe(time.Since(_since).Seconds()) - }() - return _d.base.CheckExpiredCheckoutSession(u1) -} - // CommitVote implements Datastore func (_d DatastoreWithPrometheus) CommitVote(ctx context.Context, vr VoteRecord, tx *sqlx.Tx) (err error) { _since := time.Now() @@ -254,20 +226,6 @@ func (_d DatastoreWithPrometheus) GetOrder(orderID uuid.UUID) (op1 *Order, err e return _d.base.GetOrder(orderID) } -// GetOrderByExternalID implements Datastore -func (_d DatastoreWithPrometheus) GetOrderByExternalID(externalID string) (op1 *Order, err error) { - _since := time.Now() - defer func() { - result := "ok" - if err != nil { - result = "error" - } - - datastoreDurationSummaryVec.WithLabelValues(_d.instanceName, "GetOrderByExternalID", result).Observe(time.Since(_since).Seconds()) - }() - return _d.base.GetOrderByExternalID(externalID) -} - // GetOrderCreds implements Datastore func (_d DatastoreWithPrometheus) GetOrderCreds(orderID uuid.UUID, isSigned bool) (oa1 []OrderCreds, err error) { _since := time.Now() @@ -296,20 +254,6 @@ func (_d DatastoreWithPrometheus) GetOrderCredsByItemID(orderID uuid.UUID, itemI return _d.base.GetOrderCredsByItemID(orderID, itemID, isSigned) } -// GetOrderItem implements Datastore -func (_d DatastoreWithPrometheus) GetOrderItem(ctx context.Context, itemID uuid.UUID) (op1 *OrderItem, err error) { - _since := time.Now() - defer func() { - result := "ok" - if err != nil { - result = "error" - } - - datastoreDurationSummaryVec.WithLabelValues(_d.instanceName, "GetOrderItem", result).Observe(time.Since(_since).Seconds()) - }() - return _d.base.GetOrderItem(ctx, itemID) -} - // GetOutboxMovAvgDurationSeconds implements Datastore func (_d DatastoreWithPrometheus) GetOutboxMovAvgDurationSeconds() (i1 int64, err error) { _since := time.Now() @@ -548,20 +492,6 @@ func (_d DatastoreWithPrometheus) InsertVote(ctx context.Context, vr VoteRecord) return _d.base.InsertVote(ctx, vr) } -// IsStripeSub implements Datastore -func (_d DatastoreWithPrometheus) IsStripeSub(u1 uuid.UUID) (b1 bool, s1 string, err error) { - _since := time.Now() - defer func() { - result := "ok" - if err != nil { - result = "error" - } - - datastoreDurationSummaryVec.WithLabelValues(_d.instanceName, "IsStripeSub", result).Observe(time.Since(_since).Seconds()) - }() - return _d.base.IsStripeSub(u1) -} - // MarkVoteErrored implements Datastore func (_d DatastoreWithPrometheus) MarkVoteErrored(ctx context.Context, vr VoteRecord, tx *sqlx.Tx) (err error) { _since := time.Now() @@ -667,20 +597,6 @@ func (_d DatastoreWithPrometheus) UpdateOrder(orderID uuid.UUID, status string) return _d.base.UpdateOrder(orderID, status) } -// UpdateOrderMetadata implements Datastore -func (_d DatastoreWithPrometheus) UpdateOrderMetadata(orderID uuid.UUID, key string, value string) (err error) { - _since := time.Now() - defer func() { - result := "ok" - if err != nil { - result = "error" - } - - datastoreDurationSummaryVec.WithLabelValues(_d.instanceName, "UpdateOrderMetadata", result).Observe(time.Since(_since).Seconds()) - }() - return _d.base.UpdateOrderMetadata(orderID, key, value) -} - // UpdateSigningOrderRequestOutboxTx implements Datastore func (_d DatastoreWithPrometheus) UpdateSigningOrderRequestOutboxTx(ctx context.Context, tx *sqlx.Tx, requestID uuid.UUID, completedAt time.Time) (err error) { _since := time.Now() diff --git a/services/skus/key.go b/services/skus/key.go index a004f227a..96200bc9f 100644 --- a/services/skus/key.go +++ b/services/skus/key.go @@ -162,26 +162,6 @@ func merchantFromCtx(ctx context.Context) (string, error) { return merchant, nil } -// validateOrderMerchantAndCaveats checks that the current authentication of the request has -// permissions to this order by cross-checking the merchant and caveats in context. -func (s *Service) validateOrderMerchantAndCaveats(ctx context.Context, oid uuid.UUID) error { - merchant, err := merchantFromCtx(ctx) - if err != nil { - return err - } - - order, err := s.orderRepo.Get(ctx, s.Datastore.RawDB(), oid) - if err != nil { - return err - } - - if order.MerchantID != merchant { - return errMerchantMismatch - } - - return validateOrderCvt(order, caveatsFromCtx(ctx)) -} - // NewAuthMwr returns a handler that authorises requests via http signature or simple tokens. func NewAuthMwr(ks httpsignature.Keystore) func(http.Handler) http.Handler { merchantVerifier := httpsignature.ParameterizedKeystoreVerifier{ diff --git a/services/skus/key_test.go b/services/skus/key_test.go index 19a30623b..52386ca2c 100644 --- a/services/skus/key_test.go +++ b/services/skus/key_test.go @@ -1,9 +1,7 @@ package skus import ( - "context" "crypto" - "database/sql" "encoding/base64" "encoding/hex" "fmt" @@ -15,16 +13,13 @@ import ( sqlmock "github.com/DATA-DOG/go-sqlmock" "github.com/jmoiron/sqlx" - uuid "github.com/satori/go.uuid" "github.com/stretchr/testify/assert" - must "github.com/stretchr/testify/require" "github.com/brave-intl/bat-go/libs/cryptography" "github.com/brave-intl/bat-go/libs/datastore" "github.com/brave-intl/bat-go/libs/httpsignature" "github.com/brave-intl/bat-go/libs/middleware" - "github.com/brave-intl/bat-go/services/skus/model" "github.com/brave-intl/bat-go/services/skus/storage/repository" ) @@ -257,224 +252,3 @@ func TestMerchantSignedMiddleware(t *testing.T) { handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusOK, rr.Code, "request with merchant auth should succeed") } - -func TestValidateOrderMerchantAndCaveats(t *testing.T) { - type tcGiven struct { - orderID uuid.UUID - merch string - cvt map[string]string - repo *repository.MockOrder - } - - type testCase struct { - name string - given tcGiven - exp error - } - - tests := []testCase{ - { - name: "invalid_order", - given: tcGiven{ - orderID: uuid.Must(uuid.FromString("0fb1d6ba-5d39-4f69-830b-c92c4640c86e")), - merch: "brave.com", - - repo: &repository.MockOrder{ - FnGet: func(ctx context.Context, dbi sqlx.QueryerContext, id uuid.UUID) (*model.Order, error) { - return nil, model.ErrOrderNotFound - }, - }, - }, - exp: model.ErrOrderNotFound, - }, - - { - name: "merchant_no_caveats", - given: tcGiven{ - orderID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - merch: "brave.com", - - repo: &repository.MockOrder{ - FnGet: func(ctx context.Context, dbi sqlx.QueryerContext, id uuid.UUID) (*model.Order, error) { - result := &model.Order{ - ID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - Currency: "BAT", - MerchantID: "brave.com", - Location: datastore.NullString{ - NullString: sql.NullString{ - Valid: true, - String: "test.brave.com", - }, - }, - Status: "paid", - } - - return result, nil - }, - }, - }, - }, - - { - name: "incorrect_merchant", - given: tcGiven{ - orderID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - merch: "brave.software", - - repo: &repository.MockOrder{ - FnGet: func(ctx context.Context, dbi sqlx.QueryerContext, id uuid.UUID) (*model.Order, error) { - result := &model.Order{ - ID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - Currency: "BAT", - MerchantID: "brave.com", - Location: datastore.NullString{ - NullString: sql.NullString{ - Valid: true, - String: "test.brave.com", - }, - }, - Status: "paid", - } - - return result, nil - }, - }, - }, - exp: errMerchantMismatch, - }, - - { - name: "merchant_location", - given: tcGiven{ - orderID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - merch: "brave.com", - cvt: map[string]string{"location": "test.brave.com"}, - - repo: &repository.MockOrder{ - FnGet: func(ctx context.Context, dbi sqlx.QueryerContext, id uuid.UUID) (*model.Order, error) { - result := &model.Order{ - ID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - Currency: "BAT", - MerchantID: "brave.com", - Location: datastore.NullString{ - NullString: sql.NullString{ - Valid: true, - String: "test.brave.com", - }, - }, - Status: "paid", - } - - return result, nil - }, - }, - }, - }, - - { - name: "incorrect_location", - given: tcGiven{ - orderID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - merch: "brave.com", - cvt: map[string]string{"location": "test.brave.software"}, - - repo: &repository.MockOrder{ - FnGet: func(ctx context.Context, dbi sqlx.QueryerContext, id uuid.UUID) (*model.Order, error) { - result := &model.Order{ - ID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - Currency: "BAT", - MerchantID: "brave.com", - Location: datastore.NullString{ - NullString: sql.NullString{ - Valid: true, - String: "test.brave.com", - }, - }, - Status: "paid", - } - - return result, nil - }, - }, - }, - exp: errLocationMismatch, - }, - - { - name: "unexpected_sku", - given: tcGiven{ - orderID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - merch: "brave.com", - cvt: map[string]string{"location": "test.brave.com", "sku": "some_sku"}, - - repo: &repository.MockOrder{ - FnGet: func(ctx context.Context, dbi sqlx.QueryerContext, id uuid.UUID) (*model.Order, error) { - result := &model.Order{ - ID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - Currency: "BAT", - MerchantID: "brave.com", - Location: datastore.NullString{ - NullString: sql.NullString{ - Valid: true, - String: "test.brave.com", - }, - }, - Status: "paid", - } - - return result, nil - }, - }, - }, - exp: errUnexpectedSKUCvt, - }, - - { - name: "empty_order_location", - given: tcGiven{ - orderID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - merch: "brave.com", - cvt: map[string]string{"location": "test.brave.com"}, - - repo: &repository.MockOrder{ - FnGet: func(ctx context.Context, dbi sqlx.QueryerContext, id uuid.UUID) (*model.Order, error) { - result := &model.Order{ - ID: uuid.Must(uuid.FromString("056bf179-1c07-4787-bd36-db51a83ad139")), - Currency: "BAT", - MerchantID: "brave.com", - Status: "paid", - } - - return result, nil - }, - }, - }, - }, - } - - // Need a database instance in Datastore. - // Not using mocks (as the suppressed return value suggests). - dbi, _, err := sqlmock.New() - must.Equal(t, nil, err) - - ds := &Postgres{ - Postgres: datastore.Postgres{DB: sqlx.NewDb(dbi, "postgres")}, - } - - for i := range tests { - tc := tests[i] - - t.Run(tc.name, func(t *testing.T) { - ctx := context.WithValue(context.Background(), merchantCtxKey{}, tc.given.merch) - ctx = context.WithValue(ctx, caveatsCtxKey{}, tc.given.cvt) - - svc := &Service{ - Datastore: ds, - orderRepo: tc.given.repo, - } - - err := svc.validateOrderMerchantAndCaveats(ctx, tc.given.orderID) - assert.Equal(t, tc.exp, err) - }) - } -} diff --git a/services/skus/mockdatastore.go b/services/skus/mockdatastore.go index be7ea32dc..7fd1cd274 100644 --- a/services/skus/mockdatastore.go +++ b/services/skus/mockdatastore.go @@ -42,20 +42,6 @@ func (m *MockDatastore) EXPECT() *MockDatastoreMockRecorder { return m.recorder } -// AppendOrderMetadata mocks base method. -func (m *MockDatastore) AppendOrderMetadata(arg0 context.Context, arg1 *go_uuid.UUID, arg2, arg3 string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AppendOrderMetadata", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 -} - -// AppendOrderMetadata indicates an expected call of AppendOrderMetadata. -func (mr *MockDatastoreMockRecorder) AppendOrderMetadata(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AppendOrderMetadata", reflect.TypeOf((*MockDatastore)(nil).AppendOrderMetadata), arg0, arg1, arg2, arg3) -} - // BeginTx mocks base method. func (m *MockDatastore) BeginTx() (*sqlx.Tx, error) { m.ctrl.T.Helper() @@ -71,22 +57,6 @@ func (mr *MockDatastoreMockRecorder) BeginTx() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BeginTx", reflect.TypeOf((*MockDatastore)(nil).BeginTx)) } -// CheckExpiredCheckoutSession mocks base method. -func (m *MockDatastore) CheckExpiredCheckoutSession(arg0 go_uuid.UUID) (bool, string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CheckExpiredCheckoutSession", arg0) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(string) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 -} - -// CheckExpiredCheckoutSession indicates an expected call of CheckExpiredCheckoutSession. -func (mr *MockDatastoreMockRecorder) CheckExpiredCheckoutSession(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckExpiredCheckoutSession", reflect.TypeOf((*MockDatastore)(nil).CheckExpiredCheckoutSession), arg0) -} - // CommitVote mocks base method. func (m *MockDatastore) CommitVote(ctx context.Context, vr VoteRecord, tx *sqlx.Tx) error { m.ctrl.T.Helper() @@ -263,21 +233,6 @@ func (mr *MockDatastoreMockRecorder) GetOrder(orderID interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrder", reflect.TypeOf((*MockDatastore)(nil).GetOrder), orderID) } -// GetOrderByExternalID mocks base method. -func (m *MockDatastore) GetOrderByExternalID(externalID string) (*Order, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetOrderByExternalID", externalID) - ret0, _ := ret[0].(*Order) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetOrderByExternalID indicates an expected call of GetOrderByExternalID. -func (mr *MockDatastoreMockRecorder) GetOrderByExternalID(externalID interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrderByExternalID", reflect.TypeOf((*MockDatastore)(nil).GetOrderByExternalID), externalID) -} - // GetOrderCreds mocks base method. func (m *MockDatastore) GetOrderCreds(orderID go_uuid.UUID, isSigned bool) ([]OrderCreds, error) { m.ctrl.T.Helper() @@ -308,21 +263,6 @@ func (mr *MockDatastoreMockRecorder) GetOrderCredsByItemID(orderID, itemID, isSi return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrderCredsByItemID", reflect.TypeOf((*MockDatastore)(nil).GetOrderCredsByItemID), orderID, itemID, isSigned) } -// GetOrderItem mocks base method. -func (m *MockDatastore) GetOrderItem(ctx context.Context, itemID go_uuid.UUID) (*OrderItem, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetOrderItem", ctx, itemID) - ret0, _ := ret[0].(*OrderItem) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetOrderItem indicates an expected call of GetOrderItem. -func (mr *MockDatastoreMockRecorder) GetOrderItem(ctx, itemID interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrderItem", reflect.TypeOf((*MockDatastore)(nil).GetOrderItem), ctx, itemID) -} - // GetOutboxMovAvgDurationSeconds mocks base method. func (m *MockDatastore) GetOutboxMovAvgDurationSeconds() (int64, error) { m.ctrl.T.Helper() @@ -575,22 +515,6 @@ func (mr *MockDatastoreMockRecorder) InsertVote(ctx, vr interface{}) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertVote", reflect.TypeOf((*MockDatastore)(nil).InsertVote), ctx, vr) } -// IsStripeSub mocks base method. -func (m *MockDatastore) IsStripeSub(arg0 go_uuid.UUID) (bool, string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsStripeSub", arg0) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(string) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 -} - -// IsStripeSub indicates an expected call of IsStripeSub. -func (mr *MockDatastoreMockRecorder) IsStripeSub(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsStripeSub", reflect.TypeOf((*MockDatastore)(nil).IsStripeSub), arg0) -} - // MarkVoteErrored mocks base method. func (m *MockDatastore) MarkVoteErrored(ctx context.Context, vr VoteRecord, tx *sqlx.Tx) error { m.ctrl.T.Helper() @@ -706,20 +630,6 @@ func (mr *MockDatastoreMockRecorder) UpdateOrder(orderID, status interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrder", reflect.TypeOf((*MockDatastore)(nil).UpdateOrder), orderID, status) } -// UpdateOrderMetadata mocks base method. -func (m *MockDatastore) UpdateOrderMetadata(orderID go_uuid.UUID, key, value string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateOrderMetadata", orderID, key, value) - ret0, _ := ret[0].(error) - return ret0 -} - -// UpdateOrderMetadata indicates an expected call of UpdateOrderMetadata. -func (mr *MockDatastoreMockRecorder) UpdateOrderMetadata(orderID, key, value interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrderMetadata", reflect.TypeOf((*MockDatastore)(nil).UpdateOrderMetadata), orderID, key, value) -} - // UpdateSigningOrderRequestOutboxTx mocks base method. func (m *MockDatastore) UpdateSigningOrderRequestOutboxTx(ctx context.Context, tx *sqlx.Tx, requestID go_uuid.UUID, completedAt time.Time) error { m.ctrl.T.Helper() diff --git a/services/skus/service.go b/services/skus/service.go index d08d0c671..5de20f632 100644 --- a/services/skus/service.go +++ b/services/skus/service.go @@ -23,7 +23,6 @@ import ( "github.com/shopspring/decimal" "github.com/stripe/stripe-go/v72" "github.com/stripe/stripe-go/v72/client" - "github.com/stripe/stripe-go/v72/sub" "google.golang.org/api/idtoken" "google.golang.org/api/option" @@ -700,62 +699,6 @@ func (s *Service) resetNumPaymentFailed(ctx context.Context, dbi sqlx.ExecerCont return s.orderRepo.AppendMetadataInt(ctx, dbi, id, "numPaymentFailed", 0) } -// CancelOrderLegacy cancels an order, propagates to stripe if needed. -func (s *Service) CancelOrderLegacy(orderID uuid.UUID) error { - // TODO: Refactor this later. Now here's a quick fix. - ord, err := s.Datastore.GetOrder(orderID) - if err != nil { - return err - } - - if ord == nil { - return model.ErrOrderNotFound - } - - subID, ok := ord.StripeSubID() - if ok && subID != "" { - // Cancel the stripe subscription. - if _, err := sub.Cancel(subID, nil); err != nil { - // Error out if it's not 404. - if !isErrStripeNotFound(err) { - return fmt.Errorf("failed to cancel stripe subscription: %w", err) - } - } - - // Cancel even for 404. - return s.Datastore.UpdateOrder(orderID, OrderStatusCanceled) - } - - if ord.IsIOS() || ord.IsAndroid() { - return s.Datastore.UpdateOrder(orderID, OrderStatusCanceled) - } - - // Try to find by order_id in Stripe. - params := &stripe.SubscriptionSearchParams{} - params.Query = fmt.Sprintf("status:'active' AND metadata['orderID']:'%s'", orderID.String()) - - ctx := context.TODO() - - iter := sub.Search(params) - for iter.Next() { - sb := iter.Subscription() - if _, err := sub.Cancel(sb.ID, nil); err != nil { - // It seems that already canceled subscriptions might return 404. - if isErrStripeNotFound(err) { - continue - } - - return fmt.Errorf("failed to cancel stripe subscription: %w", err) - } - - if err := s.Datastore.AppendOrderMetadata(ctx, &orderID, "stripeSubscriptionId", sb.ID); err != nil { - return fmt.Errorf("failed to update order metadata with subscription id: %w", err) - } - } - - return s.Datastore.UpdateOrder(orderID, OrderStatusCanceled) -} - func (s *Service) setOrderTrialDays(ctx context.Context, orderID uuid.UUID, req *model.SetTrialDaysRequest, now time.Time) error { tx, err := s.Datastore.RawDB().BeginTxx(ctx, nil) if err != nil { diff --git a/services/skus/storage/repository/repository_test.go b/services/skus/storage/repository/repository_test.go index 8f406d926..c5e70997f 100644 --- a/services/skus/storage/repository/repository_test.go +++ b/services/skus/storage/repository/repository_test.go @@ -12,6 +12,7 @@ import ( "github.com/jmoiron/sqlx" "github.com/lib/pq" uuid "github.com/satori/go.uuid" + "github.com/shopspring/decimal" should "github.com/stretchr/testify/assert" must "github.com/stretchr/testify/require" @@ -1218,6 +1219,138 @@ func TestOrder_IncrementNumPayFailed(t *testing.T) { } } +func TestOrder_GetByExternalID(t *testing.T) { + dbi, err := setupDBI() + must.Equal(t, nil, err) + + defer func() { + _, _ = dbi.Exec("TRUNCATE TABLE orders;") + }() + + type tcGiven struct { + extID string + fnBefore func(ctx context.Context, dbi sqlx.ExecerContext) error + } + + type tcExpected struct { + ord *model.Order + err error + } + + type testCase struct { + name string + given tcGiven + exp tcExpected + } + + tests := []testCase{ + { + name: "not_found", + given: tcGiven{ + extID: "ext_id_01", + }, + exp: tcExpected{ + err: model.ErrOrderNotFound, + }, + }, + + { + name: "not_found_no_metadata", + given: tcGiven{ + extID: "ext_id_01", + fnBefore: func(ctx context.Context, dbi sqlx.ExecerContext) error { + const q = `INSERT INTO orders ( + id, merchant_id, status, currency, total_price, created_at, updated_at + ) + VALUES ( + 'facade00-0000-4000-a000-000000000000', + 'brave.com', + 'paid', + 'USD', + 9.99, + '2024-01-01 00:00:01', + '2024-01-01 00:00:01' + );` + + _, err := dbi.ExecContext(ctx, q) + + return err + }, + }, + exp: tcExpected{ + err: model.ErrOrderNotFound, + }, + }, + + { + name: "found", + given: tcGiven{ + extID: "ext_id_01", + fnBefore: func(ctx context.Context, dbi sqlx.ExecerContext) error { + const q = `INSERT INTO orders ( + id, merchant_id, status, currency, total_price, created_at, updated_at, metadata + ) + VALUES ( + 'facade00-0000-4000-a000-000000000000', + 'brave.com', + 'paid', + 'USD', + 9.99, + '2024-01-01 00:00:01', + '2024-01-01 00:00:01', + '{"externalID": "ext_id_01"}' + );` + + _, err := dbi.ExecContext(ctx, q) + + return err + }, + }, + exp: tcExpected{ + ord: &model.Order{ + ID: uuid.Must(uuid.FromString("facade00-0000-4000-a000-000000000000'")), + MerchantID: "brave.com", + Status: model.OrderStatusPaid, + Currency: "USD", + TotalPrice: decimal.RequireFromString("9.99"), + CreatedAt: time.Date(2024, time.January, 1, 0, 0, 1, 0, time.UTC), + UpdatedAt: time.Date(2024, time.January, 1, 0, 0, 1, 0, time.UTC), + Metadata: datastore.Metadata{"externalID": "ext_id_01"}, + }, + }, + }, + } + + repo := repository.NewOrder() + + for i := range tests { + tc := tests[i] + + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + + tx, err := dbi.BeginTxx(ctx, &sql.TxOptions{Isolation: sql.LevelReadUncommitted}) + must.NoError(t, err) + + t.Cleanup(func() { _ = tx.Rollback() }) + + if tc.given.fnBefore != nil { + err := tc.given.fnBefore(ctx, tx) + must.NoError(t, err) + } + + actual, err := repo.GetByExternalID(ctx, tx, tc.given.extID) + must.Equal(t, tc.exp.err, err) + + if tc.exp.err != nil { + return + } + + should.Equal(t, tc.exp.ord, actual) + }) + } +} + func ptrString(s string) *string { return &s }