From 8d8aed20b1f516c28d7176669a7ffbb4d8a004c3 Mon Sep 17 00:00:00 2001 From: Ian Luo Date: Fri, 23 Nov 2018 15:03:35 +0800 Subject: [PATCH] Merge pull request #2823, Code review for ConfigurationUtils and Environment. * change method signature to Configuration getConfiguration(String prefix, String id), in this way, we expose interface to the caller instead of concreate impl * remove compositeconfiguration from ConfigurationUtils --- .../dubbo/common/config/Environment.java | 6 +- .../apache/dubbo/config/AbstractConfig.java | 6 +- .../dubbo/config/AbstractInterfaceConfig.java | 2 +- .../apache/dubbo/config/ServiceConfig.java | 2 +- .../configcenter/ConfigurationUtils.java | 59 +++---------------- .../dubbo/container/log4j/Log4jContainer.java | 10 ++-- 6 files changed, 25 insertions(+), 60 deletions(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/config/Environment.java b/dubbo-common/src/main/java/org/apache/dubbo/common/config/Environment.java index a6ce939593..746fbc31dd 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/config/Environment.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/config/Environment.java @@ -110,7 +110,7 @@ public class Environment { * @param id * @return */ - public CompositeConfiguration getStartupCompositeConf(String prefix, String id) { + public Configuration getConfiguration(String prefix, String id) { CompositeConfiguration compositeConfiguration = new CompositeConfiguration(); compositeConfiguration.addConfiguration(this.getSystemConfig(prefix, id)); compositeConfiguration.addConfiguration(this.getAppExternalConfig(prefix, id)); @@ -125,6 +125,10 @@ public class Environment { return compositeConfiguration; } + public Configuration getConfiguration() { + return getConfiguration(null, null); + } + private static String toKey(String prefix, String id) { StringBuilder sb = new StringBuilder(); if (StringUtils.isNotEmpty(prefix)) { diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java index a2ecb87622..8dfe27e935 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractConfig.java @@ -18,7 +18,7 @@ package org.apache.dubbo.config; import org.apache.dubbo.common.Constants; import org.apache.dubbo.common.URL; -import org.apache.dubbo.common.config.CompositeConfiguration; +import org.apache.dubbo.common.config.Configuration; import org.apache.dubbo.common.config.Environment; import org.apache.dubbo.common.extension.ExtensionLoader; import org.apache.dubbo.common.logger.Logger; @@ -499,14 +499,14 @@ public abstract class AbstractConfig implements Serializable { try { Environment env = Environment.getInstance(); env.addAppConfig(getPrefix(), getId(), getMetaData()); - CompositeConfiguration compositeConfiguration = env.getStartupCompositeConf(getPrefix(), getId()); + Configuration configuration = env.getConfiguration(getPrefix(), getId()); // loop methods, get override value and set the new value back to method Method[] methods = getClass().getMethods(); for (Method method : methods) { if (ClassHelper.isSetter(method)) { try { - String value = compositeConfiguration.getString(extractPropertyName(getClass(), method)); + String value = configuration.getString(extractPropertyName(getClass(), method)); // isTypeMatch() is called to avoid duplicate and incorrect update, for example, we have two 'setGeneric' methods in ReferenceConfig. if (value != null && ClassHelper.isTypeMatch(method.getParameterTypes()[0], value)) { method.invoke(this, ClassHelper.convertPrimitive(method.getParameterTypes()[0], value)); diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java index 69dfc2a397..438534d15d 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java @@ -131,7 +131,7 @@ public abstract class AbstractInterfaceConfig extends AbstractMethodConfig { if (registries == null || registries.isEmpty()) { registries = new ArrayList<>(); - String registryIds = Environment.getInstance().getStartupCompositeConf(null, null).getString("dubbo.registries"); + String registryIds = Environment.getInstance().getConfiguration(null, null).getString("dubbo.registries"); if (StringUtils.isNotEmpty(registryIds)) { Arrays.stream(Constants.COMMA_SPLIT_PATTERN.split(registryIds)) .map(regId -> { diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java index 4bc8a4d134..0853584be0 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java @@ -733,7 +733,7 @@ public class ServiceConfig extends AbstractServiceConfig { // backward compatibility if (protocols == null || protocols.isEmpty()) { //check 'dubbo.protocols=dubboProtocolId,hessianProtocolId' and decide if we need multiple protocols - String protocolIds = Environment.getInstance().getStartupCompositeConf(null, null).getString("dubbo.protocols"); + String protocolIds = Environment.getInstance().getConfiguration(null, null).getString("dubbo.protocols"); if (StringUtils.isNotEmpty(protocolIds)) { Arrays.stream(Constants.COMMA_SPLIT_PATTERN.split(protocolIds)) .map(pId -> { diff --git a/dubbo-configcenter/dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/ConfigurationUtils.java b/dubbo-configcenter/dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/ConfigurationUtils.java index a0c659b10b..ea8a78d233 100644 --- a/dubbo-configcenter/dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/ConfigurationUtils.java +++ b/dubbo-configcenter/dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/ConfigurationUtils.java @@ -17,61 +17,27 @@ package org.apache.dubbo.configcenter; import org.apache.dubbo.common.Constants; -import org.apache.dubbo.common.URL; -import org.apache.dubbo.common.config.CompositeConfiguration; +import org.apache.dubbo.common.config.Configuration; import org.apache.dubbo.common.config.Environment; -import org.apache.dubbo.common.extension.ExtensionLoader; import org.apache.dubbo.common.utils.CollectionUtils; -import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; + +import static org.apache.dubbo.common.extension.ExtensionLoader.getExtensionLoader; /** * Utilities for manipulating configurations from different sources */ public class ConfigurationUtils { - private static final CompositeConfiguration compositeConfiguration; - - static { - compositeConfiguration = new CompositeConfiguration(); - compositeConfiguration.addConfiguration(getDynamicConfiguration()); - compositeConfiguration.addConfiguration(Environment.getInstance().getAppExternalConfig(null, null)); - compositeConfiguration.addConfiguration(Environment.getInstance().getExternalConfig(null, null)); - compositeConfiguration.addConfiguration(Environment.getInstance().getSystemConfig(null, null)); - compositeConfiguration.addConfiguration(Environment.getInstance().getPropertiesConfig(null, null)); - } - - private volatile Map runtimeCompositeConfsHolder = new ConcurrentHashMap<>(); - - /** - * FIXME This method will recreate Configuration for each RPC, how much latency affect will this action has on performance? - * - * @param url, the url metadata. - * @param method, the method name the RPC is trying to invoke. - * @return - */ - public static CompositeConfiguration getRuntimeCompositeConf(URL url, String method) { - CompositeConfiguration compositeConfiguration = new CompositeConfiguration(); - - String app = url.getParameter(Constants.APPLICATION_KEY); - String service = url.getServiceKey(); - compositeConfiguration.addConfiguration(new ConfigurationWrapper(app, service, method, getDynamicConfiguration())); - - compositeConfiguration.addConfiguration(url.toConfiguration()); - - return compositeConfiguration; - } - /** * If user opens DynamicConfig, the extension instance must has been created during the initialization of * ConfigCenterConfig with the right extension type user specifies. If no DynamicConfig presents, * NopDynamicConfiguration will be used. */ public static DynamicConfiguration getDynamicConfiguration() { - Set configurations = ExtensionLoader.getExtensionLoader(DynamicConfiguration.class).getExtensions(); + Set configurations = getExtensionLoader(DynamicConfiguration.class).getExtensions(); if (CollectionUtils.isEmpty(configurations)) { - return ExtensionLoader.getExtensionLoader(DynamicConfiguration.class).getDefaultExtension(); + return getExtensionLoader(DynamicConfiguration.class).getDefaultExtension(); } else { return configurations.iterator().next(); } @@ -80,7 +46,9 @@ public class ConfigurationUtils { @SuppressWarnings("deprecation") public static int getServerShutdownTimeout() { int timeout = Constants.DEFAULT_SERVER_SHUTDOWN_TIMEOUT; - String value = getProperty(Constants.SHUTDOWN_WAIT_KEY); + Configuration configuration = Environment.getInstance().getConfiguration(); + String value = configuration.getString(Constants.SHUTDOWN_WAIT_KEY); + if (value != null && value.length() > 0) { try { timeout = Integer.parseInt(value); @@ -88,7 +56,7 @@ public class ConfigurationUtils { // ignore } } else { - value = getProperty(Constants.SHUTDOWN_WAIT_SECONDS_KEY); + value = configuration.getString(Constants.SHUTDOWN_WAIT_SECONDS_KEY); if (value != null && value.length() > 0) { try { timeout = Integer.parseInt(value) * 1000; @@ -99,13 +67,4 @@ public class ConfigurationUtils { } return timeout; } - - public static String getProperty(String key) { - return compositeConfiguration.getString(key); - } - - public static String getProperty(String key, String defaultValue) { - return compositeConfiguration.getString(key, defaultValue); - } - } diff --git a/dubbo-container/dubbo-container-log4j/src/main/java/org/apache/dubbo/container/log4j/Log4jContainer.java b/dubbo-container/dubbo-container-log4j/src/main/java/org/apache/dubbo/container/log4j/Log4jContainer.java index 78304bb8ca..d7827e90e2 100644 --- a/dubbo-container/dubbo-container-log4j/src/main/java/org/apache/dubbo/container/log4j/Log4jContainer.java +++ b/dubbo-container/dubbo-container-log4j/src/main/java/org/apache/dubbo/container/log4j/Log4jContainer.java @@ -16,7 +16,8 @@ */ package org.apache.dubbo.container.log4j; -import org.apache.dubbo.configcenter.ConfigurationUtils; +import org.apache.dubbo.common.config.Configuration; +import org.apache.dubbo.common.config.Environment; import org.apache.dubbo.container.Container; import org.apache.log4j.Appender; @@ -43,9 +44,10 @@ public class Log4jContainer implements Container { @Override @SuppressWarnings("unchecked") public void start() { - String file = ConfigurationUtils.getProperty(LOG4J_FILE); + Configuration configuration = Environment.getInstance().getConfiguration(); + String file = configuration.getString(LOG4J_FILE); if (file != null && file.length() > 0) { - String level = ConfigurationUtils.getProperty(LOG4J_LEVEL); + String level = configuration.getString(LOG4J_LEVEL); if (level == null || level.length() == 0) { level = DEFAULT_LOG4J_LEVEL; } @@ -59,7 +61,7 @@ public class Log4jContainer implements Container { properties.setProperty("log4j.appender.application.layout.ConversionPattern", "%d [%t] %-5p %C{6} (%F:%L) - %m%n"); PropertyConfigurator.configure(properties); } - String subdirectory = ConfigurationUtils.getProperty(LOG4J_SUBDIRECTORY); + String subdirectory = configuration.getString(LOG4J_SUBDIRECTORY); if (subdirectory != null && subdirectory.length() > 0) { Enumeration ls = LogManager.getCurrentLoggers(); while (ls.hasMoreElements()) {