From c9e40fa35353c0763eed53c47b1327efa0c5346f Mon Sep 17 00:00:00 2001 From: Heemin Kim Date: Wed, 24 May 2023 16:46:24 -0700 Subject: [PATCH] Do not index blank value and do not enrich null property (#320) Signed-off-by: Heemin Kim --- .../ip2geo/common/GeoIpDataFacade.java | 4 ++ .../ip2geo/processor/Ip2GeoProcessor.java | 1 + .../ip2geo/common/GeoIpDataFacadeTests.java | 6 +-- .../processor/Ip2GeoProcessorTests.java | 41 +++++++++++++++++++ 4 files changed, 49 insertions(+), 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 index 4b6d9fe9..e9c32417 100644 --- 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 index b2c7e678..7c4f8120 100644 --- 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 filteredGeoData(final Map 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 index cd4c9bad..a5ca25fd 100644 --- 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 index 31451ea8..e83fc764 100644 --- 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 source = new HashMap<>(); + String ip = randomIpAddress(); + source.put("ip", ip); + Map ipToGeoData = Collections.emptyMap(); + IngestDocument document = new IngestDocument(source, new HashMap<>()); + BiConsumer 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 source = new HashMap<>(); + String ip = randomIpAddress(); + source.put("ip", ip); + Map ipToGeoData = Map.of("country", "USA"); + IngestDocument document = new IngestDocument(source, new HashMap<>()); + BiConsumer 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 config) throws Exception { Datasource datasource = new Datasource(); datasource.setName(datasourceName);