Skip to content

Commit

Permalink
Fix top category queries
Browse files Browse the repository at this point in the history
- Moved group by before order by
- Fixed off by one error in the offset

Affects issues:
- Fixed #2206
  • Loading branch information
AuroraLS3 committed Jan 9, 2022
1 parent 4974aff commit ed17ebd
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ public static Query<Optional<String>> fetchNthTop10PlaytimePlayerOn(ServerUUID s
WHERE + SessionsTable.SERVER_UUID + "=?" +
AND + SessionsTable.SESSION_START + ">?" +
AND + SessionsTable.SESSION_END + "<?" +
ORDER_BY + "playtime DESC" +
GROUP_BY + "name" +
ORDER_BY + "playtime DESC" +
LIMIT + "10" +
OFFSET + "?";

Expand All @@ -54,7 +54,7 @@ public void prepare(PreparedStatement statement) throws SQLException {
statement.setString(1, serverUUID.toString());
statement.setLong(2, after);
statement.setLong(3, before);
statement.setInt(4, n);
statement.setInt(4, n - 1);
}

@Override
Expand All @@ -76,8 +76,8 @@ public static Query<Optional<String>> fetchNthTop10ActivePlaytimePlayerOn(Server
WHERE + SessionsTable.SERVER_UUID + "=?" +
AND + SessionsTable.SESSION_START + ">?" +
AND + SessionsTable.SESSION_END + "<?" +
ORDER_BY + "active_playtime DESC" +
GROUP_BY + "name" +
ORDER_BY + "active_playtime DESC" +
LIMIT + "10" +
OFFSET + "?";

Expand All @@ -87,7 +87,7 @@ public void prepare(PreparedStatement statement) throws SQLException {
statement.setString(1, serverUUID.toString());
statement.setLong(2, after);
statement.setLong(3, before);
statement.setInt(4, n);
statement.setInt(4, n - 1);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
import com.djrapitops.plan.identification.ServerInfo;
import com.djrapitops.plan.identification.ServerUUID;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.storage.database.queries.*;
import com.djrapitops.plan.storage.database.queries.ExtensionsDatabaseTest;
import com.djrapitops.plan.storage.database.queries.QueriesTestAggregate;
import com.djrapitops.plan.storage.database.transactions.StoreServerInformationTransaction;
import com.djrapitops.plan.storage.database.transactions.commands.RemoveEverythingTransaction;
import com.djrapitops.plan.storage.database.transactions.init.CreateTablesTransaction;
Expand Down Expand Up @@ -59,18 +60,7 @@
* @see utilities.CIProperties for assumed MySQL setup.
*/
@ExtendWith(MockitoExtension.class)
class MySQLTest implements DatabaseTest,
DatabaseBackupTest,
ExtensionsDatabaseTest,
ActivityIndexQueriesTest,
GeolocationQueriesTest,
NicknameQueriesTest,
PingQueriesTest,
SessionQueriesTest,
ServerQueriesTest,
TPSQueriesTest,
UserInfoQueriesTest,
WebUserQueriesTest {
class MySQLTest implements DatabaseTest, QueriesTestAggregate {

private static final int TEST_PORT_NUMBER = RandomData.randomInt(9005, 9500);

Expand Down Expand Up @@ -113,6 +103,7 @@ public void applyPatch() {
db().executeTransaction(new StoreServerInformationTransaction(new Server(serverUUID(), TestConstants.SERVER_NAME, "")));
assertEquals(serverUUID(), ((SQLDB) db()).getServerUUIDSupplier().get());
}

@AfterAll
static void disableSystem() {
if (database != null) database.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
import com.djrapitops.plan.identification.ServerInfo;
import com.djrapitops.plan.identification.ServerUUID;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.storage.database.queries.*;
import com.djrapitops.plan.storage.database.queries.ExtensionsDatabaseTest;
import com.djrapitops.plan.storage.database.queries.QueriesTestAggregate;
import com.djrapitops.plan.storage.database.transactions.StoreServerInformationTransaction;
import com.djrapitops.plan.storage.database.transactions.commands.RemoveEverythingTransaction;
import com.djrapitops.plan.storage.database.transactions.init.CreateTablesTransaction;
Expand Down Expand Up @@ -52,18 +53,7 @@
* @see ExtensionsDatabaseTest
*/
@ExtendWith(MockitoExtension.class)
public class SQLiteTest implements DatabaseTest,
DatabaseBackupTest,
ExtensionsDatabaseTest,
ActivityIndexQueriesTest,
GeolocationQueriesTest,
NicknameQueriesTest,
PingQueriesTest,
SessionQueriesTest,
ServerQueriesTest,
TPSQueriesTest,
UserInfoQueriesTest,
WebUserQueriesTest {
public class SQLiteTest implements DatabaseTest, QueriesTestAggregate {

private static final int TEST_PORT_NUMBER = RandomData.randomInt(9005, 9500);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.queries;

import com.djrapitops.plan.storage.database.queries.analysis.TopListQueriesTest;

public interface QueriesTestAggregate extends
ActivityIndexQueriesTest,
DatabaseBackupTest,
ExtensionsDatabaseTest,
GeolocationQueriesTest,
NicknameQueriesTest,
PingQueriesTest,
ServerQueriesTest,
SessionQueriesTest,
TopListQueriesTest,
TPSQueriesTest,
UserInfoQueriesTest,
WebUserQueriesTest {
/* Collects all query tests together so its easier to implement database tests */
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* 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.queries.analysis;

import com.djrapitops.plan.gathering.domain.FinishedSession;
import com.djrapitops.plan.storage.database.DatabaseTestPreparer;
import com.djrapitops.plan.storage.database.queries.DataStoreQueries;
import com.djrapitops.plan.storage.database.transactions.events.PlayerServerRegisterTransaction;
import com.djrapitops.plan.storage.database.transactions.events.WorldNameStoreTransaction;
import org.junit.jupiter.api.Test;
import utilities.RandomData;
import utilities.TestConstants;

import static org.junit.jupiter.api.Assertions.assertEquals;

public interface TopListQueriesTest extends DatabaseTestPreparer {

private void storeSessionForTopListQueries() {
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));
db().executeTransaction(new PlayerServerRegisterTransaction(player2UUID, RandomData::randomTime,
TestConstants.PLAYER_TWO_NAME, serverUUID(), TestConstants.GET_PLAYER_HOSTNAME));
FinishedSession session = RandomData.randomSession(serverUUID(), worlds, playerUUID, player2UUID);
execute(DataStoreQueries.storeSession(session));
}

@Test
default void topActivePlaytimeListQueryReturnsSinglePlayer() {
storeSessionForTopListQueries();

String expected = TestConstants.PLAYER_ONE_NAME;
String result = db().query(TopListQueries.fetchNthTop10ActivePlaytimePlayerOn(serverUUID(), 1, 0, System.currentTimeMillis()))
.orElseThrow(AssertionError::new);
assertEquals(expected, result);
}

@Test
default void topPlaytimeListQueryReturnsSinglePlayer() {
storeSessionForTopListQueries();

String expected = TestConstants.PLAYER_ONE_NAME;
String result = db().query(TopListQueries.fetchNthTop10ActivePlaytimePlayerOn(serverUUID(), 1, 0, System.currentTimeMillis()))
.orElseThrow(AssertionError::new);
assertEquals(expected, result);
}

}

0 comments on commit ed17ebd

Please sign in to comment.