matrei commented on code in PR #15409:
URL: https://github.com/apache/grails-core/pull/15409#discussion_r2925160459
##########
grails-test-suite-uber/src/test/groovy/org/grails/commons/GrailsPluginManagerTests.groovy:
##########
@@ -116,12 +117,13 @@ hibernate {
}
void testDefaultGrailsPluginManager() {
- DefaultGrailsPluginManager manager = new
DefaultGrailsPluginManager(RESOURCE_PATH,ga)
- assertEquals(1, manager.getPluginResources().length)
+ def discovery = new GrailsPluginDiscovery(RESOURCE_PATH)
+ DefaultGrailsPluginManager manager = new
DefaultGrailsPluginManager(ga, discovery)
+ assertEquals(1, discovery.getPluginResources().length)
}
void testLoadPlugins() {
- GrailsPluginManager manager = new
DefaultGrailsPluginManager(RESOURCE_PATH,ga)
+ GrailsPluginManager manager = new DefaultGrailsPluginManager(ga, new
GrailsPluginDiscovery())
Review Comment:
`def`?
##########
grails-test-suite-uber/src/test/groovy/org/grails/plugins/PluginLoadOrderTests.groovy:
##########
@@ -62,16 +67,21 @@ class ThreeGrailsPlugin {
def three = gcl.loadClass("ThreeGrailsPlugin")
def four = gcl.loadClass("FourGrailsPlugin")
def five = gcl.loadClass("FiveGrailsPlugin")
- def pluginManager = new DefaultGrailsPluginManager([one,two,three,
four,five] as Class[],
- new DefaultGrailsApplication())
-
- pluginManager.loadCorePlugins = false
+ GrailsPluginDiscovery pluginDiscovery = new
GrailsPluginDiscovery([one, two, three, four, five] as Class[],)
Review Comment:
`def`?
##########
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy:
##########
@@ -71,17 +78,18 @@ class GrailsMicronautGrailsPlugin extends Plugin {
[plugins, pluginsFromContext].each { pluginsToProcess ->
pluginsToProcess
.findAll { it.propertySource != null }
Review Comment:
You have deprecated `GrailsPlugin.propertySource` but it is still used here?
##########
grails-core/src/test/groovy/org/grails/plugins/GrailsPluginTests.groovy:
##########
@@ -162,16 +165,36 @@ class TestGrailsPlugin {
''')
DefaultGrailsApplication application = new DefaultGrailsApplication()
- def pluginManager = new DefaultGrailsPluginManager([test1] as Class[],
application)
-
-pluginManager.loadPlugins()
+
+ // Create and set up application context
+ def appCtx = new GenericApplicationContext()
+ application.setMainContext(appCtx)
+
+ // Create discovery bean configured for unit tests
+ GrailsPluginDiscovery discovery = new GrailsPluginDiscovery(new
Class<?>[]{test1})
+
+ // simulate the call in GrailsEnvironmentPostProcessor to get the
metadata
+ discovery.getPlugins(new StandardEnvironment())
+
+ def pluginManager = new DefaultGrailsPluginManager(application,
discovery)
+ pluginManager.loadPlugins()
assertNotNull pluginManager.getGrailsPlugin("test")
String originalEnv = System.getProperty(Environment.KEY)
try {
System.setProperty(Environment.KEY,
Environment.PRODUCTION.getName())
- pluginManager = new DefaultGrailsPluginManager([test1] as Class[],
application)
+ // Create new application and context for production environment
test
+ application = new DefaultGrailsApplication()
+ appCtx = new GenericApplicationContext()
+ application.setMainContext(appCtx)
+
+ // Create new discovery bean for production environment test
+ discovery = new GrailsPluginDiscovery(new Class<?>[]{test1})
+ discovery.setLoadClasspathPlugins(false)
Review Comment:
Groovy property setter?
##########
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.
Review Comment:
I don't understand this sentence. Does it mean: "This class finds and
provides access to the Grails plugins on the classpath?
##########
grails-test-suite-uber/src/test/groovy/org/apache/grails/core/plugins/filters/PluginFilterFactoryTests.java:
##########
@@ -16,59 +16,73 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.plugins;
+package org.apache.grails.core.plugins.filters;
+import grails.config.Settings;
import grails.plugins.PluginFilter;
import org.junit.jupiter.api.Test;
import java.util.Set;
+import org.springframework.mock.env.MockEnvironment;
+
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertTrue;
-@SuppressWarnings("rawtypes")
public class PluginFilterFactoryTests {
+ MockEnvironment createEnvironment(String includes, String excludes) {
+ MockEnvironment env = new MockEnvironment();
+ if(includes != null) {
+ env.setProperty(Settings.PLUGIN_INCLUDES, includes);
+ }
+ if(excludes != null) {
+ env.setProperty(Settings.PLUGIN_EXCLUDES, excludes);
+ }
+ return env;
+ }
+
@Test
- public void testIncludeFilterOne() throws Exception {
+ public void testIncludeFilterOne() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter("one", null);
- assertTrue(bean instanceof IncludingPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment("one", null));
+ assertInstanceOf(IncludingPluginFilter.class, bean);
IncludingPluginFilter filter = (IncludingPluginFilter)bean;
- Set suppliedNames = filter.getSuppliedNames();
+ Set<String> suppliedNames = filter.getSuppliedNames();
assertEquals(1, suppliedNames.size());
assertTrue(suppliedNames.contains("one"));
}
@Test
- public void testIncludeFilter() throws Exception {
+ public void testIncludeFilter() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter("one, two", " three , four ");
- assertTrue(bean instanceof IncludingPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment("one, two", "
three , four "));
+ assertInstanceOf(IncludingPluginFilter.class, bean);
IncludingPluginFilter filter = (IncludingPluginFilter)bean;
- Set suppliedNames = filter.getSuppliedNames();
+ Set<String> suppliedNames = filter.getSuppliedNames();
Review Comment:
`var` (and also above)?
##########
grails-test-suite-uber/src/test/groovy/org/apache/grails/core/plugins/filters/PluginFilterFactoryTests.java:
##########
@@ -16,59 +16,73 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.plugins;
+package org.apache.grails.core.plugins.filters;
+import grails.config.Settings;
import grails.plugins.PluginFilter;
import org.junit.jupiter.api.Test;
import java.util.Set;
+import org.springframework.mock.env.MockEnvironment;
+
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertTrue;
-@SuppressWarnings("rawtypes")
public class PluginFilterFactoryTests {
+ MockEnvironment createEnvironment(String includes, String excludes) {
+ MockEnvironment env = new MockEnvironment();
+ if(includes != null) {
+ env.setProperty(Settings.PLUGIN_INCLUDES, includes);
+ }
+ if(excludes != null) {
+ env.setProperty(Settings.PLUGIN_EXCLUDES, excludes);
+ }
+ return env;
+ }
+
@Test
- public void testIncludeFilterOne() throws Exception {
+ public void testIncludeFilterOne() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter("one", null);
- assertTrue(bean instanceof IncludingPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment("one", null));
+ assertInstanceOf(IncludingPluginFilter.class, bean);
IncludingPluginFilter filter = (IncludingPluginFilter)bean;
- Set suppliedNames = filter.getSuppliedNames();
+ Set<String> suppliedNames = filter.getSuppliedNames();
assertEquals(1, suppliedNames.size());
assertTrue(suppliedNames.contains("one"));
}
@Test
- public void testIncludeFilter() throws Exception {
+ public void testIncludeFilter() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter("one, two", " three , four ");
- assertTrue(bean instanceof IncludingPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment("one, two", "
three , four "));
+ assertInstanceOf(IncludingPluginFilter.class, bean);
IncludingPluginFilter filter = (IncludingPluginFilter)bean;
- Set suppliedNames = filter.getSuppliedNames();
+ Set<String> suppliedNames = filter.getSuppliedNames();
assertEquals(2, suppliedNames.size());
assertTrue(suppliedNames.contains("two"));
}
@Test
- public void testExcludeFilter() throws Exception {
+ public void testExcludeFilter() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter(null, " three , four ");
- assertTrue(bean instanceof ExcludingPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment(null, " three
, four "));
+ assertInstanceOf(ExcludingPluginFilter.class, bean);
ExcludingPluginFilter filter = (ExcludingPluginFilter)bean;
- Set suppliedNames = filter.getSuppliedNames();
+ Set<String> suppliedNames = filter.getSuppliedNames();
assertEquals(2, suppliedNames.size());
assertTrue(suppliedNames.contains("four"));
}
@Test
- public void testDefaultFilter() throws Exception {
+ public void testDefaultFilter() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter(null, null);
- assertTrue(bean instanceof IdentityPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment(null, null));
Review Comment:
`var` and also above?
##########
grails-core/src/test/groovy/org/apache/grails/core/plugins/GrailsPluginUtilsSpec.groovy:
##########
@@ -0,0 +1,333 @@
+/*
+ * 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 spock.lang.Specification
+import spock.lang.Unroll
+
+import java.net.URLClassLoader
+
+/**
+ * Test suite for GrailsPluginUtils utility methods
+ */
+class GrailsPluginUtilsSpec extends Specification {
+
+ @Unroll
+ def "isPluginVersionCompatible should check that plugin with
grailsVersion=#pluginGrailsVersion is compatible with grails #grailsVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0", // pluginVersion
+ pluginGrailsVersion, // pluginSupportedVersion
+ grailsVersion, // grailsVersion
+ "test-plugin" // pluginDescription
+ )
+
+ then:
+ compatible == expectedCompatible
+
+ where:
+ grailsVersion | pluginGrailsVersion || expectedCompatible
+ "1.0" | "3.3.1 > *" || false
+ "2.5" | "3.0.1" || false
+ "3.0.0" | "3.3.10 > *" || false
+ "3.3.10" | "4.0.0 > *" || false
+ "4.0.1" | "3.0.0.BUILD-SNAPSHOT > *" || true
+ "4.0.1" | "4.0.1" || true
+ "4.0.1" | "3.0.1" || false
+ "4.0.1" | "3.3.1 > *" || true
+ "4.0.1" | "3.3.10 > *" || true
+ }
+
+ def "isPluginVersionCompatible should handle null grailsVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0",
+ "3.3.10 > *",
+ null, // grailsVersion is null
+ "test-plugin"
+ )
+
+ then:
+ compatible == true
+ }
+
+ def "isPluginVersionCompatible should handle null
pluginSupportedVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0",
+ null, // pluginSupportedVersion is null
+ "4.0.1",
+ "test-plugin"
Review Comment:
Single quotes?
##########
grails-core/src/test/groovy/org/apache/grails/core/plugins/GrailsPluginUtilsSpec.groovy:
##########
@@ -0,0 +1,333 @@
+/*
+ * 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 spock.lang.Specification
+import spock.lang.Unroll
+
+import java.net.URLClassLoader
+
+/**
+ * Test suite for GrailsPluginUtils utility methods
+ */
+class GrailsPluginUtilsSpec extends Specification {
+
+ @Unroll
+ def "isPluginVersionCompatible should check that plugin with
grailsVersion=#pluginGrailsVersion is compatible with grails #grailsVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0", // pluginVersion
+ pluginGrailsVersion, // pluginSupportedVersion
+ grailsVersion, // grailsVersion
+ "test-plugin" // pluginDescription
+ )
+
+ then:
+ compatible == expectedCompatible
+
+ where:
+ grailsVersion | pluginGrailsVersion || expectedCompatible
+ "1.0" | "3.3.1 > *" || false
+ "2.5" | "3.0.1" || false
+ "3.0.0" | "3.3.10 > *" || false
+ "3.3.10" | "4.0.0 > *" || false
+ "4.0.1" | "3.0.0.BUILD-SNAPSHOT > *" || true
+ "4.0.1" | "4.0.1" || true
+ "4.0.1" | "3.0.1" || false
+ "4.0.1" | "3.3.1 > *" || true
+ "4.0.1" | "3.3.10 > *" || true
+ }
+
+ def "isPluginVersionCompatible should handle null grailsVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0",
+ "3.3.10 > *",
+ null, // grailsVersion is null
+ "test-plugin"
+ )
+
+ then:
+ compatible == true
+ }
+
+ def "isPluginVersionCompatible should handle null
pluginSupportedVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0",
+ null, // pluginSupportedVersion is null
+ "4.0.1",
+ "test-plugin"
+ )
+
+ then:
+ compatible == true
+ }
+
+ def "isPluginVersionCompatible should handle @ in
pluginSupportedVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0",
+ "4.0.0@", // contains @
+ "3.0.0",
+ "test-plugin"
Review Comment:
Single quotes?
##########
grails-core/src/test/groovy/grails/plugins/DefaultGrailsPluginManagerSpec.groovy:
##########
@@ -19,79 +19,40 @@
package grails.plugins
import grails.core.GrailsApplication
-import grails.util.Metadata
-import org.apache.commons.logging.Log
-import org.grails.plugins.DefaultGrailsPlugin
+import org.apache.grails.core.plugins.GrailsPluginDiscovery
import spock.lang.Specification
-import spock.lang.Unroll
+/**
+ * Test suite for DefaultGrailsPluginManager
+ */
class DefaultGrailsPluginManagerSpec extends Specification {
- @Unroll
- def "should return #pluginGrailsVersion as plugin grails version"() {
+ def "plugin manager can be created with application and discovery bean"() {
given:
- GrailsApplication app = stubGrailsApplicationWithVersion("4.0.1")
- DefaultGrailsPluginManager sut = buildPluginsManager(app)
- DefaultGrailsPlugin plugin = stubPluginWithGrailsVersion(app,
pluginGrailsVersion)
+ GrailsApplication app = Mock(GrailsApplication)
+ GrailsPluginDiscovery discovery = Mock(GrailsPluginDiscovery)
when:
- def version = sut.getPluginGrailsVersion(plugin)
+ DefaultGrailsPluginManager manager = new
DefaultGrailsPluginManager(app, discovery)
then:
- version == pluginGrailsVersion
-
- where:
- pluginGrailsVersion | _
- "3.3.10 > *" | _
+ manager != null
}
- @Unroll
- def "it should check that plugin with grailsVersion=#pluginGrailsVersion
is compatible with grails #grailsVersion"() {
+ def "plugin manager can be created with just application"() {
given:
- GrailsApplication app = stubGrailsApplicationWithVersion(grailsVersion)
- DefaultGrailsPluginManager sut = buildPluginsManager(app)
- DefaultGrailsPlugin plugin = stubPluginWithGrailsVersion(app,
pluginGrailsVersion)
+ GrailsApplication app = Mock(GrailsApplication)
+ GrailsPluginDiscovery discovery = Mock(GrailsPluginDiscovery)
when:
- def compatible = sut.isCompatiblePlugin(plugin)
+ DefaultGrailsPluginManager manager = new
DefaultGrailsPluginManager(app, discovery)
Review Comment:
`def` x 3
##########
grails-core/src/main/groovy/grails/boot/config/GrailsEnvironmentPostProcessor.java:
##########
Review Comment:
Why Java (general question for all new classes)?
##########
grails-core/src/main/groovy/org/apache/grails/core/plugins/GrailsPluginLoadMetadata.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.Map;
+import java.util.Set;
+
+import grails.plugins.exceptions.PluginException;
+import grails.util.Environment;
+
+/**
+ * Lightweight value class holding plugin metadata extracted from a plugin
+ * class.
+ *
+ * @param name the logical plugin name (e.g., "core", "myPlugin")
+ * @param grailsVersion the grailsVersion this plugin supports
+ * @param pluginClass the plugin's class
+ * @param loadAfterNames plugin names this plugin should load after
+ * @param loadBeforeNames plugin names this plugin should load before
+ * @param dependsOnNames plugin names this plugin depends on (used for
+ * transitive dependency resolution during filtering, not for
+ * sort ordering)
+ * @param environments the environments this plugin is enabled for, or empty
if enabled for all environments
+ * @param status the status of the plugin
Review Comment:
There is no `status` param, should it be `enabled`?
##########
grails-testing-support-core/src/main/groovy/org/grails/testing/GrailsApplicationBuilder.groovy:
##########
@@ -151,11 +152,20 @@ class GrailsApplicationBuilder {
}
protected void prepareContext(ConfigurableApplicationContext
applicationContext, ConfigurableBeanFactory beanFactory) {
- registerGrailsAppPostProcessorBean(beanFactory)
+ def discovery = registerPluginDiscoveryBean(beanFactory)
+ registerGrailsAppPostProcessorBean(beanFactory, discovery)
AnnotationConfigUtils.registerAnnotationConfigProcessors((BeanDefinitionRegistry)
beanFactory)
new
ConfigDataApplicationContextInitializer().initialize(applicationContext)
}
+ protected GrailsPluginDiscovery
registerPluginDiscoveryBean(ConfigurableBeanFactory beanFactory) {
+ GrailsPluginDiscovery discovery = new GrailsPluginDiscovery()
Review Comment:
Use `def` for local variables where the type can be inferred?
##########
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy:
##########
@@ -71,17 +78,18 @@ class GrailsMicronautGrailsPlugin extends Plugin {
[plugins, pluginsFromContext].each { pluginsToProcess ->
pluginsToProcess
.findAll { it.propertySource != null }
- .each {
- log.debug('Loading configurations from {} plugin to
the parent Micronaut context', it.name)
- // If invoking the source as `.source`, the
NavigableMapPropertySource will return null,
- // while invoking the getter, it will return the
correct value
- micronautEnv.addPropertySource(
- PropertySource.of(
- "grails.plugins.$it.name",
- (Map) it.propertySource.getSource(),
- --priority
- )
- )
+ .each { plugin ->
+ // Look up the plugin's property source from the
Spring environment,
+ // where it was loaded by
GrailsEnvironmentPostProcessor
+ String pluginName = plugin.name
+ String ymlSourceName = pluginName + PLUGIN_YML_SUFFIX
+ String groovySourceName = pluginName +
PLUGIN_GROOVY_SUFFIX
+ org.springframework.core.env.PropertySource<?>
springPs =
+ springPropertySources.get(ymlSourceName) ?:
springPropertySources.get(groovySourceName)
Review Comment:
4 x `def`
##########
grails-core/src/test/groovy/org/grails/plugins/GrailsPluginTests.groovy:
##########
@@ -162,16 +165,36 @@ class TestGrailsPlugin {
''')
DefaultGrailsApplication application = new DefaultGrailsApplication()
- def pluginManager = new DefaultGrailsPluginManager([test1] as Class[],
application)
-
-pluginManager.loadPlugins()
+
+ // Create and set up application context
+ def appCtx = new GenericApplicationContext()
+ application.setMainContext(appCtx)
+
+ // Create discovery bean configured for unit tests
+ GrailsPluginDiscovery discovery = new GrailsPluginDiscovery(new
Class<?>[]{test1})
+
+ // simulate the call in GrailsEnvironmentPostProcessor to get the
metadata
+ discovery.getPlugins(new StandardEnvironment())
+
+ def pluginManager = new DefaultGrailsPluginManager(application,
discovery)
+ pluginManager.loadPlugins()
assertNotNull pluginManager.getGrailsPlugin("test")
String originalEnv = System.getProperty(Environment.KEY)
try {
System.setProperty(Environment.KEY,
Environment.PRODUCTION.getName())
- pluginManager = new DefaultGrailsPluginManager([test1] as Class[],
application)
+ // Create new application and context for production environment
test
+ application = new DefaultGrailsApplication()
+ appCtx = new GenericApplicationContext()
+ application.setMainContext(appCtx)
Review Comment:
Groovy property setter?
##########
grails-micronaut/src/main/groovy/org/apache/grails/micronaut/GrailsMicronautGrailsPlugin.groovy:
##########
@@ -71,17 +78,18 @@ class GrailsMicronautGrailsPlugin extends Plugin {
[plugins, pluginsFromContext].each { pluginsToProcess ->
pluginsToProcess
.findAll { it.propertySource != null }
- .each {
- log.debug('Loading configurations from {} plugin to
the parent Micronaut context', it.name)
- // If invoking the source as `.source`, the
NavigableMapPropertySource will return null,
- // while invoking the getter, it will return the
correct value
- micronautEnv.addPropertySource(
- PropertySource.of(
- "grails.plugins.$it.name",
- (Map) it.propertySource.getSource(),
- --priority
- )
- )
+ .each { plugin ->
+ // Look up the plugin's property source from the
Spring environment,
+ // where it was loaded by
GrailsEnvironmentPostProcessor
+ String pluginName = plugin.name
+ String ymlSourceName = pluginName + PLUGIN_YML_SUFFIX
+ String groovySourceName = pluginName +
PLUGIN_GROOVY_SUFFIX
+ org.springframework.core.env.PropertySource<?>
springPs =
+ springPropertySources.get(ymlSourceName) ?:
springPropertySources.get(groovySourceName)
+ if (springPs instanceof EnumerablePropertySource) {
+ log.debug('Loading configurations from {} plugin
to the parent Micronaut context', pluginName)
+
micronautEnv.addPropertySource(PropertySource.of("grails.plugins.${pluginName}",
(Map) springPs.getSource(), --priority))
Review Comment:
Use Groovy property getter?
##########
grails-core/src/main/groovy/grails/boot/config/GrailsEnvironmentPostProcessor.java:
##########
Review Comment:
Why not Groovy?
##########
grails-bootstrap/src/main/groovy/grails/plugins/GrailsPluginInfo.java:
##########
@@ -60,7 +60,7 @@ public interface GrailsPluginInfo {
String getFullName();
/**
- * Returns the location of the Resource that represents the plugin
descriptor (the *GrailsPlugin.groovy file)
+ * Returns the location of the Resource that represents the plugin
descriptor (the *GrailsPlugin.groovy or grails-plugin.yml file)
Review Comment:
Isn't the `grails-plugin.yml` created from the `*GrailsPlugin.groovy` file
so this is essentially the same thing.
##########
grails-core/src/test/groovy/org/grails/plugins/GrailsPluginConfigurationClass.groovy:
##########
@@ -50,81 +43,18 @@ class GrailsPluginConfigurationClass extends
GrailsAutoConfiguration {
final String grailsVersion = '4.0.1'
def gcl = new GroovyClassLoader()
- GrailsPlugin plugin = new MockTestGrailsPlugin(gcl.parseClass("""class
TestGrailsPlugin {
+ GrailsPlugin plugin = new DefaultGrailsPlugin(gcl.parseClass("""class
TestGrailsPlugin {
def version = '1.0.0'
def grailsVersion = '$grailsVersion'
def loadAfter = ['testTwo']
}"""), grailsApplication)
- GrailsPlugin plugin2 = new
MockTestTwoGrailsPlugin(gcl.parseClass("""class TestTwoGrailsPlugin {
+ GrailsPlugin plugin2 = new DefaultGrailsPlugin(gcl.parseClass("""class
TestTwoGrailsPlugin {
Review Comment:
`def`?
##########
grails-core/src/test/groovy/org/grails/core/io/ResourceLocatorSpec.groovy:
##########
@@ -18,9 +18,13 @@
*/
package org.grails.core.io
-import groovy.xml.XmlSlurper
+
Review Comment:
Remove empty line?
##########
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:
Refrain from using the get* ideom for non-getters?
##########
grails-core/src/test/groovy/org/grails/plugins/GrailsPluginConfigurationClass.groovy:
##########
@@ -50,81 +43,18 @@ class GrailsPluginConfigurationClass extends
GrailsAutoConfiguration {
final String grailsVersion = '4.0.1'
def gcl = new GroovyClassLoader()
- GrailsPlugin plugin = new MockTestGrailsPlugin(gcl.parseClass("""class
TestGrailsPlugin {
+ GrailsPlugin plugin = new DefaultGrailsPlugin(gcl.parseClass("""class
TestGrailsPlugin {
Review Comment:
`def`?
##########
grails-testing-support-core/src/main/resources/META-INF/services/grails.plugins.GrailsPluginConfigFilter:
##########
Review Comment:
What is this file used for?
##########
grails-core/src/test/groovy/org/grails/plugins/BinaryPluginSpec.groovy:
##########
@@ -72,67 +71,57 @@ class BinaryPluginSpec extends Specification {
cssResource == null
}
- def "Test plugin with both plugin.yml and plugin.groovy throws
exception"() {
+ def "Test getPropertySource returns null when no mainContext is set on
GrailsApplication"() {
when:
- def descriptor = new BinaryGrailsPluginDescriptor(new
ByteArrayResource(testBinary.getBytes('UTF-8')),
['org.grails.plugins.TestBinaryResource'])
- MockConfigBinaryGrailsPlugin.YAML_EXISTS = true
- MockConfigBinaryGrailsPlugin.GROOVY_EXISTS = true
- new MockConfigBinaryGrailsPlugin(descriptor)
+ def descriptor = new GrailsPluginDescriptor(new
ByteArrayResource(testBinary.getBytes('UTF-8')),
['org.grails.plugins.TestBinaryResource'], [])
+ def binaryPlugin = new BinaryGrailsPlugin(TestBinaryGrailsPlugin,
descriptor, new DefaultGrailsApplication())
then:
- thrown(RuntimeException)
+ binaryPlugin.getPropertySource() == null
Review Comment:
Groovy property getter?
##########
grails-core/src/test/groovy/org/grails/plugins/GrailsPluginTests.groovy:
##########
@@ -162,16 +165,36 @@ class TestGrailsPlugin {
''')
DefaultGrailsApplication application = new DefaultGrailsApplication()
- def pluginManager = new DefaultGrailsPluginManager([test1] as Class[],
application)
-
-pluginManager.loadPlugins()
+
+ // Create and set up application context
+ def appCtx = new GenericApplicationContext()
+ application.setMainContext(appCtx)
+
+ // Create discovery bean configured for unit tests
+ GrailsPluginDiscovery discovery = new GrailsPluginDiscovery(new
Class<?>[]{test1})
Review Comment:
`def`?
##########
grails-core/src/main/groovy/org/grails/plugins/MockGrailsPluginManager.java:
##########
@@ -28,14 +28,20 @@
import grails.core.GrailsApplication;
import grails.plugins.GrailsPlugin;
import grails.plugins.exceptions.PluginException;
+import org.apache.grails.core.plugins.GrailsPluginDiscovery;
/**
* @author Graeme Rocher
* @since 0.4
*/
public class MockGrailsPluginManager extends AbstractGrailsPluginManager {
public MockGrailsPluginManager(GrailsApplication application) {
- super(application);
+ super(application, new MockGrailsPluginDiscovery());
+ loadPlugins();
Review Comment:
```suggestion
this(application, new MockGrailsPluginDiscovery());
```
##########
grails-test-suite-uber/src/test/groovy/org/apache/grails/core/plugins/filters/PluginFilterFactoryTests.java:
##########
@@ -16,59 +16,73 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.plugins;
+package org.apache.grails.core.plugins.filters;
+import grails.config.Settings;
import grails.plugins.PluginFilter;
import org.junit.jupiter.api.Test;
import java.util.Set;
+import org.springframework.mock.env.MockEnvironment;
+
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertTrue;
-@SuppressWarnings("rawtypes")
public class PluginFilterFactoryTests {
+ MockEnvironment createEnvironment(String includes, String excludes) {
+ MockEnvironment env = new MockEnvironment();
+ if(includes != null) {
+ env.setProperty(Settings.PLUGIN_INCLUDES, includes);
+ }
+ if(excludes != null) {
+ env.setProperty(Settings.PLUGIN_EXCLUDES, excludes);
+ }
+ return env;
+ }
+
@Test
- public void testIncludeFilterOne() throws Exception {
+ public void testIncludeFilterOne() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter("one", null);
- assertTrue(bean instanceof IncludingPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment("one", null));
+ assertInstanceOf(IncludingPluginFilter.class, bean);
IncludingPluginFilter filter = (IncludingPluginFilter)bean;
- Set suppliedNames = filter.getSuppliedNames();
+ Set<String> suppliedNames = filter.getSuppliedNames();
assertEquals(1, suppliedNames.size());
assertTrue(suppliedNames.contains("one"));
}
@Test
- public void testIncludeFilter() throws Exception {
+ public void testIncludeFilter() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter("one, two", " three , four ");
- assertTrue(bean instanceof IncludingPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment("one, two", "
three , four "));
+ assertInstanceOf(IncludingPluginFilter.class, bean);
IncludingPluginFilter filter = (IncludingPluginFilter)bean;
- Set suppliedNames = filter.getSuppliedNames();
+ Set<String> suppliedNames = filter.getSuppliedNames();
assertEquals(2, suppliedNames.size());
assertTrue(suppliedNames.contains("two"));
}
@Test
- public void testExcludeFilter() throws Exception {
+ public void testExcludeFilter() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter(null, " three , four ");
- assertTrue(bean instanceof ExcludingPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment(null, " three
, four "));
+ assertInstanceOf(ExcludingPluginFilter.class, bean);
ExcludingPluginFilter filter = (ExcludingPluginFilter)bean;
- Set suppliedNames = filter.getSuppliedNames();
+ Set<String> suppliedNames = filter.getSuppliedNames();
Review Comment:
`var` (and also above)?
##########
grails-testing-support-core/src/main/groovy/org/grails/testing/GrailsApplicationBuilder.groovy:
##########
@@ -151,11 +152,20 @@ class GrailsApplicationBuilder {
}
protected void prepareContext(ConfigurableApplicationContext
applicationContext, ConfigurableBeanFactory beanFactory) {
- registerGrailsAppPostProcessorBean(beanFactory)
+ def discovery = registerPluginDiscoveryBean(beanFactory)
+ registerGrailsAppPostProcessorBean(beanFactory, discovery)
AnnotationConfigUtils.registerAnnotationConfigProcessors((BeanDefinitionRegistry)
beanFactory)
new
ConfigDataApplicationContextInitializer().initialize(applicationContext)
}
+ protected GrailsPluginDiscovery
registerPluginDiscoveryBean(ConfigurableBeanFactory beanFactory) {
+ GrailsPluginDiscovery discovery = new GrailsPluginDiscovery()
+ // we must load the classpath since the plugin manager needs to find
the default plugins
+ discovery.setPluginFilter(new IncludingPluginFilter(includePlugins ?:
DEFAULT_INCLUDED_PLUGINS))
Review Comment:
Use Groovy property setter?
##########
grails-core/src/test/groovy/org/apache/grails/core/plugins/GrailsPluginUtilsSpec.groovy:
##########
@@ -0,0 +1,333 @@
+/*
+ * 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 spock.lang.Specification
+import spock.lang.Unroll
+
+import java.net.URLClassLoader
+
+/**
+ * Test suite for GrailsPluginUtils utility methods
+ */
+class GrailsPluginUtilsSpec extends Specification {
+
+ @Unroll
+ def "isPluginVersionCompatible should check that plugin with
grailsVersion=#pluginGrailsVersion is compatible with grails #grailsVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0", // pluginVersion
+ pluginGrailsVersion, // pluginSupportedVersion
+ grailsVersion, // grailsVersion
+ "test-plugin" // pluginDescription
Review Comment:
Single quotes?
##########
grails-testing-support-core/src/main/groovy/org/grails/testing/GrailsApplicationBuilder.groovy:
##########
@@ -151,11 +152,20 @@ class GrailsApplicationBuilder {
}
protected void prepareContext(ConfigurableApplicationContext
applicationContext, ConfigurableBeanFactory beanFactory) {
- registerGrailsAppPostProcessorBean(beanFactory)
+ def discovery = registerPluginDiscoveryBean(beanFactory)
+ registerGrailsAppPostProcessorBean(beanFactory, discovery)
AnnotationConfigUtils.registerAnnotationConfigProcessors((BeanDefinitionRegistry)
beanFactory)
new
ConfigDataApplicationContextInitializer().initialize(applicationContext)
}
+ protected GrailsPluginDiscovery
registerPluginDiscoveryBean(ConfigurableBeanFactory beanFactory) {
+ GrailsPluginDiscovery discovery = new GrailsPluginDiscovery()
+ // we must load the classpath since the plugin manager needs to find
the default plugins
+ discovery.setPluginFilter(new IncludingPluginFilter(includePlugins ?:
DEFAULT_INCLUDED_PLUGINS))
+ (beanFactory as
DefaultListableBeanFactory).registerSingleton(GrailsPluginDiscovery.BEAN_NAME,
discovery)
Review Comment:
Why cast?
##########
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:
`xml` is unused
##########
grails-test-suite-uber/src/test/groovy/org/apache/grails/core/plugins/filters/PluginFilterFactoryTests.java:
##########
@@ -16,59 +16,73 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.plugins;
+package org.apache.grails.core.plugins.filters;
+import grails.config.Settings;
import grails.plugins.PluginFilter;
import org.junit.jupiter.api.Test;
import java.util.Set;
+import org.springframework.mock.env.MockEnvironment;
+
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertTrue;
-@SuppressWarnings("rawtypes")
public class PluginFilterFactoryTests {
+ MockEnvironment createEnvironment(String includes, String excludes) {
+ MockEnvironment env = new MockEnvironment();
Review Comment:
`var`?
##########
grails-test-suite-uber/src/test/groovy/org/grails/commons/GrailsPluginManagerTests.groovy:
##########
@@ -116,12 +117,13 @@ hibernate {
}
void testDefaultGrailsPluginManager() {
- DefaultGrailsPluginManager manager = new
DefaultGrailsPluginManager(RESOURCE_PATH,ga)
- assertEquals(1, manager.getPluginResources().length)
+ def discovery = new GrailsPluginDiscovery(RESOURCE_PATH)
+ DefaultGrailsPluginManager manager = new
DefaultGrailsPluginManager(ga, discovery)
Review Comment:
`manager` is unused and can be removed
##########
grails-test-suite-uber/src/test/groovy/org/apache/grails/core/plugins/filters/PluginFilterFactoryTests.java:
##########
@@ -16,59 +16,73 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.grails.plugins;
+package org.apache.grails.core.plugins.filters;
+import grails.config.Settings;
import grails.plugins.PluginFilter;
import org.junit.jupiter.api.Test;
import java.util.Set;
+import org.springframework.mock.env.MockEnvironment;
+
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertTrue;
-@SuppressWarnings("rawtypes")
public class PluginFilterFactoryTests {
+ MockEnvironment createEnvironment(String includes, String excludes) {
+ MockEnvironment env = new MockEnvironment();
+ if(includes != null) {
+ env.setProperty(Settings.PLUGIN_INCLUDES, includes);
+ }
+ if(excludes != null) {
+ env.setProperty(Settings.PLUGIN_EXCLUDES, excludes);
+ }
+ return env;
+ }
+
@Test
- public void testIncludeFilterOne() throws Exception {
+ public void testIncludeFilterOne() {
PluginFilterRetriever fb = new PluginFilterRetriever();
- PluginFilter bean = fb.getPluginFilter("one", null);
- assertTrue(bean instanceof IncludingPluginFilter);
+ PluginFilter bean = fb.getPluginFilter(createEnvironment("one", null));
+ assertInstanceOf(IncludingPluginFilter.class, bean);
IncludingPluginFilter filter = (IncludingPluginFilter)bean;
- Set suppliedNames = filter.getSuppliedNames();
+ Set<String> suppliedNames = filter.getSuppliedNames();
Review Comment:
`var` (and also above)?
##########
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)
def resource = new MockBinaryPluginResource(str.bytes)
- def descriptor = new BinaryGrailsPluginDescriptor(resource,
['org.grails.plugins.TestBinaryGrailsPlugin'])
+ def descriptor = new GrailsPluginDescriptor(resource,
['org.grails.plugins.TestBinaryGrailsPlugin'], [])
resource.relativesResources['static/css/main.css'] = new
ByteArrayResource(''.bytes)
- def binaryPlugin = new BinaryGrailsPlugin(TestBinaryGrailsPlugin,
descriptor, new DefaultGrailsApplication())
+
+ def grailsApp = new DefaultGrailsApplication()
+ def appCtx = new
org.springframework.context.support.GenericApplicationContext()
+ grailsApp.setMainContext(appCtx)
Review Comment:
- Use Groovy property getter?
- Inline `new GenericApplicationContext()`
- Import `org.springframework.context.support.GenericApplicationContext`
##########
grails-core/src/test/groovy/org/apache/grails/core/plugins/GrailsPluginUtilsSpec.groovy:
##########
@@ -0,0 +1,333 @@
+/*
+ * 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 spock.lang.Specification
+import spock.lang.Unroll
+
+import java.net.URLClassLoader
+
+/**
+ * Test suite for GrailsPluginUtils utility methods
+ */
+class GrailsPluginUtilsSpec extends Specification {
+
+ @Unroll
+ def "isPluginVersionCompatible should check that plugin with
grailsVersion=#pluginGrailsVersion is compatible with grails #grailsVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0", // pluginVersion
+ pluginGrailsVersion, // pluginSupportedVersion
+ grailsVersion, // grailsVersion
+ "test-plugin" // pluginDescription
+ )
+
+ then:
+ compatible == expectedCompatible
+
+ where:
+ grailsVersion | pluginGrailsVersion || expectedCompatible
+ "1.0" | "3.3.1 > *" || false
+ "2.5" | "3.0.1" || false
+ "3.0.0" | "3.3.10 > *" || false
+ "3.3.10" | "4.0.0 > *" || false
+ "4.0.1" | "3.0.0.BUILD-SNAPSHOT > *" || true
+ "4.0.1" | "4.0.1" || true
+ "4.0.1" | "3.0.1" || false
+ "4.0.1" | "3.3.1 > *" || true
+ "4.0.1" | "3.3.10 > *" || true
+ }
+
+ def "isPluginVersionCompatible should handle null grailsVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0",
+ "3.3.10 > *",
+ null, // grailsVersion is null
+ "test-plugin"
Review Comment:
Single quotes?
##########
grails-core/src/test/groovy/org/apache/grails/core/plugins/GrailsPluginUtilsSpec.groovy:
##########
@@ -0,0 +1,333 @@
+/*
+ * 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 spock.lang.Specification
+import spock.lang.Unroll
+
+import java.net.URLClassLoader
+
+/**
+ * Test suite for GrailsPluginUtils utility methods
+ */
+class GrailsPluginUtilsSpec extends Specification {
+
+ @Unroll
+ def "isPluginVersionCompatible should check that plugin with
grailsVersion=#pluginGrailsVersion is compatible with grails #grailsVersion"() {
+ when:
+ def compatible = GrailsPluginUtils.isPluginVersionCompatible(
+ "1.0.0", // pluginVersion
+ pluginGrailsVersion, // pluginSupportedVersion
+ grailsVersion, // grailsVersion
+ "test-plugin" // pluginDescription
+ )
+
+ then:
+ compatible == expectedCompatible
+
+ where:
+ grailsVersion | pluginGrailsVersion || expectedCompatible
+ "1.0" | "3.3.1 > *" || false
+ "2.5" | "3.0.1" || false
+ "3.0.0" | "3.3.10 > *" || false
+ "3.3.10" | "4.0.0 > *" || false
+ "4.0.1" | "3.0.0.BUILD-SNAPSHOT > *" || true
+ "4.0.1" | "4.0.1" || true
+ "4.0.1" | "3.0.1" || false
+ "4.0.1" | "3.3.1 > *" || true
+ "4.0.1" | "3.3.10 > *" || true
Review Comment:
Single quotes?
##########
grails-core/src/main/groovy/org/grails/plugins/AbstractGrailsPluginManager.java:
##########
@@ -124,10 +144,8 @@ protected void checkInitialised() {
}
public GrailsPlugin getFailedPlugin(String name) {
- if (name.indexOf('-') > -1) {
- name =
GrailsNameUtils.getPropertyNameForLowerCaseHyphenSeparatedName(name);
- }
- return failedPlugins.get(name);
+ GrailsPluginInfo metadata =
pluginDiscovery.getFailedPlugins().get(GrailsPluginUtils.normalizePluginName(name));
Review Comment:
`var`?
##########
grails-core/src/main/groovy/grails/plugins/Plugin.groovy:
##########
@@ -75,7 +75,7 @@ abstract class Plugin implements GrailsApplicationLifeCycle,
GrailsApplicationAw
/**
* Whether the plugin is enabled
*/
- boolean enabled = true
+ public boolean enabled = true
Review Comment:
Why `public`?
##########
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 {
Review Comment:
Why `final`?
##########
grails-core/src/main/groovy/grails/plugins/GrailsPluginSorter.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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 grails.plugins;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+
+/**
+ * Canonical topological sort for Grails plugins based on {@code loadAfter}
+ * and {@code loadBefore} declarations.
+ *
+ * <p>This class provides the single shared implementation of plugin ordering
that
+ * is used by both {@link DefaultGrailsPluginManager} (at runtime, operating on
+ * {@link GrailsPlugin} instances) and
+ * {@link GrailsEnvironmentPostProcessor} (early in the
Review Comment:
Use full path?
##########
grails-core/src/main/groovy/org/grails/plugins/AbstractGrailsPlugin.java:
##########
@@ -93,32 +80,43 @@ public AbstractGrailsPlugin(Class<?> pluginClass,
GrailsApplication application)
"] is not a Grails plugin (class name must end with
'GrailsPlugin')");
this.grailsApplication = application;
this.pluginClass = pluginClass;
- Resource resource = readPluginConfiguration(pluginClass);
-
- if (resource != null && resource.exists()) {
- final String filename = resource.getFilename();
- try {
- if (filename.equals(PLUGIN_YML)) {
- YamlPropertySourceLoader propertySourceLoader = new
YamlPropertySourceLoader();
- this.propertySource =
propertySourceLoader.load(GrailsNameUtils.getLogicalPropertyName(pluginClass.getSimpleName(),
"GrailsPlugin") + "-" + PLUGIN_YML, resource,
DEFAULT_CONFIG_IGNORE_LIST).stream().findFirst().orElse(null);
- } else if (filename.equals(PLUGIN_GROOVY)) {
- GroovyConfigPropertySourceLoader propertySourceLoader =
new GroovyConfigPropertySourceLoader();
- this.propertySource =
propertySourceLoader.load(GrailsNameUtils.getLogicalPropertyName(pluginClass.getSimpleName(),
"GrailsPlugin") + "-" + PLUGIN_GROOVY, resource,
DEFAULT_CONFIG_IGNORE_LIST).stream().findFirst().orElse(null);
- }
- } catch (IOException e) {
- LOG.warn("Error loading " + filename + " for plugin: " +
pluginClass.getName() + ": " + e.getMessage(), e);
- }
- }
}
+ /**
+ * Retrieves the plugin's property source from the Spring {@link
ConfigurableEnvironment}.
+ *
+ * <p>Plugin configuration files ({@code plugin.yml} or {@code
plugin.groovy}) are loaded
+ * early in the application lifecycle by
+ * {@link grails.boot.config.GrailsEnvironmentPostProcessor} and
registered as named
+ * property sources in the environment. This method looks up the property
source by the
+ * expected name ({@code "<pluginName>-plugin.yml"} or {@code
"<pluginName>-plugin.groovy"}).</p>
+ *
+ * @return the plugin's property source, or {@code null} if no
configuration was loaded
+ * or the application context is not yet available
+ */
@Override
+ @Deprecated(forRemoval = true)
Review Comment:
since?
##########
grails-core/src/main/groovy/org/apache/grails/core/GrailsBootstrapRegistryInitializer.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.springframework.boot.BootstrapRegistry;
+import org.springframework.boot.BootstrapRegistryInitializer;
+
+import org.apache.grails.core.plugins.GrailsPluginDiscovery;
+
+/**
+ * Registers the {@link GrailsPluginDiscovery} in the Spring Boot Bootstrap
context so it can be accessed during
+ * the early lifecycle of the application & later promoted to actual bean.
+ *
+ * <p>This ensures that both the early-lifecycle
+ * {@link GrailsEnvironmentPostProcessor} and the
Review Comment:
Use full path?
##########
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) {
Review Comment:
Why `package-private`?
##########
grails-core/src/test/groovy/org/apache/grails/core/plugins/GrailsPluginDiscoverySpec.groovy:
##########
@@ -0,0 +1,303 @@
+/*
+ * 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 spock.lang.Specification
+
+class GrailsPluginDiscoverySpec extends Specification {
+
+ def "scanPluginDescriptorResources discovers descriptors with rich info
from classpath"() {
+ when:
+ def descriptors = GrailsPluginUtils.scanPluginDescriptorResources(
+ Thread.currentThread().getContextClassLoader())
Review Comment:
Groovy property getter?
##########
grails-core/src/main/groovy/grails/plugins/GrailsPlugin.java:
##########
@@ -263,6 +263,20 @@ public interface GrailsPlugin extends
ApplicationContextAware, Comparable, Grail
*/
String getDependentVersion(String name);
+ /**
+ * Returns the plugin's configuration as a {@link PropertySource}, looked
up from the
+ * Spring {@link org.springframework.core.env.Environment} by naming
convention.
+ *
+ * <p>Plugin configuration is loaded early in the Spring Boot lifecycle by
+ * {@code GrailsEnvironmentPostProcessor} and registered as a
+ * property source named {@code <pluginName>-plugin.yml} or {@code
<pluginName>-plugin.groovy}.
+ * This method retrieves that property source from the application
context's environment.</p>
+ *
+ * @deprecated no longer used to populate plugin-specific configuration,
see {@code GrailsEnvironmentPostProcessor}
+ * @return the plugin's property source from the environment, or {@code
null} if the
+ * GrailsApplication main context is not yet available or no
matching property source exists
+ */
+ @Deprecated(forRemoval = true)
Review Comment:
`since`?
##########
grails-core/src/test/groovy/grails/plugins/DefaultGrailsPluginManagerSpec.groovy:
##########
@@ -19,79 +19,40 @@
package grails.plugins
import grails.core.GrailsApplication
-import grails.util.Metadata
-import org.apache.commons.logging.Log
-import org.grails.plugins.DefaultGrailsPlugin
+import org.apache.grails.core.plugins.GrailsPluginDiscovery
import spock.lang.Specification
-import spock.lang.Unroll
+/**
+ * Test suite for DefaultGrailsPluginManager
+ */
class DefaultGrailsPluginManagerSpec extends Specification {
- @Unroll
- def "should return #pluginGrailsVersion as plugin grails version"() {
+ def "plugin manager can be created with application and discovery bean"() {
given:
- GrailsApplication app = stubGrailsApplicationWithVersion("4.0.1")
- DefaultGrailsPluginManager sut = buildPluginsManager(app)
- DefaultGrailsPlugin plugin = stubPluginWithGrailsVersion(app,
pluginGrailsVersion)
+ GrailsApplication app = Mock(GrailsApplication)
+ GrailsPluginDiscovery discovery = Mock(GrailsPluginDiscovery)
when:
- def version = sut.getPluginGrailsVersion(plugin)
+ DefaultGrailsPluginManager manager = new
DefaultGrailsPluginManager(app, discovery)
Review Comment:
`def` x 3
##########
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?
##########
grails-core/src/main/groovy/grails/plugins/GrailsPluginSorter.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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 grails.plugins;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+
+/**
+ * Canonical topological sort for Grails plugins based on {@code loadAfter}
+ * and {@code loadBefore} declarations.
+ *
+ * <p>This class provides the single shared implementation of plugin ordering
that
+ * is used by both {@link DefaultGrailsPluginManager} (at runtime, operating on
+ * {@link GrailsPlugin} instances) and
+ * {@link GrailsEnvironmentPostProcessor} (early in the
+ * lifecycle, operating on lightweight {@code PluginInfo} records before the
+ * ApplicationContext is available).</p>
+ *
+ * <p>The algorithm is a DFS-based topological sort:</p>
+ * <ol>
+ * <li>{@code loadAfter} creates edges from a plugin to its
+ * dependencies (the plugin must load <em>after</em> the named
plugins).</li>
+ * <li>{@code loadBefore} creates reverse edges (the <em>named</em> plugin
must
+ * load after <em>this</em> plugin).</li>
+ * <li>A recursive DFS visits dependencies first, then adds the current node,
+ * producing a valid topological order.</li>
+ * </ol>
+ *
+ * <p><strong>Note:</strong> {@code dependsOn} is intentionally <em>not</em>
used
+ * for ordering. In the original {@link DefaultGrailsPluginManager}, {@code
dependsOn}
+ * is only used for dependency <em>resolution</em> (checking that required
plugins
+ * are present), not for determining load order. Load order is controlled
exclusively
+ * by {@code loadAfter} and {@code loadBefore}.</p>
+ *
+ * <p>The sort is generic: callers provide accessor functions so the same
algorithm
+ * works with any plugin representation (full {@link GrailsPlugin} objects,
lightweight
+ * metadata records, etc.).</p>
+ *
+ * @since 7.1
+ */
+public final class GrailsPluginSorter {
+
+ private GrailsPluginSorter() {
+ // utility class
+ }
+
+ /**
+ * Performs a topological sort of the given plugins.
+ *
+ * <p>This is the single canonical sort algorithm used across the
framework to
+ * ensure that plugin load order is identical whether plugins are loaded
early
+ * (by the {@code EnvironmentPostProcessor} for configuration) or later
(by the
+ * {@code DefaultGrailsPluginManager} for full lifecycle).</p>
+ *
+ * @param <T> the plugin type
+ * @param plugins the plugins to sort
+ * @param nameExtractor extracts the logical plugin name from a plugin
+ * @param loadAfterExtractor extracts the {@code loadAfter} names from a
plugin
+ * @param loadBeforeExtractor extracts the {@code loadBefore} names from a
plugin
+ * @param pluginLookup resolves a plugin by its logical name (returns
{@code null}
+ * if the named plugin is not present)
+ * @return a new list with the plugins in topological order
+ */
+ public static <T> List<T> sort(
+ List<T> plugins,
+ Function<T, String> nameExtractor,
Review Comment:
`nameExtractor` is unused
--
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]
