codeconsole commented on code in PR #15666:
URL: https://github.com/apache/grails-core/pull/15666#discussion_r3653346327
##########
grails-core/src/main/groovy/org/apache/grails/core/GrailsBootstrapRegistryInitializer.java:
##########
@@ -62,5 +78,60 @@ public void initialize(BootstrapRegistry registry) {
.registerSingleton(PluginDiscovery.BEAN_NAME, discovery);
LOG.debug("Promoted GrailsPluginDiscovery to ApplicationContext as
'{}'", PluginDiscovery.BEAN_NAME);
});
+
+ // Resolve the configured StackTraceFilterer from the environment
(same two keys
+ // GrailsExceptionResolver honours) and install + promote it the same
way, before refresh().
+ registry.addCloseListener(event -> {
+ ConfigurableApplicationContext applicationContext =
event.getApplicationContext();
+ StackTraceFilterer filterer =
resolveConfiguredStackTraceFilterer(applicationContext.getEnvironment());
+ GrailsUtil.initializeStackFilterer(filterer);
+ applicationContext.getBeanFactory()
+ .registerSingleton(StackTraceFilterer.BEAN_NAME, filterer);
+ LOG.debug("Promoted StackTraceFilterer to ApplicationContext as
'{}'", StackTraceFilterer.BEAN_NAME);
+ });
+ }
+
+ /**
+ * Resolves a {@link StackTraceFilterer} from the given environment,
honouring
+ * {@link Settings#SETTING_LOGGING_STACKTRACE_FILTER_CLASS} and
+ * {@link Settings#SETTING_LOG_FULL_STACKTRACE_ON_FILTER}.
+ *
+ * <p>Resolves the class name manually via {@link ClassUtils#forName}
rather than
+ * {@code environment.getProperty(key, Class.class)} — neither Spring's
default conversion
+ * service nor Spring Boot's {@code ApplicationConversionService} register
a String-to-Class
+ * converter, so that call would throw {@code ConverterNotFoundException}
for a real class name.
+ *
+ * <p>Defensive: any failure reading config or instantiating the
configured class falls back to
+ * a plain {@link DefaultStackTraceFilterer}, matching the resolver's own
fallback behaviour.
+ */
+ private StackTraceFilterer resolveConfiguredStackTraceFilterer(Environment
environment) {
+ Class<? extends StackTraceFilterer> filtererClass =
DefaultStackTraceFilterer.class;
+ String configuredClassName =
environment.getProperty(Settings.SETTING_LOGGING_STACKTRACE_FILTER_CLASS);
Review Comment:
Fixed — reads the raw value and accepts both `Class` and `CharSequence`.
Worse than you thought: `getProperty(key)` on a `Class` throws
`ConverterNotFoundException` out of `bootstrapContext.close()`, so it fails
startup rather than falling back. Both forms now covered by tests.
##########
grails-core/src/main/groovy/org/apache/grails/core/GrailsBootstrapRegistryInitializer.java:
##########
@@ -62,5 +78,60 @@ public void initialize(BootstrapRegistry registry) {
.registerSingleton(PluginDiscovery.BEAN_NAME, discovery);
LOG.debug("Promoted GrailsPluginDiscovery to ApplicationContext as
'{}'", PluginDiscovery.BEAN_NAME);
});
+
+ // Resolve the configured StackTraceFilterer from the environment
(same two keys
+ // GrailsExceptionResolver honours) and install + promote it the same
way, before refresh().
+ registry.addCloseListener(event -> {
+ ConfigurableApplicationContext applicationContext =
event.getApplicationContext();
+ StackTraceFilterer filterer =
resolveConfiguredStackTraceFilterer(applicationContext.getEnvironment());
+ GrailsUtil.initializeStackFilterer(filterer);
+ applicationContext.getBeanFactory()
+ .registerSingleton(StackTraceFilterer.BEAN_NAME, filterer);
+ LOG.debug("Promoted StackTraceFilterer to ApplicationContext as
'{}'", StackTraceFilterer.BEAN_NAME);
+ });
+ }
+
+ /**
+ * Resolves a {@link StackTraceFilterer} from the given environment,
honouring
+ * {@link Settings#SETTING_LOGGING_STACKTRACE_FILTER_CLASS} and
+ * {@link Settings#SETTING_LOG_FULL_STACKTRACE_ON_FILTER}.
+ *
+ * <p>Resolves the class name manually via {@link ClassUtils#forName}
rather than
+ * {@code environment.getProperty(key, Class.class)} — neither Spring's
default conversion
+ * service nor Spring Boot's {@code ApplicationConversionService} register
a String-to-Class
+ * converter, so that call would throw {@code ConverterNotFoundException}
for a real class name.
+ *
+ * <p>Defensive: any failure reading config or instantiating the
configured class falls back to
+ * a plain {@link DefaultStackTraceFilterer}, matching the resolver's own
fallback behaviour.
+ */
+ private StackTraceFilterer resolveConfiguredStackTraceFilterer(Environment
environment) {
+ Class<? extends StackTraceFilterer> filtererClass =
DefaultStackTraceFilterer.class;
+ String configuredClassName =
environment.getProperty(Settings.SETTING_LOGGING_STACKTRACE_FILTER_CLASS);
+ if (StringUtils.hasText(configuredClassName)) {
+ try {
+ filtererClass = ClassUtils.forName(configuredClassName,
getClass().getClassLoader())
Review Comment:
Fixed — uses `applicationContext.getClassLoader()`. Test defines a filterer
in a loader grails-core can't see, standing in for the devtools split.
##########
grails-core/src/main/groovy/org/apache/grails/core/GrailsBootstrapRegistryInitializer.java:
##########
@@ -62,5 +78,60 @@ public void initialize(BootstrapRegistry registry) {
.registerSingleton(PluginDiscovery.BEAN_NAME, discovery);
LOG.debug("Promoted GrailsPluginDiscovery to ApplicationContext as
'{}'", PluginDiscovery.BEAN_NAME);
});
+
+ // Resolve the configured StackTraceFilterer from the environment
(same two keys
+ // GrailsExceptionResolver honours) and install + promote it the same
way, before refresh().
+ registry.addCloseListener(event -> {
+ ConfigurableApplicationContext applicationContext =
event.getApplicationContext();
+ StackTraceFilterer filterer =
resolveConfiguredStackTraceFilterer(applicationContext.getEnvironment());
+ GrailsUtil.initializeStackFilterer(filterer);
+ applicationContext.getBeanFactory()
+ .registerSingleton(StackTraceFilterer.BEAN_NAME, filterer);
+ LOG.debug("Promoted StackTraceFilterer to ApplicationContext as
'{}'", StackTraceFilterer.BEAN_NAME);
+ });
+ }
+
+ /**
+ * Resolves a {@link StackTraceFilterer} from the given environment,
honouring
+ * {@link Settings#SETTING_LOGGING_STACKTRACE_FILTER_CLASS} and
+ * {@link Settings#SETTING_LOG_FULL_STACKTRACE_ON_FILTER}.
+ *
+ * <p>Resolves the class name manually via {@link ClassUtils#forName}
rather than
+ * {@code environment.getProperty(key, Class.class)} — neither Spring's
default conversion
+ * service nor Spring Boot's {@code ApplicationConversionService} register
a String-to-Class
+ * converter, so that call would throw {@code ConverterNotFoundException}
for a real class name.
+ *
+ * <p>Defensive: any failure reading config or instantiating the
configured class falls back to
Review Comment:
Fixed — both reads guarded independently. `logFullStackTraceOnFilter:
yes-please` is now a regression test.
##########
grails-bootstrap/src/main/groovy/org/grails/exceptions/reporting/StackTraceFilterer.java:
##########
@@ -35,6 +35,16 @@ public interface StackTraceFilterer {
String FULL_STACK_TRACE_MESSAGE = "Full Stack Trace:";
String SYS_PROP_DISPLAY_FULL_STACKTRACE = "grails.full.stacktrace";
+ /**
+ * Name under which the {@link
org.apache.grails.core.GrailsBootstrapRegistryInitializer}
+ * promotes the config-resolved filterer as an {@code ApplicationContext}
singleton bean,
+ * so later-lifecycle consumers (e.g. {@code GrailsExceptionResolver})
reuse the same
+ * instance instead of instantiating a second copy from config.
+ *
+ * @since 8.0
+ */
+ String BEAN_NAME = "stackTraceFilterer";
Review Comment:
Moved to
`GrailsBootstrapRegistryInitializer.STACK_TRACE_FILTERER_BEAN_NAME`, same as
`PluginDiscovery.BEAN_NAME`. On the drive-by: there's no `GrailsConsole` class
in the repo any more, so no stale comment to fix.
##########
grails-web-mvc/src/main/groovy/org/grails/web/errors/GrailsExceptionResolver.java:
##########
@@ -453,6 +460,28 @@ protected void createStackFilterer() {
applyLogFullStackTraceOnFilter();
}
+ /**
+ * Looks up the {@link StackTraceFilterer} that
+ * {@link org.apache.grails.core.GrailsBootstrapRegistryInitializer}
promoted to the
+ * {@code ApplicationContext} during bootstrap, so this resolver reuses
that instance instead
+ * of instantiating a second copy from config. Returns {@code null} when
no such bean is
+ * registered — e.g. a {@code GrailsApplication} wired up outside the
normal Spring Boot
+ * bootstrap sequence — in which case {@link #createStackFilterer()} falls
back to its own
+ * construction.
+ */
+ protected StackTraceFilterer resolvePromotedStackTraceFilterer() {
+ ApplicationContext context = grailsApplication.getMainContext();
+ if (context == null) {
+ return null;
+ }
+ try {
+ return context.getBean(StackTraceFilterer.BEAN_NAME,
StackTraceFilterer.class);
+ }
+ catch (NoSuchBeanDefinitionException e) {
Review Comment:
Widened to `BeansException`, with a test. On your question: a later bean
definition wins — `registerBeanDefinition` → `resetBeanDefinition` →
`destroySingleton` kills the manual singleton — so the resolver gets the app's
bean and `GrailsUtil` keeps the bootstrap one. Documented on
`resolvePromotedStackTraceFilterer`.
--
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]
