Add a JSON RecordBuilder to the GenericJsonSchema

[vroyer]

Motivation

Provide a JSON GenericRecord builder allowing to produce JsonGenericRecord from the GenericJsonSchema.

Modifications

Add a new class org.apache.pulsar.client.impl.schema.generic.JsonRecordBuilderImpl
Modify the org.apache.pulsar.client.impl.schema.generic.GenericJsonSchema.newRecordBuilder()

Verifying this change

Add a unit test in org.apache.pulsar.client.impl.schema.JSONSchemaTest

[eolivelli]

nice work
I left a few comments

[eolivelli]

LGTM

please remove the "star import", then the patch is good to go from my point of view.

@sijie @codelipenghui can you please take a look ?

[codelipenghui]

Why not use the ObjectNode directly? Use the ObjectMapper to generate the JsonNode also create an internal map there,

Looks like:

ObjectNode node = JsonNodeFactory.instance.objectNode();
node.putPOJO("a", "a");

So that we don't need to create a HashMap for each JsonRecoredBuilder.

And we can reuse the ObjectNode by adding it to the ThreadLocal? We can call ObjectNode.removeAll() after build the GenericRecord.

[eolivelli]

@codelipenghui
I prefer to not use ThreadLocal, as it will complicate the usage, as the user is not able to have two builder on the same thread, with the risk of mixing data from two different records.

We can have a simple implementation now and enhance it when needed.
Creating just one object is not a big deal for the JVM.

we do not have such complexity for the AVRO record builder

pulsar/pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/generic/AvroRecordBuilderImpl.java

Line 29 in 5955930

class AvroRecordBuilderImpl implements GenericRecordBuilder {

[sijie]

Jackson already have a pretty good json node factory to build json node. We should use it instead of building the logic again that Jackson already did.

[vroyer]

The ObjectNode builder also uses a Map to build a JsonNode (a LinkedHashMap), but requires to call the a dedicated put() method for each native types or putPOJO() for Objects (like in such a code).

The ObjectMapper.valueToTree(map) produce the same result, a JsonNode, and the underlying serialiser properly deals with native and POJO objects in the same way without serializing it into "basic" nodes. The ObjectNode allows to filter or alter nodes, but we don't need here in the build() method, so the ObjectMapper.valueToTree() it is simple and efficient here.

Moreover, the ObjectMapper is thread-safe and a static instance is enough to do the job.

[sijie]

Jackson already have a pretty good json node factory to build json node. We should use it instead of building the logic again that Jackson already did.

[eolivelli]

@sijie as you told that you are okay with this patch, can you please click on "Approve" ? otherwise we cannot merge it