Compat rest api #22
Open
Compat rest api #22
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Providing a REST infrastructure to handle compatible APIs. Rest APIs that were removed in 8 will be available only if an HTTP request had a compatible header. At the moment is is expecting an Accept header to contain application/vnd.elasticsearch+json;compatible-with=7. This might be changed though when elastic#52370 is resolved. The REST changes contain enriching RestRequest with a parameter Compatible-With. The value is taken from Accept header. See RestRequest creation. Used when deciding if a request to a compatible handler can be dispatched. See RestController. //TODO this at the moment can be simplified to only use a value from a header. However in rest layer we often have only access to Params, so headers won't be accessible when implementing some compatible code enriching XContentBuilder with a compatible version value. See AbstractRestChannel. Used when shape of a response is changed. see DocWriteResponse and GetResult MediaType parsing change - see XContentType. because we expect a different media type now application/vnd.elasticsearch+json;compatible-with=7 (when previously application/json or similar) testing changes - compat tests will have a compatible header set. see AbstractCompatRestTest.java Compatible rest handlers - based on RestIndexAction and RestGetAction. See modules/rest-compatibility. These extend already existing handlers and register them in a RestCompatPlugin under a typed paths. They will only be accessible when a compatible header is present (use of compatibilityRequired method from RestHandler interface) Without this PR 283 tests from 7.x are failing. - rest api spec tests only with this PR 228 tests are passing - with almost all tests from index/* package passed (missing 2 tests that require change for include_type_param, to be added in a new PR) relates elastic#51816
…4103) Adding validation of Accept And Content-Type headers. The idea is to verify combinations of presence and values of both headers depending if a request has a content, headers are versioned (type/vnd.elasticsearch+subtype;compatible-with) . It also changes the way media type is parsed (previously always assuming application as a type i.e application/json) It should expect different types like text - used in SQL Not adding a compatible headers for _cat api. These in order to return a default text do not expect Accept header (Content-type is not needed as content is not present) See elastic#53228 (comment) for more context
…astic#54197) Refactoring of the compatible infrastructure to allow registering multiple RestAction under the same path. It only extends the current mechanism which allowed registering RestAction under the same path but with different method. Now it also uses a version together with a method to find a matching RestAction This PR also provides a V7 compatible RestCreateIndexAction that needs include_Type_param and a different logic for parsing mapping from its body. fixed get/index tests CompatRestIT. test {yaml=get/21_stored_fields_with_types/Stored fields} CompatRestIT. test {yaml=get/71_source_filtering_with_types/Source filtering} CompatRestIT. test {yaml=index/70_mix_typeless_typeful/Index call that introduces new field mappings} CompatRestIT. test {yaml=index/70_mix_typeless_typeful/Index with typeless API on an index that has types} however the last one from get is still failing CompatRestIT. test {yaml=get/100_mix_typeless_typeful/GET with typeless API on an index that has relates elastic#54160
…PIs (elastic#54572) This adds a compatible APIs for typed endpoints removed in elastic#41640 202 failing tests These 2 tests are explicitly fixed by this PR CompatRestIT. test {yaml=mtermvectors/21_deprecated_with_types/Deprecated camel case and _ parameters should fail in Term Vectors query} CompatRestIT. test {yaml=mtermvectors/30_mix_typeless_typeful/mtermvectors without types on an index that has types} I think more yml tests should be added to 7.x to cover these endpoints 17th june 1306tests | 197failures | 16ignored | 10m56.91sduration
Validating if registered compatible rest handlers are overriding
compatibleWith method and specify the right compatible version
("197 błędów / +197")
based on compat/search branch
fixed tests from delete and update directories.
Delete test is still not fixed
CompatRestIT. test {yaml=delete/70_mix_typeless_typeful/DELETE with typeless API on an index that has types}
current state
1306tests | 174failures
previously
1306tests | 197failures
relates elastic#54160
based on pgomulka:compat/delete_update elastic#58246 CompatRestIT. test {yaml=indices.get_field_mapping/30_missing_type/Raise 404 when type doesn't exist} because of that change (custom type to _doc) a test that was expecting 404 for a type that did not exist, passes now as the logic is trying to fetch for _doc (any type that exist on that index will be good). CompatRestIT. test {yaml=indices.get_mapping/11_basic_with_types/Get /{index}/_mapping with empty mappings} this is because I can't tell if the mapping contained a type or not when fetching a mapping. CompatRestIT. test {yaml=indices.get_mapping/20_missing_type/Existent and non-existent type returns 404 and the existing type} CompatRestIT. test {yaml=indices.get_mapping/20_missing_type/Existent and non-existent types returns 404 and the existing type} CompatRestIT. test {yaml=indices.get_mapping/20_missing_type/No type matching pattern returns 404} CompatRestIT. test {yaml=indices.get_mapping/20_missing_type/Non-existent type returns 404} failing - (not returning 404) - because type is always found (defaulted to _doc) CompatRestIT. test {yaml=indices.get_mapping/20_missing_type/Type missing when no types exist} CompatRestIT. test {yaml=indices.get_mapping/61_empty_with_types/Check empty mapping when getting all mappings via /_mapping} CompatRestIT. test {yaml=indices.get_template/11_basic_with_types/Get template with no mappings} CompatRestIT. test {yaml=indices.get_template/11_basic_with_types/Get template} CompatRestIT. test {yaml=indices.put_mapping/20_mix_typeless_typeful/PUT mapping with _doc on an index that has types} CompatRestIT. test {yaml=indices.put_mapping/20_mix_typeless_typeful/PUT mapping with typeless API on an index that has types} elastic#47364 elastic#41676
pgomulka
pushed a commit
that referenced
this pull request
Mar 16, 2021
…astic#69765) Previously we did not resolve the attributes recursively which meant that if a field or expression was re-aliased multiple times (through multiple levels of subqueries), the aliases were only resolved one level down. This led to failed query translation because `ReferenceAttribute`s were pointing to non-existing attributes during query translation. For example the query ```sql SELECT i AS j FROM ( SELECT int AS i FROM test) ORDER BY j ``` failed during translation because the `OrderBy` resolved the `j` ReferenceAttribute to another `i` ReferenceAttribute that was later removed by an Optimization: ``` OrderBy[[Order[j{r}#4,ASC,LAST]]] ! OrderBy[[Order[i{r}#2,ASC,LAST]]] \_Project[[j]] = \_Project[[j]] \_Project[[i]] ! \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] ! ``` By resolving the `Attributes` recursively both `j{r}` and `i{r}` will resolve to `test.int{f}` above: ``` OrderBy[[Order[test.int{f}#22,ASC,LAST]]] = OrderBy[[Order[test.int{f}#22,ASC,LAST]]] \_Project[[j]] = \_Project[[j]] \_Project[[i]] ! \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] ! ``` The scope of recursive resolution depends on how the `AttributeMap` is constructed and populated. Fixes elastic#67237
pgomulka
pushed a commit
that referenced
this pull request
Mar 31, 2021
…astic#69765) (elastic#70322) Previously we did not resolve the attributes recursively which meant that if a field or expression was re-aliased multiple times (through multiple levels of subqueries), the aliases were only resolved one level down. This led to failed query translation because `ReferenceAttribute`s were pointing to non-existing attributes during query translation. For example the query ```sql SELECT i AS j FROM ( SELECT int AS i FROM test) ORDER BY j ``` failed during translation because the `OrderBy` resolved the `j` ReferenceAttribute to another `i` ReferenceAttribute that was later removed by an Optimization: ``` OrderBy[[Order[j{r}#4,ASC,LAST]]] ! OrderBy[[Order[i{r}#2,ASC,LAST]]] \_Project[[j]] = \_Project[[j]] \_Project[[i]] ! \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] ! ``` By resolving the `Attributes` recursively both `j{r}` and `i{r}` will resolve to `test.int{f}` above: ``` OrderBy[[Order[test.int{f}#22,ASC,LAST]]] = OrderBy[[Order[test.int{f}#22,ASC,LAST]]] \_Project[[j]] = \_Project[[j]] \_Project[[i]] ! \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] ! ``` The scope of recursive resolution depends on how the `AttributeMap` is constructed and populated. Fixes elastic#67237
pgomulka
pushed a commit
that referenced
this pull request
Mar 31, 2021
…astic#69765) (elastic#70325) Previously we did not resolve the attributes recursively which meant that if a field or expression was re-aliased multiple times (through multiple levels of subqueries), the aliases were only resolved one level down. This led to failed query translation because `ReferenceAttribute`s were pointing to non-existing attributes during query translation. For example the query ```sql SELECT i AS j FROM ( SELECT int AS i FROM test) ORDER BY j ``` failed during translation because the `OrderBy` resolved the `j` ReferenceAttribute to another `i` ReferenceAttribute that was later removed by an Optimization: ``` OrderBy[[Order[j{r}#4,ASC,LAST]]] ! OrderBy[[Order[i{r}#2,ASC,LAST]]] \_Project[[j]] = \_Project[[j]] \_Project[[i]] ! \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] ! ``` By resolving the `Attributes` recursively both `j{r}` and `i{r}` will resolve to `test.int{f}` above: ``` OrderBy[[Order[test.int{f}#22,ASC,LAST]]] = OrderBy[[Order[test.int{f}#22,ASC,LAST]]] \_Project[[j]] = \_Project[[j]] \_Project[[i]] ! \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] \_EsRelation[test][date{f}#6, some{f}#7, some.string{f}#8, some.string..] ! ``` The scope of recursive resolution depends on how the `AttributeMap` is constructed and populated. Fixes elastic#67237
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
gradle check?