From b90dc602418278050b8433c5dd48f379bc45d47a Mon Sep 17 00:00:00 2001 From: Marly Fleitas Date: Tue, 16 Jul 2019 21:12:14 +0200 Subject: [PATCH] AsyncWorker: add GetResult() method Adds an overridable `GetResult()` method, providing arguments to the callback invoked in `OnOK()`. Refs: https://github.com/nodejs/node-addon-api/issues/231#issuecomment-511504055 PR-URL: https://github.com/nodejs/node-addon-api/pull/512 Reviewed-By: Michael Dawson Reviewed-By: NickNaso Reviewed-By: Gabriel Schulhof --- doc/async_worker.md | 12 +++++++++++- napi-inl.h | 11 +++++++++-- napi.h | 1 + test/asyncworker.cc | 33 +++++++++++++++++++++++++++++++++ test/asyncworker.js | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 3 deletions(-) diff --git a/doc/async_worker.md b/doc/async_worker.md index 0c641c6..a13e8a1 100644 --- a/doc/async_worker.md +++ b/doc/async_worker.md @@ -107,11 +107,21 @@ virtual void Napi::AsyncWorker::Execute() = 0; This method is invoked when the computation in the `Execute` method ends. The default implementation runs the Callback optionally provided when the AsyncWorker class -was created. +was created. The callback will by default receive no arguments. To provide arguments, +override the `GetResult()` method. ```cpp virtual void Napi::AsyncWorker::OnOK(); ``` +### GetResult + +This method returns the arguments passed to the Callback invoked by the default +`OnOK()` implementation. The default implementation returns an empty vector, +providing no arguments to the Callback. + +```cpp +virtual std::vector Napi::AsyncWorker::GetResult(Napi::Env env); +``` ### OnError diff --git a/napi-inl.h b/napi-inl.h index 0db3c98..11822d4 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -3686,7 +3686,7 @@ inline void AsyncWorker::SuppressDestruct() { inline void AsyncWorker::OnOK() { if (!_callback.IsEmpty()) { - _callback.Call(_receiver.Value(), std::initializer_list{}); + _callback.Call(_receiver.Value(), GetResult(_callback.Env())); } } @@ -3700,7 +3700,14 @@ inline void AsyncWorker::SetError(const std::string& error) { _error = error; } -inline void AsyncWorker::OnExecute(napi_env /*env*/, void* this_pointer) { +inline std::vector AsyncWorker::GetResult(Napi::Env /*env*/) { + return {}; +} +// The OnExecute method receives an napi_env argument. However, do NOT +// use it within this method, as it does not run on the main thread and must +// not run any method that would cause JavaScript to run. In practice, this +// means that almost any use of napi_env will be incorrect. +inline void AsyncWorker::OnExecute(napi_env /*DO_NOT_USE*/, void* this_pointer) { AsyncWorker* self = static_cast(this_pointer); #ifdef NAPI_CPP_EXCEPTIONS try { diff --git a/napi.h b/napi.h index c194641..d772fd5 100644 --- a/napi.h +++ b/napi.h @@ -1812,6 +1812,7 @@ namespace Napi { virtual void OnOK(); virtual void OnError(const Error& e); virtual void Destroy(); + virtual std::vector GetResult(Napi::Env env); void SetError(const std::string& error); diff --git a/test/asyncworker.cc b/test/asyncworker.cc index bbd7e0b..3241465 100644 --- a/test/asyncworker.cc +++ b/test/asyncworker.cc @@ -29,6 +29,38 @@ class TestWorker : public AsyncWorker { bool _succeed; }; +class TestWorkerWithResult : public AsyncWorker { +public: + static void DoWork(const CallbackInfo& info) { + bool succeed = info[0].As(); + Object resource = info[1].As(); + Function cb = info[2].As(); + Value data = info[3]; + + TestWorkerWithResult* worker = new TestWorkerWithResult(cb, "TestResource", resource); + worker->Receiver().Set("data", data); + worker->_succeed = succeed; + worker->Queue(); + } + +protected: + void Execute() override { + if (!_succeed) { + SetError("test error"); + } + } + + std::vector GetResult(Napi::Env env) override { + return {Boolean::New(env, _succeed), + String::New(env, _succeed ? "ok" : "error")}; + } + +private: + TestWorkerWithResult(Function cb, const char* resource_name, const Object& resource) + : AsyncWorker(cb, resource_name, resource) {} + bool _succeed; +}; + class TestWorkerNoCallback : public AsyncWorker { public: static Value DoWork(const CallbackInfo& info) { @@ -65,5 +97,6 @@ Object InitAsyncWorker(Env env) { Object exports = Object::New(env); exports["doWork"] = Function::New(env, TestWorker::DoWork); exports["doWorkNoCallback"] = Function::New(env, TestWorkerNoCallback::DoWork); + exports["doWorkWithResult"] = Function::New(env, TestWorkerWithResult::DoWork); return exports; } diff --git a/test/asyncworker.js b/test/asyncworker.js index 676afd5..04415d5 100644 --- a/test/asyncworker.js +++ b/test/asyncworker.js @@ -66,6 +66,14 @@ function test(binding) { assert.strictEqual(typeof this, 'object'); assert.strictEqual(this.data, 'test data'); }, 'test data'); + + binding.asyncworker.doWorkWithResult(true, {}, function (succeed, succeedString) { + assert(arguments.length == 2); + assert(succeed); + assert(succeedString == "ok"); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + }, 'test data'); return; } @@ -91,6 +99,30 @@ function test(binding) { }).catch(common.mustNotCall()); } + { + const hooks = installAsyncHooksForTest(); + const triggerAsyncId = async_hooks.executionAsyncId(); + binding.asyncworker.doWorkWithResult(true, { foo: 'foo' }, function (succeed, succeedString) { + assert(arguments.length == 2); + assert(succeed); + assert(succeedString == "ok"); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + }, 'test data'); + + hooks.then(actual => { + assert.deepStrictEqual(actual, [ + { eventName: 'init', + type: 'TestResource', + triggerAsyncId: triggerAsyncId, + resource: { foo: 'foo' } }, + { eventName: 'before' }, + { eventName: 'after' }, + { eventName: 'destroy' } + ]); + }).catch(common.mustNotCall()); + } + { const hooks = installAsyncHooksForTest(); const triggerAsyncId = async_hooks.executionAsyncId();