borinquenkid commented on code in PR #15972:
URL: https://github.com/apache/grails-core/pull/15972#discussion_r3564695412
##########
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:
Fixed in 3be1f37632. I initially tried registering `neo4j-java-driver` as a
BOM-managed version in `dependencies.gradle`, but that broke
`validateDependencyVersions` repo-wide — `grails-data-mongodb-docs` (unrelated)
transitively resolves a newer driver via Spring Boot's BOM, which then exceeds
the artificially-low `4.4.13` floor the BOM would claim to manage. Reverted
that and kept the original `gradle.properties`-based approach, now with a
comment explaining why this is a deliberate, documented exception rather than
something the BOM should manage (per the repo's own
`validateDependencyVersions` rules).
Also dropped the unjustified `logbackClassicVersion` pin — no reason for it
was ever documented and it wasn't actually needed; it now inherits from Spring
Boot's BOM.
##########
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:
Partially addressed in 3be1f37632: registered the `org.grails` ->
`org.apache.grails` coordinate mappings for all three published Neo4j modules
in `RENAME.md` and `rename_gradle_artifacts.sh` (mirroring the `redis_mappings`
pattern).
Left open, as judgment calls rather than defaults:
- Renaming `gorm-neo4j-spring-boot` to `grails-data-neo4j-spring-boot`
(matching the mongodb/hibernate5 precedent) — agree this is cheapest before the
first release, but it touches `project(...)` references across
`settings.gradle`/build files, so it felt like a decision for maintainers
rather than something to default into.
- The developer-list merge check into `PublishPlugin.groovy` — haven't
verified this yet.
##########
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:
Partially addressed in 3be1f37632. Checkstyle is genuinely clean here (0
violations), so I removed `ignoreFailures` for it entirely.
Found something concerning while verifying: CodeNarc is **not** actually
clean, despite this module's own PR5 test plan claiming "0 CodeNarc violations"
— a stale Gradle build cache was hiding it. A `--rerun-tasks` check shows 270
CodeNarc violations across 24 files, mostly in the
`org.grails.datastore.gorm.neo4j.*` Cypher query builder classes that PR5's
cleanup never actually touched (it only covered `grails.neo4j.*`,
`grails.neo4j.mapping.MappingBuilder`, and `grails.neo4j.services.Cypher`).
I've kept `ignoreFailures` for CodeNarc only, with a comment documenting the
real count and why `codenarcFix` is unsafe to blindly run here (a Cypher
literal with an embedded single quote would break if its outer quotes were
flipped). This needs a dedicated follow-up pass.
##########
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:
Partially addressed in 3be1f37632: added `onlyNeo4jTests`/`skipNeo4jTests`
flags to `test-config.gradle`, `functional-test-config.gradle`, and
`grails-data-tck-config.gradle` (matching the hibernate5/hibernate7/mongodb
pattern), documented in `DEVELOPMENT.md`.
Deliberately left open: a dedicated Neo4j CI job and gating snapshot
publishing on it in `.github/workflows/gradle.yml`. That's a bigger call (adds
to the CI matrix's cost/runtime) that felt like it needed a maintainer decision
rather than a default addition.
##########
grails-data-neo4j/GORM_REGISTRY_MIGRATION.md:
##########
@@ -0,0 +1,181 @@
+# Neo4j → GormRegistry: migration plan
Review Comment:
Done in 3be1f37632 — removed from the repo. The durable operational facts it
captured (the `codenarcFix`-corrupts-Cypher-strings hazard, the force-block
rationale) were already preserved as code comments in the relevant build files;
the rest was PR-sequencing narrative that's now redundant with the PR
descriptions.
##########
grails-data-neo4j/examples/test-data-service/build.gradle:
##########
@@ -1,37 +0,0 @@
-/*
Review Comment:
Not addressed in this pass — this is a test-coverage/feature gap rather than
a build/CI issue, so it was out of scope for the fixes just pushed
(3be1f37632). Still open.
##########
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:
Not addressed in this pass either — Forge integration and the
naming-convention question are feature/product decisions, not build issues, so
I left them out of scope for the fixes just pushed (3be1f37632). Still open.
--
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]
