I work to correct the @CommitAfter problems. Since I have 'overwritten' the
Tapestry HibernateManager class, I see the following behavior.
For every request a new session object is created and opened.
So when you have something like this:
myPage() {
A: userService.getUserProfile();
B: locationService.updateLocationOfUser();
C: weatherService.getCurrentWeather(userService.getLocation(user));
}
This is done inside a transaction and rolled back at the end.
So now a coworker of yours forgot the @CommitAfter. Since the current
implementation
creates a new transaction when the session object is created, and when the
session is closed
the current transaction is aborted the transaction is:
T1 = {A, B, C} + rollback
Now the fun is on updateLocation the service stores the new location of the
user inside the database and getCurrentWeather is using the location of the
user. In your test case this is
marked as correct since it is correct behavior. The location is updated and
the correct weather
is requested and shown.
But in reality your code is broken since actually after that request the
user location is not(!)
permanently stored but aborted. So your workflow is down and you spent
hours of debugging
and reading.
So now your coworker saw the problem and adds the annotation @CommitAfter
to locationService.updateLocationOfUser.
So what happens?
you get T1 {A} + commit, T2{B} + commit, T3 {C} + rollback.
Your unit test went through but actually you created a strange bug inside
your application. Between B and C you have a window where your database
might introduce a different location and your request which you expect as a
single unit will be actually broken down into three independend transaction.
Ok you can say its my fault and you might be right but we live in a test
driven environment we dont want to design ahead even for transactions
unless we do something stupid here.
Also between those transactions we might use the Object from A wrongly it
might contain staled data that are used in T2 and T3. So we have all sorts
of problems.
also I can do something like this:
session.get(User), service.updateUser(requestInformation), session.flush().
And It works. I forgot to mark my new service method as transactional and
it works. Since everytime tapestry creates a session a new transaction is
started and everytime it aborts or commits a transaction a new one
automatically begins.
So there is no way tapestry prevent me from doing stupid things.
You can blame it on me for sure. But that is not how the world works.
Also with the current implementation you will never hit a slave in a
Master/Slave SQL server setup. Since all transactions are marked as being
able to write, you always hit the master server even if you are just going
to use a read only access maybe even with a lower isolation level.
Today I switched on strict transaction declaration. I can do nothing within
a session (beside some status, connection, close etc) without a surrounding
transaction. Also I may not nest write transaction inside a read only
transaction (something that is possible now but results into two commits).
Guess what. This solution reports bugs over bugs. It was done wrong several
times.
What appears to be one transaction were many. Including all sort of changes
to introduce bugs that are a nightmare to analyse and find. Noone can get
when your database gets corrupted because you now have three transactions
instead of a single one because you called two service methods also being
annotated @AfterCommit.
This should be changed. Please do something about it. I had the same
complain when I walked into this 2008 and now I walked into it again and it
was not fixed. I consider the tapestry hibernate modul as being broken
until this is fixed and I guess you might feel the same.
Cheers,
Martin (Kersten),
Germany.
PS: I know you dont like rebinding but we need it. There is no point in
being not able to replace a already bind service implementation especially
when it is introduced by a sub module. Also I was able to simply work
around this. So please try to fix it also. I know the modules are bind in
random order so a rebind might result into logically unsolvable problems.
But please rething and try to offer a rebind. I would have needed one.
