clean: some todo

formancehq · Oct 16, 2024 · f16edd8 · f16edd8
1 parent 9752f1e
commit f16edd8
Show file tree

Hide file tree

Showing 15 changed files with 136 additions and 125 deletions.
diff --git a/go.mod b/go.mod
@@ -16,7 +16,7 @@ require (
 	github.com/antlr/antlr4/runtime/Go/antlr v1.4.10
 	github.com/bluele/gcache v0.0.2
 	github.com/formancehq/formance-sdk-go/v2 v2.3.1
-	github.com/formancehq/go-libs v1.7.2-0.20240924132015-19831aaf64de
+	github.com/formancehq/go-libs v1.7.2-0.20240924160248-25cc3741eba3
 	github.com/formancehq/stack/ledger/client v0.0.0-00010101000000-000000000000
 	github.com/go-chi/chi/v5 v5.1.0
 	github.com/go-chi/cors v1.2.1

diff --git a/go.sum b/go.sum
@@ -89,8 +89,8 @@ github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4=
 github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI=
 github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg=
 github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
-github.com/formancehq/go-libs v1.7.2-0.20240924132015-19831aaf64de h1:lIUxdWJwm2YaSnS6SsYqvMfJodV3Rk0qREReePjaY6k=
-github.com/formancehq/go-libs v1.7.2-0.20240924132015-19831aaf64de/go.mod h1:ynmWBbsdhVyjE+MxneMErtgd/RnNAk892VuIhZE2fps=
+github.com/formancehq/go-libs v1.7.2-0.20240924160248-25cc3741eba3 h1:kVhzD+1pd8U8e423d15DQTscWAD4602VdRomuIebqiE=
+github.com/formancehq/go-libs v1.7.2-0.20240924160248-25cc3741eba3/go.mod h1:ynmWBbsdhVyjE+MxneMErtgd/RnNAk892VuIhZE2fps=
 github.com/fortytw2/leaktest v1.3.0 h1:u8491cBMTQ8ft8aeV+adlcytMZylmA5nnwwkRZjI8vw=
 github.com/fortytw2/leaktest v1.3.0/go.mod h1:jDsjWgpAGjm2CA7WthBh/CdZYEPF31XHquHwclZch5g=
 github.com/go-chi/chi v4.1.2+incompatible h1:fGFk2Gmi/YKXk0OmGfBh0WgmN3XB8lVnEyNz34tQRec=

diff --git a/internal/api/v1/controllers_stats.go b/internal/api/v1/controllers_stats.go
@@ -10,7 +10,7 @@ import (
 func getStats(w http.ResponseWriter, r *http.Request) {
 	l := common.LedgerFromContext(r.Context())
 
-	stats, err := l.Stats(r.Context())
+	stats, err := l.GetStats(r.Context())
 	if err != nil {
 		api.InternalServerError(w, r, err)
 		return

diff --git a/internal/api/v1/controllers_stats_test.go b/internal/api/v1/controllers_stats_test.go
@@ -24,7 +24,7 @@ func TestGetStats(t *testing.T) {
 	}
 
 	ledgerController.EXPECT().
-		Stats(gomock.Any()).
+		GetStats(gomock.Any()).
 		Return(expectedStats, nil)
 
 	req := httptest.NewRequest(http.MethodGet, "/xxx/stats", nil)

diff --git a/internal/api/v2/controllers_stats.go b/internal/api/v2/controllers_stats.go
@@ -10,7 +10,7 @@ import (
 func readStats(w http.ResponseWriter, r *http.Request) {
 	l := common.LedgerFromContext(r.Context())
 
-	stats, err := l.Stats(r.Context())
+	stats, err := l.GetStats(r.Context())
 	if err != nil {
 		api.InternalServerError(w, r, err)
 		return

diff --git a/internal/api/v2/controllers_stats_test.go b/internal/api/v2/controllers_stats_test.go
@@ -24,7 +24,7 @@ func TestStats(t *testing.T) {
 	}
 
 	ledgerController.EXPECT().
-		Stats(gomock.Any()).
+		GetStats(gomock.Any()).
 		Return(expectedStats, nil)
 
 	req := httptest.NewRequest(http.MethodGet, "/xxx/stats", nil)

diff --git a/internal/controller/ledger/controller.go b/internal/controller/ledger/controller.go
@@ -12,18 +12,22 @@ import (
 //go:generate mockgen -source controller.go -destination controller_generated.go -package ledger . Controller
 
 type Controller interface {
+	// IsDatabaseUpToDate check if the ledger store is up to date, including the bucket and the ledger specifics
+	// It returns true if up to date
+	IsDatabaseUpToDate(ctx context.Context) (bool, error)
+	GetMigrationsInfo(ctx context.Context) ([]migrations.Info, error)
+	GetStats(ctx context.Context) (Stats, error)
+
 	GetAccount(ctx context.Context, query GetAccountQuery) (*ledger.ExpandedAccount, error)
 	ListAccounts(ctx context.Context, query ListAccountsQuery) (*bunpaginate.Cursor[ledger.ExpandedAccount], error)
 	CountAccounts(ctx context.Context, query ListAccountsQuery) (int, error)
-	GetAggregatedBalances(ctx context.Context, q GetAggregatedBalanceQuery) (ledger.BalancesByAssets, error)
-	GetMigrationsInfo(ctx context.Context) ([]migrations.Info, error)
-	Stats(ctx context.Context) (Stats, error)
 	ListLogs(ctx context.Context, query GetLogsQuery) (*bunpaginate.Cursor[ledger.Log], error)
 	CountTransactions(ctx context.Context, query ListTransactionsQuery) (int, error)
 	ListTransactions(ctx context.Context, query ListTransactionsQuery) (*bunpaginate.Cursor[ledger.ExpandedTransaction], error)
 	GetTransaction(ctx context.Context, query GetTransactionQuery) (*ledger.ExpandedTransaction, error)
+	GetVolumesWithBalances(ctx context.Context, q GetVolumesWithBalancesQuery) (*bunpaginate.Cursor[ledger.VolumesWithBalanceByAssetByAccount], error)
+	GetAggregatedBalances(ctx context.Context, q GetAggregatedBalanceQuery) (ledger.BalancesByAssets, error)
 
-	// todo: trace errors and report on interfaces
 	// CreateTransaction accept a numscript script and returns a CreatedTransaction
 	// It can return following errors:
 	//  * ErrCompilationFailed
@@ -33,15 +37,34 @@ type Controller interface {
 	//  * ErrIdempotencyKeyConflict
 	//  * ErrInsufficientFunds
 	CreateTransaction(ctx context.Context, parameters Parameters, data ledger.RunScript) (*ledger.Transaction, error)
+	// RevertTransaction allow to revert a transaction.
+	// It can return following errors:
+	//  * ErrInsufficientFunds
+	//  * ErrAlreadyReverted
+	//  * ErrNotFound
+	// Parameter force indicate we want to force revert the transaction even if the accounts does not have funds
+	// Parameter atEffectiveDate indicate we want to set the timestamp of the newly created transaction on the timestamp of the reverted transaction
 	RevertTransaction(ctx context.Context, parameters Parameters, id int, force, atEffectiveDate bool) (*ledger.Transaction, error)
+	// SaveTransactionMetadata allow to add metadata to an existing transaction
+	// It can return following errors:
+	//  * ErrNotFound
 	SaveTransactionMetadata(ctx context.Context, parameters Parameters, id int, m metadata.Metadata) error
+	// SaveAccountMetadata allow to add metadata to an account
+	// If the account does not exist, it is created
 	SaveAccountMetadata(ctx context.Context, parameters Parameters, id string, m metadata.Metadata) error
+	// DeleteTransactionMetadata allow to remove metadata of a transaction
+	// It can return following errors:
+	//  * ErrNotFound : indicate the transaction was not found OR the metadata does not exist on the transaction
 	DeleteTransactionMetadata(ctx context.Context, parameters Parameters, id int, key string) error
+	// DeleteAccountMetadata allow to remove metadata of an account
+	// It can return following errors:
+	//  * ErrNotFound : indicate the account was not found OR the metadata does not exist on the account
 	DeleteAccountMetadata(ctx context.Context, parameters Parameters, targetID string, key string) error
+	// Import allow to import the logs of an existing ledger
+	// It can return following errors:
+	//  * ErrImport
+	// Logs hash must be valid and the ledger.Ledger must be in 'initializing' state
 	Import(ctx context.Context, stream chan ledger.Log) error
+	// Export allow to export the logs of a ledger
 	Export(ctx context.Context, w ExportWriter) error
-
-	IsDatabaseUpToDate(ctx context.Context) (bool, error)
-
-	GetVolumesWithBalances(ctx context.Context, q GetVolumesWithBalancesQuery) (*bunpaginate.Cursor[ledger.VolumesWithBalanceByAssetByAccount], error)
 }
diff --git a/internal/controller/ledger/controller_default.go b/internal/controller/ledger/controller_default.go
@@ -315,7 +315,6 @@ func (ctrl *DefaultController) CreateTransaction(ctx context.Context, parameters
 				finalMetadata[k] = v
 			}
 
-			// todo: define timestamp if not already defined
 			transaction := ledger.NewTransaction().
 				WithPostings(result.Postings...).
 				WithMetadata(finalMetadata).
@@ -391,6 +390,7 @@ func (ctrl *DefaultController) RevertTransaction(ctx context.Context, parameters
 					for asset, finalBalance := range forAccount {
 						if finalBalance.Cmp(new(big.Int)) < 0 {
 							// todo: break dependency on machine package
+							// notes(gfyrag): wait for the new interpreter
 							return nil, machine.NewErrInsufficientFund("insufficient fund for %s/%s", account, asset)
 						}
 					}

diff --git a/internal/controller/ledger/controller_generated.go b/internal/controller/ledger/controller_generated.go
diff --git a/internal/controller/ledger/stats.go b/internal/controller/ledger/stats.go
@@ -11,7 +11,7 @@ type Stats struct {
 	Accounts     int `json:"accounts"`
 }
 
-func (ctrl *DefaultController) Stats(ctx context.Context) (Stats, error) {
+func (ctrl *DefaultController) GetStats(ctx context.Context) (Stats, error) {
 	var stats Stats
 
 	transactions, err := ctrl.store.CountTransactions(ctx, NewListTransactionsQuery(NewPaginatedQueryOptions(PITFilterWithVolumes{})))

diff --git a/internal/controller/ledger/stats_test.go b/internal/controller/ledger/stats_test.go
@@ -30,7 +30,7 @@ func TestStats(t *testing.T) {
 		NewMockListener(ctrl),
 		machineFactory,
 	)
-	stats, err := ledgerController.Stats(ctx)
+	stats, err := ledgerController.GetStats(ctx)
 	require.NoError(t, err)
 	require.Equal(t, 10, stats.Transactions)
 	require.Equal(t, 10, stats.Accounts)

diff --git a/internal/machine/vm/machine.go b/internal/machine/vm/machine.go
@@ -552,7 +552,6 @@ func (m *Machine) ResolveBalances(ctx context.Context, store Store) error {
 }
 
 func (m *Machine) ResolveResources(ctx context.Context, store Store) error {
-	//TODO(gfyrag): Is that really required? Feel like defensive programming.
 	if m.resolveCalled {
 		return errors.New("tried to call ResolveResources twice")
 	}

diff --git a/internal/machine/vm/machine_test.go b/internal/machine/vm/machine_test.go
@@ -1729,7 +1729,6 @@ func TestSetVarsFromJSON(t *testing.T) {
 			err = m.SetVarsFromJSON(tc.vars)
 			if tc.expectedError != nil {
 				require.Error(t, err)
-				//TODO(gfyrag): refine error handling of SetVars/ResolveResources/ResolveBalances
 				require.Equal(t, tc.expectedError.Error(), err.Error())
 			} else {
 				require.Nil(t, err)

diff --git a/internal/storage/ledger/store.go b/internal/storage/ledger/store.go
@@ -39,6 +39,8 @@ func (s *Store) WithDB(db bun.IDB) *Store {
 	}
 }
 
+// todo: merge with bucket migration info
+// todo: add test
 func (s *Store) GetMigrationsInfo(ctx context.Context) ([]migrations.Info, error) {
 	return getMigrator(s.ledger).GetMigrations(ctx, s.db)
 }