Avoids loading islands into cache unless they are needed. (#2373)

* Avoids loading islands into cache unless they are needed.

* Adjust methods that were calling all islands

When we cached all island, this was an inexpensive call but not now. The
methods remain but pull from the database directly. The use of them were
changed to be player specific.
This commit is contained in:
tastybento 2024-05-23 21:42:14 -07:00 committed by GitHub
parent 8aba736383
commit e2d9c2ce34
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
24 changed files with 133 additions and 111 deletions

View File

@ -11,10 +11,8 @@ import org.bstats.charts.AdvancedPie;
import org.bstats.charts.SimpleBarChart;
import org.bstats.charts.SimplePie;
import org.bstats.charts.SingleLineChart;
import org.bukkit.Bukkit;
import world.bentobox.bentobox.api.addons.GameModeAddon;
import world.bentobox.bentobox.api.flags.Flag;
/**
* @author Poslovitch

View File

@ -75,12 +75,6 @@ public class IslandSetnameCommand extends CompositeCommand {
name = ChatColor.translateAlternateColorCodes('&', name);
}
// Check if the name doesn't already exist in the gamemode
if (getSettings().isNameUniqueness() && getIslands().nameExists(getWorld(), name)) {
user.sendMessage("commands.island.setname.name-already-exists");
return false;
}
return true;
}

View File

@ -14,7 +14,6 @@ import world.bentobox.bentobox.api.events.team.TeamEvent;
import world.bentobox.bentobox.api.localization.TextVariables;
import world.bentobox.bentobox.api.user.User;
import world.bentobox.bentobox.database.objects.Island;
import world.bentobox.bentobox.managers.IslandsManager;
import world.bentobox.bentobox.managers.RanksManager;
import world.bentobox.bentobox.util.Util;

View File

@ -24,7 +24,6 @@ import org.bukkit.potion.PotionEffectType;
import org.bukkit.potion.PotionType;
import org.jetbrains.annotations.Nullable;
import com.google.common.base.Enums;
import com.meowj.langutils.lang.LanguageHelper;
import world.bentobox.bentobox.BentoBox;

View File

@ -189,7 +189,7 @@ public class JoinLeaveListener implements Listener {
}
private void updateIslandRange(User user) {
plugin.getIslands().getIslands().stream()
plugin.getIslands().getIslands(user.getUniqueId()).stream()
.filter(island -> island.getOwner() != null && island.getOwner().equals(user.getUniqueId()))
.forEach(island -> {
// Check if new owner has a different range permission than the island size
@ -220,8 +220,7 @@ public class JoinLeaveListener implements Listener {
// Remove any coops if all the island players have left
// Go through all the islands this player is a member of, check if all members
// have left, remove coops
plugin.getIslands().getIslands().stream()
plugin.getIslands().getIslands(event.getPlayer().getUniqueId()).stream()
.filter(island -> island.getMembers().containsKey(event.getPlayer().getUniqueId())).forEach(island -> {
// Are there any online players still for this island?
if (Bukkit.getOnlinePlayers().stream().filter(p -> !event.getPlayer().equals(p))

View File

@ -1,6 +1,5 @@
package world.bentobox.bentobox.listeners.flags.protection;
import java.io.IOException;
import java.util.List;
import org.bukkit.Location;
@ -23,7 +22,6 @@ import com.google.common.base.Optional;
import world.bentobox.bentobox.api.flags.FlagListener;
import world.bentobox.bentobox.lists.Flags;
import world.bentobox.bentobox.util.Util;
/**
* Protects islands from visitors blowing things up

View File

@ -18,7 +18,6 @@ import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;
import org.bukkit.Bukkit;
import org.bukkit.ChatColor;
import org.bukkit.Location;
import org.bukkit.Material;
import org.bukkit.Tag;
@ -98,7 +97,7 @@ public class IslandsManager {
this.plugin = plugin;
// Set up the database handler to store and retrieve Island classes
handler = new Database<>(plugin, Island.class);
islandCache = new IslandCache();
islandCache = new IslandCache(handler);
// 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<>();
@ -375,6 +374,17 @@ public class IslandsManager {
return islandCache.getIslands(world, uniqueId);
}
/**
* Gets all the islands for this player in any world where this player has any presence
*
* @param uniqueId user's UUID
* @return List of islands or empty list if none found for user
*/
@NonNull
public List<Island> getIslands(UUID uniqueId) {
return islandCache.getIslands(uniqueId);
}
/**
* Gets all the islands for this player in this world that this player owns.
*
@ -441,7 +451,7 @@ public class IslandsManager {
*/
@NonNull
public Collection<Island> getIslands() {
return islandCache.getIslands();
return handler.loadObjects().stream().toList();
}
/**
@ -455,7 +465,7 @@ public class IslandsManager {
*/
@NonNull
public Collection<Island> getIslands(@NonNull World world) {
return islandCache.getIslands(world);
return handler.loadObjects().stream().filter(i -> world.equals(i.getWorld())).toList();
}
/**
@ -1678,20 +1688,6 @@ public class IslandsManager {
this.saveAll();
}
/**
* Returns whether the specified island custom name exists in this world.
*
* @param world World of the gamemode
* @param name Name of an island
* @return {@code true} if there is an island with the specified name in this
* world, {@code false} otherwise.
* @since 1.7.0
*/
public boolean nameExists(@NonNull World world, @NonNull String name) {
return getIslands(world).stream().map(Island::getName).filter(Objects::nonNull)
.anyMatch(n -> ChatColor.stripColor(n).equals(ChatColor.stripColor(name)));
}
/**
* Is user mid home teleport?
*

View File

@ -21,6 +21,7 @@ import org.eclipse.jdt.annotation.Nullable;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.api.flags.Flag;
import world.bentobox.bentobox.database.Database;
import world.bentobox.bentobox.database.objects.Island;
import world.bentobox.bentobox.managers.RanksManager;
import world.bentobox.bentobox.util.Util;
@ -45,11 +46,13 @@ public class IslandCache {
@NonNull
private final Map<@NonNull World, @NonNull IslandGrid> grids;
private final @NonNull Database<Island> handler;
public IslandCache() {
public IslandCache(@NonNull Database<Island> handler) {
islandsById = new HashMap<>();
islandsByUUID = new HashMap<>();
grids = new HashMap<>();
this.handler = handler;
}
/**
@ -91,7 +94,7 @@ public class IslandCache {
}
/**
* Adds an island to the grid
* Adds an island to the grid, used for new islands
*
* @param island island to add, not null
* @return true if successfully added, false if not
@ -113,14 +116,13 @@ public class IslandCache {
}
/**
* Adds a player's UUID to the look up for islands. Does no checking
* Adds a player's UUID to the look up for islands. Does no checking. The island for this player must have been added beforehand.
*
* @param uuid player's uuid
* @param island island to associate with this uuid. Only one island can be
* associated per world.
*/
public void addPlayer(@NonNull UUID uuid, @NonNull Island island) {
this.setIslandById(island);
this.islandsByUUID.computeIfAbsent(uuid, k -> new HashSet<>()).add(island.getUniqueId());
}
@ -395,7 +397,8 @@ public class IslandCache {
*/
@Nullable
public Island getIslandById(@NonNull String uniqueId) {
return islandsById.get(uniqueId);
// Load from cache or database
return islandsById.computeIfAbsent(uniqueId, handler::loadObject);
}
private Island setIslandById(Island island) {
@ -443,4 +446,13 @@ public class IslandCache {
return islandsById.keySet();
}
/**
* Get a unmodifiable list of all islands this player is involved with
* @param uniqueId player's UUID
* @return list of islands
*/
public @NonNull List<Island> getIslands(UUID uniqueId) {
return islandsByUUID.get(uniqueId).stream().map(this::getIslandById).toList();
}
}

View File

@ -46,7 +46,6 @@ import world.bentobox.bentobox.database.objects.IslandDeletion;
import world.bentobox.bentobox.hooks.ItemsAdderHook;
import world.bentobox.bentobox.hooks.SlimefunHook;
import world.bentobox.bentobox.util.MyBiomeGrid;
import world.bentobox.bentobox.util.Util;
/**
* Regenerates by using a seed world. The seed world is created using the same generator as the game

View File

@ -7,15 +7,20 @@
package world.bentobox.bentobox.panels.customizable;
import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.bukkit.World;
import org.bukkit.event.inventory.ClickType;
import org.bukkit.inventory.ItemStack;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
import java.io.File;
import java.util.*;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.api.addons.GameModeAddon;

View File

@ -7,16 +7,21 @@
package world.bentobox.bentobox.panels.customizable;
import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.apache.commons.lang.WordUtils;
import org.bukkit.Material;
import org.bukkit.event.inventory.ClickType;
import org.bukkit.inventory.ItemStack;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
import java.io.File;
import java.util.*;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.api.addons.GameModeAddon;

View File

@ -7,7 +7,6 @@ import org.bukkit.Material;
import org.bukkit.World;
import org.eclipse.jdt.annotation.NonNull;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.api.flags.Flag.Type;
import world.bentobox.bentobox.api.flags.clicklisteners.WorldToggleClick;
import world.bentobox.bentobox.api.localization.TextVariables;

View File

@ -1,14 +1,18 @@
package world.bentobox.bentobox.util;
import java.net.URL;
import java.util.*;
import java.util.Arrays;
import java.util.Base64;
import java.util.Locale;
import java.util.MissingFormatArgumentException;
import java.util.Optional;
import java.util.UUID;
import org.bukkit.Bukkit;
import org.bukkit.DyeColor;
import org.bukkit.Material;
import org.bukkit.block.banner.Pattern;
import org.bukkit.block.banner.PatternType;
import org.bukkit.inventory.ItemFactory;
import org.bukkit.inventory.ItemStack;
import org.bukkit.inventory.meta.BannerMeta;
import org.bukkit.inventory.meta.Damageable;

View File

@ -4,7 +4,6 @@ import java.util.UUID;
import org.bukkit.Material;
import org.bukkit.inventory.ItemStack;
import org.bukkit.inventory.meta.SkullMeta;
import org.bukkit.profile.PlayerProfile;

View File

@ -42,7 +42,6 @@ import org.powermock.modules.junit4.PowerMockRunner;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.Settings;
import world.bentobox.bentobox.api.commands.CompositeCommand;
import world.bentobox.bentobox.api.flags.Flag.Mode;
import world.bentobox.bentobox.api.localization.TextVariables;
import world.bentobox.bentobox.api.user.User;
import world.bentobox.bentobox.database.objects.Island;

View File

@ -9,7 +9,6 @@ import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
@ -227,29 +226,6 @@ public class IslandSetnameCommandTest {
verify(user).sendMessage("commands.island.setname.name-too-long", TextVariables.NUMBER, "20");
}
/**
* Test method for {@link world.bentobox.bentobox.api.commands.island.IslandSetnameCommand#canExecute(world.bentobox.bentobox.api.user.User, java.lang.String, java.util.List)}.
*/
@Test
public void testIslandSetnameCommandNameNotUnique() {
settings.setNameUniqueness(true);
when(im.nameExists(eq(world), anyString())).thenReturn(true);
assertFalse(isc.canExecute(user, isc.getLabel(), List.of("name2")));
verify(user).sendMessage("commands.island.setname.name-already-exists");
}
/**
* Test method for {@link world.bentobox.bentobox.api.commands.island.IslandSetnameCommand#canExecute(world.bentobox.bentobox.api.user.User, java.lang.String, java.util.List)}.
*/
@Test
public void testIslandSetnameCommandNameApplyColors() {
when(user.hasPermission(anyString())).thenReturn(true);
settings.setNameUniqueness(true);
when(im.nameExists(world, "name§b")).thenReturn(true);
assertFalse(isc.canExecute(user, isc.getLabel(), List.of("name&b")));
verify(user).sendMessage("commands.island.setname.name-already-exists");
}
/**
* Test method for {@link world.bentobox.bentobox.api.commands.island.IslandSetnameCommand#canExecute(world.bentobox.bentobox.api.user.User, java.lang.String, java.util.List)}.
*/

View File

@ -166,6 +166,7 @@ public class JoinLeaveListenerTest {
when(im.getIsland(any(), any(User.class))).thenReturn(island);
when(im.getIsland(any(), any(UUID.class))).thenReturn(island);
when(im.getIslands()).thenReturn(Collections.singletonList(island));
when(im.getIslands(any(UUID.class))).thenReturn(Collections.singletonList(island));
Map<UUID, Integer> memberMap = new HashMap<>();
memberMap.put(uuid, RanksManager.OWNER_RANK);

View File

@ -20,7 +20,6 @@ import org.bukkit.Material;
import org.bukkit.Tag;
import org.bukkit.block.Block;
import org.bukkit.block.BlockFace;
import org.bukkit.block.BlockState;
import org.bukkit.block.Sign;
import org.bukkit.event.Event;
import org.bukkit.event.block.Action;

View File

@ -25,8 +25,6 @@ import org.bukkit.Bukkit;
import org.bukkit.Location;
import org.bukkit.Material;
import org.bukkit.World;
import org.bukkit.damage.DamageSource;
import org.bukkit.damage.DamageType;
import org.bukkit.entity.AreaEffectCloud;
import org.bukkit.entity.Arrow;
import org.bukkit.entity.Creeper;

View File

@ -2,6 +2,8 @@ package world.bentobox.bentobox.managers;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@ -71,10 +73,12 @@ public abstract class RanksManagerBeforeClassTest {
public RanksManager rm;
protected static AbstractDatabaseHandler<Object> h;
protected static Object savedObject;
@SuppressWarnings("unchecked")
@BeforeClass
public static void beforeClass() throws IllegalAccessException, InvocationTargetException, IntrospectionException {
public static void beforeClass() throws IllegalAccessException, InvocationTargetException, IntrospectionException,
InstantiationException, ClassNotFoundException, NoSuchMethodException {
// This has to be done beforeClass otherwise the tests will interfere with each
// other
h = mock(AbstractDatabaseHandler.class);
@ -83,7 +87,27 @@ public abstract class RanksManagerBeforeClassTest {
DatabaseSetup dbSetup = mock(DatabaseSetup.class);
when(DatabaseSetup.getDatabase()).thenReturn(dbSetup);
when(dbSetup.getHandler(any())).thenReturn(h);
when(h.saveObject(any())).thenReturn(CompletableFuture.completedFuture(true));
//when(h.saveObject(any())).thenReturn(CompletableFuture.completedFuture(true));
// Capture the parameter passed to saveObject() and store it in savedObject
doAnswer(invocation -> {
savedObject = invocation.getArgument(0);
return CompletableFuture.completedFuture(true);
}).when(h).saveObject(any());
// Now when loadObject() is called, return the savedObject
when(h.loadObject(any())).thenAnswer(invocation -> savedObject);
// Delete object
doAnswer(invocation -> {
savedObject = null;
return null;
}).when(h).deleteObject(any());
doAnswer(invocation -> {
savedObject = null;
return null;
}).when(h).deleteID(anyString());
}
@Before
@ -95,6 +119,8 @@ public abstract class RanksManagerBeforeClassTest {
when(RanksManager.getInstance()).thenReturn(rm);
when(rm.getRanks()).thenReturn(DEFAULT_RANKS);
when(rm.getRank(anyInt())).thenReturn("");
// Clear savedObject
savedObject = null;
}
@After

View File

@ -6,6 +6,7 @@ import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@ -33,17 +34,17 @@ import com.google.common.collect.ImmutableSet.Builder;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.api.flags.Flag;
import world.bentobox.bentobox.database.Database;
import world.bentobox.bentobox.database.objects.Island;
import world.bentobox.bentobox.managers.IslandWorldManager;
import world.bentobox.bentobox.managers.IslandsManager;
import world.bentobox.bentobox.managers.RanksManagerBeforeClassTest;
import world.bentobox.bentobox.util.Util;
@RunWith(PowerMockRunner.class)
@PrepareForTest({ BentoBox.class, Util.class })
public class IslandCacheTest {
public class IslandCacheTest extends RanksManagerBeforeClassTest {
@Mock
private BentoBox plugin;
@Mock
private World world;
@Mock
@ -60,7 +61,10 @@ public class IslandCacheTest {
private Flag flag;
@Mock
private IslandsManager im;
// Database
Database<Island> db;
@SuppressWarnings("unchecked")
@Before
public void setUp() throws Exception {
// Plugin
@ -74,22 +78,24 @@ public class IslandCacheTest {
when(iwm.inWorld(any(Location.class))).thenReturn(true);
PowerMockito.mockStatic(Util.class);
when(Util.getWorld(Mockito.any())).thenReturn(world);
when(Util.getWorld(any())).thenReturn(world);
// Mock up IslandsManager
when(plugin.getIslands()).thenReturn(im);
// Island
when(island.getWorld()).thenReturn(world);
when(island.getUniqueId()).thenReturn("uniqueId");
@NonNull
String uniqueId = UUID.randomUUID().toString();
when(island.getUniqueId()).thenReturn(uniqueId);
// Location
when(location.getWorld()).thenReturn(world);
when(location.getBlockX()).thenReturn(0);
when(location.getBlockY()).thenReturn(0);
when(location.getBlockZ()).thenReturn(0);
// Island
when(island.getWorld()).thenReturn(world);
when(island.getUniqueId()).thenReturn("uniqueId");
when(island.inIslandSpace(anyInt(), anyInt())).thenReturn(true);
@NonNull
String uniqueId = UUID.randomUUID().toString();
when(island.getUniqueId()).thenReturn(uniqueId);
when(island.getCenter()).thenReturn(location);
when(island.getOwner()).thenReturn(owner);
when(island.isOwned()).thenReturn(true);
@ -102,8 +108,11 @@ public class IslandCacheTest {
when(island.getMinX()).thenReturn(-200);
when(island.getMinZ()).thenReturn(-200);
// database must be mocked here
db = mock(Database.class);
// New cache
ic = new IslandCache();
ic = new IslandCache(db);
}
@After
@ -117,6 +126,7 @@ public class IslandCacheTest {
@Test
public void testAddIsland() {
assertTrue(ic.addIsland(island));
assertEquals(island, ic.getIslandAt(island.getCenter()));
// Check if they are added
assertEquals(island, ic.getIsland(world, owner));
}
@ -126,6 +136,7 @@ public class IslandCacheTest {
*/
@Test
public void testAddPlayer() {
ic.addIsland(island);
UUID playerUUID = UUID.randomUUID();
ic.addPlayer(playerUUID, island);
// Check if they are added
@ -162,7 +173,7 @@ public class IslandCacheTest {
@Test
public void testGetIslandAtLocation() {
// Set coords to be in island space
when(island.inIslandSpace(Mockito.any(Integer.class), Mockito.any(Integer.class))).thenReturn(true);
when(island.inIslandSpace(any(Integer.class), any(Integer.class))).thenReturn(true);
// Set plugin
Util.setPlugin(plugin);
ic.addIsland(island);
@ -179,7 +190,7 @@ public class IslandCacheTest {
assertEquals(island, ic.getIslandAt(location2));
when(island.inIslandSpace(Mockito.any(Integer.class), Mockito.any(Integer.class))).thenReturn(false);
when(island.inIslandSpace(any(Integer.class), any(Integer.class))).thenReturn(false);
assertNull(ic.getIslandAt(location2));
}

View File

@ -9,7 +9,12 @@ import static org.mockito.Mockito.when;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.*;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import org.bukkit.Bukkit;
import org.bukkit.Material;

View File

@ -1,6 +1,23 @@
package world.bentobox.bentobox.panels.customizable;
import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.UUID;
import org.bukkit.Bukkit;
import org.bukkit.Material;
import org.bukkit.World;
@ -23,23 +40,14 @@ import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import org.powermock.reflect.Whitebox;
import java.awt.Panel;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.*;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.api.addons.GameModeAddon;
import world.bentobox.bentobox.api.commands.CompositeCommand;
import world.bentobox.bentobox.api.localization.BentoBoxLocale;
import world.bentobox.bentobox.api.panels.builders.PanelBuilder;
import world.bentobox.bentobox.api.user.User;
import world.bentobox.bentobox.managers.LocalesManager;
import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
/**
* @author tastybento
*

View File

@ -6,13 +6,9 @@ import static org.junit.Assert.assertNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.Arrays;
import java.util.List;
import org.bukkit.Bukkit;
import org.bukkit.Material;
import org.bukkit.UnsafeValues;
@ -22,12 +18,9 @@ import org.bukkit.inventory.meta.BannerMeta;
import org.bukkit.inventory.meta.ItemMeta;
import org.bukkit.inventory.meta.PotionMeta;
import org.bukkit.inventory.meta.SkullMeta;
import org.bukkit.potion.PotionData;
import org.bukkit.potion.PotionType;
import org.bukkit.profile.PlayerProfile;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;