firely-net-sdk icon indicating copy to clipboard operation
firely-net-sdk copied to clipboard

Better integration with Json.Net serialization

Open joshclark opened this issue 5 years ago • 6 comments

I have classes that contain embedded FHIR models.

class Foo 
{
    public MyComplexType Bar {get; set;}
    public Patient MyPatient {get; set;}
}

When I try to serialize this class to JSON it fails to do so correctly because Patient needs to use the FhirJsonSerializer. Good news! Json.Net provides a way for be to "teach" it how to convert a type to JSON. So I write this:

    public class FhirModelsJsonConverter : JsonConverter<Base>
    {
        public override void WriteJson(JsonWriter writer, Base value, JsonSerializer serializer)
        {
            var fhirJsonSerializer = new FhirJsonSerializer(new SerializerSettings { Pretty = true, AppendNewLine = true });
            fhirJsonSerializer.Serialize(value, writer);
        }

        public override Base ReadJson(JsonReader reader, Type objectType, Base existingValue, bool hasExistingValue, JsonSerializer serializer)
        {
            var defaultSettings = new ParserSettings { PermissiveParsing = true };
            var jsonParser = new FhirJsonParser(defaultSettings);
            return jsonParser.Parse(reader);
        }
    }

Now all I have to do is tell Json.Net to use this converter when it does its thing.

var settings = new JsonSerializerSettings { Formatting = Formatting.Indented };
settings.Converters.Add(new FhirModelsJsonConverter());
var jsonSerializer = JsonSerializer.CreateDefault(settings);

StringBuilder sb = new StringBuilder(256);
StringWriter sw = new StringWriter(sb, CultureInfo.InvariantCulture);
using (JsonTextWriter jsonWriter = new JsonTextWriter(sw))
{
  jsonWriter.Formatting = serializer.Formatting;
  serializer.Serialize(jsonWriter, obj);
 };

var json = sw.ToString();

This works for this instance of the serializer, but now I need to make sure every library that wants to use Json.Net get configured correctly. ASP.NET, for example, has its own copy of JsonSerializerSettings that it uses.

(There is JsonConvert.DefaultSettings which starts to handle this but it is global and I'm in a library so changing the global settings in someone else's process is less than ideal).

TL;DR?

If this library added the JsonConverterAttribute to the definition of Base (https://github.com/FirelyTeam/fhir-net-api/blob/master/src/Hl7.Fhir.Core/Model/Base.cs#L48) I think it would allow users to just Serialize (and Deserialize) with Json.Net like so:

var json = JsonConvert.SerializeObject(myFoo);

Thoughts? Is this just not something that's been thought about or is there a reason why this would not "fit" with the direction of this library?

joshclark avatar Dec 31 '19 18:12 joshclark

Hi @joshclark, I am not a contributor to this project but I did run in the same trouble that you had. Like you said, the JsonConverterAttribute is a convenient way to configure your NewtonsoftJson and not the only way.

In my scenario (C# .Net Core 3.0), I created a similar FhirModelsJsonConverter and added that in Startup.cs:

public void ConfigureServices(IServiceCollection services)
{
  services.AddControllers().AddNewtonsoftJson(
    options => options.SerializerSettings.Converters.Add(new FhirModelsJsonConverter()));
}

In my controllers, I can now rely on NewtonsoftJson functionality:

public IActionResult MyControllerFunction([FromBody] Base resource)
{
   // do stuff here
}

If I were to use this in many different libraries (like your scenario), why not extract the configuration of NewtonsoftJson as an extension and publish it as a package? In every library, add a one-liner that executes the extension.

While it is not an answer whether the JsonConvertAttribute should be added, it might also help others that are looking to integrate it. I would not know what would happen if the attribute is set, and the user would not want to use NewtonsoftJson in their own library.

mjkemna avatar Feb 27 '20 11:02 mjkemna

FHIR JSON needs to be serialised in a special manner - see the rules in https://www.hl7.org/fhir/json.html. Are you sure this library would know how to do it?

vadi2 avatar Feb 27 '20 11:02 vadi2

@vadi2 the fhir-net-api library even uses NewtonsoftJson under the hood. What @joshclark and I are doing is configuring the NewtonsoftJson instance inside our own projects to use the parser and serializer from the fhir-net-api that adheres to the rules you have listed. The suggestion of @joshclark to add that attribute would ensure that it is automatically configured.

mjkemna avatar Feb 27 '20 12:02 mjkemna

Using your controller like this also means it is json only, and doesn't handle xml too. There are open source implementations of the handlers for that too.

brianpos avatar Aug 10 '20 08:08 brianpos

We've been adding [Serializable] and [DataContract] to help with (binary) .NET serialization - I don't see a problem in adding yet one more tag to Base to make this possible.

Well, there might be one thing: this creates a dependency from the data classes to the json library. forcing every project (even those not using newtonsoft) to have this dependency :-(

ewoutkramer avatar Sep 10 '20 15:09 ewoutkramer

Well, there might be one thing: this creates a dependency from the data classes to the json library. forcing every project (even those not using newtonsoft) to have this dependency :-(

Doesn't Hl7.Fhir.Core reference Hl7.Fhir.Serialization which depends on Newtonsoft.Json? So aren't all those projects getting that dependency anyway?

joshclark avatar Sep 10 '20 18:09 joshclark

We're actually concentrating our effort on System.Text.Json nowadays, so I don't want to do this.

ewoutkramer avatar Jul 19 '23 13:07 ewoutkramer