Deserialization Broken using Newtonsoft on 4.x

[Lewellync]

Describe the bug Since moving to Fhir.NET SDK 4.x, we have encountered errors deserializing certain objects (FhirDateTime, Encounter.ParticipantComponent, to name a few), and get the same error message:

Cannot deserialize readonly or fixed size dictionary

To Reproduce Steps to reproduce the behavior:

    [Test]
    public void SerializeDeserializeEncounterFhirJsonRequest_A08Min()
    {
      var fhirDateTime = new FhirDateTime("2014-09-02T12:22:37-04:00");

      var serializedDateTime = JsonConvert.SerializeObject(fhirDateTime, _jsonSettings);
      var deserializedDateTime = JsonConvert.DeserializeObject<FhirDateTime>(serializedDateTime, _jsonSettings);
    }

Expected behavior The deserialization succeeds

Screenshots If applicable, add screenshots to help explain your problem.

Version used:

• FHIR Version: STU3
• Version: 4.2.1
• .NET Framework 4.8
• Newtonsoft.Json 13.0.1

Additional context None of these issues occur in the version we were using prior (3.3.0)

Here is our converter we pass into the JsonSettings

 public class FhirConverter : JsonConverter
  {
    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
      var s = new FhirJsonSerializer();
      s.Serialize((Resource)value, writer);
    }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
      if (reader.TokenType == JsonToken.Null) return null;
      var s = new FhirJsonParser();
      return s.Parse(reader, objectType);
    }

    public override bool CanConvert(Type objectType)
    {
      return typeof(Resource).IsAssignableFrom(objectType);
    }
  }

    private readonly JsonSerializerSettings _jsonSettings = new JsonSerializerSettings
    {
      NullValueHandling = NullValueHandling.Ignore,
      Converters = new List<JsonConverter> { new FhirConverter(), new StringEnumConverter() },
    };

[marcovisserFurore]

Hi @Lewellync. Thanks for your feedback. I think the cause lies in the inheritance of our Pocos. For example, the poco (resource) Patient derives from Base and Base implements, since version 4.x, the interface IReadOnlyDictionary. We have done this to implement our new and faster Json deserializer/serializer based on System.Text.Json (see also the documentation here).

Perhaps you can use this functionality as well. Your unittest would be then:

        [Test]
        {
            var options = new JsonSerializerOptions().ForFhir(typeof(FhirDateTime).Assembly).Pretty();

            var fhirDateTime = new FhirDateTime("2014-09-02T12:22:37-04:00");
            var serializedDateTime = System.Text.Json.JsonSerializer.Serialize(fhirDateTime, options);
            var deserializedDateTime = System.Text.Json.JsonSerializer.Deserialize<FhirDateTime>(serializedDateTime, options);

            deserializedDateTime.Should().BeEquivalentTo(fhirDateTime);
        }

[marcovisserFurore]

On a side note: using a Newtonsoft Json serializer out of the box will not work to serialize and deserialize resources, because of some special rules in the serialization. For example Extensions are represented in Json like _birthDay. See also the documentation here. That's why we offer in our SDK specials Json and Xml serializers to overcome these special rules. For more information see our documentation site.

[Lewellync]

@marcovisserFurore

Unfortunately, the extension methods for System.Text.Json aren't available to us (we are locked into using .NET Framework 4.8, for the moment, and it looks like there are code guards around those extension methods preventing our use with the System.Text.Json nuget package), and I couldn't figure out how it would work without that extension method.

I didn't think we were using them out of the box, I thought that's what the FhirConverter class was doing (checking if the incoming class is Resource and then passing it to the FhirJsonSerializer/FhirJsonParser) Is there additional in-between work that needs to be done with Newtonsoft?

[oldefezziwig]

I created the following additional implementation of JsonConverter

  public class FhirModelBaseConverter : JsonConverter
  {
    public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
    {
      var s = new FhirJsonSerializer();
      s.Serialize((Base)value, writer);
    }

    public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer)
    {
      var s = new FhirJsonParser();
      return s.Parse(reader, objectType);
    }

    public override bool CanConvert(Type objectType)
    {
      var canConvert = typeof(Base).IsAssignableFrom(objectType);
      return canConvert;
    }
  }

and when I add it to the list of Converters as in the following it seems to resolve a lot of these issues (i.e. no longer gets the "Cannot deserialize readonly or fixed size dictionary" exception):

   private readonly JsonSerializerSettings _jsonSettings = new JsonSerializerSettings
    {
      NullValueHandling = NullValueHandling.Ignore,
      Converters = new List<JsonConverter> { new FhirConverter(), new StringEnumConverter() },
    };

Can anyone think of any issues with this approach

[marcovisserFurore]

If you are serializing/deserializing your own classes that contains Firely .NET SDK poco's, then the above approach is a good solution. See here an example:

    public class MyOwnClass
    {
        public int MyProperty { get; set; }
        public Resource FhirObject { get; set; }
    }

        [TestMethod]
        public void MyTestMethod()
        {
            // Create serializer settings
            var serializerSettings = new JsonSerializerSettings
            {
                ReferenceLoopHandling = ReferenceLoopHandling.Ignore,
                NullValueHandling = NullValueHandling.Ignore,
                Formatting = Formatting.Indented,
                ObjectCreationHandling = ObjectCreationHandling.Replace
            };

            serializerSettings.Converters.Add(new FhirConverter());
            serializerSettings.Converters.Add(new StringEnumConverter());

            var myClass = new MyOwnClass { MyProperty = 1, FhirObject = new Patient { Active = true } };

            var s = JsonConvert.SerializeObject(myClass, serializerSettings);

            // Deserialize a CodeableConcept
            var deserializedObject = JsonConvert.DeserializeObject<MyOwnClass>(s, serializerSettings);
        }

When you are purely serializing/deserializing Firely .NET SDK poco's then I would recommend the Firely SDK serializers/deserializer:

        [TestMethod]
        public void MyTestMethod1()
        {
            var patient = new Patient { Active = true };
            var serializer = new FhirJsonSerializer();
            var parser = new FhirJsonParser();
            var json = serializer.SerializeToString(patient);

            var deserializedPatient = parser.Parse<Patient>(json);

        }

[Lewellync]

Yes, we are often serializing/deserializing them in our own classes. I think I've isolated the issue to just BackboneElements. If the BackboneElement, say a ParticipantComponent, is inside a DomainResource, the FhirConverter has no issue working with it. If this BackboneElement is used on its own (and I haven't tested with all of them, so this is an assumption), the FhirConverter cannot handle deserialization.

The code below demonstrates that issue on the code versions I mentioned when opening this ticket. Is this intended behavior, or an unforeseen use-case?

using System;
using System.Collections.Generic;
using Hl7.Fhir.Model;
using Hl7.Fhir.Serialization;
using Newtonsoft.Json;
using NUnit.Framework;

namespace FirelyIssue2209
{
  [TestFixture]
  public class FirelyIssue2209
  {
    private class BackboneElementExample
    {
      public int MyProperty { get; set; }
      public Encounter.ParticipantComponent ParticipantComponent { get; set; }
    }

    private class DomainResourceExample
    {
      public int MyProperty { get; set; }
      public Encounter Encounter { get; set; }
    }

    private class FhirConverter : JsonConverter
    {
      public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
      {
        var s = new FhirJsonSerializer();
        s.Serialize((Base)value, writer);
      }

      public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
      {
        var s = new FhirJsonParser();
        return s.Parse(reader, objectType);
      }

      public override bool CanConvert(Type objectType)
      {
        return typeof(Base).IsAssignableFrom(objectType);
      }
    }

    private JsonSerializerSettings _jsonSettings = new JsonSerializerSettings
    {
      ReferenceLoopHandling = ReferenceLoopHandling.Ignore,
      NullValueHandling = NullValueHandling.Ignore,
      Formatting = Formatting.Indented,
      ObjectCreationHandling = ObjectCreationHandling.Replace
    };

    [SetUp]
    public void SetUp()
    {
      _jsonSettings.Converters.Add(new FhirConverter());
    }

    [Test]
    public void TestBackboneElement()
    {
      // Fails!
      var example = new BackboneElementExample
      {
        MyProperty = 23,
        ParticipantComponent = new Encounter.ParticipantComponent
        {
          Individual = new ResourceReference("beep")
        }
      };

      var serialize = JsonConvert.SerializeObject(example, _jsonSettings);
      var deserialize = JsonConvert.DeserializeObject<BackboneElementExample>(serialize);
    }

    [Test]
    public void TestDomainResource()
    {
      // Succeeds!
      var example = new DomainResourceExample
      {
        Encounter = new Encounter
        {
          Appointment = new ResourceReference("beep")
        },
        MyProperty = 23
      };

      var serialize = JsonConvert.SerializeObject(example, _jsonSettings);
      var deserialize = JsonConvert.DeserializeObject<BackboneElementExample>(serialize);
    }

    [Test]
    public void TestBackBoneElementInDomainResource()
    {
      // Succeeds!
      var example = new DomainResourceExample
      {
        Encounter = new Encounter
        {
          Appointment = new ResourceReference("beep"),
          Participant = new List<Encounter.ParticipantComponent>
          {
            new Encounter.ParticipantComponent
            {
              Individual = new ResourceReference("beep")
            }
          }
        },
        MyProperty = 23
      };

      var serialize = JsonConvert.SerializeObject(example, _jsonSettings);
      var deserialize = JsonConvert.DeserializeObject<BackboneElementExample>(serialize);
    }
  }
}

[marcovisserFurore]

Thanks @Lewellync for your extensive unit test. I dived into this and found out that you also need the _jsonSettings for the method JsonConvert.DeserializeObject. When you apply this, you will notice that the unit test TestBackboneElement still fails, because the Firely SDK throws the following exception:

System.InvalidOperationException: Root object has no type indication (resourceType) and therefore cannot be used to construct an FhirJsonNode. Alternatively, specify a nodeName using the parameter.

In other words: our SDK cannot find type information about Encounter.ParticipantComponent, which is correct because this not a FHIR type or resource: it is an embedded type in the type Encounter. The SDK uses the following method to determine the FHIR type:

ModelInfo.GetFhirTypeNameForType(dataType) 

This return null for ModelInfo.GetFhirTypeNameForType(typeof(Encounter.ParticipantComponent)) .

The other unit tests are successful, because the root of those resources is Encounter, which is valid FHIR resource.

So I am afraid this won't work at the moment.

[oldefezziwig]

So I am afraid this won't work at the moment.

@marcovisserFurore thank you for delving into this. Do you know of any workaround for this in 4.1 or should a feature request be created for this? We determined that we need to upgrade to 4.1 but we also need to be able to serialize/deserialize our own classes that contain Hl7.Fhir.Model members (but also our other defined members) like the following class:

public class EncounterFhirJsonRequest
  {
    public Account Account { get; set; }

    public Encounter PrimaryEncounter { get; set; }

    public List<Tuple<Practitioner, Encounter.ParticipantComponent>> Participants { get; set; }

    public PrimaryVisitDetectionStrategy PrimaryVisitInAccountDetectionStrategy { get; set; } // an example of our own defined type/member in this class

    public Appointment Appointment { get; set; }

    public List<AllergyIntolerance> AllergyIntolerances { get; set; }

    public override ResourceType PayloadResourceType { get; set; } = ResourceType.Encounter;

  }

[marcovisserFurore]

In the previous versions (< 4.0) of this SDK, Encounter.ParticipantComponent could also not be parsed when you use the JsonConvert.DeserializeObject with FhirConverter. By the way this converter is always needed when you are parsing FHIR, because the normal Newtonsoft Json Parser cannot handle resources with Extensions.

By ignoring this FhirConverter in the previous versions of our SDK, the parsing went well by accident. From version 4.0 we added the interface IReadOnlyDictionary, which the Newtonsoft cannot handle. Hence the runtime error.

Because we don't know the type we are not able to deserialize those embedded BackboneElements.

I will discuss this in our next standup (next Monday), but I don't give you much chance. Can you explain in the meantime, why you are using here Encounter.ParticipantComponent and not the complete Encouter?

[oldefezziwig]

@marcovisserFurore why is a DomainResource like Encounter considered an IReadonlyDictionary? I get that other collections like BackboneElement could be considered IReadonlyDictionary (because they are collections) but not everything is a Dictionary, right? Or is that referenced in the FHIR STU3 spec somewhere that they should all be considered "dictionaries"?

[marcovisserFurore]

We add this IReadonlyDictionary interface to Base because we needed that for our new and faster serializers/deseriazer. See more information about these here.

[Lewellync]

We're using the Encounter.ParticipantComponent in our custom classes because of how we logically check, verify, and create FHIR resources from messages. Having each part that needs to be resolved as its own field (Encounter.ParticipantComponents are paired with Practitioner resources, and exist to store the ParticipantType field which Practitioner does not have), allows our code to logically check each individual thing, rather than having to create a "false" Encounter only to deconstruct and reconstruct later.

TLDR: for ParticipantComponent, we create them on their own to store information related to a Practitioner we need to resolve before adding it to the Encounter.

[brianpos]

If you're only looking at doing one type you can do this trick to get around it (works in the 3.x series, haven't tried in the newer versions) I needed it for some internal work I did in the past.

            // Hack to get type to serialize
            Type t = typeof(CodeSystem.ConceptDefinitionComponent);
            string rn = "ConceptDefinitionComponent";
            if (!ModelInfo.FhirTypeToCsType.ContainsKey(rn))
            {
                ModelInfo.FhirCsTypeToString.Add(t, rn);
                ModelInfo.FhirTypeToCsType.Add(rn, t);
            }

[ewoutkramer]

We're on thin ice here, since FHIR does not specify a serialization/deserialization for subtrees, as we are discussing here. For example, it's impossible to serialize a FhirBoolean to json, because it would result in two properties in the parent object (this referred to here: https://docs.fire.ly/projects/Firely-NET-SDK/parsing/itypedelement-serialization.html#working-with-subtrees).

Anyway, in this case, this would be fixable. In the mean time, there is an easy way around it, by looking at what the FhirJsonParser.Parse() extension method does internally (this is just a convenience method really):

  /// <inheritdoc cref="ParseAsync(JsonReader, Type)" />
  public Base Parse(JsonReader reader, Type dataType = null)
  {
            var rootName = dataType != null ? ModelInfo.GetFhirTypeNameForType(dataType) : null;
            var jsonReader = FhirJsonNode.Read(reader, rootName, buildNodeSettings(Settings));
            return Parse(jsonReader, dataType);
  }

We are figuring out the node name for a subtree, just so we can create an ITypedElement for it (which is then used to create a Poco from). Unfortunately, the ModelInfo.GetFhirTypeNameForType() won't do the trick - but in fact, the node name does not matter for deserialization into a Poco.

These two lines should do the trick:

   var jsonReader = FhirJsonNode.Read(reader, "dummy");
   return jsonReader.ToPoco(dataType);

You should probably use the overloads with the settings objects of Read() and ToPoco() if you need that level of control.

[brianpos]

Yes, what I described is definitely outside the FHIR spec, and only useful internally - so do use with caution, and you should understand what it is doing internally before adopting (and was really only intended for the old serializers, don't know if that works with the new ones).

[marcovisserFurore]

Unfortunately, the solution that Ewout is given here, does not work. Because somewhere during parsing the Deserializer finds out that the component is an abstract type and will throw an error.

[marcovisserFurore]

We close this issue, because we do not support serilazing/deserializing subtrees.