SPM Prep – Move some async perform types at the WordPressComRESTAPIInterface level
#788
Conversation
This is just a convenience refactor to make future diff clearer
As per Swift API naming guidelines on arguments with default values
mokagio
changed the title
async perform types at the WordPressComRESTAPIInterface levelasync perform types at the WordPressComRESTAPIInterface level
|
|
||
| @property (strong, nonatomic, readonly) NSURLSession * _Nonnull urlSession; | ||
|
|
||
| @property (strong, nonatomic, readonly) void (^ _Nullable invalidTokenHandler)(void); |
There was a problem hiding this comment.
All these properties went in the protocol to make the async perform methods work.
The async perform methods, which like in WordPressComRestApi are Swift-only, of course.
I considered using an extension or a different protocol, but it seemed neat to have it all in WordPressComRESTAPIInterfacing, so that Swift code can used an upgraded version of the same interface that the Objective-C code uses. This will actually be useful in the cases where we have an ObjC superclass with the API client conforming to WordPressComRESTAPIInterfacing and a subclass using the API client, too.
| self.parameters = parameters | ||
| } | ||
|
|
||
| public func encode(to encoder: Encoder) throws { |
There was a problem hiding this comment.
When the type became public, a public init became necessary because the compiler does not synthesize one outside of the package.
As for encode(to:), it needs to be public because Encodable is declared as part of the public type interface.
🤔 In hindsight, I suppose the Encodable part could be moved into an internal extension, but I think it's okay to leave it as is. Let me know if you can think of a strong reason to hide it.
| /// Other error occured. | ||
| case unknown(underlyingError: Error) | ||
|
|
||
| static func unparsableResponse(response: HTTPURLResponse?, body: Data?) -> Self { | ||
| return WordPressAPIError<EndpointError>.unparsableResponse(response: response, body: body, underlyingError: URLError(.cannotParseResponse)) | ||
| } |
There was a problem hiding this comment.
I didn't look up when, but relatively recently, I think, Swift acquired default values for associated parameters. So we can remove this builder method in favor of defining the underlying error as a default for the case.
| @@ -173,9 +177,6 @@ open class WordPressComRestApi: NSObject { | |||
| } | |||
| } | |||
|
|
|||
| @objc func setInvalidTokenHandler(_ handler: @escaping () -> Void) { | |||
| invalidTokenHandler = handler | |||
| } | |||
There was a problem hiding this comment.
Became redundant once invalidTokenHandler became a property in the protocol.
Description
This was part of #784 but I've been asked to wait on the actual
extensionremoval because of conflicting WIP work.All these changes should be fine to merge regardless, as are mostly about changing access control, injecting new params, and moving methods at the protocol level.
Testing Details
See CI.
CHANGELOG.mdif necessary. — N.A.