Skip to content

Commit

Permalink
[NU-1836] Handle extension methods in NoParamMethodPropertyAccessor a…
Browse files Browse the repository at this point in the history 
…nd PrimitiveTypePropertyAccessor
  • Loading branch information
Łukasz Bigorajski committed Oct 24, 2024
1 parent 494a0c7 commit 7169eec
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 35 deletions.
2 changes: 1 addition & 1 deletion designer/server/src/test/scala/pl/touk/nussknacker/ui/api/ExpressionSuggesterSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ class ExpressionSuggesterSpec
)
}

test("should suggest parameters for casts methods") {
test("should suggest parameters for conversion methods") {
spelSuggestionsFor("#unknown.to('')", column = 13) should contain theSameElementsAs List(
suggestion("BigDecimal", Typed[java.math.BigDecimal]),
suggestion("Boolean", Typed[java.lang.Boolean]),
Expand Down
11 changes: 9 additions & 2 deletions scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,12 @@ private[spel] class Typer(
val w = Writer.value[List[ExpressionParseError], TypingResult](Unknown)
if (methodExecutionForUnknownAllowed)
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 @@ -679,6 +683,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
45 changes: 35 additions & 10 deletions ...-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/internal/propertyAccessors.scala
View file
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 Down Expand Up @@ -52,33 +54,56 @@ object propertyAccessors {
however it's not so easy to extend and in interpreted mode we skip original implementation
*/
object NoParamMethodPropertyAccessor extends ReflectivePropertyAccessor with ReadOnly with Caching {
private val methodInvoker = new ExtensionsAwareMethodInvoker()
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
// *do not* extend ReflectivePropertyAccessor and we force interpreted mode
// TODO: figure out how to make bytecode generation work also in this case
object PrimitiveOrWrappersPropertyAccessor extends PropertyAccessor with ReadOnly with Caching {
private val methodInvoker = new ExtensionsAwareMethodInvoker()
private val emptyArray = Array[AnyRef]()

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

Expand All @@ -87,12 +112,12 @@ object propertyAccessors {
method: Method,
target: Any,
context: EvaluationContext
): Any = method.invoke(target)
): Any = methodInvoker.invoke(method)(target, emptyArray)

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)
}

}
Expand Down
48 changes: 26 additions & 22 deletions scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1448,17 +1448,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 @@ -1663,25 +1663,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 @@ -1718,6 +1718,10 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
}
}

test("should allow use no param method property accessor on unknown") {
evaluate[String]("#unknownString.value.toString") shouldBe "unknown"
}

}

case class SampleObject(list: java.util.List[SampleValue])
Expand Down

0 comments on commit 7169eec

Please sign in to comment.