NIFI-969 Added support for standard JSON #1179
Conversation
|
Reviewing... |
|
|
||
| runner.assertAllFlowFilesTransferred(ConvertAvroToJSON.REL_SUCCESS, 1); | ||
| final MockFlowFile out = runner.getFlowFilesForRelationship(ConvertAvroToJSON.REL_SUCCESS).get(0); | ||
| out.assertContentEquals("{\"name\":\"Alyssa\",\"favorite_number\":{\"int\":256},\"favorite_color\":null}"); |
There was a problem hiding this comment.
With standard JSON, shouldn't favorite_number be 256, not {"int":256} ?
There was a problem hiding this comment.
@mattyb149 Not sure, but that is what is outputted by JSONEncoder and as you can see it's different then the result of toString(). Perhaps it needs to be flipped where toString() is in fact standard JSON and JSONEncoder provides AVRO JSON. Thoughts?
There was a problem hiding this comment.
Yes it looks backwards as-is, @rdblue what do you think? It seems like toString() is giving standard JSON and the JSONEncoder is giving AVRO-JSON...
There was a problem hiding this comment.
JSONEncoder produces Avro JSON according to the Avro spec, and the toString produces something like it, but not quite either Avro JSON or just normal JSON you would expect. I would not rely on the toString representation because it isn't well defined and may change in future releases. I'd say the best option is to build a new conversion for normal JSON.
There was a problem hiding this comment.
Thanks @rdblue , will polish the code to reflect that
| @@ -225,4 +229,14 @@ public void process(final InputStream rawIn, final OutputStream rawOut) throws I | |||
| flowFile = session.putAttribute(flowFile, CoreAttributes.MIME_TYPE.key(), "application/json"); | |||
| session.transfer(flowFile, REL_SUCCESS); | |||
| } | |||
|
|
|||
| private byte[] toAvroJSON(Schema shcemaToUse, GenericRecord datum) throws IOException { | |||
There was a problem hiding this comment.
typo "shcemaToUse" should be "schemaToUse"
| out.write('['); | ||
| } | ||
| byte[] outputBytes = (currRecord == null) ? EMPTY_JSON_OBJECT | ||
| : (useAvroJson ? toAvroJSON(stream.getSchema(), currRecord) : genericData.toString(currRecord).getBytes(StandardCharsets.UTF_8)); |
There was a problem hiding this comment.
Looks like toString() is still being used for encoding here, Ryan recommended a different encoder for normal JSON. Not sure what that entails though, @rdblue can you elaborate on the approach? At first glance it looks like you might need to subclass JsonEncoder to handle unions differently.
| @@ -239,6 +239,37 @@ public void testSingleSchemalessAvroMessage_wrapSingleMessage_noContainer() thro | |||
| } | |||
|
|
|||
| @Test | |||
| public void testSingleSchemalessAvroMessage_wrapSingleMessage_noContainer_StandardJson() throws IOException { | |||
There was a problem hiding this comment.
The test method name says StandardJson but the USE_AVRO_JSON property is set to true in the test
| @@ -266,6 +297,34 @@ public void testMultipleAvroMessages() throws IOException { | |||
| } | |||
|
|
|||
| @Test | |||
| public void testMultipleAvroMessagesStandardJson() throws IOException { | |||
There was a problem hiding this comment.
The test method name says StandardJson but the USE_AVRO_JSON property is set to true in the test
|
I took the liberty of writing a StandardJsonEncoder that mimics the output of GenericData.toString(datum), the commit to add that to the PR is mattyb149@879efb7 @olegz @rdblue If this satisfies the concerns, please feel free to use it for this PR/Jira. |
|
The issue described in the underlying JIRA and this PR is essentially addressed or to be addressed/maintained by a new Schema Registry based transformer effort - #1436 |
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
polish