chore(tree): document the restriction-cache primitive; drop dead notify code
Release-cycle audit flagged WsService.invalidateSpaceRestrictionCache and WsTreeService.notifyPageRestricted/notifyPermissionGranted as never-wired dead code. Investigation: this community fork has NO page-permission grant/revoke/ restrict mutation site (the page-access repo mutators have zero callers — that flow is EE / not yet built), so there is nothing to wire them into. - Keep invalidateSpaceRestrictionCache (it's the one-line correctness primitive the future permission-mutation path must call to avoid the 30s stale-cache window) but document exactly that + add a test that it deletes only the space-scoped cache key. - Remove the untested, security-adjacent dead methods notifyPageRestricted / notifyPermissionGranted and their now-orphaned helpers emitToUsers / emitToSpaceExceptUsers (no remaining references; build confirms). A future permission-change realtime feature can reintroduce them wired + tested. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -7,7 +7,10 @@ import {
|
||||
PageMovedEvent,
|
||||
TreeNodeSnapshot,
|
||||
} from '../database/listeners/page.listener';
|
||||
import { getSpaceRoomName } from './ws.utils';
|
||||
import {
|
||||
getSpaceRoomName,
|
||||
WS_SPACE_RESTRICTION_CACHE_PREFIX,
|
||||
} from './ws.utils';
|
||||
|
||||
const snapshot: TreeNodeSnapshot = {
|
||||
id: 'page-1',
|
||||
@@ -291,6 +294,15 @@ describe('WsService.emitTreeEvent', () => {
|
||||
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('emitDeleteToUnauthorized sends ONLY to sockets whose user lacks page access', async () => {
|
||||
pagePermissionRepo.getUserIdsWithPageAccess.mockResolvedValue(['user-ok']);
|
||||
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { Page } from '@docmost/db/types/entity.types';
|
||||
import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo';
|
||||
import { WsService } from './ws.service';
|
||||
import {
|
||||
@@ -145,43 +144,4 @@ export class WsTreeService {
|
||||
spaceId,
|
||||
});
|
||||
}
|
||||
|
||||
async notifyPageRestricted(page: Page, excludeUserId: string): Promise<void> {
|
||||
await this.wsService.emitToSpaceExceptUsers(page.spaceId, [excludeUserId], {
|
||||
operation: 'deleteTreeNode',
|
||||
spaceId: page.spaceId,
|
||||
payload: {
|
||||
node: {
|
||||
id: page.id,
|
||||
slugId: page.slugId,
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
async notifyPermissionGranted(page: Page, userIds: string[]): Promise<void> {
|
||||
if (userIds.length === 0) return;
|
||||
|
||||
await this.wsService.emitToUsers(userIds, {
|
||||
operation: 'addTreeNode',
|
||||
spaceId: page.spaceId,
|
||||
payload: {
|
||||
parentId: page.parentPageId ?? null,
|
||||
index: 0,
|
||||
data: {
|
||||
id: page.id,
|
||||
slugId: page.slugId,
|
||||
name: page.title ?? '',
|
||||
title: page.title,
|
||||
icon: page.icon,
|
||||
position: page.position,
|
||||
spaceId: page.spaceId,
|
||||
parentPageId: page.parentPageId,
|
||||
creatorId: page.creatorId,
|
||||
hasChildren: false,
|
||||
children: [],
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,7 +8,6 @@ import {
|
||||
WS_SPACE_RESTRICTION_CACHE_PREFIX,
|
||||
WS_CACHE_TTL_MS,
|
||||
getSpaceRoomName,
|
||||
getUserRoomName,
|
||||
} from './ws.utils';
|
||||
|
||||
@Injectable()
|
||||
@@ -57,6 +56,22 @@ export class WsService {
|
||||
await this.broadcastToAuthorizedUsers(room, client.id, pageId, data);
|
||||
}
|
||||
|
||||
// Drop the cached spaceHasRestrictions verdict for a space. spaceHasRestrictions
|
||||
// caches "does this space have ANY restricted page" for WS_CACHE_TTL_MS (30s),
|
||||
// and emitTreeEvent / emitCommentEvent take a room-wide fast path when it is
|
||||
// false. The FIRST time a space gains a restriction (or loses its last one)
|
||||
// this cached verdict goes stale for up to the TTL, during which a title/icon-
|
||||
// bearing tree payload could fan out to the whole room. This MUST be called by
|
||||
// whatever code creates or removes a page's restriction (the page-access /
|
||||
// page-permission grant/revoke/restrict path), passing the affected page's
|
||||
// spaceId, so the next emit re-reads hasRestrictedPagesInSpace.
|
||||
//
|
||||
// NOTE: on this branch there is no permission-mutation site to call this from —
|
||||
// the page-access/page-permission repo mutators (insertPageAccess /
|
||||
// insertPagePermissions / deletePagePermission* / updatePagePermissionRole)
|
||||
// have ZERO callers in apps/server/src; PageAccessService only validates access.
|
||||
// This primitive is kept (and tested) so that flow, when it lands, has the
|
||||
// correct hook to invalidate the cache.
|
||||
async invalidateSpaceRestrictionCache(spaceId: string): Promise<void> {
|
||||
await this.cacheManager.del(
|
||||
`${WS_SPACE_RESTRICTION_CACHE_PREFIX}${spaceId}`,
|
||||
@@ -183,29 +198,6 @@ export class WsService {
|
||||
await this.broadcastToAuthorizedUsers(room, null, pageId, data);
|
||||
}
|
||||
|
||||
async emitToUsers(userIds: string[], data: any): Promise<void> {
|
||||
if (userIds.length === 0) return;
|
||||
const rooms = userIds.map((id) => getUserRoomName(id));
|
||||
this.server.to(rooms).emit('message', data);
|
||||
}
|
||||
|
||||
async emitToSpaceExceptUsers(
|
||||
spaceId: string,
|
||||
excludeUserIds: string[],
|
||||
data: any,
|
||||
): Promise<void> {
|
||||
const room = getSpaceRoomName(spaceId);
|
||||
const sockets = await this.server.in(room).fetchSockets();
|
||||
const excludeSet = new Set(excludeUserIds);
|
||||
|
||||
for (const socket of sockets) {
|
||||
const userId = socket.data.userId as string;
|
||||
if (userId && !excludeSet.has(userId)) {
|
||||
socket.emit('message', data);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
isTreeEvent(data: any): boolean {
|
||||
return TREE_EVENTS.has(data?.operation) && !!data?.spaceId;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user