From b8a62c8caa3651321adc595f5e254d249c523a8e Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Thu, 13 Feb 2025 12:48:06 +0100 Subject: [PATCH] GH-108 - Simplify collection of backing bean lists for PluginRegistry instances. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now use the API newly introduced in Spring Framework 6.2.3 / 7.0 M2 to select filtered bean instances from an ObjectProvider (via ….stream(Predicate> filter)). This allows us to avoid having to proxy a List as the ultimate target for the plugin registry. Instead, we can switch to a simple Supplier (SingletonSupplier in particular) to delay the actual bean lookup. String first pass of refactorings and reducing the exposure of intermediate types previously necessary to also be able to handle the ability to inject lists of beans (see GH-107). Also remove the previously AOT runtime hints registration. --- .../plugin/core/OrderAwarePluginRegistry.java | 95 ++------ .../plugin/core/PluginRegistrySupport.java | 44 ++-- .../plugin/core/SimplePluginRegistry.java | 6 +- .../core/aot/PluginRegistryRuntimeHints.java | 46 ---- ...uginRegistriesBeanDefinitionRegistrar.java | 4 +- .../support/AbstractTypeAwareSupport.java | 227 ------------------ .../support/PluginRegistryFactoryBean.java | 65 ++++- .../resources/META-INF/spring/aot.factories | 2 - 8 files changed, 101 insertions(+), 388 deletions(-) delete mode 100644 core/src/main/java/org/springframework/plugin/core/aot/PluginRegistryRuntimeHints.java delete mode 100644 core/src/main/java/org/springframework/plugin/core/support/AbstractTypeAwareSupport.java delete mode 100644 core/src/main/resources/META-INF/spring/aot.factories diff --git a/core/src/main/java/org/springframework/plugin/core/OrderAwarePluginRegistry.java b/core/src/main/java/org/springframework/plugin/core/OrderAwarePluginRegistry.java index bceb87a..4e02759 100644 --- a/core/src/main/java/org/springframework/plugin/core/OrderAwarePluginRegistry.java +++ b/core/src/main/java/org/springframework/plugin/core/OrderAwarePluginRegistry.java @@ -20,9 +20,11 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.function.Supplier; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.util.Assert; +import org.springframework.util.function.SingletonSupplier; /** * {@link PluginRegistry} implementation that be made aware of a certain ordering of {@link Plugin}s. By default it @@ -55,9 +57,15 @@ public class OrderAwarePluginRegistry, S> extends SimplePlug * @param comparator the {@link Comparator} to be used for ordering the {@link Plugin}s or {@literal null} if the * {@code #DEFAULT_COMPARATOR} shall be used. */ - protected OrderAwarePluginRegistry(List plugins, Comparator comparator) { + protected OrderAwarePluginRegistry(Supplier> plugins, Comparator comparator) { - super(plugins); + super(SingletonSupplier.of(() -> { + + var result = new ArrayList<>(plugins.get()); + Collections.sort(result, comparator); + + return result; + })); Assert.notNull(comparator, "Comparator must not be null!"); @@ -71,7 +79,7 @@ public class OrderAwarePluginRegistry, S> extends SimplePlug * @since 2.0 */ public static > OrderAwarePluginRegistry empty() { - return create(Collections.emptyList()); + return of(Collections.emptyList()); } /** @@ -124,6 +132,10 @@ public class OrderAwarePluginRegistry, S> extends SimplePlug return of(plugins, DEFAULT_REVERSE_COMPARATOR); } + public static > OrderAwarePluginRegistry ofReverse(Supplier> plugins) { + return of(plugins, DEFAULT_REVERSE_COMPARATOR); + } + /** * Creates a new {@link OrderAwarePluginRegistry} with the given plugins. * @@ -137,85 +149,16 @@ public class OrderAwarePluginRegistry, S> extends SimplePlug Assert.notNull(plugins, "Plugins must not be null!"); Assert.notNull(comparator, "Comparator must not be null!"); - return new OrderAwarePluginRegistry<>(plugins, comparator); + return of(() -> plugins, comparator); } - /** - * Creates a new {@link OrderAwarePluginRegistry} using the {@code #DEFAULT_COMPARATOR}. - * - * @return - * @deprecated since 2.0, for removal in 2.1. Prefer {@link PluginRegistry#empty()}. - */ - @Deprecated - public static > OrderAwarePluginRegistry create() { - return empty(); - } - - /** - * Creates a new {@link OrderAwarePluginRegistry} using the given {@link Comparator} for ordering contained - * {@link Plugin}s. - * - * @param comparator must not be {@literal null}. - * @return - * @deprecated since 2.0, for removal in 2.1. Prefer {@link PluginRegistry#of(Comparator)}. - */ - @Deprecated - public static > OrderAwarePluginRegistry create(Comparator comparator) { - - Assert.notNull(comparator, "Comparator must not be null!"); - - return of(Collections.emptyList(), comparator); - } - - /** - * Creates a new {@link OrderAwarePluginRegistry} with the given plugins. - * - * @param plugins must not be {@literal null}. - * @return - * @deprecated since 2.0, for removal in 2.1. Prefer {@link PluginRegistry#of(List)}. - */ - @Deprecated - public static > OrderAwarePluginRegistry create(List plugins) { + public static > OrderAwarePluginRegistry of(Supplier> plugins) { return of(plugins, DEFAULT_COMPARATOR); } - /** - * Creates a new {@link OrderAwarePluginRegistry} with the given {@link Plugin}s and the order of the {@link Plugin}s - * reverted. - * - * @param plugins must not be {@literal null}. - * @return - * @deprecated since 2.0, for removal in 2.1. Prefer {@link OrderAwarePluginRegistry#ofReverse(List)} - */ - @Deprecated - public static > OrderAwarePluginRegistry createReverse(List plugins) { - return of(plugins, DEFAULT_REVERSE_COMPARATOR); - } - - /** - * Creates a new {@link OrderAwarePluginRegistry} with the given plugins. - * - * @param plugins must not be {@literal null}. - * @return - * @deprecated since 2.0, for removal in 2.1. Prefer {@link PluginRegistry#of(List, Comparator)}. - */ - @Deprecated - public static > OrderAwarePluginRegistry create(List plugins, + public static > OrderAwarePluginRegistry of(Supplier> plugins, Comparator comparator) { - - return of(plugins, comparator); - } - - /* - * (non-Javadoc) - * @see org.springframework.plugin.core.PluginRegistrySupport#initialize(java.util.List) - */ - @Override - protected List initialize(List plugins) { - - List result = super.initialize(plugins); - Collections.sort(result, comparator); - return result; + return new OrderAwarePluginRegistry<>(plugins, comparator); } /** diff --git a/core/src/main/java/org/springframework/plugin/core/PluginRegistrySupport.java b/core/src/main/java/org/springframework/plugin/core/PluginRegistrySupport.java index fb73a27..d159716 100644 --- a/core/src/main/java/org/springframework/plugin/core/PluginRegistrySupport.java +++ b/core/src/main/java/org/springframework/plugin/core/PluginRegistrySupport.java @@ -15,12 +15,12 @@ */ package org.springframework.plugin.core; -import java.util.ArrayList; import java.util.Iterator; import java.util.List; -import java.util.stream.Collectors; +import java.util.function.Supplier; import org.springframework.util.Assert; +import org.springframework.util.function.SingletonSupplier; /** * Base class for {@link PluginRegistry} implementations. Implements an initialization mechanism triggered on forst @@ -28,10 +28,9 @@ import org.springframework.util.Assert; * * @author Oliver Gierke */ -public abstract class PluginRegistrySupport, S> implements PluginRegistry, Iterable { +abstract class PluginRegistrySupport, S> implements PluginRegistry, Iterable { - private List plugins; - private boolean initialized; + private final Supplier> plugins; /** * Creates a new {@link PluginRegistrySupport} instance using the given plugins. @@ -43,8 +42,15 @@ public abstract class PluginRegistrySupport, S> implements P Assert.notNull(plugins, "Plugins must not be null!"); - this.plugins = plugins == null ? new ArrayList<>() : (List) plugins; - this.initialized = false; + this.plugins = SingletonSupplier.of((List) plugins.stream().filter(it -> it != null).toList()); + } + + @SuppressWarnings("unchecked") + protected PluginRegistrySupport(Supplier> plugins) { + + this.plugins = () -> (List) plugins.get().stream() + .filter(it -> it != null) + .toList(); } /** @@ -55,29 +61,7 @@ public abstract class PluginRegistrySupport, S> implements P * @return all plugins of the registry */ public List getPlugins() { - - if (!initialized) { - this.plugins = initialize(this.plugins); - this.initialized = true; - } - - return plugins; - } - - /** - * Callback to initialize the plugin {@link List}. Will create a defensive copy of the {@link List} to potentially - * unwrap a {@link List} proxy. Will filter {@literal null} values from the source list as well. - * - * @param plugins must not be {@literal null}. - * @return - */ - protected synchronized List initialize(List plugins) { - - Assert.notNull(plugins, "Plugins must not be null!"); - - return plugins.stream() // - .filter(it -> it != null) // - .collect(Collectors.toList()); + return plugins.get(); } /* diff --git a/core/src/main/java/org/springframework/plugin/core/SimplePluginRegistry.java b/core/src/main/java/org/springframework/plugin/core/SimplePluginRegistry.java index eec0a39..440f4ab 100644 --- a/core/src/main/java/org/springframework/plugin/core/SimplePluginRegistry.java +++ b/core/src/main/java/org/springframework/plugin/core/SimplePluginRegistry.java @@ -31,7 +31,7 @@ import org.springframework.util.Assert; * * @author Oliver Gierke */ -public class SimplePluginRegistry, S> extends PluginRegistrySupport { +class SimplePluginRegistry, S> extends PluginRegistrySupport { /** * Creates a new {@code SimplePluginRegistry}. Will create an empty registry if {@literal null} is provided. @@ -42,6 +42,10 @@ public class SimplePluginRegistry, S> extends PluginRegistry super(plugins); } + protected SimplePluginRegistry(Supplier> plugins) { + super(plugins); + } + /** * Creates a new {@link SimplePluginRegistry}. * diff --git a/core/src/main/java/org/springframework/plugin/core/aot/PluginRegistryRuntimeHints.java b/core/src/main/java/org/springframework/plugin/core/aot/PluginRegistryRuntimeHints.java deleted file mode 100644 index af093e0..0000000 --- a/core/src/main/java/org/springframework/plugin/core/aot/PluginRegistryRuntimeHints.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright 2022 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.plugin.core.aot; - -import java.util.List; - -import org.springframework.aop.SpringProxy; -import org.springframework.aop.framework.Advised; -import org.springframework.aot.hint.RuntimeHints; -import org.springframework.aot.hint.RuntimeHintsRegistrar; -import org.springframework.core.DecoratingProxy; -import org.springframework.plugin.core.support.AbstractTypeAwareSupport; - -/** - * Registers proxy runtime hints to make sure {@link AbstractTypeAwareSupport} can create a {@link List} proxy as - * needed. - * - * @author Oliver Drotbohm - * @since 3.0 - */ -class PluginRegistryRuntimeHints implements RuntimeHintsRegistrar { - - /* - * (non-Javadoc) - * @see org.springframework.aot.hint.RuntimeHintsRegistrar#registerHints(org.springframework.aot.hint.RuntimeHints, java.lang.ClassLoader) - */ - @Override - public void registerHints(RuntimeHints hints, ClassLoader classLoader) { - - hints.proxies() // - .registerJdkProxy(List.class, SpringProxy.class, Advised.class, DecoratingProxy.class); - } -} diff --git a/core/src/main/java/org/springframework/plugin/core/config/PluginRegistriesBeanDefinitionRegistrar.java b/core/src/main/java/org/springframework/plugin/core/config/PluginRegistriesBeanDefinitionRegistrar.java index 3669fcf..43e3741 100644 --- a/core/src/main/java/org/springframework/plugin/core/config/PluginRegistriesBeanDefinitionRegistrar.java +++ b/core/src/main/java/org/springframework/plugin/core/config/PluginRegistriesBeanDefinitionRegistrar.java @@ -28,7 +28,6 @@ import org.springframework.core.ResolvableType; import org.springframework.core.type.AnnotationMetadata; import org.springframework.plugin.core.OrderAwarePluginRegistry; import org.springframework.plugin.core.Plugin; -import org.springframework.plugin.core.PluginRegistry; import org.springframework.plugin.core.support.PluginRegistryFactoryBean; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -63,9 +62,8 @@ public class PluginRegistriesBeanDefinitionRegistrar implements ImportBeanDefini for (Class type : types) { - RootBeanDefinition beanDefinition = new RootBeanDefinition(getTargetType(type, PluginRegistryFactoryBean.class)); + RootBeanDefinition beanDefinition = new RootBeanDefinition(PluginRegistryFactoryBean.class); beanDefinition.setTargetType(getTargetType(type, OrderAwarePluginRegistry.class)); - beanDefinition.setBeanClass(PluginRegistryFactoryBean.class); beanDefinition.getPropertyValues().addPropertyValue("type", type); Qualifier annotation = type.getAnnotation(Qualifier.class); diff --git a/core/src/main/java/org/springframework/plugin/core/support/AbstractTypeAwareSupport.java b/core/src/main/java/org/springframework/plugin/core/support/AbstractTypeAwareSupport.java deleted file mode 100644 index 3baa63a..0000000 --- a/core/src/main/java/org/springframework/plugin/core/support/AbstractTypeAwareSupport.java +++ /dev/null @@ -1,227 +0,0 @@ -/* - * Copyright 2008-2012 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.plugin.core.support; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.function.Predicate; - -import org.springframework.aop.TargetSource; -import org.springframework.aop.framework.ProxyFactory; -import org.springframework.beans.factory.InitializingBean; -import org.springframework.beans.factory.ListableBeanFactory; -import org.springframework.context.ApplicationContext; -import org.springframework.context.ApplicationContextAware; -import org.springframework.context.ApplicationListener; -import org.springframework.context.event.ContextRefreshedEvent; -import org.springframework.lang.NonNull; -import org.springframework.lang.Nullable; -import org.springframework.util.Assert; - -/** - * Abstract base class to implement types that need access to all beans of a given type from the - * {@link ApplicationContext}. - * - * @author Oliver Gierke - */ -public abstract class AbstractTypeAwareSupport - implements ApplicationContextAware, ApplicationListener, InitializingBean { - - private @Nullable ApplicationContext context; - private @Nullable Class type; - private @Nullable BeansOfTypeTargetSource targetSource; - private Collection> exclusions = Collections.emptySet(); - - /* - * (non-Javadoc) - * @see org.springframework.context.ApplicationContextAware#setApplicationContext(org.springframework.context.ApplicationContext) - */ - public void setApplicationContext(ApplicationContext context) { - this.context = context; - } - - /** - * Configures the type of beans to be looked up. - * - * @param type the type to set - */ - public void setType(Class type) { - this.type = type; - } - - /** - * Configures the types to be excluded from the lookup. - * - * @param exclusions - */ - public void setExclusions(Class[] exclusions) { - this.exclusions = Arrays.asList(exclusions); - } - - /** - * Returns all beans from the {@link ApplicationContext} that match the given type. - * - * @return - */ - @SuppressWarnings("unchecked") - protected List getBeans() { - - TargetSource targetSource = this.targetSource; - - if (targetSource == null) { - throw new IllegalStateException("Traget source not initialized!"); - } - - ProxyFactory factory = new ProxyFactory(List.class, targetSource); - - return (List) factory.getProxy(); - } - - /* - * (non-Javadoc) - * @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet() - */ - public void afterPropertiesSet() { - - ApplicationContext context = this.context; - - if (context == null) { - throw new IllegalStateException("ApplicationContext not set!"); - } - - Class type = this.type; - - if (type == null) { - throw new IllegalStateException("No type configured!"); - } - - this.targetSource = new BeansOfTypeTargetSource(context, type, false, exclusions); - } - - /* - * (non-Javadoc) - * @see org.springframework.context.ApplicationListener#onApplicationEvent(org.springframework.context.ApplicationEvent) - */ - public void onApplicationEvent(ContextRefreshedEvent event) { - - if (event.getApplicationContext().equals(context) && targetSource != null) { - targetSource.freeze(); - } - } - - /** - * {@link TargetSource} implementation that returns all beans of the configured type from the - * {@link ListableBeanFactory} the instance was set up with. Allows freezing the lookup as calls to - * {@link ListableBeanFactory#getBeansOfType(Class, boolean, boolean)} are potentially expensive as the entire factory - * has to be scanned for type information. - * - * @author Oliver Gierke - */ - static class BeansOfTypeTargetSource implements TargetSource { - - private final ListableBeanFactory context; - private final Class type; - private final boolean eagerInit; - private final Collection> exclusions; - - private boolean frozen = false; - private @Nullable Collection components; - - /** - * Creates a new {@link BeansOfTypeTargetSource} using the given {@link ListableBeanFactory} to lookup beans of the - * given type. - * - * @param context must not be {@literal null}. - * @param type must not be {@literal null}. - * @param eagerInit whether to eagerly init {@link FactoryBean}s, defaults to {@literal false}. - * @param exclusions which types to exclude from the lookup, must not be {@literal null}. - */ - public BeansOfTypeTargetSource(ListableBeanFactory context, Class type, boolean eagerInit, - Collection> exclusions) { - - Assert.notNull(context, "ListableBeanFactory must not be null!"); - Assert.notNull(type, "Type must not be null!"); - Assert.notNull(exclusions, "Exclusions must not be null!"); - - this.context = context; - this.type = type; - this.eagerInit = eagerInit; - this.exclusions = exclusions; - this.components = null; - } - - /** - * Freezes the {@link TargetSource} so that the next access to {@link #getTarget()} will get the results cached and - * reused. - */ - public void freeze() { - this.frozen = true; - } - - /* - * (non-Javadoc) - * @see org.springframework.aop.TargetSource#getTargetClass() - */ - @NonNull - public Class getTargetClass() { - return List.class; - } - - /* - * (non-Javadoc) - * @see org.springframework.aop.TargetSource#isStatic() - */ - public boolean isStatic() { - return frozen; - } - - /* - * (non-Javadoc) - * @see org.springframework.aop.TargetSource#getTarget() - */ - @NonNull - @SuppressWarnings({ "rawtypes", "unchecked" }) - public synchronized Object getTarget() throws Exception { - - Collection components = this.components == null // - ? getBeansOfTypeExcept(type, exclusions) // - : this.components; - - if (frozen && this.components == null) { - this.components = components; - } - - return new ArrayList(components); - } - - /* - * (non-Javadoc) - * @see org.springframework.aop.TargetSource#releaseTarget(java.lang.Object) - */ - public void releaseTarget(Object target) throws Exception {} - - private Collection getBeansOfTypeExcept(Class type, Collection> exceptions) { - - return context.getBeanProvider(type, eagerInit) - .stream(Predicate.not(exceptions::contains)) - .map(Object.class::cast) - .toList(); - } - } -} diff --git a/core/src/main/java/org/springframework/plugin/core/support/PluginRegistryFactoryBean.java b/core/src/main/java/org/springframework/plugin/core/support/PluginRegistryFactoryBean.java index 5805ec6..c10bef2 100644 --- a/core/src/main/java/org/springframework/plugin/core/support/PluginRegistryFactoryBean.java +++ b/core/src/main/java/org/springframework/plugin/core/support/PluginRegistryFactoryBean.java @@ -15,8 +15,20 @@ */ package org.springframework.plugin.core.support; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.function.Predicate; +import java.util.function.Supplier; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; import org.springframework.plugin.core.OrderAwarePluginRegistry; import org.springframework.plugin.core.Plugin; import org.springframework.plugin.core.PluginRegistry; @@ -26,8 +38,44 @@ import org.springframework.plugin.core.PluginRegistry; * * @author Oliver Gierke */ -public class PluginRegistryFactoryBean, S> extends AbstractTypeAwareSupport - implements FactoryBean> { +public class PluginRegistryFactoryBean, S> + implements FactoryBean>, BeanFactoryAware { + + private Collection> exclusions = Collections.emptySet(); + private @Nullable Class type; + private ListableBeanFactory factory; + + /** + * Configures the type of beans to be looked up. + * + * @param type the type to set + */ + public void setType(Class type) { + this.type = type; + } + + /** + * Configures the types to be excluded from the lookup. + * + * @param exclusions + */ + public void setExclusions(Class[] exclusions) { + this.exclusions = Arrays.asList(exclusions); + } + + /* + * (non-Javadoc) + * @see org.springframework.beans.factory.BeanFactoryAware#setBeanFactory(org.springframework.beans.factory.BeanFactory) + */ + @Override + public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + + if (!(beanFactory instanceof ListableBeanFactory factory)) { + throw new IllegalArgumentException("Expected a ListableBeanFactory!"); + } + + this.factory = factory; + } /* * (non-Javadoc) @@ -35,7 +83,18 @@ public class PluginRegistryFactoryBean, S> extends AbstractT */ @NonNull public OrderAwarePluginRegistry getObject() { - return OrderAwarePluginRegistry.of(getBeans()); + + var type = this.type; + + if (type == null) { + throw new IllegalStateException("No plugin type configured!"); + } + + Supplier> plugins = () -> factory.getBeanProvider(type, false) + .stream(Predicate.not(exclusions::contains)) + .toList(); + + return OrderAwarePluginRegistry.of(plugins); } /* diff --git a/core/src/main/resources/META-INF/spring/aot.factories b/core/src/main/resources/META-INF/spring/aot.factories deleted file mode 100644 index d2d4891..0000000 --- a/core/src/main/resources/META-INF/spring/aot.factories +++ /dev/null @@ -1,2 +0,0 @@ -org.springframework.aot.hint.RuntimeHintsRegistrar=\ - org.springframework.plugin.core.aot.PluginRegistryRuntimeHints