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

Potential Type Mismatch with Long Values in ConfigSpec #186

Open
andrew0030 opened this issue Nov 5, 2024 · 2 comments
Open

Potential Type Mismatch with Long Values in ConfigSpec #186

andrew0030 opened this issue Nov 5, 2024 · 2 comments
Assignees

Comments

@andrew0030
Copy link

I'm not 100% sure if this is a "bug", however I think its something worth pointing out.

Short Description:

I encountered an issue with ConfigSpec when working with Long values. If a Long value within a Long range is defined in a ConfigSpec, and the value is small (e.g., 500), ConfigSpec#correct(config) seems to detect an error in the config. I suspect this occurs because the value is stored as an Integer internally, resulting in a type mismatch when checked against the Long range.

Detailed Description:

This issue surfaced while implementing an auto-reloading config that includes an onAutoReload listener. This listener verifies the config’s validity using the ConfigSpec. If the config is not correct, the listener attempts to correct and save it. However, after each correction, the listener re-triggers due to the save, leading to an expected re-check. The issue is that ConfigSpec consistently finds the Long value invalid, attempts to "correct" it again to the same Long value, and saves it again. This cycle repeats indefinitely.

Below is a minimal example of the setup:

// Basic FileConfig setup with auto-reload and listener; the format used is TOML.
FileConfig config = FileConfig.builder(configFilePath).autoreload().onAutoReload(this.autoReloadListener()).build();
config.load(); // Assuming the file is initially empty, values are null until corrected.
// ConfigSpec with a defined Long entry.
ConfigSpec configSpec = new ConfigSpec();
Long defaultValue = this.getValue(); // Assume this returns a Long with the value 500.
Long minValue = Long.MIN_VALUE;
Long maxValue = Long.MAX_VALUE;
configSpec.defineInRange("someLongValue", defaultValue, minValue, maxValue);
// The auto-reload listener for config correction.
private Runnable autoReloadListener() {
    return this::correctIfNeeded;
}

private void correctIfNeeded() {
    ConfigSpec configSpec = this.getConfigSpec();
    boolean isConfigCorrect = configSpec.isCorrect(this.config);
    
    // Handle config correction if needed.
    if (!isConfigCorrect) {
        ConfigSpec.CorrectionListener listener = (action, path, incorrectValue, correctedValue) -> {
            String pathString = String.join(".", path);
            System.out.println("Correction: " + pathString + " Invalid: " + incorrectValue + " Corrected: " + correctedValue);
        };
        int correctionCount = configSpec.correct(this.config, listener);
        System.out.println("Adjusted " + correctionCount + " values");

        // Save the corrected config to the file to apply the corrections permanently.
        this.config.save();
    }
}

The expected behavior is:

  1. ConfigSpec detects a null value in the config.
  2. ConfigSpec corrects it to 500 (the default Long specified).
  3. The corrected config is saved, triggering the listener.
  4. On re-checking, the config should now be valid and not require further correction.

Instead, what seems to happen is that ConfigSpec interprets the small Long value (500) as an Integer, then tries to re-correct it on each check. This results in an infinite loop of corrections.

Workaround:

A workaround that I found effective is to define a custom predicate, allowing ConfigSpec to accept either Integer or Long values, and manually check if the value is within the Long range:

Long defaultValue = this.getValue(); // Assume this returns a Long with the value 500.
Long minValue = Long.MIN_VALUE;
Long maxValue = Long.MAX_VALUE;
configSpec.define("someLongValue", defaultValue, o -> {
    if (o instanceof Integer || o instanceof Long) {
        long longValue = ((Number) o).longValue();
        return longValue >= minVal && longValue <= maxVal;
    }
    return false;
});

Conclusion:

This workaround has resolved the issue, but it would be ideal if ConfigSpec could natively handle Long values correctly, regardless of the value size. This enhancement would remove the need for custom predicates when specifying ranges that span the entire Long type.

@TheElectronWill
Copy link
Owner

Thank you for your detailed report. The situation is indeed far from ideal.
I won't have the time to investigate it soon, but I keep it in mind :)

@TheElectronWill TheElectronWill self-assigned this Nov 12, 2024
@andrew0030
Copy link
Author

That's good to hear.
I also I want to note, this may also affect other types like Byte or Short

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants