GEODE-6674: only create StringBuilder if needed (apache#3511)

* added unit tests for Put65 when region or key are null * no longer allocates a StringBuilder for every client put message
b-xiang · Apr 30, 2019 · 52abb90 · 52abb90
1 parent 59c76ee
commit 52abb90
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 3 deletions.
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Put65.java
@@ -69,7 +69,6 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve
       final SecurityService securityService, long p_start)
       throws IOException, InterruptedException {
     long start = p_start;
-    final StringBuilder errMessage = new StringBuilder();
     final CacheServerStats stats = serverConnection.getCacheServerStats();
 
     // requiresResponse = true;
@@ -159,6 +158,7 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve
 
     // Process the put request
     if (key == null || regionName == null) {
+      final StringBuilder errMessage = new StringBuilder();
       if (key == null) {
         final String putMsg = " The input key for the put request is null";
         if (isDebugEnabled) {
@@ -193,8 +193,7 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve
       if (isDebugEnabled) {
         logger.debug("{}:{}", serverConnection.getName(), putMsg);
       }
-      errMessage.append(putMsg);
-      writeErrorResponse(clientMessage, MessageType.PUT_DATA_ERROR, errMessage.toString(),
+      writeErrorResponse(clientMessage, MessageType.PUT_DATA_ERROR, putMsg,
           serverConnection);
       serverConnection.setAsTrue(RESPONDED);
       return;

diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put65Test.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put65Test.java
@@ -168,6 +168,32 @@ public void noSecurityShouldSucceed() throws Exception {
     verify(this.replyMessage).send(this.serverConnection);
   }
 
+  @Test
+  public void noRegionNameShouldFail() throws Exception {
+    when(this.securityService.isClientSecurityRequired()).thenReturn(false);
+    when(this.regionNamePart.getString()).thenReturn(null);
+
+    this.put65.cmdExecute(this.message, this.serverConnection, this.securityService, 0);
+
+    ArgumentCaptor<String> argument = ArgumentCaptor.forClass(String.class);
+    verify(this.errorResponseMessage).addStringPart(argument.capture());
+    assertThat(argument.getValue()).contains("The input region name for the put request is null");
+    verify(this.errorResponseMessage).send(this.serverConnection);
+  }
+
+  @Test
+  public void noKeyShouldFail() throws Exception {
+    when(this.securityService.isClientSecurityRequired()).thenReturn(false);
+    when(this.keyPart.getStringOrObject()).thenReturn(null);
+
+    this.put65.cmdExecute(this.message, this.serverConnection, this.securityService, 0);
+
+    ArgumentCaptor<String> argument = ArgumentCaptor.forClass(String.class);
+    verify(this.errorResponseMessage).addStringPart(argument.capture());
+    assertThat(argument.getValue()).contains("The input key for the put request is null");
+    verify(this.errorResponseMessage).send(this.serverConnection);
+  }
+
   @Test
   public void integratedSecurityShouldSucceedIfAuthorized() throws Exception {
     when(this.securityService.isClientSecurityRequired()).thenReturn(true);