[swift5] Abstract away URLSession (#11651) #12110
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors. Let me know if you need help fixing it. |
leszek-s
force-pushed
the
fix_issue11651
branch
from
8262580 to
83a54d4
Compare
4brunu
reviewed
Apr 13, 2022
| extension URLSession: NetworkSession { | ||
| public func loadData(with request: URLRequest, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask { | ||
| let task = dataTask(with: request, completionHandler: completionHandler) | ||
| task.resume() |
There was a problem hiding this comment.
@leszek-s I think we shouldn't resume the request here, because this is an unexpected side effect.
We should try to mimic the URLSession API as much as we can.
There was a problem hiding this comment.
If the methods in the protocol, are exactly the same as the URLSession correspondent one's, we don't need to implement the methods, because the mapping is automatic.
https://christiantietze.de/posts/2016/02/mock-replace-adapter/
There was a problem hiding this comment.
@4brunu I've put it here to give a possibility to have a better control on when the task is started if someone for example wants to delay some tasks. However i'm still thinking about some other solutions.
There was a problem hiding this comment.
To do that, maybe we can override the method resume and add the logic there.
There was a problem hiding this comment.
resume() is a method of URLSessionDataTask and not URLSession so we can't do that. Do you think it would be a better idea to add a method to this NetworkSession/URLSessionProtocol protocol for example resumeTask(dataTask: URLSessionDataTask) with a default implementation and use that instead of calling resume on dataTask?
There was a problem hiding this comment.
OK I removed this resume() and went with simple URLSessionProtocol with the same method as original. I currently do not have a better idea how to handle that resume method so maybe lets just do it with a minimal change as originally propose in the issue. I think delaying task should still somehow be possible by overriding execute method so for now we can live without that control over resume().
4brunu
reviewed
Apr 13, 2022
| @@ -170,8 +200,6 @@ private var credentialStore = SynchronizedDictionary<Int, URLCredential>() | |||
| challengeHandlerStore[dataTask.taskIdentifier] = taskDidReceiveChallenge | |||
| credentialStore[dataTask.taskIdentifier] = credential | |||
|
|
|||
| dataTask.resume() | |||
There was a problem hiding this comment.
@leszek-s in continuation of the above comment, this change should be reverted.
4brunu
reviewed
Apr 13, 2022
| @@ -9,6 +9,18 @@ import Foundation | |||
| import MobileCoreServices | |||
| #endif | |||
|
|
|||
| public protocol NetworkSession { | |||
There was a problem hiding this comment.
@leszek-s and if we make this protocol match the URLSession methods signatures, maybe we can rename it to something like URLSessionProtocol.
There was a problem hiding this comment.
I changed it to URLSessionProtocol as proposed.
|
@leszek-s Hey, |
leszek-s
force-pushed
the
fix_issue11651
branch
from
83a54d4 to
209feae
Compare
|
Hey, currently I don't have time to look into this. |
4brunu
reviewed
Apr 18, 2022
| @@ -9,6 +9,12 @@ import Foundation | |||
| import MobileCoreServices | |||
| #endif | |||
|
|
|||
| public protocol URLSessionProtocol { | |||
There was a problem hiding this comment.
@leszek-s instead of public, could you please use {{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}}?
There was a problem hiding this comment.
I've changed it as suggested.
|
@leszek-s Other than the comment that I left, it looks good to me 👍 |
leszek-s
force-pushed
the
fix_issue11651
branch
from
209feae to
aec84fa
Compare
4brunu
approved these changes
Apr 19, 2022
rk0n
pushed a commit
to rk0n/openapi-generator
that referenced
this pull request
Apr 24, 2022
spacether
added a commit
that referenced
this pull request
Apr 26, 2022
* Add source folder variable to fastapi fix 12118 * Add generated sample files for python-fastapi fix 12118 * [python-experimental] fixes json + charset use case (#12114) * Adds code to detect json content type when charset is also set * Updates template to properly render content type, regenerates samples * Adds test_json_with_charset * Reverts version file * Fixes typo * Add example allOf with single ref (#10948) * Add example allOf with single ref * fix dart-dio-next handling of that case * Refactor without vendor extension * Regenerate newer samples * Add a sample of an enum model array in query params (#12107) * [typescript-fetch] drop support typescript under v4.0 (#12102) * [typescript-fetch] drop support typescript under v4.0 * [typescript-fetch] update docs * [typescript-fetch] update package-lock.json manually & fix test * [typescript-fetch] fix test * update samples * [dart] Remove old dio generator (to be replaced with dart-dio-next) (#12109) * remove old `dart-dio` generator which will be replaced by `dart-dio-next` in a seperate PR * remove left-over `DartJaguarClinetCodegen` class which was sunset a while ago and is unused * fix: respect configured generator URL in swagger config (#12064) * fix: respect configured generator URL in swagger config The generated OpenAPI spec does not reflect the GENERATOR_HOST which causes wrong generated code and non-functional snippets in the UI. This PR improves that by adding the relevant parts to the spec. * style: use `OpenAPI` instead of `Swagger` * refactor: make Set creation Java 8 compatible * fix: add missing import * [typescript*] drop support typescript below 4.0 (#12123) * [typescript-axios] drop support typescript below 4.0 & update samples * [typescript-axios] update package.json & package-lock.json * [typescript-node] drop support typescript below 4.0 & update samples * [typescript-nestjs] drop support typescript below 4.0 & update samples * [typescript-redux-query] drop support typescript below 4.0 & update samples * [typescript-aurelia] drop support typescript below 4.0 & update samples * [typescript-jquery] drop support typescript below 4.0 & update samples * [typescript] drop support typescript below 4.0 & update samples * Upgrade haskell-servant to latest LTS (#12092) * [C++][Qt] update petstore to 3.0 spec (#12124) * test update sampels * update samples * add file * update readme with onesignal (#12126) * [typescript-fetch] Removed functions that are unused when withoutRuntime is true. (#12101) * [typescript-fetch] remove unused function when withoutRuntimeCheks option to true * [typescript-fetch] update samples * [kotlin][client] fix encoding of individual parts of a multipart request (#11911) * [kotlin][client] fix encoding (and Content-Type headers) of individual parts of a multipart request * [kotlin][client] fix incorrect handling of binary downloads * [kotlin][client] update samples * [python-experimental] Allow response media types to omit schema (#12135) * Adds issue spec file and attemts to generate code from it * Adds missing schema definitions * Skips fromProperty invocation if the passed in schema is none in getContent * Makes MediaType.schema optional * Adds checking that the content type is in self.content * Sets ApiResponse body type as Unset if there is no schema for it * Handles schema = None case * Adds endpoint without response schema * Reverts version files * Adds test_response_without_schema * improve errorObjectType to avoid regression (#12131) * [php-slim4] Add monolog package as default logger (#12137) * Add monolog to templates * Remove default values from DI\get helper It turned out \DI\get expects only single argument, current method call doesn't throw any errors but it should be corrected anyway. * Refresh samples * Bump async from 2.6.3 to 2.6.4 in /website (#12148) Bumps [async](https://github.com/caolan/async) from 2.6.3 to 2.6.4. - [Release notes](https://github.com/caolan/async/releases) - [Changelog](https://github.com/caolan/async/blob/v2.6.4/CHANGELOG.md) - [Commits](caolan/async@v2.6.3...v2.6.4) --- updated-dependencies: - dependency-name: async dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * do not put the invalid value of the enum to a JSON structure (#12133) * [Java] Ignore return value for Java file assert classes (#12145) * Add @mkusaka to TS technical committee (#12150) * Fix documentation for users of AbstractJavaJAXRSServerCodegen (#12142) The default for `sourceFolder` in the documentation was wrong. * [Dart][client] Adjust toJson method to use '_json' instead of 'json' to avoid shadowing fields named 'json' (#12127) (#12138) * [Dart][client] Updated dart samples Co-authored-by: 0xnf <0xnf> * [dart] Support/Fix sourceFolder parameter and docs (#12113) * [dart] Support/Fix sourceFolder parameter and docs * allow passing the sourceFolder to generators * not sure how this translates to the dart2 generator due to the `part` files * fix doc generation not showing default value of CLI options * [dart] Delete old generator doc files * [Ruby] Add support for faraday 2.x (#12112) * [ruby] Add faraday 2.x support * Remove redundant params_encoder config * Memoize Faraday connection and refactor * [Java/Spring] all-of and one-of Improvements and Fixes (was #12075) (#12089) * Fix Bug in OneOfImplementorAdditionalData pulling in wrong vars to one-of-implementors. Support parentVars in order to support fluent setter with inherited properties. Squashed commit of the following: commit f945c943777a1a496d7de8fc0a188842d9efb1ac Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 18:22:54 2022 +0200 Polishing commit 23ce1d0ff1faff53e85ca4362f33660962aa6a92 Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 17:15:28 2022 +0200 Add JavaDoc commit fee70fde5709afa67f3aabd4f48ba496df63a884 Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 17:11:17 2022 +0200 Add imports for inherited Properties commit 29509aaac51750fbd33c00a57d32cac34cbcbb90 Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 13:40:36 2022 +0200 Generate Samples commit 1d19d5465137d3af712f2fd3b4ae4474c58af15e Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 13:21:23 2022 +0200 SpringCodegen: Support parentVars in order to support fluent setter with inherited properties. commit 2217a77bb747d0b07ef17407a6b5dd5c624a2551 Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 07:18:50 2022 +0200 Add allVars to omit list in OneOfImplementorAdditionalData commit 90499a3b0a187971bfe25deb6355c3444dcf89a7 Author: Lars Uffmann <[email protected]> Date: Wed Apr 6 16:40:23 2022 +0200 Works exactly as needed for oneOf/allOf in Java/Spring commit b6d496d772e0d0a8d87a3b8cdba8fd3ca4db7f3f Author: Lars Uffmann <[email protected]> Date: Wed Apr 6 15:16:27 2022 +0200 Debug Session: identify critical codep path commit 85722360038107f15841d5acc448d93dee513a06 Author: Lars Uffmann <[email protected]> Date: Tue Apr 5 09:56:38 2022 +0200 Add test case to reproduce issue. commit 14acc5cd974bb5260f3751015558807e2eb1a8a1 Author: Lars Uffmann <[email protected]> Date: Tue Apr 5 06:57:30 2022 +0200 Add config to reproduce the issue * Adjust indentation. Co-authored-by: Lars Uffmann <[email protected]> * [REQ][Ruby] Ruby Allow Follow Redirect (#12047) * Add `follow_location` option Implementation of #10028 * regenerated clients * set follow_location default to true * Adds UUID to python-experimental (#12153) * Adds UUID to python-exp, allows uuid models to be generated * Adds test_UUIDString uuid model test * Fixes uuid properties in python-exp, changes maps to object data type, adds uuid data type * Adds maps data type back in * Adds missing Null and AnyType definitions and adds them to python-experimental * Generator docs updated, added missing uuid, null, anytype, and object * Adds uuid support description * Docs updated * update url to travis ci * Adds not to CodegenComposedSchemas and uses it in python-exp (#12146) Updates docs * [python-experimental] fixes bug where some singleton representations raised a RecursionError (#12157) * Adds issue components and endpoint * Regenerates samples * Fixes singleton repr, removes issue components and endpoint * Removes unused endpoint * Reverts file * Adds tests of enum, boolean, and none representations * Uses super repr for singletons that ere not none, true, or false * [swift5] Abstract away URLSession (#11651) (#12110) * [Java][OkHTTP] fix empty request body handling (#12172) * Better inline model resolver to handle inline schema in array item (#12104) * better support of inline schema in array item * update tests * update samples * regenerate samples * fix allof naming, remove files * add files * update samples * update readme * fix tests * update samples * update samples * add new files * update test spec * add back tests * remove unused files * comment out python test * update js test using own spec * remove files * remove unused files * remove files * remove unused files * better handling of allOf with a single type * comment out go test * remove test_all_of_with_single_ref_single_ref_type.py * fix inline resolver, uncomment go test * [Inline model resolver] minor enhancements/refactoring (#12175) * better code format * better code format, minor refactor * [python-experimental] Fixes enum is comparison (#12176) * Fixes enum is comparison * Reverts file * [php] make ObjectSerializer::toString actually return a string (#12158) * update php samples * Allow selection of MP REST API version for MicroProfile REST client g… (#12043) * Allow selection of MP REST API version for MicroProfile REST client generation * fix typo in pom.xml * fix typo in pom.xml, update samples * add exception when incorrect MP Rest Client version is chosen * [Java][microprofile] update API test template to work with v3.0 (#12177) * update microprofile api test to work with 3.0 * minor format change * update samples * Return type for Azure funcs (#12115) * Azure func return type * Changed to Task<IActionResult<T> * Readme * update doc * Improvements to csharp-netcore-function generator (#12183) * improvements to csharp-netcore-function generator * update samples * update doc, samples * [java-micronaut] Support Optional for non-required properties (#12144) The Micronaut generator by default adds the @nullable annotation to non-required properties and allows using the Jackson JsonNullable wrapper but it is not possible to use java.util.Optional as a wrapper for optional properties. This change adds support for using the Optional wrapper for non-required properties. * update java samples * [typescript-fetch] allow initOverrides with async function (#12098) * [typescript-fetch] allow initOverrides with async function * [typescript-fetch] update samples * [typescript-fetch] refactoring initFnction apply * [typescript-fetch] update samples * [typescript-fetch] refactoring create body function * [typescript-fetch] update samples * [typescript-fetch] make interface more flexible * [typescript-fetch] update samples * [typescript-fetch] support 2.x version of typescript & update samples * [typescript-axios] update samples * [typescript-fetch] refactor: add type alias * [typescript-fetch] override with init params even if initOverrides is function * [typescript-fetch] update samples * [Wsdl] Adding cli-option for generating different versions of WSDL-files regarding Media type versioning (content negotiation) (#12206) * add wsdl version generation * add option to use specified operationId * update samples * update cli description * [python-flask] Fix return type too strict (#12190) Flask (and connexion by extension) allows the return type to be either just the body, or the body & status code, or the body & status code & headers. This commit fixes the stated `rtype` to allow the latter two cases. * update samples, docs * Fixing bug in Kotlin Client with BigDecimal default value (#12213) * Fix duplication of "Api" when structPrefix is set (#12128) This fixes a minor duplication of the word "Api", which is already part of the classname template parameter and doesn't need to be repeated when structPrefix is set. * add samples/client/petstore/kotlin-bigdecimal-default to kotlin ci tests * remove spring-mvc samples (#12222) * [Micronaut] Add option to describe response wrappers (#12186) * Minor refactor for Micronaut generators * Add support for security roles in micronaut server generator * Micronaut Server Generator refactor the x-roles String variable * Add support for Micronaut HttpResponse wrapper * Generate samples * Optimize the usage of context-path for Micronaut server * Emit default values for aspnetcore 3 value types (#11280) Fixes #10772 for aspnetcore 3+ This allows numbers to be set to zero, and booleans to be set to false. It may make sense to port this fix to the other C# generators, though it was partially fixed (for booleans only) in the netcore client in PR9042. * update samples Co-authored-by: Justin Black <[email protected]> Co-authored-by: Peter Leibiger <[email protected]> Co-authored-by: mkusaka <[email protected]> Co-authored-by: William Cheng <[email protected]> Co-authored-by: Florian Greinacher <[email protected]> Co-authored-by: Tom Bärwinkel <[email protected]> Co-authored-by: Anton Koscejev <[email protected]> Co-authored-by: Yuriy Belenko <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Hui Yu <[email protected]> Co-authored-by: Oleh Kurpiak <[email protected]> Co-authored-by: Eric Wolf <[email protected]> Co-authored-by: 0xNF <[email protected]> Co-authored-by: Yohei Kitamura <[email protected]> Co-authored-by: cachescrubber <[email protected]> Co-authored-by: Lars Uffmann <[email protected]> Co-authored-by: Connor Moore <[email protected]> Co-authored-by: leszek-s <[email protected]> Co-authored-by: fengelniederhammer <[email protected]> Co-authored-by: Andrii Serkes <[email protected]> Co-authored-by: Abrhm7786 <[email protected]> Co-authored-by: Auke Schrijnen <[email protected]> Co-authored-by: adessoDpd <[email protected]> Co-authored-by: Oliver Ford <[email protected]> Co-authored-by: Johan Sjöblom <[email protected]> Co-authored-by: Noah Fontes <[email protected]> Co-authored-by: Andriy Dmytruk <[email protected]> Co-authored-by: matt beary <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added abstraction of URLSession in swift5 template