borinquenkid commented on PR #16651: URL: https://github.com/apache/grails-core/pull/16651#issuecomment-6207393713
Thanks for taking a look. What drives the change is #14545, and I don't think it changes the rule that objects that fail to validate don't save. **The user-visible case.** An `@Transactional` service saves several unrelated instances, and one of them fails `validate()` (the reporter's demo: independent `Comment` rows, one invalid). At commit nothing is written, so the valid, already-saved rows are silently lost. Several users on the issue hit the same thing (a Grails 6 user on Hibernate 5, and a report on Grails 8.0.0 with Hibernate 7.4.10), so it is not specific to the Hibernate 7 module. **Why.** `validate()` puts the session in `FlushMode.MANUAL` while it runs. The `finally` restored the previous mode only when validation passed, so after a failed `validate()` the session stayed in MANUAL for the rest of the transaction and the commit flushed nothing: changes made before and after the failed call are both lost. Only sessions in COMMIT (the default for read-write transactions) or MANUAL are affected. A session in AUTO is not, because `GrailsHibernateTemplate` restores it. So whether a failed `validate()` drops unrelated writes depends on the flush mode, which looks like a side effect of the MANUAL toggle rather than a designed rule. **What the change does and does not do.** It restores the flush mode unconditionally, and nothing else. An instance that failed validation is still not persisted: `save()` on it still returns null, and `ValidateRestoresFlushModeSpec` (H5) and `HibernateGormValidationApiSpec` (H7) assert that an invalid instance that was never attached is not written by the commit flush. A modified, already-loaded instance that is invalid is only flushed if the application itself keeps it dirty in the session, as before; the spec for that case discards it and checks the original row is untouched. I haven't checked whether the restore-only-on-success was a deliberate choice when the Hibernate 5 module was written. I couldn't find anything that says so, and the reports above suggest it surprises users. If you'd like a functional test that mirrors the reporter's service in the example apps, I can add one. -- 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]
