From e4e80419c5d50802ab2ace8893b7f4ed23998f30 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Mon, 7 Nov 2022 15:47:19 -0500 Subject: [PATCH] GH-3938: Fix HTTP XML configuration for ambiguity (#3939) * GH-3938: Fix HTTP XML configuration for ambiguity Fixes https://github.com/spring-projects/spring-integration/issues/3938 The `encoding-mode` is a property of the `RestTemplate`. Therefore, it cannot be set on the component configuration together with an externally injected `rest-template` Even if `HttpRequestExecutingMessageHandler` has an assertion for such an ambiguity, the XML parser just ignores this `encoding-mode` when it encounters the `rest-template` * Fix `HttpAdapterParsingUtils.verifyNoRestTemplateAttributes()` to check for not allowed attributes with a `encoding-mode` as well * Remove a `default` from the `encoding-mode` to not cause an ambiguity in the parser * Fix some typos in the `spring-integration-http.xsd` * Rework `OutboundResponseTypeTests` to JUnit 5 **Cherry-pick to `5.5.x`** * * Fix error handling for `encoding-mode` in the `HttpAdapterParsingUtils` * Cover `encoding-mode` and `rest-template` ambiguity with a test against failing XML configuration * * Improve error message for ambiguous attributes in the `HttpAdapterParsingUtils` --- .../http/config/HttpAdapterParsingUtils.java | 24 +++-- .../http/config/spring-integration-http.xsd | 23 ++--- ...seTypeTests-context-encoding-mode-fail.xml | 16 ++++ .../OutboundResponseTypeTests-context.xml | 5 +- .../config/OutboundResponseTypeTests.java | 92 +++++++++---------- 5 files changed, 89 insertions(+), 71 deletions(-) create mode 100644 spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests-context-encoding-mode-fail.xml diff --git a/spring-integration-http/src/main/java/org/springframework/integration/http/config/HttpAdapterParsingUtils.java b/spring-integration-http/src/main/java/org/springframework/integration/http/config/HttpAdapterParsingUtils.java index 94903f3a11..ed1bc6f518 100644 --- a/spring-integration-http/src/main/java/org/springframework/integration/http/config/HttpAdapterParsingUtils.java +++ b/spring-integration-http/src/main/java/org/springframework/integration/http/config/HttpAdapterParsingUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 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. @@ -36,9 +36,10 @@ import org.springframework.util.xml.DomUtils; * @author Gary Russell * @author Artem Bilan * @author Shiliang Li + * * @since 2.0.2 */ -abstract class HttpAdapterParsingUtils { +final class HttpAdapterParsingUtils { static final String[] SYNC_REST_TEMPLATE_REFERENCE_ATTRIBUTES = { "request-factory", "error-handler", "message-converters" @@ -48,14 +49,22 @@ abstract class HttpAdapterParsingUtils { for (String attributeName : SYNC_REST_TEMPLATE_REFERENCE_ATTRIBUTES) { if (element.hasAttribute(attributeName)) { parserContext.getReaderContext().error("When providing a 'rest-template' reference, the '" - + attributeName + "' attribute is not allowed.", + + attributeName + "' attribute is not allowed, " + + "it must be set on the provided template instead", parserContext.extractSource(element)); } } + + if (element.hasAttribute("encoding-mode")) { + parserContext.getReaderContext().error("When providing a 'rest-template' reference, " + + "the 'encoding-mode' must be set on the 'RestTemplate.uriTemplateHandler' property.", + parserContext.extractSource(element)); + } } static void configureUriVariableExpressions(BeanDefinitionBuilder builder, ParserContext parserContext, Element element) { + String uriVariablesExpression = element.getAttribute("uri-variables-expression"); List uriVariableElements = DomUtils.getChildElementsByTagName(element, "uri-variable"); @@ -73,7 +82,7 @@ abstract class HttpAdapterParsingUtils { } if (hasUriVariableExpressions) { - ManagedMap uriVariableExpressions = new ManagedMap(); + ManagedMap uriVariableExpressions = new ManagedMap<>(); for (Element uriVariableElement : uriVariableElements) { String name = uriVariableElement.getAttribute("name"); String expression = uriVariableElement.getAttribute("expression"); @@ -146,8 +155,8 @@ abstract class HttpAdapterParsingUtils { if (hasExpectedResponseType && hasExpectedResponseTypeExpression) { parserContext.getReaderContext() - .error("The 'expected-response-type' and 'expected-response-type-expression' are mutually exclusive. " + - "You can only have one or the other", element); + .error("The 'expected-response-type' and 'expected-response-type-expression' " + + "are mutually exclusive. You can only have one or the other", element); } RootBeanDefinition expressionDef = null; @@ -164,4 +173,7 @@ abstract class HttpAdapterParsingUtils { } } + private HttpAdapterParsingUtils() { + } + } diff --git a/spring-integration-http/src/main/resources/org/springframework/integration/http/config/spring-integration-http.xsd b/spring-integration-http/src/main/resources/org/springframework/integration/http/config/spring-integration-http.xsd index b1ccaf59eb..a4690b647e 100644 --- a/spring-integration-http/src/main/resources/org/springframework/integration/http/config/spring-integration-http.xsd +++ b/spring-integration-http/src/main/resources/org/springframework/integration/http/config/spring-integration-http.xsd @@ -73,8 +73,7 @@ By default 'status-code-expression' is null, meaning that the default '200 OK' response status will be returned. The 'http:inbound-gateway' resolves the 'status code' from the 'http_statusCode' header of the - reply - Message. + reply Message. @@ -412,7 +411,7 @@ Specify whether the outbound message's payload should be extracted - when preparing the request body. Otherwise the Message instance itself + when preparing the request body. Otherwise, the Message instance itself will be serialized. The default value is 'true'. @@ -484,7 +483,7 @@ Specifies whether the outbound message's payload should be extracted - when preparing the request body. Otherwise the Message instance itself + when preparing the request body. Otherwise, the Message instance itself will be serialized. The default value is 'true'. @@ -493,8 +492,8 @@ @@ -602,7 +601,8 @@ @@ -759,7 +759,7 @@ - Set to "true" if the the browser should include any cookies associated to the domain + Set to "true" if the browser should include any cookies associated to the domain of the request being annotated, or "false" if it should not. Empty string "" means undefined. If true, the pre-flight response will include the header 'Access-Control-Allow-Credentials=true'. Default value is "true". @@ -826,7 +826,7 @@ Provide a reference to a list of HttpMessageConverter instances. If specified, - these converters will replace all of the default converters that would normally + these converters will replace all default converters that would normally be present on the underlying RestTemplate. @@ -852,10 +852,11 @@ ]]> - + Set the encoding mode during URI building. + Defaults to 'TEMPLATE_AND_VALUES' @@ -948,7 +949,7 @@ diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests-context-encoding-mode-fail.xml b/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests-context-encoding-mode-fail.xml new file mode 100644 index 0000000000..fe1494d97a --- /dev/null +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests-context-encoding-mode-fail.xml @@ -0,0 +1,16 @@ + + + + + + + + + + diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests-context.xml b/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests-context.xml index e3df25b474..fbfa038403 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests-context.xml +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests-context.xml @@ -3,15 +3,12 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:int="http://www.springframework.org/schema/integration" xmlns:int-http="http://www.springframework.org/schema/integration/http" - xmlns:util="http://www.springframework.org/schema/util" xsi:schemaLocation="http://www.springframework.org/schema/integration/http https://www.springframework.org/schema/integration/http/spring-integration-http.xsd http://www.springframework.org/schema/integration https://www.springframework.org/schema/integration/spring-integration.xsd http://www.springframework.org/schema/beans - https://www.springframework.org/schema/beans/spring-beans.xsd - http://www.springframework.org/schema/util - https://www.springframework.org/schema/util/spring-util.xsd"> + https://www.springframework.org/schema/beans/spring-beans.xsd"> diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests.java index ea3c967656..a500d5af44 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/config/OutboundResponseTypeTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 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. @@ -17,20 +17,17 @@ package org.springframework.integration.http.config; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.fail; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.springframework.test.web.client.match.MockRestRequestMatchers.header; import static org.springframework.test.web.client.match.MockRestRequestMatchers.method; import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; import java.util.Collections; -import java.util.Map; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; -import org.springframework.beans.BeansException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.context.support.ClassPathXmlApplicationContext; @@ -44,8 +41,7 @@ import org.springframework.messaging.MessageChannel; import org.springframework.messaging.MessageHandlingException; import org.springframework.messaging.support.GenericMessage; import org.springframework.test.annotation.DirtiesContext; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; import org.springframework.test.web.client.MockRestServiceServer; import org.springframework.web.client.RestTemplate; @@ -53,13 +49,10 @@ import org.springframework.web.client.RestTemplate; * @author Oleg Zhurakousky * @author Artem Bilan * @author Gary Russell - * @since 2.2 * - *

- * see https://jira.springsource.org/browse/INT-2397 + * @since 2.2 */ -@ContextConfiguration -@RunWith(SpringJUnit4ClassRunner.class) +@SpringJUnitConfig @DirtiesContext public class OutboundResponseTypeTests { @@ -95,18 +88,18 @@ public class OutboundResponseTypeTests { private MockRestServiceServer mockServer; - @Before + @BeforeEach public void setup() { this.mockServer = MockRestServiceServer.createServer(this.restTemplate); } @Test - public void testDefaultResponseType() throws Exception { + public void testDefaultResponseType() { this.mockServer.expect(requestTo("/testApps/outboundResponse")) .andExpect(method(HttpMethod.POST)) .andRespond(withSuccess(HttpMethod.POST.name(), MediaType.TEXT_PLAIN)); - this.requestChannel.send(new GenericMessage("Hello")); + this.requestChannel.send(new GenericMessage<>("Hello")); Message message = this.replyChannel.receive(5000); assertThat(message).isNotNull(); assertThat(message.getPayload() instanceof ResponseEntity).isTrue(); @@ -115,12 +108,12 @@ public class OutboundResponseTypeTests { } @Test - public void testWithResponseTypeSet() throws Exception { + public void testWithResponseTypeSet() { this.mockServer.expect(requestTo("/testApps/outboundResponse")) .andExpect(method(HttpMethod.POST)) .andRespond(withSuccess(HttpMethod.POST.name(), MediaType.TEXT_PLAIN)); - this.resTypeSetChannel.send(new GenericMessage("Hello")); + this.resTypeSetChannel.send(new GenericMessage<>("Hello")); Message message = this.replyChannel.receive(5000); assertThat(message).isNotNull(); assertThat(message.getPayload() instanceof String).isTrue(); @@ -129,12 +122,12 @@ public class OutboundResponseTypeTests { } @Test - public void testWithResponseTypeExpressionSet() throws Exception { + public void testWithResponseTypeExpressionSet() { this.mockServer.expect(requestTo("/testApps/outboundResponse")) .andExpect(method(HttpMethod.POST)) .andRespond(withSuccess(HttpMethod.POST.name(), MediaType.TEXT_PLAIN)); - this.resTypeExpressionSetChannel.send(new GenericMessage("java.lang.String")); + this.resTypeExpressionSetChannel.send(new GenericMessage<>("java.lang.String")); Message message = this.replyChannel.receive(5000); assertThat(message).isNotNull(); assertThat(message.getPayload() instanceof String).isTrue(); @@ -143,7 +136,7 @@ public class OutboundResponseTypeTests { } @Test - public void testWithResponseTypeExpressionSetAsClass() throws Exception { + public void testWithResponseTypeExpressionSetAsClass() { this.mockServer = MockRestServiceServer.createServer(this.restTemplateWithConverters); this.mockServer.expect(requestTo("/testApps/outboundResponse")) .andExpect(method(HttpMethod.POST)) @@ -158,12 +151,12 @@ public class OutboundResponseTypeTests { } @Test - public void testInt2706ResponseTypeExpressionAsPrimitive() throws Exception { + public void testInt2706ResponseTypeExpressionAsPrimitive() { this.mockServer.expect(requestTo("/testApps/outboundResponse")) .andExpect(method(HttpMethod.POST)) .andRespond(withSuccess(HttpMethod.POST.name(), MediaType.TEXT_PLAIN)); - this.resTypeExpressionSetChannel.send(new GenericMessage("byte[]")); + this.resTypeExpressionSetChannel.send(new GenericMessage<>("byte[]")); Message message = this.replyChannel.receive(5000); assertThat(message).isNotNull(); assertThat(message.getPayload() instanceof byte[]).isTrue(); @@ -172,12 +165,12 @@ public class OutboundResponseTypeTests { } @Test - public void testInt2706ResponseTypePrimitiveArrayClassAsString() throws Exception { + public void testInt2706ResponseTypePrimitiveArrayClassAsString() { this.mockServer.expect(requestTo("/testApps/outboundResponse")) .andExpect(method(HttpMethod.POST)) .andRespond(withSuccess(HttpMethod.POST.name(), MediaType.TEXT_PLAIN)); - this.resPrimitiveStringPresentationChannel.send(new GenericMessage("hello".getBytes())); + this.resPrimitiveStringPresentationChannel.send(new GenericMessage<>("hello".getBytes())); Message message = this.replyChannel.receive(5000); assertThat(message).isNotNull(); assertThat(message.getPayload() instanceof byte[]).isTrue(); @@ -186,44 +179,43 @@ public class OutboundResponseTypeTests { } @Test - public void testInt3052InvalidResponseType() throws Exception { - try { - this.invalidResponseTypeChannel.send(new GenericMessage("hello".getBytes())); - fail("IllegalStateException expected."); - } - catch (Exception e) { - assertThat(e).isInstanceOf(MessageHandlingException.class); - Throwable t = e.getCause(); - assertThat(t).isInstanceOf(IllegalStateException.class); - assertThat(t.getMessage()).contains("'expectedResponseType' can be an instance of " + - "'Class', 'String' or 'ParameterizedTypeReference'"); - } + public void testInt3052InvalidResponseType() { + assertThatExceptionOfType(MessageHandlingException.class) + .isThrownBy(() -> this.invalidResponseTypeChannel.send(new GenericMessage<>("hello".getBytes()))) + .withCauseInstanceOf(IllegalStateException.class) + .withStackTraceContaining("'expectedResponseType' can be an instance of " + + "'Class', 'String' or 'ParameterizedTypeReference'"); } @Test - public void testMutuallyExclusivityInMethodAndMethodExpression() throws Exception { - try { - new ClassPathXmlApplicationContext("OutboundResponseTypeTests-context-fail.xml", this.getClass()).close(); - fail("Expected BeansException"); - } - catch (BeansException e) { - assertThat(e instanceof BeanDefinitionParsingException).isTrue(); - assertThat(e.getMessage().contains("The 'expected-response-type' " + - "and 'expected-response-type-expression' are mutually exclusive")).isTrue(); - } + public void testMutuallyExclusivityInMethodAndMethodExpression() { + assertThatExceptionOfType(BeanDefinitionParsingException.class) + .isThrownBy(() -> new ClassPathXmlApplicationContext("OutboundResponseTypeTests-context-fail.xml", + getClass())) + .withMessageContaining("The 'expected-response-type' " + + "and 'expected-response-type-expression' are mutually exclusive"); } @Test - public void testContentTypePropagation() throws Exception { + public void testContentTypePropagation() { this.mockServer.expect(requestTo("/testApps/outboundResponse")) .andExpect(method(HttpMethod.POST)) .andExpect(header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON.toString())) .andRespond(withSuccess(HttpMethod.POST.name(), MediaType.TEXT_PLAIN)); this.contentTypePropagationChannel - .send(new GenericMessage>(Collections.singletonMap("foo", "bar"))); + .send(new GenericMessage<>(Collections.singletonMap("foo", "bar"))); this.mockServer.verify(); } + @Test + public void notAllowedEncodingModeWhenExternalRestTemplate() { + assertThatExceptionOfType(BeanDefinitionParsingException.class) + .isThrownBy(() -> new ClassPathXmlApplicationContext( + "OutboundResponseTypeTests-context-encoding-mode-fail.xml", getClass())) + .withMessageContaining("When providing a 'rest-template' reference, " + + "the 'encoding-mode' must be set on the 'RestTemplate.uriTemplateHandler' property."); + } + }