Checking if config exists not working as expected

Plugins Plugin Development
1

So I am finally getting around to learning Sponge, and I’ve hit my first bump. Sponge uses a Path object to reference where the config file is and a ConfigurationLoader to do all the actual load operations and whatnot. Ok great, I have that set up like this:

@Inject
@ConfigDir(sharedRoot = true)
private Path configDir;

@Inject
@DefaultConfig(sharedRoot = true)
private ConfigurationLoader<CommentedConfigurationNode> configLoader;

Ok, so I have my config file’s “default template” in the jar (using Maven, the path is set up according to the Sponge Docs, I checked that thread already), and I have my plugin set to check if a config exists on the file system already, and if not to extract the default. Looks like this:

if (!this.configDir.toFile().exists()) {
    log.info("File doesn't exist, extracting");
    URL configFile = this.getClass().getResource("Default.conf");
    configLoader = HoconConfigurationLoader.builder().setURL(configFile).build();
} else {
    log.info("File exists, referencing");
    configLoader = HoconConfigurationLoader.builder().setPath(configDir).build();
}

Ok, great, now what the heck is wrong with this?

[Server thread/INFO] [groupbroadcaster]: File exists, referencing`

That’s fine, right? Except the file doesn’t exist on my file system yet. So apparently, the call to

!this.configDir.toFile().exists()

is telling my plugin that yes, the file does in fact exist on the file system and please do read from it, even though it does not exist and you most certainly should not try to read from it. I guess from all of this, my main question is, did I mess something up, or is there an inherent problem with converting a Path to a File, and checking if it exists?

2

this.configDir is the shared directory for configs (config/) not your config file.
Your config do not exists but the directory config/ does. So .exists() return true;

Instead of this.configDir.toFile() use this.configDir.resolve("myConfig.conf").toFile()

3

First off, a lot of the file boxing can be replaced with the Files class. For example: Files.exists(configDir).
Second, if you @Inject your ConfigurationLoader, no additional setup needs to be done, ever. I am curious as to why you are reassigning the configLoader variable since it is already injected. The only reason additional setup needs to be done is if you make the loader manually, which, again, injection eliminates the need for.
Additionally, you are setting a directory as the location for the loader. If you decide to make a loader manually, it must use the location of the actual file.
And lastly, consider using the Asset API for your default configuration.

4

So, if I’m putting two and two together here…

I should keep the ConfigurationLoader I have, and keep it injected, but never reassign it. When it comes time to check if the config exists or not, I need to use this.configDir.resolve("filename").toFile() or else I’m grabbing a shared directory. If it does exist, it will be ready to load and should work peacefully automagically. If it doesn’t exist, use the Asset API to grab the default from inside my jar. If and only if I am grabbing the default from inside the jar, I should reassign the configLoader to use that file. This file should then be written to the file system. Check one last time if the file exists. If so, load from it. If not, then something went wrong with the file system and that’s another fish to fry.

Am I interpreting this right?

5

I honestly wouldn’t reassign it ever.

Even after copying the defaults over (if they don’t already exist) that config loader should still be good to load the copied file, it’s still pointing at where you copied it to, as long as you have only injected the loader and not actually loaded anything I think you are still fine.

There is also a method that allows you to load a config (with fallback) that allows you to load the defaults if nodes are missing values in the user specified config that you may want to consider using. However this has the downside of people being unable to leave things unset, or remove default configuration nodes if there is a legitimate reason for them to do so.

6

You don’t need to check if the config exists or not. If your configurationloader is injected, it always works. Sponge sets that part up for you. The Asset API can also handle copying the defaults.

7

Ok, I see now what’s going on. Just had time to look over the Asset API. Something like this.

Asset asset = plugin.getAsset("Default.conf").get();

if (asset != null) {
    if (Files.notExists(configDir)) {
        try {
            asset.copyToFile(configDir);
        } catch (IOException e) {
            e.printStackTrace();
            getLogger().error("Could not unpack the default config from the jar! Maybe your Minecraft server doesn't have write permissions?");
            return;
        }
    }
} else {
    getLogger().error("Could not find the default config file in the jar! Did you open the jar and delete it?");
    return;
}

try {
    rootNode = configLoader.load();
} catch (IOException e) {
    e.printStackTrace();
    getLogger().error("An IOException occured while trying to load the config; does your Minecraft server not have read permissions?");
    return;
}

I was confused because I was doing what the Configuration Loaders section of the docs said to do for loading configurations from inside the jar. Doesn’t reference the Asset API.

8 
Asset asset = plugin.getAsset("...").get();
if (asset != null) {
    ...
}

will simply not work. plugin.getAsset() returns an Optional, and if you call get() on the Optional and the asset is not present, you will get an exception. The get method will never return null. I suggest reading more on Optionals here.

Now, the correct way to check if an asset is present would be like so:

Optional<Asset> optionalAsset = plugin.getAsset("...");
if (optionalAsset.isPresent()) {
    Asset asset = optionalAsset.get();
}

Also I should note that there is a PR currently open on the SpongeDocs repo that makes some modifications to the configuration docs. The PR still needs to be updated to use the asset API, however the PR itself hasn’t been updated in a while.

9

In addition to what @12AwsomeMan34 said, again, stop using the config directory for things. What you need is the actual config file, not the directory that holds it.

1 Like
10

Some more tinkering and I’ve got it compiled and running properly. Just had to change that Path to the exact file and I went ahead and changed get() to orElse(null). I’m sorry that I’m not jumping on the Optional<> train but I prefer just doing a cut and dry null check where necessary. For reference, the final product is:

Asset asset = plugin.getAsset("GroupBroadcasterConfig.conf").orElse(null);
Path configPath = new File(configDir + File.separator + "GroupBroadcasterConfig.conf").toPath();

if (asset != null) {
    if (Files.notExists(configPath)) {
        try {
            asset.copyToFile(configPath);
        } catch (IOException e) {
            e.printStackTrace();
            getLogger().error("Could not unpack the default config from the jar! Maybe your Minecraft server doesn't have write permissions?");
            return;
        }
    }
} else {
    getLogger().error("Could not find the default config file in the jar! Did you open the jar and delete it?");
    return;
}

try {
    rootNode = configLoader.load();
} catch (IOException e) {
    e.printStackTrace();
    getLogger().error("An IOException occured while trying to load the config; aborting plugin startup.");
    return;
}
11

Some advices:
Your code should work but you can still simplify it :stuck_out_tongue: :
use
Path configPath = configDir.resolve("GroupBroadcasterConfig.conf");
instead of
Path configPath = new File(configDir + File.separator + "GroupBroadcasterConfig.conf").toPath();

Moreover, when you want to print an exception in the logger
Instead of
e.printStackTrace();
getLogger().error("My message");
You should use
getLogger().error("My message", e);

12

Noted with the resolve(), and I’ll stick to stack traces. Trying to slam a stack trace down a logger just seems… messy, and I’d rather let those print naturally.

13

Then just throw the exception. Listener methods can throw anything.

14

If the OP still needs help with this, this is the code that I use to check if my config exists or not:
Sponge.getAssetManager().getAsset(plugin, "your_config.conf").ifPresent(obj -> { try { if(Files.notExists(defaultConfig, LinkOption.NOFOLLOW_LINKS)) { obj.copyToFile(defaultConfig); } } catch (IOException e) { e.printStackTrace(); } });

Sorry for terrible formatting :stuck_out_tongue:

15

I should mark this solved. I already got it figured out but thanks.

16

If you worked it out, post the solution as it helps others.

17

For reference, here’s what I ended up with:

Asset asset = pluginContainer.getAsset("groupbroadcaster.conf").orElse(null);
Path configPath = configDir.resolve("groupbroadcaster.conf");

if (Files.notExists(configPath)) {
    if (asset != null) {
        try {
            asset.copyToFile(configPath);
        } catch (IOException e) {
            e.printStackTrace();
            log.error("Could not unpack the default config from the jar! Maybe your Minecraft server doesn't have write permissions?");
            return;
        }
    } else {
        log.error("Could not find the default config file in the jar! Did you open the jar and delete it?");
        return;
    }
}

try {
    rootNode = configLoader.load();
} catch (IOException e) {
    e.printStackTrace();
    return;
}
18

You can replace the null check on asset with

plugin.getAsset("groupbroadcaster.conf").ifPresent(asset -> {

});

Because that’s kind of the whole point of using Optionals

19

I purposely avoid the Optionals and null check on my own.

20

Um…Why?

1 Like