-----------------------------------------------------------
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
> 
>

Reply via email to