diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index aa2d288..14b7866 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -5,6 +5,7 @@ on: branches: [ "main" ] pull_request: branches: [ "main" ] + workflow_dispatch: jobs: build: diff --git a/PayForMe.xcodeproj/project.pbxproj b/PayForMe.xcodeproj/project.pbxproj index e5acab8..cf7dad7 100644 --- a/PayForMe.xcodeproj/project.pbxproj +++ b/PayForMe.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + C34C21CC3AD72833B6C06C9D /* LoadErrorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD167A293B2F0746C0A09C2E /* LoadErrorTests.swift */; }; 481FB4FB23E964C8003BD108 /* ProjectManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 481FB4FA23E964C8003BD108 /* ProjectManager.swift */; }; 481FB4FD23EAD78F003BD108 /* AddProjectManualViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 481FB4FC23EAD78F003BD108 /* AddProjectManualViewModel.swift */; }; 489995DA23F6EC6F008B7E38 /* OnboardingView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 489995D923F6EC6F008B7E38 /* OnboardingView.swift */; }; @@ -88,6 +89,7 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + CD167A293B2F0746C0A09C2E /* LoadErrorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadErrorTests.swift; sourceTree = ""; }; 0865D2DAF541B24078D1DC41 /* TestHelpers.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = TestHelpers.swift; sourceTree = ""; }; 143F41EE78329905894C8A29 /* BalanceCalculationTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = BalanceCalculationTests.swift; sourceTree = ""; }; 316FBA7C0CE4ABB51EC2A121 /* NetworkRequestTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = NetworkRequestTests.swift; sourceTree = ""; }; @@ -228,6 +230,7 @@ isa = PBXGroup; children = ( 654754E32528B29F00A82EB6 /* AddProjectManuallyTests.swift */, + CD167A293B2F0746C0A09C2E /* LoadErrorTests.swift */, 654754E52528B29F00A82EB6 /* Info.plist */, 6523FC4B25580EEF00BCD843 /* UrlExtensionsTests.swift */, 0865D2DAF541B24078D1DC41 /* TestHelpers.swift */, @@ -544,6 +547,7 @@ buildActionMask = 2147483647; files = ( 654754E42528B29F00A82EB6 /* AddProjectManuallyTests.swift in Sources */, + C34C21CC3AD72833B6C06C9D /* LoadErrorTests.swift in Sources */, 6523FC4C25580EEF00BCD843 /* UrlExtensionsTests.swift in Sources */, 57BC8D25B91CE40F4669650C /* TestHelpers.swift in Sources */, D279295CFA25D9CC69C6F27C /* BillTests.swift in Sources */, diff --git a/PayForMe/Services/NetworkService.swift b/PayForMe/Services/NetworkService.swift index 42ba278..09cf79a 100644 --- a/PayForMe/Services/NetworkService.swift +++ b/PayForMe/Services/NetworkService.swift @@ -8,6 +8,49 @@ import Combine import Foundation +/// Why loading a project's data failed. +/// +/// The load publishers used to declare `Never` as their failure type, which +/// left everything upstream unable to tell "this project is empty" from "the +/// server rejected us". Changing a project's password on the server therefore +/// showed empty lists and no explanation at all (#37). +enum LoadError: Error, Equatable, Identifiable { + /// Credentials rejected. Usually the project password was changed on the + /// server after it was added here. + case unauthorized + case notFound + case http(Int) + /// No usable connection, or a response that was not HTTP at all. + case connection + /// The server answered, but not with something we could decode. + case invalidResponse + + var id: String { String(describing: self) } + + init(statusCode: Int) { + switch statusCode { + case 401, 403: + self = .unauthorized + case 404: + self = .notFound + default: + self = .http(statusCode) + } + } + + init(_ error: Error) { + switch error { + case let serverError as LoadError: + self = serverError + case is URLError: + self = .connection + default: + // Decoding failed: the server answered, we just could not read it. + self = .invalidResponse + } + } +} + class NetworkService { static let shared = NetworkService() @@ -34,16 +77,25 @@ class NetworkService { let networkActivityPublisher = PassthroughSubject() - func loadBillsPublisher(_ project: Project) -> AnyPublisher<[Bill], Never> { + /// Turns a response into its body or into a typed failure. Both cases used + /// to collapse into "no value emitted", which is precisely what made a + /// rejected password indistinguishable from an empty project. + private static func validate(_ data: Data, _ response: URLResponse) throws -> Data { + guard let httpResponse = response as? HTTPURLResponse else { + throw LoadError.connection + } + guard httpResponse.statusCode == 200 else { + throw LoadError(statusCode: httpResponse.statusCode) + } + return data + } + + func loadBillsPublisher(_ project: Project) -> AnyPublisher<[Bill], LoadError> { let request = buildURLRequest("bills", params: [:], project: project) return URLSession.shared.dataTaskPublisher(for: request) - .compactMap { data, response -> Data? in - guard let httpResponse = response as? HTTPURLResponse else { print("Network Error"); return nil } - guard httpResponse.statusCode == 200 else { print("Network Error: Status code: \(httpResponse.statusCode) \(httpResponse.description)"); return nil } - return data - } + .tryMap { data, response in try NetworkService.validate(data, response) } .decode(type: [Bill].self, decoder: decoder) - .replaceError(with: []) + .mapError { LoadError($0) } .map { $0.sorted { if let l1 = $0.lastchanged, @@ -57,16 +109,12 @@ class NetworkService { .eraseToAnyPublisher() } - func loadMembersPublisher(_ project: Project) -> AnyPublisher<[Int: Person], Never> { + func loadMembersPublisher(_ project: Project) -> AnyPublisher<[Int: Person], LoadError> { let request = buildURLRequest("members", params: [:], project: project) return URLSession.shared.dataTaskPublisher(for: request) - .compactMap { data, response -> Data? in - guard let httpResponse = response as? HTTPURLResponse else { print("Network Error"); return nil } - guard httpResponse.statusCode == 200 else { print("Network Error: Status code: \(httpResponse.statusCode) \(httpResponse.description)"); return nil } - return data - } + .tryMap { data, response in try NetworkService.validate(data, response) } .decode(type: [Person].self, decoder: decoder) - .replaceError(with: []) + .mapError { LoadError($0) } .map { members in let filtered = members.filter { diff --git a/PayForMe/Services/ProjectManager.swift b/PayForMe/Services/ProjectManager.swift index a8b474c..ace5144 100644 --- a/PayForMe/Services/ProjectManager.swift +++ b/PayForMe/Services/ProjectManager.swift @@ -26,6 +26,11 @@ class ProjectManager: ObservableObject { @Published var openedByURL: URL? + /// The last failure while loading bills and members, or nil after a load + /// that worked. Set so the UI can say something instead of showing lists + /// that are empty for no visible reason (#37). + @Published var loadingError: LoadError? + private init() { print("init") projects = storageService.loadProjects() @@ -80,8 +85,15 @@ class ProjectManager: ObservableObject { .receive(on: DispatchQueue.main) .handleEvents(receiveCancel: invokeCompletionOnce) .sink( - receiveCompletion: { _ in invokeCompletionOnce() }, + receiveCompletion: { [weak self] result in + if case let .failure(error) = result { + self?.loadingError = error + } + invokeCompletionOnce() + }, receiveValue: { [weak self] project in + // A load that worked clears whatever went wrong last time. + self?.loadingError = nil self?.currentProject = project invokeCompletionOnce() } diff --git a/PayForMe/Strings/cs.lproj/Localizable.strings b/PayForMe/Strings/cs.lproj/Localizable.strings index 7bbb911..8672b08 100644 --- a/PayForMe/Strings/cs.lproj/Localizable.strings +++ b/PayForMe/Strings/cs.lproj/Localizable.strings @@ -150,3 +150,10 @@ "member_error_forbidden" = "To obvykle vyžaduje práva správce projektu. Sdílené heslo projektu může umožňovat jen přístup účastníka."; "member_error_network" = "Není spojení se serverem. Zkontrolujte síť a zkuste to znovu."; "member_error_generic" = "Server odmítl požadavek (HTTP %d)."; + +"Could not load project" = "Projekt se nepodařilo načíst"; +"load_error_unauthorized" = "Server odmítl uložené heslo. Pokud se heslo projektu změnilo, přidej projekt znovu s novým heslem."; +"load_error_not_found" = "Server tento projekt nezná. Možná byl smazán nebo je špatné ID projektu."; +"load_error_connection" = "Žádné spojení se serverem. Zkontroluj síť a zkus to znovu."; +"load_error_invalid_response" = "Odpověď serveru nelze přečíst. Možná nejde o server Cospend ani iHateMoney."; +"load_error_generic" = "Server projekt neodeslal (HTTP %d)."; diff --git a/PayForMe/Strings/de.lproj/Localizable.strings b/PayForMe/Strings/de.lproj/Localizable.strings index 0c4cfdb..8d685d3 100644 --- a/PayForMe/Strings/de.lproj/Localizable.strings +++ b/PayForMe/Strings/de.lproj/Localizable.strings @@ -154,3 +154,10 @@ "member_error_forbidden" = "Für den Erfolg sind meist Maintainer-Rechte am Projekt nötig. Du kannst diese Berechtigungen jederzeit erteilen, indem du die Einstellungen des Freigabelinks in Cospend (deiner Nextcloud-App) änderst."; "member_error_network" = "Keine Verbindung zum Server. Bitte Netzwerk prüfen und erneut versuchen."; "member_error_generic" = "Der Server hat die Anfrage abgelehnt (HTTP %d)."; + +"Could not load project" = "Projekt konnte nicht geladen werden"; +"load_error_unauthorized" = "Der Server hat das gespeicherte Passwort abgelehnt. Wurde das Projektpasswort geändert, füge das Projekt mit dem neuen Passwort erneut hinzu."; +"load_error_not_found" = "Der Server kennt dieses Projekt nicht. Möglicherweise wurde es gelöscht oder die Projekt-ID stimmt nicht."; +"load_error_connection" = "Keine Verbindung zum Server. Bitte prüfe dein Netzwerk und versuche es erneut."; +"load_error_invalid_response" = "Die Antwort des Servers war nicht lesbar. Möglicherweise ist es kein Cospend- oder iHateMoney-Server."; +"load_error_generic" = "Der Server hat das Projekt nicht herausgegeben (HTTP %d)."; diff --git a/PayForMe/Strings/en.lproj/Localizable.strings b/PayForMe/Strings/en.lproj/Localizable.strings index 8fc28df..5c73bef 100644 --- a/PayForMe/Strings/en.lproj/Localizable.strings +++ b/PayForMe/Strings/en.lproj/Localizable.strings @@ -157,3 +157,10 @@ "member_error_forbidden" = "This usually requires maintainer rights for the project. You can grant those at any time by changing the share-link's settings in Cospend (your Nextcloud-App)."; "member_error_network" = "No connection to the server. Please check your network and try again."; "member_error_generic" = "The server rejected the request (HTTP %d)."; + +"Could not load project" = "Could not load project"; +"load_error_unauthorized" = "The server rejected the stored password. If the project password was changed, add the project again with the new one."; +"load_error_not_found" = "The server does not know this project. It may have been deleted, or the project id may be wrong."; +"load_error_connection" = "No connection to the server. Please check your network and try again."; +"load_error_invalid_response" = "The server's answer could not be read. It may not be a Cospend or iHateMoney server."; +"load_error_generic" = "The server refused to send the project (HTTP %d)."; diff --git a/PayForMe/Strings/es.lproj/Localizable.strings b/PayForMe/Strings/es.lproj/Localizable.strings index c297e8d..fd50c45 100644 --- a/PayForMe/Strings/es.lproj/Localizable.strings +++ b/PayForMe/Strings/es.lproj/Localizable.strings @@ -157,3 +157,10 @@ "Enter invite link" = "Introducir el enlace de invitación"; "invite_link_hint" = "Pega el enlace de invitación completo. El servidor y el proyecto se detectan automáticamente."; "Server returned an invalid HTTP response" = "El servidor devolvió una respuesta HTTP no válida"; + +"Could not load project" = "No se pudo cargar el proyecto"; +"load_error_unauthorized" = "El servidor rechazó la contraseña guardada. Si se cambió la contraseña del proyecto, añádelo de nuevo con la nueva."; +"load_error_not_found" = "El servidor no conoce este proyecto. Puede que se haya eliminado o que el identificador sea incorrecto."; +"load_error_connection" = "Sin conexión con el servidor. Comprueba tu red e inténtalo de nuevo."; +"load_error_invalid_response" = "No se pudo leer la respuesta del servidor. Puede que no sea un servidor Cospend o iHateMoney."; +"load_error_generic" = "El servidor se negó a enviar el proyecto (HTTP %d)."; diff --git a/PayForMe/Strings/fr.lproj/Localizable.strings b/PayForMe/Strings/fr.lproj/Localizable.strings index 3f52860..11677eb 100644 --- a/PayForMe/Strings/fr.lproj/Localizable.strings +++ b/PayForMe/Strings/fr.lproj/Localizable.strings @@ -157,3 +157,10 @@ "Enter invite link" = "Saisir le lien d'invitation"; "invite_link_hint" = "Collez le lien d'invitation complet. Le serveur et le projet sont détectés automatiquement."; "Server returned an invalid HTTP response" = "Le serveur a renvoyé une réponse HTTP non valide"; + +"Could not load project" = "Impossible de charger le projet"; +"load_error_unauthorized" = "Le serveur a refusé le mot de passe enregistré. Si le mot de passe du projet a changé, ajoutez à nouveau le projet avec le nouveau."; +"load_error_not_found" = "Le serveur ne connaît pas ce projet. Il a peut-être été supprimé, ou l'identifiant est incorrect."; +"load_error_connection" = "Pas de connexion au serveur. Vérifiez votre réseau et réessayez."; +"load_error_invalid_response" = "La réponse du serveur est illisible. Ce n'est peut-être pas un serveur Cospend ou iHateMoney."; +"load_error_generic" = "Le serveur a refusé d'envoyer le projet (HTTP %d)."; diff --git a/PayForMe/Strings/ru.lproj/Localizable.strings b/PayForMe/Strings/ru.lproj/Localizable.strings index 985ec73..04d276e 100644 --- a/PayForMe/Strings/ru.lproj/Localizable.strings +++ b/PayForMe/Strings/ru.lproj/Localizable.strings @@ -156,3 +156,10 @@ "member_error_forbidden" = "Обычно для этого нужны права сопровождающего проекта. Пароль общего доступа может давать только права участника."; "member_error_network" = "Нет соединения с сервером. Проверьте сеть и повторите попытку."; "member_error_generic" = "Сервер отклонил запрос (HTTP %d)."; + +"Could not load project" = "Не удалось загрузить проект"; +"load_error_unauthorized" = "Сервер отклонил сохранённый пароль. Если пароль проекта изменился, добавьте проект заново с новым паролем."; +"load_error_not_found" = "Сервер не знает этот проект. Возможно, он удалён или указан неверный идентификатор."; +"load_error_connection" = "Нет соединения с сервером. Проверьте сеть и попробуйте снова."; +"load_error_invalid_response" = "Не удалось прочитать ответ сервера. Возможно, это не сервер Cospend или iHateMoney."; +"load_error_generic" = "Сервер отказался передать проект (HTTP %d)."; diff --git a/PayForMe/Views/ContentView.swift b/PayForMe/Views/ContentView.swift index 829ff1e..f75e80a 100644 --- a/PayForMe/Views/ContentView.swift +++ b/PayForMe/Views/ContentView.swift @@ -46,6 +46,28 @@ struct ContentView: View { manager.loadBillsAndMembers() } } + // Without this the app just shows empty lists when the server turns us + // away — the complaint behind #37. + .alert(item: $manager.loadingError) { error in + Alert(title: Text("Could not load project"), + message: Text(Self.message(for: error)), + dismissButton: .default(Text("OK"))) + } + } + + static func message(for error: LoadError) -> String { + switch error { + case .unauthorized: + return NSLocalizedString("load_error_unauthorized", comment: "Loading failed because the credentials were rejected") + case .notFound: + return NSLocalizedString("load_error_not_found", comment: "Loading failed because the project does not exist on the server") + case .connection: + return NSLocalizedString("load_error_connection", comment: "Loading failed because the server could not be reached") + case .invalidResponse: + return NSLocalizedString("load_error_invalid_response", comment: "Loading failed because the server's answer could not be read") + case let .http(code): + return String(format: NSLocalizedString("load_error_generic", comment: "Loading failed with an HTTP status code"), code) + } } var tabBar: some View { diff --git a/PayForMeTests/LoadErrorTests.swift b/PayForMeTests/LoadErrorTests.swift new file mode 100644 index 0000000..d5fcd6f --- /dev/null +++ b/PayForMeTests/LoadErrorTests.swift @@ -0,0 +1,137 @@ +// +// LoadErrorTests.swift +// PayForMeTests +// +// Changing a project's password on the server left the app showing empty +// lists and no explanation — #37. The cause was structural: both load +// publishers declared `Never` as their failure type, so every failure had to +// be turned into a value before it could leave NetworkService. An HTTP 401 was +// dropped by a `compactMap` and a URLError became `[]`, which is exactly what +// a project with no members looks like. +// +// These tests pin down that a failure now reaches the caller as a failure. +// + +import Combine +import XCTest +@testable import PayForMe + +final class LoadErrorTests: XCTestCase { + + private var subscriptions = Set() + private var savedProject: Project! + + override func setUp() { + super.setUp() + URLProtocol.registerClass(MockURLProtocol.self) + MockURLProtocol.reset() + savedProject = ProjectManager.shared.currentProject + } + + override func tearDown() { + URLProtocol.unregisterClass(MockURLProtocol.self) + subscriptions.removeAll() + ProjectManager.shared.currentProject = savedProject + super.tearDown() + } + + /// Runs a load against a stubbed response and returns how it ended. + private func failure(loading load: (Project) -> AnyPublisher<[Bill], LoadError>, + status: Int, + body: String = "[]", + file: StaticString = #filePath, + line: UInt = #line) -> LoadError? { + MockURLProtocol.requestHandler = { request in + (.status(status, for: request.url!), Data(body.utf8)) + } + + var received: LoadError? + let exp = expectation(description: "publisher finished") + load(Project.makeCospend()) + .sink(receiveCompletion: { completion in + if case let .failure(error) = completion { received = error } + exp.fulfill() + }, receiveValue: { _ in }) + .store(in: &subscriptions) + waitForExpectations(timeout: 2) + return received + } + + // MARK: The case from the issue + + /// The password was changed on the server: Cospend answers 401, and until + /// now that was indistinguishable from a project with no bills. + func testRejectedCredentialsReachTheCaller() { + XCTAssertEqual(failure(loading: NetworkService.shared.loadBillsPublisher, status: 401), + .unauthorized) + } + + func testForbiddenCountsAsRejectedCredentials() { + XCTAssertEqual(failure(loading: NetworkService.shared.loadBillsPublisher, status: 403), + .unauthorized) + } + + // MARK: The other outcomes + + func testAnUnexpectedStatusIsPassedThroughVerbatim() { + XCTAssertEqual(failure(loading: NetworkService.shared.loadBillsPublisher, status: 500), + .http(500)) + } + + /// A 200 whose body is not what we expect — pointing the app at something + /// that is not a Cospend or iHateMoney server. + func testAnUnreadableBodyIsAnInvalidResponse() { + XCTAssertEqual(failure(loading: NetworkService.shared.loadBillsPublisher, + status: 200, body: "hello"), + .invalidResponse) + } + + func testMembersFailTheSameWay() { + MockURLProtocol.requestHandler = { request in + (.status(401, for: request.url!), Data("[]".utf8)) + } + + var received: LoadError? + let exp = expectation(description: "publisher finished") + NetworkService.shared.loadMembersPublisher(Project.makeCospend()) + .sink(receiveCompletion: { completion in + if case let .failure(error) = completion { received = error } + exp.fulfill() + }, receiveValue: { _ in + XCTFail("no members should arrive on 401") + }) + .store(in: &subscriptions) + waitForExpectations(timeout: 2) + XCTAssertEqual(received, .unauthorized) + } + + // MARK: Status-code mapping + + func testStatusCodesMapToTheRightCase() { + XCTAssertEqual(LoadError(statusCode: 401), .unauthorized) + XCTAssertEqual(LoadError(statusCode: 403), .unauthorized) + XCTAssertEqual(LoadError(statusCode: 404), .notFound) + XCTAssertEqual(LoadError(statusCode: 503), .http(503)) + } + + // MARK: What the user is told + + /// Every case needs a message, and it has to be a translated one. A missing + /// entry in Localizable.strings makes NSLocalizedString hand back the key + /// itself, which would ship as "load_error_unauthorized" on screen. + func testEveryErrorHasATranslatedMessage() { + let errors: [LoadError] = [.unauthorized, .notFound, .connection, .invalidResponse, .http(500)] + var messages = Set() + + for error in errors { + let message = ContentView.message(for: error) + XCTAssertFalse(message.isEmpty, "\(error) has no message") + XCTAssertFalse(message.hasPrefix("load_error_"), + "\(error) falls back to the raw key — the string is missing from Localizable.strings") + messages.insert(message) + } + + XCTAssertEqual(messages.count, errors.count, + "each error needs its own message, otherwise they are not worth distinguishing") + } +} diff --git a/PayForMeTests/NetworkRequestTests.swift b/PayForMeTests/NetworkRequestTests.swift index 848687a..c95f3a3 100644 --- a/PayForMeTests/NetworkRequestTests.swift +++ b/PayForMeTests/NetworkRequestTests.swift @@ -77,7 +77,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -98,7 +98,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadMembersPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -116,7 +116,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -136,7 +136,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -156,7 +156,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -176,7 +176,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -194,7 +194,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -264,40 +264,49 @@ class NetworkRequestTests: XCTestCase { // MARK: - loadBills() — graceful empty result on error - func testLoadBills_on404_publisherCompletesWithoutEmittingBills() { + func testLoadBills_on404_failsWithNotFound() { let project = Project.makeCospend() MockURLProtocol.requestHandler = { req in (.notFound(for: req.url!), Data()) } - var didReceiveBills = false - let exp = expectation(description: "publisher completes without emitting (status quo, not ideal)") + var received: LoadError? + let exp = expectation(description: "publisher fails") NetworkService.shared.loadBillsPublisher(project) - .handleEvents(receiveCompletion: { _ in exp.fulfill() }) - .sink { _ in didReceiveBills = true } + .sink(receiveCompletion: { completion in + if case let .failure(error) = completion { received = error } + exp.fulfill() + }, receiveValue: { _ in + XCTFail("no bills should arrive on 404") + }) .store(in: &subscriptions) waitForExpectations(timeout: 2) - XCTAssertFalse(didReceiveBills, - "loadBillsPublisher must NOT emit on 404 — but should when proper feedback is implemented") + XCTAssertEqual(received, .notFound, + "a missing project has to reach the UI, not vanish into an empty list") } - func testLoadMembers_returnsEmptyDictOnNetworkFailure() { + /// This used to emit `[:]`, which the UI could not tell apart from a project + /// that genuinely has no members. + func testLoadMembers_onNetworkFailure_failsWithConnection() { let project = Project.makeCospend() MockURLProtocol.requestHandler = { _ in throw URLError(.timedOut) } - let exp = expectation(description: "empty members received") + var received: LoadError? + let exp = expectation(description: "publisher fails") NetworkService.shared.loadMembersPublisher(project) - .sink { members in - XCTAssertTrue(members.isEmpty, - "loadMembers must return [:] on network failure, not crash") + .sink(receiveCompletion: { completion in + if case let .failure(error) = completion { received = error } exp.fulfill() - } + }, receiveValue: { _ in + XCTFail("no members should arrive when the connection failed") + }) .store(in: &subscriptions) waitForExpectations(timeout: 2) + XCTAssertEqual(received, .connection) } // MARK: - HTTP methods @@ -308,7 +317,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -321,7 +330,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadMembersPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -337,7 +346,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.postBillPublisher(bill: .make()) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -353,7 +362,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.postBillPublisher(bill: .make()) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -369,7 +378,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.postBillPublisher(bill: .make()) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -387,7 +396,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.postBillPublisher(bill: .make()) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -430,7 +439,7 @@ class NetworkRequestTests: XCTestCase { let bill = Bill.make(id: 42) let exp = expectation(description: "request intercepted") NetworkService.shared.updateBillPublisher(bill: bill) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -449,7 +458,7 @@ class NetworkRequestTests: XCTestCase { let bill = Bill.make(id: 7) let exp = expectation(description: "request intercepted") NetworkService.shared.deleteBillPublisher(bill: bill) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -469,7 +478,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.createMemberPublisher(name: "Alice") - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) diff --git a/PayForMeTests/TestHelpers.swift b/PayForMeTests/TestHelpers.swift index 76bdddf..7a256ac 100644 --- a/PayForMeTests/TestHelpers.swift +++ b/PayForMeTests/TestHelpers.swift @@ -147,4 +147,8 @@ extension HTTPURLResponse { static func notFound(for url: URL) -> HTTPURLResponse { HTTPURLResponse(url: url, statusCode: 404, httpVersion: nil, headerFields: nil)! } + + static func status(_ code: Int, for url: URL) -> HTTPURLResponse { + HTTPURLResponse(url: url, statusCode: code, httpVersion: nil, headerFields: nil)! + } }