Skip to content

Commit 21046c8

Browse files
lixiaojieeralf0131
authored andcommitted
some optimize on ExtensionLoader (#3307)
* some optimize on ExtensionLoader * make ci rerun * fix compile error * fix ci failure
1 parent 2300fca commit 21046c8

File tree

3 files changed

+44
-45
lines changed

3 files changed

+44
-45
lines changed

dubbo-common/src/main/java/org/apache/dubbo/common/extension/ExtensionLoader.java

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,10 @@ public static <T> ExtensionLoader<T> getExtensionLoader(Class<T> type) {
110110
throw new IllegalArgumentException("Extension type == null");
111111
}
112112
if (!type.isInterface()) {
113-
throw new IllegalArgumentException("Extension type(" + type + ") is not interface!");
113+
throw new IllegalArgumentException("Extension type (" + type + ") is not interface!");
114114
}
115115
if (!withExtensionAnnotation(type)) {
116-
throw new IllegalArgumentException("Extension type(" + type +
116+
throw new IllegalArgumentException("Extension type (" + type +
117117
") is not extension, because WITHOUT @" + SPI.class.getSimpleName() + " Annotation!");
118118
}
119119

@@ -354,8 +354,7 @@ public T getExtension(String name) {
354354
*/
355355
public T getDefaultExtension() {
356356
getExtensionClasses();
357-
if (null == cachedDefaultName || cachedDefaultName.length() == 0
358-
|| "true".equals(cachedDefaultName)) {
357+
if (StringUtils.isBlank(cachedDefaultName) || "true".equals(cachedDefaultName)) {
359358
return null;
360359
}
361360
return getExtension(cachedDefaultName);
@@ -394,11 +393,11 @@ public void addExtension(String name, Class<?> clazz) {
394393

395394
if (!type.isAssignableFrom(clazz)) {
396395
throw new IllegalStateException("Input type " +
397-
clazz + "not implement Extension " + type);
396+
clazz + " doesn't implement the Extension " + type);
398397
}
399398
if (clazz.isInterface()) {
400399
throw new IllegalStateException("Input type " +
401-
clazz + "can not be interface!");
400+
clazz + " can't be interface!");
402401
}
403402

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

413412
cachedNames.put(clazz, name);
414413
cachedClasses.get().put(name, clazz);
415414
} else {
416415
if (cachedAdaptiveClass != null) {
417-
throw new IllegalStateException("Adaptive Extension already existed(Extension " + type + ")!");
416+
throw new IllegalStateException("Adaptive Extension already exists (Extension " + type + ")!");
418417
}
419418

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

436435
if (!type.isAssignableFrom(clazz)) {
437436
throw new IllegalStateException("Input type " +
438-
clazz + "not implement Extension " + type);
437+
clazz + " doesn't implement Extension " + type);
439438
}
440439
if (clazz.isInterface()) {
441440
throw new IllegalStateException("Input type " +
442-
clazz + "can not be interface!");
441+
clazz + " can't be interface!");
443442
}
444443

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

454453
cachedNames.put(clazz, name);
455454
cachedClasses.get().put(name, clazz);
456455
cachedInstances.remove(name);
457456
} else {
458457
if (cachedAdaptiveClass == null) {
459-
throw new IllegalStateException("Adaptive Extension not existed(Extension " + type + ")!");
458+
throw new IllegalStateException("Adaptive Extension doesn't exist (Extension " + type + ")!");
460459
}
461460

462461
cachedAdaptiveClass = clazz;
@@ -477,12 +476,12 @@ public T getAdaptiveExtension() {
477476
cachedAdaptiveInstance.set(instance);
478477
} catch (Throwable t) {
479478
createAdaptiveInstanceError = t;
480-
throw new IllegalStateException("fail to create adaptive instance: " + t.toString(), t);
479+
throw new IllegalStateException("Failed to create adaptive instance: " + t.toString(), t);
481480
}
482481
}
483482
}
484483
} else {
485-
throw new IllegalStateException("fail to create adaptive instance: " + createAdaptiveInstanceError.toString(), createAdaptiveInstanceError);
484+
throw new IllegalStateException("Failed to create adaptive instance: " + createAdaptiveInstanceError.toString(), createAdaptiveInstanceError);
486485
}
487486
}
488487

@@ -535,8 +534,8 @@ private T createExtension(String name) {
535534
}
536535
return instance;
537536
} catch (Throwable t) {
538-
throw new IllegalStateException("Extension instance(name: " + name + ", class: " +
539-
type + ") could not be instantiated: " + t.getMessage(), t);
537+
throw new IllegalStateException("Extension instance (name: " + name + ", class: " +
538+
type + ") couldn't be instantiated: " + t.getMessage(), t);
540539
}
541540
}
542541

@@ -564,7 +563,7 @@ private T injectExtension(T instance) {
564563
method.invoke(instance, object);
565564
}
566565
} catch (Exception e) {
567-
logger.error("fail to inject via method " + method.getName()
566+
logger.error("Failed to inject via method " + method.getName()
568567
+ " of interface " + type.getName() + ": " + e.getMessage(), e);
569568
}
570569
}
@@ -608,7 +607,7 @@ private Map<String, Class<?>> loadExtensionClasses() {
608607
if ((value = value.trim()).length() > 0) {
609608
String[] names = NAME_SEPARATOR.split(value);
610609
if (names.length > 1) {
611-
throw new IllegalStateException("more than 1 default extension name on extension " + type.getName()
610+
throw new IllegalStateException("More than 1 default extension name on extension " + type.getName()
612611
+ ": " + Arrays.toString(names));
613612
}
614613
if (names.length == 1) {
@@ -644,7 +643,7 @@ private void loadDirectory(Map<String, Class<?>> extensionClasses, String dir, S
644643
}
645644
}
646645
} catch (Throwable t) {
647-
logger.error("Exception when load extension class(interface: " +
646+
logger.error("Exception occured when loading extension class (interface: " +
648647
type + ", description file: " + fileName + ").", t);
649648
}
650649
}
@@ -672,7 +671,7 @@ private void loadResource(Map<String, Class<?>> extensionClasses, ClassLoader cl
672671
loadClass(extensionClasses, resourceURL, Class.forName(line, true, classLoader), name);
673672
}
674673
} catch (Throwable t) {
675-
IllegalStateException e = new IllegalStateException("Failed to load extension class(interface: " + type + ", class line: " + line + ") in " + resourceURL + ", cause: " + t.getMessage(), t);
674+
IllegalStateException e = new IllegalStateException("Failed to load extension class (interface: " + type + ", class line: " + line + ") in " + resourceURL + ", cause: " + t.getMessage(), t);
676675
exceptions.put(line, e);
677676
}
678677
}
@@ -681,16 +680,16 @@ private void loadResource(Map<String, Class<?>> extensionClasses, ClassLoader cl
681680
reader.close();
682681
}
683682
} catch (Throwable t) {
684-
logger.error("Exception when load extension class(interface: " +
683+
logger.error("Exception occured when loading extension class (interface: " +
685684
type + ", class file: " + resourceURL + ") in " + resourceURL, t);
686685
}
687686
}
688687

689688
private void loadClass(Map<String, Class<?>> extensionClasses, java.net.URL resourceURL, Class<?> clazz, String name) throws NoSuchMethodException {
690689
if (!type.isAssignableFrom(clazz)) {
691-
throw new IllegalStateException("Error when load extension class(interface: " +
690+
throw new IllegalStateException("Error occured when loading extension class (interface: " +
692691
type + ", class line: " + clazz.getName() + "), class "
693-
+ clazz.getName() + "is not subtype of interface.");
692+
+ clazz.getName() + " is not subtype of interface.");
694693
}
695694
if (clazz.isAnnotationPresent(Adaptive.class)) {
696695
if (cachedAdaptiveClass == null) {
@@ -703,7 +702,7 @@ private void loadClass(Map<String, Class<?>> extensionClasses, java.net.URL reso
703702
} else if (isWrapperClass(clazz)) {
704703
Set<Class<?>> wrappers = cachedWrapperClasses;
705704
if (wrappers == null) {
706-
cachedWrapperClasses = new ConcurrentHashSet<Class<?>>();
705+
cachedWrapperClasses = new ConcurrentHashSet<>();
707706
wrappers = cachedWrapperClasses;
708707
}
709708
wrappers.add(clazz);
@@ -769,7 +768,7 @@ private T createAdaptiveExtension() {
769768
try {
770769
return injectExtension((T) getAdaptiveExtensionClass().newInstance());
771770
} catch (Exception e) {
772-
throw new IllegalStateException("Can not create adaptive extension " + type + ", cause: " + e.getMessage(), e);
771+
throw new IllegalStateException("Can't create adaptive extension " + type + ", cause: " + e.getMessage(), e);
773772
}
774773
}
775774

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

806805
codeBuilder.append("package ").append(type.getPackage().getName()).append(";");
@@ -815,7 +814,7 @@ private String createAdaptiveExtensionClassCode() {
815814
Adaptive adaptiveAnnotation = method.getAnnotation(Adaptive.class);
816815
StringBuilder code = new StringBuilder(512);
817816
if (adaptiveAnnotation == null) {
818-
code.append("throw new UnsupportedOperationException(\"method ")
817+
code.append("throw new UnsupportedOperationException(\"The method ")
819818
.append(method.toString()).append(" of interface ")
820819
.append(type.getName()).append(" is not adaptive method!\");");
821820
} else {
@@ -858,7 +857,7 @@ private String createAdaptiveExtensionClassCode() {
858857
}
859858
}
860859
if (attribMethod == null) {
861-
throw new IllegalStateException("fail to create adaptive class for interface " + type.getName()
860+
throw new IllegalStateException("Failed to create adaptive class for interface " + type.getName()
862861
+ ": not found url parameter or url attribute in parameters of method " + method.getName());
863862
}
864863

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

dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoaderTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public void test_getExtensionLoader_NotInterface() throws Exception {
8787
fail();
8888
} catch (IllegalArgumentException expected) {
8989
assertThat(expected.getMessage(),
90-
containsString("Extension type(class org.apache.dubbo.common.extension.ExtensionLoaderTest) is not interface"));
90+
containsString("Extension type (class org.apache.dubbo.common.extension.ExtensionLoaderTest) is not interface"));
9191
}
9292
}
9393

@@ -263,7 +263,7 @@ public void test_AddExtension_ExceptionWhenExistedExtension() throws Exception {
263263
ExtensionLoader.getExtensionLoader(AddExt1.class).addExtension("impl1", AddExt1_ManualAdd1.class);
264264
fail();
265265
} catch (IllegalStateException expected) {
266-
assertThat(expected.getMessage(), containsString("Extension name impl1 already existed(Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)!"));
266+
assertThat(expected.getMessage(), containsString("Extension name impl1 already exists (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)!"));
267267
}
268268
}
269269

@@ -286,7 +286,7 @@ public void test_AddExtension_Adaptive_ExceptionWhenExistedAdaptive() throws Exc
286286
loader.addExtension(null, AddExt1_ManualAdaptive.class);
287287
fail();
288288
} catch (IllegalStateException expected) {
289-
assertThat(expected.getMessage(), containsString("Adaptive Extension already existed(Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)!"));
289+
assertThat(expected.getMessage(), containsString("Adaptive Extension already exists (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)!"));
290290
}
291291
}
292292

@@ -335,7 +335,7 @@ public void test_replaceExtension_ExceptionWhenNotExistedExtension() throws Exce
335335
ExtensionLoader.getExtensionLoader(AddExt1.class).replaceExtension("NotExistedExtension", AddExt1_ManualAdd1.class);
336336
fail();
337337
} catch (IllegalStateException expected) {
338-
assertThat(expected.getMessage(), containsString("Extension name NotExistedExtension not existed(Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)"));
338+
assertThat(expected.getMessage(), containsString("Extension name NotExistedExtension doesn't exist (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt1)"));
339339
}
340340
}
341341

@@ -347,7 +347,7 @@ public void test_replaceExtension_Adaptive_ExceptionWhenNotExistedExtension() th
347347
loader.replaceExtension(null, AddExt4_ManualAdaptive.class);
348348
fail();
349349
} catch (IllegalStateException expected) {
350-
assertThat(expected.getMessage(), containsString("Adaptive Extension not existed(Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt4)"));
350+
assertThat(expected.getMessage(), containsString("Adaptive Extension doesn't exist (Extension interface org.apache.dubbo.common.extension.ext8_add.AddExt4)"));
351351
}
352352
}
353353

@@ -361,7 +361,7 @@ public void test_InitError() throws Exception {
361361
loader.getExtension("error");
362362
fail();
363363
} catch (IllegalStateException expected) {
364-
assertThat(expected.getMessage(), containsString("Failed to load extension class(interface: interface org.apache.dubbo.common.extension.ext7.InitErrorExt"));
364+
assertThat(expected.getMessage(), containsString("Failed to load extension class (interface: interface org.apache.dubbo.common.extension.ext7.InitErrorExt"));
365365
assertThat(expected.getCause(), instanceOf(ExceptionInInitializerError.class));
366366
}
367367
}
@@ -437,4 +437,4 @@ public void testInjectExtension() {
437437
Assertions.assertNull(injectExtImpl.getGenericType());
438438
}
439439

440-
}
440+
}

0 commit comments

Comments
 (0)