Skip to content

Commit

Permalink
some optimize on ExtensionLoader (apache#3307)
Browse files Browse the repository at this point in the history 
* some optimize on ExtensionLoader
* make ci rerun
* fix compile error
* fix ci failure
  • Loading branch information
@lixiaojiee @ralf0131
lixiaojiee authored and ralf0131 committed Jan 26, 2019
1 parent 2300fca commit 21046c8
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 45 deletions.
57 changes: 28 additions & 29 deletions dubbo-common/src/main/java/org/apache/dubbo/common/extension/ExtensionLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ public static <T> ExtensionLoader<T> getExtensionLoader(Class<T> type) {
throw new IllegalArgumentException("Extension type == null");
}
if (!type.isInterface()) {
throw new IllegalArgumentException("Extension type(" + type + ") is not interface!");
throw new IllegalArgumentException("Extension type (" + type + ") is not interface!");
}
if (!withExtensionAnnotation(type)) {
throw new IllegalArgumentException("Extension type(" + type +
throw new IllegalArgumentException("Extension type (" + type +
") is not extension, because WITHOUT @" + SPI.class.getSimpleName() + " Annotation!");
}

Expand Down Expand Up @@ -354,8 +354,7 @@ public T getExtension(String name) {
*/
public T getDefaultExtension() {
getExtensionClasses();
if (null == cachedDefaultName || cachedDefaultName.length() == 0
|| "true".equals(cachedDefaultName)) {
if (StringUtils.isBlank(cachedDefaultName) || "true".equals(cachedDefaultName)) {
return null;
}
return getExtension(cachedDefaultName);
Expand Down Expand Up @@ -394,11 +393,11 @@ public void addExtension(String name, Class<?> clazz) {

if (!type.isAssignableFrom(clazz)) {
throw new IllegalStateException("Input type " +
clazz + "not implement Extension " + type);
clazz + " doesn't implement the Extension " + type);
}
if (clazz.isInterface()) {
throw new IllegalStateException("Input type " +
clazz + "can not be interface!");
clazz + " can't be interface!");
}

if (!clazz.isAnnotationPresent(Adaptive.class)) {
Expand All @@ -407,14 +406,14 @@ public void addExtension(String name, Class<?> clazz) {
}
if (cachedClasses.get().containsKey(name)) {
throw new IllegalStateException("Extension name " +
name + " already existed(Extension " + type + ")!");
name + " already exists (Extension " + type + ")!");
}

cachedNames.put(clazz, name);
cachedClasses.get().put(name, clazz);
} else {
if (cachedAdaptiveClass != null) {
throw new IllegalStateException("Adaptive Extension already existed(Extension " + type + ")!");
throw new IllegalStateException("Adaptive Extension already exists (Extension " + type + ")!");
}

cachedAdaptiveClass = clazz;
Expand All @@ -435,11 +434,11 @@ public void replaceExtension(String name, Class<?> clazz) {

if (!type.isAssignableFrom(clazz)) {
throw new IllegalStateException("Input type " +
clazz + "not implement Extension " + type);
clazz + " doesn't implement Extension " + type);
}
if (clazz.isInterface()) {
throw new IllegalStateException("Input type " +
clazz + "can not be interface!");
clazz + " can't be interface!");
}

if (!clazz.isAnnotationPresent(Adaptive.class)) {
Expand All @@ -448,15 +447,15 @@ public void replaceExtension(String name, Class<?> clazz) {
}
if (!cachedClasses.get().containsKey(name)) {
throw new IllegalStateException("Extension name " +
name + " not existed(Extension " + type + ")!");
name + " doesn't exist (Extension " + type + ")!");
}

cachedNames.put(clazz, name);
cachedClasses.get().put(name, clazz);
cachedInstances.remove(name);
} else {
if (cachedAdaptiveClass == null) {
throw new IllegalStateException("Adaptive Extension not existed(Extension " + type + ")!");
throw new IllegalStateException("Adaptive Extension doesn't exist (Extension " + type + ")!");
}

cachedAdaptiveClass = clazz;
Expand All @@ -477,12 +476,12 @@ public T getAdaptiveExtension() {
cachedAdaptiveInstance.set(instance);
} catch (Throwable t) {
createAdaptiveInstanceError = t;
throw new IllegalStateException("fail to create adaptive instance: " + t.toString(), t);
throw new IllegalStateException("Failed to create adaptive instance: " + t.toString(), t);
}
}
}
} else {
throw new IllegalStateException("fail to create adaptive instance: " + createAdaptiveInstanceError.toString(), createAdaptiveInstanceError);
throw new IllegalStateException("Failed to create adaptive instance: " + createAdaptiveInstanceError.toString(), createAdaptiveInstanceError);
}
}

Expand Down Expand Up @@ -535,8 +534,8 @@ private T createExtension(String name) {
}
return instance;
} catch (Throwable t) {
throw new IllegalStateException("Extension instance(name: " + name + ", class: " +
type + ") could not be instantiated: " + t.getMessage(), t);
throw new IllegalStateException("Extension instance (name: " + name + ", class: " +
type + ") couldn't be instantiated: " + t.getMessage(), t);
}
}

Expand Down Expand Up @@ -564,7 +563,7 @@ private T injectExtension(T instance) {
method.invoke(instance, object);
}
} catch (Exception e) {
logger.error("fail to inject via method " + method.getName()
logger.error("Failed to inject via method " + method.getName()
+ " of interface " + type.getName() + ": " + e.getMessage(), e);
}
}
Expand Down Expand Up @@ -608,7 +607,7 @@ private Map<String, Class<?>> loadExtensionClasses() {
if ((value = value.trim()).length() > 0) {
String[] names = NAME_SEPARATOR.split(value);
if (names.length > 1) {
throw new IllegalStateException("more than 1 default extension name on extension " + type.getName()
throw new IllegalStateException("More than 1 default extension name on extension " + type.getName()
+ ": " + Arrays.toString(names));
}
if (names.length == 1) {
Expand Down Expand Up @@ -644,7 +643,7 @@ private void loadDirectory(Map<String, Class<?>> extensionClasses, String dir, S
}
}
} catch (Throwable t) {
logger.error("Exception when load extension class(interface: " +
logger.error("Exception occured when loading extension class (interface: " +
type + ", description file: " + fileName + ").", t);
}
}
Expand Down Expand Up @@ -672,7 +671,7 @@ private void loadResource(Map<String, Class<?>> extensionClasses, ClassLoader cl
loadClass(extensionClasses, resourceURL, Class.forName(line, true, classLoader), name);
}
} 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);
}
}
Expand All @@ -681,16 +680,16 @@ private void loadResource(Map<String, Class<?>> extensionClasses, ClassLoader cl
reader.close();
}
} catch (Throwable t) {
logger.error("Exception when load extension class(interface: " +
logger.error("Exception occured when loading extension class (interface: " +
type + ", class file: " + resourceURL + ") in " + resourceURL, t);
}
}

private void loadClass(Map<String, Class<?>> extensionClasses, java.net.URL resourceURL, Class<?> clazz, String name) throws NoSuchMethodException {
if (!type.isAssignableFrom(clazz)) {
throw new IllegalStateException("Error when load extension class(interface: " +
throw new IllegalStateException("Error occured when loading extension class (interface: " +
type + ", class line: " + clazz.getName() + "), class "
+ clazz.getName() + "is not subtype of interface.");
+ clazz.getName() + " is not subtype of interface.");
}
if (clazz.isAnnotationPresent(Adaptive.class)) {
if (cachedAdaptiveClass == null) {
Expand All @@ -703,7 +702,7 @@ private void loadClass(Map<String, Class<?>> extensionClasses, java.net.URL reso
} else if (isWrapperClass(clazz)) {
Set<Class<?>> wrappers = cachedWrapperClasses;
if (wrappers == null) {
cachedWrapperClasses = new ConcurrentHashSet<Class<?>>();
cachedWrapperClasses = new ConcurrentHashSet<>();
wrappers = cachedWrapperClasses;
}
wrappers.add(clazz);
Expand Down Expand Up @@ -769,7 +768,7 @@ private T createAdaptiveExtension() {
try {
return injectExtension((T) getAdaptiveExtensionClass().newInstance());
} catch (Exception e) {
throw new IllegalStateException("Can not create adaptive extension " + type + ", cause: " + e.getMessage(), e);
throw new IllegalStateException("Can't create adaptive extension " + type + ", cause: " + e.getMessage(), e);
}
}

Expand Down Expand Up @@ -800,7 +799,7 @@ private String createAdaptiveExtensionClassCode() {
}
// no need to generate adaptive class since there's no adaptive method found.
if (!hasAdaptiveAnnotation) {
throw new IllegalStateException("No adaptive method on extension " + type.getName() + ", refuse to create the adaptive class!");
throw new IllegalStateException("No adaptive method exist on extension " + type.getName() + ", refuse to create the adaptive class!");
}

codeBuilder.append("package ").append(type.getPackage().getName()).append(";");
Expand All @@ -815,7 +814,7 @@ private String createAdaptiveExtensionClassCode() {
Adaptive adaptiveAnnotation = method.getAnnotation(Adaptive.class);
StringBuilder code = new StringBuilder(512);
if (adaptiveAnnotation == null) {
code.append("throw new UnsupportedOperationException(\"method ")
code.append("throw new UnsupportedOperationException(\"The method ")
.append(method.toString()).append(" of interface ")
.append(type.getName()).append(" is not adaptive method!\");");
} else {
Expand Down Expand Up @@ -858,7 +857,7 @@ private String createAdaptiveExtensionClassCode() {
}
}
if (attribMethod == null) {
throw new IllegalStateException("fail to create adaptive class for interface " + type.getName()
throw new IllegalStateException("Failed to create adaptive class for interface " + type.getName()
+ ": not found url parameter or url attribute in parameters of method " + method.getName());
}

Expand Down Expand Up @@ -934,7 +933,7 @@ private String createAdaptiveExtensionClassCode() {
code.append("\nString extName = ").append(getNameCode).append(";");
// check extName == null?
String s = String.format("\nif(extName == null) " +
"throw new IllegalStateException(\"Fail to get extension(%s) name from url(\" + url.toString() + \") use keys(%s)\");",
"throw new IllegalStateException(\"Failed to get extension (%s) name from url (\" + url.toString() + \") use keys(%s)\");",
type.getName(), Arrays.toString(value));
code.append(s);

Expand Down
14 changes: 7 additions & 7 deletions dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public void test_getExtensionLoader_NotInterface() throws Exception {
fail();
} catch (IllegalArgumentException expected) {
assertThat(expected.getMessage(),
containsString("Extension type(class org.apache.dubbo.common.extension.ExtensionLoaderTest) is not interface"));
containsString("Extension type (class org.apache.dubbo.common.extension.ExtensionLoaderTest) is not interface"));
}
}

Expand Down Expand Up @@ -263,7 +263,7 @@ public void test_AddExtension_ExceptionWhenExistedExtension() throws Exception {
ExtensionLoader.getExtensionLoader(AddExt1.class).addExtension("impl1", AddExt1_ManualAdd1.class);
fail();
} catch (IllegalStateException expected) {
assertThat(expected.getMessage(), containsString("Extension name impl1 already existed(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)!"));
}
}

Expand All @@ -286,7 +286,7 @@ 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 existed(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)!"));
}
}

Expand Down Expand Up @@ -335,7 +335,7 @@ public void test_replaceExtension_ExceptionWhenNotExistedExtension() throws Exce
ExtensionLoader.getExtensionLoader(AddExt1.class).replaceExtension("NotExistedExtension", AddExt1_ManualAdd1.class);
fail();
} catch (IllegalStateException expected) {
assertThat(expected.getMessage(), containsString("Extension name NotExistedExtension not existed(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)"));
}
}

Expand All @@ -347,7 +347,7 @@ public void test_replaceExtension_Adaptive_ExceptionWhenNotExistedExtension() th
loader.replaceExtension(null, AddExt4_ManualAdaptive.class);
fail();
} catch (IllegalStateException expected) {
assertThat(expected.getMessage(), containsString("Adaptive Extension not existed(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)"));
}
}

Expand All @@ -361,7 +361,7 @@ 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.getCause(), instanceOf(ExceptionInInitializerError.class));
}
}
Expand Down Expand Up @@ -437,4 +437,4 @@ public void testInjectExtension() {
Assertions.assertNull(injectExtImpl.getGenericType());
}

}
}
Loading

0 comments on commit 21046c8

Please sign in to comment.