4

I am trying to learn Combine and it is a PITA for me. I never learned RX Swift, so this is all new to me. I am sure I am missing something simple with this one, but hoping for some help.

I am trying to fetch some JSON from an API and load it in a List view. I have a view model that conforms to ObservableObject and updates a @Published property which is an array. I use that VM to load my list, but it looks like the view loads way before this API returns (List showing up blank). I was hoping these property wrappers would do what I thought they were supposed to and re-render the view whenever the object changed.

Like I said, I am sure I am missing something simple. If you can spot it, I would love the help. Thanks!

class PhotosViewModel: ObservableObject {

    var cancellable: AnyCancellable?

    @Published var photos = Photos()

    func load(user collection: String) {
        guard let url = URL(string: "https://api.unsplash.com/users/\(collection)/collections?client_id=\(Keys.unsplashAPIKey)") else {
            return
        }
        cancellable = URLSession.shared.dataTaskPublisher(for: url)
            .map { $0.data }
            .decode(type: Photos.self, decoder: JSONDecoder())
            .replaceError(with: defaultPhotosObject)
            .receive(on: RunLoop.main)
            .assign(to: \.photos, on: self)
    }

}
struct PhotoListView: View {
    @EnvironmentObject var photosViewModel: PhotosViewModel
    var body: some View {
        NavigationView {
            List(photosViewModel.photos) { photo in
                NavigationLink(destination: PhotoDetailView(photo)) {
                    PhotoRow(photo)
                }
            }.navigationBarTitle("Photos")
        }
    }
}
struct PhotoRow: View {
    var photo: Photo
    init(_ photo: Photo) {
        self.photo = photo
    }
    var body: some View {
        HStack {
            ThumbnailImageLoadingView(photo.coverPhoto.urls.thumb)
            VStack(alignment: .leading) {
                Text(photo.title)
                    .font(.headline)
                Text(photo.user.firstName)
                    .font(.body)
            }
            .padding(.leading, 5)
        }
        .padding(5)
    }
}
4
  • 1
    I don't see anything wrong with provided code, so problem is not there. Two questions: 1) where do you instantiate PhotosViewModel? 2) where do you call load? ... though I'm not sure about this .replaceError(with: []) - have you tested if API returned correct data? Commented Dec 27, 2019 at 19:17
  • Thanks @Asperi. So, because the API wasn't returning fast enough, I am instantiating the VM in the SceneDelegate and calling load() there too. At first, I was doing both in the List view. Will get back to you on replaceError Commented Dec 27, 2019 at 19:30
  • Good call, I used QuickType to write my model and it looks like something went wrong there. I will have to do some manual work on that. I will update the post when I can fix it! Commented Dec 27, 2019 at 19:47
  • What is the Photos class? It's possible the photos property, if it is a reference, may not change when you make the assign in the network publisher (the values it points to will change, but that doesn't trigger a redraw). Try making it just an Array of photos and see what happens, not a published object. Commented Dec 28, 2019 at 16:00

2 Answers 2

3

Based on your updated solution, here are some improvement suggestions (that wont fit in a comment).

PhotosViewModel improvement suggestions

Might I just make the recommendation of changing your load function from returning Void (i.e. returning nothing), to be returning AnyPublisher<Photos, Never> and skipping the last step .assign(to:on:).

One advantage of this is that your code takes one step toward being testable.

Instead of replaceError with some default value you can use catch together with Empty(completeImmediately: <TRUE/FALSE>). Because is it always possible to come up with any relevant default value? Maybe in this case? Maybe "empty photos"? If so then you can either make Photos conform to ExpressibleByArrayLiteral and use replaceError(with: []) or you can create a static var named empty, allowing for replaceError(with: .empty).

To sum up my suggestions in a code block:

public class PhotosViewModel: ObservableObject {

    @Published var photos = Photos()

    // var cancellable: AnyCancellable? -> change to Set<AnyCancellable>
    private var cancellables = Set<AnyCancellable>()
    private let urlSession: URLSession

    public init(urlSession: URLSession = .init()) {
        self.urlSession = urlSession
    }
}

private extension PhotosViewModel {}
    func populatePhotoCollection(named nameOfPhotoCollection: String) {
        fetchPhotoCollection(named: nameOfPhotoCollection)
            .assign(to: \.photos, on: self)
            .store(in: &cancellables)
    }

    func fetchPhotoCollection(named nameOfPhotoCollection: String) -> AnyPublisher<Photos, Never> {
        func emptyPublisher(completeImmediately: Bool = true) -> AnyPublisher<Photos, Never> {
            Empty<Photos, Never>(completeImmediately: completeImmediately).eraseToAnyPublisher()
        }

        // This really ought to be moved to some APIClient
        guard let url = URL(string: "https://api.unsplash.com/users/\(collection)/collections?client_id=\(Keys.unsplashAPIKey)") else {
            return emptyPublisher()
        }

        return urlSession.dataTaskPublisher(for: url)
            .map { $0.data }
            .decode(type: Photos.self, decoder: JSONDecoder())
            .catch { error -> AnyPublisher<Photos, Never> in
                print("☣️ error decoding: \(error)")
                return emptyPublisher()
            }
            .receive(on: RunLoop.main)
            .eraseToAnyPublisher()
    }
}

*Client suggestion

You might want to write some kind of HTTPClient/APIClient/RESTClient and take a look at HTTP Status codes.

This is a highly modular (and one might argue - overly engineered) solution using a DataFetcher and a DefaultHTTPClient conforming to a HTTPClient protocol:

DataFetcher

public final class DataFetcher {

    private let dataFromRequest:  (URLRequest) -> AnyPublisher<Data, HTTPError.NetworkingError>
    public init(dataFromRequest: @escaping  (URLRequest) -> AnyPublisher<Data, HTTPError.NetworkingError>) {
        self.dataFromRequest = dataFromRequest
    }
}

public extension DataFetcher {
    func fetchData(request: URLRequest) -> AnyPublisher<Data, HTTPError.NetworkingError> {
        dataFromRequest(request)
    }
}

// MARK: Convenience init
public extension DataFetcher {

    static func urlResponse(
        errorMessageFromDataMapper: ErrorMessageFromDataMapper,
        headerInterceptor: (([AnyHashable: Any]) -> Void)?,
        badStatusCodeInterceptor: ((UInt) -> Void)?,
        _ dataAndUrlResponsePublisher: @escaping (URLRequest) -> AnyPublisher<(data: Data, response: URLResponse), URLError>
    ) -> DataFetcher {

        DataFetcher { request in
            dataAndUrlResponsePublisher(request)
                .mapError { HTTPError.NetworkingError.urlError($0) }
                .tryMap { data, response -> Data in
                    guard let httpResponse = response as? HTTPURLResponse else {
                        throw HTTPError.NetworkingError.invalidServerResponse(response)
                    }

                    headerInterceptor?(httpResponse.allHeaderFields)

                    guard case 200...299 = httpResponse.statusCode else {

                        badStatusCodeInterceptor?(UInt(httpResponse.statusCode))

                        let dataAsErrorMessage = errorMessageFromDataMapper.errorMessage(from: data) ?? "Failed to decode error from data"
                        print("⚠️ bad status code, error message: <\(dataAsErrorMessage)>, httpResponse: `\(httpResponse.debugDescription)`")
                        throw HTTPError.NetworkingError.invalidServerStatusCode(httpResponse.statusCode)
                    }
                    return data
            }
            .mapError { castOrKill(instance: $0, toType: HTTPError.NetworkingError.self) }
            .eraseToAnyPublisher()

        }
    }

    // MARK: From URLSession
    static func usingURLSession(
        errorMessageFromDataMapper: ErrorMessageFromDataMapper,
        headerInterceptor: (([AnyHashable: Any]) -> Void)?,
        badStatusCodeInterceptor: ((UInt) -> Void)?,
        urlSession: URLSession = .shared
    ) -> DataFetcher {

        .urlResponse(
            errorMessageFromDataMapper: errorMessageFromDataMapper,
            headerInterceptor: headerInterceptor,
            badStatusCodeInterceptor: badStatusCodeInterceptor
        ) { urlSession.dataTaskPublisher(for: $0).eraseToAnyPublisher() }
    }
}

HTTPClient

public final class DefaultHTTPClient {
    public typealias Error = HTTPError

    public let baseUrl: URL

    private let jsonDecoder: JSONDecoder
    private let dataFetcher: DataFetcher

    private var cancellables = Set<AnyCancellable>()

    public init(
        baseURL: URL,
        dataFetcher: DataFetcher,
        jsonDecoder: JSONDecoder = .init()
    ) {
        self.baseUrl = baseURL
        self.dataFetcher = dataFetcher
        self.jsonDecoder = jsonDecoder
    }
}

// MARK: HTTPClient
public extension DefaultHTTPClient {

    func perform(absoluteUrlRequest urlRequest: URLRequest) -> AnyPublisher<Data, HTTPError.NetworkingError> {
        return Combine.Deferred {
            return Future<Data, HTTPError.NetworkingError> { [weak self] promise in

                guard let self = self else {
                    promise(.failure(.clientWasDeinitialized))
                    return
                }

                self.dataFetcher.fetchData(request: urlRequest)

                    .sink(
                        receiveCompletion: { completion in
                            guard case .failure(let error) = completion else { return }
                            promise(.failure(error))
                    },
                        receiveValue: { data in
                            promise(.success(data))
                    }
                ).store(in: &self.cancellables)
            }
        }.eraseToAnyPublisher()
    }

    func performRequest(pathRelativeToBase path: String) -> AnyPublisher<Data, HTTPError.NetworkingError> {
        let url = URL(string: path, relativeTo: baseUrl)!
        let urlRequest = URLRequest(url: url)
        return perform(absoluteUrlRequest: urlRequest)
    }

    func fetch<D>(urlRequest: URLRequest, decodeAs: D.Type) -> AnyPublisher<D, HTTPError> where D: Decodable {
        return perform(absoluteUrlRequest: urlRequest)
            .mapError { print("☢️ got networking error: \($0)"); return castOrKill(instance: $0, toType: HTTPError.NetworkingError.self) }
            .mapError { HTTPError.networkingError($0) }
            .decode(type: D.self, decoder: self.jsonDecoder)
            .mapError { print("☢️ 🚨 got decoding error: \($0)"); return castOrKill(instance: $0, toType: DecodingError.self) }
            .mapError { Error.serializationError(.decodingError($0)) }
            .eraseToAnyPublisher()
    }

}

Helpers

public protocol ErrorMessageFromDataMapper {
    func errorMessage(from data: Data) -> String?
}


public enum HTTPError: Swift.Error {
    case failedToCreateRequest(String)
    case networkingError(NetworkingError)
    case serializationError(SerializationError)
}

public extension HTTPError {
    enum NetworkingError: Swift.Error {
        case urlError(URLError)
        case invalidServerResponse(URLResponse)
        case invalidServerStatusCode(Int)
        case clientWasDeinitialized
    }

    enum SerializationError: Swift.Error {
        case decodingError(DecodingError)
        case inputDataNilOrZeroLength
        case stringSerializationFailed(encoding: String.Encoding)
    }
}

internal func castOrKill<T>(
    instance anyInstance: Any,
    toType expectedType: T.Type,
    _ file: String = #file,
    _ line: Int = #line
) -> T {

    guard let instance = anyInstance as? T else {
        let incorrectTypeString = String(describing: Mirror(reflecting: anyInstance).subjectType)
        fatalError("Expected variable '\(anyInstance)' (type: '\(incorrectTypeString)') to be of type `\(expectedType)`, file: \(file), line:\(line)")
    }
    return instance
}

Sign up to request clarification or add additional context in comments.

2 Comments

Thanks @Sajjon! I love both suggestions and will want to use that in the future.
@christinam wanna give me an upvote and/or accepter answer? :)
1

This ended up being an issue with my Codable struct not being set up properly. Once I added a default object in the .replaceError method instead of a blank array (Thanks @Asperi), I was able to see the decoding error and fix it. Works like a charm now!

Original:

    func load(user collection: String) {
        guard let url = URL(string: "https://api.unsplash.com/users/\(collection)/collections?client_id=\(Keys.unsplashAPIKey)") else {
            return
        }
        cancellable = URLSession.shared.dataTaskPublisher(for: url)
            .map { $0.data }
            .decode(type: Photos.self, decoder: JSONDecoder())
            .replaceError(with: [])
            .receive(on: RunLoop.main)
            .assign(to: \.photos, on: self)
    }

Updated:

    func load(user collection: String) {
        guard let url = URL(string: "https://api.unsplash.com/users/\(collection)/collections?client_id=\(Keys.unsplashAPIKey)") else {
            return
        }
        cancellable = URLSession.shared.dataTaskPublisher(for: url)
            .map { $0.data }
            .decode(type: Photos.self, decoder: JSONDecoder())
            .replaceError(with: defaultPhotosObject)
            .receive(on: RunLoop.main)
            .assign(to: \.photos, on: self)
    }

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.