Skip to content

Commit

Permalink
Fixed some items not being synced due to errors (#760)
Browse files Browse the repository at this point in the history
  • Loading branch information
@michalrentka
michalrentka authored Aug 31, 2023
1 parent a712ec5 commit d2b6c5e
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 11 deletions.
30 changes: 20 additions & 10 deletions Zotero/Controllers/Sync/SyncController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1144,13 +1144,14 @@ final class SyncController: SynchronizationController {
completion: { [weak self] result in
self?.accessQueue.async(flags: .barrier) {
self?.batchProcessor = nil
self?.finishBatchesSyncAction(for: libraryId, object: object, result: result)
let keys: [String] = batches.reduce([]) { $0 + $1.keys }
self?.finishBatchesSyncAction(for: libraryId, object: object, result: result, keys: keys)
}
})
self.batchProcessor?.start()
}

private func finishBatchesSyncAction(for libraryId: LibraryIdentifier, object: SyncObject, result: Swift.Result<SyncBatchResponse, Error>) {
private func finishBatchesSyncAction(for libraryId: LibraryIdentifier, object: SyncObject, result: Swift.Result<SyncBatchResponse, Error>, keys: [String]) {
switch result {
case .success((let failedKeys, let parseErrors, _))://let itemConflicts)):
let nonFatalErrors = parseErrors.map({
Expand Down Expand Up @@ -1194,7 +1195,10 @@ final class SyncController: SynchronizationController {
self.abort(error: error)

case .nonFatal(let error):
self.handleNonFatal(error: error, libraryId: libraryId, version: nil)
self.handleNonFatal(error: error, libraryId: libraryId, version: nil) { [weak self] in
self?.markForResync(keys: keys, libraryId: libraryId, object: object)
return false
}
}
}
}
Expand Down Expand Up @@ -1647,6 +1651,7 @@ final class SyncController: SynchronizationController {
if failedBeforeReachingApi {
self.handleAllUploadsFailedBeforeReachingZoteroBackend(in: libraryId)
}
return true
})
}
}
Expand Down Expand Up @@ -1704,6 +1709,7 @@ final class SyncController: SynchronizationController {
if failedBeforeReachingApi {
self.handleAllUploadsFailedBeforeReachingZoteroBackend(in: libraryId)
}
return true
})
}

Expand Down Expand Up @@ -2103,10 +2109,10 @@ final class SyncController: SynchronizationController {
/// - parameter libraryId: Library identifier.
/// - parameter version: Optional version number received from backend.
/// - parameter additionalAction: Optional action taken before next action in queue is processed.
private func handleNonFatal(error: SyncError.NonFatal, libraryId: LibraryIdentifier, version: Int?, additionalAction: (() -> Void)? = nil) {
private func handleNonFatal(error: SyncError.NonFatal, libraryId: LibraryIdentifier, version: Int?, additionalAction: (() -> Bool)? = nil) {
let appendAndContinue: () -> Void = {
self.nonFatalErrors.append(error)
additionalAction?()
guard additionalAction?() ?? true else { return }
self.processNextAction()
}

Expand All @@ -2121,7 +2127,7 @@ final class SyncController: SynchronizationController {
if let version = version {
self.handleUnchangedFailure(lastVersion: version, libraryId: libraryId, additionalAction: additionalAction)
} else {
additionalAction?()
guard additionalAction?() ?? true else { return }
self.processNextAction()
}

Expand All @@ -2137,7 +2143,7 @@ final class SyncController: SynchronizationController {
/// - parameter libraryId: Library identifier which reached quota limit.
/// - parameter error: NonFatal error that caused this
/// - additiionalAction: Additional actions that should be applied to queue
private func removeRemainingUploads(for libraryId: LibraryIdentifier, andAppendError error: SyncError.NonFatal, additionalAction: (() -> Void)?) {
private func removeRemainingUploads(for libraryId: LibraryIdentifier, andAppendError error: SyncError.NonFatal, additionalAction: (() -> Bool)?) {
DDLogInfo("Sync: received quota limit for \(libraryId)")

self.queue.removeAll(where: { action in
Expand All @@ -2154,7 +2160,7 @@ final class SyncController: SynchronizationController {
self.nonFatalErrors.append(error)
}

additionalAction?()
guard additionalAction?() ?? true else { return }

self.processNextAction()
}
Expand All @@ -2166,13 +2172,16 @@ final class SyncController: SynchronizationController {
/// - parameter version: Current version number which returned this error.
/// - parameter libraryId: Identifier of current library, which is being synced.
/// - returns: `true` if there was a unchanged error, `false` otherwise.
private func handleUnchangedFailure(lastVersion: Int, libraryId: LibraryIdentifier, additionalAction: (() -> Void)?) {
private func handleUnchangedFailure(lastVersion: Int, libraryId: LibraryIdentifier, additionalAction: (() -> Bool)?) {
DDLogInfo("Sync: received unchanged error, store version: \(lastVersion)")
self.lastReturnedVersion = lastVersion

// If current sync type is `.full` we don't want to skip anything.
guard self.type != .full else {
self.processNextAction()
let shouldProcessNext = additionalAction?() ?? true
if shouldProcessNext {
self.processNextAction()
}
return
}

Expand All @@ -2199,6 +2208,7 @@ final class SyncController: SynchronizationController {

toDelete.reversed().forEach { self.queue.remove(at: $0) }

guard additionalAction?() ?? true else { return }
self.processNextAction()
}

Expand Down
74 changes: 73 additions & 1 deletion ZoteroTests/SyncControllerSpec.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,79 @@ final class SyncControllerSpec: QuickSpec {
syncController.start(type: .normal, libraries: .all, retryAttempt: 0)
}
}


it("should mark object as outdated if API request returned 500 (or other non 400 response code and syncRetries should be increased)") {
let header = ["last-modified-version": "3"]
let libraryId = userLibraryId
let objects = SyncObject.allCases
let itemKey = "AAAAAAAA"

try! realm.write {
let item = RItem()
item.key = itemKey
item.syncState = .synced
item.syncRetries = 0
item.version = 2
item.libraryId = .custom(.myLibrary)
realm.add(item)
}

createStub(for: GroupVersionsRequest(userId: userId), baseUrl: baseUrl, headers: header, jsonResponse: [:] as [String: Any])
objects.forEach { object in
if object == .item {
createStub(
for: VersionsRequest(libraryId: libraryId, userId: userId, objectType: object, version: 0),
baseUrl: baseUrl,
headers: header,
jsonResponse: [itemKey: 3]
)
} else {
createStub(
for: VersionsRequest(libraryId: libraryId, userId: userId, objectType: object, version: 0),
baseUrl: baseUrl,
headers: header,
jsonResponse: [:] as [String: Any]
)
}
}
createStub(for: KeyRequest(), baseUrl: baseUrl, url: Bundle(for: Self.self).url(forResource: "test_keys", withExtension: "json")!)
createStub(
for: ObjectsRequest(libraryId: libraryId, userId: userId, objectType: .item, keys: "\(itemKey)"),
baseUrl: baseUrl,
headers: header,
statusCode: 500,
jsonResponse: [:] as [String: Any]
)
createStub(
for: SettingsRequest(libraryId: libraryId, userId: userId, version: 0),
baseUrl: baseUrl,
headers: header,
jsonResponse: ["tagColors": ["value": [] as [Any], "version": 2] as [String: Any]]
)
createStub(
for: DeletionsRequest(libraryId: libraryId, userId: userId, version: 0),
baseUrl: baseUrl,
headers: header,
jsonResponse: ["collections": [] as [Any], "searches": [] as [Any], "items": [] as [Any], "tags": [] as [Any]]
)

waitUntil(timeout: .seconds(10)) { doneAction in
syncController.reportFinish = { _ in
let realm = try! Realm(configuration: realmConfig)
realm.refresh()

let item = realm.objects(RItem.self).filter(.key(itemKey, in: .custom(.myLibrary))).first
expect(item).toNot(beNil())
expect(item?.syncState).to(equal(.outdated))
expect(item?.syncRetries).to(equal(1))

doneAction()
}

syncController.start(type: .normal, libraries: .all, retryAttempt: 0)
}
}

it("should mark object as needsSync if not parsed correctly and syncRetries should be increased") {
let header = ["last-modified-version": "3"]
let libraryId = userLibraryId
Expand Down

0 comments on commit d2b6c5e

Please sign in to comment.