libninjacom / plaid-rs

Rust client library for Plaid, generated from Plaid's OpenAPI spec
https://docs.rs/plaid/
MIT License
11 stars 6 forks source link

Why have all the models been changed to expect Strings instead of references in v2? #1

Closed thomasmost closed 2 years ago

thomasmost commented 2 years ago

It's a lot more annoying to initialize the models in v2 now that each model expects a String instead of an &str with a lifetime.

Was there a technical reason for this change?

(See an example of the old link_token models) https://github.com/ammubhave/plaid-rs/blob/main/src/link_token.rs

kurtbuilds commented 2 years ago

That's a great point. At first glance, I agree it seems it'd be easier to take a &'a str.

For clarity's sake, can you provide a code example before/after where you're encountering this issue? It shouldn't be too challenging to implement this.

thomasmost commented 2 years ago

Sure. So, for example, this in v1:

const PLAID_PRODUCTS: [&'static str; 1] = ["transactions"];
const PLAID_COUNTRY_CODES: [&'static str; 2] = ["US", "CA"];
...
  let configs = plaid::link_token::LinkTokenConfigs {
    user,
    client_name: "Bend",
    language: "en",
    country_codes: &PLAID_COUNTRY_CODES,
    products: Some(&PLAID_PRODUCTS),
    webhook: Some("https://api.bend.green/webhook"),
    access_token: None,
    link_customization_name: None,
    account_filters: None,
    redirect_uri: None,
    android_package_name: None,
  };

...becomes something like this:

const PLAID_PRODUCTS: [&'static str; 1] = ["transactions"];
const PLAID_COUNTRY_CODES: [&'static str; 2] = ["US", "CA"];
...
  let configs = plaid::model::LinkTokenCreateRequest {
    user,
    client_name: String::from("Bend"),
    language: String::from("en"),
    country_codes: Vec::from(&PLAID_COUNTRY_CODES),
    products: Some(Vec::from(&PLAID_PRODUCTS)),
    webhook: Some(String::from("https://api.bend.green/webhook")),
    access_token: None,
    link_customization_name: None,
    account_filters: None,
    redirect_uri: None,
    android_package_name: None,
  };

Working with the static constants is particularly annoying, I don't even think what's here would work in fact—we might need to iterate over the slices and map them to a Vec<String>

kurtbuilds commented 2 years ago

Hey @thomasmost This is a breaking change, so it needs to be released as 3.0, but 3.x supports reference types as arguments rather than owned. I included several changes that should positively impact developer experience as part of 3.x:

    let item_get = client.link_token_create(LinkTokenCreateRequired {
        client_name: "",
        language: "",
        country_codes: &["US", "CA"],
        user: LinkTokenCreateRequestUser {
            client_user_id: "".to_string(),
            ..Default::default()
        },
    })

This means your code only has to bother with required args, and then optional arguments are chained using builder pattern before calling send().

There's now an example of plaid link in the examples/ folder: Link.

Let me know if this fixes your issues!

thomasmost commented 2 years ago

@kurtbuilds the defaults support is very nice, but as far as I can tell v3 still expects owned objects and does not accept reference types as indicated in your example above.

kurtbuilds commented 2 years ago

Can you give an example where you're seeing that? For link_token_create, it takes an owned LinkTokenCreateRequired struct, which takes several strings as reference types (Link). Other functions, like transactions_get, take reference types directly (Link).

I wonder if the confusion stems from, with v2 onward, you don't directly create these structs, because the structs are associated with a PlaidClient, which handles authentication, can support request recording (e.g. test fixtures) and in the future, might support features like retry. Instead, you call it as in the example code in my previous comment.

Internally, it does map them to owned data types, but that's an implementation detail.

thomasmost commented 2 years ago

OH. Aha. Got it, yes! Missed that detail