Use @NotNull in methods rather than @Nullable

Separate safe external methods from internal only methods
Remove unnecessary methods

src/main/java/com/gmail/nossr50/mcMMO.java

@@ -345,7 +345,6 @@ public class mcMMO extends JavaPlugin {
 
             formulaManager.saveFormula();
             holidayManager.saveAnniversaryFiles();
-            placeStore.cleanUp();       // Cleanup empty metadata stores
             placeStore.closeAll();
         }
 

src/main/java/com/gmail/nossr50/util/blockmeta/ChunkManager.java

@@ -1,127 +1,10 @@
 package com.gmail.nossr50.util.blockmeta;
 
 import org.bukkit.World;
-import org.bukkit.block.Block;
-import org.bukkit.block.BlockState;
-import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.NotNull;
 
-public interface ChunkManager {
+public interface ChunkManager extends UserBlockTracker {
     void closeAll();
-
-    /**
-     * Saves a given Chunk's Chunklet data
-     *
-     * @param cx Chunk X coordinate that is to be saved
-     * @param cz Chunk Z coordinate that is to be saved
-     * @param world World that the Chunk is in
-     */
-    void saveChunk(int cx, int cz, @Nullable World world);
-
-    /**
-     * Informs the ChunkletManager a chunk is unloaded
-     *
-     * @param cx Chunk X coordinate that is unloaded
-     * @param cz Chunk Z coordinate that is unloaded
-     * @param world World that the chunk was unloaded in
-     */
-    void chunkUnloaded(int cx, int cz, @Nullable World world);
-
-    /**
-     * Save all ChunkletStores related to the given world
-     *
-     * @param world World to save
-     */
-    void saveWorld(World world);
-
-    /**
-     * Unload all ChunkletStores from memory related to the given world after saving them
-     *
-     * @param world World to unload
-     */
-    void unloadWorld(World world);
-
-    /**
-     * Save all ChunkletStores
-     */
-    void saveAll();
-
-    /**
-     * Check to see if a given location is set to true
-     *
-     * @param x X coordinate to check
-     * @param y Y coordinate to check
-     * @param z Z coordinate to check
-     * @param world World to check in
-     * @return true if the given location is set to true, false if otherwise
-     */
-    boolean isTrue(int x, int y, int z, @Nullable World world);
-
-    /**
-     * Check to see if a given block location is set to true
-     *
-     * @param block Block location to check
-     * @return true if the given block location is set to true, false if otherwise
-     */
-    boolean isTrue(@Nullable Block block);
-
-    /**
-     * Check to see if a given BlockState location is set to true
-     *
-     * @param blockState BlockState to check
-     * @return true if the given BlockState location is set to true, false if otherwise
-     */
-    boolean isTrue(@Nullable BlockState blockState);
-
-    /**
-     * Set a given location to true, should create stores as necessary if the location does not exist
-     *
-     * @param x X coordinate to set
-     * @param y Y coordinate to set
-     * @param z Z coordinate to set
-     * @param world World to set in
-     */
-    void setTrue(int x, int y, int z, @Nullable World world);
-
-    /**
-     * Set a given block location to true, should create stores as necessary if the location does not exist
-     *
-     * @param block Block location to set
-     */
-    void setTrue(@Nullable Block block);
-
-    /**
-     * Set a given BlockState location to true, should create stores as necessary if the location does not exist
-     *
-     * @param blockState BlockState location to set
-     */
-    void setTrue(@Nullable BlockState blockState);
-
-    /**
-     * Set a given location to false, should not create stores if one does not exist for the given location
-     *
-     * @param x X coordinate to set
-     * @param y Y coordinate to set
-     * @param z Z coordinate to set
-     * @param world World to set in
-     */
-    void setFalse(int x, int y, int z, @Nullable World world);
-
-    /**
-     * Set a given block location to false, should not create stores if one does not exist for the given location
-     *
-     * @param block Block location to set
-     */
-    void setFalse(@Nullable Block block);
-
-    /**
-     * Set a given BlockState location to false, should not create stores if one does not exist for the given location
-     *
-     * @param blockState BlockState location to set
-     */
-    void setFalse(@Nullable BlockState blockState);
-
-    /**
-     * Delete any ChunkletStores that are empty
-     */
-    void cleanUp();
+    void chunkUnloaded(int cx, int cz, @NotNull World world);
+    void unloadWorld(@NotNull World world);
 }

src/main/java/com/gmail/nossr50/util/blockmeta/ChunkStore.java

@@ -1,6 +1,5 @@
 package com.gmail.nossr50.util.blockmeta;
 
-import org.bukkit.World;
 import org.jetbrains.annotations.NotNull;
 
 import java.util.UUID;

src/main/java/com/gmail/nossr50/util/blockmeta/HashChunkManager.java

@@ -1,6 +1,5 @@
 package com.gmail.nossr50.util.blockmeta;
 
-import com.gmail.nossr50.mcMMO;
 import org.bukkit.Bukkit;
 import org.bukkit.World;
 import org.bukkit.block.Block;
@@ -8,13 +7,16 @@ import org.bukkit.block.BlockState;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.io.*;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.File;
+import java.io.IOException;
 import java.util.*;
 
 public class HashChunkManager implements ChunkManager {
-    private final @NotNull HashMap<CoordinateKey, McMMOSimpleRegionFile> regionMap = new HashMap<>(); // Tracks active regions
-    private final @NotNull HashMap<CoordinateKey, HashSet<CoordinateKey>> chunkUsageMap = new HashMap<>(); // Tracks active chunks by region
-    private final @NotNull HashMap<CoordinateKey, ChunkStore> chunkMap = new HashMap<>(); // Tracks active chunks
+    private final HashMap<CoordinateKey, McMMOSimpleRegionFile> regionMap = new HashMap<>(); // Tracks active regions
+    private final HashMap<CoordinateKey, HashSet<CoordinateKey>> chunkUsageMap = new HashMap<>(); // Tracks active chunks by region
+    private final HashMap<CoordinateKey, ChunkStore> chunkMap = new HashMap<>(); // Tracks active chunks
 
     @Override
     public synchronized void closeAll() {
@@ -23,7 +25,10 @@ public class HashChunkManager implements ChunkManager {
         {
             if (!chunkStore.isDirty())
                 continue;
-            writeChunkStore(Bukkit.getWorld(chunkStore.getWorldId()), chunkStore);
+            World world = Bukkit.getWorld(chunkStore.getWorldId());
+            if (world == null)
+                continue; // Oh well
+            writeChunkStore(world, chunkStore);
         }
         // Clear in memory chunks
         chunkMap.clear();
@@ -104,56 +109,12 @@ public class HashChunkManager implements ChunkManager {
     }
 
     @Override
-    public synchronized void saveChunk(int cx, int cz, @Nullable World world) {
-        if (world == null)
-            return;
-
-        CoordinateKey chunkKey = toChunkKey(world.getUID(), cx, cz);
-
-        ChunkStore out = chunkMap.get(chunkKey);
-
-        if (out == null)
-            return;
-
-        if (!out.isDirty())
-            return;
-
-        writeChunkStore(world, out);
-    }
-
-    @Override
-    public synchronized void chunkUnloaded(int cx, int cz, @Nullable World world) {
-        if (world == null)
-            return;
-
+    public synchronized void chunkUnloaded(int cx, int cz, @NotNull World world) {
         unloadChunk(cx, cz, world);
     }
 
     @Override
-    public synchronized void saveWorld(@Nullable World world) {
-        if (world == null)
-            return;
-
-        UUID wID = world.getUID();
-
-        // Save all teh chunks
-        for (ChunkStore chunkStore : chunkMap.values()) {
-            if (!chunkStore.isDirty())
-                continue;
-            if (!wID.equals(chunkStore.getWorldId()))
-                continue;
-            try {
-                writeChunkStore(world, chunkStore);
-            }
-            catch (Exception ignore) { }
-        }
-    }
-
-    @Override
-    public synchronized void unloadWorld(@Nullable World world) {
-        if (world == null)
-            return;
-
+    public synchronized void unloadWorld(@NotNull World world) {
         UUID wID = world.getUID();
 
         // Save and remove all the chunks
@@ -179,18 +140,7 @@ public class HashChunkManager implements ChunkManager {
         }
     }
 
-    @Override
-    public synchronized void saveAll() {
-        for (World world : mcMMO.p.getServer().getWorlds()) {
-            saveWorld(world);
-        }
-    }
-
-    @Override
-    public synchronized boolean isTrue(int x, int y, int z, @Nullable World world) {
-        if (world == null)
-            return false;
-
+    private synchronized boolean isTrue(int x, int y, int z, @NotNull World world) {
         CoordinateKey chunkKey = blockCoordinateToChunkKey(world.getUID(), x, y, z);
 
         // Get chunk, load from file if necessary
@@ -216,67 +166,36 @@ public class HashChunkManager implements ChunkManager {
     }
 
     @Override
-    public synchronized boolean isTrue(@Nullable Block block) {
-        if (block == null)
-            return false;
-
+    public synchronized boolean isTrue(@NotNull Block block) {
         return isTrue(block.getX(), block.getY(), block.getZ(), block.getWorld());
     }
 
     @Override
-    public synchronized boolean isTrue(@Nullable BlockState blockState) {
-        if (blockState == null)
-            return false;
-
+    public synchronized boolean isTrue(@NotNull BlockState blockState) {
         return isTrue(blockState.getX(), blockState.getY(), blockState.getZ(), blockState.getWorld());
     }
 
     @Override
-    public synchronized void setTrue(int x, int y, int z, @Nullable World world) {
-        set(x, y, z, world, true);
-    }
-
-    @Override
-    public synchronized void setTrue(@Nullable Block block) {
-        if (block == null)
-            return;
-
-        setTrue(block.getX(), block.getY(), block.getZ(), block.getWorld());
+    public synchronized void setTrue(@NotNull Block block) {
+        set(block.getX(), block.getY(), block.getZ(), block.getWorld(), true);
     }
 
     @Override
-    public synchronized void setTrue(@Nullable BlockState blockState) {
-        if (blockState == null)
-            return;
-
-        setTrue(blockState.getX(), blockState.getY(), blockState.getZ(), blockState.getWorld());
+    public synchronized void setTrue(@NotNull BlockState blockState) {
+        set(blockState.getX(), blockState.getY(), blockState.getZ(), blockState.getWorld(), true);
     }
 
     @Override
-    public synchronized void setFalse(int x, int y, int z, @Nullable World world) {
-        set(x, y, z, world, false);
+    public synchronized void setFalse(@NotNull Block block) {
+        set(block.getX(), block.getY(), block.getZ(), block.getWorld(), false);
     }
 
     @Override
-    public synchronized void setFalse(@Nullable Block block) {
-        if (block == null)
-            return;
-
-        setFalse(block.getX(), block.getY(), block.getZ(), block.getWorld());
+    public synchronized void setFalse(@NotNull BlockState blockState) {
+        set(blockState.getX(), blockState.getY(), blockState.getZ(), blockState.getWorld(), false);
     }
 
-    @Override
-    public synchronized void setFalse(@Nullable BlockState blockState) {
-        if (blockState == null)
-            return;
-
-        setFalse(blockState.getX(), blockState.getY(), blockState.getZ(), blockState.getWorld());
-    }
-
-    public synchronized void set(int x, int y, int z, @Nullable World world, boolean value){
-        if (world == null)
-            return;
-
+    private synchronized void set(int x, int y, int z, @NotNull World world, boolean value){
         CoordinateKey chunkKey = blockCoordinateToChunkKey(world.getUID(), x, y, z);
 
         // Get/Load/Create chunkstore
@@ -350,7 +269,4 @@ public class HashChunkManager implements ChunkManager {
             return Objects.hash(worldID, x, z);
         }
     }
-
-    @Override
-    public synchronized void cleanUp() {}
 }

src/main/java/com/gmail/nossr50/util/blockmeta/NullChunkManager.java

@@ -3,6 +3,7 @@ package com.gmail.nossr50.util.blockmeta;
 import org.bukkit.World;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockState;
+import org.jetbrains.annotations.NotNull;
 
 public class NullChunkManager implements ChunkManager {
 
@@ -10,53 +11,30 @@ public class NullChunkManager implements ChunkManager {
     public void closeAll() {}
 
     @Override
-    public void saveChunk(int cx, int cz, World world) {}
+    public void chunkUnloaded(int cx, int cz, @NotNull World world) {}
 
     @Override
-    public void chunkUnloaded(int cx, int cz, World world) {}
+    public void unloadWorld(@NotNull World world) {}
 
     @Override
-    public void saveWorld(World world) {}
-
-    @Override
-    public void unloadWorld(World world) {}
-
-    @Override
-    public void saveAll() {}
-
-    @Override
-    public boolean isTrue(int x, int y, int z, World world) {
-        return false;
-    }
-
-    @Override
-    public boolean isTrue(Block block) {
+    public boolean isTrue(@NotNull Block block) {
         return false;
     }
 
     @Override
-    public boolean isTrue(BlockState blockState) {
+    public boolean isTrue(@NotNull BlockState blockState) {
         return false;
     }
 
     @Override
-    public void setTrue(int x, int y, int z, World world) {}
-
-    @Override
-    public void setTrue(Block block) {}
-
-    @Override
-    public void setTrue(BlockState blockState) {}
-
-    @Override
-    public void setFalse(int x, int y, int z, World world) {}
+    public void setTrue(@NotNull Block block) {}
 
     @Override
-    public void setFalse(Block block) {}
+    public void setTrue(@NotNull BlockState blockState) {}
 
     @Override
-    public void setFalse(BlockState blockState) {}
+    public void setFalse(@NotNull Block block) {}
 
     @Override
-    public void cleanUp() {}
+    public void setFalse(@NotNull BlockState blockState) {}
 }

src/main/java/com/gmail/nossr50/util/blockmeta/UserBlockTracker.java

@@ -0,0 +1,56 @@
+package com.gmail.nossr50.util.blockmeta;
+
+import com.gmail.nossr50.mcMMO;
+import org.bukkit.block.Block;
+import org.bukkit.block.BlockState;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Contains blockstore methods that are safe for external plugins to access.
+ * An instance can be retrieved via {@link mcMMO#getPlaceStore() mcMMO.getPlaceStore()}
+ */
+public interface UserBlockTracker {
+    /**
+     * Check to see if a given block location is set to true
+     *
+     * @param block Block location to check
+     * @return true if the given block location is set to true, false if otherwise
+     */
+    boolean isTrue(@NotNull Block block);
+
+    /**
+     * Check to see if a given BlockState location is set to true
+     *
+     * @param blockState BlockState to check
+     * @return true if the given BlockState location is set to true, false if otherwise
+     */
+    boolean isTrue(@NotNull BlockState blockState);
+
+    /**
+     * Set a given block location to true
+     *
+     * @param block Block location to set
+     */
+    void setTrue(@NotNull Block block);
+
+    /**
+     * Set a given BlockState location to true
+     *
+     * @param blockState BlockState location to set
+     */
+    void setTrue(@NotNull BlockState blockState);
+
+    /**
+     * Set a given block location to false
+     *
+     * @param block Block location to set
+     */
+    void setFalse(@NotNull Block block);
+
+    /**
+     * Set a given BlockState location to false
+     *
+     * @param blockState BlockState location to set
+     */
+    void setFalse(@NotNull BlockState blockState);
+}

src/test/java/ChunkStoreTest.java

@@ -2,6 +2,7 @@ import com.gmail.nossr50.util.blockmeta.*;
 import com.google.common.io.Files;
 import org.bukkit.Bukkit;
 import org.bukkit.World;
+import org.bukkit.block.Block;
 import org.jetbrains.annotations.NotNull;
 import org.junit.*;
 import org.junit.runner.RunWith;
@@ -141,9 +142,20 @@ public class ChunkStoreTest {
     @Test
     public void testRegressionChunkMirrorBug() {
         ChunkManager chunkManager = new HashChunkManager();
-        chunkManager.setTrue(15,0,15, mockWorld);
-        chunkManager.setFalse(-15, 0, -15, mockWorld);
-        Assert.assertTrue(chunkManager.isTrue(15, 0, 15, mockWorld));
+        Block mockBlockA = mock(Block.class);
+        Mockito.when(mockBlockA.getX()).thenReturn(15);
+        Mockito.when(mockBlockA.getZ()).thenReturn(15);
+        Mockito.when(mockBlockA.getY()).thenReturn(0);
+        Mockito.when(mockBlockA.getWorld()).thenReturn(mockWorld);
+        Block mockBlockB = mock(Block.class);
+        Mockito.when(mockBlockB.getX()).thenReturn(-15);
+        Mockito.when(mockBlockB.getZ()).thenReturn(-15);
+        Mockito.when(mockBlockB.getY()).thenReturn(0);
+        Mockito.when(mockBlockB.getWorld()).thenReturn(mockWorld);
+
+        chunkManager.setTrue(mockBlockA);
+        chunkManager.setFalse(mockBlockB);
+        Assert.assertTrue(chunkManager.isTrue(mockBlockA));
     }
 
     private interface Delegate {