Skip to content

Commit b654932

Browse files
keankean
committed
More resilient Reader Search response parsing
1 parent 133324c commit b654932

File tree

6 files changed

+186
-28
lines changed

6 files changed

+186
-28
lines changed

‎Modules/Sources/WordPressKit/ReaderFeed.swift‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import Foundation
55
/// (read/feed?q=query)
66
///
77
public struct ReaderFeed: Decodable {
8-
public let url: URL
9-
public let title: String
8+
public let url: URL?
9+
public let title: String?
1010
public let feedDescription: String?
1111
public let feedID: String?
1212
public let blogID: String?
@@ -28,9 +28,11 @@ public struct ReaderFeed: Decodable {
2828
case site
2929
}
3030

31-
private enum SiteKeys: CodingKey {
31+
private enum SiteKeys: String, CodingKey {
3232
case description
3333
case icon
34+
case url = "URL"
35+
case name
3436
}
3537

3638
private enum IconKeys: CodingKey {
@@ -44,8 +46,8 @@ public struct ReaderFeed: Decodable {
4446
// - We want to decode whatever we can get, and not fail if neither of those exist
4547
let rootContainer = try decoder.container(keyedBy: CodingKeys.self)
4648

47-
url = try rootContainer.decode(URL.self, forKey: .url)
48-
title = try rootContainer.decode(String.self, forKey: .title)
49+
var feedURL = try? rootContainer.decodeIfPresent(URL.self, forKey: .url)
50+
var title = try? rootContainer.decodeIfPresent(String.self, forKey: .title)
4951
feedID = try? rootContainer.decode(String.self, forKey: .feedID)
5052
blogID = try? rootContainer.decode(String.self, forKey: .blogID)
5153

@@ -60,16 +62,27 @@ public struct ReaderFeed: Decodable {
6062

6163
let iconContainer = try siteContainer.nestedContainer(keyedBy: IconKeys.self, forKey: .icon)
6264
blavatarURL = try? iconContainer.decode(URL.self, forKey: .img)
65+
66+
// Fixes CMM-1002: in some cases, the backend fails to embed certain fields
67+
// directly in the feed object
68+
if feedURL == nil {
69+
feedURL = try? siteContainer.decodeIfPresent(URL.self, forKey: .url)
70+
}
71+
if title == nil {
72+
title = try? siteContainer.decodeIfPresent(String.self, forKey: .name)
73+
}
6374
} catch {
6475
}
6576

77+
self.url = feedURL
78+
self.title = title
6679
self.feedDescription = feedDescription
6780
self.blavatarURL = blavatarURL
6881
}
6982
}
7083

7184
extension ReaderFeed: CustomStringConvertible {
7285
public var description: String {
73-
return "<Feed | URL: \(url), title: \(title), feedID: \(String(describing: feedID)), blogID: \(String(describing: blogID))>"
86+
return "<Feed | URL: \(String(describing: url)), title: \(String(describing: title)), feedID: \(String(describing: feedID)), blogID: \(String(describing: blogID))>"
7487
}
7588
}

‎Modules/Sources/WordPressKit/ReaderSiteSearchServiceRemote.swift‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public class ReaderSiteSearchServiceRemote: ServiceRemoteWordPressComREST {
2020
public func performSearch(_ query: String,
2121
offset: Int = 0,
2222
count: Int,
23-
success: @escaping (_ results: [ReaderFeed], _ hasMore: Bool, _ feedCount: Int) -> Void,
23+
success: @escaping (_ results: [ReaderFeed], _ hasMore: Bool, _ total: Int?) -> Void,
2424
failure: @escaping (Error) -> Void) {
2525
let endpoint = "read/feed"
2626
let path = self.path(forEndpoint: endpoint, withVersion: ._1_1)
@@ -38,7 +38,7 @@ public class ReaderSiteSearchServiceRemote: ServiceRemoteWordPressComREST {
3838
success: { response, _ in
3939
do {
4040
let (results, total) = try self.mapSearchResponse(response)
41-
let hasMore = total > (offset + count)
41+
let hasMore = (total ?? 0) > (offset + count)
4242
success(results, hasMore, total)
4343
} catch {
4444
failure(error)
@@ -52,7 +52,7 @@ public class ReaderSiteSearchServiceRemote: ServiceRemoteWordPressComREST {
5252

5353
private extension ReaderSiteSearchServiceRemote {
5454

55-
func mapSearchResponse(_ response: Any) throws -> ([ReaderFeed], Int) {
55+
func mapSearchResponse(_ response: Any) throws -> ([ReaderFeed], Int?) {
5656
do {
5757
let decoder = JSONDecoder()
5858
let data = try JSONSerialization.data(withJSONObject: response, options: [])
@@ -73,9 +73,9 @@ private extension ReaderSiteSearchServiceRemote {
7373
/// The Reader feed search endpoint returns feeds in a key named `feeds` key.
7474
/// This entity allows us to do parse that and the total feed count using JSONDecoder.
7575
///
76-
private struct ReaderFeedEnvelope: Decodable {
76+
struct ReaderFeedEnvelope: Decodable {
7777
let feeds: [ReaderFeed]
78-
let total: Int
78+
let total: Int?
7979

8080
private enum CodingKeys: String, CodingKey {
8181
case feeds = "feeds"

‎Tests/WordPressKitTests/WordPressKitTests/Tests/ReederFeedTests.swift‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
import Foundation
2+
import Testing
3+
4+
@testable import WordPressKit
5+
6+
struct ReaderFeedTests {
7+
8+
@Test func decodesReaderFeedEnvelopeWithSiteFallbacks() throws {
9+
// GIVEN: JSON response where URL and title are not embedded at root level
10+
let jsonData = try #require(readerFeedJSON.data(using: .utf8))
11+
12+
// WHEN: Decoding the envelope
13+
let decoder = JSONDecoder()
14+
let envelope = try decoder.decode(ReaderFeedEnvelope.self, from: jsonData)
15+
16+
// THEN: Envelope contains feeds array
17+
#expect(envelope.feeds.count == 1)
18+
19+
let feed = try #require(envelope.feeds.first)
20+
21+
// THEN: Feed ID is decoded from root level
22+
#expect(feed.feedID == "188407")
23+
24+
// THEN: URL falls back to data.site.URL since not present at root
25+
#expect(feed.url?.absoluteString == "https://ma.tt")
26+
27+
// THEN: Title falls back to data.site.name since not present at root
28+
#expect(feed.title == "Matt Mullenweg")
29+
30+
// THEN: Description is decoded from data.site.description
31+
#expect(feed.feedDescription == "Unlucky in Cards")
32+
33+
// THEN: Blavatar URL is decoded from data.site.icon.img
34+
#expect(feed.blavatarURL?.absoluteString == "https://ma.tt/files/2024/01/cropped-matt-favicon.png")
35+
}
36+
}
37+
38+
// MARK: - Test Data
39+
40+
private let readerFeedJSON = """
41+
{
42+
"feeds": [
43+
{
44+
"subscribe_URL": "https://ma.tt/feed/",
45+
"feed_ID": "188407",
46+
"meta": {
47+
"links": {
48+
"feed": "https://public-api.wordpress.com/rest/v1.1/read/feed/188407",
49+
"site": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865"
50+
},
51+
"data": {
52+
"site": {
53+
"ID": 1047865,
54+
"name": "Matt Mullenweg",
55+
"description": "Unlucky in Cards",
56+
"URL": "https://ma.tt",
57+
"jetpack": true,
58+
"jetpack_connection": true,
59+
"post_count": 5599,
60+
"subscribers_count": 4520,
61+
"lang": "en-US",
62+
"icon": {
63+
"img": "https://ma.tt/files/2024/01/cropped-matt-favicon.png",
64+
"ico": "https://ma.tt/files/2024/01/cropped-matt-favicon.png?w=16"
65+
},
66+
"logo": {
67+
"id": 0,
68+
"sizes": [],
69+
"url": ""
70+
},
71+
"visible": true,
72+
"is_private": false,
73+
"is_coming_soon": false,
74+
"is_following": false,
75+
"organization_id": 0,
76+
"meta": {
77+
"links": {
78+
"self": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865",
79+
"help": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865/help",
80+
"posts": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865/posts/",
81+
"comments": "https://public-api.wordpress.com/rest/v1.1/sites/1047865/comments/",
82+
"xmlrpc": "https://ma.tt/blog/xmlrpc.php"
83+
}
84+
},
85+
"launch_status": false,
86+
"site_migration": {
87+
"is_complete": false,
88+
"in_progress": false
89+
},
90+
"is_fse_active": false,
91+
"is_fse_eligible": false,
92+
"is_core_site_editor_enabled": false,
93+
"is_wpcom_atomic": false,
94+
"is_wpcom_staging_site": false,
95+
"is_deleted": false,
96+
"is_a4a_client": false,
97+
"is_a4a_dev_site": false,
98+
"is_wpcom_flex": false,
99+
"capabilities": {
100+
"edit_pages": false,
101+
"edit_posts": false,
102+
"edit_others_posts": false,
103+
"edit_theme_options": false,
104+
"list_users": false,
105+
"manage_categories": false,
106+
"manage_options": false,
107+
"publish_posts": false,
108+
"upload_files": false,
109+
"view_stats": false
110+
},
111+
"is_multi_author": true,
112+
"feed_ID": 188407,
113+
"feed_URL": "http://ma.tt/feed",
114+
"header_image": false,
115+
"owner": {
116+
"ID": 5,
117+
"login": "matt",
118+
"name": "Matt",
119+
"first_name": "Matt",
120+
"last_name": "Mullenweg",
121+
"nice_name": "matt",
122+
"URL": "https://matt.blog/",
123+
"avatar_URL": "https://0.gravatar.com/avatar/33252cd1f33526af53580fcb1736172f06e6716f32afdd1be19ec3096d15dea5?s=96&d=retro&r=G",
124+
"profile_URL": "https://gravatar.com/matt",
125+
"ip_address": false,
126+
"site_visible": true,
127+
"has_avatar": true
128+
},
129+
"subscription": {
130+
"delivery_methods": {
131+
"email": null,
132+
"notification": {
133+
"send_posts": false
134+
}
135+
}
136+
},
137+
"is_blocked": false,
138+
"unseen_count": 0
139+
}
140+
}
141+
}
142+
}
143+
]
144+
}
145+
"""

‎WordPress/Classes/Services/ReaderSiteSearchService.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import WordPressData
33
import WordPressShared
44
import WordPressKit
55

6-
typealias ReaderSiteSearchSuccessBlock = (_ feeds: [ReaderFeed], _ hasMore: Bool, _ feedCount: Int) -> Void
6+
typealias ReaderSiteSearchSuccessBlock = (_ feeds: [ReaderFeed], _ hasMore: Bool, _ total: Int?) -> Void
77
typealias ReaderSiteSearchFailureBlock = (_ error: Error?) -> Void
88

99
/// Allows searching for sites / feeds in the Reader.
@@ -50,8 +50,8 @@ class ReaderSiteSearchService {
5050
remote.performSearch(query,
5151
offset: page * Constants.pageSize,
5252
count: Constants.pageSize,
53-
success: { (feeds, hasMore, feedCount) in
54-
success(feeds, hasMore, feedCount)
53+
success: { (feeds, hasMore, total) in
54+
success(feeds, hasMore, total)
5555
}, failure: { error in
5656
DDLogError("Error while performing Reader site search: \(String(describing: error))")
5757
failure(error)

‎WordPress/Classes/ViewRelated/Reader/Controllers/ReaderFeedCell.swift‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,24 @@ struct ReaderFeedCell: View {
1515
.font(.body)
1616
.lineLimit(1)
1717

18-
Text(subtitle)
19-
.font(.footnote)
20-
.foregroundStyle(.secondary)
21-
.lineLimit(2)
18+
if let subtitle {
19+
Text(subtitle)
20+
.font(.footnote)
21+
.foregroundStyle(.secondary)
22+
.lineLimit(2)
23+
}
2224
}
2325
}
2426
}
2527

2628
var title: String {
27-
let title = feed.title.stringByDecodingXMLCharacters()
28-
if !title.isEmpty {
29+
if let title = feed.title?.stringByDecodingXMLCharacters(), !title.isEmpty {
2930
return title
3031
}
31-
return feed.urlForDisplay
32+
return feed.urlForDisplay ?? "–"
3233
}
3334

34-
var subtitle: String {
35+
var subtitle: String? {
3536
if let description = feed.feedDescription, !description.isEmpty {
3637
return description.stringByDecodingXMLCharacters()
3738
}
@@ -51,7 +52,10 @@ extension SiteIconViewModel {
5152
private extension ReaderFeed {
5253
/// Strips the protocol and query from the URL.
5354
///
54-
var urlForDisplay: String {
55+
var urlForDisplay: String? {
56+
guard let url else {
57+
return nil
58+
}
5559
guard let components = URLComponents(url: url, resolvingAgainstBaseURL: false),
5660
let host = components.host else {
5761
return url.absoluteString

‎WordPress/Classes/ViewRelated/Reader/Controllers/ReaderSiteSearchViewController.swift‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,10 @@ class ReaderSiteSearchViewController: UITableViewController {
2525
reloadData()
2626
}
2727
}
28-
fileprivate var totalFeedCount: Int = 0
2928

3029
var searchQuery: String? = nil {
3130
didSet {
3231
feeds = []
33-
totalFeedCount = 0
34-
3532
syncHelper.syncContentWithUserInteraction(false)
3633
}
3734
}
@@ -77,9 +74,8 @@ class ReaderSiteSearchViewController: UITableViewController {
7774
let service = ReaderSiteSearchService(coreDataStack: ContextManager.shared)
7875
service.performSearch(with: query,
7976
page: page,
80-
success: { [weak self] (feeds, hasMore, totalFeeds) in
77+
success: { [weak self] feeds, hasMore, _ in
8178
self?.feeds.append(contentsOf: feeds)
82-
self?.totalFeedCount = totalFeeds
8379
self?.reloadData(hasMoreResults: hasMore)
8480
success?(hasMore)
8581
}, failure: { [weak self] error in

0 commit comments

Comments
 (0)