Skip to content

Commit 9d3a90e

Browse files
i2h3i2h3
committed
fix: Directory enumeration.
- Our corporate instance was a real world use case in which the retrieved metadata of a directory and its content is no longer fulfilling the previous assumption that the first item is related to the queried directory itself. Instead, it can be anywhere in the array. Hence the implementation had to be changed to detect it by other means while taking the special `__NC_ROOT__` case into consideration. - While reading, I also applied code style changes based on conventional Swift code style. - Further, I hope to improve clarity by renaming some symbols like `DirectoryReadConversionActor` to `DirectoryMetadataContainer` which hints its purpose a lot better in my opinion. - Also replaced some occurrences of empty strings or `__NC_ROOT__` literals with the symbol reference to NextcloudKit. Signed-off-by: Iva Horn <142165879+i2h3@users.noreply.github.com>
1 parent bf4d890 commit 9d3a90e

8 files changed

Lines changed: 173 additions & 127 deletions

File tree

‎Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,35 +56,35 @@ extension Enumerator {
5656
return (metadatas, nil, nil, nil, error)
5757
}
5858

59-
guard var (directoryMetadata, _, metadatas) = await files.toDirectoryReadMetadatas(account: account) else {
60-
logger.error("Could not convert NKFiles to DirectoryReadMetadatas!")
59+
guard var (directory, _, files) = await files.toSendableDirectoryMetadata(account: account, directoryToRead: serverUrl) else {
60+
logger.error("Failed to convert array of NKFile to directory and files metadata objects!")
6161
return (nil, nil, nil, nil, .invalidData)
6262
}
6363

6464
// STORE DATA FOR CURRENTLY SCANNED DIRECTORY
65-
guard directoryMetadata.directory else {
66-
logger.error("Expected directory metadata but received file metadata for serverUrl: \(serverUrl)")
65+
guard directory.directory else {
66+
logger.error("Expected directory metadata but received file metadata!", [.url: serverUrl])
6767
return (nil, nil, nil, nil, .invalidData)
6868
}
6969

70-
if let existingMetadata = dbManager.itemMetadata(ocId: directoryMetadata.ocId) {
71-
directoryMetadata.downloaded = existingMetadata.downloaded
72-
directoryMetadata.keepDownloaded = existingMetadata.keepDownloaded
70+
if let existingMetadata = dbManager.itemMetadata(ocId: directory.ocId) {
71+
directory.downloaded = existingMetadata.downloaded
72+
directory.keepDownloaded = existingMetadata.keepDownloaded
7373
}
7474

75-
directoryMetadata.visitedDirectory = true
75+
directory.visitedDirectory = true
7676

77-
metadatas.insert(directoryMetadata, at: 0)
77+
files.insert(directory, at: 0)
7878

7979
let changedMetadatas = dbManager.depth1ReadUpdateItemMetadatas(
8080
account: account.ncKitAccount,
8181
serverUrl: serverUrl,
82-
updatedMetadatas: metadatas,
82+
updatedMetadatas: files,
8383
keepExistingDownloadState: true
8484
)
8585

8686
return (
87-
metadatas,
87+
files,
8888
changedMetadatas.newMetadatas,
8989
changedMetadatas.updatedMetadatas,
9090
changedMetadatas.deletedMetadatas,
@@ -214,9 +214,7 @@ extension Enumerator {
214214

215215
return ([metadata], newMetadatas, updatedMetadatas, nil, nextPage, nil)
216216
} else if depth == .targetAndDirectChildren {
217-
let (
218-
allMetadatas, newMetadatas, updatedMetadatas, deletedMetadatas, readError
219-
) = await handleDepth1ReadFileOrFolder(
217+
let (allMetadatas, newMetadatas, updatedMetadatas, deletedMetadatas, readError) = await handleDepth1ReadFileOrFolder(
220218
serverUrl: serverUrl,
221219
account: account,
222220
dbManager: dbManager,

‎Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift‎

Lines changed: 65 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,21 @@ extension NKFile {
1818
func toItemMetadata(uploaded: Bool = true) -> SendableItemMetadata {
1919
let creationDate = creationDate ?? date
2020
let uploadDate = uploadDate ?? date
21-
let classFile = (contentType == "text/markdown" || contentType == "text/x-markdown")
22-
&& classFile == NKTypeClassFile.unknow.rawValue
21+
22+
let classFile = (contentType == "text/markdown" || contentType == "text/x-markdown") && classFile == NKTypeClassFile.unknow.rawValue
2323
? NKTypeClassFile.document.rawValue
2424
: classFile
25+
2526
// Support for finding the correct filename for e2ee files should go here
2627

2728
// Don't ask me why, NextcloudKit renames and moves the root folder details
2829
// Also don't ask me why, but, NextcloudKit marks the NKFile for this as not a directory
2930
let rootServerUrl = urlBase + Account.webDavFilesUrlSuffix + userId
3031
let rootRequiresFixup = serverUrl == rootServerUrl && fileName == NextcloudKit.shared.nkCommonInstance.rootFileName
31-
let ocId = rootRequiresFixup
32-
? NSFileProviderItemIdentifier.rootContainer.rawValue
33-
: self.ocId
32+
let ocId = rootRequiresFixup ? NSFileProviderItemIdentifier.rootContainer.rawValue : self.ocId
3433
let directory = rootRequiresFixup ? true : self.directory
3534
let serverUrl = rootRequiresFixup ? rootServerUrl : self.serverUrl
36-
let fileName = rootRequiresFixup ? "" : self.fileName
35+
let fileName = rootRequiresFixup ? NextcloudKit.shared.nkCommonInstance.rootFileName : self.fileName
3736

3837
return SendableItemMetadata(
3938
ocId: ocId,
@@ -94,46 +93,79 @@ extension NKFile {
9493
}
9594
}
9695

96+
///
97+
/// Data container intended for use in combination with `concurrentChunkedForEach` to safely and concurrently convert a lot of metadata objects.
98+
///
99+
private final actor DirectoryMetadataContainer: Sendable {
100+
let root: SendableItemMetadata
101+
var directories: [SendableItemMetadata] = []
102+
var files: [SendableItemMetadata] = []
97103

104+
init(for root: SendableItemMetadata) {
105+
self.root = root
106+
}
98107

99-
extension Array<NKFile> {
100-
private final actor DirectoryReadConversionActor: Sendable {
101-
let directoryMetadata: SendableItemMetadata
102-
var childDirectoriesMetadatas: [SendableItemMetadata] = []
103-
var metadatas: [SendableItemMetadata] = []
104-
105-
func convertedMetadatas() -> (
106-
SendableItemMetadata, [SendableItemMetadata], [SendableItemMetadata]
107-
) {
108-
(directoryMetadata, childDirectoriesMetadatas, metadatas)
108+
///
109+
/// Insert a new item into the container.
110+
///
111+
func add(_ item: SendableItemMetadata) {
112+
files.append(item)
113+
114+
if item.directory {
115+
directories.append(item)
109116
}
117+
}
118+
119+
///
120+
/// Return a tuple of the total current content.
121+
///
122+
func content() -> (SendableItemMetadata, [SendableItemMetadata], [SendableItemMetadata]) {
123+
(root, directories, files)
124+
}
125+
}
110126

111-
init(target: SendableItemMetadata) {
112-
self.directoryMetadata = target
127+
extension Array<NKFile> {
128+
///
129+
/// Determine whether the given `NKFile` is the metadata object for the read remote directory.
130+
///
131+
func isDirectoryToRead(_ file: NKFile, directoryToRead: String) -> Bool {
132+
if file.serverUrl == directoryToRead && file.fileName == NextcloudKit.shared.nkCommonInstance.rootFileName {
133+
return true
113134
}
114135

115-
func add(metadata: SendableItemMetadata) {
116-
metadatas.append(metadata)
117-
if metadata.directory {
118-
childDirectoriesMetadatas.append(metadata)
119-
}
136+
if file.directory, "\(file.serverUrl)/\(file.fileName)" == directoryToRead {
137+
return true
120138
}
139+
140+
return false
121141
}
122142

123-
func toDirectoryReadMetadatas(account: Account) async -> (
124-
directoryMetadata: SendableItemMetadata,
125-
childDirectoriesMetadatas: [SendableItemMetadata],
126-
metadatas: [SendableItemMetadata]
127-
)? {
128-
guard var targetDirectoryMetadata = first?.toItemMetadata() else {
143+
///
144+
/// Convert an array of `NKFile` to `SendableItemMetadata`.
145+
///
146+
/// - Parameters:
147+
/// - account: The account which the metadata belongs to.
148+
/// - directoryToRead: The root path of the directory which this metadata comes from. This is required to distinguish the correct item for the metadata of the read directory itself from its children.
149+
///
150+
/// - Returns: A tuple consisting of the metadata for the read directory itself (`root`), any child directories (`directories`) and separately any directly containted files (`files`).
151+
///
152+
func toSendableDirectoryMetadata(account: Account, directoryToRead: String) async -> (root: SendableItemMetadata, directories: [SendableItemMetadata], files: [SendableItemMetadata])? {
153+
guard let root = first(where: { isDirectoryToRead($0, directoryToRead: directoryToRead) })?.toItemMetadata() else {
129154
return nil
130155
}
131-
let conversionActor = DirectoryReadConversionActor(target: targetDirectoryMetadata)
156+
157+
let container = DirectoryMetadataContainer(for: root)
158+
132159
if self.count > 1 {
133-
await self[1...].concurrentChunkedForEach { file in
134-
await conversionActor.add(metadata: file.toItemMetadata())
160+
await concurrentChunkedForEach { file in
161+
guard isDirectoryToRead(file, directoryToRead: directoryToRead) == false else {
162+
return
163+
}
164+
165+
await container.add(file.toItemMetadata())
135166
}
136167
}
137-
return await conversionActor.convertedMetadatas()
168+
169+
return await container.content()
138170
}
139171
}

‎Sources/NextcloudFileProviderKit/Item/Item+Create.swift‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,16 @@ public extension Item {
8787
))
8888
}
8989

90-
guard var (directoryMetadata, _, _) = await files.toDirectoryReadMetadatas(account: account)
91-
else {
92-
logger.error("Received nil directory read metadatas during conversion")
90+
guard var (directory, _, _) = await files.toSendableDirectoryMetadata(account: account, directoryToRead: remotePath) else {
91+
logger.error("Failed to resolve directory metadata on item conversion!")
9392
return (nil, NSFileProviderError(.cannotSynchronize))
9493
}
95-
directoryMetadata.downloaded = true
96-
dbManager.addItemMetadata(directoryMetadata)
94+
95+
directory.downloaded = true
96+
dbManager.addItemMetadata(directory)
9797

9898
let fpItem = Item(
99-
metadata: directoryMetadata,
99+
metadata: directory,
100100
parentItemIdentifier: parentItemIdentifier,
101101
account: account,
102102
remoteInterface: remoteInterface,

‎Tests/Interface/MockRemoteInterface.swift‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,7 @@ public class MockRemoteInterface: RemoteInterface {
614614
}
615615

616616
let sanitisedPath = sanitisedPath(remotePath, account: account)
617+
617618
guard sanitisedPath != "/" else {
618619
return remotePath.hasPrefix(account.trashUrl) ? rootTrashItem : rootItem
619620
}
@@ -624,11 +625,15 @@ public class MockRemoteInterface: RemoteInterface {
624625

625626
while pathComponents?.isEmpty == false {
626627
let component = pathComponents?.removeFirst()
627-
guard component?.isEmpty == false,
628-
let nextNode = currentNode?.children.first(where: { $0.name == component })
629-
else { return nil }
630628

631-
guard pathComponents?.isEmpty == false else { return nextNode } // This is the target
629+
guard component?.isEmpty == false, let nextNode = currentNode?.children.first(where: { $0.name == component }) else {
630+
return nil
631+
}
632+
633+
guard pathComponents?.isEmpty == false else {
634+
return nextNode // This is the target
635+
}
636+
632637
currentNode = nextNode
633638
}
634639

@@ -1031,12 +1036,15 @@ public class MockRemoteInterface: RemoteInterface {
10311036
taskHandler: @escaping (URLSessionTask) -> Void = { _ in }
10321037
) async -> (account: String, files: [NKFile], data: AFDataResponse<Data>?, error: NKError) {
10331038
var remotePath = remotePath
1039+
10341040
if remotePath.last == "." {
10351041
remotePath.removeLast()
10361042
}
1043+
10371044
if remotePath.last == "/" {
10381045
remotePath.removeLast()
10391046
}
1047+
10401048
print("Enumerating \(remotePath)")
10411049

10421050
// Call the enumerate call handler if it exists

‎Tests/Interface/MockRemoteItem.swift‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public class MockRemoteItem: Equatable {
5656
MockRemoteItem(
5757
identifier: NSFileProviderItemIdentifier.rootContainer.rawValue,
5858
versionIdentifier: "root",
59-
name: "",
59+
name: NextcloudKit.shared.nkCommonInstance.rootFileName,
6060
remotePath: account.davFilesUrl,
6161
directory: true,
6262
account: account.ncKitAccount,
@@ -120,17 +120,15 @@ public class MockRemoteItem: Equatable {
120120
let isRoot = identifier == NSFileProviderItemIdentifier.rootContainer.rawValue
121121
var file = NKFile()
122122
file.fileName = isRoot
123-
? "__NC_ROOT__"
123+
? NextcloudKit.shared.nkCommonInstance.rootFileName
124124
: trashbinOriginalLocation?.split(separator: "/").last?.toString() ?? name
125125
file.size = size
126126
file.date = creationDate
127127
file.directory = isRoot ? false : directory
128128
file.etag = versionIdentifier
129129
file.ocId = identifier
130130
file.fileId = identifier.replacingOccurrences(of: trashedItemIdSuffix, with: "")
131-
file.serverUrl = isRoot
132-
? serverUrl + "/remote.php/dav/files/" + userId
133-
: parent?.remotePath ?? remotePath
131+
file.serverUrl = isRoot ? "\(serverUrl)/remote.php/dav/files/\(userId)" : parent?.remotePath ?? serverUrl
134132
file.account = account
135133
file.user = username
136134
file.userId = userId

‎Tests/InterfaceTests/MockRemoteInterfaceTests.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-License-Identifier: GPL-2.0-or-later
33

44
import XCTest
5+
import NextcloudKit
56
@testable import NextcloudFileProviderKit
67
@testable import NextcloudFileProviderKitMocks
78
@testable import TestInterface
@@ -503,8 +504,7 @@ final class MockRemoteInterfaceTests: XCTestCase {
503504
let targetRootFile = result.files.first
504505
let expectedRoot = remoteInterface.rootItem
505506
XCTAssertEqual(targetRootFile?.ocId, expectedRoot?.identifier)
506-
XCTAssertEqual(targetRootFile?.fileName, "__NC_ROOT__") // NextcloudKit gives the root dir this name
507-
XCTAssertNotEqual(targetRootFile?.fileName, expectedRoot?.name)
507+
XCTAssertEqual(targetRootFile?.fileName, NextcloudKit.shared.nkCommonInstance.rootFileName) // NextcloudKit gives the root dir this name
508508
XCTAssertEqual(targetRootFile?.serverUrl, "https://mock.nc.com/remote.php/dav/files/testUserId") // NextcloudKit gives the root dir this url
509509
XCTAssertEqual(targetRootFile?.date, expectedRoot?.creationDate)
510510
XCTAssertEqual(targetRootFile?.etag, expectedRoot?.versionIdentifier)

0 commit comments

Comments
 (0)