Remove AsyncCommandHelper for new AsyncCommandBuilder.

Helper suffers from race conditions for short-lived tasks, leading to
some poor UX conditions such as errors not propagating to the user
(because the exception handler wasn't attached to the future yet), or
lack of success messages.

This commit replaces that system by a Builder which takes a callable to
begin, and then takes supervisor, delay message, and the success and
failure messages and handlers as parts of the builder. The success and
failure handlers wrap the callable itself before submitting to the
executor so they will always be run. The supervisor and delay are added
as listeners to the future since they aren't required if the task is
sufficiently short-lived (and to maintain compatibility with the classes
which are now in WorldEdit).

The builder also supports Components for success and failure messages,
as well as consumers of the callable's result or exception for better
customization of output, instead of having to rely on adding a callback
to the future.

The future is still returned for certain special usages.

worldguard-core/src/main/java/com/sk89q/worldguard/commands/WorldGuardCommands.java

@@ -29,7 +29,6 @@
 import com.sk89q.minecraft.util.commands.CommandPermissions;
 import com.sk89q.minecraft.util.commands.NestedCommand;
 import com.sk89q.worldedit.WorldEdit;
-import com.sk89q.worldedit.command.util.AsyncCommandHelper;
 import com.sk89q.worldedit.extension.platform.Actor;
 import com.sk89q.worldedit.extension.platform.Capability;
 import com.sk89q.worldedit.util.auth.AuthorizationException;
@@ -41,6 +40,7 @@
 import com.sk89q.worldedit.util.paste.ActorCallbackPaste;
 import com.sk89q.worldedit.util.report.ReportList;
 import com.sk89q.worldedit.util.report.SystemInfoReport;
+import com.sk89q.worldedit.util.task.FutureForwardingTask;
 import com.sk89q.worldedit.util.task.Task;
 import com.sk89q.worldedit.util.task.TaskStateComparator;
 import com.sk89q.worldedit.world.World;
@@ -142,7 +142,7 @@ public void report(CommandContext args, final Actor sender) throws CommandExcept
 
         if (args.hasFlag('p')) {
             sender.checkPermission("worldguard.report.pastebin");
-            ActorCallbackPaste.pastebin(worldGuard.getSupervisor(), sender, result, "WorldGuard report: %s.report", worldGuard.getExceptionConverter());
+            ActorCallbackPaste.pastebin(worldGuard.getSupervisor(), sender, result, "WorldGuard report: %s.report");
         }
     }
 
@@ -196,14 +196,12 @@ public void profile(final CommandContext args, final Actor sender) throws Comman
         }
 
         sender.print(TextComponent.of("Starting CPU profiling. Use ", TextColor.LIGHT_PURPLE)
-                .append(TextComponent.of("/wg stopprofle", TextColor.AQUA)
+                .append(TextComponent.of("/wg stopprofile", TextColor.AQUA)
                         .clickEvent(ClickEvent.of(ClickEvent.Action.RUN_COMMAND, "/wg stopprofile")))
-                .append(TextComponent.of(" to stop profling.", TextColor.LIGHT_PURPLE)));
-        AsyncCommandHelper.wrap(sampler.getFuture(), worldGuard.getSupervisor(), sender, worldGuard.getExceptionConverter())
-                .formatUsing(minutes)
-                .registerWithSupervisor("Running CPU profiler for %d minute(s)...")
-                .sendMessageAfterDelay("(Please wait... profiling for %d minute(s)...)")
-                .thenTellErrorsOnly("CPU profiling failed");
+                .append(TextComponent.of(" to stop profiling.", TextColor.LIGHT_PURPLE)));
+
+        worldGuard.getSupervisor().monitor(FutureForwardingTask.create(
+                sampler.getFuture(), "CPU profiling for " + minutes + " minutes", sender));
 
         sampler.getFuture().addListener(() -> {
             synchronized (WorldGuardCommands.this) {
@@ -225,7 +223,7 @@ public void onSuccess(Sampler result) {
                 }
 
                 if (pastebin) {
-                    ActorCallbackPaste.pastebin(worldGuard.getSupervisor(), sender, output, "Profile result: %s.profile", worldGuard.getExceptionConverter());
+                    ActorCallbackPaste.pastebin(worldGuard.getSupervisor(), sender, output, "Profile result: %s.profile");
                 }
             }
 

worldguard-core/src/main/java/com/sk89q/worldguard/commands/region/FlagHelperBox.java

@@ -82,7 +82,7 @@ class FlagHelperBox extends PaginationBox {
     private final RegionPermissionModel perms;
 
     FlagHelperBox(World world, ProtectedRegion region, RegionPermissionModel perms) {
-        super("Flags for " + region.getId(), "/rg flags -w " + world.getName() + " " + region.getId() + " %page%");
+        super("Flags for " + region.getId(), "/rg flags -w " + world.getName() + " -p %page% " + region.getId());
         this.world = world;
         this.region = region;
         this.perms = perms;

worldguard-core/src/main/java/com/sk89q/worldguard/commands/region/MemberCommands.java

@@ -19,13 +19,11 @@
 
 package com.sk89q.worldguard.commands.region;
 
-import com.google.common.util.concurrent.Futures;
-import com.google.common.util.concurrent.ListenableFuture;
 import com.sk89q.minecraft.util.commands.Command;
 import com.sk89q.minecraft.util.commands.CommandContext;
 import com.sk89q.minecraft.util.commands.CommandException;
 import com.sk89q.minecraft.util.commands.CommandPermissionsException;
-import com.sk89q.worldedit.command.util.AsyncCommandHelper;
+import com.sk89q.worldedit.command.util.AsyncCommandBuilder;
 import com.sk89q.worldedit.extension.platform.Actor;
 import com.sk89q.worldedit.util.auth.AuthorizationException;
 import com.sk89q.worldedit.world.World;
@@ -37,6 +35,8 @@
 import com.sk89q.worldguard.protection.util.DomainInputResolver;
 import com.sk89q.worldguard.protection.util.DomainInputResolver.UserLocatorPolicy;
 
+import java.util.concurrent.Callable;
+
 public class MemberCommands extends RegionCommandsBase {
 
     private final WorldGuard worldGuard;
@@ -68,16 +68,13 @@ public void addMember(CommandContext args, Actor sender) throws CommandException
                 WorldGuard.getInstance().getProfileService(), args.getParsedPaddedSlice(1, 0));
         resolver.setLocatorPolicy(args.hasFlag('n') ? UserLocatorPolicy.NAME_ONLY : UserLocatorPolicy.UUID_ONLY);
 
-        // Then add it to the members
-        ListenableFuture<DefaultDomain> future = Futures.transform(
-                WorldGuard.getInstance().getExecutorService().submit(resolver),
-                resolver.createAddAllFunction(region.getMembers()));
 
-        AsyncCommandHelper.wrap(future, worldGuard.getSupervisor(), sender, worldGuard.getExceptionConverter())
-                .formatUsing(region.getId(), world.getName())
-                .registerWithSupervisor("Adding members to the region '%s' on '%s'")
-                .sendMessageAfterDelay("(Please wait... querying player names...)")
-                .thenRespondWith("Region '%s' updated with new members.", "Failed to add new members");
+        final String description = String.format("Adding members to the region '%s' on '%s'", region.getId(), world.getName());
+        AsyncCommandBuilder.wrap(resolver, sender)
+                .registerWithSupervisor(worldGuard.getSupervisor(), description)
+                .onSuccess(String.format("Region '%s' updated with new members.", region.getId()), region.getMembers()::addAll)
+                .onFailure("Failed to add new members", worldGuard.getExceptionConverter())
+                .buildAndExec(worldGuard.getExecutorService());
     }
 
     @Command(aliases = {"addowner", "addowner", "ao"},
@@ -127,16 +124,13 @@ public void addOwner(CommandContext args, Actor sender) throws CommandException,
                 WorldGuard.getInstance().getProfileService(), args.getParsedPaddedSlice(1, 0));
         resolver.setLocatorPolicy(args.hasFlag('n') ? UserLocatorPolicy.NAME_ONLY : UserLocatorPolicy.UUID_ONLY);
 
-        // Then add it to the owners
-        ListenableFuture<DefaultDomain> future = Futures.transform(
-                WorldGuard.getInstance().getExecutorService().submit(resolver),
-                resolver.createAddAllFunction(region.getOwners()));
 
-        AsyncCommandHelper.wrap(future, worldGuard.getSupervisor(), sender, worldGuard.getExceptionConverter())
-                .formatUsing(region.getId(), world.getName())
-                .registerWithSupervisor("Adding owners to the region '%s' on '%s'")
-                .sendMessageAfterDelay("(Please wait... querying player names...)")
-                .thenRespondWith("Region '%s' updated with new owners.", "Failed to add new owners");
+        final String description = String.format("Adding owners to the region '%s' on '%s'", region.getId(), world.getName());
+        AsyncCommandBuilder.wrap(resolver, sender)
+                .registerWithSupervisor(worldGuard.getSupervisor(), description)
+                .onSuccess(String.format("Region '%s' updated with new owners.", region.getId()), region.getOwners()::addAll)
+                .onFailure("Failed to add new owners", worldGuard.getExceptionConverter())
+                .buildAndExec(worldGuard.getExecutorService());
     }
 
     @Command(aliases = {"removemember", "remmember", "removemem", "remmem", "rm"},
@@ -157,12 +151,9 @@ public void removeMember(CommandContext args, Actor sender) throws CommandExcept
             throw new CommandPermissionsException();
         }
 
-        ListenableFuture<?> future;
-
+        Callable<DefaultDomain> callable;
         if (args.hasFlag('a')) {
-            region.getMembers().removeAll();
-
-            future = Futures.immediateFuture(null);
+            callable = region::getMembers;
         } else {
             if (args.argsLength() < 2) {
                 throw new CommandException("List some names to remove, or use -a to remove all.");
@@ -173,17 +164,16 @@ public void removeMember(CommandContext args, Actor sender) throws CommandExcept
                     WorldGuard.getInstance().getProfileService(), args.getParsedPaddedSlice(1, 0));
             resolver.setLocatorPolicy(args.hasFlag('n') ? UserLocatorPolicy.NAME_ONLY : UserLocatorPolicy.UUID_AND_NAME);
 
-            // Then remove it from the members
-            future = Futures.transform(
-                    WorldGuard.getInstance().getExecutorService().submit(resolver),
-                    resolver.createRemoveAllFunction(region.getMembers()));
+            callable = resolver;
         }
 
-        AsyncCommandHelper.wrap(future, worldGuard.getSupervisor(), sender, worldGuard.getExceptionConverter())
-                .formatUsing(region.getId(), world.getName())
-                .registerWithSupervisor("Removing members from the region '%s' on '%s'")
+        final String description = String.format("Removing members from the region '%s' on '%s'", region.getId(), world.getName());
+        AsyncCommandBuilder.wrap(callable, sender)
+                .registerWithSupervisor(worldGuard.getSupervisor(), description)
                 .sendMessageAfterDelay("(Please wait... querying player names...)")
-                .thenRespondWith("Region '%s' updated with members removed.", "Failed to remove members");
+                .onSuccess(String.format("Region '%s' updated with members removed.", region.getId()), region.getMembers()::removeAll)
+                .onFailure("Failed to remove members", worldGuard.getExceptionConverter())
+                .buildAndExec(worldGuard.getExecutorService());
     }
 
     @Command(aliases = {"removeowner", "remowner", "ro"},
@@ -204,12 +194,9 @@ public void removeOwner(CommandContext args, Actor sender) throws CommandExcepti
             throw new CommandPermissionsException();
         }
 
-        ListenableFuture<?> future;
-
+        Callable<DefaultDomain> callable;
         if (args.hasFlag('a')) {
-            region.getOwners().removeAll();
-
-            future = Futures.immediateFuture(null);
+            callable = region::getOwners;
         } else {
             if (args.argsLength() < 2) {
                 throw new CommandException("List some names to remove, or use -a to remove all.");
@@ -220,16 +207,15 @@ public void removeOwner(CommandContext args, Actor sender) throws CommandExcepti
                     WorldGuard.getInstance().getProfileService(), args.getParsedPaddedSlice(1, 0));
             resolver.setLocatorPolicy(args.hasFlag('n') ? UserLocatorPolicy.NAME_ONLY : UserLocatorPolicy.UUID_AND_NAME);
 
-            // Then remove it from the owners
-            future = Futures.transform(
-                    WorldGuard.getInstance().getExecutorService().submit(resolver),
-                    resolver.createRemoveAllFunction(region.getOwners()));
+            callable = resolver;
         }
 
-        AsyncCommandHelper.wrap(future, worldGuard.getSupervisor(), sender, worldGuard.getExceptionConverter())
-                .formatUsing(region.getId(), world.getName())
-                .registerWithSupervisor("Removing owners from the region '%s' on '%s'")
+        final String description = String.format("Removing owners from the region '%s' on '%s'", region.getId(), world.getName());
+        AsyncCommandBuilder.wrap(callable, sender)
+                .registerWithSupervisor(worldGuard.getSupervisor(), description)
                 .sendMessageAfterDelay("(Please wait... querying player names...)")
-                .thenRespondWith("Region '%s' updated with owners removed.", "Failed to remove owners");
+                .onSuccess(String.format("Region '%s' updated with owners removed.", region.getId()), region.getOwners()::removeAll)
+                .onFailure("Failed to remove owners", worldGuard.getExceptionConverter())
+                .buildAndExec(worldGuard.getExecutorService());
     }
 }