From cadecc24661879f097a5b09c365940fd2626a7bc Mon Sep 17 00:00:00 2001 From: Bruce Schuchardt Date: Tue, 2 Jan 2018 14:26:52 -0800 Subject: [PATCH] GEODE-4092 New protocol does not have an API to get the best server to connect to GetAllAvailableServers has been replaced by GetServer. The handler uses the ServerLocator's processRequest API to ensure we have the same functionality as existing non-protobuf locator request handling. If a server isn't found we are returning Success with a null server location. Should this be changed to return an error code? --- .../experimental/driver/ProtobufDriver.java | 31 ++++----- .../src/main/proto/v1/clientProtocol.proto | 6 +- .../src/main/proto/v1/locator_API.proto | 9 +-- ...er.java => GetServerOperationHandler.java} | 64 +++++++++++++------ .../ProtobufOperationContextRegistry.java | 10 +-- .../utilities/ProtobufRequestUtilities.java | 5 +- ...atorConnectionAuthenticationDUnitTest.java | 42 +++++------- .../LocatorConnectionDUnitTest.java | 14 ++-- ...> GetServerOperationHandlerJUnitTest.java} | 63 ++++++++---------- 9 files changed, 123 insertions(+), 121 deletions(-) rename geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/{GetAvailableServersOperationHandler.java => GetServerOperationHandler.java} (50%) rename geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/{GetAvailableServersOperationHandlerJUnitTest.java => GetServerOperationHandlerJUnitTest.java} (55%) diff --git a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufDriver.java b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufDriver.java index aab5eb6e0c5b..1f6bfdecac5a 100644 --- a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufDriver.java +++ b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufDriver.java @@ -19,8 +19,6 @@ import java.io.OutputStream; import java.net.InetSocketAddress; import java.net.Socket; -import java.util.ArrayList; -import java.util.Collection; import java.util.HashSet; import java.util.Set; @@ -60,9 +58,8 @@ public class ProtobufDriver implements Driver { */ ProtobufDriver(Set locators) throws IOException { this.locators = locators; - Collection servers = getAvailableServers(); - InetSocketAddress anyServer = servers.iterator().next(); - socket = new Socket(anyServer.getAddress(), anyServer.getPort()); + InetSocketAddress server = findAServer(); + socket = new Socket(server.getAddress(), server.getPort()); final OutputStream outputStream = socket.getOutputStream(); ProtocolVersion.NewConnectionClientVersion.newBuilder() @@ -103,13 +100,12 @@ public Region getRegion(String regionName) { } /** - * Queries a locator for the GemFire servers that have Protobuf enabled. + * Queries locators for a Geode server that has Protobuf enabled. * - * @return Set of Internet-address-or-host-name/port pairs of the GemFire servers that have - * Protobuf enabled. + * @return The server chosen by the Locator service for this client * @throws IOException */ - private Collection getAvailableServers() throws IOException { + private InetSocketAddress findAServer() throws IOException { IOException lastException = null; for (InetSocketAddress locator : locators) { @@ -131,22 +127,17 @@ private Collection getAvailableServers() throws IOException { ClientProtocol.Message.newBuilder() .setRequest(ClientProtocol.Request.newBuilder() - .setGetAvailableServersRequest(LocatorAPI.GetAvailableServersRequest.newBuilder())) + .setGetServerRequest(LocatorAPI.GetServerRequest.newBuilder())) .build().writeDelimitedTo(outputStream); - LocatorAPI.GetAvailableServersResponse getAvailableServersResponse = ClientProtocol.Message - .parseDelimitedFrom(inputStream).getResponse().getGetAvailableServersResponse(); - if (getAvailableServersResponse.getServersCount() < 1) { + LocatorAPI.GetServerResponse getServerResponse = ClientProtocol.Message + .parseDelimitedFrom(inputStream).getResponse().getGetServerResponse(); + if (!getServerResponse.hasServer()) { continue; } - ArrayList availableServers = - new ArrayList<>(getAvailableServersResponse.getServersCount()); - for (int i = 0; i < getAvailableServersResponse.getServersCount(); ++i) { - final BasicTypes.Server server = getAvailableServersResponse.getServers(i); - availableServers.add(new InetSocketAddress(server.getHostname(), server.getPort())); - } - return availableServers; + BasicTypes.Server server = getServerResponse.getServer(); + return new InetSocketAddress(server.getHostname(), server.getPort()); } catch (IOException e) { lastException = e; } diff --git a/geode-protobuf-messages/src/main/proto/v1/clientProtocol.proto b/geode-protobuf-messages/src/main/proto/v1/clientProtocol.proto index b744fa6f80d4..21bc2fad3a77 100644 --- a/geode-protobuf-messages/src/main/proto/v1/clientProtocol.proto +++ b/geode-protobuf-messages/src/main/proto/v1/clientProtocol.proto @@ -41,7 +41,8 @@ message Request { GetAllRequest getAllRequest = 13; RemoveRequest removeRequest = 14; - GetAvailableServersRequest getAvailableServersRequest = 40; + GetServerRequest getServerRequest = 40; + GetRegionNamesRequest getRegionNamesRequest = 41; GetRegionRequest getRegionRequest = 42; @@ -57,7 +58,8 @@ message Response { GetAllResponse getAllResponse = 13; RemoveResponse removeResponse = 14; - GetAvailableServersResponse getAvailableServersResponse = 40; + GetServerResponse getServerResponse = 40; + GetRegionNamesResponse getRegionNamesResponse = 41; GetRegionResponse getRegionResponse = 42; diff --git a/geode-protobuf-messages/src/main/proto/v1/locator_API.proto b/geode-protobuf-messages/src/main/proto/v1/locator_API.proto index 902cec49f5f6..0f3f5746f3f0 100644 --- a/geode-protobuf-messages/src/main/proto/v1/locator_API.proto +++ b/geode-protobuf-messages/src/main/proto/v1/locator_API.proto @@ -23,10 +23,11 @@ package org.apache.geode.internal.protocol.protobuf.v1; import "v1/basicTypes.proto"; -message GetAvailableServersRequest { - +message GetServerRequest { + repeated Server excludedServers = 1; + string serverGroup = 2; } -message GetAvailableServersResponse { - repeated Server servers = 1; +message GetServerResponse { + Server server = 1; } diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandler.java similarity index 50% rename from geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandler.java rename to geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandler.java index 1979b709186d..f3a568753cfc 100644 --- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandler.java +++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandler.java @@ -14,14 +14,22 @@ */ package org.apache.geode.internal.protocol.protobuf.v1.operations; +import static org.apache.geode.internal.protocol.ProtocolErrorCode.INVALID_REQUEST; + import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import org.apache.geode.annotations.Experimental; +import org.apache.geode.cache.client.internal.locator.ClientConnectionRequest; +import org.apache.geode.cache.client.internal.locator.ClientConnectionResponse; import org.apache.geode.distributed.internal.InternalLocator; import org.apache.geode.distributed.internal.ServerLocation; import org.apache.geode.internal.exception.InvalidExecutionContextException; +import org.apache.geode.internal.protocol.Failure; import org.apache.geode.internal.protocol.MessageExecutionContext; import org.apache.geode.internal.protocol.Result; import org.apache.geode.internal.protocol.Success; @@ -31,38 +39,54 @@ import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol; import org.apache.geode.internal.protocol.protobuf.v1.LocatorAPI; import org.apache.geode.internal.protocol.protobuf.v1.ProtobufSerializationService; +import org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufResponseUtilities; import org.apache.geode.internal.protocol.serialization.SerializationService; import org.apache.geode.internal.protocol.state.ConnectionTerminatingStateProcessor; @Experimental -public class GetAvailableServersOperationHandler implements - ProtobufOperationHandler { +public class GetServerOperationHandler + implements ProtobufOperationHandler { @Override - public Result process( - ProtobufSerializationService serializationService, - LocatorAPI.GetAvailableServersRequest request, + public Result process( + ProtobufSerializationService serializationService, LocatorAPI.GetServerRequest request, MessageExecutionContext messageExecutionContext) throws InvalidExecutionContextException { + // A client may send a set of servers to exclude and/or a server-group. + Set excludedServers = new HashSet<>(); + List excludedServersList = request.getExcludedServersList(); + for (BasicTypes.Server server : excludedServersList) { + excludedServers.add(new ServerLocation(server.getHostname(), server.getPort())); + } + + // note: an empty string is okay - the ServerLocator code checks for this + String serverGroup = request.getServerGroup(); + messageExecutionContext.setConnectionStateProcessor(new ConnectionTerminatingStateProcessor()); InternalLocator internalLocator = (InternalLocator) messageExecutionContext.getLocator(); - ArrayList serversFromSnapshot = - internalLocator.getServerLocatorAdvisee().getLoadSnapshot().getServers(null); - if (serversFromSnapshot == null) { - serversFromSnapshot = new ArrayList(); + + // In order to ensure that proper checks are performed on the request we will use + // the locator's processRequest() API. We assume that all servers have Protobuf + // enabled. + ClientConnectionRequest clientConnectionRequest = + new ClientConnectionRequest(excludedServers, serverGroup); + ClientConnectionResponse connectionResponse = (ClientConnectionResponse) internalLocator + .getServerLocatorAdvisee().processRequest(clientConnectionRequest); + + ServerLocation serverLocation = null; + if (connectionResponse != null) { + serverLocation = connectionResponse.getServer(); } - Collection servers = (Collection) serversFromSnapshot - .stream().map(serverLocation -> getServerProtobufMessage((ServerLocation) serverLocation)) - .collect(Collectors.toList()); - LocatorAPI.GetAvailableServersResponse.Builder builder = - LocatorAPI.GetAvailableServersResponse.newBuilder().addAllServers(servers); - return Success.of(builder.build()); - } + LocatorAPI.GetServerResponse.Builder builder = LocatorAPI.GetServerResponse.newBuilder(); - private BasicTypes.Server getServerProtobufMessage(ServerLocation serverLocation) { - BasicTypes.Server.Builder serverBuilder = BasicTypes.Server.newBuilder(); - serverBuilder.setHostname(serverLocation.getHostName()).setPort(serverLocation.getPort()); - return serverBuilder.build(); + if (serverLocation != null) { + BasicTypes.Server.Builder serverBuilder = BasicTypes.Server.newBuilder(); + serverBuilder.setHostname(serverLocation.getHostName()).setPort(serverLocation.getPort()); + BasicTypes.Server server = serverBuilder.build(); + builder.setServer(server); + } + + return Success.of(builder.build()); } } diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/registry/ProtobufOperationContextRegistry.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/registry/ProtobufOperationContextRegistry.java index 065bd5d22a55..03c6f70eccf1 100644 --- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/registry/ProtobufOperationContextRegistry.java +++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/registry/ProtobufOperationContextRegistry.java @@ -23,10 +23,10 @@ import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol.Request.RequestAPICase; import org.apache.geode.internal.protocol.protobuf.v1.ProtobufOperationContext; import org.apache.geode.internal.protocol.protobuf.v1.operations.GetAllRequestOperationHandler; -import org.apache.geode.internal.protocol.protobuf.v1.operations.GetAvailableServersOperationHandler; import org.apache.geode.internal.protocol.protobuf.v1.operations.GetRegionNamesRequestOperationHandler; import org.apache.geode.internal.protocol.protobuf.v1.operations.GetRegionRequestOperationHandler; import org.apache.geode.internal.protocol.protobuf.v1.operations.GetRequestOperationHandler; +import org.apache.geode.internal.protocol.protobuf.v1.operations.GetServerOperationHandler; import org.apache.geode.internal.protocol.protobuf.v1.operations.PutAllRequestOperationHandler; import org.apache.geode.internal.protocol.protobuf.v1.operations.PutRequestOperationHandler; import org.apache.geode.internal.protocol.protobuf.v1.operations.RemoveRequestOperationHandler; @@ -103,10 +103,10 @@ private void addContexts() { new ResourcePermission(ResourcePermission.Resource.DATA, ResourcePermission.Operation.READ))); - operationContexts.put(RequestAPICase.GETAVAILABLESERVERSREQUEST, - new ProtobufOperationContext<>(ClientProtocol.Request::getGetAvailableServersRequest, - new GetAvailableServersOperationHandler(), - opsResp -> ClientProtocol.Response.newBuilder().setGetAvailableServersResponse(opsResp), + operationContexts.put(RequestAPICase.GETSERVERREQUEST, + new ProtobufOperationContext<>(ClientProtocol.Request::getGetServerRequest, + new GetServerOperationHandler(), + opsResp -> ClientProtocol.Response.newBuilder().setGetServerResponse(opsResp), new ResourcePermission(ResourcePermission.Resource.CLUSTER, ResourcePermission.Operation.READ))); } diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufRequestUtilities.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufRequestUtilities.java index e1f7e6aafbd3..355aa700f945 100644 --- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufRequestUtilities.java +++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufRequestUtilities.java @@ -110,9 +110,8 @@ public static ClientProtocol.Request createPutAllRequest(String regionName, return ClientProtocol.Request.newBuilder().setPutAllRequest(putAllRequestBuilder).build(); } - public static LocatorAPI.GetAvailableServersRequest createGetAvailableServersRequest() { - LocatorAPI.GetAvailableServersRequest.Builder builder = - LocatorAPI.GetAvailableServersRequest.newBuilder(); + public static LocatorAPI.GetServerRequest createGetServerRequest() { + LocatorAPI.GetServerRequest.Builder builder = LocatorAPI.GetServerRequest.newBuilder(); return builder.build(); } } diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionAuthenticationDUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionAuthenticationDUnitTest.java index 9dbe83a43f76..c93453d02811 100644 --- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionAuthenticationDUnitTest.java +++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionAuthenticationDUnitTest.java @@ -17,25 +17,21 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.net.Socket; -import java.util.List; import java.util.Properties; -import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.junit.rules.ExpectedException; import org.apache.geode.cache.server.CacheServer; import org.apache.geode.distributed.ConfigurationProperties; import org.apache.geode.distributed.Locator; import org.apache.geode.internal.cache.InternalCache; -import org.apache.geode.internal.protocol.exception.InvalidProtocolMessageException; -import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes.Server; import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol; import org.apache.geode.internal.protocol.protobuf.v1.ConnectionAPI; import org.apache.geode.internal.protocol.protobuf.v1.MessageUtil; @@ -43,7 +39,6 @@ import org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufRequestUtilities; import org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufUtilities; import org.apache.geode.security.SimpleTestSecurityManager; -import org.apache.geode.test.dunit.DistributedTestUtils; import org.apache.geode.test.dunit.Host; import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase; import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties; @@ -83,8 +78,8 @@ private Socket createSocket() throws IOException { } /** - * Test that if the locator has a security manager, an authorized client is allowed to get the - * available servers + * Test that if the locator has a security manager, an authorized client is allowed to get an + * available server */ @Test public void authorizedClientCanGetServersIfSecurityIsEnabled() throws Throwable { @@ -97,9 +92,9 @@ public void authorizedClientCanGetServersIfSecurityIsEnabled() throws Throwable .putCredentials("security-username", "cluster").putCredentials("security-password", "cluster")) .build()); - ClientProtocol.Message getAvailableServersRequestMessage = ProtobufUtilities - .createProtobufMessage(protobufRequestBuilder.setGetAvailableServersRequest( - ProtobufRequestUtilities.createGetAvailableServersRequest()).build()); + ClientProtocol.Message GetServerRequestMessage = + ProtobufUtilities.createProtobufMessage(protobufRequestBuilder + .setGetServerRequest(ProtobufRequestUtilities.createGetServerRequest()).build()); ProtobufProtocolSerializer protobufProtocolSerializer = new ProtobufProtocolSerializer(); @@ -110,14 +105,12 @@ public void authorizedClientCanGetServersIfSecurityIsEnabled() throws Throwable protobufProtocolSerializer.deserialize(socket.getInputStream()); assertEquals(true, authorizationResponse.getResponse().getAuthenticationResponse().getAuthenticated()); - protobufProtocolSerializer.serialize(getAvailableServersRequestMessage, - socket.getOutputStream()); + protobufProtocolSerializer.serialize(GetServerRequestMessage, socket.getOutputStream()); - ClientProtocol.Message getAvailableServersResponseMessage = + ClientProtocol.Message GetServerResponseMessage = protobufProtocolSerializer.deserialize(socket.getInputStream()); - assertEquals("Got response: " + getAvailableServersResponseMessage, 1, - getAvailableServersResponseMessage.getResponse().getGetAvailableServersResponse() - .getServersCount()); + assertTrue("Got response: " + GetServerResponseMessage, + GetServerResponseMessage.getResponse().getGetServerResponse().hasServer()); } } @@ -129,20 +122,19 @@ public void authorizedClientCanGetServersIfSecurityIsEnabled() throws Throwable public void unauthorizedClientCannotGetServersIfSecurityIsEnabled() throws Throwable { ClientProtocol.Request.Builder protobufRequestBuilder = ProtobufUtilities.createProtobufRequestBuilder(); - ClientProtocol.Message getAvailableServersRequestMessage = ProtobufUtilities - .createProtobufMessage(protobufRequestBuilder.setGetAvailableServersRequest( - ProtobufRequestUtilities.createGetAvailableServersRequest()).build()); + ClientProtocol.Message getServerRequestMessage = + ProtobufUtilities.createProtobufMessage(protobufRequestBuilder + .setGetServerRequest(ProtobufRequestUtilities.createGetServerRequest()).build()); ProtobufProtocolSerializer protobufProtocolSerializer = new ProtobufProtocolSerializer(); try (Socket socket = createSocket()) { - protobufProtocolSerializer.serialize(getAvailableServersRequestMessage, - socket.getOutputStream()); + protobufProtocolSerializer.serialize(getServerRequestMessage, socket.getOutputStream()); - ClientProtocol.Message getAvailableServersResponseMessage = + ClientProtocol.Message getServerResponseMessage = protobufProtocolSerializer.deserialize(socket.getInputStream()); - assertNotNull("Got response: " + getAvailableServersResponseMessage, - getAvailableServersRequestMessage.getResponse().getErrorResponse()); + assertNotNull("Got response: " + getServerResponseMessage, + getServerRequestMessage.getResponse().getErrorResponse()); } } diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java index 451aca3d40c9..1b3af527ef3e 100644 --- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java +++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java @@ -15,6 +15,7 @@ package org.apache.geode.internal.protocol.protobuf.v1.acceptance; +import static org.apache.geode.internal.Assert.assertTrue; import static org.apache.geode.internal.cache.tier.CommunicationMode.ProtobufClientServerProtocol; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -84,9 +85,9 @@ private Socket createSocket() throws IOException { public void testGetAvailableServersWithStats() throws Throwable { ClientProtocol.Request.Builder protobufRequestBuilder = ProtobufUtilities.createProtobufRequestBuilder(); - ClientProtocol.Message getAvailableServersRequestMessage = ProtobufUtilities - .createProtobufMessage(protobufRequestBuilder.setGetAvailableServersRequest( - ProtobufRequestUtilities.createGetAvailableServersRequest()).build()); + ClientProtocol.Message getAvailableServersRequestMessage = + ProtobufUtilities.createProtobufMessage(protobufRequestBuilder + .setGetServerRequest(ProtobufRequestUtilities.createGetServerRequest()).build()); ProtobufProtocolSerializer protobufProtocolSerializer = new ProtobufProtocolSerializer(); @@ -220,11 +221,10 @@ private void validateGetAvailableServersResponse( assertEquals(ClientProtocol.Message.MessageTypeCase.RESPONSE, getAvailableServersResponseMessage.getMessageTypeCase()); ClientProtocol.Response messageResponse = getAvailableServersResponseMessage.getResponse(); - assertEquals(ClientProtocol.Response.ResponseAPICase.GETAVAILABLESERVERSRESPONSE, + assertEquals(ClientProtocol.Response.ResponseAPICase.GETSERVERRESPONSE, messageResponse.getResponseAPICase()); - LocatorAPI.GetAvailableServersResponse getAvailableServersResponse = - messageResponse.getGetAvailableServersResponse(); - assertEquals(1, getAvailableServersResponse.getServersCount()); + LocatorAPI.GetServerResponse getServerResponse = messageResponse.getGetServerResponse(); + assertTrue(getServerResponse.hasServer()); } private void validateStats(long messagesReceived, long messagesSent, long bytesReceived, diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandlerJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandlerJUnitTest.java similarity index 55% rename from geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandlerJUnitTest.java rename to geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandlerJUnitTest.java index 1babc8b47343..bebd58944062 100644 --- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandlerJUnitTest.java +++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandlerJUnitTest.java @@ -15,6 +15,7 @@ package org.apache.geode.internal.protocol.protobuf.v1.operations; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -26,6 +27,8 @@ import org.junit.Test; import org.junit.experimental.categories.Category; +import org.apache.geode.cache.client.internal.locator.ClientConnectionRequest; +import org.apache.geode.cache.client.internal.locator.ClientConnectionResponse; import org.apache.geode.distributed.internal.InternalLocator; import org.apache.geode.distributed.internal.LocatorLoadSnapshot; import org.apache.geode.distributed.internal.ServerLocation; @@ -35,12 +38,12 @@ import org.apache.geode.internal.protocol.TestExecutionContext; import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes; import org.apache.geode.internal.protocol.protobuf.v1.LocatorAPI; -import org.apache.geode.internal.protocol.protobuf.v1.LocatorAPI.GetAvailableServersResponse; +import org.apache.geode.internal.protocol.protobuf.v1.LocatorAPI.GetServerResponse; import org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufRequestUtilities; import org.apache.geode.test.junit.categories.UnitTest; @Category(UnitTest.class) -public class GetAvailableServersOperationHandlerJUnitTest extends OperationHandlerJUnitTest { +public class GetServerOperationHandlerJUnitTest extends OperationHandlerJUnitTest { private final String HOSTNAME_1 = "hostname1"; private final int PORT_1 = 12345; @@ -49,63 +52,53 @@ public class GetAvailableServersOperationHandlerJUnitTest extends OperationHandl private final int PORT_2 = 23456; private InternalLocator internalLocatorMock; - private LocatorLoadSnapshot locatorLoadSnapshot; + ServerLocator serverLocatorAdviseeMock; @Before public void setUp() throws Exception { super.setUp(); - operationHandler = new GetAvailableServersOperationHandler(); + operationHandler = new GetServerOperationHandler(); internalLocatorMock = mock(InternalLocator.class); - ServerLocator serverLocatorAdviseeMock = mock(ServerLocator.class); - locatorLoadSnapshot = mock(LocatorLoadSnapshot.class); + serverLocatorAdviseeMock = mock(ServerLocator.class); when(internalLocatorMock.getServerLocatorAdvisee()).thenReturn(serverLocatorAdviseeMock); - when(serverLocatorAdviseeMock.getLoadSnapshot()).thenReturn(locatorLoadSnapshot); } @Test public void testServerReturnedFromHandler() throws Exception { - ArrayList serverList = new ArrayList<>(); - serverList.add(new ServerLocation(HOSTNAME_1, PORT_1)); - serverList.add(new ServerLocation(HOSTNAME_2, PORT_2)); - when(locatorLoadSnapshot.getServers(null)).thenReturn(serverList); - - LocatorAPI.GetAvailableServersRequest getAvailableServersRequest = - ProtobufRequestUtilities.createGetAvailableServersRequest(); - Result operationHandlerResult = getOperationHandlerResult(getAvailableServersRequest); + when(serverLocatorAdviseeMock.processRequest(any(Object.class))) + .thenReturn(new ClientConnectionResponse(new ServerLocation(HOSTNAME_1, PORT_1))); + + LocatorAPI.GetServerRequest GetServerRequest = + ProtobufRequestUtilities.createGetServerRequest(); + Result operationHandlerResult = getOperationHandlerResult(GetServerRequest); assertTrue(operationHandlerResult instanceof Success); - ValidateGetAvailableServersResponse( - (GetAvailableServersResponse) operationHandlerResult.getMessage()); + validateGetServerResponse((GetServerResponse) operationHandlerResult.getMessage()); } @Test - public void testWhenServersFromSnapshotAreNullReturnsEmtpy() throws Exception { - when(locatorLoadSnapshot.getServers(any())).thenReturn(null); + public void testWhenServersFromSnapshotAreNullDoesNotReturnAServer() throws Exception { + when(serverLocatorAdviseeMock.processRequest(any(Object.class))).thenReturn(null); - LocatorAPI.GetAvailableServersRequest getAvailableServersRequest = - ProtobufRequestUtilities.createGetAvailableServersRequest(); - Result operationHandlerResult = getOperationHandlerResult(getAvailableServersRequest); + LocatorAPI.GetServerRequest GetServerRequest = + ProtobufRequestUtilities.createGetServerRequest(); + Result operationHandlerResult = getOperationHandlerResult(GetServerRequest); assertTrue(operationHandlerResult instanceof Success); - GetAvailableServersResponse availableServersResponse = - (GetAvailableServersResponse) operationHandlerResult.getMessage(); - assertEquals(0, availableServersResponse.getServersCount()); + GetServerResponse serverResponse = (GetServerResponse) operationHandlerResult.getMessage(); + assertFalse(serverResponse.hasServer()); } - private Result getOperationHandlerResult( - LocatorAPI.GetAvailableServersRequest getAvailableServersRequest) throws Exception { - return operationHandler.process(serializationService, getAvailableServersRequest, + private Result getOperationHandlerResult(LocatorAPI.GetServerRequest GetServerRequest) + throws Exception { + return operationHandler.process(serializationService, GetServerRequest, TestExecutionContext.getLocatorExecutionContext(internalLocatorMock)); } - private void ValidateGetAvailableServersResponse( - GetAvailableServersResponse getAvailableServersResponse) { - assertEquals(2, getAvailableServersResponse.getServersCount()); - BasicTypes.Server server = getAvailableServersResponse.getServers(0); + private void validateGetServerResponse(GetServerResponse GetServerResponse) { + assertTrue(GetServerResponse.hasServer()); + BasicTypes.Server server = GetServerResponse.getServer(); assertEquals(HOSTNAME_1, server.getHostname()); assertEquals(PORT_1, server.getPort()); - server = getAvailableServersResponse.getServers(1); - assertEquals(HOSTNAME_2, server.getHostname()); - assertEquals(PORT_2, server.getPort()); } }