Fix return value of HRANDFIELD With Values when count is negative

src/main/java/redis/clients/jedis/BuilderFactory.java

@@ -241,23 +241,43 @@ public String toString() {
     }
   };
 
-  public static final Builder<Map<byte[], byte[]>> BINARY_MAP_FROM_PAIRS = new Builder<Map<byte[], byte[]>>() {
+  public static final Builder<List<Map.Entry<byte[], byte[]>>> BINARY_PAIR_LIST = new Builder<List<Map.Entry<byte[], byte[]>>>() {
     @Override
     @SuppressWarnings("unchecked")
-    public Map<byte[], byte[]> build(Object data) {
+    public List<Map.Entry<byte[], byte[]>> build(Object data) {
+      final List<byte[]> flatHash = (List<byte[]>) data;
+      final List<Map.Entry<byte[], byte[]>> pairList = new ArrayList<>();
+      final Iterator<byte[]> iterator = flatHash.iterator();
+      while (iterator.hasNext()) {
+        pairList.add(new AbstractMap.SimpleEntry<>(iterator.next(), iterator.next()));
+      }
+
+      return pairList;
+    }
+
+    @Override
+    public String toString() {
+      return "List<Map.Entry<byte[], byte[]>>";
+    }
+  };
+
+  public static final Builder<List<Map.Entry<byte[], byte[]>>> BINARY_PAIR_LIST_FROM_PAIRS = new Builder<List<Map.Entry<byte[], byte[]>>>() {
+    @Override
+    @SuppressWarnings("unchecked")
+    public List<Map.Entry<byte[], byte[]>> build(Object data) {
       final List<Object> list = (List<Object>) data;
-      final Map<byte[], byte[]> map = new JedisByteHashMap();
+      final List<Map.Entry<byte[], byte[]>> pairList = new ArrayList<>();
       for (Object object : list) {
         final List<byte[]> flat = (List<byte[]>) object;
-        map.put(flat.get(0), flat.get(1));
+        pairList.add(new AbstractMap.SimpleEntry<>(flat.get(0), flat.get(1)));
       }
 
-      return map;
+      return pairList;
     }
 
     @Override
     public String toString() {
-      return "Map<byte[], byte[]>";
+      return "List<Map.Entry<byte[], byte[]>>";
     }
   };
 
@@ -335,6 +355,48 @@ public String toString() {
     }
   };
 
+  public static final Builder<List<Map.Entry<String, String>>> STRING_PAIR_LIST = new Builder<List<Map.Entry<String, String>>>() {
+    @Override
+    @SuppressWarnings("unchecked")
+    public List<Map.Entry<String, String>> build(Object data) {
+      final List<byte[]> flatHash = (List<byte[]>) data;
+      final List<Map.Entry<String, String>> pairList = new ArrayList<>();
+      final Iterator<byte[]> iterator = flatHash.iterator();
+      while (iterator.hasNext()) {
+        pairList.add(new AbstractMap.SimpleEntry<>(STRING.build(iterator.next()), STRING.build(iterator.next())));
+      }
+
+      return pairList;
+    }
+
+    @Override
+    public String toString() {
+      return "List<Map.Entry<String, String>>";
+    }
+
+  };
+
+  public static final Builder<List<Map.Entry<String, String>>> STRING_PAIR_LIST_FROM_PAIRS = new Builder<List<Map.Entry<String, String>>>() {
+    @Override
+    @SuppressWarnings("unchecked")
+    public List<Map.Entry<String, String>> build(Object data) {
+      final List<Object> list = (List<Object>) data;
+      final List<Map.Entry<String, String>> pairList = new ArrayList<>();
+      for (Object object : list) {
+        final List<byte[]> flat = (List<byte[]>) object;
+        pairList.add(new AbstractMap.SimpleEntry<>(SafeEncoder.encode(flat.get(0)), SafeEncoder.encode(flat.get(1))));
+      }
+
+      return pairList;
+    }
+
+    @Override
+    public String toString() {
+      return "List<Map.Entry<String, String>>";
+    }
+
+  };
+
   public static final Builder<KeyedListElement> KEYED_LIST_ELEMENT = new Builder<KeyedListElement>() {
     @Override
     @SuppressWarnings("unchecked")

src/main/java/redis/clients/jedis/CommandObjects.java

@@ -1090,9 +1090,9 @@ public final CommandObject<List<String>> hrandfield(String key, long count) {
     return new CommandObject<>(commandArguments(HRANDFIELD).key(key).add(count), BuilderFactory.STRING_LIST);
   }
 
-  public final CommandObject<Map<String, String>> hrandfieldWithValues(String key, long count) {
+  public final CommandObject<List<Map.Entry<String, String>>> hrandfieldWithValues(String key, long count) {
     return new CommandObject<>(commandArguments(HRANDFIELD).key(key).add(count).add(WITHVALUES),
-        proto != RedisProtocol.RESP3 ? BuilderFactory.STRING_MAP : BuilderFactory.STRING_MAP_FROM_PAIRS);
+        proto != RedisProtocol.RESP3 ? BuilderFactory.STRING_PAIR_LIST : BuilderFactory.STRING_PAIR_LIST_FROM_PAIRS);
   }
 
   public final CommandObject<Map<byte[], byte[]>> hgetAll(byte[] key) {
@@ -1107,9 +1107,9 @@ public final CommandObject<List<byte[]>> hrandfield(byte[] key, long count) {
     return new CommandObject<>(commandArguments(HRANDFIELD).key(key).add(count), BuilderFactory.BINARY_LIST);
   }
 
-  public final CommandObject<Map<byte[], byte[]>> hrandfieldWithValues(byte[] key, long count) {
+  public final CommandObject<List<Map.Entry<byte[], byte[]>>> hrandfieldWithValues(byte[] key, long count) {
     return new CommandObject<>(commandArguments(HRANDFIELD).key(key).add(count).add(WITHVALUES),
-        proto != RedisProtocol.RESP3 ? BuilderFactory.BINARY_MAP : BuilderFactory.BINARY_MAP_FROM_PAIRS);
+        proto != RedisProtocol.RESP3 ? BuilderFactory.BINARY_PAIR_LIST : BuilderFactory.BINARY_PAIR_LIST_FROM_PAIRS);
   }
 
   public final CommandObject<ScanResult<Map.Entry<String, String>>> hscan(String key, String cursor, ScanParams params) {

src/main/java/redis/clients/jedis/Jedis.java

@@ -1372,7 +1372,7 @@ public List<byte[]> hrandfield(final byte[] key, final long count) {
    * @return One or multiple random fields with values from a hash.
    */
   @Override
-  public Map<byte[], byte[]> hrandfieldWithValues(final byte[] key, final long count) {
+  public List<Map.Entry<byte[], byte[]>> hrandfieldWithValues(final byte[] key, final long count) {
     checkIsInMultiOrPipeline();
     return connection.executeCommand(commandObjects.hrandfieldWithValues(key, count));
   }
@@ -5799,7 +5799,7 @@ public List<String> hrandfield(final String key, final long count) {
    * @return one or multiple random fields with values from a hash.
    */
   @Override
-  public Map<String, String> hrandfieldWithValues(final String key, final long count) {
+  public List<Map.Entry<String, String>> hrandfieldWithValues(final String key, final long count) {
     checkIsInMultiOrPipeline();
     return connection.executeCommand(commandObjects.hrandfieldWithValues(key, count));
   }

src/main/java/redis/clients/jedis/MultiNodePipelineBase.java

@@ -807,7 +807,7 @@ public Response<List<String>> hrandfield(String key, long count) {
   }
 
   @Override
-  public Response<Map<String, String>> hrandfieldWithValues(String key, long count) {
+  public Response<List<Map.Entry<String, String>>> hrandfieldWithValues(String key, long count) {
     return appendCommand(commandObjects.hrandfieldWithValues(key, count));
   }
 
@@ -2067,7 +2067,7 @@ public Response<List<byte[]>> hrandfield(byte[] key, long count) {
   }
 
   @Override
-  public Response<Map<byte[], byte[]>> hrandfieldWithValues(byte[] key, long count) {
+  public Response<List<Map.Entry<byte[], byte[]>>> hrandfieldWithValues(byte[] key, long count) {
     return appendCommand(commandObjects.hrandfieldWithValues(key, count));
   }
 

src/main/java/redis/clients/jedis/Pipeline.java

@@ -761,7 +761,7 @@ public Response<List<String>> hrandfield(String key, long count) {
   }
 
   @Override
-  public Response<Map<String, String>> hrandfieldWithValues(String key, long count) {
+  public Response<List<Map.Entry<String, String>>> hrandfieldWithValues(String key, long count) {
     return appendCommand(commandObjects.hrandfieldWithValues(key, count));
   }
 
@@ -2020,7 +2020,7 @@ public Response<List<byte[]>> hrandfield(byte[] key, long count) {
   }
 
   @Override
-  public Response<Map<byte[], byte[]>> hrandfieldWithValues(byte[] key, long count) {
+  public Response<List<Map.Entry<byte[], byte[]>>> hrandfieldWithValues(byte[] key, long count) {
     return appendCommand(commandObjects.hrandfieldWithValues(key, count));
   }
 

src/main/java/redis/clients/jedis/TransactionBase.java

@@ -856,7 +856,7 @@ public Response<List<String>> hrandfield(String key, long count) {
   }
 
   @Override
-  public Response<Map<String, String>> hrandfieldWithValues(String key, long count) {
+  public Response<List<Map.Entry<String, String>>> hrandfieldWithValues(String key, long count) {
     return appendCommand(commandObjects.hrandfieldWithValues(key, count));
   }
 
@@ -2117,7 +2117,7 @@ public Response<List<byte[]>> hrandfield(byte[] key, long count) {
   }
 
   @Override
-  public Response<Map<byte[], byte[]>> hrandfieldWithValues(byte[] key, long count) {
+  public Response<List<Map.Entry<byte[], byte[]>>> hrandfieldWithValues(byte[] key, long count) {
     return appendCommand(commandObjects.hrandfieldWithValues(key, count));
   }
 

src/main/java/redis/clients/jedis/UnifiedJedis.java

@@ -1517,7 +1517,7 @@ public List<String> hrandfield(String key, long count) {
   }
 
   @Override
-  public Map<String, String> hrandfieldWithValues(String key, long count) {
+  public List<Map.Entry<String, String>> hrandfieldWithValues(String key, long count) {
     return executeCommand(commandObjects.hrandfieldWithValues(key, count));
   }
 
@@ -1542,7 +1542,7 @@ public List<byte[]> hrandfield(byte[] key, long count) {
   }
 
   @Override
-  public Map<byte[], byte[]> hrandfieldWithValues(byte[] key, long count) {
+  public List<Map.Entry<byte[], byte[]>> hrandfieldWithValues(byte[] key, long count) {
     return executeCommand(commandObjects.hrandfieldWithValues(key, count));
   }
 

src/main/java/redis/clients/jedis/commands/HashBinaryCommands.java

@@ -41,7 +41,7 @@ public interface HashBinaryCommands {
 
   List<byte[]> hrandfield(byte[] key, long count);
 
-  Map<byte[], byte[]> hrandfieldWithValues(byte[] key, long count);
+  List<Map.Entry<byte[], byte[]>> hrandfieldWithValues(byte[] key, long count);
 
   default ScanResult<Map.Entry<byte[], byte[]>> hscan(byte[] key, byte[] cursor) {
     return hscan(key, cursor, new ScanParams());

src/main/java/redis/clients/jedis/commands/HashCommands.java

@@ -41,7 +41,7 @@ public interface HashCommands {
 
   List<String> hrandfield(String key, long count);
 
-  Map<String, String> hrandfieldWithValues(String key, long count);
+  List<Map.Entry<String, String>> hrandfieldWithValues(String key, long count);
 
   default ScanResult<Map.Entry<String, String>> hscan(String key, String cursor) {
     return hscan(key, cursor, new ScanParams());

src/main/java/redis/clients/jedis/commands/HashPipelineBinaryCommands.java

@@ -42,7 +42,7 @@ public interface HashPipelineBinaryCommands {
 
   Response<List<byte[]>> hrandfield(byte[] key, long count);
 
-  Response<Map<byte[], byte[]>> hrandfieldWithValues(byte[] key, long count);
+  Response<List<Map.Entry<byte[], byte[]>>> hrandfieldWithValues(byte[] key, long count);
 
   default Response<ScanResult<Map.Entry<byte[], byte[]>>> hscan(byte[] key, byte[] cursor) {
     return hscan(key, cursor, new ScanParams());

src/main/java/redis/clients/jedis/commands/HashPipelineCommands.java

@@ -42,7 +42,7 @@ public interface HashPipelineCommands {
 
   Response<List<String>> hrandfield(String key, long count);
 
-  Response<Map<String, String>> hrandfieldWithValues(String key, long count);
+  Response<List<Map.Entry<String, String>>> hrandfieldWithValues(String key, long count);
 
   default Response<ScanResult<Map.Entry<String, String>>> hscan(String key, String cursor) {
     return hscan(key, cursor, new ScanParams());

src/test/java/redis/clients/jedis/ClusterPipeliningTest.java

@@ -9,6 +9,7 @@
 import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -650,8 +651,9 @@ public void clusterPipelineHash() {
     Response<List<String>> r12 = p.hmget("myhash", "field1", "field2");
     Response<String> r13 = p.hrandfield("myotherhash");
     Response<List<String>> r14 = p.hrandfield("myotherhash", 2);
-    Response<Map<String, String>> r15 = p.hrandfieldWithValues("myotherhash", 2);
+    Response<List<Map.Entry<String, String>>> r15 = p.hrandfieldWithValues("myotherhash", 2);
     Response<Long> r16 = p.hstrlen("myhash", "field1");
+    Response<List<Map.Entry<String, String>>> r17 = p.hrandfieldWithValues("myotherhash", -2);
 
     p.sync();
     assertEquals(Long.valueOf(1), r1.get());
@@ -668,8 +670,9 @@ public void clusterPipelineHash() {
     assertEquals(vals2, r12.get());
     assertTrue(hm.keySet().contains(r13.get()));
     assertEquals(2, r14.get().size());
-    assertTrue(r15.get().containsKey("field3") && r15.get().containsValue("5"));
+    Assert.assertTrue(r15.get().contains(new AbstractMap.SimpleEntry<>("field3", "5")));
     assertEquals(Long.valueOf(5), r16.get());
+    Assert.assertTrue(r17.get().contains(new AbstractMap.SimpleEntry<>("field3", "5")) || r17.get().contains(new AbstractMap.SimpleEntry<>("field2", "2")));
   }
 
   @Test

src/test/java/redis/clients/jedis/commands/jedis/HashesCommandsTest.java

@@ -445,7 +445,8 @@ public void testBinaryHstrLen() {
   public void hrandfield() {
     assertNull(jedis.hrandfield("foo"));
     assertEquals(Collections.emptyList(), jedis.hrandfield("foo", 1));
-    assertEquals(Collections.emptyMap(), jedis.hrandfieldWithValues("foo", 1));
+    assertEquals(Collections.emptyList(), jedis.hrandfieldWithValues("foo", 1));
+    assertEquals(Collections.emptyList(), jedis.hrandfieldWithValues("foo", -1));
 
     Map<String, String> hash = new LinkedHashMap<>();
     hash.put("bar", "bar");
@@ -457,16 +458,23 @@ public void hrandfield() {
     assertTrue(hash.containsKey(jedis.hrandfield("foo")));
     assertEquals(2, jedis.hrandfield("foo", 2).size());
 
-    Map<String, String> actual = jedis.hrandfieldWithValues("foo", 2);
+    List<Map.Entry<String, String>> actual = jedis.hrandfieldWithValues("foo", 2);
     assertNotNull(actual);
     assertEquals(2, actual.size());
-    Map.Entry entry = actual.entrySet().iterator().next();
+    Map.Entry entry = actual.get(0);
+    assertEquals(hash.get(entry.getKey()), entry.getValue());
+
+    actual = jedis.hrandfieldWithValues("foo", -2);
+    assertNotNull(actual);
+    assertEquals(2, actual.size());
+    entry = actual.get(0);
     assertEquals(hash.get(entry.getKey()), entry.getValue());
 
     // binary
     assertNull(jedis.hrandfield(bfoo));
     assertEquals(Collections.emptyList(), jedis.hrandfield(bfoo, 1));
-    assertEquals(Collections.emptyMap(), jedis.hrandfieldWithValues(bfoo, 1));
+    assertEquals(Collections.emptyList(), jedis.hrandfieldWithValues(bfoo, 1));
+    assertEquals(Collections.emptyList(), jedis.hrandfieldWithValues(bfoo, -1));
 
     Map<byte[], byte[]> bhash = new JedisByteHashMap();
     bhash.put(bbar, bbar);
@@ -478,10 +486,16 @@ public void hrandfield() {
     assertTrue(bhash.containsKey(jedis.hrandfield(bfoo)));
     assertEquals(2, jedis.hrandfield(bfoo, 2).size());
 
-    Map<byte[], byte[]> bactual = jedis.hrandfieldWithValues(bfoo, 2);
+    List<Map.Entry<byte[], byte[]>> bactual = jedis.hrandfieldWithValues(bfoo, 2);
+    assertNotNull(bactual);
+    assertEquals(2, bactual.size());
+    Map.Entry bentry = bactual.get(0);
+    assertArrayEquals(bhash.get(bentry.getKey()), (byte[]) bentry.getValue());
+
+    bactual = jedis.hrandfieldWithValues(bfoo, -2);
     assertNotNull(bactual);
     assertEquals(2, bactual.size());
-    Map.Entry bentry = bactual.entrySet().iterator().next();
+    bentry = bactual.get(0);
     assertArrayEquals(bhash.get(bentry.getKey()), (byte[]) bentry.getValue());
   }
 }

src/test/java/redis/clients/jedis/commands/unified/HashesCommandsTestBase.java

@@ -421,7 +421,8 @@ public void testBinaryHstrLen() {
   public void hrandfield() {
     assertNull(jedis.hrandfield("foo"));
     assertEquals(Collections.emptyList(), jedis.hrandfield("foo", 1));
-    assertEquals(Collections.emptyMap(), jedis.hrandfieldWithValues("foo", 1));
+    assertEquals(Collections.emptyList(), jedis.hrandfieldWithValues("foo", 1));
+    assertEquals(Collections.emptyList(), jedis.hrandfieldWithValues("foo", -11));
 
     Map<String, String> hash = new LinkedHashMap<>();
     hash.put("bar", "bar");
@@ -433,16 +434,23 @@ public void hrandfield() {
     assertTrue(hash.containsKey(jedis.hrandfield("foo")));
     assertEquals(2, jedis.hrandfield("foo", 2).size());
 
-    Map<String, String> actual = jedis.hrandfieldWithValues("foo", 2);
+    List<Map.Entry<String, String>> actual = jedis.hrandfieldWithValues("foo", 2);
     assertNotNull(actual);
     assertEquals(2, actual.size());
-    Map.Entry entry = actual.entrySet().iterator().next();
+    Map.Entry entry = actual.get(0);
+    assertEquals(hash.get(entry.getKey()), entry.getValue());
+
+    actual = jedis.hrandfieldWithValues("foo", -2);
+    assertNotNull(actual);
+    assertEquals(2, actual.size());
+    entry = actual.get(0);
     assertEquals(hash.get(entry.getKey()), entry.getValue());
 
     // binary
     assertNull(jedis.hrandfield(bfoo));
     assertEquals(Collections.emptyList(), jedis.hrandfield(bfoo, 1));
-    assertEquals(Collections.emptyMap(), jedis.hrandfieldWithValues(bfoo, 1));
+    assertEquals(Collections.emptyList(), jedis.hrandfieldWithValues(bfoo, 1));
+    assertEquals(Collections.emptyList(), jedis.hrandfieldWithValues(bfoo, -1));
 
     Map<byte[], byte[]> bhash = new JedisByteHashMap();
     bhash.put(bbar, bbar);
@@ -454,10 +462,16 @@ public void hrandfield() {
     assertTrue(bhash.containsKey(jedis.hrandfield(bfoo)));
     assertEquals(2, jedis.hrandfield(bfoo, 2).size());
 
-    Map<byte[], byte[]> bactual = jedis.hrandfieldWithValues(bfoo, 2);
+    List<Map.Entry<byte[], byte[]>> bactual = jedis.hrandfieldWithValues(bfoo, 2);
+    assertNotNull(bactual);
+    assertEquals(2, bactual.size());
+    Map.Entry bentry = bactual.get(0);
+    assertArrayEquals(bhash.get(bentry.getKey()), (byte[]) bentry.getValue());
+
+    bactual = jedis.hrandfieldWithValues(bfoo, -2);
     assertNotNull(bactual);
     assertEquals(2, bactual.size());
-    Map.Entry bentry = bactual.entrySet().iterator().next();
+    bentry = bactual.get(0);
     assertArrayEquals(bhash.get(bentry.getKey()), (byte[]) bentry.getValue());
   }
 }