Merge pull request #3142 from Multiverse/ben/mv5/teleport-intercept

Properly implement teleport intercept config option

src/main/java/org/mvplugins/multiverse/core/config/MVCoreConfigNodes.java

@@ -94,7 +94,8 @@ private <N extends Node> N node(N node) {
     final ConfigNode<Boolean> TELEPORT_INTERCEPT = node(ConfigNode.builder("world.teleport-intercept", Boolean.class)
             .comment("")
             .comment("If this is set to true, Multiverse will enforce access permissions for all teleportation,")
-            .comment("including teleportation from other plugins.")
+            .comment("including teleportation from other plugins. You should not disable this unless you are facing")
+            .comment("conflict with another plugin handling teleportation.")
             .defaultValue(true)
             .name("teleport-intercept")
             .build());

src/main/java/org/mvplugins/multiverse/core/listeners/MVPlayerListener.java

@@ -236,20 +236,31 @@ public void playerTeleport(PlayerTeleportEvent event) {
             return;
         }
         Player teleportee = event.getPlayer();
-        CommandSender teleporter;
-        Optional<String> teleporterName = teleportQueue.popFromQueue(teleportee.getName());
-        if (teleporterName.isPresent()) {
+        CommandSender teleporter = null;
+        Option<String> teleporterName = teleportQueue.popFromQueue(teleportee.getName());
+        if (teleporterName.isDefined()) {
             if (teleporterName.equals("CONSOLE")) {
                 Logging.finer("We know the teleporter is the console! Magical!");
                 teleporter = this.server.getConsoleSender();
             } else {
                 teleporter = this.server.getPlayerExact(teleporterName.get());
             }
-        } else {
-            teleporter = teleportee;
+            if (teleporter != null) {
+                Logging.finer("Inferred sender '" + teleporter + "' from name '"
+                        + teleporterName + "', fetched from name '" + teleportee.getName() + "'");
+            }
         }
-        Logging.finer("Inferred sender '" + teleporter + "' from name '"
-                + teleporterName + "', fetched from name '" + teleportee.getName() + "'");
+
+        if (teleporter == null) {
+            if (config.getTeleportIntercept()) {
+                teleporter = teleportee;
+            } else {
+                Logging.finer("Teleport for %s was not initiated by multiverse and " +
+                        "teleport intercept is disabled. Ignoring...", teleportee.getName());
+                return;
+            }
+        }
+
         LoadedMultiverseWorld fromWorld = getWorldManager().getLoadedWorld(event.getFrom().getWorld().getName()).getOrNull();
         LoadedMultiverseWorld toWorld = getWorldManager().getLoadedWorld(event.getTo().getWorld().getName()).getOrNull();
         if (toWorld == null) {
@@ -265,16 +276,17 @@ public void playerTeleport(PlayerTeleportEvent event) {
             return;
         }
 
-        ResultChain entryResult = worldEntryCheckerProvider.forSender(teleporter).canEnterWorld(fromWorld, toWorld)
+        CommandSender finalTeleporter = teleporter;
+        ResultChain entryResult = worldEntryCheckerProvider.forSender(finalTeleporter).canEnterWorld(fromWorld, toWorld)
                 .onSuccessReason(EntryFeeResult.Success.class, reason -> {
                     if (reason == EntryFeeResult.Success.ENOUGH_MONEY) {
-                        economist.payEntryFee((Player) teleporter, toWorld);
+                        economist.payEntryFee((Player) finalTeleporter, toWorld);
                         // Send payment receipt
                     }
                 })
                 .onFailure(results -> {
                     event.setCancelled(true);
-                    getCommandManager().getCommandIssuer(teleporter).sendError(results.getLastResultMessage());
+                    getCommandManager().getCommandIssuer(finalTeleporter).sendError(results.getLastResultMessage());
                 });
 
         Logging.fine("Teleport result: %s", entryResult);

src/main/java/org/mvplugins/multiverse/core/teleportation/AsyncSafetyTeleporter.java

@@ -147,8 +147,11 @@ public AsyncAttempt<Void, TeleportResult.Failure> teleport(
             return AsyncAttempt.failure(TeleportResult.Failure.NULL_LOCATION);
         }
 
-        boolean shouldAddToQueue = teleporter != null && teleportee instanceof Player;
+        boolean shouldAddToQueue = teleportee instanceof Player;
         if (shouldAddToQueue) {
+            if (teleporter == null) {
+                teleporter = teleportee;
+            }
             teleportQueue.addToQueue(teleporter.getName(), teleportee.getName());
         }
 
@@ -164,11 +167,11 @@ private AsyncAttempt<Void, TeleportResult.Failure> doAsyncTeleport(
                     teleportee.getName(), location, exception.getMessage());
             return Attempt.failure(TeleportResult.Failure.TELEPORT_FAILED_EXCEPTION);
         }).mapAttempt(success -> {
-            Logging.finer("Teleported async %s to %s", teleportee.getName(), location);
+            if (shouldRemoveFromQueue) {
+                teleportQueue.popFromQueue(teleportee.getName());
+            }
             if (success) {
-                if (shouldRemoveFromQueue) {
-                    teleportQueue.popFromQueue(teleportee.getName());
-                }
+                Logging.finer("Teleported async %s to %s", teleportee.getName(), location);
                 return Attempt.success(null);
             }
             return Attempt.failure(TeleportResult.Failure.TELEPORT_FAILED);

src/main/java/org/mvplugins/multiverse/core/teleportation/TeleportQueue.java

@@ -2,9 +2,9 @@
 
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Optional;
 
 import com.dumptruckman.minecraft.util.Logging;
+import io.vavr.control.Option;
 import org.jvnet.hk2.annotations.Service;
 
 @Service
@@ -32,12 +32,12 @@ public void addToQueue(String teleporter, String teleportee) {
      * @param playerName The teleported player (the teleportee).
      * @return The player that teleported the other one (the teleporter).
      */
-    public Optional<String> popFromQueue(String playerName) {
+    public Option<String> popFromQueue(String playerName) {
         if (teleportQueue.containsKey(playerName)) {
             String teleportee = teleportQueue.get(playerName);
             teleportQueue.remove(playerName);
-            return Optional.of(teleportee);
+            return Option.of(teleportee);
         }
-        return Optional.empty();
+        return Option.none();
     }
 }

src/test/java/org/mvplugins/multiverse/core/config/ConfigTeleportInterceptTest.kt

@@ -0,0 +1,57 @@
+package org.mvplugins.multiverse.core.config
+
+import org.bukkit.Location
+import org.mockbukkit.mockbukkit.entity.PlayerMock
+import org.mvplugins.multiverse.core.TestWithMockBukkit
+import org.mvplugins.multiverse.core.teleportation.AsyncSafetyTeleporter
+import org.mvplugins.multiverse.core.world.WorldManager
+import org.mvplugins.multiverse.core.world.options.CreateWorldOptions
+import java.lang.AssertionError
+import kotlin.test.*
+
+class ConfigTeleportInterceptTest : TestWithMockBukkit() {
+
+    private lateinit var config: MVCoreConfig
+    private lateinit var safetyTeleporter: AsyncSafetyTeleporter
+    private lateinit var player: PlayerMock
+    private lateinit var location: Location
+
+    @BeforeTest
+    fun setUp() {
+        config = serviceLocator.getActiveService(MVCoreConfig::class.java).takeIf { it != null } ?: run {
+            throw IllegalStateException("MVCoreConfig is not available as a service") }
+
+        safetyTeleporter = serviceLocator.getActiveService(AsyncSafetyTeleporter::class.java).takeIf { it != null } ?: run {
+            throw IllegalStateException("AsyncSafetyTeleporter is not available as a service") }
+
+        val worldManager = serviceLocator.getActiveService(WorldManager::class.java).takeIf { it != null } ?: run {
+            throw IllegalStateException("WorldManager is not available as a service") }
+
+        player = server.addPlayer()
+        worldManager.createWorld(CreateWorldOptions.worldName("world2")).get()
+        location = Location(server.getWorld("world2"), 0.0, 0.0, 0.0)
+        config.enforceAccess = true
+    }
+
+    @Test
+    fun `Generic teleport with teleport intercept enabled`() {
+        config.teleportIntercept = true
+        assertFalse(player.teleport(location))
+        assertEquals("world", player.world.name)
+    }
+
+    @Test
+    fun `Generic teleport with teleport intercept disabled - no access checking`() {
+        config.teleportIntercept = false
+        assertTrue(player.teleport(location))
+        assertEquals("world2", player.world.name)
+    }
+
+    @Test
+    fun `Multiverse API teleport with teleport intercept disabled`() {
+        config.teleportIntercept = false
+        safetyTeleporter.teleport(player, location).toAttempt()
+            .onSuccess (Runnable { throw AssertionError("Teleport should have failed") })
+            .onFailure(Runnable { assertEquals("world", player.world.name) })
+    }
+}