diff --git a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/ConfigurationUtils.java b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/ConfigurationUtils.java index 7e4de927f74..182ff61c421 100644 --- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/ConfigurationUtils.java +++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/ConfigurationUtils.java @@ -19,6 +19,7 @@ package org.elasticsearch.ingest.processor; +import java.util.List; import java.util.Map; public final class ConfigurationUtils { @@ -26,6 +27,26 @@ public final class ConfigurationUtils { private ConfigurationUtils() { } + /** + * Returns and removes the specified optional property from the specified configuration map. + * + * If the property value isn't of type string a {@link IllegalArgumentException} is thrown. + */ + public static String readOptionalStringProperty(Map configuration, String propertyName) { + Object value = configuration.remove(propertyName); + return readString(propertyName, value); + } + + /** + * Returns and removes the specified property from the specified configuration map. + * + * If the property value isn't of type string an {@link IllegalArgumentException} is thrown. + * If the property is missing an {@link IllegalArgumentException} is thrown + */ + public static String readStringProperty(Map configuration, String propertyName) { + return readStringProperty(configuration, propertyName, null); + } + /** * Returns and removes the specified property from the specified configuration map. * @@ -39,13 +60,36 @@ public final class ConfigurationUtils { } else if (value == null) { throw new IllegalArgumentException("required property [" + propertyName + "] is missing"); } - - if (value instanceof String) { - return (String) value; - } else { - throw new IllegalArgumentException("property [" + propertyName + "] isn't a string, but of type [" + value.getClass() + "]"); - } + return readString(propertyName, value); } + private static String readString(String propertyName, Object value) { + if (value == null) { + return null; + } + if (value instanceof String) { + return (String) value; + } + throw new IllegalArgumentException("property [" + propertyName + "] isn't a string, but of type [" + value.getClass().getName() + "]"); + } + /** + * Returns and removes the specified property of type list from the specified configuration map. + * + * If the property value isn't of type list an {@link IllegalArgumentException} is thrown. + * If the property is missing an {@link IllegalArgumentException} is thrown + */ + public static List readStringList(Map configuration, String propertyName) { + Object value = configuration.remove(propertyName); + if (value == null) { + throw new IllegalArgumentException("required property [" + propertyName + "] is missing"); + } + if (value instanceof List) { + @SuppressWarnings("unchecked") + List stringList = (List) value; + return stringList; + } else { + throw new IllegalArgumentException("property [" + propertyName + "] isn't a list, but of type [" + value.getClass().getName() + "]"); + } + } } diff --git a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/date/DateProcessor.java b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/date/DateProcessor.java index d8ea69424c8..4c61cc8ee7a 100644 --- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/date/DateProcessor.java +++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/date/DateProcessor.java @@ -20,9 +20,11 @@ package org.elasticsearch.ingest.processor.date; import org.elasticsearch.ingest.Data; +import org.elasticsearch.ingest.processor.ConfigurationUtils; import org.elasticsearch.ingest.processor.Processor; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; +import org.joda.time.format.ISODateTimeFormat; import java.util.ArrayList; import java.util.List; @@ -38,6 +40,7 @@ public final class DateProcessor implements Processor { private final Locale locale; private final String matchField; private final String targetField; + private final List matchFormats; private final List dateParsers; DateProcessor(DateTimeZone timezone, Locale locale, String matchField, List matchFormats, String targetField) { @@ -45,6 +48,7 @@ public final class DateProcessor implements Processor { this.locale = locale; this.matchField = matchField; this.targetField = targetField; + this.matchFormats = matchFormats; this.dateParsers = new ArrayList<>(); for (String matchFormat : matchFormats) { dateParsers.add(DateParserFactory.createDateParser(matchFormat, timezone, locale)); @@ -62,8 +66,7 @@ public final class DateProcessor implements Processor { try { dateTime = dateParser.parseDateTime(value); } catch(Exception e) { - //TODO is there a better way other than catching exception? - //try the next parser + //try the next parser and keep track of the last exception lastException = e; } } @@ -72,26 +75,41 @@ public final class DateProcessor implements Processor { throw new IllegalArgumentException("unable to parse date [" + value + "]", lastException); } - String dateAsISO8601 = dateTime.toString(); - data.addField(targetField, dateAsISO8601); + data.addField(targetField, ISODateTimeFormat.dateTime().print(dateTime)); } - public static class Factory implements Processor.Factory { + DateTimeZone getTimezone() { + return timezone; + } + + Locale getLocale() { + return locale; + } + + String getMatchField() { + return matchField; + } + + String getTargetField() { + return targetField; + } + + List getMatchFormats() { + return matchFormats; + } + + public static class Factory implements Processor.Factory { @SuppressWarnings("unchecked") - public Processor create(Map config) { - String timezoneString = (String) config.get("timezone"); - DateTimeZone timezone = (timezoneString == null) ? DateTimeZone.UTC : DateTimeZone.forID(timezoneString); - String localeString = (String) config.get("locale"); + public DateProcessor create(Map config) { + String matchField = ConfigurationUtils.readStringProperty(config, "match_field"); + String targetField = ConfigurationUtils.readStringProperty(config, "target_field", DEFAULT_TARGET_FIELD); + String timezoneString = ConfigurationUtils.readOptionalStringProperty(config, "timezone"); + DateTimeZone timezone = timezoneString == null ? DateTimeZone.UTC : DateTimeZone.forID(timezoneString); + String localeString = ConfigurationUtils.readOptionalStringProperty(config, "locale"); Locale locale = localeString == null ? Locale.ENGLISH : Locale.forLanguageTag(localeString); - String matchField = (String) config.get("match_field"); - List matchFormats = (List) config.get("match_formats"); - String targetField = (String) config.get("target_field"); - if (targetField == null) { - targetField = DEFAULT_TARGET_FIELD; - } + List matchFormats = ConfigurationUtils.readStringList(config, "match_formats"); return new DateProcessor(timezone, locale, matchField, matchFormats, targetField); } } - } diff --git a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/geoip/GeoIpProcessor.java b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/geoip/GeoIpProcessor.java index 020b88a23e0..e0ecd61370e 100644 --- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/geoip/GeoIpProcessor.java +++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/geoip/GeoIpProcessor.java @@ -164,7 +164,7 @@ public final class GeoIpProcessor implements Processor { private final DatabaseReaderService databaseReaderService = new DatabaseReaderService(); public GeoIpProcessor create(Map config) throws IOException { - String ipField = readStringProperty(config, "ip_field", null); + String ipField = readStringProperty(config, "ip_field"); String targetField = readStringProperty(config, "target_field", "geoip"); String databaseFile = readStringProperty(config, "database_file", "GeoLite2-City.mmdb"); diff --git a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/grok/GrokProcessor.java b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/grok/GrokProcessor.java index 9f7a7c78c07..bdba25c7c78 100644 --- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/grok/GrokProcessor.java +++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/grok/GrokProcessor.java @@ -68,8 +68,8 @@ public final class GrokProcessor implements Processor { private Path grokConfigDirectory; public GrokProcessor create(Map config) throws IOException { - String matchField = ConfigurationUtils.readStringProperty(config, "field", null); - String matchPattern = ConfigurationUtils.readStringProperty(config, "pattern", null); + String matchField = ConfigurationUtils.readStringProperty(config, "field"); + String matchPattern = ConfigurationUtils.readStringProperty(config, "pattern"); Map patternBank = new HashMap<>(); Path patternsDirectory = grokConfigDirectory.resolve("patterns"); try (DirectoryStream stream = Files.newDirectoryStream(patternsDirectory)) { diff --git a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/simple/SimpleProcessor.java b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/simple/SimpleProcessor.java index 0bfd07de462..d8dfc230a7b 100644 --- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/simple/SimpleProcessor.java +++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/simple/SimpleProcessor.java @@ -55,10 +55,10 @@ public final class SimpleProcessor implements Processor { public static class Factory implements Processor.Factory { public SimpleProcessor create(Map config) { - String path = ConfigurationUtils.readStringProperty(config, "path", null); - String expectedValue = ConfigurationUtils.readStringProperty(config, "expected_value", null); + String path = ConfigurationUtils.readStringProperty(config, "path"); + String expectedValue = ConfigurationUtils.readStringProperty(config, "expected_value"); String addField = ConfigurationUtils.readStringProperty(config, "add_field", null); - String addFieldValue = ConfigurationUtils.readStringProperty(config, "add_field_value", null); + String addFieldValue = ConfigurationUtils.readStringProperty(config, "add_field_value"); return new SimpleProcessor(path, expectedValue, addField, addFieldValue); } diff --git a/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/date/DateProcessorFactoryTests.java b/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/date/DateProcessorFactoryTests.java new file mode 100644 index 00000000000..e637bf19012 --- /dev/null +++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/date/DateProcessorFactoryTests.java @@ -0,0 +1,142 @@ +/* + * Licensed to ElasticSearch and Shay Banon under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. ElasticSearch licenses this + * file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.ingest.processor.date; + +import org.elasticsearch.test.ESTestCase; +import org.joda.time.DateTimeZone; + +import java.util.*; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; + +public class DateProcessorFactoryTests extends ESTestCase { + + public void testBuildDefaults() throws Exception { + DateProcessor.Factory factory = new DateProcessor.Factory(); + Map config = new HashMap<>(); + String sourceField = randomAsciiOfLengthBetween(1, 10); + config.put("match_field", sourceField); + config.put("match_formats", Collections.singletonList("dd/MM/yyyyy")); + + DateProcessor processor = factory.create(config); + assertThat(processor.getMatchField(), equalTo(sourceField)); + assertThat(processor.getTargetField(), equalTo(DateProcessor.DEFAULT_TARGET_FIELD)); + assertThat(processor.getMatchFormats(), equalTo(Collections.singletonList("dd/MM/yyyyy"))); + assertThat(processor.getLocale(), equalTo(Locale.ENGLISH)); + assertThat(processor.getTimezone(), equalTo(DateTimeZone.UTC)); + } + + public void testMatchFieldIsMandatory() throws Exception { + DateProcessor.Factory factory = new DateProcessor.Factory(); + Map config = new HashMap<>(); + String targetField = randomAsciiOfLengthBetween(1, 10); + config.put("target_field", targetField); + config.put("match_formats", Collections.singletonList("dd/MM/yyyyy")); + + try { + factory.create(config); + fail("processor creation should have failed"); + } catch(IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("required property [match_field] is missing")); + } + } + + public void testMatchFormatsIsMandatory() throws Exception { + DateProcessor.Factory factory = new DateProcessor.Factory(); + Map config = new HashMap<>(); + String sourceField = randomAsciiOfLengthBetween(1, 10); + String targetField = randomAsciiOfLengthBetween(1, 10); + config.put("match_field", sourceField); + config.put("target_field", targetField); + + try { + factory.create(config); + fail("processor creation should have failed"); + } catch(IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("required property [match_formats] is missing")); + } + } + + public void testParseLocale() throws Exception { + DateProcessor.Factory factory = new DateProcessor.Factory(); + Map config = new HashMap<>(); + String sourceField = randomAsciiOfLengthBetween(1, 10); + config.put("match_field", sourceField); + config.put("match_formats", Collections.singletonList("dd/MM/yyyyy")); + Locale locale = randomLocale(random()); + config.put("locale", locale.toLanguageTag()); + + DateProcessor processor = factory.create(config); + assertThat(processor.getLocale(), equalTo(locale)); + } + + public void testParseTimezone() throws Exception { + DateProcessor.Factory factory = new DateProcessor.Factory(); + Map config = new HashMap<>(); + String sourceField = randomAsciiOfLengthBetween(1, 10); + config.put("match_field", sourceField); + config.put("match_formats", Collections.singletonList("dd/MM/yyyyy")); + DateTimeZone timeZone = DateTimeZone.forTimeZone(randomTimeZone(random())); + config.put("timezone", timeZone.getID()); + + DateProcessor processor = factory.create(config); + assertThat(processor.getTimezone(), equalTo(timeZone)); + } + + public void testParseMatchFormats() throws Exception { + DateProcessor.Factory factory = new DateProcessor.Factory(); + Map config = new HashMap<>(); + String sourceField = randomAsciiOfLengthBetween(1, 10); + config.put("match_field", sourceField); + config.put("match_formats", Arrays.asList("dd/MM/yyyy", "dd-MM-yyyy")); + + DateProcessor processor = factory.create(config); + assertThat(processor.getMatchFormats(), equalTo(Arrays.asList("dd/MM/yyyy", "dd-MM-yyyy"))); + } + + public void testParseMatchFormatsFailure() throws Exception { + DateProcessor.Factory factory = new DateProcessor.Factory(); + Map config = new HashMap<>(); + String sourceField = randomAsciiOfLengthBetween(1, 10); + config.put("match_field", sourceField); + config.put("match_formats", "dd/MM/yyyy"); + + try { + factory.create(config); + fail("processor creation should have failed"); + } catch(IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("property [match_formats] isn't a list, but of type [java.lang.String]")); + } + } + + public void testParseTargetField() throws Exception { + DateProcessor.Factory factory = new DateProcessor.Factory(); + Map config = new HashMap<>(); + String sourceField = randomAsciiOfLengthBetween(1, 10); + String targetField = randomAsciiOfLengthBetween(1, 10); + config.put("match_field", sourceField); + config.put("target_field", targetField); + config.put("match_formats", Arrays.asList("dd/MM/yyyy", "dd-MM-yyyy")); + + DateProcessor processor = factory.create(config); + assertThat(processor.getTargetField(), equalTo(targetField)); + } +} diff --git a/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/geoip/GeoIpProcessorFactoryTests.java b/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/geoip/GeoIpProcessorFactoryTests.java index ad270d18e96..236e91818fb 100644 --- a/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/geoip/GeoIpProcessorFactoryTests.java +++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/geoip/GeoIpProcessorFactoryTests.java @@ -94,6 +94,4 @@ public class GeoIpProcessorFactoryTests extends ESTestCase { assertThat(e.getMessage(), startsWith("database file [does-not-exist.mmdb] doesn't exist in")); } } - - }