Skip to content

Commit

Permalink
Fail early if an index request has the wrong type (#77417)
Browse files Browse the repository at this point in the history
If an index request contains a type that is different to the type defined on the
index, we fail the request with an error. However, this check happens fairly
late on in the process, and if the index request has generated dynamic updates
that conflict with the existing mappings on the index then merge errors will
get thrown instead. This is confusing to users, as it is not at all obvious why
these conflicting mappings have been generated.

This commit adds a check for valid types early in the indexing process, so that
requests with invalid types will be failed with a more informative error message.

Fixes #77411
  • Loading branch information
romseygeek committed Sep 9, 2021
1 parent 08f8b80 commit 0614de2
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,11 @@
body: { foo: bar }

- do:
catch: bad_request
catch: /Invalid type|final mapping would have more than 1 type/
delete:
index: index
type: some_random_type
id: 1

- match: { error.root_cause.0.reason: "/Rejecting.mapping.update.to.\\[index\\].as.the.final.mapping.would.have.more.than.1.type.*/" }

- do:
delete:
index: index
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
type: keyword

- do:
catch: /the final mapping would have more than 1 type/
catch: /Invalid type|final mapping would have more than 1 type/
index:
index: test-1
type: my_type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,4 +192,32 @@ public void testDeleteIndexWhileIndexing() throws Exception {
}
}

public void testIndexWithWrongTypeFailsEarly() {
client().admin().indices().prepareCreate("bulk_with_wrong_type")
.addMapping("_doc", "foo", "type=keyword")
.get();

Exception e = expectThrows(IllegalArgumentException.class,
() -> client().prepareIndex("bulk_with_wrong_type", "wrongtype")
.setSource("{\"foo\":\"value\"}", XContentType.JSON)
.get());
assertEquals("Invalid type: expecting [_doc] but got [wrongtype]", e.getMessage());

BulkResponse response = client().prepareBulk()
.add(client().prepareIndex("bulk_with_wrong_type", "_doc")
.setSource("{\"foo\":\"value\"}", XContentType.JSON))
.add(client().prepareIndex("bulk_with_wrong_type", "type")
.setSource("{\"foo\":\"value\"}", XContentType.JSON))
.get();
assertTrue(response.hasFailures());
assertThat(response.buildFailureMessage(), containsString("Invalid type: expecting [_doc] but got [type]"));

client().prepareIndex("bulk_with_wrong_type", "_doc", "1")
.setSource("{\"foo\":\"value\"}", XContentType.JSON)
.get();
e = expectThrows(IllegalArgumentException.class,
() -> client().prepareDelete("bulk_with_wrong_type", "type", "1").get());
assertEquals("Invalid type: expecting [_doc] but got [type]", e.getMessage());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,18 @@ public DocumentMapper documentMapper(String type) {
return null;
}

/**
* Check that the resolved type can be used for indexing or deletions
*/
public void validateType(String type) {
if (mapper == null) {
return;
}
if (type.equals(mapper.type()) == false) {
throw new IllegalArgumentException("Invalid type: expecting [" + mapper.type() + "] but got [" + type + "]");
}
}

/**
* Returns {@code true} if the given {@code mappingSource} includes a type
* as a top-level object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,7 @@ public static Engine.Index prepareIndex(MapperService mapperService, String type
long startTime = System.nanoTime();
assert source.dynamicTemplates().isEmpty() || origin == Engine.Operation.Origin.PRIMARY :
"dynamic_templates parameter can only be associated with primary operations";
mapperService.validateType(type);
DocumentMapper documentMapper = mapperService.documentMapper(type);
Mapping mapping = null;
if (documentMapper == null) {
Expand Down Expand Up @@ -987,6 +988,7 @@ private Engine.DeleteResult applyDeleteOperation(Engine engine, long seqNo, long
// TODO: clean this up when types are gone
String resolvedType = mapperService.resolveDocumentType(type);
try {
mapperService.validateType(resolvedType);
DocumentMapper documentMapper = mapperService.documentMapper(resolvedType);
if (documentMapper == null) {
documentMapper = DocumentMapper.createEmpty(resolvedType, mapperService);
Expand Down

0 comments on commit 0614de2

Please sign in to comment.