szetszwo commented on code in PR #9733:
URL: https://github.com/apache/ozone/pull/9733#discussion_r2805369721
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/GeneratedMessageCodec.java:
##########
@@ -17,34 +17,35 @@
package org.apache.hadoop.hdds.scm.ha.io;
-import com.google.protobuf.ByteString;
-import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.Message;
import java.lang.reflect.InvocationTargetException;
import org.apache.hadoop.hdds.scm.ha.ReflectionUtil;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import
org.apache.ratis.thirdparty.com.google.protobuf.InvalidProtocolBufferException;
/**
- * {@link Codec} for {@link Message} objects.
+ * {@link Codec} implementation for non-shaded
+ * {@link com.google.protobuf.Message} objects.
*/
public class GeneratedMessageCodec implements Codec {
@Override
- public ByteString serialize(Object object) {
- return ((Message)object).toByteString();
+ public ByteString serialize(Object object)
+ throws
org.apache.ratis.thirdparty.com.google.protobuf.InvalidProtocolBufferException {
+ final byte[] bytes = ((Message) object).toByteString().toByteArray();
+ return ByteString.copyFrom(bytes);
Review Comment:
Use unsafeWarp as below?
```java
UnsafeByteOperations.unsafeWrap(((Message)
object).toByteString().asReadOnlyByteBuffer());
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/GeneratedMessageCodec.java:
##########
@@ -17,34 +17,35 @@
package org.apache.hadoop.hdds.scm.ha.io;
-import com.google.protobuf.ByteString;
-import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.Message;
import java.lang.reflect.InvocationTargetException;
import org.apache.hadoop.hdds.scm.ha.ReflectionUtil;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import
org.apache.ratis.thirdparty.com.google.protobuf.InvalidProtocolBufferException;
/**
- * {@link Codec} for {@link Message} objects.
+ * {@link Codec} implementation for non-shaded
+ * {@link com.google.protobuf.Message} objects.
*/
public class GeneratedMessageCodec implements Codec {
@Override
- public ByteString serialize(Object object) {
- return ((Message)object).toByteString();
+ public ByteString serialize(Object object)
+ throws
org.apache.ratis.thirdparty.com.google.protobuf.InvalidProtocolBufferException {
+ final byte[] bytes = ((Message) object).toByteString().toByteArray();
+ return ByteString.copyFrom(bytes);
}
@Override
- public Message deserialize(Class<?> type, ByteString value)
- throws InvalidProtocolBufferException {
+ public Object deserialize(Class<?> type, ByteString value)
+ throws
org.apache.ratis.thirdparty.com.google.protobuf.InvalidProtocolBufferException {
Review Comment:
Keep `throws InvalidProtocolBufferException` since the import is changed.
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMRatisRequest.java:
##########
@@ -173,8 +173,8 @@ public void testDecodeMissingArgumentTypeShouldFail()
throws Exception {
.build();
Message msg = Message.valueOf(
- org.apache.ratis.thirdparty.com.google.protobuf.ByteString.copyFrom(
- proto.toByteArray()));
+ org.apache.ratis.thirdparty.com.google.protobuf.UnsafeByteOperations
Review Comment:
import UnsafeByteOperations.
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMRatisProtocolCompatibility.java:
##########
@@ -44,66 +46,35 @@ static <T> ByteString randomValue(Class<T> clazz) {
}
final String string = builder.toString();
assertEquals(length, string.length());
- return ByteString.copyFromUtf8(string);
+ return
org.apache.ratis.thirdparty.com.google.protobuf.ByteString.copyFromUtf8(string);
} else if (clazz == Integer.class) {
final ByteBuffer buffer = ByteBuffer.allocate(4);
buffer.putInt(RANDOM.nextInt());
- return ByteString.copyFrom(buffer.array());
+ return
org.apache.ratis.thirdparty.com.google.protobuf.ByteString.copyFrom(buffer.array());
} else if (clazz == byte[].class) {
final byte[] bytes = new byte[RANDOM.nextInt(3)];
RANDOM.nextBytes(bytes);
- return ByteString.copyFrom(bytes);
+ return
org.apache.ratis.thirdparty.com.google.protobuf.ByteString.copyFrom(bytes);
Review Comment:
ByteString is already imported.
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMRatisProtocolCompatibility.java:
##########
@@ -35,7 +37,7 @@ public class TestSCMRatisProtocolCompatibility {
static final Random RANDOM = new Random();
static final Class<?>[] TYPES = {String.class, Integer.class, byte[].class};
- static <T> ByteString randomValue(Class<T> clazz) {
+ static <T> org.apache.ratis.thirdparty.com.google.protobuf.ByteString
randomValueProto3(Class<T> clazz) {
Review Comment:
ByteString is already imported.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
