-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix an issue with duplicated subscriptions for Jetpack-connected sites #25026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,87 +4,132 @@ import Foundation | |
| /// Encapsulates details of a single feed returned by the Reader feed search API | ||
| /// (read/feed?q=query) | ||
| /// | ||
| /// The API returns different structures depending on the site type: | ||
| /// - WordPress.com sites: Data at root level (URL, title, blog_ID) | ||
| /// - Jetpack sites: Data in meta.data.feed | ||
| /// - External RSS feeds: Data in meta.data.feed, blog_ID is "0" | ||
| /// | ||
| public struct ReaderFeed: Decodable { | ||
| public let url: URL? | ||
| public let title: String? | ||
| public let feedDescription: String? | ||
| public let feedID: String? | ||
| public let blogID: String? | ||
| public let blavatarURL: URL? | ||
| public var feedID: String? { | ||
| let id = feed?.feedID ?? site?.feedID.map(String.init) | ||
| return id?.nonEmptyID | ||
| } | ||
|
|
||
| private enum CodingKeys: String, CodingKey { | ||
| case url = "URL" | ||
| case title = "title" | ||
| case feedID = "feed_ID" | ||
| case blogID = "blog_ID" | ||
| case meta = "meta" | ||
| public var blogID: String? { | ||
| let id = feed?.blogID ?? site?.id.map(String.init) | ||
| return id?.nonEmptyID | ||
| } | ||
|
|
||
| private enum MetaKeys: CodingKey { | ||
| case data | ||
| /// Site/Feed URL with fallback: data.site → data.feed | ||
| /// Prioritizes site URL over feed URL for canonical representation | ||
| public var url: URL? { | ||
| site?.url ?? feed?.url | ||
| } | ||
|
|
||
| private enum DataKeys: CodingKey { | ||
| case site | ||
| case feed | ||
| /// Site/Feed title with fallback: data.site → data.feed | ||
| /// Prioritizes site name over feed name | ||
| public var title: String? { | ||
| site?.name ?? feed?.name | ||
| } | ||
|
|
||
| /// Feed description with fallback: data.site → data.feed | ||
| public var description: String? { | ||
| site?.description ?? feed?.description | ||
| } | ||
|
|
||
| /// Site icon/avatar URL, prioritizing data.site.icon.img over data.feed.image | ||
| public var iconURL: URL? { | ||
| site?.iconURL ?? feed?.imageURL | ||
| } | ||
|
|
||
| // MARK: - Decodable | ||
|
|
||
| /// Feed data from meta.data.feed | ||
| private var feed: FeedData? | ||
|
|
||
| /// Site data from meta.data.site | ||
| private var site: SiteData? | ||
|
|
||
| public init(from decoder: Decoder) throws { | ||
| // We have to manually decode the feed from the JSON, for a couple of reasons: | ||
| // - Some feeds have no `icon` dictionary | ||
| // - Some feeds have no `data` dictionary | ||
| // - We want to decode whatever we can get, and not fail if neither of those exist | ||
| let rootContainer = try decoder.container(keyedBy: CodingKeys.self) | ||
|
|
||
| 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 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 { | ||
| let parsed = try ReaderFeedJSON(from: decoder) | ||
| self.feed = parsed.meta?.data?.feed | ||
| self.site = parsed.meta?.data?.site | ||
|
|
||
| // If feed data not found, try parsing inline data from root (WordPress.com format) | ||
| if self.feed == nil, let inlineData = try? InlineData(from: decoder) { | ||
| self.feed = FeedData(from: inlineData) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| self.url = feedURL | ||
| self.title = title | ||
| self.feedDescription = feedDescription | ||
| self.blavatarURL = blavatarURL | ||
| private struct ReaderFeedJSON: Decodable { | ||
| struct Meta: Decodable { | ||
| struct Data: Decodable { | ||
| var feed: FeedData? | ||
| var site: SiteData? | ||
| } | ||
|
|
||
| var data: Data? | ||
| } | ||
|
|
||
| var meta: Meta? | ||
| } | ||
|
|
||
| private struct SiteOrFeedData: Decodable { | ||
| var title: String? | ||
| var description: String? | ||
| var iconURL: URL? | ||
| var url: URL? | ||
| /// Represents feed-specific data from meta.data.feed | ||
| private struct FeedData: Decodable { | ||
| let feedID: String? | ||
| let blogID: String? | ||
| let name: String? | ||
| let url: URL? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. During testing the JSON parsing above, I found out by accident that this You can search "WordPress.com News" and check out the "wpsites.net" site in the response.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can change the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a custom |
||
| let description: String? | ||
| let imageURL: URL? | ||
|
|
||
| enum CodingKeys: String, CodingKey { | ||
| case description | ||
| case icon | ||
| private enum CodingKeys: String, CodingKey { | ||
| case feedID = "feed_ID" | ||
| case blogID = "blog_ID" | ||
| case name = "name" | ||
| case url = "URL" | ||
| case name | ||
| case description = "description" | ||
| case imageURL = "image" | ||
| } | ||
|
|
||
| init(from decoder: Decoder) throws { | ||
| let container = try decoder.container(keyedBy: CodingKeys.self) | ||
|
|
||
| feedID = try? container.decodeIfPresent(String.self, forKey: .feedID) | ||
| blogID = try? container.decodeIfPresent(String.self, forKey: .blogID) | ||
| name = try? container.decodeIfPresent(String.self, forKey: .name) | ||
| url = try? container.decodeIfPresent(URL.self, forKey: .url) | ||
| description = try? container.decodeIfPresent(String.self, forKey: .description) | ||
| imageURL = try? container.decodeIfPresent(URL.self, forKey: .imageURL) | ||
| } | ||
|
|
||
| init(from inlineData: InlineData) { | ||
| self.feedID = inlineData.feedID | ||
| self.blogID = inlineData.blogID | ||
| self.name = inlineData.title | ||
| self.url = inlineData.url | ||
| self.description = nil | ||
| self.imageURL = nil | ||
| } | ||
| } | ||
|
|
||
| /// Represents site-specific data from meta.data.site | ||
| private struct SiteData: Decodable { | ||
| let feedID: Int? | ||
| let id: Int? | ||
| let name: String? | ||
| let url: URL? | ||
| let description: String? | ||
| let iconURL: URL? | ||
|
|
||
| private enum CodingKeys: String, CodingKey { | ||
| case feedID = "feed_ID" | ||
| case id = "ID" | ||
| case name = "name" | ||
| case url = "URL" | ||
| case description = "description" | ||
| case icon = "icon" | ||
| } | ||
|
|
||
| private enum IconKeys: CodingKey { | ||
|
|
@@ -94,19 +139,43 @@ private struct SiteOrFeedData: Decodable { | |
| 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) | ||
| feedID = try? container.decodeIfPresent(Int.self, forKey: .feedID) | ||
| id = try? container.decodeIfPresent(Int.self, forKey: .id) | ||
| name = try? container.decodeIfPresent(String.self, forKey: .name) | ||
| url = try? container.decodeIfPresent(URL.self, forKey: .url) | ||
| description = try? container.decodeIfPresent(String.self, forKey: .description) | ||
|
|
||
| // Try to decode the icon URL from the nested icon dictionary | ||
| // Decode icon.img if icon dictionary exists | ||
| if let iconContainer = try? container.nestedContainer(keyedBy: IconKeys.self, forKey: .icon) { | ||
| iconURL = try? iconContainer.decode(URL.self, forKey: .img) | ||
| } else { | ||
| iconURL = nil | ||
| } | ||
| } | ||
| } | ||
|
|
||
| extension ReaderFeed: CustomStringConvertible { | ||
| public var description: String { | ||
| return "<Feed | URL: \(String(describing: url)), title: \(String(describing: title)), feedID: \(String(describing: feedID)), blogID: \(String(describing: blogID))>" | ||
| /// Represents inline feed data (WordPress.com sites) | ||
| /// Used when feed data appears at root level instead of nested in meta.data.feed. | ||
| /// In practice, it should never be necessary. It's a fallback. | ||
| private struct InlineData: Decodable { | ||
| let feedID: String? | ||
| let blogID: String? | ||
| let title: String? | ||
| let url: URL? | ||
|
|
||
| private enum CodingKeys: String, CodingKey { | ||
| case feedID = "feed_ID" | ||
| case blogID = "blog_ID" | ||
| case title = "title" | ||
| case url = "URL" | ||
| } | ||
| } | ||
|
|
||
| private extension String { | ||
| var nonEmptyID: String? { | ||
| guard !isEmpty && self != "0" else { | ||
| return nil | ||
| } | ||
| return self | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| { | ||
| "feeds": [ | ||
| { | ||
| "subscribe_URL": "https://ma.tt/feed/", | ||
| "feed_ID": "188407", | ||
| "meta": { | ||
| "links": { | ||
| "feed": "https://public-api.wordpress.com/rest/v1.1/read/feed/188407", | ||
| "site": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865" | ||
| }, | ||
| "data": { | ||
| "feed": { | ||
| "blog_ID": "1047865", | ||
| "feed_ID": "188407", | ||
| "blog_owner": { | ||
| "ID": 5, | ||
| "name": "Matt" | ||
| }, | ||
| "name": "Matt Mullenweg", | ||
| "URL": "https://ma.tt/", | ||
| "feed_URL": "http://ma.tt/feed", | ||
| "subscribers_count": 4520, | ||
| "is_following": false, | ||
| "last_update": "2025-11-27T07:18:10+00:00", | ||
| "last_checked": "2025-11-27T19:04:42+00:00", | ||
| "marked_for_refresh": false, | ||
| "next_refresh_time": null, | ||
| "organization_id": 0, | ||
| "subscription_id": null, | ||
| "unseen_count": 0, | ||
| "meta": { | ||
| "links": { | ||
| "self": "https://public-api.wordpress.com/rest/v1.1/read/feed/188407", | ||
| "site": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865" | ||
| } | ||
| }, | ||
| "resolved_feed_url": "https://ma.tt/feed/", | ||
| "image": "https://i0.wp.com/ma.tt/files/2024/01/cropped-matt-favicon.png?fit=32%2C32&quality=80&ssl=1", | ||
| "description": "Unlucky in Cards" | ||
| }, | ||
| "site": { | ||
| "ID": 1047865, | ||
| "name": "Matt Mullenweg", | ||
| "description": "Unlucky in Cards", | ||
| "URL": "https://ma.tt", | ||
| "jetpack": true, | ||
| "jetpack_connection": true, | ||
| "post_count": 5600, | ||
| "subscribers_count": 4520, | ||
| "lang": "en-US", | ||
| "icon": { | ||
| "img": "https://ma.tt/files/2024/01/cropped-matt-favicon.png", | ||
| "ico": "https://ma.tt/files/2024/01/cropped-matt-favicon.png?w=16" | ||
| }, | ||
| "logo": { | ||
| "id": 0, | ||
| "sizes": [], | ||
| "url": "" | ||
| }, | ||
| "visible": true, | ||
| "is_private": false, | ||
| "is_coming_soon": false, | ||
| "is_following": false, | ||
| "organization_id": 0, | ||
| "meta": { | ||
| "links": { | ||
| "self": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865", | ||
| "help": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865/help", | ||
| "posts": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865/posts/", | ||
| "comments": "https://public-api.wordpress.com/rest/v1.1/sites/1047865/comments/", | ||
| "xmlrpc": "https://ma.tt/blog/xmlrpc.php" | ||
| } | ||
| }, | ||
| "launch_status": false, | ||
| "site_migration": { | ||
| "is_complete": false, | ||
| "in_progress": false | ||
| }, | ||
| "is_fse_active": false, | ||
| "is_fse_eligible": false, | ||
| "is_core_site_editor_enabled": false, | ||
| "is_wpcom_atomic": false, | ||
| "is_wpcom_staging_site": false, | ||
| "is_deleted": false, | ||
| "is_a4a_client": false, | ||
| "is_a4a_dev_site": false, | ||
| "is_wpcom_flex": false, | ||
| "capabilities": { | ||
| "edit_pages": false, | ||
| "edit_posts": false, | ||
| "edit_others_posts": false, | ||
| "edit_theme_options": false, | ||
| "list_users": false, | ||
| "manage_categories": false, | ||
| "manage_options": false, | ||
| "publish_posts": false, | ||
| "upload_files": false, | ||
| "view_stats": false | ||
| }, | ||
| "is_multi_author": true, | ||
| "feed_ID": 188407, | ||
| "feed_URL": "http://ma.tt/feed", | ||
| "header_image": false, | ||
| "owner": { | ||
| "ID": 5, | ||
| "login": "matt", | ||
| "name": "Matt", | ||
| "first_name": "Matt", | ||
| "last_name": "Mullenweg", | ||
| "nice_name": "matt", | ||
| "URL": "https://matt.blog/", | ||
| "avatar_URL": "https://0.gravatar.com/avatar/33252cd1f33526af53580fcb1736172f06e6716f32afdd1be19ec3096d15dea5?s=96&d=retro&r=G", | ||
| "profile_URL": "https://gravatar.com/matt", | ||
| "ip_address": false, | ||
| "site_visible": true, | ||
| "has_avatar": true | ||
| }, | ||
| "subscription": { | ||
| "delivery_methods": { | ||
| "email": null, | ||
| "notification": { | ||
| "send_posts": false | ||
| } | ||
| } | ||
| }, | ||
| "is_blocked": false, | ||
| "unseen_count": 0 | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opted to skip reading the fields inlined in the response and use
data.feedanddata.sitedirectly as these seem to be more reliable for all three scenarios.