Jandex - Follow-up on Marko's work

[gsmet]

Hi @marko-bekhta and happy new year!

So here is the current state of the Jandex implementation rebased on master. Sorry for it taking so long, I got sick during the holidays and wasn't sufficiently well to make progress on this.

I squashed all your preliminary work in one unique commit as it turned out to be a real pain to rebase given all the changes made by Gunnar in his container work. So I thought fixing the conflicts only once next time would be great ;).

I think it's in a not too bad shape. What is a bit of a shame is that we can use the index only to a certain extent and then we need to go back to reflection. I think we won't be able to do otherwise except if we start a major refactoring to avoid depending too much on reflection in the core HV. Not sure it's gonna happen soon so let's keep it that way for now.

So, the next question is what to do next?

I think it might be a good idea to build an index for each HV module containing annotations we might look for and also an index for validation-api. This could be done with the Jandex Maven plugin (https://github.com/wildfly/jandex-maven-plugin). I think it would be a good idea to create separate indexes for main classes and test classes.

I think you might then be able to assemble a view of all these indexes with CompositeIndex, asking the ClassLoader for all META-INF/jandex.idx resources.

As for the test failure you have, I really got no idea. It works perfectly for me in Eclipse and Maven with the following configuration:

[gsmet:~/git/hibernate-validator] pr/585(+77/-8)+ ± java -version
java version "1.8.0_102"
Java(TM) SE Runtime Environment (build 1.8.0_102-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.102-b14, mixed mode)
[gsmet:~/git/hibernate-validator] pr/585(+77/-8)+ ± mvn -v
Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 2015-11-10T17:41:47+01:00)
Maven home: /opt/apache-maven
Java version: 1.8.0_102, vendor: Oracle Corporation
Java home: /opt/jdk1.8.0_102/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "4.8.15-200.fc24.x86_64", arch: "amd64", family: "unix"

[marko-bekhta]

Hi @gsmet !

Sorry to hear that, hope you feel better now! As for the tests - I've run it with maven -U option and they passed now... so maybe it was some problem with the local libs ? I don't know ... but it works now! so this issue is resolved (at least for now :) ). I'll look into that maven plugin and let you know when I have something ...

[marko-bekhta]

Also one more thing - I was thinking about this metadata provider - and it is very similar to the annotation based one with similar logic but different types. But as you were saying about the index - that we can only use it up to certain part and then move to reflection, the same apply here - that without some major changes it seems not possible to group the same logic of both providers (annotation and index based ones) in one place ...

[marko-bekhta]

And then there's also this thought that I had related to HV in general, not related to Jandex, about unwrapping validated values with UnwrapValidatedValue annotation. I was thinking that with type argument annotations this UnwrapValidatedValue seems to become redundant. And having less things to support, and check for, might make the code run a bit faster ? You probably already thought about this and have some decision about it, otherwise it might be something to think about ...

I don't know if this would be of any value to you but I was feeling like share this thought with you... :)

[gsmet]

@marko-bekhta UnwrapValidatedValue will be removed in HV 6. It is being replaced by another annotation called ConstraintsAppyTo which has a slightly different behavior. Gunnar added it to the spec in the appendix related to value extraction but it has not been implement yet.

[gsmet]

Also one more thing - I was thinking about this metadata provider - and it is very similar to the annotation based one with similar logic but different types. But as you were saying about the index - that we can only use it up to certain part and then move to reflection, the same apply here - that without some major changes it seems not possible to group the same logic of both providers (annotation and index based ones) in one place ...

Yes. My current opinion is that we should try to get Jandex to work and see how it behaves. If we see it has some value, we will think about how we have to change HV to better fit this addition. Not relying to much on the reflection API in our internals is one of it I think.