Implemented the version migration service and its unit tests

[gamerover98]

Hi, this pull request contains a new feature that supports configuration versioning. The complete description of this feature is fully described at Proposal for New Feature - Version Control Support.

Scenario

Let's imagine that we have an old config like the following:

version: 1
potatoes: 4
tomatoes: 10

Our software requires an update and we have a new configuration but we need to migrate from the old to the new one with a result like to following:

version: 2
shelf:
  potatoes: 4
  tomatoes: 10

Obviously, the migration must keep the old property values.

Do the migration

First of all, this is the SettingsHolder of our updated software:

public final class MyAppVersionConfiguration implements SettingsHolder {

    public static final Property<Integer> VERSION_NUMBER = newProperty("version", 2);
    public static final Property<Integer> SHELF_POPATOES = newProperty("shelf.potatoes", 40);
    public static final Property<Integer> SHELF_TOMATOES = newProperty("shelf.tomatoes", 100);

    private MyAppVersionConfiguration() {
        // nothing to do.
    }
}

Now we can register this SettingsManager:

MigrationService migrationService = getVersionMigrationService();

SettingsManager settingsManager = SettingsManagerBuilder
  .withYamlFile(configFile)
  .configurationData(MyAppVersionConfiguration.class)
  .migrationService(migrationService) // use my migration service
  .create();

private static VersionMigrationService getVersionMigrationService() {
    return new VersionMigrationService(
        2, // current configuration version
        Collections.singletonList(
            new VersionMigrationService.Migration() {

                @Override
                public int fromVersion() {
                    return 1; // the old config version
                }

                @Override
                public int toVersion() {
                    return 2; // in this case, the migration version is the same as the current configuration migration.
                }

                @Override
                public void migrate(@NotNull PropertyReader reader, @NotNull ConfigurationData configurationData) {
                    Property<Integer> oldPotatoesProperty = PropertyInitializer.newProperty("potatoes", 4);
                    Property<Integer> oldTomatoesProperty = PropertyInitializer.newProperty("tomatoes", 10);

                    Property<Integer> newPotatoesProperty = PropertyInitializer.newProperty("shelf.potatoes", 4);
                    Property<Integer> newTomatoesProperty = PropertyInitializer.newProperty("shelf.tomatoes", 10);

                    MigrationUtils.moveProperty(oldPotatoesProperty, newPotatoesProperty, reader, configurationData);
                    MigrationUtils.moveProperty(oldTomatoesProperty, newTomatoesProperty, reader, configurationData);
                }
            }
        ));
}

Edge cases

What happens if...

1. the version property isn't present in the config file: it will be added as new property with the starting value (by default, it is valued at 1).
2. the version value is lower than the starting version: by default, an IllegalStateException is thrown. You can edit the behavior by overriding the handleTooLowerVersion(int version) method.
3. the version value is greater than the current version: by default, an IllegalStateException is thrown. You can edit the behavior by overriding the handleTooHigherVersion(int version) method.

---

Other changes

The PlainMigrationService.moveProperty(oldProperty, newProperty, reader, configurationData) method has been moved to a new class called MigrationUtils.

---

Looking forward to your feedback and collaboration on this enhancement. Thank you

Fix #344

[ljacqu]

This is really nice and allows for migrations to be defined successively without having to "react" to whatever is in the config, as the current migration service forces one to do. Very neat! I will leave some comments a bit later but think this is a great enhancement—thank you!

[gamerover98]

Hi, I would like to ask you if you could make the changes to the test section that you requested me to modify in whichever way you think is best. It would be beneficial to include new test suites to cover more use cases, such as migration from various different versions. Currently, the only available test pertains to the migration from a single version, specifically from versions 1 to 2.

[ljacqu]

Thank you—I'm on it! I've also merged Migration with VersionMigration and re-added fromVersion(). That way, VersionMigration has the entire information about what it migrates, and the VersionMigrationService can create the map without the calling user having to do that. You'll get to have your say once the changes are in of course. In particular, I think we'll need a last round to talk about what happens if the version is out of range. I pushed you to remove the exceptions, but interestingly, not having exceptions means if a wrong value is set as version, we can never really "get out of that". At least not if the version is too small. I'm currently writing tests and will make sure to have such scenarios as well, so once the tests are done we'll have more to refer to, and I hope I'll have more of a full picture :smile:

[ljacqu]

I'm still working on adding Javadoc on VersionMigrationService but wanted to push my other changes in case I was taking too long haha. This took a long time because it took me a while to form an opinion on what the migration service should do in various corner cases. Happy for your input on this, and everything else.

Basically I ended up setting the version to the current version no matter what. Because if the migration service triggers a reset, it means that any old values (properties that no longer exist in the SettingsHolder classes, or properties whose value has changed and is now deemed invalid) would anyway disappear. So I think it's dangerous to run migrations and maybe not end at the current version and to save that version instead of the current version.

What I'm saying is a situation like this:

Migrations:
 1 to 2
 2 to 3
 4 to 5

Current version: 5

YAML file:
  version: 1

The migration service will set that to 5 even though it only ran the migrations 1->2, 2->3. I'll try to reflect that in the Javadoc of the VersionMigrationService, which, as mentioned, I'm rewriting for the third time and trying to phrase sensibly :joy:

Edit: Since this might not fit everyone's needs, I tried to separate the logic into smaller portions that could be overridden. Hence the birth of runApplicableMigrations, which could be called by someone that wants to override performMigrations, OR the other way around: the former method could be overridden if a tweak is needed in how the migrations are picked up, without having to duplicate the rest. I think it worked out OK, but I'm sorry if you're a bit surprised that I turned everything upside down!

[gamerover98]

That's okay, this is your project, and my code was meant to serve as a reference for something else. I've always wanted to incorporate this feature into my projects.

I'm thankful for the opportunity to assist you, and I'll strive to provide similar help for other ideas as well. Have a nice day 😸

[ljacqu]

I very much appreciate your contributions—be it in form of code, comments or opinions. Happy for your thoughts on any issue as well as any review findings you have in any ConfigMe code. It helps make the right decisions for integrators of ConfigMe like you :smile: I mainly function issue-based, so even if it's not really a bug or feature, creating a GitHub issue is a great way to discuss a topic! Otherwise, my Discord username is cyrkel.

I've pushed the documentation for the migration service. Please comment on anything you don't like in this PR. I will do the same later with a clearer head (might be tomorrow). So I think a merge is right on the horizon now :grin:

Thanks again for everything! Really appreciate your work on this project.