From 022a69044643e6386767985e5d9157030d42d064 Mon Sep 17 00:00:00 2001 From: Aurora Lahtela <24460436+AuroraLS3@users.noreply.github.com> Date: Wed, 23 Nov 2022 17:31:04 +0200 Subject: [PATCH] Implemented parameterized array Sql generation This bit of code allows passing lists of data in parameterized queries, which is useful for defining WHERE x IN (?,?,?,?) in a dynamic way. - Adds AccessControlTest cases for a valid /v1/query call --- .../ExtensionQueryResultTableDataQuery.java | 41 ++++++----- .../queries/QueryParameterSetter.java | 17 ++++- .../storage/database/sql/building/Sql.java | 9 +++ .../delivery/webserver/AccessControlTest.java | 4 ++ .../database/sql/building/SqlTest.java | 70 +++++++++++++++++++ 5 files changed, 118 insertions(+), 23 deletions(-) create mode 100644 Plan/common/src/test/java/com/djrapitops/plan/storage/database/sql/building/SqlTest.java diff --git a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/queries/ExtensionQueryResultTableDataQuery.java b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/queries/ExtensionQueryResultTableDataQuery.java index ce2cd9cdb..5171354a2 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/queries/ExtensionQueryResultTableDataQuery.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/queries/ExtensionQueryResultTableDataQuery.java @@ -25,6 +25,7 @@ import com.djrapitops.plan.identification.ServerUUID; import com.djrapitops.plan.storage.database.SQLDB; import com.djrapitops.plan.storage.database.queries.Query; import com.djrapitops.plan.storage.database.queries.QueryStatement; +import com.djrapitops.plan.storage.database.sql.building.Sql; import com.djrapitops.plan.storage.database.sql.tables.*; import org.apache.commons.text.TextStringBuilder; @@ -79,7 +80,7 @@ public class ExtensionQueryResultTableDataQuery implements Query> fetchPlayerData() { String selectUuids = SELECT + UsersTable.USER_UUID + FROM + UsersTable.TABLE_NAME + - WHERE + UsersTable.ID + " IN (" + new TextStringBuilder().appendWithSeparators(userIds, ",") + ")"; + WHERE + UsersTable.ID + " IN (" + Sql.nParameters(userIds.size()) + ")"; String sql = SELECT + "v1." + ExtensionPlayerValueTable.USER_UUID + " as uuid," + @@ -104,19 +105,13 @@ public class ExtensionQueryResultTableDataQuery implements Query(sql, 1000) { - @Override - public void prepare(PreparedStatement statement) throws SQLException { - statement.setBoolean(1, true); // Select only values that should be shown - statement.setBoolean(2, false); // Don't select player_name String values - statement.setString(3, serverUUID.toString()); - } - - @Override - public Map processResults(ResultSet set) throws SQLException { - return extractDataByPlayer(set); - } - }; + return db -> db.queryMap(sql, this::extractPlayer, HashMap::new, + userIds, + true, // Select only values that should be shown + false, // Don't select player_name String values + serverUUID) + .entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().build())); } private Query> fetchPlayerGroups() { @@ -155,17 +150,21 @@ public class ExtensionQueryResultTableDataQuery implements Query dataByPlayer = new HashMap<>(); while (set.next()) { - UUID playerUUID = UUID.fromString(set.getString("uuid")); - ExtensionTabData.Builder data = dataByPlayer.getOrDefault(playerUUID, new ExtensionTabData.Builder(null)); - - ExtensionDescription extensionDescription = extractDescription(set); - extractAndPutDataTo(data, extensionDescription, set); - - dataByPlayer.put(playerUUID, data); + extractPlayer(set, dataByPlayer); } return dataByPlayer.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().build())); } + private void extractPlayer(ResultSet set, Map dataByPlayer) throws SQLException { + UUID playerUUID = UUID.fromString(set.getString("uuid")); + ExtensionTabData.Builder data = dataByPlayer.getOrDefault(playerUUID, new ExtensionTabData.Builder(null)); + + ExtensionDescription extensionDescription = extractDescription(set); + extractAndPutDataTo(data, extensionDescription, set); + + dataByPlayer.put(playerUUID, data); + } + private void extractAndPutDataTo(ExtensionTabData.Builder extensionTab, ExtensionDescription description, ResultSet set) throws SQLException { String groupValue = set.getString("group_value"); if (groupValue != null) { diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/QueryParameterSetter.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/QueryParameterSetter.java index c91de3f6d..9265f9e7d 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/QueryParameterSetter.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/QueryParameterSetter.java @@ -21,6 +21,7 @@ import com.djrapitops.plan.identification.ServerUUID; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Types; +import java.util.Collection; import java.util.UUID; public class QueryParameterSetter { @@ -30,8 +31,20 @@ public class QueryParameterSetter { public static void setParameters(PreparedStatement statement, Object... parameters) throws SQLException { int index = 1; for (Object parameter : parameters) { - setParameter(statement, index, parameter); - index++; + if (parameter instanceof Object[]) { + for (Object arrayParameter : ((Object[]) parameter)) { + setParameter(statement, index, arrayParameter); + index++; + } + } else if (parameter instanceof Collection) { + for (Object collectionParameter : ((Collection) parameter)) { + setParameter(statement, index, collectionParameter); + index++; + } + } else { + setParameter(statement, index, parameter); + index++; + } } } 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 1310faf78..6641054b8 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,10 +16,13 @@ */ package com.djrapitops.plan.storage.database.sql.building; +import org.apache.commons.text.TextStringBuilder; + import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Types; import java.util.concurrent.TimeUnit; +import java.util.stream.IntStream; /** * Duplicate String reducing utility class for SQL language Strings. @@ -57,6 +60,12 @@ public abstract class Sql { private static final String MAX = "MAX("; private static final String VARCHAR = "varchar("; + public static String nParameters(int n) { + return new TextStringBuilder() + .appendWithSeparators(IntStream.range(0, n).mapToObj(i -> "?").iterator(), ",") + .toString(); + } + public static String varchar(int length) { return VARCHAR + length + ')'; } diff --git a/Plan/common/src/test/java/com/djrapitops/plan/delivery/webserver/AccessControlTest.java b/Plan/common/src/test/java/com/djrapitops/plan/delivery/webserver/AccessControlTest.java index 3684dd451..f516066b6 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/delivery/webserver/AccessControlTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/delivery/webserver/AccessControlTest.java @@ -205,6 +205,7 @@ class AccessControlTest { "/query,200", "/v1/filters,200", "/v1/query,400", + "/v1/query?q=%5B%5D&view=%7B%22afterDate%22%3A%2224%2F10%2F2022%22%2C%22afterTime%22%3A%2218%3A21%22%2C%22beforeDate%22%3A%2223%2F11%2F2022%22%2C%22beforeTime%22%3A%2217%3A21%22%2C%22servers%22%3A%5B%0A%7B%22serverUUID%22%3A%22" + TestConstants.SERVER_UUID_STRING + "%22%2C%22serverName%22%3A%22" + TestConstants.SERVER_NAME + "%22%2C%22proxy%22%3Afalse%7D%5D%7D,200", "/v1/errors,200", "/errors,200", "/v1/network/listServers,200", @@ -278,6 +279,7 @@ class AccessControlTest { "/query,200", "/v1/filters,200", "/v1/query,400", + "/v1/query?q=%5B%5D&view=%7B%22afterDate%22%3A%2224%2F10%2F2022%22%2C%22afterTime%22%3A%2218%3A21%22%2C%22beforeDate%22%3A%2223%2F11%2F2022%22%2C%22beforeTime%22%3A%2217%3A21%22%2C%22servers%22%3A%5B%0A%7B%22serverUUID%22%3A%22" + TestConstants.SERVER_UUID_STRING + "%22%2C%22serverName%22%3A%22" + TestConstants.SERVER_NAME + "%22%2C%22proxy%22%3Afalse%7D%5D%7D,200", "/v1/errors,403", "/errors,403", "/v1/network/listServers,403", @@ -351,6 +353,7 @@ class AccessControlTest { "/query,403", "/v1/filters,403", "/v1/query,403", + "/v1/query?q=%5B%5D&view=%7B%22afterDate%22%3A%2224%2F10%2F2022%22%2C%22afterTime%22%3A%2218%3A21%22%2C%22beforeDate%22%3A%2223%2F11%2F2022%22%2C%22beforeTime%22%3A%2217%3A21%22%2C%22servers%22%3A%5B%0A%7B%22serverUUID%22%3A%22" + TestConstants.SERVER_UUID_STRING + "%22%2C%22serverName%22%3A%22" + TestConstants.SERVER_NAME + "%22%2C%22proxy%22%3Afalse%7D%5D%7D,403", "/v1/errors,403", "/errors,403", "/v1/network/listServers,403", @@ -423,6 +426,7 @@ class AccessControlTest { "/v1/players,403", "/query,403", "/v1/filters,403", + "/v1/query?q=%5B%5D&view=%7B%22afterDate%22%3A%2224%2F10%2F2022%22%2C%22afterTime%22%3A%2218%3A21%22%2C%22beforeDate%22%3A%2223%2F11%2F2022%22%2C%22beforeTime%22%3A%2217%3A21%22%2C%22servers%22%3A%5B%0A%7B%22serverUUID%22%3A%22" + TestConstants.SERVER_UUID_STRING + "%22%2C%22serverName%22%3A%22" + TestConstants.SERVER_NAME + "%22%2C%22proxy%22%3Afalse%7D%5D%7D,403", "/v1/query,403", "/v1/network/listServers,403", "/v1/network/serverOptions,403", diff --git a/Plan/common/src/test/java/com/djrapitops/plan/storage/database/sql/building/SqlTest.java b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/sql/building/SqlTest.java new file mode 100644 index 000000000..b61bd7057 --- /dev/null +++ b/Plan/common/src/test/java/com/djrapitops/plan/storage/database/sql/building/SqlTest.java @@ -0,0 +1,70 @@ +/* + * 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 . + */ +package com.djrapitops.plan.storage.database.sql.building; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.RepeatedTest; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import utilities.RandomData; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author AuroraLS3 + */ +class SqlTest { + + @ParameterizedTest(name = "Generating {0} parameters generates {1}") + @CsvSource(delimiter = ';', value = { + "1;?", + "2;?,?", + "5;?,?,?,?,?" + }) + void nParametersReturns(Integer n, String expected) { + String result = Sql.nParameters(n); + assertEquals(expected, result); + } + + @Test + @DisplayName("Generating 0 parameters generates '' (empty string)") + void zeroParametersReturnsEmpty() { + String result = Sql.nParameters(0); + assertEquals("", result); + } + + @RepeatedTest(10) + @DisplayName("Generating n (random) parameters generates n '?' characters and n-1 ',' characters") + void randomParametersReturns() { + int n = RandomData.randomInt(10, 50); + + String result = Sql.nParameters(n); + int questions = 0; + int commas = 0; + for (char c : result.toCharArray()) { + if (c == '?') { + questions++; + } else if (c == ',') { + commas++; + } + } + assertEquals(n, questions); + assertEquals(n - 1, commas); + } + +} \ No newline at end of file