You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2019/10/30 13:57:09 UTC

[zeppelin] branch master updated: [ZEPPELIN-4386] Resource.invokeMethod() doesn't find the right method to call

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

zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new 7c00e51  [ZEPPELIN-4386] Resource.invokeMethod() doesn't find the right method to call
7c00e51 is described below

commit 7c00e517a07961bec1b2174127706118b53d554d
Author: Gyula Komlossi <gk...@cloudera.com>
AuthorDate: Thu Oct 24 18:50:37 2019 +0200

    [ZEPPELIN-4386] Resource.invokeMethod() doesn't find the right method to call
    
    ### What is this PR for?
    The logic in Resource.invokeMethod() seems to be failing to find the correct method signature, if the method to invoke is overloaded and the parameter types for invokeMethod() are not specified.
    
    ### What type of PR is it?
    Bug Fix
    
    ### Todos
    --
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/browse/ZEPPELIN-4386
    
    ### How should this be tested?
    New unit tests added to cover these scenarios.
    [CI link](https://travis-ci.org/gkomlossi/zeppelin/builds/602501345)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No
    
    Author: Gyula Komlossi <gk...@cloudera.com>
    
    Closes #3492 from gkomlossi/master and squashes the following commits:
    
    c4f919f1a [Gyula Komlossi] [ZEPPELIN-4386] Resource.invokeMethod() doesn't find the right method to call
---
 .../org/apache/zeppelin/resource/Resource.java     | 45 ++++++++++++----------
 .../org/apache/zeppelin/resource/ResourceTest.java | 13 ++++++-
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/resource/Resource.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/resource/Resource.java
index c671707..32eaeb2 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/resource/Resource.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/resource/Resource.java
@@ -38,6 +38,7 @@ import java.nio.ByteBuffer;
  * Information and reference to the resource
  */
 public class Resource implements JsonSerializable, Serializable {
+  private static final Logger LOGGER = LoggerFactory.getLogger(Resource.class);
   private static final Gson gson = new Gson();
 
   private final transient Object r;
@@ -269,37 +270,41 @@ public class Resource implements JsonSerializable, Serializable {
    */
   public Object invokeMethod(
           String methodName, Type[] types, Object[] params, String returnResourceName) throws ClassNotFoundException {
-    Type[] methodTypes = null;
-    Object [] methodParams = null;
+    Object[] convertedParams = null;
+    Class[] classes = null;
+
     if (types != null) {
-      methodTypes = types;
-      methodParams = params;
+      convertedParams = convertParams(types, params);
+      classes = classFromType(types);
     } else {
       // inference method param types
       boolean found = false;
       Method[] methods = r.getClass().getDeclaredMethods();
+
       for (Method m : methods) {
+        // try to find method by name
         if (!m.getName().equals(methodName)) {
           continue;
         }
-        Type[] paramTypes = m.getGenericParameterTypes();
-        Object[] paramValues = new Object[paramTypes.length];
 
-        int pidx = 0;
-        for (int i = 0; i < paramTypes.length; i++) {
-          if (pidx == params.length) {  // not enough param for this method signature
+        Type[] paramTypes = m.getGenericParameterTypes();
+        if (paramTypes.length != params.length) {
+          // parameter count doesn't match
+          continue;
+        } else {
+          try {
+            // try to convert parameters
+            convertedParams = convertParams(paramTypes, params);
+          } catch (Exception e) {
+            LOGGER.info(
+                String.format("The parameter types of method \'%s\' don't match with the arguments", m.getName()));
             continue;
-          } else {
-            paramValues[i] = params[pidx++];
           }
         }
 
-        if (pidx == params.length) {  // param number does not match
-          found = true;
-          methodParams = paramValues;
-          methodTypes = paramTypes;
-          break;
-        }
+        classes = classFromType(paramTypes);
+        found = true;
+        break;
       }
 
       if (!found) {
@@ -307,12 +312,10 @@ public class Resource implements JsonSerializable, Serializable {
       }
     }
 
-    Class[] classes = classFromType(methodTypes);
-
     if (returnResourceName == null) {
-      return invokeMethod(methodName, classes, convertParams(methodTypes, methodParams));
+      return invokeMethod(methodName, classes, convertedParams);
     } else {
-      return invokeMethod(methodName, classes, convertParams(methodTypes, methodParams), returnResourceName);
+      return invokeMethod(methodName, classes, convertedParams, returnResourceName);
     }
   }
 
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/resource/ResourceTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/resource/ResourceTest.java
index 211d85d..0445229 100644
--- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/resource/ResourceTest.java
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/resource/ResourceTest.java
@@ -47,13 +47,24 @@ public class ResourceTest {
   public void testInvokeMethod_shouldAbleToInvokeMethodWithTypeInference() throws ClassNotFoundException, NoSuchMethodException, InvocationTargetException, IllegalAccessException {
     Resource r = new Resource(null, new ResourceId("pool1", "name1"), "object");
     assertEquals("ect", r.invokeMethod("substring", new Object[]{3}));
+    assertEquals("obj", r.invokeMethod("substring", new Object[]{0,3}));
     assertEquals(true, r.invokeMethod("startsWith", new Object[]{"obj"}));
 
+    assertEquals(2, r.invokeMethod("indexOf", new Object[]{'j'}));
+    assertEquals(4, r.invokeMethod("indexOf", new Object[]{"ct",3}));
+
     assertEquals("ect", r.invokeMethod("substring", new ArrayList<>(Arrays.asList(3))));
+    assertEquals("ec", r.invokeMethod("substring", new ArrayList<>(Arrays.asList(3,5))));
     assertEquals(true, r.invokeMethod("startsWith", new ArrayList<>(Arrays.asList("obj"))));
   }
 
-  @Test
+  @Test(expected = ClassNotFoundException.class)
+  public void testInvokeMethod_shouldNotAbleToInvokeMethodWithTypeInference() throws ClassNotFoundException, NoSuchMethodException, InvocationTargetException, IllegalAccessException {
+    Resource r = new Resource(null, new ResourceId("pool1", "name1"), "object");
+    r.invokeMethod("indexOf", new Object[]{"ct",3,4});
+  }
+
+    @Test
   public void testInvokeMethod_shouldAbleToInvokeMethodWithParamClassName() throws ClassNotFoundException, NoSuchMethodException, InvocationTargetException, IllegalAccessException {
     Resource r = new Resource(null, new ResourceId("pool1", "name1"), "object");
     assertEquals("ect", r.invokeMethod("substring", new String[]{"int"}, new Object[]{3}));