Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue 913修复 #917

Merged
merged 2 commits into from
Nov 10, 2022
Merged

issue 913修复 #917

merged 2 commits into from
Nov 10, 2022

Conversation

jiangqiang1996
Copy link
Contributor

新增bug修复以及对应测试类。修复集合不可变时反序列化出错的问题。

#913

@wenshao
Copy link
Member

wenshao commented Nov 9, 2022

单测没跑过,你需要处理下

@jiangqiang1996
Copy link
Contributor Author

单测没跑过,你需要处理下

貌似有个地方的问题没办法从根本上解决,例如TypeUtils这个类的写法。按照之前的写法,根据className获取它的类型。示例代码:
image

但是由于jdk高版本新增了一些内置的集合类型,然而fastjson2本身是基于jdk8开发,没办法通过代码获取到高版本具体的Class,也就是我图中圈中的部分,没办法通过java代码去获取。因此,我这里的处理是根据字符串className反射得到Class。如果反射获取失败,则统一返回了 Collections.unmodifiableList(new ArrayList<>()).getClass();

        if (className.startsWith("java.util.ImmutableCollections$")) {
            try {
                return Class.forName(className);
            } catch (ClassNotFoundException e) {
                return CLASS_UNMODIFIABLE_LIST;
            }
        }

下面图中的所有类都是不可变的,新版本新增的,没办法一一判读,所以直接用了上面的方式。
image
image

所以fastjson2在运行于jdk9以上版本时,对于集合的处理总会出现些不可预期的类型变化。

@jiangqiang1996
Copy link
Contributor Author

com.alibaba.fastjson2.issues.Issue312.java
这个类我用之前的版本 单元测试貌似也跑不过。

@jiangqiang1996
Copy link
Contributor Author

image
下面是我自己项目写了个测试,输出结果和上面测试结果一致。
image

@codecov-commenter
Copy link

Codecov Report

Base: 70.08% // Head: 71.71% // Increases project coverage by +1.63% 🎉

Coverage data is based on head (4165b43) compared to base (8767345).
Patch coverage: 23.07% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #917      +/-   ##
============================================
+ Coverage     70.08%   71.71%   +1.63%     
- Complexity    14990    15376     +386     
============================================
  Files           664      664              
  Lines         67155    67677     +522     
  Branches      14063    14194     +131     
============================================
+ Hits          47063    48535    +1472     
+ Misses        14408    13418     -990     
- Partials       5684     5724      +40     
Impacted Files Coverage Δ
...on2/reader/FieldReaderCollectionFieldReadOnly.java 80.76% <0.00%> (-6.74%) ⬇️
...n2/reader/FieldReaderCollectionMethodReadOnly.java 71.69% <0.00%> (-2.82%) ⬇️
...ain/java/com/alibaba/fastjson2/util/TypeUtils.java 87.51% <42.85%> (-0.43%) ⬇️
...tible/src/main/java/com/alibaba/fastjson/JSON.java 64.03% <0.00%> (-7.12%) ⬇️
.../src/main/java/com/alibaba/fastjson/JSONArray.java 82.24% <0.00%> (-1.52%) ⬇️
...n/java/com/alibaba/fastjson2/JSONWriterPretty.java 93.07% <0.00%> (-0.73%) ⬇️
...in/java/com/alibaba/fastjson2/JSONWriterUTF16.java 79.91% <0.00%> (-0.71%) ⬇️
...alibaba/fastjson2/reader/ObjectReaderProvider.java 75.76% <0.00%> (-0.43%) ⬇️
...m/alibaba/fastjson2/internal/asm/MethodWriter.java 89.39% <0.00%> (-0.18%) ⬇️
...m/alibaba/fastjson2/benchmark/LargeFile2MTest.java 0.00% <0.00%> (ø)
... and 46 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@wenshao wenshao merged commit 124809c into alibaba:main Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants