fix: advisory lock for page move

apps/server/src/core/page/services/page.service.ts

@@ -1,5 +1,6 @@
 import {
   BadRequestException,
+  ConflictException,
   Injectable,
   Logger,
   NotFoundException,
@@ -20,7 +21,7 @@ import { generateJitteredKeyBetween } from 'fractional-indexing-jittered';
 import { MovePageDto } from '../dto/move-page.dto';
 import { generateSlugId } from '../../../common/helpers';
 import { getPageTitle } from '../../../common/helpers';
-import { executeTx } from '@docmost/db/utils';
+import { dbOrTx, executeTx } from '@docmost/db/utils';
 import { AttachmentRepo } from '@docmost/db/repos/attachment/attachment.repo';
 import { v7 as uuid7 } from 'uuid';
 import {
@@ -174,10 +175,14 @@ export class PageService {
     return page;
   }
 
-  async nextPagePosition(spaceId: string, parentPageId?: string) {
+  async nextPagePosition(
+    spaceId: string,
+    parentPageId?: string,
+    trx?: KyselyTransaction,
+  ) {
     let pagePosition: string;
 
-    const lastPageQuery = this.db
+    const lastPageQuery = dbOrTx(this.db, trx)
       .selectFrom('pages')
       .select(['position'])
       .where('spaceId', '=', spaceId)
@@ -391,22 +396,33 @@ export class PageService {
   }
 
   async movePageToSpace(rootPage: Page, spaceId: string, userId: string) {
-    let childPageIds: string[] = [];
+    return executeTx(this.db, async (trx) => {
+      await this.pageRepo.lockPageHierarchySpaces(
+        [rootPage.spaceId, spaceId],
+        trx,
+      );
 
-    const allPages = await this.pageRepo.getPageAndDescendants(rootPage.id, {
-      includeContent: false,
+      const currentRootPage = await this.pageRepo.findById(rootPage.id, {
+        trx,
       });
+      if (!currentRootPage || currentRootPage.deletedAt) {
+        throw new NotFoundException('Page to move not found');
+      }
+      if (currentRootPage.spaceId !== rootPage.spaceId) {
+        throw new ConflictException('Page location changed; retry the move');
+      }
 
-    // Filter to only accessible pages while maintaining tree integrity
+      const allPages = await this.pageRepo.getPageAndDescendants(
+        currentRootPage.id,
+        { includeContent: false, trx },
+      );
       const accessiblePages = await this.filterAccessibleTreePages(
         allPages,
-      rootPage.id,
+        currentRootPage.id,
         userId,
-      rootPage.spaceId,
+        currentRootPage.spaceId,
       );
       const accessibleIds = new Set(accessiblePages.map((p) => p.id));
-
-    // Find inaccessible pages whose parent is being moved - these need to be orphaned
       const pagesToOrphan = allPages.filter(
         (p) =>
           !accessibleIds.has(p.id) &&
@@ -414,12 +430,12 @@ export class PageService {
           accessibleIds.has(p.parentPageId),
       );
 
-    await executeTx(this.db, async (trx) => {
       // Orphan inaccessible child pages (make them root pages in original space)
       for (const page of pagesToOrphan) {
         const orphanPosition = await this.nextPagePosition(
-          rootPage.spaceId,
+          currentRootPage.spaceId,
           null,
+          trx,
         );
         await this.pageRepo.updatePage(
           { parentPageId: null, position: orphanPosition },
@@ -429,16 +445,18 @@ export class PageService {
       }
 
       // Update root page
-      const nextPosition = await this.nextPagePosition(spaceId);
+      const nextPosition = await this.nextPagePosition(spaceId, null, trx);
       await this.pageRepo.updatePage(
         { spaceId, parentPageId: null, position: nextPosition },
-        rootPage.id,
+        currentRootPage.id,
         trx,
       );
 
       const pageIdsToMove = accessiblePages.map((p) => p.id);
 
-      childPageIds = pageIdsToMove.filter((id) => id !== rootPage.id);
+      const childPageIds = pageIdsToMove.filter(
+        (id) => id !== currentRootPage.id,
+      );
 
       if (pageIdsToMove.length > 1) {
         // Update sub pages (all accessible pages except root)
@@ -501,7 +519,7 @@ export class PageService {
           {
             pageIds: pageIdsToMove,
             spaceId,
-            workspaceId: rootPage.workspaceId,
+            workspaceId: currentRootPage.workspaceId,
           },
           {
             attempts: 2,
@@ -512,9 +530,9 @@ export class PageService {
           },
         );
       }
-    });
 
       return { childPageIds };
+    });
   }
 
   async duplicatePage(
@@ -825,20 +843,46 @@ export class PageService {
       throw new BadRequestException('A page cannot be its own parent');
     }
 
+    await executeTx(this.db, async (trx) => {
+      await this.pageRepo.lockPageHierarchySpaces(
+        [movedPage.spaceId],
+        trx,
+      );
+
+      const currentPage = await this.pageRepo.findById(dto.pageId, { trx });
+      if (!currentPage || currentPage.deletedAt) {
+        throw new NotFoundException('Moved page not found');
+      }
+      if (currentPage.spaceId !== movedPage.spaceId) {
+        throw new ConflictException('Page location changed; retry the move');
+      }
+
       let parentPageId = null;
-    if (movedPage.parentPageId === dto.parentPageId) {
+      if (currentPage.parentPageId === dto.parentPageId) {
         parentPageId = undefined;
       } else {
-      // changing the page's parent
         if (dto.parentPageId) {
-        const parentPage = await this.pageRepo.findById(dto.parentPageId);
+          const parentPage = await this.pageRepo.findById(dto.parentPageId, {
+            trx,
+          });
           if (
             !parentPage ||
             parentPage.deletedAt ||
-          parentPage.spaceId !== movedPage.spaceId
+            parentPage.spaceId !== currentPage.spaceId
           ) {
             throw new NotFoundException('Parent page not found');
           }
+          if (
+            await this.pageRepo.isPageDescendant(
+              dto.pageId,
+              parentPage.id,
+              trx,
+            )
+          ) {
+            throw new BadRequestException(
+              'A page cannot be moved under its descendant',
+            );
+          }
           parentPageId = parentPage.id;
         }
       }
@@ -849,7 +893,9 @@ export class PageService {
           parentPageId: parentPageId,
         },
         dto.pageId,
+        trx,
       );
+    });
   }
 
   async getPageBreadCrumbs(childPageId: string) {

apps/server/src/database/repos/page/page.repo.ts

@@ -161,6 +161,22 @@ export class PageRepo {
     return result;
   }
 
+  async lockPageHierarchySpaces(
+    spaceIds: string[],
+    trx: KyselyTransaction,
+  ): Promise<void> {
+    const sortedSpaceIds = [...new Set(spaceIds)].sort();
+
+    for (const spaceId of sortedSpaceIds) {
+      await sql`
+        SELECT pg_advisory_xact_lock(
+          hashtext('page-hierarchy'),
+          hashtext(${spaceId})
+        )
+      `.execute(trx);
+    }
+  }
+
   async insertPage(
     insertablePage: InsertablePage,
     trx?: KyselyTransaction,
@@ -489,9 +505,9 @@ export class PageRepo {
 
   async getPageAndDescendants(
     parentPageId: string,
-    opts: { includeContent: boolean },
+    opts: { includeContent: boolean; trx?: KyselyTransaction },
   ) {
-    return this.db
+    return dbOrTx(this.db, opts.trx)
       .withRecursive('page_hierarchy', (db) =>
         db
           .selectFrom('pages')
@@ -535,6 +551,36 @@ export class PageRepo {
       .execute();
   }
 
+  async isPageDescendant(
+    ancestorPageId: string,
+    descendantPageId: string,
+    trx?: KyselyTransaction,
+  ): Promise<boolean> {
+    const result = await dbOrTx(this.db, trx)
+      .withRecursive('page_ancestors', (db) =>
+        db
+          .selectFrom('pages')
+          .select(['id', 'parentPageId'])
+          .where('id', '=', descendantPageId)
+          .union((exp) =>
+            exp
+              .selectFrom('pages as parent')
+              .select(['parent.id', 'parent.parentPageId'])
+              .innerJoin(
+                'page_ancestors as ancestor',
+                'ancestor.parentPageId',
+                'parent.id',
+              ),
+          ),
+      )
+      .selectFrom('page_ancestors')
+      .select('id')
+      .where('id', '=', ancestorPageId)
+      .executeTakeFirst();
+
+    return Boolean(result);
+  }
+
   /**
    * Get page and all descendants, excluding restricted pages and their subtrees.
    * More efficient than getPageAndDescendants + filtering because: