OOZIE-2748 NPE in LauncherMapper.printArgs() (pbacsko via rkanter)

(cherry picked from commit c7fa12b)

core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java

@@ -2941,4 +2941,30 @@ public void testSetRootLoggerLevel() throws Exception {
         assertEquals("DEBUG", conf.get(oozieActionHiveRootLogger));
     }
 
+    public void testEmptyArgs() throws Exception {
+        String actionXml = "<java>" +
+                "<job-tracker>" + getJobTrackerUri() + "</job-tracker>" +
+                "<name-node>" + getNameNodeUri() + "</name-node>" +
+                "<main-class>" + LauncherMainTester.class.getName() + "</main-class>" +
+                "<arg></arg>" +
+                "</java>";
+
+        Context context = createContext(actionXml, null);
+        final RunningJob runningJob = submitAction(context);
+        waitFor(60 * 1000, new Predicate() {
+            @Override
+            public boolean evaluate() throws Exception {
+                return runningJob.isComplete();
+            }
+        });
+        assertTrue(runningJob.isSuccessful());
+        ActionExecutor ae = new JavaActionExecutor();
+        ae.check(context, context.getAction());
+        assertTrue(ae.isCompleted(context.getAction().getExternalStatus()));
+        assertEquals("SUCCEEDED", context.getAction().getExternalStatus());
+        assertNull(context.getAction().getData());
+
+        ae.end(context, context.getAction());
+        assertEquals(WorkflowAction.Status.OK, context.getAction().getStatus());
+    }
 }

release-log.txt

@@ -1,5 +1,6 @@
 -- Oozie 4.3.1 release
 
+OOZIE-2748 NPE in LauncherMapper.printArgs() (pbacsko via rkanter)
 OOZIE-2654 Zookeeper dependent services should not depend on Connectionstate to be valid before cleaning up (venkatnrangan via abhishekbafna)
 OOZIE-2690 OOZIE NPE while executing kill() (abhishekbafna via jaydeepvishwakarma)
 

sharelib/oozie/pom.xml

@@ -61,6 +61,12 @@
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-all</artifactId>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>org.apache.oozie</groupId>
             <artifactId>oozie-hadoop-utils</artifactId>

sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java

@@ -50,6 +50,8 @@
 import org.apache.hadoop.mapred.OutputCollector;
 import org.apache.hadoop.mapred.Reporter;
 
+import com.google.common.base.Strings;
+
 public class LauncherMapper<K1, V1, K2, V2> implements Mapper<K1, V1, K2, V2>, Runnable {
 
     static final String CONF_OOZIE_ACTION_MAIN_CLASS = "oozie.launcher.action.main.class";
@@ -494,10 +496,21 @@ private void executePrepare() throws IOException, LauncherException {
 
     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++) {
-            args[i] = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + 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);
+            }
         }
-        return args;
+
+        // 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);
+
+        return retArray;
     }
 
     private void setupHeartBeater(Reporter reporter) {

sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java

@@ -0,0 +1,88 @@
+/**
+ * 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.oozie.action.hadoop;
+
+import static org.apache.oozie.action.hadoop.LauncherMapper.CONF_OOZIE_ACTION_MAIN_ARG_COUNT;
+import static org.apache.oozie.action.hadoop.LauncherMapper.CONF_OOZIE_ACTION_MAIN_ARG_PREFIX;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Matchers.eq;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.apache.hadoop.conf.Configuration;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.google.common.collect.Lists;
+
+@RunWith(MockitoJUnitRunner.class)
+public class TestLauncherMapper {
+    @Mock
+    private Configuration conf;  // we have to use mock, because conf.set(null) throws exception
+
+    @Test
+    public void testLauncherMapperArgsHandlingWithoutNulls() {
+       setupConf(Lists.newArrayList("a", "b", "c"));
+
+       String args[] = LauncherMapper.getMainArguments(conf);
+
+       assertTrue(Arrays.equals(new String[] { "a", "b", "c"}, args));
+    }
+
+    @Test
+    public void testLauncherMapperArgsHandlingWhenArgsContainNulls() {
+        setupConf(Lists.newArrayList("a", null, "b", null, "c"));
+
+        String args[] = LauncherMapper.getMainArguments(conf);
+
+        assertTrue(Arrays.equals(new String[] { "a", "b", "c"}, args));
+    }
+
+    @Test
+    public void testLauncherMapperArgsHandlingWhenArgsContainsNullsOnly() {
+        setupConf(Lists.<String>newArrayList(null, null, null));
+
+        String args[] = LauncherMapper.getMainArguments(conf);
+
+        assertTrue(Arrays.equals(new String[] {}, args));
+    }
+
+    @Test
+    public void testLauncherMapperArgsHandlingWhenArgsContainsOneNull() {
+        setupConf(Lists.<String>newArrayList((String) null));
+
+        String args[] = LauncherMapper.getMainArguments(conf);
+
+        assertTrue(Arrays.equals(new String[] {}, args));
+    }
+
+    private void setupConf(List<String> argList) {
+        int argCount = argList.size();
+
+        given(conf.getInt(eq(CONF_OOZIE_ACTION_MAIN_ARG_COUNT), eq(0))).willReturn(argCount);
+
+        for (int i = 0; i < argCount; i++) {
+            given(conf.get(eq(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i))).willReturn(argList.get(i));
+        }
+    }
+}