From 93c8d06f23d62c850a170302b4587a545d36a156 Mon Sep 17 00:00:00 2001 From: Christoph Hagen Date: Thu, 9 Jan 2025 09:31:05 +0100 Subject: [PATCH] Show errors during loading --- CHDataManagement.xcodeproj/project.pbxproj | 4 ++ .../Page Generators/PageGenerator.swift | 5 +- CHDataManagement/Main/MainView.swift | 11 ++-- CHDataManagement/Model/Content.swift | 11 ++-- CHDataManagement/Model/FileResource.swift | 2 + CHDataManagement/Model/Item/ItemType.swift | 2 + .../Model/Loading/LoadingContext.swift | 6 ++ .../Model/Loading/ModelLoader.swift | 17 +++++ CHDataManagement/Storage/ErrorPrinter.swift | 10 +++ .../Storage/SecurityBookmark.swift | 62 +++++++++++-------- CHDataManagement/Storage/Storage.swift | 4 +- .../Views/Pages/PageContentResultsView.swift | 2 +- 12 files changed, 97 insertions(+), 39 deletions(-) create mode 100644 CHDataManagement/Storage/ErrorPrinter.swift diff --git a/CHDataManagement.xcodeproj/project.pbxproj b/CHDataManagement.xcodeproj/project.pbxproj index db60bf3..d7037ca 100644 --- a/CHDataManagement.xcodeproj/project.pbxproj +++ b/CHDataManagement.xcodeproj/project.pbxproj @@ -183,6 +183,7 @@ E2FD1D212D2EB22900B48627 /* ModelLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FD1D202D2EB22700B48627 /* ModelLoader.swift */; }; E2FD1D232D2EB27000B48627 /* LoadingResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FD1D222D2EB26C00B48627 /* LoadingResult.swift */; }; E2FD1D252D2EBA8000B48627 /* TagOverview.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FD1D242D2EBA7C00B48627 /* TagOverview.swift */; }; + E2FD1D282D2F2DAD00B48627 /* ErrorPrinter.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FD1D272D2F2D9100B48627 /* ErrorPrinter.swift */; }; E2FE0EE62D15A0B5002963B7 /* GenerationResults.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FE0EE52D15A0B1002963B7 /* GenerationResults.swift */; }; E2FE0EE82D16D4A3002963B7 /* ConvertThrowing.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FE0EE72D16D4A3002963B7 /* ConvertThrowing.swift */; }; E2FE0EEC2D1C1253002963B7 /* MultiFileSelectionView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FE0EEB2D1C124E002963B7 /* MultiFileSelectionView.swift */; }; @@ -412,6 +413,7 @@ E2FD1D202D2EB22700B48627 /* ModelLoader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ModelLoader.swift; sourceTree = ""; }; E2FD1D222D2EB26C00B48627 /* LoadingResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadingResult.swift; sourceTree = ""; }; E2FD1D242D2EBA7C00B48627 /* TagOverview.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TagOverview.swift; sourceTree = ""; }; + E2FD1D272D2F2D9100B48627 /* ErrorPrinter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ErrorPrinter.swift; sourceTree = ""; }; E2FE0EE52D15A0B1002963B7 /* GenerationResults.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GenerationResults.swift; sourceTree = ""; }; E2FE0EE72D16D4A3002963B7 /* ConvertThrowing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConvertThrowing.swift; sourceTree = ""; }; E2FE0EEB2D1C124E002963B7 /* MultiFileSelectionView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MultiFileSelectionView.swift; sourceTree = ""; }; @@ -699,6 +701,7 @@ E2A37D0F2CE5375E0000979F /* Storage */ = { isa = PBXGroup; children = ( + E2FD1D272D2F2D9100B48627 /* ErrorPrinter.swift */, E229904D2D135349009F8D77 /* SecurityBookmark.swift */, E22990492D10BB90009F8D77 /* SecurityScopeBookmark.swift */, E22990472D10B7B7009F8D77 /* StorageAccessError.swift */, @@ -1216,6 +1219,7 @@ E29D31A52D0CD03F0051B7F4 /* FileSelectionView.swift in Sources */, E22990322D0F767B009F8D77 /* DatePropertyView.swift in Sources */, E2A21C302CB490F90060935B /* HorizontalCenter.swift in Sources */, + E2FD1D282D2F2DAD00B48627 /* ErrorPrinter.swift in Sources */, E2FD1D1F2D2E9CC200B48627 /* ItemId.swift in Sources */, E2FE0EFA2D25AFBA002963B7 /* PageHeader.swift in Sources */, E25DA5252CFF73AB00AEF16D /* NSSize+Scaling.swift in Sources */, diff --git a/CHDataManagement/Generator/Page Generators/PageGenerator.swift b/CHDataManagement/Generator/Page Generators/PageGenerator.swift index f106ce9..b9dac44 100644 --- a/CHDataManagement/Generator/Page Generators/PageGenerator.swift +++ b/CHDataManagement/Generator/Page Generators/PageGenerator.swift @@ -35,7 +35,10 @@ final class PageGenerator { rawPageContent = existing } else { rawPageContent = makeEmptyPageContent(in: language) - results.markPageAsEmpty() + // Only mark non-draft pages as empty + if !page.isDraft { + results.markPageAsEmpty() + } } let pageContent = contentGenerator.generatePage(from: rawPageContent) diff --git a/CHDataManagement/Main/MainView.swift b/CHDataManagement/Main/MainView.swift index 8e80b2d..d006861 100644 --- a/CHDataManagement/Main/MainView.swift +++ b/CHDataManagement/Main/MainView.swift @@ -60,6 +60,9 @@ struct MainView: App { @State private var selectedTag: Tag? + @State + private var selectedFile: FileResource? + @State private var selectedSection: SettingsSection = .folders @@ -85,7 +88,7 @@ struct MainView: App { case .tags: TagListView(selectedTag: $selectedTag) case .files: - FileListView(selectedFile: $content.selectedFile) + FileListView(selectedFile: $selectedFile) case .generation: SettingsListView(selectedSection: $selectedSection) } @@ -101,7 +104,7 @@ struct MainView: App { case .tags: SelectedContentView(selected: $selectedTag) case .files: - SelectedContentView(selected: $content.selectedFile) + SelectedContentView(selected: $selectedFile) case .generation: GenerationContentView(selected: $selectedSection) } @@ -117,7 +120,7 @@ struct MainView: App { case .tags: SelectedDetailView(selected: $selectedTag) case .files: - SelectedDetailView(selected: $content.selectedFile) + SelectedDetailView(selected: $selectedFile) case .generation: GenerationDetailView(section: selectedSection) } @@ -133,7 +136,7 @@ struct MainView: App { case .tags: AddTagView(selected: $selectedTag) case .files: - AddFileView(selectedFile: $content.selectedFile) + AddFileView(selectedFile: $selectedFile) case .generation: Text("Not implemented") } diff --git a/CHDataManagement/Model/Content.swift b/CHDataManagement/Model/Content.swift index 3e8fbcb..e01805d 100644 --- a/CHDataManagement/Model/Content.swift +++ b/CHDataManagement/Model/Content.swift @@ -40,9 +40,6 @@ final class Content: ObservableObject { @Published private(set) var shouldGenerateWebsite = false - @State - var selectedFile: FileResource? - let imageGenerator: ImageGenerator init(settings: Settings, @@ -136,16 +133,18 @@ final class Content: ObservableObject { tag.remove(file) } settings.remove(file) - if selectedFile == file { - selectedFile = nil - } } func file(withOutputPath: String) -> FileResource? { files.first { $0.absoluteUrl == withOutputPath } } + private let errorPrinter = ErrorPrinter() + func loadFromDisk(callback: @escaping (_ errors: [String]) -> ()) { + defer { + storage.contentScope?.delegate = errorPrinter + } DispatchQueue.global().async { let loader = ModelLoader(content: self, storage: self.storage) let result = loader.load() diff --git a/CHDataManagement/Model/FileResource.swift b/CHDataManagement/Model/FileResource.swift index 3b27be4..765f25c 100644 --- a/CHDataManagement/Model/FileResource.swift +++ b/CHDataManagement/Model/FileResource.swift @@ -3,6 +3,8 @@ import SwiftUI final class FileResource: Item, LocalizedItem { + override var itemType: ItemType { .file } + let type: FileType /// Indicate if the file content is stored by the app diff --git a/CHDataManagement/Model/Item/ItemType.swift b/CHDataManagement/Model/Item/ItemType.swift index b443295..91858e0 100644 --- a/CHDataManagement/Model/Item/ItemType.swift +++ b/CHDataManagement/Model/Item/ItemType.swift @@ -7,6 +7,8 @@ enum ItemType: String, Equatable, Hashable { case tag = "tag" + case file = "file" + case tagOverview = "tag-overview" } diff --git a/CHDataManagement/Model/Loading/LoadingContext.swift b/CHDataManagement/Model/Loading/LoadingContext.swift index 0f3d9aa..c9c8ec2 100644 --- a/CHDataManagement/Model/Loading/LoadingContext.swift +++ b/CHDataManagement/Model/Loading/LoadingContext.swift @@ -105,6 +105,12 @@ final class LoadingContext { return nil } return tagOverview + case .file: + guard let id = itemId.id else { + error("Missing file id in itemId") + return nil + } + return file(id) } } } diff --git a/CHDataManagement/Model/Loading/ModelLoader.swift b/CHDataManagement/Model/Loading/ModelLoader.swift index 0fd6dc7..7345114 100644 --- a/CHDataManagement/Model/Loading/ModelLoader.swift +++ b/CHDataManagement/Model/Loading/ModelLoader.swift @@ -1,4 +1,17 @@ +final class LoadingErrorHandler: SecurityBookmarkErrorDelegate { + + let context: LoadingContext + + init(context: LoadingContext) { + self.context = context + } + + func securityBookmark(error: String) { + context.error("\(error)") + } +} + final class ModelLoader { let content: Content @@ -7,10 +20,14 @@ final class ModelLoader { let context: LoadingContext + let errorHandler: LoadingErrorHandler + init(content: Content, storage: Storage) { self.content = content self.storage = storage self.context = .init(content: content) + self.errorHandler = .init(context: context) + storage.contentScope?.delegate = errorHandler } func load() -> LoadingResult { diff --git a/CHDataManagement/Storage/ErrorPrinter.swift b/CHDataManagement/Storage/ErrorPrinter.swift new file mode 100644 index 0000000..c248e09 --- /dev/null +++ b/CHDataManagement/Storage/ErrorPrinter.swift @@ -0,0 +1,10 @@ +final class ErrorPrinter { + +} + +extension ErrorPrinter: SecurityBookmarkErrorDelegate { + + func securityBookmark(error: String) { + print(error) + } +} diff --git a/CHDataManagement/Storage/SecurityBookmark.swift b/CHDataManagement/Storage/SecurityBookmark.swift index 982a0c1..48dcd83 100644 --- a/CHDataManagement/Storage/SecurityBookmark.swift +++ b/CHDataManagement/Storage/SecurityBookmark.swift @@ -1,6 +1,11 @@ import Foundation import AppKit +protocol SecurityBookmarkErrorDelegate: AnyObject { + + func securityBookmark(error: String) +} + struct SecurityBookmark { enum OverwriteBehaviour { @@ -20,6 +25,8 @@ struct SecurityBookmark { private let fm = FileManager.default + weak var delegate: SecurityBookmarkErrorDelegate? + init(url: URL, isStale: Bool) { self.url = url self.isStale = isStale @@ -31,9 +38,7 @@ struct SecurityBookmark { func openFinderWindow(relativePath: String) { with(relativePath: relativePath) { path in - print("Opening file at \(path)") NSWorkspace.shared.activateFileViewerSelecting([path]) - return } } @@ -60,7 +65,7 @@ struct SecurityBookmark { do { data = try encoder.encode(value) } catch { - print("Failed to encode \(value): \(error)") + delegate?.securityBookmark(error: "Failed to encode \(value): \(error)") return false } return write(data, to: relativePath) @@ -71,6 +76,7 @@ struct SecurityBookmark { createParentFolder: Bool = true, ifFileExists overwrite: OverwriteBehaviour = .writeIfChanged) -> Bool { guard let data = string.data(using: .utf8) else { + delegate?.securityBookmark(error: "Failed to encode string to write to \(relativePath)") return false } return write(data, to: relativePath, createParentFolder: createParentFolder, ifFileExists: overwrite) @@ -85,7 +91,9 @@ struct SecurityBookmark { if exists(file) { switch overwrite { - case .fail: return false + case .fail: + delegate?.securityBookmark(error: "Failed to write \(relativePath): File exists") + return false case .skip: return true case .write: break case .writeIfChanged: @@ -99,7 +107,7 @@ struct SecurityBookmark { try createParentIfNeeded(of: file) try data.write(to: file) } catch { - print("Failed to write to file \(url.path()): \(error)") + delegate?.securityBookmark(error: "Failed to write \(relativePath): \(error)") return false } return true @@ -124,7 +132,11 @@ struct SecurityBookmark { guard let data = readData(at: relativePath) else { return nil } - return String(data: data, encoding: .utf8) + guard let result = String(data: data, encoding: .utf8) else { + delegate?.securityBookmark(error: "Failed to read \(relativePath): invalid UTF-8") + return nil + } + return result } func readData(at relativePath: String) -> Data? { @@ -135,7 +147,7 @@ struct SecurityBookmark { do { return try Data(contentsOf: file) } catch { - print("Storage: Failed to read file \(relativePath): \(error)") + delegate?.securityBookmark(error: "Failed to read \(relativePath) \(error)") return nil } } @@ -148,7 +160,7 @@ struct SecurityBookmark { do { return try decoder.decode(T.self, from: data) } catch { - print("Failed to decode file \(relativePath): \(error)") + delegate?.securityBookmark(error: "Failed to decode \(relativePath): \(error)") return nil } } @@ -163,7 +175,7 @@ struct SecurityBookmark { with(relativePath: relativeSource) { source in if !exists(source) { if !failIfMissing { return true } - print("ContentScope: Could not find file \(relativeSource) to move") + delegate?.securityBookmark(error: "Failed to move \(relativeSource): File does not exist") return false } @@ -171,7 +183,7 @@ struct SecurityBookmark { if exists(destination) { switch overwrite { case .fail: - print("ContentScope: Could not move file \(relativeSource), file exists") + delegate?.securityBookmark(error: "Failed to move to \(relativeDestination): File already exists") return false case .skip: return true case .write: break @@ -190,7 +202,7 @@ struct SecurityBookmark { try fm.moveItem(at: source, to: destination) return true } catch { - print("Failed to move \(source.path()) to \(destination.path())") + delegate?.securityBookmark(error: "Failed to move \(source.path()) to \(destination.path()): \(error)") return false } } @@ -204,7 +216,9 @@ struct SecurityBookmark { do { if destination.exists { switch overwrite { - case .fail: return false + case .fail: + delegate?.securityBookmark(error: "Failed to copy to \(relativePath): File already exists") + return false case .skip: return true case .write: break case .writeIfChanged: @@ -220,7 +234,7 @@ struct SecurityBookmark { try fm.copyItem(at: externalFile, to: destination) return true } catch { - print("Failed to copy \(externalFile.path()) to \(destination.path())") + delegate?.securityBookmark(error: "Failed to copy \(externalFile.path()) to \(relativePath): \(error)") return false } } @@ -229,14 +243,13 @@ struct SecurityBookmark { func deleteFile(at relativePath: String) -> Bool { with(relativePath: relativePath) { file in guard exists(file) else { - print("Scope: No file to delete at \(file.path())") return true } do { try fm.removeItem(at: file) return true } catch { - print("Failed to delete file \(file.path()): \(error)") + delegate?.securityBookmark(error: "Failed to delete \(relativePath): \(error)") return false } } @@ -291,9 +304,7 @@ struct SecurityBookmark { } func files(inRelativeFolder relativePath: String) -> [URL]? { - with(relativePath: relativePath) { folder in - files(in: folder) - } + with(relativePath: relativePath, perform: files) } /** @@ -312,14 +323,13 @@ struct SecurityBookmark { do { data = try Data(contentsOf: url) } catch { -#warning("Get these errors") - print("Storage: Failed to read file \(url.path()): \(error)") + delegate?.securityBookmark(error: "Failed to read \(url.path()): \(error)") return } do { items[id] = try decoder.decode(T.self, from: data) } catch { - print("Storage: Failed to decode file \(url.path()): \(error)") + delegate?.securityBookmark(error: "Failed to decode \(url.path()): \(error)") return } } @@ -341,7 +351,7 @@ struct SecurityBookmark { */ func perform(_ operation: (URL) -> Bool) -> Bool { guard url.startAccessingSecurityScopedResource() else { - print("Failed to start security scope") + delegate?.securityBookmark(error: "Failed to start security scope") return false } defer { url.stopAccessingSecurityScopedResource() } @@ -353,7 +363,7 @@ struct SecurityBookmark { */ func perform(_ operation: (URL) -> T?) -> T? { guard url.startAccessingSecurityScopedResource() else { - print("Failed to start security scope") + delegate?.securityBookmark(error: "Failed to start security scope") return nil } defer { url.stopAccessingSecurityScopedResource() } @@ -367,7 +377,7 @@ struct SecurityBookmark { try createIfNeeded(folder) return true } catch { - print("Failed to create folder \(folder.path())") + delegate?.securityBookmark(error: "Failed to create folder \(folder.path())") return false } } @@ -383,7 +393,7 @@ struct SecurityBookmark { do { try fm.removeItem(at: file) } catch { - print("Failed to remove \(file.path()): \(error)") + delegate?.securityBookmark(error: "Failed to delete \(file.path()): \(error)") return false } return true @@ -409,7 +419,7 @@ struct SecurityBookmark { do { return try files(in: folder).filter { !$0.lastPathComponent.hasPrefix(".") } } catch { - print("Failed to read list of files in \(folder.path())") + delegate?.securityBookmark(error: "Failed to read list of files in \(folder.path())") return nil } } diff --git a/CHDataManagement/Storage/Storage.swift b/CHDataManagement/Storage/Storage.swift index fa9b07d..92dd7ad 100644 --- a/CHDataManagement/Storage/Storage.swift +++ b/CHDataManagement/Storage/Storage.swift @@ -313,7 +313,9 @@ final class Storage: ObservableObject { - Returns: A dictionary with the file ids as keys and the metadata file as a value. */ func loadAllFiles() -> [String : (data: FileResource.Data, isExternal: Bool)]? { - guard let contentScope else { return nil } + guard let contentScope else { + return nil + } guard let list: [String : FileResource.Data] = contentScope.decodeJsonFiles(in: fileInfoFolderName) else { return nil } diff --git a/CHDataManagement/Views/Pages/PageContentResultsView.swift b/CHDataManagement/Views/Pages/PageContentResultsView.swift index fa771a5..8973592 100644 --- a/CHDataManagement/Views/Pages/PageContentResultsView.swift +++ b/CHDataManagement/Views/Pages/PageContentResultsView.swift @@ -67,7 +67,7 @@ struct PageContentResultsView: View { @ObservedObject var results: PageGenerationResults - #warning("Rework to only show a single popup will all files, and indicate missing ones") + #warning("Rework to only show a single popup with all files, and indicate missing ones") private var totalFileCount: Int { results.usedFiles.count + results.missingFiles.count + results.missingLinkedFiles.count }