OOZIE-2872 Address backward compatibility issue introduced by OOZIE-2748 
(pbacsko)

(cherry picked from commit e0b7cde711b1b9e1a03660ec635041eeb9755049)

        core/src/main/resources/oozie-default.xml


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

Branch: refs/heads/branch-4.3
Commit: 7ea8f19a756fd8775671bd9bd71b210666547e6d
Parents: f553c4e
Author: Peter Bacsko <pbac...@cloudera.com>
Authored: Mon May 15 12:50:28 2017 +0200
Committer: satishsaley <satishsa...@apache.org>
Committed: Fri Dec 8 16:34:55 2017 -0800

----------------------------------------------------------------------
 .../oozie/action/hadoop/JavaActionExecutor.java |  6 ++++
 core/src/main/resources/oozie-default.xml       |  8 +++++
 .../action/hadoop/TestJavaActionExecutor.java   | 17 +++++++--
 release-log.txt                                 |  1 +
 .../oozie/action/hadoop/LauncherMapper.java     | 35 ++++++++++++------
 .../oozie/action/hadoop/TestLauncherMapper.java | 37 +++++++++++++++++---
 6 files changed, 87 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
b/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
index 8a403dc..50577fd 100644
--- a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
+++ b/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
@@ -993,6 +993,12 @@ public class JavaActionExecutor extends ActionExecutor {
                 args[i] = list.get(i).getTextTrim();
             }
             LauncherMapperHelper.setupMainArguments(launcherJobConf, args);
+            // backward compatibility flag - see OOZIE-2872
+            if 
(ConfigurationService.getBoolean(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED)) {
+                
launcherJobConf.setBoolean(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED, true);
+            } else {
+                
launcherJobConf.setBoolean(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED, false);
+            }
 
             // Make mapred.child.java.opts and mapreduce.map.java.opts equal, 
but give values from the latter priority; also append
             // <java-opt> and <java-opts> and give those highest priority

http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/core/src/main/resources/oozie-default.xml
----------------------------------------------------------------------
diff --git a/core/src/main/resources/oozie-default.xml 
b/core/src/main/resources/oozie-default.xml
index 8c55a0d..dbab18b 100644
--- a/core/src/main/resources/oozie-default.xml
+++ b/core/src/main/resources/oozie-default.xml
@@ -2850,4 +2850,12 @@ will be the requeue interval for the actions which are 
waiting for a long time w
         </description>
     </property>
 
+    <property>
+        <name>oozie.actions.null.args.allowed</name>
+        <value>true</value>
+        <description>
+            When set to true, empty arguments (like &lt;arg&gt;&lt;/arg&gt;) 
will be passed as "null" to the main method of a
+            given action. That is, the args[] array will contain "null" 
elements. When set to false, then "nulls" are removed.
+        </description>
+    </property>
 </configuration>

http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java
----------------------------------------------------------------------
diff --git 
a/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java 
b/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java
index 1c4b429..a6e1d50 100644
--- 
a/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java
+++ 
b/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java
@@ -2941,7 +2941,18 @@ public class TestJavaActionExecutor extends 
ActionExecutorTestCase {
         assertEquals("DEBUG", conf.get(oozieActionHiveRootLogger));
     }
 
-    public void testEmptyArgs() throws Exception {
+    public void testEmptyArgsWithNullArgsNotAllowed() throws Exception {
+        testEmptyArgs(false, "SUCCEEDED", WorkflowAction.Status.OK);
+    }
+
+    public void testEmptyArgsWithNullArgsAllowed() throws Exception {
+        testEmptyArgs(true, "FAILED/KILLED", WorkflowAction.Status.ERROR);
+    }
+
+    private void testEmptyArgs(boolean nullArgsAllowed, String 
expectedExternalStatus, WorkflowAction.Status expectedStatus)
+            throws Exception {
+        
ConfigurationService.setBoolean(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED, 
nullArgsAllowed);
+
         String actionXml = "<java>" +
                 "<job-tracker>" + getJobTrackerUri() + "</job-tracker>" +
                 "<name-node>" + getNameNodeUri() + "</name-node>" +
@@ -2961,10 +2972,10 @@ public class TestJavaActionExecutor extends 
ActionExecutorTestCase {
         ActionExecutor ae = new JavaActionExecutor();
         ae.check(context, context.getAction());
         assertTrue(ae.isCompleted(context.getAction().getExternalStatus()));
-        assertEquals("SUCCEEDED", context.getAction().getExternalStatus());
+        assertEquals(expectedExternalStatus, 
context.getAction().getExternalStatus());
         assertNull(context.getAction().getData());
 
         ae.end(context, context.getAction());
-        assertEquals(WorkflowAction.Status.OK, 
context.getAction().getStatus());
+        assertEquals(expectedStatus, context.getAction().getStatus());
     }
 }

http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/release-log.txt
----------------------------------------------------------------------
diff --git a/release-log.txt b/release-log.txt
index 2b36ec0..a79d8c2 100644
--- a/release-log.txt
+++ b/release-log.txt
@@ -1,5 +1,6 @@
 -- Oozie 4.3.1 release
 
+OOZIE-2872 Address backward compatibility issue introduced by OOZIE-2748 
(pbacsko)
 OOZIE-2863 SLACalculatorMemory.loadOnRestart causing delay in server start 
(puru via satishsaley)
 OOZIE-2862 Coord change command doesn't change job to running if job was 
killed without creating any actions (puru)
 OOZIE-2811 amend Add support for filtering out properties from 
SparkConfigurationService

http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
----------------------------------------------------------------------
diff --git 
a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
 
b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
index 8edebac..8657c67 100644
--- 
a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
+++ 
b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
@@ -63,6 +63,7 @@ public class LauncherMapper<K1, V1, K2, V2> implements 
Mapper<K1, V1, K2, V2>, R
     static final String CONF_OOZIE_EXTERNAL_STATS_MAX_SIZE = 
"oozie.external.stats.max.size";
     static final String OOZIE_ACTION_CONFIG_CLASS = ACTION_PREFIX + 
"config.class";
     static final String CONF_OOZIE_ACTION_FS_GLOB_MAX = ACTION_PREFIX + 
"fs.glob.max";
+    static final String CONF_OOZIE_NULL_ARGS_ALLOWED = ACTION_PREFIX + 
"null.args.allowed";
 
     static final String COUNTER_GROUP = "oozie.launcher";
     static final String COUNTER_LAUNCHER_ERROR = "oozie.launcher.error";
@@ -497,18 +498,28 @@ public class LauncherMapper<K1, V1, K2, V2> implements 
Mapper<K1, V1, K2, V2>, R
     public static String[] getMainArguments(Configuration conf) {
         String[] args = new 
String[conf.getInt(CONF_OOZIE_ACTION_MAIN_ARG_COUNT, 0)];
 
-        int pos = 0;
-        for (int i = 0; i < args.length; i++) {
-            String arg = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i);
-            if (!Strings.isNullOrEmpty(arg)) {
-                args[pos++] = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i);
+        String[] retArray;
+
+        if (conf.getBoolean(CONF_OOZIE_NULL_ARGS_ALLOWED, true)) {
+            for (int i = 0; i < args.length; i++) {
+                args[i] = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i);
+            }
+
+            retArray = args;
+        } else {
+            int pos = 0;
+            for (int i = 0; i < args.length; i++) {
+                String arg = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i);
+                if (!Strings.isNullOrEmpty(arg)) {
+                    args[pos++] = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + 
i);
+                }
             }
-        }
 
-        // this is to skip null args, that is <arg></arg> in the workflow XML 
-- in this case,
-        // args[] might look like {"arg1", "arg2", null, null} at this point
-        String[] retArray = new String[pos];
-        System.arraycopy(args, 0, retArray, 0, pos);
+            // this is to skip null args, that is <arg></arg> in the workflow 
XML -- in this case,
+            // args[] might look like {"arg1", "arg2", null, null} at this 
point
+            retArray = new String[pos];
+            System.arraycopy(args, 0, retArray, 0, pos);
+        }
 
         return retArray;
     }
@@ -632,6 +643,10 @@ public class LauncherMapper<K1, V1, K2, V2> implements 
Mapper<K1, V1, K2, V2>, R
         System.out.println(banner);
         boolean maskNextArg = false;
         for (String arg : args) {
+            if (arg == null) {
+                arg = "null"; // prevent NPE in pwd masking
+            }
+
             if (maskNextArg) {
                 System.out.println("             " + "********");
                 maskNextArg = false;

http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java
----------------------------------------------------------------------
diff --git 
a/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java
 
b/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java
index 1dd8002..51b1d6f 100644
--- 
a/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java
+++ 
b/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java
@@ -23,6 +23,7 @@ import static 
org.apache.oozie.action.hadoop.LauncherMapper.CONF_OOZIE_ACTION_MA
 import static org.junit.Assert.assertTrue;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Matchers.anyBoolean;
 
 import java.util.Arrays;
 import java.util.List;
@@ -41,8 +42,9 @@ public class TestLauncherMapper {
     private Configuration conf;  // we have to use mock, because 
conf.set(null) throws exception
 
     @Test
-    public void testLauncherMapperArgsHandlingWithoutNulls() {
+    public void testArgsHandlingWithoutNullsAndNullsNotAllowed() {
        setupConf(Lists.newArrayList("a", "b", "c"));
+       setEnableNullArgsAllowed(false);
 
        String args[] = LauncherMapper.getMainArguments(conf);
 
@@ -50,8 +52,9 @@ public class TestLauncherMapper {
     }
 
     @Test
-    public void testLauncherMapperArgsHandlingWhenArgsContainNulls() {
+    public void testHandlingWhenArgsContainNullsAndNullsNotAllowed() {
         setupConf(Lists.newArrayList("a", null, "b", null, "c"));
+        setEnableNullArgsAllowed(false);
 
         String args[] = LauncherMapper.getMainArguments(conf);
 
@@ -59,8 +62,9 @@ public class TestLauncherMapper {
     }
 
     @Test
-    public void testLauncherMapperArgsHandlingWhenArgsContainsNullsOnly() {
+    public void testArgsHandlingWhenArgsContainsNullsOnlyAndNullsNotAllowed() {
         setupConf(Lists.<String>newArrayList(null, null, null));
+        setEnableNullArgsAllowed(false);
 
         String args[] = LauncherMapper.getMainArguments(conf);
 
@@ -68,14 +72,35 @@ public class TestLauncherMapper {
     }
 
     @Test
-    public void testLauncherMapperArgsHandlingWhenArgsContainsOneNull() {
+    public void testArgsHandlingWhenArgsContainsOneNullAndNullsNotAllowed() {
         setupConf(Lists.<String>newArrayList((String) null));
+        setEnableNullArgsAllowed(false);
 
         String args[] = LauncherMapper.getMainArguments(conf);
 
         assertTrue(Arrays.equals(new String[] {}, args));
     }
 
+    @Test
+    public void testHandlingWhenArgsContainNullsAndNullAllowed() {
+        setupConf(Lists.newArrayList("a", null, "b", null, "c"));
+        setEnableNullArgsAllowed(true);
+
+        String args[] = LauncherMapper.getMainArguments(conf);
+
+        assertTrue(Arrays.equals(new String[] { "a", null, "b", null, "c"}, 
args));
+    }
+
+    @Test
+    public void testArgsHandlingWhenArgsContainsOneNullAndNullsAllowed() {
+        setupConf(Lists.<String>newArrayList((String) null));
+        setEnableNullArgsAllowed(true);
+
+        String args[] = LauncherMapper.getMainArguments(conf);
+
+        assertTrue(Arrays.equals(new String[] { null }, args));
+    }
+
     private void setupConf(List<String> argList) {
         int argCount = argList.size();
 
@@ -85,4 +110,8 @@ public class TestLauncherMapper {
             given(conf.get(eq(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + 
i))).willReturn(argList.get(i));
         }
     }
+
+    private void setEnableNullArgsAllowed(boolean nullArgsAllowed) {
+        given(conf.getBoolean(eq(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED), 
anyBoolean())).willReturn(nullArgsAllowed);
+    }
 }

Reply via email to