This is an automated email from the ASF dual-hosted git repository.

wirebaron 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 37ff715  GEODE-4414: Support explicit nulls in geode's protobuf 
protocol (#1437)
37ff715 is described below

commit 37ff71531d900c160f4d2e05042f509f96da60ef
Author: Brian Rowe <br...@pivotal.io>
AuthorDate: Thu Feb 22 11:07:27 2018 -0800

    GEODE-4414: Support explicit nulls in geode's protobuf protocol (#1437)
---
 geode-protobuf-messages/src/main/proto/v1/basicTypes.proto     |  4 ++++
 .../protocol/protobuf/v1/ProtobufSerializationService.java     | 10 +++++++---
 .../ExecuteFunctionOnGroupRequestOperationHandler.java         |  6 +++++-
 .../ExecuteFunctionOnMemberRequestOperationHandler.java        |  6 +++++-
 .../ExecuteFunctionOnRegionRequestOperationHandler.java        |  6 +++++-
 .../protobuf/v1/operations/GetAllRequestOperationHandler.java  |  5 +++++
 .../protobuf/v1/operations/GetRequestOperationHandler.java     |  3 +++
 .../protobuf/v1/operations/PutAllRequestOperationHandler.java  |  7 +++++--
 .../protobuf/v1/operations/PutRequestOperationHandler.java     |  5 +++++
 .../protobuf/v1/operations/RemoveRequestOperationHandler.java  |  4 ++++
 .../protobuf/v1/utilities/ProtobufUtilitiesJUnitTest.java      |  5 +++++
 11 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto 
b/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
index 2fa98b8..a0ca4aa 100644
--- a/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
+++ b/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
@@ -22,6 +22,8 @@
 syntax = "proto3";
 package org.apache.geode.internal.protocol.protobuf.v1;
 
+import "google/protobuf/struct.proto";
+
 message Entry {
     EncodedValue key = 1;
     EncodedValue value = 2;
@@ -40,6 +42,8 @@ message EncodedValue {
         string stringResult = 9;
 
         string jsonObjectResult = 10;
+
+        google.protobuf.NullValue nullResult = 11;
     }
 }
 
diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufSerializationService.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufSerializationService.java
index 33a42aa..e2a8429 100644
--- 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufSerializationService.java
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufSerializationService.java
@@ -15,6 +15,7 @@
 package org.apache.geode.internal.protocol.protobuf.v1;
 
 import com.google.protobuf.ByteString;
+import com.google.protobuf.NullValue;
 
 import org.apache.geode.annotations.Experimental;
 import 
org.apache.geode.internal.protocol.protobuf.v1.serialization.JsonPdxConverter;
@@ -38,11 +39,10 @@ public class ProtobufSerializationService implements 
SerializationService<BasicT
   @Override
   public BasicTypes.EncodedValue encode(Object value) throws EncodingException 
{
     if (value == null) {
-      return BasicTypes.EncodedValue.getDefaultInstance();
+      return 
BasicTypes.EncodedValue.newBuilder().setNullResult(NullValue.NULL_VALUE).build();
     }
 
     BasicTypes.EncodedValue.Builder builder = 
BasicTypes.EncodedValue.newBuilder();
-
     try {
       ProtobufEncodingTypes protobufEncodingTypes = 
ProtobufEncodingTypes.valueOf(value.getClass());
       switch (protobufEncodingTypes) {
@@ -86,10 +86,14 @@ public class ProtobufSerializationService implements 
SerializationService<BasicT
           builder.setJsonObjectResult(jsonPdxConverter.encode((PdxInstance) 
value));
           break;
         }
+        default:
+          throw new EncodingException("No handler for protobuf type "
+              + ProtobufEncodingTypes.valueOf(value.getClass()).toString());
       }
     } catch (UnknownProtobufEncodingType unknownProtobufEncodingType) {
       throw new EncodingException("No protobuf encoding for type " + 
value.getClass().getName());
     }
+
     return builder.build();
   }
 
@@ -121,7 +125,7 @@ public class ProtobufSerializationService implements 
SerializationService<BasicT
         return encodedValue.getStringResult();
       case JSONOBJECTRESULT:
         return jsonPdxConverter.decode(encodedValue.getJsonObjectResult());
-      case VALUE_NOT_SET:
+      case NULLRESULT:
         return null;
       default:
         throw new DecodingException(
diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnGroupRequestOperationHandler.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnGroupRequestOperationHandler.java
index 04cffb0..3224e81 100644
--- 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnGroupRequestOperationHandler.java
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnGroupRequestOperationHandler.java
@@ -85,7 +85,11 @@ public class ExecuteFunctionOnGroupRequestOperationHandler 
extends
   @Override
   protected Object getFunctionArguments(ExecuteFunctionOnGroupRequest request,
       ProtobufSerializationService serializationService) throws 
DecodingException {
-    return serializationService.decode(request.getArguments());
+    if (request.hasArguments()) {
+      return serializationService.decode(request.getArguments());
+    } else {
+      return null;
+    }
   }
 
   @Override
diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnMemberRequestOperationHandler.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnMemberRequestOperationHandler.java
index d9e2ee7..63ca258 100644
--- 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnMemberRequestOperationHandler.java
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnMemberRequestOperationHandler.java
@@ -84,7 +84,11 @@ public class ExecuteFunctionOnMemberRequestOperationHandler 
extends
   @Override
   protected Object getFunctionArguments(ExecuteFunctionOnMemberRequest request,
       ProtobufSerializationService serializationService) throws 
DecodingException {
-    return serializationService.decode(request.getArguments());
+    if (request.hasArguments()) {
+      return serializationService.decode(request.getArguments());
+    } else {
+      return null;
+    }
   }
 
   @Override
diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnRegionRequestOperationHandler.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnRegionRequestOperationHandler.java
index 3f79826..7c43f5d 100644
--- 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnRegionRequestOperationHandler.java
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnRegionRequestOperationHandler.java
@@ -71,7 +71,11 @@ public class ExecuteFunctionOnRegionRequestOperationHandler 
extends
   @Override
   protected Object getFunctionArguments(ExecuteFunctionOnRegionRequest request,
       ProtobufSerializationService serializationService) throws 
DecodingException {
-    return serializationService.decode(request.getArguments());
+    if (request.hasArguments()) {
+      return serializationService.decode(request.getArguments());
+    } else {
+      return null;
+    }
   }
 
   @Override
diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandler.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandler.java
index b70b471..a672842 100644
--- 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandler.java
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandler.java
@@ -71,6 +71,11 @@ public class GetAllRequestOperationHandler
     try {
 
       Object decodedKey = serializationService.decode(key);
+      if (decodedKey == null) {
+        responseBuilder
+            .addFailures(buildKeyedError(key, "NULL is not a valid key for 
get.", INVALID_REQUEST));
+        return;
+      }
       Object value = region.get(decodedKey);
       BasicTypes.Entry entry =
           ProtobufUtilities.createEntry(serializationService, decodedKey, 
value);
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 4bbff03..76f85a0 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
@@ -52,6 +52,9 @@ public class GetRequestOperationHandler
       
messageExecutionContext.getCache().setReadSerializedForCurrentThread(true);
 
       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);
 
       if (resultValue == null) {
diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutAllRequestOperationHandler.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutAllRequestOperationHandler.java
index 94a9bb2..e331af5 100644
--- 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutAllRequestOperationHandler.java
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutAllRequestOperationHandler.java
@@ -70,9 +70,12 @@ public class PutAllRequestOperationHandler
   private void processSinglePut(RegionAPI.PutAllResponse.Builder builder,
       SerializationService serializationService, Region region, 
BasicTypes.Entry entry) {
     try {
-
-      Object decodedKey = serializationService.decode(entry.getKey());
       Object decodedValue = serializationService.decode(entry.getValue());
+      Object decodedKey = serializationService.decode(entry.getKey());
+      if (decodedKey == null || decodedValue == null) {
+        builder.addFailedKeys(
+            buildKeyedError(entry, INVALID_REQUEST, "Key and value must both 
be non-NULL"));
+      }
       region.put(decodedKey, decodedValue);
 
     } catch (DecodingException ex) {
diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutRequestOperationHandler.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutRequestOperationHandler.java
index 1969c3b..a39fab8 100644
--- 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutRequestOperationHandler.java
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutRequestOperationHandler.java
@@ -53,6 +53,11 @@ public class PutRequestOperationHandler
 
       Object decodedValue = serializationService.decode(entry.getValue());
       Object decodedKey = serializationService.decode(entry.getKey());
+      if (decodedKey == null || decodedValue == null) {
+        return Failure.of(BasicTypes.ErrorCode.INVALID_REQUEST,
+            "Key and value must both be non-NULL");
+      }
+
       try {
         region.put(decodedKey, decodedValue);
         return Success.of(RegionAPI.PutResponse.newBuilder().build());
diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/RemoveRequestOperationHandler.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/RemoveRequestOperationHandler.java
index d81cb12..dbc146d 100644
--- 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/RemoveRequestOperationHandler.java
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/RemoveRequestOperationHandler.java
@@ -50,6 +50,10 @@ public class RemoveRequestOperationHandler
     long startTime = messageExecutionContext.getStatistics().startOperation();
     try {
       Object decodedKey = serializationService.decode(request.getKey());
+      if (decodedKey == null) {
+        return Failure.of(BasicTypes.ErrorCode.INVALID_REQUEST,
+            "NULL is not a valid key for removal.");
+      }
       region.remove(decodedKey);
 
       return Success.of(RegionAPI.RemoveResponse.newBuilder().build());
diff --git 
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilitiesJUnitTest.java
 
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilitiesJUnitTest.java
index 52ef576..199b97d 100644
--- 
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilitiesJUnitTest.java
+++ 
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilitiesJUnitTest.java
@@ -144,6 +144,11 @@ public class ProtobufUtilitiesJUnitTest {
     createAndVerifyEncodedValue("Some String text to test");
   }
 
+  @Test
+  public void doesANullValueSuccessfullyEncodeIntoEncodedValues() throws 
Exception {
+    createAndVerifyEncodedValue(null);
+  }
+
   private <T> void createAndVerifyEncodedValue(T testObj) throws 
EncodingException {
     BasicTypes.EncodedValue encodedValue = 
protobufSerializationService.encode(testObj);
     if (testObj instanceof byte[]) {

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

Reply via email to