Skip to content

Commit ab3b961

Browse files
authored
ObjectListFilter filter values to target type coercion (#589)
* Refactored `ObjectListFilter`: modularized, added type coercion - Moved filter operators and LINQ logic to ObjectListFilterModule.fs - Added `TypeCoercion.fs` for automatic filter value coercion (`Guid`, `DateTime`, F# DUs, etc.) - Introduced `FilterValueCoercer` and extended `ObjectListFilterLinqOptions` for custom coercion - Centralized filter suffix constants in `FilterSuffixConstants.fs` - Updated `SchemaDefinitions.fs` to use new suffix constants - Added `vtryFind` and `vtryPick` utilities for arrays/lists in `Extensions.fs` - Improved code style, documentation, and function signatures * Refactor 'ObjectListFilter' to use 'System.Text.Json' coercion Replaces custom value coercers with 'System.Text.Json'-based coercion in 'ObjectListFilter', supporting advanced scenarios like F# DUs and CLR enums via 'JsonSerializerOptions'. Updates 'ObjectListFilterLinqOptions' to accept 'JsonSerializerOptions'. Refactors 'TypeCoercion' module to use JSON serialization/deserialization for all type conversions. Updates filter application logic and expands the test suite with new files to cover a wide range of coercion scenarios. Updates documentation and usage examples accordingly. * Updateв schema, add type coercion guide, bug report, tools * Added bug report for InputObject array type mismatch with analysis and test cases * Added type coercion guide for ObjectListFilter with usage and API docs * Introduced format-changed-files.ps1 to batch-format changed F# files via Fantomas * Updated schema snapshots for relay-style connections and new scalars * Refactored field_aliases.fsx for relay-style friends connection * Optimized TypeCoercion.fs to use Utf8JsonWriter for value coercion * Added prompt template for automated PR/issue description generation * Rebase fix * Update filters to use `CurrentCulture` string comparison Updated all string comparison operations in `ObjectListFilter` and filter parsing logic to use `StringComparer.CurrentCulture` or `StringComparer.CurrentCultureIgnoreCase` instead of `Ordinal`/`OrdinalIgnoreCase`. Adjusted related test expectations to match. This ensures string-based filters now respect the current culture's case rules. * Rebase fixes * Removed unnecessary `ObjectListFilterValidationException` * Added test traits * AI review fixes * Fix ObjectListFilter IN coercion behavior and add converter/no-converter tests
1 parent c1e8510 commit ab3b961

15 files changed

Lines changed: 1369 additions & 376 deletions

src/FSharp.Data.GraphQL.Server.Middleware/FSharp.Data.GraphQL.Server.Middleware.fsproj

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
</ItemGroup>
2121

2222
<ItemGroup>
23-
<Compile Include="ObjectListFilter.fs" />
2423
<Compile Include="FilterSuffixConstants.fs" />
24+
<Compile Include="ObjectListFilter.fs" />
25+
<Compile Include="TypeCoercion.fs" />
26+
<Compile Include="ObjectListFilterModule.fs" />
2527
<Compile Include="TypeSystemExtensions.fs" />
2628
<Compile Include="SchemaDefinitions.fs" />
2729
<Compile Include="MiddlewareDefinitions.fs" />

src/FSharp.Data.GraphQL.Server.Middleware/ObjectListFilter.fs

Lines changed: 20 additions & 365 deletions
Large diffs are not rendered by default.

src/FSharp.Data.GraphQL.Server.Middleware/ObjectListFilterModule.fs

Lines changed: 407 additions & 0 deletions
Large diffs are not rendered by default.

src/FSharp.Data.GraphQL.Server.Middleware/SchemaDefinitions.fs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ type private ComparisonOperator =
2020
| LessThanOrEqual of string
2121
| In of string
2222

23-
2423
let rec private coerceObjectListFilterInput (variables : Variables) inputValue : Result<ObjectListFilter voption, IGQLError list> =
2524

2625
let parseFieldCondition (s : string) =
@@ -184,7 +183,7 @@ let ObjectListFilterType : InputCustomDefinition<ObjectListFilter> = {
184183
(String.concat
185184
" "
186185
[
187-
"The ObjectListFilter value represents field filters for object lists."
186+
"The `ObjectListFilter` value represents field filters for object lists."
188187
"Lowercase string suffixes such as `_starts_with`/`_sw`, `_ends_with`/`_ew`, `_contains` (no shorthand), and `_equals`/`_eq` are case-insensitive when applied to string fields."
189188
"Capitalized string suffixes such as `_Starts_With`/`_SW`, `_Ends_With`/`_EW`, `_Contains` (no shorthand), and `_Equals`/`_EQ` are case-sensitive when applied to string fields."
190189
"Comparison suffixes such as `_gt`, `_gte`, `_lt`, `_lte`, and `_in` are also supported."
Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
namespace FSharp.Data.GraphQL.Server.Middleware
2+
3+
open System
4+
open System.Buffers
5+
open System.Collections.Generic
6+
open System.Reflection
7+
open System.Text.Json
8+
open FSharp.Data.GraphQL
9+
open FSharp.Data.GraphQL.Extensions
10+
11+
[<RequireQualifiedAccess>]
12+
module TypeCoercion =
13+
14+
/// Case-insensitive instance property lookup. The middleware lowercases field names during
15+
/// parsing, so we must also ignore casing here.
16+
let propertyBindFlags =
17+
BindingFlags.Public
18+
||| BindingFlags.Instance
19+
||| BindingFlags.IgnoreCase
20+
21+
// Cached type references
22+
let private stringType = typeof<string>
23+
24+
/// <summary>
25+
/// If <paramref name="t"/> is <c>voption</c>, <c>option</c>, or <c>Skippable</c>, returns the inner type; otherwise <see langword="ValueNone"/>.
26+
/// </summary>
27+
let tryUnwrapOption (t : Type) : Type voption =
28+
if t.IsGenericType then
29+
let fullName = t.GetGenericTypeDefinition().FullName
30+
if
31+
fullName.StartsWith ReflectionHelper.ValueOptionTypeName
32+
|| fullName.StartsWith ReflectionHelper.OptionTypeName
33+
|| fullName.StartsWith ReflectionHelper.SkippableTypeName
34+
then
35+
ValueSome (t.GetGenericArguments().[0])
36+
else
37+
ValueNone
38+
else
39+
ValueNone
40+
41+
let unwrapOption (t : Type) : Type =
42+
tryUnwrapOption t |> ValueOption.defaultValue t
43+
44+
/// <summary>
45+
/// If <paramref name="t"/> is a generic collection, returns the element type. Handles both concrete collections (where <c>IEnumerable</c> is an
46+
/// implemented interface) and properties typed directly as <c>IEnumerable&lt;T&gt;</c>.
47+
/// </summary>
48+
let tryUnwrapEnumerableElement (t : Type) : Type voption =
49+
let isEnumerableInterface (i : Type) =
50+
i.IsGenericType
51+
&& Type.(=) (i.GetGenericTypeDefinition (), typedefof<IEnumerable<_>>)
52+
if Type.(=) (t, stringType) then
53+
ValueNone
54+
elif t.IsArray then
55+
t.GetElementType () |> ValueOption.ofObj
56+
elif isEnumerableInterface t then
57+
ValueSome (t.GetGenericArguments()[0])
58+
else
59+
t.GetInterfaces ()
60+
|> Array.vtryFind isEnumerableInterface
61+
|> ValueOption.map (fun i -> i.GetGenericArguments()[0])
62+
63+
/// <summary>
64+
/// Suffixes the middleware's parser preserves on <c>FieldFilter.FieldName</c> for scalar operators (e.g. <c>meetingId_eq</c>, <c>
65+
/// validFrom_gte</c>). They must be stripped before resolving the actual CLR property.
66+
/// These correspond to the lowercase variants produced after Phase 2 parsing in <c>SchemaDefinitions.parseFieldCondition</c>. Longer suffixes are
67+
/// listed first to prevent shorter ones (e.g. <c>_gt</c>) from incorrectly matching longer ones (e.g. <c>_gte</c>).
68+
/// </summary>
69+
let operatorSuffixes =
70+
[|
71+
// String operators (case-insensitive variants)
72+
FilterSuffixConstants.CI.StartsWithSuffix
73+
FilterSuffixConstants.CI.EndsWithSuffix
74+
FilterSuffixConstants.CI.SWSuffix
75+
FilterSuffixConstants.CI.EWSuffix
76+
FilterSuffixConstants.CI.ContainsSuffix
77+
FilterSuffixConstants.CI.EqualsSuffix
78+
FilterSuffixConstants.CI.EQSuffix
79+
// String operators (case-sensitive variants)
80+
FilterSuffixConstants.CS.StartsWithSuffix
81+
FilterSuffixConstants.CS.EndsWithSuffix
82+
FilterSuffixConstants.CS.SWSuffix
83+
FilterSuffixConstants.CS.EWSuffix
84+
FilterSuffixConstants.CS.ContainsSuffix
85+
FilterSuffixConstants.CS.EqualsSuffix
86+
FilterSuffixConstants.CS.EQSuffix
87+
// Numeric/comparison operators (from root)
88+
FilterSuffixConstants.GreaterThanOrEqualSuffix
89+
FilterSuffixConstants.LessThanOrEqualSuffix
90+
FilterSuffixConstants.GreaterThanSuffix
91+
FilterSuffixConstants.LessThanSuffix
92+
FilterSuffixConstants.GTESuffix
93+
FilterSuffixConstants.LTESuffix
94+
FilterSuffixConstants.GTSuffix
95+
FilterSuffixConstants.LTSuffix
96+
FilterSuffixConstants.InSuffix
97+
|]
98+
99+
let stripOperatorSuffix (fieldName : string) : string =
100+
operatorSuffixes
101+
|> Array.vtryFind (fun s -> fieldName.EndsWith (s, StringComparison.OrdinalIgnoreCase))
102+
|> ValueOption.map (fun s -> fieldName.Substring (0, fieldName.Length - s.Length))
103+
|> ValueOption.defaultValue fieldName
104+
105+
/// <summary>
106+
/// Writes a boxed GraphQL scalar primitive as a JSON token directly into <paramref name="writer"/>. Strings become JSON strings; numbers and
107+
/// booleans become raw JSON tokens. Returns <c>true</c> if the value was written; <c>false</c> if the type is unsupported.
108+
/// </summary>
109+
let private writeJsonValue (value : obj) (writer : Utf8JsonWriter) : bool =
110+
match value with
111+
| :? string as s ->
112+
writer.WriteStringValue s
113+
true
114+
| :? bool as b ->
115+
writer.WriteBooleanValue b
116+
true
117+
| :? int64 as n ->
118+
writer.WriteNumberValue n
119+
true
120+
| :? int as n ->
121+
writer.WriteNumberValue n
122+
true
123+
| :? double as n ->
124+
writer.WriteNumberValue n
125+
true
126+
| :? float32 as n ->
127+
writer.WriteNumberValue n
128+
true
129+
| :? decimal as n ->
130+
writer.WriteNumberValue n
131+
true
132+
| _ ->
133+
false
134+
135+
// Suppress nullness warnings for the obj / objnull mixture.
136+
#nowarn "3261"
137+
/// <summary>
138+
/// Tries to coerce a value into <paramref name="targetType"/> using STJ deserialization. Primitives are written directly as JSON bytes via
139+
/// <see cref="writeJsonValue"/> into an <see cref="ArrayBufferWriter{T}"/>, then deserialized from <c>ReadOnlySpan&lt;byte&gt;</c>.
140+
/// Already-correct values pass through unchanged. No intermediate string or <see cref="JsonDocument"/> is allocated.
141+
/// </summary>
142+
let tryCoerceValue (jsonOptions : JsonSerializerOptions voption) (targetType : Type) (value : objnull) : obj voption =
143+
if isNull value then
144+
ValueNone
145+
elif targetType.IsInstanceOfType value then
146+
ValueSome value
147+
else
148+
let buffer = ArrayBufferWriter<byte> 64
149+
use writer = new Utf8JsonWriter (buffer)
150+
if not (writeJsonValue value writer) then
151+
ValueNone
152+
else
153+
writer.Flush ()
154+
try
155+
let opts = jsonOptions |> ValueOption.defaultValue JsonSerializerOptions.Default
156+
JsonSerializer.Deserialize (buffer.WrittenSpan, targetType, opts) |> ValueSome
157+
with _ ->
158+
ValueNone
159+
160+
/// <summary>
161+
/// Coerces an entire <see cref="ObjectListFilter"/> tree recursively by resolving the entities's properties and converting filter values into the
162+
/// property's CLR type.
163+
/// </summary>
164+
let rec coerceFilter (jsonOptions : JsonSerializerOptions voption) (entityType : Type) (filter : ObjectListFilter) : ObjectListFilter =
165+
match filter with
166+
| And (l, r) -> And (coerceFilter jsonOptions entityType l, coerceFilter jsonOptions entityType r)
167+
| Or (l, r) -> Or (coerceFilter jsonOptions entityType l, coerceFilter jsonOptions entityType r)
168+
| Not f -> Not (coerceFilter jsonOptions entityType f)
169+
| OfTypes _ -> filter
170+
| Equals (ff, cmp) ->
171+
match entityType.GetProperty (stripOperatorSuffix ff.FieldName, propertyBindFlags) with
172+
| null -> filter
173+
| prop ->
174+
let unwrapped = unwrapOption prop.PropertyType
175+
match tryCoerceValue jsonOptions unwrapped (box ff.Value) with
176+
| ValueNone -> filter
177+
| ValueSome (:? IComparable as coerced) -> Equals ({ ff with Value = coerced }, cmp)
178+
| ValueSome _ -> filter
179+
| GreaterThan ff
180+
| GreaterThanOrEqual ff
181+
| LessThan ff
182+
| LessThanOrEqual ff as originalFilter ->
183+
match entityType.GetProperty (stripOperatorSuffix ff.FieldName, propertyBindFlags) with
184+
| null -> filter
185+
| prop ->
186+
let unwrapped = unwrapOption prop.PropertyType
187+
match tryCoerceValue jsonOptions unwrapped (box ff.Value) with
188+
| ValueNone -> filter
189+
| ValueSome (:? IComparable as coerced) ->
190+
let coercedField = { ff with Value = coerced }
191+
match originalFilter with
192+
| GreaterThan _ -> GreaterThan coercedField
193+
| GreaterThanOrEqual _ -> GreaterThanOrEqual coercedField
194+
| LessThan _ -> LessThan coercedField
195+
| LessThanOrEqual _ -> LessThanOrEqual coercedField
196+
| _ -> filter
197+
| ValueSome _ -> filter
198+
| In ff ->
199+
match entityType.GetProperty (stripOperatorSuffix ff.FieldName, propertyBindFlags) with
200+
| null -> filter
201+
| prop ->
202+
let unwrapped = unwrapOption prop.PropertyType
203+
204+
let struct (coercedValues, failedValues) =
205+
ff.Value
206+
|> List.fold
207+
(fun struct (coerced, failed) value ->
208+
match tryCoerceValue jsonOptions unwrapped value with
209+
| ValueSome coercedValue -> (coercedValue :: coerced, failed)
210+
| ValueNone -> struct (coerced, value :: failed))
211+
([], [])
212+
213+
match failedValues with
214+
| [] -> In { ff with Value = List.rev coercedValues }
215+
| _ ->
216+
let failedValuesText =
217+
failedValues
218+
|> Seq.rev
219+
|> Seq.map (sprintf "%A")
220+
|> String.concat ", "
221+
222+
invalidArg
223+
(nameof filter)
224+
($"Unable to coerce one or more values for '{ff.FieldName}' to '{unwrapped.FullName}'. Uncoerced values: [{failedValuesText}]")
225+
| StartsWith (ff, cmp)
226+
| EndsWith (ff, cmp) as originalFilter ->
227+
match entityType.GetProperty (stripOperatorSuffix ff.FieldName, propertyBindFlags) with
228+
| null -> filter
229+
| _ ->
230+
match tryCoerceValue jsonOptions stringType (box ff.Value) with
231+
| ValueNone -> filter
232+
| ValueSome coerced ->
233+
let coercedField = { ff with Value = coerced :?> string }
234+
match originalFilter with
235+
| StartsWith (_, cmp) -> StartsWith (coercedField, cmp)
236+
| EndsWith (_, cmp) -> EndsWith (coercedField, cmp)
237+
| _ -> filter
238+
| Contains (ff, cmp) ->
239+
match entityType.GetProperty (stripOperatorSuffix ff.FieldName, propertyBindFlags) with
240+
| null -> filter
241+
| prop ->
242+
let unwrapped = unwrapOption prop.PropertyType
243+
let coercionTarget =
244+
match tryUnwrapEnumerableElement unwrapped with
245+
| ValueSome elementType -> elementType
246+
| ValueNone -> stringType
247+
match tryCoerceValue jsonOptions coercionTarget (box ff.Value) with
248+
| ValueNone -> filter
249+
| ValueSome (:? IComparable as coerced) -> Contains ({ ff with Value = coerced }, cmp)
250+
| ValueSome _ -> filter
251+
| FilterField ff ->
252+
match entityType.GetProperty (ff.FieldName, propertyBindFlags) with
253+
| null -> filter
254+
| prop ->
255+
let unwrapped = unwrapOption prop.PropertyType
256+
let nestedType =
257+
tryUnwrapEnumerableElement unwrapped
258+
|> ValueOption.defaultValue unwrapped
259+
FilterField { FieldName = ff.FieldName; Value = coerceFilter jsonOptions nestedType ff.Value }

src/FSharp.Data.GraphQL.Server.Middleware/TypeSystemExtensions.fs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ namespace FSharp.Data.GraphQL.Server.Middleware
33
open System
44
open System.Collections.Immutable
55
open System.Linq
6-
open FsToolkit.ErrorHandling
76

8-
open FSharp.Data.GraphQL
97
open FSharp.Data.GraphQL.Types
108

119
/// Contains extensions for the type system.

src/FSharp.Data.GraphQL.Shared/Helpers/Extensions.fs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,35 @@ module Array =
8585
i <- i + 1
8686
Array.sub temp 0 i
8787

88+
/// <summary>
89+
/// Attempts to find the first element in an array that satisfies the given predicate.
90+
/// </summary>
91+
/// <param name="predicate">Function to test each element.</param>
92+
/// <param name="source">The input array.</param>
93+
/// <returns>ValueSome of the first matching element, or ValueNone if no match is found.</returns>
94+
let vtryFind predicate (source : 'T array) =
95+
let mutable result = ValueNone
96+
let mutable i = 0
97+
while i < source.Length && result.IsNone do
98+
if predicate source[i] then
99+
result <- ValueSome source[i]
100+
i <- i + 1
101+
result
102+
103+
/// <summary>
104+
/// Applies a function to each element of an array and returns the first result where the function returns ValueSome.
105+
/// </summary>
106+
/// <param name="mapping">Function to apply to each element.</param>
107+
/// <param name="source">The input array.</param>
108+
/// <returns>ValueSome of the first successful mapping result, or ValueNone if no match is found.</returns>
109+
let vtryPick mapping (source : 'T array) =
110+
let mutable result = ValueNone
111+
let mutable i = 0
112+
while i < source.Length && result.IsNone do
113+
result <- mapping source[i]
114+
i <- i + 1
115+
result
116+
88117
module List =
89118

90119
/// <summary>
@@ -99,6 +128,35 @@ module List =
99128
|> List.filter (fun x -> not <| List.exists(fun y -> f(x) = f(y)) listy)
100129
uniqx @ listy
101130

131+
/// <summary>
132+
/// Attempts to find the first element in a list that satisfies the given predicate.
133+
/// </summary>
134+
/// <param name="predicate">Function to test each element.</param>
135+
/// <param name="source">The input list.</param>
136+
/// <returns>ValueSome of the first matching element, or ValueNone if no match is found.</returns>
137+
let rec vtryFind predicate (source : 'T list) =
138+
match source with
139+
| [] -> ValueNone
140+
| head :: tail ->
141+
if predicate head then
142+
ValueSome head
143+
else
144+
vtryFind predicate tail
145+
146+
/// <summary>
147+
/// Applies a function to each element of a list and returns the first result where the function returns ValueSome.
148+
/// </summary>
149+
/// <param name="mapping">Function to apply to each element.</param>
150+
/// <param name="source">The input list.</param>
151+
/// <returns>ValueSome of the first successful mapping result, or ValueNone if no match is found.</returns>
152+
let rec vtryPick mapping (source : 'T list) =
153+
match source with
154+
| [] -> ValueNone
155+
| head :: tail ->
156+
match mapping head with
157+
| ValueSome result -> ValueSome result
158+
| ValueNone -> vtryPick mapping tail
159+
102160
module Set =
103161

104162
/// <summary>

tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,11 @@
7575
<Compile Include="Variables and Inputs\InputEnumTests.fs" />
7676
<Compile Include="Variables and Inputs\InputListTests.fs" />
7777
<Compile Include="SelectLinqTests.fs" />
78-
<Compile Include="ObjectListFilterLinqTests.fs" />
79-
<Compile Include="ObjectListFilterLinqGenerateTests.fs" />
78+
<Compile Include="ObjectListFilter\ObjectListFilterLinqTests.fs" />
79+
<Compile Include="ObjectListFilter\ObjectListFilterLinqGenerateTests.fs" />
80+
<Compile Include="ObjectListFilter\TypeCoercionTests.Common.fs" />
81+
<Compile Include="ObjectListFilter\TypeCoercionValueTests.fs" />
82+
<Compile Include="ObjectListFilter\TypeCoercionFilterTests.fs" />
8083
<Compile Include="DeferredTests.fs" />
8184
<Compile Include="SubscriptionTests.fs" />
8285
<Compile Include="MiddlewareTests.fs" />

0 commit comments

Comments
 (0)