Check if root mapping is actually valid #6093
Conversation
| private void checkRootKeysValid(Map<String, Object> root) { | ||
| for (String key : root.keySet()) { | ||
| if (!rootTypeParsers.containsKey(key) && !key.equals("properties")) { | ||
| throw new MapperParsingException("Got unrecognized key "+ key + " in root of mapping for type "); |
There was a problem hiding this comment.
Maybe enclose key with brackets to make clearer where it starts and where it ends?
throw new MapperParsingException("Got unrecognized key ["+ key + "] in root of mapping for type");
|
I added some commits to implement the comments. I am unsure about the RootObjectMapper properties ("dynamic_templates", ...). I added two commits: One just checks for all the field names that I found in the RootObjectMapper code ("check rootObjectMapper keys also") and another that converts the Strings that are checked in there to ParseFields ("make RootObjectMapper keys ParseFields") but I am unsure if this makes the code much better. |
|
There are still options missing, like "analyzer", "index_analyzer",... |
When a mapping is declared and the type is known from the uri then the type can be skipped in the body (see elastic#4483). However, there was no check if the given keys actually make a valid mapping. closes elastic#5864
|
I pushed a new version, the old one can still be found at: https://github.com/brwe/elasticsearch/tree/issue-5864-v1 Instead of collecting all keys that can be in a root object mapping I split the root object and object parsing into properties that can be in the root object, those that can't and those that can be in both. In the root object parsing, I remove each found key from the mapping and then check if the mapping is empty after parsing. This has the nice side effect that now also mappings of the form |
| docBuilder.put(typeParser.parse(fieldName, (Map<String, Object>) fieldNode, parserContext)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ImmutableMap<String, Object> attributes = ImmutableMap.of(); | ||
| if (mapping.containsKey("_meta")) { | ||
| attributes = ImmutableMap.copyOf((Map<String, Object>) mapping.get("_meta")); |
There was a problem hiding this comment.
you can replace the call to get with a call to remove to not need to remove the _meta key on the next line
|
Apart from minor style issues this looks good to me. |
|
thanks for the review, I added some commits |
|
LGTM! |
include_in_all can also be set on type level (root object). This fixes a regression introduced in elastic#6093 closes elastic#6304
clintongormley
added
>enhancement
:Search/Mapping
include_in_all can also be set on type level (root object). This fixes a regression introduced in elastic#6093 closes elastic#6304
When a mapping is declared and the type is known from the uri
then the type can be skipped in the body (see #4483). However,
there was no check if the given keys actually make a valid mapping.
closes #5864