Skip to content

Commit f98666d

Browse files
committed
Fix pCloud eventual consistency issues in createFolder, moveFile, moveFolder, uploadFile
Replace path-based checkForItemExistence with parent-based name collision detection that checks both files and folders. This prevents pCloud from creating a folder with the same name as an existing file (which pCloud uniquely allows). Add consistency barrier to pCloud integration tests to wait for listFolder API to catch up after setup.
1 parent 57d91f3 commit f98666d

2 files changed

Lines changed: 72 additions & 31 deletions

File tree

Sources/CryptomatorCloudAccess/PCloud/PCloudCloudProvider.swift

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -54,28 +54,36 @@ public class PCloudCloudProvider: CloudProvider {
5454
if isDirectory.boolValue {
5555
return Promise(CloudProviderError.itemTypeMismatch)
5656
}
57-
return fetchItemMetadata(at: cloudPath).then { metadata -> Void in
58-
if !replaceExisting || (replaceExisting && metadata.itemType == .folder) {
59-
throw CloudProviderError.itemAlreadyExists
60-
}
61-
}.recover { error -> Void in
62-
guard case CloudProviderError.itemNotFound = error else {
63-
throw error
57+
return resolveParentPath(forItemAt: cloudPath).then { parentItem -> Promise<PCloudItem> in
58+
let targetName = cloudPath.lastPathComponent
59+
return self.client.listFolder(parentItem.identifier, recursively: false).execute().then { metadata -> PCloudItem in
60+
let existingContent = metadata.contents.first(where: { $0.fileMetadata?.name == targetName || $0.folderMetadata?.name == targetName })
61+
if let existingContent = existingContent {
62+
if !replaceExisting || existingContent.folderMetadata != nil {
63+
throw CloudProviderError.itemAlreadyExists
64+
}
65+
}
66+
return parentItem
67+
}.recover { error -> PCloudItem in
68+
guard let error = error as? CallError<PCloudAPI.ListFolder.Error> else {
69+
throw error
70+
}
71+
if case CallError.methodError(.folderDoesNotExist) = error {
72+
throw CloudProviderError.parentFolderDoesNotExist
73+
} else {
74+
throw error
75+
}
6476
}
65-
}.then { _ -> Promise<PCloudItem> in
66-
return self.resolveParentPath(forItemAt: cloudPath)
6777
}.then { parentItem in
6878
return self.uploadFile(for: parentItem, from: localURL, to: cloudPath)
6979
}
7080
}
7181

7282
public func createFolder(at cloudPath: CloudPath) -> Promise<Void> {
73-
return checkForItemExistence(at: cloudPath).then { itemExists -> Void in
74-
if itemExists {
75-
throw CloudProviderError.itemAlreadyExists
83+
return resolveParentPath(forItemAt: cloudPath).then { parentItem -> Promise<PCloudItem> in
84+
return self.checkForNameCollision(cloudPath.lastPathComponent, inFolder: parentItem).then {
85+
return parentItem
7686
}
77-
}.then {
78-
self.resolveParentPath(forItemAt: cloudPath)
7987
}.then { parentItem in
8088
return self.createFolder(for: parentItem, with: cloudPath.lastPathComponent)
8189
}
@@ -94,26 +102,14 @@ public class PCloudCloudProvider: CloudProvider {
94102
}
95103

96104
public func moveFile(from sourceCloudPath: CloudPath, to targetCloudPath: CloudPath) -> Promise<Void> {
97-
return checkForItemExistence(at: targetCloudPath).then { itemExists -> Void in
98-
if itemExists {
99-
throw CloudProviderError.itemAlreadyExists
100-
}
101-
}.then {
102-
return all(self.resolvePath(forItemAt: sourceCloudPath), self.resolveParentPath(forItemAt: targetCloudPath))
103-
}.then { item, targetParentItem in
104-
return self.moveFile(from: item, toParent: targetParentItem, targetCloudPath: targetCloudPath)
105+
return resolveForMove(from: sourceCloudPath, to: targetCloudPath).then { sourceItem, targetParentItem in
106+
return self.moveFile(from: sourceItem, toParent: targetParentItem, targetCloudPath: targetCloudPath)
105107
}
106108
}
107109

108110
public func moveFolder(from sourceCloudPath: CloudPath, to targetCloudPath: CloudPath) -> Promise<Void> {
109-
return checkForItemExistence(at: targetCloudPath).then { itemExists -> Void in
110-
if itemExists {
111-
throw CloudProviderError.itemAlreadyExists
112-
}
113-
}.then {
114-
return all(self.resolvePath(forItemAt: sourceCloudPath), self.resolveParentPath(forItemAt: targetCloudPath))
115-
}.then { item, targetParentItem in
116-
return self.moveFolder(from: item, toParent: targetParentItem, targetCloudPath: targetCloudPath)
111+
return resolveForMove(from: sourceCloudPath, to: targetCloudPath).then { sourceItem, targetParentItem in
112+
return self.moveFolder(from: sourceItem, toParent: targetParentItem, targetCloudPath: targetCloudPath)
117113
}
118114
}
119115

@@ -405,6 +401,15 @@ public class PCloudCloudProvider: CloudProvider {
405401
return Promise(item)
406402
}
407403

404+
private func resolveForMove(from sourceCloudPath: CloudPath, to targetCloudPath: CloudPath) -> Promise<(PCloudItem, PCloudItem)> {
405+
return resolveParentPath(forItemAt: targetCloudPath).then { targetParentItem -> Promise<(PCloudItem, PCloudItem)> in
406+
return all(
407+
self.checkForNameCollision(targetCloudPath.lastPathComponent, inFolder: targetParentItem).then { targetParentItem },
408+
self.resolvePath(forItemAt: sourceCloudPath)
409+
)
410+
}
411+
}
412+
408413
private func resolveParentPath(forItemAt cloudPath: CloudPath) -> Promise<PCloudItem> {
409414
let parentCloudPath = cloudPath.deletingLastPathComponent()
410415
return resolvePath(forItemAt: parentCloudPath).recover { error -> PCloudItem in
@@ -456,6 +461,23 @@ public class PCloudCloudProvider: CloudProvider {
456461

457462
// MARK: - Helpers
458463

464+
private func checkForNameCollision(_ name: String, inFolder parentItem: PCloudItem) -> Promise<Void> {
465+
return client.listFolder(parentItem.identifier, recursively: false).execute().then { metadata -> Void in
466+
if metadata.contents.contains(where: { $0.fileMetadata?.name == name || $0.folderMetadata?.name == name }) {
467+
throw CloudProviderError.itemAlreadyExists
468+
}
469+
}.recover { error -> Void in
470+
guard let error = error as? CallError<PCloudAPI.ListFolder.Error> else {
471+
throw error
472+
}
473+
if case CallError.methodError(.folderDoesNotExist) = error {
474+
throw CloudProviderError.parentFolderDoesNotExist
475+
} else {
476+
throw error
477+
}
478+
}
479+
}
480+
459481
private func convertToCloudItemMetadata(_ content: Content, at cloudPath: CloudPath) throws -> CloudItemMetadata {
460482
if let fileMetadata = content.fileMetadata {
461483
return convertToCloudItemMetadata(fileMetadata, at: cloudPath)

Tests/CryptomatorCloudAccessIntegrationTests/PCloud/PCloudCloudProviderIntegrationTests.swift

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ import CryptomatorCloudAccessCore
1212
import CryptomatorCloudAccess
1313
#endif
1414
import PCloudSDKSwift
15-
import Promises
1615
import XCTest
16+
@testable import Promises
1717

1818
class PCloudCloudProviderIntegrationTests: CloudAccessIntegrationTestWithAuthentication {
1919
override class var defaultTestSuite: XCTestSuite {
@@ -29,6 +29,25 @@ class PCloudCloudProviderIntegrationTests: CloudAccessIntegrationTestWithAuthent
2929
// swiftlint:disable:next force_try
3030
setUpProvider = try! PCloudCloudProvider(client: client)
3131
super.setUp()
32+
guard classSetUpError == nil else { return }
33+
// Wait for pCloud's eventual consistency to catch up after setUp uploaded all test fixtures.
34+
let expectedItemCount = 6 // 5 files (test 0-4.txt) + 1 folder (testFolder)
35+
_ = waitForConsistency(provider: setUpProvider, folderPath: integrationTestRootCloudPath, expectedItemCount: expectedItemCount)
36+
guard waitForPromises(timeout: 60.0) else {
37+
classSetUpError = IntegrationTestError.oneTimeSetUpTimeout
38+
return
39+
}
40+
}
41+
42+
private static func waitForConsistency(provider: CloudProvider, folderPath: CloudPath, expectedItemCount: Int, attempt: Int = 0) -> Promise<Void> {
43+
return provider.fetchItemList(forFolderAt: folderPath, withPageToken: nil).then { itemList -> Promise<Void> in
44+
if itemList.items.count >= expectedItemCount || attempt >= 10 {
45+
return Promise(())
46+
}
47+
return Promise(()).delay(1.0).then {
48+
return waitForConsistency(provider: provider, folderPath: folderPath, expectedItemCount: expectedItemCount, attempt: attempt + 1)
49+
}
50+
}
3251
}
3352

3453
override func setUpWithError() throws {

0 commit comments

Comments
 (0)