This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
The following commit(s) were added to refs/heads/master by this push:
new 1be8451 [Improvement] modify code to avoid raw use of parameterized
class (#46)
1be8451 is described below
commit 1be84513f727f144356d6d22fa21ec4082045357
Author: Xianjin YE <[email protected]>
AuthorDate: Sun Jul 10 12:46:41 2022 +0800
[Improvement] modify code to avoid raw use of parameterized class (#46)
### What changes were proposed in this pull request?
1. reduce the raw use of parameterized class
2. use `Collections.singletonList` instead of `Arrays.asList` for single
element
3. add type annotation for some method signature and variable declaration
### Why are the changes needed?
This makes javac and IntelliJ's compiler happy
### Does this PR introduce _any_ user-facing change?
NO
### How was this patch tested?
Existing tests should be sufficient.
---
.../java/org/apache/uniffle/common/config/ConfigOptions.java | 8 ++++----
.../java/org/apache/uniffle/common/config/ConfigUtils.java | 6 +++---
.../java/org/apache/uniffle/common/config/RssBaseConf.java | 2 +-
.../java/org/apache/uniffle/common/util/RssUtilsTest.java | 11 ++++++-----
.../java/org/apache/uniffle/coordinator/CoordinatorConf.java | 2 +-
.../uniffle/coordinator/PartitionRangeAssignmentTest.java | 2 +-
.../java/org/apache/uniffle/server/ShuffleServerConf.java | 2 +-
.../main/java/org/apache/uniffle/server/ShuffleUploader.java | 2 +-
8 files changed, 18 insertions(+), 17 deletions(-)
diff --git
a/common/src/main/java/org/apache/uniffle/common/config/ConfigOptions.java
b/common/src/main/java/org/apache/uniffle/common/config/ConfigOptions.java
index 4a429ac..9193a74 100644
--- a/common/src/main/java/org/apache/uniffle/common/config/ConfigOptions.java
+++ b/common/src/main/java/org/apache/uniffle/common/config/ConfigOptions.java
@@ -173,12 +173,12 @@ public class ConfigOptions {
this.converter = converter;
}
- public ListConfigOptionBuilder asList() {
+ public ListConfigOptionBuilder<T> asList() {
return new ListConfigOptionBuilder<T>(key, clazz, converter);
}
// todo: errorMsg shouldn't contain key
- public TypedConfigOptionBuilder checkValue(Function<T, Boolean>
checkValue, String errMsg) {
+ public TypedConfigOptionBuilder<T> checkValue(Function<T, Boolean>
checkValue, String errMsg) {
Function<Object, T> newConverter = (v) -> {
T newValue = this.converter.apply(v);
if (!checkValue.apply(newValue)) {
@@ -186,7 +186,7 @@ public class ConfigOptions {
}
return newValue;
};
- return new TypedConfigOptionBuilder(key, clazz, newConverter);
+ return new TypedConfigOptionBuilder<>(key, clazz, newConverter);
}
/**
@@ -250,7 +250,7 @@ public class ConfigOptions {
};
}
- public ListConfigOptionBuilder checkValue(Function<E, Boolean>
checkValueFunc, String errMsg) {
+ public ListConfigOptionBuilder<E> checkValue(Function<E, Boolean>
checkValueFunc, String errMsg) {
final Function<Object, List<E>> listConverFunc = asListConverter;
Function<Object, List<E>> newConverter = (v) -> {
List<E> list = listConverFunc.apply(v);
diff --git
a/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java
b/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java
index 69b432c..023f287 100644
--- a/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java
+++ b/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java
@@ -160,15 +160,15 @@ public class ConfigUtils {
return Double.parseDouble(o.toString());
}
- public static List<ConfigOption> getAllConfigOptions(Class confClass) {
- List<ConfigOption> configOptionList = Lists.newArrayList();
+ public static List<ConfigOption<Object>> getAllConfigOptions(Class
confClass) {
+ List<ConfigOption<Object>> configOptionList = Lists.newArrayList();
try {
Field[] fields = confClass.getFields();
for (Field field : fields) {
int modifiers = field.getModifiers();
if (isStatic(modifiers) && isPublic(modifiers)
&& isFinal(modifiers) &&
field.getType().isAssignableFrom(ConfigOption.class)) {
- configOptionList.add((ConfigOption) field.get(null));
+ configOptionList.add((ConfigOption<Object>) field.get(null));
}
}
} catch (Exception e) {
diff --git
a/common/src/main/java/org/apache/uniffle/common/config/RssBaseConf.java
b/common/src/main/java/org/apache/uniffle/common/config/RssBaseConf.java
index 6f24b59..d1db6d1 100644
--- a/common/src/main/java/org/apache/uniffle/common/config/RssBaseConf.java
+++ b/common/src/main/java/org/apache/uniffle/common/config/RssBaseConf.java
@@ -162,7 +162,7 @@ public class RssBaseConf extends RssConf {
return false;
}
- List<ConfigOption> configOptions =
ConfigUtils.getAllConfigOptions(RssBaseConf.class);
+ List<ConfigOption<Object>> configOptions =
ConfigUtils.getAllConfigOptions(RssBaseConf.class);
properties.forEach((k, v) -> {
configOptions.forEach(config -> {
if (config.key().equalsIgnoreCase(k)) {
diff --git
a/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java
b/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java
index 20ff5f3..c1c95e2 100644
--- a/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java
+++ b/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java
@@ -20,8 +20,8 @@ package org.apache.uniffle.common.util;
import java.lang.reflect.Field;
import java.net.InetAddress;
import java.nio.ByteBuffer;
-import java.util.Arrays;
import java.util.List;
+import java.util.Collections;
import java.util.Map;
import java.util.Objects;
import java.util.Random;
@@ -153,26 +153,26 @@ public class RssUtilsTest {
@Test
public void testLoadExtentions() {
- List<String> exts = Arrays.asList("Dummy");
+ List<String> exts = Collections.singletonList("Dummy");
try {
RssUtils.loadExtensions(RssUtilTestDummy.class, exts, 1);
} catch (RuntimeException e) {
assertTrue(e.getMessage().startsWith("java.lang.ClassNotFoundException:
Dummy"));
}
- exts =
Arrays.asList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummyFailNotSub");
+ exts =
Collections.singletonList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummyFailNotSub");
try {
RssUtils.loadExtensions(RssUtilTestDummy.class, exts, 1);
} catch (RuntimeException e) {
assertTrue(e.getMessage().contains("RssUtilTestDummyFailNotSub is not
subclass of "
+ "org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummy"));
}
- exts =
Arrays.asList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummyNoConstructor");
+ exts =
Collections.singletonList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummyNoConstructor");
try {
RssUtils.loadExtensions(RssUtilTestDummy.class, exts, "Test");
} catch (RuntimeException e) {
assertTrue(e.getMessage().contains("RssUtilTestDummyNoConstructor.<init>()"));
}
- exts =
Arrays.asList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummySuccess");
+ exts =
Collections.singletonList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummySuccess");
String testStr = String.valueOf(new Random().nextInt());
List<RssUtilTestDummy> extsObjs =
RssUtils.loadExtensions(RssUtilTestDummy.class, exts, testStr);
assertEquals(1, extsObjs.size());
@@ -197,6 +197,7 @@ public class RssUtilsTest {
}
}
+ @SuppressWarnings("unchecked")
public static void setEnv(String key, String value) {
try {
Map<String, String> env = System.getenv();
diff --git
a/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java
b/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java
index e89b348..0fbd562 100644
---
a/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java
+++
b/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java
@@ -140,7 +140,7 @@ public class CoordinatorConf extends RssBaseConf {
loadCommonConf(properties);
- List<ConfigOption> configOptions =
ConfigUtils.getAllConfigOptions(CoordinatorConf.class);
+ List<ConfigOption<Object>> configOptions =
ConfigUtils.getAllConfigOptions(CoordinatorConf.class);
properties.forEach((k, v) -> {
configOptions.forEach(config -> {
if (config.key().equalsIgnoreCase(k)) {
diff --git
a/coordinator/src/test/java/org/apache/uniffle/coordinator/PartitionRangeAssignmentTest.java
b/coordinator/src/test/java/org/apache/uniffle/coordinator/PartitionRangeAssignmentTest.java
index 552710d..715cb04 100644
---
a/coordinator/src/test/java/org/apache/uniffle/coordinator/PartitionRangeAssignmentTest.java
+++
b/coordinator/src/test/java/org/apache/uniffle/coordinator/PartitionRangeAssignmentTest.java
@@ -33,7 +33,7 @@ public class PartitionRangeAssignmentTest {
@Test
public void test() {
- SortedMap sortedMap = new TreeMap();
+ SortedMap<PartitionRange, List<ServerNode>> sortedMap = new TreeMap<>();
for (int i = 0; i < 9; i = i + 3) {
PartitionRange range = new PartitionRange(i, i + 2);
List<ServerNode> nodes = Collections.singletonList(new ServerNode(
diff --git
a/server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java
b/server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java
index 7f0a74a..c4131db 100644
--- a/server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java
+++ b/server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java
@@ -376,7 +376,7 @@ public class ShuffleServerConf extends RssBaseConf {
loadCommonConf(properties);
- List<ConfigOption> configOptions =
ConfigUtils.getAllConfigOptions(ShuffleServerConf.class);
+ List<ConfigOption<Object>> configOptions =
ConfigUtils.getAllConfigOptions(ShuffleServerConf.class);
properties.forEach((k, v) -> {
configOptions.forEach(config -> {
diff --git
a/server/src/main/java/org/apache/uniffle/server/ShuffleUploader.java
b/server/src/main/java/org/apache/uniffle/server/ShuffleUploader.java
index b64a2c3..f1b83b4 100644
--- a/server/src/main/java/org/apache/uniffle/server/ShuffleUploader.java
+++ b/server/src/main/java/org/apache/uniffle/server/ShuffleUploader.java
@@ -413,7 +413,7 @@ public class ShuffleUploader {
return shuffleFileInfoList;
}
- private List getNotUploadedPartitions(String key) {
+ private List<Integer> getNotUploadedPartitions(String key) {
RoaringBitmap bitmap = localStorage.getNotUploadedPartitions(key);
List<Integer> partitionList = Lists.newArrayList();
for (int p : bitmap) {
