diff --git a/devs.md b/devs.md index d618e88c..a9acec7c 100644 --- a/devs.md +++ b/devs.md @@ -144,6 +144,8 @@ The application is composed from Services, ServiceModules, serviceFeatures, add- The normal composition order is the Service Hub, Replicator provider registration, ServiceModules, serviceFeatures, add-ons, and finally legacy Module binding. A serviceFeature may therefore consume an already constructed ServiceModule. Preferring a serviceFeature for new composition is a dependency-boundary rule, not an initialisation-order rule. +Declare the feature host type explicitly when a narrow dependency contract is intended. `NecessaryServices` selects service keys from `THub` and module keys from `ServiceModules`; it retains `context` only when the selected source provides it, and defaults to `ServiceHub`. `createServiceFeature()` returns the supplied function with its complete signature and result type, but it does not infer dependency keys from the callback body. Unannotated legacy callbacks retain the full default `ServiceHub` contract for compatibility. Put feature-only collaborators, such as a borrowed `LogFunction`, in the feature's own dependency type. + Mutable state is permitted in a serviceFeature. State alone is not a reason to create a class, a ServiceModule, or retain an AbstractModule. Prefer one private context, with module-level functions which receive that context, when identity and polymorphism are not part of the contract. Separate the state, transitions, and invariants from the surrounding function which registers lifecycle handlers and connects downstream effects. Give the stateful boundary narrow collaborators rather than `LiveSyncBaseCore`. Use a class when stable object identity, replaceable implementations, or an explicit external-resource lifecycle such as serialised ownership, `dispose()`, or `abort()` is part of the contract. Use a ServiceModule when that operational capability or resource lifecycle must also be shared explicitly by several consumers. Do not introduce a class merely to group dependencies or make private functions callable. @@ -200,6 +202,8 @@ steps required to add a built-in provider. Commonlib owns one stable `LiveSyncP2PService`, its `P2PRoomSessionOwner`, and the replaceable Trystero room session. Host commands, event handlers, and views consume the focused transport, connection-probe admission, directory, peer-admission, transfer, change-relay, configuration, and diagnostic views returned by the service feature. They must not retain the deprecated compatibility Replicator as an ordinary service locator, close Trystero-owned raw peers, or install another Trystero transport generation at the application root. The exact implemented ownership and shutdown boundaries are recorded in Commonlib's [P2P transport lifecycle](https://github.com/vrtmrz/livesync-commonlib/blob/main/docs/p2p-transport-lifecycle.md) design document. +The Obsidian `useP2PReplicatorUI` feature declares only `API`, `appLifecycle`, `setting`, and `replicator`, and consumes the public `P2PServiceViews` type instead of the broader `UseP2PReplicatorResult`. Its separate `LiveSyncCore` argument and global `eventHub` remain the legacy pane bridge; this type narrowing leaves the existing presentation and lifecycle wiring intact. + The [TURN connection settings design](docs/design_docs/renewable_turn_credentials.md) describes how the host prepares temporary ICE credentials in a connection-only settings copy. It covers room reuse and expiry, replication continuation, profile persistence and sharing, and report redaction. ### Conflict Merge Policy diff --git a/docs/design_docs/service_feature_and_legacy_module_boundaries.md b/docs/design_docs/service_feature_and_legacy_module_boundaries.md index 65ec41ec..8dd3539d 100644 --- a/docs/design_docs/service_feature_and_legacy_module_boundaries.md +++ b/docs/design_docs/service_feature_and_legacy_module_boundaries.md @@ -1,7 +1,7 @@ --- -date: 2026-09-04 -commonlib-version: "0.1.21" -self-hosted-livesync-version: "1.0.24" +date: 2026-10-25 +commonlib-version: "0.2.0" +self-hosted-livesync-version: "1.0.40" status: accepted --- @@ -104,14 +104,22 @@ For new behaviour, keep Obsidian-specific presentation or registration in an Obs ### A small serviceFeature: language initialisation -`src/serviceFeatures/onLayoutReady/enablei18n.ts` declares only `setting`, `API`, and `appLifecycle`: +`src/serviceFeatures/onLayoutReady/enablei18n.ts` declares only `setting`, `API`, and `appLifecycle`, with an explicit dependency type: ```typescript -export const enableI18nFeature = createServiceFeature(async ({ services: { setting, API, appLifecycle } }) => { - // Apply the language, persist a change, and register unload clean-up. -}); +type EnableI18nHost = NecessaryServices<"setting" | "API" | "appLifecycle", never>; + +export const enableI18nFeature = createServiceFeature( + async (host: EnableI18nHost): Promise => { + const { setting, API, appLifecycle } = host.services; + // Apply the language, persist a change, and register unload clean-up. + return true; + } +); ``` +The host annotation keeps the dependency contract visible because `createServiceFeature()` does not infer dependency keys from the callback body. This feature retains its `Promise` result. + The local `ObsidianLanguageAppliedNotice` class is still appropriate. It owns one replaceable Obsidian `Notice` and has an explicit `clear()` lifetime operation. The class is not used as a service locator, and the serviceFeature owns its construction and host binding. ### Operation and composition: database preparation @@ -130,6 +138,22 @@ The operation does not need an application Module identity. ### Ordered start-up composition and registration-only features +Compatibility review uses distinct controller and composer dependencies: + +```typescript +type CompatibilityReviewDependencies = NecessaryServicesInterfaces< + "setting" | "config" | "vault" | "control", + never +>; + +type CompatibilityReviewHost = NecessaryServices< + "setting" | "config" | "vault" | "control" | "appLifecycle" | "API", + never +>; +``` + +The controller receives the four interface-only Services it uses. The composer adds `appLifecycle` and `API` for ordered lifecycle handlers and command registration; the priority-30 compatibility review remains after flag-file recovery. + Configured Vault admission and the checks which follow database preparation are composed by `src/serviceFeatures/startupLifecycle/`. The directory keeps onboarding admission, compromised-chunk inspection, incomplete-document repair, Config Doctor, and the obsolete bulk-send setting migration as separate operations. One feature composer owns their order and receives the compatibility-review wait operation explicitly; an individual operation does not call the composer. The layout-ready admission handler uses priority 1. This preserves ordinary priority-0 host integration before admission, while keeping an unconfigured Vault outside the flag-file recovery handlers at priorities 5, 10, and 20, and the compatibility review at priority 30. Admission belongs to one plug-in process: an initially unconfigured process remains inert until setup restarts it, and declining the requested restart does not trigger an in-process reconfiguration. Changing an admitted process back to unconfigured retires its Config Doctor and incomplete-document repair request handlers. The handlers also recheck the current configured state and database readiness when invoked, so a pending restart cannot expose partially initialised or retired state. The first-initialise handler rechecks admission before retaining the established order after the file watcher has been started: database readiness, compromised chunks, incomplete documents, compatibility review, Config Doctor, and the bulk-send setting migration. @@ -144,7 +168,19 @@ The state remains private to the composed feature. It does not become a `LiveSyn ### Implemented composition: conflict resolution -Conflict checking and resolution are composed for every host by `useConflictResolutionFeature`. The feature owns its `QueueProcessor` privately and registers the conflict Service handlers directly. Its operations receive explicit collaborators for settings, active-file state, database and storage access, replication, logging, and host events. No consumer locates a conflict Module or retains the queue. +Conflict checking and resolution are composed for every host by `useConflictResolutionFeature`. The feature owns its `QueueProcessor` privately and registers the conflict Service handlers directly. Its host type selects the required service and module contracts and adds the borrowed logger explicitly: + +```typescript +type ConflictResolutionHost = NecessaryServices< + "appLifecycle" | "conflict" | "database" | "replication" | "setting" | "vault", + "databaseFileAccess" | "fileHandler" | "storageAccess", + InjectableServiceHub +> & { + readonly log: LogFunction; +}; +``` + +The operations receive explicit collaborators for settings, active-file state, database and storage access, replication, a borrowed `LogFunction`, and host events. `LiveSyncBaseCore` supplies `log` using `createInstanceLogFunction("SF:ConflictResolution", this.services.API)`. Because feature composition precedes legacy Module binding, the host supplies the existing lazy service logger and does not read `logResource.value` before binding. `ModuleLog` and the host retain resource ownership. No consumer locates a conflict Module or retains the queue. The scheduling queue remains one state owner. It publishes `conflictProcessQueueCount`, coalesces pending checks for the same path, and makes `ensureAllProcessed()` wait for conflict resolution to finish. Repeated resolver invocations for one path retain only the newest waiting request and close an active comparison for that path before waiting for the per-file resolver, while comparisons for other paths remain open. Resolution remains host-neutral and communicates dialogue cancellation through `services.context.events`, so CLI, WebApp, and Obsidian compositions use their own selected event channel. diff --git a/src/LiveSyncBaseCore.ts b/src/LiveSyncBaseCore.ts index 9870a448..777bc164 100644 --- a/src/LiveSyncBaseCore.ts +++ b/src/LiveSyncBaseCore.ts @@ -31,6 +31,7 @@ import { useReplicationScheduling, type ReplicationSchedulingControl } from "./s import { createCentralReplicatorProviderDefinitions } from "./common/replicatorProviders"; import { useReplicationFeature } from "./serviceFeatures/replication"; import { useConflictResolutionFeature } from "./serviceFeatures/conflictResolution"; +import { createInstanceLogFunction } from "@vrtmrz/livesync-commonlib/compat/services/lib/logUtils"; import { useBasicCommandsFeature } from "./serviceFeatures/basicCommands"; /** Focused views returned by serviceFeatures which the host may consume during composition. */ @@ -290,7 +291,11 @@ export class LiveSyncBaseCore< * (Please refer `serviceFeatures` for more details) */ initialiseServiceFeatures(): LiveSyncCoreFeatureViews { - useConflictResolutionFeature(this); + useConflictResolutionFeature({ + services: this.services, + serviceModules: this.serviceModules, + log: createInstanceLogFunction("SF:ConflictResolution", this.services.API), + }); useTargetFilters(this); // enable target filter feature. usePrepareDatabaseForUse(this); diff --git a/src/modules/features/SettingDialogue/LiveSyncSetting.ts b/src/modules/features/SettingDialogue/LiveSyncSetting.ts index 19ff983e..455eebd6 100644 --- a/src/modules/features/SettingDialogue/LiveSyncSetting.ts +++ b/src/modules/features/SettingDialogue/LiveSyncSetting.ts @@ -14,7 +14,7 @@ import { statusDisplay, type ConfigurationItem, } from "@vrtmrz/livesync-commonlib/compat/common/types"; -import { type ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts"; +import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts"; import { type AllSettingItemKey, getConfig, @@ -24,7 +24,8 @@ import { type AllBooleanItemKey, } from "./settingConstants.ts"; import { $msg } from "@/common/translation"; -import { wrapMemo, type AutoWireOption, type OnUpdateResult } from "./SettingPane.ts"; +import type { AutoWireOption, OnUpdateResult } from "./SettingPane.ts"; +import { wrapMemo } from "octagonal-wheels/function"; import { setButtonDestructiveState } from "./settingComponentStyles.ts"; export class LiveSyncSetting extends Setting { @@ -109,9 +110,13 @@ export class LiveSyncSetting extends Setting { } LiveSyncSetting.env.requestUpdate(); } - autoWireText(key: AllStringItemKey, opt?: AutoWireOption) { + private autoWireStringComponent( + key: AllStringItemKey, + addComponent: (callback: (component: TComponent) => void) => void, + opt?: AutoWireOption + ) { const conf = this.autoWireSetting(key, opt); - this.addText((text) => { + addComponent((text) => { this.autoWiredComponent = text; const setValue = wrapMemo((value: string) => { text.setValue(value); @@ -127,26 +132,25 @@ export class LiveSyncSetting extends Setting { } this.autoWireComponent(this.autoWiredComponent, conf, opt); }); + } + autoWireText(key: AllStringItemKey, opt?: AutoWireOption) { + this.autoWireStringComponent( + key, + (callback) => { + this.addText(callback); + }, + opt + ); return this; } autoWireTextArea(key: AllStringItemKey, opt?: AutoWireOption) { - const conf = this.autoWireSetting(key, opt); - this.addTextArea((text) => { - this.autoWiredComponent = text; - const setValue = wrapMemo((value: string) => { - text.setValue(value); - }); - this.invalidateValue = () => setValue(`${LiveSyncSetting.env.editingSettings[key]}`); - this.invalidateValue(); - text.onChange(async (value) => { - await this.commitValue(value); - }); - if (opt?.isPassword) { - text.inputEl.setAttribute("type", "password"); - this.hasPassword = true; - } - this.autoWireComponent(this.autoWiredComponent, conf, opt); - }); + this.autoWireStringComponent( + key, + (callback) => { + this.addTextArea(callback); + }, + opt + ); return this; } autoWireNumeric( diff --git a/src/modules/features/SettingDialogue/LiveSyncSetting.wiring.unit.spec.ts b/src/modules/features/SettingDialogue/LiveSyncSetting.wiring.unit.spec.ts new file mode 100644 index 00000000..5335bc6d --- /dev/null +++ b/src/modules/features/SettingDialogue/LiveSyncSetting.wiring.unit.spec.ts @@ -0,0 +1,227 @@ +import { describe, expect, it, vi } from "vitest"; + +const liveSyncSettingHarness = vi.hoisted(() => ({ + components: [] as unknown[], + settingElements: [] as unknown[], +})); + +vi.mock("@/deps.ts", () => { + class StringComponent { + values: string[] = []; + changeHandler?: (value: string) => Promise; + placeholder?: string; + inputEl = { + attributes: new Map(), + setAttribute(name: string, value: string) { + this.attributes.set(name, value); + }, + toggleClass: (_name: string, _enabled: boolean) => {}, + }; + setValue(value: string) { + this.values.push(value); + } + onChange(handler: (value: string) => Promise) { + this.changeHandler = handler; + return this; + } + setPlaceholder(value: string) { + this.placeholder = value; + return this; + } + constructor() { + liveSyncSettingHarness.components.push(this); + } + } + class TextComponent extends StringComponent {} + class TextAreaComponent extends StringComponent {} + class Setting { + settingEl = { + classes: [] as { name: string; enabled: boolean }[], + toggleClass(name: string, enabled: boolean) { + this.classes.push({ name, enabled }); + }, + }; + constructor(_containerEl: HTMLElement) { + liveSyncSettingHarness.settingElements.push(this.settingEl); + } + setDesc(_description: string | DocumentFragment) { + return this; + } + setName(_name: string | DocumentFragment) { + return this; + } + addText(callback: (component: TextComponent) => void) { + callback(new TextComponent()); + return this; + } + addTextArea(callback: (component: TextAreaComponent) => void) { + callback(new TextAreaComponent()); + return this; + } + setTooltip(_description: string) { + return this; + } + setDisabled(_disabled: boolean) { + return this; + } + } + return { Setting, TextComponent, TextAreaComponent, ButtonComponent: class {} }; +}); +vi.mock("./ObsidianLiveSyncSettingTab.ts", () => ({})); +vi.mock("@/common/translation", () => ({ $msg: (key: string) => key })); +vi.mock("@vrtmrz/livesync-commonlib/compat/common/types", () => ({ + LEVEL_ADVANCED: 1, + LEVEL_POWER_USER: 2, + statusDisplay: () => "", +})); +vi.mock("./settingConstants.ts", () => ({ + getConfig: (key: string) => ({ + name: key, + desc: `description for ${key}`, + placeHolder: `configured placeholder for ${key}`, + }), +})); +vi.mock("./settingComponentStyles.ts", () => ({ setButtonDestructiveState: vi.fn() })); +vi.mock("octagonal-wheels/collection", () => ({ unique: (items: unknown[]) => [...new Set(items)] })); + +import { LiveSyncSetting } from "./LiveSyncSetting.ts"; + +type WireKind = "text" | "textarea"; +type StringComponentHarness = { + values: string[]; + changeHandler?: (value: string) => Promise; + placeholder?: string; + inputEl: { attributes: Map }; +}; +type SettingElementHarness = { classes: { name: string; enabled: boolean }[] }; + +function deferred() { + let resolve!: () => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + +function wireStringSetting( + kind: WireKind, + options: { + holdValue?: boolean; + isPassword?: boolean; + placeHolder?: string; + onUpdate?: () => { visibility?: boolean }; + } = {} +) { + const key = kind === "text" ? "syncOnlyRegEx" : "ignoreFiles"; + const events: string[] = []; + const editingSettings: Record = { [key]: 17 }; + const saveSettings = vi.fn(async (_keys: string[]) => {}); + const requestUpdate = vi.fn(() => events.push("update")); + const env = { + settingComponents: [] as unknown[], + editingSettings, + saveSettings, + requestUpdate, + }; + LiveSyncSetting.env = env as never; + + const setting = new LiveSyncSetting({} as HTMLElement); + const onUpdate = vi.fn(options.onUpdate ?? (() => ({ visibility: false }))); + const wireOptions = { ...options, onUpdate }; + if (kind === "text") { + setting.autoWireText(key as never, wireOptions); + } else { + setting.autoWireTextArea(key as never, wireOptions); + } + + return { + key, + events, + editingSettings, + saveSettings, + requestUpdate, + setting, + component: setting.autoWiredComponent as unknown as StringComponentHarness, + settingElement: setting.settingEl as unknown as SettingElementHarness, + onUpdate, + }; +} + +describe("LiveSyncSetting string controls", () => { + it.each(["text", "textarea"] as const)("wires %s through the shared string commit contract", async (kind) => { + const { + key, + events, + editingSettings, + saveSettings, + requestUpdate, + setting, + component, + settingElement, + onUpdate, + } = wireStringSetting(kind, { isPassword: true, placeHolder: "option placeholder" }); + + expect(component.values).toEqual(["17"]); + setting.invalidateValue?.(); + expect(component.values).toEqual(["17"]); + editingSettings[key] = 18; + setting.invalidateValue?.(); + expect(component.values).toEqual(["17", "18"]); + expect(component.inputEl.attributes.get("type")).toBe("password"); + expect(setting.hasPassword).toBe(true); + expect(setting.updateHandlers.size).toBe(1); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(setting._getComputedStatus()).toEqual({ visibility: false }); + expect(onUpdate).toHaveBeenCalledTimes(2); + expect(settingElement.classes).toContainEqual({ name: "sls-setting-hidden", enabled: true }); + if (kind === "text") { + expect(component.placeholder).toBe("configured placeholder for syncOnlyRegEx"); + } else { + expect(component.placeholder).toBeUndefined(); + } + + const pendingSave = deferred(); + saveSettings.mockImplementationOnce(async (keys) => { + events.push(`save:${keys.join(",")}`); + await pendingSave.promise; + events.push("saved"); + }); + const commit = component.changeHandler!("changed"); + expect(editingSettings[key]).toBe("changed"); + expect(saveSettings).toHaveBeenCalledWith([key]); + expect(events).toEqual([`save:${key}`]); + expect(requestUpdate).not.toHaveBeenCalled(); + pendingSave.resolve(); + await commit; + expect(events).toEqual([`save:${key}`, "saved", "update"]); + + events.length = 0; + await component.changeHandler!("changed"); + expect(saveSettings).toHaveBeenCalledTimes(1); + expect(events).toEqual(["update"]); + + events.length = 0; + const failure = new Error("setting save failed"); + saveSettings.mockImplementationOnce(async (keys) => { + events.push(`save:${keys.join(",")}`); + throw failure; + }); + await expect(component.changeHandler!("failed")).rejects.toBe(failure); + expect(editingSettings[key]).toBe("failed"); + expect(events).toEqual([`save:${key}`]); + expect(requestUpdate).toHaveBeenCalledTimes(2); + }); + + it.each(["text", "textarea"] as const)("keeps %s held values in the editing copy without saving", async (kind) => { + const { key, editingSettings, saveSettings, requestUpdate, component, events } = wireStringSetting(kind, { + holdValue: true, + }); + + await component.changeHandler!("held value"); + + expect(editingSettings[key]).toBe("held value"); + expect(saveSettings).not.toHaveBeenCalled(); + expect(requestUpdate).toHaveBeenCalledOnce(); + expect(events).toEqual(["update"]); + }); +}); diff --git a/src/modules/features/SettingDialogue/PaneSelector.ts b/src/modules/features/SettingDialogue/PaneSelector.ts index 783d6e99..77a98c0a 100644 --- a/src/modules/features/SettingDialogue/PaneSelector.ts +++ b/src/modules/features/SettingDialogue/PaneSelector.ts @@ -1,4 +1,8 @@ -import { LEVEL_ADVANCED, type CustomRegExpSource } from "@vrtmrz/livesync-commonlib/compat/common/types"; +import { + LEVEL_ADVANCED, + type CustomRegExpSource, + type CustomRegExpSourceList, +} from "@vrtmrz/livesync-commonlib/compat/common/types"; import { constructCustomRegExpList, splitCustomRegExpList } from "@vrtmrz/livesync-commonlib/compat/common/utils"; import MultipleRegExpControl from "./MultipleRegExpControl.svelte"; import { LiveSyncSetting as Setting } from "./LiveSyncSetting.ts"; @@ -7,6 +11,35 @@ import type { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab.ts import type { PageFunctions } from "./SettingPane.ts"; import { visibleOnly } from "./SettingPane.ts"; import { $msg } from "@/common/translation"; + +type RegExpControlOwner = Pick< + ObsidianLiveSyncSettingTab, + "saveAllDirtySettings" | "requestPageRefresh" | "lifetimeComponent" +>; + +function mountMultipleRegExpControl( + owner: RegExpControlOwner, + target: HTMLElement, + source: CustomRegExpSourceList, + delimiter: D, + assign: (serializedPatterns: CustomRegExpSourceList) => void +): void { + const patterns = splitCustomRegExpList(source, delimiter); + const control = mount(MultipleRegExpControl, { + target, + props: { + patterns, + originals: [...patterns], + apply: async (newPatterns: CustomRegExpSource[]) => { + assign(constructCustomRegExpList(newPatterns, delimiter)); + await owner.saveAllDirtySettings(); + owner.requestPageRefresh(); + }, + }, + }); + owner.lifetimeComponent.register(() => void unmount(control)); +} + export function paneSelector(this: ObsidianLiveSyncSettingTab, paneEl: HTMLElement, { addPanel }: PageFunctions): void { void addPanel(paneEl, $msg("Normal Files")).then((paneEl) => { const syncFilesSetting = new Setting(paneEl) @@ -16,37 +49,29 @@ export function paneSelector(this: ObsidianLiveSyncSettingTab, paneEl: HTMLEleme "(RegExp) Limit normal files to matching paths. Leave empty to allow all normal file paths. Hidden File Sync uses its own target patterns." ) ); - const syncFilesControl = mount(MultipleRegExpControl, { - target: syncFilesSetting.controlEl, - props: { - patterns: splitCustomRegExpList(this.editingSettings.syncOnlyRegEx, "|[]|"), - originals: splitCustomRegExpList(this.editingSettings.syncOnlyRegEx, "|[]|"), - apply: async (newPatterns: CustomRegExpSource[]) => { - this.editingSettings.syncOnlyRegEx = constructCustomRegExpList(newPatterns, "|[]|"); - await this.saveAllDirtySettings(); - this.requestPageRefresh(); - }, - }, - }); - this.lifetimeComponent.register(() => void unmount(syncFilesControl)); + mountMultipleRegExpControl( + this, + syncFilesSetting.controlEl, + this.editingSettings.syncOnlyRegEx, + "|[]|", + (value) => { + this.editingSettings.syncOnlyRegEx = value; + } + ); const nonSyncFilesSetting = new Setting(paneEl) .setName($msg("Non-Synchronising files")) .setDesc($msg("(RegExp) Skip changes to matching normal and hidden files, locally and remotely.")); - const nonSyncFilesControl = mount(MultipleRegExpControl, { - target: nonSyncFilesSetting.controlEl, - props: { - patterns: splitCustomRegExpList(this.editingSettings.syncIgnoreRegEx, "|[]|"), - originals: splitCustomRegExpList(this.editingSettings.syncIgnoreRegEx, "|[]|"), - apply: async (newPatterns: CustomRegExpSource[]) => { - this.editingSettings.syncIgnoreRegEx = constructCustomRegExpList(newPatterns, "|[]|"); - await this.saveAllDirtySettings(); - this.requestPageRefresh(); - }, - }, - }); - this.lifetimeComponent.register(() => void unmount(nonSyncFilesControl)); + mountMultipleRegExpControl( + this, + nonSyncFilesSetting.controlEl, + this.editingSettings.syncIgnoreRegEx, + "|[]|", + (value) => { + this.editingSettings.syncIgnoreRegEx = value; + } + ); new Setting(paneEl).autoWireNumeric("syncMaxSizeInMB", { clampMin: 0 }); new Setting(paneEl).autoWireToggle("useIgnoreFiles"); @@ -58,41 +83,30 @@ export function paneSelector(this: ObsidianLiveSyncSettingTab, paneEl: HTMLEleme const targetPatternSetting = new Setting(paneEl) .setName($msg("Target patterns")) .setDesc($msg("Patterns to match files for syncing")); - const patTarget = splitCustomRegExpList(this.editingSettings.syncInternalFilesTargetPatterns, ","); - const targetPatternControl = mount(MultipleRegExpControl, { - target: targetPatternSetting.controlEl, - props: { - patterns: patTarget, - originals: [...patTarget], - apply: async (newPatterns: CustomRegExpSource[]) => { - this.editingSettings.syncInternalFilesTargetPatterns = constructCustomRegExpList(newPatterns, ","); - await this.saveAllDirtySettings(); - this.requestPageRefresh(); - }, - }, - }); - this.lifetimeComponent.register(() => void unmount(targetPatternControl)); + mountMultipleRegExpControl( + this, + targetPatternSetting.controlEl, + this.editingSettings.syncInternalFilesTargetPatterns, + ",", + (value) => { + this.editingSettings.syncInternalFilesTargetPatterns = value; + } + ); const defaultSkipPattern = "\\/node_modules\\/, \\/\\.git\\/, ^\\.git\\/, \\/obsidian-livesync\\/"; const defaultSkipPatternXPlat = defaultSkipPattern + ",\\/workspace$ ,\\/workspace.json$,\\/workspace-mobile.json$"; - const pat = splitCustomRegExpList(this.editingSettings.syncInternalFilesIgnorePatterns, ","); const patSetting = new Setting(paneEl).setName($msg("Ignore patterns")).setDesc(""); - - const ignorePatternControl = mount(MultipleRegExpControl, { - target: patSetting.controlEl, - props: { - patterns: pat, - originals: [...pat], - apply: async (newPatterns: CustomRegExpSource[]) => { - this.editingSettings.syncInternalFilesIgnorePatterns = constructCustomRegExpList(newPatterns, ","); - await this.saveAllDirtySettings(); - this.requestPageRefresh(); - }, - }, - }); - this.lifetimeComponent.register(() => void unmount(ignorePatternControl)); + mountMultipleRegExpControl( + this, + patSetting.controlEl, + this.editingSettings.syncInternalFilesIgnorePatterns, + ",", + (value) => { + this.editingSettings.syncInternalFilesIgnorePatterns = value; + } + ); const addDefaultPatterns = async (patterns: string) => { const oldList = splitCustomRegExpList(this.editingSettings.syncInternalFilesIgnorePatterns, ","); @@ -122,22 +136,14 @@ export function paneSelector(this: ObsidianLiveSyncSettingTab, paneEl: HTMLEleme const overwritePatterns = new Setting(paneEl) .setName($msg("Overwrite patterns")) .setDesc($msg("Patterns to match files for overwriting instead of merging")); - const patTarget2 = splitCustomRegExpList(this.editingSettings.syncInternalFileOverwritePatterns, ","); - const overwritePatternControl = mount(MultipleRegExpControl, { - target: overwritePatterns.controlEl, - props: { - patterns: patTarget2, - originals: [...patTarget2], - apply: async (newPatterns: CustomRegExpSource[]) => { - this.editingSettings.syncInternalFileOverwritePatterns = constructCustomRegExpList( - newPatterns, - "," - ); - await this.saveAllDirtySettings(); - this.requestPageRefresh(); - }, - }, - }); - this.lifetimeComponent.register(() => void unmount(overwritePatternControl)); + mountMultipleRegExpControl( + this, + overwritePatterns.controlEl, + this.editingSettings.syncInternalFileOverwritePatterns, + ",", + (value) => { + this.editingSettings.syncInternalFileOverwritePatterns = value; + } + ); }); } diff --git a/src/modules/features/SettingDialogue/PaneSelector.unit.spec.ts b/src/modules/features/SettingDialogue/PaneSelector.unit.spec.ts new file mode 100644 index 00000000..a53efdf2 --- /dev/null +++ b/src/modules/features/SettingDialogue/PaneSelector.unit.spec.ts @@ -0,0 +1,229 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const paneSelectorHarness = vi.hoisted(() => ({ + mounts: [] as { component: unknown; target: unknown; props: Record; handle: unknown }[], + unmounted: [] as unknown[], + settings: [] as { containerEl: HTMLElement; controlEl: unknown }[], + buttons: [] as { text: string; handler?: () => void | Promise }[], +})); + +vi.mock("@/common/translation", () => ({ $msg: (value: string) => value })); +vi.mock("./MultipleRegExpControl.svelte", () => ({ default: { name: "MultipleRegExpControl" } })); +vi.mock("svelte", () => ({ + mount: vi.fn((component: unknown, options: { target: unknown; props: Record }) => { + const handle = { id: paneSelectorHarness.mounts.length }; + paneSelectorHarness.mounts.push({ component, target: options.target, props: options.props, handle }); + return handle; + }), + unmount: vi.fn((component: unknown) => { + paneSelectorHarness.unmounted.push(component); + }), +})); +vi.mock("./LiveSyncSetting.ts", () => ({ + LiveSyncSetting: class { + controlEl: { id: number }; + constructor(containerEl: HTMLElement) { + this.controlEl = { id: paneSelectorHarness.settings.length }; + paneSelectorHarness.settings.push({ containerEl, controlEl: this.controlEl }); + } + setName(_name: string) { + return this; + } + setDesc(_description: string) { + return this; + } + autoWireNumeric(_key: string, _options: unknown) { + return this; + } + autoWireToggle(_key: string) { + return this; + } + autoWireTextArea(_key: string, _options: unknown) { + return this; + } + addButton( + callback: (button: { + setButtonText: (text: string) => unknown; + onClick: (handler: () => void | Promise) => unknown; + }) => void + ) { + const button = { + text: "", + handler: undefined as (() => void | Promise) | undefined, + setButtonText(text: string) { + this.text = text; + return this; + }, + onClick(handler: () => void | Promise) { + this.handler = handler; + paneSelectorHarness.buttons.push(this); + return this; + }, + }; + callback(button); + return this; + } + }, +})); + +import { paneSelector } from "./PaneSelector.ts"; + +type PatternControlProps = { + patterns: string[]; + originals: string[]; + apply: (patterns: string[]) => Promise; +}; + +function deferred() { + let resolve!: () => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + +async function renderSelector() { + const normalPanel = { id: "normal" } as unknown as HTMLElement; + const hiddenPanel = { id: "hidden" } as unknown as HTMLElement; + const addPanel = vi.fn((_parent: HTMLElement, title: string) => + Promise.resolve(title === "Normal Files" ? normalPanel : hiddenPanel) + ); + const disposers: (() => void)[] = []; + const tab = { + editingSettings: { + syncOnlyRegEx: "alpha|[]|beta", + syncIgnoreRegEx: "skip|[]|also-skip", + syncInternalFilesTargetPatterns: "target-a,target-b", + syncInternalFilesIgnorePatterns: "existing,\\/node_modules\\/", + syncInternalFileOverwritePatterns: "overwrite-a,overwrite-b", + }, + saveAllDirtySettings: vi.fn(async () => {}), + requestPageRefresh: vi.fn(), + lifetimeComponent: { register: (dispose: () => void) => disposers.push(dispose) }, + }; + + paneSelector.call(tab as never, {} as HTMLElement, { addPanel } as never); + await vi.waitFor(() => expect(paneSelectorHarness.mounts).toHaveLength(5)); + return { tab, addPanel, disposers }; +} + +function controlProps(index: number): PatternControlProps { + return paneSelectorHarness.mounts[index]!.props as unknown as PatternControlProps; +} + +beforeEach(() => { + paneSelectorHarness.mounts.length = 0; + paneSelectorHarness.unmounted.length = 0; + paneSelectorHarness.settings.length = 0; + paneSelectorHarness.buttons.length = 0; + vi.clearAllMocks(); +}); + +describe("paneSelector multiple-pattern controls", () => { + it("keeps all five bindings, independent originals, delimiter choices, save ordering, and owned cleanup", async () => { + const { tab, addPanel, disposers } = await renderSelector(); + expect(addPanel.mock.calls.map(([, title]) => title)).toEqual(["Normal Files", "Hidden Files"]); + expect(paneSelectorHarness.mounts.map(({ props }) => (props as PatternControlProps).patterns)).toEqual([ + ["alpha", "beta"], + ["skip", "also-skip"], + ["target-a", "target-b"], + ["existing", "\\/node_modules\\/"], + ["overwrite-a", "overwrite-b"], + ]); + + for (let index = 0; index < paneSelectorHarness.mounts.length; index++) { + const { patterns, originals } = controlProps(index); + expect(originals).toEqual(patterns); + expect(originals).not.toBe(patterns); + patterns.push("edited-current-value"); + expect(originals).not.toContain("edited-current-value"); + } + + const cases = [ + { + index: 0, + key: "syncOnlyRegEx", + patterns: ["new-normal-a", "new-normal-b"], + value: "new-normal-a|[]|new-normal-b", + }, + { + index: 1, + key: "syncIgnoreRegEx", + patterns: ["new-skip-a", "new-skip-b"], + value: "new-skip-a|[]|new-skip-b", + }, + { + index: 2, + key: "syncInternalFilesTargetPatterns", + patterns: ["new-target-a", "new-target-b"], + value: "new-target-a,new-target-b", + }, + { + index: 3, + key: "syncInternalFilesIgnorePatterns", + patterns: ["new-ignore-a", "new-ignore-b"], + value: "new-ignore-a,new-ignore-b", + }, + { + index: 4, + key: "syncInternalFileOverwritePatterns", + patterns: ["new-overwrite-a", "new-overwrite-b"], + value: "new-overwrite-a,new-overwrite-b", + }, + ] as const; + const events: string[] = []; + for (let index = 0; index < cases.length; index++) { + const item = cases[index]!; + const pendingSave = deferred(); + events.length = 0; + tab.saveAllDirtySettings.mockImplementationOnce(async () => { + events.push(`save:${item.key}`); + await pendingSave.promise; + events.push("saved"); + }); + tab.requestPageRefresh.mockImplementationOnce(() => { + events.push("refresh"); + }); + + const applying = controlProps(item.index).apply([...item.patterns]); + expect(tab.editingSettings[item.key as keyof typeof tab.editingSettings]).toBe(item.value); + expect(tab.saveAllDirtySettings).toHaveBeenCalledTimes(index + 1); + expect(events).toEqual([`save:${item.key}`]); + expect(tab.requestPageRefresh).toHaveBeenCalledTimes(index); + + pendingSave.resolve(); + await applying; + expect(events).toEqual([`save:${item.key}`, "saved", "refresh"]); + expect(tab.requestPageRefresh).toHaveBeenCalledTimes(index + 1); + } + + const failure = new Error("settings save failed"); + tab.saveAllDirtySettings.mockRejectedValueOnce(failure); + await expect(controlProps(2).apply(["assigned-before-failure"])).rejects.toBe(failure); + expect(tab.editingSettings.syncInternalFilesTargetPatterns).toBe("assigned-before-failure"); + expect(tab.requestPageRefresh).toHaveBeenCalledTimes(cases.length); + + expect(disposers).toHaveLength(5); + disposers.forEach((dispose) => dispose()); + expect(paneSelectorHarness.unmounted).toEqual(paneSelectorHarness.mounts.map(({ handle }) => handle)); + }); + + it("merges default hidden-file ignores with existing patterns and removes duplicates", async () => { + const { tab } = await renderSelector(); + const defaultButton = paneSelectorHarness.buttons.find(({ text }) => text === "Default"); + const crossPlatformButton = paneSelectorHarness.buttons.find(({ text }) => text === "Cross-platform"); + expect(defaultButton?.handler).toBeDefined(); + expect(crossPlatformButton?.handler).toBeDefined(); + + await defaultButton!.handler!(); + expect(tab.editingSettings.syncInternalFilesIgnorePatterns).toBe( + "existing,\\/node_modules\\/, \\/\\.git\\/, ^\\.git\\/, \\/obsidian-livesync\\/" + ); + await crossPlatformButton!.handler!(); + expect(tab.editingSettings.syncInternalFilesIgnorePatterns).toBe( + "existing,\\/node_modules\\/, \\/\\.git\\/, ^\\.git\\/, \\/obsidian-livesync\\/,\\/workspace$ ,\\/workspace.json$,\\/workspace-mobile.json$" + ); + expect(tab.saveAllDirtySettings).toHaveBeenCalledTimes(2); + expect(tab.requestPageRefresh).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/modules/features/SettingDialogue/SettingPane.ts b/src/modules/features/SettingDialogue/SettingPane.ts index 442c2f93..57bd447c 100644 --- a/src/modules/features/SettingDialogue/SettingPane.ts +++ b/src/modules/features/SettingDialogue/SettingPane.ts @@ -97,16 +97,6 @@ export function findAttrFromParent(el: HTMLElement, attr: string): string { return ""; } -export function wrapMemo(func: (arg: T) => void) { - let buf: T | undefined = undefined; - return (arg: T) => { - if (buf !== arg) { - func(arg); - buf = arg; - } - }; -} - /** * Defers pane construction until the owning settings page can confirm that its * render scope is still active. diff --git a/src/serviceFeatures/basicCommands.ts b/src/serviceFeatures/basicCommands.ts index 2bd622e5..8c6c3095 100644 --- a/src/serviceFeatures/basicCommands.ts +++ b/src/serviceFeatures/basicCommands.ts @@ -1,3 +1,4 @@ +import { createCommandCheck } from "@vrtmrz/obsidian-plugin-kit/command"; import { LOG_LEVEL_NOTICE } from "octagonal-wheels/common/logger"; import { fireAndForget } from "octagonal-wheels/promises"; import type { NecessaryServices } from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; @@ -69,14 +70,12 @@ export function useBasicCommandsFeature(host: BasicCommandsHost): void { services.API.addCommand({ id: "livesync-dump", name: $msg("Copy database information for the active file"), - checkCallback: (checking) => { - const file = services.vault.getActiveFilePath(); - if (!file) return false; - if (!checking) { + checkCallback: createCommandCheck( + () => services.vault.getActiveFilePath(), + (file) => { fireAndForget(() => copyFileDatabaseInfo(createFileDatabaseInfoCore(host), file)); } - return true; - }, + ), }); services.API.addCommand({ @@ -115,13 +114,12 @@ export function useBasicCommandsFeature(host: BasicCommandsHost): void { services.API.addCommand({ id: "livesync-scan-files", name: "Scan storage and database again", - checkCallback: (checking) => { - if (!services.setting.currentSettings().useAdvancedMode) return false; - if (!checking) { + checkCallback: createCommandCheck( + () => services.setting.currentSettings().useAdvancedMode, + () => { fireAndForget(() => services.vault.scanVault(true)); } - return true; - }, + ), }); services.API.addCommand({ @@ -135,13 +133,12 @@ export function useBasicCommandsFeature(host: BasicCommandsHost): void { services.API.addCommand({ id: "livesync-abortsync", name: "Abort synchronization immediately", - checkCallback: (checking) => { - if (!services.setting.currentSettings().useAdvancedMode) return false; - if (!checking) { + checkCallback: createCommandCheck( + () => services.setting.currentSettings().useAdvancedMode, + () => { fireAndForget(() => services.replication.stopActiveTransfer()); } - return true; - }, + ), }); return Promise.resolve(true); diff --git a/src/serviceFeatures/basicCommands.unit.spec.ts b/src/serviceFeatures/basicCommands.unit.spec.ts index 356caa2a..0e10adf0 100644 --- a/src/serviceFeatures/basicCommands.unit.spec.ts +++ b/src/serviceFeatures/basicCommands.unit.spec.ts @@ -228,6 +228,49 @@ describe("useBasicCommandsFeature", () => { expect(adapter).not.toBe(fixture.host); }); + it("rechecks availability on execution and captures the current file only once", async () => { + const fixture = createFixture(); + await initialise(fixture); + vi.mocked(copyFileDatabaseInfo).mockClear(); + const dump = fixture.getCommand("livesync-dump"); + expect(dump.checkCallback?.(true)).toBe(true); + fixture.services.vault.getActiveFilePath.mockReturnValue(undefined); + expect(dump.checkCallback?.(false)).toBe(false); + expect(copyFileDatabaseInfo).not.toHaveBeenCalled(); + + fixture.services.vault.getActiveFilePath + .mockReset() + .mockReturnValueOnce("selected.md") + .mockReturnValue("later.md"); + expect(dump.checkCallback?.(false)).toBe(true); + await vi.waitFor(() => expect(copyFileDatabaseInfo).toHaveBeenCalledOnce()); + expect(fixture.services.vault.getActiveFilePath).toHaveBeenCalledOnce(); + expect(vi.mocked(copyFileDatabaseInfo).mock.calls[0][1]).toBe("selected.md"); + + fixture.settings.useAdvancedMode = true; + for (const id of ["livesync-scan-files", "livesync-abortsync"]) { + expect(fixture.getCommand(id).checkCallback?.(true)).toBe(true); + } + fixture.settings.useAdvancedMode = false; + for (const id of ["livesync-scan-files", "livesync-abortsync"]) { + expect(fixture.getCommand(id).checkCallback?.(false)).toBe(false); + } + expect(fixture.services.vault.scanVault).not.toHaveBeenCalled(); + expect(fixture.services.replication.stopActiveTransfer).not.toHaveBeenCalled(); + }); + + it("propagates an availability failure without running the command", async () => { + const fixture = createFixture(); + await initialise(fixture); + vi.mocked(copyFileDatabaseInfo).mockClear(); + const failure = new Error("Vault unavailable"); + fixture.services.vault.getActiveFilePath.mockImplementation(() => { + throw failure; + }); + expect(() => fixture.getCommand("livesync-dump").checkCallback?.(false)).toThrow(failure); + expect(copyFileDatabaseInfo).not.toHaveBeenCalled(); + }); + it("commits pending file events from the batch command", async () => { const fixture = createFixture(); await initialise(fixture); diff --git a/src/serviceFeatures/compatibilityReview.ts b/src/serviceFeatures/compatibilityReview.ts index 7c79f98f..bdc7b90a 100644 --- a/src/serviceFeatures/compatibilityReview.ts +++ b/src/serviceFeatures/compatibilityReview.ts @@ -1,6 +1,9 @@ import { fireAndForget } from "octagonal-wheels/promises"; import { VER } from "@vrtmrz/livesync-commonlib/compat/common/types"; -import type { LiveSyncCore } from "@/main.ts"; +import type { + NecessaryServices, + NecessaryServicesInterfaces, +} from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; import { COMPATIBILITY_PAUSE_SETTING_MESSAGE, DATABASE_COMPATIBILITY_VERSION_KEY, @@ -24,6 +27,16 @@ export interface CompatibilityReviewUi { clearReminder(): void; } +export type CompatibilityReviewDependencies = NecessaryServicesInterfaces< + "setting" | "config" | "vault" | "control", + never +>; + +export type CompatibilityReviewHost = NecessaryServices< + "setting" | "config" | "vault" | "control" | "appLifecycle" | "API", + never +>; + export class CompatibilityReviewController { private pause: CompatibilityPause | undefined; private activeReview: Promise | undefined; @@ -31,7 +44,7 @@ export class CompatibilityReviewController { private disposed = false; constructor( - private readonly core: LiveSyncCore, + private readonly core: CompatibilityReviewDependencies, private readonly ui: CompatibilityReviewUi, private readonly currentVersion: number = VER ) {} @@ -161,7 +174,10 @@ export class CompatibilityReviewController { } } -export function useCompatibilityReview(core: LiveSyncCore, ui: CompatibilityReviewUi): CompatibilityReviewController { +export function useCompatibilityReview( + core: CompatibilityReviewHost, + ui: CompatibilityReviewUi +): CompatibilityReviewController { const controller = new CompatibilityReviewController(core, ui); core.services.appLifecycle.onSettingLoaded.addHandler(() => controller.initialise()); core.services.appLifecycle.onLayoutReady.addHandler(() => { diff --git a/src/serviceFeatures/compatibilityReview.unit.spec.ts b/src/serviceFeatures/compatibilityReview.unit.spec.ts index 44a1b05b..61a508f0 100644 --- a/src/serviceFeatures/compatibilityReview.unit.spec.ts +++ b/src/serviceFeatures/compatibilityReview.unit.spec.ts @@ -1,4 +1,8 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, expectTypeOf, it, vi } from "vitest"; +import type { + NecessaryServices, + NecessaryServicesInterfaces, +} from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; import { COMPATIBILITY_PAUSE_SETTING_MESSAGE, DATABASE_COMPATIBILITY_VERSION_KEY, @@ -101,6 +105,32 @@ function createFixture( } describe("compatibility review controller", () => { + it("accepts operation services without retaining an application-core contract", () => { + type Dependencies = NecessaryServicesInterfaces<"setting" | "config" | "vault" | "control", never>; + type ControllerHost = ConstructorParameters[0]; + + expectTypeOf().toExtend(); + expectTypeOf().toEqualTypeOf<"services" | "serviceModules">(); + expectTypeOf().toEqualTypeOf<"setting" | "config" | "vault" | "control">(); + expectTypeOf().toBeNever(); + }); + + it("composes the review from its operation services and lifecycle and command ports", () => { + type Dependencies = NecessaryServices< + "setting" | "config" | "vault" | "control" | "appLifecycle" | "API", + never + >; + type FeatureHost = Parameters[0]; + + expectTypeOf().toExtend(); + expectTypeOf().toEqualTypeOf<"services" | "serviceModules">(); + expectTypeOf().toEqualTypeOf< + "context" | "setting" | "config" | "vault" | "control" | "appLifecycle" | "API" + >(); + expectTypeOf().toBeNever(); + expectTypeOf>().toEqualTypeOf(); + }); + beforeEach(() => { vi.restoreAllMocks(); }); diff --git a/src/serviceFeatures/conflictResolution/conflictResolution.unit.spec.ts b/src/serviceFeatures/conflictResolution/conflictResolution.unit.spec.ts index a3f618d9..b0aa10cf 100644 --- a/src/serviceFeatures/conflictResolution/conflictResolution.unit.spec.ts +++ b/src/serviceFeatures/conflictResolution/conflictResolution.unit.spec.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, expectTypeOf, it, vi } from "vitest"; import { createServiceContext } from "@vrtmrz/livesync-commonlib/context"; import { InjectableConflictService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableConflictService"; import { @@ -15,6 +15,12 @@ import { } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { EVENT_CONFLICT_CANCELLED } from "@vrtmrz/livesync-commonlib/compat/events/coreEvents"; import { NO_INTERACTION } from "@vrtmrz/livesync-commonlib/replication"; +import type { IAPIService } from "@vrtmrz/livesync-commonlib/compat/services/base/IService"; +import { + createInstanceLogFunction, + MARK_LOG_SEPARATOR, + type LogFunction, +} from "@vrtmrz/livesync-commonlib/compat/services/lib/logUtils"; import type { ConflictResolutionHost } from "./index"; import { createConflictResolutionOperations, useConflictResolutionFeature } from "./index"; import type { ConflictResolutionOperationsDependencies } from "./operations"; @@ -64,13 +70,15 @@ function createHarness(options: HarnessOptions = {}) { vault: { getActiveFilePath: activeFile }, }; const serviceModules = { databaseFileAccess, fileHandler, storageAccess }; + const log = createInstanceLogFunction("SF:ConflictResolution", services.API as unknown as IAPIService); if (options.compose !== false) { - useConflictResolutionFeature({ services, serviceModules } as unknown as ConflictResolutionHost); + useConflictResolutionFeature({ services, serviceModules, log } as unknown as ConflictResolutionHost); } return { addLog, + log, conflict, context, databaseFileAccess, @@ -151,6 +159,87 @@ function createOperationsHarness() { } describe("conflict resolution serviceFeature", () => { + it("declares a borrowed logger and only the services used by conflict resolution", () => { + expectTypeOf().toEqualTypeOf< + "context" | "appLifecycle" | "conflict" | "database" | "replication" | "setting" | "vault" + >(); + expectTypeOf().toEqualTypeOf(); + }); + + it("uses the injected logger for the checker and resolver without an API service", async () => { + const harness = createHarness({ + compose: false, + settings: { checkConflictOnlyOnOpen: true }, + activeFile: "active.md" as FilePathWithPrefix, + }); + const { API, ...services } = harness.services; + const log = vi.fn(); + useConflictResolutionFeature({ + services, + serviceModules: harness.serviceModules, + log, + } as unknown as ConflictResolutionHost); + harness.databaseFileAccess.fetchEntryMeta.mockResolvedValue(false); + + await harness.conflict.queueCheckForIfOpen("other.md" as FilePathWithPrefix); + await harness.conflict.resolveByNewest("missing.md" as FilePathWithPrefix); + + expect(log).toHaveBeenNthCalledWith( + 1, + "other.md is conflicted, merging process has been postponed.", + LOG_LEVEL_NOTICE + ); + expect(log).toHaveBeenNthCalledWith(2, "Could not get current revision of missing.md"); + expect(API.addLog).not.toHaveBeenCalled(); + }); + + it("keeps separately selected loggers isolated when legacy API services are shared", async () => { + const first = createHarness({ compose: false }); + const second = createHarness({ compose: false }); + const firstLog = vi.fn(); + const secondLog = vi.fn(); + const sharedAPI = first.services.API; + useConflictResolutionFeature({ + services: first.services, + serviceModules: first.serviceModules, + log: firstLog, + } as unknown as ConflictResolutionHost); + useConflictResolutionFeature({ + services: { ...second.services, API: sharedAPI }, + serviceModules: second.serviceModules, + log: secondLog, + } as unknown as ConflictResolutionHost); + first.databaseFileAccess.fetchEntryMeta.mockResolvedValue(false); + second.databaseFileAccess.fetchEntryMeta.mockResolvedValue(false); + + await first.conflict.resolveByNewest("first.md" as FilePathWithPrefix); + await second.conflict.resolveByNewest("second.md" as FilePathWithPrefix); + + expect(firstLog).toHaveBeenCalledExactlyOnceWith("Could not get current revision of first.md"); + expect(secondLog).toHaveBeenCalledExactlyOnceWith("Could not get current revision of second.md"); + expect(sharedAPI.addLog).not.toHaveBeenCalled(); + }); + + it("retains the prefix, level, and key selected by the host logger", async () => { + const harness = createHarness({ compose: false }); + const { API, ...services } = harness.services; + useConflictResolutionFeature({ + services, + serviceModules: harness.serviceModules, + log: harness.log, + } as unknown as ConflictResolutionHost); + + await harness.conflict.resolveByDeletingRevision("same.md" as FilePathWithPrefix, "1-old", "same"); + await harness.conflict.resolveAllConflictedFilesByNewerOnes(); + + expect(API.addLog).toHaveBeenCalledWith( + `[SF:ConflictResolution]${MARK_LOG_SEPARATOR} same.md has been merged automatically`, + LOG_LEVEL_INFO, + "" + ); + expect(API.addLog).toHaveBeenLastCalledWith("Done!", LOG_LEVEL_NOTICE, "resolveAllConflictedFilesByNewerOnes"); + }); + it("keeps resolver operations on narrow collaborators and extension seams", async () => { const harness = createOperationsHarness(); const path = "same.md" as FilePathWithPrefix; @@ -557,6 +646,7 @@ describe("conflict resolution serviceFeature", () => { useConflictResolutionFeature({ services: harness.services, serviceModules: harness.serviceModules, + log: harness.log, } as unknown as ConflictResolutionHost); for (const registration of registrations) { diff --git a/src/serviceFeatures/conflictResolution/index.ts b/src/serviceFeatures/conflictResolution/index.ts index 2d446681..62cd7bf1 100644 --- a/src/serviceFeatures/conflictResolution/index.ts +++ b/src/serviceFeatures/conflictResolution/index.ts @@ -1,25 +1,23 @@ -import type { NecessaryServices } from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; -import { createInstanceLogFunction } from "@vrtmrz/livesync-commonlib/compat/services/lib/logUtils"; -import type { InjectableConflictService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableConflictService"; -import type { ServiceContext } from "@vrtmrz/livesync-commonlib/context"; +import { + createServiceFeature, + type NecessaryServices, +} from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; +import type { LogFunction } from "@vrtmrz/livesync-commonlib/compat/services/lib/logUtils"; +import type { InjectableServiceHub } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableServiceHub"; import { createConflictCheckingHandlers } from "./checker"; import { createConflictResolutionOperations } from "./operations"; -type ConflictResolutionServices = NecessaryServices< - "API" | "appLifecycle" | "conflict" | "database" | "replication" | "setting" | "vault", - "databaseFileAccess" | "fileHandler" | "storageAccess" ->; - -export type ConflictResolutionHost = ConflictResolutionServices & { - readonly services: ConflictResolutionServices["services"] & { - readonly conflict: InjectableConflictService; - }; +export type ConflictResolutionHost = NecessaryServices< + "appLifecycle" | "conflict" | "database" | "replication" | "setting" | "vault", + "databaseFileAccess" | "fileHandler" | "storageAccess", + InjectableServiceHub +> & { + readonly log: LogFunction; }; -/** Compose the host-neutral conflict checker and resolver handlers. */ -export function useConflictResolutionFeature(host: ConflictResolutionHost): void { - const { services, serviceModules } = host; - const log = createInstanceLogFunction("SF:ConflictResolution", services.API); +/** Compose the host-neutral conflict handlers with the host's borrowed logger. */ +export const useConflictResolutionFeature = createServiceFeature((host: ConflictResolutionHost): void => { + const { services, serviceModules, log } = host; const operations = createConflictResolutionOperations({ events: services.context.events, databaseFileAccess: serviceModules.databaseFileAccess, @@ -48,7 +46,7 @@ export function useConflictResolutionFeature(host: ConflictResolutionHost): void services.conflict.resolve.setHandler(operations.resolve); services.conflict.resolveByNewest.setHandler(operations.resolveByNewest); services.conflict.resolveAllConflictedFilesByNewerOnes.setHandler(operations.resolveAllConflictedFilesByNewerOnes); -} +}); export { createConflictCheckingHandlers } from "./checker"; export type { ConflictCheckingDependencies, ConflictCheckingHandlers } from "./checker"; diff --git a/src/serviceFeatures/onLayoutReady/enablei18n.ts b/src/serviceFeatures/onLayoutReady/enablei18n.ts index 5a83eafb..74f367eb 100644 --- a/src/serviceFeatures/onLayoutReady/enablei18n.ts +++ b/src/serviceFeatures/onLayoutReady/enablei18n.ts @@ -1,9 +1,12 @@ import { getLanguage, Notice, requireApiVersion } from "@/deps"; +import type { NecessaryServices } from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; import { createServiceFeature } from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; import { SUPPORTED_I18N_LANGS, type I18N_LANGS } from "@/common/rosetta"; import { $msg, __onMissingTranslation, setLang } from "@/common/translation"; import { LOG_LEVEL_VERBOSE } from "octagonal-wheels/common/logger"; +type EnableI18nHost = NecessaryServices<"setting" | "API" | "appLifecycle", never>; + function tryGetLanguage(onError: (error: unknown) => void) { if (requireApiVersion("1.8.7")) { try { @@ -48,7 +51,8 @@ class ObsidianLanguageAppliedNotice { } } -export const enableI18nFeature = createServiceFeature(async ({ services: { setting, API, appLifecycle } }) => { +export const enableI18nFeature = createServiceFeature(async (host: EnableI18nHost) => { + const { setting, API, appLifecycle } = host.services; // Clear missing translation handler to avoid unnecessary warnings. __onMissingTranslation(() => {}); let isChanged = false; diff --git a/src/serviceFeatures/onLayoutReady/enablei18n.unit.spec.ts b/src/serviceFeatures/onLayoutReady/enablei18n.unit.spec.ts index 8f88eb62..960c414d 100644 --- a/src/serviceFeatures/onLayoutReady/enablei18n.unit.spec.ts +++ b/src/serviceFeatures/onLayoutReady/enablei18n.unit.spec.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, expectTypeOf, it, vi } from "vitest"; const noticeState = vi.hoisted(() => ({ instances: [] as Array<{ hide: ReturnType; duration: number }>, @@ -63,6 +63,14 @@ describe("automatic display language", () => { vi.restoreAllMocks(); }); + it("declares only the services it needs and returns a boolean promise", () => { + type FeatureHost = Parameters[0]; + + expectTypeOf().toEqualTypeOf<"context" | "setting" | "API" | "appLifecycle">(); + expectTypeOf().toBeNever(); + expectTypeOf>().toEqualTypeOf>(); + }); + it("lets start-up continue and opens translation details only from a persistent Notice", async () => { const settings = { displayLanguage: "" }; const applyPartial = vi.fn(async (partial: Partial) => Object.assign(settings, partial)); diff --git a/src/serviceFeatures/setupObsidian/setupUri.ts b/src/serviceFeatures/setupObsidian/setupUri.ts index 3295c5d3..96b33378 100644 --- a/src/serviceFeatures/setupObsidian/setupUri.ts +++ b/src/serviceFeatures/setupObsidian/setupUri.ts @@ -1,3 +1,4 @@ +import { createCommandCheck } from "@vrtmrz/obsidian-plugin-kit/command"; import { LOG_LEVEL_NOTICE, type ObsidianLiveSyncSettings } from "@vrtmrz/livesync-commonlib/compat/common/types"; import type { LogFunction } from "@vrtmrz/livesync-commonlib/compat/services/lib/logUtils"; import { createInstanceLogFunction } from "@vrtmrz/livesync-commonlib/compat/services/lib/logUtils"; @@ -112,33 +113,40 @@ export function useSetupURIFeature(host: NecessaryServices<"API" | "UI" | "setti host.services.API.addCommand({ id: "livesync-copysetupuri", name: "Copy settings as a new setup URI", - checkCallback: (checking) => { - if (!host.services.setting.currentSettings().isConfigured) return false; - if (!checking) fireAndForget(copySetupURI(host, log)); - return true; - }, + checkCallback: createCommandCheck( + () => host.services.setting.currentSettings().isConfigured, + () => { + fireAndForget(copySetupURI(host, log)); + } + ), }); host.services.API.addCommand({ id: "livesync-copysetupuri-short", name: "Copy settings as a new setup URI (With customization sync)", - checkCallback: (checking) => { - const settings = host.services.setting.currentSettings(); - if (!settings.isConfigured || !settings.usePluginSync) return false; - if (!checking) fireAndForget(copySetupURI(host, log, false)); - return true; - }, + checkCallback: createCommandCheck( + () => { + const settings = host.services.setting.currentSettings(); + return settings.isConfigured && settings.usePluginSync; + }, + () => { + fireAndForget(copySetupURI(host, log, false)); + } + ), }); host.services.API.addCommand({ id: "livesync-copysetupurifull", name: "Copy settings as a new setup URI (Full)", - checkCallback: (checking) => { - const settings = host.services.setting.currentSettings(); - if (!settings.isConfigured || !settings.useAdvancedMode) return false; - if (!checking) fireAndForget(copySetupURIFull(host, log)); - return true; - }, + checkCallback: createCommandCheck( + () => { + const settings = host.services.setting.currentSettings(); + return settings.isConfigured && settings.useAdvancedMode; + }, + () => { + fireAndForget(copySetupURIFull(host, log)); + } + ), }); host.services.context.events.onEvent(EVENT_REQUEST_COPY_SETUP_URI, () => diff --git a/src/serviceFeatures/setupObsidian/setupUri.unit.spec.ts b/src/serviceFeatures/setupObsidian/setupUri.unit.spec.ts index 8a23d50a..9f632f67 100644 --- a/src/serviceFeatures/setupObsidian/setupUri.unit.spec.ts +++ b/src/serviceFeatures/setupObsidian/setupUri.unit.spec.ts @@ -221,7 +221,7 @@ describe("setupObsidian/setupUri", () => { }, UI: { confirm: { - askString: vi.fn(() => "pass"), + askString: vi.fn(async () => false), }, promptCopyToClipboard: vi.fn(() => true), }, @@ -269,7 +269,7 @@ describe("setupObsidian/setupUri", () => { }, UI: { confirm: { - askString: vi.fn(() => "pass"), + askString: vi.fn(async () => false), }, promptCopyToClipboard: vi.fn(() => true), }, @@ -292,5 +292,12 @@ describe("setupObsidian/setupUri", () => { settings.useAdvancedMode = true; expect(command("livesync-copysetupuri-short").checkCallback?.(true)).toBe(true); expect(command("livesync-copysetupurifull").checkCallback?.(true)).toBe(true); + expect(host.services.UI.confirm.askString).not.toHaveBeenCalled(); + settings.isConfigured = false; + for (const registered of commands) expect(registered.checkCallback?.(false)).toBe(false); + expect(host.services.UI.confirm.askString).not.toHaveBeenCalled(); + settings.isConfigured = true; + for (const registered of commands) expect(registered.checkCallback?.(false)).toBe(true); + expect(host.services.UI.confirm.askString).toHaveBeenCalledTimes(3); }); }); diff --git a/src/serviceFeatures/useDatabaseDiagnostics.ts b/src/serviceFeatures/useDatabaseDiagnostics.ts index 2fa974c8..85e582b9 100644 --- a/src/serviceFeatures/useDatabaseDiagnostics.ts +++ b/src/serviceFeatures/useDatabaseDiagnostics.ts @@ -1,3 +1,4 @@ +import { createCommandCheck } from "@vrtmrz/obsidian-plugin-kit/command"; import { createServiceFeature, type NecessaryServices, @@ -46,22 +47,23 @@ export const useDatabaseDiagnosticsFeature = createServiceFeature((host: Databas id: "analyse-database", name: "Analyse Database Usage (advanced)", icon: "database-search", - checkCallback: (checking) => { - if (!services.setting.currentSettings().useAdvancedMode || !services.database.isDatabaseReady()) - return false; - if (!checking) void open("usage"); - return true; - }, + checkCallback: createCommandCheck( + () => services.setting.currentSettings().useAdvancedMode && services.database.isDatabaseReady(), + () => { + void open("usage"); + } + ), }); services.API.addCommand({ id: "inspect-database-integrity", name: $msg("Inspect database integrity"), icon: "database-search", - checkCallback: (checking) => { - if (!services.database.isDatabaseReady()) return false; - if (!checking) void open("integrity"); - return true; - }, + checkCallback: createCommandCheck( + () => services.database.isDatabaseReady(), + () => { + void open("integrity"); + } + ), }); return Promise.resolve(true); }); diff --git a/src/serviceFeatures/useDatabaseDiagnostics.unit.spec.ts b/src/serviceFeatures/useDatabaseDiagnostics.unit.spec.ts index 8372b5ef..df948e2a 100644 --- a/src/serviceFeatures/useDatabaseDiagnostics.unit.spec.ts +++ b/src/serviceFeatures/useDatabaseDiagnostics.unit.spec.ts @@ -82,6 +82,26 @@ describe("database diagnostics serviceFeature", () => { expect(data.openDialog).not.toHaveBeenCalled(); }); + it.each(["analyse-database", "inspect-database-integrity"])( + "rechecks database availability before executing %s", + async (id) => { + const data = fixture(); + data.settings.useAdvancedMode = true; + for (const initialise of data.initialise) await initialise(); + const command = data.commands.find((candidate) => candidate.id === id)!; + expect(command.checkCallback(true)).toBe(true); + expect(data.openDialog).not.toHaveBeenCalled(); + data.setReady(false); + expect(command.checkCallback(false)).toBe(false); + expect(data.openDialog).not.toHaveBeenCalled(); + data.setReady(true); + expect(command.checkCallback(false)).toBe(true); + expect(data.openDialog).toHaveBeenCalledOnce(); + expect(data.openDialog.mock.calls[0][1].kind).toBe(id === "analyse-database" ? "usage" : "integrity"); + data.close(); + } + ); + it("uses the existing dialogue manager and performs no analysis until the run choice is submitted", async () => { vi.mocked(runDatabaseDiagnostic).mockClear(); const data = fixture(); diff --git a/src/serviceFeatures/useP2PReplicatorUI.ts b/src/serviceFeatures/useP2PReplicatorUI.ts index afa1b112..09b60fb4 100644 --- a/src/serviceFeatures/useP2PReplicatorUI.ts +++ b/src/serviceFeatures/useP2PReplicatorUI.ts @@ -1,6 +1,6 @@ import { eventHub, EVENT_REQUEST_OPEN_P2P } from "@/common/events"; import type { NecessaryServices } from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; -import { type UseP2PReplicatorResult } from "@vrtmrz/livesync-commonlib/compat/replication/trystero/UseP2PReplicatorResult"; +import type { P2PServiceViews } from "@vrtmrz/livesync-commonlib/p2p"; import { P2PServerStatusPaneView, VIEW_TYPE_P2P_SERVER_STATUS, @@ -47,22 +47,9 @@ export function hasP2PConfiguration(settings: Partial) */ export function useP2PReplicatorUI( - host: NecessaryServices< - | "API" - | "appLifecycle" - | "setting" - | "vault" - | "database" - | "databaseEvents" - | "keyValueDB" - | "replication" - | "config" - | "UI" - | "replicator", - never - >, + host: NecessaryServices<"API" | "appLifecycle" | "setting" | "replicator", never>, core: LiveSyncCore, - replicator: UseP2PReplicatorResult, + replicator: P2PServiceViews, openInteractiveReplication: OpenInteractiveP2PReplication ) { const api = host.services.API as { diff --git a/src/serviceFeatures/useP2PReplicatorUI.unit.spec.ts b/src/serviceFeatures/useP2PReplicatorUI.unit.spec.ts index 87e4cf4f..650c864d 100644 --- a/src/serviceFeatures/useP2PReplicatorUI.unit.spec.ts +++ b/src/serviceFeatures/useP2PReplicatorUI.unit.spec.ts @@ -1,4 +1,6 @@ -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, expectTypeOf, it, vi } from "vitest"; +import type { NecessaryServices } from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; +import type { P2PServiceViews } from "@vrtmrz/livesync-commonlib/p2p"; import { createServiceContext } from "@vrtmrz/livesync-commonlib/context"; import { eventHub, EVENT_REQUEST_OPEN_P2P } from "@/common/events"; @@ -16,6 +18,19 @@ import { useP2PReplicatorUI } from "./useP2PReplicatorUI"; const noopOpenInteractiveReplication = () => Promise.resolve(false); describe("useP2PReplicatorUI commands", () => { + it("accepts only the UI services and focused P2P views", () => { + type Dependencies = NecessaryServices<"API" | "appLifecycle" | "setting" | "replicator", never>; + type Arguments = Parameters; + + expectTypeOf().toExtend(); + expectTypeOf().toEqualTypeOf< + "context" | "API" | "appLifecycle" | "setting" | "replicator" + >(); + expectTypeOf().toBeNever(); + expectTypeOf().toExtend(); + expectTypeOf().toEqualTypeOf(); + }); + it("waits for settings to load before deciding whether to show the P2P ribbon", async () => { let initialise: (() => Promise) | undefined; let settingLoaded: (() => Promise) | undefined;