Skip to content

Commit

Permalink
[Enhancement]: language level migration (#3485)
Browse files Browse the repository at this point in the history 
* use java7 diamond operator

* replace Collections.sort with List.sort

* extract duplicated code blocks

* use StandardCharsets.UTF_8

* use try-with-resources

* use java7 diamond operator

* enhance log message

* fix unit tests failures
  • Loading branch information
@kezhenxu94 @CrazyHZM
kezhenxu94 authored and CrazyHZM committed Feb 15, 2019
1 parent e72e0b2 commit 9c049f6
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 33 deletions.
59 changes: 29 additions & 30 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 @@ -33,6 +33,7 @@
import java.io.InputStreamReader;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -71,30 +72,30 @@ public class ExtensionLoader<T> {

private static final Pattern NAME_SEPARATOR = Pattern.compile("\\s*[,]+\\s*");

private static final ConcurrentMap<Class<?>, ExtensionLoader<?>> EXTENSION_LOADERS = new ConcurrentHashMap<Class<?>, ExtensionLoader<?>>();
private static final ConcurrentMap<Class<?>, ExtensionLoader<?>> EXTENSION_LOADERS = new ConcurrentHashMap<>();

private static final ConcurrentMap<Class<?>, Object> EXTENSION_INSTANCES = new ConcurrentHashMap<Class<?>, Object>();
private static final ConcurrentMap<Class<?>, Object> EXTENSION_INSTANCES = new ConcurrentHashMap<>();

// ==============================

private final Class<?> type;

private final ExtensionFactory objectFactory;

private final ConcurrentMap<Class<?>, String> cachedNames = new ConcurrentHashMap<Class<?>, String>();
private final ConcurrentMap<Class<?>, String> cachedNames = new ConcurrentHashMap<>();

private final Holder<Map<String, Class<?>>> cachedClasses = new Holder<Map<String, Class<?>>>();
private final Holder<Map<String, Class<?>>> cachedClasses = new Holder<>();

private final Map<String, Object> cachedActivates = new ConcurrentHashMap<String, Object>();
private final ConcurrentMap<String, Holder<Object>> cachedInstances = new ConcurrentHashMap<String, Holder<Object>>();
private final Holder<Object> cachedAdaptiveInstance = new Holder<Object>();
private final Map<String, Object> cachedActivates = new ConcurrentHashMap<>();
private final ConcurrentMap<String, Holder<Object>> cachedInstances = new ConcurrentHashMap<>();
private final Holder<Object> cachedAdaptiveInstance = new Holder<>();
private volatile Class<?> cachedAdaptiveClass = null;
private String cachedDefaultName;
private volatile Throwable createAdaptiveInstanceError;

private Set<Class<?>> cachedWrapperClasses;

private Map<String, IllegalStateException> exceptions = new ConcurrentHashMap<String, IllegalStateException>();
private Map<String, IllegalStateException> exceptions = new ConcurrentHashMap<>();

private ExtensionLoader(Class<?> type) {
this.type = type;
Expand All @@ -111,11 +112,11 @@ 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 an interface!");
}
if (!withExtensionAnnotation(type)) {
throw new IllegalArgumentException("Extension type (" + type +
") is not extension, because WITHOUT @" + SPI.class.getSimpleName() + " Annotation!");
") is not an extension, because it is NOT annotated with @" + SPI.class.getSimpleName() + "!");
}

ExtensionLoader<T> loader = (ExtensionLoader<T>) EXTENSION_LOADERS.get(type);
Expand Down Expand Up @@ -201,8 +202,8 @@ public List<T> getActivateExtension(URL url, String key, String group) {
* @see org.apache.dubbo.common.extension.Activate
*/
public List<T> getActivateExtension(URL url, String[] values, String group) {
List<T> exts = new ArrayList<T>();
List<String> names = values == null ? new ArrayList<String>(0) : Arrays.asList(values);
List<T> exts = new ArrayList<>();
List<String> names = values == null ? new ArrayList<>(0) : Arrays.asList(values);
if (!names.contains(Constants.REMOVE_VALUE_PREFIX + Constants.DEFAULT_KEY)) {
getExtensionClasses();
for (Map.Entry<String, Object> entry : cachedActivates.entrySet()) {
Expand All @@ -229,9 +230,9 @@ && isActive(activateValue, url)) {
}
}
}
Collections.sort(exts, ActivateComparator.COMPARATOR);
exts.sort(ActivateComparator.COMPARATOR);
}
List<T> usrs = new ArrayList<T>();
List<T> usrs = new ArrayList<>();
for (int i = 0; i < names.size(); i++) {
String name = names.get(i);
if (!name.startsWith(Constants.REMOVE_VALUE_PREFIX)
Expand Down Expand Up @@ -297,12 +298,17 @@ public T getLoadedExtension(String name) {
if (StringUtils.isEmpty(name)) {
throw new IllegalArgumentException("Extension name == null");
}
Holder<Object> holder = getOrCreateHolder(name);
return (T) holder.get();
}

private Holder<Object> getOrCreateHolder(String name) {
Holder<Object> holder = cachedInstances.get(name);
if (holder == null) {
cachedInstances.putIfAbsent(name, new Holder<Object>());
cachedInstances.putIfAbsent(name, new Holder<>());
holder = cachedInstances.get(name);
}
return (T) holder.get();
return holder;
}

/**
Expand All @@ -313,7 +319,7 @@ public T getLoadedExtension(String name) {
* @see #getSupportedExtensions()
*/
public Set<String> getLoadedExtensions() {
return Collections.unmodifiableSet(new TreeSet<String>(cachedInstances.keySet()));
return Collections.unmodifiableSet(new TreeSet<>(cachedInstances.keySet()));
}

public Object getLoadedAdaptiveExtensionInstances() {
Expand All @@ -332,11 +338,7 @@ public T getExtension(String name) {
if ("true".equals(name)) {
return getDefaultExtension();
}
Holder<Object> holder = cachedInstances.get(name);
if (holder == null) {
cachedInstances.putIfAbsent(name, new Holder<Object>());
holder = cachedInstances.get(name);
}
Holder<Object> holder = getOrCreateHolder(name);
Object instance = holder.get();
if (instance == null) {
synchronized (holder) {
Expand Down Expand Up @@ -371,7 +373,7 @@ public boolean hasExtension(String name) {

public Set<String> getSupportedExtensions() {
Map<String, Class<?>> clazzes = getExtensionClasses();
return Collections.unmodifiableSet(new TreeSet<String>(clazzes.keySet()));
return Collections.unmodifiableSet(new TreeSet<>(clazzes.keySet()));
}

/**
Expand Down Expand Up @@ -680,8 +682,7 @@ private void loadDirectory(Map<String, Class<?>> extensionClasses, String dir, S

private void loadResource(Map<String, Class<?>> extensionClasses, ClassLoader classLoader, java.net.URL resourceURL) {
try {
BufferedReader reader = new BufferedReader(new InputStreamReader(resourceURL.openStream(), "utf-8"));
try {
try (BufferedReader reader = new BufferedReader(new InputStreamReader(resourceURL.openStream(), StandardCharsets.UTF_8))) {
String line;
while ((line = reader.readLine()) != null) {
final int ci = line.indexOf('#');
Expand All @@ -706,8 +707,6 @@ private void loadResource(Map<String, Class<?>> extensionClasses, ClassLoader cl
}
}
}
} finally {
reader.close();
}
} catch (Throwable t) {
logger.error("Exception occurred when loading extension class (interface: " +
Expand All @@ -733,7 +732,7 @@ private void loadClass(Map<String, Class<?>> extensionClasses, java.net.URL reso
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]);
Expand Down Expand Up @@ -798,7 +797,7 @@ private void cacheAdaptiveClass(Class<?> clazz) {
}

/**
* cache wrapper class
* cache wrapper class
* <p>
* like: ProtocolFilterWrapper, ProtocolListenerWrapper
*/
Expand All @@ -810,7 +809,7 @@ private void cacheWrapperClass(Class<?> clazz) {
}

/**
* test if clazz is a wrapper class
* test if clazz is a wrapper class
* <p>
* which has Constructor with given class type as its only argument
*/
Expand Down
6 changes: 3 additions & 3 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 an interface"));
}
}

Expand All @@ -99,8 +99,8 @@ public void test_getExtensionLoader_NotSpiAnnotation() throws Exception {
} catch (IllegalArgumentException expected) {
assertThat(expected.getMessage(),
allOf(containsString("org.apache.dubbo.common.extension.NoSpiExt"),
containsString("is not extension"),
containsString("WITHOUT @SPI Annotation")));
containsString("is not an extension"),
containsString("NOT annotated with @SPI")));
}
}

Expand Down

0 comments on commit 9c049f6

Please sign in to comment.