diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistence.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistence.java index 35cefb48..f07e82cd 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistence.java +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistence.java @@ -70,13 +70,10 @@ public class LimboPersistence implements SettingsDependent { @Override public void reload(Settings settings) { LimboPersistenceType persistenceType = settings.getProperty(LimboSettings.LIMBO_PERSISTENCE_TYPE); - if (handler == null || handler.getType() != persistenceType) { - // If we're changing from an existing handler, output a quick hint that nothing is converted. - if (handler != null) { - ConsoleLogger.info("Limbo persistence type has changed! Note that the data is not converted."); - } - - handler = handlerFactory.newInstance(persistenceType.getImplementationClass()); + // If we're changing from an existing handler, output a quick hint that nothing is converted. + if (handler != null && handler.getType() != persistenceType) { + ConsoleLogger.info("Limbo persistence type has changed! Note that the data is not converted."); } + handler = handlerFactory.newInstance(persistenceType.getImplementationClass()); } } diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceType.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceType.java index c998f95a..68b4611b 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceType.java +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceType.java @@ -5,12 +5,16 @@ package fr.xephi.authme.data.limbo.persistence; */ public enum LimboPersistenceType { + /** Store each LimboPlayer in a separate file. */ INDIVIDUAL_FILES(SeparateFilePersistenceHandler.class), + /** Store all LimboPlayers in the same file. */ SINGLE_FILE(SingleFilePersistenceHandler.class), + /** Distribute LimboPlayers by segments into a set number of files. */ SEGMENT_FILES(SegmentFilesPersistenceHolder.class), + /** No persistence to disk. */ DISABLED(NoOpPersistenceHandler.class); private final Class implementationClass; diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentConfiguration.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentConfiguration.java index d2f0d202..5053ba52 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentConfiguration.java +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentConfiguration.java @@ -38,8 +38,8 @@ public enum SegmentConfiguration { /** 1. */ ONE(1, 1), - /** 2. */ - TWO(2, 1), + ///** 2. */ + //TWO(2, 1), /** 4. */ FOUR(4, 1), diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentFilesPersistenceHolder.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentFilesPersistenceHolder.java index 24751fe0..e786ca48 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentFilesPersistenceHolder.java +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentFilesPersistenceHolder.java @@ -17,11 +17,14 @@ import org.bukkit.entity.Player; import javax.inject.Inject; import java.io.File; import java.io.FileWriter; -import java.io.IOException; import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Optional; /** * Persistence handler for LimboPlayer objects by distributing the objects to store @@ -51,7 +54,8 @@ class SegmentFilesPersistenceHolder implements LimboPersistenceHandler { segmentNameBuilder = new SegmentNameBuilder(settings.getProperty(LimboSettings.SEGMENT_DISTRIBUTION)); - // TODO #1125: Check for other segment files and attempt to convert? + convertOldDataToCurrentSegmentScheme(); + deleteEmptyFiles(); } @Override @@ -94,16 +98,16 @@ class SegmentFilesPersistenceHolder implements LimboPersistenceHandler { } } + @Override + public LimboPersistenceType getType() { + return LimboPersistenceType.SEGMENT_FILES; + } + private void saveEntries(Map entries, File file) { - if (entries.isEmpty()) { - // TODO #1125: Probably should do a sweep of empty files on startup / shutdown, but not all the time - FileUtils.delete(file); - } else { - try (FileWriter fw = new FileWriter(file)) { - gson.toJson(entries, fw); - } catch (IOException e) { - ConsoleLogger.logException("Could not write to '" + file + "':", e); - } + try (FileWriter fw = new FileWriter(file)) { + gson.toJson(entries, fw); + } catch (Exception e) { + ConsoleLogger.logException("Could not write to '" + file + "':", e); } } @@ -114,7 +118,7 @@ class SegmentFilesPersistenceHolder implements LimboPersistenceHandler { try { return gson.fromJson(Files.toString(file, StandardCharsets.UTF_8), LIMBO_MAP_TYPE); - } catch (IOException e) { + } catch (Exception e) { ConsoleLogger.logException("Failed reading '" + file + "':", e); } return null; @@ -125,8 +129,98 @@ class SegmentFilesPersistenceHolder implements LimboPersistenceHandler { return new File(cacheFolder, segment + "-limbo.json"); } - @Override - public LimboPersistenceType getType() { - return LimboPersistenceType.SINGLE_FILE; + /** + * Loads segment files in the cache folder that don't correspond to the current segmenting scheme + * and migrates the data into files of the current segments. This allows a player to change the + * segment size without any loss of data. + */ + private void convertOldDataToCurrentSegmentScheme() { + String currentPrefix = segmentNameBuilder.getPrefix(); + File[] files = listFiles(cacheFolder); + Map allLimboPlayers = new HashMap<>(); + List migratedFiles = new ArrayList<>(); + + for (File file : files) { + if (isLimboJsonFile(file) && !file.getName().startsWith(currentPrefix)) { + Map data = readLimboPlayers(file); + if (data != null) { + allLimboPlayers.putAll(data); + migratedFiles.add(file); + } + } + } + + if (!allLimboPlayers.isEmpty()) { + saveToNewSegments(allLimboPlayers); + migratedFiles.forEach(FileUtils::delete); + } + } + + /** + * Saves the LimboPlayer data read from old segmenting schemes into the current segmenting scheme. + * + * @param limbosFromOldSegments the limbo players to store into the current segment files + */ + private void saveToNewSegments(Map limbosFromOldSegments) { + Map> limboBySegment = groupBySegment(limbosFromOldSegments); + + ConsoleLogger.info("Saving " + limbosFromOldSegments.size() + " LimboPlayers from old segments into " + + limboBySegment.size() + " current segments"); + for (Map.Entry> entry : limboBySegment.entrySet()) { + File file = new File(cacheFolder, entry.getKey() + "-limbo.json"); + Map limbosToSave = Optional.ofNullable(readLimboPlayers(file)) + .orElseGet(HashMap::new); + limbosToSave.putAll(entry.getValue()); + saveEntries(limbosToSave, file); + } + } + + /** + * Converts a Map of UUID to LimboPlayers to a 2-dimensional Map of LimboPlayers by segment ID and UUID. + * {@code Map(uuid -> LimboPlayer) to Map(segment -> Map(uuid -> LimboPlayer))} + * + * @param readLimboPlayers the limbo players to order by segment + * @return limbo players ordered by segment ID and associated player UUID + */ + private Map> groupBySegment(Map readLimboPlayers) { + Map> limboBySegment = new HashMap<>(); + for (Map.Entry entry : readLimboPlayers.entrySet()) { + String segmentId = segmentNameBuilder.createSegmentName(entry.getKey()); + limboBySegment.computeIfAbsent(segmentId, s -> new HashMap<>()) + .put(entry.getKey(), entry.getValue()); + } + return limboBySegment; + } + + /** + * Deletes files from the current segmenting scheme that are empty. + */ + private void deleteEmptyFiles() { + File[] files = listFiles(cacheFolder); + + long deletedFiles = Arrays.stream(files) + // typically the size is 2 because there's an empty JSON map: {} + .filter(f -> isLimboJsonFile(f) && f.length() < 3) + .peek(FileUtils::delete) + .count(); + ConsoleLogger.debug("Limbo: Deleted {0} empty segment files", deletedFiles); + } + + /** + * @param file the file to check + * @return true if it is a segment file storing Limbo data, false otherwise + */ + private static boolean isLimboJsonFile(File file) { + String name = file.getName(); + return name.startsWith("seg") && name.endsWith("-limbo.json"); + } + + private static File[] listFiles(File folder) { + File[] files = folder.listFiles(); + if (files == null) { + ConsoleLogger.warning("Could not get files of '" + folder + "'"); + return new File[0]; + } + return files; } } diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentNameBuilder.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentNameBuilder.java index df517fba..52e1141b 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentNameBuilder.java +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/SegmentNameBuilder.java @@ -25,15 +25,28 @@ class SegmentNameBuilder { this.charToSegmentChar = buildCharMap(distribution); } + /** + * Returns the segment ID for the given UUID. + * + * @param uuid the player's uuid to get the segment for + * @return id the uuid belongs to + */ String createSegmentName(String uuid) { if (distribution == 16) { return prefix + uuid.substring(0, length); } else { - return prefix + createSegmentName(uuid.substring(0, length).toCharArray()); + return prefix + buildSegmentName(uuid.substring(0, length).toCharArray()); } } - private String createSegmentName(char[] chars) { + /** + * @return the prefix used for the current segment configuration + */ + String getPrefix() { + return prefix; + } + + private String buildSegmentName(char[] chars) { if (chars.length == 1) { return String.valueOf(charToSegmentChar.get(chars[0])); } diff --git a/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java b/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java index d8d7104e..a48db6b3 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java @@ -34,14 +34,14 @@ public final class LimboSettings implements SettingsHolder { "This setting only affects SEGMENT_FILES persistence. The segment file", "persistence attempts to reduce the number of files by distributing players into various", "buckets based on their UUID. This setting defines into how many files the players should", - "be distributed. Possible values: ONE, TWO, FOUR, EIGHT, SIXTEEN, THIRTY_TWO, SIXTY_FOUR,", + "be distributed. Possible values: ONE, FOUR, EIGHT, SIXTEEN, THIRTY_TWO, SIXTY_FOUR,", "ONE_TWENTY for 128, TWO_FIFTY for 256.", "For example, if you expect 100 non-logged in players, setting to SIXTEEN will average", "6.25 players per file (100 / 16). If you set to ONE, like persistence SINGLE_FILE, only", "one file will be used. Contrary to SINGLE_FILE, it won't keep the entries in cache, which", "may deliver different results in terms of performance.", - "Note: if you change this setting you lose all stored LimboPlayer data because the", - "distribution of players will be different." + "Note: if you change this setting all data will be migrated. If you have a lot of data,", + "change this setting only on server restart, not with /authme reload." }) public static final Property SEGMENT_DISTRIBUTION = newProperty(SegmentConfiguration.class, "limbo.persistence.segmentDistribution", SegmentConfiguration.SIXTEEN); diff --git a/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerMatchers.java b/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerMatchers.java new file mode 100644 index 00000000..4c4793a3 --- /dev/null +++ b/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerMatchers.java @@ -0,0 +1,112 @@ +package fr.xephi.authme.data.limbo; + +import org.bukkit.Location; +import org.bukkit.World; +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.hamcrest.TypeSafeMatcher; + +import static java.lang.String.format; + +/** + * Contains matchers for LimboPlayer. + */ +public final class LimboPlayerMatchers { + + private LimboPlayerMatchers() { + } + + public static Matcher isLimbo(LimboPlayer limbo) { + return isLimbo(limbo.isOperator(), limbo.getGroup(), limbo.isCanFly(), + limbo.getWalkSpeed(), limbo.getFlySpeed()); + } + + public static Matcher isLimbo(boolean isOp, String group, boolean canFly, + float walkSpeed, float flySpeed) { + return new TypeSafeMatcher() { + @Override + protected boolean matchesSafely(LimboPlayer item) { + return item.isOperator() == isOp && item.getGroup().equals(group) && item.isCanFly() == canFly + && walkSpeed == item.getWalkSpeed() && flySpeed == item.getFlySpeed(); + } + + @Override + public void describeTo(Description description) { + description.appendText(format("Limbo with isOp=%s, group=%s, canFly=%s, walkSpeed=%f, flySpeed=%f", + isOp, group, canFly, walkSpeed, flySpeed)); + } + + @Override + public void describeMismatchSafely(LimboPlayer item, Description description) { + description.appendText(format("Limbo with isOp=%s, group=%s, canFly=%s, walkSpeed=%f, flySpeed=%f", + item.isOperator(), item.getGroup(), item.isCanFly(), item.getWalkSpeed(), item.getFlySpeed())); + } + }; + } + + public static Matcher hasLocation(String world, double x, double y, double z) { + return new TypeSafeMatcher() { + @Override + protected boolean matchesSafely(LimboPlayer item) { + Location location = item.getLocation(); + return location.getWorld().getName().equals(world) + && location.getX() == x && location.getY() == y && location.getZ() == z; + } + + @Override + public void describeTo(Description description) { + description.appendText(format("Limbo with location: world=%s, x=%f, y=%f, z=%f", + world, x, y, z)); + } + + @Override + public void describeMismatchSafely(LimboPlayer item, Description description) { + Location location = item.getLocation(); + if (location == null) { + description.appendText("Limbo with location = null"); + } else { + description.appendText(format("Limbo with location: world=%s, x=%f, y=%f, z=%f", + location.getWorld().getName(), location.getX(), location.getY(), location.getZ())); + } + } + }; + } + + public static Matcher hasLocation(World world, double x, double y, double z) { + return hasLocation(world.getName(), x, y, z); + } + + public static Matcher hasLocation(String world, double x, double y, double z, float yaw, float pitch) { + return new TypeSafeMatcher() { + @Override + protected boolean matchesSafely(LimboPlayer item) { + Location location = item.getLocation(); + return hasLocation(location.getWorld(), location.getX(), location.getY(), location.getZ()).matches(item) + && location.getYaw() == yaw && location.getPitch() == pitch; + } + + @Override + public void describeTo(Description description) { + description.appendText(format("Limbo with location: world=%s, x=%f, y=%f, z=%f, yaw=%f, pitch=%f", + world, x, y, z, yaw, pitch)); + } + + @Override + public void describeMismatchSafely(LimboPlayer item, Description description) { + Location location = item.getLocation(); + if (location == null) { + description.appendText("Limbo with location = null"); + } else { + description.appendText(format("Limbo with location: world=%s, x=%f, y=%f, z=%f, yaw=%f, pitch=%f", + location.getWorld().getName(), location.getX(), location.getY(), location.getZ(), + location.getYaw(), location.getPitch())); + } + } + }; + } + + public static Matcher hasLocation(Location location) { + return hasLocation(location.getWorld().getName(), location.getX(), location.getY(), location.getZ(), + location.getYaw(), location.getPitch()); + } +} diff --git a/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTest.java b/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTest.java index 3511771d..d253869f 100644 --- a/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTest.java +++ b/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTest.java @@ -15,7 +15,6 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.Mockito; import java.util.logging.Logger; @@ -33,7 +32,6 @@ import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.hamcrest.MockitoHamcrest.argThat; /** @@ -103,21 +101,6 @@ public class LimboPersistenceTest { assertThat(getHandler(), instanceOf(LimboPersistenceType.INDIVIDUAL_FILES.getImplementationClass())); } - @Test - public void shouldNotReinitializeHandlerForSameType() { - // given - LimboPersistenceHandler currentHandler = getHandler(); - Mockito.reset(handlerFactory); - given(currentHandler.getType()).willCallRealMethod(); - - // when - limboPersistence.reload(settings); - - // then - verifyZeroInteractions(handlerFactory); - assertThat(currentHandler, sameInstance(getHandler())); - } - @Test public void shouldHandleExceptionWhenGettingLimbo() { // given diff --git a/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTypeTest.java b/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTypeTest.java new file mode 100644 index 00000000..248ab9c1 --- /dev/null +++ b/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTypeTest.java @@ -0,0 +1,48 @@ +package fr.xephi.authme.data.limbo.persistence; + +import org.junit.Test; + +import java.util.HashSet; +import java.util.Set; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Test for {@link LimboPersistenceType}. + */ +public class LimboPersistenceTypeTest { + + @Test + public void shouldHaveUniqueImplementationClasses() { + // given + Set> classes = new HashSet<>(); + + // when / then + for (LimboPersistenceType persistenceType : LimboPersistenceType.values()) { + if (!classes.add(persistenceType.getImplementationClass())) { + fail("Implementation class '" + persistenceType.getImplementationClass() + "' from '" + + persistenceType + "' already encountered previously"); + } + } + } + + @Test + public void shouldHaveTypeReturnedFromImplementationClass() { + for (LimboPersistenceType persistenceType : LimboPersistenceType.values()) { + // given + LimboPersistenceHandler implementationMock = mock(persistenceType.getImplementationClass()); + given(implementationMock.getType()).willCallRealMethod(); + + // when + LimboPersistenceType returnedType = implementationMock.getType(); + + // then + assertThat(returnedType, equalTo(persistenceType)); + } + } + +} diff --git a/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentConfigurationTest.java b/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentConfigurationTest.java index f5e5e124..34d23723 100644 --- a/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentConfigurationTest.java +++ b/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentConfigurationTest.java @@ -32,16 +32,16 @@ public class SegmentConfigurationTest { @Test public void shouldHaveDifferentSegmentSizes() { // given - Set sizes = new HashSet<>(); + Set segmentTotals = new HashSet<>(); // when / then for (SegmentConfiguration entry : SegmentConfiguration.values()) { - int segSize = (int) Math.pow(entry.getDistribution(), entry.getLength()); + int totalSegments = entry.getTotalSegments(); assertThat(entry + " must have a positive segment size", - segSize, greaterThan(0)); + totalSegments, greaterThan(0)); - assertThat(entry + " has a segment size that was already encountered (" + segSize + ")", - sizes.add(segSize), equalTo(true)); + assertThat(entry + " has a segment total that was already encountered (" + totalSegments + ")", + segmentTotals.add(totalSegments), equalTo(true)); } } } diff --git a/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentFilesPersistenceHolderTest.java b/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentFilesPersistenceHolderTest.java new file mode 100644 index 00000000..064fc9c3 --- /dev/null +++ b/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentFilesPersistenceHolderTest.java @@ -0,0 +1,205 @@ +package fr.xephi.authme.data.limbo.persistence; + +import ch.jalu.injector.testing.BeforeInjecting; +import ch.jalu.injector.testing.DelayedInjectionRunner; +import ch.jalu.injector.testing.InjectDelayed; +import com.google.common.io.Files; +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.limbo.LimboPlayer; +import fr.xephi.authme.initialization.DataFolder; +import fr.xephi.authme.service.BukkitService; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.LimboSettings; +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Player; +import org.hamcrest.Matcher; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.mockito.Mock; + +import java.io.File; +import java.io.IOException; +import java.util.UUID; + +import static fr.xephi.authme.data.limbo.LimboPlayerMatchers.hasLocation; +import static fr.xephi.authme.data.limbo.LimboPlayerMatchers.isLimbo; +import static java.util.UUID.fromString; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.arrayContainingInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Test for {@link SegmentFilesPersistenceHolder}. + */ +@RunWith(DelayedInjectionRunner.class) +public class SegmentFilesPersistenceHolderTest { + + /** Player is in seg32-10110 and should be migrated into seg16-f. */ + private static final UUID MIGRATED_UUID = fromString("f6a97c88-7c8f-c12e-4931-6206d4ca067d"); + private static final Matcher MIGRATED_LIMBO_MATCHER = + isLimbo(false, "noob", true, 0.2f, 0.1f); + + /** Existing player in seg16-f. */ + private static final UUID UUID_FAB69 = fromString("fab69c88-2cd0-1fed-f00d-dead14ca067d"); + private static final Matcher FAB69_MATCHER = + isLimbo(false, "", false, 0.2f, 0.1f); + + /** Player in seg16-8. */ + private static final UUID UUID_STAFF = fromString("88897c88-7c8f-c12e-4931-6206d4ca067d"); + private static final Matcher STAFF_MATCHER = + isLimbo(true, "staff", false, 0.3f, 0.1f); + + /** Player in seg16-8. */ + private static final UUID UUID_8C679 = fromString("8c679491-1234-abcd-9102-1fa6e0cc3f81"); + private static final Matcher SC679_MATCHER = + isLimbo(false, "primary", true, 0.1f, 0.0f); + + /** UUID for which no data is stored (belongs to a segment file that does not exist, seg16-4). */ + private static final UUID UNKNOWN_UUID = fromString("42d1cc0b-8f12-d04a-e7ba-a067d05cdc39"); + + /** UUID for which no data is stored (belongs to an existing segment file: seg16-8). */ + private static final UUID UNKNOWN_UUID2 = fromString("84d1cc0b-8f12-d04a-e7ba-a067d05cdc39"); + + + @InjectDelayed + private SegmentFilesPersistenceHolder persistenceHandler; + + @Mock + private Settings settings; + @Mock + private BukkitService bukkitService; + @DataFolder + private File dataFolder; + private File playerDataFolder; + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @BeforeClass + public static void initLogger() { + TestHelper.setupLogger(); + } + + @BeforeInjecting + public void setUpClasses() throws IOException { + given(settings.getProperty(LimboSettings.SEGMENT_DISTRIBUTION)).willReturn(SegmentConfiguration.SIXTEEN); + dataFolder = temporaryFolder.newFolder(); + playerDataFolder = new File(dataFolder, "playerdata"); + playerDataFolder.mkdir(); + + File limboFilesFolder = new File("src/test/resources/fr/xephi/authme/data/limbo"); + for (File file : limboFilesFolder.listFiles()) { + File from = new File(playerDataFolder, file.getName()); + Files.copy(file, from); + } + + given(bukkitService.getWorld(anyString())) + .willAnswer(invocation -> { + World world = mock(World.class); + given(world.getName()).willReturn(invocation.getArgument(0)); + return world; + }); + } + + // Note ljacqu 20170314: These tests are a little slow to set up; therefore we sometimes + // test things in one test that would traditionally belong into two separate tests + + @Test + public void shouldMigrateOldSegmentFilesOnStartup() { + // Ensure that only the files of the current segmenting scheme remain + assertThat(playerDataFolder.list(), arrayContainingInAnyOrder("seg16-8-limbo.json", "seg16-f-limbo.json")); + + // Check that the expected limbo players can be read + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(MIGRATED_UUID)), MIGRATED_LIMBO_MATCHER); + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UUID_FAB69)), FAB69_MATCHER); + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UUID_STAFF)), STAFF_MATCHER); + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UUID_8C679)), SC679_MATCHER); + + // Check that unknown players are null (whose segment file exists and does not exist) + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UNKNOWN_UUID)), nullValue()); + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UNKNOWN_UUID2)), nullValue()); + } + + @Test + public void shouldRemovePlayer() { + // given + Player playerToRemove = mockPlayerWithUuid(UUID_STAFF); + Player unknownPlayerToRemove = mockPlayerWithUuid(UNKNOWN_UUID); + + // when + persistenceHandler.removeLimboPlayer(playerToRemove); + persistenceHandler.removeLimboPlayer(unknownPlayerToRemove); + + // then + assertThat(persistenceHandler.getLimboPlayer(playerToRemove), nullValue()); + assertThat(persistenceHandler.getLimboPlayer(unknownPlayerToRemove), nullValue()); + // Player in same segment should still exist... + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UUID_8C679)), SC679_MATCHER); + + // Check that we didn't create seg16-4 by deleting UNKNOWN_UUID. + assertThat(playerDataFolder.list(), arrayContainingInAnyOrder("seg16-8-limbo.json", "seg16-f-limbo.json")); + } + + @Test + public void shouldAddPlayer() { + // given + Player uuidToAdd1 = mockPlayerWithUuid(UNKNOWN_UUID); + Location location1 = new Location(mockWorldWithName("1world"), 120, 60, -80, 0.42345f, 120.32f); + LimboPlayer limbo1 = new LimboPlayer(location1, false, "group-1", true, 0.1f, 0.2f); + Player uuidToAdd2 = mockPlayerWithUuid(UNKNOWN_UUID2); + Location location2 = new Location(mockWorldWithName("2world"), -40, 20, 33, 4.235f, 8.32299f); + LimboPlayer limbo2 = new LimboPlayer(location2, true, "", false, 0.0f, 0.25f); + + // when + persistenceHandler.saveLimboPlayer(uuidToAdd1, limbo1); + persistenceHandler.saveLimboPlayer(uuidToAdd2, limbo2); + + // then + LimboPlayer addedPlayer1 = persistenceHandler.getLimboPlayer(uuidToAdd1); + assertThat(addedPlayer1, isLimbo(limbo1)); + assertThat(addedPlayer1, hasLocation(location1)); + LimboPlayer addedPlayer2 = persistenceHandler.getLimboPlayer(uuidToAdd2); + assertThat(addedPlayer2, isLimbo(limbo2)); + assertThat(addedPlayer2, hasLocation(location2)); + + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(MIGRATED_UUID)), MIGRATED_LIMBO_MATCHER); + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UUID_FAB69)), FAB69_MATCHER); + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UUID_STAFF)), STAFF_MATCHER); + assertThat(persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UUID_8C679)), SC679_MATCHER); + } + + @Test + public void shouldHandleReadErrorGracefully() throws IOException { + // given + // assumption + File invalidFile = new File(playerDataFolder, "seg16-4-limbo.json"); + assertThat(invalidFile.exists(), equalTo(false)); + Files.write("not valid json".getBytes(), invalidFile); + + // when + LimboPlayer result = persistenceHandler.getLimboPlayer(mockPlayerWithUuid(UNKNOWN_UUID)); + + // then + assertThat(result, nullValue()); + } + + private static Player mockPlayerWithUuid(UUID uuid) { + Player player = mock(Player.class); + given(player.getUniqueId()).willReturn(uuid); + return player; + } + + private static World mockWorldWithName(String name) { + World world = mock(World.class); + given(world.getName()).willReturn(name); + return world; + } +} diff --git a/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentNameBuilderTest.java b/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentNameBuilderTest.java index 41c3b0ff..64843db8 100644 --- a/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentNameBuilderTest.java +++ b/src/test/java/fr/xephi/authme/data/limbo/persistence/SegmentNameBuilderTest.java @@ -11,7 +11,6 @@ import static fr.xephi.authme.data.limbo.persistence.SegmentConfiguration.ONE; import static fr.xephi.authme.data.limbo.persistence.SegmentConfiguration.SIXTEEN; import static fr.xephi.authme.data.limbo.persistence.SegmentConfiguration.SIXTY_FOUR; import static fr.xephi.authme.data.limbo.persistence.SegmentConfiguration.THIRTY_TWO; -import static fr.xephi.authme.data.limbo.persistence.SegmentConfiguration.TWO; import static fr.xephi.authme.data.limbo.persistence.SegmentConfiguration.TWO_FIFTY; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; @@ -73,17 +72,6 @@ public class SegmentNameBuilderTest { assertThat(nameBuilder.createSegmentName("329"), equalTo("seg1-0")); } - @Test - public void shouldCreateTwoSegments() { - // given - SegmentNameBuilder nameBuilder = new SegmentNameBuilder(TWO); - - // when / then - assertThat(nameBuilder.createSegmentName("f6c"), equalTo("seg2-1")); - assertThat(nameBuilder.createSegmentName("29f"), equalTo("seg2-0")); - assertThat(nameBuilder.createSegmentName("983"), equalTo("seg2-1")); - } - @Test public void shouldCreateFourSegments() { // given diff --git a/src/test/resources/fr/xephi/authme/data/limbo/seg16-8-limbo.json b/src/test/resources/fr/xephi/authme/data/limbo/seg16-8-limbo.json new file mode 100644 index 00000000..84da8a26 --- /dev/null +++ b/src/test/resources/fr/xephi/authme/data/limbo/seg16-8-limbo.json @@ -0,0 +1,32 @@ +{ + "88897c88-7c8f-c12e-4931-6206d4ca067d": { + "location": { + "world": "world", + "x": -196.69999998807907, + "y": 67.0, + "z": 5.699999988079071, + "yaw": 222.14977, + "pitch": 10.649977 + }, + "group": "staff", + "operator": true, + "can-fly": false, + "walk-speed": 0.3, + "fly-speed": 0.1 + }, + "8c679491-1234-abcd-9102-1fa6e0cc3f81": { + "location": { + "world": "nether", + "x": 300.12345, + "y": 42.3, + "z": -72.482749988079071, + "yaw": 100.27788, + "pitch": 4.242111 + }, + "group": "primary", + "operator": false, + "can-fly": true, + "walk-speed": 0.1, + "fly-speed": 0.0 + } +} diff --git a/src/test/resources/fr/xephi/authme/data/limbo/seg16-f-limbo.json b/src/test/resources/fr/xephi/authme/data/limbo/seg16-f-limbo.json new file mode 100644 index 00000000..9f256262 --- /dev/null +++ b/src/test/resources/fr/xephi/authme/data/limbo/seg16-f-limbo.json @@ -0,0 +1,17 @@ +{ + "fab69c88-2cd0-1fed-f00d-dead14ca067d": { + "location": { + "world": "world", + "x": -196.69999998807907, + "y": 67.0, + "z": 5.699999988079071, + "yaw": 222.14977, + "pitch": 10.649977 + }, + "group": "", + "operator": false, + "can-fly": false, + "walk-speed": 0.2, + "fly-speed": 0.1 + } +} diff --git a/src/test/resources/fr/xephi/authme/data/limbo/seg32-01011-limbo.json b/src/test/resources/fr/xephi/authme/data/limbo/seg32-01011-limbo.json new file mode 100644 index 00000000..0967ef42 --- /dev/null +++ b/src/test/resources/fr/xephi/authme/data/limbo/seg32-01011-limbo.json @@ -0,0 +1 @@ +{} diff --git a/src/test/resources/fr/xephi/authme/data/limbo/seg32-10110-limbo.json b/src/test/resources/fr/xephi/authme/data/limbo/seg32-10110-limbo.json new file mode 100644 index 00000000..b50fb8bd --- /dev/null +++ b/src/test/resources/fr/xephi/authme/data/limbo/seg32-10110-limbo.json @@ -0,0 +1,17 @@ +{ + "f6a97c88-7c8f-c12e-4931-6206d4ca067d": { + "location": { + "world": "lobby", + "x": -120.31415, + "y": 25.0, + "z": -80.71234, + "yaw": 22.14977, + "pitch": 40.649977 + }, + "group": "noob", + "operator": false, + "can-fly": true, + "walk-speed": 0.2, + "fly-speed": 0.1 + } +}