[ISSUE #9396] Use fastjson2 in all modules#9397
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #9397 +/- ##
=============================================
+ Coverage 48.41% 49.67% +1.25%
- Complexity 12292 12695 +403
=============================================
Files 1315 1315
Lines 93918 93931 +13
Branches 12046 12049 +3
=============================================
+ Hits 45475 46662 +1187
+ Misses 42836 41556 -1280
- Partials 5607 5713 +106 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
1、Could you provide the performance benchmark data for the original Fastjson 1? |
Thank you for your review. The data for the two questions are as follows: 
Regarding question 2, I encountered the camel case problem of getCId during the replacement test. The solution is to move the @JSONField(name = "c") annotation from the attribute to the get/set method. All the replaced codes are covered by unit tests. |
|
Could you develop a tool to validate whether objects with camelCase properties are compatible with Fastjson 2, so we can check compatibility? @yx9o |
Let me understand. We need to develop a tool to test all objects in the project that have irregular camel case naming to ensure that fastjson1 and fastjson2 serialization are compatible, right? |
Yes, but we should only validate classes involved in JSON serialization/deserialization. |
|
Hi @fuyou001 , please help review again. According to your review, I added the following serialization compatibility test tool class:
|
When running RemotingSerializableCompatTest, I encountered failed classes. Here's the output for your review: |
|
@yx9o Subclasses of ConfigManager must also be checked |
Hello, I received your response. I have tested this class separately in the current branch and the develop branch. No errors occur. There is no "failed: xx" error output in the class itself. May I ask how you run this test? |
ConfigManager's subclasses are in multiple different packages and cannot be tested uniformly using tool classes. I want to do fastjson1 serialization and fastjson2 deserialization compatibility verification in each subclass's Test. Is this OK? |
RemotingSerializableCompatTest.testCompatibilityCheck method , I change :
|
I changed it like this and there was no error. Is there a difference in the environment? May I ask what branch, platform and jdk you are running on? |
This PR branch and uses JDK 11. There are many issues concerning compatibility reported at https://github.com/alibaba/fastjson2/issues. Be extremely cautious Upgrading to fastjson2 involves high risks |
I have passed the debugging under jdk11, please help review it again, thank you. |
|
@yx9o The conflicts need to be handled |
# Conflicts: # broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java # broker/src/main/java/org/apache/rocketmq/broker/topic/TopicQueueMappingManager.java # broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java # broker/src/test/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManagerTest.java
# Conflicts: # broker/src/main/java/org/apache/rocketmq/broker/processor/PopMessageProcessor.java # broker/src/main/java/org/apache/rocketmq/broker/transaction/TransactionMetrics.java # broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java # store/src/main/java/org/apache/rocketmq/store/timer/TimerMetrics.java
Corrected, please review and verify again, thank you. |
|
@yx9o Some problems were encountered during testing yet com.alibaba.fastjson2.JSONException: not support input EA==, offset 22, character ", line 1, column 23, fastjson-version 2.0.43 {"acks":[{"b":"EA==","c":"gid-72","it":60000,"pt":1758617828232,"q":0,"r":"0","rq":4,"so":245397,"t":"topic-a72"}],"brokerName":"rocketmq-broker-rmq-cn-htq4fg13h09-0"} |
# Conflicts: # broker/src/test/java/org/apache/rocketmq/broker/processor/ChangeInvisibleTimeProcessorTest.java
The adjustment has been completed, please review again, thank you. |
|
希望增加bitset的兼容性测试，在目前的测试中该字段是null，可以增加模拟值去比较兼容性。 |
Added, please review again, thank you. |
# Conflicts: # broker/src/main/java/org/apache/rocketmq/broker/config/v1/RocksDBConfigManager.java # broker/src/main/java/org/apache/rocketmq/broker/config/v1/RocksDBConsumerOffsetManager.java # broker/src/main/java/org/apache/rocketmq/broker/config/v1/RocksDBSubscriptionGroupManager.java # broker/src/main/java/org/apache/rocketmq/broker/config/v1/RocksDBTopicConfigManager.java # broker/src/main/java/org/apache/rocketmq/broker/processor/PopMessageProcessor.java
# Conflicts: # broker/src/main/java/org/apache/rocketmq/broker/pop/orderly/QueueLevelConsumerManager.java # broker/src/main/java/org/apache/rocketmq/broker/processor/AckMessageProcessor.java # broker/src/main/java/org/apache/rocketmq/broker/processor/ChangeInvisibleTimeProcessor.java
5 participants
Fixes #9396 .