-
Notifications
You must be signed in to change notification settings - Fork 138
[PM-38443] feat: Add FillAssistRepository and sync integration #2812
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
Open
andrebispo5
wants to merge
24
commits into
main
Choose a base branch
from
pm-38443/consume-fill-assist-data-and-cache
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 10 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
64b5123
[PM-38443] feat: Add FillAssistRepository and sync integration
andrebispo5 42e8f20
[PM-38443] fix: Remove SHA-256 integrity check and use URLSession.shaβ¦
andrebispo5 5d23296
[PM-38443] fix: Add @MainActor to FillAssistRepositoryTests to fix teβ¦
andrebispo5 333a8cc
[PM-38443] fix: Remove unused Services typealias and add selector pooβ¦
andrebispo5 1decad0
[PM-38443] fix: Update FillAssistRepository to use Constants.FillAssist
andrebispo5 993de63
[PM-38443] fix: Rename FillAssistRepository methods, inject TimeProviβ¦
andrebispo5 10bd65a
[PM-38443] test: Convert FillAssistRepositoryTests to Swift Testing aβ¦
andrebispo5 c9fe9f6
[PM-38443] fix: Add @MainActor and extract helpers to fix FillAssistRβ¦
andrebispo5 4d9842c
[PM-38443] fix: Filter empty field arrays from pooled rules and injecβ¦
andrebispo5 7738dc2
[PM-38443] refactor: Replace JSON-string test helpers with direct strβ¦
andrebispo5 67d82cf
[PM-38443] fix: Add missing DocC comments to FormsMapResponseModel inβ¦
andrebispo5 6e9e6e7
[PM-38444] feat: Use FillAssist cached rules to guide DOM field targeβ¦
andrebispo5 50b9219
Merge branch 'main' into pm-38443/consume-fill-assist-data-and-cache
andrebispo5 4a5f77e
[PM-39851] fix: Skip Fill Assist sync only when cache is readable andβ¦
andrebispo5 124ff13
[PM-39851] fix: Skip Fill Assist sync only when cache is readable andβ¦
andrebispo5 3511fee
[PM-39851] refactor: Use send(_:) instead of download for Fill Assistβ¦
andrebispo5 35fcc50
[PM-39851] refactor: Remove unused HTTPService.download(filename:) meβ¦
andrebispo5 6859eab
[PM-39851] fix: Run Fill Assist sync independently of vault revision β¦
andrebispo5 aefc828
[PM-39851] test: Fix FillAssistRepositoryTests for cache-readable synβ¦
andrebispo5 72516fc
[PM-38443] test: Add error-path coverage for the Fill Assist toggle
andrebispo5 a34f5e5
[PM-39851] test: Fix FillAssistAPIServiceTests for send(_:)-based reqβ¦
andrebispo5 a486c51
[PM-38443] fix: Allow autofill reprompt flow when Fill Assist is enabled
andrebispo5 9f164cb
[PM-38443] refactor: Delegate Fill Assist cache cleanup on logout to β¦
andrebispo5 b2286cc
[PM-38443] test: Disable Fill Assist in the autofill-not-supported teβ¦
andrebispo5 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
174 changes: 174 additions & 0 deletions
174
BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| import BitwardenKit | ||
| import Foundation | ||
|
|
||
| // MARK: - FillAssistRepository | ||
|
|
||
| /// A protocol for a repository that manages fetching and caching fill-assist targeting rules. | ||
| /// | ||
| protocol FillAssistRepository { // sourcery: AutoMockable | ||
| /// Fetches and caches fill-assist rules for the active account. | ||
| /// | ||
| func syncRules() async | ||
|
|
||
| /// Returns the cached fill-assist rules for a given hostname, or `nil` if unavailable. | ||
| /// | ||
| /// - Parameter hostname: The hostname to look up. | ||
| /// - Returns: The cached `FillAssistHostRules`, or `nil`. | ||
| /// | ||
| func rules(for hostname: String) async -> FillAssistHostRules? | ||
|
|
||
| /// Clears all cached fill-assist data for the active account. | ||
| /// | ||
| func clearRules() async throws | ||
| } | ||
|
|
||
| // MARK: - DefaultFillAssistRepository | ||
|
andrebispo5 marked this conversation as resolved.
|
||
|
|
||
| /// The default implementation of `FillAssistRepository`. | ||
| /// | ||
| class DefaultFillAssistRepository: FillAssistRepository { | ||
| // MARK: Private Properties | ||
|
|
||
| /// The store for persisting fill-assist cached data. | ||
| private let appSettingsStore: AppSettingsStore | ||
|
|
||
| /// The service for checking feature flags and configuration. | ||
| private let configService: ConfigService | ||
|
|
||
| /// The service for reporting non-fatal errors. | ||
| private let errorReporter: ErrorReporter | ||
|
|
||
| /// The service for accessing environment URLs. | ||
| private let environmentService: EnvironmentService | ||
|
|
||
| /// The API service for fetching fill-assist data. | ||
| private let fillAssistAPIService: FillAssistAPIService | ||
|
|
||
| /// The service for accessing account state. | ||
| private let stateService: StateService | ||
|
|
||
| /// The provider of the current time, used for interval checks. | ||
| private let timeProvider: TimeProvider | ||
|
|
||
| // MARK: Initialization | ||
|
|
||
| /// Creates a `DefaultFillAssistRepository`. | ||
| /// | ||
| /// - Parameters: | ||
| /// - appSettingsStore: The store for persisting fill-assist cached data. | ||
| /// - configService: The service for checking feature flags and configuration. | ||
| /// - environmentService: The service for accessing environment URLs. | ||
| /// - errorReporter: The service for reporting non-fatal errors. | ||
| /// - fillAssistAPIService: The API service for fetching fill-assist data. | ||
| /// - stateService: The service for accessing account state. | ||
| /// - timeProvider: The provider of the current time. | ||
| /// | ||
| init( | ||
| appSettingsStore: AppSettingsStore, | ||
| configService: ConfigService, | ||
| environmentService: EnvironmentService, | ||
| errorReporter: ErrorReporter, | ||
| fillAssistAPIService: FillAssistAPIService, | ||
| stateService: StateService, | ||
| timeProvider: TimeProvider, | ||
| ) { | ||
| self.appSettingsStore = appSettingsStore | ||
| self.configService = configService | ||
| self.environmentService = environmentService | ||
| self.errorReporter = errorReporter | ||
| self.fillAssistAPIService = fillAssistAPIService | ||
| self.stateService = stateService | ||
| self.timeProvider = timeProvider | ||
| } | ||
|
|
||
| // MARK: FillAssistRepository | ||
|
|
||
| func syncRules() async { | ||
| do { | ||
| try await performSync() | ||
| } catch { | ||
| // Sync failures are non-fatal β existing cache remains available. | ||
| errorReporter.log(error: error) | ||
| } | ||
| } | ||
|
|
||
| func rules(for hostname: String) async -> FillAssistHostRules? { | ||
| guard let userId = try? await stateService.getActiveAccountId() else { return nil } | ||
| return appSettingsStore.fillAssistCachedData(userId: userId)?.rules[hostname] | ||
| } | ||
|
|
||
| func clearRules() async throws { | ||
| let userId = try await stateService.getActiveAccountId() | ||
| appSettingsStore.setFillAssistCachedData(nil, userId: userId) | ||
| } | ||
|
|
||
| // MARK: Private | ||
|
|
||
| /// Runs the full sync pipeline if sync conditions are met | ||
| /// | ||
| private func performSync() async throws { | ||
| // 1. Feature flag guard. | ||
|
andrebispo5 marked this conversation as resolved.
Outdated
|
||
| guard await configService.getFeatureFlag(.fillAssistTargetingRules) else { return } | ||
|
|
||
| // 2. Time-interval guard β skip if last fetch was less than fillAssistUpdateInterval ago. | ||
| let sourceUrl = environmentService.fillAssistRulesURL | ||
| let userId = try await stateService.getActiveAccountId() | ||
| let lastFetch = appSettingsStore.fillAssistLastFetchTimestamp(userId: userId) | ||
| if let lastFetch, timeProvider.presentTime.timeIntervalSince(lastFetch) < Constants.FillAssist.updateInterval { | ||
| return | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π€ I believe we should consider checking |
||
|
|
||
| // 3. Fetch manifest. | ||
| let manifest = try await fillAssistAPIService.getManifest() | ||
|
|
||
| // 4. Resolve the non-deprecated entry for the current forms version. | ||
| guard let entry = manifest.maps["forms"]?[Constants.FillAssist.formsVersion], | ||
| !entry.deprecated | ||
| else { return } | ||
|
|
||
| // 5. If cid and source URL are unchanged, update timestamp and skip download. | ||
| let cached = appSettingsStore.fillAssistCachedData(userId: userId) | ||
| if cached?.cid == entry.cid, cached?.sourceUrl == sourceUrl.absoluteString { | ||
| appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) | ||
| return | ||
| } | ||
|
|
||
| // 6. Download forms file. | ||
| let formsMap = try await fillAssistAPIService.getFormsMap(filename: entry.filename) | ||
|
|
||
| // 7. Validate schema major version; update timestamp and skip if unsupported. | ||
| let schemaMajor = formsMap.schemaVersion.split(separator: ".").first.map(String.init) ?? "" | ||
| guard schemaMajor == Constants.FillAssist.expectedSchemaMajor else { | ||
| appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) | ||
| return | ||
| } | ||
|
|
||
| // 8. Parse, cache, and update timestamp. | ||
| let rules = buildRules(from: formsMap) | ||
| let data = FillAssistCachedData(cid: entry.cid, rules: rules, sourceUrl: sourceUrl.absoluteString) | ||
| appSettingsStore.setFillAssistCachedData(data, userId: userId) | ||
| appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) | ||
| } | ||
|
|
||
| /// Builds a `[hostname: FillAssistHostRules]` dictionary by pooling all non-null field | ||
| /// definitions across each host's top-level forms and every pathname entry. | ||
| /// | ||
| private func buildRules(from formsMap: FormsMapResponseModel) -> [String: FillAssistHostRules] { | ||
| var result = [String: FillAssistHostRules]() | ||
| for (hostname, hostEntry) in formsMap.hosts { | ||
|
fedemkr marked this conversation as resolved.
|
||
| var pooled = [String: [FillAssistFieldAttributes]]() | ||
| let allForms = hostEntry.allForms | ||
| for form in allForms { | ||
| for (fieldKey, selectors) in form.fields { | ||
| let attrs = selectors.flatMap(\.attributes) | ||
| pooled[fieldKey, default: []].append(contentsOf: attrs) | ||
| } | ||
| } | ||
| pooled = pooled.filter { !$0.value.isEmpty } | ||
| if !pooled.isEmpty { | ||
| result[hostname] = FillAssistHostRules(fields: pooled) | ||
| } | ||
|
fedemkr marked this conversation as resolved.
|
||
| } | ||
| return result | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.