fix(expressions): Restore delegation to default SpEL type converter (#…

…1075) * fix(expressions): Restore delegation to default SpEL type converter The type interceptor for detecting remote artifact URIs was not properly delegating `canConvert`, which resulted in only String -> String being auto-converted in SpEL. This breaks many expressions, as SpEL's default `StandardTypeConverter` handles common cases like int -> long. * Formatting
spinnaker · Jul 12, 2023 · 4febb41 · 4febb41
1 parent 221b939
commit 4febb41
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 7 deletions.
diff --git a/...src/main/java/com/netflix/spinnaker/kork/expressions/ArtifactUriToReferenceConverter.java b/...src/main/java/com/netflix/spinnaker/kork/expressions/ArtifactUriToReferenceConverter.java
@@ -37,19 +37,20 @@ public ArtifactUriToReferenceConverter(ArtifactStore artifactStore) {
 
  @Override
  public boolean canConvert(TypeDescriptor sourceType, @NotNull TypeDescriptor targetType) {
- if (sourceType == null) {
- return false;
-  }
+ return isArtifactUriType(sourceType, targetType)
+  || defaultTypeConverter.canConvert(sourceType, targetType);
+ }
 
- return sourceType.getObjectType() == String.class && targetType.getObjectType() == String.class;
+ private boolean isArtifactUriType(TypeDescriptor sourceType, @NotNull TypeDescriptor targetType) {
+ return sourceType != null
+ && sourceType.getObjectType() == String.class
+ && targetType.getObjectType() == String.class;
  }
 
  @Override
  public Object convertValue(
  Object value, TypeDescriptor sourceType, @NotNull TypeDescriptor targetType) {
- // For some obscene reason(s), SpEL does not use this in the
- // FunctionReference call when calling a method. So we call it internally
- if (!canConvert(sourceType, targetType)) {
+ if (!isArtifactUriType(sourceType, targetType)) {
  return defaultTypeConverter.convertValue(value, sourceType, targetType);
  }
 

diff --git a/...ressions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupportTest.java b/...ressions/src/test/java/com/netflix/spinnaker/kork/expressions/ExpressionsSupportTest.java
@@ -131,6 +131,26 @@ public void artifactReferenceInSpEL() {
  assertThat(evaluated).isEqualTo(expectedValue);
  }
 
+ @Test
+ public void delegatesTypeConversion() {
+ // If a thing is not an artifact URI, it should delegate to StandardTypeConverter
+ ExpressionProperties expressionProperties = new ExpressionProperties();
+
+ // StandardTypeConverter does things like convert ints to longs
+ String testInput = ("${new java.util.UUID(0,0).toString()}");
+ Map<String, Object> testContext = Map.of();
+
+ String evaluated =
+ new ExpressionTransform(parserContext, parser, Function.identity())
+ .transformString(
+ testInput,
+ new ExpressionsSupport(null, expressionProperties)
+ .buildEvaluationContext(testContext, true),
+ new ExpressionEvaluationSummary());
+
+ assertThat(evaluated).isEqualTo("00000000-0000-0000-0000-000000000000");
+ }
+
  public class MockArtifactStore extends ArtifactStore {
  public Map<String, String> cache = new HashMap<>();