From 3eedda6c9d4acf2bb762bd543f5992916c52075e Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 18 Mar 2020 17:31:00 -0400 Subject: [PATCH] GH- 3223: Build `ResolvableType` after mapping Fixes https://github.com/spring-projects/spring-integration/issues/3223 The `json__TypeId__` header may have a value which cannot be resolved to the `Class` in the current classpath So, skip `ResolvableType` building logic until we really sure that end-user wants to map JSON headers * WARN a build exception that we can't load a class for the `json__TypeId__` when we try to build a `ResolvableType` in the `DefaultAmqpHeaderMapper` * Document the negation feature for JSON headers **Cherry-pick to 5.2.x** --- .../amqp/support/DefaultAmqpHeaderMapper.java | 15 +-- .../support/DefaultAmqpHeaderMapperTests.java | 33 ++++-- .../mapping/AbstractHeaderMapper.java | 100 ++++++++++-------- src/reference/asciidoc/amqp.adoc | 10 +- 4 files changed, 82 insertions(+), 76 deletions(-) diff --git a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/support/DefaultAmqpHeaderMapper.java b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/support/DefaultAmqpHeaderMapper.java index 17d89133b2..66815c307b 100644 --- a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/support/DefaultAmqpHeaderMapper.java +++ b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/support/DefaultAmqpHeaderMapper.java @@ -151,26 +151,13 @@ public class DefaultAmqpHeaderMapper extends AbstractHeaderMapper headers) { - Object typeIdHeader = headers.get(JsonHeaders.TYPE_ID); - if (typeIdHeader != null) { - headers.put(JsonHeaders.RESOLVABLE_TYPE, - JsonHeaders.buildResolvableType(getClassLoader(), typeIdHeader, - headers.get(JsonHeaders.CONTENT_TYPE_ID), headers.get(JsonHeaders.KEY_TYPE_ID))); - } - } - /** * Extract user-defined headers from an AMQP MessageProperties instance. */ diff --git a/spring-integration-amqp/src/test/java/org/springframework/integration/amqp/support/DefaultAmqpHeaderMapperTests.java b/spring-integration-amqp/src/test/java/org/springframework/integration/amqp/support/DefaultAmqpHeaderMapperTests.java index 1b252f991d..2a6b5f9376 100644 --- a/spring-integration-amqp/src/test/java/org/springframework/integration/amqp/support/DefaultAmqpHeaderMapperTests.java +++ b/spring-integration-amqp/src/test/java/org/springframework/integration/amqp/support/DefaultAmqpHeaderMapperTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -26,12 +26,13 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.springframework.amqp.core.Message; import org.springframework.amqp.core.MessageDeliveryMode; import org.springframework.amqp.core.MessageProperties; import org.springframework.amqp.support.AmqpHeaders; +import org.springframework.amqp.support.converter.AbstractJavaTypeMapper; import org.springframework.amqp.support.converter.Jackson2JsonMessageConverter; import org.springframework.amqp.support.converter.MessageConverter; import org.springframework.core.ResolvableType; @@ -68,7 +69,7 @@ public class DefaultAmqpHeaderMapperTests { @Test public void fromHeaders() { DefaultAmqpHeaderMapper headerMapper = DefaultAmqpHeaderMapper.outboundMapper(); - Map headerMap = new HashMap(); + Map headerMap = new HashMap<>(); headerMap.put(AmqpHeaders.APP_ID, "test.appId"); headerMap.put(AmqpHeaders.CLUSTER_ID, "test.clusterId"); headerMap.put(AmqpHeaders.CONTENT_ENCODING, "test.contentEncoding"); @@ -130,7 +131,7 @@ public class DefaultAmqpHeaderMapperTests { @Test public void fromHeadersWithContentTypeAsMediaType() { DefaultAmqpHeaderMapper headerMapper = DefaultAmqpHeaderMapper.inboundMapper(); - Map headerMap = new HashMap(); + Map headerMap = new HashMap<>(); MediaType contentType = MediaType.parseMediaType("text/html"); headerMap.put(AmqpHeaders.CONTENT_TYPE, contentType); @@ -152,7 +153,7 @@ public class DefaultAmqpHeaderMapperTests { @Test public void fromHeadersWithContentTypeAsMimeType() { DefaultAmqpHeaderMapper headerMapper = DefaultAmqpHeaderMapper.inboundMapper(); - Map headerMap = new HashMap(); + Map headerMap = new HashMap<>(); MimeType contentType = MimeType.valueOf("text/html"); headerMap.put(AmqpHeaders.CONTENT_TYPE, contentType); @@ -217,7 +218,7 @@ public class DefaultAmqpHeaderMapperTests { } @Test - public void toHeadersNonContenType() { + public void toHeadersNonContentType() { DefaultAmqpHeaderMapper headerMapper = DefaultAmqpHeaderMapper.inboundMapper(); MessageProperties amqpProperties = new MessageProperties(); amqpProperties.setAppId("test.appId"); @@ -244,7 +245,7 @@ public class DefaultAmqpHeaderMapperTests { @Test public void messageIdNotMappedToAmqpProperties() { DefaultAmqpHeaderMapper headerMapper = DefaultAmqpHeaderMapper.inboundMapper(); - Map headerMap = new HashMap(); + Map headerMap = new HashMap<>(); headerMap.put(MessageHeaders.ID, "msg-id"); MessageHeaders integrationHeaders = new MessageHeaders(headerMap); MessageProperties amqpProperties = new MessageProperties(); @@ -255,7 +256,7 @@ public class DefaultAmqpHeaderMapperTests { @Test public void messageTimestampNotMappedToAmqpProperties() { DefaultAmqpHeaderMapper headerMapper = DefaultAmqpHeaderMapper.inboundMapper(); - Map headerMap = new HashMap(); + Map headerMap = new HashMap<>(); headerMap.put(MessageHeaders.TIMESTAMP, 1234L); MessageHeaders integrationHeaders = new MessageHeaders(headerMap); MessageProperties amqpProperties = new MessageProperties(); @@ -263,13 +264,13 @@ public class DefaultAmqpHeaderMapperTests { assertThat(amqpProperties.getHeaders().get(MessageHeaders.TIMESTAMP)).isNull(); } - @Test // INT-2090 + @Test public void jsonTypeIdNotOverwritten() { DefaultAmqpHeaderMapper headerMapper = DefaultAmqpHeaderMapper.inboundMapper(); MessageConverter converter = new Jackson2JsonMessageConverter(); MessageProperties amqpProperties = new MessageProperties(); converter.toMessage("123", amqpProperties); - Map headerMap = new HashMap(); + Map headerMap = new HashMap<>(); headerMap.put("__TypeId__", "java.lang.Integer"); MessageHeaders integrationHeaders = new MessageHeaders(headerMap); headerMapper.fromHeadersToRequest(integrationHeaders, amqpProperties); @@ -325,4 +326,16 @@ public class DefaultAmqpHeaderMapperTests { assertThat(amqpProperties.getHeaders()).doesNotContainKeys(JsonHeaders.RESOLVABLE_TYPE); } + @Test + public void jsonHeadersNotMapped() { + DefaultAmqpHeaderMapper headerMapper = DefaultAmqpHeaderMapper.inboundMapper(); + headerMapper.setRequestHeaderNames("!json_*", "*"); + MessageProperties amqpProperties = new MessageProperties(); + amqpProperties.getHeaders().put(AbstractJavaTypeMapper.DEFAULT_CLASSID_FIELD_NAME, "test.type"); + Map headers = headerMapper.toHeadersFromRequest(amqpProperties); + assertThat(headers) + .doesNotContainKeys(JsonHeaders.RESOLVABLE_TYPE, JsonHeaders.TYPE_ID) + .containsKey(AbstractJavaTypeMapper.DEFAULT_CLASSID_FIELD_NAME); + } + } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/mapping/AbstractHeaderMapper.java b/spring-integration-core/src/main/java/org/springframework/integration/mapping/AbstractHeaderMapper.java index b583abc296..3e3cefce03 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/mapping/AbstractHeaderMapper.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/mapping/AbstractHeaderMapper.java @@ -29,6 +29,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanClassLoaderAware; +import org.springframework.core.ResolvableType; +import org.springframework.integration.mapping.support.JsonHeaders; import org.springframework.lang.Nullable; import org.springframework.messaging.MessageChannel; import org.springframework.messaging.MessageHeaders; @@ -67,8 +69,8 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe */ public static final String NON_STANDARD_HEADER_NAME_PATTERN = "NON_STANDARD_HEADERS"; - private static final Collection TRANSIENT_HEADER_NAMES = Arrays.asList( - MessageHeaders.ID, MessageHeaders.TIMESTAMP); + private static final Collection TRANSIENT_HEADER_NAMES = + Arrays.asList(MessageHeaders.ID, MessageHeaders.TIMESTAMP); protected final Log logger = LogFactory.getLog(getClass()); // NOSONAR final @@ -155,7 +157,7 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe * @return a header mapper that match if any of the specified patters match */ protected HeaderMatcher createHeaderMatcher(Collection patterns) { - List matchers = new ArrayList(); + List matchers = new ArrayList<>(); for (String pattern : patterns) { if (STANDARD_REQUEST_HEADER_NAME_PATTERN.equals(pattern)) { matchers.add(new ContentBasedHeaderMatcher(true, this.requestHeaderNames)); @@ -190,40 +192,38 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe @Override public void fromHeadersToRequest(MessageHeaders headers, T target) { - this.fromHeaders(headers, target, this.requestHeaderMatcher); + fromHeaders(headers, target, this.requestHeaderMatcher); } @Override public void fromHeadersToReply(MessageHeaders headers, T target) { - this.fromHeaders(headers, target, this.replyHeaderMatcher); + fromHeaders(headers, target, this.replyHeaderMatcher); } @Override public Map toHeadersFromRequest(T source) { - return this.toHeaders(source, this.requestHeaderMatcher); + return toHeaders(source, this.requestHeaderMatcher); } @Override public Map toHeadersFromReply(T source) { - return this.toHeaders(source, this.replyHeaderMatcher); + return toHeaders(source, this.replyHeaderMatcher); } private void fromHeaders(MessageHeaders headers, T target, HeaderMatcher headerMatcher) { try { - Map subset = new HashMap(); + Map subset = new HashMap<>(); for (Map.Entry entry : headers.entrySet()) { String headerName = entry.getKey(); - if (this.shouldMapHeader(headerName, headerMatcher)) { + if (shouldMapHeader(headerName, headerMatcher)) { subset.put(headerName, entry.getValue()); } } - this.populateStandardHeaders(headers, subset, target); - this.populateUserDefinedHeaders(subset, target); + populateStandardHeaders(headers, subset, target); + populateUserDefinedHeaders(subset, target); } catch (Exception e) { - if (this.logger.isWarnEnabled()) { - this.logger.warn("error occurred while mapping from MessageHeaders", e); - } + this.logger.warn("error occurred while mapping from MessageHeaders", e); } } @@ -234,8 +234,8 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe if (value != null && !isMessageChannel(headerName, value)) { try { if (!headerName.startsWith(this.standardHeaderPrefix)) { - String key = this.createTargetPropertyName(headerName, true); - this.populateUserDefinedHeader(key, value, target); + String key = createTargetPropertyName(headerName, true); + populateUserDefinedHeader(key, value, target); } } catch (Exception e) { @@ -262,21 +262,30 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe * a {@link org.springframework.messaging.Message}. */ private Map toHeaders(T source, HeaderMatcher headerMatcher) { - Map headers = new HashMap(); + Map headers = new HashMap<>(); Map standardHeaders = extractStandardHeaders(source); - this.copyHeaders(standardHeaders, headers, headerMatcher); + copyHeaders(standardHeaders, headers, headerMatcher); Map userDefinedHeaders = extractUserDefinedHeaders(source); - this.copyHeaders(userDefinedHeaders, headers, headerMatcher); + copyHeaders(userDefinedHeaders, headers, headerMatcher); return headers; } - private void copyHeaders(Map source, Map target, HeaderMatcher headerMatcher) { + private void copyHeaders(Map source, Map target, HeaderMatcher headerMatcher) { if (!CollectionUtils.isEmpty(source)) { for (Map.Entry entry : source.entrySet()) { try { - String headerName = this.createTargetPropertyName(entry.getKey(), false); - if (this.shouldMapHeader(headerName, headerMatcher)) { - target.put(headerName, entry.getValue()); + String headerName = createTargetPropertyName(entry.getKey(), false); + if (shouldMapHeader(headerName, headerMatcher)) { + Object value = entry.getValue(); + target.put(headerName, value); + if (JsonHeaders.TYPE_ID.equals(headerName) && value != null) { + ResolvableType resolvableType = + createJsonResolvableTypHeaderInAny(value, source.get(JsonHeaders.CONTENT_TYPE_ID), + source.get(JsonHeaders.KEY_TYPE_ID)); + if (resolvableType != null) { + target.put(JsonHeaders.RESOLVABLE_TYPE, resolvableType); + } + } } } catch (Exception e) { @@ -289,6 +298,20 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe } } + @Nullable + private ResolvableType createJsonResolvableTypHeaderInAny(Object typeId, @Nullable Object contentId, + @Nullable Object keyId) { + + try { + return JsonHeaders.buildResolvableType(getClassLoader(), typeId, + contentId, keyId); + } + catch (Exception e) { + this.logger.warn("Cannot build a ResolvableType from 'json__TypeId__' header", e); + } + return null; + } + private boolean shouldMapHeader(String headerName, HeaderMatcher headerMatcher) { return !(!StringUtils.hasText(headerName) || getTransientHeaderNames().contains(headerName)) && headerMatcher.matchHeader(headerName); @@ -303,8 +326,8 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe } if (!type.isAssignableFrom(value.getClass())) { if (this.logger.isWarnEnabled()) { - this.logger.warn("skipping header '" + name + "' since it is not of expected type [" + type + "], it is [" + - value.getClass() + "]"); + this.logger.warn("skipping header '" + name + "' since it is not of expected type [" + type + "], " + + "it is [" + value.getClass() + "]"); } return null; } @@ -380,6 +403,7 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe * Strategy interface to determine if a given header name matches. * @since 4.1 */ + @FunctionalInterface public interface HeaderMatcher { /** @@ -393,7 +417,9 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe * Return true if this match should be explicitly excluded from the mapping. * @return true if negated. */ - boolean isNegated(); + default boolean isNegated() { + return false; + } } @@ -440,11 +466,6 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe return false; } - @Override - public boolean isNegated() { - return false; - } - } /** @@ -457,7 +478,7 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe private static final Log logger = LogFactory.getLog(HeaderMatcher.class); - private final Collection patterns = new ArrayList(); + private final Collection patterns = new ArrayList<>(); public PatternBasedHeaderMatcher(Collection patterns) { Assert.notNull(patterns, "Patterns must no be null"); @@ -482,11 +503,6 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe return false; } - @Override - public boolean isNegated() { - return false; - } - } /** @@ -566,11 +582,6 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe return result; } - @Override - public boolean isNegated() { - return false; - } - } /** @@ -608,11 +619,6 @@ public abstract class AbstractHeaderMapper implements RequestReplyHeaderMappe return false; } - @Override - public boolean isNegated() { - return false; - } - } } diff --git a/src/reference/asciidoc/amqp.adoc b/src/reference/asciidoc/amqp.adoc index cb4bba1281..ec76639fcb 100644 --- a/src/reference/asciidoc/amqp.adoc +++ b/src/reference/asciidoc/amqp.adoc @@ -1436,10 +1436,8 @@ The `org.springframework.amqp.support.AmqpHeaders` class identifies the default [[header-copy-caution]] CAUTION: As mentioned earlier in this section, using a header mapping pattern of`*` is a common way to copy all headers. -However, this can have some unexpected side effects, because certain RabbitMQ proprietary properties/headers are also -copied. -For example, when you use https://www.rabbitmq.com/federated-exchanges.html[federation], the received message may have -a property named `x-received-from`, which contains the node that sent the message. +However, this can have some unexpected side effects, because certain RabbitMQ proprietary properties/headers are also copied. +For example, when you use https://www.rabbitmq.com/federated-exchanges.html[federation], the received message may have a property named `x-received-from`, which contains the node that sent the message. If you use the wildcard character `*` for the request and reply header mapping on the inbound gateway, this header is copied, which may cause some issues with federation. This reply message may be federated back to the sending broker, which may think that a message is looping and, as a result, silently drop it. If you wish to use the convenience of wildcard header mapping, you may need to filter out some headers in the downstream flow. @@ -1452,6 +1450,8 @@ Instead, this header is mapped to `amqp_receivedDeliveryMode`, which is not mapp Starting with version 4.3, patterns in the header mappings can be negated by preceding the pattern with `!`. Negated patterns get priority, so a list such as `STANDARD_REQUEST_HEADERS,thing1,ba*,!thing2,!thing3,qux,!thing1` does not map `thing1` (nor `thing2` nor `thing3`). The standard headers plus `bad` and `qux` are mapped. +The negation technique can be useful for example to not map JSON type headers for incoming messages when a JSON deserialization logic is done in the receiver downstream different way. +For this purpose a `!json_*` pattern should be configured for header mapper of the inbound channel adapter/gateway. IMPORTANT: If you have a user-defined header that begins with `!` that you do wish to map, you need to escape it with `\`, as follows: `STANDARD_REQUEST_HEADERS,\!myBangHeader`. The header named `!myBangHeader` is now mapped. @@ -1468,7 +1468,7 @@ For example an inbound HTTP message sent to a RabbitMQ queue. The `contentType` header is mapped to Spring AMQP's `MessageProperties.contentType` property and that is subsequently mapped to RabbitMQ's `content_type` property. -Prior to _version 5.1_, this header was also mapped as an entry in the `MessageProperties.headers` map; this was incorrect and, furthermore, the value could be wrong since the underlying Spring AMQP message converter might have changed the content type. +Prior to version 5.1, this header was also mapped as an entry in the `MessageProperties.headers` map; this was incorrect and, furthermore, the value could be wrong since the underlying Spring AMQP message converter might have changed the content type. Such a change would be reflected in the first-class `content_type` property, but not in the RabbitMQ headers map. Inbound mapping ignored the headers map value. `contentType` is no longer mapped to an entry in the headers map.