Remove type parameter from CreateIndexRequest.mapping(type, XContentBuilder)
#50586
Conversation
romseygeek
added
:Search Foundations/Mapping
|
Pinging @elastic/es-search (:Search/Mapping) |
|
Pinging @elastic/es-distributed (:Distributed/CRUD) |
| .startObject() | ||
| .startObject("test") | ||
| .startObject("_doc") |
There was a problem hiding this comment.
To double-check I understand -- we could already remove the mentions to _doc in mapping definitions, but we'll postpone that for another PR? This approach makes sense to me, as it helps keep the current PR focused.
There was a problem hiding this comment.
That's correct - there's also still a bit of inconsistency as to when things need to be wrapped and when not, which this PR moves us one more step towards resolving.
| * @param source The mapping source | ||
| */ | ||
| public CreateIndexRequest mapping(String type, XContentBuilder source) { | ||
| return mapping(type, BytesReference.bytes(source), source.contentType()); | ||
| public CreateIndexRequest mapping(XContentBuilder source) { |
There was a problem hiding this comment.
For consistency with the changes in CreateIndexRequestBuilder, it would be great if we also removed the type parameter from public CreateIndexRequest mapping(String type, Map<String, ?> source), and converted as many callers as possible to use the new signature. Here's a suggestion for how to do it:
public CreateIndexRequest mapping(Map<String, ?> source) {
return mapping(MapperService.SINGLE_MAPPING_NAME, source);
}
private CreateIndexRequest mapping(String type, Map<String, ?> source) {
// wrap it in a type map if its not
if (source.size() != 1 || !source.containsKey(type)) {
source = Map.of(MapperService.SINGLE_MAPPING_NAME, source);
}
...
}
The private method is still needed to support CreateIndexRequest#source.
| @@ -291,7 +283,7 @@ else if (MapperService.SINGLE_MAPPING_NAME.equals(type) == false) { | |||
| * ("field1", "type=string,store=true"). | |||
| */ | |||
| public CreateIndexRequest mapping(String type, Object... source) { | |||
| mapping(type, PutMappingRequest.buildFromSimplifiedDef(type, source)); | |||
| mapping(PutMappingRequest.buildFromSimplifiedDef(MapperService.SINGLE_MAPPING_NAME, source)); | |||
There was a problem hiding this comment.
For context, what's the plan to remove these remaining methods that accept Object... source?
There was a problem hiding this comment.
My current plan is to have a new method, mappingFromSimplifiedDef(Objects... source), and remove the existing method, to make sure that we get compilation failures rather than silently treating the type parameter as part of the source.
…Builder)` (elastic#50586) This continues the removal of type parameters from CreateIndexRequest.mapping methods started in elastic#50419. Here the removed methods are almost entirely in test code, with the exception of a change to TransformIndex in the transform plugin. Relates to elastic#41059
This continues the removal of type parameters from
CreateIndexRequest.mappingmethods started in #50419. Here the removed methods are almost entirely in test
code, with the exception of a change to
TransformIndexin the transform plugin.Relates to #41059