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

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(a)hibernate.org&gt; 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(a)hibernate.org&gt; 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(a)hibernate.org&gt; > > 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(a)hibernate.org&gt; 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(a)hibernate.org&gt;: > >> > > >> >> 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(a)gmail.com&gt; > >> >> 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(a)hibernate.org&gt; > >> >> > 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(a)hibernate.org&gt; > >> >> > > 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(a)lists.jboss.org > >> >> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > >> >> > > > >> >> > _______________________________________________ > >> >> > hibernate-dev mailing list > >> >> > hibernate-dev(a)lists.jboss.org > >> >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev > >> >> > > >> >> _______________________________________________ > >> >> hibernate-dev mailing list > >> >> hibernate-dev(a)lists.jboss.org > >> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > >> >> > >> > _______________________________________________ > >> > hibernate-dev mailing list > >> > hibernate-dev(a)lists.jboss.org > >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev > >> _______________________________________________ > >> hibernate-dev mailing list > >> hibernate-dev(a)lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/hibernate-dev