Fixes use manually registered bean not works (#73)

* Fixes use manually registered bean not works * clean beanDefinitionMap when application is ready
DanielLiu1123 · Dec 1, 2024 · 7161c96 · 7161c96
1 parent 409cf3a
commit 7161c96
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 34 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -1,5 +1,5 @@
 group=io.github.danielliu1123
-version=3.4.0-SNAPSHOT
+version=3.4.0.1-SNAPSHOT
 
 springBootVersion=3.4.0
 # https://docs.spring.io/spring-cloud-release/reference/index.html

diff --git a/...configure/src/main/java/io/github/danielliu1123/httpexchange/HttpClientBeanRegistrar.java b/...configure/src/main/java/io/github/danielliu1123/httpexchange/HttpClientBeanRegistrar.java
@@ -18,7 +18,6 @@
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider;
-import org.springframework.core.ResolvableType;
 import org.springframework.core.env.Environment;
 import org.springframework.core.type.AnnotationMetadata;
 import org.springframework.core.type.ClassMetadata;
@@ -36,7 +35,8 @@ class HttpClientBeanRegistrar {
     private final BeanDefinitionRegistry registry;
     private final Environment environment;
 
-    private Map<String, List<BeanDefinition>> classNameToBeanDefinitions;
+    private static final HashMap<BeanDefinitionRegistry, Map<Class<?>, List<BeanDefinition>>> beanDefinitionMap =
+            new HashMap<>();
 
     public HttpClientBeanRegistrar(BeanDefinitionRegistry registry, Environment environment) {
         this.registry = registry;
@@ -55,22 +55,20 @@ public void register(String... basePackages) {
 
     public void register(Class<?>... clients) {
         for (Class<?> client : clients) {
-            registerHttpClientBean(registry, client.getName());
+            registerHttpClientBean(registry, client);
         }
     }
 
     /**
      * Register HTTP client beans the specified class name.
      *
-     * @param registry  {@link BeanDefinitionRegistry}
-     * @param className class name of HTTP client interface
+     * @param registry {@link BeanDefinitionRegistry}
+     * @param clz      class name
      */
     @SneakyThrows
-    private void registerHttpClientBean(BeanDefinitionRegistry registry, String className) {
-        Class<?> clz = Class.forName(className);
-
+    private void registerHttpClientBean(BeanDefinitionRegistry registry, Class<?> clz) {
         if (!clz.isInterface()) {
-            throw new IllegalArgumentException(className + " is not an interface");
+            throw new IllegalArgumentException(clz.getName() + " is not an interface");
         }
 
         if (!isHttpExchangeInterface(clz)) {
@@ -81,39 +79,39 @@ private void registerHttpClientBean(BeanDefinitionRegistry registry, String clas
             throw new IllegalArgumentException("BeanDefinitionRegistry is not a DefaultListableBeanFactory");
         }
 
-        initClassNameToBeanDefinitions(bf);
+        addBeanDefinitionCache(bf);
 
-        if (hasManualRegistered(className)) {
+        if (hasManualRegistered(registry, clz)) {
             if (log.isDebugEnabled()) {
-                log.debug("HTTP client bean '{}' is already registered, skip auto registration", className);
+                log.debug("HTTP client bean '{}' is already registered, skip auto registration", clz.getName());
             }
             return;
         }
 
         HttpExchangeUtil.registerHttpExchangeBean(bf, environment, clz);
     }
 
-    private void initClassNameToBeanDefinitions(DefaultListableBeanFactory bf) {
-        if (classNameToBeanDefinitions == null) {
-            classNameToBeanDefinitions = new HashMap<>();
-            for (var beanDefinitionName : bf.getBeanDefinitionNames()) {
-                var beanDefinition = bf.getBeanDefinition(beanDefinitionName);
-                var type = beanDefinition.getResolvableType();
-                if (!ResolvableType.NONE.equalsType(type)) {
-                    Class<?> clz = type.resolve();
-                    if (clz != null) {
-                        classNameToBeanDefinitions
-                                .computeIfAbsent(clz.getName(), k -> new ArrayList<>())
-                                .add(beanDefinition);
-                    }
-                }
+    private static void addBeanDefinitionCache(DefaultListableBeanFactory bf) {
+        if (beanDefinitionMap.containsKey(bf)) {
+            return;
+        }
+        for (var beanDefinitionName : bf.getBeanDefinitionNames()) {
+            var beanDefinition = bf.getBeanDefinition(beanDefinitionName);
+            var clz = Util.getBeanDefinitionClass(beanDefinition);
+            if (clz != null) {
+                beanDefinitionMap
+                        .computeIfAbsent(bf, k -> new HashMap<>())
+                        .computeIfAbsent(clz, k -> new ArrayList<>())
+                        .add(beanDefinition);
             }
         }
     }
 
-    private boolean hasManualRegistered(String className) {
-        var bds = classNameToBeanDefinitions.getOrDefault(className, List.of());
-        return !bds.isEmpty();
+    private static boolean hasManualRegistered(BeanDefinitionRegistry registry, Class<?> clz) {
+        return !beanDefinitionMap
+                .getOrDefault(registry, Map.of())
+                .getOrDefault(clz, List.of())
+                .isEmpty();
     }
 
     private static ClassPathScanningCandidateComponentProvider getScanner() {
@@ -141,10 +139,15 @@ private void registerBeans4BasePackages(Collection<String> basePackages) {
         for (String pkg : basePackages) {
             Set<BeanDefinition> beanDefinitions = scanner.findCandidateComponents(pkg);
             for (BeanDefinition bd : beanDefinitions) {
-                if (bd.getBeanClassName() != null) {
-                    registerHttpClientBean(registry, bd.getBeanClassName());
+                var clz = Util.getBeanDefinitionClass(bd);
+                if (clz != null) {
+                    registerHttpClientBean(registry, clz);
                 }
             }
         }
     }
+
+    static void clearBeanDefinitionCache(BeanDefinitionRegistry registry) {
+        beanDefinitionMap.remove(registry); // Only used in startup phase
+    }
 }
diff --git a/...ure/src/main/java/io/github/danielliu1123/httpexchange/HttpExchangeAutoConfiguration.java b/...ure/src/main/java/io/github/danielliu1123/httpexchange/HttpExchangeAutoConfiguration.java
@@ -3,13 +3,16 @@
 import static io.github.danielliu1123.httpexchange.Checker.checkUnusedConfig;
 
 import org.springframework.beans.factory.DisposableBean;
+import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.boot.CommandLineRunner;
 import org.springframework.boot.autoconfigure.AutoConfiguration;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.boot.context.event.ApplicationReadyEvent;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.boot.web.client.RestClientCustomizer;
 import org.springframework.boot.web.client.RestTemplateCustomizer;
+import org.springframework.context.ApplicationListener;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Lazy;
 
@@ -21,7 +24,15 @@
 @AutoConfiguration
 @ConditionalOnProperty(prefix = HttpExchangeProperties.PREFIX, name = "enabled", matchIfMissing = true)
 @EnableConfigurationProperties(HttpExchangeProperties.class)
-public class HttpExchangeAutoConfiguration implements DisposableBean {
+public class HttpExchangeAutoConfiguration implements DisposableBean, ApplicationListener<ApplicationReadyEvent> {
+
+    @Override
+    public void onApplicationEvent(ApplicationReadyEvent event) {
+        var bf = event.getApplicationContext().getBeanFactory();
+        if (bf instanceof BeanDefinitionRegistry bdr) {
+            HttpClientBeanRegistrar.clearBeanDefinitionCache(bdr);
+        }
+    }
 
     @Bean
     static HttpClientBeanDefinitionRegistry httpClientBeanDefinitionRegistry() {

diff --git a/...ge-spring-boot-autoconfigure/src/main/java/io/github/danielliu1123/httpexchange/Util.java b/...ge-spring-boot-autoconfigure/src/main/java/io/github/danielliu1123/httpexchange/Util.java
@@ -1,13 +1,17 @@
 package io.github.danielliu1123.httpexchange;
 
+import jakarta.annotation.Nullable;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
 import lombok.experimental.UtilityClass;
+import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition;
+import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.boot.SpringBootVersion;
 import org.springframework.boot.context.properties.bind.Binder;
+import org.springframework.core.ResolvableType;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.core.env.Environment;
 import org.springframework.util.AntPathMatcher;
@@ -89,4 +93,42 @@ public static boolean hasAnnotation(Class<?> clz, Class<? extends Annotation> an
     public static boolean isSpringBootVersionLessThan340() {
         return SpringBootVersion.getVersion().compareTo("3.4.0") < 0;
     }
+
+    /**
+     * Get class of bean definition.
+     *
+     * @param beanDefinition bean definition
+     * @return class of bean definition
+     */
+    @Nullable
+    public static Class<?> getBeanDefinitionClass(BeanDefinition beanDefinition) {
+        // try to get class from factory method metadata
+        // @Configuration + @Bean
+        if (beanDefinition instanceof AnnotatedBeanDefinition abd) {
+            var metadata = abd.getFactoryMethodMetadata();
+            if (metadata != null) {
+                // Maybe there has @Conditional on the method,
+                // Class may not present.
+                return forName(metadata.getReturnTypeName());
+            }
+        }
+        var rt = beanDefinition.getResolvableType();
+        if (ResolvableType.NONE.equalsType(rt)) {
+            var beanClassName = beanDefinition.getBeanClassName();
+            if (beanClassName == null) {
+                return null;
+            }
+            return forName(beanClassName);
+        }
+        return rt.resolve();
+    }
+
+    @Nullable
+    public static Class<?> forName(String beanClassName) {
+        try {
+            return Class.forName(beanClassName);
+        } catch (ClassNotFoundException e) {
+            return null;
+        }
+    }
 }
diff --git a/...httpexchange/ManualRegisterBeanTests.java → ...tpexchange/RegisterBeanManuallyTests.java b/...httpexchange/ManualRegisterBeanTests.java → ...tpexchange/RegisterBeanManuallyTests.java
@@ -18,7 +18,7 @@
 import org.springframework.web.service.annotation.GetExchange;
 import org.springframework.web.service.invoker.HttpServiceProxyFactory;
 
-class ManualRegisterBeanTests {
+class RegisterBeanManuallyTests {
 
     @Test
     void useAutoRegisteredBean_whenNoManualRegisteredBean() {