Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ElasticsearchIndicesClient#getMapping(GetMappingRequest) throws on reading response when mapping contains an object property #45

Closed
sothawo opened this issue Nov 11, 2021 · 12 comments
Assignees
Labels
Category: Bug Something isn't working v7.16.0

Comments

@sothawo
Copy link
Contributor

sothawo commented Nov 11, 2021

Using version 7.15.2.

When an index mapping contains a property that is an object,
co.elastic.clients.elasticsearch.indices.ElasticsearchIndicesClient#getMapping(co.elastic.clients.elasticsearch.indices.GetMappingRequest) crashes on parsing the result.

This can be tested with the following code:

class Name {
	String first;
	String last;

	public Name(String first, String last) {
		this.first = first;
		this.last = last;
	}
       // getter+setter
}

class Person {
	String id;
	Name name;

	public Person(String id, Name name) {
		this.id = id;
		this.name = name;
	}
       // getter+setter
}

ElasticsearchClient client = ... // setup the ElasticsearchClient
String index = "testindex";
Person person = new Person("42", new Name("Ford", "Prefect"));
client.index(b -> b.index(index).id(person.id).document(person));
GetMappingResponse getMappingResponse = client.indices().getMapping(mrb -> mrb.index(index));

This will produce the following error:

jakarta.json.stream.JsonParsingException: Property 'type' not found

	at co.elastic.clients.json.JsonpUtils.lookAheadFieldValue(JsonpUtils.java:139)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:128)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:95)
	at co.elastic.clients.json.BuildFunctionDeserializer.deserialize(BuildFunctionDeserializer.java:42)
	at co.elastic.clients.json.JsonpDeserializer$LazyDeserializer.deserialize(JsonpDeserializer.java:205)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:102)
	at co.elastic.clients.json.JsonpDeserializer$StringMapDeserializer.deserialize(JsonpDeserializer.java:461)
	at co.elastic.clients.json.JsonpDeserializer$StringMapDeserializer.deserialize(JsonpDeserializer.java:447)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:102)
	at co.elastic.clients.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:68)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:122)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:95)
	at co.elastic.clients.json.BuildFunctionDeserializer.deserialize(BuildFunctionDeserializer.java:42)
	at co.elastic.clients.json.JsonpDeserializer$LazyDeserializer.deserialize(JsonpDeserializer.java:205)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:102)
	at co.elastic.clients.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:68)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:122)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:95)
	at co.elastic.clients.json.BuildFunctionDeserializer.deserialize(BuildFunctionDeserializer.java:42)
	at co.elastic.clients.json.JsonpDeserializer$LazyDeserializer.deserialize(JsonpDeserializer.java:205)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:102)
	at co.elastic.clients.base.DictionaryResponse.lambda$setupDictionaryResponseDeserializer$0(DictionaryResponse.java:148)
	at co.elastic.clients.json.ObjectDeserializer.parseUnknownField(ObjectDeserializer.java:150)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:120)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:95)
	at co.elastic.clients.json.BuildFunctionDeserializer.deserialize(BuildFunctionDeserializer.java:42)
	at co.elastic.clients.json.JsonpDeserializer$LazyDeserializer.deserialize(JsonpDeserializer.java:205)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:102)
	at co.elastic.clients.base.rest_client.RestClientTransport.getHighLevelResponse(RestClientTransport.java:242)
	at co.elastic.clients.base.rest_client.RestClientTransport.performRequest(RestClientTransport.java:104)
	at co.elastic.clients.elasticsearch.indices.ElasticsearchIndicesClient.getMapping(ElasticsearchIndicesClient.java:1044)
	at co.elastic.clients.elasticsearch.indices.ElasticsearchIndicesClient.getMapping(ElasticsearchIndicesClient.java:1061)

The mapping returned from the server:

{
  "testindex" : {
    "mappings" : {
      "properties" : {
        "id" : {
          "type" : "text",
          "fields" : {
            "keyword" : {
              "type" : "keyword",
              "ignore_above" : 256
            }
          }
        },
        "name" : {
          "properties" : {
            "first" : {
              "type" : "text",
              "fields" : {
                "keyword" : {
                  "type" : "keyword",
                  "ignore_above" : 256
                }
              }
            },
            "last" : {
              "type" : "text",
              "fields" : {
                "keyword" : {
                  "type" : "keyword",
                  "ignore_above" : 256
                }
              }
            }
          }
        }
      }
    }
  }
}

I debugged through the code; when reading the properties, the parser comes to the name entry and then on finding a START_OBJECT tries to get the type property of the object, basically expecting something like

{
    "type": "object",   <===  Elasticsearch does not send a type entry here
    "properties": {
      "first": {
        "type": "text",
        "fields": {
          "keyword": {
            "type": "keyword",
            "ignore_above": 256
          }
        }
      },
      "last": {
        "type": "text",
        "fields": {
          "keyword": {
            "type": "keyword",
            "ignore_above": 256
          }
        }
      }
    }

Even when the mapping is stored explicitly in Elasticsearch with the type object, it is not returned on getting the mapping.

@pullannagari
Copy link

Hi @sothawo , I am new to contributing to open source - can I pick this issue and start working?

@sothawo
Copy link
Contributor Author

sothawo commented Nov 19, 2021

I am not maintaining this project, just reporting this issue. @swallez surely can tell you more.

@swallez swallez added v7.15 Category: Bug Something isn't working labels Nov 19, 2021
@swallez swallez self-assigned this Nov 19, 2021
@swallez
Copy link
Member

swallez commented Nov 19, 2021

@sothawo this was a bug in the API spec that is used to generate the client code. This has been fixed in the upcoming 7.16 (I have checked that it works).

The exception thrown for missing properties has also been improved in 7.16 to report not only the property name but also the class name.

Since the API spec is a huge effort to formalize the Elasticsearch API, we may keep on encountering this kind of issues. I've added a way to have scoped disabling of required properties checks in order to not be blocked by this kind of issue. Yes, it's kind of a hack, but a least allows to move forward without having to wait for the next release of the client just for that 😉

@swallez swallez closed this as completed Nov 19, 2021
@swallez
Copy link
Member

swallez commented Nov 19, 2021

@pullannagari as you may have seen from the comment above, it has been solved already!

It's nice for you to want to contribute to open source projects. Now this project is a bit uncommon in that it has two really distinct parts:

  • the Elasticsearch API implementation, in the co.elastic.clients.elasticsearch package which is entirely generated from the API specification. Contribution there has to be done through that specification, and so isn't straightforward. We do however appreciate reports of discrepancies with what Elasticsearch actually expects in requests and returns in responses.
  • the supporting framework in other packages, tests, and everything else, which are "normal" handwritten code and where code contributions are welcome.

@sothawo
Copy link
Contributor Author

sothawo commented Nov 19, 2021

@swallez I am currently adding the support for this client to Spring Data Elasticsearch and am running my integration tests against ES with both the RestHighLevelClient and the new client. For tests that fail I have a JUnit @DisabledIf("newElasticsearchClient") flag with which I exclude the test that show errors from currently not yet supported features. This way I can keep integrating and when a new version comes out I can check which of these tests then will pass.

I'm building this in a way so that in the next version of Spring Data Elasticsearch (4.4, about in a half year) the default client will still be the RestHighLevelClient and the new client will be optional. The idea/plan is for the next big release that is based on Java 17 with Spring 6 support (end of next year) to use the new client and drop the RestHighLevelClient support then.

Addition: I will keep adding issues if I find places that do not work as supposed, that's alright?

@pullannagari
Copy link

@pullannagari as you may have seen from the comment above, it has been solved already!

It's nice for you to want to contribute to open source projects. Now this project is a bit uncommon in that it has two really distinct parts:

  • the Elasticsearch API implementation, in the co.elastic.clients.elasticsearch package which is entirely generated from the API specification. Contribution there has to be done through that specification, and so isn't straightforward. We do however appreciate reports of discrepancies with what Elasticsearch actually expects in requests and returns in responses.
  • the supporting framework in other packages, tests, and everything else, which are "normal" handwritten code and where code contributions are welcome.

Thanks @swallez for taking time to look into my request, will look the advised topics further.

@sothawo
Copy link
Contributor Author

sothawo commented Nov 20, 2021

@swallez I just tried it with the 7.16.0-SNAPSHOT version from the elasticsearch snapshot repository and still get the exception

@sothawo
Copy link
Contributor Author

sothawo commented Nov 20, 2021

The exception thrown for missing properties has also been improved in 7.16 to report not only the property name but also the class name.

I don't see that either in 7.16, even if I build 7.16 locally directly from the code and use that jar.

This is thrown for existing defined properties of a class, but the code in question sets this on the property deserializer (https://github.com/elastic/elasticsearch-java/blob/7.16/java-client/src/main/java/co/elastic/clients/elasticsearch/_types/mapping/Property.java#L1076) which is later used in a lookahead to get the real type. There the check throwing the exception is not used.

@swallez swallez reopened this Nov 23, 2021
@swallez
Copy link
Member

swallez commented Nov 24, 2021

Finally nailed it. ES indeed sends a type field (the discriminant for polymorphic deserialization) for all property types except "object", so we have to consider it as the default value is type is missing.

Fixed, verified with a unit test and an integration test. Please cross-check.

@sothawo
Copy link
Contributor Author

sothawo commented Nov 24, 2021

I can confirm that this works with the 7.16.0-SNAPSHOT.

@sothawo sothawo closed this as completed Nov 24, 2021
@LordOfTheHelmet
Copy link

Hi @swallez ,
i still get this exception in 7.17.0 with the following mapping:

{
   "settings":{
      "index":{
         "number_of_shards":"2",
         "number_of_replicas":"0",
         "analysis":{
            "analyzer":{
               "de_stop":{
                  "filter":[
                     "lowercase",
                     "german_normalization"
                  ],
                  "tokenizer":"standard"
               }
            }
         }
      }
   },
   "mappings":{
      "dynamic":"strict",
      "properties":{
         "title":{
            "type":"keyword"
         }
      }
   }
}

The stacktrace reads like:

Exception in thread "main" jakarta.json.stream.JsonParsingException: Property 'type' not found
	at co.elastic.clients.json.JsonpUtils.lookAheadFieldValue(JsonpUtils.java:135)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:184)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:137)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.BuildFunctionDeserializer.deserialize(BuildFunctionDeserializer.java:47)
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43)
	at co.elastic.clients.json.JsonpDeserializerBase$StringMapDeserializer.deserialize(JsonpDeserializerBase.java:346)
	at co.elastic.clients.json.JsonpDeserializerBase$StringMapDeserializer.deserialize(JsonpDeserializerBase.java:332)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:72)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:176)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:137)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:79)
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43)
	at co.elastic.clients.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:72)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:176)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:137)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:79)
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43)
	at co.elastic.clients.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:72)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:176)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:137)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:79)
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43)
	at co.elastic.clients.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:72)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:176)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:137)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:79)
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43)
	at co.elastic.clients.transport.endpoints.DictionaryResponse.lambda$setupDictionaryResponseDeserializer$0(DictionaryResponse.java:148)
	at co.elastic.clients.json.ObjectDeserializer.parseUnknownField(ObjectDeserializer.java:205)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:174)
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:137)
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75)
	at co.elastic.clients.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:79)
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43)
	at co.elastic.clients.transport.rest_client.RestClientTransport.decodeResponse(RestClientTransport.java:328)
	at co.elastic.clients.transport.rest_client.RestClientTransport.getHighLevelResponse(RestClientTransport.java:294)
	at co.elastic.clients.transport.rest_client.RestClientTransport.performRequest(RestClientTransport.java:147)
	at co.elastic.clients.elasticsearch.indices.ElasticsearchIndicesClient.get(ElasticsearchIndicesClient.java:934)
	at co.elastic.clients.elasticsearch.indices.ElasticsearchIndicesClient.get(ElasticsearchIndicesClient.java:950)

Should I open a new bug report for this one or could this be addressed here?

Thanks for your help.

@swallez
Copy link
Member

swallez commented Feb 11, 2022

@LordOfTheHelmet please open a new issue as this is a different issue from this one. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Bug Something isn't working v7.16.0
Projects
None yet
Development

No branches or pull requests

4 participants