search

gpc / grails-audit-logging-plugin

The Grails Audit Logging Plugin
Apache License 2.0
50 stars 60 forks source link

Changes to withNewSession semantics make it useless for audit log listener #173

Closed longwa closed 3 years ago

longwa commented 6 years ago

Per behavior changes in: https://github.com/grails/grails-data-mapping/issues/1130

The call to withNewSession in the listener should be removed as it isn't occurring in the same transactional context as the caller anymore.

Audit logging should be atomic to the transaction making the changes such that both commit or rollback together. Previously, withNewSession was useful to prevent contaminating the outer session.

longwa commented 6 years ago

I'm testing this change in our application now. Assuming no issues, we should probably include this in 3.0.2 as well.

longwa commented 6 years ago

Doing a bit more testing, it actually is required in some cases as the listener is being called during a flush and should add anything else to the session in that case. However, it's not clear with the withNewSession behavior if the transactional context is preserved. One option could be to defer the actual save logic until after successful commit of the transaction by using TransactionSynchronizationManager.registerSynchronization and add an afterCommit handler.

That might not be a bad idea anyway since it would defer any impact of the audit log until successful commit in all cases (not just in the flushMode = COMMIT) case.