Skip to content

Commit 5f932d9

Browse files
committed
fixup! ObjectListFilter filter values to target type coercion (#589)
1 parent a2c54ec commit 5f932d9

3 files changed

Lines changed: 216 additions & 7 deletions

File tree

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

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,21 @@ open System.Runtime.InteropServices
4040
type private CompareDiscriminatorExpression<'T, 'D> = Expression<Func<'T, 'D, bool>>
4141

4242
/// <summary>
43-
/// Optional configuration for LINQ translation including discriminator handling.
43+
/// Initializes a new instance of <see cref="ObjectListFilterLinqOptions{T,D}"/> with optional
44+
/// LINQ translation settings including discriminator handling and <c>In</c>-operator behavior.
4445
/// </summary>
46+
/// <typeparam name="T">Entity type.</typeparam>
47+
/// <typeparam name="D">Discriminator value type.</typeparam>
48+
/// <param name="compareDiscriminator">
49+
/// Optional custom discriminator comparison expression.
50+
/// </param>
51+
/// <param name="getDiscriminatorValue">
52+
/// Optional discriminator value resolver for <c>OfTypes</c> filtering.
53+
/// </param>
54+
/// <param name="jsonOptions">
55+
/// Optional serializer settings used during filter value coercion.
56+
/// </param>
57+
4558
/// <example id="item-1"><code lang="fsharp">
4659
/// // discriminator custom condition
4760
/// let result () =
@@ -70,35 +83,58 @@ type private CompareDiscriminatorExpression<'T, 'D> = Expression<Func<'T, 'D, bo
7083
/// </code></example>
7184
type ObjectListFilterLinqOptions<'T, 'D>
7285
(
86+
/// Optional custom discriminator comparison expression.
7387
[<Optional>] compareDiscriminator : CompareDiscriminatorExpression<'T, 'D> | null,
88+
/// <summary>Optional discriminator value resolver used for <c>OfTypes</c> filtering.</summary>
7489
[<Optional>] getDiscriminatorValue : (Type -> 'D) | null,
90+
/// Optional serializer settings used during filter value coercion.
7591
[<Optional>] jsonOptions : JsonSerializerOptions | null
7692
) =
7793

94+
/// <summary>Gets the optional custom discriminator comparison expression.</summary>
7895
member _.CompareDiscriminator = compareDiscriminator |> ValueOption.ofObj
96+
97+
/// <summary>Gets the optional discriminator value resolver.</summary>
7998
member _.GetDiscriminatorValue = getDiscriminatorValue |> ValueOption.ofObj
99+
100+
/// <summary>Gets optional serializer settings used during filter coercion.</summary>
80101
member _.JsonOptions = jsonOptions |> ValueOption.ofObj
81102

103+
/// <summary>Default options with all features disabled.</summary>
82104
static member None = ObjectListFilterLinqOptions<'T, 'D> (null, null, null)
83105

106+
/// Creates a discriminator comparison expression from a discriminator selector.
84107
static member GetCompareDiscriminator (getDiscriminatorValue : Expression<Func<'T, 'D>>) =
85108
let tParam = Expression.Parameter (typeof<'T>, "x")
86109
let dParam = Expression.Parameter (typeof<'D>, "d")
87110
let body = Expression.Equal (Expression.Invoke (getDiscriminatorValue, tParam), dParam)
88111
Expression.Lambda<Func<'T, 'D, bool>> (body, tParam, dParam)
89112

113+
/// Initializes options using a discriminator selector expression.
90114
new (getDiscriminator : Expression<Func<'T, 'D>>) =
91115
ObjectListFilterLinqOptions<'T, 'D> (ObjectListFilterLinqOptions.GetCompareDiscriminator getDiscriminator, null, null)
116+
117+
/// Initializes options using a custom discriminator comparison expression.
92118
new (compareDiscriminator : CompareDiscriminatorExpression<'T, 'D>) =
93119
ObjectListFilterLinqOptions<'T, 'D> (compareDiscriminator, null, null)
120+
121+
/// Initializes options using a discriminator value resolver.
94122
new (getDiscriminatorValue : Type -> 'D) =
95123
ObjectListFilterLinqOptions<'T, 'D> (null, getDiscriminatorValue, null)
124+
125+
/// Initializes options using both discriminator selector and value resolver.
96126
new (getDiscriminator : Expression<Func<'T, 'D>>, getDiscriminatorValue : Type -> 'D) =
97127
ObjectListFilterLinqOptions<'T, 'D> (ObjectListFilterLinqOptions.GetCompareDiscriminator getDiscriminator, getDiscriminatorValue, null)
128+
129+
/// Initializes options using serializer settings for filter coercion.
98130
new (jsonOptions : JsonSerializerOptions) =
99131
ObjectListFilterLinqOptions<'T, 'D> (null, null, jsonOptions)
132+
133+
/// Initializes options using discriminator selector and serializer settings.
100134
new (getDiscriminator : Expression<Func<'T, 'D>>, jsonOptions : JsonSerializerOptions) =
101135
ObjectListFilterLinqOptions<'T, 'D> (ObjectListFilterLinqOptions.GetCompareDiscriminator getDiscriminator, null, jsonOptions)
136+
137+
/// Initializes options using discriminator comparison and serializer settings.
102138
new (compareDiscriminator : CompareDiscriminatorExpression<'T, 'D>, jsonOptions : JsonSerializerOptions) =
103139
ObjectListFilterLinqOptions<'T, 'D> (compareDiscriminator, null, jsonOptions)
104140

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

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,25 @@ module ObjectListFilter =
263263
raise (MissingMemberException message)
264264
| anyGenericStaticMethod -> anyGenericStaticMethod.MakeGenericMethod ([| elementType |])
265265

266+
let private normalizeInValue (fieldType : Type) (value : obj) : obj =
267+
let normalized = Values.normalizeOptional fieldType value
268+
if obj.ReferenceEquals (normalized, null) then
269+
null
270+
elif fieldType.IsGenericType && fieldType.GetGenericTypeDefinition () = typedefof<Nullable<_>> then
271+
let underlyingType = Nullable.GetUnderlyingType fieldType
272+
if not (obj.ReferenceEquals (underlyingType, null)) && normalized.GetType () = underlyingType then
273+
Activator.CreateInstance (fieldType, normalized)
274+
else
275+
normalized
276+
else
277+
normalized
278+
279+
let private materializeTypedInArray (fieldType : Type) (values : obj list) : Array =
280+
let array = Array.CreateInstance (fieldType, values.Length)
281+
values
282+
|> List.iteri (fun index value -> array.SetValue (normalizeInValue fieldType value, index))
283+
array
284+
266285
let rec buildFilterExpr isEnumerableQuery (param : SourceExpression) buildTypeDiscriminatorCheck filter : Expression =
267286

268287
let build = buildFilterExpr isEnumerableQuery param buildTypeDiscriminatorCheck
@@ -447,12 +466,10 @@ module ObjectListFilter =
447466
param.Value
448467
else
449468
Expression.PropertyOrField (param, f.FieldName)
450-
// The list is already coerced to the correct element type by TypeCoercion.coerceFilter.
451-
// We use objectType for the expression because providers like Cosmos cannot convert from
452-
// obj list to IEnumerable<T> at runtime. The coerced values are already boxed correctly,
453-
// so Enumerable.Contains<object> works universally.
454-
let enumerableContains = getEnumerableContainsMethod objectType
455-
Expression.Call (enumerableContains, Expression.Constant f.Value, Expression.Convert (``member``, objectType))
469+
let fieldType = ``member``.Type
470+
let typedValues = materializeTypedInArray fieldType f.Value
471+
let enumerableContains = getEnumerableContainsMethod fieldType
472+
Expression.Call (enumerableContains, Expression.Constant typedValues, ``member``)
456473
| In f -> Expression.Constant (false)
457474
| OfTypes types ->
458475
types

tests/FSharp.Data.GraphQL.Tests/ObjectListFilter/TypeCoercionFilterInOperatorTests.fs

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
[<Xunit.Trait (Tests.TraitType.ObjectListFilterOperator, "in")>]
33
module FSharp.Data.GraphQL.Tests.ObjectListFilter.TypeCoercion.InOperatorTests
44

5+
open System
6+
open System.Linq
7+
open System.Linq.Expressions
58
open Xunit
9+
open FSharp.Data.GraphQL.Shared
610
open FSharp.Data.GraphQL.Server.Middleware
711
open FSharp.Data.GraphQL.Tests.ObjectListFilter.TypeCoercion.Common
812

@@ -11,6 +15,96 @@ open FSharp.Data.GraphQL.Tests.ObjectListFilter.TypeCoercion.Common
1115
// Covers all coercion types: CLR enum, DU-as-enum, Guid, single-case DU, and primitives
1216
// ──────────────────────────────────────────────────────────────────────────────
1317

18+
/// Minimal entity model used to validate `In` translation for nullable fields.
19+
type NullableEntity = {
20+
/// Primary identifier used in test assertions.
21+
Id: int
22+
/// Nullable scalar field used to assert typed `Contains<Nullable<int>>` expression generation.
23+
MaybeId: Nullable<int>
24+
}
25+
26+
/// Query options for nullable-expression tests with JSON coercion enabled.
27+
let private nullableOptions =
28+
ObjectListFilterLinqOptions<NullableEntity, obj> (Json.getSerializerOptions Seq.empty)
29+
30+
/// In-memory source for nullable-field `In` tests.
31+
let private nullableData =
32+
[|
33+
{ Id = 1; MaybeId = Nullable 1 }
34+
{ Id = 2; MaybeId = Nullable() }
35+
{ Id = 3; MaybeId = Nullable 3 }
36+
|]
37+
38+
/// Applies an `ObjectListFilter` to the nullable-field test dataset.
39+
let private applyNullableFilter (filter : ObjectListFilter) =
40+
filter.ApplyTo (nullableData.AsQueryable (), nullableOptions) |> Seq.toList
41+
42+
/// Traverses an expression tree and returns the first `Enumerable.Contains` call node.
43+
let private tryFindEnumerableContainsCall (expr : Expression) : MethodCallExpression option =
44+
let rec find (node : Expression) =
45+
match node with
46+
| :? MethodCallExpression as call
47+
when call.Method.Name = "Contains"
48+
&& call.Method.DeclaringType = typeof<Enumerable>
49+
&& call.Arguments.Count = 2 ->
50+
Some call
51+
| :? MethodCallExpression as call ->
52+
let fromObject =
53+
if isNull call.Object then None else find call.Object
54+
match fromObject with
55+
| Some found -> Some found
56+
| None -> call.Arguments |> Seq.tryPick find
57+
| :? UnaryExpression as unary -> find unary.Operand
58+
| :? LambdaExpression as lambda -> find lambda.Body
59+
| :? BinaryExpression as binary ->
60+
match find binary.Left with
61+
| Some found -> Some found
62+
| None -> find binary.Right
63+
| :? MemberExpression as memberExpr ->
64+
if isNull memberExpr.Expression then None else find memberExpr.Expression
65+
| _ -> None
66+
find expr
67+
68+
/// Builds a filtered query and extracts the generated `Enumerable.Contains` call from its expression tree.
69+
let private findEnumerableContainsCall<'T, 'D> (options : ObjectListFilterLinqOptions<'T, 'D>) (filter : ObjectListFilter) =
70+
let query = Enumerable.Empty<'T>().AsQueryable()
71+
let result = query.Apply (filter, options)
72+
match tryFindEnumerableContainsCall result.Expression with
73+
| Some call -> call
74+
| None ->
75+
fail "Expected to find Enumerable.Contains call in generated expression tree"
76+
Unchecked.defaultof<MethodCallExpression>
77+
78+
/// Asserts that `In` is translated to a strongly typed `Enumerable.Contains<TField>` expression.
79+
/// Validates method generic argument, typed values container, and non-boxed member argument.
80+
let private assertTypedInExpression<'T, 'D>
81+
(options : ObjectListFilterLinqOptions<'T, 'D>)
82+
(filter : ObjectListFilter)
83+
(expectedElementType : Type)
84+
=
85+
let containsCall = findEnumerableContainsCall options filter
86+
containsCall.Method.GetGenericArguments().[0] |> equals expectedElementType
87+
88+
let valuesArgType = containsCall.Arguments.[0].Type
89+
if valuesArgType = typeof<obj> || valuesArgType = typeof<obj list> then
90+
fail $"Expected a strongly typed values argument, but got {valuesArgType.FullName}"
91+
92+
let actualElementType =
93+
if valuesArgType.IsArray then
94+
valuesArgType.GetElementType ()
95+
elif valuesArgType.IsGenericType then
96+
valuesArgType.GetGenericArguments().[0]
97+
else
98+
fail $"Expected array or generic collection values argument, got {valuesArgType.FullName}"
99+
Unchecked.defaultof<Type>
100+
101+
actualElementType |> equals expectedElementType
102+
103+
match containsCall.Arguments.[1] with
104+
| :? UnaryExpression as unary when unary.NodeType = ExpressionType.Convert && unary.Type = typeof<obj> ->
105+
fail "Expected In member argument to remain strongly typed without boxing to object"
106+
| _ -> ()
107+
14108
[<Fact>]
15109
let ``In operator coerces string primitives`` () =
16110
let filter = In { FieldName = "name"; Value = [ box "Alice"; box "Bob" ] }
@@ -77,3 +171,65 @@ let ``In operator coerces single-case DU wrapping Guid`` () =
77171
let result = applyFilter filter
78172
result |> List.length |> equals 2
79173
result |> List.map (fun e -> e.Name) |> List.sort |> equals [ "Alice"; "Charlie" ]
174+
175+
[<Fact>]
176+
let ``In operator coerces Nullable int primitives`` () =
177+
let filter = In { FieldName = "maybeId"; Value = [ box 1; box 3 ] }
178+
let result = applyNullableFilter filter
179+
result |> List.map (fun e -> e.Id) |> List.sort |> equals [ 1; 3 ]
180+
181+
[<Fact>]
182+
let ``In operator expression uses typed contains for string primitive field`` () =
183+
let filter = In { FieldName = "name"; Value = [ box "Alice"; box "Bob" ] }
184+
assertTypedInExpression filterOptions filter typeof<string>
185+
186+
[<Fact>]
187+
let ``In operator expression uses typed contains for int primitive field`` () =
188+
let filter = In { FieldName = "id"; Value = [ box 1; box 3 ] }
189+
assertTypedInExpression filterOptions filter typeof<int>
190+
191+
[<Fact>]
192+
let ``In operator expression uses typed contains for CLR enum field`` () =
193+
let filter = In { FieldName = "color"; Value = [ box "Red"; box "Blue" ] }
194+
assertTypedInExpression filterOptions filter typeof<Color>
195+
196+
[<Fact>]
197+
let ``In operator expression uses typed contains for fieldless DU field`` () =
198+
let filter = In { FieldName = "status"; Value = [ box "Active"; box "Pending" ] }
199+
assertTypedInExpression filterOptions filter typeof<Status>
200+
201+
[<Fact>]
202+
let ``In operator expression uses typed contains for Guid field`` () =
203+
let filter = In { FieldName = "guidField"; Value = [ box "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" ] }
204+
assertTypedInExpression filterOptions filter typeof<Guid>
205+
206+
[<Fact>]
207+
let ``In operator expression uses typed contains for single-case DU string field`` () =
208+
let filter = In { FieldName = "wrappedName"; Value = [ box "Alice"; box "Charlie" ] }
209+
assertTypedInExpression filterOptions filter typeof<WrappedString>
210+
211+
[<Fact>]
212+
let ``In operator expression uses typed contains for single-case DU int field`` () =
213+
let filter = In { FieldName = "wrappedScore"; Value = [ box 10; box 30 ] }
214+
assertTypedInExpression filterOptions filter typeof<WrappedInt>
215+
216+
[<Fact>]
217+
let ``In operator expression uses typed contains for single-case DU Guid field`` () =
218+
let filter = In { FieldName = "wrappedGuid"; Value = [ box "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" ] }
219+
assertTypedInExpression filterOptions filter typeof<WrappedGuid>
220+
221+
[<Fact>]
222+
let ``In operator expression uses typed contains for option field`` () =
223+
let filter = In { FieldName = "optionName"; Value = [ box "Alice"; box "Charlie" ] }
224+
assertTypedInExpression filterOptions filter typeof<string option>
225+
226+
[<Fact>]
227+
let ``In operator expression uses typed contains for voption field`` () =
228+
let filter = In { FieldName = "vOptionId"; Value = [ box 1; box 3 ] }
229+
assertTypedInExpression filterOptions filter typeof<int voption>
230+
231+
[<Fact>]
232+
let ``In operator expression uses typed contains for Nullable field`` () =
233+
let filter = In { FieldName = "maybeId"; Value = [ box 1; box 3 ] }
234+
assertTypedInExpression nullableOptions filter typeof<Nullable<int>>
235+

0 commit comments

Comments
 (0)