Refactor file script tests to be real unit tests

core/src/test/java/org/elasticsearch/script/FileScriptTests.java

@@ -0,0 +1,84 @@
+/*
+ * 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.script;
+
+import org.elasticsearch.common.ContextAndHeaderHolder;
+import org.elasticsearch.common.bytes.BytesArray;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.script.mustache.MustacheScriptEngineService;
+import org.elasticsearch.test.ESTestCase;
+import org.junit.Test;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+import static org.elasticsearch.common.settings.Settings.settingsBuilder;
+import static org.hamcrest.Matchers.containsString;
+
+// TODO: these really should just be part of ScriptService tests, there is nothing special about them
+public class FileScriptTests extends ESTestCase {
+
+    ScriptService makeScriptService(Settings settings) throws Exception {
+        Path homeDir = createTempDir();
+        Path scriptsDir = homeDir.resolve("config").resolve("scripts");
+        Files.createDirectories(scriptsDir);
+        Path mockscript = scriptsDir.resolve("script1.mockscript");
+        Files.write(mockscript, "1".getBytes("UTF-8"));
+        settings = Settings.builder()
+            .put("path.home", homeDir)
+                // no file watching, so we don't need a ResourceWatcherService
+            .put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING, false)
+            .put(settings)
+            .build();
+        Set<ScriptEngineService> engines = new HashSet<>(Collections.singletonList(new MockScriptEngine()));
+        return new ScriptService(settings, new Environment(settings), engines, null, new ScriptContextRegistry(Collections.emptyList()));
+    }
+
+    public void testFileScriptFound() throws Exception {
+        ContextAndHeaderHolder contextAndHeaders = new ContextAndHeaderHolder();
+        Settings settings = Settings.builder()
+            .put("script.engine." + MockScriptEngine.NAME + ".file.aggs", false).build();
+        ScriptService scriptService = makeScriptService(settings);
+        Script script = new Script("script1", ScriptService.ScriptType.FILE, MockScriptEngine.NAME, null);
+        assertNotNull(scriptService.compile(script, ScriptContext.Standard.SEARCH, contextAndHeaders));
+    }
+
+    public void testAllOpsDisabled() throws Exception {
+        ContextAndHeaderHolder contextAndHeaders = new ContextAndHeaderHolder();
+        Settings settings = Settings.builder()
+            .put("script.engine." + MockScriptEngine.NAME + ".file.aggs", false)
+            .put("script.engine." + MockScriptEngine.NAME + ".file.search", false)
+            .put("script.engine." + MockScriptEngine.NAME + ".file.mapping", false)
+            .put("script.engine." + MockScriptEngine.NAME + ".file.update", false).build();
+        ScriptService scriptService = makeScriptService(settings);
+        Script script = new Script("script1", ScriptService.ScriptType.FILE, MockScriptEngine.NAME, null);
+        for (ScriptContext context : ScriptContext.Standard.values()) {
+            try {
+                scriptService.compile(script, context, contextAndHeaders);
+                fail(context.getKey() + " script should have been rejected");
+            } catch(Exception e) {
+                assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [file], operation [" + context.getKey() + "] and lang [" + MockScriptEngine.NAME + "] are disabled"));
+            }
+        }
+    }
+}

core/src/test/java/org/elasticsearch/script/MockScriptEngine.java

@@ -0,0 +1,81 @@
+/*
+ * 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.script;
+
+import org.elasticsearch.common.Nullable;
+import org.elasticsearch.search.lookup.SearchLookup;
+
+import java.io.IOException;
+import java.util.Map;
+
+/**
+ * A dummy script engine used for testing. Scripts must be a number. Running the script
+ */
+public class MockScriptEngine implements ScriptEngineService {
+    public static final String NAME = "mockscript";
+
+    @Override
+    public String[] types() {
+        return new String[]{ NAME };
+    }
+
+    @Override
+    public String[] extensions() {
+        return types();
+    }
+
+    @Override
+    public boolean sandboxed() {
+        return true;
+    }
+
+    @Override
+    public Object compile(String script) {
+        return Integer.parseInt(script);
+    }
+
+    @Override
+    public ExecutableScript executable(CompiledScript compiledScript, @Nullable Map<String, Object> vars) {
+        return null;
+    }
+
+    @Override
+    public SearchScript search(CompiledScript compiledScript, SearchLookup lookup, @Nullable Map<String, Object> vars) {
+        return null;
+    }
+
+    @Override
+    public Object execute(CompiledScript compiledScript, Map<String, Object> vars) {
+        return null;
+    }
+
+    @Override
+    public Object unwrap(Object value) {
+        return null;
+    }
+
+    @Override
+    public void scriptRemoved(@Nullable CompiledScript script) {
+    }
+
+    @Override
+    public void close() throws IOException {
+    }
+}

core/src/test/java/org/elasticsearch/script/OnDiskScriptIT.java

@@ -1,103 +0,0 @@
-/*
- * 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.script;
-
-import org.elasticsearch.action.index.IndexRequestBuilder;
-import org.elasticsearch.action.search.SearchResponse;
-import org.elasticsearch.common.bytes.BytesArray;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.script.mustache.MustacheScriptEngineService;
-import org.elasticsearch.search.SearchHit;
-import org.elasticsearch.test.ESIntegTestCase;
-import org.junit.Test;
-
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.ExecutionException;
-
-import static org.elasticsearch.common.settings.Settings.settingsBuilder;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.equalTo;
-
-//Use Suite scope so that paths get set correctly
-@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE)
-public class OnDiskScriptIT extends ESIntegTestCase {
-
-    @Override
-    public Settings nodeSettings(int nodeOrdinal) {
-        //Set path so ScriptService will pick up the test scripts
-        return settingsBuilder().put(super.nodeSettings(nodeOrdinal))
-                .put("path.conf", this.getDataPath("config"))
-                .put("script.engine.expression.file.aggs", "off")
-                .put("script.engine.mustache.file.aggs", "off")
-                .put("script.engine.mustache.file.search", "off")
-                .put("script.engine.mustache.file.mapping", "off")
-                .put("script.engine.mustache.file.update", "off").build();
-    }
-
-    @Test
-    public void testFieldOnDiskScript()  throws ExecutionException, InterruptedException {
-        List<IndexRequestBuilder> builders = new ArrayList<>();
-        builders.add(client().prepareIndex("test", "scriptTest", "1").setSource("{\"theField\":\"foo\"}"));
-        builders.add(client().prepareIndex("test", "scriptTest", "2").setSource("{\"theField\":\"foo 2\"}"));
-        builders.add(client().prepareIndex("test", "scriptTest", "3").setSource("{\"theField\":\"foo 3\"}"));
-        builders.add(client().prepareIndex("test", "scriptTest", "4").setSource("{\"theField\":\"foo 4\"}"));
-        builders.add(client().prepareIndex("test", "scriptTest", "5").setSource("{\"theField\":\"bar\"}"));
-        indexRandom(true, builders);
-
-        String query = "{ \"query\" : { \"match_all\": {}} , \"script_fields\" : { \"test1\" : { \"script_file\" : \"script1\" }, \"test2\" : { \"script_file\" : \"script2\", \"params\":{\"factor\":3}  }}, size:1}";
-        SearchResponse searchResponse = client().prepareSearch().setSource(new BytesArray(query)).setIndices("test").setTypes("scriptTest").get();
-        assertHitCount(searchResponse, 5);
-        assertTrue(searchResponse.getHits().hits().length == 1);
-        SearchHit sh = searchResponse.getHits().getAt(0);
-        assertThat((Integer)sh.field("test1").getValue(), equalTo(2));
-        assertThat((Integer)sh.field("test2").getValue(), equalTo(6));
-    }
-
-    @Test
-    public void testAllOpsDisabledOnDiskScripts() {
-        //whether we even compile or cache the on disk scripts doesn't change the end result (the returned error)
-        client().prepareIndex("test", "scriptTest", "1").setSource("{\"theField\":\"foo\"}").get();
-        refresh();
-        String source = "{\"aggs\": {\"test\": { \"terms\" : { \"script_file\":\"script1\", \"lang\": \"mustache\" } } } }";
-        try {
-            client().prepareSearch("test").setSource(new BytesArray(source)).get();
-            fail("aggs script should have been rejected");
-        } catch(Exception e) {
-            assertThat(e.toString(), containsString("scripts of type [file], operation [aggs] and lang [mustache] are disabled"));
-        }
-        String query = "{ \"query\" : { \"match_all\": {}} , \"script_fields\" : { \"test1\" : { \"script_file\" : \"script1\", \"lang\":\"mustache\" }}, size:1}";
-        try {
-            client().prepareSearch().setSource(new BytesArray(query)).setIndices("test").setTypes("scriptTest").get();
-            fail("search script should have been rejected");
-        } catch(Exception e) {
-            assertThat(e.toString(), containsString("scripts of type [file], operation [search] and lang [mustache] are disabled"));
-        }
-        try {
-            client().prepareUpdate("test", "scriptTest", "1")
-                    .setScript(new Script("script1", ScriptService.ScriptType.FILE, MustacheScriptEngineService.NAME, null)).get();
-            fail("update script should have been rejected");
-        } catch (Exception e) {
-            assertThat(e.getMessage(), containsString("failed to execute script"));
-            assertThat(e.getCause().getMessage(), containsString("scripts of type [file], operation [update] and lang [mustache] are disabled"));
-        }
-    }
-
-}

core/src/test/java/org/elasticsearch/script/ScriptContextTests.java

@@ -20,24 +20,19 @@
 package org.elasticsearch.script;
 
 import org.elasticsearch.common.ContextAndHeaderHolder;
-import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
-import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.test.ESTestCase;
 
-import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 
-public class CustomScriptContextTests extends ESTestCase {
+public class ScriptContextTests extends ESTestCase {
 
     private static final String PLUGIN_NAME = "testplugin";
-    private static final String SCRIPT_LANG = "customlang";
 
     ScriptService makeScriptService() throws Exception {
         Settings settings = Settings.builder()
@@ -45,7 +40,7 @@ public class CustomScriptContextTests extends ESTestCase {
             // no file watching, so we don't need a ResourceWatcherService
             .put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING, false)
             .put("script." + PLUGIN_NAME + "_custom_globally_disabled_op", false)
-            .put("script.engine." + SCRIPT_LANG + ".inline." + PLUGIN_NAME + "_custom_exp_disabled_op", false)
+            .put("script.engine." + MockScriptEngine.NAME + ".inline." + PLUGIN_NAME + "_custom_exp_disabled_op", false)
             .build();
         Set<ScriptEngineService> engines = new HashSet<>(Collections.singletonList(new MockScriptEngine()));
         List<ScriptContext.Plugin> customContexts = Arrays.asList(
@@ -60,11 +55,11 @@ public class CustomScriptContextTests extends ESTestCase {
         ScriptService scriptService = makeScriptService();
         for (ScriptService.ScriptType scriptType : ScriptService.ScriptType.values()) {
             try {
-                Script script = new Script("1", scriptType, SCRIPT_LANG, null);
+                Script script = new Script("1", scriptType, MockScriptEngine.NAME, null);
                 scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_globally_disabled_op"), contextAndHeaders);
                 fail("script compilation should have been rejected");
             } catch (ScriptException e) {
-                assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [" + scriptType + "], operation [" + PLUGIN_NAME + "_custom_globally_disabled_op] and lang [" + SCRIPT_LANG + "] are disabled"));
+                assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [" + scriptType + "], operation [" + PLUGIN_NAME + "_custom_globally_disabled_op] and lang [" + MockScriptEngine.NAME + "] are disabled"));
             }
         }
     }
@@ -72,12 +67,12 @@ public class CustomScriptContextTests extends ESTestCase {
     public void testCustomScriptContextSettings() throws Exception {
         ContextAndHeaderHolder contextAndHeaders = new ContextAndHeaderHolder();
         ScriptService scriptService = makeScriptService();
-        Script script = new Script("1", ScriptService.ScriptType.INLINE, SCRIPT_LANG, null);
+        Script script = new Script("1", ScriptService.ScriptType.INLINE, MockScriptEngine.NAME, null);
         try {
             scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_exp_disabled_op"), contextAndHeaders);
             fail("script compilation should have been rejected");
         } catch (ScriptException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [inline], operation [" + PLUGIN_NAME + "_custom_exp_disabled_op] and lang [" + SCRIPT_LANG + "] are disabled"));
+            assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [inline], operation [" + PLUGIN_NAME + "_custom_exp_disabled_op] and lang [" + MockScriptEngine.NAME + "] are disabled"));
         }
 
         // still works for other script contexts
@@ -91,7 +86,7 @@ public class CustomScriptContextTests extends ESTestCase {
         ScriptService scriptService = makeScriptService();
         for (ScriptService.ScriptType scriptType : ScriptService.ScriptType.values()) {
             try {
-                Script script = new Script("1", scriptType, SCRIPT_LANG, null);
+                Script script = new Script("1", scriptType, MockScriptEngine.NAME, null);
                 scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "unknown"), contextAndHeaders);
                 fail("script compilation should have been rejected");
             } catch (IllegalArgumentException e) {
@@ -111,7 +106,7 @@ public class CustomScriptContextTests extends ESTestCase {
         ScriptService scriptService = makeScriptService();
         for (ScriptService.ScriptType scriptType : ScriptService.ScriptType.values()) {
             try {
-                Script script = new Script("1", scriptType, SCRIPT_LANG, null);
+                Script script = new Script("1", scriptType, MockScriptEngine.NAME, null);
                 scriptService.compile(script, context, contextAndHeaders);
                 fail("script compilation should have been rejected");
             } catch (IllegalArgumentException e) {
@@ -120,42 +115,4 @@ public class CustomScriptContextTests extends ESTestCase {
         }
     }
 
-    static class MockScriptEngine implements ScriptEngineService {
-        @Override
-        public String[] types() {
-            return new String[] {SCRIPT_LANG};
-        }
-        @Override
-        public String[] extensions() {
-            return types();
-        }
-        @Override
-        public boolean sandboxed() {
-            return true;
-        }
-        @Override
-        public Object compile(String script) {
-            return Integer.parseInt(script);
-        }
-        @Override
-        public ExecutableScript executable(CompiledScript compiledScript, @Nullable Map<String, Object> vars) {
-            return null;
-        }
-        @Override
-        public SearchScript search(CompiledScript compiledScript, SearchLookup lookup, @Nullable Map<String, Object> vars) {
-            return null;
-        }
-        @Override
-        public Object execute(CompiledScript compiledScript, Map<String, Object> vars) {
-            return null;
-        }
-        @Override
-        public Object unwrap(Object value) {
-            return null;
-        }
-        @Override
-        public void scriptRemoved(@Nullable CompiledScript script) {}
-        @Override
-        public void close() throws IOException {}
-    }
 }