jdaugherty commented on code in PR #15541:
URL: https://github.com/apache/grails-core/pull/15541#discussion_r3029682952
##########
dependencies.gradle:
##########
@@ -73,7 +74,7 @@ ext {
'commons-codec.version' : '1.18.0',
'commons-lang3.version' : '3.20.0',
'geb-spock.version' : '8.0.1',
- 'groovy.version' : '4.0.30',
+ 'groovy.version' : '4.0.31',
Review Comment:
Below this line, spring Boot has these dependencies:
<jackson-2-bom.version>2.21.2</jackson-2-bom.version>
<jackson-bom.version>3.1.0</jackson-bom.version>
We need to be matching the jackson version & we need to adopt jackson2.
##########
build.gradle:
##########
@@ -74,6 +74,14 @@ subprojects {
def cacheHours = isCiBuild || isReproducibleBuild ? 0 : 24
cacheDynamicVersionsFor(cacheHours, 'hours')
cacheChangingModulesFor(cacheHours, 'hours')
+
Review Comment:
I dont' think this is the right place to use this. We should be using
enforcePlatform() instead since groovy 4 is a constraint there.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/SpringSessionContext.java:
##########
@@ -0,0 +1,144 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateExceptionTranslator.java:
##########
@@ -0,0 +1,103 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateOperations.java:
##########
@@ -0,0 +1,857 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateQueryException.java:
##########
@@ -0,0 +1,48 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateTransactionManager.java:
##########
@@ -0,0 +1,928 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
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:
Isn't this still included in org.springframework.boot:spring-boot-webmvc -
why change this?
##########
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:
This needs removed & the code formatted to our additions. Although I don't
see any code added now.
##########
grails-test-examples/app1/build.gradle:
##########
@@ -92,4 +92,10 @@ apply {
from
rootProject.layout.projectDirectory.file('gradle/functional-test-config.gradle')
from
rootProject.layout.projectDirectory.file('gradle/test-webjar-asset-config.gradle')
from
rootProject.layout.projectDirectory.file('gradle/grails-extension-gradle-config.gradle')
+}
+
+// Disabled: grails-spring-security plugin is incompatible with Spring Boot 4
Review Comment:
Let's add some type of identifier to make these easier to find & see they
are related to Spring Boot 4? maybe // TODO: BOOT4
##########
grails-test-examples/gsp-layout/src/test/groovy/org/apache/grails/views/gsp/layout/AbstractGrailsTagTests.groovy:
##########
@@ -96,6 +96,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
Review Comment:
remove since theme support is removed
##########
grails-controllers/build.gradle:
##########
@@ -43,6 +43,8 @@ dependencies {
api 'org.apache.groovy:groovy'
api 'org.springframework.boot:spring-boot-autoconfigure'
+ api 'org.springframework.boot:spring-boot-webmvc'
Review Comment:
Can we document why we're adding this to grails-controllers when it was
originally provided by grails-web-common?
##########
dependencies.gradle:
##########
@@ -48,22 +48,23 @@ ext {
// Note: the name of the dependency must be the prefix of the property
name so properties in the pom are resolved correctly
gradleBomDependencies = [
- 'ant' :
"org.apache.ant:ant:${gradleBomDependencyVersions['ant.version']}",
- 'ant-junit' :
"org.apache.ant:ant-junit:${gradleBomDependencyVersions['ant.version']}",
- 'asciidoctor-gradle-jvm':
"org.asciidoctor:asciidoctor-gradle-jvm:${gradleBomDependencyVersions['asciidoctor-gradle-jvm.version']}",
- 'asciidoctorj' :
"org.asciidoctor:asciidoctorj:${gradleBomDependencyVersions['asciidoctorj.version']}",
- 'asset-pipeline-gradle' :
"cloud.wondrify:asset-pipeline-gradle:${gradleBomDependencyVersions['asset-pipeline-gradle.version']}",
- 'byte-buddy' :
"net.bytebuddy:byte-buddy:${gradleBomDependencyVersions['byte-buddy.version']}",
- 'commons-text' :
"org.apache.commons:commons-text:${gradleBomDependencyVersions['commons-text.version']}",
- 'directory-watcher' :
"io.methvin:directory-watcher:${gradleBomDependencyVersions['directory-watcher.version']}",
- 'grails-publish-plugin' :
"org.apache.grails.gradle:grails-publish:${gradleBomDependencyVersions['grails-publish-plugin.version']}",
- 'jansi' :
"org.fusesource.jansi:jansi:${gradleBomDependencyVersions['jansi.version']}",
- 'javaparser-core' :
"com.github.javaparser:javaparser-core:${gradleBomDependencyVersions['javaparser-core.version']}",
- 'jline' :
"jline:jline:${gradleBomDependencyVersions['jline.version']}",
- 'jna' :
"net.java.dev.jna:jna:${gradleBomDependencyVersions['jna.version']}",
- 'objenesis' :
"org.objenesis:objenesis:${gradleBomDependencyVersions['objenesis.version']}",
- 'spring-boot-cli' :
"org.springframework.boot:spring-boot-cli:${gradleBomDependencyVersions['spring-boot.version']}",
- 'spring-boot-gradle' :
"org.springframework.boot:spring-boot-gradle-plugin:${gradleBomDependencyVersions['spring-boot.version']}",
+ 'ant' :
"org.apache.ant:ant:${gradleBomDependencyVersions['ant.version']}",
+ 'ant-junit' :
"org.apache.ant:ant-junit:${gradleBomDependencyVersions['ant.version']}",
+ 'asciidoctorj-gradle-jvm':
"org.asciidoctor:asciidoctor-gradle-jvm:${gradleBomDependencyVersions['asciidoctor-gradle-jvm.version']}",
Review Comment:
The key is supposed to match the artifact name or be a prefix of it. Have
you checked extract constraints? I suspect this broke it.
##########
grails-data-hibernate5/core/build.gradle:
##########
@@ -45,6 +45,8 @@ dependencies {
api 'org.apache.groovy:groovy'
api project(':grails-datamapping-core')
api 'org.springframework:spring-orm'
+ api 'org.springframework:spring-web'
Review Comment:
Why are we adding spring web here?
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/SpringFlushSynchronization.java:
##########
@@ -0,0 +1,56 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-bom/build.gradle:
##########
@@ -224,6 +224,15 @@ ext {
for (Map.Entry<String, String> property :
pomProperties.entrySet()) {
propertiesNode.appendNode(property.key, property.value)
}
+
+ // Override Spring Boot's groovy.version property with Grails'
version
+ // Spring Boot 4.0.5 defaults to Groovy 5.0.4, but Grails 8.0.x
uses Groovy 4.0.31
+ def groovyVersionNode = propertiesNode.'groovy.version'
Review Comment:
If we are defining groovy version, it should already be overridden because
of how the constraints are built. Why is this necessary?
##########
grails-web-common/src/main/groovy/org/grails/web/config/http/GrailsFilterOrder.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.grails.web.config.http;
+
+/**
+ * Constants for filter ordering in Grails applications.
+ * <p>
+ * These constants were previously obtained from Spring Boot's {@code
SecurityProperties}
+ * class, but were removed in Spring Boot 4.0. They are now defined here for
use by
+ * Grails filter configuration.
+ *
+ * @since 8.0
+ */
+public final class GrailsFilterOrder {
+
+ private GrailsFilterOrder() {
+ // Utility class
+ }
+
+ /**
+ * Default order of Spring Security's Filter in the servlet container
(i.e. amongst
+ * other filters registered with the container). There is no connection
between this
+ * and the {@code @Order} on a {@code SecurityFilterChain}.
+ * <p>
+ * The value {@code -100} matches what was previously defined in Spring
Boot's
+ * {@code SecurityProperties.DEFAULT_FILTER_ORDER} (computed as
+ * {@code OrderedFilter.REQUEST_WRAPPER_FILTER_MAX_ORDER - 100}) before it
was
+ * removed in Spring Boot 4.0.
+ */
+ public static final int DEFAULT_FILTER_ORDER = -100;
Review Comment:
Isn't the issue that Spring is moving away from filter recommendations &
they're wanting people to adopt their fluent API (HttpSecurity configuration)?
##########
grails-data-mongodb/boot-plugin/src/test/groovy/org/grails/datastore/gorm/mongodb/boot/autoconfigure/MongoDbGormAutoConfigurationSpec.groovy:
##########
@@ -44,8 +44,8 @@ class MongoDbGormAutoConfigurationSpec extends
AutoStartedMongoSpec {
}
void setupSpec() {
- System.setProperty('spring.data.mongodb.host', dbContainer.getHost())
- System.setProperty('spring.data.mongodb.port',
dbContainer.getMappedPort(AbstractMongoGrailsExtension.DEFAULT_MONGO_PORT) as
String)
+ System.setProperty('spring.mongodb.host', dbContainer.getHost())
Review Comment:
We need to add documentation for these config changes.
##########
grails-web-boot/src/test/groovy/grails/boot/EmbeddedContainerWithGrailsSpec.groovy:
##########
@@ -18,63 +18,33 @@
*/
package grails.boot
-import org.springframework.core.env.ConfigurableEnvironment
-import org.springframework.web.context.support.StandardServletEnvironment
-
import grails.artefact.Artefact
import grails.boot.config.GrailsAutoConfiguration
import grails.web.Controller
import org.springframework.boot.autoconfigure.EnableAutoConfiguration
-import
org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory
-import
org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext
-import
org.springframework.boot.web.servlet.server.ConfigurableServletWebServerFactory
-import org.springframework.context.annotation.Bean
+import spock.lang.Ignore
import spock.lang.Specification
-import org.apache.grails.core.plugins.DefaultPluginDiscovery
-import org.apache.grails.core.plugins.PluginDiscovery
-
/**
- * Created by graemerocher on 28/05/14.
+ * Tests loading Grails in an embedded server configuration.
+ *
+ * TODO: Rework for Spring Boot 4.0 modularized embedded server APIs.
+ * Embedded server classes moved to spring-boot-web-server and
spring-boot-tomcat modules
+ * and require updated test patterns.
*/
+@Ignore("Spring Boot 4.0: Embedded server test infrastructure needs
significant rework due to modularization. " +
Review Comment:
This is one of the core features of Grails. At the minimum this should be
`PendingFeature` I don't think we can merge this without embedded server
support - that's how most people run Grails.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/support/OpenSessionInViewInterceptor.java:
##########
@@ -0,0 +1,219 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/SpringSessionSynchronization.java:
##########
@@ -0,0 +1,148 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/SpringJtaSessionContext.java:
##########
@@ -0,0 +1,49 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateObjectRetrievalFailureException.java:
##########
@@ -0,0 +1,59 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/ConfigurableJtaPlatform.java:
##########
@@ -0,0 +1,113 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-datamapping-core/src/test/groovy/grails/gorm/annotation/transactions/TransactionalTransformSpec.groovy:
##########
@@ -198,7 +198,7 @@ import grails.gorm.transactions.Transactional
mySpec.getDeclaredMethod('$tt__$spock_feature_0_0', Object, Object,
Object, TransactionStatus)
and:"The spec can be called"
- mySpec.newInstance().'$tt__$spock_feature_0_0'(2,2,4,new
DefaultTransactionStatus(new Object(), true, true, false, false, null))
+ mySpec.newInstance().'$tt__$spock_feature_0_0'(2,2,4,new
DefaultTransactionStatus(null, new Object(), true, true, false, false, false,
null))
Review Comment:
Do you know the idea behind the new synchronization logic? Do we have a gap
now in our logic?
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/support/AsyncRequestInterceptor.java:
##########
@@ -0,0 +1,124 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/build.gradle:
##########
@@ -91,3 +93,8 @@ apply {
from
rootProject.layout.projectDirectory.file('gradle/grails-data-tck-config.gradle')
from rootProject.layout.projectDirectory.file('gradle/docs-config.gradle')
}
+
+tasks.named('checkstyleMain', Checkstyle) {
+ // Exclude copied Spring Framework classes from checkstyle (they follow
Spring's code style)
+ exclude('**/org/grails/orm/hibernate/support/hibernate5/**')
Review Comment:
The amount of code being imported here signifies we need to have another
project exporting that code. We shouldn't grow the size of hte hibernate5
impelmentation. Instead create a `grails-data-hibernate5/spring-orm` directory
and put the files there so it's clear where these files came from.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/SpringBeanContainer.java:
##########
@@ -0,0 +1,270 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/test/groovy/org/apache/grails/data/hibernate5/core/GrailsDataHibernate5TckManager.groovy:
##########
@@ -35,8 +35,8 @@ import org.hibernate.SessionFactory
import org.hibernate.dialect.H2Dialect
import org.springframework.beans.factory.DisposableBean
import org.springframework.context.ApplicationContext
-import org.springframework.orm.hibernate5.SessionFactoryUtils
-import org.springframework.orm.hibernate5.SessionHolder
+import org.grails.orm.hibernate.support.hibernate5.SessionFactoryUtils
Review Comment:
The documentation needs to be updated for all of these package moves so
people understand how to update their applications.
##########
grails-data-hibernate5/core/build.gradle:
##########
@@ -91,3 +93,8 @@ apply {
from
rootProject.layout.projectDirectory.file('gradle/grails-data-tck-config.gradle')
from rootProject.layout.projectDirectory.file('gradle/docs-config.gradle')
}
+
+tasks.named('checkstyleMain', Checkstyle) {
Review Comment:
We should have a consistent code style. We will likely make changes to
those files over time. This needs removed and those files need reformatted.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/SessionFactoryUtils.java:
##########
@@ -0,0 +1,263 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy:
##########
@@ -452,11 +450,6 @@ ${importStatements}
}
}
- project.logger.info('Configuring CLASSIC boot loader for Micronaut
compatibility in {}', project.name)
Review Comment:
Have you captured this as a TODO? Does micronaut support Spring Boot 4 now?
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/LocalSessionFactoryBuilder.java:
##########
@@ -0,0 +1,468 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateJdbcException.java:
##########
@@ -0,0 +1,59 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateCallback.java:
##########
@@ -0,0 +1,55 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/SessionHolder.java:
##########
@@ -0,0 +1,84 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateOptimisticLockingFailureException.java:
##########
@@ -0,0 +1,49 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateTemplate.java:
##########
@@ -0,0 +1,1185 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/HibernateSystemException.java:
##########
@@ -0,0 +1,45 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
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:
Remove the theme support instead.
##########
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:
Remove the method instead (I'll forego further comments on theme related
removal)
##########
grails-data-hibernate5/core/src/main/java/org/grails/orm/hibernate/support/hibernate5/LocalSessionFactoryBean.java:
##########
@@ -0,0 +1,665 @@
+/*
Review Comment:
You cannot change the copyright for imported code. You must leave the
original copyright & reference it in the NOTICE / LICENSE files.
##########
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:
Shouldn't this be `api` to match before?
##########
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:
What's the history of this change? Why is this here, is it related to
spring boot 4change?
##########
grails-test-examples/gsp-layout/src/integration-test/groovy/GrailsLayoutSpec.groovy:
##########
@@ -39,6 +40,7 @@ class GrailsLayoutSpec extends ContainerGebSpec {
pageSource.contains('This is so cool.')
}
+ @Ignore('JSP support removed in Spring Framework 7 - see #15457')
Review Comment:
So it's not that jsp support was removed, it's that the theme support was
removed? I think this is a major bug.
##########
grails-test-examples/gsp-layout/src/test/groovy/org/apache/grails/views/gsp/layout/AbstractGrailsTagTests.groovy:
##########
@@ -282,7 +286,7 @@ abstract class AbstractGrailsTagTests {
mockManager.registerProvidedArtefacts(grailsApplication)
def mockControllerClass = gcl.parseClass('class MockController { def
index = {} } ')
- ctx = new AnnotationConfigServletWebServerApplicationContext()
+ ctx = new GenericWebApplicationContext()
Review Comment:
By including the mvc library, can't this be reverted?
##########
grails-url-mappings/src/main/groovy/org/grails/plugins/web/mapping/UrlMappingsAutoConfiguration.java:
##########
@@ -82,10 +80,9 @@ public GrailsCorsFilter
grailsCorsFilter(GrailsCorsConfiguration grailsCorsConfi
}
@Bean
- public UrlMappingsErrorPageCustomizer
urlMappingsErrorPageCustomizer(ObjectProvider<UrlMappings> urlMappingsProvider)
{
- UrlMappingsErrorPageCustomizer errorPageCustomizer = new
UrlMappingsErrorPageCustomizer();
-
errorPageCustomizer.setUrlMappings(urlMappingsProvider.getIfAvailable());
- return errorPageCustomizer;
+ @ConditionalOnMissingBean
+ public UrlMappingsErrorPageCustomizer urlMappingsErrorPageCustomizer() {
Review Comment:
Is this really the same thing? The url mappings provider does not
necessarily mean the eror r page customizer.
##########
grails-test-examples/plugins/micronaut-singleton/build.gradle:
##########
@@ -43,3 +43,8 @@ dependencies {
apply {
from
rootProject.layout.projectDirectory.file('gradle/grails-extension-gradle-config.gradle')
}
+
+// Groovydoc not needed for test example plugins
+tasks.named('groovydoc') {
Review Comment:
Why disable this? We should just disable it as part of the functional test
configuration if we want to disable all of it. I suspect this was a compile
error since the class was no longer reference - which indicates this may be
masking a real a bug (missing dependencies)
##########
grails-test-examples/gsp-sitemesh3/build.gradle:
##########
@@ -67,4 +67,10 @@ apply {
from
rootProject.layout.projectDirectory.file('gradle/functional-test-config.gradle')
from
rootProject.layout.projectDirectory.file('gradle/test-webjar-asset-config.gradle')
from
rootProject.layout.projectDirectory.file('gradle/grails-extension-gradle-config.gradle')
+}
+
+// Disabled: SiteMesh3 is incompatible with Spring Framework 7
+// Re-enable when SiteMesh3 integration is updated.
+tasks.named('integrationTest') {
Review Comment:
Let's add that TODO marker to understand? Also if sitemesh3 is incompatible
should we even include the project or publish it in the bom? I think we should
strip those and add it back once it's working.
##########
grails-testing-support-core/src/main/groovy/org/grails/testing/GrailsApplicationBuilder.groovy:
##########
@@ -127,22 +146,43 @@ class GrailsApplicationBuilder {
protected ConfigurableApplicationContext createMainContext(Object
servletContext) {
ConfigurableApplicationContext context
if (isServletApiPresent && servletContext != null) {
- context = (AnnotationConfigServletWebApplicationContext)
ClassUtils.forName('org.springframework.boot.web.servlet.context.AnnotationConfigServletWebApplicationContext').getDeclaredConstructor().newInstance()
- ((AnnotationConfigServletWebApplicationContext)
context).servletContext = (ServletContext) servletContext
+ // Spring Boot 4.0/Spring 7.0: Use GenericWebApplicationContext
with manual annotation support
Review Comment:
I don't think this is correct. I'm still seeing these originally classes in
Spring 4.
##########
grails-test-examples/gsp-layout/src/test/groovy/org/apache/grails/views/gsp/layout/AbstractGrailsTagTests.groovy:
##########
@@ -365,8 +369,7 @@ abstract class AbstractGrailsTagTests {
}
private void initThemeSource(request, MessageSource messageSource) {
Review Comment:
remove since theme support is removed
##########
grails-web-url-mappings/src/main/groovy/grails/web/mapping/ResponseRedirector.groovy:
##########
@@ -139,7 +139,7 @@ class ResponseRedirector {
status = moved ? HttpStatus.MOVED_PERMANENTLY.value() :
HttpStatus.PERMANENT_REDIRECT.value()
}
else {
- status = moved ? HttpStatus.MOVED_TEMPORARILY.value() :
HttpStatus.TEMPORARY_REDIRECT.value()
+ status = moved ? HttpStatus.FOUND.value() :
HttpStatus.TEMPORARY_REDIRECT.value()
Review Comment:
We should add to our upgrade guide that MOVED_TEMPORARILY was deprecated by
spring and rmoved since FOUND is the same code.
##########
grails-test-examples/app1/build.gradle:
##########
@@ -92,4 +92,10 @@ apply {
from
rootProject.layout.projectDirectory.file('gradle/functional-test-config.gradle')
from
rootProject.layout.projectDirectory.file('gradle/test-webjar-asset-config.gradle')
from
rootProject.layout.projectDirectory.file('gradle/grails-extension-gradle-config.gradle')
+}
+
+// Disabled: grails-spring-security plugin is incompatible with Spring Boot 4
Review Comment:
Actually, even betteer, let's add a shared gradle file for these so we know
exactly where we disabled these.
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -158,6 +158,5 @@ class UrlMappingsInfoHandlerAdapter implements
HandlerAdapter, ApplicationContex
return null
}
- @Override
long getLastModified(HttpServletRequest request, Object handler) { -1 }
Review Comment:
This method can be removed since it was deprecated and removed.
##########
grails-web-boot/src/test/groovy/grails/boot/GrailsSpringApplicationSpec.groovy:
##########
@@ -19,42 +19,29 @@
package grails.boot
import grails.boot.config.GrailsAutoConfiguration
-import org.springframework.boot.SpringApplication
import org.springframework.boot.autoconfigure.EnableAutoConfiguration
-import
org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory
-import
org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext
-import
org.springframework.boot.web.servlet.server.ConfigurableServletWebServerFactory
-import org.springframework.context.annotation.Bean
+import spock.lang.Ignore
import spock.lang.Specification
/**
- * Created by graemerocher on 28/05/14.
+ * Tests running Grails via SpringApplication with an embedded server.
+ *
+ * TODO: Rework for Spring Boot 4.0 modularized embedded server APIs.
+ * Embedded server classes moved to spring-boot-web-server and
spring-boot-tomcat modules
+ * and require updated test patterns.
*/
-class GrailsSpringApplicationSpec extends Specification{
-
- AnnotationConfigServletWebServerApplicationContext context
-
- void cleanup() {
- context.close()
- }
+@Ignore("Spring Boot 4.0: Embedded server test infrastructure needs
significant rework due to modularization. " +
Review Comment:
`PendingFeature` - as previouslyl mentioned, I think this is masking a major
problem.
##########
grails-test-core/src/main/groovy/org/grails/plugins/testing/AbstractGrailsMockHttpServletResponse.groovy:
##########
@@ -109,7 +109,7 @@ abstract class AbstractGrailsMockHttpServletResponse
extends MockHttpServletResp
final webRequest = GrailsWebRequest.lookup()
webRequest?.currentRequest?.removeAttribute(GrailsApplicationAttributes.REDIRECT_ISSUED)
setCommitted(false)
- def field = ReflectionUtils.findField(MockHttpServletResponse,
'writer')
+ def field = ReflectionUtils.findField(MockHttpServletResponse,
'outputStream')
Review Comment:
I'm looking
https://github.com/spring-projects/spring-framework/blame/main/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java#L102
and it looks liek the field you want to change is still called writer?
--
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]
