Repository navigation
refactor(grails-data-mongodb): idiomatic Groovy cleanup of the J2G output - #16682
Open
borinquenkid wants to merge 19 commits into
Open
borinquenkid wants to merge 19 commits into
borinquenkid wants to merge 19 commits into
Conversation
…-groovy-datamapping-validation
…tor/java-to-groovy-grails-datamapping-core
…ctor/java-to-groovy-grails-encoder
…-to-groovy-runtime-small
…to-groovy-datamapping-satellites
…tor/java-to-groovy-grails-data-mongodb
…o-groovy-datamapping-validation
…ctor/java-to-groovy-grails-datamapping-core
…actor/java-to-groovy-grails-encoder
…a-to-groovy-runtime-small
…-to-groovy-datamapping-satellites
…ctor/java-to-groovy-grails-data-mongodb
…onversion output Behaviour-preserving idiom cleanup of the Java-to-Groovy conversion's output for grails-data-mongodb (bson/core/embedded/spring-data). No public API or behaviour change. Applied: - for (X x : coll) -> for (X x in coll): 69 sites across BsonQuery, MongoDatastore, MongoSession, MongoMappingContext, AbstractMongoObectEntityPersister, MongoEntityPersister, MongoQuery, AbstractMongoSession, EmbeddedMongoInitializer, JsonWriter (a plain colon for-each the converter had left as Java syntax). - Collection literals for no-arg constructors (new ArrayList() -> [], new HashMap<>() -> [:], new LinkedHashSet<>() -> [] as Set, declared type kept): ~40 sites in BsonQuery, MongoDatastore, MongoSession, MongoMappingContext, AbstractMongoObectEntityPersister, MongoEntityPersister, MongoQuery, EmbeddedMongoInitializer. Copy-constructor and sized-capacity call sites left alone. - String/boxed .equals( -> ==, Objects.equals(a, b) -> a == b: MongoDatastore (index name/connection-source/TTL/text-key comparisons), AbstractMongoObectEntityPersister (isDirty), MongoQuery (identity-property name check), ReplicaSetInitiator (errorCode, Boolean.TRUE primary check). Verified every operand pair is same-typed Strings or non-numeric before converting; left Arrays.asList/getKey().equals cases that touch raw Objects or fixed-size array views. - Property syntax for verified collision-free accessors: JsonReader/JsonWriter's inherited AbstractBsonReader/AbstractBsonWriter state/context/currentBsonType/name accessors (checked via javap against org.mongodb:bson for isX/getX pairs before touching any of them); BsonBinary.type/.data, BsonRegularExpression.pattern/options; RestartableMongoClient's five no-arg MongoClient/MongoCluster delegate getters; MongoCommandException.errorCode; Spring's ResourceHolderSupport (synchronizedWithTransaction, isX-only, no getX pair); MongoQuery.Near's redundant hand-written setMaxDistance(Distance) removed in favour of the property Groovy already generates for the field, with its two call sites in Near/NearSphere switched to property assignment. - instanceof + redundant cast on a local -> flow-typed call: WithinBox/WithinCircle's Box/Circle argument, Near/NearSphere's Number argument, MongoQuery's ToOne property. - Minor cleanup: dropped unneeded `this.` on unshadowed fields in AbstractMongoSession; `.getMessage()` -> `.message` (JsonReader, MongoTransaction). Deliberately left (Tier B exclusions, all confirmed by reading the accessor or by javap before deciding): - Every EmbeddedMongoSettings accessor (getPort/getVersion/getDatabaseDir/ getReplicaSet/isPersistent/isReplicaSet) kept as explicit method calls everywhere they're called (FlapdoodleMongoBackend, InMemoryMongoBackend): the class's own equals() documents that getReplicaSet()/isReplicaSet() is a genuine isX/getX collision, and each has javadoc that would be lost by converting to a property. - JsonReader/JsonWriter's getContext()/getContextType() re-declarations and the `this.@field` direct access in their Mark/Context inner classes: left exactly as converted, since each exists only to reach a protected member across the inner/outer class boundary the comments explain. - JsonWriter.Context is not @CompileStatic (confirmed by javap: its .hasElements write still compiles to ScriptBytecodeAdapter.setGroovyObjectProperty); pre-existing, not introduced by this pass, so left alone rather than papering over it with a cast. - JsonScanner, JsonTokenType, EmbeddedQueryEncoder, IndexBuildResult, MissingIndex, UndeclaredIndex, WithinPolygon, MongoIdCoercion, EmbeddedMongoBackend, EmbeddedMongoSettings, EmbeddedMongoLifecycle, FlapdoodleMongoBackend, InMemoryMongoBackend, RunningEmbeddedMongo, ContextStoppedServerApplication: read in full, already idiomatic (records, interfaces, or already using property syntax/collection literals/this.is() correctly) - left unchanged. - JsonReader/JsonScanner/JsonWriter's hand-rolled char-based tokenizer loops: no loop-level rewrites, per the hot-path rule; only null-safety/accessor idioms touched. - `MongoDatastore.options` ternary (new HashMap<>(rawOptions) : new HashMap<>()) left as-is rather than converting only one arm. - AbstractMongoObectEntityPersister's Arrays.asList(keys) kept: fixed-size array view passed to a method this file doesn't control. - No wholesale getter/setter property-syntax sweep of MongoDatastore (2.9k lines), MongoSession, MongoQuery (1.7k lines), AbstractMongoObectEntityPersister or MongoEntityPersister beyond the sites above: verifying every cross-module PersistentEntity/PersistentProperty/Criterion accessor for isX/getX collisions at that volume was out of scope for one pass: left as explicit method calls. Verification: - grails-data-mongodb-bson: compileGroovy/compileTestGroovy clean; test 323/0; codenarcMain/codenarcTest/checkstyleMain clean (0 violations, checkstyle NO-SOURCE: no Java sources). - grails-data-mongodb-core: compileGroovy/compileTestGroovy clean; test 413/0 (includes the TCK suite against the in-memory embedded backend); codenarcMain/codenarcTest/checkstyleMain clean (checkstyle NO-SOURCE). - grails-data-mongodb-embedded: compileGroovy/compileTestGroovy clean; test 161/0; codenarcMain/codenarcTest/checkstyleMain clean (checkstyle NO-SOURCE). - grails-data-mongodb-spring-data: compileGroovy/compileTestGroovy clean; test 60/0; codenarcMain/codenarcTest/checkstyleMain clean (1 file, 0 violations). - javap dynamic-dispatch audit on JsonReader, JsonWriter, RestartableMongoClient, MongoQuery, MongoDatastore, EmbeddedMongoInitializer: only initMetaClass/ compareEqual/compareNotEqual (harmless) plus the three pre-existing, unrelated setGroovyObjectProperty sites noted above; none in code this pass touched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…yism/grails-data-mongodb # Conflicts: # grails-data-mongodb/bson/src/main/groovy/org/grails/datastore/bson/json/JsonReader.groovy # grails-data-mongodb/bson/src/main/groovy/org/grails/datastore/bson/json/JsonToken.groovy # grails-data-mongodb/bson/src/main/groovy/org/grails/datastore/bson/json/JsonWriter.groovy # grails-data-mongodb/bson/src/main/groovy/org/grails/datastore/bson/query/BsonQuery.groovy # grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/config/MongoMappingContext.groovy # grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/connections/RestartableMongoClient.groovy # grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/query/MongoQuery.groovy
…yism/grails-data-mongodb
…yism/grails-data-mongodb
borinquenkid
marked this pull request as ready for review
October 20, 2026 02:09
…m/grails-data-mongodb
…yism/grails-data-mongodb
…yism/grails-data-mongodb
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on #16592 (java-to-groovy conversion for
grails-data-mongodb).Idiom pass over that conversion's output: the conversion PR keeps the original Java semicolons and double-quoted strings, under a
@SuppressWarnings('UnnecessarySemicolon')header and aTODO(groovyism-cleanup)comment, so git still recognizes each.java->.groovypair as a rename (see matrei's review on #16576). This PR removes that scaffolding and applies the Groovy idioms CodeNarc expects.Verification
CI note: this stacked branch inherits the temporary
CHANGEME FOR ACCEPTANCEworkflow-trigger commit from its base, so no CI runs on it. That commit must be reverted, and CI green, before this PR merges (blocker).🤖 Generated with Claude Code