Skip to content

Commit

Permalink
GH-3114: Honor SpEL contract in ExpressionEvalMap
Browse files Browse the repository at this point in the history 
Fixes #3114

The contract of SpEL with its
`getValue(EvaluationContext context, @nullable Object rootObject)` is
that we need to deal with provided `rootObject` even if it is `null`
and don't consult with `context.getRootObject()`

* Fix `ExpressionEvalMap` to have an internal `rootExplicitlySet`
to indicate that `root` explicitly provided by consumer, even if it is null.
According this flag call respective `Expression.getValue()`
* Add `@Nullable` to methods and their arguments into `ExpressionEvalMap`
& `FunctionExpression` to honor `Expression` contracts
* Populate an `HttpEntity` explicitly into `ExpressionEvalMap` from the
`HttpRequestHandlingEndpointSupport` and `WebFluxInboundEndpoint` for
full picture

* Fix JavaDocs indentations in the `ExpressionEvalMap`
  • Loading branch information
@artembilan @garyrussell
artembilan authored and garyrussell committed Nov 25, 2019
1 parent ee7be04 commit 516ecbc
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 35 deletions.
51 changes: 36 additions & 15 deletions ...tion-core/src/main/java/org/springframework/integration/expression/ExpressionEvalMap.java
View file
Expand Up @@ -47,13 +47,13 @@
* <pre class="code">
* {@code
*ExpressionEvalMap evalMap = ExpressionEvalMap
* .from(expressions)
* .usingCallback(new EvaluationCallback() {
* Object evaluate(Expression expression) {
* // return some expression evaluation
* }
* })
* .build();
* .from(expressions)
* .usingCallback(new EvaluationCallback() {
* Object evaluate(Expression expression) {
* // return some expression evaluation
* }
* })
* .build();
*}
* </pre>
* <p>
Expand Down Expand Up @@ -84,6 +84,7 @@ private ExpressionEvalMap(Map<String, ?> original, EvaluationCallback evaluation
* from {@link #original} and returns the result of evaluation using {@link #evaluationCallback}.
*/
@Override
@Nullable
public Object get(Object key) {
Object value = this.original.get(key);
if (value != null) {
Expand All @@ -106,9 +107,9 @@ else if (value instanceof String) {

@Override
public Set<Map.Entry<String, Object>> entrySet() {
return this.original.entrySet()
return this.original.keySet()
.stream()
.map(e -> new SimpleImmutableEntry<>(e.getKey(), get(e.getKey())))
.map((key) -> new SimpleImmutableEntry<>(key, get(key)))
.collect(Collectors.toSet());
}

Expand Down Expand Up @@ -206,22 +207,35 @@ public interface EvaluationCallback {
*/
public static class ComponentsEvaluationCallback implements EvaluationCallback {

@Nullable
private final EvaluationContext context;

@Nullable
private final Object root;

private final boolean rootExplicitlySet;

@Nullable
private final Class<?> returnType;

public ComponentsEvaluationCallback(EvaluationContext context, Object root, Class<?> returnType) {
public ComponentsEvaluationCallback(@Nullable EvaluationContext context, @Nullable Object root,
boolean rootExplicitlySet, @Nullable Class<?> returnType) {

this.context = context;
this.root = root;
this.rootExplicitlySet = rootExplicitlySet;
this.returnType = returnType;
}

@Override
public Object evaluate(Expression expression) {
if (this.context != null) {
return expression.getValue(this.context, this.root, this.returnType);
if (this.rootExplicitlySet) {
return expression.getValue(this.context, this.root, this.returnType);
}
else {
return expression.getValue(this.context, this.returnType);
}
}
return expression.getValue(this.root, this.returnType);
}
Expand All @@ -238,10 +252,15 @@ public static final class ExpressionEvalMapBuilder {

private EvaluationCallback evaluationCallback;

@Nullable
private EvaluationContext context;

@Nullable
private Object root;

private boolean rootExplicitlySet;

@Nullable
private Class<?> returnType;

private final ExpressionEvalMapComponentsBuilder evalMapComponentsBuilder =
Expand All @@ -267,8 +286,9 @@ public ExpressionEvalMapComponentsBuilder usingEvaluationContext(EvaluationConte
return this.evalMapComponentsBuilder;
}

public ExpressionEvalMapComponentsBuilder withRoot(Object root) {
public ExpressionEvalMapComponentsBuilder withRoot(@Nullable Object root) {
this.root = root;
this.rootExplicitlySet = true;
return this.evalMapComponentsBuilder;

}
Expand All @@ -295,7 +315,8 @@ public ExpressionEvalMap build() {
else {
return new ExpressionEvalMap(ExpressionEvalMapBuilder.this.expressions,
new ComponentsEvaluationCallback(ExpressionEvalMapBuilder.this.context,
ExpressionEvalMapBuilder.this.root, ExpressionEvalMapBuilder.this.returnType));
ExpressionEvalMapBuilder.this.root, ExpressionEvalMapBuilder.this.rootExplicitlySet,
ExpressionEvalMapBuilder.this.returnType));
}
}

Expand All @@ -315,7 +336,7 @@ public ExpressionEvalMapComponentsBuilder usingEvaluationContext(EvaluationConte
}

@Override
public ExpressionEvalMapComponentsBuilder withRoot(Object root) {
public ExpressionEvalMapComponentsBuilder withRoot(@Nullable Object root) {
return ExpressionEvalMapBuilder.this.withRoot(root);
}

Expand All @@ -340,7 +361,7 @@ public interface ExpressionEvalMapComponentsBuilder extends ExpressionEvalMapFin

ExpressionEvalMapComponentsBuilder usingEvaluationContext(EvaluationContext context);

ExpressionEvalMapComponentsBuilder withRoot(Object root);
ExpressionEvalMapComponentsBuilder withRoot(@Nullable Object root);

ExpressionEvalMapComponentsBuilder withReturnType(Class<?> returnType);

Expand Down
52 changes: 34 additions & 18 deletions ...ion-core/src/main/java/org/springframework/integration/expression/FunctionExpression.java
View file
Expand Up @@ -25,6 +25,7 @@
import org.springframework.expression.TypedValue;
import org.springframework.expression.common.ExpressionUtils;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

/**
Expand All @@ -47,6 +48,7 @@
*
* @author Artem Bilan
* @author Gary Russell
*
* @since 5.0
*/
public class FunctionExpression<S> implements Expression {
Expand All @@ -65,46 +67,58 @@ public FunctionExpression(Function<S, ?> function) {
}

@Override
@Nullable
public Object getValue() throws EvaluationException {
return this.function.apply(null);
}

@Override
@Nullable
@SuppressWarnings("unchecked")
public Object getValue(Object rootObject) throws EvaluationException {
public Object getValue(@Nullable Object rootObject) throws EvaluationException {
return this.function.apply((S) rootObject);
}

@Override
public <T> T getValue(Class<T> desiredResultType) throws EvaluationException {
@Nullable
public <T> T getValue(@Nullable Class<T> desiredResultType) throws EvaluationException {
return getValue(this.defaultContext, desiredResultType);
}

@Override
public <T> T getValue(Object rootObject, Class<T> desiredResultType) throws EvaluationException {
@Nullable
public <T> T getValue(@Nullable Object rootObject, @Nullable Class<T> desiredResultType)
throws EvaluationException {

return getValue(this.defaultContext, rootObject, desiredResultType);
}

@Override
@Nullable
public Object getValue(EvaluationContext context) throws EvaluationException {
Object root = context.getRootObject().getValue();
return root == null ? getValue() : getValue(root);
return getValue(context.getRootObject().getValue());
}

@Override
public Object getValue(EvaluationContext context, Object rootObject) throws EvaluationException {
@Nullable
public Object getValue(EvaluationContext context, @Nullable Object rootObject) throws EvaluationException {
return getValue(rootObject);
}

@Override
public <T> T getValue(EvaluationContext context, Class<T> desiredResultType) throws EvaluationException {
@Nullable
public <T> T getValue(EvaluationContext context, @Nullable Class<T> desiredResultType) throws EvaluationException {
return ExpressionUtils.convertTypedValue(context, new TypedValue(getValue(context)), desiredResultType);
}

@Override
public <T> T getValue(EvaluationContext context, Object rootObject, Class<T> desiredResultType)
@Nullable
public <T> T getValue(EvaluationContext context, @Nullable Object rootObject, @Nullable Class<T> desiredResultType)
throws EvaluationException {
return ExpressionUtils.convertTypedValue(context, new TypedValue(getValue(rootObject)), desiredResultType);

return ExpressionUtils.convertTypedValue(context,
new TypedValue(getValue(context, rootObject)),
desiredResultType);
}

@Override
Expand All @@ -113,7 +127,7 @@ public Class<?> getValueType() throws EvaluationException {
}

@Override
public Class<?> getValueType(Object rootObject) throws EvaluationException {
public Class<?> getValueType(@Nullable Object rootObject) throws EvaluationException {
throw this.readOnlyException;
}

Expand All @@ -123,7 +137,7 @@ public Class<?> getValueType(EvaluationContext context) throws EvaluationExcepti
}

@Override
public Class<?> getValueType(EvaluationContext context, Object rootObject) throws EvaluationException {
public Class<?> getValueType(EvaluationContext context, @Nullable Object rootObject) throws EvaluationException {
throw this.readOnlyException;
}

Expand All @@ -133,7 +147,7 @@ public TypeDescriptor getValueTypeDescriptor() throws EvaluationException {
}

@Override
public TypeDescriptor getValueTypeDescriptor(Object rootObject) throws EvaluationException {
public TypeDescriptor getValueTypeDescriptor(@Nullable Object rootObject) throws EvaluationException {
throw this.readOnlyException;
}

Expand All @@ -143,23 +157,25 @@ public TypeDescriptor getValueTypeDescriptor(EvaluationContext context) throws E
}

@Override
public TypeDescriptor getValueTypeDescriptor(EvaluationContext context, Object rootObject)
public TypeDescriptor getValueTypeDescriptor(EvaluationContext context, @Nullable Object rootObject)
throws EvaluationException {
throw this.readOnlyException;
}

@Override
public void setValue(EvaluationContext context, Object value) throws EvaluationException {
public void setValue(EvaluationContext context, @Nullable Object value) throws EvaluationException {
throw this.readOnlyException;
}

@Override
public void setValue(Object rootObject, Object value) throws EvaluationException {
public void setValue(@Nullable Object rootObject, @Nullable Object value) throws EvaluationException {
throw this.readOnlyException;
}

@Override
public void setValue(EvaluationContext context, Object rootObject, Object value) throws EvaluationException {
public void setValue(EvaluationContext context, @Nullable Object rootObject, @Nullable Object value)
throws EvaluationException {

throw this.readOnlyException;
}

Expand All @@ -169,12 +185,12 @@ public boolean isWritable(EvaluationContext context) throws EvaluationException
}

@Override
public boolean isWritable(EvaluationContext context, Object rootObject) throws EvaluationException {
public boolean isWritable(EvaluationContext context, @Nullable Object rootObject) throws EvaluationException {
return false;
}

@Override
public boolean isWritable(Object rootObject) throws EvaluationException {
public boolean isWritable(@Nullable Object rootObject) throws EvaluationException {
return false;
}

Expand Down
1 change: 1 addition & 0 deletions ...java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java
View file
Expand Up @@ -276,6 +276,7 @@ private Message<?> actualDoHandleRequest(HttpServletRequest servletRequest, Requ
headers.putAll(
ExpressionEvalMap.from(getHeaderExpressions())
.usingEvaluationContext(evaluationContext)
.withRoot(httpEntity)
.build());
}

Expand Down
6 changes: 4 additions & 2 deletions ...integration-http/src/test/java/org/springframework/integration/http/dsl/HttpDslTests.java
View file
Expand Up @@ -181,8 +181,9 @@ public void testMultiPartFiles() throws Exception {

Message<?> result = this.multiPartFilesChannel.receive(10_000);

assertThat(result).isNotNull();
assertThat(result.getHeaders()).containsEntry("contentLength", -1L);
assertThat(result)
.isNotNull()
.extracting(Message::getPayload)
.satisfies((payload) ->
assertThat((Map<String, ?>) payload)
Expand Down Expand Up @@ -321,7 +322,8 @@ public IntegrationFlow httpProxyErrorFlow() {
@Bean
public IntegrationFlow multiPartFilesFlow() {
return IntegrationFlows
.from(Http.inboundChannelAdapter("/multiPartFiles"))
.from(Http.inboundChannelAdapter("/multiPartFiles")
.headerFunction("contentLength", (entity) -> entity.getHeaders().getContentLength()))
.channel((c) -> c.queue("multiPartFilesChannel"))
.get();
}
Expand Down
1 change: 1 addition & 0 deletions ...src/main/java/org/springframework/integration/webflux/inbound/WebFluxInboundEndpoint.java
View file
Expand Up @@ -268,6 +268,7 @@ private Mono<Tuple2<Message<Object>, RequestEntity<?>>> buildMessage(RequestEnti
headers.putAll(
ExpressionEvalMap.from(getHeaderExpressions())
.usingEvaluationContext(evaluationContext)
.withRoot(httpEntity)
.build());
}

Expand Down

0 comments on commit 516ecbc

Please sign in to comment.