tokio-rs / toasty

An async ORM for Rust (incubating)
MIT License
1.09k stars 26 forks source link

get all entities? #35

Open open-schnick opened 4 weeks ago

open-schnick commented 4 weeks ago

Hi i have the following code expecting that i get one entity back which is not the case:

    let new_artist = Artist::create()
        .name("Artist")
        .exec(&db)
        .await
        .unwrap();

    Song::create()
        .title("Crazy Good Song")
        .artist(&new_artist)
        .exec(&db)
        .await
        .unwrap();

    let all_songs = Song::find_many_by_id()
        .all(&db)
        .await
        .unwrap()
        .collect::<Vec<Song>>()
        .await
        .unwrap();

    assert_eq!(all_songs.len(), 1);

Is this a api misuse by me or a bug in the application?
Also i think having a Song::find_all() would make sense from a readability and usability perspective

PS: I know logging is on the roadmap but here having tracing log the executed statement would make it easier to debug

open-schnick commented 4 weeks ago

i am also happy to help implementing tracing but i am not really an expert on the correct usage of tracing in libs especially when performance is a concern

dmgolembiowski commented 4 weeks ago

hey hayden @hds you're lowkey a subject matter specialist on this. any suggestions?

carllerche commented 4 weeks ago

Adding tracing support would be welcome. We can do it incrementally.

carllerche commented 4 weeks ago

Hi i have the following code expecting that i get one entity back which is not the case:

    let new_artist = Artist::create()
        .name("Artist")
        .exec(&db)
        .await
        .unwrap();

    Song::create()
        .title("Crazy Good Song")
        .artist(&new_artist)
        .exec(&db)
        .await
        .unwrap();

    let all_songs = Song::find_many_by_id()
        .all(&db)
        .await
        .unwrap()
        .collect::<Vec<Song>>()
        .await
        .unwrap();

    assert_eq!(all_songs.len(), 1);

Is this a api misuse by me or a bug in the application? Also i think having a Song::find_all() would make sense from a readability and usability perspective

PS: I know logging is on the roadmap but here having tracing log the executed statement would make it easier to debug

This is not returning anything because this is the API to find many using a batch of IDs. You did not include any IDs so nothing is returned. I am on mobile so I will have to look into how to get all entities later. I know this isn’t supported on all data stores (eg dynamodb) so it will have to be an opt-in api based on target database support.

kevinbader commented 3 weeks ago

So find_many_by_id without IDs should not compile, right?

carllerche commented 3 weeks ago

So find_many_by_id without IDs should not compile, right?

It would be nice if it didn't, but I don't know if there is an obvious way to achieve that without complicating the API. Toasty aims for a more straightforward public API at the expense of maximizing type state.

Another approach for find_many_by_id would be to take the ID batch as an argument. So something like:

Song::find_many_by_ids(&[id1, id2, id3])

It would probably have to be a T: IntoIterator argument to avoid always having to pass in a slice.