d-exclaimation / pioneer

GraphQL for Swift.
https://pioneer.dexclaimation.com
Apache License 2.0
39 stars 8 forks source link

JSON Incorrectly Encoded #85

Closed Sherlouk closed 2 years ago

Sherlouk commented 2 years ago

Describe the bug

As per the GraphQL specification the order of the returned data keys should match that of those provided in the query. Currently, it's randomised.

From looking into the code this appears to be caused by this line:

try res.content.encode(result)

On line 73 of the Pioneer+Http file. By default, this line will use the default JSONEncoder which is incorrect. We should be using the GraphQLJSONEncoder which is provided by the underlying GraphQL library.

It would also be nice if you gave us the ability to customise this encoder when we initialise the Pioneer struct.

Confirmed on v0.9.4

d-exclaimation commented 2 years ago

Yep, this is a bug that I missed. I'll have the fix in #84.

d-exclaimation commented 2 years ago

I'll see what I can do with using any other encoder since I also need to make sure that encoder is used for WebSocket operations as well

d-exclaimation commented 2 years ago

v0.10.0 should fix this. For providing custom encoder, I added it through the httpHandler method instead of having when initialising Pioneer struct, which meant to use a custom encoder, you'll have to opt out of applyMiddleware and perform manual routing.

I think I can add custom encoder to applyMiddleware as well, but I have a lot of things to do as of now. I'll get back to this later on.

Sherlouk commented 2 years ago

So does look like the results are being sorted as expected which is awesome, but I'm noticing that when updating the dateEncodingStrategy that the changes aren't being used.

            let encoder = GraphQLJSONEncoder()
            encoder.dateEncodingStrategy = .iso8601

            builder.post { req in
                try await pioneer.httpHandler(req: req, using: encoder)
            }

Output: "date": 685103355.215434,

d-exclaimation commented 2 years ago

That's quite odd. I checked that httpHandler did use the given encoder and that I have made GraphQLJSONEncoder conforms to ContentEncoder properly (i.e. manually encoding as ContentEncoder did work).

What I can found so far is that before any GraphQLResult were being encoded into JSON, any dates had turned in Map.number after the graphql execution. I am not too sure where exactly the problem is though.

d-exclaimation commented 2 years ago

Alright, I think I figured it out.

By default, if you are using Graphiti and when you are creating the schema object, you are using the default Coders which dictates how any of the Scalar going to be encoded to the Map enum using the MapEncoder, which defaults it date encoding to .deferredToDate

Try changing that to use a custom Coders and see if that fixes the issue

let coders = Coders()
coders.encoder.dateEncodingStrategy = .iso8601
try Schema<Resolver, Context>(coders: coders) {
    // your schema here
}
Sherlouk commented 2 years ago

Thanks! Can confirm the above solution does work 👍