From 0799674de3a5602fce62bbb71128f84243f499ed Mon Sep 17 00:00:00 2001 From: Rsl1122 Date: Thu, 31 Jan 2019 21:04:47 +0200 Subject: [PATCH] Refactored GeoInfoTable#getNetworkGeolocations to a query: - Changed WorldMap to use the Map instead of calculating from a list, saves memory. - Network container now uses the new query to save memory --- .../store/containers/NetworkContainer.java | 11 +++--- .../queries/ServerAggregateQueries.java | 23 ++++++++++++ .../plan/db/sql/tables/GeoInfoTable.java | 22 ------------ .../graphs/special/SpecialGraphFactory.java | 5 +++ .../html/graphs/special/WorldMap.java | 32 +++++++++++------ .../com/djrapitops/plan/db/CommonDBTest.java | 28 +-------------- .../com/djrapitops/plan/db/MySQLTest.java | 36 +++++++++++++++++++ .../com/djrapitops/plan/db/SQLiteTest.java | 31 ++++++++++++++++ 8 files changed, 124 insertions(+), 64 deletions(-) diff --git a/Plan/common/src/main/java/com/djrapitops/plan/data/store/containers/NetworkContainer.java b/Plan/common/src/main/java/com/djrapitops/plan/data/store/containers/NetworkContainer.java index 01a7aeb78..677b6301a 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/data/store/containers/NetworkContainer.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/data/store/containers/NetworkContainer.java @@ -23,6 +23,7 @@ import com.djrapitops.plan.data.store.keys.ServerKeys; import com.djrapitops.plan.data.store.mutators.PlayersMutator; import com.djrapitops.plan.data.store.mutators.TPSMutator; import com.djrapitops.plan.data.store.mutators.health.NetworkHealthInformation; +import com.djrapitops.plan.db.Database; import com.djrapitops.plan.db.access.queries.ServerAggregateQueries; import com.djrapitops.plan.system.database.DBSystem; import com.djrapitops.plan.system.info.server.Server; @@ -63,10 +64,10 @@ public class NetworkContainer extends DataContainer { private final PlanConfig config; private final Locale locale; private final Theme theme; - private final DBSystem dbSystem; private final ServerProperties serverProperties; private final Formatters formatters; private final Graphs graphs; + private final Database database; public NetworkContainer( ServerContainer bungeeContainer, @@ -84,7 +85,7 @@ public class NetworkContainer extends DataContainer { this.config = config; this.locale = locale; this.theme = theme; - this.dbSystem = dbSystem; + this.database = dbSystem.getDatabase(); this.serverProperties = serverProperties; this.formatters = formatters; this.graphs = graphs; @@ -98,13 +99,13 @@ public class NetworkContainer extends DataContainer { } private void addServerBoxes() { - putSupplier(NetworkKeys.NETWORK_PLAYER_ONLINE_DATA, () -> dbSystem.getDatabase().fetch().getPlayersOnlineForServers( + putSupplier(NetworkKeys.NETWORK_PLAYER_ONLINE_DATA, () -> database.fetch().getPlayersOnlineForServers( getValue(NetworkKeys.BUKKIT_SERVERS).orElse(new ArrayList<>())) ); putSupplier(NetworkKeys.SERVERS_TAB, () -> { StringBuilder serverBoxes = new StringBuilder(); Map> playersOnlineData = getValue(NetworkKeys.NETWORK_PLAYER_ONLINE_DATA).orElse(new HashMap<>()); - Map userCounts = dbSystem.getDatabase().query(ServerAggregateQueries.serverUserCounts()); + Map userCounts = database.query(ServerAggregateQueries.serverUserCounts()); Collection servers = getValue(NetworkKeys.BUKKIT_SERVERS).orElse(new ArrayList<>()); servers.stream() .sorted((one, two) -> String.CASE_INSENSITIVE_ORDER.compare(one.getName(), two.getName())) @@ -168,7 +169,7 @@ public class NetworkContainer extends DataContainer { private void addPlayerInformation() { putSupplier(NetworkKeys.PLAYERS_TOTAL, () -> getUnsafe(NetworkKeys.PLAYERS_MUTATOR).count()); putSupplier(NetworkKeys.WORLD_MAP_SERIES, () -> - graphs.special().worldMap(PlayersMutator.forContainer(bungeeContainer)).toHighChartsSeries() + graphs.special().worldMap(database.query(ServerAggregateQueries.networkGeolocationCounts())).toHighChartsSeries() ); Key geolocationBarChart = new Key<>(BarGraph.class, "GEOLOCATION_BAR_GRAPH"); putSupplier(geolocationBarChart, () -> graphs.bar().geolocationBarGraph(getUnsafe(NetworkKeys.PLAYERS_MUTATOR))); diff --git a/Plan/common/src/main/java/com/djrapitops/plan/db/access/queries/ServerAggregateQueries.java b/Plan/common/src/main/java/com/djrapitops/plan/db/access/queries/ServerAggregateQueries.java index 22f77777b..e5f3c610c 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/db/access/queries/ServerAggregateQueries.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/db/access/queries/ServerAggregateQueries.java @@ -180,4 +180,27 @@ public class ServerAggregateQueries { } }; } + + public static Query> networkGeolocationCounts() { + String subQuery = "SELECT " + + GeoInfoTable.GEOLOCATION + ", " + + GeoInfoTable.LAST_USED + ", " + + "MAX(" + GeoInfoTable.LAST_USED + ") as m" + + " FROM " + GeoInfoTable.TABLE_NAME + + " GROUP BY " + GeoInfoTable.USER_UUID; + String sql = "SELECT " + GeoInfoTable.GEOLOCATION + ", COUNT(1) as c FROM (" + subQuery + ") AS q1" + + " WHERE " + GeoInfoTable.LAST_USED + " = m" + + " GROUP BY " + GeoInfoTable.GEOLOCATION; + + return new QueryAllStatement>(sql) { + @Override + public Map processResults(ResultSet set) throws SQLException { + Map geolocationCounts = new HashMap<>(); + while (set.next()) { + geolocationCounts.put(set.getString(GeoInfoTable.GEOLOCATION), set.getInt("c")); + } + return geolocationCounts; + } + }; + } } \ No newline at end of file diff --git a/Plan/common/src/main/java/com/djrapitops/plan/db/sql/tables/GeoInfoTable.java b/Plan/common/src/main/java/com/djrapitops/plan/db/sql/tables/GeoInfoTable.java index 34a5966e3..ad355ed44 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/db/sql/tables/GeoInfoTable.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/db/sql/tables/GeoInfoTable.java @@ -16,19 +16,11 @@ */ package com.djrapitops.plan.db.sql.tables; -import com.djrapitops.plan.data.container.GeoInfo; import com.djrapitops.plan.db.DBType; import com.djrapitops.plan.db.SQLDB; -import com.djrapitops.plan.db.access.queries.LargeFetchQueries; import com.djrapitops.plan.db.patches.*; import com.djrapitops.plan.db.sql.parsing.CreateTableParser; import com.djrapitops.plan.db.sql.parsing.Sql; -import com.djrapitops.plan.utilities.comparators.GeoInfoComparator; - -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.UUID; /** * Table that is in charge of storing common IP and Geolocation data for users. @@ -84,18 +76,4 @@ public class GeoInfoTable extends Table { .toString(); } - public List getNetworkGeolocations() { - List geolocations = new ArrayList<>(); - - Map> geoInfo = db.query(LargeFetchQueries.fetchAllGeoInformation()); - for (List userGeoInfos : geoInfo.values()) { - if (userGeoInfos.isEmpty()) { - continue; - } - userGeoInfos.sort(new GeoInfoComparator()); - geolocations.add(userGeoInfos.get(0).getGeolocation()); - } - - return geolocations; - } } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/utilities/html/graphs/special/SpecialGraphFactory.java b/Plan/common/src/main/java/com/djrapitops/plan/utilities/html/graphs/special/SpecialGraphFactory.java index cccbfef90..f84c33eaf 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/utilities/html/graphs/special/SpecialGraphFactory.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/utilities/html/graphs/special/SpecialGraphFactory.java @@ -23,6 +23,7 @@ import com.djrapitops.plan.utilities.html.graphs.HighChart; import javax.inject.Inject; import javax.inject.Singleton; import java.util.Collection; +import java.util.Map; /** * Factory class for different objects representing special HTML graphs. @@ -41,6 +42,10 @@ public class SpecialGraphFactory { return new PunchCard(sessions); } + public HighChart worldMap(Map geolocationCounts) { + return new WorldMap(geolocationCounts); + } + public HighChart worldMap(PlayersMutator mutator) { return new WorldMap(mutator); } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/utilities/html/graphs/special/WorldMap.java b/Plan/common/src/main/java/com/djrapitops/plan/utilities/html/graphs/special/WorldMap.java index cde060009..60e19373c 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/utilities/html/graphs/special/WorldMap.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/utilities/html/graphs/special/WorldMap.java @@ -23,6 +23,7 @@ import org.apache.commons.text.TextStringBuilder; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; /** * World Map that uses iso-a3 specification of Country codes. @@ -31,13 +32,17 @@ import java.util.Map; */ public class WorldMap implements HighChart { - private final List geoLocations; + private final Map geoCodeCounts; WorldMap(PlayersMutator mutator) { - this.geoLocations = mutator.getGeolocations(); + this.geoCodeCounts = toGeoCodeCounts(mutator.getGeolocations()); } - private static Map getGeoCodes(Map geoCodeCounts) { + WorldMap(Map geolocationCounts) { + this.geoCodeCounts = toGeoCodeCounts(geolocationCounts); + } + + private static Map getGeoCodes() { Map geoCodes = new HashMap<>(); // Countries & Codes have been copied from a iso-a3 specification file. // Each index corresponds to each code. @@ -48,23 +53,30 @@ public class WorldMap implements HighChart { String countryCode = codes[i]; geoCodes.put(country.toLowerCase(), countryCode); - geoCodeCounts.put(countryCode, 0); } return geoCodes; } - @Override - public String toHighChartsSeries() { + private Map toGeoCodeCounts(Map geolocationCounts) { + Map geoCodes = getGeoCodes(); + return geolocationCounts.entrySet().stream() + .collect(Collectors.toMap(entry -> geoCodes.get(entry.getKey().toLowerCase()), Map.Entry::getValue)); + } + + private Map toGeoCodeCounts(List geoLocations) { Map geoCodeCounts = new HashMap<>(); - Map geoCodes = getGeoCodes(geoCodeCounts); + Map geoCodes = getGeoCodes(); for (String geoLocation : geoLocations) { String countryCode = geoCodes.get(geoLocation.toLowerCase()); - if (countryCode != null) { - geoCodeCounts.computeIfPresent(countryCode, (computedCountry, amount) -> amount + 1); - } + geoCodeCounts.put(countryCode, geoCodeCounts.getOrDefault(countryCode, 0) + 1); } + return geoCodeCounts; + } + + @Override + public String toHighChartsSeries() { TextStringBuilder dataBuilder = new TextStringBuilder("["); dataBuilder.appendWithSeparators( diff --git a/Plan/common/src/test/java/com/djrapitops/plan/db/CommonDBTest.java b/Plan/common/src/test/java/com/djrapitops/plan/db/CommonDBTest.java index c33873a83..6fed389e7 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/db/CommonDBTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/db/CommonDBTest.java @@ -606,7 +606,7 @@ public abstract class CommonDBTest { securityTable.addNewUser(new WebUser("Test", "RandomGarbageBlah", 0)); } - private void saveGeoInfo(UUID uuid, GeoInfo geoInfo) { + void saveGeoInfo(UUID uuid, GeoInfo geoInfo) { db.executeTransaction(new GeoInfoStoreTransaction(uuid, geoInfo)); } @@ -800,32 +800,6 @@ public abstract class CommonDBTest { assertTrue(db.check().isPlayerRegisteredOnThisServer(playerUUID)); } - @Test - public void testGetNetworkGeolocations() { - GeoInfoTable geoInfoTable = db.getGeoInfoTable(); - UUID firstUuid = UUID.randomUUID(); - UUID secondUuid = UUID.randomUUID(); - UUID thirdUuid = UUID.randomUUID(); - - UsersTable usersTable = db.getUsersTable(); - usersTable.registerUser(firstUuid, 0, ""); - usersTable.registerUser(secondUuid, 0, ""); - usersTable.registerUser(thirdUuid, 0, ""); - - saveGeoInfo(firstUuid, new GeoInfo("-", "Test1", 0, "3")); - GeoInfo secondInfo = new GeoInfo("-", "Test2", 5, "3"); - saveGeoInfo(firstUuid, secondInfo); - saveGeoInfo(secondUuid, new GeoInfo("-", "Test3", 0, "3")); - saveGeoInfo(thirdUuid, new GeoInfo("-", "Test4", 0, "3")); - - List geolocations = geoInfoTable.getNetworkGeolocations(); - - assertNotNull(geolocations); - assertFalse(geolocations.isEmpty()); - assertEquals(3, geolocations.size()); - assertTrue(geolocations.contains(secondInfo.getGeolocation())); - } - @Test public void testNewContainerForPlayer() throws NoSuchAlgorithmException { saveAllData(); diff --git a/Plan/common/src/test/java/com/djrapitops/plan/db/MySQLTest.java b/Plan/common/src/test/java/com/djrapitops/plan/db/MySQLTest.java index 08b0aa401..88cc248a5 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/db/MySQLTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/db/MySQLTest.java @@ -16,11 +16,20 @@ */ package com.djrapitops.plan.db; +import com.djrapitops.plan.data.container.GeoInfo; +import com.djrapitops.plan.db.access.queries.ServerAggregateQueries; +import com.djrapitops.plan.db.sql.tables.UsersTable; import com.djrapitops.plan.system.settings.config.PlanConfig; import com.djrapitops.plan.system.settings.paths.DatabaseSettings; import org.junit.BeforeClass; +import org.junit.Test; import utilities.CIProperties; +import java.util.HashMap; +import java.util.Map; +import java.util.UUID; + +import static org.junit.Assert.assertEquals; import static org.junit.Assume.assumeTrue; /** @@ -54,4 +63,31 @@ public class MySQLTest extends CommonDBTest { db.execute("CREATE DATABASE Plan"); db.execute("USE Plan"); } + + @Test + public void networkGeolocationsAreCountedAppropriately() { + UUID firstUuid = UUID.randomUUID(); + UUID secondUuid = UUID.randomUUID(); + UUID thirdUuid = UUID.randomUUID(); + + UsersTable usersTable = db.getUsersTable(); + usersTable.registerUser(firstUuid, 0, ""); + usersTable.registerUser(secondUuid, 0, ""); + usersTable.registerUser(thirdUuid, 0, ""); + + saveGeoInfo(firstUuid, new GeoInfo("-", "Norway", 0, "3")); + saveGeoInfo(firstUuid, new GeoInfo("-", "Finland", 5, "3")); + saveGeoInfo(secondUuid, new GeoInfo("-", "Sweden", 0, "3")); + saveGeoInfo(thirdUuid, new GeoInfo("-", "Denmark", 0, "3")); + + Map got = db.query(ServerAggregateQueries.networkGeolocationCounts()); + + Map expected = new HashMap<>(); + // first user has a more recent connection from Finland so their country should be counted as Finland. + expected.put("Finland", 1); + expected.put("Sweden", 1); + expected.put("Denmark", 1); + + assertEquals(expected, got); + } } diff --git a/Plan/common/src/test/java/com/djrapitops/plan/db/SQLiteTest.java b/Plan/common/src/test/java/com/djrapitops/plan/db/SQLiteTest.java index 69b17d719..6d5211322 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/db/SQLiteTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/db/SQLiteTest.java @@ -17,15 +17,19 @@ package com.djrapitops.plan.db; import com.djrapitops.plan.api.exceptions.database.DBInitException; +import com.djrapitops.plan.data.container.GeoInfo; import com.djrapitops.plan.db.access.queries.LargeFetchQueries; import com.djrapitops.plan.db.access.queries.OptionalFetchQueries; +import com.djrapitops.plan.db.access.queries.ServerAggregateQueries; import com.djrapitops.plan.db.sql.tables.ServerTable; +import com.djrapitops.plan.db.sql.tables.UsersTable; import com.djrapitops.plan.system.info.server.Server; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; +import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.UUID; @@ -86,4 +90,31 @@ public class SQLiteTest extends CommonDBTest { assertEquals(1, serverInformation.values().stream().filter(Server::isNotProxy).count()); assertEquals(1, serverInformation.values().stream().filter(Server::isProxy).count()); } + + @Test + public void networkGeolocationsAreCountedAppropriately() { + UUID firstUuid = UUID.randomUUID(); + UUID secondUuid = UUID.randomUUID(); + UUID thirdUuid = UUID.randomUUID(); + + UsersTable usersTable = db.getUsersTable(); + usersTable.registerUser(firstUuid, 0, ""); + usersTable.registerUser(secondUuid, 0, ""); + usersTable.registerUser(thirdUuid, 0, ""); + + saveGeoInfo(firstUuid, new GeoInfo("-", "Norway", 0, "3")); + saveGeoInfo(firstUuid, new GeoInfo("-", "Finland", 5, "3")); + saveGeoInfo(secondUuid, new GeoInfo("-", "Sweden", 0, "3")); + saveGeoInfo(thirdUuid, new GeoInfo("-", "Denmark", 0, "3")); + + Map got = db.query(ServerAggregateQueries.networkGeolocationCounts()); + + Map expected = new HashMap<>(); + // first user has a more recent connection from Finland so their country should be counted as Finland. + expected.put("Finland", 1); + expected.put("Sweden", 1); + expected.put("Denmark", 1); + + assertEquals(expected, got); + } }