Consider supporting read-only or write-only JsonConverters

[TannerBrunscheon]

Description

System.Text.Json
I have the use case where I need to customize the write of a JsonConverter on certain POCOs. To accomplish this, I am using the JsonConverterFactory to make a generic JsonConverter that overwrites the write function. The issue I am running into is on the read overwrite. Since this converter is generic, it is being used for the reading and writing of the POCO. All the code I have seen is throwing a NotImplementedException on read, which bubbles up in my code. Other things I have tried is using JsonConverter. Deserialize to try to call the default converters read but that seems to reference the same read that is running causing a recursive loop.

Configuration

.Net 5

Code

I am using it as a decorator of a class.

[JsonConverter(typeof(JsonPropertyConverter))]
	public class Model

This doesn't work

private class JsonConverterInner<T> : JsonConverter<T>
		{
			JsonSerializerOptions _options;

			public JsonPropertyOrderConverterInner(JsonSerializerOptions options)
			{
				_options = options;
			}

			public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) 
			{
				throw new NotImplementedException("Don't use me to read JSON");
			}
			public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
			{
(value edits here)
				JsonSerializer.Serialize(writer, value, options);
			}
		}

This also doesnt work

public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
    JsonSerializer.Deserialize<T>(ref reader, options)

[TannerBrunscheon]

Does anyone have any idea on how to do this?

[layomia]

Are you describing the need for a feature where a converter could indicate that only one out of the Read and Write method should be called when (de)serializing? For example, CanRead and CanWrite properties from Newtosonft.Json?

---

Other things I have tried is using JsonConverter. Deserialize to try to call the default converters read but that seems to reference the same read that is running causing a recursive loop.

On the recursive loop/stack overflow due to calling back into the serializer from a custom converter, please see this doc example.

[layomia]

Linking #36785 since it addresses extensible converters.

[TannerBrunscheon]

Are you describing the need for a feature where a converter could indicate that only one out of the Read and Write method should be called when (de)serializing? For example, CanRead and CanWrite properties from Newtosonft.Json?

Yeah basically I need a write only converter

On the recursive loop/stack overflow due to calling back into the serializer from a custom converter, please see this doc example.

I tried doing something like

public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
    JsonSerializer.Deserialize<T>(ref reader)

as the docs suggest and it got caught in the loop again.

[layomia]

Yeah basically I need a write only converter

Thanks.

Noting this as a reasonable feature request. It requires extensive design, particularly around if two different converters can handle the same type (i.e one handles read, and the other handles write).

[layomia]

I tried doing something like

public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
JsonSerializer.Deserialize(ref reader)
as the docs suggest and it got caught in the loop again.

@TannerBrunscheon - can you please provide a full repro, showing the call to the serializer & all types in the object graph? I'm trying to understand if there's a bug here, which would likely need a separate issue to address.

[TannerBrunscheon]

TextJSONRepro.zip

@layomia One thing I noticed when creating this repro is that when taking in a value, if you call Deserialize or Serialize on the same value that you took in, you get the same stack overflow error. I don't know why you would ever do this except in cases of read only/write only converters and thus it would be solved by adding in read only/write only converters.

[layomia]

@TannerBrunscheon - the workaround for the stack-overflow is to make sure that the converter that the serializer picks for the type you are passing is not the same converter that you are in. This might involve not passing the same options instance that was passed to the converter. This approach and others are documented over here: https://docs.microsoft.com/dotnet/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to?pivots=dotnet-5-0#required-properties.

---

For the feature to have a read or write-only converter, we can consider that for the future.

[eiriktsarpalis]

For the feature to have a read or write-only converter, we can consider that for the future.

What would be the expected behavior of a read-only or write-only converter when we attempt to read or write, respectively? I'm guessing it would fall back to the default converter for the type? Or would it simply fail the operation?

[adam8797]

Any updates on this? This is quite a useful feature of Newtonsoft that is making transitioning to System.Text.Json a bit tricky in some aspects.

[eiriktsarpalis]

You can try using the following workaround to get read-only and write-only converters with default fallback semantics (assuming you're using the reflection serializer):

public abstract class ReadOnlyJsonConverter<T> : JsonConverter<T>
{
    private readonly JsonConverter<T> _fallbackConverter = (JsonConverter<T>)JsonSerializerOptions.Default.GetConverter(typeof(T));

    public sealed override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
        => _fallbackConverter.Write(writer, value, options);
}

public abstract class WriteOnlyJsonConverter<T> : JsonConverter<T>
{
    private readonly JsonConverter<T> _fallbackConverter = (JsonConverter<T>)JsonSerializerOptions.Default.GetConverter(typeof(T));

    public sealed override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        => _fallbackConverter.Read(ref reader, typeToConvert, options);
}

Then implement your read-only or write-only custom converter by deriving from the relevant class:

public class MyIntConverter : WriteOnlyJsonConverter<int>
{
    public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options)
        => writer.WriteStringValue(value.ToString(CultureInfo.InvariantCulture));
}

Any updates on this?

We have no plans on implementing built-in CanRead and CanWrite support to STJ's JsonConverter currently. The reason fundamentally is that it would be nontrivial to support fallback semantics for the case of the source generated serializer (which JsonConverter also needs to support).

[adam8797]

@eiriktsarpalis thank you for the snippets! I'll try these out.

[Xriuk]

For the feature to have a read or write-only converter, we can consider that for the future.

What would be the expected behavior of a read-only or write-only converter when we attempt to read or write, respectively? I'm guessing it would fall back to the default converter for the type? Or would it simply fail the operation?

Couldn't the converter simply throw NotSupportedException like said in this paragraph (and the following one)?
https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/converters-how-to?pivots=dotnet-8-0#notsupportedexception

When you want to disallow certain types, throw a NotSupportedException.

I'm guessing this is (or could be) supported in both Read/Write and then the serialized would look for the next converter in the list which should be the default one.

[Balkoth]

@eiriktsarpalis
Your snippets crash with a stackoverflow exception at initializing _fallbackConverter .

[eiriktsarpalis]

@Balkoth you should avoid specifying the custom converter via JsonConverterAttribute because it creates a circular dependency. Use JsonSerializerOptions.Converters instead.

[Balkoth]

But then there is no need to do it this way if you have to specify it at the location you use it. Because at this point i can just use one that does throw NotImplementedException as no reading is going on there.

To me this seems like basic functionality to call the default Read or Write methods in case you only need to modify one of them. And to have any positive gains it needs to be possible directly in the converter implementing JsonConverter<T>.

[eiriktsarpalis]

To me this seems like basic functionality to call the default Read or Write methods

It's simply not possible given the way that STJ is currently architected. Adding a JsonConverterAttribute annotation on a type sets the default converter for that type. There is this issue that proposes exposing the built-in converter types, but this would require specifying an explicit fallback for the type you are working with.

[Balkoth]

Then make it possible? What is holding you back?

This feels like Microsoft invented virtual methods but without __super to call the base class.

[eiriktsarpalis]

Then make it possible? What is holding you back?

Other priorities. We are currently working on issues flagged with the 9.0.0 milestone.

This feels like Microsoft invented virtual methods but without __super to call the base class.

Converters use a fundamentally different composition model, so it's the not the best analogy.

[Balkoth]

So this issue is open since Dec 2020 and googling for this problem finds lot of results without resolution, as there is none as you just confirmed. So when will this issue make it?

Not being able to call the default for Read and Write is basically a showstopper. Was this never brought up when JsonConverter<T> was designed?

[eiriktsarpalis]

as there is none as you just confirmed.

Like I said, using JsonSerializerOptions.Converters instead of JsonConverterAttribute provides the workaround that you require.

Was this never brought up when JsonConverter was designed?

No library or abstraction can cater to every requirement. You can use the suggested workaround, or you can wait until a solution in this space is implemented.

[julealgon]

@Xriuk

Couldn't the converter simply throw NotSupportedException like said in this paragraph (and the following one)? https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/converters-how-to?pivots=dotnet-8-0#notsupportedexception

When you want to disallow certain types, throw a NotSupportedException.

I'm guessing this is (or could be) supported in both Read/Write and then the serialized would look for the next converter in the list which should be the default one.

Exceptions should be used for exceptional behavior. Using NotSupportedException to skip a converter and move to the next one would be a bad use of exceptions (also known as "use exception as control flow") and would also be slow due to how slow exceptions are in general. It is not a good idea to ever introduce exceptions in these flows because they can happen very frequently under normal situations.

[adam8797]

I think @layomia had it right back in 2021 when this issue was first opened.

Are you describing the need for a feature where a converter could indicate that only one out of the Read and Write method should be called when (de)serializing? For example, CanRead and CanWrite properties from Newtosonft.Json?

Newtonsoft.Json seemed to inform much of the design of this library, and they solved this issue quite simply with CanRead and CanWrite properties. I think keeping in line with that would also help those who are porting over from Newtonsoft.

[julealgon]

@adam8797 why not just have 2 interfaces, one for reading and another for writing, and if you want to support both, you implement both?

That seems like a better design to me than having CanRead / CanWrite properties (sometimes coupled with throwing NotSupportedException like Stream does).

[adam8797]

Few reasons I think the CanRead/CanWrite way is the way the maintainers should pursue:

1. Compatibility. It wouldn't require a breaking change to the JsonConverter<T> type. New virtual get-only properties could be added which default to true, so that the behavior of existing converters remains the same.
2. We've been here before. To my previous point, Newtonsoft.Json did this already. That in and of itself doesn't mean its the best way, but we know it works.
3. Ease of adoption I've had to port a few projects now from using Newtonsoft.Json to System.Text.Json (the reason I'm interested in this issue) and I've had to port converters which used that CanRead/CanWrite properties. Lowering the friction to move to this library would (imho) help adoption. We still have a few projects which use Newtonsoft.Json on .net8 because we still need weird obscure features it supports, and this is one of them.
4. Performance I'll admit there may be a better way this could be done with pattern matching, but I think there's a good chance you'll run into reflection while trying to implement the interface approach, especially once generics get involved. A single virtual property call is going to be faster then reflection.

Bit shakier on that last bullet, because that may be totally solvable. However I think the first three points still stand, and provided enough of a reason to take the CanRead/CanWrite approach

[rjgotten]

Wouldn't even need CanRead / CanWrite explicitly. Just extend the contract for JsonConverterFactory to include an overloaded signature for CanConvert - e.g.

bool CanConvert(Type typeToConvert, SerializationDirection direction)

where

public enum SerializationDirection 
{
  Read,
  Write
}

[rjgotten]

@eiriktsarpalis
It's simply not possible given the way that STJ is currently architected. Adding a JsonConverterAttribute annotation on a type sets the default converter for that type. There is this issue that proposes exposing the built-in converter types, but this would require specifying an explicit fallback for the type you are working with.

It's actually entirely possible.
And I built it.

The lynchpin for it is in

JsonTypeInfo.CreateJsonTypeInfo(Type typeToConvert, JsonSerializerOptions options)

This overload skips checking the [JsonConverter] attributes. It uses only the custom converters from the JsonSerializerOptions .Converters collection and the built-in converters.

You can achieve a 'use-only-for-reading' or 'use-only-for-writing' adapter over a JsonConverter with it, which will fall through to the next eligible custom converter or built-in converter:

public sealed class ReadOnly<TInner> : JsonConverterFactory
  where TInner: JsonConverter, new()
  
public sealed class WriteOnly<TInner> : JsonConverterFactory
  where TInner: JsonConverter, new()

The converter factories here would create converters that delegate to instances of TInner (and flatten TInner if its a JsonConverterFactory itself) for reading or writing using the usual technique of casting the TInner converter to JsonConverter<TValue> and directly accessing its Read or Write method.

For the other branch, where TInner should not be used, they instead derive a cloned JsonSerializerOptions that adds another converter factory to the front of the list of converters, which will then match this type with highest priority. Because STJ gives precedence to global converters over attributes on types, it will pre-empt those as well and will be selected when performing a nested JsonSerializer.Serialize or .Deserialize call passing either the writer or reader and these modified options.

That new converter factory can then inside its CreateConverter create a third set of options, from which it removes itself again and which it then uses with the JsonTypeInfo.CreateJsonTypeInfo overload mentioned at the beginning to pull out the matching JsonConverter implementation from either the remaining custom converters, or the built-in converters.

Thus 'skipping' over any [JsonConverter(typeof(ReadOnly<>))] or [JsonConverter(typeof(WriteOnly<>))] attribute on the type itself, when writing or reading respectively, and falling through to whatever the 'next converter in line' would be.

---

...
Come to think of it - you could probably lift the JsonTypeInfo.CreateJsonTypeInfo usage into the first converter factory and explicitly establish the precise fallback converter there already, then pass separate read and write delegated converters into the wrapping converter the first factory spits out.

[EDIT]: Yes, indeed you totally could...