Add integration tests for filters

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/README.asciidoc

@@ -395,7 +395,9 @@ match all of those selectors. The following selectors are supported:
 
 - `version`: Only nodes who's version is within the range will receive the
 request. The syntax for the pattern is the same as when `version` is within
-`skip`.
+`skip` but also supports `current` which selects nodes of the current version.
+`current` is useful when running mixed version tests if the results vary based
+on the version of the node that received the request.
 - `attribute`: Only nodes that have an attribute matching the name and value
 of the provided attribute match.
 Looks like:

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/220_filters_bucket.yml

@@ -275,23 +275,26 @@ setup:
 
 
 ---
-"cache":
+"cache busting":
   - skip:
       version: " - 7.10.99"
       reason:  cache fixed in 7.11.0
+      features: node_selector
 
   - do:
-       bulk:
-         refresh: true
-         body:
-           - index:
-               _index: test_1
-               _id:    100
-           - int_field: 1
-             double_field: 1.0
-             string_field: foo bar
+      bulk:
+        refresh: true
+        body:
+          - index:
+              _index: test_1
+              _id:    100
+          - int_field: 1
+            double_field: 1.0
+            string_field: foo bar
 
   - do:
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
       search:
         index: test_1
         body:
@@ -323,6 +326,8 @@ setup:
 
   # This should be entirely fresh because updating the mapping busted the cache.
   - do:
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
       search:
         index: test_1
         body:
@@ -392,3 +397,110 @@ nested:
   - match: { hits.total.value: 1 }
   - length: { aggregations.f.buckets: 1 }
   - match: { aggregations.f.buckets.foo.doc_count: 1 }
+
+---
+"cache hits":
+  - skip:
+      features: node_selector
+
+  - do:
+      indices.create:
+          index: test
+          body:
+            settings:
+              number_of_shards: 1
+              number_of_replicas: 0
+            mappings:
+              properties:
+                mentions:
+                  type: keyword
+                notifications:
+                  type: keyword
+
+  - do:
+      bulk:
+        refresh: true
+        index: test
+        body: |
+          {"index":{"_id": "foo|bar|baz0"}}
+          {"notifications" : ["abc"]}
+          {"index":{"_id": "foo|bar|baz1"}}
+          {"mentions" : ["abc"]}
+
+  - do:
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
+      search:
+        rest_total_hits_as_int: true
+        size: 0
+        request_cache: true
+        index: test
+        body:
+          aggs:
+            itemsNotify:
+              filters:
+                filters:
+                  lookup:
+                    terms:
+                      mentions:
+                        index: test
+                        id: foo|bar|baz0
+                        path: notifications
+                  std:
+                    terms:
+                      mentions: ["abc"]
+              aggs:
+                mentions:
+                  terms:
+                    field: mentions
+
+  # validate result
+  - match: { hits.total: 2 }
+  - match: { aggregations.itemsNotify.buckets.lookup.doc_count: 1 }
+  - match: { aggregations.itemsNotify.buckets.std.doc_count: 1 }
+
+  # The first request will miss the cache
+  - do:
+      indices.stats: { index: test, metric: request_cache}
+  - match: { _shards.total: 1 }
+  - match: { indices.test.total.request_cache.hit_count: 0 }
+  - match: { indices.test.total.request_cache.miss_count: 1 }
+
+  - do:
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
+      search:
+        rest_total_hits_as_int: true
+        size: 0
+        request_cache: true
+        index: test
+        body:
+          aggs:
+            itemsNotify:
+              filters:
+                filters:
+                  lookup:
+                    terms:
+                      mentions:
+                        index: test
+                        id: foo|bar|baz0
+                        path: notifications
+                  std:
+                    terms:
+                      mentions: ["abc"]
+              aggs:
+                mentions:
+                  terms:
+                    field: mentions
+
+  # validate result
+  - match: { hits.total: 2 }
+  - match: { aggregations.itemsNotify.buckets.lookup.doc_count: 1 }
+  - match: { aggregations.itemsNotify.buckets.std.doc_count: 1 }
+
+  # The second result with hit the cache
+  - do:
+      indices.stats: { index: test, metric: request_cache}
+  - match: { _shards.total: 1 }
+  - match: { indices.test.total.request_cache.hit_count: 1 }
+  - match: { indices.test.total.request_cache.miss_count: 1 }

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/50_filter.yml

@@ -14,66 +14,157 @@ setup:
                   type: keyword
 
   - do:
-      index:
+      bulk:
+        refresh: true
         index: test
-        id: foo|bar|baz0
-        body: { "notifications" : ["abc"] }
+        body: |
+          {"index":{"_id": "foo|bar|baz0"}}
+          {"notifications" : ["abc"]}
+          {"index":{"_id": "foo|bar|baz1"}}
+          {"mentions" : ["abc"]}
 
-  - do:
-      index:
-        index: test
-        id: foo|bar|baz1
-        body: { "mentions" : ["abc"] }
+---
+"Terms lookup gets cached":
+  - skip:
+      features: node_selector
 
   - do:
-      indices.refresh: {}
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
+      search:
+        rest_total_hits_as_int: true
+        size: 0
+        request_cache: true
+        body:
+          aggs:
+            itemsNotify:
+              filter:
+                terms:
+                  mentions:
+                    index: test
+                    id: foo|bar|baz0
+                    path: notifications
+              aggs:
+                mentions:
+                  terms:
+                    field: mentions
 
----
-"Filter aggs with terms lookup and ensure it's cached":
-  # Because the filter agg rewrites the terms lookup in the rewrite phase the request can be cached
+  # validate result
+  - match: { hits.total: 2 }
+  - match: { aggregations.itemsNotify.doc_count: 1 }
+  - length: { aggregations.itemsNotify.mentions.buckets: 1 }
+  - match: { aggregations.itemsNotify.mentions.buckets.0.key: "abc" }
 
+  # The first request will miss the cache
+  - do:
+      indices.stats: { index: test, metric: request_cache}
+  - match: { _shards.total: 1 }
+  - match: { indices.test.total.request_cache.hit_count: 0 }
+  - match: { indices.test.total.request_cache.miss_count: 1 }
 
   - do:
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
       search:
         rest_total_hits_as_int: true
         size: 0
         request_cache: true
-        body: {"aggs": { "itemsNotify": { "filter": { "terms": { "mentions": { "index": "test", "id": "foo|bar|baz0", "path": "notifications"}}}, "aggs": { "mentions" : {"terms" : { "field" : "mentions" }}}}}}
+        body:
+          aggs:
+            itemsNotify:
+              filter:
+                terms:
+                  mentions:
+                    index: test
+                    id: foo|bar|baz0
+                    path: notifications
+              aggs:
+                mentions:
+                  terms:
+                    field: mentions
 
   # validate result
   - match: { hits.total: 2 }
   - match: { aggregations.itemsNotify.doc_count: 1 }
   - length: { aggregations.itemsNotify.mentions.buckets: 1 }
   - match: { aggregations.itemsNotify.mentions.buckets.0.key: "abc" }
-  # we are using a lookup - this should not cache
+
+  # The second result with hit the cache
   - do:
       indices.stats: { index: test, metric: request_cache}
   - match: { _shards.total: 1 }
-  - match: { _all.total.request_cache.hit_count: 0 }
-  - match: { _all.total.request_cache.miss_count: 1 }
-  - is_true: indices.test
+  - match: { indices.test.total.request_cache.hit_count: 1 }
+  - match: { indices.test.total.request_cache.miss_count: 1 }
 
 ---
-"Filter aggs no lookup and ensure it's cached":
-  # now run without lookup and ensure we get cached or at least do the lookup
+"Standard queries get cached":
+  - skip:
+      features: node_selector
+
   - do:
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
       search:
         rest_total_hits_as_int: true
         size: 0
         request_cache: true
-        body: {"aggs": { "itemsNotify": { "filter": { "terms": { "mentions": ["abc"]}}, "aggs": { "mentions" : {"terms" : { "field" : "mentions" }}}}}}
+        body:
+          aggs:
+            itemsNotify:
+              filter:
+                terms:
+                  mentions: ["abc"]
+              aggs:
+                mentions:
+                  terms:
+                    field: mentions
 
+  # Validate result
   - match: { hits.total: 2 }
   - match: { aggregations.itemsNotify.doc_count: 1 }
   - length: { aggregations.itemsNotify.mentions.buckets: 1 }
   - match: { aggregations.itemsNotify.mentions.buckets.0.key: "abc" }
+
+  # The first request will miss the cache
   - do:
       indices.stats: { index: test, metric: request_cache}
   - match: { _shards.total: 1 }
-  - match: { _all.total.request_cache.hit_count: 0 }
-  - match: { _all.total.request_cache.miss_count: 1 }
+  - match: { indices.test.total.request_cache.hit_count: 0 }
+  - match: { indices.test.total.request_cache.miss_count: 1 }
   - is_true: indices.test
 
+  # Try again - it'll cache
+  - do:
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
+      search:
+        rest_total_hits_as_int: true
+        size: 0
+        request_cache: true
+        body:
+          aggs:
+            itemsNotify:
+              filter:
+                terms:
+                  mentions: ["abc"]
+              aggs:
+                mentions:
+                  terms:
+                    field: mentions
+
+  # Validate result
+  - match: { hits.total: 2 }
+  - match: { aggregations.itemsNotify.doc_count: 1 }
+  - length: { aggregations.itemsNotify.mentions.buckets: 1 }
+  - match: { aggregations.itemsNotify.mentions.buckets.0.key: "abc" }
+
+  # The first request will miss the cache
+  - do:
+      indices.stats: { index: test, metric: request_cache}
+  - match: { _shards.total: 1 }
+  - match: { indices.test.total.request_cache.hit_count: 1 }
+  - match: { indices.test.total.request_cache.miss_count: 1 }
+
 ---
 "As a child of terms":
   - do:

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/70_adjacency_matrix.yml

@@ -67,6 +67,8 @@ setup:
   - skip:
       version: " - 7.8.99"
       reason:  fixed in 7.9.0
+      features: node_selector
+
   - do:
       indices.create:
           index: lookup
@@ -91,6 +93,8 @@ setup:
           { "num": [4] }
 
   - do:
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
       search:
         index: test
         body:
@@ -142,6 +146,8 @@ setup:
 
   # The second request should hit the cache
   - do:
+      node_selector:
+        version: current # the version of the node that parsed the request is part of the cache key.
       search:
         index: test
         body:

server/src/main/java/org/elasticsearch/indices/IndicesRequestCache.java

@@ -252,6 +252,19 @@ public int hashCode() {
             result = 31 * result + value.hashCode();
             return result;
         }
+
+        @Override
+        public String toString() {
+            return "Key(mappingKey=["
+                + mappingCacheKey
+                + "],readerKey=["
+                + readerCacheKey
+                + "],entityKey=["
+                + entity.getCacheIdentity()
+                + ",value=" // BytesRef's toString already has [] so we don't add it here
+                + value.toBytesRef() // BytesRef has a readable toString
+                + ")";
+        }
     }
 
     private class CleanupKey implements IndexReader.ClosedListener {