From 44dd551c19edbc4f049b4ee05196fbd71a18ed73 Mon Sep 17 00:00:00 2001 From: Kim Jun Hyeong Date: Wed, 7 May 2025 23:56:22 +0900 Subject: [PATCH 1/5] Fix proxy attribute name mismatch in RetryOperationsInterceptor Signed-off-by: Kim Jun Hyeong --- .../retry/interceptor/RetryOperationsInterceptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/retry/interceptor/RetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/interceptor/RetryOperationsInterceptor.java index a8eba86d..2a25cb1b 100644 --- a/src/main/java/org/springframework/retry/interceptor/RetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/interceptor/RetryOperationsInterceptor.java @@ -137,7 +137,7 @@ public Object doWithRetry(RetryContext context) throws Exception { finally { RetryContext context = RetrySynchronizationManager.getContext(); if (context != null) { - context.removeAttribute("__proxy__"); + context.removeAttribute("___proxy___"); } } } From ff11b1b1fe7d68f836af38168197cf4db0841f30 Mon Sep 17 00:00:00 2001 From: Kim Jun Hyeong Date: Thu, 8 May 2025 04:00:26 +0900 Subject: [PATCH 2/5] Add tests for RetryOperationsInterceptor proxy attribute cleanup Signed-off-by: Kim Jun Hyeong --- .../RetryOperationsInterceptorTests.java | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java index 887d9027..487d0282 100644 --- a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java +++ b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java @@ -36,9 +36,11 @@ import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; import org.springframework.retry.RetryListener; +import org.springframework.retry.context.RetryContextSupport; import org.springframework.retry.listener.MethodInvocationRetryListenerSupport; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.policy.SimpleRetryPolicy; +import org.springframework.retry.support.RetrySynchronizationManager; import org.springframework.retry.support.RetryTemplate; import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -47,6 +49,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.junit.jupiter.api.Assertions.fail; /** * @author Dave Syer @@ -257,6 +260,70 @@ public Object proceed() { })).withMessageContaining("MethodInvocation"); } + @Test + public void testProxyAttributeCleanupWithCorrectKey() { + RetryContext directContext = new RetryContextSupport(null); + Object mockProxy = new Object(); + RetrySynchronizationManager.register(directContext); + directContext.setAttribute("___proxy___", mockProxy); + RetryContext retrievedContext = RetrySynchronizationManager.getContext(); + retrievedContext.removeAttribute("___proxy___"); + assertThat(RetrySynchronizationManager.getContext().getAttribute("___proxy___")).isNull(); + RetrySynchronizationManager.clear(); + } + + @Test + public void testProxyAttributeRemainWithWrongKey() { + RetryContext directContext = new RetryContextSupport(null); + Object mockProxy = new Object(); + RetrySynchronizationManager.register(directContext); + directContext.setAttribute("___proxy___", mockProxy); + RetryContext retrievedContext = RetrySynchronizationManager.getContext(); + retrievedContext.removeAttribute("__proxy__"); + Object remainingProxy = RetrySynchronizationManager.getContext().getAttribute("___proxy___"); + assertThat(remainingProxy).isNotNull(); + assertThat(remainingProxy).isSameAs(mockProxy); + RetrySynchronizationManager.clear(); + } + + @Test + public void testProxyAttributeCleanupEvenWhenIllegalStateExceptionThrown() { + RetryContext context = new RetryContextSupport(null); + Object mockProxy = new Object(); + RetrySynchronizationManager.register(context); + context.setAttribute("___proxy___", mockProxy); + assertThat(context.getAttribute("___proxy___")).isNotNull(); + assertThatIllegalStateException().isThrownBy(() -> this.interceptor.invoke(new MethodInvocation() { + @Override + public Method getMethod() { + return ClassUtils.getMethod(RetryOperationsInterceptorTests.class, + "testProxyAttributeCleanupEvenWhenIllegalStateExceptionThrown"); + } + + @Override + public Object[] getArguments() { + return new Object[0]; + } + + @Override + public Object proceed() { + return null; + } + + @Override + public Object getThis() { + return new Object(); + } + + @Override + public AccessibleObject getStaticPart() { + return null; + } + })).withMessageContaining("MethodInvocation"); + assertThat(context.getAttribute("___proxy___")).isNull(); + RetrySynchronizationManager.clear(); + } + public static interface Service { void service() throws Exception; From 606eaccc783cc07dc03b592df43a3dd03b96a231 Mon Sep 17 00:00:00 2001 From: Kim Jun Hyeong Date: Thu, 8 May 2025 04:03:27 +0900 Subject: [PATCH 3/5] Add author to RetryOperationsInterceptorTests Signed-off-by: Kim Jun Hyeong --- .../retry/interceptor/RetryOperationsInterceptorTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java index 487d0282..64379d5a 100644 --- a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java +++ b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java @@ -58,6 +58,7 @@ * @author Stéphane Nicoll * @author Henning Pöttker * @author Artem Bilan + * @author Kim Jun Hyeong */ public class RetryOperationsInterceptorTests { From 85ba0ddf1fee72898d0782bba5d3feff8c401fde Mon Sep 17 00:00:00 2001 From: Kim Jun Hyeong Date: Thu, 8 May 2025 04:28:31 +0900 Subject: [PATCH 4/5] Remove unused import fail Signed-off-by: Kim Jun Hyeong --- .../retry/interceptor/RetryOperationsInterceptorTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java index 64379d5a..106eea9d 100644 --- a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java +++ b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java @@ -49,7 +49,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; -import static org.junit.jupiter.api.Assertions.fail; /** * @author Dave Syer From be5bc042294c313c9e601b6c573562c4356a4130 Mon Sep 17 00:00:00 2001 From: Kim Jun Hyeong Date: Thu, 8 May 2025 17:18:01 +0900 Subject: [PATCH 5/5] Fix RetryOperationsInterceptorTests extract MethodInvocation and simplify mocking Signed-off-by: Kim Jun Hyeong --- .../RetryOperationsInterceptorTests.java | 38 +++++-------------- 1 file changed, 9 insertions(+), 29 deletions(-) diff --git a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java index 106eea9d..d86d3cc1 100644 --- a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java +++ b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2023 the original author or authors. + * Copyright 2006-2025 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. @@ -49,6 +49,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * @author Dave Syer @@ -287,39 +289,17 @@ public void testProxyAttributeRemainWithWrongKey() { } @Test - public void testProxyAttributeCleanupEvenWhenIllegalStateExceptionThrown() { + public void testProxyAttributeCleanupEvenWhenIllegalStateExceptionThrown() throws NoSuchMethodException { RetryContext context = new RetryContextSupport(null); Object mockProxy = new Object(); RetrySynchronizationManager.register(context); context.setAttribute("___proxy___", mockProxy); assertThat(context.getAttribute("___proxy___")).isNotNull(); - assertThatIllegalStateException().isThrownBy(() -> this.interceptor.invoke(new MethodInvocation() { - @Override - public Method getMethod() { - return ClassUtils.getMethod(RetryOperationsInterceptorTests.class, - "testProxyAttributeCleanupEvenWhenIllegalStateExceptionThrown"); - } - - @Override - public Object[] getArguments() { - return new Object[0]; - } - - @Override - public Object proceed() { - return null; - } - - @Override - public Object getThis() { - return new Object(); - } - - @Override - public AccessibleObject getStaticPart() { - return null; - } - })).withMessageContaining("MethodInvocation"); + MethodInvocation mockInvocation = mock(MethodInvocation.class); + Method testMethod = this.getClass().getMethod("testProxyAttributeCleanupEvenWhenIllegalStateExceptionThrown"); + when(mockInvocation.getMethod()).thenReturn(testMethod); + assertThatIllegalStateException().isThrownBy(() -> this.interceptor.invoke(mockInvocation)) + .withMessageContaining("MethodInvocation"); assertThat(context.getAttribute("___proxy___")).isNull(); RetrySynchronizationManager.clear(); }