Merge pull request #16065 from nik9000/xlint_mustache

Cleanup ContextAndHeaderTransportTests

core/src/main/java/org/elasticsearch/action/support/ActionFilter.java

@@ -40,13 +40,13 @@ public interface ActionFilter {
      * Enables filtering the execution of an action on the request side, either by sending a response through the
      * {@link ActionListener} or by continuing the execution through the given {@link ActionFilterChain chain}
      */
-    void apply(Task task, String action, ActionRequest request, ActionListener listener, ActionFilterChain chain);
+    void apply(Task task, String action, ActionRequest<?> request, ActionListener<?> listener, ActionFilterChain chain);
 
     /**
      * Enables filtering the execution of an action on the response side, either by sending a response through the
      * {@link ActionListener} or by continuing the execution through the given {@link ActionFilterChain chain}
      */
-    void apply(String action, ActionResponse response, ActionListener listener, ActionFilterChain chain);
+    void apply(String action, ActionResponse response, ActionListener<?> listener, ActionFilterChain chain);
 
     /**
      * A simple base class for injectable action filters that spares the implementation from handling the
@@ -60,7 +60,7 @@ public interface ActionFilter {
         }
 
         @Override
-        public final void apply(Task task, String action, ActionRequest request, ActionListener listener, ActionFilterChain chain) {
+        public final void apply(Task task, String action, ActionRequest<?> request, ActionListener<?> listener, ActionFilterChain chain) {
             if (apply(action, request, listener)) {
                 chain.proceed(task, action, request, listener);
             }
@@ -70,10 +70,10 @@ public interface ActionFilter {
          * Applies this filter and returns {@code true} if the execution chain should proceed, or {@code false}
          * if it should be aborted since the filter already handled the request and called the given listener.
          */
-        protected abstract boolean apply(String action, ActionRequest request, ActionListener listener);
+        protected abstract boolean apply(String action, ActionRequest<?> request, ActionListener<?> listener);
 
         @Override
-        public final void apply(String action, ActionResponse response, ActionListener listener, ActionFilterChain chain) {
+        public final void apply(String action, ActionResponse response, ActionListener<?> listener, ActionFilterChain chain) {
             if (apply(action, response, listener)) {
                 chain.proceed(action, response, listener);
             }
@@ -83,6 +83,6 @@ public interface ActionFilter {
          * Applies this filter and returns {@code true} if the execution chain should proceed, or {@code false}
          * if it should be aborted since the filter already handled the response by calling the given listener.
          */
-        protected abstract boolean apply(String action, ActionResponse response, ActionListener listener);
+        protected abstract boolean apply(String action, ActionResponse response, ActionListener<?> listener);
     }
 }

modules/lang-groovy/src/test/java/org/elasticsearch/messy/tests/ContextAndHeaderTransportTests.java

@@ -19,18 +19,10 @@
 
 package org.elasticsearch.messy.tests;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.Locale;
-import java.util.concurrent.CopyOnWriteArrayList;
-
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClients;
 import org.elasticsearch.action.Action;
 import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.action.ActionModule;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestBuilder;
 import org.elasticsearch.action.ActionResponse;
@@ -42,13 +34,9 @@ import org.elasticsearch.action.indexedscripts.put.PutIndexedScriptResponse;
 import org.elasticsearch.action.percolate.PercolateResponse;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchResponse;
-import org.elasticsearch.action.support.ActionFilter;
 import org.elasticsearch.action.termvectors.MultiTermVectorsRequest;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.client.FilterClient;
-import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.lucene.search.function.CombineFunction;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.http.HttpServerTransport;
@@ -66,6 +54,7 @@ import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptService.ScriptType;
 import org.elasticsearch.script.groovy.GroovyPlugin;
 import org.elasticsearch.script.groovy.GroovyScriptEngineService;
+import org.elasticsearch.test.ActionRecordingPlugin;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
 import org.elasticsearch.test.rest.client.http.HttpRequestBuilder;
@@ -73,6 +62,10 @@ import org.elasticsearch.test.rest.client.http.HttpResponse;
 import org.junit.After;
 import org.junit.Before;
 
+import java.util.Collection;
+import java.util.List;
+import java.util.Locale;
+
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
 import static org.elasticsearch.common.settings.Settings.settingsBuilder;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -89,7 +82,6 @@ import static org.hamcrest.Matchers.is;
 
 @ClusterScope(scope = SUITE)
 public class ContextAndHeaderTransportTests extends ESIntegTestCase {
-    private static final List<ActionRequest> requests =  new CopyOnWriteArrayList<>();
     private String randomHeaderKey = randomAsciiOfLength(10);
     private String randomHeaderValue = randomAsciiOfLength(20);
     private String queryIndex = "query-" + randomAsciiOfLength(10).toLowerCase(Locale.ROOT);
@@ -106,7 +98,7 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
 
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
-        return pluginList(ActionLoggingPlugin.class, GroovyPlugin.class);
+        return pluginList(ActionRecordingPlugin.class, GroovyPlugin.class);
     }
 
     @Before
@@ -128,7 +120,7 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
                 .setSettings(settings).addMapping("type", mapping));
         ensureGreen(queryIndex, lookupIndex);
 
-        requests.clear();
+        ActionRecordingPlugin.clear();
     }
 
     @After
@@ -193,7 +185,7 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
                 .get();
         assertNoFailures(searchResponse);
         assertHitCount(searchResponse, 1);
-        assertThat(requests, hasSize(greaterThan(0)));
+        assertThat(ActionRecordingPlugin.allRequests(), hasSize(greaterThan(0)));
 
         assertGetRequestsContainHeaders();
     }
@@ -281,7 +273,7 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
                 .execute();
 
         assertThat(response, hasStatus(OK));
-        List<SearchRequest> searchRequests = getRequests(SearchRequest.class);
+        List<SearchRequest> searchRequests = ActionRecordingPlugin.requestsOfType(SearchRequest.class);
         assertThat(searchRequests, hasSize(greaterThan(0)));
         for (SearchRequest searchRequest : searchRequests) {
             assertThat(searchRequest.hasHeader(releventHeaderName), is(true));
@@ -290,20 +282,9 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
         }
     }
 
-    private <T> List<T> getRequests(Class<T> clazz) {
+    private void assertRequestsContainHeader(Class<? extends ActionRequest<?>> clazz) {
-        List<T> results = new ArrayList<>();
+        List<? extends ActionRequest<?>> classRequests = ActionRecordingPlugin.requestsOfType(clazz);
-        for (ActionRequest request : requests) {
+        for (ActionRequest<?> request : classRequests) {
-            if (request.getClass().equals(clazz)) {
-                results.add((T) request);
-            }
-        }
-
-        return results;
-    }
-
-    private void assertRequestsContainHeader(Class<? extends ActionRequest> clazz) {
-        List<? extends ActionRequest> classRequests = getRequests(clazz);
-        for (ActionRequest request : classRequests) {
             assertRequestContainsHeader(request);
         }
     }
@@ -313,7 +294,7 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
     }
 
     private void assertGetRequestsContainHeaders(String index) {
-        List<GetRequest> getRequests = getRequests(GetRequest.class);
+        List<GetRequest> getRequests = ActionRecordingPlugin.requestsOfType(GetRequest.class);
         assertThat(getRequests, hasSize(greaterThan(0)));
 
         for (GetRequest request : getRequests) {
@@ -324,7 +305,7 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
         }
     }
 
-    private void assertRequestContainsHeader(ActionRequest request) {
+    private void assertRequestContainsHeader(ActionRequest<?> request) {
         String msg = String.format(Locale.ROOT, "Expected header %s to be in request %s", randomHeaderKey, request.getClass().getName());
         if (request instanceof IndexRequest) {
             IndexRequest indexRequest = (IndexRequest) request;
@@ -352,58 +333,4 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
 
         return filterClient;
     }
-
-    public static class ActionLoggingPlugin extends Plugin {
-
-        @Override
-        public String name() {
-            return "test-action-logging";
-        }
-
-        @Override
-        public String description() {
-            return "Test action logging";
-        }
-
-        @Override
-        public Collection<Module> nodeModules() {
-            return Collections.<Module>singletonList(new ActionLoggingModule());
-        }
-
-        public void onModule(ActionModule module) {
-            module.registerFilter(LoggingFilter.class);
-        }
-    }
-
-    public static class ActionLoggingModule extends AbstractModule {
-        @Override
-        protected void configure() {
-            bind(LoggingFilter.class).asEagerSingleton();
-        }
-
-    }
-
-    public static class LoggingFilter extends ActionFilter.Simple {
-
-        @Inject
-        public LoggingFilter(Settings settings) {
-            super(settings);
-        }
-
-        @Override
-        public int order() {
-            return 999;
-        }
-
-        @Override
-        protected boolean apply(String action, ActionRequest request, ActionListener listener) {
-            requests.add(request);
-            return true;
-        }
-
-        @Override
-        protected boolean apply(String action, ActionResponse response, ActionListener listener) {
-            return true;
-        }
-    }
 }

modules/lang-mustache/build.gradle

@@ -26,8 +26,6 @@ dependencies {
   compile "com.github.spullara.mustache.java:compiler:0.9.1"
 }
 
-compileTestJava.options.compilerArgs << '-Xlint:-rawtypes,-unchecked'
-
 integTest {
   cluster {
     systemProperty 'es.script.inline', 'on'

modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/ContextAndHeaderTransportTests.java

@@ -19,19 +19,8 @@
 
 package org.elasticsearch.messy.tests;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.concurrent.CopyOnWriteArrayList;
-
 import org.elasticsearch.action.Action;
 import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.action.ActionModule;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestBuilder;
 import org.elasticsearch.action.ActionResponse;
@@ -39,17 +28,12 @@ import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
 import org.elasticsearch.action.admin.indices.refresh.RefreshRequest;
 import org.elasticsearch.action.get.GetRequest;
 import org.elasticsearch.action.index.IndexRequest;
-import org.elasticsearch.action.index.IndexRequestBuilder;
 import org.elasticsearch.action.indexedscripts.put.PutIndexedScriptRequest;
 import org.elasticsearch.action.indexedscripts.put.PutIndexedScriptResponse;
 import org.elasticsearch.action.search.SearchRequestBuilder;
 import org.elasticsearch.action.search.SearchResponse;
-import org.elasticsearch.action.support.ActionFilter;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.client.FilterClient;
-import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
@@ -62,11 +46,20 @@ import org.elasticsearch.script.mustache.MustacheScriptEngineService;
 import org.elasticsearch.search.suggest.Suggest;
 import org.elasticsearch.search.suggest.SuggestBuilder;
 import org.elasticsearch.search.suggest.phrase.PhraseSuggestionBuilder;
+import org.elasticsearch.test.ActionRecordingPlugin;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
 import org.junit.After;
 import org.junit.Before;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
 import static org.elasticsearch.common.settings.Settings.settingsBuilder;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -84,7 +77,6 @@ import static org.hamcrest.Matchers.is;
 
 @ClusterScope(scope = SUITE)
 public class ContextAndHeaderTransportTests extends ESIntegTestCase {
-    private static final List<ActionRequest> requests =  new CopyOnWriteArrayList<>();
     private String randomHeaderKey = randomAsciiOfLength(10);
     private String randomHeaderValue = randomAsciiOfLength(20);
     private String queryIndex = "query-" + randomAsciiOfLength(10).toLowerCase(Locale.ROOT);
@@ -101,7 +93,7 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
 
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
-        return pluginList(ActionLoggingPlugin.class, MustachePlugin.class);
+        return pluginList(ActionRecordingPlugin.class, MustachePlugin.class);
     }
 
     @Before
@@ -122,14 +114,13 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
         assertAcked(transportClient().admin().indices().prepareCreate(queryIndex)
                 .setSettings(settings).addMapping("type", mapping));
         ensureGreen(queryIndex, lookupIndex);
-
-        requests.clear();
     }
 
     @After
     public void checkAllRequestsContainHeaders() {
         assertRequestsContainHeader(IndexRequest.class);
         assertRequestsContainHeader(RefreshRequest.class);
+        ActionRecordingPlugin.clear();
     }
 
     public void testThatIndexedScriptGetRequestInTemplateQueryContainsContextAndHeaders() throws Exception {
@@ -216,7 +207,6 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
         titles.add("Representative Government");
         titles.add("Election");
 
-        List<IndexRequestBuilder> builders = new ArrayList<>();
         for (String title: titles) {
             transportClient().prepareIndex("test", "type1").setSource("title", title).get();
         }
@@ -272,30 +262,15 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
         assertRequestsContainHeader(PutIndexedScriptRequest.class);
     }
 
-    private <T> List<T> getRequests(Class<T> clazz) {
+    private void assertRequestsContainHeader(Class<? extends ActionRequest<?>> clazz) {
-        List<T> results = new ArrayList<>();
+        List<? extends ActionRequest<?>> classRequests = ActionRecordingPlugin.requestsOfType(clazz);
-        for (ActionRequest request : requests) {
+        for (ActionRequest<?> request : classRequests) {
-            if (request.getClass().equals(clazz)) {
-                results.add((T) request);
-            }
-        }
-
-        return results;
-    }
-
-    private void assertRequestsContainHeader(Class<? extends ActionRequest> clazz) {
-        List<? extends ActionRequest> classRequests = getRequests(clazz);
-        for (ActionRequest request : classRequests) {
             assertRequestContainsHeader(request);
         }
     }
 
-    private void assertGetRequestsContainHeaders() {
-        assertGetRequestsContainHeaders(this.lookupIndex);
-    }
-
     private void assertGetRequestsContainHeaders(String index) {
-        List<GetRequest> getRequests = getRequests(GetRequest.class);
+        List<GetRequest> getRequests = ActionRecordingPlugin.requestsOfType(GetRequest.class);
         assertThat(getRequests, hasSize(greaterThan(0)));
 
         for (GetRequest request : getRequests) {
@@ -306,7 +281,7 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
         }
     }
 
-    private void assertRequestContainsHeader(ActionRequest request) {
+    private void assertRequestContainsHeader(ActionRequest<?> request) {
         String msg = String.format(Locale.ROOT, "Expected header %s to be in request %s", randomHeaderKey, request.getClass().getName());
         if (request instanceof IndexRequest) {
             IndexRequest indexRequest = (IndexRequest) request;
@@ -334,58 +309,4 @@ public class ContextAndHeaderTransportTests extends ESIntegTestCase {
 
         return filterClient;
     }
-
-    public static class ActionLoggingPlugin extends Plugin {
-
-        @Override
-        public String name() {
-            return "test-action-logging";
-        }
-
-        @Override
-        public String description() {
-            return "Test action logging";
-        }
-
-        @Override
-        public Collection<Module> nodeModules() {
-            return Collections.<Module>singletonList(new ActionLoggingModule());
-        }
-
-        public void onModule(ActionModule module) {
-            module.registerFilter(LoggingFilter.class);
-        }
-    }
-
-    public static class ActionLoggingModule extends AbstractModule {
-        @Override
-        protected void configure() {
-            bind(LoggingFilter.class).asEagerSingleton();
-        }
-
-    }
-
-    public static class LoggingFilter extends ActionFilter.Simple {
-
-        @Inject
-        public LoggingFilter(Settings settings) {
-            super(settings);
-        }
-
-        @Override
-        public int order() {
-            return 999;
-        }
-
-        @Override
-        protected boolean apply(String action, ActionRequest request, ActionListener listener) {
-            requests.add(request);
-            return true;
-        }
-
-        @Override
-        protected boolean apply(String action, ActionResponse response, ActionListener listener) {
-            return true;
-        }
-    }
 }

test/framework/src/main/java/org/elasticsearch/test/ActionRecordingPlugin.java

@@ -0,0 +1,138 @@
+/*
+ * 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;
+
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.ActionModule;
+import org.elasticsearch.action.ActionRequest;
+import org.elasticsearch.action.ActionResponse;
+import org.elasticsearch.action.support.ActionFilter;
+import org.elasticsearch.common.inject.AbstractModule;
+import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.inject.Module;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.plugins.Plugin;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+
+import static java.util.Collections.unmodifiableList;
+
+/**
+ * Plugin that registers a filter that records actions.
+ */
+public class ActionRecordingPlugin extends Plugin {
+    /**
+     * Fetch all the requests recorded by the test plugin. The list is an
+     * immutable, moment in time snapshot.
+     */
+    public static List<ActionRequest<?>> allRequests() {
+        List<ActionRequest<?>> requests = new ArrayList<>();
+        for (RecordingFilter filter : ESIntegTestCase.internalCluster().getInstances(RecordingFilter.class)) {
+            requests.addAll(filter.requests);
+        }
+        return unmodifiableList(requests);
+    }
+
+    /**
+     * Fetch all requests recorded by the test plugin of a certain type. The
+     * list is an immutable, moment in time snapshot.
+     */
+    public static <T> List<T> requestsOfType(Class<T> type) {
+        List<T> requests = new ArrayList<>();
+        for (RecordingFilter filter : ESIntegTestCase.internalCluster().getInstances(RecordingFilter.class)) {
+            for (ActionRequest<?> request : filter.requests) {
+                if (type.isInstance(request)) {
+                    requests.add(type.cast(request));
+                }
+            }
+        }
+        return unmodifiableList(requests);
+    }
+
+    /**
+     * Clear all the recorded requests. Use between test methods that shared a
+     * suite scoped cluster.
+     */
+    public static void clear() {
+        for (RecordingFilter filter : ESIntegTestCase.internalCluster().getInstances(RecordingFilter.class)) {
+            filter.requests.clear();
+        }
+    }
+
+    @Override
+    public String name() {
+        return "test-action-logging";
+    }
+
+    @Override
+    public String description() {
+        return "Test action logging";
+    }
+
+    @Override
+    public Collection<Module> nodeModules() {
+        return Collections.<Module>singletonList(new ActionRecordingModule());
+    }
+
+    public void onModule(ActionModule module) {
+        module.registerFilter(RecordingFilter.class);
+    }
+
+    public static class ActionRecordingModule extends AbstractModule {
+        @Override
+        protected void configure() {
+            bind(RecordingFilter.class).asEagerSingleton();
+        }
+
+    }
+
+    public static class RecordingFilter extends ActionFilter.Simple {
+        private final List<ActionRequest<?>> requests = new CopyOnWriteArrayList<>();
+
+        @Inject
+        public RecordingFilter(Settings settings) {
+            super(settings);
+        }
+
+        public List<ActionRequest<?>> getRequests() {
+            return new ArrayList<>(requests);
+        }
+
+        @Override
+        public int order() {
+            return 999;
+        }
+
+        @Override
+        protected boolean apply(String action, ActionRequest<?> request, ActionListener<?> listener) {
+            requests.add(request);
+            return true;
+        }
+
+        @Override
+        protected boolean apply(String action, ActionResponse response, ActionListener<?> listener) {
+            return true;
+        }
+    }
+}