修改请求参数转换逻辑 #1347

miaomiaoLmm · 2023-07-17T07:53:20Z

修改请求参数转换逻辑:
问题：rpc请求序列化异常,rpc请求进行request参数序列化，由于子类没有重写apply方法，导致从父类获取apply方法中的request和response类型，由于类型定义为范型，request参数序列化过程会自动转换为linkedHashMap类型并请求apply方法，由于apply方法中调用的process方法确定了实际的request类型，导致linkedHashMap转换Request类型异常

interface A<T,R>{
  R apply(T t);
}

public abstract class B<T,R> implements A<T,R>{
   @Override
    public R apply(T request) {
        return this.process(request);
    }

protected abstract R process(T request);
}      

class C extends B<Request,Response>{
    @Override
    protected Response process(Request request) {
        logger.info("Hello FaaS !");
        Response resp = new Response();
        resp.setUrl(request.getUrl());
        return resp;
    }

}

}

解决方案：判断重写apply方法的class类是否为目标类，如果不是目标类，通过获取目标类上确定的Request和Response类型进行请求参数序列化。

sofastack-cla · 2023-07-17T07:54:26Z

Hi @miaomiaoLmm, welcome to SOFAStack community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

sofastack-cla · 2023-07-17T08:01:29Z

Hi @miaomiaoLmm, welcome to SOFAStack community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

codecov · 2023-07-20T01:34:52Z

Codecov Report

Merging #1347 (d4489eb) into master (8ca4f28) will decrease coverage by 0.01%.
Report is 4 commits behind head on master.
The diff coverage is 88.88%.

❗ Current head d4489eb differs from pull request most recent head c37390e. Consider uploading reports for the commit c37390e to get more accurate results

@@             Coverage Diff              @@
##             master    #1347      +/-   ##
============================================
- Coverage     72.05%   72.05%   -0.01%     
- Complexity      783      784       +1     
============================================
  Files           416      416              
  Lines         17661    17668       +7     
  Branches       2752     2753       +1     
============================================
+ Hits          12726    12730       +4     
  Misses         3533     3533              
- Partials       1402     1405       +3

Files Changed	Coverage Δ
...m/alipay/sofa/rpc/codec/jackson/JacksonHelper.java	`84.78% <88.88%> (+0.16%)`	⬆️

... and 2 files with indirect coverage changes

EvenLjj · 2023-07-20T16:18:58Z

codec/codec-jackson/src/main/java/com/alipay/sofa/rpc/codec/jackson/JacksonHelper.java

        if (jsonMethod == null) {
            throw new SofaRpcRuntimeException(LogCodes.getLog(LogCodes.ERROR_METHOD_NOT_FOUND, clazz.getName(),
                methodName));
        }

-        // parse request types
-        Type[] parameterTypes = jsonMethod.getGenericParameterTypes();
+        Type[] parameterTypes;


@miaomiaoLmm Please add a single test to ensure problem resolution.

OrezzerO · 2023-07-27T03:19:16Z

codec/codec-jackson/src/test/java/com/alipay/sofa/rpc/codec/jackson/JacksonSerializerTest.java

+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.*;


Please don't use * in import statements. You can set it in IDE configuration.

New pull request I removed util.*

Lo1nt · 2023-08-01T09:43:57Z

pls do a 'mvn clean package -Dmaven.test.skip=true' before u commit and push

Lo1nt · 2023-08-04T03:04:57Z

codec/codec-jackson/src/main/java/com/alipay/sofa/rpc/codec/jackson/JacksonHelper.java

+        if (jsonMethod.getDeclaringClass() != clazz && !clazz.isInterface()) {
+            parameterTypes = new Type[1];
+            Type type = clazz.getGenericSuperclass();
+            ParameterizedType parameterizedType = (ParameterizedType) type;


What if clazz.getGenericSuperclass() returns type which is not ParameterizedType?

Say if Class c extends B implement A, B may not always be a ParameterizedType.

The new pull request fixes the parent object non-ParameterizedType conversion problem

… father object/super object definition apply method, and definition requestType and responseType, type change fixed success 2. The TestRequest2 method is provided to test the problem of parent class non-ParameterizedType conversion

Lo1nt · 2023-08-07T08:00:55Z

codec/codec-jackson/src/main/java/com/alipay/sofa/rpc/codec/jackson/JacksonHelper.java

+            try {
+                ParameterizedType parameterizedType = (ParameterizedType) type;
+                // parse request types
+                parameterTypes[0] = parameterizedType.getActualTypeArguments()[0];


这里感觉还是有问题的，应该无法保证所有走到这里的case都是 Class A<X, Y> 的形式？ Class A<X, Y, Z> 或者 Class A<X> 应该都会触发异常

Lo1nt · 2023-08-07T08:01:46Z

codec/codec-jackson/src/main/java/com/alipay/sofa/rpc/codec/jackson/JacksonHelper.java

+        if (jsonMethod.getDeclaringClass() != clazz && !clazz.isInterface()) {
+            parameterTypes = new Type[1];
+            Type type = clazz.getGenericSuperclass();
+            try {


这里可以用if else来处理，尽量不要使用 try catch 当 ifelse 用

stale · 2023-10-09T20:34:00Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

修改请求参数转换逻辑

8ecc033

sofastack-cla bot added cla:no Need sign CLA First-time contributor First-time contributor size/S labels Jul 17, 2023

miaomiaoLmm closed this Jul 17, 2023

miaomiaoLmm reopened this Jul 17, 2023

miaomiaoLmm marked this pull request as draft July 17, 2023 08:03

sofastack-cla bot added cla:yes CLA is ok and removed cla:no Need sign CLA labels Jul 17, 2023

EvenLjj marked this pull request as ready for review July 20, 2023 16:13

EvenLjj reviewed Jul 20, 2023

View reviewed changes

新增测试用例

15640da

sofastack-cla bot added size/L and removed size/S labels Jul 27, 2023

新增测试用例

a2488a1

OrezzerO reviewed Jul 27, 2023

View reviewed changes

cancel util.*

81d9074

EvenLjj added this to the 5.11.0 milestone Jul 28, 2023

淼淼 added 2 commits August 2, 2023 17:10

Merge branch 'master' into sofa-rpc-obj-convert-repair

3bb1706

Unit test after submitting 'mvn clean package -Dmaven.test.skip=true'

d4489eb

Lo1nt reviewed Aug 4, 2023

View reviewed changes

淼淼 added 3 commits August 4, 2023 15:59

Lo1nt requested changes Aug 7, 2023

View reviewed changes

修改类型判断处理，取消try catch 使用if else判断

c37390e

stale bot added the wontfix This will not be worked on label Oct 9, 2023

stale bot closed this Oct 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

修改请求参数转换逻辑 #1347

修改请求参数转换逻辑 #1347

miaomiaoLmm commented Jul 17, 2023 •

edited by zhenjunMa

Loading

sofastack-cla bot commented Jul 17, 2023

sofastack-cla bot commented Jul 17, 2023

codecov bot commented Jul 20, 2023 •

edited

Loading

EvenLjj Jul 20, 2023

OrezzerO Jul 27, 2023

miaomiaoLmm Jul 27, 2023

Lo1nt commented Aug 1, 2023

Lo1nt Aug 4, 2023 •

edited

Loading

382235471 Aug 4, 2023

Lo1nt Aug 7, 2023 •

edited

Loading

Lo1nt Aug 7, 2023

stale bot commented Oct 9, 2023

修改请求参数转换逻辑 #1347

修改请求参数转换逻辑 #1347

Conversation

miaomiaoLmm commented Jul 17, 2023 • edited by zhenjunMa Loading

sofastack-cla bot commented Jul 17, 2023

sofastack-cla bot commented Jul 17, 2023

codecov bot commented Jul 20, 2023 • edited Loading

Codecov Report

EvenLjj Jul 20, 2023

Choose a reason for hiding this comment

OrezzerO Jul 27, 2023

Choose a reason for hiding this comment

miaomiaoLmm Jul 27, 2023

Choose a reason for hiding this comment

Lo1nt commented Aug 1, 2023

Lo1nt Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

382235471 Aug 4, 2023

Choose a reason for hiding this comment

Lo1nt Aug 7, 2023 • edited Loading

Choose a reason for hiding this comment

Lo1nt Aug 7, 2023

Choose a reason for hiding this comment

stale bot commented Oct 9, 2023

miaomiaoLmm commented Jul 17, 2023 •

edited by zhenjunMa

Loading

codecov bot commented Jul 20, 2023 •

edited

Loading

Lo1nt Aug 4, 2023 •

edited

Loading

Lo1nt Aug 7, 2023 •

edited

Loading