Fix user deletion not being correctly processed on SQL backends (#3664)

This commit is contained in:
Luck 2023-06-18 21:38:19 +01:00
parent 5dd808752c
commit 9f1e74fa7c
No known key found for this signature in database
GPG Key ID: EFA9B3EC5FD90F8B
4 changed files with 114 additions and 3 deletions

View File

@ -27,6 +27,8 @@ package me.lucko.luckperms.common.model.nodemap;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableSet;
import me.lucko.luckperms.common.model.manager.group.GroupManager;
import me.lucko.luckperms.common.node.types.Inheritance;
import me.lucko.luckperms.common.util.Difference;
import net.luckperms.api.context.ContextSet;
import net.luckperms.api.context.ImmutableContextSet;
@ -86,6 +88,12 @@ public class RecordedNodeMap implements NodeMap {
}
}
public Difference<Node> addDefaultNodeToChangeSet() {
Difference<Node> diff = new Difference<>();
diff.recordChange(Difference.ChangeType.ADD, Inheritance.builder(GroupManager.DEFAULT_GROUP_NAME).build());
return record(diff);
}
private Difference<Node> record(Difference<Node> result) {
this.lock.lock();
try {

View File

@ -399,6 +399,13 @@ public class SqlStorage implements StorageImplementation {
return true;
});
// if the user only has the default group, delete their data
boolean isDefaultUser = !this.plugin.getUserManager().isNonDefaultUser(user);
if (changes != null && isDefaultUser) {
user.normalData().addDefaultNodeToChangeSet();
changes = null;
}
if (changes == null) {
try (Connection c = this.connectionFactory.getConnection()) {
deleteUser(c, user.getUniqueId());

View File

@ -28,20 +28,31 @@ package me.lucko.luckperms.common.storage;
import com.google.common.collect.ImmutableSet;
import me.lucko.luckperms.common.actionlog.Log;
import me.lucko.luckperms.common.actionlog.LoggedAction;
import me.lucko.luckperms.common.config.ConfigKeys;
import me.lucko.luckperms.common.config.LuckPermsConfiguration;
import me.lucko.luckperms.common.event.EventDispatcher;
import me.lucko.luckperms.common.model.Group;
import me.lucko.luckperms.common.model.PrimaryGroupHolder;
import me.lucko.luckperms.common.model.User;
import me.lucko.luckperms.common.model.manager.group.GroupManager;
import me.lucko.luckperms.common.model.manager.group.StandardGroupManager;
import me.lucko.luckperms.common.model.manager.user.StandardUserManager;
import me.lucko.luckperms.common.model.manager.user.UserManager;
import me.lucko.luckperms.common.node.types.Inheritance;
import me.lucko.luckperms.common.node.types.Permission;
import me.lucko.luckperms.common.plugin.LuckPermsPlugin;
import me.lucko.luckperms.common.plugin.bootstrap.LuckPermsBootstrap;
import me.lucko.luckperms.common.plugin.scheduler.SchedulerAdapter;
import me.lucko.luckperms.common.storage.implementation.sql.SqlStorage;
import me.lucko.luckperms.common.storage.implementation.sql.connection.ConnectionFactory;
import me.lucko.luckperms.common.storage.implementation.sql.connection.file.NonClosableConnection;
import net.luckperms.api.actionlog.Action;
import net.luckperms.api.model.PlayerSaveResult;
import net.luckperms.api.model.PlayerSaveResult.Outcome;
import net.luckperms.api.model.data.DataType;
import net.luckperms.api.node.Node;
import net.luckperms.api.node.types.InheritanceNode;
import net.luckperms.api.node.types.PermissionNode;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@ -58,6 +69,7 @@ import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
@ -66,18 +78,25 @@ import static org.mockito.AdditionalAnswers.answer;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
public class SqlStorageTest {
@Mock private LuckPermsPlugin plugin;
@Mock private LuckPermsBootstrap bootstrap;
@Mock private LuckPermsConfiguration configuration;
private SqlStorage storage;
@BeforeEach
public void setupMocksAndDatabase() throws Exception {
lenient().when(this.plugin.getBootstrap()).thenReturn(this.bootstrap);
lenient().when(this.plugin.getConfiguration()).thenReturn(this.configuration);
lenient().when(this.plugin.getEventDispatcher()).thenReturn(mock(EventDispatcher.class));
lenient().when(this.bootstrap.getScheduler()).thenReturn(mock(SchedulerAdapter.class));
lenient().when(this.configuration.get(ConfigKeys.PRIMARY_GROUP_CALCULATION)).thenReturn(PrimaryGroupHolder.AllParentsByWeight::new);
lenient().when(this.configuration.get(ConfigKeys.PRIMARY_GROUP_CALCULATION_METHOD)).thenReturn("parents-by-weight");
lenient().when(this.bootstrap.getResourceStream(anyString()))
.then(answer((String path) -> SqlStorageTest.class.getClassLoader().getResourceAsStream(path)));
lenient().when(this.plugin.getEventDispatcher()).thenReturn(mock(EventDispatcher.class));
@ -197,6 +216,48 @@ public class SqlStorageTest {
assertEquals(nodes, loaded.normalData().asSet());
}
@Test
public void testSaveAndDeleteUser() throws SQLException {
StandardUserManager userManager = new StandardUserManager(this.plugin);
//noinspection unchecked,rawtypes
when(this.plugin.getUserManager()).thenReturn((UserManager) userManager);
UUID exampleUniqueId = UUID.fromString("069a79f4-44e9-4726-a5be-fca90e38aaf5");
String exampleUsername = "Notch";
PermissionNode examplePermission = Permission.builder()
.permission("test.1")
.withContext("server", "test")
.build();
InheritanceNode defaultGroupNode = Inheritance.builder(GroupManager.DEFAULT_GROUP_NAME).build();
// create a default user, assert that is doesn't appear in unique users list
this.storage.savePlayerData(exampleUniqueId, exampleUsername);
assertFalse(this.storage.getUniqueUsers().contains(exampleUniqueId));
// give the user a node, assert that it does appear in unique users list
User user = this.storage.loadUser(exampleUniqueId, exampleUsername);
user.setNode(DataType.NORMAL, examplePermission, true);
this.storage.saveUser(user);
assertTrue(this.storage.getUniqueUsers().contains(exampleUniqueId));
// clear all nodes (reset to default) and assert that it does not appear in unique users list
user.clearNodes(DataType.NORMAL, null, true);
this.storage.saveUser(user);
assertFalse(this.storage.getUniqueUsers().contains(exampleUniqueId));
assertEquals(ImmutableSet.of(defaultGroupNode), user.normalData().asSet());
// give it a node again, assert that it shows as a unique user
user.setNode(DataType.NORMAL, examplePermission, true);
this.storage.saveUser(user);
assertTrue(this.storage.getUniqueUsers().contains(exampleUniqueId));
assertEquals(ImmutableSet.of(defaultGroupNode, examplePermission), user.normalData().asSet());
// reload user data from the db and assert that it is unchanged
user = this.storage.loadUser(exampleUniqueId, exampleUsername);
assertEquals(ImmutableSet.of(defaultGroupNode, examplePermission), user.normalData().asSet());
}
private static class TestH2ConnectionFactory implements ConnectionFactory {
private final NonClosableConnection connection;

View File

@ -62,7 +62,11 @@ import java.time.ZoneOffset;
import java.util.Map;
import java.util.UUID;
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@Testcontainers
public class StorageIntegrationTest {
@ -119,8 +123,10 @@ public class StorageIntegrationTest {
// try to create / save a user
UUID exampleUniqueId = UUID.fromString("c1d60c50-70b5-4722-8057-87767557e50d");
plugin.getStorage().savePlayerData(exampleUniqueId, "Luck").join();
User user = plugin.getStorage().loadUser(exampleUniqueId, "Luck").join();
String exampleUsername = "Luck";
plugin.getStorage().savePlayerData(exampleUniqueId, exampleUsername).join();
User user = plugin.getStorage().loadUser(exampleUniqueId, exampleUsername).join();
user.setNode(DataType.NORMAL, TEST_PERMISSION_1, true);
user.setNode(DataType.NORMAL, TEST_PERMISSION_2, true);
user.setNode(DataType.NORMAL, TEST_GROUP, true);
@ -137,6 +143,35 @@ public class StorageIntegrationTest {
User testUser = plugin.getStorage().loadUser(exampleUniqueId, null).join();
assertNotNull(testUser);
assertEquals(ImmutableSet.of(Inheritance.builder("default").build(), TEST_PERMISSION_1, TEST_PERMISSION_2, TEST_GROUP, TEST_PREFIX, TEST_META), testUser.normalData().asSet());
assertTrue(exampleUsername.equalsIgnoreCase(testUser.getUsername().orElse("unknown")));
// create another user
UUID otherExampleUniqueId = UUID.fromString("069a79f4-44e9-4726-a5be-fca90e38aaf5");
String otherExampleUsername = "Notch";
plugin.getStorage().savePlayerData(otherExampleUniqueId, otherExampleUsername).join();
assertFalse(plugin.getStorage().getUniqueUsers().join().contains(otherExampleUniqueId));
User otherUser = plugin.getStorage().loadUser(otherExampleUniqueId, otherExampleUsername).join();
otherUser.setNode(DataType.NORMAL, TEST_PERMISSION_1, true);
plugin.getStorage().saveUser(otherUser).join();
assertTrue(plugin.getStorage().getUniqueUsers().join().contains(otherExampleUniqueId));
otherUser.clearNodes(DataType.NORMAL, null, true);
plugin.getStorage().saveUser(otherUser).join();
assertFalse(plugin.getStorage().getUniqueUsers().join().contains(otherExampleUniqueId));
// test uuid/username lookup
assertEquals(otherExampleUniqueId, plugin.getStorage().getPlayerUniqueId(otherExampleUsername).join());
assertTrue(otherExampleUsername.equalsIgnoreCase(plugin.getStorage().getPlayerName(otherExampleUniqueId).join()));
plugin.getStorage().deletePlayerData(otherExampleUniqueId).join();
assertNull(plugin.getStorage().getPlayerUniqueId(otherExampleUsername).join());
assertNull(plugin.getStorage().getPlayerName(otherExampleUniqueId).join());
assertNull(plugin.getStorage().getPlayerUniqueId("example").join());
assertNull(plugin.getStorage().getPlayerName(UUID.randomUUID()).join());
}
@Nested