Allow Orianna to initialize without the default config present

[PseudoResonance]

I'm trying to load Orianna as a plugin that is loaded into a program separately, but it seems that Google's getResource uses the wrong classloader and therefore cannot find the default configuration. Plus, as it's a plugin, I can't expect environment variables to be set, so I have no way of loading Orianna.

The best idea I can think of is creating a method dedicated to initializing Orianna that can take an optional parameter of a file or inputstream or something, of the config, so that it can be programmatically set.

[robrua]

Hey, sounds like it's crashing during the classloading of the Orianna class because of https://github.com/meraki-analytics/orianna/blob/master/orianna/src/main/java/com/merakianalytics/orianna/Orianna.java#L301?

I'd hope that the final fallback @ https://github.com/meraki-analytics/orianna/blob/733e4df5478271967adb7c754fa0e95d8a05b9e8/orianna/src/main/java/com/merakianalytics/orianna/Orianna.java#L455 would help with that, though it's definitely not an ideal setup...

If it's not failing @ classloading, there are a few utility functions to load a config from disk or progammatically https://github.com/meraki-analytics/orianna/blob/733e4df5478271967adb7c754fa0e95d8a05b9e8/orianna/src/main/java/com/merakianalytics/orianna/Orianna.java#L693-L703

Anywho, it sounds like maybe none of that will solve your problem. Can you send some more details about your setup and what specifically is breaking & how (ideally w/stack trace)? Also if there's an easy way I can reproduce this on my end I'd be happy to take a look at properly supporting this use case.

[PseudoResonance]

I mean, maybe my plugin loader is doing something weird that causes Google's Resources utility to get the wrong classloader, but it fails to find the resource and throws an IllegalArgumentException and the class fails to initialize. The resource is definitely present in the plugin though, as I can verify it's there by running this.getClass().getClassLoader().getResource("com/merakianalytics/orianna/default-orianna-config.json");

Here's the stack trace from the error though.

java.lang.ExceptionInInitializerError
    at com.github.pseudoresonance.resonantbot.leagueoflegends.LeagueOfLegendsCommand.setup(LeagueOfLegendsCommand.java:74)
    at com.github.pseudoresonance.resonantbot.leagueoflegends.LeagueOfLegends.onEnable(LeagueOfLegends.java:45)
    at com.github.pseudoresonance.resonantbot.api.Plugin.setEnabled(Plugin.java:37)
    at com.github.pseudoresonance.resonantbot.PluginFileLoader.enablePlugin(PluginFileLoader.java:76)
    at com.github.pseudoresonance.resonantbot.PluginManager.reload(PluginManager.java:42)
    at com.github.pseudoresonance.resonantbot.ResonantBot.launch(ResonantBot.java:113)
    at com.github.pseudoresonance.resonantbot.ResonantBot.startup(ResonantBot.java:68)
    at com.github.pseudoresonance.resonantbot.ResonantBot.main(ResonantBot.java:58)
Caused by: java.lang.IllegalArgumentException: resource com/merakianalytics/orianna/default-orianna-config.json not found.
    at com.google.common.base.Preconditions.checkArgument(Preconditions.java:148)
    at com.google.common.io.Resources.getResource(Resources.java:229)
    at com.merakianalytics.orianna.Orianna.defaultSettings(Orianna.java:452)
    at com.merakianalytics.orianna.Orianna.<clinit>(Orianna.java:301)
    ... 8 more

If that final fallback of creating a new configuration is enough until the default config is loaded in though, I guess just catching the IllegalArgumentException from the Resources util would probably be enough.

I think the reason why Google's Resources util gets the wrong classloader is because it gets the current thread's classloader with Thread.currentThread().getContextClassLoader(), however the returned classloader is the one of the main program, not of the plugin. If that were to fail, the fallback it uses would likely be the same, as Guava is used by the main program as well, so it would definitely not find the plugin.. I'm not sure if there's a good way around that though.

[robrua]

I just pushed a fix to catch that IllegalArgumentException and continue on to the next fallback. Hopefully you'll be able to load it with that fix, though I'm going to investigate a bit and see if I can find a good solution for the setup you're describing that can still get the default config from the Ori jar... Let me know if it doesn't solve your problem in the meantime.

[PseudoResonance]

Well, I got it to load the default configuration after failing to find the one in the package by running Orianna.loadConfiguration(Resources.asCharSource(this.getClassLoader().getResource("com/merakianalytics/orianna/default-orianna-config.json"), Charset.forName("UTF-8"))); immediately at launch of the plugin, so that it would use the right classloader.

This worked, but then I found that Jackson also uses the same method of getting the classloader as Google in com.fasterxml.jackson.databind.type.TypeFactory... So whenever I ran a request for something, Jackson would try to load in one of the Orianna classes, such as com.merakianalytics.orianna.datapipeline.riotapi.ChampionAPI and even though the class would load perfectly fine, because Jackson explicitly defines which classloader to use, their method failed... You can see it here: https://github.com/FasterXML/jackson-databind/blob/master/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java#L304

I ended up just going with this recommendation and setting the context classloader at the start of execution of any Orianna code, then resetting it to the main classloader in a finally block. I guess the idea of the context classloader is that if the thread is being used by things requiring different classloaders, you're supposed to set the context classloader to whatever is currently relevant. I had no idea that this existed before today, so I'm glad learned something I guess.

[robrua]

Nice, sounds like you got everything working then?

I think my position based on what you've said is that your current solution is the "most correct" way to deal with this. Do you think we should look into a built-in fix for Ori to try to avoid this in the future, or are you satisfied w/the fix you used?

[PseudoResonance]

I think that the fix you pushed earlier of catching the missing resource is probably the best option. It'd be nice if the classloader thing worked better on its own, but I don't think there's a good solution for that on the API side, nor is it something Orianna can really fix, as it has to do with how Jackson works.

Thanks for the help and the great API!