Fix an issue with Reader failing to parse some search results (#25022)

* Remove loading state title

* More resilient Reader Search response parsing

* Update release notes

Author: kean

Modules/Sources/WordPressKit/ReaderFeed.swift

@@ -5,8 +5,8 @@ import Foundation
 /// (read/feed?q=query)
 ///
 public struct ReaderFeed: Decodable {
-    public let url: URL
-    public let title: String
+    public let url: URL?
+    public let title: String?
     public let feedDescription: String?
     public let feedID: String?
     public let blogID: String?
@@ -26,15 +26,7 @@ public struct ReaderFeed: Decodable {
 
     private enum DataKeys: CodingKey {
         case site
-    }
-
-    private enum SiteKeys: CodingKey {
-        case description
-        case icon
-    }
-
-    private enum IconKeys: CodingKey {
-        case img
+        case feed
     }
 
     public init(from decoder: Decoder) throws {
@@ -44,32 +36,77 @@ public struct ReaderFeed: Decodable {
         // - We want to decode whatever we can get, and not fail if neither of those exist
         let rootContainer = try decoder.container(keyedBy: CodingKeys.self)
 
-        url = try rootContainer.decode(URL.self, forKey: .url)
-        title = try rootContainer.decode(String.self, forKey: .title)
+        var feedURL = try? rootContainer.decodeIfPresent(URL.self, forKey: .url)
+        var title = try? rootContainer.decodeIfPresent(String.self, forKey: .title)
         feedID = try? rootContainer.decode(String.self, forKey: .feedID)
         blogID = try? rootContainer.decode(String.self, forKey: .blogID)
 
         var feedDescription: String?
         var blavatarURL: URL?
 
+        // Try to parse both site and feed data from meta.data
         do {
             let metaContainer = try rootContainer.nestedContainer(keyedBy: MetaKeys.self, forKey: .meta)
             let dataContainer = try metaContainer.nestedContainer(keyedBy: DataKeys.self, forKey: .data)
-            let siteContainer = try dataContainer.nestedContainer(keyedBy: SiteKeys.self, forKey: .site)
-            feedDescription = try? siteContainer.decode(String.self, forKey: .description)
 
-            let iconContainer = try siteContainer.nestedContainer(keyedBy: IconKeys.self, forKey: .icon)
-            blavatarURL = try? iconContainer.decode(URL.self, forKey: .img)
+            let siteData = try? dataContainer.decode(SiteOrFeedData.self, forKey: .site)
+            let feedData = try? dataContainer.decode(SiteOrFeedData.self, forKey: .feed)
+
+            // Use data from either source, preferring site data when both are available
+            feedDescription = siteData?.description ?? feedData?.description
+            blavatarURL = siteData?.iconURL ?? feedData?.iconURL
+
+            // Fixes CMM-1002: in some cases, the backend fails to embed certain fields
+            // directly in the feed object
+            if feedURL == nil {
+                feedURL = siteData?.url ?? feedData?.url
+            }
+            if title == nil {
+                title = siteData?.title ?? feedData?.title
+            }
         } catch {
         }
 
+        self.url = feedURL
+        self.title = title
         self.feedDescription = feedDescription
         self.blavatarURL = blavatarURL
     }
 }
 
+private struct SiteOrFeedData: Decodable {
+    var title: String?
+    var description: String?
+    var iconURL: URL?
+    var url: URL?
+
+    enum CodingKeys: String, CodingKey {
+        case description
+        case icon
+        case url = "URL"
+        case name
+    }
+
+    private enum IconKeys: CodingKey {
+        case img
+    }
+
+    init(from decoder: Decoder) throws {
+        let container = try decoder.container(keyedBy: CodingKeys.self)
+
+        title = try? container.decodeIfPresent(String.self, forKey: .name)
+        description = try? container.decodeIfPresent(String.self, forKey: .description)
+        url = try? container.decodeIfPresent(URL.self, forKey: .url)
+
+        // Try to decode the icon URL from the nested icon dictionary
+        if let iconContainer = try? container.nestedContainer(keyedBy: IconKeys.self, forKey: .icon) {
+            iconURL = try? iconContainer.decode(URL.self, forKey: .img)
+        }
+    }
+}
+
 extension ReaderFeed: CustomStringConvertible {
     public var description: String {
-        return "<Feed | URL: \(url), title: \(title), feedID: \(String(describing: feedID)), blogID: \(String(describing: blogID))>"
+        return "<Feed | URL: \(String(describing: url)), title: \(String(describing: title)), feedID: \(String(describing: feedID)), blogID: \(String(describing: blogID))>"
     }
 }

Modules/Sources/WordPressKit/ReaderSiteSearchServiceRemote.swift

@@ -20,7 +20,7 @@ public class ReaderSiteSearchServiceRemote: ServiceRemoteWordPressComREST {
     public func performSearch(_ query: String,
                               offset: Int = 0,
                               count: Int,
-                              success: @escaping (_ results: [ReaderFeed], _ hasMore: Bool, _ feedCount: Int) -> Void,
+                              success: @escaping (_ results: [ReaderFeed], _ hasMore: Bool, _ total: Int?) -> Void,
                               failure: @escaping (Error) -> Void) {
         let endpoint = "read/feed"
         let path = self.path(forEndpoint: endpoint, withVersion: ._1_1)
@@ -29,7 +29,7 @@ public class ReaderSiteSearchServiceRemote: ServiceRemoteWordPressComREST {
             "offset": offset as AnyObject,
             "exclude_followed": false as AnyObject,
             "sort": "relevance" as AnyObject,
-            "meta": "site" as AnyObject,
+            "meta": "site,feed" as AnyObject,
             "q": query as AnyObject
         ]
 
@@ -38,7 +38,7 @@ public class ReaderSiteSearchServiceRemote: ServiceRemoteWordPressComREST {
                                 success: { response, _ in
                                     do {
                                         let (results, total) = try self.mapSearchResponse(response)
-                                        let hasMore = total > (offset + count)
+                                        let hasMore = (total ?? 0) > (offset + count)
                                         success(results, hasMore, total)
                                     } catch {
                                         failure(error)
@@ -52,7 +52,7 @@ public class ReaderSiteSearchServiceRemote: ServiceRemoteWordPressComREST {
 
 private extension ReaderSiteSearchServiceRemote {
 
-    func mapSearchResponse(_ response: Any) throws -> ([ReaderFeed], Int) {
+    func mapSearchResponse(_ response: Any) throws -> ([ReaderFeed], Int?) {
         do {
             let decoder = JSONDecoder()
             let data = try JSONSerialization.data(withJSONObject: response, options: [])
@@ -73,9 +73,9 @@ private extension ReaderSiteSearchServiceRemote {
 /// The Reader feed search endpoint returns feeds in a key named `feeds` key.
 /// This entity allows us to do parse that and the total feed count using JSONDecoder.
 ///
-private struct ReaderFeedEnvelope: Decodable {
+struct ReaderFeedEnvelope: Decodable {
     let feeds: [ReaderFeed]
-    let total: Int
+    let total: Int?
 
     private enum CodingKeys: String, CodingKey {
         case feeds = "feeds"

RELEASE-NOTES.txt

@@ -14,6 +14,7 @@
 * [*] Add "Email to Subscribers" row to "Publishing" sheet [#24946]
 * [*] Add permalink preview in the slug editor and make other improvements [#24949]
 * [*] Add two accessible font sizes to Reader display settings [#25013]
+* [*] Fix an issue with Reader failing to parse some search results [#25022]
 * [*] Add "Taxonomies" to Site Settings [#24955]
 * [*] Update "Categories" picker to indicate multiple selection [#24952]
 * [*] Fix a bug where the app can't access some Jetpack connected sites [#24976]

Tests/WordPressKitTests/WordPressKitTests/Mock Data/reader-site-search-failure.json

@@ -10,7 +10,6 @@ class ReaderSiteSearchServiceRemoteTests: RemoteTestCase, RESTTestable {
     let performSearchSuccessNoIconFilename = "reader-site-search-success-no-icon.json"
     let performSearchSuccessNoDataFilename = "reader-site-search-success-no-data.json"
     let performSearchSuccessHasMoreFilename = "reader-site-search-success-hasmore.json"
-    let performSearchFailureFilename = "reader-site-search-failure.json"
     let performSearchBlogIDFallbackFilename = "reader-site-search-blog-id-fallback.json"
     let performSearchFailsWithNoBlogOrFeedIDFilename = "reader-site-search-no-blog-or-feed-id.json"
 
@@ -148,27 +147,6 @@ class ReaderSiteSearchServiceRemoteTests: RemoteTestCase, RESTTestable {
         waitForExpectations(timeout: timeout, handler: nil)
     }
 
-    func testPerformSearchFailure() {
-        let expect = expectation(description: "Perform Reader site search fails if no URL is present")
-
-        stubRemoteResponse(performSearchEndpoint, filename: performSearchFailureFilename, contentType: .ApplicationJSON)
-        remote.performSearch("discover",
-                             count: 10, success: { (_, _, _) in
-                                XCTFail("This callback shouldn't get called")
-                                expect.fulfill()
-        }, failure: { error in
-            typealias ResponseError = ReaderSiteSearchServiceRemote.ResponseError
-            guard case ResponseError.decodingFailure? = error as? ResponseError else {
-                XCTFail("Expected a decodingFailure error")
-                expect.fulfill()
-                return
-            }
-
-            expect.fulfill()
-        })
-        waitForExpectations(timeout: timeout, handler: nil)
-    }
-
     func testPerformSearchBlogIDFallback() {
         let expect = expectation(description: "Perform Reader site search falls back to parsing blog ID if no feed ID is present")