codeconsole opened a new pull request, #16214:
URL: https://github.com/apache/grails-core/pull/16214
## Problem
`@ReadOnly` means different things depending on which datastore is
underneath.
On Hibernate it genuinely suppresses the flush:
```groovy
// GrailsHibernateTransactionManager:55
if (definition.isReadOnly()) {
holder.session.setHibernateFlushMode(FlushMode.MANUAL)
}
```
On the `DatastoreTransactionManager` path there is no equivalent. `doBegin`
sets `FlushModeType.COMMIT` — already the `Session` default, and as strict as
the JPA enum gets, since it has no `MANUAL`/`NEVER` — and `doCommit`'s guard is
then defeated by the transaction's own commit:
```java
// DatastoreTransactionManager.doCommit
if (!status.isReadOnly()) {
if (session != null) { ... session.flush(); } // skipped when read-only
}
transaction.commit(); // ...which flushes
anyway
```
Both implementations flush unconditionally — `MongoTransaction.commit()` and
`SessionOnlyTransaction.commit()`. So a read-only transaction writes whatever
the surrounding session had queued.
That matters because a read-only transaction has no pending operations of
its own. Anything it flushes belongs to the caller's session, and gets written
at a moment the caller did not choose. The sharp version is a read taken
*during* a flush — a referential check in a validator, or a `beforeInsert` hook
— where the commit re-validates the entity being saved and the validator reads
again, recursing until the stack is gone.
## Change
`Session.beginTransaction(TransactionDefinition)` already existed for
exactly this; `AbstractSession` discarded the argument:
```java
public Transaction beginTransaction(TransactionDefinition definition) {
transaction = beginTransactionInternal(); // definition dropped
return transaction;
}
```
It now passes the definition to an overridable
`beginTransactionInternal(TransactionDefinition)` whose default delegates to
the existing no-arg method, so datastores that do not override it — Neo4j and
the simple map datastore — keep exactly the path they had today. Mongo
overrides it, and `MongoTransaction` / `SessionOnlyTransaction` decline to
flush when the definition is read-only.
Read-write commits are untouched.
## Tests
Three added, on both paths — server-side transactions
(`MongoTransactionSpec`) and the session-only fallback
(`MongoTransactionDisabledSpec`):
- a read-only transaction commits without flushing the surrounding session
(both paths)
- a read-write transaction still flushes it (guards the scope of the change)
Both read-only tests fail on `8.0.x` without the source change and pass with
it; the read-write test passes either way. Full `:grails-datastore-core:test`,
`:grails-data-mongodb-core:test` and `:grails-data-simple:test` are green.
## Notes
Independent of #16212 — that one removes a `@ReadOnly` from `GormService` in
scaffolding and does not depend on this; this stands on its own if that one is
rejected, and vice versa. #16212 is where the behaviour was first noticed.
Worth a maintainer's judgement: applications that write inside an
`@ReadOnly` method on a `DatastoreTransactionManager` datastore currently get
those writes at commit, and after this they would not. That is already the
behaviour on Hibernate, so this makes the two consistent rather than inventing
a new rule — but it is a behaviour change, so if you would rather it waited for
a major, or landed with a warning when a read-only commit finds pending
operations, I am happy to rework it.
--
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]
