This is an automated email from the ASF dual-hosted git repository.
albumenj pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/dubbo.git
The following commit(s) were added to refs/heads/3.0 by this push:
new dcb2d703fb Optimize the logic of ConfigConfigurationAdapter to get
prefixed meta… (#10389)
dcb2d703fb is described below
commit dcb2d703fbc5c53bbd7e5be52b1b7554566a9481
Author: 灼华 <[email protected]>
AuthorDate: Mon Aug 1 14:41:47 2022 +0800
Optimize the logic of ConfigConfigurationAdapter to get prefixed meta…
(#10389)
* Optimize the logic of ConfigConfigurationAdapter to get prefixed metadata
* Remove unused import
* Rollback
* Rollback
---
.../org/apache/dubbo/config/AbstractConfig.java | 66 +++++++++------
.../apache/dubbo/config/ReferenceConfigBase.java | 9 ++-
.../org/apache/dubbo/config/RegistryConfig.java | 6 +-
.../org/apache/dubbo/config/ServiceConfigBase.java | 11 ++-
.../config/context/ConfigConfigurationAdapter.java | 9 +--
.../apache/dubbo/config/context/ConfigMode.java | 2 +-
.../apache/dubbo/config/AbstractConfigTest.java | 94 +++++++++++++++++++++-
.../apache/dubbo/config/RegistryConfigTest.java | 10 ++-
8 files changed, 161 insertions(+), 46 deletions(-)
diff --git
a/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractConfig.java
b/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractConfig.java
index 05e83a40e8..b341bdd0f9 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractConfig.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractConfig.java
@@ -162,6 +162,10 @@ public abstract class AbstractConfig implements
Serializable {
appendParameters0(parameters, config, null, false);
}
+ public static void appendAttributes(Map<String, String> parameters, Object
config, String prefix) {
+ appendParameters0(parameters, config, prefix, false);
+ }
+
private static void appendParameters0(Map<String, String> parameters,
Object config, String prefix, boolean asParameters) {
if (config == null) {
return;
@@ -261,6 +265,10 @@ public abstract class AbstractConfig implements
Serializable {
return "get" + name.substring(0, 1).toUpperCase() + name.substring(1);
}
+ private static String calculatePropertyToSetter(String name) {
+ return "set" + name.substring(0, 1).toUpperCase() + name.substring(1);
+ }
+
private static String calculatePropertyFromGetter(String name) {
int i = name.startsWith("get") ? 3 : 2;
return StringUtils.camelToSplitName(name.substring(i, i +
1).toLowerCase() + name.substring(i + 1), ".");
@@ -468,7 +476,7 @@ public abstract class AbstractConfig implements
Serializable {
if ("interfaceClass".equals(property) ||
"interfaceName".equals(property)) {
property = "interface";
}
- String setter = "set" + property.substring(0,
1).toUpperCase() + property.substring(1);
+ String setter = calculatePropertyToSetter(property);
Object value = method.invoke(annotation);
if (value != null &&
!value.equals(method.getDefaultValue())) {
Class<?> parameterType =
ReflectUtils.getBoxedClass(method.getReturnType());
@@ -496,7 +504,7 @@ public abstract class AbstractConfig implements
Serializable {
/**
* <p>
* <b>The new instance of the AbstractConfig subclass should return empty
metadata.</b>
- * The purpose is is to get the attributes set by the user instead of the
default value when the {@link #refresh()} method handles attribute overrides.
+ * The purpose is to get the attributes set by the user instead of the
default value when the {@link #refresh()} method handles attribute overrides.
* </p>
*
* <p><b>The default value of the field should be set in the {@link
#checkDefault()} method</b>,
@@ -514,8 +522,12 @@ public abstract class AbstractConfig implements
Serializable {
* @see AbstractConfig#appendParameters(Map, Object, String)
*/
public Map<String, String> getMetaData() {
+ return getMetaData(null);
+ }
+
+ public Map<String, String> getMetaData(String prefix) {
Map<String, String> metaData = new HashMap<>();
- appendAttributes(metaData, this);
+ appendAttributes(metaData, this, prefix);
return metaData;
}
@@ -656,14 +668,15 @@ public abstract class AbstractConfig implements
Serializable {
// Search props starts with PREFIX in order
String preferredPrefix = null;
- for (String prefix : getPrefixes()) {
+ List<String> prefixes = getPrefixes();
+ for (String prefix : prefixes) {
if (ConfigurationUtils.hasSubProperties(configurationMaps,
prefix)) {
preferredPrefix = prefix;
break;
}
}
if (preferredPrefix == null) {
- preferredPrefix = getPrefixes().get(0);
+ preferredPrefix = prefixes.get(0);
}
// Extract sub props (which key was starts with preferredPrefix)
Collection<Map<String, String>> instanceConfigMaps =
environment.getConfigurationMaps(this, preferredPrefix);
@@ -738,6 +751,19 @@ public abstract class AbstractConfig implements
Serializable {
} else if (isParametersSetter(method)) {
String propertyName = extractPropertyName(method.getName());
+ String value =
StringUtils.trim(configuration.getString(propertyName));
+ Map<String, String> parameterMap;
+ if (StringUtils.hasText(value)) {
+ parameterMap = StringUtils.parseParameters(value);
+ } else {
+ // in this case, maybe parameters.item3=value3.
+ parameterMap =
ConfigurationUtils.getSubProperties(properties, PARAMETERS);
+ }
+ Map<String, String> newMap = convert(parameterMap, "");
+ if (CollectionUtils.isEmptyMap(newMap)) {
+ continue;
+ }
+
// get old map from original obj
Map<String, String> oldMap = null;
try {
@@ -751,16 +777,6 @@ public abstract class AbstractConfig implements
Serializable {
}
- String value =
StringUtils.trim(configuration.getString(propertyName));
- Map<String, String> parameterMap;
- if (StringUtils.hasText(value)) {
- parameterMap = StringUtils.parseParameters(value);
- } else {
- // in this case, maybe parameters.item3=value3.
- parameterMap =
ConfigurationUtils.getSubProperties(properties, PARAMETERS);
- }
- Map<String, String> newMap = convert(parameterMap, "");
-
// if old map is null, directly set params
if (oldMap == null) {
invokeSetParameters(newMap, obj);
@@ -917,17 +933,15 @@ public abstract class AbstractConfig implements
Serializable {
buf.append(getTagName(getClass()));
for (Method method : getAttributedMethods()) {
try {
- if (MethodUtils.isGetter(method)) {
- String name = method.getName();
- String key = calculateAttributeFromGetter(name);
- Object value = method.invoke(this);
- if (value != null) {
- buf.append(' ');
- buf.append(key);
- buf.append("=\"");
- buf.append(key.equals("password") ? "******" :
value);
- buf.append('\"');
- }
+ String name = method.getName();
+ String key = calculateAttributeFromGetter(name);
+ Object value = method.invoke(this);
+ if (value != null) {
+ buf.append(' ');
+ buf.append(key);
+ buf.append("=\"");
+ buf.append(key.equals("password") ? "******" : value);
+ buf.append('\"');
}
} catch (Exception e) {
logger.warn(e.getMessage(), e);
diff --git
a/dubbo-common/src/main/java/org/apache/dubbo/config/ReferenceConfigBase.java
b/dubbo-common/src/main/java/org/apache/dubbo/config/ReferenceConfigBase.java
index f97cda53fc..e2c18094db 100644
---
a/dubbo-common/src/main/java/org/apache/dubbo/config/ReferenceConfigBase.java
+++
b/dubbo-common/src/main/java/org/apache/dubbo/config/ReferenceConfigBase.java
@@ -149,6 +149,11 @@ public abstract class ReferenceConfigBase<T> extends
AbstractReferenceConfig {
@Override
public Map<String, String> getMetaData() {
+ return getMetaData(null);
+ }
+
+ @Override
+ public Map<String, String> getMetaData(String prefix) {
Map<String, String> metaData = new HashMap<>();
ConsumerConfig consumer = this.getConsumer();
// consumer should be initialized at preProcessRefresh()
@@ -156,8 +161,8 @@ public abstract class ReferenceConfigBase<T> extends
AbstractReferenceConfig {
throw new IllegalStateException("Consumer is not initialized");
}
// use consumer attributes as default value
- appendAttributes(metaData, consumer);
- appendAttributes(metaData, this);
+ appendAttributes(metaData, consumer, prefix);
+ appendAttributes(metaData, this, prefix);
return metaData;
}
diff --git
a/dubbo-common/src/main/java/org/apache/dubbo/config/RegistryConfig.java
b/dubbo-common/src/main/java/org/apache/dubbo/config/RegistryConfig.java
index f3863a589a..e4fb3a9d6e 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/config/RegistryConfig.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/config/RegistryConfig.java
@@ -88,7 +88,7 @@ public class RegistryConfig extends AbstractConfig {
private String zone;
/**
- * The group the services registry in
+ * The group that services registry in
*/
private String group;
@@ -125,12 +125,12 @@ public class RegistryConfig extends AbstractConfig {
private Boolean dynamic;
/**
- * Whether to export service on the register center
+ * Whether to allow exporting service on the register center
*/
private Boolean register;
/**
- * Whether allow to subscribe service on the register center
+ * Whether to allow subscribing service on the register center
*/
private Boolean subscribe;
diff --git
a/dubbo-common/src/main/java/org/apache/dubbo/config/ServiceConfigBase.java
b/dubbo-common/src/main/java/org/apache/dubbo/config/ServiceConfigBase.java
index ef6d5d696b..84a6076351 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/config/ServiceConfigBase.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/config/ServiceConfigBase.java
@@ -234,16 +234,21 @@ public abstract class ServiceConfigBase<T> extends
AbstractServiceConfig {
@Override
public Map<String, String> getMetaData() {
+ return getMetaData(null);
+ }
+
+ @Override
+ public Map<String, String> getMetaData(String prefix) {
Map<String, String> metaData = new HashMap<>();
ProviderConfig provider = this.getProvider();
- // provider should be inited at preProcessRefresh()
+ // provider should be initialized at preProcessRefresh()
if (isRefreshed() && provider == null) {
throw new IllegalStateException("Provider is not initialized");
}
// use provider attributes as default value
- appendAttributes(metaData, provider);
+ appendAttributes(metaData, provider, prefix);
// Finally, put the service's attributes, overriding previous
attributes
- appendAttributes(metaData, this);
+ appendAttributes(metaData, this, prefix);
return metaData;
}
diff --git
a/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigConfigurationAdapter.java
b/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigConfigurationAdapter.java
index 57faf25495..2b141e8a8b 100644
---
a/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigConfigurationAdapter.java
+++
b/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigConfigurationAdapter.java
@@ -20,7 +20,6 @@ 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;
/**
@@ -31,14 +30,10 @@ public class ConfigConfigurationAdapter implements
Configuration {
private Map<String, String> metaData;
public ConfigConfigurationAdapter(AbstractConfig config, String prefix) {
- Map<String, String> configMetadata = config.getMetaData();
if (StringUtils.hasText(prefix)) {
- metaData = new HashMap<>(configMetadata.size(), 1.0f);
- for (Map.Entry<String, String> entry : configMetadata.entrySet()) {
- metaData.put(prefix + "." + entry.getKey(), entry.getValue());
- }
+ metaData = config.getMetaData(prefix);
} else {
- metaData = configMetadata;
+ metaData = config.getMetaData();
}
}
diff --git
a/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigMode.java
b/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigMode.java
index 5ca32398be..90580c22e0 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigMode.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigMode.java
@@ -22,7 +22,7 @@ package org.apache.dubbo.config.context;
*/
public enum ConfigMode {
/**
- * Strict mode: accept only one config for unique config type, throw
exceptions if found more than one configs for an unique config type.
+ * Strict mode: accept only one config for unique config type, throw
exceptions if found more than one config for a unique config type.
*/
STRICT,
diff --git
a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractConfigTest.java
b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractConfigTest.java
index f9e87663f1..6fc328fcb1 100644
---
a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractConfigTest.java
+++
b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/AbstractConfigTest.java
@@ -19,11 +19,14 @@ package org.apache.dubbo.config;
import org.apache.dubbo.common.utils.StringUtils;
import org.apache.dubbo.config.api.Greeting;
import org.apache.dubbo.config.bootstrap.DubboBootstrap;
+import org.apache.dubbo.config.context.ConfigMode;
import org.apache.dubbo.config.support.Nested;
import org.apache.dubbo.config.support.Parameter;
import org.apache.dubbo.config.utils.ConfigValidationUtils;
import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.ScopeModel;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
@@ -173,7 +176,7 @@ public class AbstractConfigTest {
@Test
public void testAppendAttributes1() throws Exception {
- ParameterConfig config = new ParameterConfig(1, "hello/world", 30,
"password");
+ ParameterConfig config = new ParameterConfig(1, "hello/world", 30,
"password","BEIJING");
Map<String, String> parameters = new HashMap<>();
AbstractConfig.appendParameters(parameters, config);
@@ -188,6 +191,8 @@ public class AbstractConfigTest {
Assertions.assertEquals(String.valueOf(config.getNumber()),
attributes.get("number"));
Assertions.assertEquals(String.valueOf(config.getAge()),
attributes.get("age"));
Assertions.assertEquals(StringUtils.encodeParameters(config.getParameters()),
attributes.get("parameters"));
+ Assertions.assertTrue(parameters.containsKey("detail.address"));//
detailAddress -> detail.address
+ Assertions.assertTrue(attributes.containsKey("detail-address"));//
detailAddress -> detail-address
}
@Test
@@ -514,6 +519,65 @@ public class AbstractConfigTest {
}
}
+ @Test
+ public void testRefreshParametersWithOverrideConfigMode() {
+ FrameworkModel frameworkModel = new FrameworkModel();
+ try {
+ // test OVERRIDE_ALL configMode
+ {
+ SysProps.setProperty(ConfigKeys.DUBBO_CONFIG_MODE,
ConfigMode.OVERRIDE_ALL.name());
+ SysProps.setProperty("dubbo.override.address",
"system://127.0.0.1:2181");
+ SysProps.setProperty("dubbo.override.protocol", "system");
+ SysProps.setProperty("dubbo.override.parameters",
"[{key1:systemValue1},{key2:systemValue2}]");
+
+ ApplicationModel applicationModel1 =
frameworkModel.newApplication();
+ OverrideConfig overrideConfig = new
OverrideConfig(applicationModel1);
+ overrideConfig.setAddress("override-config://127.0.0.1:2181");
+ overrideConfig.setProtocol("override-config");
+ Map<String, String> parameters = new HashMap<>();
+ parameters.put("key1", "value1");
+ parameters.put("key3", "value3");
+ overrideConfig.setParameters(parameters);
+
+ // overrideConfig's config is overridden by system config
+ overrideConfig.refresh();
+ Assertions.assertEquals(overrideConfig.getAddress(),
"system://127.0.0.1:2181");
+ Assertions.assertEquals(overrideConfig.getProtocol(),
"system");
+ Assertions.assertEquals(overrideConfig.getParameters(),
+
StringUtils.parseParameters("[{key1:systemValue1},{key2:systemValue2},{key3:value3}]"));
+
+ }
+ // test OVERRIDE_IF_ABSENT configMode
+ {
+ SysProps.setProperty(ConfigKeys.DUBBO_CONFIG_MODE,
ConfigMode.OVERRIDE_IF_ABSENT.name());
+ SysProps.setProperty("dubbo.override.address",
"system://127.0.0.1:2181");
+ SysProps.setProperty("dubbo.override.protocol", "system");
+ SysProps.setProperty("dubbo.override.parameters",
"[{key1:systemValue1},{key2:systemValue2}]");
+ SysProps.setProperty("dubbo.override.key", "systemKey");
+
+ ApplicationModel applicationModel =
frameworkModel.newApplication();
+ OverrideConfig overrideConfig = new
OverrideConfig(applicationModel);
+ overrideConfig.setAddress("override-config://127.0.0.1:2181");
+ overrideConfig.setProtocol("override-config");
+ Map<String, String> parameters = new HashMap<>();
+ parameters.put("key1", "value1");
+ parameters.put("key3", "value3");
+ overrideConfig.setParameters(parameters);
+
+ // overrideConfig's config is overridden/set by system config
only when the overrideConfig's config is absent/empty
+ overrideConfig.refresh();
+ Assertions.assertEquals(overrideConfig.getAddress(),
"override-config://127.0.0.1:2181");
+ Assertions.assertEquals(overrideConfig.getProtocol(),
"override-config");
+ Assertions.assertEquals(overrideConfig.getKey(), "systemKey");
+ Assertions.assertEquals(overrideConfig.getParameters(),
+
StringUtils.parseParameters("[{key1:value1},{key2:systemValue2},{key3:value3}]"));
+ }
+
+ } finally {
+ frameworkModel.destroy();
+ }
+ }
+
@Test
public void testOnlyPrefixedKeyTakeEffect() {
try {
@@ -561,6 +625,13 @@ public class AbstractConfigTest {
Assertions.assertEquals("override-config", metaData.get("exclude"));
Assertions.assertNull(metaData.get("key"));
Assertions.assertNull(metaData.get("key2"));
+
+ // with prefix
+ Map<String, String> prefixMetadata =
overrideConfig.getMetaData(OverrideConfig.getTypePrefix(OverrideConfig.class));
+ Assertions.assertEquals("override-config://127.0.0.1:2181",
prefixMetadata.get("dubbo.override.address"));
+ Assertions.assertEquals("override-config",
prefixMetadata.get("dubbo.override.protocol"));
+ Assertions.assertEquals("override-config://",
prefixMetadata.get("dubbo.override.escape"));
+ Assertions.assertEquals("override-config",
prefixMetadata.get("dubbo.override.exclude"));
}
@Test
@@ -618,6 +689,13 @@ public class AbstractConfigTest {
public String notConflictKey2;
protected Map<String, String> parameters;
+ public OverrideConfig() {
+ }
+
+ public OverrideConfig(ScopeModel scopeModel) {
+ super(scopeModel);
+ }
+
public String getAddress() {
return address;
}
@@ -791,15 +869,29 @@ public class AbstractConfigTest {
private String name;
private int age;
private String secret;
+ private String detailAddress;
ParameterConfig() {
}
ParameterConfig(int number, String name, int age, String secret) {
+ this(number, name, age, secret, "");
+ }
+
+ ParameterConfig(int number, String name, int age, String secret,String
detailAddress) {
this.number = number;
this.name = name;
this.age = age;
this.secret = secret;
+ this.detailAddress = detailAddress;
+ }
+
+ public String getDetailAddress() {
+ return detailAddress;
+ }
+
+ public void setDetailAddress(String detailAddress) {
+ this.detailAddress = detailAddress;
}
@Parameter(key = "num", append = true)
diff --git
a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/RegistryConfigTest.java
b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/RegistryConfigTest.java
index d2b7ba5662..d8fd191a32 100644
---
a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/RegistryConfigTest.java
+++
b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/RegistryConfigTest.java
@@ -65,9 +65,13 @@ public class RegistryConfigTest {
@Test
public void testAddress() throws Exception {
RegistryConfig registry = new RegistryConfig();
- registry.setAddress("localhost");
- assertThat(registry.getAddress(), equalTo("localhost"));
- Map<String, String> parameters = new HashMap<String, String>();
+
registry.setAddress("zookeeper://mrh:123@localhost:9103/registry?backup=localhost:9104&k1=v1");
+ assertThat(registry.getAddress(),
equalTo("zookeeper://mrh:123@localhost:9103/registry?backup=localhost:9104&k1=v1"));
+ assertThat(registry.getProtocol(), equalTo("zookeeper"));
+ assertThat(registry.getUsername(), equalTo("mrh"));
+ assertThat(registry.getPassword(), equalTo("123"));
+ assertThat(registry.getParameters().get("k1"), equalTo("v1"));
+ Map<String, String> parameters = new HashMap<>();
RegistryConfig.appendParameters(parameters, registry);
assertThat(parameters, not(hasKey("address")));
}
