NIFI-15856 - JsonRecordSetWriter silently ignores Timestamp Format#11158
NIFI-15856 - JsonRecordSetWriter silently ignores Timestamp Format#11158pvillard31 wants to merge 2 commits intoapache:mainfrom
Conversation
exceptionfactory
left a comment
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for implementing this new property to clarify behavior @pvillard31. I noted a few minor things, but this looks close to completion.
| assertTrue(output.contains("\"ignoredExtra\":\"preserved\""), | ||
| "Default constructor preserves legacy fast-path behavior: raw bytes should be emitted verbatim"); |
There was a problem hiding this comment.
Should the output be equal, so that the strings can be compared, instead of checking for contains()?
| final ByteArrayOutputStream fastPathBaos = new ByteArrayOutputStream(); | ||
| try (final WriteJsonResult writer = new WriteJsonResult(Mockito.mock(ComponentLog.class), schema, new SchemaNameAsAttribute(), fastPathBaos, false, | ||
| NullSuppression.NEVER_SUPPRESS, OutputGrouping.OUTPUT_ARRAY, RecordFieldType.DATE.getDefaultFormat(), | ||
| RecordFieldType.TIME.getDefaultFormat(), "yyyy-MM-dd'T'HH:mm:ss.SSSX", |
There was a problem hiding this comment.
It would be helpful to declare the date format in this method and use it in both places.
| final Map<String, Object> values = new HashMap<>(); | ||
| values.put("event", eventTimestamp); | ||
|
|
||
| final String rawForm = "{\"event\":\"2025-03-20T17:33:11.000+0000\"}"; |
There was a problem hiding this comment.
It would be helpful to declare the timestamp string and use String formatting here.
| .description(""" | ||
| Controls a throughput optimization that only applies to pure JSON pass-through flows. When set to true, and all of the \ | ||
| following conditions are met, the writer will emit the record's original JSON bytes verbatim instead of re-serializing from typed field \ | ||
| values: (1) the upstream reader is JsonTreeReader, (2) no data change, (3) the reader's and writer's record schemas are identical, \ |
There was a problem hiding this comment.
Would the JsonPathReader also work? The details are helpful, but I also wonder whether it is necessary to be this detailed.
| public static final PropertyDescriptor REUSE_INPUT_SERIALIZATION = new PropertyDescriptor.Builder() | ||
| .name("Reuse Input Serialization") |
There was a problem hiding this comment.
Minor naming, but perhaps Use Input Serialization would be better?
|
Thanks for the review @exceptionfactory - I pushed a commit to address your comments. Let me know what you think about the new property description. |
2 participants
Summary
NIFI-15856 - JsonRecordSetWriter silently ignores Timestamp Format
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation