Skip to content

Commit d8c41c2

Browse files
snicollsnicoll
committed
Defensively unwrap CacheConnectionFactory
This commit refines the optimization introduced in gh-39816 to only unwrap our own caching connection factory. The more advanced unwrap algorithm is still available, but opt-in only. Unwrapping more aggressively may break use cases where the wrapped ConnectionFactory is required, i.e. for transactional purposes. Closes gh-43277
1 parent 6029b01 commit d8c41c2

File tree

8 files changed

+122
-51
lines changed

8 files changed

+122
-51
lines changed

‎spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jms/JmsAnnotationDrivenConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ DefaultJmsListenerContainerFactoryConfigurer jmsListenerContainerFactoryConfigur
9090
DefaultJmsListenerContainerFactory jmsListenerContainerFactory(
9191
DefaultJmsListenerContainerFactoryConfigurer configurer, ConnectionFactory connectionFactory) {
9292
DefaultJmsListenerContainerFactory factory = new DefaultJmsListenerContainerFactory();
93-
configurer.configure(factory, ConnectionFactoryUnwrapper.unwrap(connectionFactory));
93+
configurer.configure(factory, ConnectionFactoryUnwrapper.unwrapCaching(connectionFactory));
9494
return factory;
9595
}
9696

‎spring-boot-project/spring-boot-docs/src/docs/antora/modules/reference/pages/messaging/jms.adoc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ In most scenarios, message listener containers should be configured against the
165165
This way each listener container has its own connection and this gives full responsibility to it in terms of local recovery.
166166
The auto-configuration uses javadoc:org.springframework.boot.jms.ConnectionFactoryUnwrapper[] to unwrap the native connection factory from the auto-configured one.
167167

168+
NOTE: The auto-configuration only unwraps `CachedConnectionFactory`.
169+
168170
By default, the default factory is transactional.
169171
If you run in an infrastructure where a javadoc:org.springframework.transaction.jta.JtaTransactionManager[] is present, it is associated to the listener container by default.
170172
If not, the `sessionTransacted` flag is enabled.

‎spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/howto/messaging/disabletransactedjmssession/MyJmsConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public class MyJmsConfiguration {
3131
public DefaultJmsListenerContainerFactory jmsListenerContainerFactory(ConnectionFactory connectionFactory,
3232
DefaultJmsListenerContainerFactoryConfigurer configurer) {
3333
DefaultJmsListenerContainerFactory listenerFactory = new DefaultJmsListenerContainerFactory();
34-
configurer.configure(listenerFactory, ConnectionFactoryUnwrapper.unwrap(connectionFactory));
34+
configurer.configure(listenerFactory, ConnectionFactoryUnwrapper.unwrapCaching(connectionFactory));
3535
listenerFactory.setTransactionManager(null);
3636
listenerFactory.setSessionTransacted(false);
3737
return listenerFactory;

‎spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/messaging/jms/receiving/custom/MyJmsConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public class MyJmsConfiguration {
3131
public DefaultJmsListenerContainerFactory myFactory(DefaultJmsListenerContainerFactoryConfigurer configurer,
3232
ConnectionFactory connectionFactory) {
3333
DefaultJmsListenerContainerFactory factory = new DefaultJmsListenerContainerFactory();
34-
configurer.configure(factory, ConnectionFactoryUnwrapper.unwrap(connectionFactory));
34+
configurer.configure(factory, ConnectionFactoryUnwrapper.unwrapCaching(connectionFactory));
3535
factory.setMessageConverter(new MyMessageConverter());
3636
return factory;
3737
}

‎spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/howto/messaging/disabletransactedjmssession/MyJmsConfiguration.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class MyJmsConfiguration {
3131
fun jmsListenerContainerFactory(connectionFactory: ConnectionFactory?,
3232
configurer: DefaultJmsListenerContainerFactoryConfigurer): DefaultJmsListenerContainerFactory {
3333
val listenerFactory = DefaultJmsListenerContainerFactory()
34-
configurer.configure(listenerFactory, ConnectionFactoryUnwrapper.unwrap(connectionFactory))
34+
configurer.configure(listenerFactory, ConnectionFactoryUnwrapper.unwrapCaching(connectionFactory))
3535
listenerFactory.setTransactionManager(null)
3636
listenerFactory.setSessionTransacted(false)
3737
return listenerFactory

‎spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/messaging/jms/receiving/custom/MyJmsConfiguration.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class MyJmsConfiguration {
3030
fun myFactory(configurer: DefaultJmsListenerContainerFactoryConfigurer,
3131
connectionFactory: ConnectionFactory): DefaultJmsListenerContainerFactory {
3232
val factory = DefaultJmsListenerContainerFactory()
33-
configurer.configure(factory, ConnectionFactoryUnwrapper.unwrap(connectionFactory))
33+
configurer.configure(factory, ConnectionFactoryUnwrapper.unwrapCaching(connectionFactory))
3434
factory.setMessageConverter(MyMessageConverter())
3535
return factory
3636
}

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jms/ConnectionFactoryUnwrapper.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,23 @@ public final class ConnectionFactoryUnwrapper {
3333
private ConnectionFactoryUnwrapper() {
3434
}
3535

36+
/**
37+
* Return the native {@link ConnectionFactory} by unwrapping ot from a
38+
* {@link CachingConnectionFactory}. Return the given {@link ConnectionFactory} if no
39+
* {@link CachingConnectionFactory} wrapper has been detected.
40+
* @param connectionFactory a connection factory
41+
* @return the native connection factory that a {@link CachingConnectionFactory}
42+
* wraps, if any
43+
* @since 3.4.1
44+
*/
45+
public static ConnectionFactory unwrapCaching(ConnectionFactory connectionFactory) {
46+
if (connectionFactory instanceof CachingConnectionFactory cachingConnectionFactory) {
47+
ConnectionFactory unwrapedConnectionFactory = cachingConnectionFactory.getTargetConnectionFactory();
48+
return (unwrapedConnectionFactory != null) ? unwrapCaching(unwrapedConnectionFactory) : connectionFactory;
49+
}
50+
return connectionFactory;
51+
}
52+
3653
/**
3754
* Return the native {@link ConnectionFactory} by unwrapping it from a cache or pool
3855
* connection factory. Return the given {@link ConnectionFactory} if no caching

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jms/ConnectionFactoryUnwrapperTests.java

Lines changed: 98 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.boot.jms;
1818

1919
import jakarta.jms.ConnectionFactory;
20+
import org.junit.jupiter.api.Nested;
2021
import org.junit.jupiter.api.Test;
2122
import org.messaginghub.pooled.jms.JmsPoolConnectionFactory;
2223

@@ -35,59 +36,110 @@
3536
*/
3637
class ConnectionFactoryUnwrapperTests {
3738

38-
@Test
39-
void unwrapWithSingleConnectionFactory() {
40-
ConnectionFactory connectionFactory = new SingleConnectionFactory();
41-
assertThat(ConnectionFactoryUnwrapper.unwrap(connectionFactory)).isSameAs(connectionFactory);
42-
}
39+
@Nested
40+
class UnwrapCaching {
4341

44-
@Test
45-
void unwrapWithConnectionFactory() {
46-
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
47-
assertThat(ConnectionFactoryUnwrapper.unwrap(connectionFactory)).isSameAs(connectionFactory);
48-
}
42+
@Test
43+
void unwrapWithSingleConnectionFactory() {
44+
ConnectionFactory connectionFactory = new SingleConnectionFactory();
45+
assertThat(unwrapCaching(connectionFactory)).isSameAs(connectionFactory);
46+
}
4947

50-
@Test
51-
void unwrapWithCachingConnectionFactory() {
52-
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
53-
assertThat(ConnectionFactoryUnwrapper.unwrap(new CachingConnectionFactory(connectionFactory)))
54-
.isSameAs(connectionFactory);
55-
}
48+
@Test
49+
void unwrapWithConnectionFactory() {
50+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
51+
assertThat(unwrapCaching(connectionFactory)).isSameAs(connectionFactory);
52+
}
5653

57-
@Test
58-
void unwrapWithNestedCachingConnectionFactories() {
59-
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
60-
CachingConnectionFactory firstCachingConnectionFactory = new CachingConnectionFactory(connectionFactory);
61-
CachingConnectionFactory secondCachingConnectionFactory = new CachingConnectionFactory(
62-
firstCachingConnectionFactory);
63-
assertThat(ConnectionFactoryUnwrapper.unwrap(secondCachingConnectionFactory)).isSameAs(connectionFactory);
64-
}
54+
@Test
55+
void unwrapWithCachingConnectionFactory() {
56+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
57+
assertThat(unwrapCaching(new CachingConnectionFactory(connectionFactory))).isSameAs(connectionFactory);
58+
}
6559

66-
@Test
67-
void unwrapWithJmsPoolConnectionFactory() {
68-
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
69-
JmsPoolConnectionFactory poolConnectionFactory = new JmsPoolConnectionFactory();
70-
poolConnectionFactory.setConnectionFactory(connectionFactory);
71-
assertThat(ConnectionFactoryUnwrapper.unwrap(poolConnectionFactory)).isSameAs(connectionFactory);
72-
}
60+
@Test
61+
void unwrapWithNestedCachingConnectionFactories() {
62+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
63+
CachingConnectionFactory firstCachingConnectionFactory = new CachingConnectionFactory(connectionFactory);
64+
CachingConnectionFactory secondCachingConnectionFactory = new CachingConnectionFactory(
65+
firstCachingConnectionFactory);
66+
assertThat(unwrapCaching(secondCachingConnectionFactory)).isSameAs(connectionFactory);
67+
}
68+
69+
@Test
70+
void unwrapWithJmsPoolConnectionFactory() {
71+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
72+
JmsPoolConnectionFactory poolConnectionFactory = new JmsPoolConnectionFactory();
73+
poolConnectionFactory.setConnectionFactory(connectionFactory);
74+
assertThat(unwrapCaching(poolConnectionFactory)).isSameAs(poolConnectionFactory);
75+
}
76+
77+
private ConnectionFactory unwrapCaching(ConnectionFactory connectionFactory) {
78+
return ConnectionFactoryUnwrapper.unwrapCaching(connectionFactory);
79+
}
7380

74-
@Test
75-
void unwrapWithNestedJmsPoolConnectionFactories() {
76-
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
77-
JmsPoolConnectionFactory firstPooledConnectionFactory = new JmsPoolConnectionFactory();
78-
firstPooledConnectionFactory.setConnectionFactory(connectionFactory);
79-
JmsPoolConnectionFactory secondPooledConnectionFactory = new JmsPoolConnectionFactory();
80-
secondPooledConnectionFactory.setConnectionFactory(firstPooledConnectionFactory);
81-
assertThat(ConnectionFactoryUnwrapper.unwrap(secondPooledConnectionFactory)).isSameAs(connectionFactory);
8281
}
8382

84-
@Test
85-
@ClassPathExclusions("pooled-jms-*")
86-
void unwrapWithoutJmsPoolOnClasspath() {
87-
assertThat(ClassUtils.isPresent("org.messaginghub.pooled.jms.JmsPoolConnectionFactory", null)).isFalse();
88-
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
89-
assertThat(ConnectionFactoryUnwrapper.unwrap(new CachingConnectionFactory(connectionFactory)))
90-
.isSameAs(connectionFactory);
83+
@Nested
84+
class Unwrap {
85+
86+
@Test
87+
void unwrapWithSingleConnectionFactory() {
88+
ConnectionFactory connectionFactory = new SingleConnectionFactory();
89+
assertThat(unwrap(connectionFactory)).isSameAs(connectionFactory);
90+
}
91+
92+
@Test
93+
void unwrapWithConnectionFactory() {
94+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
95+
assertThat(unwrap(connectionFactory)).isSameAs(connectionFactory);
96+
}
97+
98+
@Test
99+
void unwrapWithCachingConnectionFactory() {
100+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
101+
assertThat(unwrap(new CachingConnectionFactory(connectionFactory))).isSameAs(connectionFactory);
102+
}
103+
104+
@Test
105+
void unwrapWithNestedCachingConnectionFactories() {
106+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
107+
CachingConnectionFactory firstCachingConnectionFactory = new CachingConnectionFactory(connectionFactory);
108+
CachingConnectionFactory secondCachingConnectionFactory = new CachingConnectionFactory(
109+
firstCachingConnectionFactory);
110+
assertThat(unwrap(secondCachingConnectionFactory)).isSameAs(connectionFactory);
111+
}
112+
113+
@Test
114+
void unwrapWithJmsPoolConnectionFactory() {
115+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
116+
JmsPoolConnectionFactory poolConnectionFactory = new JmsPoolConnectionFactory();
117+
poolConnectionFactory.setConnectionFactory(connectionFactory);
118+
assertThat(unwrap(poolConnectionFactory)).isSameAs(connectionFactory);
119+
}
120+
121+
@Test
122+
void unwrapWithNestedJmsPoolConnectionFactories() {
123+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
124+
JmsPoolConnectionFactory firstPooledConnectionFactory = new JmsPoolConnectionFactory();
125+
firstPooledConnectionFactory.setConnectionFactory(connectionFactory);
126+
JmsPoolConnectionFactory secondPooledConnectionFactory = new JmsPoolConnectionFactory();
127+
secondPooledConnectionFactory.setConnectionFactory(firstPooledConnectionFactory);
128+
assertThat(unwrap(secondPooledConnectionFactory)).isSameAs(connectionFactory);
129+
}
130+
131+
@Test
132+
@ClassPathExclusions("pooled-jms-*")
133+
void unwrapWithoutJmsPoolOnClasspath() {
134+
assertThat(ClassUtils.isPresent("org.messaginghub.pooled.jms.JmsPoolConnectionFactory", null)).isFalse();
135+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
136+
assertThat(unwrap(new CachingConnectionFactory(connectionFactory))).isSameAs(connectionFactory);
137+
}
138+
139+
private ConnectionFactory unwrap(ConnectionFactory connectionFactory) {
140+
return ConnectionFactoryUnwrapper.unwrap(connectionFactory);
141+
}
142+
91143
}
92144

93145
}

0 commit comments

Comments
 (0)