Skip to content

Commit 8526d17

Browse files
gmaroulielasticmachinejbaiera
authored and
alexey-ivanov-es
committed
IndexNameExpressionResolver refactoring (elastic#116085)
* 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]>
1 parent 19ddd20 commit 8526d17

File tree

6 files changed

+677
-863
lines changed

6 files changed

+677
-863
lines changed

‎server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java‎

Lines changed: 446 additions & 389 deletions
Large diffs are not rendered by default.

‎server/src/test/java/org/elasticsearch/cluster/metadata/DateMathExpressionResolverTests.java‎

Lines changed: 64 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -10,163 +10,90 @@
1010
package org.elasticsearch.cluster.metadata;
1111

1212
import org.elasticsearch.ElasticsearchParseException;
13-
import org.elasticsearch.action.support.IndicesOptions;
14-
import org.elasticsearch.cluster.ClusterName;
15-
import org.elasticsearch.cluster.ClusterState;
16-
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.Context;
1713
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.DateMathExpressionResolver;
18-
import org.elasticsearch.indices.SystemIndices.SystemIndexAccessLevel;
1914
import org.elasticsearch.test.ESTestCase;
20-
import org.hamcrest.Matchers;
2115

2216
import java.time.Instant;
2317
import java.time.ZoneId;
2418
import java.time.ZoneOffset;
2519
import java.time.ZonedDateTime;
2620
import java.time.format.DateTimeFormatter;
27-
import java.util.ArrayList;
28-
import java.util.Arrays;
29-
import java.util.Collections;
30-
import java.util.List;
3121
import java.util.Locale;
22+
import java.util.function.LongSupplier;
3223

3324
import static org.hamcrest.Matchers.containsString;
3425
import static org.hamcrest.Matchers.equalTo;
3526

3627
public class DateMathExpressionResolverTests extends ESTestCase {
3728

38-
private final Context context = new Context(
39-
ClusterState.builder(new ClusterName("_name")).build(),
40-
IndicesOptions.strictExpand(),
41-
SystemIndexAccessLevel.NONE
42-
);
29+
private final long now = randomMillisUpToYear9999();
30+
private final LongSupplier getTime = () -> now;
4331

44-
private static ZonedDateTime dateFromMillis(long millis) {
45-
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC);
46-
}
32+
public void testNoDateMathExpression() {
33+
String expression = randomAlphaOfLength(10);
34+
assertThat(DateMathExpressionResolver.resolveExpression(expression, getTime), equalTo(expression));
4735

48-
private static String formatDate(String pattern, ZonedDateTime zonedDateTime) {
49-
DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern(pattern, Locale.ROOT);
50-
return dateFormatter.format(zonedDateTime);
36+
expression = "*";
37+
assertThat(DateMathExpressionResolver.resolveExpression(expression, getTime), equalTo(expression));
5138
}
5239

53-
public void testNormal() throws Exception {
54-
int numIndexExpressions = randomIntBetween(1, 9);
55-
List<String> indexExpressions = new ArrayList<>(numIndexExpressions);
56-
for (int i = 0; i < numIndexExpressions; i++) {
57-
indexExpressions.add(randomAlphaOfLength(10));
58-
}
59-
List<String> result = DateMathExpressionResolver.resolve(context, indexExpressions);
60-
assertThat(result.size(), equalTo(indexExpressions.size()));
61-
for (int i = 0; i < indexExpressions.size(); i++) {
62-
assertThat(result.get(i), equalTo(indexExpressions.get(i)));
63-
}
64-
}
40+
public void testExpression() {
41+
String result = DateMathExpressionResolver.resolveExpression("<.marvel-{now}>", getTime);
42+
assertThat(result, equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));
6543

66-
public void testExpression() throws Exception {
67-
List<String> indexExpressions = Arrays.asList("<.marvel-{now}>", "<.watch_history-{now}>", "<logstash-{now}>");
68-
List<String> result = DateMathExpressionResolver.resolve(context, indexExpressions);
69-
assertThat(result.size(), equalTo(3));
70-
assertThat(result.get(0), equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
71-
assertThat(result.get(1), equalTo(".watch_history-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
72-
assertThat(result.get(2), equalTo("logstash-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
44+
result = DateMathExpressionResolver.resolveExpression("<.watch_history-{now}>", getTime);
45+
assertThat(result, equalTo(".watch_history-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));
46+
47+
result = DateMathExpressionResolver.resolveExpression("<logstash-{now}>", getTime);
48+
assertThat(result, equalTo("logstash-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));
7349
}
7450

7551
public void testExpressionWithWildcardAndExclusions() {
76-
List<String> indexExpressions = Arrays.asList(
77-
"<-before-inner-{now}>",
78-
"-<before-outer-{now}>",
79-
"<wild*card-{now}*>",
80-
"<-after-inner-{now}>",
81-
"-<after-outer-{now}>"
82-
);
83-
List<String> result = DateMathExpressionResolver.resolve(context, indexExpressions);
84-
assertThat(
85-
result,
86-
Matchers.contains(
87-
equalTo("-before-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
88-
equalTo("-<before-outer-{now}>"), // doesn't evaluate because it doesn't start with "<" and it is not an exclusion
89-
equalTo("wild*card-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime())) + "*"),
90-
equalTo("-after-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
91-
equalTo("-after-outer-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime())))
92-
)
93-
);
94-
Context noWildcardExpandContext = new Context(
95-
ClusterState.builder(new ClusterName("_name")).build(),
96-
IndicesOptions.strictSingleIndexNoExpandForbidClosed(),
97-
SystemIndexAccessLevel.NONE
98-
);
99-
result = DateMathExpressionResolver.resolve(noWildcardExpandContext, indexExpressions);
100-
assertThat(
101-
result,
102-
Matchers.contains(
103-
equalTo("-before-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
104-
// doesn't evaluate because it doesn't start with "<" and there can't be exclusions without wildcard expansion
105-
equalTo("-<before-outer-{now}>"),
106-
equalTo("wild*card-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime())) + "*"),
107-
equalTo("-after-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
108-
// doesn't evaluate because it doesn't start with "<" and there can't be exclusions without wildcard expansion
109-
equalTo("-<after-outer-{now}>")
110-
)
111-
);
112-
}
52+
String result = DateMathExpressionResolver.resolveExpression("<-before-inner-{now}>", getTime);
53+
assertThat(result, equalTo("-before-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));
54+
55+
result = DateMathExpressionResolver.resolveExpression("<wild*card-{now}*>", getTime);
56+
assertThat(result, equalTo("wild*card-" + formatDate("uuuu.MM.dd", dateFromMillis(now)) + "*"));
57+
58+
result = DateMathExpressionResolver.resolveExpression("<-after-inner-{now}>", getTime);
59+
assertThat(result, equalTo("-after-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));
11360

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

119-
public void testExpression_Static() throws Exception {
120-
List<String> result = DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-test>"));
121-
assertThat(result.size(), equalTo(1));
122-
assertThat(result.get(0), equalTo(".marvel-test"));
63+
public void testExpression_Static() {
64+
String result = DateMathExpressionResolver.resolveExpression("<.marvel-test>", getTime);
65+
assertThat(result, equalTo(".marvel-test"));
12366
}
12467

125-
public void testExpression_MultiParts() throws Exception {
126-
List<String> result = DateMathExpressionResolver.resolve(context, Arrays.asList("<.text1-{now/d}-text2-{now/M}>"));
127-
assertThat(result.size(), equalTo(1));
68+
public void testExpression_MultiParts() {
69+
String result = DateMathExpressionResolver.resolveExpression("<.text1-{now/d}-text2-{now/M}>", getTime);
12870
assertThat(
129-
result.get(0),
71+
result,
13072
equalTo(
13173
".text1-"
132-
+ formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))
74+
+ formatDate("uuuu.MM.dd", dateFromMillis(now))
13375
+ "-text2-"
134-
+ formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()).withDayOfMonth(1))
76+
+ formatDate("uuuu.MM.dd", dateFromMillis(now).withDayOfMonth(1))
13577
)
13678
);
13779
}
13880

139-
public void testExpression_CustomFormat() throws Exception {
140-
List<String> results = DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-{now/d{yyyy.MM.dd}}>"));
141-
assertThat(results.size(), equalTo(1));
142-
assertThat(results.get(0), equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
143-
}
144-
145-
public void testExpression_EscapeStatic() throws Exception {
146-
List<String> result = DateMathExpressionResolver.resolve(context, Arrays.asList("<.mar\\{v\\}el-{now/d}>"));
147-
assertThat(result.size(), equalTo(1));
148-
assertThat(result.get(0), equalTo(".mar{v}el-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
81+
public void testExpression_CustomFormat() {
82+
String result = DateMathExpressionResolver.resolveExpression("<.marvel-{now/d{yyyy.MM.dd}}>", getTime);
83+
assertThat(result, equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(now))));
14984
}
15085

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

157-
public void testExpression_MixedArray() throws Exception {
158-
List<String> result = DateMathExpressionResolver.resolve(
159-
context,
160-
Arrays.asList("name1", "<.marvel-{now/d}>", "name2", "<.logstash-{now/M{uuuu.MM}}>")
161-
);
162-
assertThat(result.size(), equalTo(4));
163-
assertThat(result.get(0), equalTo("name1"));
164-
assertThat(result.get(1), equalTo(".marvel-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
165-
assertThat(result.get(2), equalTo("name2"));
166-
assertThat(result.get(3), equalTo(".logstash-" + formatDate("uuuu.MM", dateFromMillis(context.getStartTime()).withDayOfMonth(1))));
91+
public void testExpression_EscapeDateFormat() {
92+
String result = DateMathExpressionResolver.resolveExpression("<.marvel-{now/d{'\\{year\\}'yyyy}}>", getTime);
93+
assertThat(result, equalTo(".marvel-" + formatDate("'{year}'yyyy", dateFromMillis(now))));
16794
}
16895

169-
public void testExpression_CustomTimeZoneInIndexName() throws Exception {
96+
public void testExpression_CustomTimeZoneInIndexName() {
17097
ZoneId timeZone;
17198
int hoursOffset;
17299
int minutesOffset = 0;
@@ -194,57 +121,57 @@ public void testExpression_CustomTimeZoneInIndexName() throws Exception {
194121
// rounding to today 00:00
195122
now = ZonedDateTime.now(ZoneOffset.UTC).withHour(0).withMinute(0).withSecond(0);
196123
}
197-
Context context = new Context(
198-
this.context.getState(),
199-
this.context.getOptions(),
200-
now.toInstant().toEpochMilli(),
201-
SystemIndexAccessLevel.NONE,
202-
name -> false,
203-
name -> false
204-
);
205-
List<String> results = DateMathExpressionResolver.resolve(
206-
context,
207-
Arrays.asList("<.marvel-{now/d{yyyy.MM.dd|" + timeZone.getId() + "}}>")
124+
125+
String result = DateMathExpressionResolver.resolveExpression(
126+
"<.marvel-{now/d{yyyy.MM.dd|" + timeZone.getId() + "}}>",
127+
() -> now.toInstant().toEpochMilli()
208128
);
209-
assertThat(results.size(), equalTo(1));
210-
logger.info("timezone: [{}], now [{}], name: [{}]", timeZone, now, results.get(0));
211-
assertThat(results.get(0), equalTo(".marvel-" + formatDate("uuuu.MM.dd", now.withZoneSameInstant(timeZone))));
129+
logger.info("timezone: [{}], now [{}], name: [{}]", timeZone, now, result);
130+
assertThat(result, equalTo(".marvel-" + formatDate("uuuu.MM.dd", now.withZoneSameInstant(timeZone))));
212131
}
213132

214-
public void testExpressionInvalidUnescaped() throws Exception {
133+
public void testExpressionInvalidUnescaped() {
215134
Exception e = expectThrows(
216135
ElasticsearchParseException.class,
217-
() -> DateMathExpressionResolver.resolve(context, Arrays.asList("<.mar}vel-{now/d}>"))
136+
() -> DateMathExpressionResolver.resolveExpression("<.mar}vel-{now/d}>", getTime)
218137
);
219138
assertThat(e.getMessage(), containsString("invalid dynamic name expression"));
220139
assertThat(e.getMessage(), containsString("invalid character at position ["));
221140
}
222141

223-
public void testExpressionInvalidDateMathFormat() throws Exception {
142+
public void testExpressionInvalidDateMathFormat() {
224143
Exception e = expectThrows(
225144
ElasticsearchParseException.class,
226-
() -> DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-{now/d{}>"))
145+
() -> DateMathExpressionResolver.resolveExpression("<.marvel-{now/d{}>", getTime)
227146
);
228147
assertThat(e.getMessage(), containsString("invalid dynamic name expression"));
229148
assertThat(e.getMessage(), containsString("date math placeholder is open ended"));
230149
}
231150

232-
public void testExpressionInvalidEmptyDateMathFormat() throws Exception {
151+
public void testExpressionInvalidEmptyDateMathFormat() {
233152
Exception e = expectThrows(
234153
ElasticsearchParseException.class,
235-
() -> DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-{now/d{}}>"))
154+
() -> DateMathExpressionResolver.resolveExpression("<.marvel-{now/d{}}>", getTime)
236155
);
237156
assertThat(e.getMessage(), containsString("invalid dynamic name expression"));
238157
assertThat(e.getMessage(), containsString("missing date format"));
239158
}
240159

241-
public void testExpressionInvalidOpenEnded() throws Exception {
160+
public void testExpressionInvalidOpenEnded() {
242161
Exception e = expectThrows(
243162
ElasticsearchParseException.class,
244-
() -> DateMathExpressionResolver.resolve(context, Arrays.asList("<.marvel-{now/d>"))
163+
() -> DateMathExpressionResolver.resolveExpression("<.marvel-{now/d>", getTime)
245164
);
246165
assertThat(e.getMessage(), containsString("invalid dynamic name expression"));
247166
assertThat(e.getMessage(), containsString("date math placeholder is open ended"));
248167
}
249168

169+
static ZonedDateTime dateFromMillis(long millis) {
170+
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC);
171+
}
172+
173+
static String formatDate(String pattern, ZonedDateTime zonedDateTime) {
174+
DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern(pattern, Locale.ROOT);
175+
return dateFormatter.format(zonedDateTime);
176+
}
250177
}

0 commit comments

Comments
 (0)