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");

Reply via email to