You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by pi...@apache.org on 2018/03/02 19:44:00 UTC

[geode] branch develop updated: GEODE-3465: Enhance error message and add tests for when group is specified. (#1527)

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 <md...@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
pivotalsarge@apache.org.