[JOHNZON-278] Return constant instead of new instance of empty block / empty array

[mkarg]

Even in these days of modern Java, reducing memory consumption, GC stress and memory fragmentation, are beneficial side aspects.

If makes no sense to create new empty instances of JsonArray and JsonObject again and again. For this case, JSON-P provides special constants.

It makes sense to return these constants always.

[mkarg]

This PR has to stay in draft mode as long as Johnzon depends on Apache Geronimo's JSON-P buggy JSON-P API JARs: In contrast to the official Jakarta JSON-P API JARs, those Apache files implement the empty JsonObject constant in a way which is incompatible with JSON-P's JsonObject JavaDocs. As a result, three of Johnzon Core's unit tests will fail. Once Geronimo's JSON-P files are bug-fixed, or once Johnzon Core switches to the official JSON-P files, this PR can be merged safely, as all other tests do pass.

[rmannibucau]

@mkarg can you detail the geronimo issue? Two things to take into account: johnzon must work with both API (so the option to switch to jakarta is not really one we can take here) and geronimo passed the TCK so I'm not sure what you mean, can you precise it?

Side note: our empty object impl is available at https://github.com/apache/geronimo-specs/blob/trunk/geronimo-json_1.1_spec/src/main/java/javax/json/EmptyJsonObject.java and was done to make the impl independent of any impl.

[mkarg]

@mkarg can you detail the geronimo issue?

JsonObjectImplTest.testGetStringMissingKeyShouldThrowNullPointerException Expected exception: java.lang.NullPointerException:

• The JSON-P API requires JsonObject.getString("missing") to throw NullPointerException for non-existent key "missing".
• Johnzon Core's JsonObjectImplTest correctly tests this behavior.
• Jakarta's original JSON-P API (EmptyObject) correctly throws NullPointerException .
• Geronimo's variant of the JSON-P API (EmptyJsonObject) incorrectly returns null instead of throwing an exception.

JsonObjectImplTest.testGetStringWithDefaultShouldReturnDefault:146 expected:<default> but was:<null>:

• The JSON-P API requires JsonObject.getString("missing", "default") to return "default" for non-existent key "missing".
• Johnzon Core's JsonObjectImplTest correctly tests this behavior.
• Jakarta's original JSON-P API (EmptyObject) correctly returns "default".
• Geronimo's variant of the JSON-P API (EmptyJsonObject) incorrectly returns null instead of "default".

JsonObjectImplTest.testIsNullMissingKeyShouldThrowNullPointerException Expected exception: java.lang.NullPointerException:

• The JSON-P API requires JsonObject.isNull("missing") to throw NullPointerException for non-existent key "missing".
• Johnzon Core's JsonObjectImplTest correctly tests this behavior.
• Jakarta's original JSON-P API (EmptyObject) correctly throws NullPointerException.
• Geronimo's variant of the JSON-P API (EmptyJsonObject) incorrectly returns true instead of throwing an exception.

As a result the Johnzon tests fail when runnign against Geronimo's variant of JSON-P API while it passes when running against Jakarta's original JSON-P API.

A short term solution could be to temporarily switch from Geronimo to Jakarta until Geronimo published a fix for their variant of the JSON-P API.

[mkarg]

See Apache Geronimo Bug Report and Apache Geronimo PR.

[mkarg]

geronimo passed the TCK

Then apparently the JSON-P TCK is incomplete or is buggy or is not meant to test the API JAR itself (here: Geronimo Spec) but just the implementing product (Here: Johnzon Core). Possibly the TCK was authored with the idea that the original API JAR is used always, so there wouldn't be a need to test itself. Also the TCK is just intended to certify compliance to the rules laid out be Spec document and API, but does not guarantee general bug-freeness of that product.

Side note: our empty object impl is available at https://github.com/apache/geronimo-specs/blob/trunk/geronimo-json_1.1_spec/src/main/java/javax/json/EmptyJsonObject.java and was done to make the impl independent of any impl.

Maybe there is a misunderstanding: Jakarta JSON-P API is not an implementation of JSON-P, it is the JSON-P spec itself. It already is independent of any implementing product.

[rmannibucau]

Will merge it next week

Side note: no misunderstanding, the spec defined constant so wrongly put an impl in the API but no worries Geronimo PR will be merged within some hours.

[rmannibucau]

@mkarg https://repository.apache.org/content/repositories/snapshots/org/apache/geronimo/specs/geronimo-json_1.1_spec/1.3-SNAPSHOT/geronimo-json_1.1_spec-1.3-20190923.065842-1.pom is available now with your fix, can you update the PR please to let us merge it?

side note: just thinking about it: we can also define constants in johnzon, would avoid to wait for a new API release and still be compliant, no strong preference to be honest.

[mkarg]

@rmannibucau Damned, removed the draft state too hastily -- actually many other Johnzon tests will fail when using Geronimo 1.3-SNAPSHOT.

[mkarg]

we can also define constants in johnzon, would avoid to wait for a new API release and still be compliant, no strong preference to be honest.

As this PR is just about an enhancement in master I do not see a need for that. I think it is good to wait for the updated dependency.

[mkarg]

@rmannibucau Damned, removed the draft state too hastily -- actually many other Johnzon tests will fail when using Geronimo 1.3-SNAPSHOT.

Ah, the trouble comes from the Apache Repository. It does only conaint the actual API JAR, but not the `-source' and '-javadoc' jars, so the build fails because Maven wants to have those. Can you please put all artifacts into Apache Repository, instead of just the API JAR? That would be great. Thanks! :-)

[mkarg]

Off-Topic: Too bad that it is not possible to switch a PR back to draft mode. :-(

[rmannibucau]

@mkarg should be updated now, -U ;)

[mkarg]

Works fine now, thanks a lot! :-)

[mkarg]

@rmannibucau All tests pass. PR is ready to merge now, really. :-)

[mkarg]

@rmannibucau Thanks a lot for merging! 👍

Question: I noticed that the latest 1.2.0-SNAPSHOT on https://repository.apache.org/content/repositories/snapshots/org/apache/johnzon/johnzon-core/1.2.0-SNAPSHOT/ is from August 20. Isn't there a nightly build job?