From f26294d9dd4beec252b5afb3f942883b2bdbc852 Mon Sep 17 00:00:00 2001 From: Rsl1122 Date: Tue, 18 Sep 2018 18:18:52 +0300 Subject: [PATCH] [Debt] Turned GeolocationCache methods non-static getInstance is not necessary as GeolocationCache can be injected everywhere where it is required (Mostly at gathering layer) Discovered old GeolocationCacheTest that was removed. --- .../plan/system/cache/GeolocationCache.java | 34 +++---- .../system/importing/BukkitImportSystem.java | 6 +- .../system/importing/importers/Importer.java | 11 ++- .../importers/OfflinePlayerImporter.java | 4 +- .../processors/player/IPUpdateProcessor.java | 7 +- .../processors/player/PlayerProcessors.java | 8 +- .../plan/data/cache/GeolocationCacheTest.java | 80 --------------- .../system/cache/GeolocationCacheTest.java | 99 +++++++++++++------ 8 files changed, 110 insertions(+), 139 deletions(-) delete mode 100644 Plan/src/test/java/com/djrapitops/plan/data/cache/GeolocationCacheTest.java diff --git a/Plan/src/main/java/com/djrapitops/plan/system/cache/GeolocationCache.java b/Plan/src/main/java/com/djrapitops/plan/system/cache/GeolocationCache.java index 9f4a69313..0aeb066d6 100644 --- a/Plan/src/main/java/com/djrapitops/plan/system/cache/GeolocationCache.java +++ b/Plan/src/main/java/com/djrapitops/plan/system/cache/GeolocationCache.java @@ -9,7 +9,6 @@ import com.djrapitops.plan.system.settings.Settings; import com.djrapitops.plan.system.settings.config.PlanConfig; import com.djrapitops.plugin.logging.L; import com.djrapitops.plugin.logging.console.PluginLogger; -import com.djrapitops.plugin.utilities.Verify; import com.maxmind.geoip2.DatabaseReader; import com.maxmind.geoip2.exception.GeoIp2Exception; import com.maxmind.geoip2.model.CountryResponse; @@ -59,7 +58,8 @@ public class GeolocationCache implements SubSystem { this.fileSystem = fileSystem; this.config = config; this.logger = logger; - cached = new HashMap<>(); + + this.cached = new HashMap<>(); } @Override @@ -67,7 +67,7 @@ public class GeolocationCache implements SubSystem { geolocationDB = fileSystem.getFileFromPluginFolder("GeoIP.dat"); if (config.isTrue(Settings.DATA_GEOLOCATIONS)) { try { - GeolocationCache.checkDB(); + checkDB(); } catch (UnknownHostException e) { logger.error(locale.getString(PluginLang.ENABLE_NOTIFY_GEOLOCATIONS_INTERNET_REQUIRED)); } catch (IOException e) { @@ -90,33 +90,27 @@ public class GeolocationCache implements SubSystem { * if that happens, "Not Known" will be returned. * @see #getUnCachedCountry(String) */ - public static String getCountry(String ipAddress) { + public String getCountry(String ipAddress) { String country = getCachedCountry(ipAddress); if (country != null) { return country; } else { country = getUnCachedCountry(ipAddress); - getInstance().cached.put(ipAddress, country); + cached.put(ipAddress, country); return country; } } - private static GeolocationCache getInstance() { - GeolocationCache geolocationCache = CacheSystem.getInstance().getGeolocationCache(); - Verify.nullCheck(geolocationCache, () -> new IllegalStateException("GeolocationCache was not initialized.")); - return geolocationCache; - } - /** * Returns the cached country * * @param ipAddress The IP Address which is retrieved out of the cache * @return The cached country, {@code null} if the country is not cached */ - private static String getCachedCountry(String ipAddress) { - return getInstance().cached.get(ipAddress); + private String getCachedCountry(String ipAddress) { + return cached.get(ipAddress); } /** @@ -133,14 +127,14 @@ public class GeolocationCache implements SubSystem { * @see http://maxmind.com * @see #getCountry(String) */ - private static String getUnCachedCountry(String ipAddress) { + private String getUnCachedCountry(String ipAddress) { if ("127.0.0.1".equals(ipAddress)) { return "Local Machine"; } try { checkDB(); - try (DatabaseReader reader = new DatabaseReader.Builder(getInstance().geolocationDB).build()) { + try (DatabaseReader reader = new DatabaseReader.Builder(geolocationDB).build()) { InetAddress inetAddress = InetAddress.getByName(ipAddress); CountryResponse response = reader.country(inetAddress); @@ -159,13 +153,13 @@ public class GeolocationCache implements SubSystem { * * @throws IOException when an error at download or saving the DB happens */ - public static void checkDB() throws IOException { - if (getInstance().geolocationDB.exists()) { + private void checkDB() throws IOException { + if (geolocationDB.exists()) { return; } URL downloadSite = new URL("http://geolite.maxmind.com/download/geoip/database/GeoLite2-Country.mmdb.gz"); try (ReadableByteChannel rbc = Channels.newChannel(new GZIPInputStream(downloadSite.openStream())); - FileOutputStream fos = new FileOutputStream(getInstance().geolocationDB.getAbsoluteFile())) { + FileOutputStream fos = new FileOutputStream(geolocationDB.getAbsoluteFile())) { fos.getChannel().transferFrom(rbc, 0, Long.MAX_VALUE); } } @@ -176,8 +170,8 @@ public class GeolocationCache implements SubSystem { * @param ipAddress The IP Address which is checked * @return true if the IP Address is cached */ - public static boolean isCached(String ipAddress) { - return getInstance().cached.containsKey(ipAddress); + boolean isCached(String ipAddress) { + return cached.containsKey(ipAddress); } @Override diff --git a/Plan/src/main/java/com/djrapitops/plan/system/importing/BukkitImportSystem.java b/Plan/src/main/java/com/djrapitops/plan/system/importing/BukkitImportSystem.java index 767748d91..62917650b 100644 --- a/Plan/src/main/java/com/djrapitops/plan/system/importing/BukkitImportSystem.java +++ b/Plan/src/main/java/com/djrapitops/plan/system/importing/BukkitImportSystem.java @@ -1,5 +1,6 @@ package com.djrapitops.plan.system.importing; +import com.djrapitops.plan.system.cache.GeolocationCache; import com.djrapitops.plan.system.database.databases.Database; import com.djrapitops.plan.system.importing.importers.OfflinePlayerImporter; import com.djrapitops.plan.system.info.server.ServerInfo; @@ -15,20 +16,23 @@ import javax.inject.Singleton; @Singleton public class BukkitImportSystem extends ImportSystem { + private final GeolocationCache geolocationCache; private final Database database; private final ServerInfo serverInfo; @Inject public BukkitImportSystem( + GeolocationCache geolocationCache, Database database, ServerInfo serverInfo ) { + this.geolocationCache = geolocationCache; this.database = database; this.serverInfo = serverInfo; } @Override void registerImporters() { - registerImporter(new OfflinePlayerImporter(database, serverInfo)); + registerImporter(new OfflinePlayerImporter(geolocationCache, database, serverInfo)); } } \ No newline at end of file diff --git a/Plan/src/main/java/com/djrapitops/plan/system/importing/importers/Importer.java b/Plan/src/main/java/com/djrapitops/plan/system/importing/importers/Importer.java index d542310b7..9dbbc4ba4 100644 --- a/Plan/src/main/java/com/djrapitops/plan/system/importing/importers/Importer.java +++ b/Plan/src/main/java/com/djrapitops/plan/system/importing/importers/Importer.java @@ -36,12 +36,19 @@ import java.util.stream.Collectors; */ public abstract class Importer { + private final GeolocationCache geolocationCache; private final Database database; private final UUID serverUUID; private final String name; - protected Importer(Database database, ServerInfo serverInfo, String name) { + protected Importer( + GeolocationCache geolocationCache, + Database database, + ServerInfo serverInfo, + String name + ) { + this.geolocationCache = geolocationCache; this.database = database; this.serverUUID = serverInfo.getServerUUID(); @@ -188,7 +195,7 @@ public abstract class Importer { return userImportData.getIps().parallelStream() .map(ip -> { - String geoLoc = GeolocationCache.getCountry(ip); + String geoLoc = geolocationCache.getCountry(ip); try { return new GeoInfo(ip, geoLoc, date, new SHA256Hash(ip).create()); } catch (NoSuchAlgorithmException e) { diff --git a/Plan/src/main/java/com/djrapitops/plan/system/importing/importers/OfflinePlayerImporter.java b/Plan/src/main/java/com/djrapitops/plan/system/importing/importers/OfflinePlayerImporter.java index aa23fb80a..e798eb28e 100644 --- a/Plan/src/main/java/com/djrapitops/plan/system/importing/importers/OfflinePlayerImporter.java +++ b/Plan/src/main/java/com/djrapitops/plan/system/importing/importers/OfflinePlayerImporter.java @@ -4,6 +4,7 @@ */ package com.djrapitops.plan.system.importing.importers; +import com.djrapitops.plan.system.cache.GeolocationCache; import com.djrapitops.plan.system.database.databases.Database; import com.djrapitops.plan.system.importing.data.ServerImportData; import com.djrapitops.plan.system.importing.data.UserImportData; @@ -27,10 +28,11 @@ public class OfflinePlayerImporter extends Importer { @Inject public OfflinePlayerImporter( + GeolocationCache geolocationCache, Database database, ServerInfo serverInfo ) { - super(database, serverInfo, "offline"); + super(geolocationCache, database, serverInfo, "offline"); } @Override diff --git a/Plan/src/main/java/com/djrapitops/plan/system/processing/processors/player/IPUpdateProcessor.java b/Plan/src/main/java/com/djrapitops/plan/system/processing/processors/player/IPUpdateProcessor.java index 6ed837b63..525679f25 100644 --- a/Plan/src/main/java/com/djrapitops/plan/system/processing/processors/player/IPUpdateProcessor.java +++ b/Plan/src/main/java/com/djrapitops/plan/system/processing/processors/player/IPUpdateProcessor.java @@ -25,21 +25,24 @@ public class IPUpdateProcessor implements CriticalRunnable { private final long time; private final Database database; + private final GeolocationCache geolocationCache; IPUpdateProcessor( UUID uuid, InetAddress ip, long time, - Database database + Database database, + GeolocationCache geolocationCache ) { this.uuid = uuid; this.ip = ip; this.time = time; this.database = database; + this.geolocationCache = geolocationCache; } @Override public void run() { try { - String country = GeolocationCache.getCountry(ip.getHostAddress()); + String country = geolocationCache.getCountry(ip.getHostAddress()); GeoInfo geoInfo = new GeoInfo(ip, country, time); database.save().geoInfo(uuid, geoInfo); } catch (NoSuchAlgorithmException ignore) { diff --git a/Plan/src/main/java/com/djrapitops/plan/system/processing/processors/player/PlayerProcessors.java b/Plan/src/main/java/com/djrapitops/plan/system/processing/processors/player/PlayerProcessors.java index 1d5da45a1..57c91db5d 100644 --- a/Plan/src/main/java/com/djrapitops/plan/system/processing/processors/player/PlayerProcessors.java +++ b/Plan/src/main/java/com/djrapitops/plan/system/processing/processors/player/PlayerProcessors.java @@ -2,6 +2,7 @@ package com.djrapitops.plan.system.processing.processors.player; import com.djrapitops.plan.data.store.objects.DateObj; import com.djrapitops.plan.system.cache.DataCache; +import com.djrapitops.plan.system.cache.GeolocationCache; import com.djrapitops.plan.system.database.databases.Database; import com.djrapitops.plan.system.processing.Processing; import dagger.Lazy; @@ -24,16 +25,19 @@ public class PlayerProcessors { private final Lazy processing; private final Lazy database; private final Lazy dataCache; + private final Lazy geolocationCache; @Inject public PlayerProcessors( Lazy processing, Lazy database, - Lazy dataCache + Lazy dataCache, + Lazy geolocationCache ) { this.processing = processing; this.database = database; this.dataCache = dataCache; + this.geolocationCache = geolocationCache; } public BanAndOpProcessor banAndOpProcessor(UUID uuid, Supplier banned, boolean op) { @@ -49,7 +53,7 @@ public class PlayerProcessors { } public IPUpdateProcessor ipUpdateProcessor(UUID uuid, InetAddress ip, long time) { - return new IPUpdateProcessor(uuid, ip, time, database.get()); + return new IPUpdateProcessor(uuid, ip, time, database.get(), geolocationCache.get()); } public KickProcessor kickProcessor(UUID uuid) { diff --git a/Plan/src/test/java/com/djrapitops/plan/data/cache/GeolocationCacheTest.java b/Plan/src/test/java/com/djrapitops/plan/data/cache/GeolocationCacheTest.java deleted file mode 100644 index 9e7771bad..000000000 --- a/Plan/src/test/java/com/djrapitops/plan/data/cache/GeolocationCacheTest.java +++ /dev/null @@ -1,80 +0,0 @@ -package com.djrapitops.plan.data.cache; - -import com.djrapitops.plan.system.cache.CacheSystem; -import com.djrapitops.plan.system.cache.GeolocationCache; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; -import utilities.mocks.SystemMockUtil; - -import java.util.HashMap; -import java.util.Map; - -import static junit.framework.TestCase.*; - -/** - * @author Fuzzlemann - */ -@RunWith(MockitoJUnitRunner.Silent.class) -public class GeolocationCacheTest { - - private final Map ipsToCountries = new HashMap<>(); - - @ClassRule - public static TemporaryFolder temporaryFolder = new TemporaryFolder(); - - @BeforeClass - public static void setUpClass() throws Exception { - SystemMockUtil.setUp(temporaryFolder.getRoot()) - .enableConfigSystem() - .enableCacheSystem(); - } - - @Before - public void setUp() { - CacheSystem.getInstance().getGeolocationCache().clearCache(); - - ipsToCountries.put("8.8.8.8", "United States"); - ipsToCountries.put("8.8.4.4", "United States"); - ipsToCountries.put("4.4.2.2", "United States"); - ipsToCountries.put("208.67.222.222", "United States"); - ipsToCountries.put("208.67.220.220", "United States"); - ipsToCountries.put("205.210.42.205", "Canada"); - ipsToCountries.put("64.68.200.200", "Canada"); - ipsToCountries.put("0.0.0.0", "Not Known"); - ipsToCountries.put("127.0.0.1", "Local Machine"); - } - - @Test - public void testCountryGetting() { - for (Map.Entry entry : ipsToCountries.entrySet()) { - String ip = entry.getKey(); - String expCountry = entry.getValue(); - - String country = GeolocationCache.getCountry(ip); - - assertEquals(country, expCountry); - } - } - - @Test - public void testCaching() { - for (Map.Entry entry : ipsToCountries.entrySet()) { - String ip = entry.getKey(); - String expIp = entry.getValue(); - - assertFalse(GeolocationCache.isCached(ip)); - String countrySecondCall = GeolocationCache.getCountry(ip); - assertTrue(GeolocationCache.isCached(ip)); - - String countryThirdCall = GeolocationCache.getCountry(ip); - - assertEquals(countrySecondCall, countryThirdCall); - assertEquals(countryThirdCall, expIp); - } - } -} diff --git a/Plan/src/test/java/com/djrapitops/plan/system/cache/GeolocationCacheTest.java b/Plan/src/test/java/com/djrapitops/plan/system/cache/GeolocationCacheTest.java index 29a9dd676..8189fa9eb 100644 --- a/Plan/src/test/java/com/djrapitops/plan/system/cache/GeolocationCacheTest.java +++ b/Plan/src/test/java/com/djrapitops/plan/system/cache/GeolocationCacheTest.java @@ -1,55 +1,92 @@ package com.djrapitops.plan.system.cache; -import com.djrapitops.plan.Plan; -import com.djrapitops.plan.api.exceptions.EnableException; -import com.djrapitops.plan.system.PlanSystem; -import com.djrapitops.plugin.StaticHolder; +import com.djrapitops.plan.system.file.FileSystem; +import com.djrapitops.plan.system.locale.Locale; +import com.djrapitops.plan.system.settings.config.PlanConfig; +import com.djrapitops.plugin.logging.console.TestPluginLogger; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.ClassRule; -import org.junit.Ignore; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import utilities.mocks.BukkitMockUtil; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import utilities.mocks.SystemMockUtil; -import static org.junit.Assert.assertEquals; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import static junit.framework.TestCase.*; +import static org.mockito.Mockito.doReturn; /** - * Test for GeolocationCache. - * - * @author Rsl1122 + * @author Fuzzlemann */ +@RunWith(MockitoJUnitRunner.Silent.class) public class GeolocationCacheTest { + private final Map ipsToCountries = new HashMap<>(); + + @Mock + private FileSystem fileSystem; + @Mock + private PlanConfig config; + private GeolocationCache geolocationCache; + @ClassRule public static TemporaryFolder temporaryFolder = new TemporaryFolder(); - private static Plan planMock; @BeforeClass public static void setUpClass() throws Exception { - BukkitMockUtil mockUtil = BukkitMockUtil.setUp() - .withDataFolder(temporaryFolder.getRoot()) - .withLogging() - .withPluginDescription() - .withResourceFetchingFromJar() - .withServer(); - planMock = mockUtil.getPlanMock(); - StaticHolder.saveInstance(GeolocationCacheTest.class, planMock.getClass()); + SystemMockUtil.setUp(temporaryFolder.getRoot()) + .enableConfigSystem() + .enableCacheSystem(); + } + + @Before + public void setUp() throws IOException { + doReturn(temporaryFolder.newFile("GeoIP.dat")).when(fileSystem.getFileFromPluginFolder("GeoIP.dat")); + geolocationCache = new GeolocationCache(new Locale(), fileSystem, config, new TestPluginLogger()); + + ipsToCountries.put("8.8.8.8", "United States"); + ipsToCountries.put("8.8.4.4", "United States"); + ipsToCountries.put("4.4.2.2", "United States"); + ipsToCountries.put("208.67.222.222", "United States"); + ipsToCountries.put("208.67.220.220", "United States"); + ipsToCountries.put("205.210.42.205", "Canada"); + ipsToCountries.put("64.68.200.200", "Canada"); + ipsToCountries.put("0.0.0.0", "Not Known"); + ipsToCountries.put("127.0.0.1", "Local Machine"); } @Test - @Ignore - public void testGeolocationCache() throws EnableException { -// Settings.WEBSERVER_PORT.setTemporaryValue(9005); - PlanSystem system = null; //TODO - try { - system.enable(); + public void testCountryGetting() { + for (Map.Entry entry : ipsToCountries.entrySet()) { + String ip = entry.getKey(); + String expCountry = entry.getValue(); - String expected = "Germany"; - String result = GeolocationCache.getCountry("141.52.255.1"); - assertEquals(expected, result); - } finally { - system.disable(); + String country = geolocationCache.getCountry(ip); + + assertEquals(country, expCountry); } } -} \ No newline at end of file + @Test + public void testCaching() { + for (Map.Entry entry : ipsToCountries.entrySet()) { + String ip = entry.getKey(); + String expIp = entry.getValue(); + + assertFalse(geolocationCache.isCached(ip)); + String countrySecondCall = geolocationCache.getCountry(ip); + assertTrue(geolocationCache.isCached(ip)); + + String countryThirdCall = geolocationCache.getCountry(ip); + + assertEquals(countrySecondCall, countryThirdCall); + assertEquals(countryThirdCall, expIp); + } + } +}