geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gosulli...@apache.org
Subject [geode] branch develop updated: GEODE-2999, GEODE-4836: Add PutIfAbsent to the Protobuf protocol.
Date Wed, 21 Mar 2018 17:47:11 GMT
This is an automated email from the ASF dual-hosted git repository.

gosullivan 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 c689dfd  GEODE-2999, GEODE-4836: Add PutIfAbsent to the Protobuf protocol.
c689dfd is described below

commit c689dfd452e6462ca23969c6d50fbff51100e9f3
Author: Galen O'Sullivan <gosullivan@pivotal.io>
AuthorDate: Wed Mar 14 17:03:10 2018 -0700

    GEODE-2999, GEODE-4836: Add PutIfAbsent to the Protobuf protocol.
    
    fix issues with getRequest not properly returning null.
    
    This reverts commit 7a3da494e3b506e8cd485999db7a3cd42e200f85, which
    originally reverted 6214a43be9d31fd0be1d133a8f0ad7379ea3f9c2.
    
    This closes #1617
---
 .../geode/experimental/driver/ProtobufRegion.java  |  14 ++
 .../apache/geode/experimental/driver/Region.java   |  11 ++
 .../geode/experimental/driver/ValueEncoder.java    |   9 +-
 .../experimental/driver/RegionIntegrationTest.java |  23 +++
 .../experimental/driver/ValueEncoderTest.java      |   2 +-
 .../src/main/proto/v1/basicTypes.proto             |   1 +
 .../src/main/proto/v1/clientProtocol.proto         |   3 +
 .../src/main/proto/v1/region_API.proto             |   9 +
 .../protocol/protobuf/v1/ProtobufOpsProcessor.java |   6 +-
 .../protobuf/v1/ProtobufStreamProcessor.java       |   1 -
 .../v1/operations/GetRequestOperationHandler.java  |   4 -
 ...ava => PutIfAbsentRequestOperationHandler.java} |  60 +++----
 .../registry/ProtobufOperationContextRegistry.java |  38 ++--
 .../protobuf/v1/utilities/ProtobufUtilities.java   |   7 +-
 .../internal/protocol/protobuf/v1/MessageUtil.java |  17 ++
 .../protobuf/v1/ProtobufRequestUtilities.java      |  19 +-
 .../v1/acceptance/CacheConnectionJUnitTest.java    |  15 +-
 .../v1/acceptance/CacheOperationsJUnitTest.java    |  22 +--
 .../GetAllRequestOperationHandlerJUnitTest.java    |   4 +-
 .../GetAndPutJsonDocumentsDUnitTest.java           |   4 +-
 .../GetRequestOperationHandlerJUnitTest.java       |   5 +-
 .../PutIfAbsentRequestIntegrationTest.java         | 168 ++++++++++++++++++
 ...utIfAbsentRequestOperationHandlerJUnitTest.java | 197 +++++++++++++++++++++
 23 files changed, 538 insertions(+), 101 deletions(-)

diff --git a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufRegion.java b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufRegion.java
index 2870687..a7fc501 100644
--- a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufRegion.java
+++ b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufRegion.java
@@ -148,6 +148,20 @@ public class ProtobufRegion<K, V> implements Region<K, V> {
   }
 
   @Override
+  public V putIfAbsent(K key, V value) throws IOException {
+    final RegionAPI.PutIfAbsentRequest.Builder putIfAbsentRequest = RegionAPI.PutIfAbsentRequest
+        .newBuilder().setRegionName(name).setEntry(ValueEncoder.encodeEntry(key, value));
+
+    final Message request = Message.newBuilder().setPutIfAbsentRequest(putIfAbsentRequest).build();
+
+    final RegionAPI.PutIfAbsentResponse putIfAbsentResponse = protobufChannel
+        .sendRequest(request, MessageTypeCase.PUTIFABSENTRESPONSE).getPutIfAbsentResponse();
+
+
+    return (V) ValueEncoder.decodeValue(putIfAbsentResponse.getOldValue());
+  }
+
+  @Override
   public void remove(K key) throws IOException {
     final Message request = Message.newBuilder()
         .setRemoveRequest(
diff --git a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/Region.java b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/Region.java
index bbdbe2d..32cb381 100644
--- a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/Region.java
+++ b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/Region.java
@@ -86,6 +86,17 @@ public interface Region<K, V> {
   void clear() throws IOException;
 
   /**
+   * Puts the <code>value</code> into this region for the <code>key</code> if <code>key</code> does
+   * not already have a value associated with it.
+   *
+   * @return null if the value was set; the current value otherwise.
+   *         NOTE that if the value in the region was set to null, this method will return null
+   *         without setting a new value.
+   * @throws IOException
+   */
+  V putIfAbsent(K key, V value) throws IOException;
+
+  /**
    * Removes any value associated with the <code>key</code> from this region.
    *
    * @param key Unique key associated with a value.
diff --git a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ValueEncoder.java b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ValueEncoder.java
index 9c9b656..3d66bfe 100644
--- a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ValueEncoder.java
+++ b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ValueEncoder.java
@@ -14,7 +14,10 @@
  */
 package org.apache.geode.experimental.driver;
 
+import java.util.Objects;
+
 import com.google.protobuf.ByteString;
+import com.google.protobuf.NullValue;
 
 import org.apache.geode.annotations.Experimental;
 import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
@@ -35,7 +38,9 @@ class ValueEncoder {
    */
   static BasicTypes.EncodedValue encodeValue(Object unencodedValue) {
     BasicTypes.EncodedValue.Builder builder = BasicTypes.EncodedValue.newBuilder();
-    if (Integer.class.equals(unencodedValue.getClass())) {
+    if (Objects.isNull(unencodedValue)) {
+      builder.setNullResult(NullValue.NULL_VALUE);
+    } else if (Integer.class.equals(unencodedValue.getClass())) {
       builder.setIntResult((Integer) unencodedValue);
     } else if (Long.class.equals(unencodedValue.getClass())) {
       builder.setLongResult((Long) unencodedValue);
@@ -91,7 +96,7 @@ class ValueEncoder {
         return encodedValue.getStringResult();
       case JSONOBJECTRESULT:
         return JSONWrapper.wrapJSON(encodedValue.getJsonObjectResult());
-      case VALUE_NOT_SET:
+      case NULLRESULT:
         return null;
       default:
         throw new IllegalStateException(
diff --git a/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/RegionIntegrationTest.java b/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/RegionIntegrationTest.java
index 78af30c..6782ec2 100644
--- a/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/RegionIntegrationTest.java
+++ b/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/RegionIntegrationTest.java
@@ -95,6 +95,29 @@ public class RegionIntegrationTest extends IntegrationTestBase {
   }
 
   @Test
+  public void putIfAbsent() throws Exception {
+    Region<JSONWrapper, JSONWrapper> region = driver.getRegion("region");
+    JSONWrapper document = JSONWrapper.wrapJSON(jsonDocument);
+
+    assertNull(region.putIfAbsent(document, document));
+
+    JSONWrapper value = region.get(document);
+    assertEquals(document, value);
+    assertEquals(1, serverRegion.size());
+
+    assertEquals(document, region.putIfAbsent(document, JSONWrapper.wrapJSON("{3 : 2}")));
+    value = region.get(document);
+    assertEquals(document, value);
+    assertEquals(1, serverRegion.size());
+
+    org.apache.geode.cache.Region.Entry entry =
+        (org.apache.geode.cache.Region.Entry) serverRegion.entrySet().iterator().next();
+
+    assertTrue(PdxInstance.class.isAssignableFrom(entry.getKey().getClass()));
+    assertTrue(PdxInstance.class.isAssignableFrom(entry.getValue().getClass()));
+  }
+
+  @Test
   public void removeWithJSONKey() throws Exception {
     Region<JSONWrapper, JSONWrapper> region = driver.getRegion("region");
     JSONWrapper document = JSONWrapper.wrapJSON(jsonDocument);
diff --git a/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/ValueEncoderTest.java b/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/ValueEncoderTest.java
index c8e7353..3a2989d 100644
--- a/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/ValueEncoderTest.java
+++ b/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/ValueEncoderTest.java
@@ -33,7 +33,7 @@ public class ValueEncoderTest {
 
   @Test
   public void encodeAndDecode() throws Exception {
-    final Object[] objects = {37, (short) 37, (byte) 37, 37L, 37., 37.F, true, "hello, world",
+    final Object[] objects = {37, (short) 37, (byte) 37, 37L, 37., 37.F, true, "hello, world", null,
         JSONWrapper.wrapJSON(jsonDocument)};
     for (Object object : objects) {
       assertEquals(object, ValueEncoder.decodeValue(ValueEncoder.encodeValue(object)));
diff --git a/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto b/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
index a12718b..e8cfa15 100644
--- a/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
+++ b/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
@@ -82,6 +82,7 @@ enum ErrorCode {
     AUTHENTICATION_NOT_SUPPORTED = 13;
     AUTHORIZATION_FAILED = 20;
     INVALID_REQUEST = 50;
+    UNSUPPORTED_OPERATION = 60;
     SERVER_ERROR = 100;
     NO_AVAILABLE_SERVER = 101;
 }
diff --git a/geode-protobuf-messages/src/main/proto/v1/clientProtocol.proto b/geode-protobuf-messages/src/main/proto/v1/clientProtocol.proto
index 2b21c89..c3b0f5f 100644
--- a/geode-protobuf-messages/src/main/proto/v1/clientProtocol.proto
+++ b/geode-protobuf-messages/src/main/proto/v1/clientProtocol.proto
@@ -78,6 +78,9 @@ message Message {
 
         ClearRequest clearRequest = 32;
         ClearResponse clearResponse = 33;
+
+        PutIfAbsentRequest putIfAbsentRequest = 34;
+        PutIfAbsentResponse putIfAbsentResponse = 35;
     }
 }
 
diff --git a/geode-protobuf-messages/src/main/proto/v1/region_API.proto b/geode-protobuf-messages/src/main/proto/v1/region_API.proto
index 407409f..b70765b 100644
--- a/geode-protobuf-messages/src/main/proto/v1/region_API.proto
+++ b/geode-protobuf-messages/src/main/proto/v1/region_API.proto
@@ -32,6 +32,15 @@ message PutResponse {
     // message presence indicates success.
 }
 
+message PutIfAbsentRequest {
+    string regionName = 1;
+    Entry entry = 2;
+}
+
+message PutIfAbsentResponse {
+    EncodedValue oldValue = 1;
+}
+
 message GetRequest {
     string regionName = 1;
     EncodedValue key = 2;
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufOpsProcessor.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufOpsProcessor.java
index 1f2d201..cfc71b3 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufOpsProcessor.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufOpsProcessor.java
@@ -25,7 +25,6 @@ import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.De
 import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.EncodingException;
 import org.apache.geode.internal.protocol.protobuf.v1.state.ProtobufConnectionTerminatingStateProcessor;
 import org.apache.geode.internal.protocol.protobuf.v1.state.exception.ConnectionStateException;
-import org.apache.geode.internal.protocol.protobuf.v1.state.exception.OperationNotAuthorizedException;
 
 /**
  * This handles protobuf requests by determining the operation type of the request and dispatching
@@ -86,6 +85,11 @@ public class ProtobufOpsProcessor {
       logger.error(exception);
       return Failure.of(BasicTypes.ErrorCode.INVALID_REQUEST,
           "Invalid execution context found for operation.");
+    } catch (UnsupportedOperationException exception) {
+      logger.error("Unsupported operation exception for request {}", requestType);
+      logger.error(exception);
+      return Failure.of(BasicTypes.ErrorCode.UNSUPPORTED_OPERATION,
+          "Unsupported operation:" + exception.getMessage());
     } finally {
       context.getStatistics().endOperation(startTime);
     }
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufStreamProcessor.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufStreamProcessor.java
index e72774f..1558ee0 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufStreamProcessor.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufStreamProcessor.java
@@ -27,7 +27,6 @@ import org.apache.geode.internal.protocol.protobuf.statistics.ClientStatistics;
 import org.apache.geode.internal.protocol.protobuf.v1.registry.ProtobufOperationContextRegistry;
 import org.apache.geode.internal.protocol.protobuf.v1.serializer.ProtobufProtocolSerializer;
 import org.apache.geode.internal.protocol.protobuf.v1.serializer.exception.InvalidProtocolMessageException;
-import org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufUtilities;
 
 /**
  * This object handles an incoming stream containing protobuf messages. It parses the protobuf
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandler.java
index 856bb13..089274b 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandler.java
@@ -58,10 +58,6 @@ public class GetRequestOperationHandler
       }
       Object resultValue = region.get(decodedKey);
 
-      if (resultValue == null) {
-        return Success.of(RegionAPI.GetResponse.newBuilder().build());
-      }
-
       BasicTypes.EncodedValue encodedValue = serializationService.encode(resultValue);
       return Success.of(RegionAPI.GetResponse.newBuilder().setResult(encodedValue).build());
     } finally {
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutIfAbsentRequestOperationHandler.java
similarity index 56%
copy from geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandler.java
copy to geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutIfAbsentRequestOperationHandler.java
index 856bb13..9713183 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutIfAbsentRequestOperationHandler.java
@@ -14,12 +14,11 @@
  */
 package org.apache.geode.internal.protocol.protobuf.v1.operations;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-import org.apache.geode.annotations.Experimental;
 import org.apache.geode.cache.Region;
 import org.apache.geode.internal.exception.InvalidExecutionContextException;
-import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.protocol.operations.ProtobufOperationHandler;
 import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
 import org.apache.geode.internal.protocol.protobuf.v1.Failure;
@@ -30,49 +29,50 @@ import org.apache.geode.internal.protocol.protobuf.v1.Result;
 import org.apache.geode.internal.protocol.protobuf.v1.Success;
 import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.DecodingException;
 import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.EncodingException;
+import org.apache.geode.internal.protocol.protobuf.v1.state.exception.ConnectionStateException;
 import org.apache.geode.security.ResourcePermission;
 
-@Experimental
-public class GetRequestOperationHandler
-    implements ProtobufOperationHandler<RegionAPI.GetRequest, RegionAPI.GetResponse> {
-  private static final Logger logger = LogService.getLogger();
+public class PutIfAbsentRequestOperationHandler implements
+    ProtobufOperationHandler<RegionAPI.PutIfAbsentRequest, RegionAPI.PutIfAbsentResponse> {
+  private static final Logger logger = LogManager.getLogger();
 
   @Override
-  public Result<RegionAPI.GetResponse> process(ProtobufSerializationService serializationService,
-      RegionAPI.GetRequest request, MessageExecutionContext messageExecutionContext)
-      throws InvalidExecutionContextException, EncodingException, DecodingException {
-    String regionName = request.getRegionName();
-    Region region = messageExecutionContext.getCache().getRegion(regionName);
+  public Result<RegionAPI.PutIfAbsentResponse> process(
+      ProtobufSerializationService serializationService, RegionAPI.PutIfAbsentRequest request,
+      MessageExecutionContext messageExecutionContext) throws InvalidExecutionContextException,
+      ConnectionStateException, EncodingException, DecodingException {
+
+    final String regionName = request.getRegionName();
+
+    Region<Object, Object> region;
+    try {
+      region = messageExecutionContext.getCache().getRegion(regionName);
+    } catch (IllegalArgumentException ex) {
+      return Failure.of(BasicTypes.ErrorCode.INVALID_REQUEST,
+          "Invalid region name: \"" + regionName + "\"");
+    }
+
     if (region == null) {
-      logger.error("Received get request for nonexistent region: {}", regionName);
+      logger.error("Received PutIfAbsentRequest for nonexistent region: {}", regionName);
       return Failure.of(BasicTypes.ErrorCode.SERVER_ERROR,
           "Region \"" + regionName + "\" not found");
     }
 
-    try {
-      messageExecutionContext.getCache().setReadSerializedForCurrentThread(true);
+    final BasicTypes.Entry entry = request.getEntry();
 
-      Object decodedKey = serializationService.decode(request.getKey());
-      if (decodedKey == null) {
-        return Failure.of(BasicTypes.ErrorCode.INVALID_REQUEST, "Performing a get on a NULL key.");
-      }
-      Object resultValue = region.get(decodedKey);
+    Object decodedValue = serializationService.decode(entry.getValue());
+    Object decodedKey = serializationService.decode(entry.getKey());
 
-      if (resultValue == null) {
-        return Success.of(RegionAPI.GetResponse.newBuilder().build());
-      }
+    final Object oldValue = region.putIfAbsent(decodedKey, decodedValue);
 
-      BasicTypes.EncodedValue encodedValue = serializationService.encode(resultValue);
-      return Success.of(RegionAPI.GetResponse.newBuilder().setResult(encodedValue).build());
-    } finally {
-      messageExecutionContext.getCache().setReadSerializedForCurrentThread(false);
-    }
+    return Success.of(RegionAPI.PutIfAbsentResponse.newBuilder()
+        .setOldValue(serializationService.encode(oldValue)).build());
   }
 
-  public static ResourcePermission determineRequiredPermission(RegionAPI.GetRequest request,
+  public static ResourcePermission determineRequiredPermission(RegionAPI.PutIfAbsentRequest request,
       ProtobufSerializationService serializer) throws DecodingException {
     return new ResourcePermission(ResourcePermission.Resource.DATA,
-        ResourcePermission.Operation.READ, request.getRegionName(),
-        serializer.decode(request.getKey()).toString());
+        ResourcePermission.Operation.WRITE, request.getRegionName(),
+        serializer.decode(request.getEntry().getKey()).toString());
   }
 }
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 d950baa..64a5cba 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
@@ -36,6 +36,7 @@ import org.apache.geode.internal.protocol.protobuf.v1.operations.GetSizeRequestO
 import org.apache.geode.internal.protocol.protobuf.v1.operations.KeySetOperationHandler;
 import org.apache.geode.internal.protocol.protobuf.v1.operations.OqlQueryRequestOperationHandler;
 import org.apache.geode.internal.protocol.protobuf.v1.operations.PutAllRequestOperationHandler;
+import org.apache.geode.internal.protocol.protobuf.v1.operations.PutIfAbsentRequestOperationHandler;
 import org.apache.geode.internal.protocol.protobuf.v1.operations.PutRequestOperationHandler;
 import org.apache.geode.internal.protocol.protobuf.v1.operations.RemoveRequestOperationHandler;
 import org.apache.geode.internal.protocol.protobuf.v1.operations.security.AuthenticationRequestOperationHandler;
@@ -46,15 +47,14 @@ import org.apache.geode.security.ResourcePermission.Resource;
 
 @Experimental
 public class ProtobufOperationContextRegistry {
-  private Map<ClientProtocol.Message.MessageTypeCase, ProtobufOperationContext> operationContexts =
+  private final Map<MessageTypeCase, ProtobufOperationContext> operationContexts =
       new ConcurrentHashMap<>();
 
   public ProtobufOperationContextRegistry() {
     addContexts();
   }
 
-  public ProtobufOperationContext getOperationContext(
-      ClientProtocol.Message.MessageTypeCase apiCase) {
+  public ProtobufOperationContext getOperationContext(MessageTypeCase apiCase) {
     return operationContexts.get(apiCase);
   }
 
@@ -67,69 +67,69 @@ public class ProtobufOperationContextRegistry {
   }
 
   private void addContexts() {
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.AUTHENTICATIONREQUEST,
+    operationContexts.put(MessageTypeCase.AUTHENTICATIONREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getAuthenticationRequest,
             new AuthenticationRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setAuthenticationResponse(opsResp),
             this::skipAuthorizationCheck));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.DISCONNECTCLIENTREQUEST,
+    operationContexts.put(MessageTypeCase.DISCONNECTCLIENTREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getDisconnectClientRequest,
             new DisconnectClientRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setDisconnectClientResponse(opsResp),
             this::skipAuthorizationCheck));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.GETREQUEST,
+    operationContexts.put(MessageTypeCase.GETREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getGetRequest,
             new GetRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setGetResponse(opsResp),
             GetRequestOperationHandler::determineRequiredPermission));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.GETALLREQUEST,
+    operationContexts.put(MessageTypeCase.GETALLREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getGetAllRequest,
             new GetAllRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setGetAllResponse(opsResp),
             // May require per-key checks, will be handled by OperationHandler
             this::skipAuthorizationCheck));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.PUTREQUEST,
+    operationContexts.put(MessageTypeCase.PUTREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getPutRequest,
             new PutRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setPutResponse(opsResp),
             PutRequestOperationHandler::determineRequiredPermission));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.PUTALLREQUEST,
+    operationContexts.put(MessageTypeCase.PUTALLREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getPutAllRequest,
             new PutAllRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setPutAllResponse(opsResp),
             // May require per-key checks, will be handled by OperationHandler
             this::skipAuthorizationCheck));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.REMOVEREQUEST,
+    operationContexts.put(MessageTypeCase.REMOVEREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getRemoveRequest,
             new RemoveRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setRemoveResponse(opsResp),
             RemoveRequestOperationHandler::determineRequiredPermission));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.GETREGIONNAMESREQUEST,
+    operationContexts.put(MessageTypeCase.GETREGIONNAMESREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getGetRegionNamesRequest,
             new GetRegionNamesRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setGetRegionNamesResponse(opsResp),
             ResourcePermissions.DATA_READ));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.GETSIZEREQUEST,
+    operationContexts.put(MessageTypeCase.GETSIZEREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getGetSizeRequest,
             new GetSizeRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setGetSizeResponse(opsResp),
             ResourcePermissions.DATA_READ));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.GETSERVERREQUEST,
+    operationContexts.put(MessageTypeCase.GETSERVERREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getGetServerRequest,
             new GetServerOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setGetServerResponse(opsResp),
             ResourcePermissions.CLUSTER_READ));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.EXECUTEFUNCTIONONREGIONREQUEST,
+    operationContexts.put(MessageTypeCase.EXECUTEFUNCTIONONREGIONREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getExecuteFunctionOnRegionRequest,
             new ExecuteFunctionOnRegionRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder()
@@ -138,7 +138,7 @@ public class ProtobufOperationContextRegistry {
             // requirements.
             this::skipAuthorizationCheck));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.EXECUTEFUNCTIONONMEMBERREQUEST,
+    operationContexts.put(MessageTypeCase.EXECUTEFUNCTIONONMEMBERREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getExecuteFunctionOnMemberRequest,
             new ExecuteFunctionOnMemberRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder()
@@ -147,7 +147,7 @@ public class ProtobufOperationContextRegistry {
             // requirements.
             this::skipAuthorizationCheck));
 
-    operationContexts.put(ClientProtocol.Message.MessageTypeCase.EXECUTEFUNCTIONONGROUPREQUEST,
+    operationContexts.put(MessageTypeCase.EXECUTEFUNCTIONONGROUPREQUEST,
         new ProtobufOperationContext<>(ClientProtocol.Message::getExecuteFunctionOnGroupRequest,
             new ExecuteFunctionOnGroupRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder()
@@ -174,5 +174,11 @@ public class ProtobufOperationContextRegistry {
             new ClearRequestOperationHandler(),
             opsResp -> ClientProtocol.Message.newBuilder().setClearResponse(opsResp),
             ClearRequestOperationHandler::determineRequiredPermission));
+
+    operationContexts.put(MessageTypeCase.PUTIFABSENTREQUEST,
+        new ProtobufOperationContext<>(ClientProtocol.Message::getPutIfAbsentRequest,
+            new PutIfAbsentRequestOperationHandler(),
+            opsResp -> ClientProtocol.Message.newBuilder().setPutIfAbsentResponse(opsResp),
+            PutIfAbsentRequestOperationHandler::determineRequiredPermission));
   }
 }
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilities.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilities.java
index d7e2a2d..877aac0 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilities.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilities.java
@@ -25,6 +25,9 @@ import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.En
  * This class contains helper functions for assistance in creating protobuf objects. This class is
  * mainly focused on helper functions which can be used in building BasicTypes for use in other
  * messages or those used to create the top level Message objects.
+ * <p>
+ * Helper functions specific to creating ClientProtocol.Messages can be found at
+ * {@link ProtobufRequestUtilities}
  */
 @Experimental
 public abstract class ProtobufUtilities {
@@ -56,10 +59,6 @@ public abstract class ProtobufUtilities {
    */
   public static BasicTypes.Entry createEntry(ProtobufSerializationService serializationService,
       Object unencodedKey, Object unencodedValue) throws EncodingException {
-    if (unencodedValue == null) {
-      return BasicTypes.Entry.newBuilder().setKey(serializationService.encode(unencodedKey))
-          .build();
-    }
     return createEntry(serializationService.encode(unencodedKey),
         serializationService.encode(unencodedValue));
   }
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/MessageUtil.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/MessageUtil.java
index 7080202..0e83b93 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/MessageUtil.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/MessageUtil.java
@@ -14,6 +14,7 @@
  */
 package org.apache.geode.internal.protocol.protobuf.v1;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.ByteArrayInputStream;
@@ -24,7 +25,10 @@ import java.net.Socket;
 import com.google.protobuf.MessageLite;
 
 import org.apache.geode.internal.protocol.protobuf.ProtocolVersion;
+import org.apache.geode.internal.protocol.protobuf.v1.ProtobufRequestUtilities;
 import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.EncodingException;
+import org.apache.geode.internal.protocol.protobuf.v1.serializer.ProtobufProtocolSerializer;
+import org.apache.geode.internal.protocol.protobuf.v1.serializer.exception.InvalidProtocolMessageException;
 import org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufUtilities;
 
 public class MessageUtil {
@@ -89,4 +93,17 @@ public class MessageUtil {
       throw new RuntimeException(e); // never happens.
     }
   }
+
+  public static void validateGetResponse(Socket socket,
+      ProtobufProtocolSerializer protobufProtocolSerializer, Object expectedValue)
+      throws InvalidProtocolMessageException, IOException {
+
+    ClientProtocol.Message response =
+        protobufProtocolSerializer.deserialize(socket.getInputStream());
+    assertEquals(ClientProtocol.Message.MessageTypeCase.GETRESPONSE, response.getMessageTypeCase());
+    RegionAPI.GetResponse getResponse = response.getGetResponse();
+    BasicTypes.EncodedValue result = getResponse.getResult();
+    assertEquals(BasicTypes.EncodedValue.ValueCase.STRINGRESULT, result.getValueCase());
+    assertEquals(expectedValue, result.getStringResult());
+  }
 }
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufRequestUtilities.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufRequestUtilities.java
index 873fe02..2faa4bb 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufRequestUtilities.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufRequestUtilities.java
@@ -17,10 +17,7 @@ package org.apache.geode.internal.protocol.protobuf.v1;
 import java.util.Set;
 
 import org.apache.geode.annotations.Experimental;
-import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
-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.RegionAPI;
+import org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufUtilities;
 
 /**
  * This class contains helper functions for generating ClientProtocol.Message objects
@@ -92,6 +89,20 @@ public abstract class ProtobufRequestUtilities {
   }
 
   /**
+   * Creates a request object containing a RegionAPI.PutIfAbsentRequest
+   *
+   * @param region - Name of the region to put data in
+   * @param entry - Encoded key,value pair, see createEntry in {@link ProtobufRequestUtilities}
+   * @return Request object containing the passed params.
+   */
+  public static ClientProtocol.Message createPutIfAbsentRequest(String region,
+      BasicTypes.Entry entry) {
+    RegionAPI.PutIfAbsentRequest putIfAbsentRequest =
+        RegionAPI.PutIfAbsentRequest.newBuilder().setRegionName(region).setEntry(entry).build();
+    return ClientProtocol.Message.newBuilder().setPutIfAbsentRequest(putIfAbsentRequest).build();
+  }
+
+  /**
    * Create a request to get the values for multiple keys
    *
    * @param regionName - Name of the region to fetch from
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheConnectionJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheConnectionJUnitTest.java
index bfb1ff8..677d966 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheConnectionJUnitTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheConnectionJUnitTest.java
@@ -22,6 +22,7 @@ import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_REQUIRE_AUTHENTICATION;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD;
+import static org.apache.geode.internal.protocol.protobuf.v1.MessageUtil.validateGetResponse;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -61,11 +62,9 @@ import org.apache.geode.internal.cache.tier.sockets.AcceptorImpl;
 import org.apache.geode.internal.net.SocketCreator;
 import org.apache.geode.internal.net.SocketCreatorFactory;
 import org.apache.geode.internal.protocol.protobuf.statistics.ProtobufClientStatistics;
-import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
 import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol;
 import org.apache.geode.internal.protocol.protobuf.v1.MessageUtil;
 import org.apache.geode.internal.protocol.protobuf.v1.ProtobufSerializationService;
-import org.apache.geode.internal.protocol.protobuf.v1.RegionAPI;
 import org.apache.geode.internal.protocol.protobuf.v1.serializer.ProtobufProtocolSerializer;
 import org.apache.geode.internal.protocol.protobuf.v1.serializer.exception.InvalidProtocolMessageException;
 import org.apache.geode.test.junit.categories.IntegrationTest;
@@ -215,18 +214,6 @@ public class CacheConnectionJUnitTest {
     assertEquals(ClientProtocol.Message.MessageTypeCase.PUTRESPONSE, response.getMessageTypeCase());
   }
 
-  private void validateGetResponse(Socket socket,
-      ProtobufProtocolSerializer protobufProtocolSerializer, Object expectedValue)
-      throws InvalidProtocolMessageException, IOException {
-    ClientProtocol.Message response = deserializeResponse(socket, protobufProtocolSerializer);
-
-    assertEquals(ClientProtocol.Message.MessageTypeCase.GETRESPONSE, response.getMessageTypeCase());
-    RegionAPI.GetResponse getResponse = response.getGetResponse();
-    BasicTypes.EncodedValue result = getResponse.getResult();
-    assertEquals(BasicTypes.EncodedValue.ValueCase.STRINGRESULT, result.getValueCase());
-    assertEquals(expectedValue, result.getStringResult());
-  }
-
   private ClientProtocol.Message deserializeResponse(Socket socket,
       ProtobufProtocolSerializer protobufProtocolSerializer)
       throws InvalidProtocolMessageException, IOException {
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheOperationsJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheOperationsJUnitTest.java
index a997590..c312532 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheOperationsJUnitTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheOperationsJUnitTest.java
@@ -22,6 +22,7 @@ import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_REQUIRE_AUTHENTICATION;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD;
+import static org.apache.geode.internal.protocol.protobuf.v1.MessageUtil.validateGetResponse;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -101,6 +102,7 @@ public class CacheOperationsJUnitTest {
 
   @Rule
   public TestName testName = new TestName();
+  private ProtobufProtocolSerializer protobufProtocolSerializer;
 
   @Before
   public void setup() throws Exception {
@@ -139,6 +141,7 @@ public class CacheOperationsJUnitTest {
     MessageUtil.performAndVerifyHandshake(socket);
 
     serializationService = new ProtobufSerializationService();
+    protobufProtocolSerializer = new ProtobufProtocolSerializer();
   }
 
   @After
@@ -174,7 +177,7 @@ public class CacheOperationsJUnitTest {
         ProtobufRequestUtilities.createGetAllRequest(TEST_REGION, getEntries);
 
     ClientProtocol.Message getAllMessage =
-        ProtobufUtilities.createProtobufRequestWithGetAllRequest(getAllRequest);
+        ClientProtocol.Message.newBuilder().setGetAllRequest(getAllRequest).build();
     protobufProtocolSerializer.serialize(getAllMessage, outputStream);
     validateGetAllResponse(socket, protobufProtocolSerializer);
 
@@ -194,7 +197,6 @@ public class CacheOperationsJUnitTest {
     regionFactory.create(regionName);
     System.setProperty("geode.feature-protobuf-protocol", "true");
 
-    ProtobufProtocolSerializer protobufProtocolSerializer = new ProtobufProtocolSerializer();
     Set<BasicTypes.Entry> putEntries = new HashSet<>();
     putEntries.add(ProtobufUtilities.createEntry(serializationService, 2.2f, TEST_MULTIOP_VALUE1));
     putEntries.add(ProtobufUtilities.createEntry(serializationService, TEST_MULTIOP_KEY2,
@@ -224,7 +226,6 @@ public class CacheOperationsJUnitTest {
   @Test
   public void testResponseToGetWithNoData() throws Exception {
     // Get request without any data set must return a null
-    ProtobufProtocolSerializer protobufProtocolSerializer = new ProtobufProtocolSerializer();
     ClientProtocol.Message getMessage =
         MessageUtil.makeGetRequestMessage(serializationService, TEST_KEY, TEST_REGION);
     protobufProtocolSerializer.serialize(getMessage, outputStream);
@@ -233,12 +234,11 @@ public class CacheOperationsJUnitTest {
     assertEquals(ClientProtocol.Message.MessageTypeCase.GETRESPONSE, response.getMessageTypeCase());
     RegionAPI.GetResponse getResponse = response.getGetResponse();
 
-    assertFalse(getResponse.hasResult());
+    assertEquals(null, serializationService.decode(getResponse.getResult()));
   }
 
   @Test
   public void testNewProtocolGetRegionNamesCallSucceeds() throws Exception {
-    ProtobufProtocolSerializer protobufProtocolSerializer = new ProtobufProtocolSerializer();
     RegionAPI.GetRegionNamesRequest getRegionNamesRequest =
         ProtobufRequestUtilities.createGetRegionNamesRequest();
 
@@ -252,7 +252,6 @@ public class CacheOperationsJUnitTest {
   public void testNewProtocolGetSizeCall() throws Exception {
     System.setProperty("geode.feature-protobuf-protocol", "true");
 
-    ProtobufProtocolSerializer protobufProtocolSerializer = new ProtobufProtocolSerializer();
     ClientProtocol.Message putMessage = ProtobufRequestUtilities.createPutRequest(TEST_REGION,
         ProtobufUtilities.createEntry(serializationService, TEST_KEY, TEST_VALUE));
     protobufProtocolSerializer.serialize(putMessage, outputStream);
@@ -269,17 +268,6 @@ public class CacheOperationsJUnitTest {
     assertEquals(1, getSizeResponse.getSize());
   }
 
-  private void validateGetResponse(Socket socket,
-      ProtobufProtocolSerializer protobufProtocolSerializer, Object expectedValue)
-      throws InvalidProtocolMessageException, IOException {
-    ClientProtocol.Message response = deserializeResponse(socket, protobufProtocolSerializer);
-
-    assertEquals(ClientProtocol.Message.MessageTypeCase.GETRESPONSE, response.getMessageTypeCase());
-    RegionAPI.GetResponse getResponse = response.getGetResponse();
-    BasicTypes.EncodedValue result = getResponse.getResult();
-    assertEquals(BasicTypes.EncodedValue.ValueCase.STRINGRESULT, result.getValueCase());
-    assertEquals(expectedValue, result.getStringResult());
-  }
 
   private ClientProtocol.Message deserializeResponse(Socket socket,
       ProtobufProtocolSerializer protobufProtocolSerializer)
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandlerJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandlerJUnitTest.java
index 9ceb31f..e21f051 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandlerJUnitTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandlerJUnitTest.java
@@ -147,7 +147,7 @@ public class GetAllRequestOperationHandlerJUnitTest extends OperationHandlerJUni
   }
 
   @Test
-  public void singeNullKey() throws Exception {
+  public void singleNullKey() throws Exception {
     HashSet<BasicTypes.EncodedValue> testKeys = new HashSet<>();
     testKeys.add(serializationService.encode(NO_VALUE_PRESENT_FOR_THIS_KEY));
     RegionAPI.GetAllRequest getAllRequest =
@@ -158,7 +158,7 @@ public class GetAllRequestOperationHandlerJUnitTest extends OperationHandlerJUni
     assertTrue(result instanceof Success);
     RegionAPI.GetAllResponse message = (RegionAPI.GetAllResponse) result.getMessage();
     assertEquals(1, message.getEntriesCount());
-    assertFalse(message.getEntries(0).hasValue());
+    assertEquals(null, serializationService.decode(message.getEntries(0).getValue()));
     assertEquals(NO_VALUE_PRESENT_FOR_THIS_KEY, message.getEntries(0).getKey().getStringResult());
 
     verify(regionStub, times(1)).get(NO_VALUE_PRESENT_FOR_THIS_KEY);
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAndPutJsonDocumentsDUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAndPutJsonDocumentsDUnitTest.java
index e51b6df..2ddfbcb 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAndPutJsonDocumentsDUnitTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAndPutJsonDocumentsDUnitTest.java
@@ -77,7 +77,7 @@ public class GetAndPutJsonDocumentsDUnitTest extends JUnit4CacheTestCase {
 
   private static ProtobufSerializationService serializationService;
 
-  VM storingVM;
+  private VM storingVM;
 
   @Before
   public void setUp() throws Exception {
@@ -103,8 +103,6 @@ public class GetAndPutJsonDocumentsDUnitTest extends JUnit4CacheTestCase {
     }
   }
 
-
-
   @Test
   public void testThatGetReturnsJSONDocumentForPdxInstance() throws Exception {
     storeTestDocument();
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandlerJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandlerJUnitTest.java
index 973fa5a..4e7af95 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandlerJUnitTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandlerJUnitTest.java
@@ -100,10 +100,11 @@ public class GetRequestOperationHandlerJUnitTest extends OperationHandlerJUnitTe
   @Test
   public void processReturnsLookupFailureWhenKeyFoundWithNoValue() throws Exception {
     RegionAPI.GetRequest getRequest = generateTestRequest(false, false, true);
-    Result response = operationHandler.process(serializationService, getRequest,
-        TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+    Result<RegionAPI.GetResponse> response = operationHandler.process(serializationService,
+        getRequest, TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
 
     Assert.assertTrue(response instanceof Success);
+    Assert.assertNull(serializationService.decode(response.getMessage().getResult()));
   }
 
   @Test(expected = DecodingException.class)
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutIfAbsentRequestIntegrationTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutIfAbsentRequestIntegrationTest.java
new file mode 100644
index 0000000..008e495
--- /dev/null
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutIfAbsentRequestIntegrationTest.java
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF 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.apache.geode.internal.protocol.protobuf.v1.operations;
+
+import static org.apache.geode.internal.protocol.protobuf.v1.MessageUtil.validateGetResponse;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.Socket;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+import org.awaitility.Awaitility;
+import org.junit.After;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.internal.net.SocketCreatorFactory;
+import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
+import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol;
+import org.apache.geode.internal.protocol.protobuf.v1.MessageUtil;
+import org.apache.geode.internal.protocol.protobuf.v1.ProtobufRequestUtilities;
+import org.apache.geode.internal.protocol.protobuf.v1.ProtobufSerializationService;
+import org.apache.geode.internal.protocol.protobuf.v1.RegionAPI;
+import org.apache.geode.internal.protocol.protobuf.v1.serializer.ProtobufProtocolSerializer;
+import org.apache.geode.internal.protocol.protobuf.v1.serializer.exception.InvalidProtocolMessageException;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+public class PutIfAbsentRequestIntegrationTest {
+  private static final String TEST_REGION = "testRegion";
+  private static final Object TEST_KEY = "testKey";
+  private Cache cache;
+  private Socket socket;
+  private OutputStream outputStream;
+  private ProtobufSerializationService serializationService;
+  private ProtobufProtocolSerializer protobufProtocolSerializer;
+
+  @Rule
+  public final RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
+  private InputStream inputStream;
+
+  private void doSetup(DataPolicy dataPolicy) throws IOException {
+    System.setProperty("geode.feature-protobuf-protocol", "true");
+
+    CacheFactory cacheFactory = new CacheFactory(new Properties());
+    cacheFactory.set(ConfigurationProperties.MCAST_PORT, "0");
+    cacheFactory.set(ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION, "false");
+    cacheFactory.set(ConfigurationProperties.USE_CLUSTER_CONFIGURATION, "false");
+    cache = cacheFactory.create();
+
+    CacheServer cacheServer = cache.addCacheServer();
+    final int cacheServerPort = AvailablePortHelper.getRandomAvailableTCPPort();
+    cacheServer.setPort(cacheServerPort);
+    cacheServer.start();
+
+    RegionFactory<Object, Object> regionFactory = cache.createRegionFactory();
+    regionFactory.setDataPolicy(dataPolicy);
+    regionFactory.create(TEST_REGION);
+
+
+    socket = new Socket("localhost", cacheServerPort);
+    Awaitility.await().atMost(5, TimeUnit.SECONDS).until(socket::isConnected);
+    outputStream = socket.getOutputStream();
+    inputStream = socket.getInputStream();
+
+    MessageUtil.performAndVerifyHandshake(socket);
+
+    serializationService = new ProtobufSerializationService();
+    protobufProtocolSerializer = new ProtobufProtocolSerializer();
+  }
+
+  @After
+  public void cleanUp() throws IOException {
+    cache.close();
+    socket.close();
+    SocketCreatorFactory.close();
+  }
+
+  @Test
+  public void testPutIfAbsentRequest() throws Exception {
+    doSetup(DataPolicy.REPLICATE);
+
+    final BasicTypes.EncodedValue encodedKey = serializationService.encode(TEST_KEY);
+    final String testValue = "testValue";
+    final String testValue2 = "testValue2";
+    final BasicTypes.Entry entry1 = BasicTypes.Entry.newBuilder().setKey(encodedKey)
+        .setValue(serializationService.encode(testValue)).build();
+    assertNull(serializationService.decode(doPutIfAbsent(entry1).getOldValue()));
+
+    protobufProtocolSerializer.serialize(
+        ProtobufRequestUtilities.createGetRequest(TEST_REGION, encodedKey),
+        socket.getOutputStream());
+    validateGetResponse(socket, protobufProtocolSerializer, testValue);
+
+    final BasicTypes.Entry entry2 = BasicTypes.Entry.newBuilder().setKey(encodedKey)
+        .setValue(serializationService.encode(testValue2)).build();
+
+    // same value still present
+    assertEquals(testValue, serializationService.decode(doPutIfAbsent(entry2).getOldValue()));
+    protobufProtocolSerializer.serialize(
+        ProtobufRequestUtilities.createGetRequest(TEST_REGION, encodedKey),
+        socket.getOutputStream());
+    validateGetResponse(socket, protobufProtocolSerializer, testValue);
+  }
+
+  /**
+   * This should fail because DataPolicy.NORMAL doesn't allow concurrent cache ops.
+   */
+  @Test
+  public void testPutIfAbsentRequestOnDataPolicyNormal() throws Exception {
+    doSetup(DataPolicy.NORMAL);
+
+    final BasicTypes.EncodedValue encodedKey = serializationService.encode(TEST_KEY);
+    final String testValue = "testValue";
+    final BasicTypes.EncodedValue encodedValue = serializationService.encode(testValue);
+    final BasicTypes.Entry entry =
+        BasicTypes.Entry.newBuilder().setKey(encodedKey).setValue(encodedValue).build();
+    ProtobufRequestUtilities.createPutIfAbsentRequest(TEST_REGION, entry)
+        .writeDelimitedTo(outputStream);
+
+    final ClientProtocol.Message response = ClientProtocol.Message.parseDelimitedFrom(inputStream);
+
+    assertEquals(ClientProtocol.Message.MessageTypeCase.ERRORRESPONSE,
+        response.getMessageTypeCase());
+    assertEquals(BasicTypes.ErrorCode.UNSUPPORTED_OPERATION,
+        response.getErrorResponse().getError().getErrorCode());
+  }
+
+  private RegionAPI.PutIfAbsentResponse doPutIfAbsent(BasicTypes.Entry entry)
+      throws IOException, InvalidProtocolMessageException {
+    final ClientProtocol.Message putIfAbsentRequest =
+        ProtobufRequestUtilities.createPutIfAbsentRequest(TEST_REGION, entry);
+
+    protobufProtocolSerializer.serialize(putIfAbsentRequest, outputStream);
+    ClientProtocol.Message response = protobufProtocolSerializer.deserialize(inputStream);
+
+    assertEquals(ClientProtocol.Message.MessageTypeCase.PUTIFABSENTRESPONSE,
+        response.getMessageTypeCase());
+    return response.getPutIfAbsentResponse();
+  }
+
+
+}
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutIfAbsentRequestOperationHandlerJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutIfAbsentRequestOperationHandlerJUnitTest.java
new file mode 100644
index 0000000..6531f75
--- /dev/null
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutIfAbsentRequestOperationHandlerJUnitTest.java
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF 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.apache.geode.internal.protocol.protobuf.v1.operations;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Region;
+import org.apache.geode.internal.protocol.TestExecutionContext;
+import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
+import org.apache.geode.internal.protocol.protobuf.v1.RegionAPI;
+import org.apache.geode.internal.protocol.protobuf.v1.Result;
+import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.DecodingException;
+import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.EncodingException;
+import org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufUtilities;
+import org.apache.geode.internal.util.PasswordUtil;
+import org.apache.geode.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+@SuppressWarnings("unchecked") // Region lacks generics when we look it up
+public class PutIfAbsentRequestOperationHandlerJUnitTest extends OperationHandlerJUnitTest {
+  private final String TEST_KEY = "my key";
+  private final String TEST_VALUE = "99";
+  private final String TEST_REGION = "test region";
+  private Region regionMock;
+  private PutIfAbsentRequestOperationHandler operationHandler;
+
+  @Before
+  public void setUp() throws Exception {
+    regionMock = mock(Region.class);
+    operationHandler = new PutIfAbsentRequestOperationHandler();
+    when(cacheStub.getRegion(TEST_REGION)).thenReturn(regionMock);
+  }
+
+  @Test
+  public void newEntrySucceeds() throws Exception {
+    when(regionMock.putIfAbsent(TEST_KEY, TEST_VALUE)).thenReturn(null);
+
+    Result<RegionAPI.PutIfAbsentResponse> result1 = operationHandler.process(serializationService,
+        generateTestRequest(), TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+
+    assertNull(serializationService.decode(result1.getMessage().getOldValue()));
+
+    verify(regionMock).putIfAbsent(TEST_KEY, TEST_VALUE);
+    verify(regionMock, times(1)).putIfAbsent(any(), any());
+  }
+
+  @Test
+  public void existingEntryFails() throws Exception {
+    when(regionMock.putIfAbsent(TEST_KEY, TEST_VALUE)).thenReturn(1);
+
+    Result<RegionAPI.PutIfAbsentResponse> result1 = operationHandler.process(serializationService,
+        generateTestRequest(), TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+
+    assertNotNull(serializationService.decode(result1.getMessage().getOldValue()));
+
+    verify(regionMock).putIfAbsent(TEST_KEY, TEST_VALUE);
+    verify(regionMock, times(1)).putIfAbsent(any(), any());
+  }
+
+  @Test
+  public void nullValuePassedThrough() throws Exception {
+    final RegionAPI.PutIfAbsentRequest request =
+        RegionAPI.PutIfAbsentRequest.newBuilder().setRegionName(TEST_REGION)
+            .setEntry(ProtobufUtilities.createEntry(serializationService, TEST_KEY, null)).build();
+
+    Result<RegionAPI.PutIfAbsentResponse> response = operationHandler.process(serializationService,
+        request, TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+
+    assertNull(serializationService.decode(response.getMessage().getOldValue()));
+
+    verify(regionMock).putIfAbsent(TEST_KEY, null);
+  }
+
+  @Test
+  public void nullKeyPassedThrough() throws Exception {
+    final RegionAPI.PutIfAbsentRequest request = RegionAPI.PutIfAbsentRequest.newBuilder()
+        .setRegionName(TEST_REGION)
+        .setEntry(ProtobufUtilities.createEntry(serializationService, null, TEST_VALUE)).build();
+
+    Result<RegionAPI.PutIfAbsentResponse> response = operationHandler.process(serializationService,
+        request, TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+
+    assertNull(serializationService.decode(response.getMessage().getOldValue()));
+
+    verify(regionMock).putIfAbsent(null, TEST_VALUE);
+  }
+
+  @Test
+  public void failsWithNoAuthCacheExecutionContext() throws Exception {
+    Result<RegionAPI.PutIfAbsentResponse> result1 = operationHandler.process(serializationService,
+        RegionAPI.PutIfAbsentRequest.newBuilder().build(),
+        TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+
+    assertEquals(BasicTypes.ErrorCode.SERVER_ERROR,
+        result1.getErrorMessage().getError().getErrorCode());
+  }
+
+  @Test(expected = DecodingException.class)
+  public void unsetEntrythrowsDecodingException() throws Exception {
+    Result<RegionAPI.PutIfAbsentResponse> result1 =
+        operationHandler.process(serializationService, generateTestRequest(true, false),
+            TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+
+    assertEquals(BasicTypes.ErrorCode.INVALID_REQUEST,
+        result1.getErrorMessage().getError().getErrorCode());
+  }
+
+  @Test
+  public void unsetRegionGetsServerError() throws Exception {
+    Result<RegionAPI.PutIfAbsentResponse> result1 =
+        operationHandler.process(serializationService, generateTestRequest(false, true),
+            TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+
+    assertEquals(BasicTypes.ErrorCode.SERVER_ERROR,
+        result1.getErrorMessage().getError().getErrorCode());
+  }
+
+  @Test
+  public void nonexistingRegionReturnsServerError() throws Exception {
+    when(cacheStub.getRegion(TEST_REGION)).thenReturn(null);
+
+    Result<RegionAPI.PutIfAbsentResponse> result1 = operationHandler.process(serializationService,
+        generateTestRequest(), TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+
+    assertEquals(BasicTypes.ErrorCode.SERVER_ERROR,
+        result1.getErrorMessage().getError().getErrorCode());
+  }
+
+  /**
+   * Some regions (DataPolicy.NORMAL, for example) don't support concurrent ops such as putIfAbsent.
+   */
+  @Test(expected = UnsupportedOperationException.class)
+  public void unsupportedOperation() throws Exception {
+    when(regionMock.putIfAbsent(any(), any())).thenThrow(new UnsupportedOperationException());
+
+    Result<RegionAPI.PutIfAbsentResponse> result1 = operationHandler.process(serializationService,
+        generateTestRequest(), TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+    assertEquals(BasicTypes.ErrorCode.INVALID_REQUEST,
+        result1.getErrorMessage().getError().getErrorCode());
+  }
+
+  @Test
+  public void invalidRegionReturnsInvalidRequestError() throws Exception {
+    // doesn't test which regions are invalid; those are documented under Cache.getRegion.
+    when(cacheStub.getRegion(any())).thenThrow(new IllegalArgumentException());
+
+    Result<RegionAPI.PutIfAbsentResponse> result1 = operationHandler.process(serializationService,
+        generateTestRequest(), TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
+    assertEquals(BasicTypes.ErrorCode.INVALID_REQUEST,
+        result1.getErrorMessage().getError().getErrorCode());
+  }
+
+  private RegionAPI.PutIfAbsentRequest generateTestRequest(boolean includeRegion,
+      boolean includeEntry) throws EncodingException {
+    RegionAPI.PutIfAbsentRequest.Builder builder = RegionAPI.PutIfAbsentRequest.newBuilder();
+
+    if (includeRegion) {
+      builder.setRegionName(TEST_REGION);
+    }
+
+    if (includeEntry) {
+      BasicTypes.EncodedValue testKey = serializationService.encode(TEST_KEY);
+      BasicTypes.EncodedValue testValue = serializationService.encode(TEST_VALUE);
+      BasicTypes.Entry testEntry = ProtobufUtilities.createEntry(testKey, testValue);
+      builder.setEntry(testEntry);
+    }
+
+    return builder.build();
+  }
+
+  private RegionAPI.PutIfAbsentRequest generateTestRequest() throws EncodingException {
+    return generateTestRequest(true, true);
+  }
+}

-- 
To stop receiving notification emails like this one, please contact
gosullivan@apache.org.

Mime
View raw message