Refactor, configuration override does not work properly. (#5709)

This commit is contained in:
ken.lj 2020-02-20 15:00:26 +08:00 committed by GitHub
parent ff14005475
commit 12d99d686f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 134 additions and 173 deletions

View File

@ -1,52 +0,0 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.dubbo.common.config;
import org.apache.dubbo.common.utils.StringUtils;
/**
* This is an abstraction specially customized for the sequence Dubbo retrieves properties.
*/
public abstract class AbstractPrefixConfiguration implements Configuration {
protected String id;
protected String prefix;
public AbstractPrefixConfiguration(String prefix, String id) {
if (StringUtils.isNotEmpty(prefix) && !prefix.endsWith(".")) {
this.prefix = prefix + ".";
} else {
this.prefix = prefix;
}
this.id = id;
}
@Override
public Object getProperty(String key, Object defaultValue) {
Object value = null;
if (StringUtils.isNotEmpty(prefix)) {
if (StringUtils.isNotEmpty(id)) {
value = getInternalProperty(prefix + id + "." + key);
}
if (value == null) {
value = getInternalProperty(prefix + key);
}
} else {
value = getInternalProperty(key);
}
return value != null ? value : defaultValue;
}
}

View File

@ -18,27 +18,41 @@ package org.apache.dubbo.common.config;
import org.apache.dubbo.common.logger.Logger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.utils.StringUtils;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
/**
*
* This is an abstraction specially customized for the sequence Dubbo retrieves properties.
*/
public class CompositeConfiguration implements Configuration {
private Logger logger = LoggerFactory.getLogger(CompositeConfiguration.class);
private String id;
private String prefix;
/**
* List holding all the configuration
*/
private List<Configuration> configList = new LinkedList<Configuration>();
public CompositeConfiguration() {
this(null, null);
}
public CompositeConfiguration(String prefix, String id) {
if (StringUtils.isNotEmpty(prefix) && !prefix.endsWith(".")) {
this.prefix = prefix + ".";
} else {
this.prefix = prefix;
}
this.id = id;
}
public CompositeConfiguration(Configuration... configurations) {
this();
if (configurations != null && configurations.length > 0) {
Arrays.stream(configurations).filter(config -> !configList.contains(config)).forEach(configList::add);
}
@ -83,4 +97,20 @@ public class CompositeConfiguration implements Configuration {
public boolean containsKey(String key) {
return configList.stream().anyMatch(c -> c.containsKey(key));
}
@Override
public Object getProperty(String key, Object defaultValue) {
Object value = null;
if (StringUtils.isNotEmpty(prefix)) {
if (StringUtils.isNotEmpty(id)) {
value = getInternalProperty(prefix + id + "." + key);
}
if (value == null) {
value = getInternalProperty(prefix + key);
}
} else {
value = getInternalProperty(key);
}
return value != null ? value : defaultValue;
}
}

View File

@ -17,12 +17,12 @@
package org.apache.dubbo.common.config;
import org.apache.dubbo.common.config.configcenter.DynamicConfiguration;
import org.apache.dubbo.common.constants.CommonConstants;
import org.apache.dubbo.common.context.FrameworkExt;
import org.apache.dubbo.common.context.LifecycleAdapter;
import org.apache.dubbo.common.extension.DisableInject;
import org.apache.dubbo.common.utils.StringUtils;
import org.apache.dubbo.config.AbstractConfig;
import org.apache.dubbo.config.ConfigCenterConfig;
import org.apache.dubbo.config.context.ConfigConfigurationAdapter;
import org.apache.dubbo.config.context.ConfigManager;
import org.apache.dubbo.rpc.model.ApplicationModel;
@ -31,15 +31,19 @@ import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
public class Environment extends LifecycleAdapter implements FrameworkExt {
public static final String NAME = "environment";
private Map<String, PropertiesConfiguration> propertiesConfigs = new ConcurrentHashMap<>();
private Map<String, SystemConfiguration> systemConfigs = new ConcurrentHashMap<>();
private Map<String, EnvironmentConfiguration> environmentConfigs = new ConcurrentHashMap<>();
private Map<String, InmemoryConfiguration> externalConfigs = new ConcurrentHashMap<>();
private Map<String, InmemoryConfiguration> appExternalConfigs = new ConcurrentHashMap<>();
private final PropertiesConfiguration propertiesConfiguration;
private final SystemConfiguration systemConfiguration;
private final EnvironmentConfiguration environmentConfiguration;
private final InmemoryConfiguration externalConfiguration;
private final InmemoryConfiguration appExternalConfiguration;
private final ConcurrentMap<AbstractConfig, CompositeConfiguration> prefixedConfigurations = new ConcurrentHashMap<>();
private CompositeConfiguration globalConfiguration;
private Map<String, String> externalConfigurationMap = new HashMap<>();
private Map<String, String> appExternalConfigurationMap = new HashMap<>();
@ -48,6 +52,14 @@ public class Environment extends LifecycleAdapter implements FrameworkExt {
private DynamicConfiguration dynamicConfiguration;
public Environment() {
this.propertiesConfiguration = new PropertiesConfiguration();
this.systemConfiguration = new SystemConfiguration();
this.environmentConfiguration = new EnvironmentConfiguration();
this.externalConfiguration = new InmemoryConfiguration();
this.appExternalConfiguration = new InmemoryConfiguration();
}
@Override
public void initialize() throws IllegalStateException {
ConfigManager configManager = ApplicationModel.getConfigManager();
@ -58,34 +70,9 @@ public class Environment extends LifecycleAdapter implements FrameworkExt {
this.setAppExternalConfigMap(config.getAppExternalConfiguration());
}
});
}
public PropertiesConfiguration getPropertiesConfig(String prefix, String id) {
return propertiesConfigs.computeIfAbsent(toKey(prefix, id), k -> new PropertiesConfiguration(prefix, id));
}
public SystemConfiguration getSystemConfig(String prefix, String id) {
return systemConfigs.computeIfAbsent(toKey(prefix, id), k -> new SystemConfiguration(prefix, id));
}
public InmemoryConfiguration getExternalConfig(String prefix, String id) {
return externalConfigs.computeIfAbsent(toKey(prefix, id), k -> {
InmemoryConfiguration configuration = new InmemoryConfiguration(prefix, id);
configuration.setProperties(externalConfigurationMap);
return configuration;
});
}
public InmemoryConfiguration getAppExternalConfig(String prefix, String id) {
return appExternalConfigs.computeIfAbsent(toKey(prefix, id), k -> {
InmemoryConfiguration configuration = new InmemoryConfiguration(prefix, id);
configuration.setProperties(appExternalConfigurationMap);
return configuration;
});
}
public EnvironmentConfiguration getEnvironmentConfig(String prefix, String id) {
return environmentConfigs.computeIfAbsent(toKey(prefix, id), k -> new EnvironmentConfiguration(prefix, id));
this.externalConfiguration.setProperties(externalConfigurationMap);
this.appExternalConfiguration.setProperties(appExternalConfigurationMap);
}
@DisableInject
@ -119,46 +106,65 @@ public class Environment extends LifecycleAdapter implements FrameworkExt {
}
/**
* Create new instance for each call, since it will be called only at startup, I think there's no big deal of the potential cost.
* Otherwise, if use cache, we should make sure each Config has a unique id which is difficult to guarantee because is on the user's side,
* especially when it comes to ServiceConfig and ReferenceConfig.
* At start-up, Dubbo is driven by various configuration, such as Application, Registry, Protocol, etc.
* All configurations will be converged into a data bus - URL, and then drive the subsequent process.
* <p>
* At present, there are many configuration sources, including AbstractConfig (API, XML, annotation), - D, config center, etc.
* This method helps us to filter out the most priority values from various configuration sources.
*
* @param prefix
* @param id
* @param config
* @return
*/
public CompositeConfiguration getConfiguration(String prefix, String id) {
CompositeConfiguration compositeConfiguration = new CompositeConfiguration();
// Config center has the highest priority
compositeConfiguration.addConfiguration(this.getSystemConfig(prefix, id));
compositeConfiguration.addConfiguration(this.getEnvironmentConfig(prefix, id));
compositeConfiguration.addConfiguration(this.getAppExternalConfig(prefix, id));
compositeConfiguration.addConfiguration(this.getExternalConfig(prefix, id));
compositeConfiguration.addConfiguration(this.getPropertiesConfig(prefix, id));
return compositeConfiguration;
public synchronized CompositeConfiguration getPrefixedConfiguration(AbstractConfig config) {
// CompositeConfiguration prefixedConfiguration =
// prefixedConfigurations.putIfAbsent(config, new CompositeConfiguration(config.getPrefix(), config.getId()));
// if (prefixedConfiguration != null) {
// return prefixedConfiguration;
// }
// prefixedConfiguration = prefixedConfigurations.get(config);
CompositeConfiguration prefixedConfiguration = new CompositeConfiguration(config.getPrefix(), config.getId());
Configuration configuration = new ConfigConfigurationAdapter(config);
if (this.isConfigCenterFirst()) {
// The sequence would be: SystemConfiguration -> AppExternalConfiguration -> ExternalConfiguration -> AbstractConfig -> PropertiesConfiguration
// Config center has the highest priority
prefixedConfiguration.addConfiguration(systemConfiguration);
prefixedConfiguration.addConfiguration(environmentConfiguration);
prefixedConfiguration.addConfiguration(appExternalConfiguration);
prefixedConfiguration.addConfiguration(externalConfiguration);
prefixedConfiguration.addConfiguration(configuration);
prefixedConfiguration.addConfiguration(propertiesConfiguration);
} else {
// The sequence would be: SystemConfiguration -> AbstractConfig -> AppExternalConfiguration -> ExternalConfiguration -> PropertiesConfiguration
// Config center has the highest priority
prefixedConfiguration.addConfiguration(systemConfiguration);
prefixedConfiguration.addConfiguration(environmentConfiguration);
prefixedConfiguration.addConfiguration(configuration);
prefixedConfiguration.addConfiguration(appExternalConfiguration);
prefixedConfiguration.addConfiguration(externalConfiguration);
prefixedConfiguration.addConfiguration(propertiesConfiguration);
}
return prefixedConfiguration;
}
/**
* There are two ways to get configuration during exposure / reference or at runtime:
* 1. URL, The value in the URL is relatively fixed. we can get value directly.
* 2. The configuration exposed in this method is convenient for us to query the latest values from multiple
* prioritized sources, it also guarantees that configs changed dynamically can take effect on the fly.
*/
public Configuration getConfiguration() {
return getConfiguration(null, null);
}
private static String toKey(String prefix, String id) {
StringBuilder sb = new StringBuilder();
if (StringUtils.isNotEmpty(prefix)) {
sb.append(prefix);
if (globalConfiguration == null) {
globalConfiguration = new CompositeConfiguration();
if (dynamicConfiguration != null) {
globalConfiguration.addConfiguration(dynamicConfiguration);
}
globalConfiguration.addConfiguration(systemConfiguration);
globalConfiguration.addConfiguration(environmentConfiguration);
globalConfiguration.addConfiguration(appExternalConfiguration);
globalConfiguration.addConfiguration(externalConfiguration);
globalConfiguration.addConfiguration(propertiesConfiguration);
}
if (StringUtils.isNotEmpty(id)) {
sb.append(id);
}
if (sb.length() > 0 && sb.charAt(sb.length() - 1) != '.') {
sb.append(".");
}
if (sb.length() > 0) {
return sb.toString();
}
return CommonConstants.DUBBO;
return globalConfiguration;
}
public boolean isConfigCenterFirst() {
@ -187,13 +193,13 @@ public class Environment extends LifecycleAdapter implements FrameworkExt {
// For test
public void clearExternalConfigs() {
this.externalConfigs.clear();
this.externalConfiguration.clear();
this.externalConfigurationMap.clear();
}
// For test
public void clearAppExternalConfigs() {
this.appExternalConfigs.clear();
this.appExternalConfiguration.clear();
this.appExternalConfigurationMap.clear();
}
}

View File

@ -21,15 +21,7 @@ import org.apache.dubbo.common.utils.StringUtils;
/**
* Configuration from system environment
*/
public class EnvironmentConfiguration extends AbstractPrefixConfiguration {
public EnvironmentConfiguration(String prefix, String id) {
super(prefix, id);
}
public EnvironmentConfiguration() {
this(null, null);
}
public class EnvironmentConfiguration implements Configuration {
@Override
public Object getInternalProperty(String key) {

View File

@ -22,19 +22,11 @@ import java.util.Map;
/**
* In-memory configuration
*/
public class InmemoryConfiguration extends AbstractPrefixConfiguration {
public class InmemoryConfiguration implements Configuration {
// stores the configuration key-value pairs
private Map<String, String> store = new LinkedHashMap<>();
public InmemoryConfiguration(String prefix, String id) {
super(prefix, id);
}
public InmemoryConfiguration() {
this(null, null);
}
@Override
public Object getInternalProperty(String key) {
return store.get(key);
@ -64,4 +56,9 @@ public class InmemoryConfiguration extends AbstractPrefixConfiguration {
this.store = properties;
}
}
// for unit test
public void clear() {
this.store.clear();
}
}

View File

@ -27,11 +27,9 @@ import java.util.Set;
/**
* Configuration from system properties and dubbo.properties
*/
public class PropertiesConfiguration extends AbstractPrefixConfiguration {
public PropertiesConfiguration(String prefix, String id) {
super(prefix, id);
public class PropertiesConfiguration implements Configuration {
public PropertiesConfiguration() {
ExtensionLoader<OrderedPropertiesProvider> propertiesProviderExtensionLoader = ExtensionLoader.getExtensionLoader(OrderedPropertiesProvider.class);
Set<String> propertiesProviderNames = propertiesProviderExtensionLoader.getSupportedExtensions();
if (propertiesProviderNames == null || propertiesProviderNames.isEmpty()) {
@ -59,10 +57,6 @@ public class PropertiesConfiguration extends AbstractPrefixConfiguration {
ConfigUtils.setProperties(properties);
}
public PropertiesConfiguration() {
this(null, null);
}
@Override
public Object getInternalProperty(String key) {
return ConfigUtils.getProperty(key);

View File

@ -19,18 +19,11 @@ package org.apache.dubbo.common.config;
/**
* FIXME: is this really necessary? PropertiesConfiguration should have already covered this:
* @see PropertiesConfiguration
*
* @See ConfigUtils#getProperty(String)
* @see PropertiesConfiguration
*/
public class SystemConfiguration extends AbstractPrefixConfiguration {
public SystemConfiguration(String prefix, String id) {
super(prefix, id);
}
public SystemConfiguration() {
this(null, null);
}
public class SystemConfiguration implements Configuration {
@Override
public Object getInternalProperty(String key) {

View File

@ -18,7 +18,6 @@ package org.apache.dubbo.config;
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.constants.CommonConstants;
import org.apache.dubbo.common.logger.Logger;
@ -28,7 +27,6 @@ import org.apache.dubbo.common.utils.CollectionUtils;
import org.apache.dubbo.common.utils.MethodUtils;
import org.apache.dubbo.common.utils.ReflectUtils;
import org.apache.dubbo.common.utils.StringUtils;
import org.apache.dubbo.config.context.ConfigConfigurationAdapter;
import org.apache.dubbo.config.context.ConfigManager;
import org.apache.dubbo.config.support.Parameter;
import org.apache.dubbo.rpc.model.ApplicationModel;
@ -459,16 +457,7 @@ public abstract class AbstractConfig implements Serializable {
public void refresh() {
Environment env = ApplicationModel.getEnvironment();
try {
CompositeConfiguration compositeConfiguration = env.getConfiguration(getPrefix(), getId());
Configuration config = new ConfigConfigurationAdapter(this);
if (env.isConfigCenterFirst()) {
// The sequence would be: SystemConfiguration -> AppExternalConfiguration -> ExternalConfiguration -> AbstractConfig -> PropertiesConfiguration
compositeConfiguration.addConfiguration(4, config);
} else {
// The sequence would be: SystemConfiguration -> AbstractConfig -> AppExternalConfiguration -> ExternalConfiguration -> PropertiesConfiguration
compositeConfiguration.addConfiguration(2, config);
}
CompositeConfiguration compositeConfiguration = env.getPrefixedConfiguration(this);
// loop methods, get override value and set the new value back to method
Method[] methods = getClass().getMethods();
for (Method method : methods) {

View File

@ -17,8 +17,10 @@
package org.apache.dubbo.config.context;
import org.apache.dubbo.common.config.Configuration;
import org.apache.dubbo.common.utils.StringUtils;
import org.apache.dubbo.config.AbstractConfig;
import java.util.HashMap;
import java.util.Map;
/**
@ -29,7 +31,13 @@ public class ConfigConfigurationAdapter implements Configuration {
private Map<String, String> metaData;
public ConfigConfigurationAdapter(AbstractConfig config) {
this.metaData = config.getMetaData();
Map<String, String> configMetadata = config.getMetaData();
metaData = new HashMap<>(configMetadata.size());
for (Map.Entry<String, String> entry : configMetadata.entrySet()) {
String prefix = config.getPrefix().endsWith(".") ? config.getPrefix() : config.getPrefix() + ".";
String id = StringUtils.isEmpty(config.getId()) ? "" : config.getId() + ".";
metaData.put(prefix + id + entry.getKey(), entry.getValue());
}
}
@Override

View File

@ -24,8 +24,8 @@ public class PropertiesConfigurationTest {
@Test
public void testOrderPropertiesProviders() {
PropertiesConfiguration configuration = new PropertiesConfiguration("test", null);
Assertions.assertTrue(configuration.getInternalProperty("testKey").equals("999"));
PropertiesConfiguration configuration = new PropertiesConfiguration();
Assertions.assertEquals("999", configuration.getInternalProperty("testKey"));
}
}

View File

@ -305,6 +305,7 @@ public class AbstractConfigTest {
// @Parameter(key="key2", useKeyAsProperty=true)
external.put("dubbo.override.key2", "external");
ApplicationModel.getEnvironment().setExternalConfigMap(external);
ApplicationModel.getEnvironment().initialize();
System.setProperty("dubbo.override.address", "system://127.0.0.1:2181");
System.setProperty("dubbo.override.protocol", "system");
@ -401,6 +402,7 @@ public class AbstractConfigTest {
// @Parameter(key="key2", useKeyAsProperty=true)
external.put("dubbo.override.key2", "external");
ApplicationModel.getEnvironment().setExternalConfigMap(external);
ApplicationModel.getEnvironment().initialize();
overrideConfig.refresh();
@ -435,6 +437,7 @@ public class AbstractConfigTest {
// @Parameter(key="key2", useKeyAsProperty=true)
external.put("dubbo.override.key2", "external");
ApplicationModel.getEnvironment().setExternalConfigMap(external);
ApplicationModel.getEnvironment().initialize();
ConfigCenterConfig configCenter = new ConfigCenterConfig();
overrideConfig.setConfigCenter(configCenter);
@ -464,6 +467,7 @@ public class AbstractConfigTest {
Map<String, String> external = new HashMap<>();
external.put("dubbo.override.parameters", "[{key3:value3},{key4:value4},{key2:value5}]");
ApplicationModel.getEnvironment().setExternalConfigMap(external);
ApplicationModel.getEnvironment().initialize();
ConfigCenterConfig configCenter = new ConfigCenterConfig();
overrideConfig.setConfigCenter(configCenter);