apache / apache/dubbo

adapitve spi should invoke `clazz.getConstructor()` to guarantee usability

Open
#9,449 0 comments 0 reactions 0 assignees View on GitHub
type/proposal
Dominant language
Java
Stars
41.6k
Forks
26.4k
Avg merge
15h 13m
Merged PRs (30d)
4

Description

adapitve spi should invoke `clazz.getConstructor()` to guarantee usability

> 自适应spi应该调用`clazz.getConstructor()`来保证可用性

in method `org.apache.dubbo.common.extension.ExtensionLoader#loadClass`

For normal SPI, we make sure we don't cache unavailable extension point implementations by calling 'clazz.getconstructor ()', but we don't do this for adaptive extensions

```java
private void loadClass(Map> extensionClasses, java.net.URL resourceURL, Class clazz, String name,
boolean overridden) throws NoSuchMethodException {
if (!type.isAssignableFrom(clazz)) {
throw new IllegalStateException("Error occurred when loading extension class (interface: " +
type + ", class line: " + clazz.getName() + "), class "
+ clazz.getName() + " is not subtype of interface.");
}
if (clazz.isAnnotationPresent(Adaptive.class)) {
cacheAdaptiveClass(clazz, overridden);
} else if (isWrapperClass(clazz)) {
cacheWrapperClass(clazz);
} else {
clazz.getConstructor();
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);
}
}

String[] names = NAME_SEPARATOR.split(name);
if (ArrayUtils.isNotEmpty(names)) {
cacheActivateClass(clazz, names[0]);
for (String n : names) {
cacheName(clazz, n);
saveInExtensionClass(extensionClasses, clazz, n, overridden);
}
}
}
}
```

I think the following makes more sense

```java
private void loadClass(Map> extensionClasses, java.net.URL resourceURL, Class clazz, String name,
boolean overridden) throws NoSuchMethodException {
if (!type.isAssignableFrom(clazz)) {
throw new IllegalStateException("Error occurred when loading extension class (interface: " +
type + ", class line: " + clazz.getName() + "), class "
+ clazz.getName() + " is not subtype of interface.");
}

if (isWrapperClass(clazz)) {
cacheWrapperClass(clazz);
return;
}
// guarantee usability of spi
clazz.getConstructor();
if (clazz.isAnnotationPresent(Adaptive.class)) {
cacheAdaptiveClass(clazz, overridden);
return;
}

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);
}
}

String[] names = NAME_SEPARATOR.split(name);
if (ArrayUtils.isNotEmpty(names)) {
cacheActivateClass(clazz, names[0]);
for (String n : names) {
cacheName(clazz, n);
saveInExtensionClass(extensionClasses, clazz, n, overridden);
}
}
}
```

Contributor guide

Open the contributing guide

Research direction

Start at org.apache.dubbo.common.extension.ExtensionLoader#loadClass and compare the existing constructor check for normal SPI classes with the adaptive-extension branch. Verify how adaptive classes without a usable public constructor are handled, then add or update focused tests so unusable adaptive extensions are rejected while valid ones remain loadable.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.