Adjust Client::issue to accept request input via reference

The Client::issue method takes ownership of the input parameters to use
for the request to issue. That, however, is actually unnecessary: none
of the internals actually require ownership of this data.
As such, this change adjusts the signature of the method to accept the
request input via a reference, eliminating the need to clone or
otherwise duplicate requests in certain client code.

CHANGELOG.md

@@ -1,6 +1,7 @@
 Unreleased
 ----------
 - Added `ApiInfo::from_parts` constructor
+- Adjusted `Client::issue` to accept request input via reference
 - Introduced `ConversionError` type
   - Replaced `unwrap`s with proper error variants
 - Updated `ActivityType` enum to be in sync with upstream variants

README.md

@@ -48,7 +48,7 @@ let request = order::OrderReqInit {
 .init("AAPL", Side::Buy, 1);
 
 let order = client
-  .issue::<order::Post>(request)
+  .issue::<order::Post>(&request)
   .await
   .unwrap();
 ```

examples/order.rs

@@ -1,4 +1,4 @@
-// Copyright (C) 2020 Daniel Mueller <[email protected]>
+// Copyright (C) 2020-2021 Daniel Mueller <[email protected]>
 // SPDX-License-Identifier: GPL-3.0-or-later
 
 use apca::api::v2::order;
@@ -29,6 +29,6 @@ async fn main() {
   // We want to go long on AAPL, buying a single share.
   .init("AAPL", order::Side::Buy, 1);
 
-  let order = client.issue::<order::Post>(request).await.unwrap();
+  let order = client.issue::<order::Post>(&request).await.unwrap();
   println!("Created order {}", order.id.to_hyphenated_ref());
 }

src/api/v2/account.rs

@@ -225,7 +225,7 @@ mod tests {
   async fn request_account() {
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
-    let account = client.issue::<Get>(()).await.unwrap();
+    let account = client.issue::<Get>(&()).await.unwrap();
 
     assert_eq!(account.currency, "USD");
     assert!(!account.account_blocked);
@@ -242,7 +242,7 @@ mod tests {
   async fn request_account_with_invalid_credentials() {
     let api_info = ApiInfo::from_parts(API_BASE_URL, "invalid", "invalid-too").unwrap();
     let client = Client::new(api_info);
-    let result = client.issue::<Get>(()).await;
+    let result = client.issue::<Get>(&()).await;
 
     let err = result.unwrap_err();
     match err {

src/api/v2/account_activities.rs

@@ -551,7 +551,7 @@ mod tests {
       ],
       ..Default::default()
     };
-    let activities = client.issue::<Get>(request).await.unwrap();
+    let activities = client.issue::<Get>(&request).await.unwrap();
 
     assert!(!activities.is_empty());
 
@@ -578,7 +578,7 @@ mod tests {
       types: vec![ActivityType::Fill],
       ..Default::default()
     };
-    let activities = client.issue::<Get>(request).await.unwrap();
+    let activities = client.issue::<Get>(&request).await.unwrap();
 
     assert!(!activities.is_empty());
 
@@ -600,7 +600,7 @@ mod tests {
       direction: Direction::Ascending,
       ..Default::default()
     };
-    let activities = client.issue::<Get>(request).await.unwrap();
+    let activities = client.issue::<Get>(&request).await.unwrap();
 
     // We don't really have a better way to test this than testing that
     // we parsed something. Note that this may not work for newly
@@ -625,7 +625,7 @@ mod tests {
       page_size: Some(1),
       ..Default::default()
     };
-    let activities = client.issue::<Get>(request.clone()).await.unwrap();
+    let activities = client.issue::<Get>(&request).await.unwrap();
     // We already make the assumption that there are some activities
     // available for us to work with in other tests, so we continue down
     // this road here.
@@ -634,7 +634,7 @@ mod tests {
 
     request.page_token = Some(newest_activity.id().to_string());
 
-    let activities = client.issue::<Get>(request.clone()).await.unwrap();
+    let activities = client.issue::<Get>(&request).await.unwrap();
     assert_eq!(activities.len(), 1);
     let next_activity = &activities[0];
 
@@ -654,7 +654,7 @@ mod tests {
       ..Default::default()
     };
 
-    let activities = client.issue::<Get>(request.clone()).await.unwrap();
+    let activities = client.issue::<Get>(&request).await.unwrap();
     assert_eq!(activities.len(), 1);
 
     let time = activities[0].time();
@@ -669,7 +669,7 @@ mod tests {
 
     // Make another request, this time asking for activities after the
     // first one that was reported.
-    let activities = client.issue::<Get>(request.clone()).await.unwrap();
+    let activities = client.issue::<Get>(&request).await.unwrap();
     assert_eq!(activities.len(), 1);
     assert!(activities[0].time() > time);
   }
@@ -685,13 +685,13 @@ mod tests {
       ..Default::default()
     };
 
-    let activities = client.issue::<Get>(request.clone()).await.unwrap();
+    let activities = client.issue::<Get>(&request).await.unwrap();
     assert_eq!(activities.len(), 2);
 
     let time = activities[1].time();
     request.until = Some(time - Duration::from_micros(1));
 
-    let activities = client.issue::<Get>(request.clone()).await.unwrap();
+    let activities = client.issue::<Get>(&request).await.unwrap();
     assert_eq!(activities.len(), 1);
     assert!(activities[0].time() < time);
   }

src/api/v2/account_config.rs

@@ -115,7 +115,7 @@ mod tests {
   async fn retrieve_and_update_configuration() {
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
-    let config = client.issue::<Get>(()).await.unwrap();
+    let config = client.issue::<Get>(&()).await.unwrap();
 
     // We invert the trade confirmation strategy, which should be a
     // change not affecting any tests running concurrently.
@@ -128,11 +128,11 @@ mod tests {
       trade_confirmation: new_confirmation,
       ..config
     };
-    let patch_result = client.issue::<Patch>(patched).await;
+    let patch_result = client.issue::<Patch>(&patched).await;
     // Also retrieve the configuration again.
-    let get_result = client.issue::<Get>(()).await;
+    let get_result = client.issue::<Get>(&()).await;
     // Revert back to the original setting.
-    let reverted = client.issue::<Patch>(config).await.unwrap();
+    let reverted = client.issue::<Patch>(&config).await.unwrap();
 
     assert_eq!(patch_result.unwrap(), patched);
     assert_eq!(get_result.unwrap(), patched);

src/api/v2/asset.rs

@@ -1,4 +1,4 @@
-// Copyright (C) 2019-2020 Daniel Mueller <[email protected]>
+// Copyright (C) 2019-2021 Daniel Mueller <[email protected]>
 // SPDX-License-Identifier: GPL-3.0-or-later
 
 use std::fmt::Display;
@@ -454,7 +454,7 @@ mod tests {
     async fn test(symbol: Symbol) {
       let api_info = ApiInfo::from_env().unwrap();
       let client = Client::new(api_info);
-      let asset = client.issue::<Get>(symbol).await.unwrap();
+      let asset = client.issue::<Get>(&symbol).await.unwrap();
 
       // The AAPL asset ID, retrieved out-of-band.
       let id = Id(Uuid::parse_str("b0b6dd9d-8b9b-48a9-ba46-b9d54906e415").unwrap());

src/api/v2/assets.rs

@@ -79,7 +79,7 @@ mod tests {
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
     let request = AssetsReqInit::default().init();
-    let assets = client.issue::<Get>(request).await.unwrap();
+    let assets = client.issue::<Get>(&request).await.unwrap();
 
     let asset = assets.iter().find(|x| x.symbol == "AAPL").unwrap();
     assert_eq!(asset.class, Class::UsEquity);

src/api/v2/clock.rs

@@ -1,4 +1,4 @@
-// Copyright (C) 2019-2020 Daniel Mueller <[email protected]>
+// Copyright (C) 2019-2021 Daniel Mueller <[email protected]>
 // SPDX-License-Identifier: GPL-3.0-or-later
 
 use std::time::SystemTime;
@@ -76,7 +76,7 @@ mod tests {
 
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
-    let clock = client.issue::<Get>(()).await.unwrap();
+    let clock = client.issue::<Get>(&()).await.unwrap();
 
     // We want to sanitize the current time being reported at least to a
     // certain degree. For that we assume that our local time is

src/api/v2/events.rs

@@ -198,7 +198,7 @@ mod tests {
     pin_mut!(stream);
 
     let order = order_aapl(&client).await.unwrap();
-    client.issue::<order::Delete>(order.id).await.unwrap();
+    client.issue::<order::Delete>(&order.id).await.unwrap();
 
     let trade = stream
       .try_filter_map(|res| {

src/api/v2/order.rs

@@ -842,8 +842,8 @@ mod tests {
       let api_info = ApiInfo::from_env().unwrap();
       let client = Client::new(api_info);
 
-      let order = client.issue::<Post>(request).await?;
-      client.issue::<Delete>(order.id).await.unwrap();
+      let order = client.issue::<Post>(&request).await?;
+      client.issue::<Delete>(&order.id).await.unwrap();
 
       assert_eq!(order.symbol, "SPY");
       assert_eq!(order.quantity, 1);
@@ -886,8 +886,8 @@ mod tests {
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
 
-    let order = client.issue::<Post>(request).await.unwrap();
-    client.issue::<Delete>(order.id).await.unwrap();
+    let order = client.issue::<Post>(&request).await.unwrap();
+    client.issue::<Delete>(&order.id).await.unwrap();
 
     assert_eq!(order.symbol, "SPY");
     assert_eq!(order.quantity, 1);
@@ -915,8 +915,8 @@ mod tests {
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
 
-    let order = client.issue::<Post>(request).await.unwrap();
-    client.issue::<Delete>(order.id).await.unwrap();
+    let order = client.issue::<Post>(&request).await.unwrap();
+    client.issue::<Delete>(&order.id).await.unwrap();
 
     assert_eq!(order.symbol, "SPY");
     assert_eq!(order.quantity, 1);
@@ -945,11 +945,11 @@ mod tests {
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
 
-    let order = client.issue::<Post>(request).await.unwrap();
-    client.issue::<Delete>(order.id).await.unwrap();
+    let order = client.issue::<Post>(&request).await.unwrap();
+    client.issue::<Delete>(&order.id).await.unwrap();
 
     for leg in &order.legs {
-      client.issue::<Delete>(leg.id).await.unwrap();
+      client.issue::<Delete>(&leg.id).await.unwrap();
     }
 
     assert_eq!(order.symbol, "SPY");
@@ -979,11 +979,11 @@ mod tests {
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
 
-    let order = client.issue::<Post>(request).await.unwrap();
-    client.issue::<Delete>(order.id).await.unwrap();
+    let order = client.issue::<Post>(&request).await.unwrap();
+    client.issue::<Delete>(&order.id).await.unwrap();
 
     for leg in &order.legs {
-      client.issue::<Delete>(leg.id).await.unwrap();
+      client.issue::<Delete>(&leg.id).await.unwrap();
     }
 
     assert_eq!(order.symbol, "SPY");
@@ -1013,9 +1013,9 @@ mod tests {
       }
       .init("AAPL", Side::Buy, 1);
 
-      match client.issue::<Post>(request).await {
+      match client.issue::<Post>(&request).await {
         Ok(order) => {
-          client.issue::<Delete>(order.id).await.unwrap();
+          client.issue::<Delete>(&order.id).await.unwrap();
 
           assert_eq!(order.time_in_force, time_in_force);
         },
@@ -1042,7 +1042,7 @@ mod tests {
     }
     .init("AAPL", Side::Buy, 100000);
 
-    let result = client.issue::<Post>(request).await;
+    let result = client.issue::<Post>(&request).await;
     let err = result.unwrap_err();
 
     match err {
@@ -1056,7 +1056,7 @@ mod tests {
     let id = Id(Uuid::parse_str("00000000-0000-0000-0000-000000000000").unwrap());
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
-    let result = client.issue::<Delete>(id).await;
+    let result = client.issue::<Delete>(&id).await;
     let err = result.unwrap_err();
 
     match err {
@@ -1070,8 +1070,8 @@ mod tests {
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
     let posted = order_aapl(&client).await.unwrap();
-    let result = client.issue::<Get>(posted.id).await;
-    client.issue::<Delete>(posted.id).await.unwrap();
+    let result = client.issue::<Get>(&posted.id).await;
+    client.issue::<Delete>(&posted.id).await.unwrap();
     let gotten = result.unwrap();
 
     // We can't simply compare the two orders for equality, because some
@@ -1091,7 +1091,7 @@ mod tests {
     let id = Id(Uuid::parse_str("00000000-0000-0000-0000-000000000000").unwrap());
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
-    let result = client.issue::<Get>(id).await;
+    let result = client.issue::<Get>(&id).await;
     let err = result.unwrap_err();
 
     match err {
@@ -1113,7 +1113,7 @@ mod tests {
 
     // We are submitting a market order with extended_hours, that is
     // invalid as per the Alpaca documentation.
-    let result = client.issue::<Post>(request).await;
+    let result = client.issue::<Post>(&request).await;
     let err = result.unwrap_err();
 
     match err {
@@ -1133,7 +1133,7 @@ mod tests {
 
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
-    let order = client.issue::<Post>(request).await.unwrap();
+    let order = client.issue::<Post>(&request).await.unwrap();
 
     let request = ChangeReqInit {
       quantity: 2,
@@ -1143,14 +1143,14 @@ mod tests {
     }
     .init();
 
-    let result = client.issue::<Patch>((order.id, request)).await;
+    let result = client.issue::<Patch>(&(order.id, request)).await;
     let id = if let Ok(replaced) = &result {
       replaced.id
     } else {
       order.id
     };
 
-    client.issue::<Delete>(id).await.unwrap();
+    client.issue::<Delete>(&id).await.unwrap();
 
     match result {
       Ok(order) => {
@@ -1181,7 +1181,7 @@ mod tests {
 
     let api_info = ApiInfo::from_env().unwrap();
     let client = Client::new(api_info);
-    let order = client.issue::<Post>(request).await.unwrap();
+    let order = client.issue::<Post>(&request).await.unwrap();
     assert_eq!(order.trail_price, Some(Num::from(20)));
 
     let request = ChangeReqInit {
@@ -1190,14 +1190,14 @@ mod tests {
     }
     .init();
 
-    let result = client.issue::<Patch>((order.id, request)).await;
+    let result = client.issue::<Patch>(&(order.id, request)).await;
     let id = if let Ok(replaced) = &result {
       replaced.id
     } else {
       order.id
     };
 
-    client.issue::<Delete>(id).await.unwrap();
+    client.issue::<Delete>(&id).await.unwrap();
 
     match result {
       Ok(order) => {
@@ -1227,10 +1227,10 @@ mod tests {
     let client = Client::new(api_info);
 
     let (issued, retrieved) = client
-      .issue::<Post>(request.clone())
+      .issue::<Post>(&request)
       .and_then(|order| async {
-        let retrieved = client.issue::<GetByClientId>(client_order_id.clone()).await;
-        client.issue::<Delete>(order.id).await.unwrap();
+        let retrieved = client.issue::<GetByClientId>(&client_order_id).await;
+        client.issue::<Delete>(&order.id).await.unwrap();
         Ok((order, retrieved.unwrap()))
       })
       .await
@@ -1242,7 +1242,7 @@ mod tests {
 
     // We should not be able to submit another order with the same
     // client ID.
-    let err = client.issue::<Post>(request).await.unwrap_err();
+    let err = client.issue::<Post>(&request).await.unwrap_err();
 
     match err {
       RequestError::Endpoint(PostError::InvalidInput(..)) => (),