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

IOS-8044 Fix multiple leaks #3940

Merged
merged 3 commits into from
Sep 27, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ class CommonUserTokensManager {
private let derivationStyle: DerivationStyle?
private let existingCurves: [EllipticCurve]
private let longHashesSupported: Bool
private weak var keysDerivingProvider: KeysDerivingProvider?

weak var keysDerivingProvider: KeysDerivingProvider?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

техническая правка, чтобы завести фабрику


private var pendingUserTokensSyncCompletions: [() -> Void] = []
private var bag: Set<AnyCancellable> = []

Expand All @@ -34,7 +36,6 @@ class CommonUserTokensManager {
walletModelsManager: WalletModelsManager,
derivationStyle: DerivationStyle?,
derivationManager: DerivationManager?,
keysDerivingProvider: KeysDerivingProvider,
existingCurves: [EllipticCurve],
longHashesSupported: Bool
) {
Expand All @@ -44,7 +45,6 @@ class CommonUserTokensManager {
self.walletModelsManager = walletModelsManager
self.derivationStyle = derivationStyle
self.derivationManager = derivationManager
self.keysDerivingProvider = keysDerivingProvider
self.existingCurves = existingCurves
self.longHashesSupported = longHashesSupported
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class CommonUserWalletRepository: UserWalletRepository {
names: models.map(\.name)
)

let userWalletModel = CommonUserWalletModel(cardInfo: cardInfo)
let userWalletModel = CommonUserWalletModelFactory().makeModel(cardInfo: cardInfo)
if let userWalletModel {
initializeServices(for: userWalletModel)
}
Expand Down Expand Up @@ -498,7 +498,7 @@ class CommonUserWalletRepository: UserWalletRepository {
migrateNamesIfNeeded(&savedUserWallets)

models = savedUserWallets.map { userWalletStorageItem in
if let userWallet = CommonUserWalletModel(userWallet: userWalletStorageItem) {
if let userWallet = CommonUserWalletModelFactory().makeModel(userWallet: userWalletStorageItem) {
return userWallet
} else {
return LockedUserWalletModel(with: userWalletStorageItem)
Expand All @@ -511,7 +511,7 @@ class CommonUserWalletRepository: UserWalletRepository {
guard let index = models.firstIndex(where: { $0.userWalletId == userWalletId }) else { return }

guard let savedUserWallet = savedUserWallet(with: userWalletId),
let userWalletModel = CommonUserWalletModel(userWallet: savedUserWallet) else {
let userWalletModel = CommonUserWalletModelFactory().makeModel(userWallet: savedUserWallet) else {
return
}

Expand Down
101 changes: 30 additions & 71 deletions Tangem/App/ViewModels/CommonUserWalletModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,42 +21,17 @@ class CommonUserWalletModel {
@Injected(\.pushNotificationsInteractor) private var pushNotificationsInteractor: PushNotificationsInteractor

let walletModelsManager: WalletModelsManager

var userTokensManager: UserTokensManager { _userTokensManager }

private lazy var _userTokensManager = CommonUserTokensManager(
userWalletId: userWalletId,
shouldLoadSwapAvailability: config.isFeatureVisible(.swapping),
userTokenListManager: userTokenListManager,
walletModelsManager: walletModelsManager,
derivationStyle: config.derivationStyle,
derivationManager: derivationManager,
keysDerivingProvider: self,
existingCurves: config.existingCurves,
longHashesSupported: config.hasFeature(.longHashes)
)

let userTokensManager: UserTokensManager
let userTokenListManager: UserTokenListManager
let keysRepository: KeysRepository
let derivationManager: DerivationManager?
Copy link
Collaborator Author

@tureck1y tureck1y Sep 24, 2024

Choose a reason for hiding this comment

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

CommonUserWalletModel не деинитилась из-за derivationManager, который юзается в других сервисах, которые, в свою очередь лежали тут как lazy. Без изменения логики унес всю сборку из конструктора в отдельную фабрику, что в принципе чище, заодно и решило утечку

let totalBalanceProvider: TotalBalanceProviding

private let walletManagersRepository: WalletManagersRepository
private let cardImageProvider = CardImageProvider()

private var associatedCardIds: Set<String>

lazy var derivationManager: DerivationManager? = {
guard config.hasFeature(.hdWallets) else {
return nil
}

let commonDerivationManager = CommonDerivationManager(
keysRepository: keysRepository,
userTokenListManager: userTokenListManager
)

commonDerivationManager.delegate = self
return commonDerivationManager
}()

var signer: TangemSigner { _signer }

var cardId: String { cardInfo.card.cardId }
Expand All @@ -77,12 +52,6 @@ class CommonUserWalletModel {

let userWalletId: UserWalletId

lazy var totalBalanceProvider: TotalBalanceProviding = TotalBalanceProvider(
userWalletId: userWalletId,
walletModelsManager: walletModelsManager,
derivationManager: derivationManager
)

private(set) var cardInfo: CardInfo
private var tangemSdk: TangemSdk?
var config: UserWalletConfig
Expand All @@ -99,44 +68,34 @@ class CommonUserWalletModel {
}
}

convenience init?(userWallet: StoredUserWallet) {
let cardInfo = userWallet.cardInfo()
self.init(cardInfo: cardInfo)
let allIds = associatedCardIds.union(userWallet.associatedCardIds)
associatedCardIds = allIds
}

init?(cardInfo: CardInfo) {
let config = UserWalletConfigFactory(cardInfo).makeConfig()
associatedCardIds = [cardInfo.card.cardId]
guard let userWalletIdSeed = config.userWalletIdSeed,
let walletManagerFactory = try? config.makeAnyWalletManagerFactory() else {
return nil
}

init(
cardInfo: CardInfo,
config: UserWalletConfig,
userWalletId: UserWalletId,
associatedCardIds: Set<String>,
walletManagersRepository: WalletManagersRepository,
walletModelsManager: WalletModelsManager,
userTokensManager: CommonUserTokensManager,
userTokenListManager: UserTokenListManager,
keysRepository: KeysRepository,
derivationManager: DerivationManager?,
totalBalanceProvider: TotalBalanceProviding
) {
self.cardInfo = cardInfo
self.config = config
keysRepository = CommonKeysRepository(with: cardInfo.card.wallets)

userWalletId = UserWalletId(with: userWalletIdSeed)
userTokenListManager = CommonUserTokenListManager(
userWalletId: userWalletId.value,
supportedBlockchains: config.supportedBlockchains,
hdWalletsSupported: config.hasFeature(.hdWallets),
hasTokenSynchronization: config.hasFeature(.tokenSynchronization),
defaultBlockchains: config.defaultBlockchains
)

walletManagersRepository = CommonWalletManagersRepository(
keysProvider: keysRepository,
userTokenListManager: userTokenListManager,
walletManagerFactory: walletManagerFactory
)

walletModelsManager = CommonWalletModelsManager(
walletManagersRepository: walletManagersRepository,
walletModelsFactory: config.makeWalletModelsFactory()
)
self.userWalletId = userWalletId

var associatedCardIds = associatedCardIds
associatedCardIds.insert(cardInfo.card.cardId)
self.associatedCardIds = associatedCardIds

self.walletManagersRepository = walletManagersRepository
self.walletModelsManager = walletModelsManager
self.userTokensManager = userTokensManager
self.userTokenListManager = userTokenListManager
self.keysRepository = keysRepository
self.derivationManager = derivationManager
self.totalBalanceProvider = totalBalanceProvider

_signer = config.tangemSigner
_userWalletNamePublisher = .init(cardInfo.name)
Expand Down
97 changes: 97 additions & 0 deletions Tangem/App/ViewModels/CommonUserWalletModelFactory.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
//
// CommonUserWalletModelFactory.swift
// Tangem
//
// Created by Alexander Osokin on 24.09.2024.
// Copyright © 2024 Tangem AG. All rights reserved.
//

import Foundation

struct CommonUserWalletModelFactory {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

По сути код без каких-либо изменений логики переехал сюда

func makeModel(userWallet: StoredUserWallet) -> CommonUserWalletModel? {
let cardInfo = userWallet.cardInfo()
return makeModel(cardInfo: cardInfo, associatedCardIds: userWallet.associatedCardIds)
}

func makeModel(cardInfo: CardInfo, associatedCardIds: Set<String> = []) -> CommonUserWalletModel? {
let config = UserWalletConfigFactory(cardInfo).makeConfig()

guard let userWalletIdSeed = config.userWalletIdSeed,
let walletManagerFactory = try? config.makeAnyWalletManagerFactory() else {
return nil
}
Comment on lines +20 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: как будто так чуток читабельнее

Suggested change
guard let userWalletIdSeed = config.userWalletIdSeed,
let walletManagerFactory = try? config.makeAnyWalletManagerFactory() else {
return nil
}
guard
let userWalletIdSeed = config.userWalletIdSeed,
let walletManagerFactory = try? config.makeAnyWalletManagerFactory()
else {
return nil
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Когда на это смотришь в гитхабе, то да) А в коде, с подсветкой синтаксиса кажется и так норм)


let userWalletId = UserWalletId(with: userWalletIdSeed)

let keysRepository = CommonKeysRepository(with: cardInfo.card.wallets)

let userTokenListManager = CommonUserTokenListManager(
userWalletId: userWalletId.value,
supportedBlockchains: config.supportedBlockchains,
hdWalletsSupported: config.hasFeature(.hdWallets),
hasTokenSynchronization: config.hasFeature(.tokenSynchronization),
defaultBlockchains: config.defaultBlockchains
)

let walletManagersRepository = CommonWalletManagersRepository(
keysProvider: keysRepository,
userTokenListManager: userTokenListManager,
walletManagerFactory: walletManagerFactory
)

let walletModelsManager = CommonWalletModelsManager(
walletManagersRepository: walletManagersRepository,
walletModelsFactory: config.makeWalletModelsFactory()
)

let derivationManager: CommonDerivationManager? = {
guard config.hasFeature(.hdWallets) else {
return nil
}

let commonDerivationManager = CommonDerivationManager(
keysRepository: keysRepository,
userTokenListManager: userTokenListManager
)

return commonDerivationManager
}()

let totalBalanceProvider = TotalBalanceProvider(
userWalletId: userWalletId,
walletModelsManager: walletModelsManager,
derivationManager: derivationManager
)

let userTokensManager = CommonUserTokensManager(
userWalletId: userWalletId,
shouldLoadSwapAvailability: config.isFeatureVisible(.swapping),
userTokenListManager: userTokenListManager,
walletModelsManager: walletModelsManager,
derivationStyle: config.derivationStyle,
derivationManager: derivationManager,
existingCurves: config.existingCurves,
longHashesSupported: config.hasFeature(.longHashes)
)

let model = CommonUserWalletModel(
cardInfo: cardInfo,
config: config,
userWalletId: userWalletId,
associatedCardIds: associatedCardIds,
walletManagersRepository: walletManagersRepository,
walletModelsManager: walletModelsManager,
userTokensManager: userTokensManager,
userTokenListManager: userTokenListManager,
keysRepository: keysRepository,
derivationManager: derivationManager,
totalBalanceProvider: totalBalanceProvider
)

derivationManager?.delegate = model
userTokensManager.keysDerivingProvider = model

return model
}
}
2 changes: 1 addition & 1 deletion Tangem/Common/Preview/PreviewCard.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ enum PreviewCard {
var userWalletModel: CommonUserWalletModel {
let card = CardDTO(card: card)
let ci = CardInfo(card: card, walletData: walletData, name: "Name")
let vm = CommonUserWalletModel(cardInfo: ci)!
let vm = CommonUserWalletModelFactory().makeModel(cardInfo: ci)!
if let blockchain = blockchain {
let factory = WalletManagerFactory(
config: .init(
Expand Down
40 changes: 26 additions & 14 deletions Tangem/Common/UI/RefreshableScrollView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,36 +14,27 @@ typealias OnRefresh = (_ completionHandler: @escaping RefreshCompletionHandler)
/// Author: The SwiftUI Lab.
/// Full article: https://swiftui-lab.com/scrollview-pull-to-refresh/.
struct RefreshableScrollView<Content: View>: View {
let onRefresh: OnRefresh
let content: Content
private let refreshContainer: RefreshContainer

init(
onRefresh: @escaping OnRefresh,
@ViewBuilder content: () -> Content
) {
self.onRefresh = onRefresh
self.content = content()
refreshContainer = RefreshContainer(onRefresh: onRefresh)
}

var body: some View {
if #available(iOS 16, *) {
ScrollView(.vertical, showsIndicators: false) {
content
}
.refreshable {
await refreshAsync()
.refreshable { [weak refreshContainer] in
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Самая жесткая утечка, помог коммент @Vyachesl0ve. Теперь в памяти ничего не остается, это юзалось и в свопе, так что там тоже пофиксится, если конечно не найдется других утечек

await refreshContainer?.refreshAsync()
}
} else {
RefreshableScrollViewCompat(onRefresh: onRefresh, content: content)
}
}

@MainActor
private func refreshAsync() async {
await withCheckedContinuation { continuation in
onRefresh {
continuation.resume()
}
RefreshableScrollViewCompat(onRefresh: refreshContainer.onRefresh, content: content)
}
}
}
Expand Down Expand Up @@ -167,6 +158,27 @@ private struct RefreshableScrollViewCompat<Content: View>: View {
}
}

// MARK: - RefreshContainer

extension RefreshableScrollView {
private final class RefreshContainer {
let onRefresh: OnRefresh

init(onRefresh: @escaping OnRefresh) {
self.onRefresh = onRefresh
}

@MainActor
func refreshAsync() async {
await withCheckedContinuation { continuation in
onRefresh {
continuation.resume()
}
}
}
}
}

// MARK: - Previews

struct RefreshableScrollViewView_Previews: PreviewProvider {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ final class MultiWalletMainContentViewModel: ObservableObject {

var tokenListSyncSubscription: AnyCancellable?
tokenListSyncSubscription = Publishers.Zip(tokenListSyncPublisher, sectionsPublisher)
.prefix(1)
.receive(on: DispatchQueue.main)
.withWeakCaptureOf(self)
.sink { viewModel, _ in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class OnboardingViewModel<Step: OnboardingStep, Coordinator: OnboardingRoutable>
}

func initializeUserWallet(from cardInfo: CardInfo) {
guard let userWallet = CommonUserWalletModel(cardInfo: cardInfo) else { return }
guard let userWallet = CommonUserWalletModelFactory().makeModel(cardInfo: cardInfo) else { return }

userWalletRepository.initializeServices(for: userWallet)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import Foundation

extension CommonUserWalletModel {
static let mock = CommonUserWalletModel(
static let mock = CommonUserWalletModelFactory().makeModel(
cardInfo: CardMock.wallet.cardInfo
)
}
Loading
Loading