COMMAND INFO reply contains subcommand detail (redis#4022)

* COMMAND INFO reply contains subcommand detail * Add 'name' in CommandInfo Fixes redis#4020 ___ * COMMAND INFO reply contains subcommand detail * Add 'name' in CommandInfo * Safeguard test from future ACL subcommands * Ensure reply size * Move null checking into COMMAND_INFO_BUILDER from inside loop and reduce parsing 'name' multiple times
sdotsenko · Nov 11, 2024 · 2fdf928 · 2fdf928
1 parent 9b88636
commit 2fdf928
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 30 deletions.
diff --git a/src/main/java/redis/clients/jedis/BuilderFactory.java b/src/main/java/redis/clients/jedis/BuilderFactory.java
@@ -975,30 +975,8 @@ public Map<String, CommandDocument> build(Object data) {
     }
   };
 
-  public static final Builder<Map<String, CommandInfo>> COMMAND_INFO_RESPONSE = new Builder<Map<String, CommandInfo>>() {
-    @Override
-    public Map<String, CommandInfo> build(Object data) {
-      if (data == null) {
-        return null;
-      }
-
-      List<Object> rawList = (List<Object>) data;
-      Map<String, CommandInfo> map = new HashMap<>(rawList.size());
-
-      for (Object rawCommandInfo : rawList) {
-        if (rawCommandInfo == null) {
-          continue;
-        }
-
-        List<Object> commandInfo = (List<Object>) rawCommandInfo;
-        String name = STRING.build(commandInfo.get(0));
-        CommandInfo info = CommandInfo.COMMAND_INFO_BUILDER.build(commandInfo);
-        map.put(name, info);
-      }
-
-      return map;
-    }
-  };
+  @Deprecated
+  public static final Builder<Map<String, CommandInfo>> COMMAND_INFO_RESPONSE = CommandInfo.COMMAND_INFO_RESPONSE;
 
   public static final Builder<Map<String, LatencyLatestInfo>> LATENCY_LATEST_RESPONSE = new Builder<Map<String, LatencyLatestInfo>>() {
     @Override

diff --git a/src/main/java/redis/clients/jedis/Jedis.java b/src/main/java/redis/clients/jedis/Jedis.java
@@ -8250,7 +8250,7 @@ public List<KeyValue<String, List<String>>> commandGetKeysAndFlags(String... com
   public Map<String, CommandInfo> commandInfo(String... commands) {
     checkIsInMultiOrPipeline();
     connection.sendCommand(COMMAND, joinParameters(Keyword.INFO.name(), commands));
-    return BuilderFactory.COMMAND_INFO_RESPONSE.build(connection.getOne());
+    return CommandInfo.COMMAND_INFO_RESPONSE.build(connection.getOne());
   }
 
   public List<String> commandList() {

diff --git a/src/main/java/redis/clients/jedis/resps/CommandInfo.java b/src/main/java/redis/clients/jedis/resps/CommandInfo.java
@@ -2,23 +2,40 @@
 
 import redis.clients.jedis.Builder;
 
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
-import static redis.clients.jedis.BuilderFactory.STRING_LIST;
 import static redis.clients.jedis.BuilderFactory.LONG;
+import static redis.clients.jedis.BuilderFactory.STRING;
+import static redis.clients.jedis.BuilderFactory.STRING_LIST;
 
 public class CommandInfo {
+
+  private final String name;
   private final long arity;
   private final List<String> flags;
   private final long firstKey;
   private final long lastKey;
   private final long step;
   private final List<String> aclCategories;
   private final List<String> tips;
-  private final List<String> subcommands;
+  private final Map<String, CommandInfo> subcommands;
 
+  /**
+   * THIS IGNORES 'subcommands' parameter.
+   * @param subcommands WILL BE IGNORED
+   * @deprecated
+   */
+  @Deprecated
   public CommandInfo(long arity, List<String> flags, long firstKey, long lastKey, long step,
       List<String> aclCategories, List<String> tips, List<String> subcommands) {
+    this((String) null, arity, flags, firstKey, lastKey, step, aclCategories, tips, (Map) null);
+  }
+
+  private CommandInfo(String name, long arity, List<String> flags, long firstKey, long lastKey, long step,
+      List<String> aclCategories, List<String> tips, Map<String, CommandInfo> subcommands) {
+    this.name = name;
     this.arity = arity;
     this.flags = flags;
     this.firstKey = firstKey;
@@ -29,6 +46,13 @@ public CommandInfo(long arity, List<String> flags, long firstKey, long lastKey,
     this.subcommands = subcommands;
   }
 
+  /**
+   * Command name
+   */
+  public String getName() {
+    return name;
+  }
+
   /**
    * Arity is the number of arguments a command expects. It follows a simple pattern:
    * A positive integer means a fixed number of arguments.
@@ -89,25 +113,55 @@ public List<String> getTips() {
   /**
    * All the command's subcommands, if any
    */
-  public List<String> getSubcommands() {
+  public Map<String, CommandInfo> getSubcommands() {
     return subcommands;
   }
 
   public static final Builder<CommandInfo> COMMAND_INFO_BUILDER = new Builder<CommandInfo>() {
     @Override
     public CommandInfo build(Object data) {
+      if (data == null) {
+        return null;
+      }
+
       List<Object> commandData = (List<Object>) data;
+      if (commandData.isEmpty()) {
+        return null;
+      }
 
+      String name = STRING.build(commandData.get(0));
       long arity = LONG.build(commandData.get(1));
       List<String> flags = STRING_LIST.build(commandData.get(2));
       long firstKey = LONG.build(commandData.get(3));
       long lastKey = LONG.build(commandData.get(4));
       long step = LONG.build(commandData.get(5));
       List<String> aclCategories = STRING_LIST.build(commandData.get(6));
       List<String> tips = STRING_LIST.build(commandData.get(7));
-      List<String> subcommands = STRING_LIST.build(commandData.get(9));
+      Map<String, CommandInfo> subcommands = COMMAND_INFO_RESPONSE.build(commandData.get(9));
+
+      return new CommandInfo(name, arity, flags, firstKey, lastKey, step, aclCategories, tips, subcommands);
+    }
+  };
 
-      return new CommandInfo(arity, flags, firstKey, lastKey, step, aclCategories, tips, subcommands);
+  public static final Builder<Map<String, CommandInfo>> COMMAND_INFO_RESPONSE = new Builder<Map<String, CommandInfo>>() {
+    @Override
+    public Map<String, CommandInfo> build(Object data) {
+      if (data == null) {
+        return null;
+      }
+
+      List<Object> rawList = (List<Object>) data;
+      Map<String, CommandInfo> map = new HashMap<>(rawList.size());
+
+      for (Object rawCommandInfo : rawList) {
+        CommandInfo info = CommandInfo.COMMAND_INFO_BUILDER.build(rawCommandInfo);
+        if (info != null) {
+          map.put(info.getName(), info);
+        }
+      }
+
+      return map;
     }
   };
+
 }
diff --git a/src/test/java/redis/clients/jedis/commands/jedis/ControlCommandsTest.java b/src/test/java/redis/clients/jedis/commands/jedis/ControlCommandsTest.java
@@ -520,6 +520,27 @@ public void commandInfo() {
     assertEquals(0, setInfo.getSubcommands().size());
   }
 
+  @Test // GitHub Issue #4020
+  public void commandInfoAcl() {
+    Map<String, CommandInfo> infos = jedis.commandInfo("ACL");
+    assertThat(infos, Matchers.aMapWithSize(1));
+
+    CommandInfo aclInfo = infos.get("acl");
+    assertEquals(-2, aclInfo.getArity());
+    assertEquals(0, aclInfo.getFlags().size());
+    assertEquals(0, aclInfo.getFirstKey());
+    assertEquals(0, aclInfo.getLastKey());
+    assertEquals(0, aclInfo.getStep());
+    assertEquals(1, aclInfo.getAclCategories().size());
+    assertEquals(0, aclInfo.getTips().size());
+    assertThat(aclInfo.getSubcommands().size(), Matchers.greaterThanOrEqualTo(13));
+    aclInfo.getSubcommands().forEach((name, subcommand) -> {
+      assertThat(name, Matchers.startsWith("acl|"));
+      assertNotNull(subcommand);
+      assertEquals(name, subcommand.getName());
+    });
+  }
+
   @Test
   public void commandList() {
     List<String> commands = jedis.commandList();