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 #553] Feat: Integrate RocketMQ 5.0 client with Spring #554

Merged
merged 13 commits into from
Jul 30, 2023

Conversation

1294566108
Copy link
Contributor

What is the purpose of the change

RocketMQ recently released the 5.0 client SDK. At present, we have integrated the 4.x version of SDK to Spring, so we also need to integrate the 5.0 SDK client to Spring to better and faster use the new version of the client's Spring integration.

Brief changelog

  1. Add New annotations support the producers and consumers of the 5.0 SDK client
  2. Provides compatibility methods for the Spring Messaging interface
  3. Add message container template for message sending and receiving

@1294566108 1294566108 changed the title [ISSU #553] Feat: Integrate RocketMQ 5.0 client with Spring [ISSUE #553] Feat: Integrate RocketMQ 5.0 client with Spring Apr 23, 2023
@aaron-ai
Copy link
Member

aaron-ai commented May 8, 2023

@1294566108 Fantastic work!

Meanwhile, could you fix the CI issue?

@1294566108
Copy link
Contributor Author

1294566108 commented May 8, 2023

Okay, I just uploaded the code again. It seems that the local compilation seems to have passed

image

Copy link
Member

@aaron-ai aaron-ai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide more documentation or code examples to help us understand how to utilize the 5.x client?

@1294566108
Copy link
Contributor Author

Okay, I will prepare a User Guide and Readme documentation first

Comment on lines 3 to 5
/**
* @author Akai
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz remove the author info.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that this file is unnecessary.

pom.xml Outdated
Comment on lines 218 to 220
<module>rocketmq-client-spring-boot</module>
<module>rocketmq-client-spring-boot-parent</module>
<module>rocketmq-client-spring-boot-starter</module>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the name of the module cannot be well distinguished from the original remoting client. Maybe rename o rocketmq-grpc-client-spring-boot?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a possibility that we don't emphasize gRPC, since the artifact id of 5.x SDK doesn't contain the keyword "gRPC" either?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this UserGuide need to be presented to users like a wiki document? If so, it is necessary to consider writing a bilingual version of the document like this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this UserGuide need to be presented to users like a wiki document? If so, it is necessary to consider writing a bilingual version of the document like this.

Yes, I think we also need a user guide.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a possibility that we don't emphasize gRPC, since the artifact id of 5.x SDK doesn't contain the keyword "gRPC" either?

Or rocketmq-v5-client-spring-boot?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RongtongJin I agree with you.

@1294566108
Copy link
Contributor Author

May 27, 2023

The recent additions and modifications are as follows

  1. Add README-CN.md as a User Guide
  2. Add the test code of the ACL authority control module
  3. Modify the asynchronous sending method


### 修改application.properties

**rocketmq.producer.topic:**用于给生产者设置topic名称(可选,但建议使用),生产者可以在消息发布之前**预取**topic路由。<br />**demo.rocketmq.normal-topic:**用户自定义消息发送的topic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
这里的加粗似乎有点问题。

Comment on lines 255 to 269
void testSendNormalMessage() {
SendReceipt sendReceipt = rocketMQClientTemplate.syncSendNormalMessage(normalTopic, new UserMessage()
.setId(1).setUserName("name").setUserAge((byte) 3));
System.out.printf("normalSend to topic %s sendReceipt=%s %n", normalTopic, sendReceipt);

sendReceipt = rocketMQClientTemplate.syncSendNormalMessage(normalTopic, "normal message");
System.out.printf("normalSend to topic %s sendReceipt=%s %n", normalTopic, sendReceipt);

sendReceipt = rocketMQClientTemplate.syncSendNormalMessage(normalTopic, "byte message".getBytes(StandardCharsets.UTF_8));
System.out.printf("normalSend to topic %s sendReceipt=%s %n", normalTopic, sendReceipt);

sendReceipt = rocketMQClientTemplate.syncSendNormalMessage(normalTopic, MessageBuilder.
withPayload("test message".getBytes()).build());
System.out.printf("normalSend to topic %s sendReceipt=%s %n", normalTopic, sendReceipt);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part of the example is unnecessary.

pom.xml Outdated
Comment on lines 218 to 220
<module>rocketmq-client-spring-boot</module>
<module>rocketmq-client-spring-boot-parent</module>
<module>rocketmq-client-spring-boot-starter</module>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a possibility that we don't emphasize gRPC, since the artifact id of 5.x SDK doesn't contain the keyword "gRPC" either?

Or rocketmq-v5-client-spring-boot?

@1294566108
Copy link
Contributor Author

June 2nd

The modifications are as follows:

  1. Remove unnecessary code in README-CN.md
  2. Modifying Errors in README-CN.md File to Avoid Error Reporting
  3. Change the module name from rocketmq-client-spring-boot to rocketmq-v5-client-spring-boot

Copy link
Contributor

@RongtongJin RongtongJin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@1294566108 Plz pass the license checker workflow
image

Copy link
Contributor

@RongtongJin RongtongJin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@1294566108 Plz pass the license checker workflow
image

@1294566108
Copy link
Contributor Author

Test the example module and the results are as follows

  • TEST FOR【rocketmq-v5-client-producer-acl-demo】&【rocketmq-v5-client-consume-acl-demo】successfully
    Includeing accessKey and secretKey
    image
    image

  • TEST FOR 【rocketmq-v5-client-producer-demo】&【rocketmq-v5-client-consume-demo】successfully
    Including normal、fifo、delay、transaction message
    image

image

  • TEST FOR asyc message's sending and receiving successfully
    Including Asynchronous sending and receiving
    image
    image

@1294566108
Copy link
Contributor Author

May I ask if the current testing coverage is sufficient and if there are any additional test cases that need to be added

@1294566108 1294566108 requested a review from aaron-ai July 15, 2023 02:33
@RongtongJin RongtongJin merged commit b5a43b2 into apache:master Jul 30, 2023
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.

3 participants