From 6154bc297e798c659dd93ac09dbc283a932dc20c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Burak=20Varl=C4=B1?= <burakvar@amazon.co.uk>
Date: Tue, 25 Oct 2022 13:29:47 +0100
Subject: [PATCH] Add some documentation about moving data back-and-forth
 between Rust and Python

---
 .../src/middleware/mod.rs                     | 67 +++++++++++++++++++
 .../src/middleware/pytests/request.rs         |  2 +-
 .../src/middleware/request.rs                 |  2 +-
 .../src/middleware/response.rs                |  2 +-
 4 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/rust-runtime/aws-smithy-http-server-python/src/middleware/mod.rs b/rust-runtime/aws-smithy-http-server-python/src/middleware/mod.rs
index 61274c8b346..f2d17f5dff5 100644
--- a/rust-runtime/aws-smithy-http-server-python/src/middleware/mod.rs
+++ b/rust-runtime/aws-smithy-http-server-python/src/middleware/mod.rs
@@ -4,6 +4,73 @@
  */
 
 //! Schedule pure-Python middlewares as [tower::Layer]s.
+//!
+//! # Moving data from Rust to Python and back
+//!
+//! In middlewares we need to move some data back-and-forth between Rust and Python.
+//! When you move some data from Rust to Python you can't get its ownership back,
+//! you can only get `&T` or `&mut T` but not `T` unless you clone it.
+//!
+//! In order to overcome this shortcoming we are using wrappers for Python that holds
+//! pure-Rust types with [Option]s and provides `take_inner(&mut self) -> Option<T>`
+//! method to get the ownership of `T` back.
+//!
+//! For example:
+//! ```no_run
+//! # use pyo3::prelude::*;
+//! # use pyo3::exceptions::PyRuntimeError;
+//! # enum PyMiddlewareError {
+//! #     InnerGone
+//! # }
+//! # impl From<PyMiddlewareError> for PyErr {
+//! #     fn from(_: PyMiddlewareError) -> PyErr {
+//! #         PyRuntimeError::new_err("inner gone")
+//! #     }
+//! # }
+//! // Pure Rust type
+//! struct Inner {
+//!     num: i32
+//! }
+//!
+//! // Python wrapper
+//! #[pyclass]
+//! pub struct Wrapper(Option<Inner>);
+//!
+//! impl Wrapper {
+//!     // Call when Python is done processing the `Wrapper`
+//!     // to get ownership of `Inner` back
+//!     pub fn take_inner(&mut self) -> Option<Inner> {
+//!         self.0.take()
+//!     }
+//! }
+//!
+//! // Python exposed methods checks if `Wrapper` still has the `Inner` and
+//! // fails with `InnerGone` otherwise.
+//! #[pymethods]
+//! impl Wrapper {
+//!     #[getter]
+//!     fn num(&self) -> PyResult<i32> {
+//!         self.0
+//!             .as_ref()
+//!             .map(|inner| inner.num)
+//!             .ok_or_else(|| PyMiddlewareError::InnerGone.into())
+//!     }
+//!
+//!     #[setter]
+//!     fn set_num(&mut self, num: i32) -> PyResult<()> {
+//!         match self.0.as_mut() {
+//!             Some(inner) => {
+//!                 inner.num = num;
+//!                 Ok(())
+//!             }
+//!             None => Err(PyMiddlewareError::InnerGone.into()),
+//!         }
+//!     }
+//! }
+//! ```
+//!
+//! You can see this pattern in [PyRequest], [PyResponse] and the others.
+//!
 
 mod error;
 mod handler;
diff --git a/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/request.rs b/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/request.rs
index 804647fe1cd..7a82f41b685 100644
--- a/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/request.rs
+++ b/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/request.rs
@@ -53,7 +53,7 @@ async fn accessing_and_changing_request_body() -> PyResult<()> {
             py,
             r#"
 async def handler(req):
-    # TODO: why we need to wrap with `bytes`?
+    # TODO(Ergonomics): why we need to wrap with `bytes`?
     assert bytes(await req.body) == b"hello world"
 
     req.body = b"hello world from middleware"
diff --git a/rust-runtime/aws-smithy-http-server-python/src/middleware/request.rs b/rust-runtime/aws-smithy-http-server-python/src/middleware/request.rs
index 49ca273402c..d5620e9a342 100644
--- a/rust-runtime/aws-smithy-http-server-python/src/middleware/request.rs
+++ b/rust-runtime/aws-smithy-http-server-python/src/middleware/request.rs
@@ -129,7 +129,7 @@ impl PyRequest {
                 body_guard.replace(Body::from(body));
                 buf
             };
-            // TODO: can we use `PyBytes` here?
+            // TODO(Perf): can we use `PyBytes` here?
             Ok(body.to_vec())
         })
     }
diff --git a/rust-runtime/aws-smithy-http-server-python/src/middleware/response.rs b/rust-runtime/aws-smithy-http-server-python/src/middleware/response.rs
index 6b5a4a02fca..06d209722cd 100644
--- a/rust-runtime/aws-smithy-http-server-python/src/middleware/response.rs
+++ b/rust-runtime/aws-smithy-http-server-python/src/middleware/response.rs
@@ -141,7 +141,7 @@ impl PyResponse {
                 body_guard.replace(to_boxed(body));
                 buf
             };
-            // TODO: can we use `PyBytes` here?
+            // TODO(Perf): can we use `PyBytes` here?
             Ok(body.to_vec())
         })
     }