Resolve map author names locally

API/api/src/main/java/tc/oc/api/docs/virtual/MapDoc.java

@@ -5,6 +5,7 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
+import java.util.stream.Stream;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -50,6 +51,12 @@ public interface MapDoc extends Model {
     Collection<UUID> author_uuids();
     Collection<UUID> contributor_uuids();
 
+    @Serialize(false)
+    default Stream<UUID> authorAndContributorUuids() {
+        return Stream.concat(author_uuids().stream(),
+                             contributor_uuids().stream());
+    }
+
     @Serialize
     interface Team extends Model {
         @Nonnull String name();

API/api/src/main/java/tc/oc/api/maps/MapService.java

@@ -13,5 +13,8 @@ public interface MapService extends ModelService<MapDoc, MapDoc> {
 
     ListenableFuture<MapRatingsResponse> getRatings(MapRatingsRequest request);
 
-    ListenableFuture<MapUpdateMultiResponse> updateMapsAndLookupAuthors(Collection<? extends MapDoc> maps);
+    /**
+     * Send map updates to the backend, and retrieve data about map contributors.
+     */
+    UpdateMapsResponse updateMaps(Collection<? extends MapDoc> maps);
 }

API/api/src/main/java/tc/oc/api/maps/NullMapService.java

@@ -7,6 +7,7 @@ import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import tc.oc.api.docs.MapRating;
 import tc.oc.api.docs.virtual.MapDoc;
+import tc.oc.api.message.types.UpdateMultiResponse;
 import tc.oc.api.model.NullModelService;
 
 public class NullMapService extends NullModelService<MapDoc, MapDoc> implements MapService {
@@ -22,7 +23,10 @@ public class NullMapService extends NullModelService<MapDoc, MapDoc> implements
     }
 
     @Override
-    public ListenableFuture<MapUpdateMultiResponse> updateMapsAndLookupAuthors(Collection<? extends MapDoc> maps) {
+    public UpdateMapsResponse updateMaps(Collection<? extends MapDoc> maps) {
-        return Futures.immediateFuture(new MapUpdateMultiResponse(Collections.emptyMap()));
+        return new UpdateMapsResponse(
+            Futures.immediateFuture(UpdateMultiResponse.EMPTY),
+            Collections.emptyMap()
+        );
     }
 }

API/api/src/main/java/tc/oc/api/maps/UpdateMapsResponse.java

@@ -0,0 +1,38 @@
+package tc.oc.api.maps;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.UUID;
+
+import com.google.common.util.concurrent.ListenableFuture;
+import tc.oc.api.docs.virtual.UserDoc;
+import tc.oc.api.message.types.UpdateMultiResponse;
+
+/**
+ * Result of {@link MapService#updateMaps(Collection)}
+ */
+public class UpdateMapsResponse {
+
+    private final ListenableFuture<UpdateMultiResponse> maps;
+    private final Map<UUID, ListenableFuture<UserDoc.Identity>> authors;
+
+    public UpdateMapsResponse(ListenableFuture<UpdateMultiResponse> maps, Map<UUID, ListenableFuture<UserDoc.Identity>> authors) {
+        this.maps = maps;
+        this.authors = authors;
+    }
+
+    /**
+     * Result of updating all the maps
+     */
+    public ListenableFuture<UpdateMultiResponse> maps() {
+        return maps;
+    }
+
+    /**
+     * Result of each individual map contributor lookup,
+     * which may not all arrive at the same time.
+     */
+    public Map<UUID, ListenableFuture<UserDoc.Identity>> authors() {
+        return authors;
+    }
+}

API/api/src/main/java/tc/oc/api/users/UserSearchRequest.java

@@ -5,6 +5,7 @@ import javax.annotation.Nullable;
 import tc.oc.api.annotations.Serialize;
 import tc.oc.api.docs.PlayerId;
 import tc.oc.api.docs.virtual.Document;
+import tc.oc.minecraft.api.user.UserUtils;
 
 public class UserSearchRequest implements Document {
     @Serialize public final String username;

API/api/src/main/java/tc/oc/api/users/UserUtils.java

@@ -1,10 +0,0 @@
-package tc.oc.api.users;
-
-import tc.oc.commons.core.formatting.StringUtils;
-
-public interface UserUtils {
-
-    static String sanitizeUsername(String username) {
-        return StringUtils.truncate(username.replaceAll("[^A-Za-z0-9_]", ""), 16);
-    }
-}

API/minecraft/src/main/java/tc/oc/api/minecraft/maps/LocalMapService.java

@@ -2,7 +2,6 @@ package tc.oc.api.minecraft.maps;
 
 import java.util.Collection;
 import java.util.Collections;
-import java.util.stream.Stream;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 
@@ -13,9 +12,12 @@ import tc.oc.api.docs.virtual.MapDoc;
 import tc.oc.api.maps.MapRatingsRequest;
 import tc.oc.api.maps.MapRatingsResponse;
 import tc.oc.api.maps.MapService;
-import tc.oc.api.maps.MapUpdateMultiResponse;
+import tc.oc.api.maps.UpdateMapsResponse;
+import tc.oc.api.message.types.UpdateMultiResponse;
 import tc.oc.api.minecraft.users.UserStore;
 import tc.oc.api.model.NullModelService;
+import tc.oc.api.users.UserService;
+import tc.oc.api.util.UUIDs;
 import tc.oc.commons.core.stream.Collectors;
 import tc.oc.minecraft.api.entity.Player;
 
@@ -23,6 +25,7 @@ import tc.oc.minecraft.api.entity.Player;
 public class LocalMapService extends NullModelService<MapDoc, MapDoc> implements MapService {
 
     @Inject private UserStore<Player> userStore;
+    @Inject private UserService userService;
 
     @Override
     public ListenableFuture<?> rate(MapRating rating) {
@@ -35,14 +38,13 @@ public class LocalMapService extends NullModelService<MapDoc, MapDoc> implements
     }
 
     @Override
-    public ListenableFuture<MapUpdateMultiResponse> updateMapsAndLookupAuthors(Collection<? extends MapDoc> maps) {
+    public UpdateMapsResponse updateMaps(Collection<? extends MapDoc> maps) {
-        return Futures.immediateFuture(new MapUpdateMultiResponse(
+        return new UpdateMapsResponse(
+            Futures.immediateFuture(UpdateMultiResponse.EMPTY),
             maps.stream()
-                .flatMap(map -> Stream.concat(map.author_uuids().stream(),
+                .flatMap(MapDoc::authorAndContributorUuids)
-                                              map.contributor_uuids().stream()))
+                .distinct()
-                .collect(Collectors.mappingTo(uuid -> userStore.byUuid(uuid)
+                .collect(Collectors.mappingTo(uuid -> (ListenableFuture) userService.find(() -> UUIDs.normalize(uuid))))
-                                                               .flatMap(userStore::user)
+        );
-                                                               .orElse(null)))
-        ));
     }
 }

API/minecraft/src/main/java/tc/oc/api/minecraft/servers/LocalServerDocument.java

@@ -17,7 +17,7 @@ import tc.oc.api.docs.virtual.MapDoc;
 import tc.oc.api.docs.virtual.MatchDoc;
 import tc.oc.api.docs.virtual.ServerDoc;
 import tc.oc.api.minecraft.config.MinecraftApiConfiguration;
-import tc.oc.minecraft.api.entity.OfflinePlayer;
+import tc.oc.minecraft.api.user.User;
 import tc.oc.minecraft.api.server.LocalServer;
 
 @Singleton
@@ -162,8 +162,8 @@ public class LocalServerDocument extends StartupServerDocument implements Server
     @Override
     public Map<UUID, String> operators() {
         final ImmutableMap.Builder<UUID, String> ops = ImmutableMap.builder();
-        for(OfflinePlayer op : minecraftServer.getOperators()) {
+        for(User op : minecraftServer.getOperators()) {
-            ops.put(op.getUniqueId(), op.getLastKnownName().orElse("Player"));
+            ops.put(op.getUniqueId(), op.name().orElse("Player"));
         }
         return ops.build();
     }

API/minecraft/src/main/java/tc/oc/api/minecraft/users/LocalUserDocument.java

@@ -15,18 +15,24 @@ import tc.oc.api.docs.SimplePlayerId;
 import tc.oc.api.docs.User;
 import tc.oc.api.docs.virtual.UserDoc;
 import tc.oc.api.minecraft.servers.DefaultPermissions;
-import tc.oc.minecraft.api.entity.OfflinePlayer;
 
 public class LocalUserDocument extends SimplePlayerId implements User {
 
-    private final OfflinePlayer player;
+    private final UUID uuid;
+    private final String ip;
 
-    public LocalUserDocument(OfflinePlayer player) {
+    LocalUserDocument(UUID uuid, String name, String ip) {
-        super(player.getUniqueId().toString(),
+        super(uuid.toString(), uuid.toString(), name);
-              player.getUniqueId().toString(),
+        this.uuid = uuid;
-              player.getLastKnownName().orElse(""));
+        this.ip = ip;
+    }
 
-        this.player = player;
+    LocalUserDocument(tc.oc.minecraft.api.user.User user) {
+        this(user.getUniqueId(),
+             user.getName(),
+             user.onlinePlayer()
+                 .map(p -> p.getAddress().getHostString())
+                 .orElse(""));
     }
 
     @Override
@@ -41,7 +47,7 @@ public class LocalUserDocument extends SimplePlayerId implements User {
 
     @Override
     public UUID uuid() {
-        return player.getUniqueId();
+        return uuid;
     }
 
     @Override
@@ -76,9 +82,7 @@ public class LocalUserDocument extends SimplePlayerId implements User {
 
     @Override
     public String mc_last_sign_in_ip() {
-        return player.onlinePlayer()
+        return ip;
-                     .map(p -> p.getAddress().getHostString())
-                     .orElse("");
     }
 
     @Override

API/minecraft/src/main/java/tc/oc/api/minecraft/users/LocalUserService.java

@@ -30,39 +30,43 @@ import tc.oc.api.users.UserSearchResponse;
 import tc.oc.api.users.UserService;
 import tc.oc.api.users.UserUpdateResponse;
 import tc.oc.commons.core.concurrent.FutureUtils;
-import tc.oc.minecraft.api.entity.OfflinePlayer;
+import tc.oc.minecraft.api.user.UserFinder;
-import tc.oc.minecraft.api.server.LocalServer;
 
 @Singleton
-public class LocalUserService extends NullModelService<User, UserDoc.Partial> implements UserService {
+class LocalUserService extends NullModelService<User, UserDoc.Partial> implements UserService {
 
-    @Inject private LocalServer minecraftServer;
     @Inject private LocalSessionFactory sessionFactory;
+    @Inject private UserFinder userFinder;
 
     @Override
     public ListenableFuture<User> find(UserId userId) {
-        return Futures.immediateFuture(new LocalUserDocument(minecraftServer.getOfflinePlayer(UUID.fromString(userId.player_id()))));
+        return FutureUtils.mapSync(
+            userFinder.findUserAsync(UUID.fromString(userId.player_id())),
+            user -> {
+                if(user.hasValidId() && user.name().isPresent()) {
+                    return new LocalUserDocument(user);
+                }
+                throw new NotFound("No user with UUID " + userId.player_id());
+            }
+        );
     }
 
     @Override
     public ListenableFuture<UserSearchResponse> search(UserSearchRequest request) {
-        for(OfflinePlayer player : minecraftServer.getSavedPlayers()) {
+        return FutureUtils.mapSync(
-            if(player.getLastKnownName()
+            userFinder.findUserAsync(request.username),
-                     .filter(name -> name.equalsIgnoreCase(request.username))
+            user -> {
-                     .isPresent()) {
+                if(user.hasValidId()) {
-                return Futures.immediateFuture(new UserSearchResponse(new LocalUserDocument(player),
+                    return new UserSearchResponse(new LocalUserDocument(user), user.isOnline(), false, null, null);
-                                                                      player.isOnline(),
+                }
-                                                                      false,
+                throw new NotFound("No user named '" + request.username + "'");
-                                                                      null,
-                                                                      null));
             }
-        }
+        );
-        return Futures.immediateFailedFuture(new NotFound("No user named '" + request.username + "'", null));
     }
 
     @Override
     public ListenableFuture<LoginResponse> login(LoginRequest request) {
-        final User user = new LocalUserDocument(minecraftServer.getOfflinePlayer(request.uuid));
+        final User user = new LocalUserDocument(request.uuid, request.username, request.ip.getHostAddress());
         final Session session = request.start_session ? sessionFactory.newSession(user, request.ip)
                                                       : null;
 

API/ocn/src/main/java/tc/oc/api/ocn/MapUpdateMultiResponse.java

@@ -1,4 +1,4 @@
-package tc.oc.api.maps;
+package tc.oc.api.ocn;
 
 import java.util.Collections;
 import java.util.Map;

API/ocn/src/main/java/tc/oc/api/ocn/OCNMapService.java

@@ -6,12 +6,16 @@ import javax.inject.Singleton;
 import com.google.common.util.concurrent.ListenableFuture;
 import tc.oc.api.docs.MapRating;
 import tc.oc.api.docs.virtual.MapDoc;
+import tc.oc.api.docs.virtual.UserDoc;
+import tc.oc.api.exceptions.NotFound;
 import tc.oc.api.http.HttpOption;
 import tc.oc.api.maps.MapRatingsRequest;
 import tc.oc.api.maps.MapRatingsResponse;
 import tc.oc.api.maps.MapService;
-import tc.oc.api.maps.MapUpdateMultiResponse;
+import tc.oc.api.maps.UpdateMapsResponse;
 import tc.oc.api.model.HttpModelService;
+import tc.oc.commons.core.concurrent.FutureUtils;
+import tc.oc.commons.core.stream.Collectors;
 
 @Singleton
 class OCNMapService extends HttpModelService<MapDoc, MapDoc> implements MapService {
@@ -24,7 +28,21 @@ class OCNMapService extends HttpModelService<MapDoc, MapDoc> implements MapServi
         return this.client().post(memberUri(request.map_id, "get_ratings"), request, MapRatingsResponse.class, HttpOption.INFINITE_RETRY);
     }
 
-    public ListenableFuture<MapUpdateMultiResponse> updateMapsAndLookupAuthors(Collection<? extends MapDoc> maps) {
+    public UpdateMapsResponse updateMaps(Collection<? extends MapDoc> maps) {
-        return updateMulti(maps, MapUpdateMultiResponse.class);
+        final ListenableFuture<MapUpdateMultiResponse> future = updateMulti(maps, MapUpdateMultiResponse.class);
+        return new UpdateMapsResponse(
+            (ListenableFuture) future,
+            maps.stream()
+                .flatMap(MapDoc::authorAndContributorUuids)
+                .distinct()
+                .collect(Collectors.mappingTo(uuid -> FutureUtils.mapSync(
+                    future,
+                    response -> {
+                        final UserDoc.Identity user = response.users_by_uuid.get(uuid);
+                        if(user != null) return user;
+                        throw new NotFound();
+                    }
+                )))
+        );
     }
 }

PGM/src/main/java/tc/oc/pgm/map/MapDocument.java

@@ -4,6 +4,7 @@ import java.net.URL;
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.UUID;
 import javax.annotation.Nullable;
@@ -25,6 +26,7 @@ public class MapDocument extends AbstractModel implements MapDoc {
     private final InfoModule infoModule;
     private final MapInfo info;
     private final List<MapDoc.Team> teams;
+    private final List<UUID> authors, contributors;
 
     @Inject private MapDocument(MapFolder folder, InfoModule infoModule, MapInfo info, List<TeamFactory> teams) {
         this.folder = folder;
@@ -33,6 +35,16 @@ public class MapDocument extends AbstractModel implements MapDoc {
         this.teams = teams.stream()
                           .map(TeamFactory::getDocument)
                           .collect(toImmutableList());
+
+        this.authors = info.authors.stream()
+                                   .map(Contributor::getUuid)
+                                   .filter(Objects::nonNull)
+                                   .collect(toImmutableList());
+
+        this.contributors = info.contributors.stream()
+                                             .map(Contributor::getUuid)
+                                             .filter(Objects::nonNull)
+                                             .collect(toImmutableList());
     }
 
     @Override
@@ -113,18 +125,12 @@ public class MapDocument extends AbstractModel implements MapDoc {
 
     @Override
     public Collection<UUID> author_uuids() {
-        return info.authors.stream()
+        return authors;
-            .map(Contributor::getUuid)
-            .filter(uuid -> uuid != null)
-            .collect(toImmutableList());
     }
 
     @Override
     public Collection<UUID> contributor_uuids() {
-        return info.contributors.stream()
+        return contributors;
-            .map(Contributor::getUuid)
-            .filter(uuid -> uuid != null)
-            .collect(toImmutableList());
     }
 
 }

PGM/src/main/java/tc/oc/pgm/map/MapInfo.java

@@ -2,6 +2,7 @@ package tc.oc.pgm.map;
 
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 
 import com.google.common.collect.ComparisonChain;
@@ -153,8 +154,8 @@ public class MapInfo implements Comparable<MapInfo> {
         return Contributor.filterNamed(this.contributors);
     }
 
-    public Iterable<Contributor> getAllContributors() {
+    public Stream<Contributor> allContributors() {
-        return Iterables.concat(authors, contributors);
+        return Stream.concat(authors.stream(), contributors.stream());
     }
 
     public boolean isAuthor(PlayerId player) {

PGM/src/main/java/tc/oc/pgm/map/MapLibrary.java

@@ -10,7 +10,7 @@ import java.util.logging.Logger;
 import javax.annotation.Nullable;
 
 import com.google.common.util.concurrent.ListenableFuture;
-import tc.oc.api.maps.MapUpdateMultiResponse;
+import tc.oc.api.message.types.UpdateMultiResponse;
 
 public interface MapLibrary {
 
@@ -44,7 +44,7 @@ public interface MapLibrary {
 
     Collection<PGMMap> getDirtyMaps();
 
-    ListenableFuture<MapUpdateMultiResponse> pushAllMaps();
+    ListenableFuture<UpdateMultiResponse> pushAllMaps();
 
-    ListenableFuture<MapUpdateMultiResponse> pushDirtyMaps();
+    ListenableFuture<UpdateMultiResponse> pushDirtyMaps();
 }

PGM/src/main/java/tc/oc/pgm/map/MapLibraryImpl.java

@@ -6,6 +6,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
@@ -23,12 +24,14 @@ import com.google.common.collect.SetMultimap;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
 import tc.oc.api.docs.virtual.UserDoc;
+import tc.oc.api.exceptions.NotFound;
 import tc.oc.api.maps.MapService;
-import tc.oc.api.maps.MapUpdateMultiResponse;
+import tc.oc.api.maps.UpdateMapsResponse;
-import tc.oc.minecraft.scheduler.SyncExecutor;
+import tc.oc.api.message.types.UpdateMultiResponse;
 import tc.oc.commons.core.logging.Loggers;
-import tc.oc.commons.core.util.Pair;
+import tc.oc.commons.core.stream.Collectors;
 import tc.oc.commons.core.util.SystemFutureCallback;
+import tc.oc.minecraft.scheduler.SyncExecutor;
 
 @Singleton
 public class MapLibraryImpl implements MapLibrary {
@@ -173,66 +176,73 @@ public class MapLibraryImpl implements MapLibrary {
     }
 
     @Override
-    public ListenableFuture<MapUpdateMultiResponse> pushAllMaps() {
+    public ListenableFuture<UpdateMultiResponse> pushAllMaps() {
         return pushMaps(getMaps());
     }
 
     @Override
-    public ListenableFuture<MapUpdateMultiResponse> pushDirtyMaps() {
+    public ListenableFuture<UpdateMultiResponse> pushDirtyMaps() {
         return pushMaps(getDirtyMaps());
     }
 
-    private ListenableFuture<MapUpdateMultiResponse> pushMaps(final Collection<PGMMap> maps) {
+    private ListenableFuture<UpdateMultiResponse> pushMaps(final Collection<PGMMap> maps) {
         final Set<PGMMap> pushedMaps = ImmutableSet.copyOf(maps);
-        logger.info("Pushing " + pushedMaps.size() + " maps");
 
-        final SettableFuture<MapUpdateMultiResponse> future = SettableFuture.create();
+        final SetMultimap<UUID, PGMMap> mapsByContributorId = maps.stream().collect(Collectors.indexingByMulti(
+            map -> map.getInfo()
+                      .allContributors()
+                      .map(Contributor::getUuid)
+                      .filter(Objects::nonNull)
+        ));
+
+        logger.info("Pushing " + pushedMaps.size() +
+                    " maps and resolving " + mapsByContributorId.keySet().size() + " contributor names");
+
+        final UpdateMapsResponse response = mapService.updateMaps(
+            Collections2.transform(pushedMaps, PGMMap::getDocument)
+        );
+
+        response.authors().forEach(
+            (uuid, future) -> syncExecutor.callback(
+                future,
+                SystemFutureCallback.<UserDoc.Identity>onSuccess(
+                    user -> {
+                        logger.fine(() -> "Resolved map author " + uuid + " to " + user.username());
+                        mapsByContributorId.removeAll(uuid).forEach(
+                            map -> map.getInfo().allContributors().forEach(contributor -> {
+                                if(uuid.equals(contributor.getUuid())) {
+                                    contributor.setUser(user);
+                                }
+                            })
+                        );
+                    }
+                ).onFailure(NotFound.class, ex ->
+                    mapsByContributorId.removeAll(uuid).forEach(
+                        map -> map.getLogger().severe("Contributor UUID not found: " + uuid)
+                    )
+                ).onCompletion(() -> {
+                    if(mapsByContributorId.isEmpty()) {
+                        logger.info("Finished resolving map contributors");
+                    }
+                })
+            )
+        );
+
+        final SettableFuture<UpdateMultiResponse> future = SettableFuture.create();
         syncExecutor.callback(
-            mapService.updateMapsAndLookupAuthors(Collections2.transform(pushedMaps, PGMMap::getDocument)),
+            response.maps(),
-            new SystemFutureCallback<MapUpdateMultiResponse>() {
+            SystemFutureCallback.<UpdateMultiResponse>onSuccess(mapResponse -> {
-                @Override public void onSuccessThrows(MapUpdateMultiResponse result) throws Exception {
+                logger.info("Push complete: " + mapResponse);
-                    logger.info("Push complete: " + result);
+                logErrors(mapResponse);
-                    logErrors(result);
+                pushedMaps.forEach(PGMMap::markPushed);
-                    pushedMaps.forEach(PGMMap::markPushed);
+                future.set(mapResponse);
-                    resolveContributors(pushedMaps, result.users_by_uuid);
+            }).onFailure(Throwable.class, future::setException)
-                    future.set(result);
-                }
-
-                @Override
-                public void onFailure(Throwable e) {
-                    super.onFailure(e);
-                    future.setException(e);
-                }
-            }
         );
 
         return future;
     }
 
-    private void resolveContributors(Collection<PGMMap> maps, Map<UUID, UserDoc.Identity> usersByUuid) {
+    private void logErrors(UpdateMultiResponse response) {
-        Set<Pair<PGMMap, Contributor>> missing = new HashSet<>();
-
-        for(PGMMap map : maps) {
-            for(Contributor contributor : map.getInfo().getAllContributors()) {
-                if(contributor.needsLookup()) {
-                    final UserDoc.Identity user = usersByUuid.get(contributor.getUuid());
-                    if(user != null) {
-                        contributor.setUser(user);
-                    } else {
-                        missing.add(Pair.create(map, contributor));
-                    }
-                }
-            }
-        }
-
-        logger.info("Resolved " + usersByUuid.size() + " contributor UUIDs");
-
-        for(Pair<PGMMap, Contributor> pair : missing) {
-            pair.first.getLogger().severe("Contributor UUID not found: " + pair.second.getUuid());
-        }
-    }
-
-    private void logErrors(MapUpdateMultiResponse response) {
         for(Map.Entry<String, Map<String, List<String>>> mapEntry : response.errors.entrySet()) {
             final PGMMap map = needMapById(mapEntry.getKey());
             for(Map.Entry<String, List<String>> propEntry : mapEntry.getValue().entrySet()) {

Util/bukkit/src/main/java/tc/oc/commons/bukkit/event/EventHandlerScanner.java

@@ -68,6 +68,6 @@ public abstract class EventHandlerScanner<Event, HandlerAnnotation extends Annot
     public SetMultimap<EventKey<? extends Event>, Info> findEventHandlers(Class<? extends Listener> listener) {
         return findEventHandlerMethods(listener)
             .map(method -> createHandlerInfo(method, findEventType(method), findAnnotation(method)))
-            .collect(Collectors.toSetMultimap(EventSubscriber::key));
+            .collect(Collectors.toImmutableSetMultimap(EventSubscriber::key));
     }
 }

Util/core/src/main/java/tc/oc/commons/core/commands/CommandFutureCallback.java

@@ -80,7 +80,7 @@ public class CommandFutureCallback<T> extends SystemFutureCallback<T> {
     }
 
     @Override
-    protected void onFailureUnhandled(Throwable e) {
+    protected void handleDefaultFailure(Throwable e) {
         exceptionHandlerFactory.create(audience, command)
                                .handleException(e, this, creationSite);
     }

Util/core/src/main/java/tc/oc/commons/core/concurrent/FutureUtils.java

@@ -15,6 +15,7 @@ import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.Uninterruptibles;
 import tc.oc.commons.core.util.Pair;
+import tc.oc.commons.core.util.ThrowingFunction;
 
 public final class FutureUtils {
     private FutureUtils() {}
@@ -85,15 +86,24 @@ public final class FutureUtils {
     /**
      * Equivalent to {@link Futures#transform}, but can be passed a lambda unambiguously
      */
-    public static <T, R> ListenableFuture<R> mapSync(ListenableFuture<T> in, Function<T, R> op) {
+    public static <T, R> ListenableFuture<R> mapSync(ListenableFuture<T> in, ThrowingFunction<T, R, ?> op) {
-        return Futures.transform(in, op::apply);
+        return mapSync(in, op, MoreExecutors.sameThreadExecutor());
     }
-    public static <T, R> ListenableFuture<R> mapSync(ListenableFuture<T> in, Function<T, R> op, Executor executor) {
+
-        return Futures.transform(in, op::apply, executor);
+    public static <T, R> ListenableFuture<R> mapSync(ListenableFuture<T> in, ThrowingFunction<T, R, ?> op, Executor executor) {
+        return mapAsync(in, out -> {
+            try {
+                return Futures.immediateFuture(op.applyThrows(out));
+            } catch(Throwable ex) {
+                return Futures.immediateFailedFuture(ex);
+            }
+        }, executor);
     }
+
     public static <T, R> ListenableFuture<R> mapAsync(ListenableFuture<T> in, AsyncFunction<T, R> op) {
         return Futures.transform(in, op);
     }
+
     public static <T, R> ListenableFuture<R> mapAsync(ListenableFuture<T> in, AsyncFunction<T, R> op, Executor executor) {
         return Futures.transform(in, op, executor);
     }

Util/core/src/main/java/tc/oc/commons/core/stream/Collectors.java

@@ -1,6 +1,7 @@
 package tc.oc.commons.core.stream;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
@@ -11,14 +12,17 @@ import java.util.function.BinaryOperator;
 import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collector;
+import java.util.stream.Stream;
 
+import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.SetMultimap;
 import tc.oc.commons.core.random.Entropy;
 import tc.oc.commons.core.random.MutableEntropy;
 import tc.oc.commons.core.util.AmbiguousElementException;
@@ -99,25 +103,21 @@ public final class Collectors {
         };
     }
 
-    public static <T, K, V> Collector<T, ?, ImmutableListMultimap<K, V>> toListMultimap(Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends V> valueMapper) {
+    public static <T, K> Collector<T, ?, ImmutableSetMultimap<K, T>> toImmutableSetMultimap(Function<? super T, ? extends K> keyMapper) {
-        return new MultimapCollector<>(ImmutableListMultimap::builder, keyMapper, valueMapper);
+        return toImmutableSetMultimap(keyMapper, identity());
     }
 
-    public static <T, K> Collector<T, ?, ImmutableSetMultimap<K, T>> toSetMultimap(Function<? super T, ? extends K> keyMapper) {
+    public static <T, K, V> Collector<T, ?, ImmutableSetMultimap<K, V>> toImmutableSetMultimap(Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends V> valueMapper) {
-        return toSetMultimap(keyMapper, identity());
+        return new ImmutableMultimapCollector<>(ImmutableSetMultimap::builder, t -> Stream.of(keyMapper.apply(t)), t -> Stream.of(valueMapper.apply(t)));
     }
 
-    public static <T, K, V> Collector<T, ?, ImmutableSetMultimap<K, V>> toSetMultimap(Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends V> valueMapper) {
+    private static class ImmutableMultimapCollector<T, K, V, A extends ImmutableMultimap.Builder<K, V>, R extends ImmutableMultimap<K, V>> implements Collector<T, A, R> {
-        return new MultimapCollector<>(ImmutableSetMultimap::builder, keyMapper, valueMapper);
-    }
-
-    private static class MultimapCollector<T, K, V, A extends ImmutableMultimap.Builder<K, V>, R extends ImmutableMultimap<K, V>> implements Collector<T, A, R> {
 
         private final Supplier<A> builderSupplier;
-        private final Function<? super T, ? extends K> keyMapper;
+        private final Function<? super T, Stream<? extends K>> keyMapper;
-        private final Function<? super T, ? extends V> valueMapper;
+        private final Function<? super T, Stream<? extends V>> valueMapper;
 
-        private MultimapCollector(Supplier<A> builderSupplier, Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends V> valueMapper) {
+        private ImmutableMultimapCollector(Supplier<A> builderSupplier, Function<? super T, Stream<? extends K>> keyMapper, Function<? super T, Stream<? extends V>> valueMapper) {
             this.builderSupplier = builderSupplier;
             this.keyMapper = keyMapper;
             this.valueMapper = valueMapper;
@@ -131,11 +131,11 @@ public final class Collectors {
         @Override
         public BiConsumer<A, T> accumulator() {
             return (builder, t) -> {
-                final K key = keyMapper.apply(t);
+                keyMapper.apply(t).forEach(
-                final V value = valueMapper.apply(t);
+                    key -> valueMapper.apply(t).forEach(
-                if(key != null && value != null) {
+                        value -> builder.put(key, value)
-                    builder.put(key, value);
+                    )
-                }
+                );
             };
         }
 
@@ -160,6 +160,61 @@ public final class Collectors {
         }
     }
 
+    public static <T, K> Collector<T, ?, SetMultimap<K, T>> indexingByMulti(Function<? super T, Stream<? extends K>> keyMapper) {
+        return new MultimapCollector<>(HashMultimap::create, keyMapper, Stream::of);
+    }
+
+    public static <T, V> Collector<T, ?, SetMultimap<T, V>> mappingToMulti(Function<? super T, Stream<? extends V>> valueMapper) {
+        return new MultimapCollector<>(HashMultimap::create, Stream::of, valueMapper);
+    }
+
+    private static class MultimapCollector<T, K, V, A extends Multimap> implements Collector<T, A, A> {
+
+        private final Supplier<A> multimapSupplier;
+        private final Function<? super T, Stream<? extends K>> keyMapper;
+        private final Function<? super T, Stream<? extends V>> valueMapper;
+
+        private MultimapCollector(Supplier<A> multimapSupplier, Function<? super T, Stream<? extends K>> keyMapper, Function<? super T, Stream<? extends V>> valueMapper) {
+            this.multimapSupplier = multimapSupplier;
+            this.keyMapper = keyMapper;
+            this.valueMapper = valueMapper;
+        }
+
+        @Override
+        public Supplier<A> supplier() {
+            return multimapSupplier;
+        }
+
+        @Override
+        public BiConsumer<A, T> accumulator() {
+            return (multimap, t) -> {
+                keyMapper.apply(t).forEach(
+                    key -> valueMapper.apply(t).forEach(
+                        value -> multimap.put(key, value)
+                    )
+                );
+            };
+        }
+
+        @Override
+        public BinaryOperator<A> combiner() {
+            return (m1, m2) -> {
+                m1.putAll(m2);
+                return m1;
+            };
+        }
+
+        @Override
+        public Function<A, A> finisher() {
+            return Function.identity();
+        }
+
+        @Override
+        public Set<Characteristics> characteristics() {
+            return Collections.singleton(Characteristics.IDENTITY_FINISH);
+        }
+    }
+
     private static class ListCollector<T, R> implements Collector<T, ArrayList<T>, R> {
 
         private final Function<ArrayList<T>, R> finisher;

Util/core/src/main/java/tc/oc/commons/core/util/SystemFutureCallback.java

@@ -1,5 +1,7 @@
 package tc.oc.commons.core.util;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
@@ -25,6 +27,7 @@ public class SystemFutureCallback<T> implements FutureCallback<T> {
     protected final StackTrace creationSite;
     private final @Nullable ThrowingConsumer<? super T, ?> successHandler;
     private final ListMultimap<Class<? extends Throwable>, ThrowingConsumer> failureHandlers = ArrayListMultimap.create();
+    private final List<ThrowingRunnable<?>> completionHandlers = new ArrayList<>();
 
     public static <T> SystemFutureCallback<T> onSuccess(ThrowingConsumer<? super T, ?> handler) {
         return new SystemFutureCallback<>(checkNotNull(handler));
@@ -53,6 +56,11 @@ public class SystemFutureCallback<T> implements FutureCallback<T> {
         return this;
     }
 
+    public SystemFutureCallback<T> onCompletion(ThrowingRunnable<Throwable> handler) {
+        completionHandlers.add(handler);
+        return this;
+    }
+
     /**
      * @deprecated use {@link #SystemFutureCallback(ThrowingConsumer)}
      */
@@ -68,16 +76,22 @@ public class SystemFutureCallback<T> implements FutureCallback<T> {
                 onSuccessThrows(result);
             }
         } catch(Throwable e) {
-            onFailure(e);
+            handleFailure(e);
         }
-    }
+        handleCompletion();
-
-    protected void onFailureUnhandled(Throwable e) {
-        exceptionHandler.handleException(e, this, creationSite);
     }
 
     @Override
     public void onFailure(Throwable e) {
+        handleFailure(e);
+        handleCompletion();
+    }
+
+    protected void handleDefaultFailure(Throwable e) {
+        exceptionHandler.handleException(e, this, creationSite);
+    }
+
+    private void handleFailure(Throwable e) {
         boolean handled = false;
         for(Map.Entry<Class<? extends Throwable>, ThrowingConsumer> handler : failureHandlers.entries()) {
             if(handler.getKey().isInstance(e)) {
@@ -85,12 +99,22 @@ public class SystemFutureCallback<T> implements FutureCallback<T> {
                     handler.getValue().acceptThrows(e);
                     handled = true;
                 } catch(Throwable e1) {
-                    onFailureUnhandled(e1);
+                    handleDefaultFailure(e1);
                 }
             }
         }
         if(!handled) {
-            onFailureUnhandled(e);
+            handleDefaultFailure(e);
+        }
+    }
+
+    private void handleCompletion() {
+        for(ThrowingRunnable<?> handler : completionHandlers) {
+            try {
+                handler.runThrows();
+            } catch(Throwable e) {
+                handleFailure(e);
+            }
         }
     }
 }