matrei commented on code in PR #15409: URL: https://github.com/apache/grails-core/pull/15409#discussion_r2975126834
########## grails-core/src/main/groovy/org/apache/grails/core/plugins/GrailsPluginDiscovery.java: ########## @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.grails.core.plugins; + +import java.util.Collection; +import java.util.Map; + +import org.springframework.core.env.Environment; +import org.springframework.core.io.Resource; + +import grails.plugins.PluginFilter; + +/** + * This class is responsible for locating Grails Plugins that should be loaded. + * + * @since 7.1 + */ +public interface GrailsPluginDiscovery { + + /** + * After the bootstrap process, the name of the bean that this plugin discovery will be promoted with + */ + String BEAN_NAME = "grailsPluginDiscovery"; + + /** + * Initializes the plugin discovery mechanism. This method is called during the {@link grails.boot.config.GrailsEnvironmentPostProcessor} + * + * @param environment the environment to check for plugin exclusions + */ + void init(Environment environment); + + /** + * This method is typically only used in testing, specifically unit tests. + * + * @return Plugins that were defined dynamically outside of the classpath + */ + GrailsPluginInfo[] getDynamicPlugins(); + + /** + * @return Plugins that failed to load during the discovery process + */ + Map<String, GrailsPluginInfo> getFailedPlugins(); + + /** + * @return Plugins that were discovered during the bootstrap process, no ordering guaranteed + */ + Collection<GrailsPluginInfo> getPlugins(); + + /** + * @param pluginName a plugin name to search for, the name may not be normalized + * @return the plugin information if found + */ + GrailsPluginInfo getPlugin(String pluginName); + + /** + * + * @param pluginName a plugin name to search for, the name may not be normalized + * @param version the required version of the plugin + * @return if a plugin was found, and it meets the required version it will be returned, otherwise null + */ + GrailsPluginInfo getPlugin(String pluginName, Object version); + + /** + * @return plugins ordered by a topographical sort. + */ + Collection<GrailsPluginInfo> getOrderedPlugins(); + + /** + * @return the order the plugins were loaded in. + */ + Collection<GrailsPluginInfo> getLoadOrderedPlugins(); + + /** + * @param name the name of the plugin - it may not be normalized + * @return true if a plugin with the given name exists, false otherwise + */ + boolean hasPlugin(String name); + + /** + * @param plugin the plugin to get the observers for + * @return the plugins that are observing the given plugin, empty if none + */ + Collection<GrailsPluginInfo> getPluginObservers(GrailsPluginInfo plugin); Review Comment: Use a method name that does not use the getter idiom, like `findPluginObservers(GrailsPluginInfo plugin)`? ########## grails-core/src/main/groovy/org/apache/grails/core/plugins/GrailsPluginDiscovery.java: ########## @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.grails.core.plugins; + +import java.util.Collection; +import java.util.Map; + +import org.springframework.core.env.Environment; +import org.springframework.core.io.Resource; + +import grails.plugins.PluginFilter; + +/** + * This class is responsible for locating Grails Plugins that should be loaded. + * + * @since 7.1 + */ +public interface GrailsPluginDiscovery { + + /** + * After the bootstrap process, the name of the bean that this plugin discovery will be promoted with + */ + String BEAN_NAME = "grailsPluginDiscovery"; + + /** + * Initializes the plugin discovery mechanism. This method is called during the {@link grails.boot.config.GrailsEnvironmentPostProcessor} + * + * @param environment the environment to check for plugin exclusions + */ + void init(Environment environment); + + /** + * This method is typically only used in testing, specifically unit tests. + * + * @return Plugins that were defined dynamically outside of the classpath + */ + GrailsPluginInfo[] getDynamicPlugins(); + + /** + * @return Plugins that failed to load during the discovery process + */ + Map<String, GrailsPluginInfo> getFailedPlugins(); + + /** + * @return Plugins that were discovered during the bootstrap process, no ordering guaranteed + */ + Collection<GrailsPluginInfo> getPlugins(); + + /** + * @param pluginName a plugin name to search for, the name may not be normalized + * @return the plugin information if found + */ + GrailsPluginInfo getPlugin(String pluginName); Review Comment: Use a method name that does not use the getter idiom, like `findPlugin(String pluginName)`? ########## grails-core/src/main/groovy/org/apache/grails/core/plugins/GrailsPluginDiscovery.java: ########## @@ -0,0 +1,593 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.grails.core.plugins; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +import groovy.lang.GroovyClassLoader; +import org.codehaus.groovy.control.CompilationFailedException; +import org.codehaus.groovy.runtime.IOGroovyMethods; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.springframework.core.env.Environment; +import org.springframework.core.io.Resource; +import org.springframework.core.io.support.PathMatchingResourcePatternResolver; + +import grails.plugins.GrailsPluginSorter; +import grails.plugins.GrailsVersionUtils; +import grails.plugins.PluginFilter; +import grails.plugins.exceptions.PluginException; +import grails.util.Metadata; +import org.apache.grails.core.plugins.filters.PluginFilterRetriever; +import org.grails.core.io.CachingPathMatchingResourcePatternResolver; +import org.grails.io.support.GrailsResourceUtils; + +/** + * This class provides the canonical implementations of Grails Plugin Discovery. + * + * @since 7.1 + */ +public class GrailsPluginDiscovery { + + public static final String BEAN_NAME = "grailsPluginDiscovery"; + private static final Logger LOG = LoggerFactory.getLogger(GrailsPluginDiscovery.class); + + protected Metadata applicationMeta = Metadata.getCurrent(); + protected Resource[] pluginResources = new Resource[0]; + protected Class<?>[] pluginClasses = new Class[0]; + /** + * plugins ordered by the load order + */ + protected LinkedHashMap<String, GrailsPluginInfo> plugins; + /** + * plugins sorted by the topographical sort + */ + protected List<GrailsPluginInfo> orderedPlugins; + /** + * plugins sorted by the load order + */ + protected List<GrailsPluginInfo> loadOrderedPlugins; + protected List<GrailsPluginInfo> dynamicPlugins; + protected Map<String, Set<GrailsPluginInfo>> pluginToObserverMap; + protected List<GrailsPluginInfo> delayedLoadPlugins; + protected Map<String, GrailsPluginInfo> failedPlugins; + protected Map<GrailsPluginInfo, String[]> delayedEvictions; + protected PluginFilter pluginFilter; + protected boolean loadClasspathPlugins = true; + protected boolean requireClasspathPlugin = true; + protected final PluginFilterRetriever filterRetriever; + + public GrailsPluginDiscovery() { + this(new PluginFilterRetriever()); + } + + public GrailsPluginDiscovery(String resourcePath) { + this(); + + PathMatchingResourcePatternResolver resolver = CachingPathMatchingResourcePatternResolver.INSTANCE; + try { + pluginResources = resolver.getResources(resourcePath); + } catch (IOException ioe) { + LOG.debug("Unable to load plugins for resource path {}", resourcePath, ioe); + } + } + + public GrailsPluginDiscovery(Class<?>[] pluginClasses) { + this(); + this.pluginClasses = pluginClasses; + } + + public GrailsPluginDiscovery(String[] pluginResources) { + this(); + + PathMatchingResourcePatternResolver resolver = CachingPathMatchingResourcePatternResolver.INSTANCE; + + List<Resource> resourceList = new ArrayList<>(); + for (String resourcePath : pluginResources) { + try { + resourceList.addAll(Arrays.asList(resolver.getResources(resourcePath))); + } catch (IOException ioe) { + LOG.debug("Unable to load plugins for resource path {}", resourcePath, ioe); + } + } + + this.pluginResources = resourceList.toArray(new Resource[0]); + } + + public GrailsPluginDiscovery(Resource[] pluginFiles) { + this(); + this.pluginResources = pluginFiles; + } + + public GrailsPluginDiscovery(PluginFilterRetriever filterRetriever) { + this.filterRetriever = filterRetriever; + } + + public GrailsPluginInfo[] getDynamicPlugins() { + return dynamicPlugins.toArray(new GrailsPluginInfo[0]); + } + + public Map<String, GrailsPluginInfo> getFailedPlugins() { + return failedPlugins; + } + + public GrailsPluginInfo getPlugin(String pluginName, Object version, Environment environment) { + if (plugins == null) { + if (environment == null) { + throw new IllegalArgumentException("Environment must be provided to fetch a plugin"); + } + findPlugins(environment); + } + + GrailsPluginInfo plugin = plugins.get(GrailsPluginUtils.normalizePluginName(pluginName)); + if (plugin != null && GrailsVersionUtils.isValidVersion(plugin.pluginVersion(), version.toString())) { + return plugin; + } + return null; + } + + public boolean hasPlugin(String name) { + return plugins.containsKey(GrailsPluginUtils.normalizePluginName(name)); + } + + public GrailsPluginInfo getPlugin(String pluginName, Environment environment) { + if (plugins == null) { + if (environment == null) { + throw new IllegalArgumentException("Environment must be provided to fetch a plugin"); + } + + findPlugins(environment); + } + + return plugins.get(GrailsPluginUtils.normalizePluginName(pluginName)); + } + + public Collection<GrailsPluginInfo> getPluginObservers(GrailsPluginInfo plugin) { + Objects.requireNonNull(plugin, "Argument [plugin] cannot be null"); + + Collection<GrailsPluginInfo> c = pluginToObserverMap.get(plugin.name()); + + // Add any wildcard observers. + Collection<GrailsPluginInfo> wildcardObservers = pluginToObserverMap.get("*"); + if (wildcardObservers != null) { + if (c != null) { + c.addAll(wildcardObservers); + } else { + c = wildcardObservers; + } + } + + if (c != null) { + // Make sure this plugin is not observing itself! + c.remove(plugin); + return c; + } + + return Collections.emptySet(); + } + + public Collection<GrailsPluginInfo> getPlugins(Environment environment) { + return plugins == null ? findPlugins(environment).values() : plugins.values(); + } + + /** + * @return plugins ordered by a topographical sort. + */ + public Collection<GrailsPluginInfo> getOrderedPlugins(Environment environment) { + if (orderedPlugins == null) { + findPlugins(environment); + } + + return orderedPlugins; + } + + /** + * @return the order the plugins were loaded in. + */ + public Collection<GrailsPluginInfo> getLoadOrderedPlugins(Environment environment) { Review Comment: There is still: - `getPlugin(String pluginName)` - `getPlugin(String pluginName, Object version)` - `getPluginObservers(GrailsPluginInfo plugin)` Can we use `findPlugin*`? ########## grails-core/src/test/groovy/org/grails/core/io/ResourceLocatorSpec.groovy: ########## @@ -73,9 +80,15 @@ class ResourceLocatorSpec extends Specification { def xml = new XmlSlurper().parseText(str) Review Comment: Question remains ########## grails-core/src/main/groovy/org/apache/grails/core/plugins/GrailsPluginInfo.java: ########## @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.grails.core.plugins; + +import org.springframework.core.io.Resource; + +/** + * Lightweight value class holding plugin metadata needed for ordering and + * configuration loading. Wraps {@link GrailsPluginLoadMetadata} + * with an additional configuration resource reference. + */ +public final class GrailsPluginInfo { + + private final GrailsPluginLoadMetadata metadata; + private final GrailsPluginDescriptor pluginDescriptor; + private final Resource configResource; + private final boolean dynamic; + + /** + * Creates a new {@code PluginInfo}. + * + * @param metadata the plugin metadata from {@link GrailsPluginDiscovery} + * @param configResource the plugin's configuration resource ({@code plugin.yml} or + * {@code plugin.groovy}), or {@code null} if no config file exists + */ + GrailsPluginInfo(GrailsPluginDescriptor pluginDescriptor, GrailsPluginLoadMetadata metadata, Resource configResource, boolean dynamic) { + this.metadata = metadata; + this.pluginDescriptor = pluginDescriptor; + this.configResource = configResource; + this.dynamic = dynamic; + } + + public String name() { + return metadata.name(); + } + + public String pluginVersion() { + return metadata.pluginVersion(); + } + + public Class<?> pluginClass() { + return metadata.pluginClass(); + } + + public Resource configResource() { + return configResource; + } + + public GrailsPluginLoadMetadata metadata() { + return metadata; + } + + public String[] loadAfterNames() { + return metadata.loadAfterNames(); + } + + public String[] loadBeforeNames() { + return metadata.loadBeforeNames(); + } + + public String[] dependsOnNames() { + return metadata.dependsOnNames(); + } + + public String[] observedPluginNames() { + return metadata.observedPluginNames(); + } + + public boolean isDynamic() { + return dynamic; + } + + public String grailsVersion() { + return metadata().grailsVersion(); + } + + public String[] evictions() { + return metadata.evictions(); + } + + public GrailsPluginDescriptor pluginDescriptor() { + return pluginDescriptor; + } Review Comment: > Why the record-style naming instead of getters? The question remains, the class naming and Java vs Groovy was broader sidetracks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
