borinquenkid commented on PR #12148:
URL: https://github.com/apache/grails-core/pull/12148#issuecomment-5098993477

   Verified this is still unimplemented as of the 8.0.x branch, but the 
proposed behavior is not compatible with GORM's data binding and, 
realistically, never will be.
   
   `propertyName[index].field` notation (e.g. `people[0].id`) is already the 
syntax GORM's data binder uses to bind indexed `List`/`Set`/array associations 
— it reads these flat parameter keys itself during binding. Having 
`GrailsParameterMap` eagerly parse `foo[0].bar` into a grouped submap under 
`foo` (as requested here and in #12142) corrupts the value the binder receives 
for indexed collection properties, breaking that binding path.
   
   I implemented the change described in this PR against 8.0.x and confirmed it 
regresses existing indexed-collection binding: 
`DataBindingTests#testBindingWithIndexedBlankId`, 
`#testUpdatingSetElementByIdThatDoesNotExist`, 
`#testBindingObjectsWithHashcodeAndEqualsToASet`, and 
`#testBindintToNestedArray` all fail with the change applied, and all pass 
without it.
   
   There's no way to distinguish the two intents syntactically — `foo[0].bar` 
means "group by index" under this proposal and "bind element 0 of collection 
foo" under existing GORM behavior, and they're the same string. Closing as 
won't-fix; a fix would require rethinking indexed collection binding at the 
data-binder level rather than in `GrailsParameterMap`.


-- 
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]

Reply via email to