References on all the things

[Emilgardis]

Started work on making everything use references instead of allocating on multiple places

https://github.com/twitch-rs/twitch_api/tree/references

This solution should work, but I'm not sure the approach is good.

I'd want requests to be able to take references, and responses return them too.

Suggestions for how to avoid unnecessary allocations in responses hugely appreciated!

[Emilgardis]

https://docs.rs/aliri_braid seems like a good way to solve this!

[Nerixyz]

I tried to "convert" the GetUsersRequest as a POC to use reference types (see diffs below). Using references in the struct has the consequence, that we can't pass a "static"/immediate reference to a slice, but need to create a variable for the array first:

let req = GetUsersRequest::builder().id(&["44322889".into()]).build();

This will result in the following error:

error[E0716]: temporary value dropped while borrowed
   --> src\helix\endpoints\users\get_users.rs:112:46
    |
112 |     let req = GetUsersRequest::builder().id(&["44322889".into()]).build();
    |                                              ^^^^^^^^^^^^^^^^^^^         - temporary value is freed at the end of this statement
    |                                              |
    |                                              creates a temporary which is freed while still in use
...
139 |     let uri = req.get_uri().unwrap();
    |               ------------- borrow later used here
    |
help: consider using a `let` binding to create a longer lived value
    |
112 ~     let binding = ["44322889".into()];
113 ~     let req = GetUsersRequest::builder().id(&binding).build();
    |

Fix:

let ids = ["44322889".into()];
let req = GetUsersRequest::builder().id(&ids).build();

Diffs:

helix/endpoints/users/get_users.rs

diff --git a/src/helix/endpoints/users/get_users.rs b/src/helix/endpoints/users/get_users.rs
index 49333d8d3..0f5789471 100644
--- a/src/helix/endpoints/users/get_users.rs
+++ b/src/helix/endpoints/users/get_users.rs
@@ -25,9 +25,11 @@
 //! # let client: helix::HelixClient<'static, client::DummyHttpClient> = helix::HelixClient::default();
 //! # let token = twitch_oauth2::AccessToken::new("validtoken".to_string());
 //! # let token = twitch_oauth2::UserToken::from_existing(&client, token, None, None).await?;
+//! let user_ids = ["1234".into()];
+//! let usernames = ["justintvfan".into()];
 //! let request = get_users::GetUsersRequest::builder()
-//!     .id(vec!["1234".into()])
-//!     .login(vec!["justintvfan".into()])
+//!     .id(&user_ids)
+//!     .login(&usernames)
 //!     .build();
 //! let response: Vec<get_users::User> = client.req_get(request, &token).await?.data;
 //! # Ok(())
@@ -43,15 +45,15 @@ use helix::RequestGet;
 /// Query Parameters for [Get Users](super::get_users)
 ///
 /// [`get-users`](https://dev.twitch.tv/docs/api/reference#get-users)
-#[derive(PartialEq, typed_builder::TypedBuilder, Deserialize, Serialize, Clone, Debug)]
+#[derive(PartialEq, typed_builder::TypedBuilder, Serialize, Clone, Debug)]
 #[non_exhaustive]
-pub struct GetUsersRequest {
+pub struct GetUsersRequest<'a> {
     /// User ID. Multiple user IDs can be specified. Limit: 100.
     #[builder(default)]
-    pub id: Vec<types::UserId>,
+    pub id: &'a [&'a types::UserIdRef],
     /// User login name. Multiple login names can be specified. Limit: 100.
     #[builder(default)]
-    pub login: Vec<types::UserName>,
+    pub login: &'a [&'a types::UserNameRef],
 }
 
 /// Return Values for [Get Users](super::get_users)
@@ -91,7 +93,7 @@ pub struct User {
     pub view_count: usize,
 }
 
-impl Request for GetUsersRequest {
+impl<'a> Request for GetUsersRequest<'a> {
     type Response = Vec<User>;
 
     #[cfg(feature = "twitch_oauth2")]
@@ -101,15 +103,14 @@ impl Request for GetUsersRequest {
     const SCOPE: &'static [twitch_oauth2::Scope] = &[];
 }
 
-impl RequestGet for GetUsersRequest {}
+impl<'a> RequestGet for GetUsersRequest<'a> {}
 
 #[cfg(test)]
 #[test]
 fn test_request() {
     use helix::*;
-    let req = GetUsersRequest::builder()
-        .id(vec!["44322889".into()])
-        .build();
+    let ids = ["44322889".into()];
+    let req = GetUsersRequest::builder().id(&ids).build();
 
     // From twitch docs
     // FIXME: This is not valid anymore. Twitch....

helix/client/client_ext.rs

diff --git a/src/helix/client/client_ext.rs b/src/helix/client/client_ext.rs
index 4d70b442c..4d963b6d1 100644
--- a/src/helix/client/client_ext.rs
+++ b/src/helix/client/client_ext.rs
@@ -12,7 +12,7 @@ impl<'a, C: crate::HttpClient<'a> + Sync> HelixClient<'a, C> {
     /// Get [User](helix::users::User) from user login
     pub async fn get_user_from_login<T>(
         &'a self,
-        login: impl Into<types::UserName>,
+        login: impl AsRef<types::UserNameRef>,
         token: &T,
     ) -> Result<Option<helix::users::User>, ClientError<'a, C>>
     where
@@ -20,7 +20,7 @@ impl<'a, C: crate::HttpClient<'a> + Sync> HelixClient<'a, C> {
     {
         self.req_get(
             helix::users::GetUsersRequest::builder()
-                .login(vec![login.into()])
+                .login(&[login.as_ref()])
                 .build(),
             token,
         )
@@ -31,7 +31,7 @@ impl<'a, C: crate::HttpClient<'a> + Sync> HelixClient<'a, C> {
     /// Get [User](helix::users::User) from user id
     pub async fn get_user_from_id<T>(
         &'a self,
-        id: impl Into<types::UserId>,
+        id: impl AsRef<types::UserIdRef>,
         token: &T,
     ) -> Result<Option<helix::users::User>, ClientError<'a, C>>
     where
@@ -39,7 +39,7 @@ impl<'a, C: crate::HttpClient<'a> + Sync> HelixClient<'a, C> {
     {
         self.req_get(
             helix::users::GetUsersRequest::builder()
-                .id(vec![id.into()])
+                .id(&[id.as_ref()])
                 .build(),
             token,
         )
@@ -50,7 +50,7 @@ impl<'a, C: crate::HttpClient<'a> + Sync> HelixClient<'a, C> {
     /// Get [ChannelInformation](helix::channels::ChannelInformation) from a broadcasters login
     pub async fn get_channel_from_login<T>(
         &'a self,
-        login: impl Into<types::UserName>,
+        login: impl AsRef<types::UserNameRef>,
         token: &T,
     ) -> Result<Option<helix::channels::ChannelInformation>, ClientError<'a, C>>
     where
@@ -66,7 +66,7 @@ impl<'a, C: crate::HttpClient<'a> + Sync> HelixClient<'a, C> {
     /// Get [ChannelInformation](helix::channels::ChannelInformation) from a broadcasters id
     pub async fn get_channel_from_id<T>(
         &'a self,
-        id: impl Into<types::UserId>,
+        id: impl AsRef<types::UserIdRef>,
         token: &T,
     ) -> Result<Option<helix::channels::ChannelInformation>, ClientError<'a, C>>
     where
@@ -74,7 +74,7 @@ impl<'a, C: crate::HttpClient<'a> + Sync> HelixClient<'a, C> {
     {
         self.req_get(
             helix::channels::GetChannelInformationRequest::builder()
-                .broadcaster_id(id.into())
+                .broadcaster_id(id.as_ref())
                 .build(),
             token,
         )
@@ -361,7 +361,7 @@ impl<'a, C: crate::HttpClient<'a> + Sync> HelixClient<'a, C> {
     /// Get a users, with login, follow count
     pub async fn get_total_followers_from_login<T>(
         &'a self,
-        login: impl Into<types::UserName>,
+        login: impl AsRef<types::UserNameRef>,
         token: &T,
     ) -> Result<Option<i64>, ClientError<'a, C>>
     where
@@ -547,7 +547,7 @@ impl<'a, C: crate::HttpClient<'a> + Sync> HelixClient<'a, C> {
     /// Get channel emotes in channel with user login
     pub async fn get_channel_emotes_from_login<T>(
         &'a self,
-        login: impl Into<types::UserName>,
+        login: impl AsRef<types::UserNameRef>,
         token: &T,
     ) -> Result<Option<Vec<helix::chat::ChannelEmote>>, ClientError<'a, C>>
     where

[Emilgardis]

this looks promising for the deserialization: yoke and zerofrom

[Emilgardis]

Should investigate if there is a better surface for storing multiple values than

foos: Cow<'a, [&'a FooRef]>,

the issue is that requiring FooRef means that there is a situation where your collection is Vec<Foo> and to actually pass it to the request you'd need to do let v: Vec<&'a FooRef> = vec.iter().map(Deref::deref).collect() which is another allocation.

It'd be neat to abstract this so that all variants works. i.e passing &[Foo], [Foo; _], Vec<Foo>, Vec<&FooRef>, &[&FooRef].

The trait that unifies these should be IntoIterator<Item = Into<&FooRef>>, however it's not very nice. This is what I came up with

use std::borrow::Cow;
use twitch_types as types;

fn main() {
    let ids: Vec<types::UserId> = vec!["123".into(), "456".into(), "789".into()];
    println!("{:p}", &*ids);

    let req = Request {
        id: Cow::from(&ids),
        _pd: <_>::default(),
    };
    println!("{:p}", &*req.id);
}

pub struct Request<'a, 'i, IT, D: 'a>
where
    &'i IT: IntoIterator<Item = D>,
    IT: ToOwned + ?Sized,
    D: ?Sized + Into<&'a types::UserIdRef>,
{
    pub id: Cow<'i, IT>,
    _pd: std::marker::PhantomData<&'a ()>,
}

    Finished dev [unoptimized + debuginfo] target(s) in 0.36s
     Running `target/debug/twitch_api_ref_coll`
0x1516069e0
0x1516069e0

[Emilgardis]

Maybe an enum

enum Cowtainer<'a, T> where T: ToOwned + ?Sized {
    One(Cow<'a, T>),
    Multiple(Cow<'a, [&'a T]>),
}

[Emilgardis]

Lessons from ICU4X (with great talk https://www.youtube.com/watch?v=DM2DI3ZI_BQ)

VarZeroVec basically implements Cow<'a, [&'a T]> but nicer (depending on how you see it)

I'm still not satisfied though, I'll experiment with this

[laundmo]

as a new user of this crate, the usage of braid and Cow in many places is not something i have encountered in any other crate and leads to quite verbose code. for example, going from a Option<Cursor> to a Option<Cow<CursorRef>> requires this monstrosity: input.as_ref().map(|c| Cow::Borrowed(c.as_ref()))

that is simply not a nice API. i hope it can be improved in some way.

personally i would be perfectly fine with just using String everywhere - the few extra allocations are a price i would happily pay to be able to simplify my code a lot.

[laundmo]

if there is no willingness to drop braid, i imaging a From implementation could be used:

impl From<Option<Cursor>> for Option<CursorRef> { /*...*/}