Skip to content
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

Don't cancel sync on webdav request failure #776

Merged
merged 2 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions Zotero/Assets/en.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@
"errors.sync_toolbar.show_items" = "Show Items";
"errors.sync_toolbar.forbidden" = "Invalid username or password";
"errors.sync_toolbar.forbidden_message" = "The Zotero sync server did not accept your username and password.\n\nPlease log out and log in with correct login information.";
"errors.sync_toolbar.webdav_request_failed" = "Your WebDAV server returned an HTTP %d error for a %@ request.";
"errors.items.loading" = "Could not load items.";
"errors.items.deletion" = "Could not remove item.";
"errors.items.deletion_from_collection" = "Could not remove item from collection.";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,16 @@ struct AuthorizeUploadSyncAction: SyncAction {

var result: Single<AuthorizeUploadResponse> {
DDLogInfo("AuthorizeUploadSyncAction: authorize upload")
let request = AuthorizeUploadRequest(libraryId: self.libraryId, userId: self.userId, key: self.key, filename: self.filename,
filesize: self.filesize, md5: self.md5, mtime: self.mtime, oldMd5: self.oldMd5)
let request = AuthorizeUploadRequest(
libraryId: self.libraryId,
userId: self.userId,
key: self.key,
filename: self.filename,
filesize: self.filesize,
md5: self.md5,
mtime: self.mtime,
oldMd5: self.oldMd5
)
return self.apiClient.send(request: request, queue: self.queue)
.mapData(httpMethod: request.httpMethod.rawValue)
.observe(on: self.scheduler)
Expand Down
18 changes: 17 additions & 1 deletion Zotero/Controllers/Sync/SyncController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,19 @@ final class SyncController: SynchronizationController {
retryLibraries.append(libraryId)
}

case .unknown, .schema, .parsing, .apiError, .unchanged, .quotaLimit, .attachmentMissing, .insufficientSpace, .webDavDeletion, .webDavDeletionFailed, .webDavVerification, .webDavDownload:
case .unknown,
.schema,
.parsing,
.apiError,
.unchanged,
.quotaLimit,
.attachmentMissing,
.insufficientSpace,
.webDavDeletion,
.webDavDeletionFailed,
.webDavVerification,
.webDavDownload,
.webDavUpload:
reportErrors.append(error)
}
}
Expand Down Expand Up @@ -1969,6 +1981,10 @@ final class SyncController: SynchronizationController {
return .nonFatal(.webDavDownload(error))
}

if let error = error as? WebDavError.Upload {
return .nonFatal(.webDavUpload(error))
}

if let error = error as? ZoteroApiError {
switch error {
case .unchanged: return .nonFatal(.unchanged)
Expand Down
90 changes: 59 additions & 31 deletions Zotero/Controllers/WebDAV/WebDavController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ enum WebDavError {

enum Upload: Swift.Error {
case cantCreatePropData
case apiError(AFError)
}
}

Expand Down Expand Up @@ -151,31 +152,49 @@ final class WebDavControllerImpl: WebDavController {
func prepareForUpload(key: String, mtime: Int, hash: String, file: File, queue: DispatchQueue) -> Single<WebDavUploadResult> {
DDLogInfo("WebDavController: prepare for upload")
return self.checkServerIfNeeded(queue: queue)
.flatMap({ url -> Single<MetadataResult> in
return self.checkMetadata(key: key, mtime: mtime, hash: hash, url: url, queue: queue)
})
.flatMap({ result -> Single<WebDavUploadResult> in
switch result {
case .unchanged:
return Single.just(.exists)

case .new(let url):
return self.zip(file: file, key: key).flatMap({ return Single.just(.new(url, $0)) })

case .mtimeChanged(let mtime):
return self.update(mtime: mtime, key: key, queue: queue).flatMap({ Single.just(.exists) })

case .changed(let url):
// If metadata were available on WebDAV, but they changed, remove original .prop file.
return self.removeExistingMetadata(key: key, url: url, queue: queue)
.flatMap({ self.zip(file: file, key: key) })
.flatMap({ Single.just(.new(url, $0)) })
}
})
.flatMap({ url -> Single<MetadataResult> in
return self.checkMetadata(key: key, mtime: mtime, hash: hash, url: url, queue: queue)
})
.flatMap({ result -> Single<WebDavUploadResult> in
switch result {
case .unchanged:
return Single.just(.exists)

case .new(let url):
return self.zip(file: file, key: key).flatMap({ return Single.just(.new(url, $0)) })

case .mtimeChanged(let mtime):
return self.update(mtime: mtime, key: key, queue: queue).flatMap({ Single.just(.exists) })

case .changed(let url):
// If metadata were available on WebDAV, but they changed, remove original .prop file.
return self.removeExistingMetadata(key: key, url: url, queue: queue)
.flatMap({ self.zip(file: file, key: key) })
.flatMap({ Single.just(.new(url, $0)) })
}
})
.catch { error in
if let responseError = error as? AFResponseError {
throw WebDavError.Upload.apiError(responseError.error)
}
if let alamoError = error as? AFError {
throw WebDavError.Upload.apiError(alamoError)
}
throw error
}
}

func upload(request: AttachmentUploadRequest, fromFile file: File, queue: DispatchQueue) -> Single<(Data?, HTTPURLResponse)> {
return self.apiClient.upload(request: request, fromFile: file, queue: queue)
.catch { error in
if let responseError = error as? AFResponseError {
throw WebDavError.Upload.apiError(responseError.error)
}
if let alamoError = error as? AFError {
throw WebDavError.Upload.apiError(alamoError)
}
throw error
}
}

/// Finishes upload to WebDAV. If successful, uploads new metadata .prop file to WebDAV. In both cases removes temporary ZIP file created in `prepareForUpload`.
Expand Down Expand Up @@ -315,29 +334,38 @@ final class WebDavControllerImpl: WebDavController {
private func removeExistingMetadata(key: String, url: URL, queue: DispatchQueue) -> Single<()> {
DDLogInfo("WebDavController: remove metadata for \(key)")
return self.apiClient.send(request: WebDavDeleteRequest(url: url.appendingPathComponent(key + ".prop")), queue: queue)
.flatMap({ _ in return Single.just(()) })
.flatMap({ _ in return Single.just(()) })
}

private func uploadMetadata(key: String, mtime: Int, hash: String, url: URL, queue: DispatchQueue) -> Single<()> {
DDLogInfo("WebDavController: upload metadata for \(key)")
let metadataProp = "<properties version=\"1\"><mtime>\(mtime)</mtime><hash>\(hash)</hash></properties>"
guard let data = metadataProp.data(using: .utf8) else { return Single.error(WebDavError.Upload.cantCreatePropData) }
return self.apiClient.send(request: WebDavWriteRequest(url: url.appendingPathComponent(key + ".prop"), data: data), queue: queue)
.flatMap({ _ in return Single.just(()) })
.flatMap({ _ in return Single.just(()) })
.catch { error in
if let responseError = error as? AFResponseError {
throw WebDavError.Upload.apiError(responseError.error)
}
if let alamoError = error as? AFError {
throw WebDavError.Upload.apiError(alamoError)
}
throw error
}
}

private func checkMetadata(key: String, mtime: Int, hash: String, url: URL, queue: DispatchQueue) -> Single<MetadataResult> {
DDLogInfo("WebDavController: check metadata for \(key)")
return self.metadata(key: key, url: url, queue: queue)
.flatMap({ remoteData -> Single<MetadataResult> in
guard let (remoteMtime, remoteHash) = remoteData else { return Single.just(.new(url)) }
.flatMap({ remoteData -> Single<MetadataResult> in
guard let (remoteMtime, remoteHash) = remoteData else { return Single.just(.new(url)) }

if hash == remoteHash {
return Single.just(mtime == remoteMtime ? .unchanged : .mtimeChanged(remoteMtime))
} else {
return Single.just(.changed(url))
}
})
if hash == remoteHash {
return Single.just(mtime == remoteMtime ? .unchanged : .mtimeChanged(remoteMtime))
} else {
return Single.just(.changed(url))
}
})
}

/// Loads metadata of item from WebDAV server.
Expand Down
4 changes: 4 additions & 0 deletions Zotero/Extensions/Localizable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,10 @@ internal enum L10n {
internal static func webdavItemProp(_ p1: Any) -> String {
return L10n.tr("Localizable", "errors.sync_toolbar.webdav_item_prop", String(describing: p1), fallback: "Invalid prop file: %@")
}
/// Your WebDAV server returned an HTTP %d error for a %@ request.
internal static func webdavRequestFailed(_ p1: Int, _ p2: Any) -> String {
return L10n.tr("Localizable", "errors.sync_toolbar.webdav_request_failed", p1, String(describing: p2), fallback: "Your WebDAV server returned an HTTP %d error for a %@ request.")
}
}
internal enum Translators {
/// Could not update translators from bundle. Would you like to try again?
Expand Down
1 change: 1 addition & 0 deletions Zotero/Models/Sync/SyncErrors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ enum SyncError {
case webDavDeletionFailed(error: String, library: String)
case webDavVerification(WebDavError.Verification)
case webDavDownload(WebDavError.Download)
case webDavUpload(WebDavError.Upload)
case preconditionFailed(LibraryIdentifier)

var isVersionMismatch: Bool {
Expand Down
9 changes: 9 additions & 0 deletions Zotero/Scenes/General/Views/SyncToolbarController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,15 @@ final class SyncToolbarController {
case .notChanged: break // Should not happen
}

case .webDavUpload(let error):
switch error {
case .cantCreatePropData: break // Should not happen

case .apiError(let error):
guard let statusCode = error.unacceptableStatusCode else { break }
return (L10n.Errors.SyncToolbar.webdavRequestFailed(statusCode, error.url?.absoluteString ?? ""), nil)
michalrentka marked this conversation as resolved.
Show resolved Hide resolved
}

case .annotationDidSplit(let string, let keys, let libraryId):
return (string, SyncError.ErrorData(itemKeys: Array(keys), libraryId: libraryId))

Expand Down