DisableUserResponse._DESERIALIZER should use emptyObject, not fixedValue

[henryptung]

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.

[shinsj4653]

Hello, I would like to suggest my modification of the code that can solve this issue.

elasticsearch-java/java-client/src/main/java/co/elastic/clients/elasticsearch/security/DisableUserResponse.java

Lines 62 to 63 in 7eb733f

	public static final JsonpDeserializer<DisableUserResponse> _DESERIALIZER = JsonpDeserializer
	.fixedValue(DisableUserResponse._INSTANCE);

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.

elasticsearch-java/java-client/src/main/java/co/elastic/clients/elasticsearch/security/EnableUserResponse.java

Lines 62 to 63 in 7eb733f

	public static final JsonpDeserializer<EnableUserResponse> _DESERIALIZER = JsonpDeserializer
	.fixedValue(EnableUserResponse._INSTANCE);

elasticsearch-java/java-client-serverless/src/main/java/co/elastic/clients/elasticsearch/indices/SimulateIndexTemplateResponse.java

Lines 63 to 64 in 7eb733f

	public static final JsonpDeserializer<SimulateIndexTemplateResponse> _DESERIALIZER = JsonpDeserializer
	.fixedValue(SimulateIndexTemplateResponse._INSTANCE);

elasticsearch-java/java-client/src/main/java/co/elastic/clients/elasticsearch/security/ChangePasswordResponse.java

Lines 63 to 64 in 7eb733f

	public static final JsonpDeserializer<ChangePasswordResponse> _DESERIALIZER = JsonpDeserializer
	.fixedValue(ChangePasswordResponse._INSTANCE);

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!

[shinsj4653]

I made #772 PR! PTAL when you have time 🙇‍♂️

[shinsj4653]

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.

[l-trotta]

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

[shinsj4653]

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!

[l-trotta]

Hello! I've fixed this issue in the generator and pushed the new code to the java client, you can see it for example in the EnableUserResponse class. Thanks again for providing the original fix :)