Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with Bean Validation 2.0 and JPA 2.2 [SPR-13482] #18061

Closed
spring-projects-issues opened this issue Sep 22, 2015 · 11 comments
Closed
Assignees
Labels
type: task A general task
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Sep 22, 2015

Juergen Hoeller opened SPR-13482 and commented

There are loose plans for JPA 2.2 and Bean Validation 2.0 revisions within the Java EE 8 umbrella in 2017. Let's make sure to be fully compatible with those API updates and support their new features where feasible (it looks like there won't be much to do anyway).

http://beanvalidation.org/2.0/spec/2.0.0.beta1/
https://jcp.org/aboutJava/communityprocess/maintenance/jsr338/ChangeLog-JPA-2.2-MR.txt


Issue Links:

1 votes, 5 watchers

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Let's reduce this ticket to Bean Validation 2.0, since Java EE 8 will only have a minor JPA 2.1 maintenance revision now.

@spring-projects-issues
Copy link
Collaborator Author

Gunnar Morling commented

Hi, we'll release Bean Validation 2.0.0.Alpha1 and Hibernate Validator 6.0.0.Alpha1 within the next few days. Happy about any feedback of course. If there is anything you think we should consider in the spec to ease usage withing Spring-based applications, please let us know, e.g. by sending a mail to beanvalidation-dev.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Hey Gunnar Morling, thanks for reaching out! We'll definitely give it a spin once available, possibly in time for our 5.0 M5 still.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Gunnar Morling, Hibernate Validator 6.0 Alpha 1 works fine with Spring as far as I can tell.

The only gotcha is the new ValidatorFactory.getClockProvider() method which forces our LocalValidatorFactoryBean decorator to declare it when compiling against the Bean Validation 2.0 API. That's not a big deal since this method is not meant to be invoked on that decorator at runtime anyway. It just means we have to keep compiling against the Bean Validation 1.1 API for the time being in order to remain compatible with both 1.1 and 2.0 at runtime.

Just wondering: Could you possibly declare such new methods as default methods (even returning null or throwing an UnsupportedOperationException), easing the job of existing ValidatorFactory / Validator decorators? Alternatively, for getClockProvider() specifically, could you possibly declare it as Supplier<Clock> instead of introducing a new interface that is only present in BV 2.0 (and therefore cannot be referenced by BV 1.1 compatible decorators)?

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Gunnar Morling, Hibernate Validator 6.0 Alpha 2 works equally well with Spring at runtime, at least for my initial round of testing.

ValidatorFactory.getClockProvider() remains an issue in terms of compiling against BV 2.0 API while retaining compatibility with BV 1.1 at runtime, since ClockProvider is a BV-2.0-specific API type which we are forced to declare in ValidatorFactory decorators but which fails to load at runtime against a BV 1.1 setup. So at present, we need to keep compiling against BV 1.1, with our LocalValidatorFactoryBean decorator simply not declaring the getClockProvider() method.

That said, it would sure be nice to upgrade our build to BV 2.0 proper. So as suggested above, I'm still in favor of having ValidatorFactory.getClockProvider() declared as a default method (analogous to the new HttpServletRequest.getPushBuilder() method in Servlet 4.0), or possibly redesigning the ClockProvider interface as Supplier<Clock> (a declaration of which would load fine against BV 1.1 on Java 8 as well). Any chance you could consider either of the two for BV 2.0 still?

@spring-projects-issues
Copy link
Collaborator Author

Gunnar Morling commented

Juergen Hoeller, sorry, I somehow missed your comment from February, the safest is always to use our mailing list.

Concerning the clock provider contract we could make it a Supplier theoretically, but I prefer the more explicit contract. This allows us to better document its semantics (e.g. that implementations must be thread-safe) and we also could add other methods in future revisions (although I don't see any candidates for those right now).

On making getClockProvider() a default method, I'm concerned that this would be counter-productive towards the indented usage, as BV providers really should implement this.

I'm wondering though what issue you actually are seeing. I'd have expected that the new type doesn't cause issues when using your decorator at runtime with the 1.1 API (as long as the new method isn't invoked of course). I just did a test, compiling a decorator against 2.0.0.Alpha2 and using it with the 1.1.0.Final API at runtime. I could invoke methods on the decorator without any problem.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Gunnar Morling, such method signatures with types that don't exist on the classpath caused a lot of subtle trouble for us over the years. It's a brittle arrangement: Any attempt to reflect upon that class (e.g. Class.getMethods()) usually breaks since the JVM tries to resolve every method signature in that step. Such an arrangement tends to break within a debugger as well where the JVM eagerly resolves signatures seen by the debugger. At least that's the case historically; you could easily verify by manually calling .class.getMethods() on your decorator type.

As for ValidatorFactory enforcing the implementation of certain signatures, I guess there's a mix of two roles for that interface: It's at the same time a primary client entry point (where client-level decorators may come in) and a sort of SPI. 2.0's getClockProvider(), like the 1.1 getParameterNameProvider, only really matters for the SPI perspective, so decorators generally don't really care about it.

In Servlet 4.0, they have a similar situation with HttpServletRequest - often decorated - and opted for declaring such methods with new API types as default methods - essentially just for leniency towards decorators which need to remain compatible with Servlet 3.1. Of course they want e.g. HttpServletRequest.newPushBuilder() to be implemented by Servlet 4.0 containers but they nevertheless made it a default method for the decorator scenario. Like for Servlet 4.0, isn't provider-level enforcement of new methods strongly enough covered by the spec document and TCK for BV 2.0 as well?

@spring-projects-issues
Copy link
Collaborator Author

Gunnar Morling commented

I see your point about Class#getMethods(). It can be impractical, but nothing insurmountable either (depending on your use case for iterating the methods you could work with a fixed lists of methods you expect ein either case). How would making it a default method help with this issue, though? Won't you have the same problem of the ClockProvider type not being available?

Do you have a pointer to the related discussion in Servlet 4.0 by any chance? I feel it's a bit of a miss-use of default methods really.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Iterating the methods usually comes from some kind of container environment, e.g. trying to find certain annotations. For better or for worse, it's the norm in both a Spring container (with annotating processing activated) and a CDI environment these days, applied to any class that appears in a list of candidate component types or even just the declared return type of a factory method.

As for the exposure of such a type on an interface (whether as a default method or not), that depends on the version of that interface in the classpath then: In BV 1.1, getClockProvider() simply won't be there, so a decorator class resolved against that version of the interface won't have the ClockProvider type appearing anywhere at runtime... As long as the decorator class doesn't declare getClockProvider() itself, it'll be a rock solid arrangement at runtime.

What we currently do (and historically always did) is to simply compile against the minimum version of any such target API, with the compiler not forcing us to implement any such new methods then. As long as those new methods aren't actually called at runtime (which won't ever be the case here on a ValidatorFactory decorator), this won't be issue - and reflection will work reliably as well. This is how we bridged BV 1.0 vs 1.1 in terms of getParameterNameProvider() and forExecutables(), simply building our decorators against BV 1.0 at compile-time and letting them run against BV 1.0 or 1.1 at runtime.

Now for Spring Framework 5.0, it's just a nuisance that we can't build against BV 2.0 (and HV 6.0) then; we'll have to keep building against BV 1.1 (and HV 5.4) until we can raise our minimum requirement to BV 2.0. In fact, we'd love to raise that requirement immediately, and BV 2.0 is easy enough to bring into Boot or Tomcat/Jetty stacks: However, we couldn't reliably run on EE 7 stacks anymore then which expose BV 1.1 by default. Even on WildFly, we need to support unpatched versions: It's not really an option to enforce a BV 2.0 upgrade for WildFly 10.x there; the best we can do is to leniently support BV 2.0 when present at runtime.

Good question about such discussions on the Servlet 4.0 EG; we'd have to ask them. They've consistently applied that principle since Servlet 4.0 b01 without much discussion, it seems. As for misuse of default methods, well, arguably the default method mechanism has been introduced for exactly such compatibility stories with existing implementations, in particular on java.util.Collection and co: see e.g. how Collection.isEmpty() doesn't have a default implementation (could easily do size() == 0) but all new additions in Java 8 are consistently declared as default methods there... Servlet 4.0 effectively does the same to HttpServletRequest (not to other API types by the way, e.g. not to ServletContext which typically doesn't get decorated by user code).

@spring-projects-issues
Copy link
Collaborator Author

Gunnar Morling commented

Juergen Hoeller, I've been thinking a bit more about the default method route.

I first assumed you suggested to add an empty default method (which is why I considered it as a "hack"), but I came to realize that you probably meant it to be an actually functional default method implementation. That would indeed be inline with the semantics of default methods, but I don't think we can do it. The problem is that those methods return the effectively configured implementations for message parameter, clock provider et al. So we cannot do a simple default implementation (e.g. for clock provider, based on the VM's time and timezone) but instead would have to implement the retrieval of configured values via XML and so on. That's not really feasible to do for the API.

I guess there's a mix of two roles for that interface: It's at the same time a primary client entry point [...] and a sort of SPI. 2.0's getClockProvider() [...] only really matters for the SPI perspective

Those methods actually also matter from the client perspective. Their use case is to obtain the value configured for the factory and create a Validator with a specific configuration via ValidatorFactory#usingContext(), using the factory-level values as a delegate. So not exposing getClockProvider() on the decorator would restrict capabilities exposed to the client (admittedly, it's a rather specific use case).

Would it be possible that you provide two decorators, one for BV 1.1 and one for 2.0. You'd probably have to have two modules of sort, or some switch in your bootstrapping routine so it plugs in the right decorator version. I see how it'd be a tad more complex on your side, but I think that'd be the cleanest approach to take.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I'll mark this ticket as resolved now, with Hibernate Validator 6.0 beta 2 tested and a few further steps taken for our upcoming 5.0 RC2:

LocalValidatorFactoryBean documents the limitations in its decorator implementation and has a commented-out version of getClockProvider() for the time being. An easy way out is unwrap(ValidatorFactory.class).getClockProvider() if really needed, and we'll add an explicit declaration once we go Bean Validation 2.0+ in Spring 6. This seems to pragmatically cover our purposes there; if this happens to be a real limitation for somebody, we can also introduce a dynamic-proxy-based variant of LocalValidatorFactoryBean which reflectively supports getClockProvider (if present at runtime) without the need to unwrap first.

Also, SharedEntityManagerCreator detects JPA 2.2's getResultStream method as query-terminating now. It is probably recommendable to avoid result streaming outside of a transaction to begin with... but just in case somebody tries it, we close our temporary EntityManager handle at that point, just like we do for getResultList and co.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

No branches or pull requests

2 participants