Handle null values in serialization when the missing properties workaround is enabled

[aOvOb]

Closes #557.

Problem

The documented workaround for MissingRequiredPropertyException — ApiTypeHelper.DANGEROUS_disableRequiredPropertiesCheck(true) — only fixes deserialization. Once active, required reference fields (String, BigDecimal, BigInteger, JsonValue) can be null in the deserialized object. Re-serializing such an object (most commonly via .toString()) then throws NullPointerException, because generated serializeInternal methods call e.g. generator.write((String) null) and JSON-P providers like Parsson reject it.

This is also the underlying cause of the workaround failure reported in #1082.

Approach

Introduce a package-private NullSafeJsonGenerator that delegates to a wrapped JsonGenerator but translates write(...) calls with a null reference value into the corresponding writeNull(...) call:

• write((String|BigDecimal|BigInteger|JsonValue) null) → writeNull()
• write(name, (String|BigDecimal|BigInteger|JsonValue) null) → writeNull(name)
• All other methods (primitive overloads, writeStartObject, writeKey, etc.) pass through unchanged.

The wrapper is applied in JsonpUtils.toString(JsonpSerializable, JsonpMapper, StringBuilder) and JsonpUtils.toJsonString(Object, JsonpMapper) only when ApiTypeHelper.requiredPropertiesCheckDisabled() returns true, so behavior in normal operation is unchanged.

Test

Added BehaviorsTest.testDangerousDisablePropertyCheckReferenceSerialization, modeled after the existing testDangerousDisablePropertyCheckPrimitive but omitting the required String field cluster_name from a HealthResponse.

Before this change the test fails with:

java.lang.NullPointerException: Cannot invoke "java.lang.CharSequence.length()" because "string" is null
  at org.eclipse.parsson.JsonGeneratorImpl.writeEscapedString(JsonGeneratorImpl.java:543)
  at co.elastic.clients.elasticsearch.cluster.health.HealthResponseBody.serializeInternal(HealthResponseBody.java:345)

After the change the test passes and the response renders with "cluster_name":null.

All other serialization-related unit tests in :java-client:test continue to pass.

Scope / Notes

• Covers the two JsonpUtils entry points used by toString() paths. Users who call serializable.serialize(generator, mapper) directly with their own JsonGenerator are not covered — NullSafeJsonGenerator can be made public for that use case if maintainers prefer.

[cla-checker-service]

💚 CLA has been signed

[aOvOb]

CLA signed.

[l-trotta]

Hello @aOvOb, thanks a lot for contributing! This is a good solution for the issue, there's just a little problem that needs addressing: NullSafeJsonGenerator shouldn't implement JsonGenerator, instead it should extend DelegatingJsonGenerator.

DelegatingJsonGenerator wraps JsonGenerator and it's used by both (2 and 3) Jackson mappers to handle custom objects (like Maps or POJOs in general), so the current implementation would fail here, where it would have to unwrap the DelegatingJsonGenerator twice to find the actual generator.

You can test this yourself running:

    @Test
    public void testToJsonStringWithMapWhileWorkaroundActive() {
        JacksonJsonpMapper mapper = new JacksonJsonpMapper();
        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));
        }
    }

Instead of changing the jackson implementation code, IMO the best way to handle this is to have NullSafeJsonGenerator extend DelegatingJsonGenerator, this way it will be the higher level wrapper and it will only have to override the nullable methods.

Another thing: the current unit tests didn't catch this behavior, so could you add those cases to the tests? Meaning Map and POJO serialization. Lastly, it would be ideal if NullSafeJsonGeneratorTest could test all json mapper implementations, this can be easily done by making it extend ModelTestCase (you can check other classes that do so as an example), so that you can call:

StringWriter writer = new StringWriter();
JsonGenerator generator = new NullSafeJsonGenerator(this.mapper.jsonProvider().createGenerator(writer));

to get a random json mapper implementation every time the unit tests are run.

Thanks again :)

[aOvOb]

Hello @l-trotta !
Thanks for the review and the detailed explanation!

I've refactored the implementation as suggested:

• NullSafeJsonGenerator now extends DelegatingJsonGenerator and only overrides the four methods handling nullable reference values (String, BigDecimal, BigInteger, JsonValue). The write(name, value) overloads are inherited and delegate through the existing writeKey + write(...) dispatch path.
• NullSafeJsonGeneratorTest now extends ModelTestCase, so the null-handling and pass-through tests are exercised against every configured JSON mapper implementation.
• Added Map and POJO serialization tests with the workaround enabled, using JacksonJsonpMapper directly (mirroring the example you shared) to cover the unwrap path through JacksonJsonpMapper.serialize.

Thanks again for the guidance :)

[l-trotta]

@aOvOb thanks for the quick fixes, good work :) one last thing before merging: can you make the map and pojo unit test also use the random mapper? I know the bug was a jackson behavior specifically, but randomizing that test will help testing future additions too.

[l-trotta]

LGTM!