Merge branch 'master' of git.zakscode.com:ztimson/zim-utils into fix/install-kwix-arm64

Reviewed-on: #1

package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@ztimson/zim-utils",
-	"version": "0.3.7",
+	"version": "0.3.8",
 	"description": "Native, dependency-light ZIM archive reader/searcher and Kiwix catalog downloader for Node.js",
 	"author": "Zak Timson",
 	"license": "MIT",

src/manager.js

@@ -5,8 +5,13 @@ import {Readable} from 'node:stream';
 import {KiwixServer} from './server.js';
 import {zimCatalog, zimCatalogInfo, CATALOG_URL} from './catalog.js';
 
+const VISIBLE_TIMEOUT = 5_000;
+const VISIBLE_POLL_INTERVAL = 200;
+
 /** Manages a local directory of ZIM archives: catalog search, downloads, update checks, deletion.
- * Reuses (or owns & lazily starts) a KiwixServer for local listing/search, so `list()` and `catalog()` return the same shape. */
+ * Reuses (or owns & lazily starts) a KiwixServer for local listing/search, so `list()` and `catalog()` return the same shape.
+ * Doesn't drive library.xml/kiwix-serve reloads itself - the owning KiwixServer watches its directory and
+ * reloads automatically whenever .zim files change, no matter which ZimManager (local or remote-attached) wrote them. */
 export class ZimManager {
 	#catalogUrl;
 	#dir;
@@ -56,6 +61,20 @@ export class ZimManager {
 		return {res, url: m[1]};
 	}
 
+	/** Polls list() until `filename` shows up (or disappears, if `expect: false`), so callers get an accurate
+	 * status back rather than one that's ahead of what the server has actually picked up yet. The owning
+	 * KiwixServer's directory watcher does the real reload work in the background; this just waits for it. */
+	async #waitUntilVisible(filename, {expect = true, timeout = VISIBLE_TIMEOUT} = {}) {
+		const deadline = Date.now() + timeout;
+		while (Date.now() < deadline) {
+			const local = await this.list();
+			const present = local.some(l => l.file === filename);
+			if (present === expect) return true;
+			await new Promise(r => setTimeout(r, VISIBLE_POLL_INTERVAL));
+		}
+		return false;
+	}
+
 	async #update(name, catalogEntry, localMatch, force) {
 		const remoteDate = catalogEntry.updated ? new Date(catalogEntry.updated) : null;
 		const localDate = localMatch?.updated ?? null;
@@ -67,8 +86,8 @@ export class ZimManager {
 		const destPath = path.join(this.#dir, filename);
 		await this.#download(catalogEntry.href, destPath);
 		if (localMatch && localMatch.file !== filename) await fs.promises.rm(path.join(this.#dir, localMatch.file), {force: true});
-		const server = await this.#ensureServer();
-		await server.reload();
+		await this.#ensureServer();
+		await this.#waitUntilVisible(filename, {expect: true});
 		return {name, status: 'updated', file: filename};
 	}
 
@@ -87,8 +106,8 @@ export class ZimManager {
 		if (!match) throw new Error(`ZIM not found locally: ${nameOrFile}`);
 		const file = match.href + (match.href.endsWith('.zim') ? '' : '.zim');
 		await fs.promises.rm(path.join(this.#dir, file), {force: true});
-		const server = await this.#ensureServer();
-		await server.reload();
+		await this.#ensureServer();
+		await this.#waitUntilVisible(file, {expect: false});
 		return {file: match.file, status: 'deleted'};
 	}
 
@@ -101,7 +120,7 @@ export class ZimManager {
 		const exact = local.find(l => l.file === filename);
 		if (exact && !force) return {name: exact.name ?? this.#stripDate(filename), status: 'skipped', reason: 'already downloaded', file: filename};
 		const name = this.#stripDate(filename);
-		const localMatch = local.find(l => this.#stripDate(l.file) === name) ?? null;
+		const localMatch = local.find(l => l.name === name) ?? null;
 		const catalogEntry = await zimCatalogInfo(name, this.#catalogUrl) || {name, updated: null, href};
 		return this.#update(name, catalogEntry, localMatch, force);
 	}

src/server.js

@@ -6,13 +6,14 @@ import net from 'node:net';
 import fs from 'node:fs';
 import path from 'node:path';
 import {fileURLToPath} from 'node:url';
-import {fromXml} from '@ztimson/utils';
+import {fromXml, makeArray} from '@ztimson/utils';
 
 const execFileAsync = promisify(execFile);
 const __dirname = path.dirname(fileURLToPath(import.meta.url));
 const DEFAULT_BIN_DIR = path.join(__dirname, '..', 'bin'); // npm package root/bin - where bin/install.js drops the kiwix-tools binaries
 const READY_TIMEOUT = 10_000;
 const READY_POLL_INTERVAL = 100;
+const WATCH_DEBOUNCE = 300;
 
 function findFreePort() {
 	return new Promise((resolve, reject) => {
@@ -25,7 +26,9 @@ function findFreePort() {
 	});
 }
 
-/** Owns a kiwix-serve process's full lifecycle: library.xml, start/stop/reload, content + search access. */
+/** Owns a kiwix-serve process's full lifecycle: library.xml, start/stop/reload, content + search access.
+ * Watches its own directory for .zim files appearing/disappearing (from *any* writer - itself, a remote-attached
+ * ZimManager, a script, whatever) and keeps library.xml + the running kiwix-serve process in sync automatically. */
 export class KiwixServer {
 	static #empty = '<?xml version="1.0" encoding="UTF-8" ?>\n<library version="20110515"></library>\n';
 
@@ -36,6 +39,8 @@ export class KiwixServer {
 	#libraryPath;
 	#child = null;
 	#remote; // baseUrl string if attached to an externally-managed kiwix-serve, else null
+	#watcher = null;
+	#watchTimer = null;
 
 	get port() { return this.#port; }
 	get running() { return !!this.#remote || !!this.#child; }
@@ -43,7 +48,9 @@ export class KiwixServer {
 
 	/** @param {{port?: number, host?: string, binDir?: string, url?: string}} [opts]
 	 * url: attach to an already-running kiwix-serve (e.g. one started elsewhere in your codebase) instead of
-	 * spawning/owning one - start/stop/reload become no-ops, and library.xml is read over HTTP instead of disk. */
+	 * spawning/owning one - start/stop become no-ops, and library.xml is read over HTTP instead of disk.
+	 * Whichever instance *does* own the process is responsible for watching the shared directory; an attached
+	 * instance doesn't need its own watcher; it just reads whatever the owner already reloaded. */
 	constructor(dir, {port, host = '127.0.0.1', binDir = DEFAULT_BIN_DIR, url} = {}) {
 		this.#dir = dir;
 		this.#host = host;
@@ -78,7 +85,9 @@ export class KiwixServer {
 		}
 	}
 
-	/** Rebuilds library.xml from scratch by scanning `dir` for .zim files - no-op if attached to a remote server. */
+	/** Rebuilds library.xml from scratch by scanning `dir` for .zim files - no-op if attached to a remote server.
+	 * Rewriting the file bumps its mtime, which kiwix-serve (started with --monitorLibrary) picks up on its own
+	 * and hot-reloads without needing a restart. */
 	async #rebuildLibrary() {
 		if (this.#remote) return;
 		await fs.promises.rm(this.#libraryPath, {force: true});
@@ -103,15 +112,39 @@ export class KiwixServer {
 		return (await fs.promises.readdir(this.#dir).catch(() => [])).filter(f => f.endsWith('.zim'));
 	}
 
-	/** Rebuilds library.xml and starts kiwix-serve. Resolves once the server is responding. */
+	/** Watches `dir` for .zim files being added/removed by anyone (this process, a remote-attached ZimManager,
+	 * a manual copy) and debounces a library rebuild so kiwix-serve's --monitorLibrary picks it up. Only runs
+	 * while we own the process - an attached instance has nothing local to watch on behalf of. */
+	#watchDir() {
+		this.#watcher?.close();
+		this.#watcher = fs.watch(this.#dir, (_event, filename) => {
+			if (!filename?.endsWith('.zim')) return;
+			clearTimeout(this.#watchTimer);
+			this.#watchTimer = setTimeout(() => this.#rebuildLibrary().catch(() => {}), WATCH_DEBOUNCE);
+		});
+		this.#watcher.on('error', () => {}); // e.g. dir removed out from under us - just stop watching, don't crash the process
+	}
+
+	#unwatchDir() {
+		clearTimeout(this.#watchTimer);
+		this.#watchTimer = null;
+		this.#watcher?.close();
+		this.#watcher = null;
+	}
+
+	/** Rebuilds library.xml and starts kiwix-serve. Resolves once the server is responding.
+	 * Starts with --monitorLibrary so the process reloads itself whenever library.xml's mtime changes -
+	 * no restart needed for updates after this. */
 	async start() {
 		if (this.#remote || this.#child) return;
 		await fs.promises.mkdir(this.#dir, {recursive: true});
 		await this.#rebuildLibrary();
 		this.#port ??= await findFreePort();
 
-		this.#child = spawn(this.#bin('kiwix-serve'), ['--library', '-i', this.#host, '-p', String(this.#port), this.#libraryPath], {stdio: 'ignore'});
-		this.#child.on('exit', () => { this.#child = null; });
+		this.#child = spawn(this.#bin('kiwix-serve'),
+			['--library', '--monitorLibrary', '-i', this.#host, '-p', String(this.#port), this.#libraryPath],
+			{stdio: 'ignore'});
+		this.#child.on('exit', () => { this.#child = null; this.#unwatchDir(); });
 
 		try {
 			await this.#waitUntilReady();
@@ -119,10 +152,13 @@ export class KiwixServer {
 			await this.stop();
 			throw e;
 		}
+
+		this.#watchDir();
 	}
 
 	/** Gracefully stops kiwix-serve, if we own it. No-op if attached to a remote instance. */
 	async stop() {
+		this.#unwatchDir();
 		if (this.#remote || !this.#child) return;
 		const child = this.#child;
 		await new Promise(resolve => {
@@ -137,11 +173,13 @@ export class KiwixServer {
 		await this.start();
 	}
 
-	/** Rebuilds library.xml from disk and restarts kiwix-serve. No-op if attached to a remote instance -
-	 * whoever owns that process is responsible for reloading it. */
+	/** Forces an immediate library rebuild rather than waiting for the directory watcher's debounce to fire.
+	 * Not required for correctness (the watcher does this automatically for any writer), just a manual way to
+	 * skip the ~300ms wait when you already know something changed. No-op if attached to a remote instance -
+	 * whoever owns that process already reloads itself. */
 	async reload() {
 		if (this.#remote || !this.#child) return;
-		await this.restart();
+		await this.#rebuildLibrary();
 	}
 
 	/** Local catalog listing - same flat shape as the online catalog (catalog.js), plus a `file` field. */
@@ -150,9 +188,9 @@ export class KiwixServer {
 		const xml = await this.#fetchLibraryXml();
 		if (!xml) return [];
 		const entries = fromXml(xml);
-		return (entries?.library?.book || []).map(e => {
+		return makeArray(entries?.library?.book || []).map(e => {
 			const tags = e.tags.split(';');
-			const name = e.path.replaceAll('.zim', '');
+			const name = e.path.replace('.zim', '');
 			return {
 				id: e.id,
 				title: e.title,