This is an automated email from the ASF dual-hosted git repository. pivotalsarge pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push: new 5380ed5 GEODE-3465: Enhance error message and add tests for when group is specified. (#1527) 5380ed5 is described below commit 5380ed5998ebe7ef6dea7c4a6c15d8f878f951b9 Author: Michael "Sarge" Dodge <mdo...@pivotal.io> AuthorDate: Fri Mar 2 11:43:57 2018 -0800 GEODE-3465: Enhance error message and add tests for when group is specified. (#1527) --- .../v1/operations/GetServerOperationHandler.java | 11 +++- .../v1/utilities/ProtobufRequestUtilities.java | 17 ++++++ .../GetServerOperationHandlerJUnitTest.java | 71 +++++++++++++++------- 3 files changed, 74 insertions(+), 25 deletions(-) diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandler.java index 4d22b78..528bc2e 100644 --- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandler.java +++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandler.java @@ -19,6 +19,7 @@ import static org.apache.geode.internal.protocol.protobuf.v1.BasicTypes.ErrorCod import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import org.apache.geode.annotations.Experimental; @@ -69,13 +70,17 @@ public class GetServerOperationHandler .getServerLocatorAdvisee().processRequest(clientConnectionRequest); ServerLocation serverLocation = null; - if (connectionResponse != null) { + if (connectionResponse != null && connectionResponse.hasResult()) { serverLocation = connectionResponse.getServer(); } if (serverLocation == null) { - return Failure.of(NO_AVAILABLE_SERVER, "Unable to find a server for you"); - + StringBuilder builder = new StringBuilder("Unable to find a server"); + if (!Objects.isNull(serverGroup) && !serverGroup.isEmpty()) { + builder.append(" in server group "); + builder.append(serverGroup); + } + return Failure.of(NO_AVAILABLE_SERVER, builder.toString()); } else { LocatorAPI.GetServerResponse.Builder builder = LocatorAPI.GetServerResponse.newBuilder(); BasicTypes.Server.Builder serverBuilder = BasicTypes.Server.newBuilder(); 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 9ba4e5b..c4a4833 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 @@ -109,8 +109,25 @@ public abstract class ProtobufRequestUtilities { return ClientProtocol.Message.newBuilder().setPutAllRequest(putAllRequestBuilder).build(); } + /** + * Create a request to retrieve a server. + * + * @return Request object containing the get-server request. + */ public static LocatorAPI.GetServerRequest createGetServerRequest() { LocatorAPI.GetServerRequest.Builder builder = LocatorAPI.GetServerRequest.newBuilder(); return builder.build(); } + + /** + * Create a request to retrieve a server from a server group. + * + * @param serverGroup Server group name. + * @return Request object containing the get-server request. + */ + public static LocatorAPI.GetServerRequest createGetServerRequest(String serverGroup) { + LocatorAPI.GetServerRequest.Builder builder = LocatorAPI.GetServerRequest.newBuilder(); + builder.setServerGroup(serverGroup); + return builder.build(); + } } diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandlerJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandlerJUnitTest.java index 415c7fb..5985c55 100644 --- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandlerJUnitTest.java +++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetServerOperationHandlerJUnitTest.java @@ -16,6 +16,7 @@ package org.apache.geode.internal.protocol.protobuf.v1.operations; import static org.apache.geode.internal.protocol.protobuf.v1.BasicTypes.ErrorCode.NO_AVAILABLE_SERVER; 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; @@ -25,6 +26,7 @@ import org.junit.Before; 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.ServerLocation; @@ -42,18 +44,15 @@ import org.apache.geode.test.junit.categories.UnitTest; @Category(UnitTest.class) public class GetServerOperationHandlerJUnitTest extends OperationHandlerJUnitTest { - - private final String HOSTNAME_1 = "hostname1"; - private final int PORT_1 = 12345; - - private final String HOSTNAME_2 = "hostname2"; - private final int PORT_2 = 23456; - - private InternalLocator internalLocatorMock; + final String HOSTNAME = "hostname"; + final int PORT = 12345; + final String EXISTENT_GROUP = "existent"; + final String NONEXISTENT_GROUP = "nonexistent"; + InternalLocator internalLocatorMock; ServerLocator serverLocatorAdviseeMock; @Before - public void setUp() throws Exception { + public void setUp() { operationHandler = new GetServerOperationHandler(); internalLocatorMock = mock(InternalLocator.class); serverLocatorAdviseeMock = mock(ServerLocator.class); @@ -64,27 +63,55 @@ public class GetServerOperationHandlerJUnitTest extends OperationHandlerJUnitTes @Test public void testServerReturnedFromHandler() throws Exception { when(serverLocatorAdviseeMock.processRequest(any(Object.class))) - .thenReturn(new ClientConnectionResponse(new ServerLocation(HOSTNAME_1, PORT_1))); + .thenReturn(new ClientConnectionResponse(new ServerLocation(HOSTNAME, PORT))); - LocatorAPI.GetServerRequest GetServerRequest = + LocatorAPI.GetServerRequest getServerRequest = ProtobufRequestUtilities.createGetServerRequest(); - Result operationHandlerResult = getOperationHandlerResult(GetServerRequest); + Result operationHandlerResult = getOperationHandlerResult(getServerRequest); assertTrue(operationHandlerResult instanceof Success); validateGetServerResponse((GetServerResponse) operationHandlerResult.getMessage()); } @Test - public void testExceptionReturnedWhenNoServers() throws Exception { + public void testErrorReturnedWhenNoServers() throws Exception { when(serverLocatorAdviseeMock.processRequest(any(Object.class))).thenReturn(null); - LocatorAPI.GetServerRequest GetServerRequest = + LocatorAPI.GetServerRequest getServerRequest = ProtobufRequestUtilities.createGetServerRequest(); - Result operationHandlerResult = getOperationHandlerResult(GetServerRequest); + Result operationHandlerResult = getOperationHandlerResult(getServerRequest); + assertTrue(operationHandlerResult instanceof Failure); + Failure failure = (Failure) operationHandlerResult; + ClientProtocol.ErrorResponse errorResponse = failure.getErrorMessage(); + assertEquals(NO_AVAILABLE_SERVER, errorResponse.getError().getErrorCode()); + } + + @Test + public void testServerReturnedForExistentGroup() throws Exception { + when( + serverLocatorAdviseeMock.processRequest(new ClientConnectionRequest(any(), EXISTENT_GROUP))) + .thenReturn(new ClientConnectionResponse(new ServerLocation(HOSTNAME, PORT))); + + LocatorAPI.GetServerRequest getServerRequest = + ProtobufRequestUtilities.createGetServerRequest(EXISTENT_GROUP); + Result operationHandlerResult = getOperationHandlerResult(getServerRequest); + assertTrue(operationHandlerResult instanceof Success); + validateGetServerResponse((GetServerResponse) operationHandlerResult.getMessage()); + } + + @Test + public void testErrorReturnedForNonexistentGroup() throws Exception { + when(serverLocatorAdviseeMock + .processRequest(new ClientConnectionRequest(any(), NONEXISTENT_GROUP))) + .thenReturn(new ClientConnectionResponse(null)); + + LocatorAPI.GetServerRequest getServerRequest = + ProtobufRequestUtilities.createGetServerRequest(NONEXISTENT_GROUP); + Result operationHandlerResult = getOperationHandlerResult(getServerRequest); assertTrue(operationHandlerResult instanceof Failure); Failure failure = (Failure) operationHandlerResult; - ClientProtocol.ErrorResponse errorResponse = - (ClientProtocol.ErrorResponse) failure.getErrorMessage(); + ClientProtocol.ErrorResponse errorResponse = failure.getErrorMessage(); assertEquals(NO_AVAILABLE_SERVER, errorResponse.getError().getErrorCode()); + assertTrue(errorResponse.getError().getMessage().contains(NONEXISTENT_GROUP)); } private Result getOperationHandlerResult(LocatorAPI.GetServerRequest GetServerRequest) @@ -93,10 +120,10 @@ public class GetServerOperationHandlerJUnitTest extends OperationHandlerJUnitTes TestExecutionContext.getLocatorExecutionContext(internalLocatorMock)); } - private void validateGetServerResponse(GetServerResponse GetServerResponse) { - assertTrue(GetServerResponse.hasServer()); - BasicTypes.Server server = GetServerResponse.getServer(); - assertEquals(HOSTNAME_1, server.getHostname()); - assertEquals(PORT_1, server.getPort()); + private void validateGetServerResponse(GetServerResponse getServerResponse) { + assertTrue(getServerResponse.hasServer()); + BasicTypes.Server server = getServerResponse.getServer(); + assertEquals(HOSTNAME, server.getHostname()); + assertEquals(PORT, server.getPort()); } } -- To stop receiving notification emails like this one, please contact pivotalsa...@apache.org.