Merge pull request #115 from formancehq/feat/payments-v3-ENG-1429

feat: (payments) simplify loop logic in moneycorp plugin and add unit tests

internal/connectors/plugins/currency/currency.go

@@ -7,6 +7,8 @@ import (
 )
 
 var (
+	ErrMissingCurrencies = errors.New("missing currencies")
+
 	UnsupportedCurrencies = map[string]struct{}{
 		"HUF": {},
 		"ISK": {},
@@ -203,7 +205,7 @@ func GetPrecision(currencies map[string]int, cur string) (int, error) {
 
 	def, ok := currencies[asset]
 	if !ok {
-		return 0, errors.New("missing currencies")
+		return 0, ErrMissingCurrencies
 	}
 
 	return def, nil

internal/connectors/plugins/public/moneycorp/accounts.go

@@ -5,7 +5,9 @@ import (
 	"encoding/json"
 	"time"
 
+	"github.com/formancehq/payments/internal/connectors/plugins/public/moneycorp/client"
 	"github.com/formancehq/payments/internal/models"
+	"github.com/formancehq/payments/internal/utils/pagination"
 )
 
 type accountsState struct {
@@ -15,7 +17,7 @@ type accountsState struct {
 	LastIDCreated string `json:"lastIDCreated"`
 }
 
-func (p Plugin) fetchNextAccounts(ctx context.Context, req models.FetchNextAccountsRequest) (models.FetchNextAccountsResponse, error) {
+func (p *Plugin) fetchNextAccounts(ctx context.Context, req models.FetchNextAccountsRequest) (models.FetchNextAccountsResponse, error) {
 	var oldState accountsState
 	if req.State != nil {
 		if err := json.Unmarshal(req.State, &oldState); err != nil {
@@ -28,51 +30,33 @@ func (p Plugin) fetchNextAccounts(ctx context.Context, req models.FetchNextAccou
 		LastIDCreated: oldState.LastIDCreated,
 	}
 
-	var accounts []models.PSPAccount
+	accounts := make([]models.PSPAccount, 0, req.PageSize)
 	hasMore := false
 	for page := oldState.LastPage; ; page++ {
 		newState.LastPage = page
+		pageSize := req.PageSize - len(accounts)
 
-		pagedAccounts, err := p.client.GetAccounts(ctx, page, req.PageSize)
+		pagedAccounts, err := p.client.GetAccounts(ctx, page, pageSize)
 		if err != nil {
 			return models.FetchNextAccountsResponse{}, err
 		}
-
 		if len(pagedAccounts) == 0 {
+			hasMore = false
 			break
 		}
 
-		for _, account := range pagedAccounts {
-			if account.ID <= oldState.LastIDCreated {
-				continue
-			}
-
-			raw, err := json.Marshal(account)
-			if err != nil {
-				return models.FetchNextAccountsResponse{}, err
-			}
-
-			accounts = append(accounts, models.PSPAccount{
-				Reference: account.ID,
-				// Moneycorp does not send the opening date of the account
-				CreatedAt: time.Now().UTC(),
-				Name:      &account.Attributes.AccountName,
-				Raw:       raw,
-			})
-
-			newState.LastIDCreated = account.ID
-
-			if len(accounts) >= req.PageSize {
-				break
-			}
+		accounts, err = toPSPAccounts(oldState.LastIDCreated, accounts, pagedAccounts)
+		if err != nil {
+			return models.FetchNextAccountsResponse{}, err
 		}
-
-		if len(pagedAccounts) < req.PageSize {
+		if len(accounts) == 0 {
 			break
 		}
+		newState.LastIDCreated = accounts[len(accounts)-1].Reference
 
-		if len(accounts) >= req.PageSize {
-			hasMore = true
+		needMore := true
+		needMore, hasMore = pagination.ShouldFetchMore(accounts, pagedAccounts, pageSize)
+		if !needMore {
 			break
 		}
 	}
@@ -88,3 +72,29 @@ func (p Plugin) fetchNextAccounts(ctx context.Context, req models.FetchNextAccou
 		HasMore:  hasMore,
 	}, nil
 }
+
+func toPSPAccounts(
+	lastIDSeen string,
+	accounts []models.PSPAccount,
+	pagedAccounts []*client.Account,
+) ([]models.PSPAccount, error) {
+	for _, account := range pagedAccounts {
+		if account.ID <= lastIDSeen {
+			continue
+		}
+
+		raw, err := json.Marshal(account)
+		if err != nil {
+			return accounts, err
+		}
+
+		accounts = append(accounts, models.PSPAccount{
+			Reference: account.ID,
+			// Moneycorp does not send the opening date of the account
+			CreatedAt: time.Now().UTC(),
+			Name:      &account.Attributes.AccountName,
+			Raw:       raw,
+		})
+	}
+	return accounts, nil
+}

internal/connectors/plugins/public/moneycorp/accounts_test.go

@@ -0,0 +1,60 @@
+package moneycorp
+
+import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/formancehq/payments/internal/connectors/plugins/public/moneycorp/client"
+	"github.com/formancehq/payments/internal/models"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	gomock "go.uber.org/mock/gomock"
+)
+
+var _ = Describe("Moneycorp Plugin Accounts", func() {
+	Context("fetch next accounts", func() {
+		var (
+			plg *Plugin
+			m   *client.MockClient
+
+			pageSize       int
+			sampleAccounts []*client.Account
+		)
+
+		BeforeEach(func() {
+			ctrl := gomock.NewController(GinkgoT())
+			m = client.NewMockClient(ctrl)
+			plg = &Plugin{client: m}
+			pageSize = 15
+
+			sampleAccounts = make([]*client.Account, 0)
+			for i := 0; i < pageSize; i++ {
+				sampleAccounts = append(sampleAccounts, &client.Account{
+					ID: fmt.Sprintf("moneycorp-reference-%d", i),
+				})
+			}
+
+		})
+		It("fetches next accounts", func(ctx SpecContext) {
+			req := models.FetchNextAccountsRequest{
+				State:    json.RawMessage(`{}`),
+				PageSize: pageSize,
+			}
+			m.EXPECT().GetAccounts(ctx, gomock.Any(), pageSize).Return(
+				sampleAccounts,
+				nil,
+			)
+			res, err := plg.FetchNextAccounts(ctx, req)
+			Expect(err).To(BeNil())
+			Expect(res.HasMore).To(BeTrue())
+			Expect(res.Accounts).To(HaveLen(req.PageSize))
+
+			var state accountsState
+
+			err = json.Unmarshal(res.NewState, &state)
+			Expect(err).To(BeNil())
+			Expect(state.LastPage).To(Equal(0))
+			Expect(state.LastIDCreated).To(Equal(res.Accounts[len(res.Accounts)-1].Reference))
+		})
+	})
+})

internal/connectors/plugins/public/moneycorp/balances.go

@@ -9,7 +9,7 @@ import (
 	"github.com/formancehq/payments/internal/models"
 )
 
-func (p Plugin) fetchNextBalances(ctx context.Context, req models.FetchNextBalancesRequest) (models.FetchNextBalancesResponse, error) {
+func (p *Plugin) fetchNextBalances(ctx context.Context, req models.FetchNextBalancesRequest) (models.FetchNextBalancesResponse, error) {
 	var from models.PSPAccount
 	if req.FromPayload == nil {
 		return models.FetchNextBalancesResponse{}, models.ErrMissingFromPayloadInRequest

internal/connectors/plugins/public/moneycorp/balances_test.go

@@ -0,0 +1,62 @@
+package moneycorp
+
+import (
+	"encoding/json"
+	"fmt"
+	"math/big"
+	"strings"
+
+	"github.com/formancehq/payments/internal/connectors/plugins/public/moneycorp/client"
+	"github.com/formancehq/payments/internal/models"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	gomock "go.uber.org/mock/gomock"
+)
+
+var _ = Describe("Moneycorp Plugin Balances", func() {
+	var (
+		plg *Plugin
+	)
+
+	Context("fetch next balances", func() {
+		var (
+			m *client.MockClient
+
+			accRef         string
+			sampleBalance  *client.Balance
+			expectedAmount *big.Int
+		)
+
+		BeforeEach(func() {
+			ctrl := gomock.NewController(GinkgoT())
+			m = client.NewMockClient(ctrl)
+			plg = &Plugin{client: m}
+
+			accRef = "abc"
+			expectedAmount = big.NewInt(309900)
+			sampleBalance = &client.Balance{
+				Attributes: client.Attributes{
+					CurrencyCode:     "AED",
+					AvailableBalance: json.Number("3099"),
+				},
+			}
+		})
+		It("fetches next balances", func(ctx SpecContext) {
+			req := models.FetchNextBalancesRequest{
+				FromPayload: json.RawMessage(fmt.Sprintf(`{"reference": "%s"}`, accRef)),
+				State:       json.RawMessage(`{}`),
+			}
+			m.EXPECT().GetAccountBalances(ctx, accRef).Return(
+				[]*client.Balance{sampleBalance},
+				nil,
+			)
+			res, err := plg.FetchNextBalances(ctx, req)
+			Expect(err).To(BeNil())
+			Expect(res.Balances).To(HaveLen(1))
+
+			Expect(res.Balances[0].AccountReference).To(Equal(accRef))
+			Expect(res.Balances[0].Amount).To(BeEquivalentTo(expectedAmount))
+			Expect(res.Balances[0].Asset).To(HavePrefix(strings.ToUpper(sampleBalance.Attributes.CurrencyCode)))
+		})
+	})
+})

internal/connectors/plugins/public/moneycorp/client/accounts.go

@@ -18,7 +18,7 @@ type Account struct {
 	} `json:"attributes"`
 }
 
-func (c *Client) GetAccounts(ctx context.Context, page int, pageSize int) ([]*Account, error) {
+func (c *client) GetAccounts(ctx context.Context, page int, pageSize int) ([]*Account, error) {
 	// TODO(polo, crimson): metrics
 	// metrics can also be embedded in wrapper
 	// f := connectors.ClientMetrics(ctx, "moneycorp", "list_accounts")

internal/connectors/plugins/public/moneycorp/client/balances.go

@@ -12,18 +12,20 @@ type balancesResponse struct {
 }
 
 type Balance struct {
-	ID         string `json:"id"`
-	Attributes struct {
-		CurrencyCode     string      `json:"currencyCode"`
-		OverallBalance   json.Number `json:"overallBalance"`
-		AvailableBalance json.Number `json:"availableBalance"`
-		ClearedBalance   json.Number `json:"clearedBalance"`
-		ReservedBalance  json.Number `json:"reservedBalance"`
-		UnclearedBalance json.Number `json:"unclearedBalance"`
-	} `json:"attributes"`
+	ID         string     `json:"id"`
+	Attributes Attributes `json:"attributes"`
 }
 
-func (c *Client) GetAccountBalances(ctx context.Context, accountID string) ([]*Balance, error) {
+type Attributes struct {
+	CurrencyCode     string      `json:"currencyCode"`
+	OverallBalance   json.Number `json:"overallBalance"`
+	AvailableBalance json.Number `json:"availableBalance"`
+	ClearedBalance   json.Number `json:"clearedBalance"`
+	ReservedBalance  json.Number `json:"reservedBalance"`
+	UnclearedBalance json.Number `json:"unclearedBalance"`
+}
+
+func (c *client) GetAccountBalances(ctx context.Context, accountID string) ([]*Balance, error) {
 	// TODO(polo): metrics
 	// f := connectors.ClientMetrics(ctx, "moneycorp", "list_account_balances")
 	// now := time.Now()

internal/connectors/plugins/public/moneycorp/client/client.go

@@ -1,19 +1,29 @@
 package client
 
 import (
+	"context"
 	"net/http"
 	"strings"
+	"time"
 
 	"github.com/formancehq/payments/internal/connectors/httpwrapper"
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 )
 
-type Client struct {
+//go:generate mockgen -source client.go -destination client_generated.go -package client . Client
+type Client interface {
+	GetAccounts(ctx context.Context, page int, pageSize int) ([]*Account, error)
+	GetAccountBalances(ctx context.Context, accountID string) ([]*Balance, error)
+	GetRecipients(ctx context.Context, accountID string, page int, pageSize int) ([]*Recipient, error)
+	GetTransactions(ctx context.Context, accountID string, page, pageSize int, lastCreatedAt time.Time) ([]*Transaction, error)
+}
+
+type client struct {
 	httpClient httpwrapper.Client
 	endpoint   string
 }
 
-func New(clientID, apiKey, endpoint string) (*Client, error) {
+func New(clientID, apiKey, endpoint string) (*client, error) {
 	config := &httpwrapper.Config{
 		Transport: &apiTransport{
 			clientID:   clientID,
@@ -36,7 +46,7 @@ func New(clientID, apiKey, endpoint string) (*Client, error) {
 	endpoint = strings.TrimSuffix(endpoint, "/")
 
 	httpClient, err := httpwrapper.NewClient(config)
-	c := &Client{
+	c := &client{
 		httpClient: httpClient,
 		endpoint:   endpoint,
 	}