Skip to content

Commit f5fc777

Browse files
keankean
authored
Fix an issue with duplicated subscriptions for Jetpack-connected sites (#25026)
* Fix an issue with subscribing to Jetpack-connected site from search leading to duplicated subscriptions * Simpler and more resilient parsing * Update tests
1 parent c032167 commit f5fc777

File tree

10 files changed

+571
-201
lines changed

10 files changed

+571
-201
lines changed

‎Modules/Sources/WordPressKit/ReaderFeed.swift‎

Lines changed: 140 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -4,87 +4,132 @@ import Foundation
44
/// Encapsulates details of a single feed returned by the Reader feed search API
55
/// (read/feed?q=query)
66
///
7+
/// The API returns different structures depending on the site type:
8+
/// - WordPress.com sites: Data at root level (URL, title, blog_ID)
9+
/// - Jetpack sites: Data in meta.data.feed
10+
/// - External RSS feeds: Data in meta.data.feed, blog_ID is "0"
11+
///
712
public struct ReaderFeed: Decodable {
8-
public let url: URL?
9-
public let title: String?
10-
public let feedDescription: String?
11-
public let feedID: String?
12-
public let blogID: String?
13-
public let blavatarURL: URL?
13+
public var feedID: String? {
14+
let id = feed?.feedID ?? site?.feedID.map(String.init)
15+
return id?.nonEmptyID
16+
}
1417

15-
private enum CodingKeys: String, CodingKey {
16-
case url = "URL"
17-
case title = "title"
18-
case feedID = "feed_ID"
19-
case blogID = "blog_ID"
20-
case meta = "meta"
18+
public var blogID: String? {
19+
let id = feed?.blogID ?? site?.id.map(String.init)
20+
return id?.nonEmptyID
2121
}
2222

23-
private enum MetaKeys: CodingKey {
24-
case data
23+
/// Site/Feed URL with fallback: data.site → data.feed
24+
/// Prioritizes site URL over feed URL for canonical representation
25+
public var url: URL? {
26+
site?.url ?? feed?.url
2527
}
2628

27-
private enum DataKeys: CodingKey {
28-
case site
29-
case feed
29+
/// Site/Feed title with fallback: data.site → data.feed
30+
/// Prioritizes site name over feed name
31+
public var title: String? {
32+
site?.name ?? feed?.name
3033
}
3134

35+
/// Feed description with fallback: data.site → data.feed
36+
public var description: String? {
37+
site?.description ?? feed?.description
38+
}
39+
40+
/// Site icon/avatar URL, prioritizing data.site.icon.img over data.feed.image
41+
public var iconURL: URL? {
42+
site?.iconURL ?? feed?.imageURL
43+
}
44+
45+
// MARK: - Decodable
46+
47+
/// Feed data from meta.data.feed
48+
private var feed: FeedData?
49+
50+
/// Site data from meta.data.site
51+
private var site: SiteData?
52+
3253
public init(from decoder: Decoder) throws {
33-
// We have to manually decode the feed from the JSON, for a couple of reasons:
34-
// - Some feeds have no `icon` dictionary
35-
// - Some feeds have no `data` dictionary
36-
// - We want to decode whatever we can get, and not fail if neither of those exist
37-
let rootContainer = try decoder.container(keyedBy: CodingKeys.self)
38-
39-
var feedURL = try? rootContainer.decodeIfPresent(URL.self, forKey: .url)
40-
var title = try? rootContainer.decodeIfPresent(String.self, forKey: .title)
41-
feedID = try? rootContainer.decode(String.self, forKey: .feedID)
42-
blogID = try? rootContainer.decode(String.self, forKey: .blogID)
43-
44-
var feedDescription: String?
45-
var blavatarURL: URL?
46-
47-
// Try to parse both site and feed data from meta.data
48-
do {
49-
let metaContainer = try rootContainer.nestedContainer(keyedBy: MetaKeys.self, forKey: .meta)
50-
let dataContainer = try metaContainer.nestedContainer(keyedBy: DataKeys.self, forKey: .data)
51-
52-
let siteData = try? dataContainer.decode(SiteOrFeedData.self, forKey: .site)
53-
let feedData = try? dataContainer.decode(SiteOrFeedData.self, forKey: .feed)
54-
55-
// Use data from either source, preferring site data when both are available
56-
feedDescription = siteData?.description ?? feedData?.description
57-
blavatarURL = siteData?.iconURL ?? feedData?.iconURL
58-
59-
// Fixes CMM-1002: in some cases, the backend fails to embed certain fields
60-
// directly in the feed object
61-
if feedURL == nil {
62-
feedURL = siteData?.url ?? feedData?.url
63-
}
64-
if title == nil {
65-
title = siteData?.title ?? feedData?.title
66-
}
67-
} catch {
54+
let parsed = try ReaderFeedJSON(from: decoder)
55+
self.feed = parsed.meta?.data?.feed
56+
self.site = parsed.meta?.data?.site
57+
58+
// If feed data not found, try parsing inline data from root (WordPress.com format)
59+
if self.feed == nil, let inlineData = try? InlineData(from: decoder) {
60+
self.feed = FeedData(from: inlineData)
6861
}
62+
}
63+
}
6964

70-
self.url = feedURL
71-
self.title = title
72-
self.feedDescription = feedDescription
73-
self.blavatarURL = blavatarURL
65+
private struct ReaderFeedJSON: Decodable {
66+
struct Meta: Decodable {
67+
struct Data: Decodable {
68+
var feed: FeedData?
69+
var site: SiteData?
70+
}
71+
72+
var data: Data?
7473
}
74+
75+
var meta: Meta?
7576
}
7677

77-
private struct SiteOrFeedData: Decodable {
78-
var title: String?
79-
var description: String?
80-
var iconURL: URL?
81-
var url: URL?
78+
/// Represents feed-specific data from meta.data.feed
79+
private struct FeedData: Decodable {
80+
let feedID: String?
81+
let blogID: String?
82+
let name: String?
83+
let url: URL?
84+
let description: String?
85+
let imageURL: URL?
8286

83-
enum CodingKeys: String, CodingKey {
84-
case description
85-
case icon
87+
private enum CodingKeys: String, CodingKey {
88+
case feedID = "feed_ID"
89+
case blogID = "blog_ID"
90+
case name = "name"
8691
case url = "URL"
87-
case name
92+
case description = "description"
93+
case imageURL = "image"
94+
}
95+
96+
init(from decoder: Decoder) throws {
97+
let container = try decoder.container(keyedBy: CodingKeys.self)
98+
99+
feedID = try? container.decodeIfPresent(String.self, forKey: .feedID)
100+
blogID = try? container.decodeIfPresent(String.self, forKey: .blogID)
101+
name = try? container.decodeIfPresent(String.self, forKey: .name)
102+
url = try? container.decodeIfPresent(URL.self, forKey: .url)
103+
description = try? container.decodeIfPresent(String.self, forKey: .description)
104+
imageURL = try? container.decodeIfPresent(URL.self, forKey: .imageURL)
105+
}
106+
107+
init(from inlineData: InlineData) {
108+
self.feedID = inlineData.feedID
109+
self.blogID = inlineData.blogID
110+
self.name = inlineData.title
111+
self.url = inlineData.url
112+
self.description = nil
113+
self.imageURL = nil
114+
}
115+
}
116+
117+
/// Represents site-specific data from meta.data.site
118+
private struct SiteData: Decodable {
119+
let feedID: Int?
120+
let id: Int?
121+
let name: String?
122+
let url: URL?
123+
let description: String?
124+
let iconURL: URL?
125+
126+
private enum CodingKeys: String, CodingKey {
127+
case feedID = "feed_ID"
128+
case id = "ID"
129+
case name = "name"
130+
case url = "URL"
131+
case description = "description"
132+
case icon = "icon"
88133
}
89134

90135
private enum IconKeys: CodingKey {
@@ -94,19 +139,43 @@ private struct SiteOrFeedData: Decodable {
94139
init(from decoder: Decoder) throws {
95140
let container = try decoder.container(keyedBy: CodingKeys.self)
96141

97-
title = try? container.decodeIfPresent(String.self, forKey: .name)
98-
description = try? container.decodeIfPresent(String.self, forKey: .description)
142+
feedID = try? container.decodeIfPresent(Int.self, forKey: .feedID)
143+
id = try? container.decodeIfPresent(Int.self, forKey: .id)
144+
name = try? container.decodeIfPresent(String.self, forKey: .name)
99145
url = try? container.decodeIfPresent(URL.self, forKey: .url)
146+
description = try? container.decodeIfPresent(String.self, forKey: .description)
100147

101-
// Try to decode the icon URL from the nested icon dictionary
148+
// Decode icon.img if icon dictionary exists
102149
if let iconContainer = try? container.nestedContainer(keyedBy: IconKeys.self, forKey: .icon) {
103150
iconURL = try? iconContainer.decode(URL.self, forKey: .img)
151+
} else {
152+
iconURL = nil
104153
}
105154
}
106155
}
107156

108-
extension ReaderFeed: CustomStringConvertible {
109-
public var description: String {
110-
return "<Feed | URL: \(String(describing: url)), title: \(String(describing: title)), feedID: \(String(describing: feedID)), blogID: \(String(describing: blogID))>"
157+
/// Represents inline feed data (WordPress.com sites)
158+
/// Used when feed data appears at root level instead of nested in meta.data.feed.
159+
/// In practice, it should never be necessary. It's a fallback.
160+
private struct InlineData: Decodable {
161+
let feedID: String?
162+
let blogID: String?
163+
let title: String?
164+
let url: URL?
165+
166+
private enum CodingKeys: String, CodingKey {
167+
case feedID = "feed_ID"
168+
case blogID = "blog_ID"
169+
case title = "title"
170+
case url = "URL"
171+
}
172+
}
173+
174+
private extension String {
175+
var nonEmptyID: String? {
176+
guard !isEmpty && self != "0" else {
177+
return nil
178+
}
179+
return self
111180
}
112181
}

‎RELEASE-NOTES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
* [*] Fix overly long related post titles in Reader [#25011]
2424
* [*] Increase number of lines for post tiles in Reader to three [#25019]
2525
* [*] Fix horizontal insets in Reader article view [#25010]
26+
* [*] Fix an issue with duplicated subscriptions for Jetpack-connected sites [#25026]
2627
* [*] Fixed several reader bugs causing posts to load strangely, or not at all [#25016]
2728

2829
26.4

‎Tests/WordPressKitTests/WordPressKitTests/Mock Data/Reader/reader-search-response-01.json‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
{
2+
"feeds": [
3+
{
4+
"subscribe_URL": "https://ma.tt/feed/",
5+
"feed_ID": "188407",
6+
"meta": {
7+
"links": {
8+
"feed": "https://public-api.wordpress.com/rest/v1.1/read/feed/188407",
9+
"site": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865"
10+
},
11+
"data": {
12+
"feed": {
13+
"blog_ID": "1047865",
14+
"feed_ID": "188407",
15+
"blog_owner": {
16+
"ID": 5,
17+
"name": "Matt"
18+
},
19+
"name": "Matt Mullenweg",
20+
"URL": "https://ma.tt/",
21+
"feed_URL": "http://ma.tt/feed",
22+
"subscribers_count": 4520,
23+
"is_following": false,
24+
"last_update": "2025-11-27T07:18:10+00:00",
25+
"last_checked": "2025-11-27T19:04:42+00:00",
26+
"marked_for_refresh": false,
27+
"next_refresh_time": null,
28+
"organization_id": 0,
29+
"subscription_id": null,
30+
"unseen_count": 0,
31+
"meta": {
32+
"links": {
33+
"self": "https://public-api.wordpress.com/rest/v1.1/read/feed/188407",
34+
"site": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865"
35+
}
36+
},
37+
"resolved_feed_url": "https://ma.tt/feed/",
38+
"image": "https://i0.wp.com/ma.tt/files/2024/01/cropped-matt-favicon.png?fit=32%2C32&amp;quality=80&amp;ssl=1",
39+
"description": "Unlucky in Cards"
40+
},
41+
"site": {
42+
"ID": 1047865,
43+
"name": "Matt Mullenweg",
44+
"description": "Unlucky in Cards",
45+
"URL": "https://ma.tt",
46+
"jetpack": true,
47+
"jetpack_connection": true,
48+
"post_count": 5600,
49+
"subscribers_count": 4520,
50+
"lang": "en-US",
51+
"icon": {
52+
"img": "https://ma.tt/files/2024/01/cropped-matt-favicon.png",
53+
"ico": "https://ma.tt/files/2024/01/cropped-matt-favicon.png?w=16"
54+
},
55+
"logo": {
56+
"id": 0,
57+
"sizes": [],
58+
"url": ""
59+
},
60+
"visible": true,
61+
"is_private": false,
62+
"is_coming_soon": false,
63+
"is_following": false,
64+
"organization_id": 0,
65+
"meta": {
66+
"links": {
67+
"self": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865",
68+
"help": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865/help",
69+
"posts": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865/posts/",
70+
"comments": "https://public-api.wordpress.com/rest/v1.1/sites/1047865/comments/",
71+
"xmlrpc": "https://ma.tt/blog/xmlrpc.php"
72+
}
73+
},
74+
"launch_status": false,
75+
"site_migration": {
76+
"is_complete": false,
77+
"in_progress": false
78+
},
79+
"is_fse_active": false,
80+
"is_fse_eligible": false,
81+
"is_core_site_editor_enabled": false,
82+
"is_wpcom_atomic": false,
83+
"is_wpcom_staging_site": false,
84+
"is_deleted": false,
85+
"is_a4a_client": false,
86+
"is_a4a_dev_site": false,
87+
"is_wpcom_flex": false,
88+
"capabilities": {
89+
"edit_pages": false,
90+
"edit_posts": false,
91+
"edit_others_posts": false,
92+
"edit_theme_options": false,
93+
"list_users": false,
94+
"manage_categories": false,
95+
"manage_options": false,
96+
"publish_posts": false,
97+
"upload_files": false,
98+
"view_stats": false
99+
},
100+
"is_multi_author": true,
101+
"feed_ID": 188407,
102+
"feed_URL": "http://ma.tt/feed",
103+
"header_image": false,
104+
"owner": {
105+
"ID": 5,
106+
"login": "matt",
107+
"name": "Matt",
108+
"first_name": "Matt",
109+
"last_name": "Mullenweg",
110+
"nice_name": "matt",
111+
"URL": "https://matt.blog/",
112+
"avatar_URL": "https://0.gravatar.com/avatar/33252cd1f33526af53580fcb1736172f06e6716f32afdd1be19ec3096d15dea5?s=96&d=retro&r=G",
113+
"profile_URL": "https://gravatar.com/matt",
114+
"ip_address": false,
115+
"site_visible": true,
116+
"has_avatar": true
117+
},
118+
"subscription": {
119+
"delivery_methods": {
120+
"email": null,
121+
"notification": {
122+
"send_posts": false
123+
}
124+
}
125+
},
126+
"is_blocked": false,
127+
"unseen_count": 0
128+
}
129+
}
130+
}
131+
}
132+
]
133+
}

0 commit comments

Comments
 (0)