-
Notifications
You must be signed in to change notification settings - Fork 121
[Local Catalog] Persist catalog downloads in the background #16342
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
Changes from 1 commit
fabb1de
a93899e
be0cf24
5046bfd
f350297
db3f9d4
5d29062
06ae494
bc5c1bf
fa75864
d0558a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| import Foundation | ||
| import Testing | ||
| @testable import Networking | ||
|
|
||
| struct BackgroundCatalogDownloadCoordinatorTests { | ||
|
|
||
| @Test func handleBackgroundSessionEvent_loads_saved_state() async { | ||
| // Given | ||
| let sessionIdentifier = "com.woocommerce.pos.catalog.download.123" | ||
| let siteID: Int64 = 456 | ||
| let state = BackgroundDownloadState( | ||
| sessionIdentifier: sessionIdentifier, | ||
| siteID: siteID, | ||
| startedAt: Date() | ||
| ) | ||
| BackgroundDownloadState.save(state) | ||
|
|
||
| let mockDownloader = MockBackgroundDownloader() | ||
| mockDownloader.mockFileURL = URL(fileURLWithPath: "/tmp/test.json") | ||
| let coordinator = BackgroundCatalogDownloadCoordinator(backgroundDownloader: mockDownloader) | ||
|
|
||
| var parsedSiteID: Int64? | ||
| var parsedFileURL: URL? | ||
|
|
||
| // When | ||
| await coordinator.handleBackgroundSessionEvent( | ||
| sessionIdentifier: sessionIdentifier, | ||
| completionHandler: { }, | ||
| parseHandler: { fileURL, siteID in | ||
| parsedFileURL = fileURL | ||
| parsedSiteID = siteID | ||
| } | ||
| ) | ||
|
|
||
| // Then | ||
| #expect(parsedSiteID == siteID) | ||
| #expect(parsedFileURL?.path == "/tmp/test.json") | ||
|
|
||
| // Cleanup | ||
| BackgroundDownloadState.clear() | ||
| } | ||
|
|
||
| @Test func handleBackgroundSessionEvent_calls_completion_handler_when_no_state() async { | ||
| // Given | ||
| BackgroundDownloadState.clear() // Ensure no saved state | ||
| let sessionIdentifier = "com.woocommerce.pos.catalog.download.999" | ||
| let mockDownloader = MockBackgroundDownloader() | ||
| let coordinator = BackgroundCatalogDownloadCoordinator(backgroundDownloader: mockDownloader) | ||
|
|
||
| var completionCalled = false | ||
| var parseCalled = false | ||
|
|
||
| // When | ||
| await coordinator.handleBackgroundSessionEvent( | ||
| sessionIdentifier: sessionIdentifier, | ||
| completionHandler: { | ||
| completionCalled = true | ||
| }, | ||
| parseHandler: { _, _ in | ||
| parseCalled = true | ||
| } | ||
| ) | ||
|
|
||
| // Then | ||
| #expect(completionCalled == true) | ||
| #expect(parseCalled == false) // Should not parse without state | ||
| } | ||
|
|
||
| @Test func handleBackgroundSessionEvent_clears_state_after_processing() async { | ||
| // Given | ||
| let sessionIdentifier = "com.woocommerce.pos.catalog.download.789" | ||
| let state = BackgroundDownloadState( | ||
| sessionIdentifier: sessionIdentifier, | ||
| siteID: 111, | ||
| startedAt: Date() | ||
| ) | ||
| BackgroundDownloadState.save(state) | ||
|
|
||
| let mockDownloader = MockBackgroundDownloader() | ||
| mockDownloader.mockFileURL = URL(fileURLWithPath: "/tmp/test.json") | ||
| let coordinator = BackgroundCatalogDownloadCoordinator(backgroundDownloader: mockDownloader) | ||
|
|
||
| // When | ||
| await coordinator.handleBackgroundSessionEvent( | ||
| sessionIdentifier: sessionIdentifier, | ||
| completionHandler: { }, | ||
| parseHandler: { _, _ in } | ||
| ) | ||
|
|
||
| // Then - state should be cleared | ||
| let loadedState = BackgroundDownloadState.load(for: sessionIdentifier) | ||
| #expect(loadedState == nil) | ||
| } | ||
|
|
||
| @Test func handleBackgroundSessionEvent_reconnects_to_session() async { | ||
| // Given | ||
| let sessionIdentifier = "com.woocommerce.pos.catalog.download.reconnect" | ||
| let state = BackgroundDownloadState( | ||
| sessionIdentifier: sessionIdentifier, | ||
| siteID: 222, | ||
| startedAt: Date() | ||
| ) | ||
| BackgroundDownloadState.save(state) | ||
|
|
||
| let mockDownloader = MockBackgroundDownloader() | ||
| mockDownloader.mockFileURL = URL(fileURLWithPath: "/tmp/catalog.json") | ||
| let coordinator = BackgroundCatalogDownloadCoordinator(backgroundDownloader: mockDownloader) | ||
|
|
||
| // When | ||
| await coordinator.handleBackgroundSessionEvent( | ||
| sessionIdentifier: sessionIdentifier, | ||
| completionHandler: { }, | ||
| parseHandler: { _, _ in } | ||
| ) | ||
|
|
||
| // Then | ||
| #expect(mockDownloader.reconnectSessionCallCount == 1) | ||
| #expect(mockDownloader.lastReconnectSessionIdentifier == sessionIdentifier) | ||
|
|
||
| // Cleanup | ||
| BackgroundDownloadState.clear() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| import Foundation | ||
| import Testing | ||
| @testable import Networking | ||
|
|
||
| struct BackgroundDownloadStateTests { | ||
|
|
||
| @Test func save_persists_state_to_userdefaults() { | ||
| // Given | ||
| let state = BackgroundDownloadState( | ||
| sessionIdentifier: "test.session.123", | ||
| siteID: 456, | ||
| startedAt: Date() | ||
| ) | ||
|
|
||
| // When | ||
| BackgroundDownloadState.save(state) | ||
|
|
||
| // Then | ||
| let loaded = BackgroundDownloadState.load(for: "test.session.123") | ||
| #expect(loaded?.sessionIdentifier == "test.session.123") | ||
| #expect(loaded?.siteID == 456) | ||
| #expect(loaded?.startedAt != nil) | ||
|
|
||
| // Cleanup | ||
| BackgroundDownloadState.clear() | ||
| } | ||
|
|
||
| @Test func load_returns_nil_for_nonexistent_session() { | ||
| // Given | ||
| BackgroundDownloadState.clear() | ||
|
|
||
| // When | ||
| let loaded = BackgroundDownloadState.load(for: "nonexistent.session") | ||
|
|
||
| // Then | ||
| #expect(loaded == nil) | ||
| } | ||
|
|
||
| @Test func load_returns_nil_for_different_session_identifier() { | ||
| // Given | ||
| let state = BackgroundDownloadState( | ||
| sessionIdentifier: "session.A", | ||
| siteID: 123, | ||
| startedAt: Date() | ||
| ) | ||
| BackgroundDownloadState.save(state) | ||
|
|
||
| // When | ||
| let loaded = BackgroundDownloadState.load(for: "session.B") | ||
|
|
||
| // Then | ||
| #expect(loaded == nil) | ||
|
|
||
| // Cleanup | ||
| BackgroundDownloadState.clear() | ||
| } | ||
|
|
||
| @Test func clear_removes_saved_state() { | ||
| // Given | ||
| let state = BackgroundDownloadState( | ||
| sessionIdentifier: "test.session", | ||
| siteID: 789, | ||
| startedAt: Date() | ||
| ) | ||
| BackgroundDownloadState.save(state) | ||
|
|
||
| // When | ||
| BackgroundDownloadState.clear() | ||
|
|
||
| // Then | ||
| let loaded = BackgroundDownloadState.load(for: "test.session") | ||
| #expect(loaded == nil) | ||
| } | ||
|
|
||
| @Test func save_overwrites_previous_state() { | ||
| // Given | ||
| let firstState = BackgroundDownloadState( | ||
| sessionIdentifier: "session.1", | ||
| siteID: 100, | ||
| startedAt: Date() | ||
| ) | ||
| BackgroundDownloadState.save(firstState) | ||
|
|
||
| let secondState = BackgroundDownloadState( | ||
| sessionIdentifier: "session.2", | ||
| siteID: 200, | ||
| startedAt: Date() | ||
| ) | ||
|
|
||
| // When | ||
| BackgroundDownloadState.save(secondState) | ||
|
|
||
| // Then | ||
| let loadedFirst = BackgroundDownloadState.load(for: "session.1") | ||
| let loadedSecond = BackgroundDownloadState.load(for: "session.2") | ||
|
|
||
| #expect(loadedFirst == nil) // First session is overwritten | ||
| #expect(loadedSecond?.sessionIdentifier == "session.2") | ||
| #expect(loadedSecond?.siteID == 200) | ||
|
|
||
| // Cleanup | ||
| BackgroundDownloadState.clear() | ||
| } | ||
| } |
|
Contributor
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. Not from this PR, but consider making a helper function in this test suite for clean up the temp files and background state, I think it's easy to miss the difference between |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAU this clean up is not needed, as we already perform it at the end of
handleBackgroundSessionEventright?Not necessarily a suggestion, just something to think about: I wonder if there could be test isolation issues when these run in parallel, or a test crashes/fails before cleanup which could affect subsequent tests since these share
UserDefaults.standard