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.

Sign up for GitHub

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 #8] Offer a more user-friendly format to configure nameserver #171

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[ISSUE #8] Offer a more user-friendly format to configure nameserver #171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e312ff7
feat(config):user can config name-server with format `host:port,host:…
RongtongJin Nov 14, 2019
3b3364d
fix(config):cancel must configure name server
RongtongJin Nov 15, 2019
bd397ec
chore(config):delete useless code
RongtongJin Nov 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions ...src/main/java/org/apache/rocketmq/spring/autoconfigure/ExtProducerResetConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@

package org.apache.rocketmq.spring.autoconfigure;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.Map;
import java.util.Objects;

import org.apache.rocketmq.acl.common.AclClientRPCHook;
import org.apache.rocketmq.acl.common.SessionCredentials;
import org.apache.rocketmq.client.exception.MQClientException;
import org.apache.rocketmq.client.producer.DefaultMQProducer;
import org.apache.rocketmq.spring.annotation.ExtRocketMQTemplateConfiguration;
import org.apache.rocketmq.spring.core.RocketMQTemplate;
import org.apache.rocketmq.spring.support.RocketMQMessageConverter;
import org.apache.rocketmq.spring.support.RocketMQUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.aop.framework.AopProxyUtils;
Expand All @@ -41,8 +42,6 @@
import org.springframework.core.env.StandardEnvironment;
import org.springframework.util.StringUtils;

import com.fasterxml.jackson.databind.ObjectMapper;

@Configuration
public class ExtProducerResetConfiguration implements ApplicationContextAware, SmartInitializingSingleton {
private final static Logger log = LoggerFactory.getLogger(ExtProducerResetConfiguration.class);
Expand Down Expand Up @@ -152,7 +151,7 @@ private void validate(ExtRocketMQTemplateConfiguration annotation,
}

if (rocketMQProperties.getNameServer() == null ||
rocketMQProperties.getNameServer().equals(environment.resolvePlaceholders(annotation.nameServer()))) {
RocketMQUtil.getNameServerString(rocketMQProperties.getNameServer()).equals(environment.resolvePlaceholders(annotation.nameServer()))) {
throw new BeanDefinitionValidationException(
"Bad annotation definition in @ExtRocketMQTemplateConfiguration, nameServer property is same with " +
"global property, please use the default RocketMQTemplate!");
Expand Down
7 changes: 3 additions & 4 deletions ...rc/main/java/org/apache/rocketmq/spring/autoconfigure/ListenerContainerConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,19 @@

package org.apache.rocketmq.spring.autoconfigure;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicLong;

import org.apache.rocketmq.client.AccessChannel;
import org.apache.rocketmq.spring.annotation.ConsumeMode;
import org.apache.rocketmq.spring.annotation.MessageModel;
import org.apache.rocketmq.spring.annotation.RocketMQMessageListener;
import org.apache.rocketmq.spring.core.RocketMQListener;
import org.apache.rocketmq.spring.support.DefaultRocketMQListenerContainer;
import org.apache.rocketmq.spring.support.RocketMQMessageConverter;
import org.apache.rocketmq.spring.support.RocketMQUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.aop.framework.AopProxyUtils;
Expand All @@ -43,8 +44,6 @@
import org.springframework.core.env.StandardEnvironment;
import org.springframework.util.StringUtils;

import com.fasterxml.jackson.databind.ObjectMapper;

@Configuration
public class ListenerContainerConfiguration implements ApplicationContextAware, SmartInitializingSingleton {
private final static Logger log = LoggerFactory.getLogger(ListenerContainerConfiguration.class);
Expand Down Expand Up @@ -134,7 +133,7 @@ private DefaultRocketMQListenerContainer createRocketMQListenerContainer(String
container.setRocketMQMessageListener(annotation);

String nameServer = environment.resolvePlaceholders(annotation.nameServer());
nameServer = StringUtils.isEmpty(nameServer) ? rocketMQProperties.getNameServer() : nameServer;
nameServer = StringUtils.isEmpty(nameServer) ? RocketMQUtil.getNameServerString(rocketMQProperties.getNameServer()) : nameServer;
String accessChannel = environment.resolvePlaceholders(annotation.accessChannel());
container.setNameServer(nameServer);
if (!StringUtils.isEmpty(accessChannel)) {
Expand Down
15 changes: 7 additions & 8 deletions ...oot/src/main/java/org/apache/rocketmq/spring/autoconfigure/RocketMQAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@

package org.apache.rocketmq.spring.autoconfigure;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.List;
import javax.annotation.PostConstruct;

import org.apache.rocketmq.acl.common.AclClientRPCHook;
import org.apache.rocketmq.acl.common.SessionCredentials;
import org.apache.rocketmq.client.AccessChannel;
Expand All @@ -29,6 +30,7 @@
import org.apache.rocketmq.spring.config.TransactionHandlerRegistry;
import org.apache.rocketmq.spring.core.RocketMQTemplate;
import org.apache.rocketmq.spring.support.RocketMQMessageConverter;
import org.apache.rocketmq.spring.support.RocketMQUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -48,8 +50,6 @@
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;

import com.fasterxml.jackson.databind.ObjectMapper;

@Configuration
@EnableConfigurationProperties(RocketMQProperties.class)
@ConditionalOnClass({MQAdmin.class, ObjectMapper.class})
Expand All @@ -73,12 +73,12 @@ public void checkProperties() {

@Bean
@ConditionalOnMissingBean(DefaultMQProducer.class)
@ConditionalOnProperty(prefix = "rocketmq", value = {"name-server", "producer.group"})
@ConditionalOnProperty(prefix = "rocketmq", value = {"producer.group"})
public DefaultMQProducer defaultMQProducer(RocketMQProperties rocketMQProperties) {
RocketMQProperties.Producer producerConfig = rocketMQProperties.getProducer();
String nameServer = rocketMQProperties.getNameServer();
List<String> nameServer = rocketMQProperties.getNameServer();
String groupName = producerConfig.getGroup();
Assert.hasText(nameServer, "[rocketmq.name-server] must not be null");
Assert.notEmpty(nameServer, "[rocketmq.name-server] must not be null");
Assert.hasText(groupName, "[rocketmq.producer.group] must not be null");

String accessChannel = rocketMQProperties.getAccessChannel();
Expand All @@ -95,8 +95,7 @@ public DefaultMQProducer defaultMQProducer(RocketMQProperties rocketMQProperties
producer = new DefaultMQProducer(groupName, rocketMQProperties.getProducer().isEnableMsgTrace(),
rocketMQProperties.getProducer().getCustomizedTraceTopic());
}

producer.setNamesrvAddr(nameServer);
producer.setNamesrvAddr(RocketMQUtil.getNameServerString(nameServer));
if (!StringUtils.isEmpty(accessChannel)) {
producer.setAccessChannel(AccessChannel.valueOf(accessChannel));
}
Expand Down
14 changes: 7 additions & 7 deletions ...pring-boot/src/main/java/org/apache/rocketmq/spring/autoconfigure/RocketMQProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,20 @@

package org.apache.rocketmq.spring.autoconfigure;

import org.apache.rocketmq.common.MixAll;
import org.springframework.boot.context.properties.ConfigurationProperties;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.apache.rocketmq.common.MixAll;
import org.springframework.boot.context.properties.ConfigurationProperties;

@SuppressWarnings("WeakerAccess")
@ConfigurationProperties(prefix = "rocketmq")
public class RocketMQProperties {

/**
* The name server for rocketMQ, formats: `host:port;host:port`.
* The name servers for rocketMQ, formats: `host:port,host:port`.
*/
private String nameServer;
private List<String> nameServer;
Copy link
Contributor

Choose a reason for hiding this comment

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

The name seems a bit odd for a collections. Have you considered naming that servers ? If you want to retain backward compatibility, you could deprecate this one with a replacement metadata to servers and then apply the old logic if nameServer is set. If both are set, I would throw an exception.

You could then remove nameServer in the next feature release.


/**
* Enum type for accesChannel, values: LOCAL, CLOUD
Expand All @@ -49,11 +49,11 @@ public class RocketMQProperties {
*/
private Consumer consumer = new Consumer();

public String getNameServer() {
public List<String> getNameServer() {
return nameServer;
}

public void setNameServer(String nameServer) {
public void setNameServer(List<String> nameServer) {
this.nameServer = nameServer;
}

Expand Down
24 changes: 20 additions & 4 deletions rocketmq-spring-boot/src/main/java/org/apache/rocketmq/spring/support/RocketMQUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
package org.apache.rocketmq.spring.support;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.nio.charset.Charset;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import org.apache.rocketmq.acl.common.AclClientRPCHook;
import org.apache.rocketmq.acl.common.SessionCredentials;
import org.apache.rocketmq.client.exception.MQClientException;
Expand All @@ -39,10 +43,6 @@
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;

import java.nio.charset.Charset;
import java.util.Map;
import java.util.Objects;

public class RocketMQUtil {
private final static Logger log = LoggerFactory.getLogger(RocketMQUtil.class);

Expand Down Expand Up @@ -254,4 +254,20 @@ public static String getInstanceName(RPCHook rpcHook, String identify) {
.append(separator).append(UtilAll.getPid());
return instanceName.toString();
}

public static String getNameServerString(List<String> nameServer) {
if (nameServer == null || nameServer.isEmpty()) {
throw new IllegalArgumentException("property nameServer is empty");
}
String nameServerToString;
if (nameServer.size() > 1) {
nameServerToString = String.join(";", nameServer);
} else {
nameServerToString = nameServer.get(0);
if (nameServerToString.contains(";")) {
log.warn("name-server format `host:port;host:port` is deprecated.");
Copy link
Member

Choose a reason for hiding this comment

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

Should have a recommendation for a newer style.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO that logic shows a separate property is warranted.

}
}
return nameServerToString;
}
}
17 changes: 14 additions & 3 deletions rocketmq-spring-boot/src/test/java/org/apache/rocketmq/spring/support/RocketMQUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@
package org.apache.rocketmq.spring.support;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.junit.Test;
import org.springframework.messaging.Message;
import org.springframework.messaging.support.MessageBuilder;

import java.util.Arrays;

import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

public class RocketMQUtilTest {

Expand Down Expand Up @@ -92,4 +95,12 @@ public void testHeaderConvertToSpringMsg() {
assertEquals("tags", springMsg.getHeaders().get(RocketMQHeaders.PREFIX + RocketMQHeaders.TAGS));
}

@Test
public void testGetNameServerString() {
List<String> nameServer = new ArrayList<>();
nameServer.add("localhost:9876");
nameServer.add("10.0.0.1:9876");
assertEquals(RocketMQUtil.getNameServerString(nameServer), "localhost:9876;10.0.0.1:9876");
}

}