Avoid re-wrapping query validation exceptions into `IllegalArgumentException` it they already are one

[odrotbohm]

History

In around 2012 we faced the problem of some persistence providers not producing the correct exceptions for calls to EntityManager.createQuery(…) (reported here). As a response, we added a rather broad catch clause to SimpleJpaQuery.validateQuery(…) that wraps any RuntimeException in an IllegalArgumentException.

Problem

If the original exception already is an IAE, and that seems to be the case as of today, we will just end up re-wrapping it without further benefit. This was brought up here, just recently.

Suggested solution

We could actually just throw the original exception to avoid the wrapping for this particular case.

[odrotbohm]

Digging into this, it's unfortunately not as easy as anticipated. The bootstrap of a repository includes a couple of layers of abstraction to be able to plug different implementation strategies. Schematically, they look something like this:

─ 1. Inititalization of the repository instance
  └─ 2. Initializing each query method
     └─ 3. Initializing queries corresponding to the query method
        └─ 4. Invoking the persistence provider for actual verification

The assumed obsolete step is 3, as I originally assumed that one @Query method would lead to the declared query be validated. In other words, abstraction level 2 would just be enough to capture the context of level two. There are two complications to that:

1. Level 2 is the level of abstraction at which the query lookup strategies live, i.e. the code that decides whether a method is backed by a named query, declared query or a derived one. Level 2 doesn't actually know about the actual query yet.
2. On level three, we now use the query declared in @Query, but for pagination cases, we have to verify two queries.

On level three, we currently do not report the actual query being validated, which might create the impression that 3 is identical to 2, which it is not. So, I think we rather should enrich the exception message on level 3 with the actual query and keep that wrapping.

[odrotbohm]

It turns out, we actually could avoid one layer of indirection, if QueryCreationException in Spring Data Commons offered a factory method to take a message, QueryMethod and cause. In that case, SimpleJpaQuery could already produce that, and it would bubble through QueryExecutorMethodInterceptor.lookupQuery(…) which usually catches all exceptions but QCE and adds the repository / method context.

The method signatures in QCE are a bit of a mess, as the parameter order doesn't follow any consistent pattern. Maybe we can use the chance to clean that up a bit and add the necessary overload. Happy to take this forward, I'd just like a round of feedback before I go ahead and do so. Also fine, if someone from the core team wanted to take ownership.

[YongGoose]

@odrotbohm

I’m interested in this issue.
May I ask if there are any updates on it?