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

[Swift5] Add an async execute API to RequestBuilder<T> #14416

Merged
merged 4 commits into from
Jan 10, 2023

Conversation

yosshi4486
Copy link
Contributor

@yosshi4486 yosshi4486 commented Jan 10, 2023

Add a new async API interface to RequestBuilder<T>. I changed APIs.mustache, then updated petstore samples by running ./bin/generate-samples.sh ./bin/configs/swift5*.

Issue: #14415

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Mentions

@4brunu

@4brunu
Copy link
Contributor

4brunu commented Jan 10, 2023

Hi, could you please elaborate on the use case where you need this? Thanks

@@ -65,6 +65,32 @@ import Vapor
return requestTask
}

{{#useAsyncAwait}}
@discardableResult
{{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}open{{/nonPublicApi}} func execute() async throws -> Response<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we should have an @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) before the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed it!

@yosshi4486
Copy link
Contributor Author

yosshi4486 commented Jan 10, 2023

Hi, could you please elaborate on the use case where you need this?

Ok @4brunu . I'd like to use the API in a situation where making a builder object is better than using high-level async API (like open class func usersIdTimeline(id: String) async throws -> Get2UsersIdTimelinesReverseChronologicalResponse)

An example of the usecase is computing onetime values and passing them into header fields, such as a signature or crypt things. I suppose using a builder instance and calling addHeaders method is better than setting onetime values into a global OpenAPIClientAPI.customHeaders property.

Currently, I can't find a way of passing the builder as a high-level API's parameter, or executing it with async/await.

My Concrete Usecase

I use openapi-gen swift5 client with an OAuth client library, like bellow:

let oauthClient = OAuthSwiftClient(
    consumerKey: "xxx",
    consumerSecret: "xxx",
    oauthToken: "xxx",
    oauthTokenSecret: "xxx",
    version: .oauth1
)

// Because oauth_signature needs a complete URL, I make a builder object in advance.
let tweetAPIRequestBuilder = TweetsAPI.usersIdTimelineWithRequestBuilder(id: "xxx")

// Get computed oauth authorization header fields
guard let computedHTTPHeaderFields = oauthClient.makeRequest(.init(url: URL(string: tweetAPIRequestBuilder.URLString)!)).config.urlRequest.allHTTPHeaderFields else {
    return
}

// Set onetime oauth values into the builder object's headers
tweetAPIRequestBuilder.addHeaders(computedHTTPHeaderFields)

// I can write like this, but...
// OpenAPIClientAPI.customHeaders = computedHTTPHeaderFields
// Task { let response = try? await TweetsAPI.usersIdTimeline(id: "xxx") }

// Call `try await builder.execute()` is simple
Task {
    do {
        let response = try await tweetAPIRequestBuilder.execute()
        let tweets = response.body.data
    } catch {
        print(error)
    }
}

@4brunu
Copy link
Contributor

4brunu commented Jan 10, 2023

Thanks for explaining the use case.
Looks reasonable.
Could you please fix the issue that I open in code review?
That may help with the CI failure.
Thanks

Copy link
Contributor

@4brunu 4brunu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍
Thanks for creating this PR.

@4brunu 4brunu merged commit 4044e72 into OpenAPITools:master Jan 10, 2023
@yosshi4486
Copy link
Contributor Author

Thank you for the code review!

@yosshi4486 yosshi4486 deleted the async-execute-interface branch January 10, 2023 17:14
@wing328 wing328 added this to the 6.3.0 milestone Jan 20, 2023
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.

3 participants