jdaugherty commented on code in PR #15994:
URL: https://github.com/apache/grails-core/pull/15994#discussion_r3608731718
##########
grails-testing-support-core/src/main/groovy/org/grails/testing/GrailsUnitTest.groovy:
##########
@@ -118,6 +131,15 @@ trait GrailsUnitTest {
}
} catch (NoSuchMethodException e) {}
+ try {
+ Method beanRegistrarMethod = clazz.getMethod('beanRegistrar')
+ BeanRegistrar registrar = (BeanRegistrar)
beanRegistrarMethod.invoke(plugin)
+ if (registrar != null) {
+ defineBeans(registrar)
+ return
Review Comment:
At boot, `GrailsApplicationPostProcessor` applies a lifecycle's
`doWithSpring()` DSL first and then its `beanRegistrar()`, so a plugin that
defines both hooks (e.g. one that is mid-migration) gets both sets of beans.
Here the first hook found wins and the other is skipped — with the current
ordering, a plugin defining both would only get its `doWithSpring()` beans in
the test context and its registrar beans would silently be missing. Consider
applying the DSL (when present) and then the registrar (when present) without
the early returns, mirroring the boot order.
##########
grails-redis/src/main/groovy/grails/plugins/redis/util/RedisConfigurationUtil.groovy:
##########
@@ -99,6 +92,84 @@ class RedisConfigurationUtil {
}
}
+ /**
+ * Registers the pool-config, pool and service bean definitions for a
redis connection
+ * directly against a {@link BeanDefinitionRegistry}, mirroring the beans
the
+ * {@link #configureService} closure wires through the bean builder DSL.
Used by the redis
+ * plugin's {@code beanRegistrar()}-registered post-processor.
+ */
+ static void configureService(BeanDefinitionRegistry registry, def
redisConfigMap, String key, Class serviceClass) {
Review Comment:
Unlike the controllers/services/interceptors post-processors (and the guards
in the url-mappings and datasource ones), this registry variant registers
`redisPoolConfig*`, `redisPool*` and `redisService*` without a
`containsBeanDefinition` check, so a definition contributed earlier under one
of these names is overwritten rather than winning. That breaks the "an existing
definition for a bean name always wins" contract the other converted
post-processors follow — and since `RedisBeanDefinitionsPostProcessor` runs at
highest precedence, an application override registered before it would be
clobbered silently. Consider skipping names that already exist in the registry,
either here or in the post-processor.
##########
grails-cache/src/main/groovy/grails/plugin/cache/GrailsCacheAutoConfiguration.groovy:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.plugin.cache
+
+import groovy.transform.CompileStatic
+
+import org.springframework.beans.factory.annotation.Value
+import org.springframework.boot.autoconfigure.AutoConfiguration
+import
org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
+import org.springframework.context.annotation.Bean
+
+import org.grails.plugin.cache.GrailsCacheManager
+
+/**
+ * Auto-configures the cache plugin's default cache manager and key generator.
Registered here
+ * rather than by the plugin descriptor so that a bean contributed by the
application or another
+ * plugin — for example a cache-provider plugin's {@code grailsCacheManager} —
makes the default
+ * back off cleanly instead of triggering a bean-definition override.
+ *
+ * @since 8.0
+ */
+@AutoConfiguration
+@ConditionalOnProperty(name = 'grails.cache.enabled', matchIfMissing = true)
+@CompileStatic
+class GrailsCacheAutoConfiguration {
+
+ @Value('${grails.cache.cacheManager:}')
+ String cacheManagerType
+
+ @Bean
+ @ConditionalOnMissingBean(name = 'customCacheKeyGenerator')
+ CustomCacheKeyGenerator customCacheKeyGenerator() {
+ new CustomCacheKeyGenerator()
+ }
+
+ @Bean
+ @ConditionalOnMissingBean(name = 'grailsCacheManager')
+ GrailsCacheManager grailsCacheManager(CachePluginConfiguration
grailsCacheConfiguration) {
Review Comment:
`grailsCacheConfiguration` is only contributed by the cache plugin
descriptor's registrar, but this auto-configuration is gated on
`grails.cache.enabled` alone. In a context where the jar is on the classpath
but the plugin is not active (plugin excluded, or a plain Boot context that
never runs the plugin registration phase), this bean method still runs and the
`CachePluginConfiguration` parameter fails to resolve, breaking refresh —
before this change everything lived in the descriptor, so deactivating the
plugin removed the whole set together. Gating this method (or the class) with
`@ConditionalOnBean(CachePluginConfiguration)` would keep the two in lockstep;
the registrar-phase definitions are visible to auto-configuration conditions
per the ordering this PR already relies on.
--
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]
