From 69092a3f267b67c8067c19f98ab08bad2f936aad Mon Sep 17 00:00:00 2001
From: Matt Ranney <mjr@ranney.com>
Date: Tue, 15 Nov 2011 15:21:49 -1000
Subject: [PATCH] [GH-67] - hgetall now returns null instead of {} on empty
 reply

---
 README.md |  5 +++--
 index.js  | 37 +++++++++++++++++++++----------------
 test.js   |  4 ++--
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/README.md b/README.md
index 02a873b191..fba431d2b9 100644
--- a/README.md
+++ b/README.md
@@ -206,7 +206,8 @@ to start over.
 
 ## Friendlier hash commands
 
-Most Redis commands take a single String or an Array of Strings as arguments, and replies are sent back as a single String or an Array of Strings.  When dealing with hash values, there are a couple of useful exceptions to this.
+Most Redis commands take a single String or an Array of Strings as arguments, and replies are sent back as a single String or an Array of Strings.
+When dealing with hash values, there are a couple of useful exceptions to this.
 
 ### client.hgetall(hash)
 
@@ -512,7 +513,7 @@ Defaults to 1.7.  The default initial connection retry is 250, so the second ret
 
 ## TODO
 
-Better tests for monitor mode, auth, disconnect/reconnect, and all combinations thereof.
+Better tests for auth, disconnect/reconnect, and all combinations thereof.
 
 Stream large set/get values into and out of Redis.  Otherwise the entire value must be in node's memory.
 
diff --git a/index.js b/index.js
index 5edd3bffb6..fe10451412 100644
--- a/index.js
+++ b/index.js
@@ -471,6 +471,23 @@ function try_callback(callback, reply) {
     }
 }
 
+// hgetall converts its replies to an Object.  If the reply is empty, null is returned.
+function reply_to_object(reply) {
+    var obj = {}, j, jl, key, val;
+
+    if (reply.length === 0) {
+        return null;
+    }
+
+    for (j = 0, jl = reply.length; j < jl; j += 2) {
+        key = reply[j].toString();
+        val = reply[j + 1];
+        obj[key] = val;
+    }
+
+    return obj;
+}
+
 RedisClient.prototype.return_reply = function (reply) {
     var command_obj, obj, i, len, key, val, type, timestamp, argindex, args, queue_len;
     
@@ -489,15 +506,9 @@ RedisClient.prototype.return_reply = function (reply) {
 
     if (command_obj && !command_obj.sub_command) {
         if (typeof command_obj.callback === "function") {
-            // HGETALL special case replies with keyed Buffers
+            // TODO - confusing and error-prone that hgetall is special cased in two places
             if (reply && 'hgetall' === command_obj.command.toLowerCase()) {
-                obj = {};
-                for (i = 0, len = reply.length; i < len; i += 2) {
-                    key = reply[i].toString();
-                    val = reply[i + 1];
-                    obj[key] = val;
-                }
-                reply = obj;
+                reply = reply_to_object(reply);
             }
 
             try_callback(command_obj.callback, reply);
@@ -901,15 +912,9 @@ Multi.prototype.exec = function (callback) {
                 reply = replies[i - 1];
                 args = self.queue[i];
 
-                // Convert HGETALL reply to object
+                // TODO - confusing and error-prone that hgetall is special cased in two places
                 if (reply && args[0].toLowerCase() === "hgetall") {
-                    obj = {};
-                    for (j = 0, jl = reply.length; j < jl; j += 2) {
-                        key = reply[j].toString();
-                        val = reply[j + 1];
-                        obj[key] = val;
-                    }
-                    replies[i - 1] = reply = obj;
+                    replies[i - 1] = reply = reply_to_object(reply);
                 }
 
                 if (typeof args[args.length - 1] === "function") {
diff --git a/test.js b/test.js
index 3c68d40009..d960928f0d 100644
--- a/test.js
+++ b/test.js
@@ -670,9 +670,9 @@ tests.HGETALL = function () {
 tests.HGETALL_NULL = function () {
     var name = "HGETALL_NULL";
 
-    client.hgetall('missing', function (err, obj) {
+    client.hgetall("missing", function (err, obj) {
         assert.strictEqual(null, err);
-        assert.deepEqual([], obj);
+        assert.strictEqual(null, obj);
         next(name);
     });
 };