Skip to content

Commit

Permalink
fix(expressions): Better handling for composite spel (#2135)
Browse files Browse the repository at this point in the history 
- broke out the main transform method into type specific ones
- small cosmetic changes: corrected inconsitency var names including allowUnknownKeys
- added a way to stringify certain types before returning
  • Loading branch information
@jeyrschabu
jeyrschabu authored Apr 10, 2018
1 parent 16acfdd commit 0f9ffdf
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 49 deletions.
2 changes: 1 addition & 1 deletion ...re/src/main/java/com/netflix/spinnaker/orca/pipeline/expressions/ExpressionEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@
import java.util.Map;

public interface ExpressionEvaluator {
Map<String, Object> evaluate(Map<String, Object> source, Object rootObject, ExpressionEvaluationSummary summary, boolean ignoreUnknownProperties);
Map<String, Object> evaluate(Map<String, Object> source, Object rootObject, ExpressionEvaluationSummary summary, boolean allowUnknownKeys);
}
132 changes: 88 additions & 44 deletions ...re/src/main/java/com/netflix/spinnaker/orca/pipeline/expressions/ExpressionTransform.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.pipeline.expressions;

import com.netflix.spinnaker.orca.ExecutionStatus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.expression.EvaluationContext;
Expand All @@ -30,73 +31,115 @@
import java.util.stream.Stream;

import static java.lang.String.format;
import static java.util.Collections.emptyMap;
import static java.util.Collections.emptySet;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;

public class ExpressionTransform {
private final Logger log = LoggerFactory.getLogger(getClass());

private static final List<String> EXECUTION_AWARE_FUNCTIONS = Arrays.asList("judgment", "judgement", "stage", "stageExists", "deployedServerGroups");
private static final List<String> EXECUTION_AWARE_ALIASES = Arrays.asList("deployedServerGroups");
private static final List<String> EXECUTION_AWARE_ALIASES = Collections.singletonList("deployedServerGroups");
private static final List<Class> STRINGIFYABLE_TYPES = Collections.singletonList(ExecutionStatus.class);
private final ParserContext parserContext;
private final ExpressionParser parser;

public ExpressionTransform(ParserContext parserContext, ExpressionParser parser) {
ExpressionTransform(ParserContext parserContext, ExpressionParser parser) {
this.parserContext = parserContext;
this.parser = parser;
}

public <T> T transform(T source, EvaluationContext evaluationContext, ExpressionEvaluationSummary summary) {
return transform(source, evaluationContext, summary, emptyMap());
}

/**
* Traverses and attempts to evaluate expressions
* Failures can either be INFO (for a simple unresolved expression) or ERROR when an exception is thrown
* @return the transformed source object
*/
public <T> T transform(T source, EvaluationContext evaluationContext, ExpressionEvaluationSummary summary, Map<String, ?> additionalContext) {
if (source == null) {
return null;
}

if (source instanceof Map) {
Map<String, Object> copy = Collections.unmodifiableMap((Map) source);
Map<String, Object> result = new HashMap<>();
for (Map.Entry<String, Object> entry : ((Map<String, Object>) source).entrySet()) {
public Map<String, Object> transformMap(Map<String, Object> source,
EvaluationContext evaluationContext,
ExpressionEvaluationSummary summary) {
Map<String, Object> result = new HashMap<>();
Map<String, Object> copy = Collections.unmodifiableMap(source);
source.forEach((key, value) -> {
if (value instanceof Map) {
result.put(
transform(entry.getKey(), evaluationContext, summary, copy),
transform(entry.getValue(), evaluationContext, summary, copy)
transformString(key, evaluationContext, summary, copy).toString(),
transformMap((Map) value, evaluationContext, summary)
);
} else if (value instanceof List) {
result.put(
transformString(key, evaluationContext, summary, copy).toString(),
transformList((List) value, evaluationContext, summary, copy)
);
} else {
result.put(
transformString(key, evaluationContext, summary, copy).toString(),
transformString(value, evaluationContext, summary, copy)
);
}
return (T) result;
} else if (source instanceof List) {
return (T) ((List) source).stream().map(it ->
transform(it, evaluationContext, summary)
).collect(toList());
} else if ((source instanceof CharSequence) && source.toString().contains(parserContext.getExpressionPrefix())) {
String literalExpression = source.toString();
literalExpression = includeExecutionParameter(literalExpression);

T result = null;
Expression exp;
});

return result;
}

private List transformList(List source,
EvaluationContext evaluationContext,
ExpressionEvaluationSummary summary,
Map<String, Object> additionalContext) {
List<Object> result = new ArrayList<>();
for (Object obj : source) {
if (obj instanceof Map) {
result.add(
transformMap((Map<String, Object>) obj, evaluationContext, summary)
);
} else if (obj instanceof List) {
result.addAll(
transformList((List) obj, evaluationContext, summary, additionalContext)
);
} else {
result.add(
transformString(obj, evaluationContext, summary, additionalContext)
);
}
}

return result;
}

private Object transformString(Object source,
EvaluationContext evaluationContext,
ExpressionEvaluationSummary summary,
Map<String, Object> additionalContext) {
if (source instanceof String && source.toString().contains(parserContext.getExpressionPrefix())) {
String literalExpression = includeExecutionParameter(source.toString());
Object result = null;
String escapedExpressionString = null;
Throwable exception = null;
try {
exp = parser.parseExpression(literalExpression, parserContext);
Expression exp = parser.parseExpression(literalExpression, parserContext);
escapedExpressionString = escapeExpression(exp);
result = (T) exp.getValue(evaluationContext);
if (exp instanceof CompositeStringExpression) {
StringBuilder sb = new StringBuilder();
Expression[] expressions = ((CompositeStringExpression) exp).getExpressions();
for (Expression e : expressions) {
String value = e.getValue(evaluationContext, String.class);
if (value == null) {
value = String.format("${%s}", e.getExpressionString());
}
sb.append(value);
}

result = sb.toString();
} else {
result = exp.getValue(evaluationContext);
}
} catch (Exception e) {
log.info("Failed to evaluate {}, returning raw value {}", source, e.getMessage());
exception = e;
} finally {
Set keys = getKeys(source, additionalContext);
Object fields = !keys.isEmpty() ? keys : literalExpression;
String errorDescription = format("Failed to evaluate %s ", fields);
escapedExpressionString = escapedExpressionString != null ? escapedExpressionString : escapeSimpleExpression(source.toString());
if (exception != null) {
Set<String> keys = getKeys(source, additionalContext);
Object fields = !keys.isEmpty() ? keys : literalExpression;
String errorDescription = format("Failed to evaluate %s ", fields);
Throwable originalException = unwrapOriginalException(exception);
if (originalException == null || originalException.getMessage() == null || originalException.getMessage().contains(exception.getMessage())) {
errorDescription += exception.getMessage();
Expand All @@ -113,9 +156,6 @@ public <T> T transform(T source, EvaluationContext evaluationContext, Expression

result = source;
} else if (result == null) {
Set<String> keys = getKeys(source, additionalContext);
Object fields = !keys.isEmpty() ? keys : literalExpression;
String errorDescription = format("Failed to evaluate %s ", fields);
summary.add(
escapedExpressionString,
ExpressionEvaluationSummary.Result.Level.INFO,
Expand All @@ -130,16 +170,20 @@ public <T> T transform(T source, EvaluationContext evaluationContext, Expression
summary.incrementTotalEvaluated();
}

if (STRINGIFYABLE_TYPES.contains(result.getClass())) {
result = result.toString();
}

return result;
} else {
return source;
}

return source;
}

/**
* finds parent keys by value in a nested map
*/
private static Set<String> getKeys(Object value, final Map<String, ?> map) {
private Set<String> getKeys(Object value, final Map<String, Object> map) {
if (map == null || map.isEmpty()) {
return emptySet();
}
Expand All @@ -148,7 +192,7 @@ private static Set<String> getKeys(Object value, final Map<String, ?> map) {
.entrySet()
.stream()
.filter(it ->
flatten(it.getValue()).collect(toSet()).stream().flatMap(Stream::of).collect(toSet())/*.flatten()*/.contains(value)
flatten(it.getValue()).collect(toSet()).stream().flatMap(Stream::of).collect(toSet()).contains(value)
)
.map(Map.Entry::getKey)
.collect(toSet());
Expand Down Expand Up @@ -179,7 +223,7 @@ private static Throwable unwrapOriginalException(Throwable e) {
/**
* Helper to escape an expression: stripping ${ }
*/
static String escapeExpression(Expression expression) {
private String escapeExpression(Expression expression) {
if (expression instanceof CompositeStringExpression) {
StringBuilder sb = new StringBuilder();
for (Expression e : ((CompositeStringExpression) expression).getExpressions()) {
Expand Down Expand Up @@ -212,7 +256,7 @@ static String escapeSimpleExpression(String expression) {
* @param e #stage('property') becomes #stage(#root.execution, 'property')
* @return an execution aware helper function
*/
private static String includeExecutionParameter(String e) {
private String includeExecutionParameter(String e) {
String expression = e;
for (String fn : EXECUTION_AWARE_FUNCTIONS) {
if (expression.contains("#" + fn) && !expression.contains("#" + fn + "( #root.execution, ")) {
Expand Down
6 changes: 3 additions & 3 deletions ...ain/java/com/netflix/spinnaker/orca/pipeline/expressions/PipelineExpressionEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ public PipelineExpressionEvaluator(final ContextFunctionConfiguration contextFun
}

@Override
public Map<String, Object> evaluate(Map<String, Object> source, Object rootObject, ExpressionEvaluationSummary summary, boolean ignoreUnknownProperties) {
StandardEvaluationContext evaluationContext = newEvaluationContext(rootObject, ignoreUnknownProperties);
return new ExpressionTransform(parserContext, parser).transform(source, evaluationContext, summary);
public Map<String, Object> evaluate(Map<String, Object> source, Object rootObject, ExpressionEvaluationSummary summary, boolean allowUnknownKeys) {
StandardEvaluationContext evaluationContext = newEvaluationContext(rootObject, allowUnknownKeys);
return new ExpressionTransform(parserContext, parser).transformMap(source, evaluationContext, summary);
}
}

Expand Down
34 changes: 33 additions & 1 deletion ...test/groovy/com/netflix/spinnaker/orca/pipeline/util/ContextParameterProcessorSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class ContextParameterProcessorSpec extends Specification {
'support SPEL string methods with args' | '${ replaceTest.replaceAll("-","") }' | 'stackwithhyphens'
'make any string alphanumerical for deploy' | '${ #alphanumerical(replaceTest) }' | 'stackwithhyphens'
'make any string alphanumerical for deploy' | '''${#readJson('{ "newValue":"two" }')[#root.replaceMe]}''' | 'two' // [#root.parameters.cluster]
'support composite expression' | 'a.${replaceMe}.b.${replaceMe}' | 'a.newValue.b.newValue'
'leave unresolved expressions alone' | 'a.${ENV1}.b.${ENV2}' | 'a.${ENV1}.b.${ENV2}'
'support partial resolution for composites' | 'a.${ENV1}.b.${replaceMe}' | 'a.${ENV1}.b.newValue'
}

@Unroll
Expand Down Expand Up @@ -205,7 +208,6 @@ class ContextParameterProcessorSpec extends Specification {

where:
desc | sourceValue | expectedValue | allowUnknownKeys
'should blank out null' | '${noexists}-foo' | '-foo' | true
'should leave alone non existing' | '${noexists}-foo' | '${noexists}-foo' | false
'should handle elvis' | '${noexists ?: "bacon"}' | 'bacon' | true
'should leave elvis expression untouched' | '${noexists ?: "bacon"}' | '${noexists ?: "bacon"}' | false
Expand Down Expand Up @@ -856,6 +858,36 @@ class ContextParameterProcessorSpec extends Specification {
notThrown(SpelHelperFunctionException)
}

def "should get stage status as String"() {
given:
def pipe = pipeline {
stage {
refId = "1"
type = "wait"
name = "wait stage"
context = [status: '${#stage("manualJudgment stage")["status"]}']
}
stage {
refId = "2"
type = "manualJudgment"
name = "manualJudgment stage"
status = SUCCEEDED
context = [judgmentInput: "Real Judgment input"]
}
}

and:
def stage = pipe.stageByRef("1")
def ctx = contextParameterProcessor.buildExecutionContext(stage, true)

when:
def result = contextParameterProcessor.process(stage.context, ctx, true)

then:
result.status == "SUCCEEDED"
result.status instanceof String
}

static escapeExpression(String expression) {
return ExpressionTransform.escapeSimpleExpression(expression)
}
Expand Down

0 comments on commit 0f9ffdf

Please sign in to comment.