szetszwo commented on code in PR #9952:
URL: https://github.com/apache/ozone/pull/9952#discussion_r2972628118


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/ScmCodecFactory.java:
##########
@@ -74,35 +75,46 @@ public final class ScmCodecFactory {
     putEnum(NodeType.class, NodeType::forNumber);
 
     // Must be the last one
-    final ClassResolver resolver = new ClassResolver(codecs.keySet());
+    resolver = new ClassResolver(codecs.keySet());
     codecs.put(List.class, new ScmListCodec(resolver));
   }
 
-  static <T extends Message> void putProto(T proto) {
+  private <T extends Message> void putProto(T proto) {
     final Class<? extends Message> clazz = proto.getClass();
     codecs.put(clazz, new 
ScmNonShadedGeneratedMessageCodec<>(clazz.getSimpleName(), 
proto.getParserForType()));
   }
 
-  static <T extends Enum<T> & ProtocolMessageEnum> void putEnum(
+  private <T extends Enum<T> & ProtocolMessageEnum> void putEnum(
       Class<T> enumClass, IntFunction<T> forNumber) {
     codecs.put(enumClass, new ScmEnumCodec<>(enumClass, forNumber));
   }
 
-  private ScmCodecFactory() { }
+  public static ScmCodecFactory getInstance() {
+    return INSTANCE;
+  }
 
-  public static ScmCodec getCodec(Class<?> type)
+  public Class<?> resolve(String className)

Review Comment:
   Add one more resolve method for Class<?>.
   ```java
     public Class<?> resolve(Class<?> clazz) throws 
InvalidProtocolBufferException {
       return resolver.get(clazz);
     }
   ```



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/ScmCodecFactory.java:
##########
@@ -74,35 +75,46 @@ public final class ScmCodecFactory {
     putEnum(NodeType.class, NodeType::forNumber);
 
     // Must be the last one
-    final ClassResolver resolver = new ClassResolver(codecs.keySet());
+    resolver = new ClassResolver(codecs.keySet());
     codecs.put(List.class, new ScmListCodec(resolver));
   }
 
-  static <T extends Message> void putProto(T proto) {
+  private <T extends Message> void putProto(T proto) {
     final Class<? extends Message> clazz = proto.getClass();
     codecs.put(clazz, new 
ScmNonShadedGeneratedMessageCodec<>(clazz.getSimpleName(), 
proto.getParserForType()));
   }
 
-  static <T extends Enum<T> & ProtocolMessageEnum> void putEnum(
+  private <T extends Enum<T> & ProtocolMessageEnum> void putEnum(
       Class<T> enumClass, IntFunction<T> forNumber) {
     codecs.put(enumClass, new ScmEnumCodec<>(enumClass, forNumber));
   }
 
-  private ScmCodecFactory() { }
+  public static ScmCodecFactory getInstance() {
+    return INSTANCE;
+  }
 
-  public static ScmCodec getCodec(Class<?> type)
+  public Class<?> resolve(String className)
       throws InvalidProtocolBufferException {
-    final List<Class<?>> classes = new ArrayList<>();
-    classes.add(type);
-    classes.addAll(ClassUtils.getAllSuperclasses(type));
-    classes.addAll(ClassUtils.getAllInterfaces(type));
-    for (Class<?> clazz : classes) {
-      if (codecs.containsKey(clazz)) {
-        return codecs.get(clazz);
-      }
+    return resolver.get(className);
+  }
+
+  public ScmCodec getCodec(Class<?> resolved) throws 
InvalidProtocolBufferException {
+    final ScmCodec<?> codec = codecs.get(resolved);
+    if (codec != null) {
+      return codec;
+    }
+
+    throw new InvalidProtocolBufferException("Codec not found for " + 
resolved);
+  }
+
+  public Class<?> getClass(String className)

Review Comment:
   Add one more map to avoid calling Class.forName(..) multiple times for the 
same class. 
   ```java
     public Class<?> getClass(String className) throws 
InvalidProtocolBufferException {
       final Class<?> found = classes.get(className);
       if (found != null) {
         return found;
       }
   
       final Class<?> clazz;
       try {
         clazz = Class.forName(className);
       } catch (ClassNotFoundException e) {
         throw new InvalidProtocolBufferException("Class not found for " + 
className, e);
       }
       classes.put(className, clazz);
       return clazz;
     }
   ```



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/ScmCodecFactory.java:
##########
@@ -120,31 +132,41 @@ static class ClassResolver {
     }
 
     Class<?> get(String className) throws InvalidProtocolBufferException {
-      final Class<?> c = provided.get(className);
-      if (c != null) {
-        return c;
-      }
-      throw new InvalidProtocolBufferException("Class not found for " + 
className);
+      return get(null, className);
     }
 
     Class<?> get(Class<?> clazz) throws InvalidProtocolBufferException {
-      final String className = clazz.getName();
+      return get(clazz, clazz.getName());
+    }
+
+    private Class<?> get(Class<?> clazz, String className) throws 
InvalidProtocolBufferException {
+      Objects.requireNonNull(className, "className == null");
       final Class<?> c = provided.get(className);
       if (c != null) {
         return c;
       }
+
       final Class<?> found = resolved.get(className);
       if (found != null) {
         return found;
       }
 
+      if (clazz == null) {
+        try {
+          clazz = Class.forName(className);
+        } catch (ClassNotFoundException e) {
+          throw new InvalidProtocolBufferException("Class not found for " + 
className, e);
+        }

Review Comment:
   Reuse the getClass method
   ```java
           clazz = getInstance().getClass(className);
   ```



-- 
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]

Reply via email to