Elastic.Transport.UnexpectedTransportException when using GetMappingAsync and dynamic_templates

[gpetrou]

Elastic.Clients.Elasticsearch version: 8.13.14

Elasticsearch version: 8.12.2

.NET runtime version: 8.0

Operating system version:

Description of the problem including expected versus actual behavior:
When calling GetMappingAsync with our template that contains dynamic_templates there is a The JSON value could not be converted to System.Collections.Generic.ICollection1[System.String]. Path: $ | LineNumber: 0 | BytePositionInLine: 11.` error.
This seems to be working just fine until version 8.13.13.

Steps to reproduce:
Use a template that contains dynamic_templates. For example,

"dynamic_templates": [
  {
    "test_template": {
      "match_mapping_type": "string",
      "path_match": "xyz.*",
      "mapping": {
        "index": true,
        "analyzer": "fulltext",
        "search_analyzer": "synonym_analyzer",
        "type": "text",
        "store": false
      }
    }
  }
],

Call elasticsearchClient.Indices.GetMappingAsync

Expected behavior
GetMappingAsync works as before.

Provide ConnectionSettings (if relevant):

Provide DebugInformation (if relevant):

[gpetrou]

@flobernd I assume something in #8206 caused this issue. I see in such PRs that there are no test changes. Do related tests exist in a separate repository? I wonder how this library is actually being tested since I also see that GitHub builds are always in a red state for some time now.

[flobernd]

Hi @gpetrou, could you please post the complete callstack of the exception?

I assume something in #8206 caused this issue. I see in such PRs that there are no test changes.

The clients (not only .NET) are mainly generated on base of the Elasticsearch Specification. The main reason for this is to keep the clients maintainable without too much manual effort (it's only me working on the client at the moment; same situation for the other language clients).

The specification changes are validated as part of the PR approval workflow. There is an automation that compares the specification data-structures with pre-recorded Elasticsearch request/response payloads.

Besides that, we have a large test suite (our so called YAML Tests) that runs in the background. These are integration tests. For statically typed languages, like .NET, this is significantly harder to implement in a generic way than for dynamically typed languages.

I also see that GitHub builds are always in a red state for some time now

Besides the mentioned tests and safety measures, the .NET client maintains a separate test suite. These tests were ported over from NEST (however, not all tests made it into the new client so far). These are unit tests and integration tests.

With 8.13, we switched to a completely revised code generator and we introduced some breaking changes in the process. The .NET specific tests are currently not all fixed to support the new syntax. This is on my todo list, like e.g. mentioned in #8290.

TLDR: Yes, there are multiple levels of tests and validations, but for this issue, either no test case exists, or the bug is very specific to the .NET client and the related code generation.

[flobernd]

The change that caused the regression was this one:

elastic/elasticsearch-specification@982cc3c#diff-8e5507a948ffac5400a7733eaca961f127eaf60241e49ccd3d46d9b4db100d27

In the end, it indeed boils down to an issue in the code generator. The DynamicTemplate class uses a custom JsonConverter for (de-)serialization, but the code generator incorrectly adds the SingleOrManyCollectionConverter attribute to the properties (which does not have any effect in this case):

https://github.com/elastic/elasticsearch-net/pull/8206/files#diff-82812ac7f2ee95aefe28ce925e1d4da54921ebbb7c017dabeccc3584d3c8284aR53-R54

Instead, special deserialization code must be generated in the custom converter, e.g. in this line:

https://github.com/elastic/elasticsearch-net/pull/8206/files#diff-82812ac7f2ee95aefe28ce925e1d4da54921ebbb7c017dabeccc3584d3c8284aR120

[flobernd]

This was actually fixed already in #8245.

Upgrading to https://github.com/elastic/elasticsearch-net/releases/tag/8.14.3 should solve the issue.

Duplicate of #8244

[flobernd]

Closing this for now. Please let me know, if you have further questions.