Skip to content

Add PolymorphicTypenameConverter class to handle __typename deserialization #343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using System;
using System.Text.Json;
using System.Text.Json.Serialization;

namespace GraphQL.Client.Serializer.SystemTextJson
{
public abstract class PolymorphicTypenameConverter<T> : JsonConverter<T>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can find a class name which makes the use case for this converter a little bit more obvious (in that it's tied to the builtin GraphQL __typename field).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FromTypenameConverter ?

Copy link
Collaborator

@rose-a rose-a Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GraphQLTypenameConverter? 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is already inside GraphQL.* namespace.

{
protected abstract Type GetTypeFromTypenameField(string typename);

public override bool CanConvert(Type typeToConvert) =>
typeof(T).IsAssignableFrom(typeToConvert);

public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
var clone = reader; // cause its a struct
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was not obvious for me too. But the Utf8JsonReader is a struct and many contains pointers to data to read. So to get the __typename we need to read ahead the data and then move back and deserialize the object. But the reader can only read forward. So we make use of the struct and clone it by assign it to another variable. So both clone and reader and read to differenc positions from each others.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Better to put a comment into sources.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By itself, this class without descendants carries little benefit. You can make at least one descendant, which discovers all the classes in the declaring assembly.


if (clone.TokenType == JsonTokenType.StartObject)
clone.Read();
if (clone.TokenType != JsonTokenType.PropertyName || clone.GetString() != "__typename")

throw new JsonException();

clone.Read();
var type = Descriminator(clone.GetString());
object deserialize = JsonSerializer.Deserialize(ref reader, type, options);
return (T)deserialize;
}

public override void Write(Utf8JsonWriter writer, T obj, JsonSerializerOptions options) =>
throw new NotSupportedException();
}
}