From 58f3d0fce000cf849f4d9674820b4da7c75d59b1 Mon Sep 17 00:00:00 2001
From: Marcin Grzejszczak <mgrzejszczak@pivotal.io>
Date: Tue, 22 Aug 2017 12:52:35 +0200
Subject: [PATCH] Added fallback approach to try to create a JDK proxy when
 CGLIB one fails

without this change when a CGLIB proxy fails to be created an exception is thrown
with this change we're trying to save the situation by trying to create a JDK proxy. If that also won't work then we're throwing an exception.

fixes #684
---
 .../async/ExecutorBeanPostProcessor.java      | 29 ++++++--
 .../async/ExecutorBeanPostProcessorTests.java | 67 +++++++++++++++++++
 2 files changed, 91 insertions(+), 5 deletions(-)
 create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessorTests.java

diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessor.java
index a95b918b7..deb6373db 100644
--- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessor.java
+++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessor.java
@@ -22,6 +22,9 @@ import java.util.concurrent.Executor;
 
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.springframework.aop.framework.AopConfigException;
 import org.springframework.aop.framework.ProxyFactoryBean;
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
@@ -39,6 +42,8 @@ import org.springframework.util.ReflectionUtils;
  */
 class ExecutorBeanPostProcessor implements BeanPostProcessor {
 
+	private static final Log log = LogFactory.getLog(ExecutorBeanPostProcessor.class);
+
 	private final BeanFactory beanFactory;
 
 	ExecutorBeanPostProcessor(BeanFactory beanFactory) {
@@ -60,14 +65,28 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor {
 			boolean classFinal = Modifier.isFinal(bean.getClass().getModifiers());
 			boolean cglibProxy = !methodFinal && !classFinal;
 			Executor executor = (Executor) bean;
-			ProxyFactoryBean factory = new ProxyFactoryBean();
-			factory.setProxyTargetClass(cglibProxy);
-			factory.addAdvice(new ExecutorMethodInterceptor(executor, this.beanFactory));
-			factory.setTarget(bean);
-			return factory.getObject();
+			try {
+				return createProxy(bean, cglibProxy, executor);
+			} catch (AopConfigException e) {
+				if (cglibProxy) {
+					if (log.isDebugEnabled()) {
+						log.debug("Exception occurred while trying to create a proxy, falling back to JDK proxy", e);
+					}
+					return createProxy(bean, false, executor);
+				}
+				throw e;
+			}
 		}
 		return bean;
 	}
+
+	Object createProxy(Object bean, boolean cglibProxy, Executor executor) {
+		ProxyFactoryBean factory = new ProxyFactoryBean();
+		factory.setProxyTargetClass(cglibProxy);
+		factory.addAdvice(new ExecutorMethodInterceptor(executor, this.beanFactory));
+		factory.setTarget(bean);
+		return factory.getObject();
+	}
 }
 
 class ExecutorMethodInterceptor implements MethodInterceptor {
diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessorTests.java
new file mode 100644
index 000000000..11c42ed64
--- /dev/null
+++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessorTests.java
@@ -0,0 +1,67 @@
+package org.springframework.cloud.sleuth.instrument.async;
+
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.aop.framework.AopConfigException;
+import org.springframework.beans.factory.BeanFactory;
+import org.springframework.util.ClassUtils;
+
+import static org.assertj.core.api.BDDAssertions.then;
+import static org.assertj.core.api.BDDAssertions.thenThrownBy;
+
+/**
+ * @author Marcin Grzejszczak
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ExecutorBeanPostProcessorTests {
+
+	@Mock BeanFactory beanFactory;
+
+	@Test
+	public void should_create_a_cglib_proxy_by_default() throws Exception {
+		Object o = new ExecutorBeanPostProcessor(this.beanFactory)
+				.postProcessAfterInitialization(new Foo(), "foo");
+
+		then(o).isInstanceOf(Foo.class);
+		then(ClassUtils.isCglibProxy(o)).isTrue();
+	}
+
+	class Foo implements Executor {
+		@Override public void execute(Runnable command) {
+
+		}
+	}
+
+	@Test
+	public void should_create_jdk_proxy_when_cglib_fails_to_be_done() throws Exception {
+		ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor();
+
+		Object o = new ExecutorBeanPostProcessor(this.beanFactory)
+				.postProcessAfterInitialization(service, "foo");
+
+		then(o).isInstanceOf(ScheduledExecutorService.class);
+		then(ClassUtils.isCglibProxy(o)).isFalse();
+	}
+
+	@Test
+	public void should_throw_exception_when_it_is_not_possible_to_create_any_proxy() throws Exception {
+		ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor();
+		ExecutorBeanPostProcessor bpp = new ExecutorBeanPostProcessor(this.beanFactory) {
+			@Override Object createProxy(Object bean, boolean cglibProxy,
+					Executor executor) {
+				throw new AopConfigException("foo");
+			}
+		};
+
+		thenThrownBy(() -> bpp.postProcessAfterInitialization(service, "foo"))
+				.isInstanceOf(AopConfigException.class)
+				.hasMessage("foo");
+	}
+
+}
\ No newline at end of file