fix: harden danbooru decoding and pagination flow

4 files changed, 282 insertions, 18 deletions

diff --git a/Sora/Data/Booru/BooruManager.swift b/Sora/Data/Booru/BooruManager.swift
index 2459f2d..5753089 100644
--- a/Sora/Data/Booru/BooruManager.swift
+++ b/Sora/Data/Booru/BooruManager.swift
@@ -114,12 +114,14 @@ class BooruManager: ObservableObject {  // swiftlint:disable:this type_body_leng
 
   private func fetchPostsWithRetry(url: URL) async -> [BooruPost] {
     let maxAttempts = 4
+    var requestURL = url
+    var retriedDanbooruWithoutCredentials = false
 
     for attempt in 1...maxAttempts {
       if Task.isCancelled { return [] }
 
       do {
-        let data = try await requestURL(url)
+        let data = try await self.requestURL(requestURL)
 
         if Task.isCancelled { return [] }
 
@@ -149,6 +151,23 @@ class BooruManager: ObservableObject {  // swiftlint:disable:this type_body_leng
         }
       } catch {
         if !Task.isCancelled {
+          if flavor == .danbooru,
+            !retriedDanbooruWithoutCredentials,
+            (error as? AFError)?.responseCode == 401,
+            let unauthenticatedURL = Self.removingDanbooruCredentials(from: requestURL),
+            unauthenticatedURL != requestURL
+          {
+            retriedDanbooruWithoutCredentials = true
+            requestURL = unauthenticatedURL
+
+            debugPrint(
+              "BooruManager.fetchPosts(\(attempt)): unauthorized credentials for \(domain),"
+                + " retrying without API credentials."
+            )
+
+            continue
+          }
+
           self.error = error
 
           debugPrint("BooruManager.fetchPosts(\(attempt)): \(error)")
@@ -161,6 +180,21 @@ class BooruManager: ObservableObject {  // swiftlint:disable:this type_body_leng
     return []
   }
 
+  private static func removingDanbooruCredentials(from url: URL) -> URL? {
+    guard var components = URLComponents(url: url, resolvingAgainstBaseURL: false) else {
+      return nil
+    }
+    guard let existingQueryItems = components.queryItems else {
+      return components.url
+    }
+
+    components.queryItems = existingQueryItems.filter { queryItem in
+      queryItem.name != "login" && queryItem.name != "api_key"
+    }
+
+    return components.url
+  }
+
   func clearCachedPages() {
     pageCache.removeAllObjects()
     urlCache.removeAll()
@@ -278,9 +312,20 @@ class BooruManager: ObservableObject {  // swiftlint:disable:this type_body_leng
   }
 
   // MARK: - Private Methods
+  private func danbooruPageToken(for page: Int) -> String {
+    guard page > 1 else { return "1" }
+
+    guard let minimumPostID = posts.lazy.compactMap({ Int($0.id) }).min() else {
+      return String(page)
+    }
+
+    return "b\(minimumPostID)"
+  }
+
   func url(forPosts page: Int, limit: Int, tags: [String]) -> URL? {
     let tagString = tags.joined(separator: "+")
-    let cacheKey = "posts_\(page)_\(limit)_\(tagString.hashValue)"
+    let pageCacheValue = flavor == .danbooru ? danbooruPageToken(for: page) : String(page)
+    let cacheKey = "posts_\(pageCacheValue)_\(limit)_\(tagString.hashValue)"
 
     if let cachedURL = urlCache[cacheKey] {
       return cachedURL
@@ -297,7 +342,8 @@ class BooruManager: ObservableObject {  // swiftlint:disable:this type_body_leng
       components.path = "/posts.json"
 
       var queryItems = [
-        URLQueryItem(name: "page", value: String(page)),
+        URLQueryItem(name: "page", value: danbooruPageToken(for: page)),
+        URLQueryItem(name: "limit", value: String(limit)),
         URLQueryItem(name: "tags", value: tagString),
       ]
 
@@ -493,6 +539,7 @@ class BooruManager: ObservableObject {  // swiftlint:disable:this type_body_leng
 
   func requestURL(_ url: URL) async throws -> Data {
     try await AF.request(url, headers: ["User-Agent": userAgent])
+      .validate(statusCode: 200..<300)
       .serializingData()
       .value
   }
diff --git a/Sora/Data/Danbooru/DanbooruPost.swift b/Sora/Data/Danbooru/DanbooruPost.swift
index 216816e..c619fce 100644
--- a/Sora/Data/Danbooru/DanbooruPost.swift
+++ b/Sora/Data/Danbooru/DanbooruPost.swift
@@ -6,16 +6,16 @@ struct DanbooruPost: Decodable {
   let uploaderId: Int
   let score: Int
   let source: String
-  let md5: String
+  let md5: String?
   let rating: String
   let imageWidth: Int
   let imageHeight: Int
   let tagString: String
   let parentId: Int?
-  let mediaAsset: DanbooruMediaAsset
-  let fileURL: String
-  let largeFileURL: String
-  let previewFileURL: String
+  let mediaAsset: DanbooruMediaAsset?
+  let fileURL: String?
+  let largeFileURL: String?
+  let previewFileURL: String?
   let isDeleted: Bool
 
   enum CodingKeys: String, CodingKey {
@@ -38,24 +38,32 @@ struct DanbooruPost: Decodable {
   }
 
   func toBooruPost() -> BooruPost? {
-    guard let fileURL = URL(string: fileURL),
-      let sampleURL = URL(string: largeFileURL),
-      let previewURL = URL(string: previewFileURL)
+    guard let fileURLString = fileURL,
+      let fileURL = URL(string: fileURLString)
     else {
       return nil
     }
 
-    let previewVariant = mediaAsset.variants.first { $0.type == "180x180" }
+    let sampleURLString = largeFileURL ?? fileURLString
+    let previewURLString = previewFileURL ?? largeFileURL ?? fileURLString
+
+    guard let sampleURL = URL(string: sampleURLString),
+      let previewURL = URL(string: previewURLString)
+    else {
+      return nil
+    }
+
+    let previewVariant = mediaAsset?.variants.first { $0.type == "180x180" }
     let sampleVariant =
-      mediaAsset.variants.first { $0.type == "sample" }
-      ?? mediaAsset.variants.first { $0.type == "original" }
+      mediaAsset?.variants.first { $0.type == "sample" }
+      ?? mediaAsset?.variants.first { $0.type == "original" }
 
     return BooruPost(
       id: String(id),
       height: imageHeight,
       score: String(score),
       fileURL: fileURL,
-      parentID: parentId != nil ? String(parentId!) : "",
+      parentID: parentId.map(String.init) ?? "",
       sampleURL: sampleURL,
       sampleWidth: sampleVariant?.width ?? imageWidth,
       sampleHeight: sampleVariant?.height ?? imageHeight,
@@ -64,7 +72,7 @@ struct DanbooruPost: Decodable {
       tags: tagString.components(separatedBy: " ").filter { !$0.isEmpty },
       width: imageWidth,
       change: nil,
-      md5: md5,
+      md5: md5 ?? "",
       creatorID: String(uploaderId),
       authorID: nil,
       createdAt: createdAt,
diff --git a/Sora/Data/Danbooru/DanbooruPostParser.swift b/Sora/Data/Danbooru/DanbooruPostParser.swift
index f990ed5..1e5cdc4 100644
--- a/Sora/Data/Danbooru/DanbooruPostParser.swift
+++ b/Sora/Data/Danbooru/DanbooruPostParser.swift
@@ -1,6 +1,16 @@
 import Foundation
 
 nonisolated class DanbooruPostParser {
+  private struct FailableDecodable<Value: Decodable>: Decodable {
+    let value: Value?
+
+    init(from decoder: Decoder) throws {
+      let container = try decoder.singleValueContainer()
+
+      value = try? container.decode(Value.self)
+    }
+  }
+
   private let data: Data
 
   init(data: Data) {
@@ -17,10 +27,20 @@ nonisolated class DanbooruPostParser {
     }
 
     do {
-      return try decoder.decode([DanbooruPost].self, from: data).compactMap { post in
-        post.toBooruPost()
+      let decodedPosts = try decoder.decode([FailableDecodable<DanbooruPost>].self, from: data)
+      let validPosts = decodedPosts.compactMap(\.value)
+      let droppedRecordCount = decodedPosts.count - validPosts.count
+
+      if droppedRecordCount > 0 {
+        debugPrint(
+          "DanbooruPostParser.parse: dropped \(droppedRecordCount) malformed records while decoding page."
+        )
       }
+
+      return validPosts.compactMap { post in post.toBooruPost() }
     } catch {
+      debugPrint("DanbooruPostParser.parse: failed to decode response: \(error)")
+
       return []
     }
   }
diff --git a/SoraTests/ViewDerivedDataTests.swift b/SoraTests/ViewDerivedDataTests.swift
index ed83654..6572b70 100644
--- a/SoraTests/ViewDerivedDataTests.swift
+++ b/SoraTests/ViewDerivedDataTests.swift
@@ -1,3 +1,5 @@
+// swiftlint:disable file_length
+
 import XCTest
 
 final class ViewDerivedDataTests: XCTestCase {  // swiftlint:disable:this type_body_length
@@ -296,6 +298,193 @@ final class ViewDerivedDataTests: XCTestCase {  // swiftlint:disable:this type_b
     )
   }
 
+  func testBooruManagerDanbooruPostsIncludeLimitParameter() throws {
+    let source = try loadSource(at: "Sora/Data/Booru/BooruManager.swift")
+    let urlBuilderSection = try extractFunction(
+      named: "func url(forPosts page: Int, limit: Int, tags: [String]) -> URL?",
+      from: source
+    )
+    let danbooruCaseStart = try XCTUnwrap(urlBuilderSection.range(of: "case .danbooru:")?.lowerBound)
+    let danbooruCaseEnd = try XCTUnwrap(
+      urlBuilderSection.range(of: "case .moebooru:", range: danbooruCaseStart..<urlBuilderSection.endIndex)?
+        .lowerBound
+    )
+    let danbooruSection = String(urlBuilderSection[danbooruCaseStart..<danbooruCaseEnd])
+    let danbooruLimitQueryCount = tokenCount(
+      matching: #"URLQueryItem\(name:\s*"limit""#,
+      in: danbooruSection
+    )
+
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      danbooruLimitQueryCount,
+      0,
+      "Danbooru requests should include a `limit` query parameter."
+    )
+  }
+
+  func testBooruManagerDanbooruPostsUseBeforeCursorPagination() throws {
+    let source = try loadSource(at: "Sora/Data/Booru/BooruManager.swift")
+    let urlBuilderSection = try extractFunction(
+      named: "func url(forPosts page: Int, limit: Int, tags: [String]) -> URL?",
+      from: source
+    )
+    let danbooruCaseStart = try XCTUnwrap(urlBuilderSection.range(of: "case .danbooru:")?.lowerBound)
+    let danbooruCaseEnd = try XCTUnwrap(
+      urlBuilderSection.range(of: "case .moebooru:", range: danbooruCaseStart..<urlBuilderSection.endIndex)?
+        .lowerBound
+    )
+    let danbooruSection = String(urlBuilderSection[danbooruCaseStart..<danbooruCaseEnd])
+    let pageTokenFunctionSection = try extractFunction(
+      named: "private func danbooruPageToken(for page: Int) -> String",
+      from: source
+    )
+    let danbooruCursorPageQueryCount = tokenCount(
+      matching: #"URLQueryItem\(name:\s*"page",\s*value:\s*danbooruPageToken\(for:\s*page\)\)"#,
+      in: danbooruSection
+    )
+    let beforeCursorCount = tokenCount(
+      matching: #""b\\\#\("#,
+      in: pageTokenFunctionSection
+    )
+
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      danbooruCursorPageQueryCount,
+      0,
+      "Danbooru requests should derive the `page` query using a cursor-aware token helper."
+    )
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      beforeCursorCount,
+      0,
+      "Danbooru cursor pagination should build `page=b<id>` for subsequent pages."
+    )
+  }
+
+  func testBooruManagerValidatesHTTPStatusCodesForRequests() throws {
+    let source = try loadSource(at: "Sora/Data/Booru/BooruManager.swift")
+    let requestURLSection = try extractFunction(
+      named: "func requestURL(_ url: URL) async throws -> Data",
+      from: source
+    )
+    let statusValidationCount = tokenCount(
+      matching: #"\.validate\(statusCode:\s*200\.\.<300\)"#,
+      in: requestURLSection
+    )
+
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      statusValidationCount,
+      0,
+      "BooruManager should validate HTTP status codes to avoid silent API failures."
+    )
+  }
+
+  func testBooruManagerDanbooruFallsBackToUnauthenticatedRequestsOnUnauthorized() throws {
+    let source = try loadSource(at: "Sora/Data/Booru/BooruManager.swift")
+    let retrySection = try extractFunction(
+      named: "private func fetchPostsWithRetry(url: URL) async -> [BooruPost]",
+      from: source
+    )
+    let credentialStripperSection = try extractFunction(
+      named: "private static func removingDanbooruCredentials(from url: URL) -> URL?",
+      from: source
+    )
+    let unauthorizedResponseCheckCount = tokenCount(
+      matching: #"responseCode\s*==\s*401"#,
+      in: retrySection
+    )
+    let fallbackInvocationCount = tokenCount(
+      matching: #"removingDanbooruCredentials\(from:\s*requestURL\)"#,
+      in: retrySection
+    )
+    let strippedQueryItemCount = tokenCount(
+      matching: #"queryItem\.name\s*!=\s*"login"\s*&&\s*queryItem\.name\s*!=\s*"api_key""#,
+      in: credentialStripperSection
+    )
+
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      unauthorizedResponseCheckCount,
+      0,
+      "Danbooru fetch retries should detect unauthorized responses."
+    )
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      fallbackInvocationCount,
+      0,
+      "Danbooru fetch retries should retry without credentials after unauthorized responses."
+    )
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      strippedQueryItemCount,
+      0,
+      "Credential fallback should remove both `login` and `api_key` query parameters."
+    )
+  }
+
+  func testDanbooruPostModelUsesOptionalVisibilityDependentFields() throws {
+    let source = try loadSource(at: "Sora/Data/Danbooru/DanbooruPost.swift")
+    let optionalFileURLCount = tokenCount(matching: #"let\s+fileURL:\s+String\?"#, in: source)
+    let optionalLargeFileURLCount = tokenCount(matching: #"let\s+largeFileURL:\s+String\?"#, in: source)
+    let optionalPreviewURLCount = tokenCount(
+      matching: #"let\s+previewFileURL:\s+String\?"#,
+      in: source
+    )
+    let optionalMD5Count = tokenCount(matching: #"let\s+md5:\s+String\?"#, in: source)
+
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      optionalFileURLCount,
+      0,
+      "Danbooru model should decode visibility-dependent file URLs as optional."
+    )
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      optionalLargeFileURLCount,
+      0,
+      "Danbooru model should decode visibility-dependent large file URLs as optional."
+    )
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      optionalPreviewURLCount,
+      0,
+      "Danbooru model should decode visibility-dependent preview URLs as optional."
+    )
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      optionalMD5Count,
+      0,
+      "Danbooru model should decode visibility-dependent MD5 values as optional."
+    )
+  }
+
+  func testDanbooruParserUsesLossyDecodingForMixedRecordValidity() throws {
+    let source = try loadSource(at: "Sora/Data/Danbooru/DanbooruPostParser.swift")
+    let lossyDecoderUsageCount = tokenCount(
+      matching: #"\[FailableDecodable<DanbooruPost>\]\.self"#,
+      in: source
+    )
+    let malformedRecordDebugCount = tokenCount(
+      matching: #"dropped\s*\\\#\(droppedRecordCount\)\s*malformed records"#,
+      in: source
+    )
+
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      lossyDecoderUsageCount,
+      0,
+      "Danbooru parser should decode lossily so one malformed item doesn't drop the entire page."
+    )
+    // swiftlint:disable:next prefer_nimble
+    XCTAssertGreaterThan(
+      malformedRecordDebugCount,
+      0,
+      "Danbooru parser should log dropped malformed records for observability."
+    )
+  }
+
   func testThumbnailGridViewAvoidsDirectColumnArrayIndexing() throws {
     let source = try loadSource(at: "Sora/Views/Shared/ThumbnailGridView.swift")
     let normalizedSource = strippingCommentsAndStrings(from: source)