Skip to content

Commit 153ac57

Browse files
claucambraclaucambra
authored
Merge pull request #80 from claucambra/bugfix/mitigate-no-parentitemidentifier
Implement mitigations for "parent item identifier not found" errors
2 parents 1fd2ee1 + 63eb9a4 commit 153ac57

15 files changed

Lines changed: 446 additions & 214 deletions

‎Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift‎

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,25 @@ public let relativeDatabaseFolderPath = "Database/"
2424
public let databaseFilename = "fileproviderextdatabase.realm"
2525

2626
public final class FilesDatabaseManager: Sendable {
27-
enum ErrorCode: Int {
27+
public enum ErrorCode: Int {
2828
case metadataNotFound = -1000
29+
case parentMetadataNotFound = -1001
30+
}
31+
public enum ErrorUserInfoKey: String {
32+
case missingParentServerUrlAndFileName = "MissingParentServerUrlAndFileName"
33+
}
34+
static let errorDomain = "FilesDatabaseManager"
35+
static func error(code: ErrorCode, userInfo: [String: String]) -> NSError {
36+
NSError(domain: Self.errorDomain, code: code.rawValue, userInfo: userInfo)
37+
}
38+
static func parentMetadataNotFoundError(itemUrl: String) -> NSError {
39+
error(
40+
code: .parentMetadataNotFound,
41+
userInfo: [ErrorUserInfoKey.missingParentServerUrlAndFileName.rawValue: itemUrl]
42+
)
2943
}
3044

3145
private static let schemaVersion = stable2_0SchemaVersion
32-
static let errorDomain = "FilesDatabaseManager"
3346
static let logger = Logger(subsystem: Logger.subsystem, category: "filesdatabase")
3447
let account: Account
3548

@@ -553,34 +566,50 @@ public final class FilesDatabaseManager: Sendable {
553566
return .trashContainer
554567
}
555568

556-
guard let itemParentDirectory = parentDirectoryMetadataForItem(metadata) else {
569+
guard let parentDirectoryMetadata = parentDirectoryMetadataForItem(metadata) else {
557570
Self.logger.error(
558571
"""
559-
Could not get item parent directory metadata for metadata.
572+
Could not get item parent directory item metadata for metadata.
560573
ocID: \(metadata.ocId, privacy: .public),
561-
etag: \(metadata.etag, privacy: .public),
574+
etag: \(metadata.etag, privacy: .public),
562575
fileName: \(metadata.fileName, privacy: .public),
563576
serverUrl: \(metadata.serverUrl, privacy: .public),
564577
account: \(metadata.account, privacy: .public),
565578
"""
566579
)
567580
return nil
568581
}
582+
return NSFileProviderItemIdentifier(parentDirectoryMetadata.ocId)
583+
}
569584

570-
if let parentDirectoryMetadata = itemMetadata(ocId: itemParentDirectory.ocId) {
571-
return NSFileProviderItemIdentifier(parentDirectoryMetadata.ocId)
585+
public func parentItemIdentifierWithRemoteFallback(
586+
fromMetadata metadata: SendableItemMetadata,
587+
remoteInterface: RemoteInterface,
588+
account: Account
589+
) async -> NSFileProviderItemIdentifier? {
590+
if let parentItemIdentifier = parentItemIdentifierFromMetadata(metadata) {
591+
return parentItemIdentifier
572592
}
573593

574-
Self.logger.error(
575-
"""
576-
Could not get item parent directory item metadata for metadata.
577-
ocID: \(metadata.ocId, privacy: .public),
578-
etag: \(metadata.etag, privacy: .public),
579-
fileName: \(metadata.fileName, privacy: .public),
580-
serverUrl: \(metadata.serverUrl, privacy: .public),
581-
account: \(metadata.account, privacy: .public),
582-
"""
594+
let (metadatas, _, _, _, error) = await Enumerator.readServerUrl(
595+
metadata.serverUrl,
596+
account: account,
597+
remoteInterface: remoteInterface,
598+
dbManager: self,
599+
depth: .target
583600
)
584-
return nil
601+
guard error == nil, let parentMetadata = metadatas?.first else {
602+
Self.logger.error(
603+
"""
604+
Could not retrieve parent item identifier remotely, received error.
605+
target metadata: \(metadata.ocId, privacy: .public)
606+
target filename: \(metadata.fileName, privacy: .public)
607+
received metadatas: \(metadatas?.count ?? 0, privacy: .public)
608+
error: \(error?.errorDescription ?? "NO ERROR", privacy: .public)
609+
"""
610+
)
611+
return nil
612+
}
613+
return NSFileProviderItemIdentifier(parentMetadata.ocId)
585614
}
586615
}

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,18 @@ extension Enumerator {
2525
}
2626

2727
Task { [metadatas] in
28-
let items = await metadatas.toFileProviderItems(
29-
account: account, remoteInterface: remoteInterface, dbManager: dbManager
30-
)
31-
32-
Task { @MainActor in
33-
observer.didEnumerate(items)
34-
Self.logger.info("Did enumerate \(items.count) trash items")
35-
observer.finishEnumerating(upTo: fileProviderPageforNumPage(numPage))
28+
do {
29+
let items = try await metadatas.toFileProviderItems(
30+
account: account, remoteInterface: remoteInterface, dbManager: dbManager
31+
)
32+
Task { @MainActor in
33+
observer.didEnumerate(items)
34+
Self.logger.info("Did enumerate \(items.count) trash items")
35+
observer.finishEnumerating(upTo: fileProviderPageforNumPage(numPage))
36+
}
37+
} catch let error {
38+
Self.logger.info("Unexpected error enumerating trash items, observing error.")
39+
Task { @MainActor in observer.finishEnumeratingWithError(error) }
3640
}
3741
}
3842
}

‎Sources/NextcloudFileProviderKit/Enumeration/Enumerator.swift‎

Lines changed: 132 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -612,28 +612,56 @@ public class Enumerator: NSObject, NSFileProviderEnumerator {
612612
remoteInterface: RemoteInterface,
613613
dbManager: FilesDatabaseManager,
614614
numPage: Int,
615-
itemMetadatas: [SendableItemMetadata]
615+
itemMetadatas: [SendableItemMetadata],
616+
handleInvalidParent: Bool = true
616617
) {
617618
Task {
618-
let items = await itemMetadatas.toFileProviderItems(
619-
account: account, remoteInterface: remoteInterface, dbManager: dbManager
620-
)
619+
do {
620+
let items = try await itemMetadatas.toFileProviderItems(
621+
account: account, remoteInterface: remoteInterface, dbManager: dbManager
622+
)
621623

622-
Task { @MainActor in
623-
observer.didEnumerate(items)
624-
Self.logger.info("Did enumerate \(items.count) items")
625-
626-
// TODO: Handle paging properly
627-
/*
628-
if items.count == maxItemsPerFileProviderPage {
629-
let nextPage = numPage + 1
630-
let providerPage = NSFileProviderPage("\(nextPage)".data(using: .utf8)!)
631-
observer.finishEnumerating(upTo: providerPage)
632-
} else {
633-
observer.finishEnumerating(upTo: nil)
634-
}
635-
*/
636-
observer.finishEnumerating(upTo: fileProviderPageforNumPage(numPage))
624+
Task { @MainActor in
625+
observer.didEnumerate(items)
626+
Self.logger.info("Did enumerate \(items.count) items")
627+
628+
// TODO: Handle paging properly
629+
/*
630+
if items.count == maxItemsPerFileProviderPage {
631+
let nextPage = numPage + 1
632+
let providerPage = NSFileProviderPage("\(nextPage)".data(using: .utf8)!)
633+
observer.finishEnumerating(upTo: providerPage)
634+
} else {
635+
observer.finishEnumerating(upTo: nil)
636+
}
637+
*/
638+
observer.finishEnumerating(upTo: fileProviderPageforNumPage(numPage))
639+
}
640+
} catch let error as NSError { // This error can only mean a missing parent item identifier
641+
guard handleInvalidParent else {
642+
Self.logger.info("Not handling invalid parent in enumeration")
643+
observer.finishEnumeratingWithError(error)
644+
return
645+
}
646+
do {
647+
let metadata = try await Self.attemptInvalidParentRecovery(
648+
error: error,
649+
account: account,
650+
remoteInterface: remoteInterface,
651+
dbManager: dbManager
652+
)
653+
Self.completeEnumerationObserver(
654+
observer,
655+
account: account,
656+
remoteInterface: remoteInterface,
657+
dbManager: dbManager,
658+
numPage: numPage,
659+
itemMetadatas: [metadata] + itemMetadatas,
660+
handleInvalidParent: false
661+
)
662+
} catch let error {
663+
observer.finishEnumeratingWithError(error)
664+
}
637665
}
638666
}
639667
}
@@ -647,7 +675,8 @@ public class Enumerator: NSObject, NSFileProviderEnumerator {
647675
dbManager: FilesDatabaseManager,
648676
newMetadatas: [SendableItemMetadata]?,
649677
updatedMetadatas: [SendableItemMetadata]?,
650-
deletedMetadatas: [SendableItemMetadata]?
678+
deletedMetadatas: [SendableItemMetadata]?,
679+
handleInvalidParent: Bool = true
651680
) {
652681
guard newMetadatas != nil || updatedMetadatas != nil || deletedMetadatas != nil else {
653682
Self.logger.error(
@@ -687,23 +716,95 @@ public class Enumerator: NSObject, NSFileProviderEnumerator {
687716
}
688717

689718
Task { [allUpdatedMetadatas, allDeletedMetadatas] in
690-
let updatedItems = await allUpdatedMetadatas.toFileProviderItems(
691-
account: account, remoteInterface: remoteInterface, dbManager: dbManager
692-
)
719+
do {
720+
let updatedItems = try await allUpdatedMetadatas.toFileProviderItems(
721+
account: account, remoteInterface: remoteInterface, dbManager: dbManager
722+
)
693723

694-
Task { @MainActor in
695-
if !updatedItems.isEmpty {
696-
observer.didUpdate(updatedItems)
697-
}
724+
Task { @MainActor in
725+
if !updatedItems.isEmpty {
726+
observer.didUpdate(updatedItems)
727+
}
698728

699-
Self.logger.info(
729+
Self.logger.info(
700730
"""
701731
Processed \(updatedItems.count) new or updated metadatas.
702-
\(allDeletedMetadatas.count) deleted metadatas.
732+
\(allDeletedMetadatas.count) deleted metadatas.
703733
"""
704-
)
705-
observer.finishEnumeratingChanges(upTo: anchor, moreComing: false)
734+
)
735+
observer.finishEnumeratingChanges(upTo: anchor, moreComing: false)
736+
}
737+
} catch let error as NSError { // This error can only mean a missing parent item identifier
738+
guard handleInvalidParent else {
739+
Self.logger.info("Not handling invalid parent in change enumeration")
740+
observer.finishEnumeratingWithError(error)
741+
return
742+
}
743+
do {
744+
let metadata = try await Self.attemptInvalidParentRecovery(
745+
error: error,
746+
account: account,
747+
remoteInterface: remoteInterface,
748+
dbManager: dbManager
749+
)
750+
var modifiedNewMetadatas = newMetadatas
751+
modifiedNewMetadatas?.append(metadata)
752+
Self.completeChangesObserver(
753+
observer,
754+
anchor: anchor,
755+
enumeratedItemIdentifier: enumeratedItemIdentifier,
756+
account: account,
757+
remoteInterface: remoteInterface,
758+
dbManager: dbManager,
759+
newMetadatas: modifiedNewMetadatas,
760+
updatedMetadatas: updatedMetadatas,
761+
deletedMetadatas: deletedMetadatas,
762+
handleInvalidParent: false
763+
)
764+
} catch let error {
765+
observer.finishEnumeratingWithError(error)
766+
}
706767
}
707768
}
708769
}
770+
771+
private static func attemptInvalidParentRecovery(
772+
error: NSError,
773+
account: Account,
774+
remoteInterface: RemoteInterface,
775+
dbManager: FilesDatabaseManager
776+
) async throws -> SendableItemMetadata {
777+
Self.logger.info("Attempting recovery from invalid parent identifier.")
778+
// Try to recover from errors involving missing metadata for a parent
779+
let userInfoKey =
780+
FilesDatabaseManager.ErrorUserInfoKey.missingParentServerUrlAndFileName.rawValue
781+
guard let urlToEnumerate = (error as NSError).userInfo[userInfoKey] as? String else {
782+
Self.logger.fault("No missing parent server url and filename in error user info.")
783+
assert(false)
784+
throw NSError()
785+
}
786+
787+
Self.logger.info(
788+
"Recovering from invalid parent identifier at \(urlToEnumerate, privacy: .public)"
789+
)
790+
let (metadatas, _, _, _, error) = await Enumerator.readServerUrl(
791+
urlToEnumerate,
792+
account: account,
793+
remoteInterface: remoteInterface,
794+
dbManager: dbManager,
795+
depth: .target
796+
)
797+
guard error == nil || error == .success, let metadata = metadatas?.first else {
798+
Self.logger.error(
799+
"""
800+
Problem retrieving parent for metadata.
801+
Error: \(error?.errorDescription ?? "NONE", privacy: .public)
802+
Metadatas: \(metadatas?.count ?? -1, privacy: .public)
803+
"""
804+
)
805+
throw error?.fileProviderError ?? NSFileProviderError(.cannotSynchronize)
806+
}
807+
// Provide it to the caller method so it can ingest it into the database and fix future errs
808+
return metadata
809+
}
709810
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,19 @@ extension Array {
3636
}
3737

3838
func concurrentChunkedCompactMap<T>(
39-
into size: Int = defaultChunkSize, transform: @escaping (Element) -> T?
40-
) async -> [T] {
41-
await withTaskGroup(of: [T].self) { group in
39+
into size: Int = defaultChunkSize, transform: @escaping (Element) throws -> T?
40+
) async throws -> [T] {
41+
try await withThrowingTaskGroup(of: [T].self) { group in
4242
var results = [T]()
4343
results.reserveCapacity(self.count)
4444

4545
for chunk in chunked(into: size) {
4646
group.addTask {
47-
return chunk.compactMap { transform($0) }
47+
return try chunk.compactMap { try transform($0) }
4848
}
4949
}
5050

51-
for await chunkResult in group {
51+
for try await chunkResult in group {
5252
results += chunkResult
5353
}
5454

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,13 @@ extension NKError {
8383
handlingCollisionAgainstItemInRemotePath problemRemotePath: String,
8484
dbManager: FilesDatabaseManager,
8585
remoteInterface: RemoteInterface
86-
) -> Error? {
86+
) async -> Error? {
8787
guard fileProviderError?.code == .filenameCollision else {
8888
return fileProviderError as Error?
8989
}
9090
guard let collidingItemMetadata = dbManager.itemMetadata(
9191
account: dbManager.account.ncKitAccount, locatedAtRemoteUrl: problemRemotePath
92-
), let collidingItem = Item.storedItem(
92+
), let collidingItem = await Item.storedItem(
9393
identifier: .init(collidingItemMetadata.ocId),
9494
account: dbManager.account,
9595
remoteInterface: remoteInterface,

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public extension Item {
4444
\(createError.errorDescription, privacy: .public)
4545
"""
4646
)
47-
return (nil, createError.fileProviderError(
47+
return (nil, await createError.fileProviderError(
4848
handlingCollisionAgainstItemInRemotePath: remotePath,
4949
dbManager: dbManager,
5050
remoteInterface: remoteInterface
@@ -79,7 +79,7 @@ public extension Item {
7979
\(readError.errorDescription, privacy: .public)
8080
"""
8181
)
82-
return (nil, readError.fileProviderError(
82+
return (nil, await readError.fileProviderError(
8383
handlingCollisionAgainstItemInRemotePath: remotePath,
8484
dbManager: dbManager,
8585
remoteInterface: remoteInterface
@@ -151,7 +151,7 @@ public extension Item {
151151
received ocId: \(ocId ?? "empty", privacy: .public)
152152
"""
153153
)
154-
return (nil, error.fileProviderError(
154+
return (nil, await error.fileProviderError(
155155
handlingCollisionAgainstItemInRemotePath: remotePath,
156156
dbManager: dbManager,
157157
remoteInterface: remoteInterface

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,10 @@ public extension Item {
282282

283283
dbManager.addItemMetadata(updatedMetadata)
284284

285-
guard let parentItemIdentifier = dbManager.parentItemIdentifierFromMetadata(
286-
updatedMetadata
285+
guard let parentItemIdentifier = await dbManager.parentItemIdentifierWithRemoteFallback(
286+
fromMetadata: metadata,
287+
remoteInterface: remoteInterface,
288+
account: account
287289
) else {
288290
Self.logger.error(
289291
"""

0 commit comments

Comments
 (0)