You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@grails.apache.org by ja...@apache.org on 2026/03/19 03:04:38 UTC

(grails-core) branch bridge-backward-compat-shims updated (1f2507bb4e -> ea63dd2694)

This is an automated email from the ASF dual-hosted git repository.

jamesfredley pushed a change to branch bridge-backward-compat-shims
in repository https://gitbox.apache.org/repos/asf/grails-core.git


    from 1f2507bb4e Make deprecated plugin manager shims call reset/init on discovery
     new a3a4c318ee Use plugins map as init guard, add reset warning log
     new ea63dd2694 Harden bridge shims to fail fast instead of silently degrading

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../config/GrailsApplicationPostProcessor.groovy   | 25 +++++---
 .../grails/plugins/DefaultGrailsPluginManager.java | 35 +++++++----
 .../core/plugins/DefaultGrailsPluginDiscovery.java |  9 ++-
 .../grails/plugins/MockGrailsPluginDiscovery.java  | 24 ++++----
 .../org/grails/plugins/PluginFilterRetriever.java  | 68 ++++------------------
 5 files changed, 70 insertions(+), 91 deletions(-)


(grails-core) 02/02: Harden bridge shims to fail fast instead of silently degrading

Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jamesfredley pushed a commit to branch bridge-backward-compat-shims
in repository https://gitbox.apache.org/repos/asf/grails-core.git

commit ea63dd26940dada566ca4aa9bd53043bb1e4b851
Author: James Fredley <ja...@users.noreply.github.com>
AuthorDate: Wed Mar 18 23:04:16 2026 -0400

    Harden bridge shims to fail fast instead of silently degrading
    
    Address review feedback to make deprecated bridge shims throw
    clear errors when misused, rather than silently creating duplicate
    instances or behaving differently from the new API.
    
    - GrailsApplicationPostProcessor: resolvePluginDiscovery() throws
      IllegalStateException if the discovery bean is not in the context
      instead of falling back to a new DefaultGrailsPluginDiscovery
    - GrailsApplicationPostProcessor: customizePluginManager() throws
      UnsupportedOperationException instead of being a silent no-op
    - DefaultGrailsPluginManager: deprecated constructors now fetch the
      existing discovery bean from GrailsApplication.getMainContext(),
      call reset()/init() to reinitialize, and log a warning instead
      of creating a new unmanaged DefaultGrailsPluginDiscovery instance
    - PluginFilterRetriever: getPluginFilter(Config) throws
      UnsupportedOperationException like CorePluginFinder, since
      Environment always has all configuration
    
    Assisted-by: Claude Code <Cl...@Claude.ai>
---
 .../config/GrailsApplicationPostProcessor.groovy   | 25 +++++---
 .../grails/plugins/DefaultGrailsPluginManager.java | 35 +++++++----
 .../org/grails/plugins/PluginFilterRetriever.java  | 68 ++++------------------
 3 files changed, 53 insertions(+), 75 deletions(-)

diff --git a/grails-core/src/main/groovy/grails/boot/config/GrailsApplicationPostProcessor.groovy b/grails-core/src/main/groovy/grails/boot/config/GrailsApplicationPostProcessor.groovy
index 298ffc5bde..c33543da21 100644
--- a/grails-core/src/main/groovy/grails/boot/config/GrailsApplicationPostProcessor.groovy
+++ b/grails-core/src/main/groovy/grails/boot/config/GrailsApplicationPostProcessor.groovy
@@ -109,23 +109,34 @@ class GrailsApplicationPostProcessor implements BeanDefinitionRegistryPostProces
     }
 
     /**
-     * Resolves the {@link GrailsPluginDiscovery} from the application context if available,
-     * otherwise creates a new default instance. The bootstrap registry promotes the discovery
-     * bean before context refresh, so it is safe to look up by bean name at this stage.
+     * Resolves the {@link GrailsPluginDiscovery} from the application context.
+     * The bootstrap registry promotes the discovery bean before context refresh,
+     * so it is always available by bean name at this stage.
+     *
+     * @throws IllegalStateException if the application context is null or does not contain a GrailsPluginDiscovery bean
      */
     private static GrailsPluginDiscovery resolvePluginDiscovery(ApplicationContext applicationContext) {
-        if (applicationContext != null && applicationContext.containsBean(GrailsPluginDiscovery.BEAN_NAME)) {
-            return (GrailsPluginDiscovery) applicationContext.getBean(GrailsPluginDiscovery.BEAN_NAME)
+        if (applicationContext == null || !applicationContext.containsBean(GrailsPluginDiscovery.BEAN_NAME)) {
+            throw new IllegalStateException(
+                    'GrailsPluginDiscovery bean not found in ApplicationContext. ' +
+                    'Use GrailsApplicationPostProcessor(GrailsApplicationLifeCycle, ApplicationContext, GrailsPluginDiscovery, Class[]) instead.'
+            )
         }
-        return new org.apache.grails.core.plugins.DefaultGrailsPluginDiscovery()
+        return (GrailsPluginDiscovery) applicationContext.getBean(GrailsPluginDiscovery.BEAN_NAME)
     }
 
     /**
      * @deprecated Plugin manager customization is now handled through {@link org.apache.grails.core.plugins.GrailsPluginDiscovery}.
-     * This method is a no-op and will be removed in Grails 8.0.0.
+     * This method will be removed in Grails 8.0.0.
+     *
+     * @throws UnsupportedOperationException always - callers must switch to {@link org.apache.grails.core.plugins.GrailsPluginDiscovery}
      */
     @Deprecated(forRemoval = true, since = "7.1")
     protected void customizePluginManager(GrailsPluginManager pluginManager) {
+        throw new UnsupportedOperationException(
+                'customizePluginManager() is no longer supported. ' +
+                'Use org.apache.grails.core.plugins.GrailsPluginDiscovery to configure plugin discovery instead.'
+        )
     }
 
     protected final void initializeGrailsApplication(ApplicationContext applicationContext) {
diff --git a/grails-core/src/main/groovy/grails/plugins/DefaultGrailsPluginManager.java b/grails-core/src/main/groovy/grails/plugins/DefaultGrailsPluginManager.java
index 7dc2d3adec..8b467b92c9 100644
--- a/grails-core/src/main/groovy/grails/plugins/DefaultGrailsPluginManager.java
+++ b/grails-core/src/main/groovy/grails/plugins/DefaultGrailsPluginManager.java
@@ -41,7 +41,6 @@ import org.springframework.core.io.Resource;
 import grails.core.GrailsApplication;
 import grails.core.support.ParentApplicationContextAware;
 import grails.plugins.exceptions.PluginException;
-import org.apache.grails.core.plugins.DefaultGrailsPluginDiscovery;
 import org.apache.grails.core.plugins.GrailsPluginDescriptor;
 import org.apache.grails.core.plugins.GrailsPluginDiscovery;
 import org.apache.grails.core.plugins.GrailsPluginInfo;
@@ -96,32 +95,48 @@ public class DefaultGrailsPluginManager extends AbstractGrailsPluginManager {
 
     /**
      * @deprecated Use {@link #DefaultGrailsPluginManager(GrailsApplication, GrailsPluginDiscovery)} instead.
-     * Plugin discovery is now handled by {@link GrailsPluginDiscovery}. This constructor forwards
-     * the resource path to the discovery instance. Will be removed in Grails 8.0.0.
+     * Plugin discovery is now handled by {@link GrailsPluginDiscovery}. This constructor fetches the
+     * existing discovery bean, resets and reinitializes it. Will be removed in Grails 8.0.0.
      */
     @Deprecated(forRemoval = true, since = "7.1")
     public DefaultGrailsPluginManager(String resourcePath, GrailsApplication application) {
-        this(application, new DefaultGrailsPluginDiscovery(resourcePath));
+        this(application, resolveAndReinitializeDiscovery(application));
     }
 
     /**
      * @deprecated Use {@link #DefaultGrailsPluginManager(GrailsApplication, GrailsPluginDiscovery)} instead.
-     * Plugin discovery is now handled by {@link GrailsPluginDiscovery}. This constructor forwards
-     * the plugin resources to the discovery instance. Will be removed in Grails 8.0.0.
+     * Plugin discovery is now handled by {@link GrailsPluginDiscovery}. This constructor fetches the
+     * existing discovery bean, resets and reinitializes it. Will be removed in Grails 8.0.0.
      */
     @Deprecated(forRemoval = true, since = "7.1")
     public DefaultGrailsPluginManager(String[] pluginResources, GrailsApplication application) {
-        this(application, new DefaultGrailsPluginDiscovery(pluginResources));
+        this(application, resolveAndReinitializeDiscovery(application));
     }
 
     /**
      * @deprecated Use {@link #DefaultGrailsPluginManager(GrailsApplication, GrailsPluginDiscovery)} instead.
-     * Plugin discovery is now handled by {@link GrailsPluginDiscovery}. This constructor forwards
-     * the plugin classes to the discovery instance. Will be removed in Grails 8.0.0.
+     * Plugin discovery is now handled by {@link GrailsPluginDiscovery}. This constructor fetches the
+     * existing discovery bean, resets and reinitializes it. Will be removed in Grails 8.0.0.
      */
     @Deprecated(forRemoval = true, since = "7.1")
     public DefaultGrailsPluginManager(Class<?>[] plugins, GrailsApplication application) {
-        this(application, new DefaultGrailsPluginDiscovery(plugins));
+        this(application, resolveAndReinitializeDiscovery(application));
+    }
+
+    /**
+     * Resolves the {@link GrailsPluginDiscovery} bean from the application context,
+     * resets and reinitializes it. The GrailsApplication always has an application context
+     * set when these deprecated constructors are called.
+     */
+    private static GrailsPluginDiscovery resolveAndReinitializeDiscovery(GrailsApplication application) {
+        ApplicationContext ctx = application.getMainContext();
+        GrailsPluginDiscovery discovery = (GrailsPluginDiscovery) ctx.getBean(GrailsPluginDiscovery.BEAN_NAME);
+        LOG.warn("Using deprecated DefaultGrailsPluginManager constructor. " +
+                "Plugin discovery should be configured through the GrailsPluginDiscovery bean. " +
+                "Reinitializing plugin discovery.");
+        discovery.reset();
+        discovery.init(ctx.getEnvironment());
+        return discovery;
     }
 
     public GrailsPlugin[] getUserPlugins() {
diff --git a/grails-core/src/main/groovy/org/grails/plugins/PluginFilterRetriever.java b/grails-core/src/main/groovy/org/grails/plugins/PluginFilterRetriever.java
index fb09be4f2b..7d18a13ce3 100644
--- a/grails-core/src/main/groovy/org/grails/plugins/PluginFilterRetriever.java
+++ b/grails-core/src/main/groovy/org/grails/plugins/PluginFilterRetriever.java
@@ -18,76 +18,28 @@
  */
 package org.grails.plugins;
 
-import java.util.Collection;
-import java.util.LinkedHashSet;
-import java.util.Set;
-
-import org.springframework.core.env.Environment;
-import org.springframework.util.StringUtils;
-
 import grails.config.Config;
-import grails.config.Settings;
 import grails.plugins.PluginFilter;
 
 /**
  * @deprecated Use {@link org.apache.grails.core.plugins.filters.PluginFilterRetriever} instead.
- * This compatibility bridge will be removed in Grails 8.0.0.
+ * This compatibility stub will be removed in Grails 8.0.0.
  */
 @Deprecated(forRemoval = true, since = "7.1")
 public class PluginFilterRetriever extends org.apache.grails.core.plugins.filters.PluginFilterRetriever {
 
-    /**
-     * @deprecated Use {@link org.apache.grails.core.plugins.filters.PluginFilterRetriever#getPluginFilter(Environment)} instead.
-     */
-    @Deprecated(forRemoval = true, since = "7.1")
-    public PluginFilter getPluginFilter(Config config) {
-        if (config == null) {
-            throw new IllegalArgumentException("Config should not be null");
-        }
-
-        if (config instanceof Environment environment) {
-            return super.getPluginFilter(environment);
-        }
-
-        Object includes = config.getProperty(Settings.PLUGIN_INCLUDES, Object.class, null);
-        Object excludes = config.getProperty(Settings.PLUGIN_EXCLUDES, Object.class, null);
-        return getPluginFilter(includes, excludes);
-    }
+    private static final String UNSUPPORTED_MESSAGE =
+            "PluginFilterRetriever.getPluginFilter(Config) is no longer supported. " +
+            "Use org.apache.grails.core.plugins.filters.PluginFilterRetriever.getPluginFilter(Environment) instead.";
 
     /**
-     * @deprecated Use {@link org.apache.grails.core.plugins.filters.PluginFilterRetriever#getPluginFilter(Environment)} instead.
+     * @deprecated Use {@link org.apache.grails.core.plugins.filters.PluginFilterRetriever#getPluginFilter(org.springframework.core.env.Environment)} instead.
+     * The Environment always has all configuration, so it is adaptable for all use cases.
+     *
+     * @throws UnsupportedOperationException always
      */
     @Deprecated(forRemoval = true, since = "7.1")
-    PluginFilter getPluginFilter(Object includes, Object excludes) {
-        if (includes != null) {
-            if (includes instanceof Collection<?> includesCollection) {
-                return new org.apache.grails.core.plugins.filters.IncludingPluginFilter(toSet(includesCollection));
-            }
-            return new org.apache.grails.core.plugins.filters.IncludingPluginFilter(StringUtils.commaDelimitedListToStringArray(includes.toString()));
-        }
-
-        if (excludes != null) {
-            if (excludes instanceof Collection<?> excludesCollection) {
-                return new org.apache.grails.core.plugins.filters.ExcludingPluginFilter(toSet(excludesCollection));
-            }
-            return new org.apache.grails.core.plugins.filters.ExcludingPluginFilter(StringUtils.commaDelimitedListToStringArray(excludes.toString()));
-        }
-
-        return new org.apache.grails.core.plugins.filters.NoOpPluginFilter();
-    }
-
-    private static Set<String> toSet(Collection<?> values) {
-        var set = new LinkedHashSet<String>();
-        for (Object v : values) {
-            if (v == null) {
-                continue;
-            }
-
-            String s = v.toString().trim();
-            if (!s.isEmpty()) {
-                set.add(s);
-            }
-        }
-        return set;
+    public PluginFilter getPluginFilter(Config config) {
+        throw new UnsupportedOperationException(UNSUPPORTED_MESSAGE);
     }
 }


(grails-core) 01/02: Use plugins map as init guard, add reset warning log

Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jamesfredley pushed a commit to branch bridge-backward-compat-shims
in repository https://gitbox.apache.org/repos/asf/grails-core.git

commit a3a4c318ee8626d44cd65d2174e2bd3d7d4b07cc
Author: James Fredley <ja...@users.noreply.github.com>
AuthorDate: Wed Mar 18 23:04:03 2026 -0400

    Use plugins map as init guard, add reset warning log
    
    Remove the separate 'initialized' boolean flag from
    DefaultGrailsPluginDiscovery. The plugins map being null now
    indicates uninitialized state, which is simpler and avoids
    redundant tracking.
    
    - init() guards on plugins == null instead of !initialized
    - reset() sets plugins = null so init() can re-run after reset
    - reset() logs a warning so users know plugins are being reloaded
    - populatePlugins() initializes plugins to a new map after the
      internal reset() call
    - MockGrailsPluginDiscovery: remove init() no-op override, let
      parent booleans control classpath scanning behavior
    - MockGrailsPluginDiscovery: initPluginsIfNotDefined() initializes
      all collections directly instead of calling reset()
    
    Assisted-by: Claude Code <Cl...@Claude.ai>
---
 .../core/plugins/DefaultGrailsPluginDiscovery.java |  9 ++++----
 .../grails/plugins/MockGrailsPluginDiscovery.java  | 24 ++++++++++++----------
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultGrailsPluginDiscovery.java b/grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultGrailsPluginDiscovery.java
index 02c7a94f55..12bf879374 100644
--- a/grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultGrailsPluginDiscovery.java
+++ b/grails-core/src/main/groovy/org/apache/grails/core/plugins/DefaultGrailsPluginDiscovery.java
@@ -53,7 +53,6 @@ public class DefaultGrailsPluginDiscovery implements GrailsPluginDiscovery {
     protected boolean loadClasspathPlugins = true;
     protected boolean requireClasspathPlugin = true;
     protected final PluginFilterRetriever filterRetriever;
-    private boolean initialized = false;
 
     public DefaultGrailsPluginDiscovery() {
         this(new PluginFilterRetriever());
@@ -110,7 +109,7 @@ public class DefaultGrailsPluginDiscovery implements GrailsPluginDiscovery {
     }
 
     public void init(Environment environment) {
-        if (!initialized) {
+        if (plugins == null) {
             if (environment == null) {
                 throw new IllegalArgumentException("Environment must be provided to determine plugin order");
             }
@@ -246,6 +245,7 @@ public class DefaultGrailsPluginDiscovery implements GrailsPluginDiscovery {
         List<GrailsPluginInfo> filteredPlugins = filterPlugins(allPlugins, environment);
 
         reset();
+        plugins = new LinkedHashMap<>();
 
         if (filteredPlugins.isEmpty()) {
             LOG.debug("All plugins were excluded by plugin filtering");
@@ -268,7 +268,6 @@ public class DefaultGrailsPluginDiscovery implements GrailsPluginDiscovery {
                 GrailsPluginInfo::loadAfterNames,
                 GrailsPluginInfo::loadBeforeNames
         );
-        initialized = true;
     }
 
     private void processDelayedEvictions() {
@@ -554,8 +553,8 @@ public class DefaultGrailsPluginDiscovery implements GrailsPluginDiscovery {
 
     @Override
     public void reset() {
-        initialized = false;
-        plugins = new LinkedHashMap<>();
+        LOG.warn("Resetting plugin discovery - plugins will be reloaded on next init()");
+        plugins = null;
         loadOrderedPlugins = new ArrayList<>();
         pluginToObserverMap = new HashMap<>();
         delayedLoadPlugins = new LinkedList<>();
diff --git a/grails-core/src/main/groovy/org/grails/plugins/MockGrailsPluginDiscovery.java b/grails-core/src/main/groovy/org/grails/plugins/MockGrailsPluginDiscovery.java
index 10930e3bed..c5a590875f 100644
--- a/grails-core/src/main/groovy/org/grails/plugins/MockGrailsPluginDiscovery.java
+++ b/grails-core/src/main/groovy/org/grails/plugins/MockGrailsPluginDiscovery.java
@@ -18,31 +18,27 @@
  */
 package org.grails.plugins;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.LinkedList;
+
 import grails.plugins.GrailsPlugin;
 import org.apache.grails.core.plugins.DefaultGrailsPluginDiscovery;
 import org.apache.grails.core.plugins.GrailsPluginInfo;
 import org.apache.grails.core.plugins.GrailsPluginUtils;
-import org.springframework.core.env.Environment;
 
 public class MockGrailsPluginDiscovery extends DefaultGrailsPluginDiscovery {
 
     public MockGrailsPluginDiscovery() {
         super();
-        reset(); // do not search on the classpath by default
+        initPluginsIfNotDefined();
     }
 
     public MockGrailsPluginDiscovery(Class<?>[] pluginClasses) {
         super(pluginClasses);
     }
 
-    /**
-     * No-op: mock discovery does not scan the classpath.
-     * Plugins are registered manually via {@link #registerMockPlugin}.
-     */
-    @Override
-    public void init(Environment environment) {
-    }
-
     public void registerMockPlugin(GrailsPlugin plugin) {
         registerMockPlugin(GrailsPluginUtils.createPluginInfo(plugin.getPluginClass(), null, true));
     }
@@ -61,7 +57,13 @@ public class MockGrailsPluginDiscovery extends DefaultGrailsPluginDiscovery {
 
     private void initPluginsIfNotDefined() {
         if (plugins == null) {
-            reset();
+            plugins = new LinkedHashMap<>();
+            loadOrderedPlugins = new ArrayList<>();
+            orderedPlugins = new ArrayList<>();
+            pluginToObserverMap = new HashMap<>();
+            delayedLoadPlugins = new LinkedList<>();
+            failedPlugins = new HashMap<>();
+            delayedEvictions = new HashMap<>();
         }
     }
 }