Skip to content

Commit

Permalink
fix apache#6426, replace unnecessary directoryUrl with consumerUrl (a…
Browse files Browse the repository at this point in the history 
…pache#6666)
  • Loading branch information
@chickenlj
chickenlj authored Sep 1, 2020
1 parent 12c18f6 commit 611be16
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 63 deletions.
2 changes: 1 addition & 1 deletion dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/directory/AbstractDirectory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public abstract class AbstractDirectory<T> implements Directory<T> {

private volatile boolean destroyed = false;

private volatile URL consumerUrl;
protected volatile URL consumerUrl;

protected final Map<String, String> queryMap; // Initialization at construction time, assertion not null
protected final String consumedProtocol;
Expand Down
35 changes: 8 additions & 27 deletions ...bo-registry-api/src/main/java/org/apache/dubbo/registry/integration/DynamicDirectory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.apache.dubbo.registry.integration;

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.URLBuilder;
import org.apache.dubbo.common.Version;
import org.apache.dubbo.common.extension.ExtensionLoader;
import org.apache.dubbo.common.logger.Logger;
Expand All @@ -43,14 +42,9 @@
import java.util.Set;

import static org.apache.dubbo.common.constants.CommonConstants.ANY_VALUE;
import static org.apache.dubbo.common.constants.CommonConstants.DUBBO;
import static org.apache.dubbo.common.constants.CommonConstants.GROUP_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.INTERFACE_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.MONITOR_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.PROTOCOL_KEY;
import static org.apache.dubbo.common.constants.RegistryConstants.CATEGORY_KEY;
import static org.apache.dubbo.common.constants.RegistryConstants.CONSUMERS_CATEGORY;
import static org.apache.dubbo.registry.Constants.REGISTER_IP_KEY;
import static org.apache.dubbo.registry.Constants.REGISTER_KEY;
import static org.apache.dubbo.registry.Constants.SIMPLIFIED_KEY;
import static org.apache.dubbo.registry.integration.InterfaceCompatibleRegistryProtocol.DEFAULT_REGISTER_CONSUMER_KEYS;
Expand All @@ -71,15 +65,14 @@ public abstract class DynamicDirectory<T> extends AbstractDirectory<T> implement

protected final String serviceKey; // Initialization at construction time, assertion not null
protected final Class<T> serviceType; // Initialization at construction time, assertion not null
protected final URL directoryUrl; // Initialization at construction time, assertion not null, and always assign non null value
protected final boolean multiGroup;
protected Protocol protocol; // Initialization at the time of injection, the assertion is not null
protected Registry registry; // Initialization at the time of injection, the assertion is not null
protected volatile boolean forbidden = false;
protected boolean shouldRegister;
protected boolean shouldSimplified;

protected volatile URL overrideDirectoryUrl; // Initialization at construction time, assertion not null, and always assign non null value
protected volatile URL overrideConsumerUrl; // Initialization at construction time, assertion not null, and always assign non null value

protected volatile URL registeredConsumerUrl;

Expand Down Expand Up @@ -114,8 +107,7 @@ public DynamicDirectory(Class<T> serviceType, URL url) {
this.serviceType = serviceType;
this.serviceKey = super.getConsumerUrl().getServiceKey();

this.overrideDirectoryUrl = this.directoryUrl = turnRegistryUrlToConsumerUrl(url);
String group = directoryUrl.getParameter(GROUP_KEY, "");
String group = queryMap.get(GROUP_KEY) != null ? queryMap.get(GROUP_KEY) : "";
this.multiGroup = group != null && (ANY_VALUE.equals(group) || group.contains(","));
}

Expand All @@ -124,18 +116,6 @@ public void addServiceListener(ServiceInstancesChangedListener instanceListener)
this.serviceListener = instanceListener;
}

private URL turnRegistryUrlToConsumerUrl(URL url) {
return URLBuilder.from(url)
.setHost(queryMap.get(REGISTER_IP_KEY) == null ? url.getHost() : queryMap.get(REGISTER_IP_KEY))
.setPort(0)
.setProtocol(queryMap.get(PROTOCOL_KEY) == null ? DUBBO : queryMap.get(PROTOCOL_KEY))
.setPath(queryMap.get(INTERFACE_KEY))
.clearParameters()
.addParameters(queryMap)
.removeParameter(MONITOR_KEY)
.build();
}

public void setProtocol(Protocol protocol) {
this.protocol = protocol;
}
Expand Down Expand Up @@ -197,11 +177,6 @@ public List<Invoker<T>> getAllInvokers() {
return invokers;
}

@Override
public URL getConsumerUrl() {
return this.overrideDirectoryUrl;
}

public URL getRegisteredConsumerUrl() {
return registeredConsumerUrl;
}
Expand Down Expand Up @@ -247,4 +222,10 @@ public List<Invoker<T>> getInvokers() {
return invokers;
}

@Override
public void setConsumerUrl(URL consumerUrl) {
this.consumerUrl = consumerUrl;
this.overrideConsumerUrl = consumerUrl;
}

}
35 changes: 15 additions & 20 deletions ...o-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryDirectory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public RegistryDirectory(Class<T> serviceType, URL url) {
@Override
public void subscribe(URL url) {
setConsumerUrl(url);
// overrideConsumerUrl();
CONSUMER_CONFIGURATION_LISTENER.addNotifyListener(this);
referenceConfigurationListener = new ReferenceConfigurationListener(this, url);
registry.subscribe(url, this);
Expand Down Expand Up @@ -181,7 +182,7 @@ private String judgeCategory(URL url) {

private void refreshOverrideAndInvoker(List<URL> urls) {
// mock zookeeper://xxx?mock=return null
overrideDirectoryUrl();
overrideConsumerUrl();
refreshInvoker(urls);
}

Expand Down Expand Up @@ -393,13 +394,10 @@ private URL mergeUrl(URL providerUrl) {

providerUrl = providerUrl.addParameter(Constants.CHECK_KEY, String.valueOf(false)); // Do not check whether the connection is successful or not, always create Invoker!

// The combination of directoryUrl and override is at the end of notify, which can't be handled here
// this.overrideDirectoryUrl = this.overrideDirectoryUrl.addParametersIfAbsent(providerUrl.getParameters()); // Merge the provider side parameters

if ((providerUrl.getPath() == null || providerUrl.getPath()
.length() == 0) && DUBBO_PROTOCOL.equals(providerUrl.getProtocol())) { // Compatible version 1.0
//fix by tony.chenl DUBBO-44
String path = directoryUrl.getParameter(INTERFACE_KEY);
String path = getConsumerUrl().getParameter(INTERFACE_KEY);
if (path != null) {
int i = path.indexOf('/');
if (i >= 0) {
Expand Down Expand Up @@ -537,11 +535,6 @@ public List<Invoker<T>> getAllInvokers() {
return invokers;
}

@Override
public URL getConsumerUrl() {
return this.overrideDirectoryUrl;
}

public URL getRegisteredConsumerUrl() {
return registeredConsumerUrl;
}
Expand Down Expand Up @@ -604,23 +597,25 @@ private boolean isNotCompatibleFor26x(URL url) {
return StringUtils.isEmpty(url.getParameter(COMPATIBLE_CONFIG_KEY));
}

private void overrideDirectoryUrl() {
private void overrideConsumerUrl() {
// merge override parameters
this.overrideDirectoryUrl = directoryUrl;
List<Configurator> localConfigurators = this.configurators; // local reference
doOverrideUrl(localConfigurators);
List<Configurator> localAppDynamicConfigurators = CONSUMER_CONFIGURATION_LISTENER.getConfigurators(); // local reference
doOverrideUrl(localAppDynamicConfigurators);
if (referenceConfigurationListener != null) {
List<Configurator> localDynamicConfigurators = referenceConfigurationListener.getConfigurators(); // local reference
doOverrideUrl(localDynamicConfigurators);
this.overrideConsumerUrl = getConsumerUrl();
if (overrideConsumerUrl != null) {
List<Configurator> localConfigurators = this.configurators; // local reference
doOverrideUrl(localConfigurators);
List<Configurator> localAppDynamicConfigurators = CONSUMER_CONFIGURATION_LISTENER.getConfigurators(); // local reference
doOverrideUrl(localAppDynamicConfigurators);
if (referenceConfigurationListener != null) {
List<Configurator> localDynamicConfigurators = referenceConfigurationListener.getConfigurators(); // local reference
doOverrideUrl(localDynamicConfigurators);
}
}
}

private void doOverrideUrl(List<Configurator> configurators) {
if (CollectionUtils.isNotEmpty(configurators)) {
for (Configurator configurator : configurators) {
this.overrideDirectoryUrl = configurator.configure(overrideDirectoryUrl);
this.overrideConsumerUrl = configurator.configure(overrideConsumerUrl);
}
}
}
Expand Down
20 changes: 8 additions & 12 deletions ...registry-default/src/test/java/org/apache/dubbo/registry/dubbo/RegistryDirectoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
package org.apache.dubbo.registry.dubbo;

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.URLBuilder;
import org.apache.dubbo.common.extension.ExtensionLoader;
import org.apache.dubbo.common.utils.LogUtil;
import org.apache.dubbo.common.utils.NetUtils;
import org.apache.dubbo.common.utils.StringUtils;
import org.apache.dubbo.registry.NotifyListener;
import org.apache.dubbo.registry.Registry;
import org.apache.dubbo.registry.RegistryFactory;
Expand Down Expand Up @@ -58,7 +58,6 @@
import static org.apache.dubbo.common.constants.CommonConstants.APPLICATION_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.CONSUMER_SIDE;
import static org.apache.dubbo.common.constants.CommonConstants.DISABLED_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.DUBBO;
import static org.apache.dubbo.common.constants.CommonConstants.ENABLED_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.LOADBALANCE_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.SIDE_KEY;
Expand All @@ -68,6 +67,7 @@
import static org.apache.dubbo.common.constants.RegistryConstants.PROVIDERS_CATEGORY;
import static org.apache.dubbo.common.constants.RegistryConstants.ROUTERS_CATEGORY;
import static org.apache.dubbo.common.constants.RegistryConstants.ROUTE_PROTOCOL;
import static org.apache.dubbo.registry.Constants.CONSUMER_PROTOCOL;
import static org.apache.dubbo.rpc.Constants.MOCK_KEY;
import static org.apache.dubbo.rpc.cluster.Constants.INVOCATION_NEED_MOCK;
import static org.apache.dubbo.rpc.cluster.Constants.MOCK_PROTOCOL;
Expand Down Expand Up @@ -103,7 +103,9 @@ private RegistryDirectory getRegistryDirectory(URL url) {
registryDirectory.setProtocol(protocol);
registryDirectory.setRegistry(registry);
registryDirectory.setRouterChain(RouterChain.buildChain(url));
registryDirectory.subscribe(url);
Map<String, String> queryMap = StringUtils.parseQueryString(url.getParameterAndDecoded(REFER_KEY));
URL subscribeUrl = new URL(CONSUMER_PROTOCOL, "10.20.30.40", 0, url.getServiceInterface(), queryMap);
registryDirectory.subscribe(subscribeUrl);
// asert empty
List invokers = registryDirectory.list(invocation);
Assertions.assertEquals(0, invokers.size());
Expand Down Expand Up @@ -141,20 +143,14 @@ public void test_Constructor_WithErrorParam() {

@Test
public void test_Constructor_CheckStatus() throws Exception {
URL url = URL.valueOf("notsupported://10.20.30.40/" + service + "?a=b").addParameterAndEncoded(REFER_KEY, "foo=bar");
URL url = URL.valueOf("notsupported://10.20.30.40/" + service + "?a=b").addParameterAndEncoded(REFER_KEY,
"foo=bar");
RegistryDirectory reg = getRegistryDirectory(url);
Field field = reg.getClass().getSuperclass().getSuperclass().getDeclaredField("queryMap");
field.setAccessible(true);
Map<String, String> queryMap = (Map<String, String>) field.get(reg);
Assertions.assertEquals("bar", queryMap.get("foo"));
URL consumerUrl = URLBuilder.from(url)
.setPort(0)
.setProtocol(DUBBO)
.setPath(null)
.clearParameters()
.addParameters(queryMap)
.build();
Assertions.assertEquals(consumerUrl, reg.getConsumerUrl());
Assertions.assertEquals(url.setProtocol(CONSUMER_PROTOCOL).clearParameters().addParameter("foo", "bar"), reg.getConsumerUrl());
}

@Test
Expand Down
5 changes: 2 additions & 3 deletions ...stry-default/src/test/java/org/apache/dubbo/registry/dubbo/RegistryStatusCheckerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.apache.dubbo.registry.dubbo;

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.URLBuilder;
import org.apache.dubbo.common.extension.ExtensionLoader;
import org.apache.dubbo.common.status.Status;
import org.apache.dubbo.registry.RegistryFactory;
Expand Down Expand Up @@ -65,7 +64,7 @@ public void testCheckOK() {
assertEquals(Status.Level.OK, new RegistryStatusChecker().check().getLevel());

String message = new RegistryStatusChecker().check().getMessage();
Assertions.assertTrue(message.contains(URLBuilder.from(registryUrl).setPort(0).build().getAddress() + "(connected)"));
Assertions.assertTrue(message.contains(URLBuilder.from(registryUrl2).setPort(0).build().getAddress() + "(connected)"));
Assertions.assertTrue(message.contains(registryUrl.getAddress() + "(connected)"));
Assertions.assertTrue(message.contains(registryUrl2.getAddress() + "(connected)"));
}
}

0 comments on commit 611be16

Please sign in to comment.