From 3684af1b93ee171c48bfcc69f7157856682bace4 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 24 Aug 2018 11:57:46 +0200 Subject: [PATCH] [Rollup] Move getMetadata() methods out of rollup config objects (#32579) This committ removes the getMetadata() methods from the DateHistoGroupConfig and HistoGroupConfig objects. This way the configuration objects do not rely on RollupField.formatMetaField() anymore and do not expose a getMetadata() method that is tighlty coupled to the rollup indexer. --- .../rollup/job/DateHistogramGroupConfig.java | 4 -- .../core/rollup/job/HistogramGroupConfig.java | 5 +- .../xpack/rollup/job/RollupIndexer.java | 21 ++++++-- .../xpack/rollup/job/RollupIndexerTests.java | 49 +++++++++++++++++++ 4 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerTests.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java index 77dfa1cbbb1c3..281277043c829 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java @@ -211,10 +211,6 @@ public Map toAggCap() { return map; } - public Map getMetadata() { - return Collections.singletonMap(RollupField.formatMetaField(RollupField.INTERVAL), interval.toString()); - } - public void validateMappings(Map> fieldCapsResponse, ActionRequestValidationException validationException) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfig.java index 0480050bf52f0..1e1f88a7c20e1 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistogramGroupConfig.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; @@ -115,8 +116,8 @@ public Map toAggCap() { return map; } - public Map getMetadata() { - return Collections.singletonMap(RollupField.formatMetaField(RollupField.INTERVAL), interval); + public Set getAllFields() { + return Arrays.stream(fields).collect(Collectors.toSet()); } public void validateMappings(Map> fieldCapsResponse, diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java index 87294706b3b7d..d1db021361c8c 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java @@ -23,6 +23,7 @@ import org.elasticsearch.xpack.core.rollup.RollupField; import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; import org.elasticsearch.xpack.core.rollup.job.GroupConfig; +import org.elasticsearch.xpack.core.rollup.job.HistogramGroupConfig; import org.elasticsearch.xpack.core.rollup.job.IndexerState; import org.elasticsearch.xpack.core.rollup.job.RollupJob; import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; @@ -392,15 +393,12 @@ private SearchRequest buildSearchRequest() { private CompositeAggregationBuilder createCompositeBuilder(RollupJobConfig config) { final GroupConfig groupConfig = config.getGroupConfig(); List> builders = new ArrayList<>(); - Map metadata = new HashMap<>(); // Add all the agg builders to our request in order: date_histo -> histo -> terms if (groupConfig != null) { builders.addAll(groupConfig.getDateHistogram().toBuilders()); - metadata.putAll(groupConfig.getDateHistogram().getMetadata()); if (groupConfig.getHistogram() != null) { builders.addAll(groupConfig.getHistogram().toBuilders()); - metadata.putAll(groupConfig.getHistogram().getMetadata()); } if (groupConfig.getTerms() != null) { builders.addAll(groupConfig.getTerms().toBuilders()); @@ -409,6 +407,8 @@ private CompositeAggregationBuilder createCompositeBuilder(RollupJobConfig confi CompositeAggregationBuilder composite = new CompositeAggregationBuilder(AGGREGATION_NAME, builders); config.getMetricsConfig().forEach(m -> m.toBuilders().forEach(composite::subAggregation)); + + final Map metadata = createMetadata(groupConfig); if (metadata.isEmpty() == false) { composite.setMetaData(metadata); } @@ -441,5 +441,20 @@ private QueryBuilder createBoundaryQuery(Map position) { .format("epoch_millis"); return query; } + + static Map createMetadata(final GroupConfig groupConfig) { + final Map metadata = new HashMap<>(); + if (groupConfig != null) { + // Add all the metadata in order: date_histo -> histo + final DateHistogramGroupConfig dateHistogram = groupConfig.getDateHistogram(); + metadata.put(RollupField.formatMetaField(RollupField.INTERVAL), dateHistogram.getInterval().toString()); + + final HistogramGroupConfig histogram = groupConfig.getHistogram(); + if (histogram != null) { + metadata.put(RollupField.formatMetaField(RollupField.INTERVAL), histogram.getInterval()); + } + } + return metadata; + } } diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerTests.java new file mode 100644 index 0000000000000..5ab85e2ffa743 --- /dev/null +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerTests.java @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.rollup.job; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers; +import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; +import org.elasticsearch.xpack.core.rollup.job.GroupConfig; +import org.elasticsearch.xpack.core.rollup.job.HistogramGroupConfig; + +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; + +public class RollupIndexerTests extends ESTestCase { + + public void testCreateMetadataNoGroupConfig() { + final Map metadata = RollupIndexer.createMetadata(null); + assertNotNull(metadata); + assertTrue(metadata.isEmpty()); + } + + public void testCreateMetadataWithDateHistogramGroupConfigOnly() { + final DateHistogramGroupConfig dateHistogram = ConfigTestHelpers.randomDateHistogramGroupConfig(random()); + final GroupConfig groupConfig = new GroupConfig(dateHistogram); + + final Map metadata = RollupIndexer.createMetadata(groupConfig); + assertEquals(1, metadata.size()); + assertTrue(metadata.containsKey("_rollup.interval")); + Object value = metadata.get("_rollup.interval"); + assertThat(value, equalTo(dateHistogram.getInterval().toString())); + } + + public void testCreateMetadata() { + final DateHistogramGroupConfig dateHistogram = ConfigTestHelpers.randomDateHistogramGroupConfig(random()); + final HistogramGroupConfig histogram = ConfigTestHelpers.randomHistogramGroupConfig(random()); + final GroupConfig groupConfig = new GroupConfig(dateHistogram, histogram, null); + + final Map metadata = RollupIndexer.createMetadata(groupConfig); + assertEquals(1, metadata.size()); + assertTrue(metadata.containsKey("_rollup.interval")); + Object value = metadata.get("_rollup.interval"); + assertThat(value, equalTo(histogram.getInterval())); + } +} +