redis_server_protocol: add RedisReply::SerializeToIOBuf and UT

src/brpc/policy/redis_protocol.cpp

@@ -75,10 +75,11 @@ int Consume(void* meta, bthread::TaskIterator<ExecutionQueueContext*>& iter) {
         RedisReply output;
         conn->OnRedisMessage(ctx->message, &output, &ctx->arena);
         butil::IOBuf sendbuf;
-        sendbuf.append("+OK\r\n");
+        output.SerializeToIOBuf(&sendbuf);
         Socket::WriteOptions wopt;
         wopt.ignore_eovercrowded = true;
-        s->Write(&sendbuf, &wopt);
+        LOG_IF(WARNING, s->Write(&sendbuf, &wopt) != 0)
+            << "Fail to send redis reply";
     }
     return 0;
 }
@@ -114,20 +115,21 @@ ParseResult ParseRedisMessage(butil::IOBuf* source, Socket* socket,
     }
     const Server* server = static_cast<const Server*>(arg);
     if (server) {
-        RedisConnection* conn = server->options().redis_service->NewConnection();
-        if (!conn) {
-            LOG(ERROR) << "Fail to new redis connection from redis service";
-            return MakeParseError(PARSE_ERROR_TRY_OTHERS);
-        }
         ServerContext* ctx = static_cast<ServerContext*>(socket->parsing_context());
         if (ctx == NULL) {
+            RedisConnection* conn = server->options().redis_service->NewConnection();
+            if (!conn) {
+                LOG(ERROR) << "Fail to new redis connection from redis service";
+                return MakeParseError(PARSE_ERROR_TRY_OTHERS);
+            }
             ctx = new ServerContext;
             if (ctx->init(conn) != 0) {
+                delete conn;
                 delete ctx;
                 LOG(ERROR) << "Fail to init redis ServerContext";
                 return MakeParseError(PARSE_ERROR_NO_RESOURCE);
             }
-            socket->initialize_parsing_context(&ctx);
+            socket->reset_parsing_context(&ctx);
         }
         std::unique_ptr<ExecutionQueueContext> task(new ExecutionQueueContext);
         RedisReply message;

src/brpc/redis_reply.cpp

@@ -24,6 +24,7 @@
 namespace brpc {
 
 //BAIDU_CASSERT(sizeof(RedisReply) == 24, size_match);
+const uint32_t RedisReply::npos = (uint32_t)-1;
 
 const char* RedisReplyTypeToString(RedisReplyType type) {
     switch (type) {
@@ -38,8 +39,56 @@ const char* RedisReplyTypeToString(RedisReplyType type) {
 }
 
 bool RedisReply::SerializeToIOBuf(butil::IOBuf* buf) {
-    //TODO
-
+    butil::IOBufBuilder builder;
+    switch (_type) {
+        case REDIS_REPLY_ERROR:
+        case REDIS_REPLY_STATUS:
+            buf->push_back((_type == REDIS_REPLY_ERROR)? '-' : '+');
+            if (_length < sizeof(_data.short_str)) {
+                buf->append(_data.short_str, _length);
+            } else {
+                buf->append(_data.long_str, _length);
+            }
+            buf->append("\r\n");
+            break;
+        case REDIS_REPLY_INTEGER:
+            builder << ':' << _data.integer << "\r\n";
+            buf->append(builder.buf());
+            break;
+        case REDIS_REPLY_STRING:
+            // Since _length is unsigned, we have to int casting _length to
+            // represent nil string
+            builder << '$' << (int)_length << "\r\n";
+            buf->append(builder.buf());
+            if (_length == npos) {
+                break;
+            }
+            if (_length < sizeof(_data.short_str)) {
+                buf->append(_data.short_str, _length);
+            } else {
+                buf->append(_data.long_str, _length);
+            }
+            buf->append("\r\n");
+            break;
+        case REDIS_REPLY_ARRAY:
+            builder << '*' << (int)_length << "\r\n";
+            buf->append(builder.buf());
+            if (_length == npos) {
+                break;
+            }
+            for (size_t i = 0; i < _length; ++i) {
+                if (!_data.array.replies[i].SerializeToIOBuf(buf)) {
+                    return false;
+                }
+            }
+            break;
+        case REDIS_REPLY_NIL:
+            buf->append("$-1\r\n");
+            break;
+        default:
+            CHECK(false) << "unknown redis type=" << _type;
+            return false;
+    }
     return true;
 }
 

src/brpc/redis_reply.h

@@ -58,7 +58,7 @@ public:
 
     bool set_nil_string();  // "$-1\r\n"
     bool set_array(int size, butil::Arena* arena);  // size == -1 means nil array("*-1\r\n")
-    bool set_simple_string(const std::string& str, butil::Arena* arena);
+    bool set_status(const std::string& str, butil::Arena* arena);
     bool set_error(const std::string& str, butil::Arena* arena);
     bool set_integer(int64_t value);
     bool set_bulk_string(const std::string& str, butil::Arena* arena);
@@ -87,6 +87,7 @@ public:
     // Get the index-th sub reply. If this reply is not an array, a nil reply
     // is returned (call stacks are not logged)
     const RedisReply& operator[](size_t index) const;
+    RedisReply& operator[](size_t index);
 
     // Parse from `buf' which may be incomplete and allocate needed memory
     // on `arena'.
@@ -100,6 +101,7 @@ public:
     // the complexity in worst case may be O(N^2).
     // Returns PARSE_ERROR_ABSOLUTELY_WRONG if the parsing failed.
     ParseError ConsumePartialIOBuf(butil::IOBuf& buf, butil::Arena* arena);
+
     // 
     bool SerializeToIOBuf(butil::IOBuf* buf);
 
@@ -121,6 +123,8 @@ public:
     void CopyFromSameArena(const RedisReply& other);
 
 private:
+    static const uint32_t npos;
+
     // RedisReply does not own the memory of fields, copying must be done
     // by calling CopyFrom[Different|Same]Arena.
     DISALLOW_COPY_AND_ASSIGN(RedisReply);
@@ -155,7 +159,11 @@ inline RedisReply::RedisReply()
     _data.array.replies = NULL;
 }
 
-inline bool RedisReply::is_nil() const { return _type == REDIS_REPLY_NIL; }
+inline bool RedisReply::is_nil() const {
+    return (_type == REDIS_REPLY_NIL) ||
+        ((_type == REDIS_REPLY_STRING || _type == REDIS_REPLY_ARRAY) &&
+         _length == uint32_t(-1));
+}
 inline bool RedisReply::is_error() const { return _type == REDIS_REPLY_ERROR; }
 inline bool RedisReply::is_integer() const { return _type == REDIS_REPLY_INTEGER; }
 inline bool RedisReply::is_string() const
@@ -173,14 +181,14 @@ inline int64_t RedisReply::integer() const {
 
 inline bool RedisReply::set_nil_string() {
     _type = REDIS_REPLY_STRING;
-    _length = -1;
+    _length = npos;
     return true;
 }
 
 inline bool RedisReply::set_array(int size, butil::Arena* arena) {
     _type = REDIS_REPLY_ARRAY;
     if (size < 0) {
-        _length = -1;
+        _length = npos;
         return true;
     } else if (size == 0) {
         _length = 0;
@@ -217,7 +225,7 @@ inline bool RedisReply::set_basic_string(const std::string& str, butil::Arena* a
     return true;
 }
 
-inline bool RedisReply::set_simple_string(const std::string& str, butil::Arena* arena) {
+inline bool RedisReply::set_status(const std::string& str, butil::Arena* arena) {
     return set_basic_string(str, arena, REDIS_REPLY_STATUS);
 }
 
@@ -279,6 +287,11 @@ inline size_t RedisReply::size() const {
     return (is_array() ? _length : 0);
 }
 
+inline RedisReply& RedisReply::operator[](size_t index) {
+    return const_cast<RedisReply&>(
+            const_cast<const RedisReply*>(this)->operator[](index));
+}
+
 inline const RedisReply& RedisReply::operator[](size_t index) const {
     if (is_array() && index < _length) {
         return _data.array.replies[index];