Skip to content

Commit fc0e9a9

Browse files
xperiandriCopilot
xperiandri
and
Copilot
authored
Switched resolver and execution logic to voption (#614)
* Switched to `voption` for resolver and execution logic * Refactored codebase to use F# `voption` (value option) instead of option for improved performance and clarity, especially in resolver and execution logic. * Replaced `Option` functions with `ValueOption` equivalents, added `vtryPick` for arrays and extended `vtryFind` for lists. * Updated pattern matching and function signatures to use `ValueSome`/`ValueNone`. * Changed resolver result types to use `IObservable<_> voption`. * Improved error handling, string formatting, and added `objectOptionCast`/`toValueOption` helpers in `Reflection.fs`. * Updated code comments and XML docs. * Ensured backward compatibility and better nullability handling. * Moved collection/option helpers to a new module * Refactored and reorganized collection and option helpers from `Extensions.fs` and `ObjAndStructConversions.fs` into `Helpers/CollectionExtensions.fs`. * Moved `kvp`, `kvpObj`, and `IDictionary` extension methods to the new file. * Removed redundant implementations from the original files. * Updated usages to reference the new module and clarified documentation comments. * Ensured helpers are internal or auto-opened for seamless usage. * Fix boxed ValueNone handling in toValueOption Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com> * Guard toValueOption against null FullName Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com> * Handle missing FullName in toValueOption Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com> * Tighten option type detection in toValueOption Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com> * fixup! Fix boxed ValueNone handling in toValueOption --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>
1 parent 63d1e59 commit fc0e9a9

12 files changed

Lines changed: 473 additions & 423 deletions

File tree

‎src/FSharp.Data.GraphQL.Client/BaseTypes.fs‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,15 @@ type RecordBase (name : string, properties : RecordProperty seq) =
147147
| :? string -> v // We need this because strings are enumerables, and we don't want to enumerate them recursively as an object
148148
| :? EnumBase as v -> v.GetValue () |> box
149149
| :? RecordBase as v -> box (v.ToDictionary ())
150-
| OptionValue v -> v |> Option.map mapDictionaryValue |> Option.toObj
150+
| OptionValue v -> v |> ValueOption.map mapDictionaryValue |> ValueOption.toObj
151151
| EnumerableValue v -> v |> Array.map mapDictionaryValue |> box
152152
| _ -> v
153153
x.GetProperties ()
154-
|> Seq.choose (fun p ->
154+
|> Seq.vchoose (fun p ->
155155
if not (isNull p.Value) then
156-
Some (p.Name, mapDictionaryValue p.Value)
156+
ValueSome (p.Name, mapDictionaryValue p.Value)
157157
else
158-
None)
158+
ValueNone)
159159
|> dict
160160

161161
override x.ToString () =
@@ -309,8 +309,8 @@ module internal JsonValueHelper =
309309

310310
let getTypeName (fields : (string * JsonValue) seq) =
311311
fields
312-
|> Seq.tryFind (fun (name, _) -> name = "__typename")
313-
|> Option.map (fun (_, value) ->
312+
|> Seq.vtryFind (fun (name, _) -> name = "__typename")
313+
|> ValueOption.map (fun (_, value) ->
314314
match value with
315315
| JsonValue.String x -> x
316316
| _ -> failwithf "Expected \"__typename\" field to be a string field, but it was %A." value)
@@ -379,16 +379,16 @@ module internal JsonValueHelper =
379379
| JsonValue.Record props ->
380380
let typeName =
381381
match getTypeName props with
382-
| Some typeName -> typeName
383-
| None -> failwith "Expected type to have a \"__typename\" field, but it was not found."
382+
| ValueSome typeName -> typeName
383+
| ValueNone -> failwith "Expected type to have a \"__typename\" field, but it was not found."
384384
let mapRecordProperty (aliasOrName : string, value : JsonValue) =
385385
let schemaField =
386386
match
387387
schemaField.Fields
388-
|> Array.tryFind (fun f -> f.AliasOrName = aliasOrName)
388+
|> Array.vtryFind (fun f -> f.AliasOrName = aliasOrName)
389389
with
390-
| Some f -> f
391-
| None ->
390+
| ValueSome f -> f
391+
| ValueNone ->
392392
failwithf
393393
"Expected to find field information for field with alias or name \"%s\" of type \"%s\" but it was not found."
394394
aliasOrName
@@ -479,49 +479,49 @@ module internal JsonValueHelper =
479479
let getErrors (errors : JsonValue[]) =
480480
let tryFindField fieldName (fields : (string * JsonValue)[]) =
481481
fields
482-
|> Array.tryFind (fun (name, _) -> name = fieldName)
483-
|> Option.map snd
482+
|> Array.vtryFind (fun (name, _) -> name = fieldName)
483+
|> ValueOption.map snd
484484

485-
let parsePath =
486-
function
487-
| Some (JsonValue.Array path) ->
485+
let parsePath jsonValueOpt =
486+
match jsonValueOpt with
487+
| ValueSome (JsonValue.Array path) ->
488488
let pathMapper =
489489
function
490490
| JsonValue.String x -> box x
491491
| JsonValue.Integer x -> box x
492492
| _ -> failwith "Error parsing response errors. An item in the path is neither a String nor an Integer."
493493
path |> Array.map pathMapper
494-
| Some JsonValue.Null
495-
| None -> [||]
494+
| ValueSome JsonValue.Null
495+
| ValueNone -> [||]
496496
| _ -> failwith "Error parsing response errors. Path field must be an Array."
497497

498-
let parseLocations =
499-
function
500-
| Some (JsonValue.Array locations) ->
498+
let parseLocations jsonValueOpt =
499+
match jsonValueOpt with
500+
| ValueSome (JsonValue.Array locations) ->
501501
let parseLocation =
502502
function
503503
| JsonValue.Record locationFields ->
504504
match tryFindField "line" locationFields, tryFindField "column" locationFields with
505-
| Some (JsonValue.Integer line), Some (JsonValue.Integer column) -> { Line = line; Column = column }
505+
| ValueSome (JsonValue.Integer line), ValueSome (JsonValue.Integer column) -> { Line = line; Column = column }
506506
| _ -> failwith "Error parsing response errors. A location item must contain Integer fields named \"line\" and \"column\"."
507507
| _ -> failwith "Error parsing response errors. A location item is not a Record."
508508
locations |> Array.map parseLocation
509-
| Some JsonValue.Null
510-
| None -> [||]
509+
| ValueSome JsonValue.Null
510+
| ValueNone -> [||]
511511
| _ -> failwith "Error parsing response errors. Locations field must be an Array."
512512

513-
let parseExtensions =
514-
function
515-
| Some (JsonValue.Record fields) -> Serialization.deserializeMap fields
516-
| Some JsonValue.Null
517-
| None -> Map.empty
513+
let parseExtensions jsonValueOpt =
514+
match jsonValueOpt with
515+
| ValueSome (JsonValue.Record fields) -> Serialization.deserializeMap fields
516+
| ValueSome JsonValue.Null
517+
| ValueNone -> Map.empty
518518
| _ -> failwith "Error parsing response errors. Extensions field must be a Record."
519519

520520
let errorMapper =
521521
function
522522
| JsonValue.Record fields ->
523523
match tryFindField "message" fields with
524-
| Some (JsonValue.String message) -> {
524+
| ValueSome (JsonValue.String message) -> {
525525
Message = message
526526
Locations = tryFindField "locations" fields |> parseLocations
527527
Path = tryFindField "path" fields |> parsePath
@@ -597,6 +597,6 @@ module VariableMapping =
597597
| :? string -> value
598598
| :? EnumBase as v -> v.GetValue () |> box
599599
| :? RecordBase as v -> v.ToDictionary () |> box
600-
| OptionValue v -> v |> Option.map mapVariableValue |> box
600+
| OptionValue v -> v |> ValueOption.map mapVariableValue |> ValueOption.toObj
601601
| EnumerableValue v -> v |> Array.map mapVariableValue |> box
602602
| v -> v

‎src/FSharp.Data.GraphQL.Client/GraphQLClient.fs‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -125,24 +125,21 @@ module GraphQLClient =
125125
let rec tryMapFileVariable (name : string, value : obj) =
126126
match value with
127127
| null
128-
| :? string -> None
129-
| :? Upload as x -> Some [| name, x |]
130-
| OptionValue x -> x |> Option.bind (fun x -> tryMapFileVariable (name, x))
128+
| :? string -> [||]
129+
| :? Upload as x -> [| struct (name, x) |]
130+
| OptionValue x -> x |> ValueOption.map (fun x -> tryMapFileVariable (name, x)) |> ValueOption.defaultValue [||]
131131
| :? IDictionary<string, obj> as x ->
132132
x
133-
|> Seq.collect (fun kvp ->
134-
tryMapFileVariable (name + "." + (kvp.Key.FirstCharLower ()), kvp.Value)
135-
|> Option.defaultValue [||])
136-
|> Array.ofSeq
137-
|> Some
133+
|> Seq.collect (fun kvp -> tryMapFileVariable (name + "." + (kvp.Key.FirstCharLower ()), kvp.Value))
134+
|> Seq.toArray
138135
| EnumerableValue x ->
139136
x
140-
|> Array.mapi (fun ix x -> tryMapFileVariable ($"%s{name}.%i{ix}", x))
141-
|> Array.collect (Option.defaultValue [||])
142-
|> Some
143-
| _ -> None
137+
|> Seq.mapi (fun ix x -> tryMapFileVariable ($"%s{name}.%i{ix}", x))
138+
|> Seq.collect id
139+
|> Seq.toArray
140+
| _ -> [||]
144141
request.Variables
145-
|> Array.collect (tryMapFileVariable >> (Option.defaultValue [||]))
142+
|> Array.collect tryMapFileVariable
146143

147144
let operationContent =
148145
let variables =
@@ -171,15 +168,15 @@ module GraphQLClient =
171168
let mapContent =
172169
let files =
173170
files
174-
|> Array.mapi (fun ix (name, _) -> ix.ToString (), JsonValue.Array [| JsonValue.String ("variables." + name) |])
171+
|> Array.mapi (fun ix struct (name, _) -> ix.ToString (), JsonValue.Array [| JsonValue.String ("variables." + name) |])
175172
|> JsonValue.Record
176173
let content = new StringContent (files.ToString (JsonSaveOptions.DisableFormatting))
177174
content.Headers.Add ("Content-Disposition", "form-data; name=\"map\"")
178175
content
179176
content.Add (mapContent)
180177
let fileContents =
181178
files
182-
|> Seq.mapi (fun _ (_, value) ->
179+
|> Seq.mapi (fun _ struct (_, value) ->
183180
let content = new StreamContent (value.Stream)
184181
content.Headers.Add ("Content-Disposition", $"form-data; name=\"%s{value.Name}\"; filename=\"%s{value.FileName}\"")
185182
content.Headers.Add ("Content-Type", value.ContentType)

‎src/FSharp.Data.GraphQL.Client/ReflectionPatterns.fs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,8 @@ module ReflectionPatterns =
143143
let xtype = x.GetType()
144144
let tryGetValue optionType =
145145
match FSharpValue.GetUnionFields(x, optionType) with
146-
| (_, [|value|]) -> ValueSome (OptionValue Some value)
147-
| _ -> ValueSome (OptionValue None)
146+
| (_, [|value|]) -> ValueSome (OptionValue ValueSome value)
147+
| _ -> ValueSome (OptionValue ValueNone)
148148
if isOption xtype
149149
then tryGetValue xtype
150150
elif isValueOption xtype
@@ -162,10 +162,10 @@ module ReflectionPatterns =
162162
let isOption = isOption t
163163
match value, isOption with
164164
| null, true -> makeNone t
165-
| OptionValue (Some null), true -> box (makeSome (Convert.ChangeType(null, t)))
166-
| OptionValue (Some value), true -> box (makeSome value)
167-
| OptionValue (Some value), false -> Convert.ChangeType(value, t)
168-
| OptionValue None, false -> Convert.ChangeType(null, t)
169-
| OptionValue None, true -> box (makeNone t)
165+
| OptionValue (ValueSome null), true -> box (makeSome (Convert.ChangeType(null, t)))
166+
| OptionValue (ValueSome value), true -> box (makeSome value)
167+
| OptionValue (ValueSome value), false -> Convert.ChangeType(value, t)
168+
| OptionValue ValueNone, false -> Convert.ChangeType(null, t)
169+
| OptionValue ValueNone, true -> box (makeNone t)
170170
| value, true -> makeSome value
171171
| value, false -> Convert.ChangeType(value, t)

‎src/FSharp.Data.GraphQL.Client/Serialization.fs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ module Serialization =
169169
Tracer.runAndMeasureExecutionTime $"Converted object type %O{t} to JsonValue" (fun _ ->
170170
match x with
171171
| null -> JsonValue.Null
172-
| OptionValue None -> JsonValue.Null
172+
| OptionValue ValueNone -> JsonValue.Null
173173
| :? int as x -> JsonValue.Integer (int x)
174174
| :? float as x -> JsonValue.Float x
175175
| :? string as x -> JsonValue.String x
@@ -189,7 +189,7 @@ module Serialization =
189189
items
190190
|> Array.map toJsonValue
191191
|> JsonValue.Array
192-
| OptionValue (Some x) -> toJsonValue x
192+
| OptionValue (ValueSome x) -> toJsonValue x
193193
| EnumValue x -> JsonValue.String x
194194
| _ ->
195195
let props = t.GetProperties(BindingFlags.Public ||| BindingFlags.Instance)

0 commit comments

Comments
 (0)