Skip to content

Commit

Permalink
[NU-1836] Handle extension methods in PropertyAccessors (#7118)
Browse files Browse the repository at this point in the history
  • Loading branch information
lukasz-bigorajski authored Nov 4, 2024
1 parent 1456449 commit 25bb577
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ class ExtensionsAwareMethodInvoker(classDefinitionSet: ClassDefinitionSet) {
toInvocationTargetConvertersByClass
.get(method.getDeclaringClass)
.map(_.toInvocationTarget(target))
// Maybe in future we could write some mechanism to invoke extension methods statically. What I mean is to
// find correct extension based on target and then implement simple switch to fire method based on name
.map(impl => method.invoke(impl, arguments: _*))
.getOrElse {
throw new IllegalArgumentException(s"Extension method: ${method.getName} is not implemented")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ private[spel] class Typer(
staticMethodInvocationsChecking: Boolean,
classDefinitionSet: ClassDefinitionSet,
evaluationContextPreparer: EvaluationContextPreparer,
methodExecutionForUnknownAllowed: Boolean,
anyMethodExecutionForUnknownAllowed: Boolean,
dynamicPropertyAccessAllowed: Boolean
) extends LazyLogging {

Expand All @@ -73,7 +73,7 @@ private[spel] class Typer(
private lazy val evaluationContext: EvaluationContext =
evaluationContextPreparer.prepareEvaluationContext(Context(""), Map.empty)

private val methodReferenceTyper = new MethodReferenceTyper(classDefinitionSet, methodExecutionForUnknownAllowed)
private val methodReferenceTyper = new MethodReferenceTyper(classDefinitionSet, anyMethodExecutionForUnknownAllowed)

private lazy val typeReferenceTyper = new TypeReferenceTyper(evaluationContext, classDefinitionSet)

Expand Down Expand Up @@ -607,10 +607,14 @@ private[spel] class Typer(
private def extractProperty(e: PropertyOrFieldReference, t: TypingResult): TypingR[TypingResult] = t match {
case Unknown =>
val w = Writer.value[List[ExpressionParseError], TypingResult](Unknown)
if (methodExecutionForUnknownAllowed)
if (anyMethodExecutionForUnknownAllowed) {
w
else
w.tell(List(IllegalPropertyAccessError(Unknown)))
} else {
// we allow some methods to be used on unknown
unknownPropertyTypeBasedOnMethod(e)
.map(valid)
.getOrElse(w.tell(List(IllegalPropertyAccessError(Unknown))))
}
case TypedNull =>
invalid(IllegalPropertyAccessError(TypedNull), fallbackType = TypedNull)
case s: SingleTypingResult =>
Expand Down Expand Up @@ -681,6 +685,9 @@ private[spel] class Typer(
classDefinitionSet.get(clazz.klass).flatMap(_.getPropertyOrFieldType(invocationTarget, e.getName))
}

private def unknownPropertyTypeBasedOnMethod(e: PropertyOrFieldReference): Option[TypingResult] =
classDefinitionSet.unknown.flatMap(_.getPropertyOrFieldType(Unknown, e.getName))

private def extractIterativeType(parent: TypingResult): TypingR[TypingResult] = parent match {
case tc: SingleTypingResult
if tc.runtimeObjType.canBeSubclassOf(Typed[java.util.Collection[_]]) ||
Expand Down Expand Up @@ -762,7 +769,7 @@ private[spel] class Typer(
staticMethodInvocationsChecking,
classDefinitionSet,
evaluationContextPreparer,
methodExecutionForUnknownAllowed,
anyMethodExecutionForUnknownAllowed,
dynamicPropertyAccessAllowed
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ object EvaluationContextPreparer {
val conversionService = determineConversionService(expressionConfig)
val propertyAccessors =
internal.propertyAccessors.configured(
MethodAccessChecker.create(classDefinitionSet, expressionConfig.dynamicPropertyAccessAllowed)
MethodAccessChecker.create(classDefinitionSet, expressionConfig.dynamicPropertyAccessAllowed),
classDefinitionSet,
)
new EvaluationContextPreparer(
classLoader,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import org.springframework.util.ClassUtils
import pl.touk.nussknacker.engine.api.dict.DictInstance
import pl.touk.nussknacker.engine.api.exception.NonTransientException
import pl.touk.nussknacker.engine.definition.clazz.ClassDefinitionSet
import pl.touk.nussknacker.engine.extension.{ExtensionAwareMethodsDiscovery, ExtensionsAwareMethodInvoker}

import java.lang.reflect.{Field, Method, Modifier}
import java.util.Optional
Expand All @@ -17,7 +18,8 @@ object propertyAccessors {
// This general order can be overridden - each accessor can define target classes for which it will have precedence -
// through the `getSpecificTargetClasses` method.
def configured(
accessChecker: MethodAccessChecker
accessChecker: MethodAccessChecker,
set: ClassDefinitionSet
): Seq[PropertyAccessor] = {
Seq(
MapPropertyAccessor, // must be before NoParamMethodPropertyAccessor and ReflectivePropertyAccessor
Expand All @@ -29,9 +31,10 @@ object propertyAccessors {
new StaticPropertyAccessor(accessChecker),
TypedDictInstancePropertyAccessor, // must be before NoParamMethodPropertyAccessor
new NoParamMethodPropertyAccessor(accessChecker),
new ExtensionMethodsPropertyAccessor(accessChecker, set),
// it can add performance overhead so it will be better to keep it on the bottom
MapLikePropertyAccessor,
MapMissingPropertyToNullAccessor, // must be after NoParamMethodPropertyAccessor
MapMissingPropertyToNullAccessor, // must be after NoParamMethodPropertyAccessor, ExtensionMethodsPropertyAccessor
)
}

Expand Down Expand Up @@ -304,6 +307,34 @@ object propertyAccessors {
override def getSpecificTargetClasses: Array[Class[_]] = null
}

// It's not optimized to generate byteCode like in ReflectivePropertyAccessor so it's running only in interpreted mode
private class ExtensionMethodsPropertyAccessor(
accessChecker: MethodAccessChecker,
set: ClassDefinitionSet
) extends PropertyAccessor
with ReadOnly
with Caching {
private val methodInvoker = new ExtensionsAwareMethodInvoker(set)
private val emptyArray = Array[AnyRef]()

override def getSpecificTargetClasses: Array[Class[_]] = null

override protected def invokeMethod(
propertyName: String,
method: Method,
target: Any,
context: EvaluationContext
): Any = methodInvoker.invoke(method)(target, emptyArray)

override protected def reallyFindMethod(name: String, target: Class[_]): Option[Method] =
accessChecker.checkAccessIfMethodFound(target) {
ExtensionAwareMethodsDiscovery
.discover(target)
.find(m => m.getParameterCount == 0 && m.getName == name)
}

}

trait Caching extends CachingBase { self: PropertyAccessor =>

override def canRead(context: EvaluationContext, target: scala.Any, name: String): Boolean =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1618,17 +1618,17 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
Table(
("expression", "expectedType", "expectedResult"),
(
"#stringMap.![{key: #this.key + '_k', value: #this.value + '_v'}].toMap()",
"#stringMap.![{key: #this.key + '_k', value: #this.value + '_v'}].toMap",
mapStringStringType,
Map("foo_k" -> "bar_v", "baz_k" -> "qux_v").asJava
),
(
"#mapWithDifferentValueTypes.![{key: #this.key, value: #this.value}].toMap()",
"#mapWithDifferentValueTypes.![{key: #this.key, value: #this.value}].toMap",
mapStringUnknownType,
mapWithDifferentValueTypes
),
(
"#nullableMap.![{key: #this.key, value: #this.value}].toMap()",
"#nullableMap.![{key: #this.key, value: #this.value}].toMap",
mapStringStringType,
nullableMap
)
Expand Down Expand Up @@ -1872,25 +1872,25 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
("#unknownMap.value.is('Map')", true),
("#unknownListOfTuples.value.is('List')", true),
("#unknownListOfTuples.value.is('Map')", true),
("#unknownBoolean.value.isBoolean()", true),
("#unknownBooleanString.value.isBoolean()", true),
("#unknownString.value.isBoolean()", false),
("#unknownLong.value.isLong()", true),
("#unknownLongString.value.isLong()", true),
("#unknownString.value.isLong()", false),
("#unknownDouble.value.isDouble()", true),
("#unknownDoubleString.value.isDouble()", true),
("#unknownString.value.isDouble()", false),
("#unknownBigDecimal.value.isBigDecimal()", true),
("#unknownBigDecimalString.value.isBigDecimal()", true),
("#unknownString.value.isBigDecimal()", false),
("#unknownList.value.isList()", true),
("#unknownList.value.isMap()", false),
("#unknownMap.value.isList()", false),
("#unknownMap.value.isMap()", true),
("#unknownListOfTuples.value.isList()", true),
("#unknownListOfTuples.value.isMap()", true),
("#arrayOfUnknown.isList()", true),
("#unknownBoolean.value.isBoolean", true),
("#unknownBooleanString.value.isBoolean", true),
("#unknownString.value.isBoolean", false),
("#unknownLong.value.isLong", true),
("#unknownLongString.value.isLong", true),
("#unknownString.value.isLong", false),
("#unknownDouble.value.isDouble", true),
("#unknownDoubleString.value.isDouble", true),
("#unknownString.value.isDouble", false),
("#unknownBigDecimal.value.isBigDecimal", true),
("#unknownBigDecimalString.value.isBigDecimal", true),
("#unknownString.value.isBigDecimal", false),
("#unknownList.value.isList", true),
("#unknownList.value.isMap", false),
("#unknownMap.value.isList", false),
("#unknownMap.value.isMap", true),
("#unknownListOfTuples.value.isList", true),
("#unknownListOfTuples.value.isMap", true),
("#arrayOfUnknown.isList", true),
)
) { (expression, result) =>
evaluate[Any](expression, customCtx) shouldBe result
Expand Down Expand Up @@ -1933,6 +1933,13 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
parsed.evaluateSync[Any](ctx) shouldBe List("a").asJava
}

test("should allow use no param method property accessor on unknown") {
val customCtx = ctx.withVariable("unknownInt", ContainerOfUnknown("11"))
val parsed = parse[Any]("#unknownInt.value.toLongOrNull", customCtx).validValue
parsed.evaluateSync[Any](customCtx) shouldBe 11
parsed.evaluateSync[Any](customCtx) shouldBe 11
}

}

case class SampleObject(list: java.util.List[SampleValue])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class TyperSpec extends AnyFunSuite with Matchers with ValidatedValuesDetailedMe
staticMethodInvocationsChecking = false,
classDefinitionSet = ClassDefinitionTestUtils.createDefinitionWithDefaultsAndExtensions,
evaluationContextPreparer = null,
methodExecutionForUnknownAllowed = false,
anyMethodExecutionForUnknownAllowed = false,
dynamicPropertyAccessAllowed = dynamicPropertyAccessAllowed
)

Expand Down

0 comments on commit 25bb577

Please sign in to comment.