matrei commented on PR #16222: URL: https://github.com/apache/grails-core/pull/16222#issuecomment-5437050994
> Thanks for this — commits 2 and 3 (binding a `Serializable` action parameter, and generating scaffolded controllers with `Serializable id`) look like solid, independent bug fixes. Both hit any domain class that already follows the current GORM for MongoDB guide (i.e. one that already declares `String id`), so I'd support merging those regardless of what happens below. > > My concern is with commit 1 (`defaultIdType`): the type of an id already has a knob, and it's the declaration in the domain class itself. `String id` is one line, per-entity, visible in source, works under IDE compilation (which this system-property mechanism doesn't, per the PR's own limitations), and is what the GORM for MongoDB guide has recommended all along. The implicit-`Long` path this compensates for only works because GORM for MongoDB fakes auto-increment through an internal sequence/counter collection — the same shard-hostile bottleneck this PR's description calls out. As far as I can tell, the only concrete behavior change here is that an undeclared id on a MongoDB-mapped class gets `String` instead of `Long`; Hibernate's and Neo4j's trait providers are untouched. > > So the effect of the new SPI method (`getDefaultIdentityType()`), the Gradle plugin class, and the system property threaded through the compiler is to let the build config supply, invisibly and at a distance, a fact that belongs in the source of truth — at the cost that reading the domain class no longer tells you its id type (you need the build config plus the store the class resolves to), plus the IDE/Gradle divergence and the plugin-jar freezing noted in the limitations. Unlike `id generator:`, which is a real degree of freedom the type can't express, the id type has an existing, better home. > > I'd rather see a compile-time warning when a MongoDB-mapped domain class has no declared id — nudging the developer to write `String id` (or whatever they intend) explicitly — than machinery that makes the omission work. > > Requesting changes on commit 1 pending discussion of the above; happy to be convinced there's a use case that needs automatic inference rather than a warning-and-explicit-declare approach. @borinquenkid I think the use case is that a domain class in a plugin can be used by both hibernate and mongodb. -- 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]
