-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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][client] allow request cancellation and authentication flow to work together #11019
[swift5][client] allow request cancellation and authentication flow to work together #11019
Conversation
@denizdogan @davidhorvath @funzin @jarrodparkes I would appreciate if you could help to review this PR 🙂 |
@4brunu going to try and dig into this today — thanks for reaching out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@4brunu I was able to copy/paste the BearerRequestBuilderFactory
example without modification. Was there something else I needed to check here? From what I gather, prior to this change, the example wouldn't work/compile?
modules/openapi-generator/src/main/resources/swift5/Models.mustache
Outdated
Show resolved
Hide resolved
Yes, it wouldn't compile. |
@4brunu just ran a little test with the Network Link Conditioner tool (so I could emulate a bad network), and the cancelling worked fine 👍 print("starting task1")
let task1 = PetstoreClientAPI.PetAPI.getPetById(petId: 9223372036854775807) { data, error in
if let data = data {
dump(data)
}
}
DispatchQueue.main.asyncAfter(deadline: .now() + 2) {
print("cancelling task1")
task1.cancel()
}
print("starting task2")
let task2 = PetstoreClientAPI.PetAPI.getPetById(petId: 9223372036854775807) { data, error in
if let data = data {
dump(data)
}
}
DispatchQueue.main.asyncAfter(deadline: .now() + 15) {
print("cancelling task2")
task2.cancel()
} And the output...
Notice how |
Thanks for helping testing this. |
@wing328 this one is ready to merge from me 👍 |
Fixes #11002
With the introduction of cancellable request on the swift generator, it broke the authentication flow and this PR tries to fix that.
Instead of returning the
URLSessionTask
, we return a class of ours,OpenAPIRequestCancellable
, and the users may call the cancel method ofOpenAPIRequestCancellable
.There is one
OpenAPIRequestCancellable
instance per request.On the other hand, the
URLSessionImplementations/AlamofireImplementations
save the current request,URLSessionTask
in case of URLSession orRequest
in case of Alamofire, in theOpenAPIRequestCancellable
so that the request can be later on cancelled.In case there is a retry of the request, we save the new
URLSessionTask
in the current request, so that the new request can be cancelled.PR checklist
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.
master
(5.3.0),6.0.x
@jgavris (2017/07) @ehyche (2017/08) @Edubits (2017/09) @jaz-ah (2017/09) @4brunu (2019/11)