From 7463d4e440c0cd4c9db07a7c9e4949e7dcf60bd9 Mon Sep 17 00:00:00 2001 From: Aurora Lahtela <24460436+AuroraLS3@users.noreply.github.com> Date: Mon, 11 Mar 2024 20:52:41 +0200 Subject: [PATCH] Fix exception related to CONCAT on SQLite in Extension boolean storage Affects issues: - Fixed #3514 --- .../StorePlayerBooleanResultTransaction.java | 4 +- .../StoreServerBooleanResultTransaction.java | 4 +- .../storage/database/sql/building/Sql.java | 10 +++ .../patches/SecurityTableGroupPatch.java | 4 +- .../queries/ExtensionsDatabaseTest.java | 80 +++++++++++++++++++ 5 files changed, 96 insertions(+), 6 deletions(-) diff --git a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StorePlayerBooleanResultTransaction.java b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StorePlayerBooleanResultTransaction.java index 070a794bf..82ebb1e05 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StorePlayerBooleanResultTransaction.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StorePlayerBooleanResultTransaction.java @@ -155,7 +155,7 @@ public class StorePlayerBooleanResultTransaction extends ThrowawayTransaction { "FROM plan_extension_providers indb " + "JOIN plan_extension_providers unfulfilled ON unfulfilled.condition_name=" + // This gives the unfulfilled condition, eg. if value is true not_condition is unfulfilled. - (value ? "CONCAT('not_', " : "") + "indb.provided_condition" + (value ? ")" : "") + + (value ? Sql.concat(dbType, "'not_'", "indb.provided_condition") : "indb.provided_condition") + " AND indb.plugin_id=unfulfilled.plugin_id" + " WHERE indb.id=" + ExtensionProviderTable.STATEMENT_SELECT_PROVIDER_ID + " AND indb.provided_condition IS NOT NULL"; @@ -181,7 +181,7 @@ public class StorePlayerBooleanResultTransaction extends ThrowawayTransaction { "FROM plan_extension_providers indb " + "JOIN plan_extension_tables unfulfilled ON unfulfilled.condition_name=" + // This gives the unfulfilled condition, eg. if value is true not_condition is unfulfilled. - (value ? "CONCAT('not_', " : "") + "indb.provided_condition" + (value ? ")" : "") + + (value ? Sql.concat(dbType, "'not_'", "indb.provided_condition") : "indb.provided_condition") + " AND indb.plugin_id=unfulfilled.plugin_id" + " WHERE indb.id=" + ExtensionProviderTable.STATEMENT_SELECT_PROVIDER_ID + " AND indb.provided_condition IS NOT NULL"; diff --git a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StoreServerBooleanResultTransaction.java b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StoreServerBooleanResultTransaction.java index 4f9cbcfef..87516963e 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StoreServerBooleanResultTransaction.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/transactions/results/StoreServerBooleanResultTransaction.java @@ -134,7 +134,7 @@ public class StoreServerBooleanResultTransaction extends ThrowawayTransaction { "FROM plan_extension_providers indb " + "JOIN plan_extension_providers unfulfilled ON unfulfilled.condition_name=" + // This gives the unfulfilled condition, eg. if value is true not_condition is unfulfilled. - (value ? "CONCAT('not_', " : "") + "indb.provided_condition" + (value ? ")" : "") + + (value ? Sql.concat(dbType, "'not_'", "indb.provided_condition") : "indb.provided_condition") + " AND indb.plugin_id=unfulfilled.plugin_id" + " WHERE indb.id=" + ExtensionProviderTable.STATEMENT_SELECT_PROVIDER_ID + " AND indb.provided_condition IS NOT NULL"; @@ -166,7 +166,7 @@ public class StoreServerBooleanResultTransaction extends ThrowawayTransaction { "FROM plan_extension_providers indb " + "JOIN plan_extension_tables unfulfilled ON unfulfilled.condition_name=" + // This gives the unfulfilled condition, eg. if value is true not_condition is unfulfilled. - (value ? "CONCAT('not_', " : "") + "indb.provided_condition" + (value ? ")" : "") + + (value ? Sql.concat(dbType, "'not_'", "indb.provided_condition") : "indb.provided_condition") + " AND indb.plugin_id=unfulfilled.plugin_id" + " WHERE indb.id=" + ExtensionProviderTable.STATEMENT_SELECT_PROVIDER_ID + " AND indb.provided_condition IS NOT NULL"; diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/building/Sql.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/building/Sql.java index 6641054b8..fc00e0423 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/building/Sql.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/building/Sql.java @@ -16,6 +16,7 @@ */ package com.djrapitops.plan.storage.database.sql.building; +import com.djrapitops.plan.storage.database.DBType; import org.apache.commons.text.TextStringBuilder; import java.sql.PreparedStatement; @@ -96,6 +97,15 @@ public abstract class Sql { } } + public static String concat(DBType dbType, String one, String two) { + if (dbType == DBType.MYSQL) { + return "CONCAT(" + one + ',' + two + ")"; + } else if (dbType == DBType.SQLITE) { + return one + " || " + two; + } + return one + two; + } + public abstract String epochSecondToDate(String sql); public abstract String dateToEpochSecond(String sql); diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/patches/SecurityTableGroupPatch.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/patches/SecurityTableGroupPatch.java index c71c3aa6f..bc61ee09a 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/patches/SecurityTableGroupPatch.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/transactions/patches/SecurityTableGroupPatch.java @@ -17,7 +17,7 @@ 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.sql.building.Sql; import com.djrapitops.plan.storage.database.sql.tables.webuser.SecurityTable; import com.djrapitops.plan.storage.database.sql.tables.webuser.WebGroupTable; @@ -71,7 +71,7 @@ public class SecurityTableGroupPatch extends Patch { SecurityTable.USERNAME + ',' + SecurityTable.LINKED_TO + ',' + SecurityTable.SALT_PASSWORD_HASH + ',' + - "(" + SELECT + WebGroupTable.ID + FROM + WebGroupTable.TABLE_NAME + WHERE + WebGroupTable.NAME + "=" + (dbType == DBType.SQLITE ? "'legacy_level_' || permission_level" : "CONCAT('legacy_level_', permission_level)") + ")" + + "(" + SELECT + WebGroupTable.ID + FROM + WebGroupTable.TABLE_NAME + WHERE + WebGroupTable.NAME + "=" + Sql.concat(dbType, "'legacy_level_'", "permission_level") + ")" + FROM + tempTableName ); diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/ExtensionsDatabaseTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/ExtensionsDatabaseTest.java index 4df3d3d8a..36570e474 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/ExtensionsDatabaseTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/ExtensionsDatabaseTest.java @@ -234,6 +234,58 @@ public interface ExtensionsDatabaseTest extends DatabaseTestPreparer { checkThatPlayerDataExists(ConditionalExtension.condition); } + @Test + default void unsatisfiedPlayerConditionalResultsAreCleanedCompletely() { + db().executeTransaction(new PlayerRegisterTransaction(playerUUID, System::currentTimeMillis, TestConstants.PLAYER_ONE_NAME)); + + ExtensionSvc extensionService = extensionService(); + + extensionService.register(new RemovingConditionalExtension()); + + RemovingConditionalExtension.condition = true; + extensionService.updatePlayerValues(playerUUID, TestConstants.PLAYER_ONE_NAME, CallEvents.MANUAL); + + List ofServer = db().query(new ExtensionPlayerDataQuery(playerUUID)).get(serverUUID()); + assertTrue(ofServer != null && !ofServer.isEmpty() && !ofServer.get(0).getTabs().isEmpty(), "There was no data left"); + ExtensionTabData tabData = ofServer.get(0).getTabs().get(0); + assertEquals(RemovingConditionalExtension.condition, tabData.getString("conditionalValue").isPresent()); + + // Reverse condition + RemovingConditionalExtension.condition = false; + extensionService.updatePlayerValues(playerUUID, TestConstants.PLAYER_ONE_NAME, CallEvents.MANUAL); + + ofServer = db().query(new ExtensionPlayerDataQuery(playerUUID)).get(serverUUID()); + assertTrue(ofServer != null && !ofServer.isEmpty() && !ofServer.get(0).getTabs().isEmpty(), "There was no data left"); + tabData = ofServer.get(0).getTabs().get(0); + assertEquals(RemovingConditionalExtension.condition, tabData.getString("conditionalValue").isPresent()); + } + + @Test + default void unsatisfiedServerConditionalResultsAreCleanedCompletely() { + db().executeTransaction(new PlayerRegisterTransaction(playerUUID, System::currentTimeMillis, TestConstants.PLAYER_ONE_NAME)); + + ExtensionSvc extensionService = extensionService(); + + extensionService.register(new RemovingConditionalExtension()); + + RemovingConditionalExtension.condition = true; + extensionService.updateServerValues(CallEvents.MANUAL); + + List ofServer = db().query(new ExtensionServerDataQuery(serverUUID())); + assertTrue(ofServer != null && !ofServer.isEmpty() && !ofServer.get(0).getTabs().isEmpty(), "There was no data left"); + ExtensionTabData tabData = ofServer.get(0).getTabs().get(0); + assertEquals(RemovingConditionalExtension.condition, tabData.getString("conditionalValue").isPresent()); + + // Reverse condition + RemovingConditionalExtension.condition = false; + extensionService.updateServerValues(CallEvents.MANUAL); + + ofServer = db().query(new ExtensionServerDataQuery(serverUUID())); + assertTrue(ofServer != null && !ofServer.isEmpty() && !ofServer.get(0).getTabs().isEmpty(), "There was no data left"); + tabData = ofServer.get(0).getTabs().get(0); + assertEquals(RemovingConditionalExtension.condition, tabData.getString("conditionalValue").isPresent()); + } + default void checkThatPlayerDataExists(boolean condition) { if (condition) { // Condition is true, conditional values exist List ofServer = db().query(new ExtensionPlayerDataQuery(playerUUID)).get(serverUUID()); @@ -437,6 +489,34 @@ public interface ExtensionsDatabaseTest extends DatabaseTestPreparer { } } + @PluginInfo(name = "ConditionalExtension") + class RemovingConditionalExtension implements DataExtension { + + static boolean condition = true; + + @BooleanProvider(text = "a boolean", conditionName = "condition") + public boolean isCondition(UUID playerUUID) { + return condition; + } + + @StringProvider(text = "Conditional Value") + @Conditional("condition") + public String conditionalValue(UUID playerUUID) { + return "Conditional"; + } + + @BooleanProvider(text = "a boolean", conditionName = "condition") + public boolean isCondition() { + return condition; + } + + @StringProvider(text = "Conditional Value") + @Conditional("condition") + public String conditionalValue() { + return "Conditional"; + } + } + @PluginInfo(name = "ServerExtension") class ServerExtension implements DataExtension { @NumberProvider(text = "a number")