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 820dbd97801..d85a34cf1b3 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.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 static List getLoadingStrategies() { 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 boolean withExtensionAnnotation(Class type) { @@ -270,6 +272,7 @@ public List getActivateExtension(URL url, String[] values, String group) { List activateExtensions = new ArrayList<>(); // solve the bug of using @SPI's wrapper method to report a null pointer exception. TreeMap activateExtensionsMap = new TreeMap<>(ActivateComparator.COMPARATOR); + Set loadedNames = new HashSet<>(); Set names = CollectionUtils.ofSet(values); if (!names.contains(REMOVE_VALUE_PREFIX + DEFAULT_KEY)) { getExtensionClasses(); @@ -291,11 +294,13 @@ public List getActivateExtension(URL url, String[] values, String group) { 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 @@ && isActive(activateValue, url)) { 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 @@ private Map> loadExtensionClasses() { Map> 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 @@ private void loadResource(Map> extensionClasses, ClassLoader cl 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 @@ private void loadClass(Map> extensionClasses, java.net.URL reso 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 @@ private void saveInExtensionClass(Map> extensionClasses, Class< } 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 @@ private void cacheActivateClass(Class clazz, String name) { 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 @@ private Class getAdaptiveExtensionClass() { 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 05378368698..9ebfc038702 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 void test_getExtension_ExceptionNoExtension() throws Exception { 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 void test_getExtension_ExceptionNoExtension_WrapperNotAffactName() throws 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 void test_AddExtension() throws Exception { 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 void test_AddExtension_ExceptionWhenExistedExtension() throws Exception { 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 void test_AddExtension_Adaptive_ExceptionWhenExistedAdaptive() throws Exc 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 void test_replaceExtension() throws Exception { 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 void test_replaceExtension_ExceptionWhenNotExistedExtension() throws Exce 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 void test_replaceExtension_Adaptive_ExceptionWhenNotExistedExtension() th 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,11 +394,24 @@ public void test_InitError() throws Exception { 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 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