From 5e868e3fb728d78ab02b91e3350aa1886b7a4ac4 Mon Sep 17 00:00:00 2001 From: tastybento Date: Sat, 19 Sep 2026 19:58:22 +0100 Subject: [PATCH] Share the metadata map-copy helper between Players and Island SonarCloud flagged the toConcurrentMap helper and getMetaData body added in #3082 as a duplicated block across Players and Island. Move the helper to a static method on MetaDataAble so there is one copy, and have both implementations call it. Co-Authored-By: Claude Fable 5.1 --- .../bentobox/api/metadata/MetaDataAble.java | 21 ++++++++++++++++ .../bentobox/database/objects/Island.java | 25 +++---------------- .../bentobox/database/objects/Players.java | 25 +++---------------- 3 files changed, 27 insertions(+), 44 deletions(-) diff --git a/src/main/java/world/bentobox/bentobox/api/metadata/MetaDataAble.java b/src/main/java/world/bentobox/bentobox/api/metadata/MetaDataAble.java index 4e59d0fc1..4721fa3ad 100644 --- a/src/main/java/world/bentobox/bentobox/api/metadata/MetaDataAble.java +++ b/src/main/java/world/bentobox/bentobox/api/metadata/MetaDataAble.java @@ -1,6 +1,7 @@ package world.bentobox.bentobox.api.metadata; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.Optional; /** @@ -56,4 +57,24 @@ default Optional removeMetaData(String key) { return getMetaData().map(m -> m.remove(key)); } + /** + * Copies the given map into a new {@link ConcurrentHashMap}, dropping null keys and values, + * which a concurrent map cannot hold. Implementations use this to make their backing map + * safe for concurrent access without mutating the caller's map. + * @param source map to copy, may be null + * @return a mutable, thread-safe copy + * @since 3.23.0 + */ + static Map toConcurrentMap(Map source) { + Map result = new ConcurrentHashMap<>(); + if (source != null) { + source.forEach((key, value) -> { + if (key != null && value != null) { + result.put(key, value); + } + }); + } + return result; + } + } diff --git a/src/main/java/world/bentobox/bentobox/database/objects/Island.java b/src/main/java/world/bentobox/bentobox/database/objects/Island.java index aa788e893..ccc884cfb 100644 --- a/src/main/java/world/bentobox/bentobox/database/objects/Island.java +++ b/src/main/java/world/bentobox/bentobox/database/objects/Island.java @@ -13,7 +13,6 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; @@ -293,7 +292,7 @@ public Island(Island island) { this.maxHomes = island.getMaxHomes(); this.maxMembers = new HashMap<>(island.getMaxMembers()); this.members.putAll(island.getMembers()); - this.metaData = island.getMetaData().map(Island::toConcurrentMap).orElse(null); + this.metaData = island.getMetaData().map(MetaDataAble::toConcurrentMap).orElse(null); this.name = island.getName(); this.owner = island.getOwner(); this.protectionRange = island.getProtectionRange(); @@ -1756,36 +1755,18 @@ public void setReserved(boolean reserved) { @Override public Optional> getMetaData() { if (!(metaData instanceof ConcurrentMap)) { - metaData = toConcurrentMap(metaData); + metaData = MetaDataAble.toConcurrentMap(metaData); } return Optional.of(metaData); } - /** - * Copies the given map into a new {@link ConcurrentHashMap}, dropping null keys and values, - * which a concurrent map cannot hold. - * @param source map to copy, may be null - * @return a mutable, thread-safe copy - */ - private static Map toConcurrentMap(Map source) { - Map result = new ConcurrentHashMap<>(); - if (source != null) { - source.forEach((key, value) -> { - if (key != null && value != null) { - result.put(key, value); - } - }); - } - return result; - } - /** * @param metaData the metaData to set * @since 1.15.4 */ @Override public void setMetaData(Map metaData) { - this.metaData = metaData == null ? null : toConcurrentMap(metaData); + this.metaData = metaData == null ? null : MetaDataAble.toConcurrentMap(metaData); setChanged(); } diff --git a/src/main/java/world/bentobox/bentobox/database/objects/Players.java b/src/main/java/world/bentobox/bentobox/database/objects/Players.java index 835675e90..410a7eac7 100644 --- a/src/main/java/world/bentobox/bentobox/database/objects/Players.java +++ b/src/main/java/world/bentobox/bentobox/database/objects/Players.java @@ -6,7 +6,6 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import org.bukkit.Bukkit; @@ -254,7 +253,7 @@ public void addToPendingKick(World world) *

* The map may arrive as {@code null} (new player), as an immutable or Gson-created map * (deserialization), or as a plain {@link HashMap}. All of these are copied once into a - * {@link ConcurrentHashMap}. Metadata is read by many addons on hot paths such as + * {@link java.util.concurrent.ConcurrentHashMap}. Metadata is read by many addons on hot paths such as * {@code PlayerMoveEvent} and can be touched from other threads, so the backing map must * tolerate concurrent access without corrupting itself. * @return the metaData @@ -264,29 +263,11 @@ public void addToPendingKick(World world) @Override public Optional> getMetaData() { if (!(metaData instanceof ConcurrentMap)) { - metaData = toConcurrentMap(metaData); + metaData = MetaDataAble.toConcurrentMap(metaData); } return Optional.of(metaData); } - /** - * Copies the given map into a new {@link ConcurrentHashMap}, dropping null keys and values, - * which a concurrent map cannot hold. - * @param source map to copy, may be null - * @return a mutable, thread-safe copy - */ - private static Map toConcurrentMap(Map source) { - Map result = new ConcurrentHashMap<>(); - if (source != null) { - source.forEach((key, value) -> { - if (key != null && value != null) { - result.put(key, value); - } - }); - } - return result; - } - /** * Sets the player's metadata. The map is copied into a thread-safe map, so the caller's map * is never mutated and immutable maps are accepted. @@ -296,7 +277,7 @@ private static Map toConcurrentMap(Map metaData) { - this.metaData = metaData == null ? null : toConcurrentMap(metaData); + this.metaData = metaData == null ? null : MetaDataAble.toConcurrentMap(metaData); } /**