Introduce Swift-only request APIs

[kmcbride]

These changes take the idea behind SPTDataLoaderBlockWrapper.m and expand upon it to provide protocol-oriented APIs that better utilize Swift language features.

Example Usage

struct Album: Decodable {
    let title: String
    let artistName: String
}

let dataLoader = dataLoaderFactory.makeDataLoader()
let request = SPTDataLoaderRequest(url: albumURL, sourceIdentifier: "album-feature")

dataLoader.request(request) { (response: Response<Album, Error>) in
    switch response.result {
    case .success(let album):
        print("Loaded album: \(album.title) by \(album.artistName)"
    case .failure(let error):
        print("Failed to load album: \(error)")
    }
}

Adding Custom Response Serializer

import SwiftProtobuf

struct ProtobufResponseSerializer<Message: SwiftProtobuf.Message>: ResponseSerializer {
    func serialize(response: SPTDataLoaderResponse) throws -> Message {
        if let error = response.error {
            throw error
        }

        return try Message(serializedData: response.body ?? Data())
    }
}

extension DataLoader {
    @discardableResult
    func request<Message: SwiftProtobuf.Message>(
        _ request: SPTDataLoaderRequest,
        completionHandler: @escaping (Response<Message, Error>) -> Void
    ) -> SPTDataLoaderCancellationToken? {
        let serializer = ProtobufResponseSerializer<Message>()

        return self.request(request, serializer: serializer, completionHandler: completionHandler)
    }
}

[rastersize]

@kmcbride One opportunity of improvement I see with the request-handler closure is to expose the HTTP response. So that the closure can for example look at the HTTP status code. I find that quite useful form time to time. For example when distinguishing between a network error and a server error. Or when deciding how to handle an error, like differentiating between a HTTP 403 and 500 status code.

Also, have you considered adding a method for parsing server errors? E.g. something like:

dataLoader
    .validate(ServerError.self)
    .request(request) { (response: Response<Album, ServerError>) in ... }

// Or
dataLoader.request(validateUsing: Validator.self, request) { (response: Response<Album, ServerError>) in ... }

// Or
dataLoader.request(validateUsing: validator, request) { (response: Response<Album, ServerError>) in ... }

The handler closure would still need to handle Error though, as we could be getting a network error 🤔

[kmcbride]

@rastersize I like the idea of being able to chain a validator and I did explore it at one point, but it seemed like it would require refactoring internals beyond what could be provided by a simple wrapper. Including it as a parameter to the request method is a possibility, but would cause a bit of API bloat due to the inability to use default values in the protocol. In the interim, I've added an additional request(…) function that provides the raw SPTDataLoaderResponse to the completion handler. It's also possible to provide a custom serializer that makes use of the response object to determine what result to provide.

[rastersize]

@kmcbride @dflems Btw. since we’re adding Swift overlays to SPTDataLoader, should we also add our .swiftlint.yml config and run it in CI?

[rastersize]

@kmcbride I think this looks good to merge. The only thing left, in my mind, is to fix the Codecov issue. Which I think could be fixed, or at least partially fixed, by updating the codecov.yml file to include the new tests directory.

[dflems]

@rastersize The codecov file only has ignores not paths to look for code. Something is broken on their end.

@kmcbride If you give me write access to your fork, I can fiddle with this locally.

[rastersize]

My comment was a bit confusing, I meant to add the new tests directory to the exclude list. I was hoping that would get Codecov to pick things up. If nothing else it should get rid of the warnings added to the test files.

[kmcbride]

@rastersize @dflems I've somewhat fixed the codecov issues:

1. I corrected the codecov.yml because the tests directory stopped being ignored quite a while ago 🙂
2. I ran the script locally in order to upload the correct coverage — for some reason the line hit counts are reported as 0 from the CI build

[codecov]

Codecov Report

Merging #194 into master will increase coverage by 0.36%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #194      +/-   ##
==========================================
+ Coverage   94.03%   94.39%   +0.36%     
==========================================
  Files          17       21       +4     
  Lines        1039     1106      +67     
==========================================
+ Hits          977     1044      +67     
  Misses         62       62              

Flag	Coverage Δ
#ios	94.39% <100.00%> (+0.36%)	⬆️
#macos	94.39% <100.00%> (+0.36%)	⬆️
#tvos	94.39% <100.00%> (+0.36%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
Sources/SPTDataLoader/NSDictionary+HeaderSize.m	100.00% <ø> (ø)
Sources/SPTDataLoader/SPTDataLoader.m	95.45% <ø> (ø)
Sources/SPTDataLoader/SPTDataLoaderBlockWrapper.m	100.00% <ø> (ø)
...DataLoaderCancellationTokenFactoryImplementation.m	100.00% <ø> (ø)
...der/SPTDataLoaderCancellationTokenImplementation.m	100.00% <ø> (ø)
...rces/SPTDataLoader/SPTDataLoaderExponentialTimer.m	100.00% <ø> (ø)
Sources/SPTDataLoader/SPTDataLoaderFactory.m	95.65% <ø> (ø)
Sources/SPTDataLoader/SPTDataLoaderRateLimiter.m	98.33% <ø> (ø)
Sources/SPTDataLoader/SPTDataLoaderRequest.m	100.00% <ø> (ø)
...es/SPTDataLoader/SPTDataLoaderRequestTaskHandler.m	99.01% <ø> (ø)
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1b6127...6061069. Read the comment docs.