From abbee1a0509ed78222899e782be060363ac001df Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 29 Apr 2025 19:51:58 -0700 Subject: [PATCH] Use bean class loader when creating interface clients Update interface client code to replace `GroupsMetadata.loadClass` calls with `ClassUtils.resolveClassName` passing in the bean class loader. Since the bean class loader in injected after construction, some minor refactoring has been applied to `HttpServiceProxyRegistryFactoryBean`. The class now stores `GroupsMetadata` and only loads the types in in `afterPropertiesSet`. The `HttpServiceProxyFactory` class has also been updated to ensure that the proxy is created using the class loader of the service type, rather than the thread context class loader. Fixes gh-34846 --- .../invoker/HttpServiceProxyFactory.java | 9 +++- .../AbstractHttpServiceRegistrar.java | 13 ++++- .../web/service/registry/GroupsMetadata.java | 24 ++++----- .../HttpServiceProxyRegistryFactoryBean.java | 54 +++++++++++++------ ...stClientProxyRegistryIntegrationTests.java | 19 +++++++ .../GroupsMetadataValueDelegateTests.java | 8 +-- .../registry/HttpServiceRegistrarTests.java | 2 +- 7 files changed, 93 insertions(+), 36 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java index ab492b6d37..37cdf52676 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java +++ b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpServiceProxyFactory.java @@ -89,7 +89,14 @@ public final class HttpServiceProxyFactory { .map(method -> createHttpServiceMethod(serviceType, method)) .toList(); - return ProxyFactory.getProxy(serviceType, new HttpServiceMethodInterceptor(httpServiceMethods)); + return getProxy(serviceType, httpServiceMethods); + } + + @SuppressWarnings("unchecked") + private S getProxy(Class serviceType, List httpServiceMethods) { + MethodInterceptor interceptor = new HttpServiceMethodInterceptor(httpServiceMethods); + ProxyFactory proxyFactory = new ProxyFactory(serviceType, interceptor); + return (S) proxyFactory.getProxy(serviceType.getClassLoader()); } private boolean isExchangeMethod(Method method) { diff --git a/spring-web/src/main/java/org/springframework/web/service/registry/AbstractHttpServiceRegistrar.java b/spring-web/src/main/java/org/springframework/web/service/registry/AbstractHttpServiceRegistrar.java index 1683df7fbf..edf58b1750 100644 --- a/spring-web/src/main/java/org/springframework/web/service/registry/AbstractHttpServiceRegistrar.java +++ b/spring-web/src/main/java/org/springframework/web/service/registry/AbstractHttpServiceRegistrar.java @@ -19,6 +19,7 @@ package org.springframework.web.service.registry; import org.jspecify.annotations.Nullable; import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition; @@ -38,6 +39,7 @@ import org.springframework.core.type.MethodMetadata; import org.springframework.core.type.classreading.MetadataReader; import org.springframework.core.type.filter.AnnotationTypeFilter; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.web.service.annotation.HttpExchange; /** @@ -74,7 +76,7 @@ import org.springframework.web.service.annotation.HttpExchange; * @see HttpServiceProxyRegistryFactoryBean */ public abstract class AbstractHttpServiceRegistrar implements - ImportBeanDefinitionRegistrar, EnvironmentAware, ResourceLoaderAware, BeanFactoryAware { + ImportBeanDefinitionRegistrar, EnvironmentAware, ResourceLoaderAware, BeanFactoryAware, BeanClassLoaderAware { /** * The bean name of the {@link HttpServiceProxyRegistry}. @@ -91,6 +93,8 @@ public abstract class AbstractHttpServiceRegistrar implements private @Nullable BeanFactory beanFactory; + private @Nullable ClassLoader beanClassLoader; + private final GroupsMetadata groupsMetadata = new GroupsMetadata(); private @Nullable ClassPathScanningCandidateComponentProvider scanner; @@ -121,6 +125,11 @@ public abstract class AbstractHttpServiceRegistrar implements this.beanFactory = beanFactory; } + @Override + public void setBeanClassLoader(ClassLoader beanClassLoader) { + this.beanClassLoader = beanClassLoader; + } + @Override public final void registerBeanDefinitions( @@ -197,7 +206,7 @@ public abstract class AbstractHttpServiceRegistrar implements private Object getProxyInstance(String groupName, String httpServiceType) { Assert.state(this.beanFactory != null, "BeanFactory has not been set"); HttpServiceProxyRegistry registry = this.beanFactory.getBean(HTTP_SERVICE_PROXY_REGISTRY_BEAN_NAME, HttpServiceProxyRegistry.class); - return registry.getClient(groupName, GroupsMetadata.loadClass(httpServiceType)); + return registry.getClient(groupName, ClassUtils.resolveClassName(httpServiceType, this.beanClassLoader)); } diff --git a/spring-web/src/main/java/org/springframework/web/service/registry/GroupsMetadata.java b/spring-web/src/main/java/org/springframework/web/service/registry/GroupsMetadata.java index d3cad9c0ae..2822d048dc 100644 --- a/spring-web/src/main/java/org/springframework/web/service/registry/GroupsMetadata.java +++ b/spring-web/src/main/java/org/springframework/web/service/registry/GroupsMetadata.java @@ -26,6 +26,8 @@ import java.util.function.BiConsumer; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.jspecify.annotations.Nullable; + import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -82,17 +84,11 @@ final class GroupsMetadata { /** * Create the {@link HttpServiceGroup}s for all registrations. */ - public Collection groups() { - return this.groupMap.values().stream().map(DefaultRegistration::toHttpServiceGroup).toList(); - } - - public static Class loadClass(String type) { - try { - return ClassUtils.forName(type, GroupsMetadata.class.getClassLoader()); - } - catch (ClassNotFoundException ex) { - throw new IllegalStateException("Failed to load '" + type + "'", ex); - } + public Collection groups(@Nullable ClassLoader classLoader) { + return this.groupMap.values() + .stream() + .map(registration -> registration.toHttpServiceGroup(classLoader)) + .toList(); } /** @@ -169,10 +165,12 @@ final class GroupsMetadata { /** * Create the {@link HttpServiceGroup} from the metadata. */ - public HttpServiceGroup toHttpServiceGroup() { + public HttpServiceGroup toHttpServiceGroup(@Nullable ClassLoader classLoader) { return new RegisteredGroup(this.name, (this.clientType.isUnspecified() ? HttpServiceGroup.ClientType.REST_CLIENT : this.clientType), - this.typeNames.stream().map(GroupsMetadata::loadClass).collect(Collectors.toSet())); + this.typeNames.stream() + .map(typeName -> ClassUtils.resolveClassName(typeName, classLoader)) + .collect(Collectors.toSet())); } @Override diff --git a/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java b/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java index fcfc13f40a..408e3e6ff8 100644 --- a/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java +++ b/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java @@ -31,6 +31,7 @@ import org.jspecify.annotations.Nullable; import org.springframework.beans.BeanUtils; import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.context.ApplicationContext; @@ -57,27 +58,24 @@ import org.springframework.web.service.invoker.HttpServiceProxyFactory; * @see AbstractHttpServiceRegistrar */ public final class HttpServiceProxyRegistryFactoryBean - implements ApplicationContextAware, InitializingBean, FactoryBean { + implements ApplicationContextAware, BeanClassLoaderAware, InitializingBean, + FactoryBean { private static final Map> groupAdapters = GroupAdapterInitializer.initGroupAdapters(); - private final Set groupSet; + private final GroupsMetadata groupsMetadata; private @Nullable ApplicationContext applicationContext; + private @Nullable ClassLoader beanClassLoader; + private @Nullable HttpServiceProxyRegistry proxyRegistry; HttpServiceProxyRegistryFactoryBean(GroupsMetadata groupsMetadata) { - this.groupSet = groupsMetadata.groups().stream() - .map(group -> { - HttpServiceGroupAdapter adapter = groupAdapters.get(group.clientType()); - Assert.state(adapter != null, "No HttpServiceGroupAdapter for type " + group.clientType()); - return new ProxyHttpServiceGroup(group, adapter); - }) - .collect(Collectors.toSet()); + this.groupsMetadata = groupsMetadata; } @@ -86,6 +84,11 @@ public final class HttpServiceProxyRegistryFactoryBean this.applicationContext = applicationContext; } + @Override + public void setBeanClassLoader(ClassLoader beanClassLoader) { + this.beanClassLoader = beanClassLoader; + } + @Override public Class getObjectType() { return HttpServiceProxyRegistry.class; @@ -95,18 +98,25 @@ public final class HttpServiceProxyRegistryFactoryBean @Override public void afterPropertiesSet() { Assert.notNull(this.applicationContext, "ApplicationContext not initialized"); + Assert.notNull(this.beanClassLoader, "BeanClassLoader not initialized"); + + // Create the groups from the metadata + Set groups = this.groupsMetadata.groups(this.beanClassLoader) + .stream() + .map(ProxyHttpServiceGroup::new) + .collect(Collectors.toSet()); // Apply group configurers groupAdapters.forEach((clientType, groupAdapter) -> this.applicationContext.getBeanProvider(groupAdapter.getConfigurerType()) .orderedStream() - .forEach(configurer -> configurer.configureGroups(new DefaultGroups<>(clientType)))); + .forEach(configurer -> configurer.configureGroups(new DefaultGroups<>(groups, clientType)))); // Create proxies - Map, Object>> groupProxyMap = this.groupSet.stream() + Map, Object>> proxies = groups.stream() .collect(Collectors.toMap(ProxyHttpServiceGroup::name, ProxyHttpServiceGroup::createProxies)); - this.proxyRegistry = new DefaultHttpServiceProxyRegistry(groupProxyMap); + this.proxyRegistry = new DefaultHttpServiceProxyRegistry(proxies); } @@ -159,12 +169,17 @@ public final class HttpServiceProxyRegistryFactoryBean private BiConsumer proxyFactoryConfigurer = (group, builder) -> {}; + ProxyHttpServiceGroup(HttpServiceGroup group) { + this(group, getHttpServiceGroupAdapter(group.clientType())); + } + ProxyHttpServiceGroup(HttpServiceGroup group, HttpServiceGroupAdapter groupAdapter) { this.declaredGroup = group; this.groupAdapter = groupAdapter; this.clientBuilder = groupAdapter.createClientBuilder(); } + @Override public String name() { return this.declaredGroup.name(); @@ -208,17 +223,26 @@ public final class HttpServiceProxyRegistryFactoryBean public String toString() { return getClass().getSimpleName() + "[id=" + name() + "]"; } + + private static HttpServiceGroupAdapter getHttpServiceGroupAdapter(HttpServiceGroup.ClientType clientType) { + HttpServiceGroupAdapter adapter = groupAdapters.get(clientType); + Assert.state(adapter != null, "No HttpServiceGroupAdapter for type " + clientType); + return adapter; + } } /** * Default implementation of Groups that helps to configure the set of declared groups. */ - private final class DefaultGroups implements HttpServiceGroupConfigurer.Groups { + private static final class DefaultGroups implements HttpServiceGroupConfigurer.Groups { + + private final Set groups; private Predicate filter; - DefaultGroups(HttpServiceGroup.ClientType clientType) { + DefaultGroups(Set groups, HttpServiceGroup.ClientType clientType) { + this.groups = groups; this.filter = group -> group.clientType().equals(clientType); } @@ -255,7 +279,7 @@ public final class HttpServiceProxyRegistryFactoryBean BiConsumer clientConfigurer, BiConsumer proxyFactoryConfigurer) { - groupSet.stream().filter(this.filter).forEach(group -> + this.groups.stream().filter(this.filter).forEach(group -> group.apply(clientConfigurer, proxyFactoryConfigurer)); } } diff --git a/spring-web/src/test/java/org/springframework/web/client/support/RestClientProxyRegistryIntegrationTests.java b/spring-web/src/test/java/org/springframework/web/client/support/RestClientProxyRegistryIntegrationTests.java index fde053c11d..5c01a2efd4 100644 --- a/spring-web/src/test/java/org/springframework/web/client/support/RestClientProxyRegistryIntegrationTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/support/RestClientProxyRegistryIntegrationTests.java @@ -23,6 +23,7 @@ import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -30,7 +31,9 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.core.OverridingClassLoader; import org.springframework.core.type.AnnotationMetadata; +import org.springframework.util.ClassUtils; import org.springframework.web.service.registry.AbstractHttpServiceRegistrar; import org.springframework.web.service.registry.HttpServiceProxyRegistry; import org.springframework.web.service.registry.ImportHttpServices; @@ -112,6 +115,22 @@ public class RestClientProxyRegistryIntegrationTests { assertThat(request.getPath()).isEqualTo("/greetingB?input=b"); } + @Test + void beansAreCreatedUsingBeanClassLoader() { + ClassLoader beanClassLoader = new OverridingClassLoader(getClass().getClassLoader()) { + + protected boolean isEligibleForOverriding(String className) { + return className.contains("EchoA"); + }; + }; + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.setClassLoader(beanClassLoader); + context.register(ClassUtils.resolveClassName(ListingConfig.class.getName(), beanClassLoader)); + context.refresh(); + assertThat(context.getBean(ClassUtils.resolveClassName(EchoA.class.getName(), beanClassLoader)) + .getClass() + .getClassLoader()).isSameAs(beanClassLoader); + } private static class ClientConfig { diff --git a/spring-web/src/test/java/org/springframework/web/service/registry/GroupsMetadataValueDelegateTests.java b/spring-web/src/test/java/org/springframework/web/service/registry/GroupsMetadataValueDelegateTests.java index 944964a469..cca4c5bee9 100644 --- a/spring-web/src/test/java/org/springframework/web/service/registry/GroupsMetadataValueDelegateTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/registry/GroupsMetadataValueDelegateTests.java @@ -92,7 +92,7 @@ class GroupsMetadataValueDelegateTests { @Test void generateGroupsMetadataEmpty() { compile(new GroupsMetadata(), (instance, compiled) -> assertThat(instance) - .isInstanceOfSatisfying(GroupsMetadata.class, metadata -> assertThat(metadata.groups()).isEmpty())); + .isInstanceOfSatisfying(GroupsMetadata.class, metadata -> assertThat(metadata.groups(compiled.getClassLoader())).isEmpty())); } @Test @@ -100,7 +100,7 @@ class GroupsMetadataValueDelegateTests { GroupsMetadata groupsMetadata = new GroupsMetadata(); groupsMetadata.getOrCreateGroup("test-group", ClientType.REST_CLIENT).httpServiceTypeNames().add(EchoA.class.getName()); compile(groupsMetadata, (instance, compiled) -> assertThat(instance) - .isInstanceOfSatisfying(GroupsMetadata.class, metadata -> assertThat(metadata.groups()) + .isInstanceOfSatisfying(GroupsMetadata.class, metadata -> assertThat(metadata.groups(compiled.getClassLoader())) .singleElement().satisfies(hasHttpServiceGroup("test-group", ClientType.REST_CLIENT, EchoA.class)))); } @@ -115,7 +115,7 @@ class GroupsMetadataValueDelegateTests { Function valueCodeGeneratorFactory = generatedClass -> ValueCodeGenerator.withDefaults().add(List.of(new GroupsMetadataValueDelegate())); compile(valueCodeGeneratorFactory, groupsMetadata, (instance, compiled) -> assertThat(instance) - .isInstanceOfSatisfying(GroupsMetadata.class, metadata -> assertThat(metadata.groups()) + .isInstanceOfSatisfying(GroupsMetadata.class, metadata -> assertThat(metadata.groups(compiled.getClassLoader())) .satisfiesOnlyOnce(hasHttpServiceGroup("test-group", ClientType.REST_CLIENT, EchoA.class, EchoB.class)) .satisfiesOnlyOnce(hasHttpServiceGroup("another-group", ClientType.WEB_CLIENT, GreetingA.class, GreetingB.class)) .hasSize(2))); @@ -130,7 +130,7 @@ class GroupsMetadataValueDelegateTests { .addAll(List.of(GreetingA.class.getName(), GreetingB.class.getName())); compile(groupsMetadata, (instance, compiled) -> assertThat(instance) - .isInstanceOfSatisfying(GroupsMetadata.class, metadata -> assertThat(metadata.groups()) + .isInstanceOfSatisfying(GroupsMetadata.class, metadata -> assertThat(metadata.groups(compiled.getClassLoader())) .satisfiesOnlyOnce(hasHttpServiceGroup("test-group", ClientType.REST_CLIENT, EchoA.class, EchoB.class)) .satisfiesOnlyOnce(hasHttpServiceGroup("another-group", ClientType.WEB_CLIENT, GreetingA.class, GreetingB.class)) .hasSize(2))); diff --git a/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceRegistrarTests.java b/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceRegistrarTests.java index 2b208dd045..d48d8e5b0a 100644 --- a/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceRegistrarTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceRegistrarTests.java @@ -159,7 +159,7 @@ public class HttpServiceRegistrarTests { GroupsMetadata metadata = (GroupsMetadata) valueHolder.getValue(); assertThat(metadata).isNotNull(); - return metadata.groups().stream() + return metadata.groups(null).stream() .collect(Collectors.toMap(HttpServiceGroup::name, Function.identity())); }