GeoIP2-java
GeoIP2-java copied to clipboard
Added `reflect-config.json` required to support graalvm native-image build.
This PR, together with https://github.com/maxmind/MaxMind-DB-Reader-java/pull/166 adds full native-image support in geoip2-java.
Previously application, incorportaing geoip2-java threw exception when attempt was made to resolve some address:
com.maxmind.db.ConstructorNotFoundException: No constructor on class com.maxmind.geoip2.model.CityResponse with the MaxMindDbConstructor annotation was found.
at com.maxmind.db.Decoder.findConstructor(Decoder.java:473)
at com.maxmind.db.Decoder.decodeMapIntoObject(Decoder.java:394)
at com.maxmind.db.Decoder.decodeMap(Decoder.java:341)
at com.maxmind.db.Decoder.decodeByType(Decoder.java:162)
at com.maxmind.db.Decoder.decode(Decoder.java:151)
at com.maxmind.db.Decoder.decode(Decoder.java:76)
at com.maxmind.db.Reader.resolveDataPointer(Reader.java:413)
at com.maxmind.db.Reader.getRecord(Reader.java:185)
at com.maxmind.geoip2.DatabaseReader.get(DatabaseReader.java:280)
at com.maxmind.geoip2.DatabaseReader.getCity(DatabaseReader.java:365)
at com.maxmind.geoip2.DatabaseReader.tryCity(DatabaseReader.java:359)
NOTE: This PR requires new release of maxmind/MaxMind-DB-Reader-java with native-image support.
This config seems very likely to break as we add new classes. I see GraalVM issue on wildcard support, apparently filed by users of this library. It seems unlikely that will be added any time soon, but maybe there is a way to add a test to prevent future breakage.
Although I don't know much about native images, from the discussion on that issue, I assume it is possible to specify the config in the application including the library rather than including it here. Without an automated way to ensure that we do not accidentally break applications as we add new classes, I am inclined to hold off on merging this.
hey @oschwald, you're right, this PR comes from my app that uses geoip2-java and gets compiled into native binary. Currently, the only 3rd library used in the app, that requires separate, custom reflect-config.json is geoip2-java 🤷; I wanted to improve the situation and developer UX and supplied a patch so that everyone can benefit from it.
And yes, sure, library maintainers need to continously maintain native-image references when they add/remove classes.
AWS SDK2 example:
- https://github.com/search?q=repo%3Aaws%2Faws-sdk-java-v2+reflect-config.json&type=code
- https://github.com/search?q=repo%3Aaws%2Faws-sdk-java-v2+native-image.properties&type=code
- https://github.com/search?q=repo%3Aaws%2Faws-sdk-java-v2%20proxy-config.json&type=code
The best way to make sure that last commit didn't break native-image compatibility is by having a native-image test, see aws sdk2 example
Thanks for the information. I think I would want a test to exist before merging this, as without it, we are unlikely to provide the kind of support that users of the library could rely on.
@bfg I just had same exception, do you know are there any other library we can use which works with graalvm without this problem ? in my case I have a spring boot web app, are there any easy work around for this ?
@ozkanpakdil hey, sorry for late reply, but in the end i wrote generic solution; I generate reflection-config.json for various SDKs as part of test-suite run, see
https://gist.github.com/bfg/afc9a512885cd511929096dc0c7067c2