From 1a764bb8d7bc474589e6a02ff025e86fb2b80999 Mon Sep 17 00:00:00 2001
From: David Pedersen <david.pdrsn@gmail.com>
Date: Sun, 24 Oct 2021 19:49:31 +0200
Subject: [PATCH] Remove `MaybeSharedNode` from `Router` (#403)

In #363 I added `MaybeSharedNode` which makes `Router` faster to clone
_if_ you're using `into_make_service`. However I would like instead like
to find a solution that works even when you're not using
`into_make_service`.

Using an `Arc<RwLock<Node<_>>>` is probably the only solution but would
like to experiment. For now I'm gonna rollback `MaybeSharedNode`.
Changing it wont require user facing changes.
---
 src/routing/mod.rs | 60 ++++++----------------------------------------
 1 file changed, 7 insertions(+), 53 deletions(-)

diff --git a/src/routing/mod.rs b/src/routing/mod.rs
index 991f691b..912c70a5 100644
--- a/src/routing/mod.rs
+++ b/src/routing/mod.rs
@@ -20,7 +20,6 @@ use std::{
     fmt,
     future::ready,
     marker::PhantomData,
-    sync::Arc,
     task::{Context, Poll},
 };
 use tower::{util::ServiceExt, ServiceBuilder};
@@ -50,17 +49,7 @@ impl RouteId {
 #[derive(Clone)]
 pub struct Router<S> {
     routes: S,
-    node: MaybeSharedNode,
-}
-
-// optimization that allows us to only clone the whole `Node` if we're actually
-// mutating it while building the router. Once we've created a `MakeService` we
-// no longer need to add routes and can `Arc` the node making it cheaper to
-// clone
-#[derive(Clone)]
-enum MaybeSharedNode {
-    NotShared(Node<RouteId>),
-    Shared(Arc<Node<RouteId>>),
+    node: Node<RouteId>,
 }
 
 impl Router<EmptyRouter> {
@@ -71,7 +60,7 @@ impl Router<EmptyRouter> {
     pub fn new() -> Self {
         Self {
             routes: EmptyRouter::not_found(),
-            node: MaybeSharedNode::NotShared(Node::new()),
+            node: Node::new(),
         }
     }
 }
@@ -181,7 +170,7 @@ impl<S> Router<S> {
     {
         let id = RouteId::next();
 
-        if let Err(err) = self.update_node(|node| node.insert(path, id)) {
+        if let Err(err) = self.node.insert(path, id) {
             panic!("Invalid route: {}", err);
         }
 
@@ -332,7 +321,7 @@ impl<S> Router<S> {
             format!("{}/*{}", path, NEST_TAIL_PARAM)
         };
 
-        if let Err(err) = self.update_node(|node| node.insert(path, id)) {
+        if let Err(err) = self.node.insert(path, id) {
             panic!("Invalid route: {}", err);
         }
 
@@ -496,7 +485,7 @@ impl<S> Router<S> {
     where
         S: Clone,
     {
-        IntoMakeService::new(self.into_shared_node())
+        IntoMakeService::new(self)
     }
 
     /// Convert this router into a [`MakeService`], that will store `C`'s
@@ -586,7 +575,7 @@ impl<S> Router<S> {
         S: Clone,
         C: Connected<Target>,
     {
-        IntoMakeServiceWithConnectInfo::new(self.into_shared_node())
+        IntoMakeServiceWithConnectInfo::new(self)
     }
 
     /// Merge two routers into one.
@@ -636,41 +625,6 @@ impl<S> Router<S> {
             node: self.node,
         }
     }
-
-    fn update_node<F, T>(&mut self, f: F) -> T
-    where
-        F: FnOnce(&mut Node<RouteId>) -> T,
-    {
-        match &mut self.node {
-            MaybeSharedNode::NotShared(node) => f(node),
-            MaybeSharedNode::Shared(shared_node) => {
-                let mut node: Node<_> = Clone::clone(&*shared_node);
-                let result = f(&mut node);
-                self.node = MaybeSharedNode::NotShared(node);
-                result
-            }
-        }
-    }
-
-    fn get_node(&self) -> &Node<RouteId> {
-        match &self.node {
-            MaybeSharedNode::NotShared(node) => node,
-            MaybeSharedNode::Shared(shared_node) => &*shared_node,
-        }
-    }
-
-    fn into_shared_node(self) -> Self {
-        let node = match self.node {
-            MaybeSharedNode::NotShared(node) => MaybeSharedNode::Shared(Arc::new(node)),
-            MaybeSharedNode::Shared(shared_node) => {
-                MaybeSharedNode::Shared(Arc::clone(&shared_node))
-            }
-        };
-        Self {
-            routes: self.routes,
-            node,
-        }
-    }
 }
 
 impl<ReqBody, ResBody, S> Service<Request<ReqBody>> for Router<S>
@@ -695,7 +649,7 @@ where
         }
 
         let path = req.uri().path().to_string();
-        if let Ok(match_) = self.get_node().at(&path) {
+        if let Ok(match_) = self.node.at(&path) {
             let id = *match_.value;
             req.extensions_mut().insert(id);