From 0b5a5493053cba3b5084f5b44bf5886919ba82c8 Mon Sep 17 00:00:00 2001 From: javanna Date: Mon, 27 Jun 2016 11:03:19 +0200 Subject: [PATCH] [TEST] remove special treatment for stashed $body in REST tests, instead always evaluate the stash through ObjectPath When we introduced docs testing we added a special case for $body in Stash, so that the last stashed body could be evaluated, and expressions like "$body.took" could be extracted out of it. We can instead do that for any object in the stash, by simply wrapping the internal map in an ObjectPath instance. We can then drop the special stashResponse method and go back to using the ordinary stashValue too. The downside of this change is that it adds a feature that may not be supported by other REST test runners, namely the evaluation of compouned paths from the stash. If we have "object" stashed as an object, it is now possible to extract directly each subobject of it as well e.g. "object.subobject.field1". None of the current REST tests rely on this, but our docs snippets tests do. --- .../org/elasticsearch/test/rest/ObjectPath.java | 2 +- .../test/rest/RestTestExecutionContext.java | 2 +- .../java/org/elasticsearch/test/rest/Stash.java | 17 ++--------------- .../test/rest/test/ObjectPathTests.java | 8 ++++++++ 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ObjectPath.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ObjectPath.java index da27c3864fc..5ee1fbf521a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ObjectPath.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ObjectPath.java @@ -42,7 +42,7 @@ public class ObjectPath { } } - public ObjectPath(Object object) throws IOException { + public ObjectPath(Object object) { this.object = object; } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/RestTestExecutionContext.java b/test/framework/src/main/java/org/elasticsearch/test/rest/RestTestExecutionContext.java index d3acac8d79b..6d95c7e893f 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/RestTestExecutionContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/RestTestExecutionContext.java @@ -76,7 +76,7 @@ public class RestTestExecutionContext implements Closeable { try { response = callApiInternal(apiName, requestParams, body, headers); //we always stash the last response body - stash.stashResponse(response); + stash.stashValue("body", response.getBody()); return response; } catch(ResponseException e) { response = new RestTestResponse(e); diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/Stash.java b/test/framework/src/main/java/org/elasticsearch/test/rest/Stash.java index 3bac0ef4ff1..13c5b9ff563 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/Stash.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/Stash.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.test.rest.client.RestTestResponse; import java.io.IOException; import java.util.HashMap; @@ -42,7 +41,7 @@ public class Stash implements ToXContent { public static final Stash EMPTY = new Stash(); private final Map stash = new HashMap<>(); - private RestTestResponse response; + private final ObjectPath stashObjectPath = new ObjectPath(stash); /** * Allows to saved a specific field in the stash as key-value pair @@ -55,12 +54,6 @@ public class Stash implements ToXContent { } } - public void stashResponse(RestTestResponse response) throws IOException { - // TODO we can almost certainly save time by lazily evaluating the body - stashValue("body", response.getBody()); - this.response = response; - } - /** * Clears the previously stashed values */ @@ -88,13 +81,7 @@ public class Stash implements ToXContent { * as arguments for following requests (e.g. scroll_id) */ public Object getValue(String key) throws IOException { - if (key.startsWith("$body.")) { - if (response == null) { - return null; - } - return response.evaluate(key.substring("$body".length()), this); - } - Object stashedValue = stash.get(key.substring(1)); + Object stashedValue = stashObjectPath.evaluate(key.substring(1)); if (stashedValue == null) { throw new IllegalArgumentException("stashed value not found for key [" + key + "]"); } diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/test/ObjectPathTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/test/ObjectPathTests.java index ac3a755d344..4a21fefe803 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/test/ObjectPathTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/test/ObjectPathTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.test.rest.ObjectPath; import org.elasticsearch.test.rest.Stash; import java.io.IOException; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -197,6 +198,13 @@ public class ObjectPathTests extends ESTestCase { Object object = objectPath.evaluate("field1.$placeholder.element1", stash); assertThat(object, notNullValue()); assertThat(object.toString(), equalTo("value1")); + + Map stashedObject = new HashMap<>(); + stashedObject.put("subobject", "elements"); + stash.stashValue("object", stashedObject); + object = objectPath.evaluate("field1.$object\\.subobject.element1", stash); + assertThat(object, notNullValue()); + assertThat(object.toString(), equalTo("value1")); } @SuppressWarnings("unchecked")