You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by al...@apache.org on 2021/07/11 13:08:18 UTC
[dubbo] branch 3.0 updated: [3.0] improve duplicated config
checking and add tests for reference annotation. (#8253)
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 8b470e5 [3.0] improve duplicated config checking and add tests for reference annotation. (#8253)
8b470e5 is described below
commit 8b470e518caafddcb7f876d3eb21a637079354ea
Author: Gong Dewei <ky...@qq.com>
AuthorDate: Sun Jul 11 21:08:00 2021 +0800
[3.0] improve duplicated config checking and add tests for reference annotation. (#8253)
* Add tests for reference annotation
* Improve duplicated config checking
* extract config "dubbo.config.ignore-duplicated-interface"
---
.../java/org/apache/dubbo/config/ConfigKeys.java | 5 +
.../apache/dubbo/config/context/ConfigManager.java | 115 +++++++++++++--------
.../JavaConfigReferenceBeanConditionalTest4.java | 111 ++++++++++++++++++++
.../config/spring/reference/ReferenceKeyTest.java | 2 +-
.../javaconfig/JavaConfigReferenceBeanTest.java | 63 +++++++++++
5 files changed, 251 insertions(+), 45 deletions(-)
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/config/ConfigKeys.java b/dubbo-common/src/main/java/org/apache/dubbo/config/ConfigKeys.java
index a753c63..d1b200d 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/config/ConfigKeys.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/config/ConfigKeys.java
@@ -43,5 +43,10 @@ public interface ConfigKeys {
*/
String DUBBO_CONFIG_IGNORE_INVALID_METHOD_CONFIG = "dubbo.config.ignore-invalid-method-config";
+ /**
+ * Ignore duplicated interface (service/reference) config. Default value is false.
+ */
+ String DUBBO_CONFIG_IGNORE_DUPLICATED_INTERFACE = "dubbo.config.ignore-duplicated-interface";
+
}
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigManager.java b/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigManager.java
index 3782114..56d72a3 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigManager.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/config/context/ConfigManager.java
@@ -16,6 +16,7 @@
*/
package org.apache.dubbo.config.context;
+import org.apache.dubbo.common.config.CompositeConfiguration;
import org.apache.dubbo.common.context.FrameworkExt;
import org.apache.dubbo.common.context.LifecycleAdapter;
import org.apache.dubbo.common.extension.DisableInject;
@@ -81,14 +82,16 @@ public class ConfigManager extends LifecycleAdapter implements FrameworkExt {
final Map<String, Map<String, AbstractConfig>> configsCache = newMap();
- private Map<String, ReferenceConfigBase> referenceConfigCache = new HashMap<>();
+ private Map<String, AbstractInterfaceConfig> referenceConfigCache = new HashMap<>();
- private Map<String, ServiceConfigBase> serviceConfigCache = new HashMap<>();
+ private Map<String, AbstractInterfaceConfig> serviceConfigCache = new HashMap<>();
private Set<AbstractConfig> duplicatedConfigs = new HashSet<>();
private ConfigMode configMode = ConfigMode.STRICT;
+ private boolean ignoreDuplicatedInterface = false;
+
private static Map<Class, AtomicInteger> configIdIndexes = new ConcurrentHashMap<>();
private static Set<Class<? extends AbstractConfig>> uniqueConfigTypes = new ConcurrentHashSet<>();
@@ -113,18 +116,24 @@ public class ConfigManager extends LifecycleAdapter implements FrameworkExt {
@Override
public void initialize() throws IllegalStateException {
- String configModeStr = null;
+ CompositeConfiguration configuration = ApplicationModel.getEnvironment().getConfiguration();
+ String configModeStr = (String) configuration.getProperty(DUBBO_CONFIG_MODE);
try {
- configModeStr = (String) ApplicationModel.getEnvironment().getConfiguration().getProperty(DUBBO_CONFIG_MODE);
if (StringUtils.hasText(configModeStr)) {
this.configMode = ConfigMode.valueOf(configModeStr.toUpperCase());
}
- logger.info("Dubbo config mode: " + configMode);
} catch (Exception e) {
String msg = "Illegal '" + DUBBO_CONFIG_MODE + "' config value [" + configModeStr + "], available values " + Arrays.toString(ConfigMode.values());
logger.error(msg, e);
throw new IllegalArgumentException(msg, e);
}
+
+ String ignoreDuplicatedInterfaceStr = (String) configuration
+ .getProperty(ConfigKeys.DUBBO_CONFIG_IGNORE_DUPLICATED_INTERFACE);
+ if (ignoreDuplicatedInterfaceStr != null) {
+ this.ignoreDuplicatedInterface = Boolean.parseBoolean(ignoreDuplicatedInterfaceStr);
+ }
+ logger.info("Dubbo config mode: " + configMode +", ignore duplicated interface: " + ignoreDuplicatedInterface);
}
@@ -641,38 +650,17 @@ public class ConfigManager extends LifecycleAdapter implements FrameworkExt {
return config;
}
- // check duplicated config
+ // check duplicated configs
// TODO Is there any problem with ignoring duplicate and equivalent but different ReferenceConfig instances?
- if (config instanceof ReferenceConfigBase) {
- // special check service and reference config, speed up the processing of a large number of instances
- ReferenceConfigBase<?> referenceConfig = (ReferenceConfigBase<?>) config;
- String uniqueServiceName = referenceConfig.getUniqueServiceName();
- ReferenceConfigBase prevReferenceConfig = referenceConfigCache.putIfAbsent(uniqueServiceName, referenceConfig);
- if (prevReferenceConfig != null) {
- if (prevReferenceConfig == config) {
- return config;
- }
- if (isIgnoreDuplicateService(uniqueServiceName, prevReferenceConfig, config)) {
- return (C) prevReferenceConfig;
- }
- }
- } else if (config instanceof ServiceConfigBase) {
- ServiceConfigBase serviceConfig = (ServiceConfigBase) config;
- String uniqueServiceName = serviceConfig.getUniqueServiceName();
- ServiceConfigBase prevServiceConfig = serviceConfigCache.putIfAbsent(uniqueServiceName, serviceConfig);
- if (prevServiceConfig != null) {
- if (prevServiceConfig == config) {
- return config;
- }
- if (isIgnoreDuplicateService(uniqueServiceName, prevServiceConfig, config)) {
- return (C) prevServiceConfig;
- }
+ // special check service and reference config by unique service name, speed up the processing of large number of instances
+ if (config instanceof ReferenceConfigBase || config instanceof ServiceConfigBase) {
+ C existedConfig = (C) checkDuplicatedInterfaceConfig((AbstractInterfaceConfig) config);
+ if (existedConfig != null) {
+ return existedConfig;
}
} else {
// find by value
- Optional<C> prevConfig = configsMap.values().stream()
- .filter(val -> isEquals(val, config))
- .findFirst();
+ Optional<C> prevConfig = findConfigByValue(configsMap.values(), config);
if (prevConfig.isPresent()) {
if (prevConfig.get() == config) {
// the new one is same as existing one
@@ -736,20 +724,59 @@ public class ConfigManager extends LifecycleAdapter implements FrameworkExt {
return config;
}
- private <C extends AbstractConfig> boolean isIgnoreDuplicateService(String uniqueServiceName, AbstractInterfaceConfig prevConfig, C config) {
- String configType = config.getClass().getSimpleName();
- String msg = "Found equivalent " + configType + " with unique service name [" +
- uniqueServiceName + "], previous: " + prevConfig + ", later: " + config + ". " +
- "There can only be one instance of " + configType + " with the same triple (group, interface, version). " +
- "If multiple instances are required for the same interface, please use a different group or version.";
+ private <C extends AbstractConfig> Optional<C> findConfigByValue(Collection<C> values, C config) {
+ // 1. find same config instance (speed up raw api usage)
+ Optional<C> prevConfig = values.stream().filter(val -> val == config).findFirst();
+ if (prevConfig.isPresent()) {
+ return prevConfig;
+ }
+
+ // 2. find equal config
+ prevConfig = values.stream()
+ .filter(val -> isEquals(val, config))
+ .findFirst();
+ return prevConfig;
+ }
- if (logger.isWarnEnabled() && duplicatedConfigs.add(config)) {
- logger.warn(msg);
+ /**
+ * check duplicated ReferenceConfig/ServiceConfig
+ * @param config
+ */
+ private AbstractInterfaceConfig checkDuplicatedInterfaceConfig(AbstractInterfaceConfig config) {
+ String uniqueServiceName;
+ Map<String, AbstractInterfaceConfig> configCache;
+ if (config instanceof ReferenceConfigBase) {
+ ReferenceConfigBase<?> referenceConfig = (ReferenceConfigBase<?>) config;
+ uniqueServiceName = referenceConfig.getUniqueServiceName();
+ configCache = referenceConfigCache;
+ } else if (config instanceof ServiceConfigBase) {
+ ServiceConfigBase serviceConfig = (ServiceConfigBase) config;
+ uniqueServiceName = serviceConfig.getUniqueServiceName();
+ configCache = serviceConfigCache;
+ } else {
+ throw new IllegalArgumentException("Illegal type of parameter 'config' : " + config.getClass().getName());
}
- if (configMode == ConfigMode.STRICT) {
- throw new IllegalStateException(msg);
+
+ AbstractInterfaceConfig prevConfig = configCache.putIfAbsent(uniqueServiceName, config);
+ if (prevConfig != null) {
+ if (prevConfig == config) {
+ return config;
+ }
+
+ String configType = config.getClass().getSimpleName();
+ String msg = "Found multiple " + configType + "s with unique service name [" +
+ uniqueServiceName + "], previous: " + prevConfig + ", later: " + config + ". " +
+ "There can only be one instance of " + configType + " with the same triple (group, interface, version). " +
+ "If multiple instances are required for the same interface, please use a different group or version.";
+
+ if (logger.isWarnEnabled() && duplicatedConfigs.add(config)) {
+ logger.warn(msg);
+ }
+ if (!ignoreDuplicatedInterface) {
+ throw new IllegalStateException(msg);
+ }
}
- return true;
+ return prevConfig;
}
public static <C extends AbstractConfig> String generateConfigId(C config) {
diff --git a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/boot/conditional4/JavaConfigReferenceBeanConditionalTest4.java b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/boot/conditional4/JavaConfigReferenceBeanConditionalTest4.java
new file mode 100644
index 0000000..b6b9ff8
--- /dev/null
+++ b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/boot/conditional4/JavaConfigReferenceBeanConditionalTest4.java
@@ -0,0 +1,111 @@
+/*
+ * 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.config.spring.boot.conditional4;
+
+import org.apache.dubbo.config.annotation.DubboReference;
+import org.apache.dubbo.config.bootstrap.DubboBootstrap;
+import org.apache.dubbo.config.spring.ReferenceBean;
+import org.apache.dubbo.config.spring.ZooKeeperServer;
+import org.apache.dubbo.config.spring.api.HelloService;
+import org.apache.dubbo.config.spring.context.annotation.EnableDubbo;
+import org.apache.dubbo.config.spring.context.annotation.provider.HelloServiceImpl;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.annotation.Order;
+
+import java.util.Map;
+
+/**
+ * issue: https://github.com/apache/dubbo-spring-boot-project/issues/779
+ */
+@SpringBootTest(
+ properties = {
+ "dubbo.application.name=consumer-app",
+ "dubbo.registry.address=N/A",
+ "myapp.group=demo"
+ },
+ classes = {
+ JavaConfigReferenceBeanConditionalTest4.class
+ }
+)
+@Configuration
+//@ComponentScan
+@EnableDubbo
+public class JavaConfigReferenceBeanConditionalTest4 {
+
+ @BeforeAll
+ public static void setUp(){
+ ZooKeeperServer.start();
+ DubboBootstrap.reset();
+ }
+
+ @AfterAll
+ public static void tearDown(){
+ DubboBootstrap.reset();
+ }
+
+ @Autowired
+ private HelloService helloService;
+
+ @Autowired
+ private ApplicationContext applicationContext;
+
+ @Test
+ public void testConsumer() {
+
+ Map<String, HelloService> helloServiceMap = applicationContext.getBeansOfType(HelloService.class);
+ Assertions.assertEquals(1, helloServiceMap.size());
+ Assertions.assertNull(helloServiceMap.get("helloService"));
+ HelloService helloService = helloServiceMap.get("helloServiceImpl");
+ Assertions.assertNotNull(helloService);
+ Assertions.assertTrue(helloService instanceof HelloServiceImpl, "Not expected bean type: "+helloService.getClass());
+ }
+
+ @Order(Integer.MAX_VALUE-2)
+ @Configuration
+ public static class ServiceBeanConfiguration {
+
+ @Bean
+ public HelloService helloServiceImpl() {
+ return new HelloServiceImpl();
+ }
+ }
+
+ // make sure that the one using condition runs after.
+ @Order(Integer.MAX_VALUE-1)
+ @Configuration
+ public static class AnnotationBeanConfiguration {
+
+ //TEST Conditional, this bean should be ignored
+ @Bean
+ @ConditionalOnMissingBean(HelloService.class)
+ @DubboReference(group = "${myapp.group}", init = false)
+ public ReferenceBean<HelloService> helloService() {
+ return new ReferenceBean();
+ }
+
+ }
+
+}
diff --git a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/ReferenceKeyTest.java b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/ReferenceKeyTest.java
index 02385c8..71d55e3 100644
--- a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/ReferenceKeyTest.java
+++ b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/ReferenceKeyTest.java
@@ -117,7 +117,7 @@ public class ReferenceKeyTest {
Assertions.fail("Reference bean check failed");
} catch (BeansException e) {
String msg = getStackTrace(e);
- Assertions.assertTrue(msg.contains("Found equivalent ReferenceConfig with unique service name [demo/org.apache.dubbo.config.spring.api.DemoService:1.2.3]"), msg);
+ Assertions.assertTrue(msg.contains("Found multiple ReferenceConfigs with unique service name [demo/org.apache.dubbo.config.spring.api.DemoService:1.2.3]"), msg);
// Assertions.assertTrue(msg.contains("Already exists another reference bean with the same bean name and type but difference attributes"), msg);
// Assertions.assertTrue(msg.contains("ConsumerConfiguration2.demoService"), msg);
}
diff --git a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/javaconfig/JavaConfigReferenceBeanTest.java b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/javaconfig/JavaConfigReferenceBeanTest.java
index 9831808..0b0351a 100644
--- a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/javaconfig/JavaConfigReferenceBeanTest.java
+++ b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/javaconfig/JavaConfigReferenceBeanTest.java
@@ -40,6 +40,8 @@ import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.PropertySource;
+import java.io.PrintWriter;
+import java.io.StringWriter;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
@@ -61,6 +63,50 @@ public class JavaConfigReferenceBeanTest {
DubboBootstrap.reset();
}
+ @Test
+ public void testAnnotationAtField() {
+ Assertions.assertEquals(0, SpringExtensionFactory.getContexts().size());
+ AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(CommonConfig.class,
+ AnnotationAtFieldConfiguration.class);
+
+ Map<String, HelloService> helloServiceMap = context.getBeansOfType(HelloService.class);
+ Assertions.assertEquals(2, helloServiceMap.size());
+ Assertions.assertNotNull(helloServiceMap.get("helloService"));
+ Assertions.assertNotNull(helloServiceMap.get("helloServiceImpl"));
+
+ Map<String, ReferenceBean> referenceBeanMap = context.getBeansOfType(ReferenceBean.class);
+ Assertions.assertEquals(1, referenceBeanMap.size());
+ ReferenceBean referenceBean = referenceBeanMap.get("&helloService");
+ Assertions.assertEquals("demo", referenceBean.getGroup());
+ Assertions.assertEquals(HelloService.class, referenceBean.getInterfaceClass());
+ Assertions.assertEquals(HelloService.class.getName(), referenceBean.getServiceInterface());
+
+ context.close();
+ Assertions.assertEquals(1, SpringExtensionFactory.getContexts().size());
+ }
+
+ @Test
+ public void testAnnotationAtField2() {
+ AnnotationConfigApplicationContext context = null;
+ try {
+ context = new AnnotationConfigApplicationContext(CommonConfig.class,
+ AnnotationAtFieldConfiguration.class, AnnotationAtFieldConfiguration2.class);
+ Assertions.fail("Should not load duplicated @DubboReference fields");
+ } catch (Exception e) {
+ String msg = getStackTrace(e);
+ Assertions.assertTrue(msg.contains("Found multiple ReferenceConfigs with unique service name [demo/org.apache.dubbo.config.spring.api.HelloService]"), msg);
+ } finally {
+ if (context != null) {
+ context.close();
+ }
+ }
+ }
+
+ private String getStackTrace(Throwable ex) {
+ StringWriter stringWriter = new StringWriter();
+ ex.printStackTrace(new PrintWriter(stringWriter));
+ return stringWriter.toString();
+ }
@Test
public void testAnnotationBean() {
@@ -281,6 +327,23 @@ public class JavaConfigReferenceBeanTest {
}
}
+
+ @Configuration
+ public static class AnnotationAtFieldConfiguration {
+
+ @DubboReference(group = "${myapp.group}")
+ private HelloService helloService;
+
+ }
+
+ @Configuration
+ public static class AnnotationAtFieldConfiguration2 {
+
+ @DubboReference(group = "${myapp.group}", timeout = 2000)
+ private HelloService helloService;
+
+ }
+
@Configuration
public static class AnnotationBeanConfiguration {