From de6b39fe8329459afbf82041a2aa713b7a368507 Mon Sep 17 00:00:00 2001 From: Maxence Maireaux Date: Fri, 16 May 2025 12:24:07 +0200 Subject: [PATCH] fix: return 400 instead of 500 for idempotency key reuse with different body --- .gitignore | 1 + docs/api/README.md | 2 + internal/api/common/errors.go | 17 +- .../v1/controllers_accounts_add_metadata.go | 8 +- .../controllers_accounts_delete_metadata.go | 5 +- .../controllers_transactions_add_metadata.go | 3 +- .../api/v1/controllers_transactions_create.go | 5 +- ...ontrollers_transactions_delete_metadata.go | 8 +- .../v2/controllers_accounts_add_metadata.go | 6 +- .../controllers_accounts_delete_metadata.go | 5 +- .../v2/controllers_ledgers_delete_metadata.go | 5 +- .../v2/controllers_ledgers_update_metadata.go | 6 +- .../controllers_transactions_add_metadata.go | 3 +- .../api/v2/controllers_transactions_create.go | 7 +- ...ontrollers_transactions_delete_metadata.go | 8 +- .../api/v2/controllers_transactions_revert.go | 1 + openapi.yaml | 10 + openapi/v2.yaml | 10 + pkg/client/.speakeasy/gen.lock | 4 +- .../v2deleteaccountmetadatarequest.md | 11 +- .../v2deletetransactionmetadatarequest.md | 3 +- .../operations/v2deleteaccountmetadata.go | 9 + .../operations/v2deletetransactionmetadata.go | 9 + pkg/client/v2.go | 4 + test/e2e/api_accounts_metadata_test.go | 177 ++++++++++++++++- test/e2e/api_idempotency_test.go | 186 ++++++++++++++++++ test/e2e/api_transactions_metadata_test.go | 166 ++++++++++++++-- 27 files changed, 622 insertions(+), 57 deletions(-) create mode 100644 test/e2e/api_idempotency_test.go diff --git a/.gitignore b/.gitignore index d4e5695b2f..a7d0c4cddd 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ dumps .env !.envrc Pulumi.*.yaml +coverage.txt \ No newline at end of file diff --git a/docs/api/README.md b/docs/api/README.md index 7b9e0470f2..9d87f9b890 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -966,6 +966,7 @@ Authorization ( Scopes: ledger:write ) DELETE http://localhost:8080/v2/{ledger}/transactions/{id}/metadata/{key} HTTP/1.1 Host: localhost:8080 Accept: application/json +Idempotency-Key: string ``` @@ -980,6 +981,7 @@ Delete metadata by key |ledger|path|string|true|Name of the ledger.| |id|path|integer(bigint)|true|Transaction ID.| |key|path|string|true|The key to remove.| +|Idempotency-Key|header|string|false|Use an idempotency key| > Example responses diff --git a/internal/api/common/errors.go b/internal/api/common/errors.go index d9738fa432..9a16e5790a 100644 --- a/internal/api/common/errors.go +++ b/internal/api/common/errors.go @@ -2,11 +2,13 @@ package common import ( "errors" + "net/http" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/go-libs/v3/logging" "github.com/formancehq/go-libs/v3/otlp" "github.com/formancehq/go-libs/v3/platform/postgres" - "net/http" + ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger" ) const ( @@ -37,6 +39,19 @@ func HandleCommonErrors(w http.ResponseWriter, r *http.Request, err error) { } } +func HandleCommonWriteErrors(w http.ResponseWriter, r *http.Request, err error) { + switch { + case errors.Is(err, ledgercontroller.ErrIdempotencyKeyConflict{}): + api.WriteErrorResponse(w, http.StatusConflict, ErrConflict, err) + case errors.Is(err, ledgercontroller.ErrInvalidIdempotencyInput{}): + api.BadRequest(w, ErrValidation, err) + case errors.Is(err, ledgercontroller.ErrNotFound): + api.NotFound(w, err) + default: + HandleCommonErrors(w, r, err) + } +} + func InternalServerError(w http.ResponseWriter, r *http.Request, err error) { otlp.RecordError(r.Context(), err) logging.FromContext(r.Context()).Error(err) diff --git a/internal/api/v1/controllers_accounts_add_metadata.go b/internal/api/v1/controllers_accounts_add_metadata.go index 6e78af89db..defcf19da6 100644 --- a/internal/api/v1/controllers_accounts_add_metadata.go +++ b/internal/api/v1/controllers_accounts_add_metadata.go @@ -2,12 +2,14 @@ package v1 import ( "encoding/json" - "github.com/formancehq/ledger/internal/controller/ledger" - "github.com/formancehq/ledger/pkg/accounts" "net/http" "net/url" + "github.com/formancehq/ledger/internal/controller/ledger" + "github.com/formancehq/ledger/pkg/accounts" + "errors" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/go-libs/v3/metadata" "github.com/formancehq/ledger/internal/api/common" @@ -38,7 +40,7 @@ func addAccountMetadata(w http.ResponseWriter, r *http.Request) { Metadata: m, })) if err != nil { - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) return } diff --git a/internal/api/v1/controllers_accounts_delete_metadata.go b/internal/api/v1/controllers_accounts_delete_metadata.go index 160a6742d6..6861d6392f 100644 --- a/internal/api/v1/controllers_accounts_delete_metadata.go +++ b/internal/api/v1/controllers_accounts_delete_metadata.go @@ -1,10 +1,11 @@ package v1 import ( - "github.com/formancehq/ledger/internal/controller/ledger" "net/http" "net/url" + "github.com/formancehq/ledger/internal/controller/ledger" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/ledger/internal/api/common" "github.com/go-chi/chi/v5" @@ -25,7 +26,7 @@ func deleteAccountMetadata(w http.ResponseWriter, r *http.Request) { Key: chi.URLParam(r, "key"), }), ); err != nil { - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) return } diff --git a/internal/api/v1/controllers_transactions_add_metadata.go b/internal/api/v1/controllers_transactions_add_metadata.go index 794ce0a9b0..b9ca80e0cf 100644 --- a/internal/api/v1/controllers_transactions_add_metadata.go +++ b/internal/api/v1/controllers_transactions_add_metadata.go @@ -8,6 +8,7 @@ import ( ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger" "errors" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/go-libs/v3/metadata" "github.com/formancehq/ledger/internal/api/common" @@ -37,7 +38,7 @@ func addTransactionMetadata(w http.ResponseWriter, r *http.Request) { case errors.Is(err, ledgercontroller.ErrNotFound): api.NotFound(w, err) default: - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) } return } diff --git a/internal/api/v1/controllers_transactions_create.go b/internal/api/v1/controllers_transactions_create.go index 115459f1e5..f61e278744 100644 --- a/internal/api/v1/controllers_transactions_create.go +++ b/internal/api/v1/controllers_transactions_create.go @@ -9,6 +9,7 @@ import ( ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger" "errors" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/go-libs/v3/metadata" "github.com/formancehq/go-libs/v3/time" @@ -100,7 +101,7 @@ func createTransaction(w http.ResponseWriter, r *http.Request) { case errors.Is(err, ledgercontroller.ErrTransactionReferenceConflict{}): api.WriteErrorResponse(w, http.StatusConflict, common.ErrConflict, err) default: - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) } return } @@ -135,7 +136,7 @@ func createTransaction(w http.ResponseWriter, r *http.Request) { case errors.Is(err, ledgercontroller.ErrTransactionReferenceConflict{}): api.WriteErrorResponse(w, http.StatusConflict, common.ErrConflict, err) default: - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) } return } diff --git a/internal/api/v1/controllers_transactions_delete_metadata.go b/internal/api/v1/controllers_transactions_delete_metadata.go index 4ce1012797..7527f711f2 100644 --- a/internal/api/v1/controllers_transactions_delete_metadata.go +++ b/internal/api/v1/controllers_transactions_delete_metadata.go @@ -7,6 +7,7 @@ import ( ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger" "errors" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/ledger/internal/api/common" "github.com/go-chi/chi/v5" @@ -27,12 +28,7 @@ func deleteTransactionMetadata(w http.ResponseWriter, r *http.Request) { TransactionID: transactionID, Key: metadataKey, })); err != nil { - switch { - case errors.Is(err, ledgercontroller.ErrNotFound): - api.NotFound(w, err) - default: - common.HandleCommonErrors(w, r, err) - } + common.HandleCommonWriteErrors(w, r, err) return } diff --git a/internal/api/v2/controllers_accounts_add_metadata.go b/internal/api/v2/controllers_accounts_add_metadata.go index 18070b6823..d337d46b4f 100644 --- a/internal/api/v2/controllers_accounts_add_metadata.go +++ b/internal/api/v2/controllers_accounts_add_metadata.go @@ -2,11 +2,13 @@ package v2 import ( "encoding/json" - "github.com/formancehq/ledger/internal/controller/ledger" "net/http" "net/url" + "github.com/formancehq/ledger/internal/controller/ledger" + "errors" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/go-libs/v3/metadata" "github.com/formancehq/ledger/internal/api/common" @@ -33,7 +35,7 @@ func addAccountMetadata(w http.ResponseWriter, r *http.Request) { Metadata: m, })) if err != nil { - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) return } diff --git a/internal/api/v2/controllers_accounts_delete_metadata.go b/internal/api/v2/controllers_accounts_delete_metadata.go index 17f568701c..32d67c1e4b 100644 --- a/internal/api/v2/controllers_accounts_delete_metadata.go +++ b/internal/api/v2/controllers_accounts_delete_metadata.go @@ -1,10 +1,11 @@ package v2 import ( - "github.com/formancehq/ledger/internal/controller/ledger" "net/http" "net/url" + "github.com/formancehq/ledger/internal/controller/ledger" + "github.com/go-chi/chi/v5" "github.com/formancehq/go-libs/v3/api" @@ -26,7 +27,7 @@ func deleteAccountMetadata(w http.ResponseWriter, r *http.Request) { Key: chi.URLParam(r, "key"), }), ); err != nil { - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) return } diff --git a/internal/api/v2/controllers_ledgers_delete_metadata.go b/internal/api/v2/controllers_ledgers_delete_metadata.go index 32cf4804a0..a7fa6e681d 100644 --- a/internal/api/v2/controllers_ledgers_delete_metadata.go +++ b/internal/api/v2/controllers_ledgers_delete_metadata.go @@ -1,9 +1,10 @@ package v2 import ( - "github.com/formancehq/ledger/internal/api/common" "net/http" + "github.com/formancehq/ledger/internal/api/common" + "github.com/formancehq/ledger/internal/controller/system" "github.com/formancehq/go-libs/v3/api" @@ -13,7 +14,7 @@ import ( func deleteLedgerMetadata(b system.Controller) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if err := b.DeleteLedgerMetadata(r.Context(), chi.URLParam(r, "ledger"), chi.URLParam(r, "key")); err != nil { - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) return } diff --git a/internal/api/v2/controllers_ledgers_update_metadata.go b/internal/api/v2/controllers_ledgers_update_metadata.go index 8f95ae443f..fca8e5dcbf 100644 --- a/internal/api/v2/controllers_ledgers_update_metadata.go +++ b/internal/api/v2/controllers_ledgers_update_metadata.go @@ -2,10 +2,12 @@ package v2 import ( "encoding/json" - "github.com/formancehq/ledger/internal/api/common" "net/http" + "github.com/formancehq/ledger/internal/api/common" + "errors" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/go-libs/v3/metadata" systemcontroller "github.com/formancehq/ledger/internal/controller/system" @@ -22,7 +24,7 @@ func updateLedgerMetadata(systemController systemcontroller.Controller) http.Han } if err := systemController.UpdateLedgerMetadata(r.Context(), chi.URLParam(r, "ledger"), m); err != nil { - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) return } diff --git a/internal/api/v2/controllers_transactions_add_metadata.go b/internal/api/v2/controllers_transactions_add_metadata.go index 399553beae..2bb49a371a 100644 --- a/internal/api/v2/controllers_transactions_add_metadata.go +++ b/internal/api/v2/controllers_transactions_add_metadata.go @@ -8,6 +8,7 @@ import ( ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger" "errors" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/go-libs/v3/metadata" "github.com/formancehq/ledger/internal/api/common" @@ -37,7 +38,7 @@ func addTransactionMetadata(w http.ResponseWriter, r *http.Request) { case errors.Is(err, ledgercontroller.ErrNotFound): api.NotFound(w, err) default: - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) } return } diff --git a/internal/api/v2/controllers_transactions_create.go b/internal/api/v2/controllers_transactions_create.go index 180871bfa0..013f188825 100644 --- a/internal/api/v2/controllers_transactions_create.go +++ b/internal/api/v2/controllers_transactions_create.go @@ -2,9 +2,10 @@ package v2 import ( "encoding/json" - "github.com/formancehq/ledger/internal/api/bulking" "net/http" + "github.com/formancehq/ledger/internal/api/bulking" + ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger" "errors" @@ -51,14 +52,12 @@ func createTransaction(w http.ResponseWriter, r *http.Request) { api.BadRequest(w, common.ErrNoPostings, err) case errors.Is(err, ledgercontroller.ErrTransactionReferenceConflict{}): api.WriteErrorResponse(w, http.StatusConflict, common.ErrConflict, err) - case errors.Is(err, ledgercontroller.ErrInvalidIdempotencyInput{}): - api.BadRequest(w, common.ErrValidation, err) case errors.Is(err, ledgercontroller.ErrParsing{}): api.BadRequest(w, common.ErrInterpreterParse, err) case errors.Is(err, ledgercontroller.ErrRuntime{}): api.BadRequest(w, common.ErrInterpreterRuntime, err) default: - common.HandleCommonErrors(w, r, err) + common.HandleCommonWriteErrors(w, r, err) } return } diff --git a/internal/api/v2/controllers_transactions_delete_metadata.go b/internal/api/v2/controllers_transactions_delete_metadata.go index 3cee6e63b8..2380075699 100644 --- a/internal/api/v2/controllers_transactions_delete_metadata.go +++ b/internal/api/v2/controllers_transactions_delete_metadata.go @@ -8,7 +8,6 @@ import ( "github.com/go-chi/chi/v5" - "errors" "github.com/formancehq/ledger/internal/api/common" "github.com/formancehq/go-libs/v3/api" @@ -29,12 +28,7 @@ func deleteTransactionMetadata(w http.ResponseWriter, r *http.Request) { TransactionID: txID, Key: metadataKey, })); err != nil { - switch { - case errors.Is(err, ledgercontroller.ErrNotFound): - api.NotFound(w, err) - default: - common.HandleCommonErrors(w, r, err) - } + common.HandleCommonWriteErrors(w, r, err) return } diff --git a/internal/api/v2/controllers_transactions_revert.go b/internal/api/v2/controllers_transactions_revert.go index be1a65161d..b35ddc1814 100644 --- a/internal/api/v2/controllers_transactions_revert.go +++ b/internal/api/v2/controllers_transactions_revert.go @@ -7,6 +7,7 @@ import ( ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger" "errors" + "github.com/formancehq/go-libs/v3/api" "github.com/formancehq/ledger/internal/api/common" "github.com/go-chi/chi/v5" diff --git a/openapi.yaml b/openapi.yaml index b13c963f76..23d751dea4 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1864,6 +1864,11 @@ paths: schema: type: string example: foo + - name: Idempotency-Key + in: header + description: Use an idempotency key + schema: + type: string responses: 2XX: description: Key deleted @@ -2241,6 +2246,11 @@ paths: schema: type: string example: foo + - name: Idempotency-Key + in: header + description: Use an idempotency key + schema: + type: string responses: 2XX: description: Key deleted diff --git a/openapi/v2.yaml b/openapi/v2.yaml index eb2e2880b7..d64c698872 100644 --- a/openapi/v2.yaml +++ b/openapi/v2.yaml @@ -604,6 +604,11 @@ paths: schema: type: string example: foo + - name: Idempotency-Key + in: header + description: Use an idempotency key + schema: + type: string responses: 2XX: description: Key deleted @@ -983,6 +988,11 @@ paths: schema: type: string example: foo + - name: Idempotency-Key + in: header + description: Use an idempotency key + schema: + type: string responses: 2XX: description: Key deleted diff --git a/pkg/client/.speakeasy/gen.lock b/pkg/client/.speakeasy/gen.lock index 0afa074ca6..25da8bb7c8 100644 --- a/pkg/client/.speakeasy/gen.lock +++ b/pkg/client/.speakeasy/gen.lock @@ -1,7 +1,7 @@ lockVersion: 2.0.0 id: a9ac79e1-e429-4ee3-96c4-ec973f19bec3 management: - docChecksum: ba1cd35a327e7c3ca6b907a3a850d1ac + docChecksum: aa90e15d37f175280618bd0108917475 docVersion: v2 speakeasyVersion: 1.517.3 generationVersion: 2.548.6 @@ -785,6 +785,7 @@ examples: ledger: "ledger001" address: "96609 Cummings Canyon" key: "foo" + header: {} responses: default: application/json: {"errorCode": "VALIDATION", "errorMessage": "[VALIDATION] invalid 'cursor' query param", "details": "https://play.numscript.org/?payload=eyJlcnJvciI6ImFjY291bnQgaGFkIGluc3VmZmljaWVudCBmdW5kcyJ9"} @@ -873,6 +874,7 @@ examples: ledger: "ledger001" id: 1234 key: "foo" + header: {} responses: default: application/json: {"errorCode": "VALIDATION", "errorMessage": "[VALIDATION] invalid 'cursor' query param", "details": "https://play.numscript.org/?payload=eyJlcnJvciI6ImFjY291bnQgaGFkIGluc3VmZmljaWVudCBmdW5kcyJ9"} diff --git a/pkg/client/docs/models/operations/v2deleteaccountmetadatarequest.md b/pkg/client/docs/models/operations/v2deleteaccountmetadatarequest.md index 1a9c58e7fc..e4a6a157aa 100644 --- a/pkg/client/docs/models/operations/v2deleteaccountmetadatarequest.md +++ b/pkg/client/docs/models/operations/v2deleteaccountmetadatarequest.md @@ -3,8 +3,9 @@ ## Fields -| Field | Type | Required | Description | Example | -| ------------------- | ------------------- | ------------------- | ------------------- | ------------------- | -| `Ledger` | *string* | :heavy_check_mark: | Name of the ledger. | ledger001 | -| `Address` | *string* | :heavy_check_mark: | Account address | | -| `Key` | *string* | :heavy_check_mark: | The key to remove. | foo | \ No newline at end of file +| Field | Type | Required | Description | Example | +| ---------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- | +| `Ledger` | *string* | :heavy_check_mark: | Name of the ledger. | ledger001 | +| `Address` | *string* | :heavy_check_mark: | Account address | | +| `Key` | *string* | :heavy_check_mark: | The key to remove. | foo | +| `IdempotencyKey` | **string* | :heavy_minus_sign: | Use an idempotency key | | \ No newline at end of file diff --git a/pkg/client/docs/models/operations/v2deletetransactionmetadatarequest.md b/pkg/client/docs/models/operations/v2deletetransactionmetadatarequest.md index 4d2ba6adf4..fe1844b748 100644 --- a/pkg/client/docs/models/operations/v2deletetransactionmetadatarequest.md +++ b/pkg/client/docs/models/operations/v2deletetransactionmetadatarequest.md @@ -7,4 +7,5 @@ | ------------------------------------------- | ------------------------------------------- | ------------------------------------------- | ------------------------------------------- | ------------------------------------------- | | `Ledger` | *string* | :heavy_check_mark: | Name of the ledger. | ledger001 | | `ID` | [*big.Int](https://pkg.go.dev/math/big#Int) | :heavy_check_mark: | Transaction ID. | 1234 | -| `Key` | *string* | :heavy_check_mark: | The key to remove. | foo | \ No newline at end of file +| `Key` | *string* | :heavy_check_mark: | The key to remove. | foo | +| `IdempotencyKey` | **string* | :heavy_minus_sign: | Use an idempotency key | | \ No newline at end of file diff --git a/pkg/client/models/operations/v2deleteaccountmetadata.go b/pkg/client/models/operations/v2deleteaccountmetadata.go index aee0f853fb..c3fbf196c4 100644 --- a/pkg/client/models/operations/v2deleteaccountmetadata.go +++ b/pkg/client/models/operations/v2deleteaccountmetadata.go @@ -13,6 +13,8 @@ type V2DeleteAccountMetadataRequest struct { Address string `pathParam:"style=simple,explode=false,name=address"` // The key to remove. Key string `pathParam:"style=simple,explode=false,name=key"` + // Use an idempotency key + IdempotencyKey *string `header:"style=simple,explode=false,name=Idempotency-Key"` } func (o *V2DeleteAccountMetadataRequest) GetLedger() string { @@ -36,6 +38,13 @@ func (o *V2DeleteAccountMetadataRequest) GetKey() string { return o.Key } +func (o *V2DeleteAccountMetadataRequest) GetIdempotencyKey() *string { + if o == nil { + return nil + } + return o.IdempotencyKey +} + type V2DeleteAccountMetadataResponse struct { HTTPMeta components.HTTPMetadata `json:"-"` } diff --git a/pkg/client/models/operations/v2deletetransactionmetadata.go b/pkg/client/models/operations/v2deletetransactionmetadata.go index 7ad4d88ba2..fe774941a1 100644 --- a/pkg/client/models/operations/v2deletetransactionmetadata.go +++ b/pkg/client/models/operations/v2deletetransactionmetadata.go @@ -15,6 +15,8 @@ type V2DeleteTransactionMetadataRequest struct { ID *big.Int `pathParam:"style=simple,explode=false,name=id"` // The key to remove. Key string `pathParam:"style=simple,explode=false,name=key"` + // Use an idempotency key + IdempotencyKey *string `header:"style=simple,explode=false,name=Idempotency-Key"` } func (v V2DeleteTransactionMetadataRequest) MarshalJSON() ([]byte, error) { @@ -49,6 +51,13 @@ func (o *V2DeleteTransactionMetadataRequest) GetKey() string { return o.Key } +func (o *V2DeleteTransactionMetadataRequest) GetIdempotencyKey() *string { + if o == nil { + return nil + } + return o.IdempotencyKey +} + type V2DeleteTransactionMetadataResponse struct { HTTPMeta components.HTTPMetadata `json:"-"` } diff --git a/pkg/client/v2.go b/pkg/client/v2.go index 76dba5141b..35b1ae69fc 100644 --- a/pkg/client/v2.go +++ b/pkg/client/v2.go @@ -2342,6 +2342,8 @@ func (s *V2) DeleteAccountMetadata(ctx context.Context, request operations.V2Del req.Header.Set("Accept", "application/json") req.Header.Set("User-Agent", s.sdkConfiguration.UserAgent) + utils.PopulateHeaders(ctx, req, request, nil) + if err := utils.PopulateSecurity(ctx, req, s.sdkConfiguration.Security); err != nil { return nil, err } @@ -3767,6 +3769,8 @@ func (s *V2) DeleteTransactionMetadata(ctx context.Context, request operations.V req.Header.Set("Accept", "application/json") req.Header.Set("User-Agent", s.sdkConfiguration.UserAgent) + utils.PopulateHeaders(ctx, req, request, nil) + if err := utils.PopulateSecurity(ctx, req, s.sdkConfiguration.Security); err != nil { return nil, err } diff --git a/test/e2e/api_accounts_metadata_test.go b/test/e2e/api_accounts_metadata_test.go index 9a2b36d667..58861a8a95 100644 --- a/test/e2e/api_accounts_metadata_test.go +++ b/test/e2e/api_accounts_metadata_test.go @@ -4,7 +4,8 @@ package test_suite import ( "github.com/formancehq/go-libs/v3/logging" - "github.com/formancehq/go-libs/v3/testing/deferred/ginkgo" + "github.com/formancehq/go-libs/v3/pointer" + . "github.com/formancehq/go-libs/v3/testing/api" . "github.com/formancehq/go-libs/v3/testing/deferred/ginkgo" "github.com/formancehq/go-libs/v3/testing/platform/natstesting" "github.com/formancehq/go-libs/v3/testing/platform/pgtesting" @@ -13,6 +14,7 @@ import ( "github.com/formancehq/ledger/pkg/client/models/operations" ledgerevents "github.com/formancehq/ledger/pkg/events" . "github.com/formancehq/ledger/pkg/testserver" + "github.com/formancehq/ledger/pkg/testserver/ginkgo" . "github.com/formancehq/ledger/pkg/testserver/ginkgo" "github.com/nats-io/nats.go" . "github.com/onsi/ginkgo/v2" @@ -23,11 +25,11 @@ var _ = Context("Ledger accounts metadata API tests", func() { var ( db = UseTemplatedDatabase() ctx = logging.TestingContext() - natsURL = ginkgo.DeferMap(natsServer, (*natstesting.NatsServer).ClientURL) + natsURL = DeferMap(natsServer, (*natstesting.NatsServer).ClientURL) ) - testServer := DeferTestServer( - ginkgo.DeferMap(db, (*pgtesting.Database).ConnectionOptions), + testServer := ginkgo.DeferTestServer( + DeferMap(db, (*pgtesting.Database).ConnectionOptions), testservice.WithInstruments( testservice.NatsInstrumentation(natsURL), testservice.DebugInstrumentation(debug), @@ -165,4 +167,171 @@ var _ = Context("Ledger accounts metadata API tests", func() { }) }) }) + + When("using idempotency keys with account metadata", func() { + var ( + metadata = map[string]string{ + "type": "checking", + "tier": "premium", + } + ) + + It("should succeed when using the same idempotency key with same data", func(specContext SpecContext) { + // First call with idempotency key + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataToAccount( + ctx, + operations.V2AddMetadataToAccountRequest{ + RequestBody: metadata, + Address: "account-ik-test", + Ledger: "default", + IdempotencyKey: pointer.For("account-meta-key-1"), + }, + ) + Expect(err).ToNot(HaveOccurred()) + + // Second call with same idempotency key and same data + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataToAccount( + ctx, + operations.V2AddMetadataToAccountRequest{ + RequestBody: metadata, + Address: "account-ik-test", + Ledger: "default", + IdempotencyKey: pointer.For("account-meta-key-1"), + }, + ) + Expect(err).ToNot(HaveOccurred()) + + // Verify metadata was set correctly + response, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.GetAccount( + ctx, + operations.V2GetAccountRequest{ + Address: "account-ik-test", + Ledger: "default", + }, + ) + Expect(err).ToNot(HaveOccurred()) + Expect(response.V2AccountResponse.Data.Metadata).Should(Equal(metadata)) + }) + + It("should fail when using the same idempotency key with different data", func(specContext SpecContext) { + // First call with idempotency key + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataToAccount( + ctx, + operations.V2AddMetadataToAccountRequest{ + RequestBody: metadata, + Address: "account-ik-test-2", + Ledger: "default", + IdempotencyKey: pointer.For("account-meta-key-2"), + }, + ) + Expect(err).ToNot(HaveOccurred()) + + // Second call with same idempotency key but different data + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataToAccount( + ctx, + operations.V2AddMetadataToAccountRequest{ + RequestBody: map[string]string{ + "type": "savings", + "tier": "basic", + }, + Address: "account-ik-test-2", + Ledger: "default", + IdempotencyKey: pointer.For("account-meta-key-2"), + }, + ) + Expect(err).To(HaveOccurred()) + Expect(err).To(HaveErrorCode(string(components.V2ErrorsEnumValidation))) + }) + + It("should succeed when deleting metadata with idempotency key", func(specContext SpecContext) { + // First add metadata + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataToAccount( + ctx, + operations.V2AddMetadataToAccountRequest{ + RequestBody: metadata, + Address: "account-ik-delete-test", + Ledger: "default", + }, + ) + Expect(err).ToNot(HaveOccurred()) + + ikPtr := pointer.For("account-delete-key-1") + + // Delete metadata with idempotency key + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.DeleteAccountMetadata( + ctx, + operations.V2DeleteAccountMetadataRequest{ + Address: "account-ik-delete-test", + Ledger: "default", + Key: "type", + IdempotencyKey: ikPtr, + }, + ) + Expect(err).ToNot(HaveOccurred()) + + // Second delete with same idempotency key should succeed (idempotent) + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.DeleteAccountMetadata( + ctx, + operations.V2DeleteAccountMetadataRequest{ + Address: "account-ik-delete-test", + Ledger: "default", + Key: "type", + IdempotencyKey: ikPtr, + }, + ) + Expect(err).ToNot(HaveOccurred()) + + // Verify metadata was partially deleted + response, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.GetAccount( + ctx, + operations.V2GetAccountRequest{ + Address: "account-ik-delete-test", + Ledger: "default", + }, + ) + Expect(err).ToNot(HaveOccurred()) + Expect(response.V2AccountResponse.Data.Metadata).Should(Equal(map[string]string{ + "tier": "premium", + })) + }) + + It("should fail when deleting metadata with same idempotency key but different parameters", func(specContext SpecContext) { + // First add metadata + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataToAccount( + ctx, + operations.V2AddMetadataToAccountRequest{ + RequestBody: metadata, + Address: "account-ik-delete-test-2", + Ledger: "default", + }, + ) + Expect(err).ToNot(HaveOccurred()) + + ikPtr := pointer.For("account-delete-key-2") + + // Delete "type" metadata with idempotency key + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.DeleteAccountMetadata( + ctx, + operations.V2DeleteAccountMetadataRequest{ + Address: "account-ik-delete-test-2", + Ledger: "default", + Key: "type", + IdempotencyKey: ikPtr, + }, + ) + Expect(err).ToNot(HaveOccurred()) + + // Try to delete "tier" with same idempotency key should fail + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.DeleteAccountMetadata( + ctx, + operations.V2DeleteAccountMetadataRequest{ + Address: "account-ik-delete-test-2", + Ledger: "default", + Key: "tier", // Different key + IdempotencyKey: ikPtr, + }, + ) + Expect(err).To(HaveOccurred()) + }) + }) }) diff --git a/test/e2e/api_idempotency_test.go b/test/e2e/api_idempotency_test.go new file mode 100644 index 0000000000..aa4ca18756 --- /dev/null +++ b/test/e2e/api_idempotency_test.go @@ -0,0 +1,186 @@ +//go:build it + +package test_suite + +import ( + "math/big" + "time" + + "github.com/formancehq/go-libs/v3/logging" + "github.com/formancehq/go-libs/v3/pointer" + . "github.com/formancehq/go-libs/v3/testing/api" + . "github.com/formancehq/go-libs/v3/testing/deferred/ginkgo" + "github.com/formancehq/go-libs/v3/testing/platform/natstesting" + "github.com/formancehq/go-libs/v3/testing/platform/pgtesting" + "github.com/formancehq/go-libs/v3/testing/testservice" + "github.com/formancehq/ledger/pkg/client/models/components" + "github.com/formancehq/ledger/pkg/client/models/operations" + . "github.com/formancehq/ledger/pkg/testserver" + "github.com/formancehq/ledger/pkg/testserver/ginkgo" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Context("Idempotency key tests", func() { + var ( + db = UseTemplatedDatabase() + ctx = logging.TestingContext() + natsURL = DeferMap(natsServer, (*natstesting.NatsServer).ClientURL) + ) + + testServer := ginkgo.DeferTestServer( + DeferMap(db, (*pgtesting.Database).ConnectionOptions), + testservice.WithInstruments( + testservice.NatsInstrumentation(natsURL), + testservice.DebugInstrumentation(debug), + testservice.OutputInstrumentation(GinkgoWriter), + ), + testservice.WithLogger(GinkgoT()), + ) + + BeforeEach(func(specContext SpecContext) { + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.CreateLedger(ctx, operations.V2CreateLedgerRequest{ + Ledger: "default", + }) + Expect(err).To(BeNil()) + }) + + When("creating transactions with idempotency key", func() { + var ( + timestamp = time.Now().Round(time.Second).UTC() + tx components.V2Transaction + txRequest operations.V2CreateTransactionRequest + ) + + BeforeEach(func() { + txRequest = operations.V2CreateTransactionRequest{ + V2PostTransaction: components.V2PostTransaction{ + Metadata: map[string]string{}, + Postings: []components.V2Posting{ + { + Amount: big.NewInt(100), + Asset: "USD", + Source: "world", + Destination: "alice", + }, + }, + Timestamp: ×tamp, + }, + Ledger: "default", + IdempotencyKey: pointer.For("tx-idempotency-key-1"), + } + }) + + It("should succeed on the first call and be idempotent on subsequent calls", func(specContext SpecContext) { + // First call + response, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.CreateTransaction(ctx, txRequest) + Expect(err).ToNot(HaveOccurred()) + tx = response.V2CreateTransactionResponse.Data + id := tx.ID + + // Second call with same idempotency key should return the same result + response, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.CreateTransaction(ctx, txRequest) + Expect(err).ToNot(HaveOccurred()) + tx = response.V2CreateTransactionResponse.Data + Expect(tx.ID).To(Equal(id)) + }) + + It("should fail when using the same idempotency key with different data", func(specContext SpecContext) { + // First call + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.CreateTransaction(ctx, txRequest) + Expect(err).ToNot(HaveOccurred()) + + // Second call with same idempotency key but different data + txRequest.V2PostTransaction.Postings[0].Amount = big.NewInt(200) + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.CreateTransaction(ctx, txRequest) + Expect(err).To(HaveOccurred()) + Expect(err).To(HaveErrorCode(string(components.V2ErrorsEnumValidation))) + }) + }) + + When("adding metadata with idempotency key", func() { + var ( + timestamp = time.Now().Round(time.Second).UTC() + tx components.V2Transaction + metadata map[string]string + ) + + BeforeEach(func(specContext SpecContext) { + // Create a transaction first + response, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.CreateTransaction(ctx, operations.V2CreateTransactionRequest{ + V2PostTransaction: components.V2PostTransaction{ + Metadata: map[string]string{}, + Postings: []components.V2Posting{ + { + Amount: big.NewInt(100), + Asset: "USD", + Source: "world", + Destination: "alice", + }, + }, + Timestamp: ×tamp, + }, + Ledger: "default", + }) + Expect(err).ToNot(HaveOccurred()) + tx = response.V2CreateTransactionResponse.Data + + metadata = map[string]string{ + "foo": "bar", + } + }) + + It("should succeed on the first call and be idempotent on subsequent calls", func(specContext SpecContext) { + // First call to add metadata with idempotency key + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataOnTransaction(ctx, operations.V2AddMetadataOnTransactionRequest{ + RequestBody: metadata, + Ledger: "default", + ID: tx.ID, + IdempotencyKey: pointer.For("metadata-idempotency-key-1"), + }) + Expect(err).To(Succeed()) + + // Verify metadata was added + getResponse, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.GetTransaction(ctx, operations.V2GetTransactionRequest{ + Ledger: "default", + ID: tx.ID, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(getResponse.V2GetTransactionResponse.Data.Metadata).Should(Equal(metadata)) + + // Second call with same idempotency key should succeed + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataOnTransaction(ctx, operations.V2AddMetadataOnTransactionRequest{ + RequestBody: metadata, + Ledger: "default", + ID: tx.ID, + IdempotencyKey: pointer.For("metadata-idempotency-key-1"), + }) + Expect(err).To(Succeed()) + }) + + It("should fail when using the same idempotency key with different data", func(specContext SpecContext) { + // First call + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataOnTransaction(ctx, operations.V2AddMetadataOnTransactionRequest{ + RequestBody: metadata, + Ledger: "default", + ID: tx.ID, + IdempotencyKey: pointer.For("metadata-idempotency-key-2"), + }) + Expect(err).To(Succeed()) + + // Second call with same idempotency key but different metadata + differentMetadata := map[string]string{ + "foo": "baz", + } + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataOnTransaction(ctx, operations.V2AddMetadataOnTransactionRequest{ + RequestBody: differentMetadata, + Ledger: "default", + ID: tx.ID, + IdempotencyKey: pointer.For("metadata-idempotency-key-2"), + }) + Expect(err).To(HaveOccurred()) + Expect(err).To(HaveErrorCode(string(components.V2ErrorsEnumValidation))) + }) + }) +}) diff --git a/test/e2e/api_transactions_metadata_test.go b/test/e2e/api_transactions_metadata_test.go index ceee85c313..0954794457 100644 --- a/test/e2e/api_transactions_metadata_test.go +++ b/test/e2e/api_transactions_metadata_test.go @@ -3,17 +3,20 @@ package test_suite import ( + "math/big" + "time" + "github.com/formancehq/go-libs/v3/logging" + "github.com/formancehq/go-libs/v3/pointer" . "github.com/formancehq/go-libs/v3/testing/api" . "github.com/formancehq/go-libs/v3/testing/deferred/ginkgo" + "github.com/formancehq/go-libs/v3/testing/platform/natstesting" "github.com/formancehq/go-libs/v3/testing/platform/pgtesting" "github.com/formancehq/go-libs/v3/testing/testservice" "github.com/formancehq/ledger/pkg/client/models/components" "github.com/formancehq/ledger/pkg/client/models/operations" . "github.com/formancehq/ledger/pkg/testserver" "github.com/formancehq/ledger/pkg/testserver/ginkgo" - "math/big" - "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -21,13 +24,15 @@ import ( var _ = Context("Ledger accounts list API tests", func() { var ( - db = UseTemplatedDatabase() - ctx = logging.TestingContext() + db = UseTemplatedDatabase() + ctx = logging.TestingContext() + natsURL = DeferMap(natsServer, (*natstesting.NatsServer).ClientURL) ) testServer := ginkgo.DeferTestServer( DeferMap(db, (*pgtesting.Database).ConnectionOptions), testservice.WithInstruments( + testservice.NatsInstrumentation(natsURL), testservice.DebugInstrumentation(debug), testservice.OutputInstrumentation(GinkgoWriter), ), @@ -43,12 +48,11 @@ var _ = Context("Ledger accounts list API tests", func() { When("creating a transaction on a ledger", func() { var ( timestamp = time.Now().Round(time.Second).UTC() - rsp *operations.V2CreateTransactionResponse - err error + txID *big.Int ) BeforeEach(func(specContext SpecContext) { // Create a transaction - rsp, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.CreateTransaction( + createResponse, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.CreateTransaction( ctx, operations.V2CreateTransactionRequest{ V2PostTransaction: components.V2PostTransaction{ @@ -67,13 +71,14 @@ var _ = Context("Ledger accounts list API tests", func() { }, ) Expect(err).ToNot(HaveOccurred()) + txID = createResponse.V2CreateTransactionResponse.Data.ID // Check existence on api - _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.GetTransaction( + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.GetTransaction( ctx, operations.V2GetTransactionRequest{ Ledger: "default", - ID: rsp.V2CreateTransactionResponse.Data.ID, + ID: txID, }, ) Expect(err).ToNot(HaveOccurred()) @@ -104,7 +109,7 @@ var _ = Context("Ledger accounts list API tests", func() { operations.V2AddMetadataOnTransactionRequest{ RequestBody: metadata, Ledger: "default", - ID: rsp.V2CreateTransactionResponse.Data.ID, + ID: txID, }, ) Expect(err).To(Succeed()) @@ -114,12 +119,151 @@ var _ = Context("Ledger accounts list API tests", func() { ctx, operations.V2GetTransactionRequest{ Ledger: "default", - ID: rsp.V2CreateTransactionResponse.Data.ID, + ID: txID, }, ) Expect(err).ToNot(HaveOccurred()) Expect(response.V2GetTransactionResponse.Data.Metadata).Should(Equal(metadata)) }) + When("deleting a metadata with idempotency key", func() { + It("should succeed on first call and be idempotent on second call", func(specContext SpecContext) { + ikPtr := pointer.For("delete-key-1") + + // First call to delete metadata with idempotency key + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.DeleteTransactionMetadata( + ctx, + operations.V2DeleteTransactionMetadataRequest{ + Ledger: "default", + ID: txID, + Key: "foo", + IdempotencyKey: ikPtr, + }, + ) + Expect(err).ToNot(HaveOccurred()) + + // Verify metadata was deleted + response, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.GetTransaction( + ctx, + operations.V2GetTransactionRequest{ + Ledger: "default", + ID: txID, + }, + ) + Expect(err).ToNot(HaveOccurred()) + Expect(response.V2GetTransactionResponse.Data.Metadata).Should(BeEmpty()) + + // Second call with same idempotency key should succeed + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.DeleteTransactionMetadata( + ctx, + operations.V2DeleteTransactionMetadataRequest{ + Ledger: "default", + ID: txID, + Key: "foo", + IdempotencyKey: ikPtr, + }, + ) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should fail when using the same idempotency key with different parameters", func(specContext SpecContext) { + // Add two metadata entries + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataOnTransaction( + ctx, + operations.V2AddMetadataOnTransactionRequest{ + RequestBody: map[string]string{ + "foo": "bar", + "baz": "qux", + }, + Ledger: "default", + ID: txID, + }, + ) + Expect(err).To(Succeed()) + + ikPtr := pointer.For("delete-key-2") + + // First call to delete "foo" with idempotency key + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.DeleteTransactionMetadata( + ctx, + operations.V2DeleteTransactionMetadataRequest{ + Ledger: "default", + ID: txID, + Key: "foo", + IdempotencyKey: ikPtr, + }, + ) + Expect(err).ToNot(HaveOccurred()) + + // Second call with same idempotency key but different key to delete + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.DeleteTransactionMetadata( + ctx, + operations.V2DeleteTransactionMetadataRequest{ + Ledger: "default", + ID: txID, + Key: "baz", // Different key + IdempotencyKey: ikPtr, + }, + ) + Expect(err).To(HaveOccurred()) + }) + }) + + When("using the same idempotency key with same data", func() { + It("should succeed and return the same result", func(specContext SpecContext) { + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataOnTransaction( + ctx, + operations.V2AddMetadataOnTransactionRequest{ + RequestBody: metadata, + Ledger: "default", + ID: txID, + IdempotencyKey: pointer.For("test-idempotency-key"), + }, + ) + Expect(err).To(Succeed()) + + // Second call with same idempotency key and same data + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataOnTransaction( + ctx, + operations.V2AddMetadataOnTransactionRequest{ + RequestBody: metadata, + Ledger: "default", + ID: txID, + IdempotencyKey: pointer.For("test-idempotency-key"), + }, + ) + Expect(err).To(Succeed()) + }) + }) + + When("using the same idempotency key with different data", func() { + It("should fail with ValidationError", func(specContext SpecContext) { + _, err := Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataOnTransaction( + ctx, + operations.V2AddMetadataOnTransactionRequest{ + RequestBody: metadata, + Ledger: "default", + ID: txID, + IdempotencyKey: pointer.For("test-idempotency-key-2"), + }, + ) + Expect(err).To(Succeed()) + + // Second call with same idempotency key but different data + _, err = Wait(specContext, DeferClient(testServer)).Ledger.V2.AddMetadataOnTransaction( + ctx, + operations.V2AddMetadataOnTransactionRequest{ + RequestBody: map[string]string{ + "foo": "different-value", + }, + Ledger: "default", + ID: txID, + IdempotencyKey: pointer.For("test-idempotency-key-2"), + }, + ) + Expect(err).To(HaveOccurred()) + Expect(err).To(HaveErrorCode(string(components.V2ErrorsEnumValidation))) + }) + }) }) }) })