Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve ergonomics of OpenAPI definition generation #68

Closed
jclulow opened this issue Dec 22, 2020 · 6 comments
Closed

improve ergonomics of OpenAPI definition generation #68

jclulow opened this issue Dec 22, 2020 · 6 comments
Assignees

Comments

@jclulow
Copy link
Collaborator

jclulow commented Dec 22, 2020

The current print_openapi() is a little unwieldy; it contains a non-optional argument after a sea of optionals, and even if you intend to provide all of the optionals it's a lot of strings that you have to put in the right order.

A simple call, with no optionals, looks like this today:

api.print_openapi(
    &mut f,
    &"Keeper API".to_string(),
    None,
    None,
    None,
    None,
    None,
    None,
    None,
    &"1.0".to_string(),
)?;

I suspect it'd be better as a struct, like:

api.print_openapi(
    &mut f,
    OpenApiDetails {
        name: "Keeper API".to_string(),
        version: "1.0".to_string(),
        ..Default::default()
    },
)?;

It'd be easier and clearer to then incrementally add additional fields; e.g.,

api.print_openapi(
    &mut f,
    OpenApiDetails {
        name: "Keeper API".to_string(),
        version: "1.0".to_string(),
        license_name: Some("CDDL").to_string(),
        ..Default::default()
    },
)?;
@ahl
Copy link
Collaborator

ahl commented Dec 31, 2020

Agreed that this is unwieldy, but I don't like the idea of picking defaults for fields that truly require the caller to specify something e.g. the title or version which are mandated by the spec. I'm inclined to do something like this:

api.print_openapi(
    &mut f,
    OpenApiDetails::new(title, version)
        .license_name("CDDL")
);

... but I'm worried that's not very rusty and would welcome suggestions for how it might be improved.

@jclulow
Copy link
Collaborator Author

jclulow commented Dec 31, 2020

That sounds great! The point about defaults for values that need something specified is a good one, and the Default pattern doesn't have a good answer there.

I believe what you're describing is indeed quite idiomatic, and is generally caller the builder pattern. See also std::process::Command or std::fs::OpenOptions.

@jclulow
Copy link
Collaborator Author

jclulow commented Dec 31, 2020

Actually even better might be api.printer(name, version).licence(...).description(...).write(stream)?.

@ahl
Copy link
Collaborator

ahl commented Dec 31, 2020

Yeah, I'm a pretty big fan of the builder pattern in Java, I just haven't seen employed much in Rust.

Agreed that what I had proposed wouldn't actually work, and that last version looks good.

@jclulow
Copy link
Collaborator Author

jclulow commented Dec 31, 2020

I'll take a swing at an implementation?

@ahl
Copy link
Collaborator

ahl commented Dec 31, 2020

Feel free or I'm happy to get do it--let me know. I was just looking at this, but it might be overkill / odd for a public interface:

https://crates.io/crates/typed-builder

@jclulow jclulow changed the title print_openapi() should accept a Default-able struct instead of positional arguments improve ergonomics of OpenAPI definition generation Jan 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants