szetszwo commented on code in PR #4990:
URL: https://github.com/apache/ozone/pull/4990#discussion_r1245494315
##########
hadoop-hdds/annotations/src/main/java/org/apache/ozone/annotations/ReplicateAnnotationProcessor.java:
##########
@@ -27,30 +27,48 @@
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.TypeMirror;
import javax.tools.Diagnostic.Kind;
-import java.util.List;
+import java.io.IOException;
+import java.util.Collections;
import java.util.Set;
-import java.util.concurrent.TimeoutException;
+
+import org.apache.commons.lang3.ClassUtils;
+
+import static java.util.stream.Collectors.toSet;
/**
* Annotation Processor that verifies if the methods that are marked with
* Replicate annotation have proper method signature which throws
* TimeoutException.
*/
-@SupportedAnnotationTypes("org.apache.hadoop.hdds.scm.metadata.Replicate")
+@SupportedAnnotationTypes(ReplicateAnnotationProcessor.ANNOTATION_NAME)
@SupportedSourceVersion(SourceVersion.RELEASE_8)
public class ReplicateAnnotationProcessor extends AbstractProcessor {
- private static final String ANNOTATION_SIMPLE_NAME = "Replicate";
+ static final String ANNOTATION_NAME =
+ "org.apache.hadoop.hdds.scm.metadata.Replicate";
+ private static final String REQUIRED_EXCEPTION =
+ "org.apache.hadoop.hdds.scm.exceptions.SCMException";
+
+ private static final Set<String> EXCEPTIONS;
+ static {
+ Set<String> set = ClassUtils.getAllSuperclasses(IOException.class)
Review Comment:
We should not allow super classes (e.g. `Exception`) since it would allow
throwing anything (e.g. `ExecutionException`).
##########
hadoop-hdds/annotations/src/main/java/org/apache/ozone/annotations/ReplicateAnnotationProcessor.java:
##########
@@ -67,14 +85,14 @@ private void checkMethodSignature(Element element) {
return;
}
final ExecutableElement executableElement = (ExecutableElement) element;
- final List<? extends TypeMirror> exceptions =
- executableElement.getThrownTypes();
+ final boolean found = executableElement.getThrownTypes().stream()
+ .map(TypeMirror::toString)
Review Comment:
`toString` may depend on the Java vendor. It seems the correct way is to
use `processingEnv.getTypeUtils()` as mentioned in the javadoc
https://docs.oracle.com/javase/8/docs/api/javax/lang/model/type/TypeMirror.html
> Types should be compared using the utility methods in
[Types](https://docs.oracle.com/javase/8/docs/api/javax/lang/model/util/Types.html).
...
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/symmetric/SecretKeyManager.java:
##########
@@ -109,7 +108,7 @@ public boolean isInitialized() {
*
* @return true if rotation actually happens, false if it doesn't.
*/
- public synchronized boolean checkAndRotate() throws TimeoutException {
+ public synchronized boolean checkAndRotate() throws Exception {
Review Comment:
It should throw `SCMException`.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/symmetric/SecretKeyManager.java:
##########
@@ -72,7 +71,7 @@ public SecretKeyManager(SecretKeyState state,
* SecretKeys from local file, or generate new keys if the file doesn't
* exist.
*/
- public synchronized void checkAndInitialize() throws TimeoutException {
+ public synchronized void checkAndInitialize() throws Exception {
Review Comment:
It should throw `SCMException`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
