Skip to content

Commit

Permalink
[Dubbo-3875] dubbo TagRouter does not work with dubbo:parameter. (apa…
Browse files Browse the repository at this point in the history 
…che#3883)

* avoid 'dubbo.tag' from provider side being override on consumer side
* not necessary to keep 'default.' prefix for configs come from ConsumerConfig and ProviderConfig
* fix ut caused by removing of 'default.' prefix
  • Loading branch information
@chickenlj @vio-lin
chickenlj authored and vio-lin committed Apr 29, 2019
1 parent 06d672c commit 9ee79b9
Show file tree
Hide file tree
Showing 13 changed files with 69 additions and 85 deletions.
30 changes: 12 additions & 18 deletions dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/ClusterUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,24 +95,11 @@ public static URL mergeUrl(URL remoteUrl, Map<String, String> localMap) {
}
if (remoteMap != null && remoteMap.size() > 0) {
// Use version passed from provider side
String dubbo = remoteMap.get(Constants.DUBBO_VERSION_KEY);
if (dubbo != null && dubbo.length() > 0) {
map.put(Constants.DUBBO_VERSION_KEY, dubbo);
}
String version = remoteMap.get(Constants.VERSION_KEY);
if (version != null && version.length() > 0) {
map.put(Constants.VERSION_KEY, version);
}
String methods = remoteMap.get(Constants.METHODS_KEY);
if (methods != null && methods.length() > 0) {
map.put(Constants.METHODS_KEY, methods);
}
// Reserve timestamp of provider url.
String remoteTimestamp = remoteMap.get(Constants.TIMESTAMP_KEY);
if (remoteTimestamp != null && remoteTimestamp.length() > 0) {
map.put(Constants.REMOTE_TIMESTAMP_KEY, remoteMap.get(Constants.TIMESTAMP_KEY));
}

reserveRemoteValue(Constants.DUBBO_VERSION_KEY, map, remoteMap);
reserveRemoteValue(Constants.VERSION_KEY, map, remoteMap);
reserveRemoteValue(Constants.METHODS_KEY, map, remoteMap);
reserveRemoteValue(Constants.TIMESTAMP_KEY, map, remoteMap);
reserveRemoteValue(Constants.TAG_KEY, map, remoteMap);
// TODO, for compatibility consideration, we cannot simply change the value behind APPLICATION_KEY from Consumer to Provider. So just add an extra key here.
// Reserve application name from provider.
map.put(Constants.REMOTE_APPLICATION_KEY, remoteMap.get(Constants.APPLICATION_KEY));
Expand All @@ -135,4 +122,11 @@ public static URL mergeUrl(URL remoteUrl, Map<String, String> localMap) {
return remoteUrl.clearParameters().addParameters(map);
}

private static void reserveRemoteValue(String key, Map<String, String> map, Map<String, String> remoteMap) {
String remoteValue = remoteMap.get(key);
if (StringUtils.isNotEmpty(remoteValue)) {
map.put(key, remoteValue);
}
}

}
11 changes: 11 additions & 0 deletions ...onfig/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ public abstract class AbstractInterfaceConfig extends AbstractMethodConfig {
// the scope for referring/exporting a service, if it's local, it means searching in current JVM only.
private String scope;

protected String tag;

/**
* Check whether the registry config is exists, and then conversion it to {@link RegistryConfig}
*/
Expand Down Expand Up @@ -837,4 +839,13 @@ public MetricsConfig getMetrics() {
public void setMetrics(MetricsConfig metrics) {
this.metrics = metrics;
}

@Parameter(key = Constants.TAG_KEY, useKeyAsProperty = false)
public String getTag() {
return tag;
}

public void setTag(String tag) {
this.tag = tag;
}
}
14 changes: 2 additions & 12 deletions ...-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractServiceConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
package org.apache.dubbo.config;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.config.context.ConfigManager;
import org.apache.dubbo.common.utils.CollectionUtils;
import org.apache.dubbo.config.context.ConfigManager;
import org.apache.dubbo.config.support.Parameter;
import org.apache.dubbo.rpc.ExporterListener;

Expand Down Expand Up @@ -92,8 +92,7 @@ public abstract class AbstractServiceConfig extends AbstractInterfaceConfig {
*/
protected List<ProtocolConfig> protocols;
protected String protocolIds;
// provider tag
protected String tag;

// max allowed execute times
private Integer executes;

Expand Down Expand Up @@ -288,13 +287,4 @@ public String getSerialization() {
public void setSerialization(String serialization) {
this.serialization = serialization;
}

@Parameter(key = Constants.TAG_KEY, useKeyAsProperty = false)
public String getTag() {
return tag;
}

public void setTag(String tag) {
this.tag = tag;
}
}
22 changes: 8 additions & 14 deletions dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,9 @@ private void init() {
appendParameters(map, metrics);
appendParameters(map, application);
appendParameters(map, module);
appendParameters(map, consumer, Constants.DEFAULT_KEY);
// remove 'default.' prefix for configs from ConsumerConfig
// appendParameters(map, consumer, Constants.DEFAULT_KEY);
appendParameters(map, consumer);
appendParameters(map, this);
Map<String, Object> attributes = null;
if (CollectionUtils.isNotEmpty(methods)) {
Expand Down Expand Up @@ -457,22 +459,14 @@ protected boolean shouldInit() {
}

private void checkDefault() {
createConsumerIfAbsent();
}

private void createConsumerIfAbsent() {
if (consumer != null) {
return;
}
setConsumer(
ConfigManager.getInstance()
.getDefaultConsumer()
.orElseGet(() -> {
ConsumerConfig consumerConfig = new ConsumerConfig();
consumerConfig.refresh();
return consumerConfig;
})
);
setConsumer(ConfigManager.getInstance().getDefaultConsumer().orElseGet(() -> {
ConsumerConfig consumerConfig = new ConsumerConfig();
consumerConfig.refresh();
return consumerConfig;
}));
}

private void completeCompoundConfigs() {
Expand Down
4 changes: 3 additions & 1 deletion dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,9 @@ private void doExportUrlsFor1Protocol(ProtocolConfig protocolConfig, List<URL> r
appendParameters(map, metrics);
appendParameters(map, application);
appendParameters(map, module);
appendParameters(map, provider, Constants.DEFAULT_KEY);
// remove 'default.' prefix for configs from ProviderConfig
// appendParameters(map, provider, Constants.DEFAULT_KEY);
appendParameters(map, provider);
appendParameters(map, protocolConfig);
appendParameters(map, this);
if (CollectionUtils.isNotEmpty(methods)) {
Expand Down
5 changes: 5 additions & 0 deletions ...o-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/annotation/Reference.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@
*/
String protocol() default "";

/**
* Service tag name
*/
String tag() default "";

/**
* methods support
* @return
Expand Down
10 changes: 10 additions & 0 deletions ...o-config-api/src/main/java/org/apache/dubbo/config/builders/AbstractInterfaceBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ public abstract class AbstractInterfaceBuilder<T extends AbstractInterfaceConfig
// the scope for referring/exporting a service, if it's local, it means searching in current JVM only.
private String scope;

private String tag;

/**
* @param local
* @see org.apache.dubbo.config.builders.AbstractInterfaceBuilder#stub(String)
Expand Down Expand Up @@ -267,6 +269,11 @@ public B scope(String scope) {
return getThis();
}

public B tag(String tag) {
this.tag = tag;
return getThis();
}

@Override
public void build(T instance) {
super.build(instance);
Expand Down Expand Up @@ -331,5 +338,8 @@ public void build(T instance) {
if (!StringUtils.isEmpty(scope)) {
instance.setScope(scope);
}
if (StringUtils.isNotEmpty(tag)) {
instance.setTag(tag);
}
}
}
11 changes: 1 addition & 10 deletions ...bbo-config-api/src/main/java/org/apache/dubbo/config/builders/AbstractServiceBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ public abstract class AbstractServiceBuilder<T extends AbstractServiceConfig, B
*/
protected List<ProtocolConfig> protocols;
protected String protocolIds;
// provider tag
protected String tag;

// max allowed execute times
private Integer executes;

Expand Down Expand Up @@ -197,11 +196,6 @@ public B protocolIds(String protocolIds) {
return getThis();
}

public B tag(String tag) {
this.tag = tag;
return getThis();
}

public B executes(Integer executes) {
this.executes = executes;
return getThis();
Expand Down Expand Up @@ -262,9 +256,6 @@ public void build(T instance) {
if (!StringUtils.isEmpty(protocolIds)) {
instance.setProtocolIds(protocolIds);
}
if (tag != null) {
instance.setTag(tag);
}
if (executes != null) {
instance.setExecutes(executes);
}
Expand Down
2 changes: 1 addition & 1 deletion dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ServiceConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public void testExport() throws Exception {
assertThat(url.getParameters(), hasEntry(Constants.APPLICATION_KEY, "app"));
assertThat(url.getParameters(), hasKey(Constants.BIND_IP_KEY));
assertThat(url.getParameters(), hasKey(Constants.BIND_PORT_KEY));
assertThat(url.getParameters(), hasEntry(Constants.DEFAULT_KEY + "." + Constants.EXPORT_KEY, "true"));
assertThat(url.getParameters(), hasEntry(Constants.EXPORT_KEY, "true"));
assertThat(url.getParameters(), hasEntry("echo.0.callback", "false"));
assertThat(url.getParameters(), hasEntry(Constants.GENERIC_KEY, "false"));
assertThat(url.getParameters(), hasEntry(Constants.INTERFACE_KEY, DemoService.class.getName()));
Expand Down
13 changes: 3 additions & 10 deletions .../dubbo-config-api/src/test/java/org/apache/dubbo/config/url/InvokerSideConfigUrlTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
import org.apache.dubbo.config.mock.MockRegistry;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -112,18 +112,11 @@ public class InvokerSideConfigUrlTest extends UrlTestBase {
//{"", "", "", "", "", "", "", "", "", ""},
};

private Object consumerConfTable[][] = {
{"timeout", "default.timeout", "int", 5000, 8000, "", "", "", "", ""},
{"retries", "default.retries", "int", 2, 5, "", "", "", "", ""},
{"loadbalance", "default.loadbalance", "string", "random", "leastactive", "", "", "", "", ""},
{"async", "default.async", "boolean", false, true, "", "", "", "", ""},
{"connections", "default.connections", "int", 100, 5, "", "", "", "", ""},
private Object consumerConfTable[][] = {{"timeout", "timeout", "int", 5000, 8000, "", "", "", "", ""}, {"retries", "retries", "int", 2, 5, "", "", "", "", ""}, {"loadbalance", "loadbalance", "string", "random", "leastactive", "", "", "", "", ""}, {"async", "async", "boolean", false, true, "", "", "", "", ""}, {"connections", "connections", "int", 100, 5, "", "", "", "", ""},
// {"generic", "generic", "boolean", false, false, "", "", "", "", ""},
{"check", "check", "boolean", true, false, "", "", "", "", ""},
{"proxy", "proxy", "string", "javassist", "jdk", "javassist", "", "", "", ""},
{"owner", "owner", "string", "", "haomin", "", "", "", "", ""},
{"actives", "default.actives", "int", 0, 5, "", "", "", "", ""},
{"cluster", "default.cluster", "string", "failover", "forking", "", "", "", "", ""},
{"owner", "owner", "string", "", "haomin", "", "", "", "", ""}, {"actives", "actives", "int", 0, 5, "", "", "", "", ""}, {"cluster", "cluster", "string", "failover", "forking", "", "", "", "", ""},
{"filter", "", "string", "", "", "", "", "", "", ""},
{"listener", "", "string", "", "", "", "", "", "", ""},
// {"", "", "", "", "", "", "", "", "", ""},
Expand Down
8 changes: 1 addition & 7 deletions dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/url/UrlTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,7 @@ public class UrlTestBase {
// {"subscribe", "subscribe", "boolean", true, false, "", "", "", "", ""},
{"dynamic", "dynamic", "boolean", true, false, "", "", "", "", ""},
};
protected Object provConfTable[][] = {
{"cluster", "default.cluster", "string", "string", "failover", "failfast", "failsafe", "", "", ""},
{"async", "default.async", "boolean", false, true, "", "", "", "", ""},
{"loadbalance", "default.loadbalance", "string", "random", "leastactive", "", "", "", "", ""},
{"connections", "default.connections", "int", 0, 60, "", "", "", "", ""},
{"retries", "default.retries", "int", 2, 60, "", "", "", "", ""},
{"timeout", "default.timeout", "int", 5000, 60, "", "", "", "", ""},
protected Object provConfTable[][] = {{"cluster", "cluster", "string", "string", "failover", "failfast", "failsafe", "", "", ""}, {"async", "async", "boolean", false, true, "", "", "", "", ""}, {"loadbalance", "loadbalance", "string", "random", "leastactive", "", "", "", "", ""}, {"connections", "connections", "int", 0, 60, "", "", "", "", ""}, {"retries", "retries", "int", 2, 60, "", "", "", "", ""}, {"timeout", "timeout", "int", 5000, 60, "", "", "", "", ""},
//change by fengting listener 没有缺省值
//{"listener", "exporter.listener", "string", "", "", "", "", "", "", ""},
//{"filter", "service.filter", "string", "", "", "", "", "", "", ""},
Expand Down
12 changes: 6 additions & 6 deletions dubbo-config/dubbo-config-spring/src/main/resources/META-INF/compat/dubbo.xsd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@
<![CDATA[ Defines the service visibility, choise:[local remote]. default is remote, which can be invoked by network。 ]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="tag" type="xsd:string">
<xsd:annotation>
<xsd:documentation>
<![CDATA[ Defines the service tag]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:extension>
</xsd:complexContent>
</xsd:complexType>
Expand Down Expand Up @@ -317,12 +323,6 @@
<xsd:documentation><![CDATA[ The serialization protocol of service. ]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="tag" type="xsd:string">
<xsd:annotation>
<xsd:documentation>
<![CDATA[ Defines the service tag]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:anyAttribute namespace="##other" processContents="lax"/>
</xsd:extension>
</xsd:complexContent>
Expand Down
12 changes: 6 additions & 6 deletions dubbo-config/dubbo-config-spring/src/main/resources/META-INF/dubbo.xsd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@
<![CDATA[ Defines the service visibility, choise:[local remote]. default is remote, which can be invoked by network。 ]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="tag" type="xsd:string">
<xsd:annotation>
<xsd:documentation>
<![CDATA[ Defines the service tag]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
</xsd:extension>
</xsd:complexContent>
</xsd:complexType>
Expand Down Expand Up @@ -317,12 +323,6 @@
<xsd:documentation><![CDATA[ The serialization protocol of service. ]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:attribute name="tag" type="xsd:string">
<xsd:annotation>
<xsd:documentation>
<![CDATA[ Defines the service tag]]></xsd:documentation>
</xsd:annotation>
</xsd:attribute>
<xsd:anyAttribute namespace="##other" processContents="lax"/>
</xsd:extension>
</xsd:complexContent>
Expand Down

0 comments on commit 9ee79b9

Please sign in to comment.