Skip to content

Commit b4ca71d

Browse files
committed
GH-10083: Apply Nullability to the transformer package
Related to: #10083 The `transformer` pattern cannot produce `null` at all. Therefore, making respective `Transformer.transform()` contract as non-null leads to changes in many places where it was possible to return null with subsequent `ReplyRequiredException` thrown from the `handler`. The current fix is a bit radical and transformer-specific: whenever we encounter `null` in the `transform()` call, we thrown a `MessageTransformationException` instead. Which is indeed making more sense from the `Transformer` perspective. We may treat it as a breaking change, but the docs still state clearly that there is an exception in case of null. Although it is different now. The Nullability work here has also led to the review of the `getComponentType()` implementation for the `Transformer` hierarchy. Plus, the `AbstractMessageProcessingTransformer` is now an `IntegrationObjectSupport`. The `MessageTransformingHandler` performs respective propagation in its `doInit()` to to the `transformer` if that one is `IntegrationObjectSupport`. The `HeaderEnricher` doesn't do a hard reference to the provided `Map` anymore to avoid potential mutation of the source externally. This has led to the change in the `HeaderEnricherSpec` to create a `HeaderEnricher` later in the `doGet()` with all respective properties propagation. Some of the test have suffered changes as well according to a new logic of the `transformer` package
1 parent 17e6a31 commit b4ca71d

File tree

45 files changed

+491
-368
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+491
-368
lines changed

spring-integration-core/src/main/java/org/springframework/integration/dsl/HeaderEnricherSpec.java

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,15 @@ public class HeaderEnricherSpec extends ConsumerEndpointSpec<HeaderEnricherSpec,
5959

6060
private static final String HEADERS_MUST_NOT_BE_NULL = "'headers' must not be null";
6161

62-
protected final Map<String, HeaderValueMessageProcessor<?>> headerToAdd = new HashMap<>(); // NOSONAR - final
62+
private final Map<String, HeaderValueMessageProcessor<?>> headerToAdd = new HashMap<>();
6363

64-
protected final HeaderEnricher headerEnricher = new HeaderEnricher(this.headerToAdd); // NOSONAR - final
64+
private boolean defaultOverwrite;
65+
66+
private boolean shouldSkipNulls;
67+
68+
private @Nullable MessageProcessor<?> messageProcessor;
6569

6670
protected HeaderEnricherSpec() {
67-
this.handler = new MessageTransformingHandler(this.headerEnricher);
6871
}
6972

7073
/**
@@ -75,7 +78,7 @@ protected HeaderEnricherSpec() {
7578
* @see HeaderEnricher#setDefaultOverwrite(boolean)
7679
*/
7780
public HeaderEnricherSpec defaultOverwrite(boolean defaultOverwrite) {
78-
this.headerEnricher.setDefaultOverwrite(defaultOverwrite);
81+
this.defaultOverwrite = defaultOverwrite;
7982
return _this();
8083
}
8184

@@ -85,7 +88,7 @@ public HeaderEnricherSpec defaultOverwrite(boolean defaultOverwrite) {
8588
* @see HeaderEnricher#setShouldSkipNulls(boolean)
8689
*/
8790
public HeaderEnricherSpec shouldSkipNulls(boolean shouldSkipNulls) {
88-
this.headerEnricher.setShouldSkipNulls(shouldSkipNulls);
91+
this.shouldSkipNulls = shouldSkipNulls;
8992
return _this();
9093
}
9194

@@ -99,7 +102,7 @@ public HeaderEnricherSpec shouldSkipNulls(boolean shouldSkipNulls) {
99102
* @see HeaderEnricher#setMessageProcessor(MessageProcessor)
100103
*/
101104
public HeaderEnricherSpec messageProcessor(MessageProcessor<?> messageProcessor) {
102-
this.headerEnricher.setMessageProcessor(messageProcessor);
105+
this.messageProcessor = messageProcessor;
103106
return _this();
104107
}
105108

@@ -877,7 +880,14 @@ public HeaderEnricherSpec headerChannelsToString(@Nullable String timeToLiveExpr
877880

878881
@Override
879882
protected Tuple2<ConsumerEndpointFactoryBean, MessageTransformingHandler> doGet() {
880-
this.componentsToRegister.put(this.headerEnricher, null);
883+
HeaderEnricher headerEnricher = new HeaderEnricher(this.headerToAdd);
884+
headerEnricher.setDefaultOverwrite(this.defaultOverwrite);
885+
headerEnricher.setShouldSkipNulls(this.shouldSkipNulls);
886+
if (this.messageProcessor != null) {
887+
headerEnricher.setMessageProcessor(this.messageProcessor);
888+
}
889+
this.handler = new MessageTransformingHandler(headerEnricher);
890+
this.componentsToRegister.put(headerEnricher, null);
881891
return super.doGet();
882892
}
883893

spring-integration-core/src/main/java/org/springframework/integration/dsl/Transformers.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ public static <T> DecodingTransformer<T> decoding(Codec codec, Expression typeEx
298298
* @return the {@link StreamTransformer} instance.
299299
*/
300300
public static StreamTransformer fromStream() {
301-
return fromStream(null);
301+
return new StreamTransformer();
302302
}
303303

304304
/**
@@ -307,7 +307,7 @@ public static StreamTransformer fromStream() {
307307
* @param charset the charset.
308308
* @return the {@link StreamTransformer} instance.
309309
*/
310-
public static StreamTransformer fromStream(@Nullable String charset) {
310+
public static StreamTransformer fromStream(String charset) {
311311
return new StreamTransformer(charset);
312312
}
313313

spring-integration-core/src/main/java/org/springframework/integration/support/AbstractIntegrationMessageBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public AbstractIntegrationMessageBuilder<T> setPriority(Integer priority) {
154154
* @see #copyHeadersIfAbsent(Map)
155155
*/
156156
public AbstractIntegrationMessageBuilder<T> filterAndCopyHeadersIfAbsent(Map<String, ?> headersToCopy,
157-
@Nullable String... headerPatternsToFilter) {
157+
String @Nullable ... headerPatternsToFilter) {
158158

159159
Map<String, ?> headers = headersToCopy;
160160

spring-integration-core/src/main/java/org/springframework/integration/transformer/AbstractMessageProcessingTransformer.java

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,14 @@
1818

1919
import java.util.Arrays;
2020

21-
import org.springframework.beans.factory.BeanFactory;
21+
import org.jspecify.annotations.Nullable;
22+
2223
import org.springframework.beans.factory.BeanFactoryAware;
2324
import org.springframework.context.Lifecycle;
25+
import org.springframework.integration.context.IntegrationObjectSupport;
2426
import org.springframework.integration.handler.MessageProcessor;
2527
import org.springframework.integration.support.AbstractIntegrationMessageBuilder;
26-
import org.springframework.integration.support.DefaultMessageBuilderFactory;
27-
import org.springframework.integration.support.MessageBuilderFactory;
2828
import org.springframework.integration.support.management.ManageableLifecycle;
29-
import org.springframework.integration.support.utils.IntegrationUtils;
3029
import org.springframework.messaging.Message;
3130
import org.springframework.messaging.MessageHeaders;
3231
import org.springframework.util.Assert;
@@ -40,17 +39,12 @@
4039
* @author Ngoc Nhan
4140
*/
4241
public abstract class AbstractMessageProcessingTransformer
43-
implements Transformer, BeanFactoryAware, ManageableLifecycle {
42+
extends IntegrationObjectSupport
43+
implements Transformer, ManageableLifecycle {
4444

4545
private final MessageProcessor<?> messageProcessor;
4646

47-
private BeanFactory beanFactory;
48-
49-
private MessageBuilderFactory messageBuilderFactory = new DefaultMessageBuilderFactory();
50-
51-
private boolean messageBuilderFactorySet;
52-
53-
private String[] notPropagatedHeaders;
47+
private String @Nullable [] notPropagatedHeaders;
5448

5549
private boolean selectiveHeaderPropagation;
5650

@@ -60,21 +54,11 @@ protected AbstractMessageProcessingTransformer(MessageProcessor<?> messageProces
6054
}
6155

6256
@Override
63-
public void setBeanFactory(BeanFactory beanFactory) {
64-
this.beanFactory = beanFactory;
57+
protected void onInit() {
58+
super.onInit();
6559
if (this.messageProcessor instanceof BeanFactoryAware beanFactoryAware) {
66-
beanFactoryAware.setBeanFactory(beanFactory);
67-
}
68-
}
69-
70-
protected MessageBuilderFactory getMessageBuilderFactory() {
71-
if (!this.messageBuilderFactorySet) {
72-
if (this.beanFactory != null) {
73-
this.messageBuilderFactory = IntegrationUtils.getMessageBuilderFactory(this.beanFactory);
74-
}
75-
this.messageBuilderFactorySet = true;
60+
beanFactoryAware.setBeanFactory(getBeanFactory());
7661
}
77-
return this.messageBuilderFactory;
7862
}
7963

8064
@Override
@@ -99,7 +83,7 @@ public boolean isRunning() {
9983
/**
10084
* Set headers that will NOT be copied from the inbound message if
10185
* the handler is configured to copy headers.
102-
* @param headers the headers to not propagate from the inbound message.
86+
* @param headers the headers do not propagate from the inbound message.
10387
* @since 5.1
10488
*/
10589
public void setNotPropagatedHeaders(String... headers) {
@@ -115,16 +99,17 @@ public void setNotPropagatedHeaders(String... headers) {
11599
public final Message<?> transform(Message<?> message) {
116100
Object result = this.messageProcessor.processMessage(message);
117101
if (result == null) {
118-
return null;
102+
throw new MessageTransformationException(message,
103+
"MessageProcessor returned null in: " + getComponentName());
119104
}
120-
if (result instanceof Message<?>) {
121-
return (Message<?>) result;
105+
if (result instanceof Message<?> messageToReply) {
106+
return messageToReply;
122107
}
123108

124109
AbstractIntegrationMessageBuilder<?> messageBuilder;
125110

126-
if (result instanceof AbstractIntegrationMessageBuilder<?>) {
127-
messageBuilder = (AbstractIntegrationMessageBuilder<?>) result;
111+
if (result instanceof AbstractIntegrationMessageBuilder<?> integrationMessageBuilder) {
112+
messageBuilder = integrationMessageBuilder;
128113
}
129114
else {
130115
messageBuilder = getMessageBuilderFactory().withPayload(result);

spring-integration-core/src/main/java/org/springframework/integration/transformer/AbstractPayloadTransformer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public abstract class AbstractPayloadTransformer<T, U> extends AbstractTransform
3434
@Override
3535
@SuppressWarnings("unchecked")
3636
public final U doTransform(Message<?> message) {
37-
return this.transformPayload((T) message.getPayload());
37+
return transformPayload((T) message.getPayload());
3838
}
3939

4040
protected abstract U transformPayload(T payload);

spring-integration-core/src/main/java/org/springframework/integration/transformer/AbstractTransformer.java

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,38 +24,31 @@
2424
*
2525
* @author Mark Fisher
2626
* @author Oleg Zhurakousky
27+
* @author Artem Bilan
2728
*/
2829
public abstract class AbstractTransformer extends IntegrationObjectSupport implements Transformer {
2930

3031
@Override
3132
public final Message<?> transform(Message<?> message) {
3233
try {
33-
Object result = this.doTransform(message);
34-
if (result == null) {
35-
return null;
36-
}
37-
return (result instanceof Message) ? (Message<?>) result
34+
Object result = doTransform(message);
35+
return result instanceof Message<?> resultMessage
36+
? resultMessage
3837
: getMessageBuilderFactory().withPayload(result).copyHeaders(message.getHeaders()).build();
3938
}
40-
catch (MessageTransformationException e) { // NOSONAR - catch and throw
41-
throw e;
42-
}
43-
catch (Exception e) {
44-
throw new MessageTransformationException(message, "failed to transform message", e);
39+
catch (Exception ex) {
40+
if (ex instanceof MessageTransformationException messageTransformationException) {
41+
throw messageTransformationException;
42+
}
43+
throw new MessageTransformationException(message, "failed to transform message", ex);
4544
}
4645
}
4746

48-
@Override
49-
public String getComponentType() {
50-
return "transformer";
51-
}
52-
5347
/**
5448
* Subclasses must implement this method to provide the transformation
5549
* logic. If the return value is itself a Message, it will be used as the
5650
* result. Otherwise, any non-null return value will be used as the payload
5751
* of the result Message.
58-
*
5952
* @param message The message.
6053
* @return The result of the transformation.
6154
*/

spring-integration-core/src/main/java/org/springframework/integration/transformer/ClaimCheckInTransformer.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ public class ClaimCheckInTransformer extends AbstractTransformer implements Inte
3939

4040
/**
4141
* Create a claim check-in transformer that will delegate to the provided MessageStore.
42-
*
4342
* @param messageStore The message store.
4443
*/
4544
public ClaimCheckInTransformer(MessageStore messageStore) {
@@ -59,7 +58,6 @@ public IntegrationPatternType getIntegrationPatternType() {
5958

6059
@Override
6160
protected Object doTransform(Message<?> message) {
62-
Assert.notNull(message, "message must not be null");
6361
UUID id = message.getHeaders().getId();
6462
Assert.notNull(id, "ID header must not be null");
6563
this.messageStore.addMessage(message);

spring-integration-core/src/main/java/org/springframework/integration/transformer/ClaimCheckOutTransformer.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ public class ClaimCheckOutTransformer extends AbstractTransformer implements Int
4747

4848
/**
4949
* Create a claim check-out transformer that will delegate to the provided MessageStore.
50-
*
5150
* @param messageStore The message store.
5251
*/
5352
public ClaimCheckOutTransformer(MessageStore messageStore) {
@@ -71,7 +70,6 @@ public IntegrationPatternType getIntegrationPatternType() {
7170

7271
@Override
7372
protected Object doTransform(Message<?> message) {
74-
Assert.notNull(message, "message must not be null");
7573
Assert.isTrue(message.getPayload() instanceof UUID, "payload must be a UUID");
7674
UUID id = (UUID) message.getPayload();
7775
Message<?> retrievedMessage;

0 commit comments

Comments
 (0)