From afc227db9c858e8ea26c10bc50f95dcb00bd41cf Mon Sep 17 00:00:00 2001 From: Paul Lin Date: Thu, 10 Aug 2023 11:40:14 +0800 Subject: [PATCH] [KYUUBI #5149] [Improvement] Correct error message of ReflectUtils's invokeAs when method not found ### _Why are the changes needed?_ Currently, overloaded methods are considered the same and deduplicated in ReflectUtils, thus not easy to tell why no method is found. This PR fixes the problem by adding the argument lists. In addition, it fixes the problem that the arg classes are not printed correctly. ### _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 - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request Closes #5149 from link3280/reflect_exception_msg. Closes #5149 b27d743fc [Paul Lin] Update test case 0c04f2709 [Paul Lin] Improve error msg of ReflectUtils Authored-by: Paul Lin Signed-off-by: Paul Lin --- .../apache/kyuubi/util/reflect/ReflectUtils.scala | 10 ++++++---- .../kyuubi/util/reflect/ReflectUtilsSuite.scala | 12 ++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/kyuubi-util-scala/src/main/scala/org/apache/kyuubi/util/reflect/ReflectUtils.scala b/kyuubi-util-scala/src/main/scala/org/apache/kyuubi/util/reflect/ReflectUtils.scala index 78b0e1df7..08916b8d1 100644 --- a/kyuubi-util-scala/src/main/scala/org/apache/kyuubi/util/reflect/ReflectUtils.scala +++ b/kyuubi-util-scala/src/main/scala/org/apache/kyuubi/util/reflect/ReflectUtils.scala @@ -93,11 +93,13 @@ object ReflectUtils { } catch { case e: Exception => val candidates = - (clz.getDeclaredMethods ++ clz.getMethods).map(_.getName).distinct.sorted - val argClassesNames = argClasses.map(_.getClass.getName) + (clz.getDeclaredMethods ++ clz.getMethods) + .map(m => s"${m.getName}(${m.getParameterTypes.map(_.getName).mkString(", ")})") + .distinct.sorted + val argClassesNames = argClasses.map(_.getName) throw new RuntimeException( - s"Method $methodName(${argClassesNames.mkString(",")})" + - s" not found in $clz [${candidates.mkString(",")}]", + s"Method $methodName(${argClassesNames.mkString(", ")})" + + s" not found in $clz [${candidates.mkString(", ")}]", e) } } diff --git a/kyuubi-util-scala/src/test/java/org/apache/kyuubi/util/reflect/ReflectUtilsSuite.scala b/kyuubi-util-scala/src/test/java/org/apache/kyuubi/util/reflect/ReflectUtilsSuite.scala index dbfd234de..d81d1d9f9 100644 --- a/kyuubi-util-scala/src/test/java/org/apache/kyuubi/util/reflect/ReflectUtilsSuite.scala +++ b/kyuubi-util-scala/src/test/java/org/apache/kyuubi/util/reflect/ReflectUtilsSuite.scala @@ -66,6 +66,18 @@ class ReflectUtilsSuite extends AnyFunSuite { assertResult("field5")(getField[String](ObjectA, "field5")) assertResult("field6")(getField[String](ObjectA, "field6")) } + + test("test invokeAs method not found exception") { + val exception = intercept[RuntimeException]{ + invokeAs[String](ObjectA, "methodNotExists", (classOf[String], "arg1"), + (classOf[String], "arg2")) + } + assert(exception.getMessage === + "Method methodNotExists(java.lang.String, java.lang.String) not found " + + "in class org.apache.kyuubi.util.reflect.ObjectA$ " + + "[equals(java.lang.Object), field5(), field6(), getClass(), hashCode(), method5(), " + + "method6(), notify(), notifyAll(), toString(), wait(), wait(long), wait(long, int)]") + } } class ClassA(val field0: String = "field0") {