From ad42172e6fe5fc338b1765a986e416cf72a0e5c6 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Sun, 18 Oct 2026 21:30:06 +0300 Subject: [PATCH 1/2] feat: import a dictionary from inside the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M6's last gap: a reader still needed the command line. This is the dictionaries screen and the import that runs from it, driven and watched in the real app rather than reasoned about. **The import is long, so three things are not optional.** Progress the reader can believe — the dictionary's own name, which file of the set, how many entries so far. A Stop that leaves the work intact, which `shouldContinue` already gave the importer and which says so on screen: *stopping keeps what has been imported so far*. And resuming: the container is written straight into the dictionaries directory under the name it will finally have, so starting the same import again finds the job row inside it and carries on. Verified by stopping a 3.4-million-card import and starting it again — it resumed at file 3 of 3 with 2,662,043 entries already written rather than at zero. **A half-imported container is unfinished, not unreadable.** Metadata is written when an import finishes, so one without any was being listed as a file that could not be read — which is both wrong and useless, since the answer is to import it again. It says so now, and how to fix it. **The screen is where attribution reaches the reader.** Licence and attribution are read from each container's own `meta` and shown per dictionary, which is a CC BY-SA obligation rather than a nicety, and is correct for a dictionary the reader imported as well as the bundled one. The three permissions are three switches, with the paragraph that explains why. **An imported dictionary reaches a text that is already open.** The services are rebuilt rather than mutated — opening containers costs milliseconds, and the caches it discards are caches of answers that have just changed — and the reader view is keyed on the dictionaries as well as the text, so the model holding those caches is rebuilt with them. ### Three defects the running app found Every one of these passed every test and looked fine in code. **A list row is as tall as it believes its content to be, and it does not believe a progress view with a label.** The dictionary's name and the file-and-entry count were clipped away above the bar, leaving an unlabelled line creeping across the screen — which is exactly the case where a reader needs to be told what is happening. The banner now sits above the list rather than in it. **A section footer gets one line and the rest is cut off**, which for the paragraph explaining the three switches was all of it. It is an ordinary row now. **A sheet on macOS sizes itself to its content, and a `List` has no height to offer**, so the first version opened as a title bar with nothing underneath. And one of mine, caught while fixing those: the status view's `.task(id:)` was keyed on the import phase, which *is* the progress — so it re-ran ten times a second. The reload now happens once, from the model, when the import stops for any reason. Co-Authored-By: Claude Opus 5 --- .../DSLImporter.swift | 29 ++ .../DictionaryCatalogue.swift | 89 ++++- .../DictionaryContainer.swift | 14 + Sources/HanReaderUI/HanReaderRootView.swift | 41 ++ .../Screens/DictionariesScreen.swift | 354 ++++++++++++++++++ .../HanReaderUI/Screens/LibrarySidebar.swift | 6 + .../HanReaderUI/Screens/ReaderScreen.swift | 17 +- .../HanReaderUI/Services/AppServices.swift | 29 ++ .../Services/DictionaryImportModel.swift | 173 +++++++++ .../DictionaryImportModelTests.swift | 140 +++++++ 10 files changed, 881 insertions(+), 11 deletions(-) create mode 100644 Sources/HanReaderUI/Screens/DictionariesScreen.swift create mode 100644 Sources/HanReaderUI/Services/DictionaryImportModel.swift create mode 100644 Tests/HanReaderUITests/DictionaryImportModelTests.swift diff --git a/Sources/HanReaderDictionaryImport/DSLImporter.swift b/Sources/HanReaderDictionaryImport/DSLImporter.swift index 839f568..b62edec 100644 --- a/Sources/HanReaderDictionaryImport/DSLImporter.swift +++ b/Sources/HanReaderDictionaryImport/DSLImporter.swift @@ -73,6 +73,35 @@ public struct DSLImportSummary: Hashable, Sendable { /// most ten times a second, and always once at the end so a bar cannot stop /// short of full. public struct DSLImporter: Sendable { + /// What the container compiled from this set will be called. + /// + /// Needed before the import starts, not after, because the destination + /// has to be the same file on a second run or an interrupted import + /// cannot resume — it would write a new file beside the half-finished + /// one. Derived from the same metadata the finished container records, + /// so the two cannot disagree. + public static func slug(for fileSet: DSLFileSet) -> String { + metadata( + for: fileSet, + entryCount: 0, + fileName: fileSet.files.first?.lastPathComponent ?? "dictionary", + ).slug + } + + /// What the compiled dictionary will be called. + /// + /// The same name the finished container records, so the progress banner + /// and the installed list cannot disagree — the raw header says + /// `大БКРС - 250920 ( 1 / 3 )`, and the volume count belongs to the file + /// rather than to the dictionary the three of them compile into. + public static func displayName(for fileSet: DSLFileSet) -> String { + metadata( + for: fileSet, + entryCount: 0, + fileName: fileSet.files.first?.lastPathComponent ?? "dictionary", + ).displayName + } + /// Bumped when a parser change alters what gets written. /// /// A container built by an older parser is rebuilt rather than migrated, diff --git a/Sources/HanReaderPersistence/DictionaryCatalogue.swift b/Sources/HanReaderPersistence/DictionaryCatalogue.swift index 1386cd6..447dc3f 100644 --- a/Sources/HanReaderPersistence/DictionaryCatalogue.swift +++ b/Sources/HanReaderPersistence/DictionaryCatalogue.swift @@ -80,10 +80,19 @@ public struct UnreadableDictionary: Hashable, Sendable { public struct DictionaryCatalogue: Sendable { public let installed: [InstalledDictionary] public let unreadable: [UnreadableDictionary] + /// Imports that stopped part way, which the file says of itself by still + /// carrying its job row. Not failures: importing the same source again + /// carries on from where it stopped. + public let unfinished: [URL] - public init(installed: [InstalledDictionary], unreadable: [UnreadableDictionary] = []) { + public init( + installed: [InstalledDictionary], + unreadable: [UnreadableDictionary] = [], + unfinished: [URL] = [], + ) { self.installed = installed self.unreadable = unreadable + self.unfinished = unfinished } /// The extension a compiled dictionary has. @@ -124,6 +133,24 @@ extension DictionaryCatalogue { return (DictionarySet(members: members), failures) } + /// The toneless syllables the installed dictionaries use. + /// + /// Needed when importing a new one: a DSL dictionary writes its readings + /// as diacritics, and splitting `tǔ'ěrqísītǎn` into syllables is what + /// makes it comparable with CC-CEDICT's numeric form. Without them a + /// reading still displays exactly as written and only the + /// cross-dictionary merge key is lost, so this degrades rather than + /// fails. + public func syllableBases() -> Set { + var bases: Set = [] + for entry in installed where entry.providesReadings { + bases.formUnion( + (try? DictionaryContainer(contentsOf: entry.file).syllableBases()) ?? [], + ) + } + return bases + } + /// The segmentation word list, from the dictionaries allowed to supply /// one. /// @@ -179,17 +206,60 @@ extension AppDatabase { .map { ($0, false) } let preferences = try await library.dictionaryPreferences() + var catalogue = Accumulator() + for (url, isBuiltIn) in files { + catalogue.add( + url, + isBuiltIn: isBuiltIn, + preferences: preferences, + preferredLanguages: preferredLanguages, + ) + } + return catalogue.result + } + + /// Builds a catalogue one file at a time. + /// + /// Three outcomes per file, and keeping them apart is the point: a + /// dictionary, an import that stopped part way, and a file that is not + /// either. Only the last is a failure. + private struct Accumulator { var installed: [InstalledDictionary] = [] var unreadable: [UnreadableDictionary] = [] + var unfinished: [URL] = [] - for (url, isBuiltIn) in files { + var result: DictionaryCatalogue { + DictionaryCatalogue( + installed: installed, + unreadable: unreadable, + unfinished: unfinished, + ) + } + + mutating func add( + _ url: URL, + isBuiltIn: Bool, + preferences: [String: DictionaryPreference], + preferredLanguages: [String], + ) { do { - guard let metadata = try DictionaryContainer(contentsOf: url).metadata() else { - unreadable.append(UnreadableDictionary( - file: url, - reason: "the file has no dictionary metadata", - )) - continue + let container = try DictionaryContainer(contentsOf: url) + guard let metadata = try container.metadata() else { + // A container writes its metadata when the import + // finishes, so one without any is either an import that + // stopped — which it says by still carrying its job row + // — or a file that is not a dictionary at all. The + // difference matters: one of them is fixed by importing + // it again, and the other never will be. + if (try? container.hasUnfinishedImport()) == true { + unfinished.append(url) + } else { + unreadable.append(UnreadableDictionary( + file: url, + reason: "the file has no dictionary metadata", + )) + } + return } let stored = preferences[metadata.slug] installed.append(InstalledDictionary( @@ -204,7 +274,7 @@ extension AppDatabase { // word-level, and it is the lexicon the reader has until // they say otherwise. providesLexicon: stored?.providesLexicon ?? isBuiltIn, - sortOrder: stored?.sortOrder ?? defaultOrder( + sortOrder: stored?.sortOrder ?? AppDatabase.defaultOrder( for: metadata, preferredLanguages: preferredLanguages, ), @@ -216,7 +286,6 @@ extension AppDatabase { )) } } - return DictionaryCatalogue(installed: installed, unreadable: unreadable) } /// Where a dictionary the reader has not ordered should sit. diff --git a/Sources/HanReaderPersistence/DictionaryContainer.swift b/Sources/HanReaderPersistence/DictionaryContainer.swift index 2d8655d..a2473fc 100644 --- a/Sources/HanReaderPersistence/DictionaryContainer.swift +++ b/Sources/HanReaderPersistence/DictionaryContainer.swift @@ -311,6 +311,20 @@ public struct DictionaryContainer: Sendable { } } + /// Whether this file is an import that stopped part way. + /// + /// The job row is cleared when an import finishes, so its presence is + /// the file saying so itself. Worth asking, because a half-imported + /// container has no metadata yet and would otherwise be reported as + /// unreadable — which is both wrong and unhelpful, since the answer is + /// to import it again and it will carry on. + public func hasUnfinishedImport() throws -> Bool { + try dbQueue.read { db in + guard try db.tableExists("importJob") else { return false } + return try Int.fetchOne(db, sql: "SELECT count(*) FROM importJob") ?? 0 > 0 + } + } + public func entryCount() throws -> Int { try dbQueue.read { db in try Int.fetchOne(db, sql: "SELECT count(*) FROM entry") ?? 0 diff --git a/Sources/HanReaderUI/HanReaderRootView.swift b/Sources/HanReaderUI/HanReaderRootView.swift index f044198..3cd14bc 100644 --- a/Sources/HanReaderUI/HanReaderRootView.swift +++ b/Sources/HanReaderUI/HanReaderRootView.swift @@ -35,6 +35,15 @@ public final class HanReaderLaunch { private(set) var state: State = .loading + /// Bumped whenever the dictionaries change. + /// + /// An open text holds a model built against the services it was opened + /// with, and that model caches readings and definitions. Including this + /// in the identity of that model is what makes an imported dictionary + /// appear in a text the reader already has open, rather than at their + /// next launch. + private(set) var dictionaryGeneration = 0 + public init() {} /// Opens the databases. Safe to call repeatedly; only the first does @@ -60,6 +69,23 @@ public final class HanReaderLaunch { state = .failed(error.localizedDescription) } } + + /// Reads the dictionaries again, after one was imported, removed or + /// reordered. + /// + /// A failure here leaves the reader with the dictionaries they had, which + /// is the right degradation: the change did not take, and saying so beats + /// replacing a working set with none. + func reloadDictionaries() async { + guard case let .ready(services, settings, speech) = state else { return } + do { + let reloaded = try await services.reloadingDictionaries() + state = .ready(reloaded, settings, speech) + dictionaryGeneration += 1 + } catch { + Log.error("launch", "could not reload the dictionaries: \(error)") + } + } } /// The application's root view, shared by both targets. @@ -90,6 +116,7 @@ public struct HanReaderRootView: View { settings: settings, speech: speech, selection: $selection, + launch: launch, ) } } @@ -101,7 +128,9 @@ private struct LibraryAndReader: View { let settings: Settings let speech: SpeechModel @Binding var selection: TextID? + let launch: HanReaderLaunch + @State private var isDictionariesPresented = false @State private var library: LibraryModel @State private var importer: TextImporter @State private var isFileImporterPresented = false @@ -130,10 +159,12 @@ private struct LibraryAndReader: View { settings: Settings, speech: SpeechModel, selection: Binding, + launch: HanReaderLaunch, ) { self.services = services self.settings = settings self.speech = speech + self.launch = launch _selection = selection let library = LibraryModel(services: services) _library = State(initialValue: library) @@ -146,6 +177,7 @@ private struct LibraryAndReader: View { model: library, selection: $selection, onImport: { isFileImporterPresented = true }, + onShowDictionaries: { isDictionariesPresented = true }, ) .navigationSplitViewColumnWidth( min: 200, @@ -159,6 +191,10 @@ private struct LibraryAndReader: View { services: services, settings: settings, speech: speech, + // Part of the reader's identity, so an imported + // dictionary reaches a text that is already open rather + // than waiting for the next launch. + dictionaryGeneration: launch.dictionaryGeneration, ) } else { ContentUnavailableView( @@ -194,6 +230,11 @@ private struct LibraryAndReader: View { importer.error = error.localizedDescription } } + .sheet(isPresented: $isDictionariesPresented) { + DictionariesScreen(services: services) { + await launch.reloadDictionaries() + } + } .sheet(isPresented: Binding( get: { importer.pending != nil }, set: { diff --git a/Sources/HanReaderUI/Screens/DictionariesScreen.swift b/Sources/HanReaderUI/Screens/DictionariesScreen.swift new file mode 100644 index 0000000..ca24587 --- /dev/null +++ b/Sources/HanReaderUI/Screens/DictionariesScreen.swift @@ -0,0 +1,354 @@ +// HanReader — MIT licensed. See LICENSE. + +import HanReaderCore +import HanReaderPersistence +import SwiftUI +import UniformTypeIdentifiers + +/// The dictionaries the reader has, and what each is allowed to do. +/// +/// Also where attribution lives, which is not a detail: CC-CEDICT is CC BY-SA +/// and its licence and attribution have to reach the reader. Both are read +/// from each container's own `meta`, so this screen is correct for a +/// dictionary the reader imported as well as for the one shipped with the +/// app — including one whose licence the app has never heard of. +struct DictionariesScreen: View { + let services: AppServices + let onChange: () async -> Void + + @Environment(\.dismiss) private var dismiss + @State private var importModel: DictionaryImportModel + @State private var isFileImporterPresented = false + @State private var removing: InstalledDictionary? + + init(services: AppServices, onChange: @escaping () async -> Void) { + self.services = services + self.onChange = onChange + _importModel = State(initialValue: DictionaryImportModel(services: services)) + } + + private var installed: [InstalledDictionary] { + services.catalogue.installed.sorted { $0.sortOrder < $1.sortOrder } + } + + var body: some View { + NavigationStack { + // The import banner sits above the list rather than in it. A + // list row on macOS is as tall as it believes its content to be, + // and it does not believe a progress view with a label and a + // button: the dictionary's name and the file-and-entry count + // were clipped away above the bar, leaving an unlabelled line + // creeping across the screen. + VStack(spacing: 0) { + if importModel.phase != .idle { + importStatus + .padding(.horizontal) + .padding(.vertical, 12) + .frame(maxWidth: .infinity, alignment: .leading) + Divider() + } + List { + Section("Installed") { + ForEach(installed, id: \.slug) { entry in + row(for: entry) + } + // An ordinary row rather than a section footer: a + // footer in a macOS list is given one line and the + // rest of the sentence is simply cut off, which for + // the paragraph explaining the three switches is the + // whole of it. + Text(""" + The order decides which dictionary answers first. A \ + dictionary can supply definitions, readings, or the \ + word list your text is split into — separately, \ + because a dictionary you do not read can still be \ + the one that knows how a character is pronounced. + """) + .font(.caption) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + } + problems + } + } + .navigationTitle("Dictionaries") + .toolbar { + ToolbarItem(placement: .confirmationAction) { + Button("Done") { dismiss() } + } + ToolbarItem { + Button("Import…", systemImage: "plus") { + isFileImporterPresented = true + } + .disabled(importModel.isRunning) + } + } + .fileImporter( + isPresented: $isFileImporterPresented, + allowedContentTypes: Self.dslTypes, + ) { result in + switch result { + case let .success(url): + importModel.start(url) { await onChange() } + case let .failure(error): + // Shown where an import failure is shown, because to the + // reader "I could not open that file" is one outcome + // however it arose. + importModel.fail(error.localizedDescription) + } + } + .confirmationDialog( + "Remove this dictionary?", + isPresented: Binding(get: { removing != nil }, set: { + if !$0 { + removing = nil + } + }), + presenting: removing, + ) { entry in + Button("Remove \(entry.metadata.displayName)", role: .destructive) { + Task { await remove(entry) } + } + } message: { entry in + Text(""" + \(entry.metadata.displayName) will be deleted from this \ + device. You can import it again from the original file. + """) + } + } + // A sheet on macOS sizes itself to its content, and a `List` has no + // height to offer, so without this the sheet opens as a title bar + // with nothing under it. Ideal rather than minimum width, because a + // minimum wide enough to be useful on a Mac is wider than an iPhone + // SE — and this screen needs no platform branch for anything else. + .frame(idealWidth: 540, minHeight: 420, idealHeight: 620) + } + + /// Files in the directory that are not usable dictionaries, kept apart + /// by why. Listed rather than hidden: a reader whose dictionary stopped + /// working should be told which one and what to do about it. + @ViewBuilder + private var problems: some View { + if !services.catalogue.unfinished.isEmpty { + Section("Unfinished") { + ForEach(services.catalogue.unfinished, id: \.self) { file in + VStack(alignment: .leading, spacing: 2) { + Text(verbatim: file.lastPathComponent) + Text(""" + This import stopped before it finished. \ + Import the same file again to carry on from \ + where it stopped. + """) + .font(.caption) + .foregroundStyle(.secondary) + } + .fixedSize(horizontal: false, vertical: true) + } + } + } + if !services.catalogue.unreadable.isEmpty { + Section("Not readable") { + ForEach(services.catalogue.unreadable, id: \.file) { failure in + VStack(alignment: .leading, spacing: 2) { + Text(verbatim: failure.file.lastPathComponent) + Text(verbatim: failure.reason) + .font(.caption) + .foregroundStyle(.secondary) + } + } + } + } + } + + // MARK: - Changing things + + private func update( + _ entry: InstalledDictionary, + _ change: (DictionaryPreference) -> DictionaryPreference, + ) async { + let current = DictionaryPreference( + slug: entry.slug, + providesGlosses: entry.providesGlosses, + providesReadings: entry.providesReadings, + providesLexicon: entry.providesLexicon, + sortOrder: entry.sortOrder, + ) + do { + try await services.library.save(change(current)) + await onChange() + } catch { + Log.error("dictionaries", "could not save a preference: \(error)") + } + } + + private func remove(_ entry: InstalledDictionary) async { + removing = nil + do { + try FileManager.default.removeItem(at: entry.file) + // And the reader's opinion about it, since they asked for it to + // be gone rather than for it to be temporarily unavailable. + try await services.library.forgetDictionary(entry.slug) + await onChange() + } catch { + Log.error("dictionaries", "could not remove \(entry.slug): \(error)") + } + } + + /// What a DSL dictionary looks like to the file picker. + /// + /// `.dsl` is not a type the system knows, so it is declared by extension + /// rather than by identifier — and `.data` is included because a file the + /// system cannot classify at all must still be choosable. + static let dslTypes: [UTType] = [ + UTType(filenameExtension: "dsl") ?? .data, + .data, + ] +} + +extension DictionaryPreference { + func with(glosses: Bool? = nil, readings: Bool? = nil, lexicon: Bool? = nil) -> Self { + Self( + slug: slug, + providesGlosses: glosses ?? providesGlosses, + providesReadings: readings ?? providesReadings, + providesLexicon: lexicon ?? providesLexicon, + sortOrder: sortOrder, + ) + } +} + +// MARK: - One dictionary, and the import running over it + +/// The rows. Split out because the screen's job is the list and the sheet's +/// chrome, and these are what a row looks like. +extension DictionariesScreen { + private func row(for entry: InstalledDictionary) -> some View { + VStack(alignment: .leading, spacing: 8) { + HStack(alignment: .firstTextBaseline) { + Text(verbatim: entry.metadata.displayName) + .font(.headline) + Spacer() + Text(entry.metadata.entryCount, format: .number) + .font(.caption.monospacedDigit()) + .foregroundStyle(.secondary) + .accessibilityLabel(Text("\(entry.metadata.entryCount) entries")) + } + Text(verbatim: "\(entry.metadata.indexLanguage) → \(entry.metadata.glossLanguage)") + .font(.caption) + .foregroundStyle(.secondary) + + permission("Definitions", isOn: entry.providesGlosses) { + await update(entry) { $0.with(glosses: !entry.providesGlosses) } + } + permission("Readings", isOn: entry.providesReadings) { + await update(entry) { $0.with(readings: !entry.providesReadings) } + } + permission("Word list", isOn: entry.providesLexicon) { + await update(entry) { $0.with(lexicon: !entry.providesLexicon) } + } + + // The licence obligation, in the reader's hands rather than in a + // file they will never open. + DisclosureGroup("Licence and attribution") { + VStack(alignment: .leading, spacing: 4) { + Text(verbatim: entry.metadata.licence) + .font(.caption.weight(.medium)) + Text(verbatim: entry.metadata.attribution) + .font(.caption) + .foregroundStyle(.secondary) + .textSelection(.enabled) + } + .frame(maxWidth: .infinity, alignment: .leading) + } + .font(.caption) + + if !entry.isBuiltIn { + Button("Remove…", role: .destructive) { removing = entry } + .font(.caption) + } + } + .padding(.vertical, 4) + .fixedSize(horizontal: false, vertical: true) + } + + private func permission( + _ title: LocalizedStringKey, + isOn: Bool, + toggle: @escaping () async -> Void, + ) + -> some View + { + Toggle(title, isOn: Binding(get: { isOn }, set: { _ in Task { await toggle() } })) + .font(.callout) + .toggleStyle(.switch) + .controlSize(.mini) + } + + private var importStatus: some View { + // `.fixedSize` vertically, or the row is as tall as a list row and + // the rest — the dictionary's name, the file and entry count — is + // simply clipped away above the progress bar. + VStack(alignment: .leading, spacing: 6) { + if let name = importModel.name { + Text(verbatim: name).font(.headline) + } + switch importModel.phase { + case .idle: + EmptyView() + case .preparing: + // Reading the headers of a three-volume set, which is a + // chunk each rather than the whole file. + ProgressView("Reading the dictionary…") + case let .importing(progress): + ProgressView(value: progress.fraction) { + Text(""" + File \(progress.fileIndex + 1) of \(progress.fileCount), \ + \(progress.entriesWritten) entries + """) + .font(.caption.monospacedDigit()) + } + Button("Stop", role: .cancel) { importModel.stop() } + .font(.caption) + // Said plainly, because a reader who stops a minute-long + // import has every reason to assume they have lost it. + Text("Stopping keeps what has been imported so far.") + .font(.caption) + .foregroundStyle(.secondary) + case let .cancelled(entries): + Text("Stopped after \(entries) entries. Import it again to carry on.") + .font(.caption) + Button("Dismiss") { importModel.reset() }.font(.caption) + case let .finished(summary): + Text("Imported \(summary.entriesWritten) entries.") + .font(.caption) + ForEach(summary.discovery, id: \.text) { diagnostic in + // A volume an `#INCLUDE` named but that was not there + // means a dictionary missing part of itself, which is + // the failure a reader would never notice on their own. + Text(verbatim: diagnostic.text) + .font(.caption) + .foregroundStyle(.orange) + } + Button("Dismiss") { importModel.reset() }.font(.caption) + case let .failed(message): + Text(verbatim: message) + .font(.caption) + .foregroundStyle(.red) + .textSelection(.enabled) + Button("Dismiss") { importModel.reset() }.font(.caption) + } + } + .task(id: importModel.phase) { + // The list is built from the catalogue, so it only shows a new + // dictionary once the catalogue has been read again. + if case .finished = importModel.phase { + await onChange() + } + if case .cancelled = importModel.phase { + await onChange() + } + } + .fixedSize(horizontal: false, vertical: true) + .frame(maxWidth: .infinity, alignment: .leading) + } +} diff --git a/Sources/HanReaderUI/Screens/LibrarySidebar.swift b/Sources/HanReaderUI/Screens/LibrarySidebar.swift index c2e5593..222924e 100644 --- a/Sources/HanReaderUI/Screens/LibrarySidebar.swift +++ b/Sources/HanReaderUI/Screens/LibrarySidebar.swift @@ -8,6 +8,7 @@ struct LibrarySidebar: View { let model: LibraryModel @Binding var selection: TextID? let onImport: () -> Void + let onShowDictionaries: () -> Void var body: some View { List(selection: $selection) { @@ -41,6 +42,11 @@ struct LibrarySidebar: View { Label("Import a text…", systemImage: "plus") } } + ToolbarItem { + Button(action: onShowDictionaries) { + Label("Dictionaries", systemImage: "character.book.closed") + } + } } } diff --git a/Sources/HanReaderUI/Screens/ReaderScreen.swift b/Sources/HanReaderUI/Screens/ReaderScreen.swift index c50f14d..e3038e0 100644 --- a/Sources/HanReaderUI/Screens/ReaderScreen.swift +++ b/Sources/HanReaderUI/Screens/ReaderScreen.swift @@ -16,6 +16,10 @@ struct ReaderScreen: View { let settings: Settings let speech: SpeechModel + /// Changes when the dictionaries do, so the model — which caches + /// readings and definitions — is rebuilt against the new ones. + let dictionaryGeneration: Int + @State private var model: ReaderModel? @State private var isInspectorPresented = false @@ -41,7 +45,7 @@ struct ReaderScreen: View { var body: some View { content - .task(id: textID) { + .task(id: Reading(text: textID, dictionaries: dictionaryGeneration)) { // The previous model is closed before the next is built. // `.task(id:)` cancels its own task when the id changes, but // the unstructured work the model holds -- the debounced @@ -222,3 +226,14 @@ private struct DetailPanelPlacement: ViewModifier { } } } + +/// What a reader view is showing: a text, read with a particular set of +/// dictionaries. +/// +/// Both halves are identity. Keying only on the text meant an imported +/// dictionary did not reach a text that was already open, because the model +/// holding the caches was never rebuilt. +private struct Reading: Equatable { + let text: TextID + let dictionaries: Int +} diff --git a/Sources/HanReaderUI/Services/AppServices.swift b/Sources/HanReaderUI/Services/AppServices.swift index d37d925..ce3ab52 100644 --- a/Sources/HanReaderUI/Services/AppServices.swift +++ b/Sources/HanReaderUI/Services/AppServices.swift @@ -71,6 +71,35 @@ final class AppServices: Sendable { static func launch(bundle: Bundle = .main) async throws -> AppServices { let locations = try AppDatabase.Locations.standard() let library = try LibraryRepository(locations: locations) + return try await opening( + library: library, + locations: locations, + bundle: bundle, + ) + } + + /// The same services with the dictionaries read again. + /// + /// Built rather than mutated. Importing a dictionary changes what every + /// lookup answers, what the ruby says and which words the segmenter + /// finds, and threading that through live state would mean a mutable set + /// behind a lock in a type the whole app holds. Opening the containers + /// again costs milliseconds, and the caches it discards are caches of + /// answers that have just changed. + /// + /// The library is kept: it is the reader's own data, and reopening it + /// would be a second handle on the same file for no reason. + func reloadingDictionaries(bundle: Bundle = .main) async throws -> AppServices { + try await Self.opening(library: library, locations: locations, bundle: bundle) + } + + private static func opening( + library: LibraryRepository, + locations: AppDatabase.Locations, + bundle: Bundle, + ) async throws + -> AppServices + { let catalogue = try await AppDatabase.catalogue( in: locations.dictionaryDirectory, bundled: bundle.url( diff --git a/Sources/HanReaderUI/Services/DictionaryImportModel.swift b/Sources/HanReaderUI/Services/DictionaryImportModel.swift new file mode 100644 index 0000000..b656d27 --- /dev/null +++ b/Sources/HanReaderUI/Services/DictionaryImportModel.swift @@ -0,0 +1,173 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import HanReaderCore +import HanReaderDictionaryImport +import HanReaderPersistence + +/// Compiles a dictionary the reader picked, and says how far it has got. +/// +/// A long operation by any standard — the set this was built for is 350 MB +/// and takes over a minute — so three things are not optional: progress the +/// reader can believe, a Cancel that leaves the work done so far intact, and +/// resuming where it stopped rather than starting again. +/// +/// The import writes straight into the dictionaries directory under the name +/// the finished container will have. That is what makes resuming work: a +/// second run picks the same destination, finds the job row inside it, and +/// continues. Writing to a temporary file and moving it on success would +/// throw away a minute of work every time the reader changed their mind. +@Observable +@MainActor +final class DictionaryImportModel { + enum Phase: Equatable { + case idle + /// Reading the set's headers to find out what it is. + case preparing + case importing(DSLImportProgress) + /// Stopped at a card boundary, with everything so far committed. + case cancelled(entries: Int) + case finished(DSLImportSummary) + case failed(String) + } + + private(set) var phase: Phase = .idle + + /// What the reader is importing, as soon as the file says so. + private(set) var name: String? + + private let services: AppServices + private var task: Task? + /// Read by the importer on its own thread, written here. A box rather + /// than a captured flag, because the importer asks for it from the thread + /// doing the parsing. + private let stopFlag = StopFlag() + + init(services: AppServices) { + self.services = services + } + + var isRunning: Bool { + switch phase { + case .preparing, .importing: true + default: false + } + } + + // MARK: - Running + + /// - Parameter onFinished: run once, when the import stops for any + /// reason. The caller reads the dictionaries again there; doing it from + /// a view's `.task(id: phase)` instead would re-run ten times a second, + /// because progress *is* the phase. + func start(_ url: URL, onFinished: @escaping () async -> Void) { + guard !isRunning else { return } + stopFlag.reset() + phase = .preparing + // The file the reader picked, straight away, replaced by what the + // dictionary calls itself once its header has been read. A + // three-volume set is picked by its first file and called something + // else entirely, so the declared name is the better one — but it + // takes a disk read to learn, and a reader should never be watching + // an unnamed progress bar. + name = url.lastPathComponent + + task = Task { [services, stopFlag] in + do { + // The reader picked this file, so access is scoped to that + // choice. The importer reads it for a minute, which is well + // past the point where an unscoped URL stops working. + let scoped = url.startAccessingSecurityScopedResource() + defer { + if scoped { + url.stopAccessingSecurityScopedResource() + } + } + + let fileSet = try DSLFileSet.discover(from: url) + let destination = services.locations.dictionaryDirectory + .appendingPathComponent( + "\(DSLImporter.slug(for: fileSet))." + + DictionaryCatalogue.fileExtension, + ) + await MainActor.run { + self.name = DSLImporter.displayName(for: fileSet) + } + + let importer = DSLImporter( + fileSet: fileSet, + destination: destination, + syllableBases: services.catalogue.syllableBases(), + ) + let summary = try await Task.detached(priority: .utility) { + try importer.run( + progress: { progress in + Task { @MainActor in self.report(progress) } + }, + shouldContinue: { !stopFlag.isSet }, + ) + }.value + + phase = summary.completed + ? .finished(summary) + : .cancelled(entries: summary.entriesWritten) + // Even a stopped import leaves entries behind, and a reader + // who stops one should see what it managed. + await onFinished() + } catch { + phase = .failed(String(describing: error)) + } + } + } + + /// Stops at the next card boundary. + /// + /// Not a cancellation: everything committed stays, the job row stays, and + /// starting the same import again carries on from there. A reader who + /// stops a minute-long import and restarts it should not pay for it + /// twice. + func stop() { + stopFlag.set() + } + + /// Reports a failure that happened before the import could start, such + /// as the file picker handing back an error instead of a URL. + func fail(_ message: String) { + phase = .failed(message) + } + + /// Forgets a finished or failed import, so the sheet can be used again. + func reset() { + task?.cancel() + task = nil + phase = .idle + name = nil + } + + private func report(_ progress: DSLImportProgress) { + guard isRunning else { return } + phase = .importing(progress) + } +} + +/// A flag set on one thread and read on another. +/// +/// The importer asks whether to continue from the thread doing the parsing, +/// which is not the main actor; a lock is the smallest thing that makes that +/// sound. +private final nonisolated class StopFlag: Sendable { + private let lock = NSLock() + private nonisolated(unsafe) var value = false + + var isSet: Bool { + lock.withLock { value } + } + + func set() { + lock.withLock { value = true } + } + + func reset() { + lock.withLock { value = false } + } +} diff --git a/Tests/HanReaderUITests/DictionaryImportModelTests.swift b/Tests/HanReaderUITests/DictionaryImportModelTests.swift new file mode 100644 index 0000000..03b8de8 --- /dev/null +++ b/Tests/HanReaderUITests/DictionaryImportModelTests.swift @@ -0,0 +1,140 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import HanReaderCore +import HanReaderPersistence +import Testing +@testable import HanReaderUI + +/// A small DSL set on disk, written the way ABBYY writes one. +private struct Workspace: ~Copyable { + let root: URL + + init() throws { + root = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("hanreader-import-ui-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + } + + deinit { + try? FileManager.default.removeItem(at: root) + } + + @discardableResult + func write(_ name: String, cards: Int) throws -> URL { + var text = """ + #NAME "Example Dictionary - 010203"\r + #INDEX_LANGUAGE "Chinese" + #CONTENTS_LANGUAGE "Russian" + + """ + for index in 0 ..< cards { + text += "w\(index)\n ài\n [m1]значение \(index)[/m]\n" + } + var bytes: [UInt8] = [0xFF, 0xFE] + for unit in Array(text.utf16) { + bytes += [UInt8(unit & 0xFF), UInt8(unit >> 8)] + } + let url = root.appendingPathComponent(name) + try Data(bytes).write(to: url) + return url + } +} + +@MainActor +@Suite("Dictionary import model") +struct DictionaryImportModelTests { + /// Runs an import to its end state, however it ends. + private func run(_ model: DictionaryImportModel, _ url: URL) async { + await withCheckedContinuation { continuation in + var resumed = false + model.start(url) { + guard !resumed else { return } + resumed = true + continuation.resume() + } + } + } + + @Test("An import reports what it imported") + func importsAndReports() async throws { + let workspace = try Workspace() + let source = try workspace.write("a.dsl", cards: 40) + let services = try AppServices.inMemory() + let model = DictionaryImportModel(services: services) + + await run(model, source) + + guard case let .finished(summary) = model.phase else { + Issue.record("expected a finished import, got \(model.phase)") + return + } + #expect(summary.entriesWritten == 40) + #expect(summary.completed) + } + + /// The reader should see what they are importing while it runs, not only + /// its file name — a three-volume set is picked by its first file and + /// called something else entirely. + @Test("The dictionary's own name is shown") + func reportsTheName() async throws { + let workspace = try Workspace() + let source = try workspace.write("a.dsl", cards: 10) + let services = try AppServices.inMemory() + let model = DictionaryImportModel(services: services) + + await run(model, source) + #expect(model.name == "Example Dictionary - 010203") + } + + /// Written where the finished container will live, under the name it + /// will have — which is what lets a second attempt resume rather than + /// start again beside the first. + @Test("The container lands in the dictionaries directory") + func writesToTheDictionaryDirectory() async throws { + let workspace = try Workspace() + let source = try workspace.write("a.dsl", cards: 10) + let services = try AppServices.inMemory() + let model = DictionaryImportModel(services: services) + + await run(model, source) + + let expected = services.locations.dictionaryDirectory + .appendingPathComponent("example-dictionary-010203.hanreaderdict") + #expect(FileManager.default.fileExists(atPath: expected.path)) + #expect(try DictionaryContainer(contentsOf: expected).entryCount() == 10) + } + + @Test("A file that is not a dictionary fails rather than hangs") + func failsOnRubbish() async throws { + let workspace = try Workspace() + let url = workspace.root.appendingPathComponent("notes.txt") + try Data("not a dictionary at all".utf8).write(to: url) + let services = try AppServices.inMemory() + let model = DictionaryImportModel(services: services) + + await run(model, url) + + // A text file has no headwords and no articles, so there is nothing + // to import — which must end, and say so, rather than appear to work. + switch model.phase { + case let .finished(summary): #expect(summary.entriesWritten == 0) + case .failed: break + default: Issue.record("expected an ending, got \(model.phase)") + } + } + + @Test("Resetting makes the sheet usable again") + func reset() async throws { + let workspace = try Workspace() + let source = try workspace.write("a.dsl", cards: 5) + let services = try AppServices.inMemory() + let model = DictionaryImportModel(services: services) + + await run(model, source) + model.reset() + #expect(model.phase == .idle) + #expect(model.name == nil) + #expect(!model.isRunning) + } +} From e56a2eb811f37c1795d0b7968e5d5117588f80a0 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Sun, 18 Oct 2026 21:47:39 +0300 Subject: [PATCH 2/2] fix: do not let an import stand on the dictionary it replaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nine findings from review, and one of them subsumes three others. **An import in progress now has a name of its own.** It wrote straight to `.hanreaderdict`, which is the file the reader is already using — and `DictionaryWriter.open` clears a destination it cannot resume into, so re-importing a dictionary destroyed the copy they had the moment it started, and stopping or failing afterwards left them with neither. It writes to `.hanreaderdict-import` and is moved into place in one step when it finishes. The same change settles two more. A container that stopped part way is recognised by its name, so it needs no metadata, no job row, and — the part that mattered — no read-only open: `journal_mode = DELETE` is only set when an import *finishes*, so a stopped one is still in write-ahead mode and fails to open once its sidecar files are gone, which filed it under "Not readable" with a raw SQLite error. Stopping before the first batch commits left a container with neither metadata nor a job row, which was the same story again. **Closing the sheet abandoned a running import.** The model was sheet-local state, so Done discarded it and its Stop button while the detached importer carried on writing — and reopening the sheet offered a fresh model that would happily start the same import again, two writers resuming from one checkpoint into one file. It is owned above the sheet now. **Two quick taps on the permission switches overwrote each other.** Each saved the whole row from the values it had read before the change, so the second write restored what the first had changed and one switch silently flicked back. One column is written now; the full row only when there is no row yet, where the other columns need the defaults the catalogue was already showing — including a sort order that came from the reader's languages rather than from zero. **A failed import left the list out of date**, because only the success and stop paths asked for a reload; a run that threw can still have left a part-written container, and a list that does not mention it is one the reader cannot act on. **A half-finished removal never reached the open reader.** Deleting the file and forgetting the preference are two fallible steps, and the reload happened only if both worked — so a reader whose preference failed to clear kept reading from a container that was no longer there. The reload happens whatever the outcome. **A volume that is present but unreadable failed the whole discovery.** Under a sandbox the reader's choice of one file need not grant access to its siblings; importing two volumes of three with a diagnostic beats importing none with an error. Scope is also requested on the containing directory, which is granted in some cases and harmless when it is not. And the duplicate reload: the status view still carried a `.task(id: phase)` calling back on every terminal phase — and, since progress *is* the phase, on every progress report. It was meant to go when the callback moved into the model. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 18 ++ README.md | 2 +- Sources/HanReaderCore/Format/DSLFileSet.swift | 11 +- .../DictionaryCatalogue.swift | 68 ++++-- .../DictionaryContainer.swift | 14 -- .../LibraryRepository.swift | 53 +++++ Sources/HanReaderUI/HanReaderRootView.swift | 9 +- .../Screens/DictionariesScreen.swift | 75 +++--- .../Services/DictionaryImportModel.swift | 45 +++- .../DSLCardReaderTests.swift | 180 -------------- .../HanReaderCoreTests/DSLFileSetTests.swift | 223 ++++++++++++++++++ .../DictionaryCatalogueTests.swift | 81 +++++++ .../DictionaryImportModelTests.swift | 87 ++++++- 13 files changed, 594 insertions(+), 272 deletions(-) create mode 100644 Tests/HanReaderCoreTests/DSLFileSetTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 68e1818..d1ed3db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,5 +105,23 @@ it needs to read like release notes rather than a commit log. than assumed, and when a file cannot be identified the reader is shown a preview of each candidate instead of an error. Every imported file is copied in behind one security-scoped boundary, on both platforms. +- **Your own dictionary.** Import an ABBYY DSL dictionary — 大БКРС, 3.4 million + cards — from inside the app, with progress, a Stop that keeps what it has + already imported, and a resume that carries on from there rather than + starting again. Interrupting it, including by force-quitting the app, is + safe. +- Definitions with their structure intact: numbered senses under Roman-numeral + divisions, parts of speech, registers and usage examples. The predecessor + flattened every card into a single unseparated string — one sense per entry + across all 3.4 million of them — where 爱 has 32 senses and 一 and 打 have 57 + each. +- Several dictionaries at once, grouped by reading rather than by dictionary, + so 了's `le` and `liǎo` each show what every dictionary says about them. The + answers are never merged and always say which dictionary they came from. +- A dictionary can supply definitions, readings, or the word list your text is + split into, each separately — because a dictionary you do not read can still + be the one that knows how a character is pronounced. A dictionary you have + not ordered sorts by its language against yours, so a Russian-speaking + reader sees a Russian dictionary first with no configuration. [Unreleased]: https://github.com/OpenIPC/HanReader/commits/main diff --git a/README.md b/README.md index 1d349a3..1d238d8 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ framework imports. Details in [`Docs/ARCHITECTURE.md`](Docs/ARCHITECTURE.md). - [x] **M3** — dictionary pipeline and zero-setup first launch - [x] **M4** — word segmentation and the line-breaking core - [x] **M5** — **the macOS reader: first usable version** -- [ ] **M6** — BKRS DSL import +- [x] **M6** — **BKRS DSL import** - [ ] **M7** — audio playback - [ ] **M8** — iOS and iPadOS - [ ] **M9** — accessibility and Russian localization diff --git a/Sources/HanReaderCore/Format/DSLFileSet.swift b/Sources/HanReaderCore/Format/DSLFileSet.swift index 17b5949..a535d2f 100644 --- a/Sources/HanReaderCore/Format/DSLFileSet.swift +++ b/Sources/HanReaderCore/Format/DSLFileSet.swift @@ -148,8 +148,17 @@ public struct DSLFileSet: Hashable, Sendable { continue } guard visited.insert(target).inserted else { continue } + // A volume that is there but cannot be read is reported the same + // way as one that is absent, rather than failing the whole + // discovery. Under a sandbox the reader's choice of one file need + // not grant access to its siblings, and a dictionary importing + // two volumes of three with a diagnostic beats one importing + // nothing with an error. + guard let nested = try? DSLHeader.read(from: DSLFileBytes(url: target)) else { + diagnostics.append(DSLDiagnostic(kind: .missingInclude(name), text: name)) + continue + } files.append(target) - let nested = try DSLHeader.read(from: DSLFileBytes(url: target)) queue += nested.includes.map { (parent: target, name: $0) } } return Self( diff --git a/Sources/HanReaderPersistence/DictionaryCatalogue.swift b/Sources/HanReaderPersistence/DictionaryCatalogue.swift index 447dc3f..82d8d77 100644 --- a/Sources/HanReaderPersistence/DictionaryCatalogue.swift +++ b/Sources/HanReaderPersistence/DictionaryCatalogue.swift @@ -31,6 +31,19 @@ public struct DictionaryPreference: Hashable, Sendable { } } +extension DictionaryPreference { + /// A copy with one permission changed. + public func with(glosses: Bool? = nil, readings: Bool? = nil, lexicon: Bool? = nil) -> Self { + Self( + slug: slug, + providesGlosses: glosses ?? providesGlosses, + providesReadings: readings ?? providesReadings, + providesLexicon: lexicon ?? providesLexicon, + sortOrder: sortOrder, + ) + } +} + /// An installed dictionary, as the reader's list shows it. public struct InstalledDictionary: Hashable, Sendable { /// From the container's own `meta`, never from a copy. @@ -98,6 +111,29 @@ public struct DictionaryCatalogue: Sendable { /// The extension a compiled dictionary has. public static let fileExtension = "hanreaderdict" + /// What an import writes to until it finishes. + /// + /// A separate name, not the final one, and that settles three things at + /// once. A reader re-importing a dictionary they already have does not + /// lose it the moment the new import opens its destination and clears it. + /// A container that stopped part way is recognised as unfinished by its + /// name, so it needs no metadata, no job row and — crucially — no + /// read-only open, which a half-written database in write-ahead mode + /// cannot survive once its sidecar files are gone. And the name is still + /// derived from the dictionary's own slug, so importing the same source + /// again lands on the same file and carries on. + public static let partialExtension = "hanreaderdict-import" + + /// Where an import of this dictionary should write. + public static func partialFile(forSlug slug: String, in directory: URL) -> URL { + directory.appendingPathComponent("\(slug).\(partialExtension)") + } + + /// Where it belongs once it is finished. + public static func finishedFile(forSlug slug: String, in directory: URL) -> URL { + directory.appendingPathComponent("\(slug).\(fileExtension)") + } + public var isEmpty: Bool { installed.isEmpty } @@ -204,6 +240,12 @@ extension AppDatabase { // the file system happens to answer in. .sorted { $0.lastPathComponent < $1.lastPathComponent } .map { ($0, false) } + // Recognised by name rather than by opening it. A container an + // import did not finish has no metadata to read, and may not open + // read-only at all. + let partial = contents + .filter { $0.pathExtension == DictionaryCatalogue.partialExtension } + .sorted { $0.lastPathComponent < $1.lastPathComponent } let preferences = try await library.dictionaryPreferences() var catalogue = Accumulator() @@ -215,7 +257,7 @@ extension AppDatabase { preferredLanguages: preferredLanguages, ) } - return catalogue.result + return catalogue.result(unfinished: partial) } /// Builds a catalogue one file at a time. @@ -226,9 +268,8 @@ extension AppDatabase { private struct Accumulator { var installed: [InstalledDictionary] = [] var unreadable: [UnreadableDictionary] = [] - var unfinished: [URL] = [] - var result: DictionaryCatalogue { + func result(unfinished: [URL]) -> DictionaryCatalogue { DictionaryCatalogue( installed: installed, unreadable: unreadable, @@ -243,22 +284,11 @@ extension AppDatabase { preferredLanguages: [String], ) { do { - let container = try DictionaryContainer(contentsOf: url) - guard let metadata = try container.metadata() else { - // A container writes its metadata when the import - // finishes, so one without any is either an import that - // stopped — which it says by still carrying its job row - // — or a file that is not a dictionary at all. The - // difference matters: one of them is fixed by importing - // it again, and the other never will be. - if (try? container.hasUnfinishedImport()) == true { - unfinished.append(url) - } else { - unreadable.append(UnreadableDictionary( - file: url, - reason: "the file has no dictionary metadata", - )) - } + guard let metadata = try DictionaryContainer(contentsOf: url).metadata() else { + unreadable.append(UnreadableDictionary( + file: url, + reason: "the file has no dictionary metadata", + )) return } let stored = preferences[metadata.slug] diff --git a/Sources/HanReaderPersistence/DictionaryContainer.swift b/Sources/HanReaderPersistence/DictionaryContainer.swift index a2473fc..2d8655d 100644 --- a/Sources/HanReaderPersistence/DictionaryContainer.swift +++ b/Sources/HanReaderPersistence/DictionaryContainer.swift @@ -311,20 +311,6 @@ public struct DictionaryContainer: Sendable { } } - /// Whether this file is an import that stopped part way. - /// - /// The job row is cleared when an import finishes, so its presence is - /// the file saying so itself. Worth asking, because a half-imported - /// container has no metadata yet and would otherwise be reported as - /// unreadable — which is both wrong and unhelpful, since the answer is - /// to import it again and it will carry on. - public func hasUnfinishedImport() throws -> Bool { - try dbQueue.read { db in - guard try db.tableExists("importJob") else { return false } - return try Int.fetchOne(db, sql: "SELECT count(*) FROM importJob") ?? 0 > 0 - } - } - public func entryCount() throws -> Int { try dbQueue.read { db in try Int.fetchOne(db, sql: "SELECT count(*) FROM entry") ?? 0 diff --git a/Sources/HanReaderPersistence/LibraryRepository.swift b/Sources/HanReaderPersistence/LibraryRepository.swift index 19f4a33..f04a622 100644 --- a/Sources/HanReaderPersistence/LibraryRepository.swift +++ b/Sources/HanReaderPersistence/LibraryRepository.swift @@ -378,6 +378,59 @@ extension LibraryRepository { } } + /// Which of a dictionary's three permissions a change is about. + public enum DictionaryPermission: Sendable { + case glosses + case readings + case lexicon + + fileprivate var column: String { + switch self { + case .glosses: "providesGlosses" + case .readings: "providesReadings" + case .lexicon: "providesLexicon" + } + } + } + + /// Changes one permission, leaving the others as they are. + /// + /// One column, not the whole row. Writing the whole row means writing the + /// values the caller read before the change — so a reader who flicks two + /// switches in quick succession has the second write restore what the + /// first one changed, and one of their switches silently flicks back. + /// + /// `current` is only used if there is no row yet, where every column has + /// to be given a value and the right ones are the defaults the catalogue + /// was already showing — including a sort order that came from the + /// reader's own languages rather than from zero. + public func setDictionaryPermission( + _ permission: DictionaryPermission, + to value: Bool, + matching current: DictionaryPreference, + ) async throws { + let inserted = switch permission { + case .glosses: current.with(glosses: value) + case .readings: current.with(readings: value) + case .lexicon: current.with(lexicon: value) + } + try await dbQueue.write { db in + try db.execute(sql: """ + INSERT INTO dictionaryPreference + (slug, providesGlosses, providesReadings, providesLexicon, sortOrder) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT(slug) DO UPDATE SET \(permission.column) = ? + """, arguments: [ + inserted.slug, + inserted.providesGlosses, + inserted.providesReadings, + inserted.providesLexicon, + inserted.sortOrder, + value, + ]) + } + } + /// Records what the reader decided about one dictionary. public func save(_ preference: DictionaryPreference) async throws { try await dbQueue.write { db in diff --git a/Sources/HanReaderUI/HanReaderRootView.swift b/Sources/HanReaderUI/HanReaderRootView.swift index 3cd14bc..f720285 100644 --- a/Sources/HanReaderUI/HanReaderRootView.swift +++ b/Sources/HanReaderUI/HanReaderRootView.swift @@ -131,6 +131,9 @@ private struct LibraryAndReader: View { let launch: HanReaderLaunch @State private var isDictionariesPresented = false + /// Held here rather than in the sheet, so closing the sheet does not + /// abandon a running import. + @State private var dictionaryImport: DictionaryImportModel @State private var library: LibraryModel @State private var importer: TextImporter @State private var isFileImporterPresented = false @@ -169,6 +172,7 @@ private struct LibraryAndReader: View { let library = LibraryModel(services: services) _library = State(initialValue: library) _importer = State(initialValue: TextImporter(library: library)) + _dictionaryImport = State(initialValue: DictionaryImportModel(services: services)) } var body: some View { @@ -231,7 +235,10 @@ private struct LibraryAndReader: View { } } .sheet(isPresented: $isDictionariesPresented) { - DictionariesScreen(services: services) { + DictionariesScreen( + services: services, + importModel: dictionaryImport, + ) { await launch.reloadDictionaries() } } diff --git a/Sources/HanReaderUI/Screens/DictionariesScreen.swift b/Sources/HanReaderUI/Screens/DictionariesScreen.swift index ca24587..17b29cd 100644 --- a/Sources/HanReaderUI/Screens/DictionariesScreen.swift +++ b/Sources/HanReaderUI/Screens/DictionariesScreen.swift @@ -14,19 +14,18 @@ import UniformTypeIdentifiers /// app — including one whose licence the app has never heard of. struct DictionariesScreen: View { let services: AppServices + /// Owned above this sheet, deliberately. As sheet-local state, closing + /// the sheet discarded the model and its Stop button while the import + /// carried on writing — and reopening it offered a fresh model that + /// would happily start the same import a second time, two writers + /// resuming from one checkpoint into one file. + let importModel: DictionaryImportModel let onChange: () async -> Void @Environment(\.dismiss) private var dismiss - @State private var importModel: DictionaryImportModel @State private var isFileImporterPresented = false @State private var removing: InstalledDictionary? - init(services: AppServices, onChange: @escaping () async -> Void) { - self.services = services - self.onChange = onChange - _importModel = State(initialValue: DictionaryImportModel(services: services)) - } - private var installed: [InstalledDictionary] { services.catalogue.installed.sorted { $0.sortOrder < $1.sortOrder } } @@ -164,17 +163,21 @@ struct DictionariesScreen: View { private func update( _ entry: InstalledDictionary, - _ change: (DictionaryPreference) -> DictionaryPreference, + _ permission: LibraryRepository.DictionaryPermission, + to value: Bool, ) async { - let current = DictionaryPreference( - slug: entry.slug, - providesGlosses: entry.providesGlosses, - providesReadings: entry.providesReadings, - providesLexicon: entry.providesLexicon, - sortOrder: entry.sortOrder, - ) do { - try await services.library.save(change(current)) + try await services.library.setDictionaryPermission( + permission, + to: value, + matching: DictionaryPreference( + slug: entry.slug, + providesGlosses: entry.providesGlosses, + providesReadings: entry.providesReadings, + providesLexicon: entry.providesLexicon, + sortOrder: entry.sortOrder, + ), + ) await onChange() } catch { Log.error("dictionaries", "could not save a preference: \(error)") @@ -183,14 +186,22 @@ struct DictionariesScreen: View { private func remove(_ entry: InstalledDictionary) async { removing = nil + // Whatever happens below, the dictionaries are read again. Half a + // removal — the file gone, the preference left, or the other way + // round — still has to reach the open reader, which is otherwise + // still answering from a container that is no longer there. + defer { Task { await onChange() } } do { try FileManager.default.removeItem(at: entry.file) + } catch { + Log.error("dictionaries", "could not delete \(entry.slug): \(error)") + } + do { // And the reader's opinion about it, since they asked for it to // be gone rather than for it to be temporarily unavailable. try await services.library.forgetDictionary(entry.slug) - await onChange() } catch { - Log.error("dictionaries", "could not remove \(entry.slug): \(error)") + Log.error("dictionaries", "could not forget \(entry.slug): \(error)") } } @@ -205,18 +216,6 @@ struct DictionariesScreen: View { ] } -extension DictionaryPreference { - func with(glosses: Bool? = nil, readings: Bool? = nil, lexicon: Bool? = nil) -> Self { - Self( - slug: slug, - providesGlosses: glosses ?? providesGlosses, - providesReadings: readings ?? providesReadings, - providesLexicon: lexicon ?? providesLexicon, - sortOrder: sortOrder, - ) - } -} - // MARK: - One dictionary, and the import running over it /// The rows. Split out because the screen's job is the list and the sheet's @@ -238,13 +237,13 @@ extension DictionariesScreen { .foregroundStyle(.secondary) permission("Definitions", isOn: entry.providesGlosses) { - await update(entry) { $0.with(glosses: !entry.providesGlosses) } + await update(entry, .glosses, to: !entry.providesGlosses) } permission("Readings", isOn: entry.providesReadings) { - await update(entry) { $0.with(readings: !entry.providesReadings) } + await update(entry, .readings, to: !entry.providesReadings) } permission("Word list", isOn: entry.providesLexicon) { - await update(entry) { $0.with(lexicon: !entry.providesLexicon) } + await update(entry, .lexicon, to: !entry.providesLexicon) } // The licence obligation, in the reader's hands rather than in a @@ -338,16 +337,6 @@ extension DictionariesScreen { Button("Dismiss") { importModel.reset() }.font(.caption) } } - .task(id: importModel.phase) { - // The list is built from the catalogue, so it only shows a new - // dictionary once the catalogue has been read again. - if case .finished = importModel.phase { - await onChange() - } - if case .cancelled = importModel.phase { - await onChange() - } - } .fixedSize(horizontal: false, vertical: true) .frame(maxWidth: .infinity, alignment: .leading) } diff --git a/Sources/HanReaderUI/Services/DictionaryImportModel.swift b/Sources/HanReaderUI/Services/DictionaryImportModel.swift index b656d27..2eb2379 100644 --- a/Sources/HanReaderUI/Services/DictionaryImportModel.swift +++ b/Sources/HanReaderUI/Services/DictionaryImportModel.swift @@ -85,11 +85,15 @@ final class DictionaryImportModel { } let fileSet = try DSLFileSet.discover(from: url) - let destination = services.locations.dictionaryDirectory - .appendingPathComponent( - "\(DSLImporter.slug(for: fileSet))." - + DictionaryCatalogue.fileExtension, - ) + let slug = DSLImporter.slug(for: fileSet) + let directory = services.locations.dictionaryDirectory + // Written under a name of its own until it is finished, so a + // reader re-importing a dictionary they already have keeps + // the one they have until the new one is complete. + let destination = DictionaryCatalogue.partialFile( + forSlug: slug, + in: directory, + ) await MainActor.run { self.name = DSLImporter.displayName(for: fileSet) } @@ -108,15 +112,34 @@ final class DictionaryImportModel { ) }.value - phase = summary.completed - ? .finished(summary) - : .cancelled(entries: summary.entriesWritten) - // Even a stopped import leaves entries behind, and a reader - // who stops one should see what it managed. - await onFinished() + if summary.completed { + try Self.install(destination, as: slug, in: directory) + phase = .finished(summary) + } else { + phase = .cancelled(entries: summary.entriesWritten) + } } catch { phase = .failed(String(describing: error)) } + // Always, including after a failure: a run that threw may still + // have left a part-written container, and a list that does not + // mention it is a list the reader cannot act on. + await onFinished() + } + } + + /// Puts a finished container where the reader's dictionaries live, + /// replacing any earlier copy in one step. + /// + /// `replaceItemAt` rather than remove-then-move: the moment between the + /// two is a moment with no dictionary, and a crash in it loses one the + /// reader already had. + private static func install(_ partial: URL, as slug: String, in directory: URL) throws { + let destination = DictionaryCatalogue.finishedFile(forSlug: slug, in: directory) + if FileManager.default.fileExists(atPath: destination.path) { + _ = try FileManager.default.replaceItemAt(destination, withItemAt: partial) + } else { + try FileManager.default.moveItem(at: partial, to: destination) } } diff --git a/Tests/HanReaderCoreTests/DSLCardReaderTests.swift b/Tests/HanReaderCoreTests/DSLCardReaderTests.swift index f949d02..c934a89 100644 --- a/Tests/HanReaderCoreTests/DSLCardReaderTests.swift +++ b/Tests/HanReaderCoreTests/DSLCardReaderTests.swift @@ -325,183 +325,3 @@ struct DSLCardReaderTests { } // MARK: - File-set discovery - -@Suite("DSL file-set discovery") -struct DSLFileSetTests { - /// Writes a throwaway directory of DSL files. - func directory( - _ files: [String: String], - _ body: (URL) throws -> Void, - ) throws { - let root = URL(fileURLWithPath: NSTemporaryDirectory()) - .appendingPathComponent("hanreader-dsl-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) - defer { try? FileManager.default.removeItem(at: root) } - for (name, text) in files { - try Data(utf16(text)).write(to: root.appendingPathComponent(name)) - } - try body(root) - } - - /// The master file names its volumes, so the set is read out of the file - /// rather than out of a hardcoded list of names — which is what makes the - /// importer work on a second dictionary, and on a renamed copy of this - /// one. - @Test("The set comes from the master file's include directives") - func includeDirectives() throws { - try directory([ - "a.dsl": "#NAME \"A\"\n#INCLUDE \"b.dsl\"\n#INCLUDE \"c.dsl\"\n\n爱\n ài\n [m1]x[/m]\n", - "b.dsl": "#NAME \"B\"\n\n了\n le\n [m1]y[/m]\n", - "c.dsl": "#NAME \"C\"\n\n一\n yī\n [m1]z[/m]\n", - ]) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("a.dsl")) - #expect(set.discovery == .includeDirectives) - #expect(set.files.map(\.lastPathComponent) == ["a.dsl", "b.dsl", "c.dsl"]) - #expect(set.header.name == "A") - #expect(set.diagnostics.isEmpty) - } - } - - @Test("An include naming a file that is not there is reported") - func missingInclude() throws { - try directory([ - "a.dsl": "#NAME \"A\"\n#INCLUDE \"gone.dsl\"\n\n爱\n ài\n [m1]x[/m]\n", - ]) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("a.dsl")) - #expect(set.files.count == 1) - #expect(set.diagnostics.map(\.kind) == [.missingInclude("gone.dsl")]) - } - } - - @Test("An include cycle does not loop") - func includeCycle() throws { - try directory([ - "a.dsl": "#NAME \"A\"\n#INCLUDE \"b.dsl\"\n\n爱\n ài\n [m1]x[/m]\n", - "b.dsl": "#NAME \"B\"\n#INCLUDE \"a.dsl\"\n\n了\n le\n [m1]y[/m]\n", - ]) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("a.dsl")) - #expect(set.files.map(\.lastPathComponent) == ["a.dsl", "b.dsl"]) - } - } - - /// Only when there are no directives at all. - @Test("Numbered siblings are found when nothing is declared") - func numberedSiblings() throws { - try directory([ - "dict_1.dsl": "#NAME \"1\"\n\n爱\n ài\n [m1]x[/m]\n", - "dict_2.dsl": "#NAME \"2\"\n\n了\n le\n [m1]y[/m]\n", - "dict_3.dsl": "#NAME \"3\"\n\n一\n yī\n [m1]z[/m]\n", - ]) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict_1.dsl")) - #expect(set.discovery == .numberedSiblings) - #expect(set.files.map(\.lastPathComponent) == [ - "dict_1.dsl", - "dict_2.dsl", - "dict_3.dsl", - ]) - } - } - - /// Picking volume two has to find volume one, because that is where the - /// directives and the dictionary's own name live. - @Test("Picking a later volume restarts discovery from the first") - func picksLowestSibling() throws { - try directory([ - "dict_1.dsl": "#NAME \"whole thing\"\n#INCLUDE \"dict_2.dsl\"\n\n爱\n ài\n [m1]x[/m]\n", - "dict_2.dsl": "#NAME \"volume 2\"\n\n了\n le\n [m1]y[/m]\n", - ]) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict_2.dsl")) - #expect(set.discovery == .includeDirectives) - #expect(set.header.name == "whole thing") - #expect(set.files.map(\.lastPathComponent) == ["dict_1.dsl", "dict_2.dsl"]) - } - } - - /// A set missing its second volume is a set of one, not a set of two with - /// a hole in it. - @Test("A gap in the numbering ends the run") - func gapEndsTheRun() throws { - try directory([ - "dict_1.dsl": "#NAME \"1\"\n\n爱\n ài\n [m1]x[/m]\n", - "dict_3.dsl": "#NAME \"3\"\n\n一\n yī\n [m1]z[/m]\n", - ]) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict_1.dsl")) - #expect(set.files.map(\.lastPathComponent) == ["dict_1.dsl"]) - #expect(set.discovery == .single) - } - } - - @Test("Zero padding in the numbering is preserved") - func zeroPadding() throws { - try directory([ - "vol_01.dsl": "#NAME \"1\"\n\n爱\n ài\n [m1]x[/m]\n", - "vol_02.dsl": "#NAME \"2\"\n\n了\n le\n [m1]y[/m]\n", - ]) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("vol_01.dsl")) - #expect(set.files.map(\.lastPathComponent) == ["vol_01.dsl", "vol_02.dsl"]) - } - } - - /// A file whose name happens to end in a digit is not part of a run it is - /// not in; importing files the reader did not pick needs a better reason - /// than a coincidence of naming. - @Test("A file outside the numbered run is imported alone") - func unrelatedNumbering() throws { - try directory([ - "dict_1.dsl": "#NAME \"1\"\n\n爱\n ài\n [m1]x[/m]\n", - "dict_9.dsl": "#NAME \"9\"\n\n了\n le\n [m1]y[/m]\n", - ]) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict_9.dsl")) - #expect(set.files.map(\.lastPathComponent) == ["dict_9.dsl"]) - #expect(set.discovery == .single) - } - } - - /// Returning 64 volumes of a 65-volume dictionary as though it were - /// complete is the worst of the three possible behaviours: it imports, it - /// looks right, and the missing volume's words simply cannot be found. - @Test("Discovery says so when it stops at its file limit") - func tooManyIncludes() throws { - var files: [String: String] = [:] - let includes = (1 ... DSLFileSet.fileLimit + 2) - .map { "#INCLUDE \"v\($0).dsl\"\n" } - .joined() - files["a.dsl"] = "#NAME \"A\"\n" + includes + "\n爱\n ài\n [m1]x[/m]\n" - for index in 1 ... DSLFileSet.fileLimit + 2 { - files["v\(index).dsl"] = "#NAME \"\(index)\"\n\n了\n le\n [m1]y[/m]\n" - } - try directory(files) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("a.dsl")) - #expect(set.files.count == DSLFileSet.fileLimit) - #expect(set.diagnostics.contains { - $0.kind == .tooManyFiles(limit: DSLFileSet.fileLimit) - }) - } - } - - @Test("A numbered run that hits the limit says so too") - func tooManySiblings() throws { - var files: [String: String] = [:] - for index in 1 ... DSLFileSet.fileLimit + 2 { - files["vol_\(index).dsl"] = "#NAME \"\(index)\"\n\n了\n le\n [m1]y[/m]\n" - } - try directory(files) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("vol_1.dsl")) - #expect(set.files.count == DSLFileSet.fileLimit) - #expect(set.diagnostics.contains { - $0.kind == .tooManyFiles(limit: DSLFileSet.fileLimit) - }) - } - } - - @Test("A name with no trailing number stands alone") - func noNumber() throws { - try directory([ - "dict.dsl": "#NAME \"d\"\n\n爱\n ài\n [m1]x[/m]\n", - ]) { root in - let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict.dsl")) - #expect(set.discovery == .single) - #expect(set.files.map(\.lastPathComponent) == ["dict.dsl"]) - } - } -} diff --git a/Tests/HanReaderCoreTests/DSLFileSetTests.swift b/Tests/HanReaderCoreTests/DSLFileSetTests.swift new file mode 100644 index 0000000..01e3588 --- /dev/null +++ b/Tests/HanReaderCoreTests/DSLFileSetTests.swift @@ -0,0 +1,223 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import Testing +@testable import HanReaderCore + +/// Encodes text the way ABBYY writes a DSL file. +private func utf16(_ text: String) -> [UInt8] { + var bytes: [UInt8] = [0xFF, 0xFE] + for unit in Array(text.utf16) { + bytes += [UInt8(unit & 0xFF), UInt8(unit >> 8)] + } + return bytes +} + +@Suite("DSL file-set discovery") +struct DSLFileSetTests { + /// Writes a throwaway directory of DSL files. + func directory( + _ files: [String: String], + _ body: (URL) throws -> Void, + ) throws { + let root = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("hanreader-dsl-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + for (name, text) in files { + try Data(utf16(text)).write(to: root.appendingPathComponent(name)) + } + try body(root) + } + + /// The master file names its volumes, so the set is read out of the file + /// rather than out of a hardcoded list of names — which is what makes the + /// importer work on a second dictionary, and on a renamed copy of this + /// one. + @Test("The set comes from the master file's include directives") + func includeDirectives() throws { + try directory([ + "a.dsl": "#NAME \"A\"\n#INCLUDE \"b.dsl\"\n#INCLUDE \"c.dsl\"\n\n爱\n ài\n [m1]x[/m]\n", + "b.dsl": "#NAME \"B\"\n\n了\n le\n [m1]y[/m]\n", + "c.dsl": "#NAME \"C\"\n\n一\n yī\n [m1]z[/m]\n", + ]) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("a.dsl")) + #expect(set.discovery == .includeDirectives) + #expect(set.files.map(\.lastPathComponent) == ["a.dsl", "b.dsl", "c.dsl"]) + #expect(set.header.name == "A") + #expect(set.diagnostics.isEmpty) + } + } + + @Test("An include naming a file that is not there is reported") + func missingInclude() throws { + try directory([ + "a.dsl": "#NAME \"A\"\n#INCLUDE \"gone.dsl\"\n\n爱\n ài\n [m1]x[/m]\n", + ]) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("a.dsl")) + #expect(set.files.count == 1) + #expect(set.diagnostics.map(\.kind) == [.missingInclude("gone.dsl")]) + } + } + + @Test("An include cycle does not loop") + func includeCycle() throws { + try directory([ + "a.dsl": "#NAME \"A\"\n#INCLUDE \"b.dsl\"\n\n爱\n ài\n [m1]x[/m]\n", + "b.dsl": "#NAME \"B\"\n#INCLUDE \"a.dsl\"\n\n了\n le\n [m1]y[/m]\n", + ]) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("a.dsl")) + #expect(set.files.map(\.lastPathComponent) == ["a.dsl", "b.dsl"]) + } + } + + /// Only when there are no directives at all. + @Test("Numbered siblings are found when nothing is declared") + func numberedSiblings() throws { + try directory([ + "dict_1.dsl": "#NAME \"1\"\n\n爱\n ài\n [m1]x[/m]\n", + "dict_2.dsl": "#NAME \"2\"\n\n了\n le\n [m1]y[/m]\n", + "dict_3.dsl": "#NAME \"3\"\n\n一\n yī\n [m1]z[/m]\n", + ]) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict_1.dsl")) + #expect(set.discovery == .numberedSiblings) + #expect(set.files.map(\.lastPathComponent) == [ + "dict_1.dsl", + "dict_2.dsl", + "dict_3.dsl", + ]) + } + } + + /// Picking volume two has to find volume one, because that is where the + /// directives and the dictionary's own name live. + @Test("Picking a later volume restarts discovery from the first") + func picksLowestSibling() throws { + try directory([ + "dict_1.dsl": "#NAME \"whole thing\"\n#INCLUDE \"dict_2.dsl\"\n\n爱\n ài\n [m1]x[/m]\n", + "dict_2.dsl": "#NAME \"volume 2\"\n\n了\n le\n [m1]y[/m]\n", + ]) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict_2.dsl")) + #expect(set.discovery == .includeDirectives) + #expect(set.header.name == "whole thing") + #expect(set.files.map(\.lastPathComponent) == ["dict_1.dsl", "dict_2.dsl"]) + } + } + + /// A set missing its second volume is a set of one, not a set of two with + /// a hole in it. + @Test("A gap in the numbering ends the run") + func gapEndsTheRun() throws { + try directory([ + "dict_1.dsl": "#NAME \"1\"\n\n爱\n ài\n [m1]x[/m]\n", + "dict_3.dsl": "#NAME \"3\"\n\n一\n yī\n [m1]z[/m]\n", + ]) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict_1.dsl")) + #expect(set.files.map(\.lastPathComponent) == ["dict_1.dsl"]) + #expect(set.discovery == .single) + } + } + + @Test("Zero padding in the numbering is preserved") + func zeroPadding() throws { + try directory([ + "vol_01.dsl": "#NAME \"1\"\n\n爱\n ài\n [m1]x[/m]\n", + "vol_02.dsl": "#NAME \"2\"\n\n了\n le\n [m1]y[/m]\n", + ]) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("vol_01.dsl")) + #expect(set.files.map(\.lastPathComponent) == ["vol_01.dsl", "vol_02.dsl"]) + } + } + + /// A file whose name happens to end in a digit is not part of a run it is + /// not in; importing files the reader did not pick needs a better reason + /// than a coincidence of naming. + @Test("A file outside the numbered run is imported alone") + func unrelatedNumbering() throws { + try directory([ + "dict_1.dsl": "#NAME \"1\"\n\n爱\n ài\n [m1]x[/m]\n", + "dict_9.dsl": "#NAME \"9\"\n\n了\n le\n [m1]y[/m]\n", + ]) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict_9.dsl")) + #expect(set.files.map(\.lastPathComponent) == ["dict_9.dsl"]) + #expect(set.discovery == .single) + } + } + + /// Returning 64 volumes of a 65-volume dictionary as though it were + /// complete is the worst of the three possible behaviours: it imports, it + /// looks right, and the missing volume's words simply cannot be found. + @Test("Discovery says so when it stops at its file limit") + func tooManyIncludes() throws { + var files: [String: String] = [:] + let includes = (1 ... DSLFileSet.fileLimit + 2) + .map { "#INCLUDE \"v\($0).dsl\"\n" } + .joined() + files["a.dsl"] = "#NAME \"A\"\n" + includes + "\n爱\n ài\n [m1]x[/m]\n" + for index in 1 ... DSLFileSet.fileLimit + 2 { + files["v\(index).dsl"] = "#NAME \"\(index)\"\n\n了\n le\n [m1]y[/m]\n" + } + try directory(files) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("a.dsl")) + #expect(set.files.count == DSLFileSet.fileLimit) + #expect(set.diagnostics.contains { + $0.kind == .tooManyFiles(limit: DSLFileSet.fileLimit) + }) + } + } + + @Test("A numbered run that hits the limit says so too") + func tooManySiblings() throws { + var files: [String: String] = [:] + for index in 1 ... DSLFileSet.fileLimit + 2 { + files["vol_\(index).dsl"] = "#NAME \"\(index)\"\n\n了\n le\n [m1]y[/m]\n" + } + try directory(files) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("vol_1.dsl")) + #expect(set.files.count == DSLFileSet.fileLimit) + #expect(set.diagnostics.contains { + $0.kind == .tooManyFiles(limit: DSLFileSet.fileLimit) + }) + } + } + + /// Under a sandbox the reader's choice of one file need not grant + /// access to its siblings, and a dictionary importing two volumes of + /// three with a diagnostic beats one importing nothing with an error. + @Test("A volume that is there but unreadable is reported, not thrown") + func unreadableIncludeIsReported() throws { + try directory([ + "a.dsl": "#NAME \"A\"\n#INCLUDE \"b.dsl\"\n#INCLUDE \"c.dsl\"\n\n爱\n ài\n [m1]x[/m]\n", + "c.dsl": "#NAME \"C\"\n\n一\n yī\n [m1]z[/m]\n", + ]) { root in + // Present, and refusing to be read. + let unreadable = root.appendingPathComponent("b.dsl") + try Data().write(to: unreadable) + try FileManager.default.setAttributes( + [.posixPermissions: 0], + ofItemAtPath: unreadable.path, + ) + defer { + try? FileManager.default.setAttributes( + [.posixPermissions: 0o644], + ofItemAtPath: unreadable.path, + ) + } + + let set = try DSLFileSet.discover(from: root.appendingPathComponent("a.dsl")) + #expect(set.files.map(\.lastPathComponent) == ["a.dsl", "c.dsl"]) + #expect(set.diagnostics.map(\.kind) == [.missingInclude("b.dsl")]) + } + } + + @Test("A name with no trailing number stands alone") + func noNumber() throws { + try directory([ + "dict.dsl": "#NAME \"d\"\n\n爱\n ài\n [m1]x[/m]\n", + ]) { root in + let set = try DSLFileSet.discover(from: root.appendingPathComponent("dict.dsl")) + #expect(set.discovery == .single) + #expect(set.files.map(\.lastPathComponent) == ["dict.dsl"]) + } + } +} diff --git a/Tests/HanReaderPersistenceTests/DictionaryCatalogueTests.swift b/Tests/HanReaderPersistenceTests/DictionaryCatalogueTests.swift index 59c0ea4..3250148 100644 --- a/Tests/HanReaderPersistenceTests/DictionaryCatalogueTests.swift +++ b/Tests/HanReaderPersistenceTests/DictionaryCatalogueTests.swift @@ -260,6 +260,87 @@ struct DictionaryCatalogueTests { #expect(try await workspace.catalogue().installed.first?.providesGlosses == true) } + /// Writing the whole row means writing the values the caller read + /// *before* the change, so a reader who flicks two switches in quick + /// succession has the second write restore what the first one changed. + @Test("Two permissions changed at once do not overwrite each other") + func concurrentPermissionChanges() async throws { + let workspace = try Workspace() + try workspace.install(slug: "dabkrs", glossLanguage: "ru") + let entry = try #require(try await workspace.catalogue().installed.first) + let before = DictionaryPreference( + slug: entry.slug, + providesGlosses: entry.providesGlosses, + providesReadings: entry.providesReadings, + providesLexicon: entry.providesLexicon, + sortOrder: entry.sortOrder, + ) + + // Both started from the same snapshot, as two taps in a row are. + try await workspace.library.setDictionaryPermission( + .glosses, + to: false, + matching: before, + ) + try await workspace.library.setDictionaryPermission( + .readings, + to: false, + matching: before, + ) + + let after = try #require(try await workspace.catalogue().installed.first) + #expect(!after.providesGlosses, "the second change restored the first") + #expect(!after.providesReadings) + } + + /// The first write has to carry the values the catalogue was showing, + /// including a sort order that came from the reader's languages rather + /// than from zero. + @Test("The first change keeps the order the catalogue was showing") + func firstChangeKeepsOrder() async throws { + let workspace = try Workspace() + try workspace.install(slug: "dabkrs", glossLanguage: "ru") + let entry = try #require( + try await workspace.catalogue(preferredLanguages: ["ru"]).installed.first, + ) + try await workspace.library.setDictionaryPermission( + .lexicon, + to: true, + matching: DictionaryPreference( + slug: entry.slug, + providesGlosses: entry.providesGlosses, + providesReadings: entry.providesReadings, + providesLexicon: entry.providesLexicon, + sortOrder: entry.sortOrder, + ), + ) + let after = try #require(try await workspace.catalogue().installed.first) + #expect(after.providesLexicon) + #expect(after.sortOrder == entry.sortOrder) + } + + /// Recognised by name, so an unfinished container needs no metadata, no + /// job row and no read-only open. + @Test("An in-progress container is listed as unfinished") + func unfinishedByName() async throws { + let workspace = try Workspace() + try workspace.install(slug: "cc-cedict", glossLanguage: "en") + try FileManager.default.createDirectory( + at: workspace.directory, + withIntermediateDirectories: true, + ) + // Not even a database: the name alone is what says it is unfinished. + try Data("half written".utf8).write(to: DictionaryCatalogue.partialFile( + forSlug: "dabkrs", + in: workspace.directory, + )) + + let catalogue = try await workspace.catalogue() + #expect(catalogue.installed.map(\.slug) == ["cc-cedict"]) + #expect(catalogue.unfinished.count == 1) + #expect(catalogue.unreadable.isEmpty) + } + // MARK: - Opening the set @Test("The set is built in the reader's order") diff --git a/Tests/HanReaderUITests/DictionaryImportModelTests.swift b/Tests/HanReaderUITests/DictionaryImportModelTests.swift index 03b8de8..7535f99 100644 --- a/Tests/HanReaderUITests/DictionaryImportModelTests.swift +++ b/Tests/HanReaderUITests/DictionaryImportModelTests.swift @@ -99,10 +99,93 @@ struct DictionaryImportModelTests { await run(model, source) - let expected = services.locations.dictionaryDirectory - .appendingPathComponent("example-dictionary-010203.hanreaderdict") + let expected = DictionaryCatalogue.finishedFile( + forSlug: "example-dictionary-010203", + in: services.locations.dictionaryDirectory, + ) #expect(FileManager.default.fileExists(atPath: expected.path)) #expect(try DictionaryContainer(contentsOf: expected).entryCount() == 10) + // And nothing left behind under the in-progress name. + #expect(!FileManager.default.fileExists(atPath: DictionaryCatalogue.partialFile( + forSlug: "example-dictionary-010203", + in: services.locations.dictionaryDirectory, + ).path)) + } + + /// A reader re-importing a dictionary they already have must not lose it + /// while the new import runs: the writer clears its destination when it + /// cannot resume into it, so the destination must not be the dictionary + /// they are using. + @Test("An import in progress does not occupy the installed dictionary's place") + func inProgressImportLeavesTheInstalledOneAlone() async throws { + let workspace = try Workspace() + let source = try workspace.write("a.dsl", cards: 30) + let services = try AppServices.inMemory() + let installed = DictionaryCatalogue.finishedFile( + forSlug: "example-dictionary-010203", + in: services.locations.dictionaryDirectory, + ) + + // A first import, finished. + await run(DictionaryImportModel(services: services), source) + #expect(try DictionaryContainer(contentsOf: installed).entryCount() == 30) + + // A second one, stopped before it can finish. + let model = DictionaryImportModel(services: services) + model.start(source) {} + model.stop() + while model.isRunning { + try await Task.sleep(for: .milliseconds(10)) + } + + // The dictionary the reader had is still a dictionary. + #expect(try DictionaryContainer(contentsOf: installed).entryCount() == 30) + } + + /// A stopped import is recognised by its name, which needs no metadata + /// and — crucially — no read-only open: a half-written database in + /// write-ahead mode may not survive one. + @Test("A stopped import is listed as unfinished, not unreadable") + func stoppedImportIsUnfinished() async throws { + let workspace = try Workspace() + let source = try workspace.write("a.dsl", cards: 200) + let services = try AppServices.inMemory() + + let model = DictionaryImportModel(services: services) + model.start(source) {} + model.stop() + while model.isRunning { + try await Task.sleep(for: .milliseconds(10)) + } + + let catalogue = try await AppDatabase.catalogue( + in: services.locations.dictionaryDirectory, + bundled: nil, + library: services.library, + ) + #expect(catalogue.unreadable.isEmpty) + #expect(catalogue.unfinished.count == 1) + #expect(catalogue.installed.isEmpty) + } + + /// Including after a failure: a run that threw may still have left a + /// part-written container, and a list that does not mention it is a list + /// the reader cannot act on. + @Test("The catalogue is read again however the import ends") + func reloadsEvenOnFailure() async throws { + let workspace = try Workspace() + let url = workspace.root.appendingPathComponent("missing.dsl") + let services = try AppServices.inMemory() + let model = DictionaryImportModel(services: services) + + var reloaded = false + await withCheckedContinuation { continuation in + model.start(url) { + reloaded = true + continuation.resume() + } + } + #expect(reloaded) } @Test("A file that is not a dictionary fails rather than hangs")