Skip to content

Commit

Permalink
IndexNameExpressionResolver refactoring (elastic#116085)
Browse files Browse the repository at this point in the history 
* Refactor DateMathExpressionResolver.

In this commit we reduce the scope of the DateMathExpressionResolver to only handle one expression at a time. This simplifies the code since it move the preprocessing from the date math calculation.

Furthermore, we simplify the API, so it does not need a context.

Finally, the reduced scope allowed us to reduce the test footprint. The tests are targeted only to the single expression date math resolution and any test with expression combinations will be moved to the IndexNameExpressionResolverTests.

* Create SystemResourceAccess.

In this class we collect all the related access checks to system indices.

These checks are not straight forward and there are different rules that apply on different parts of the code.

In this PR, we just collect them in one place to allow further analysis to determine if these differences are a feature or a bug.

* Refactor WildcardExpressionResolver.

In this PR we reduced the scope of the WildcardExpressionResolver to resolve one expression at a time. It also still supports the `*`.

This allows us to reduce the scope of the test as well.

Furthermore, we switched the usage of streams to more imperative code to reduce the object creation.

* Refactor expression resolution to resources.

In this PR we bring all the previous steps together. We change the expression resolution, instead of processing lists of expressions to completely resolve one expression to its resources before moving to the next.

This intends to increase the maintainability of the code, because we can debug it easier and we reduce the code duplication when dealing with exclusions and other pre-processing tasks.

* Fix format

* Bug fix: do the empty check on wildcard expressions on each wildcard

* Polishing

* Optimise for no wildcards

* Fix test name typo

* Replace for-each loops with for-i loops

---------

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: James Baiera <[email protected]>
  • Loading branch information
@gmarouli @elasticmachine @jbaiera
3 people authored Nov 21, 2024
1 parent b9bac36 commit 7dc2cc6
Show file tree
Hide file tree
Showing 6 changed files with 677 additions and 863 deletions.
835 changes: 446 additions & 389 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java
View file

Large diffs are not rendered by default.

201 changes: 64 additions & 137 deletions server/src/test/java/org/elasticsearch/cluster/metadata/DateMathExpressionResolverTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,163 +10,90 @@
package org.elasticsearch.cluster.metadata;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.Context;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.DateMathExpressionResolver;
import org.elasticsearch.indices.SystemIndices.SystemIndexAccessLevel;
import org.elasticsearch.test.ESTestCase;
import org.hamcrest.Matchers;

import java.time.Instant;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.function.LongSupplier;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

public class DateMathExpressionResolverTests extends ESTestCase {

private final Context context = new Context(
ClusterState.builder(new ClusterName("_name")).build(),
IndicesOptions.strictExpand(),
SystemIndexAccessLevel.NONE
);
private final long now = randomMillisUpToYear9999();
private final LongSupplier getTime = () -> now;

private static ZonedDateTime dateFromMillis(long millis) {
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC);
}
public void testNoDateMathExpression() {
String expression = randomAlphaOfLength(10);
assertThat(DateMathExpressionResolver.resolveExpression(expression, getTime), equalTo(expression));

private static String formatDate(String pattern, ZonedDateTime zonedDateTime) {
DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern(pattern, Locale.ROOT);
return dateFormatter.format(zonedDateTime);
expression = "*";
assertThat(DateMathExpressionResolver.resolveExpression(expression, getTime), equalTo(expression));
}

public void testNormal() throws Exception {
int numIndexExpressions = randomIntBetween(1, 9);
List<String> indexExpressions = new ArrayList<>(numIndexExpressions);
for (int i = 0; i < numIndexExpressions; i++) {
indexExpressions.add(randomAlphaOfLength(10));
}
List<String> result = DateMathExpressionResolver.resolve(context, indexExpressions);
assertThat(result.size(), equalTo(indexExpressions.size()));
for (int i = 0; i < indexExpressions.size(); i++) {
assertThat(result.get(i), equalTo(indexExpressions.get(i)));
}
}
public void testExpression() {
String result = DateMathExpressionResolver.resolveExpression("<.marvel-{now}>", getTime);
assertThat(result, equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));

public void testExpression() throws Exception {
List<String> indexExpressions = Arrays.asList("<.marvel-{now}>", "<.watch_history-{now}>", "<logstash-{now}>");
List<String> result = DateMathExpressionResolver.resolve(context, indexExpressions);
assertThat(result.size(), equalTo(3));
assertThat(result.get(0), equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
assertThat(result.get(1), equalTo(".watch_history-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
assertThat(result.get(2), equalTo("logstash-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
result = DateMathExpressionResolver.resolveExpression("<.watch_history-{now}>", getTime);
assertThat(result, equalTo(".watch_history-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));

result = DateMathExpressionResolver.resolveExpression("<logstash-{now}>", getTime);
assertThat(result, equalTo("logstash-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));
}

public void testExpressionWithWildcardAndExclusions() {
List<String> indexExpressions = Arrays.asList(
"<-before-inner-{now}>",
"-<before-outer-{now}>",
"<wild*card-{now}*>",
"<-after-inner-{now}>",
"-<after-outer-{now}>"
);
List<String> result = DateMathExpressionResolver.resolve(context, indexExpressions);
assertThat(
result,
Matchers.contains(
equalTo("-before-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
equalTo("-<before-outer-{now}>"), // doesn't evaluate because it doesn't start with "<" and it is not an exclusion
equalTo("wild*card-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime())) + "*"),
equalTo("-after-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
equalTo("-after-outer-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime())))
)
);
Context noWildcardExpandContext = new Context(
ClusterState.builder(new ClusterName("_name")).build(),
IndicesOptions.strictSingleIndexNoExpandForbidClosed(),
SystemIndexAccessLevel.NONE
);
result = DateMathExpressionResolver.resolve(noWildcardExpandContext, indexExpressions);
assertThat(
result,
Matchers.contains(
equalTo("-before-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
// doesn't evaluate because it doesn't start with "<" and there can't be exclusions without wildcard expansion
equalTo("-<before-outer-{now}>"),
equalTo("wild*card-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime())) + "*"),
equalTo("-after-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
// doesn't evaluate because it doesn't start with "<" and there can't be exclusions without wildcard expansion
equalTo("-<after-outer-{now}>")
)
);
}
String result = DateMathExpressionResolver.resolveExpression("<-before-inner-{now}>", getTime);
assertThat(result, equalTo("-before-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));

result = DateMathExpressionResolver.resolveExpression("<wild*card-{now}*>", getTime);
assertThat(result, equalTo("wild*card-" + formatDate("uuuu.MM.dd", dateFromMillis(now)) + "*"));

result = DateMathExpressionResolver.resolveExpression("<-after-inner-{now}>", getTime);
assertThat(result, equalTo("-after-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));

public void testEmpty() throws Exception {
List<String> result = DateMathExpressionResolver.resolve(context, Collections.<String>emptyList());
assertThat(result.size(), equalTo(0));
}

public void testExpression_Static() throws Exception {
List<String> result = DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-test>"));
assertThat(result.size(), equalTo(1));
assertThat(result.get(0), equalTo(".marvel-test"));
public void testExpression_Static() {
String result = DateMathExpressionResolver.resolveExpression("<.marvel-test>", getTime);
assertThat(result, equalTo(".marvel-test"));
}

public void testExpression_MultiParts() throws Exception {
List<String> result = DateMathExpressionResolver.resolve(context, Arrays.asList("<.text1-{now/d}-text2-{now/M}>"));
assertThat(result.size(), equalTo(1));
public void testExpression_MultiParts() {
String result = DateMathExpressionResolver.resolveExpression("<.text1-{now/d}-text2-{now/M}>", getTime);
assertThat(
result.get(0),
result,
equalTo(
".text1-"
+ formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))
+ formatDate("uuuu.MM.dd", dateFromMillis(now))
+ "-text2-"
+ formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()).withDayOfMonth(1))
+ formatDate("uuuu.MM.dd", dateFromMillis(now).withDayOfMonth(1))
)
);
}

public void testExpression_CustomFormat() throws Exception {
List<String> results = DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-{now/d{yyyy.MM.dd}}>"));
assertThat(results.size(), equalTo(1));
assertThat(results.get(0), equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
}

public void testExpression_EscapeStatic() throws Exception {
List<String> result = DateMathExpressionResolver.resolve(context, Arrays.asList("<.mar\\{v\\}el-{now/d}>"));
assertThat(result.size(), equalTo(1));
assertThat(result.get(0), equalTo(".mar{v}el-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
public void testExpression_CustomFormat() {
String result = DateMathExpressionResolver.resolveExpression("<.marvel-{now/d{yyyy.MM.dd}}>", getTime);
assertThat(result, equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));
}

public void testExpression_EscapeDateFormat() throws Exception {
List<String> result = DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-{now/d{'\\{year\\}'yyyy}}>"));
assertThat(result.size(), equalTo(1));
assertThat(result.get(0), equalTo(".marvel-" + formatDate("'{year}'yyyy", dateFromMillis(context.getStartTime()))));
public void testExpression_EscapeStatic() {
String result = DateMathExpressionResolver.resolveExpression("<.mar\\{v\\}el-{now/d}>", getTime);
assertThat(result, equalTo(".mar{v}el-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));
}

public void testExpression_MixedArray() throws Exception {
List<String> result = DateMathExpressionResolver.resolve(
context,
Arrays.asList("name1", "<.marvel-{now/d}>", "name2", "<.logstash-{now/M{uuuu.MM}}>")
);
assertThat(result.size(), equalTo(4));
assertThat(result.get(0), equalTo("name1"));
assertThat(result.get(1), equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
assertThat(result.get(2), equalTo("name2"));
assertThat(result.get(3), equalTo(".logstash-" + formatDate("uuuu.MM", dateFromMillis(context.getStartTime()).withDayOfMonth(1))));
public void testExpression_EscapeDateFormat() {
String result = DateMathExpressionResolver.resolveExpression("<.marvel-{now/d{'\\{year\\}'yyyy}}>", getTime);
assertThat(result, equalTo(".marvel-" + formatDate("'{year}'yyyy", dateFromMillis(now))));
}

public void testExpression_CustomTimeZoneInIndexName() throws Exception {
public void testExpression_CustomTimeZoneInIndexName() {
ZoneId timeZone;
int hoursOffset;
int minutesOffset = 0;
Expand Down Expand Up @@ -194,57 +121,57 @@ public void testExpression_CustomTimeZoneInIndexName() throws Exception {
// rounding to today 00:00
now = ZonedDateTime.now(ZoneOffset.UTC).withHour(0).withMinute(0).withSecond(0);
}
Context context = new Context(
this.context.getState(),
this.context.getOptions(),
now.toInstant().toEpochMilli(),
SystemIndexAccessLevel.NONE,
name -> false,
name -> false
);
List<String> results = DateMathExpressionResolver.resolve(
context,
Arrays.asList("<.marvel-{now/d{yyyy.MM.dd|" + timeZone.getId() + "}}>")

String result = DateMathExpressionResolver.resolveExpression(
"<.marvel-{now/d{yyyy.MM.dd|" + timeZone.getId() + "}}>",
() -> now.toInstant().toEpochMilli()
);
assertThat(results.size(), equalTo(1));
logger.info("timezone: [{}], now [{}], name: [{}]", timeZone, now, results.get(0));
assertThat(results.get(0), equalTo(".marvel-" + formatDate("uuuu.MM.dd", now.withZoneSameInstant(timeZone))));
logger.info("timezone: [{}], now [{}], name: [{}]", timeZone, now, result);
assertThat(result, equalTo(".marvel-" + formatDate("uuuu.MM.dd", now.withZoneSameInstant(timeZone))));
}

public void testExpressionInvalidUnescaped() throws Exception {
public void testExpressionInvalidUnescaped() {
Exception e = expectThrows(
ElasticsearchParseException.class,
() -> DateMathExpressionResolver.resolve(context, Arrays.asList("<.mar}vel-{now/d}>"))
() -> DateMathExpressionResolver.resolveExpression("<.mar}vel-{now/d}>", getTime)
);
assertThat(e.getMessage(), containsString("invalid dynamic name expression"));
assertThat(e.getMessage(), containsString("invalid character at position ["));
}

public void testExpressionInvalidDateMathFormat() throws Exception {
public void testExpressionInvalidDateMathFormat() {
Exception e = expectThrows(
ElasticsearchParseException.class,
() -> DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-{now/d{}>"))
() -> DateMathExpressionResolver.resolveExpression("<.marvel-{now/d{}>", getTime)
);
assertThat(e.getMessage(), containsString("invalid dynamic name expression"));
assertThat(e.getMessage(), containsString("date math placeholder is open ended"));
}

public void testExpressionInvalidEmptyDateMathFormat() throws Exception {
public void testExpressionInvalidEmptyDateMathFormat() {
Exception e = expectThrows(
ElasticsearchParseException.class,
() -> DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-{now/d{}}>"))
() -> DateMathExpressionResolver.resolveExpression("<.marvel-{now/d{}}>", getTime)
);
assertThat(e.getMessage(), containsString("invalid dynamic name expression"));
assertThat(e.getMessage(), containsString("missing date format"));
}

public void testExpressionInvalidOpenEnded() throws Exception {
public void testExpressionInvalidOpenEnded() {
Exception e = expectThrows(
ElasticsearchParseException.class,
() -> DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-{now/d>"))
() -> DateMathExpressionResolver.resolveExpression("<.marvel-{now/d>", getTime)
);
assertThat(e.getMessage(), containsString("invalid dynamic name expression"));
assertThat(e.getMessage(), containsString("date math placeholder is open ended"));
}

static ZonedDateTime dateFromMillis(long millis) {
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC);
}

static String formatDate(String pattern, ZonedDateTime zonedDateTime) {
DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern(pattern, Locale.ROOT);
return dateFormatter.format(zonedDateTime);
}
}
Loading

0 comments on commit 7dc2cc6

Please sign in to comment.