spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.25k stars 37.98k forks source link

HibernateAccessor.flushIfNecessary(): do not flush new Session in case of FlushMode.AUTO/COMMIT [SPR-2922] #7608

Closed spring-projects-issues closed 17 years ago

spring-projects-issues commented 17 years ago

Thomas Whitmore opened **SPR-2922*** and commented

Hi Costin, Juergen !

FLUSH_COMMIT mode doesn't work at all like supposed to.

FLUSH_NEVER sets the Session that way also; prevents your explicit Tx.Commit() from flushing anything; thus typically nothing saved.

this has come up on the forums (I've posted there also). we note that in other threads where there was a good answer, Costin pops up quite quickly with a response... and he's thoroughly absent from this one.

conclusion: HibernateAccessor. flushIfNecessary() is broken or significantly flawed in design. but Spring is great, otherwise :-)

Cheers, T


Affects: 2.0 final

spring-projects-issues commented 17 years ago

Juergen Hoeller commented

Sorry, it's not clear to me what you expect here. I got a bit confused by your description... What did you actually intend to achieve? What exactly was costly to debug for you?

HibernateAccessor is the base class of HibernateTemplate and HibernateInterceptor. In that context, it allows for applying a local flush mode for the operations performed by that accessor. The specified flush modes are simply applied to the underlying Hibernate Session, for the duration of each operation. FLUSH_EAGER is a bit of a special case, since it's essentially AUTO with a flush at the end of the current operation. This is how it's defined.

Accordingly, FLUSH_COMMIT or FLUSH_NEVER do not change the overall transaction's semantics; they rather just affect the local operation's scope. This can be used to suppress automatic flushes for a specific scope, etc. If you're running outside of a transaction, the flush modes show direct effect on the short-lived Session, but well, it's recommended to always run within a managed transaction in the first place.

Juergen

spring-projects-issues commented 17 years ago

Thomas Whitmore commented

Hi Juergen,

I have used the OpenSessionInViewInterceptor to open Hibernate Sessions around web request handling, but have chosen very clear & explicit transaction control -- ie. programmatic. This uses the Hibernate JDBCTransaction, obtained via session.beginTransaction().

The problem appears to be, the HibernateAccessor:

/**
 * Flush the given Hibernate Session if necessary.
 * @param existingTransaction if executing within an existing transaction
 */
protected void flushIfNecessary(Session session, boolean existingTransaction) throws HibernateException {
    if (getFlushMode() == FLUSH_EAGER || (!existingTransaction && getFlushMode() != FLUSH_NEVER)) {
        logger.debug("Eagerly flushing Hibernate session");
        session.flush();
    }
}

Since I am performing a Commit explicitly on the JDBC transaction, and wish that to flush the Hb Session, I need a flush-mode greater than 'NEVER'. I perform the commit in the Controller programatically, as explicit control over state changes is I believe a strongly preferrable design practice.

However the code illustrated above, will also issue a second FLUSH from the 'postHandle()' method; unless the flush-mode is locked down to NEVER. The 'postHandle()' method is hard-coded to call with existingTransaction=false, this is quite probably correct as my Tx lifespan is shorter (only within a specific duration of Controller processing) than the request-handling overall.

Completing the circle, if I do set flush-mode NEVER, then my call to Tx.Commit now will not flush from Hibernate... the difficult debugging is the length of time to figure out why nothing gets saved :(


Specific fix:

Code: (written in here, not tested - my practical fix is a method override on this.)

protected void flushIfNecessary(Session session, boolean existingTransaction) throws HibernateException {
    int mode = getFlushMode();
    if (mode == FLUSH_EAGER || (!existingTransaction && mode() != FLUSH_NEVER && mode != FLUSH_COMMIT)) {
        logger.debug("Eagerly flushing Hibernate session");
        session.flush();
    }
}

Thanks Juergen, Cheers,

Thomas

If the flush-mode is NEVER

The code

spring-projects-issues commented 17 years ago

Thomas Whitmore commented

... And I'm quite happy with a non-managed Transaction; until such time as I need otherwise.

Simple, explicit & programmatic 'control' over state-changes in an information system, is measurably the simplest, least complex and most reliable of controlling state.

This is by direct 'imperative' statements, which are first-class within the language; have the most-direct correspondence with the explicit/ imperative nature of actions issued to the database; and have provably lowest complexity in terms of control-path, cyclomatic complexity or any similar measure.

My controller code (snippet):

    // create/ update Bean to HB
    //
    Session sess = dbService.getSession();
    //
    Serializable id = state.getEditingId();
    String idStr = (id != null ? Info.strOrErr(id) : "<new>");
    log.debug( "save edited", idStr);
    //
    DbTransaction tx = dbService.beginTransaction( sess);
    try {

        doBeforeSave( bean, rh, sess);
        doSave( bean, rh, sess);
        tx.commit();

    } finally {
        tx.endOrRollback();
    }
    id = dbService.getBeanId( bean, sess);
    log.debug( "  saved", id);

Couple of extra design points can be seen, can't go thru in depth... but: 1) Wrapping hibernate's Transaction to have an 'idempotent' EndOrRollback() method is simpler; allows reliable closure in a 'finally' clause, rather than a 'catch' clause which must do something with the exception.

2) I replace standard Controller with a more-strongly structured controller; basically to 'flatten' request-handling, decrease the depth of template-method use; and make info such as IsNewForm()/ state/ form/ bean/ kind of request/ standardized selector & param1/2 for commands, etc, available 'flat' anywhere in the Controller via an RequestHandling public 'bus' object.

RH is basically a set of public fields... If Java allowed read-only fields with setters to write I might use those; until then, access rather than encapsulation is the point.

RH also includes, of course, the HttpRequest and HttpResponse. :)

Depth of call-heirarchy reduces with this approach, from typically 5-6 down to about 3; biggest benefits are in end- to- end information flow between 'intake' of Controller and 'execution/ output' end. eg HandleSubmit.

(Example case; providing a Delete button or DetailList add/remove commands on-page, which either bypass or do not block on validation.)

Cheers, Thomas

spring-projects-issues commented 17 years ago

Juergen Hoeller commented

Changing the status of this to an enhancement request: The present flush mode arrangement assumes that there is either a managed transaction that the HibernateTemplate participates in, or that it is operating on a new non-transactional Session. The present FlushMode AUTO/COMMIT behavior arguably makes sense on that basis, and also has to be kept for backwards compatibility (it's been that way for years).

What you intend to use HibernateTemplate and its flush modes for is local transaction management within a HibernateCallback, which was never intended in the original HibernateTemplate design. Arguably this is more reasonable than relying on new non-transactional Sessions, at least for write operations, so we might change the template's flush semantics accordingly; however, we cannot do this in a point release, so it will have to wait for Spring 2.1.

For the time being, consider creating a custom HibernateTemplate subclass, overriding the protected "flushIfNecessary" method to not flush unless in flush mode EAGER (or, for your purposes, simply never flush in the template at all).

Juergen

spring-projects-issues commented 17 years ago

Thomas Whitmore commented

Hi Juergen, thanks for your comments.

Actually, I'm not actually using HibernateTemplate or Callback to wrap the execution of my code -- at least not directly.

Instead the app. design uses SessionInView Interceptor approach to provide a Hb session, across the lifespan of the Controller and View rendering; as the simplest & most flat solution to 'read data' requirements.

In the case of transactioning, I'd strongly prefer 'explicit' transaction control; for security & fundamental reasons of 'action control' in the application design. (Don't do anything significant, like updating the database, without explicit permission -- in the form of an instruction to do so).

Design- wise it seems to me that explicit TX control, being the simplest & flattest means to perform a TX, should work out of the box? —

Since it's the simplest, I expect it would often be the first solution considered by many developers; also the major approach documented by Hibernate. Covering a significant portion of the 'solution space', especially the simplest solutions... and with a significant 'complexity of investigation/ resolution'... we can thus prove mathematically (almost) that the result is a high 'developer headscratch total'.

:)

The present flush mode arrangement assumes that there is either a managed transaction that the HibernateTemplate participates in,

So if I wanted to obtain a Managed Transaction, this (might) work better. Suggestions as to where/ how to start looking at these, if I want to use it in a explicitly demarcated/ controlled way ?

The present FlushMode AUTO/COMMIT behavior arguably makes sense on that basis, and also has to be kept for backwards compatibility (it's been that way for years).

Off the top of my head, can't see why FlushIfNecessary( mode=COMMIT) is true. Committing the Hibernate TX triggers a flush at that time... that flush (runs thru different mechanism) is the only one necessary.

I suppose this must be one of the downsides, of unifying everything into central Tx Management ? Is that the architectural reason for this?

What you intend to use HibernateTemplate and its flush modes for is *local transaction management within a

HibernateCallback*, which was never intended in the original HibernateTemplate design. Arguably this is more reasonable than relying on new non-transactional Sessions, at least for write operations, so we might change the template's flush semantics accordingly; however, we cannot do this in a point release, so it will have to wait for Spring 2.1.

Hmm. Since then I've moved on thru Long Sessions, and other advanced technology. I have a very effective & nicely generalized interceptor that provides these... and have been on some fair deep investigations of Hb and Spring code :)

I've considered providing this to you (and Spring), the interceptor is general & performant -- but unfortunately locked to our app infrastructure, since it requires more advanced Controller and Page State capabilities than Spring currently provides... there are a range of design limitations appearing/ or near on the horizon, as I consider a couple of other infrastructure possibilities.

For the time being, consider creating a custom HibernateTemplate subclass, overriding the protected "flushIfNecessary" method to not flush unless in flush mode EAGER (or, for your purposes, simply never flush in the template at all).

Yeah, the first was already my solution. As to the second, I think SessionInView Interceptor will always call flushIfNecessary() at the end of the request which is indeed correct -- but more so if FlushIfNecessary rule operates correctly :)

I'd be interested to send you some framework code... if I can get permission to release.

The style of this is quite different to what you've been seeing, and incorporates some very nice innovations. Might just blow your socks off. It would be for interest/ informational use/ and maybe use some of the techniques... it's at a higher level than the Spring framework itself, so not just a bunch of code to drop in.

Flick me an email if you're interested -

Cheers, Thomas

spring-projects-issues commented 17 years ago

Juergen Hoeller commented

Closing this as "Won't Fix" for the time being, since we won't change HibernateTemplate's semantics due to backwards compatibility concerns - and we haven't had any votes for this issue. Note that you can always use Hibernate's SessionFactory directly (openSession - beginTransaction - commit/rollback - Session.close) instead of going with HibernateTemplate in such a scenario. The SessionFactory may of course still be injected by Spring; the DAO would just not use the HibernateTemplate helper class here.

That said, watch out what we're doing in the oncoming Spring Core and Spring Web Flow releases in terms of conversational persistence context management! This might come pretty close to what you eventually went with in your scenario.

Juergen