Fixes and improves book saving

Removes redundancy
Fixes broken duplicate checking
Moves the save function to its own class

src/main/java/net/knarcraft/bookswithoutborders/BooksWithoutBorders.java

@@ -7,6 +7,7 @@ import java.io.FileWriter;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -18,13 +19,13 @@ import net.knarcraft.bookswithoutborders.command.CommandDelete;
 import net.knarcraft.bookswithoutborders.command.CommandEncrypt;
 import net.knarcraft.bookswithoutborders.command.CommandGive;
 import net.knarcraft.bookswithoutborders.command.CommandGroupEncrypt;
+import net.knarcraft.bookswithoutborders.command.CommandSave;
 import net.knarcraft.bookswithoutborders.command.CommandSavePublic;
 import net.knarcraft.bookswithoutborders.command.CommandSetBookPrice;
 import net.knarcraft.bookswithoutborders.command.CommandSetLore;
 import net.knarcraft.bookswithoutborders.command.CommandUnSign;
 import net.knarcraft.bookswithoutborders.command.GiveTabCompleter;
 import net.knarcraft.bookswithoutborders.state.EncryptionStyle;
-import net.knarcraft.bookswithoutborders.state.ItemSlot;
 import net.knarcraft.bookswithoutborders.utility.BookFormatter;
 import net.knarcraft.bookswithoutborders.utility.EncryptionHelper;
 import net.knarcraft.bookswithoutborders.utility.FileHelper;
@@ -70,9 +71,9 @@ public class BooksWithoutBorders extends JavaPlugin {
     protected static BooksWithoutBordersListener bL;
     protected static ConsoleCommandSender consoleSender;
     public static String bookFolder;
-    public static ChatColor errorColor = ChatColor.RED;
+    public static final ChatColor errorColor = ChatColor.RED;
-    protected static ChatColor successColor = ChatColor.GREEN;
+    protected static final ChatColor successColor = ChatColor.GREEN;
-    public static ChatColor commandColor = ChatColor.YELLOW;
+    public static final ChatColor commandColor = ChatColor.YELLOW;
 
     @Override
     public void onEnable() {
@@ -141,6 +142,11 @@ public class BooksWithoutBorders extends JavaPlugin {
         if (savePublicCommand != null) {
             savePublicCommand.setExecutor(new CommandSavePublic(this));
         }
+
+        PluginCommand saveCommand = this.getCommand("save");
+        if (saveCommand != null) {
+            saveCommand.setExecutor(new CommandSave(this));
+        }
     }
 
     protected boolean init() {
@@ -482,27 +488,6 @@ public class BooksWithoutBorders extends JavaPlugin {
             //Player only commands
             if (sender instanceof Player player) {
 
-                if (args[0].equalsIgnoreCase("save")) {
-                    if (!sender.hasPermission("bookswithoutborders.save")) {
-                        sendErrorMessage(sender, " You don't have permission to use this command!");
-                        return false;
-                    }
-
-                    ItemSlot holdingSlot = InventoryHelper.getHeldSlotBook(player, false, false, false, false);
-                    if (holdingSlot != ItemSlot.NONE) {
-                        ItemStack holdingItem = InventoryHelper.getHeldItem(player, holdingSlot == ItemSlot.MAIN_HAND);
-                        if (args.length == 2) {
-                            saveBook(player, holdingItem, args[1], false);
-                        } else {
-                            saveBook(player, holdingItem, "false", false);
-                        }
-                        return true;
-                    } else {
-                        sendErrorMessage(sender, "You must be holding a written book or book and quill to save it!");
-                        return false;
-                    }
-                }
-
                 if (args[0].equalsIgnoreCase("loadPublic")) {
                     if (!sender.hasPermission("bookswithoutborders.loadpublic")) {
                         sendErrorMessage(sender, " You don't have permission to use this command!");
@@ -773,40 +758,32 @@ public class BooksWithoutBorders extends JavaPlugin {
         return false;
     }
 
+    /**
+     * Removes any special character from a filename
+     * @param fileName <p>The file name to clean</p>
+     * @return <p>The cleaned file name</p>
+     */
     public String cleanString(String fileName) {
-        //removes illegal characters
-        if (fileName.contains("/")) {
         fileName = fileName.replace("/", "");
-        }
-        if (fileName.contains("\\")) {
         fileName = fileName.replace("\\", "");
-        }
-        if (fileName.contains("*")) {
         fileName = fileName.replace("*", "");
-        }
-        if (fileName.contains(":")) {
         fileName = fileName.replace(":", "");
-        }
-        if (fileName.contains("|")) {
         fileName = fileName.replace("|", "");
-        }
-        if (fileName.contains("<")) {
         fileName = fileName.replace("<", "");
-        }
-        if (fileName.contains(">")) {
         fileName = fileName.replace(">", "");
-        }
-        if (fileName.contains("?")) {
         fileName = fileName.replace("?", "");
-        }
-        if (fileName.contains("\"")) {
         fileName = fileName.replace("\"", "");
-        }
         return fileName;
     }
 
-    protected String fixName(String fileName, Boolean isLoad) {
+    /**
-        if (isLoad) {
+     * Changes spaces to underscores or underscores to spaces, depending on context
+     * @param fileName <p>The file name to fix</p>
+     * @param isLoading <p>Whether loading from a file as opposed to saving to a file</p>
+     * @return <p>The fixed name</p>
+     */
+    protected String fixName(String fileName, Boolean isLoading) {
+        if (isLoading) {
             fileName = fileName.replace("_", " ");
         } else {
             fileName = fileName.replace(" ", "_");
@@ -918,20 +895,29 @@ public class BooksWithoutBorders extends JavaPlugin {
         return bDat;
     }
 
-    public void saveBook(Player player, ItemStack heldBook, String dupe, Boolean saveToPublicFolder) {
+    /**
-        //Notice: Could be both a signed or unsigned book
+     * Saves a book to a file
+     *
+     * @param player             <p>The player holding the book</p>
+     * @param heldBook           <p>The book held</p>
+     * @param overwrite          <p>Whether to overwrite any existing books</p>
+     * @param saveToPublicFolder <p>Whether to save the book to the public folder instead of the player folder</p>
+     */
+    public void saveBook(Player player, ItemStack heldBook, boolean overwrite, boolean saveToPublicFolder) {
         BookMeta book = (BookMeta) heldBook.getItemMeta();
         if (book == null) {
             sendErrorMessage(player, "Unable to get metadata for your held book!");
             return;
         }
 
-        String path;
+        String savePath;
         if (saveToPublicFolder) {
-            path = bookFolder;
+            savePath = bookFolder;
         } else {
-            path = bookFolder + cleanString(player.getName()) + SLASH;
+            savePath = bookFolder + cleanString(player.getName()) + SLASH;
         }
+
+        //Generate book filename
         String fileName;
         if (!book.hasTitle()) {
             fileName = "Untitled," + player.getName();
@@ -940,61 +926,55 @@ public class BooksWithoutBorders extends JavaPlugin {
         }
         fileName = cleanString(fileName);
         fileName = fixName(fileName, false);
-        int dupes = 0;
 
-        //checks to see if the file name is already taken and adjusts it if so
+        //Make sure the used folders exist
-        File file = new File(path);
+        File file = new File(savePath);
-
-        //if this is a private save, make sure the dir exists
         if (!file.exists() && !file.mkdir()) {
             sendErrorMessage(player, "Saving Failed! If this continues to happen, consult server admin!");
             return;
         }
-
         File[] foundFiles = file.listFiles();
         if (foundFiles == null) {
             sendErrorMessage(player, "Saving Failed! If this continues to happen, consult server admin!");
             return;
         }
 
-        if (foundFiles.length > 0) {
+        sendErrorMessage(player, String.valueOf(overwrite));
-            for (int x = 0; x < foundFiles.length; x++) {
+        sendErrorMessage(player, fileName);
-                if (dupes == 0) {
+        sendErrorMessage(player, Arrays.toString(foundFiles));
-                    if (foundFiles[x].getName().equalsIgnoreCase(fileName)) {
+
-                        dupes++;
+        //Find any duplicates of the book
-                        x = -1;
+        int foundDuplicates = FileHelper.findDuplicates(foundFiles, fileName);
-                    }
+
-                } else {
+        //Deal with duplicates
-                    if (foundFiles[x].getName().equalsIgnoreCase("(" + dupes + ")" + fileName)) {
+        if (foundDuplicates > 0) {
-                        dupes++;
+            //TODO: Decide if this makes sense or needs to be changed
-                        x = -1;
+            //Skip duplicate book
-                    }
+            if (!fileName.contains("Untitled") && !overwrite) {
-                }
-            }
-        }
-        if (dupes > 0) {
-            if (!fileName.contains("Untitled") && dupe.equalsIgnoreCase("false")) {
                 sendErrorMessage(player, "Book is already saved!");
                 sendErrorMessage(player, "Use " + commandColor + "/bwb Save true " + errorColor + "to overwrite!");
                 return;
             }
 
-            if (dupes > bookDuplicateLimit) {
+            //Skip if duplicate limit is reached
+            if (foundDuplicates > bookDuplicateLimit) {
                 sendErrorMessage(player, "Maximum amount of " + fileName + " duplicates reached!");
                 sendErrorMessage(player, "Use " + commandColor + "/bwb Save true " + errorColor + "to overwrite!");
                 return;
             }
 
-            if (fileName.contains("Untitled") && dupe.equalsIgnoreCase("false")) {
+            //Alter duplicated filename
-                fileName = "(" + dupes + ")" + fileName;
+            if (fileName.contains("Untitled") && !overwrite) {
+                fileName = "(" + foundDuplicates + ")" + fileName;
             }
         }
 
         try {
-            if (useYml)
+            if (useYml) {
-                bookToYml(path, fileName, book);
+                bookToYml(savePath, fileName, book);
-            else
+            } else {
-                bookToTXT(path, fileName, book);
+                bookToTXT(savePath, fileName, book);
+            }
 
             sendSuccessMessage(player, "Book Saved as \"" + fileName + "\"");
         } catch (IOException e) {

src/main/java/net/knarcraft/bookswithoutborders/command/CommandSave.java

@@ -1,12 +1,52 @@
 package net.knarcraft.bookswithoutborders.command;
 
+import net.knarcraft.bookswithoutborders.BooksWithoutBorders;
+import net.knarcraft.bookswithoutborders.state.ItemSlot;
+import net.knarcraft.bookswithoutborders.utility.InventoryHelper;
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandExecutor;
 import org.bukkit.command.CommandSender;
+import org.bukkit.entity.Player;
+import org.bukkit.inventory.ItemStack;
 
+/**
+ * Command executor for the save command
+ */
 public class CommandSave implements CommandExecutor {
+
+    private final BooksWithoutBorders booksWithoutBorders;
+
+    public CommandSave(BooksWithoutBorders booksWithoutBorders) {
+        this.booksWithoutBorders = booksWithoutBorders;
+    }
+
     @Override
     public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
+        return saveHeldBook(sender, args, false);
+    }
+
+    /**
+     * Saves the player's held book if it exists
+     * @param sender <p>The sender of the command</p>
+     * @param args <p>The arguments given</p>
+     * @param savePublic <p>Whether to save the book in the public directory or the player directory</p>
+     * @return <p>True if a book was saved successfully</p>
+     */
+    boolean saveHeldBook(CommandSender sender, String[] args, boolean savePublic) {
+        if (!(sender instanceof Player player)) {
+            BooksWithoutBorders.sendErrorMessage(sender, "This command can only be used by a player!");
             return false;
         }
+
+        ItemSlot holdingSlot = InventoryHelper.getHeldSlotBook(player, false, false, false, false);
+        if (holdingSlot != ItemSlot.NONE) {
+            ItemStack holdingItem = InventoryHelper.getHeldItem(player, holdingSlot == ItemSlot.MAIN_HAND);
+            boolean duplicate = args.length == 1 && Boolean.parseBoolean(args[0]);
+            booksWithoutBorders.saveBook(player, holdingItem, duplicate, savePublic);
+            return true;
+        } else {
+            BooksWithoutBorders.sendErrorMessage(sender, "You must be holding a written book or book and quill to save it!");
+            return false;
+        }
+    }
 }

src/main/java/net/knarcraft/bookswithoutborders/command/CommandSavePublic.java

@@ -1,45 +1,22 @@
 package net.knarcraft.bookswithoutborders.command;
 
 import net.knarcraft.bookswithoutborders.BooksWithoutBorders;
-import net.knarcraft.bookswithoutborders.state.ItemSlot;
-import net.knarcraft.bookswithoutborders.utility.InventoryHelper;
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandExecutor;
 import org.bukkit.command.CommandSender;
-import org.bukkit.entity.Player;
-import org.bukkit.inventory.ItemStack;
 
 /**
  * Command executor for the save public command
  */
-public class CommandSavePublic implements CommandExecutor {
+public class CommandSavePublic extends CommandSave implements CommandExecutor {
-
-    private final BooksWithoutBorders booksWithoutBorders;
 
     public CommandSavePublic(BooksWithoutBorders booksWithoutBorders) {
-        this.booksWithoutBorders = booksWithoutBorders;
+        super(booksWithoutBorders);
     }
 
     @Override
     public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
-        if (!(sender instanceof Player player)) {
+        return saveHeldBook(sender, args, true);
-            BooksWithoutBorders.sendErrorMessage(sender, "This command can only be used by a player!");
-            return false;
-        }
-
-        ItemSlot holdingSlot = InventoryHelper.getHeldSlotBook(player, false, false, false, false);
-        if (holdingSlot != ItemSlot.NONE) {
-            ItemStack holdingItem = InventoryHelper.getHeldItem(player, holdingSlot == ItemSlot.MAIN_HAND);
-            if (args.length == 1) {
-                booksWithoutBorders.saveBook(player, holdingItem, args[0], true);
-            } else {
-                booksWithoutBorders.saveBook(player, holdingItem, "false", true);
-            }
-            return true;
-        } else {
-            BooksWithoutBorders.sendErrorMessage(sender, "You must be holding a written book or book and quill to save it!");
-            return false;
-        }
     }
 
 }

src/main/java/net/knarcraft/bookswithoutborders/command/CommandSetBookPrice.java

@@ -24,7 +24,8 @@ public class CommandSetBookPrice implements CommandExecutor {
     public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
         //Clear the current price
         if (args.length == 0) {
-            return clearItemPrice(sender);
+            clearItemPrice(sender);
+            return true;
         }
 
         //Warn about missing arguments
@@ -60,9 +61,8 @@ public class CommandSetBookPrice implements CommandExecutor {
     /**
      * Removes the book price
      * @param sender <p>The sender of the command</p>
-     * @return <p>True if the price was changed successfully</p>
      */
-    private boolean clearItemPrice(CommandSender sender) {
+    private void clearItemPrice(CommandSender sender) {
         BooksWithoutBorders.bookPriceType = null;
         BooksWithoutBorders.bookPriceQuantity = 0;
         booksWithoutBorders.getConfig().set("Options.Price_to_create_book.Item_type", "Item type name");
@@ -70,7 +70,6 @@ public class CommandSetBookPrice implements CommandExecutor {
         booksWithoutBorders.saveConfig();
 
         BooksWithoutBorders.sendSuccessMessage(sender, "Price to create books removed!");
-        return true;
     }
 
     /**

src/main/java/net/knarcraft/bookswithoutborders/command/GiveTabCompleter.java

@@ -13,7 +13,7 @@ import java.util.List;
 
 public class GiveTabCompleter implements TabCompleter {
 
-    BooksWithoutBorders booksWithoutBorders;
+    final BooksWithoutBorders booksWithoutBorders;
 
     public GiveTabCompleter(BooksWithoutBorders booksWithoutBorders) {
         this.booksWithoutBorders = booksWithoutBorders;

src/main/java/net/knarcraft/bookswithoutborders/utility/FileHelper.java

@@ -7,6 +7,7 @@ import org.bukkit.command.CommandSender;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.regex.Pattern;
 
 public class FileHelper {
 
@@ -85,4 +86,21 @@ public class FileHelper {
         return fileList;
     }
 
+    /**
+     * Finds any duplicate book files
+     *
+     * @param foundFiles <p>The files to check for duplicates</p>
+     * @param fileName   <p>The name of the file which might already exist</p>
+     * @return <p>The number of found duplicates</p>
+     */
+    public static int findDuplicates(File[] foundFiles, String fileName) {
+        int foundDuplicates = 0;
+        for (File foundFile : foundFiles) {
+            if (foundFile.getName().matches("(\\([0-9]+\\))?" + Pattern.quote(fileName) + "(\\.yml|\\.txt)?")) {
+                foundDuplicates++;
+            }
+        }
+        return foundDuplicates;
+    }
+
 }

src/main/resources/plugin.yml

@@ -53,6 +53,10 @@ commands:
     description: Saves the held book to the public books folder
     usage: /<command> [duplicate (true/false)]
     permission: bookswithoutborders.savepublic
+  save:
+    description: Saves the held book to the holding player's folder
+    usage: /<command>
+    permission: bookswithoutborders.save
 permissions:
   bookswithoutborders.admin:
     description: Grants all permissions

src/test/java/net/knarcraft/bookswithoutborders/util/FileHelperTest.java

@@ -0,0 +1,25 @@
+package net.knarcraft.bookswithoutborders.util;
+
+import net.knarcraft.bookswithoutborders.utility.FileHelper;
+import org.junit.Test;
+
+import java.io.File;
+
+import static org.junit.Assert.assertEquals;
+
+public class FileHelperTest {
+
+    @Test
+    public void findDuplicatesTest() {
+        File[] files = new File[5];
+        files[0] = new File("test/asd/book+)Fish.yml");
+        files[1] = new File("test/asd/book+)Crab.yml");
+        files[2] = new File("test/asd/(3)book+)Crab.yml");
+        files[3] = new File("test/asd/(2)book+)Crab.yml");
+        files[4] = new File("test/asd/(1)book+)Crab.yml");
+        assertEquals(4, FileHelper.findDuplicates(files, "book+)Crab.yml"));
+        assertEquals(1, FileHelper.findDuplicates(files, "book+)Fish.yml"));
+        assertEquals(0, FileHelper.findDuplicates(files, "book+)Horse.yml"));
+    }
+
+}