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

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