New lib class: KeyValueDatabase

[Cooldude2606]

tl;dr a new class should be implemented in the lib which provides key value pair persistent storage, this can be used by the controller and plugins.

The Problem

Currently there is boilerplate that needs to be copied into plugins if you want to have persistent data storage. The most common case is for key value pairs which implement the subscribable value interface. There are also cases within the controller which are repeated such as loadModPacks loadSaves loadSystems and their save counterparts. This can all be pulled out to a new class to keep the code dry and easier to maintain.

Proposed Solution

A new lib class named KeyValueDatabase which extends (or implements) Map<SubscribableValue.id, SubscribableValue> in addition to the following methods:

• A constructor which accepts a file path to save/load from and an 'on load' function to handle migration of loaded data.
• A load method which populates the mapping from the provided file, potentially this could be a static method that returns and instance of the class, but it should not be part of the constructor to avoid having async file reads within it.
• A save method which saves a json representation of the mapping to a file if the mapping has been modified since last load/save. This is currently tracked by the dirty flags so those would be pulled into this class similar to UserManager.
• A 'touch' method to set the updated at property of a value and the dirty flag of the collection. Alternatively, reimplement set to do this.

Further Work

After this has been completed it may be possible to further reduce code repetition by having the class provide a handleSubscription method and a valueUpdated event to remove the boilerplate found in cases like handleInstanceDetailsSubscription and savesUpdated.

Additionally, this class could incorporate JSON schemas to validate the type of loaded data after migrations have been applied. This would improve the robustness between versions and catch format mismatches between versions. It would also allow for better feedback to the cluster admin as it will highlight which value has caused errors and why.

[Cooldude2606]

There was discussion about the possibility of race conditions between getting a copy of a value and setting the updated value. The simplist solution was to use an "compare and exchange" method which throws an error when there is a conflict. However, this had cumbersome syntax and boiler plate. Ultimately we concluded that a comment on the set method would be sufficient due to the low likelihood of a collision and the ease of mitigation.

Also discussioned was the decision to make values readonly, this will require a copy to be made and mutated and then set as the new value. The goal is to force the use of a set method rather than expecting the dev to remember to call touch after mutating the value.