Clarify javadocs. Refactor migration strategies.

Author: RezzedUp

config-values-bukkit/src/main/java/community/leaf/configvalues/bukkit/data/YamlDataFile.java

@@ -162,11 +162,9 @@ public void migrateValues(List<YamlValue<?>> values, ConfigurationSection existi
     {
         for (YamlValue<?> value : values)
         {
-            if (value.migrations().isEmpty()) { continue; }
-            
-            for (Migration policy : value.migrations())
+            for (Migration migration : value.migrations())
             {
-                policy.migrate(existing, this, value);
+                migration.migrate(existing, this, value.key());
             }
         }
     }

config-values-bukkit/src/main/java/community/leaf/configvalues/bukkit/migrations/Migration.java

@@ -7,7 +7,6 @@
  */
 package community.leaf.configvalues.bukkit.migrations;
 
-import community.leaf.configvalues.bukkit.YamlValue;
 import community.leaf.configvalues.bukkit.data.YamlDataSource;
 import org.bukkit.configuration.ConfigurationSection;
 
@@ -35,9 +34,8 @@ static Migration from(String key, MigrationStrategy strategy)
      * Migrates values at the specified key by
      * copying them from one config to another.
      *
-     * <p>This is the simplest migration strategy:
-     * it leaves the existing value untouched
-     * and intact.</p>
+     * <p>This is the simplest migration strategy: it
+     * leaves the existing value untouched and intact.</p>
      *
      * @param key   the old value's key
      *
@@ -53,9 +51,11 @@ static Migration copy(String key)
      * Migrates values at the specified key by
      * moving them from one config to another.
      *
-     * <p>If migration is successful, the old
-     * value is <b>removed</b> from the source
-     * configuration.</p>
+     * <p>Note: the value located at this migration's
+     * key is always <b>removed</b> from the source
+     * configuration regardless of whether migration
+     * is successful (thus ensuring the value is
+     * always "moved" to the destination).</p>
      *
      * @param key   the old value's key
      *
@@ -82,20 +82,21 @@ static Migration move(String key)
     MigrationStrategy strategy();
 
     /**
-     * Migrates the specific value (retrieved with
-     * {@link #key()}) from an existing configuration
-     * into another by submitting it to the destination
-     * YAML data at the provided value's key.
+     * Migrates the specific value from an existing
+     * configuration (retrieved with {@link #key()})
+     * to the destination YAML data at the provided key.
      *
-     * <p>If the retrieved value doesn't exist, then
-     * nothing is migrated.</p>
+     * <p>If the retrieved "existing" value doesn't exist,
+     * then nothing is migrated.</p>
      *
-     * @param existing  existing configuration
-     * @param data      destination yaml data
-     * @param value     destination yaml value
+     * @param existing          existing configuration
+     * @param destination       destination yaml data
+     * @param destinationKey    destination value key
+     *
+     * @see MigrationStrategy#migrate(ConfigurationSection, String, YamlDataSource, String)
      */
-    default void migrate(ConfigurationSection existing, YamlDataSource data, YamlValue<?> value)
+    default void migrate(ConfigurationSection existing, YamlDataSource destination, String destinationKey)
     {
-        strategy().migrate(existing, key(), data, value);
+        strategy().migrate(existing, key(), destination, destinationKey);
     }
 }

config-values-bukkit/src/main/java/community/leaf/configvalues/bukkit/migrations/MigrationStrategy.java

@@ -7,30 +7,39 @@
  */
 package community.leaf.configvalues.bukkit.migrations;
 
-import community.leaf.configvalues.bukkit.YamlValue;
 import community.leaf.configvalues.bukkit.data.YamlDataSource;
 import org.bukkit.configuration.ConfigurationSection;
 
 /**
- * Mechanism that migrates values from
- * one configuration to another.
+ * Migrates values from one configuration to another.
  */
 @FunctionalInterface
 public interface MigrationStrategy
 {
     /**
      * Migrates a value from an existing configuration
-     * into another by retrieving it from the specified
-     * key then submitting it to the destination YAML
-     * data at the provided value's key.
+     * to another by retrieving then submitting it to
+     * the destination YAML.
      *
-     * <p>If the retrieved value doesn't exist, then
-     * nothing is migrated.</p>
+     * <p>Note:</p>
      *
-     * @param existing  existing configuration
-     * @param key       key to retrieve the existing value from
-     * @param data      destination yaml data
-     * @param value     destination yaml value
+     * <ul>
+     *     <li>
+     *         If the destination YAML already has a value at
+     *         the destination key, then migration is skipped
+     *         (but some implementations may still modify the
+     *         existing value, such as removing it).
+     *     </li>
+     *     <li>
+     *         If the retrieved "existing" value doesn't exist,
+     *         then nothing is migrated.
+     *     </li>
+     * </ul>
+     *
+     * @param existing          existing configuration
+     * @param existingKey       key to retrieve the value from
+     * @param destination       destination yaml data
+     * @param destinationKey    destination value key
      */
-    void migrate(ConfigurationSection existing, String key, YamlDataSource data, YamlValue<?> value);
+    void migrate(ConfigurationSection existing, String existingKey, YamlDataSource destination, String destinationKey);
 }

config-values-bukkit/src/main/java/community/leaf/configvalues/bukkit/migrations/Migrations.java

@@ -7,7 +7,6 @@
  */
 package community.leaf.configvalues.bukkit.migrations;
 
-import community.leaf.configvalues.bukkit.YamlValue;
 import community.leaf.configvalues.bukkit.data.YamlDataSource;
 import org.bukkit.configuration.ConfigurationSection;
 import pl.tlinkowski.annotation.basic.NullOr;
@@ -18,19 +17,20 @@ final class Migrations
 {
     private Migrations() { throw new UnsupportedOperationException(); }
 
-    static void copy(ConfigurationSection existing, String key, YamlDataSource data, YamlValue<?> value)
+    static void copy(ConfigurationSection existing, String existingKey, YamlDataSource destination, String destinationKey)
     {
-        @NullOr Object maybe = existing.get(key);
-        if (maybe != null) { data.set(value.key(), maybe); }
+        if (destination.data().isSet(destinationKey)) { return; }
+        @NullOr Object maybe = existing.get(existingKey);
+        if (maybe != null) { destination.set(destinationKey, maybe); }
     }
 
-    static void move(ConfigurationSection existing, String key, YamlDataSource data, YamlValue<?> value)
+    static void move(ConfigurationSection existing, String existingKey, YamlDataSource destination, String destinationKey)
     {
-        @NullOr Object maybe = existing.get(key);
-        if (maybe == null) { return; }
+        copy(existing, existingKey, destination, destinationKey);
 
-        data.set(value.key(), maybe);
-        existing.set(key, null);
+        // Only remove from source config if keys are distinct
+        // (protects the destination value if the config is migrating from itself)
+        if (!existingKey.equals(destinationKey)) { existing.set(existingKey, null); }
     }
 
     static final class Impl implements Migration