From 4d9b6378545a33293999ad31ea3775bb2aa20977 Mon Sep 17 00:00:00 2001 From: lvshichang Date: Sat, 11 Nov 2023 02:37:18 +0800 Subject: [PATCH 1/2] bugfix: Compatible with Protobuf empty object. --- .../rpc/codec/bolt/SofaRpcSerialization.java | 14 +++++++------- .../transport/triple/TripleClientInvoker.java | 18 +++++++----------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/remoting/remoting-bolt/src/main/java/com/alipay/sofa/rpc/codec/bolt/SofaRpcSerialization.java b/remoting/remoting-bolt/src/main/java/com/alipay/sofa/rpc/codec/bolt/SofaRpcSerialization.java index 655c8fd87..c1fc9885d 100644 --- a/remoting/remoting-bolt/src/main/java/com/alipay/sofa/rpc/codec/bolt/SofaRpcSerialization.java +++ b/remoting/remoting-bolt/src/main/java/com/alipay/sofa/rpc/codec/bolt/SofaRpcSerialization.java @@ -269,8 +269,9 @@ public boolean deserializeContent(Request reque long deserializeStartTime = System.nanoTime(); try { byte[] content = requestCommand.getContent(); - if (content == null || content.length == 0) { - throw new DeserializationException("Content of request is null"); + // The content may be empty in protobuf protocol scenario. + if (content == null) { + content = new byte[0]; } String service = headerMap.get(RemotingConstants.HEAD_SERVICE); ClassLoader oldClassLoader = Thread.currentThread().getContextClassLoader(); @@ -282,8 +283,7 @@ public boolean deserializeContent(Request reque Serializer rpcSerializer = com.alipay.sofa.rpc.codec.SerializerFactory .getSerializer(requestCommand.getSerializer()); Object sofaRequest = ClassUtils.forName(requestCommand.getRequestClass()).newInstance(); - rpcSerializer.decode(new ByteArrayWrapperByteBuf(requestCommand.getContent()), - sofaRequest, headerMap); + rpcSerializer.decode(new ByteArrayWrapperByteBuf(content), sofaRequest, headerMap); //for service mesh or other scene, we need to add more info from header setRequestPropertiesWithHeaderInfo(headerMap, sofaRequest); @@ -431,8 +431,8 @@ public boolean deserializeContent(Response re RpcResponseCommand responseCommand = (RpcResponseCommand) response; byte serializer = response.getSerializer(); byte[] content = responseCommand.getContent(); - if (content == null || content.length == 0) { - return false; + if (content == null) { + content = new byte[0]; } long deserializeStartTime = System.nanoTime(); @@ -451,7 +451,7 @@ public boolean deserializeContent(Response re (String) invokeContext.get(RemotingConstants.HEAD_GENERIC_TYPE)); Serializer rpcSerializer = com.alipay.sofa.rpc.codec.SerializerFactory.getSerializer(serializer); - rpcSerializer.decode(new ByteArrayWrapperByteBuf(responseCommand.getContent()), sofaResponse, header); + rpcSerializer.decode(new ByteArrayWrapperByteBuf(content), sofaResponse, header); if (sofaResponse instanceof SofaResponse) { parseResponseHeader(header, (SofaResponse) sofaResponse); } diff --git a/remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java b/remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java index 7079f3f13..d910b524d 100644 --- a/remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java +++ b/remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/transport/triple/TripleClientInvoker.java @@ -137,14 +137,12 @@ public SofaResponse invoke(SofaRequest sofaRequest, int timeout) buildCustomCallOptions(sofaRequest, timeout), request); SofaResponse sofaResponse = new SofaResponse(); - byte[] responseDate = response.getData().toByteArray(); + byte[] responseData = response.getData().toByteArray(); Class returnType = sofaRequest.getMethod().getReturnType(); if (returnType != void.class) { - if (responseDate != null && responseDate.length > 0) { - Serializer responseSerializer = SerializerFactory.getSerializer(response.getSerializeType()); - Object appResponse = responseSerializer.decode(new ByteArrayWrapperByteBuf(responseDate), returnType, null); - sofaResponse.setAppResponse(appResponse); - } + Serializer responseSerializer = SerializerFactory.getSerializer(response.getSerializeType()); + Object appResponse = responseSerializer.decode(new ByteArrayWrapperByteBuf(responseData), returnType, null); + sofaResponse.setAppResponse(appResponse); } return sofaResponse; @@ -250,13 +248,11 @@ private void processSuccess(boolean needDecode, RpcInternalContext context, Sofa Object appResponse = o; if (needDecode) { Response response = (Response) o; - byte[] responseDate = response.getData().toByteArray(); + byte[] responseData = response.getData().toByteArray(); Class returnType = sofaRequest.getMethod().getReturnType(); if (returnType != void.class) { - if (responseDate != null && responseDate.length > 0) { - Serializer responseSerializer = SerializerFactory.getSerializer(response.getSerializeType()); - appResponse = responseSerializer.decode(new ByteArrayWrapperByteBuf(responseDate), returnType, null); - } + Serializer responseSerializer = SerializerFactory.getSerializer(response.getSerializeType()); + appResponse = responseSerializer.decode(new ByteArrayWrapperByteBuf(responseData), returnType, null); } } From 818b533e1aa380875bb5a25812a6483e659d7eac Mon Sep 17 00:00:00 2001 From: lvshichang Date: Sun, 3 Dec 2023 23:40:31 +0800 Subject: [PATCH 2/2] feat: add some unit test. --- bom/pom.xml | 6 ++ pom.xml | 6 +- remoting/remoting-bolt/pom.xml | 11 +++ .../codec/bolt/SofaRpcSerializationTest.java | 96 +++++++++++++++---- 4 files changed, 97 insertions(+), 22 deletions(-) diff --git a/bom/pom.xml b/bom/pom.xml index 5756a04f0..ddd3acede 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -539,6 +539,12 @@ 4.3.0 test + + org.mockito + mockito-inline + 4.3.0 + test + io.netty diff --git a/pom.xml b/pom.xml index 57e7f943e..e4562ae61 100644 --- a/pom.xml +++ b/pom.xml @@ -60,7 +60,11 @@ mockito-core test - + + org.mockito + mockito-inline + test + org.openjdk.jmh jmh-core diff --git a/remoting/remoting-bolt/pom.xml b/remoting/remoting-bolt/pom.xml index 11ca92182..59797e104 100644 --- a/remoting/remoting-bolt/pom.xml +++ b/remoting/remoting-bolt/pom.xml @@ -36,6 +36,12 @@ netty-all + + com.google.protobuf + protobuf-java + test + + com.alipay.sofa sofa-rpc-test-common @@ -52,6 +58,11 @@ junit test + + org.mockito + mockito-inline + test + diff --git a/remoting/remoting-bolt/src/test/java/com/alipay/sofa/rpc/codec/bolt/SofaRpcSerializationTest.java b/remoting/remoting-bolt/src/test/java/com/alipay/sofa/rpc/codec/bolt/SofaRpcSerializationTest.java index 646f12200..179436859 100644 --- a/remoting/remoting-bolt/src/test/java/com/alipay/sofa/rpc/codec/bolt/SofaRpcSerializationTest.java +++ b/remoting/remoting-bolt/src/test/java/com/alipay/sofa/rpc/codec/bolt/SofaRpcSerializationTest.java @@ -16,39 +16,93 @@ */ package com.alipay.sofa.rpc.codec.bolt; +import static org.mockito.ArgumentMatchers.anyByte; + +import java.util.HashMap; +import java.util.Map; + +import org.junit.Assert; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import com.alipay.remoting.InvokeContext; import com.alipay.remoting.exception.DeserializationException; import com.alipay.remoting.exception.SerializationException; -import com.alipay.remoting.rpc.protocol.RpcRequestCommand; import com.alipay.remoting.rpc.protocol.RpcResponseCommand; +import com.alipay.sofa.rpc.codec.Serializer; +import com.alipay.sofa.rpc.codec.SerializerFactory; import com.alipay.sofa.rpc.common.RemotingConstants; import com.alipay.sofa.rpc.context.RpcInternalContext; +import com.alipay.sofa.rpc.core.exception.SofaRpcException; import com.alipay.sofa.rpc.core.request.SofaRequest; -import org.junit.Assert; -import org.junit.Test; -import java.util.HashMap; -import java.util.Map; +import com.alipay.sofa.rpc.transport.AbstractByteBuf; +import com.google.protobuf.Empty; +import com.google.protobuf.Internal; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.MessageLite; public class SofaRpcSerializationTest { + static class SimpleResponse { + public SimpleResponse() { + } + + private Object realResponse; + + public Object getRealResponse() { + return realResponse; + } + + public void setRealResponse(Object realResponse) { + this.realResponse = realResponse; + } + } + + static class SimpleSerializer implements Serializer { + @Override + public AbstractByteBuf encode(Object object, Map context) throws SofaRpcException { + throw new UnsupportedOperationException(object.getClass().getName()); + } + + @Override + public Object decode(AbstractByteBuf data, Class clazz, Map context) throws SofaRpcException { + try { + if (MessageLite.class.isAssignableFrom(clazz)) { + MessageLite defaultInstance = Internal.getDefaultInstance(clazz); + return defaultInstance.getParserForType().parseFrom(data.array()); + } + } catch (InvalidProtocolBufferException e) { + throw new RuntimeException(e); + } + throw new UnsupportedOperationException(); + } + + @Override + public void decode(AbstractByteBuf data, Object template, Map context) throws SofaRpcException { + if (template instanceof SimpleResponse) { + Object realResponse = decode(data, Empty.class, context); + ((SimpleResponse) template).setRealResponse(realResponse); + return; + } + throw new UnsupportedOperationException(); + } + } @Test - public void deserializeRequestContent() { - String traceId = "traceId"; - String rpcId = "rpcId"; - Map headerMap = new HashMap<>(); - headerMap.put("rpc_trace_context.sofaTraceId", traceId); - headerMap.put("rpc_trace_context.sofaRpcId", rpcId); + public void testSerializeEmptyRequest() throws DeserializationException { + SofaRpcSerialization serialization = new SofaRpcSerialization(); - RpcRequestCommand command = new RpcRequestCommand(); - command.setRequestHeader(headerMap); - SofaRpcSerialization sofaRpcSerialization = new SofaRpcSerialization(); - boolean exp = false; - try { - sofaRpcSerialization.deserializeContent(command); - } catch (DeserializationException e) { - exp = true; - Assert.assertEquals("Content of request is null, traceId=" + traceId + ", rpcId=" + rpcId, e.getMessage()); + try (MockedStatic mocked = Mockito.mockStatic(SerializerFactory.class)) { + mocked.when(() -> SerializerFactory.getSerializer(anyByte())).thenReturn(new SimpleSerializer()); + + RpcResponseCommand response = new RpcResponseCommand(); + response.setResponseClass(SimpleResponse.class.getName()); + response.setContent(null); + + Assert.assertTrue(serialization.deserializeContent(response, new InvokeContext())); + SimpleResponse simpleResponse = (SimpleResponse) response.getResponseObject(); + Assert.assertEquals(Empty.getDefaultInstance(), simpleResponse.getRealResponse()); } - Assert.assertTrue(exp); } @Test