Do not index blank value and do not enrich null property (#320)

Signed-off-by: Heemin Kim <[email protected]>
opensearch-project · May 24, 2023 · c9e40fa · c9e40fa
1 parent 4b8fd80
commit c9e40fa
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 3 deletions.
diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/common/GeoIpDataFacade.java b/src/main/java/org/opensearch/geospatial/ip2geo/common/GeoIpDataFacade.java
@@ -34,6 +34,7 @@
 import org.apache.commons.csv.CSVFormat;
 import org.apache.commons.csv.CSVParser;
 import org.apache.commons.csv.CSVRecord;
+import org.apache.logging.log4j.util.Strings;
 import org.opensearch.OpenSearchException;
 import org.opensearch.SpecialPermission;
 import org.opensearch.action.ActionListener;
@@ -220,6 +221,9 @@ public XContentBuilder createDocument(final String[] fields, final String[] valu
         builder.field(IP_RANGE_FIELD_NAME, values[0]);
         builder.startObject(DATA_FIELD_NAME);
         for (int i = 1; i < fields.length; i++) {
+            if (Strings.isBlank(values[i])) {
+                continue;
+            }
             builder.field(fields[i], values[i]);
         }
         builder.endObject();

diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessor.java b/src/main/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessor.java
@@ -198,6 +198,7 @@ private Map<String, Object> filteredGeoData(final Map<String, Object> geoData, f
 
         filteredGeoData = properties.stream()
             .filter(p -> p.equals(PROPERTY_IP) == false)
+            .filter(p -> geoData.containsKey(p))
             .collect(Collectors.toMap(p -> p, p -> geoData.get(p)));
         if (properties.contains(PROPERTY_IP)) {
             filteredGeoData.put(PROPERTY_IP, ip);

diff --git a/src/test/java/org/opensearch/geospatial/ip2geo/common/GeoIpDataFacadeTests.java b/src/test/java/org/opensearch/geospatial/ip2geo/common/GeoIpDataFacadeTests.java
@@ -105,9 +105,9 @@ public void testCreateIndexIfNotExistsWithoutExistingIndex() {
     }
 
     @SneakyThrows
-    public void testCreateDocument() {
-        String[] names = { "ip", "country", "city" };
-        String[] values = { "1.0.0.0/25", "USA", "Seattle" };
+    public void testCreateDocument_whenBlankValue_thenDoNotAdd() {
+        String[] names = { "ip", "country", "location", "city" };
+        String[] values = { "1.0.0.0/25", "USA", " ", "Seattle" };
         assertEquals(
             "{\"_cidr\":\"1.0.0.0/25\",\"_data\":{\"country\":\"USA\",\"city\":\"Seattle\"}}",
             Strings.toString(noOpsGeoIpDataFacade.createDocument(names, values))

diff --git a/src/test/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessorTests.java b/src/test/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessorTests.java
@@ -24,6 +24,8 @@
 import java.util.Map;
 import java.util.function.BiConsumer;
 
+import lombok.SneakyThrows;
+
 import org.junit.Before;
 import org.mockito.ArgumentCaptor;
 import org.opensearch.OpenSearchException;
@@ -318,6 +320,45 @@ public void testExecuteInternal() throws Exception {
         );
     }
 
+    @SneakyThrows
+    public void testHandleSingleIp_whenEmptyGeoData_thenTargetFieldShouldNotSet() {
+        String datasourceName = GeospatialTestHelper.randomLowerCaseString();
+        Ip2GeoProcessor processor = createProcessor(datasourceName, Collections.emptyMap());
+        Map<String, Object> source = new HashMap<>();
+        String ip = randomIpAddress();
+        source.put("ip", ip);
+        Map<String, Object> ipToGeoData = Collections.emptyMap();
+        IngestDocument document = new IngestDocument(source, new HashMap<>());
+        BiConsumer<IngestDocument, Exception> handler = mock(BiConsumer.class);
+
+        // Run
+        processor.handleSingleIp(ip, ipToGeoData, document, handler);
+
+        // Verify
+        assertFalse(document.hasField(DEFAULT_TARGET_FIELD));
+        verify(handler).accept(document, null);
+    }
+
+    @SneakyThrows
+    public void testHandleSingleIp_whenNoValueForGivenProperty_thenDoNotAdd() {
+        String datasourceName = GeospatialTestHelper.randomLowerCaseString();
+        Ip2GeoProcessor processor = createProcessor(datasourceName, Map.of("properties", Arrays.asList("city", "country")));
+        Map<String, Object> source = new HashMap<>();
+        String ip = randomIpAddress();
+        source.put("ip", ip);
+        Map<String, Object> ipToGeoData = Map.of("country", "USA");
+        IngestDocument document = new IngestDocument(source, new HashMap<>());
+        BiConsumer<IngestDocument, Exception> handler = mock(BiConsumer.class);
+
+        // Run
+        processor.handleSingleIp(ip, ipToGeoData, document, handler);
+
+        // Verify
+        assertEquals("USA", document.getFieldValue(DEFAULT_TARGET_FIELD, Map.class).get("country"));
+        assertNull(document.getFieldValue(DEFAULT_TARGET_FIELD, Map.class).get("city"));
+        verify(handler).accept(document, null);
+    }
+
     private Ip2GeoProcessor createProcessor(final String datasourceName, final Map<String, Object> config) throws Exception {
         Datasource datasource = new Datasource();
         datasource.setName(datasourceName);