Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NU-1836] Handle extension methods in NoParamMethodPropertyAccessor #7044

Closed
wants to merge 1 commit into from
Closed

[NU-1836] Handle extension methods in NoParamMethodPropertyAccessor #7044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .../src/main/scala/pl/touk/nussknacker/engine/spel/internal/OptimizedEvaluationContext.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
44 changes: 35 additions & 9 deletions ...-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/propertyAccessors.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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(),
Expand All @@ -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
Expand All @@ -51,28 +53,52 @@ 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(
propertyName: String,
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
Expand Down
24 changes: 12 additions & 12 deletions scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down
Loading