Skip to content

Commit 9bddef7

Browse files
JeremyDahlgrenJeremyDahlgren
committed
Address code review comments, add unit test case
1 parent 7b00f2c commit 9bddef7

File tree

2 files changed

+78
-55
lines changed

2 files changed

+78
-55
lines changed

‎server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetAliasesAction.java‎

Lines changed: 56 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.elasticsearch.xcontent.XContentBuilder;
3737

3838
import java.io.IOException;
39+
import java.util.Collections;
3940
import java.util.HashSet;
4041
import java.util.List;
4142
import java.util.Locale;
@@ -84,67 +85,21 @@ static RestResponse buildRestResponse(
8485
) throws Exception {
8586
final Set<String> indicesToDisplay = new HashSet<>();
8687
final Set<String> returnedAliasNames = new HashSet<>();
87-
if (aliasesExplicitlyRequested || requestedAliases.length > 0) {
88-
for (final Map.Entry<String, List<AliasMetadata>> cursor : responseAliasMap.entrySet()) {
89-
final var aliases = cursor.getValue();
90-
if (aliases.isEmpty() == false) {
91-
if (aliasesExplicitlyRequested) {
92-
// only display indices that have aliases
93-
indicesToDisplay.add(cursor.getKey());
94-
}
95-
if (requestedAliases.length > 0) {
96-
for (final AliasMetadata aliasMetadata : aliases) {
97-
returnedAliasNames.add(aliasMetadata.alias());
98-
}
99-
}
100-
}
101-
}
102-
}
103-
if (requestedAliases.length > 0) {
88+
if (aliasesExplicitlyRequested) {
89+
responseAliasMap.entrySet().stream().filter(entry -> entry.getValue().isEmpty() == false).forEach(entry -> {
90+
// only display indices that have aliases
91+
indicesToDisplay.add(entry.getKey());
92+
entry.getValue().forEach(aliasMetadata -> returnedAliasNames.add(aliasMetadata.alias()));
93+
});
94+
10495
dataStreamAliases.entrySet()
10596
.stream()
10697
.flatMap(entry -> entry.getValue().stream())
10798
.forEach(dataStreamAlias -> returnedAliasNames.add(dataStreamAlias.getName()));
10899
}
109100

110-
// compute explicitly requested aliases that have are not returned in the result
111-
final SortedSet<String> missingAliases = new TreeSet<>();
112-
// first wildcard index, leading "-" as an alias name after this index means
113-
// that it is an exclusion
114-
int firstWildcardIndex = requestedAliases.length;
115-
for (int i = 0; i < requestedAliases.length; i++) {
116-
if (Regex.isSimpleMatchPattern(requestedAliases[i])) {
117-
firstWildcardIndex = i;
118-
break;
119-
}
120-
}
121-
for (int i = 0; i < requestedAliases.length; i++) {
122-
if (Metadata.ALL.equals(requestedAliases[i])
123-
|| Regex.isSimpleMatchPattern(requestedAliases[i])
124-
|| (i > firstWildcardIndex && requestedAliases[i].charAt(0) == '-')) {
125-
// only explicitly requested aliases will be called out as missing (404)
126-
continue;
127-
}
128-
// check if aliases[i] is subsequently excluded
129-
int j = Math.max(i + 1, firstWildcardIndex);
130-
for (; j < requestedAliases.length; j++) {
131-
if (requestedAliases[j].charAt(0) == '-') {
132-
// this is an exclude pattern
133-
if (Regex.simpleMatch(requestedAliases[j].substring(1), requestedAliases[i])
134-
|| Metadata.ALL.equals(requestedAliases[j].substring(1))) {
135-
// aliases[i] is excluded by aliases[j]
136-
break;
137-
}
138-
}
139-
}
140-
if (j == requestedAliases.length) {
141-
// explicitly requested aliases[i] is not excluded by any subsequent "-" wildcard in expression
142-
if (false == returnedAliasNames.contains(requestedAliases[i])) {
143-
// aliases[i] is not in the result set
144-
missingAliases.add(requestedAliases[i]);
145-
}
146-
}
147-
}
101+
// compute explicitly requested aliases that would not be returned in the result
102+
final var missingAliases = computeMissingAliases(requestedAliases, returnedAliasNames);
148103

149104
final RestStatus status;
150105
builder.startObject();
@@ -248,4 +203,50 @@ public RestResponse buildResponse(GetAliasesResponse response, XContentBuilder b
248203
});
249204
}
250205

206+
private static SortedSet<String> computeMissingAliases(String[] requestedAliases, Set<String> returnedAliasNames) {
207+
if (requestedAliases.length == 0) {
208+
return Collections.emptySortedSet();
209+
}
210+
211+
final var missingAliases = new TreeSet<String>();
212+
213+
// first wildcard index, leading "-" as an alias name after this index means
214+
// that it is an exclusion
215+
int firstWildcardIndex = requestedAliases.length;
216+
for (int i = 0; i < requestedAliases.length; i++) {
217+
if (Regex.isSimpleMatchPattern(requestedAliases[i])) {
218+
firstWildcardIndex = i;
219+
break;
220+
}
221+
}
222+
for (int i = 0; i < requestedAliases.length; i++) {
223+
if (Metadata.ALL.equals(requestedAliases[i])
224+
|| Regex.isSimpleMatchPattern(requestedAliases[i])
225+
|| (i > firstWildcardIndex && requestedAliases[i].charAt(0) == '-')) {
226+
// only explicitly requested aliases will be called out as missing (404)
227+
continue;
228+
}
229+
// check if aliases[i] is subsequently excluded
230+
int j = Math.max(i + 1, firstWildcardIndex);
231+
for (; j < requestedAliases.length; j++) {
232+
if (requestedAliases[j].charAt(0) == '-') {
233+
// this is an exclude pattern
234+
if (Regex.simpleMatch(requestedAliases[j].substring(1), requestedAliases[i])
235+
|| Metadata.ALL.equals(requestedAliases[j].substring(1))) {
236+
// aliases[i] is excluded by aliases[j]
237+
break;
238+
}
239+
}
240+
}
241+
if (j == requestedAliases.length) {
242+
// explicitly requested aliases[i] is not excluded by any subsequent "-" wildcard in expression
243+
if (false == returnedAliasNames.contains(requestedAliases[i])) {
244+
// aliases[i] is not in the result set
245+
missingAliases.add(requestedAliases[i]);
246+
}
247+
}
248+
}
249+
250+
return missingAliases;
251+
}
251252
}

‎server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetAliasesActionTests.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.cluster.metadata.AliasMetadata;
1313
import org.elasticsearch.rest.RestResponse;
1414
import org.elasticsearch.test.ESTestCase;
15+
import org.elasticsearch.test.rest.FakeRestRequest;
1516
import org.elasticsearch.xcontent.XContentBuilder;
1617
import org.elasticsearch.xcontent.XContentFactory;
1718
import org.elasticsearch.xcontent.XContentType;
@@ -51,6 +52,27 @@ public void testBareRequest() throws Exception {
5152
assertThat(restResponse.content().utf8ToString(), equalTo("{\"index\":{\"aliases\":{\"foo\":{},\"foobar\":{}}}}"));
5253
}
5354

55+
public void testNameParamWithAllValue() throws Exception {
56+
final XContentBuilder xContentBuilder = XContentFactory.contentBuilder(XContentType.JSON);
57+
final var req = new FakeRestRequest.Builder(xContentRegistry()).withParams(Map.of("name", "_all")).build();
58+
final RestResponse restResponse = RestGetAliasesAction.buildRestResponse(
59+
req.hasParam("name"),
60+
req.paramAsStringArrayOrEmptyIfAll("name"),
61+
Map.of(
62+
"index",
63+
Arrays.asList(AliasMetadata.builder("foo").build(), AliasMetadata.builder("foobar").build()),
64+
"index2",
65+
List.of()
66+
),
67+
Map.of(),
68+
xContentBuilder
69+
);
70+
assertThat(restResponse.status(), equalTo(OK));
71+
assertThat(restResponse.contentType(), equalTo("application/json"));
72+
// Verify we don't get "index2" since it has no aliases.
73+
assertThat(restResponse.content().utf8ToString(), equalTo("{\"index\":{\"aliases\":{\"foo\":{},\"foobar\":{}}}}"));
74+
}
75+
5476
public void testSimpleAliasWildcardMatchingNothing() throws Exception {
5577
final XContentBuilder xContentBuilder = XContentFactory.contentBuilder(XContentType.JSON);
5678
final RestResponse restResponse = RestGetAliasesAction.buildRestResponse(

0 commit comments

Comments
 (0)