exceptionfactory commented on code in PR #8417: URL: https://github.com/apache/nifi/pull/8417#discussion_r1510159047
########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/EncodingType.java: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum EncodingType implements DescribedValue { + BASE64_ENCODING("Base64", "Sets the encoding type to 'Base64'."), Review Comment: ```suggestion BASE64("Base64", "Encode or decode using Base64 set of characters"), ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/EncodingType.java: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum EncodingType implements DescribedValue { + BASE64_ENCODING("Base64", "Sets the encoding type to 'Base64'."), + BASE32_ENCODING("Base32", "Sets the encoding type to 'Base32'."), Review Comment: ```suggestion BASE32("Base32", "Encode or decode using Base32 set of characters"), ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/EncodeContent.java: ########## @@ -129,31 +173,35 @@ public void onTrigger(final ProcessContext context, final ProcessSession session } } - private static StreamCallback getStreamCallback(final boolean encode, final String encoding) { - if (encode) { - if (encoding.equalsIgnoreCase(BASE64_ENCODING)) { - return new EncodeBase64(); - } else if (encoding.equalsIgnoreCase(BASE32_ENCODING)) { - return new EncodeBase32(); - } else { - return new EncodeHex(); - } - } else { - if (encoding.equalsIgnoreCase(BASE64_ENCODING)) { - return new DecodeBase64(); - } else if (encoding.equalsIgnoreCase(BASE32_ENCODING)) { - return new DecodeBase32(); - } else { - return new DecodeHex(); - } + private static StreamCallback getStreamCallback(final boolean encode, final EncodingType encoding, + final int lineLength, final String lineSeparator) { + switch(encoding) { + case BASE64_ENCODING: + return encode ? new EncodeBase64(lineLength, lineSeparator) : new DecodeBase64(); + case BASE32_ENCODING: + return encode ? new EncodeBase32(lineLength, lineSeparator) : new DecodeBase32(); + default: + return encode ? new EncodeHex() : new DecodeHex(); } } private static class EncodeBase64 implements StreamCallback { + private int lineLength; + private String lineSeparator; + + public EncodeBase64(final int lineLength, Review Comment: ```suggestion private EncodeBase64(final int lineLength, ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/EncodingType.java: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum EncodingType implements DescribedValue { + BASE64_ENCODING("Base64", "Sets the encoding type to 'Base64'."), + BASE32_ENCODING("Base32", "Sets the encoding type to 'Base32'."), + HEX_ENCODING("Hexadecimal", "Sets the encoding type to 'Hexadecimal'."); Review Comment: ```suggestion HEXADECIMAL("Hexadecimal", "Encode or decode using hexadecimal set of characters"); ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/LineOutputMode.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum LineOutputMode implements DescribedValue { + SINGLE_LINE("Single Line", "The encoded FlowFile content will be output as a single line."), + MULTIPLE_LINES("Multiple Lines", "The encoded FlowFile content will be output as multiple lines."); Review Comment: ```suggestion MULTIPLE_LINES("Multiple Lines", "The encoded content will be written as multiple lines."); ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/EncodingMode.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum EncodingMode implements DescribedValue { + ENCODE("Encode", "Sets the operation mode to 'encode'."), + DECODE("Decode", "Sets the operation mode to 'decode'."); Review Comment: ```suggestion DECODE("Decode", "Transform encoded input to original representation"); ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/LineOutputMode.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum LineOutputMode implements DescribedValue { + SINGLE_LINE("Single Line", "The encoded FlowFile content will be output as a single line."), Review Comment: ```suggestion SINGLE_LINE("Single Line", "The encoded content will be written as a single line."), ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestEncodeContent.java: ########## @@ -70,96 +102,310 @@ public void testFailDecodeNotBase64() throws IOException { } @Test - public void testFailDecodeNotBase64ButIsAMultipleOfFourBytes() { - final TestRunner testRunner = TestRunners.newTestRunner(new EncodeContent()); + void testEncodeDecodeSpecialCharsBase64() { + final String specialChars = "!@#$%^&*()_+{}:\"<>?[];',./~`-="; + final String expectedOutput = "IUAjJCVeJiooKV8re306Ijw+P1tdOycsLi9+YC09" + System.lineSeparator(); - testRunner.setProperty(EncodeContent.MODE, EncodeContent.DECODE_MODE); - testRunner.setProperty(EncodeContent.ENCODING, EncodeContent.BASE64_ENCODING); - - testRunner.enqueue("four@@@@multiple".getBytes()); - testRunner.clearTransferState(); - testRunner.run(); + executeTestSuccessHelper(EncodingMode.ENCODE, EncodingType.BASE64_ENCODING, specialChars, expectedOutput); + testRunner.clearTransferState(); // clear the state for the next test + executeTestSuccessHelper(EncodingMode.DECODE, EncodingType.BASE64_ENCODING, expectedOutput, specialChars); + } - testRunner.assertAllFlowFilesTransferred(EncodeContent.REL_FAILURE, 1); + @ParameterizedTest + @MethodSource("encodeBase32Args") + void testBasicDecodeBase32(final String input, final String expectedOutput) { + // we'll use the same args from `encodeBase32Args`, only flip around input and output Review Comment: Recommend removing the use of colloquial `we'll`: ```suggestion // use the same args from `encodeBase32Args`, only flip around input and output ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/EncodingMode.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum EncodingMode implements DescribedValue { + ENCODE("Encode", "Sets the operation mode to 'encode'."), Review Comment: Recommend adjusting the description: ```suggestion ENCODE("Encode", "Transform original input to encoded representation"), ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/EncodingMode.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum EncodingMode implements DescribedValue { + ENCODE("Encode", "Sets the operation mode to 'encode'."), + DECODE("Decode", "Sets the operation mode to 'decode'."); + + EncodingMode(String displayName, String description) { + this.displayName = displayName; + this.description = description; + } + + private final String displayName; + private final String description; + + @Override + public String getValue() { + return name(); Review Comment: The `name()` needs to be changed, because it would return `DECODE` instead of `Decode`, which is the current allowable value. Recommend changing the `displayName` property to `value` and returning `value` in both cases. ```suggestion return value; ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/EncodeContent.java: ########## @@ -29,55 +37,88 @@ import org.apache.nifi.annotation.documentation.CapabilityDescription; import org.apache.nifi.annotation.documentation.Tags; import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.components.Validator; +import org.apache.nifi.expression.ExpressionLanguageScope; import org.apache.nifi.flowfile.FlowFile; import org.apache.nifi.processor.AbstractProcessor; import org.apache.nifi.processor.ProcessContext; import org.apache.nifi.processor.ProcessSession; import org.apache.nifi.processor.Relationship; import org.apache.nifi.processor.io.StreamCallback; +import org.apache.nifi.processor.util.StandardValidators; +import org.apache.nifi.processors.standard.encoding.EncodingMode; +import org.apache.nifi.processors.standard.encoding.EncodingType; +import org.apache.nifi.processors.standard.encoding.LineOutputMode; import org.apache.nifi.processors.standard.util.ValidatingBase32InputStream; import org.apache.nifi.processors.standard.util.ValidatingBase64InputStream; import org.apache.nifi.stream.io.StreamUtils; import org.apache.nifi.util.StopWatch; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.util.Arrays; -import java.util.List; -import java.util.Set; -import java.util.concurrent.TimeUnit; - @SideEffectFree @SupportsBatching @InputRequirement(Requirement.INPUT_REQUIRED) @Tags({"encode", "decode", "base64", "base32", "hex"}) @CapabilityDescription("Encode or decode the contents of a FlowFile using Base64, Base32, or hex encoding schemes") public class EncodeContent extends AbstractProcessor { - public static final String ENCODE_MODE = "Encode"; - public static final String DECODE_MODE = "Decode"; - - public static final String BASE64_ENCODING = "base64"; - public static final String BASE32_ENCODING = "base32"; - public static final String HEX_ENCODING = "hex"; - public static final PropertyDescriptor MODE = new PropertyDescriptor.Builder() .name("Mode") .description("Specifies whether the content should be encoded or decoded") .required(true) - .allowableValues(ENCODE_MODE, DECODE_MODE) - .defaultValue(ENCODE_MODE) + .allowableValues(EncodingMode.class) + .defaultValue(EncodingMode.ENCODE) .build(); public static final PropertyDescriptor ENCODING = new PropertyDescriptor.Builder() .name("Encoding") .description("Specifies the type of encoding used") .required(true) - .allowableValues(BASE64_ENCODING, BASE32_ENCODING, HEX_ENCODING) - .defaultValue(BASE64_ENCODING) + .allowableValues(EncodingType.class) + .defaultValue(EncodingType.BASE64_ENCODING) + .build(); + + static final PropertyDescriptor LINE_OUTPUT_MODE = new PropertyDescriptor.Builder() + .name("Line Output Mode") + .displayName("Line Output Mode") + .description("If set to 'single-line', the encoded FlowFile content will output as a single line. If set to 'multiple-lines', " + + "it will output as multiple lines. This property is only applicable when Base64 or Base32 encoding is selected.") Review Comment: This conditional nature of the property is already rendered automatically. Recommend the following description: ```suggestion .description("Controls the line formatting for encoded content based on selected property values.") ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/EncodingType.java: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum EncodingType implements DescribedValue { + BASE64_ENCODING("Base64", "Sets the encoding type to 'Base64'."), + BASE32_ENCODING("Base32", "Sets the encoding type to 'Base32'."), + HEX_ENCODING("Hexadecimal", "Sets the encoding type to 'Hexadecimal'."); + + private final String displayName; + private final String description; + + EncodingType(String displayName, String description) { + this.displayName = displayName; + this.description = description; + } Review Comment: In this case, the current allowable values are different, so I recommend creating a new `value` member variable and assigning it based on the current strings, which are `base64`, `base32`, and `hex`. ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestEncodeContent.java: ########## @@ -70,96 +102,310 @@ public void testFailDecodeNotBase64() throws IOException { } @Test - public void testFailDecodeNotBase64ButIsAMultipleOfFourBytes() { - final TestRunner testRunner = TestRunners.newTestRunner(new EncodeContent()); + void testEncodeDecodeSpecialCharsBase64() { + final String specialChars = "!@#$%^&*()_+{}:\"<>?[];',./~`-="; + final String expectedOutput = "IUAjJCVeJiooKV8re306Ijw+P1tdOycsLi9+YC09" + System.lineSeparator(); - testRunner.setProperty(EncodeContent.MODE, EncodeContent.DECODE_MODE); - testRunner.setProperty(EncodeContent.ENCODING, EncodeContent.BASE64_ENCODING); - - testRunner.enqueue("four@@@@multiple".getBytes()); - testRunner.clearTransferState(); - testRunner.run(); + executeTestSuccessHelper(EncodingMode.ENCODE, EncodingType.BASE64_ENCODING, specialChars, expectedOutput); + testRunner.clearTransferState(); // clear the state for the next test + executeTestSuccessHelper(EncodingMode.DECODE, EncodingType.BASE64_ENCODING, expectedOutput, specialChars); + } - testRunner.assertAllFlowFilesTransferred(EncodeContent.REL_FAILURE, 1); + @ParameterizedTest + @MethodSource("encodeBase32Args") + void testBasicDecodeBase32(final String input, final String expectedOutput) { + // we'll use the same args from `encodeBase32Args`, only flip around input and output + executeTestSuccessHelper(EncodingMode.DECODE, EncodingType.BASE32_ENCODING, expectedOutput, input); } - @Test - public void testBase32RoundTrip() throws IOException { - final TestRunner testRunner = TestRunners.newTestRunner(new EncodeContent()); + @ParameterizedTest + @MethodSource("encodeBase64Args") + void testBasicDecodeBase64(final String input, final String expectedOutput) { + // we'll use the same args from `encodeBase64Args`, only flip around input and output + executeTestSuccessHelper(EncodingMode.DECODE, EncodingType.BASE64_ENCODING, expectedOutput, input); + } - testRunner.setProperty(EncodeContent.MODE, EncodeContent.ENCODE_MODE); - testRunner.setProperty(EncodeContent.ENCODING, EncodeContent.BASE32_ENCODING); + @ParameterizedTest + @MethodSource("encodeHexArgs") + void testBasicDecodeHex(final String input, final String expectedOutput) { + // we'll use the same args from `encodeHexArgs`, only flip around input and output + executeTestSuccessHelper(EncodingMode.DECODE, EncodingType.HEX_ENCODING, expectedOutput, input); + } - testRunner.enqueue(FILE_PATH); - testRunner.clearTransferState(); - testRunner.run(); + @ParameterizedTest + @MethodSource("encodeHexArgs") + void testBasicEncodeHex(final String input, final String expectedOutput) { + executeTestSuccessHelper(EncodingMode.ENCODE, EncodingType.HEX_ENCODING, input, expectedOutput); + } - testRunner.assertAllFlowFilesTransferred(EncodeContent.REL_SUCCESS, 1); + private static Stream<Arguments> encodeHexArgs() { + return Stream.of( + Arguments.of("hello", "68656C6C6F"), + Arguments.of("foo", "666F6F"), + Arguments.of("你好", "E4BDA0E5A5BD"), + Arguments.of("Здравствуйте", "D097D0B4D180D0B0D0B2D181D182D0B2D183D0B9D182D0B5") + ); + } + + @ParameterizedTest + @MethodSource("encodeBase32Args") + void testBasicEncodeBase32(final String input, final String expectedOutput) { + executeTestSuccessHelper(EncodingMode.ENCODE, EncodingType.BASE32_ENCODING, input, expectedOutput); + } - MockFlowFile flowFile = testRunner.getFlowFilesForRelationship(EncodeContent.REL_SUCCESS).get(0); - testRunner.assertQueueEmpty(); + private static Stream<Arguments> encodeBase32Args() { + return Stream.of( + Arguments.of("hello", "NBSWY3DP" + System.lineSeparator()), + Arguments.of("foo", "MZXW6===" + System.lineSeparator()), + Arguments.of("你好", "4S62BZNFXU======" + System.lineSeparator()), + Arguments.of("Здравствуйте", "2CL5BNGRQDILBUFS2GA5DAWQWLIYHUFZ2GBNBNI=" + System.lineSeparator()) + ); + } + + @ParameterizedTest + @MethodSource("encodeBase64Args") + void testBasicEncodeBase64(final String input, final String expectedOutput) { + executeTestSuccessHelper(EncodingMode.ENCODE, EncodingType.BASE64_ENCODING, input, expectedOutput); + } - testRunner.setProperty(EncodeContent.MODE, EncodeContent.DECODE_MODE); - testRunner.enqueue(flowFile); - testRunner.clearTransferState(); - testRunner.run(); - testRunner.assertAllFlowFilesTransferred(EncodeContent.REL_SUCCESS, 1); + private static Stream<Arguments> encodeBase64Args() { + return Stream.of( + Arguments.of("hello", "aGVsbG8=" + System.lineSeparator()), + Arguments.of("foo", "Zm9v" + System.lineSeparator()), + Arguments.of("你好", "5L2g5aW9" + System.lineSeparator()), + Arguments.of("Здравствуйте", "0JfQtNGA0LDQstGB0YLQstGD0LnRgtC1" + System.lineSeparator()) + ); + } - flowFile = testRunner.getFlowFilesForRelationship(EncodeContent.REL_SUCCESS).get(0); - flowFile.assertContentEquals(FILE_PATH); + @Test + void testBlankValueShouldNotFail() { + executeTestSuccessHelper(EncodingMode.ENCODE, + EncodingType.BASE64_ENCODING, + StringUtils.EMPTY, + StringUtils.EMPTY); } @Test - public void testFailDecodeNotBase32() throws IOException { - final TestRunner testRunner = TestRunners.newTestRunner(new EncodeContent()); + void testEncodeContentMultipleLinesBase64() { + // this input is greater than 57 bytes, sure to generate multiple lines in base64 + final String expectedOutput = "TG9yZW0gaXBzdW0gZG9sb3Igc2l0IGFtZXQsIGNvbnNlY3RldHVyIGFkaXBpc2NpbmcgZWxpdCwg" + System.lineSeparator() + + "c2VkIGRvIGVpdXNtb2QgdGVtcG9yIGluY2lkaWR1bnQgdXQgbGFib3JlIGV0IGRvbG9yZSBtYWdu" + System.lineSeparator() + + "YSBhbGlxdWEu" + System.lineSeparator(); + + // Execute the test using the helper method + executeTestHelper(EncodingMode.ENCODE, + EncodingType.BASE64_ENCODING, + LOREM_IPSUM, + LineOutputMode.MULTIPLE_LINES, + expectedOutput, + EncodeContent.REL_SUCCESS); + } - testRunner.setProperty(EncodeContent.MODE, EncodeContent.DECODE_MODE); - testRunner.setProperty(EncodeContent.ENCODING, EncodeContent.BASE32_ENCODING); + @Test + void testEncodeContentSingleLineBase64() { + // this input is greater than 57 bytes, sure to generate multiple lines in base64 + final String expectedOutput = "TG9yZW0gaXBzdW0gZG9sb3Igc2l0IGFtZXQsIGNvbnNlY3RldHVyIGFkaXBpc2NpbmcgZWxpdCwg" + + "c2VkIGRvIGVpdXNtb2QgdGVtcG9yIGluY2lkaWR1bnQgdXQgbGFib3JlIGV0IGRvbG9yZSBtYWdu" + + "YSBhbGlxdWEu"; + + // Execute the test using the helper method + executeTestHelper(EncodingMode.ENCODE, + EncodingType.BASE64_ENCODING, + LOREM_IPSUM, + LineOutputMode.SINGLE_LINE, + expectedOutput, + EncodeContent.REL_SUCCESS); + } - testRunner.enqueue(FILE_PATH); - testRunner.clearTransferState(); - testRunner.run(); + @Test + void testEncodeContentSingleLineBase32() { + // this input is greater than 57 bytes, sure to generate multiple lines in base64 + final String expectedOutput = "JRXXEZLNEBUXA43VNUQGI33MN5ZCA43JOQQGC3LFOQWCAY3PNZZWKY3UMV2HK4RAMFSGS4DJONRWS3THEBSWY2LUF" + + "QQHGZLEEBSG6IDFNF2XG3LPMQQHIZLNOBXXEIDJNZRWSZDJMR2W45BAOV2CA3DBMJXXEZJAMV2CAZDPNRXXEZJANVQWO3TBEBQWY2LROVQS4==="; + + // Execute the test using the helper method + executeTestHelper(EncodingMode.ENCODE, + EncodingType.BASE32_ENCODING, + LOREM_IPSUM, + LineOutputMode.SINGLE_LINE, + expectedOutput, + EncodeContent.REL_SUCCESS); + } - testRunner.assertAllFlowFilesTransferred(EncodeContent.REL_FAILURE, 1); + @Test + void testEncodeContentMultipleLinesBase32() { + // this input is greater than 57 bytes, sure to generate multiple lines in base64 + final String expectedOutput = "JRXXEZLNEBUXA43VNUQGI33MN5ZCA43JOQQGC3LFOQWCAY3PNZZWKY3UMV2HK4RAMFSGS4DJ" + System.lineSeparator() + + "ONRWS3THEBSWY2LUFQQHGZLEEBSG6IDFNF2XG3LPMQQHIZLNOBXXEIDJNZRWSZDJMR2W45BA" + System.lineSeparator() + + "OV2CA3DBMJXXEZJAMV2CAZDPNRXXEZJANVQWO3TBEBQWY2LROVQS4===" + System.lineSeparator(); + + // Execute the test using the helper method + executeTestHelper(EncodingMode.ENCODE, + EncodingType.BASE32_ENCODING, + LOREM_IPSUM, + LineOutputMode.MULTIPLE_LINES, // set false to output multiple lines + expectedOutput, + EncodeContent.REL_SUCCESS); } @Test - public void testHexRoundTrip() throws IOException { - final TestRunner testRunner = TestRunners.newTestRunner(new EncodeContent()); + void testEncodeContentMultipleLinesNonStandardLengthBase32() { + // this input is greater than 57 bytes, sure to generate multiple lines in base64 + final String expectedOutput = "JRXXEZLNEBUXA43VNUQGI33MN5ZCA43JOQQGC3LFOQWCAY3PNZZWKY3UMV2HK4RAMFSGS4DJONRWS3TH" + System.lineSeparator() + + "EBSWY2LUFQQHGZLEEBSG6IDFNF2XG3LPMQQHIZLNOBXXEIDJNZRWSZDJMR2W45BAOV2CA3DBMJXXEZJA" + System.lineSeparator() + + "MV2CAZDPNRXXEZJANVQWO3TBEBQWY2LROVQS4===" + System.lineSeparator(); + + // Execute the test using the helper method + executeTestHelper(EncodingMode.ENCODE, + EncodingType.BASE32_ENCODING, + LOREM_IPSUM, + LineOutputMode.MULTIPLE_LINES, + 80, + System.lineSeparator(), + expectedOutput, + EncodeContent.REL_SUCCESS); + } - testRunner.setProperty(EncodeContent.MODE, EncodeContent.ENCODE_MODE); - testRunner.setProperty(EncodeContent.ENCODING, EncodeContent.HEX_ENCODING); + @Test + void testThatLineLengthIsIgnoredIfSingleLineOutputTrueBase32() { + // this input is greater than 57 bytes, sure to generate multiple lines in base64 + final String expectedOutput = "JRXXEZLNEBUXA43VNUQGI33MN5ZCA43JOQQGC3LFOQWCAY3PNZZWKY3UMV2HK4RAMFSGS4DJONRWS3THEBSWY2LUFQQHGZLEEB" + + "SG6IDFNF2XG3LPMQQHIZLNOBXXEIDJNZRWSZDJMR2W45BAOV2CA3DBMJXXEZJAMV2CAZDPNRXXEZJANVQWO3TBEBQWY2LROVQS4==="; + + // Setting a low value for `lineLength` but single line true ensures that `lineLength` is ignored + executeTestHelper(EncodingMode.ENCODE, + EncodingType.BASE32_ENCODING, + LOREM_IPSUM, + LineOutputMode.SINGLE_LINE, + 2, // set a low value >= 0 Review Comment: Recommend removing the extra spaces. ```suggestion 2, // set a low value >= 0 ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/EncodingMode.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum EncodingMode implements DescribedValue { + ENCODE("Encode", "Sets the operation mode to 'encode'."), + DECODE("Decode", "Sets the operation mode to 'decode'."); + + EncodingMode(String displayName, String description) { + this.displayName = displayName; + this.description = description; + } + + private final String displayName; Review Comment: ```suggestion EncodingMode(String displayName, String description) { this.value = value; this.description = description; } private final String value; ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/EncodeContent.java: ########## @@ -29,55 +37,88 @@ import org.apache.nifi.annotation.documentation.CapabilityDescription; import org.apache.nifi.annotation.documentation.Tags; import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.components.Validator; +import org.apache.nifi.expression.ExpressionLanguageScope; import org.apache.nifi.flowfile.FlowFile; import org.apache.nifi.processor.AbstractProcessor; import org.apache.nifi.processor.ProcessContext; import org.apache.nifi.processor.ProcessSession; import org.apache.nifi.processor.Relationship; import org.apache.nifi.processor.io.StreamCallback; +import org.apache.nifi.processor.util.StandardValidators; +import org.apache.nifi.processors.standard.encoding.EncodingMode; +import org.apache.nifi.processors.standard.encoding.EncodingType; +import org.apache.nifi.processors.standard.encoding.LineOutputMode; import org.apache.nifi.processors.standard.util.ValidatingBase32InputStream; import org.apache.nifi.processors.standard.util.ValidatingBase64InputStream; import org.apache.nifi.stream.io.StreamUtils; import org.apache.nifi.util.StopWatch; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.util.Arrays; -import java.util.List; -import java.util.Set; -import java.util.concurrent.TimeUnit; - @SideEffectFree @SupportsBatching @InputRequirement(Requirement.INPUT_REQUIRED) @Tags({"encode", "decode", "base64", "base32", "hex"}) @CapabilityDescription("Encode or decode the contents of a FlowFile using Base64, Base32, or hex encoding schemes") public class EncodeContent extends AbstractProcessor { - public static final String ENCODE_MODE = "Encode"; - public static final String DECODE_MODE = "Decode"; - - public static final String BASE64_ENCODING = "base64"; - public static final String BASE32_ENCODING = "base32"; - public static final String HEX_ENCODING = "hex"; - public static final PropertyDescriptor MODE = new PropertyDescriptor.Builder() .name("Mode") .description("Specifies whether the content should be encoded or decoded") .required(true) - .allowableValues(ENCODE_MODE, DECODE_MODE) - .defaultValue(ENCODE_MODE) + .allowableValues(EncodingMode.class) + .defaultValue(EncodingMode.ENCODE) .build(); public static final PropertyDescriptor ENCODING = new PropertyDescriptor.Builder() .name("Encoding") .description("Specifies the type of encoding used") .required(true) - .allowableValues(BASE64_ENCODING, BASE32_ENCODING, HEX_ENCODING) - .defaultValue(BASE64_ENCODING) + .allowableValues(EncodingType.class) + .defaultValue(EncodingType.BASE64_ENCODING) + .build(); + + static final PropertyDescriptor LINE_OUTPUT_MODE = new PropertyDescriptor.Builder() + .name("Line Output Mode") + .displayName("Line Output Mode") + .description("If set to 'single-line', the encoded FlowFile content will output as a single line. If set to 'multiple-lines', " + + "it will output as multiple lines. This property is only applicable when Base64 or Base32 encoding is selected.") + .required(false) + .defaultValue(LineOutputMode.SINGLE_LINE) + .allowableValues(LineOutputMode.class) + .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) + .dependsOn(MODE, EncodingMode.ENCODE) + .dependsOn(ENCODING, EncodingType.BASE64_ENCODING, EncodingType.BASE32_ENCODING) .build(); + static final PropertyDescriptor ENCODED_LINE_SEPARATOR = new PropertyDescriptor.Builder() + .name("Encoded Content Line Separator") + .displayName("Encoded Content Line Separator") + .description("Each line of encoded data will be terminated with this byte sequence (e.g. \\r\\n" + + "). This property defaults to the system-dependent line separator string. If `line-length` <= 0, " + + "the `line-separator` property is not used. This property is not used for `hex` encoding.") + .required(false) + .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) + .defaultValue(System.lineSeparator()) + .addValidator(Validator.VALID) + .dependsOn(MODE, EncodingMode.ENCODE) + .dependsOn(ENCODING, EncodingType.BASE64_ENCODING, EncodingType.BASE32_ENCODING) + .build(); Review Comment: Is this property necessary? It seems best to hard-code the line-feed character `\n`, rather than making this platform-dependent or configurable. ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/encoding/EncodingMode.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.processors.standard.encoding; + +import org.apache.nifi.components.DescribedValue; + +public enum EncodingMode implements DescribedValue { + ENCODE("Encode", "Sets the operation mode to 'encode'."), + DECODE("Decode", "Sets the operation mode to 'decode'."); + + EncodingMode(String displayName, String description) { + this.displayName = displayName; + this.description = description; + } + + private final String displayName; + private final String description; + + @Override + public String getValue() { + return name(); + } + + @Override + public String getDisplayName() { + return displayName; Review Comment: ```suggestion return value; ``` ########## nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/EncodeContent.java: ########## @@ -29,55 +37,88 @@ import org.apache.nifi.annotation.documentation.CapabilityDescription; import org.apache.nifi.annotation.documentation.Tags; import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.components.Validator; +import org.apache.nifi.expression.ExpressionLanguageScope; import org.apache.nifi.flowfile.FlowFile; import org.apache.nifi.processor.AbstractProcessor; import org.apache.nifi.processor.ProcessContext; import org.apache.nifi.processor.ProcessSession; import org.apache.nifi.processor.Relationship; import org.apache.nifi.processor.io.StreamCallback; +import org.apache.nifi.processor.util.StandardValidators; +import org.apache.nifi.processors.standard.encoding.EncodingMode; +import org.apache.nifi.processors.standard.encoding.EncodingType; +import org.apache.nifi.processors.standard.encoding.LineOutputMode; import org.apache.nifi.processors.standard.util.ValidatingBase32InputStream; import org.apache.nifi.processors.standard.util.ValidatingBase64InputStream; import org.apache.nifi.stream.io.StreamUtils; import org.apache.nifi.util.StopWatch; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.util.Arrays; -import java.util.List; -import java.util.Set; -import java.util.concurrent.TimeUnit; - @SideEffectFree @SupportsBatching @InputRequirement(Requirement.INPUT_REQUIRED) @Tags({"encode", "decode", "base64", "base32", "hex"}) @CapabilityDescription("Encode or decode the contents of a FlowFile using Base64, Base32, or hex encoding schemes") public class EncodeContent extends AbstractProcessor { - public static final String ENCODE_MODE = "Encode"; - public static final String DECODE_MODE = "Decode"; - - public static final String BASE64_ENCODING = "base64"; - public static final String BASE32_ENCODING = "base32"; - public static final String HEX_ENCODING = "hex"; - public static final PropertyDescriptor MODE = new PropertyDescriptor.Builder() .name("Mode") .description("Specifies whether the content should be encoded or decoded") .required(true) - .allowableValues(ENCODE_MODE, DECODE_MODE) - .defaultValue(ENCODE_MODE) + .allowableValues(EncodingMode.class) + .defaultValue(EncodingMode.ENCODE) .build(); public static final PropertyDescriptor ENCODING = new PropertyDescriptor.Builder() .name("Encoding") .description("Specifies the type of encoding used") .required(true) - .allowableValues(BASE64_ENCODING, BASE32_ENCODING, HEX_ENCODING) - .defaultValue(BASE64_ENCODING) + .allowableValues(EncodingType.class) + .defaultValue(EncodingType.BASE64_ENCODING) + .build(); + + static final PropertyDescriptor LINE_OUTPUT_MODE = new PropertyDescriptor.Builder() + .name("Line Output Mode") + .displayName("Line Output Mode") + .description("If set to 'single-line', the encoded FlowFile content will output as a single line. If set to 'multiple-lines', " + + "it will output as multiple lines. This property is only applicable when Base64 or Base32 encoding is selected.") + .required(false) + .defaultValue(LineOutputMode.SINGLE_LINE) + .allowableValues(LineOutputMode.class) + .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) + .dependsOn(MODE, EncodingMode.ENCODE) + .dependsOn(ENCODING, EncodingType.BASE64_ENCODING, EncodingType.BASE32_ENCODING) .build(); + static final PropertyDescriptor ENCODED_LINE_SEPARATOR = new PropertyDescriptor.Builder() + .name("Encoded Content Line Separator") + .displayName("Encoded Content Line Separator") + .description("Each line of encoded data will be terminated with this byte sequence (e.g. \\r\\n" + + "). This property defaults to the system-dependent line separator string. If `line-length` <= 0, " + + "the `line-separator` property is not used. This property is not used for `hex` encoding.") + .required(false) + .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) + .defaultValue(System.lineSeparator()) + .addValidator(Validator.VALID) + .dependsOn(MODE, EncodingMode.ENCODE) + .dependsOn(ENCODING, EncodingType.BASE64_ENCODING, EncodingType.BASE32_ENCODING) + .build(); + + static final PropertyDescriptor ENCODED_LINE_LENGTH = new PropertyDescriptor.Builder() + .name("Encoded Content Line Length") + .displayName("Encoded Content Line Length") + .description("Each line of encoded data will contain `encoded-line-length` characters (rounded down to the nearest multiple of 4). " + + "If `encoded-line-length` <= 0, the encoded data is not divided into lines. This property is " + + "ignored if `line-output-mode` is set to `single-line`.") Review Comment: This description needs some rewording and clarification. It is not clear why it is rounded to a multiple of 4, and again, the conditional nature of the property does not need to be stated. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
