From 5111113b39d21f0562d75a04ed2b40b2e5c73915 Mon Sep 17 00:00:00 2001 From: Nicola Del Gobbo Date: Fri, 17 Nov 2017 15:47:03 +0100 Subject: [PATCH 01/22] Update package.json to start porting --- package.json | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 6f463c7..de06ecd 100644 --- a/package.json +++ b/package.json @@ -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.0.0" }, "engines": { - "node": ">= 0.10.0" + "node": ">= 4.0.0" }, "repository": { "type": "git", From a86f7beeaef87ce90b8b111cde7882d5ebe590ec Mon Sep 17 00:00:00 2001 From: Nicola Del Gobbo Date: Tue, 21 Nov 2017 04:23:04 +0100 Subject: [PATCH 02/22] Update node-addon-api version to 1.1.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index de06ecd..d6964cf 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "gypfile": true, "dependencies": { "bindings": "^1.3.0", - "node-addon-api": "^1.0.0" + "node-addon-api": "^1.1.0" }, "engines": { "node": ">= 4.0.0" From f7c11b63462153bc5daf2e116c567ebdcc03aee1 Mon Sep 17 00:00:00 2001 From: Nicola Del Gobbo Date: Tue, 21 Nov 2017 04:34:00 +0100 Subject: [PATCH 03/22] Update binding.gyp --- binding.gyp | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) 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': [" Date: Tue, 21 Nov 2017 04:48:53 +0100 Subject: [PATCH 04/22] Porting entry point for addon --- src/hiredis.cc | 13 ++++++------- src/reader.h | 10 +++++----- 2 files changed, 11 insertions(+), 12 deletions(-) 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.h b/src/reader.h index 6b04a9f..1714437 100644 --- a/src/reader.h +++ b/src/reader.h @@ -1,11 +1,11 @@ -#include +#include #include -#if NODE_MODULE_VERSION < NODE_0_12_MODULE_VERSION +/*#if NODE_MODULE_VERSION < NODE_0_12_MODULE_VERSION #define _USE_CUSTOM_BUFFER_POOL 1 #else #define _USE_CUSTOM_BUFFER_POOL 0 -#endif +#endif*/ namespace hiredis { @@ -43,13 +43,13 @@ class Reader : public Nan::ObjectWrap { * replies. This defaults to false, so strings are returned by default. */ bool return_buffers; -#if _USE_CUSTOM_BUFFER_POOL +/*#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 +#endif*/ }; }; From baef9371d5fe731017eedaa9733f7903dfbace3e Mon Sep 17 00:00:00 2001 From: Nicola Del Gobbo Date: Tue, 21 Nov 2017 06:20:24 +0100 Subject: [PATCH 05/22] Removed 0.10 and 0.12 from CI --- .travis.yml | 2 -- appveyor.yml | 2 -- 2 files changed, 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 50e952b..03d60be 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,8 +17,6 @@ node_js: - "6" - "5" - "4" - - "0.12" - - "0.10" notifications: email: false diff --git a/appveyor.yml b/appveyor.yml index 8e5d877..6db1218 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -6,8 +6,6 @@ environment: - nodejs_version: 6 - nodejs_version: 5 - nodejs_version: 4 - - nodejs_version: 0.12 - - nodejs_version: 0.10 platform: - x86 From 8ad8aa32d93a3af2633d6802a486403a14e5f804 Mon Sep 17 00:00:00 2001 From: Nicola Del Gobbo Date: Tue, 21 Nov 2017 06:51:34 +0100 Subject: [PATCH 06/22] First step on reader.h refactor --- src/reader.cc | 65 ++++++++++++++++++++++----------------------------- src/reader.h | 18 +++++++------- 2 files changed, 36 insertions(+), 47 deletions(-) diff --git a/src/reader.cc b/src/reader.cc index b7659bf..5efc342 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -74,15 +74,12 @@ static redisReplyObjectFunctions v8ReplyFunctions = { 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 +Reader::Reader(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) { + this->return_buffers = false; + this->reader = redisReaderCreateWithFunctions(&v8ReplyFunctions); + this->eader->privdata = this; + /*#if _USE_CUSTOM_BUFFER_POOL if (return_buffers) { Local global = Context::GetCurrent()->Global(); Local bv = global->Get(String::NewSymbol("Buffer")); @@ -90,14 +87,14 @@ Reader::Reader(bool return_buffers) : Local bf = Local::Cast(bv); buffer_fn = Persistent::New(bf); - buffer_pool_length = 8*1024; /* Same as node */ + 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_); } -#endif -} +#endif*/ + } Reader::~Reader() { redisReaderFree(reader); @@ -123,7 +120,7 @@ inline Local Reader::createString(char *str, size_t len) { } } -#if _USE_CUSTOM_BUFFER_POOL +/*#if _USE_CUSTOM_BUFFER_POOL Local Reader::createBufferFromPool(char *str, size_t len) { HandleScope scope; Local argv[3]; @@ -146,34 +143,22 @@ Local Reader::createBufferFromPool(char *str, size_t len) { 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(); - } - - Reader *r = new Reader(return_buffers); - r->Wrap(info.This()); - info.GetReturnValue().Set(info.This()); +#endif*/ + +Napi::Object Reader::Initialize(Napi::Env env, Napi::Object exports) { + 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; } -NAN_MODULE_INIT(Reader::Initialize) { - Nan::HandleScope scope; +Napi::Value Reader::Feed(const Napi::CallbackInfo& info) { - Local t = Nan::New(New); - - 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()); -} -NAN_METHOD(Reader::Feed) { Reader *r = Nan::ObjectWrap::Unwrap(info.This()); if (info.Length() == 0) { @@ -198,9 +183,13 @@ NAN_METHOD(Reader::Feed) { } info.GetReturnValue().Set(info.This()); + + } -NAN_METHOD(Reader::Get) { +Napi::Value Reader::Get(const Napi::CallbackInfo& info) { + + Reader *r = Nan::ObjectWrap::Unwrap(info.This()); void *index = NULL; Local reply; @@ -224,4 +213,6 @@ NAN_METHOD(Reader::Get) { } info.GetReturnValue().Set(reply); + + } diff --git a/src/reader.h b/src/reader.h index 1714437..fc06df7 100644 --- a/src/reader.h +++ b/src/reader.h @@ -9,17 +9,15 @@ namespace hiredis { -using namespace v8; - -class Reader : public Nan::ObjectWrap { +class Reader : public Napi::ObjectWrap { public: - Reader(bool); + explicit Reader(const Napi::CallbackInfo& info); ~Reader(); - static NAN_MODULE_INIT(Initialize); - static NAN_METHOD(New); - static NAN_METHOD(Feed); - static NAN_METHOD(Get); + static Napi::Object Initialize(Napi::Env, Napi::Object exports); + + Napi::Value Feed(const Napi::CallbackInfo& info); + Napi::Value Get(const Napi::CallbackInfo& info); /* Objects created by the reply object functions need to get back to the * reader when the reply is requested via Reader::Get(). Keep temporary @@ -31,10 +29,10 @@ class Reader : public Nan::ObjectWrap { * 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]; + //Nan::Persistent handle[9]; /* Helper function to create string/buffer objects. */ - Local createString(char *str, size_t len); + //Local createString(char *str, size_t len); private: redisReader *reader; From 65458aa48795798c098f7b4227a610a1bab37b13 Mon Sep 17 00:00:00 2001 From: NickNaso Date: Wed, 29 Nov 2017 21:09:45 +0100 Subject: [PATCH 07/22] Updated CI to test over nodejs version 4 5 6 7 8 9 --- .travis.yml | 3 +++ appveyor.yml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/.travis.yml b/.travis.yml index 03d60be..4544355 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,6 +14,9 @@ before_install: - node --version | grep -q 'v0.8' && npm install -g npm@2 || true node_js: + - "9" + - "8" + - "7" - "6" - "5" - "4" diff --git a/appveyor.yml b/appveyor.yml index 6db1218..e2a4117 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -3,6 +3,9 @@ init: environment: matrix: + - nodejs_version: 9 + - nodejs_version: 8 + - nodejs_version: 7 - nodejs_version: 6 - nodejs_version: 5 - nodejs_version: 4 From 5dc1d6ae549fb27ecc5b573ef39468684092d262 Mon Sep 17 00:00:00 2001 From: NickNaso Date: Thu, 30 Nov 2017 13:42:25 +0100 Subject: [PATCH 08/22] cleanup old code --- src/reader.cc | 198 +------------------------------------------------- src/reader.h | 18 +++-- 2 files changed, 14 insertions(+), 202 deletions(-) diff --git a/src/reader.cc b/src/reader.cc index 5efc342..ca6505f 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -4,146 +4,9 @@ using namespace hiredis; -static void *tryParentize(const redisReadTask *task, const Local &v) { - Nan::HandleScope scope; +Reader::Reader(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) {} - 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; - } 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; - } -} - -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(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) { - this->return_buffers = false; - this->reader = redisReaderCreateWithFunctions(&v8ReplyFunctions); - this->eader->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_); - } -#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); - } -#else - return Nan::CopyBuffer(str,len).ToLocalChecked(); -#endif - } else { - return Nan::New(str,len).ToLocalChecked(); - } -} - -/*#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; - } - - 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*/ +Reader::~Reader() {} Napi::Object Reader::Initialize(Napi::Env env, Napi::Object exports) { Napi::Function tpl = DefineClass(env, "Reader", { @@ -157,62 +20,9 @@ Napi::Object Reader::Initialize(Napi::Env env, Napi::Object exports) { } Napi::Value Reader::Feed(const Napi::CallbackInfo& info) { - - - 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()); - } else { - Nan::ThrowError("Invalid argument"); - } - } - - info.GetReturnValue().Set(info.This()); - - + return info.Env().Undefined(); } Napi::Value Reader::Get(const Napi::CallbackInfo& info) { - - - Reader *r = Nan::ObjectWrap::Unwrap(info.This()); - void *index = NULL; - Local reply; - int i; - - if (redisReaderGetReply(r->reader,&index) == REDIS_OK) { - if (index == 0) { - return; - } 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(); - } - } - } else { - Nan::ThrowError(r->reader->errstr); - } - - info.GetReturnValue().Set(reply); - - + return info.Env().Undefined(); } diff --git a/src/reader.h b/src/reader.h index fc06df7..5b0fd38 100644 --- a/src/reader.h +++ b/src/reader.h @@ -19,6 +19,15 @@ class Reader : public Napi::ObjectWrap { Napi::Value Feed(const Napi::CallbackInfo& info); Napi::Value Get(const Napi::CallbackInfo& info); +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; + /* 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 @@ -29,18 +38,11 @@ class Reader : public Napi::ObjectWrap { * 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]; + Napi::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; From f701311114148967d3994eaba17805ebbc49d687 Mon Sep 17 00:00:00 2001 From: NickNaso Date: Sat, 3 Mar 2018 00:54:04 +0100 Subject: [PATCH 09/22] First step of porting hiredis to N-API --- package.json | 2 +- src/hiredis.cc | 2 +- src/reader.cc | 2 +- src/reader.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index d6964cf..da4cd9e 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "gypfile": true, "dependencies": { "bindings": "^1.3.0", - "node-addon-api": "^1.1.0" + "node-addon-api": "^1.2.0" }, "engines": { "node": ">= 4.0.0" diff --git a/src/hiredis.cc b/src/hiredis.cc index fd5482d..3370eba 100644 --- a/src/hiredis.cc +++ b/src/hiredis.cc @@ -6,4 +6,4 @@ Napi::Object Init(Napi::Env env, Napi::Object exports) { return exports; } -NODE_API_MODULE(hiredis, Init) \ No newline at end of file +NODE_API_MODULE(NODE_GYP_MODEULE_NAME, Init) \ No newline at end of file diff --git a/src/reader.cc b/src/reader.cc index ca6505f..208e52f 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -4,7 +4,7 @@ using namespace hiredis; -Reader::Reader(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) {} +Reader::Reader(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) {} Reader::~Reader() {} diff --git a/src/reader.h b/src/reader.h index 5b0fd38..8af6486 100644 --- a/src/reader.h +++ b/src/reader.h @@ -38,7 +38,7 @@ class Reader : public Napi::ObjectWrap { * 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::Persistent handle[9]; + //static Napi::Persistent handle[9]; /* Helper function to create string/buffer objects. */ //Local createString(char *str, size_t len); From b1ae91afd4bf583003072e86bbdc535a0f9f213a Mon Sep 17 00:00:00 2001 From: NickNaso Date: Sun, 1 Apr 2018 12:22:41 +0200 Subject: [PATCH 10/22] Start working on hiredis porting to N-API --- hiredis.js | 2 +- package.json | 2 +- src/hiredis.cc | 2 +- src/reader.cc | 177 +++++++++++++++++++++++++++++++++++++++++++++++-- src/reader.h | 24 +++---- 5 files changed, 187 insertions(+), 20 deletions(-) diff --git a/hiredis.js b/hiredis.js index 230bf9d..5e87cc2 100644 --- a/hiredis.js +++ b/hiredis.js @@ -1,5 +1,5 @@ var net = require("net"), - hiredis = require('bindings')('hiredis.node'); + hiredis = require('bindings')('hiredis'); var bufStar = new Buffer("*", "ascii"); var bufDollar = new Buffer("$", "ascii"); diff --git a/package.json b/package.json index da4cd9e..7943bf6 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ ], "main": "hiredis", "scripts": { - "test": "node test/reader.js && node test/writer.js", + "test": "node test/reader.js", "install": "node-gyp rebuild" }, "gypfile": true, diff --git a/src/hiredis.cc b/src/hiredis.cc index 3370eba..fd5482d 100644 --- a/src/hiredis.cc +++ b/src/hiredis.cc @@ -6,4 +6,4 @@ Napi::Object Init(Napi::Env env, Napi::Object exports) { return exports; } -NODE_API_MODULE(NODE_GYP_MODEULE_NAME, Init) \ No newline at end of file +NODE_API_MODULE(hiredis, Init) \ No newline at end of file diff --git a/src/reader.cc b/src/reader.cc index 208e52f..8233af5 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -4,11 +4,143 @@ using namespace hiredis; -Reader::Reader(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) {} +/*static void *tryParentize(const redisReadTask *task, const Napi::Value &v) { + //Nan::HandleScope scope; -Reader::~Reader() {} + 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; + } 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; + } +}*/ + +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()); + //Local larray = lvalue.As(); + 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); + 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 { + // There is no parent, so this value is the root object. + r->handle[1].Reset(v); + return (void*)1; + } + +} + +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)); + +} + +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) + //v = Napi::Error::New(scope.Env(), v.ToString()); + return tryParentize(task,v); +} + +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)); +} + +static void *createNil(const redisReadTask *task) { + Reader *r = reinterpret_cast(task->privdata); + Napi::HandleScope scope(r->Env()); + return tryParentize(task, scope.Env().Null()); +} + +static redisReplyObjectFunctions v8ReplyFunctions = { + createString, + createArray, + createInteger, + createNil, + 0 // No free function: cleanup is done in Reader::Get. +}; + +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; +} + +Reader::~Reader() { + redisReaderFree(this->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 Napi::Value Reader::createString(char *str, size_t len) { + if (this->return_buffers) { + return Napi::Buffer::Copy(this->Env(), str, len); + } else { + return Napi::String::New(this->Env(), str, len); + } +} 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) @@ -20,9 +152,46 @@ Napi::Object Reader::Initialize(Napi::Env env, Napi::Object exports) { } Napi::Value Reader::Feed(const Napi::CallbackInfo& info) { - return info.Env().Undefined(); + Napi::Env env = info.Env(); + if (info.Length() == 0) { + throw Napi::TypeError::New(env, "First argument must be a string or buffer"); + } else { + if (info[0].IsBuffer()) { + Napi::Buffer buffer = info[0].As>(); + 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"); + } + } + return info.This(); } Napi::Value Reader::Get(const Napi::CallbackInfo& info) { - return info.Env().Undefined(); + Napi::Env env = info.Env(); + void *index = NULL; + Napi::Value reply; + int i; + if (redisReaderGetReply(this->reader, &index) == REDIS_OK) { + if (index == 0) { + 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 8af6486..42ba4c3 100644 --- a/src/reader.h +++ b/src/reader.h @@ -18,16 +18,8 @@ class Reader : public Napi::ObjectWrap { Napi::Value Feed(const Napi::CallbackInfo& info); Napi::Value Get(const Napi::CallbackInfo& info); - -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; - + /* 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 @@ -38,10 +30,16 @@ class Reader : public Napi::ObjectWrap { * 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. */ - //static Napi::Persistent handle[9]; + static Napi::Reference 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; + + static Napi::FunctionReference constructor; /*#if _USE_CUSTOM_BUFFER_POOL Local createBufferFromPool(char *str, size_t len); From 87d83dbe5b642908638cfb14a13988d753b289a8 Mon Sep 17 00:00:00 2001 From: NickNaso Date: Mon, 2 Apr 2018 15:40:32 +0200 Subject: [PATCH 11/22] Update on native interface --- src/reader.cc | 313 ++++++++++++++++++++++---------------------------- src/reader.h | 80 ++++++------- 2 files changed, 180 insertions(+), 213 deletions(-) diff --git a/src/reader.cc b/src/reader.cc index 8233af5..544d50f 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -2,196 +2,163 @@ #include #include "reader.h" -using namespace hiredis; - -/*static void *tryParentize(const redisReadTask *task, const Napi::Value &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; - } 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; - } -}*/ - -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()); - //Local larray = lvalue.As(); - 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); - return (void*)vidx; +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()); + //Local larray = lvalue.As(); + 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); + 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; + // There is no parent, so this value is the root object. + r->handle[1].Reset(v); + return (void*)1; + } } - -} - -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)); - -} -static void *createString(const redisReadTask *task, char *str, size_t len) { - Reader *r = reinterpret_cast(task->privdata); - Napi::HandleScope scope(r->Env()); + 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)); + } - Napi::Value v(r->createString(str, len)); - if (task->type == REDIS_REPLY_ERROR) - //v = Napi::Error::New(scope.Env(), v.ToString()); - return tryParentize(task,v); -} + 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) { + // TODO + //v = Napi::Error::New(scope.Env(), v.ToString()); + } + return tryParentize(task, v); + } -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)); -} + 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)); + } -static void *createNil(const redisReadTask *task) { - Reader *r = reinterpret_cast(task->privdata); - Napi::HandleScope scope(r->Env()); - return tryParentize(task, scope.Env().Null()); -} + static void *createNil(const redisReadTask *task) { + Reader *r = reinterpret_cast(task->privdata); + Napi::HandleScope scope(r->Env()); + return tryParentize(task, scope.Env().Null()); + } -static redisReplyObjectFunctions v8ReplyFunctions = { - createString, - createArray, - createInteger, - createNil, - 0 // No free function: cleanup is done in Reader::Get. -}; - -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; -} + 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; + } -Reader::~Reader() { - redisReaderFree(this->reader); -} + Reader::~Reader() { + redisReaderFree(this->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 Napi::Value Reader::createString(char *str, size_t len) { - if (this->return_buffers) { - return Napi::Buffer::Copy(this->Env(), str, len); - } else { - return Napi::String::New(this->Env(), str, len); + /* 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 { + return Napi::String::New(this->Env(), str, len); + } } -} -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; -} + 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; + } -Napi::Value 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 { - if (info[0].IsBuffer()) { - Napi::Buffer buffer = info[0].As>(); - 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()); + Napi::Value 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 { - throw Napi::Error::New(env, "invalid argument"); + if (info[0].IsBuffer()) { + Napi::Buffer buffer = info[0].As>(); + 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"); + } } + return info.This(); } - return info.This(); -} -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) { - 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(); + 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); } - } else { - throw Napi::Error::New(env, this->reader->errstr); + return reply; } - return reply; } diff --git a/src/reader.h b/src/reader.h index 42ba4c3..07b5d27 100644 --- a/src/reader.h +++ b/src/reader.h @@ -9,46 +9,46 @@ namespace hiredis { -class Reader : public Napi::ObjectWrap { -public: - explicit Reader(const Napi::CallbackInfo& info); - ~Reader(); - - static Napi::Object Initialize(Napi::Env, Napi::Object exports); - - Napi::Value 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. */ - static 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; - -/*#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); + + Napi::Value 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; + + /*#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*/ + }; }; From 3b5fb600d4187291739aa9792c7bb69372cfc3c5 Mon Sep 17 00:00:00 2001 From: NickNaso Date: Wed, 4 Apr 2018 22:22:18 +0200 Subject: [PATCH 12/22] Removed unused element on interface --- src/reader.h | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/reader.h b/src/reader.h index 07b5d27..beac019 100644 --- a/src/reader.h +++ b/src/reader.h @@ -15,8 +15,7 @@ namespace hiredis { ~Reader(); static Napi::Object Initialize(Napi::Env env, Napi::Object exports); - - Napi::Value Feed(const Napi::CallbackInfo& info); + 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); @@ -31,7 +30,7 @@ namespace hiredis { * 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; @@ -40,14 +39,6 @@ namespace hiredis { bool return_buffers; static Napi::FunctionReference constructor; - - /*#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*/ }; }; From 8804d39650118cc2fae664137c4f0e6b04fd7f82 Mon Sep 17 00:00:00 2001 From: NickNaso Date: Wed, 4 Apr 2018 22:22:57 +0200 Subject: [PATCH 13/22] Fixed problem on Reference --- package.json | 2 +- raw.js | 9 +++ src/reader.cc | 26 ++++--- test/testify.js | 185 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 207 insertions(+), 15 deletions(-) create mode 100644 raw.js create mode 100644 test/testify.js diff --git a/package.json b/package.json index 7943bf6..17ad907 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ ], "main": "hiredis", "scripts": { - "test": "node test/reader.js", + "test": "node test/testify.js", "install": "node-gyp rebuild" }, "gypfile": true, diff --git a/raw.js b/raw.js new file mode 100644 index 0000000..b65d0f5 --- /dev/null +++ b/raw.js @@ -0,0 +1,9 @@ +var hiredis = require('bindings')('hiredis') + +var reader = new hiredis.Reader(); +console.log(reader.feed) +console.log(reader.get) + +reader.feed("+OK\r\n"); +console.log("feed called") +console.log(reader.get()) \ No newline at end of file diff --git a/src/reader.cc b/src/reader.cc index 544d50f..91f31ff 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -1,6 +1,7 @@ #include #include #include "reader.h" +#include namespace hiredis { @@ -13,10 +14,8 @@ namespace hiredis { 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()); - //Local larray = lvalue.As(); Napi::Array larray = lvalue.As(); larray.Set(task->idx, v); @@ -25,16 +24,15 @@ namespace hiredis { // its parent array. vidx = pidx+1; if (v.IsArray()) { - r->handle[vidx].Reset(v); + 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 { - // There is no parent, so this value is the root object. - r->handle[1].Reset(v); + } else { + r->handle[1].Reset(v.ToObject()); return (void*)1; } } @@ -50,8 +48,8 @@ namespace hiredis { Napi::HandleScope scope(r->Env()); Napi::Value v(r->createString(str, len)); if (task->type == REDIS_REPLY_ERROR) { - // TODO - //v = Napi::Error::New(scope.Env(), v.ToString()); + std::cout << "ERRORE " << REDIS_REPLY_ERROR << std::endl; + //return tryParentize(task, Napi::Error::New(scope.Env(), v.ToString())); } return tryParentize(task, v); } @@ -108,8 +106,8 @@ namespace hiredis { 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) + InstanceMethod("get", &Reader::Get), + InstanceMethod("feed", &Reader::Feed) }); constructor = Napi::Persistent(tpl); constructor.SuppressDestruct(); @@ -117,7 +115,7 @@ namespace hiredis { return exports; } - Napi::Value Reader::Feed(const Napi::CallbackInfo& info) { + 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"); @@ -136,7 +134,7 @@ namespace hiredis { throw Napi::Error::New(env, "invalid argument"); } } - return info.This(); + //return info.This(); } Napi::Value Reader::Get(const Napi::CallbackInfo& info) { @@ -148,10 +146,10 @@ namespace hiredis { if (index == 0) { return env.Undefined(); } else { - /* Complete replies should always have a root object at index 1. */ + // 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. */ + // Dispose and clear used handles. for (i = 1; i < 3; i++) { this->handle[i].Reset(); } diff --git a/test/testify.js b/test/testify.js new file mode 100644 index 0000000..786d5cd --- /dev/null +++ b/test/testify.js @@ -0,0 +1,185 @@ +var assert = require("assert"), + test = require("./testlib")(), + hiredis = require("../hiredis"); + +test("CreateReader", function() { + var reader = new hiredis.Reader(); + assert.notEqual(reader, null); +}); + +test("StatusReply", function() { + var reader = new hiredis.Reader(); + reader.feed("+OK\r\n"); + assert.equal("OK", reader.get()); +}); + +test("StatusReplyAsBuffer", function() { + var reader = new hiredis.Reader({ return_buffers: true }); + reader.feed("+OK\r\n"); + var reply = reader.get(); + assert.ok(Buffer.isBuffer(reply)); + assert.equal("OK", reply.toString()); +}); + +test("IntegerReply", function() { + var reader = new hiredis.Reader(); + reader.feed(":1\r\n"); + assert.equal(1, reader.get()); +}); + +test("LargeIntegerReply", function() { + var reader = new hiredis.Reader(); + reader.feed(":9223372036854775807\r\n"); + // We test for a different value here, as JavaScript has no 64-bit integers, + // only IEEE double precision floating point numbers + assert.equal("9223372036854776000", String(reader.get())); +}); + +/*test("ErrorReply", function() { + var reader = new hiredis.Reader(); + reader.feed("-ERR foo\r\n"); + var reply = reader.get(); + assert.equal(Error, reply.constructor); + assert.equal("ERR foo", reply.message); +});*/ + +/*test("ErrorReplyWithReturnBuffers", function() { + var reader = new hiredis.Reader({ return_buffers: true }); + reader.feed("-ERR foo\r\n"); + var reply = reader.get(); + assert.equal(Error, reply.constructor); + assert.equal("ERR foo", reply.message); +});*/ + +/*test("NullBulkReply", function() { + var reader = new hiredis.Reader(); + reader.feed("$-1\r\n"); + assert.equal(null, reader.get()); +});*/ + +test("EmptyBulkReply", function() { + var reader = new hiredis.Reader(); + reader.feed("$0\r\n\r\n"); + assert.equal("", reader.get()); +}); + +test("BulkReply", function() { + var reader = new hiredis.Reader(); + reader.feed("$3\r\nfoo\r\n"); + assert.equal("foo", reader.get()); +}); + +test("BulkReplyAsBuffer", function() { + var reader = new hiredis.Reader({ return_buffers: true }); + reader.feed("$3\r\nfoo\r\n"); + var reply = reader.get(); + assert.ok(Buffer.isBuffer(reply)); + assert.equal("foo", reply.toString()); +}); + +test("BulkReplyWithEncoding", function() { + var reader = new hiredis.Reader(); + reader.feed("$" + Buffer.byteLength("☃") + "\r\n☃\r\n"); + assert.equal("☃", reader.get()); +}); + +/*test("NullMultiBulkReply", function() { + var reader = new hiredis.Reader(); + reader.feed("*-1\r\n"); + assert.equal(null, reader.get()); +});*/ + +test("EmptyMultiBulkReply", function() { + var reader = new hiredis.Reader(); + reader.feed("*0\r\n"); + assert.deepEqual([], reader.get()); +}); + +test("MultiBulkReply", function() { + var reader = new hiredis.Reader(); + reader.feed("*2\r\n$3\r\nfoo\r\n$3\r\nbar\r\n"); + assert.deepEqual(["foo", "bar"], reader.get()); +}); + +test("NestedMultiBulkReply", function() { + var reader = new hiredis.Reader(); + reader.feed("*2\r\n*2\r\n$3\r\nfoo\r\n$3\r\nbar\r\n$3\r\nqux\r\n"); + assert.deepEqual([["foo", "bar"], "qux"], reader.get()); +}); + +test("DeeplyNestedMultiBulkReply", function() { + var i; + var reader = new hiredis.Reader(); + var expected = 1; + + for (i = 0; i < 8; i++) { + reader.feed("*1\r\n"); + expected = [expected]; + } + + reader.feed(":1\r\n"); + + assert.deepEqual(reader.get(), expected); +}); + +test("TooDeeplyNestedMultiBulkReply", function() { + var i; + var reader = new hiredis.Reader(); + + for (i = 0; i < 9; i++) { + reader.feed("*1\r\n"); + } + + reader.feed(":1\r\n"); + + assert.throws( + function() { + reader.get(); + }, + /nested multi/ + ); +}); + +test("MultiBulkReplyWithNonStringValues", function() { + var reader = new hiredis.Reader(); + reader.feed("*3\r\n:1\r\n+OK\r\n$-1\r\n"); + assert.deepEqual([1, "OK", null], reader.get()); +}); + +/*test("FeedWithBuffer", function() { + var reader = new hiredis.Reader(); + reader.feed(new Buffer("$3\r\nfoo\r\n")); + assert.deepEqual("foo", reader.get()); +});*/ + +/*test("UndefinedReplyOnIncompleteFeed", function() { + var reader = new hiredis.Reader(); + reader.feed("$3\r\nfoo"); + assert.deepEqual(undefined, reader.get()); + reader.feed("\r\n"); + assert.deepEqual("foo", reader.get()); +});*/ + +test("Leaks", function() { + /* The "leaks" utility is only available on OSX. */ + if (process.platform != "darwin") return; + + var done = 0; + var leaks = require('child_process').spawn("leaks", [process.pid]); + leaks.stdout.on("data", function(data) { + var str = data.toString(); + var notice = "Node 0.2.5 always leaks 16 bytes (this is " + process.versions.node + ")"; + var matches; + if ((matches = /(\d+) leaks?/i.exec(str)) != null) { + if (parseInt(matches[1]) > 0) { + console.log(str); + console.log('\x1B[31mNotice: ' + notice + '\x1B[0m'); + } + } + done = 1; + }); + + process.on('exit', function() { + assert.ok(done, "Leaks test should have completed"); + }); +}); \ No newline at end of file From d33e45414dda820120385e4165b0508f64ebf4b2 Mon Sep 17 00:00:00 2001 From: NickNaso Date: Wed, 4 Apr 2018 22:24:20 +0200 Subject: [PATCH 14/22] Removed unused code from the interface --- src/reader.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/reader.h b/src/reader.h index beac019..89e83e2 100644 --- a/src/reader.h +++ b/src/reader.h @@ -1,12 +1,6 @@ #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 { class Reader : public Napi::ObjectWrap { From 35db9c6d9bbd1c2f1c1f8e0d24538261885395f6 Mon Sep 17 00:00:00 2001 From: Nicola Del Gobbo Date: Wed, 4 Apr 2018 23:38:10 +0200 Subject: [PATCH 15/22] Fix some little errors --- package.json | 2 +- raw.js | 9 --- src/reader.cc | 26 +++---- test/testify.js | 185 ------------------------------------------------ 4 files changed, 15 insertions(+), 207 deletions(-) delete mode 100644 raw.js delete mode 100644 test/testify.js diff --git a/package.json b/package.json index 17ad907..7943bf6 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ ], "main": "hiredis", "scripts": { - "test": "node test/testify.js", + "test": "node test/reader.js", "install": "node-gyp rebuild" }, "gypfile": true, diff --git a/raw.js b/raw.js deleted file mode 100644 index b65d0f5..0000000 --- a/raw.js +++ /dev/null @@ -1,9 +0,0 @@ -var hiredis = require('bindings')('hiredis') - -var reader = new hiredis.Reader(); -console.log(reader.feed) -console.log(reader.get) - -reader.feed("+OK\r\n"); -console.log("feed called") -console.log(reader.get()) \ No newline at end of file diff --git a/src/reader.cc b/src/reader.cc index 91f31ff..544d50f 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -1,7 +1,6 @@ #include #include #include "reader.h" -#include namespace hiredis { @@ -14,8 +13,10 @@ namespace hiredis { 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()); + //Local larray = lvalue.As(); Napi::Array larray = lvalue.As(); larray.Set(task->idx, v); @@ -24,15 +25,16 @@ namespace hiredis { // its parent array. vidx = pidx+1; if (v.IsArray()) { - r->handle[vidx].Reset(v.ToObject()); + r->handle[vidx].Reset(v); 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 { - r->handle[1].Reset(v.ToObject()); + } else { + // There is no parent, so this value is the root object. + r->handle[1].Reset(v); return (void*)1; } } @@ -48,8 +50,8 @@ namespace hiredis { Napi::HandleScope scope(r->Env()); Napi::Value v(r->createString(str, len)); if (task->type == REDIS_REPLY_ERROR) { - std::cout << "ERRORE " << REDIS_REPLY_ERROR << std::endl; - //return tryParentize(task, Napi::Error::New(scope.Env(), v.ToString())); + // TODO + //v = Napi::Error::New(scope.Env(), v.ToString()); } return tryParentize(task, v); } @@ -106,8 +108,8 @@ namespace hiredis { 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) + InstanceMethod("get", &Reader::Get), + InstanceMethod("feed", &Reader::Feed) }); constructor = Napi::Persistent(tpl); constructor.SuppressDestruct(); @@ -115,7 +117,7 @@ namespace hiredis { return exports; } - void Reader::Feed(const Napi::CallbackInfo& info) { + Napi::Value 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"); @@ -134,7 +136,7 @@ namespace hiredis { throw Napi::Error::New(env, "invalid argument"); } } - //return info.This(); + return info.This(); } Napi::Value Reader::Get(const Napi::CallbackInfo& info) { @@ -146,10 +148,10 @@ namespace hiredis { if (index == 0) { return env.Undefined(); } else { - // Complete replies should always have a root object at index 1. + /* 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. + /* Dispose and clear used handles. */ for (i = 1; i < 3; i++) { this->handle[i].Reset(); } diff --git a/test/testify.js b/test/testify.js deleted file mode 100644 index 786d5cd..0000000 --- a/test/testify.js +++ /dev/null @@ -1,185 +0,0 @@ -var assert = require("assert"), - test = require("./testlib")(), - hiredis = require("../hiredis"); - -test("CreateReader", function() { - var reader = new hiredis.Reader(); - assert.notEqual(reader, null); -}); - -test("StatusReply", function() { - var reader = new hiredis.Reader(); - reader.feed("+OK\r\n"); - assert.equal("OK", reader.get()); -}); - -test("StatusReplyAsBuffer", function() { - var reader = new hiredis.Reader({ return_buffers: true }); - reader.feed("+OK\r\n"); - var reply = reader.get(); - assert.ok(Buffer.isBuffer(reply)); - assert.equal("OK", reply.toString()); -}); - -test("IntegerReply", function() { - var reader = new hiredis.Reader(); - reader.feed(":1\r\n"); - assert.equal(1, reader.get()); -}); - -test("LargeIntegerReply", function() { - var reader = new hiredis.Reader(); - reader.feed(":9223372036854775807\r\n"); - // We test for a different value here, as JavaScript has no 64-bit integers, - // only IEEE double precision floating point numbers - assert.equal("9223372036854776000", String(reader.get())); -}); - -/*test("ErrorReply", function() { - var reader = new hiredis.Reader(); - reader.feed("-ERR foo\r\n"); - var reply = reader.get(); - assert.equal(Error, reply.constructor); - assert.equal("ERR foo", reply.message); -});*/ - -/*test("ErrorReplyWithReturnBuffers", function() { - var reader = new hiredis.Reader({ return_buffers: true }); - reader.feed("-ERR foo\r\n"); - var reply = reader.get(); - assert.equal(Error, reply.constructor); - assert.equal("ERR foo", reply.message); -});*/ - -/*test("NullBulkReply", function() { - var reader = new hiredis.Reader(); - reader.feed("$-1\r\n"); - assert.equal(null, reader.get()); -});*/ - -test("EmptyBulkReply", function() { - var reader = new hiredis.Reader(); - reader.feed("$0\r\n\r\n"); - assert.equal("", reader.get()); -}); - -test("BulkReply", function() { - var reader = new hiredis.Reader(); - reader.feed("$3\r\nfoo\r\n"); - assert.equal("foo", reader.get()); -}); - -test("BulkReplyAsBuffer", function() { - var reader = new hiredis.Reader({ return_buffers: true }); - reader.feed("$3\r\nfoo\r\n"); - var reply = reader.get(); - assert.ok(Buffer.isBuffer(reply)); - assert.equal("foo", reply.toString()); -}); - -test("BulkReplyWithEncoding", function() { - var reader = new hiredis.Reader(); - reader.feed("$" + Buffer.byteLength("☃") + "\r\n☃\r\n"); - assert.equal("☃", reader.get()); -}); - -/*test("NullMultiBulkReply", function() { - var reader = new hiredis.Reader(); - reader.feed("*-1\r\n"); - assert.equal(null, reader.get()); -});*/ - -test("EmptyMultiBulkReply", function() { - var reader = new hiredis.Reader(); - reader.feed("*0\r\n"); - assert.deepEqual([], reader.get()); -}); - -test("MultiBulkReply", function() { - var reader = new hiredis.Reader(); - reader.feed("*2\r\n$3\r\nfoo\r\n$3\r\nbar\r\n"); - assert.deepEqual(["foo", "bar"], reader.get()); -}); - -test("NestedMultiBulkReply", function() { - var reader = new hiredis.Reader(); - reader.feed("*2\r\n*2\r\n$3\r\nfoo\r\n$3\r\nbar\r\n$3\r\nqux\r\n"); - assert.deepEqual([["foo", "bar"], "qux"], reader.get()); -}); - -test("DeeplyNestedMultiBulkReply", function() { - var i; - var reader = new hiredis.Reader(); - var expected = 1; - - for (i = 0; i < 8; i++) { - reader.feed("*1\r\n"); - expected = [expected]; - } - - reader.feed(":1\r\n"); - - assert.deepEqual(reader.get(), expected); -}); - -test("TooDeeplyNestedMultiBulkReply", function() { - var i; - var reader = new hiredis.Reader(); - - for (i = 0; i < 9; i++) { - reader.feed("*1\r\n"); - } - - reader.feed(":1\r\n"); - - assert.throws( - function() { - reader.get(); - }, - /nested multi/ - ); -}); - -test("MultiBulkReplyWithNonStringValues", function() { - var reader = new hiredis.Reader(); - reader.feed("*3\r\n:1\r\n+OK\r\n$-1\r\n"); - assert.deepEqual([1, "OK", null], reader.get()); -}); - -/*test("FeedWithBuffer", function() { - var reader = new hiredis.Reader(); - reader.feed(new Buffer("$3\r\nfoo\r\n")); - assert.deepEqual("foo", reader.get()); -});*/ - -/*test("UndefinedReplyOnIncompleteFeed", function() { - var reader = new hiredis.Reader(); - reader.feed("$3\r\nfoo"); - assert.deepEqual(undefined, reader.get()); - reader.feed("\r\n"); - assert.deepEqual("foo", reader.get()); -});*/ - -test("Leaks", function() { - /* The "leaks" utility is only available on OSX. */ - if (process.platform != "darwin") return; - - var done = 0; - var leaks = require('child_process').spawn("leaks", [process.pid]); - leaks.stdout.on("data", function(data) { - var str = data.toString(); - var notice = "Node 0.2.5 always leaks 16 bytes (this is " + process.versions.node + ")"; - var matches; - if ((matches = /(\d+) leaks?/i.exec(str)) != null) { - if (parseInt(matches[1]) > 0) { - console.log(str); - console.log('\x1B[31mNotice: ' + notice + '\x1B[0m'); - } - } - done = 1; - }); - - process.on('exit', function() { - assert.ok(done, "Leaks test should have completed"); - }); -}); \ No newline at end of file From deca2824fd76f2cf491133c6af3bd68b397b0037 Mon Sep 17 00:00:00 2001 From: Nicola Del Gobbo Date: Wed, 4 Apr 2018 23:53:23 +0200 Subject: [PATCH 16/22] Auto stash before revert of "Fixed problem on Reference" --- raw.js | 9 +++++++++ src/reader.cc | 20 +++++++++++--------- test/testify.js | 26 ++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 raw.js create mode 100644 test/testify.js diff --git a/raw.js b/raw.js new file mode 100644 index 0000000..c9c8694 --- /dev/null +++ b/raw.js @@ -0,0 +1,9 @@ +var hiredis = require('bindings')('hiredis') + +var reader = new hiredis.Reader(); + +test("NullBulkReply", function() { + var reader = new hiredis.Reader(); + reader.feed("$-1\r\n"); + assert.equal(null, reader.get()); +}); \ No newline at end of file diff --git a/src/reader.cc b/src/reader.cc index 544d50f..5601975 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -5,10 +5,12 @@ namespace hiredis { static void *tryParentize(const redisReadTask *task, const Napi::Value &v) { + std::cout << "called tryparentize" << std::endl; Napi::HandleScope scope(v.Env()); Reader *r = reinterpret_cast(task->privdata); size_t pidx, vidx; if (task->parent != NULL) { + std::cout << "1 è diverso dal cazzo di null" << std::endl; pidx = (size_t)task->parent->obj; assert(pidx > 0 && pidx < 9); @@ -16,8 +18,7 @@ namespace hiredis { Napi::Value lvalue = Napi::Value(scope.Env(), r->handle[pidx].Value()); assert(lvalue.IsArray()); - //Local larray = lvalue.As(); - Napi::Array larray = lvalue.As(); + Napi::Array larray = lvalue.As(); larray.Set(task->idx, v); // Store the handle when this is an inner array. Otherwise, hiredis @@ -25,7 +26,8 @@ namespace hiredis { // its parent array. vidx = pidx+1; if (v.IsArray()) { - r->handle[vidx].Reset(v); + std::cout << "v.IsArray()" << std::endl; + r->handle[vidx].Reset(v.ToObject()); return (void*)vidx; } else { // Return value doesn't matter for inner value, as long as it is @@ -33,8 +35,8 @@ namespace hiredis { return (void*)0xcafef00d; } } else { - // There is no parent, so this value is the root object. - r->handle[1].Reset(v); + std::cout << "Ugliù ma qua risulta null" << std::endl; + r->handle[1].Reset(); return (void*)1; } } @@ -50,8 +52,8 @@ namespace hiredis { Napi::HandleScope scope(r->Env()); Napi::Value v(r->createString(str, len)); if (task->type == REDIS_REPLY_ERROR) { - // TODO - //v = Napi::Error::New(scope.Env(), v.ToString()); + std::cout << "ERRORE " << REDIS_REPLY_ERROR << std::endl; + //return tryParentize(task, Napi::Error::New(scope.Env(), v.ToString())); } return tryParentize(task, v); } @@ -64,7 +66,7 @@ namespace hiredis { static void *createNil(const redisReadTask *task) { Reader *r = reinterpret_cast(task->privdata); - Napi::HandleScope scope(r->Env()); + Napi::HandleScope scope(r->Env()); return tryParentize(task, scope.Env().Null()); } @@ -124,7 +126,7 @@ namespace hiredis { } else { if (info[0].IsBuffer()) { Napi::Buffer buffer = info[0].As>(); - char *data = buffer.Data(); + const char *data = buffer.Data(); size_t length = buffer.Length(); /* Can't handle OOM for now. */ assert(redisReaderFeed(this->reader, data, length) == REDIS_OK); diff --git a/test/testify.js b/test/testify.js new file mode 100644 index 0000000..7b11d5d --- /dev/null +++ b/test/testify.js @@ -0,0 +1,26 @@ +var assert = require("assert"), + test = require("./testlib")(), + hiredis = require("../hiredis"); + +test("StatusReplyAsBuffer", function() { + var reader = new hiredis.Reader({ return_buffers: true }); + reader.feed("+OK\r\n"); + var reply = reader.get(); + assert.ok(Buffer.isBuffer(reply)); + assert.equal("OK", reply.toString()); +}); + +/*test("FeedWithBuffer", function() { + var reader = new hiredis.Reader(); + reader.feed(new Buffer("$3\r\nfoo\r\n")); + assert.deepEqual("foo", reader.get()); +});*/ + +/*test("UndefinedReplyOnIncompleteFeed", function() { + var reader = new hiredis.Reader(); + reader.feed("$3\r\nfoo"); + assert.deepEqual(undefined, reader.get()); + reader.feed("\r\n"); + assert.deepEqual("foo", reader.get()); +});*/ + From 2acc6b14fb394984ba5716edbd43912dc7bf1ab7 Mon Sep 17 00:00:00 2001 From: NickNaso Date: Thu, 5 Apr 2018 01:45:20 +0200 Subject: [PATCH 17/22] End preliminary work on porting to N-API --- package.json | 2 +- src/reader.cc | 23 +++++++++++------------ test/reader.js | 4 ++-- test/testify.js | 26 -------------------------- 4 files changed, 14 insertions(+), 41 deletions(-) delete mode 100644 test/testify.js diff --git a/package.json b/package.json index 7943bf6..da4cd9e 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ ], "main": "hiredis", "scripts": { - "test": "node test/reader.js", + "test": "node test/reader.js && node test/writer.js", "install": "node-gyp rebuild" }, "gypfile": true, diff --git a/src/reader.cc b/src/reader.cc index 5601975..86c5c02 100644 --- a/src/reader.cc +++ b/src/reader.cc @@ -1,16 +1,15 @@ #include #include #include "reader.h" +#include namespace hiredis { static void *tryParentize(const redisReadTask *task, const Napi::Value &v) { - std::cout << "called tryparentize" << std::endl; Napi::HandleScope scope(v.Env()); Reader *r = reinterpret_cast(task->privdata); size_t pidx, vidx; if (task->parent != NULL) { - std::cout << "1 è diverso dal cazzo di null" << std::endl; pidx = (size_t)task->parent->obj; assert(pidx > 0 && pidx < 9); @@ -26,7 +25,6 @@ namespace hiredis { // its parent array. vidx = pidx+1; if (v.IsArray()) { - std::cout << "v.IsArray()" << std::endl; r->handle[vidx].Reset(v.ToObject()); return (void*)vidx; } else { @@ -35,8 +33,11 @@ namespace hiredis { return (void*)0xcafef00d; } } else { - std::cout << "Ugliù ma qua risulta null" << std::endl; - r->handle[1].Reset(); + if (v.IsNull()) { + r->handle[1].Reset(); + } else { + r->handle[1].Reset(v.ToObject()); + } return (void*)1; } } @@ -52,8 +53,7 @@ namespace hiredis { Napi::HandleScope scope(r->Env()); Napi::Value v(r->createString(str, len)); if (task->type == REDIS_REPLY_ERROR) { - std::cout << "ERRORE " << REDIS_REPLY_ERROR << std::endl; - //return tryParentize(task, Napi::Error::New(scope.Env(), v.ToString())); + return tryParentize(task, Napi::Error::New(scope.Env(), v.ToString()).Value()); } return tryParentize(task, v); } @@ -93,7 +93,7 @@ namespace hiredis { } Reader::~Reader() { - redisReaderFree(this->reader); + redisReaderFree(this->reader); } /* Don't declare an extra scope here, so the objects are created within the @@ -110,8 +110,8 @@ namespace hiredis { 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) + InstanceMethod("get", &Reader::Get), + InstanceMethod("feed", &Reader::Feed) }); constructor = Napi::Persistent(tpl); constructor.SuppressDestruct(); @@ -119,7 +119,7 @@ namespace hiredis { return exports; } - Napi::Value Reader::Feed(const Napi::CallbackInfo& info) { + 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"); @@ -138,7 +138,6 @@ namespace hiredis { throw Napi::Error::New(env, "invalid argument"); } } - return info.This(); } Napi::Value Reader::Get(const Napi::CallbackInfo& info) { 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() { diff --git a/test/testify.js b/test/testify.js deleted file mode 100644 index 7b11d5d..0000000 --- a/test/testify.js +++ /dev/null @@ -1,26 +0,0 @@ -var assert = require("assert"), - test = require("./testlib")(), - hiredis = require("../hiredis"); - -test("StatusReplyAsBuffer", function() { - var reader = new hiredis.Reader({ return_buffers: true }); - reader.feed("+OK\r\n"); - var reply = reader.get(); - assert.ok(Buffer.isBuffer(reply)); - assert.equal("OK", reply.toString()); -}); - -/*test("FeedWithBuffer", function() { - var reader = new hiredis.Reader(); - reader.feed(new Buffer("$3\r\nfoo\r\n")); - assert.deepEqual("foo", reader.get()); -});*/ - -/*test("UndefinedReplyOnIncompleteFeed", function() { - var reader = new hiredis.Reader(); - reader.feed("$3\r\nfoo"); - assert.deepEqual(undefined, reader.get()); - reader.feed("\r\n"); - assert.deepEqual("foo", reader.get()); -});*/ - From 4bb094bd5e091210fc028938740a7e5bb5a9778d Mon Sep 17 00:00:00 2001 From: NickNaso Date: Thu, 5 Apr 2018 01:53:26 +0200 Subject: [PATCH 18/22] Deleted unuseful file used on development phase --- raw.js | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 raw.js diff --git a/raw.js b/raw.js deleted file mode 100644 index c9c8694..0000000 --- a/raw.js +++ /dev/null @@ -1,9 +0,0 @@ -var hiredis = require('bindings')('hiredis') - -var reader = new hiredis.Reader(); - -test("NullBulkReply", function() { - var reader = new hiredis.Reader(); - reader.feed("$-1\r\n"); - assert.equal(null, reader.get()); -}); \ No newline at end of file From 4e2ee7ea8f8774d1325d0965e5b1dafd6534363a Mon Sep 17 00:00:00 2001 From: NickNaso Date: Thu, 5 Apr 2018 01:54:46 +0200 Subject: [PATCH 19/22] End preliminary work to porting on N-API --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index da4cd9e..9a3e688 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "hiredis", "description": "Wrapper for reply processing code in hiredis", - "version": "0.5.0", + "version": "0.5.1-napi", "homepage": "http://github.com/redis/hiredis-node", "author": "Jan-Erik Rediger ", "contributors": [ From bb5037c1cdede22ce2194bba52b7e6a6c866c072 Mon Sep 17 00:00:00 2001 From: NickNaso Date: Tue, 8 May 2018 22:11:08 +0200 Subject: [PATCH 20/22] Updated node-addon-api to 1.3.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9a3e688..a55e70f 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "gypfile": true, "dependencies": { "bindings": "^1.3.0", - "node-addon-api": "^1.2.0" + "node-addon-api": "^1.3.0" }, "engines": { "node": ">= 4.0.0" From 4479fa5e0c4eaec6a326f9847248faa560e0d23d Mon Sep 17 00:00:00 2001 From: NickNaso Date: Wed, 9 May 2018 16:37:02 +0200 Subject: [PATCH 21/22] Updated CI with node 10 and removed --msvs_version=2013 --- .travis.yml | 1 + appveyor.yml | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4544355..4ef768a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,6 +14,7 @@ before_install: - node --version | grep -q 'v0.8' && npm install -g npm@2 || true node_js: + - "10" - "9" - "8" - "7" diff --git a/appveyor.yml b/appveyor.yml index e2a4117..75adffd 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -3,6 +3,7 @@ init: environment: matrix: + - nodejs_version: 10 - nodejs_version: 9 - nodejs_version: 8 - nodejs_version: 7 @@ -17,7 +18,7 @@ platform: install: - ps: Install-Product node $env:nodejs_version $env:platform - git submodule update --init --recursive - - npm install --msvs_version=2013 + - npm install build: off From bb3bab2cf47fe6139177e06e0cd23cd5a351172e Mon Sep 17 00:00:00 2001 From: NickNaso Date: Wed, 9 May 2018 21:01:03 +0200 Subject: [PATCH 22/22] Update CI configuration --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index 75adffd..0cad4ee 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -18,6 +18,7 @@ platform: install: - ps: Install-Product node $env:nodejs_version $env:platform - git submodule update --init --recursive + - 'if "%nodejs_version%" LEQ 5 (npm install -g npm@5) else (npm install -g npm@latest)' - npm install build: off