You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by pv...@apache.org on 2016/09/13 20:56:12 UTC

nifi git commit: NIFI-2752 Correct ReplaceText default pattern and unit tests * Corrected the DEFAULT_REGEX pattern. * Added tests to isolate regex capture group problem and verify corrected functionality. * Removed short circuit logic that masked con

Repository: nifi
Updated Branches:
  refs/heads/master 02ca2a0d7 -> c72a9aa54


NIFI-2752 Correct ReplaceText default pattern and unit tests
 * Corrected the DEFAULT_REGEX pattern.
 * Added tests to isolate regex capture group problem and verify corrected functionality.
 * Removed short circuit logic that masked configuration errors and created inconsistent processor behavior.

This closes #1007.


Project: http://git-wip-us.apache.org/repos/asf/nifi/repo
Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/c72a9aa5
Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/c72a9aa5
Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/c72a9aa5

Branch: refs/heads/master
Commit: c72a9aa54ba349bdc7567ec741a36e11b6463a3e
Parents: 02ca2a0
Author: Joe Skora <js...@apache.org>
Authored: Mon Sep 12 15:20:06 2016 -0400
Committer: Pierre Villard <pi...@gmail.com>
Committed: Tue Sep 13 22:55:55 2016 +0200

----------------------------------------------------------------------
 .../nifi/processors/standard/ReplaceText.java   |  9 +--
 .../processors/standard/TestReplaceText.java    | 60 +++++++++++++++++++-
 2 files changed, 60 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/nifi/blob/c72a9aa5/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java
----------------------------------------------------------------------
diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java
index 59e0da6..a542048 100644
--- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java
+++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceText.java
@@ -84,7 +84,7 @@ public class ReplaceText extends AbstractProcessor {
     public static final String literalReplaceValue = "Literal Replace";
     public static final String alwaysReplace = "Always Replace";
     private static final Pattern backReferencePattern = Pattern.compile("\\$(\\d+)");
-    private static final String DEFAULT_REGEX = "(?s:^.*$)";
+    private static final String DEFAULT_REGEX = "(?s)(^.*$)";
     private static final String DEFAULT_REPLACEMENT_VALUE = "$1";
 
     // Prepend and Append will just insert the replacement value at the beginning or end
@@ -214,13 +214,6 @@ public class ReplaceText extends AbstractProcessor {
         String unsubstitutedReplacement = context.getProperty(REPLACEMENT_VALUE).getValue();
         final String replacementStrategy = context.getProperty(REPLACEMENT_STRATEGY).getValue();
 
-        if (replacementStrategy.equalsIgnoreCase(regexReplaceValue) && unsubstitutedRegex.equals(DEFAULT_REGEX) && unsubstitutedReplacement.equals(DEFAULT_REPLACEMENT_VALUE)) {
-            // This pattern says replace content with itself. We can highly optimize this process by simply transferring
-            // all FlowFiles to the 'success' relationship
-            session.transfer(flowFiles, REL_SUCCESS);
-            return;
-        }
-
         final Charset charset = Charset.forName(context.getProperty(CHARACTER_SET).getValue());
         final int maxBufferSize = context.getProperty(MAX_BUFFER_SIZE).asDataSize(DataUnit.B).intValue();
 

http://git-wip-us.apache.org/repos/asf/nifi/blob/c72a9aa5/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceText.java
----------------------------------------------------------------------
diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceText.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceText.java
index a6e0971..b65bff1 100644
--- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceText.java
+++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceText.java
@@ -30,10 +30,15 @@ import org.apache.nifi.util.MockFlowFile;
 import org.apache.nifi.util.TestRunner;
 import org.apache.nifi.util.TestRunners;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class TestReplaceText {
 
+    @Rule
+    public ExpectedException exception = ExpectedException.none();
+
     @Test
     public void testConfigurationCornerCase() throws IOException {
         final TestRunner runner = TestRunners.newTestRunner(new ReplaceText());
@@ -64,7 +69,7 @@ public class TestReplaceText {
     }
 
     @Test
-    public void testWithEscaped$InReplacemenmt() throws IOException {
+    public void testWithEscaped$InReplacement() throws IOException {
         final TestRunner runner = TestRunners.newTestRunner(new ReplaceText());
         runner.setValidateExpressionUsage(false);
         runner.setProperty(ReplaceText.SEARCH_VALUE, "(?s:^.*$)");
@@ -1106,7 +1111,60 @@ public class TestReplaceText {
         out.assertContentEquals("abc.txt\nabc.txt\r\nabc.txt\n");
     }
 
+    @Test
+    public void testRegexWithBadCaptureGroup() throws IOException {
+        // Test the old Default Regex and with a custom Replacement Value that should fail because the
+        // Perl regex "(?s:^.*$)" must be written "(?s)(^.*$)" in Java for there to be a capture group.
+        //      private static final String DEFAULT_REGEX = "(?s:^.*$)";
+        final TestRunner runner = TestRunners.newTestRunner(new ReplaceText());
+        runner.setValidateExpressionUsage(false);
+        runner.setProperty(ReplaceText.SEARCH_VALUE, "(?s:^.*$)");
+        runner.setProperty(ReplaceText.REPLACEMENT_VALUE, "${'$1':toUpper()}"); // should uppercase group but there is none
+        runner.setProperty(ReplaceText.REPLACEMENT_STRATEGY, ReplaceText.REGEX_REPLACE);
+        runner.setProperty(ReplaceText.EVALUATION_MODE, ReplaceText.ENTIRE_TEXT);
+
+        runner.enqueue("testing\n123".getBytes());
+        runner.run();
+
+        runner.assertAllFlowFilesTransferred(ReplaceText.REL_SUCCESS, 1);
+        final MockFlowFile out = runner.getFlowFilesForRelationship(ReplaceText.REL_SUCCESS).get(0);
+        out.assertContentEquals("");
+    }
 
+    @Test
+    public void testRegexWithGoodCaptureGroup() throws IOException {
+        // Test the new Default Regex and with a custom Replacement Values that should succeed.
+        final TestRunner runner = TestRunners.newTestRunner(new ReplaceText());
+        runner.setValidateExpressionUsage(false);
+        runner.setProperty(ReplaceText.SEARCH_VALUE, "(?s)(^.*$)");
+        runner.setProperty(ReplaceText.REPLACEMENT_VALUE, "${'$1':toUpper()}"); // will uppercase group with good Java regex
+        runner.setProperty(ReplaceText.REPLACEMENT_STRATEGY, ReplaceText.REGEX_REPLACE);
+        runner.setProperty(ReplaceText.EVALUATION_MODE, ReplaceText.ENTIRE_TEXT);
+
+        runner.enqueue("testing\n123".getBytes());
+        runner.run();
+
+        runner.assertAllFlowFilesTransferred(ReplaceText.REL_SUCCESS, 1);
+        final MockFlowFile out = runner.getFlowFilesForRelationship(ReplaceText.REL_SUCCESS).get(0);
+        out.assertContentEquals("TESTING\n123");
+    }
+
+    @Test
+    public void testRegexNoCaptureDefaultReplacement() throws IOException {
+        // Test the old Default Regex and new Default Regex with the default replacement.  This should fail
+        // because the regex does not create a capture group.
+        final TestRunner runner = TestRunners.newTestRunner(new ReplaceText());
+        runner.setValidateExpressionUsage(false);
+        runner.setProperty(ReplaceText.SEARCH_VALUE, "(?s:^.*$)");
+        runner.setProperty(ReplaceText.REPLACEMENT_VALUE, "$1");
+        runner.setProperty(ReplaceText.REPLACEMENT_STRATEGY, ReplaceText.REGEX_REPLACE);
+        runner.setProperty(ReplaceText.EVALUATION_MODE, ReplaceText.ENTIRE_TEXT);
+
+        exception.expect(AssertionError.class);
+        exception.expectMessage("java.lang.IndexOutOfBoundsException: No group 1");
+        runner.enqueue("testing\n123".getBytes());
+        runner.run();
+    }
 
     private String translateNewLines(final File file) throws IOException {
         return translateNewLines(file.toPath());