Skip to content

Commit 4e83157

Browse files
committed
Refactor: use SpringJdbcDelegate inner class and ClassUtils.isPresent guard
Replace reflection-based chain walking with direct DelegatingDataSource and AbstractRoutingDataSource API calls. Isolate spring-jdbc references in a private inner class so the outer class loads cleanly when spring-jdbc is absent. ClassUtils.isPresent gates the delegate call, preserving the original wrap-everything behaviour for non-spring-jdbc environments. Promote spring-jdbc from test to optional scope to enable compile-time type checking in the inner class.
1 parent 343150b commit 4e83157

3 files changed

Lines changed: 114 additions & 70 deletions

File tree

datasource-micrometer-spring-boot/pom.xml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@
8282
<artifactId>HikariCP</artifactId>
8383
<optional>true</optional>
8484
</dependency>
85+
<dependency>
86+
<groupId>org.springframework</groupId>
87+
<artifactId>spring-jdbc</artifactId>
88+
<optional>true</optional>
89+
</dependency>
8590

8691
<!-- Integration test -->
8792
<dependency>
@@ -104,11 +109,6 @@
104109
<artifactId>spring-boot-micrometer-tracing-test</artifactId>
105110
<scope>test</scope>
106111
</dependency>
107-
<dependency>
108-
<groupId>org.springframework</groupId>
109-
<artifactId>spring-jdbc</artifactId>
110-
<scope>test</scope>
111-
</dependency>
112112
<dependency>
113113
<groupId>com.h2database</groupId>
114114
<artifactId>h2</artifactId>

datasource-micrometer-spring-boot/src/main/java/net/ttddyy/observation/boot/autoconfigure/DataSourceObservationBeanPostProcessor.java

Lines changed: 40 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,6 @@
1616

1717
package net.ttddyy.observation.boot.autoconfigure;
1818

19-
import java.lang.reflect.Field;
20-
import java.lang.reflect.Method;
21-
import java.util.Collections;
22-
import java.util.IdentityHashMap;
23-
import java.util.Map;
24-
import java.util.Set;
25-
2619
import javax.sql.DataSource;
2720

2821
import net.ttddyy.dsproxy.listener.MethodExecutionListener;
@@ -38,6 +31,9 @@
3831
import org.springframework.beans.BeansException;
3932
import org.springframework.beans.factory.ObjectProvider;
4033
import org.springframework.beans.factory.config.BeanPostProcessor;
34+
import org.springframework.jdbc.datasource.DelegatingDataSource;
35+
import org.springframework.jdbc.datasource.lookup.AbstractRoutingDataSource;
36+
import org.springframework.util.ClassUtils;
4137

4238
/**
4339
* A {@link BeanPostProcessor} to instrument {@link DataSource} beans.
@@ -46,11 +42,9 @@
4642
*/
4743
public class DataSourceObservationBeanPostProcessor implements BeanPostProcessor {
4844

49-
/**
50-
* Tracks datasource instances (pre-proxy) that this post-processor has already
51-
* instrumented, so routing/delegating wrappers referencing them can be skipped.
52-
*/
53-
private final Set<DataSource> proxiedDataSources = Collections.newSetFromMap(new IdentityHashMap<>());
45+
static final boolean SPRING_JDBC_PRESENT = ClassUtils.isPresent(
46+
"org.springframework.jdbc.datasource.DelegatingDataSource",
47+
DataSourceObservationBeanPostProcessor.class.getClassLoader());
5448

5549
private final ObjectProvider<JdbcProperties> jdbcPropertiesProvider;
5650

@@ -106,15 +100,12 @@ public Object postProcessAfterInitialization(Object bean, String beanName) throw
106100
this.proxyDataSourceBuilderCustomizers.orderedStream()
107101
.forEach(customizer -> customizer.customize(builder, dataSource, beanName, dataSourceName));
108102
DataSourceType dataSourceType = getJdbcProperties().getDatasourceProxy().getType();
109-
Object proxy;
110103
if (dataSourceType == DataSourceType.PROXY || dataSourceType == DataSourceType.SPRING_PROXY) {
111-
proxy = builder.buildProxy();
104+
return builder.buildProxy();
112105
}
113106
else {
114-
proxy = builder.build();
107+
return builder.build();
115108
}
116-
this.proxiedDataSources.add(dataSource);
117-
return proxy;
118109
}
119110
else {
120111
return bean;
@@ -123,60 +114,18 @@ public Object postProcessAfterInitialization(Object bean, String beanName) throw
123114

124115
/**
125116
* Returns {@code true} if {@code ds} is a routing or delegating wrapper whose chain
126-
* contains a datasource that this post-processor has already instrumented. Detected
127-
* via reflection to avoid a compile-time dependency on spring-jdbc.
117+
* contains a datasource already instrumented by datasource-proxy.
128118
*
129-
* <p>Two common patterns are checked:
130-
* <ul>
131-
* <li>{@code getTargetDataSource()} — covers {@code DelegatingDataSource} (e.g.
132-
* {@code LazyConnectionDataSourceProxy})</li>
133-
* <li>{@code resolvedDataSources} field — covers {@code AbstractRoutingDataSource}
134-
* subclasses</li>
135-
* </ul>
119+
* <p>When {@code spring-jdbc} is absent the check is skipped and returns {@code false},
120+
* preserving the pre-existing behaviour of wrapping all datasource beans.
136121
*/
137-
private boolean containsAlreadyProxiedTarget(DataSource ds) {
138-
// DelegatingDataSource pattern: getTargetDataSource()
139-
try {
140-
Method m = ds.getClass().getMethod("getTargetDataSource");
141-
Object target = m.invoke(ds);
142-
if (target instanceof DataSource targetDs) {
143-
return this.proxiedDataSources.contains(targetDs) || isProxyJdbcObject(targetDs)
144-
|| containsAlreadyProxiedTarget(targetDs);
145-
}
146-
}
147-
catch (ReflectiveOperationException ignored) {
148-
}
149-
150-
// AbstractRoutingDataSource pattern: resolvedDataSources field
151-
Class<?> cls = ds.getClass();
152-
while (cls != null && cls != Object.class) {
153-
try {
154-
Field f = cls.getDeclaredField("resolvedDataSources");
155-
f.setAccessible(true);
156-
@SuppressWarnings("unchecked")
157-
Map<Object, DataSource> resolved = (Map<Object, DataSource>) f.get(ds);
158-
if (resolved != null) {
159-
return resolved.values()
160-
.stream()
161-
.anyMatch(t -> this.proxiedDataSources.contains(t) || isProxyJdbcObject(t)
162-
|| containsAlreadyProxiedTarget(t));
163-
}
164-
break;
165-
}
166-
catch (NoSuchFieldException e) {
167-
cls = cls.getSuperclass();
168-
}
169-
catch (IllegalAccessException ignored) {
170-
break;
171-
}
122+
private static boolean containsAlreadyProxiedTarget(DataSource ds) {
123+
if (SPRING_JDBC_PRESENT) {
124+
return SpringJdbcDelegate.containsProxiedTarget(ds);
172125
}
173126
return false;
174127
}
175128

176-
private static boolean isProxyJdbcObject(DataSource ds) {
177-
return ds instanceof ProxyJdbcObject;
178-
}
179-
180129
private DataSourceProxyBuilderConfigurer getConfigurer() {
181130
if (this.dataSourceProxyBuilderConfigurer == null) {
182131
this.dataSourceProxyBuilderConfigurer = new DataSourceProxyBuilderConfigurer(getJdbcProperties(),
@@ -198,4 +147,30 @@ private JdbcProperties getJdbcProperties() {
198147
return this.jdbcPropertiesProvider.getObject();
199148
}
200149

150+
/**
151+
* Isolated in a separate class so that the JVM only loads it when
152+
* {@code spring-jdbc} is actually on the classpath. If it were inlined in the
153+
* outer class, the mere presence of {@link DelegatingDataSource} and
154+
* {@link AbstractRoutingDataSource} in the constant pool would cause a
155+
* {@link NoClassDefFoundError} at class-load time when {@code spring-jdbc} is absent.
156+
*/
157+
private static final class SpringJdbcDelegate {
158+
159+
static boolean containsProxiedTarget(final DataSource ds) {
160+
if (ds instanceof DelegatingDataSource delegating) {
161+
final DataSource target = delegating.getTargetDataSource();
162+
return target != null
163+
&& (target instanceof ProxyJdbcObject || containsProxiedTarget(target));
164+
}
165+
if (ds instanceof AbstractRoutingDataSource routing) {
166+
return routing.getResolvedDataSources()
167+
.values()
168+
.stream()
169+
.anyMatch(target -> target instanceof ProxyJdbcObject || containsProxiedTarget(target));
170+
}
171+
return false;
172+
}
173+
174+
}
175+
201176
}

datasource-micrometer-spring-boot/src/test/java/net/ttddyy/observation/boot/autoconfigure/DataSourceObservationBeanPostProcessorTests.java

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@
1616

1717
package net.ttddyy.observation.boot.autoconfigure;
1818

19+
import java.io.File;
20+
import java.net.URL;
21+
import java.net.URLClassLoader;
1922
import java.sql.Connection;
23+
import java.util.Arrays;
2024
import java.util.Map;
2125
import java.util.Set;
2226
import java.util.stream.Stream;
@@ -189,6 +193,46 @@ else if (type == DataSourceType.CONCRETE) {
189193
}
190194
}
191195

196+
@Test
197+
void springJdbcPresentIsTrueWhenSpringJdbcOnClasspath() {
198+
// Package-private access — no reflection needed
199+
assertThat(DataSourceObservationBeanPostProcessor.SPRING_JDBC_PRESENT).isTrue();
200+
}
201+
202+
@Test
203+
void springJdbcPresentIsFalseWhenSpringJdbcAbsent() throws Exception {
204+
// Build URLs from the current classpath so we can reload the class fresh
205+
URL[] urls = Arrays.stream(System.getProperty("java.class.path").split(File.pathSeparator))
206+
.map(entry -> {
207+
try {
208+
return new File(entry).toURI().toURL();
209+
}
210+
catch (Exception ex) {
211+
throw new RuntimeException(ex);
212+
}
213+
})
214+
.toArray(URL[]::new);
215+
216+
// null parent → bootstrap classloader only; our URLs cover the rest.
217+
// The filter makes spring-jdbc invisible so ClassUtils.isPresent returns false
218+
// and SPRING_JDBC_PRESENT is set to false when the class is loaded fresh.
219+
try (URLClassLoader isolatedLoader = new URLClassLoader(urls, null) {
220+
@Override
221+
public Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
222+
if (name.startsWith("org.springframework.jdbc.datasource")) {
223+
throw new ClassNotFoundException(name);
224+
}
225+
return super.loadClass(name, resolve);
226+
}
227+
}) {
228+
Class<?> processorClass = isolatedLoader
229+
.loadClass("net.ttddyy.observation.boot.autoconfigure.DataSourceObservationBeanPostProcessor");
230+
java.lang.reflect.Field field = processorClass.getDeclaredField("SPRING_JDBC_PRESENT");
231+
field.setAccessible(true);
232+
assertThat(field.get(null)).isEqualTo(false);
233+
}
234+
}
235+
192236
@Test
193237
void delegatingDataSourceWrappingAlreadyProxiedTargetIsSkipped() throws Exception {
194238
setupProcessorForProxying();
@@ -278,6 +322,31 @@ void independentDataSourceIsStillProxied() throws Exception {
278322
assertThat(secondResult).isInstanceOf(ProxyJdbcObject.class);
279323
}
280324

325+
@Test
326+
void routingDatasourceWithUnproxiedTargetsIsStillProxied() throws Exception {
327+
// When targets are plain (not ProxyJdbcObject), the routing wrapper itself
328+
// must be proxied. This covers the fallback when spring-jdbc is absent
329+
// (SPRING_JDBC_PRESENT=false → containsAlreadyProxiedTarget always returns false)
330+
// and the normal case where the routing datasource is processed before its targets.
331+
setupProcessorForProxying();
332+
333+
final DataSource physicalRw = mockPhysicalDataSource();
334+
final DataSource physicalRo = mockPhysicalDataSource();
335+
336+
final AbstractRoutingDataSource router = new AbstractRoutingDataSource() {
337+
@Override
338+
protected Object determineCurrentLookupKey() {
339+
return "rw";
340+
}
341+
};
342+
router.setTargetDataSources(Map.of("rw", physicalRw, "ro", physicalRo));
343+
router.setDefaultTargetDataSource(physicalRw);
344+
router.afterPropertiesSet();
345+
346+
final Object result = this.processor.postProcessAfterInitialization(router, "actualDataSource");
347+
assertThat(result).isInstanceOf(ProxyJdbcObject.class);
348+
}
349+
281350
private void setupProcessorForProxying() {
282351
JdbcProperties jdbcProperties = new JdbcProperties();
283352
given(this.jdbcPropertiesProvider.getObject()).willReturn(jdbcProperties);

0 commit comments

Comments
 (0)