elasticsearch-java
elasticsearch-java copied to clipboard
DisableUserResponse._DESERIALIZER should use emptyObject, not fixedValue
Java API client version
8.11.1
Java version
17
Elasticsearch Version
8.11.1
Problem description
The disable user endpoint actually returns an empty object, given server-side code here https://github.com/elastic/elasticsearch/blob/21f059166458978a5f3289686f1d218f0814c4ff/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestSetEnabledAction.java#L74.
However, the Java client uses JsonpDeserializer.fixedValue(T)
here, which expects no JSON content at all (i.e. accepts no JSON events at all). Rather, it should be using JsonpDeserializer.emptyObject(T)
, which expects/consumes an empty object and then returns the singleton result value.
Note: Think the same applies to EnableUserResponse
and ChangePasswordResponse
. Not sure why SimulateIndexTemplateResponse
is using .fixedValue
, given the server-side SimulateIndexTemplateResponse
actually contains JSON content/data, but fairly confident all uses of JsonpDeserializer.fixedValue
are suspect.
Hello, I would like to suggest my modification of the code that can solve this issue.
https://github.com/elastic/elasticsearch-java/blob/7eb733fcc5dbfeaf3ae45b64363b3ce995d6b5fe/java-client/src/main/java/co/elastic/clients/elasticsearch/security/DisableUserResponse.java#L62-L63
From this java client code and according to henryptung, you can see the DisableUserResponse._DESERIALIZER is using JsonpDeserializer.fixedValue(T)
, which I propose changing it to using JsonpDeserializer.emptyObject(T)
, due to the fact that the disable user endpoint returns an empty object given the link below.
- https://github.com/elastic/elasticsearch/blob/21f059166458978a5f3289686f1d218f0814c4ff/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestSetEnabledAction.java#L74.
This link also applies to EnableUserResponse
, since RestSetEnabledAction handles the enable user endpoint as well.
I’m assuming that when the code is generated from the Elasticsearch API specification, it is being set to use only the fixedValue function.
Also, I searched for the other server-side codes that returns emptyObject. Here are the codes that I found.
- https://github.com/elastic/elasticsearch/blob/b4938e16457dc69d392235eaf404a6dad9ddb717/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestChangePasswordAction.java#L84-L85
- https://github.com/elastic/elasticsearch/blob/b4938e16457dc69d392235eaf404a6dad9ddb717/server/src/main/java/org/elasticsearch/action/admin/indices/template/post/SimulateIndexTemplateResponse.java#L118-L134
As you can see, ChangePasswordResponse
should also be using .emptyObject(T)
, since it actually returns an emptyObject from the server-side.
For SimulateIndexTemplateResponse
server-side code, it might not always return an empty object, but there's a possibility of returning one. Therefore, I find it necessary for the response's _DESERIALIZER to use .emptyObject(T)
as well, since the emptyObject function from JsonpDeserializer
looks like this.
// JsonpDeserializer.java
static <T> JsonpDeserializer<T> emptyObject(T value) {
return new JsonpDeserializerBase<T>(EnumSet.of(Event.START_OBJECT)) {
@Override
public T deserialize(JsonParser parser, JsonpMapper mapper, Event event) {
if (event == Event.VALUE_NULL) {
return null;
}
JsonpUtils.expectNextEvent(parser, Event.END_OBJECT);
return value;
}
};
}
Unfortunately in java client, the response’s _DESERIALIZER is using fixedValue(T)
. You can notice this in these code links.
https://github.com/elastic/elasticsearch-java/blob/7eb733fcc5dbfeaf3ae45b64363b3ce995d6b5fe/java-client/src/main/java/co/elastic/clients/elasticsearch/security/EnableUserResponse.java#L62-L63
https://github.com/elastic/elasticsearch-java/blob/7eb733fcc5dbfeaf3ae45b64363b3ce995d6b5fe/java-client-serverless/src/main/java/co/elastic/clients/elasticsearch/indices/SimulateIndexTemplateResponse.java#L63-L64
https://github.com/elastic/elasticsearch-java/blob/7eb733fcc5dbfeaf3ae45b64363b3ce995d6b5fe/java-client/src/main/java/co/elastic/clients/elasticsearch/security/ChangePasswordResponse.java#L63-L64
Therefore, I made a PR that satisfies the modification offers that I mentioned in this comment.
I changed four response’s _DESERIALIZER to use emptyObject(T)
. Also, the ./gradlew check
was successful after my changes. Can you take a look at it and notify me if there is anything more to reflect or consider?
I appreciate your time and input. Have a great day!
I made #772 PR! PTAL when you have time 🙇♂️
Hello @l-trotta. I would like to request a review for my PR and let me know if it is in the right direction to solve the issue.
Hello! Thank you for your time and your interest in the java client :) unfortunately we cannot accept PRs that modify generated files, because they would just be overwritten when the code gets generated again. In this particular case, to change this code we would have to modify the java client generator, which is private. I'll keep your PR open because that's probably the result we want to end up with! Thanks again
Thank you so much for the detailed review! I'll continue using the Elasticsearch client and open some issues if I run into any! :) Have a great day!