jdaugherty commented on code in PR #15972:
URL: https://github.com/apache/grails-core/pull/15972#discussion_r3565184096
##########
gradle/grails-data-tck-config.gradle:
##########
@@ -94,11 +94,13 @@ tasks.withType(Test).configureEach { Test it ->
return false
}
- if (project.hasProperty('onlySpringSecurityTests')) {
+ // Neo4j module names don't share a common prefix
(grails-datastore-gorm-neo4j predates
Review Comment:
I assume these comments are out of date? We should rename them - we voted on
these names so we must rename
##########
grails-data-neo4j/boot-plugin/build.gradle:
##########
@@ -17,27 +17,77 @@
* under the License.
*/
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.publish'
+ id 'org.apache.grails.buildsrc.sbom'
+ id 'org.apache.grails.gradle.grails-code-style'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+ext {
+ gormApiDocs = true
+}
+
dependencies {
- compileOnly "org.springframework.boot:spring-boot-cli:$springBootVersion",
{
- exclude group:'org.codehaus.groovy', module:'groovy'
- exclude group:'jline', module:'jline'
- }
- api "org.springframework.boot:spring-boot-autoconfigure:$springBootVersion"
+ implementation platform(project(':grails-bom'))
- api "org.codehaus.groovy:groovy"
- api project(":grails-datastore-gorm-neo4j")
- api "org.springframework:spring-tx:$springVersion"
+ api 'org.springframework.boot:spring-boot-autoconfigure'
+ // DispatcherServletAutoConfiguration (referenced via
@AutoConfigureBefore) moved to spring-boot-webmvc.
+ api 'org.springframework.boot:spring-boot-webmvc'
+ api 'org.apache.groovy:groovy'
+ api project(':grails-datastore-gorm-neo4j')
+ api 'org.springframework:spring-tx'
testRuntimeOnly "org.neo4j.test:neo4j-harness:$neo4jVersion"
- testImplementation ("org.spockframework:spock-core:$spockVersion") {
- exclude group: 'junit', module: 'junit-dep'
- exclude group: 'org.codehaus.groovy', module: 'groovy-all'
- exclude group: 'org.hamcrest', module: 'hamcrest-core'
- transitive = false
+ testImplementation 'org.spockframework:spock-core'
+ testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
+}
+
+test {
+ useJUnitPlatform()
+
+ // The embedded Neo4j 3.5.x test harness reflectively pokes JDK internals
(Throwable's
+ // message field, sun.nio.ch.FileChannelImpl's lock accessors) at startup;
JDK 9+ strong
+ // encapsulation blocks that without these opens.
+ jvmArgs = [
+ '--add-opens', 'java.base/java.lang=ALL-UNNAMED',
+ '--add-opens', 'java.base/sun.nio.ch=ALL-UNNAMED',
+ ]
+}
+
+// The Spring Boot BOM (pulled in transitively via grails-bom) force-upgrades
Jetty to a 12.x
+// platform version. The embedded Neo4j 3.5.x test harness (neo4j-harness,
test-only) is compiled
+// against Jetty 9.4 and is binary-incompatible with Jetty 12's restructured
handler/server APIs.
+// Scoped to the test classpaths only, matching
grails-datastore-gorm-neo4j/build.gradle.
+def neo4jHarnessJettyVersion = '9.4.43.v20210629'
+[configurations.testCompileClasspath,
configurations.testRuntimeClasspath].each {
+ it.resolutionStrategy {
+ force "org.eclipse.jetty:jetty-server:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-servlet:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-webapp:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-security:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-http:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-io:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-util:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-util-ajax:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-xml:$neo4jHarnessJettyVersion"
}
- testImplementation
"org.springframework.boot:spring-boot-cli:$springBootVersion", {
- exclude group:'org.codehaus.groovy', module:'groovy'
- exclude group:'jline', module:'jline'
+}
+
+// grails-datastore-gorm-neo4j's own code (e.g. Neo4jQuery#executeQuery) calls
+// Driver#defaultTypeSystem(), which neo4j-java-driver 6.x (pulled in
transitively via the Spring
+// Boot BOM) removed - so this module's test classpath needs the same force to
actually exercise
+// that code path.
+configurations.all {
+ resolutionStrategy {
+ force "org.neo4j.driver:neo4j-java-driver:$neo4jDriverVersion"
Review Comment:
We need to fix the bom for this, and not do this.
##########
grails-data-neo4j/grails-datastore-gorm-neo4j/build.gradle:
##########
@@ -17,93 +17,122 @@
* under the License.
*/
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.publish'
+ id 'org.apache.grails.buildsrc.sbom'
+ id 'org.apache.grails.gradle.grails-code-style'
+}
+
+version = projectVersion
+group = 'org.apache.grails.data'
+
+ext {
+ gormApiDocs = true
+ pomTitle = 'GORM for Neo4j'
+ pomDescription = 'Provides a GORM Object Mapping implementation for the
Neo4j Graph Database'
+}
+
sourceSets.main.java.srcDirs = []
sourceSets.main.groovy.srcDirs += ["src/main/java"]
dependencies {
+
+ implementation platform(project(':grails-bom'))
+
api "org.neo4j.driver:neo4j-java-driver:$neo4jDriverVersion"
- api
"org.apache.grails.data:grails-datamapping-validation:$datastoreVersion"
- api "org.apache.grails.data:grails-datamapping-core:$datastoreVersion"
+ api project(':grails-datamapping-validation')
+ api project(':grails-datamapping-core')
// only needed for web dependencies
- compileOnly "org.apache.grails.data:grails-datastore-web:$datastoreVersion"
+ compileOnly project(':grails-datastore-web')
compileOnly "org.neo4j.test:neo4j-harness:$neo4jVersion"
- implementation "org.javassist:javassist:$javassistVersion"
+ implementation 'org.javassist:javassist'
+
testImplementation "org.neo4j.test:neo4j-harness:$neo4jVersion"
- testImplementation
"org.apache.grails.data:grails-datamapping-core-test:$datastoreVersion"
- testImplementation
"org.apache.grails.data:grails-datamapping-tck-tests:$datastoreVersion"
- testImplementation
"org.hibernate:hibernate-validator:$hibernateValidatorVersion"
+ testImplementation project(':grails-datamapping-core-test')
+ testImplementation project(':grails-datamapping-tck')
+ testImplementation 'org.spockframework:spock-core'
+ testImplementation 'jakarta.validation:jakarta.validation-api'
+ testImplementation 'org.hibernate.validator:hibernate-validator'
testImplementation "org.codehaus.gpars:gpars:$gparsVersion"
- testImplementation "cglib:cglib-nodep:$cglibNodepVersion"
- testImplementation "org.objenesis:objenesis:${objenesisVersion}"
-
- testRuntimeOnly "org.springframework:spring-aop:$springVersion"
- testRuntimeOnly "ch.qos.logback:logback-classic:1.4.14"
- testRuntimeOnly "javax.el:javax.el-api:3.0.0"
- testRuntimeOnly "org.glassfish.web:el-impl:2.2.1-b05"
+ testImplementation 'org.objenesis:objenesis'
+
+ testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
+
+ // Spock's Mock() support needs this at runtime but spock-core doesn't
declare it as a dependency
+ testRuntimeOnly "io.leangen.geantyref:geantyref:$geantyrefVersion"
+ testRuntimeOnly 'net.bytebuddy:byte-buddy' // Required by Spock's mocking
support (cglib doesn't work on JDK 21+)
+ testRuntimeOnly 'org.springframework:spring-aop'
+ testRuntimeOnly "ch.qos.logback:logback-classic:$logbackClassicVersion"
+ testRuntimeOnly "jakarta.el:jakarta.el-api:$elApiVersion"
+ testRuntimeOnly
"org.glassfish.expressly:expressly:$defaultElImplementationVersion"
+}
+// The Spring Boot BOM (pulled in transitively via grails-bom) force-upgrades
Jetty to a
+// 12.x platform version and neo4j-java-driver to 6.x.
+def neo4jHarnessJettyVersion = '9.4.43.v20210629'
+
+// The embedded Neo4j 3.5.x test harness (neo4j-harness, test-only) is
compiled against Jetty
+// 9.4 and is binary-incompatible with Jetty 12's restructured handler/server
APIs. Scoped to
+// the test classpaths only, since main code never touches Jetty directly.
+[configurations.testCompileClasspath,
configurations.testRuntimeClasspath].each {
+ it.resolutionStrategy {
+ force "org.eclipse.jetty:jetty-server:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-servlet:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-webapp:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-security:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-http:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-io:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-util:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-util-ajax:$neo4jHarnessJettyVersion",
+ "org.eclipse.jetty:jetty-xml:$neo4jHarnessJettyVersion"
+ }
+}
+
+// This module's own main code (e.g. Neo4jQuery#executeQuery) calls
Driver#defaultTypeSystem(),
+// which driver 6.x removed - so this force applies to all configurations,
main and test alike.
+configurations.all {
+ resolutionStrategy {
+ force "org.neo4j.driver:neo4j-java-driver:$neo4jDriverVersion"
+ }
}
test {
useJUnitPlatform()
- maxParallelForks = configuredTestParallel
+ systemProperty('neo4j.gorm.suite', System.getProperty('neo4j.gorm.suite')
?: true)
+ maxParallelForks = (findProperty('maxTestParallel') as Integer) ?: 1
forkEvery = 10
- jvmArgs = ['-Xmx1028M']
- afterSuite {
- System.out.print('.')
- System.out.flush()
- }
+ // The embedded Neo4j 3.5.x test harness reflectively pokes JDK internals
(Throwable's
+ // message field, sun.nio.ch.FileChannelImpl's lock accessors) at startup;
JDK 9+ strong
+ // encapsulation blocks that without these opens.
+ jvmArgs = [
+ '-Xmx1028M',
+ '--add-opens', 'java.base/java.lang=ALL-UNNAMED',
+ '--add-opens', 'java.base/sun.nio.ch=ALL-UNNAMED',
+ ]
}
-test.doFirst {
- def toBaseClassRelativePathWithoutExtension = { String base, String
classFile ->
- if (classFile.startsWith(base)) {
- def sansClass = classFile[0 .. classFile.size() - ".class".size()
- 1]
- def dollarIndex = sansClass.indexOf('$')
- def baseClass = dollarIndex > 0 ? sansClass[0..dollarIndex - 1] :
sansClass
- def relative = baseClass - base - '/'
- relative
- }
- else {
- null
- }
- }
- def tckClassesFile = project
- .configurations
- .testCompileClasspath
- .resolvedConfiguration
- .getResolvedArtifacts()
- .find { resolved ->
- resolved.moduleVersion.id.name ==
'grails-datamapping-tck-tests'
- }.file
-
- def tckClassesDir = project.file("${project.buildDir}/tck")
- copy {
- from zipTree(tckClassesFile)
- into tckClassesDir
- }
- copy {
- from tckClassesDir
- into sourceSets.test.output.classesDirs.find {
it.path.contains('classes/groovy') }
- include "**/*.class"
- exclude { details ->
- // Do not copy across any TCK class (or nested classes of that
class)
- // If there is a corresponding source file in the particular
modules
- // test source tree. Allows a module to override a test/helper.
-
- if (!details.file.isFile()) {
- return false
- }
- def candidatePath = details.file.absolutePath
- def relativePath =
toBaseClassRelativePathWithoutExtension(tckClassesDir.absolutePath,
candidatePath)
-
- if (relativePath == null) {
- throw new IllegalStateException("$candidatePath does not
appear to be in the TCK")
- }
-
- project.file("src/test/groovy/${relativePath}.groovy").exists()
- }
- }
+apply {
+ from
rootProject.layout.projectDirectory.file('gradle/grails-data-tck-config.gradle')
+ from rootProject.layout.projectDirectory.file('gradle/docs-config.gradle')
+}
+
+// This module predates the repo's Checkstyle/CodeNarc gate entirely: it was a
standalone Gradle
+// build (never included in root settings.gradle) until this PR wired it in,
so its Grails 3-era
+// source has never been checked against these rules. ~1,400 pre-existing
violations across the
+// module are tracked as a follow-up cleanup PR rather than fixed here -
codenarcFix's automated
+// fixes for SpaceAroundMapEntryColon/UnnecessaryGString are unsafe on this
module specifically,
+// since they rewrite string *contents* and this module embeds Cypher queries
in string literals
+// throughout (e.g. "MATCH (n:Label)"). Reports still generate; only
build-breaking is suppressed.
+tasks.withType(Checkstyle).configureEach {
+ ignoreFailures = true
Review Comment:
Re-checked at the current head (40b7aa1bf1): commit dca3d09c6c is not on
this branch — `ignoreFailures = true` is still present for CodeNarc in this
file, so the 270-violation cleanup described above has not landed. Please push
it.
##########
grails-data-neo4j/boot-plugin/build.gradle:
##########
@@ -17,27 +17,77 @@
* under the License.
*/
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.publish'
+ id 'org.apache.grails.buildsrc.sbom'
+ id 'org.apache.grails.gradle.grails-code-style'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+ext {
+ gormApiDocs = true
+}
+
dependencies {
- compileOnly "org.springframework.boot:spring-boot-cli:$springBootVersion",
{
- exclude group:'org.codehaus.groovy', module:'groovy'
- exclude group:'jline', module:'jline'
- }
- api "org.springframework.boot:spring-boot-autoconfigure:$springBootVersion"
+ implementation platform(project(':grails-bom'))
- api "org.codehaus.groovy:groovy"
- api project(":grails-datastore-gorm-neo4j")
- api "org.springframework:spring-tx:$springVersion"
+ api 'org.springframework.boot:spring-boot-autoconfigure'
+ // DispatcherServletAutoConfiguration (referenced via
@AutoConfigureBefore) moved to spring-boot-webmvc.
+ api 'org.springframework.boot:spring-boot-webmvc'
+ api 'org.apache.groovy:groovy'
+ api project(':grails-datastore-gorm-neo4j')
+ api 'org.springframework:spring-tx'
testRuntimeOnly "org.neo4j.test:neo4j-harness:$neo4jVersion"
- testImplementation ("org.spockframework:spock-core:$spockVersion") {
- exclude group: 'junit', module: 'junit-dep'
- exclude group: 'org.codehaus.groovy', module: 'groovy-all'
- exclude group: 'org.hamcrest', module: 'hamcrest-core'
- transitive = false
+ testImplementation 'org.spockframework:spock-core'
+ testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
+}
+
+test {
+ useJUnitPlatform()
+
+ // The embedded Neo4j 3.5.x test harness reflectively pokes JDK internals
(Throwable's
+ // message field, sun.nio.ch.FileChannelImpl's lock accessors) at startup;
JDK 9+ strong
+ // encapsulation blocks that without these opens.
+ jvmArgs = [
+ '--add-opens', 'java.base/java.lang=ALL-UNNAMED',
+ '--add-opens', 'java.base/sun.nio.ch=ALL-UNNAMED',
+ ]
+}
+
+// The Spring Boot BOM (pulled in transitively via grails-bom) force-upgrades
Jetty to a 12.x
+// platform version. The embedded Neo4j 3.5.x test harness (neo4j-harness,
test-only) is compiled
Review Comment:
There's no version of neo4j that is made against a newer jetty verison?
##########
grails-data-neo4j/boot-plugin/build.gradle:
##########
@@ -17,27 +17,77 @@
* under the License.
*/
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.publish'
+ id 'org.apache.grails.buildsrc.sbom'
+ id 'org.apache.grails.gradle.grails-code-style'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+ext {
+ gormApiDocs = true
+}
+
dependencies {
- compileOnly "org.springframework.boot:spring-boot-cli:$springBootVersion",
{
- exclude group:'org.codehaus.groovy', module:'groovy'
- exclude group:'jline', module:'jline'
- }
- api "org.springframework.boot:spring-boot-autoconfigure:$springBootVersion"
+ implementation platform(project(':grails-bom'))
- api "org.codehaus.groovy:groovy"
- api project(":grails-datastore-gorm-neo4j")
- api "org.springframework:spring-tx:$springVersion"
+ api 'org.springframework.boot:spring-boot-autoconfigure'
+ // DispatcherServletAutoConfiguration (referenced via
@AutoConfigureBefore) moved to spring-boot-webmvc.
+ api 'org.springframework.boot:spring-boot-webmvc'
+ api 'org.apache.groovy:groovy'
+ api project(':grails-datastore-gorm-neo4j')
+ api 'org.springframework:spring-tx'
testRuntimeOnly "org.neo4j.test:neo4j-harness:$neo4jVersion"
- testImplementation ("org.spockframework:spock-core:$spockVersion") {
- exclude group: 'junit', module: 'junit-dep'
- exclude group: 'org.codehaus.groovy', module: 'groovy-all'
- exclude group: 'org.hamcrest', module: 'hamcrest-core'
- transitive = false
+ testImplementation 'org.spockframework:spock-core'
+ testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
+}
+
+test {
Review Comment:
For every other projects we've never left this in the main build file -
we've extracted a project specific file so that it can be reused if needed.
##########
grails-data-neo4j/examples/test-data-service/build.gradle:
##########
@@ -1,37 +0,0 @@
-/*
Review Comment:
Re-checked at the current head (40b7aa1bf1): neither 0ca480de0d
(neo4j-standalone) nor 3afc76ae58 (test-data-service) is on this branch —
grails-test-examples/neo4j/ still contains only the original three apps. Please
push them.
##########
gradle.properties:
##########
@@ -40,6 +41,9 @@ jnrPosixVersion=3.1.20
joddWotVersion=3.3.8
joptSimpleVersion=5.0.4
jspApiVersion=4.0.0
+logbackClassicVersion=1.4.14
+neo4jDriverVersion=4.4.13
Review Comment:
Verified at the current head (40b7aa1bf1): the strictly constraint in
dependencies.gradle is consumed by grails-bom's build (which declares
customBomDependencies with `strictly`), so the BOM now manages and wins this
version. This thread is resolved.
##########
gradle/publish-root-config.gradle:
##########
@@ -138,6 +138,10 @@ def publishedProjects = [
// graphql
'grails-data-graphql',
'grails-data-graphql-core',
+ // neo4j
+ 'grails-data-neo4j',
+ 'grails-datastore-gorm-neo4j',
+ 'gorm-neo4j-spring-boot',
Review Comment:
Re-checked at the current head (40b7aa1bf1): the RENAME.md /
rename_gradle_artifacts.sh mappings landed, but the `gorm-neo4j-spring-boot` →
`grails-data-neo4j-spring-boot` rename described above references commit
d580d61465, which is not on this branch — settings.gradle and
publish-root-config.gradle still use the old name. Looks like that work was
never pushed; please push the branch.
##########
settings.gradle:
##########
@@ -397,6 +414,16 @@
project(':grails-test-examples-mongodb-test-data-service').projectDir = new File
include 'grails-test-examples-mongodb-gson-templates'
project(':grails-test-examples-mongodb-gson-templates').projectDir = new
File(settingsDir, 'grails-test-examples/mongodb/gson-templates')
+// functional tests - neo4j examples
+include 'grails-test-examples-neo4j-grails3-neo4j'
+project(':grails-test-examples-neo4j-grails3-neo4j').projectDir = new
File(settingsDir, 'grails-test-examples/neo4j/grails3-neo4j')
Review Comment:
Re-checked at the current head (40b7aa1bf1): none of e2e4522931 (Forge
integration), 0163e87554 (grails-datastore-gorm-neo4j →
grails-data-neo4j-core), or 7eedcb91cc (example-app renames) is on this branch
— grails-forge/ is untouched and settings.gradle still uses the old module and
example names. Please push them.
##########
settings.gradle:
##########
@@ -397,6 +414,16 @@
project(':grails-test-examples-mongodb-test-data-service').projectDir = new File
include 'grails-test-examples-mongodb-gson-templates'
project(':grails-test-examples-mongodb-gson-templates').projectDir = new
File(settingsDir, 'grails-test-examples/mongodb/gson-templates')
+// functional tests - neo4j examples
+include 'grails-test-examples-neo4j-grails3-neo4j'
Review Comment:
The onlyNeo4jTests/skipNeo4jTests wiring and the DEVELOPMENT.md update are
on the branch, but commit a6aa7bd8cd (the neo4jFunctional CI job and publish
gating) is not — .github/workflows/gradle.yml has no Neo4j references at the
current head (40b7aa1bf1). Please push it.
--
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]
