RPC: Initial implementation of a multiple-argument dispatch

This code uses some kludges (discussed with Ian previously and
hopefully well documented here) to get a type-identifier for each
type in a const context.  It then defines a macro to declare a
type-erased versions of a concrete implementation functions, and
register those implementations to be called later.

We will probably want to tweak a bunch of this code as we move ahead.

Cargo.lock

@@ -270,6 +270,12 @@ dependencies = [
  "tracing-subscriber",
 ]
 
+[[package]]
+name = "assert-impl"
+version = "0.1.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c3464313de0c867016e3e69d7e1e9ae3499bcc4c18e12283d381359ed38b5b9e"
+
 [[package]]
 name = "async-broadcast"
 version = "0.5.1"
@@ -4379,8 +4385,13 @@ dependencies = [
 name = "tor-rpccmd"
 version = "0.0.1"
 dependencies = [
+ "assert-impl",
+ "downcast-rs",
  "futures",
  "futures-await-test",
+ "inventory",
+ "once_cell",
+ "paste",
  "serde",
  "thiserror",
  "typetag",

crates/arti-rpcserver/src/msgs.rs

@@ -148,6 +148,7 @@ mod test {
 
     #[typetag::deserialize(name = "dummy")]
     impl rpc::Command for DummyCmd {}
+    tor_rpccmd::impl_const_type_id!(DummyCmd); // XXXX
 
     #[derive(Serialize)]
     struct DummyResponse {

crates/tor-rpccmd/Cargo.toml

@@ -12,12 +12,16 @@ categories = ["asynchronous"]
 repository = "https://gitlab.torproject.org/tpo/core/arti.git/"
 
 [dependencies]
-#XXXX remove whatever is unneeded here.
+downcast-rs = "1.2.0"
 futures = "0.3.14"
-serde = { version = "1.0.103", features = [ "derive"] }
+futures-await-test = "0.3.0"
+inventory = "0.3.5"
+once_cell = "1"
+paste = "1"
+serde = { version = "1.0.103", features = ["derive"] }
 thiserror = "1"
 typetag = "0.2.7"
 
 [dev-dependencies]
-#XXXX remove whatever is unneeded here.
+assert-impl = "0.1.3"
 futures-await-test = "0.3.0"

crates/tor-rpccmd/src/cmd.rs

@@ -1,3 +1,7 @@
+use downcast_rs::Downcast;
+
+use crate::typeid::GetConstTypeId_;
+
 /// The parameters and method name associated with a given Request.
 ///
 /// We use [`typetag`] here so that we define `Command`s in other crates.
@@ -10,7 +14,6 @@
 // TODO RPC: Possible issue here is that, if this trait is public, anybody outside
 // of Arti can use this trait to add new commands to the RPC engine. Should we
 // care?
-#[typetag::deserialize(tag = "method", content = "data")]
-pub trait Command: std::fmt::Debug + Send {
-    // TODO RPC: this will need some kind of "run this command" trait.
-}
+#[typetag::deserialize(tag = "method", content = "params")]
+pub trait Command: GetConstTypeId_ + std::fmt::Debug + Send + Downcast {}
+downcast_rs::impl_downcast!(Command);

crates/tor-rpccmd/src/dispatch.rs

@@ -0,0 +1,228 @@
+//! A multiple-argument dispatch system for our RPC system.
+//!
+//! Our RPC functionality is polymorphic in Commands (what we're told to do) and
+//! Objects (the things that we give the commands to); we want to be able to
+//! provide different implementations for each command, on each object.
+
+use std::collections::HashMap;
+use std::sync::Arc;
+
+use futures::future::BoxFuture;
+use once_cell::sync::Lazy;
+
+use crate::typeid::ConstTypeId_;
+use crate::{Command, Context, Object};
+
+/// The return type from an RPC function.
+type RpcResult = String; // XXXX Not the actual type we want.
+
+// A boxed future holding the result of an RPC command.
+type RpcResultFuture = BoxFuture<'static, RpcResult>;
+
+/// A type-erased RPC-command invocation function.
+///
+/// This function takes `Arc`s rather than a reference, so that it can return a
+/// `'static` future.
+type ErasedInvokeFn = fn(Arc<dyn Object>, Box<dyn Command>, Arc<dyn Context>) -> RpcResultFuture;
+
+/// An entry for our dynamic dispatch code.
+///
+/// These are generated using [`inventory`] by our `rpc_invoke_fn` macro;
+/// they are later collected into a more efficient data structure.
+#[doc(hidden)]
+pub struct InvokeEntry_ {
+    obj_id: ConstTypeId_,
+    cmd_id: ConstTypeId_,
+    func: ErasedInvokeFn,
+}
+
+// Note that using `inventory` here means that _anybody_ can define new
+// commands!  This may not be the greatest property.
+inventory::collect!(InvokeEntry_);
+
+impl InvokeEntry_ {
+    /// Create a new `InvokeEntry_`.
+    #[doc(hidden)]
+    pub const fn new(obj_id: ConstTypeId_, cmd_id: ConstTypeId_, func: ErasedInvokeFn) -> Self {
+        InvokeEntry_ {
+            obj_id,
+            cmd_id,
+            func,
+        }
+    }
+}
+
+/// Declare an RPC function that will be used to call a single type of [`Command`] on a
+/// single type of [`Object`].
+///
+/// # Example
+///
+/// ```
+/// use tor_rpccmd as rpc;
+///
+/// #[derive(Debug)]
+/// struct ExampleObject {}
+/// impl rpc::Object for ExampleObject {}
+///
+/// #[derive(Debug,serde::Deserialize)]
+/// struct ExampleCommand {}
+/// #[typetag::deserialize]
+/// impl rpc::Command for ExampleCommand {}
+///
+/// // TODO RPC Hide this macro.
+/// rpc::impl_const_type_id!{ExampleObject ExampleCommand}
+///
+/// rpc::rpc_invoke_fn!{
+///     // XXXX wrong return type.
+///     async fn example(obj: ExampleObject, cmd: ExampleCommand, ctx) -> String {
+///         // XXXX In this function, obj is actually an Arc<ExampleObject>,
+///         //      cmd is actually a Box<ExampleCommand>,
+///         //      and ctx is actually an Arc<dyn rpc::Context>!
+///         //     
+///         // XXXX This is quite infelicitous, since the details are hidden.  We should
+///         //      probably make the type information explicit instead.
+///         println!("Running example command!");
+///         "here is your result".into()
+///     }
+/// }
+/// ```
+#[macro_export]
+macro_rules! rpc_invoke_fn {
+    {
+        $(#[$meta:meta])*
+        $v:vis async fn $name:ident($obj:ident : $objtype:ty, $cmd:ident: $cmdtype:ty, $ctx:ident) -> $rtype:ty {
+            $($body:tt)*
+        }
+        $( $($more:tt)+ )?
+    } => {$crate::paste::paste!{
+        // First we declare the function that the user gave us.
+        $(#[$meta])*
+        // XXXX mangling the types here is not good; see comment in example above.
+        $v async fn $name($obj: std::sync::Arc<$objtype>, $cmd: Box<$cmdtype>, $ctx: std::sync::Arc<dyn $crate::Context>) -> $rtype {
+           $($body)*
+        }
+        // Now we declare a type-erased version of the function that takes Arc<dyn> and Box<dyn> arguments, and returns
+        // a boxed future.
+        #[doc(hidden)]
+        fn [<_typeerased_ $name>](obj: std::sync::Arc<dyn $crate::Object>, cmd: Box<dyn $crate::Command>, ctx: std::sync::Arc<dyn $crate::Context>) -> $crate::futures::future::BoxFuture<'static, $rtype> {
+            use $crate::futures::FutureExt;
+            let obj = obj
+                .downcast_arc::<$objtype>()
+                .unwrap_or_else(|_| panic!());
+            let cmd = cmd
+                .downcast::<$cmdtype>()
+                .unwrap_or_else(|_| panic!());
+            $name(obj, cmd, ctx).boxed()
+        }
+        // Finally we use `inventory` to register the type-erased function with
+        // the right types.
+        $crate::inventory::submit!{
+            $crate::dispatch::InvokeEntry_::new(
+                <$objtype as $crate::typeid::HasConstTypeId_>::CONST_TYPE_ID_,
+                <$cmdtype as $crate::typeid::HasConstTypeId_>::CONST_TYPE_ID_,
+                [<_typeerased_ $name >]
+            )
+        }
+
+        $(rpc_invoke_fn!{$($more)+})?
+    }}
+}
+
+/// Actual types to use when looking up a function in our HashMap.
+#[derive(Eq, PartialEq, Clone, Debug, Hash)]
+struct FuncType {
+    obj_id: ConstTypeId_,
+    cmd_id: ConstTypeId_,
+}
+
+/// Table mapping `FuncType` to `ErasedInvokeFn`.
+///
+/// This is constructed once, the first time we use our dispatch code.
+static FUNCTION_TABLE: Lazy<HashMap<FuncType, ErasedInvokeFn>> = Lazy::new(|| {
+    // We want to assert that there are no duplicates, so we can't use "collect"
+    let mut map = HashMap::new();
+    for ent in inventory::iter::<InvokeEntry_>() {
+        let InvokeEntry_ {
+            obj_id,
+            cmd_id,
+            func,
+        } = *ent;
+        let old_val = map.insert(FuncType { obj_id, cmd_id }, func);
+        assert!(
+            old_val.is_none(),
+            "Tried to register two RPC functions with the same type IDs!"
+        );
+    }
+    map
+});
+
+/// An error that occurred while trying to invoke a command on an object.
+#[derive(Debug, thiserror::Error)]
+#[non_exhaustive]
+pub enum InvokeError {
+    /// There is no implementation for the given combination of object
+    /// type and command type.
+    #[error("No implementation for provided object and command types.")]
+    NoImpl,
+}
+
+/// Try to find an appropriate function for calling a given RPC command on a
+/// given RPC-visible object.
+///
+/// On success, return a Future.
+pub fn invoke_command(
+    obj: Arc<dyn Object>,
+    cmd: Box<dyn Command>,
+    ctx: Arc<dyn Context>,
+) -> Result<RpcResultFuture, InvokeError> {
+    let func_type = FuncType {
+        obj_id: obj.const_type_id(),
+        cmd_id: cmd.const_type_id(),
+    };
+
+    let func = FUNCTION_TABLE.get(&func_type).ok_or(InvokeError::NoImpl)?;
+
+    Ok(func(obj, cmd, ctx))
+}
+
+#[cfg(test)]
+mod test {
+    use futures_await_test::async_test;
+
+    pub struct Animal {}
+
+    #[derive(Debug, serde::Deserialize)]
+    pub struct SayHi {}
+    crate::impl_const_type_id!(Animal SayHi);
+    impl crate::Object for Animal {}
+    #[typetag::deserialize]
+    impl crate::Command for SayHi {}
+
+    rpc_invoke_fn! {
+        /// Hello there
+        async fn invoke(_obj: Animal, cmd: SayHi, _ctx) -> String {
+            format!("{:?}", cmd)
+        }
+    }
+
+    struct Ctx {}
+    impl crate::Context for Ctx {
+        fn lookup_object(
+            &self,
+            _id: &crate::ObjectId,
+        ) -> Option<std::sync::Arc<dyn crate::Object>> {
+            todo!()
+        }
+    }
+
+    // TODO RPC: Improve this test!
+    #[async_test]
+    async fn t() {
+        use super::*;
+        let animal: Arc<dyn Object> = Arc::new(Animal {});
+        let hi: Box<dyn Command> = Box::new(SayHi {});
+        let ctx = Arc::new(Ctx {});
+        let s = invoke_command(animal, hi, ctx).unwrap().await;
+        assert_eq!(&s, "SayHi");
+    }
+}

crates/tor-rpccmd/src/lib.rs

@@ -4,7 +4,61 @@
 //! <!-- @@ end lint list maintained by maint/add_warning @@ -->
 
 mod cmd;
+pub mod dispatch;
 mod obj;
+#[doc(hidden)]
+pub mod typeid;
+
+use std::sync::Arc;
 
 pub use cmd::Command;
+pub use dispatch::invoke_command;
 pub use obj::{Object, ObjectId};
+
+#[doc(hidden)]
+pub use {downcast_rs, futures, inventory, paste};
+
+/// An error returned from [`ContextExt::lookup`].
+///
+/// TODO RPC: This type should be made to conform with however we represent RPC
+/// errors.
+#[derive(Debug, Clone, thiserror::Error)]
+#[non_exhaustive]
+pub enum LookupError {
+    /// The specified object does not (currently) exist,
+    /// or the user does not have permission to access it.
+    #[error("No visible object with ID {0:?}")]
+    NoObject(ObjectId),
+
+    /// The specified object exists, but does not have the
+    /// expected type.
+    #[error("Unexpected type on object with ID {0:?}")]
+    WrongType(ObjectId),
+}
+
+/// A trait describing the context in which an RPC command is executed.
+pub trait Context: Send + Sync {
+    /// Look up an object by identity within this context.
+    ///
+    /// A return of `None` may indicate that the object has disappeared,
+    /// that the object doesn't exist,
+    /// that the [`ObjectId`] is ill-formed,
+    /// or that the user has no permission to access the object.
+    fn lookup_object(&self, id: &ObjectId) -> Option<Arc<dyn Object>>;
+}
+
+/// Extension trait for [`Context`].
+///
+/// This is a separate trait so that `Context` can be object-safe.
+pub trait ContextExt: Context {
+    /// Look up an object of a given type, and downcast it.
+    ///
+    /// Return an error if the object can't be found, or has the wrong type.
+    fn lookup<T: Object>(&self, id: &ObjectId) -> Result<Arc<T>, LookupError> {
+        self.lookup_object(id)
+            .ok_or_else(|| LookupError::NoObject(id.clone()))?
+            .downcast_arc()
+            .map_err(|_| LookupError::WrongType(id.clone()))
+    }
+}
+impl<T: Context> ContextExt for T {}

crates/tor-rpccmd/src/obj.rs

@@ -1,6 +1,10 @@
+use downcast_rs::DowncastSync;
 use serde::{Deserialize, Serialize};
 
-pub trait Object {}
+use crate::typeid::GetConstTypeId_;
+
+pub trait Object: GetConstTypeId_ + DowncastSync {}
+downcast_rs::impl_downcast!(sync Object);
 
 /// An identifier for an Object within the context of a Session.
 ///

crates/tor-rpccmd/src/typeid.rs

@@ -0,0 +1,99 @@
+//! A kludgy replacement for [`std::any::TypeId`] that can be used in a constant context.
+
+/// A less helpful variant of `std::any::TypeId` that can be used in a const
+/// context.
+///
+/// Until the [relevant Rust feature] is stabilized, it's not possible to get a
+/// TypeId for a type and store it in a const.  But sadly, we need to do so for
+/// our dispatch code.
+///
+/// Thus, we use a nasty hack: we use the address of the function
+/// `TypeId::of::<T>` as the identifier for the type of T.
+///
+/// This type and the module containing it are hidden: Nobody should actually
+/// use it outside of our dispatch code.  Once we can use `TypeId` instead, we
+/// should and will.
+///
+/// To make a type participate in this system, use the [`impl_const_type_id`]
+/// macro.
+///
+/// **Do not mention this type outside of this module.**
+///
+/// [relevant Rust feature]: https://github.com/rust-lang/rust/issues/77125
+#[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)]
+pub struct ConstTypeId_(
+    /// Sadly this has to be `pub` so we can construct these from other crates.
+    ///
+    /// We could make a constructor, but there is no point.
+    pub *const (),
+);
+
+// Safety: We never actually access the pointer.
+unsafe impl Send for ConstTypeId_ {}
+// Safety: We never actually access the pointer.
+unsafe impl Sync for ConstTypeId_ {}
+
+/// An object for which we can access a [`ConstTypeId_`] dynamically.
+///
+/// **Do not mention this type outside of this module.**
+pub trait GetConstTypeId_ {
+    fn const_type_id(&self) -> ConstTypeId_;
+}
+
+/// An object for which we can get a [`ConstTypeId_`] at compile time.
+///
+/// This is precisely the functionality that [`std::any::TypeId`] doesn't
+/// currently have.
+///
+/// **Do not mention this type outside of this module.**
+pub trait HasConstTypeId_ {
+    const CONST_TYPE_ID_: ConstTypeId_;
+}
+
+/// Implement [`GetConstTypeId_`] and [`HasConstTypeId_`] for one or more types.
+///
+/// To avoid truly unpleasant consequences, this macro only works on simple
+/// identifiers, so you can't run it on arbitrary types, or on types in other
+/// modules.
+#[macro_export]
+macro_rules! impl_const_type_id {
+    { $($type:ident)* } => {
+        $(
+            impl $crate::typeid::HasConstTypeId_ for $type {
+                const CONST_TYPE_ID_: $crate::typeid::ConstTypeId_ = $crate::typeid::ConstTypeId_(
+                    std::any::TypeId::of::<$type> as *const ()
+                );
+            }
+
+            impl $crate::typeid::GetConstTypeId_ for $type {
+                fn const_type_id(&self) -> $crate::typeid::ConstTypeId_ {
+                    <$type as $crate::typeid::HasConstTypeId_>::CONST_TYPE_ID_
+                }
+            }
+        )*
+    }
+}
+pub use impl_const_type_id;
+
+#[cfg(test)]
+mod test {
+    use assert_impl::assert_impl;
+
+    struct Foo(usize);
+    struct Bar {}
+
+    crate::impl_const_type_id! {Foo Bar}
+
+    #[test]
+    fn typeid_basics() {
+        use super::*;
+        assert_impl!(Send: ConstTypeId_);
+        assert_impl!(Sync: ConstTypeId_);
+        let foo1 = Foo(3);
+        let foo2 = Foo(4);
+        let bar = Bar {};
+
+        assert_eq!(foo1.const_type_id(), foo2.const_type_id());
+        assert_ne!(foo1.const_type_id(), bar.const_type_id());
+    }
+}