satellite/payments/stripecoinpayments: avoid mock cross-talk in tests

The tests were using global variables for keeping the mock state, which
was indexed by the satellite ID. However, the satellite ID-s are
deterministic and it's possible for two tests end up using the same
mocks.

Instead make the mock creation not depend on the satellite ID and
instead require it being configured via paymentsconfig.

This fixes TestAutoFreezeChore failure.

Change-Id: I531d3550a934fbb36cff2973be96fd43b7edc44a

cmd/satellite/billing.go

@@ -11,7 +11,6 @@ import (
 	"github.com/zeebo/errs"
 	"go.uber.org/zap"
 
-	"storj.io/common/storj"
 	"storj.io/common/uuid"
 	"storj.io/private/process"
 	"storj.io/storj/satellite"
@@ -43,14 +42,15 @@ func setupPayments(log *zap.Logger, db satellite.DB) (*stripecoinpayments.Servic
 
 	var stripeClient stripecoinpayments.StripeClient
 	switch pc.Provider {
-	default:
+	case "": // just new mock, only used in testing binaries
 		stripeClient = stripecoinpayments.NewStripeMock(
-			storj.NodeID{},
 			db.StripeCoinPayments().Customers(),
 			db.Console().Users(),
 		)
 	case "stripecoinpayments":
 		stripeClient = stripecoinpayments.NewStripeClient(log, pc.StripeCoinPayments)
+	default:
+		return nil, errs.New("invalid stripe coin payments provider %q", pc.Provider)
 	}
 
 	prices, err := pc.UsagePrice.ToModel()

private/testplanet/satellite.go

@@ -58,6 +58,7 @@ import (
 	"storj.io/storj/satellite/overlay"
 	"storj.io/storj/satellite/overlay/offlinenodes"
 	"storj.io/storj/satellite/overlay/straynodes"
+	"storj.io/storj/satellite/payments/stripecoinpayments"
 	"storj.io/storj/satellite/repair/checker"
 	"storj.io/storj/satellite/repair/repairer"
 	"storj.io/storj/satellite/reputation"
@@ -524,6 +525,9 @@ func (planet *Planet) newSatellite(ctx context.Context, prefix string, index int
 	rollupsWriteCache := orders.NewRollupsWriteCache(log.Named("orders-write-cache"), db.Orders(), config.Orders.FlushBatchSize)
 	planet.databases = append(planet.databases, rollupsWriteCacheCloser{rollupsWriteCache})
 
+	config.Payments.Provider = "mock"
+	config.Payments.MockProvider = stripecoinpayments.NewStripeMock(db.StripeCoinPayments().Customers(), db.Console().Users())
+
 	peer, err := satellite.New(log, identity, db, metabaseDB, revocationDB, liveAccounting, rollupsWriteCache, versionInfo, &config, nil)
 	if err != nil {
 		return nil, errs.Wrap(err)

satellite/admin.go

@@ -138,16 +138,18 @@ func NewAdmin(log *zap.Logger, full *identity.FullIdentity, db DB, metabaseDB *m
 		pc := config.Payments
 
 		var stripeClient stripecoinpayments.StripeClient
-		var err error
 		switch pc.Provider {
-		default:
+		case "": // just new mock, only used in testing binaries
 			stripeClient = stripecoinpayments.NewStripeMock(
-				peer.ID(),
 				peer.DB.StripeCoinPayments().Customers(),
 				peer.DB.Console().Users(),
 			)
+		case "mock":
+			stripeClient = pc.MockProvider
 		case "stripecoinpayments":
 			stripeClient = stripecoinpayments.NewStripeClient(log, pc.StripeCoinPayments)
+		default:
+			return nil, errs.New("invalid stripe coin payments provider %q", pc.Provider)
 		}
 
 		prices, err := pc.UsagePrice.ToModel()

satellite/api.go

@@ -520,14 +520,17 @@ func NewAPI(log *zap.Logger, full *identity.FullIdentity, db DB,
 
 		var stripeClient stripecoinpayments.StripeClient
 		switch pc.Provider {
-		default:
+		case "": // just new mock, only used in testing binaries
 			stripeClient = stripecoinpayments.NewStripeMock(
-				peer.ID(),
 				peer.DB.StripeCoinPayments().Customers(),
 				peer.DB.Console().Users(),
 			)
+		case "mock":
+			stripeClient = pc.MockProvider
 		case "stripecoinpayments":
 			stripeClient = stripecoinpayments.NewStripeClient(log, pc.StripeCoinPayments)
+		default:
+			return nil, errs.New("invalid stripe coin payments provider %q", pc.Provider)
 		}
 
 		prices, err := pc.UsagePrice.ToModel()

satellite/console/consoleweb/consoleql/mutation_test.go

@@ -15,7 +15,6 @@ import (
 	"go.uber.org/zap/zaptest"
 
 	"storj.io/common/testcontext"
-	"storj.io/common/testrand"
 	"storj.io/common/uuid"
 	"storj.io/storj/private/post"
 	"storj.io/storj/private/testplanet"
@@ -83,7 +82,6 @@ func TestGraphqlMutation(t *testing.T) {
 		paymentsService, err := stripecoinpayments.NewService(
 			log.Named("payments.stripe:service"),
 			stripecoinpayments.NewStripeMock(
-				testrand.NodeID(),
 				db.StripeCoinPayments().Customers(),
 				db.Console().Users(),
 			),

satellite/console/consoleweb/consoleql/query_test.go

@@ -14,7 +14,6 @@ import (
 	"go.uber.org/zap/zaptest"
 
 	"storj.io/common/testcontext"
-	"storj.io/common/testrand"
 	"storj.io/storj/private/testplanet"
 	"storj.io/storj/private/testredis"
 	"storj.io/storj/satellite/accounting"
@@ -67,7 +66,6 @@ func TestGraphqlQuery(t *testing.T) {
 		paymentsService, err := stripecoinpayments.NewService(
 			log.Named("payments.stripe:service"),
 			stripecoinpayments.NewStripeMock(
-				testrand.NodeID(),
 				db.StripeCoinPayments().Customers(),
 				db.Console().Users(),
 			),

satellite/core.go

@@ -533,14 +533,17 @@ func New(log *zap.Logger, full *identity.FullIdentity, db DB,
 
 		var stripeClient stripecoinpayments.StripeClient
 		switch pc.Provider {
-		default:
+		case "": // just new mock, only used in testing binaries
 			stripeClient = stripecoinpayments.NewStripeMock(
-				peer.ID(),
 				peer.DB.StripeCoinPayments().Customers(),
 				peer.DB.Console().Users(),
 			)
+		case "mock":
+			stripeClient = pc.MockProvider
 		case "stripecoinpayments":
 			stripeClient = stripecoinpayments.NewStripeClient(log, pc.StripeCoinPayments)
+		default:
+			return nil, errs.New("invalid stripe coin payments provider %q", pc.Provider)
 		}
 
 		prices, err := pc.UsagePrice.ToModel()

satellite/payments/paymentsconfig/config.go

@@ -25,6 +25,8 @@ var Error = errs.Class("payments config")
 // Config defines global payments config.
 type Config struct {
 	Provider     string                          `help:"payments provider to use" default:""`
+	MockProvider stripecoinpayments.StripeClient `internal:"true"`
+
 	BillingConfig            billing.Config
 	StripeCoinPayments       stripecoinpayments.Config
 	Storjscan                storjscan.Config

satellite/payments/stripecoinpayments/accounts_test.go

@@ -11,7 +11,6 @@ import (
 	"go.uber.org/zap/zaptest"
 
 	"storj.io/common/testcontext"
-	"storj.io/common/testrand"
 	"storj.io/storj/private/testplanet"
 	"storj.io/storj/private/testredis"
 	"storj.io/storj/satellite/accounting"
@@ -61,7 +60,6 @@ func TestSignupCouponCodes(t *testing.T) {
 		paymentsService, err := stripecoinpayments.NewService(
 			log.Named("payments.stripe:service"),
 			stripecoinpayments.NewStripeMock(
-				testrand.NodeID(),
 				db.StripeCoinPayments().Customers(),
 				db.Console().Users(),
 			),

satellite/payments/stripecoinpayments/stripemock.go

@@ -19,7 +19,6 @@ import (
 	"github.com/stripe/stripe-go/v72/paymentmethod"
 	"github.com/stripe/stripe-go/v72/promotioncode"
 
-	"storj.io/common/storj"
 	"storj.io/common/testrand"
 	"storj.io/common/uuid"
 	"storj.io/storj/satellite/console"
@@ -50,22 +49,6 @@ const (
 	TestPaymentMethodsAttachFailure = "test_payment_methods_attach_failure"
 )
 
-// mocks synchronized map for caching mockStripeClient.
-//
-// The satellite has a Core part and API part which mostly duplicate each
-// other. Each of them have a StripeClient instance. This is not a problem in
-// production, because the stripeClient implementation is stateless and calls
-// the Web API of the same Stripe backend. But it is a problem in test
-// environments as the mockStripeClient is stateful - the data is stored in
-// in-memory maps. Therefore, we need the Core and API parts share the same
-// instance of mockStripeClient.
-var mocks = struct {
-	sync.Mutex
-	m map[storj.NodeID]*mockStripeState
-}{
-	m: make(map[storj.NodeID]*mockStripeState),
-}
-
 var (
 	testPromoCodes = map[string]*stripe.PromotionCode{
 		"promo1": {
@@ -108,6 +91,8 @@ var (
 
 // mockStripeState Stripe client mock.
 type mockStripeState struct {
+	mu sync.Mutex
+
 	customers                   *mockCustomersState
 	paymentMethods              *mockPaymentMethods
 	invoices                    *mockInvoices
@@ -138,26 +123,15 @@ var mockEmptyQuery = stripe.Query(func(*stripe.Params, *form.Values) ([]interfac
 // If called by CLI tool, the id param should be a zero value, i.e. storj.NodeID{}.
 // If called by satellitedb test case, the id param should be a random value,
 // i.e. testrand.NodeID().
-func NewStripeMock(id storj.NodeID, customersDB CustomersDB, usersDB console.Users) StripeClient {
+func NewStripeMock(customersDB CustomersDB, usersDB console.Users) StripeClient {
-	mocks.Lock()
+	state := &mockStripeState{}
-	defer mocks.Unlock()
+	state.customers = &mockCustomersState{}
-
+	state.paymentMethods = newMockPaymentMethods(state)
-	state, ok := mocks.m[id]
+	state.invoiceItems = newMockInvoiceItems(state)
-	if !ok {
+	state.invoices = newMockInvoices(state, state.invoiceItems)
-		state = &mockStripeState{
+	state.customerBalanceTransactions = newMockCustomerBalanceTransactions(state)
-			customers:                   &mockCustomersState{},
+	state.charges = &mockCharges{}
-			paymentMethods:              newMockPaymentMethods(),
+	state.promoCodes = newMockPromoCodes(state)
-			invoices:                    newMockInvoices(),
-			invoiceItems:                newMockInvoiceItems(),
-			customerBalanceTransactions: newMockCustomerBalanceTransactions(),
-			charges:                     &mockCharges{},
-			promoCodes: &mockPromoCodes{
-				promoCodes: testPromoCodes,
-			},
-		}
-		state.invoices.invoiceItems = state.invoiceItems
-		mocks.m[id] = state
-	}
 
 	return &mockStripeClient{
 		customersDB:     customersDB,
@@ -167,10 +141,11 @@ func NewStripeMock(id storj.NodeID, customersDB CustomersDB, usersDB console.Use
 }
 
 func (m *mockStripeClient) Customers() StripeCustomers {
-	mocks.Lock()
+	m.mu.Lock()
-	defer mocks.Unlock()
+	defer m.mu.Unlock()
-
 	return &mockCustomers{
+		root: m.mockStripeState,
+
 		customersDB: m.customersDB,
 		usersDB:     m.usersDB,
 		state:       m.customers,
@@ -207,6 +182,8 @@ func (m *mockStripeClient) CreditNotes() StripeCreditNotes {
 }
 
 type mockCustomers struct {
+	root *mockStripeState
+
 	customersDB CustomersDB
 	usersDB     console.Users
 	state       *mockCustomersState
@@ -222,8 +199,8 @@ type mockCustomersState struct {
 // We need to repopulate the mock on every restart to ensure that requests to the mock
 // for existing users won't fail with errors like "customer not found".
 func (m *mockCustomers) repopulate() error {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	if !m.state.repopulated {
 		const limit = 25
@@ -297,8 +274,8 @@ func (m *mockCustomers) New(params *stripe.CustomerParams) (*stripe.Customer, er
 		customer.Discount = &stripe.Discount{Coupon: mockCoupons[c.ID]}
 	}
 
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	m.state.customers = append(m.state.customers, customer)
 	return customer, nil
@@ -309,8 +286,8 @@ func (m *mockCustomers) Get(id string, params *stripe.CustomerParams) (*stripe.C
 		return nil, err
 	}
 
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	for _, customer := range m.state.customers {
 		if id == customer.ID {
@@ -335,8 +312,8 @@ func (m *mockCustomers) Update(id string, params *stripe.CustomerParams) (*strip
 		return customer, nil
 	}
 
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	if params.Metadata != nil {
 		customer.Metadata = params.Metadata
@@ -358,14 +335,16 @@ func (m *mockCustomers) Update(id string, params *stripe.CustomerParams) (*strip
 }
 
 type mockPaymentMethods struct {
+	root *mockStripeState
 	// attached contains a mapping of customerID to its paymentMethods
 	attached map[string][]*stripe.PaymentMethod
 	// unattached contains created but not attached paymentMethods
 	unattached []*stripe.PaymentMethod
 }
 
-func newMockPaymentMethods() *mockPaymentMethods {
+func newMockPaymentMethods(root *mockStripeState) *mockPaymentMethods {
 	return &mockPaymentMethods{
+		root:     root,
 		attached: map[string][]*stripe.PaymentMethod{},
 	}
 }
@@ -384,8 +363,8 @@ func (c *listContainer) GetListMeta() *stripe.ListMeta {
 }
 
 func (m *mockPaymentMethods) List(listParams *stripe.PaymentMethodListParams) *paymentmethod.Iter {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	listMeta := &stripe.ListMeta{
 		HasMore:    false,
@@ -435,8 +414,8 @@ func (m *mockPaymentMethods) New(params *stripe.PaymentMethodParams) (*stripe.Pa
 		Type: stripe.PaymentMethodTypeCard,
 	}
 
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	m.unattached = append(m.unattached, newMethod)
 
@@ -444,8 +423,8 @@ func (m *mockPaymentMethods) New(params *stripe.PaymentMethodParams) (*stripe.Pa
 }
 
 func (m *mockPaymentMethods) Attach(id string, params *stripe.PaymentMethodAttachParams) (*stripe.PaymentMethod, error) {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	var method *stripe.PaymentMethod
 	for _, candidate := range m.unattached {
@@ -465,8 +444,8 @@ func (m *mockPaymentMethods) Attach(id string, params *stripe.PaymentMethodAttac
 }
 
 func (m *mockPaymentMethods) Detach(id string, params *stripe.PaymentMethodDetachParams) (*stripe.PaymentMethod, error) {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	var unattached *stripe.PaymentMethod
 	for user, userMethods := range m.attached {
@@ -485,19 +464,23 @@ func (m *mockPaymentMethods) Detach(id string, params *stripe.PaymentMethodDetac
 }
 
 type mockInvoices struct {
+	root *mockStripeState
+
 	invoices     map[string][]*stripe.Invoice
 	invoiceItems *mockInvoiceItems
 }
 
-func newMockInvoices() *mockInvoices {
+func newMockInvoices(root *mockStripeState, invoiceItems *mockInvoiceItems) *mockInvoices {
 	return &mockInvoices{
+		root:         root,
 		invoices:     make(map[string][]*stripe.Invoice),
+		invoiceItems: invoiceItems,
 	}
 }
 
 func (m *mockInvoices) New(params *stripe.InvoiceParams) (*stripe.Invoice, error) {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	items, ok := m.invoiceItems.items[*params.Customer]
 	if !ok || len(items) == 0 {
@@ -549,8 +532,8 @@ func (m *mockInvoices) New(params *stripe.InvoiceParams) (*stripe.Invoice, error
 }
 
 func (m *mockInvoices) List(listParams *stripe.InvoiceListParams) *invoice.Iter {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	listMeta := &stripe.ListMeta{
 		HasMore:    false,
@@ -642,11 +625,13 @@ func (m *mockInvoices) Del(id string, params *stripe.InvoiceParams) (*stripe.Inv
 }
 
 type mockInvoiceItems struct {
+	root  *mockStripeState
 	items map[string][]*stripe.InvoiceItem
 }
 
-func newMockInvoiceItems() *mockInvoiceItems {
+func newMockInvoiceItems(root *mockStripeState) *mockInvoiceItems {
 	return &mockInvoiceItems{
+		root:  root,
 		items: make(map[string][]*stripe.InvoiceItem),
 	}
 }
@@ -660,8 +645,8 @@ func (m *mockInvoiceItems) Del(id string, params *stripe.InvoiceItemParams) (*st
 }
 
 func (m *mockInvoiceItems) New(params *stripe.InvoiceItemParams) (*stripe.InvoiceItem, error) {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	item := &stripe.InvoiceItem{
 		Metadata: params.Metadata,
@@ -678,8 +663,8 @@ func (m *mockInvoiceItems) New(params *stripe.InvoiceItemParams) (*stripe.Invoic
 }
 
 func (m *mockInvoiceItems) List(listParams *stripe.InvoiceItemListParams) *invoiceitem.Iter {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	listMeta := &stripe.ListMeta{
 		HasMore:    false,
@@ -704,11 +689,13 @@ func (m *mockInvoiceItems) List(listParams *stripe.InvoiceItemListParams) *invoi
 }
 
 type mockCustomerBalanceTransactions struct {
+	root         *mockStripeState
 	transactions map[string][]*stripe.CustomerBalanceTransaction
 }
 
-func newMockCustomerBalanceTransactions() *mockCustomerBalanceTransactions {
+func newMockCustomerBalanceTransactions(root *mockStripeState) *mockCustomerBalanceTransactions {
 	return &mockCustomerBalanceTransactions{
+		root:         root,
 		transactions: make(map[string][]*stripe.CustomerBalanceTransaction),
 	}
 }
@@ -722,8 +709,8 @@ func (m *mockCustomerBalanceTransactions) New(params *stripe.CustomerBalanceTran
 		Created:     time.Now().Unix(),
 	}
 
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	m.transactions[*params.Customer] = append(m.transactions[*params.Customer], tx)
 
@@ -731,8 +718,8 @@ func (m *mockCustomerBalanceTransactions) New(params *stripe.CustomerBalanceTran
 }
 
 func (m *mockCustomerBalanceTransactions) List(listParams *stripe.CustomerBalanceTransactionListParams) *customerbalancetransaction.Iter {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	query := stripe.Query(func(p *stripe.Params, b *form.Values) ([]interface{}, stripe.ListContainer, error) {
 		txs := m.transactions[*listParams.Customer]
@@ -762,12 +749,21 @@ func (m *mockCharges) List(listParams *stripe.ChargeListParams) *charge.Iter {
 }
 
 type mockPromoCodes struct {
+	root *mockStripeState
+
 	promoCodes map[string]*stripe.PromotionCode
 }
 
+func newMockPromoCodes(root *mockStripeState) *mockPromoCodes {
+	return &mockPromoCodes{
+		root:       root,
+		promoCodes: testPromoCodes,
+	}
+}
+
 func (m *mockPromoCodes) List(params *stripe.PromotionCodeListParams) *promotioncode.Iter {
-	mocks.Lock()
+	m.root.mu.Lock()
-	defer mocks.Unlock()
+	defer m.root.mu.Unlock()
 
 	query := stripe.Query(func(p *stripe.Params, b *form.Values) ([]interface{}, stripe.ListContainer, error) {
 		promoCode := m.promoCodes[*params.Code]