#1125 Limbo persistence: convert old segments, add tests

- On startup / reload the playerdata folder is scanned for old segment files, whose data is migrated before they are deleted - Add tests for segment files persistence holder
2017-03-14 20:46:28 +01:00
parent 9c3baa7f14
commit 84acc4557a
16 changed files with 574 additions and 63 deletions
@@ -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<LimboPlayer> isLimbo(LimboPlayer limbo) {
+        return isLimbo(limbo.isOperator(), limbo.getGroup(), limbo.isCanFly(),
+            limbo.getWalkSpeed(), limbo.getFlySpeed());
+    }
+
+    public static Matcher<LimboPlayer> isLimbo(boolean isOp, String group, boolean canFly,
+                                               float walkSpeed, float flySpeed) {
+        return new TypeSafeMatcher<LimboPlayer>() {
+            @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<LimboPlayer> hasLocation(String world, double x, double y, double z) {
+        return new TypeSafeMatcher<LimboPlayer>() {
+            @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<LimboPlayer> hasLocation(World world, double x, double y, double z) {
+        return hasLocation(world.getName(), x, y, z);
+    }
+
+    public static Matcher<LimboPlayer> hasLocation(String world, double x, double y, double z, float yaw, float pitch) {
+        return new TypeSafeMatcher<LimboPlayer>() {
+            @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<LimboPlayer> hasLocation(Location location) {
+        return hasLocation(location.getWorld().getName(), location.getX(), location.getY(), location.getZ(),
+            location.getYaw(), location.getPitch());
+    }
+}
@@ -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
@@ -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<Class<?>> 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));
+        }
+    }
+
+}
@@ -32,16 +32,16 @@ public class SegmentConfigurationTest {
    @Test
    public void shouldHaveDifferentSegmentSizes() {
        // given
-        Set<Integer> sizes = new HashSet<>();
+        Set<Integer> 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));
        }
    }
 }
@@ -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<LimboPlayer> 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<LimboPlayer> 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<LimboPlayer> 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<LimboPlayer> 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;
+    }
+}
@@ -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
@@ -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
+  }
+}
@@ -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
+  }
+}
@@ -0,0 +1 @@
+{}
@@ -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
+  }
+}