Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

some optimize on ExtensionLoader #3307

Merged
merged 5 commits into from
Jan 26, 2019
Merged

some optimize on ExtensionLoader #3307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f94450b
some optimize on ExtensionLoader
lixiaojiee Jan 22, 2019
a5028b9
make ci rerun
lixiaojiee Jan 24, 2019
6d2204b
fix compile error
lixiaojiee Jan 24, 2019
978ce81
Merge remote-tracking branch 'remotes/upstream/master' into 20190122-B
lixiaojiee Jan 24, 2019
3e0a57a
fix ci failure
lixiaojiee Jan 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 28 additions & 29 deletions dubbo-common/src/main/java/org/apache/dubbo/common/extension/ExtensionLoader.java
View file
Open in desktop
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
Open in desktop
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