refactor(6194): wrap existing Cleanup instead of duplicating it

Develop already ships a working revision-cleanup path under
`src/node/utils/Cleanup.ts` with two public helpers —
`deleteAllRevisions(padId)` (collapse full history via
copyPadWithoutHistory) and `deleteRevisions(padId, keepRevisions)`
(keep the last N). The admin-settings UI wires these up but neither
is exposed on the public API, and there's no CLI for operators who
want to run compaction outside the web UI. That's the gap this PR
now fills.

Changes from the prior revision of this PR:

- Drop `pad.compactHistory()` — it re-implemented what
  `Cleanup.deleteAllRevisions` already does. Remove the duplicate.
- `API.compactPad(padID, keepRevisions?)` now delegates to Cleanup:
    • keepRevisions null/undefined → deleteAllRevisions (full collapse)
    • keepRevisions >= 0          → deleteRevisions(N)  (keep last N)
  Returns {ok, mode: 'all' | 'keepLast', keepRevisions?}.
- APIHandler `1.3.1`: signature updated to take `keepRevisions`
  instead of `authorId`.
- `bin/compactPad.ts`: accepts `--keep N` for the keep-last mode,
  shows before/after revision counts so operators see concrete
  savings.
- Backend tests rewritten around the public API surface (mode
  reporting, text preservation, input validation) rather than
  internal method plumbing that no longer exists.

Net: strictly a thin public-API and CLI veneer over already-tested
Cleanup helpers. No new low-level logic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: JohnMcLear

bin/compactPad.ts

@@ -1,17 +1,20 @@
 'use strict';
 
 /*
- * Compact a pad's revision history in place.
+ * Compact a pad's revision history to reclaim database space.
  *
- * Usage: node bin/compactPad.js <padID>
+ * Usage:
+ *   node bin/compactPad.js <padID>           # collapse all history
+ *   node bin/compactPad.js <padID> --keep N  # keep only the last N revisions
  *
- * Collapses every existing revision into a single base revision that
- * reproduces the current pad content. Text, attributes, and chat history
- * are preserved; saved-revision bookmarks are cleared. Destructive —
- * export the pad as `.etherpad` first for a backup.
+ * Wraps the existing Cleanup helper (src/node/utils/Cleanup.ts) via the
+ * compactPad HTTP API so admins can trigger it from the CLI without
+ * routing through the admin settings UI. Destructive — export the pad as
+ * `.etherpad` first for backup.
  *
- * Implements issue #6194 (admins need a way to reclaim DB space on
- * long-lived pads without rotating to a new pad ID).
+ * Issue #6194: long-lived pads with heavy edit history accumulate hundreds
+ * of megabytes in the DB; this tool is the per-pad brick for reclaiming
+ * that space without rotating to a new pad ID.
  */
 import path from 'node:path';
 import fs from 'node:fs';
@@ -26,12 +29,26 @@ const settings = require('ep_etherpad-lite/tests/container/loadSettings').loadSe
 
 axios.defaults.baseURL = `http://${settings.ip}:${settings.port}`;
 
-if (process.argv.length !== 3) {
-  console.error('Use: node bin/compactPad.js <padID>');
+const usage = () => {
+  console.error('Usage:');
+  console.error('  node bin/compactPad.js <padID>');
+  console.error('  node bin/compactPad.js <padID> --keep <N>');
   process.exit(2);
-}
+};
+
+const args = process.argv.slice(2);
+if (args.length < 1 || args.length > 3) usage();
+const padId = args[0];
 
-const padId = process.argv[2];
+let keepRevisions: number | null = null;
+if (args.length === 3) {
+  if (args[1] !== '--keep') usage();
+  keepRevisions = Number(args[2]);
+  if (!Number.isInteger(keepRevisions) || keepRevisions < 0) {
+    console.error(`--keep expects a non-negative integer; got ${args[2]}`);
+    process.exit(2);
+  }
+}
 
 // get the API Key
 const filePath = path.join(__dirname, '../APIKEY.txt');
@@ -42,25 +59,33 @@ const apikey = fs.readFileSync(filePath, {encoding: 'utf-8'}).trim();
   const apiVersion: string | undefined = apiInfo.data.currentVersion;
   if (!apiVersion) throw new Error('No version set in API');
 
-  // Pre-flight: report current revision count so the operator sees the
-  // savings. getRevisionsCount is older than compactPad so every
-  // supporting server has it.
+  // Pre-flight: show current revision count so operators can eyeball impact.
   const countUri = `/api/${apiVersion}/getRevisionsCount?apikey=${apikey}&padID=${padId}`;
   const countRes = await axios.get(countUri);
   if (countRes.data.code !== 0) {
-    console.error(`Failed to read revision count: ${JSON.stringify(countRes.data)}`);
+    console.error(`getRevisionsCount failed: ${JSON.stringify(countRes.data)}`);
     process.exit(1);
   }
   const before: number = countRes.data.data.revisions;
-  console.log(`Pad ${padId}: ${before + 1} revision(s) on disk.`);
+  const strategy = keepRevisions == null ? 'collapse all' : `keep last ${keepRevisions}`;
+  console.log(`Pad ${padId}: ${before + 1} revision(s). Strategy: ${strategy}.`);
 
-  const uri = `/api/${apiVersion}/compactPad?apikey=${apikey}&padID=${padId}`;
-  const result = await axios.post(uri);
+  const params = new URLSearchParams({apikey, padID: padId});
+  if (keepRevisions != null) params.set('keepRevisions', String(keepRevisions));
+  const result = await axios.post(`/api/${apiVersion}/compactPad?${params.toString()}`);
   if (result.data.code !== 0) {
     console.error(`compactPad failed: ${JSON.stringify(result.data)}`);
     process.exit(1);
   }
-  const removed: number = result.data.data.removed;
-  console.log(`Compacted pad ${padId}: removed ${removed} revision(s). ` +
-              'Pad now has a single base revision reproducing the current content.');
+
+  // Post-flight: the pad is now compacted. Re-read the rev count so the
+  // operator sees concrete savings.
+  const afterRes = await axios.get(countUri);
+  const after: number | undefined = afterRes.data?.data?.revisions;
+  if (after != null) {
+    console.log(`Done. Pad ${padId}: ${after + 1} revision(s) remaining ` +
+                `(was ${before + 1}).`);
+  } else {
+    console.log('Done.');
+  }
 })();

src/node/db/API.ts

@@ -636,26 +636,41 @@ exports.copyPadWithoutHistory = async (sourceID: string, destinationID: string,
 };
 
 /**
-compactPad(padID, [authorId]) collapses the pad's revision history into a
-single base revision that reproduces the current atext, reclaiming database
-space (issue #6194). Pad text, attributes, and chat history are preserved;
-saved-revision bookmarks are cleared. Destructive — recommend exporting the
-`.etherpad` snapshot first.
+compactPad(padID, [keepRevisions]) collapses the pad's revision history to
+reclaim database space (issue #6194). Wraps the existing `Cleanup` helper
+so admins can trigger it over the public API / CLI rather than only
+through the admin settings UI.
+
+When `keepRevisions` is omitted (or `null`), all history is collapsed
+into a single base revision that reproduces the current atext
+(equivalent to a freshly-imported pad). When set to a positive integer
+N, the pad keeps only its last N revisions (equivalent to
+`cleanup.keepRevisions`). Pad text and chat history are preserved in
+both modes. Destructive — recommend exporting the `.etherpad` snapshot
+first.
 
 Example returns:
 
-{code: 0, message:"ok", data: {removed: 87}}
+{code: 0, message:"ok", data: {ok: true, mode: "all"}}
 {code: 1, message:"padID does not exist", data: null}
 
  @param {String} padID the id of the pad to compact
- @param {String} authorId the id of the author to attribute the new base
-     revision to, defaulting to empty string (anonymous)
- @returns the number of revisions removed
+ @param {Number|null} keepRevisions number of recent revisions to keep;
+     null / omitted collapses the full history
 */
-exports.compactPad = async (padID: string, authorId = '') => {
+exports.compactPad = async (padID: string, keepRevisions: number | null = null) => {
   const pad = await getPadSafe(padID, true);
-  const removed = await pad.compactHistory(authorId);
-  return {removed};
+  const cleanup = require('../utils/Cleanup');
+  if (keepRevisions == null) {
+    await cleanup.deleteAllRevisions(pad.id);
+    return {ok: true, mode: 'all'};
+  }
+  const keep = Number(keepRevisions);
+  if (!Number.isFinite(keep) || keep < 0) {
+    throw new CustomError('keepRevisions must be a non-negative integer', 'apierror');
+  }
+  const ok = await cleanup.deleteRevisions(pad.id, keep);
+  return {ok, mode: 'keepLast', keepRevisions: keep};
 };
 
 /**

src/node/db/Pad.ts

@@ -554,92 +554,6 @@ class Pad {
         (authorID) => authorManager.addPad(authorID, destinationID)));
   }
 
-  /**
-   * Compact the pad's revision history in place (issue #6194).
-   *
-   * Etherpad keeps every revision forever, so long-lived pads eventually
-   * dominate the database — the issue describes a ~400 MB Postgres for a
-   * ~2-month-old instance with ~100 users. There is no safe way to prune
-   * arbitrary middle revisions (Etherpad reconstructs state by composing
-   * forward from key revisions), but we CAN collapse the entire history
-   * into a minimal set of revisions that reproduce the current atext. The
-   * existing `copyPadWithoutHistory` does this for a new pad ID — we lean
-   * on it via a temporary pad, then swap records back.
-   *
-   * After compaction:
-   *   • head ≤ 1 (a single content revision on top of the initial \n seed,
-   *     matching the shape of a freshly-imported pad)
-   *   • text, attributes, and pool unchanged
-   *   • chat history untouched
-   *   • saved-revision bookmarks cleared
-   *
-   * Destructive — run an `.etherpad` export first for backup.
-   *
-   * @param authorId The author to attribute the collapsed revision to.
-   *     Defaults to empty (anonymous) which matches the existing
-   *     copyPadWithoutHistory path.
-   * @returns The number of revisions removed.
-   */
-  async compactHistory(authorId = '') {
-    const originalHead = this.head;
-    if (originalHead <= 1) return 0;
-
-    // Spin up a temp pad holding just the current snapshot. This runs the
-    // tested copyPadWithoutHistory path unchanged — it handles the
-    // "pad starts with \n\n then splice in the real atext" dance, preserves
-    // attributes/pool, and produces exactly head=1 on the destination.
-    const tempId = `__compact_tmp__${this.id}_${Date.now()}_${Math.floor(Math.random() * 1e9)}`;
-    await this.copyPadWithoutHistory(tempId, false, authorId);
-
-    try {
-      const tempPad = await padManager.getPad(tempId);
-      const tempHead = tempPad.head;
-
-      // Load every rev record from the temp pad into memory so we can write
-      // them over this pad's keys after deleting the old ones.
-      const newRevs: Array<any> = [];
-      for (let r = 0; r <= tempHead; r++) {
-        // @ts-ignore
-        newRevs.push(await this.db.get(`pad:${tempId}:revs:${r}`));
-      }
-
-      // Drop every existing revision record.
-      const deletions: Promise<void>[] = [];
-      for (let r = 0; r <= originalHead; r++) {
-        // @ts-ignore
-        deletions.push(this.db.remove(`pad:${this.id}:revs:${r}`));
-      }
-      await Promise.all(deletions);
-
-      // Write the compacted revs under this pad's keys.
-      await Promise.all(newRevs.map((rec, r) =>
-          // @ts-ignore
-          this.db.set(`pad:${this.id}:revs:${r}`, rec)));
-
-      // Mirror the temp pad's in-memory state back into this one (the atext
-      // and pool have already been normalized by copyPadWithoutHistory to
-      // match what now lives in the rev records).
-      this.savedRevisions = [];
-      this.head = tempHead;
-      this.pool = tempPad.pool;
-      this.atext = tempPad.atext;
-      await this.saveToDatabase();
-
-      // Throw the temp pad away; it has served its purpose.
-      await tempPad.remove();
-
-      return originalHead - tempHead;
-    } catch (err) {
-      // Best-effort cleanup of the temp pad if anything went wrong after it
-      // was created. Never mask the original error.
-      try {
-        const tempPad = await padManager.getPad(tempId);
-        await tempPad.remove();
-      } catch { /* ignore */ }
-      throw err;
-    }
-  }
-
   async copyPadWithoutHistory(destinationID: string, force: string|boolean, authorId = '') {
     // flush the source pad
     this.saveToDatabase();

src/tests/backend/specs/compactPad.ts

@@ -5,7 +5,9 @@ const common = require('../common');
 const padManager = require('../../../node/db/PadManager');
 const api = require('../../../node/db/API');
 
-// Regression + behavior tests for https://github.com/ether/etherpad/issues/6194.
+// Coverage for the compactPad API endpoint added in #6194.
+// The underlying Cleanup logic is tested where it lives; these tests just
+// verify the public-API wiring and argument handling.
 describe(__filename, function () {
   let padId: string;
 
@@ -14,16 +16,8 @@ describe(__filename, function () {
     assert(!await padManager.doesPadExist(padId));
   });
 
-  describe('pad.compactHistory()', function () {
-    it('no-ops a pad that is already at head <= 1', async function () {
-      const pad = await padManager.getPad(padId);
-      // Fresh pads land at head=0 (just the defaultText rev); compactHistory
-      // has nothing useful to do on a pad that short.
-      const removed = await pad.compactHistory();
-      assert.strictEqual(removed, 0);
-    });
-
-    it('collapses history to head<=1 while preserving text', async function () {
+  describe('API.compactPad()', function () {
+    it('collapses all history when keepRevisions is omitted', async function () {
       const pad = await padManager.getPad(padId);
       await pad.appendText('line 1\n');
       await pad.appendText('line 2\n');
@@ -32,59 +26,50 @@ describe(__filename, function () {
       const expectedText = pad.atext.text;
       assert.ok(before >= 3, `expected at least 3 revs, got ${before}`);
 
-      const removed = await pad.compactHistory();
+      const result = await api.compactPad(padId);
+      assert.deepStrictEqual(result, {ok: true, mode: 'all'});
 
-      // The collapsed pad matches the shape of a freshly-imported pad
-      // (head=1: a seed rev + the full-content rev). Exact count depends
-      // on whether the defaultText-init counted as rev 0, but the
-      // invariant is `head <= 1`.
-      const afterHead = pad.getHeadRevisionNumber();
-      assert.ok(afterHead <= 1, `expected head<=1 after compact, got ${afterHead}`);
-      assert.strictEqual(removed, before - afterHead);
-      assert.strictEqual(pad.atext.text, expectedText);
-      // Reload from DB to confirm the collapse actually landed.
+      // Reload: the compacted pad lands at head<=1 (matches the shape
+      // `copyPadWithoutHistory` produces), text unchanged.
       const reloaded = await padManager.getPad(padId);
-      assert.strictEqual(reloaded.getHeadRevisionNumber(), afterHead);
+      assert.ok(reloaded.getHeadRevisionNumber() <= 1,
+          `expected head<=1, got ${reloaded.getHeadRevisionNumber()}`);
       assert.strictEqual(reloaded.atext.text, expectedText);
     });
 
-    it('drops saved-revision bookmarks', async function () {
+    it('keeps only the last N revisions when keepRevisions is a number',
+        async function () {
+          const pad = await padManager.getPad(padId);
+          for (let i = 0; i < 6; i++) await pad.appendText(`line ${i}\n`);
+          const before = pad.getHeadRevisionNumber();
+          const expectedText = pad.atext.text;
+
+          const result = await api.compactPad(padId, 2);
+          assert.strictEqual(result.mode, 'keepLast');
+          assert.strictEqual(result.keepRevisions, 2);
+
+          const reloaded = await padManager.getPad(padId);
+          // Exact head depends on Cleanup internals; the invariant we can
+          // assert is that the head is <= before and the content survives.
+          assert.ok(reloaded.getHeadRevisionNumber() <= before);
+          assert.strictEqual(reloaded.atext.text, expectedText);
+        });
+
+    it('rejects negative keepRevisions', async function () {
       const pad = await padManager.getPad(padId);
-      await pad.appendText('content line 1\n');
-      await pad.appendText('content line 2\n');
-      // @ts-ignore — savedRevisions is private but set from JSON on load.
-      pad.savedRevisions.push({revNum: pad.getHeadRevisionNumber()});
-      await pad.compactHistory();
-      // @ts-ignore
-      assert.deepStrictEqual(pad.savedRevisions, []);
+      await pad.appendText('content\n');
+      await assert.rejects(
+          () => api.compactPad(padId, -1),
+          /keepRevisions must be a non-negative integer/);
     });
 
-    it('leaves subsequent edits appending cleanly on top of the collapsed base', async function () {
+    it('rejects non-numeric keepRevisions', async function () {
       const pad = await padManager.getPad(padId);
-      await pad.appendText('first\n');
-      await pad.appendText('second\n');
-      await pad.appendText('third\n');
-      await pad.compactHistory();
-      const postCompactHead = pad.getHeadRevisionNumber();
-      await pad.appendText('fourth\n');
-      assert.strictEqual(pad.getHeadRevisionNumber(), postCompactHead + 1);
-      assert.ok(pad.atext.text.includes('fourth'),
-          `expected "fourth" in post-compact text: ${pad.atext.text}`);
+      await pad.appendText('content\n');
+      await assert.rejects(
+          // @ts-ignore - deliberately passing an invalid type
+          () => api.compactPad(padId, 'nope'),
+          /keepRevisions must be a non-negative integer/);
     });
   });
-
-  describe('API.compactPad()', function () {
-    it('reports the number of revisions removed and compacts the pad',
-        async function () {
-          const pad = await padManager.getPad(padId);
-          await pad.appendText('alpha\n');
-          await pad.appendText('beta\n');
-          await pad.appendText('gamma\n');
-          const before = pad.getHeadRevisionNumber();
-          const result = await api.compactPad(padId);
-          const afterHead = pad.getHeadRevisionNumber();
-          assert.ok(afterHead <= 1);
-          assert.strictEqual(result.removed, before - afterHead);
-        });
-  });
 });