Skip to content

GH 3936 : Request attribute for webclient in webflux integration has been added #3950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

GH 3936 : Request attribute for webclient in webflux integration has been added #3950

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e42f236
Add webclient request attribute
saxenaj Nov 16, 2022
7483e14
Add author name
Nov 16, 2022
4209c2e
Changes for review comments
Nov 18, 2022
8237a99
Changes for review comments
Nov 18, 2022
8d051e3
Changes for review comments
Nov 18, 2022
c60c3b8
Changes for review comments
Nov 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions ...a/org/springframework/integration/webflux/config/WebFluxOutboundChannelAdapterParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -31,7 +31,7 @@
* Parser for the 'outbound-channel-adapter' element of the webflux namespace.
*
* @author Artem Bilan
*
* @author Jatin Saxena
* @since 5.0
*/
public class WebFluxOutboundChannelAdapterParser extends HttpOutboundChannelAdapterParser {
Expand Down Expand Up @@ -79,6 +79,17 @@ else if (hasTypeExpression) {
.getBeanDefinition());
}

String attributeVariablesExpression = element.getAttribute("attribute-variables-expression");
boolean hasAttibuteVariablesExpression = StringUtils.hasText(attributeVariablesExpression);

if (hasAttibuteVariablesExpression) {
builder.addPropertyValue("attributeVariablesExpression",
BeanDefinitionBuilder.rootBeanDefinition(ExpressionFactoryBean.class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See IntegrationNamespaceUtils.createExpressionDefIfAttributeDefined() instead for convenience.

Copy link
Contributor Author

@saxenaj saxenaj Nov 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code has been updated to use IntegrationNamespaceUtils.createExpressionDefIfAttributeDefined()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saxenaj ,

you know, there is no reason to add this kind of comments if you agree with us: we just simply will see that in the next commit you add to the PR.
Otherwise I got at a bout 10 emails from you instead of just one about a new commit. 😄

.addConstructorArgValue(attributeVariablesExpression)
.getBeanDefinition());
}


return builder;
}

Expand Down
29 changes: 29 additions & 0 deletions ...g/springframework/integration/webflux/outbound/WebFluxRequestExecutingMessageHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

import org.springframework.beans.factory.BeanFactory;
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.core.io.Resource;
import org.springframework.expression.Expression;
import org.springframework.expression.common.LiteralExpression;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatusCode;
Expand All @@ -35,6 +37,7 @@
import org.springframework.http.ResponseEntity;
import org.springframework.http.client.reactive.ClientHttpRequest;
import org.springframework.http.client.reactive.ClientHttpResponse;
import org.springframework.integration.expression.ExpressionUtils;
import org.springframework.integration.expression.ValueExpression;
import org.springframework.integration.http.outbound.AbstractHttpRequestExecutingMessageHandler;
import org.springframework.lang.Nullable;
Expand All @@ -57,6 +60,7 @@
* @author Artem Bilan
* @author Gary Russell
* @author David Graff
* @author Jatin Saxena
*
* @since 5.0
*
Expand All @@ -75,6 +79,11 @@ public class WebFluxRequestExecutingMessageHandler extends AbstractHttpRequestEx

private Expression publisherElementTypeExpression;

private Expression attributeVariablesExpression;

private StandardEvaluationContext evaluationContext;


/**
* Create a handler that will send requests to the provided URI.
* @param uri The URI.
Expand Down Expand Up @@ -193,11 +202,22 @@ public void setPublisherElementTypeExpression(Expression publisherElementTypeExp
this.publisherElementTypeExpression = publisherElementTypeExpression;
}

public void setAttributeVariablesExpression(Expression attributeVariablesExpression) {
Assert.notNull(attributeVariablesExpression, "'attributeVariablesExpression' must not be null");
this.attributeVariablesExpression = attributeVariablesExpression;
}

@Override
public String getComponentType() {
return (isExpectReply() ? "webflux:outbound-gateway" : "webflux:outbound-channel-adapter");
}

@Override
protected final void doInit() {
BeanFactory beanFactory = getBeanFactory();
this.evaluationContext = ExpressionUtils.createStandardEvaluationContext(beanFactory);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just have a single line here: doesn't look like there is a reason in extracting beanFactory variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beanFactory variable has been removed and code has been updated to single line

}

@Override
@Nullable
protected Object exchange(Object uri, HttpMethod httpMethod, HttpEntity<?> httpRequest,
Expand Down Expand Up @@ -230,6 +250,12 @@ private WebClient.RequestBodySpec createRequestBodySpec(Object uri, HttpMethod h
}

requestSpec = requestSpec.headers(headers -> headers.putAll(httpRequest.getHeaders()));

if(attributeVariablesExpression != null) {
Map<String, Object> attributeMap = evaluateAttributeVariables(requestMessage);
requestSpec = requestSpec.attributes(map -> map.putAll(attributeMap));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do some optimization not calling this method if attributeMap.isEmpty()!

}

BodyInserter<?, ? super ClientHttpRequest> inserter = buildBodyInserterForRequest(requestMessage, httpRequest);
if (inserter != null) {
requestSpec.body(inserter);
Expand Down Expand Up @@ -369,4 +395,7 @@ private Object createReplyFromResponse(Mono<ResponseEntity<Flux<Object>>> respon
.map(this::getReply);
}

private Map<String,Object> evaluateAttributeVariables(Message<?> requestMessage){
return this.attributeVariablesExpression.getValue(this.evaluationContext, requestMessage, Map.class);
}
}
14 changes: 14 additions & 0 deletions ...n/resources/org/springframework/integration/webflux/config/spring-integration-webflux.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,13 @@
</xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="attribute-variables-expression" type="xsd:string">
<xsd:annotation>
<xsd:documentation><![CDATA[
Specifies the SpEL expression to be evaluated as a Map for request attributes.
]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:complexType>
</xsd:element>

Expand Down Expand Up @@ -592,6 +599,13 @@
<xsd:union memberTypes="xsd:boolean xsd:string"/>
</xsd:simpleType>
</xsd:attribute>
<xsd:attribute name="attribute-variables-expression" type="xsd:string">
<xsd:annotation>
<xsd:documentation><![CDATA[
Specifies the SpEL expression to be evaluated as a Map for request attributes.
]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:extension>
</xsd:complexContent>
</xsd:complexType>
Expand Down
3 changes: 2 additions & 1 deletion .../springframework/integration/webflux/config/WebFluxOutboundGatewayParserTests-context.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
reply-payload-to-flux="true"
body-extractor="bodyExtractor"
publisher-element-type-expression="headers.elementType"
extract-response-body="false">
extract-response-body="false"
attribute-variables-expression="{name:{first:'Nikola',last:'Tesla'},dob:{day:10,month:'July',year:1856}}">
<uri-variable name="foo" expression="headers.bar"/>
</outbound-gateway>

Expand Down
3 changes: 3 additions & 0 deletions ...ava/org/springframework/integration/webflux/config/WebFluxOutboundGatewayParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

/**
* @author Artem Bilan
* @author Jatin Saxena
*
* @since 5.0
*/
Expand Down Expand Up @@ -129,6 +130,8 @@ public void reactiveFullConfig() {
.isEqualTo("headers.elementType");
assertThat(handlerAccessor.getPropertyValue("extractResponseBody"))
.isEqualTo(false);
assertThat(handlerAccessor.getPropertyValue("attributeVariablesExpression.expression"))
.isEqualTo("{name:{first:'Nikola',last:'Tesla'},dob:{day:10,month:'July',year:1856}}");
}

}
22 changes: 17 additions & 5 deletions ...ingframework/integration/webflux/outbound/WebFluxRequestExecutingMessageHandlerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,19 @@

package org.springframework.integration.webflux.outbound;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

import java.nio.charset.StandardCharsets;
import java.time.Duration;

import org.junit.jupiter.api.Test;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

import org.springframework.beans.factory.BeanFactory;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferFactory;
import org.springframework.core.io.buffer.DataBufferLimitException;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.client.reactive.ClientHttpConnector;
Expand All @@ -44,13 +46,15 @@
import org.springframework.web.reactive.function.client.WebClient;
import org.springframework.web.reactive.function.client.WebClientResponseException;

import static org.assertj.core.api.Assertions.assertThat;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong imports re-order according our new code style.
Please, rebase to the latest main and see src/idea or src/eclipse for editor configs to import into your IDE.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import sequence has been updated to follow code style


/**
* @author Shiliang Li
* @author Artem Bilan
* @author David Graff
*
* @author Jatin Saxena
* @since 5.0
*/
class WebFluxRequestExecutingMessageHandlerTests {
Expand Down Expand Up @@ -214,6 +218,10 @@ void testFluxReply() {
reactiveHandler.setExpectedResponseType(String.class);
reactiveHandler.setReplyPayloadToFlux(true);

reactiveHandler.setAttributeVariablesExpression(new SpelExpressionParser().parseExpression("{name:{first:'Nikola',last:'Tesla'},dob:{day:10,month:'July',year:1856}}"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer no more than 120 symbols in one code line.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, doesn't look like you do some verification for those attributes to propagate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New test case has been added to check the propagation of attributes. Also code has been changes to keep less than 120 char in one line.

setBeanFactory(reactiveHandler);
reactiveHandler.afterPropertiesSet();

reactiveHandler.handleMessage(MessageBuilder.withPayload(Mono.just("hello, world")).build());

Message<?> receive = replyChannel.receive(10_000);
Expand Down Expand Up @@ -376,4 +384,8 @@ void testMaxInMemorySizeExceeded() {
.isEqualTo("Exceeded limit on max bytes to buffer : 1");
}

private void setBeanFactory(WebFluxRequestExecutingMessageHandler handler) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we need this method, if you simply can do that directly in a line of code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code has been updated accordingly.

handler.setBeanFactory(mock(BeanFactory.class));
}

}