From 3d61d9e0d8ebfc130ccb768155455eaa2ddb98d2 Mon Sep 17 00:00:00 2001 From: Vedran Pavic Date: Mon, 12 Jun 2023 10:46:22 +0200 Subject: [PATCH 1/2] Handle custom JMS acknowledgment modes as client acknowledge (#30619) This commit updates JmsAccessor to handle custom JMS acknowledgment modes as client acknowledge, which is useful when working with JMS providers that provide non-standard variations of CLIENT_ACKNOWLEDGE, such as AWS SQS and its UNORDERED_ACKNOWLEDGE mode. --- .../jms/support/JmsAccessor.java | 7 ++++-- .../jms/support/JmsAccessorTests.java | 22 ++++++++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/spring-jms/src/main/java/org/springframework/jms/support/JmsAccessor.java b/spring-jms/src/main/java/org/springframework/jms/support/JmsAccessor.java index 1eea339744..a7a45c81d5 100644 --- a/spring-jms/src/main/java/org/springframework/jms/support/JmsAccessor.java +++ b/spring-jms/src/main/java/org/springframework/jms/support/JmsAccessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -223,7 +223,10 @@ public abstract class JmsAccessor implements InitializingBean { * @see jakarta.jms.Session#CLIENT_ACKNOWLEDGE */ protected boolean isClientAcknowledge(Session session) throws JMSException { - return (session.getAcknowledgeMode() == Session.CLIENT_ACKNOWLEDGE); + int mode = session.getAcknowledgeMode(); + return (mode != Session.SESSION_TRANSACTED && + mode != Session.AUTO_ACKNOWLEDGE && + mode != Session.DUPS_OK_ACKNOWLEDGE); } } diff --git a/spring-jms/src/test/java/org/springframework/jms/support/JmsAccessorTests.java b/spring-jms/src/test/java/org/springframework/jms/support/JmsAccessorTests.java index 1e2f4b6676..516fc82365 100644 --- a/spring-jms/src/test/java/org/springframework/jms/support/JmsAccessorTests.java +++ b/spring-jms/src/test/java/org/springframework/jms/support/JmsAccessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 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. @@ -21,24 +21,27 @@ import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; /** * Unit tests for the {@link JmsAccessor} class. * * @author Rick Evans * @author Chris Beams + * @author Vedran Pavic */ -public class JmsAccessorTests { +class JmsAccessorTests { @Test - public void testChokesIfConnectionFactoryIsNotSupplied() throws Exception { + void testChokesIfConnectionFactoryIsNotSupplied() { JmsAccessor accessor = new StubJmsAccessor(); assertThatIllegalArgumentException().isThrownBy( accessor::afterPropertiesSet); } @Test - public void testSessionTransactedModeReallyDoesDefaultToFalse() throws Exception { + void testSessionTransactedModeReallyDoesDefaultToFalse() { JmsAccessor accessor = new StubJmsAccessor(); assertThat(accessor.isSessionTransacted()).as("The [sessionTransacted] property of JmsAccessor must default to " + "false. Change this test (and the attendant Javadoc) if you have " + @@ -46,7 +49,7 @@ public class JmsAccessorTests { } @Test - public void testAcknowledgeModeReallyDoesDefaultToAutoAcknowledge() throws Exception { + void testAcknowledgeModeReallyDoesDefaultToAutoAcknowledge() { JmsAccessor accessor = new StubJmsAccessor(); assertThat(accessor.getSessionAcknowledgeMode()).as("The [sessionAcknowledgeMode] property of JmsAccessor must default to " + "[Session.AUTO_ACKNOWLEDGE]. Change this test (and the attendant " + @@ -54,11 +57,18 @@ public class JmsAccessorTests { } @Test - public void testSetAcknowledgeModeNameChokesIfBadAckModeIsSupplied() throws Exception { + void testSetAcknowledgeModeNameChokesIfBadAckModeIsSupplied() { assertThatIllegalArgumentException().isThrownBy(() -> new StubJmsAccessor().setSessionAcknowledgeModeName("Tally ho chaps!")); } + @Test + void testCustomAcknowledgeModeIsConsideredClientAcknowledge() throws Exception { + Session session = mock(Session.class); + given(session.getAcknowledgeMode()).willReturn(100); + JmsAccessor accessor = new StubJmsAccessor(); + assertThat(accessor.isClientAcknowledge(session)).isTrue(); + } /** * Crummy, stub, do-nothing subclass of the JmsAccessor class for use in testing. From 0a5aff1b60f3082d07c523198e9f9d35aea6ffa3 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 12 Jun 2023 10:48:57 +0200 Subject: [PATCH 2/2] Specific check for parent of spring-aop ClassLoader Also applied to getProxyClass now. Closes gh-30389 --- .../aop/framework/JdkDynamicAopProxy.java | 35 +++++++++++++++---- .../aop/framework/ProxyFactoryTests.java | 12 +++++-- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java index 6d557b62b3..e5f9c08531 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java @@ -120,18 +120,39 @@ final class JdkDynamicAopProxy implements AopProxy, InvocationHandler, Serializa if (logger.isTraceEnabled()) { logger.trace("Creating JDK dynamic proxy: " + this.advised.getTargetSource()); } - if (classLoader == null || classLoader.getParent() == null) { - // JDK bootstrap loader or platform loader suggested -> - // use higher-level loader which can see Spring infrastructure classes - classLoader = getClass().getClassLoader(); - } - return Proxy.newProxyInstance(classLoader, this.proxiedInterfaces, this); + return Proxy.newProxyInstance(determineClassLoader(classLoader), this.proxiedInterfaces, this); } @SuppressWarnings("deprecation") @Override public Class getProxyClass(@Nullable ClassLoader classLoader) { - return Proxy.getProxyClass(classLoader, this.proxiedInterfaces); + return Proxy.getProxyClass(determineClassLoader(classLoader), this.proxiedInterfaces); + } + + /** + * Determine whether the JDK bootstrap or platform loader has been suggested -> + * use higher-level loader which can see Spring infrastructure classes instead. + */ + private ClassLoader determineClassLoader(@Nullable ClassLoader classLoader) { + if (classLoader == null) { + // JDK bootstrap loader -> use spring-aop ClassLoader instead. + return getClass().getClassLoader(); + } + if (classLoader.getParent() == null) { + // Potentially the JDK platform loader on JDK 9+ + ClassLoader aopClassLoader = getClass().getClassLoader(); + ClassLoader aopParent = aopClassLoader.getParent(); + while (aopParent != null) { + if (classLoader == aopParent) { + // Suggested ClassLoader is ancestor of spring-aop ClassLoader + // -> use spring-aop ClassLoader itself instead. + return aopClassLoader; + } + aopParent = aopParent.getParent(); + } + } + // Regular case: use suggested ClassLoader as-is. + return classLoader; } /** diff --git a/spring-aop/src/test/java/org/springframework/aop/framework/ProxyFactoryTests.java b/spring-aop/src/test/java/org/springframework/aop/framework/ProxyFactoryTests.java index effbed9f8d..a0f5291650 100644 --- a/spring-aop/src/test/java/org/springframework/aop/framework/ProxyFactoryTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/framework/ProxyFactoryTests.java @@ -389,7 +389,9 @@ public class ProxyFactoryTests { CharSequence target = "test"; ProxyFactory pf = new ProxyFactory(target); ClassLoader cl = target.getClass().getClassLoader(); - assertThat(((CharSequence) pf.getProxy(cl)).toString()).isEqualTo(target); + CharSequence proxy = (CharSequence) pf.getProxy(cl); + assertThat(proxy.toString()).isEqualTo(target); + assertThat(pf.getProxyClass(cl)).isSameAs(proxy.getClass()); } @Test @@ -398,7 +400,9 @@ public class ProxyFactoryTests { ProxyFactory pf = new ProxyFactory(target); pf.setProxyTargetClass(true); ClassLoader cl = target.getClass().getClassLoader(); - assertThat(((Date) pf.getProxy(cl)).getTime()).isEqualTo(target.getTime()); + Date proxy = (Date) pf.getProxy(cl); + assertThat(proxy.getTime()).isEqualTo(target.getTime()); + assertThat(pf.getProxyClass(cl)).isSameAs(proxy.getClass()); } @Test @@ -415,7 +419,9 @@ public class ProxyFactoryTests { }; ProxyFactory pf = new ProxyFactory(target); ClassLoader cl = Savepoint.class.getClassLoader(); - assertThat(((Savepoint) pf.getProxy(cl)).getSavepointName()).isEqualTo("sp"); + Savepoint proxy = (Savepoint) pf.getProxy(cl); + assertThat(proxy.getSavepointName()).isEqualTo("sp"); + assertThat(pf.getProxyClass(cl)).isSameAs(proxy.getClass()); }