feat: support multiple response types

[geoffreygarrett]

Description

This pull request introduces support for multiple response types in the generated client. The following changes have been made:

1. Update .gitignore:

   • Added .idea/ and .env to ignore IDE settings and environment configuration files.

2. Support Multiple Response Types:

   • Added functionality to handle multiple response types.
   • The changes are designed to be additive to the progenitor-impl codebase, avoiding reworking the existing code in oxidecomputer/typify.
   • Added map_inner to the base ResponseValue as impl From<ResponseValue<T>> for ResponseValue<MULTI_ENUM> can't be done in the generated code due to its package scope.

References

• Issue Multiple response types not supported #344: Multiple response types not supported
• Issue Path to GitHub client #395: Path to GitHub client

Notes

This implementation is not suggested as the optimal solution but rather a practical one that can be used immediately. I believe that the ResponseValue and its handling need a rework closer to a more generic approach, as illustrated by @augustuswm here.

However, this solution is purely additive to the progenitor-impl codebase, avoiding reworking anything in oxidecomputer/typify to allow for this.

[geoffreygarrett]

Firstly, really cool work, I found the project while looking for a way to generate a clean GitHub client API for a couple endpoints, which ended up being ironic. Though the generated interfaces here are really clean and aesthetic, so I kept coming back to progenitor wondering what it would take.

Context & Results

Extracting the following API paths from the latest spec api.github.com.json:

• /repos/{owner}/{repo}/actions/secrets
• /repos/{owner}/{repo}/actions/secrets/public-key
• /repos/{owner}/{repo}/actions/secrets/{secret_name}

Results in:

• target.json

First issue I've tried to solve was for /repos/{owner}/{repo}/actions/secrets/{secret_name} concerning multi types for this response:

"responses": {
      "201": {
        "description": "Response when creating a secret",
        "content": {
          "application/json": {
            "schema": {
              "$ref": "#/components/schemas/empty-object"
            },
            "examples": {
              "default": {
                "value": null
              }
            }
          }
        }
      },
      "204": {
        "description": "Response when updating a secret"
      }
    },

This now generates (under generated types):

#[derive(Debug)]
pub enum EmptyObjectOrNone {
    EmptyObject(EmptyObject),
    None,
}
impl From<EmptyObject> for EmptyObjectOrNone {
    fn from(value: EmptyObject) -> Self {
        EmptyObjectOrNone::EmptyObject(value)
    }
}
impl From<()> for EmptyObjectOrNone {
    fn from(_: ()) -> Self {
        EmptyObjectOrNone::None
    }
}

And is handled in the generated send as:

 match response.status().as_u16() {
  201u16 => {
      ResponseValue::from_response(response)
          .await
          .map(|v: ResponseValue<types::EmptyObject>| {
              v.map_inner(types::EmptyObjectOrNone::from)
          })
  }
  204u16 => {
      Ok(ResponseValue::empty(response))
          .map(|v: ResponseValue<()>| {
              v.map_inner(types::EmptyObjectOrNone::from)
          })
  }
  _ => Err(Error::UnexpectedResponse(response)),
}

Allowing for:

let result_multi = client
    .actions_create_or_update_repo_secret()
    .owner(owner.to_string())
    .repo(repo_name.to_string())
    .secret_name(test_secret_name)
    .body(types::ActionsCreateOrUpdateRepoSecretBody::builder()
        .encrypted_value(types::ActionsCreateOrUpdateRepoSecretBodyEncryptedValue::from_str(&encrypted_value).unwrap())
        .key_id(Some(public_key.key_id))
    )
    .send()
    .await;

match result_multi.unwrap().into_inner() {
    types::EmptyObjectOrNone::EmptyObject(_) => {
        println!("Secret created successfully");
    }
    types::EmptyObjectOrNone::None => {
        println!("Secret updated successfully");
    }
}

You can verify with the following, apologies I didn't make it into a repository:

• multi-reponse-demo.zip

Swap the repo_name, owner and add a GITHUB_ACCESS_TOKEN to the .env.

[felix-ulonska]

Is there work to do to get this merged? Can I assist? @ahl

[ahl]

There's a lot here I like.

• ResponseValue::map makes a ton of sense
• decoding the response, and them mapping into the particular enum variant is great
• generating an enum for the various success responses

This exposes some problems and untidiness in the existing code:

• method_sig_body is kind of a big mess
• extract_resposnes is also a bit of a mess; the docs don't match the implementation

Here are some things I'd like to see in the eventual fix:

• Let's put the generated response type in a different mod; we could even do something like response::operation_id::Success
• We'll want to integrate this new concept into the code rather than having it off to the side
• We'll want to do this to support multiple error responses as well as multiple success responses
• We've got to have some integration tests to validate the generated code

[ahl]

Is there work to do to get this merged? Can I assist? @ahl

A very concrete way to help here would be to propose some tests and validate the output

[upachler]

I'd really like to see this get into progenitor.

@geoffreygarrett: I tried merge the current main branch into this, but there's a number of merge conflicts. Would it be possible for you to get this branch up to speed yourself?

In general, I'd be interested how the chances are to get this change to be accepted (@ahl please comment). In order to get my stuff done I need multiple response support as well as #1110. I can contribute tests if that's what's missing, but I'll only put in the effort if I see real chances to get this accepted soon.

Other PRs like my #1110 or #934 have been sitting there for quite a while now without receiving as much attention as I hoped they would, which doesn't help getting my hopes up for this one...

[geoffreygarrett]

@upachler the PR sat there for a while, and it seemed like a desired feature for the full GitHub API support which is why I lost a bit of hope in the project, and yet—to my knowledge, it's still leading in what it tried to be.

Unfortunately I don't think I can reasonably invest time updating the PR unless some maintainer interest is shown.