Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Types removal - PUT and GET IndexTemplates. #35866

Closed
wants to merge 1 commit into from

Conversation

markharwood
Copy link
Contributor

Add support for the include_type_name flag.
For HLRC support PutIndexTemplateRequest now includes checks to change the toXContent format if it looks like custom doc types are used in mappings.

@markharwood markharwood added >enhancement WIP :Data Management/Indices APIs APIs to create and manage indices and templates v7.0.0 labels Nov 23, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@markharwood
Copy link
Contributor Author

jenkins test this

@markharwood markharwood force-pushed the fix/35190TemplatePut branch 3 times, most recently from 637bc3e to f7d218b Compare November 30, 2018 15:01
@markharwood
Copy link
Contributor Author

markharwood commented Nov 30, 2018

I've refactored PutIndexTemplateRequest as part of this PR so that we can expose it safely in HLRC.

Internally the mappings Map contains JSON strings keyed on the doc type.
These JSON strings are supposed to be of the form "mappings":{ "my_doc_type": { "properties":... and a method internalMapping is used to insert the choice of doc type into the string if it is missing from the provided content (it is assumed clients can omit this). I've kept this approach.
However, PITR has many public methods with different ways of passing mapping blobs (XContent, maps, byte arrays etc) and not all of these ran the provided content past the internalMappings storage function - some injected typeless content strings, untreated straight into the mappings map which violates the assumption that these internal mapping strings are always typed.

This was the set of inconsistent approaches used:

method param type upgrades mappings to internal typed format
mapping BytesReference N
mapping Map<String, Object> Y
mapping XContentBuilder N
mapping Object... source N
source BytesReference Y
source byte[] Y
source XContentBuilder Y
source Object... source Y
source Map<String, Object> Y

Several of these methods call each other internally. I refactored them so that they all ultimately call internalMappings to ensure a type is injected where missing as that is the internal storage format.
When we use PITR in HLRC and render to XContent we strip the type name if it is the default of "_doc".
Other parts of PITR assumed the type names were in the mapping content when parsing and I have deprecated these (methods called mapping or source) and introduced alternative sourceNoDocTypes and mappingNoDocTypes equivalents that assume there's no type name in the mapping content provided. These are used in the HLRC examples.
This may seem odd but the need for this is that while it is possible for internalMapping to detect typeless content and "upgrade" to typed it is not possible for source(Map<String, Object> templateSource) to detect already typed content (legal choices of doc type name can clash with mapping fields like properties). This method is delegated to a lot internally. Sadly, HLRC clients need to be explicit about whether the content is typed or untyped by the choice of method name they invoke.

@markharwood
Copy link
Contributor Author

jenkins test this

For HLRC support PutIndexTemplateRequest now includes checks to change the toXContent format if it looks like custom doc types are used.
Removed getTypedTemplate method that violated REST-api contract and added parameter to the GetIndexTemplatesRequest for the HLRC client to request typed templates
Removed doc types from HLRC documentation examples
PITR.toXContent() now strips type name from `mapping->_doc->properties` to make untyped REST requests with include_type_name=false
Tidied PutIndexTemplateRequest inconsistencies - some public methods ran mappings through the `internalMappings` validation and some didn’t. All do now.
Added new APIs to take mappings with no doc types embedded in mapping definition blobs which are apis we will document in HLRC. The legacy `source` and `mapping` apis that support type names in the blob are all marked as deprecated.
@markharwood
Copy link
Contributor Author

markharwood commented Jan 10, 2019

The outstanding work required here is that:

  1. We need an equivalent of Add an include_type_name option to 6.x. (#29453) #37147 to add 6.x support for include_type_name with index templates.
  2. Once 1) is committed I need to switch this PR's default to include_type_name=false (preferably referring to a constant for this policy recommended as part of Update the default for include_type_name to false. #37285 (comment)). This PR will then need a bunch of existing yaml test changes similar to those in Update the default for include_type_name to false. #37285 which makes these tests 6.x and 7.0 compatible by adding include_type_name=true to tests that use typed templates

@markharwood
Copy link
Contributor Author

This overlaps with #37210 so not doing any more on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates >enhancement v8.0.0-alpha1 WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants