Skip to content
Draft
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 @@ -285,4 +285,98 @@ final class UpNextChangesDataManagerTests: DataManagerTestCase {
XCTAssertNotNil(updateActions, "\(impl): Update actions should be accessible")
}
}

// MARK: - Login Sync Bug Tests

/// This test demonstrates the bug where pending replace actions persist through login sync.
///
/// The bug occurs because:
/// 1. During login sync, changes are not collected, so latestActionTime stays at 0
/// 2. clearSyncedData(latestActionTime: 0) calls deleteChangesOlderThan(utcTime: 0)
/// 3. Since all pending changes have positive timestamps (milliseconds since 1970),
/// the query "DELETE WHERE utcTime <= 0" deletes nothing
/// 4. Old replace actions persist and get sent on the next non-login sync
func testDeleteChangesOlderThanZeroDoesNotDeleteReplaceAction_DemonstratesBug() throws {
try runWithBothImplementations { dataManager, impl in
let podcast = self.createTestPodcast(dataManager: dataManager)
_ = self.createTestEpisode(uuid: "episode-1", podcast: podcast, dataManager: dataManager)

// Simulate: Device creates an empty replace action (e.g., when queue is cleared)
dataManager.saveReplace(episodeList: [])

// Verify replace action exists
let replaceActionBefore = dataManager.findReplaceAction()
XCTAssertNotNil(replaceActionBefore, "\(impl): Replace action should exist before deletion attempt")
XCTAssertEqual(replaceActionBefore?.uuids, "", "\(impl): Replace action should have empty episode list")

// Simulate login sync: clearSyncedData is called with latestActionTime = 0
// This is the bug - it should clear pending changes but doesn't
dataManager.deleteChangesOlderThan(utcTime: 0)

// BUG: Replace action still exists because its utcTime > 0
let replaceActionAfter = dataManager.findReplaceAction()
XCTAssertNotNil(replaceActionAfter, "\(impl): BUG DEMONSTRATED - Replace action persists after deleteChangesOlderThan(0)")
}
}

/// This test demonstrates the fix: using Int64.max to clear all pending changes.
func testDeleteChangesOlderThanMaxDeletesAllChanges_DemonstratesFix() throws {
try runWithBothImplementations { dataManager, impl in
let podcast = self.createTestPodcast(dataManager: dataManager)
_ = self.createTestEpisode(uuid: "episode-1", podcast: podcast, dataManager: dataManager)
let episode2 = self.createTestEpisode(uuid: "episode-2", podcast: podcast, dataManager: dataManager)

// Create both a replace action and update actions
dataManager.saveReplace(episodeList: [])
dataManager.saveUpNextAddToTop(episodeUuid: episode2.uuid)

// Verify actions exist
XCTAssertNotNil(dataManager.findReplaceAction(), "\(impl): Replace action should exist")

// FIX: Use Int64.max to clear all pending changes during login sync
dataManager.deleteChangesOlderThan(utcTime: Int64.max)

// All changes should be cleared
let replaceAction = dataManager.findReplaceAction()
let updateActions = dataManager.findUpdateActions()

XCTAssertNil(replaceAction, "\(impl): FIX DEMONSTRATED - Replace action cleared with Int64.max")
XCTAssertTrue(updateActions.isEmpty, "\(impl): FIX DEMONSTRATED - Update actions cleared with Int64.max")
}
}

/// This test simulates the full bug scenario:
/// 1. Empty replace action is created
/// 2. Login sync happens (simulated by deleteChangesOlderThan(0))
/// 3. Replace action persists
/// 4. On next sync, the stale replace would be found and sent
func testStaleReplaceActionPersistsThroughLoginSync() throws {
try runWithBothImplementations { dataManager, impl in
let podcast = self.createTestPodcast(dataManager: dataManager)
_ = self.createTestEpisode(uuid: "episode-1", podcast: podcast, dataManager: dataManager)

// Step 1: Device has empty queue, creates empty replace action
dataManager.saveReplace(episodeList: [])

let originalReplace = dataManager.findReplaceAction()
XCTAssertNotNil(originalReplace, "\(impl): Original replace should exist")
let originalTimestamp = originalReplace!.utcTime
XCTAssertGreaterThan(originalTimestamp, 0, "\(impl): Replace should have positive timestamp")

// Step 2: Simulate login sync - this should clear stale changes but doesn't
// In real code: latestActionTime = 0 because changes aren't collected during login
dataManager.deleteChangesOlderThan(utcTime: 0)

// Step 3: Verify stale replace still exists (the bug)
let staleReplace = dataManager.findReplaceAction()
XCTAssertNotNil(staleReplace, "\(impl): Stale replace persists through login sync")
XCTAssertEqual(staleReplace?.utcTime, originalTimestamp, "\(impl): Same stale replace action")

// Step 4: On next non-login sync, findReplaceAction() would return this stale action
// and it would be sent to the server, potentially clearing the server's queue
let replaceToSync = dataManager.findReplaceAction()
XCTAssertNotNil(replaceToSync, "\(impl): Stale replace would be synced on next sync")
XCTAssertEqual(replaceToSync?.uuids, "", "\(impl): Empty replace would clear server queue")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,13 @@ class UpNextSyncTask: ApiBaseTask {
// save the server last modified so we can send it back next time. For legacy compatibility this is stored as a string
UserDefaults.standard.set("\(response.serverModified)", forKey: ServerConstants.UserDefaults.upNextServerLastModified)

clearSyncedData(latestActionTime: latestActionTime)
// For login syncs, clear all pending changes since we've applied server state
// and any pending changes are stale.
if SyncManager.syncReason == .login && FeatureFlag.clearPendingUpNextChangesOnLogin.enabled {
clearSyncedData(latestActionTime: Int64.max)
} else {
clearSyncedData(latestActionTime: latestActionTime)
}
} catch {
FileLog.shared.addMessage("UpNextSyncTask: Failed to decode server data")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
@testable import PocketCastsServer
@testable import PocketCastsDataModel
@testable import PocketCastsUtils
import GRDB
import XCTest

/// Tests for UpNextSyncTask, specifically the login sync bug fix.
///
/// Bug: During login sync, pending Up Next changes (especially replace actions) persist
/// because `clearSyncedData(latestActionTime: 0)` doesn't delete any changes with positive timestamps.
///
/// Fix: When `clearPendingUpNextChangesOnLogin` feature flag is enabled, use `Int64.max`
/// instead of `latestActionTime` during login sync to clear all stale pending changes.
final class UpNextSyncTaskTests: XCTestCase {
private var originalDataManager: DataManager!
private var dataManager: DataManager!
private var originalSyncReason: SyncManager.SyncingReason?
private let featureFlagMock = FeatureFlagMock()

override func setUp() {
super.setUp()
originalDataManager = DataManager.sharedManager
dataManager = DataManager(dbQueue: GRDBQueue(dbPool: try! DatabasePool(path: NSTemporaryDirectory().appending("\(UUID().uuidString).sqlite"))))
DataManager.sharedManager = dataManager
originalSyncReason = SyncManager.syncReason
}

override func tearDown() {
featureFlagMock.reset()
SyncManager.syncReason = originalSyncReason
DataManager.sharedManager = originalDataManager
dataManager = nil
originalDataManager = nil
super.tearDown()
}

// MARK: - Login Sync Bug Regression Tests

/// Test that verifies the bug: without the fix, stale replace actions persist through login sync.
///
/// This test FAILS when the fix is NOT applied (feature flag enabled but fix code missing).
/// This test PASSES when the fix IS applied (feature flag enabled and fix code present).
func testLoginSyncClearsPendingReplaceAction_WithFixEnabled() throws {
// Enable the fix
featureFlagMock.set(.clearPendingUpNextChangesOnLogin, value: true)

// Create a stale empty replace action (simulates device clearing queue while offline)
dataManager.saveReplace(episodeList: [])

// Verify replace action exists
XCTAssertNotNil(dataManager.findReplaceAction(), "Replace action should exist before sync")

// Set sync reason to login
SyncManager.syncReason = .login

// Create mock server response with some episodes
let serverResponse = createMockUpNextResponse(episodeUUIDs: ["server-episode-1", "server-episode-2"])

// Process the server response (this calls clearSyncedData internally)
let syncTask = UpNextSyncTask()
syncTask.process(serverData: serverResponse, latestActionTime: 0)

// With the fix enabled, the stale replace action should be cleared
let replaceActionAfterSync = dataManager.findReplaceAction()
XCTAssertNil(replaceActionAfterSync, "Stale replace action should be cleared during login sync when fix is enabled")
}

/// Test that verifies the bug exists when fix is disabled.
///
/// This test documents the buggy behavior - the replace action persists.
func testLoginSyncDoesNotClearPendingReplaceAction_WithFixDisabled() throws {
// Disable the fix (default behavior, demonstrates the bug)
featureFlagMock.set(.clearPendingUpNextChangesOnLogin, value: false)

// Create a stale empty replace action
dataManager.saveReplace(episodeList: [])

// Verify replace action exists
let replaceActionBefore = dataManager.findReplaceAction()
XCTAssertNotNil(replaceActionBefore, "Replace action should exist before sync")

// Set sync reason to login
SyncManager.syncReason = .login

// Create mock server response
let serverResponse = createMockUpNextResponse(episodeUUIDs: ["server-episode-1"])

// Process the server response
let syncTask = UpNextSyncTask()
syncTask.process(serverData: serverResponse, latestActionTime: 0)

// BUG: Without the fix, the stale replace action persists
let replaceActionAfterSync = dataManager.findReplaceAction()
XCTAssertNotNil(replaceActionAfterSync, "BUG: Replace action persists through login sync when fix is disabled")
}

/// Test that non-login syncs still work correctly (changes are cleared based on latestActionTime).
func testNonLoginSyncClearsChangesBasedOnLatestActionTime() throws {
// This test verifies we didn't break normal sync behavior

// Create a replace action
dataManager.saveReplace(episodeList: ["episode-1"])

let replaceAction = dataManager.findReplaceAction()
XCTAssertNotNil(replaceAction, "Replace action should exist")
let actionTime = replaceAction!.utcTime

// Set sync reason to something other than login
SyncManager.syncReason = .replace

// Create mock server response
let serverResponse = createMockUpNextResponse(episodeUUIDs: ["episode-1"])

// Process with latestActionTime matching the replace action's time
let syncTask = UpNextSyncTask()
syncTask.process(serverData: serverResponse, latestActionTime: actionTime)

// The replace action should be cleared because latestActionTime >= its utcTime
let replaceActionAfterSync = dataManager.findReplaceAction()
XCTAssertNil(replaceActionAfterSync, "Replace action should be cleared during normal sync")
}

// MARK: - Helpers

private func createMockUpNextResponse(episodeUUIDs: [String]) -> Data {
var response = Api_UpNextResponse()
response.serverModified = Int64(Date().timeIntervalSince1970 * 1000)

for uuid in episodeUUIDs {
var episode = Api_UpNextResponse.EpisodeResponse()
episode.uuid = uuid
episode.title = "Test Episode \(uuid)"
episode.podcast = "test-podcast"
episode.url = "https://example.com/\(uuid).mp3"
response.episodes.append(episode)
}

return try! response.serializedData()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ public enum FeatureFlag: String, CaseIterable {
/// Use WCSessionFileTransfer to send logs from watchOS to iPhone instead of sendMessage reply
case watchLogFileTransfer

/// Clear all pending Up Next changes on login sync to prevent stale replace actions from being sent
case clearPendingUpNextChangesOnLogin

public var enabled: Bool {
if let overriddenValue = FeatureFlagOverrideStore().overriddenValue(for: self) {
return overriddenValue
Expand Down Expand Up @@ -480,6 +483,8 @@ public enum FeatureFlag: String, CaseIterable {
true
case .watchLogFileTransfer:
true
case .clearPendingUpNextChangesOnLogin:
true
}
}

Expand Down