diff --git a/src/main/java/org/graylog/plugins/map/geoip/GeoIpResolverEngine.java b/src/main/java/org/graylog/plugins/map/geoip/GeoIpResolverEngine.java index 1a0e0d30433c..81525c86eedd 100644 --- a/src/main/java/org/graylog/plugins/map/geoip/GeoIpResolverEngine.java +++ b/src/main/java/org/graylog/plugins/map/geoip/GeoIpResolverEngine.java @@ -18,7 +18,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; -import com.google.common.collect.Lists; +import com.google.auto.value.AutoValue; import com.google.common.net.InetAddresses; import com.maxmind.geoip2.DatabaseReader; import com.maxmind.geoip2.model.CityResponse; @@ -32,8 +32,8 @@ import java.io.IOException; import java.net.InetAddress; import java.nio.file.Files; -import java.util.List; import java.util.Map; +import java.util.Optional; import static com.codahale.metrics.MetricRegistry.name; import static com.google.common.base.Strings.isNullOrEmpty; @@ -71,42 +71,35 @@ public boolean filter(Message message) { for (Map.Entry field : message.getFields().entrySet()) { String key = field.getKey() + "_geolocation"; - final List coordinates = extractGeoLocationInformation(field.getValue()); - if (coordinates.size() == 2) { - // We will store the coordinates as a "lat,long" string - final String stringGeoPoint = coordinates.get(1) + "," + coordinates.get(0); - message.addField(key, stringGeoPoint); - } + final Optional coordinates = extractGeoLocationInformation(field.getValue()); + // We will store the coordinates as a "lat,long" string + coordinates.ifPresent(c -> message.addField(key, c.latitude() + "" + c.longitude())); } return false; } - protected List extractGeoLocationInformation(Object fieldValue) { - final List coordinates = Lists.newArrayList(); - + protected Optional extractGeoLocationInformation(Object fieldValue) { if (!(fieldValue instanceof String) || isNullOrEmpty((String) fieldValue)) { - return coordinates; + return Optional.empty(); } final String stringFieldValue = (String) fieldValue; final InetAddress ipAddress = this.getIpFromFieldValue(stringFieldValue); if (ipAddress == null) { - return coordinates; + return Optional.empty(); } try { try (Timer.Context ignored = resolveTime.time()) { final CityResponse response = databaseReader.city(ipAddress); final Location location = response.getLocation(); - coordinates.add(location.getLongitude()); - coordinates.add(location.getLatitude()); + return Optional.of(Coordinates.create(location.getLatitude(), location.getLongitude())); } } catch (Exception e) { LOG.debug("Could not get location from IP {}", ipAddress.getHostAddress(), e); + return Optional.empty(); } - - return coordinates; } protected InetAddress getIpFromFieldValue(String fieldValue) { @@ -118,4 +111,15 @@ protected InetAddress getIpFromFieldValue(String fieldValue) { return null; } + + @AutoValue + static abstract class Coordinates { + public abstract double latitude(); + + public abstract double longitude(); + + public static Coordinates create(double latitude, double longitude) { + return new AutoValue_GeoIpResolverEngine_Coordinates(latitude, longitude); + } + } } diff --git a/src/test/java/org/graylog/plugins/map/geoip/GeoIpResolverEngineTest.java b/src/test/java/org/graylog/plugins/map/geoip/GeoIpResolverEngineTest.java index 11763fbecba9..34e704936044 100644 --- a/src/test/java/org/graylog/plugins/map/geoip/GeoIpResolverEngineTest.java +++ b/src/test/java/org/graylog/plugins/map/geoip/GeoIpResolverEngineTest.java @@ -30,12 +30,14 @@ import java.net.URISyntaxException; import java.util.List; import java.util.Map; +import java.util.Optional; import static com.codahale.metrics.MetricRegistry.name; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; public class GeoIpResolverEngineTest { @@ -88,10 +90,10 @@ public void trimFieldValueBeforeLookup() throws Exception { public void extractGeoLocationInformation() throws Exception { final GeoIpResolverEngine resolver = new GeoIpResolverEngine(config, metricRegistry); - List coordinates = resolver.extractGeoLocationInformation("1.2.3.4"); - assertEquals(coordinates.size(), 2, "Should extract geo location information from public addresses"); - List coordinates2 = resolver.extractGeoLocationInformation("192.168.0.1"); - assertEquals(coordinates2.size(), 0, "Should not extract geo location information from private addresses"); + Optional coordinates = resolver.extractGeoLocationInformation("1.2.3.4"); + assertTrue(coordinates.isPresent(), "Should extract geo location information from public addresses"); + Optional coordinates2 = resolver.extractGeoLocationInformation("192.168.0.1"); + assertFalse(coordinates2.isPresent(), "Should not extract geo location information from private addresses"); } @Test