Repository: redbadger/crux (language: rust)

Issue:
Implement async API in crux_http
See #179 for context.

This _should_ hopefully be a backwards compatible refactoring


Discussion / hints:

Fix patch:
diff --git a/crux_http/src/request_builder.rs b/crux_http/src/request_builder.rs
index 47d60190..fb363343 100644
--- a/crux_http/src/request_builder.rs
+++ b/crux_http/src/request_builder.rs
@@ -291,7 +291,9 @@ where
     }
 
     /// Decode a String from the response body prior to dispatching it to the apps `update`
-    /// function
+    /// function.
+    ///
+    /// This has no effect when used with the [async API](RequestBuilder::send_async).
     ///
     /// # Examples
     ///
@@ -317,7 +319,9 @@ where
     }
 
     /// Decode a `T` from a JSON response body prior to dispatching it to the apps `update`
-    /// function
+    /// function.
+    ///
+    /// This has no effect when used with the [async API](RequestBuilder::send_async).
     ///
     /// # Examples
     ///
@@ -391,20 +395,35 @@ where
                 .update_app(make_event(self.expectation.decode(resp)));
         });
     }
+
+    /// Sends the constructed `Request` and returns a future that resolves to [`ResponseAsync`].
+    /// but does not consume it or convert the body to an expected format.
+    ///
+    /// Note that this is equivalent to calling `.into_future()` on the `RequestBuilder`, which
+    /// will happen implicitly when calling `.await` on the builder, which does implement
+    /// [`IntoFuture`](std::future::IntoFuture). Calling `.await` on the builder is recommended.
+    ///
+    /// Not all code working with futures (such as the `join` macro) works with `IntoFuture` (yet?), so this
+    /// method is provided as a more discoverable `.into_future` alias, and may be deprecated later.
+    pub fn send_async(self) -> BoxFuture<'static, Result<ResponseAsync>> {
+        <Self as std::future::IntoFuture>::into_future(self)
+    }
 }
 
-impl std::future::IntoFuture for RequestBuilder<()> {
+impl<T, Eb> std::future::IntoFuture for RequestBuilder<T, Eb> {
     type Output = Result<ResponseAsync>;
 
     type IntoFuture = BoxFuture<'static, Result<ResponseAsync>>;
 
+    /// Sends the constructed `Request` and returns a future that resolves to the response
     fn into_future(self) -> Self::IntoFuture {
-        Box::pin(async move {
-            let CapOrClient::Client(client) = self.cap_or_client else {
-                panic!("Tried to await a RequestBuilder in a non-middleware context");
+        Box::pin({
+            let client = match self.cap_or_client {
+                CapOrClient::Client(c) => c,
+                CapOrClient::Capability(c) => c.client,
             };
 
-            client.send(self.req.unwrap()).await
+            async move { client.send(self.req.unwrap()).await }
         })
     }
 }
diff --git a/crux_http/src/response/response_async.rs b/crux_http/src/response/response_async.rs
index 1df03651..56bdd1aa 100644
--- a/crux_http/src/response/response_async.rs
+++ b/crux_http/src/response/response_async.rs
@@ -16,10 +16,8 @@ use std::task::{Context, Poll};
 use super::decode::decode_body;
 
 pin_project_lite::pin_project! {
-    /// An HTTP response that exposes async methods, for use inside middleware.
-    ///
-    /// If you're not writing middleware you'll never need to interact with
-    /// this type and can probably ignore it.
+    /// An HTTP response that exposes async methods. This is to support async
+    /// use and middleware.
     pub struct ResponseAsync {
         #[pin]
         res: crate::http::Response,


Test patch:
diff --git a/crux_http/tests/with_tester.rs b/crux_http/tests/with_tester.rs
index 8fbc0b35..7901e174 100644
--- a/crux_http/tests/with_tester.rs
+++ b/crux_http/tests/with_tester.rs
@@ -1,7 +1,12 @@
 mod shared {
 
+    use std::{cmp::max, future::IntoFuture};
+
+    use crux_core::compose::Compose;
     use crux_http::Http;
     use crux_macros::Effect;
+    use futures_util::join;
+    use http_types::StatusCode;
     use serde::{Deserialize, Serialize};
 
     #[derive(Default)]
@@ -11,6 +16,9 @@ mod shared {
     pub enum Event {
         Get,
         Post,
+        GetPostChain,
+        ConcurrentGets,
+        ComposeComplete(StatusCode),
 
         // events local to the core
         Set(crux_http::Result<crux_http::Response<String>>),
@@ -50,6 +58,53 @@ mod shared {
                         .expect_string()
                         .send(Event::Set);
                 }
+                Event::GetPostChain => caps.compose.spawn(|context| {
+                    let caps = caps.clone();
+
+                    async move {
+                        let mut response = caps
+                            .http
+                            .get("http://example.com")
+                            .await
+                            .expect("Send async should succeed");
+                        let text = response
+                            .body_string()
+                            .await
+                            .expect("response should have body");
+
+                        let response = caps
+                            .http
+                            .post(format!("http://example.com/{}", text))
+                            .await
+                            .expect("Send async should succeed");
+
+                        context.update_app(Event::ComposeComplete(response.status()))
+                    }
+                }),
+                Event::ConcurrentGets => caps.compose.spawn(|ctx| {
+                    let caps = caps.clone();
+
+                    async move {
+                        let one = caps.http.get("http://example.com/one").into_future();
+                        let two = caps.http.get("http://example.com/two").send_async();
+
+                        let (response_one, response_two) = join!(one, two);
+
+                        let one = response_one.expect("Send async should succeed");
+                        let two = response_two.expect("Send async should succeed");
+
+                        let status = StatusCode::try_from(max::<u16>(
+                            one.status().into(),
+                            two.status().into(),
+                        ))
+                        .unwrap();
+
+                        ctx.update_app(Event::ComposeComplete(status))
+                    }
+                }),
+                Event::ComposeComplete(status) => {
+                    model.values.push(status.to_string());
+                }
                 Event::Set(Ok(mut response)) => {
                     model.body = response.take_body().unwrap();
                     model.values = response
@@ -70,9 +125,11 @@ mod shared {
         }
     }
 
-    #[derive(Effect)]
+    #[derive(Effect, Clone)]
     pub(crate) struct Capabilities {
         pub http: Http<Event>,
+        #[effect(skip)]
+        pub compose: Compose<Event>,
     }
 }
 
@@ -152,4 +209,86 @@ mod tests {
             assert_eq!(*response.body().unwrap(), "\"The Body\"".to_string());
         });
     }
+
+    #[test]
+    fn get_post_chain() {
+        let app = AppTester::<App, _>::default();
+        let mut model = Model::default();
+
+        let mut update = app.update(Event::GetPostChain, &mut model);
+
+        let Effect::Http(request) = update.effects_mut().next().unwrap();
+        let http_request = &request.operation;
+
+        assert_eq!(
+            *http_request,
+            HttpRequest::get("http://example.com/").build()
+        );
+
+        let mut update = app
+            .resolve(request, HttpResponse::ok().body("secret_place").build())
+            .expect("Resolves successfully");
+
+        assert!(update.events.is_empty());
+
+        let Effect::Http(request) = update.effects_mut().next().unwrap();
+        let http_request = &request.operation;
+
+        assert_eq!(
+            *http_request,
+            HttpRequest::post("http://example.com/secret_place").build()
+        );
+
+        let update = app
+            .resolve(request, HttpResponse::status(201).build())
+            .expect("Resolves successfully");
+
+        let actual = update.events.clone();
+        assert_matches!(&actual[..], [Event::ComposeComplete(status)] => {
+            assert_eq!(*status, 201);
+        });
+    }
+
+    #[test]
+    fn concurrent_gets() {
+        let app = AppTester::<App, _>::default();
+        let mut model = Model::default();
+
+        let mut update = app.update(Event::ConcurrentGets, &mut model);
+        let mut effects = update.effects_mut();
+
+        let Effect::Http(request_one) = effects.next().unwrap();
+        let http_request = &request_one.operation;
+
+        assert_eq!(
+            *http_request,
+            HttpRequest::get("http://example.com/one").build()
+        );
+
+        let Effect::Http(request_two) = effects.next().unwrap();
+        let http_request = &request_two.operation;
+
+        assert_eq!(
+            *http_request,
+            HttpRequest::get("http://example.com/two").build()
+        );
+
+        // Resolve second request first, should not matter
+        let update = app
+            .resolve(request_two, HttpResponse::ok().body("one").build())
+            .expect("Resolves successfully");
+
+        // Nothing happens yet
+        assert!(update.effects.is_empty());
+        assert!(update.events.is_empty());
+
+        let update = app
+            .resolve(request_one, HttpResponse::ok().body("one").build())
+            .expect("Resolves successfully");
+
+        let actual = update.events.clone();
+        assert_matches!(&actual[..], [Event::ComposeComplete(status)] => {
+            assert_eq!(*status, 200);
+        });
+    }
 }