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

Dependency reflections 0.9.12 doesn't add scanners on empty urls #60

Closed
porunov opened this issue Mar 6, 2020 · 1 comment · Fixed by #63
Closed

Dependency reflections 0.9.12 doesn't add scanners on empty urls #60

porunov opened this issue Mar 6, 2020 · 1 comment · Fixed by #63

Comments

@porunov
Copy link
Contributor

porunov commented Mar 6, 2020

In reflections version 0.9.12 there is a bug which may indirectly be a problem when Ferma is used with JanusGraph.
Here in JanusGraph we retrieve PreInitializeConfigOptions even if there is no scanUrls.
https://github.com/JanusGraph/janusgraph/blob/677a57aa985a7ea47af70d0eaba30171d765c79d/janusgraph-core/src/main/java/org/janusgraph/core/util/ReflectiveConfigOptionLoader.java#L248

This works fine till reflections version 0.9.12 which introduces the next bug: ronmamo/reflections#273

The thing is that the method getTypesAnnotatedWith should return empty collection but instead it breaks existing contract and returns a wrong exception.

I think, it would be good to downgrade reflections to 0.9.11 till the bug isn't fixed. After that, it would be better to avoid 0.9.12 version and upgrade directly to the version where it is fixed (i.e. 0.9.13 or later).

@freemo
Copy link
Member

freemo commented Mar 6, 2020

I can approve of this. Would it be possible to write a unit test that demonstrates this bug to assist us in upgrading it later on?

porunov added a commit to porunov/Ferma that referenced this issue Mar 6, 2020
porunov added a commit to porunov/Ferma that referenced this issue Mar 7, 2020
@freemo freemo closed this as completed in ce6b55d Mar 8, 2020
freemo added a commit that referenced this issue Mar 8, 2020
Upgrade: Remove direct and indirect Guava dependency, update reflections (fixes #60)

See merge request Ferma/Ferma!3
@freemo freemo closed this as completed in #63 Mar 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants