----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60217/#review178410 -----------------------------------------------------------
geode-protobuf/src/main/java/org/apache/geode/ProtobufUtilities.java Lines 22 (patched) <https://reviews.apache.org/r/60217/#comment252368> The new classes are all missing javadocs geode-protobuf/src/main/java/org/apache/geode/protocol/OpsProcessor.java Lines 45 (patched) <https://reviews.apache.org/r/60217/#comment252353> Create a Logger and use it to log a stack trace with information about the context of the exception. private static final Logger logger = LogService.getLogger(); geode-protobuf/src/main/java/org/apache/geode/protocol/operations/OperationHandler.java Lines 26 (patched) <https://reviews.apache.org/r/60217/#comment252378> I think all interface methods should have javadocs geode-protobuf/src/main/java/org/apache/geode/protocol/operations/ProtobufRequestOperationParser.java Lines 29 (patched) <https://reviews.apache.org/r/60217/#comment252377> Maybe InternalGemFireException would be better than RuntimeException here. geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationService.java Lines 22 (patched) <https://reviews.apache.org/r/60217/#comment252379> method javadocs please geode-protobuf/src/main/java/org/apache/geode/serialization/registry/SerializationCodecRegistry.java Lines 29 (patched) <https://reviews.apache.org/r/60217/#comment252380> Since the collection of codecs is fixed by the enumeration why are you using a ServiceLoader to scan the classpath for annotated codecs? That seems inefficient. I think this should be removed until such time as the set of codecs is actually extensible. It will be a lot faster to just have a table of the supported codec classes and iterate over it. - Bruce Schuchardt On June 20, 2017, 12:17 a.m., Brian Rowe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60217/ > ----------------------------------------------------------- > > (Updated June 20, 2017, 12:17 a.m.) > > > Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen > O'Sullivan, Hitesh Khamesra, and Udo Kohlmeyer. > > > Repository: geode > > > Description > ------- > > This change adds a new module for handling client stresms encoded using the > new ProtoBuf protocol. At the top level this can be integrated by passing in > the input/output streams and cache reference to the ProtobufStreamProcessor. > This will decode the message and ultimately dispatch it to an operation > specific handler which will call back into the passed cache object. Note > that this not currently hooked up to geode at all, GEODE-3075 is tracking the > work needed to hook this up at that level. > > This change mainly contains the plumbing and encoding/decoding logic, but > also contain the Get operation handler as a proof of concept. Future work > will be needed to handle other types of operations. > > > Diffs > ----- > > geode-protobuf/build.gradle PRE-CREATION > geode-protobuf/src/main/java/org/apache/geode/ProtobufUtilities.java > PRE-CREATION > geode-protobuf/src/main/java/org/apache/geode/protocol/OpsProcessor.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/exception/InvalidProtocolMessageException.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/handler/ProtobufStreamProcessor.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/handler/ProtocolHandler.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/handler/protobuf/ProtobufProtocolHandler.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/operations/OperationHandler.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/operations/ProtobufRequestOperationParser.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/operations/protobuf/GetRequestOperationHandler.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/OperationsHandlerRegistry.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerAlreadyRegisteredException.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/protocol/operations/registry/exception/OperationHandlerNotRegisteredException.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/ProtobufSerializationService.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationService.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/SerializationType.java > PRE-CREATION > geode-protobuf/src/main/java/org/apache/geode/serialization/TypeCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/BinaryCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/BooleanCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/ByteCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/DoubleCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/FloatCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/IntCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/JSONCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/LongCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/ShortCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/codec/StringCodec.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/exception/UnsupportedEncodingTypeException.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/protobuf/translation/EncodingTypeTranslator.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/protobuf/translation/exception/UnsupportedEncodingTypeException.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/registry/SerializationCodecRegistry.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/registry/exception/CodecAlreadyRegisteredForTypeException.java > PRE-CREATION > > geode-protobuf/src/main/java/org/apache/geode/serialization/registry/exception/CodecNotRegisteredForTypeException.java > PRE-CREATION > geode-protobuf/src/main/proto/basicTypes.proto PRE-CREATION > geode-protobuf/src/main/proto/clientProtocol.proto PRE-CREATION > geode-protobuf/src/main/proto/region_API.proto PRE-CREATION > geode-protobuf/src/main/proto/server_API.proto PRE-CREATION > > geode-protobuf/src/main/resources/META-INF/services/org.apache.geode.protocol.handler.ProtocolHandler > PRE-CREATION > > geode-protobuf/src/main/resources/META-INF/services/org.apache.geode.protocol.operations.OperationHandler > PRE-CREATION > > geode-protobuf/src/main/resources/META-INF/services/org.apache.geode.serialization.TypeCodec > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/client/protocol/IntegrationTest.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/client/protocol/MessageUtil.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/client/protocol/OpsHandler.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/client/protocol/OpsProcessorTest.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/protocol/handler/ProtobufProtocolHandlerJUnitTest.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/protocol/operations/protobuf/GetRequestOperationHandlerTest.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/protocol/operations/registry/OperationsHandlerRegistryJUnitTest.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/serialization/ProtobufSerializationServiceImplTest.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/serialization/codec/StringCodecJUnitTest.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/serialization/protobuf/translation/EncodingTypeToSerializationTypeTranslatorJUnitTest.java > PRE-CREATION > > geode-protobuf/src/test/java/org/apache/geode/serialization/registry/CodecRegistryJUnitTest.java > PRE-CREATION > gradle/rat.gradle 1bea5843b > settings.gradle c0fdb6e4f > > > Diff: https://reviews.apache.org/r/60217/diff/1/ > > > Testing > ------- > > Precheckin in progress. Unit tests added for all new classes, integration > test added for entire module. > > > Thanks, > > Brian Rowe > >