Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| e609832ae4 | |||
| ee03da4018 | |||
| 28251b1e08 |
@@ -1,3 +1,10 @@
|
||||
export const HISTORY_INTERVAL = 5 * 60 * 1000;
|
||||
export const HISTORY_FAST_INTERVAL = 60 * 1000;
|
||||
export const HISTORY_FAST_THRESHOLD = 5 * 60 * 1000;
|
||||
|
||||
// #348 — debounce window for the per-page RAG re-embed job. Repeated saves
|
||||
// within this window collapse to a single delayed job (coalesced by a stable
|
||||
// jobId), so active editing does not pile up expensive re-embeds (external API
|
||||
// + page_embeddings rewrite, concurrency 1). The worker reads the CURRENT page
|
||||
// state at run time, so the last content within the window wins.
|
||||
export const EMBED_DEBOUNCE_MS = 30 * 1000;
|
||||
|
||||
@@ -431,7 +431,17 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
|
||||
it('uses the canonical page.id (not the slugId doc name) for post-store side effects (#260)', async () => {
|
||||
const SLUG = 'slug-1'; // persistedHumanPage.slugId; findById resolves it
|
||||
const document = ydocFor(doc('NEW AGENT CONTENT'));
|
||||
pageRepo.findById.mockResolvedValue(persistedHumanPage('NEW AGENT CONTENT'));
|
||||
// #348 — the transclusion sync now runs only when the new OR the previously
|
||||
// persisted content carries a transclusion-family node. Give the persisted
|
||||
// (old) content a pageEmbed so the sync path is exercised and the #260
|
||||
// UUID-vs-slugId contract asserted below is still verified.
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('NEW AGENT CONTENT'),
|
||||
content: {
|
||||
type: 'doc',
|
||||
content: [{ type: 'pageEmbed', attrs: { sourcePageId: 'src-1' } }],
|
||||
},
|
||||
});
|
||||
pageHistoryRepo.findPageLastHistory.mockResolvedValue(null);
|
||||
|
||||
// A `page.<slugId>` document name (the bug's smoking gun), agent store over
|
||||
|
||||
@@ -36,6 +36,7 @@ import {
|
||||
import { Page } from '@docmost/db/types/entity.types';
|
||||
import { CollabHistoryService } from '../services/collab-history.service';
|
||||
import {
|
||||
EMBED_DEBOUNCE_MS,
|
||||
HISTORY_FAST_INTERVAL,
|
||||
HISTORY_FAST_THRESHOLD,
|
||||
HISTORY_INTERVAL,
|
||||
@@ -45,6 +46,7 @@ import {
|
||||
observeCollabLoad,
|
||||
observeCollabStore,
|
||||
} from '../../integrations/metrics/metrics.registry';
|
||||
import { hasTransclusionFamilyNodes } from '../../core/page/transclusion/utils/transclusion-prosemirror.util';
|
||||
|
||||
/**
|
||||
* #251 — wire format of the client→server stateless message that signals a
|
||||
@@ -450,7 +452,18 @@ export class PersistenceExtension implements Extension {
|
||||
// Use the canonical page UUID (page.id), not the doc-name id, which may be
|
||||
// a slugId for a `page.<slugId>` doc (#260). The transclusion/reference
|
||||
// syncs write uuid-typed columns, so a slugId here threw Postgres 22P02.
|
||||
await this.syncTransclusion(page.id, page.workspaceId, tiptapJson);
|
||||
//
|
||||
// #348 — skip the three sync SELECTs when neither the new content nor the
|
||||
// previously-persisted content has any transclusion/reference/pageEmbed
|
||||
// node: nothing to insert, and (the DB mirrors the old content) nothing to
|
||||
// delete. Whenever either side has one, run the idempotent sync exactly as
|
||||
// before so removals are still reconciled.
|
||||
if (
|
||||
hasTransclusionFamilyNodes(tiptapJson) ||
|
||||
hasTransclusionFamilyNodes(page.content)
|
||||
) {
|
||||
await this.syncTransclusion(page.id, page.workspaceId, tiptapJson);
|
||||
}
|
||||
}
|
||||
|
||||
if (page) {
|
||||
@@ -466,7 +479,17 @@ export class PersistenceExtension implements Extension {
|
||||
(m) => m.entityId,
|
||||
);
|
||||
|
||||
if (userMentions.length > 0) {
|
||||
// #348 — only enqueue when the mentioned-user set actually GAINED a member.
|
||||
// The processor (processPageMention) already no-ops when every current
|
||||
// mention was present before (newMentions.length === 0), so skipping the
|
||||
// enqueue in that case is behavior-identical and avoids piling up no-op jobs
|
||||
// on every save of a page that merely CONTAINS (unchanged) mentions.
|
||||
const oldMentionedUserIdSet = new Set(oldMentionedUserIds);
|
||||
const hasNewMentionedUser = userMentions.some(
|
||||
(m) => !oldMentionedUserIdSet.has(m.entityId),
|
||||
);
|
||||
|
||||
if (hasNewMentionedUser) {
|
||||
await this.notificationQueue.add(QueueJob.PAGE_MENTION_NOTIFICATION, {
|
||||
userMentions: userMentions.map((m) => ({
|
||||
userId: m.entityId,
|
||||
@@ -481,12 +504,23 @@ export class PersistenceExtension implements Extension {
|
||||
} as IPageMentionNotificationJob);
|
||||
}
|
||||
|
||||
await this.aiQueue.add(QueueJob.PAGE_CONTENT_UPDATED, {
|
||||
// Canonical UUID: the embedding reindex resolves pages by uuid, so a
|
||||
// slugId here threw Postgres 22P02 invalid-uuid (#260).
|
||||
pageIds: [page.id],
|
||||
workspaceId: page.workspaceId,
|
||||
});
|
||||
await this.aiQueue.add(
|
||||
QueueJob.PAGE_CONTENT_UPDATED,
|
||||
{
|
||||
// Canonical UUID: the embedding reindex resolves pages by uuid, so a
|
||||
// slugId here threw Postgres 22P02 invalid-uuid (#260).
|
||||
pageIds: [page.id],
|
||||
workspaceId: page.workspaceId,
|
||||
},
|
||||
// #348 — coalesce re-embeds during active editing. A stable per-page
|
||||
// jobId + delay means repeated saves within EMBED_DEBOUNCE_MS collapse
|
||||
// to one delayed job instead of one expensive re-embed per save. The
|
||||
// worker reads the current page state at run time, so last content wins.
|
||||
// BullMQ forbids ':' in custom job ids (Redis key separator), so '-' is
|
||||
// used; page.id is a UUID, so the id is unique per page. removeOnComplete
|
||||
// (queue.module) frees the id after each run so the next window re-arms.
|
||||
{ jobId: `embed-${page.id}`, delay: EMBED_DEBOUNCE_MS },
|
||||
);
|
||||
|
||||
await this.enqueuePageHistory(page, lastUpdatedSource);
|
||||
}
|
||||
|
||||
@@ -220,6 +220,13 @@ export class RedisSyncExtension<TCE extends CustomEvents> implements Extension {
|
||||
};
|
||||
|
||||
async maintainLock(documentName: string) {
|
||||
// #348 — clear any existing timer for this document before installing a new
|
||||
// one. Without this, a second maintainLock for the same document (a
|
||||
// reload-without-unload) overwrites this.locks[documentName] and leaks the
|
||||
// previous interval, which keeps firing SET forever with no way to clear it.
|
||||
if (this.locks[documentName]) {
|
||||
clearInterval(this.locks[documentName]);
|
||||
}
|
||||
this.locks[documentName] = setInterval(() => {
|
||||
this.pub.set(
|
||||
this.getKey(documentName),
|
||||
|
||||
@@ -4,8 +4,21 @@ export const CacheKey = {
|
||||
`perm:space-roles:${userId}:${spaceId}`,
|
||||
PAGE_CAN_EDIT: (userId: string, pageId: string) =>
|
||||
`perm:can-edit:${userId}:${pageId}`,
|
||||
// #348 — DomainMiddleware workspace resolution. Self-hosted resolves the single
|
||||
// workspace (constant key); cloud resolves by the request subdomain (lowercased
|
||||
// to match the case-insensitive `LOWER(hostname)` lookup). Every WorkspaceRepo
|
||||
// mutator busts these, so staleness is bounded by both explicit invalidation and
|
||||
// the short TTL below.
|
||||
WORKSPACE_SELF_HOSTED: 'workspace:self-hosted',
|
||||
WORKSPACE_BY_HOST: (subdomain: string) =>
|
||||
`workspace:byhost:${subdomain.toLowerCase()}`,
|
||||
};
|
||||
|
||||
// Permission caches dedupe repeated checks within and across short request bursts.
|
||||
// 5s keeps staleness on revocations bounded.
|
||||
export const PERMISSION_CACHE_TTL_MS = 5_000;
|
||||
|
||||
// #348 — workspace row changes rarely; a short TTL bounds staleness of
|
||||
// security-relevant fields (enforceSso/enforceMfa/status) even if an explicit
|
||||
// bust is ever missed, while still removing the per-request workspace query.
|
||||
export const WORKSPACE_CACHE_TTL_MS = 15_000;
|
||||
|
||||
@@ -1,13 +1,42 @@
|
||||
import { Injectable, NestMiddleware, NotFoundException } from '@nestjs/common';
|
||||
import { Inject, Injectable, NestMiddleware } from '@nestjs/common';
|
||||
import { FastifyRequest, FastifyReply } from 'fastify';
|
||||
import { CACHE_MANAGER } from '@nestjs/cache-manager';
|
||||
import { Cache } from 'cache-manager';
|
||||
import { EnvironmentService } from '../../integrations/environment/environment.service';
|
||||
import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo';
|
||||
import { Workspace } from '@docmost/db/types/entity.types';
|
||||
import { withCache } from '../helpers/with-cache';
|
||||
import { CacheKey, WORKSPACE_CACHE_TTL_MS } from '../helpers/cache-keys';
|
||||
|
||||
// #348 — timestamptz columns on the workspace row. The cache store (Keyv/Redis)
|
||||
// JSON-serializes values, so a cached workspace comes back with these fields as
|
||||
// ISO strings. Reviving them to Date keeps the cached path byte-identical to the
|
||||
// direct DB path (postgres.js returns Date), so nothing downstream can observe a
|
||||
// cache hit vs miss. Idempotent: `new Date(date)` on an already-Date value is a
|
||||
// no-op-equivalent. Keep in sync with the workspace timestamptz columns.
|
||||
const WORKSPACE_DATE_FIELDS: Array<keyof Workspace> = [
|
||||
'createdAt',
|
||||
'updatedAt',
|
||||
'deletedAt',
|
||||
'trialEndAt',
|
||||
];
|
||||
|
||||
function reviveWorkspaceDates(workspace: Workspace): Workspace {
|
||||
for (const field of WORKSPACE_DATE_FIELDS) {
|
||||
const value = workspace[field];
|
||||
if (value != null) {
|
||||
(workspace as any)[field] = new Date(value as any);
|
||||
}
|
||||
}
|
||||
return workspace;
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class DomainMiddleware implements NestMiddleware {
|
||||
constructor(
|
||||
private workspaceRepo: WorkspaceRepo,
|
||||
private environmentService: EnvironmentService,
|
||||
@Inject(CACHE_MANAGER) private readonly cacheManager: Cache,
|
||||
) {}
|
||||
async use(
|
||||
req: FastifyRequest['raw'],
|
||||
@@ -15,13 +44,21 @@ export class DomainMiddleware implements NestMiddleware {
|
||||
next: () => void,
|
||||
) {
|
||||
if (this.environmentService.isSelfHosted()) {
|
||||
const workspace = await this.workspaceRepo.findFirst();
|
||||
// #348 — cache the single-workspace lookup that runs on every request.
|
||||
// Invalidated by every WorkspaceRepo mutator (see bustWorkspaceCache).
|
||||
const workspace = await withCache(
|
||||
this.cacheManager,
|
||||
CacheKey.WORKSPACE_SELF_HOSTED,
|
||||
WORKSPACE_CACHE_TTL_MS,
|
||||
() => this.workspaceRepo.findFirst(),
|
||||
);
|
||||
if (!workspace) {
|
||||
//throw new NotFoundException('Workspace not found');
|
||||
(req as any).workspaceId = null;
|
||||
return next();
|
||||
}
|
||||
|
||||
reviveWorkspaceDates(workspace);
|
||||
// TODO: unify
|
||||
(req as any).workspaceId = workspace.id;
|
||||
(req as any).workspace = workspace;
|
||||
@@ -29,13 +66,21 @@ export class DomainMiddleware implements NestMiddleware {
|
||||
const header = req.headers.host;
|
||||
const subdomain = header.split('.')[0];
|
||||
|
||||
const workspace = await this.workspaceRepo.findByHostname(subdomain);
|
||||
// #348 — cache per-subdomain workspace resolution. Keyed by subdomain (the
|
||||
// hostname column); busted per hostname by every WorkspaceRepo mutator.
|
||||
const workspace = await withCache(
|
||||
this.cacheManager,
|
||||
CacheKey.WORKSPACE_BY_HOST(subdomain),
|
||||
WORKSPACE_CACHE_TTL_MS,
|
||||
() => this.workspaceRepo.findByHostname(subdomain),
|
||||
);
|
||||
|
||||
if (!workspace) {
|
||||
(req as any).workspaceId = null;
|
||||
return next();
|
||||
}
|
||||
|
||||
reviveWorkspaceDates(workspace);
|
||||
(req as any).workspaceId = workspace.id;
|
||||
(req as any).workspace = workspace;
|
||||
}
|
||||
|
||||
@@ -51,7 +51,21 @@ export class JwtStrategy extends PassportStrategy(Strategy, 'jwt') {
|
||||
throw new UnauthorizedException();
|
||||
}
|
||||
|
||||
const workspace = await this.workspaceRepo.findById(payload.workspaceId);
|
||||
// #348 — reuse the workspace DomainMiddleware already loaded for this request
|
||||
// instead of re-querying it. `validate()` above has confirmed
|
||||
// `req.raw.workspaceId === payload.workspaceId` (or that it is unset), and the
|
||||
// middleware sets `req.raw.workspace` alongside `req.raw.workspaceId` from the
|
||||
// SAME workspace row, so when the ids match this is that row. NOTE it is the
|
||||
// middleware's `selectAll` object (a superset of the fallback `findById` base
|
||||
// fields — it also carries licenseKey/auditRetentionDays); that is harmless
|
||||
// here because every consumer reads this workspace via the AuthWorkspace
|
||||
// decorator, which already preferred `req.raw.workspace` (the selectAll object)
|
||||
// over `req.user.workspace` before this change. Fall back to the query if the
|
||||
// middleware did not populate it (a path that bypasses DomainMiddleware).
|
||||
const workspace =
|
||||
req.raw.workspace && req.raw.workspaceId === payload.workspaceId
|
||||
? req.raw.workspace
|
||||
: await this.workspaceRepo.findById(payload.workspaceId);
|
||||
|
||||
if (!workspace) {
|
||||
throw new UnauthorizedException();
|
||||
|
||||
@@ -38,6 +38,8 @@ export class FavoriteService {
|
||||
await this.pagePermissionRepo.filterAccessiblePageIds({
|
||||
pageIds: result.items,
|
||||
userId,
|
||||
// #348 — favorites load at app-start; enable the workspace short-circuit.
|
||||
workspaceId,
|
||||
});
|
||||
const accessibleSet = new Set(accessibleIds);
|
||||
result.items = result.items.filter((id) => accessibleSet.has(id));
|
||||
@@ -125,6 +127,8 @@ export class FavoriteService {
|
||||
await this.pagePermissionRepo.filterAccessiblePageIds({
|
||||
pageIds,
|
||||
userId,
|
||||
// #348 — workspace-level short-circuit for the favorites list.
|
||||
workspaceId,
|
||||
});
|
||||
accessiblePageSet = new Set(accessibleIds);
|
||||
}
|
||||
|
||||
@@ -23,7 +23,12 @@ export class NotificationController {
|
||||
@Body() dto: ListNotificationsDto,
|
||||
@AuthUser() user: User,
|
||||
) {
|
||||
return this.notificationService.findByUserId(user.id, dto, dto.type);
|
||||
return this.notificationService.findByUserId(
|
||||
user.id,
|
||||
dto,
|
||||
dto.type,
|
||||
user.workspaceId,
|
||||
);
|
||||
}
|
||||
|
||||
@HttpCode(HttpStatus.OK)
|
||||
|
||||
@@ -45,6 +45,7 @@ export class NotificationService {
|
||||
userId: string,
|
||||
pagination: PaginationOptions,
|
||||
type: NotificationTab = 'all',
|
||||
workspaceId?: string | null,
|
||||
) {
|
||||
const result = await this.notificationRepo.findByUserId(
|
||||
userId,
|
||||
@@ -61,6 +62,8 @@ export class NotificationService {
|
||||
await this.pagePermissionRepo.filterAccessiblePageIds({
|
||||
pageIds,
|
||||
userId,
|
||||
// #348 — notifications list; enable the workspace short-circuit.
|
||||
workspaceId,
|
||||
});
|
||||
const accessibleSet = new Set(accessiblePageIds);
|
||||
|
||||
|
||||
@@ -446,7 +446,11 @@ export class PageController {
|
||||
);
|
||||
}
|
||||
|
||||
return this.pageService.getRecentPages(user.id, pagination);
|
||||
return this.pageService.getRecentPages(
|
||||
user.id,
|
||||
pagination,
|
||||
user.workspaceId,
|
||||
);
|
||||
}
|
||||
|
||||
@HttpCode(HttpStatus.OK)
|
||||
@@ -469,7 +473,13 @@ export class PageController {
|
||||
}
|
||||
}
|
||||
|
||||
return this.pageService.getCreatedByPages(targetUserId, user.id, pagination, dto.spaceId);
|
||||
return this.pageService.getCreatedByPages(
|
||||
targetUserId,
|
||||
user.id,
|
||||
pagination,
|
||||
dto.spaceId,
|
||||
user.workspaceId,
|
||||
);
|
||||
}
|
||||
|
||||
@HttpCode(HttpStatus.OK)
|
||||
|
||||
@@ -1165,6 +1165,7 @@ export class PageService {
|
||||
async getRecentPages(
|
||||
userId: string,
|
||||
pagination: PaginationOptions,
|
||||
workspaceId?: string | null,
|
||||
): Promise<CursorPaginationResult<Page>> {
|
||||
const result = await this.pageRepo.getRecentPages(userId, pagination);
|
||||
|
||||
@@ -1174,6 +1175,8 @@ export class PageService {
|
||||
await this.pagePermissionRepo.filterAccessiblePageIds({
|
||||
pageIds,
|
||||
userId,
|
||||
// #348 — cross-space "recent"; enable the workspace short-circuit.
|
||||
workspaceId,
|
||||
});
|
||||
const accessibleSet = new Set(accessibleIds);
|
||||
result.items = result.items.filter((p) => accessibleSet.has(p.id));
|
||||
@@ -1187,6 +1190,7 @@ export class PageService {
|
||||
requestingUserId: string,
|
||||
pagination: PaginationOptions,
|
||||
spaceId?: string,
|
||||
workspaceId?: string | null,
|
||||
): Promise<CursorPaginationResult<Page>> {
|
||||
const result = await this.pageRepo.getCreatedByPages(
|
||||
creatorId,
|
||||
@@ -1201,6 +1205,9 @@ export class PageService {
|
||||
await this.pagePermissionRepo.filterAccessiblePageIds({
|
||||
pageIds,
|
||||
userId: requestingUserId,
|
||||
spaceId,
|
||||
// #348 — enable the workspace short-circuit when not space-scoped.
|
||||
workspaceId,
|
||||
});
|
||||
const accessibleSet = new Set(accessibleIds);
|
||||
result.items = result.items.filter((p) => accessibleSet.has(p.id));
|
||||
|
||||
@@ -93,6 +93,41 @@ function collectNodes<T>(
|
||||
return Array.from(byKey.values());
|
||||
}
|
||||
|
||||
/**
|
||||
* #348 — cheap early-exit probe: does this doc contain ANY node the transclusion
|
||||
* syncs care about (`transclusionSource` / `transclusionReference` / `pageEmbed`)?
|
||||
* Lets the collab store skip the three sync SELECTs when neither the previous nor
|
||||
* the new content has any such node — there is nothing to insert, and (since the
|
||||
* DB mirrors the previously-persisted content) nothing to delete. Walks once and
|
||||
* short-circuits on the first match; uses the same depth ceiling as the
|
||||
* collectors. Deliberately does NOT skip `transclusionSource` subtrees: it only
|
||||
* answers "any node present?", so descending everywhere is strictly conservative
|
||||
* (it can never wrongly report "none").
|
||||
*/
|
||||
export function hasTransclusionFamilyNodes(doc: unknown): boolean {
|
||||
const visit = (node: any, depth: number): boolean => {
|
||||
if (!node || typeof node !== 'object') return false;
|
||||
if (depth > MAX_PM_WALK_DEPTH) return false;
|
||||
|
||||
if (
|
||||
node.type === TRANSCLUSION_TYPE ||
|
||||
node.type === REFERENCE_TYPE ||
|
||||
node.type === PAGE_EMBED_TYPE
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (Array.isArray(node.content)) {
|
||||
for (const child of node.content) {
|
||||
if (visit(child, depth + 1)) return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
};
|
||||
|
||||
return visit(doc, 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Walks a ProseMirror JSON document and returns one snapshot per top-level
|
||||
* `transclusion` node. Does not recurse into transclusions (schema disallows
|
||||
|
||||
@@ -155,6 +155,8 @@ export class SearchService {
|
||||
pageIds,
|
||||
userId: opts.userId,
|
||||
spaceId: searchParams.spaceId,
|
||||
// #348 — enables the workspace-level short-circuit when not space-scoped.
|
||||
workspaceId: opts.workspaceId,
|
||||
});
|
||||
const accessibleSet = new Set(accessibleIds);
|
||||
results = results.filter((r: any) => accessibleSet.has(r.id));
|
||||
@@ -266,6 +268,8 @@ export class SearchService {
|
||||
await this.pagePermissionRepo.filterAccessiblePageIds({
|
||||
pageIds,
|
||||
userId,
|
||||
// #348 — workspace-level short-circuit for the suggest path.
|
||||
workspaceId,
|
||||
});
|
||||
const accessibleSet = new Set(accessibleIds);
|
||||
pages = pages.filter((p) => accessibleSet.has(p.id));
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
import { type Kysely, sql } from 'kysely';
|
||||
|
||||
/**
|
||||
* #348 — targeted hot-path indexes.
|
||||
*
|
||||
* 1. GIN trigram indexes for `/search/suggest`. That endpoint runs a
|
||||
* leading-wildcard `LOWER(f_unaccent(col)) LIKE '%q%'` per keystroke, which
|
||||
* is a sequential scan without a trigram index. The index EXPRESSIONS below
|
||||
* are `LOWER(f_unaccent(title|name))`, matching the predicates in
|
||||
* search.service.ts exactly so the planner uses them (verified with EXPLAIN:
|
||||
* the suggest predicate resolves to a Bitmap Index Scan on these indexes).
|
||||
*
|
||||
* IMMUTABLE-wrapper fix (required for the index to build): `f_unaccent` was
|
||||
* defined as `SELECT unaccent('unaccent', $1)` (the two-arg, dictionary-named
|
||||
* unaccent). That body CANNOT be used in an index expression: when Postgres
|
||||
* inlines the IMMUTABLE SQL wrapper while building the index it fails to
|
||||
* resolve the two-arg call (`function unaccent(unknown, text) does not exist`,
|
||||
* the `'unaccent'` literal loses its regdictionary coercion). The single-arg
|
||||
* `unaccent($1)` is the same operation (the default text-search dictionary IS
|
||||
* `unaccent`; verified byte-equal on accented samples), and — crucially —
|
||||
* SCHEMA-QUALIFIED as `public.unaccent($1)` it inlines cleanly, so the index
|
||||
* builds. We therefore `CREATE OR REPLACE` `f_unaccent` to the qualified
|
||||
* single-arg body. This is output-identical for every existing caller (the
|
||||
* tsvector trigger, the main `tsv @@` search, and the suggest LIKE), so no
|
||||
* reindex/backfill is needed; `down()` restores the original two-arg body.
|
||||
* (The `unaccent` extension is installed in `public` in this codebase, which
|
||||
* is why `public.unaccent` is the correct qualification.)
|
||||
*
|
||||
* 2. Composite indexes for two ORDER-BY-only-on-id queries that currently sort
|
||||
* on top of a created_at index:
|
||||
* - page_history: `findPageHistoryByPageId` does WHERE page_id ORDER BY id
|
||||
* DESC, but only `(page_id, created_at DESC)` exists → extra sort.
|
||||
* - comments: `findPageComments` does WHERE page_id ORDER BY id ASC, but only
|
||||
* `(page_id)` exists → extra sort.
|
||||
*
|
||||
* DEPLOY-TIME LOCK WARNING: these are plain (non-CONCURRENT) CREATE INDEX
|
||||
* statements — CONCURRENTLY is impossible because Kysely runs each migration in a
|
||||
* transaction. They take a SHARE lock that BLOCKS writes (INSERT/UPDATE/DELETE) on
|
||||
* pages/users/groups/comments/page_history for the duration of the build. The two
|
||||
* GIN trigram builds on pages.title / users.name are the slow ones and can take
|
||||
* minutes on a large tenant → a write-outage window during the deploy migration.
|
||||
* For large installations, run this migration in a maintenance window, or build
|
||||
* the trigram indexes out-of-band with CREATE INDEX CONCURRENTLY before deploying
|
||||
* (then this migration's `IF NOT EXISTS` is a no-op). Small/typical tenants are
|
||||
* unaffected.
|
||||
*/
|
||||
export async function up(db: Kysely<any>): Promise<void> {
|
||||
// Index-compatible, output-identical redefinition of f_unaccent (see header).
|
||||
await sql`
|
||||
CREATE OR REPLACE FUNCTION f_unaccent(text)
|
||||
RETURNS text
|
||||
LANGUAGE sql
|
||||
IMMUTABLE PARALLEL SAFE STRICT
|
||||
AS $func$
|
||||
SELECT public.unaccent($1);
|
||||
$func$
|
||||
`.execute(db);
|
||||
|
||||
// Search-suggest trigram indexes. Expressions match search.service.ts.
|
||||
await sql`
|
||||
CREATE INDEX IF NOT EXISTS idx_pages_title_trgm
|
||||
ON pages USING gin ((LOWER(f_unaccent(title))) gin_trgm_ops)
|
||||
`.execute(db);
|
||||
|
||||
await sql`
|
||||
CREATE INDEX IF NOT EXISTS idx_users_name_trgm
|
||||
ON users USING gin ((LOWER(f_unaccent(name))) gin_trgm_ops)
|
||||
`.execute(db);
|
||||
|
||||
await sql`
|
||||
CREATE INDEX IF NOT EXISTS idx_groups_name_trgm
|
||||
ON groups USING gin ((LOWER(f_unaccent(name))) gin_trgm_ops)
|
||||
`.execute(db);
|
||||
|
||||
// page_history: WHERE page_id ORDER BY id DESC (findPageHistoryByPageId).
|
||||
await sql`
|
||||
CREATE INDEX IF NOT EXISTS idx_page_history_page_id
|
||||
ON page_history (page_id, id DESC)
|
||||
`.execute(db);
|
||||
|
||||
// comments: WHERE page_id ORDER BY id ASC (findPageComments).
|
||||
await sql`
|
||||
CREATE INDEX IF NOT EXISTS idx_comments_page_id_id
|
||||
ON comments (page_id, id)
|
||||
`.execute(db);
|
||||
|
||||
// page_access(workspace_id): #348 made hasRestrictedPagesInWorkspace uncached
|
||||
// (F1 fix), so `EXISTS(SELECT 1 FROM page_access WHERE workspace_id=?)` now runs
|
||||
// per-request on every whole-workspace list endpoint (global search + suggest,
|
||||
// favorites, notifications, recent, created-by). page_access only had a
|
||||
// space_id index → that EXISTS was a seq scan in the common zero-restriction
|
||||
// case. This index makes it an index-only existence probe.
|
||||
await sql`
|
||||
CREATE INDEX IF NOT EXISTS idx_page_access_workspace_id
|
||||
ON page_access (workspace_id)
|
||||
`.execute(db);
|
||||
}
|
||||
|
||||
export async function down(db: Kysely<any>): Promise<void> {
|
||||
// Drop the expression indexes before restoring the function body.
|
||||
await sql`DROP INDEX IF EXISTS idx_pages_title_trgm`.execute(db);
|
||||
await sql`DROP INDEX IF EXISTS idx_users_name_trgm`.execute(db);
|
||||
await sql`DROP INDEX IF EXISTS idx_groups_name_trgm`.execute(db);
|
||||
await sql`DROP INDEX IF EXISTS idx_page_history_page_id`.execute(db);
|
||||
await sql`DROP INDEX IF EXISTS idx_comments_page_id_id`.execute(db);
|
||||
await sql`DROP INDEX IF EXISTS idx_page_access_workspace_id`.execute(db);
|
||||
|
||||
// Restore the original two-arg (dictionary-named) f_unaccent body.
|
||||
await sql`
|
||||
CREATE OR REPLACE FUNCTION f_unaccent(text)
|
||||
RETURNS text
|
||||
LANGUAGE sql
|
||||
IMMUTABLE PARALLEL SAFE STRICT
|
||||
AS $func$
|
||||
SELECT unaccent('unaccent', $1);
|
||||
$func$
|
||||
`.execute(db);
|
||||
}
|
||||
@@ -657,8 +657,9 @@ export class PagePermissionRepo {
|
||||
pageIds: string[];
|
||||
userId: string;
|
||||
spaceId?: string;
|
||||
workspaceId?: string | null;
|
||||
}): Promise<string[]> {
|
||||
const { pageIds, userId, spaceId } = opts;
|
||||
const { pageIds, userId, spaceId, workspaceId } = opts;
|
||||
if (pageIds.length === 0) return [];
|
||||
|
||||
if (spaceId) {
|
||||
@@ -666,6 +667,17 @@ export class PagePermissionRepo {
|
||||
if (!hasRestrictions) {
|
||||
return pageIds;
|
||||
}
|
||||
} else if (workspaceId) {
|
||||
// #348 — whole-workspace callers (no spaceId: favorites, notifications,
|
||||
// recent, created-by, global search) skip the recursive-ancestor CTE + anti
|
||||
// -join entirely when the workspace has ZERO restricted pages. When any
|
||||
// restriction DOES exist, fall through to the identical CTE below, so
|
||||
// behavior is unchanged whenever restrictions are present.
|
||||
const hasRestrictions =
|
||||
await this.hasRestrictedPagesInWorkspace(workspaceId);
|
||||
if (!hasRestrictions) {
|
||||
return pageIds;
|
||||
}
|
||||
}
|
||||
|
||||
const results = await this.db
|
||||
@@ -903,6 +915,39 @@ export class PagePermissionRepo {
|
||||
return Boolean(result?.exists);
|
||||
}
|
||||
|
||||
/**
|
||||
* Workspace-level analogue of hasRestrictedPagesInSpace: does ANY page in the
|
||||
* whole workspace carry a restriction? Lets whole-workspace access filters
|
||||
* short-circuit the recursive-ancestor CTE when nothing is restricted at all.
|
||||
*
|
||||
* UNCACHED (like the sibling hasRestrictedPagesInSpace) — a single cheap
|
||||
* `EXISTS(pageAccess WHERE workspaceId=?)` per call. This is an ACCESS-CONTROL
|
||||
* gate on whole-workspace list endpoints, so it must never go stale: caching it
|
||||
* (even 5s) reintroduced a leak the space-path never had — a concurrent
|
||||
* whole-workspace read in the insert->commit window of the FIRST restricted page
|
||||
* could re-populate `false` under withCache (read-then-set, no del-during-read
|
||||
* guard) and override the insert bust, leaking that page to unauthorized users
|
||||
* for up to the TTL (#348 review F1). An uncached EXISTS removes both the
|
||||
* cache/DB asymmetry with hasRestrictedPagesInSpace and that race; the space
|
||||
* path already accepts this exact per-call cost.
|
||||
*/
|
||||
async hasRestrictedPagesInWorkspace(workspaceId: string): Promise<boolean> {
|
||||
const result = await this.db
|
||||
.selectNoFrom((eb) =>
|
||||
eb
|
||||
.exists(
|
||||
eb
|
||||
.selectFrom('pageAccess')
|
||||
.select(sql`1`.as('one'))
|
||||
.where('pageAccess.workspaceId', '=', workspaceId),
|
||||
)
|
||||
.as('exists'),
|
||||
)
|
||||
.executeTakeFirst();
|
||||
|
||||
return Boolean(result?.exists);
|
||||
}
|
||||
|
||||
/**
|
||||
* Given a list of parent page IDs, return which ones have at least one accessible child.
|
||||
* Efficient batch query for sidebar hasChildren calculation.
|
||||
|
||||
@@ -581,6 +581,9 @@ export class PageRepo {
|
||||
const query = this.db
|
||||
.selectFrom('pages')
|
||||
.select(this.baseFields)
|
||||
// NOTE: `content` IS needed here — the trash UI reads page.content to render
|
||||
// the deleted-page preview modal (trash.tsx handlePageClick ->
|
||||
// TrashPageContentModal pageContent). Do NOT drop it (see #348 review F3).
|
||||
.select('content')
|
||||
.select((eb) => this.withSpace(eb))
|
||||
.select((eb) => this.withDeletedBy(eb))
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { Inject, Injectable } from '@nestjs/common';
|
||||
import { CACHE_MANAGER } from '@nestjs/cache-manager';
|
||||
import { Cache } from 'cache-manager';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { KyselyDB, KyselyTransaction } from '../../types/kysely.types';
|
||||
import { dbOrTx } from '../../utils';
|
||||
@@ -9,6 +11,7 @@ import {
|
||||
} from '@docmost/db/types/entity.types';
|
||||
import { ExpressionBuilder, sql } from 'kysely';
|
||||
import { DB, Workspaces } from '@docmost/db/types/db';
|
||||
import { CacheKey } from '../../../common/helpers/cache-keys';
|
||||
|
||||
/**
|
||||
* Writable `settings.ai.provider` keys, enforced at this generic SQL layer. This
|
||||
@@ -61,7 +64,34 @@ export class WorkspaceRepo {
|
||||
'temporaryNoteHours',
|
||||
'isScimEnabled',
|
||||
];
|
||||
constructor(@InjectKysely() private readonly db: KyselyDB) {}
|
||||
constructor(
|
||||
@InjectKysely() private readonly db: KyselyDB,
|
||||
@Inject(CACHE_MANAGER) private readonly cacheManager: Cache,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* #348 — bust the DomainMiddleware workspace caches after any workspace write.
|
||||
* Deletes BOTH the self-hosted (constant) key and the cloud per-hostname key so
|
||||
* a single implementation covers either deployment mode (the irrelevant key is a
|
||||
* harmless no-op). Best-effort: a cache error must never fail the write, and a
|
||||
* missed bust is bounded by WORKSPACE_CACHE_TTL_MS. Note: a hostname RENAME only
|
||||
* busts the NEW hostname's key (the row returned here carries the new hostname);
|
||||
* the old key expires via TTL.
|
||||
*/
|
||||
private async bustWorkspaceCache(
|
||||
workspace?: Pick<Workspace, 'hostname'> | undefined,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await this.cacheManager.del(CacheKey.WORKSPACE_SELF_HOSTED);
|
||||
if (workspace?.hostname) {
|
||||
await this.cacheManager.del(
|
||||
CacheKey.WORKSPACE_BY_HOST(workspace.hostname),
|
||||
);
|
||||
}
|
||||
} catch {
|
||||
// cache is best-effort; TTL is the backstop
|
||||
}
|
||||
}
|
||||
|
||||
async findById(
|
||||
workspaceId: string,
|
||||
@@ -144,12 +174,14 @@ export class WorkspaceRepo {
|
||||
trx?: KyselyTransaction,
|
||||
): Promise<Workspace> {
|
||||
const db = dbOrTx(this.db, trx);
|
||||
return db
|
||||
const workspace = await db
|
||||
.updateTable('workspaces')
|
||||
.set({ ...updatableWorkspace, updatedAt: new Date() })
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
async insertWorkspace(
|
||||
@@ -157,11 +189,14 @@ export class WorkspaceRepo {
|
||||
trx?: KyselyTransaction,
|
||||
): Promise<Workspace> {
|
||||
const db = dbOrTx(this.db, trx);
|
||||
return db
|
||||
const workspace = await db
|
||||
.insertInto('workspaces')
|
||||
.values(insertableWorkspace)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
// Bust the cached "not found" so a fresh install / new tenant is seen at once.
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
async count(): Promise<number> {
|
||||
@@ -203,7 +238,7 @@ export class WorkspaceRepo {
|
||||
trx?: KyselyTransaction,
|
||||
) {
|
||||
const db = dbOrTx(this.db, trx);
|
||||
return db
|
||||
const workspace = await db
|
||||
.updateTable('workspaces')
|
||||
.set({
|
||||
settings: sql`COALESCE(settings, '{}'::jsonb)
|
||||
@@ -214,6 +249,8 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
async updateAiSettings(
|
||||
@@ -223,7 +260,7 @@ export class WorkspaceRepo {
|
||||
trx?: KyselyTransaction,
|
||||
) {
|
||||
const db = dbOrTx(this.db, trx);
|
||||
return db
|
||||
const workspace = await db
|
||||
.updateTable('workspaces')
|
||||
.set({
|
||||
settings: sql`COALESCE(settings, '{}'::jsonb)
|
||||
@@ -234,6 +271,8 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -272,7 +311,7 @@ export class WorkspaceRepo {
|
||||
entries.flatMap(([k, v]) => [sql.lit(k), sql`${v}::text`]),
|
||||
)})`
|
||||
: sql`'{}'::jsonb`;
|
||||
return db
|
||||
const workspace = await db
|
||||
.updateTable('workspaces')
|
||||
.set({
|
||||
settings: sql`COALESCE(settings, '{}'::jsonb) || jsonb_build_object(
|
||||
@@ -287,6 +326,8 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -303,7 +344,7 @@ export class WorkspaceRepo {
|
||||
trx?: KyselyTransaction,
|
||||
) {
|
||||
const db = dbOrTx(this.db, trx);
|
||||
return db
|
||||
const workspace = await db
|
||||
.updateTable('workspaces')
|
||||
.set({
|
||||
settings: sql`COALESCE(settings, '{}'::jsonb)
|
||||
@@ -313,6 +354,8 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
async updateSharingSettings(
|
||||
@@ -322,7 +365,7 @@ export class WorkspaceRepo {
|
||||
trx?: KyselyTransaction,
|
||||
) {
|
||||
const db = dbOrTx(this.db, trx);
|
||||
return db
|
||||
const workspace = await db
|
||||
.updateTable('workspaces')
|
||||
.set({
|
||||
settings: sql`COALESCE(settings, '{}'::jsonb)
|
||||
@@ -333,6 +376,8 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
async updateTemplateSettings(
|
||||
@@ -342,7 +387,7 @@ export class WorkspaceRepo {
|
||||
trx?: KyselyTransaction,
|
||||
) {
|
||||
const db = dbOrTx(this.db, trx);
|
||||
return db
|
||||
const workspace = await db
|
||||
.updateTable('workspaces')
|
||||
.set({
|
||||
settings: sql`COALESCE(settings, '{}'::jsonb)
|
||||
@@ -353,6 +398,8 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
import { Kysely } from 'kysely';
|
||||
import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo';
|
||||
import { GroupRepo } from '@docmost/db/repos/group/group.repo';
|
||||
import {
|
||||
getTestDb,
|
||||
destroyTestDb,
|
||||
createWorkspace,
|
||||
createSpace,
|
||||
createUser,
|
||||
createPage,
|
||||
} from './db';
|
||||
|
||||
/**
|
||||
* #348 — the whole-workspace access-filter short-circuit is an ACCESS-CONTROL
|
||||
* path, so it must produce the SAME result as the full recursive-ancestor CTE.
|
||||
*
|
||||
* filterAccessiblePageIds({ workspaceId }) (no spaceId — the favorites /
|
||||
* notifications / recent / created-by / global-search callers) skips the CTE only
|
||||
* when the workspace has ZERO restricted pages. A page is "restricted &
|
||||
* inaccessible" when it (or an ancestor) has a `pageAccess` row and the user has
|
||||
* no matching `pagePermissions`. Driven against real Postgres, asserts:
|
||||
* 1. zero restrictions -> short-circuit returns the full input set;
|
||||
* 2. a restriction present -> the CTE runs and drops the page the user can't
|
||||
* reach while keeping the reachable ones (behavior unchanged);
|
||||
* 3. inserting the FIRST pageAccess flips hasRestrictedPagesInWorkspace
|
||||
* false -> true immediately (the 0->1 transition — now uncached, no stale
|
||||
* window, review F1); it is scoped per workspace.
|
||||
*/
|
||||
describe('#348 filterAccessiblePageIds workspace short-circuit (real PG)', () => {
|
||||
let db: Kysely<any>;
|
||||
let repo: PagePermissionRepo;
|
||||
let workspaceId: string;
|
||||
let otherWorkspaceId: string;
|
||||
let userId: string;
|
||||
let spaceId: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
db = getTestDb();
|
||||
// hasRestrictedPagesInWorkspace is now uncached, and no other cached
|
||||
// permission path is exercised here, so a no-op cache stub suffices.
|
||||
const cacheStub = {
|
||||
get: async () => undefined,
|
||||
set: async () => undefined,
|
||||
del: async () => undefined,
|
||||
} as never;
|
||||
repo = new PagePermissionRepo(db, new GroupRepo(db), cacheStub);
|
||||
|
||||
const ws = await createWorkspace(db);
|
||||
workspaceId = ws.id;
|
||||
const other = await createWorkspace(db);
|
||||
otherWorkspaceId = other.id;
|
||||
const user = await createUser(db, workspaceId);
|
||||
userId = user.id;
|
||||
const space = await createSpace(db, workspaceId);
|
||||
spaceId = space.id;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await destroyTestDb();
|
||||
});
|
||||
|
||||
it('zero restrictions: short-circuit returns the full input set', async () => {
|
||||
const p1 = await createPage(db, { workspaceId, spaceId });
|
||||
const p2 = await createPage(db, { workspaceId, spaceId });
|
||||
|
||||
expect(await repo.hasRestrictedPagesInWorkspace(workspaceId)).toBe(false);
|
||||
|
||||
const ids = [p1.id, p2.id];
|
||||
const filtered = await repo.filterAccessiblePageIds({
|
||||
pageIds: ids,
|
||||
userId,
|
||||
workspaceId,
|
||||
});
|
||||
expect(new Set(filtered)).toEqual(new Set(ids));
|
||||
});
|
||||
|
||||
it('a restriction present: filters out the page the user cannot reach', async () => {
|
||||
const openPage = await createPage(db, { workspaceId, spaceId });
|
||||
const restrictedPage = await createPage(db, { workspaceId, spaceId });
|
||||
|
||||
// Add a pageAccess row on restrictedPage with NO matching pagePermissions for
|
||||
// `userId` → the CTE anti-join marks it inaccessible for this user.
|
||||
await db
|
||||
.insertInto('pageAccess')
|
||||
.values({
|
||||
pageId: restrictedPage.id,
|
||||
workspaceId,
|
||||
spaceId,
|
||||
accessLevel: 'read',
|
||||
creatorId: userId,
|
||||
})
|
||||
.execute();
|
||||
|
||||
// 0->1 transition is reflected immediately (uncached).
|
||||
expect(await repo.hasRestrictedPagesInWorkspace(workspaceId)).toBe(true);
|
||||
|
||||
const filtered = await repo.filterAccessiblePageIds({
|
||||
pageIds: [openPage.id, restrictedPage.id],
|
||||
userId,
|
||||
workspaceId,
|
||||
});
|
||||
expect(filtered).toContain(openPage.id);
|
||||
expect(filtered).not.toContain(restrictedPage.id);
|
||||
});
|
||||
|
||||
it('hasRestrictedPagesInWorkspace is scoped per workspace', async () => {
|
||||
// The other workspace has no pageAccess rows → still false, unaffected by the
|
||||
// restriction added above in `workspaceId`.
|
||||
expect(await repo.hasRestrictedPagesInWorkspace(otherWorkspaceId)).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -1,7 +1,25 @@
|
||||
import { Kysely } from 'kysely';
|
||||
import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo';
|
||||
import { CacheKey } from 'src/common/helpers/cache-keys';
|
||||
import { getTestDb, destroyTestDb, createWorkspace } from './db';
|
||||
|
||||
// A minimal Map-backed cache double with a working `del` (the previous `{}` stub
|
||||
// made bustWorkspaceCache's `del` throw into its own try/catch, so the #348
|
||||
// invalidation was never actually exercised — review F6).
|
||||
function makeCacheDouble() {
|
||||
const store = new Map<string, unknown>();
|
||||
return {
|
||||
store,
|
||||
get: async (k: string) => store.get(k),
|
||||
set: async (k: string, v: unknown) => {
|
||||
store.set(k, v);
|
||||
},
|
||||
del: async (k: string) => {
|
||||
store.delete(k);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* A — WorkspaceRepo.updateSetting jsonb-MERGE (the html-embed kill-switch
|
||||
* write-half). Setting a single top-level key must NOT clobber sibling
|
||||
@@ -15,7 +33,9 @@ describe('WorkspaceRepo.updateSetting (jsonb merge) [integration]', () => {
|
||||
beforeAll(() => {
|
||||
db = getTestDb();
|
||||
// Repos are plain classes taking @InjectKysely() db — instantiate directly.
|
||||
repo = new WorkspaceRepo(db as any);
|
||||
// 2nd arg is CACHE_MANAGER (used only to bust the #348 workspace cache); a
|
||||
// stub is fine here since bustWorkspaceCache is best-effort (try/catch).
|
||||
repo = new WorkspaceRepo(db as any, {} as any);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
@@ -58,3 +78,62 @@ describe('WorkspaceRepo.updateSetting (jsonb merge) [integration]', () => {
|
||||
expect(updated.settings).toEqual({ htmlEmbed: false });
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* #348 F6 — the DomainMiddleware workspace cache (WORKSPACE_SELF_HOSTED /
|
||||
* WORKSPACE_BY_HOST, 15s TTL) caches security-relevant fields (enforceSso/
|
||||
* enforceMfa/status). Its correctness rests entirely on bustWorkspaceCache being
|
||||
* called from every mutator. This exercises the real invalidation with a working
|
||||
* cache double (not the {} stub, whose del throws-and-swallows): warm the cache
|
||||
* like DomainMiddleware, mutate, and assert the busted key is gone so a stale
|
||||
* workspace row can't outlive the mutation.
|
||||
*/
|
||||
describe('WorkspaceRepo bustWorkspaceCache invalidation [integration]', () => {
|
||||
let db: Kysely<any>;
|
||||
|
||||
beforeAll(() => {
|
||||
db = getTestDb();
|
||||
});
|
||||
afterAll(async () => {
|
||||
await destroyTestDb();
|
||||
});
|
||||
|
||||
it('updateSetting busts the self-hosted workspace cache key', async () => {
|
||||
const cache = makeCacheDouble();
|
||||
const repo = new WorkspaceRepo(db as any, cache as any);
|
||||
const ws = await createWorkspace(db, { settings: {} });
|
||||
|
||||
// Warm the cache as DomainMiddleware would (self-hosted key).
|
||||
cache.store.set(CacheKey.WORKSPACE_SELF_HOSTED, ws);
|
||||
expect(cache.store.has(CacheKey.WORKSPACE_SELF_HOSTED)).toBe(true);
|
||||
|
||||
await repo.updateSetting(ws.id, 'htmlEmbed', true);
|
||||
|
||||
// The mutation must have invalidated the cached row.
|
||||
expect(cache.store.has(CacheKey.WORKSPACE_SELF_HOSTED)).toBe(false);
|
||||
});
|
||||
|
||||
it('updateSharingSettings busts the by-host workspace cache key too', async () => {
|
||||
const cache = makeCacheDouble();
|
||||
const repo = new WorkspaceRepo(db as any, cache as any);
|
||||
const ws = await createWorkspace(db, { settings: {} });
|
||||
// createWorkspace assigns a unique hostname; read it back for the by-host key.
|
||||
const { hostname } = await db
|
||||
.selectFrom('workspaces')
|
||||
.select(['hostname'])
|
||||
.where('id', '=', ws.id)
|
||||
.executeTakeFirstOrThrow();
|
||||
|
||||
// Warm BOTH keys (self-hosted + by-host); the by-host bust needs the row's
|
||||
// hostname, which the mutator returns from the DB.
|
||||
cache.store.set(CacheKey.WORKSPACE_SELF_HOSTED, ws);
|
||||
cache.store.set(CacheKey.WORKSPACE_BY_HOST(hostname as string), ws);
|
||||
|
||||
await repo.updateSharingSettings(ws.id, 'allowInvite', true);
|
||||
|
||||
expect(cache.store.has(CacheKey.WORKSPACE_SELF_HOSTED)).toBe(false);
|
||||
expect(cache.store.has(CacheKey.WORKSPACE_BY_HOST(hostname as string))).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -82,7 +82,6 @@ import {
|
||||
canonicalizeFootnotes,
|
||||
insertInlineFootnote,
|
||||
} from "./lib/transforms.js";
|
||||
import { normalizeAndMergeFootnotes } from "./lib/footnote-normalize-merge.js";
|
||||
import vm from "node:vm";
|
||||
|
||||
// Supported image types, kept as two lookup tables so both a local file
|
||||
@@ -1557,8 +1556,6 @@ export class DocmostClient {
|
||||
// leave footnotes out of order, orphaned, or in multiple lists — the bottom
|
||||
// list + numbering are always derived from reference order. No-op when the
|
||||
// footnotes are already canonical.
|
||||
// #419: normalize + merge glyph-forked definitions before canonicalizing.
|
||||
doc = normalizeAndMergeFootnotes(doc);
|
||||
doc = canonicalizeFootnotes(doc);
|
||||
|
||||
// Write the BODY first, then the title (#159 split-brain): a failed body
|
||||
@@ -1823,8 +1820,7 @@ export class DocmostClient {
|
||||
// footnotes before copying — a no-op on already-canonical source content, but
|
||||
// it guarantees a copy can never propagate a non-canonical footnote topology
|
||||
// to the target (parity with the other full-doc write paths).
|
||||
// #419: normalize + merge glyph-forked definitions before canonicalizing.
|
||||
const canonical = canonicalizeFootnotes(normalizeAndMergeFootnotes(content));
|
||||
const canonical = canonicalizeFootnotes(content);
|
||||
|
||||
const collabToken = await this.getCollabTokenWithReauth();
|
||||
// Open the TARGET collab doc by its canonical UUID, never the slugId (#260).
|
||||
@@ -4079,8 +4075,7 @@ export class DocmostClient {
|
||||
// path can leave footnotes out of order / orphaned / in a raw `[^id]`
|
||||
// block. In a dryRun preview this may surface footnote edits the script
|
||||
// author did not write (the canonicalizer tidied them) — that is expected.
|
||||
// #419: normalize + merge glyph-forked definitions before canonicalizing.
|
||||
const result = canonicalizeFootnotes(normalizeAndMergeFootnotes(raw));
|
||||
const result = canonicalizeFootnotes(raw);
|
||||
newDoc = result;
|
||||
return result;
|
||||
};
|
||||
|
||||
@@ -15,7 +15,6 @@ import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
|
||||
import { withPageLock } from "./page-lock.js";
|
||||
import { sanitizeForYjs, findUnstorableAttr } from "./node-ops.js";
|
||||
import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
|
||||
import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
|
||||
import { VerifyReport } from "./diff.js";
|
||||
import { acquireCollabSession } from "./collab-session.js";
|
||||
|
||||
@@ -83,12 +82,7 @@ global.WebSocket = WebSocket;
|
||||
export async function markdownToProseMirrorCanonical(
|
||||
markdownContent: string,
|
||||
): Promise<any> {
|
||||
// #419: normalize + merge glyph-forked footnote definitions BEFORE
|
||||
// canonicalizing, so the canonicalizer re-hangs references and drops the
|
||||
// now-orphaned duplicate definitions.
|
||||
return canonicalizeFootnotes(
|
||||
normalizeAndMergeFootnotes(await markdownToProseMirror(markdownContent)),
|
||||
);
|
||||
return canonicalizeFootnotes(await markdownToProseMirror(markdownContent));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,280 +0,0 @@
|
||||
/**
|
||||
* Deterministic server-side NORMALIZATION + MERGE of footnote DEFINITIONS
|
||||
* (MCP, PURE).
|
||||
*
|
||||
* Problem (#419): footnotes with the same meaning but different GLYPHS —
|
||||
* typographic quotes («…»/“…”) vs ASCII "…", em/en-dash vs `-`, non-breaking
|
||||
* space vs normal space, differing space counts — are not recognized as equal
|
||||
* and "fork": two definitions appear where the author meant one. The existing
|
||||
* de-dup paths miss this: `footnoteContentKey` (footnote-authoring.ts) only
|
||||
* collapses ASCII whitespace (quotes/dashes/NBSP untouched), and
|
||||
* `canonicalizeFootnotes` keys purely by `attrs.id` (the two forks have
|
||||
* different ids), so neither glues the forks together.
|
||||
*
|
||||
* This pass fixes that DETERMINISTICALLY on the MCP write-paths (an LLM
|
||||
* instruction gives no glue guarantee). It:
|
||||
* 1. Normalizes the TEXT of every `footnoteDefinition`'s text nodes IN PLACE
|
||||
* (typographic quotes -> ASCII "/', dashes -> `-`, NBSP & friends ->
|
||||
* normal space, whitespace runs collapsed, whole-definition edges
|
||||
* trimmed) — unconditionally, for ALL definitions, KEEPING their marks.
|
||||
* 2. Computes a MERGE KEY per definition (normalized text + an ATTRS-AWARE
|
||||
* inline-mark signature, via the local `footnoteMergeKey`), so notes that
|
||||
* read the same but differ in formatting (bold vs plain) OR in a mark
|
||||
* attribute (a `link` with a different `href`, differing `code`/`highlight`
|
||||
* attrs) are NOT merged. See `footnoteMergeKey` for why this diverges from
|
||||
* the shared type-only `footnoteContentKey`.
|
||||
* 3. Maps every duplicate definition id to the FIRST (document-order)
|
||||
* definition's id and re-hangs `footnoteReference` nodes onto it.
|
||||
*
|
||||
* Duplicate definitions keep their original ids but now have NO references, so
|
||||
* the canonicalizer that runs immediately after this pass removes them as
|
||||
* orphans and derives the single tail list + numbering. This pass therefore
|
||||
* MUST run BEFORE `canonicalizeFootnotes(doc)` at every write-path call-site
|
||||
* (see the enforcement rule in `footnote-canonicalize.ts`).
|
||||
*
|
||||
* Accepted tradeoff: the exact typographic glyphs of the SURVIVING footnote are
|
||||
* rewritten to ASCII, in exchange for a GUARANTEED merge. Scope is strictly
|
||||
* INSIDE `footnoteDefinition` — body text (normal paragraphs) is never touched.
|
||||
*
|
||||
* Pure: deep-clones its input, deterministic, idempotent (a re-run is a no-op —
|
||||
* text is already normalized and references already point at the canonical id,
|
||||
* so no spurious mutations / git-sync churn).
|
||||
*/
|
||||
|
||||
const FOOTNOTE_DEFINITION_NAME = "footnoteDefinition";
|
||||
const FOOTNOTE_REFERENCE_NAME = "footnoteReference";
|
||||
|
||||
/**
|
||||
* Typographic glyph maps. DUPLICATED from `comment-anchor.ts` (the source of
|
||||
* truth, `normalizeForMatch`) on purpose: those constants are private there and
|
||||
* bound to that module's anchor-matching golden tests, so extracting them would
|
||||
* risk changing anchor behaviour. Keeping a local copy makes this pass fully
|
||||
* self-contained. If the anchor maps grow, mirror the change here.
|
||||
*/
|
||||
/** Typographic double-quote variants mapped to ASCII `"`. */
|
||||
const DOUBLE_QUOTES = "«»„“”‟〝〞"";
|
||||
/** Typographic single-quote/apostrophe variants mapped to ASCII `'`. */
|
||||
const SINGLE_QUOTES = "‘’‚‛";
|
||||
/** Dash variants mapped to ASCII `-`. */
|
||||
const DASHES = "–—―−‐‑‒";
|
||||
|
||||
function cloneJson<T>(v: T): T {
|
||||
if (typeof structuredClone === "function") return structuredClone(v);
|
||||
return JSON.parse(JSON.stringify(v)) as T;
|
||||
}
|
||||
|
||||
/**
|
||||
* True for any character we collapse/replace with a single normal space.
|
||||
* Mirrors `comment-anchor.ts`'s `isWhitespaceChar`: ASCII whitespace (`\s`
|
||||
* covers tab/newline) plus the non-breaking / special spaces listed explicitly
|
||||
* for determinism across engines.
|
||||
*/
|
||||
function isWhitespaceChar(ch: string): boolean {
|
||||
return (
|
||||
/\s/.test(ch) ||
|
||||
ch === " " || // no-break space
|
||||
ch === " " || // figure space
|
||||
ch === " " || // narrow no-break space
|
||||
ch === " " || // thin space
|
||||
ch === " " || // hair space
|
||||
ch === " " || // en space
|
||||
ch === " " // em space
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Map typographic quotes/dashes to ASCII and collapse every whitespace run
|
||||
* (including NBSP & friends) to a SINGLE normal space. Does NOT trim — the
|
||||
* whole-definition edge trim is applied separately so inter-node spacing across
|
||||
* a multi-text-node definition is preserved.
|
||||
*/
|
||||
function normalizeAndCollapse(s: string): string {
|
||||
let out = "";
|
||||
let i = 0;
|
||||
while (i < s.length) {
|
||||
const ch = s[i];
|
||||
if (isWhitespaceChar(ch)) {
|
||||
while (i < s.length && isWhitespaceChar(s[i])) i++;
|
||||
out += " ";
|
||||
continue;
|
||||
}
|
||||
let mapped = ch;
|
||||
if (DOUBLE_QUOTES.indexOf(ch) !== -1) mapped = '"';
|
||||
else if (SINGLE_QUOTES.indexOf(ch) !== -1) mapped = "'";
|
||||
else if (DASHES.indexOf(ch) !== -1) mapped = "-";
|
||||
out += mapped;
|
||||
i++;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Collect every text node inside `def`, in document order (deep). */
|
||||
function collectTextNodes(node: any, out: any[]): void {
|
||||
if (!node || typeof node !== "object") return;
|
||||
if (node.type === "text" && typeof node.text === "string") out.push(node);
|
||||
if (Array.isArray(node.content)) {
|
||||
for (const child of node.content) collectTextNodes(child, out);
|
||||
}
|
||||
}
|
||||
|
||||
/** Collect every `footnoteDefinition` node in document order (deep). */
|
||||
function collectDefinitions(node: any, out: any[]): void {
|
||||
if (!node || typeof node !== "object") return;
|
||||
if (node.type === FOOTNOTE_DEFINITION_NAME) out.push(node);
|
||||
if (Array.isArray(node.content)) {
|
||||
for (const child of node.content) collectDefinitions(child, out);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize the text of one definition's text nodes IN PLACE: map glyphs +
|
||||
* collapse whitespace on every node (marks untouched), then trim the leading
|
||||
* edge of the first text node and the trailing edge of the last so the
|
||||
* definition as a whole is trimmed WITHOUT dropping the spacing between two
|
||||
* adjacent text nodes. The edge trims are guarded so an all-whitespace edge
|
||||
* node is never emptied into a schema-invalid empty text node.
|
||||
*/
|
||||
function normalizeDefinitionText(def: any): void {
|
||||
const textNodes: any[] = [];
|
||||
collectTextNodes(def, textNodes);
|
||||
for (const t of textNodes) {
|
||||
// Skip text carrying a `code` mark: inline code is a verbatim literal, not
|
||||
// prose typography. Rewriting quotes/dashes/special-spaces there would
|
||||
// corrupt the literal's meaning (a string literal, an em-dash flag, i18n).
|
||||
// Leaving it untouched also makes it contribute its RAW text to
|
||||
// `footnoteMergeKey`, so two notes differing only by glyphs inside code
|
||||
// stay distinct (while prose glyph-forks still merge). See #419.
|
||||
if ((t.marks || []).some((m: any) => m?.type === "code")) continue;
|
||||
t.text = normalizeAndCollapse(t.text);
|
||||
}
|
||||
if (textNodes.length === 0) return;
|
||||
const hasCodeMark = (t: any): boolean =>
|
||||
(t.marks || []).some((m: any) => m?.type === "code");
|
||||
const first = textNodes[0];
|
||||
if (!hasCodeMark(first)) {
|
||||
const startTrimmed = first.text.replace(/^ +/, "");
|
||||
if (startTrimmed !== "") first.text = startTrimmed;
|
||||
}
|
||||
const last = textNodes[textNodes.length - 1];
|
||||
if (!hasCodeMark(last)) {
|
||||
const endTrimmed = last.text.replace(/ +$/, "");
|
||||
if (endTrimmed !== "") last.text = endTrimmed;
|
||||
}
|
||||
}
|
||||
|
||||
/** Rewrite `footnoteReference` ids IN PLACE using `defIdToCanon` (deep). */
|
||||
function rehangReferences(
|
||||
node: any,
|
||||
defIdToCanon: Map<string, string>,
|
||||
): void {
|
||||
if (!node || typeof node !== "object") return;
|
||||
if (node.type === FOOTNOTE_REFERENCE_NAME) {
|
||||
const id = node?.attrs?.id;
|
||||
if (typeof id === "string") {
|
||||
const canon = defIdToCanon.get(id);
|
||||
if (canon && canon !== id) node.attrs.id = canon;
|
||||
}
|
||||
}
|
||||
if (Array.isArray(node.content)) {
|
||||
for (const child of node.content) rehangReferences(child, defIdToCanon);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Stable, order-independent serialization of a mark's `attrs`: sort keys so the
|
||||
* same attrs always yield the same string regardless of authoring order. Empty /
|
||||
* missing attrs -> "" (so an attr-less mark keys identically to a type-only mark
|
||||
* signature, preserving bold-vs-plain parity).
|
||||
*/
|
||||
function stableAttrs(attrs: any): string {
|
||||
if (!attrs || typeof attrs !== "object") return "";
|
||||
const sorted: Record<string, any> = {};
|
||||
for (const k of Object.keys(attrs).sort()) sorted[k] = attrs[k];
|
||||
return JSON.stringify(sorted);
|
||||
}
|
||||
|
||||
/**
|
||||
* ATTRS-AWARE merge key for a footnote definition. Deliberately DIVERGES from
|
||||
* the shared `footnoteContentKey` (footnote-authoring.ts): that key's mark
|
||||
* signature is TYPE-ONLY (`m.type`), so two definitions with identical visible
|
||||
* text but marks differing only in ATTRIBUTES — most importantly a `link` with a
|
||||
* different `href` (footnotes are usually citations/links), also `code` /
|
||||
* `highlight` with differing attrs — collapse to the SAME key and get merged;
|
||||
* one definition then loses its references and the canonicalizer deletes it as an
|
||||
* orphan, silently dropping a distinct link target (data loss, #419).
|
||||
*
|
||||
* This key folds each mark's `attrs` (stable, sorted-key serialization) into the
|
||||
* signature, so different-href / different-attr notes stay separate. We do NOT
|
||||
* change `footnoteContentKey` itself: it is shared with the live
|
||||
* `insertInlineFootnote` / `commentsToFootnotes` dedup and altering it there
|
||||
* would change their behaviour — out of scope here.
|
||||
*
|
||||
* The TEXT portion mirrors `footnoteContentKey` exactly (per text node
|
||||
* `text + mark-signature`, concatenated, whitespace-collapsed, trimmed) over the
|
||||
* already-in-place-normalized text, so empty text still yields "" (empties never
|
||||
* collapse) and merge parity with the rest of the pass is preserved.
|
||||
*/
|
||||
function footnoteMergeKey(defNode: any): string {
|
||||
const parts: string[] = [];
|
||||
const visit = (n: any): void => {
|
||||
if (!n || typeof n !== "object") return;
|
||||
if (n.type === "text" && typeof n.text === "string") {
|
||||
const marks = Array.isArray(n.marks)
|
||||
? n.marks
|
||||
.filter((m: any) => m && m.type)
|
||||
.map((m: any) => `${m.type}${stableAttrs(m.attrs)}`)
|
||||
.sort()
|
||||
.join(",")
|
||||
: "";
|
||||
parts.push(`${n.text}${marks}`);
|
||||
}
|
||||
if (Array.isArray(n.content)) for (const c of n.content) visit(c);
|
||||
};
|
||||
visit(defNode);
|
||||
return parts
|
||||
.join("")
|
||||
.replace(/[ \t\r\n]+/g, " ")
|
||||
.trim();
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize footnote-definition text and merge definitions whose normalized
|
||||
* text (+ mark signature) matches. See the file header for the full contract.
|
||||
* Pure (deep-clones input, deterministic, idempotent). Intended to run
|
||||
* immediately BEFORE `canonicalizeFootnotes(doc)`.
|
||||
*/
|
||||
export function normalizeAndMergeFootnotes<T = any>(doc: T): T {
|
||||
if (doc == null || typeof doc !== "object") return doc;
|
||||
const out = cloneJson(doc) as any;
|
||||
|
||||
// 1) All definitions in document order; normalize each one's text in place.
|
||||
const defNodes: any[] = [];
|
||||
collectDefinitions(out, defNodes);
|
||||
for (const def of defNodes) normalizeDefinitionText(def);
|
||||
|
||||
// 2) Merge key per definition (normalized text + inline-mark signature). The
|
||||
// first definition in document order per key wins; later ones map onto it.
|
||||
// Empty-text definitions (key === "") are NOT merged — otherwise every
|
||||
// empty footnote would collapse into one (parity with insertInlineFootnote).
|
||||
const keyToCanon = new Map<string, string>();
|
||||
const defIdToCanon = new Map<string, string>();
|
||||
for (const def of defNodes) {
|
||||
const id = def?.attrs?.id;
|
||||
if (typeof id !== "string" || id === "") continue;
|
||||
const key = footnoteMergeKey(def);
|
||||
if (key === "") continue;
|
||||
const canon = keyToCanon.get(key);
|
||||
if (canon === undefined) {
|
||||
keyToCanon.set(key, id);
|
||||
} else if (canon !== id) {
|
||||
defIdToCanon.set(id, canon);
|
||||
}
|
||||
}
|
||||
|
||||
// 3) Re-hang references from duplicate ids onto the canonical id. Duplicate
|
||||
// definitions keep their ids but now have no references -> the following
|
||||
// canonicalizer pass drops them as orphans.
|
||||
if (defIdToCanon.size > 0) rehangReferences(out, defIdToCanon);
|
||||
|
||||
return out;
|
||||
}
|
||||
@@ -16,7 +16,6 @@
|
||||
|
||||
import { blockPlainText } from "./node-ops.js";
|
||||
import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
|
||||
import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
|
||||
import {
|
||||
footnoteContentKey,
|
||||
makeFootnoteDefinition,
|
||||
@@ -767,8 +766,6 @@ export function insertInlineFootnote(
|
||||
appendDefinition(working, makeFootnoteDefinition(footnoteId, inline));
|
||||
}
|
||||
|
||||
// #419: normalize + merge glyph-forked definitions before canonicalizing.
|
||||
working = normalizeAndMergeFootnotes(working);
|
||||
// Derive numbering + the single bottom list deterministically.
|
||||
working = canonicalizeFootnotes(working);
|
||||
return { doc: working, inserted: true, footnoteId, reused };
|
||||
|
||||
@@ -1,317 +0,0 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { normalizeAndMergeFootnotes } from "../../build/lib/footnote-normalize-merge.js";
|
||||
import { canonicalizeFootnotes } from "../../build/lib/footnote-canonicalize.js";
|
||||
|
||||
function findAll(node, type, acc = []) {
|
||||
if (!node || typeof node !== "object") return acc;
|
||||
if (node.type === type) acc.push(node);
|
||||
if (Array.isArray(node.content)) {
|
||||
for (const c of node.content) findAll(c, type, acc);
|
||||
}
|
||||
return acc;
|
||||
}
|
||||
const defs = (doc) => findAll(doc, "footnoteDefinition");
|
||||
const defIds = (doc) => defs(doc).map((d) => d.attrs.id);
|
||||
const refIds = (doc) => findAll(doc, "footnoteReference").map((r) => r.attrs.id);
|
||||
const defText = (d) =>
|
||||
findAll(d, "text")
|
||||
.map((t) => t.text)
|
||||
.join("");
|
||||
|
||||
const ref = (id) => ({ type: "footnoteReference", attrs: { id } });
|
||||
const para = (...inline) => ({ type: "paragraph", content: inline });
|
||||
const txt = (text, marks) =>
|
||||
marks ? { type: "text", text, marks } : { type: "text", text };
|
||||
const def = (id, ...inline) => ({
|
||||
type: "footnoteDefinition",
|
||||
attrs: { id },
|
||||
content: [para(...inline)],
|
||||
});
|
||||
const list = (...defs) => ({ type: "footnotesList", content: defs });
|
||||
const doc = (...content) => ({ type: "doc", content });
|
||||
|
||||
// --- Normalization + merge of glyph forks ----------------------------------
|
||||
|
||||
test("typographic double quotes «…» vs \"…\" merge into one", () => {
|
||||
const d = doc(
|
||||
para(txt("a"), ref("A"), txt(" b"), ref("B")),
|
||||
list(def("A", txt("«word»")), def("B", txt('"word"'))),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
// Both references now point at the first definition's id.
|
||||
assert.deepEqual(refIds(out), ["A", "A"]);
|
||||
// Surviving text is ASCII-normalized.
|
||||
assert.equal(defText(defs(out)[0]), '"word"');
|
||||
// Duplicate def kept its id (canonicalizer removes it as an orphan later).
|
||||
const canon = canonicalizeFootnotes(out);
|
||||
assert.deepEqual(defIds(canon), ["A"]);
|
||||
assert.equal(findAll(canon, "footnotesList").length, 1);
|
||||
});
|
||||
|
||||
test("em/en dash and hyphen merge", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B"), ref("C")),
|
||||
list(
|
||||
def("A", txt("see — here")),
|
||||
def("B", txt("see – here")),
|
||||
def("C", txt("see - here")),
|
||||
),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
assert.deepEqual(refIds(out), ["A", "A", "A"]);
|
||||
assert.equal(defText(defs(out)[0]), "see - here");
|
||||
});
|
||||
|
||||
test("NBSP and extra spaces merge with normal spacing", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(
|
||||
def("A", txt("foo bar")), // NBSP
|
||||
def("B", txt("foo bar")), // collapsed spaces
|
||||
),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
assert.deepEqual(refIds(out), ["A", "A"]);
|
||||
assert.equal(defText(defs(out)[0]), "foo bar");
|
||||
});
|
||||
|
||||
test("same text but different styling (bold vs plain) does NOT merge", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(
|
||||
def("A", txt("word", [{ type: "bold" }])),
|
||||
def("B", txt("word")),
|
||||
),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
// No re-hang: references keep their own ids.
|
||||
assert.deepEqual(refIds(out), ["A", "B"]);
|
||||
assert.deepEqual(defIds(out), ["A", "B"]);
|
||||
// Marks preserved on the surviving text node.
|
||||
assert.deepEqual(defs(out)[0].content[0].content[0].marks, [
|
||||
{ type: "bold" },
|
||||
]);
|
||||
});
|
||||
|
||||
test("same text but a link mark with different href does NOT merge (data-loss guard)", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(
|
||||
def("A", txt("source", [{ type: "link", attrs: { href: "https://a.example/1" } }])),
|
||||
def("B", txt("source", [{ type: "link", attrs: { href: "https://b.example/2" } }])),
|
||||
),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
// No re-hang: each reference keeps its own definition (distinct link target).
|
||||
assert.deepEqual(refIds(out), ["A", "B"]);
|
||||
assert.deepEqual(defIds(out), ["A", "B"]);
|
||||
// Both distinct hrefs survive.
|
||||
assert.deepEqual(
|
||||
defs(out).map((dn) => dn.content[0].content[0].marks[0].attrs.href),
|
||||
["https://a.example/1", "https://b.example/2"],
|
||||
);
|
||||
// Canonicalize keeps both as two tail entries (neither is an orphan).
|
||||
const canon = canonicalizeFootnotes(out);
|
||||
assert.deepEqual(defIds(canon), ["A", "B"]);
|
||||
assert.deepEqual(refIds(canon), ["A", "B"]);
|
||||
});
|
||||
|
||||
test("same text and SAME link href still merges (attrs-aware key doesn't over-separate)", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(
|
||||
def("A", txt("source", [{ type: "link", attrs: { href: "https://a.example/1" } }])),
|
||||
def("B", txt("source", [{ type: "link", attrs: { href: "https://a.example/1" } }])),
|
||||
),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
assert.deepEqual(refIds(out), ["A", "A"]);
|
||||
const canon = canonicalizeFootnotes(out);
|
||||
assert.deepEqual(defIds(canon), ["A"]);
|
||||
assert.equal(findAll(canon, "footnotesList").length, 1);
|
||||
});
|
||||
|
||||
test("marks are kept on merged (surviving) definition text", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(
|
||||
def("A", txt("«x»", [{ type: "italic" }])),
|
||||
def("B", txt("«x»", [{ type: "italic" }])),
|
||||
),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
assert.deepEqual(refIds(out), ["A", "A"]);
|
||||
assert.deepEqual(defs(out)[0].content[0].content[0].marks, [
|
||||
{ type: "italic" },
|
||||
]);
|
||||
assert.equal(defText(defs(out)[0]), '"x"');
|
||||
});
|
||||
|
||||
// --- Inline code is verbatim (not typography) ------------------------------
|
||||
|
||||
test("text inside a code mark is left verbatim; prose in the same def is normalized", () => {
|
||||
const d = doc(
|
||||
para(ref("A")),
|
||||
list({
|
||||
type: "footnoteDefinition",
|
||||
attrs: { id: "A" },
|
||||
content: [
|
||||
para(
|
||||
txt("a—b «x»", [{ type: "code" }]),
|
||||
txt(" prose «y» — z"),
|
||||
),
|
||||
],
|
||||
}),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
const nodes = findAll(defs(out)[0], "text");
|
||||
// Code node: byte-for-byte unchanged (typography preserved).
|
||||
assert.equal(nodes[0].text, "a—b «x»");
|
||||
// Prose node: dashes/quotes normalized to ASCII.
|
||||
assert.equal(nodes[1].text, ' prose "y" - z');
|
||||
});
|
||||
|
||||
test("two notes differing ONLY by glyphs inside a code mark do NOT merge", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(
|
||||
def("A", txt("«x»", [{ type: "code" }]), txt(" same prose «q»")),
|
||||
def("B", txt('"x"', [{ type: "code" }]), txt(" same prose «q»")),
|
||||
),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
// Prose is identical after normalization, but the code literals differ raw
|
||||
// -> the merge key diverges -> both definitions survive, no re-hang.
|
||||
assert.deepEqual(refIds(out), ["A", "B"]);
|
||||
assert.deepEqual(defIds(out), ["A", "B"]);
|
||||
// Each code literal stays verbatim.
|
||||
assert.equal(defs(out)[0].content[0].content[0].text, "«x»");
|
||||
assert.equal(defs(out)[1].content[0].content[0].text, '"x"');
|
||||
// Both survive canonicalization (neither is an orphan).
|
||||
const canon = canonicalizeFootnotes(out);
|
||||
assert.deepEqual(defIds(canon), ["A", "B"]);
|
||||
assert.deepEqual(refIds(canon), ["A", "B"]);
|
||||
});
|
||||
|
||||
// --- Composition with the canonicalizer ------------------------------------
|
||||
|
||||
test("pass + canonicalize: single tail list and sequential numbering", () => {
|
||||
const d = doc(
|
||||
para(txt("intro "), ref("A"), txt(" middle "), ref("B")),
|
||||
list(def("A", txt("«note»")), def("B", txt('"note"'))),
|
||||
);
|
||||
const canon = canonicalizeFootnotes(normalizeAndMergeFootnotes(d));
|
||||
assert.equal(findAll(canon, "footnotesList").length, 1);
|
||||
assert.deepEqual(defIds(canon), ["A"]);
|
||||
assert.deepEqual(refIds(canon), ["A", "A"]);
|
||||
});
|
||||
|
||||
// --- Idempotency -----------------------------------------------------------
|
||||
|
||||
test("idempotent: a second run is a no-op", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(def("A", txt("«word»")), def("B", txt('"word"'))),
|
||||
);
|
||||
const once = normalizeAndMergeFootnotes(d);
|
||||
const twice = normalizeAndMergeFootnotes(once);
|
||||
assert.deepEqual(twice, once);
|
||||
});
|
||||
|
||||
test("input document is not mutated (pure)", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(def("A", txt("«word»")), def("B", txt('"word"'))),
|
||||
);
|
||||
const snapshot = JSON.parse(JSON.stringify(d));
|
||||
normalizeAndMergeFootnotes(d);
|
||||
assert.deepEqual(d, snapshot);
|
||||
});
|
||||
|
||||
// --- Nested definitions ----------------------------------------------------
|
||||
|
||||
test("definitions nested in a callout are normalized and merged", () => {
|
||||
const callout = (...content) => ({
|
||||
type: "callout",
|
||||
attrs: { type: "info" },
|
||||
content,
|
||||
});
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
callout(list(def("A", txt("«c»")), def("B", txt('"c"')))),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
assert.deepEqual(refIds(out), ["A", "A"]);
|
||||
assert.equal(defText(defs(out)[0]), '"c"');
|
||||
});
|
||||
|
||||
// --- Empty footnotes -------------------------------------------------------
|
||||
|
||||
test("empty footnotes do NOT collapse into each other", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(def("A", txt("")), { type: "footnoteDefinition", attrs: { id: "B" }, content: [{ type: "paragraph" }] }),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
// Both empty definitions keep distinct ids; references unchanged.
|
||||
assert.deepEqual(refIds(out), ["A", "B"]);
|
||||
assert.deepEqual(defIds(out), ["A", "B"]);
|
||||
});
|
||||
|
||||
// --- Body text left untouched ----------------------------------------------
|
||||
|
||||
test("body text (outside footnotes) is NOT normalized", () => {
|
||||
const d = doc(
|
||||
para(txt("body «quoted» — dash"), ref("A")),
|
||||
list(def("A", txt("«note»"))),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
// Body paragraph keeps its typographic glyphs verbatim.
|
||||
assert.equal(out.content[0].content[0].text, "body «quoted» — dash");
|
||||
// Footnote text IS normalized.
|
||||
assert.equal(defText(defs(out)[0]), '"note"');
|
||||
});
|
||||
|
||||
// --- Multi-paragraph structure preserved -----------------------------------
|
||||
|
||||
test("multi-paragraph definition: text normalized, structure preserved", () => {
|
||||
const d = doc(
|
||||
para(ref("A")),
|
||||
list({
|
||||
type: "footnoteDefinition",
|
||||
attrs: { id: "A" },
|
||||
content: [para(txt("«p1»")), para(txt("p2 — end"))],
|
||||
}),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
const def0 = defs(out)[0];
|
||||
assert.equal(def0.content.length, 2);
|
||||
assert.equal(def0.content[0].content[0].text, '"p1"');
|
||||
assert.equal(def0.content[1].content[0].text, "p2 - end");
|
||||
});
|
||||
|
||||
// --- Multi-reference footnote not broken -----------------------------------
|
||||
|
||||
test("one id shared by multiple references is preserved", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), txt(" x "), ref("A")),
|
||||
list(def("A", txt("note"))),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
assert.deepEqual(refIds(out), ["A", "A"]);
|
||||
assert.deepEqual(defIds(out), ["A"]);
|
||||
});
|
||||
|
||||
// --- Whole-definition edge trim --------------------------------------------
|
||||
|
||||
test("leading/trailing whitespace is trimmed for the merge and stored text", () => {
|
||||
const d = doc(
|
||||
para(ref("A"), ref("B")),
|
||||
list(def("A", txt(" hello ")), def("B", txt("hello"))),
|
||||
);
|
||||
const out = normalizeAndMergeFootnotes(d);
|
||||
assert.deepEqual(refIds(out), ["A", "A"]);
|
||||
assert.equal(defText(defs(out)[0]), "hello");
|
||||
});
|
||||
Reference in New Issue
Block a user