- Create command under /authme debug that allows to change the 'nullable' status of MySQL columns (currently last date and email only) - We need to offer a default value for forum integrations that have a NOT NULL email column. Offering a command avoids us from force-migrating existing databases while still offering migrations in both directions - Change in default value handling: lack of values are not handled by setting default values to the PlayerAuth anymore, and reading a default value from the database into a PlayerAuth will be translated into null by the PlayerAuth builder - When a new database is created, email and lastlogin are now nullable and lack a default a value Open points: - Finish MySqlDefaultChangerTest - Revise purging logic (#792) - Allow to have more columns nullable (#814)
This commit is contained in:
@@ -162,20 +162,36 @@ public class AuthMeApiTest {
|
||||
public void shouldGetLastLogin() {
|
||||
// given
|
||||
String name = "David";
|
||||
Player player = mockPlayerWithName(name);
|
||||
PlayerAuth auth = PlayerAuth.builder().name(name)
|
||||
.lastLogin(1501597979)
|
||||
.lastLogin(1501597979L)
|
||||
.build();
|
||||
given(playerCache.getAuth(name)).willReturn(auth);
|
||||
|
||||
// when
|
||||
Date result = api.getLastLogin(player.getName());
|
||||
Date result = api.getLastLogin(name);
|
||||
|
||||
// then
|
||||
assertThat(result, not(nullValue()));
|
||||
assertThat(result, equalTo(new Date(1501597979)));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldHandleNullLastLogin() {
|
||||
// given
|
||||
String name = "John";
|
||||
PlayerAuth auth = PlayerAuth.builder().name(name)
|
||||
.lastLogin(null)
|
||||
.build();
|
||||
given(dataSource.getAuth(name)).willReturn(auth);
|
||||
|
||||
// when
|
||||
Date result = api.getLastLogin(name);
|
||||
|
||||
// then
|
||||
assertThat(result, nullValue());
|
||||
verify(dataSource).getAuth(name);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldReturnNullForUnavailablePlayer() {
|
||||
// given
|
||||
|
||||
@@ -111,4 +111,25 @@ public class LastLoginCommandTest {
|
||||
assertThat(captor.getAllValues().get(2), containsString("123.45.66.77"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldHandleNullLastLoginDate() {
|
||||
// given
|
||||
String name = "player";
|
||||
PlayerAuth auth = PlayerAuth.builder()
|
||||
.name(name)
|
||||
.lastIp("123.45.67.89")
|
||||
.build();
|
||||
given(dataSource.getAuth(name)).willReturn(auth);
|
||||
CommandSender sender = mock(CommandSender.class);
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Collections.singletonList(name));
|
||||
|
||||
// then
|
||||
verify(dataSource).getAuth(name);
|
||||
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
|
||||
verify(sender, times(2)).sendMessage(captor.capture());
|
||||
assertThat(captor.getAllValues().get(0), allOf(containsString(name), containsString("never")));
|
||||
assertThat(captor.getAllValues().get(1), containsString("123.45.67.89"));
|
||||
}
|
||||
}
|
||||
|
||||
+69
@@ -0,0 +1,69 @@
|
||||
package fr.xephi.authme.command.executable.authme.debug;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
/**
|
||||
* Consistency test for {@link MySqlDefaultChanger.Columns} enum.
|
||||
*/
|
||||
public class MySqlDefaultChangerColumnsTest {
|
||||
|
||||
@Test
|
||||
public void shouldAllHaveDifferentNameProperty() {
|
||||
// given
|
||||
Set<String> properties = new HashSet<>();
|
||||
|
||||
// when / then
|
||||
for (MySqlDefaultChanger.Columns col : MySqlDefaultChanger.Columns.values()) {
|
||||
if (!properties.add(col.columnName.getPath())) {
|
||||
fail("Column '" + col + "' has a column name property path that was already encountered: "
|
||||
+ col.columnName.getPath());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldHaveMatchingNullableAndNotNullDefinition() {
|
||||
for (MySqlDefaultChanger.Columns col : MySqlDefaultChanger.Columns.values()) {
|
||||
verifyHasCorrespondingColumnDefinitions(col);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldHaveMatchingDefaultValueInNotNullDefinition() {
|
||||
for (MySqlDefaultChanger.Columns col : MySqlDefaultChanger.Columns.values()) {
|
||||
verifyHasSameDefaultValueInNotNullDefinition(col);
|
||||
}
|
||||
}
|
||||
|
||||
private void verifyHasCorrespondingColumnDefinitions(MySqlDefaultChanger.Columns column) {
|
||||
// given / when
|
||||
String nullable = column.nullableDefinition;
|
||||
String notNull = column.notNullDefinition;
|
||||
|
||||
// then
|
||||
String expectedNotNull = nullable + " NOT NULL DEFAULT ";
|
||||
assertThat(column.name(), notNull.startsWith(expectedNotNull), equalTo(true));
|
||||
// Check that `notNull` length is bigger because we expect a value after DEFAULT
|
||||
assertThat(column.name(), notNull.length() > expectedNotNull.length(), equalTo(true));
|
||||
}
|
||||
|
||||
private void verifyHasSameDefaultValueInNotNullDefinition(MySqlDefaultChanger.Columns column) {
|
||||
// given / when
|
||||
String notNull = column.notNullDefinition;
|
||||
Object defaultValue = column.defaultValue;
|
||||
|
||||
// then
|
||||
String defaultValueAsString = String.valueOf(defaultValue);
|
||||
if (!notNull.endsWith("DEFAULT " + defaultValueAsString)
|
||||
&& !notNull.endsWith("DEFAULT '" + defaultValueAsString + "'")) {
|
||||
fail("Expected '" + column + "' not-null definition to contain DEFAULT " + defaultValueAsString);
|
||||
}
|
||||
}
|
||||
}
|
||||
+60
@@ -0,0 +1,60 @@
|
||||
package fr.xephi.authme.command.executable.authme.debug;
|
||||
|
||||
import fr.xephi.authme.ReflectionTestUtils;
|
||||
import fr.xephi.authme.data.auth.PlayerCache;
|
||||
import fr.xephi.authme.datasource.CacheDataSource;
|
||||
import fr.xephi.authme.datasource.DataSource;
|
||||
import fr.xephi.authme.settings.Settings;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.junit.MockitoJUnitRunner;
|
||||
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.mockito.Mockito.mock;
|
||||
|
||||
/**
|
||||
* Test for {@link MySqlDefaultChanger}.
|
||||
*/
|
||||
@RunWith(MockitoJUnitRunner.class)
|
||||
public class MySqlDefaultChangerTest {
|
||||
|
||||
@Mock
|
||||
private Settings settings;
|
||||
|
||||
@Test
|
||||
public void shouldReturnSameDataSourceInstance() {
|
||||
// given
|
||||
DataSource dataSource = mock(DataSource.class);
|
||||
|
||||
// when
|
||||
DataSource result = MySqlDefaultChanger.unwrapSourceFromCacheDataSource(dataSource);
|
||||
|
||||
// then
|
||||
assertThat(result, equalTo(dataSource));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldUnwrapCacheDataSource() {
|
||||
// given
|
||||
DataSource source = mock(DataSource.class);
|
||||
PlayerCache playerCache = mock(PlayerCache.class);
|
||||
CacheDataSource cacheDataSource = new CacheDataSource(source, playerCache);
|
||||
|
||||
// when
|
||||
DataSource result = MySqlDefaultChanger.unwrapSourceFromCacheDataSource(cacheDataSource);
|
||||
|
||||
// then
|
||||
assertThat(result, equalTo(source));
|
||||
}
|
||||
|
||||
// TODO #792: Add more tests
|
||||
|
||||
private MySqlDefaultChanger createDefaultChanger(DataSource dataSource) {
|
||||
MySqlDefaultChanger defaultChanger = new MySqlDefaultChanger();
|
||||
ReflectionTestUtils.setField(defaultChanger, "dataSource", dataSource);
|
||||
ReflectionTestUtils.setField(defaultChanger, "settings", settings);
|
||||
return defaultChanger;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,63 @@
|
||||
package fr.xephi.authme.data.auth;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.hamcrest.Matchers.nullValue;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
/**
|
||||
* Test for {@link PlayerAuth} and its builder.
|
||||
*/
|
||||
public class PlayerAuthTest {
|
||||
|
||||
@Test
|
||||
public void shouldRemoveDatabaseDefaults() {
|
||||
// given / when
|
||||
PlayerAuth auth = PlayerAuth.builder()
|
||||
.name("Bobby")
|
||||
.lastLogin(0L)
|
||||
.email("your@email.com")
|
||||
.build();
|
||||
|
||||
// then
|
||||
assertThat(auth.getNickname(), equalTo("bobby"));
|
||||
assertThat(auth.getLastLogin(), nullValue());
|
||||
assertThat(auth.getEmail(), nullValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldThrowForMissingName() {
|
||||
try {
|
||||
// given / when
|
||||
PlayerAuth.builder()
|
||||
.email("test@example.org")
|
||||
.groupId(3)
|
||||
.build();
|
||||
|
||||
// then
|
||||
fail("Expected exception to be thrown");
|
||||
} catch (NullPointerException e) {
|
||||
// all good
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldCreatePlayerAuthWithNullValues() {
|
||||
// given / when
|
||||
PlayerAuth auth = PlayerAuth.builder()
|
||||
.name("Charlie")
|
||||
.email(null)
|
||||
.lastLogin(null)
|
||||
.groupId(19)
|
||||
.locPitch(123.004f)
|
||||
.build();
|
||||
|
||||
// then
|
||||
assertThat(auth.getEmail(), nullValue());
|
||||
assertThat(auth.getLastLogin(), nullValue());
|
||||
assertThat(auth.getGroupId(), equalTo(19));
|
||||
assertThat(auth.getPitch(), equalTo(123.004f));
|
||||
}
|
||||
}
|
||||
@@ -2,6 +2,7 @@ package fr.xephi.authme.datasource;
|
||||
|
||||
import fr.xephi.authme.data.auth.PlayerAuth;
|
||||
import fr.xephi.authme.security.crypts.HashedPassword;
|
||||
import org.junit.Ignore;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.util.Arrays;
|
||||
@@ -96,7 +97,7 @@ public abstract class AbstractDataSourceIntegrationTest {
|
||||
// then
|
||||
assertThat(invalidAuth, nullValue());
|
||||
|
||||
assertThat(bobbyAuth, hasAuthBasicData("bobby", "Bobby", "your@email.com", "123.45.67.89"));
|
||||
assertThat(bobbyAuth, hasAuthBasicData("bobby", "Bobby", null, "123.45.67.89"));
|
||||
assertThat(bobbyAuth, hasAuthLocation(1.05, 2.1, 4.2, "world", -0.44f, 2.77f));
|
||||
assertThat(bobbyAuth, hasRegistrationInfo("127.0.4.22", 1436778723L));
|
||||
assertThat(bobbyAuth.getLastLogin(), equalTo(1449136800L));
|
||||
@@ -142,9 +143,9 @@ public abstract class AbstractDataSourceIntegrationTest {
|
||||
// then
|
||||
assertThat(response, equalTo(true));
|
||||
assertThat(authList, hasSize(2));
|
||||
assertThat(authList, hasItem(hasAuthBasicData("bobby", "Bobby", "your@email.com", "123.45.67.89")));
|
||||
assertThat(authList, hasItem(hasAuthBasicData("bobby", "Bobby", null, "123.45.67.89")));
|
||||
assertThat(newAuthList, hasSize(3));
|
||||
assertThat(newAuthList, hasItem(hasAuthBasicData("bobby", "Bobby", "your@email.com", "123.45.67.89")));
|
||||
assertThat(newAuthList, hasItem(hasAuthBasicData("bobby", "Bobby", null, "123.45.67.89")));
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -222,7 +223,7 @@ public abstract class AbstractDataSourceIntegrationTest {
|
||||
// then
|
||||
assertThat(response, equalTo(true));
|
||||
PlayerAuth result = dataSource.getAuth("bobby");
|
||||
assertThat(result, hasAuthBasicData("bobby", "BOBBY", "your@email.com", "12.12.12.12"));
|
||||
assertThat(result, hasAuthBasicData("bobby", "BOBBY", null, "12.12.12.12"));
|
||||
assertThat(result.getLastLogin(), equalTo(123L));
|
||||
}
|
||||
|
||||
@@ -327,10 +328,11 @@ public abstract class AbstractDataSourceIntegrationTest {
|
||||
|
||||
// then
|
||||
assertThat(response1 && response2, equalTo(true));
|
||||
assertThat(dataSource.getAuth("bobby"), hasAuthBasicData("bobby", "BOBBY", "your@email.com", "123.45.67.89"));
|
||||
assertThat(dataSource.getAuth("bobby"), hasAuthBasicData("bobby", "BOBBY", null, "123.45.67.89"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@Ignore // TODO #792: Fix purging logic
|
||||
public void shouldGetRecordsToPurge() {
|
||||
// given
|
||||
DataSource dataSource = getDataSource();
|
||||
|
||||
@@ -61,13 +61,13 @@ public class FlatFileIntegrationTest {
|
||||
|
||||
// then
|
||||
assertThat(authList, hasSize(7));
|
||||
assertThat(getName("bobby", authList), hasAuthBasicData("bobby", "bobby", "your@email.com", "123.45.67.89"));
|
||||
assertThat(getName("bobby", authList), hasAuthBasicData("bobby", "bobby", null, "123.45.67.89"));
|
||||
assertThat(getName("bobby", authList), hasAuthLocation(1.05, 2.1, 4.2, "world", 0, 0));
|
||||
assertThat(getName("bobby", authList).getPassword(), equalToHash("$SHA$11aa0706173d7272$dbba966"));
|
||||
assertThat(getName("twofields", authList), hasAuthBasicData("twofields", "twofields", "your@email.com", "127.0.0.1"));
|
||||
assertThat(getName("twofields", authList), hasAuthBasicData("twofields", "twofields", null, "127.0.0.1"));
|
||||
assertThat(getName("twofields", authList).getPassword(), equalToHash("hash1234"));
|
||||
assertThat(getName("threefields", authList), hasAuthBasicData("threefields", "threefields", "your@email.com", "33.33.33.33"));
|
||||
assertThat(getName("fourfields", authList), hasAuthBasicData("fourfields", "fourfields", "your@email.com", "4.4.4.4"));
|
||||
assertThat(getName("threefields", authList), hasAuthBasicData("threefields", "threefields", null, "33.33.33.33"));
|
||||
assertThat(getName("fourfields", authList), hasAuthBasicData("fourfields", "fourfields", null, "4.4.4.4"));
|
||||
assertThat(getName("fourfields", authList).getLastLogin(), equalTo(404040404L));
|
||||
assertThat(getName("sevenfields", authList), hasAuthLocation(7.7, 14.14, 21.21, "world", 0, 0));
|
||||
assertThat(getName("eightfields", authList), hasAuthLocation(8.8, 17.6, 26.4, "eightworld", 0, 0));
|
||||
|
||||
@@ -63,11 +63,11 @@ public class ForceFlatToSqliteTest {
|
||||
ArgumentCaptor<PlayerAuth> authCaptor = ArgumentCaptor.forClass(PlayerAuth.class);
|
||||
verify(dataSource, times(7)).saveAuth(authCaptor.capture());
|
||||
List<PlayerAuth> auths = authCaptor.getAllValues();
|
||||
assertThat(auths, hasItem(hasAuthBasicData("bobby", "Player", "your@email.com", "123.45.67.89")));
|
||||
assertThat(auths, hasItem(hasAuthBasicData("bobby", "Player", null, "123.45.67.89")));
|
||||
assertThat(auths, hasItem(hasAuthLocation(1.05, 2.1, 4.2, "world", 0, 0)));
|
||||
assertThat(auths, hasItem(hasAuthBasicData("user", "Player", "user@example.org", "34.56.78.90")));
|
||||
assertThat(auths, hasItem(hasAuthLocation(124.1, 76.3, -127.8, "nether", 0, 0)));
|
||||
assertThat(auths, hasItem(hasAuthBasicData("eightfields", "Player", "your@email.com", "6.6.6.66")));
|
||||
assertThat(auths, hasItem(hasAuthBasicData("eightfields", "Player", null, "6.6.6.66")));
|
||||
assertThat(auths, hasItem(hasAuthLocation(8.8, 17.6, 26.4, "eightworld", 0, 0)));
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user