From 5e5ca3d249d15ddbfe3ca258ca0e81e9a2ba58d1 Mon Sep 17 00:00:00 2001 From: beefcake123 Date: Sat, 13 Dec 2025 17:32:35 +0100 Subject: [PATCH] Stability update of v2.6.1 --- pom.xml | 6 +- .../commandrunners/ShopUserSubCommand.java | 6 + .../tradeshop/data/storage/DataStorage.java | 14 +++ .../data/storage/Json/JsonConfiguration.java | 115 ++++++++++++++++-- .../java/org/shanerx/tradeshop/shop/Shop.java | 12 +- .../org/shanerx/tradeshop/shop/ShopChest.java | 94 ++++++++++---- .../shop/listeners/ShopCreateListener.java | 6 + .../tradeshop/shoplocation/ShopChunk.java | 12 +- .../org/shanerx/tradeshop/utils/Utils.java | 7 +- 9 files changed, 224 insertions(+), 48 deletions(-) diff --git a/pom.xml b/pom.xml index 98870e47..e13d8238 100644 --- a/pom.xml +++ b/pom.xml @@ -161,8 +161,8 @@ maven-compiler-plugin 3.8.1 - 1.8 - 1.8 + 17 + 17 @@ -257,7 +257,7 @@ org.spigotmc spigot-api - 1.19.2-R0.1-SNAPSHOT + 1.21.10-R0.1-SNAPSHOT provided diff --git a/src/main/java/org/shanerx/tradeshop/commands/commandrunners/ShopUserSubCommand.java b/src/main/java/org/shanerx/tradeshop/commands/commandrunners/ShopUserSubCommand.java index 0b58ed2f..c8879944 100644 --- a/src/main/java/org/shanerx/tradeshop/commands/commandrunners/ShopUserSubCommand.java +++ b/src/main/java/org/shanerx/tradeshop/commands/commandrunners/ShopUserSubCommand.java @@ -97,6 +97,12 @@ public void editUser(ShopRole role, ShopChange change) { Set ownedShops = new HashSet<>(); Map updateStatuses = new HashMap<>(); + target = Bukkit.getOfflinePlayer(command.getArgAt(1)); + if(target == null) { + Message.PLAYER_NOT_FOUND.sendMessage(command.getPlayerSender()); + return; + } + Shop tempShop = shopUserCommandStart(Bukkit.getOfflinePlayer(command.getArgAt(1)), applyAllOwned); if (applyAllOwned) { diff --git a/src/main/java/org/shanerx/tradeshop/data/storage/DataStorage.java b/src/main/java/org/shanerx/tradeshop/data/storage/DataStorage.java index 8434bbed..ee075f2f 100644 --- a/src/main/java/org/shanerx/tradeshop/data/storage/DataStorage.java +++ b/src/main/java/org/shanerx/tradeshop/data/storage/DataStorage.java @@ -42,6 +42,9 @@ import org.shanerx.tradeshop.utils.Utils; import org.shanerx.tradeshop.utils.debug.DebugLevels; +import com.google.gson.JsonParseException; +import com.google.gson.stream.MalformedJsonException; + import java.io.File; import java.util.ArrayList; import java.util.Arrays; @@ -108,7 +111,18 @@ public int getShopCountInWorld(World world) { if (folder.exists() && folder.listFiles() != null) { for (File file : folder.listFiles()) { if (file.getName().contains(world.getName())) + try { count += new JsonShopConfiguration(ShopChunk.deserialize(file.getName().replace(".json", ""))).size(); + } catch(Exception ex){ + if(ex instanceof MalformedJsonException) + { + TradeShop.getPlugin().getLogger().warning("[REINARD] - Malformed JSON in file: " + file.getName()); + } + else + { + TradeShop.getPlugin().getLogger().warning("[REINARD] - Error during loading shop" + file.getName() + ": " + ex.getClass().toString() + ": " + ex.getMessage() + ": " + ex.getCause()); + } + } } } break; diff --git a/src/main/java/org/shanerx/tradeshop/data/storage/Json/JsonConfiguration.java b/src/main/java/org/shanerx/tradeshop/data/storage/Json/JsonConfiguration.java index 7c796d5a..c5aaff76 100644 --- a/src/main/java/org/shanerx/tradeshop/data/storage/Json/JsonConfiguration.java +++ b/src/main/java/org/shanerx/tradeshop/data/storage/Json/JsonConfiguration.java @@ -36,6 +36,9 @@ import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.logging.Level; class JsonConfiguration extends Utils { @@ -44,6 +47,7 @@ class JsonConfiguration extends Utils { protected JsonObject jsonObj; private final TradeShop PLUGIN = TradeShop.getPlugin(); + private static final String TEMP_FOLDER_NAME = ".tmp"; /** * Creates a JsonConfiguration object assisting with managing JSON data @@ -68,6 +72,18 @@ public static File getPath(String folderFromData) { return new File(TradeShop.getPlugin().getDataFolder().getAbsolutePath() + File.separator + "Data" + File.separator + folderFromData); } + /** + * Gets the temporary folder for storing temporary files during writes. + * Creates the folder if it doesn't exist. + */ + private File getTempFolder() { + File tempFolder = new File(TradeShop.getPlugin().getDataFolder().getAbsolutePath() + File.separator + TEMP_FOLDER_NAME); + if (!tempFolder.exists()) { + tempFolder.mkdirs(); + } + return tempFolder; + } + private void buildFilePath() { try { pathFile.mkdirs(); @@ -78,25 +94,110 @@ private void buildFilePath() { } protected void loadFile() { - try { - jsonObj = new JsonParser().parse(new FileReader(file)).getAsJsonObject(); + try (FileReader fileReader = new FileReader(file)) { + jsonObj = new JsonParser().parse(fileReader).getAsJsonObject(); } catch (FileNotFoundException e) { PLUGIN.getLogger().log(Level.SEVERE, "Could not load " + file.getName() + " file! Data may be lost!", e); + jsonObj = new JsonObject(); } catch (IllegalStateException e) { jsonObj = new JsonObject(); + } catch (IOException e) { + PLUGIN.getLogger().log(Level.SEVERE, "Could not read " + file.getName() + " file!", e); + jsonObj = new JsonObject(); } } + /** + * Saves the JSON object to file using atomic write operation. + * This prevents data corruption by writing to a temporary file first, + * then atomically replacing the original file. + * + * This approach resolves issues with enchanted books and other complex + * serializable items that may cause partial writes or file corruption. + */ protected void saveFile() { String str = gson.toJson(jsonObj); - if (!str.isEmpty()) { - try { - FileWriter fileWriter = new FileWriter(this.file); + if (str.isEmpty()) { + return; + } + + File tempFile = null; + try { + // Create temporary file in a separate .tmp folder + // This prevents the temporary files from interfering with deserialization logic + File tempFolder = getTempFolder(); + tempFile = File.createTempFile(file.getName() + "_", ".tmp", tempFolder); + + // Write data to temporary file + try (FileWriter fileWriter = new FileWriter(tempFile, StandardCharsets.UTF_8)) { fileWriter.write(str); fileWriter.flush(); - fileWriter.close(); + } + + // Verify the temporary file was written correctly before replacing + if (tempFile.length() == 0) { + PLUGIN.getLogger().log(Level.SEVERE, "Failed to write to temporary file for " + file.getName() + "! File may be corrupted."); + if (tempFile.delete()) { + PLUGIN.getLogger().log(Level.INFO, "Cleaned up empty temporary file: " + tempFile.getPath()); + } + return; + } + + // Attempt to replace the file with retry logic for Windows file locking + replaceFileWithRetry(tempFile, file); + + } catch (IOException e) { + PLUGIN.getLogger().log(Level.SEVERE, "Could not save " + file.getName() + " file! Data may be lost!", e); + + // Cleanup temporary file on error + if (tempFile != null && tempFile.exists()) { + if (!tempFile.delete()) { + PLUGIN.getLogger().log(Level.WARNING, "Failed to cleanup temporary file: " + tempFile.getPath()); + } + } + } + } + + /** + * Attempts to replace the target file with the temporary file, with retry logic. + * This handles Windows file locking issues where files may be temporarily in use. + */ + private void replaceFileWithRetry(File tempFile, File targetFile) { + final int MAX_RETRIES = 3; + final long RETRY_DELAY_MS = 100; + + for (int attempt = 1; attempt <= MAX_RETRIES; attempt++) { + try { + // Try atomic move first + try { + Files.move(tempFile.toPath(), targetFile.toPath(), StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); + return; // Success + } catch (IOException atomicMoveException) { + // Atomic move failed, try regular move (non-atomic) + try { + Files.move(tempFile.toPath(), targetFile.toPath(), StandardCopyOption.REPLACE_EXISTING); + return; // Success + } catch (IOException regularMoveException) { + // Both methods failed, throw to retry logic + throw regularMoveException; + } + } + } catch (IOException e) { - PLUGIN.getLogger().log(Level.SEVERE, "Could not save " + file.getName() + " file! Data may be lost!", e); + // If this is the last attempt, log error and keep temp file + if (attempt == MAX_RETRIES) { + PLUGIN.getLogger().log(Level.SEVERE, "Could not save " + targetFile.getName() + " file after " + MAX_RETRIES + " attempts! Temp file available at: " + tempFile.getAbsolutePath(), e); + return; + } + + // Wait before retrying (gives file handles time to release) + try { + Thread.sleep(RETRY_DELAY_MS); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + PLUGIN.getLogger().log(Level.WARNING, "File save retry interrupted for " + targetFile.getName()); + return; + } } } } diff --git a/src/main/java/org/shanerx/tradeshop/shop/Shop.java b/src/main/java/org/shanerx/tradeshop/shop/Shop.java index 2bc32e1e..3d985846 100644 --- a/src/main/java/org/shanerx/tradeshop/shop/Shop.java +++ b/src/main/java/org/shanerx/tradeshop/shop/Shop.java @@ -702,7 +702,7 @@ public void setShopSettings(Map> newSettings) { * @return list of ShopUsers. */ public List getUsers(ShopRole... roles) { - return getUsersExcluding(Collections.emptyList(), roles); + return getUsersUUID(roles).stream().map((uuid) -> new ShopUser(uuid, ShopRole.MANAGER)).collect(Collectors.toList()); } /** @@ -713,13 +713,7 @@ public List getUsers(ShopRole... roles) { * @return list of ShopUsers. */ public List getUsersExcluding(List excludedPlayers, ShopRole... roles) { - List users = new ArrayList<>(); - getUsers(roles).forEach(user -> { - if (!excludedPlayers.contains(user.getUUID())) - users.add(user); - }); - - return users; + return getUsers(roles).stream().filter((shopUser) -> !excludedPlayers.contains(shopUser.getUUID())).collect(Collectors.toList()); } /** @@ -936,7 +930,7 @@ public void fixSide(ShopItemSide side) { Set matSet = new HashSet<>(); - ogItems.forEach((item) -> matSet.add(item.getItemStack().getType())); + ogItems.stream().filter(shopItemStack -> shopItemStack.getItemStack() != null).forEach((item) -> matSet.add(item.getItemStack().getType())); if (ogItems.size() > 1 && ogItems.size() != matSet.size()) { diff --git a/src/main/java/org/shanerx/tradeshop/shop/ShopChest.java b/src/main/java/org/shanerx/tradeshop/shop/ShopChest.java index 56cbfa56..707853a5 100644 --- a/src/main/java/org/shanerx/tradeshop/shop/ShopChest.java +++ b/src/main/java/org/shanerx/tradeshop/shop/ShopChest.java @@ -76,41 +76,83 @@ public ShopChest(Block chest, UUID owner, Location sign) { public static boolean isShopChest(Block checking) { try { + // Check if input is valid + if (checking == null) { + plugin.getDebugger().log("isShopChest: Block input is null", DebugLevels.PROTECTION); + return false; + } + if (isDoubleChest(checking)) { + //plugin.getDebugger().log("isShopChest: Block is a double chest", DebugLevels.PROTECTION); DoubleChest dbl = getDoubleChest(checking); - return ((Container) dbl.getLeftSide()).getPersistentDataContainer().has(plugin.getSignKey(), PersistentDataType.STRING) || - ((Container) dbl.getRightSide()).getPersistentDataContainer().has(plugin.getSignKey(), PersistentDataType.STRING); + if (dbl == null) { + plugin.getDebugger().log("isShopChest: getDoubleChest returned null", DebugLevels.PROTECTION); + return false; + } + + // Check left side + InventoryHolder leftSide = dbl.getLeftSide(); + if (leftSide == null) { + //plugin.getDebugger().log("isShopChest: Double chest left side is null", DebugLevels.PROTECTION); + } else { + Container leftContainer = (Container) leftSide; + if (leftContainer.getPersistentDataContainer() == null) { + //plugin.getDebugger().log("isShopChest: Left container persistent data container is null", DebugLevels.PROTECTION); + } else if (leftContainer.getPersistentDataContainer().has(plugin.getSignKey(), PersistentDataType.STRING)) { + //plugin.getDebugger().log("isShopChest: Found sign key in left side of double chest", DebugLevels.PROTECTION); + return true; + } + } + + // Check right side + InventoryHolder rightSide = dbl.getRightSide(); + if (rightSide == null) { + //plugin.getDebugger().log("isShopChest: Double chest right side is null", DebugLevels.PROTECTION); + } else { + Container rightContainer = (Container) rightSide; + if (rightContainer.getPersistentDataContainer() == null) { + //plugin.getDebugger().log("isShopChest: Right container persistent data container is null", DebugLevels.PROTECTION); + } else if (rightContainer.getPersistentDataContainer().has(plugin.getSignKey(), PersistentDataType.STRING)) { + //plugin.getDebugger().log("isShopChest: Found sign key in right side of double chest", DebugLevels.PROTECTION); + return true; + } + } + + return false; } - boolean conHas = ((Container) checking.getState()).getPersistentDataContainer().has(plugin.getSignKey(), PersistentDataType.STRING); - return conHas; - } catch (NullPointerException | ClassCastException ex) { - plugin.getDebugger().log("NPE thrown during isShopChest by: \n" + ex.getCause(), DebugLevels.PROTECTION); - } - return false; - // Old MoveEvent Est ~50mspt - /*plugin.getDebugger().log("isShopChest checking Block at " + new ShopLocation(checking.getLocation()).serialize() + "", DebugLevels.PROTECTION); - try { - if (isDoubleChest(checking)) { - DoubleChest dbl = getDoubleChest(checking); - boolean leftHas = ((Container) dbl.getLeftSide()).getPersistentDataContainer().has(plugin.getSignKey(), PersistentDataType.STRING), - rightHas = ((Container) dbl.getRightSide()).getPersistentDataContainer().has(plugin.getSignKey(), PersistentDataType.STRING); + // Single chest check + //plugin.getDebugger().log("isShopChest: Block is a single inventory", DebugLevels.PROTECTION); + BlockState state = checking.getState(); + if (state == null) { + plugin.getDebugger().log("isShopChest: Block state is null", DebugLevels.PROTECTION); + return false; + } - plugin.getDebugger().log("Block is DoubleChest", DebugLevels.PROTECTION); - plugin.getDebugger().log("Left side PerData: " + (leftHas ? ((Container) dbl.getLeftSide()).getPersistentDataContainer().get(plugin.getSignKey(), PersistentDataType.STRING) : "null"), DebugLevels.PROTECTION); - plugin.getDebugger().log("Right side PerData: " + (rightHas ? ((Container) dbl.getRightSide()).getPersistentDataContainer().get(plugin.getSignKey(), PersistentDataType.STRING) : "null"), DebugLevels.PROTECTION); + if (!(state instanceof Container)) { + plugin.getDebugger().log("isShopChest: Block state is not a Container: " + state.getType().toString(), DebugLevels.PROTECTION); + return false; + } - return leftHas || rightHas; + Container container = (Container) state; + if (container.getPersistentDataContainer() == null) { + plugin.getDebugger().log("isShopChest: Container persistent data container is null", DebugLevels.PROTECTION); + return false; } - boolean conHas = ((Container) checking.getState()).getPersistentDataContainer().has(plugin.getSignKey(), PersistentDataType.STRING); - plugin.getDebugger().log("Block is SINGLE inventory", DebugLevels.PROTECTION); - plugin.getDebugger().log("Storage Block PerData: " + (conHas ? ((Container) checking.getState()).getPersistentDataContainer().get(plugin.getSignKey(), PersistentDataType.STRING) : "null"), DebugLevels.PROTECTION); - return conHas; - } catch (NullPointerException | ClassCastException ex) { - plugin.getDebugger().log("NPE thrown during isShopChest by: \n" + ex.getCause(), DebugLevels.PROTECTION); + + boolean conHas = container.getPersistentDataContainer().has(plugin.getSignKey(), PersistentDataType.STRING); + //plugin.getDebugger().log("isShopChest: Single chest sign key check result: " + conHas, DebugLevels.PROTECTION); + return conHas; + + } catch (NullPointerException ex) { + plugin.getDebugger().log("NPE thrown during isShopChest: " + ex.getMessage(), DebugLevels.PROTECTION); + plugin.getDebugger().log("Stack trace: ", DebugLevels.PROTECTION); + ex.printStackTrace(); + } catch (ClassCastException ex) { + plugin.getDebugger().log("ClassCastException thrown during isShopChest: " + ex.getMessage(), DebugLevels.PROTECTION); } - return false;*/ + return false; } public static boolean isShopChest(Inventory checking) { diff --git a/src/main/java/org/shanerx/tradeshop/shop/listeners/ShopCreateListener.java b/src/main/java/org/shanerx/tradeshop/shop/listeners/ShopCreateListener.java index 5fdf12ff..84ad2291 100644 --- a/src/main/java/org/shanerx/tradeshop/shop/listeners/ShopCreateListener.java +++ b/src/main/java/org/shanerx/tradeshop/shop/listeners/ShopCreateListener.java @@ -38,6 +38,7 @@ import org.shanerx.tradeshop.shop.Shop; import org.shanerx.tradeshop.shop.ShopType; import org.shanerx.tradeshop.utils.Utils; +import org.shanerx.tradeshop.utils.debug.DebugLevels; @SuppressWarnings("unused") public class ShopCreateListener extends Utils implements Listener { @@ -55,10 +56,15 @@ public void onSignChange(SignChangeEvent event) { shopSign.setLine(3, event.getLine(3)); if (!ShopType.isShop(shopSign)) { + TradeShop.getPlugin().getDebugger().log("ShopType.isShop() returned false for sign: " + event.getLine(0), DebugLevels.SHOP_CREATION); return; } ShopType shopType = ShopType.getType(shopSign); + if (shopType == null) { + TradeShop.getPlugin().getDebugger().log("ShopType.getType() returned null for sign: " + event.getLine(0), DebugLevels.SHOP_CREATION); + return; + } Player p = event.getPlayer(); // Clear the first line since we already know it is going to be a Shop, and we have the type to pass separately diff --git a/src/main/java/org/shanerx/tradeshop/shoplocation/ShopChunk.java b/src/main/java/org/shanerx/tradeshop/shoplocation/ShopChunk.java index 55948838..ce61cac1 100644 --- a/src/main/java/org/shanerx/tradeshop/shoplocation/ShopChunk.java +++ b/src/main/java/org/shanerx/tradeshop/shoplocation/ShopChunk.java @@ -29,10 +29,13 @@ import org.bukkit.Chunk; import org.bukkit.ChunkSnapshot; import org.bukkit.World; +import org.shanerx.tradeshop.TradeShop; import java.io.Serializable; +import java.util.logging.Level; public class ShopChunk implements Serializable { + final private String div = ";;"; private final World world; @@ -59,9 +62,14 @@ public static ShopChunk deserialize(String loc) { World world = Bukkit.getWorld(locA[1]); if (world == null) world = Bukkit.getWorld(locA[1].replace("-", "_")); - int x = Integer.parseInt(locA[2]), z = Integer.parseInt(locA[3]); - + try { + int x = Integer.parseInt(locA[2]); + int z = Integer.parseInt(locA[3]); return new ShopChunk(world, x, z); + } catch (NumberFormatException e) { + TradeShop.getPlugin().getLogger().log(Level.SEVERE, "Failed to deserialize ShopChunk from string: " + loc, e); + return null; + } } return null; diff --git a/src/main/java/org/shanerx/tradeshop/utils/Utils.java b/src/main/java/org/shanerx/tradeshop/utils/Utils.java index 90f11c95..630bce20 100644 --- a/src/main/java/org/shanerx/tradeshop/utils/Utils.java +++ b/src/main/java/org/shanerx/tradeshop/utils/Utils.java @@ -473,11 +473,13 @@ public Tuple> canExchangeAll(Shop shop, Inventor */ public Shop createShop(Sign shopSign, Player creator, ShopType shopType, ItemStack cost, ItemStack product, SignChangeEvent event) { if (ShopType.isShop(shopSign)) { + TradeShop.getPlugin().getDebugger().log("ShopType.isShop() returned true for sign: " + shopSign.getLine(0), DebugLevels.SHOP_CREATION); Message.EXISTING_SHOP.sendMessage(creator); return null; } if (!shopType.checkPerm(creator)) { + TradeShop.getPlugin().getDebugger().log("ShopType.checkPerm() returned false for player: " + creator.getName(), DebugLevels.SHOP_CREATION); Message.NO_TS_CREATE_PERMISSION.sendMessage(creator); return null; } @@ -485,11 +487,13 @@ public Shop createShop(Sign shopSign, Player creator, ShopType shopType, ItemSta ShopUser owner = new ShopUser(creator, ShopRole.OWNER); if (!checkShopChest(shopSign.getBlock()) && !shopType.isITrade()) { + TradeShop.getPlugin().getDebugger().log("checkShopChest() returned false for sign at location: " + shopSign.getLocation().toString(), DebugLevels.SHOP_CREATION); Message.NO_CHEST.sendMessage(creator); return null; } if (Setting.MAX_SHOPS_PER_CHUNK.getInt() <= PLUGIN.getDataStorage().getShopCountInChunk(shopSign.getChunk())) { + PLUGIN.getDebugger().log("Max shops per chunk reached at chunk: " + shopSign.getChunk().toString(), DebugLevels.SHOP_CREATION); Message.TOO_MANY_CHESTS.sendMessage(creator); return null; } @@ -506,11 +510,13 @@ public Shop createShop(Sign shopSign, Player creator, ShopType shopType, ItemSta } if (shopChest.hasOwner() && !shopChest.getOwner().equals(owner.getUUID())) { + PLUGIN.getDebugger().log("ShopChest has owner: " + shopChest.getOwner().toString() + " which does not match creator: " + owner.getUUID().toString(), DebugLevels.SHOP_CREATION); Message.NO_SHOP_PERMISSION.sendMessage(creator); return null; } if (shopChest.hasShopSign() && !shopChest.getShopSign().getLocation().equals(shopSign.getLocation())) { + PLUGIN.getDebugger().log("ShopChest is already linked to another shop sign at location: " + shopChest.getShopSign().getLocation().toString(), DebugLevels.SHOP_CREATION); Message.EXISTING_SHOP.sendMessage(creator); return null; } @@ -534,7 +540,6 @@ public Shop createShop(Sign shopSign, Player creator, ShopType shopType, ItemSta PLUGIN.getDebugger().log("-----Pre-Event-----", DebugLevels.SHOP_CREATION); PLUGIN.getDebugger().log(shop.toDebug(), DebugLevels.SHOP_CREATION); - PlayerShopCreateEvent shopCreateEvent = new PlayerShopCreateEvent(creator, shop); Bukkit.getPluginManager().callEvent(shopCreateEvent); PLUGIN.getDebugger().log("-----Post-Event-----", DebugLevels.SHOP_CREATION);