exceptionfactory commented on a change in pull request #5083: URL: https://github.com/apache/nifi/pull/5083#discussion_r635195935
########## File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/FlattenJson.java ########## @@ -157,25 +191,35 @@ public void onTrigger(final ProcessContext context, final ProcessSession session final String mode = context.getProperty(FLATTEN_MODE).getValue(); final FlattenMode flattenMode = getFlattenMode(mode); - String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(flowFile).getValue(); - + final String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(flowFile).getValue(); + final String returnType = context.getProperty(RETURN_TYPE).getValue(); + final PrintMode printMode = context.getProperty(PRETTY_PRINT).asBoolean() ? PrintMode.PRETTY : PrintMode.MINIMAL; try { - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - session.exportTo(flowFile, bos); - bos.close(); - - String raw = new String(bos.toByteArray()); - final String flattened = new JsonFlattener(raw) - .withFlattenMode(flattenMode) - .withSeparator(separator.charAt(0)) - .withStringEscapePolicy(() -> StringEscapeUtils.ESCAPE_JSON) - .flatten(); - - flowFile = session.write(flowFile, os -> os.write(flattened.getBytes())); + final StringBuilder contents = new StringBuilder(); + session.read(flowFile, in -> contents.append(IOUtils.toString(in, Charset.defaultCharset()))); + + final String resultedJson; + if (returnType.equals(RETURN_TYPE_FLATTEN)) { + resultedJson = new JsonFlattener(contents.toString()) + .withFlattenMode(flattenMode) + .withSeparator(separator.charAt(0)) + .withStringEscapePolicy(() -> StringEscapeUtils.ESCAPE_JSON) + .withPrintMode(printMode) + .flatten(); + } else { + resultedJson = new JsonUnflattener(contents.toString()) + .withFlattenMode(flattenMode) + .withSeparator(separator.charAt(0)) + .withPrintMode(printMode) + .unflatten(); + } + + flowFile = session.write(flowFile, out -> out.write(resultedJson.getBytes())); Review comment: Related to the comment on handling input, specifying a character set on String.getBytes() would clarify the expected output as opposed to relying on the system defaults. ########## File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/FlattenJson.java ########## @@ -157,25 +191,35 @@ public void onTrigger(final ProcessContext context, final ProcessSession session final String mode = context.getProperty(FLATTEN_MODE).getValue(); final FlattenMode flattenMode = getFlattenMode(mode); - String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(flowFile).getValue(); - + final String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(flowFile).getValue(); + final String returnType = context.getProperty(RETURN_TYPE).getValue(); + final PrintMode printMode = context.getProperty(PRETTY_PRINT).asBoolean() ? PrintMode.PRETTY : PrintMode.MINIMAL; try { - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - session.exportTo(flowFile, bos); - bos.close(); - - String raw = new String(bos.toByteArray()); - final String flattened = new JsonFlattener(raw) - .withFlattenMode(flattenMode) - .withSeparator(separator.charAt(0)) - .withStringEscapePolicy(() -> StringEscapeUtils.ESCAPE_JSON) - .flatten(); - - flowFile = session.write(flowFile, os -> os.write(flattened.getBytes())); + final StringBuilder contents = new StringBuilder(); + session.read(flowFile, in -> contents.append(IOUtils.toString(in, Charset.defaultCharset()))); + + final String resultedJson; + if (returnType.equals(RETURN_TYPE_FLATTEN)) { + resultedJson = new JsonFlattener(contents.toString()) + .withFlattenMode(flattenMode) + .withSeparator(separator.charAt(0)) Review comment: The separator character could be declared once outside of the conditional and reused as opposed to calling `separator.charAt(0)` in both conditional blocks. ########## File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/FlattenJson.java ########## @@ -157,25 +191,35 @@ public void onTrigger(final ProcessContext context, final ProcessSession session final String mode = context.getProperty(FLATTEN_MODE).getValue(); final FlattenMode flattenMode = getFlattenMode(mode); - String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(flowFile).getValue(); - + final String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(flowFile).getValue(); + final String returnType = context.getProperty(RETURN_TYPE).getValue(); + final PrintMode printMode = context.getProperty(PRETTY_PRINT).asBoolean() ? PrintMode.PRETTY : PrintMode.MINIMAL; try { - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - session.exportTo(flowFile, bos); - bos.close(); - - String raw = new String(bos.toByteArray()); - final String flattened = new JsonFlattener(raw) - .withFlattenMode(flattenMode) - .withSeparator(separator.charAt(0)) - .withStringEscapePolicy(() -> StringEscapeUtils.ESCAPE_JSON) - .flatten(); - - flowFile = session.write(flowFile, os -> os.write(flattened.getBytes())); + final StringBuilder contents = new StringBuilder(); + session.read(flowFile, in -> contents.append(IOUtils.toString(in, Charset.defaultCharset()))); + + final String resultedJson; + if (returnType.equals(RETURN_TYPE_FLATTEN)) { + resultedJson = new JsonFlattener(contents.toString()) + .withFlattenMode(flattenMode) + .withSeparator(separator.charAt(0)) + .withStringEscapePolicy(() -> StringEscapeUtils.ESCAPE_JSON) + .withPrintMode(printMode) + .flatten(); + } else { + resultedJson = new JsonUnflattener(contents.toString()) + .withFlattenMode(flattenMode) + .withSeparator(separator.charAt(0)) + .withPrintMode(printMode) + .unflatten(); + } + + flowFile = session.write(flowFile, out -> out.write(resultedJson.getBytes())); session.transfer(flowFile, REL_SUCCESS); - } catch (Exception ex) { + } catch (Exception e) { + getLogger().error("Failed to {} json due to {}", new Object[]{returnType, e}); Review comment: Recent updates to the logger interface now allow passing placeholder values as variable arguments. If the goal is to include the stack trace of the exception, which would be helpful, then the second placeholder should be removed from the log message string. ```suggestion getLogger().error("Failed to {} JSON", returnType, e); ``` ########## File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/FlattenJson.java ########## @@ -157,25 +191,35 @@ public void onTrigger(final ProcessContext context, final ProcessSession session final String mode = context.getProperty(FLATTEN_MODE).getValue(); final FlattenMode flattenMode = getFlattenMode(mode); - String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(flowFile).getValue(); - + final String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(flowFile).getValue(); + final String returnType = context.getProperty(RETURN_TYPE).getValue(); + final PrintMode printMode = context.getProperty(PRETTY_PRINT).asBoolean() ? PrintMode.PRETTY : PrintMode.MINIMAL; try { - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - session.exportTo(flowFile, bos); - bos.close(); - - String raw = new String(bos.toByteArray()); - final String flattened = new JsonFlattener(raw) - .withFlattenMode(flattenMode) - .withSeparator(separator.charAt(0)) - .withStringEscapePolicy(() -> StringEscapeUtils.ESCAPE_JSON) - .flatten(); - - flowFile = session.write(flowFile, os -> os.write(flattened.getBytes())); + final StringBuilder contents = new StringBuilder(); + session.read(flowFile, in -> contents.append(IOUtils.toString(in, Charset.defaultCharset()))); Review comment: Although the previous approach relied on the system default character set when converting from byte array to string, what do you think about either making UTF-8 the standard character set, or adding a new processor property to configure the character set? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org