From df697c53e2e335bbd80cfee12891917c906e75e9 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 13 Mar 2019 18:54:10 -0400 Subject: [PATCH] GH-2806: Add generics support to HTTP Inbound Fixes https://github.com/spring-projects/spring-integration/issues/2806 **Cherry-pick to 5.1.x** --- .../http/inbound/BaseHttpInboundEndpoint.java | 32 ++--- .../HttpRequestHandlingEndpointSupport.java | 109 +++++++++--------- .../HttpRequestHandlingMessagingGateway.java | 9 +- ...pRequestHandlingMessagingGatewayTests.java | 60 +++++++++- 4 files changed, 137 insertions(+), 73 deletions(-) diff --git a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/BaseHttpInboundEndpoint.java b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/BaseHttpInboundEndpoint.java index 5abaca61ea..43655a7068 100644 --- a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/BaseHttpInboundEndpoint.java +++ b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/BaseHttpInboundEndpoint.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2018 the original author or authors. + * Copyright 2017-2019 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. @@ -66,23 +66,23 @@ public class BaseHttpInboundEndpoint extends MessagingGatewaySupport implements protected final AtomicInteger activeCount = new AtomicInteger(); - private volatile ResolvableType requestPayloadType = null; + private ResolvableType requestPayloadType = null; - private volatile HeaderMapper headerMapper = DefaultHttpHeaderMapper.inboundMapper(); + private HeaderMapper headerMapper = DefaultHttpHeaderMapper.inboundMapper(); - private volatile boolean extractReplyPayload = true; + private boolean extractReplyPayload = true; - private volatile Expression statusCodeExpression; + private Expression statusCodeExpression; - private volatile EvaluationContext evaluationContext; + private EvaluationContext evaluationContext; - private volatile RequestMapping requestMapping = new RequestMapping(); + private RequestMapping requestMapping = new RequestMapping(); - private volatile Expression payloadExpression; + private Expression payloadExpression; - private volatile Map headerExpressions; + private Map headerExpressions; - private volatile CrossOrigin crossOrigin; + private CrossOrigin crossOrigin; public BaseHttpInboundEndpoint(boolean expectReply) { super(expectReply); @@ -279,8 +279,13 @@ public class BaseHttpInboundEndpoint extends MessagingGatewaySupport implements } protected HttpStatus evaluateHttpStatus(HttpEntity httpEntity) { - Object value = this.statusCodeExpression.getValue(this.evaluationContext, httpEntity); - return buildHttpStatus(value); + if (this.statusCodeExpression != null) { + Object value = this.statusCodeExpression.getValue(this.evaluationContext, httpEntity); + return buildHttpStatus(value); + } + else { + return HttpStatus.INTERNAL_SERVER_ERROR; + } } protected HttpStatus resolveHttpStatusFromHeaders(MessageHeaders headers) { @@ -328,8 +333,7 @@ public class BaseHttpInboundEndpoint extends MessagingGatewaySupport implements * @return true or false if HTTP request can contain the body */ protected boolean isReadable(HttpRequest request) { - HttpMethod method = request.getMethod(); - return method == null ? false : !(CollectionUtils.containsInstance(nonReadableBodyHttpMethods, method)); + return !(CollectionUtils.containsInstance(nonReadableBodyHttpMethods, request.getMethod())); } } diff --git a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java index 415fa7e5be..15d109481e 100644 --- a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java +++ b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingEndpointSupport.java @@ -17,6 +17,7 @@ package org.springframework.integration.http.inbound; import java.io.IOException; +import java.lang.reflect.Type; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -39,6 +40,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.RequestEntity; import org.springframework.http.converter.ByteArrayHttpMessageConverter; +import org.springframework.http.converter.GenericHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.ResourceHttpMessageConverter; import org.springframework.http.converter.StringHttpMessageConverter; @@ -93,7 +95,7 @@ import org.springframework.web.servlet.HandlerMapping; * In a request-reply scenario, the reply Message's payload will be extracted prior * to generating a response by default. * To have the entire serialized Message available for the response, switch the - * {@link #extractReplyPayload} value to {@code false}. + * {@code extractReplyPayload} value to {@code false}. * * @author Mark Fisher * @author Oleg Zhurakousky @@ -146,27 +148,21 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound stringHttpMessageConverter.setWriteAcceptCharset(false); this.defaultMessageConverters.add(stringHttpMessageConverter); this.defaultMessageConverters.add(new ResourceHttpMessageConverter()); - SourceHttpMessageConverter sourceConverter = new SourceHttpMessageConverter(); + SourceHttpMessageConverter sourceConverter = new SourceHttpMessageConverter<>(); this.defaultMessageConverters.add(sourceConverter); if (jaxb2Present) { this.defaultMessageConverters.add(new Jaxb2RootElementHttpMessageConverter()); - if (logger.isDebugEnabled()) { - logger.debug("'Jaxb2RootElementHttpMessageConverter' was added to the 'defaultMessageConverters'."); - } + logger.debug("'Jaxb2RootElementHttpMessageConverter' was added to the 'defaultMessageConverters'."); } if (JacksonPresent.isJackson2Present()) { this.defaultMessageConverters.add(new MappingJackson2HttpMessageConverter()); - if (logger.isDebugEnabled()) { - logger.debug("'MappingJackson2HttpMessageConverter' was added to the 'defaultMessageConverters'."); - } + logger.debug("'MappingJackson2HttpMessageConverter' was added to the 'defaultMessageConverters'."); } if (romeToolsPresent) { this.defaultMessageConverters.add(new AtomFeedHttpMessageConverter()); this.defaultMessageConverters.add(new RssChannelHttpMessageConverter()); - if (logger.isDebugEnabled()) { - logger.debug("'AtomFeedHttpMessageConverter' was added to the 'defaultMessageConverters'."); - logger.debug("'RssChannelHttpMessageConverter' was added to the 'defaultMessageConverters'."); - } + logger.debug("'AtomFeedHttpMessageConverter' was added to the 'defaultMessageConverters'."); + logger.debug("'RssChannelHttpMessageConverter' was added to the 'defaultMessageConverters'."); } } @@ -176,8 +172,9 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound * @param messageConverters The message converters. */ public void setMessageConverters(List> messageConverters) { + Assert.notNull(messageConverters, "'messageConverters' must not be null"); Assert.noNullElements(messageConverters.toArray(), "'messageConverters' must not contain null entries"); - List> localConverters = new ArrayList>(messageConverters); + List> localConverters = new ArrayList<>(messageConverters); if (this.mergeWithDefaultConverters) { localConverters.addAll(this.defaultMessageConverters); this.convertersMerged = true; @@ -266,14 +263,13 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound this.activeCount.incrementAndGet(); try { - StandardEvaluationContext evaluationContext = this.createEvaluationContext(); + StandardEvaluationContext evaluationContext = createEvaluationContext(); evaluationContext.setRootObject(httpEntity); evaluationContext.setVariable("requestAttributes", RequestContextHolder.currentRequestAttributes()); - MultiValueMap requestParams = this.convertParameterMap(servletRequest.getParameterMap()); + MultiValueMap requestParams = convertParameterMap(servletRequest.getParameterMap()); evaluationContext.setVariable("requestParams", requestParams); - evaluationContext.setVariable("requestHeaders", new ServletServerHttpRequest(servletRequest).getHeaders()); Cookie[] requestCookies = servletRequest.getCookies(); @@ -335,12 +331,16 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound AbstractIntegrationMessageBuilder messageBuilder = null; if (payload instanceof Message) { - messageBuilder = this.getMessageBuilderFactory().fromMessage((Message) payload) - .copyHeadersIfAbsent(headers); + messageBuilder = + getMessageBuilderFactory() + .fromMessage((Message) payload) + .copyHeadersIfAbsent(headers); } else { Assert.state(payload != null, "payload cannot be null"); - messageBuilder = this.getMessageBuilderFactory().withPayload(payload).copyHeaders(headers); + messageBuilder = getMessageBuilderFactory() + .withPayload(payload) + .copyHeaders(headers); } HttpMethod method = httpEntity.getMethod(); @@ -359,30 +359,24 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound Message reply = null; if (this.expectReply) { try { - reply = this.sendAndReceiveMessage(message); + reply = sendAndReceiveMessage(message); } catch (MessageTimeoutException e) { - if (getStatusCodeExpression() != null) { - reply = getMessageBuilderFactory().withPayload(e.getMessage()) - .setHeader(org.springframework.integration.http.HttpHeaders.STATUS_CODE, - evaluateHttpStatus(httpEntity)) - .build(); - } - else { - reply = getMessageBuilderFactory().withPayload(e.getMessage()) - .setHeader(org.springframework.integration.http.HttpHeaders.STATUS_CODE, - HttpStatus.INTERNAL_SERVER_ERROR) - .build(); - } + reply = + getMessageBuilderFactory() + .withPayload(e.getMessage()) + .setHeader(org.springframework.integration.http.HttpHeaders.STATUS_CODE, + evaluateHttpStatus(httpEntity)) + .build(); } } else { - this.send(message); + send(message); } return reply; } finally { - this.postProcessRequest(servletRequest); + postProcessRequest(servletRequest); this.activeCount.decrementAndGet(); } } @@ -391,7 +385,8 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound if (logger.isDebugEnabled()) { logger.debug("Endpoint is stopped; returning status " + HttpStatus.SERVICE_UNAVAILABLE); } - return this.getMessageBuilderFactory().withPayload("Endpoint is stopped") + return getMessageBuilderFactory() + .withPayload("Endpoint is stopped") .setHeader(org.springframework.integration.http.HttpHeaders.STATUS_CODE, HttpStatus.SERVICE_UNAVAILABLE) .build(); } @@ -401,7 +396,7 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound * sets up the {@link ServletServerHttpResponse}. * @param response The ServletServerHttpResponse. * @param replyMessage The reply message. - * @return The message payload (if {@link #extractReplyPayload}) otherwise the message. + * @return The message payload (if {@code extractReplyPayload}) otherwise the message. */ protected final Object setupResponseAndConvertReply(ServletServerHttpResponse response, Message replyMessage) { getHeaderMapper().fromHeaders(replyMessage.getHeaders(), response.getHeaders()); @@ -461,7 +456,7 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound * Converts a servlet request's parameterMap to a {@link MultiValueMap}. */ private MultiValueMap convertParameterMap(Map parameterMap) { - MultiValueMap convertedMap = new LinkedMultiValueMap(parameterMap.size()); + MultiValueMap convertedMap = new LinkedMultiValueMap<>(parameterMap.size()); for (Map.Entry entry : parameterMap.entrySet()) { String[] values = entry.getValue(); for (String value : values) { @@ -487,29 +482,37 @@ public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInbound contentType = MediaType.APPLICATION_OCTET_STREAM; } ResolvableType requestPayloadType = getRequestPayloadType(); - Class expectedType; if (requestPayloadType == null) { - expectedType = "text".equals(contentType.getType()) ? String.class : byte[].class; - } - else { - expectedType = requestPayloadType.resolve(); + requestPayloadType = + ResolvableType.forClass( + "text".equals(contentType.getType()) + ? String.class + : byte[].class); } - /* - * TODO: resolve() can return null, which is not valid for canRead(). - * Perhaps we should coerce to String/byte[] instead of attempting - * to convert. However this might be a breaking change - 5.2? - * Hence NOSONAR below. - */ + Type targetType = requestPayloadType.getType(); + Class targetClass = requestPayloadType.toClass(); + for (HttpMessageConverter converter : this.messageConverters) { - if (converter.canRead(expectedType, contentType)) { - return converter.read((Class) expectedType, request); + GenericHttpMessageConverter genericConverter = + converter instanceof GenericHttpMessageConverter + ? (GenericHttpMessageConverter) converter + : null; + if (genericConverter != null + ? genericConverter.canRead(targetType, null, contentType) : + (converter.canRead(targetClass, contentType))) { + + if (genericConverter != null) { + return genericConverter.read(targetType, null, request); + } + else { + return converter.read((Class) targetClass, request); + } } } - throw new MessagingException(// NOSONAR might be null; see comment above. + throw new MessagingException( "Could not convert request: no suitable HttpMessageConverter found for expected type [" - + expectedType != null ? expectedType.getName() : "null" - + "] and content type [" + contentType + "]"); + + requestPayloadType + "] and content type [" + contentType + "]"); } } diff --git a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGateway.java b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGateway.java index 0c95e014dd..38a3c46fa4 100644 --- a/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGateway.java +++ b/spring-integration-http/src/main/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGateway.java @@ -44,10 +44,11 @@ import org.springframework.web.HttpRequestHandler; * (e.g. 200 OK). *

* The default supported request methods are GET and POST, but the list of values can be configured with the - * {@link RequestMapping#methods} property. The payload generated from a GET request (or HEAD or OPTIONS if supported) will - * be a {@link org.springframework.util.MultiValueMap} containing the parameter values. For a request containing a body - * (e.g. a POST), the type - * of the payload is determined by the {@link #setRequestPayloadTypeClass(Class)} request payload type}. + * {@link RequestMapping#getMethods()} property. + * The payload generated from a GET request (or HEAD or OPTIONS if supported) will + * be a {@link org.springframework.util.MultiValueMap} containing the parameter values. + * For a request containing a body (e.g. a POST), the type of the payload is determined + * by the {@link #setRequestPayloadTypeClass(Class)} request payload type}. *

* If the HTTP request is a multipart and a "multipartResolver" bean has been defined in the context, then it will be * converted by the diff --git a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayTests.java b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayTests.java index 5c28ea22b7..e3c5e13f63 100644 --- a/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayTests.java +++ b/spring-integration-http/src/test/java/org/springframework/integration/http/inbound/HttpRequestHandlingMessagingGatewayTests.java @@ -16,6 +16,7 @@ package org.springframework.integration.http.inbound; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -38,6 +39,8 @@ import org.hamcrest.Matchers; import org.junit.Test; import org.springframework.beans.factory.BeanFactory; +import org.springframework.core.ParameterizedTypeReference; +import org.springframework.core.ResolvableType; import org.springframework.expression.common.LiteralExpression; import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpOutputMessage; @@ -64,6 +67,8 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.SerializationUtils; import org.springframework.web.multipart.MultipartResolver; +import com.fasterxml.jackson.databind.ObjectMapper; + /** * @author Mark Fisher * @author Gary Russell @@ -274,9 +279,52 @@ public class HttpRequestHandlingMessagingGatewayTests extends AbstractHttpInboun assertEquals(TestBean.class, message.getPayload().getClass()); TestBean result = (TestBean) message.getPayload(); assertEquals("T. Bean", result.name); - assertEquals(84, result.age); + assertEquals(42, result.age); } + @Test + public void testJsonRequestBody() throws Exception { + QueueChannel channel = new QueueChannel(); + HttpRequestHandlingMessagingGateway gateway = new HttpRequestHandlingMessagingGateway(false); + gateway.setBeanFactory(mock(BeanFactory.class)); + ParameterizedTypeReference> parameterizedTypeReference = + new ParameterizedTypeReference>() { + + }; + gateway.setRequestPayloadType(ResolvableType.forType(parameterizedTypeReference)); + gateway.setRequestChannel(channel); + gateway.afterPropertiesSet(); + gateway.start(); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/test"); + + request.setContentType("application/json"); + TestBean testBean = new TestBean(); + testBean.setName("T. Bean"); + testBean.setAge(42); + request.setContent(new ObjectMapper().writeValueAsBytes(new TestBean[] { testBean })); + MockHttpServletResponse response = new MockHttpServletResponse(); + gateway.handleRequest(request, response); + byte[] bytes = response.getContentAsByteArray(); + assertThat(bytes).isNotNull(); + Message message = channel.receive(0); + + assertThat(message).isNotNull() + .extracting(Message::getPayload) + .isInstanceOf(List.class) + .asList() + .hasSize(1) + .element(0) + .isInstanceOf(TestBean.class) + .satisfies((actual) -> { + TestBean bean = (TestBean) actual; + assertThat(bean).extracting(TestBean::getName).isEqualTo("T. Bean"); + assertThat(bean).extracting(TestBean::getAge).isEqualTo(42); + }); + + } + + @Test public void INT2680DuplicateContentTypeHeader() throws Exception { @@ -496,7 +544,15 @@ public class HttpRequestHandlingMessagingGatewayTests extends AbstractHttpInboun } public void setAge(int age) { - this.age = age * 2; + this.age = age; + } + + public String getName() { + return name; + } + + public int getAge() { + return age; } }