From 3902c96a206022a0bffa2f1e88b73fecad546b59 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 8 Jan 2013 15:28:42 -0500 Subject: [PATCH] INT-2874 Fix JSON/JMS Incompatibility 2.2.0 changed ObjectToJsonTransformr to add content-type to simplify AMQP applications. However, in a JMS environment, the DefaultJmsHeaderMapper attempted to map (and failed) to map the header, emitting a warning log entry. JMS does not allow '-' in property names. Add code such that: 1. If the ObjectToJsonTransformer is configured to use a content-type of an empty String (after trimming), suppress the addition of the header to the output message. For consistency, if the inbound message already has a content type header, and the transformer is configured with an empty String, remove the header. 2. Change the DefaultJmsHeaderMapper to map the MessageHeaders.CONTENT_TYPE header (content-type) to/from a JMS compliant property name (content_type). INT-2874 PR Comments Move constant to JmsHeaderMapper interface and rename. Fix failing test (explicit set content-type to ""). INT-2874 Documentation Updates Add docbook and schema documentation clarifying the behavior of the transformer with respect to setting the `content-type` header. INT-2874 Polishing - Remove Header Removal After further discussion, we decided to not remove an existing header, if 'content-type' is set to "". This is because there was no way to handle the case of NOT adding a header when none present, while retaining a header if it was already present. added test for empty content-type attrib and no existing header (while merging) --- .../json/ObjectToJsonTransformer.java | 15 +++++---- .../config/xml/spring-integration-2.2.xsd | 16 +++++++--- .../ObjectToJsonTransformerParserTests.java | 22 ++++++++++--- .../json/ObjectToJsonTransformerTests.java | 24 +++++++++----- .../jms/DefaultJmsHeaderMapper.java | 22 ++++++++----- .../integration/jms/JmsHeaderMapper.java | 11 +++++-- .../jms/DefaultJmsHeaderMapperTests.java | 31 ++++++++++++++++--- src/reference/docbook/transformer.xml | 14 +++++++-- 8 files changed, 117 insertions(+), 38 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/json/ObjectToJsonTransformer.java b/spring-integration-core/src/main/java/org/springframework/integration/json/ObjectToJsonTransformer.java index c1e85e35ad..c0d5dc9c92 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/json/ObjectToJsonTransformer.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/json/ObjectToJsonTransformer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -18,13 +18,13 @@ package org.springframework.integration.json; import java.io.StringWriter; import org.codehaus.jackson.map.ObjectMapper; - import org.springframework.integration.Message; import org.springframework.integration.MessageHeaders; import org.springframework.integration.support.MessageBuilder; import org.springframework.integration.transformer.AbstractTransformer; import org.springframework.util.Assert; import org.springframework.util.LinkedCaseInsensitiveMap; +import org.springframework.util.StringUtils; /** * Transformer implementation that converts a payload instance into a JSON string representation. @@ -32,6 +32,7 @@ import org.springframework.util.LinkedCaseInsensitiveMap; * @author Mark Fisher * @author James Carr * @author Oleg Zhurakousky + * @author Gary Russell * @since 2.0 */ public class ObjectToJsonTransformer extends AbstractTransformer { @@ -61,7 +62,7 @@ public class ObjectToJsonTransformer extends AbstractTransformer { // only null assertion is needed since "" is a valid value Assert.notNull(contentType, "'contentType' must not be null"); this.contentTypeExplicitlySet = true; - this.contentType = contentType; + this.contentType = contentType.trim(); } private String transformPayload(Object payload) throws Exception { @@ -80,11 +81,13 @@ public class ObjectToJsonTransformer extends AbstractTransformer { if (headers.containsKey(MessageHeaders.CONTENT_TYPE)) { if (this.contentTypeExplicitlySet){ - // override - headers.put(MessageHeaders.CONTENT_TYPE, this.contentType); + // override, unless empty + if (StringUtils.hasLength(this.contentType)) { + headers.put(MessageHeaders.CONTENT_TYPE, this.contentType); + } } } - else { + else if (StringUtils.hasLength(this.contentType)) { headers.put(MessageHeaders.CONTENT_TYPE, this.contentType); } messageBuilder.copyHeaders(headers); diff --git a/spring-integration-core/src/main/resources/org/springframework/integration/config/xml/spring-integration-2.2.xsd b/spring-integration-core/src/main/resources/org/springframework/integration/config/xml/spring-integration-2.2.xsd index 2a1d2e02bc..8a69102be4 100644 --- a/spring-integration-core/src/main/resources/org/springframework/integration/config/xml/spring-integration-2.2.xsd +++ b/spring-integration-core/src/main/resources/org/springframework/integration/config/xml/spring-integration-2.2.xsd @@ -2106,11 +2106,17 @@ - - Allows you to set 'content-type' Message header. In the case that a content-type header is already present, - we will only override that value IF this attribute is explicitely set with - content-type value (e.g., content-type="text/xml") - + before or after the transformer. + ]]> diff --git a/spring-integration-core/src/test/java/org/springframework/integration/json/ObjectToJsonTransformerParserTests.java b/spring-integration-core/src/test/java/org/springframework/integration/json/ObjectToJsonTransformerParserTests.java index 6462889ab8..8388a87d33 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/json/ObjectToJsonTransformerParserTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/json/ObjectToJsonTransformerParserTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2013 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,6 +17,7 @@ package org.springframework.integration.json; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -31,6 +32,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; import org.springframework.integration.Message; import org.springframework.integration.MessageChannel; +import org.springframework.integration.MessageHeaders; import org.springframework.integration.channel.QueueChannel; import org.springframework.integration.support.MessageBuilder; import org.springframework.integration.test.util.TestUtils; @@ -40,12 +42,13 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; /** * @author Mark Fisher * @author Oleg Zhurakousky + * @author Gary Russell * @since 2.0 */ @ContextConfiguration @RunWith(SpringJUnit4ClassRunner.class) public class ObjectToJsonTransformerParserTests { - + @Autowired private volatile ApplicationContext context; @@ -56,11 +59,15 @@ public class ObjectToJsonTransformerParserTests { private volatile MessageChannel customObjectMapperInput; @Test - public void testContextType(){ - ObjectToJsonTransformer transformer = + public void testContentType(){ + ObjectToJsonTransformer transformer = TestUtils.getPropertyValue(context.getBean("defaultTransformer"), "handler.transformer", ObjectToJsonTransformer.class); assertEquals("application/json", TestUtils.getPropertyValue(transformer, "contentType")); + Message transformed = transformer.transform(MessageBuilder.withPayload("foo").build()); + assertTrue(transformed.getHeaders().containsKey(MessageHeaders.CONTENT_TYPE)); + assertEquals("application/json", transformed.getHeaders().get(MessageHeaders.CONTENT_TYPE)); + transformer = TestUtils.getPropertyValue(context.getBean("customTransformer"), "handler.transformer", ObjectToJsonTransformer.class); assertEquals("application/json", TestUtils.getPropertyValue(transformer, "contentType")); @@ -69,6 +76,13 @@ public class ObjectToJsonTransformerParserTests { TestUtils.getPropertyValue(context.getBean("emptyContentTypeTransformer"), "handler.transformer", ObjectToJsonTransformer.class); assertEquals("", TestUtils.getPropertyValue(transformer, "contentType")); + transformed = transformer.transform(MessageBuilder.withPayload("foo").build()); + assertFalse(transformed.getHeaders().containsKey(MessageHeaders.CONTENT_TYPE)); + + transformed = transformer.transform(MessageBuilder.withPayload("foo").setHeader(MessageHeaders.CONTENT_TYPE, "foo").build()); + assertNotNull(transformed.getHeaders().get(MessageHeaders.CONTENT_TYPE)); + assertEquals("foo", transformed.getHeaders().get(MessageHeaders.CONTENT_TYPE)); + transformer = TestUtils.getPropertyValue(context.getBean("overridenContentTypeTransformer"), "handler.transformer", ObjectToJsonTransformer.class); assertEquals("text/xml", TestUtils.getPropertyValue(transformer, "contentType")); diff --git a/spring-integration-core/src/test/java/org/springframework/integration/json/ObjectToJsonTransformerTests.java b/spring-integration-core/src/test/java/org/springframework/integration/json/ObjectToJsonTransformerTests.java index d0a95048e4..40ed23380a 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/json/ObjectToJsonTransformerTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/json/ObjectToJsonTransformerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -16,24 +16,25 @@ package org.springframework.integration.json; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + import java.util.regex.Matcher; import java.util.regex.Pattern; import org.codehaus.jackson.JsonGenerator.Feature; import org.codehaus.jackson.map.ObjectMapper; import org.junit.Test; - import org.springframework.integration.Message; import org.springframework.integration.MessageHeaders; import org.springframework.integration.message.GenericMessage; import org.springframework.integration.support.MessageBuilder; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - /** * @author Mark Fisher * @author Oleg Zhurakousky + * @author Gary Russell * @since 2.0 */ public class ObjectToJsonTransformerTests { @@ -71,13 +72,22 @@ public class ObjectToJsonTransformerTests { @Test public void withProvidedContentTypeAsEmptyString() throws Exception { + ObjectToJsonTransformer transformer = new ObjectToJsonTransformer(); + transformer.setContentType(""); + Message message = MessageBuilder.withPayload("foo").build(); + Message result = transformer.transform(message); + assertFalse(result.getHeaders().containsKey(MessageHeaders.CONTENT_TYPE)); + } + + @Test + public void withProvidedContentTypeAsEmptyStringDoesNotOverride() throws Exception { ObjectToJsonTransformer transformer = new ObjectToJsonTransformer(); transformer.setContentType(""); Message message = MessageBuilder.withPayload("foo").setHeader(MessageHeaders.CONTENT_TYPE, "text/xml").build(); Message result = transformer.transform(message); - assertEquals("", result.getHeaders().get(MessageHeaders.CONTENT_TYPE)); + assertEquals("text/xml", result.getHeaders().get(MessageHeaders.CONTENT_TYPE)); } - + @Test(expected=IllegalArgumentException.class) public void withProvidedContentTypeAsNull() throws Exception { ObjectToJsonTransformer transformer = new ObjectToJsonTransformer(); diff --git a/spring-integration-jms/src/main/java/org/springframework/integration/jms/DefaultJmsHeaderMapper.java b/spring-integration-jms/src/main/java/org/springframework/integration/jms/DefaultJmsHeaderMapper.java index 9d7a6196a1..fdfebebf2c 100644 --- a/spring-integration-jms/src/main/java/org/springframework/integration/jms/DefaultJmsHeaderMapper.java +++ b/spring-integration-jms/src/main/java/org/springframework/integration/jms/DefaultJmsHeaderMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2013 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. @@ -28,7 +28,6 @@ import javax.jms.JMSException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.springframework.integration.MessageHeaders; import org.springframework.util.StringUtils; @@ -47,8 +46,9 @@ import org.springframework.util.StringUtils; * Note that the JMSMessageID and JMSRedelivered flag are only copied * from a JMS Message. Those values will not be passed * along from a Spring Integration Message to an outbound JMS Message. - * + * * @author Mark Fisher + * @author Gary Russell */ public class DefaultJmsHeaderMapper implements JmsHeaderMapper { @@ -68,9 +68,9 @@ public class DefaultJmsHeaderMapper implements JmsHeaderMapper { * for any JMS property that is being mapped into the MessageHeaders. * The Default is an empty string (no prefix). *

- * This does not affect the JMS properties covered by the specification/API, + * This does not affect the JMS properties covered by the specification/API, * such as JMSCorrelationID, etc. The header names used for mapping such - * properties are all defined in our {@link JmsHeaders}. + * properties are all defined in our {@link JmsHeaders}. */ public void setInboundPrefix(String inboundPrefix) { this.inboundPrefix = (inboundPrefix != null) ? inboundPrefix : ""; @@ -81,9 +81,9 @@ public class DefaultJmsHeaderMapper implements JmsHeaderMapper { * integration message header that is being mapped into the JMS Message. * The Default is an empty string (no prefix). *

- * This does not affect the JMS properties covered by the specification/API, + * This does not affect the JMS properties covered by the specification/API, * such as JMSCorrelationID, etc. The header names used for mapping such - * properties are all defined in our {@link JmsHeaders}. + * properties are all defined in our {@link JmsHeaders}. */ public void setOutboundPrefix(String outboundPrefix) { this.outboundPrefix = (outboundPrefix != null) ? outboundPrefix : ""; @@ -228,23 +228,31 @@ public class DefaultJmsHeaderMapper implements JmsHeaderMapper { /** * Adds the outbound prefix if necessary. + * Converts {@link MessageHeaders#CONTENT_TYPE} to content_type for JMS compliance. */ private String fromHeaderName(String headerName) { String propertyName = headerName; if (StringUtils.hasText(this.outboundPrefix) && !propertyName.startsWith(this.outboundPrefix)) { propertyName = this.outboundPrefix + headerName; } + else if (MessageHeaders.CONTENT_TYPE.equals(headerName)) { + propertyName = CONTENT_TYPE_PROPERTY; + } return propertyName; } /** * Adds the inbound prefix if necessary. + * Converts content_type to {@link MessageHeaders#CONTENT_TYPE}. */ private String toHeaderName(String propertyName) { String headerName = propertyName; if (StringUtils.hasText(this.inboundPrefix) && !headerName.startsWith(this.inboundPrefix)) { headerName = this.inboundPrefix + propertyName; } + else if (CONTENT_TYPE_PROPERTY.equals(propertyName)) { + headerName = MessageHeaders.CONTENT_TYPE; + } return headerName; } diff --git a/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsHeaderMapper.java b/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsHeaderMapper.java index 0fdb38d604..70965b9d20 100644 --- a/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsHeaderMapper.java +++ b/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsHeaderMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2013 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. @@ -24,9 +24,14 @@ import org.springframework.integration.mapping.HeaderMapper; * Strategy interface for mapping integration Message headers to an outbound * JMS Message (e.g. to configure JMS properties) or extracting integration * header values from an inbound JMS Message. - * + * * @author Mark Fisher * @author Oleg Zhurakousky + * @author Gary Russell */ -public interface JmsHeaderMapper extends HeaderMapper {} +public interface JmsHeaderMapper extends HeaderMapper { + + static final String CONTENT_TYPE_PROPERTY = "content_type"; + +} diff --git a/spring-integration-jms/src/test/java/org/springframework/integration/jms/DefaultJmsHeaderMapperTests.java b/spring-integration-jms/src/test/java/org/springframework/integration/jms/DefaultJmsHeaderMapperTests.java index 7f23ef5a1f..1ca8bf5715 100644 --- a/spring-integration-jms/src/test/java/org/springframework/integration/jms/DefaultJmsHeaderMapperTests.java +++ b/spring-integration-jms/src/test/java/org/springframework/integration/jms/DefaultJmsHeaderMapperTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2013 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. @@ -28,14 +28,13 @@ import javax.jms.Destination; import javax.jms.JMSException; import org.junit.Test; - import org.springframework.integration.Message; -import org.springframework.integration.jms.DefaultJmsHeaderMapper; -import org.springframework.integration.jms.JmsHeaders; +import org.springframework.integration.MessageHeaders; import org.springframework.integration.support.MessageBuilder; /** * @author Mark Fisher + * @author Gary Russell */ public class DefaultJmsHeaderMapperTests { @@ -115,6 +114,19 @@ public class DefaultJmsHeaderMapperTests { assertNull(jmsMessage.getJMSType()); } + @Test + public void testContentTypePropertyMappedFromHeader() throws JMSException { + Message message = MessageBuilder.withPayload("test") + .setHeader(MessageHeaders.CONTENT_TYPE, "foo") + .build(); + DefaultJmsHeaderMapper mapper = new DefaultJmsHeaderMapper(); + javax.jms.Message jmsMessage = new StubTextMessage(); + mapper.fromHeaders(message.getHeaders(), jmsMessage); + Object value = jmsMessage.getObjectProperty("content_type"); + assertNotNull(value); + assertEquals("foo", value); + } + @Test public void testUserDefinedPropertyMappedFromHeader() throws JMSException { Message message = MessageBuilder.withPayload("test") @@ -217,6 +229,17 @@ public class DefaultJmsHeaderMapperTests { assertEquals(timestamp, attrib); } + @Test + public void testContentTypePropertyMappedToHeader() throws JMSException { + javax.jms.Message jmsMessage = new StubTextMessage(); + jmsMessage.setStringProperty("content_type", "foo"); + DefaultJmsHeaderMapper mapper = new DefaultJmsHeaderMapper(); + Map headers = mapper.toHeaders(jmsMessage); + Object attrib = headers.get(MessageHeaders.CONTENT_TYPE); + assertNotNull(attrib); + assertEquals("foo", attrib); + } + @Test public void testUserDefinedPropertyMappedToHeader() throws JMSException { javax.jms.Message jmsMessage = new StubTextMessage(); diff --git a/src/reference/docbook/transformer.xml b/src/reference/docbook/transformer.xml index c70041b94d..f0a8e0dd73 100644 --- a/src/reference/docbook/transformer.xml +++ b/src/reference/docbook/transformer.xml @@ -230,8 +230,18 @@ public class Kid { Beginning with version 2.2, the object-to-json-transformer sets the content-type - header to application/json, by default. It can be overridden using the content-type - attribute. The default behavior has a side affect - causing applications with the following + header to application/json, by default, if the input message does not already have that header + present. + + + It you wish to set the content type header to some other value, or explicitly overwrite any existing header + with some value (including application/json), use the content-type + attribute. If you wish to suppress the setting of the header, set the content-type + attribute to an empty string (""). This will result in a message with no content-type + header, unless such a header was present on the input message. + + + The behavior of adding the default header has a side affect - causing applications with the following sequence to fail: