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

feat: support multiple response types #857

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

geoffreygarrett
Copy link

@geoffreygarrett geoffreygarrett commented Jul 13, 2024

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

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.

…er#395)

Add support for multiple response types in the generated client, including handling JSON deserializable types, empty bodies, streams of bytes, and upgraded connections.

This change addresses the following issues:
- Multiple response types not supported (oxidecomputer#344)
- Path to GitHub client (oxidecomputer#395)
@geoffreygarrett
Copy link
Author

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:

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:

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

@geoffreygarrett geoffreygarrett marked this pull request as draft July 14, 2024 11:26
@geoffreygarrett geoffreygarrett marked this pull request as ready for review July 14, 2024 11:26
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

Successfully merging this pull request may close these issues.

1 participant