[incubator-kyuubi] branch master updated: [KYUUBI #3571] Fix NPE for batch command line

[fe...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

feiwang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git


The following commit(s) were added to refs/heads/master by this push:
     new a6f933f85 [KYUUBI #3571] Fix NPE for batch command line
a6f933f85 is described below

commit a6f933f85a3e76ea312afaa194a6a83cc4f0c702
Author: Fei Wang <fw...@ebay.com>
AuthorDate: Wed Sep 28 20:50:54 2022 +0800

    [KYUUBI #3571] Fix NPE for batch command line
    
    ### _Why are the changes needed?_
    
    Fix NPE if batch request/args/config is not specified.
    
    ### _How was this patch tested?_
    - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
    
    - [ ] Add screenshots for manual tests if appropriate
    
    - [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #3571 from turboFei/npe_batch.
    
    Closes #3571
    
    682de4f4 [Fei Wang] fix npe if value is null
    f5b94d98 [Fei Wang] ut
    68ee3101 [Fei Wang] fix npe
    
    Authored-by: Fei Wang <fw...@ebay.com>
    Signed-off-by: Fei Wang <fw...@ebay.com>
---
 .../kyuubi/ctl/cmd/create/CreateBatchCommand.scala   | 18 ++++++++++++------
 .../kyuubi/server/rest/client/BatchCliSuite.scala    | 20 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala b/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala
index 3d418d928..3c5c6c55c 100644
--- a/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala
+++ b/kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala
@@ -16,13 +16,13 @@
  */
 package org.apache.kyuubi.ctl.cmd.create
 
-import java.util.{List => JList, Map => JMap}
+import java.util.{Collections, List => JList, Map => JMap}
 
 import scala.collection.JavaConverters._
 
 import org.apache.kyuubi.client.BatchRestApi
 import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest}
-import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.{CliConfig, ControlCliException}
 import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
 import org.apache.kyuubi.ctl.cmd.Command
 import org.apache.kyuubi.ctl.util.{CtlUtils, Render, Validator}
@@ -40,10 +40,16 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
 
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
-      val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
-        .map { case (k, v) => (k.toString, v.toString) }.asJava
-      val args = request.get("args").asInstanceOf[JList[Object]].asScala
-        .map(x => x.toString).asJava
+      if (request == null) {
+        error(s"No batch request field specified in yaml")
+        throw ControlCliException(1)
+      }
+      val config = Option(request.get("configs").asInstanceOf[JMap[Object, Object]].asScala)
+        .map(_.map { case (k, v) => (k.toString, Option(v).map(_.toString).getOrElse("")) }.asJava)
+        .getOrElse(Collections.emptyMap())
+      val args = Option(request.get("args").asInstanceOf[JList[Object]].asScala)
+        .map(_.map(x => x.toString).asJava)
+        .getOrElse(Collections.emptyList())
       val batchRequest = new BatchRequest(
         request.get("batchType").asInstanceOf[String],
         request.get("resource").asInstanceOf[String],
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchCliSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchCliSuite.scala
index d45f6f215..ff52bf7fb 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchCliSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchCliSuite.scala
@@ -17,11 +17,13 @@
 
 package org.apache.kyuubi.server.rest.client
 
+import java.io.File
 import java.net.InetAddress
 import java.nio.charset.StandardCharsets
 import java.nio.file.{Files, Paths}
 
 import org.apache.hadoop.security.UserGroupInformation
+import org.apache.hadoop.shaded.com.nimbusds.jose.util.StandardCharset
 import org.apache.hive.service.rpc.thrift.TProtocolVersion
 import org.scalatest.time.SpanSugar.convertIntToGrainOfTime
 
@@ -58,6 +60,7 @@ class BatchCliSuite extends RestClientTestHelper with TestPrematureExit with Bat
                          |    wait.completion: true
                          |    k1: v1
                          |    1: test_integer_key
+                         |    key:
                          |options:
                          |  verbose: true""".stripMargin
     Files.write(Paths.get(batchFile), batch_basic.getBytes(StandardCharsets.UTF_8))
@@ -314,6 +317,23 @@ class BatchCliSuite extends RestClientTestHelper with TestPrematureExit with Bat
     testPrematureExitForControlCli(listArgs1, "Batch List (from 0 total 0)")
   }
 
+  test("test batch yaml without request field") {
+    val tempDir = Utils.createTempDir()
+    val yamlFile1 = Files.write(
+      new File(tempDir.toFile, "f1.yaml").toPath,
+      s"""
+         |apiVersion: v1
+         |user: test_user
+         |""".stripMargin
+        .getBytes(StandardCharset.UTF_8))
+    val args = Array(
+      "create",
+      "batch",
+      "-f",
+      yamlFile1.toFile.getAbsolutePath)
+    testPrematureExitForControlCli(args, "No batch request field specified in yaml")
+  }
+
   private def getBatchIdFromBatchReport(batchReport: String): String = {
     val batchIdRegex = s"""Batch Report \\((.*)\\)""".r
     batchIdRegex.findFirstMatchIn(batchReport) match {