Ok, here comes the second part of the puzzle. The first part is the problem in ValidationExtension. Here the problem is that the original method must be used in determineConstrainedMethod when passed to isNonGetterConstrained, even if the original method was overwritten. This is necessary to ensure that the metdatata look-up via BeanDescriptor#getConstraintsForMethod will work. This is indeed convoluted in itself. If, however, the original method is used for the metadata lookup, the right interceptor bindings get created.
Now the second problem is ValidationInterceptor. Note, that the ValidatorFactory and hence the metadata cache is not shared. ValidationExtension bootstraps its on Validator just in order to determine which classes need interceptor bindings. Once ValidationInterceptor#validateMethodInvocation is called we are dealing with a different ValidatorFactory which is now the mananged CDI bean created by ValidationExtension. So initially the metadata cache is all empty and need to be re-created. This is of course not a problem per se. The problem is that the invocation instance provided by InvocationContext.getTarget is a Weld proxy. Later on in ValidationContext#forValidateParameters this proxy is used to set the rootBean and rootBeanClass. So the metadata created is really for the proxy class, not for StringImpl. This seems to work for most cases, but not for the generic use-case as in the test case. My guess is that the override checks with classmate fail in this case and hence the metadata for the proxy is different from the metadata of StringImpl itself (in fact the metadata of the proxy does not contain metadata for genericArg(String).
Really, one should generate the metadata for the proxied class, not for the proxy itself (this would also reduce a lot of unnecessary metadata creation and storage). Adding a check for a Weld proxy to ValidationContext#forValidateParameters which will set the rootBeanClass to the proxied class will make the test pass. The rub here is that this check is Weld specific. It would not work for any other proxied classes. It also needs to be checked in the engine module which cannot have a hard reference to CDI/Weld, so all proxy detection and unwrapping needs to happen via reflection.
The question of dealing with proxies has come up before (I think in the context of cascaded validation and dealing with ORM association proxies). To offer the possibility to support other types of proxies one would need a customizable proxy detection contract which would be used whenever a class needs to inferred from an instance to be validated.
Thoughts?
BTW, we already have some Weld proxy specific code when it comes to dealing with class hierarchies. Maybe in a first step we make it work for Weld proxies and extend this to a generic contract later!?
|