[hibernate-dev] Various expectation changes in hibernate-core after consolidating hibernate-entitymanager

Steve Ebersole steve at hibernate.org
Mon Apr 25 13:43:00 EDT 2016 

Currently we make a distinction between FlushMode and
`shouldFlushBeforeCompletion`.  Is there any reason for that?  I have no
idea why we decided not to have FlushMode drive
`shouldFlushBeforeCompletion`.  Anyone?


On Mon, Apr 25, 2016 at 10:36 AM Sanne Grinovero <sanne at hibernate.org>
wrote:

> On 25 April 2016 at 16:28, Steve Ebersole <steve at hibernate.org> wrote:
> > I agree with that to an extent.  However there is also a balance -
> clearly
> > any added complexity has a direct correlation to likelihood of
> errors/bugs.
> > We just have to find that balance :)
>
> Of course :)
> Just pointing this out as there was mention of "clean room" design.
>
> >
> > On Mon, Apr 25, 2016 at 9:52 AM Sanne Grinovero <sanne at hibernate.org>
> wrote:
> >>
> >> N.B. that chancing the exception types thrown by our native methods is
> >> an API change in my opinion.
> >>
> >> So while I think you can have some Hibernate exceptions now extend
> >> some of the types expected by JPA, a fully "clean room" approach is
> >> not something I'd do in a minor release.
> >>
> >> On 25 April 2016 at 15:25, Steve Ebersole <steve at hibernate.org> wrote:
> >> > I agree with Gunnar in that the underlying question here is in how we
> >> > see
> >> > the "Hibernate API" long term.  However I would consider the following
> >> > categories:
> >> >
> >> > Methods which are Hibernate-only
> >> > Methods which Hibernate had historically which share a signature with
> >> > JPA
> >> > Methods which we added to Hibernate specifically for JPA.
> >> >
> >> >
> >> > Personally, in retrospect, I would say that group (3) unequivocally
> >> > should
> >> > have been handled by just throwing the JPA exceptions.  In a
> clean-room
> >> > impl
> >> > that's what I would do.
> >> >
> >> > So I guess the question is whether we want to look at this as a
> >> > clean-room
> >> > impl or if we want to constrain ourselves to the legacy exception
> types.
> >> > I
> >> > do wonder if a hybrid approach may be the best, as touched on before.
> >> > Namely, where JPA expects an IllegalArgumentException,
> >> > IllegalStateException, etc we throw those; for the rest we'd have
> >> > Hibernate
> >> > exception extend PersistenceException.  I think that's a great
> >> > compromise.
> >> >
> >> > I'd also mention that in the IllegalArgumentException,
> >> > IllegalStateException, etc cases there are a few places where
> >> > HibernateExceptions cover the JPA case already.  E.g. JPA says that if
> >> > there
> >> > is a problem with the JPQL query (String) used to create a Query an
> >> > IllegalArgumentException should be thrown; Hibernate currently handles
> >> > this
> >> > by throwing a QuerySyntaxException - we could just make
> >> > QuerySyntaxException
> >> > extend from IllegalArgumentException rather than Hibernate's
> >> > HibernateException hierarchy.
> >> >
> >> > I think the bottom line is that having Session and EntityManager split
> >> > before caused all kinds of difficult-to-debug complexity that I'd
> rather
> >> > avoid as we consolidate them together.
> >> >
> >> >
> >> > On Mon, Apr 25, 2016 at 5:37 AM Sanne Grinovero <sanne at hibernate.org>
> >> > wrote:
> >> >>
> >> >> Gunnar's words seem wise to me: users will need to have the JPA API
> on
> >> >> classpath anyway, so I don't see why we should have - and maintain -
> >> >> strategies for different kind of exceptions.
> >> >>
> >> >> This might have been useful in the past, but looking forward?
> >> >>
> >> >> If the reasoning is that Hibernate - having more features - could
> >> >> throw more specific and informative exceptions, we could have some
> >> >> Hibernate exceptions to subclass the JPA ones?
> >> >>
> >> >> Would be nice to avoid breaking the expected exception types in a
> >> >> minor, so I'm not sure if you can do that in all cases by subclassing
> >> >> the JPA ones, but I suspect it can get you a long way.
> >> >>
> >> >>
> >> >> On 25 April 2016 at 10:13, Gunnar Morling <gunnar at hibernate.org>
> wrote:
> >> >> > The strategy approach sounds nice on first thought, but it also
> adds
> >> >> > complexity.
> >> >> >
> >> >> > I think the underlying question is: What's the long-term strategy
> >> >> > around
> >> >> > the "Classic API"? Should it remain in place for all times as a
> >> >> > complete
> >> >> > alternative to the JPA API?
> >> >> >
> >> >> > Or should we begin to deprecate it and narrow it down in favour of
> >> >> > the
> >> >> > corresponding functionality standardized in JPA, and only
> >> >> > functionality
> >> >> > not
> >> >> > present in JPA would be exposed through some kind of unwrap()?
> >> >> >
> >> >> > Without having thought about all the implications too much, I'd
> lean
> >> >> > towards the latter, in which case I vote for "1. Just move to JPA
> >> >> > expected
> >> >> > exceptions" as part of such larger effort.
> >> >> >
> >> >> > It'd be interesting to run a poll to see some figures of people
> using
> >> >> > classic vs. JPA.
> >> >> >
> >> >> > --Gunnar
> >> >> >
> >> >> >
> >> >> >
> >> >> > 2016-04-25 10:47 GMT+02:00 andrea boriero <andrea at hibernate.org>:
> >> >> >
> >> >> >> Having a Strategy gives us more flexibility  so +1.
> >> >> >>
> >> >> >> About the expectations I think what Vlad says is reasonable.
> >> >> >>
> >> >> >> On 25 April 2016 at 06:04, Vlad Mihalcea <mihalcea.vlad at gmail.com
> >
> >> >> >> wrote:
> >> >> >>
> >> >> >> > Your second email summarizes my thoughts as well. If we can
> >> >> >> > separate
> >> >> >> > the
> >> >> >> > exception handling in two separate strategies that are defined
> >> >> >> > during
> >> >> >> > bootstrap (JPA vs Hibernate),
> >> >> >> > I think that's the way to go.
> >> >> >> >
> >> >> >> > There so many projects out there that rely on the exception type
> >> >> >> > being
> >> >> >> > thrown, and changing those would make it very difficult for them
> >> >> >> > to
> >> >> >> migrate
> >> >> >> > to this new version.
> >> >> >> > But that only affects Hibernate-native projects since, for those
> >> >> >> > who
> >> >> >> > have
> >> >> >> > been using JPA, they already expect the JPA exceptions anyway.
> >> >> >> >
> >> >> >> > As for the other behavior discrepancies:
> >> >> >> >
> >> >> >> > 1. "calling EntityManager#close on a closed EntityManager should
> >> >> >> > result
> >> >> >> in
> >> >> >> > an
> >> >> >> > exception;" - that's a reasonable default and shouldn't cause
> too
> >> >> >> > much
> >> >> >> > trouble.
> >> >> >> > 2. "Another change in expectation is in regards to operations
> >> >> >> > outside
> >> >> >> > of
> >> >> >> a
> >> >> >> > transaction" - in JPA we can execute queries outside a
> >> >> >> > transaction,
> >> >> >> > but
> >> >> >> any
> >> >> >> > write will fail if there is no transactional context, which is
> >> >> >> > reasonable
> >> >> >> > for me too. If Hibernate allows writes outside of a
> transactional
> >> >> >> context,
> >> >> >> > that's definitely a thing we should not support anyway.
> >> >> >> > 3. "Asking a Session if is contains
> >> >> >> > (Session/EntityManager#contains)
> >> >> >> > a
> >> >> >> >  non-entity" - we can handle this with the separate exception
> >> >> >> > handler
> >> >> >> > strategies to retain both JPA and Hibernate behaviors.
> >> >> >> > 4. "Accessing Session/EntityManager#getTransaction.  JPA says
> that
> >> >> >> > is
> >> >> >> > only allowed
> >> >> >> > for JDBC transactions.  Hibernate always allows it." - I'd
> choose
> >> >> >> > the
> >> >> >> > Hibernate behavior because I don;t see how it can cause any
> issue
> >> >> >> > and
> >> >> >> it's
> >> >> >> > an enhancement as well.
> >> >> >> >
> >> >> >> > Vlad
> >> >> >> >
> >> >> >> >
> >> >> >> >
> >> >> >> >
> >> >> >> >
> >> >> >> > On Sat, Apr 23, 2016 at 5:29 PM, Steve Ebersole
> >> >> >> > <steve at hibernate.org>
> >> >> >> > wrote:
> >> >> >> >
> >> >> >> > > Just realized that I should have mentioned an important plan
> >> >> >> > > that
> >> >> >> > > helps
> >> >> >> > > understand the idea behind the "exception handling strategy"
> >> >> >> > > route.
> >> >> >> > > I
> >> >> >> > plan
> >> >> >> > > to keep track of how a SessionFactory was bootstrapped in some
> >> >> >> > > fashion.
> >> >> >> > So
> >> >> >> > > when it was bootstrapped from EntityManagerFactoryBuilder
> (which
> >> >> >> > > JPA
> >> >> >> > > bootstrap methods leverage) we'd select the "JPA exception
> >> >> >> > > handling"
> >> >> >> > > strategy impl.  When not, we'd use the "legacy Hibernate
> >> >> >> > > exception
> >> >> >> > > handling" strategy.
> >> >> >> > >
> >> >> >> > > On Sat, Apr 23, 2016 at 9:21 AM Steve Ebersole
> >> >> >> > > <steve at hibernate.org>
> >> >> >> > > wrote:
> >> >> >> > >
> >> >> >> > > > There are a number of "expectation changes" that come about
> >> >> >> > > > from
> >> >> >> > > > consolidating hibernate-entitymanger into hibernate-core.
> >> >> >> > > > Some
> >> >> >> > > > we
> >> >> >> have
> >> >> >> > > > discussed; some we have not.  Hopefully we can come to a
> >> >> >> > > > consensus
> >> >> >> > > regards
> >> >> >> > > > how to deal with these changes...
> >> >> >> > > >
> >> >> >> > > > The first one is different exception types.  We have
> discussed
> >> >> >> > > > this
> >> >> >> > > > before.  For now, in an effort to fix test failures and move
> >> >> >> > > > forward
> >> >> >> > with
> >> >> >> > > > developing, I simply changed failing tests to expect the JPA
> >> >> >> > > > defined
> >> >> >> > > > exceptions.  I had mentioned, where possible, to to throw a
> >> >> >> combination
> >> >> >> > > of
> >> >> >> > > > the 2 expected exceptions.  Generally this falls into 2
> >> >> >> > > > discrete
> >> >> >> > > categories:
> >> >> >> > > >
> >> >> >> > > >
> >> >> >> > > >    1. JPA expects a PersistenceException and we have
> >> >> >> > > > historically
> >> >> >> > thrown
> >> >> >> > > >    a HibernateException
> >> >> >> > > >    2. JPA expects some form of JDK RuntimeException
> >> >> >> > > >    (IllegalArgumentException, IllegalStateException, etc)
> and
> >> >> >> > > > we
> >> >> >> > > > have
> >> >> >> > > >    historically thrown a HibernateException
> >> >> >> > > >
> >> >> >> > > > It is unfortunate that Java does not allow exceptions to be
> >> >> >> > > > defined
> >> >> >> by
> >> >> >> > > > means of interfaces; that's the only "clean" way I see to do
> >> >> >> > > > this
> >> >> >> > > > -
> >> >> >> > that
> >> >> >> > > > would have allowed us to define concrete exception classes
> >> >> >> > > > that
> >> >> >> extend
> >> >> >> > > > PersistenceException, IllegalArgumentException, etc and that
> >> >> >> implement
> >> >> >> > > HibernateException.
> >> >> >> > > >
> >> >> >> > > >
> >> >> >> > > > So I see 3 potential solutions (feel free to bring up
> others).
> >> >> >> > > >
> >> >> >> > > >    1. Just move to JPA expected exceptions.
> >> >> >> > > >    2. Have HibernateException extend PersistenceException
> and
> >> >> >> > > > just
> >> >> >> not
> >> >> >> > > >    worry about the change in expectation in regards to that
> >> >> >> > > > second
> >> >> >> > > category.
> >> >> >> > > >    3. Push exception handling behind a strategy.  This would
> >> >> >> > > > have
> >> >> >> > > > to
> >> >> >> > be a
> >> >> >> > > >    pretty specific strategy for very specific cases.
> >> >> >> > > >
> >> >> >> > > > The first and second options are pretty self-explanatory and
> >> >> >> > > > straight-forward so I won't go into detail there.  Just
> >> >> >> > > > realize
> >> >> >> > > > that
> >> >> >> > > these
> >> >> >> > > > change the expectation for the user.  They'd have to change
> >> >> >> > > > their
> >> >> >> code
> >> >> >> > to
> >> >> >> > > > catch these JPA-defined exceptions.
> >> >> >> > > > The other option, I see, is to h
> >> >> >> > > >
> >> >> >> > > > The third option is perfect in theory, but it is very
> tedious.
> >> >> >> > > > For
> >> >> >> > > > example, take the case of trying to perform some operation
> on
> >> >> >> > > > a
> >> >> >> closed
> >> >> >> > > > Session/EntityManager.  Hibernate historically threw a
> >> >> >> > HibernateException
> >> >> >> > > > here.  JPA says that should result in an
> >> >> >> > > > IllegalStateException.
> >> >> >> > > > So
> >> >> >> in
> >> >> >> > > > SessionImpl#checkOpen, when the Session/EntityManager is
> >> >> >> > > > closed,
> >> >> >> > > > we'd
> >> >> >> > > > call out to the strategy to handle that condition.  Even
> more,
> >> >> >> > Hibernate
> >> >> >> > > > (historically) and JPA disagree about which methods getting
> >> >> >> > > > called
> >> >> >> on a
> >> >> >> > > > closed Session/EntityManager should lead to an exception.
> For
> >> >> >> example,
> >> >> >> > > > JPA says calling EntityManager#close on a closed
> EntityManager
> >> >> >> > > > should
> >> >> >> > > > result in an exception; Hibernate historically did not care
> if
> >> >> >> > > > you
> >> >> >> > called
> >> >> >> > > > Session#close on a closed Session.  So that is a special
> case,
> >> >> >> > > > and
> >> >> >> > every
> >> >> >> > > > one of those special cases would have to be exposed and
> >> >> >> > > > handled
> >> >> >> > > > in
> >> >> >> the
> >> >> >> > > > exception handling strategy in additional to the general
> >> >> >> > > > cases.
> >> >> >> > > >
> >> >> >> > > > Another change in expectation is in regards to operations
> >> >> >> > > > outside
> >> >> >> > > > of
> >> >> >> a
> >> >> >> > > > transaction, which I consider a questionable pattern anyway.
> >> >> >> Hibernate
> >> >> >> > > > historically allowed that; JPA explicitly disallows it.  In
> a
> >> >> >> > > > way
> >> >> >> this
> >> >> >> > > > could fall under the exception discussion above, meaning we
> >> >> >> > > > could
> >> >> >> push
> >> >> >> > > that
> >> >> >> > > > distinction behind the exception handling strategy.  Or we
> >> >> >> > > > could
> >> >> >> decide
> >> >> >> > > > that we are going to stop supporting that.
> >> >> >> > > >
> >> >> >> > > > There are a lot of other highly questionable things I have
> >> >> >> > > > seen
> >> >> >> > > > in
> >> >> >> the
> >> >> >> > > > tests that JPA explicitly disallows that I think we ought to
> >> >> >> > > > just
> >> >> >> stop
> >> >> >> > > > supporting and opt for the JPA way, although I am open to
> >> >> >> > > > discussing
> >> >> >> > them
> >> >> >> > > > if any feels strongly about them.  Some of these include:
> >> >> >> > > >
> >> >> >> > > >    - Asking a Session if is contains
> >> >> >> (Session/EntityManager#contains) a
> >> >> >> > > >    non-entity.  Hibernate historically would just return
> >> >> >> > > > false.
> >> >> >> > > > JPA
> >> >> >> > > states
> >> >> >> > > >    that should be an exception.
> >> >> >> > > >    - Accessing Session/EntityManager#getTransaction.  JPA
> says
> >> >> >> > > > that
> >> >> >> is
> >> >> >> > > >    only allowed for JDBC transactions.  Hibernate always
> >> >> >> > > > allows
> >> >> >> > > > it.
> >> >> >> > > >
> >> >> >> > > > If we go the route of an "exception handling strategy" a lot
> >> >> >> > > > of
> >> >> >> > > > the
> >> >> >> > other
> >> >> >> > > > points I mentioned above could just be pushed behind that
> >> >> >> > > > strategy.
> >> >> >> > But
> >> >> >> > > I
> >> >> >> > > > really want to start looking critically at what we support
> >> >> >> > > > today
> >> >> >> > > > that
> >> >> >> > we
> >> >> >> > > > maybe really should not be.
> >> >> >> > > >
> >> >> >> > > _______________________________________________
> >> >> >> > > hibernate-dev mailing list
> >> >> >> > > hibernate-dev at lists.jboss.org
> >> >> >> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >> >> >> > >
> >> >> >> > _______________________________________________
> >> >> >> > hibernate-dev mailing list
> >> >> >> > hibernate-dev at lists.jboss.org
> >> >> >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >> >> >> >
> >> >> >> _______________________________________________
> >> >> >> hibernate-dev mailing list
> >> >> >> hibernate-dev at lists.jboss.org
> >> >> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >> >> >>
> >> >> > _______________________________________________
> >> >> > hibernate-dev mailing list
> >> >> > hibernate-dev at lists.jboss.org
> >> >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >> >> _______________________________________________
> >> >> hibernate-dev mailing list
> >> >> hibernate-dev at lists.jboss.org
> >> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>

More information about the hibernate-dev mailing list