refactor: register log cleanup through the host API

Delegate display, subscription, and collector cleanup through APIService
instead of registering directly on the Obsidian plug-in. The Obsidian adapter
forwards the same callbacks to native component ownership.

Preserve the separate log resource which remains alive throughout asynchronous
shutdown, and cover native delegation and renderer cleanup registration.
This commit is contained in:
vorotamoroz committed 2026-11-06 12:17:31 +00:00
1 parent cf527fbf86
commit a6b97ddb3b
4 files changed
+43 -5

No files matched your search

+5 -5
View File
@@ -168,7 +168,7 @@ export class ModuleLog extends AbstractObsidianModule {
observeForLogs() {
const registerDisplay = <T extends { dispose(): void }>(display: T): T => {
this.plugin.register(() => display.dispose());
this.services.API.registerCleanup(() => display.dispose());
return display;
};
const labelReplication = registerDisplay(
@@ -211,12 +211,12 @@ export class ModuleLog extends AbstractObsidianModule {
);
const p2pProgress = registerDisplay(createP2PProgressDisplay(this.p2pLogCollector.p2pReplicationStats));
const p2pIsPrimary = reactiveSource(this.settings.remoteType === REMOTE_P2P);
this.plugin.register(
this.services.API.registerCleanup(
this.services.context.events.onEvent(EVENT_SETTING_SAVED, (settings) => {
p2pIsPrimary.value = settings.remoteType === REMOTE_P2P;
})
);
this.plugin.register(() => this.p2pLogCollector.dispose());
this.services.API.registerCleanup(() => this.p2pLogCollector.dispose());
const replicationStatLabel = computed(() =>
projectReplicationStatusLabel(
this.services.replicator.replicationStatics.value,
@@ -530,12 +530,12 @@ ${stringifyYaml(info)}
if (this.statusLine) {
const renderer = createStatusBarRenderer(this.statusLine);
this.statusLineRenderer = renderer;
this.plugin.register(() => renderer.dispose());
this.services.API.registerCleanup(() => renderer.dispose());
}
if (this.statusBar) {
const renderer = createStatusBarRenderer(this.statusBar);
this.statusBarRenderer = renderer;
this.plugin.register(() => renderer.dispose());
this.services.API.registerCleanup(() => renderer.dispose());
}
this.adjustStatusDivPosition();
this._log("Log module loaded", LOG_LEVEL_INFO);
@@ -8,6 +8,9 @@ vi.mock("@/deps.ts", () => ({
stringifyYaml: vi.fn(),
}));
vi.mock("./Log/LogPaneView.ts", () => ({ LogPaneView: class {}, VIEW_TYPE_LOG: "log" }));
vi.mock("./StatusBarRenderer.ts", () => ({
createStatusBarRenderer: vi.fn(() => ({ dispose: vi.fn() })),
}));
vi.mock("@/common/reportTool.ts", () => ({ generateReport: vi.fn() }));
vi.mock("@/common/utils.ts", () => ({ isValidPath: () => true }));
vi.mock("@/common/translation", () => ({ $msg: (key: string) => key }));
@@ -36,6 +39,7 @@ async function createHarness() {
registerWindow: vi.fn(),
addRibbonIcon: vi.fn(),
registerProtocolHandler: vi.fn(),
registerCleanup: vi.fn<(callback: () => void) => void>(),
},
context: { events: createLiveSyncEventHub(), notices: { hide: vi.fn() } },
appLifecycle: {
@@ -64,6 +68,22 @@ describe("ModuleLog service connection", () => {
vi.unstubAllGlobals();
});
it("registers status renderer cleanup through the API without native plug-in registration", async () => {
const { services, module } = await createHarness();
const { createStatusBarRenderer } = await import("./StatusBarRenderer.ts");
const factory = vi.mocked(createStatusBarRenderer);
factory.mockClear();
Object.assign(module, { statusLine: {}, statusBar: {} });
await services.appLifecycle.onSettingLoaded();
expect(factory).toHaveBeenCalledTimes(2);
expect(services.API.registerCleanup).toHaveBeenCalledTimes(2);
const renderers = factory.mock.results.map((result) => result.value);
for (const renderer of renderers) expect(renderer.dispose).not.toHaveBeenCalled();
for (const [cleanup] of services.API.registerCleanup.mock.calls) cleanup();
for (const renderer of renderers) expect(renderer.dispose).toHaveBeenCalledOnce();
await module.logResource.dispose();
});
it("routes accepted P2P diagnostics only through their owning context and disposes the subscription", async () => {
const first = await createHarness();
const second = await createHarness();
@@ -135,6 +135,11 @@ export class ObsidianAPIService extends InjectableAPIService<ObsidianServiceCont
return this.context.plugin.registerObsidianProtocolHandler(action, handler);
}
override registerCleanup(callback: () => void): void {
// Keep disposal owned by the native component, preserving its unload timing.
this.context.plugin.register(callback);
}
/**
* In Obsidian, we will use the native `requestUrl` function as the default fetch handler,
* to address unavoidable CORS issues.
@@ -248,3 +248,16 @@ describe("ObsidianAPIService.nativeFetch cache control", () => {
expect(sentHeaders().has("pragma")).toBe(false);
});
});
describe("ObsidianAPIService.registerCleanup", () => {
it("delegates the original callback to native component ownership without invoking it immediately", () => {
const register = vi.fn();
const service = new ObsidianAPIService({ app: {}, plugin: { register } } as unknown as ObsidianServiceContext);
const cleanup = vi.fn();
expect(service.registerCleanup(cleanup)).toBeUndefined();
expect(register).toHaveBeenCalledExactlyOnceWith(cleanup);
expect(cleanup).not.toHaveBeenCalled();
register.mock.calls[0][0]();
expect(cleanup).toHaveBeenCalledOnce();
});
});