From 17225959807b1e437cbf217dd90750cdec53dab9 Mon Sep 17 00:00:00 2001 From: Sylvain Wallez Date: Tue, 21 Jan 2025 15:42:05 +0100 Subject: [PATCH] Allow requests to be serialized as nothing instead of an empty object --- .../clients/json/DelegatingJsonGenerator.java | 201 ++++++++++++++++++ .../json/jackson/JacksonJsonpMapper.java | 16 +- .../transport/ElasticsearchTransportBase.java | 7 +- .../transport/endpoints/EndpointBase.java | 55 +++++ .../transport/endpoints/SimpleEndpoint.java | 4 +- .../clients/transport/TransportTest.java | 71 +++++++ 6 files changed, 346 insertions(+), 8 deletions(-) create mode 100644 java-client/src/main/java/co/elastic/clients/json/DelegatingJsonGenerator.java diff --git a/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonGenerator.java b/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonGenerator.java new file mode 100644 index 000000000..11b77c9e7 --- /dev/null +++ b/java-client/src/main/java/co/elastic/clients/json/DelegatingJsonGenerator.java @@ -0,0 +1,201 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package co.elastic.clients.json; + +import jakarta.json.JsonValue; +import jakarta.json.stream.JsonGenerator; + +import java.math.BigDecimal; +import java.math.BigInteger; + +/** + * A JSON generator that delegates to another generator. + *

+ * All convenience methods that accept a property name and an event (value, start object, start array) call separately + * {@link #writeKey(String)} and the same method without the key name. This is meant to facilitate overloading + * of methods. + */ +public class DelegatingJsonGenerator implements JsonGenerator { + protected final JsonGenerator generator; + + public DelegatingJsonGenerator(JsonGenerator generator) { + this.generator = generator; + } + + public JsonGenerator unwrap() { + return generator; + }; + + @Override + public JsonGenerator writeStartObject() { + generator.writeStartObject(); + return this; + } + + @Override + public JsonGenerator writeKey(String s) { + generator.writeKey(s); + return this; + } + + @Override + public JsonGenerator writeStartArray() { + generator.writeStartArray(); + return this; + } + + @Override + public JsonGenerator writeEnd() { + generator.writeEnd(); + return this; + } + + @Override + public JsonGenerator write(JsonValue jsonValue) { + generator.write(jsonValue); + return this; + } + + @Override + public JsonGenerator write(String s) { + generator.write(s); + return this; + } + + @Override + public JsonGenerator write(BigDecimal bigDecimal) { + generator.write(bigDecimal); + return this; + } + + @Override + public JsonGenerator write(BigInteger bigInteger) { + generator.write(bigInteger); + return this; + } + + @Override + public JsonGenerator write(int i) { + generator.write(i); + return this; + } + + @Override + public JsonGenerator write(long l) { + generator.write(l); + return this; + } + + @Override + public JsonGenerator write(double v) { + generator.write(v); + return this; + } + + @Override + public JsonGenerator write(boolean b) { + generator.write(b); + return this; + } + + @Override + public JsonGenerator writeNull() { + generator.writeNull(); + return this; + } + + @Override + public void close() { + generator.close(); + } + + @Override + public void flush() { + generator.flush(); + } + + //----- Convenience key+value methods + + @Override + public final JsonGenerator writeStartObject(String s) { + this.writeKey(s); + return this.writeStartObject(); + } + + @Override + public final JsonGenerator writeStartArray(String s) { + this.writeKey(s); + return this.writeStartArray(); + } + + @Override + public final JsonGenerator write(String s, JsonValue jsonValue) { + this.writeKey(s); + return this.write(jsonValue); + } + + @Override + public final JsonGenerator write(String s, String s1) { + this.writeKey(s); + return this.write(s1); + } + + @Override + public final JsonGenerator write(String s, BigInteger bigInteger) { + this.writeKey(s); + return this.write(bigInteger); + } + + @Override + public final JsonGenerator write(String s, BigDecimal bigDecimal) { + this.writeKey(s); + return this.write(bigDecimal); + } + + @Override + public final JsonGenerator write(String s, int i) { + this.writeKey(s); + return this.write(i); + } + + @Override + public final JsonGenerator write(String s, long l) { + this.writeKey(s); + return this.write(l); + } + + @Override + public final JsonGenerator write(String s, double v) { + this.writeKey(s); + return this.write(v); + } + + @Override + public final JsonGenerator write(String s, boolean b) { + this.writeKey(s); + return this.write(b); + } + + @Override + public final JsonGenerator writeNull(String s) { + this.writeKey(s); + return this.writeNull(); + } +} diff --git a/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpMapper.java b/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpMapper.java index 84841afee..441b5990c 100644 --- a/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpMapper.java +++ b/java-client/src/main/java/co/elastic/clients/json/jackson/JacksonJsonpMapper.java @@ -21,6 +21,7 @@ import co.elastic.clients.json.BufferingJsonGenerator; import co.elastic.clients.json.BufferingJsonpMapper; +import co.elastic.clients.json.DelegatingJsonGenerator; import co.elastic.clients.json.JsonpDeserializer; import co.elastic.clients.json.JsonpDeserializerBase; import co.elastic.clients.json.JsonpMapper; @@ -95,16 +96,23 @@ protected JsonpDeserializer getDefaultDeserializer(Type type) { @Override public void serialize(T value, JsonGenerator generator) { - if (!(generator instanceof JacksonJsonpGenerator)) { - throw new IllegalArgumentException("Jackson's ObjectMapper can only be used with the JacksonJsonpProvider"); - } - JsonpSerializer serializer = findSerializer(value); if (serializer != null) { serializer.serialize(value, generator, this); return; } + // Delegating generators are used in higher levels of serialization (e.g. filter empty top-level objects). + // At this point the object is not a JsonpSerializable and we can assume we're in a nested property holding + // a user-provided type and can unwrap to find the underlying non-delegating generator. + while (generator instanceof DelegatingJsonGenerator) { + generator = ((DelegatingJsonGenerator) generator).unwrap(); + } + + if (!(generator instanceof JacksonJsonpGenerator)) { + throw new IllegalArgumentException("Jackson's ObjectMapper can only be used with the JacksonJsonpProvider"); + } + com.fasterxml.jackson.core.JsonGenerator jkGenerator = ((JacksonJsonpGenerator)generator).jacksonGenerator(); try { objectMapper.writeValue(jkGenerator, value); diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java index 2fcc42089..c95848319 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java @@ -21,6 +21,7 @@ import co.elastic.clients.elasticsearch._types.ElasticsearchException; import co.elastic.clients.elasticsearch._types.ErrorResponse; +import co.elastic.clients.json.DelegatingJsonGenerator; import co.elastic.clients.json.JsonpDeserializer; import co.elastic.clients.json.JsonpMapper; import co.elastic.clients.json.NdJsonpSerializable; @@ -260,8 +261,10 @@ private TransportHttpClient.Request prepareTranspo JsonGenerator generator = mapper.jsonProvider().createGenerator(baos); mapper.serialize(body, generator); generator.close(); - bodyBuffers = Collections.singletonList(baos.asByteBuffer()); - headers = JsonContentTypeHeaders; + if (baos.size() > 0) { + bodyBuffers = Collections.singletonList(baos.asByteBuffer()); + headers = JsonContentTypeHeaders; + } } } diff --git a/java-client/src/main/java/co/elastic/clients/transport/endpoints/EndpointBase.java b/java-client/src/main/java/co/elastic/clients/transport/endpoints/EndpointBase.java index 460370b7c..0b2344f60 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/endpoints/EndpointBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/endpoints/EndpointBase.java @@ -21,11 +21,14 @@ import co.elastic.clients.elasticsearch._types.ErrorCause; import co.elastic.clients.elasticsearch._types.ErrorResponse; +import co.elastic.clients.json.DelegatingJsonGenerator; import co.elastic.clients.json.JsonpDeserializer; import co.elastic.clients.json.JsonpDeserializerBase; import co.elastic.clients.json.JsonpMapper; +import co.elastic.clients.json.JsonpSerializable; import co.elastic.clients.json.JsonpUtils; import co.elastic.clients.transport.Endpoint; +import jakarta.json.stream.JsonGenerator; import jakarta.json.stream.JsonParser; import javax.annotation.Nullable; @@ -69,6 +72,58 @@ static Function returnSelf() { return (Function) RETURN_SELF; } + /** + * Wraps a function's result with a serializable object that will serialize to nothing if the wrapped + * object's serialization has no property, i.e. it will either produce an empty object or nothing. + */ + public static Function nonEmptyJsonObject(Function getter) { + return (x -> x == null ? null : new NonEmptySerializable(getter.apply(x))); + } + + private static final class NonEmptySerializable implements JsonpSerializable { + private final Object value; + + public NonEmptySerializable(Object value) { + this.value = value; + } + + @Override + public void serialize(JsonGenerator generator, JsonpMapper mapper) { + // Track the first property to start the top-level object, and end it if needed in close() + JsonGenerator filter = new DelegatingJsonGenerator(generator) { + boolean gotKey = false; + + @Override + public JsonGenerator writeStartObject() { + if (gotKey) { + super.writeStartObject(); + } + return this; + } + + @Override + public JsonGenerator writeKey(String s) { + if (!gotKey) { + gotKey = true; + super.writeStartObject(); + } + super.writeKey(s); + return this; + } + + @Override + public JsonGenerator writeEnd() { + if (gotKey) { + super.writeEnd(); + } + return this; + } + }; + + mapper.serialize(value, filter); + } + } + protected final String id; protected final Function method; protected final Function requestUrl; diff --git a/java-client/src/main/java/co/elastic/clients/transport/endpoints/SimpleEndpoint.java b/java-client/src/main/java/co/elastic/clients/transport/endpoints/SimpleEndpoint.java index df07d0c7d..c466a0620 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/endpoints/SimpleEndpoint.java +++ b/java-client/src/main/java/co/elastic/clients/transport/endpoints/SimpleEndpoint.java @@ -52,7 +52,7 @@ public SimpleEndpoint( Function> pathParameters, Function> queryParameters, Function> headers, - boolean hasResponseBody, + boolean hasRequestBody, JsonpDeserializer responseParser ) { this( @@ -62,7 +62,7 @@ public SimpleEndpoint( pathParameters, queryParameters, headers, - hasResponseBody ? returnSelf() : returnNull(), + hasRequestBody ? returnSelf() : returnNull(), responseParser ); } diff --git a/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java b/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java index d25466bbd..4610e71d0 100644 --- a/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java +++ b/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java @@ -20,26 +20,41 @@ package co.elastic.clients.transport; import co.elastic.clients.elasticsearch.ElasticsearchClient; +import co.elastic.clients.json.JsonpMapper; +import co.elastic.clients.json.JsonpSerializable; import co.elastic.clients.json.jackson.JacksonJsonpMapper; +import co.elastic.clients.testkit.MockHttpClient; +import co.elastic.clients.transport.endpoints.EndpointBase; +import co.elastic.clients.transport.endpoints.SimpleEndpoint; +import co.elastic.clients.transport.endpoints.SimpleJsonEndpoint; import co.elastic.clients.transport.http.RepeatableBodyResponse; +import co.elastic.clients.transport.http.TransportHttpClient; import co.elastic.clients.transport.rest_client.RestClientOptions; import co.elastic.clients.transport.rest_client.RestClientTransport; import co.elastic.clients.util.BinaryData; import com.sun.net.httpserver.HttpServer; +import jakarta.json.JsonObject; +import jakarta.json.JsonObjectBuilder; +import jakarta.json.stream.JsonGenerator; import org.apache.http.HttpHost; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; +import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import java.io.BufferedReader; +import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStream; import java.net.InetAddress; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; +import java.util.Map; import static co.elastic.clients.util.ContentType.APPLICATION_JSON; @@ -152,4 +167,60 @@ public void testOriginalJsonBodyRetrievalException() throws Exception { assertEquals("definitely not json",sb.toString()); } } + + @Test + public void testNoBodyForEmptyObject() throws Exception { + + List requests = new ArrayList<>(); + + MockHttpClient httpClient = new MockHttpClient() { + @Override + public Response performRequest(String endpointId, @Nullable Node node, Request request, TransportOptions option) throws IOException { + requests.add(request); + return super.performRequest(endpointId, node, request, option); + } + }; + + httpClient.add("/test", "text/plain", "hello"); + + JsonpMapper mapper = new JacksonJsonpMapper(); + ElasticsearchTransport transport = new ElasticsearchTransportBase(httpClient, null, mapper) {}; + + // Send empty object = true (legacy constructor) + SimpleEndpoint endpoint = new SimpleEndpoint<>( + "test-endpoint", + x -> "GET", + x -> "/test", + x -> Collections.emptyMap(), // path params + x -> Collections.emptyMap(), // query params + x -> Collections.emptyMap(), // headers + EndpointBase.nonEmptyJsonObject(x -> x), + null + ); + + transport.performRequest(new TestValue(null), endpoint, null); + transport.performRequest(new TestValue("hi"), endpoint, null); + + assertNull(requests.get(0).body()); + assertNotNull(requests.get(1).body()); + + } + + private static class TestValue implements JsonpSerializable { + + private final String value; + + public TestValue(String value) { + this.value = value; + } + + @Override + public void serialize(JsonGenerator generator, JsonpMapper mapper) { + generator.writeStartObject(); + if (value != null) { + generator.write("value", value); + } + generator.writeEnd(); + } + } }