From 1e587406d8593f5443948d3ee464a4c470d550df Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 2 Aug 2016 17:35:31 -0400 Subject: [PATCH] Fail yaml tests and docs snippets that get unexpected warnings Adds `warnings` syntax to the yaml test that allows you to expect a `Warning` header that looks like: ``` - do: warnings: - '[index] is deprecated' - quotes are not required because yaml - but this argument is always a list, never a single string - no matter how many warnings you expect get: index: test type: test id: 1 ``` These are accessible from the docs with: ``` // TEST[warning:some warning] ``` This should help to force you to update the docs if you deprecate something. You *must* add the warnings marker to the docs or the build will fail. While you are there you *should* update the docs to add deprecation warnings visible in the rendered results. --- .../doc/RestTestsFromSnippetsTask.groovy | 18 ++++- .../gradle/doc/SnippetsTask.groovy | 11 +++- docs/README.asciidoc | 4 ++ docs/plugins/mapper-attachments.asciidoc | 14 ++-- docs/reference/indices/analyze.asciidoc | 2 +- .../reference/mapping/params/lat-lon.asciidoc | 21 ++++-- .../query-dsl/function-score-query.asciidoc | 12 ++-- .../query-dsl/indices-query.asciidoc | 5 +- docs/reference/query-dsl/mlt-query.asciidoc | 4 +- .../query-dsl/parent-id-query.asciidoc | 2 +- .../query-dsl/percolate-query.asciidoc | 2 +- .../reference/query-dsl/prefix-query.asciidoc | 3 +- .../query-dsl/template-query.asciidoc | 8 ++- .../search/request/highlighting.asciidoc | 10 +-- .../search/request/source-filtering.asciidoc | 7 +- .../test/lang_mustache/40_template_query.yaml | 22 ++++++- .../rest-api-spec/test/README.asciidoc | 20 +++++- .../rest-api-spec/test/mlt/20_docs.yaml | 6 +- .../test/search/10_source_filtering.yaml | 2 +- .../rest/yaml/ClientYamlTestResponse.java | 16 +++++ .../test/rest/yaml/Features.java | 1 + .../ClientYamlTestSuiteParseContext.java | 9 ++- .../rest/yaml/parser/DoSectionParser.java | 18 +++++ .../test/rest/yaml/section/DoSection.java | 63 +++++++++++++++++- .../yaml/parser/DoSectionParserTests.java | 58 +++++++++++++--- .../rest/yaml/section/DoSectionTests.java | 66 +++++++++++++++++++ 26 files changed, 347 insertions(+), 57 deletions(-) create mode 100644 test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy index adea61cd4f324..100715586d3a6 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy @@ -119,6 +119,7 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { current.println(" reason: $test.skipTest") } if (test.setup != null) { + // Insert a setup defined outside of the docs String setup = setups[test.setup] if (setup == null) { throw new InvalidUserDataException("Couldn't find setup " @@ -136,13 +137,23 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { response.contents.eachLine { current.println(" $it") } } - void emitDo(String method, String pathAndQuery, - String body, String catchPart, boolean inSetup) { + void emitDo(String method, String pathAndQuery, String body, + String catchPart, List warnings, boolean inSetup) { def (String path, String query) = pathAndQuery.tokenize('?') current.println(" - do:") if (catchPart != null) { current.println(" catch: $catchPart") } + if (false == warnings.isEmpty()) { + current.println(" warnings:") + for (String warning in warnings) { + // Escape " because we're going to quote the warning + String escaped = warning.replaceAll('"', '\\\\"') + /* Quote the warning in case it starts with [ which makes + * it look too much like an array. */ + current.println(" - \"$escaped\"") + } + } current.println(" raw:") current.println(" method: $method") current.println(" path: \"$path\"") @@ -200,7 +211,8 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { // Leading '/'s break the generated paths pathAndQuery = pathAndQuery.substring(1) } - emitDo(method, pathAndQuery, body, catchPart, inSetup) + emitDo(method, pathAndQuery, body, catchPart, snippet.warnings, + inSetup) } } diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/SnippetsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/SnippetsTask.groovy index afd91858e9d99..749c0f916f8aa 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/SnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/SnippetsTask.groovy @@ -37,8 +37,9 @@ public class SnippetsTask extends DefaultTask { private static final String CATCH = /catch:\s*((?:\/[^\/]+\/)|[^ \]]+)/ private static final String SKIP = /skip:([^\]]+)/ private static final String SETUP = /setup:([^ \]]+)/ + private static final String WARNING = /warning:(.+)/ private static final String TEST_SYNTAX = - /(?:$CATCH|$SUBSTITUTION|$SKIP|(continued)|$SETUP) ?/ + /(?:$CATCH|$SUBSTITUTION|$SKIP|(continued)|$SETUP|$WARNING) ?/ /** * Action to take on each snippet. Called with a single parameter, an @@ -158,6 +159,10 @@ public class SnippetsTask extends DefaultTask { snippet.setup = it.group(6) return } + if (it.group(7) != null) { + snippet.warnings.add(it.group(7)) + return + } throw new InvalidUserDataException( "Invalid test marker: $line") } @@ -230,6 +235,7 @@ public class SnippetsTask extends DefaultTask { String language = null String catchPart = null String setup = null + List warnings = new ArrayList() @Override public String toString() { @@ -254,6 +260,9 @@ public class SnippetsTask extends DefaultTask { if (setup) { result += "[setup:$setup]" } + for (String warning in warnings) { + result += "[warning:$warning]" + } } if (testResponse) { result += '// TESTRESPONSE' diff --git a/docs/README.asciidoc b/docs/README.asciidoc index da07009d13ddd..5da211c662215 100644 --- a/docs/README.asciidoc +++ b/docs/README.asciidoc @@ -28,6 +28,10 @@ are tests even if they don't have `// CONSOLE`. * `// TEST[setup:name]`: Run some setup code before running the snippet. This is useful for creating and populating indexes used in the snippet. The setup code is defined in `docs/build.gradle`. + * `// TEST[warning:some warning]`: Expect the response to include a `Warning` + header. If the response doesn't include a `Warning` header with the exact + text then the test fails. If the response includes `Warning` headers that + aren't expected then the test fails. * `// TESTRESPONSE`: Matches this snippet against the body of the response of the last test. If the response is JSON then order is ignored. With `// TEST[continued]` you can make tests that contain multiple command snippets diff --git a/docs/plugins/mapper-attachments.asciidoc b/docs/plugins/mapper-attachments.asciidoc index f2c034a317ea9..119ec10c905e9 100644 --- a/docs/plugins/mapper-attachments.asciidoc +++ b/docs/plugins/mapper-attachments.asciidoc @@ -196,14 +196,14 @@ PUT /test "file" : { "type" : "attachment", "fields" : { - "content" : {"index" : "no"}, - "title" : {"store" : "yes"}, - "date" : {"store" : "yes"}, + "content" : {"index" : true}, + "title" : {"store" : true}, + "date" : {"store" : true}, "author" : {"analyzer" : "my_analyzer"}, - "keywords" : {"store" : "yes"}, - "content_type" : {"store" : "yes"}, - "content_length" : {"store" : "yes"}, - "language" : {"store" : "yes"} + "keywords" : {"store" : true}, + "content_type" : {"store" : true}, + "content_length" : {"store" : true}, + "language" : {"store" : true} } } } diff --git a/docs/reference/indices/analyze.asciidoc b/docs/reference/indices/analyze.asciidoc index 5f75da11176d2..e5ed67bf12f4e 100644 --- a/docs/reference/indices/analyze.asciidoc +++ b/docs/reference/indices/analyze.asciidoc @@ -127,7 +127,7 @@ experimental[The format of the additional detail information is experimental and GET _analyze { "tokenizer" : "standard", - "token_filter" : ["snowball"], + "filter" : ["snowball"], "text" : "detailed output", "explain" : true, "attributes" : ["keyword"] <1> diff --git a/docs/reference/mapping/params/lat-lon.asciidoc b/docs/reference/mapping/params/lat-lon.asciidoc index c610d8a177174..88c91c30d06a3 100644 --- a/docs/reference/mapping/params/lat-lon.asciidoc +++ b/docs/reference/mapping/params/lat-lon.asciidoc @@ -1,6 +1,9 @@ [[lat-lon]] === `lat_lon` +deprecated[5.0.0, ????????] +// https://github.com/elastic/elasticsearch/issues/19792 + <> are usually performed by plugging the value of each <> field into a formula to determine whether it falls into the required area or not. Unlike most queries, the inverted index @@ -10,7 +13,7 @@ Setting `lat_lon` to `true` causes the latitude and longitude values to be indexed as numeric fields (called `.lat` and `.lon`). These fields can be used by the <> and <> queries instead of -performing in-memory calculations. +performing in-memory calculations. So this mapping: [source,js] -------------------------------------------------- @@ -27,8 +30,15 @@ PUT my_index } } } +-------------------------------------------------- +// TEST[warning:geo_point lat_lon parameter is deprecated and will be removed in the next major release] +<1> Setting `lat_lon` to true indexes the geo-point in the `location.lat` and `location.lon` fields. + +Allows these actions: -PUT my_index/my_type/1 +[source,js] +-------------------------------------------------- +PUT my_index/my_type/1?refresh { "location": { "lat": 41.12, @@ -46,18 +56,17 @@ GET my_index/_search "lon": -71 }, "distance": "50km", - "optimize_bbox": "indexed" <2> + "optimize_bbox": "indexed" <1> } } } -------------------------------------------------- // CONSOLE -<1> Setting `lat_lon` to true indexes the geo-point in the `location.lat` and `location.lon` fields. -<2> The `indexed` option tells the geo-distance query to use the inverted index instead of the in-memory calculation. +// TEST[continued] +<1> The `indexed` option tells the geo-distance query to use the inverted index instead of the in-memory calculation. Whether the in-memory or indexed operation performs better depends both on your dataset and on the types of queries that you are running. NOTE: The `lat_lon` option only makes sense for single-value `geo_point` fields. It will not work with arrays of geo-points. - diff --git a/docs/reference/query-dsl/function-score-query.asciidoc b/docs/reference/query-dsl/function-score-query.asciidoc index b1b6b56c2b263..0b6214396c9cb 100644 --- a/docs/reference/query-dsl/function-score-query.asciidoc +++ b/docs/reference/query-dsl/function-score-query.asciidoc @@ -18,7 +18,7 @@ GET /_search { "query": { "function_score": { - "query": {}, + "query": { "match_all": {} }, "boost": "5", "random_score": {}, <1> "boost_mode":"multiply" @@ -40,17 +40,17 @@ GET /_search { "query": { "function_score": { - "query": {}, + "query": { "match_all": {} }, "boost": "5", <1> "functions": [ { - "filter": {}, + "filter": { "match": { "test": "bar" } }, "random_score": {}, <2> "weight": 23 }, { - "filter": {}, - "weight": 42 + "filter": { "match": { "test": "cat" } }, + "weight": 42 } ], "max_boost": 42, @@ -170,7 +170,7 @@ you wish to inhibit this, set `"boost_mode": "replace"` The `weight` score allows you to multiply the score by the provided `weight`. This can sometimes be desired since boost value set on specific queries gets normalized, while for this score function it does -not. The number value is of type float. +not. The number value is of type float. [source,js] -------------------------------------------------- diff --git a/docs/reference/query-dsl/indices-query.asciidoc b/docs/reference/query-dsl/indices-query.asciidoc index 8f2f958086e17..112a779e3f9c6 100644 --- a/docs/reference/query-dsl/indices-query.asciidoc +++ b/docs/reference/query-dsl/indices-query.asciidoc @@ -1,6 +1,8 @@ [[query-dsl-indices-query]] === Indices Query +deprecated[5.0.0, Search on the '_index' field instead] + The `indices` query is useful in cases where a search is executed across multiple indices. It allows to specify a list of index names and an inner query that is only executed for indices matching names on that list. @@ -20,7 +22,8 @@ GET /_search } } -------------------------------------------------- -// CONSOLE +// CONSOLE +// TEST[warning:indices query is deprecated. Instead search on the '_index' field] You can use the `index` field to provide a single index. diff --git a/docs/reference/query-dsl/mlt-query.asciidoc b/docs/reference/query-dsl/mlt-query.asciidoc index b132b49f234ba..8e23afdbb8623 100644 --- a/docs/reference/query-dsl/mlt-query.asciidoc +++ b/docs/reference/query-dsl/mlt-query.asciidoc @@ -6,7 +6,7 @@ set of documents. In order to do so, MLT selects a set of representative terms of these input documents, forms a query using these terms, executes the query and returns the results. The user controls the input documents, how the terms should be selected and how the query is formed. `more_like_this` can be -shortened to `mlt` deprecated[5.0.0,use `more_like_this` instead). +shortened to `mlt` deprecated[5.0.0,use `more_like_this` instead]. The simplest use case consists of asking for documents that are similar to a provided piece of text. Here, we are asking for all movies that have some text @@ -175,7 +175,7 @@ follows a similar syntax to the `per_field_analyzer` parameter of the Additionally, to provide documents not necessarily present in the index, <> are also supported. -`unlike`:: +`unlike`:: The `unlike` parameter is used in conjunction with `like` in order not to select terms found in a chosen set of documents. In other words, we could ask for documents `like: "Apple"`, but `unlike: "cake crumble tree"`. The syntax diff --git a/docs/reference/query-dsl/parent-id-query.asciidoc b/docs/reference/query-dsl/parent-id-query.asciidoc index a7a28cf88e8d6..f662dc825c001 100644 --- a/docs/reference/query-dsl/parent-id-query.asciidoc +++ b/docs/reference/query-dsl/parent-id-query.asciidoc @@ -57,7 +57,7 @@ GET /my_index/_search { "query": { "has_parent": { - "type": "blog_post", + "parent_type": "blog_post", "query": { "term": { "_id": "1" diff --git a/docs/reference/query-dsl/percolate-query.asciidoc b/docs/reference/query-dsl/percolate-query.asciidoc index 0d079f6072cbd..2ccc84cabb990 100644 --- a/docs/reference/query-dsl/percolate-query.asciidoc +++ b/docs/reference/query-dsl/percolate-query.asciidoc @@ -19,7 +19,7 @@ PUT /my-index "doctype": { "properties": { "message": { - "type": "string" + "type": "keyword" } } }, diff --git a/docs/reference/query-dsl/prefix-query.asciidoc b/docs/reference/query-dsl/prefix-query.asciidoc index d2b75d10e5ffb..270fc925f0c50 100644 --- a/docs/reference/query-dsl/prefix-query.asciidoc +++ b/docs/reference/query-dsl/prefix-query.asciidoc @@ -28,7 +28,7 @@ GET /_search -------------------------------------------------- // CONSOLE -Or : +Or with the `prefix` deprecated[5.0.0, Use `value`] syntax: [source,js] -------------------------------------------------- @@ -39,6 +39,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:Deprecated field [prefix] used, expected [value] instead] This multi term query allows you to control how it gets rewritten using the <> diff --git a/docs/reference/query-dsl/template-query.asciidoc b/docs/reference/query-dsl/template-query.asciidoc index f66dbe56d3e79..b4b00e5babdcb 100644 --- a/docs/reference/query-dsl/template-query.asciidoc +++ b/docs/reference/query-dsl/template-query.asciidoc @@ -1,6 +1,8 @@ [[query-dsl-template-query]] === Template Query +deprecated[5.0.0, Use the <> API] + A query that accepts a query template and a map of key/value pairs to fill in template parameters. Templating is based on Mustache. For simple token substitution all you provide is a query containing some variable that you want to substitute and the actual @@ -21,6 +23,7 @@ GET /_search } ------------------------------------------ // CONSOLE +// TEST[warning:[template] query is deprecated, use search template api instead] The above request is translated into: @@ -54,6 +57,7 @@ GET /_search } ------------------------------------------ // CONSOLE +// TEST[warning:[template] query is deprecated, use search template api instead] <1> New line characters (`\n`) should be escaped as `\\n` or removed, and quotes (`"`) should be escaped as `\\"`. @@ -80,6 +84,7 @@ GET /_search } ------------------------------------------ // CONSOLE +// TEST[warning:[template] query is deprecated, use search template api instead] <1> Name of the query template in `config/scripts/`, i.e., `my_template.mustache`. @@ -113,11 +118,10 @@ GET /_search ------------------------------------------ // CONSOLE // TEST[continued] +// TEST[warning:[template] query is deprecated, use search template api instead] <1> Name of the query template in `config/scripts/`, i.e., `my_template.mustache`. There is also a dedicated `template` endpoint, allows you to template an entire search request. Please see <> for more details. - - diff --git a/docs/reference/search/request/highlighting.asciidoc b/docs/reference/search/request/highlighting.asciidoc index efb7053c17923..2e7bc9f180528 100644 --- a/docs/reference/search/request/highlighting.asciidoc +++ b/docs/reference/search/request/highlighting.asciidoc @@ -52,9 +52,9 @@ It tries hard to reflect the query matching logic in terms of understanding word [WARNING] If you want to highlight a lot of fields in a lot of documents with complex queries this highlighter will not be fast. -In its efforts to accurately reflect query logic it creates a tiny in-memory index and re-runs the original query criteria through -Lucene's query execution planner to get access to low-level match information on the current document. -This is repeated for every field and every document that needs highlighting. If this presents a performance issue in your system consider using an alternative highlighter. +In its efforts to accurately reflect query logic it creates a tiny in-memory index and re-runs the original query criteria through +Lucene's query execution planner to get access to low-level match information on the current document. +This is repeated for every field and every document that needs highlighting. If this presents a performance issue in your system consider using an alternative highlighter. [[postings-highlighter]] ==== Postings highlighter @@ -387,7 +387,7 @@ GET /_search "match_phrase": { "content": { "query": "foo bar", - "phrase_slop": 1 + "slop": 1 } } }, @@ -413,7 +413,7 @@ GET /_search "match_phrase": { "content": { "query": "foo bar", - "phrase_slop": 1, + "slop": 1, "boost": 10.0 } } diff --git a/docs/reference/search/request/source-filtering.asciidoc b/docs/reference/search/request/source-filtering.asciidoc index 08625751eecc0..03c02538a0ea6 100644 --- a/docs/reference/search/request/source-filtering.asciidoc +++ b/docs/reference/search/request/source-filtering.asciidoc @@ -53,15 +53,16 @@ GET /_search -------------------------------------------------- // CONSOLE -Finally, for complete control, you can specify both include and exclude patterns: +Finally, for complete control, you can specify both `includes` and `excludes` +patterns: [source,js] -------------------------------------------------- GET /_search { "_source": { - "include": [ "obj1.*", "obj2.*" ], - "exclude": [ "*.description" ] + "includes": [ "obj1.*", "obj2.*" ], + "excludes": [ "*.description" ] }, "query" : { "term" : { "user" : "kimchy" } diff --git a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/40_template_query.yaml b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/40_template_query.yaml index cf3e6883e45d7..2360dfc37f0bf 100644 --- a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/40_template_query.yaml +++ b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/40_template_query.yaml @@ -1,5 +1,7 @@ --- "Template query": + - skip: + features: warnings - do: index: @@ -23,54 +25,72 @@ - match: { acknowledged: true } - do: + warnings: + - '[template] query is deprecated, use search template api instead' search: body: { "query": { "template": { "inline": { "term": { "text": { "value": "{{template}}" } } }, "params": { "template": "value1" } } } } - match: { hits.total: 1 } - do: + warnings: + - '[template] query is deprecated, use search template api instead' search: body: { "query": { "template": { "file": "file_query_template", "params": { "my_value": "value1" } } } } - match: { hits.total: 1 } - do: + warnings: + - '[template] query is deprecated, use search template api instead' search: body: { "query": { "template": { "id": "1", "params": { "my_value": "value1" } } } } - match: { hits.total: 1 } - do: + warnings: + - '[template] query is deprecated, use search template api instead' search: body: { "query": { "template": { "id": "/mustache/1", "params": { "my_value": "value1" } } } } - match: { hits.total: 1 } - do: - search: + warnings: + - '[template] query is deprecated, use search template api instead' + search: body: { "query": { "template": { "inline": {"match_{{template}}": {}}, "params" : { "template" : "all" } } } } - match: { hits.total: 2 } - do: + warnings: + - '[template] query is deprecated, use search template api instead' search: body: { "query": { "template": { "inline": "{ \"term\": { \"text\": { \"value\": \"{{template}}\" } } }", "params": { "template": "value1" } } } } - match: { hits.total: 1 } - do: + warnings: + - '[template] query is deprecated, use search template api instead' search: body: { "query": { "template": { "inline": "{\"match_{{template}}\": {}}", "params" : { "template" : "all" } } } } - match: { hits.total: 2 } - do: + warnings: + - '[template] query is deprecated, use search template api instead' search: body: { "query": { "template": { "inline": "{\"match_all\": {}}", "params" : {} } } } - match: { hits.total: 2 } - do: + warnings: + - '[template] query is deprecated, use search template api instead' search: body: { "query": { "template": { "inline": "{\"query_string\": { \"query\" : \"{{query}}\" }}", "params" : { "query" : "text:\"value2 value3\"" } } } } - match: { hits.total: 1 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/README.asciidoc b/rest-api-spec/src/main/resources/rest-api-spec/test/README.asciidoc index 4e88cef4c9f15..16b607f4a22e1 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/README.asciidoc +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/README.asciidoc @@ -173,6 +173,25 @@ The argument to `catch` can be any of: If `catch` is specified, then the `response` var must be cleared, and the test should fail if no error is thrown. +If the arguments to `do` include `warnings` then we are expecting a `Warning` +header to come back from the request. If the arguments *don't* include a +`warnings` argument then we *don't* expect the response to include a `Warning` +header. The warnings must match exactly. Using it looks like this: + +.... + - do: + warnings: + - '[index] is deprecated' + - quotes are not required because yaml + - but this argument is always a list, never a single string + - no matter how many warnings you expect + get: + index: test + type: test + id: 1 +.... + + === `set` For some tests, it is necessary to extract a value from the previous `response`, in @@ -284,4 +303,3 @@ This depends on the datatype of the value being examined, eg: - length: { _tokens: 3 } # the `_tokens` array has 3 elements - length: { _source: 5 } # the `_source` hash has 5 keys .... - diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/mlt/20_docs.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/mlt/20_docs.yaml index 85d22a4dc23f7..415a38f00c10e 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/mlt/20_docs.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/mlt/20_docs.yaml @@ -41,7 +41,7 @@ body: query: more_like_this: - docs: + like: - _index: test_1 _type: test @@ -51,8 +51,8 @@ _index: test_1 _type: test _id: 2 - ids: - - 3 + - + _id: 3 include: true min_doc_freq: 0 min_term_freq: 0 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yaml index 424153aa5738e..48857522cb8aa 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yaml @@ -72,7 +72,7 @@ setup: search: body: _source: - include: [ include.field1, include.field2 ] + includes: [ include.field1, include.field2 ] query: { match_all: {} } - match: { hits.hits.0._source.include.field1: v1 } - match: { hits.hits.0._source.include.field2: v2 } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestResponse.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestResponse.java index 8f449274ea5d5..481ae752d05f0 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestResponse.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestResponse.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.test.rest.yaml; +import org.apache.http.Header; import org.apache.http.client.methods.HttpHead; import org.apache.http.util.EntityUtils; import org.elasticsearch.client.Response; @@ -25,6 +26,8 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; /** * Response obtained from a REST call, eagerly reads the response body into a string for later optional parsing. @@ -70,6 +73,19 @@ public String getReasonPhrase() { return response.getStatusLine().getReasonPhrase(); } + /** + * Get a list of all of the values of all warning headers returned in the response. + */ + public List getWarningHeaders() { + List warningHeaders = new ArrayList<>(); + for (Header header : response.getHeaders()) { + if (header.getName().equals("Warning")) { + warningHeaders.add(header.getValue()); + } + } + return warningHeaders; + } + /** * Returns the body properly parsed depending on the content type. * Might be a string or a json object parsed as a map. diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/Features.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/Features.java index df797dd53dde0..6e47220ec083b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/Features.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/Features.java @@ -41,6 +41,7 @@ public final class Features { "groovy_scripting", "headers", "stash_in_path", + "warnings", "yaml")); private Features() { diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/parser/ClientYamlTestSuiteParseContext.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/parser/ClientYamlTestSuiteParseContext.java index 73fd57c3debcb..e466c1010f6b2 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/parser/ClientYamlTestSuiteParseContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/parser/ClientYamlTestSuiteParseContext.java @@ -18,6 +18,8 @@ */ package org.elasticsearch.test.rest.yaml.parser; +import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.xcontent.XContentLocation; import org.elasticsearch.common.xcontent.XContentParser; @@ -36,7 +38,7 @@ * Context shared across the whole tests parse phase. * Provides shared parse methods and holds information needed to parse the test sections (e.g. es version) */ -public class ClientYamlTestSuiteParseContext { +public class ClientYamlTestSuiteParseContext implements ParseFieldMatcherSupplier { private static final SetupSectionParser SETUP_SECTION_PARSER = new SetupSectionParser(); private static final TeardownSectionParser TEARDOWN_SECTION_PARSER = new TeardownSectionParser(); @@ -185,4 +187,9 @@ public Tuple parseTuple() throws IOException, ClientYamlTestPars Map.Entry entry = map.entrySet().iterator().next(); return Tuple.tuple(entry.getKey(), entry.getValue()); } + + @Override + public ParseFieldMatcher getParseFieldMatcher() { + return ParseFieldMatcher.STRICT; + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParser.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParser.java index eda0f728f9330..b89aa821ec94b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParser.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParser.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.test.rest.yaml.parser; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -25,9 +26,13 @@ import org.elasticsearch.test.rest.yaml.section.DoSection; import java.io.IOException; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; +import static java.util.Collections.unmodifiableList; + /** * Parser for do sections */ @@ -44,6 +49,7 @@ public DoSection parse(ClientYamlTestSuiteParseContext parseContext) throws IOEx DoSection doSection = new DoSection(parseContext.parser().getTokenLocation()); ApiCallSection apiCallSection = null; Map headers = new HashMap<>(); + List expectedWarnings = new ArrayList<>(); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { @@ -52,6 +58,17 @@ public DoSection parse(ClientYamlTestSuiteParseContext parseContext) throws IOEx if ("catch".equals(currentFieldName)) { doSection.setCatch(parser.text()); } + } else if (token == XContentParser.Token.START_ARRAY) { + if ("warnings".equals(currentFieldName)) { + while ((token = parser.nextToken()) == XContentParser.Token.VALUE_STRING) { + expectedWarnings.add(parser.text()); + } + if (token != XContentParser.Token.END_ARRAY) { + throw new ParsingException(parser.getTokenLocation(), "[warnings] must be a string array but saw [" + token + "]"); + } + } else { + throw new ParsingException(parser.getTokenLocation(), "unknown array [" + currentFieldName + "]"); + } } else if (token == XContentParser.Token.START_OBJECT) { if ("headers".equals(currentFieldName)) { String headerName = null; @@ -97,6 +114,7 @@ public DoSection parse(ClientYamlTestSuiteParseContext parseContext) throws IOEx apiCallSection.addHeaders(headers); } doSection.setApiCallSection(apiCallSection); + doSection.setExpectedWarningHeaders(unmodifiableList(expectedWarnings)); } finally { parser.nextToken(); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index 4bad7f57a3a1f..af4a8e4f51a1b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -29,8 +29,12 @@ import java.io.IOException; import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; +import java.util.Set; +import static java.util.Collections.emptyList; import static org.elasticsearch.common.collect.Tuple.tuple; import static org.elasticsearch.test.hamcrest.RegexMatcher.matches; import static org.hamcrest.Matchers.allOf; @@ -49,6 +53,10 @@ * headers: * Authorization: Basic user:pass * Content-Type: application/json + * warnings: + * - Stuff is deprecated, yo + * - Don't use deprecated stuff + * - Please, stop. It hurts. * update: * index: test_1 * type: test @@ -63,6 +71,7 @@ public class DoSection implements ExecutableSection { private final XContentLocation location; private String catchParam; private ApiCallSection apiCallSection; + private List expectedWarningHeaders = emptyList(); public DoSection(XContentLocation location) { this.location = location; @@ -84,6 +93,22 @@ public void setApiCallSection(ApiCallSection apiCallSection) { this.apiCallSection = apiCallSection; } + /** + * Warning headers that we expect from this response. If the headers don't match exactly this request is considered to have failed. + * Defaults to emptyList. + */ + public List getExpectedWarningHeaders() { + return expectedWarningHeaders; + } + + /** + * Set the warning headers that we expect from this response. If the headers don't match exactly this request is considered to have + * failed. Defaults to emptyList. + */ + public void setExpectedWarningHeaders(List expectedWarningHeaders) { + this.expectedWarningHeaders = expectedWarningHeaders; + } + @Override public XContentLocation getLocation() { return location; @@ -100,7 +125,7 @@ public void execute(ClientYamlTestExecutionContext executionContext) throws IOEx } try { - ClientYamlTestResponse restTestResponse = executionContext.callApi(apiCallSection.getApi(), apiCallSection.getParams(), + ClientYamlTestResponse response = executionContext.callApi(apiCallSection.getApi(), apiCallSection.getParams(), apiCallSection.getBodies(), apiCallSection.getHeaders()); if (Strings.hasLength(catchParam)) { String catchStatusCode; @@ -111,8 +136,9 @@ public void execute(ClientYamlTestExecutionContext executionContext) throws IOEx } else { throw new UnsupportedOperationException("catch value [" + catchParam + "] not supported"); } - fail(formatStatusCodeMessage(restTestResponse, catchStatusCode)); + fail(formatStatusCodeMessage(response, catchStatusCode)); } + checkWarningHeaders(response.getWarningHeaders()); } catch(ClientYamlTestResponseException e) { ClientYamlTestResponse restTestResponse = e.getRestTestResponse(); if (!Strings.hasLength(catchParam)) { @@ -135,6 +161,39 @@ public void execute(ClientYamlTestExecutionContext executionContext) throws IOEx } } + /** + * Check that the response contains only the warning headers that we expect. + */ + void checkWarningHeaders(List warningHeaders) { + StringBuilder failureMessage = null; + // LinkedHashSet so that missing expected warnings come back in a predictable order which is nice for testing + Set expected = new LinkedHashSet<>(expectedWarningHeaders); + for (String header : warningHeaders) { + if (expected.remove(header)) { + // Was expected, all good. + continue; + } + if (failureMessage == null) { + failureMessage = new StringBuilder("got unexpected warning headers ["); + } + failureMessage.append('\n').append(header); + } + if (false == expected.isEmpty()) { + if (failureMessage == null) { + failureMessage = new StringBuilder(); + } else { + failureMessage.append("\n] "); + } + failureMessage.append("didn't get expected warning headers ["); + for (String header : expected) { + failureMessage.append('\n').append(header); + } + } + if (failureMessage != null) { + fail(failureMessage + "\n]"); + } + } + private void assertStatusCode(ClientYamlTestResponse restTestResponse) { Tuple> stringMatcherTuple = catches.get(catchParam); assertThat(formatStatusCodeMessage(restTestResponse, stringMatcherTuple.v1()), diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParserTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParserTests.java index 5d79432155f83..0cecc508bf512 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParserTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParserTests.java @@ -29,8 +29,10 @@ import org.hamcrest.MatcherAssert; import java.io.IOException; +import java.util.Arrays; import java.util.Map; +import static java.util.Collections.singletonList; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -344,11 +346,11 @@ public void testParseDoSectionWithCatch() throws Exception { public void testParseDoSectionWithHeaders() throws Exception { parser = YamlXContent.yamlXContent.createParser( "headers:\n" + - " Authorization: \"thing one\"\n" + - " Content-Type: \"application/json\"\n" + - "indices.get_warmer:\n" + - " index: test_index\n" + - " name: test_warmer" + " Authorization: \"thing one\"\n" + + " Content-Type: \"application/json\"\n" + + "indices.get_warmer:\n" + + " index: test_index\n" + + " name: test_warmer" ); DoSectionParser doSectionParser = new DoSectionParser(); @@ -381,9 +383,9 @@ public void testParseDoSectionWithoutClientCallSection() throws Exception { public void testParseDoSectionMultivaluedField() throws Exception { parser = YamlXContent.yamlXContent.createParser( "indices.get_field_mapping:\n" + - " index: test_index\n" + - " type: test_type\n" + - " field: [ text , text1 ]" + " index: test_index\n" + + " type: test_type\n" + + " field: [ text , text1 ]" ); DoSectionParser doSectionParser = new DoSectionParser(); @@ -400,6 +402,46 @@ public void testParseDoSectionMultivaluedField() throws Exception { assertThat(doSection.getApiCallSection().getBodies().size(), equalTo(0)); } + public void testParseDoSectionExpectedWarnings() throws Exception { + parser = YamlXContent.yamlXContent.createParser( + "indices.get_field_mapping:\n" + + " index: test_index\n" + + " type: test_type\n" + + "warnings:\n" + + " - some test warning they are typically pretty long\n" + + " - some other test warning somtimes they have [in] them" + ); + + DoSectionParser doSectionParser = new DoSectionParser(); + DoSection doSection = doSectionParser.parse(new ClientYamlTestSuiteParseContext("api", "suite", parser)); + + assertThat(doSection.getCatch(), nullValue()); + assertThat(doSection.getApiCallSection(), notNullValue()); + assertThat(doSection.getApiCallSection().getApi(), equalTo("indices.get_field_mapping")); + assertThat(doSection.getApiCallSection().getParams().size(), equalTo(2)); + assertThat(doSection.getApiCallSection().getParams().get("index"), equalTo("test_index")); + assertThat(doSection.getApiCallSection().getParams().get("type"), equalTo("test_type")); + assertThat(doSection.getApiCallSection().hasBody(), equalTo(false)); + assertThat(doSection.getApiCallSection().getBodies().size(), equalTo(0)); + assertThat(doSection.getExpectedWarningHeaders(), equalTo(Arrays.asList( + "some test warning they are typically pretty long", + "some other test warning somtimes they have [in] them"))); + + parser = YamlXContent.yamlXContent.createParser( + "indices.get_field_mapping:\n" + + " index: test_index\n" + + "warnings:\n" + + " - just one entry this time" + ); + + doSection = doSectionParser.parse(new ClientYamlTestSuiteParseContext("api", "suite", parser)); + assertThat(doSection.getCatch(), nullValue()); + assertThat(doSection.getApiCallSection(), notNullValue()); + assertThat(doSection.getExpectedWarningHeaders(), equalTo(singletonList( + "just one entry this time"))); + + } + private static void assertJsonEquals(Map actual, String expected) throws IOException { Map expectedMap; try (XContentParser parser = JsonXContent.jsonXContent.createParser(expected)) { diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java new file mode 100644 index 0000000000000..e981b2d999c06 --- /dev/null +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java @@ -0,0 +1,66 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.test.rest.yaml.section; + +import org.elasticsearch.common.xcontent.XContentLocation; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.util.Arrays; + +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; + +public class DoSectionTests extends ESTestCase { + public void testWarningHeaders() throws IOException { + DoSection section = new DoSection(new XContentLocation(1, 1)); + + // No warning headers doesn't throw an exception + section.checkWarningHeaders(emptyList()); + + // Any warning headers fail + AssertionError e = expectThrows(AssertionError.class, () -> section.checkWarningHeaders(singletonList("test"))); + assertEquals("got unexpected warning headers [\ntest\n]", e.getMessage()); + e = expectThrows(AssertionError.class, () -> section.checkWarningHeaders(Arrays.asList("test", "another", "some more"))); + assertEquals("got unexpected warning headers [\ntest\nanother\nsome more\n]", e.getMessage()); + + // But not when we expect them + section.setExpectedWarningHeaders(singletonList("test")); + section.checkWarningHeaders(singletonList("test")); + section.setExpectedWarningHeaders(Arrays.asList("test", "another", "some more")); + section.checkWarningHeaders(Arrays.asList("test", "another", "some more")); + + // But if you don't get some that you did expect, that is an error + section.setExpectedWarningHeaders(singletonList("test")); + e = expectThrows(AssertionError.class, () -> section.checkWarningHeaders(emptyList())); + assertEquals("didn't get expected warning headers [\ntest\n]", e.getMessage()); + section.setExpectedWarningHeaders(Arrays.asList("test", "another", "some more")); + e = expectThrows(AssertionError.class, () -> section.checkWarningHeaders(emptyList())); + assertEquals("didn't get expected warning headers [\ntest\nanother\nsome more\n]", e.getMessage()); + e = expectThrows(AssertionError.class, () -> section.checkWarningHeaders(Arrays.asList("test", "some more"))); + assertEquals("didn't get expected warning headers [\nanother\n]", e.getMessage()); + + // It is also an error if you get some warning you want and some you don't want + section.setExpectedWarningHeaders(Arrays.asList("test", "another", "some more")); + e = expectThrows(AssertionError.class, () -> section.checkWarningHeaders(Arrays.asList("test", "cat"))); + assertEquals("got unexpected warning headers [\ncat\n] didn't get expected warning headers [\nanother\nsome more\n]", + e.getMessage()); + } +}