Patch bad join address data if it is in database

Plan/common/src/main/java/com/djrapitops/plan/storage/database/AbstractDatabase.java

@@ -33,7 +33,7 @@ public abstract class AbstractDatabase implements Database {
 
     protected AbstractDatabase() {
         state = State.CLOSED;
-        accessLock = new DBAccessLock(this);
+        accessLock = new DBAccessLock();
     }
 
     @Override

Plan/common/src/main/java/com/djrapitops/plan/storage/database/DBAccessLock.java

@@ -19,6 +19,10 @@ package com.djrapitops.plan.storage.database;
 import com.djrapitops.plan.exceptions.database.DBClosedException;
 import com.djrapitops.plan.storage.database.transactions.Transaction;
 import com.djrapitops.plan.storage.database.transactions.init.OperationCriticalTransaction;
+import com.djrapitops.plan.utilities.java.ThrowingSupplier;
+import com.djrapitops.plan.utilities.java.ThrowingVoidFunction;
+
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * Database Lock that prevents queries and transactions from taking place before database schema is ready.
@@ -30,46 +34,61 @@ import com.djrapitops.plan.storage.database.transactions.init.OperationCriticalT
  */
 public class DBAccessLock {
 
-    private final Database database;
+    private final ReentrantLock reentrantLock;
 
-    private final Object lockObject;
-
-    public DBAccessLock(Database database) {
-        this.database = database;
-        this.lockObject = new Object();
-    }
-
-    public void checkAccess() {
-        checkAccess(false);
-    }
-
-    public void checkAccess(Transaction transaction) {
-        checkAccess(transaction instanceof OperationCriticalTransaction);
-    }
-
-    private void checkAccess(boolean isOperationCriticalTransaction) {
-        if (isOperationCriticalTransaction) {
-            return;
-        }
-        try {
-            // Wait for the database to be in OPEN or CLOSING state before allowing execution.
-            // CLOSING is not an allowed state if database was not OPEN at the time of close.
-            while (database.getState() != Database.State.OPEN && database.getState() != Database.State.CLOSING) {
-                synchronized (lockObject) {
-                    lockObject.wait();
-                    if (database.getState() == Database.State.CLOSED) {
-                        throw new DBClosedException("Database failed to open, Query has failed. (This exception is necessary to not keep query threads waiting)");
-                    }
-                }
-            }
-        } catch (InterruptedException e) {
-            Thread.currentThread().interrupt();
-        }
+    public DBAccessLock() {
+        reentrantLock = new ReentrantLock();
     }
 
     public void operabilityChanged() {
-        synchronized (lockObject) {
-            lockObject.notifyAll();
+        for (int i = 0; i < reentrantLock.getHoldCount(); i++) {
+            reentrantLock.unlock();
+        }
+    }
+
+    public <E extends Exception> void performDatabaseOperation(ThrowingVoidFunction<E> operation) throws E {
+        performDatabaseOperation(operation, false);
+    }
+
+    public <E extends Exception> void performDatabaseOperation(ThrowingVoidFunction<E> operation, Transaction transaction) throws E {
+        performDatabaseOperation(operation, transaction instanceof OperationCriticalTransaction);
+    }
+
+    private <E extends Exception> void performDatabaseOperation(ThrowingVoidFunction<E> operation, boolean isOperationCriticalTransaction) throws E {
+        if (isOperationCriticalTransaction) {
+            operation.apply();
+            return;
+        }
+        try {
+            reentrantLock.lockInterruptibly();
+            operation.apply();
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+        } finally {
+            reentrantLock.unlock();
+        }
+    }
+
+    public <T, E extends Exception> T performDatabaseOperation(ThrowingSupplier<T, E> operation) throws E {
+        return performDatabaseOperation(operation, false);
+    }
+
+    public <T, E extends Exception> T performDatabaseOperation(ThrowingSupplier<T, E> operation, Transaction transaction) throws E {
+        return performDatabaseOperation(operation, transaction instanceof OperationCriticalTransaction);
+    }
+
+    private <T, E extends Exception> T performDatabaseOperation(ThrowingSupplier<T, E> operation, boolean isOperationCriticalTransaction) throws E {
+        if (isOperationCriticalTransaction) {
+            return operation.get();
+        }
+        try {
+            reentrantLock.lockInterruptibly();
+            return operation.get();
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new DBClosedException("Operation interrupted");
+        } finally {
+            reentrantLock.unlock();
         }
     }
 }

Plan/common/src/main/java/com/djrapitops/plan/storage/database/SQLDB.java

@@ -16,6 +16,7 @@
  */
 package com.djrapitops.plan.storage.database;
 
+import com.djrapitops.plan.exceptions.database.DBClosedException;
 import com.djrapitops.plan.exceptions.database.DBInitException;
 import com.djrapitops.plan.exceptions.database.DBOpException;
 import com.djrapitops.plan.exceptions.database.FatalDBException;
@@ -238,7 +239,8 @@ public abstract class SQLDB extends AbstractDatabase {
                 new UsersTableNameLengthPatch(),
                 new SessionJoinAddressPatch(),
                 new RemoveUsernameFromAccessLogPatch(),
-                new ComponentColumnToExtensionDataPatch()
+                new ComponentColumnToExtensionDataPatch(),
+                new BadJoinAddressDataCorrectionPatch()
         };
     }
 
@@ -317,14 +319,17 @@ public abstract class SQLDB extends AbstractDatabase {
 
     @Override
     public <T> T query(Query<T> query) {
-        accessLock.checkAccess();
-        return query.executeQuery(this);
+        return accessLock.performDatabaseOperation(() -> query.executeQuery(this));
+    }
+
+    public <T> T queryWithinTransaction(Query<T> query, Transaction transaction) {
+        return accessLock.performDatabaseOperation(() -> query.executeQuery(this), transaction);
     }
 
     @Override
     public CompletableFuture<?> executeTransaction(Transaction transaction) {
         if (getState() == State.CLOSED) {
-            throw new DBOpException("Transaction tried to execute although database is closed.");
+            throw new DBClosedException("Transaction tried to execute although database is closed.");
         }
 
         Exception origin = new Exception();
@@ -337,10 +342,9 @@ public abstract class SQLDB extends AbstractDatabase {
 
         return CompletableFuture.supplyAsync(() -> {
             try {
-                accessLock.checkAccess(transaction);
-                if (!ranIntoFatalError.get()) {
-                    transaction.executeTransaction(this);
-                }
+                accessLock.performDatabaseOperation(() -> {
+                    if (!ranIntoFatalError.get()) {transaction.executeTransaction(this);}
+                }, transaction);
                 return CompletableFuture.completedFuture(null);
             } finally {
                 transactionQueueSize.decrementAndGet();

Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/QueryParameterSetter.java

@@ -59,6 +59,8 @@ public class QueryParameterSetter {
             statement.setLong(index, (Long) parameter);
         } else if (parameter instanceof Double) {
             statement.setDouble(index, (Double) parameter);
+        } else if (parameter instanceof Character) {
+            statement.setString(index, String.valueOf(parameter));
         } else if (parameter instanceof Float) {
             statement.setFloat(index, (Float) parameter);
         } else if (parameter instanceof String) {

Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/objects/JoinAddressQueries.java

@@ -200,4 +200,10 @@ public class JoinAddressQueries {
             });
         };
     }
+
+    public static Query<Optional<Integer>> getIdOfJoinAddress(String correctedAddress) {
+        String sql = SELECT + JoinAddressTable.ID + FROM + JoinAddressTable.TABLE_NAME + WHERE + JoinAddressTable.JOIN_ADDRESS + "=?";
+        return db -> db.queryOptional(sql,
+                results -> results.getInt(JoinAddressTable.ID), correctedAddress);
+    }
 }

Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/Transaction.java

@@ -206,7 +206,7 @@ public abstract class Transaction {
         } else if (query instanceof QueryAPIQuery) {
             return ((QueryAPIQuery<T>) query).executeWithConnection(connection);
         } else {
-            return query.executeQuery(db);
+            return db.queryWithinTransaction(query, this);
         }
     }
 

Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/events/StoreJoinAddressTransaction.java

@@ -26,6 +26,7 @@ import org.apache.commons.lang3.StringUtils;
 
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
+import java.util.Optional;
 import java.util.function.Supplier;
 
 import static com.djrapitops.plan.storage.database.sql.building.Sql.*;
@@ -33,6 +34,7 @@ import static com.djrapitops.plan.storage.database.sql.building.Sql.*;
 public class StoreJoinAddressTransaction extends Transaction {
 
     private final Supplier<String> joinAddress;
+    private int newId;
 
     public StoreJoinAddressTransaction(String joinAddress) {
         this(() -> joinAddress);
@@ -66,11 +68,15 @@ public class StoreJoinAddressTransaction extends Transaction {
 
     @Override
     protected void performOperations() {
-        execute(new ExecStatement(JoinAddressTable.INSERT_STATEMENT) {
+        newId = executeReturningId(new ExecStatement(JoinAddressTable.INSERT_STATEMENT) {
             @Override
             public void prepare(PreparedStatement statement) throws SQLException {
                 statement.setString(1, getAddress());
             }
         });
     }
+
+    public Optional<Integer> getNewId() {
+        return newId == -1 ? Optional.empty() : Optional.of(newId);
+    }
 }

Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/patches/BadJoinAddressDataCorrectionPatch.java

@@ -0,0 +1,123 @@
+/*
+ *  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.exceptions.database.DBOpException;
+import com.djrapitops.plan.storage.database.DBType;
+import com.djrapitops.plan.storage.database.queries.QueryParameterSetter;
+import com.djrapitops.plan.storage.database.queries.objects.JoinAddressQueries;
+import com.djrapitops.plan.storage.database.sql.building.Sql;
+import com.djrapitops.plan.storage.database.sql.tables.JoinAddressTable;
+import com.djrapitops.plan.storage.database.sql.tables.SessionsTable;
+import com.djrapitops.plan.storage.database.transactions.ExecBatchStatement;
+import com.djrapitops.plan.storage.database.transactions.ExecStatement;
+import com.djrapitops.plan.storage.database.transactions.events.StoreJoinAddressTransaction;
+import org.apache.commons.lang3.StringUtils;
+
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import static com.djrapitops.plan.storage.database.sql.building.Sql.*;
+
+/**
+ * @author AuroraLS3
+ */
+public class BadJoinAddressDataCorrectionPatch extends Patch {
+
+    private Map<String, Integer> badAddressIds;
+
+    @Override
+    public boolean hasBeenApplied() {
+        badAddressIds = getBadAddressIds();
+        badAddressIds.keySet().removeIf(address -> !address.contains("\u0000"));
+        return badAddressIds.isEmpty();
+    }
+
+    @Override
+    protected void applyPatch() {
+        Set<Integer> removeIds = new HashSet<>();
+        Map<Integer, Integer> oldToNewIds = new HashMap<>();
+        Map<String, Integer> newIds = new HashMap<>();
+        for (Map.Entry<String, Integer> entry : badAddressIds.entrySet()) {
+            String badAddress = entry.getKey();
+            Integer oldId = entry.getValue();
+            String correctedAddress = StringUtils.split(badAddress, '\u0000')[0];
+            removeIds.add(oldId);
+
+            Integer newIdStored = newIds.get(correctedAddress);
+            int newId = newIdStored == null ? getOrAddCorrectAddressId(correctedAddress) : newIdStored;
+            newIds.put(correctedAddress, newId);
+            oldToNewIds.put(oldId, newId);
+        }
+        updateOldIds(oldToNewIds);
+
+        String sql = DELETE_FROM + JoinAddressTable.TABLE_NAME +
+                WHERE + JoinAddressTable.ID + " IN (" + Sql.nParameters(removeIds.size()) + ")";
+        execute(new ExecStatement(sql) {
+            @Override
+            public void prepare(PreparedStatement statement) throws SQLException {
+                QueryParameterSetter.setParameters(statement, removeIds);
+            }
+        });
+    }
+
+    private void updateOldIds(Map<Integer, Integer> oldToNewIds) {
+        String sql = "UPDATE " + SessionsTable.TABLE_NAME + " SET " + SessionsTable.JOIN_ADDRESS_ID + "=?" +
+                WHERE + SessionsTable.JOIN_ADDRESS_ID + "=?";
+        execute(new ExecBatchStatement(sql) {
+            @Override
+            public void prepare(PreparedStatement statement) throws SQLException {
+                for (Map.Entry<Integer, Integer> entry : oldToNewIds.entrySet()) {
+                    statement.setInt(1, entry.getKey());
+                    statement.setInt(2, entry.getValue());
+                    statement.addBatch();
+                }
+            }
+        });
+    }
+
+    private int getOrAddCorrectAddressId(String correctedAddress) {
+        return query(JoinAddressQueries.getIdOfJoinAddress(correctedAddress))
+                .orElseGet(() -> storeAndGetIdOfNewAddress(correctedAddress));
+    }
+
+    private Integer storeAndGetIdOfNewAddress(String correctedAddress) {
+        StoreJoinAddressTransaction store = new StoreJoinAddressTransaction(correctedAddress);
+        executeOther(store);
+        return store.getNewId().orElseGet(this::getIdOfUnknownJoinAddress);
+    }
+
+    private Integer getIdOfUnknownJoinAddress() {
+        return query(JoinAddressQueries.getIdOfJoinAddress(JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP))
+                .orElseThrow(() -> new DBOpException("Could not get ID of join address properly"));
+    }
+
+    private Map<String, Integer> getBadAddressIds() {
+        String likeNullChar = dbType == DBType.MYSQL ? "CONCAT(\"%\", CHAR(0x00 using utf8), \"%\")"
+                : "\"%\" || CHAR(0) || \"%\"";
+        String sql = SELECT + JoinAddressTable.ID + ',' +
+                JoinAddressTable.JOIN_ADDRESS +
+                FROM + JoinAddressTable.TABLE_NAME +
+                WHERE + JoinAddressTable.JOIN_ADDRESS + " LIKE " + likeNullChar;
+        return query(db -> db.queryMap(sql, (results, map) -> map.put(results.getString(JoinAddressTable.JOIN_ADDRESS),
+                results.getInt(JoinAddressTable.ID))));
+    }
+}

Plan/common/src/test/java/com/djrapitops/plan/storage/database/DatabaseTestAggregate.java

@@ -21,6 +21,7 @@ import com.djrapitops.plan.storage.database.queries.*;
 import com.djrapitops.plan.storage.database.queries.analysis.TopListQueriesTest;
 import com.djrapitops.plan.storage.database.transactions.commands.ChangeUserUUIDTransactionTest;
 import com.djrapitops.plan.storage.database.transactions.commands.CombineUserTransactionTest;
+import com.djrapitops.plan.storage.database.transactions.patches.BadJoinAddressDataCorrectionPatchTest;
 
 public interface DatabaseTestAggregate extends
         ActivityIndexQueriesTest,
@@ -39,6 +40,7 @@ public interface DatabaseTestAggregate extends
         JoinAddressQueriesTest,
         ChangeUserUUIDTransactionTest,
         CombineUserTransactionTest,
-        ExtensionQueryResultTableDataQueryTest {
+        ExtensionQueryResultTableDataQueryTest,
+        BadJoinAddressDataCorrectionPatchTest {
     /* Collects all query tests together so its easier to implement database tests */
 }

Plan/common/src/test/java/com/djrapitops/plan/storage/database/transactions/patches/BadJoinAddressDataCorrectionPatchTest.java

@@ -0,0 +1,128 @@
+/*
+ *  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.Database;
+import com.djrapitops.plan.storage.database.DatabaseTestPreparer;
+import com.djrapitops.plan.storage.database.queries.objects.JoinAddressQueries;
+import com.djrapitops.plan.storage.database.sql.tables.JoinAddressTable;
+import com.djrapitops.plan.storage.database.transactions.Transaction;
+import com.djrapitops.plan.storage.database.transactions.events.StoreJoinAddressTransaction;
+import org.junit.jupiter.api.Test;
+import utilities.RandomData;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+/**
+ * @author AuroraLS3
+ */
+public interface BadJoinAddressDataCorrectionPatchTest extends DatabaseTestPreparer {
+
+    @Test
+    default void joinAddressWithBadDataIsNotCorrectedWhenDataIsCorrect() {
+        Database db = db();
+        String correct = "correct_address";
+        db.executeTransaction(new StoreJoinAddressTransaction(correct));
+
+        Set<String> preTestExpected = Set.of(correct, JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP);
+        Set<String> preTestResult = new HashSet<>(db.query(JoinAddressQueries.allJoinAddresses()));
+        assertEquals(preTestExpected, preTestResult);
+
+        BadJoinAddressDataCorrectionPatch patch = new BadJoinAddressDataCorrectionPatch();
+        db.executeTransaction(patch);
+
+        assertFalse(patch.wasApplied());
+    }
+
+    @Test
+    default void joinAddressWithBadDataIsCorrectedWithOriginal() {
+        Database db = db();
+        String correct = "correct_address";
+        String bad = "correct_address\u000062.6.…zwyzyty0zmnlowzmmtqynmm";
+        db.executeTransaction(new StoreJoinAddressTransaction(correct));
+        db.executeTransaction(new StoreJoinAddressTransaction(bad));
+
+        Set<String> preTestExpected = Set.of(correct, bad, JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP);
+        Set<String> preTestResult = new HashSet<>(db.query(JoinAddressQueries.allJoinAddresses()));
+        assertEquals(preTestExpected, preTestResult);
+
+        BadJoinAddressDataCorrectionPatch patch = new BadJoinAddressDataCorrectionPatch();
+        db.executeTransaction(patch);
+        assertTrue(patch.wasApplied());
+
+        Set<String> expected = Set.of(correct, JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP);
+        Set<String> result = new HashSet<>(db.query(JoinAddressQueries.allJoinAddresses()));
+        assertEquals(expected, result);
+    }
+
+    @Test
+    default void joinAddressWithBadDataIsCorrectedWithoutOriginal() {
+        Database db = db();
+        String correct = "correct_address";
+        String bad = "correct_address\u000062.6.…zwyzyty0zmnlowzmmtqynmm";
+        db.executeTransaction(new StoreJoinAddressTransaction(bad));
+
+        Set<String> preTestExpected = Set.of(bad, JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP);
+        Set<String> preTestResult = new HashSet<>(db.query(JoinAddressQueries.allJoinAddresses()));
+        assertEquals(preTestExpected, preTestResult);
+
+        BadJoinAddressDataCorrectionPatch patch = new BadJoinAddressDataCorrectionPatch();
+        db.executeTransaction(patch);
+        assertTrue(patch.wasApplied());
+
+        Set<String> expected = Set.of(correct, JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP);
+        Set<String> result = new HashSet<>(db.query(JoinAddressQueries.allJoinAddresses()));
+        assertEquals(expected, result);
+    }
+
+    @Test
+    default void joinAddressWithBadDataIsCorrectedPerformanceTest() {
+        Database db = db();
+        String correct = "correct_address";
+        String badPrefix = "correct_address\u0000";
+
+        List<String> randomEnds = RandomData.pickMultiple(50000, () -> RandomData.randomString(25));
+
+        db.executeTransaction(new StoreJoinAddressTransaction(correct));
+        db.executeTransaction(new Transaction() {
+            @Override
+            protected void performOperations() {
+                for (String randomEnd : randomEnds) {
+                    executeOther(new StoreJoinAddressTransaction(badPrefix + randomEnd));
+                }
+            }
+        });
+
+        long start = System.currentTimeMillis();
+        BadJoinAddressDataCorrectionPatch patch = new BadJoinAddressDataCorrectionPatch();
+        db.executeTransaction(patch);
+        assertTrue(patch.wasApplied());
+        long end = System.currentTimeMillis();
+        long diff = end - start;
+        assertTrue(diff < TimeUnit.SECONDS.toMillis(10L), () -> "Took too long! " + diff + " ms");
+
+        Set<String> expected = Set.of(correct, JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP);
+        Set<String> result = new HashSet<>(db.query(JoinAddressQueries.allJoinAddresses()));
+        assertEquals(expected, result);
+    }
+
+}

Plan/common/src/test/java/utilities/mocks/PluginMockComponent.java

@@ -59,7 +59,7 @@ public class PluginMockComponent {
         initComponent();
         PlanSystem system = component.system();
         system.getConfigSystem().getConfig().set(WebserverSettings.PORT, ThreadLocalRandom.current()
-                .nextInt(65535 - 10240) + 10240); // Random non-privileged port
+                .nextInt(10240) + 10240); // Random non-privileged port
         return system;
     }