Subscription refactoring (#815)

Task/Issue URL: https://app.asana.com/0/72649045549333/1206805455884775/f iOS PR: duckduckgo/iOS#2842 macOS PR: duckduckgo/macos-browser#2764 Tech Design URL: https://app.asana.com/0/481882893211075/1207147511614062/f Subscription refactoring for allowing unit testing. - DI - Removal of all singletons - Removal of all static functions use
duckduckgo · May 22, 2024 · b01a7ba · b01a7ba
1 parent a49bbac
commit b01a7ba
Show file tree

Hide file tree

Showing 33 changed files with 924 additions and 503 deletions.
diff --git a/Package.swift b/Package.swift
@@ -32,6 +32,7 @@ let package = Package(
         .library(name: "NetworkProtectionTestUtils", targets: ["NetworkProtectionTestUtils"]),
         .library(name: "SecureStorage", targets: ["SecureStorage"]),
         .library(name: "Subscription", targets: ["Subscription"]),
+        .library(name: "SubscriptionTestingUtilities", targets: ["SubscriptionTestingUtilities"]),
         .library(name: "History", targets: ["History"]),
         .library(name: "Suggestions", targets: ["Suggestions"]),
         .library(name: "PixelKit", targets: ["PixelKit"]),
@@ -331,6 +332,12 @@ let package = Package(
                 .define("DEBUG", .when(configuration: .debug))
             ]
         ),
+        .target(
+            name: "SubscriptionTestingUtilities",
+            dependencies: [
+                "Subscription"
+            ]
+        ),
         .target(
             name: "PixelKit",
             swiftSettings: [
@@ -505,6 +512,13 @@ let package = Package(
                 "TestUtils",
             ]
         ),
+        .testTarget(
+            name: "SubscriptionTests",
+            dependencies: [
+                "Subscription",
+                "SubscriptionTestingUtilities",
+            ]
+        ),
         .testTarget(
             name: "PixelKitTests",
             dependencies: [

diff --git a/Sources/BrowserServicesKit/SubscriptionFeatureAvailability.swift b/Sources/BrowserServicesKit/SubscriptionFeatureAvailability.swift
@@ -16,6 +16,7 @@
 //  limitations under the License.
 //
 
+import Foundation
 import Subscription
 
 public protocol SubscriptionFeatureAvailability {
@@ -26,9 +27,10 @@ public protocol SubscriptionFeatureAvailability {
 public final class DefaultSubscriptionFeatureAvailability: SubscriptionFeatureAvailability {
 
     private let privacyConfigurationManager: PrivacyConfigurationManaging
-    private let purchasePlatform: SubscriptionPurchaseEnvironment.Environment
+    private let purchasePlatform: SubscriptionEnvironment.PurchasePlatform
 
-    public init(privacyConfigurationManager: PrivacyConfigurationManaging, purchasePlatform: SubscriptionPurchaseEnvironment.Environment) {
+    public init(privacyConfigurationManager: PrivacyConfigurationManaging,
+                purchasePlatform: SubscriptionEnvironment.PurchasePlatform) {
         self.privacyConfigurationManager = privacyConfigurationManager
         self.purchasePlatform = purchasePlatform
     }

diff --git a/Sources/NetworkProtection/KeyManagement/NetworkProtectionTokenStore.swift b/Sources/NetworkProtection/KeyManagement/NetworkProtectionTokenStore.swift
@@ -41,7 +41,8 @@ public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenSt
     private let keychainStore: NetworkProtectionKeychainStore
     private let errorEvents: EventMapping<NetworkProtectionError>?
     private let isSubscriptionEnabled: Bool
-    private let accessTokenProvider: () -> String?
+    public typealias AccessTokenProvider = () -> String?
+    private let accessTokenProvider: AccessTokenProvider
 
     public static var authTokenPrefix: String { "ddg:" }
 
@@ -57,7 +58,7 @@ public final class NetworkProtectionKeychainTokenStore: NetworkProtectionTokenSt
                 serviceName: String = Defaults.tokenStoreService,
                 errorEvents: EventMapping<NetworkProtectionError>?,
                 isSubscriptionEnabled: Bool,
-                accessTokenProvider: @escaping () -> String?) {
+                accessTokenProvider: @escaping AccessTokenProvider) {
         keychainStore = NetworkProtectionKeychainStore(label: Defaults.tokenStoreEntryLabel,
                                                        serviceName: serviceName,
                                                        keychainType: keychainType)

diff --git a/Sources/NetworkProtection/Repositories/NetworkProtectionLocationListRepository.swift b/Sources/NetworkProtection/Repositories/NetworkProtectionLocationListRepository.swift
@@ -55,6 +55,7 @@ final public class NetworkProtectionLocationListCompositeRepository: NetworkProt
     }
 
     @MainActor
+    @discardableResult
     public func fetchLocationList() async throws -> [NetworkProtectionLocation] {
         guard !canUseCache else {
             return Self.locationList

diff --git a/Sources/Subscription/AccountStorage/AccountKeychainStorage.swift b/Sources/Subscription/AccountStorage/AccountKeychainStorage.swift
@@ -48,57 +48,56 @@ public enum AccountKeychainAccessError: Error, Equatable {
     }
 }
 
-public class AccountKeychainStorage: AccountStorage {
+public class AccountKeychainStorage: AccountStoring {
 
     public init() {}
 
     public func getAuthToken() throws -> String? {
-        try Self.getString(forField: .authToken)
+        try getString(forField: .authToken)
     }
 
     public func store(authToken: String) throws {
-        try Self.set(string: authToken, forField: .authToken)
+        try set(string: authToken, forField: .authToken)
     }
 
     public func getAccessToken() throws -> String? {
-        try Self.getString(forField: .accessToken)
+        try getString(forField: .accessToken)
     }
 
     public func store(accessToken: String) throws {
-        try Self.set(string: accessToken, forField: .accessToken)
+        try set(string: accessToken, forField: .accessToken)
     }
 
     public func getEmail() throws -> String? {
-        try Self.getString(forField: .email)
+        try getString(forField: .email)
     }
 
     public func getExternalID() throws -> String? {
-        try Self.getString(forField: .externalID)
+        try getString(forField: .externalID)
     }
 
     public func store(externalID: String?) throws {
         if let externalID = externalID, !externalID.isEmpty {
-            try Self.set(string: externalID, forField: .externalID)
+            try set(string: externalID, forField: .externalID)
         } else {
-            try Self.deleteItem(forField: .externalID)
+            try deleteItem(forField: .externalID)
         }
     }
 
     public func store(email: String?) throws {
         if let email = email, !email.isEmpty {
-            try Self.set(string: email, forField: .email)
+            try set(string: email, forField: .email)
         } else {
-            try Self.deleteItem(forField: .email)
+            try deleteItem(forField: .email)
         }
     }
 
     public func clearAuthenticationState() throws {
-        try Self.deleteItem(forField: .authToken)
-        try Self.deleteItem(forField: .accessToken)
-        try Self.deleteItem(forField: .email)
-        try Self.deleteItem(forField: .externalID)
+        try deleteItem(forField: .authToken)
+        try deleteItem(forField: .accessToken)
+        try deleteItem(forField: .email)
+        try deleteItem(forField: .externalID)
     }
-
 }
 
 private extension AccountKeychainStorage {
@@ -118,7 +117,7 @@ private extension AccountKeychainStorage {
         }
     }
 
-    static func getString(forField field: AccountKeychainField) throws -> String? {
+    func getString(forField field: AccountKeychainField) throws -> String? {
         guard let data = try retrieveData(forField: field) else {
             return nil
         }
@@ -130,7 +129,7 @@ private extension AccountKeychainStorage {
         }
     }
 
-    static func retrieveData(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws -> Data? {
+    func retrieveData(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws -> Data? {
         let query: [String: Any] = [
             kSecClass as String: kSecClassGenericPassword,
             kSecMatchLimit as String: kSecMatchLimitOne,
@@ -155,7 +154,7 @@ private extension AccountKeychainStorage {
         }
     }
 
-    static func set(string: String, forField field: AccountKeychainField) throws {
+    func set(string: String, forField field: AccountKeychainField) throws {
         guard let stringData = string.data(using: .utf8) else {
             return
         }
@@ -164,7 +163,7 @@ private extension AccountKeychainStorage {
         try store(data: stringData, forField: field)
     }
 
-    static func store(data: Data, forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws {
+    func store(data: Data, forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws {
         let query = [
             kSecClass: kSecClassGenericPassword,
             kSecAttrSynchronizable: false,
@@ -180,7 +179,7 @@ private extension AccountKeychainStorage {
         }
     }
 
-    static func deleteItem(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws {
+    func deleteItem(forField field: AccountKeychainField, useDataProtectionKeychain: Bool = true) throws {
         let query: [String: Any] = [
             kSecClass as String: kSecClassGenericPassword,
             kSecAttrService as String: field.keyValue,

diff --git a/...ption/AccountStorage/AccountStorage.swift → ...ption/AccountStorage/AccountStoring.swift b/...ption/AccountStorage/AccountStorage.swift → ...ption/AccountStorage/AccountStoring.swift
@@ -1,5 +1,5 @@
 //
-//  AccountStorage.swift
+//  AccountStoring.swift
 //
 //  Copyright © 2023 DuckDuckGo. All rights reserved.
 //
@@ -18,7 +18,7 @@
 
 import Foundation
 
-public protocol AccountStorage: AnyObject {
+public protocol AccountStoring: AnyObject {
     func getAuthToken() throws -> String?
     func store(authToken: String) throws
     func getAccessToken() throws -> String?

diff --git a/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift b/Sources/Subscription/AccountStorage/SubscriptionTokenKeychainStorage.swift
@@ -18,7 +18,7 @@
 
 import Foundation
 
-public class SubscriptionTokenKeychainStorage: SubscriptionTokenStorage {
+public class SubscriptionTokenKeychainStorage: SubscriptionTokenStoring {
 
     private let keychainType: KeychainType
 
@@ -65,6 +65,7 @@ private extension SubscriptionTokenKeychainStorage {
             throw AccountKeychainAccessError.failedToDecodeKeychainDataAsString
         }
     }
+
     func retrieveData(forField field: AccountKeychainField) throws -> Data? {
         var query = defaultAttributes()
         query[kSecAttrService] = field.keyValue
@@ -153,11 +154,8 @@ private extension SubscriptionTokenKeychainStorage {
 
 public enum KeychainType {
     case dataProtection(_ accessGroup: AccessGroup)
-
     /// Uses the system keychain.
-    ///
     case system
-
     case fileBased
 
     public enum AccessGroup {

diff --git a/...untStorage/SubscriptionTokenStorage.swift → ...untStorage/SubscriptionTokenStoring.swift b/...untStorage/SubscriptionTokenStorage.swift → ...untStorage/SubscriptionTokenStoring.swift
@@ -1,5 +1,5 @@
 //
-//  SubscriptionTokenStorage.swift
+//  SubscriptionTokenStoring.swift
 //
 //  Copyright © 2024 DuckDuckGo. All rights reserved.
 //
@@ -18,7 +18,7 @@
 
 import Foundation
 
-public protocol SubscriptionTokenStorage: AnyObject {
+public protocol SubscriptionTokenStoring: AnyObject {
     func getAccessToken() throws -> String?
     func store(accessToken: String) throws
     func removeAccessToken() throws

diff --git a/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift b/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift
@@ -23,26 +23,33 @@ import Common
 @available(macOS 12.0, iOS 15.0, *)
 public final class AppStoreAccountManagementFlow {
 
-        public enum Error: Swift.Error {
-            case noPastTransaction
-            case authenticatingWithTransactionFailed
-        }
+    private let subscriptionManager: SubscriptionManaging
+    private var accountManager: AccountManaging {
+        subscriptionManager.accountManager
+    }
+
+    public init(subscriptionManager: SubscriptionManaging) {
+        self.subscriptionManager = subscriptionManager
+    }
+
+    public enum Error: Swift.Error {
+        case noPastTransaction
+        case authenticatingWithTransactionFailed
+    }
 
     @discardableResult
-    public static func refreshAuthTokenIfNeeded(subscriptionAppGroup: String) async -> Result<String, AppStoreAccountManagementFlow.Error> {
+    public func refreshAuthTokenIfNeeded() async -> Result<String, AppStoreAccountManagementFlow.Error> {
         os_log(.info, log: .subscription, "[AppStoreAccountManagementFlow] refreshAuthTokenIfNeeded")
-        let accountManager = AccountManager(subscriptionAppGroup: subscriptionAppGroup)
-
         var authToken = accountManager.authToken ?? ""
 
         // Check if auth token if still valid
-        if case let .failure(validateTokenError) = await AuthService.validateToken(accessToken: authToken) {
+        if case let .failure(validateTokenError) = await subscriptionManager.authService.validateToken(accessToken: authToken) {
             os_log(.error, log: .subscription, "[AppStoreAccountManagementFlow] validateToken error: %{public}s", String(reflecting: validateTokenError))
 
             // In case of invalid token attempt store based authentication to obtain a new one
-            guard let lastTransactionJWSRepresentation = await PurchaseManager.mostRecentTransaction() else { return .failure(.noPastTransaction) }
+            guard let lastTransactionJWSRepresentation = await subscriptionManager.storePurchaseManager().mostRecentTransaction() else { return .failure(.noPastTransaction) }
 
-            switch await AuthService.storeLogin(signature: lastTransactionJWSRepresentation) {
+            switch await subscriptionManager.authService.storeLogin(signature: lastTransactionJWSRepresentation) {
             case .success(let response):
                 if response.externalID == accountManager.externalID {
                     authToken = response.authToken