From 93eaa01b4b9e9c6add23bc77e6aa521f8ee1515f Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 15 Aug 2011 18:43:14 +0100 Subject: [PATCH 1/9] Check for msgpack support --- Makefile | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 90ca0cf..0e04b7f 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,20 @@ INSTALL_DIRS = $(DESTDIR) \ $(DESTDIR)/$(PREFIX)/bin \ $(CONFDIR) -all: $(OUT) Makefile +all: .CHECK_FOR_MSGPACK + +.CHECK_FOR_MSGPACK: + @if [ -f /usr/lib/libmsgpack.so ]; then make .WITH_MSGPACK; else make .WITHOUT_MSGPACK ; fi + +.WITH_MSGPACK: + @echo "Building with MsgPack support" + @make CFLAGS="$(CFLAGS) -DMSGPACK=1" .REAL_BUILD + +.WITHOUT_MSGPACK: + @echo "Building without MsgPack support" + @make .REAL_BUILD + +.REAL_BUILD: $(OUT) Makefile $(OUT): $(OBJS) Makefile $(CC) $(LDFLAGS) -o $(OUT) $(OBJS) From 525c63d54a2e32b251a3bf5ea1327fa7e470d213 Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 15 Aug 2011 18:50:56 +0100 Subject: [PATCH 2/9] Added more msgpack support --- Makefile | 2 +- cmd.c | 7 ++ formats/msgpack.c | 294 ++++++++++++++++++++++++++++++++++++++++++++++ formats/msgpack.h | 20 ++++ 4 files changed, 322 insertions(+), 1 deletion(-) create mode 100644 formats/msgpack.c create mode 100644 formats/msgpack.h diff --git a/Makefile b/Makefile index 0e04b7f..6f31c30 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ all: .CHECK_FOR_MSGPACK .WITH_MSGPACK: @echo "Building with MsgPack support" - @make CFLAGS="$(CFLAGS) -DMSGPACK=1" .REAL_BUILD + @make CFLAGS="$(CFLAGS) -DMSGPACK=1" FORMAT_OBJS="$(FORMAT_OBJS) formats/msgpack.o" .REAL_BUILD .WITHOUT_MSGPACK: @echo "Building without MsgPack support" diff --git a/cmd.c b/cmd.c index 0d401d8..1679e9f 100644 --- a/cmd.c +++ b/cmd.c @@ -10,6 +10,9 @@ #include "formats/json.h" #include "formats/bson.h" #include "formats/raw.h" +#ifdef MSGPACK +#include "formats/msgpack.h" +#endif #include "formats/custom-type.h" #include @@ -265,6 +268,10 @@ cmd_select_format(struct http_client *client, struct cmd *cmd, {.s = "raw", .sz = 3, .f = raw_reply, .ct = "binary/octet-stream"}, {.s = "bson", .sz = 4, .f = bson_reply, .ct = "application/bson"}, +#ifdef MSGPACK + {.s = "msg", .sz = 3, .f = msgpack_reply, .ct = "application/x-msgpack"}, +#endif + {.s = "bin", .sz = 3, .f = custom_type_reply, .ct = "binary/octet-stream"}, {.s = "txt", .sz = 3, .f = custom_type_reply, .ct = "text/plain"}, {.s = "html", .sz = 4, .f = custom_type_reply, .ct = "text/html"}, diff --git a/formats/msgpack.c b/formats/msgpack.c new file mode 100644 index 0000000..1488fa1 --- /dev/null +++ b/formats/msgpack.c @@ -0,0 +1,294 @@ +#include "msgpack.h" +#include "common.h" +#include "cmd.h" +#include "http.h" +#include "client.h" + +#include +#include +#include +#include /* TODO: remove once msgpack is fully integrated. */ + +static json_t * +json_wrap_redis_reply(const struct cmd *cmd, const redisReply *r); + +void +msgpack_reply(redisAsyncContext *c, void *r, void *privdata) { + + redisReply *reply = r; + struct cmd *cmd = privdata; + json_t *j; + char *jstr; + (void)c; + + if(cmd == NULL) { + /* broken connection */ + return; + } + + if (reply == NULL) { /* broken Redis link */ + format_send_error(cmd, 503, "Service Unavailable"); + return; + } + + /* encode redis reply as JSON */ + j = json_wrap_redis_reply(cmd, r); + + /* get JSON as string, possibly with JSONP wrapper */ + jstr = msgpack_string_output(j, cmd->jsonp); + + /* send reply */ + format_send_reply(cmd, jstr, strlen(jstr), "application/json"); + + /* cleanup */ + json_decref(j); + free(jstr); +} + +/** + * Parse info message and return object. + */ +static json_t * +json_info_reply(const char *s) { + const char *p = s; + size_t sz = strlen(s); + + json_t *jroot = json_object(); + + /* TODO: handle new format */ + + while(p < s + sz) { + char *key, *val, *nl, *colon; + + /* find key */ + colon = strchr(p, ':'); + if(!colon) { + break; + } + key = calloc(colon - p + 1, 1); + memcpy(key, p, colon - p); + p = colon + 1; + + /* find value */ + nl = strchr(p, '\r'); + if(!nl) { + free(key); + break; + } + val = calloc(nl - p + 1, 1); + memcpy(val, p, nl - p); + p = nl + 1; + if(*p == '\n') p++; + + /* add to object */ + json_object_set_new(jroot, key, json_string(val)); + free(key); + free(val); + } + + return jroot; +} + +static json_t * +json_hgetall_reply(const redisReply *r) { + /* zip keys and values together in a json object */ + json_t *jroot; + unsigned int i; + + if(r->elements % 2 != 0) { + return NULL; + } + + jroot = json_object(); + for(i = 0; i < r->elements; i += 2) { + redisReply *k = r->element[i], *v = r->element[i+1]; + + /* keys and values need to be strings */ + if(k->type != REDIS_REPLY_STRING || v->type != REDIS_REPLY_STRING) { + json_decref(jroot); + return NULL; + } + json_object_set_new(jroot, k->str, json_string(v->str)); + } + return jroot; +} + +static json_t * +json_wrap_redis_reply(const struct cmd *cmd, const redisReply *r) { + + unsigned int i; + json_t *jlist, *jroot = json_object(); /* that's what we return */ + + + /* copy verb, as jansson only takes a char* but not its length. */ + char *verb; + if(cmd->count) { + verb = calloc(cmd->argv_len[0]+1, 1); + memcpy(verb, cmd->argv[0], cmd->argv_len[0]); + } else { + verb = strdup(""); + } + + + switch(r->type) { + case REDIS_REPLY_STATUS: + case REDIS_REPLY_ERROR: + jlist = json_array(); + json_array_append_new(jlist, + r->type == REDIS_REPLY_ERROR ? json_false() : json_true()); + json_array_append_new(jlist, json_string(r->str)); + json_object_set_new(jroot, verb, jlist); + break; + + case REDIS_REPLY_STRING: + if(strcasecmp(verb, "INFO") == 0) { + json_object_set_new(jroot, verb, json_info_reply(r->str)); + } else { + json_object_set_new(jroot, verb, json_string(r->str)); + } + break; + + case REDIS_REPLY_INTEGER: + json_object_set_new(jroot, verb, json_integer(r->integer)); + break; + + case REDIS_REPLY_ARRAY: + if(strcasecmp(verb, "HGETALL") == 0) { + json_t *jobj = json_hgetall_reply(r); + if(jobj) { + json_object_set_new(jroot, verb, jobj); + break; + } + } + jlist = json_array(); + for(i = 0; i < r->elements; ++i) { + redisReply *e = r->element[i]; + switch(e->type) { + case REDIS_REPLY_STRING: + json_array_append_new(jlist, json_string(e->str)); + break; + case REDIS_REPLY_INTEGER: + json_array_append_new(jlist, json_integer(e->integer)); + break; + default: + json_array_append_new(jlist, json_null()); + break; + } + } + json_object_set_new(jroot, verb, jlist); + break; + + default: + json_object_set_new(jroot, verb, json_null()); + break; + } + + free(verb); + return jroot; +} + + +char * +msgpack_string_output(json_t *j, const char *jsonp) { + + char *json_reply = json_dumps(j, JSON_COMPACT); + + /* check for JSONP */ + if(jsonp) { + size_t jsonp_len = strlen(jsonp); + size_t json_len = strlen(json_reply); + size_t ret_len = jsonp_len + 1 + json_len + 3; + char *ret = calloc(1 + ret_len, 1); + + memcpy(ret, jsonp, jsonp_len); + ret[jsonp_len]='('; + memcpy(ret + jsonp_len + 1, json_reply, json_len); + memcpy(ret + jsonp_len + 1 + json_len, ");\n", 3); + free(json_reply); + + return ret; + } + + return json_reply; +} + +/* extract JSON from WebSocket frame and fill struct cmd. */ +struct cmd * +msgpack_ws_extract(struct http_client *c, const char *p, size_t sz) { + + struct cmd *cmd = NULL; + json_t *j; + char *jsonz; /* null-terminated */ + + unsigned int i, cur; + int argc = 0; + json_error_t jerror; + + (void)c; + + jsonz = calloc(sz + 1, 1); + memcpy(jsonz, p, sz); + j = json_loads(jsonz, sz, &jerror); + free(jsonz); + + if(!j) { + return NULL; + } + if(json_typeof(j) != JSON_ARRAY) { + json_decref(j); + return NULL; /* invalid JSON */ + } + + /* count elements */ + for(i = 0; i < json_array_size(j); ++i) { + json_t *jelem = json_array_get(j, i); + + switch(json_typeof(jelem)) { + case JSON_STRING: + case JSON_INTEGER: + argc++; + break; + + default: + break; + } + } + + if(!argc) { /* not a single item could be decoded */ + json_decref(j); + return NULL; + } + + /* create command and add args */ + cmd = cmd_new(argc); + for(i = 0, cur = 0; i < json_array_size(j); ++i) { + json_t *jelem = json_array_get(j, i); + char *tmp; + + switch(json_typeof(jelem)) { + case JSON_STRING: + tmp = strdup(json_string_value(jelem)); + + cmd->argv[cur] = tmp; + cmd->argv_len[cur] = strlen(tmp); + cur++; + break; + + case JSON_INTEGER: + tmp = malloc(40); + sprintf(tmp, "%d", (int)json_integer_value(jelem)); + + cmd->argv[cur] = tmp; + cmd->argv_len[cur] = strlen(tmp); + cur++; + break; + + default: + break; + } + } + + json_decref(j); + return cmd; +} + diff --git a/formats/msgpack.h b/formats/msgpack.h new file mode 100644 index 0000000..6671d31 --- /dev/null +++ b/formats/msgpack.h @@ -0,0 +1,20 @@ +#ifndef MSGPACK_H +#define MSGPACK_H + +#include +#include +#include + +struct cmd; +struct http_client; + +void +msgpack_reply(redisAsyncContext *c, void *r, void *privdata); + +char * +msgpack_string_output(json_t *j, const char *jsonp); + +struct cmd * +msgpack_ws_extract(struct http_client *c, const char *p, size_t sz); + +#endif From 78932bffd47d1173e93eb949a06f1afe79d487c1 Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 15 Aug 2011 22:07:31 +0100 Subject: [PATCH 3/9] Working MessagePack implementation, if available. --- README.markdown | 2 + formats/msgpack.c | 266 ++++++++++++++++++---------------------------- formats/msgpack.h | 7 +- 3 files changed, 104 insertions(+), 171 deletions(-) diff --git a/README.markdown b/README.markdown index 3ed4081..38b6def 100644 --- a/README.markdown +++ b/README.markdown @@ -24,6 +24,7 @@ curl -d "GET/hello" http://127.0.0.1:7379/ * JSON output by default, optional JSONP parameter (`?jsonp=myFunction` or `?callback=myFunction`). * Raw Redis 2.0 protocol output with `.raw` suffix * BSON support for compact responses and MongoDB compatibility. +* MessagePack output with `.msg` suffix * HTTP 1.1 pipelining (70,000 http requests per second on a desktop Linux machine.) * Multi-threaded server, configurable number of worker threads. * WebSocket support (Currently using the “hixie-76” specification). @@ -173,6 +174,7 @@ Several content-types are available: * `.json` for `application/json` (this is the default Content-Type). * `.bson` for `application/bson`. See [http://bsonspec.org/](http://bsonspec.org/) for the specs. +* `.msg` for `application/x-msgpack`. See [http://msgpack.org/](http://msgpack.org/) for the specs. * `.txt` for `text/plain` * `.html` for `text/html` * `xhtml` for `application/xhtml+xml` diff --git a/formats/msgpack.c b/formats/msgpack.c index 1488fa1..08b79ef 100644 --- a/formats/msgpack.c +++ b/formats/msgpack.c @@ -7,18 +7,21 @@ #include #include #include -#include /* TODO: remove once msgpack is fully integrated. */ -static json_t * -json_wrap_redis_reply(const struct cmd *cmd, const redisReply *r); +struct msg_out { + char *p; + size_t sz; +}; + +static void +msgpack_wrap_redis_reply(const struct cmd *cmd, struct msg_out *, const redisReply *r); void msgpack_reply(redisAsyncContext *c, void *r, void *privdata) { redisReply *reply = r; struct cmd *cmd = privdata; - json_t *j; - char *jstr; + struct msg_out out; (void)c; if(cmd == NULL) { @@ -31,42 +34,67 @@ msgpack_reply(redisAsyncContext *c, void *r, void *privdata) { return; } - /* encode redis reply as JSON */ - j = json_wrap_redis_reply(cmd, r); + /* prepare data structure for output */ + out.p = NULL; + out.sz = 0; - /* get JSON as string, possibly with JSONP wrapper */ - jstr = msgpack_string_output(j, cmd->jsonp); + /* encode redis reply */ + msgpack_wrap_redis_reply(cmd, &out, r); /* send reply */ - format_send_reply(cmd, jstr, strlen(jstr), "application/json"); + format_send_reply(cmd, out.p, out.sz, "application/x-msgpack"); /* cleanup */ - json_decref(j); - free(jstr); + free(out.p); +} + +static int +on_msgpack_write(void *data, const char *s, unsigned int sz) { + + struct msg_out *out = data; + + out->p = realloc(out->p, out->sz + sz); + memcpy(out->p + out->sz, s, sz); + out->sz += sz; + + return sz; } /** * Parse info message and return object. */ -static json_t * -json_info_reply(const char *s) { +void +msg_info_reply(msgpack_packer* pk, const char *s, size_t sz) { const char *p = s; - size_t sz = strlen(s); - - json_t *jroot = json_object(); + unsigned int count = 0; /* TODO: handle new format */ + /* count number of lines */ + while(p < s + sz) { + p = strchr(p, '\r'); + if(!p) break; + + p++; + count++; + } + + /* create msgpack object */ + msgpack_pack_map(pk, count); + + p = s; while(p < s + sz) { char *key, *val, *nl, *colon; + size_t key_sz, val_sz; /* find key */ colon = strchr(p, ':'); if(!colon) { break; } - key = calloc(colon - p + 1, 1); - memcpy(key, p, colon - p); + key_sz = colon - p; + key = calloc(key_sz + 1, 1); + memcpy(key, p, key_sz); p = colon + 1; /* find value */ @@ -75,220 +103,128 @@ json_info_reply(const char *s) { free(key); break; } - val = calloc(nl - p + 1, 1); - memcpy(val, p, nl - p); + val_sz = nl - p; + val = calloc(val_sz + 1, 1); + memcpy(val, p, val_sz); p = nl + 1; if(*p == '\n') p++; /* add to object */ - json_object_set_new(jroot, key, json_string(val)); + msgpack_pack_raw(pk, key_sz); + msgpack_pack_raw_body(pk, key, key_sz); + msgpack_pack_raw(pk, val_sz); + msgpack_pack_raw_body(pk, val, val_sz); + free(key); free(val); } - - return jroot; } -static json_t * -json_hgetall_reply(const redisReply *r) { - /* zip keys and values together in a json object */ - json_t *jroot; +static void +msg_hgetall_reply(msgpack_packer* pk, const redisReply *r) { + /* zip keys and values together in a msgpack object */ unsigned int i; if(r->elements % 2 != 0) { - return NULL; + return; } - jroot = json_object(); + msgpack_pack_map(pk, r->elements / 2); for(i = 0; i < r->elements; i += 2) { redisReply *k = r->element[i], *v = r->element[i+1]; /* keys and values need to be strings */ if(k->type != REDIS_REPLY_STRING || v->type != REDIS_REPLY_STRING) { - json_decref(jroot); - return NULL; + return; } - json_object_set_new(jroot, k->str, json_string(v->str)); + + /* key */ + msgpack_pack_raw(pk, k->len); + msgpack_pack_raw_body(pk, k->str, k->len); + + /* value */ + msgpack_pack_raw(pk, v->len); + msgpack_pack_raw_body(pk, v->str, v->len); } - return jroot; } -static json_t * -json_wrap_redis_reply(const struct cmd *cmd, const redisReply *r) { +static void +msgpack_wrap_redis_reply(const struct cmd *cmd, struct msg_out *out, const redisReply *r) { unsigned int i; - json_t *jlist, *jroot = json_object(); /* that's what we return */ - + msgpack_packer* pk = msgpack_packer_new(out, on_msgpack_write); /* copy verb, as jansson only takes a char* but not its length. */ char *verb; + size_t verb_sz = 0; if(cmd->count) { verb = calloc(cmd->argv_len[0]+1, 1); - memcpy(verb, cmd->argv[0], cmd->argv_len[0]); + verb_sz = cmd->argv_len[0]; + memcpy(verb, cmd->argv[0], verb_sz); } else { verb = strdup(""); } + msgpack_pack_map(pk, 1); + + msgpack_pack_raw(pk, verb_sz); + msgpack_pack_raw_body(pk, verb, verb_sz); switch(r->type) { case REDIS_REPLY_STATUS: case REDIS_REPLY_ERROR: - jlist = json_array(); - json_array_append_new(jlist, - r->type == REDIS_REPLY_ERROR ? json_false() : json_true()); - json_array_append_new(jlist, json_string(r->str)); - json_object_set_new(jroot, verb, jlist); + msgpack_pack_array(pk, 2); + + if(r->type == REDIS_REPLY_ERROR) + msgpack_pack_false(pk); + else + msgpack_pack_true(pk); break; case REDIS_REPLY_STRING: if(strcasecmp(verb, "INFO") == 0) { - json_object_set_new(jroot, verb, json_info_reply(r->str)); + msg_info_reply(pk, r->str, r->len); } else { - json_object_set_new(jroot, verb, json_string(r->str)); + msgpack_pack_raw(pk, r->len); + msgpack_pack_raw_body(pk, r->str, r->len); } break; case REDIS_REPLY_INTEGER: - json_object_set_new(jroot, verb, json_integer(r->integer)); + msgpack_pack_int64(pk, r->integer); break; case REDIS_REPLY_ARRAY: if(strcasecmp(verb, "HGETALL") == 0) { - json_t *jobj = json_hgetall_reply(r); - if(jobj) { - json_object_set_new(jroot, verb, jobj); - break; - } + msg_hgetall_reply(pk, r); + break; } - jlist = json_array(); + + msgpack_pack_array(pk, r->elements); + for(i = 0; i < r->elements; ++i) { redisReply *e = r->element[i]; switch(e->type) { case REDIS_REPLY_STRING: - json_array_append_new(jlist, json_string(e->str)); + msgpack_pack_raw(pk, e->len); + msgpack_pack_raw_body(pk, e->str, e->len); break; case REDIS_REPLY_INTEGER: - json_array_append_new(jlist, json_integer(e->integer)); + msgpack_pack_int64(pk, e->integer); break; default: - json_array_append_new(jlist, json_null()); + msgpack_pack_nil(pk); break; } } - json_object_set_new(jroot, verb, jlist); + break; default: - json_object_set_new(jroot, verb, json_null()); + msgpack_pack_nil(pk); break; } free(verb); - return jroot; + msgpack_packer_free(pk); } - - -char * -msgpack_string_output(json_t *j, const char *jsonp) { - - char *json_reply = json_dumps(j, JSON_COMPACT); - - /* check for JSONP */ - if(jsonp) { - size_t jsonp_len = strlen(jsonp); - size_t json_len = strlen(json_reply); - size_t ret_len = jsonp_len + 1 + json_len + 3; - char *ret = calloc(1 + ret_len, 1); - - memcpy(ret, jsonp, jsonp_len); - ret[jsonp_len]='('; - memcpy(ret + jsonp_len + 1, json_reply, json_len); - memcpy(ret + jsonp_len + 1 + json_len, ");\n", 3); - free(json_reply); - - return ret; - } - - return json_reply; -} - -/* extract JSON from WebSocket frame and fill struct cmd. */ -struct cmd * -msgpack_ws_extract(struct http_client *c, const char *p, size_t sz) { - - struct cmd *cmd = NULL; - json_t *j; - char *jsonz; /* null-terminated */ - - unsigned int i, cur; - int argc = 0; - json_error_t jerror; - - (void)c; - - jsonz = calloc(sz + 1, 1); - memcpy(jsonz, p, sz); - j = json_loads(jsonz, sz, &jerror); - free(jsonz); - - if(!j) { - return NULL; - } - if(json_typeof(j) != JSON_ARRAY) { - json_decref(j); - return NULL; /* invalid JSON */ - } - - /* count elements */ - for(i = 0; i < json_array_size(j); ++i) { - json_t *jelem = json_array_get(j, i); - - switch(json_typeof(jelem)) { - case JSON_STRING: - case JSON_INTEGER: - argc++; - break; - - default: - break; - } - } - - if(!argc) { /* not a single item could be decoded */ - json_decref(j); - return NULL; - } - - /* create command and add args */ - cmd = cmd_new(argc); - for(i = 0, cur = 0; i < json_array_size(j); ++i) { - json_t *jelem = json_array_get(j, i); - char *tmp; - - switch(json_typeof(jelem)) { - case JSON_STRING: - tmp = strdup(json_string_value(jelem)); - - cmd->argv[cur] = tmp; - cmd->argv_len[cur] = strlen(tmp); - cur++; - break; - - case JSON_INTEGER: - tmp = malloc(40); - sprintf(tmp, "%d", (int)json_integer_value(jelem)); - - cmd->argv[cur] = tmp; - cmd->argv_len[cur] = strlen(tmp); - cur++; - break; - - default: - break; - } - } - - json_decref(j); - return cmd; -} - diff --git a/formats/msgpack.h b/formats/msgpack.h index 6671d31..54f7eca 100644 --- a/formats/msgpack.h +++ b/formats/msgpack.h @@ -2,6 +2,7 @@ #define MSGPACK_H #include +#include #include #include @@ -11,10 +12,4 @@ struct http_client; void msgpack_reply(redisAsyncContext *c, void *r, void *privdata); -char * -msgpack_string_output(json_t *j, const char *jsonp); - -struct cmd * -msgpack_ws_extract(struct http_client *c, const char *p, size_t sz); - #endif From e676f59f66fdf7386a6284489f7481e12b7c4264 Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 15 Aug 2011 22:22:44 +0100 Subject: [PATCH 4/9] MessagePack bugfixes --- formats/msgpack.c | 22 +++++++++++----------- formats/msgpack.h | 4 ---- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/formats/msgpack.c b/formats/msgpack.c index 08b79ef..f63cf51 100644 --- a/formats/msgpack.c +++ b/formats/msgpack.c @@ -65,6 +65,7 @@ on_msgpack_write(void *data, const char *s, unsigned int sz) { */ void msg_info_reply(msgpack_packer* pk, const char *s, size_t sz) { + const char *p = s; unsigned int count = 0; @@ -122,7 +123,9 @@ msg_info_reply(msgpack_packer* pk, const char *s, size_t sz) { static void msg_hgetall_reply(msgpack_packer* pk, const redisReply *r) { + /* zip keys and values together in a msgpack object */ + unsigned int i; if(r->elements % 2 != 0) { @@ -155,25 +158,23 @@ msgpack_wrap_redis_reply(const struct cmd *cmd, struct msg_out *out, const redis msgpack_packer* pk = msgpack_packer_new(out, on_msgpack_write); /* copy verb, as jansson only takes a char* but not its length. */ - char *verb; + char *verb = ""; size_t verb_sz = 0; if(cmd->count) { - verb = calloc(cmd->argv_len[0]+1, 1); verb_sz = cmd->argv_len[0]; - memcpy(verb, cmd->argv[0], verb_sz); - } else { - verb = strdup(""); + verb = cmd->argv[0]; } + /* Create map object */ msgpack_pack_map(pk, 1); + /* The single element is the verb */ msgpack_pack_raw(pk, verb_sz); msgpack_pack_raw_body(pk, verb, verb_sz); switch(r->type) { case REDIS_REPLY_STATUS: case REDIS_REPLY_ERROR: - msgpack_pack_array(pk, 2); if(r->type == REDIS_REPLY_ERROR) msgpack_pack_false(pk); @@ -182,7 +183,7 @@ msgpack_wrap_redis_reply(const struct cmd *cmd, struct msg_out *out, const redis break; case REDIS_REPLY_STRING: - if(strcasecmp(verb, "INFO") == 0) { + if(verb_sz ==4 && strncasecmp(verb, "INFO", 4) == 0) { msg_info_reply(pk, r->str, r->len); } else { msgpack_pack_raw(pk, r->len); @@ -191,11 +192,11 @@ msgpack_wrap_redis_reply(const struct cmd *cmd, struct msg_out *out, const redis break; case REDIS_REPLY_INTEGER: - msgpack_pack_int64(pk, r->integer); + msgpack_pack_int(pk, r->integer); break; case REDIS_REPLY_ARRAY: - if(strcasecmp(verb, "HGETALL") == 0) { + if(verb_sz == 7 && strncasecmp(verb, "HGETALL", 7) == 0) { msg_hgetall_reply(pk, r); break; } @@ -210,7 +211,7 @@ msgpack_wrap_redis_reply(const struct cmd *cmd, struct msg_out *out, const redis msgpack_pack_raw_body(pk, e->str, e->len); break; case REDIS_REPLY_INTEGER: - msgpack_pack_int64(pk, e->integer); + msgpack_pack_int(pk, e->integer); break; default: msgpack_pack_nil(pk); @@ -225,6 +226,5 @@ msgpack_wrap_redis_reply(const struct cmd *cmd, struct msg_out *out, const redis break; } - free(verb); msgpack_packer_free(pk); } diff --git a/formats/msgpack.h b/formats/msgpack.h index 54f7eca..796caec 100644 --- a/formats/msgpack.h +++ b/formats/msgpack.h @@ -1,14 +1,10 @@ #ifndef MSGPACK_H #define MSGPACK_H -#include #include #include #include -struct cmd; -struct http_client; - void msgpack_reply(redisAsyncContext *c, void *r, void *privdata); From 1cd57619fb8e921ae0911d5a349319ebbc1ee6a9 Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 15 Aug 2011 22:32:59 +0100 Subject: [PATCH 5/9] Added MessagePack tests and fixed status reply --- formats/msgpack.c | 6 +++++ tests/basic.py | 58 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/formats/msgpack.c b/formats/msgpack.c index f63cf51..dc1ab0c 100644 --- a/formats/msgpack.c +++ b/formats/msgpack.c @@ -175,11 +175,17 @@ msgpack_wrap_redis_reply(const struct cmd *cmd, struct msg_out *out, const redis switch(r->type) { case REDIS_REPLY_STATUS: case REDIS_REPLY_ERROR: + msgpack_pack_array(pk, 2); + /* first element: book */ if(r->type == REDIS_REPLY_ERROR) msgpack_pack_false(pk); else msgpack_pack_true(pk); + + /* second element: message */ + msgpack_pack_raw(pk, r->len); + msgpack_pack_raw_body(pk, r->str, r->len); break; case REDIS_REPLY_STRING: diff --git a/tests/basic.py b/tests/basic.py index c85f5a2..0cf46d1 100755 --- a/tests/basic.py +++ b/tests/basic.py @@ -5,6 +5,10 @@ try: import bson except: bson = None +try: + import msgpack +except: + msgpack = None host = '127.0.0.1' @@ -177,6 +181,60 @@ class TestBSon(TestWebdis): self.assertTrue(obj[0]['UNKNOWN'][0] == False) self.assertTrue(isinstance(obj[0]['UNKNOWN'][1], bson.Binary)) +def need_msgpack(fn): + def wrapper(self): + if msgpack: + fn(self) + return wrapper + +class TestMsgPack(TestWebdis): + + @need_msgpack + def test_set(self): + "success type (+OK)" + self.query('DEL/hello') + f = self.query('SET/hello/world.msg') + self.assertTrue(f.headers.getheader('Content-Type') == 'application/x-msgpack') + obj = msgpack.loads(f.read()) + self.assertTrue(obj == {'SET': (True, 'OK')}) + + @need_msgpack + def test_get(self): + "string type" + self.query('SET/hello/world') + f = self.query('GET/hello.msg') + obj = msgpack.loads(f.read()) + self.assertTrue(obj == {'GET': 'world'}) + + @need_msgpack + def test_incr(self): + "integer type" + self.query('DEL/hello') + f = self.query('INCR/hello.msg') + obj = msgpack.loads(f.read()) + self.assertTrue(obj == {'INCR': 1}) + + @need_msgpack + def test_list(self): + "list type" + self.query('DEL/hello') + self.query('RPUSH/hello/abc') + self.query('RPUSH/hello/def') + f = self.query('LRANGE/hello/0/-1.msg') + obj = msgpack.loads(f.read()) + self.assertTrue(obj == {'LRANGE': ('abc', 'def')}) + + @need_msgpack + def test_error(self): + "error return type" + f = self.query('UNKNOWN/COMMAND.msg') + obj = msgpack.loads(f.read()) + self.assertTrue('UNKNOWN' in obj) + self.assertTrue(isinstance(obj, dict)) + self.assertTrue(isinstance(obj['UNKNOWN'], tuple)) + self.assertTrue(obj['UNKNOWN'][0] == False) + self.assertTrue(isinstance(obj['UNKNOWN'][1], str)) + class TestETag(TestWebdis): def test_etag_match(self): From cc194b11233a058fcd87ef560b8e27c54b2a55fd Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 15 Aug 2011 22:36:02 +0100 Subject: [PATCH 6/9] Fixed test for HTTP-400 errors --- tests/basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/basic.py b/tests/basic.py index 0cf46d1..1d55d2d 100755 --- a/tests/basic.py +++ b/tests/basic.py @@ -259,7 +259,7 @@ class TestBadRequest(TestWebdis): self.query('DEL/hello') self.query('LPUSH/hello/world') # "hello" is a list. try: - f = self.query('LRANGE/hello/world.txt') # let's try a range query on it (valid) but as text (invalid) + f = self.query('LRANGE/hello/0/-1.txt') # let's try a range query on it (valid) but as text (invalid) except urllib2.HTTPError as e: self.assertTrue(e.code == 400) return From c2987f07cd4c0717fdb18a4396f3f09cacc17be9 Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 15 Aug 2011 22:36:02 +0100 Subject: [PATCH 7/9] Fixed test for HTTP-400 errors --- tests/basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/basic.py b/tests/basic.py index c85f5a2..f46f4fe 100755 --- a/tests/basic.py +++ b/tests/basic.py @@ -201,7 +201,7 @@ class TestBadRequest(TestWebdis): self.query('DEL/hello') self.query('LPUSH/hello/world') # "hello" is a list. try: - f = self.query('LRANGE/hello/world.txt') # let's try a range query on it (valid) but as text (invalid) + f = self.query('LRANGE/hello/0/-1.txt') # let's try a range query on it (valid) but as text (invalid) except urllib2.HTTPError as e: self.assertTrue(e.code == 400) return From a4408b6486ddfc70a7af56a07c9cef6457a68094 Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 15 Aug 2011 22:43:08 +0100 Subject: [PATCH 8/9] Removed static hiredis function from build --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6f31c30..1adde05 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ OUT=webdis -HIREDIS_OBJ=hiredis/hiredis.o hiredis/sds.o hiredis/net.o hiredis/async.o hiredis/dict.o +HIREDIS_OBJ=hiredis/hiredis.o hiredis/sds.o hiredis/net.o hiredis/async.o JANSSON_OBJ=jansson/src/dump.o jansson/src/error.o jansson/src/hashtable.o jansson/src/load.o jansson/src/strbuffer.o jansson/src/utf.o jansson/src/value.o jansson/src/variadic.o FORMAT_OBJS=formats/json.o formats/raw.o formats/common.o formats/custom-type.o formats/bson.o HTTP_PARSER_OBJS=http-parser/http_parser.o From 4e07580a4cee715447ee54aa6164c795ed358436 Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 15 Aug 2011 23:00:50 +0100 Subject: [PATCH 9/9] Better Makefile for MessagePack --- Makefile | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index 1adde05..876271d 100644 --- a/Makefile +++ b/Makefile @@ -1,14 +1,28 @@ OUT=webdis HIREDIS_OBJ=hiredis/hiredis.o hiredis/sds.o hiredis/net.o hiredis/async.o -JANSSON_OBJ=jansson/src/dump.o jansson/src/error.o jansson/src/hashtable.o jansson/src/load.o jansson/src/strbuffer.o jansson/src/utf.o jansson/src/value.o jansson/src/variadic.o +JANSSON_OBJ=jansson/src/dump.o jansson/src/error.o jansson/src/hashtable.o jansson/src/load.o jansson/src/strbuffer.o jansson/src/utf.o jansson/src/value.o jansson/src/variadic.o FORMAT_OBJS=formats/json.o formats/raw.o formats/common.o formats/custom-type.o formats/bson.o HTTP_PARSER_OBJS=http-parser/http_parser.o -DEPS=$(FORMAT_OBJS) $(HIREDIS_OBJ) $(JANSSON_OBJ) $(HTTP_PARSER_OBJS) -OBJS=webdis.o cmd.o worker.o slog.o server.o libb64/cencode.o acl.o md5/md5.o http.o client.o websocket.o pool.o conf.o $(DEPS) CFLAGS=-O3 -Wall -Wextra -I. -Ijansson/src -Ihttp-parser LDFLAGS=-levent -pthread +# check for MessagePack +MSGPACK_LIB=$(shell ls /usr/lib/libmsgpack.so) +ifneq ($(strip $(MSGPACK_LIB)),) + FORMAT_OBJS += formats/msgpack.o + CFLAGS += -DMSGPACK=1 + LDFLAGS += -lmsgpack +else + CFLAGS += -DMSGPACK=0 +endif + + +DEPS=$(FORMAT_OBJS) $(HIREDIS_OBJ) $(JANSSON_OBJ) $(HTTP_PARSER_OBJS) +OBJS=webdis.o cmd.o worker.o slog.o server.o libb64/cencode.o acl.o md5/md5.o http.o client.o websocket.o pool.o conf.o $(DEPS) + + + PREFIX ?= /usr/local CONFDIR ?= $(DESTDIR)/etc @@ -17,20 +31,7 @@ INSTALL_DIRS = $(DESTDIR) \ $(DESTDIR)/$(PREFIX)/bin \ $(CONFDIR) -all: .CHECK_FOR_MSGPACK - -.CHECK_FOR_MSGPACK: - @if [ -f /usr/lib/libmsgpack.so ]; then make .WITH_MSGPACK; else make .WITHOUT_MSGPACK ; fi - -.WITH_MSGPACK: - @echo "Building with MsgPack support" - @make CFLAGS="$(CFLAGS) -DMSGPACK=1" FORMAT_OBJS="$(FORMAT_OBJS) formats/msgpack.o" .REAL_BUILD - -.WITHOUT_MSGPACK: - @echo "Building without MsgPack support" - @make .REAL_BUILD - -.REAL_BUILD: $(OUT) Makefile +all: $(OUT) Makefile $(OUT): $(OBJS) Makefile $(CC) $(LDFLAGS) -o $(OUT) $(OBJS)