Fix registering plan_user row if missing when required

There was one error code for MySQL that used wrong message context to detect the missing user_id properly. Wrote tests for all the extra functionality ensuring no exceptions occur. Affects issues: - Fixed #2361 - Fixed #2343
plan-player-analytics · May 14, 2022 · 8bbbde4 · 8bbbde4
1 parent 0c7d001
commit 8bbbde4
Show file tree

Hide file tree

Showing 10 changed files with 189 additions and 17 deletions.
diff --git a/Plan/common/src/main/java/com/djrapitops/plan/delivery/domain/PlayerIdentifier.java b/Plan/common/src/main/java/com/djrapitops/plan/delivery/domain/PlayerIdentifier.java
@@ -37,15 +37,15 @@ public String getName() {
     }
 
     public boolean isSame(PlayerIdentifier that) {
-        return Objects.equals(getUuid(), that.getUuid()) && Objects.equals(getName(), that.getName());
+        return Objects.equals(this, that);
     }
 
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         PlayerIdentifier that = (PlayerIdentifier) o;
-        return Objects.equals(getUuid(), that.getUuid()) && Objects.equals(getName(), that.getName());
+        return Objects.equals(getUuid(), that.getUuid());
     }
 
     @Override

diff --git a/Plan/common/src/main/java/com/djrapitops/plan/exceptions/database/DBOpException.java b/Plan/common/src/main/java/com/djrapitops/plan/exceptions/database/DBOpException.java
@@ -132,7 +132,7 @@ public static DBOpException forCause(String sql, SQLException e) {
                 break;
             case 1048:
                 // MySQL
-                context.related("Not null constraint violation")
+                context.related(CONSTRAINT_VIOLATION)
                         .whatToDo("Report this error. NOT NULL constraint violation occurred.");
                 break;
             default:

diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/SQLDB.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/SQLDB.java
@@ -225,6 +225,7 @@ Patch[] patches() {
                 new SessionsOptimizationPatch(),
                 new UserInfoHostnameAllowNullPatch(),
                 new RegisterDateMinimizationPatch(),
+                new UsersTableNameLengthPatch()
         };
     }
 

diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/tables/UsersTable.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/tables/UsersTable.java
@@ -31,6 +31,7 @@
  * Patches that apply to this table:
  * {@link com.djrapitops.plan.storage.database.transactions.patches.Version10Patch}
  * {@link com.djrapitops.plan.storage.database.transactions.patches.RegisterDateMinimizationPatch}
+ * {@link com.djrapitops.plan.storage.database.transactions.patches.UsersTableNameLengthPatch}
  *
  * @author AuroraLS3
  */
@@ -58,7 +59,7 @@ public static String createTableSQL(DBType dbType) {
                 .column(ID, Sql.INT).primaryKey()
                 .column(USER_UUID, Sql.varchar(36)).notNull().unique()
                 .column(REGISTERED, Sql.LONG).notNull()
-                .column(USER_NAME, Sql.varchar(16)).notNull()
+                .column(USER_NAME, Sql.varchar(36)).notNull()
                 .column(TIMES_KICKED, Sql.INT).notNull().defaultValue("0")
                 .toString();
     }

diff --git a/...apitops/plan/storage/database/transactions/patches/ExtensionTableRowValueLengthPatch.java b/...apitops/plan/storage/database/transactions/patches/ExtensionTableRowValueLengthPatch.java
@@ -17,7 +17,6 @@
 package com.djrapitops.plan.storage.database.transactions.patches;
 
 import com.djrapitops.plan.storage.database.DBType;
-import com.djrapitops.plan.storage.database.queries.schema.MySQLSchemaQueries;
 import com.djrapitops.plan.storage.database.sql.building.Sql;
 import com.djrapitops.plan.storage.database.sql.tables.ExtensionPlayerTableValueTable;
 import com.djrapitops.plan.storage.database.sql.tables.ExtensionServerTableValueTable;
@@ -44,10 +43,6 @@ public boolean hasBeenApplied() {
                         && columnVarcharLength(serverTable, ExtensionServerTableValueTable.VALUE_5) >= 250;
     }
 
-    private int columnVarcharLength(String table, String column) {
-        return query(MySQLSchemaQueries.columnVarcharLength(table, column));
-    }
-
     @Override
     protected void applyPatch() {
         increaseLength(playerTable, ExtensionPlayerTableValueTable.VALUE_1);

diff --git a/...common/src/main/java/com/djrapitops/plan/storage/database/transactions/patches/Patch.java b/...common/src/main/java/com/djrapitops/plan/storage/database/transactions/patches/Patch.java
@@ -157,4 +157,8 @@ public Boolean processResults(ResultSet set) throws SQLException {
             }
         });
     }
+
+    protected int columnVarcharLength(String table, String column) {
+        return dbType == DBType.SQLITE ? Integer.MAX_VALUE : query(MySQLSchemaQueries.columnVarcharLength(table, column));
+    }
 }
diff --git a/.../com/djrapitops/plan/storage/database/transactions/patches/UsersTableNameLengthPatch.java b/.../com/djrapitops/plan/storage/database/transactions/patches/UsersTableNameLengthPatch.java
@@ -0,0 +1,34 @@
+/*
+ *  This file is part of Player Analytics (Plan).
+ *
+ *  Plan is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU Lesser General Public License v3 as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  Plan is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public License
+ *  along with Plan. If not, see <https://www.gnu.org/licenses/>.
+ */
+package com.djrapitops.plan.storage.database.transactions.patches;
+
+import com.djrapitops.plan.storage.database.DBType;
+import com.djrapitops.plan.storage.database.sql.building.Sql;
+import com.djrapitops.plan.storage.database.sql.tables.UsersTable;
+
+public class UsersTableNameLengthPatch extends Patch {
+    @Override
+    public boolean hasBeenApplied() {
+        return dbType == DBType.SQLITE || // SQLite does not limit varchar lengths
+                columnVarcharLength(UsersTable.TABLE_NAME, UsersTable.USER_NAME) >= 36;
+    }
+
+    @Override
+    protected void applyPatch() {
+        execute("ALTER TABLE " + UsersTable.TABLE_NAME + " MODIFY " + UsersTable.USER_NAME + " " + Sql.varchar(36));
+    }
+}
diff --git a/...on/src/test/java/com/djrapitops/plan/storage/database/queries/GeolocationQueriesTest.java b/...on/src/test/java/com/djrapitops/plan/storage/database/queries/GeolocationQueriesTest.java
@@ -17,6 +17,7 @@
 package com.djrapitops.plan.storage.database.queries;
 
 import com.djrapitops.plan.delivery.domain.DateObj;
+import com.djrapitops.plan.gathering.domain.BaseUser;
 import com.djrapitops.plan.gathering.domain.GeoInfo;
 import com.djrapitops.plan.gathering.domain.Ping;
 import com.djrapitops.plan.storage.database.Database;
@@ -27,6 +28,7 @@
 import com.djrapitops.plan.storage.database.transactions.commands.RemoveEverythingTransaction;
 import com.djrapitops.plan.storage.database.transactions.events.GeoInfoStoreTransaction;
 import com.djrapitops.plan.storage.database.transactions.events.PingStoreTransaction;
+import com.djrapitops.plan.storage.database.transactions.events.PlayerRegisterTransaction;
 import com.djrapitops.plan.storage.database.transactions.events.PlayerServerRegisterTransaction;
 import org.junit.jupiter.api.Test;
 import utilities.RandomData;
@@ -40,6 +42,32 @@
 
 public interface GeolocationQueriesTest extends DatabaseTestPreparer {
 
+    @Test
+    default void geoInfoStoreTransactionOutOfOrderDoesNotFailDueToMissingUser() {
+        List<GeoInfo> expected = RandomData.randomGeoInfo();
+        for (GeoInfo geoInfo : expected) {
+            save(playerUUID, geoInfo);
+        }
+
+        List<GeoInfo> result = db().query(GeoInfoQueries.fetchAllGeoInformation()).get(playerUUID);
+        assertEquals(expected, result);
+    }
+
+    @Test
+    default void geoInfoStoreTransactionOutOfOrderUpdatesUserInformation() {
+        List<GeoInfo> geoInfos = RandomData.randomGeoInfo();
+        for (GeoInfo geoInfo : geoInfos) {
+            save(playerUUID, geoInfo);
+        }
+
+        long registerDate = RandomData.randomTime();
+        db().executeTransaction(new PlayerRegisterTransaction(playerUUID, () -> registerDate, TestConstants.PLAYER_ONE_NAME));
+
+        Optional<BaseUser> expected = Optional.of(new BaseUser(playerUUID, TestConstants.PLAYER_ONE_NAME, registerDate, 0));
+        Optional<BaseUser> result = db().query(BaseUserQueries.fetchBaseUserOfPlayer(playerUUID));
+        assertEquals(expected, result);
+    }
+
     @Test
     default void geoInformationIsStored() {
         db().executeTransaction(new PlayerServerRegisterTransaction(playerUUID, RandomData::randomTime,

diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/PingQueriesTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/PingQueriesTest.java
@@ -17,20 +17,20 @@
 package com.djrapitops.plan.storage.database.queries;
 
 import com.djrapitops.plan.delivery.domain.DateObj;
+import com.djrapitops.plan.gathering.domain.BaseUser;
 import com.djrapitops.plan.gathering.domain.Ping;
 import com.djrapitops.plan.storage.database.DatabaseTestPreparer;
+import com.djrapitops.plan.storage.database.queries.objects.BaseUserQueries;
 import com.djrapitops.plan.storage.database.queries.objects.PingQueries;
 import com.djrapitops.plan.storage.database.transactions.commands.RemoveEverythingTransaction;
 import com.djrapitops.plan.storage.database.transactions.events.PingStoreTransaction;
+import com.djrapitops.plan.storage.database.transactions.events.PlayerRegisterTransaction;
 import com.djrapitops.plan.storage.database.transactions.events.PlayerServerRegisterTransaction;
 import org.junit.jupiter.api.Test;
 import utilities.RandomData;
 import utilities.TestConstants;
 
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
+import java.util.*;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -42,6 +42,35 @@ private void prepareForPingStorage() {
                 TestConstants.PLAYER_ONE_NAME, serverUUID(), TestConstants.GET_PLAYER_HOSTNAME));
     }
 
+    @Test
+    default void pingStoreTransactionOutOfOrderDoesNotFailDueToMissingUser() {
+        DateObj<Integer> saved = RandomData.randomIntDateObject();
+        int value = saved.getValue();
+        db().executeTransaction(new PingStoreTransaction(playerUUID, serverUUID(),
+                Collections.singletonList(saved)
+        ));
+
+        Map<UUID, List<Ping>> expected = Collections.singletonMap(playerUUID, Collections.singletonList(
+                new Ping(saved.getDate(), serverUUID(), value, value, value)
+        ));
+        Map<UUID, List<Ping>> fetched = db().query(PingQueries.fetchAllPingData());
+        assertEquals(expected, fetched);
+    }
+
+
+    @Test
+    default void pingStoreTransactionOutOfOrderUpdatesUserInformation() {
+        db().executeTransaction(new PingStoreTransaction(playerUUID, serverUUID(),
+                Collections.singletonList(RandomData.randomIntDateObject())
+        ));
+        long registerDate = RandomData.randomTime();
+        db().executeTransaction(new PlayerRegisterTransaction(playerUUID, () -> registerDate, TestConstants.PLAYER_ONE_NAME));
+
+        Optional<BaseUser> expected = Optional.of(new BaseUser(playerUUID, TestConstants.PLAYER_ONE_NAME, registerDate, 0));
+        Optional<BaseUser> result = db().query(BaseUserQueries.fetchBaseUserOfPlayer(playerUUID));
+        assertEquals(expected, result);
+    }
+
     @Test
     default void singlePingIsStored() {
         prepareForPingStorage();

diff --git a/...common/src/test/java/com/djrapitops/plan/storage/database/queries/SessionQueriesTest.java b/...common/src/test/java/com/djrapitops/plan/storage/database/queries/SessionQueriesTest.java
@@ -19,14 +19,12 @@
 import com.djrapitops.plan.delivery.domain.TablePlayer;
 import com.djrapitops.plan.delivery.domain.container.PlayerContainer;
 import com.djrapitops.plan.delivery.domain.mutators.SessionsMutator;
-import com.djrapitops.plan.gathering.domain.FinishedSession;
-import com.djrapitops.plan.gathering.domain.PlayerKill;
-import com.djrapitops.plan.gathering.domain.PlayerKills;
-import com.djrapitops.plan.gathering.domain.WorldTimes;
+import com.djrapitops.plan.gathering.domain.*;
 import com.djrapitops.plan.identification.Server;
 import com.djrapitops.plan.identification.ServerUUID;
 import com.djrapitops.plan.storage.database.DatabaseTestPreparer;
 import com.djrapitops.plan.storage.database.queries.containers.PlayerContainerQuery;
+import com.djrapitops.plan.storage.database.queries.objects.BaseUserQueries;
 import com.djrapitops.plan.storage.database.queries.objects.KillQueries;
 import com.djrapitops.plan.storage.database.queries.objects.SessionQueries;
 import com.djrapitops.plan.storage.database.queries.objects.WorldTimesQueries;
@@ -36,7 +34,9 @@
 import com.djrapitops.plan.storage.database.transactions.StoreServerInformationTransaction;
 import com.djrapitops.plan.storage.database.transactions.Transaction;
 import com.djrapitops.plan.storage.database.transactions.commands.RemoveEverythingTransaction;
+import com.djrapitops.plan.storage.database.transactions.events.PlayerRegisterTransaction;
 import com.djrapitops.plan.storage.database.transactions.events.PlayerServerRegisterTransaction;
+import com.djrapitops.plan.storage.database.transactions.events.SessionEndTransaction;
 import com.djrapitops.plan.storage.database.transactions.events.WorldNameStoreTransaction;
 import com.djrapitops.plan.utilities.java.Maps;
 import net.playeranalytics.plugin.scheduling.TimeAmount;
@@ -53,6 +53,86 @@
 
 public interface SessionQueriesTest extends DatabaseTestPreparer {
 
+    @Test
+    default void sessionStoreTransactionOutOfOrderDoesNotFailDueToMissingMainUser() {
+        db().executeTransaction(new WorldNameStoreTransaction(serverUUID(), worlds[0]));
+        db().executeTransaction(new WorldNameStoreTransaction(serverUUID(), worlds[1]));
+        db().executeTransaction(new PlayerServerRegisterTransaction(player2UUID, RandomData::randomTime,
+                TestConstants.PLAYER_TWO_NAME, serverUUID(), TestConstants.GET_PLAYER_HOSTNAME));
+
+        FinishedSession session = RandomData.randomSession(serverUUID(), worlds, playerUUID, player2UUID);
+
+        db().executeTransaction(new SessionEndTransaction(session));
+
+        Map<ServerUUID, List<FinishedSession>> sessions = db().query(SessionQueries.fetchSessionsOfPlayer(playerUUID));
+        List<FinishedSession> savedSessions = sessions.get(serverUUID());
+
+        assertNotNull(savedSessions);
+        assertEquals(1, savedSessions.size());
+
+        assertEquals(session, savedSessions.get(0));
+    }
+
+    @Test
+    default void sessionStoreTransactionOutOfOrderDoesNotFailDueToMissingKilledUser() {
+        db().executeTransaction(new WorldNameStoreTransaction(serverUUID(), worlds[0]));
+        db().executeTransaction(new WorldNameStoreTransaction(serverUUID(), worlds[1]));
+        db().executeTransaction(new PlayerServerRegisterTransaction(playerUUID, RandomData::randomTime,
+                TestConstants.PLAYER_ONE_NAME, serverUUID(), TestConstants.GET_PLAYER_HOSTNAME));
+
+        FinishedSession session = RandomData.randomSession(serverUUID(), worlds, playerUUID, player2UUID);
+        db().executeTransaction(new SessionEndTransaction(session));
+
+        Map<ServerUUID, List<FinishedSession>> sessions = db().query(SessionQueries.fetchSessionsOfPlayer(playerUUID));
+        List<FinishedSession> savedSessions = sessions.get(serverUUID());
+
+        assertNotNull(savedSessions);
+        assertEquals(1, savedSessions.size());
+
+        // Query doesn't fetch kills since the uuid for killed player is missing
+        session.getExtraData(PlayerKills.class)
+                .map(PlayerKills::asList)
+                .ifPresent(List::clear);
+
+        assertEquals(session, savedSessions.get(0));
+    }
+
+    @Test
+    default void killsAreAvailableAfter2ndUserRegisterEvenIfOutOfOrder() {
+        db().executeTransaction(new WorldNameStoreTransaction(serverUUID(), worlds[0]));
+        db().executeTransaction(new WorldNameStoreTransaction(serverUUID(), worlds[1]));
+        db().executeTransaction(new PlayerServerRegisterTransaction(playerUUID, RandomData::randomTime,
+                TestConstants.PLAYER_ONE_NAME, serverUUID(), TestConstants.GET_PLAYER_HOSTNAME));
+        FinishedSession session = RandomData.randomSession(serverUUID(), worlds, playerUUID, player2UUID);
+        db().executeTransaction(new SessionEndTransaction(session));
+        // killed user is registered after session already ended.
+        db().executeTransaction(new PlayerServerRegisterTransaction(player2UUID, RandomData::randomTime,
+                TestConstants.PLAYER_TWO_NAME, serverUUID(), TestConstants.GET_PLAYER_HOSTNAME));
+
+        Map<ServerUUID, List<FinishedSession>> sessions = db().query(SessionQueries.fetchSessionsOfPlayer(playerUUID));
+        List<FinishedSession> savedSessions = sessions.get(serverUUID());
+
+        assertNotNull(savedSessions);
+        assertEquals(1, savedSessions.size());
+
+        assertEquals(session, savedSessions.get(0));
+    }
+
+    @Test
+    default void sessionStoreTransactionOutOfOrderUpdatesUserInformation() {
+        db().executeTransaction(new WorldNameStoreTransaction(serverUUID(), worlds[0]));
+        db().executeTransaction(new WorldNameStoreTransaction(serverUUID(), worlds[1]));
+        FinishedSession session = RandomData.randomSession(serverUUID(), worlds, playerUUID, player2UUID);
+        db().executeTransaction(new SessionEndTransaction(session));
+
+        long registerDate = RandomData.randomTime();
+        db().executeTransaction(new PlayerRegisterTransaction(playerUUID, () -> registerDate, TestConstants.PLAYER_ONE_NAME));
+
+        Optional<BaseUser> expected = Optional.of(new BaseUser(playerUUID, TestConstants.PLAYER_ONE_NAME, registerDate, 0));
+        Optional<BaseUser> result = db().query(BaseUserQueries.fetchBaseUserOfPlayer(playerUUID));
+        assertEquals(expected, result);
+    }
+
     @Test
     default void sessionPlaytimeIsCalculatedCorrectlyAfterStorage() {
         prepareForSessionSave();