From 41412dd9bd661c78bfd0a57ac5092741ee03f84b Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 4 Jul 2018 11:32:35 +0200 Subject: [PATCH 1/5] Detach Transport from TransportService (#31727) Today TransportService is tightly coupled with Transport since it requires an instance of TransportService in order to receive responses and send requests. This is mainly due to the Request and Response handlers being maintained in TransportService but also because of the lack of a proper callback interface. This change moves request handler registry and response handler registration into Transport and adds all necessary methods to `TransportConnectionListener` in order to remove the `TransportService` dependency from `Transport` Transport now accepts one or more `TransportConnectionListener` instances that are executed sequentially in a blocking fashion. --- .../netty4/Netty4ScheduledPingTests.java | 4 +- .../discovery/zen/FaultDetection.java | 18 +- .../elasticsearch/transport/TcpTransport.java | 137 +++++-- .../elasticsearch/transport/Transport.java | 150 +++++++- .../TransportConnectionListener.java | 65 +++- .../transport/TransportService.java | 356 ++++++++---------- .../node/tasks/CancellableTasksTests.java | 2 +- .../node/tasks/TransportTasksActionTests.java | 28 +- .../action/main/MainActionTests.java | 6 +- .../TransportMultiSearchActionTests.java | 6 +- .../TransportMasterNodeActionTests.java | 26 +- .../BroadcastReplicationTests.java | 2 +- .../TransportReplicationActionTests.java | 42 ++- .../TransportWriteActionTests.java | 11 +- .../transport/FailAndRetryMockTransport.java | 52 ++- .../cluster/NodeConnectionsServiceTests.java | 45 ++- .../indices/cluster/ClusterStateChanges.java | 2 +- ...ClusterStateServiceRandomUpdatesTests.java | 3 +- .../transport/ActionNamesIT.java | 54 --- .../transport/TransportActionProxyTests.java | 28 +- .../elasticsearch/test/ESIntegTestCase.java | 26 ++ .../test/transport/CapturingTransport.java | 62 ++- .../test/transport/MockTransportService.java | 42 ++- .../AbstractSimpleTransportTestCase.java | 152 ++++---- .../action/TransportXPackInfoActionTests.java | 5 +- .../SecurityServerTransportServiceTests.java | 18 +- .../role/TransportDeleteRoleActionTests.java | 13 +- .../role/TransportGetRolesActionTests.java | 17 +- .../role/TransportPutRoleActionTests.java | 14 +- .../TransportGetRoleMappingsActionTests.java | 5 +- .../TransportPutRoleMappingActionTests.java | 5 +- ...sportSamlInvalidateSessionActionTests.java | 5 +- .../saml/TransportSamlLogoutActionTests.java | 3 +- .../TransportAuthenticateActionTests.java | 13 +- .../TransportChangePasswordActionTests.java | 20 +- .../user/TransportDeleteUserActionTests.java | 21 +- .../user/TransportGetUsersActionTests.java | 17 +- .../TransportHasPrivilegesActionTests.java | 8 +- .../user/TransportPutUserActionTests.java | 21 +- .../user/TransportSetEnabledActionTests.java | 21 +- 40 files changed, 910 insertions(+), 615 deletions(-) delete mode 100644 server/src/test/java/org/elasticsearch/transport/ActionNamesIT.java diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/transport/netty4/Netty4ScheduledPingTests.java b/modules/transport-netty4/src/test/java/org/elasticsearch/transport/netty4/Netty4ScheduledPingTests.java index b967a7ea41069..4aed64459dba5 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/transport/netty4/Netty4ScheduledPingTests.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/transport/netty4/Netty4ScheduledPingTests.java @@ -88,7 +88,7 @@ public void testScheduledPing() throws Exception { assertThat(nettyA.getPing().getFailedPings(), equalTo(0L)); assertThat(nettyB.getPing().getFailedPings(), equalTo(0L)); - serviceA.registerRequestHandler("sayHello", TransportRequest.Empty::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:sayHello", TransportRequest.Empty::new, ThreadPool.Names.GENERIC, new TransportRequestHandler() { @Override public void messageReceived(TransportRequest.Empty request, TransportChannel channel) { @@ -103,7 +103,7 @@ public void messageReceived(TransportRequest.Empty request, TransportChannel cha int rounds = scaledRandomIntBetween(100, 5000); for (int i = 0; i < rounds; i++) { - serviceB.submitRequest(nodeA, "sayHello", + serviceB.submitRequest(nodeA, "internal:sayHello", TransportRequest.Empty.INSTANCE, TransportRequestOptions.builder().withCompress(randomBoolean()).build(), new TransportResponseHandler() { @Override diff --git a/server/src/main/java/org/elasticsearch/discovery/zen/FaultDetection.java b/server/src/main/java/org/elasticsearch/discovery/zen/FaultDetection.java index 715e8be03efb2..5d9b1687e4295 100644 --- a/server/src/main/java/org/elasticsearch/discovery/zen/FaultDetection.java +++ b/server/src/main/java/org/elasticsearch/discovery/zen/FaultDetection.java @@ -28,6 +28,7 @@ import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportConnectionListener; import org.elasticsearch.transport.TransportService; @@ -93,13 +94,20 @@ public void close() { abstract void handleTransportDisconnect(DiscoveryNode node); private class FDConnectionListener implements TransportConnectionListener { - @Override - public void onNodeConnected(DiscoveryNode node) { - } - @Override public void onNodeDisconnected(DiscoveryNode node) { - handleTransportDisconnect(node); + AbstractRunnable runnable = new AbstractRunnable() { + @Override + public void onFailure(Exception e) { + logger.warn("failed to handle transport disconnect for node: {}", node); + } + + @Override + protected void doRun() { + handleTransportDisconnect(node); + } + }; + threadPool.generic().execute(runnable); } } diff --git a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java index 9f7dba5651d6f..118987e2fa099 100644 --- a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java +++ b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java @@ -37,6 +37,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.bytes.CompositeBytesReference; +import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.component.Lifecycle; import org.elasticsearch.common.compress.Compressor; @@ -97,10 +98,10 @@ import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -201,7 +202,7 @@ public abstract class TcpTransport extends AbstractLifecycleComponent implements protected final NetworkService networkService; protected final Set profileSettings; - private volatile TransportService transportService; + private final DelegatingTransportConnectionListener transportListener = new DelegatingTransportConnectionListener(); private final ConcurrentMap profileBoundAddresses = newConcurrentMap(); // node id to actual channel @@ -221,12 +222,13 @@ public abstract class TcpTransport extends AbstractLifecycleComponent implements protected final ConnectionProfile defaultConnectionProfile; private final ConcurrentMap pendingHandshakes = new ConcurrentHashMap<>(); - private final AtomicLong requestIdGenerator = new AtomicLong(); private final CounterMetric numHandshakes = new CounterMetric(); private static final String HANDSHAKE_ACTION_NAME = "internal:tcp/handshake"; private final MeanMetric readBytesMetric = new MeanMetric(); private final MeanMetric transmittedBytesMetric = new MeanMetric(); + private volatile Map requestHandlers = Collections.emptyMap(); + private final ResponseHandlers responseHandlers = new ResponseHandlers(); public TcpTransport(String transportName, Settings settings, ThreadPool threadPool, BigArrays bigArrays, CircuitBreakerService circuitBreakerService, NamedWriteableRegistry namedWriteableRegistry, @@ -283,6 +285,16 @@ protected void doStart() { } } + @Override + public void addConnectionListener(TransportConnectionListener listener) { + transportListener.listeners.add(listener); + } + + @Override + public boolean removeConnectionListener(TransportConnectionListener listener) { + return transportListener.listeners.remove(listener); + } + @Override public CircuitBreaker getInFlightRequestBreaker() { // We always obtain a fresh breaker to reflect changes to the breaker configuration. @@ -290,11 +302,11 @@ public CircuitBreaker getInFlightRequestBreaker() { } @Override - public void setTransportService(TransportService service) { - if (service.getRequestHandler(HANDSHAKE_ACTION_NAME) != null) { - throw new IllegalStateException(HANDSHAKE_ACTION_NAME + " is a reserved request handler and must not be registered"); + public synchronized void registerRequestHandler(RequestHandlerRegistry reg) { + if (requestHandlers.containsKey(reg.getAction())) { + throw new IllegalArgumentException("transport handlers for action " + reg.getAction() + " is already registered"); } - this.transportService = service; + requestHandlers = MapBuilder.newMapBuilder(requestHandlers).put(reg.getAction(), reg).immutableMap(); } private static class HandshakeResponseHandler implements TransportResponseHandler { @@ -479,7 +491,7 @@ public void close() { boolean block = lifecycle.stopped() && Transports.isTransportThread(Thread.currentThread()) == false; TcpChannel.closeChannels(channels, block); } finally { - transportService.onConnectionClosed(this); + transportListener.onConnectionClosed(this); } } } @@ -535,7 +547,7 @@ public void connectToNode(DiscoveryNode node, ConnectionProfile connectionProfil logger.debug("connected to node [{}]", node); } try { - transportService.onNodeConnected(node); + transportListener.onNodeConnected(node); } finally { if (nodeChannels.isClosed()) { // we got closed concurrently due to a disconnect or some other event on the channel. @@ -547,7 +559,7 @@ public void connectToNode(DiscoveryNode node, ConnectionProfile connectionProfil // try to remove it first either way one of the two wins even if the callback has run before we even added the // tuple to the map since in that case we remove it here again if (connectedNodes.remove(node, nodeChannels)) { - transportService.onNodeDisconnected(node); + transportListener.onNodeDisconnected(node); } throw new NodeNotConnectedException(node, "connection concurrently closed"); } @@ -649,7 +661,7 @@ public final NodeChannels openConnection(DiscoveryNode node, ConnectionProfile c // At this point we should construct the connection, notify the transport service, and attach close listeners to the // underlying channels. nodeChannels = new NodeChannels(node, channels, connectionProfile, version); - transportService.onConnectionOpened(nodeChannels); + transportListener.onConnectionOpened(nodeChannels); final NodeChannels finalNodeChannels = nodeChannels; final AtomicBoolean runOnce = new AtomicBoolean(false); Consumer onClose = c -> { @@ -692,7 +704,7 @@ private void disconnectFromNodeCloseAndNotify(DiscoveryNode node, NodeChannels n if (closeLock.readLock().tryLock()) { try { if (connectedNodes.remove(node, nodeChannels)) { - transportService.onNodeDisconnected(node); + transportListener.onNodeDisconnected(node); } } finally { closeLock.readLock().unlock(); @@ -719,7 +731,7 @@ public void disconnectFromNode(DiscoveryNode node) { } finally { closeLock.readLock().unlock(); if (nodeChannels != null) { // if we found it and removed it we close and notify - IOUtils.closeWhileHandlingException(nodeChannels, () -> transportService.onNodeDisconnected(node)); + IOUtils.closeWhileHandlingException(nodeChannels, () -> transportListener.onNodeDisconnected(node)); } } } @@ -976,7 +988,7 @@ protected final void doStop() { Map.Entry next = iterator.next(); try { IOUtils.closeWhileHandlingException(next.getValue()); - transportService.onNodeDisconnected(next.getKey()); + transportListener.onNodeDisconnected(next.getKey()); } finally { iterator.remove(); } @@ -1120,7 +1132,7 @@ private void sendRequestToChannel(final DiscoveryNode node, final TcpChannel cha final TransportRequestOptions finalOptions = options; // this might be called in a different thread SendListener onRequestSent = new SendListener(channel, stream, - () -> transportService.onRequestSent(node, requestId, action, request, finalOptions), message.length()); + () -> transportListener.onRequestSent(node, requestId, action, request, finalOptions), message.length()); internalSendMessage(channel, message, onRequestSent); addedReleaseListener = true; } finally { @@ -1174,7 +1186,7 @@ public void sendErrorResponse( final BytesReference header = buildHeader(requestId, status, nodeVersion, bytes.length()); CompositeBytesReference message = new CompositeBytesReference(header, bytes); SendListener onResponseSent = new SendListener(channel, null, - () -> transportService.onResponseSent(requestId, action, error), message.length()); + () -> transportListener.onResponseSent(requestId, action, error), message.length()); internalSendMessage(channel, message, onResponseSent); } } @@ -1223,7 +1235,7 @@ private void sendResponse( final TransportResponseOptions finalOptions = options; // this might be called in a different thread SendListener listener = new SendListener(channel, stream, - () -> transportService.onResponseSent(requestId, action, response, finalOptions), message.length()); + () -> transportListener.onResponseSent(requestId, action, response, finalOptions), message.length()); internalSendMessage(channel, message, listener); addedReleaseListener = true; } finally { @@ -1418,7 +1430,7 @@ public final void messageReceived(BytesReference reference, TcpChannel channel, if (isHandshake) { handler = pendingHandshakes.remove(requestId); } else { - TransportResponseHandler theHandler = transportService.onResponseReceived(requestId); + TransportResponseHandler theHandler = responseHandlers.onResponseReceived(requestId, transportListener); if (theHandler == null && TransportStatus.isError(status)) { handler = pendingHandshakes.remove(requestId); } else { @@ -1525,7 +1537,7 @@ protected String handleRequest(TcpChannel channel, String profileName, final Str features = Collections.emptySet(); } final String action = stream.readString(); - transportService.onRequestReceived(requestId, action); + transportListener.onRequestReceived(requestId, action); TransportChannel transportChannel = null; try { if (TransportStatus.isHandshake(status)) { @@ -1533,7 +1545,7 @@ protected String handleRequest(TcpChannel channel, String profileName, final Str sendResponse(version, features, channel, response, requestId, HANDSHAKE_ACTION_NAME, TransportResponseOptions.EMPTY, TransportStatus.setHandshake((byte) 0)); } else { - final RequestHandlerRegistry reg = transportService.getRequestHandler(action); + final RequestHandlerRegistry reg = getRequestHandler(action); if (reg == null) { throw new ActionNotFoundTransportException(action); } @@ -1640,7 +1652,7 @@ public void writeTo(StreamOutput out) throws IOException { protected Version executeHandshake(DiscoveryNode node, TcpChannel channel, TimeValue timeout) throws IOException, InterruptedException { numHandshakes.inc(); - final long requestId = newRequestId(); + final long requestId = responseHandlers.newRequestId(); final HandshakeResponseHandler handler = new HandshakeResponseHandler(channel); AtomicReference versionRef = handler.versionRef; AtomicReference exceptionRef = handler.exceptionRef; @@ -1690,11 +1702,6 @@ final long getNumHandshakes() { return numHandshakes.count(); // for testing } - @Override - public long newRequestId() { - return requestIdGenerator.incrementAndGet(); - } - /** * Called once the channel is closed for instance due to a disconnect or a closed socket etc. */ @@ -1838,4 +1845,82 @@ public ProfileSettings(Settings settings, String profileName) { PUBLISH_PORT_PROFILE.getConcreteSettingForNamespace(profileName).get(settings); } } + + private static final class DelegatingTransportConnectionListener implements TransportConnectionListener { + private final List listeners = new CopyOnWriteArrayList<>(); + + @Override + public void onRequestReceived(long requestId, String action) { + for (TransportConnectionListener listener : listeners) { + listener.onRequestReceived(requestId, action); + } + } + + @Override + public void onResponseSent(long requestId, String action, TransportResponse response, TransportResponseOptions finalOptions) { + for (TransportConnectionListener listener : listeners) { + listener.onResponseSent(requestId, action, response, finalOptions); + } + } + + @Override + public void onResponseSent(long requestId, String action, Exception error) { + for (TransportConnectionListener listener : listeners) { + listener.onResponseSent(requestId, action, error); + } + } + + @Override + public void onRequestSent(DiscoveryNode node, long requestId, String action, TransportRequest request, + TransportRequestOptions finalOptions) { + for (TransportConnectionListener listener : listeners) { + listener.onRequestSent(node, requestId, action, request, finalOptions); + } + } + + @Override + public void onNodeDisconnected(DiscoveryNode key) { + for (TransportConnectionListener listener : listeners) { + listener.onNodeDisconnected(key); + } + } + + @Override + public void onConnectionOpened(Connection nodeChannels) { + for (TransportConnectionListener listener : listeners) { + listener.onConnectionOpened(nodeChannels); + } + } + + @Override + public void onNodeConnected(DiscoveryNode node) { + for (TransportConnectionListener listener : listeners) { + listener.onNodeConnected(node); + } + } + + @Override + public void onConnectionClosed(Connection nodeChannels) { + for (TransportConnectionListener listener : listeners) { + listener.onConnectionClosed(nodeChannels); + } + } + + @Override + public void onResponseReceived(long requestId, ResponseContext holder) { + for (TransportConnectionListener listener : listeners) { + listener.onResponseReceived(requestId, holder); + } + } + } + + @Override + public final ResponseHandlers getResponseHandlers() { + return responseHandlers; + } + + @Override + public final RequestHandlerRegistry getRequestHandler(String action) { + return requestHandlers.get(action); + } } diff --git a/server/src/main/java/org/elasticsearch/transport/Transport.java b/server/src/main/java/org/elasticsearch/transport/Transport.java index 6ef698f1740b3..74235479657bf 100644 --- a/server/src/main/java/org/elasticsearch/transport/Transport.java +++ b/server/src/main/java/org/elasticsearch/transport/Transport.java @@ -29,18 +29,45 @@ import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.TransportAddress; +import org.elasticsearch.common.util.concurrent.ConcurrentCollections; +import org.elasticsearch.common.util.concurrent.ConcurrentMapLong; import java.io.Closeable; import java.io.IOException; import java.net.UnknownHostException; +import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Predicate; public interface Transport extends LifecycleComponent { Setting TRANSPORT_TCP_COMPRESS = Setting.boolSetting("transport.tcp.compress", false, Property.NodeScope); - void setTransportService(TransportService service); + /** + * Registers a new request handler + */ + void registerRequestHandler(RequestHandlerRegistry reg); + + /** + * Returns the registered request handler registry for the given action or null if it's not registered + * @param action the action to look up + */ + RequestHandlerRegistry getRequestHandler(String action); + + /** + * Adds a new event listener + * @param listener the listener to add + */ + void addConnectionListener(TransportConnectionListener listener); + + /** + * Removes an event listener + * @param listener the listener to remove + * @return true iff the listener was removed otherwise false + */ + boolean removeConnectionListener(TransportConnectionListener listener); /** * The address the transport is bound on. @@ -75,17 +102,15 @@ void connectToNode(DiscoveryNode node, ConnectionProfile connectionProfile, */ void disconnectFromNode(DiscoveryNode node); + /** + * Returns a list of all local adresses for this transport + */ List getLocalAddresses(); default CircuitBreaker getInFlightRequestBreaker() { return new NoopCircuitBreaker("in-flight-noop"); } - /** - * Returns a new request ID to use when sending a message via {@link Connection#sendRequest(long, String, - * TransportRequest, TransportRequestOptions)} - */ - long newRequestId(); /** * Returns a connection for the given node if the node is connected. * Connections returned from this method must not be closed. The lifecycle of this connection is maintained by the Transport @@ -107,6 +132,8 @@ default CircuitBreaker getInFlightRequestBreaker() { TransportStats getStats(); + ResponseHandlers getResponseHandlers(); + /** * A unidirectional connection to a {@link DiscoveryNode} */ @@ -118,6 +145,10 @@ interface Connection extends Closeable { /** * Sends the request to the node this connection is associated with + * @param requestId see {@link ResponseHandlers#add(ResponseContext)} for details + * @param action the action to execute + * @param request the request to send + * @param options request options to apply * @throws NodeNotConnectedException if the given node is not connected */ void sendRequest(long requestId, String action, TransportRequest request, TransportRequestOptions options) throws @@ -138,4 +169,111 @@ default Object getCacheKey() { return this; } } + + /** + * This class represents a response context that encapsulates the actual response handler, the action and the conneciton it was + * executed on. + */ + final class ResponseContext { + + private final TransportResponseHandler handler; + + private final Connection connection; + + private final String action; + + ResponseContext(TransportResponseHandler handler, Connection connection, String action) { + this.handler = handler; + this.connection = connection; + this.action = action; + } + + public TransportResponseHandler handler() { + return handler; + } + + public Connection connection() { + return this.connection; + } + + public String action() { + return this.action; + } + } + + /** + * This class is a registry that allows + */ + final class ResponseHandlers { + private final ConcurrentMapLong handlers = ConcurrentCollections + .newConcurrentMapLongWithAggressiveConcurrency(); + private final AtomicLong requestIdGenerator = new AtomicLong(); + + /** + * Returns true if the give request ID has a context associated with it. + */ + public boolean contains(long requestId) { + return handlers.containsKey(requestId); + } + + /** + * Removes and return the {@link ResponseContext} for the given request ID or returns + * null if no context is associated with this request ID. + */ + public ResponseContext remove(long requestId) { + return handlers.remove(requestId); + } + + /** + * Adds a new response context and associates it with a new request ID. + * @return the new request ID + * @see Connection#sendRequest(long, String, TransportRequest, TransportRequestOptions) + */ + public long add(ResponseContext holder) { + long requestId = newRequestId(); + ResponseContext existing = handlers.put(requestId, holder); + assert existing == null : "request ID already in use: " + requestId; + return requestId; + } + + /** + * Returns a new request ID to use when sending a message via {@link Connection#sendRequest(long, String, + * TransportRequest, TransportRequestOptions)} + */ + long newRequestId() { + return requestIdGenerator.incrementAndGet(); + } + + /** + * Removes and returns all {@link ResponseContext} instances that match the predicate + */ + public List prune(Predicate predicate) { + final List holders = new ArrayList<>(); + for (Map.Entry entry : handlers.entrySet()) { + ResponseContext holder = entry.getValue(); + if (predicate.test(holder)) { + ResponseContext remove = handlers.remove(entry.getKey()); + if (remove != null) { + holders.add(holder); + } + } + } + return holders; + } + + /** + * called by the {@link Transport} implementation when a response or an exception has been received for a previously + * sent request (before any processing or deserialization was done). Returns the appropriate response handler or null if not + * found. + */ + public TransportResponseHandler onResponseReceived(final long requestId, TransportConnectionListener listener) { + ResponseContext context = handlers.remove(requestId); + listener.onResponseReceived(requestId, context); + if (context == null) { + return null; + } else { + return context.handler(); + } + } + } } diff --git a/server/src/main/java/org/elasticsearch/transport/TransportConnectionListener.java b/server/src/main/java/org/elasticsearch/transport/TransportConnectionListener.java index de767986b9f09..0ee2ed5828d44 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportConnectionListener.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportConnectionListener.java @@ -21,26 +21,75 @@ import org.elasticsearch.cluster.node.DiscoveryNode; +/** + * A listener interface that allows to react on transport events. All methods may be + * executed on network threads. Consumers must fork in the case of long running or blocking + * operations. + */ public interface TransportConnectionListener { /** - * Called once a node connection is opened and registered. + * Called once a request is received + * @param requestId the internal request ID + * @param action the request action + * */ - default void onNodeConnected(DiscoveryNode node) {} + default void onRequestReceived(long requestId, String action) {} /** - * Called once a node connection is closed and unregistered. + * Called for every action response sent after the response has been passed to the underlying network implementation. + * @param requestId the request ID (unique per client) + * @param action the request action + * @param response the response send + * @param finalOptions the response options */ - default void onNodeDisconnected(DiscoveryNode node) {} + default void onResponseSent(long requestId, String action, TransportResponse response, TransportResponseOptions finalOptions) {} + + /*** + * Called for every failed action response after the response has been passed to the underlying network implementation. + * @param requestId the request ID (unique per client) + * @param action the request action + * @param error the error sent back to the caller + */ + default void onResponseSent(long requestId, String action, Exception error) {} /** - * Called once a node connection is closed. The connection might not have been registered in the - * transport as a shared connection to a specific node + * Called for every request sent to a server after the request has been passed to the underlying network implementation + * @param node the node the request was sent to + * @param requestId the internal request id + * @param action the action name + * @param request the actual request + * @param finalOptions the request options */ - default void onConnectionClosed(Transport.Connection connection) {} + default void onRequestSent(DiscoveryNode node, long requestId, String action, TransportRequest request, + TransportRequestOptions finalOptions) {} /** - * Called once a node connection is opened. + * Called once a connection was opened + * @param connection the connection */ default void onConnectionOpened(Transport.Connection connection) {} + + /** + * Called once a connection ws closed. + * @param connection the closed connection + */ + default void onConnectionClosed(Transport.Connection connection) {} + + /** + * Called for every response received + * @param requestId the request id for this reponse + * @param context the response context or null if the context was already processed ie. due to a timeout. + */ + default void onResponseReceived(long requestId, Transport.ResponseContext context) {} + + /** + * Called once a node connection is opened and registered. + */ + default void onNodeConnected(DiscoveryNode node) {} + + /** + * Called once a node connection is closed and unregistered. + */ + default void onNodeDisconnected(DiscoveryNode node) {} } diff --git a/server/src/main/java/org/elasticsearch/transport/TransportService.java b/server/src/main/java/org/elasticsearch/transport/TransportService.java index 656d8c3841769..d887519ee968b 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportService.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportService.java @@ -30,7 +30,6 @@ import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -45,8 +44,6 @@ import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.common.util.concurrent.AbstractRunnable; -import org.elasticsearch.common.util.concurrent.ConcurrentCollections; -import org.elasticsearch.common.util.concurrent.ConcurrentMapLong; import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; import org.elasticsearch.common.util.concurrent.FutureUtils; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -59,24 +56,23 @@ import java.net.UnknownHostException; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; -import java.util.stream.Stream; import static java.util.Collections.emptyList; import static org.elasticsearch.common.settings.Setting.listSetting; -public class TransportService extends AbstractLifecycleComponent { +public class TransportService extends AbstractLifecycleComponent implements TransportConnectionListener { public static final String DIRECT_RESPONSE_PROFILE = ".direct"; public static final String HANDSHAKE_ACTION_NAME = "internal:transport/handshake"; @@ -89,14 +85,7 @@ public class TransportService extends AbstractLifecycleComponent { private final TransportInterceptor.AsyncSender asyncSender; private final Function localNodeFactory; private final boolean connectToRemoteCluster; - - volatile Map requestHandlers = Collections.emptyMap(); - final Object requestHandlerMutex = new Object(); - - final ConcurrentMapLong clientHandlers = ConcurrentCollections.newConcurrentMapLongWithAggressiveConcurrency(); - - final CopyOnWriteArrayList connectionListeners = new CopyOnWriteArrayList<>(); - + private final Transport.ResponseHandlers responseHandlers; private final TransportInterceptor interceptor; // An LRU (don't really care about concurrency here) that holds the latest timed out requests so if they @@ -138,12 +127,12 @@ public DiscoveryNode getNode() { @Override public void sendRequest(long requestId, String action, TransportRequest request, TransportRequestOptions options) - throws IOException, TransportException { + throws TransportException { sendLocalRequest(requestId, action, request, options); } @Override - public void close() throws IOException { + public void close() { } }; @@ -172,6 +161,7 @@ public TransportService(Settings settings, Transport transport, ThreadPool threa this.asyncSender = interceptor.interceptSender(this::sendRequestInternal); this.connectToRemoteCluster = RemoteClusterService.ENABLE_REMOTE_CLUSTERS.get(settings); remoteClusterService = new RemoteClusterService(settings, this); + responseHandlers = transport.getResponseHandlers(); if (clusterSettings != null) { clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_INCLUDE_SETTING, this::setTracerLogInclude); clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_EXCLUDE_SETTING, this::setTracerLogExclude); @@ -179,6 +169,13 @@ public TransportService(Settings settings, Transport transport, ThreadPool threa remoteClusterService.listenForUpdates(clusterSettings); } } + registerRequestHandler( + HANDSHAKE_ACTION_NAME, + () -> HandshakeRequest.INSTANCE, + ThreadPool.Names.SAME, + false, false, + (request, channel) -> channel.sendResponse( + new HandshakeResponse(localNode, clusterName, localNode.getVersion()))); } public RemoteClusterService getRemoteClusterService() { @@ -202,7 +199,7 @@ protected TaskManager createTaskManager(Settings settings, ThreadPool threadPool * * @return the executor service */ - protected ExecutorService getExecutorService() { + private ExecutorService getExecutorService() { return threadPool.generic(); } @@ -216,9 +213,8 @@ void setTracerLogExclude(List tracerLogExclude) { @Override protected void doStart() { - transport.setTransportService(this); + transport.addConnectionListener(this); transport.start(); - if (transport.boundAddress() != null && logger.isInfoEnabled()) { logger.info("{}", transport.boundAddress()); for (Map.Entry entry : transport.profileBoundAddresses().entrySet()) { @@ -226,13 +222,6 @@ protected void doStart() { } } localNode = localNodeFactory.apply(transport.boundAddress()); - registerRequestHandler( - HANDSHAKE_ACTION_NAME, - () -> HandshakeRequest.INSTANCE, - ThreadPool.Names.SAME, - false, false, - (request, channel) -> channel.sendResponse( - new HandshakeResponse(localNode, clusterName, localNode.getVersion()))); if (connectToRemoteCluster) { // here we start to connect to the remote clusters remoteClusterService.initializeRemoteClusters(); @@ -246,36 +235,33 @@ protected void doStop() { } finally { // in case the transport is not connected to our local node (thus cleaned on node disconnect) // make sure to clean any leftover on going handles - for (Map.Entry entry : clientHandlers.entrySet()) { - final RequestHolder holderToNotify = clientHandlers.remove(entry.getKey()); - if (holderToNotify != null) { - // callback that an exception happened, but on a different thread since we don't - // want handlers to worry about stack overflows - getExecutorService().execute(new AbstractRunnable() { - @Override - public void onRejection(Exception e) { - // if we get rejected during node shutdown we don't wanna bubble it up - logger.debug( - () -> new ParameterizedMessage( - "failed to notify response handler on rejection, action: {}", - holderToNotify.action()), - e); - } - @Override - public void onFailure(Exception e) { - logger.warn( - () -> new ParameterizedMessage( - "failed to notify response handler on exception, action: {}", - holderToNotify.action()), - e); - } - @Override - public void doRun() { - TransportException ex = new TransportException("transport stopped, action: " + holderToNotify.action()); - holderToNotify.handler().handleException(ex); - } - }); - } + for (final Transport.ResponseContext holderToNotify : responseHandlers.prune(h -> true)) { + // callback that an exception happened, but on a different thread since we don't + // want handlers to worry about stack overflows + getExecutorService().execute(new AbstractRunnable() { + @Override + public void onRejection(Exception e) { + // if we get rejected during node shutdown we don't wanna bubble it up + logger.debug( + () -> new ParameterizedMessage( + "failed to notify response handler on rejection, action: {}", + holderToNotify.action()), + e); + } + @Override + public void onFailure(Exception e) { + logger.warn( + () -> new ParameterizedMessage( + "failed to notify response handler on exception, action: {}", + holderToNotify.action()), + e); + } + @Override + public void doRun() { + TransportException ex = new TransportException("transport stopped, action: " + holderToNotify.action()); + holderToNotify.handler().handleException(ex); + } + }); } } } @@ -479,11 +465,11 @@ public void disconnectFromNode(DiscoveryNode node) { } public void addConnectionListener(TransportConnectionListener listener) { - connectionListeners.add(listener); + transport.addConnectionListener(listener); } public void removeConnectionListener(TransportConnectionListener listener) { - connectionListeners.remove(listener); + transport.removeConnectionListener(listener); } public TransportFuture submitRequest(DiscoveryNode node, String action, TransportRequest request, @@ -594,18 +580,19 @@ private void sendRequestInternal(final Transport.C throw new IllegalStateException("can't send request to a null connection"); } DiscoveryNode node = connection.getNode(); - final long requestId = transport.newRequestId(); + + Supplier storedContextSupplier = threadPool.getThreadContext().newRestorableContext(true); + ContextRestoreResponseHandler responseHandler = new ContextRestoreResponseHandler<>(storedContextSupplier, handler); + // TODO we can probably fold this entire request ID dance into connection.sendReqeust but it will be a bigger refactoring + final long requestId = responseHandlers.add(new Transport.ResponseContext<>(responseHandler, connection, action)); final TimeoutHandler timeoutHandler; + if (options.timeout() != null) { + timeoutHandler = new TimeoutHandler(requestId, connection.getNode(), action); + responseHandler.setTimeoutHandler(timeoutHandler); + } else { + timeoutHandler = null; + } try { - - if (options.timeout() == null) { - timeoutHandler = null; - } else { - timeoutHandler = new TimeoutHandler(requestId); - } - Supplier storedContextSupplier = threadPool.getThreadContext().newRestorableContext(true); - TransportResponseHandler responseHandler = new ContextRestoreResponseHandler<>(storedContextSupplier, handler); - clientHandlers.put(requestId, new RequestHolder<>(responseHandler, connection, action, timeoutHandler)); if (lifecycle.stoppedOrClosed()) { // if we are not started the exception handling will remove the RequestHolder again and calls the handler to notify // the caller. It will only notify if the toStop code hasn't done the work yet. @@ -619,10 +606,12 @@ private void sendRequestInternal(final Transport.C } catch (final Exception e) { // usually happen either because we failed to connect to the node // or because we failed serializing the message - final RequestHolder holderToNotify = clientHandlers.remove(requestId); + final Transport.ResponseContext contextToNotify = responseHandlers.remove(requestId); // If holderToNotify == null then handler has already been taken care of. - if (holderToNotify != null) { - holderToNotify.cancelTimeout(); + if (contextToNotify != null) { + if (timeoutHandler != null) { + timeoutHandler.cancel(); + } // callback that an exception happened, but on a different thread since we don't // want handlers to worry about stack overflows final SendRequestTransportException sendRequestException = new SendRequestTransportException(node, action, e); @@ -633,7 +622,7 @@ public void onRejection(Exception e) { logger.debug( () -> new ParameterizedMessage( "failed to notify response handler on rejection, action: {}", - holderToNotify.action()), + contextToNotify.action()), e); } @Override @@ -641,12 +630,12 @@ public void onFailure(Exception e) { logger.warn( () -> new ParameterizedMessage( "failed to notify response handler on exception, action: {}", - holderToNotify.action()), + contextToNotify.action()), e); } @Override protected void doRun() throws Exception { - holderToNotify.handler().handleException(sendRequestException); + contextToNotify.handler().handleException(sendRequestException); } }); } else { @@ -722,6 +711,44 @@ public TransportAddress[] addressesFromString(String address, int perAddressLimi return transport.addressesFromString(address, perAddressLimit); } + /** + * A set of all valid action prefixes. + */ + public static final Set VALID_ACTION_PREFIXES = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( + "indices:admin", + "indices:monitor", + "indices:data/write", + "indices:data/read", + "indices:internal", + "cluster:admin", + "cluster:monitor", + "cluster:internal", + "internal:" + ))); + + private void validateActionName(String actionName) { + // TODO we should makes this a hard validation and throw an exception but we need a good way to add backwards layer + // for it. Maybe start with a deprecation layer + if (isValidActionName(actionName) == false) { + logger.warn("invalid action name [" + actionName + "] must start with one of: " + + TransportService.VALID_ACTION_PREFIXES ); + } + } + + /** + * Returns true iff the action name starts with a valid prefix. + * + * @see #VALID_ACTION_PREFIXES + */ + public static boolean isValidActionName(String actionName) { + for (String prefix : VALID_ACTION_PREFIXES) { + if (actionName.startsWith(prefix)) { + return true; + } + } + return false; + } + /** * Registers a new request handler * @@ -732,10 +759,11 @@ public TransportAddress[] addressesFromString(String address, int perAddressLimi */ public void registerRequestHandler(String action, Supplier requestFactory, String executor, TransportRequestHandler handler) { + validateActionName(action); handler = interceptor.interceptHandler(action, executor, false, handler); RequestHandlerRegistry reg = new RequestHandlerRegistry<>( action, Streamable.newWriteableReader(requestFactory), taskManager, handler, executor, false, true); - registerRequestHandler(reg); + transport.registerRequestHandler(reg); } /** @@ -749,10 +777,11 @@ public void registerRequestHandler(String act public void registerRequestHandler(String action, String executor, Writeable.Reader requestReader, TransportRequestHandler handler) { + validateActionName(action); handler = interceptor.interceptHandler(action, executor, false, handler); RequestHandlerRegistry reg = new RequestHandlerRegistry<>( action, requestReader, taskManager, handler, executor, false, true); - registerRequestHandler(reg); + transport.registerRequestHandler(reg); } /** @@ -769,10 +798,11 @@ public void registerRequestHandler(String act String executor, boolean forceExecution, boolean canTripCircuitBreaker, TransportRequestHandler handler) { + validateActionName(action); handler = interceptor.interceptHandler(action, executor, forceExecution, handler); RequestHandlerRegistry reg = new RequestHandlerRegistry<>( action, Streamable.newWriteableReader(request), taskManager, handler, executor, forceExecution, canTripCircuitBreaker); - registerRequestHandler(reg); + transport.registerRequestHandler(reg); } /** @@ -790,24 +820,16 @@ public void registerRequestHandler(String act boolean canTripCircuitBreaker, Writeable.Reader requestReader, TransportRequestHandler handler) { + validateActionName(action); handler = interceptor.interceptHandler(action, executor, forceExecution, handler); RequestHandlerRegistry reg = new RequestHandlerRegistry<>( action, requestReader, taskManager, handler, executor, forceExecution, canTripCircuitBreaker); - registerRequestHandler(reg); - } - - private void registerRequestHandler(RequestHandlerRegistry reg) { - synchronized (requestHandlerMutex) { - if (requestHandlers.containsKey(reg.getAction())) { - throw new IllegalArgumentException("transport handlers for action " + reg.getAction() + " is already registered"); - } - requestHandlers = MapBuilder.newMapBuilder(requestHandlers).put(reg.getAction(), reg).immutableMap(); - } + transport.registerRequestHandler(reg); } /** called by the {@link Transport} implementation once a request has been sent */ - void onRequestSent(DiscoveryNode node, long requestId, String action, TransportRequest request, - TransportRequestOptions options) { + public void onRequestSent(DiscoveryNode node, long requestId, String action, TransportRequest request, + TransportRequestOptions options) { if (traceEnabled() && shouldTraceAction(action)) { traceRequestSent(node, requestId, action, options); } @@ -818,14 +840,14 @@ protected boolean traceEnabled() { } /** called by the {@link Transport} implementation once a response was sent to calling node */ - void onResponseSent(long requestId, String action, TransportResponse response, TransportResponseOptions options) { + public void onResponseSent(long requestId, String action, TransportResponse response, TransportResponseOptions options) { if (traceEnabled() && shouldTraceAction(action)) { traceResponseSent(requestId, action); } } /** called by the {@link Transport} implementation after an exception was sent as a response to an incoming request */ - void onResponseSent(long requestId, String action, Exception e) { + public void onResponseSent(long requestId, String action, Exception e) { if (traceEnabled() && shouldTraceAction(action)) { traceResponseSent(requestId, action, e); } @@ -839,7 +861,7 @@ protected void traceResponseSent(long requestId, String action, Exception e) { * called by the {@link Transport} implementation when an incoming request arrives but before * any parsing of it has happened (with the exception of the requestId and action) */ - void onRequestReceived(long requestId, String action) { + public void onRequestReceived(long requestId, String action) { try { blockIncomingRequestsLatch.await(); } catch (InterruptedException e) { @@ -851,33 +873,24 @@ void onRequestReceived(long requestId, String action) { } public RequestHandlerRegistry getRequestHandler(String action) { - return requestHandlers.get(action); + return transport.getRequestHandler(action); } - /** - * called by the {@link Transport} implementation when a response or an exception has been received for a previously - * sent request (before any processing or deserialization was done). Returns the appropriate response handler or null if not - * found. - */ - public TransportResponseHandler onResponseReceived(final long requestId) { - RequestHolder holder = clientHandlers.remove(requestId); + @Override + public void onResponseReceived(long requestId, Transport.ResponseContext holder) { if (holder == null) { checkForTimeout(requestId); - return null; - } - holder.cancelTimeout(); - if (traceEnabled() && shouldTraceAction(holder.action())) { + } else if (traceEnabled() && shouldTraceAction(holder.action())) { traceReceivedResponse(requestId, holder.connection().getNode(), holder.action()); } - return holder.handler(); } private void checkForTimeout(long requestId) { // lets see if its in the timeout holder, but sync on mutex to make sure any ongoing timeout handling has finished final DiscoveryNode sourceNode; final String action; - assert clientHandlers.get(requestId) == null; + assert responseHandlers.contains(requestId) == false; TimeoutInfoHolder timeoutInfoHolder = timeoutInfoHandlers.remove(requestId); if (timeoutInfoHolder != null) { long time = System.currentTimeMillis(); @@ -903,48 +916,18 @@ private void checkForTimeout(long requestId) { } } - void onNodeConnected(final DiscoveryNode node) { - // capture listeners before spawning the background callback so the following pattern won't trigger a call - // connectToNode(); connection is completed successfully - // addConnectionListener(); this listener shouldn't be called - final Stream listenersToNotify = TransportService.this.connectionListeners.stream(); - getExecutorService().execute(() -> listenersToNotify.forEach(listener -> listener.onNodeConnected(node))); - } - - void onConnectionOpened(Transport.Connection connection) { - // capture listeners before spawning the background callback so the following pattern won't trigger a call - // connectToNode(); connection is completed successfully - // addConnectionListener(); this listener shouldn't be called - final Stream listenersToNotify = TransportService.this.connectionListeners.stream(); - getExecutorService().execute(() -> listenersToNotify.forEach(listener -> listener.onConnectionOpened(connection))); - } - - public void onNodeDisconnected(final DiscoveryNode node) { + @Override + public void onConnectionClosed(Transport.Connection connection) { try { - getExecutorService().execute( () -> { - for (final TransportConnectionListener connectionListener : connectionListeners) { - connectionListener.onNodeDisconnected(node); + List pruned = responseHandlers.prune(h -> h.connection().getCacheKey().equals(connection + .getCacheKey())); + // callback that an exception happened, but on a different thread since we don't + // want handlers to worry about stack overflows + getExecutorService().execute(() -> { + for (Transport.ResponseContext holderToNotify : pruned) { + holderToNotify.handler().handleException(new NodeDisconnectedException(connection.getNode(), holderToNotify.action())); } }); - } catch (EsRejectedExecutionException ex) { - logger.debug("Rejected execution on NodeDisconnected", ex); - } - } - - void onConnectionClosed(Transport.Connection connection) { - try { - for (Map.Entry entry : clientHandlers.entrySet()) { - RequestHolder holder = entry.getValue(); - if (holder.connection().getCacheKey().equals(connection.getCacheKey())) { - final RequestHolder holderToNotify = clientHandlers.remove(entry.getKey()); - if (holderToNotify != null) { - // callback that an exception happened, but on a different thread since we don't - // want handlers to worry about stack overflows - getExecutorService().execute(() -> holderToNotify.handler().handleException(new NodeDisconnectedException( - connection.getNode(), holderToNotify.action()))); - } - } - } } catch (EsRejectedExecutionException ex) { logger.debug("Rejected execution on onConnectionClosed", ex); } @@ -970,32 +953,31 @@ protected void traceRequestSent(DiscoveryNode node, long requestId, String actio tracerLog.trace("[{}][{}] sent to [{}] (timeout: [{}])", requestId, action, node, options.timeout()); } - class TimeoutHandler implements Runnable { + final class TimeoutHandler implements Runnable { private final long requestId; - private final long sentTime = System.currentTimeMillis(); - + private final String action; + private final DiscoveryNode node; volatile ScheduledFuture future; - TimeoutHandler(long requestId) { + TimeoutHandler(long requestId, DiscoveryNode node, String action) { this.requestId = requestId; + this.node = node; + this.action = action; } @Override public void run() { - // we get first to make sure we only add the TimeoutInfoHandler if needed. - final RequestHolder holder = clientHandlers.get(requestId); - if (holder != null) { - // add it to the timeout information holder, in case we are going to get a response later + if (responseHandlers.contains(requestId)) { long timeoutTime = System.currentTimeMillis(); - timeoutInfoHandlers.put(requestId, new TimeoutInfoHolder(holder.connection().getNode(), holder.action(), sentTime, - timeoutTime)); + timeoutInfoHandlers.put(requestId, new TimeoutInfoHolder(node, action, sentTime, timeoutTime)); // now that we have the information visible via timeoutInfoHandlers, we try to remove the request id - final RequestHolder removedHolder = clientHandlers.remove(requestId); - if (removedHolder != null) { - assert removedHolder == holder : "two different holder instances for request [" + requestId + "]"; - removedHolder.handler().handleException( + final Transport.ResponseContext holder = responseHandlers.remove(requestId); + if (holder != null) { + assert holder.action().equals(action); + assert holder.connection().getNode().equals(node); + holder.handler().handleException( new ReceiveTimeoutTransportException(holder.connection().getNode(), holder.action(), "request_id [" + requestId + "] timed out after [" + (timeoutTime - sentTime) + "ms]")); } else { @@ -1006,11 +988,11 @@ public void run() { } /** - * cancels timeout handling. this is a best effort only to avoid running it. remove the requestId from {@link #clientHandlers} + * cancels timeout handling. this is a best effort only to avoid running it. remove the requestId from {@link #responseHandlers} * to make sure this doesn't run. */ public void cancel() { - assert clientHandlers.get(requestId) == null : + assert responseHandlers.contains(requestId) == false : "cancel must be called after the requestId [" + requestId + "] has been removed from clientHandlers"; FutureUtils.cancel(future); } @@ -1047,42 +1029,6 @@ public long timeoutTime() { } } - static class RequestHolder { - - private final TransportResponseHandler handler; - - private final Transport.Connection connection; - - private final String action; - - private final TimeoutHandler timeoutHandler; - - RequestHolder(TransportResponseHandler handler, Transport.Connection connection, String action, TimeoutHandler timeoutHandler) { - this.handler = handler; - this.connection = connection; - this.action = action; - this.timeoutHandler = timeoutHandler; - } - - public TransportResponseHandler handler() { - return handler; - } - - public Transport.Connection connection() { - return this.connection; - } - - public String action() { - return this.action; - } - - public void cancelTimeout() { - if (timeoutHandler != null) { - timeoutHandler.cancel(); - } - } - } - /** * This handler wrapper ensures that the response thread executes with the correct thread context. Before any of the handle methods * are invoked we restore the context. @@ -1091,6 +1037,7 @@ public static final class ContextRestoreResponseHandler delegate; private final Supplier contextSupplier; + private volatile TimeoutHandler handler; public ContextRestoreResponseHandler(Supplier contextSupplier, TransportResponseHandler delegate) { this.delegate = delegate; @@ -1104,6 +1051,9 @@ public T read(StreamInput in) throws IOException { @Override public void handleResponse(T response) { + if(handler != null) { + handler.cancel(); + } try (ThreadContext.StoredContext ignore = contextSupplier.get()) { delegate.handleResponse(response); } @@ -1111,6 +1061,9 @@ public void handleResponse(T response) { @Override public void handleException(TransportException exp) { + if(handler != null) { + handler.cancel(); + } try (ThreadContext.StoredContext ignore = contextSupplier.get()) { delegate.handleException(exp); } @@ -1126,6 +1079,10 @@ public String toString() { return getClass().getName() + "/" + delegate.toString(); } + void setTimeoutHandler(TimeoutHandler handler) { + this.handler = handler; + } + } static class DirectResponseChannel implements TransportChannel { @@ -1159,7 +1116,7 @@ public void sendResponse(TransportResponse response) throws IOException { @Override public void sendResponse(final TransportResponse response, TransportResponseOptions options) throws IOException { service.onResponseSent(requestId, action, response, options); - final TransportResponseHandler handler = service.onResponseReceived(requestId); + final TransportResponseHandler handler = service.responseHandlers.onResponseReceived(requestId, service); // ignore if its null, the service logs it if (handler != null) { final String executor = handler.executor(); @@ -1183,7 +1140,7 @@ protected void processResponse(TransportResponseHandler handler, TransportRespon @Override public void sendResponse(Exception exception) throws IOException { service.onResponseSent(requestId, action, exception); - final TransportResponseHandler handler = service.onResponseReceived(requestId); + final TransportResponseHandler handler = service.responseHandlers.onResponseReceived(requestId, service); // ignore if its null, the service logs it if (handler != null) { final RemoteTransportException rtx = wrapInRemote(exception); @@ -1224,6 +1181,7 @@ public Version getVersion() { } } + /** * Returns the internal thread pool */ diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/CancellableTasksTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/CancellableTasksTests.java index 6b2e2040bca80..c4958ab15ef94 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/CancellableTasksTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/CancellableTasksTests.java @@ -214,7 +214,7 @@ private Task startCancellableTestNodesAction(boolean waitForActionToStart, Colle for (int i = 0; i < testNodes.length; i++) { boolean shouldBlock = blockOnNodes.contains(testNodes[i]); logger.info("The action in the node [{}] should block: [{}]", testNodes[i].getNodeId(), shouldBlock); - actions[i] = new CancellableTestNodesAction(CLUSTER_SETTINGS, "testAction", threadPool, testNodes[i] + actions[i] = new CancellableTestNodesAction(CLUSTER_SETTINGS, "internal:testAction", threadPool, testNodes[i] .clusterService, testNodes[i].transportService, shouldBlock, actionLatch); } Task task = actions[0].execute(request, listener); diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TransportTasksActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TransportTasksActionTests.java index cb6f2b57b2bd0..366243aed9a2e 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TransportTasksActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TransportTasksActionTests.java @@ -296,7 +296,7 @@ private Task startBlockingTestNodesAction(CountDownLatch checkLatch, NodesReques TestNodesAction[] actions = new TestNodesAction[nodesCount]; for (int i = 0; i < testNodes.length; i++) { final int node = i; - actions[i] = new TestNodesAction(CLUSTER_SETTINGS, "testAction", threadPool, testNodes[i].clusterService, + actions[i] = new TestNodesAction(CLUSTER_SETTINGS, "internal:testAction", threadPool, testNodes[i].clusterService, testNodes[i].transportService) { @Override protected NodeResponse nodeOperation(NodeRequest request) { @@ -361,7 +361,7 @@ public void onFailure(Exception e) { int testNodeNum = randomIntBetween(0, testNodes.length - 1); TestNode testNode = testNodes[testNodeNum]; ListTasksRequest listTasksRequest = new ListTasksRequest(); - listTasksRequest.setActions("testAction*"); // pick all test actions + listTasksRequest.setActions("internal:testAction*"); // pick all test actions logger.info("Listing currently running tasks using node [{}]", testNodeNum); ListTasksResponse response = testNode.transportListTasksAction.execute(listTasksRequest).get(); logger.info("Checking currently running tasks"); @@ -381,7 +381,7 @@ public void onFailure(Exception e) { // Check task counts using transport with filtering testNode = testNodes[randomIntBetween(0, testNodes.length - 1)]; listTasksRequest = new ListTasksRequest(); - listTasksRequest.setActions("testAction[n]"); // only pick node actions + listTasksRequest.setActions("internal:testAction[n]"); // only pick node actions response = testNode.transportListTasksAction.execute(listTasksRequest).get(); assertEquals(testNodes.length, response.getPerNodeTasks().size()); for (Map.Entry> entry : response.getPerNodeTasks().entrySet()) { @@ -404,7 +404,7 @@ public void onFailure(Exception e) { } // Make sure that the main task on coordinating node is the task that was returned to us by execute() - listTasksRequest.setActions("testAction"); // only pick the main task + listTasksRequest.setActions("internal:testAction"); // only pick the main task response = testNode.transportListTasksAction.execute(listTasksRequest).get(); assertEquals(1, response.getTasks().size()); assertEquals(mainTask.getId(), response.getTasks().get(0).getId()); @@ -432,7 +432,7 @@ public void testFindChildTasks() throws Exception { // Get the parent task ListTasksRequest listTasksRequest = new ListTasksRequest(); - listTasksRequest.setActions("testAction"); + listTasksRequest.setActions("internal:testAction"); ListTasksResponse response = testNode.transportListTasksAction.execute(listTasksRequest).get(); assertEquals(1, response.getTasks().size()); String parentNode = response.getTasks().get(0).getTaskId().getNodeId(); @@ -444,7 +444,7 @@ public void testFindChildTasks() throws Exception { response = testNode.transportListTasksAction.execute(listTasksRequest).get(); assertEquals(testNodes.length, response.getTasks().size()); for (TaskInfo task : response.getTasks()) { - assertEquals("testAction[n]", task.getAction()); + assertEquals("internal:testAction[n]", task.getAction()); assertEquals(parentNode, task.getParentTaskId().getNodeId()); assertEquals(parentTaskId, task.getParentTaskId().getId()); } @@ -487,7 +487,7 @@ public void testTasksDescriptions() throws Exception { // Check task counts using transport with filtering TestNode testNode = testNodes[randomIntBetween(0, testNodes.length - 1)]; ListTasksRequest listTasksRequest = new ListTasksRequest(); - listTasksRequest.setActions("testAction[n]"); // only pick node actions + listTasksRequest.setActions("internal:testAction[n]"); // only pick node actions ListTasksResponse response = testNode.transportListTasksAction.execute(listTasksRequest).get(); assertEquals(testNodes.length, response.getPerNodeTasks().size()); for (Map.Entry> entry : response.getPerNodeTasks().entrySet()) { @@ -529,7 +529,7 @@ public void onFailure(Exception e) { responseLatch.countDown(); } }); - String actionName = "testAction"; // only pick the main action + String actionName = "internal:testAction"; // only pick the main action // Try to cancel main task using action name CancelTasksRequest request = new CancelTasksRequest(); @@ -578,10 +578,10 @@ public void testFailedTasksCount() throws ExecutionException, InterruptedExcepti setupTestNodes(settings); connectNodes(testNodes); TestNodesAction[] actions = new TestNodesAction[nodesCount]; - RecordingTaskManagerListener[] listeners = setupListeners(testNodes, "testAction*"); + RecordingTaskManagerListener[] listeners = setupListeners(testNodes, "internal:testAction*"); for (int i = 0; i < testNodes.length; i++) { final int node = i; - actions[i] = new TestNodesAction(CLUSTER_SETTINGS, "testAction", threadPool, testNodes[i].clusterService, + actions[i] = new TestNodesAction(CLUSTER_SETTINGS, "internal:testAction", threadPool, testNodes[i].clusterService, testNodes[i].transportService) { @Override protected NodeResponse nodeOperation(NodeRequest request) { @@ -621,7 +621,7 @@ public void testTaskLevelActionFailures() throws ExecutionException, Interrupted for (int i = 0; i < testNodes.length; i++) { final int node = i; // Simulate task action that fails on one of the tasks on one of the nodes - tasksActions[i] = new TestTasksAction(CLUSTER_SETTINGS, "testTasksAction", threadPool, testNodes[i].clusterService, + tasksActions[i] = new TestTasksAction(CLUSTER_SETTINGS, "internal:testTasksAction", threadPool, testNodes[i].clusterService, testNodes[i].transportService) { @Override protected void taskOperation(TestTasksRequest request, Task task, ActionListener listener) { @@ -659,7 +659,7 @@ protected void taskOperation(TestTasksRequest request, Task task, ActionListener // Run task action on node tasks that are currently running // should be successful on all nodes except one TestTasksRequest testTasksRequest = new TestTasksRequest(); - testTasksRequest.setActions("testAction[n]"); // pick all test actions + testTasksRequest.setActions("internal:testAction[n]"); // pick all test actions TestTasksResponse response = tasksActions[0].execute(testTasksRequest).get(); assertThat(response.getTaskFailures(), hasSize(1)); // one task failed assertThat(response.getTaskFailures().get(0).getReason(), containsString("Task level failure")); @@ -700,7 +700,7 @@ public void testTaskNodeFiltering() throws ExecutionException, InterruptedExcept final int node = i; // Simulate a task action that works on all nodes except nodes listed in filterNodes. // We are testing that it works. - tasksActions[i] = new TestTasksAction(CLUSTER_SETTINGS, "testTasksAction", threadPool, + tasksActions[i] = new TestTasksAction(CLUSTER_SETTINGS, "internal:testTasksAction", threadPool, testNodes[i].clusterService, testNodes[i].transportService) { @Override @@ -729,7 +729,7 @@ protected void taskOperation(TestTasksRequest request, Task task, ActionListener // Run task action on node tasks that are currently running // should be successful on all nodes except nodes that we filtered out TestTasksRequest testTasksRequest = new TestTasksRequest(); - testTasksRequest.setActions("testAction[n]"); // pick all test actions + testTasksRequest.setActions("internal:testAction[n]"); // pick all test actions TestTasksResponse response = tasksActions[randomIntBetween(0, nodesCount - 1)].execute(testTasksRequest).get(); // Get successful responses from all nodes except nodes that we filtered out diff --git a/server/src/test/java/org/elasticsearch/action/main/MainActionTests.java b/server/src/test/java/org/elasticsearch/action/main/MainActionTests.java index 34f9bc15ecfa6..fbf3aeaec9740 100644 --- a/server/src/test/java/org/elasticsearch/action/main/MainActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/main/MainActionTests.java @@ -1,3 +1,4 @@ + /* * Licensed to Elasticsearch under one or more contributor * license agreements. See the NOTICE file distributed with @@ -32,6 +33,7 @@ import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import java.util.Collections; @@ -68,8 +70,8 @@ public void testMainActionClusterAvailable() { ClusterState state = ClusterState.builder(clusterName).blocks(blocks).build(); when(clusterService.state()).thenReturn(state); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportMainAction action = new TransportMainAction(settings, mock(ThreadPool.class), transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), clusterService); AtomicReference responseRef = new AtomicReference<>(); diff --git a/server/src/test/java/org/elasticsearch/action/search/TransportMultiSearchActionTests.java b/server/src/test/java/org/elasticsearch/action/search/TransportMultiSearchActionTests.java index 6b6bbc4ae98a4..8dd1e70857dd2 100644 --- a/server/src/test/java/org/elasticsearch/action/search/TransportMultiSearchActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/TransportMultiSearchActionTests.java @@ -26,7 +26,6 @@ import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; @@ -37,6 +36,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.junit.After; import org.junit.Before; @@ -81,8 +81,7 @@ public void testBatchExecute() throws Exception { ActionFilters actionFilters = mock(ActionFilters.class); when(actionFilters.filters()).thenReturn(new ActionFilter[0]); ThreadPool threadPool = new ThreadPool(settings); - TaskManager taskManager = mock(TaskManager.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, threadPool, + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), threadPool, TransportService.NOOP_TRANSPORT_INTERCEPTOR, boundAddress -> DiscoveryNode.createLocal(settings, boundAddress.publishAddress(), UUIDs.randomBase64UUID()), null, Collections.emptySet()) { @@ -93,7 +92,6 @@ public TaskManager getTaskManager() { }; ClusterService clusterService = mock(ClusterService.class); when(clusterService.state()).thenReturn(ClusterState.builder(new ClusterName("test")).build()); - IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(Settings.EMPTY); // Keep track of the number of concurrent searches started by multi search api, // and if there are more searches than is allowed create an error and remember that. diff --git a/server/src/test/java/org/elasticsearch/action/support/master/TransportMasterNodeActionTests.java b/server/src/test/java/org/elasticsearch/action/support/master/TransportMasterNodeActionTests.java index f2b18a8c8f561..b27bc9ad79432 100644 --- a/server/src/test/java/org/elasticsearch/action/support/master/TransportMasterNodeActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/master/TransportMasterNodeActionTests.java @@ -174,7 +174,7 @@ public void testLocalOperationWithoutBlocks() throws ExecutionException, Interru setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); - new Action(Settings.EMPTY, "testAction", transportService, clusterService, threadPool) { + new Action(Settings.EMPTY, "internal:testAction", transportService, clusterService, threadPool) { @Override protected void masterOperation(Task task, Request request, ClusterState state, ActionListener listener) throws Exception { if (masterOperationFailure) { @@ -211,7 +211,7 @@ public void testLocalOperationWithBlocks() throws ExecutionException, Interrupte .blocks(ClusterBlocks.builder().addGlobalBlock(block)).build(); setState(clusterService, stateWithBlock); - new Action(Settings.EMPTY, "testAction", transportService, clusterService, threadPool) { + new Action(Settings.EMPTY, "internal:testAction", transportService, clusterService, threadPool) { @Override protected ClusterBlockException checkBlock(Request request, ClusterState state) { Set blocks = state.blocks().global(); @@ -253,7 +253,7 @@ public void testCheckBlockThrowsException() throws InterruptedException { .blocks(ClusterBlocks.builder().addGlobalBlock(block)).build(); setState(clusterService, stateWithBlock); - new Action(Settings.EMPTY, "testAction", transportService, clusterService, threadPool) { + new Action(Settings.EMPTY, "internal:testAction", transportService, clusterService, threadPool) { @Override protected ClusterBlockException checkBlock(Request request, ClusterState state) { Set blocks = state.blocks().global(); @@ -281,7 +281,7 @@ public void testForceLocalOperation() throws ExecutionException, InterruptedExce setState(clusterService, ClusterStateCreationUtils.state(localNode, randomFrom(localNode, remoteNode, null), allNodes)); - new Action(Settings.EMPTY, "testAction", transportService, clusterService, threadPool) { + new Action(Settings.EMPTY, "internal:testAction", transportService, clusterService, threadPool) { @Override protected boolean localExecute(Request request) { return true; @@ -296,7 +296,7 @@ public void testMasterNotAvailable() throws ExecutionException, InterruptedExcep Request request = new Request().masterNodeTimeout(TimeValue.timeValueSeconds(0)); setState(clusterService, ClusterStateCreationUtils.state(localNode, null, allNodes)); PlainActionFuture listener = new PlainActionFuture<>(); - new Action(Settings.EMPTY, "testAction", transportService, clusterService, threadPool).execute(request, listener); + new Action(Settings.EMPTY, "internal:testAction", transportService, clusterService, threadPool).execute(request, listener); assertTrue(listener.isDone()); assertListenerThrows("MasterNotDiscoveredException should be thrown", listener, MasterNotDiscoveredException.class); } @@ -305,7 +305,7 @@ public void testMasterBecomesAvailable() throws ExecutionException, InterruptedE Request request = new Request(); setState(clusterService, ClusterStateCreationUtils.state(localNode, null, allNodes)); PlainActionFuture listener = new PlainActionFuture<>(); - new Action(Settings.EMPTY, "testAction", transportService, clusterService, threadPool).execute(request, listener); + new Action(Settings.EMPTY, "internal:testAction", transportService, clusterService, threadPool).execute(request, listener); assertFalse(listener.isDone()); setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); assertTrue(listener.isDone()); @@ -317,13 +317,13 @@ public void testDelegateToMaster() throws ExecutionException, InterruptedExcepti setState(clusterService, ClusterStateCreationUtils.state(localNode, remoteNode, allNodes)); PlainActionFuture listener = new PlainActionFuture<>(); - new Action(Settings.EMPTY, "testAction", transportService, clusterService, threadPool).execute(request, listener); + new Action(Settings.EMPTY, "internal:testAction", transportService, clusterService, threadPool).execute(request, listener); assertThat(transport.capturedRequests().length, equalTo(1)); CapturingTransport.CapturedRequest capturedRequest = transport.capturedRequests()[0]; assertTrue(capturedRequest.node.isMasterNode()); assertThat(capturedRequest.request, equalTo(request)); - assertThat(capturedRequest.action, equalTo("testAction")); + assertThat(capturedRequest.action, equalTo("internal:testAction")); Response response = new Response(); transport.handleResponse(capturedRequest.requestId, response); @@ -340,14 +340,14 @@ public void testDelegateToFailingMaster() throws ExecutionException, Interrupted .version(randomIntBetween(0, 10))); // use a random base version so it can go down when simulating a restart. PlainActionFuture listener = new PlainActionFuture<>(); - new Action(Settings.EMPTY, "testAction", transportService, clusterService, threadPool).execute(request, listener); + new Action(Settings.EMPTY, "internal:testAction", transportService, clusterService, threadPool).execute(request, listener); CapturingTransport.CapturedRequest[] capturedRequests = transport.getCapturedRequestsAndClear(); assertThat(capturedRequests.length, equalTo(1)); CapturingTransport.CapturedRequest capturedRequest = capturedRequests[0]; assertTrue(capturedRequest.node.isMasterNode()); assertThat(capturedRequest.request, equalTo(request)); - assertThat(capturedRequest.action, equalTo("testAction")); + assertThat(capturedRequest.action, equalTo("internal:testAction")); if (rejoinSameMaster) { transport.handleRemoteError(capturedRequest.requestId, new ConnectTransportException(masterNode, "Fake error")); @@ -380,7 +380,7 @@ public void testDelegateToFailingMaster() throws ExecutionException, Interrupted capturedRequest = capturedRequests[0]; assertTrue(capturedRequest.node.isMasterNode()); assertThat(capturedRequest.request, equalTo(request)); - assertThat(capturedRequest.action, equalTo("testAction")); + assertThat(capturedRequest.action, equalTo("internal:testAction")); } else if (failsWithConnectTransportException) { transport.handleRemoteError(capturedRequest.requestId, new ConnectTransportException(masterNode, "Fake error")); assertFalse(listener.isDone()); @@ -413,7 +413,7 @@ public void testMasterFailoverAfterStepDown() throws ExecutionException, Interru setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); - new Action(Settings.EMPTY, "testAction", transportService, clusterService, threadPool) { + new Action(Settings.EMPTY, "internal:testAction", transportService, clusterService, threadPool) { @Override protected void masterOperation(Request request, ClusterState state, ActionListener listener) throws Exception { // The other node has become master, simulate failures of this node while publishing cluster state through ZenDiscovery @@ -429,7 +429,7 @@ protected void masterOperation(Request request, ClusterState state, ActionListen CapturingTransport.CapturedRequest capturedRequest = transport.capturedRequests()[0]; assertTrue(capturedRequest.node.isMasterNode()); assertThat(capturedRequest.request, equalTo(request)); - assertThat(capturedRequest.action, equalTo("testAction")); + assertThat(capturedRequest.action, equalTo("internal:testAction")); transport.handleResponse(capturedRequest.requestId, response); assertTrue(listener.isDone()); diff --git a/server/src/test/java/org/elasticsearch/action/support/replication/BroadcastReplicationTests.java b/server/src/test/java/org/elasticsearch/action/support/replication/BroadcastReplicationTests.java index d2a51070c9298..ad8697817aa50 100644 --- a/server/src/test/java/org/elasticsearch/action/support/replication/BroadcastReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/replication/BroadcastReplicationTests.java @@ -205,7 +205,7 @@ private class TestBroadcastReplicationAction extends TransportBroadcastReplicati TestBroadcastReplicationAction(Settings settings, ThreadPool threadPool, ClusterService clusterService, TransportService transportService, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver, TransportReplicationAction replicatedBroadcastShardAction) { - super("test-broadcast-replication-action", DummyBroadcastRequest::new, settings, threadPool, clusterService, transportService, + super("internal:test-broadcast-replication-action", DummyBroadcastRequest::new, settings, threadPool, clusterService, transportService, actionFilters, indexNameExpressionResolver, replicatedBroadcastShardAction); } diff --git a/server/src/test/java/org/elasticsearch/action/support/replication/TransportReplicationActionTests.java b/server/src/test/java/org/elasticsearch/action/support/replication/TransportReplicationActionTests.java index a34c755e05272..08301e99d6a69 100644 --- a/server/src/test/java/org/elasticsearch/action/support/replication/TransportReplicationActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/replication/TransportReplicationActionTests.java @@ -168,7 +168,7 @@ public void setUp() throws Exception { transportService.start(); transportService.acceptIncomingRequests(); shardStateAction = new ShardStateAction(Settings.EMPTY, clusterService, transportService, null, null, threadPool); - action = new TestAction(Settings.EMPTY, "testAction", transportService, clusterService, shardStateAction, threadPool); + action = new TestAction(Settings.EMPTY, "internal:testAction", transportService, clusterService, shardStateAction, threadPool); } @After @@ -196,7 +196,7 @@ public void testBlocks() throws ExecutionException, InterruptedException { Request request = new Request(); PlainActionFuture listener = new PlainActionFuture<>(); ReplicationTask task = maybeTask(); - TestAction action = new TestAction(Settings.EMPTY, "testActionWithBlocks", + TestAction action = new TestAction(Settings.EMPTY, "internal:testActionWithBlocks", transportService, clusterService, shardStateAction, threadPool) { @Override protected ClusterBlockLevel globalBlockLevel() { @@ -236,7 +236,8 @@ protected ClusterBlockLevel globalBlockLevel() { ClusterBlockException.class); assertIndexShardUninitialized(); - action = new TestAction(Settings.EMPTY, "testActionWithNoBlocks", transportService, clusterService, shardStateAction, threadPool) { + action = new TestAction(Settings.EMPTY, "internal:testActionWithNoBlocks", transportService, clusterService, shardStateAction, + threadPool) { @Override protected ClusterBlockLevel globalBlockLevel() { return null; @@ -287,7 +288,7 @@ public void testNotStartedPrimary() throws InterruptedException, ExecutionExcept transport.getCapturedRequestsByTargetNodeAndClear().get(primaryNodeId); assertThat(capturedRequests, notNullValue()); assertThat(capturedRequests.size(), equalTo(1)); - assertThat(capturedRequests.get(0).action, equalTo("testAction[p]")); + assertThat(capturedRequests.get(0).action, equalTo("internal:testAction[p]")); assertIndexShardCounter(0); } @@ -339,7 +340,7 @@ public void testNoRerouteOnStaleClusterState() throws InterruptedException, Exec transport.getCapturedRequestsByTargetNodeAndClear().get(primaryNodeId); assertThat(capturedRequests, notNullValue()); assertThat(capturedRequests.size(), equalTo(1)); - assertThat(capturedRequests.get(0).action, equalTo("testAction[p]")); + assertThat(capturedRequests.get(0).action, equalTo("internal:testAction[p]")); assertIndexShardCounter(0); } @@ -378,7 +379,7 @@ public void testClosedIndexOnReroute() throws InterruptedException { ReplicationTask task = maybeTask(); ClusterBlockLevel indexBlockLevel = randomBoolean() ? ClusterBlockLevel.WRITE : null; - TestAction action = new TestAction(Settings.EMPTY, "testActionWithBlocks", transportService, + TestAction action = new TestAction(Settings.EMPTY, "internal:testActionWithBlocks", transportService, clusterService, shardStateAction, threadPool) { @Override protected ClusterBlockLevel indexBlockLevel() { @@ -416,7 +417,7 @@ public void testStalePrimaryShardOnReroute() throws InterruptedException { reroutePhase.run(); CapturingTransport.CapturedRequest[] capturedRequests = transport.getCapturedRequestsAndClear(); assertThat(capturedRequests, arrayWithSize(1)); - assertThat(capturedRequests[0].action, equalTo("testAction[p]")); + assertThat(capturedRequests[0].action, equalTo("internal:testAction[p]")); assertPhase(task, "waiting_on_primary"); assertFalse(request.isRetrySet.get()); transport.handleRemoteError(capturedRequests[0].requestId, randomRetryPrimaryException(shardId)); @@ -427,7 +428,7 @@ public void testStalePrimaryShardOnReroute() throws InterruptedException { assertThat(listener.isDone(), equalTo(false)); capturedRequests = transport.getCapturedRequestsAndClear(); assertThat(capturedRequests, arrayWithSize(1)); - assertThat(capturedRequests[0].action, equalTo("testAction[p]")); + assertThat(capturedRequests[0].action, equalTo("internal:testAction[p]")); assertPhase(task, "waiting_on_primary"); transport.handleRemoteError(capturedRequests[0].requestId, randomRetryPrimaryException(shardId)); assertListenerThrows("must throw index not found exception", listener, ElasticsearchException.class); @@ -438,7 +439,7 @@ public void testStalePrimaryShardOnReroute() throws InterruptedException { setState(clusterService, clusterService.state()); capturedRequests = transport.getCapturedRequestsAndClear(); assertThat(capturedRequests, arrayWithSize(1)); - assertThat(capturedRequests[0].action, equalTo("testAction[p]")); + assertThat(capturedRequests[0].action, equalTo("internal:testAction[p]")); } } @@ -474,10 +475,10 @@ public void testRoutePhaseExecutesRequest() { assertThat(capturedRequests, notNullValue()); assertThat(capturedRequests.size(), equalTo(1)); if (clusterService.state().nodes().getLocalNodeId().equals(primaryNodeId)) { - assertThat(capturedRequests.get(0).action, equalTo("testAction[p]")); + assertThat(capturedRequests.get(0).action, equalTo("internal:testAction[p]")); assertPhase(task, "waiting_on_primary"); } else { - assertThat(capturedRequests.get(0).action, equalTo("testAction")); + assertThat(capturedRequests.get(0).action, equalTo("internal:testAction")); assertPhase(task, "rerouted"); } assertFalse(request.isRetrySet.get()); @@ -531,7 +532,7 @@ public void execute() throws Exception { transport.capturedRequestsByTargetNode().get(primaryShard.relocatingNodeId()); assertThat(requests, notNullValue()); assertThat(requests.size(), equalTo(1)); - assertThat("primary request was not delegated to relocation target", requests.get(0).action, equalTo("testAction[p]")); + assertThat("primary request was not delegated to relocation target", requests.get(0).action, equalTo("internal:testAction[p]")); assertThat("primary term not properly set on primary delegation", ((TransportReplicationAction.ConcreteShardRequest)requests.get(0).request).getPrimaryTerm(), equalTo(primaryTerm)); assertPhase(task, "primary_delegation"); @@ -705,7 +706,8 @@ public void testSeqNoIsSetOnPrimary() throws Exception { }; TestAction action = - new TestAction(Settings.EMPTY, "testSeqNoIsSetOnPrimary", transportService, clusterService, shardStateAction, threadPool) { + new TestAction(Settings.EMPTY, "internal:testSeqNoIsSetOnPrimary", transportService, clusterService, shardStateAction, + threadPool) { @Override protected IndexShard getIndexShard(ShardId shardId) { return shard; @@ -788,8 +790,8 @@ public void testReplicasCounter() throws Exception { final ShardRouting replicaRouting = state.getRoutingTable().shardRoutingTable(shardId).replicaShards().get(0); boolean throwException = randomBoolean(); final ReplicationTask task = maybeTask(); - TestAction action = new TestAction(Settings.EMPTY, "testActionWithExceptions", transportService, clusterService, shardStateAction, - threadPool) { + TestAction action = new TestAction(Settings.EMPTY, "internal:testActionWithExceptions", transportService, clusterService, + shardStateAction, threadPool) { @Override protected ReplicaResult shardOperationOnReplica(Request request, IndexShard replica) { assertIndexShardCounter(1); @@ -924,8 +926,8 @@ public void testRetryOnReplica() throws Exception { setState(clusterService, state); AtomicBoolean throwException = new AtomicBoolean(true); final ReplicationTask task = maybeTask(); - TestAction action = new TestAction(Settings.EMPTY, "testActionWithExceptions", transportService, clusterService, shardStateAction, - threadPool) { + TestAction action = new TestAction(Settings.EMPTY, "internal:testActionWithExceptions", transportService, clusterService, + shardStateAction, threadPool) { @Override protected ReplicaResult shardOperationOnReplica(Request request, IndexShard replica) { assertPhase(task, "replica"); @@ -960,7 +962,7 @@ protected ReplicaResult shardOperationOnReplica(Request request, IndexShard repl assertThat(capturedRequests, notNullValue()); assertThat(capturedRequests.size(), equalTo(1)); final CapturingTransport.CapturedRequest capturedRequest = capturedRequests.get(0); - assertThat(capturedRequest.action, equalTo("testActionWithExceptions[r]")); + assertThat(capturedRequest.action, equalTo("internal:testActionWithExceptions[r]")); assertThat(capturedRequest.request, instanceOf(TransportReplicationAction.ConcreteReplicaRequest.class)); assertThat(((TransportReplicationAction.ConcreteReplicaRequest) capturedRequest.request).getGlobalCheckpoint(), equalTo(checkpoint)); @@ -988,8 +990,8 @@ public void testRetryOnReplicaWithRealTransport() throws Exception { transportService.acceptIncomingRequests(); AtomicBoolean calledSuccessfully = new AtomicBoolean(false); - TestAction action = new TestAction(Settings.EMPTY, "testActionWithExceptions", transportService, clusterService, shardStateAction, - threadPool) { + TestAction action = new TestAction(Settings.EMPTY, "internal:testActionWithExceptions", transportService, clusterService, + shardStateAction, threadPool) { @Override protected ReplicaResult shardOperationOnReplica(Request request, IndexShard replica) { assertPhase(task, "replica"); diff --git a/server/src/test/java/org/elasticsearch/action/support/replication/TransportWriteActionTests.java b/server/src/test/java/org/elasticsearch/action/support/replication/TransportWriteActionTests.java index b894188dabef5..bfcc5938a8690 100644 --- a/server/src/test/java/org/elasticsearch/action/support/replication/TransportWriteActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/replication/TransportWriteActionTests.java @@ -52,6 +52,7 @@ import org.elasticsearch.test.transport.CapturingTransport; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportException; import org.elasticsearch.transport.TransportResponse; import org.elasticsearch.transport.TransportService; @@ -259,7 +260,7 @@ public void testReplicaProxy() throws InterruptedException, ExecutionException { transportService.start(); transportService.acceptIncomingRequests(); ShardStateAction shardStateAction = new ShardStateAction(Settings.EMPTY, clusterService, transportService, null, null, threadPool); - TestAction action = new TestAction(Settings.EMPTY, "testAction", transportService, + TestAction action = new TestAction(Settings.EMPTY, "internal:testAction", transportService, clusterService, shardStateAction, threadPool); final String index = "test"; final ShardId shardId = new ShardId(index, "_na_", 0); @@ -355,10 +356,10 @@ protected TestAction() { } protected TestAction(boolean withDocumentFailureOnPrimary, boolean withDocumentFailureOnReplica) { - super(Settings.EMPTY, "test", - new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, - Collections.emptySet()), null, - null, null, null, new ActionFilters(new HashSet<>()), new IndexNameExpressionResolver(Settings.EMPTY), TestRequest::new, + super(Settings.EMPTY, "internal:test", + new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, + x -> null, null, Collections.emptySet()), null, null, null, null, + new ActionFilters(new HashSet<>()), new IndexNameExpressionResolver(Settings.EMPTY), TestRequest::new, TestRequest::new, ThreadPool.Names.SAME); this.withDocumentFailureOnPrimary = withDocumentFailureOnPrimary; this.withDocumentFailureOnReplica = withDocumentFailureOnReplica; diff --git a/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java b/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java index 9be0d55d77e6a..669a678b77e31 100644 --- a/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java +++ b/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java @@ -27,6 +27,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.CheckedBiConsumer; +import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.component.Lifecycle; import org.elasticsearch.common.component.LifecycleListener; import org.elasticsearch.common.settings.Settings; @@ -34,13 +35,14 @@ import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.transport.ConnectTransportException; import org.elasticsearch.transport.ConnectionProfile; +import org.elasticsearch.transport.RequestHandlerRegistry; import org.elasticsearch.transport.Transport; +import org.elasticsearch.transport.TransportConnectionListener; import org.elasticsearch.transport.TransportException; import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.transport.TransportRequestOptions; import org.elasticsearch.transport.TransportResponse; import org.elasticsearch.transport.TransportResponseHandler; -import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.TransportStats; import java.io.IOException; @@ -51,22 +53,22 @@ import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicLong; abstract class FailAndRetryMockTransport implements Transport { private final Random random; private final ClusterName clusterName; + private volatile Map requestHandlers = Collections.emptyMap(); + final Object requestHandlerMutex = new Object(); + private final ResponseHandlers responseHandlers = new ResponseHandlers(); + private TransportConnectionListener listener; private boolean connectMode = true; - private TransportService transportService; - private final AtomicInteger connectTransportExceptions = new AtomicInteger(); private final AtomicInteger failures = new AtomicInteger(); private final AtomicInteger successes = new AtomicInteger(); private final Set triedNodes = new CopyOnWriteArraySet<>(); - private final AtomicLong requestId = new AtomicLong(); FailAndRetryMockTransport(Random random, ClusterName clusterName) { this.random = new Random(random.nextLong()); @@ -90,12 +92,12 @@ public void sendRequest(long requestId, String action, TransportRequest request, //we make sure that nodes get added to the connected ones when calling addTransportAddress, by returning proper nodes info if (connectMode) { if (TransportLivenessAction.NAME.equals(action)) { - TransportResponseHandler transportResponseHandler = transportService.onResponseReceived(requestId); + TransportResponseHandler transportResponseHandler = responseHandlers.onResponseReceived(requestId, listener); transportResponseHandler.handleResponse(new LivenessResponse(ClusterName.CLUSTER_NAME_SETTING. getDefault(Settings.EMPTY), node)); } else if (ClusterStateAction.NAME.equals(action)) { - TransportResponseHandler transportResponseHandler = transportService.onResponseReceived(requestId); + TransportResponseHandler transportResponseHandler = responseHandlers.onResponseReceived(requestId, listener); ClusterState clusterState = getMockClusterState(node); transportResponseHandler.handleResponse(new ClusterStateResponse(clusterName, clusterState, 0L)); } else { @@ -116,7 +118,7 @@ public void sendRequest(long requestId, String action, TransportRequest request, //throw whatever exception that is not a subclass of ConnectTransportException throw new IllegalStateException(); } else { - TransportResponseHandler transportResponseHandler = transportService.onResponseReceived(requestId); + TransportResponseHandler transportResponseHandler = responseHandlers.onResponseReceived(requestId, listener); if (random.nextBoolean()) { successes.incrementAndGet(); transportResponseHandler.handleResponse(newResponse()); @@ -162,10 +164,6 @@ public Set triedNodes() { return triedNodes; } - @Override - public void setTransportService(TransportService transportServiceAdapter) { - this.transportService = transportServiceAdapter; - } @Override public BoundTransportAddress boundAddress() { @@ -224,12 +222,36 @@ public Map profileBoundAddresses() { } @Override - public long newRequestId() { - return requestId.incrementAndGet(); + public TransportStats getStats() { + throw new UnsupportedOperationException(); } @Override - public TransportStats getStats() { + public void registerRequestHandler(RequestHandlerRegistry reg) { + synchronized (requestHandlerMutex) { + if (requestHandlers.containsKey(reg.getAction())) { + throw new IllegalArgumentException("transport handlers for action " + reg.getAction() + " is already registered"); + } + requestHandlers = MapBuilder.newMapBuilder(requestHandlers).put(reg.getAction(), reg).immutableMap(); + } + } + @Override + public ResponseHandlers getResponseHandlers() { + return responseHandlers; + } + + @Override + public RequestHandlerRegistry getRequestHandler(String action) { + return requestHandlers.get(action); + } + + @Override + public void addConnectionListener(TransportConnectionListener listener) { + this.listener = listener; + } + + @Override + public boolean removeConnectionListener(TransportConnectionListener listener) { throw new UnsupportedOperationException(); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java index 828b385f85fa5..eb93dad5db8a0 100644 --- a/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java @@ -35,7 +35,9 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.ConnectTransportException; import org.elasticsearch.transport.ConnectionProfile; +import org.elasticsearch.transport.RequestHandlerRegistry; import org.elasticsearch.transport.Transport; +import org.elasticsearch.transport.TransportConnectionListener; import org.elasticsearch.transport.TransportException; import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.transport.TransportRequestOptions; @@ -54,7 +56,6 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; import static org.hamcrest.Matchers.equalTo; @@ -171,12 +172,28 @@ public void tearDown() throws Exception { } final class MockTransport implements Transport { - private final AtomicLong requestId = new AtomicLong(); Set connectedNodes = ConcurrentCollections.newConcurrentSet(); volatile boolean randomConnectionExceptions = false; + private ResponseHandlers responseHandlers = new ResponseHandlers(); + private TransportConnectionListener listener = new TransportConnectionListener() {}; @Override - public void setTransportService(TransportService service) { + public void registerRequestHandler(RequestHandlerRegistry reg) { + } + + @Override + public RequestHandlerRegistry getRequestHandler(String action) { + return null; + } + + @Override + public void addConnectionListener(TransportConnectionListener listener) { + this.listener = listener; + } + + @Override + public boolean removeConnectionListener(TransportConnectionListener listener) { + throw new UnsupportedOperationException(); } @Override @@ -208,12 +225,14 @@ public void connectToNode(DiscoveryNode node, ConnectionProfile connectionProfil throw new ConnectTransportException(node, "simulated"); } connectedNodes.add(node); + listener.onNodeConnected(node); } } @Override public void disconnectFromNode(DiscoveryNode node) { connectedNodes.remove(node); + listener.onNodeDisconnected(node); } @Override @@ -226,20 +245,22 @@ public DiscoveryNode getNode() { @Override public void sendRequest(long requestId, String action, TransportRequest request, TransportRequestOptions options) - throws IOException, TransportException { + throws TransportException { } @Override - public void close() throws IOException { + public void close() { } }; } @Override - public Connection openConnection(DiscoveryNode node, ConnectionProfile profile) throws IOException { - return getConnection(node); + public Connection openConnection(DiscoveryNode node, ConnectionProfile profile) { + Connection connection = getConnection(node); + listener.onConnectionOpened(connection); + return connection; } @Override @@ -247,11 +268,6 @@ public List getLocalAddresses() { return null; } - @Override - public long newRequestId() { - return requestId.incrementAndGet(); - } - @Override public Lifecycle.State lifecycleState() { return null; @@ -278,5 +294,10 @@ public void close() {} public TransportStats getStats() { throw new UnsupportedOperationException(); } + + @Override + public ResponseHandlers getResponseHandlers() { + return responseHandlers; + } } } diff --git a/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java b/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java index 4b4ad67f356a8..7ae4e5b76af42 100644 --- a/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java +++ b/server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java @@ -140,7 +140,7 @@ public ClusterStateChanges(NamedXContentRegistry xContentRegistry, ThreadPool th IndexNameExpressionResolver indexNameExpressionResolver = new IndexNameExpressionResolver(settings); DestructiveOperations destructiveOperations = new DestructiveOperations(settings, clusterSettings); Environment environment = TestEnvironment.newEnvironment(settings); - Transport transport = null; // it's not used + Transport transport = mock(Transport.class); // it's not used // mocks clusterService = mock(ClusterService.class); diff --git a/server/src/test/java/org/elasticsearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java b/server/src/test/java/org/elasticsearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java index 4a496167c80c1..cc971ed1b043a 100644 --- a/server/src/test/java/org/elasticsearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java +++ b/server/src/test/java/org/elasticsearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java @@ -53,6 +53,7 @@ import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import java.util.ArrayList; @@ -399,7 +400,7 @@ private IndicesClusterStateService createIndicesClusterStateService(DiscoveryNod when(threadPool.generic()).thenReturn(mock(ExecutorService.class)); final MockIndicesService indicesService = indicesServiceSupplier.get(); final Settings settings = Settings.builder().put("node.name", discoveryNode.getName()).build(); - final TransportService transportService = new TransportService(settings, null, threadPool, + final TransportService transportService = new TransportService(settings, mock(Transport.class), threadPool, TransportService.NOOP_TRANSPORT_INTERCEPTOR, boundAddress -> DiscoveryNode.createLocal(settings, boundAddress.publishAddress(), UUIDs.randomBase64UUID()), null, Collections.emptySet()); diff --git a/server/src/test/java/org/elasticsearch/transport/ActionNamesIT.java b/server/src/test/java/org/elasticsearch/transport/ActionNamesIT.java deleted file mode 100644 index 8ad8d42e5ef99..0000000000000 --- a/server/src/test/java/org/elasticsearch/transport/ActionNamesIT.java +++ /dev/null @@ -1,54 +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.transport; - -import org.elasticsearch.test.ESIntegTestCase; - -import static org.hamcrest.CoreMatchers.either; -import static org.hamcrest.CoreMatchers.startsWith; - -/** - * This test verifies that all of the action names follow our defined naming conventions. - * The identified categories are: - * - indices:admin: apis that allow to perform administration tasks against indices - * - indices:data: apis that are about data - * - indices:read: apis that read data - * - indices:write: apis that write data - * - cluster:admin: cluster apis that allow to perform administration tasks - * - cluster:monitor: cluster apis that allow to monitor the system - * - internal: internal actions that are used from node to node but not directly exposed to users - * - * Any transport action belongs to one of the above categories and its name starts with its category, followed by a '/' - * and the name of the api itself (e.g. cluster:admin/nodes/restart). - * When an api exposes multiple transport handlers, some of which are invoked internally during the execution of the api, - * we use the `[n]` suffix to identify node actions and the `[s]` suffix to identify shard actions. - */ -public class ActionNamesIT extends ESIntegTestCase { - public void testActionNamesCategories() throws NoSuchFieldException, IllegalAccessException { - TransportService transportService = internalCluster().getInstance(TransportService.class); - for (String action : transportService.requestHandlers.keySet()) { - assertThat("action doesn't belong to known category", action, - either(startsWith("indices:admin")).or(startsWith("indices:monitor")) - .or(startsWith("indices:data/read")).or(startsWith("indices:data/write")) - .or(startsWith("cluster:admin")).or(startsWith("cluster:monitor")) - .or(startsWith("internal:"))); - } - } -} diff --git a/server/src/test/java/org/elasticsearch/transport/TransportActionProxyTests.java b/server/src/test/java/org/elasticsearch/transport/TransportActionProxyTests.java index 3f4ae7bdd2d76..46e1ffdfbce23 100644 --- a/server/src/test/java/org/elasticsearch/transport/TransportActionProxyTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TransportActionProxyTests.java @@ -87,36 +87,36 @@ private MockTransportService buildService(final Version version) { public void testSendMessage() throws InterruptedException { - serviceA.registerRequestHandler("/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceA.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); response.targetNode = "TS_A"; channel.sendResponse(response); }); - TransportActionProxy.registerProxyAction(serviceA, "/test", SimpleTestResponse::new); + TransportActionProxy.registerProxyAction(serviceA, "internal:test", SimpleTestResponse::new); serviceA.connectToNode(nodeB); - serviceB.registerRequestHandler("/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceB.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); response.targetNode = "TS_B"; channel.sendResponse(response); }); - TransportActionProxy.registerProxyAction(serviceB, "/test", SimpleTestResponse::new); + TransportActionProxy.registerProxyAction(serviceB, "internal:test", SimpleTestResponse::new); serviceB.connectToNode(nodeC); - serviceC.registerRequestHandler("/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceC.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); response.targetNode = "TS_C"; channel.sendResponse(response); }); - TransportActionProxy.registerProxyAction(serviceC, "/test", SimpleTestResponse::new); + TransportActionProxy.registerProxyAction(serviceC, "internal:test", SimpleTestResponse::new); CountDownLatch latch = new CountDownLatch(1); - serviceA.sendRequest(nodeB, TransportActionProxy.getProxyAction("/test"), TransportActionProxy.wrapRequest(nodeC, + serviceA.sendRequest(nodeB, TransportActionProxy.getProxyAction("internal:test"), TransportActionProxy.wrapRequest(nodeC, new SimpleTestRequest("TS_A")), new TransportResponseHandler() { @Override public SimpleTestResponse newInstance() { @@ -150,33 +150,33 @@ public String executor() { } public void testException() throws InterruptedException { - serviceA.registerRequestHandler("/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceA.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); response.targetNode = "TS_A"; channel.sendResponse(response); }); - TransportActionProxy.registerProxyAction(serviceA, "/test", SimpleTestResponse::new); + TransportActionProxy.registerProxyAction(serviceA, "internal:test", SimpleTestResponse::new); serviceA.connectToNode(nodeB); - serviceB.registerRequestHandler("/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceB.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); response.targetNode = "TS_B"; channel.sendResponse(response); }); - TransportActionProxy.registerProxyAction(serviceB, "/test", SimpleTestResponse::new); + TransportActionProxy.registerProxyAction(serviceB, "internal:test", SimpleTestResponse::new); serviceB.connectToNode(nodeC); - serviceC.registerRequestHandler("/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceC.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { throw new ElasticsearchException("greetings from TS_C"); }); - TransportActionProxy.registerProxyAction(serviceC, "/test", SimpleTestResponse::new); + TransportActionProxy.registerProxyAction(serviceC, "internal:test", SimpleTestResponse::new); CountDownLatch latch = new CountDownLatch(1); - serviceA.sendRequest(nodeB, TransportActionProxy.getProxyAction("/test"), TransportActionProxy.wrapRequest(nodeC, + serviceA.sendRequest(nodeB, TransportActionProxy.getProxyAction("internal:test"), TransportActionProxy.wrapRequest(nodeC, new SimpleTestRequest("TS_A")), new TransportResponseHandler() { @Override public SimpleTestResponse newInstance() { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index 4a5b67c2e6f16..2197b09988042 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -104,6 +104,7 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -139,6 +140,7 @@ import org.elasticsearch.indices.store.IndicesStore; import org.elasticsearch.ingest.IngestMetadata; import org.elasticsearch.node.NodeMocksPlugin; +import org.elasticsearch.plugins.NetworkPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.script.ScriptMetaData; @@ -152,6 +154,10 @@ import org.elasticsearch.test.disruption.ServiceDisruptionScheme; import org.elasticsearch.test.store.MockFSIndexStore; import org.elasticsearch.test.transport.MockTransportService; +import org.elasticsearch.transport.TransportInterceptor; +import org.elasticsearch.transport.TransportRequest; +import org.elasticsearch.transport.TransportRequestHandler; +import org.elasticsearch.transport.TransportService; import org.hamcrest.Matchers; import org.junit.After; import org.junit.AfterClass; @@ -2005,6 +2011,7 @@ protected Collection> getMockPlugins() { mocks.add(TestZenDiscovery.TestPlugin.class); } mocks.add(TestSeedPlugin.class); + mocks.add(AssertActionNamePlugin.class); return Collections.unmodifiableList(mocks); } @@ -2015,6 +2022,25 @@ public List> getSettings() { } } + public static final class AssertActionNamePlugin extends Plugin implements NetworkPlugin { + @Override + public List getTransportInterceptors(NamedWriteableRegistry namedWriteableRegistry, + ThreadContext threadContext) { + return Arrays.asList(new TransportInterceptor() { + @Override + public TransportRequestHandler interceptHandler(String action, String executor, + boolean forceExecution, + TransportRequestHandler actualHandler) { + if (TransportService.isValidActionName(action) == false) { + throw new IllegalArgumentException("invalid action name [" + action + "] must start with one of: " + + TransportService.VALID_ACTION_PREFIXES ); + } + return actualHandler; + } + }); + } + } + /** * Returns the client ratio configured via */ diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/CapturingTransport.java b/test/framework/src/main/java/org/elasticsearch/test/transport/CapturingTransport.java index 318c70c2933d8..ffdf79c0636b2 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/CapturingTransport.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/CapturingTransport.java @@ -23,6 +23,7 @@ import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.CheckedBiConsumer; import org.elasticsearch.common.Randomness; +import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.component.Lifecycle; import org.elasticsearch.common.component.LifecycleListener; @@ -33,13 +34,14 @@ import org.elasticsearch.transport.ConnectTransportException; import org.elasticsearch.transport.ConnectionProfile; import org.elasticsearch.transport.RemoteTransportException; +import org.elasticsearch.transport.RequestHandlerRegistry; import org.elasticsearch.transport.SendRequestTransportException; import org.elasticsearch.transport.Transport; +import org.elasticsearch.transport.TransportConnectionListener; import org.elasticsearch.transport.TransportException; import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.transport.TransportRequestOptions; import org.elasticsearch.transport.TransportResponse; -import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.TransportStats; import java.io.IOException; @@ -54,14 +56,16 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicLong; import static org.apache.lucene.util.LuceneTestCase.rarely; /** A transport class that doesn't send anything but rather captures all requests for inspection from tests */ public class CapturingTransport implements Transport { - private TransportService transportService; + private volatile Map requestHandlers = Collections.emptyMap(); + final Object requestHandlerMutex = new Object(); + private final ResponseHandlers responseHandlers = new ResponseHandlers(); + private TransportConnectionListener listener; public static class CapturedRequest { public final DiscoveryNode node; @@ -79,8 +83,6 @@ public CapturedRequest(DiscoveryNode node, long requestId, String action, Transp private ConcurrentMap> requests = new ConcurrentHashMap<>(); private BlockingQueue capturedRequests = ConcurrentCollections.newBlockingQueue(); - private final AtomicLong requestId = new AtomicLong(); - /** returns all requests captured so far. Doesn't clear the captured request list. See {@link #clear()} */ public CapturedRequest[] capturedRequests() { @@ -137,7 +139,7 @@ public void clear() { /** simulate a response for the given requestId */ public void handleResponse(final long requestId, final TransportResponse response) { - transportService.onResponseReceived(requestId).handleResponse(response); + responseHandlers.onResponseReceived(requestId, listener).handleResponse(response); } /** @@ -189,7 +191,7 @@ public void handleRemoteError(final long requestId, final Throwable t) { * @param e the failure */ public void handleError(final long requestId, final TransportException e) { - transportService.onResponseReceived(requestId).handleException(e); + responseHandlers.onResponseReceived(requestId, listener).handleException(e); } @Override @@ -219,11 +221,6 @@ public TransportStats getStats() { throw new UnsupportedOperationException(); } - @Override - public void setTransportService(TransportService transportService) { - this.transportService = transportService; - } - @Override public BoundTransportAddress boundAddress() { return null; @@ -285,11 +282,6 @@ public List getLocalAddresses() { return Collections.emptyList(); } - @Override - public long newRequestId() { - return requestId.incrementAndGet(); - } - public Connection getConnection(DiscoveryNode node) { try { return openConnection(node, null); @@ -297,4 +289,40 @@ public Connection getConnection(DiscoveryNode node) { throw new UncheckedIOException(e); } } + + @Override + public void registerRequestHandler(RequestHandlerRegistry reg) { + synchronized (requestHandlerMutex) { + if (requestHandlers.containsKey(reg.getAction())) { + throw new IllegalArgumentException("transport handlers for action " + reg.getAction() + " is already registered"); + } + requestHandlers = MapBuilder.newMapBuilder(requestHandlers).put(reg.getAction(), reg).immutableMap(); + } + } + @Override + public ResponseHandlers getResponseHandlers() { + return responseHandlers; + } + + @Override + public RequestHandlerRegistry getRequestHandler(String action) { + return requestHandlers.get(action); + } + + @Override + public void addConnectionListener(TransportConnectionListener listener) { + if (this.listener != null) { + throw new IllegalStateException("listener already set"); + } + this.listener = listener; + } + + @Override + public boolean removeConnectionListener(TransportConnectionListener listener) { + if (listener == this.listener) { + this.listener = null; + return true; + } + return false; + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java index 6654444066d52..7f818de29d430 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java @@ -53,6 +53,7 @@ import org.elasticsearch.transport.RequestHandlerRegistry; import org.elasticsearch.transport.TcpTransport; import org.elasticsearch.transport.Transport; +import org.elasticsearch.transport.TransportConnectionListener; import org.elasticsearch.transport.TransportException; import org.elasticsearch.transport.TransportInterceptor; import org.elasticsearch.transport.TransportRequest; @@ -72,7 +73,6 @@ import java.util.Set; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; @@ -169,17 +169,6 @@ protected TaskManager createTaskManager(Settings settings, ThreadPool threadPool } } - private volatile String executorName; - - public void setExecutorName(final String executorName) { - this.executorName = executorName; - } - - @Override - protected ExecutorService getExecutorService() { - return executorName == null ? super.getExecutorService() : getThreadPool().executor(executorName); - } - /** * Clears all the registered rules. */ @@ -559,8 +548,23 @@ public DelegateTransport(Transport transport) { } @Override - public void setTransportService(TransportService service) { - transport.setTransportService(service); + public void addConnectionListener(TransportConnectionListener listener) { + transport.addConnectionListener(listener); + } + + @Override + public boolean removeConnectionListener(TransportConnectionListener listener) { + return transport.removeConnectionListener(listener); + } + + @Override + public void registerRequestHandler(RequestHandlerRegistry reg) { + transport.registerRequestHandler(reg); + } + + @Override + public RequestHandlerRegistry getRequestHandler(String action) { + return transport.getRequestHandler(action); } @Override @@ -595,11 +599,6 @@ public List getLocalAddresses() { return transport.getLocalAddresses(); } - @Override - public long newRequestId() { - return transport.newRequestId(); - } - @Override public Connection getConnection(DiscoveryNode node) { return new FilteredConnection(transport.getConnection(node)) { @@ -627,6 +626,11 @@ public TransportStats getStats() { return transport.getStats(); } + @Override + public ResponseHandlers getResponseHandlers() { + return transport.getResponseHandlers(); + } + @Override public Lifecycle.State lifecycleState() { return transport.lifecycleState(); diff --git a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java index 2cc6ec5690b07..eda5ae8606a07 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java @@ -203,7 +203,7 @@ public void assertNoPendingHandshakes(Transport transport) { public void testHelloWorld() { - serviceA.registerRequestHandler("sayHello", StringMessageRequest::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:sayHello", StringMessageRequest::new, ThreadPool.Names.GENERIC, (request, channel) -> { assertThat("moshe", equalTo(request.message)); try { @@ -214,7 +214,7 @@ public void testHelloWorld() { } }); - TransportFuture res = serviceB.submitRequest(nodeA, "sayHello", + TransportFuture res = serviceB.submitRequest(nodeA, "internal:sayHello", new StringMessageRequest("moshe"), new TransportResponseHandler() { @Override public StringMessageResponse newInstance() { @@ -245,7 +245,7 @@ public void handleException(TransportException exp) { assertThat(e.getMessage(), false, equalTo(true)); } - res = serviceB.submitRequest(nodeA, "sayHello", new StringMessageRequest("moshe"), + res = serviceB.submitRequest(nodeA, "internal:sayHello", new StringMessageRequest("moshe"), TransportRequestOptions.builder().withCompress(true).build(), new TransportResponseHandler() { @Override public StringMessageResponse newInstance() { @@ -279,7 +279,7 @@ public void handleException(TransportException exp) { public void testThreadContext() throws ExecutionException, InterruptedException { - serviceA.registerRequestHandler("ping_pong", StringMessageRequest::new, ThreadPool.Names.GENERIC, (request, channel) -> { + serviceA.registerRequestHandler("internal:ping_pong", StringMessageRequest::new, ThreadPool.Names.GENERIC, (request, channel) -> { assertEquals("ping_user", threadPool.getThreadContext().getHeader("test.ping.user")); assertNull(threadPool.getThreadContext().getTransient("my_private_context")); try { @@ -323,7 +323,7 @@ public void handleException(TransportException exp) { threadPool.getThreadContext().putHeader("test.ping.user", "ping_user"); threadPool.getThreadContext().putTransient("my_private_context", context); - TransportFuture res = serviceB.submitRequest(nodeA, "ping_pong", ping, responseHandler); + TransportFuture res = serviceB.submitRequest(nodeA, "internal:ping_pong", ping, responseHandler); StringMessageResponse message = res.get(); assertThat("pong", equalTo(message.message)); @@ -337,7 +337,7 @@ public void testLocalNodeConnection() throws InterruptedException { // this should be a noop serviceA.disconnectFromNode(nodeA); final AtomicReference exception = new AtomicReference<>(); - serviceA.registerRequestHandler("localNode", StringMessageRequest::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:localNode", StringMessageRequest::new, ThreadPool.Names.GENERIC, (request, channel) -> { try { channel.sendResponse(new StringMessageResponse(request.message)); @@ -347,7 +347,8 @@ public void testLocalNodeConnection() throws InterruptedException { }); final AtomicReference responseString = new AtomicReference<>(); final CountDownLatch responseLatch = new CountDownLatch(1); - serviceA.sendRequest(nodeA, "localNode", new StringMessageRequest("test"), new TransportResponseHandler() { + serviceA.sendRequest(nodeA, "internal:localNode", new StringMessageRequest("test"), + new TransportResponseHandler() { @Override public StringMessageResponse newInstance() { return new StringMessageResponse(); @@ -388,7 +389,7 @@ public void testAdapterSendReceiveCallbacks() throws Exception { fail(e.getMessage()); } }; - final String ACTION = "action"; + final String ACTION = "internal:action"; serviceA.registerRequestHandler(ACTION, TransportRequest.Empty::new, ThreadPool.Names.GENERIC, requestHandler); serviceB.registerRequestHandler(ACTION, TransportRequest.Empty::new, ThreadPool.Names.GENERIC, @@ -483,7 +484,7 @@ public void requestSent(DiscoveryNode node, long requestId, String action, Trans } public void testVoidMessageCompressed() { - serviceA.registerRequestHandler("sayHello", TransportRequest.Empty::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:sayHello", TransportRequest.Empty::new, ThreadPool.Names.GENERIC, (request, channel) -> { try { TransportResponseOptions responseOptions = TransportResponseOptions.builder().withCompress(true).build(); @@ -494,7 +495,7 @@ public void testVoidMessageCompressed() { } }); - TransportFuture res = serviceB.submitRequest(nodeA, "sayHello", + TransportFuture res = serviceB.submitRequest(nodeA, "internal:sayHello", TransportRequest.Empty.INSTANCE, TransportRequestOptions.builder().withCompress(true).build(), new TransportResponseHandler() { @Override @@ -527,7 +528,7 @@ public void handleException(TransportException exp) { } public void testHelloWorldCompressed() { - serviceA.registerRequestHandler("sayHello", StringMessageRequest::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:sayHello", StringMessageRequest::new, ThreadPool.Names.GENERIC, new TransportRequestHandler() { @Override public void messageReceived(StringMessageRequest request, TransportChannel channel) { @@ -542,7 +543,7 @@ public void messageReceived(StringMessageRequest request, TransportChannel chann } }); - TransportFuture res = serviceB.submitRequest(nodeA, "sayHello", + TransportFuture res = serviceB.submitRequest(nodeA, "internal:sayHello", new StringMessageRequest("moshe"), TransportRequestOptions.builder().withCompress(true).build(), new TransportResponseHandler() { @Override @@ -576,7 +577,7 @@ public void handleException(TransportException exp) { } public void testErrorMessage() { - serviceA.registerRequestHandler("sayHelloException", StringMessageRequest::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:sayHelloException", StringMessageRequest::new, ThreadPool.Names.GENERIC, new TransportRequestHandler() { @Override public void messageReceived(StringMessageRequest request, TransportChannel channel) throws Exception { @@ -585,7 +586,7 @@ public void messageReceived(StringMessageRequest request, TransportChannel chann } }); - TransportFuture res = serviceB.submitRequest(nodeA, "sayHelloException", + TransportFuture res = serviceB.submitRequest(nodeA, "internal:sayHelloException", new StringMessageRequest("moshe"), new TransportResponseHandler() { @Override public StringMessageResponse newInstance() { @@ -637,7 +638,7 @@ public void onNodeDisconnected(DiscoveryNode node) { public void testConcurrentSendRespondAndDisconnect() throws BrokenBarrierException, InterruptedException { Set sendingErrors = ConcurrentCollections.newConcurrentSet(); Set responseErrors = ConcurrentCollections.newConcurrentSet(); - serviceA.registerRequestHandler("test", TestRequest::new, + serviceA.registerRequestHandler("internal:test", TestRequest::new, randomBoolean() ? ThreadPool.Names.SAME : ThreadPool.Names.GENERIC, (request, channel) -> { try { channel.sendResponse(new TestResponse()); @@ -654,7 +655,7 @@ public void testConcurrentSendRespondAndDisconnect() throws BrokenBarrierExcepti logger.trace("caught exception while responding from node B", e); } }; - serviceB.registerRequestHandler("test", TestRequest::new, ThreadPool.Names.SAME, ignoringRequestHandler); + serviceB.registerRequestHandler("internal:test", TestRequest::new, ThreadPool.Names.SAME, ignoringRequestHandler); int halfSenders = scaledRandomIntBetween(3, 10); final CyclicBarrier go = new CyclicBarrier(halfSenders * 2 + 1); @@ -710,7 +711,7 @@ protected void doRun() throws Exception { final String info = sender + "_" + iter; final DiscoveryNode node = nodeB; // capture now try { - serviceA.sendRequest(node, "test", new TestRequest(info), + serviceA.sendRequest(node, "internal:test", new TestRequest(info), new ActionListenerResponseHandler<>(listener, TestResponse::new)); try { listener.actionGet(); @@ -740,7 +741,7 @@ public void onAfter() { // simulate restart of nodeB serviceB.close(); MockTransportService newService = buildService("TS_B_" + i, version1, null); - newService.registerRequestHandler("test", TestRequest::new, ThreadPool.Names.SAME, ignoringRequestHandler); + newService.registerRequestHandler("internal:test", TestRequest::new, ThreadPool.Names.SAME, ignoringRequestHandler); serviceB = newService; nodeB = newService.getLocalDiscoNode(); serviceB.connectToNode(nodeA); @@ -761,7 +762,7 @@ public void onAfter() { public void testNotifyOnShutdown() throws Exception { final CountDownLatch latch2 = new CountDownLatch(1); try { - serviceA.registerRequestHandler("foobar", StringMessageRequest::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:foobar", StringMessageRequest::new, ThreadPool.Names.GENERIC, (request, channel) -> { try { latch2.await(); @@ -771,7 +772,7 @@ public void testNotifyOnShutdown() throws Exception { fail(e.getMessage()); } }); - TransportFuture foobar = serviceB.submitRequest(nodeA, "foobar", + TransportFuture foobar = serviceB.submitRequest(nodeA, "internal:foobar", new StringMessageRequest(""), TransportRequestOptions.EMPTY, EmptyTransportResponseHandler.INSTANCE_SAME); latch2.countDown(); try { @@ -787,7 +788,7 @@ public void testNotifyOnShutdown() throws Exception { } public void testTimeoutSendExceptionWithNeverSendingBackResponse() throws Exception { - serviceA.registerRequestHandler("sayHelloTimeoutNoResponse", StringMessageRequest::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:sayHelloTimeoutNoResponse", StringMessageRequest::new, ThreadPool.Names.GENERIC, new TransportRequestHandler() { @Override public void messageReceived(StringMessageRequest request, TransportChannel channel) { @@ -796,7 +797,7 @@ public void messageReceived(StringMessageRequest request, TransportChannel chann } }); - TransportFuture res = serviceB.submitRequest(nodeA, "sayHelloTimeoutNoResponse", + TransportFuture res = serviceB.submitRequest(nodeA, "internal:sayHelloTimeoutNoResponse", new StringMessageRequest("moshe"), TransportRequestOptions.builder().withTimeout(100).build(), new TransportResponseHandler() { @Override @@ -832,7 +833,7 @@ public void testTimeoutSendExceptionWithDelayedResponse() throws Exception { CountDownLatch waitForever = new CountDownLatch(1); CountDownLatch doneWaitingForever = new CountDownLatch(1); Semaphore inFlight = new Semaphore(Integer.MAX_VALUE); - serviceA.registerRequestHandler("sayHelloTimeoutDelayedResponse", StringMessageRequest::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:sayHelloTimeoutDelayedResponse", StringMessageRequest::new, ThreadPool.Names.GENERIC, new TransportRequestHandler() { @Override public void messageReceived(StringMessageRequest request, TransportChannel channel) throws InterruptedException { @@ -860,7 +861,7 @@ public void messageReceived(StringMessageRequest request, TransportChannel chann } }); final CountDownLatch latch = new CountDownLatch(1); - TransportFuture res = serviceB.submitRequest(nodeA, "sayHelloTimeoutDelayedResponse", + TransportFuture res = serviceB.submitRequest(nodeA, "internal:sayHelloTimeoutDelayedResponse", new StringMessageRequest("forever"), TransportRequestOptions.builder().withTimeout(100).build(), new TransportResponseHandler() { @Override @@ -898,7 +899,7 @@ public void handleException(TransportException exp) { for (int i = 0; i < 10; i++) { final int counter = i; // now, try and send another request, this times, with a short timeout - TransportFuture result = serviceB.submitRequest(nodeA, "sayHelloTimeoutDelayedResponse", + TransportFuture result = serviceB.submitRequest(nodeA, "internal:sayHelloTimeoutDelayedResponse", new StringMessageRequest(counter + "ms"), TransportRequestOptions.builder().withTimeout(3000).build(), new TransportResponseHandler() { @Override @@ -973,12 +974,12 @@ public String executor() { } }; - serviceA.registerRequestHandler("test", StringMessageRequest::new, ThreadPool.Names.SAME, handler); - serviceA.registerRequestHandler("testError", StringMessageRequest::new, ThreadPool.Names.SAME, handlerWithError); - serviceB.registerRequestHandler("test", StringMessageRequest::new, ThreadPool.Names.SAME, handler); - serviceB.registerRequestHandler("testError", StringMessageRequest::new, ThreadPool.Names.SAME, handlerWithError); + serviceA.registerRequestHandler("internal:test", StringMessageRequest::new, ThreadPool.Names.SAME, handler); + serviceA.registerRequestHandler("internal:testError", StringMessageRequest::new, ThreadPool.Names.SAME, handlerWithError); + serviceB.registerRequestHandler("internal:test", StringMessageRequest::new, ThreadPool.Names.SAME, handler); + serviceB.registerRequestHandler("internal:testError", StringMessageRequest::new, ThreadPool.Names.SAME, handlerWithError); - final Tracer tracer = new Tracer(new HashSet<>(Arrays.asList("test", "testError"))); + final Tracer tracer = new Tracer(new HashSet<>(Arrays.asList("internal:test", "internal:testError"))); // the tracer is invoked concurrently after the actual action is executed. that means a Tracer#requestSent can still be in-flight // from a handshake executed on connect in the setup method. this might confuse this test since it expects exact number of // invocations. To prevent any unrelated events messing with this test we filter on the actions we execute in this test. @@ -989,7 +990,7 @@ public String executor() { boolean timeout = randomBoolean(); TransportRequestOptions options = timeout ? TransportRequestOptions.builder().withTimeout(1).build() : TransportRequestOptions.EMPTY; - serviceA.sendRequest(nodeB, "test", new StringMessageRequest("", 10), options, noopResponseHandler); + serviceA.sendRequest(nodeB, "internal:test", new StringMessageRequest("", 10), options, noopResponseHandler); requestCompleted.acquire(); tracer.expectedEvents.get().await(); assertThat("didn't see request sent", tracer.sawRequestSent, equalTo(true)); @@ -999,7 +1000,7 @@ public String executor() { assertThat("saw error sent", tracer.sawErrorSent, equalTo(false)); tracer.reset(4); - serviceA.sendRequest(nodeB, "testError", new StringMessageRequest(""), noopResponseHandler); + serviceA.sendRequest(nodeB, "internal:testError", new StringMessageRequest(""), noopResponseHandler); requestCompleted.acquire(); tracer.expectedEvents.get().await(); assertThat("didn't see request sent", tracer.sawRequestSent, equalTo(true)); @@ -1015,7 +1016,7 @@ public String executor() { includeSettings = randomBoolean() ? "*" : ""; excludeSettings = "*Error"; } else { - includeSettings = "test"; + includeSettings = "internal:test"; excludeSettings = "DOESN'T_MATCH"; } clusterSettings.applySettings(Settings.builder() @@ -1024,7 +1025,7 @@ public String executor() { .build()); tracer.reset(4); - serviceA.sendRequest(nodeB, "test", new StringMessageRequest(""), noopResponseHandler); + serviceA.sendRequest(nodeB, "internal:test", new StringMessageRequest(""), noopResponseHandler); requestCompleted.acquire(); tracer.expectedEvents.get().await(); assertThat("didn't see request sent", tracer.sawRequestSent, equalTo(true)); @@ -1034,7 +1035,7 @@ public String executor() { assertThat("saw error sent", tracer.sawErrorSent, equalTo(false)); tracer.reset(2); - serviceA.sendRequest(nodeB, "testError", new StringMessageRequest(""), noopResponseHandler); + serviceA.sendRequest(nodeB, "internal:testError", new StringMessageRequest(""), noopResponseHandler); requestCompleted.acquire(); tracer.expectedEvents.get().await(); assertThat("saw request sent", tracer.sawRequestSent, equalTo(false)); @@ -1253,7 +1254,7 @@ public void writeTo(StreamOutput out) throws IOException { } public void testVersionFrom0to1() throws Exception { - serviceB.registerRequestHandler("/version", Version1Request::new, ThreadPool.Names.SAME, + serviceB.registerRequestHandler("internal:version", Version1Request::new, ThreadPool.Names.SAME, new TransportRequestHandler() { @Override public void messageReceived(Version1Request request, TransportChannel channel) throws Exception { @@ -1269,7 +1270,7 @@ public void messageReceived(Version1Request request, TransportChannel channel) t Version0Request version0Request = new Version0Request(); version0Request.value1 = 1; - Version0Response version0Response = serviceA.submitRequest(nodeB, "/version", version0Request, + Version0Response version0Response = serviceA.submitRequest(nodeB, "internal:version", version0Request, new TransportResponseHandler() { @Override public Version0Response newInstance() { @@ -1297,7 +1298,7 @@ public String executor() { } public void testVersionFrom1to0() throws Exception { - serviceA.registerRequestHandler("/version", Version0Request::new, ThreadPool.Names.SAME, + serviceA.registerRequestHandler("internal:version", Version0Request::new, ThreadPool.Names.SAME, new TransportRequestHandler() { @Override public void messageReceived(Version0Request request, TransportChannel channel) throws Exception { @@ -1312,7 +1313,7 @@ public void messageReceived(Version0Request request, TransportChannel channel) t Version1Request version1Request = new Version1Request(); version1Request.value1 = 1; version1Request.value2 = 2; - Version1Response version1Response = serviceB.submitRequest(nodeA, "/version", version1Request, + Version1Response version1Response = serviceB.submitRequest(nodeA, "internal:version", version1Request, new TransportResponseHandler() { @Override public Version1Response newInstance() { @@ -1342,7 +1343,7 @@ public String executor() { } public void testVersionFrom1to1() throws Exception { - serviceB.registerRequestHandler("/version", Version1Request::new, ThreadPool.Names.SAME, + serviceB.registerRequestHandler("internal:/version", Version1Request::new, ThreadPool.Names.SAME, (request, channel) -> { assertThat(request.value1, equalTo(1)); assertThat(request.value2, equalTo(2)); @@ -1356,7 +1357,7 @@ public void testVersionFrom1to1() throws Exception { Version1Request version1Request = new Version1Request(); version1Request.value1 = 1; version1Request.value2 = 2; - Version1Response version1Response = serviceB.submitRequest(nodeB, "/version", version1Request, + Version1Response version1Response = serviceB.submitRequest(nodeB, "internal:version", version1Request, new TransportResponseHandler() { @Override public Version1Response newInstance() { @@ -1386,7 +1387,7 @@ public String executor() { } public void testVersionFrom0to0() throws Exception { - serviceA.registerRequestHandler("/version", Version0Request::new, ThreadPool.Names.SAME, + serviceA.registerRequestHandler("internal:/version", Version0Request::new, ThreadPool.Names.SAME, (request, channel) -> { assertThat(request.value1, equalTo(1)); Version0Response response = new Version0Response(); @@ -1397,7 +1398,7 @@ public void testVersionFrom0to0() throws Exception { Version0Request version0Request = new Version0Request(); version0Request.value1 = 1; - Version0Response version0Response = serviceA.submitRequest(nodeA, "/version", version0Request, + Version0Response version0Response = serviceA.submitRequest(nodeA, "internal:version", version0Request, new TransportResponseHandler() { @Override public Version0Response newInstance() { @@ -1425,7 +1426,7 @@ public String executor() { } public void testMockFailToSendNoConnectRule() throws Exception { - serviceA.registerRequestHandler("sayHello", StringMessageRequest::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:sayHello", StringMessageRequest::new, ThreadPool.Names.GENERIC, (request, channel) -> { assertThat("moshe", equalTo(request.message)); throw new RuntimeException("bad message !!!"); @@ -1433,7 +1434,7 @@ public void testMockFailToSendNoConnectRule() throws Exception { serviceB.addFailToSendNoConnectRule(serviceA); - TransportFuture res = serviceB.submitRequest(nodeA, "sayHello", + TransportFuture res = serviceB.submitRequest(nodeA, "internal:sayHello", new StringMessageRequest("moshe"), new TransportResponseHandler() { @Override public StringMessageResponse newInstance() { @@ -1482,7 +1483,7 @@ public void handleException(TransportException exp) { } public void testMockUnresponsiveRule() throws IOException { - serviceA.registerRequestHandler("sayHello", StringMessageRequest::new, ThreadPool.Names.GENERIC, + serviceA.registerRequestHandler("internal:sayHello", StringMessageRequest::new, ThreadPool.Names.GENERIC, (request, channel) -> { assertThat("moshe", equalTo(request.message)); throw new RuntimeException("bad message !!!"); @@ -1490,7 +1491,7 @@ public void testMockUnresponsiveRule() throws IOException { serviceB.addUnresponsiveRule(serviceA); - TransportFuture res = serviceB.submitRequest(nodeA, "sayHello", + TransportFuture res = serviceB.submitRequest(nodeA, "internal:sayHello", new StringMessageRequest("moshe"), TransportRequestOptions.builder().withTimeout(100).build(), new TransportResponseHandler() { @Override @@ -1537,7 +1538,7 @@ public void testHostOnMessages() throws InterruptedException { final CountDownLatch latch = new CountDownLatch(2); final AtomicReference addressA = new AtomicReference<>(); final AtomicReference addressB = new AtomicReference<>(); - serviceB.registerRequestHandler("action1", TestRequest::new, ThreadPool.Names.SAME, new TransportRequestHandler() { + serviceB.registerRequestHandler("internal:action1", TestRequest::new, ThreadPool.Names.SAME, new TransportRequestHandler() { @Override public void messageReceived(TestRequest request, TransportChannel channel) throws Exception { addressA.set(request.remoteAddress()); @@ -1545,7 +1546,7 @@ public void messageReceived(TestRequest request, TransportChannel channel) throw latch.countDown(); } }); - serviceA.sendRequest(nodeB, "action1", new TestRequest(), new TransportResponseHandler() { + serviceA.sendRequest(nodeB, "internal:action1", new TestRequest(), new TransportResponseHandler() { @Override public TestResponse newInstance() { return new TestResponse(); @@ -1580,7 +1581,7 @@ public void testBlockingIncomingRequests() throws Exception { try (TransportService service = buildService("TS_TEST", version0, null, Settings.EMPTY, false, false)) { AtomicBoolean requestProcessed = new AtomicBoolean(false); - service.registerRequestHandler("action", TestRequest::new, ThreadPool.Names.SAME, + service.registerRequestHandler("internal:action", TestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { requestProcessed.set(true); channel.sendResponse(TransportResponse.Empty.INSTANCE); @@ -1592,7 +1593,7 @@ public void testBlockingIncomingRequests() throws Exception { Settings.EMPTY, true, false); try (Transport.Connection connection = serviceA.openConnection(node, null)) { CountDownLatch latch = new CountDownLatch(1); - serviceA.sendRequest(connection, "action", new TestRequest(), TransportRequestOptions.EMPTY, + serviceA.sendRequest(connection, "internal:action", new TestRequest(), TransportRequestOptions.EMPTY, new TransportResponseHandler() { @Override public TestResponse newInstance() { @@ -1754,7 +1755,7 @@ public void messageReceived(TestRequest request, TransportChannel channel) throw if (randomBoolean() && request.resendCount++ < 20) { DiscoveryNode node = randomFrom(nodeA, nodeB, nodeC); logger.debug("send secondary request from {} to {} - {}", toNodeMap.get(service), node, request.info); - service.sendRequest(node, "action1", new TestRequest("secondary " + request.info), + service.sendRequest(node, "internal:action1", new TestRequest("secondary " + request.info), TransportRequestOptions.builder().withCompress(randomBoolean()).build(), new TransportResponseHandler() { @Override @@ -1798,11 +1799,11 @@ public String executor() { } } - serviceB.registerRequestHandler("action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), + serviceB.registerRequestHandler("internal:action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), new TestRequestHandler(serviceB)); - serviceC.registerRequestHandler("action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), + serviceC.registerRequestHandler("internal:action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), new TestRequestHandler(serviceC)); - serviceA.registerRequestHandler("action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), + serviceA.registerRequestHandler("internal:action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), new TestRequestHandler(serviceA)); int iters = randomIntBetween(30, 60); CountDownLatch allRequestsDone = new CountDownLatch(iters); @@ -1845,7 +1846,7 @@ public String executor() { TransportService service = randomFrom(serviceC, serviceB, serviceA); DiscoveryNode node = randomFrom(nodeC, nodeB, nodeA); logger.debug("send from {} to {}", toNodeMap.get(service), node); - service.sendRequest(node, "action1", new TestRequest("REQ[" + i + "]"), + service.sendRequest(node, "internal:action1", new TestRequest("REQ[" + i + "]"), TransportRequestOptions.builder().withCompress(randomBoolean()).build(), new TestResponseHandler(i)); } logger.debug("waiting for response"); @@ -1866,7 +1867,7 @@ public String executor() { } public void testRegisterHandlerTwice() { - serviceB.registerRequestHandler("action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), + serviceB.registerRequestHandler("internal:action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), (request, message) -> { throw new AssertionError("boom"); }); @@ -1877,7 +1878,7 @@ public void testRegisterHandlerTwice() { }) ); - serviceA.registerRequestHandler("action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), + serviceA.registerRequestHandler("internal:action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), (request, message) -> { throw new AssertionError("boom"); }); @@ -2073,7 +2074,7 @@ public void run() { public void testResponseHeadersArePreserved() throws InterruptedException { List executors = new ArrayList<>(ThreadPool.THREAD_POOL_TYPES.keySet()); CollectionUtil.timSort(executors); // makes sure it's reproducible - serviceA.registerRequestHandler("action", TestRequest::new, ThreadPool.Names.SAME, + serviceA.registerRequestHandler("internal:action", TestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { threadPool.getThreadContext().putTransient("boom", new Object()); @@ -2125,8 +2126,8 @@ public String executor() { } }; - serviceB.sendRequest(nodeA, "action", new TestRequest(randomFrom("fail", "pass")), transportResponseHandler); - serviceA.sendRequest(nodeA, "action", new TestRequest(randomFrom("fail", "pass")), transportResponseHandler); + serviceB.sendRequest(nodeA, "internal:action", new TestRequest(randomFrom("fail", "pass")), transportResponseHandler); + serviceA.sendRequest(nodeA, "internal:action", new TestRequest(randomFrom("fail", "pass")), transportResponseHandler); latch.await(); } @@ -2134,7 +2135,7 @@ public void testHandlerIsInvokedOnConnectionClose() throws IOException, Interrup List executors = new ArrayList<>(ThreadPool.THREAD_POOL_TYPES.keySet()); CollectionUtil.timSort(executors); // makes sure it's reproducible TransportService serviceC = build(Settings.builder().put("name", "TS_TEST").build(), version0, null, true); - serviceC.registerRequestHandler("action", TestRequest::new, ThreadPool.Names.SAME, + serviceC.registerRequestHandler("internal:action", TestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { // do nothing }); @@ -2184,7 +2185,7 @@ public String executor() { TransportRequestOptions.Type.STATE); try (Transport.Connection connection = serviceB.openConnection(serviceC.getLocalNode(), builder.build())) { serviceC.close(); - serviceB.sendRequest(connection, "action", new TestRequest("boom"), TransportRequestOptions.EMPTY, + serviceB.sendRequest(connection, "internal:action", new TestRequest("boom"), TransportRequestOptions.EMPTY, transportResponseHandler); } latch.await(); @@ -2194,7 +2195,7 @@ public void testConcurrentDisconnectOnNonPublishedConnection() throws IOExceptio MockTransportService serviceC = build(Settings.builder().put("name", "TS_TEST").build(), version0, null, true); CountDownLatch receivedLatch = new CountDownLatch(1); CountDownLatch sendResponseLatch = new CountDownLatch(1); - serviceC.registerRequestHandler("action", TestRequest::new, ThreadPool.Names.SAME, + serviceC.registerRequestHandler("internal:action", TestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { // don't block on a network thread here threadPool.generic().execute(new AbstractRunnable() { @@ -2249,7 +2250,7 @@ public String executor() { TransportRequestOptions.Type.STATE); try (Transport.Connection connection = serviceB.openConnection(serviceC.getLocalNode(), builder.build())) { - serviceB.sendRequest(connection, "action", new TestRequest("hello world"), TransportRequestOptions.EMPTY, + serviceB.sendRequest(connection, "internal:action", new TestRequest("hello world"), TransportRequestOptions.EMPTY, transportResponseHandler); receivedLatch.await(); serviceC.close(); @@ -2262,7 +2263,7 @@ public void testTransportStats() throws Exception { MockTransportService serviceC = build(Settings.builder().put("name", "TS_TEST").build(), version0, null, true); CountDownLatch receivedLatch = new CountDownLatch(1); CountDownLatch sendResponseLatch = new CountDownLatch(1); - serviceB.registerRequestHandler("action", TestRequest::new, ThreadPool.Names.SAME, + serviceB.registerRequestHandler("internal:action", TestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { // don't block on a network thread here threadPool.generic().execute(new AbstractRunnable() { @@ -2329,7 +2330,7 @@ public String executor() { assertEquals(25, transportStats.getRxSize().getBytes()); assertEquals(45, transportStats.getTxSize().getBytes()); }); - serviceC.sendRequest(connection, "action", new TestRequest("hello world"), TransportRequestOptions.EMPTY, + serviceC.sendRequest(connection, "internal:action", new TestRequest("hello world"), TransportRequestOptions.EMPTY, transportResponseHandler); receivedLatch.await(); assertBusy(() -> { // netty for instance invokes this concurrently so we better use assert busy here @@ -2337,7 +2338,7 @@ public String executor() { assertEquals(1, transportStats.getRxCount()); assertEquals(2, transportStats.getTxCount()); assertEquals(25, transportStats.getRxSize().getBytes()); - assertEquals(91, transportStats.getTxSize().getBytes()); + assertEquals(100, transportStats.getTxSize().getBytes()); }); sendResponseLatch.countDown(); responseLatch.await(); @@ -2345,7 +2346,7 @@ public String executor() { assertEquals(2, stats.getRxCount()); assertEquals(2, stats.getTxCount()); assertEquals(46, stats.getRxSize().getBytes()); - assertEquals(91, stats.getTxSize().getBytes()); + assertEquals(100, stats.getTxSize().getBytes()); } finally { serviceC.close(); } @@ -2375,7 +2376,7 @@ public void testTransportStatsWithException() throws Exception { CountDownLatch sendResponseLatch = new CountDownLatch(1); Exception ex = new RuntimeException("boom"); ex.setStackTrace(new StackTraceElement[0]); - serviceB.registerRequestHandler("action", TestRequest::new, ThreadPool.Names.SAME, + serviceB.registerRequestHandler("internal:action", TestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { // don't block on a network thread here threadPool.generic().execute(new AbstractRunnable() { @@ -2442,7 +2443,7 @@ public String executor() { assertEquals(25, transportStats.getRxSize().getBytes()); assertEquals(45, transportStats.getTxSize().getBytes()); }); - serviceC.sendRequest(connection, "action", new TestRequest("hello world"), TransportRequestOptions.EMPTY, + serviceC.sendRequest(connection, "internal:action", new TestRequest("hello world"), TransportRequestOptions.EMPTY, transportResponseHandler); receivedLatch.await(); assertBusy(() -> { // netty for instance invokes this concurrently so we better use assert busy here @@ -2450,7 +2451,7 @@ public String executor() { assertEquals(1, transportStats.getRxCount()); assertEquals(2, transportStats.getTxCount()); assertEquals(25, transportStats.getRxSize().getBytes()); - assertEquals(91, transportStats.getTxSize().getBytes()); + assertEquals(100, transportStats.getTxSize().getBytes()); }); sendResponseLatch.countDown(); responseLatch.await(); @@ -2461,7 +2462,7 @@ public String executor() { // if we are bound to a IPv6 address the response address is serialized with the exception so it will be different depending // on the stack. The emphemeral port will always be in the same range assertEquals(185 + addressLen, stats.getRxSize().getBytes()); - assertEquals(91, stats.getTxSize().getBytes()); + assertEquals(100, stats.getTxSize().getBytes()); } finally { serviceC.close(); } @@ -2639,10 +2640,9 @@ public void testProfilesIncludesDefault() { .toSet())); } - public void testChannelCloseWhileConnecting() throws IOException { + public void testChannelCloseWhileConnecting() { try (MockTransportService service = build(Settings.builder().put("name", "close").build(), version0, null, true)) { - service.setExecutorName(ThreadPool.Names.SAME); // make sure stuff is executed in a blocking fashion - service.addConnectionListener(new TransportConnectionListener() { + service.transport.addConnectionListener(new TransportConnectionListener() { @Override public void onConnectionOpened(final Transport.Connection connection) { try { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/action/TransportXPackInfoActionTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/action/TransportXPackInfoActionTests.java index f87d9f33cf5e8..e46cefb77938e 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/action/TransportXPackInfoActionTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/action/TransportXPackInfoActionTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.license.LicenseService; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.XPackFeatureSet; import org.elasticsearch.license.XPackInfoResponse.FeatureSetsInfo.FeatureSet; @@ -53,8 +54,8 @@ public void testDoExecute() throws Exception { featureSets.add(fs); } - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportXPackInfoAction action = new TransportXPackInfoAction(Settings.EMPTY, mock(ThreadPool.class), transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), licenseService, featureSets); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/transport/SecurityServerTransportServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/transport/SecurityServerTransportServiceTests.java index 675c438b87fa2..c8238ab49b146 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/transport/SecurityServerTransportServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/transport/SecurityServerTransportServiceTests.java @@ -5,8 +5,6 @@ */ package org.elasticsearch.transport; -import java.util.Map; - import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.SecurityIntegTestCase; import org.elasticsearch.xpack.core.XPackSettings; @@ -24,15 +22,13 @@ protected Settings transportClientSettings() { public void testSecurityServerTransportServiceWrapsAllHandlers() { for (TransportService transportService : internalCluster().getInstances(TransportService.class)) { - for (Map.Entry entry : transportService.requestHandlers.entrySet()) { - RequestHandlerRegistry handler = entry.getValue(); - assertEquals( - "handler not wrapped by " + SecurityServerTransportInterceptor.ProfileSecuredRequestHandler.class + - "; do all the handler registration methods have overrides?", - handler.toString(), - "ProfileSecuredRequestHandler{action='" + handler.getAction() + "', executorName='" + handler.getExecutor() - + "', forceExecution=" + handler.isForceExecution() + "}"); - } + RequestHandlerRegistry handler = transportService.transport.getRequestHandler(TransportService.HANDSHAKE_ACTION_NAME); + assertEquals( + "handler not wrapped by " + SecurityServerTransportInterceptor.ProfileSecuredRequestHandler.class + + "; do all the handler registration methods have overrides?", + handler.toString(), + "ProfileSecuredRequestHandler{action='" + handler.getAction() + "', executorName='" + handler.getExecutor() + + "', forceExecution=" + handler.isForceExecution() + "}"); } } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java index 0f901830bf183..e6b7a95c37bc5 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportDeleteRoleActionTests.java @@ -12,6 +12,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.role.DeleteRoleRequest; import org.elasticsearch.xpack.core.security.action.role.DeleteRoleResponse; @@ -43,8 +44,8 @@ public class TransportDeleteRoleActionTests extends ESTestCase { public void testReservedRole() { final String roleName = randomFrom(new ArrayList<>(ReservedRolesStore.names())); NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - (x) -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, (x) -> null, null, Collections.emptySet()); TransportDeleteRoleAction action = new TransportDeleteRoleAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService); @@ -74,8 +75,8 @@ public void onFailure(Exception e) { public void testValidRole() { final String roleName = randomFrom("admin", "dept_a", "restricted"); NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - (x) -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, (x) -> null, null, Collections.emptySet()); TransportDeleteRoleAction action = new TransportDeleteRoleAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService); @@ -118,8 +119,8 @@ public void testException() { final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); final String roleName = randomFrom("admin", "dept_a", "restricted"); NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - (x) -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, (x) -> null, null, Collections.emptySet()); TransportDeleteRoleAction action = new TransportDeleteRoleAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java index 431d6cc613c16..51a682fd874e2 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.role.GetRolesRequest; import org.elasticsearch.xpack.core.security.action.role.GetRolesResponse; @@ -43,8 +44,8 @@ public class TransportGetRolesActionTests extends ESTestCase { public void testReservedRoles() { NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService + .NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetRolesAction action = new TransportGetRolesAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService, new ReservedRolesStore()); @@ -89,8 +90,8 @@ public void onFailure(Exception e) { public void testStoreRoles() { final List storeRoleDescriptors = randomRoleDescriptors(); NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetRolesAction action = new TransportGetRolesAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService, new ReservedRolesStore()); @@ -141,8 +142,8 @@ public void testGetAllOrMix() { } NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetRolesAction action = new TransportGetRolesAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService, new ReservedRolesStore()); @@ -205,8 +206,8 @@ public void testException() { final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); final List storeRoleDescriptors = randomRoleDescriptors(); NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetRolesAction action = new TransportGetRolesAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService, new ReservedRolesStore()); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java index 0ae2477ba0310..52ee2dba06d75 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportPutRoleActionTests.java @@ -12,6 +12,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.role.PutRoleRequest; import org.elasticsearch.xpack.core.security.action.role.PutRoleResponse; @@ -44,8 +45,8 @@ public class TransportPutRoleActionTests extends ESTestCase { public void testReservedRole() { final String roleName = randomFrom(new ArrayList<>(ReservedRolesStore.names())); NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportPutRoleAction action = new TransportPutRoleAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService); @@ -75,8 +76,8 @@ public void onFailure(Exception e) { public void testValidRole() { final String roleName = randomFrom("admin", "dept_a", "restricted"); NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportPutRoleAction action = new TransportPutRoleAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService); @@ -119,11 +120,10 @@ public void testException() { final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException()); final String roleName = randomFrom("admin", "dept_a", "restricted"); NativeRolesStore rolesStore = mock(NativeRolesStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportPutRoleAction action = new TransportPutRoleAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), rolesStore, transportService); - PutRoleRequest request = new PutRoleRequest(); request.name(roleName); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsActionTests.java index be3c86d6a6a52..f2508d3cb4ef0 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportGetRoleMappingsActionTests.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequest; import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsResponse; @@ -45,7 +46,7 @@ public class TransportGetRoleMappingsActionTests extends ESTestCase { @Before public void setupMocks() { store = mock(NativeRoleMappingStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); action = new TransportGetRoleMappingsAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), @@ -113,4 +114,4 @@ public void testGetAllRoles() throws Exception { assertThat(namesRef.get(), Matchers.nullValue(Set.class)); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java index da9eca7a9b61a..63597ff6963d7 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/rolemapping/TransportPutRoleMappingActionTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingRequest; import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingResponse; @@ -41,7 +42,7 @@ public class TransportPutRoleMappingActionTests extends ESTestCase { @Before public void setupMocks() { store = mock(NativeRoleMappingStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); action = new TransportPutRoleMappingAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), @@ -91,4 +92,4 @@ private PutRoleMappingResponse put(String name, FieldExpression expression, Stri action.doExecute(request, future); return future.get(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlInvalidateSessionActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlInvalidateSessionActionTests.java index 09a48a0eb1370..0b62a013e7bbd 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlInvalidateSessionActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlInvalidateSessionActionTests.java @@ -47,6 +47,7 @@ import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.client.NoOpClient; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.security.action.saml.SamlInvalidateSessionRequest; @@ -169,7 +170,7 @@ void doExecute(Action action, Request request final ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); tokenService = new TokenService(settings, Clock.systemUTC(), client, securityIndex, clusterService); - final TransportService transportService = new TransportService(Settings.EMPTY, null, null, + final TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); final Realms realms = mock(Realms.class); action = new TransportSamlInvalidateSessionAction(settings, threadPool, transportService, @@ -323,4 +324,4 @@ private Tuple storeToken(SamlNameId nameId, String session) t return future.actionGet(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutActionTests.java index eca52831d9adc..57fb470d3f813 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutActionTests.java @@ -37,6 +37,7 @@ import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xpack.core.XPackSettings; @@ -181,7 +182,7 @@ public void setup() throws Exception { final ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool); tokenService = new TokenService(settings, Clock.systemUTC(), client, securityIndex, clusterService); - final TransportService transportService = new TransportService(Settings.EMPTY, null, null, + final TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); final Realms realms = mock(Realms.class); action = new TransportSamlLogoutAction(settings, threadPool, transportService, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java index 56e714d7a7067..d09c135836a64 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java @@ -12,6 +12,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.SecurityContext; import org.elasticsearch.xpack.core.security.action.user.AuthenticateRequest; @@ -38,8 +39,8 @@ public class TransportAuthenticateActionTests extends ESTestCase { public void testInternalUser() { SecurityContext securityContext = mock(SecurityContext.class); when(securityContext.getUser()).thenReturn(randomFrom(SystemUser.INSTANCE, XPackUser.INSTANCE)); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportAuthenticateAction action = new TransportAuthenticateAction(Settings.EMPTY, mock(ThreadPool.class), transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), securityContext); @@ -64,8 +65,8 @@ public void onFailure(Exception e) { public void testNullUser() { SecurityContext securityContext = mock(SecurityContext.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportAuthenticateAction action = new TransportAuthenticateAction(Settings.EMPTY, mock(ThreadPool.class), transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), securityContext); @@ -92,8 +93,8 @@ public void testValidUser() { final User user = randomFrom(new ElasticUser(true), new KibanaUser(true), new User("joe")); SecurityContext securityContext = mock(SecurityContext.class); when(securityContext.getUser()).thenReturn(user); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportAuthenticateAction action = new TransportAuthenticateAction(Settings.EMPTY, mock(ThreadPool.class), transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), securityContext); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordActionTests.java index 78f6fd26e93ea..6e85268f5855e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportChangePasswordActionTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.SecuritySettingsSourceField; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.user.ChangePasswordRequest; import org.elasticsearch.xpack.core.security.action.user.ChangePasswordResponse; @@ -50,8 +51,8 @@ public void testAnonymousUser() { Settings settings = Settings.builder().put(AnonymousUser.ROLES_SETTING.getKey(), "superuser").build(); AnonymousUser anonymousUser = new AnonymousUser(settings); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportChangePasswordAction action = new TransportChangePasswordAction(settings, mock(ThreadPool.class), transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore); @@ -81,11 +82,10 @@ public void onFailure(Exception e) { public void testInternalUsers() { NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportChangePasswordAction action = new TransportChangePasswordAction(Settings.EMPTY, mock(ThreadPool.class), transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore); - ChangePasswordRequest request = new ChangePasswordRequest(); request.username(randomFrom(SystemUser.INSTANCE.principal(), XPackUser.INSTANCE.principal())); request.passwordHash(Hasher.BCRYPT.hash(SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)); @@ -123,11 +123,10 @@ public void testValidUser() { listener.onResponse(null); return null; }).when(usersStore).changePassword(eq(request), any(ActionListener.class)); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportChangePasswordAction action = new TransportChangePasswordAction(Settings.EMPTY, mock(ThreadPool.class), transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore); - final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>(); action.doExecute(request, new ActionListener() { @@ -164,11 +163,10 @@ public Void answer(InvocationOnMock invocation) { return null; } }).when(usersStore).changePassword(eq(request), any(ActionListener.class)); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportChangePasswordAction action = new TransportChangePasswordAction(Settings.EMPTY, mock(ThreadPool.class), transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore); - final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>(); action.doExecute(request, new ActionListener() { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportDeleteUserActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportDeleteUserActionTests.java index a60a82e87d71a..6d0b2e0356f2d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportDeleteUserActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportDeleteUserActionTests.java @@ -12,6 +12,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.user.DeleteUserRequest; import org.elasticsearch.xpack.core.security.action.user.DeleteUserResponse; @@ -47,8 +48,8 @@ public class TransportDeleteUserActionTests extends ESTestCase { public void testAnonymousUser() { Settings settings = Settings.builder().put(AnonymousUser.ROLES_SETTING.getKey(), "superuser").build(); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportDeleteUserAction action = new TransportDeleteUserAction(settings, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); @@ -76,8 +77,8 @@ public void onFailure(Exception e) { public void testInternalUser() { NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportDeleteUserAction action = new TransportDeleteUserAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); @@ -106,8 +107,8 @@ public void onFailure(Exception e) { public void testReservedUser() { final User reserved = randomFrom(new ElasticUser(true), new KibanaUser(true)); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportDeleteUserAction action = new TransportDeleteUserAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); @@ -136,8 +137,8 @@ public void onFailure(Exception e) { public void testValidUser() { final User user = new User("joe"); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportDeleteUserAction action = new TransportDeleteUserAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); @@ -177,8 +178,8 @@ public void testException() { final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new RuntimeException()); final User user = new User("joe"); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportDeleteUserAction action = new TransportDeleteUserAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportGetUsersActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportGetUsersActionTests.java index 2ad467236820f..84da4c9ffb05f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportGetUsersActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportGetUsersActionTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.user.GetUsersRequest; import org.elasticsearch.xpack.core.security.action.user.GetUsersResponse; @@ -90,8 +91,8 @@ public void testAnonymousUser() { AnonymousUser anonymousUser = new AnonymousUser(settings); ReservedRealm reservedRealm = new ReservedRealm(mock(Environment.class), settings, usersStore, anonymousUser, securityIndex, threadPool); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetUsersAction action = new TransportGetUsersAction(Settings.EMPTY, threadPool, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService, reservedRealm); @@ -125,8 +126,8 @@ public void onFailure(Exception e) { public void testInternalUser() { NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetUsersAction action = new TransportGetUsersAction(Settings.EMPTY, threadPool, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService, mock(ReservedRealm.class)); @@ -168,7 +169,7 @@ public void testReservedUsersOnly() { final int size = randomIntBetween(1, allReservedUsers.size()); final List reservedUsers = randomSubsetOf(size, allReservedUsers); final List names = reservedUsers.stream().map(User::principal).collect(Collectors.toList()); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetUsersAction action = new TransportGetUsersAction(Settings.EMPTY, threadPool, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService, reservedRealm); @@ -208,7 +209,7 @@ public void testGetAllUsers() { ReservedRealmTests.mockGetAllReservedUserInfo(usersStore, Collections.emptyMap()); ReservedRealm reservedRealm = new ReservedRealm(mock(Environment.class), settings, usersStore, new AnonymousUser(settings), securityIndex, threadPool); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetUsersAction action = new TransportGetUsersAction(Settings.EMPTY, threadPool, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService, reservedRealm); @@ -255,7 +256,7 @@ public void testGetStoreOnlyUsers() { randomFrom(Collections.singletonList(new User("joe")), Arrays.asList(new User("jane"), new User("fred")), randomUsers()); final String[] storeUsernames = storeUsers.stream().map(User::principal).collect(Collectors.toList()).toArray(Strings.EMPTY_ARRAY); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetUsersAction action = new TransportGetUsersAction(Settings.EMPTY, threadPool, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService, mock(ReservedRealm.class)); @@ -303,7 +304,7 @@ public void testException() { randomFrom(Collections.singletonList(new User("joe")), Arrays.asList(new User("jane"), new User("fred")), randomUsers()); final String[] storeUsernames = storeUsers.stream().map(User::principal).collect(Collectors.toList()).toArray(Strings.EMPTY_ARRAY); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportGetUsersAction action = new TransportGetUsersAction(Settings.EMPTY, threadPool, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService, mock(ReservedRealm.class)); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesActionTests.java index d4a256b8a0ca8..cba30cd33e88b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportHasPrivilegesActionTests.java @@ -19,6 +19,7 @@ import org.elasticsearch.mock.orig.Mockito; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest; import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; @@ -58,9 +59,8 @@ public void setup() { user = new User(randomAlphaOfLengthBetween(4, 12)); final ThreadPool threadPool = mock(ThreadPool.class); final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - final TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService - .NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + final TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); final Authentication authentication = mock(Authentication.class); threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, authentication); @@ -310,4 +310,4 @@ private static MapBuilder mapBuilder() { return MapBuilder.newMapBuilder(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportPutUserActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportPutUserActionTests.java index d059911a6807a..aef8a38e41262 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportPutUserActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportPutUserActionTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.SecuritySettingsSourceField; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.user.PutUserRequest; import org.elasticsearch.xpack.core.security.action.user.PutUserResponse; @@ -57,8 +58,8 @@ public void testAnonymousUser() { Settings settings = Settings.builder().put(AnonymousUser.ROLES_SETTING.getKey(), "superuser").build(); final AnonymousUser anonymousUser = new AnonymousUser(settings); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportPutUserAction action = new TransportPutUserAction(settings, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); @@ -87,8 +88,8 @@ public void onFailure(Exception e) { public void testSystemUser() { NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportPutUserAction action = new TransportPutUserAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); @@ -128,8 +129,8 @@ public void testReservedUser() { PlainActionFuture> userFuture = new PlainActionFuture<>(); reservedRealm.users(userFuture); final User reserved = randomFrom(userFuture.actionGet().toArray(new User[0])); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportPutUserAction action = new TransportPutUserAction(Settings.EMPTY, threadPool, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); @@ -158,8 +159,8 @@ public void onFailure(Exception e) { public void testValidUser() { final User user = new User("joe"); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportPutUserAction action = new TransportPutUserAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); @@ -204,8 +205,8 @@ public void testException() { final Exception e = randomFrom(new ElasticsearchSecurityException(""), new IllegalStateException(), new ValidationException()); final User user = new User("joe"); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportPutUserAction action = new TransportPutUserAction(Settings.EMPTY, mock(ThreadPool.class), mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore, transportService); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportSetEnabledActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportSetEnabledActionTests.java index 09fd90437523c..d371e2e737a55 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportSetEnabledActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportSetEnabledActionTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.security.action.user.SetEnabledRequest; import org.elasticsearch.xpack.core.security.action.user.SetEnabledResponse; @@ -62,8 +63,8 @@ public void testAnonymousUser() { threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, authentication); when(authentication.getUser()).thenReturn(user); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportSetEnabledAction action = new TransportSetEnabledAction(settings, threadPool, transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore); @@ -100,8 +101,8 @@ public void testInternalUser() { threadContext.putTransient(AuthenticationField.AUTHENTICATION_KEY, authentication); when(authentication.getUser()).thenReturn(user); NativeUsersStore usersStore = mock(NativeUsersStore.class); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportSetEnabledAction action = new TransportSetEnabledAction(Settings.EMPTY, threadPool, transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore); @@ -154,8 +155,8 @@ public Void answer(InvocationOnMock invocation) { } }).when(usersStore) .setEnabled(eq(user.principal()), eq(request.enabled()), eq(request.getRefreshPolicy()), any(ActionListener.class)); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportSetEnabledAction action = new TransportSetEnabledAction(Settings.EMPTY, threadPool, transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore); @@ -206,8 +207,8 @@ public Void answer(InvocationOnMock invocation) { } }).when(usersStore) .setEnabled(eq(user.principal()), eq(request.enabled()), eq(request.getRefreshPolicy()), any(ActionListener.class)); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportSetEnabledAction action = new TransportSetEnabledAction(Settings.EMPTY, threadPool, transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore); @@ -246,8 +247,8 @@ public void testUserModifyingThemselves() { request.username(user.principal()); request.enabled(randomBoolean()); request.setRefreshPolicy(randomFrom(RefreshPolicy.values())); - TransportService transportService = new TransportService(Settings.EMPTY, null, null, TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, null, Collections.emptySet()); + TransportService transportService = new TransportService(Settings.EMPTY, mock(Transport.class), null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, Collections.emptySet()); TransportSetEnabledAction action = new TransportSetEnabledAction(Settings.EMPTY, threadPool, transportService, mock(ActionFilters.class), mock(IndexNameExpressionResolver.class), usersStore); From 7a703fdf209281bddd0dd5faeedc2c17b7d717a0 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 4 Jul 2018 16:30:03 +0200 Subject: [PATCH 2/5] add missing mocks --- .../action/search/TransportMultiSearchActionTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/action/search/TransportMultiSearchActionTests.java b/server/src/test/java/org/elasticsearch/action/search/TransportMultiSearchActionTests.java index 8dd1e70857dd2..f83f8ae7fe980 100644 --- a/server/src/test/java/org/elasticsearch/action/search/TransportMultiSearchActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/TransportMultiSearchActionTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; @@ -92,6 +93,8 @@ public TaskManager getTaskManager() { }; ClusterService clusterService = mock(ClusterService.class); when(clusterService.state()).thenReturn(ClusterState.builder(new ClusterName("test")).build()); + IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(Settings.EMPTY); + TaskManager taskManager = mock(TaskManager.class); // Keep track of the number of concurrent searches started by multi search api, // and if there are more searches than is allowed create an error and remember that. From 0f8f28de57b98ad3634ed575c7cebbe6f6e6b7ef Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 4 Jul 2018 20:22:19 +0200 Subject: [PATCH 3/5] fix line len --- .../transport/AbstractSimpleTransportTestCase.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java index eda5ae8606a07..357117d47cf78 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java @@ -1538,7 +1538,8 @@ public void testHostOnMessages() throws InterruptedException { final CountDownLatch latch = new CountDownLatch(2); final AtomicReference addressA = new AtomicReference<>(); final AtomicReference addressB = new AtomicReference<>(); - serviceB.registerRequestHandler("internal:action1", TestRequest::new, ThreadPool.Names.SAME, new TransportRequestHandler() { + serviceB.registerRequestHandler("internal:action1", TestRequest::new, ThreadPool.Names.SAME, + new TransportRequestHandler() { @Override public void messageReceived(TestRequest request, TransportChannel channel) throws Exception { addressA.set(request.remoteAddress()); From 79afba1415e3aa84c7bb0d64436d2df185089341 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 4 Jul 2018 21:08:39 +0200 Subject: [PATCH 4/5] fix tp tests --- .../transport/AbstractSimpleTransportTestCase.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java index 357117d47cf78..fb8c54540ae97 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java @@ -1357,7 +1357,7 @@ public void testVersionFrom1to1() throws Exception { Version1Request version1Request = new Version1Request(); version1Request.value1 = 1; version1Request.value2 = 2; - Version1Response version1Response = serviceB.submitRequest(nodeB, "internal:version", version1Request, + Version1Response version1Response = serviceB.submitRequest(nodeB, "internal:/version", version1Request, new TransportResponseHandler() { @Override public Version1Response newInstance() { @@ -1398,7 +1398,7 @@ public void testVersionFrom0to0() throws Exception { Version0Request version0Request = new Version0Request(); version0Request.value1 = 1; - Version0Response version0Response = serviceA.submitRequest(nodeA, "internal:version", version0Request, + Version0Response version0Response = serviceA.submitRequest(nodeA, "internal:/version", version0Request, new TransportResponseHandler() { @Override public Version0Response newInstance() { @@ -1873,7 +1873,8 @@ public void testRegisterHandlerTwice() { throw new AssertionError("boom"); }); expectThrows(IllegalArgumentException.class, () -> - serviceB.registerRequestHandler("action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names.GENERIC), + serviceB.registerRequestHandler("internal:action1", TestRequest::new, randomFrom(ThreadPool.Names.SAME, ThreadPool.Names + .GENERIC), (request, message) -> { throw new AssertionError("boom"); }) @@ -2462,7 +2463,7 @@ public String executor() { int addressLen = serviceB.boundAddress().publishAddress().address().getAddress().getAddress().length; // if we are bound to a IPv6 address the response address is serialized with the exception so it will be different depending // on the stack. The emphemeral port will always be in the same range - assertEquals(185 + addressLen, stats.getRxSize().getBytes()); + assertEquals(203 + addressLen, stats.getRxSize().getBytes()); assertEquals(100, stats.getTxSize().getBytes()); } finally { serviceC.close(); From ec93b930cdbff5bb5bdc19d2d4d921eca1fcb130 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 5 Jul 2018 08:09:11 +0200 Subject: [PATCH 5/5] fix merge issues --- .../transport/TransportActionProxyTests.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/transport/TransportActionProxyTests.java b/server/src/test/java/org/elasticsearch/transport/TransportActionProxyTests.java index 46e1ffdfbce23..f684fb7dae7b1 100644 --- a/server/src/test/java/org/elasticsearch/transport/TransportActionProxyTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TransportActionProxyTests.java @@ -87,7 +87,7 @@ private MockTransportService buildService(final Version version) { public void testSendMessage() throws InterruptedException { - serviceA.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceA.registerRequestHandler("internal:test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); @@ -97,7 +97,7 @@ public void testSendMessage() throws InterruptedException { TransportActionProxy.registerProxyAction(serviceA, "internal:test", SimpleTestResponse::new); serviceA.connectToNode(nodeB); - serviceB.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceB.registerRequestHandler("internal:test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); @@ -106,7 +106,7 @@ public void testSendMessage() throws InterruptedException { }); TransportActionProxy.registerProxyAction(serviceB, "internal:test", SimpleTestResponse::new); serviceB.connectToNode(nodeC); - serviceC.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceC.registerRequestHandler("internal:test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); @@ -150,7 +150,7 @@ public String executor() { } public void testException() throws InterruptedException { - serviceA.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceA.registerRequestHandler("internal:test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); @@ -160,7 +160,7 @@ public void testException() throws InterruptedException { TransportActionProxy.registerProxyAction(serviceA, "internal:test", SimpleTestResponse::new); serviceA.connectToNode(nodeB); - serviceB.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceB.registerRequestHandler("internal:test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { assertEquals(request.sourceNode, "TS_A"); SimpleTestResponse response = new SimpleTestResponse(); @@ -169,7 +169,7 @@ public void testException() throws InterruptedException { }); TransportActionProxy.registerProxyAction(serviceB, "internal:test", SimpleTestResponse::new); serviceB.connectToNode(nodeC); - serviceC.registerRequestHandler("internal:/test", SimpleTestRequest::new, ThreadPool.Names.SAME, + serviceC.registerRequestHandler("internal:test", SimpleTestRequest::new, ThreadPool.Names.SAME, (request, channel) -> { throw new ElasticsearchException("greetings from TS_C"); });