From 530359421cea307d85d429383e9d57cd275b47d6 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 24 Dec 2020 11:53:23 +0800 Subject: [PATCH 1/2] refactor(napi): create_buffer_with_borrowed_data now accepted raw ptr --- napi/src/env.rs | 18 ++++++----- test_module/__test__/buffer.spec.ts | 14 +++++++++ test_module/src/buffer.rs | 47 ++++++++++++++++++++++++++++- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/napi/src/env.rs b/napi/src/env.rs index 2a272b57..2588a838 100644 --- a/napi/src/env.rs +++ b/napi/src/env.rs @@ -320,20 +320,19 @@ impl Env { /// Provided `finalize_callback` will be called when `Buffer` got dropped. pub unsafe fn create_buffer_with_borrowed_data( &self, - data: &[u8], + data: *const u8, + length: usize, hint: Hint, finalize_callback: Option, ) -> Result where - Finalize: FnOnce(Env, Hint), + Finalize: FnOnce(Hint, Env), { - let length = data.len(); let mut raw_value = ptr::null_mut(); - let data_ptr = data.as_ptr(); check_status!(sys::napi_create_external_buffer( self.0, length, - data_ptr as *mut c_void, + data as *mut c_void, Some( raw_finalize_with_custom_callback:: as unsafe extern "C" fn( @@ -351,7 +350,7 @@ impl Env { value: raw_value, value_type: ValueType::Object, }), - mem::ManuallyDrop::new(Vec::from_raw_parts(data_ptr as *mut u8, length, length)), + mem::ManuallyDrop::new(Vec::from_raw_parts(data as *mut u8, length, length)), )) } @@ -1065,6 +1064,9 @@ impl Env { } } +/// This function could be used for `create_buffer_with_borrowed_data` and want do noting when Buffer finalized. +pub fn noop_finalize(_hint: Hint, _env: Env) {} + unsafe extern "C" fn drop_buffer( _env: sys::napi_env, finalize_data: *mut c_void, @@ -1125,10 +1127,10 @@ unsafe extern "C" fn raw_finalize_with_custom_callback( _finalize_data: *mut c_void, finalize_hint: *mut c_void, ) where - Finalize: FnOnce(Env, Hint), + Finalize: FnOnce(Hint, Env), { let (hint, maybe_callback) = *Box::from_raw(finalize_hint as *mut (Hint, Option)); if let Some(callback) = maybe_callback { - callback(Env::from_raw(env), hint); + callback(hint, Env::from_raw(env)); }; } diff --git a/test_module/__test__/buffer.spec.ts b/test_module/__test__/buffer.spec.ts index 21ebe469..5f5d4865 100644 --- a/test_module/__test__/buffer.spec.ts +++ b/test_module/__test__/buffer.spec.ts @@ -18,3 +18,17 @@ test('should copy', (t) => { t.deepEqual(copyBuffer, fixture) t.not(fixture, copyBuffer) }) + +test('should create borrowed buffer with noop finalize', (t) => { + t.deepEqual( + bindings.createBorrowedBufferWithNoopFinalize(), + Buffer.from([1, 2, 3]), + ) +}) + +test('should create borrowed buffer with finalize', (t) => { + t.deepEqual( + bindings.createBorrowedBufferWithFinalize(), + Buffer.from([1, 2, 3]), + ) +}) diff --git a/test_module/src/buffer.rs b/test_module/src/buffer.rs index d55214e6..0b7dd157 100644 --- a/test_module/src/buffer.rs +++ b/test_module/src/buffer.rs @@ -1,6 +1,10 @@ +use std::mem::ManuallyDrop; use std::str; -use napi::{CallContext, Error, JsBuffer, JsNumber, JsObject, JsString, Result, Status}; +use napi::{ + noop_finalize, CallContext, ContextlessResult, Env, Error, JsBuffer, JsNumber, JsObject, + JsString, Result, Status, +}; #[js_function(1)] pub fn get_buffer_length(ctx: CallContext) -> Result { @@ -22,9 +26,50 @@ pub fn copy_buffer(ctx: CallContext) -> Result { ctx.env.create_buffer_copy(buffer).map(|b| b.into_raw()) } +#[contextless_function] +pub fn create_borrowed_buffer_with_noop_finalize(env: Env) -> ContextlessResult { + let data = vec![1, 2, 3]; + let data_ptr = data.as_ptr(); + let length = data.len(); + let manually_drop = ManuallyDrop::new(data); + + unsafe { + env.create_buffer_with_borrowed_data(data_ptr, length, manually_drop, Some(noop_finalize)) + } + .map(|b| Some(b.into_raw())) +} + +#[contextless_function] +pub fn create_borrowed_buffer_with_finalize(env: Env) -> ContextlessResult { + let data = vec![1, 2, 3]; + let data_ptr = data.as_ptr(); + let length = data.len(); + let manually_drop = ManuallyDrop::new(data); + + unsafe { + env.create_buffer_with_borrowed_data( + data_ptr, + length, + manually_drop, + Some(|mut hint: ManuallyDrop>, _| { + ManuallyDrop::drop(&mut hint); + }), + ) + } + .map(|b| Some(b.into_raw())) +} + pub fn register_js(exports: &mut JsObject) -> Result<()> { exports.create_named_method("getBufferLength", get_buffer_length)?; exports.create_named_method("bufferToString", buffer_to_string)?; exports.create_named_method("copyBuffer", copy_buffer)?; + exports.create_named_method( + "createBorrowedBufferWithNoopFinalize", + create_borrowed_buffer_with_noop_finalize, + )?; + exports.create_named_method( + "createBorrowedBufferWithFinalize", + create_borrowed_buffer_with_finalize, + )?; Ok(()) } From 26a2c01cd6c661b881544d7184dd596d1d8477ba Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 24 Dec 2020 11:59:42 +0800 Subject: [PATCH 2/2] feat(napi): From Serde JSON Error for Error --- napi/src/error.rs | 9 ++++++ test_module/__test__/serde/serde-json.spec.ts | 28 +++++++++++++++++++ test_module/src/serde.rs | 23 ++++++++++++++- 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 test_module/__test__/serde/serde-json.spec.ts diff --git a/napi/src/error.rs b/napi/src/error.rs index a32b8c6c..98907c72 100644 --- a/napi/src/error.rs +++ b/napi/src/error.rs @@ -8,6 +8,8 @@ use std::os::raw::{c_char, c_void}; #[cfg(feature = "serde-json")] use serde::{de, ser}; +#[cfg(feature = "serde-json")] +use serde_json::Error as SerdeJSONError; use crate::{sys, Status}; @@ -38,6 +40,13 @@ impl de::Error for Error { } } +#[cfg(feature = "serde-json")] +impl From for Error { + fn from(value: SerdeJSONError) -> Self { + Error::new(Status::InvalidArg, format!("{}", value)) + } +} + impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if !self.reason.is_empty() { diff --git a/test_module/__test__/serde/serde-json.spec.ts b/test_module/__test__/serde/serde-json.spec.ts new file mode 100644 index 00000000..637dbf9a --- /dev/null +++ b/test_module/__test__/serde/serde-json.spec.ts @@ -0,0 +1,28 @@ +import test from 'ava' + +const bindings = require('../../index.node') + +const ValidObject = { + a: 1, + b: [-1.2, 1.1, 2.2, 3.3], + c: 'Hi', +} + +const InValidObject = { + a: -1, + b: [-1, 1.1, 2.2, 3.3], + c: 'Hello', +} + +test('should from json string', (t) => { + t.throws(() => bindings.from_json_string(JSON.stringify(InValidObject))) + t.deepEqual( + ValidObject, + bindings.from_json_string(JSON.stringify(ValidObject)), + ) +}) + +test('should convert to json string', (t) => { + t.throws(() => bindings.json_to_string(InValidObject)) + t.deepEqual(JSON.stringify(ValidObject), bindings.json_to_string(ValidObject)) +}) diff --git a/test_module/src/serde.rs b/test_module/src/serde.rs index 4871d1c4..bb24543a 100644 --- a/test_module/src/serde.rs +++ b/test_module/src/serde.rs @@ -1,4 +1,6 @@ -use napi::{CallContext, JsObject, JsUndefined, JsUnknown, Result}; +use napi::{CallContext, JsObject, JsString, JsUndefined, JsUnknown, Result}; + +use serde_json::{from_str, to_string}; #[derive(Serialize, Debug, Deserialize)] struct AnObject { @@ -162,6 +164,23 @@ fn roundtrip_object(ctx: CallContext) -> Result { ctx.env.to_js_value(&de_serialized) } +#[js_function(1)] +fn from_json_string(ctx: CallContext) -> Result { + let arg0 = ctx.get::(0)?.into_utf8()?; + + let de_serialized: AnObject = from_str(arg0.as_str()?)?; + ctx.env.to_js_value(&de_serialized) +} + +#[js_function(1)] +fn json_to_string(ctx: CallContext) -> Result { + let arg0 = ctx.get::(0)?; + + let de_serialized: AnObject = ctx.env.from_js_value(arg0)?; + let json_string = to_string(&de_serialized)?; + ctx.env.create_string_from_std(json_string) +} + pub fn register_js(exports: &mut JsObject) -> Result<()> { exports.create_named_method("make_num_77", make_num_77)?; exports.create_named_method("make_num_32", make_num_32)?; @@ -178,5 +197,7 @@ pub fn register_js(exports: &mut JsObject) -> Result<()> { exports.create_named_method("expect_buffer", expect_buffer)?; exports.create_named_method("roundtrip_object", roundtrip_object)?; + exports.create_named_method("from_json_string", from_json_string)?; + exports.create_named_method("json_to_string", json_to_string)?; Ok(()) }