Refactor dispatcher system

cli/ops/compiler.rs

@@ -1,10 +1,15 @@
 // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
-use super::dispatch_json::{Deserialize, JsonOp, Value};
+use super::dispatch_json::{wrap_json_op, Deserialize, JsonOp};
 use crate::assets;
+use crate::state::DenoOpDispatcher;
 use crate::state::ThreadSafeState;
 use crate::tokio_util;
 use deno::*;
 
+// Cache
+
+pub struct OpCache;
+
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct CacheArgs {
@@ -13,75 +18,118 @@ struct CacheArgs {
   extension: String,
 }
 
-pub fn op_cache(
-  state: &ThreadSafeState,
-  args: Value,
-  _zero_copy: Option<PinnedBuf>,
-) -> Result<JsonOp, ErrBox> {
-  let args: CacheArgs = serde_json::from_value(args)?;
+impl DenoOpDispatcher for OpCache {
+  fn dispatch(
+    &self,
+    state: &ThreadSafeState,
+    control: &[u8],
+    buf: Option<PinnedBuf>,
+  ) -> CoreOp {
+    wrap_json_op(
+      move |args, _zero_copy| {
+        let args: CacheArgs = serde_json::from_value(args)?;
 
-  let module_specifier = ModuleSpecifier::resolve_url(&args.module_id)
-    .expect("Should be valid module specifier");
+        let module_specifier = ModuleSpecifier::resolve_url(&args.module_id)
+          .expect("Should be valid module specifier");
 
-  state.ts_compiler.cache_compiler_output(
-    &module_specifier,
-    &args.extension,
-    &args.contents,
-  )?;
+        state.ts_compiler.cache_compiler_output(
+          &module_specifier,
+          &args.extension,
+          &args.contents,
+        )?;
+
+        Ok(JsonOp::Sync(json!({})))
+      },
+      control,
+      buf,
+    )
+  }
 
-  Ok(JsonOp::Sync(json!({})))
+  const NAME: &'static str = "cache";
 }
 
+// Fetch Source File
+
+pub struct OpFetchSourceFile;
+
 #[derive(Deserialize)]
 struct FetchSourceFileArgs {
   specifier: String,
   referrer: String,
 }
 
-pub fn op_fetch_source_file(
-  state: &ThreadSafeState,
-  args: Value,
-  _zero_copy: Option<PinnedBuf>,
-) -> Result<JsonOp, ErrBox> {
-  let args: FetchSourceFileArgs = serde_json::from_value(args)?;
-
-  // TODO(ry) Maybe a security hole. Only the compiler worker should have access
-  // to this. Need a test to demonstrate the hole.
-  let is_dyn_import = false;
-
-  let resolved_specifier =
-    state.resolve(&args.specifier, &args.referrer, false, is_dyn_import)?;
-
-  let fut = state
-    .file_fetcher
-    .fetch_source_file_async(&resolved_specifier);
-
-  // WARNING: Here we use tokio_util::block_on() which starts a new Tokio
-  // runtime for executing the future. This is so we don't inadvernently run
-  // out of threads in the main runtime.
-  let out = tokio_util::block_on(fut)?;
-  Ok(JsonOp::Sync(json!({
-    "moduleName": out.url.to_string(),
-    "filename": out.filename.to_str().unwrap(),
-    "mediaType": out.media_type as i32,
-    "sourceCode": String::from_utf8(out.source_code).unwrap(),
-  })))
+impl DenoOpDispatcher for OpFetchSourceFile {
+  fn dispatch(
+    &self,
+    state: &ThreadSafeState,
+    control: &[u8],
+    buf: Option<PinnedBuf>,
+  ) -> CoreOp {
+    wrap_json_op(
+      move |args, _zero_copy| {
+        let args: FetchSourceFileArgs = serde_json::from_value(args)?;
+
+        // TODO(ry) Maybe a security hole. Only the compiler worker should have access
+        // to this. Need a test to demonstrate the hole.
+        let is_dyn_import = false;
+
+        let resolved_specifier = state.resolve(
+          &args.specifier,
+          &args.referrer,
+          false,
+          is_dyn_import,
+        )?;
+
+        let fut = state
+          .file_fetcher
+          .fetch_source_file_async(&resolved_specifier);
+
+        // WARNING: Here we use tokio_util::block_on() which starts a new Tokio
+        // runtime for executing the future. This is so we don't inadvernently run
+        // out of threads in the main runtime.
+        let out = tokio_util::block_on(fut)?;
+        Ok(JsonOp::Sync(json!({
+          "moduleName": out.url.to_string(),
+          "filename": out.filename.to_str().unwrap(),
+          "mediaType": out.media_type as i32,
+          "sourceCode": String::from_utf8(out.source_code).unwrap(),
+        })))
+      },
+      control,
+      buf,
+    )
+  }
+
+  const NAME: &'static str = "fetchSourceFile";
 }
 
+pub struct OpFetchAsset;
+
 #[derive(Deserialize)]
 struct FetchAssetArgs {
   name: String,
 }
 
-pub fn op_fetch_asset(
-  _state: &ThreadSafeState,
-  args: Value,
-  _zero_copy: Option<PinnedBuf>,
-) -> Result<JsonOp, ErrBox> {
-  let args: FetchAssetArgs = serde_json::from_value(args)?;
-  if let Some(source_code) = assets::get_source_code(&args.name) {
-    Ok(JsonOp::Sync(json!(source_code)))
-  } else {
-    panic!("op_fetch_asset bad asset {}", args.name)
+impl DenoOpDispatcher for OpFetchAsset {
+  fn dispatch(
+    &self,
+    _state: &ThreadSafeState,
+    control: &[u8],
+    buf: Option<PinnedBuf>,
+  ) -> CoreOp {
+    wrap_json_op(
+      move |args, _zero_copy| {
+        let args: FetchAssetArgs = serde_json::from_value(args)?;
+        if let Some(source_code) = assets::get_source_code(&args.name) {
+          Ok(JsonOp::Sync(json!(source_code)))
+        } else {
+          panic!("op_fetch_asset bad asset {}", args.name)
+        }
+      },
+      control,
+      buf,
+    )
   }
+
+  const NAME: &'static str = "fetchAsset";
 }

cli/ops/dispatch_json.rs

@@ -1,5 +1,4 @@
 // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
-use crate::state::ThreadSafeState;
 use crate::tokio_util;
 use deno::*;
 use futures::Future;
@@ -23,12 +22,6 @@ fn json_err(err: ErrBox) -> Value {
   })
 }
 
-pub type Dispatcher = fn(
-  state: &ThreadSafeState,
-  args: Value,
-  zero_copy: Option<PinnedBuf>,
-) -> Result<JsonOp, ErrBox>;
-
 fn serialize_result(
   promise_id: Option<u64>,
   result: Result<Value, ErrBox>,
@@ -50,19 +43,22 @@ struct AsyncArgs {
   promise_id: Option<u64>,
 }
 
-pub fn dispatch(
-  d: Dispatcher,
-  state: &ThreadSafeState,
+pub fn wrap_json_op<D>(
+  d: D,
   control: &[u8],
   zero_copy: Option<PinnedBuf>,
-) -> CoreOp {
+) -> CoreOp
+where
+  D: FnOnce(Value, Option<PinnedBuf>) -> Result<JsonOp, ErrBox>,
+{
   let async_args: AsyncArgs = serde_json::from_slice(control).unwrap();
   let promise_id = async_args.promise_id;
   let is_sync = promise_id.is_none();
 
   let result = serde_json::from_slice(control)
     .map_err(ErrBox::from)
-    .and_then(move |args| d(state, args, zero_copy));
+    .and_then(move |args| d(args, zero_copy));
+
   match result {
     Ok(JsonOp::Sync(sync_value)) => {
       assert!(promise_id.is_none());

cli/ops/dispatch_minimal.rs

@@ -4,7 +4,6 @@
 //! alternative to flatbuffers using a very simple list of int32s to lay out
 //! messages. The first i32 is used to determine if a message a flatbuffer
 //! message or a "minimal" message.
-use crate::state::ThreadSafeState;
 use deno::Buf;
 use deno::CoreOp;
 use deno::ErrBox;
@@ -13,7 +12,6 @@ use deno::PinnedBuf;
 use futures::Future;
 
 pub type MinimalOp = dyn Future<Item = i32, Error = ErrBox> + Send;
-pub type Dispatcher = fn(i32, Option<PinnedBuf>) -> Box<MinimalOp>;
 
 #[derive(Copy, Clone, Debug, PartialEq)]
 // This corresponds to RecordMinimal on the TS side.
@@ -72,12 +70,14 @@ fn test_parse_min_record() {
   assert_eq!(parse_min_record(&buf), None);
 }
 
-pub fn dispatch(
-  d: Dispatcher,
-  _state: &ThreadSafeState,
+pub fn wrap_minimal_op<D>(
+  d: D,
   control: &[u8],
   zero_copy: Option<PinnedBuf>,
-) -> CoreOp {
+) -> CoreOp
+where
+  D: FnOnce(i32, Option<PinnedBuf>) -> Box<MinimalOp>,
+{
   let mut record = parse_min_record(control).unwrap();
   let is_sync = record.promise_id == 0;
   let rid = record.arg;

cli/ops/errors.rs

@@ -1,56 +1,87 @@
 // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
-use super::dispatch_json::{Deserialize, JsonOp, Value};
+use super::dispatch_json::{wrap_json_op, Deserialize, JsonOp};
 use crate::fmt_errors::JSError;
 use crate::source_maps::get_orig_position;
 use crate::source_maps::CachedMaps;
+use crate::state::DenoOpDispatcher;
 use crate::state::ThreadSafeState;
 use deno::*;
 use std::collections::HashMap;
 
+// Format Error
+
+pub struct OpFormatError;
+
 #[derive(Deserialize)]
 struct FormatErrorArgs {
   error: String,
 }
 
-pub fn op_format_error(
-  state: &ThreadSafeState,
-  args: Value,
-  _zero_copy: Option<PinnedBuf>,
-) -> Result<JsonOp, ErrBox> {
-  let args: FormatErrorArgs = serde_json::from_value(args)?;
-  let error = JSError::from_json(&args.error, &state.ts_compiler);
-
-  Ok(JsonOp::Sync(json!({
-    "error": error.to_string(),
-  })))
+impl DenoOpDispatcher for OpFormatError {
+  fn dispatch(
+    &self,
+    state: &ThreadSafeState,
+    control: &[u8],
+    buf: Option<PinnedBuf>,
+  ) -> CoreOp {
+    wrap_json_op(
+      move |args, _zero_copy| {
+        let args: FormatErrorArgs = serde_json::from_value(args)?;
+        let error = JSError::from_json(&args.error, &state.ts_compiler);
+
+        Ok(JsonOp::Sync(json!({
+          "error": error.to_string(),
+        })))
+      },
+      control,
+      buf,
+    )
+  }
+
+  const NAME: &'static str = "formatError";
 }
 
+// Apply Source Map
+
+pub struct OpApplySourceMap;
+
 #[derive(Deserialize)]
-struct ApplySourceMap {
+struct ApplySourceMapArgs {
   filename: String,
   line: i32,
   column: i32,
 }
 
-pub fn op_apply_source_map(
-  state: &ThreadSafeState,
-  args: Value,
-  _zero_copy: Option<PinnedBuf>,
-) -> Result<JsonOp, ErrBox> {
-  let args: ApplySourceMap = serde_json::from_value(args)?;
-
-  let mut mappings_map: CachedMaps = HashMap::new();
-  let (orig_filename, orig_line, orig_column) = get_orig_position(
-    args.filename,
-    args.line.into(),
-    args.column.into(),
-    &mut mappings_map,
-    &state.ts_compiler,
-  );
-
-  Ok(JsonOp::Sync(json!({
-    "filename": orig_filename.to_string(),
-    "line": orig_line as u32,
-    "column": orig_column as u32,
-  })))
+impl DenoOpDispatcher for OpApplySourceMap {
+  fn dispatch(
+    &self,
+    state: &ThreadSafeState,
+    control: &[u8],
+    buf: Option<PinnedBuf>,
+  ) -> CoreOp {
+    wrap_json_op(
+      move |args, _zero_copy| {
+        let args: ApplySourceMapArgs = serde_json::from_value(args)?;
+
+        let mut mappings_map: CachedMaps = HashMap::new();
+        let (orig_filename, orig_line, orig_column) = get_orig_position(
+          args.filename,
+          args.line.into(),
+          args.column.into(),
+          &mut mappings_map,
+          &state.ts_compiler,
+        );
+
+        Ok(JsonOp::Sync(json!({
+          "filename": orig_filename.to_string(),
+          "line": orig_line as u32,
+          "column": orig_column as u32,
+        })))
+      },
+      control,
+      buf,
+    )
+  }
+
+  const NAME: &'static str = "applySourceMap";
 }