From fc9c7de94b08049dd04c4ca6016586cdac0dd2ac Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Fri, 30 Apr 2021 16:51:54 +0200 Subject: [PATCH] cleanup(core): replace OpResponse with OpResult (#10434) Drop the Value/Buffer enum since #10432 allows buffers to be serialized rust => v8 --- core/bindings.rs | 42 ++++++++-------------------------------- core/lib.rs | 2 +- core/ops.rs | 44 +++++++++--------------------------------- core/plugin_api.rs | 2 +- core/runtime.rs | 17 ++++++---------- test_plugin/src/lib.rs | 6 +++--- 6 files changed, 28 insertions(+), 85 deletions(-) diff --git a/core/bindings.rs b/core/bindings.rs index 4574e2d4e3..e1c1a7a43c 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -5,7 +5,6 @@ use crate::JsRuntime; use crate::Op; use crate::OpId; use crate::OpPayload; -use crate::OpResponse; use crate::OpTable; use crate::PromiseId; use crate::ZeroCopyBuf; @@ -154,19 +153,6 @@ pub fn set_func( obj.set(scope, key.into(), val.into()); } -pub fn boxed_slice_to_uint8array<'sc>( - scope: &mut v8::HandleScope<'sc>, - buf: Box<[u8]>, -) -> v8::Local<'sc, v8::Uint8Array> { - assert!(!buf.is_empty()); - let buf_len = buf.len(); - let backing_store = v8::ArrayBuffer::new_backing_store_from_boxed_slice(buf); - let backing_store_shared = backing_store.make_shared(); - let ab = v8::ArrayBuffer::with_backing_store(scope, &backing_store_shared); - v8::Uint8Array::new(scope, ab, 0, buf_len) - .expect("Failed to create UintArray8") -} - pub extern "C" fn host_import_module_dynamically_callback( context: v8::Local, referrer: v8::Local, @@ -368,32 +354,20 @@ fn opcall<'s>( // Buf arg (optional) let arg3 = args.get(3); - let buf: Option = if arg3.is_null_or_undefined() { - None - } else { - match v8::Local::::try_from(arg3) - .map(|view| ZeroCopyBuf::new(scope, view)) - .map_err(AnyError::from) - { - Ok(buf) => Some(buf), - Err(err) => { - throw_type_error(scope, format!("Err with buf arg: {}", err)); - return; - } + let buf: Option = match serde_v8::from_v8(scope, arg3) { + Ok(buf) => buf, + Err(err) => { + throw_type_error(scope, format!("Err with buf arg: {}", err)); + return; } }; let payload = OpPayload::new(scope, v, promise_id); let op = OpTable::route_op(op_id, state.op_state.clone(), payload, buf); match op { - Op::Sync(resp) => match resp { - OpResponse::Value(v) => { - rv.set(v.to_v8(scope).unwrap()); - } - OpResponse::Buffer(buf) => { - rv.set(boxed_slice_to_uint8array(scope, buf).into()); - } - }, + Op::Sync(result) => { + rv.set(result.to_v8(scope).unwrap()); + } Op::Async(fut) => { state.pending_ops.push(fut); state.have_unpolled_ops = true; diff --git a/core/lib.rs b/core/lib.rs index a5bc5b3ffa..adfd9a6a22 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -60,7 +60,7 @@ pub use crate::ops::OpAsyncFuture; pub use crate::ops::OpFn; pub use crate::ops::OpId; pub use crate::ops::OpPayload; -pub use crate::ops::OpResponse; +pub use crate::ops::OpResult; pub use crate::ops::OpState; pub use crate::ops::OpTable; pub use crate::ops::PromiseId; diff --git a/core/ops.rs b/core/ops.rs index 6faffba4ba..99bff406c8 100644 --- a/core/ops.rs +++ b/core/ops.rs @@ -19,7 +19,7 @@ use std::pin::Pin; use std::rc::Rc; pub type PromiseId = u64; -pub type OpAsyncFuture = Pin>>; +pub type OpAsyncFuture = Pin>>; pub type OpFn = dyn Fn(Rc>, OpPayload, Option) -> Op + 'static; pub type OpId = usize; @@ -58,13 +58,8 @@ impl<'a, 'b, 'c> OpPayload<'a, 'b, 'c> { } } -pub enum OpResponse { - Value(OpResult), - Buffer(Box<[u8]>), -} - pub enum Op { - Sync(OpResponse), + Sync(OpResult), Async(OpAsyncFuture), /// AsyncUnref is the variation of Async, which doesn't block the program /// exiting. @@ -100,14 +95,14 @@ pub struct OpError { pub fn serialize_op_result( result: Result, state: Rc>, -) -> OpResponse { - OpResponse::Value(match result { +) -> OpResult { + match result { Ok(v) => OpResult::Ok(v.into()), Err(err) => OpResult::Err(OpError { class_name: (state.borrow().get_error_class_fn)(&err), message: err.to_string(), }), - }) + } } /// Maintains the resources and ops inside a JS runtime. @@ -205,35 +200,14 @@ mod tests { let bar_id; { let op_table = &mut state.borrow_mut().op_table; - foo_id = op_table.register_op("foo", |_, _, _| { - Op::Sync(OpResponse::Buffer(b"oof!"[..].into())) - }); + foo_id = op_table + .register_op("foo", |_, _, _| Op::Sync(OpResult::Ok(321.into()))); assert_eq!(foo_id, 1); - bar_id = op_table.register_op("bar", |_, _, _| { - Op::Sync(OpResponse::Buffer(b"rab!"[..].into())) - }); + bar_id = op_table + .register_op("bar", |_, _, _| Op::Sync(OpResult::Ok(123.into()))); assert_eq!(bar_id, 2); } - let foo_res = OpTable::route_op( - foo_id, - state.clone(), - OpPayload::empty(), - Default::default(), - ); - assert!( - matches!(foo_res, Op::Sync(OpResponse::Buffer(buf)) if &*buf == b"oof!") - ); - let bar_res = OpTable::route_op( - bar_id, - state.clone(), - OpPayload::empty(), - Default::default(), - ); - assert!( - matches!(bar_res, Op::Sync(OpResponse::Buffer(buf)) if &*buf == b"rab!") - ); - let mut catalog_entries = OpTable::op_entries(state); catalog_entries.sort_by(|(_, id1), (_, id2)| id1.partial_cmp(id2).unwrap()); assert_eq!( diff --git a/core/plugin_api.rs b/core/plugin_api.rs index d1dda160f4..9f37df6f3c 100644 --- a/core/plugin_api.rs +++ b/core/plugin_api.rs @@ -10,7 +10,7 @@ pub use crate::Op; pub use crate::OpId; -pub use crate::OpResponse; +pub use crate::OpResult; pub use crate::ZeroCopyBuf; pub type InitFn = fn(&mut dyn Interface); diff --git a/core/runtime.rs b/core/runtime.rs index 7573dfb676..171abbb5e9 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -23,7 +23,7 @@ use crate::ops::*; use crate::Extension; use crate::OpMiddlewareFn; use crate::OpPayload; -use crate::OpResponse; +use crate::OpResult; use crate::OpState; use crate::PromiseId; use crate::ZeroCopyBuf; @@ -49,7 +49,7 @@ use std::sync::Once; use std::task::Context; use std::task::Poll; -type PendingOpFuture = Pin>>; +type PendingOpFuture = Pin>>; pub enum Snapshot { Static(&'static [u8]), @@ -1394,9 +1394,9 @@ impl JsRuntime { fn poll_pending_ops( &mut self, cx: &mut Context, - ) -> Vec<(PromiseId, OpResponse)> { + ) -> Vec<(PromiseId, OpResult)> { let state_rc = Self::state(self.v8_isolate()); - let mut async_responses: Vec<(PromiseId, OpResponse)> = Vec::new(); + let mut async_responses: Vec<(PromiseId, OpResult)> = Vec::new(); let mut state = state_rc.borrow_mut(); @@ -1455,7 +1455,7 @@ impl JsRuntime { // Send finished responses to JS fn async_op_response( &mut self, - async_responses: Vec<(PromiseId, OpResponse)>, + async_responses: Vec<(PromiseId, OpResult)>, ) -> Result<(), AnyError> { let state_rc = Self::state(self.v8_isolate()); @@ -1480,12 +1480,7 @@ impl JsRuntime { for overflown_response in async_responses { let (promise_id, resp) = overflown_response; args.push(v8::Integer::new(scope, promise_id as i32).into()); - args.push(match resp { - OpResponse::Value(value) => value.to_v8(scope).unwrap(), - OpResponse::Buffer(buf) => { - bindings::boxed_slice_to_uint8array(scope, buf).into() - } - }); + args.push(resp.to_v8(scope).unwrap()); } let tc_scope = &mut v8::TryCatch::new(scope); diff --git a/test_plugin/src/lib.rs b/test_plugin/src/lib.rs index 51ed6d499e..71ba698fcb 100644 --- a/test_plugin/src/lib.rs +++ b/test_plugin/src/lib.rs @@ -2,7 +2,7 @@ use deno_core::plugin_api::Interface; use deno_core::plugin_api::Op; -use deno_core::plugin_api::OpResponse; +use deno_core::plugin_api::OpResult; use deno_core::plugin_api::ZeroCopyBuf; use futures::future::FutureExt; @@ -25,7 +25,7 @@ fn op_test_sync( } let result = b"test"; let result_box: Box<[u8]> = Box::new(*result); - Op::Sync(OpResponse::Buffer(result_box)) + Op::Sync(OpResult::Ok(result_box.into())) } fn op_test_async( @@ -48,7 +48,7 @@ fn op_test_async( assert!(rx.await.is_ok()); let result = b"test"; let result_box: Box<[u8]> = Box::new(*result); - (0, OpResponse::Buffer(result_box)) + (0, OpResult::Ok(result_box.into())) }; Op::Async(fut.boxed())