From 7d2ed5928e38108aa8137b148fbfb2216b2b4a9e Mon Sep 17 00:00:00 2001 From: "Valber M. Silva de Souza" Date: Thu, 9 Feb 2023 20:45:16 +0100 Subject: [PATCH] incoporated MessageMapping into the RawMessageConverter I was originally thinking about having a dumbed down converter which was easy to test, but as a matter of fact it makes no sense to pass around an object which is not completely deserialized. The undeserialized payload is now deliberate, since it's now up to the client to define how to deserialize his own custom payload. The possibility to configure this deserialization (which will happen actually during the handling of a message like "connection_init" or "pong") will come next. --- src/Main/GraphQLWebsocketMiddleware.fs | 31 +++--- src/Main/Main.fsproj | 1 - src/Main/MessageMapping.fs | 114 --------------------- src/Main/Messages.fs | 2 +- src/Main/RawMessageConverter.fs | 128 ++++++++++++++++++++++-- tests/unit-tests/InvalidMessageTests.fs | 17 +--- tests/unit-tests/SerializationTests.fs | 60 ++++------- 7 files changed, 161 insertions(+), 192 deletions(-) delete mode 100644 src/Main/MessageMapping.fs diff --git a/src/Main/GraphQLWebsocketMiddleware.fs b/src/Main/GraphQLWebsocketMiddleware.fs index 21a6eaa..6736834 100644 --- a/src/Main/GraphQLWebsocketMiddleware.fs +++ b/src/Main/GraphQLWebsocketMiddleware.fs @@ -83,11 +83,20 @@ type GraphQLWebSocketMiddleware<'Root>(next : RequestDelegate, applicationLifeti return JsonSerializer.Serialize(raw, jsonSerializerOptions) } - let deserializeClientMessage (serializerOptions : JsonSerializerOptions) (executor : Executor<'Root>) (msg: string) = + let deserializeClientMessage (serializerOptions : JsonSerializerOptions) (msg: string) = task { - return - JsonSerializer.Deserialize(msg, serializerOptions) - |> MessageMapping.toClientMessage serializerOptions executor + try + return + JsonSerializer.Deserialize(msg, serializerOptions) + |> succeed + with + | :? InvalidMessageException as e -> + return + fail <| InvalidMessage(4400, e.ToString()) + | :? JsonException as e -> + printfn "%s" (e.ToString()) + return + fail <| InvalidMessage(4400, "invalid json in client message") } let isSocketOpen (theSocket : WebSocket) = @@ -122,14 +131,10 @@ type GraphQLWebSocketMiddleware<'Root>(next : RequestDelegate, applicationLifeti if String.IsNullOrWhiteSpace message then return None else - try - let! result = - message - |> deserializeClientMessage serializerOptions executor - return Some result - with :? JsonException as e -> - printfn "%s" (e.ToString()) - return Some (MessageMapping.invalidMsg <| "invalid json in client message") + let! result = + message + |> deserializeClientMessage serializerOptions + return Some result } let sendMessageViaSocket (jsonSerializerOptions) (socket : WebSocket) (message : ServerMessage) = @@ -359,7 +364,7 @@ type GraphQLWebSocketMiddleware<'Root>(next : RequestDelegate, applicationLifeti task { let jsonOptions = new JsonOptions() jsonOptions.SerializerOptions.PropertyNameCaseInsensitive <- true - jsonOptions.SerializerOptions.Converters.Add(new RawMessageConverter()) + jsonOptions.SerializerOptions.Converters.Add(new ClientMessageConverter<'Root>(options.SchemaExecutor)) jsonOptions.SerializerOptions.Converters.Add(new RawServerMessageConverter()) let serializerOptions = jsonOptions.SerializerOptions if false && not (ctx.Request.Path = PathString (options.EndpointUrl)) then diff --git a/src/Main/Main.fsproj b/src/Main/Main.fsproj index cd79b8a..35b56eb 100644 --- a/src/Main/Main.fsproj +++ b/src/Main/Main.fsproj @@ -14,7 +14,6 @@ - diff --git a/src/Main/MessageMapping.fs b/src/Main/MessageMapping.fs deleted file mode 100644 index a4d7637..0000000 --- a/src/Main/MessageMapping.fs +++ /dev/null @@ -1,114 +0,0 @@ -namespace GraphQLTransportWS - -module MessageMapping = - open FSharp.Data.GraphQL - open Rop - open System.Text.Json - - /// From the spec: "Receiving a message of a type or format which is not specified in this document will result in an immediate socket closure with the event 4400: <error-message>. - /// The <error-message> can be vaguely descriptive on why the received message is invalid." - let invalidMsg (explanation : string) = - InvalidMessage (4400, explanation) - |> fail - - let private requireId (raw : RawMessage) : RopResult = - match raw.Id with - | Some s -> succeed s - | None -> invalidMsg <| "property \"id\" is required for this message but was not present." - - let private resolveVariables (serializerOptions : JsonSerializerOptions) (expectedVariables : Types.VarDef list) (variableValuesObj : JsonDocument) = - try - if (not (variableValuesObj.RootElement.ValueKind.Equals(JsonValueKind.Object))) then - let offendingValueKind = variableValuesObj.RootElement.ValueKind - fail (sprintf "\"variables\" must be an object, but here it is \"%A\" instead" offendingValueKind) - else - let providedVariableValues = variableValuesObj.RootElement.EnumerateObject() |> List.ofSeq - expectedVariables - |> List.choose - (fun expectedVariable -> - providedVariableValues - |> List.tryFind(fun x -> x.Name = expectedVariable.Name) - |> Option.map - (fun providedValue -> - let boxedValue = - if providedValue.Value.ValueKind.Equals(JsonValueKind.Null) then - null :> obj - elif providedValue.Value.ValueKind.Equals(JsonValueKind.String) then - providedValue.Value.GetString() :> obj - else - JsonSerializer.Deserialize(providedValue.Value, serializerOptions) :> obj - (expectedVariable.Name, boxedValue) - ) - ) - |> succeed - finally - variableValuesObj.Dispose() - - let decodeGraphQLQuery (serializerOptions : JsonSerializerOptions) (executor : Executor<'a>) (operationName : string option) (variables : JsonDocument option) (query : string) = - let executionPlan = - match operationName with - | Some operationName -> - executor.CreateExecutionPlan(query, operationName = operationName) - | None -> - executor.CreateExecutionPlan(query) - let variablesResult : RopResult, ClientMessageProtocolFailure> = - match variables with - | None -> succeed <| Map.empty // it's none of our business here if some variables are expected. If that's the case, execution of the ExecutionPlan will take care of that later (and issue an error). - | Some variableValuesObj -> - variableValuesObj - |> resolveVariables serializerOptions executionPlan.Variables - |> mapMessagesR (fun errMsg -> InvalidMessage (CustomWebSocketStatus.invalidMessage, errMsg)) - |> mapR Map.ofList - variablesResult - |> mapR (fun variables -> - { ExecutionPlan = executionPlan - Variables = variables }) - - let private requireSubscribePayload (serializerOptions : JsonSerializerOptions) (executor : Executor<'a>) (payload : JsonDocument option) : RopResult = - match payload with - | None -> - invalidMsg <| "payload is required for this message, but none was present." - | Some p -> - let rawSubsPayload = JsonSerializer.Deserialize(p, serializerOptions) - match rawSubsPayload with - | None -> - invalidMsg <| "payload is required for this message, but none was present." - | Some subscribePayload -> - match subscribePayload.Query with - | None -> - invalidMsg <| sprintf "there was no query in the client's subscribe message!" - | Some query -> - query - |> decodeGraphQLQuery serializerOptions executor subscribePayload.OperationName subscribePayload.Variables - - - let toClientMessage (serializerOptions : JsonSerializerOptions) (executor : Executor<'a>) (raw : RawMessage) : RopResult = - match raw.Type with - | None -> - invalidMsg <| sprintf "message type was not specified by client." - | Some "connection_init" -> - ConnectionInit raw.Payload - |> succeed - | Some "ping" -> - ClientPing raw.Payload - |> succeed - | Some "pong" -> - ClientPong raw.Payload - |> succeed - | Some "complete" -> - raw - |> requireId - |> mapR ClientComplete - | Some "subscribe" -> - raw - |> requireId - |> bindR - (fun id -> - raw.Payload - |> requireSubscribePayload serializerOptions executor - |> mapR (fun payload -> (id, payload)) - ) - |> mapR Subscribe - | Some other -> - invalidMsg <| sprintf "invalid type \"%s\" specified by client." other - diff --git a/src/Main/Messages.fs b/src/Main/Messages.fs index 6be9b00..62c1a28 100644 --- a/src/Main/Messages.fs +++ b/src/Main/Messages.fs @@ -13,7 +13,7 @@ type RawSubscribePayload = type RawMessage = { Id : string option - Type : string option + Type : string Payload : JsonDocument option } type ServerRawPayload = diff --git a/src/Main/RawMessageConverter.fs b/src/Main/RawMessageConverter.fs index 8a662a5..db70618 100644 --- a/src/Main/RawMessageConverter.fs +++ b/src/Main/RawMessageConverter.fs @@ -1,16 +1,31 @@ namespace GraphQLTransportWS +open FSharp.Data.GraphQL +open Rop open System open System.Text.Json open System.Text.Json.Serialization [] -type RawMessageConverter() = - inherit JsonConverter() +type ClientMessageConverter<'Root>(executor : Executor<'Root>) = + inherit JsonConverter() let raiseInvalidMsg explanation = raise <| InvalidMessageException explanation + /// From the spec: "Receiving a message of a type or format which is not specified in this document will result in an immediate socket closure with the event 4400: <error-message>. + /// The <error-message> can be vaguely descriptive on why the received message is invalid." + let invalidMsg (explanation : string) = + InvalidMessage (4400, explanation) + |> fail + + let unpackRopResult ropResult = + match ropResult with + | Success (x, _) -> x + | Failure (failures : ClientMessageProtocolFailure list) -> + System.String.Join("\n\n", (failures |> Seq.map (fun (InvalidMessage (_, explanation)) -> explanation))) + |> raiseInvalidMsg + let getOptionalString (reader : byref) = if reader.TokenType.Equals(JsonTokenType.Null) then None @@ -23,7 +38,78 @@ type RawMessageConverter() = else raiseInvalidMsg <| sprintf "was expecting a value for property \"%s\"" propertyName - override __.Read(reader : byref, typeToConvert: Type, options: JsonSerializerOptions) : RawMessage = + let requireId (raw : RawMessage) : RopResult = + match raw.Id with + | Some s -> succeed s + | None -> invalidMsg <| "property \"id\" is required for this message but was not present." + + let resolveVariables (serializerOptions : JsonSerializerOptions) (expectedVariables : Types.VarDef list) (variableValuesObj : JsonDocument) = + try + if (not (variableValuesObj.RootElement.ValueKind.Equals(JsonValueKind.Object))) then + let offendingValueKind = variableValuesObj.RootElement.ValueKind + fail (sprintf "\"variables\" must be an object, but here it is \"%A\" instead" offendingValueKind) + else + let providedVariableValues = variableValuesObj.RootElement.EnumerateObject() |> List.ofSeq + expectedVariables + |> List.choose + (fun expectedVariable -> + providedVariableValues + |> List.tryFind(fun x -> x.Name = expectedVariable.Name) + |> Option.map + (fun providedValue -> + let boxedValue = + if providedValue.Value.ValueKind.Equals(JsonValueKind.Null) then + null :> obj + elif providedValue.Value.ValueKind.Equals(JsonValueKind.String) then + providedValue.Value.GetString() :> obj + else + JsonSerializer.Deserialize(providedValue.Value, serializerOptions) :> obj + (expectedVariable.Name, boxedValue) + ) + ) + |> succeed + finally + variableValuesObj.Dispose() + + let decodeGraphQLQuery (serializerOptions : JsonSerializerOptions) (executor : Executor<'a>) (operationName : string option) (variables : JsonDocument option) (query : string) = + let executionPlan = + match operationName with + | Some operationName -> + executor.CreateExecutionPlan(query, operationName = operationName) + | None -> + executor.CreateExecutionPlan(query) + let variablesResult : RopResult, ClientMessageProtocolFailure> = + match variables with + | None -> succeed <| Map.empty // it's none of our business here if some variables are expected. If that's the case, execution of the ExecutionPlan will take care of that later (and issue an error). + | Some variableValuesObj -> + variableValuesObj + |> resolveVariables serializerOptions executionPlan.Variables + |> mapMessagesR (fun errMsg -> InvalidMessage (CustomWebSocketStatus.invalidMessage, errMsg)) + |> mapR Map.ofList + variablesResult + |> mapR (fun variables -> + { ExecutionPlan = executionPlan + Variables = variables }) + + let requireSubscribePayload (serializerOptions : JsonSerializerOptions) (executor : Executor<'a>) (payload : JsonDocument option) : RopResult = + match payload with + | None -> + invalidMsg <| "payload is required for this message, but none was present." + | Some p -> + let rawSubsPayload = JsonSerializer.Deserialize(p, serializerOptions) + match rawSubsPayload with + | None -> + invalidMsg <| "payload is required for this message, but none was present." + | Some subscribePayload -> + match subscribePayload.Query with + | None -> + invalidMsg <| sprintf "there was no query in the client's subscribe message!" + | Some query -> + query + |> decodeGraphQLQuery serializerOptions executor subscribePayload.OperationName subscribePayload.Variables + + + let readRawMessage (reader : byref, options: JsonSerializerOptions) : RawMessage = if not (reader.TokenType.Equals(JsonTokenType.StartObject)) then raise (new JsonException((sprintf "reader's first token was not \"%A\", but \"%A\"" JsonTokenType.StartObject reader.TokenType))) else @@ -44,12 +130,42 @@ type RawMessageConverter() = match theType with | None -> raiseInvalidMsg "property \"type\" is missing" - | Some _ -> + | Some msgType -> { Id = id - Type = theType + Type = msgType Payload = payload } - override __.Write(writer : Utf8JsonWriter, value : RawMessage, options : JsonSerializerOptions) = + override __.Read(reader : byref, typeToConvert: Type, options: JsonSerializerOptions) : ClientMessage = + let raw = readRawMessage(&reader, options) + match raw.Type with + | "connection_init" -> + ConnectionInit raw.Payload + | "ping" -> + ClientPing raw.Payload + | "pong" -> + ClientPong raw.Payload + | "complete" -> + raw + |> requireId + |> mapR ClientComplete + |> unpackRopResult + | "subscribe" -> + raw + |> requireId + |> bindR + (fun id -> + raw.Payload + |> requireSubscribePayload options executor + |> mapR (fun payload -> (id, payload)) + ) + |> mapR Subscribe + |> unpackRopResult + | other -> + raiseInvalidMsg <| sprintf "invalid type \"%s\" specified by client." other + + + + override __.Write(writer : Utf8JsonWriter, value : ClientMessage, options : JsonSerializerOptions) = failwith "serializing a WebSocketClientMessage is not supported (yet(?))" [] diff --git a/tests/unit-tests/InvalidMessageTests.fs b/tests/unit-tests/InvalidMessageTests.fs index 668976b..8a25a72 100644 --- a/tests/unit-tests/InvalidMessageTests.fs +++ b/tests/unit-tests/InvalidMessageTests.fs @@ -1,34 +1,23 @@ module InvalidMessageTests -open GraphQLTransportWS.Rop open UnitTest open GraphQLTransportWS -open System open System.Text.Json open Xunit -open FSharp.Data.GraphQL.Ast let toClientMessage (theInput : string) = let serializerOptions = new JsonSerializerOptions() serializerOptions.PropertyNameCaseInsensitive <- true - serializerOptions.Converters.Add(new RawMessageConverter()) + serializerOptions.Converters.Add(new ClientMessageConverter(TestSchema.executor)) serializerOptions.Converters.Add(new RawServerMessageConverter()) - JsonSerializer.Deserialize(theInput, serializerOptions) - |> MessageMapping.toClientMessage serializerOptions TestSchema.executor + JsonSerializer.Deserialize(theInput, serializerOptions) let willResultInInvalidMessage expectedExplanation input = try let result = input |> toClientMessage - match result with - | Failure msgs -> - match msgs |> List.head with - | InvalidMessage (code, explanation) -> - Assert.Equal(4400, code) - Assert.Equal(expectedExplanation, explanation) - | other -> - Assert.Fail(sprintf "unexpected actual value: '%A'" other) + Assert.Fail(sprintf "should have failed, but succeeded with result: '%A'" result) with | :? JsonException as ex -> Assert.Equal(expectedExplanation, ex.Message) diff --git a/tests/unit-tests/SerializationTests.fs b/tests/unit-tests/SerializationTests.fs index 70cacfc..395726e 100644 --- a/tests/unit-tests/SerializationTests.fs +++ b/tests/unit-tests/SerializationTests.fs @@ -1,9 +1,7 @@ module SerializationTests -open GraphQLTransportWS.Rop open UnitTest open GraphQLTransportWS -open System open System.Text.Json open Xunit open FSharp.Data.GraphQL.Ast @@ -11,7 +9,7 @@ open FSharp.Data.GraphQL.Ast let getStdSerializerOptions () = let serializerOptions = new JsonSerializerOptions() serializerOptions.PropertyNameCaseInsensitive <- true - serializerOptions.Converters.Add(new RawMessageConverter()) + serializerOptions.Converters.Add(new ClientMessageConverter(TestSchema.executor)) serializerOptions.Converters.Add(new RawServerMessageConverter()) serializerOptions @@ -20,13 +18,10 @@ let ``Deserializes ConnectionInit correctly`` () = let serializerOptions = getStdSerializerOptions() let input = "{\"type\":\"connection_init\"}" - let resultRaw = JsonSerializer.Deserialize(input, serializerOptions) - let result = - resultRaw - |> MessageMapping.toClientMessage serializerOptions (TestSchema.executor) + let result = JsonSerializer.Deserialize(input, serializerOptions) match result with - | Success (ConnectionInit None, _) -> () // <-- expected + | ConnectionInit None -> () // <-- expected | other -> Assert.Fail(sprintf "unexpected actual value: '%A'" other) @@ -36,13 +31,10 @@ let ``Deserializes ConnectionInit with payload correctly`` () = let input = "{\"type\":\"connection_init\", \"payload\":\"hello\"}" - let resultRaw = JsonSerializer.Deserialize(input, serializerOptions) - let result = - resultRaw - |> MessageMapping.toClientMessage serializerOptions (TestSchema.executor) + let result = JsonSerializer.Deserialize(input, serializerOptions) match result with - | Success (ConnectionInit _, _) -> () // <-- expected + | ConnectionInit _ -> () // <-- expected | other -> Assert.Fail(sprintf "unexpected actual value: '%A'" other) @@ -52,13 +44,10 @@ let ``Deserializes ClientPing correctly`` () = let input = "{\"type\":\"ping\"}" - let resultRaw = JsonSerializer.Deserialize(input, serializerOptions) - let result = - resultRaw - |> MessageMapping.toClientMessage serializerOptions (TestSchema.executor) + let result = JsonSerializer.Deserialize(input, serializerOptions) match result with - | Success (ClientPing None, _) -> () // <-- expected + | ClientPing None -> () // <-- expected | other -> Assert.Fail(sprintf "unexpected actual value '%A'" other) @@ -68,13 +57,10 @@ let ``Deserializes ClientPing with payload correctly`` () = let input = "{\"type\":\"ping\", \"payload\":\"ping!\"}" - let resultRaw = JsonSerializer.Deserialize(input, serializerOptions) - let result = - resultRaw - |> MessageMapping.toClientMessage serializerOptions (TestSchema.executor) + let result = JsonSerializer.Deserialize(input, serializerOptions) match result with - | Success (ClientPing _, _) -> () // <-- expected + | ClientPing _ -> () // <-- expected | other -> Assert.Fail(sprintf "unexpected actual value '%A" other) @@ -84,13 +70,10 @@ let ``Deserializes ClientPong correctly`` () = let input = "{\"type\":\"pong\"}" - let resultRaw = JsonSerializer.Deserialize(input, serializerOptions) - let result = - resultRaw - |> MessageMapping.toClientMessage serializerOptions (TestSchema.executor) + let result = JsonSerializer.Deserialize(input, serializerOptions) match result with - | Success (ClientPong None, _) -> () // <-- expected + | ClientPong None -> () // <-- expected | other -> Assert.Fail(sprintf "unexpected actual value: '%A'" other) @@ -100,13 +83,10 @@ let ``Deserializes ClientPong with payload correctly`` () = let input = "{\"type\":\"pong\", \"payload\": \"pong!\"}" - let resultRaw = JsonSerializer.Deserialize(input, serializerOptions) - let result = - resultRaw - |> MessageMapping.toClientMessage serializerOptions (TestSchema.executor) + let result = JsonSerializer.Deserialize(input, serializerOptions) match result with - | Success (ClientPong _, _) -> () // <-- expected + | ClientPong _ -> () // <-- expected | other -> Assert.Fail(sprintf "unexpected actual value: '%A'" other) @@ -116,13 +96,10 @@ let ``Deserializes ClientComplete correctly``() = let input = "{\"id\": \"65fca2b5-f149-4a70-a055-5123dea4628f\", \"type\":\"complete\"}" - let resultRaw = JsonSerializer.Deserialize(input, serializerOptions) - let result = - resultRaw - |> MessageMapping.toClientMessage serializerOptions (TestSchema.executor) + let result = JsonSerializer.Deserialize(input, serializerOptions) match result with - | Success (ClientComplete id, _) -> + | ClientComplete id -> Assert.Equal("65fca2b5-f149-4a70-a055-5123dea4628f", id) | other -> Assert.Fail(sprintf "unexpected actual value: '%A'" other) @@ -141,13 +118,10 @@ let ``Deserializes client subscription correctly`` () = } """ - let resultRaw = JsonSerializer.Deserialize(input, serializerOptions) - let result = - resultRaw - |> MessageMapping.toClientMessage serializerOptions (TestSchema.executor) + let result = JsonSerializer.Deserialize(input, serializerOptions) match result with - | Success (Subscribe (id, payload), _) -> + | Subscribe (id, payload) -> Assert.Equal("b5d4d2ff-d262-4882-a7b9-d6aec5e4faa6", id) Assert.Equal(1, payload.ExecutionPlan.Operation.SelectionSet.Length) let watchMoonSelection = payload.ExecutionPlan.Operation.SelectionSet |> List.head