Repository: nifi Updated Branches: refs/heads/master 54402a1ec -> 7bcf9fcb5
NIFI-5814: Addressed issue in DatabaseReader class that was attempting to set values on the JSON returned by MaxMind. Instead of modifying the object directly, we should use an Injectable in the Reader so that the value read will have the appropriate values but we don't need to modify those objects returned by MaxMind Signed-off-by: Pierre Villard <pierre.villard...@gmail.com> This closes #3168. Project: http://git-wip-us.apache.org/repos/asf/nifi/repo Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/7bcf9fcb Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/7bcf9fcb Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/7bcf9fcb Branch: refs/heads/master Commit: 7bcf9fcb5da4f9b71a65314dd369d786ad032e69 Parents: 54402a1 Author: Mark Payne <marka...@hotmail.com> Authored: Mon Nov 12 12:12:16 2018 -0500 Committer: Pierre Villard <pierre.villard...@gmail.com> Committed: Mon Nov 19 11:41:43 2018 +0100 ---------------------------------------------------------------------- .../org/apache/nifi/processors/GeoEnrichIP.java | 18 ++- .../org/apache/nifi/processors/ISPEnrichIP.java | 3 +- .../nifi/processors/maxmind/DatabaseReader.java | 121 +++++++++---------- .../apache/nifi/processors/TestGeoEnrichIP.java | 5 +- .../apache/nifi/processors/TestISPEnrichIP.java | 5 +- 5 files changed, 70 insertions(+), 82 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/nifi/blob/7bcf9fcb/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIP.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIP.java b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIP.java index 175d873..8e657ec 100644 --- a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIP.java +++ b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIP.java @@ -16,12 +16,8 @@ */ package org.apache.nifi.processors; -import java.io.IOException; -import java.net.InetAddress; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.TimeUnit; - +import com.maxmind.geoip2.model.CityResponse; +import com.maxmind.geoip2.record.Subdivision; import org.apache.commons.lang3.StringUtils; import org.apache.nifi.annotation.behavior.EventDriven; import org.apache.nifi.annotation.behavior.InputRequirement; @@ -39,9 +35,11 @@ import org.apache.nifi.processor.exception.ProcessException; import org.apache.nifi.processors.maxmind.DatabaseReader; import org.apache.nifi.util.StopWatch; -import com.maxmind.geoip2.exception.GeoIp2Exception; -import com.maxmind.geoip2.model.CityResponse; -import com.maxmind.geoip2.record.Subdivision; +import java.io.IOException; +import java.net.InetAddress; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; @EventDriven @SideEffectFree @@ -102,7 +100,7 @@ public class GeoEnrichIP extends AbstractEnrichIP { try { response = dbReader.city(inetAddress); stopWatch.stop(); - } catch (final IOException | GeoIp2Exception ex) { + } catch (final IOException ex) { // Note IOException is captured again as dbReader also makes InetAddress.getByName() calls. // Most name or IP resolutions failure should have been triggered in the try loop above but // environmental conditions may trigger errors during the second resolution as well. http://git-wip-us.apache.org/repos/asf/nifi/blob/7bcf9fcb/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/ISPEnrichIP.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/ISPEnrichIP.java b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/ISPEnrichIP.java index fc159c9..781fb71 100644 --- a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/ISPEnrichIP.java +++ b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/ISPEnrichIP.java @@ -16,7 +16,6 @@ */ package org.apache.nifi.processors; -import com.maxmind.geoip2.exception.GeoIp2Exception; import com.maxmind.geoip2.model.IspResponse; import org.apache.commons.lang3.StringUtils; import org.apache.nifi.annotation.behavior.EventDriven; @@ -94,7 +93,7 @@ public class ISPEnrichIP extends AbstractEnrichIP { try { response = dbReader.isp(inetAddress); stopWatch.stop(); - } catch (final IOException | GeoIp2Exception ex) { + } catch (final IOException ex) { // Note IOException is captured again as dbReader also makes InetAddress.getByName() calls. // Most name or IP resolutions failure should have been triggered in the try loop above but // environmental conditions may trigger errors during the second resolution as well. http://git-wip-us.apache.org/repos/asf/nifi/blob/7bcf9fcb/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/maxmind/DatabaseReader.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/maxmind/DatabaseReader.java b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/maxmind/DatabaseReader.java index fb84daf..c5ecb11 100644 --- a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/maxmind/DatabaseReader.java +++ b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/maxmind/DatabaseReader.java @@ -16,30 +16,32 @@ */ package org.apache.nifi.processors.maxmind; -import java.io.Closeable; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.net.InetAddress; -import java.util.Arrays; -import java.util.List; - +import com.fasterxml.jackson.databind.BeanProperty; +import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.InjectableValues; +import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import com.maxmind.db.Metadata; import com.maxmind.db.Reader; import com.maxmind.db.Reader.FileMode; import com.maxmind.geoip2.GeoIp2Provider; -import com.maxmind.geoip2.exception.AddressNotFoundException; -import com.maxmind.geoip2.exception.GeoIp2Exception; import com.maxmind.geoip2.model.AnonymousIpResponse; import com.maxmind.geoip2.model.CityResponse; import com.maxmind.geoip2.model.ConnectionTypeResponse; import com.maxmind.geoip2.model.CountryResponse; import com.maxmind.geoip2.model.DomainResponse; import com.maxmind.geoip2.model.IspResponse; +import com.maxmind.geoip2.record.Traits; + +import java.io.Closeable; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.InetAddress; +import java.util.Arrays; +import java.util.List; /** * <p> @@ -54,10 +56,9 @@ import com.maxmind.geoip2.model.IspResponse; public class DatabaseReader implements GeoIp2Provider, Closeable { private final Reader reader; - private final ObjectMapper om; - private DatabaseReader(Builder builder) throws IOException { + private DatabaseReader(final Builder builder) throws IOException { if (builder.stream != null) { this.reader = new Reader(builder.stream); } else if (builder.database != null) { @@ -65,16 +66,13 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { } else { // This should never happen. If it does, review the Builder class // constructors for errors. - throw new IllegalArgumentException( - "Unsupported Builder configuration: expected either File or URL"); + throw new IllegalArgumentException("Unsupported Builder configuration: expected either File or URL"); } + this.om = new ObjectMapper(); - this.om.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, - false); - this.om.configure( - DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL, true); - InjectableValues inject = new InjectableValues.Std().addValue( - "locales", builder.locales); + this.om.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES,false); + this.om.configure(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL, true); + InjectableValues inject = new InjectableValues.Std().addValue("locales", builder.locales); this.om.setInjectableValues(inject); } @@ -82,9 +80,11 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { * <p> * Constructs a Builder for the DatabaseReader. The file passed to it must be a valid GeoIP2 database file. * </p> + * * <p> * <code>Builder</code> creates instances of <code>DatabaseReader</code> from values set by the methods. * </p> + * * <p> * Only the values set in the <code>Builder</code> constructor are required. * </p> @@ -150,16 +150,11 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { * @return An object of type T with the data for the IP address or null if no information could be found for the given IP address * @throws IOException if there is an error opening or reading from the file. */ - private <T> T get(InetAddress ipAddress, Class<T> cls, boolean hasTraits, - String type) throws IOException, AddressNotFoundException { - + private <T> T get(InetAddress ipAddress, Class<T> cls, boolean hasTraits, String type) throws IOException { String databaseType = this.getMetadata().getDatabaseType(); if (!databaseType.contains(type)) { - String caller = Thread.currentThread().getStackTrace()[2] - .getMethodName(); - throw new UnsupportedOperationException( - "Invalid attempt to open a " + databaseType - + " database using the " + caller + " method"); + String caller = Thread.currentThread().getStackTrace()[2].getMethodName(); + throw new UnsupportedOperationException("Invalid attempt to open a " + databaseType + " database using the " + caller + " method"); } ObjectNode node = (ObjectNode) this.reader.get(ipAddress); @@ -168,20 +163,11 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { return null; } - ObjectNode ipNode; - if (hasTraits) { - if (!node.has("traits")) { - node.set("traits", this.om.createObjectNode()); - } - ipNode = (ObjectNode) node.get("traits"); - } else { - ipNode = node; - } - ipNode.put("ip_address", ipAddress.getHostAddress()); - - return this.om.treeToValue(node, cls); + InjectableValues inject = new JsonInjector(ipAddress.getHostAddress()); + return this.om.reader(inject).treeToValue(node, cls); } + /** * <p> * Closes the database. @@ -200,15 +186,13 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { } @Override - public CountryResponse country(InetAddress ipAddress) throws IOException, - GeoIp2Exception { - return this.get(ipAddress, CountryResponse.class, true, "Country"); + public CountryResponse country(InetAddress ipAddress) throws IOException { + return get(ipAddress, CountryResponse.class, true, "Country"); } @Override - public CityResponse city(InetAddress ipAddress) throws IOException, - GeoIp2Exception { - return this.get(ipAddress, CityResponse.class, true, "City"); + public CityResponse city(InetAddress ipAddress) throws IOException { + return get(ipAddress, CityResponse.class, true, "City"); } /** @@ -216,12 +200,10 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { * * @param ipAddress IPv4 or IPv6 address to lookup. * @return a AnonymousIpResponse for the requested IP address. - * @throws GeoIp2Exception if there is an error looking up the IP * @throws IOException if there is an IO error */ - public AnonymousIpResponse anonymousIp(InetAddress ipAddress) throws IOException, - GeoIp2Exception { - return this.get(ipAddress, AnonymousIpResponse.class, false, "GeoIP2-Anonymous-IP"); + public AnonymousIpResponse anonymousIp(InetAddress ipAddress) throws IOException { + return get(ipAddress, AnonymousIpResponse.class, false, "GeoIP2-Anonymous-IP"); } /** @@ -229,13 +211,10 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { * * @param ipAddress IPv4 or IPv6 address to lookup. * @return a ConnectTypeResponse for the requested IP address. - * @throws GeoIp2Exception if there is an error looking up the IP * @throws IOException if there is an IO error */ - public ConnectionTypeResponse connectionType(InetAddress ipAddress) - throws IOException, GeoIp2Exception { - return this.get(ipAddress, ConnectionTypeResponse.class, false, - "GeoIP2-Connection-Type"); + public ConnectionTypeResponse connectionType(InetAddress ipAddress) throws IOException { + return get(ipAddress, ConnectionTypeResponse.class, false,"GeoIP2-Connection-Type"); } /** @@ -243,13 +222,10 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { * * @param ipAddress IPv4 or IPv6 address to lookup. * @return a DomainResponse for the requested IP address. - * @throws GeoIp2Exception if there is an error looking up the IP * @throws IOException if there is an IO error */ - public DomainResponse domain(InetAddress ipAddress) throws IOException, - GeoIp2Exception { - return this - .get(ipAddress, DomainResponse.class, false, "GeoIP2-Domain"); + public DomainResponse domain(InetAddress ipAddress) throws IOException { + return get(ipAddress, DomainResponse.class, false, "GeoIP2-Domain"); } /** @@ -257,12 +233,10 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { * * @param ipAddress IPv4 or IPv6 address to lookup. * @return an IspResponse for the requested IP address. - * @throws GeoIp2Exception if there is an error looking up the IP * @throws IOException if there is an IO error */ - public IspResponse isp(InetAddress ipAddress) throws IOException, - GeoIp2Exception { - return this.get(ipAddress, IspResponse.class, false, "GeoIP2-ISP"); + public IspResponse isp(InetAddress ipAddress) throws IOException { + return get(ipAddress, IspResponse.class, false, "GeoIP2-ISP"); } /** @@ -271,4 +245,23 @@ public class DatabaseReader implements GeoIp2Provider, Closeable { public Metadata getMetadata() { return this.reader.getMetadata(); } + + private class JsonInjector extends InjectableValues { + private final String ip; + + JsonInjector(final String ip) { + this.ip = ip; + } + + @Override + public Object findInjectableValue(final Object valueId, final DeserializationContext ctxt, final BeanProperty forProperty, final Object beanInstance) throws JsonMappingException { + if ("ip_address".equals(valueId)) { + return ip; + } else if ("traits".equals(valueId)) { + return new Traits(ip); + } + + return null; + } + } } http://git-wip-us.apache.org/repos/asf/nifi/blob/7bcf9fcb/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestGeoEnrichIP.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestGeoEnrichIP.java b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestGeoEnrichIP.java index 62a8f1e..f6629a0 100644 --- a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestGeoEnrichIP.java +++ b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestGeoEnrichIP.java @@ -19,7 +19,6 @@ package org.apache.nifi.processors; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.InjectableValues; import com.fasterxml.jackson.databind.ObjectMapper; -import com.maxmind.geoip2.exception.GeoIp2Exception; import com.maxmind.geoip2.model.CityResponse; import org.apache.nifi.annotation.lifecycle.OnScheduled; import org.apache.nifi.flowfile.FlowFile; @@ -239,11 +238,11 @@ public class TestGeoEnrichIP { @SuppressWarnings("unchecked") @Test - public void shouldFlowToNotFoundWhenGeoIp2ExceptionThrownFromMaxMind() throws Exception { + public void shouldFlowToNotFoundWhenExceptionThrownFromMaxMind() throws Exception { testRunner.setProperty(GeoEnrichIP.GEO_DATABASE_FILE, "./"); testRunner.setProperty(GeoEnrichIP.IP_ADDRESS_ATTRIBUTE, "ip"); - when(databaseReader.city(InetAddress.getByName("1.2.3.4"))).thenThrow(GeoIp2Exception.class); + when(databaseReader.city(InetAddress.getByName("1.2.3.4"))).thenThrow(IOException.class); final Map<String, String> attributes = new HashMap<>(); attributes.put("ip", "1.2.3.4"); http://git-wip-us.apache.org/repos/asf/nifi/blob/7bcf9fcb/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestISPEnrichIP.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestISPEnrichIP.java b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestISPEnrichIP.java index 187d0fe..8cc82ba 100644 --- a/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestISPEnrichIP.java +++ b/nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestISPEnrichIP.java @@ -19,7 +19,6 @@ package org.apache.nifi.processors; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.InjectableValues; import com.fasterxml.jackson.databind.ObjectMapper; -import com.maxmind.geoip2.exception.GeoIp2Exception; import com.maxmind.geoip2.model.IspResponse; import org.apache.nifi.annotation.lifecycle.OnScheduled; import org.apache.nifi.flowfile.FlowFile; @@ -224,11 +223,11 @@ public class TestISPEnrichIP { @SuppressWarnings("unchecked") @Test - public void shouldFlowToNotFoundWhenGeoIp2ExceptionThrownFromMaxMind() throws Exception { + public void shouldFlowToNotFoundWhenExceptionThrownFromMaxMind() throws Exception { testRunner.setProperty(ISPEnrichIP.GEO_DATABASE_FILE, "./"); testRunner.setProperty(ISPEnrichIP.IP_ADDRESS_ATTRIBUTE, "ip"); - when(databaseReader.isp(InetAddress.getByName("1.2.3.4"))).thenThrow(GeoIp2Exception.class); + when(databaseReader.isp(InetAddress.getByName("1.2.3.4"))).thenThrow(IOException.class); final Map<String, String> attributes = new HashMap<>(); attributes.put("ip", "1.2.3.4");