Fix updating primary groups for players who've never joined the server with SQL storage types (#500)

bukkit/src/main/java/me/lucko/luckperms/bukkit/migration/MigrationPermissionsEx.java

@@ -143,6 +143,10 @@ public class MigrationPermissionsEx extends SubCommand<Object> {
                 continue;
             }
 
+            // save uuid data for the user
+            plugin.getStorage().saveUUIDData(u, user.getName());
+
+            // load in a user instance
             plugin.getStorage().loadUser(u, user.getName()).join();
             User lpUser = plugin.getUserManager().getIfLoaded(u);
 

bukkit/src/main/java/me/lucko/luckperms/bukkit/migration/MigrationZPermissions.java

@@ -168,6 +168,7 @@ public class MigrationZPermissions extends SubCommand<Object> {
                 username = entity.getDisplayName();
             }
 
+            plugin.getStorage().saveUUIDData(u, String.valueOf(username)).join();
             plugin.getStorage().loadUser(u, username).join();
             User user = plugin.getUserManager().getIfLoaded(u);
 

bungee/src/main/java/me/lucko/luckperms/bungee/migration/MigrationBungeePerms.java

@@ -113,6 +113,7 @@ public class MigrationBungeePerms extends SubCommand<Object> {
             }
 
             // Make a LuckPerms user for the one being migrated.
+            plugin.getStorage().saveUUIDData(u.getUUID(), u.getName()).join();
             plugin.getStorage().loadUser(u.getUUID(), u.getName()).join();
             me.lucko.luckperms.common.model.User user = plugin.getUserManager().getIfLoaded(u.getUUID());
 

common/src/main/java/me/lucko/luckperms/common/api/delegates/StorageDelegate.java

@@ -167,7 +167,7 @@ public class StorageDelegate implements Storage {
 
     @Override
     public CompletableFuture<Boolean> saveUUIDData(@NonNull String username, @NonNull UUID uuid) {
-        return handle.noBuffer().saveUUIDData(checkUsername(username), uuid);
+        return handle.noBuffer().saveUUIDData(uuid, checkUsername(username));
     }
 
     @Override

common/src/main/java/me/lucko/luckperms/common/backup/Exporter.java

@@ -233,8 +233,8 @@ public class Exporter implements Runnable {
                                 output.add("/lp" + NodeFactory.nodeAsCommand(node, user.getUuid().toString(), false, true));
                             }
 
-                            if (!user.getPrimaryGroup().getStoredValue().equalsIgnoreCase("default")) {
-                                output.add("/lp user " + user.getUuid().toString() + " switchprimarygroup " + user.getPrimaryGroup().getStoredValue());
+                            if (!user.getPrimaryGroup().getStoredValue().orElse("default").equalsIgnoreCase("default")) {
+                                output.add("/lp user " + user.getUuid().toString() + " switchprimarygroup " + user.getPrimaryGroup().getStoredValue().get());
                             }
 
                             if (!inDefault) {

common/src/main/java/me/lucko/luckperms/common/commands/impl/generic/parent/ParentRemove.java

@@ -82,7 +82,7 @@ public class ParentRemove extends SharedSubCommand {
             boolean shouldPrevent = plugin.getConfiguration().get(ConfigKeys.PREVENT_PRIMARY_GROUP_REMOVAL) &&
                     context.isEmpty() &&
                     plugin.getConfiguration().get(ConfigKeys.PRIMARY_GROUP_CALCULATION_METHOD).equals("stored") &&
-                    user.getPrimaryGroup().getStoredValue().equalsIgnoreCase(groupName);
+                    user.getPrimaryGroup().getStoredValue().orElse("default").equalsIgnoreCase(groupName);
 
             if (shouldPrevent) {
                 Message.USER_REMOVEGROUP_ERROR_PRIMARY.send(sender);

common/src/main/java/me/lucko/luckperms/common/commands/impl/user/UserDemote.java

@@ -156,7 +156,7 @@ public class UserDemote extends SubCommand<User> {
         user.unsetPermission(oldNode);
         user.setPermission(NodeFactory.newBuilder("group." + previousGroup.getName()).withExtraContext(context).build());
 
-        if (context.isEmpty() && user.getPrimaryGroup().getStoredValue().equalsIgnoreCase(old)) {
+        if (context.isEmpty() && user.getPrimaryGroup().getStoredValue().orElse("default").equalsIgnoreCase(old)) {
             user.getPrimaryGroup().setStoredValue(previousGroup.getName());
         }
 

common/src/main/java/me/lucko/luckperms/common/commands/impl/user/UserPromote.java

@@ -167,7 +167,7 @@ public class UserPromote extends SubCommand<User> {
         user.unsetPermission(oldNode);
         user.setPermission(NodeFactory.newBuilder("group." + nextGroup.getName()).withExtraContext(context).build());
 
-        if (context.isEmpty() && user.getPrimaryGroup().getStoredValue().equalsIgnoreCase(old)) {
+        if (context.isEmpty() && user.getPrimaryGroup().getStoredValue().orElse("default").equalsIgnoreCase(old)) {
             user.getPrimaryGroup().setStoredValue(nextGroup.getName());
         }
 

common/src/main/java/me/lucko/luckperms/common/commands/impl/user/UserSwitchPrimaryGroup.java

@@ -67,7 +67,7 @@ public class UserSwitchPrimaryGroup extends SubCommand<User> {
             return CommandResult.INVALID_ARGS;
         }
 
-        if (user.getPrimaryGroup().getStoredValue().equalsIgnoreCase(group.getName())) {
+        if (user.getPrimaryGroup().getStoredValue().orElse("default").equalsIgnoreCase(group.getName())) {
             Message.USER_PRIMARYGROUP_ERROR_ALREADYHAS.send(sender, user.getFriendlyName(), group.getFriendlyName());
             return CommandResult.STATE_ERROR;
         }

common/src/main/java/me/lucko/luckperms/common/managers/GenericUserManager.java

@@ -122,7 +122,7 @@ public class GenericUserManager extends AbstractManager<UserIdentifier, User> im
     public static boolean giveDefaultIfNeeded(User user, boolean save, LuckPermsPlugin plugin) {
         boolean hasGroup = false;
 
-        if (user.getPrimaryGroup().getStoredValue() != null && !user.getPrimaryGroup().getStoredValue().isEmpty()) {
+        if (user.getPrimaryGroup().getStoredValue().isPresent()) {
             for (Node node : user.getEnduringNodes().values()) {
                 if (node.hasSpecificContext()) {
                     continue;
@@ -177,6 +177,6 @@ public class GenericUserManager extends AbstractManager<UserIdentifier, User> im
         }
 
         // Not in the default primary group
-        return !user.getPrimaryGroup().getStoredValue().equalsIgnoreCase("default");
+        return !user.getPrimaryGroup().getStoredValue().orElse("default").equalsIgnoreCase("default");
     }
 }

common/src/main/java/me/lucko/luckperms/common/primarygroup/CachedPrimaryGroupHolder.java

@@ -52,7 +52,7 @@ public abstract class CachedPrimaryGroupHolder extends StoredHolder implements S
     @Override
     public String getValue() {
         String s = cache.get();
-        return s != null ? s : getStoredValue();
+        return s != null ? s : getStoredValue().orElse("default");
     }
 
     @Override

common/src/main/java/me/lucko/luckperms/common/primarygroup/GroupInheritanceComparator.java

@@ -60,8 +60,8 @@ public class GroupInheritanceComparator implements Comparator<Group> {
 
         // failing differing group weights, check if one of the groups is a primary group
         if (origin != null) {
-            boolean o1Primary = o1.getName().equalsIgnoreCase(origin.getPrimaryGroup().getStoredValue());
-            boolean o2Primary = o2.getName().equalsIgnoreCase(origin.getPrimaryGroup().getStoredValue());
+            boolean o1Primary = o1.getName().equalsIgnoreCase(origin.getPrimaryGroup().getStoredValue().orElse("default"));
+            boolean o2Primary = o2.getName().equalsIgnoreCase(origin.getPrimaryGroup().getStoredValue().orElse("default"));
 
             // one of them is a primary group, and therefore has priority
             if (o1Primary != o2Primary) {

common/src/main/java/me/lucko/luckperms/common/primarygroup/PrimaryGroupHolder.java

@@ -25,6 +25,8 @@
 
 package me.lucko.luckperms.common.primarygroup;
 
+import java.util.Optional;
+
 /**
  * Calculates and caches a User's "primary group"
  */
@@ -42,7 +44,7 @@ public interface PrimaryGroupHolder {
      *
      * @return the stored value
      */
-    String getStoredValue();
+    Optional<String> getStoredValue();
 
     /**
      * Sets the primary group which is stored against the user's data.

common/src/main/java/me/lucko/luckperms/common/primarygroup/StoredHolder.java

@@ -25,11 +25,12 @@
 
 package me.lucko.luckperms.common.primarygroup;
 
-import lombok.Getter;
 import lombok.NonNull;
 
 import me.lucko.luckperms.common.model.User;
 
+import java.util.Optional;
+
 /**
  * Simple implementation of {@link PrimaryGroupHolder}, which just returns the stored value.
  */
@@ -37,7 +38,6 @@ public class StoredHolder implements PrimaryGroupHolder {
 
     protected final User user;
 
-    @Getter
     private String storedValue = null;
 
     public StoredHolder(@NonNull User user) {
@@ -48,7 +48,16 @@ public class StoredHolder implements PrimaryGroupHolder {
         return storedValue;
     }
 
+    @Override
+    public Optional<String> getStoredValue() {
+        return Optional.ofNullable(storedValue);
+    }
+
     public void setStoredValue(String storedValue) {
-        this.storedValue = storedValue == null ? null : storedValue.toLowerCase();
+        if (storedValue == null || storedValue.isEmpty()) {
+            this.storedValue = null;
+        } else {
+            this.storedValue = storedValue.toLowerCase();
+        }
     }
 }

common/src/main/java/me/lucko/luckperms/common/storage/AbstractStorage.java

@@ -246,8 +246,8 @@ public class AbstractStorage implements Storage {
     }
 
     @Override
-    public CompletableFuture<Boolean> saveUUIDData(String username, UUID uuid) {
-        return makeFuture(() -> backing.saveUUIDData(username, uuid));
+    public CompletableFuture<Boolean> saveUUIDData(UUID uuid, String username) {
+        return makeFuture(() -> backing.saveUUIDData(uuid, username));
     }
 
     @Override

common/src/main/java/me/lucko/luckperms/common/storage/SplitBacking.java

@@ -181,8 +181,8 @@ public class SplitBacking extends AbstractBacking {
     }
 
     @Override
-    public boolean saveUUIDData(String username, UUID uuid) {
-        return backing.get(types.get("uuid")).saveUUIDData(username, uuid);
+    public boolean saveUUIDData(UUID uuid, String username) {
+        return backing.get(types.get("uuid")).saveUUIDData(uuid, username);
     }
 
     @Override

common/src/main/java/me/lucko/luckperms/common/storage/Storage.java

@@ -99,7 +99,7 @@ public interface Storage {
 
     CompletableFuture<Boolean> deleteTrack(Track track, DeletionCause cause);
 
-    CompletableFuture<Boolean> saveUUIDData(String username, UUID uuid);
+    CompletableFuture<Boolean> saveUUIDData(UUID uuid, String username);
 
     CompletableFuture<UUID> getUUID(String username);
 

common/src/main/java/me/lucko/luckperms/common/storage/backing/AbstractBacking.java

@@ -102,7 +102,7 @@ public abstract class AbstractBacking {
 
     public abstract boolean deleteTrack(Track track);
 
-    public abstract boolean saveUUIDData(String username, UUID uuid);
+    public abstract boolean saveUUIDData(UUID uuid, String username);
 
     public abstract UUID getUUID(String username);
 

common/src/main/java/me/lucko/luckperms/common/storage/backing/file/FileUuidCache.java

@@ -51,10 +51,10 @@ public class FileUuidCache {
     /**
      * Adds a mapping to the cache
      *
-     * @param username the username of the player
      * @param uuid the uuid of the player
+     * @param username the username of the player
      */
-    public void addMapping(String username, UUID uuid) {
+    public void addMapping(UUID uuid, String username) {
         lookupMap.put(username.toLowerCase(), Maps.immutableEntry(uuid, DateUtil.unixSecondsNow()));
     }
 

common/src/main/java/me/lucko/luckperms/common/storage/backing/file/FlatfileBacking.java

@@ -317,8 +317,8 @@ public abstract class FlatfileBacking extends AbstractBacking {
     }
 
     @Override
-    public boolean saveUUIDData(String username, UUID uuid) {
-        uuidCache.addMapping(username, uuid);
+    public boolean saveUUIDData(UUID uuid, String username) {
+        uuidCache.addMapping(uuid, username);
         return true;
     }
 

common/src/main/java/me/lucko/luckperms/common/storage/backing/file/JSONBacking.java

@@ -220,7 +220,7 @@ public class JSONBacking extends FlatfileBacking {
                 JsonObject data = new JsonObject();
                 data.addProperty("uuid", user.getUuid().toString());
                 data.addProperty("name", user.getName().orElse("null"));
-                data.addProperty("primaryGroup", user.getPrimaryGroup().getStoredValue());
+                data.addProperty("primaryGroup", user.getPrimaryGroup().getStoredValue().orElse("default"));
 
                 Set<NodeModel> nodes = user.getEnduringNodes().values().stream().map(NodeModel::fromNode).collect(Collectors.toCollection(LinkedHashSet::new));
                 data.add("permissions", serializePermissions(nodes));

common/src/main/java/me/lucko/luckperms/common/storage/backing/file/YAMLBacking.java

@@ -222,7 +222,7 @@ public class YAMLBacking extends FlatfileBacking {
                 Map<String, Object> values = new LinkedHashMap<>();
                 values.put("uuid", user.getUuid().toString());
                 values.put("name", user.getName().orElse("null"));
-                values.put("primary-group", user.getPrimaryGroup().getStoredValue());
+                values.put("primary-group", user.getPrimaryGroup().getStoredValue().orElse("default"));
 
                 Set<NodeModel> data = user.getEnduringNodes().values().stream().map(NodeModel::fromNode).collect(Collectors.toCollection(LinkedHashSet::new));
                 values.put("permissions", serializePermissions(data));

common/src/main/java/me/lucko/luckperms/common/storage/backing/mongodb/MongoDBBacking.java

@@ -657,7 +657,7 @@ public class MongoDBBacking extends AbstractBacking {
     }
 
     @Override
-    public boolean saveUUIDData(String username, UUID uuid) {
+    public boolean saveUUIDData(UUID uuid, String username) {
         return call(() -> {
             MongoCollection<Document> c = database.getCollection(prefix + "uuid");
 
@@ -721,7 +721,7 @@ public class MongoDBBacking extends AbstractBacking {
     private static Document fromUser(User user) {
         Document main = new Document("_id", user.getUuid())
                 .append("name", user.getName().orElse("null"))
-                .append("primaryGroup", user.getPrimaryGroup().getStoredValue());
+                .append("primaryGroup", user.getPrimaryGroup().getStoredValue().orElse("default"));
 
         Document perms = new Document();
         for (Map.Entry<String, Boolean> e : convert(exportToLegacy(user.getEnduringNodes().values())).entrySet()) {

common/src/main/java/me/lucko/luckperms/common/storage/backing/sql/SQLBacking.java

@@ -85,6 +85,7 @@ public class SQLBacking extends AbstractBacking {
     private static final String PLAYER_SELECT = "SELECT username, primary_group FROM {prefix}players WHERE uuid=?";
     private static final String PLAYER_SELECT_UUID = "SELECT uuid FROM {prefix}players WHERE username=? LIMIT 1";
     private static final String PLAYER_SELECT_USERNAME = "SELECT username FROM {prefix}players WHERE uuid=? LIMIT 1";
+    private static final String PLAYER_SELECT_PRIMARY_GROUP = "SELECT primary_group FROM {prefix}players WHERE uuid=? LIMIT 1";
     private static final String PLAYER_INSERT = "INSERT INTO {prefix}players VALUES(?, ?, ?)";
     private static final String PLAYER_UPDATE = "UPDATE {prefix}players SET username=? WHERE uuid=?";
     private static final String PLAYER_DELETE = "DELETE FROM {prefix}players WHERE username=? AND NOT uuid=?";
@@ -479,11 +480,32 @@ public class SQLBacking extends AbstractBacking {
             }
 
             try (Connection c = provider.getConnection()) {
-                try (PreparedStatement ps = c.prepareStatement(prefix.apply(PLAYER_UPDATE_PRIMARY_GROUP))) {
-                    ps.setString(1, user.getPrimaryGroup().getStoredValue() == null ? "default" : user.getPrimaryGroup().getStoredValue());
-                    ps.setString(2, user.getUuid().toString());
-                    ps.execute();
+                boolean hasPrimaryGroupSaved;
+
+                try (PreparedStatement ps = c.prepareStatement(prefix.apply(PLAYER_SELECT_PRIMARY_GROUP))) {
+                    ps.setString(1, user.getUuid().toString());
+                    try (ResultSet rs = ps.executeQuery()) {
+                        hasPrimaryGroupSaved = rs.next();
+                    }
                 }
+
+                if (hasPrimaryGroupSaved) {
+                    // update
+                    try (PreparedStatement ps = c.prepareStatement(prefix.apply(PLAYER_UPDATE_PRIMARY_GROUP))) {
+                        ps.setString(1, user.getPrimaryGroup().getStoredValue().orElse("default"));
+                        ps.setString(2, user.getUuid().toString());
+                        ps.execute();
+                    }
+                } else {
+                    // insert
+                    try (PreparedStatement ps = c.prepareStatement(prefix.apply(PLAYER_INSERT))) {
+                        ps.setString(1, user.getUuid().toString());
+                        ps.setString(2, user.getName().orElse("null"));
+                        ps.setString(3, user.getPrimaryGroup().getStoredValue().orElse("default"));
+                        ps.execute();
+                    }
+                }
+
             } catch (SQLException e) {
                 e.printStackTrace();
                 return false;
@@ -967,7 +989,7 @@ public class SQLBacking extends AbstractBacking {
     }
 
     @Override
-    public boolean saveUUIDData(String username, UUID uuid) {
+    public boolean saveUUIDData(UUID uuid, String username) {
         final String u = username.toLowerCase();
         AtomicReference<String> remoteUserName = new AtomicReference<>(null);
 

common/src/main/java/me/lucko/luckperms/common/storage/wrappings/PhasedStorage.java

@@ -262,10 +262,10 @@ public class PhasedStorage implements Storage {
     }
 
     @Override
-    public CompletableFuture<Boolean> saveUUIDData(String username, UUID uuid) {
+    public CompletableFuture<Boolean> saveUUIDData(UUID uuid, String username) {
         phaser.register();
         try {
-            return backing.saveUUIDData(username, uuid);
+            return backing.saveUUIDData(uuid, username);
         } finally {
             phaser.arriveAndDeregister();
         }

common/src/main/java/me/lucko/luckperms/common/utils/LoginHelper.java

@@ -53,7 +53,7 @@ public class LoginHelper {
                 // No previous data for this player
                 plugin.getApiProvider().getEventFactory().handleUserFirstLogin(u, username);
                 cache.addToCache(u, u);
-                CompletableFuture<Boolean> future = plugin.getStorage().noBuffer().saveUUIDData(username, u);
+                CompletableFuture<Boolean> future = plugin.getStorage().noBuffer().saveUUIDData(u, username);
                 if (joinUuidSave) {
                     future.join();
                 }
@@ -65,7 +65,7 @@ public class LoginHelper {
             }
 
             // Online mode, no cache needed. This is just for name -> uuid lookup.
-            CompletableFuture<Boolean> future = plugin.getStorage().noBuffer().saveUUIDData(username, u);
+            CompletableFuture<Boolean> future = plugin.getStorage().noBuffer().saveUUIDData(u, username);
             if (joinUuidSave) {
                 future.join();
             }