From aadc33d260f2ae4b9ba3bf1395c29f22f02885ce Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 21 Apr 2017 17:50:22 -0700 Subject: [PATCH] Scripts: Remove unwrap method from executable scripts (#24263) The unwrap method was leftover from support javascript and python. Since those languages are removed in 6.0, this commit removes the unwrap feature from scripts. --- .../AbstractAsyncBulkByScrollAction.java | 21 +++++++++---------- .../action/update/UpdateHelper.java | 2 -- .../script/ExecutableScript.java | 10 --------- .../subphase/ScriptFieldsFetchSubPhase.java | 2 +- .../search/aggregations/metrics/SumIT.java | 5 ----- .../aggregations/metrics/ValueCountIT.java | 5 ----- .../support/ScriptValuesTests.java | 5 ----- .../index/reindex/SimpleExecutableScript.java | 12 ----------- .../bucket/script/TestScript.java | 5 ----- 9 files changed, 11 insertions(+), 56 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/bulk/byscroll/AbstractAsyncBulkByScrollAction.java b/core/src/main/java/org/elasticsearch/action/bulk/byscroll/AbstractAsyncBulkByScrollAction.java index eac901248b6..3ba07ea5538 100644 --- a/core/src/main/java/org/elasticsearch/action/bulk/byscroll/AbstractAsyncBulkByScrollAction.java +++ b/core/src/main/java/org/elasticsearch/action/bulk/byscroll/AbstractAsyncBulkByScrollAction.java @@ -799,8 +799,7 @@ public abstract class AbstractAsyncBulkByScrollAction resultCtx = (Map) executable.unwrap(context); - String newOp = (String) resultCtx.remove("op"); + String newOp = (String) context.remove("op"); if (newOp == null) { throw new IllegalArgumentException("Script cleared operation type"); } @@ -809,25 +808,25 @@ public abstract class AbstractAsyncBulkByScrollAction) resultCtx.remove(SourceFieldMapper.NAME)); + request.setSource((Map) context.remove(SourceFieldMapper.NAME)); - Object newValue = resultCtx.remove(IndexFieldMapper.NAME); + Object newValue = context.remove(IndexFieldMapper.NAME); if (false == doc.getIndex().equals(newValue)) { scriptChangedIndex(request, newValue); } - newValue = resultCtx.remove(TypeFieldMapper.NAME); + newValue = context.remove(TypeFieldMapper.NAME); if (false == doc.getType().equals(newValue)) { scriptChangedType(request, newValue); } - newValue = resultCtx.remove(IdFieldMapper.NAME); + newValue = context.remove(IdFieldMapper.NAME); if (false == doc.getId().equals(newValue)) { scriptChangedId(request, newValue); } - newValue = resultCtx.remove(VersionFieldMapper.NAME); + newValue = context.remove(VersionFieldMapper.NAME); if (false == Objects.equals(oldVersion, newValue)) { scriptChangedVersion(request, newValue); } - newValue = resultCtx.remove(ParentFieldMapper.NAME); + newValue = context.remove(ParentFieldMapper.NAME); if (false == Objects.equals(oldParent, newValue)) { scriptChangedParent(request, newValue); } @@ -835,7 +834,7 @@ public abstract class AbstractAsyncBulkByScrollAction) executableScript.unwrap(ctx); } } catch (Exception e) { throw new IllegalArgumentException("failed to execute script", e); diff --git a/core/src/main/java/org/elasticsearch/script/ExecutableScript.java b/core/src/main/java/org/elasticsearch/script/ExecutableScript.java index 70f42def216..e3f8eb4744f 100644 --- a/core/src/main/java/org/elasticsearch/script/ExecutableScript.java +++ b/core/src/main/java/org/elasticsearch/script/ExecutableScript.java @@ -38,14 +38,4 @@ public interface ExecutableScript { * Executes the script. */ Object run(); - - /** - * Unwraps a possible script value. For example, when passing vars and - * expecting the returned value to be part of the vars. Javascript and - * Python need this but other scripting engines just return the values - * passed in. - */ - default Object unwrap(Object value) { - return value; - } } diff --git a/core/src/main/java/org/elasticsearch/search/fetch/subphase/ScriptFieldsFetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/subphase/ScriptFieldsFetchSubPhase.java index c272ab6dbf0..6bed20e6b3e 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/subphase/ScriptFieldsFetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/subphase/ScriptFieldsFetchSubPhase.java @@ -50,7 +50,7 @@ public final class ScriptFieldsFetchSubPhase implements FetchSubPhase { final Object value; try { - value = leafScript.unwrap(leafScript.run()); + value = leafScript.run(); } catch (RuntimeException e) { if (scriptField.ignoreException()) { continue; diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java index 631e579ac9c..1591e6df931 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/SumIT.java @@ -564,11 +564,6 @@ public class SumIT extends AbstractNumericTestCase { return new LeafSearchScript() { - @Override - public Object unwrap(Object value) { - throw new UnsupportedOperationException(); - } - @Override public void setNextVar(String name, Object value) { vars.put(name, value); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java index 7bd3ebd8847..784635bb1d6 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountIT.java @@ -305,11 +305,6 @@ public class ValueCountIT extends ESIntegTestCase { return new LeafSearchScript() { - @Override - public Object unwrap(Object value) { - throw new UnsupportedOperationException(); - } - @Override public void setNextVar(String name, Object value) { vars.put(name, value); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/support/ScriptValuesTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/support/ScriptValuesTests.java index 11e03a969d0..38111654bbd 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/support/ScriptValuesTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/support/ScriptValuesTests.java @@ -58,11 +58,6 @@ public class ScriptValuesTests extends ESTestCase { return randomBoolean() ? values : Arrays.asList(values); } - @Override - public Object unwrap(Object value) { - throw new UnsupportedOperationException(); - } - @Override public void setScorer(Scorer scorer) { } diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/SimpleExecutableScript.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/SimpleExecutableScript.java index af2312da754..7437a5512df 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/SimpleExecutableScript.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/SimpleExecutableScript.java @@ -50,16 +50,4 @@ public class SimpleExecutableScript implements ExecutableScript { throw new IllegalArgumentException("Unsupported var [" + name + "]"); } } - - @Override - public Object unwrap(Object value) { - // Some script engines (javascript) copy any maps they unwrap - if (randomBoolean()) { - if (value instanceof Map) { - return new HashMap<>((Map) value); - } - } - // Others just return the objects plain (painless) - return value; - } } diff --git a/test/framework/src/main/java/org/elasticsearch/search/aggregations/bucket/script/TestScript.java b/test/framework/src/main/java/org/elasticsearch/search/aggregations/bucket/script/TestScript.java index 13c1340de31..8eb70722641 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/aggregations/bucket/script/TestScript.java +++ b/test/framework/src/main/java/org/elasticsearch/search/aggregations/bucket/script/TestScript.java @@ -56,9 +56,4 @@ public abstract class TestScript implements ExecutableScript{ Objects.requireNonNull(_superset_freq, "_superset_freq"); Objects.requireNonNull(_superset_size, "_superset_size"); } - - @Override - public Double unwrap(Object value) { - return ((Number) value).doubleValue(); - } }