Skip to content

Commit

Permalink
[Enhancement]: refactor categorizing with Collectors.groupingBy (apac…
Browse files Browse the repository at this point in the history 
…he#3490)
  • Loading branch information
@kezhenxu94 @lixiaojiee
kezhenxu94 authored and lixiaojiee committed Feb 19, 2019
1 parent 7c236ca commit 717c15e
Showing 1 changed file with 28 additions and 19 deletions.
47 changes: 28 additions & 19 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 @@ -52,6 +52,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
Expand All @@ -64,7 +65,6 @@
import static org.apache.dubbo.common.Constants.PROVIDERS_CATEGORY;
import static org.apache.dubbo.common.Constants.ROUTERS_CATEGORY;
import static org.apache.dubbo.common.Constants.ROUTE_PROTOCOL;
import static org.apache.dubbo.common.utils.UrlUtils.classifyUrls;


/**
Expand Down Expand Up @@ -124,7 +124,7 @@ public RegistryDirectory(Class<T> serviceType, URL url) {
this.queryMap = StringUtils.parseQueryString(url.getParameterAndDecoded(Constants.REFER_KEY));
this.overrideDirectoryUrl = this.directoryUrl = turnRegistryUrlToConsumerUrl(url);
String group = directoryUrl.getParameter(Constants.GROUP_KEY, "");
this.multiGroup = group != null && ("*".equals(group) || group.contains(","));
this.multiGroup = group != null && (Constants.ANY_VALUE.equals(group) || group.contains(","));
}

private URL turnRegistryUrlToConsumerUrl(URL url) {
Expand Down Expand Up @@ -189,21 +189,30 @@ public void destroy() {

@Override
public synchronized void notify(List<URL> urls) {
List<URL> categoryUrls = urls.stream()
Map<String, List<URL>> categoryUrls = urls.stream()
.filter(Objects::nonNull)
.filter(this::isValidCategory)
.filter(this::isNotCompatibleFor26x)
.collect(Collectors.toList());
.collect(Collectors.groupingBy(url -> {
if (UrlUtils.isConfigurator(url)) {
return CONFIGURATORS_CATEGORY;
} else if (UrlUtils.isRoute(url)) {
return ROUTERS_CATEGORY;
} else if (UrlUtils.isProvider(url)) {
return PROVIDERS_CATEGORY;
}
return "";
}));

/**
* TODO Try to refactor the processing of these three type of urls using Collectors.groupBy()?
*/
this.configurators = Configurator.toConfigurators(classifyUrls(categoryUrls, UrlUtils::isConfigurator))
.orElse(configurators);
List<URL> configuratorURLs = categoryUrls.getOrDefault(CONFIGURATORS_CATEGORY, Collections.emptyList());
this.configurators = Configurator.toConfigurators(configuratorURLs).orElse(this.configurators);

toRouters(classifyUrls(categoryUrls, UrlUtils::isRoute)).ifPresent(this::addRouters);
List<URL> routerURLs = categoryUrls.getOrDefault(ROUTERS_CATEGORY, Collections.emptyList());
toRouters(routerURLs).ifPresent(this::addRouters);

// providers
refreshOverrideAndInvoker(classifyUrls(categoryUrls, UrlUtils::isProvider));
List<URL> providerURLs = categoryUrls.getOrDefault(PROVIDERS_CATEGORY, Collections.emptyList());
refreshOverrideAndInvoker(providerURLs);
}

private void refreshOverrideAndInvoker(List<URL> urls) {
Expand Down Expand Up @@ -283,7 +292,7 @@ private void refreshInvoker(List<URL> invokerUrls) {

private List<Invoker<T>> toMergeInvokerList(List<Invoker<T>> invokers) {
List<Invoker<T>> mergedInvokers = new ArrayList<>();
Map<String, List<Invoker<T>>> groupMap = new HashMap<String, List<Invoker<T>>>();
Map<String, List<Invoker<T>>> groupMap = new HashMap<>();
for (Invoker<T> invoker : invokers) {
String group = invoker.getUrl().getParameter(Constants.GROUP_KEY, "");
groupMap.computeIfAbsent(group, k -> new ArrayList<>());
Expand Down Expand Up @@ -343,11 +352,11 @@ private Optional<List<Router>> toRouters(List<URL> urls) {
* @return invokers
*/
private Map<String, Invoker<T>> toInvokers(List<URL> urls) {
Map<String, Invoker<T>> newUrlInvokerMap = new HashMap<String, Invoker<T>>();
Map<String, Invoker<T>> newUrlInvokerMap = new HashMap<>();
if (urls == null || urls.isEmpty()) {
return newUrlInvokerMap;
}
Set<String> keys = new HashSet<String>();
Set<String> keys = new HashSet<>();
String queryProtocols = this.queryMap.get(Constants.PROTOCOL_KEY);
for (URL providerUrl : urls) {
// If protocol is configured at the reference side, only the matching protocol is selected
Expand Down Expand Up @@ -393,7 +402,7 @@ private Map<String, Invoker<T>> toInvokers(List<URL> urls) {
enabled = url.getParameter(Constants.ENABLED_KEY, true);
}
if (enabled) {
invoker = new InvokerDelegate<T>(protocol.refer(serviceType, url), url, providerUrl);
invoker = new InvokerDelegate<>(protocol.refer(serviceType, url), url, providerUrl);
}
} catch (Throwable t) {
logger.error("Failed to refer invoker for interface:" + serviceType + ",url:(" + url + ")" + t.getMessage(), t);
Expand Down Expand Up @@ -426,7 +435,7 @@ private URL mergeUrl(URL providerUrl) {
this.overrideDirectoryUrl = this.overrideDirectoryUrl.addParametersIfAbsent(providerUrl.getParameters()); // Merge the provider side parameters

if ((providerUrl.getPath() == null || providerUrl.getPath()
.length() == 0) && "dubbo".equals(providerUrl.getProtocol())) { // Compatible version 1.0
.length() == 0) && Constants.DUBBO_PROTOCOL.equals(providerUrl.getProtocol())) { // Compatible version 1.0
//fix by tony.chenl DUBBO-44
String path = directoryUrl.getParameter(Constants.INTERFACE_KEY);
if (path != null) {
Expand Down Expand Up @@ -474,7 +483,7 @@ private URL overrideWithConfigurators(List<Configurator> configurators, URL url)
private void destroyAllInvokers() {
Map<String, Invoker<T>> localUrlInvokerMap = this.urlInvokerMap; // local reference
if (localUrlInvokerMap != null) {
for (Invoker<T> invoker : new ArrayList<Invoker<T>>(localUrlInvokerMap.values())) {
for (Invoker<T> invoker : new ArrayList<>(localUrlInvokerMap.values())) {
try {
invoker.destroy();
} catch (Throwable t) {
Expand Down Expand Up @@ -505,7 +514,7 @@ private void destroyUnusedInvokers(Map<String, Invoker<T>> oldUrlInvokerMap, Map
for (Map.Entry<String, Invoker<T>> entry : oldUrlInvokerMap.entrySet()) {
if (!newInvokers.contains(entry.getValue())) {
if (deleted == null) {
deleted = new ArrayList<String>();
deleted = new ArrayList<>();
}
deleted.add(entry.getKey());
}
Expand Down Expand Up @@ -597,7 +606,7 @@ public boolean isAvailable() {
}
Map<String, Invoker<T>> localUrlInvokerMap = urlInvokerMap;
if (localUrlInvokerMap != null && localUrlInvokerMap.size() > 0) {
for (Invoker<T> invoker : new ArrayList<Invoker<T>>(localUrlInvokerMap.values())) {
for (Invoker<T> invoker : new ArrayList<>(localUrlInvokerMap.values())) {
if (invoker.isAvailable()) {
return true;
}
Expand Down

0 comments on commit 717c15e

Please sign in to comment.