I'd add this as well... I believe that in most of the cases here we are talking about (passing in a bad query, closing a closed Session, etc) these exceptions are of the "checked" sort, meaning they are easily identifiable prior to deployment. Which IMO helps mitigate somewhat the difficulty handling a change in exception type
On Mon, Apr 25, 2016 at 10:28 AM Steve Ebersole <st...@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 :) > > On Mon, Apr 25, 2016 at 9:52 AM Sanne Grinovero <sa...@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 <st...@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 <sa...@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 <gun...@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 <and...@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.v...@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 < >> st...@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 >> >> >> > > <st...@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@lists.jboss.org >> >> >> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> >> >> > > >> >> >> > _______________________________________________ >> >> >> > hibernate-dev mailing list >> >> >> > hibernate-dev@lists.jboss.org >> >> >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> >> >> > >> >> >> _______________________________________________ >> >> >> hibernate-dev mailing list >> >> >> hibernate-dev@lists.jboss.org >> >> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> >> >> >> >> > _______________________________________________ >> >> > hibernate-dev mailing list >> >> > hibernate-dev@lists.jboss.org >> >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> >> _______________________________________________ >> >> hibernate-dev mailing list >> >> hibernate-dev@lists.jboss.org >> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
