Add APQ support

[sungam3r]

fixes #548

@rose-a @Shane32 please review, especially TODO note about extensions type

Also I would like to note that design of GraphQL client from this repo allows to send arbitrary queries to the server since caller may build arbitrary strings, for example, ones with concatenated literals with different values

query { user(id: "1234567") { name } }

Server cache for APQ may be polluted. In my in-house GraphQL client I do not allow to pass arbitrary strings. Instead this client autogenerates query based on used arguments in autogenerated methods and places all values into variables so the query text itself is already constant value like

query q($userId: String!) { user(id: $userId) { name } }

Nevertheless, I'm fine to add general possibility to use APQ here.

[Shane32]

please review, especially TODO note about extensions type

I use my own GraphQL client implementation, and it is based on generics; I wouldn't have a dictionary myself. Although the serializer would handle a dictionary if it was passed it. The same may be true here (?)

[sungam3r]

Problem with extensions is in design. What if someone set request.Extensions = new Person() before passing request into client? How to deal with arbitrary initialized object? How to add new data into it? Ping @rose-a , please comment.

[rose-a]

@Shane32 how would you handle a generic extensions type when a user want's to use his own extension in combination with the integrated APQ feature?

I think making request.Extension a Dictionary<string, object?> would be sensible in that it would provide a relatively safe way of providing integrated functionality like APQ and allowing the user to add other extensions without bothering too much with the integrated stuff (of course you can still provoke conflicts, but the chances are a lot smaller...). But there will propably be someone who's just sending a string via the extensions "object" right now who will complain anyway 😉 .

It also fits the GraphQLOverHTTP Spec Draft where its specified as a map...

[sungam3r]

Then I suggest to open another PR changing extensions property to Dictionary<string, object?>. OK?

Also note that the problem is not only between built-in functionality (like APQ or something else) and user extensions. User may use graphql client in such a way that different modules may want to set their own extensions and conflict on shared object. Dictionary (map) allows easily eliminate conflict.

[Shane32]

@Shane32 how would you handle a generic extensions type when a user want's to use his own extension in combination with the integrated APQ feature?

I don't use this library, but for my library, I'd serialize the user's anonymous object to a JsonElement and append the APQ data.

Let's ask ChatGPT for a sample:

ChatGPT 4 request

Write a C# function with this signature:

Dictionary<string, object> AddAPQSupport(object extensions, int version, string sha256Hash)

The function should serialize 'extensions' to a JsonElement, expecting a map, and add two keys to the map, "version" and "sha256Hash". If extensions is null it should create a new map

ChatGPT 4 response

using System;
using System.Collections.Generic;
using System.Text.Json;

public class APQSupport
{
    public static Dictionary<string, object> AddAPQSupport(object extensions, int version, string sha256Hash)
    {
        Dictionary<string, object> resultMap;

        if (extensions == null)
        {
            resultMap = new Dictionary<string, object>();
        }
        else
        {
            try
            {
                string jsonString = JsonSerializer.Serialize(extensions);
                resultMap = JsonSerializer.Deserialize<Dictionary<string, object>>(jsonString);
            }
            catch (Exception)
            {
                throw new ArgumentException("Invalid 'extensions' input. It must be a serializable map.");
            }
        }

        resultMap["version"] = version;
        resultMap["sha256Hash"] = sha256Hash;

        return resultMap;
    }
}

This C# function creates a new map if the extensions parameter is null or serializes the input to a JsonElement and converts it to a Dictionary. Then, it adds "version" and "sha256Hash" keys to the map. If the input is not a serializable map, it throws an ArgumentException.

[Shane32]

Keep in mind that a dictionary works just as well to be fed into this as an anonymous type. Now in my own library, I also support pre-encoded JSON strings as inputs to extensions/variables, not just anonymous types. Just because we are having fun here, let's have ChatGPT add that feature, and also optimize it a bit:

Request 2

change it so that if a string is passed in to extensions, it is assumed to be json and deserialized

Request 3

utilize the SerializeToElement method so it does not need to be serialized to a string

Final ChatGPT 4 output

using System;
using System.Collections.Generic;
using System.Text.Json;

public class APQSupport
{
    public static Dictionary<string, object> AddAPQSupport(object extensions, int version, string sha256Hash)
    {
        Dictionary<string, object> resultMap;

        if (extensions == null)
        {
            resultMap = new Dictionary<string, object>();
        }
        else
        {
            try
            {
                JsonElement jsonElement;

                if (extensions is string jsonString)
                {
                    jsonElement = JsonDocument.Parse(jsonString).RootElement;
                }
                else
                {
                    jsonElement = JsonSerializer.SerializeToElement(extensions);
                }

                resultMap = JsonSerializer.Deserialize<Dictionary<string, object>>(jsonElement.GetRawText());
            }
            catch (Exception)
            {
                throw new ArgumentException("Invalid 'extensions' input. It must be a serializable map or a valid JSON string.");
            }
        }

        resultMap["version"] = version;
        resultMap["sha256Hash"] = sha256Hash;

        return resultMap;
    }
}

[Shane32]

It still doesn't deserialize directly from the JsonElement. Just deleting .GetRawText() would give me the answer I was looking for. That is a relatively new feature, not existing in .NET prior to version 6, so probably the ChatGPT AI doesn't know about it since it's knowledge cutoff is 2021 I think.

https://learn.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializer.deserialize?view=net-7.0#system-text-json-jsonserializer-deserialize-1(system-text-json-jsonelement-system-text-json-jsonserializeroptions)

[Shane32]

@rose-a If I remember right, this library supports multiple serializers, so while this answer works for me, may not be the best answer here. It's just an idea.

[rose-a]

mh... while this solution would propably work its really hacky in my opinion and it results in a Dictionary<string, object?> anyway.

So if someone puts something into the extensions object which is not serializable to a map it explodes at runtime instead of during or before compilation. (It propably would explode on the server side, anyway, at least with GraphQL.NET, as that uses an Inputs object which is a ReadOnlyDictionary<string, object?>)

[Shane32]

Yeah, no problem. Just FYI, deserializing to Dictionary<string, object> results in the child elements being JsonElement instances. So there is no unnecessary deserialization and reserialization of child nodes. And of course if APQ isn't used then it's not necessary.

[mikocot]

Problem with extensions is in design. What if someone set request.Extensions = new Person() before passing request into client? How to deal with arbitrary initialized object? How to add new data into it? Ping @rose-a , please comment.

other option is to make it a base type (with only persisted query as property), that one may inherit from, and add additional properties. It's not as flexible as anonymous objecta as many currently probably use but a bit cleaner than making it a dictionary. In any case that seems to be a breaking change in the API.

That would leave option to control the serialziation process and options and use whatever custom serialization (although we'd need to handle serialziation of those base properties for at least two default serializers)