diff --git a/Yattee/Services/API/PipedAPI.swift b/Yattee/Services/API/PipedAPI.swift index da6ddcd3..0f0711cf 100644 --- a/Yattee/Services/API/PipedAPI.swift +++ b/Yattee/Services/API/PipedAPI.swift @@ -325,7 +325,7 @@ actor PipedAPI: InstanceAPI { ) // Returns {"message": "ok"} on success - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Unsubscribes from a channel. @@ -346,7 +346,7 @@ actor PipedAPI: InstanceAPI { body: bodyData ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Fetches the user's playlists. @@ -524,7 +524,7 @@ actor PipedAPI: InstanceAPI { body: CreateRequest(name: name) ) - let response: CreateResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + let response = try await performWrite(CreateResponse.self, endpoint: endpoint, instance: instance) return response.playlistId } @@ -546,7 +546,7 @@ actor PipedAPI: InstanceAPI { body: RenameRequest(playlistId: id, newName: newName) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Replaces the description of one of the user's playlists. @@ -578,7 +578,7 @@ actor PipedAPI: InstanceAPI { body: DescriptionRequest(playlistId: id, description: description) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Deletes one of the user's playlists. @@ -597,7 +597,7 @@ actor PipedAPI: InstanceAPI { body: DeleteRequest(playlistId: id) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Appends a video to one of the user's playlists. @@ -623,7 +623,7 @@ actor PipedAPI: InstanceAPI { body: AddRequest(playlistId: id, videoIds: [videoID]) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Removes the row at a position from one of the user's playlists. @@ -650,7 +650,50 @@ actor PipedAPI: InstanceAPI { body: RemoveRequest(playlistId: id, index: index) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) + } + + /// Decodes the body of an authenticated Piped write. + /// + /// Piped answers a refused write with **HTTP 200** and `{"error": "..."}`: + /// adding a video it cannot fetch — region-blocked or deleted, neither rare + /// — and removing an index that is no longer there both come back that way, + /// so the status code settles nothing here. Decoding straight into the + /// success shape turns the server's own explanation into a decoding-failure + /// message nobody can act on, so the error shape is read first: a body that + /// carries `error` may carry `message` alongside it, and checking the + /// success shape first would read right past the failure. + /// - Parameters: + /// - type: The success shape this endpoint answers with. + /// - data: The raw response body. + /// - Returns: The decoded success shape. + /// - Throws: ``APIError/serverError(_:)`` carrying the server's message when + /// the body is an error body. + static func decodeWriteResponse(_ type: T.Type, from data: Data) throws -> T { + let decoder = HTTPClient.makeAPIDecoder() + + if let failure = try? decoder.decode(PipedErrorResponse.self, from: data) { + let message = failure.error.trimmingCharacters(in: .whitespacesAndNewlines) + if !message.isEmpty { + throw APIError.serverError(message) + } + } + + do { + return try decoder.decode(type, from: data) + } catch let error as DecodingError { + throw APIError.decodingError(error) + } + } + + /// Sends an authenticated write and reads its body the way ``decodeWriteResponse(_:from:)`` does. + private func performWrite( + _ type: T.Type, + endpoint: GenericEndpoint, + instance: Instance + ) async throws -> T { + let data = try await httpClient.fetchData(endpoint, baseURL: instance.url) + return try Self.decodeWriteResponse(type, from: data) } /// Builds an authenticated JSON write request. Piped wants the token bare in @@ -677,11 +720,20 @@ private struct PipedLoginResponse: Decodable, Sendable { let token: String } -/// Generic message response from Piped API (used by subscribe/unsubscribe). -private struct PipedMessageResponse: Decodable, Sendable { +/// Generic message response from Piped API (used by subscribe/unsubscribe and +/// every playlist write). Not private so the write-decode seam can be exercised +/// against the shape the server really answers with. +struct PipedMessageResponse: Decodable, Sendable { let message: String } +/// The body Piped returns for a refused write — under HTTP 200 as often as not. +/// Some spellings carry a `message` alongside `error`, which is why this shape +/// is read before the success one. +private struct PipedErrorResponse: Decodable, Sendable { + let error: String +} + /// Subscription info from Piped API. struct PipedSubscription: Decodable, Sendable { let url: String @@ -751,6 +803,28 @@ private func stripHTML(_ html: String) -> String { return text.trimmingCharacters(in: .whitespacesAndNewlines) } +// MARK: - Piped Sentinels + +/// Piped writes `-1` where it has no value: playlist rows come back with +/// `"views": -1` and `"uploaded": -1`, and trending rows carry the same +/// sentinel. Mapped through as-is they render as "-0B views" and, once `-1` +/// milliseconds becomes a date, "56 years ago" — so a negative reading is an +/// absent one. +enum PipedSentinel { + /// A count Piped actually has, or nil for its `-1` sentinel. + static func count(_ raw: Int64?) -> Int? { + guard let raw, raw >= 0 else { return nil } + return Int(raw) + } + + /// The upload date Piped actually has, or nil for its `-1` sentinel. + /// Piped sends this field as epoch **milliseconds**. + static func uploadedDate(_ raw: Int64?) -> Date? { + guard let raw, raw >= 0 else { return nil } + return Date(timeIntervalSince1970: TimeInterval(raw) / 1000) + } +} + // MARK: - Piped Response Models private struct PipedVideo: Decodable, Sendable { @@ -787,9 +861,9 @@ private struct PipedVideo: Decodable, Sendable { thumbnailURL: uploaderAvatar.flatMap { URL(string: $0) } ), duration: TimeInterval(duration), - publishedAt: uploaded.map { Date(timeIntervalSince1970: TimeInterval($0) / 1000) }, + publishedAt: PipedSentinel.uploadedDate(uploaded), publishedText: uploadedDate, - viewCount: views.map { Int($0) }, + viewCount: PipedSentinel.count(views), likeCount: nil, thumbnails: Thumbnail.fallbackChain(for: thumbnail.flatMap { URL(string: $0) }), isLive: duration == -1, @@ -866,9 +940,9 @@ private struct PipedStreamResponse: Decodable, Sendable { thumbnailURL: uploaderAvatar.flatMap { URL(string: $0) } ), duration: TimeInterval(duration), - publishedAt: uploaded.map { Date(timeIntervalSince1970: TimeInterval($0) / 1000) }, + publishedAt: PipedSentinel.uploadedDate(uploaded), publishedText: uploadDate, - viewCount: views.map { Int($0) }, + viewCount: PipedSentinel.count(views), likeCount: likes.map { Int($0) }, thumbnails: thumbnails, isLive: livestream ?? false, @@ -1026,9 +1100,9 @@ private struct PipedSearchItem: Decodable, Sendable { thumbnailURL: uploaderAvatar.flatMap { URL(string: $0) } ), duration: TimeInterval(duration ?? 0), - publishedAt: uploaded.map { Date(timeIntervalSince1970: TimeInterval($0) / 1000) }, + publishedAt: PipedSentinel.uploadedDate(uploaded), publishedText: uploadedDate, - viewCount: views.map { Int($0) }, + viewCount: PipedSentinel.count(views), likeCount: nil, thumbnails: Thumbnail.fallbackChain(for: thumbnail.flatMap { URL(string: $0) }), isLive: duration == -1, diff --git a/YatteeTests/PipedResponseTests.swift b/YatteeTests/PipedResponseTests.swift new file mode 100644 index 00000000..a1a2e3b1 --- /dev/null +++ b/YatteeTests/PipedResponseTests.swift @@ -0,0 +1,217 @@ +// +// PipedResponseTests.swift +// YatteeTests +// +// Piped answers with two things the app used to take at face value: `-1` +// standing in for a number it does not have, and an error body served under +// HTTP 200. Both have to be read as what they are before they reach a row or +// a toast. +// + +import Testing +import Foundation +@testable import Yattee + +// MARK: - Helpers + +private let pipedBaseURL = URL(string: "https://piped.example.com")! + +/// Builds one stream row of a Piped playlist page, with whatever the server +/// claims for the two sentinel-carrying fields. +private func pipedStreamJSON( + videoId: String, + title: String, + views: Int64?, + uploaded: Int64? +) -> String { + var fields: [String] = [ + "\"url\": \"/watch?v=\(videoId)\"", + "\"title\": \"\(title)\"", + "\"uploaderName\": \"Test Channel\"", + "\"uploaderUrl\": \"/channel/UCtest\"", + "\"duration\": 120", + "\"thumbnail\": \"https://piped.example.com/thumb.jpg\"" + ] + if let views { + fields.append("\"views\": \(views)") + } + if let uploaded { + fields.append("\"uploaded\": \(uploaded)") + } + return "{\(fields.joined(separator: ", "))}" +} + +/// Builds the first page of `/playlists/{id}`. +private func pipedPlaylistPageJSON(streams: [String]) -> Data { + Data(""" + { + "name": "Watch Later", + "uploader": "arek", + "uploaderUrl": "/channel/UCowner", + "videos": \(streams.count), + "relatedStreams": [\(streams.joined(separator: ",\n"))], + "nextpage": null + } + """.utf8) +} + +// MARK: - Sentinels + +@Suite("Piped Sentinel Values") +struct PipedSentinelTests { + + @Test("A negative count is no count at all") + func negativeCountIsNil() { + #expect(PipedSentinel.count(-1) == nil) + #expect(PipedSentinel.count(-999) == nil) + } + + @Test("A count the server has is carried through, zero included") + func realCountsSurvive() { + #expect(PipedSentinel.count(0) == 0) + #expect(PipedSentinel.count(1_234_567) == 1_234_567) + } + + @Test("A missing count stays missing") + func absentCountIsNil() { + #expect(PipedSentinel.count(nil) == nil) + } + + @Test("A negative upload timestamp is not a date in 1969") + func negativeUploadedIsNil() { + #expect(PipedSentinel.uploadedDate(-1) == nil) + } + + @Test("An upload timestamp is read as epoch milliseconds") + func uploadedIsMilliseconds() { + // 2021-01-01T00:00:00Z in milliseconds. + let date = PipedSentinel.uploadedDate(1_609_459_200_000) + #expect(date == Date(timeIntervalSince1970: 1_609_459_200)) + } + + @Test("A missing upload timestamp stays missing") + func absentUploadedIsNil() { + #expect(PipedSentinel.uploadedDate(nil) == nil) + } + + @Test("A playlist row of sentinels comes back with neither views nor a date") + func playlistRowSentinelsAreDropped() throws { + let page = pipedPlaylistPageJSON(streams: [ + pipedStreamJSON(videoId: "aaa", title: "Sentinels", views: -1, uploaded: -1) + ]) + + let snapshot = try PipedAPI.userPlaylistSnapshot( + fromPageJSON: [page], + playlistID: "playlist-uuid", + instanceURL: pipedBaseURL + ) + + let video = try #require(snapshot.entries.first?.video) + #expect(video.viewCount == nil) + #expect(video.publishedAt == nil) + } + + @Test("A playlist row with real numbers keeps them, milliseconds and all") + func playlistRowRealValuesSurvive() throws { + let page = pipedPlaylistPageJSON(streams: [ + pipedStreamJSON(videoId: "bbb", title: "Real", views: 4200, uploaded: 1_609_459_200_000) + ]) + + let snapshot = try PipedAPI.userPlaylistSnapshot( + fromPageJSON: [page], + playlistID: "playlist-uuid", + instanceURL: pipedBaseURL + ) + + let video = try #require(snapshot.entries.first?.video) + #expect(video.viewCount == 4200) + #expect(video.publishedAt == Date(timeIntervalSince1970: 1_609_459_200)) + } +} + +// MARK: - Write Responses + +@Suite("Piped Write Response Decoding") +struct PipedWriteResponseTests { + + /// Stands in for the create-playlist success shape, which is declared where + /// only that one call can see it. + private struct CreateShape: Decodable { + let playlistId: String + } + + @Test("An error body under HTTP 200 is an error, not a message") + func errorBodyBecomesServerError() throws { + let data = Data(#"{"error":"Video not found"}"#.utf8) + + #expect(throws: APIError.serverError("Video not found")) { + _ = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + } + } + + @Test("The server's own words reach the toast") + func errorMessageIsReadable() { + let data = Data(#"{"error":"Index out of bounds"}"#.utf8) + + do { + _ = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + Issue.record("An error body decoded as a success") + } catch { + #expect(error.localizedDescription.contains("Index out of bounds")) + } + } + + @Test("A success body still decodes") + func successBodyDecodes() throws { + let data = Data(#"{"message":"ok"}"#.utf8) + + let response = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + #expect(response.message == "ok") + } + + @Test("An error alongside a message is still an error") + func errorWinsOverMessage() throws { + let data = Data(#"{"error":"Video not found","message":"ok"}"#.utf8) + + #expect(throws: APIError.serverError("Video not found")) { + _ = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + } + } + + @Test("A blank error is not treated as a failure") + func blankErrorFallsThroughToTheSuccessShape() throws { + let data = Data(#"{"error":" ","message":"ok"}"#.utf8) + + let response = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + #expect(response.message == "ok") + } + + @Test("The same check guards every write shape, not just the message one") + func createShapeIsGuardedToo() throws { + let failure = Data(#"{"error":"You are not authorized"}"#.utf8) + #expect(throws: APIError.serverError("You are not authorized")) { + _ = try PipedAPI.decodeWriteResponse(CreateShape.self, from: failure) + } + + let success = Data(#"{"playlistId":"abc-123"}"#.utf8) + let response = try PipedAPI.decodeWriteResponse(CreateShape.self, from: success) + #expect(response.playlistId == "abc-123") + } + + @Test("A body that is neither shape is still a decoding failure") + func unknownBodyIsADecodingError() { + let data = Data(#"{"unexpected":true}"#.utf8) + + do { + _ = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + Issue.record("An unknown body decoded as a success") + } catch let error as APIError { + guard case .decodingError = error else { + Issue.record("Expected a decoding error, got \(error)") + return + } + } catch { + Issue.record("Expected an APIError, got \(error)") + } + } +}