Patch Classes.getAllInterfaces to return predictable array #2
Conversation
|
/cc @safris |
| @@ -36,8 +34,6 @@ public int compare(final Class<?> o1, final Class<?> o2) { | |||
|
|
|||
| private static void testGetAllInterfaces(final Object obj, final Class<?> ... expecteds) { | |||
| final Class<?>[] ifaces = Classes.getAllInterfaces(obj.getClass()); | |||
| Arrays.sort(ifaces, comparator); | |||
There was a problem hiding this comment.
Removed the sorting since getAllInterfaces will return a predictable array we can just compare it
There was a problem hiding this comment.
I think the sort is not a bad idea but if should be added in getAllInterfaces(). Otherwise it's going to be a bit hard to predict the order (typically in the case of Quarkus, we might obtain the interfaces from the Jandex index instead of recursing through the interfaces so a fully predicable order would be better).
There was a problem hiding this comment.
That being said I'm a bit unsure why the proxy needs to implement ALL the interfaces. Why would the direct interfaces not be enough?
There was a problem hiding this comment.
This is just an adjustment in the test class, not the implementation itself, we don't need the sort if they are returned in a predictable way. Per my experience Quarkus won't have all classes in the Jandex, and the method there to get all interface implementation doesn't really return all implementations (I think this is a different issue, and in my local, non public, Quarkus extensions I had to build a custom code to get all interface implementations). As per requiring all interfaces, I think that they were being preemptive to build proxies to all of them if case they are all needed, but the actual maintainers of the lib that must know why they did it like this in the first place.
As per using this with Quarkus, in the issue there (in Quarkus Repo) I provided a code that gets all the required interfaces, register them in GraalVM, so this simple change of using a LinkedHashSet instead of HashSet will actually fix Quarkus side
When doing native image we need to create the dynamic proxy configuration, as the current code uses a HashSet it does not guarantee order, thus making us add all possible permutation of the classes being used.
I've changed it to a LinkedHashSet so the order is predictable and we can easily configure GraalVM for the Dynamic Proxy.
Related to opentracing-contrib/java-jdbc#116