From 8d5afa7fbcd4c2e56520f37ac6b1cde6a013a2aa Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 23 Feb 2018 23:37:24 +0100 Subject: [PATCH] Minor: Use CommonService for permission lookup - Some changes found in a very old patch :) - drop injection of PermissionsManager in favor of CommonService - Rename IsEqualByReflectionMatcher's method to something more specific to differentiate it better from Hamcrest's equalTo() matcher --- .../authme/process/register/AsyncRegister.java | 5 +---- .../register/executors/EmailRegisterExecutor.java | 6 +----- .../fr/xephi/authme/IsEqualByReflectionMatcher.java | 4 ++-- src/test/java/fr/xephi/authme/api/NewAPITest.java | 6 +++--- .../java/fr/xephi/authme/api/v3/AuthMeApiTest.java | 6 +++--- .../executable/register/RegisterCommandTest.java | 12 ++++++------ .../authme/process/register/AsyncRegisterTest.java | 3 --- ...iderTest.java => EmailRegisterExecutorTest.java} | 13 +++++-------- .../fr/xephi/authme/util/ExceptionUtilsTest.java | 7 +++++++ 9 files changed, 28 insertions(+), 34 deletions(-) rename src/test/java/fr/xephi/authme/process/register/executors/{EmailRegisterExecutorProviderTest.java => EmailRegisterExecutorTest.java} (91%) diff --git a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java index a370b1a94..d50fe9a32 100644 --- a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java +++ b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java @@ -5,7 +5,6 @@ import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.message.MessageKey; -import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.process.AsynchronousProcess; import fr.xephi.authme.process.register.executors.RegistrationExecutor; import fr.xephi.authme.process.register.executors.RegistrationMethod; @@ -35,8 +34,6 @@ public class AsyncRegister implements AsynchronousProcess { @Inject private CommonService service; @Inject - private PermissionsManager permissionsManager; - @Inject private SingletonStore registrationExecutorFactory; @Inject private BungeeSender bungeeSender; @@ -106,7 +103,7 @@ public class AsyncRegister implements AsynchronousProcess { if (maxRegPerIp > 0 && !"127.0.0.1".equalsIgnoreCase(ip) && !"localhost".equalsIgnoreCase(ip) - && !permissionsManager.hasPermission(player, ALLOW_MULTIPLE_ACCOUNTS)) { + && !service.hasPermission(player, ALLOW_MULTIPLE_ACCOUNTS)) { List otherAccounts = database.getAllAuthsByIp(ip); if (otherAccounts.size() >= maxRegPerIp) { service.send(player, MessageKey.MAX_REGISTER_EXCEEDED, Integer.toString(maxRegPerIp), diff --git a/src/main/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutor.java b/src/main/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutor.java index 0f4153b18..c344447c8 100644 --- a/src/main/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutor.java +++ b/src/main/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutor.java @@ -4,7 +4,6 @@ import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.mail.EmailService; import fr.xephi.authme.message.MessageKey; -import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.process.SyncProcessManager; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.crypts.HashedPassword; @@ -25,9 +24,6 @@ import static fr.xephi.authme.settings.properties.EmailSettings.RECOVERY_PASSWOR */ class EmailRegisterExecutor implements RegistrationExecutor { - @Inject - private PermissionsManager permissionsManager; - @Inject private DataSource dataSource; @@ -46,7 +42,7 @@ class EmailRegisterExecutor implements RegistrationExecutor @Override public boolean isRegistrationAdmitted(EmailRegisterParams params) { final int maxRegPerEmail = commonService.getProperty(EmailSettings.MAX_REG_PER_EMAIL); - if (maxRegPerEmail > 0 && !permissionsManager.hasPermission(params.getPlayer(), ALLOW_MULTIPLE_ACCOUNTS)) { + if (maxRegPerEmail > 0 && !commonService.hasPermission(params.getPlayer(), ALLOW_MULTIPLE_ACCOUNTS)) { int otherAccounts = dataSource.countAuthsByEmail(params.getEmail()); if (otherAccounts >= maxRegPerEmail) { commonService.send(params.getPlayer(), MessageKey.MAX_REGISTER_EXCEEDED, diff --git a/src/test/java/fr/xephi/authme/IsEqualByReflectionMatcher.java b/src/test/java/fr/xephi/authme/IsEqualByReflectionMatcher.java index a6033ffd3..55df85579 100644 --- a/src/test/java/fr/xephi/authme/IsEqualByReflectionMatcher.java +++ b/src/test/java/fr/xephi/authme/IsEqualByReflectionMatcher.java @@ -31,7 +31,7 @@ public final class IsEqualByReflectionMatcher extends TypeSafeMatcher { * @param the object's type * @return the matcher for the expected object */ - public static Matcher isEqualTo(T expected) { + public static Matcher hasEqualValuesOnAllFields(T expected) { return new IsEqualByReflectionMatcher<>(expected); } @@ -70,7 +70,7 @@ public final class IsEqualByReflectionMatcher extends TypeSafeMatcher { Class currentClass = object.getClass(); while (currentClass != null) { for (Field f : currentClass.getDeclaredFields()) { - if (!Modifier.isStatic(f.getModifiers())) { + if (!Modifier.isStatic(f.getModifiers()) && !f.isSynthetic()) { fields.add(f); } } diff --git a/src/test/java/fr/xephi/authme/api/NewAPITest.java b/src/test/java/fr/xephi/authme/api/NewAPITest.java index 15440e3a6..92986c5ee 100644 --- a/src/test/java/fr/xephi/authme/api/NewAPITest.java +++ b/src/test/java/fr/xephi/authme/api/NewAPITest.java @@ -27,7 +27,7 @@ import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; -import static fr.xephi.authme.IsEqualByReflectionMatcher.isEqualTo; +import static fr.xephi.authme.IsEqualByReflectionMatcher.hasEqualValuesOnAllFields; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -305,7 +305,7 @@ public class NewAPITest { // then verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION), - argThat(isEqualTo(ApiPasswordRegisterParams.of(player, pass, true)))); + argThat(hasEqualValuesOnAllFields(ApiPasswordRegisterParams.of(player, pass, true)))); } @Test @@ -319,7 +319,7 @@ public class NewAPITest { // then verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION), - argThat(isEqualTo(ApiPasswordRegisterParams.of(player, pass, false)))); + argThat(hasEqualValuesOnAllFields(ApiPasswordRegisterParams.of(player, pass, false)))); } @Test diff --git a/src/test/java/fr/xephi/authme/api/v3/AuthMeApiTest.java b/src/test/java/fr/xephi/authme/api/v3/AuthMeApiTest.java index 433675f9c..6021b3a92 100644 --- a/src/test/java/fr/xephi/authme/api/v3/AuthMeApiTest.java +++ b/src/test/java/fr/xephi/authme/api/v3/AuthMeApiTest.java @@ -30,7 +30,7 @@ import java.util.List; import java.util.stream.Collectors; import java.time.Instant; -import static fr.xephi.authme.IsEqualByReflectionMatcher.isEqualTo; +import static fr.xephi.authme.IsEqualByReflectionMatcher.hasEqualValuesOnAllFields; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -427,7 +427,7 @@ public class AuthMeApiTest { // then verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION), - argThat(isEqualTo(ApiPasswordRegisterParams.of(player, pass, true)))); + argThat(hasEqualValuesOnAllFields(ApiPasswordRegisterParams.of(player, pass, true)))); } @Test @@ -441,7 +441,7 @@ public class AuthMeApiTest { // then verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION), - argThat(isEqualTo(ApiPasswordRegisterParams.of(player, pass, false)))); + argThat(hasEqualValuesOnAllFields(ApiPasswordRegisterParams.of(player, pass, false)))); } @Test diff --git a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java index a4d8f7aa7..48b13c931 100644 --- a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java @@ -30,7 +30,7 @@ import org.mockito.junit.MockitoJUnitRunner; import java.util.Arrays; import java.util.Collections; -import static fr.xephi.authme.IsEqualByReflectionMatcher.isEqualTo; +import static fr.xephi.authme.IsEqualByReflectionMatcher.hasEqualValuesOnAllFields; import static org.hamcrest.Matchers.containsString; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -101,7 +101,7 @@ public class RegisterCommandTest { // then verify(registrationCaptchaManager).isCaptchaRequired("test2"); verify(management).performRegister(eq(RegistrationMethod.TWO_FACTOR_REGISTRATION), - argThat(isEqualTo(TwoFactorRegisterParams.of(player)))); + argThat(hasEqualValuesOnAllFields(TwoFactorRegisterParams.of(player)))); verifyZeroInteractions(emailService); } @@ -221,7 +221,7 @@ public class RegisterCommandTest { verify(validationService).validateEmail(playerMail); verify(emailService).hasAllInformation(); verify(management).performRegister(eq(RegistrationMethod.EMAIL_REGISTRATION), - argThat(isEqualTo(EmailRegisterParams.of(player, playerMail)))); + argThat(hasEqualValuesOnAllFields(EmailRegisterParams.of(player, playerMail)))); } @Test @@ -250,7 +250,7 @@ public class RegisterCommandTest { // then verify(registrationCaptchaManager).isCaptchaRequired("newPlayer"); verify(management).performRegister(eq(RegistrationMethod.PASSWORD_REGISTRATION), - argThat(isEqualTo(PasswordRegisterParams.of(player, "myPass", null)))); + argThat(hasEqualValuesOnAllFields(PasswordRegisterParams.of(player, "myPass", null)))); } @Test @@ -268,7 +268,7 @@ public class RegisterCommandTest { // then verify(validationService).validateEmail(email); verify(management).performRegister(eq(RegistrationMethod.PASSWORD_REGISTRATION), - argThat(isEqualTo(PasswordRegisterParams.of(player, "myPass", email)))); + argThat(hasEqualValuesOnAllFields(PasswordRegisterParams.of(player, "myPass", email)))); } @Test @@ -303,7 +303,7 @@ public class RegisterCommandTest { // then verify(registrationCaptchaManager).isCaptchaRequired("Doa"); verify(management).performRegister(eq(RegistrationMethod.PASSWORD_REGISTRATION), - argThat(isEqualTo(PasswordRegisterParams.of(player, "myPass", null)))); + argThat(hasEqualValuesOnAllFields(PasswordRegisterParams.of(player, "myPass", null)))); } @Test diff --git a/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java b/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java index e044a754a..77f91ca88 100644 --- a/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java +++ b/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java @@ -5,7 +5,6 @@ import fr.xephi.authme.TestHelper; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.message.MessageKey; -import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.process.register.executors.PasswordRegisterParams; import fr.xephi.authme.process.register.executors.RegistrationExecutor; import fr.xephi.authme.process.register.executors.RegistrationMethod; @@ -39,8 +38,6 @@ public class AsyncRegisterTest { @Mock private PlayerCache playerCache; @Mock - private PermissionsManager permissionsManager; - @Mock private CommonService commonService; @Mock private DataSource dataSource; diff --git a/src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorProviderTest.java b/src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorTest.java similarity index 91% rename from src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorProviderTest.java rename to src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorTest.java index a04aa7130..290566bcd 100644 --- a/src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorProviderTest.java +++ b/src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorTest.java @@ -5,7 +5,6 @@ import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.mail.EmailService; import fr.xephi.authme.message.MessageKey; -import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.process.SyncProcessManager; import fr.xephi.authme.security.PasswordSecurity; @@ -35,13 +34,11 @@ import static org.mockito.Mockito.verifyZeroInteractions; * Test for {@link EmailRegisterExecutor}. */ @RunWith(MockitoJUnitRunner.class) -public class EmailRegisterExecutorProviderTest { +public class EmailRegisterExecutorTest { @InjectMocks private EmailRegisterExecutor executor; - @Mock - private PermissionsManager permissionsManager; @Mock private DataSource dataSource; @Mock @@ -68,7 +65,7 @@ public class EmailRegisterExecutorProviderTest { // then assertThat(result, equalTo(false)); verify(dataSource).countAuthsByEmail(email); - verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); + verify(commonService).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); verify(commonService).send(player, MessageKey.MAX_REGISTER_EXCEEDED, "3", "4", "@"); } @@ -77,7 +74,7 @@ public class EmailRegisterExecutorProviderTest { // given given(commonService.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(3); Player player = mock(Player.class); - given(permissionsManager.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(true); + given(commonService.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(true); EmailRegisterParams params = EmailRegisterParams.of(player, "test@example.com"); // when @@ -85,7 +82,7 @@ public class EmailRegisterExecutorProviderTest { // then assertThat(result, equalTo(true)); - verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); + verify(commonService).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); } @Test @@ -102,7 +99,7 @@ public class EmailRegisterExecutorProviderTest { // then assertThat(result, equalTo(true)); - verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); + verify(commonService).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); verify(dataSource).countAuthsByEmail(email); } diff --git a/src/test/java/fr/xephi/authme/util/ExceptionUtilsTest.java b/src/test/java/fr/xephi/authme/util/ExceptionUtilsTest.java index af154537e..8685d7f33 100644 --- a/src/test/java/fr/xephi/authme/util/ExceptionUtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/ExceptionUtilsTest.java @@ -1,6 +1,7 @@ package fr.xephi.authme.util; import fr.xephi.authme.ReflectionTestUtils; +import fr.xephi.authme.TestHelper; import org.junit.Test; import java.util.ConcurrentModificationException; @@ -51,4 +52,10 @@ public class ExceptionUtilsTest { assertThat(resultNpe, nullValue()); assertThat(resultUoe, sameInstance(uoe)); } + + @Test + public void shouldHaveHiddenConstructor() { + // given / when / then + TestHelper.validateHasOnlyPrivateEmptyConstructor(ExceptionUtils.class); + } }