Implements Option 2 of #93. The restricted branch of broadcastPageMoved previously resolved its audience twice — emitToAuthorizedUsers and emitDeleteToUnauthorized each ran an independent fetchSockets + getUserIdsWithPageAccess — leaving a race window between the two snapshots where a socket could receive both the move and the delete (leak) or neither (lost compensating delete). - ws.service.ts: add emitMoveWithRestrictionSplit() that takes ONE socket snapshot and ONE authorization resolution, then partitions the room: authorized users get the moveTreeNode, everyone else (unauthorized + anonymous) get the compensating deleteTreeNode. Disjoint + complete by construction. Remove the now-unused emitToAuthorizedUsers / emitDeleteToUnauthorized; keep private broadcastToAuthorizedUsers (still used by emitRestrictedAwareToSpace). - ws-tree.service.ts: broadcastPageMoved restricted branch now drives move + delete from the single method. - specs: assert the single method is used and that fetchSockets / getUserIdsWithPageAccess are each called exactly once (single snapshot); re-route ws-service.spec to emitTreeEvent after the method removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
500 lines
18 KiB
TypeScript
500 lines
18 KiB
TypeScript
import { Test, TestingModule } from '@nestjs/testing';
|
|
import { WsTreeService } from './ws-tree.service';
|
|
import { WsService } from './ws.service';
|
|
import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo';
|
|
import { CACHE_MANAGER } from '@nestjs/cache-manager';
|
|
import {
|
|
PageMovedEvent,
|
|
TreeNodeSnapshot,
|
|
} from '../database/listeners/page.listener';
|
|
import {
|
|
getSpaceRoomName,
|
|
WS_SPACE_RESTRICTION_CACHE_PREFIX,
|
|
} from './ws.utils';
|
|
|
|
const snapshot: TreeNodeSnapshot = {
|
|
id: 'page-1',
|
|
slugId: 'slug-1',
|
|
title: 'Hello',
|
|
icon: '📄',
|
|
position: 'a1',
|
|
spaceId: 'space-1',
|
|
parentPageId: null,
|
|
};
|
|
|
|
describe('WsTreeService', () => {
|
|
let service: WsTreeService;
|
|
let wsService: {
|
|
emitTreeEvent: jest.Mock;
|
|
emitToSpaceRoom: jest.Mock;
|
|
emitMoveWithRestrictionSplit: jest.Mock;
|
|
};
|
|
let pagePermissionRepo: { hasRestrictedAncestor: jest.Mock };
|
|
|
|
beforeEach(async () => {
|
|
wsService = {
|
|
emitTreeEvent: jest.fn().mockResolvedValue(undefined),
|
|
emitToSpaceRoom: jest.fn(),
|
|
emitMoveWithRestrictionSplit: jest.fn().mockResolvedValue(undefined),
|
|
};
|
|
pagePermissionRepo = {
|
|
// Default: not restricted, so broadcastPageMoved skips the compensating
|
|
// delete unless a test opts in.
|
|
hasRestrictedAncestor: jest.fn().mockResolvedValue(false),
|
|
};
|
|
|
|
const module: TestingModule = await Test.createTestingModule({
|
|
providers: [
|
|
WsTreeService,
|
|
{ provide: WsService, useValue: wsService },
|
|
{ provide: PagePermissionRepo, useValue: pagePermissionRepo },
|
|
],
|
|
}).compile();
|
|
|
|
service = module.get<WsTreeService>(WsTreeService);
|
|
});
|
|
|
|
it('broadcastPageCreated emits addTreeNode with the expected shape', async () => {
|
|
await service.broadcastPageCreated(snapshot);
|
|
|
|
expect(wsService.emitTreeEvent).toHaveBeenCalledWith(
|
|
'space-1',
|
|
'page-1',
|
|
expect.objectContaining({
|
|
operation: 'addTreeNode',
|
|
spaceId: 'space-1',
|
|
payload: expect.objectContaining({
|
|
parentId: null,
|
|
index: 0,
|
|
data: expect.objectContaining({
|
|
id: 'page-1',
|
|
slugId: 'slug-1',
|
|
name: 'Hello',
|
|
title: 'Hello',
|
|
icon: '📄',
|
|
position: 'a1',
|
|
spaceId: 'space-1',
|
|
parentPageId: null,
|
|
hasChildren: false,
|
|
children: [],
|
|
}),
|
|
}),
|
|
}),
|
|
);
|
|
});
|
|
|
|
it('broadcastPageDeleted emits deleteTreeNode with the root node only', async () => {
|
|
await service.broadcastPageDeleted({
|
|
...snapshot,
|
|
parentPageId: 'parent-9',
|
|
});
|
|
|
|
expect(wsService.emitTreeEvent).toHaveBeenCalledWith(
|
|
'space-1',
|
|
'page-1',
|
|
expect.objectContaining({
|
|
operation: 'deleteTreeNode',
|
|
spaceId: 'space-1',
|
|
payload: {
|
|
node: { id: 'page-1', slugId: 'slug-1', parentPageId: 'parent-9' },
|
|
},
|
|
}),
|
|
);
|
|
});
|
|
|
|
it('broadcastPageMoved emits moveTreeNode with old + new parent and position', async () => {
|
|
const event: PageMovedEvent = {
|
|
workspaceId: 'ws-1',
|
|
oldParentId: 'old-parent',
|
|
hasChildren: true,
|
|
node: { ...snapshot, parentPageId: 'new-parent', position: 'a5' },
|
|
};
|
|
|
|
await service.broadcastPageMoved(event);
|
|
|
|
expect(wsService.emitTreeEvent).toHaveBeenCalledWith(
|
|
'space-1',
|
|
'page-1',
|
|
expect.objectContaining({
|
|
operation: 'moveTreeNode',
|
|
spaceId: 'space-1',
|
|
payload: expect.objectContaining({
|
|
id: 'page-1',
|
|
parentId: 'new-parent',
|
|
oldParentId: 'old-parent',
|
|
index: 0,
|
|
position: 'a5',
|
|
pageData: expect.objectContaining({
|
|
id: 'page-1',
|
|
slugId: 'slug-1',
|
|
position: 'a5',
|
|
parentPageId: 'new-parent',
|
|
hasChildren: true,
|
|
}),
|
|
}),
|
|
}),
|
|
);
|
|
});
|
|
|
|
it('broadcastPageMoved into an UNrestricted location does NOT emit a compensating delete', async () => {
|
|
pagePermissionRepo.hasRestrictedAncestor.mockResolvedValue(false);
|
|
|
|
const event: PageMovedEvent = {
|
|
workspaceId: 'ws-1',
|
|
oldParentId: 'old-parent',
|
|
hasChildren: false,
|
|
node: { ...snapshot, parentPageId: 'new-parent', position: 'a5' },
|
|
};
|
|
|
|
await service.broadcastPageMoved(event);
|
|
|
|
// Normal path: move goes to the whole room via emitTreeEvent, and the
|
|
// single-snapshot move/delete split does not fire.
|
|
expect(wsService.emitTreeEvent).toHaveBeenCalledTimes(1);
|
|
expect(wsService.emitMoveWithRestrictionSplit).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('broadcastPageMoved into a RESTRICTED subtree drives the move + compensating delete from ONE single-snapshot split call', async () => {
|
|
// Destination is now under a restricted ancestor.
|
|
pagePermissionRepo.hasRestrictedAncestor.mockResolvedValue(true);
|
|
|
|
const event: PageMovedEvent = {
|
|
workspaceId: 'ws-1',
|
|
oldParentId: 'old-parent',
|
|
hasChildren: false,
|
|
node: { ...snapshot, parentPageId: 'restricted-parent', position: 'a5' },
|
|
};
|
|
|
|
await service.broadcastPageMoved(event);
|
|
|
|
// The single fresh restriction decision was read exactly once...
|
|
expect(pagePermissionRepo.hasRestrictedAncestor).toHaveBeenCalledTimes(1);
|
|
expect(pagePermissionRepo.hasRestrictedAncestor).toHaveBeenCalledWith(
|
|
'page-1',
|
|
);
|
|
|
|
// ...and it must NOT go through the cache-gated room-wide emitTreeEvent,
|
|
// which could leak the move to the whole room during the stale-cache window.
|
|
expect(wsService.emitTreeEvent).not.toHaveBeenCalled();
|
|
|
|
// BOTH the move and the compensating delete are driven from ONE call, so a
|
|
// single socket/access snapshot partitions the room (no race window).
|
|
expect(wsService.emitMoveWithRestrictionSplit).toHaveBeenCalledTimes(1);
|
|
|
|
const [spaceId, pageId, movePayload, deletePayload] =
|
|
wsService.emitMoveWithRestrictionSplit.mock.calls[0];
|
|
|
|
expect(spaceId).toBe('space-1');
|
|
expect(pageId).toBe('page-1');
|
|
|
|
// The move payload is the moveTreeNode for the moved page.
|
|
expect(movePayload).toEqual(
|
|
expect.objectContaining({
|
|
operation: 'moveTreeNode',
|
|
spaceId: 'space-1',
|
|
payload: expect.objectContaining({ id: 'page-1' }),
|
|
}),
|
|
);
|
|
|
|
// The delete payload is the compensating deleteTreeNode, scoped to the same
|
|
// page id and carrying the OLD parent id (so it disappears from where it was
|
|
// last visible).
|
|
expect(deletePayload).toEqual(
|
|
expect.objectContaining({
|
|
operation: 'deleteTreeNode',
|
|
spaceId: 'space-1',
|
|
payload: {
|
|
node: expect.objectContaining({
|
|
id: 'page-1',
|
|
slugId: 'slug-1',
|
|
parentPageId: 'old-parent',
|
|
}),
|
|
},
|
|
}),
|
|
);
|
|
expect(deletePayload.payload.node.parentPageId).toBe(event.oldParentId);
|
|
});
|
|
|
|
it('broadcastRefetchRoot emits refetchRootTreeNodeEvent to the space room', async () => {
|
|
await service.broadcastRefetchRoot('space-7');
|
|
|
|
expect(wsService.emitToSpaceRoom).toHaveBeenCalledWith('space-7', {
|
|
operation: 'refetchRootTreeNodeEvent',
|
|
spaceId: 'space-7',
|
|
});
|
|
});
|
|
});
|
|
|
|
describe('WsService.emitTreeEvent', () => {
|
|
let service: WsService;
|
|
let pagePermissionRepo: {
|
|
hasRestrictedPagesInSpace: jest.Mock;
|
|
hasRestrictedAncestor: jest.Mock;
|
|
getUserIdsWithPageAccess: jest.Mock;
|
|
};
|
|
let cache: { get: jest.Mock; set: jest.Mock; del: jest.Mock };
|
|
let roomEmit: jest.Mock;
|
|
let server: any;
|
|
|
|
beforeEach(async () => {
|
|
pagePermissionRepo = {
|
|
hasRestrictedPagesInSpace: jest.fn(),
|
|
hasRestrictedAncestor: jest.fn(),
|
|
getUserIdsWithPageAccess: jest.fn(),
|
|
};
|
|
cache = {
|
|
get: jest.fn().mockResolvedValue(null),
|
|
set: jest.fn().mockResolvedValue(undefined),
|
|
del: jest.fn(),
|
|
};
|
|
|
|
const module: TestingModule = await Test.createTestingModule({
|
|
providers: [
|
|
WsService,
|
|
{ provide: PagePermissionRepo, useValue: pagePermissionRepo },
|
|
{ provide: CACHE_MANAGER, useValue: cache },
|
|
],
|
|
}).compile();
|
|
|
|
service = module.get<WsService>(WsService);
|
|
|
|
roomEmit = jest.fn();
|
|
server = {
|
|
to: jest.fn().mockReturnValue({ emit: roomEmit }),
|
|
in: jest.fn().mockReturnValue({ fetchSockets: jest.fn() }),
|
|
};
|
|
service.setServer(server);
|
|
});
|
|
|
|
it('open space: broadcasts to the whole space room', async () => {
|
|
pagePermissionRepo.hasRestrictedPagesInSpace.mockResolvedValue(false);
|
|
|
|
const data = { operation: 'addTreeNode' };
|
|
await service.emitTreeEvent('space-1', 'page-1', data);
|
|
|
|
expect(server.to).toHaveBeenCalledWith(getSpaceRoomName('space-1'));
|
|
expect(roomEmit).toHaveBeenCalledWith('message', data);
|
|
expect(pagePermissionRepo.hasRestrictedAncestor).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('restricted page: only authorized users receive the event', async () => {
|
|
pagePermissionRepo.hasRestrictedPagesInSpace.mockResolvedValue(true);
|
|
pagePermissionRepo.hasRestrictedAncestor.mockResolvedValue(true);
|
|
pagePermissionRepo.getUserIdsWithPageAccess.mockResolvedValue(['user-ok']);
|
|
|
|
const okEmit = jest.fn();
|
|
const noEmit = jest.fn();
|
|
const sockets = [
|
|
{ id: 's1', data: { userId: 'user-ok' }, emit: okEmit },
|
|
{ id: 's2', data: { userId: 'user-no' }, emit: noEmit },
|
|
];
|
|
server.in.mockReturnValue({
|
|
fetchSockets: jest.fn().mockResolvedValue(sockets),
|
|
});
|
|
|
|
const data = { operation: 'addTreeNode' };
|
|
await service.emitTreeEvent('space-1', 'page-1', data);
|
|
|
|
// Did NOT broadcast to the whole room.
|
|
expect(roomEmit).not.toHaveBeenCalled();
|
|
expect(okEmit).toHaveBeenCalledWith('message', data);
|
|
expect(noEmit).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('emitCommentEvent open space: broadcasts to the whole space room', async () => {
|
|
// emitCommentEvent forwards to the SAME unified restriction gate as
|
|
// emitTreeEvent, so the open-space fast path must behave identically.
|
|
pagePermissionRepo.hasRestrictedPagesInSpace.mockResolvedValue(false);
|
|
|
|
const data = { operation: 'addComment' };
|
|
await service.emitCommentEvent('space-1', 'page-1', data);
|
|
|
|
expect(server.to).toHaveBeenCalledWith(getSpaceRoomName('space-1'));
|
|
expect(roomEmit).toHaveBeenCalledWith('message', data);
|
|
expect(pagePermissionRepo.hasRestrictedAncestor).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('emitCommentEvent restricted page: only authorized users receive the event', async () => {
|
|
pagePermissionRepo.hasRestrictedPagesInSpace.mockResolvedValue(true);
|
|
pagePermissionRepo.hasRestrictedAncestor.mockResolvedValue(true);
|
|
pagePermissionRepo.getUserIdsWithPageAccess.mockResolvedValue(['user-ok']);
|
|
|
|
const okEmit = jest.fn();
|
|
const noEmit = jest.fn();
|
|
const sockets = [
|
|
{ id: 's1', data: { userId: 'user-ok' }, emit: okEmit },
|
|
{ id: 's2', data: { userId: 'user-no' }, emit: noEmit },
|
|
];
|
|
server.in.mockReturnValue({
|
|
fetchSockets: jest.fn().mockResolvedValue(sockets),
|
|
});
|
|
|
|
const data = { operation: 'addComment' };
|
|
await service.emitCommentEvent('space-1', 'page-1', data);
|
|
|
|
expect(roomEmit).not.toHaveBeenCalled();
|
|
expect(okEmit).toHaveBeenCalledWith('message', data);
|
|
expect(noEmit).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('invalidateSpaceRestrictionCache deletes the cached restriction verdict for that space only', async () => {
|
|
await service.invalidateSpaceRestrictionCache('space-42');
|
|
|
|
expect(cache.del).toHaveBeenCalledTimes(1);
|
|
expect(cache.del).toHaveBeenCalledWith(
|
|
`${WS_SPACE_RESTRICTION_CACHE_PREFIX}space-42`,
|
|
);
|
|
});
|
|
|
|
it('emitMoveWithRestrictionSplit partitions the room from one snapshot: authorized -> move, unauthorized + anonymous -> delete', async () => {
|
|
pagePermissionRepo.getUserIdsWithPageAccess.mockResolvedValue(['user-ok']);
|
|
|
|
const okEmit = jest.fn();
|
|
const noEmit = jest.fn();
|
|
const anonEmit = jest.fn();
|
|
const sockets = [
|
|
{ id: 's1', data: { userId: 'user-ok' }, emit: okEmit },
|
|
{ id: 's2', data: { userId: 'user-no' }, emit: noEmit },
|
|
// Unauthenticated socket (no userId) — must receive the delete.
|
|
{ id: 's3', data: {}, emit: anonEmit },
|
|
];
|
|
server.in.mockReturnValue({
|
|
fetchSockets: jest.fn().mockResolvedValue(sockets),
|
|
});
|
|
|
|
const movePayload = { operation: 'moveTreeNode' };
|
|
const deletePayload = { operation: 'deleteTreeNode' };
|
|
await service.emitMoveWithRestrictionSplit(
|
|
'space-1',
|
|
'page-1',
|
|
movePayload,
|
|
deletePayload,
|
|
);
|
|
|
|
// Authorized socket gets ONLY the move.
|
|
expect(okEmit).toHaveBeenCalledWith('message', movePayload);
|
|
expect(okEmit).not.toHaveBeenCalledWith('message', deletePayload);
|
|
// Unauthorized + anonymous sockets get ONLY the delete.
|
|
expect(noEmit).toHaveBeenCalledWith('message', deletePayload);
|
|
expect(noEmit).not.toHaveBeenCalledWith('message', movePayload);
|
|
expect(anonEmit).toHaveBeenCalledWith('message', deletePayload);
|
|
expect(anonEmit).not.toHaveBeenCalledWith('message', movePayload);
|
|
});
|
|
});
|
|
|
|
describe('move-into-restricted disjointness contract (WsTreeService + real WsService)', () => {
|
|
// CONTRACT: a move under a restricted ancestor PARTITIONS the room from a
|
|
// SINGLE snapshot. emitMoveWithRestrictionSplit performs exactly one
|
|
// fetchSockets + one getUserIdsWithPageAccess; the authorized set (gets the
|
|
// moveTreeNode) and its complement (gets the deleteTreeNode) are disjoint and
|
|
// together cover every socket — and an anonymous (no-userId) socket lands in
|
|
// the delete set. We wire a REAL WsService (only its repo, cache and socket
|
|
// server mocked) so the partition runs against the SAME fixed socket set, the
|
|
// way it does in production.
|
|
let treeService: WsTreeService;
|
|
let pagePermissionRepo: {
|
|
hasRestrictedPagesInSpace: jest.Mock;
|
|
hasRestrictedAncestor: jest.Mock;
|
|
getUserIdsWithPageAccess: jest.Mock;
|
|
};
|
|
let fetchSockets: jest.Mock;
|
|
|
|
// Fixed room: two authorized users (one with two sockets), one unauthorized
|
|
// user, one anonymous socket.
|
|
const moveSeen: string[] = [];
|
|
const deleteSeen: string[] = [];
|
|
|
|
const mkSocket = (id: string, userId: string | undefined) => ({
|
|
id,
|
|
data: userId ? { userId } : {},
|
|
emit: jest.fn((_event: string, payload: { operation: string }) => {
|
|
if (payload.operation === 'moveTreeNode') moveSeen.push(id);
|
|
if (payload.operation === 'deleteTreeNode') deleteSeen.push(id);
|
|
}),
|
|
});
|
|
|
|
const sockets = [
|
|
mkSocket('s-ok-1', 'user-ok'), // authorized, tab 1
|
|
mkSocket('s-ok-2', 'user-ok'), // authorized, tab 2 (fan-out)
|
|
mkSocket('s-no', 'user-no'), // unauthorized
|
|
mkSocket('s-anon', undefined), // anonymous (no userId)
|
|
];
|
|
|
|
beforeEach(async () => {
|
|
moveSeen.length = 0;
|
|
deleteSeen.length = 0;
|
|
|
|
pagePermissionRepo = {
|
|
hasRestrictedPagesInSpace: jest.fn().mockResolvedValue(true),
|
|
// The move destination IS under a restricted ancestor.
|
|
hasRestrictedAncestor: jest.fn().mockResolvedValue(true),
|
|
// Only user-ok is authorized to see the page.
|
|
getUserIdsWithPageAccess: jest.fn().mockResolvedValue(['user-ok']),
|
|
};
|
|
const cache = {
|
|
get: jest.fn().mockResolvedValue(null),
|
|
set: jest.fn().mockResolvedValue(undefined),
|
|
del: jest.fn().mockResolvedValue(undefined),
|
|
};
|
|
|
|
const module: TestingModule = await Test.createTestingModule({
|
|
providers: [
|
|
WsTreeService,
|
|
WsService,
|
|
{ provide: PagePermissionRepo, useValue: pagePermissionRepo },
|
|
{ provide: CACHE_MANAGER, useValue: cache },
|
|
],
|
|
}).compile();
|
|
|
|
const wsService = module.get<WsService>(WsService);
|
|
// Capture fetchSockets so the test can assert the SINGLE-snapshot contract:
|
|
// exactly one fetchSockets call drives the whole partition.
|
|
fetchSockets = jest.fn().mockResolvedValue(sockets);
|
|
const server = {
|
|
to: jest.fn().mockReturnValue({ emit: jest.fn() }),
|
|
in: jest.fn().mockReturnValue({ fetchSockets }),
|
|
};
|
|
wsService.setServer(server as never);
|
|
|
|
treeService = module.get<WsTreeService>(WsTreeService);
|
|
});
|
|
|
|
it('authorized set (move) and complement (delete) partition the room; anon is in delete', async () => {
|
|
const event: PageMovedEvent = {
|
|
workspaceId: 'ws-1',
|
|
oldParentId: 'old-parent',
|
|
hasChildren: false,
|
|
node: { ...snapshot, parentPageId: 'restricted-parent', position: 'a5' },
|
|
};
|
|
|
|
await treeService.broadcastPageMoved(event);
|
|
|
|
const moveSet = new Set(moveSeen);
|
|
const deleteSet = new Set(deleteSeen);
|
|
|
|
// Authorized user's BOTH sockets got the move; nobody else did.
|
|
expect(moveSet).toEqual(new Set(['s-ok-1', 's-ok-2']));
|
|
// Everyone else (unauthorized + anonymous) got the delete.
|
|
expect(deleteSet).toEqual(new Set(['s-no', 's-anon']));
|
|
|
|
// DISJOINT: no socket received both a move and a delete.
|
|
const intersection = [...moveSet].filter((id) => deleteSet.has(id));
|
|
expect(intersection).toEqual([]);
|
|
|
|
// PARTITION: the two sets together cover every socket in the room exactly.
|
|
const union = new Set([...moveSet, ...deleteSet]);
|
|
expect(union).toEqual(new Set(sockets.map((s) => s.id)));
|
|
|
|
// The anonymous socket specifically lands in the DELETE set, never the move.
|
|
expect(deleteSet.has('s-anon')).toBe(true);
|
|
expect(moveSet.has('s-anon')).toBe(false);
|
|
|
|
// SINGLE SNAPSHOT: the whole partition (move + compensating delete) is driven
|
|
// from exactly ONE fetchSockets and exactly ONE getUserIdsWithPageAccess.
|
|
// This is what closes the race window — there is no second, independent
|
|
// snapshot that could disagree with the first.
|
|
expect(fetchSockets).toHaveBeenCalledTimes(1);
|
|
expect(pagePermissionRepo.getUserIdsWithPageAccess).toHaveBeenCalledTimes(1);
|
|
});
|
|
});
|