From 1361174892dd6df0f71be24d48a6551c97d42b14 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 26 Jun 2016 23:09:27 +0200 Subject: [PATCH] Encapsulate GeoLiteAPI class --- src/main/java/fr/xephi/authme/AuthMe.java | 7 +- .../fr/xephi/authme/settings/SpawnLoader.java | 4 +- .../java/fr/xephi/authme/util/GeoLiteAPI.java | 51 ++++++--- .../xephi/authme/util/ValidationService.java | 24 ++-- .../fr/xephi/authme/util/GeoLiteAPITest.java | 104 ++++++++++++++++++ .../authme/util/ValidationServiceTest.java | 13 ++- 6 files changed, 163 insertions(+), 40 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/util/GeoLiteAPITest.java diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 5f4935169..bd9b58d44 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -115,6 +115,7 @@ public class AuthMe extends JavaPlugin { private SpawnLoader spawnLoader; private BukkitService bukkitService; private AuthMeServiceInitializer initializer; + private GeoLiteAPI geoLiteApi; /* * Private instances (mail and ProtocolLib) @@ -256,9 +257,6 @@ public class AuthMe extends JavaPlugin { // Set console filter setupConsoleFilter(); - // Download and load GeoIp.dat file if absent - GeoLiteAPI.isDataAvailable(); - // Set up the mail API setupMailApi(); @@ -313,6 +311,7 @@ public class AuthMe extends JavaPlugin { commandHandler = initializer.get(CommandHandler.class); api = initializer.get(NewAPI.class); management = initializer.get(Management.class); + geoLiteApi = initializer.get(GeoLiteAPI.class); initializer.get(API.class); } @@ -678,7 +677,7 @@ public class AuthMe extends JavaPlugin { .replace("{WORLD}", player.getWorld().getName()) .replace("{SERVER}", server.getServerName()) .replace("{VERSION}", server.getBukkitVersion()) - .replace("{COUNTRY}", GeoLiteAPI.getCountryName(ipAddress)); + .replace("{COUNTRY}", geoLiteApi.getCountryName(ipAddress)); } diff --git a/src/main/java/fr/xephi/authme/settings/SpawnLoader.java b/src/main/java/fr/xephi/authme/settings/SpawnLoader.java index ecb1d379d..684e74aac 100644 --- a/src/main/java/fr/xephi/authme/settings/SpawnLoader.java +++ b/src/main/java/fr/xephi/authme/settings/SpawnLoader.java @@ -1,6 +1,5 @@ package fr.xephi.authme.settings; -import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; @@ -145,8 +144,7 @@ public class SpawnLoader implements Reloadable { * @see RestrictionSettings#SPAWN_PRIORITY */ public Location getSpawnLocation(Player player) { - AuthMe plugin = AuthMe.getInstance(); - if (plugin == null || player == null || player.getWorld() == null) { + if (player == null || player.getWorld() == null) { return null; } diff --git a/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java b/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java index 49a57cf3c..ec66d5082 100644 --- a/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java +++ b/src/main/java/fr/xephi/authme/util/GeoLiteAPI.java @@ -1,9 +1,11 @@ package fr.xephi.authme.util; +import com.google.common.annotations.VisibleForTesting; import com.maxmind.geoip.LookupService; -import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.initialization.DataFolder; +import javax.inject.Inject; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; @@ -19,10 +21,22 @@ public class GeoLiteAPI { "[LICENSE] This product uses data from the GeoLite API created by MaxMind, available at http://www.maxmind.com"; private static final String GEOIP_URL = "http://geolite.maxmind.com/download/geoip/database/GeoLiteCountry/GeoIP.dat.gz"; - private static LookupService lookupService; - private static Thread downloadTask; + private LookupService lookupService; + private Thread downloadTask; - private GeoLiteAPI() { + private final File dataFile; + + @Inject + GeoLiteAPI(@DataFolder File dataFolder) { + this.dataFile = new File(dataFolder, "GeoIP.dat"); + // Fires download of recent data or the initialization of the look up service + isDataAvailable(); + } + + @VisibleForTesting + GeoLiteAPI(@DataFolder File dataFolder, LookupService lookupService) { + this.dataFile = dataFolder; + this.lookupService = lookupService; } /** @@ -30,20 +44,19 @@ public class GeoLiteAPI { * * @return True if the data is available, false otherwise. */ - public synchronized static boolean isDataAvailable() { + private synchronized boolean isDataAvailable() { if (downloadTask != null && downloadTask.isAlive()) { return false; } if (lookupService != null) { return true; } - final File pluginFolder = AuthMe.getInstance().getDataFolder(); - final File data = new File(pluginFolder, "GeoIP.dat"); - if (data.exists()) { - boolean dataIsOld = (System.currentTimeMillis() - data.lastModified()) > TimeUnit.DAYS.toMillis(30); + + if (dataFile.exists()) { + boolean dataIsOld = (System.currentTimeMillis() - dataFile.lastModified()) > TimeUnit.DAYS.toMillis(30); if (!dataIsOld) { try { - lookupService = new LookupService(data); + lookupService = new LookupService(dataFile); ConsoleLogger.info(LICENSE); return true; } catch (IOException e) { @@ -51,13 +64,19 @@ public class GeoLiteAPI { return false; } } else { - if (!data.delete()) { + if (!dataFile.delete()) { ConsoleLogger.showError("Failed to delete GeoLiteAPI database"); } } } // Ok, let's try to download the data file! - downloadTask = new Thread(new Runnable() { + downloadTask = createDownloadTask(); + downloadTask.start(); + return false; + } + + private Thread createDownloadTask() { + return new Thread(new Runnable() { @Override public void run() { try { @@ -69,7 +88,7 @@ public class GeoLiteAPI { if (conn.getURL().toString().endsWith(".gz")) { input = new GZIPInputStream(input); } - OutputStream output = new FileOutputStream(data); + OutputStream output = new FileOutputStream(dataFile); byte[] buffer = new byte[2048]; int length = input.read(buffer); while (length >= 0) { @@ -83,8 +102,6 @@ public class GeoLiteAPI { } } }); - downloadTask.start(); - return false; } /** @@ -94,7 +111,7 @@ public class GeoLiteAPI { * * @return two-character ISO 3166-1 alpha code for the country. */ - public static String getCountryCode(String ip) { + public String getCountryCode(String ip) { if (!"127.0.0.1".equals(ip) && isDataAvailable()) { return lookupService.getCountry(ip).getCode(); } @@ -108,7 +125,7 @@ public class GeoLiteAPI { * * @return The name of the country. */ - public static String getCountryName(String ip) { + public String getCountryName(String ip) { if (!"127.0.0.1".equals(ip) && isDataAvailable()) { return lookupService.getCountry(ip).getName(); } diff --git a/src/main/java/fr/xephi/authme/util/ValidationService.java b/src/main/java/fr/xephi/authme/util/ValidationService.java index 615095d00..f2f1b0e56 100644 --- a/src/main/java/fr/xephi/authme/util/ValidationService.java +++ b/src/main/java/fr/xephi/authme/util/ValidationService.java @@ -13,6 +13,7 @@ import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.command.CommandSender; +import javax.annotation.PostConstruct; import javax.inject.Inject; import java.util.Collection; import java.util.List; @@ -23,19 +24,20 @@ import java.util.regex.Pattern; */ public class ValidationService implements Reloadable { - private final NewSetting settings; - private final DataSource dataSource; - private final PermissionsManager permissionsManager; + @Inject + private NewSetting settings; + @Inject + private DataSource dataSource; + @Inject + private PermissionsManager permissionsManager; + @Inject + private GeoLiteAPI geoLiteApi; + private Pattern passwordRegex; - @Inject - public ValidationService(NewSetting settings, DataSource dataSource, PermissionsManager permissionsManager) { - this.settings = settings; - this.dataSource = dataSource; - this.permissionsManager = permissionsManager; - reload(); - } + ValidationService() { } + @PostConstruct @Override public void reload() { passwordRegex = Utils.safePatternCompile(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX)); @@ -105,7 +107,7 @@ public class ValidationService implements Reloadable { return true; } - String countryCode = GeoLiteAPI.getCountryCode(hostAddress); + String countryCode = geoLiteApi.getCountryCode(hostAddress); return validateWhitelistAndBlacklist(countryCode, ProtectionSettings.COUNTRIES_WHITELIST, ProtectionSettings.COUNTRIES_BLACKLIST); diff --git a/src/test/java/fr/xephi/authme/util/GeoLiteAPITest.java b/src/test/java/fr/xephi/authme/util/GeoLiteAPITest.java new file mode 100644 index 000000000..c932ddbb8 --- /dev/null +++ b/src/test/java/fr/xephi/authme/util/GeoLiteAPITest.java @@ -0,0 +1,104 @@ +package fr.xephi.authme.util; + +import com.maxmind.geoip.Country; +import com.maxmind.geoip.LookupService; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import java.io.File; +import java.io.IOException; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +/** + * Test for {@link GeoLiteAPI}. + */ +@RunWith(MockitoJUnitRunner.class) +public class GeoLiteAPITest { + + private GeoLiteAPI geoLiteApi; + private File dataFolder; + @Mock + private LookupService lookupService; + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Before + public void initializeGeoLiteApi() throws IOException { + dataFolder = temporaryFolder.newFolder(); + geoLiteApi = new GeoLiteAPI(dataFolder, lookupService); + } + + @Test + public void shouldGetCountry() { + // given + String ip = "123.45.67.89"; + String countryCode = "XX"; + Country country = mock(Country.class); + given(country.getCode()).willReturn(countryCode); + given(lookupService.getCountry(ip)).willReturn(country); + + // when + String result = geoLiteApi.getCountryCode(ip); + + // then + assertThat(result, equalTo(countryCode)); + verify(lookupService).getCountry(ip); + } + + @Test + public void shouldNotLookUpCountryForLocalhostIp() { + // given + String ip = "127.0.0.1"; + + // when + String result = geoLiteApi.getCountryCode(ip); + + // then + assertThat(result, equalTo("--")); + verify(lookupService, never()).getCountry(anyString()); + } + + @Test + public void shouldLookUpCountryName() { + // given + String ip = "24.45.167.89"; + String countryName = "Ecuador"; + Country country = mock(Country.class); + given(country.getName()).willReturn(countryName); + given(lookupService.getCountry(ip)).willReturn(country); + + // when + String result = geoLiteApi.getCountryName(ip); + + // then + assertThat(result, equalTo(countryName)); + verify(lookupService).getCountry(ip); + } + + @Test + public void shouldNotLookUpCountryNameForLocalhostIp() { + // given + String ip = "127.0.0.1"; + + // when + String result = geoLiteApi.getCountryName(ip); + + // then + assertThat(result, equalTo("N/A")); + verify(lookupService, never()).getCountry(ip); + } + +} diff --git a/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java b/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java index 0d52f7758..ddfc9c161 100644 --- a/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java +++ b/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java @@ -5,17 +5,18 @@ import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PlayerStatePermission; +import fr.xephi.authme.runner.BeforeInjecting; +import fr.xephi.authme.runner.DelayedInjectionRunner; +import fr.xephi.authme.runner.InjectDelayed; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.EmailSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.ValidationService.ValidationResult; import org.bukkit.command.CommandSender; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; import java.util.Arrays; import java.util.Collections; @@ -28,9 +29,10 @@ import static org.mockito.Mockito.mock; /** * Test for {@link ValidationService}. */ -@RunWith(MockitoJUnitRunner.class) +@RunWith(DelayedInjectionRunner.class) public class ValidationServiceTest { + @InjectDelayed private ValidationService validationService; @Mock private NewSetting settings; @@ -38,8 +40,10 @@ public class ValidationServiceTest { private DataSource dataSource; @Mock private PermissionsManager permissionsManager; + @Mock + private GeoLiteAPI geoLiteApi; - @Before + @BeforeInjecting public void createService() { given(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX)).willReturn("[a-zA-Z]+"); given(settings.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).willReturn(3); @@ -47,7 +51,6 @@ public class ValidationServiceTest { given(settings.getProperty(SecuritySettings.UNSAFE_PASSWORDS)) .willReturn(Arrays.asList("unsafe", "other-unsafe")); given(settings.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(3); - validationService = new ValidationService(settings, dataSource, permissionsManager); } @Test