feat(wise): simplify loops and add tests

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

@@ -45,7 +45,7 @@ func (p *Plugin) fetchNextAccounts(ctx context.Context, req models.FetchNextAcco
 	}
 
 	for _, balance := range balances {
-		if balance.ID <= oldState.LastAccountID {
+		if oldState.LastAccountID != 0 && balance.ID <= oldState.LastAccountID {
 			continue
 		}
 

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

@@ -2,7 +2,8 @@ package wise
 
 import (
 	"encoding/json"
-	"fmt"
+	"errors"
+	"time"
 
 	"github.com/formancehq/payments/internal/connectors/plugins/public/wise/client"
 	"github.com/formancehq/payments/internal/models"
@@ -15,54 +16,151 @@ import (
 var _ = Describe("Wise Plugin Accounts", func() {
 	var (
 		plg *Plugin
-		m   *client.MockClient
 	)
 
 	BeforeEach(func() {
 		plg = &Plugin{}
-
-		ctrl := gomock.NewController(GinkgoT())
-		m = client.NewMockClient(ctrl)
-		plg.SetClient(m)
 	})
 
-	Context("fetch next accounts", func() {
+	Context("fetching next accounts", func() {
 		var (
-			balances          []client.Balance
-			expectedProfileID uint64
+			m              *client.MockClient
+			sampleBalances []client.Balance
+			now            time.Time
 		)
 
 		BeforeEach(func() {
-			expectedProfileID = 123454
-			balances = []client.Balance{
-				{ID: 14556, Type: "type1"},
-				{ID: 3334, Type: "type2"},
+			ctrl := gomock.NewController(GinkgoT())
+			m = client.NewMockClient(ctrl)
+			plg.client = m
+			now = time.Now().UTC()
+
+			sampleBalances = make([]client.Balance, 0)
+			for i := 0; i < 50; i++ {
+				sampleBalances = append(sampleBalances, client.Balance{
+					ID:       uint64(i),
+					Currency: "USD",
+					Name:     "test1",
+					Amount: client.BalanceAmount{
+						Value:    "100",
+						Currency: "USD",
+					},
+					CreationTime: now.Add(-time.Duration(50-i) * time.Minute).UTC(),
+				})
+			}
+		})
+
+		It("should return an error - get accounts error", func(ctx SpecContext) {
+			req := models.FetchNextAccountsRequest{
+				PageSize:    60,
+				FromPayload: []byte(`{"id": 0}`),
 			}
+
+			m.EXPECT().GetBalances(ctx, uint64(0)).Return(
+				[]client.Balance{},
+				errors.New("test error"),
+			)
+
+			resp, err := plg.FetchNextAccounts(ctx, req)
+			Expect(err).ToNot(BeNil())
+			Expect(err).To(MatchError("test error"))
+			Expect(resp).To(Equal(models.FetchNextAccountsResponse{}))
 		})
 
-		It("fetches accounts from wise", func(ctx SpecContext) {
+		It("should fetch next accounts - no state no results", func(ctx SpecContext) {
 			req := models.FetchNextAccountsRequest{
-				State:       json.RawMessage(`{}`),
-				FromPayload: json.RawMessage(fmt.Sprintf(`{"ID":%d}`, expectedProfileID)),
-				PageSize:    len(balances),
+				PageSize:    60,
+				FromPayload: []byte(`{"id": 0}`),
 			}
-			m.EXPECT().GetBalances(ctx, expectedProfileID).Return(
-				balances,
+
+			m.EXPECT().GetBalances(ctx, uint64(0)).Return(
+				[]client.Balance{},
 				nil,
 			)
 
-			res, err := plg.FetchNextAccounts(ctx, req)
+			resp, err := plg.FetchNextAccounts(ctx, req)
 			Expect(err).To(BeNil())
-			Expect(res.HasMore).To(BeTrue())
-			Expect(res.Accounts).To(HaveLen(req.PageSize))
-			Expect(res.Accounts[0].Reference).To(Equal(fmt.Sprint(balances[0].ID)))
-			Expect(res.Accounts[1].Reference).To(Equal(fmt.Sprint(balances[1].ID)))
+			Expect(resp.Accounts).To(HaveLen(0))
+			Expect(resp.HasMore).To(BeFalse())
+			Expect(resp.NewState).ToNot(BeNil())
 
 			var state accountsState
+			err = json.Unmarshal(resp.NewState, &state)
+			Expect(err).To(BeNil())
+			// We fetched everything, state should be resetted
+			Expect(state.LastAccountID).To(Equal(uint64(0)))
+		})
 
-			err = json.Unmarshal(res.NewState, &state)
+		It("should fetch next accounts - no state pageSize > total accounts", func(ctx SpecContext) {
+			req := models.FetchNextAccountsRequest{
+				PageSize:    60,
+				FromPayload: []byte(`{"id": 0}`),
+			}
+
+			m.EXPECT().GetBalances(ctx, uint64(0)).Return(
+				sampleBalances,
+				nil,
+			)
+
+			resp, err := plg.FetchNextAccounts(ctx, req)
+			Expect(err).To(BeNil())
+			Expect(resp.Accounts).To(HaveLen(50))
+			Expect(resp.HasMore).To(BeFalse())
+			Expect(resp.NewState).ToNot(BeNil())
+
+			var state accountsState
+			err = json.Unmarshal(resp.NewState, &state)
+			Expect(err).To(BeNil())
+			// We fetched everything, state should be resetted
+			Expect(state.LastAccountID).To(Equal(uint64(49)))
+		})
+
+		It("should fetch next accounts - no state pageSize < total accounts", func(ctx SpecContext) {
+			req := models.FetchNextAccountsRequest{
+				PageSize:    40,
+				FromPayload: []byte(`{"id": 0}`),
+			}
+
+			m.EXPECT().GetBalances(ctx, uint64(0)).Return(
+				sampleBalances[:40],
+				nil,
+			)
+
+			resp, err := plg.FetchNextAccounts(ctx, req)
+			Expect(err).To(BeNil())
+			Expect(resp.Accounts).To(HaveLen(40))
+			Expect(resp.HasMore).To(BeTrue())
+			Expect(resp.NewState).ToNot(BeNil())
+
+			var state accountsState
+			err = json.Unmarshal(resp.NewState, &state)
+			Expect(err).To(BeNil())
+			Expect(state.LastAccountID).To(Equal(uint64(39)))
+		})
+
+		It("should fetch next accounts - with state pageSize < total accounts", func(ctx SpecContext) {
+			req := models.FetchNextAccountsRequest{
+				State:       []byte(`{"lastAccountID": 38}`),
+				PageSize:    40,
+				FromPayload: []byte(`{"id": 0}`),
+			}
+
+			m.EXPECT().GetBalances(ctx, uint64(0)).Return(
+				sampleBalances,
+				nil,
+			)
+
+			resp, err := plg.FetchNextAccounts(ctx, req)
+			Expect(err).To(BeNil())
+			Expect(resp.Accounts).To(HaveLen(11))
+			Expect(resp.HasMore).To(BeFalse())
+			Expect(resp.NewState).ToNot(BeNil())
+
+			var state accountsState
+			err = json.Unmarshal(resp.NewState, &state)
 			Expect(err).To(BeNil())
-			Expect(fmt.Sprint(state.LastAccountID)).To(Equal(res.Accounts[len(res.Accounts)-1].Reference))
+			// We fetched everything, state should be resetted
+			Expect(state.LastAccountID).To(Equal(uint64(49)))
 		})
 	})
 })

internal/connectors/plugins/public/wise/client/recipient_accounts.go

@@ -13,13 +13,15 @@ type RecipientAccountsResponse struct {
 	Size                   int                 `json:"size"`
 }
 
+type Name struct {
+	FullName string `json:"fullName"`
+}
+
 type RecipientAccount struct {
 	ID       uint64 `json:"id"`
 	Profile  uint64 `json:"profileId"`
 	Currency string `json:"currency"`
-	Name     struct {
-		FullName string `json:"fullName"`
-	} `json:"name"`
+	Name     Name   `json:"name"`
 }
 
 func (c *client) GetRecipientAccounts(ctx context.Context, profileID uint64, pageSize int, seekPositionForNext uint64) (*RecipientAccountsResponse, error) {

internal/connectors/plugins/public/wise/external_accounts.go

@@ -10,6 +10,7 @@ import (
 	"github.com/formancehq/payments/internal/connectors/plugins/currency"
 	"github.com/formancehq/payments/internal/connectors/plugins/public/wise/client"
 	"github.com/formancehq/payments/internal/models"
+	"github.com/formancehq/payments/internal/utils/pagination"
 )
 
 type externalAccountsState struct {
@@ -37,50 +38,35 @@ func (p *Plugin) fetchExternalAccounts(ctx context.Context, req models.FetchNext
 	}
 
 	var accounts []models.PSPAccount
+	needMore := false
 	hasMore := false
+	lastSeekPosition := oldState.LastSeekPosition
 	for {
-		pagedExternalAccounts, err := p.client.GetRecipientAccounts(ctx, from.ID, req.PageSize, newState.LastSeekPosition)
+		pagedExternalAccounts, err := p.client.GetRecipientAccounts(ctx, from.ID, req.PageSize, lastSeekPosition)
 		if err != nil {
 			return models.FetchNextExternalAccountsResponse{}, err
 		}
 
-		if len(pagedExternalAccounts.Content) == 0 {
-			break
-		}
-
-		for _, externalAccount := range pagedExternalAccounts.Content {
-			if externalAccount.ID <= oldState.LastSeekPosition {
-				continue
-			}
-
-			raw, err := json.Marshal(externalAccount)
-			if err != nil {
-				return models.FetchNextExternalAccountsResponse{}, err
-			}
-
-			accounts = append(accounts, models.PSPAccount{
-				Reference:    strconv.FormatUint(externalAccount.ID, 10),
-				CreatedAt:    time.Now().UTC(),
-				Name:         &externalAccount.Name.FullName,
-				DefaultAsset: pointer.For(currency.FormatAsset(supportedCurrenciesWithDecimal, externalAccount.Currency)),
-				Raw:          raw,
-			})
-
-			if len(accounts) >= req.PageSize {
-				break
-			}
+		accounts, err = fillExternalAccounts(pagedExternalAccounts, accounts, oldState)
+		if err != nil {
+			return models.FetchNextExternalAccountsResponse{}, err
 		}
 
-		newState.LastSeekPosition = pagedExternalAccounts.SeekPositionForNext
-		if len(accounts) >= req.PageSize {
-			hasMore = true
+		lastSeekPosition = pagedExternalAccounts.SeekPositionForNext
+		needMore, hasMore = pagination.ShouldFetchMore(accounts, pagedExternalAccounts.Content, req.PageSize)
+		if !needMore || !hasMore {
 			break
 		}
+	}
 
-		if pagedExternalAccounts.SeekPositionForNext == 0 {
-			// No more data to fetch
-			break
-		}
+	if !needMore {
+		accounts = accounts[:req.PageSize]
+	}
+
+	if len(accounts) > 0 {
+		// No need to check the error, it's already checked in the fillExternalAccounts function
+		id, _ := strconv.ParseUint(accounts[len(accounts)-1].Reference, 10, 64)
+		newState.LastSeekPosition = id
 	}
 
 	payload, err := json.Marshal(newState)
@@ -94,3 +80,30 @@ func (p *Plugin) fetchExternalAccounts(ctx context.Context, req models.FetchNext
 		HasMore:          hasMore,
 	}, nil
 }
+
+func fillExternalAccounts(
+	pagedExternalAccounts *client.RecipientAccountsResponse,
+	accounts []models.PSPAccount,
+	oldState externalAccountsState,
+) ([]models.PSPAccount, error) {
+	for _, externalAccount := range pagedExternalAccounts.Content {
+		if oldState.LastSeekPosition != 0 && externalAccount.ID <= oldState.LastSeekPosition {
+			continue
+		}
+
+		raw, err := json.Marshal(externalAccount)
+		if err != nil {
+			return nil, err
+		}
+
+		accounts = append(accounts, models.PSPAccount{
+			Reference:    strconv.FormatUint(externalAccount.ID, 10),
+			CreatedAt:    time.Now().UTC(),
+			Name:         &externalAccount.Name.FullName,
+			DefaultAsset: pointer.For(currency.FormatAsset(supportedCurrenciesWithDecimal, externalAccount.Currency)),
+			Raw:          raw,
+		})
+	}
+
+	return accounts, nil
+}