Repository: geode Updated Branches: refs/heads/develop e02be316e -> aef891296
GEODE-2998: Add remove operation Signed-off-by: Alexander Murmann <amurm...@pivotal.io> Signed-off-by: Matt Kalinowski <mkalinow...@pivotal.io> This closes #624 Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/aef89129 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/aef89129 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/aef89129 Branch: refs/heads/develop Commit: aef89129626d9072dfc89e8acc038ecad1793c26 Parents: e02be31 Author: Brian Rowe <br...@pivotal.io> Authored: Thu Jul 6 16:36:51 2017 -0700 Committer: Hitesh Khamesra <hkhame...@pivotal.io> Committed: Tue Jul 11 14:18:39 2017 -0700 ---------------------------------------------------------------------- .../protobuf/ProtobufStreamProcessor.java | 4 + .../RemoveRequestOperationHandler.java | 65 +++++++++ .../utilities/ProtobufRequestUtilities.java | 14 ++ .../utilities/ProtobufResponseUtilities.java | 10 ++ geode-protobuf/src/main/proto/region_API.proto | 1 - .../RoundTripCacheConnectionJUnitTest.java | 20 +++ .../RemoveRequestOperationHandlerJUnitTest.java | 142 +++++++++++++++++++ 7 files changed, 255 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/aef89129/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java ---------------------------------------------------------------------- diff --git a/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java b/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java index ebd5c6a..7146392 100644 --- a/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java +++ b/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/ProtobufStreamProcessor.java @@ -24,6 +24,7 @@ import org.apache.geode.protocol.operations.registry.exception.OperationHandlerN import org.apache.geode.protocol.protobuf.operations.GetRegionNamesRequestOperationHandler; import org.apache.geode.protocol.protobuf.operations.GetRequestOperationHandler; import org.apache.geode.protocol.protobuf.operations.PutRequestOperationHandler; +import org.apache.geode.protocol.protobuf.operations.RemoveRequestOperationHandler; import org.apache.geode.protocol.protobuf.serializer.ProtobufProtocolSerializer; import org.apache.geode.protocol.protobuf.utilities.ProtobufUtilities; import org.apache.geode.serialization.registry.exception.CodecAlreadyRegisteredForTypeException; @@ -63,6 +64,9 @@ public class ProtobufStreamProcessor implements ClientProtocolMessageHandler { registry.registerOperationHandlerForOperationId( ClientProtocol.Request.RequestAPICase.GETREGIONNAMESREQUEST.getNumber(), new GetRegionNamesRequestOperationHandler()); + registry.registerOperationHandlerForOperationId( + ClientProtocol.Request.RequestAPICase.REMOVEREQUEST.getNumber(), + new RemoveRequestOperationHandler()); } public void processOneMessage(InputStream inputStream, OutputStream outputStream, Cache cache) http://git-wip-us.apache.org/repos/asf/geode/blob/aef89129/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/RemoveRequestOperationHandler.java ---------------------------------------------------------------------- diff --git a/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/RemoveRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/RemoveRequestOperationHandler.java new file mode 100644 index 0000000..e1fef85 --- /dev/null +++ b/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/RemoveRequestOperationHandler.java @@ -0,0 +1,65 @@ +/* + * 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.protocol.protobuf.operations; + +import org.apache.geode.cache.Cache; +import org.apache.geode.cache.Region; +import org.apache.geode.protocol.operations.OperationHandler; +import org.apache.geode.protocol.protobuf.BasicTypes; +import org.apache.geode.protocol.protobuf.ClientProtocol; +import org.apache.geode.protocol.protobuf.RegionAPI; +import org.apache.geode.protocol.protobuf.utilities.ProtobufResponseUtilities; +import org.apache.geode.protocol.protobuf.utilities.ProtobufUtilities; +import org.apache.geode.serialization.SerializationService; +import org.apache.geode.serialization.exception.UnsupportedEncodingTypeException; +import org.apache.geode.serialization.registry.exception.CodecNotRegisteredForTypeException; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +public class RemoveRequestOperationHandler + implements OperationHandler<ClientProtocol.Request, ClientProtocol.Response> { + private static Logger logger = LogManager.getLogger(); + + @Override + public ClientProtocol.Response process(SerializationService serializationService, + ClientProtocol.Request request, Cache cache) { + if (request.getRequestAPICase() != ClientProtocol.Request.RequestAPICase.REMOVEREQUEST) { + return ProtobufResponseUtilities.createAndLogErrorResponse(false, false, + "Improperly formatted get request message.", logger, null); + } + RegionAPI.RemoveRequest removeRequest = request.getRemoveRequest(); + + String regionName = removeRequest.getRegionName(); + Region region = cache.getRegion(regionName); + if (region == null) { + return ProtobufResponseUtilities.createErrorResponse(false, false, "Region not found"); + } + + try { + Object decodedKey = + ProtobufUtilities.decodeValue(serializationService, removeRequest.getKey()); + region.remove(decodedKey); + + return ProtobufResponseUtilities.createRemoveResponse(); + } catch (UnsupportedEncodingTypeException ex) { + // can be thrown by encoding or decoding. + return ProtobufResponseUtilities.createAndLogErrorResponse(false, false, + "Encoding not supported.", logger, ex); + } catch (CodecNotRegisteredForTypeException ex) { + return ProtobufResponseUtilities.createAndLogErrorResponse(true, false, + "Codec error in protobuf deserialization.", logger, ex); + } + } +} http://git-wip-us.apache.org/repos/asf/geode/blob/aef89129/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufRequestUtilities.java ---------------------------------------------------------------------- diff --git a/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufRequestUtilities.java b/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufRequestUtilities.java index b96f478..b246a50 100644 --- a/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufRequestUtilities.java +++ b/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufRequestUtilities.java @@ -38,6 +38,20 @@ public abstract class ProtobufRequestUtilities { } /** + * Creates a request object containing a RegionAPI.RemoveRequest + * + * @param regionName - Name of the region being deleted from + * @param key - Encoded key, see createEncodedValue in {@link ProtobufRequestUtilities} + * @return Request object containing the passed params. + */ + public static ClientProtocol.Request createRemoveRequest(String regionName, + BasicTypes.EncodedValue key) { + RegionAPI.RemoveRequest removeRequest = + RegionAPI.RemoveRequest.newBuilder().setRegionName(regionName).setKey(key).build(); + return ClientProtocol.Request.newBuilder().setRemoveRequest(removeRequest).build(); + } + + /** * Creates a request object containing a RegionAPI.GetRegionNamesRequest * * @return Request object for a getRegionNames operation http://git-wip-us.apache.org/repos/asf/geode/blob/aef89129/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufResponseUtilities.java ---------------------------------------------------------------------- diff --git a/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufResponseUtilities.java b/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufResponseUtilities.java index 2114fdb..d6ef278 100644 --- a/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufResponseUtilities.java +++ b/geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/utilities/ProtobufResponseUtilities.java @@ -80,6 +80,16 @@ public abstract class ProtobufResponseUtilities { } /** + * This creates a response object containing a RegionAPI.RemoveResponse + * + * @return A response indicating the entry with the passed key was removed + */ + public static ClientProtocol.Response createRemoveResponse() { + RegionAPI.RemoveResponse removeResponse = RegionAPI.RemoveResponse.newBuilder().build(); + return ClientProtocol.Response.newBuilder().setRemoveResponse(removeResponse).build(); + } + + /** * This creates a response object containing a RegionAPI.GetResponse * * @return A response indicating a failure to find a requested key or value http://git-wip-us.apache.org/repos/asf/geode/blob/aef89129/geode-protobuf/src/main/proto/region_API.proto ---------------------------------------------------------------------- diff --git a/geode-protobuf/src/main/proto/region_API.proto b/geode-protobuf/src/main/proto/region_API.proto index 40c124a..3108cb7 100644 --- a/geode-protobuf/src/main/proto/region_API.proto +++ b/geode-protobuf/src/main/proto/region_API.proto @@ -69,7 +69,6 @@ message RemoveRequest { } message RemoveResponse { - bool success = 1; } message RemoveAllRequest { http://git-wip-us.apache.org/repos/asf/geode/blob/aef89129/geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java index 31a8736..3838648 100644 --- a/geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java +++ b/geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java @@ -50,6 +50,7 @@ import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; /** * Test that switching on the header byte makes instances of @@ -62,6 +63,7 @@ public class RoundTripCacheConnectionJUnitTest { public static final String TEST_REGION = "testRegion"; public static final int TEST_PUT_CORRELATION_ID = 574; public static final int TEST_GET_CORRELATION_ID = 68451; + public static final int TEST_REMOVE_CORRELATION_ID = 51; private Cache cache; private int cacheServerPort; @@ -111,6 +113,13 @@ public class RoundTripCacheConnectionJUnitTest { TEST_KEY, TEST_REGION, ProtobufUtilities.createMessageHeader(TEST_GET_CORRELATION_ID)); protobufProtocolSerializer.serialize(getMessage, outputStream); validateGetResponse(socket, protobufProtocolSerializer); + + ClientProtocol.Message removeMessage = ProtobufUtilities.createProtobufRequest( + ProtobufUtilities.createMessageHeader(TEST_REMOVE_CORRELATION_ID), + ProtobufRequestUtilities.createRemoveRequest(TEST_REGION, + ProtobufUtilities.createEncodedValue(serializationService, TEST_KEY))); + protobufProtocolSerializer.serialize(removeMessage, outputStream); + validateRemoveResponse(socket, protobufProtocolSerializer); } @Test @@ -206,4 +215,15 @@ public class RoundTripCacheConnectionJUnitTest { assertEquals(1, getRegionsResponse.getRegionsCount()); assertEquals(TEST_REGION, getRegionsResponse.getRegions(0)); } + + private void validateRemoveResponse(Socket socket, + ProtobufProtocolSerializer protobufProtocolSerializer) throws Exception { + ClientProtocol.Message message = + protobufProtocolSerializer.deserialize(socket.getInputStream()); + assertEquals(TEST_REMOVE_CORRELATION_ID, message.getMessageHeader().getCorrelationId()); + assertEquals(ClientProtocol.Message.MessageTypeCase.RESPONSE, message.getMessageTypeCase()); + ClientProtocol.Response response = message.getResponse(); + assertEquals(ClientProtocol.Response.ResponseAPICase.REMOVERESPONSE, + response.getResponseAPICase()); + } } http://git-wip-us.apache.org/repos/asf/geode/blob/aef89129/geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/RemoveRequestOperationHandlerJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/RemoveRequestOperationHandlerJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/RemoveRequestOperationHandlerJUnitTest.java new file mode 100644 index 0000000..c66d2f2 --- /dev/null +++ b/geode-protobuf/src/test/java/org/apache/geode/protocol/protobuf/operations/RemoveRequestOperationHandlerJUnitTest.java @@ -0,0 +1,142 @@ +/* + * 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.protocol.protobuf.operations; + +import org.apache.geode.cache.Cache; +import org.apache.geode.cache.Region; +import org.apache.geode.protocol.protobuf.BasicTypes; +import org.apache.geode.protocol.protobuf.ClientProtocol; +import org.apache.geode.protocol.protobuf.RegionAPI; +import org.apache.geode.protocol.protobuf.utilities.ProtobufRequestUtilities; +import org.apache.geode.protocol.protobuf.utilities.ProtobufUtilities; +import org.apache.geode.serialization.SerializationService; +import org.apache.geode.serialization.codec.StringCodec; +import org.apache.geode.serialization.exception.UnsupportedEncodingTypeException; +import org.apache.geode.serialization.registry.exception.CodecAlreadyRegisteredForTypeException; +import org.apache.geode.serialization.registry.exception.CodecNotRegisteredForTypeException; +import org.apache.geode.test.dunit.Assert; +import org.apache.geode.test.junit.categories.UnitTest; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.nio.charset.Charset; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@Category(UnitTest.class) +public class RemoveRequestOperationHandlerJUnitTest { + public static final String TEST_KEY = "my key"; + public static final String TEST_VALUE = "my value"; + public static final String TEST_REGION = "test region"; + public static final String MISSING_REGION = "missing region"; + public static final String MISSING_KEY = "missing key"; + public Cache cacheStub; + public SerializationService serializationServiceStub; + private RemoveRequestOperationHandler operationHandler; + private StringCodec stringDecoder; + private Region regionStub; + + @Before + public void setUp() throws Exception { + serializationServiceStub = mock(SerializationService.class); + when(serializationServiceStub.decode(BasicTypes.EncodingType.STRING, + TEST_KEY.getBytes(Charset.forName("UTF-8")))).thenReturn(TEST_KEY); + when(serializationServiceStub.encode(BasicTypes.EncodingType.STRING, TEST_VALUE)) + .thenReturn(TEST_VALUE.getBytes(Charset.forName("UTF-8"))); + when(serializationServiceStub.encode(BasicTypes.EncodingType.STRING, TEST_KEY)) + .thenReturn(TEST_KEY.getBytes(Charset.forName("UTF-8"))); + when(serializationServiceStub.encode(BasicTypes.EncodingType.STRING, MISSING_KEY)) + .thenReturn(MISSING_KEY.getBytes(Charset.forName("UTF-8"))); + + regionStub = mock(Region.class); + when(regionStub.remove(TEST_KEY)).thenReturn(TEST_VALUE); + when(regionStub.containsKey(TEST_KEY)).thenReturn(true); + when(regionStub.containsKey(MISSING_KEY)).thenReturn(false); + + cacheStub = mock(Cache.class); + when(cacheStub.getRegion(TEST_REGION)).thenReturn(regionStub); + when(cacheStub.getRegion(MISSING_REGION)).thenReturn(null); + operationHandler = new RemoveRequestOperationHandler(); + stringDecoder = new StringCodec(); + } + + @Test + public void processValidKeyRemovesTheEntryAndReturnSuccess() + throws CodecAlreadyRegisteredForTypeException, UnsupportedEncodingTypeException, + CodecNotRegisteredForTypeException { + ClientProtocol.Request removeRequest = generateTestRequest(false, false); + ClientProtocol.Response response = + operationHandler.process(serializationServiceStub, removeRequest, cacheStub); + + Assert.assertEquals(ClientProtocol.Response.ResponseAPICase.REMOVERESPONSE, + response.getResponseAPICase()); + RegionAPI.RemoveResponse removeResponse = response.getRemoveResponse(); + verify(regionStub).remove(TEST_KEY); + } + + @Test + public void processReturnsUnsucessfulResponseForInvalidRegion() + throws CodecAlreadyRegisteredForTypeException, UnsupportedEncodingTypeException, + CodecNotRegisteredForTypeException { + ClientProtocol.Request removeRequest = generateTestRequest(true, false); + ClientProtocol.Response response = + operationHandler.process(serializationServiceStub, removeRequest, cacheStub); + + Assert.assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE, + response.getResponseAPICase()); + } + + @Test + public void processReturnsKeyNotFoundWhenKeyIsNotFound() + throws CodecAlreadyRegisteredForTypeException, UnsupportedEncodingTypeException, + CodecNotRegisteredForTypeException { + ClientProtocol.Request removeRequest = generateTestRequest(false, true); + ClientProtocol.Response response = + operationHandler.process(serializationServiceStub, removeRequest, cacheStub); + + Assert.assertEquals(ClientProtocol.Response.ResponseAPICase.REMOVERESPONSE, + response.getResponseAPICase()); + RegionAPI.RemoveResponse removeResponse = response.getRemoveResponse(); + } + + @Test + public void processReturnsErrorWhenUnableToDecodeRequest() + throws CodecAlreadyRegisteredForTypeException, UnsupportedEncodingTypeException, + CodecNotRegisteredForTypeException { + CodecNotRegisteredForTypeException exception = + new CodecNotRegisteredForTypeException("error finding codec for type"); + when(serializationServiceStub.decode(BasicTypes.EncodingType.STRING, + TEST_KEY.getBytes(Charset.forName("UTF-8")))).thenThrow(exception); + + ClientProtocol.Request removeRequest = generateTestRequest(false, false); + ClientProtocol.Response response = + operationHandler.process(serializationServiceStub, removeRequest, cacheStub); + + Assert.assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE, + response.getResponseAPICase()); + } + + private ClientProtocol.Request generateTestRequest(boolean missingRegion, boolean missingKey) + throws UnsupportedEncodingTypeException, CodecNotRegisteredForTypeException { + String region = missingRegion ? MISSING_REGION : TEST_REGION; + String key = missingKey ? MISSING_KEY : TEST_KEY; + BasicTypes.EncodedValue testKey = + ProtobufUtilities.createEncodedValue(serializationServiceStub, key); + return ProtobufRequestUtilities.createRemoveRequest(region, testKey); + } +}
