2.0.0 - Swift concurrency & authentication update #63
Conversation
This file contains hidden or 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
Implemented EmptyRoute for requests that are expected to return no data, and where we expect a 201 or 204 response.
Still working on the architecture of the async endpoints. Rather than having an `explore` resource that groups trending / popular / etc series and movies, I'm moving the endpoints as static functions of the individual resources.
…vie endpoints I experimented with using a dependency injection context so I can set the TraktManager once and have access to the instance from Route without having to pass the TraktManager through each `Resource` class, but with Swift concurrency and tests I found this to be not too safe. You would expect that the `traktManager` instance you created a resource with would perform the request so I removed it. I am still working on the architecture for the resource classes. I was going to use static functions within a Resource for functions that didn't need a specific shared id for the requests, such as series id for shows, or a user id for User requests, but have moved to using a separate resource struct so I can pass in the TraktManager to those endpoints. It's better than having `traktManager.resource.endpoint(traktManager` or anything odd like that.
…ch page with structured concurrency
…nd API customization Since the first version of TraktKit authentication was handled automatically by storing the `access_token` and `refresh_token` in the keychain, and the token expiration date in UserDefaults. This was good because consumers of the API didn't have to really worry about handling that, but it also wasn't very testable, and if you wanted to use your own storage or a different keychain implementation, you couldn't without forking the repo and changing it. This commit changes that by extracting the authentication handling from `TraktManager` into its own protocol `TraktAuthentication`, with a concrete implementation that matches the existing behavior `KeychainTraktAuthentication`. There is even a mock implementation now `TraktMockAuthStorage` that can be used for testing without overwriting your credentials in the keychain. You will now need to call `TraktManager.refreshCurrentAuthState()`, or use the async initializer before making requests to ensure `TraktManager` has the latest `access_token` if making authenticated requests.
Removing uses of option and force `try`, replacing uses of `decode` with `decodeIfPresent`, Removing redundant or unused code.
I am working on reducing the number of `performRequest` variations so that there is less room for error, and for the request handling to become outdated for a specific type. I realized that `ObjectsCompletionHandler` was redundant when I could use `ObjectCompletionHandler<[T]>`
The `/users/*/watching` endpoints returns data if a user is currently watching something, or 204 with no data if the user is not watching anything. Rather than having a custom request handler for this, I've updated the Swift model that is decoded from the data to handle no data being returned. Since both responses have a 2xx response, an error is not thrown unless the data could not be decoded.
Removed the custom request handling for `TraktCheckin`. Unfortunately because the `/checkin` endpoint returns an expiration date with the status code `409`, the response handling that happens before attempting to decode the data will throw an error. Since this appears to be the only endpoint that returns alternate data for a non-2xx response that I can think of, and because you can get the same data from `/users/*/watching`, I figured this will be ok. You can check if the error thrown is `TraktManager.TraktError.resourceAlreadyCreated` to make that request and get the end date of a checkin.
I set the cache policy to reloadIgnoringLocalAndRemoteCacheData because I was getting reports by users that data wasn't being updated. But after research into URLSession caching, and the headers sent by Trakt, the sync endpoints should be getting new data every time, while other requests are cached, so I'm removing this cache policy.
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.
No description provided.