Compare commits
2 Commits
feat/251-i
...
fix/255-ws
| Author | SHA1 | Date | |
|---|---|---|---|
|
82b042209e | ||
|
|
a0f4c86a74 |
@@ -123,7 +123,6 @@ import { countWords } from "alfaaz";
|
||||
import AutoJoiner from "@/features/editor/extensions/autojoiner.ts";
|
||||
import GlobalDragHandle from "@/features/editor/extensions/drag-handle.ts";
|
||||
import { CleanStyles } from "@/features/editor/extensions/clean-styles.ts";
|
||||
import { IntentionalClear } from "@/features/editor/extensions/intentional-clear.ts";
|
||||
|
||||
const lowlight = createLowlight(common);
|
||||
lowlight.register("mermaid", plaintext);
|
||||
@@ -487,10 +486,4 @@ export const collabExtensions: CollabExtensions = (provider, user) => [
|
||||
color: randomElement(userColors),
|
||||
},
|
||||
}),
|
||||
// #251 — emit an intentional-clear signal to the server when the user
|
||||
// deliberately empties the page, so the #248 store-side empty-guard lets that
|
||||
// one clear through while still blocking accidental empties.
|
||||
IntentionalClear.configure({
|
||||
provider,
|
||||
}),
|
||||
];
|
||||
|
||||
@@ -1,88 +0,0 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { Editor } from "@tiptap/core";
|
||||
import { Document } from "@tiptap/extension-document";
|
||||
import { Paragraph } from "@tiptap/extension-paragraph";
|
||||
import { Text } from "@tiptap/extension-text";
|
||||
import {
|
||||
IntentionalClear,
|
||||
INTENTIONAL_CLEAR_MESSAGE_TYPE,
|
||||
} from "./intentional-clear";
|
||||
|
||||
/**
|
||||
* #251 — the intentional-clear signal is driven through the REAL editor path:
|
||||
* a fresh Editor with the IntentionalClear extension, a fake provider that
|
||||
* records sendStateless, and the actual select-all + delete command the user's
|
||||
* keystroke runs. No hand-poke of any flag.
|
||||
*/
|
||||
describe("IntentionalClear extension", () => {
|
||||
let sendStateless: ReturnType<typeof vi.fn>;
|
||||
|
||||
const makeEditor = (content: unknown) =>
|
||||
new Editor({
|
||||
extensions: [
|
||||
Document,
|
||||
Paragraph,
|
||||
Text,
|
||||
IntentionalClear.configure({
|
||||
// Minimal provider stand-in: only sendStateless is exercised.
|
||||
provider: { sendStateless } as any,
|
||||
}),
|
||||
],
|
||||
content: content as any,
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
sendStateless = vi.fn();
|
||||
});
|
||||
|
||||
it("emits the clear signal when a user empties a non-empty doc (select-all + delete)", () => {
|
||||
const editor = makeEditor({
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "hello world" }] },
|
||||
],
|
||||
});
|
||||
|
||||
// The exact command path a select-all + Delete keystroke dispatches.
|
||||
editor.chain().selectAll().deleteSelection().run();
|
||||
|
||||
expect(sendStateless).toHaveBeenCalledTimes(1);
|
||||
const payload = JSON.parse(sendStateless.mock.calls[0][0]);
|
||||
expect(payload).toEqual({ type: INTENTIONAL_CLEAR_MESSAGE_TYPE });
|
||||
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
it("does NOT emit when typing into an empty doc (no non-empty → empty transition)", () => {
|
||||
const editor = makeEditor({ type: "doc", content: [{ type: "paragraph" }] });
|
||||
|
||||
editor.chain().insertContent("typed text").run();
|
||||
|
||||
expect(sendStateless).not.toHaveBeenCalled();
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
it("does NOT emit on an edit that leaves the doc non-empty", () => {
|
||||
const editor = makeEditor({
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "keep me" }] },
|
||||
],
|
||||
});
|
||||
|
||||
editor.chain().insertContent(" more").run();
|
||||
|
||||
expect(sendStateless).not.toHaveBeenCalled();
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
it("does NOT emit when the doc was already empty", () => {
|
||||
const editor = makeEditor({ type: "doc", content: [{ type: "paragraph" }] });
|
||||
|
||||
// Selecting all + delete on an already-empty doc is a no-op transition.
|
||||
editor.chain().selectAll().deleteSelection().run();
|
||||
|
||||
expect(sendStateless).not.toHaveBeenCalled();
|
||||
editor.destroy();
|
||||
});
|
||||
});
|
||||
@@ -1,94 +0,0 @@
|
||||
import { Extension } from "@tiptap/core";
|
||||
import { isChangeOrigin } from "@tiptap/extension-collaboration";
|
||||
import type { Node as PMNode } from "@tiptap/pm/model";
|
||||
import type { HocuspocusProvider } from "@hocuspocus/provider";
|
||||
|
||||
/**
|
||||
* Stateless message type sent to the server when a user deliberately clears a
|
||||
* page to empty. Kept in one place so the client emitter and the server
|
||||
* consumer (PersistenceExtension.onStateless) agree on the wire format.
|
||||
*/
|
||||
export const INTENTIONAL_CLEAR_MESSAGE_TYPE = "intentional-clear";
|
||||
|
||||
export interface IntentionalClearOptions {
|
||||
/** The collab provider used to send the stateless clear signal. */
|
||||
provider: HocuspocusProvider | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* A "document is empty" check that mirrors the server's `isEmptyParagraphDoc`
|
||||
* (collaboration.util.ts): exactly one top-level paragraph with no inline
|
||||
* content. After a select-all + delete TipTap leaves precisely this shape, so
|
||||
* matching it here keeps the client signal aligned with the server guard that
|
||||
* consumes it.
|
||||
*/
|
||||
function isEmptyParagraphDoc(doc: PMNode): boolean {
|
||||
if (doc.childCount !== 1) return false;
|
||||
const child = doc.firstChild;
|
||||
return (
|
||||
child !== null &&
|
||||
child !== undefined &&
|
||||
child.type.name === "paragraph" &&
|
||||
child.content.size === 0
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* #251 — intentional-clear signal.
|
||||
*
|
||||
* The server's #248 store-side empty-guard unconditionally refuses to overwrite
|
||||
* non-empty persisted content with an empty document, because a momentarily
|
||||
* empty live Y.Doc (a glitch, a bad merge, an emptying transclusion) is
|
||||
* indistinguishable from a real clear *at the store layer*. That protection is
|
||||
* correct, but it also blocks a user who genuinely wants to empty the page.
|
||||
*
|
||||
* This extension supplies the missing distinction. It watches LOCAL, user-driven
|
||||
* transactions and, the moment one reduces a non-empty document to the empty
|
||||
* single-paragraph shape, it sends a hocuspocus stateless message to the server.
|
||||
* The server records a short-lived, single-use "intentional clear pending" flag
|
||||
* for this document that the next (debounced) onStoreDocument consumes to let
|
||||
* that one empty write through the guard.
|
||||
*
|
||||
* What counts as an intentional clear (precise definition):
|
||||
* - the transaction actually changed the document (`docChanged`), AND
|
||||
* - it is a LOCAL user edit, not a remote collab application — remote y-sync
|
||||
* transactions are tagged and filtered out via `isChangeOrigin`, so an
|
||||
* emptiness that arrives from another client / a merge never emits a signal,
|
||||
* AND
|
||||
* - the document was non-empty before the transaction and is the empty
|
||||
* single-paragraph doc after it.
|
||||
*
|
||||
* This is exactly the select-all + Delete / Backspace (or any local command that
|
||||
* empties the doc, e.g. clearContent) keystroke path. A transient/programmatic
|
||||
* empty serialization that the server might see on the wire does NOT come with
|
||||
* this signal, so the guard still blocks it.
|
||||
*/
|
||||
export const IntentionalClear = Extension.create<IntentionalClearOptions>({
|
||||
name: "intentionalClear",
|
||||
|
||||
addOptions() {
|
||||
return {
|
||||
provider: null,
|
||||
};
|
||||
},
|
||||
|
||||
onTransaction({ transaction }) {
|
||||
if (!transaction.docChanged) return;
|
||||
// Only react to local user edits. Remote collaboration steps (and other
|
||||
// y-sync-applied changes) carry the change origin and must never be treated
|
||||
// as an intentional clear, otherwise a remote/merge-induced emptiness would
|
||||
// punch through the server guard.
|
||||
if (isChangeOrigin(transaction)) return;
|
||||
|
||||
const becameEmpty =
|
||||
!isEmptyParagraphDoc(transaction.before) &&
|
||||
isEmptyParagraphDoc(transaction.doc);
|
||||
if (!becameEmpty) return;
|
||||
|
||||
// The server reads the originating document from the connection, so the
|
||||
// payload only needs to declare intent — it cannot target another document.
|
||||
this.options.provider?.sendStateless(
|
||||
JSON.stringify({ type: INTENTIONAL_CLEAR_MESSAGE_TYPE }),
|
||||
);
|
||||
},
|
||||
});
|
||||
@@ -205,203 +205,31 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
|
||||
expect(historyQueue.add).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// #206 persist-6 / #248 — a momentarily-empty live Y.Doc must not overwrite
|
||||
// non-empty persisted content. The store-side empty-guard blocks an empty doc
|
||||
// (a client/agent glitch, a bad merge, an emptying transclusion) from wiping
|
||||
// the page silently when NO intentional-clear signal is present.
|
||||
it('does NOT overwrite non-empty content with a momentarily-empty live doc (persist-6)', async () => {
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
// #206 persist-6 — RED (it.failing): a momentarily-empty live Y.Doc must not
|
||||
// overwrite non-empty persisted content. `onStoreDocument` empty-guards the
|
||||
// LOAD path but not the STORE path, so today an empty doc (a client/agent
|
||||
// glitch, a bad merge, an emptying transclusion) is written straight over the
|
||||
// page and the content is wiped silently. A store-side empty-guard is a real
|
||||
// behaviour change (a deliberate "select-all + delete" is also empty), so it
|
||||
// is left UNFIXED pending a product decision; this documents the data-loss
|
||||
// path and flips to a normal passing test the moment the guard lands.
|
||||
it.failing(
|
||||
'does NOT overwrite non-empty content with a momentarily-empty live doc (persist-6)',
|
||||
async () => {
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
// The empty incoming doc is rejected and the rich page survives.
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// #248 — an empty-over-empty store is allowed (nothing to lose); the guard
|
||||
// only protects non-empty persisted content.
|
||||
it('allows an empty store over already-empty content (#248)', async () => {
|
||||
const liveEmptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(liveEmptyDoc);
|
||||
// Stored content is empty per isEmptyParagraphDoc (paragraph with content:[])
|
||||
// but NOT deep-equal to the normalized live doc, so the unchanged
|
||||
// short-circuit is skipped and the empty-guard is genuinely reached.
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: { type: 'doc', content: [{ type: 'paragraph', content: [] }] },
|
||||
});
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// #251 — REAL-PATH regression test. The intentional-clear signal is set via
|
||||
// the actual transport seam (ext.onStateless with the exact stateless payload
|
||||
// the client's IntentionalClear extension sends), NOT a hand-injected
|
||||
// context.intentionalClear poke. We then run the debounced store with an empty
|
||||
// live doc over non-empty persisted content and assert the empty write goes
|
||||
// through — i.e. the clear persists.
|
||||
it('persists an intentional clear signalled via the real stateless transport (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
// The client signalled a deliberate clear over the live connection.
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: false } as any,
|
||||
documentName,
|
||||
document: document as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
// The empty doc was written (the clear persisted). The persisted content is
|
||||
// the Y.Doc round-trip of the empty doc (attrs normalized), so compare
|
||||
// against fromYdoc rather than the raw literal.
|
||||
expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
|
||||
const expectedEmpty = TiptapTransformer.fromYdoc(document, 'default');
|
||||
expect(pageRepo.updatePage.mock.calls[0][0].content).toEqual(expectedEmpty);
|
||||
});
|
||||
|
||||
// #251 — retry correctness: a transient DB failure on the FIRST attempt must
|
||||
// not silently drop the clear. The intentional-clear flag is consumed ONCE
|
||||
// before the retry loop, so when attempt 1's updatePage throws (tx rolls back,
|
||||
// but the in-memory flag delete cannot roll back) the retry on attempt 2 still
|
||||
// sees the clear as allowed and writes the empty doc. On the pre-fix code
|
||||
// (consumeIntentionalClear called INSIDE the loop) attempt 1 consumed the flag,
|
||||
// attempt 2 re-read it as absent and the empty-guard BLOCKED the write — so
|
||||
// updatePage would be called once and the clear would be lost. This test fails
|
||||
// on that ordering and passes after the hoist.
|
||||
it('persists an intentional clear even when the first store attempt fails transiently (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
// The page stays non-empty in the DB across both attempts (the rolled-back
|
||||
// first attempt never changed it), exactly the failure scenario the WARNING
|
||||
// describes.
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
let attempts = 0;
|
||||
pageRepo.updatePage.mockImplementation(async () => {
|
||||
attempts += 1;
|
||||
if (attempts === 1) throw new Error('deadlock detected'); // transient
|
||||
callOrder.push('updatePage');
|
||||
});
|
||||
|
||||
// The client signalled a deliberate clear over the live connection.
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: false } as any,
|
||||
documentName,
|
||||
document: document as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
// First attempt failed and rolled back; the retry still honoured the clear
|
||||
// and wrote the empty doc (the clear survived the retry).
|
||||
expect(pageRepo.updatePage).toHaveBeenCalledTimes(2);
|
||||
const expectedEmpty = TiptapTransformer.fromYdoc(document, 'default');
|
||||
expect(pageRepo.updatePage.mock.calls[1][0].content).toEqual(expectedEmpty);
|
||||
});
|
||||
|
||||
// #251 — the signal is single-use: it is consumed by the first empty store,
|
||||
// so a SECOND accidental empty (no fresh signal) is still blocked.
|
||||
it('consumes the intentional-clear signal once; a later empty is blocked (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: false } as any,
|
||||
documentName,
|
||||
document: ydocFor(emptyDoc) as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
// First empty store consumes the signal and writes.
|
||||
await ext.onStoreDocument(buildData(ydocFor(emptyDoc), 'user') as any);
|
||||
expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Re-arm findById to non-empty (as if content came back) and fire another
|
||||
// empty store WITHOUT a new signal — the guard must block it.
|
||||
pageRepo.updatePage.mockClear();
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
await ext.onStoreDocument(buildData(ydocFor(emptyDoc), 'user') as any);
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// #251 — a read-only connection cannot arm the clear, so its empty store is
|
||||
// still blocked (defends the guard against a read-only spoof).
|
||||
it('ignores an intentional-clear signal from a read-only connection (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: true } as any,
|
||||
documentName,
|
||||
document: document as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// #251 — a non-empty store between the signal and the empty store drops the
|
||||
// pending flag ("cleared then retyped" can't leave a usable signal behind).
|
||||
it('drops a pending clear when a non-empty store intervenes (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: false } as any,
|
||||
documentName,
|
||||
document: ydocFor(emptyDoc) as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
// A non-empty store lands first → consumes/drops the stale flag.
|
||||
pageRepo.findById.mockResolvedValue(persistedHumanPage('NEW HUMAN TEXT'));
|
||||
await ext.onStoreDocument(
|
||||
buildData(ydocFor(doc('NEW HUMAN TEXT')), 'user') as any,
|
||||
);
|
||||
pageRepo.updatePage.mockClear();
|
||||
|
||||
// Now an empty store with no fresh signal must be blocked.
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
await ext.onStoreDocument(buildData(ydocFor(emptyDoc), 'user') as any);
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
});
|
||||
// Desired contract: the empty incoming doc is rejected and the rich page
|
||||
// survives. Today updatePage is called with the empty content (data loss).
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
// persist-1 — when every attempt fails the hook must NOT report a phantom
|
||||
// success: no "page.updated" badge broadcast and no history snapshot for
|
||||
|
||||
@@ -3,7 +3,6 @@ import {
|
||||
Extension,
|
||||
onChangePayload,
|
||||
onLoadDocumentPayload,
|
||||
onStatelessPayload,
|
||||
onStoreDocumentPayload,
|
||||
} from '@hocuspocus/server';
|
||||
import * as Y from 'yjs';
|
||||
@@ -42,35 +41,6 @@ import {
|
||||
} from '../constants';
|
||||
import { TransclusionService } from '../../core/page/transclusion/transclusion.service';
|
||||
|
||||
/**
|
||||
* #251 — wire format of the client→server stateless message that signals a
|
||||
* deliberate page clear. The client (IntentionalClear editor extension) sends
|
||||
* `{ type: INTENTIONAL_CLEAR_MESSAGE_TYPE }`; the document is taken from the
|
||||
* connection, not the payload, so the signal cannot be aimed at another page.
|
||||
*/
|
||||
export const INTENTIONAL_CLEAR_MESSAGE_TYPE = 'intentional-clear';
|
||||
|
||||
/**
|
||||
* #251 — how long an intentional-clear signal stays "pending" before it is
|
||||
* ignored. The signal is set on the clearing keystroke but consumed by the
|
||||
* DEBOUNCED onStoreDocument, so the TTL must comfortably exceed the collab
|
||||
* store debounce window (hocuspocus is configured with maxDebounce = 45s in
|
||||
* collaboration.gateway.ts). 60s leaves a margin while keeping the window for a
|
||||
* stale flag small; on top of the TTL, any non-empty store immediately drops a
|
||||
* pending flag (see onStoreDocument), so a "cleared then retyped" sequence can
|
||||
* never leave a usable flag behind.
|
||||
*
|
||||
* Known fail-safe limitation: the flag lives only in this node's process memory.
|
||||
* If document ownership transfers to another node, or this node crashes/restarts,
|
||||
* between the stateless signal (set on node A) and the debounced store, the
|
||||
* in-memory flag is lost and the clear is silently NOT applied — the store-side
|
||||
* empty-guard then reloads the document non-empty from the DB. This is
|
||||
* deliberately fail-safe (a lost flag preserves content rather than destroying
|
||||
* it), but it is a documented limitation, not a guarantee that every deliberate
|
||||
* clear survives a node handoff.
|
||||
*/
|
||||
export const INTENTIONAL_CLEAR_TTL_MS = 60_000;
|
||||
|
||||
/**
|
||||
* Resolve the provenance source for a coalesced snapshot.
|
||||
*
|
||||
@@ -126,13 +96,6 @@ export class PersistenceExtension implements Extension {
|
||||
// coalescing window" per document and OR it across all edits in the window,
|
||||
// so the snapshot is marked 'agent' regardless of who wrote last.
|
||||
private agentTouched: Map<string, boolean> = new Map();
|
||||
// #251 — per-document "intentional clear pending" flags. Keyed by
|
||||
// documentName, value = expiry timestamp (ms). Set by onStateless when the
|
||||
// client reports a deliberate clear; consumed once by the next
|
||||
// onStoreDocument empty-guard branch. This is the per-EDIT channel the
|
||||
// per-connection context cannot provide (a clear is an edit event, but the
|
||||
// store is debounced and connection context is fixed at authentication).
|
||||
private intentionalClear: Map<string, number> = new Map();
|
||||
|
||||
constructor(
|
||||
private readonly pageRepo: PageRepo,
|
||||
@@ -217,19 +180,6 @@ export class PersistenceExtension implements Extension {
|
||||
this.consumeAgentTouched(documentName),
|
||||
context?.actor,
|
||||
);
|
||||
// #251 — consume the intentional-clear flag ONCE, BEFORE the retry loop
|
||||
// (like consumeContributors / consumeAgentTouched above). consumeIntentional-
|
||||
// Clear ALWAYS deletes the in-memory Map entry, but a tx rollback cannot
|
||||
// un-delete it. Calling it INSIDE the loop meant: a clear armed for attempt 1
|
||||
// was consumed there, attempt 1's updatePage threw a transient error and
|
||||
// rolled back, then attempt 2 re-read non-empty content and saw the flag
|
||||
// already gone — silently downgrading the retry into a BLOCKED write, so the
|
||||
// user's deliberate clear was dropped. Hoisting makes the decision stable
|
||||
// across every attempt. This single call also preserves the "a non-empty
|
||||
// store drops a pending flag" semantics (the cleared-then-retyped case):
|
||||
// every store consumes the flag here regardless of incoming emptiness, so a
|
||||
// subsequent non-empty store can never leave a usable flag behind.
|
||||
const allowIntentionalClear = this.consumeIntentionalClear(documentName);
|
||||
|
||||
// Persist with a small bounded retry. The in-memory Y.Doc is the ONLY copy
|
||||
// of the latest edit until this hook returns: hocuspocus destroys/unloads the
|
||||
@@ -260,46 +210,6 @@ export class PersistenceExtension implements Extension {
|
||||
return;
|
||||
}
|
||||
|
||||
// #206 persist-6 / #248 — store-side empty-guard. A momentarily-empty
|
||||
// live Y.Doc (a client/agent glitch, a bad merge, a transclusion that
|
||||
// emptied) must NOT overwrite non-empty persisted content. The LOAD
|
||||
// path already guards emptiness (onLoadDocument only hydrates from db
|
||||
// when the live doc isEmpty); the STORE path did not, so an empty
|
||||
// serialization was written straight over the page, wiping it
|
||||
// silently.
|
||||
//
|
||||
// #251 — the ONE legitimate empty-over-non-empty write is a user who
|
||||
// deliberately clears the page. That intent arrives out-of-band as a
|
||||
// stateless message, NOT from the doc content, which is why it cannot
|
||||
// be spoofed for non-clear writes: the flag is only ever read on this
|
||||
// empty-incoming branch, so the worst a forged signal can do is clear
|
||||
// a page the connection may already edit. The flag was consumed ONCE
|
||||
// before the retry loop (`allowIntentionalClear`) so the decision is
|
||||
// stable across retries; a non-empty store still drops any pending
|
||||
// flag via that same hoisted consume (a "cleared then retyped"
|
||||
// sequence can't leave a usable one behind).
|
||||
const incomingEmpty = isEmptyParagraphDoc(tiptapJson as any);
|
||||
if (
|
||||
incomingEmpty &&
|
||||
page.content &&
|
||||
!isEmptyParagraphDoc(page.content as any)
|
||||
) {
|
||||
if (allowIntentionalClear) {
|
||||
this.logger.debug(
|
||||
`Intentional clear for ${pageId}: persisting empty doc over ` +
|
||||
`non-empty content (user-signalled)`,
|
||||
);
|
||||
// fall through — the empty write is allowed exactly once.
|
||||
} else {
|
||||
this.logger.warn(
|
||||
`Skipping store for ${pageId}: empty live doc would overwrite ` +
|
||||
`non-empty persisted content`,
|
||||
);
|
||||
page = null;
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
let contributorIds = undefined;
|
||||
try {
|
||||
const existingContributors = page.contributorIds || [];
|
||||
@@ -435,37 +345,6 @@ export class PersistenceExtension implements Extension {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* #251 — receive the client's deliberate-clear signal. Records a short-lived,
|
||||
* single-use pending flag for the originating document so the next
|
||||
* onStoreDocument may let one empty-over-non-empty write through the guard.
|
||||
*
|
||||
* Hardening: read-only connections cannot arm the flag, and the document is
|
||||
* taken from the connection (`data.documentName`), never the payload, so a
|
||||
* client cannot target a page it isn't editing. The flag only ever RELAXES
|
||||
* the guard for an empty write (a clear); it can never force or alter a
|
||||
* non-empty write, so it is not a guard bypass for normal content.
|
||||
*/
|
||||
async onStateless(data: onStatelessPayload) {
|
||||
const { connection, documentName, payload } = data;
|
||||
|
||||
if (connection?.readOnly) return;
|
||||
|
||||
let message: { type?: string } | undefined;
|
||||
try {
|
||||
message = JSON.parse(payload);
|
||||
} catch {
|
||||
return; // unrelated / malformed stateless message
|
||||
}
|
||||
|
||||
if (message?.type !== INTENTIONAL_CLEAR_MESSAGE_TYPE) return;
|
||||
|
||||
this.intentionalClear.set(
|
||||
documentName,
|
||||
Date.now() + INTENTIONAL_CLEAR_TTL_MS,
|
||||
);
|
||||
}
|
||||
|
||||
async onChange(data: onChangePayload) {
|
||||
const documentName = data.documentName;
|
||||
const userId = data.context?.user?.id;
|
||||
@@ -489,7 +368,6 @@ export class PersistenceExtension implements Extension {
|
||||
const documentName = data.documentName;
|
||||
this.contributors.delete(documentName);
|
||||
this.agentTouched.delete(documentName);
|
||||
this.intentionalClear.delete(documentName);
|
||||
}
|
||||
|
||||
private consumeContributors(documentName: string): string[] {
|
||||
@@ -507,18 +385,6 @@ export class PersistenceExtension implements Extension {
|
||||
return touched;
|
||||
}
|
||||
|
||||
/**
|
||||
* #251 — read and clear the intentional-clear flag for this document. Returns
|
||||
* true only if a flag was pending AND still within its TTL. Always deletes the
|
||||
* entry so the signal is strictly single-use (one clear → one allowed empty
|
||||
* write); an expired flag is treated as absent (guard still blocks).
|
||||
*/
|
||||
private consumeIntentionalClear(documentName: string): boolean {
|
||||
const expiry = this.intentionalClear.get(documentName);
|
||||
this.intentionalClear.delete(documentName);
|
||||
return expiry !== undefined && Date.now() < expiry;
|
||||
}
|
||||
|
||||
private async enqueuePageHistory(
|
||||
page: Page,
|
||||
lastUpdatedSource: string,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { Logger } from '@nestjs/common';
|
||||
import { IoAdapter } from '@nestjs/platform-socket.io';
|
||||
import { ServerOptions } from 'socket.io';
|
||||
import { createAdapter } from '@socket.io/redis-adapter';
|
||||
@@ -9,8 +10,11 @@ import {
|
||||
} from '../../common/helpers';
|
||||
|
||||
export class WsRedisIoAdapter extends IoAdapter {
|
||||
private readonly logger = new Logger(WsRedisIoAdapter.name);
|
||||
private adapterConstructor: ReturnType<typeof createAdapter>;
|
||||
private redisConfig: RedisConfig;
|
||||
private pubClient: Redis;
|
||||
private subClient: Redis;
|
||||
|
||||
async connectToRedis(): Promise<void> {
|
||||
this.redisConfig = parseRedisUrl(process.env.REDIS_URL);
|
||||
@@ -23,8 +27,13 @@ export class WsRedisIoAdapter extends IoAdapter {
|
||||
const pubClient = new Redis(process.env.REDIS_URL, options);
|
||||
const subClient = new Redis(process.env.REDIS_URL, options);
|
||||
|
||||
pubClient.on('error', (err) => () => {});
|
||||
subClient.on('error', (err) => () => {});
|
||||
pubClient.on('error', (err) => this.logger.error('socket.io redis pub client error', err));
|
||||
subClient.on('error', (err) => this.logger.error('socket.io redis sub client error', err));
|
||||
|
||||
// Hold references so the pub/sub connections can be torn down on shutdown
|
||||
// (see dispose()); otherwise these ioredis sockets leak as active handles.
|
||||
this.pubClient = pubClient;
|
||||
this.subClient = subClient;
|
||||
|
||||
this.adapterConstructor = createAdapter(pubClient, subClient);
|
||||
}
|
||||
@@ -34,4 +43,26 @@ export class WsRedisIoAdapter extends IoAdapter {
|
||||
server.adapter(this.adapterConstructor);
|
||||
return server;
|
||||
}
|
||||
|
||||
/**
|
||||
* Called once by Nest's SocketModule during application shutdown, after every
|
||||
* socket.io server has been closed. The @socket.io/redis-adapter never owns
|
||||
* the lifecycle of the ioredis pub/sub clients it is handed, so we close them
|
||||
* here to avoid leaking their TCP handles on shutdown (see issue #255).
|
||||
*
|
||||
* Uses disconnect(false) to mirror the sibling pub/sub pair in
|
||||
* collaboration/extensions/redis-sync (redis-sync.extension.ts onDestroy):
|
||||
* an immediate close with no graceful QUIT round-trip and no auto-reconnect,
|
||||
* which is what we want for idle adapter clients during teardown.
|
||||
*/
|
||||
async dispose(): Promise<void> {
|
||||
await super.dispose();
|
||||
|
||||
// dispose() is invoked once per shutdown; null the refs so a second call
|
||||
// (or any post-shutdown path) cannot act on already-closed clients.
|
||||
this.pubClient?.disconnect(false);
|
||||
this.subClient?.disconnect(false);
|
||||
this.pubClient = undefined;
|
||||
this.subClient = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user