Handle null values in serialization when the missing properties workaround is enabled (#1235)

* Handle null values in serialization when the missing properties workaround is enabled (#557)

* add unit tests for NullSafeJsonGenerator

* address review: extend DelegatingJsonGenerator, broaden test coverage

* use random mapper for Map and POJO serialization tests

Author: aOvOb

java-client/src/main/java/co/elastic/clients/json/JsonpUtils.java

@@ -21,6 +21,7 @@
 
 import co.elastic.clients.json.jackson.JacksonJsonProvider;
 import co.elastic.clients.util.AllowForbiddenApis;
+import co.elastic.clients.util.ApiTypeHelper;
 import jakarta.json.JsonException;
 import jakarta.json.JsonObject;
 import jakarta.json.JsonString;
@@ -498,7 +499,12 @@ public void close() {
             }
         };
 
-        try(JsonGenerator generator = mapper.jsonProvider().createGenerator(writer)) {
+        try(JsonGenerator rawGenerator = mapper.jsonProvider().createGenerator(writer)) {
+            // When the missing-properties workaround is active, wrap the generator so that null
+            // reference values are serialized as JSON null instead of throwing NPE. See #557.
+            JsonGenerator generator = ApiTypeHelper.requiredPropertiesCheckDisabled()
+                ? new NullSafeJsonGenerator(rawGenerator)
+                : rawGenerator;
             value.serialize(generator, mapper);
         } catch (ToStringTooLongException e) {
             // Ignore
@@ -508,7 +514,12 @@ public void close() {
 
     public static String toJsonString(Object value, JsonpMapper mapper) {
         StringWriter writer = new StringWriter();
-        JsonGenerator generator = mapper.jsonProvider().createGenerator(writer);
+        JsonGenerator rawGenerator = mapper.jsonProvider().createGenerator(writer);
+        // When the missing-properties workaround is active, wrap the generator so that null
+        // reference values are serialized as JSON null instead of throwing NPE. See #557.
+        JsonGenerator generator = ApiTypeHelper.requiredPropertiesCheckDisabled()
+            ? new NullSafeJsonGenerator(rawGenerator)
+            : rawGenerator;
         mapper.serialize(value, generator);
         generator.close();
         return writer.toString();

java-client/src/main/java/co/elastic/clients/json/NullSafeJsonGenerator.java

@@ -0,0 +1,82 @@
+/*
+ * 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 {@link DelegatingJsonGenerator} that translates {@code write(...)} calls with a {@code null} reference value
+ * into a {@link JsonGenerator#writeNull() writeNull} call. The {@code write(name, value)} overloads in
+ * {@link DelegatingJsonGenerator} are {@code final} and split into {@code writeKey(name)} + {@code write(value)},
+ * so overriding the value-only methods here is enough to also cover the name+value variants via virtual dispatch.
+ *
+ * <p>This is used as a safety net when {@link co.elastic.clients.util.ApiTypeHelper#DANGEROUS_disableRequiredPropertiesCheck(boolean)}
+ * is active: with the workaround enabled, deserialization can leave required reference fields as {@code null},
+ * and the generated {@code serializeInternal} methods would otherwise call e.g. {@code generator.write((String) null)},
+ * which throws {@link NullPointerException} in JSON-P providers such as Parsson.
+ *
+ * <p>See <a href="https://github.com/elastic/elasticsearch-java/issues/557">#557</a>.
+ */
+class NullSafeJsonGenerator extends DelegatingJsonGenerator {
+
+    NullSafeJsonGenerator(JsonGenerator delegate) {
+        super(delegate);
+    }
+
+    @Override
+    public JsonGenerator write(String value) {
+        if (value == null) {
+            generator.writeNull();
+            return this;
+        }
+        return super.write(value);
+    }
+
+    @Override
+    public JsonGenerator write(BigDecimal value) {
+        if (value == null) {
+            generator.writeNull();
+            return this;
+        }
+        return super.write(value);
+    }
+
+    @Override
+    public JsonGenerator write(BigInteger value) {
+        if (value == null) {
+            generator.writeNull();
+            return this;
+        }
+        return super.write(value);
+    }
+
+    @Override
+    public JsonGenerator write(JsonValue value) {
+        if (value == null) {
+            generator.writeNull();
+            return this;
+        }
+        return super.write(value);
+    }
+}

java-client/src/test/java/co/elastic/clients/elasticsearch/model/BehaviorsTest.java

@@ -468,4 +468,38 @@ public void testDangerousDisablePropertyCheckPrimitive(){
             assertTrue(healthResponse.toString()!=null);
         }
     }
+
+    @Test
+    public void testDangerousDisablePropertyCheckReferenceSerialization() {
+        // Repro for #557: with the workaround enabled, a null required reference
+        // field (here: clusterName, a required String) causes an NPE during
+        // re-serialization because HealthResponseBody#serializeInternal calls
+        // generator.write((String) null).
+        try (ApiTypeHelper.DisabledChecksHandle h =
+                 ApiTypeHelper.DANGEROUS_disableRequiredPropertiesCheck(true)) {
+            HealthResponse healthResponse = HealthResponse.of(hr -> hr.withJson(new StringReader("{\n" +
+                // cluster_name (required) intentionally omitted
+                "  \"status\" : \"green\",\n" +
+                "  \"timed_out\" : false,\n" +
+                "  \"number_of_nodes\" : 3,\n" +
+                "  \"number_of_data_nodes\" : 2,\n" +
+                "  \"active_primary_shards\" : 88,\n" +
+                "  \"active_shards\" : 176,\n" +
+                "  \"relocating_shards\" : 0,\n" +
+                "  \"initializing_shards\" : 0,\n" +
+                "  \"unassigned_shards\" : 0,\n" +
+                "  \"delayed_unassigned_shards\" : 0,\n" +
+                "  \"number_of_pending_tasks\" : 0,\n" +
+                "  \"number_of_in_flight_fetch\" : 0,\n" +
+                "  \"task_max_waiting_in_queue_millis\" : 0,\n" +
+                "  \"active_shards_percent_as_number\" : 100.0\n" +
+                "}")));
+
+            // deserialization succeeds — clusterName is null because the workaround is active
+            assertTrue(healthResponse.clusterName() == null);
+
+            // round-trip currently fails with NullPointerException — this is bug #557
+            assertTrue(healthResponse.toString() != null);
+        }
+    }
 }

java-client/src/test/java/co/elastic/clients/json/NullSafeJsonGeneratorTest.java

@@ -0,0 +1,140 @@
+/*
+ * 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 co.elastic.clients.testkit.ModelTestCase;
+import co.elastic.clients.util.ApiTypeHelper;
+import jakarta.json.JsonValue;
+import jakarta.json.stream.JsonGenerator;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+
+import java.io.StringWriter;
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+public class NullSafeJsonGeneratorTest extends ModelTestCase {
+
+    @Test
+    public void testNullReferencesAreWrittenAsJsonNull() {
+        StringWriter writer = new StringWriter();
+        try (JsonGenerator generator = new NullSafeJsonGenerator(mapper.jsonProvider().createGenerator(writer))) {
+            generator.writeStartObject();
+
+            // writeKey + write(value) overloads with null reference values
+            generator.writeKey("string_field");
+            generator.write((String) null);
+            generator.writeKey("decimal_field");
+            generator.write((BigDecimal) null);
+            generator.writeKey("integer_field");
+            generator.write((BigInteger) null);
+            generator.writeKey("json_value_field");
+            generator.write((JsonValue) null);
+
+            // name+value overloads with null reference values — DelegatingJsonGenerator splits these into
+            // writeKey(name) + write(value), so they reach the overrides above via virtual dispatch.
+            generator.write("named_string", (String) null);
+            generator.write("named_decimal", (BigDecimal) null);
+            generator.write("named_integer", (BigInteger) null);
+            generator.write("named_json_value", (JsonValue) null);
+
+            generator.writeEnd();
+        }
+
+        assertEquals(
+            "{" +
+                "\"string_field\":null," +
+                "\"decimal_field\":null," +
+                "\"integer_field\":null," +
+                "\"json_value_field\":null," +
+                "\"named_string\":null," +
+                "\"named_decimal\":null," +
+                "\"named_integer\":null," +
+                "\"named_json_value\":null" +
+            "}",
+            writer.toString()
+        );
+    }
+
+    @Test
+    public void testNonNullValuesPassThrough() {
+        StringWriter writer = new StringWriter();
+        try (JsonGenerator generator = new NullSafeJsonGenerator(mapper.jsonProvider().createGenerator(writer))) {
+            generator.writeStartObject();
+            generator.write("string_field", "hello");
+            generator.write("int_field", 42);
+            generator.write("long_field", 10000000000L);
+            generator.write("double_field", 1.5);
+            generator.write("bool_field", true);
+            generator.write("decimal_field", new BigDecimal("3.14"));
+            generator.write("integer_field", new BigInteger("12345"));
+            generator.writeEnd();
+        }
+
+        assertEquals(
+            "{" +
+                "\"string_field\":\"hello\"," +
+                "\"int_field\":42," +
+                "\"long_field\":10000000000," +
+                "\"double_field\":1.5," +
+                "\"bool_field\":true," +
+                "\"decimal_field\":3.14," +
+                "\"integer_field\":12345" +
+            "}",
+            writer.toString()
+        );
+    }
+
+    @Test
+    public void testToJsonStringWithMapWhileWorkaroundActive() {
+        // SimpleJsonpMapper only serializes types registered in the Java client and doesn't support arbitrary Maps.
+        Assumptions.assumeFalse(jsonImpl == JsonImpl.Simple, "SimpleJsonpMapper does not support arbitrary Map serialization");
+        Map<String, Object> map = new LinkedHashMap<>();
+        map.put("a", 1);
+        map.put("b", "two");
+
+        try (ApiTypeHelper.DisabledChecksHandle h =
+                 ApiTypeHelper.DANGEROUS_disableRequiredPropertiesCheck(true)) {
+            assertEquals("{\"a\":1,\"b\":\"two\"}", JsonpUtils.toJsonString(map, mapper));
+        }
+    }
+
+    @Test
+    public void testToJsonStringWithPojoWhileWorkaroundActive() {
+        Assumptions.assumeFalse(jsonImpl == JsonImpl.Simple, "SimpleJsonpMapper does not support POJO serialization");
+        SomePojo pojo = new SomePojo();
+        pojo.name = "test";
+        pojo.value = 42;
+
+        try (ApiTypeHelper.DisabledChecksHandle h =
+                 ApiTypeHelper.DANGEROUS_disableRequiredPropertiesCheck(true)) {
+            String json = JsonpUtils.toJsonString(pojo, mapper);
+            assertTrue(json.contains("\"name\":\"test\""), "Expected name field, got: " + json);
+            assertTrue(json.contains("\"value\":42"), "Expected value field, got: " + json);
+        }
+    }
+
+    public static class SomePojo {
+        public String name;
+        public int value;
+    }
+}