This is an automated email from the ASF dual-hosted git repository.
huxing pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-dubbo.git
The following commit(s) were added to refs/heads/master by this push:
new 21046c8 some optimize on ExtensionLoader (#3307)
21046c8 is described below
commit 21046c85ea960c754d1f695530bff1a4b0aaa729
Author: XiaoJie Li <[email protected]>
AuthorDate: Sat Jan 26 22:52:12 2019 +0800
some optimize on ExtensionLoader (#3307)
* some optimize on ExtensionLoader
* make ci rerun
* fix compile error
* fix ci failure
---
.../dubbo/common/extension/ExtensionLoader.java | 57 +++++++++++-----------
.../common/extension/ExtensionLoaderTest.java | 14 +++---
.../extension/ExtensionLoader_Adaptive_Test.java | 18 +++----
3 files changed, 44 insertions(+), 45 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 42fa01b..380dbbf 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
@@ -110,10 +110,10 @@ public class ExtensionLoader<T> {
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!");
}
@@ -354,8 +354,7 @@ public class ExtensionLoader<T> {
*/
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);
@@ -394,11 +393,11 @@ public class ExtensionLoader<T> {
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)) {
@@ -407,14 +406,14 @@ public class ExtensionLoader<T> {
}
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;
@@ -435,11 +434,11 @@ public class ExtensionLoader<T> {
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)) {
@@ -448,7 +447,7 @@ public class ExtensionLoader<T> {
}
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);
@@ -456,7 +455,7 @@ public class ExtensionLoader<T> {
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;
@@ -477,12 +476,12 @@ public class ExtensionLoader<T> {
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);
}
}
@@ -535,8 +534,8 @@ public class ExtensionLoader<T> {
}
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);
}
}
@@ -564,7 +563,7 @@ public class ExtensionLoader<T> {
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);
}
}
@@ -608,7 +607,7 @@ public class ExtensionLoader<T> {
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) {
@@ -644,7 +643,7 @@ public class ExtensionLoader<T> {
}
}
} 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);
}
}
@@ -672,7 +671,7 @@ public class ExtensionLoader<T> {
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);
}
}
@@ -681,16 +680,16 @@ public class ExtensionLoader<T> {
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) {
@@ -703,7 +702,7 @@ public class ExtensionLoader<T> {
} else if (isWrapperClass(clazz)) {
Set<Class<?>> wrappers = cachedWrapperClasses;
if (wrappers == null) {
- cachedWrapperClasses = new ConcurrentHashSet<Class<?>>();
+ cachedWrapperClasses = new ConcurrentHashSet<>();
wrappers = cachedWrapperClasses;
}
wrappers.add(clazz);
@@ -769,7 +768,7 @@ public class ExtensionLoader<T> {
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);
}
}
@@ -800,7 +799,7 @@ public class ExtensionLoader<T> {
}
// 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(";");
@@ -815,7 +814,7 @@ public class ExtensionLoader<T> {
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 {
@@ -858,7 +857,7 @@ public class ExtensionLoader<T> {
}
}
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());
}
@@ -934,7 +933,7 @@ public class ExtensionLoader<T> {
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);
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 3f767e8..b0e878c 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
@@ -87,7 +87,7 @@ public class ExtensionLoaderTest {
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"));
}
}
@@ -263,7 +263,7 @@ public class ExtensionLoaderTest {
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)!"));
}
}
@@ -286,7 +286,7 @@ public class ExtensionLoaderTest {
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)!"));
}
}
@@ -335,7 +335,7 @@ public class ExtensionLoaderTest {
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)"));
}
}
@@ -347,7 +347,7 @@ public class ExtensionLoaderTest {
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)"));
}
}
@@ -361,7 +361,7 @@ 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.getCause(),
instanceOf(ExceptionInInitializerError.class));
}
}
@@ -437,4 +437,4 @@ public class ExtensionLoaderTest {
Assertions.assertNull(injectExtImpl.getGenericType());
}
-}
\ No newline at end of file
+}
diff --git
a/dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoader_Adaptive_Test.java
b/dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoader_Adaptive_Test.java
index b4db6d4..fe64a63 100644
---
a/dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoader_Adaptive_Test.java
+++
b/dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoader_Adaptive_Test.java
@@ -147,8 +147,8 @@ public class ExtensionLoader_Adaptive_Test {
fail();
} catch (IllegalStateException expected) {
assertThat(expected.getMessage(),
- allOf(containsString("Can not create adaptive extension
interface org.apache.dubbo.common.extension.ext5.NoAdaptiveMethodExt"),
- containsString("No adaptive method on extension
org.apache.dubbo.common.extension.ext5.NoAdaptiveMethodExt, refuse to create
the adaptive class")));
+ allOf(containsString("Can't create adaptive extension
interface org.apache.dubbo.common.extension.ext5.NoAdaptiveMethodExt"),
+ containsString("No adaptive method exist on
extension org.apache.dubbo.common.extension.ext5.NoAdaptiveMethodExt, refuse to
create the adaptive class")));
}
// report same error when get is invoked for multiple times
try {
@@ -156,8 +156,8 @@ public class ExtensionLoader_Adaptive_Test {
fail();
} catch (IllegalStateException expected) {
assertThat(expected.getMessage(),
- allOf(containsString("Can not create adaptive extension
interface org.apache.dubbo.common.extension.ext5.NoAdaptiveMethodExt"),
- containsString("No adaptive method on extension
org.apache.dubbo.common.extension.ext5.NoAdaptiveMethodExt, refuse to create
the adaptive class")));
+ allOf(containsString("Can't create adaptive extension
interface org.apache.dubbo.common.extension.ext5.NoAdaptiveMethodExt"),
+ containsString("No adaptive method exist on
extension org.apache.dubbo.common.extension.ext5.NoAdaptiveMethodExt, refuse to
create the adaptive class")));
}
}
@@ -185,7 +185,7 @@ public class ExtensionLoader_Adaptive_Test {
ExtensionLoader.getExtensionLoader(NoUrlParamExt.class).getAdaptiveExtension();
fail();
} catch (Exception expected) {
- assertThat(expected.getMessage(), containsString("fail to create
adaptive class for interface "));
+ assertThat(expected.getMessage(), containsString("Failed to create
adaptive class for interface "));
assertThat(expected.getMessage(), containsString(": not found url
parameter or url attribute in parameters of method "));
}
}
@@ -218,7 +218,7 @@ public class ExtensionLoader_Adaptive_Test {
ext.echo(holder, "haha");
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("Fail to get
extension("));
+ assertThat(expected.getMessage(), containsString("Failed to get
extension"));
}
url = url.addParameter("ext2", "XXX");
@@ -281,7 +281,7 @@ public class ExtensionLoader_Adaptive_Test {
ext.echo(holder, "impl1");
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("Fail to get
extension("));
+ assertThat(expected.getMessage(), containsString("Failed to get
extension"));
}
url = url.addParameter("key1", "impl1");
@@ -290,7 +290,7 @@ public class ExtensionLoader_Adaptive_Test {
ext.echo(holder, "haha");
fail();
} catch (IllegalStateException expected) {
- assertThat(expected.getMessage(), containsString("Fail to get
extension(org.apache.dubbo.common.extension.ext2.Ext2) name from url"));
+ assertThat(expected.getMessage(), containsString("Failed to get
extension (org.apache.dubbo.common.extension.ext2.Ext2) name from url"));
}
}
@@ -319,4 +319,4 @@ public class ExtensionLoader_Adaptive_Test {
Ext6Impl2 impl = (Ext6Impl2) ext;
assertNull(impl.getList());
}
-}
\ No newline at end of file
+}
