ProductFetcherSK1: enable TimingUtil log (#3327)

`ProductFetcherSK1` had 2 public methods for fetching products: `products(withIdentifiers:completion:)` and `sk1Products(withIdentifiers:completion:)`. Only one of them was using `TimingUtil` to log slow requests, and that one happened to not be used by `ProductsManager`, which means that we weren't getting these logs. This is evident in RevenueCat/react-native-purchases#755. With this change, we'll now be able to see how long the product request took.
RevenueCat · Oct 25, 2023 · e2f7233 · e2f7233
1 parent 50bd457
commit e2f7233
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 54 deletions.
diff --git a/Sources/Purchasing/ProductsManager.swift b/Sources/Purchasing/ProductsManager.swift
@@ -84,7 +84,7 @@ class ProductsManager: NSObject, ProductsManagerType {
             }
         } else {
             self.sk1Products(withIdentifiers: identifiers) { result in
-                completion(result.map { Set($0.map(StoreProduct.init(sk1Product:))) })
+                completion(result.map { Set($0.map(StoreProduct.from(product:))) })
             }
         }
     }
@@ -122,8 +122,8 @@ class ProductsManager: NSObject, ProductsManagerType {
 private extension ProductsManager {
 
     func sk1Products(withIdentifiers identifiers: Set<String>,
-                     completion: @escaping (Result<Set<SK1Product>, PurchasesError>) -> Void) {
-        return self.productsFetcherSK1.sk1Products(withIdentifiers: identifiers, completion: completion)
+                     completion: @escaping (Result<Set<SK1StoreProduct>, PurchasesError>) -> Void) {
+        return self.productsFetcherSK1.products(withIdentifiers: identifiers, completion: completion)
     }
 
 }

diff --git a/Sources/Purchasing/StoreKit1/ProductsFetcherSK1.swift b/Sources/Purchasing/StoreKit1/ProductsFetcherSK1.swift
@@ -40,35 +40,6 @@ final class ProductsFetcherSK1: NSObject {
         self.requestTimeout = requestTimeout
     }
 
-    func sk1Products(withIdentifiers identifiers: Set<String>,
-                     completion: @escaping Callback) {
-        guard identifiers.count > 0 else {
-            completion(.success([]))
-            return
-        }
-
-        self.queue.async { [self] in
-            let productsAlreadyCached = self.cachedProductsByIdentifier.filter { key, _ in identifiers.contains(key) }
-            if productsAlreadyCached.count == identifiers.count {
-                let productsAlreadyCachedSet = Set(productsAlreadyCached.values)
-                Logger.debug(Strings.offering.products_already_cached(identifiers: identifiers))
-                completion(.success(productsAlreadyCachedSet))
-                return
-            }
-
-            if let existingHandlers = self.completionHandlers[identifiers] {
-                Logger.debug(Strings.offering.found_existing_product_request(identifiers: identifiers))
-                self.completionHandlers[identifiers] = existingHandlers + [completion]
-                return
-            }
-
-            self.completionHandlers[identifiers] = [completion]
-
-            let request = self.startRequest(forIdentifiers: identifiers, retriesLeft: Self.numberOfRetries)
-            self.scheduleCancellationInCaseOfTimeout(for: request)
-        }
-    }
-
     // Note: this isn't thread-safe and must therefore be used inside of `queue` only.
     @discardableResult
     private func startRequest(forIdentifiers identifiers: Set<String>, retriesLeft: Int) -> SKProductsRequest {
@@ -101,6 +72,35 @@ final class ProductsFetcherSK1: NSObject {
         }
     }
 
+    private func sk1Products(withIdentifiers identifiers: Set<String>,
+                             completion: @escaping Callback) {
+        guard identifiers.count > 0 else {
+            completion(.success([]))
+            return
+        }
+
+        self.queue.async { [self] in
+            let productsAlreadyCached = self.cachedProductsByIdentifier.filter { key, _ in identifiers.contains(key) }
+            if productsAlreadyCached.count == identifiers.count {
+                let productsAlreadyCachedSet = Set(productsAlreadyCached.values)
+                Logger.debug(Strings.offering.products_already_cached(identifiers: identifiers))
+                completion(.success(productsAlreadyCachedSet))
+                return
+            }
+
+            if let existingHandlers = self.completionHandlers[identifiers] {
+                Logger.debug(Strings.offering.found_existing_product_request(identifiers: identifiers))
+                self.completionHandlers[identifiers] = existingHandlers + [completion]
+                return
+            }
+
+            self.completionHandlers[identifiers] = [completion]
+
+            let request = self.startRequest(forIdentifiers: identifiers, retriesLeft: Self.numberOfRetries)
+            self.scheduleCancellationInCaseOfTimeout(for: request)
+        }
+    }
+
 }
 
 private extension ProductsFetcherSK1 {

diff --git a/Tests/UnitTests/Purchasing/ProductsFetcherSK1Tests.swift b/Tests/UnitTests/Purchasing/ProductsFetcherSK1Tests.swift
@@ -20,16 +20,16 @@ class ProductsFetcherSK1Tests: TestCase {
 
     func testProductsWithIdentifiersMakesRightRequest() {
         let productIdentifiers = Set(["1", "2", "3"])
-        productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { _ in }
+        productsFetcherSK1.products(withIdentifiers: productIdentifiers) { _ in }
         expect(self.productsRequestFactory.invokedRequestCount).toEventually(equal(1))
         expect(self.productsRequestFactory.invokedRequestParameters) == productIdentifiers
     }
 
     func testProductsWithIdentifiersCallsCompletionCorrectly() throws {
         let productIdentifiers = Set(["1", "2", "3"])
-        var receivedProducts: Result<Set<SK1Product>, PurchasesError>?
+        var receivedProducts: Result<Set<SK1StoreProduct>, PurchasesError>?
 
-        productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { products in
+        productsFetcherSK1.products(withIdentifiers: productIdentifiers) { products in
             receivedProducts = products
         }
 
@@ -45,10 +45,10 @@ class ProductsFetcherSK1Tests: TestCase {
         let productIdentifiers = Set(["1", "2", "3"])
         var completionCallCount = 0
 
-        productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { _ in
+        productsFetcherSK1.products(withIdentifiers: productIdentifiers) { _ in
             completionCallCount += 1
 
-            self.productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { _ in
+            self.productsFetcherSK1.products(withIdentifiers: productIdentifiers) { _ in
                 completionCallCount += 1
             }
         }
@@ -61,8 +61,8 @@ class ProductsFetcherSK1Tests: TestCase {
     func testProductsWithIdentifiersReturnsDoesntMakeNewRequestIfProductsAreBeingFetched() {
         let productIdentifiers = Set(["1", "2", "3"])
 
-        productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { _ in }
-        productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { _ in }
+        productsFetcherSK1.products(withIdentifiers: productIdentifiers) { _ in }
+        productsFetcherSK1.products(withIdentifiers: productIdentifiers) { _ in }
 
         expect(self.productsRequestFactory.invokedRequestCount).toEventually(equal(1),
                                                                              timeout: Self.defaultTimeoutInterval)
@@ -72,15 +72,15 @@ class ProductsFetcherSK1Tests: TestCase {
     func testProductsWithIdentifiersMakesNewRequestIfAtLeastOneNewProductRequested() {
         let firstCallProducts = Set(["1", "2", "3"])
         let secondCallProducts = Set(["1", "2", "3", "4"])
-        productsFetcherSK1.sk1Products(withIdentifiers: firstCallProducts) { _ in }
-        productsFetcherSK1.sk1Products(withIdentifiers: secondCallProducts) { _ in }
+        productsFetcherSK1.products(withIdentifiers: firstCallProducts) { _ in }
+        productsFetcherSK1.products(withIdentifiers: secondCallProducts) { _ in }
 
         expect(self.productsRequestFactory.invokedRequestCount).toEventually(equal(2))
         expect(self.productsRequestFactory.invokedRequestParametersList) == [firstCallProducts, secondCallProducts]
     }
 
     func testProductsWithIdentifiersReturnsDoesntMakeNewRequestIfProductIdentifiersEmpty() {
-        productsFetcherSK1.sk1Products(withIdentifiers: []) { _ in }
+        productsFetcherSK1.products(withIdentifiers: []) { _ in }
         expect(self.productsRequestFactory.invokedRequestCount).toEventually(equal(0),
                                                                              timeout: Self.defaultTimeoutInterval)
     }
@@ -98,9 +98,9 @@ class ProductsFetcherSK1Tests: TestCase {
         let fetcher = ProductsFetcherSK1(productsRequestFactory: productsRequestFactory,
                                          requestTimeout: timeout.seconds)
 
-        var receivedResult: Result<Set<SK1Product>, PurchasesError>?
+        var receivedResult: Result<Set<SK1StoreProduct>, PurchasesError>?
 
-        fetcher.sk1Products(withIdentifiers: productIdentifiers) { result in
+        fetcher.products(withIdentifiers: productIdentifiers) { result in
             receivedResult = result
         }
 
@@ -110,16 +110,16 @@ class ProductsFetcherSK1Tests: TestCase {
 
     func testCacheProductCachesCorrectly() {
         let productIdentifiers = Set(["1", "2", "3"])
-        let mockProducts: Set<SK1Product> = Set(productIdentifiers.map {
-            MockSK1Product(mockProductIdentifier: $0)
+        let mockProducts: Set<SK1StoreProduct> = Set(productIdentifiers.map {
+            SK1StoreProduct(sk1Product: MockSK1Product(mockProductIdentifier: $0))
         })
 
-        mockProducts.forEach { productsFetcherSK1.cacheProduct($0) }
+        mockProducts.forEach { self.productsFetcherSK1.cacheProduct($0.underlyingSK1Product) }
 
         var completionCallCount = 0
-        var receivedProducts: Result<Set<SK1Product>, PurchasesError>?
+        var receivedProducts: Result<Set<SK1StoreProduct>, PurchasesError>?
 
-        productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { products in
+        productsFetcherSK1.products(withIdentifiers: productIdentifiers) { products in
             receivedProducts = products
             completionCallCount += 1
         }
@@ -141,9 +141,9 @@ class ProductsFetcherSK1Tests: TestCase {
                                                 requestTimeout: tolerance.seconds)
 
         var completionCallCount = 0
-        var receivedResult: Result<Set<SKProduct>, PurchasesError>?
+        var receivedResult: Result<Set<SK1StoreProduct>, PurchasesError>?
 
-        productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { result in
+        productsFetcherSK1.products(withIdentifiers: productIdentifiers) { result in
             receivedResult = result
             completionCallCount += 1
         }
@@ -169,9 +169,9 @@ class ProductsFetcherSK1Tests: TestCase {
                                                 requestTimeout: tolerance.seconds)
 
         var completionCallCount = 0
-        var receivedResult: Result<Set<SKProduct>, PurchasesError>?
+        var receivedResult: Result<Set<SK1StoreProduct>, PurchasesError>?
 
-        productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { result in
+        productsFetcherSK1.products(withIdentifiers: productIdentifiers) { result in
             receivedResult = result
             completionCallCount += 1
         }
@@ -194,7 +194,7 @@ class ProductsFetcherSK1Tests: TestCase {
 
         productsFetcherSK1.clearCache()
 
-        productsFetcherSK1.sk1Products(withIdentifiers: productIdentifiers) { _ in }
+        productsFetcherSK1.products(withIdentifiers: productIdentifiers) { _ in }
 
         expect(self.productsRequestFactory.invokedRequestCount).toEventually(equal(1),
                                                                              timeout: Self.defaultTimeoutInterval)