DataNucleus JIRA is now in read-only mode. Raise any new issues in GitHub against the plugin that it applies to. DataNucleus JIRA will remain for the foreseeable future but will eventually be discontinued
Issue Details (XML | Word | Printable)

Key: NUCCORE-882
Type: New Feature New Feature
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Unassigned
Reporter: Stephane
Votes: 0
Watchers: 0

If you were logged in you would be able to see more operations.
DataNucleus Core

Allow hook for TransactionEventListener to receive beforeFlush event

Created: 25/Jul/12 10:57 PM   Updated: 04/Oct/12 05:02 PM   Resolved: 24/Aug/12 09:17 PM
Component/s: Persistence, Transactions
Affects Version/s: 3.1.0.release
Fix Version/s: 3.1.1

File Attachments: 1. Text File NUCCORE-882-cleanedup.patch (5 kB)
2. Text File patch.patch (4 kB)

Severity: Proof of Concept

 Description  « Hide

I need ane enhancement of the TransactionEventListener to get notified before flushing the transaction.

best regards,

Sort Order: Ascending order - Click to sort in descending order
Stephane added a comment - 25/Jul/12 10:59 PM
Implementation suggestion

Andy Jefferson added a comment - 26/Jul/12 08:05 AM
Thanks, but a patch ("diff -u") is what is required, since that says what has changed and is easily applied.

Stephane added a comment - 26/Jul/12 09:21 AM
here is a patch. Hope it's the right format.


Andy Jefferson added a comment - 26/Jul/12 10:54 AM
Patch doesn't apply against SVN trunk of "core" project ... in particular ObjectManagerImpl changes won't apply.

Stephane added a comment - 30/Jul/12 12:41 PM
Here is a patch for the core project.

I've made an update before creating the patch.

is it better ?

Andy Jefferson added a comment - 31/Jul/12 11:30 AM
Stephane, it applies this time, thx.
However I need some description of what is "preListener" and "postListener"; if a user wants to register a listener then fine, but don't see why they need two to do the same thing ... one taking in an Object and the other not.

Also, when a user calls pm.flush() this likely won't be triggered as far as I can see ... since that goes via ObjectManagerImpl.flush() not via TransactionImpl, hence your callback is incorrect

Stephane added a comment - 08/Aug/12 07:09 AM

I will certainly remove the second listener it seems to be useless for DataNucleus.

I've seen a detail in the MetaData class (core project). It contains the extensions as a Collection with a comment saying we don't need ordering so HashSet.
Is it a problem to move it as a List to maintain the order of declaration?

Andy Jefferson added a comment - 08/Aug/12 02:09 PM
A user can specify metadata <extension> in any order, and the order should mean nothing ... ordering in an XML file, or in annotations on a field mean nothing. So no, that change would make no sense, and Set has less overhead.

Stephane added a comment - 15/Aug/12 10:34 PM
Hi Andy,

In attachment you will find the DN2.patch. According to your suggestion I've removed the postListener it's useless.
I've also renamed the property to datanucleus.transactionEventListener.

Since the last post I think there are no more changes.

Let me know when this will be available.

many thanks

Andy Jefferson added a comment - 16/Aug/12 06:31 PM
This patch is using yours but cleaning it up (so it doesn't change the formatting of many lines in ObjectManagerImpl, and sticks to DN source formatting conventions). Please base all further patches on this one.

Comment on your patch :-
As said in a previous comment, if the user calls "pm.flush()" then flushStarted() will not be called (IMHO), so it is not consistent. ObjectManagerImpl.flush() is the place that will be called in that situation so you need to add code there to call all txn listener flushStarted() somehow.

Andy Jefferson added a comment - 17/Aug/12 11:21 AM - edited
Further comment :
you can already register your own txn listener as follows
and since the user will have to use DN classes to define their own TransactionEventListener then I don't see much point in making the registration of this something done via a persistence property - would be better to add a method to JDOPersistenceManager to register a txn listener. I just added this as

So the user can now do

The only remaining thing on this issue is the "flushStarted" event which doesn't seem to do that as you have it.

Stephane added a comment - 23/Aug/12 01:50 AM
Hi Andy,

All your last recomendations are implemented in this Patch.

thanks for your support.

Andy Jefferson added a comment - 23/Aug/12 08:15 AM

thanks, but please revise your patch so that it just shows what lines are changed. If you look at the patch it is changing pom.xml (why?), various intellij files (they aren't in SVN and don't want them), and then in ObjectManagerImpl changing many many lines ... removing a space at the end of a line? why? I need a patch that shows just the bits to change, not some reapplication of formatting (DN has its own formatting conventions already).

From what I could see in ObjectManagerImpl there is no change at all around flush() so I repeat my question ... how is a user-invoked pm.flush() being detected for this callback?

Stephane added a comment - 23/Aug/12 10:43 PM

The space changes are in fact a default behaviour in IntelliJ, it unifies the layout (replacing space by tabs...)

The changes in flush are in flushInternal at lines 3788.

Here is a new patch. I made changes in a good old editor to avoid these drawbacks.

thanks Andy


Andy Jefferson added a comment - 24/Aug/12 09:17 PM
SVN trunk has an adaption of your patch; changed event name to transactionPreFlush() to match others. Changed some of placement to make more consistent. Look in test.jdo.general "PersistenceManagerTest" for a test and what the expectation is (currently), and a TODO to improve when the flush event is called (see also TransactionImpl) so that we minimise the commit calls to flush.