diff --git a/Modules/Sources/WordPressData/Swift/Blog+Analytics.swift b/Modules/Sources/WordPressData/Swift/Blog+Analytics.swift new file mode 100644 index 000000000000..9e7645658751 --- /dev/null +++ b/Modules/Sources/WordPressData/Swift/Blog+Analytics.swift @@ -0,0 +1,20 @@ +import Foundation +import CoreData +import WordPressShared + +extension Blog: BlogAnalyticsRepresentable { + /// A `Sendable` snapshot of the properties analytics attaches to an event. + /// + /// The read runs on the blog's own context queue, so the snapshot is safe to + /// hand to the analytics layer from any thread. This matters because + /// `dotComID`'s getter can mutate the object, and `isWPForTeams` faults the + /// `options` relationship — neither is safe to touch off-queue. + public var analyticsProperties: BlogAnalyticsProperties { + guard let managedObjectContext else { + return BlogAnalyticsProperties(dotComID: dotComID.map { Int($0.int64Value) }, isWPForTeams: isWPForTeams) + } + return managedObjectContext.performAndWait { + BlogAnalyticsProperties(dotComID: dotComID.map { Int($0.int64Value) }, isWPForTeams: isWPForTeams) + } + } +} diff --git a/Modules/Sources/WordPressShared/Analytics/BlogAnalyticsProperties.swift b/Modules/Sources/WordPressShared/Analytics/BlogAnalyticsProperties.swift new file mode 100644 index 000000000000..d0b6c0c6fbcc --- /dev/null +++ b/Modules/Sources/WordPressShared/Analytics/BlogAnalyticsProperties.swift @@ -0,0 +1,48 @@ +import Foundation + +/// A `Sendable`, value-type snapshot of the site facts that analytics attaches +/// to an event. +/// +/// Analytics only needs a couple of scalars about a site — its WordPress.com ID +/// and whether it's a P2 — but historically those were read straight off a Core +/// Data `Blog` at the point of tracking, on whatever thread the caller happened +/// to be on. Reading a managed object off its context's queue is a Core Data +/// threading violation, and `Blog.dotComID`'s getter can *write* back to the +/// object. Capturing the facts in this value type moves the read to a single, +/// controlled place and lets only a `Sendable` snapshot cross into the analytics +/// layer. +/// +/// - SeeAlso: ``BlogAnalyticsRepresentable`` +public struct BlogAnalyticsProperties: Equatable, Sendable { + /// The site's WordPress.com ID (the `blog_id` property). `nil` for + /// self-hosted sites. + /// + /// `Int` rather than `Int64` so it satisfies a consumer's `as? Int` after the + /// analytics dictionary round-trips through Objective-C; `Int` is 64-bit on + /// every supported platform, so no blog ID is truncated. + public let dotComID: Int? + + /// Whether the site is a WordPress for Teams (P2) site. Drives the + /// `site_type` property. + public let isWPForTeams: Bool + + public init(dotComID: Int?, isWPForTeams: Bool) { + self.dotComID = dotComID + self.isWPForTeams = isWPForTeams + } +} + +/// A type that can describe itself to the analytics layer as a value. +/// +/// Conform model types (e.g. `Blog`) to this protocol so callers can hand +/// analytics a `Sendable` ``BlogAnalyticsProperties`` snapshot instead of a live +/// Core Data object. Only the snapshot crosses the model boundary, which keeps +/// the analytics layer free of Core Data — and free of the threading hazards +/// that come with it. +public protocol BlogAnalyticsRepresentable { + /// A value-type snapshot of the site facts analytics needs. + /// + /// A conformance that wraps a managed object must produce this snapshot on + /// the object's context queue. + var analyticsProperties: BlogAnalyticsProperties { get } +} diff --git a/Modules/Tests/WordPressDataTests/BlogAnalyticsPropertiesTests.swift b/Modules/Tests/WordPressDataTests/BlogAnalyticsPropertiesTests.swift new file mode 100644 index 000000000000..2917f3d950ce --- /dev/null +++ b/Modules/Tests/WordPressDataTests/BlogAnalyticsPropertiesTests.swift @@ -0,0 +1,35 @@ +import CoreData +import Testing +import WordPressShared +@testable import WordPressData + +@MainActor +struct BlogAnalyticsPropertiesTests { + private let contextManager = ContextManager.forTesting() + private var mainContext: NSManagedObjectContext { contextManager.mainContext } + + @Test func snapshotCapturesDotComIDAndBlogSiteType() { + let blog = BlogBuilder(mainContext, dotComID: NSNumber(value: 42)) + .with(isWPForTeamsSite: false) + .build() + + let properties = blog.analyticsProperties + + #expect(properties.dotComID == 42) + #expect(!properties.isWPForTeams) + } + + @Test func snapshotCapturesP2SiteType() { + let blog = BlogBuilder(mainContext, dotComID: NSNumber(value: 7)) + .with(isWPForTeamsSite: true) + .build() + + #expect(blog.analyticsProperties.isWPForTeams) + } + + @Test func selfHostedSiteHasNoDotComID() { + let blog = BlogBuilder(mainContext, dotComID: nil).build() + + #expect(blog.analyticsProperties.dotComID == nil) + } +} diff --git a/Tests/KeystoneTests/Tests/Misc/BlogAnalyticsTrackingTests.swift b/Tests/KeystoneTests/Tests/Misc/BlogAnalyticsTrackingTests.swift new file mode 100644 index 000000000000..316b0718568d --- /dev/null +++ b/Tests/KeystoneTests/Tests/Misc/BlogAnalyticsTrackingTests.swift @@ -0,0 +1,62 @@ +import CoreData +import XCTest +import WordPressData +import WordPressShared + +@testable import WordPress + +/// End-to-end coverage for attaching a site to an event via `WPAnalytics.track(_:properties:blog:)`. +/// +/// Confirms the `Blog` snapshot reaches the emitted event as `blog_id` / `site_type`, +/// and that `blog_id` arrives as a number (a Swift `Int` bridges to `NSNumber` at the +/// Objective-C tracker boundary). +final class BlogAnalyticsTrackingTests: CoreDataTestCase { + + override func setUp() { + super.setUp() + TestAnalyticsTracker.setup() + } + + override func tearDown() { + TestAnalyticsTracker.tearDown() + super.tearDown() + } + + func testTrackAttachesBlogIDAndP2SiteType() throws { + let blog = BlogBuilder(mainContext, dotComID: 100) + .with(isWPForTeamsSite: true) + .build() + + WPAnalytics.track(.dashboardCardShown, properties: ["type": "post"], blog: blog) + + let tracked = try XCTUnwrap(TestAnalyticsTracker.tracked.last) + let blogID: Int? = tracked.value(for: "blog_id") + XCTAssertEqual(blogID, 100) + XCTAssertEqual(tracked.value(for: "site_type"), "p2") + XCTAssertEqual(tracked.value(for: "type"), "post") + } + + func testTrackMarksNonP2SiteAsBlog() throws { + let blog = BlogBuilder(mainContext, dotComID: 200) + .with(isWPForTeamsSite: false) + .build() + + WPAnalytics.track(.dashboardCardShown, blog: blog) + + let tracked = try XCTUnwrap(TestAnalyticsTracker.tracked.last) + let blogID: Int? = tracked.value(for: "blog_id") + XCTAssertEqual(blogID, 200) + XCTAssertEqual(tracked.value(for: "site_type"), "blog") + } + + func testSelfHostedSiteTracksWithoutBlogID() throws { + let blog = BlogBuilder(mainContext, dotComID: nil).build() + + WPAnalytics.track(.dashboardCardShown, blog: blog) + + let tracked = try XCTUnwrap(TestAnalyticsTracker.tracked.last) + let blogID: Int? = tracked.value(for: "blog_id") + XCTAssertNil(blogID) + XCTAssertEqual(tracked.value(for: "site_type"), "blog") + } +} diff --git a/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift b/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift index 32b02d55756b..c61d51e0ff77 100644 --- a/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift +++ b/WordPress/Classes/Utility/Analytics/WPAnalyticsEvent.swift @@ -1978,19 +1978,41 @@ extension WPAnalytics { WPAnalytics.trackString(event.value, withProperties: mergedProperties) } - /// This will call each registered tracker and fire the given event. + /// This will call each registered tracker and fire the given event, attaching the site. /// - Parameters: /// - event: a `String` that represents the event name /// - properties: a `Hash` that represents the properties - /// - blog: a `Blog` asssociated with the event - static func track(_ event: WPAnalyticsEvent, properties: [AnyHashable: Any], blog: Blog) { + /// - blogProperties: a value-type snapshot of the site associated with the event + static func track( + _ event: WPAnalyticsEvent, + properties: [AnyHashable: Any] = [:], + blogProperties: BlogAnalyticsProperties + ) { var props = properties - props[WPAppAnalyticsKeyBlogID] = blog.dotComID + if let dotComID = blogProperties.dotComID { + props[WPAppAnalyticsKeyBlogID] = dotComID + } props[WPAppAnalyticsKeySiteType] = - blog.isWPForTeams ? WPAppAnalyticsValueSiteTypeP2 : WPAppAnalyticsValueSiteTypeBlog + blogProperties.isWPForTeams ? WPAppAnalyticsValueSiteTypeP2 : WPAppAnalyticsValueSiteTypeBlog WPAnalytics.track(event, properties: props) } + /// This will call each registered tracker and fire the given event, attaching the site. + /// + /// Only a `Sendable` ``BlogAnalyticsProperties`` snapshot crosses into the analytics + /// layer; the Core Data read happens once, in the model's `analyticsProperties`. + /// - Parameters: + /// - event: a `String` that represents the event name + /// - properties: a `Hash` that represents the properties + /// - blog: the site associated with the event + static func track( + _ event: WPAnalyticsEvent, + properties: [AnyHashable: Any] = [:], + blog: some BlogAnalyticsRepresentable + ) { + track(event, properties: properties, blogProperties: blog.analyticsProperties) + } + /// Track a Reader event /// /// This will call each registered tracker and fire the given event diff --git a/WordPress/Classes/Utility/Analytics/WPAppAnalytics+Extensions.swift b/WordPress/Classes/Utility/Analytics/WPAppAnalytics+Extensions.swift index b7783a217eb5..253f844fecd9 100644 --- a/WordPress/Classes/Utility/Analytics/WPAppAnalytics+Extensions.swift +++ b/WordPress/Classes/Utility/Analytics/WPAppAnalytics+Extensions.swift @@ -23,10 +23,13 @@ extension WPAppAnalytics { public class func track(_ stat: WPAnalyticsStat, properties: [String: Any]?, blog: Blog?) { var properties = properties ?? [:] if let blog { - if let blogID = blog.dotComID { + let blogProperties = blog.analyticsProperties + if let blogID = blogProperties.dotComID { properties[WPAppAnalyticsKeyBlogID] = blogID } - properties[WPAppAnalyticsKeySiteType] = siteType(for: blog) + properties[WPAppAnalyticsKeySiteType] = + blogProperties.isWPForTeams + ? WPAppAnalyticsValueSiteTypeP2 : WPAppAnalyticsValueSiteTypeBlog } WPAppAnalytics.track(stat, withProperties: properties) }