Skip to content

DateProcessor refactoring #124349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joegallo merged 5 commits into from
Mar 8, 2025
Merged

DateProcessor refactoring #124349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2160c9a
Conserve precious characters
joegallo Mar 7, 2025
ee8787e
This is just renderTemplate
joegallo Mar 7, 2025
1e2a8eb
Refactor DateProcessor to encapsulate the templates
joegallo Mar 7, 2025
d191b0c
Merge these two sets of methods together
joegallo Mar 7, 2025
6305025
Fix a test
joegallo Mar 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 32 additions & 35 deletions modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DateProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,9 @@ public final class DateProcessor extends AbstractProcessor {
formatter = DateFormatter.forPattern(this.outputFormat);
}

private static ZoneId newDateTimeZone(String timezone) {
return timezone == null ? ZoneOffset.UTC : ZoneId.of(timezone);
}

private static Locale newLocale(String locale) {
return locale == null ? Locale.ENGLISH : LocaleUtils.parse(locale);
}

@Override
public IngestDocument execute(IngestDocument ingestDocument) {
Object obj = ingestDocument.getFieldValue(field, Object.class);
public IngestDocument execute(IngestDocument document) {
Object obj = document.getFieldValue(field, Object.class);
String value = null;
if (obj != null) {
// Don't use Objects.toString(...) here, because null gets changed to "null" which may confuse some date parsers
Expand All @@ -115,10 +107,9 @@ public IngestDocument execute(IngestDocument ingestDocument) {
// extract the timezone and locale to use for date parsing
final ZoneId documentTimezone;
final Locale documentLocale;
final Map<String, Object> sourceAndMetadata = ingestDocument.getSourceAndMetadata();
try {
documentTimezone = newDateTimeZone(timezone == null ? null : timezone.newInstance(sourceAndMetadata).execute());
documentLocale = newLocale(locale == null ? null : locale.newInstance(sourceAndMetadata).execute());
documentTimezone = getTimezone(document);
documentLocale = getLocale(document);
} catch (Exception e) {
throw new IllegalArgumentException("unable to parse date [" + value + "]", e);
}
Expand All @@ -139,21 +130,33 @@ public IngestDocument execute(IngestDocument ingestDocument) {
throw new IllegalArgumentException("unable to parse date [" + value + "]", lastException);
}

ingestDocument.setFieldValue(targetField, formatter.format(dateTime));
return ingestDocument;
document.setFieldValue(targetField, formatter.format(dateTime));
return document;
}

@Override
public String getType() {
return TYPE;
}

TemplateScript.Factory getTimezone() {
return timezone;
// visible for testing
ZoneId getTimezone(IngestDocument document) {
String value = timezone == null ? null : document.renderTemplate(timezone);
if (value == null) {
return ZoneOffset.UTC;
} else {
return ZoneId.of(value);
}
}

TemplateScript.Factory getLocale() {
return locale;
// visible for testing
Locale getLocale(IngestDocument document) {
String value = locale == null ? null : document.renderTemplate(locale);
if (value == null) {
return Locale.ENGLISH;
} else {
return LocaleUtils.parse(value);
}
}

String getField() {
Expand Down Expand Up @@ -182,39 +185,33 @@ public Factory(ScriptService scriptService) {

public DateProcessor create(
Map<String, Processor.Factory> registry,
String processorTag,
String tag,
String description,
Map<String, Object> config,
ProjectId projectId
) throws Exception {
String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field");
String targetField = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "target_field", DEFAULT_TARGET_FIELD);
String timezoneString = ConfigurationUtils.readOptionalStringProperty(TYPE, processorTag, config, "timezone");
String field = ConfigurationUtils.readStringProperty(TYPE, tag, config, "field");
String targetField = ConfigurationUtils.readStringProperty(TYPE, tag, config, "target_field", DEFAULT_TARGET_FIELD);
String timezoneString = ConfigurationUtils.readOptionalStringProperty(TYPE, tag, config, "timezone");
TemplateScript.Factory compiledTimezoneTemplate = null;
if (timezoneString != null) {
compiledTimezoneTemplate = ConfigurationUtils.compileTemplate(
TYPE,
processorTag,
"timezone",
timezoneString,
scriptService
);
compiledTimezoneTemplate = ConfigurationUtils.compileTemplate(TYPE, tag, "timezone", timezoneString, scriptService);
}
String localeString = ConfigurationUtils.readOptionalStringProperty(TYPE, processorTag, config, "locale");
String localeString = ConfigurationUtils.readOptionalStringProperty(TYPE, tag, config, "locale");
TemplateScript.Factory compiledLocaleTemplate = null;
if (localeString != null) {
compiledLocaleTemplate = ConfigurationUtils.compileTemplate(TYPE, processorTag, "locale", localeString, scriptService);
compiledLocaleTemplate = ConfigurationUtils.compileTemplate(TYPE, tag, "locale", localeString, scriptService);
}
List<String> formats = ConfigurationUtils.readList(TYPE, processorTag, config, "formats");
String outputFormat = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "output_format", DEFAULT_OUTPUT_FORMAT);
List<String> formats = ConfigurationUtils.readList(TYPE, tag, config, "formats");
String outputFormat = ConfigurationUtils.readStringProperty(TYPE, tag, config, "output_format", DEFAULT_OUTPUT_FORMAT);
try {
DateFormatter.forPattern(outputFormat);
} catch (Exception e) {
throw new IllegalArgumentException("invalid output format [" + outputFormat + "]", e);
}

return new DateProcessor(
processorTag,
tag,
description,
compiledTimezoneTemplate,
compiledLocaleTemplate,
Expand Down
15 changes: 10 additions & 5 deletions ...ngest-common/src/test/java/org/elasticsearch/ingest/common/DateProcessorFactoryTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@
package org.elasticsearch.ingest.common;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.ingest.IngestDocument;
import org.elasticsearch.ingest.RandomDocumentPicks;
import org.elasticsearch.ingest.TestTemplateService;
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;

import java.time.ZoneId;
import java.time.ZoneOffset;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -43,8 +46,8 @@ public void testBuildDefaults() throws Exception {
assertThat(processor.getField(), equalTo(sourceField));
assertThat(processor.getTargetField(), equalTo(DateProcessor.DEFAULT_TARGET_FIELD));
assertThat(processor.getFormats(), equalTo(List.of("dd/MM/yyyyy")));
assertNull(processor.getLocale());
assertNull(processor.getTimezone());
assertThat(processor.getTimezone(null), equalTo(ZoneOffset.UTC));
assertThat(processor.getLocale(null), equalTo(Locale.ENGLISH));
}

public void testMatchFieldIsMandatory() throws Exception {
Expand Down Expand Up @@ -81,11 +84,12 @@ public void testParseLocale() throws Exception {
String sourceField = randomAlphaOfLengthBetween(1, 10);
config.put("field", sourceField);
config.put("formats", List.of("dd/MM/yyyyy"));
Locale locale = randomFrom(Locale.GERMANY, Locale.FRENCH, Locale.ROOT);
Locale locale = randomFrom(Locale.GERMANY, Locale.FRENCH, Locale.CANADA);
config.put("locale", locale.toLanguageTag());

DateProcessor processor = factory.create(null, null, null, config, null);
assertThat(processor.getLocale().newInstance(Map.of()).execute(), equalTo(locale.toLanguageTag()));
IngestDocument document = RandomDocumentPicks.randomIngestDocument(random(), Map.of());
assertThat(processor.getLocale(document), equalTo(locale));
}

public void testParseTimezone() throws Exception {
Expand All @@ -97,7 +101,8 @@ public void testParseTimezone() throws Exception {
ZoneId timezone = randomZone();
config.put("timezone", timezone.getId());
DateProcessor processor = factory.create(null, null, null, config, null);
assertThat(processor.getTimezone().newInstance(Map.of()).execute(), equalTo(timezone.getId()));
IngestDocument document = RandomDocumentPicks.randomIngestDocument(random(), Map.of());
assertThat(processor.getTimezone(document), equalTo(timezone));
}

public void testParseMatchFormats() throws Exception {
Expand Down