opensearch-project / opensearch-net

OpenSearch .NET Client
Apache License 2.0
105 stars 49 forks source link

[BUG] Deserialization Cannot Handle Types with Constructors with Optional Parameters #726

Open JCKortlang opened 3 months ago

JCKortlang commented 3 months ago

What is the bug?

Deserialization Cannot Handle Types with Constructors with Optional Parameters

System.Exception: Cannot create an instance of System.Text.Json.Nodes.JsonObject because it does not have a public constructor accepting IDictionary<System.String,System.Text.Json.Nodes.JsonNode> argument or a public parameterless constructor
   at OpenSearch.Client.InterfaceGenericDictionaryResolver.DictionaryFormatterHelper.GetFormatter(Type t)
   at OpenSearch.Client.InterfaceGenericDictionaryResolver.FormatterCache`1..cctor()

Specifically, new JsonObject() is valid but fatals the deserialziation code. The result is we cannot deserialize to a JsonObject

    //Given
    public JsonObject(JsonNodeOptions? options = null)
      : base(options)
    {
    }

How can one reproduce the bug?

using System.Text.Json.Nodes
return await openSearchClient.SearchAsync<JsonObject>(searchRequest);

What is the expected behavior?

I expect to be able to deserialize to standard .NET Json abstractions

What is your host/environment?

MacOS 13.6.7 (22G720)

Do you have any screenshots?

No

Do you have any additional context?

I am attempting to deserialize a request to an index alias that contains multiple types. Since the Search interface does not support this, I need to deserialize the response to a more generic type. Ideally one of the standard and existing Json abstractions.

Xtansia commented 3 months ago

Hi @JCKortlang,

System.Text.Json is not yet supported or used by the library (see #388), and as these generic JsonObject/JsonNode types usually require special handling rather than being treated as plain objects I don't believe that fixing the constructor issue would solve your issue.

You should be able to use dynamic/DynamicObject as the document type to handle unspecified structures. Alternatively there is also an implementation for using Newtonsoft

JCKortlang commented 3 months ago

Hello @Xtansia,

I did read through that issue. My main intent was to point out the constructor constraint causes issues. Unclear if this is intended / expected for constructors with optional parameters.

public record Foo
{
  //Would also fail?
  public Foo(object? optional = null)
  {

  }
}

Is there a mechanism to receive Document as a byte array or a value that has not been deserialized? The intent is to save the initial deserialization cost. Should I create a separate issue?

Xtansia commented 3 months ago

Yes the constructor issue is valid, will need to see how it compares in scope/effort/priority wise against the fact we intend to throw away the current JSON serialisation and move to System.Text.Json in the future.

The way to skip all deserialisation is to use the generic http methods, using BytesResponse or StringResponse like so:

var resp = await client.Http.PostAsync<BytesResponse>(
    $"/{IndexName}/_search",
    r => r.SerializableBody(new SearchDescriptor<object>()
        .Query(q => q.MatchAll())
        .Size(20)));

var bytes = resp.Body;