Skip to content

refactor(storage): inject KeychainHelper into storage classes via init#1153

Merged
datlechin merged 2 commits into
mainfrom
refactor/inject-keychain-into-storage-classes
May 9, 2026
Merged

refactor(storage): inject KeychainHelper into storage classes via init#1153
datlechin merged 2 commits into
mainfrom
refactor/inject-keychain-into-storage-classes

Conversation

@datlechin

Copy link
Copy Markdown
Member

Summary

Four storage classes that read the keychain (AIKeyStorage, SSHProfileStorage, ConnectionStorage, LicenseStorage) now hold a private let keychain: KeychainHelper and read through it instead of dialing KeychainHelper.shared 24 times.

  • Each class's init now accepts keychain: KeychainHelper = .shared (matching the pattern already used for syncTracker and appSettings in ConnectionStorage).
  • private init() is dropped on AIKeyStorage, SSHProfileStorage, and LicenseStorage so tests can inject a different keychain. The static let shared = X() line still calls the parameterless default.
  • 24 raw KeychainHelper.shared.X calls converted to keychain.X.

Why this matters

This is the next slice of audit findings 3.1 + arch 5.1 ("complete the AppServices threading"). After PRs #1151 and #1152 widened AppServices and converted services-aware files, this PR addresses the storage layer's internal cross-references. KeychainHelper is now a dependency of each storage class, not a global it reaches for. Fits the audit's recommended "complete the migration" path.

Test plan

  • Save / load / delete a database password.
  • Save / load / delete an SSH password / key passphrase / TOTP secret on an SSH profile.
  • Save / load / delete an AI provider API key.
  • Save / delete the license key (Pro purchase / sign-out).
  • swiftlint --strict clean on changed files.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin datlechin merged commit cd45b08 into main May 9, 2026
2 checks passed
@datlechin datlechin deleted the refactor/inject-keychain-into-storage-classes branch May 9, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant