review fixes

TouK · Nov 6, 2024 · e7acaf6 · e7acaf6
1 parent c7092cb
commit e7acaf6
Show file tree

Hide file tree

Showing 10 changed files with 143 additions and 206 deletions.
diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/extension/ArrayExt.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/extension/ArrayExt.scala
@@ -6,20 +6,8 @@ import pl.touk.nussknacker.engine.spel.internal.ConversionHandler
 import java.util
 import java.util.{List => JList}
 
-class ArrayExt(target: Any) extends util.AbstractList[Object] with ExtensionMethodInvocationTarget {
-  private val asList = ConversionHandler.convertArrayToList(target)
-
-  override def invoke(methodName: String, arguments: Array[Object]): Any = methodName match {
-    case "get"               => get(arguments(0).asInstanceOf[Integer])
-    case "size"              => size()
-    case "lastIndexOf"       => lastIndexOf(arguments(0))
-    case "contains"          => contains(arguments(0))
-    case "indexOf"           => indexOf(arguments(0))
-    case "containsAll"       => containsAll(arguments(0).asInstanceOf[util.Collection[_]])
-    case "isEmpty" | "empty" => isEmpty
-    case _                   => throw new IllegalAccessException(s"Cannot find method with name: '$methodName'")
-  }
-
+class ArrayWrapper(target: Any) extends util.AbstractList[Object] {
+  private val asList                                       = ConversionHandler.convertArrayToList(target)
   override def get(index: Int): AnyRef                     = asList.get(index)
   override def size(): Int                                 = asList.size()
   override def lastIndexOf(o: Any): Int                    = super.lastIndexOf(o)
@@ -28,17 +16,46 @@ class ArrayExt(target: Any) extends util.AbstractList[Object] with ExtensionMeth
   override def containsAll(c: util.Collection[_]): Boolean = super.containsAll(c)
   override def isEmpty: Boolean                            = super.isEmpty
   def empty: Boolean                                       = super.isEmpty
-
 }
 
-object ArrayExt extends ExtensionMethodsHandler[ArrayExt] {
+class ArrayExt extends ExtensionMethodHandler {
+
+  override val methodRegistry: Map[String, ExtensionMethod] = Map(
+    "get" -> new ExtensionMethod {
+      override val argsSize: Int = 1
+      override def invoke(target: Any, args: Object*): Any =
+        new ArrayWrapper(target).get(args.head.asInstanceOf[Integer])
+    },
+    "size" -> ((target: Any, _) => new ArrayWrapper(target).size()),
+    "lastIndexOf" -> new ExtensionMethod {
+      override val argsSize: Int = 1
+      override def invoke(target: Any, args: Object*): Any =
+        new ArrayWrapper(target).lastIndexOf(args.head.asInstanceOf[Integer])
+    },
+    "contains" -> new ExtensionMethod {
+      override val argsSize: Int = 1
+      override def invoke(target: Any, args: Object*): Any =
+        new ArrayWrapper(target).contains(args.head)
+    },
+    "indexOf" -> new ExtensionMethod {
+      override val argsSize: Int = 1
+      override def invoke(target: Any, args: Object*): Any =
+        new ArrayWrapper(target).indexOf(args.head)
+    },
+    "containsAll" -> new ExtensionMethod {
+      override val argsSize: Int = 1
+      override def invoke(target: Any, args: Object*): Any =
+        new ArrayWrapper(target).containsAll(args.head.asInstanceOf[util.Collection[_]])
+    },
+    "isEmpty" -> ((target: Any, _) => new ArrayWrapper(target).isEmpty()),
+    "empty"   -> ((target: Any, _) => new ArrayWrapper(target).isEmpty()),
+  )
+
+}
 
-  override val invocationTargetClass: Class[ArrayExt] = classOf[ArrayExt]
+object ArrayExt extends ExtensionMethodsDefinition {
 
-  override def createConverter(
-      set: ClassDefinitionSet
-  ): ToExtensionMethodInvocationTargetConverter[ArrayExt] =
-    (target: Any) => new ArrayExt(target)
+  override def createHandler(set: ClassDefinitionSet): ExtensionMethodHandler = new ArrayExt
 
   override def extractDefinitions(clazz: Class[_], set: ClassDefinitionSet): Map[String, List[MethodDefinition]] =
     if (clazz.isArray) {

diff --git a/...io-compiler/src/main/scala/pl/touk/nussknacker/engine/extension/CastOrConversionExt.scala b/...io-compiler/src/main/scala/pl/touk/nussknacker/engine/extension/CastOrConversionExt.scala
@@ -14,37 +14,44 @@ import java.lang.{Boolean => JBoolean}
 import scala.util.Try
 
 // todo: lbg - add casting methods to UTIL
-class CastOrConversionExt(target: Any, classesBySimpleName: Map[String, Class[_]])
-    extends ExtensionMethodInvocationTarget {
-
-  override def invoke(methodName: String, arguments: Array[Object]): Any = methodName match {
-    case "is"       => is(arguments(0).asInstanceOf[String])
-    case "to"       => to(arguments(0).asInstanceOf[String])
-    case "toOrNull" => toOrNull(arguments(0).asInstanceOf[String])
-    case _          => throw new IllegalAccessException(s"Cannot find method with name: '$methodName'")
-  }
+class CastOrConversionExt(classesBySimpleName: Map[String, Class[_]]) extends ExtensionMethodHandler {
+
+  override val methodRegistry: Map[String, ExtensionMethod] = Map(
+    "is" -> new ExtensionMethod {
+      override val argsSize: Int                           = 1
+      override def invoke(target: Any, args: Object*): Any = is(target, args.head.asInstanceOf[String])
+    },
+    "to" -> new ExtensionMethod {
+      override val argsSize: Int                           = 1
+      override def invoke(target: Any, args: Object*): Any = to(target, args.head.asInstanceOf[String])
+    },
+    "toOrNull" -> new ExtensionMethod {
+      override val argsSize: Int                           = 1
+      override def invoke(target: Any, args: Object*): Any = toOrNull(target, args.head.asInstanceOf[String])
+    },
+  )
 
-  def is(className: String): Boolean =
+  private def is(target: Any, className: String): Boolean =
     getClass(className).exists(clazz => clazz.isAssignableFrom(target.getClass)) ||
       getConversion(className).exists(_.canConvert(target))
 
-  def to(className: String): Any =
-    orElse(tryCast(className), tryConvert(className))
+  private def to(target: Any, className: String): Any =
+    orElse(tryCast(target, className), tryConvert(target, className))
       .getOrElse(throw new IllegalStateException(s"Cannot cast or convert value: $target to: '$className'"))
 
-  def toOrNull(className: String): Any =
-    orElse(tryCast(className), tryConvert(className))
+  private def toOrNull(target: Any, className: String): Any =
+    orElse(tryCast(target, className), tryConvert(target, className))
       .getOrElse(null)
 
-  private def tryCast(className: String): Either[Throwable, Any] = getClass(className) match {
+  private def tryCast(target: Any, className: String): Either[Throwable, Any] = getClass(className) match {
     case Some(clazz) => Try(clazz.cast(target)).toEither
     case None        => Left(new ClassCastException(s"Cannot cast: [$target] to: [$className]."))
   }
 
   private def getClass(className: String): Option[Class[_]] =
     classesBySimpleName.get(className.toLowerCase())
 
-  private def tryConvert(className: String): Either[Throwable, Any] =
+  private def tryConvert(target: Any, className: String): Either[Throwable, Any] =
     getConversion(className)
       .flatMap(_.convertEither(target))
 
@@ -57,7 +64,7 @@ class CastOrConversionExt(target: Any, classesBySimpleName: Map[String, Class[_]
 
 }
 
-object CastOrConversionExt extends ExtensionMethodsHandler[CastOrConversionExt] {
+object CastOrConversionExt extends ExtensionMethodsDefinition {
   private val isMethodName            = "is"
   private val toMethodName            = "to"
   private val toOrNullMethodName      = "toOrNull"
@@ -83,18 +90,14 @@ object CastOrConversionExt extends ExtensionMethodsHandler[CastOrConversionExt]
     .flatMap(c => c.resultTypeClass.classByNameAndSimpleNameLowerCase().map(n => n._1 -> c))
     .toMap
 
-  override val invocationTargetClass: Class[CastOrConversionExt] = classOf[CastOrConversionExt]
-
   def isCastOrConversionMethod(methodName: String): Boolean =
     castOrConversionMethods.contains(methodName)
 
   def allowedConversions(clazz: Class[_]): List[Conversion] = conversionsRegistry.filter(_.appliesToConversion(clazz))
 
-  override def createConverter(
-      set: ClassDefinitionSet
-  ): ToExtensionMethodInvocationTargetConverter[CastOrConversionExt] = {
+  override def createHandler(set: ClassDefinitionSet): ExtensionMethodHandler = {
     val classesBySimpleName = set.classDefinitionsMap.keySet.classesByNamesAndSimpleNamesLowerCase()
-    (target: Any) => new CastOrConversionExt(target, classesBySimpleName)
+    new CastOrConversionExt(classesBySimpleName)
   }
 
   override def extractDefinitions(clazz: Class[_], set: ClassDefinitionSet): Map[String, List[MethodDefinition]] = {

diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/extension/Conversion.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/extension/Conversion.scala
@@ -69,7 +69,7 @@ object Conversion {
 
 }
 
-trait ConversionExt[T <: ExtensionMethodInvocationTarget] extends ExtensionMethodsHandler[T] with Conversion {
+trait ConversionExt extends ExtensionMethodsDefinition with Conversion {
   private lazy val definitionsByName = definitions().groupBy(_.name)
 
   def definitions(): List[MethodDefinition] = {

diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/extension/ExtensionMethods.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/extension/ExtensionMethods.scala
@@ -1,32 +1,23 @@
 package pl.touk.nussknacker.engine.extension
 
-import org.springframework.core.MethodParameter
 import org.springframework.core.convert.TypeDescriptor
 import org.springframework.expression.{EvaluationContext, MethodExecutor, MethodResolver, TypedValue}
 import pl.touk.nussknacker.engine.definition.clazz.{ClassDefinitionSet, MethodDefinition}
-import pl.touk.nussknacker.engine.extension.ExtensionMethods.extensionMethodsHandlers
+import pl.touk.nussknacker.engine.extension.ExtensionMethods.extensionMethodsDefinitions
 
-import java.lang.reflect.{Method, Modifier}
 import java.util
 import scala.collection.concurrent.TrieMap
 
 class ExtensionMethodResolver(classDefinitionSet: ClassDefinitionSet) extends MethodResolver {
-  private type TargetConverter = ToExtensionMethodInvocationTargetConverter[_ <: ExtensionMethodInvocationTarget]
-
-  private val toInvocationTargetConvertersByHandler = extensionMethodsHandlers
-    .map(e => e -> e.createConverter(classDefinitionSet))
-    .toMap[ExtensionMethodsHandler[_], TargetConverter]
-
   private val executorsCache = new TrieMap[(String, Class[_]), Option[MethodExecutor]]()
 
   override def resolve(
       context: EvaluationContext,
       targetObject: Any,
       methodName: String,
       argumentTypes: util.List[TypeDescriptor]
-  ): MethodExecutor = {
+  ): MethodExecutor =
     maybeResolve(context, targetObject, methodName, argumentTypes).orNull
-  }
 
   def maybeResolve(
       context: EvaluationContext,
@@ -35,39 +26,24 @@ class ExtensionMethodResolver(classDefinitionSet: ClassDefinitionSet) extends Me
       argumentTypes: util.List[TypeDescriptor]
   ): Option[MethodExecutor] = {
     val targetClass = targetObject.getClass
-    executorsCache
-      .getOrElse(
-        (methodName, targetClass), {
-          val maybeExecutor = methodByConverter(targetClass, methodName, argumentTypes).map {
-            case (method, converter) => createExecutor(method, converter)
-          }
-          executorsCache.put((methodName, targetClass), maybeExecutor)
-          maybeExecutor
-        }
-      )
-  }
-
-  private def methodByConverter(
-      targetClass: Class[_],
-      methodName: String,
-      argumentTypes: util.List[TypeDescriptor]
-  ): Option[(Method, TargetConverter)] = {
-    toInvocationTargetConvertersByHandler
-      .filter(_._1.appliesToClassInRuntime(targetClass))
-      .flatMap { case (handler, converter) =>
-        handler
-          .findMethod(methodName, argumentTypes.size())
-          .map(method => method -> converter)
+    executorsCache.getOrElse(
+      (methodName, targetClass), {
+        val maybeExecutor = extensionMethodsDefinitions
+          .filter(_.appliesToClassInRuntime(targetClass))
+          .flatMap(_.createHandler(classDefinitionSet).findMethod(methodName, argumentTypes.size()))
+          .headOption
+          .map(createExecutor)
+        executorsCache.put((methodName, targetClass), maybeExecutor)
+        maybeExecutor
       }
-      .headOption
+    )
   }
 
-  private def createExecutor(method: Method, converter: TargetConverter): MethodExecutor =
+  private def createExecutor(method: ExtensionMethod): MethodExecutor =
     new MethodExecutor {
 
-      override def execute(context: EvaluationContext, target: Any, arguments: Object*): TypedValue = {
-        val value = converter.toInvocationTarget(target).invoke(method.getName, arguments.toArray)
-        new TypedValue(value, new TypeDescriptor(new MethodParameter(method, -1)).narrow(value))
+      override def execute(context: EvaluationContext, target: Any, args: Object*): TypedValue = {
+        new TypedValue(method.invoke(target, args: _*), null)
       }
 
     }
@@ -76,7 +52,7 @@ class ExtensionMethodResolver(classDefinitionSet: ClassDefinitionSet) extends Me
 
 object ExtensionMethods {
 
-  val extensionMethodsHandlers: List[ExtensionMethodsHandler[_ <: ExtensionMethodInvocationTarget]] = List(
+  val extensionMethodsDefinitions: List[ExtensionMethodsDefinition] = List(
     CastOrConversionExt,
     ArrayExt,
     ToLongConversionExt,
@@ -91,39 +67,33 @@ object ExtensionMethods {
     new ClassDefinitionSet(
       set.classDefinitionsMap.map { case (clazz, definition) =>
         clazz -> definition.copy(
-          methods = definition.methods ++ extensionMethodsHandlers.flatMap(_.extractDefinitions(clazz, set))
+          methods = definition.methods ++ extensionMethodsDefinitions.flatMap(_.extractDefinitions(clazz, set))
         )
       }.toMap // .toMap is needed by scala 2.12
     )
   }
 
 }
 
-trait ExtensionMethodInvocationTarget {
-  def invoke(methodName: String, arguments: Array[Object]): Any
+trait ExtensionMethod {
+  val argsSize = 0
+  def invoke(target: Any, args: Object*): Any
 }
 
-trait ExtensionMethodsHandler[T <: ExtensionMethodInvocationTarget] {
-  val invocationTargetClass: Class[T]
+trait ExtensionMethodHandler {
+  val methodRegistry: Map[String, ExtensionMethod]
 
-  private lazy val nonStaticMethods: Array[Method] =
-    invocationTargetClass.getDeclaredMethods
-      .filter(m => Modifier.isPublic(m.getModifiers) && !Modifier.isStatic(m.getModifiers))
+  def findMethod(methodName: String, argsSize: Int): Option[ExtensionMethod] =
+//    nonStaticMethods.find(m => m.getName.equals(methodName) && m.getParameterCount == argsSize) // todo: temp commented
+    methodRegistry.get(methodName).filter(_.argsSize == argsSize)
 
-  def findMethod(methodName: String, argsSize: Int): Option[Method] =
-    nonStaticMethods.find(m => m.getName.equals(methodName) && m.getParameterCount == argsSize)
+}
 
-  def createConverter(
-      set: ClassDefinitionSet
-  ): ToExtensionMethodInvocationTargetConverter[T]
+trait ExtensionMethodsDefinition {
+  def createHandler(set: ClassDefinitionSet): ExtensionMethodHandler
 
   def extractDefinitions(clazz: Class[_], set: ClassDefinitionSet): Map[String, List[MethodDefinition]]
 
   // For what classes is extension available in the runtime invocation
   def appliesToClassInRuntime(clazz: Class[_]): Boolean
 }
-
-trait ToExtensionMethodInvocationTargetConverter[T <: ExtensionMethodInvocationTarget] {
-  // This method should be as easy and lightweight as possible because it's fired with every method execution.
-  def toInvocationTarget(target: Any): T
-}
diff --git a/...piler/src/main/scala/pl/touk/nussknacker/engine/extension/ToBigDecimalConversionExt.scala b/...piler/src/main/scala/pl/touk/nussknacker/engine/extension/ToBigDecimalConversionExt.scala
@@ -3,33 +3,24 @@ package pl.touk.nussknacker.engine.extension
 import org.springframework.util.NumberUtils
 import pl.touk.nussknacker.engine.definition.clazz.ClassDefinitionSet
 
-import java.lang.{Boolean => JBoolean}
 import java.math.{BigDecimal => JBigDecimal, BigInteger => JBigInteger}
 import scala.util.Try
 
-class ToBigDecimalConversionExt(target: Any) extends ExtensionMethodInvocationTarget {
+class ToBigDecimalConversionExt extends ExtensionMethodHandler {
 
-  override def invoke(methodName: String, arguments: Array[Object]): Any = methodName match {
-    case "isBigDecimal"       => isBigDecimal()
-    case "toBigDecimal"       => toBigDecimal()
-    case "toBigDecimalOrNull" => toBigDecimalOrNull()
-    case _                    => throw new IllegalAccessException(s"Cannot find method with name: '$methodName'")
-  }
+  override val methodRegistry: Map[String, ExtensionMethod] = Map(
+    "isBigDecimal"       -> ((target: Any, _) => ToBigDecimalConversionExt.canConvert(target)),
+    "toBigDecimal"       -> ((target: Any, _) => ToBigDecimalConversionExt.convert(target)),
+    "toBigDecimalOrNull" -> ((target: Any, _) => ToBigDecimalConversionExt.convertOrNull(target)),
+  )
 
-  def isBigDecimal(): JBoolean          = ToBigDecimalConversionExt.canConvert(target)
-  def toBigDecimal(): JBigDecimal       = ToBigDecimalConversionExt.convert(target)
-  def toBigDecimalOrNull(): JBigDecimal = ToBigDecimalConversionExt.convertOrNull(target)
 }
 
-object ToBigDecimalConversionExt extends ConversionExt[ToBigDecimalConversionExt] with ToNumericConversion {
-  override val invocationTargetClass: Class[ToBigDecimalConversionExt] = classOf[ToBigDecimalConversionExt]
+object ToBigDecimalConversionExt extends ConversionExt with ToNumericConversion {
   override type ResultType = JBigDecimal
   override val resultTypeClass: Class[JBigDecimal] = classOf[JBigDecimal]
 
-  override def createConverter(
-      set: ClassDefinitionSet
-  ): ToExtensionMethodInvocationTargetConverter[ToBigDecimalConversionExt] =
-    (target: Any) => new ToBigDecimalConversionExt(target)
+  override def createHandler(set: ClassDefinitionSet): ExtensionMethodHandler = new ToBigDecimalConversionExt
 
   override def convertEither(value: Any): Either[Throwable, JBigDecimal] =
     value match {