search

arlyon / async-stripe

Async (and blocking!) Rust bindings for the Stripe API
https://payments.rs
Apache License 2.0
447 stars 129 forks source link

Make functions modify existing variable instead of returning new one with more recent state. #335

Open sloganking opened 1 year ago

sloganking commented 1 year ago

Is your feature request related to a problem? Please describe.

Currently, many functions such as CheckoutSession::expire() (see #334) and Subscription::cancel() take an object's ID, modify it on stripe, and then have the function return the modified value if it was successful. This is potentially problematic though, as the original object is not modified or deleted, so the programmer could mistakenly use the old object instead of the new one, and thus mistakenly be operating on outdated state.

let subscription =;// ...

let modified_subscription = Subscription::cancel(&client, &subscription.id, stripe::CancelSubscription::new());

// DANGER!, mistakenly checking status of old subscription state and not the latest state.
if subscription.status == //???

Describe the solution you'd like

It would be better if functions like Subscription::cancel() took a mutable reference to a Subscription instead of a SubscriptionId. And modified the original variable if it's state changed. That way there would be no chance of operating on outdated state.

Original idea here: https://github.com/arlyon/async-stripe/pull/334#issuecomment-1412839731

https://github.com/arlyon/async-stripe/pull/334#issuecomment-1414140815 @arlyon thinks this might be best implemented through a trait that would accept either IDs or entire objects. I am not sure how that would best be done but I leave the idea here.

Describe alternatives you've considered

No response

Additional context

No response

arlyon commented 1 year ago

To expand on this, we could impl Into<_Id> for all types and have the function be generic over anything that implements Into<_Id> such that the object itself moves.