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

Brainstorming: generate structs, not methods #199

Open
adamchalmers opened this issue May 30, 2023 · 3 comments
Open

Brainstorming: generate structs, not methods #199

adamchalmers opened this issue May 30, 2023 · 3 comments

Comments

@adamchalmers
Copy link
Contributor

adamchalmers commented May 30, 2023

Currently openapitor generates a method for each API endpoint. This method creates a reqwest::Request with a body and path/query/fragment taken from the method's parameters, sends it, handles errors, then parses the response into some output type.

So, the autogenerated methods need to do a lot of things -- create requests, send them, handle errors, parse responses.

To be clear, the current approach works great. But I'd like to propose a general principle: if we can reduce the amount of autogenerated code without causing programmers to do any boilerplate work when specs change, we should.

I propose an alternative, inspired by cloudflare-rs. In that crate, endpoints are structs, not methods. When a team adds an endpoint, they just add a new struct to the codebase and implement certain traits on it. The core maintainers of cloudflare-rs wrote an API client struct with one "call_api" method which takes any endpoint struct as parameter. The only problem with this approach is that every team has to handwrite the endpoint structs and check they're correct with the API spec. If those endpoint structs were autogenerated instead, there'd be no problem.

So basically, I propose that we (KittyCAD) automate via codegen everything which comes from OpenAPI, and we handwrite the other parts (e.g. the core HTTP, networking, error handling).

  • For every HTTP endpoint, openapitor generates a Request struct and Response struct (e.g. UnitConversionReq, UnitConversionResp). The request type has fields for the request body, query params, path params, etc, all taken from the API spec. The response type similarly has fields for the response schema.
    • Put another way, openapitor only generates Rust types for each request and response. It does not generate code to actually send to API endpoints.
    • This includes generating docs for each struct showing exactly how to instantiate it.
  • The actual API client and its concerns (sending API requests, receiving API responses, handling errors) are handled by handwritten types.

This works because the per-endpoint work is the repetitive, always-growing part. Sending/receiving/error-handling the API networking doesn't change when we add new endpoints or change an OpenAPI spec. This reduces the amount of code being generated, which means faster compile times and easier maintenance for KittyCAD engineers, because we've automated the part which changes frequently and handwritten the part that always stays the same.

How would the code-generated Request/Response structs get used by the handwritten client code?

  1. Define a trait Endpoint, which is implemented by all the autogenerated structs. Something like
trait ApiEndpoint<Response: Deserialize> {
    // These methods default to None to reduce the boilerplate for endpoints that don't have body/query params
    fn body(&self) -> Option<Bytes> { None }
    fn query(&self) -> Option<String> { None }
    fn headers(&self) -> Option<http::HeaderMap> { None }
    // These methods are always required, because every endpoint has a different path/method
    fn path(&self) -> String;
    fn method(&self) -> http::Method;
}
  1. Define a Client struct which takes any Endpoint and can send it.
struct Client{
    client: reqwest::Client,
    base_url: String,
}

impl Client {
    async fn call<Api: ApiEndpoint>(api: Api) -> Result<Api::Response> {
        let req = reqwest::RequestBuilder::new()
            .body(api.body())
            .query(api.query())
            .path(api.path())
            .method(api.method())
            .headers(api.headers())
            .build()
        let resp = self.0.send(req).await?.bytes().await?;
        let resp: Api::Response = serde_json::from_vec(resp)?;
        Ok(resp)
    }
}

Because the code for sending/receiving/error-handling is defined once and is handwritten, the Rust compiler has less work to do per-endpoint, lowering code generation pressures that result in really slow compile times in crates like async-stripe which also rely on autogenerated methods.

@adamchalmers adamchalmers changed the title Generate structs, not methods Brainstorming: generate structs, not methods May 30, 2023
@jessfraz
Copy link
Contributor

this sounds good to me, as long as we still generate the docs as well!

@adamchalmers
Copy link
Contributor Author

Awesome! Yes, I think this would actually simplify doc generation too. Because our generated docs would show users how to instantiate each endpoint struct. Then our handwritten docs would show users how to use the client and a generated struct to make an API call.

@adamchalmers
Copy link
Contributor Author

adamchalmers commented May 31, 2023

I think the new openapitor would ideally do a few passes:

  • Take in the OpenAPI spec, analyze it to figure out which Rust structs will need to be created.
    • For each OpenAPI (path, path item aka HTTP method) pair, add an "endpoint struct" to the schema
    • For each OpenAPI component (schema, parameter, response), add a "plain old serde struct" to the schema
    • If an OpenAPI type is discovered which we can't handle yet, terminate.
  • Take in Rust struct list, output the Rust package structure
    • Choose which modules will be created
    • Choose names for all the types (short, concise names where possible, longer disambiguated names where necessary)
  • Take in the Rust package structure, output generated code defining Rust structs.
    • each "plain old serde struct" becomes a struct which impls De/serialize
    • each "endpoint struct" becomes a struct which impls ApiEndpoint.

By separating the "analyze OpenAPI" phase and the "generate Rust code" phase I think the architecture will be cleaner -- each concern gets a different pass. And we guarantee that we don't start outputting Rust code until we know the OpenAPI schema can be properly analyzed and parsed by us.

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

2 participants