Skip to content
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

Paywalls: events unit and integration tests #3169

Merged
merged 5 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions RevenueCat.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,7 @@
4F34AEEB2A5DCCBA00F4BCB0 /* VerificationResultTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VerificationResultTests.swift; sourceTree = "<group>"; };
4F3C98692A44FA60009AECA3 /* ErrorResponse.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ErrorResponse.swift; sourceTree = "<group>"; };
4F3D56622A1E66A10070105A /* CustomerInfoManagerPostReceiptTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomerInfoManagerPostReceiptTests.swift; sourceTree = "<group>"; };
4F4EECE32AAFA8DA0047DE7A /* __Snapshots__ */ = {isa = PBXFileReference; lastKnownFileType = folder; path = __Snapshots__; sourceTree = "<group>"; };
4F4FF3E02A3B731A0028018C /* ETagStrings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ETagStrings.swift; sourceTree = "<group>"; };
4F54DF3E2A1D8C7500FD72BF /* MockStoreKit2TransactionFetcher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockStoreKit2TransactionFetcher.swift; sourceTree = "<group>"; };
4F54DF412A1D8D0700FD72BF /* MockTransactionPoster.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockTransactionPoster.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2351,6 +2352,7 @@
4FE6FEE62AA940E300780B45 /* Events */ = {
isa = PBXGroup;
children = (
4F4EECE32AAFA8DA0047DE7A /* __Snapshots__ */,
4FFCED812AA941B200118EF4 /* PaywallEventsBackendTests.swift */,
4FFFE6C72AA9467800B2955C /* PaywallEventsManagerTests.swift */,
4FFCED802AA941B200118EF4 /* PaywallEventsRequestTests.swift */,
Expand Down Expand Up @@ -4183,7 +4185,7 @@
CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER = YES;
CODE_SIGN_STYLE = Automatic;
INFOPLIST_FILE = Tests/BackendIntegrationTests/Info.plist;
IPHONEOS_DEPLOYMENT_TARGET = 15.0;
IPHONEOS_DEPLOYMENT_TARGET = 16.0;
LD_RUNPATH_SEARCH_PATHS = (
"$(inherited)",
"@executable_path/Frameworks",
Expand Down Expand Up @@ -4211,7 +4213,7 @@
CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER = YES;
CODE_SIGN_STYLE = Automatic;
INFOPLIST_FILE = Tests/BackendIntegrationTests/Info.plist;
IPHONEOS_DEPLOYMENT_TARGET = 15.0;
IPHONEOS_DEPLOYMENT_TARGET = 16.0;
LD_RUNPATH_SEARCH_PATHS = (
"$(inherited)",
"@executable_path/Frameworks",
Expand Down Expand Up @@ -4477,7 +4479,7 @@
CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER = YES;
CODE_SIGN_STYLE = Automatic;
INFOPLIST_FILE = Tests/BackendIntegrationTests/Info.plist;
IPHONEOS_DEPLOYMENT_TARGET = 15.0;
IPHONEOS_DEPLOYMENT_TARGET = 16.0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to be running these on iOS 17 soon. This simplifies some of the new code, since we never run them on iOS 15 anyway.

LD_RUNPATH_SEARCH_PATHS = (
"$(inherited)",
"@executable_path/Frameworks",
Expand Down Expand Up @@ -4505,7 +4507,7 @@
CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER = YES;
CODE_SIGN_STYLE = Automatic;
INFOPLIST_FILE = Tests/BackendIntegrationTests/Info.plist;
IPHONEOS_DEPLOYMENT_TARGET = 15.0;
IPHONEOS_DEPLOYMENT_TARGET = 16.0;
LD_RUNPATH_SEARCH_PATHS = (
"$(inherited)",
"@executable_path/Frameworks",
Expand Down
6 changes: 6 additions & 0 deletions Sources/Diagnostics/FileHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ actor FileHandler: FileHandlerType {
/// - Note: this loads the entire file in memory
/// For newer versions, consider using `readLines` instead.
func readFile() throws -> Data {
RCTestAssertNotMainThread()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance check.


try self.moveToBeginningOfFile()

return self.fileHandle.availableData
Expand All @@ -64,13 +66,17 @@ actor FileHandler: FileHandlerType {
/// Returns an async sequence for every line in the file
@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *)
func readLines() throws -> AsyncLineSequence<FileHandle.AsyncBytes> {
RCTestAssertNotMainThread()

try self.moveToBeginningOfFile()

return self.fileHandle.bytes.lines
}

/// Adds a line at the end of the file
func append(line: String) {
RCTestAssertNotMainThread()

self.fileHandle.seekToEndOfFile()
self.fileHandle.write(line.asData)
self.fileHandle.write(Self.lineBreakData)
Expand Down
4 changes: 2 additions & 2 deletions Sources/Logging/Strings/PaywallsStrings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ enum PaywallsStrings {
case event_flush_already_in_progress
case event_flush_with_empty_store
case event_flush_starting(count: Int)
case event_flush_failed(BackendError)
case event_flush_failed(Error)

}

Expand Down Expand Up @@ -78,7 +78,7 @@ extension PaywallsStrings: LogMessage {
return "Paywall event flush: posting \(count) events."

case let .event_flush_failed(error):
return "Paywall event flushing failed, will retry. Error: \(error.localizedDescription)"
return "Paywall event flushing failed, will retry. Error: \((error as NSError).localizedDescription)"
}
}

Expand Down
7 changes: 5 additions & 2 deletions Sources/Networking/InternalAPI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,14 @@ class InternalAPI {

extension InternalAPI {

/// - Throws: `BackendError`
@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
func postPaywallEvents(events: [PaywallStoredEvent]) async -> BackendError? {
return await Async.call { completion in
func postPaywallEvents(events: [PaywallStoredEvent]) async throws {
let error = await Async.call { completion in
self.postPaywallEvents(events: events, completion: completion)
}

if let error { throw error }
}

}
14 changes: 7 additions & 7 deletions Sources/Paywalls/Events/Networking/PaywallEventsRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ extension PaywallEventsRequest {

enum EventType: String {

case view
case cancel
case close
case impression = "paywall_impression"
case cancel = "paywall_cancel"
case close = "paywall_close"

}

Expand All @@ -47,7 +47,7 @@ extension PaywallEventsRequest {
var sessionID: String
var offeringID: String
var paywallRevision: Int
var timestamp: Date
var timestamp: UInt64
var displayMode: PaywallViewMode
var darkMode: Bool
var localeIdentifier: String
Expand All @@ -69,7 +69,7 @@ extension PaywallEventsRequest.Event {
sessionID: data.sessionIdentifier.uuidString,
offeringID: data.offeringIdentifier,
paywallRevision: data.paywallRevision,
timestamp: data.date,
timestamp: data.date.millisecondsSince1970,
displayMode: data.displayMode,
darkMode: data.darkMode,
localeIdentifier: data.localeIdentifier
Expand All @@ -85,7 +85,7 @@ private extension PaywallEvent {

var eventType: PaywallEventsRequest.EventType {
switch self {
case .view: return .view
case .view: return .impression
case .cancel: return .cancel
case .close: return .close
}
Expand All @@ -111,7 +111,7 @@ extension PaywallEventsRequest.Event: Encodable {
case timestamp
case displayMode
case darkMode
case localeIdentifier
case localeIdentifier = "locale"

}

Expand Down
12 changes: 10 additions & 2 deletions Sources/Paywalls/Events/PaywallEventStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,14 @@ internal actor PaywallEventStore: PaywallEventStoreType {
@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *)
extension PaywallEventStore {

static func createDefault() throws -> PaywallEventStore {
let url = try Self.documentsDirectory
static func createDefault(documentsDirectory: URL?) throws -> PaywallEventStore {
let documentsDirectory = try documentsDirectory ?? Self.documentsDirectory
let url = documentsDirectory
.appendingPathComponent("revenuecat")
.appendingPathComponent("paywall_event_store")

Logger.verbose(PaywallEventStoreStrings.initializing(url))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helps us make sure this is stored in a reasonable directory.


return try .init(handler: FileHandler(url))
}

Expand All @@ -116,6 +119,8 @@ extension PaywallEventStore {
@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
private enum PaywallEventStoreStrings {

case initializing(URL)

case storing_event(PaywallEvent)

case error_storing_event(Error)
Expand All @@ -131,6 +136,9 @@ extension PaywallEventStoreStrings: LogMessage {

var description: String {
switch self {
case let .initializing(directory):
return "Initializing PaywallEventStore: \(directory.absoluteString)"

case let .storing_event(event):
return "Storing event: \(event.debugDescription)"

Expand Down
25 changes: 16 additions & 9 deletions Sources/Paywalls/Events/PaywallEventsManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *)
func track(paywallEvent: PaywallEvent) async

/// - Throws: if posting events fails
/// - Returns: the number of events posted
@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *)
func flushEvents(count: Int) async
func flushEvents(count: Int) async throws -> Int
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new API makes it easier to assert (especially on integration tests) that things worked as expected.


}

Expand All @@ -46,10 +48,10 @@
await self.store.store(.init(event: paywallEvent, userID: self.userProvider.currentAppUserID))
}

func flushEvents(count: Int) async {
func flushEvents(count: Int) async throws -> Int {
guard !self.flushInProgress else {
Logger.debug(Strings.paywalls.event_flush_already_in_progress)
return
return 0

Check warning on line 54 in Sources/Paywalls/Events/PaywallEventsManager.swift

View check run for this annotation

Codecov / codecov/patch

Sources/Paywalls/Events/PaywallEventsManager.swift#L54

Added line #L54 was not covered by tests
}
self.flushInProgress = true
defer { self.flushInProgress = false }
Expand All @@ -58,21 +60,26 @@

guard !events.isEmpty else {
Logger.verbose(Strings.paywalls.event_flush_with_empty_store)
return
return 0
}

Logger.verbose(Strings.paywalls.event_flush_starting(count: events.count))

let error = await self.internalAPI.postPaywallEvents(events: events)
do {
try await self.internalAPI.postPaywallEvents(events: events)

if let error {
await self.store.clear(count)

return events.count
} catch {
Logger.error(Strings.paywalls.event_flush_failed(error))

if error.successfullySynced {
if let backendError = error as? BackendError,
backendError.successfullySynced {
await self.store.clear(count)
}
} else {
await self.store.clear(count)

throw error
}
}

Expand Down
12 changes: 11 additions & 1 deletion Sources/Purchasing/Purchases/Purchases.swift
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@
convenience init(apiKey: String,
appUserID: String?,
userDefaults: UserDefaults? = nil,
documentsDirectory: URL? = nil,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional for integration tests, similar to userDefaults, allows us to override with a temporary one that gets removed on each test run.

observerMode: Bool = false,
platformInfo: PlatformInfo? = Purchases.platformInfo,
responseVerificationMode: Signing.ResponseVerificationMode,
Expand Down Expand Up @@ -359,7 +360,7 @@
paywallEventsManager = PaywallEventsManager(
internalAPI: backend.internalAPI,
userProvider: identityManager,
store: try PaywallEventStore.createDefault()
store: try PaywallEventStore.createDefault(documentsDirectory: documentsDirectory)
)
Logger.verbose(Strings.paywalls.event_manager_initialized)
} else {
Expand Down Expand Up @@ -1266,6 +1267,7 @@
appUserID: String?,
observerMode: Bool,
userDefaults: UserDefaults?,
documentsDirectory: URL? = nil,
platformInfo: PlatformInfo?,
responseVerificationMode: Signing.ResponseVerificationMode,
storeKit2Setting: StoreKit2Setting,
Expand All @@ -1277,6 +1279,7 @@
.init(apiKey: apiKey,
appUserID: appUserID,
userDefaults: userDefaults,
documentsDirectory: documentsDirectory,
observerMode: observerMode,
platformInfo: platformInfo,
responseVerificationMode: responseVerificationMode,
Expand Down Expand Up @@ -1531,6 +1534,13 @@
self.offeringsManager.invalidateCachedOfferings(appUserID: self.appUserID)
}

/// - Throws: if posting events fails
/// - Returns: the number of events posted
@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *)
func flushPaywallEvents(count: Int) async throws -> Int {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Visible for tests.

return try await self.paywallEventsManager?.flushEvents(count: count) ?? 0
}

Check warning on line 1542 in Sources/Purchasing/Purchases/Purchases.swift

View check run for this annotation

Codecov / codecov/patch

Sources/Purchasing/Purchases/Purchases.swift#L1540-L1542

Added lines #L1540 - L1542 were not covered by tests

}

#endif
Expand Down
Loading