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/06/08 02:03:05 UTC
[dubbo] branch master updated: De-duplicate the filter returned by
the getActivateExtension method (#7600)
This is an automated email from the ASF dual-hosted git repository.
albumenj pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/dubbo.git
The following commit(s) were added to refs/heads/master by this push:
new 275ac51 De-duplicate the filter returned by the getActivateExtension method (#7600)
275ac51 is described below
commit 275ac51111a9d3234d86e27a7b946352cbe4ebc5
Author: xiaoheng1 <20...@qq.com>
AuthorDate: Tue Jun 8 10:02:41 2021 +0800
De-duplicate the filter returned by the getActivateExtension method (#7600)
* fix #7587 De-duplicate the filter returned by the getActivateExtension method
* add log.
* modify log.
* merge master
---
.../dubbo/common/extension/ExtensionLoader.java | 51 +++++++++++++++-------
.../common/extension/ExtensionLoaderTest.java | 39 +++++++++++++----
2 files changed, 66 insertions(+), 24 deletions(-)
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/extension/ExtensionLoader.java b/dubbo-common/src/main/java/org/apache/dubbo/common/extension/ExtensionLoader.java
index 820dbd9..d85a34c 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/extension/ExtensionLoader.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/extension/ExtensionLoader.java
@@ -42,6 +42,7 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
@@ -148,7 +149,8 @@ public class ExtensionLoader<T> {
private ExtensionLoader(Class<?> type) {
this.type = type;
- objectFactory = (type == ExtensionFactory.class ? null : ExtensionLoader.getExtensionLoader(ExtensionFactory.class).getAdaptiveExtension());
+ objectFactory =
+ (type == ExtensionFactory.class ? null : ExtensionLoader.getExtensionLoader(ExtensionFactory.class).getAdaptiveExtension());
}
private static <T> boolean withExtensionAnnotation(Class<T> type) {
@@ -270,6 +272,7 @@ public class ExtensionLoader<T> {
List<T> activateExtensions = new ArrayList<>();
// solve the bug of using @SPI's wrapper method to report a null pointer exception.
TreeMap<Class, T> activateExtensionsMap = new TreeMap<>(ActivateComparator.COMPARATOR);
+ Set<String> loadedNames = new HashSet<>();
Set<String> names = CollectionUtils.ofSet(values);
if (!names.contains(REMOVE_VALUE_PREFIX + DEFAULT_KEY)) {
getExtensionClasses();
@@ -291,11 +294,13 @@ public class ExtensionLoader<T> {
if (isMatchGroup(group, activateGroup)
&& !names.contains(name)
&& !names.contains(REMOVE_VALUE_PREFIX + name)
- && isActive(activateValue, url)) {
+ && isActive(activateValue, url)
+ && !loadedNames.contains(name)) {
activateExtensionsMap.put(getExtensionClass(name), getExtension(name));
+ loadedNames.add(name);
}
}
- if(!activateExtensionsMap.isEmpty()){
+ if (!activateExtensionsMap.isEmpty()) {
activateExtensions.addAll(activateExtensionsMap.values());
}
}
@@ -303,13 +308,21 @@ public class ExtensionLoader<T> {
for (String name : names) {
if (!name.startsWith(REMOVE_VALUE_PREFIX)
&& !names.contains(REMOVE_VALUE_PREFIX + name)) {
- if (DEFAULT_KEY.equals(name)) {
- if (!loadedExtensions.isEmpty()) {
- activateExtensions.addAll(0, loadedExtensions);
- loadedExtensions.clear();
+ if (!loadedNames.contains(name)) {
+ if (DEFAULT_KEY.equals(name)) {
+ if (!loadedExtensions.isEmpty()) {
+ activateExtensions.addAll(0, loadedExtensions);
+ loadedExtensions.clear();
+ }
+ } else {
+ loadedExtensions.add(getExtension(name));
}
+ loadedNames.add(name);
} else {
- loadedExtensions.add(getExtension(name));
+ // If getExtension(name) exists, getExtensionClass(name) must exist, so there is no null pointer processing here.
+ String simpleName = getExtensionClass(name).getSimpleName();
+ logger.warn("Catch duplicated filter, ExtensionLoader will ignore one of them. Please check. Filter Name: " + name +
+ ". Ignored Class Name: " + simpleName);
}
}
}
@@ -785,8 +798,10 @@ public class ExtensionLoader<T> {
Map<String, Class<?>> extensionClasses = new HashMap<>();
for (LoadingStrategy strategy : strategies) {
- loadDirectory(extensionClasses, strategy.directory(), type.getName(), strategy.preferExtensionClassLoader(), strategy.overridden(), strategy.excludedPackages());
- loadDirectory(extensionClasses, strategy.directory(), type.getName().replace("org.apache", "com.alibaba"), strategy.preferExtensionClassLoader(), strategy.overridden(), strategy.excludedPackages());
+ loadDirectory(extensionClasses, strategy.directory(), type.getName(), strategy.preferExtensionClassLoader(),
+ strategy.overridden(), strategy.excludedPackages());
+ loadDirectory(extensionClasses, strategy.directory(), type.getName().replace("org.apache", "com.alibaba"),
+ strategy.preferExtensionClassLoader(), strategy.overridden(), strategy.excludedPackages());
}
return extensionClasses;
@@ -879,7 +894,9 @@ public class ExtensionLoader<T> {
loadClass(extensionClasses, resourceURL, Class.forName(clazz, true, classLoader), name, overridden);
}
} catch (Throwable t) {
- IllegalStateException e = new IllegalStateException("Failed to load extension class (interface: " + type + ", class line: " + line + ") in " + resourceURL + ", cause: " + t.getMessage(), t);
+ IllegalStateException e = new IllegalStateException(
+ "Failed to load extension class (interface: " + type + ", class line: " + line + ") in " + resourceURL +
+ ", cause: " + t.getMessage(), t);
exceptions.put(line, e);
}
}
@@ -918,7 +935,8 @@ public class ExtensionLoader<T> {
if (StringUtils.isEmpty(name)) {
name = findAnnotationName(clazz);
if (name.length() == 0) {
- throw new IllegalStateException("No such extension name for the class " + clazz.getName() + " in the config " + resourceURL);
+ throw new IllegalStateException(
+ "No such extension name for the class " + clazz.getName() + " in the config " + resourceURL);
}
}
@@ -952,7 +970,8 @@ public class ExtensionLoader<T> {
} else if (c != clazz) {
// duplicate implementation is unacceptable
unacceptableExceptions.add(name);
- String duplicateMsg = "Duplicate extension " + type.getName() + " name " + name + " on " + c.getName() + " and " + clazz.getName();
+ String duplicateMsg =
+ "Duplicate extension " + type.getName() + " name " + name + " on " + c.getName() + " and " + clazz.getName();
logger.error(duplicateMsg);
throw new IllegalStateException(duplicateMsg);
}
@@ -969,7 +988,8 @@ public class ExtensionLoader<T> {
cachedActivates.put(name, activate);
} else {
// support com.alibaba.dubbo.common.extension.Activate
- com.alibaba.dubbo.common.extension.Activate oldActivate = clazz.getAnnotation(com.alibaba.dubbo.common.extension.Activate.class);
+ com.alibaba.dubbo.common.extension.Activate oldActivate =
+ clazz.getAnnotation(com.alibaba.dubbo.common.extension.Activate.class);
if (oldActivate != null) {
cachedActivates.put(name, oldActivate);
}
@@ -1049,7 +1069,8 @@ public class ExtensionLoader<T> {
private Class<?> createAdaptiveExtensionClass() {
String code = new AdaptiveClassCodeGenerator(type, cachedDefaultName).generate();
ClassLoader classLoader = findClassLoader();
- org.apache.dubbo.common.compiler.Compiler compiler = ExtensionLoader.getExtensionLoader(org.apache.dubbo.common.compiler.Compiler.class).getAdaptiveExtension();
+ org.apache.dubbo.common.compiler.Compiler compiler =
+ ExtensionLoader.getExtensionLoader(org.apache.dubbo.common.compiler.Compiler.class).getAdaptiveExtension();
return compiler.compile(code, classLoader);
}
diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoaderTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoaderTest.java
index 0537836..9ebfc03 100644
--- a/dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoaderTest.java
+++ b/dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoaderTest.java
@@ -174,7 +174,8 @@ public class ExtensionLoaderTest {
getExtensionLoader(SimpleExt.class).getExtension("XXX");
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("No such extension org.apache.dubbo.common.extension.ext1.SimpleExt by name XXX"));
+ assertThat(expected.getMessage(),
+ containsString("No such extension org.apache.dubbo.common.extension.ext1.SimpleExt by name XXX"));
}
}
@@ -184,7 +185,8 @@ public class ExtensionLoaderTest {
getExtensionLoader(WrappedExt.class).getExtension("XXX");
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("No such extension org.apache.dubbo.common.extension.ext6_wrap.WrappedExt by name XXX"));
+ assertThat(expected.getMessage(),
+ containsString("No such extension org.apache.dubbo.common.extension.ext6_wrap.WrappedExt by name XXX"));
}
}
@@ -257,7 +259,8 @@ public class ExtensionLoaderTest {
getExtensionLoader(AddExt1.class).getExtension("Manual1");
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("No such extension org.apache.dubbo.common.extension.ext8_add.AddExt1 by name Manual"));
+ assertThat(expected.getMessage(),
+ containsString("No such extension org.apache.dubbo.common.extension.ext8_add.AddExt1 by name Manual"));
}
getExtensionLoader(AddExt1.class).addExtension("Manual1", AddExt1_ManualAdd1.class);
@@ -286,7 +289,8 @@ public class ExtensionLoaderTest {
getExtensionLoader(AddExt1.class).addExtension("impl1", AddExt1_ManualAdd1.class);
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("Extension name impl1 already exists (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)!"));
+ assertThat(expected.getMessage(), containsString(
+ "Extension name impl1 already exists (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)!"));
}
}
@@ -309,7 +313,8 @@ public class ExtensionLoaderTest {
loader.addExtension(null, AddExt1_ManualAdaptive.class);
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("Adaptive Extension already exists (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)!"));
+ assertThat(expected.getMessage(), containsString(
+ "Adaptive Extension already exists (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)!"));
}
}
@@ -319,7 +324,8 @@ public class ExtensionLoaderTest {
getExtensionLoader(AddExt1.class).getExtension("Manual2");
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("No such extension org.apache.dubbo.common.extension.ext8_add.AddExt1 by name Manual"));
+ assertThat(expected.getMessage(),
+ containsString("No such extension org.apache.dubbo.common.extension.ext8_add.AddExt1 by name Manual"));
}
{
@@ -360,7 +366,8 @@ public class ExtensionLoaderTest {
getExtensionLoader(AddExt1.class).replaceExtension("NotExistedExtension", AddExt1_ManualAdd1.class);
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("Extension name NotExistedExtension doesn't exist (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)"));
+ assertThat(expected.getMessage(), containsString(
+ "Extension name NotExistedExtension doesn't exist (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)"));
}
}
@@ -372,7 +379,8 @@ public class ExtensionLoaderTest {
loader.replaceExtension(null, AddExt4_ManualAdaptive.class);
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("Adaptive Extension doesn't exist (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt4)"));
+ assertThat(expected.getMessage(), containsString(
+ "Adaptive Extension doesn't exist (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt4)"));
}
}
@@ -386,12 +394,25 @@ public class ExtensionLoaderTest {
loader.getExtension("error");
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("Failed to load extension class (interface: interface org.apache.dubbo.common.extension.ext7.InitErrorExt"));
+ assertThat(expected.getMessage(), containsString(
+ "Failed to load extension class (interface: interface org.apache.dubbo.common.extension.ext7.InitErrorExt"));
assertThat(expected.getMessage(), containsString("java.lang.ExceptionInInitializerError"));
}
}
@Test
+ public void testLoadActivateExtensionWithNoRepeat() {
+ URL url = URL.valueOf("test://localhost/test");
+ List<ActivateExt1> list = getExtensionLoader(ActivateExt1.class)
+ .getActivateExtension(url, new String[] {"old1", "old2"}, "default_group");
+ Assertions.assertEquals(3, list.size());
+
+ list = getExtensionLoader(ActivateExt1.class)
+ .getActivateExtension(url, new String[] {"old1", "old2", "old1"}, "default_group");
+ Assertions.assertEquals(3, list.size());
+ }
+
+ @Test
public void testLoadActivateExtension() throws Exception {
// test default
URL url = URL.valueOf("test://localhost/test");