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 #103]Support resolvePlaceholders for selectorExpression #129

Merged
merged 8 commits into from
Oct 31, 2019

Conversation

liuliuzo
Copy link
Contributor

@RocketMQMessageListener(topic =
"${demo.rocketmq.test.consumer.topic}",selectorExpression="${demo.rocketmq.test.consumer.tags}"
,consumerGroup = "${demo.rocketmq.test.consumer.group}")

What is the purpose of the change

purpose to support resolvePlaceholders for selectorExpression

Brief changelog

modify ListenerContainerConfiguration.java
modify DefaultRocketMQListenerContainer.java

liuliu added 2 commits August 22, 2019 21:32
@RocketMQMessageListener(topic =
"${demo.rocketmq.test.consumer.topic}",selectorExpression="${demo.rocketmq.test.consumer.tags}"
,consumerGroup = "${demo.rocketmq.test.consumer.group}")
@ShannonDing ShannonDing added the enhancement New feature or request label Aug 30, 2019
@ShannonDing
Copy link
Member

Ref ISSUE #103

@RongtongJin
Copy link
Contributor

I tried your modify, but it do not work. Because when container.setRocketMQMessageListener was called, the parsed selectorExpression was overwritten.

@RongtongJin
Copy link
Contributor

@liuliuzo Maybe you should delete the code this.selectorExpression = anno.selectorExpression(); in setRocketMQMessageListener method.

@liuliuzo
Copy link
Contributor Author

liuliuzo commented Sep 27, 2019

@RongtongJin sorry i will build environment and adding test before submit next time. i am not delete this.selectorExpression = anno.selectorExpression(); in setRocketMQMessageListener method. because selectorExpression should have default value '*', other idea is change container.setRocketMQMessageListener(annotation); order in createRocketMQListenerContainer

@vongosling
Copy link
Member

@liuliuzo It would be helpful to verify if you could add some test in your polish. I would like to follow up and help to merge this pr:-)

@liuliuzo
Copy link
Contributor Author

@vongosling sorry,i will better improve my pull request quality next time.

@vongosling vongosling added this to the 2.0.4 milestone Oct 28, 2019
@@ -272,5 +293,24 @@ public RocketMQTemplate extRocketMQTemplate() {
static class MyExtRocketMQTemplate extends RocketMQTemplate {
Copy link
Member

Choose a reason for hiding this comment

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

Could you modify the class MyExtRocketMQTemplate name, you could assign a concrete name Instead of a name with no actual meaning my**?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done : )

@vongosling vongosling changed the title [ISSUE #103] support resolvePlaceholders for selectorExpression [ISSUE #103]Support resolvePlaceholders for selectorExpression Oct 28, 2019
16534 added 2 commits October 28, 2019 16:39
…estyle

and update test class with actual meaning.
…estyle

and update test class with actual meaning.
@duhenglucky duhenglucky merged commit 71a1e9c into apache:master Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants