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

Offline Entitlements: don't compute offline CustomerInfo when purchasing a consumable products #2522

Merged
merged 8 commits into from
May 24, 2023
4 changes: 4 additions & 0 deletions Sources/Logging/Strings/OfflineEntitlementsStrings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ enum OfflineEntitlementsStrings {
case found_unverified_transactions_in_sk2(transactionID: UInt64, Error)

case computing_offline_customer_info_with_no_entitlement_mapping
case computing_offline_customer_info_for_consumable_product
case computing_offline_customer_info
case computing_offline_customer_info_failed(Error)
case computed_offline_customer_info(EntitlementInfos)
Expand Down Expand Up @@ -68,6 +69,9 @@ extension OfflineEntitlementsStrings: CustomStringConvertible {
case .computing_offline_customer_info_with_no_entitlement_mapping:
return "Unable to compute offline CustomerInfo with no product entitlement mapping."

case .computing_offline_customer_info_for_consumable_product:
return "Unable to compute offline CustomerInfo when purchasing consumable products."

case .computing_offline_customer_info:
return "Encountered a server error. Will attempt to compute an offline CustomerInfo from local purchases."

Expand Down
58 changes: 53 additions & 5 deletions Sources/OfflineEntitlements/PurchasedProductsFetcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class PurchasedProductsFetcher: PurchasedProductsFetcherType {
func fetchPurchasedProducts() async throws -> [PurchasedSK2Product] {
var result: [PurchasedSK2Product] = []

for transaction in await self.transactions {
for transaction in try await self.transactions {
switch transaction {
case let .unverified(transaction, verificationError):
Logger.appleWarning(
Expand All @@ -67,26 +67,30 @@ class PurchasedProductsFetcher: PurchasedProductsFetcherType {
private static let cacheDuration: DispatchTimeInterval = .minutes(5)

private var transactions: Transactions {
get async {
get async throws {
if !self.cache.isCacheStale(durationInSeconds: Self.cacheDuration.seconds),
let cache = self.cache.cachedInstance, !cache.isEmpty {
Logger.debug(Strings.offlineEntitlements.purchased_products_returning_cache(count: cache.count))
return cache
}

let result = await TimingUtil.measureAndLogIfTooSlow(
let result = try await TimingUtil.measureAndLogIfTooSlow(
threshold: .purchasedProducts,
message: Strings.offlineEntitlements.purchased_products_fetching_too_slow
) {
return await Self.fetchTransactions()
return try await Self.fetchTransactions()
}

self.cache.cache(instance: result)
return result
}
}

private static func fetchTransactions() async -> Transactions {
private static func fetchTransactions() async throws -> Transactions {
guard await !Self.hasPendingConsumablePurchase else {
throw Error.foundConsumablePurchase
}

var result: Transactions = []

Logger.debug(Strings.offlineEntitlements.purchased_products_fetching)
Expand All @@ -97,4 +101,48 @@ class PurchasedProductsFetcher: PurchasedProductsFetcherType {
return result
}

private static var hasPendingConsumablePurchase: Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to unit test these? I guess we would have to mock StoreKit. We do have integration tests so it's probably okish, but I think it's good to test things at different granularities to try to have as much coverage as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's indirectly tested, but I agree it would be useful to test independently 👍🏻 which we can do with SKTestSession.

I added a type in #2533 to do something similar, so I'm going to combine both things and test them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracted and tested in #2539 like you suggested.

get async {
return await StoreKit.Transaction.unfinished.contains {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check, but I'm guessing SK2 will cache the transactions internally right? So this shouldn't take too long.

Also, will this be executed in a background thread? I'm guessing since the API is using async/await that might be handled internally by SK2 though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The short answer is: it doesn't matter. Concurrency in Swift is different from what you'd expect. Even if this takes 1 hour, it's not stopping the thread, it creates a continuation.

$0.productType.productCategory == .nonSubscription
}
}
}

}
// MARK: - Error

@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *)
extension PurchasedProductsFetcher {

enum Error: Swift.Error, CustomNSError {

case foundConsumablePurchase

var errorUserInfo: [String: Any] {
return [
NSLocalizedDescriptionKey: Strings.offlineEntitlements
.computing_offline_customer_info_for_consumable_product.description
]
}

}

}
// MARK: - Extensions

@available(iOS 15.0, tvOS 15.0, macOS 12.0, watchOS 8.0, *)
private extension StoreKit.VerificationResult where SignedType == StoreKit.Transaction {

var productType: StoreProduct.ProductType {
return .init(self.underlyingTransaction.productType)
}

private var underlyingTransaction: StoreKit.Transaction {
switch self {
case let .unverified(transaction, _): return transaction
case let .verified(transaction): return transaction
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,29 @@ class OfflineStoreKit1IntegrationTests: BaseOfflineStoreKitIntegrationTests {
]
}

@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *)
func testPurchasingConsumableInvalidatesOfflineMode() async throws {
self.serverDown()

let logger = TestLogHandler()

do {
_ = try await self.purchaseConsumablePackage()
fail("Expected error")
} catch let error as ErrorCode {
expect(error).to(matchError(ErrorCode.unknownBackendError))

logger.verifyMessageWasLogged(
Strings.offlineEntitlements.computing_offline_customer_info_failed(
PurchasedProductsFetcher.Error.foundConsumablePurchase
),
level: .error
)
} catch {
fail("Unexpected error: \(error)")
}
}

}

class OfflineWithNoMappingStoreKitIntegrationTests: BaseOfflineStoreKitIntegrationTests {
Expand Down
12 changes: 12 additions & 0 deletions Tests/UnitTests/Mocks/MockOfflineCustomerInfoCreator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,16 @@ class MockOfflineCustomerInfoCreator: OfflineCustomerInfoCreator {
)
}

var stubbedCreatedResult: Result<CustomerInfo, Error> = .failure(ErrorUtils.customerInfoError())
var createRequested = false
var createRequestCount = 0

@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *)
override func create(for userID: String) async throws -> CustomerInfo {
self.createRequested = true
self.createRequestCount += 1

return try self.stubbedCreatedResult.get()
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ class BackendPostReceiptDataTests: BaseBackendPostReceiptDataTests {
]
}

func testPostingReceiptCreatesASubscriberInfoObject() {
func testPostingReceiptCreatesACustomerInfoObject() {
httpClient.mock(
requestPath: .postReceiptData,
response: .init(statusCode: .success, response: Self.validCustomerResponse)
Expand Down Expand Up @@ -689,6 +689,72 @@ class BackendPostReceiptDataTests: BaseBackendPostReceiptDataTests {
}
}

@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *)
func testPostingReceiptWithNoProductDataAndServerErrorComputesOfflineUser() throws {
try AvailabilityChecks.iOS15APIAvailableOrSkipTest()

let customerInfo = try CustomerInfo(data: Self.validCustomerResponse)

self.httpClient.mock(
requestPath: .postReceiptData,
response: .init(error: .serverDown())
)
self.mockOfflineCustomerInfoCreator.stubbedCreatedResult = .success(customerInfo)

let result = waitUntilValue { completed in
self.backend.post(receiptData: Self.receiptData,
appUserID: Self.userID,
isRestore: false,
productData: nil,
presentedOfferingIdentifier: nil,
observerMode: false,
initiationSource: .purchase,
subscriberAttributes: nil,
completion: { result in
completed(result)
})
}

expect(result).to(beSuccess())
expect(result?.value) === customerInfo

expect(self.mockOfflineCustomerInfoCreator.createRequested) == true
expect(self.mockOfflineCustomerInfoCreator.createRequestCount) == 1
}

@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *)
func testPostingReceiptForSubscriptionAndServerErrorComputesOfflineUser() throws {
try AvailabilityChecks.iOS15APIAvailableOrSkipTest()

let customerInfo = try CustomerInfo(data: Self.validCustomerResponse)

self.httpClient.mock(
requestPath: .postReceiptData,
response: .init(error: .serverDown())
)
self.mockOfflineCustomerInfoCreator.stubbedCreatedResult = .success(customerInfo)

let result = waitUntilValue { completed in
self.backend.post(receiptData: Self.receiptData,
appUserID: Self.userID,
isRestore: false,
productData: .createMockProductData(),
presentedOfferingIdentifier: nil,
observerMode: false,
initiationSource: .purchase,
subscriberAttributes: nil,
completion: { result in
completed(result)
})
}

expect(result).to(beSuccess())
expect(result?.value) === customerInfo

expect(self.mockOfflineCustomerInfoCreator.createRequested) == true
expect(self.mockOfflineCustomerInfoCreator.createRequestCount) == 1
}

}

// swiftlint:disable:next type_name
Expand Down
4 changes: 3 additions & 1 deletion Tests/UnitTests/Networking/Backend/BaseBackendTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class BaseBackendTests: TestCase {
private(set) var httpClient: MockHTTPClient!
private(set) var operationDispatcher: MockOperationDispatcher!
private(set) var mockProductEntitlementMappingFetcher: MockProductEntitlementMappingFetcher!
private(set) var mockOfflineCustomerInfoCreator: MockOfflineCustomerInfoCreator!
private(set) var mockPurchasedProductsFetcher: MockPurchasedProductsFetcher!
private(set) var backend: Backend!
private(set) var offerings: OfferingsAPI!
Expand All @@ -46,6 +47,7 @@ class BaseBackendTests: TestCase {
self.httpClient = self.createClient()
self.operationDispatcher = MockOperationDispatcher()
self.mockProductEntitlementMappingFetcher = MockProductEntitlementMappingFetcher()
self.mockOfflineCustomerInfoCreator = MockOfflineCustomerInfoCreator()
self.mockPurchasedProductsFetcher = MockPurchasedProductsFetcher()

let attributionFetcher = AttributionFetcher(attributionFactory: MockAttributionTypeFactory(),
Expand All @@ -55,7 +57,7 @@ class BaseBackendTests: TestCase {
operationDispatcher: self.operationDispatcher,
operationQueue: MockBackend.QueueProvider.createBackendQueue(),
systemInfo: self.systemInfo,
offlineCustomerInfoCreator: MockOfflineCustomerInfoCreator(),
offlineCustomerInfoCreator: self.mockOfflineCustomerInfoCreator,
dateProvider: MockDateProvider(stubbedNow: MockBackend.referenceDate)
)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"headers" : {
"Authorization" : "Bearer asharedsecret"
},
"request" : {
"body" : {
"app_user_id" : "user",
"attributes" : {
"$attConsentStatus" : {
"updated_at_ms" : 1678307200000,
"value" : "authorized"
}
},
"currency" : "UYU",
"fetch_token" : "YW4gYXdlc29tZSByZWNlaXB0",
"initiation_source" : "purchase",
"is_restore" : false,
"observer_mode" : false,
"price" : "15.99",
"product_id" : "product_id",
"store_country" : "ESP"
},
"method" : "POST",
"url" : "https://api.revenuecat.com/v1/receipts"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"headers" : {
"Authorization" : "Bearer asharedsecret"
},
"request" : {
"body" : {
"app_user_id" : "user",
"attributes" : {
"$attConsentStatus" : {
"updated_at_ms" : 1678307200000,
"value" : "authorized"
}
},
"fetch_token" : "YW4gYXdlc29tZSByZWNlaXB0",
"initiation_source" : "purchase",
"is_restore" : false,
"observer_mode" : false
},
"method" : "POST",
"url" : "https://api.revenuecat.com/v1/receipts"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"headers" : {
"Authorization" : "Bearer asharedsecret"
},
"request" : {
"body" : {
"app_user_id" : "user",
"attributes" : {
"$attConsentStatus" : {
"updated_at_ms" : 1678307200000,
"value" : "authorized"
}
},
"currency" : "UYU",
"fetch_token" : "YW4gYXdlc29tZSByZWNlaXB0",
"initiation_source" : "purchase",
"is_restore" : false,
"observer_mode" : false,
"price" : "15.99",
"product_id" : "product_id",
"store_country" : "ESP"
},
"method" : "POST",
"url" : "https://api.revenuecat.com/v1/receipts"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"headers" : {
"Authorization" : "Bearer asharedsecret"
},
"request" : {
"body" : {
"app_user_id" : "user",
"attributes" : {
"$attConsentStatus" : {
"updated_at_ms" : 1678307200000,
"value" : "authorized"
}
},
"fetch_token" : "YW4gYXdlc29tZSByZWNlaXB0",
"initiation_source" : "purchase",
"is_restore" : false,
"observer_mode" : false
},
"method" : "POST",
"url" : "https://api.revenuecat.com/v1/receipts"
}
}