Addressing review comments

- Remove root_context_id from create_http_stream and create_http_context calls - Move EmptyHttpContext after HttpContext declaration - Roll up all Dispatcher.create_* calls into on_create_context Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
proxy-wasm · Nov 16, 2020 · f22f308 · f22f308
1 parent fa8512a
commit f22f308
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 64 deletions.
diff --git a/examples/http_auth_random.rs b/examples/http_auth_random.rs
@@ -31,7 +31,7 @@ impl RootContext for HttpAuthRandomRoot {
         ContextType::HttpContext
     }
 
-    fn create_http_context(&self, _root_context_id: u32, _context_id: u32) -> Box<dyn HttpContext> {
+    fn create_http_context(&self, _context_id: u32) -> Box<dyn HttpContext> {
         Box::new(HttpAuthRandom)
     }
 }

diff --git a/examples/http_body.rs b/examples/http_body.rs
@@ -29,7 +29,7 @@ impl RootContext for HttpBodyRoot {
         ContextType::HttpContext
     }
 
-    fn create_http_context(&self, _context_id: u32, _root_context_id: u32) -> Box<dyn HttpContext> {
+    fn create_http_context(&self, _context_id: u32) -> Box<dyn HttpContext> {
         Box::new(HttpBody)
     }
 }

diff --git a/examples/http_headers.rs b/examples/http_headers.rs
@@ -31,7 +31,7 @@ impl RootContext for HttpHeadersRoot {
         ContextType::HttpContext
     }
 
-    fn create_http_context(&self, _context_id: u32, _root_context_id: u32) -> Box<dyn HttpContext> {
+    fn create_http_context(&self, _context_id: u32) -> Box<dyn HttpContext> {
         Box::new(HttpHeaders {
             context_id: _context_id,
         })

diff --git a/src/dispatcher.rs b/src/dispatcher.rs
@@ -60,51 +60,6 @@ impl Dispatcher {
         self.new_root.set(Some(callback));
     }
 
-    fn create_root_context(&self, context_id: u32) {
-        let new_context = match self.new_root.get() {
-            Some(f) => f(context_id),
-            None => Box::new(NoopRoot),
-        };
-        if self
-            .roots
-            .borrow_mut()
-            .insert(context_id, new_context)
-            .is_some()
-        {
-            panic!("duplicate context_id")
-        }
-    }
-
-    fn create_stream_context(&self, context_id: u32, root_context_id: u32) {
-        let new_context = match self.roots.borrow().get(&root_context_id) {
-            Some(root_context) => root_context.create_stream_context(context_id, root_context_id),
-            None => panic!("invalid root_context_id"),
-        };
-        if self
-            .streams
-            .borrow_mut()
-            .insert(context_id, new_context)
-            .is_some()
-        {
-            panic!("duplicate context_id")
-        }
-    }
-
-    fn create_http_context(&self, context_id: u32, root_context_id: u32) {
-        let new_context = match self.roots.borrow().get(&root_context_id) {
-            Some(root_context) => root_context.create_http_context(context_id, root_context_id),
-            None => panic!("invalid root_context_id"),
-        };
-        if self
-            .http_streams
-            .borrow_mut()
-            .insert(context_id, new_context)
-            .is_some()
-        {
-            panic!("duplicate context_id")
-        }
-    }
-
     fn register_callout(&self, token_id: u32) {
         if self
             .callouts
@@ -118,17 +73,46 @@ impl Dispatcher {
 
     fn on_create_context(&self, context_id: u32, root_context_id: u32) {
         if root_context_id == 0 {
-            self.create_root_context(context_id);
+            let root_context = match self.new_root.get() {
+                Some(f) => f(context_id),
+                None => Box::new(NoopRoot),
+            };
+            if self
+                .roots
+                .borrow_mut()
+                .insert(context_id, root_context)
+                .is_some()
+            {
+                panic!("duplicate context_id")
+            }
         } else if let Some(root_context) = self.roots.borrow().get(&root_context_id) {
             match root_context.get_type() {
-                ContextType::HttpContext => self.create_http_context(context_id, root_context_id),
+                ContextType::HttpContext => {
+                    let http_context = root_context.create_http_context(context_id);
+                    if self
+                        .http_streams
+                        .borrow_mut()
+                        .insert(context_id, http_context)
+                        .is_some()
+                    {
+                        panic!("duplicate context_id")
+                    }
+                }
                 ContextType::StreamContext => {
-                    self.create_stream_context(context_id, root_context_id)
+                    let stream_context = root_context.create_stream_context(context_id);
+                    if self
+                        .streams
+                        .borrow_mut()
+                        .insert(context_id, stream_context)
+                        .is_some()
+                    {
+                        panic!("duplicate context_id")
+                    }
                 }
-                ContextType::RootContext => panic!("missing constructors"),
+                ContextType::RootContext => panic!("missing ContextType on root_context"),
             }
         } else {
-            panic!("missing constructors")
+            panic!("invalid root_context_id");
         }
     }
 

diff --git a/src/traits.rs b/src/traits.rs
@@ -122,15 +122,11 @@ pub trait RootContext: Context {
 
     fn on_log(&mut self) {}
 
-    fn create_http_context(&self, _context_id: u32, _root_context_id: u32) -> Box<dyn HttpContext> {
+    fn create_http_context(&self, _context_id: u32) -> Box<dyn HttpContext> {
         Box::new(EmptyHttpContext)
     }
 
-    fn create_stream_context(
-        &self,
-        _context_id: u32,
-        _root_context_id: u32,
-    ) -> Box<dyn StreamContext> {
+    fn create_stream_context(&self, _context_id: u32) -> Box<dyn StreamContext> {
         Box::new(EmptyStreamContext)
     }
 
@@ -175,11 +171,6 @@ pub trait StreamContext: Context {
     fn on_log(&mut self) {}
 }
 
-struct EmptyHttpContext;
-
-impl HttpContext for EmptyHttpContext {}
-impl Context for EmptyHttpContext {}
-
 struct EmptyStreamContext;
 
 impl StreamContext for EmptyStreamContext {}
@@ -329,3 +320,8 @@ pub trait HttpContext: Context {
 
     fn on_log(&mut self) {}
 }
+
+struct EmptyHttpContext;
+
+impl HttpContext for EmptyHttpContext {}
+impl Context for EmptyHttpContext {}