diff --git a/docs/technical/architecture/subsystems/read-path/query-pipeline.md b/docs/technical/architecture/subsystems/read-path/query-pipeline.md index 6ed6e95387..8521b4a7b1 100644 --- a/docs/technical/architecture/subsystems/read-path/query-pipeline.md +++ b/docs/technical/architecture/subsystems/read-path/query-pipeline.md @@ -67,7 +67,10 @@ query parameter and AND-combine the result with the transactions endpoint's remaining convenience param — the `startDate`/`endDate` range — through `combineFilters` (the `reference` and `prefix` aliases were removed in EN-1540: account prefix is now `address ^= "..."`, transaction reference is structured -`{"$match":{"reference":"..."}}`); prepared-query +`{"$match":{"reference":"..."}}`); the aggregate-volumes endpoint +(`GET /{ledger}/volumes`) also parses `filter` through `parseListFilter` for the +Accounts target as its sole account selector, its `prefix` parameter removed the +same way (EN-1541); prepared-query create/update decode the JSON `filter` body field through the same helper; and `ledgerctl --filter` routes through it in `cmdutil.BuildQueryFilter`. Audit fields are written **bare** (`outcome`, `ledger`, `seq`, `proposal_id`, diff --git a/docs/technical/contributing/api-comparison.md b/docs/technical/contributing/api-comparison.md index ae1d85a4ba..1a884ca9f6 100644 --- a/docs/technical/contributing/api-comparison.md +++ b/docs/technical/contributing/api-comparison.md @@ -753,7 +753,7 @@ Read endpoints comparison with the original ledger: | `GET /v3/{ledgerName}/accounts/{address}` | ✅ | ✅ | Get an account | | `GET /v3/{ledgerName}/accounts/{address}/balances` | ❌ | ✅ | Get account balances | | `GET /v3/{ledgerName}/accounts/{address}/volumes` | ❌ | ✅ | Get account volumes | -| `GET /v3/{ledgerName}/volumes` | ✅ | ✅ | Aggregate volumes (per-asset, supports prefix filtering) | +| `GET /v3/{ledgerName}/volumes` | ✅ | ✅ | Aggregate volumes (per-asset, generic account `filter`) | | `GET /v3/{ledgerName}/logs` | ✅ | ✅ | List per-ledger logs. Supports `?after=` for pagination | | `GET /v3/{ledgerName}/stats` | ✅ | ✅ | Ledger usage statistics (transaction, volume, reference, posting, log, revert, Numscript-execution, ephemeral-evicted and transient-used counts) | | `GET /v3/{ledgerName}` | ✅ | ✅ | Get ledger info | diff --git a/internal/adapter/http/handlers_aggregate_volumes.go b/internal/adapter/http/handlers_aggregate_volumes.go index f4c4f7b613..55084c21ed 100644 --- a/internal/adapter/http/handlers_aggregate_volumes.go +++ b/internal/adapter/http/handlers_aggregate_volumes.go @@ -86,16 +86,13 @@ func (s *Server) handleAggregateVolumes(w http.ResponseWriter, r *http.Request) groupByPrefixes = strings.Split(g, ",") } - // Build optional address-prefix filter from query parameter - var filter *commonpb.QueryFilter - if prefix := r.URL.Query().Get("prefix"); prefix != "" { - filter = &commonpb.QueryFilter{ - Filter: &commonpb.QueryFilter_Address{ - Address: &commonpb.AddressMatch{ - Match: &commonpb.AddressMatch_HardcodedPrefix{HardcodedPrefix: prefix}, - }, - }, - } + // The `filter` query parameter accepts either the textual filterexpr grammar + // or the structured v2 JSON DSL (EN-1511) and is the sole account selector. + // It is compiled for the Accounts target and forwarded unchanged; aggregation + // options (precision, grouping, color collapsing) stay independent of it. + filter, ok := parseListFilter(w, r, commonpb.QueryTarget_QUERY_TARGET_ACCOUNTS) + if !ok { + return } ctx, profile := query.WithProfile(r.Context()) diff --git a/internal/adapter/http/handlers_aggregate_volumes_test.go b/internal/adapter/http/handlers_aggregate_volumes_test.go index 06a73947c0..1a0ea1a0de 100644 --- a/internal/adapter/http/handlers_aggregate_volumes_test.go +++ b/internal/adapter/http/handlers_aggregate_volumes_test.go @@ -5,10 +5,12 @@ import ( "errors" "net/http" "net/http/httptest" + "net/url" "testing" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + "google.golang.org/protobuf/proto" logging "github.com/formancehq/go-libs/v5/pkg/observe/log" @@ -78,14 +80,17 @@ func TestHandleAggregateVolumes_WithOptions(t *testing.T) { srv := newTestServer(t, backend) w := httptest.NewRecorder() - r := newRequest(t, http.MethodGet, "/my-ledger/volumes?useMaxPrecision=true&groupByPrefixes=users:,merchants:&prefix=users:", nil, map[string]string{ - "ledgerName": "my-ledger", - }) + r := newRequest(t, http.MethodGet, + "/my-ledger/volumes?useMaxPrecision=true&collapseColors=true&groupByPrefixes=users:,merchants:&filter="+ + url.QueryEscape(`address ^= "users:"`), nil, map[string]string{ + "ledgerName": "my-ledger", + }) srv.handleAggregateVolumes(w, r) require.Equal(t, http.StatusOK, w.Code) require.True(t, capturedOpts.UseMaxPrecision) + require.True(t, capturedOpts.CollapseColors) require.Equal(t, []string{"users:", "merchants:"}, capturedOpts.GroupByPrefixes) require.NotNil(t, capturedFilter) require.Equal(t, "users:", capturedFilter.GetAddress().GetHardcodedPrefix()) @@ -272,3 +277,114 @@ func TestHandleAggregateVolumes_EmitsColorAlways(t *testing.T) { require.Contains(t, w.Body.String(), `"color":""`, `empty color must surface as "color":"" not be omitted by omitempty`) } + +// TestHandleAggregateVolumes_DualFormatFilter is the endpoint-level EN-1511 +// acceptance check for volumes: the same logical account selector passed via +// `?filter=` in the textual form and in the structured JSON form reaches the +// backend as the same QueryFilter. +func TestHandleAggregateVolumes_DualFormatFilter(t *testing.T) { + t.Parallel() + + capture := func(t *testing.T, target string) *commonpb.QueryFilter { + t.Helper() + + var captured *commonpb.QueryFilter + + backend := NewMockBackend(gomock.NewController(t)) + backend.EXPECT().AggregateVolumes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, _ string, filter *commonpb.QueryFilter, _ query.AggregateOptions) (*commonpb.AggregateResult, error) { + captured = filter + + return &commonpb.AggregateResult{}, nil + }).AnyTimes() + srv := newTestServer(t, backend) + + w := httptest.NewRecorder() + r := newRequest(t, http.MethodGet, target, nil, map[string]string{"ledgerName": "my-ledger"}) + srv.handleAggregateVolumes(w, r) + + require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String()) + require.NotNil(t, captured) + + return captured + } + + fromText := capture(t, "/my-ledger/volumes?filter="+url.QueryEscape(`metadata[status] == "active"`)) + fromJSON := capture(t, "/my-ledger/volumes?filter="+url.QueryEscape(`{"$match":{"metadata[status]":"active"}}`)) + + require.True(t, proto.Equal(fromText, fromJSON), + "textual and JSON ?filter= forms must reach the backend as the same QueryFilter\n text: %v\n json: %v", + fromText, fromJSON) +} + +// TestHandleAggregateVolumes_FilterReachesBackend proves the canonical `filter` +// is the sole account selector: address and metadata conditions both reach the +// controller, and the removed `prefix=` parameter is no longer interpreted (its +// canonical replacement is the textual `address ^= ""`). +func TestHandleAggregateVolumes_FilterReachesBackend(t *testing.T) { + t.Parallel() + + capture := func(t *testing.T, target string) *commonpb.QueryFilter { + t.Helper() + + var captured *commonpb.QueryFilter + + backend := NewMockBackend(gomock.NewController(t)) + backend.EXPECT().AggregateVolumes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, _ string, filter *commonpb.QueryFilter, _ query.AggregateOptions) (*commonpb.AggregateResult, error) { + captured = filter + + return &commonpb.AggregateResult{}, nil + }).AnyTimes() + srv := newTestServer(t, backend) + + w := httptest.NewRecorder() + r := newRequest(t, http.MethodGet, target, nil, map[string]string{"ledgerName": "my-ledger"}) + srv.handleAggregateVolumes(w, r) + + require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String()) + + return captured + } + + // Address-prefix selection reaches the backend as a HardcodedPrefix (the sole + // filter is not wrapped in a redundant 1-element $and). + fromAddress := capture(t, "/my-ledger/volumes?filter="+url.QueryEscape(`address ^= "users:"`)) + require.NotNil(t, fromAddress) + require.Equal(t, "users:", fromAddress.GetAddress().GetHardcodedPrefix()) + + // Metadata selection reaches the backend as a field condition. + fromMetadata := capture(t, "/my-ledger/volumes?filter="+url.QueryEscape(`metadata[status] == "active"`)) + require.NotNil(t, fromMetadata) + require.NotNil(t, fromMetadata.GetField(), "metadata filter must reach the backend as a field condition") + + // The removed `prefix=` parameter must no longer be interpreted: passed alone + // (no `filter=`), it yields an unfiltered read (nil filter). + aliasOnly := capture(t, "/my-ledger/volumes?prefix=users:") + require.Nil(t, aliasOnly, "the removed prefix= parameter must not build a filter") +} + +// TestHandleAggregateVolumes_FilterInvalidForTarget checks that a malformed +// filter and a condition invalid on the Accounts target are both rejected with +// a 400 for both dual-format forms, without invoking the backend (the mock has +// no expectation, so any call fails the test). +func TestHandleAggregateVolumes_FilterInvalidForTarget(t *testing.T) { + t.Parallel() + + // `ledger == ...` is a logs-only condition, invalid for the Accounts target; + // `metadata[status ==` is syntactically malformed. + for _, raw := range []string{ + `ledger == "main"`, + `{"$match":{"ledger":"main"}}`, + `metadata[status ==`, + } { + srv := newTestServer(t, NewMockBackend(gomock.NewController(t))) + + w := httptest.NewRecorder() + r := newRequest(t, http.MethodGet, "/my-ledger/volumes?filter="+url.QueryEscape(raw), nil, + map[string]string{"ledgerName": "my-ledger"}) + srv.handleAggregateVolumes(w, r) + + require.Equal(t, http.StatusBadRequest, w.Code, "raw: %s", raw) + } +} diff --git a/openapi.yml b/openapi.yml index 1fab806b50..ca7b962b4a 100644 --- a/openapi.yml +++ b/openapi.yml @@ -889,7 +889,8 @@ paths: summary: Aggregate volumes description: | Returns aggregated volumes (input/output/balance per asset) across all accounts in the ledger. - Supports filtering by account address prefix, grouping by account prefixes, and max-precision merging. + Supports filtering accounts with the generic `filter` parameter, grouping by account prefixes, + and max-precision merging. Requires `ledger:AccountRead` scope. operationId: aggregateVolumes tags: @@ -899,15 +900,7 @@ paths: - {} parameters: - $ref: '#/components/parameters/LedgerName' - - name: prefix - in: query - description: | - Filter accounts whose address starts with this raw byte-level - prefix (e.g. `users:` matches `users:alice`, `users:bob`, ...). - Matching is not path-segment aware: `acc:1` also matches `acc:10`, - `acc:11`, etc. Do not use for exact-address lookups. - schema: - type: string + - $ref: '#/components/parameters/ListFilter' - name: useMaxPrecision in: query description: Merge assets with the same base at the highest observed precision