From 7644e98c86a5be6936570fcf1ea3583ac1ad16f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Bigorajski?= Date: Fri, 18 Oct 2024 14:54:18 +0200 Subject: [PATCH] [NU-1836] Handle extension methods in NoParamMethodPropertyAccessor --- .../internal/OptimizedEvaluationContext.scala | 4 +- .../spel/internal/propertyAccessors.scala | 44 +++++++++++++++---- .../engine/spel/SpelExpressionSpec.scala | 24 +++++----- 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/OptimizedEvaluationContext.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/OptimizedEvaluationContext.scala index 9d1b062316b..292c081c7e0 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/OptimizedEvaluationContext.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/OptimizedEvaluationContext.scala @@ -93,7 +93,9 @@ object EvaluationContextPreparer { classDefinitionSet: ClassDefinitionSet ): EvaluationContextPreparer = { val conversionService = determineConversionService(expressionConfig) - val propertyAccessors = internal.propertyAccessors.configured() + val methodInvoker = new ExtensionsAwareMethodInvoker(classLoader, classDefinitionSet) + val propertyAccessors = internal.propertyAccessors + .configured(methodInvoker) new EvaluationContextPreparer( classLoader, expressionConfig.globalImports, diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/propertyAccessors.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/propertyAccessors.scala index 58f92dc14c2..8db0b956f6f 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/propertyAccessors.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/propertyAccessors.scala @@ -4,9 +4,11 @@ import java.lang.reflect.{Method, Modifier} import java.util.Optional import org.apache.commons.lang3.ClassUtils import org.springframework.expression.spel.support.ReflectivePropertyAccessor +import org.springframework.expression.spel.support.ReflectivePropertyAccessor.OptimalPropertyAccessor import org.springframework.expression.{EvaluationContext, PropertyAccessor, TypedValue} import pl.touk.nussknacker.engine.api.dict.DictInstance import pl.touk.nussknacker.engine.api.exception.NonTransientException +import pl.touk.nussknacker.engine.extension.{ExtensionAwareMethodsDiscovery, ExtensionsAwareMethodInvoker} import scala.collection.concurrent.TrieMap @@ -15,7 +17,7 @@ object propertyAccessors { // Order of accessors matters - property from first accessor that returns `true` from `canRead` will be chosen. // This general order can be overridden - each accessor can define target classes for which it will have precedence - // through the `getSpecificTargetClasses` method. - def configured(): Seq[PropertyAccessor] = { + def configured(methodInvoker: ExtensionsAwareMethodInvoker): Seq[PropertyAccessor] = { Seq( MapPropertyAccessor, // must be before NoParamMethodPropertyAccessor and ReflectivePropertyAccessor new ReflectivePropertyAccessor(), @@ -25,7 +27,7 @@ object propertyAccessors { PrimitiveOrWrappersPropertyAccessor, StaticPropertyAccessor, TypedDictInstancePropertyAccessor, // must be before NoParamMethodPropertyAccessor - NoParamMethodPropertyAccessor, + new NoParamMethodPropertyAccessor(methodInvoker), // it can add performance overhead so it will be better to keep it on the bottom MapLikePropertyAccessor, MapMissingPropertyToNullAccessor, // must be after NoParamMethodPropertyAccessor @@ -51,16 +53,21 @@ object propertyAccessors { This one is a bit tricky. We extend ReflectivePropertyAccessor, as it's the only sensible way to make it compilable, however it's not so easy to extend and in interpreted mode we skip original implementation */ - object NoParamMethodPropertyAccessor extends ReflectivePropertyAccessor with ReadOnly with Caching { + class NoParamMethodPropertyAccessor(methodInvoker: ExtensionsAwareMethodInvoker) + extends ReflectivePropertyAccessor + with ReadOnly + with Caching { + + private val emptyArray = Array[AnyRef]() override def findGetterForProperty(propertyName: String, clazz: Class[_], mustBeStatic: Boolean): Method = { findMethodFromClass(propertyName, clazz).orNull } override protected def reallyFindMethod(name: String, target: Class[_]): Option[Method] = { - target.getMethods.find(m => - !ClassUtils.isPrimitiveOrWrapper(target) && m.getParameterCount == 0 && m.getName == name - ) + ExtensionAwareMethodsDiscovery + .discover(target) + .find(m => !ClassUtils.isPrimitiveOrWrapper(target) && m.getParameterCount == 0 && m.getName == name) } override protected def invokeMethod( @@ -68,11 +75,30 @@ object propertyAccessors { method: Method, target: Any, context: EvaluationContext - ): AnyRef = { - method.invoke(target) - } + ): Any = + methodInvoker.invoke(method)(target, emptyArray) override def getSpecificTargetClasses: Array[Class[_]] = null + + override def createOptimalAccessor(context: EvaluationContext, target: Any, name: String): PropertyAccessor = + super.createOptimalAccessor(context, target, name) match { + case o: OptimalPropertyAccessor => new NuOptimalAccessor(o) + case o => o + } + + private class NuOptimalAccessor(delegate: PropertyAccessor) extends PropertyAccessor { + override def getSpecificTargetClasses: Array[Class[_]] = + delegate.getSpecificTargetClasses + override def canWrite(context: EvaluationContext, target: Any, name: String): Boolean = + delegate.canWrite(context, target, name) + override def write(context: EvaluationContext, target: Any, name: String, newValue: Any): Unit = + delegate.write(context, target, name, newValue) + override def canRead(context: EvaluationContext, target: Any, name: String): Boolean = + NoParamMethodPropertyAccessor.this.canRead(context, target, name) + override def read(context: EvaluationContext, target: Any, name: String): TypedValue = + NoParamMethodPropertyAccessor.this.read(context, target, name) + } + } // Spring bytecode generation fails when we try to invoke methods on primitives, so we diff --git a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala index 0e139f20d5b..fc52852b509 100644 --- a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala +++ b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala @@ -1522,32 +1522,32 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD ("expression", "expectedType", "expectedResult"), ("1.toLong()", longTyping, 1), ("1.1.toLong()", longTyping, 1), - ("'1'.toLong()", longTyping, 1), + ("'1'.toLong", longTyping, 1), ("1.toLongOrNull()", longTyping, 1), ("1.1.toLongOrNull()", longTyping, 1), - ("'1'.toLongOrNull()", longTyping, 1), - ("'a'.toLongOrNull()", longTyping, null), + ("'1'.toLongOrNull", longTyping, 1), + ("'a'.toLongOrNull", longTyping, null), ("#unknownBoolean.value.toLongOrNull()", longTyping, null), ("1.toDouble()", doubleTyping, 1.0), ("1.1.toDouble()", doubleTyping, 1.1), - ("'1'.toDouble()", doubleTyping, 1.0), + ("'1'.toDouble", doubleTyping, 1.0), ("1.toDoubleOrNull()", doubleTyping, 1.0), ("1.1.toDoubleOrNull()", doubleTyping, 1.1), - ("'1'.toDoubleOrNull()", doubleTyping, 1.0), - ("'a'.toDoubleOrNull()", doubleTyping, null), + ("'1'.toDoubleOrNull", doubleTyping, 1.0), + ("'a'.toDoubleOrNull", doubleTyping, null), ("#unknownBoolean.value.toDoubleOrNull()", doubleTyping, null), ("1.toBigDecimal()", bigDecimalTyping, BigDecimal(1).bigDecimal), ("1.1.toBigDecimal()", bigDecimalTyping, BigDecimal(1.1).bigDecimal), - ("'1'.toBigDecimal()", bigDecimalTyping, BigDecimal(1).bigDecimal), + ("'1'.toBigDecimal", bigDecimalTyping, BigDecimal(1).bigDecimal), ("1.toBigDecimalOrNull()", bigDecimalTyping, BigDecimal(1).bigDecimal), ("1.1.toBigDecimalOrNull()", bigDecimalTyping, BigDecimal(1.1).bigDecimal), - ("'1'.toBigDecimalOrNull()", bigDecimalTyping, BigDecimal(1).bigDecimal), - ("'a'.toBigDecimalOrNull()", bigDecimalTyping, null), + ("'1'.toBigDecimalOrNull", bigDecimalTyping, BigDecimal(1).bigDecimal), + ("'a'.toBigDecimalOrNull", bigDecimalTyping, null), ("#unknownBoolean.value.toBigDecimalOrNull()", bigDecimalTyping, null), - ("'true'.toBoolean()", booleanTyping, true), + ("'true'.toBoolean", booleanTyping, true), ("#unknownInteger.value.toBooleanOrNull()", booleanTyping, null), - ("'a'.toBooleanOrNull()", booleanTyping, null), - ("'true'.toBooleanOrNull()", booleanTyping, true), + ("'a'.toBooleanOrNull", booleanTyping, null), + ("'true'.toBooleanOrNull", booleanTyping, true), ("#unknownBoolean.value.toBoolean()", booleanTyping, false), ) ) { (expression, expectedType, expectedResult) =>