Skip to content

Commit

Permalink
Consistent Indices API: Returning empty json instead of 404
Browse files Browse the repository at this point in the history
If an index exists, but a single warmer/setting/alias/type is missing
return an empty JSON instead of an 404

Relates elastic#4071
  • Loading branch information
spinscale committed Jan 14, 2014
1 parent 5b89cfd commit 474f9e5
Show file tree
Hide file tree
Showing 12 changed files with 147 additions and 19 deletions.
3 changes: 2 additions & 1 deletion rest-api-spec/test/indices.delete_alias/10_basic.yaml
Expand Up @@ -25,7 +25,8 @@
name: testali

- do:
catch: missing
indices.get_alias:
index: testind
name: testali

- match: { '': {}}
30 changes: 30 additions & 0 deletions rest-api-spec/test/indices.get_alias/10_basic.yaml
Expand Up @@ -94,3 +94,33 @@ setup:
- match: {test_index.aliases.test_alias: {}}
- match: {test_index.aliases.test_blias: {}}
- match: {test_index_2.aliases.test_alias_2: {}}

---
"Getting a non-existent alias on an existing index should return an empty body":

- do:
indices.get_alias:
index: test_index
name: non-existent

- match: { '': {}}

---
"Getting an existent and non-existent alias should return the existent and no data about the non-existent alias":

- do:
indices.get_alias:
index: test_index
name: test_alias,non-existent

- match: {test_index.aliases.test_alias: {}}
- is_false: test_index.alias.non-existent

---
"Getting warmer on an non-existent index should return 404":

- do:
catch: missing
indices.get_alias:
index: non-existent
name: foo
30 changes: 30 additions & 0 deletions rest-api-spec/test/indices.get_mapping/10_basic.yaml
Expand Up @@ -94,3 +94,33 @@ setup:
- match: {test_idx.test_type.properties.text.type: string}
- match: {test_idx.test_type.properties.text.analyzer: keyword}

---
"Getting a non-existent mapping on an existing index should return an empty body":

- do:
indices.get_mapping:
index: test_index
type: non-existent

# match: {} does not work, so I just make sure the response is empty and not a 404
- is_false: test_index

---
"Getting an existent and non-existent mapping type should return the existing type and no data about the non-existent type":

- do:
indices.get_mapping:
index: test_index
type: test_type,non-existent

- match: {test_index.test_type.properties.text.type: string}
- is_false: test_index.non-existent

---
"Getting mapping on an non-existent index should return 404":

- do:
catch: missing
indices.get_mapping:
index: non-existent
type: whatever
6 changes: 3 additions & 3 deletions rest-api-spec/test/indices.get_mapping/20_missing_type.yaml
@@ -1,5 +1,5 @@
---
"Raise 404 when type doesn't exist":
"Return empty response when type doesn't exist":
- do:
indices.create:
index: test_index
Expand All @@ -12,8 +12,8 @@
analyzer: whitespace

- do:
catch: missing
indices.get_mapping:
index: test_index
type: not_test_type


- match: { '': {}}
30 changes: 30 additions & 0 deletions rest-api-spec/test/indices.get_settings/10_basic.yaml
Expand Up @@ -94,3 +94,33 @@ setup:
- is_false: test-index-2.settings.index.number_of_shards
- match: { test-index-2.settings.index.refresh_interval: "10m" }

---
"Getting a non-existent setting on an existing index should return an empty body":

- do:
indices.get_settings:
index: test-index
name: non-existent

- match: { '': {}}

---
"Getting an existent and non-existent setting should return the existent and no data about the non-existing setting":

- do:
indices.get_settings:
index: test-index
name: index.number_of_replicas,non-existent

- match: { test-index.settings.index.number_of_replicas: "3" }
- is_false: test-index.settings.non-existent

---
"Getting setting on an non-existent index should return 404":

- do:
catch: missing
indices.get_settings:
index: non-existent
name: foo

39 changes: 32 additions & 7 deletions rest-api-spec/test/indices.put_warmer/10_basic.yaml
Expand Up @@ -12,12 +12,6 @@ setup:
cluster.health:
wait_for_status: yellow

- do:
catch: missing
indices.get_warmer:
index: test_index
name: test_warmer

- do:
indices.put_warmer:
index: test_idx
Expand Down Expand Up @@ -49,10 +43,11 @@ setup:
name: test_warmer

- do:
catch: missing
indices.get_warmer:
index: test_index
name: test_warmer
# match: {} does not work, so I just make sure the response is empty and not a 404
- is_false: test_index

---
"Getting warmers for several indices should work using *":
Expand Down Expand Up @@ -98,3 +93,33 @@ setup:
- match: {test_index.warmers.test_warmer.source.query.match_all: {}}
- match: {test_idx.warmers.test_warmer2.source.query.match_all: {}}

---
"Getting a non-existent warmer on an existing index should return an empty body":

- do:
indices.get_warmer:
index: test_index
name: non-existent

- match: { '': {}}

---
"Getting an existent and non-existent warmer should return the existent and no data about the non-existent warmer":

- do:
indices.get_warmer:
index: test_index
name: test_warmer,non-existent

- match: {test_index.warmers.test_warmer.source.query.match_all: {}}
- is_false: test_index.warmers.non-existent

---
"Getting warmer on an non-existent index should return 404":

- do:
catch: missing
indices.get_warmer:
index: non-existent
name: foo

Expand Up @@ -368,9 +368,8 @@ public ImmutableOpenMap<String, ImmutableOpenMap<String, MappingMetaData>> findM
filteredMappings.put(cursor.key, cursor.value);
}
}
if (!filteredMappings.isEmpty()) {
indexMapBuilder.put(index, filteredMappings.build());
}
// returning empty mappings help to differentiate between non-existing and indices without mapping for this type
indexMapBuilder.put(index, filteredMappings.build());
}
}
return indexMapBuilder.build();
Expand Down
Expand Up @@ -68,7 +68,11 @@ public void handleRequest(final RestRequest request, final RestChannel channel)
public void onResponse(GetAliasesResponse response) {
try {
XContentBuilder builder = RestXContentBuilder.restContentBuilder(request);
if (response.getAliases().isEmpty()) {
// empty body, if indices were specified but no aliases were
if (indices.length > 0 && response.getAliases().isEmpty()) {
channel.sendResponse(new XContentRestResponse(request, OK, RestXContentBuilder.emptyBuilder(request)));
return;
} else if (response.getAliases().isEmpty()) {
String message = String.format(Locale.ROOT, "alias [%s] missing", toNamesString(getAliasesRequest.aliases()));
builder.startObject()
.field("error", message)
Expand Down
Expand Up @@ -89,6 +89,9 @@ public void onResponse(GetMappingsResponse response) {
}

for (ObjectObjectCursor<String, ImmutableOpenMap<String, MappingMetaData>> indexEntry : mappingsByIndex) {
if (indexEntry.value.size() == 0) {
continue;
}
builder.startObject(indexEntry.key, XContentBuilder.FieldCaseConversion.NONE);
for (ObjectObjectCursor<String, MappingMetaData> typeEntry : indexEntry.value) {
builder.field(typeEntry.key);
Expand Down
Expand Up @@ -62,19 +62,21 @@ public void handleRequest(final RestRequest request, final RestChannel channel)
@Override
public void onResponse(GetSettingsResponse getSettingsResponse) {
try {
boolean foundAny = false;
XContentBuilder builder = RestXContentBuilder.restContentBuilder(request);
builder.startObject();
for (ObjectObjectCursor<String, Settings> cursor : getSettingsResponse.getIndexToSettings()) {
// no settings, jump over it to shorten the response data
if (cursor.value.getAsMap().size() == 0) {
continue;
}
builder.startObject(cursor.key, XContentBuilder.FieldCaseConversion.NONE);
foundAny = true;
builder.startObject(Fields.SETTINGS);
cursor.value.toXContent(builder, request);
builder.endObject();
builder.endObject();
}
builder.endObject();
channel.sendResponse(new XContentRestResponse(request, foundAny ? OK : NOT_FOUND, builder));
channel.sendResponse(new XContentRestResponse(request, OK, builder));
} catch (IOException e) {
onFailure(e);
}
Expand Down
Expand Up @@ -71,7 +71,7 @@ public void handleRequest(final RestRequest request, final RestChannel channel)
public void onResponse(GetWarmersResponse response) {
try {
if (indices.length > 0 && response.warmers().isEmpty()) {
channel.sendResponse(new XContentThrowableRestResponse(request, new IndexMissingException(new Index(indices[0]))));
channel.sendResponse(new XContentRestResponse(request, OK, RestXContentBuilder.emptyBuilder(request)));
return;
}

Expand Down
Expand Up @@ -67,6 +67,10 @@ public static XContentBuilder restContentBuilder(RestRequest request, @Nullable
return builder;
}

public static XContentBuilder emptyBuilder(RestRequest request) throws IOException {
return restContentBuilder(request, request.hasContent() ? request.content() : null).startObject().endObject();
}

/**
* Directly writes the source to the output builder
*/
Expand Down

0 comments on commit 474f9e5

Please sign in to comment.