ModelBinding fails with Error ... Patient is abstract .... could not be instantiated and populated

[Brilbroeder]

project-file
<PackageReference Include="Hl7.Fhir.R5" Version="5.3.0" />

startup.cs

            services
                .AddControllers()
                .AddJsonOptions(options =>
                {
                    options.JsonSerializerOptions.ForFhir(ModelInfo.ModelInspector);
                });

controller-action

        [HttpPost]
        public ActionResult Create(Patient patient)
        {
            return Ok(patient);
        }

Error :
System.NotSupportedException: The collection type 'Hl7.Fhir.Model.Patient' is abstract, an interface, or is read only, and could not be instantiated and populated. Path: $ | LineNumber: 0 | BytePositionInLine: 1.

Any suggestions ?

[ewoutkramer]

The text "The collection type" is suspicous. It's not a collection.,,,,,wait a second....

Yes, every resource is an IEnumerable<KeyValuePair<>>.

I guess this means that in the list of converters installed in the service, there's a (default) converter installed that serializes collections BEFORE our own special FHIR serializer is called. Which is entirely possible. ForFhir() just adds our converter at the end of the list.

That's probably it. Not yet a solution, but we could start to experiment by wiping the options.Converters array in the lambda before the call to ForFhir()?

[ewoutkramer]

I do regret letting Base (and subclasses) implement IReadOnlyDictionary<> directly. We've seen a few frameworks thrown off by this, much like the serializer here. Also, for example, the Visual Studio debugger shows resources as key value pairs in its enumerable viewer.

Could we explore the idea of having an indirect implementation, i.e. having an AsReadOnlyDictionary() on Base, instead of it implementing it directly? That's obviously a breaking change, but would it be worth it?

[Brilbroeder]

@ewoutkramer I tried the following. (see below) This should put your converter first. I put a breakpoint to see the number of savedConverters. It is 0. Hence this did not change anything.

                .AddJsonOptions(options =>
                {
                    var savedConverters = options.JsonSerializerOptions.Converters.ToList();
                    options.JsonSerializerOptions.Converters.Clear();
                    options.JsonSerializerOptions.ForFhir(ModelInfo.ModelInspector);

                    foreach (var savedConverter in savedConverters)
                    {
                        options.JsonSerializerOptions.Converters.Add(savedConverter);
                    }
                });

[ewoutkramer]

Ok, so that would have been too easy ;-) Still, it's clear to me that our converter, which should handle all resources, is not called, but some more generic converter now handles it, and throws this error.

[ewoutkramer]

I found it.

This is what our code in ForFhir() does:

  /// <summary>
  /// Initialize the options to serialize using the JsonFhirConverterFactory, producing compact output without whitespace.
  /// </summary>
  public static JsonSerializerOptions ForFhir(this JsonSerializerOptions options, FhirJsonConverterFactory converter)
  {
      var result = new JsonSerializerOptions(options)
      {
          Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
      };

      result.Converters.Add(converter);
      return result;
  }

In other words, we don't modify the original options passed in, but we create a new options instance, based on the original one, add our converter to it, and then return the new options. But that's not what the service configurator needs: we need to modify the original options, as the return value of ForFhir() is ignored.

[ewoutkramer]

If I am correct, then a workaround could be to capture the results of ForFhir(), and then add the converters back into the original. more or less like so:

 .AddJsonOptions(options =>
                {
                    var fhirConverters = options.ForFhir(ModelInfo.ModelInspector).Converters;               
                    foreach (var fhirConverter in fhirConverters)
                    {
                        options.JsonSerializerOptions.Converters.Add(fhirConverter);
                    }

                   options.Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping,

[Brilbroeder]

This workaround is working:

                .AddJsonOptions(options =>
                {
                    var fhirConverters = options.JsonSerializerOptions.ForFhir(ModelInfo.ModelInspector).Converters;
                    foreach (var fhirConverter in fhirConverters)
                    {
                        options.JsonSerializerOptions.Converters.Add(fhirConverter);
                    }
                    options.JsonSerializerOptions.Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping;
                });

Will there be a fix in the SDK ?

[ewoutkramer]

Yes, for sure. With this confirmed, we can fix it. I have given this issue the bug label now.

[ridafatima90]

I believe it should be something like this, we should create new converter, otherwise it is throwing error original Collection was modified error.

.AddJsonOptions(opt =>
{
    IList<JsonConverter> fhirConverters = opt.JsonSerializerOptions.ForFhir(ModelInfo.ModelInspector).Converters;
    IList<JsonConverter> convertersToAdd = new List<JsonConverter>(fhirConverters);
    foreach(JsonConverter fhirConverter in convertersToAdd)
    {
        opt.JsonSerializerOptions.Converters.Add(fhirConverter);
    }
    opt.JsonSerializerOptions.Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping;
});