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

Steve Ebersole steve at hibernate.org
Mon Apr 25 11:28:49 EDT 2016 

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 :)

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