refactor(documents): extract document ownership validation to usecases (#1023)

README.md

@@ -39,7 +39,7 @@ A live demo of the platform is available at [demo.papra.app](https://demo.papra.
 
 ## Project Status
 
-Papra is under active development, the core functionalities are stable and usable. With lots of features and improvements added regularly. 
+Papra is under active development, the core functionalities are stable and usable. With lots of features and improvements added regularly.
 
 Feedback and bug reports are highly appreciated to help us improve the platform.
 
@@ -61,12 +61,12 @@ Feedback and bug reports are highly appreciated to help us improve the platform.
 - **CLI**: Manage your documents from the command line.
 - **API, SDK and webhooks**: Build your own applications on top of Papra.
 - **i18n**: Support for multiple languages.
-- *Coming soon:* **Document sharing**: Share documents with others.
-- *Coming soon:* **Document requests**: Generate upload links for people to add documents.
-- *Coming maybe one day:* **Mobile app**: Access and upload documents on the go.
-- *Coming maybe one day:* **Desktop app**: Access and upload documents from your computer.
-- *Coming maybe one day:* **Browser extension**: Upload documents from your browser.
-- *Coming maybe one day:* **AI**: Use AI to help you manage or tag your documents.
+- _Coming soon:_ **Document sharing**: Share documents with others.
+- _Coming soon:_ **Document requests**: Generate upload links for people to add documents.
+- _Coming maybe one day:_ **Mobile app**: Access and upload documents on the go.
+- _Coming maybe one day:_ **Desktop app**: Access and upload documents from your computer.
+- _Coming maybe one day:_ **Browser extension**: Upload documents from your browser.
+- _Coming maybe one day:_ **AI**: Use AI to help you manage or tag your documents.
 
 ## Support
 

apps/papra-server/src/modules/custom-properties/definitions/document-relation/document-relation.custom-property-definition.ts

@@ -10,13 +10,13 @@ export const documentRelationCustomPropertyDefinition = defineCustomPropertyType
     inputSchema: z.array(z.string()),
 
     extendInputValidation: async ({ value: documentIds, customProperty, documentsRepository }) => {
-      const { documents } = await documentsRepository.getDocumentsByIds({ documentIds, organizationId: customProperty.organizationId });
-      const foundIds = new Set(documents.map(d => d.id));
+      const allDocumentsAreFromOrganization = await documentsRepository.areAllDocumentsInOrganization({
+        documentIds,
+        organizationId: customProperty.organizationId,
+      });
 
-      for (const documentId of documentIds) {
-        if (!foundIds.has(documentId)) {
-          throw createCustomPropertyRelatedDocumentNotFoundError();
-        }
+      if (!allDocumentsAreFromOrganization) {
+        throw createCustomPropertyRelatedDocumentNotFoundError();
       }
     },
 

apps/papra-server/src/modules/documents/documents.repository.test.ts

@@ -188,4 +188,70 @@ describe('documents repository', () => {
       });
     });
   });
+
+  describe('areAllDocumentsInOrganization', async () => {
+    const { db } = await createInMemoryDatabase({
+      organizations: [
+        { id: 'org-1', name: 'Organization 1' },
+        { id: 'org-2', name: 'Organization 2' },
+      ],
+      documents: [
+        { id: 'doc-1', organizationId: 'org-1', mimeType: 'text/plain', originalStorageKey: 'org-1/originals/doc-1.txt', name: 'file.txt', originalName: 'file.txt', originalSha256Hash: 'hash1' },
+        { id: 'doc-2', organizationId: 'org-1', mimeType: 'text/plain', originalStorageKey: 'org-1/originals/doc-2.txt', name: 'file.txt', originalName: 'file.txt', originalSha256Hash: 'hash2' },
+        { id: 'doc-3', organizationId: 'org-2', mimeType: 'text/plain', originalStorageKey: 'org-2/originals/doc-3.txt', name: 'file.txt', originalName: 'file.txt', originalSha256Hash: 'hash3' },
+      ],
+    });
+
+    const documentsRepository = createDocumentsRepository({ db });
+
+    test('check if a given list of document IDs all belong to the organization', async () => {
+      expect(
+        await documentsRepository.areAllDocumentsInOrganization({
+          documentIds: ['doc-1', 'doc-2'],
+          organizationId: 'org-1',
+        }),
+      ).to.eql(true);
+
+      expect(
+        await documentsRepository.areAllDocumentsInOrganization({
+          documentIds: ['doc-1', 'doc-3'],
+          organizationId: 'org-1',
+        }),
+      ).to.eql(false);
+
+      expect(
+        await documentsRepository.areAllDocumentsInOrganization({
+          documentIds: ['doc-3'],
+          organizationId: 'org-1',
+        }),
+      ).to.eql(false);
+    });
+
+    test('no document IDs provided, returns true', async () => {
+      expect(
+        await documentsRepository.areAllDocumentsInOrganization({
+          documentIds: [],
+          organizationId: 'org-1',
+        }),
+      ).to.eql(true);
+    });
+
+    test('non existing document ID returns false', async () => {
+      expect(
+        await documentsRepository.areAllDocumentsInOrganization({
+          documentIds: ['non-existing-doc'],
+          organizationId: 'org-1',
+        }),
+      ).to.eql(false);
+    });
+
+    test('duplicated document IDs from the organizations are accepted and return true', async () => {
+      expect(
+        await documentsRepository.areAllDocumentsInOrganization({
+          documentIds: ['doc-1', 'doc-1', 'doc-2'],
+          organizationId: 'org-1',
+        }),
+      ).to.eql(true);
+    });
+  });
 });

apps/papra-server/src/modules/documents/documents.repository.ts

@@ -9,7 +9,7 @@ import { isUniqueConstraintError } from '../shared/db/constraints.models';
 import { withPagination } from '../shared/db/pagination';
 import { createError } from '../shared/errors/errors';
 import { omitUndefined } from '../shared/objects';
-import { isDefined, isNil } from '../shared/utils';
+import { isDefined, isNil, uniq } from '../shared/utils';
 import { createDocumentAlreadyExistsError, createDocumentNotFoundError } from './documents.errors';
 import { documentsTable } from './documents.table';
 
@@ -34,7 +34,7 @@ export function createDocumentsRepository({ db }: { db: Database }) {
       getAllOrganizationUndeletedDocumentsIterator,
       updateDocument,
       getGlobalDocumentsStats,
-      getDocumentsByIds,
+      areAllDocumentsInOrganization,
     },
     { db },
   );
@@ -127,24 +127,6 @@ async function getOrganizationDeletedDocuments({ organizationId, pageIndex, page
   };
 }
 
-async function getDocumentsByIds({ documentIds, organizationId, db }: { documentIds: string[]; organizationId: string; db: Database }) {
-  if (documentIds.length === 0) {
-    return { documents: [] };
-  }
-
-  const documents = await db
-    .select({ id: documentsTable.id })
-    .from(documentsTable)
-    .where(
-      and(
-        inArray(documentsTable.id, documentIds),
-        eq(documentsTable.organizationId, organizationId),
-      ),
-    );
-
-  return { documents };
-}
-
 async function getDocumentById({ documentId, organizationId, db }: { documentId: string; organizationId: string; db: Database }) {
   const [document] = await db
     .select()
@@ -371,3 +353,29 @@ async function getGlobalDocumentsStats({ db }: { db: Database }) {
     totalDocumentsSize: Number(totalDocumentsSize ?? 0),
   };
 }
+
+export async function areAllDocumentsInOrganization({ documentIds, organizationId, db }: { documentIds: string[]; organizationId: string; db: Database }) {
+  const deduplicatedDocumentIds = uniq(documentIds);
+
+  if (deduplicatedDocumentIds.length === 0) {
+    return true;
+  }
+
+  const documents = await db
+    .select({ id: documentsTable.id })
+    .from(documentsTable)
+    .where(
+      and(
+        inArray(documentsTable.id, deduplicatedDocumentIds),
+        eq(documentsTable.organizationId, organizationId),
+      ),
+    );
+
+  const foundDocumentIds = new Set(documents.map(d => d.id));
+
+  if (foundDocumentIds.size !== deduplicatedDocumentIds.length) {
+    return false;
+  }
+
+  return documentIds.every(documentId => foundDocumentIds.has(documentId));
+}