From e84be5e4de7582bd8191bb2f492e76e2a8095fd9 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 4 Dec 2018 19:45:46 +0100 Subject: [PATCH 1/3] #1702 Consider that Vault permission handler may return null for permission groups --- .../permission/handlers/VaultHandler.java | 15 +++- .../permission/handlers/VaultHandlerTest.java | 80 +++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/permission/handlers/VaultHandlerTest.java diff --git a/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java b/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java index 6e6815fe..dad2fba5 100644 --- a/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java +++ b/src/main/java/fr/xephi/authme/permission/handlers/VaultHandler.java @@ -1,5 +1,6 @@ package fr.xephi.authme.permission.handlers; +import com.google.common.annotations.VisibleForTesting; import fr.xephi.authme.permission.PermissionNode; import fr.xephi.authme.permission.PermissionsSystemType; import net.milkbowl.vault.permission.Permission; @@ -8,6 +9,7 @@ import org.bukkit.Server; import org.bukkit.plugin.RegisteredServiceProvider; import java.util.Arrays; +import java.util.Collections; import java.util.List; /** @@ -24,7 +26,15 @@ public class VaultHandler implements PermissionHandler { this.vaultProvider = getVaultPermission(server); } - private static Permission getVaultPermission(Server server) throws PermissionHandlerException { + /** + * Returns the Vault Permission interface. + * + * @param server the bukkit server instance + * @return the vault permission instance + * @throws PermissionHandlerException if the vault permission instance cannot be retrieved + */ + @VisibleForTesting + Permission getVaultPermission(Server server) throws PermissionHandlerException { // Get the permissions provider service RegisteredServiceProvider permissionProvider = server .getServicesManager().getRegistration(Permission.class); @@ -76,7 +86,8 @@ public class VaultHandler implements PermissionHandler { @Override public List getGroups(OfflinePlayer player) { - return Arrays.asList(vaultProvider.getPlayerGroups(null, player)); + String[] groups = vaultProvider.getPlayerGroups(null, player); + return groups == null ? Collections.emptyList() : Arrays.asList(groups); } @Override diff --git a/src/test/java/fr/xephi/authme/permission/handlers/VaultHandlerTest.java b/src/test/java/fr/xephi/authme/permission/handlers/VaultHandlerTest.java new file mode 100644 index 00000000..006e865d --- /dev/null +++ b/src/test/java/fr/xephi/authme/permission/handlers/VaultHandlerTest.java @@ -0,0 +1,80 @@ +package fr.xephi.authme.permission.handlers; + +import net.milkbowl.vault.permission.Permission; +import org.bukkit.Server; +import org.bukkit.entity.Player; +import org.junit.Test; + +import java.util.List; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.empty; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +/** + * Test for {@link VaultHandler}. + */ +public class VaultHandlerTest { + + private VaultHandlerTestImpl vaultHandlerTest = VaultHandlerTestImpl.create(); + + @Test + public void shouldReturnGroups() { + // given + Permission permissionMock = vaultHandlerTest.permissionMock; + Player player = mock(Player.class); + given(permissionMock.getPlayerGroups(null, player)).willReturn(new String[]{"abc", "test"}); + + // when + List result = vaultHandlerTest.getGroups(player); + + // then + assertThat(result, contains("abc", "test")); + verify(permissionMock).getPlayerGroups(null, player); + } + + /** + * Bug #1702: VaultHandler may return null for groups list. + */ + @Test + public void shouldHandleNullAsGroups() { + // given + Permission permissionMock = vaultHandlerTest.permissionMock; + Player player = mock(Player.class); + given(permissionMock.getPlayerGroups(null, player)).willReturn(null); + + // when + List result = vaultHandlerTest.getGroups(player); + + // then + assertThat(result, empty()); + verify(permissionMock).getPlayerGroups(null, player); + } + + /** Test implementation using a mock Vault Permission instance. */ + private static final class VaultHandlerTestImpl extends VaultHandler { + + private Permission permissionMock; + + VaultHandlerTestImpl() throws PermissionHandlerException { + super(null); + } + + static VaultHandlerTestImpl create() { + try { + return new VaultHandlerTestImpl(); + } catch (PermissionHandlerException e) { + throw new IllegalStateException(e); + } + } + + @Override + Permission getVaultPermission(Server server) { + permissionMock = mock(Permission.class); + return permissionMock; + } + } +} From bce1110b13480f8a7f6f300caa3b871a09b074ed Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 4 Dec 2018 19:48:46 +0100 Subject: [PATCH 2/3] Minor: check returned permission system type in permission handler test --- .../authme/permission/PermissionsManagerInitializationTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/fr/xephi/authme/permission/PermissionsManagerInitializationTest.java b/src/test/java/fr/xephi/authme/permission/PermissionsManagerInitializationTest.java index d2cf5145..8efaa954 100644 --- a/src/test/java/fr/xephi/authme/permission/PermissionsManagerInitializationTest.java +++ b/src/test/java/fr/xephi/authme/permission/PermissionsManagerInitializationTest.java @@ -38,6 +38,7 @@ import static fr.xephi.authme.permission.PermissionsSystemType.LUCK_PERMS; import static fr.xephi.authme.permission.PermissionsSystemType.PERMISSIONS_EX; import static fr.xephi.authme.permission.PermissionsSystemType.VAULT; import static fr.xephi.authme.permission.PermissionsSystemType.Z_PERMISSIONS; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; @@ -89,6 +90,7 @@ public class PermissionsManagerInitializationTest { // then PermissionHandler handler = getHandlerFieldValue(); assertThat(handler, instanceOf(expectedHandlerType)); + assertThat(handler.getPermissionSystem(), equalTo(permissionsSystemType)); } @Test From 136b9e4b6ee0dbde3def4567c8d839484b4e03c7 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 4 Dec 2018 19:51:11 +0100 Subject: [PATCH 3/3] Close #1317 Remove migration of lastlogin column from timestamp to bigint --- .../authme/datasource/MySqlMigrater.java | 40 +------------------ 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/src/main/java/fr/xephi/authme/datasource/MySqlMigrater.java b/src/main/java/fr/xephi/authme/datasource/MySqlMigrater.java index 944691ee..79331be5 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySqlMigrater.java +++ b/src/main/java/fr/xephi/authme/datasource/MySqlMigrater.java @@ -59,49 +59,11 @@ final class MySqlMigrater { columnType = rs.getInt("DATA_TYPE"); } - if (columnType == Types.TIMESTAMP) { - migrateLastLoginColumnFromTimestamp(st, tableName, col); - } else if (columnType == Types.INTEGER) { + if (columnType == Types.INTEGER) { migrateLastLoginColumnFromInt(st, tableName, col); } } - /** - * Performs conversion of lastlogin column from timestamp type to bigint. - * - * @param st Statement object to the database - * @param tableName the table name - * @param col the column names configuration - * @see #477 - */ - private static void migrateLastLoginColumnFromTimestamp(Statement st, String tableName, - Columns col) throws SQLException { - ConsoleLogger.info("Migrating lastlogin column from timestamp to bigint"); - final String lastLoginOld = col.LAST_LOGIN + "_old"; - - // Rename lastlogin to lastlogin_old - String sql = String.format("ALTER TABLE %s CHANGE COLUMN %s %s BIGINT", - tableName, col.LAST_LOGIN, lastLoginOld); - st.execute(sql); - - // Create lastlogin column - sql = String.format("ALTER TABLE %s ADD COLUMN %s " - + "BIGINT NOT NULL DEFAULT 0 AFTER %s", - tableName, col.LAST_LOGIN, col.LAST_IP); - st.execute(sql); - - // Set values of lastlogin based on lastlogin_old - sql = String.format("UPDATE %s SET %s = UNIX_TIMESTAMP(%s) * 1000", - tableName, col.LAST_LOGIN, lastLoginOld); - st.execute(sql); - - // Drop lastlogin_old - sql = String.format("ALTER TABLE %s DROP COLUMN %s", - tableName, lastLoginOld); - st.execute(sql); - ConsoleLogger.info("Finished migration of lastlogin (timestamp to bigint)"); - } - /** * Performs conversion of lastlogin column from int to bigint. *