diff --git a/docs/adr/0004-signalr-datasource-server-side-filtering.md b/docs/adr/0004-signalr-datasource-server-side-filtering.md index defbed4..6a08d5e 100644 --- a/docs/adr/0004-signalr-datasource-server-side-filtering.md +++ b/docs/adr/0004-signalr-datasource-server-side-filtering.md @@ -1,4 +1,4 @@ -# ADR 0004: Server-side filtering for SignalR DataSource via DevExpress CriteriaOperator string + `[SignalRFilterText]` hub injection +# ADR 0004: Server-side filtering for SignalR DataSource via DevExpress CriteriaOperator string + `[FilterCriteriaParam]` hub injection ## Status @@ -28,12 +28,13 @@ Adopt a **criteria-string** server-side filter for the SignalR DataSource. The D | Concern | Decision | Layer | |---|---|---| | Filter representation | DevExpress `CriteriaOperator` **string** (`ToString()` client → `Parse()` server), NOT a serialized expression tree | — | -| Wire carriage | Rides in the always-sent `SignalParams` metadata (`SignalParams.FilterText`), NOT a positional method param | AyCode.Services (`ISignalParams`) | -| Server binding | Hub (`AcWebSignalRHubBase`) injects `SignalParams.FilterText` into a param marked `[SignalRFilterText]`, resolved **once** from the cached `AcMethodInfoModel.ParamInfos` (zero per-call reflection) | AyCode.Services.Server | -| Mismatch diagnostic | If a `FilterMode.Server` grid sends `FilterText` but the method has **no** `[SignalRFilterText]` param → **warning-log** the mismatch: a real misconfiguration (grid declared `Server`, endpoint can't filter). **Precise** — never fires for `FilterMode.Local` grids. Log level / per-tag suppression is an implementation decision | AyCode.Services.Server | +| Wire carriage | Rides in the always-sent `SignalParams` metadata (`SignalParams.FilterCriteria`), NOT a positional method param | AyCode.Services (`ISignalParams`) | +| Server binding | Hub (`AcWebSignalRHubBase`) injects `SignalParams.FilterCriteria` into a param marked `[FilterCriteriaParam]`, resolved **once** from the cached `AcMethodInfoModel.ParamInfos` (zero per-call reflection). **Convention (mandatory):** the `[FilterCriteriaParam]` parameter is the **LAST** parameter and optional (`string? filterCriteria = null`) — old clients bind positionally and the trailing param defaults to null via the existing trailing-defaults transport mechanic (no skip logic needed). The hub **validates the convention at registration time** (not-last / not-optional → loud startup diagnostic, zero per-call cost); the attribute's XML `` MUST state the mandatory last-position requirement | AyCode.Services.Server | +| Mismatch diagnostic | If a `FilterMode.Server` grid sends `FilterCriteria` but the method has **no** `[FilterCriteriaParam]` param → **warning-log** the mismatch: a real misconfiguration (grid declared `Server`, endpoint can't filter). **Precise** — never fires for `FilterMode.Local` grids. Log level / per-tag suppression is an implementation decision | AyCode.Services.Server | +| Legacy `FilterText` coexistence | `SignalParams.FilterCriteria` (serialized criteria string) is a **NEW channel, separate from the legacy free-text `FilterText`** (appended by `AcSignalRDataSource.GetContextParams()` as the last positional param; per-endpoint manual handling, e.g. `GetAllLogItemsByFilterText`-style endpoints). The legacy mechanism stays untouched — two different features, two different names | AyCode.Services | | Application | Shared **filter-service**: parse → **per-entity field whitelist** → apply to the endpoint's `IQueryable` **before** `.ToListAsync()` (SQL WHERE). Requires `DevExpress.Data` (non-UI) on the server | AyCode.Services.Server | -| Client forwarding (`FilterMode`) | MgGrid carries a `FilterMode` enum (**Local** / **Server**). `Server`: the grid forwards its `CriteriaOperator` into `SignalParams.FilterText` and a filter change triggers a server reload. `Local`: the grid filters the loaded set client-side — no forward, no round-trip (today's behavior). `FilterMode` is a **grid-level behavioral policy (intent)**, NOT a client-side claim about server capability — this is the distinction from the rejected `HasFilterTextParam`, so it does not reintroduce that drift. The server's cached reflection stays the single authority on *capability*; `FilterMode` decides *participation*. Default = user decision (lean `Local`) | AyCode.Blazor | -| Rollout | Gradual, per-endpoint opt-in: add `[SignalRFilterText] string? filterText` + one filter-service call. No endpoint-signature-position fragility, no per-endpoint duplication of filter logic | consumer plugin | +| Client forwarding (`FilterMode`) | MgGrid carries a `FilterMode` enum (**Local** / **Server**). `Server`: the grid forwards its `CriteriaOperator` into `SignalParams.FilterCriteria` and a filter change triggers a server reload. `Local`: the grid filters the loaded set client-side — no forward, no round-trip (today's behavior). `FilterMode` is a **grid-level behavioral policy (intent)**, NOT a client-side claim about server capability — this is the distinction from the rejected `HasFilterTextParam`, so it does not reintroduce that drift. The server's cached reflection stays the single authority on *capability*; `FilterMode` decides *participation*. Default = user decision (lean `Local`) | AyCode.Blazor | +| Rollout | Gradual, per-endpoint opt-in: add `[FilterCriteriaParam] string? filterText` + one filter-service call. No endpoint-signature-position fragility, no per-endpoint duplication of filter logic | consumer plugin | | Scope | **v1 = filtering** (load-all model, change-tracking untouched). **v2 = paging** (`skip`/`take`/`count` + `{items,total}` envelope) extends the same `SignalParams` channel later | — | ## Consequences @@ -43,22 +44,23 @@ Adopt a **criteria-string** server-side filter for the SignalR DataSource. The D - Wire format is a plain string → vendor-neutral on the wire, fits the existing `FilterText` channel; the framework stays DevExpress-free on the wire. - **Universal + gradual**: the injection logic lives **once** in the hub; endpoints opt in with an attribute + one service call. No positional-param fragility (the `ContextIds` object[] is variable-length). - **Single source of truth**: the server's cached reflection is authoritative on filter support; no drift-prone client-side flag duplicating it. -- **Loud, not silent, precise**: a `FilterMode.Server` grid whose endpoint lacks `[SignalRFilterText]` triggers a server warning (a real misconfiguration signal), never a silent no-filter drop — and `FilterMode.Local` grids never false-trigger it. +- **Loud, not silent, precise**: a `FilterMode.Server` grid whose endpoint lacks `[FilterCriteriaParam]` triggers a server warning (a real misconfiguration signal), never a silent no-filter drop — and `FilterMode.Local` grids never false-trigger it. - Efficient: filter applied to `IQueryable` before materialization → SQL `WHERE`. - Unblocks large grids and (with v2) all-data access, retiring the 15-day hack. - Change-tracking untouched in v1 (still load-all, just a smaller server-filtered set). **Negative:** - Adds a `DevExpress.Data` dependency to the server plugin (currently DevExpress-free server-side; it has **DevExtreme**, a different product line). `DevExpress.Data` is UI-agnostic and runs in the Razor/MVC plugin — not a Blazor dependency. -- **Field-whitelist is mandatory — DevExpress does NOT provide it.** The criteria DSL bounds the *shape* to a WHERE predicate (no arbitrary code — see Positive), but `CriteriaOperator` places **no restriction on which columns / relations / functions** a criteria may reference. Within the WHERE surface a tampered filter string can still: (a) filter on a non-displayed / sensitive column as a **blind-injection oracle** (`[PasswordHash] LIKE 'a%'` → rows-or-not leaks the value character by character); (b) navigate **relations** (`[Customer.CreditCard]`) into other tables; (c) use expensive functions / aggregate subqueries (**DoS**). The per-entity whitelist (allowed filterable columns + relation depth + function set; un-translatable features rejected, not client-eval'd) is the boundary DevExpress leaves to us. -- Opt-in discipline (narrowed): a **missing** `[SignalRFilterText]` attribute is now caught by the mismatch warning; but an attribute **present with the injected value unused** (developer forgot the filter-service call) is still a silent no-filter. +- **Field-whitelist is mandatory — DevExpress does NOT provide it.** The criteria DSL bounds the *shape* to a WHERE predicate (no arbitrary code — see Positive), but `CriteriaOperator` places **no restriction on which columns / relations / functions** a criteria may reference. Within the WHERE surface a tampered filter string can still: (a) filter on a non-displayed / sensitive column as a **blind-injection oracle** (`[PasswordHash] LIKE 'a%'` → rows-or-not leaks the value character by character); (b) navigate **relations** (`[Customer.CreditCard]`) into other tables; (c) use expensive functions / aggregate subqueries (**DoS**). The filter-service therefore enforces a **zero-config default whitelist**: only the entity's OWN mapped scalar columns are filterable; relation-path traversal and aggregate operators are DENIED by default; un-translatable criteria features are rejected, not client-eval'd. **Parse failure or over-length criteria (length cap) → hard reject** — never "no filter → return everything". Per-property opt-out via a **`[NotGridFilterable]`** attribute (defined in **AyCode.Core** so every entity layer can reference it) — name settled, **implementation DEFERRED**: do NOT build it in the first round; file a TODO entry when the filter-service implementation reaches it. +- **v1 double-filter semantics (collation) — documented risk, decision deferred.** Under `FilterMode.Server` with direct `Data` binding (v1), the server filters via SQL (typically case-insensitive collation) AND DevExpress re-applies the same criteria locally (LINQ-to-objects, case-sensitive) — rows returned by the server can be hidden locally (accented / case-differing text). Mitigation path: the filter-service **Debug-logs the incoming criteria string**; a criteria-normalization method (e.g. forcing case-insensitive comparisons) is the candidate fix; decide after the first working end-to-end trial. +- Opt-in discipline (narrowed): a **missing** `[FilterCriteriaParam]` attribute is now caught by the mismatch warning; but an attribute **present with the injected value unused** (developer forgot the filter-service call) is still a silent no-filter. - **One grid-level policy knob (`FilterMode`)**: minimal ceremony (an enum with a sensible default), but a genuine config surface. It is grid *intent*, not a server-capability claim, so it does not reintroduce the `HasFilterTextParam` drift; and it removes the false-positive warning for intentionally local grids. - SQL-translation caveats: some criteria functions may not translate via linq2db → **function whitelist**; complexity / timeout caps needed (leading-wildcard `Contains` on a large unindexed table = full scan / DoS); parse failure must **hard-reject** (never "no filter → return everything"). - `MgGridSignalRDataSource` (currently unwired) needs finishing, and `MgGridBase` diverting off direct `DxGrid.Data` binding onto it — but only for the eventual ServerPaged case; **v1 filtering can ride the existing load path** (server returns a smaller filtered set into the same collection). **Follow-ups required:** -- File SIGDS + GRID TODO entries: `[SignalRFilterText]` attribute + hub injection + **mismatch warning-log** (AyCode.Services.Server); `SignalParams.FilterText` field (AyCode.Services); filter-service + per-entity whitelist (AyCode.Services.Server); MgGrid `FilterMode` (Local / Server) + criteria serialization into `SignalParams.FilterText` on `Server` (AyCode.Blazor); add `DevExpress.Data` to the server plugin. -- **Enabled extension** (own follow-up, not core to this ADR): MgGrid declarative **quick-filter panel** — a `FilterPanel` slot on `MgGridBase` hosting `MgTagBox` (`: DxTagBox` seam) controls, each with a `FieldName` (consistent with grid columns / DevExpress `FieldName`). On selection each control calls `DxGrid.SetFieldFilterCriteria(FieldName, criteria)`; DevExpress **AND-combines the per-field criteria automatically** (+ the user's filter row) — no manual aggregation, no clobber (**NOT** `SetFilterCriteria`, which replaces the whole filter). Planned types: `MgGridFilterPanelBase` (framework, holds the grid-cascade + wiring) → `GridFilterPanel` (project seam, typically empty), plus the `MgTagBox` seam. The grid's `FilterCriteria` is the single source: locally it filters the bound `Data`; under `FilterMode.Server` the same `FilterCriteria` reaches `GridCustomDataSource.GetItemsAsync(options.FilterCriteria)` → the criteria-string path — so the component is built **once** for both. Needs a server distinct-values source under `FilterMode.Server` (see v2 paging). +- File SIGDS + GRID TODO entries: `[FilterCriteriaParam]` attribute (lives beside the other SignalR attributes in AyCode.Services; XML `` MUST state the mandatory last-position + optional convention) + hub injection + registration-time convention validation + **mismatch warning-log** (AyCode.Services.Server); `SignalParams.FilterCriteria` field (AyCode.Services); filter-service — zero-config whitelist default + parse/length hard-reject + criteria Debug-log (AyCode.Services.Server); MgGrid `FilterMode` (Local / Server) + criteria serialization into `SignalParams.FilterCriteria` on `Server` (AyCode.Blazor); add `DevExpress.Data` to the server plugin. `[NotGridFilterable]` (AyCode.Core) is **deferred** — TODO-only when the filter-service lands. +- **Enabled extension — IMPLEMENTED (2026-07-16), local mode**: MgGrid declarative **quick-filter panel**. Shipped shape (simplified from the earlier two-wrapper draft): a `FilterPanel` RenderFragment slot on `MgGridBase` that **itself wraps** its content in the styled `.mg-grid-filter-panel` container — no separate panel component (the planned `MgGridFilterPanelBase` → `GridFilterPanel` pair was built, then removed as redundant nesting). Consumers place `MgTagBox` (`: DxTagBox` seam) controls directly in the slot, each with a `FieldName` (consistent with grid columns / DevExpress `FieldName`). On selection each control calls `DxGrid.SetFieldFilterCriteria(FieldName, criteria)` via `IMgGridBase.SetFieldQuickFilter`; DevExpress **AND-combines the per-field criteria automatically** (+ the user's filter row) — no manual aggregation, no clobber (**NOT** `SetFilterCriteria`, which replaces the whole filter). A toolbar **Show/Hide Filters** toggle (`IMgGridBase.HasFilterPanel` / `IsFilterPanelVisible` / `ToggleFilterPanel`) appears only when the grid has a filter panel. Implementation notes: imperative `Values` writes inside DevExpress editors must be wrapped in `BeginUpdate()`/`EndUpdate()` (parameter-tracker throws otherwise); first consumer: Partner grid (Currency tag box). The grid's `FilterCriteria` is the single source: locally it filters the bound `Data`; under `FilterMode.Server` the same `FilterCriteria` reaches `GridCustomDataSource.GetItemsAsync(options.FilterCriteria)` → the criteria-string path — so the component is built **once** for both. Needs a server distinct-values source under `FilterMode.Server` (see v2 paging). - Verify: `DevExpress.Data` version matching the client DevExpress Blazor major; license coverage on the server package feed; linq2db translation of `CriteriaToExpressionConverter` output (function whitelist). - **Quick-search (`SearchText`) — confirmed SEPARATE from `FilterCriteria`.** Per DevExpress docs (Filter API): *"Search settings do not affect the applied filter criteria"* — `SearchText` filters in parallel, NOT folded into `FilterCriteria`. So server-side search does **not** ride the criteria-string path for free; it needs its **own** handling: a thin `SearchText → OR-Contains criteria` adapter over searchable columns (then it rides the same channel), or a separate search field. Same whitelist / DoS caveats apply (search = OR-Contains across many columns). - AyCode.Blazor cross-ref: no `docs/adr/` folder there yet → note in MGGRID docs or a future AyCode.Blazor ADR. @@ -73,7 +75,7 @@ Adopt a **criteria-string** server-side filter for the SignalR DataSource. The D - *Cost:* using it for the Blazor grids means translating the DevExpress-Blazor `CriteriaOperator` model into DevExtreme `loadOptions` — an impedance mismatch. Reasonable for the nop admin Razor grids; wrong for the Blazor app grids. - **Positional `string? filterText` parameter per endpoint** (rejected): append `filterText` as a trailing positional param. - *Cons:* with a variable-length `ContextIds` object[], "which position" is fragile; and it is per-endpoint signature editing. The `SignalParams` + attribute-injection removes both problems. -- **Naming convention (`filterText` param name) instead of an attribute** (rejected in favor of the attribute): bind by a consistently-named param + developer discipline. Rejected — silent failure on typo / rename; `[SignalRFilterText]` is self-documenting, rename-safe, and its param index precomputes into the cached method model. +- **Naming convention (`filterText` param name) instead of an attribute** (rejected in favor of the attribute): bind by a consistently-named param + developer discipline. Rejected — silent failure on typo / rename; `[FilterCriteriaParam]` is self-documenting, rename-safe, and its param index precomputes into the cached method model. - **Client-side opt-in flag (`HasFilterTextParam` on MgGrid)** (rejected): a per-grid bool **claiming** whether the endpoint has the filter param. Rejected — duplicates the server's authoritative cached reflection (drift-prone), and a forgotten flag = silent no-filter. Superseded by the `FilterMode` (Local / Server) **policy** enum + server mismatch warning: `FilterMode` declares the grid's own intent (not a server claim), the server's reflection stays the single authority on capability, and a `Server`-declared grid hitting an unsupported endpoint surfaces as a precise warning. The distinction is intent-vs-capability-claim, not "flag vs no-flag". - **Framework choke-point auto-applies to `IQueryable` results** (deferred, not chosen for v1): the hub inspects the method result and, if `IQueryable`, applies filter + page + count generically → zero endpoint changes. - *Cost:* requires endpoints to return `IQueryable` (deferred) instead of `List` — a bigger contract change with an `IQueryable`-lifetime concern (connection alive until the framework materializes). diff --git a/docs/adr/README.md b/docs/adr/README.md index 6094823..7806112 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -19,7 +19,7 @@ See [`.github/skills/adr-author/SKILL.md`](../../.github/skills/adr-author/SKILL | [0001](0001-user-bearer-token-flow.md) | User bearer-token authentication flow (HTTP + SignalR + tag dispatch) | Accepted (2026-04-25) | | [0002](0002-environment-infrastructure-to-framework.md) | Environment infrastructure lifted to the framework (BaseUrl, IsProdDb, env badge) — cross-ref to FruitBankHybridApp ADR 0002 | Accepted (2026-07-15) | | [0003](0003-acbinary-streaming-receive-architecture.md) | AcBinary streaming receive — AsyncPipeReaderInput unified primitive and transport-agnostic helpers | Accepted (2026-05-03) | -| [0004](0004-signalr-datasource-server-side-filtering.md) | Server-side filtering for SignalR DataSource via DevExpress CriteriaOperator string + `[SignalRFilterText]` hub injection | Proposed (2026-07-15) | +| [0004](0004-signalr-datasource-server-side-filtering.md) | Server-side filtering for SignalR DataSource via DevExpress CriteriaOperator string + `[FilterCriteriaParam]` hub injection | Proposed (2026-07-15) | ## Related