szetszwo commented on code in PR #1098:
URL: https://github.com/apache/ratis/pull/1098#discussion_r1631917108


##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/RaftUtils.java:
##########
@@ -86,4 +103,111 @@ public static RaftClient createClient(RaftGroup raftGroup) 
{
         .setRetryPolicy(retryPolicy)
         .build();
   }
+
+
+  /**
+   * Execute a given function with input parameter from the members of a list.
+   *
+   * @param list the input parameters
+   * @param function the function to be executed
+   * @param <T> parameter type
+   * @param <K> return value type
+   * @param <E> the exception type thrown by the given function.
+   * @return the value returned by the given function.
+   */
+  public static <T, K, E extends Throwable> K runFunction(Collection<T> list, 
CheckedFunction<T, K, E> function) {
+    for (T t : list) {
+      try {
+        K ret = function.apply(t);
+        if (ret != null) {
+          return ret;
+        }
+      } catch (Throwable e) {
+        e.printStackTrace();
+      }
+    }
+    return null;
+  }
+
+
+  public static List<RaftPeer> buildRaftPeersFromStr(String peers) {
+    List<InetSocketAddress> addresses = new ArrayList<>();
+    String[] peersArray = peers.split(",");
+    for (String peer : peersArray) {
+      addresses.add(parseInetSocketAddress(peer));
+    }
+
+    return addresses.stream()
+        .map(addr -> RaftPeer.newBuilder()
+            .setId(RaftUtils.getPeerId(addr))
+            .setAddress(addr)
+            .build()
+        ).collect(Collectors.toList());
+  }
+
+  public static RaftGroupId buildRaftGroupIdFromStr(String groupId) {
+    return (groupId != null && !groupId.equals("")) ? 
RaftGroupId.valueOf(UUID.fromString(groupId))

Review Comment:
   Use `groupId.isEmpty()`.



##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/RaftUtils.java:
##########
@@ -86,4 +103,111 @@ public static RaftClient createClient(RaftGroup raftGroup) 
{
         .setRetryPolicy(retryPolicy)
         .build();
   }
+
+
+  /**
+   * Execute a given function with input parameter from the members of a list.
+   *
+   * @param list the input parameters
+   * @param function the function to be executed
+   * @param <T> parameter type
+   * @param <K> return value type
+   * @param <E> the exception type thrown by the given function.
+   * @return the value returned by the given function.
+   */
+  public static <T, K, E extends Throwable> K runFunction(Collection<T> list, 
CheckedFunction<T, K, E> function) {

Review Comment:
   After this change, `AbstractRatisCommand.run(..)` becomes unused.  Please 
remove it.
   
   Also, is this method needed for Ozone?  If not, please make it private.  
Otherwise, let's update the names and the javadoc:
   ```java
     /**
      * Apply the given function to the given parameter a list.
      *
      * @param list the input parameter list
      * @param function the function to be applied
      * @param <PARAMETER> parameter type
      * @param <RETURN> return value type
      * @param <EXCEPTION> the exception type thrown by the given function.
      * @return the first non-null value returned by the given function applied 
to the given list.
      */
     public static <PARAMETER, RETURN, EXCEPTION extends Throwable> RETURN 
applyFunctionReturnFirstNonNull(
         Collection<PARAMETER> list, CheckedFunction<PARAMETER, RETURN, 
EXCEPTION> function) {
       for (PARAMETER parameter : list) {
         try {
           RETURN ret = function.apply(parameter);
           if (ret != null) {
             return ret;
           }
         } catch (Throwable e) {
           e.printStackTrace();
         }
       }
       return null;
     }
   ```



##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/RaftUtils.java:
##########
@@ -86,4 +103,111 @@ public static RaftClient createClient(RaftGroup raftGroup) 
{
         .setRetryPolicy(retryPolicy)
         .build();
   }
+
+
+  /**
+   * Execute a given function with input parameter from the members of a list.
+   *
+   * @param list the input parameters
+   * @param function the function to be executed
+   * @param <T> parameter type
+   * @param <K> return value type
+   * @param <E> the exception type thrown by the given function.
+   * @return the value returned by the given function.
+   */
+  public static <T, K, E extends Throwable> K runFunction(Collection<T> list, 
CheckedFunction<T, K, E> function) {
+    for (T t : list) {
+      try {
+        K ret = function.apply(t);
+        if (ret != null) {
+          return ret;
+        }
+      } catch (Throwable e) {
+        e.printStackTrace();
+      }
+    }
+    return null;
+  }
+
+
+  public static List<RaftPeer> buildRaftPeersFromStr(String peers) {
+    List<InetSocketAddress> addresses = new ArrayList<>();
+    String[] peersArray = peers.split(",");
+    for (String peer : peersArray) {
+      addresses.add(parseInetSocketAddress(peer));
+    }
+
+    return addresses.stream()
+        .map(addr -> RaftPeer.newBuilder()
+            .setId(RaftUtils.getPeerId(addr))
+            .setAddress(addr)
+            .build()
+        ).collect(Collectors.toList());
+  }
+
+  public static RaftGroupId buildRaftGroupIdFromStr(String groupId) {
+    return (groupId != null && !groupId.equals("")) ? 
RaftGroupId.valueOf(UUID.fromString(groupId))
+        : DEFAULT_RAFT_GROUP_ID;
+  }
+
+  public static RaftGroupId retrieveRemoteGroupId(RaftGroupId 
raftGroupIdFromConfig,
+                                                  List<RaftPeer> peers,
+                                                  RaftClient client, 
PrintStream printStream) throws IOException {
+    RaftGroupId remoteGroupId;
+    if (raftGroupIdFromConfig != DEFAULT_RAFT_GROUP_ID) {
+      return raftGroupIdFromConfig;
+    } else {
+      final List<RaftGroupId> groupIds = runFunction(peers,
+          p -> client.getGroupManagementApi((p.getId())).list().getGroupIds());
+
+      if (groupIds == null) {
+        printStream.println("Failed to get group ID from " + peers);
+        throw new IOException("Failed to get group ID from " + peers);
+      } else if (groupIds.size() == 1) {
+        remoteGroupId = groupIds.get(0);
+      } else {
+        printStream.println("There are more than one groups, you should 
specific one. " + groupIds);
+        throw new IOException("There are more than one groups, you should 
specific one. " + groupIds);
+      }
+    }
+
+    return remoteGroupId;
+  }
+
+  public static GroupInfoReply retrieveGroupInfoByGroupId(RaftGroupId 
remoteGroupId, List<RaftPeer> peers,
+                                                          RaftClient client, 
PrintStream printStream)
+      throws IOException {
+    GroupInfoReply groupInfoReply = runFunction(peers,
+        p -> client.getGroupManagementApi((p.getId())).info(remoteGroupId));
+    processReply(groupInfoReply,
+        printStream::println, "Failed to get group info for group id " + 
remoteGroupId.getUuid() + " from " + peers);
+    return groupInfoReply;
+  }
+
+  public static void processReply(RaftClientReply reply, Consumer<String> 
printer, String message) throws IOException {
+    processReplyInternal(reply, () -> printer.accept(message));
+  }
+
+  private static void processReplyInternal(RaftClientReply reply, Runnable 
printer) throws IOException {

Review Comment:
   Combine this two methods and use ` Supplier<String>` for  `message`, i.e.
   ```java
     public static void processReply(RaftClientReply reply, Consumer<String> 
printer, Supplier<String> message)
         throws IOException {
       if (reply == null || !reply.isSuccess()) {
         final RaftException e = Optional.ofNullable(reply)
             .map(RaftClientReply::getException)
             .orElseGet(() -> new RaftException("Reply: " + reply));
         printer.accept(message.get());
         throw new IOException(e.getMessage(), e);
       }
     }
   ```



##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/RaftUtils.java:
##########
@@ -86,4 +103,111 @@ public static RaftClient createClient(RaftGroup raftGroup) 
{
         .setRetryPolicy(retryPolicy)
         .build();
   }
+
+
+  /**
+   * Execute a given function with input parameter from the members of a list.
+   *
+   * @param list the input parameters
+   * @param function the function to be executed
+   * @param <T> parameter type
+   * @param <K> return value type
+   * @param <E> the exception type thrown by the given function.
+   * @return the value returned by the given function.
+   */
+  public static <T, K, E extends Throwable> K runFunction(Collection<T> list, 
CheckedFunction<T, K, E> function) {
+    for (T t : list) {
+      try {
+        K ret = function.apply(t);
+        if (ret != null) {
+          return ret;
+        }
+      } catch (Throwable e) {
+        e.printStackTrace();
+      }
+    }
+    return null;
+  }
+
+
+  public static List<RaftPeer> buildRaftPeersFromStr(String peers) {
+    List<InetSocketAddress> addresses = new ArrayList<>();
+    String[] peersArray = peers.split(",");
+    for (String peer : peersArray) {
+      addresses.add(parseInetSocketAddress(peer));
+    }
+
+    return addresses.stream()
+        .map(addr -> RaftPeer.newBuilder()
+            .setId(RaftUtils.getPeerId(addr))
+            .setAddress(addr)
+            .build()
+        ).collect(Collectors.toList());
+  }
+
+  public static RaftGroupId buildRaftGroupIdFromStr(String groupId) {
+    return (groupId != null && !groupId.equals("")) ? 
RaftGroupId.valueOf(UUID.fromString(groupId))
+        : DEFAULT_RAFT_GROUP_ID;
+  }
+
+  public static RaftGroupId retrieveRemoteGroupId(RaftGroupId 
raftGroupIdFromConfig,
+                                                  List<RaftPeer> peers,
+                                                  RaftClient client, 
PrintStream printStream) throws IOException {
+    RaftGroupId remoteGroupId;
+    if (raftGroupIdFromConfig != DEFAULT_RAFT_GROUP_ID) {
+      return raftGroupIdFromConfig;
+    } else {
+      final List<RaftGroupId> groupIds = runFunction(peers,
+          p -> client.getGroupManagementApi((p.getId())).list().getGroupIds());
+
+      if (groupIds == null) {
+        printStream.println("Failed to get group ID from " + peers);
+        throw new IOException("Failed to get group ID from " + peers);
+      } else if (groupIds.size() == 1) {
+        remoteGroupId = groupIds.get(0);
+      } else {
+        printStream.println("There are more than one groups, you should 
specific one. " + groupIds);

Review Comment:
   Let's also update the message.
   ```
         printStream.println("Unexpected multiple group IDs " + groupIds
             + ".  In such case, the target group ID must be specified.");
   ```



##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/command/AbstractRatisCommand.java:
##########
@@ -168,14 +148,8 @@ protected RaftPeerProto getLeader(RoleInfoProto roleInfo) {
   }
 
   protected void processReply(RaftClientReply reply, Supplier<String> 
messageSupplier) throws IOException {
-    if (reply == null || !reply.isSuccess()) {
-      final RaftException e = Optional.ofNullable(reply)
-          .map(RaftClientReply::getException)
-          .orElseGet(() -> new RaftException("Reply: " + reply));
-      final String message = messageSupplier.get();
-      printf("%s. Error: %s%n", message, e);
-      throw new IOException(message, e);
-    }
+    RaftUtils.processReply(reply,
+        getPrintStream()::println, messageSupplier.get());

Review Comment:
   Make it one line.



##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/RaftUtils.java:
##########
@@ -86,4 +103,111 @@ public static RaftClient createClient(RaftGroup raftGroup) 
{
         .setRetryPolicy(retryPolicy)
         .build();
   }
+
+
+  /**
+   * Execute a given function with input parameter from the members of a list.
+   *
+   * @param list the input parameters
+   * @param function the function to be executed
+   * @param <T> parameter type
+   * @param <K> return value type
+   * @param <E> the exception type thrown by the given function.
+   * @return the value returned by the given function.
+   */
+  public static <T, K, E extends Throwable> K runFunction(Collection<T> list, 
CheckedFunction<T, K, E> function) {
+    for (T t : list) {
+      try {
+        K ret = function.apply(t);
+        if (ret != null) {
+          return ret;
+        }
+      } catch (Throwable e) {
+        e.printStackTrace();
+      }
+    }
+    return null;
+  }
+
+
+  public static List<RaftPeer> buildRaftPeersFromStr(String peers) {
+    List<InetSocketAddress> addresses = new ArrayList<>();
+    String[] peersArray = peers.split(",");
+    for (String peer : peersArray) {
+      addresses.add(parseInetSocketAddress(peer));
+    }
+
+    return addresses.stream()
+        .map(addr -> RaftPeer.newBuilder()
+            .setId(RaftUtils.getPeerId(addr))
+            .setAddress(addr)
+            .build()
+        ).collect(Collectors.toList());
+  }
+
+  public static RaftGroupId buildRaftGroupIdFromStr(String groupId) {
+    return (groupId != null && !groupId.equals("")) ? 
RaftGroupId.valueOf(UUID.fromString(groupId))
+        : DEFAULT_RAFT_GROUP_ID;
+  }
+
+  public static RaftGroupId retrieveRemoteGroupId(RaftGroupId 
raftGroupIdFromConfig,
+                                                  List<RaftPeer> peers,
+                                                  RaftClient client, 
PrintStream printStream) throws IOException {
+    RaftGroupId remoteGroupId;
+    if (raftGroupIdFromConfig != DEFAULT_RAFT_GROUP_ID) {
+      return raftGroupIdFromConfig;
+    } else {
+      final List<RaftGroupId> groupIds = runFunction(peers,
+          p -> client.getGroupManagementApi((p.getId())).list().getGroupIds());
+
+      if (groupIds == null) {
+        printStream.println("Failed to get group ID from " + peers);
+        throw new IOException("Failed to get group ID from " + peers);
+      } else if (groupIds.size() == 1) {
+        remoteGroupId = groupIds.get(0);
+      } else {
+        printStream.println("There are more than one groups, you should 
specific one. " + groupIds);
+        throw new IOException("There are more than one groups, you should 
specific one. " + groupIds);
+      }
+    }
+
+    return remoteGroupId;
+  }
+
+  public static GroupInfoReply retrieveGroupInfoByGroupId(RaftGroupId 
remoteGroupId, List<RaftPeer> peers,
+                                                          RaftClient client, 
PrintStream printStream)
+      throws IOException {
+    GroupInfoReply groupInfoReply = runFunction(peers,
+        p -> client.getGroupManagementApi((p.getId())).info(remoteGroupId));
+    processReply(groupInfoReply,
+        printStream::println, "Failed to get group info for group id " + 
remoteGroupId.getUuid() + " from " + peers);

Review Comment:
   Use supplier
   ```java
       processReply(groupInfoReply, printStream::println,
           () -> "Failed to get group info for group id " + 
remoteGroupId.getUuid() + " from " + peers);
   ```



##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/RaftUtils.java:
##########
@@ -86,4 +103,111 @@ public static RaftClient createClient(RaftGroup raftGroup) 
{
         .setRetryPolicy(retryPolicy)
         .build();
   }
+
+
+  /**
+   * Execute a given function with input parameter from the members of a list.
+   *
+   * @param list the input parameters
+   * @param function the function to be executed
+   * @param <T> parameter type
+   * @param <K> return value type
+   * @param <E> the exception type thrown by the given function.
+   * @return the value returned by the given function.
+   */
+  public static <T, K, E extends Throwable> K runFunction(Collection<T> list, 
CheckedFunction<T, K, E> function) {
+    for (T t : list) {
+      try {
+        K ret = function.apply(t);
+        if (ret != null) {
+          return ret;
+        }
+      } catch (Throwable e) {
+        e.printStackTrace();
+      }
+    }
+    return null;
+  }
+
+
+  public static List<RaftPeer> buildRaftPeersFromStr(String peers) {
+    List<InetSocketAddress> addresses = new ArrayList<>();
+    String[] peersArray = peers.split(",");
+    for (String peer : peersArray) {
+      addresses.add(parseInetSocketAddress(peer));
+    }
+
+    return addresses.stream()
+        .map(addr -> RaftPeer.newBuilder()
+            .setId(RaftUtils.getPeerId(addr))
+            .setAddress(addr)
+            .build()
+        ).collect(Collectors.toList());
+  }
+
+  public static RaftGroupId buildRaftGroupIdFromStr(String groupId) {
+    return (groupId != null && !groupId.equals("")) ? 
RaftGroupId.valueOf(UUID.fromString(groupId))
+        : DEFAULT_RAFT_GROUP_ID;
+  }
+
+  public static RaftGroupId retrieveRemoteGroupId(RaftGroupId 
raftGroupIdFromConfig,
+                                                  List<RaftPeer> peers,
+                                                  RaftClient client, 
PrintStream printStream) throws IOException {
+    RaftGroupId remoteGroupId;
+    if (raftGroupIdFromConfig != DEFAULT_RAFT_GROUP_ID) {
+      return raftGroupIdFromConfig;
+    } else {
+      final List<RaftGroupId> groupIds = runFunction(peers,
+          p -> client.getGroupManagementApi((p.getId())).list().getGroupIds());
+
+      if (groupIds == null) {
+        printStream.println("Failed to get group ID from " + peers);
+        throw new IOException("Failed to get group ID from " + peers);
+      } else if (groupIds.size() == 1) {
+        remoteGroupId = groupIds.get(0);
+      } else {
+        printStream.println("There are more than one groups, you should 
specific one. " + groupIds);
+        throw new IOException("There are more than one groups, you should 
specific one. " + groupIds);
+      }
+    }
+
+    return remoteGroupId;
+  }

Review Comment:
   Move `remoteGroupId`, add `final` and remove `else`, i.e.
   ```java
     public static RaftGroupId retrieveRemoteGroupId(RaftGroupId 
raftGroupIdFromConfig,
                                                     List<RaftPeer> peers,
                                                     RaftClient client, 
PrintStream printStream) throws IOException {
       if (!DEFAULT_RAFT_GROUP_ID .equals(raftGroupIdFromConfig)) {
         return raftGroupIdFromConfig;
       }
   
       final RaftGroupId remoteGroupId;
       final List<RaftGroupId> groupIds = applyFunctionReturnFirstNonNull(peers,
           p -> client.getGroupManagementApi((p.getId())).list().getGroupIds());
   
       if (groupIds == null) {
         printStream.println("Failed to get group ID from " + peers);
         throw new IOException("Failed to get group ID from " + peers);
       } else if (groupIds.size() == 1) {
         remoteGroupId = groupIds.get(0);
       } else {
         printStream.println("Unexpected multiple group IDs " + groupIds
             + ".  In such case, the target group ID must be specified.");
         throw new IOException("There are more than one groups, you should 
specific one. " + groupIds);
       }
       return remoteGroupId;
     }
   ```



##########
ratis-shell/src/main/java/org/apache/ratis/shell/cli/RaftUtils.java:
##########
@@ -86,4 +103,111 @@ public static RaftClient createClient(RaftGroup raftGroup) 
{
         .setRetryPolicy(retryPolicy)
         .build();
   }
+
+
+  /**
+   * Execute a given function with input parameter from the members of a list.
+   *
+   * @param list the input parameters
+   * @param function the function to be executed
+   * @param <T> parameter type
+   * @param <K> return value type
+   * @param <E> the exception type thrown by the given function.
+   * @return the value returned by the given function.
+   */
+  public static <T, K, E extends Throwable> K runFunction(Collection<T> list, 
CheckedFunction<T, K, E> function) {
+    for (T t : list) {
+      try {
+        K ret = function.apply(t);
+        if (ret != null) {
+          return ret;
+        }
+      } catch (Throwable e) {
+        e.printStackTrace();
+      }
+    }
+    return null;
+  }
+
+
+  public static List<RaftPeer> buildRaftPeersFromStr(String peers) {
+    List<InetSocketAddress> addresses = new ArrayList<>();
+    String[] peersArray = peers.split(",");
+    for (String peer : peersArray) {
+      addresses.add(parseInetSocketAddress(peer));
+    }
+
+    return addresses.stream()
+        .map(addr -> RaftPeer.newBuilder()
+            .setId(RaftUtils.getPeerId(addr))
+            .setAddress(addr)
+            .build()
+        ).collect(Collectors.toList());
+  }
+
+  public static RaftGroupId buildRaftGroupIdFromStr(String groupId) {
+    return (groupId != null && !groupId.equals("")) ? 
RaftGroupId.valueOf(UUID.fromString(groupId))
+        : DEFAULT_RAFT_GROUP_ID;
+  }
+
+  public static RaftGroupId retrieveRemoteGroupId(RaftGroupId 
raftGroupIdFromConfig,
+                                                  List<RaftPeer> peers,
+                                                  RaftClient client, 
PrintStream printStream) throws IOException {
+    RaftGroupId remoteGroupId;
+    if (raftGroupIdFromConfig != DEFAULT_RAFT_GROUP_ID) {

Review Comment:
   Use `!DEFAULT_RAFT_GROUP_ID .equals(raftGroupIdFromConfig)`.
   



-- 
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: issues-unsubscr...@ratis.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to