README,c,test: hiredis.status introduced, hiredis.{OK,QUEUED,PONG} deprecated

This solves (1) early binding problem when it was not possible to write e.g

assert(conn:command("RETURN", "FOO") == hiredis.FOO)

and (2) naming ambiguity problem, when Redis cound introduce a
new status that will clash with lua-hiredis naming (e.g. a status,
named `command`).

All hiredis.REPLY_STATUS objects are now created and cached automatically
by the MT "magic".

hiredis.{OK,QUEUED,PONG} are now deprecated in favor of their
hiredis.status.* counterparts.

Author: agladysh

README.md

@@ -50,24 +50,19 @@ Hiredis error codes (see docs), also available as `hiredis.ERR_<something>`:
   with type `hiredis.REPLY_STATUS`.
   Common status objects are available in hiredis module table:
 
-  * `hiredis.OK`
-  * `hiredis.QUEUED`
-  * `hiredis.PONG`
+  * `hiredis.status.OK`
+  * `hiredis.status.QUEUED`
+  * `hiredis.status.PONG`
 
   It is guaranteed that these object instances are always used
   for their corresponding statuses (so you can make a simple equality check).
-
-  For replies not listed above, check const-object type and value
-  directly, or just unwrap it with `hiredis.unwrap`.
+  The same is true for any other object in `hiredis.status` table.
 
   Examples:
 
-      assert(conn:command("PING") == hiredis.PONG)
-
-      local reply = assert(conn:command("TYPE", "BADKEY"))
-      assert(reply.type == hiredis.REPLY_STATUS and reply.name == "none")
-
-      assert(assert(hiredis.unwrap(conn:command("TYPE", "NAME"))) == "string")
+      assert(conn:command("PING") == hiredis.status.PONG)
+      assert(conn:command("SET", "NAME", "lua-hiredis") == hiredis.status.OK)
+      assert(conn:command("TYPE", "NAME") == hiredis.status.string)
 
 * `REDIS_REPLY_ERROR` is a const-object with type `hiredis.REPLY_ERROR`.
   Note that Redis server errors are returned as `REDIS_REPLY_ERROR` values,
@@ -96,6 +91,19 @@ Use `hiredis.unwrap_reply()` to convert const-object to regular Lua value.
 Note: Unwrapping is not done automatically to make array reply handling
 more straightforward.
 
+Deprecated features
+-------------------
+
+For backwards compatibility following status const-objects are aliased
+in the `hiredis` module table:
+
+  * `hiredis.OK = hiredis.status.OK`
+  * `hiredis.QUEUED = hiredis.status.QUEUED`
+  * `hiredis.PONG = hiredis.status.PONG`
+
+These aliases will eventually be removed in one of future releases,
+so, please, update your code to use `hiredis.status.*` instead.
+
 More information
 ----------------
 

src/lua-hiredis.c

@@ -28,6 +28,7 @@ extern "C" {
 
 #define LUAHIREDIS_CONN_MT   "lua-hiredis.connection"
 #define LUAHIREDIS_CONST_MT  "lua-hiredis.const"
+#define LUAHIREDIS_STATUS_MT "lua-hiredis.status"
 
 #define LUAHIREDIS_MAXARGS (256)
 
@@ -110,6 +111,32 @@ static int push_new_const(
   return 1;
 }
 
+static int lstatus_index(lua_State * L)
+{
+  size_t key_len = 0;
+  const char * key = NULL;
+  luaL_checktype(L, 1, LUA_TTABLE);
+  key = luaL_checklstring(L, 2, &key_len);
+
+  push_new_const(
+      L, key, key_len, REDIS_REPLY_STATUS /* status */
+    );
+  lua_rawset(L, 1); /* t[key] = status */
+
+  lua_pushlstring(L, key, key_len); /* Push the key again */
+  lua_gettable(L, 1); /* return t[key] */
+
+  return 1;
+}
+
+/* status API */
+static const struct luaL_reg STATUS_MT[] =
+{
+  { "__index", lstatus_index },
+
+  { NULL, NULL }
+};
+
 static const struct luahiredis_Enum Errors[] =
 {
   { "ERR_IO",       REDIS_ERR_IO },
@@ -219,39 +246,13 @@ static int push_reply(lua_State * L, redisReply * pReply)
   {
     case REDIS_REPLY_STATUS:
       lua_pushvalue(L, lua_upvalueindex(1)); /* M (module table) */
+      lua_getfield(L, -1, "status"); /* status = M.status */
+      lua_remove(L, -2); /* Remove module table from stack */
 
-      lua_pushlstring(L, pReply->str, pReply->len); /* status */
-      lua_gettable(L, -2); /* M[status] */
+      lua_pushlstring(L, pReply->str, pReply->len); /* name */
+      lua_gettable(L, -2); /* status[name] */
 
-      if (lua_isnil(L, -1)) /* Not bothering with metatables */
-      {
-        lua_pop(L, 1); /* Pop nil */
-
-        /*
-        * TODO: Following code is likely to be broken due to early binding
-        * (imagine that RETURN is a command that returns given string
-        * as a status):
-        *
-        *    assert(conn:command("RETURN", "FOO") == hiredis.FOO)
-        *
-        * Here hiredis.FOO would be nil before conn:command() is called.
-        *
-        * Note that this is not relevant to the current Redis implementation
-        * (that is 2.2 and before), since it seems that it wouldn't
-        * return any status code except OK, QUEUED or PONG,
-        * all of which are already covered.
-        */
-        lua_pushlstring(L, pReply->str, pReply->len); /* status */
-        push_new_const(
-            L, pReply->str, pReply->len, REDIS_REPLY_STATUS /* const */
-          );
-        lua_settable(L, -3); /* M[status] = const */
-
-        lua_pushlstring(L, pReply->str, pReply->len); /* status */
-        lua_gettable(L, -2); /* return M[status] */
-      }
-
-      lua_remove(L, -2); /* Remove module table from stack */
+      lua_remove(L, -2); /* Remove status table from stack */
 
       break;
 
@@ -586,14 +587,24 @@ LUALIB_API int luaopen_hiredis(lua_State * L)
   push_new_const(L, "NIL", 3, REDIS_REPLY_NIL);
   lua_setfield(L, -2, LUAHIREDIS_KEY_NIL);
 
-  push_new_const(L, "OK", 2, REDIS_REPLY_STATUS);
-  lua_setfield(L, -2, "OK");
+  lua_newtable(L); /* status */
+
+  if (luaL_newmetatable(L, LUAHIREDIS_STATUS_MT))
+  {
+    luaL_register(L, NULL, STATUS_MT);
+    lua_pushliteral(L, LUAHIREDIS_STATUS_MT);
+    lua_setfield(L, -2, "__metatable");
+  }
+  lua_setmetatable(L, -2);
 
-  push_new_const(L, "QUEUED", 6, REDIS_REPLY_STATUS);
-  lua_setfield(L, -2, "QUEUED");
+  lua_getfield(L, -1, "OK");
+  lua_setfield(L, -3, "OK");     /* hiredis.OK = status.OK */
+  lua_getfield(L, -1, "QUEUED");
+  lua_setfield(L, -3, "QUEUED"); /* hiredis.QUEUED = status.QUEUED */
+  lua_getfield(L, -1, "PONG");
+  lua_setfield(L, -3, "PONG");   /* hiredis.PONG = status.PONG */
 
-  push_new_const(L, "PONG", 4, REDIS_REPLY_STATUS);
-  lua_setfield(L, -2, "PONG");
+  lua_setfield(L, -2, "status"); /* hiredis.status = status */
 
   /*
   * Register functions

test/test.lua

@@ -16,27 +16,36 @@ assert(type(assert(getmetatable(hiredis.NIL))) == "string")
 
 --------------------------------------------------------------------------------
 
-assert(type(hiredis.OK == "table"))
-assert(hiredis.OK.name == "OK")
-assert(hiredis.OK.type == hiredis.REPLY_STATUS)
-assert(tostring(hiredis.OK) == "OK")
-assert(getmetatable(hiredis.OK) == getmetatable(hiredis.NIL))
+assert(type(hiredis.status.OK == "table"))
+assert(hiredis.status.OK.name == "OK")
+assert(hiredis.status.OK.type == hiredis.REPLY_STATUS)
+assert(tostring(hiredis.status.OK) == "OK")
+assert(getmetatable(hiredis.status.OK) == getmetatable(hiredis.NIL))
+
+-- deprecated backwards compatibility
+assert(hiredis.OK == hiredis.status.OK)
 
 --------------------------------------------------------------------------------
 
-assert(type(hiredis.QUEUED== "table"))
-assert(hiredis.QUEUED.name == "QUEUED")
-assert(hiredis.QUEUED.type == hiredis.REPLY_STATUS)
-assert(tostring(hiredis.QUEUED) == "QUEUED")
-assert(getmetatable(hiredis.QUEUED) == getmetatable(hiredis.NIL))
+assert(type(hiredis.status.QUEUED == "table"))
+assert(hiredis.status.QUEUED.name == "QUEUED")
+assert(hiredis.status.QUEUED.type == hiredis.REPLY_STATUS)
+assert(tostring(hiredis.status.QUEUED) == "QUEUED")
+assert(getmetatable(hiredis.status.QUEUED) == getmetatable(hiredis.NIL))
+
+-- deprecated backwards compatibility
+assert(hiredis.QUEUED == hiredis.status.QUEUED)
 
 --------------------------------------------------------------------------------
 
-assert(type(hiredis.PONG== "table"))
-assert(hiredis.PONG.name == "PONG")
-assert(hiredis.PONG.type == hiredis.REPLY_STATUS)
-assert(tostring(hiredis.PONG) == "PONG")
-assert(getmetatable(hiredis.PONG) == getmetatable(hiredis.NIL))
+assert(type(hiredis.status.PONG == "table"))
+assert(hiredis.status.PONG.name == "PONG")
+assert(hiredis.status.PONG.type == hiredis.REPLY_STATUS)
+assert(tostring(hiredis.status.PONG) == "PONG")
+assert(getmetatable(hiredis.status.PONG) == getmetatable(hiredis.NIL))
+
+-- deprecated backwards compatibility
+assert(hiredis.PONG == hiredis.status.PONG)
 
 --------------------------------------------------------------------------------
 
@@ -48,7 +57,7 @@ local conn = assert(hiredis.connect("localhost", 6379))
 
 --------------------------------------------------------------------------------
 
-assert(conn:command("PING") == hiredis.PONG)
+assert(conn:command("PING") == hiredis.status.PONG)
 
 --------------------------------------------------------------------------------
 
@@ -63,6 +72,7 @@ assert(T.type == hiredis.REPLY_STATUS)
 assert(T.name == "string")
 
 assert(hiredis.unwrap_reply(T) == "string")
+assert(T == hiredis.status.string)
 
 --------------------------------------------------------------------------------
 
@@ -77,6 +87,7 @@ assert(T.type == hiredis.REPLY_STATUS)
 assert(T.name == "none")
 
 assert(hiredis.unwrap_reply(T) == "none")
+assert(T == hiredis.status.none)
 
 --------------------------------------------------------------------------------
 
@@ -100,34 +111,38 @@ end
 
 --------------------------------------------------------------------------------
 
-assert(assert(conn:command("MULTI")) == hiredis.OK)
-assert(assert(conn:command("SET", "MYKEY1", "MYVALUE1")) == hiredis.QUEUED)
-assert(assert(conn:command("GET", "MYKEY1")) == hiredis.QUEUED)
+assert(assert(conn:command("MULTI")) == hiredis.status.OK)
+assert(
+    assert(conn:command("SET", "MYKEY1", "MYVALUE1")) == hiredis.status.QUEUED
+  )
+assert(assert(conn:command("GET", "MYKEY1")) == hiredis.status.QUEUED)
 local t = assert(conn:command("EXEC"))
-assert(t[1] == hiredis.OK)
+assert(t[1] == hiredis.status.OK)
 assert(t[2] == "MYVALUE1")
 
 --------------------------------------------------------------------------------
 
 -- Based on actual bug scenario.
-assert(assert(conn:command("MULTI")) == hiredis.OK)
-assert(assert(conn:command("GET", "MYKEY1")) == hiredis.QUEUED)
-assert(assert(conn:command("SET", "MYKEY1", "MYVALUE2")) == hiredis.QUEUED)
+assert(assert(conn:command("MULTI")) == hiredis.status.OK)
+assert(assert(conn:command("GET", "MYKEY1")) == hiredis.status.QUEUED)
+assert(
+    assert(conn:command("SET", "MYKEY1", "MYVALUE2")) == hiredis.status.QUEUED
+  )
 local t = assert(conn:command("EXEC"))
 assert(t[1] == "MYVALUE1")
-assert(t[2] == hiredis.OK)
+assert(t[2] == hiredis.status.OK)
 
 --------------------------------------------------------------------------------
 
 assert(conn:command("MULTI"))
-assert(assert(conn:command("GET", "MYKEY1")) == hiredis.QUEUED)
+assert(assert(conn:command("GET", "MYKEY1")) == hiredis.status.QUEUED)
 
 local err = assert(conn:command("SET"))
 assert(err.type == hiredis.REPLY_ERROR)
 assert(err.name == "ERR wrong number of arguments for 'set' command")
 assert(tostring(err) == "ERR wrong number of arguments for 'set' command")
 
-assert(assert(conn:command("GET", "MYKEY1")) == hiredis.QUEUED)
+assert(assert(conn:command("GET", "MYKEY1")) == hiredis.status.QUEUED)
 local t = assert(conn:command("EXEC"))
 
 for i = 1, #t do
@@ -138,15 +153,15 @@ end
 
 assert(conn:command("MULTI"))
 
-assert(assert(conn:command("SET", "MYKEY2", 1)) == hiredis.QUEUED)
+assert(assert(conn:command("SET", "MYKEY2", 1)) == hiredis.status.QUEUED)
 
 -- Wrong value type
-assert(assert(conn:command("SADD", "MYKEY1", "MYVAL")) == hiredis.QUEUED)
+assert(assert(conn:command("SADD", "MYKEY1", "MYVAL")) == hiredis.status.QUEUED)
 
-assert(assert(conn:command("INCR", "MYKEY2")) == hiredis.QUEUED)
+assert(assert(conn:command("INCR", "MYKEY2")) == hiredis.status.QUEUED)
 local t = assert(conn:command("EXEC"))
 
-assert(t[1] == hiredis.OK)
+assert(t[1] == hiredis.status.OK)
 assert(t[2].type == hiredis.REPLY_ERROR)
 assert(
     t[2].name == "ERR Operation against a key holding the wrong kind of value"
@@ -161,13 +176,13 @@ conn:append_command("SADD", "MYKEY1", "MYVAL")
 conn:append_command("INCR", "MYKEY2")
 conn:append_command("EXEC")
 
-assert(assert(conn:get_reply()) == hiredis.OK) -- MULTI
-assert(assert(conn:get_reply()) == hiredis.QUEUED) -- SET
-assert(assert(conn:get_reply()) == hiredis.QUEUED) -- SADD
-assert(assert(conn:get_reply()) == hiredis.QUEUED) -- INCR
+assert(assert(conn:get_reply()) == hiredis.status.OK) -- MULTI
+assert(assert(conn:get_reply()) == hiredis.status.QUEUED) -- SET
+assert(assert(conn:get_reply()) == hiredis.status.QUEUED) -- SADD
+assert(assert(conn:get_reply()) == hiredis.status.QUEUED) -- INCR
 local t = assert(conn:get_reply()) -- EXEC
 
-assert(t[1] == hiredis.OK)
+assert(t[1] == hiredis.status.OK)
 assert(t[2].type == hiredis.REPLY_ERROR)
 assert(
     t[2].name == "ERR Operation against a key holding the wrong kind of value"
@@ -285,7 +300,7 @@ do
 end
 
 do
-  local r = pack(hiredis.unwrap_reply(hiredis.OK))
+  local r = pack(hiredis.unwrap_reply(hiredis.status.OK))
   assert(r.n == 2)
   assert(r[1] == "OK")
   assert(r[2] == hiredis.REPLY_STATUS)