Skip to content

Commit 6fc5491

Browse files
fedemkrfedemkr
authored
[PM-24290] Improve autofill cipher lists performance (#1939)
1 parent 9f0c578 commit 6fc5491

17 files changed

+629
-287
lines changed

‎BitwardenShared/Core/Platform/Services/ServiceContainer.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
722722
policyService: policyService,
723723
stateService: stateService,
724724
vaultListPreparedDataBuilderFactory: DefaultVaultListPreparedDataBuilderFactory(
725+
cipherService: cipherService,
725726
clientService: clientService,
726727
errorReporter: errorReporter,
727728
stateService: stateService,

‎BitwardenShared/Core/Vault/Helpers/TestHelpers/MockVaultListPreparedDataBuilder+Extensions.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ extension MockVaultListPreparedDataBuilder {
1616
helper.recordCall("addFavoriteItem")
1717
return self
1818
}
19+
addFido2ItemClosure = { _ -> VaultListPreparedDataBuilder in
20+
helper.recordCall("addFido2Item")
21+
return self
22+
}
1923
addFolderItemClosure = { _, _, _ -> VaultListPreparedDataBuilder in
2024
helper.recordCall("addFolderItem")
2125
return self

‎BitwardenShared/Core/Vault/Helpers/TestHelpers/MockVaultListSectionsBuilder+Extensions.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ extension MockVaultListSectionsBuilder {
88
helper.recordCall("addAutofillPasswordsSection")
99
return self
1010
}
11+
addAutofillCombinedSingleSectionClosure = { () -> VaultListSectionsBuilder in
12+
helper.recordCall("addAutofillCombinedSingleSection")
13+
return self
14+
}
1115
addCipherDecryptionFailureIdsClosure = { () -> VaultListSectionsBuilder in
1216
helper.recordCall("addCipherDecryptionFailureIds")
1317
return self

‎BitwardenShared/Core/Vault/Helpers/VaultListDataPreparator.swift‎

Lines changed: 93 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,44 +6,55 @@ import BitwardenSdk
66
/// This decrypts and process data iteratively in batches to improve time and memory on the overall
77
/// grouping/filtering/preparation.
88
protocol VaultListDataPreparator { // sourcery: AutoMockable
9-
/// Prepares data for the vault list builder.
9+
/// Prepares autofill's passwords data for the vault list builder.
1010
/// - Parameters:
1111
/// - ciphers: An array of `Cipher` objects to be processed.
12-
/// - collections: An array of `Collection` objects to be processed.
13-
/// - folders: An array of `Folder` objects to be processed.
1412
/// - filter: A `VaultListFilter` object that defines the filtering criteria for the vault list.
1513
/// - Returns: An optional `VaultListPreparedData` object containing the prepared data for the vault list.
1614
/// Returns `nil` if the vault is empty.
17-
func prepareData(
15+
func prepareAutofillPasswordsData(
1816
from ciphers: [Cipher],
19-
collections: [Collection],
20-
folders: [Folder],
2117
filter: VaultListFilter
2218
) async throws -> VaultListPreparedData?
2319

24-
/// Prepares group data for the vault list builder.
20+
/// Prepares autofill's data on passwords + Fido2 combined in a single section for the vault list builder.
21+
/// - Parameters:
22+
/// - ciphers: An array of `Cipher` objects to be processed.
23+
/// - filter: A `VaultListFilter` object that defines the filtering criteria for the vault list.
24+
/// - Returns: An optional `VaultListPreparedData` object containing the prepared data for the vault list.
25+
/// Returns `nil` if the vault is empty.
26+
func prepareAutofillCombinedSingleData(
27+
from ciphers: [Cipher],
28+
filter: VaultListFilter
29+
) async throws -> VaultListPreparedData?
30+
31+
/// Prepares data for the vault list builder.
2532
/// - Parameters:
2633
/// - ciphers: An array of `Cipher` objects to be processed.
2734
/// - collections: An array of `Collection` objects to be processed.
2835
/// - folders: An array of `Folder` objects to be processed.
2936
/// - filter: A `VaultListFilter` object that defines the filtering criteria for the vault list.
3037
/// - Returns: An optional `VaultListPreparedData` object containing the prepared data for the vault list.
3138
/// Returns `nil` if the vault is empty.
32-
func prepareGroupData(
39+
func prepareData(
3340
from ciphers: [Cipher],
3441
collections: [Collection],
3542
folders: [Folder],
3643
filter: VaultListFilter
3744
) async throws -> VaultListPreparedData?
3845

39-
/// Prepares autofill's passwords data for the vault list builder.
46+
/// Prepares group data for the vault list builder.
4047
/// - Parameters:
4148
/// - ciphers: An array of `Cipher` objects to be processed.
49+
/// - collections: An array of `Collection` objects to be processed.
50+
/// - folders: An array of `Folder` objects to be processed.
4251
/// - filter: A `VaultListFilter` object that defines the filtering criteria for the vault list.
4352
/// - Returns: An optional `VaultListPreparedData` object containing the prepared data for the vault list.
4453
/// Returns `nil` if the vault is empty.
45-
func prepareAutofillPasswordsData(
54+
func prepareGroupData(
4655
from ciphers: [Cipher],
56+
collections: [Collection],
57+
folders: [Folder],
4758
filter: VaultListFilter
4859
) async throws -> VaultListPreparedData?
4960
}
@@ -71,6 +82,78 @@ struct DefaultVaultListDataPreparator: VaultListDataPreparator {
7182

7283
// MARK: Methods
7384

85+
func prepareAutofillPasswordsData(
86+
from ciphers: [Cipher],
87+
filter: VaultListFilter
88+
) async throws -> VaultListPreparedData? {
89+
guard !ciphers.isEmpty, let uri = filter.uri else {
90+
return nil
91+
}
92+
93+
let cipherMatchingHelper = await cipherMatchingHelperFactory.make(uri: uri)
94+
95+
var preparedDataBuilder = vaultListPreparedDataBuilderFactory.make()
96+
let restrictedOrganizationIds = await prepareRestrictedOrganizationIds(builder: preparedDataBuilder)
97+
98+
await ciphersClientWrapperService.decryptAndProcessCiphersInBatch(
99+
ciphers: ciphers
100+
) { decryptedCipher in
101+
guard decryptedCipher.deletedDate == nil,
102+
decryptedCipher.passesRestrictItemTypesPolicy(restrictedOrganizationIds) else {
103+
return
104+
}
105+
106+
let matchResult = cipherMatchingHelper.doesCipherMatch(cipher: decryptedCipher)
107+
108+
preparedDataBuilder = await preparedDataBuilder.addItem(
109+
withMatchResult: matchResult,
110+
cipher: decryptedCipher
111+
)
112+
}
113+
114+
return preparedDataBuilder.build()
115+
}
116+
117+
func prepareAutofillCombinedSingleData(
118+
from ciphers: [Cipher],
119+
filter: VaultListFilter
120+
) async throws -> VaultListPreparedData? {
121+
guard !ciphers.isEmpty, let uri = filter.uri else {
122+
return nil
123+
}
124+
125+
let cipherMatchingHelper = await cipherMatchingHelperFactory.make(uri: uri)
126+
127+
var preparedDataBuilder = vaultListPreparedDataBuilderFactory.make()
128+
let restrictedOrganizationIds = await prepareRestrictedOrganizationIds(builder: preparedDataBuilder)
129+
130+
await ciphersClientWrapperService.decryptAndProcessCiphersInBatch(
131+
ciphers: ciphers
132+
) { decryptedCipher in
133+
guard decryptedCipher.deletedDate == nil,
134+
decryptedCipher.passesRestrictItemTypesPolicy(restrictedOrganizationIds) else {
135+
return
136+
}
137+
138+
let matchResult = cipherMatchingHelper.doesCipherMatch(cipher: decryptedCipher)
139+
guard matchResult != .none else {
140+
return
141+
}
142+
143+
guard decryptedCipher.type.loginListView?.hasFido2 == true else {
144+
preparedDataBuilder = await preparedDataBuilder.addItem(
145+
forGroup: .login,
146+
with: decryptedCipher
147+
)
148+
return
149+
}
150+
151+
preparedDataBuilder = await preparedDataBuilder.addFido2Item(cipher: decryptedCipher)
152+
}
153+
154+
return preparedDataBuilder.build()
155+
}
156+
74157
func prepareData(
75158
from ciphers: [Cipher],
76159
collections: [Collection],
@@ -164,38 +247,6 @@ struct DefaultVaultListDataPreparator: VaultListDataPreparator {
164247
return preparedDataBuilder.build()
165248
}
166249

167-
func prepareAutofillPasswordsData(
168-
from ciphers: [Cipher],
169-
filter: VaultListFilter
170-
) async throws -> VaultListPreparedData? {
171-
guard !ciphers.isEmpty, let uri = filter.uri else {
172-
return nil
173-
}
174-
175-
let cipherMatchingHelper = await cipherMatchingHelperFactory.make(uri: uri)
176-
177-
var preparedDataBuilder = vaultListPreparedDataBuilderFactory.make()
178-
let restrictedOrganizationIds: [String] = await prepareRestrictedOrganizationIds(builder: preparedDataBuilder)
179-
180-
await ciphersClientWrapperService.decryptAndProcessCiphersInBatch(
181-
ciphers: ciphers
182-
) { decryptedCipher in
183-
guard decryptedCipher.deletedDate == nil,
184-
decryptedCipher.passesRestrictItemTypesPolicy(restrictedOrganizationIds) else {
185-
return
186-
}
187-
188-
let matchResult = cipherMatchingHelper.doesCipherMatch(cipher: decryptedCipher)
189-
190-
preparedDataBuilder = await preparedDataBuilder.addItem(
191-
withMatchResult: matchResult,
192-
cipher: decryptedCipher
193-
)
194-
}
195-
196-
return preparedDataBuilder.build()
197-
}
198-
199250
// MARK: Private
200251

201252
/// Returns the restricted organization IDs for the `.restrictItemTypes` policy if enabled

‎BitwardenShared/Core/Vault/Helpers/VaultListDataPreparatorTests.swift‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,131 @@ class VaultListDataPreparatorTests: BitwardenTestCase { // swiftlint:disable:thi
7676

7777
// MARK: Tests
7878

79+
/// `prepareAutofillCombinedSingleData(from:filter:)` returns `nil` when no ciphers passed.
80+
func test_prepareAutofillCombinedSingleData_noCiphers() async throws {
81+
let result = try await subject.prepareAutofillCombinedSingleData(
82+
from: [],
83+
filter: VaultListFilter(uri: "https://example.com")
84+
)
85+
XCTAssertNil(result)
86+
}
87+
88+
/// `prepareAutofillCombinedSingleData(from:filter:)` returns `nil` when filter passed doesn't have the URI to filter.
89+
func test_prepareAutofillCombinedSingleData_noFilterUri() async throws {
90+
let result = try await subject.prepareAutofillCombinedSingleData(
91+
from: [.fixture()],
92+
filter: VaultListFilter()
93+
)
94+
XCTAssertNil(result)
95+
}
96+
97+
/// `prepareAutofillCombinedSingleData(from:filter:)` returns the prepared data for a cipher with login and no Fido2.
98+
func test_prepareAutofillCombinedSingleData_returnsPreparedDataForLoginNoFido2() async throws {
99+
ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture(
100+
login: .fixture(
101+
hasFido2: false,
102+
uris: [.fixture(uri: "https://example.com", match: .exact)]
103+
)
104+
)
105+
cipherMatchingHelper.doesCipherMatchReturnValue = .exact
106+
107+
let result = try await subject.prepareAutofillCombinedSingleData(
108+
from: [
109+
.fixture(
110+
login: .fixture(
111+
fido2Credentials: [],
112+
uris: [.fixture(uri: "https://example.com", match: .exact)]
113+
)
114+
),
115+
],
116+
filter: VaultListFilter(uri: "https://example.com")
117+
)
118+
119+
XCTAssertEqual(mockCallOrderHelper.callOrder, [
120+
"addItemForGroup"
121+
])
122+
XCTAssertNotNil(result)
123+
}
124+
125+
/// `prepareAutofillCombinedSingleData(from:filter:)` returns the prepared data for a cipher with login and Fido2.
126+
func test_prepareAutofillCombinedSingleData_returnsPreparedDataForLoginWithFido2() async throws {
127+
ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture(
128+
login: .fixture(
129+
hasFido2: true,
130+
uris: [.fixture(uri: "https://example.com", match: .exact)]
131+
)
132+
)
133+
cipherMatchingHelper.doesCipherMatchReturnValue = .exact
134+
135+
let result = try await subject.prepareAutofillCombinedSingleData(
136+
from: [
137+
.fixture(
138+
login: .fixture(
139+
fido2Credentials: [.fixture()],
140+
uris: [.fixture(uri: "https://example.com", match: .exact)]
141+
)
142+
),
143+
],
144+
filter: VaultListFilter(uri: "https://example.com")
145+
)
146+
147+
XCTAssertEqual(mockCallOrderHelper.callOrder, [
148+
"addFido2Item"
149+
])
150+
XCTAssertNotNil(result)
151+
}
152+
153+
/// `prepareAutofillCombinedSingleData(from:filter:)` returns the prepared data filtering out cipher as it doesn't pass restrict item type policy.
154+
@MainActor
155+
func test_prepareAutofillCombinedSingleData_doesNotPassRestrictItemPolicy() async throws {
156+
configService.featureFlagsBool[.removeCardPolicy] = true
157+
ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture(
158+
id: "1",
159+
organizationId: "1",
160+
type: .card(.fixture())
161+
)
162+
policyService.policyAppliesToUserPolicies = [
163+
.fixture(organizationId: "1"),
164+
]
165+
cipherMatchingHelper.doesCipherMatchReturnValue = .exact
166+
167+
let result = try await subject.prepareAutofillCombinedSingleData(
168+
from: [
169+
.fixture(type: .card),
170+
],
171+
filter: VaultListFilter(uri: "https://example.com")
172+
)
173+
174+
XCTAssertEqual(mockCallOrderHelper.callOrder, [
175+
"prepareRestrictItemsPolicyOrganizations"
176+
])
177+
XCTAssertNotNil(result)
178+
}
179+
180+
/// `prepareAutofillCombinedSingleData(from:filter:)` returns the prepared data filtering out cipher as it's deleted.
181+
@MainActor
182+
func test_prepareAutofillCombinedSingleData_deletedCipher() async throws {
183+
ciphersClientWrapperService.decryptAndProcessCiphersInBatchOnCipherParameterToPass = .fixture(
184+
id: "1",
185+
deletedDate: .now
186+
)
187+
cipherMatchingHelper.doesCipherMatchReturnValue = .exact
188+
189+
let result = try await subject.prepareAutofillCombinedSingleData(
190+
from: [
191+
.fixture(
192+
login: .fixture(
193+
uris: [.fixture(uri: "https://example.com", match: .exact)]
194+
)
195+
),
196+
],
197+
filter: VaultListFilter(uri: "https://example.com")
198+
)
199+
200+
XCTAssertTrue(mockCallOrderHelper.callOrder.isEmpty)
201+
XCTAssertNotNil(result)
202+
}
203+
79204
/// `prepareData(from:collections:folders:filter:)` returns `nil` when no ciphers passed.
80205
func test_prepareData_noCiphers() async throws {
81206
let result = try await subject.prepareData(

‎BitwardenShared/Core/Vault/Helpers/VaultListDirectors/CombinedSingleAutofillVaultListDirectorStrategy.swift‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import BitwardenKit
2+
import BitwardenSdk
3+
import Combine
4+
5+
// MARK: - CombinedSingleAutofillVaultListDirectorStrategy
6+
7+
/// The director strategy to be used to build the Autofill's passwords + Fido2 combined single section.
8+
/// This would show a single section where both passwords and Fido2 credentials are displayed.
9+
struct CombinedSingleAutofillVaultListDirectorStrategy: VaultListDirectorStrategy {
10+
// MARK: Properties
11+
12+
/// The factory for creating vault list builders.
13+
let builderFactory: VaultListSectionsBuilderFactory
14+
/// The service used to manage syncing and updates to the user's ciphers.
15+
let cipherService: CipherService
16+
/// The helper used to prepare data for the vault list builder.
17+
let vaultListDataPreparator: VaultListDataPreparator
18+
19+
func build(
20+
filter: VaultListFilter
21+
) async throws -> AsyncThrowingPublisher<AnyPublisher<VaultListData, Error>> {
22+
try await cipherService.ciphersPublisher()
23+
.asyncTryMap { ciphers in
24+
try await build(from: ciphers, filter: filter)
25+
}
26+
.eraseToAnyPublisher()
27+
.values
28+
}
29+
30+
// MARK: Private methods
31+
32+
/// Builds the vault list sections.
33+
/// - Parameters:
34+
/// - ciphers: Ciphers to filter and include in the sections.
35+
/// - filter: Filter to be used to build the sections.
36+
/// - Returns: Sections to be displayed to the user.
37+
func build(
38+
from ciphers: [Cipher],
39+
filter: VaultListFilter
40+
) async throws -> VaultListData {
41+
guard let preparedData = try await vaultListDataPreparator.prepareAutofillCombinedSingleData(
42+
from: ciphers,
43+
filter: filter
44+
) else {
45+
return VaultListData()
46+
}
47+
48+
return builderFactory.make(withData: preparedData)
49+
.addAutofillCombinedSingleSection()
50+
.build()
51+
}
52+
}

0 commit comments

Comments
 (0)