From cc83ad39ce3e8186c8785d47fa4801317c8fac4a Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Sun, 28 Nov 2021 13:07:03 -0500 Subject: [PATCH] refactor: add deno_fetch::Options for init (#12904) deno_fetch::init has a lot of parameters and generic on two types that keeps expanding over time. This refactor adds deno_fetch::Options struct for more clearly defining the various parameters. --- Cargo.lock | 7 ++++ ext/fetch/Cargo.toml | 1 + ext/fetch/lib.rs | 76 ++++++++++++++++++++++++++++--------------- runtime/build.rs | 10 +----- runtime/web_worker.rs | 18 +++++----- runtime/worker.rs | 18 +++++----- 6 files changed, 76 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c6a22cd5d..f9b235790b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -805,6 +805,7 @@ dependencies = [ "data-url", "deno_core", "deno_tls", + "dyn-clone", "http", "reqwest", "serde", @@ -1158,6 +1159,12 @@ dependencies = [ "text_lines", ] +[[package]] +name = "dyn-clone" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee2626afccd7561a06cf1367e2950c4718ea04565e20fb5029b6c7d8ad09abcf" + [[package]] name = "ecdsa" version = "0.12.4" diff --git a/ext/fetch/Cargo.toml b/ext/fetch/Cargo.toml index fdc078e818..ed36e200cf 100644 --- a/ext/fetch/Cargo.toml +++ b/ext/fetch/Cargo.toml @@ -18,6 +18,7 @@ bytes = "1.1.0" data-url = "0.1.0" deno_core = { version = "0.109.0", path = "../../core" } deno_tls = { version = "0.14.0", path = "../tls" } +dyn-clone = "1" http = "0.2.4" reqwest = { version = "0.11.4", default-features = false, features = ["rustls-tls", "stream", "gzip", "brotli"] } serde = { version = "1.0.129", features = ["derive"] } diff --git a/ext/fetch/lib.rs b/ext/fetch/lib.rs index 5bae92c8e3..3d4c51f8ff 100644 --- a/ext/fetch/lib.rs +++ b/ext/fetch/lib.rs @@ -57,18 +57,35 @@ pub use reqwest; pub use fs_fetch_handler::FsFetchHandler; -pub fn init( - user_agent: String, - root_cert_store: Option, - proxy: Option, - request_builder_hook: Option RequestBuilder>, - unsafely_ignore_certificate_errors: Option>, - client_cert_chain_and_key: Option<(String, String)>, - file_fetch_handler: FH, -) -> Extension +pub struct Options { + pub user_agent: String, + pub root_cert_store: Option, + pub proxy: Option, + pub request_builder_hook: Option RequestBuilder>, + pub unsafely_ignore_certificate_errors: Option>, + pub client_cert_chain_and_key: Option<(String, String)>, + pub file_fetch_handler: Box, +} + +struct BoxFetchHandler(Box); + +impl Default for Options { + fn default() -> Self { + Self { + user_agent: "".to_string(), + root_cert_store: None, + proxy: None, + request_builder_hook: None, + unsafely_ignore_certificate_errors: None, + client_cert_chain_and_key: None, + file_fetch_handler: Box::new(DefaultFileFetchHandler), + } + } +} + +pub fn init(options: Options) -> Extension where FP: FetchPermissions + 'static, - FH: FetchHandler + 'static, { Extension::builder() .js(include_js_files!( @@ -83,7 +100,7 @@ where "26_fetch.js", )) .ops(vec![ - ("op_fetch", op_sync(op_fetch::)), + ("op_fetch", op_sync(op_fetch::)), ("op_fetch_send", op_async(op_fetch_send)), ( "op_fetch_custom_client", @@ -93,25 +110,28 @@ where .state(move |state| { state.put::({ create_http_client( - user_agent.clone(), - root_cert_store.clone(), + options.user_agent.clone(), + options.root_cert_store.clone(), vec![], - proxy.clone(), - unsafely_ignore_certificate_errors.clone(), - client_cert_chain_and_key.clone(), + options.proxy.clone(), + options.unsafely_ignore_certificate_errors.clone(), + options.client_cert_chain_and_key.clone(), ) .unwrap() }); state.put::(HttpClientDefaults { - user_agent: user_agent.clone(), - root_cert_store: root_cert_store.clone(), - proxy: proxy.clone(), - request_builder_hook, - unsafely_ignore_certificate_errors: unsafely_ignore_certificate_errors + user_agent: options.user_agent.clone(), + root_cert_store: options.root_cert_store.clone(), + proxy: options.proxy.clone(), + request_builder_hook: options.request_builder_hook, + unsafely_ignore_certificate_errors: options + .unsafely_ignore_certificate_errors .clone(), - client_cert_chain_and_key: client_cert_chain_and_key.clone(), + client_cert_chain_and_key: options.client_cert_chain_and_key.clone(), }); - state.put::(file_fetch_handler.clone()); + state.put(BoxFetchHandler(dyn_clone::clone_box( + &*options.file_fetch_handler, + ))); Ok(()) }) .build() @@ -129,7 +149,7 @@ pub struct HttpClientDefaults { pub type CancelableResponseFuture = Pin>>; -pub trait FetchHandler: Clone { +pub trait FetchHandler: dyn_clone::DynClone { // Return the result of the fetch request consisting of a tuple of the // cancelable response result, the optional fetch body resource and the // optional cancel handle. @@ -143,6 +163,8 @@ pub trait FetchHandler: Clone { ); } +dyn_clone::clone_trait_object!(FetchHandler); + /// A default implementation which will error for every request. #[derive(Clone)] pub struct DefaultFileFetchHandler; @@ -193,14 +215,13 @@ pub struct FetchReturn { cancel_handle_rid: Option, } -pub fn op_fetch( +pub fn op_fetch( state: &mut OpState, args: FetchArgs, data: Option, ) -> Result where FP: FetchPermissions + 'static, - FH: FetchHandler + 'static, { let client = if let Some(rid) = args.client_rid { let r = state.resource_table.get::(rid)?; @@ -230,7 +251,8 @@ where ))); } - let file_fetch_handler = state.borrow_mut::(); + let BoxFetchHandler(file_fetch_handler) = + state.borrow_mut::(); let (request, maybe_request_body, maybe_cancel_handle) = file_fetch_handler.fetch_file(url); let request_rid = state.resource_table.add(FetchRequestResource(request)); diff --git a/runtime/build.rs b/runtime/build.rs index b0af848ba6..14e2e0362c 100644 --- a/runtime/build.rs +++ b/runtime/build.rs @@ -121,15 +121,7 @@ mod not_docs { deno_url::init(), deno_tls::init(), deno_web::init(deno_web::BlobStore::default(), Default::default()), - deno_fetch::init::( - "".to_owned(), - None, - None, - None, - None, - None, - deno_fetch::DefaultFileFetchHandler, // No enable_file_fetch - ), + deno_fetch::init::(Default::default()), deno_websocket::init::("".to_owned(), None, None), deno_webstorage::init(None), deno_crypto::init(None), diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index c7358bc74e..5dc627db41 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -318,15 +318,15 @@ impl WebWorker { deno_console::init(), deno_url::init(), deno_web::init(options.blob_store.clone(), Some(main_module.clone())), - deno_fetch::init::( - options.user_agent.clone(), - options.root_cert_store.clone(), - None, - None, - options.unsafely_ignore_certificate_errors.clone(), - None, - deno_fetch::FsFetchHandler, - ), + deno_fetch::init::(deno_fetch::Options { + user_agent: options.user_agent.clone(), + root_cert_store: options.root_cert_store.clone(), + unsafely_ignore_certificate_errors: options + .unsafely_ignore_certificate_errors + .clone(), + file_fetch_handler: Box::new(deno_fetch::FsFetchHandler), + ..Default::default() + }), deno_websocket::init::( options.user_agent.clone(), options.root_cert_store.clone(), diff --git a/runtime/worker.rs b/runtime/worker.rs index 1588896c8d..facbe397b2 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -101,15 +101,15 @@ impl MainWorker { options.blob_store.clone(), options.bootstrap.location.clone(), ), - deno_fetch::init::( - options.user_agent.clone(), - options.root_cert_store.clone(), - None, - None, - options.unsafely_ignore_certificate_errors.clone(), - None, - deno_fetch::FsFetchHandler, - ), + deno_fetch::init::(deno_fetch::Options { + user_agent: options.user_agent.clone(), + root_cert_store: options.root_cert_store.clone(), + unsafely_ignore_certificate_errors: options + .unsafely_ignore_certificate_errors + .clone(), + file_fetch_handler: Box::new(deno_fetch::FsFetchHandler), + ..Default::default() + }), deno_websocket::init::( options.user_agent.clone(), options.root_cert_store.clone(),