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

[Dubbo-3875] dubbo TagRouter does not work with dubbo:parameter. #3883

Merged
merged 3 commits into from
Apr 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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);
}
}

}
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;
}
}
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 @@ -91,8 +91,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 @@ -287,13 +286,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;
}
}
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
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
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
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);
}
}
}
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,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
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
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