Improved islands async deletion from database (#605)

Addresses https://github.com/BentoBoxWorld/BentoBox/issues/591

src/main/java/world/bentobox/bentobox/database/AbstractDatabaseHandler.java

@@ -120,8 +120,7 @@ public abstract class AbstractDatabaseHandler<T> {
     /**
      * Attempts to delete the object with the uniqueId
      * @param uniqueId - uniqueId of object
-     * @return true if successful, false if there is no such uniqueId
      * @since 1.1
      */
-    public abstract boolean deleteID(String uniqueId);
+    public abstract void deleteID(String uniqueId);
 }

src/main/java/world/bentobox/bentobox/database/Database.java

@@ -79,9 +79,9 @@ public class Database<T> {
     }
 
     /**
-     * Save config object
+     * Save config object. Saving may be done async.
      * @param instance to save
-     * @return true if successfully saved
+     * @return true if no immediate errors. If async, errors may occur later.
      */
     public boolean saveObject(T instance) {
         try {
@@ -106,11 +106,10 @@ public class Database<T> {
     /**
      * Attempts to delete the object with the uniqueId
      * @param uniqueId - uniqueId of object
-     * @return true if successful, false if there is no such uniqueId
      * @since 1.1
      */
-    public boolean deleteID(String uniqueId) {
-        return handler.deleteID(uniqueId);
+    public void deleteID(String uniqueId) {
+        handler.deleteID(uniqueId);
     }
 
     /**

src/main/java/world/bentobox/bentobox/database/json/JSONDatabaseHandler.java

@@ -127,7 +127,7 @@ public class JSONDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
     }
 
     @Override
-    public boolean deleteID(String uniqueId) {
+    public void deleteID(String uniqueId) {
         // The filename of the JSON file is the value of uniqueId field plus .json. Sometimes the .json is already appended.
         if (!uniqueId.endsWith(JSON)) {
             uniqueId = uniqueId + JSON;
@@ -140,12 +140,10 @@ public class JSONDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
             File file = new File(tableFolder, uniqueId);
             try {
                 Files.delete(file.toPath());
-                return true;
             } catch (IOException e) {
                 plugin.logError("Could not delete json database object! " + file.getName() + " - " + e.getMessage());
             }
         }
-        return false;
     }
 
     @Override

src/main/java/world/bentobox/bentobox/database/mariadb/MariaDBDatabaseHandler.java

@@ -158,7 +158,7 @@ public class MariaDBDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
     }
 
     @Override
-    public boolean deleteID(String uniqueId) {
+    public void deleteID(String uniqueId) {
         String sb = "DELETE FROM `" +
                 dataObject.getCanonicalName() +
                 "` WHERE uniqueId = ?";
@@ -166,10 +166,8 @@ public class MariaDBDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
             // UniqueId needs to be placed in quotes
             preparedStatement.setString(1, "\"" + uniqueId + "\"");
             preparedStatement.execute();
-            return preparedStatement.getUpdateCount() > 0;
         } catch (Exception e) {
             plugin.logError("Could not delete object " + dataObject.getCanonicalName() + " " + uniqueId + " " + e.getMessage());
-            return false;
         }
     }
 

src/main/java/world/bentobox/bentobox/database/mongodb/MongoDBDatabaseHandler.java

@@ -117,12 +117,11 @@ public class MongoDBDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
     }
 
     @Override
-    public boolean deleteID(String uniqueId) {
+    public void deleteID(String uniqueId) {
         try {
-            return collection.findOneAndDelete(new Document(MONGO_ID, uniqueId)) != null;
+            collection.findOneAndDelete(new Document(MONGO_ID, uniqueId));
         } catch (Exception e) {
             plugin.logError("Could not delete object " + dataObject.getCanonicalName() + " " + uniqueId + " " + e.getMessage());
-            return false;
         }
     }
 

src/main/java/world/bentobox/bentobox/database/mysql/MySQLDatabaseHandler.java

@@ -42,12 +42,12 @@ public class MySQLDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
     private Connection connection;
 
     /**
-     * FIFO queue for saves. Note that the assumption here is that most database objects will be held
+     * FIFO queue for saves or deletions. Note that the assumption here is that most database objects will be held
      * in memory because loading is not handled with this queue. That means that it is theoretically
      * possible to load something before it has been saved. So, in general, load your objects and then
      * save them async only when you do not need the data again immediately.
      */
-    private Queue<Runnable> saveQueue;
+    private Queue<Runnable> processQueue;
 
     /**
      * Async save task that runs repeatedly
@@ -72,13 +72,13 @@ public class MySQLDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
         }
         // Check if the table exists in the database and if not, create it
         createSchema();
-        saveQueue = new ConcurrentLinkedQueue<>();
+        processQueue = new ConcurrentLinkedQueue<>();
         if (plugin.isEnabled()) {
             asyncSaveTask = Bukkit.getScheduler().runTaskAsynchronously(plugin, () -> {
                 // Loop continuously
-                while (plugin.isEnabled() || !saveQueue.isEmpty()) {
-                    while (!saveQueue.isEmpty()) {
-                        saveQueue.poll().run();
+                while (plugin.isEnabled() || !processQueue.isEmpty()) {
+                    while (!processQueue.isEmpty()) {
+                        processQueue.poll().run();
                     }
                     // Clear the queue and then sleep
                     try {
@@ -189,7 +189,7 @@ public class MySQLDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
         String toStore = gson.toJson(instance);
         if (plugin.isEnabled()) {
             // Async
-            saveQueue.add(() -> store(instance, toStore, sb));
+            processQueue.add(() -> store(instance, toStore, sb));
         } else {
             // Sync
             store(instance, toStore, sb);
@@ -206,8 +206,19 @@ public class MySQLDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
         }
     }
 
+    /* (non-Javadoc)
+     * @see world.bentobox.bentobox.database.AbstractDatabaseHandler#deleteID(java.lang.String)
+     */
     @Override
-    public boolean deleteID(String uniqueId) {
+    public void deleteID(String uniqueId) {
+        if (plugin.isEnabled()) {
+            processQueue.add(() -> delete(uniqueId));
+        } else {
+            delete(uniqueId);
+        }
+    }
+
+    private void delete(String uniqueId) {
         String sb = "DELETE FROM `" +
                 dataObject.getCanonicalName() +
                 "` WHERE uniqueId = ?";
@@ -215,10 +226,8 @@ public class MySQLDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
             // UniqueId needs to be placed in quotes
             preparedStatement.setString(1, "\"" + uniqueId + "\"");
             preparedStatement.execute();
-            return preparedStatement.getUpdateCount() > 0;
         } catch (Exception e) {
             plugin.logError("Could not delete object " + dataObject.getCanonicalName() + " " + uniqueId + " " + e.getMessage());
-            return false;
         }
     }
 

src/main/java/world/bentobox/bentobox/database/objects/Island.java

@@ -44,11 +44,15 @@ import world.bentobox.bentobox.util.Util;
  * Managed by IslandsManager
  * Responsible for team information as well.
  *
- * @author Tastybento
+ * @author tastybento
  * @author Poslovitch
  */
 public class Island implements DataObject {
 
+    // True if this island is deleted and pending deletion from the database
+    @Expose
+    private boolean deleted = false;
+
     @Expose
     private String uniqueId = UUID.randomUUID().toString();
 
@@ -850,4 +854,18 @@ public class Island implements DataObject {
     public void setDoNotLoad(boolean doNotLoad) {
         this.doNotLoad = doNotLoad;
     }
+
+    /**
+     * @return the deleted
+     */
+    public boolean isDeleted() {
+        return deleted;
+    }
+
+    /**
+     * @param deleted the deleted to set
+     */
+    public void setDeleted(boolean deleted) {
+        this.deleted = deleted;
+    }
 }

src/main/java/world/bentobox/bentobox/database/yaml/YamlDatabaseHandler.java

@@ -65,7 +65,7 @@ public class YamlDatabaseHandler<T> extends AbstractDatabaseHandler<T> {
      * possible to load something before it has been saved. So, in general, load your objects and then
      * save them async only when you do not need the data again immediately.
      */
-    private Queue<Runnable> saveQueue;
+    private Queue<Runnable> processQueue;
 
     /**
      * Async save task that runs repeatedly
@@ -87,13 +87,13 @@ public class YamlDatabaseHandler<T> extends AbstractDatabaseHandler<T> {
      */
     YamlDatabaseHandler(BentoBox plugin, Class<T> type, DatabaseConnector databaseConnector) {
         super(plugin, type, databaseConnector);
-        saveQueue = new ConcurrentLinkedQueue<>();
+        processQueue = new ConcurrentLinkedQueue<>();
         if (plugin.isEnabled()) {
             asyncSaveTask = Bukkit.getScheduler().runTaskAsynchronously(plugin, () -> {
                 // Loop continuously
-                while (plugin.isEnabled() || !saveQueue.isEmpty()) {
-                    while (!saveQueue.isEmpty()) {
-                        saveQueue.poll().run();
+                while (plugin.isEnabled() || !processQueue.isEmpty()) {
+                    while (!processQueue.isEmpty()) {
+                        processQueue.poll().run();
                     }
                     // Clear the queue and then sleep
                     try {
@@ -439,7 +439,7 @@ public class YamlDatabaseHandler<T> extends AbstractDatabaseHandler<T> {
     private void save(String name, String data, String path, Map<String, String> yamlComments) {
         if (plugin.isEnabled()) {
             // Async
-            saveQueue.add(() -> ((YamlDatabaseConnector)databaseConnector).saveYamlFile(data, path, name, yamlComments));
+            processQueue.add(() -> ((YamlDatabaseConnector)databaseConnector).saveYamlFile(data, path, name, yamlComments));
         } else {
             // Sync for shutdown
             ((YamlDatabaseConnector)databaseConnector).saveYamlFile(data, path, name, yamlComments);
@@ -640,7 +640,15 @@ public class YamlDatabaseHandler<T> extends AbstractDatabaseHandler<T> {
     }
 
     @Override
-    public boolean deleteID(String uniqueId) {
+    public void deleteID(String uniqueId) {
+        if (plugin.isEnabled()) {
+            processQueue.add(() -> delete(uniqueId));
+        } else {
+            delete(uniqueId);
+        }
+    }
+
+    private void delete(String uniqueId) {
         // The filename of the YAML file is the value of uniqueId field plus .yml. Sometimes the .yml is already appended.
         if (!uniqueId.endsWith(YML)) {
             uniqueId = uniqueId + YML;
@@ -653,12 +661,10 @@ public class YamlDatabaseHandler<T> extends AbstractDatabaseHandler<T> {
             File file = new File(tableFolder, uniqueId);
             try {
                 Files.delete(file.toPath());
-                return true;
             } catch (IOException e) {
                 plugin.logError("Could not delete yml database object! " + file.getName() + " - " + e.getMessage());
             }
         }
-        return false;
     }
 
     /* (non-Javadoc)
@@ -668,7 +674,7 @@ public class YamlDatabaseHandler<T> extends AbstractDatabaseHandler<T> {
     public void deleteObject(T instance) throws IllegalAccessException, InvocationTargetException, IntrospectionException {
         // Null check
         if (instance == null) {
-            plugin.logError("YAM database request to delete a null. ");
+            plugin.logError("YAML database request to delete a null. ");
             return;
         }
         if (!(instance instanceof DataObject)) {

src/main/java/world/bentobox/bentobox/managers/IslandsManager.java

@@ -34,6 +34,7 @@ import world.bentobox.bentobox.api.events.IslandBaseEvent;
 import world.bentobox.bentobox.api.events.island.IslandEvent;
 import world.bentobox.bentobox.api.events.island.IslandEvent.Reason;
 import world.bentobox.bentobox.api.localization.TextVariables;
+import world.bentobox.bentobox.api.logs.LogEntry;
 import world.bentobox.bentobox.api.user.User;
 import world.bentobox.bentobox.database.Database;
 import world.bentobox.bentobox.database.objects.Island;
@@ -72,8 +73,12 @@ public class IslandsManager {
     // Island Cache
     @NonNull
     private IslandCache islandCache;
+    // Quarantined islands
     @NonNull
     private Map<UUID, List<Island>> quarantineCache;
+    // Deleted islands
+    @NonNull
+    private List<String> deletedIslands;
 
     /**
      * Islands Manager
@@ -87,6 +92,9 @@ public class IslandsManager {
         quarantineCache = new HashMap<>();
         spawn = new HashMap<>();
         last = new HashMap<>();
+        // This list should always be empty unless database deletion failed
+        // In that case a purge utility may be required in the future
+        deletedIslands = new ArrayList<>();
     }
 
     /**
@@ -266,7 +274,13 @@ public class IslandsManager {
         if (removeBlocks) {
             // Remove island from the cache
             islandCache.deleteIslandFromCache(island);
-            // Remove the island from the database
+            // Log the deletion (it shouldn't matter but may be useful)
+            island.log(new LogEntry.Builder("DELETED").build());
+            // Set the delete flag which will prevent it from being loaded even if database deletion fails
+            island.setDeleted(true);
+            // Save the island
+            handler.saveObject(island);
+            // Delete the island
             handler.deleteObject(island);
             // Remove players from island
             removePlayersFromIsland(island);
@@ -703,9 +717,12 @@ public class IslandsManager {
         islandCache.clear();
         quarantineCache.clear();
         List<Island> toQuarantine = new ArrayList<>();
-        // Only load non-quarantined island
+        // Attempt to load islands
         handler.loadObjects().stream().forEach(island -> {
-            if (island.isDoNotLoad() && island.getWorld() != null && island.getCenter() != null) {
+            if (island.isDeleted()) {
+                // These will be deleted later
+                deletedIslands.add(island.getUniqueId());
+            } else if (island.isDoNotLoad() && island.getWorld() != null && island.getCenter() != null) {
                 // Add to quarantine cache
                 quarantineCache.computeIfAbsent(island.getOwner(), k -> new ArrayList<>()).add(island);
             } else {
@@ -809,8 +826,7 @@ public class IslandsManager {
     }
 
     /**
-     * This removes players from an island overworld and nether - used when reseting or deleting an island
-     * Mobs are killed when the chunks are refreshed.
+     * This teleports players away from an island - used when reseting or deleting an island
      * @param island to remove players from
      */
     public void removePlayersFromIsland(Island island) {