jamesfredley commented on code in PR #15541:
URL: https://github.com/apache/grails-core/pull/15541#discussion_r3030364827
##########
grails-gsp/plugin/src/test/groovy/org/grails/web/taglib/AbstractGrailsTagTests.groovy:
##########
@@ -93,6 +92,10 @@ import static org.junit.jupiter.api.Assertions.fail
abstract class AbstractGrailsTagTests {
+ // Theme support was removed in Spring Framework 7.0 - define the
attribute names directly
+ private static final String THEME_SOURCE_ATTRIBUTE =
DispatcherServlet.class.getName() + ".THEME_SOURCE"
Review Comment:
Done - removed the theme support entirely (JstlUtils import, theme
constants, initThemeSource method, and the JstlUtils try/catch block) from both
copies of AbstractGrailsTagTests.
##########
grails-gsp/plugin/src/test/groovy/org/grails/web/taglib/AbstractGrailsTagTests.groovy:
##########
@@ -279,7 +282,7 @@ abstract class AbstractGrailsTagTests {
mockManager.registerProvidedArtefacts(grailsApplication)
def mockControllerClass = gcl.parseClass('class MockController { def
index = {} } ')
- ctx = new AnnotationConfigServletWebServerApplicationContext()
+ ctx = new GenericWebApplicationContext()
Review Comment:
JstlUtils/theme support was removed from Spring Framework 7 entirely - it's
not in `spring-boot-webmvc` or anywhere in the Spring 7 stack. The theme
infrastructure (`ThemeResolver`, `ThemeSource`,
`JstlUtils.exposeLocalizationContext` with theme) was deprecated in Spring 6.0
and removed in 7.0. So we've removed all theme-related code from these test
classes.
##########
grails-gsp/plugin/src/test/groovy/org/grails/web/taglib/AbstractGrailsTagTests.groovy:
##########
@@ -361,8 +364,7 @@ abstract class AbstractGrailsTagTests {
}
private void initThemeSource(request, MessageSource messageSource) {
Review Comment:
Done - removed the method entirely along with all theme-related code.
##########
grails-gsp/spring-boot/build.gradle:
##########
@@ -34,6 +34,7 @@ ext {
dependencies {
implementation platform(project(':grails-bom'))
api project(':grails-sitemesh3')
+ compileOnly 'org.springframework.boot:spring-boot-webmvc'
Review Comment:
Good catch - changed from `compileOnly` to `api` to match the previous
behavior. Added a comment explaining this is needed because Spring Framework 7
no longer re-exports `spring-webmvc` transitively.
##########
grails-logging/src/main/groovy/org/grails/compiler/logging/LoggingTransformer.java:
##########
@@ -78,11 +80,26 @@ public void performInjectionOnAnnotatedClass(SourceUnit
source, ClassNode classN
return;
}
- AnnotationNode annotationNode = new
AnnotationNode(ClassHelper.make(Slf4j.class));
- LogASTTransformation logASTTransformation = new LogASTTransformation();
- logASTTransformation.setCompilationUnit(new CompilationUnit(new
GroovyClassLoader(getClass().getClassLoader())));
- logASTTransformation.visit(new ASTNode[]{ annotationNode, classNode},
source);
- classNode.putNodeMetaData(Slf4j.class, annotationNode);
+ // Instead of adding @Slf4j annotation (which won't be processed if
added during AST transformation),
Review Comment:
Yes, this is related to Spring Boot 4. The previous implementation used
`LogASTTransformation` from Groovy which internally relied on creating a new
`CompilationUnit` - this caused issues with the updated Groovy 4 / Spring Boot
4 classloading. The fix manually injects the `log` field (mimicking what
`@Slf4j` does) by creating a `LoggerFactory.getLogger(ClassName.class)` call
directly via AST nodes, avoiding the `LogASTTransformation` dependency
entirely. Same end result, more robust approach.
##########
grails-spring/build.gradle:
##########
@@ -62,4 +62,9 @@ dependencies {
apply {
from rootProject.layout.projectDirectory.file('gradle/docs-config.gradle')
+}
+
+tasks.named('checkstyleMain', Checkstyle) {
Review Comment:
Removed the checkstyle exclusion. You're right - there are no
`org/springframework/ui/**` files in grails-spring, so the exclusion was dead
config.
--
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]
