Merge pull request elastic/elasticsearch#637 from rmuir/this_must_go

Remove remaining uses of setAccessible

Original commit: elastic/x-pack-elasticsearch@bb25e156da

shield/src/test/java/org/elasticsearch/http/netty/NettyHttpMockUtil.java

@@ -0,0 +1,22 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.http.netty;
+
+import static org.mockito.Mockito.mock;
+
+import org.elasticsearch.common.netty.OpenChannelsHandler;
+
+/** Allows setting a mock into NettyHttpServerTransport */
+public class NettyHttpMockUtil {
+    
+    /**
+     * We don't really need to start Netty for these tests, but we can't create a pipeline
+     * with a null handler. So we set it to a mock for tests.
+     */
+    public static void setOpenChannelsHandlerToMock(NettyHttpServerTransport transport) throws Exception {
+        transport.serverOpenChannels = mock(OpenChannelsHandler.class);
+    }
+}

shield/src/test/java/org/elasticsearch/shield/transport/netty/ShieldNettyHttpServerTransportTests.java

@@ -5,13 +5,12 @@
  */
 package org.elasticsearch.shield.transport.netty;
 
-import org.elasticsearch.common.netty.OpenChannelsHandler;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsFilter;
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.env.Environment;
-import org.elasticsearch.http.netty.NettyHttpServerTransport;
+import org.elasticsearch.http.netty.NettyHttpMockUtil;
 import org.elasticsearch.shield.ShieldSettingsFilter;
 import org.elasticsearch.shield.ssl.ServerSSLService;
 import org.elasticsearch.shield.transport.SSLClientAuth;
@@ -22,7 +21,6 @@ import org.jboss.netty.handler.ssl.SslHandler;
 import org.junit.Before;
 import org.junit.Test;
 
-import java.lang.reflect.Field;
 import java.nio.file.Path;
 import java.util.Locale;
 
@@ -50,7 +48,7 @@ public class ShieldNettyHttpServerTransportTests extends ESTestCase {
     public void testDefaultClientAuth() throws Exception {
         Settings settings = Settings.builder().put(ShieldNettyHttpServerTransport.HTTP_SSL_SETTING, true).build();
         ShieldNettyHttpServerTransport transport = new ShieldNettyHttpServerTransport(settings, mock(NetworkService.class), mock(BigArrays.class), mock(IPFilter.class), serverSSLService);
-        setOpenChannelsHandlerToMock(transport);
+        NettyHttpMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory();
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(false));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(false));
@@ -63,7 +61,7 @@ public class ShieldNettyHttpServerTransportTests extends ESTestCase {
                 .put(ShieldNettyHttpServerTransport.HTTP_SSL_SETTING, true)
                 .put(ShieldNettyHttpServerTransport.HTTP_CLIENT_AUTH_SETTING, value).build();
         ShieldNettyHttpServerTransport transport = new ShieldNettyHttpServerTransport(settings, mock(NetworkService.class), mock(BigArrays.class), mock(IPFilter.class), serverSSLService);
-        setOpenChannelsHandlerToMock(transport);
+        NettyHttpMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory();
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(false));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(true));
@@ -76,7 +74,7 @@ public class ShieldNettyHttpServerTransportTests extends ESTestCase {
                 .put(ShieldNettyHttpServerTransport.HTTP_SSL_SETTING, true)
                 .put(ShieldNettyHttpServerTransport.HTTP_CLIENT_AUTH_SETTING, value).build();
         ShieldNettyHttpServerTransport transport = new ShieldNettyHttpServerTransport(settings, mock(NetworkService.class), mock(BigArrays.class), mock(IPFilter.class), serverSSLService);
-        setOpenChannelsHandlerToMock(transport);
+        NettyHttpMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory();
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(true));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(false));
@@ -89,19 +87,9 @@ public class ShieldNettyHttpServerTransportTests extends ESTestCase {
                 .put(ShieldNettyHttpServerTransport.HTTP_SSL_SETTING, true)
                 .put(ShieldNettyHttpServerTransport.HTTP_CLIENT_AUTH_SETTING, value).build();
         ShieldNettyHttpServerTransport transport = new ShieldNettyHttpServerTransport(settings, mock(NetworkService.class), mock(BigArrays.class), mock(IPFilter.class), serverSSLService);
-        setOpenChannelsHandlerToMock(transport);
+        NettyHttpMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory();
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(false));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(false));
     }
-
-    /*
-     * We don't really need to start Netty for these tests, but we can't create a pipeline
-     * with a null handler. So we set it to a mock for this test using reflection.
-     */
-    private void setOpenChannelsHandlerToMock(NettyHttpServerTransport transport) throws Exception {
-        Field serverOpenChannels = NettyHttpServerTransport.class.getDeclaredField("serverOpenChannels");
-        serverOpenChannels.setAccessible(true);
-        serverOpenChannels.set(transport, mock(OpenChannelsHandler.class));
-    }
 }

shield/src/test/java/org/elasticsearch/shield/transport/netty/ShieldNettyTransportTests.java

@@ -7,7 +7,6 @@ package org.elasticsearch.shield.transport.netty;
 
 import org.elasticsearch.Version;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
-import org.elasticsearch.common.netty.OpenChannelsHandler;
 import org.elasticsearch.common.network.NetworkService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsFilter;
@@ -19,13 +18,12 @@ import org.elasticsearch.shield.ssl.ServerSSLService;
 import org.elasticsearch.shield.transport.SSLClientAuth;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.threadpool.ThreadPool;
-import org.elasticsearch.transport.netty.NettyTransport;
+import org.elasticsearch.transport.netty.NettyMockUtil;
 import org.jboss.netty.channel.ChannelPipelineFactory;
 import org.jboss.netty.handler.ssl.SslHandler;
 import org.junit.Before;
 import org.junit.Test;
 
-import java.lang.reflect.Field;
 import java.nio.file.Path;
 import java.util.Locale;
 
@@ -57,7 +55,7 @@ public class ShieldNettyTransportTests extends ESTestCase {
     public void testThatSSLCanBeDisabledByProfile() throws Exception {
         Settings settings = settingsBuilder().put("shield.transport.ssl", true).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", settingsBuilder().put("shield.ssl", false).build());
         assertThat(factory.getPipeline().get(SslHandler.class), nullValue());
     }
@@ -66,7 +64,7 @@ public class ShieldNettyTransportTests extends ESTestCase {
     public void testThatSSLCanBeEnabledByProfile() throws Exception {
         Settings settings = settingsBuilder().put("shield.transport.ssl", false).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", settingsBuilder().put("shield.ssl", true).build());
         assertThat(factory.getPipeline().get(SslHandler.class), notNullValue());
     }
@@ -75,7 +73,7 @@ public class ShieldNettyTransportTests extends ESTestCase {
     public void testThatProfileTakesDefaultSSLSetting() throws Exception {
         Settings settings = settingsBuilder().put("shield.transport.ssl", true).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", Settings.EMPTY);
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine(), notNullValue());
     }
@@ -84,7 +82,7 @@ public class ShieldNettyTransportTests extends ESTestCase {
     public void testDefaultClientAuth() throws Exception {
         Settings settings = settingsBuilder().put("shield.transport.ssl", true).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", Settings.EMPTY);
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(true));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(false));
@@ -97,7 +95,7 @@ public class ShieldNettyTransportTests extends ESTestCase {
                 .put("shield.transport.ssl", true)
                 .put(ShieldNettyTransport.TRANSPORT_CLIENT_AUTH_SETTING, value).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", Settings.EMPTY);
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(true));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(false));
@@ -110,7 +108,7 @@ public class ShieldNettyTransportTests extends ESTestCase {
                 .put("shield.transport.ssl", true)
                 .put(ShieldNettyTransport.TRANSPORT_CLIENT_AUTH_SETTING, value).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", Settings.EMPTY);
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(false));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(false));
@@ -123,7 +121,7 @@ public class ShieldNettyTransportTests extends ESTestCase {
                 .put("shield.transport.ssl", true)
                 .put(ShieldNettyTransport.TRANSPORT_CLIENT_AUTH_SETTING, value).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", Settings.EMPTY);
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(false));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(true));
@@ -134,7 +132,7 @@ public class ShieldNettyTransportTests extends ESTestCase {
         String value = randomFrom(SSLClientAuth.REQUIRED.name(), SSLClientAuth.REQUIRED.name().toLowerCase(Locale.ROOT), "true", "TRUE");
         Settings settings = settingsBuilder().put("shield.transport.ssl", true).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", Settings.builder().put(ShieldNettyTransport.TRANSPORT_PROFILE_CLIENT_AUTH_SETTING, value).build());
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(true));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(false));
@@ -145,7 +143,7 @@ public class ShieldNettyTransportTests extends ESTestCase {
         String value = randomFrom(SSLClientAuth.NO.name(), "false", "FALSE", SSLClientAuth.NO.name().toLowerCase(Locale.ROOT));
         Settings settings = settingsBuilder().put("shield.transport.ssl", true).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", Settings.builder().put(ShieldNettyTransport.TRANSPORT_PROFILE_CLIENT_AUTH_SETTING, value).build());
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(false));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(false));
@@ -156,19 +154,9 @@ public class ShieldNettyTransportTests extends ESTestCase {
         String value = randomFrom(SSLClientAuth.OPTIONAL.name(), SSLClientAuth.OPTIONAL.name().toLowerCase(Locale.ROOT));
         Settings settings = settingsBuilder().put("shield.transport.ssl", true).build();
         ShieldNettyTransport transport = new ShieldNettyTransport(settings, mock(ThreadPool.class), mock(NetworkService.class), mock(BigArrays.class), Version.CURRENT, null, serverSSLService, clientSSLService, settingsFilter, mock(NamedWriteableRegistry.class));
-        setOpenChannelsHandlerToMock(transport);
+        NettyMockUtil.setOpenChannelsHandlerToMock(transport);
         ChannelPipelineFactory factory = transport.configureServerChannelPipelineFactory("client", Settings.builder().put(ShieldNettyTransport.TRANSPORT_PROFILE_CLIENT_AUTH_SETTING, value).build());
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getNeedClientAuth(), is(false));
         assertThat(factory.getPipeline().get(SslHandler.class).getEngine().getWantClientAuth(), is(true));
     }
-
-    /*
-     * We don't really need to start Netty for these tests, but we can't create a pipeline
-     * with a null handler. So we set it to a mock for this test using reflection.
-     */
-    private void setOpenChannelsHandlerToMock(NettyTransport transport) throws Exception {
-        Field serverOpenChannels = NettyTransport.class.getDeclaredField("serverOpenChannels");
-        serverOpenChannels.setAccessible(true);
-        serverOpenChannels.set(transport, mock(OpenChannelsHandler.class));
-    }
 }

shield/src/test/java/org/elasticsearch/transport/netty/NettyMockUtil.java

@@ -0,0 +1,21 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.transport.netty;
+
+import org.elasticsearch.common.netty.OpenChannelsHandler;
+
+import static org.mockito.Mockito.mock;
+
+/** Allows setting a mock into NettyTransport */
+public class NettyMockUtil {
+    /**
+     * We don't really need to start Netty for these tests, but we can't create a pipeline
+     * with a null handler. So we set it to a mock for tests.
+     */
+    public static void setOpenChannelsHandlerToMock(NettyTransport transport) throws Exception {
+        transport.serverOpenChannels = mock(OpenChannelsHandler.class);
+    }
+}

watcher/src/main/java/org/elasticsearch/watcher/WatcherService.java

@@ -42,7 +42,8 @@ public class WatcherService extends AbstractComponent {
     private final WatchLockService watchLockService;
     private final ExecutionService executionService;
     private final WatcherIndexTemplateRegistry watcherIndexTemplateRegistry;
-    private final AtomicReference<WatcherState> state = new AtomicReference<>(WatcherState.STOPPED);
+    // package-private for testing
+    final AtomicReference<WatcherState> state = new AtomicReference<>(WatcherState.STOPPED);
 
     @Inject
     public WatcherService(Settings settings, Clock clock, TriggerService triggerService, WatchStore watchStore,

watcher/src/test/java/org/elasticsearch/watcher/WatcherServiceTests.java

@@ -62,9 +62,7 @@ public class WatcherServiceTests extends ESTestCase {
         clock = new ClockMock();
         WatcherIndexTemplateRegistry watcherIndexTemplateRegistry = mock(WatcherIndexTemplateRegistry.class);
         watcherService = new WatcherService(Settings.EMPTY, clock, triggerService, watchStore, watchParser, executionService, watchLockService, watcherIndexTemplateRegistry);
-        Field field = WatcherService.class.getDeclaredField("state");
+        AtomicReference<WatcherState> state = watcherService.state;
-        field.setAccessible(true);
-        AtomicReference<WatcherState> state = (AtomicReference<WatcherState>) field.get(watcherService);
         state.set(WatcherState.STARTED);
     }
 

watcher/src/test/java/org/elasticsearch/watcher/test/WatcherTestUtils.java

@@ -243,10 +243,7 @@ public final class WatcherTestUtils {
         Set<ScriptEngineService> engineServiceSet = new HashSet<>();
         engineServiceSet.add(mustacheScriptEngineService);
         engineServiceSet.add(groovyScriptEngineService);
-        Class scriptContextRegistryClass = Class.forName("org.elasticsearch.script.ScriptContextRegistry");
+        ScriptContextRegistry registry = new ScriptContextRegistry(Arrays.asList(ScriptServiceProxy.INSTANCE));
-        Constructor scriptContextRegistryConstructor = scriptContextRegistryClass.getDeclaredConstructors()[0];
-        scriptContextRegistryConstructor.setAccessible(true);
-        ScriptContextRegistry registry = (ScriptContextRegistry) scriptContextRegistryConstructor.newInstance(Arrays.asList(ScriptServiceProxy.INSTANCE));
 
         return  ScriptServiceProxy.of(new ScriptService(settings, new Environment(settings), engineServiceSet, new ResourceWatcherService(settings, tp), registry));
     }