diff --git a/.travis.yml b/.travis.yml index 50e952b..4ef768a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,11 +14,13 @@ before_install: - node --version | grep -q 'v0.8' && npm install -g npm@2 || true node_js: + - "10" + - "9" + - "8" + - "7" - "6" - "5" - "4" - - "0.12" - - "0.10" notifications: email: false diff --git a/appveyor.yml b/appveyor.yml index 8e5d877..0cad4ee 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -3,11 +3,13 @@ init: environment: matrix: + - nodejs_version: 10 + - nodejs_version: 9 + - nodejs_version: 8 + - nodejs_version: 7 - nodejs_version: 6 - nodejs_version: 5 - nodejs_version: 4 - - nodejs_version: 0.12 - - nodejs_version: 0.10 platform: - x86 @@ -16,7 +18,8 @@ platform: install: - ps: Install-Product node $env:nodejs_version $env:platform - git submodule update --init --recursive - - npm install --msvs_version=2013 + - 'if "%nodejs_version%" LEQ 5 (npm install -g npm@5) else (npm install -g npm@latest)' + - npm install build: off diff --git a/binding.gyp b/binding.gyp index 92feee1..8d9c1d5 100644 --- a/binding.gyp +++ b/binding.gyp @@ -6,8 +6,13 @@ 'src/hiredis.cc' , 'src/reader.cc' ], - 'include_dirs': ["", "contributors": [ @@ -9,14 +9,16 @@ ], "main": "hiredis", "scripts": { - "test": "node test/reader.js && node test/writer.js" + "test": "node test/reader.js && node test/writer.js", + "install": "node-gyp rebuild" }, + "gypfile": true, "dependencies": { - "bindings": "^1.2.1", - "nan": "^2.3.4" + "bindings": "^1.3.0", + "node-addon-api": "^1.3.0" }, "engines": { - "node": ">= 0.10.0" + "node": ">= 4.0.0" }, "repository": { "type": "git", diff --git a/src/hiredis.cc b/src/hiredis.cc index c325409..fd5482d 100644 --- a/src/hiredis.cc +++ b/src/hiredis.cc @@ -1,10 +1,9 @@ +#include #include "reader.h" -using namespace v8; - -extern "C" { - static NAN_MODULE_INIT(init) { - hiredis::Reader::Initialize(target); - } - NODE_MODULE(hiredis, init) +Napi::Object Init(Napi::Env env, Napi::Object exports) { + hiredis::Reader::Initialize(env, exports); + return exports; } + +NODE_API_MODULE(hiredis, Init) \ No newline at end of file diff --git a/src/reader.cc b/src/reader.cc index b7659bf..86c5c02 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -1,227 +1,165 @@ #include #include #include "reader.h" - -using namespace hiredis; - -static void *tryParentize(const redisReadTask *task, const Local &v) { - Nan::HandleScope scope; - - Reader *r = reinterpret_cast(task->privdata); - size_t pidx, vidx; - - if (task->parent != NULL) { - pidx = (size_t)task->parent->obj; - assert(pidx > 0 && pidx < 9); - - /* When there is a parent, it should be an array. */ - Local lvalue = Nan::New(r->handle[pidx]); - assert(lvalue->IsArray()); - Local larray = lvalue.As(); - larray->Set(task->idx,v); - - /* Store the handle when this is an inner array. Otherwise, hiredis - * doesn't care about the return value as long as the value is set in - * its parent array. */ - vidx = pidx+1; - if (v->IsArray()) { - r->handle[vidx].Reset(v); - return (void*)vidx; +#include + +namespace hiredis { + + static void *tryParentize(const redisReadTask *task, const Napi::Value &v) { + Napi::HandleScope scope(v.Env()); + Reader *r = reinterpret_cast(task->privdata); + size_t pidx, vidx; + if (task->parent != NULL) { + pidx = (size_t)task->parent->obj; + assert(pidx > 0 && pidx < 9); + + // When there is a parent, it should be an array. + + Napi::Value lvalue = Napi::Value(scope.Env(), r->handle[pidx].Value()); + assert(lvalue.IsArray()); + Napi::Array larray = lvalue.As(); + larray.Set(task->idx, v); + + // Store the handle when this is an inner array. Otherwise, hiredis + // doesn't care about the return value as long as the value is set in + // its parent array. + vidx = pidx+1; + if (v.IsArray()) { + r->handle[vidx].Reset(v.ToObject()); + return (void*)vidx; + } else { + // Return value doesn't matter for inner value, as long as it is + // not NULL (which means OOM for hiredis). + return (void*)0xcafef00d; + } } else { - /* Return value doesn't matter for inner value, as long as it is - * not NULL (which means OOM for hiredis). */ - return (void*)0xcafef00d; - } - } else { - /* There is no parent, so this value is the root object. */ - r->handle[1].Reset(v); - return (void*)1; + if (v.IsNull()) { + r->handle[1].Reset(); + } else { + r->handle[1].Reset(v.ToObject()); + } + return (void*)1; + } } -} - -static void *createArray(const redisReadTask *task, int size) { - Nan::HandleScope scope; - - return tryParentize(task, Nan::New(size)); -} -static void *createString(const redisReadTask *task, char *str, size_t len) { - Nan::HandleScope scope; - - Reader *r = reinterpret_cast(task->privdata); - Local v(r->createString(str,len)); - - if (task->type == REDIS_REPLY_ERROR) - v = Exception::Error(v->ToString()); - return tryParentize(task,v); -} - -static void *createInteger(const redisReadTask *task, long long value) { - Nan::HandleScope scope; - return tryParentize(task, Nan::New(value)); -} - -static void *createNil(const redisReadTask *task) { - Nan::HandleScope scope; - return tryParentize(task, Nan::Null()); -} - -static redisReplyObjectFunctions v8ReplyFunctions = { - createString, - createArray, - createInteger, - createNil, - 0 /* No free function: cleanup is done in Reader::Get. */ -}; - -Reader::Reader(bool return_buffers) : - return_buffers(return_buffers) -{ - Nan::HandleScope scope; - - reader = redisReaderCreateWithFunctions(&v8ReplyFunctions); - reader->privdata = this; - -#if _USE_CUSTOM_BUFFER_POOL - if (return_buffers) { - Local global = Context::GetCurrent()->Global(); - Local bv = global->Get(String::NewSymbol("Buffer")); - assert(bv->IsFunction()); - Local bf = Local::Cast(bv); - buffer_fn = Persistent::New(bf); - - buffer_pool_length = 8*1024; /* Same as node */ - buffer_pool_offset = 0; - - node::Buffer *b = node::Buffer::New(buffer_pool_length); - buffer_pool = Persistent::New(b->handle_); + static void *createArray(const redisReadTask *task, int size) { + Reader *r = reinterpret_cast(task->privdata); + Napi::HandleScope scope(r->Env()); + return tryParentize(task, Napi::Array::New(scope.Env(), size)); } -#endif -} - -Reader::~Reader() { - redisReaderFree(reader); -} -/* Don't declare an extra scope here, so the objects are created within the - * scope inherited from the caller (Reader::Get) and we don't have to the pay - * the overhead. */ -inline Local Reader::createString(char *str, size_t len) { - if (return_buffers) { -#if _USE_CUSTOM_BUFFER_POOL - if (len > buffer_pool_length) { - node::Buffer *b = node::Buffer::New(str,len); - return Local::New(b->handle_); - } else { - return createBufferFromPool(str,len); + static void *createString(const redisReadTask *task, char *str, size_t len) { + Reader *r = reinterpret_cast(task->privdata); + Napi::HandleScope scope(r->Env()); + Napi::Value v(r->createString(str, len)); + if (task->type == REDIS_REPLY_ERROR) { + return tryParentize(task, Napi::Error::New(scope.Env(), v.ToString()).Value()); } -#else - return Nan::CopyBuffer(str,len).ToLocalChecked(); -#endif - } else { - return Nan::New(str,len).ToLocalChecked(); + return tryParentize(task, v); } -} -#if _USE_CUSTOM_BUFFER_POOL -Local Reader::createBufferFromPool(char *str, size_t len) { - HandleScope scope; - Local argv[3]; - Local instance; - - assert(len <= buffer_pool_length); - if (buffer_pool_length - buffer_pool_offset < len) { - node::Buffer *b = node::Buffer::New(buffer_pool_length); - buffer_pool.Dispose(); - buffer_pool = Persistent::New(b->handle_); - buffer_pool_offset = 0; + static void *createInteger(const redisReadTask *task, long long value) { + Reader *r = reinterpret_cast(task->privdata); + Napi::HandleScope scope(r->Env()); + return tryParentize(task, Napi::Number::New(scope.Env(), value)); } - memcpy(node::Buffer::Data(buffer_pool)+buffer_pool_offset,str,len); - - argv[0] = Local::New(buffer_pool); - argv[1] = Integer::New(len); - argv[2] = Integer::New(buffer_pool_offset); - instance = buffer_fn->NewInstance(3,argv); - buffer_pool_offset += len; - return scope.Close(instance); -} -#endif - -NAN_METHOD(Reader::New) { - bool return_buffers = false; - - if (info.Length() > 0 && info[0]->IsObject()) { - Local bv = Nan::Get(info[0].As(), Nan::New("return_buffers").ToLocalChecked()).ToLocalChecked(); - if (bv->IsBoolean()) - return_buffers = Nan::To(bv).FromJust(); + static void *createNil(const redisReadTask *task) { + Reader *r = reinterpret_cast(task->privdata); + Napi::HandleScope scope(r->Env()); + return tryParentize(task, scope.Env().Null()); } - Reader *r = new Reader(return_buffers); - r->Wrap(info.This()); - info.GetReturnValue().Set(info.This()); -} - -NAN_MODULE_INIT(Reader::Initialize) { - Nan::HandleScope scope; - - Local t = Nan::New(New); + static redisReplyObjectFunctions v8ReplyFunctions = { + createString, + createArray, + createInteger, + createNil, + 0 // No free function: cleanup is done in Reader::Get. + }; + + Napi::FunctionReference Reader::constructor; + + Reader::Reader(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) { + this->return_buffers = false; + if (info.Length() > 0 && info[0].IsObject()) { + Napi::Value bv = info[0].As().Get("return_buffers"); + if (bv.IsBoolean()) { + this->return_buffers = bv; + } + } + this->reader = redisReaderCreateWithFunctions(&v8ReplyFunctions); + this->reader->privdata = this; + } - t->InstanceTemplate()->SetInternalFieldCount(1); - Nan::SetPrototypeMethod(t, "feed", Feed); - Nan::SetPrototypeMethod(t, "get", Get); - Nan::Set(target, Nan::New("Reader").ToLocalChecked(), Nan::GetFunction(t).ToLocalChecked()); -} + Reader::~Reader() { + redisReaderFree(this->reader); + } -NAN_METHOD(Reader::Feed) { - Reader *r = Nan::ObjectWrap::Unwrap(info.This()); - - if (info.Length() == 0) { - Nan::ThrowTypeError("First argument must be a string or buffer"); - } else { - if (node::Buffer::HasInstance(info[0])) { - Local buffer_object = info[0].As(); - char *data; - size_t length; - - data = node::Buffer::Data(buffer_object); - length = node::Buffer::Length(buffer_object); - - /* Can't handle OOM for now. */ - assert(redisReaderFeed(r->reader, data, length) == REDIS_OK); - } else if (info[0]->IsString()) { - Nan::Utf8String str(info[0].As()); - redisReplyReaderFeed(r->reader, *str, str.length()); + /* Don't declare an extra scope here, so the objects are created within the + * scope inherited from the caller (Reader::Get) and we don't have to the pay + * the overhead. */ + inline Napi::Value Reader::createString(char *str, size_t len) { + if (this->return_buffers) { + return Napi::Buffer::Copy(this->Env(), str, len); } else { - Nan::ThrowError("Invalid argument"); + return Napi::String::New(this->Env(), str, len); } } - info.GetReturnValue().Set(info.This()); -} - -NAN_METHOD(Reader::Get) { - Reader *r = Nan::ObjectWrap::Unwrap(info.This()); - void *index = NULL; - Local reply; - int i; + Napi::Object Reader::Initialize(Napi::Env env, Napi::Object exports) { + Napi::HandleScope scope(env); + Napi::Function tpl = DefineClass(env, "Reader", { + InstanceMethod("get", &Reader::Get), + InstanceMethod("feed", &Reader::Feed) + }); + constructor = Napi::Persistent(tpl); + constructor.SuppressDestruct(); + exports.Set("Reader", tpl); + return exports; + } - if (redisReaderGetReply(r->reader,&index) == REDIS_OK) { - if (index == 0) { - return; + void Reader::Feed(const Napi::CallbackInfo& info) { + Napi::Env env = info.Env(); + if (info.Length() == 0) { + throw Napi::TypeError::New(env, "First argument must be a string or buffer"); } else { - /* Complete replies should always have a root object at index 1. */ - assert((size_t)index == 1); - reply = Nan::New(r->handle[1]); - - /* Dispose and clear used handles. */ - for (i = 1; i < 3; i++) { - r->handle[i].Reset(); + if (info[0].IsBuffer()) { + Napi::Buffer buffer = info[0].As>(); + const char *data = buffer.Data(); + size_t length = buffer.Length(); + /* Can't handle OOM for now. */ + assert(redisReaderFeed(this->reader, data, length) == REDIS_OK); + } else if (info[0].IsString()) { + std::string input = info[0].As().Utf8Value(); + const char *str = input.c_str(); + redisReplyReaderFeed(this->reader, str, input.length()); + } else { + throw Napi::Error::New(env, "invalid argument"); } } - } else { - Nan::ThrowError(r->reader->errstr); } - info.GetReturnValue().Set(reply); + Napi::Value Reader::Get(const Napi::CallbackInfo& info) { + Napi::Env env = info.Env(); + void *index = NULL; + Napi::Value reply; + int i; + if (redisReaderGetReply(this->reader, &index) == REDIS_OK) { + if (index == 0) { + return env.Undefined(); + } else { + /* Complete replies should always have a root object at index 1. */ + assert((size_t)index == 1); + reply = Napi::Value(env, this->handle[1].Value()); + /* Dispose and clear used handles. */ + for (i = 1; i < 3; i++) { + this->handle[i].Reset(); + } + } + } else { + throw Napi::Error::New(env, this->reader->errstr); + } + return reply; + } } diff --git a/src/reader.h b/src/reader.h index 6b04a9f..89e83e2 100644 --- a/src/reader.h +++ b/src/reader.h @@ -1,56 +1,39 @@ -#include +#include #include -#if NODE_MODULE_VERSION < NODE_0_12_MODULE_VERSION -#define _USE_CUSTOM_BUFFER_POOL 1 -#else -#define _USE_CUSTOM_BUFFER_POOL 0 -#endif - namespace hiredis { -using namespace v8; - -class Reader : public Nan::ObjectWrap { -public: - Reader(bool); - ~Reader(); - - static NAN_MODULE_INIT(Initialize); - static NAN_METHOD(New); - static NAN_METHOD(Feed); - static NAN_METHOD(Get); - - /* Objects created by the reply object functions need to get back to the - * reader when the reply is requested via Reader::Get(). Keep temporary - * objects in this handle. Use an array of handles because replies may - * include nested multi bulks and child-elements need to be added to the - * right respective parent. handle[0] will be unused, so the real index of - * an object in this array can be returned from the reply object functions. - * The returned value needs to be non-zero to distinguish complete replies - * from incomplete replies. These are persistent handles because - * Reader::Get might not return a full reply and the objects need to be - * kept around for subsequent calls. */ - Nan::Persistent handle[9]; - - /* Helper function to create string/buffer objects. */ - Local createString(char *str, size_t len); - -private: - redisReader *reader; - - /* Determines whether to return strings or buffers for single line and bulk - * replies. This defaults to false, so strings are returned by default. */ - bool return_buffers; - -#if _USE_CUSTOM_BUFFER_POOL - Local createBufferFromPool(char *str, size_t len); - Persistent buffer_fn; - Persistent buffer_pool; - size_t buffer_pool_length; - size_t buffer_pool_offset; -#endif -}; + class Reader : public Napi::ObjectWrap { + public: + explicit Reader(const Napi::CallbackInfo& info); + ~Reader(); + + static Napi::Object Initialize(Napi::Env env, Napi::Object exports); + void Feed(const Napi::CallbackInfo& info); + Napi::Value Get(const Napi::CallbackInfo& info); + /* Helper function to create string/buffer objects. */ + Napi::Value createString(char *str, size_t len); + /* Objects created by the reply object functions need to get back to the + * reader when the reply is requested via Reader::Get(). Keep temporary + * objects in this handle. Use an array of handles because replies may + * include nested multi bulks and child-elements need to be added to the + * right respective parent. handle[0] will be unused, so the real index of + * an object in this array can be returned from the reply object functions. + * The returned value needs to be non-zero to distinguish complete replies + * from incomplete replies. These are persistent handles because + * Reader::Get might not return a full reply and the objects need to be + * kept around for subsequent calls. */ + Napi::Reference handle[9]; + + private: + redisReader *reader; + + /* Determines whether to return strings or buffers for single line and bulk + * replies. This defaults to false, so strings are returned by default. */ + bool return_buffers; + + static Napi::FunctionReference constructor; + }; }; diff --git a/test/reader.js b/test/reader.js index 88f3cd9..d308a52 100644 --- a/test/reader.js +++ b/test/reader.js @@ -149,7 +149,7 @@ test("MultiBulkReplyWithNonStringValues", function() { test("FeedWithBuffer", function() { var reader = new hiredis.Reader(); reader.feed(new Buffer("$3\r\nfoo\r\n")); - assert.deepEqual("foo", reader.get()); + assert.deepEqual(new String("foo"), reader.get()); }); test("UndefinedReplyOnIncompleteFeed", function() { @@ -157,7 +157,7 @@ test("UndefinedReplyOnIncompleteFeed", function() { reader.feed("$3\r\nfoo"); assert.deepEqual(undefined, reader.get()); reader.feed("\r\n"); - assert.deepEqual("foo", reader.get()); + assert.deepEqual(new String("foo"), reader.get()); }); test("Leaks", function() {