test(e2e): add more tests for POST /notes

src/api/private/notes/notes.controller.ts

@@ -88,7 +88,7 @@ export class NotesController {
   }
 
   @Post()
-  @OpenApi(201)
+  @OpenApi(201, 409, 413)
   @Permissions(Permission.CREATE)
   async createNote(
     @RequestUser() user: User,
@@ -101,7 +101,7 @@ export class NotesController {
   }
 
   @Post(':noteAlias')
-  @OpenApi(201, 400, 404, 409)
+  @OpenApi(201, 400, 404, 409, 413)
   @Permissions(Permission.CREATE)
   async createNamedNote(
     @RequestUser() user: User,

src/api/public/notes/notes.controller.ts

@@ -69,7 +69,7 @@ export class NotesController {
 
   @Permissions(Permission.CREATE)
   @Post()
-  @OpenApi(201, 403, 409)
+  @OpenApi(201, 403, 409, 413)
   async createNote(
     @RequestUser() user: User,
     @MarkdownBody() text: string,
@@ -112,6 +112,7 @@ export class NotesController {
     400,
     403,
     409,
+    413,
   )
   async createNamedNote(
     @RequestUser() user: User,

src/api/utils/descriptions.ts

@@ -22,5 +22,7 @@ export const unprocessableEntityDescription =
   "The request change can't be processed";
 export const conflictDescription =
   'The request conflicts with the current state of the application';
+export const payloadTooLargeDescription =
+  'The note is longer than the maximal allowed length of a note';
 export const internalServerErrorDescription =
   'The request triggered an internal server error.';

src/api/utils/openapi.decorator.ts

@@ -25,6 +25,7 @@ import {
   noContentDescription,
   notFoundDescription,
   okDescription,
+  payloadTooLargeDescription,
   unauthorizedDescription,
 } from './descriptions';
 
@@ -37,6 +38,7 @@ export type HttpStatusCodes =
   | 403
   | 404
   | 409
+  | 413
   | 500;
 
 /**
@@ -156,6 +158,13 @@ export const OpenApi = (
           }),
         );
         break;
+      case 413:
+        decoratorsToApply.push(
+          ApiConflictResponse({
+            description: description ?? payloadTooLargeDescription,
+          }),
+        );
+        break;
       case 500:
         decoratorsToApply.push(
           ApiInternalServerErrorResponse({

src/errors/error-mapping.ts

@@ -10,6 +10,7 @@ import {
   ConflictException,
   InternalServerErrorException,
   NotFoundException,
+  PayloadTooLargeException,
   UnauthorizedException,
 } from '@nestjs/common';
 import { HttpException } from '@nestjs/common/exceptions/http.exception';
@@ -70,6 +71,10 @@ const mapOfHedgeDocErrorsToHttpErrors: Map<string, HttpExceptionConstructor> =
       'PasswordTooWeakError',
       (object): HttpException => new BadRequestException(object),
     ],
+    [
+      'MaximumDocumentLengthExceededError',
+      (object): HttpException => new PayloadTooLargeException(object),
+    ],
   ]);
 
 @Catch()

src/errors/errors.ts

@@ -55,3 +55,7 @@ export class NoLocalIdentityError extends Error {
 export class PasswordTooWeakError extends Error {
   name = 'PasswordTooWeakError';
 }
+
+export class MaximumDocumentLengthExceededError extends Error {
+  name = 'MaximumDocumentLengthExceededError';
+}

src/notes/note.entity.ts

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -29,7 +29,7 @@ export class Note {
   @PrimaryGeneratedColumn()
   id: number;
 
-  @Column({ type: 'text' })
+  @Column({ type: 'text', unique: true })
   publicId: string;
 
   @OneToMany(

src/notes/notes.service.spec.ts

@@ -28,6 +28,7 @@ import { NoteConfig } from '../config/note.config';
 import {
   AlreadyInDBError,
   ForbiddenIdError,
+  MaximumDocumentLengthExceededError,
   NotInDBError,
 } from '../errors/errors';
 import { eventModuleConfig, NoteEvent } from '../events';
@@ -464,6 +465,37 @@ describe('NotesService', () => {
         expect(await newNote.aliases).toHaveLength(1);
         expect((await newNote.aliases)[0].name).toEqual(alias);
       });
+      describe('with maxDocumentLength 1000', () => {
+        beforeEach(() => (noteMockConfig.maxDocumentLength = 1000));
+        it('and content has length maxDocumentLength', async () => {
+          const content = 'x'.repeat(noteMockConfig.maxDocumentLength);
+          const newNote = await service.createNote(content, user, alias);
+          const revisions = await newNote.revisions;
+          expect(revisions).toHaveLength(1);
+          expect(revisions[0].content).toEqual(content);
+          expect(await newNote.historyEntries).toHaveLength(1);
+          expect(await (await newNote.historyEntries)[0].user).toEqual(user);
+          expect(await newNote.userPermissions).toHaveLength(0);
+          const groupPermissions = await newNote.groupPermissions;
+          expect(groupPermissions).toHaveLength(2);
+          expect(groupPermissions[0].canEdit).toEqual(
+            everyoneDefaultAccessPermission === DefaultAccessPermission.WRITE,
+          );
+          expect((await groupPermissions[0].group).name).toEqual(
+            SpecialGroup.EVERYONE,
+          );
+          expect(groupPermissions[1].canEdit).toEqual(
+            loggedinDefaultAccessPermission === DefaultAccessPermission.WRITE,
+          );
+          expect((await groupPermissions[1].group).name).toEqual(
+            SpecialGroup.LOGGED_IN,
+          );
+          expect(await newNote.tags).toHaveLength(0);
+          expect(await newNote.owner).toEqual(user);
+          expect(await newNote.aliases).toHaveLength(1);
+          expect((await newNote.aliases)[0].name).toEqual(alias);
+        });
+      });
       describe('with other', () => {
         beforeEach(
           () =>
@@ -510,6 +542,19 @@ describe('NotesService', () => {
           AlreadyInDBError,
         );
       });
+      describe('with maxDocumentLength 1000', () => {
+        beforeEach(() => (noteMockConfig.maxDocumentLength = 1000));
+        it('document is too long', async () => {
+          mockGroupRepo();
+          jest.spyOn(noteRepo, 'save').mockImplementationOnce(async () => {
+            throw new Error();
+          });
+          const content = 'x'.repeat(noteMockConfig.maxDocumentLength + 1);
+          await expect(
+            service.createNote(content, user, alias),
+          ).rejects.toThrow(MaximumDocumentLengthExceededError);
+        });
+      });
     });
   });
 

src/notes/notes.service.ts

@@ -14,6 +14,7 @@ import noteConfiguration, { NoteConfig } from '../config/note.config';
 import {
   AlreadyInDBError,
   ForbiddenIdError,
+  MaximumDocumentLengthExceededError,
   NotInDBError,
 } from '../errors/errors';
 import { NoteEvent } from '../events';
@@ -79,11 +80,12 @@ export class NotesService {
    * @async
    * Create a new note.
    * @param {string} noteContent - the content the new note should have
-   * @param {string=} alias - a optional alias the note should have
+   * @param {string=} alias - an optional alias the note should have
    * @param {User=} owner - the owner of the note
    * @return {Note} the newly created note
    * @throws {AlreadyInDBError} a note with the requested id or alias already exists
    * @throws {ForbiddenIdError} the requested id or alias is forbidden
+   * @throws {MaximumDocumentLengthExceededError} the noteContent is longer than the maxDocumentLength
    */
   async createNote(
     noteContent: string,
@@ -94,6 +96,9 @@ export class NotesService {
       this.checkNoteIdOrAlias(alias);
     }
     const newNote = Note.create(owner, alias);
+    if (noteContent.length > this.noteConfig.maxDocumentLength) {
+      throw new MaximumDocumentLengthExceededError();
+    }
     //TODO: Calculate patch
     newNote.revisions = Promise.resolve([
       Revision.create(noteContent, noteContent, newNote as Note) as Revision,
@@ -134,6 +139,14 @@ export class NotesService {
         throw new AlreadyInDBError(
           `A note with the alias '${alias}' already exists.`,
         );
+      } else if ((e as Error).message.includes('publicId')) {
+        this.logger.debug(
+          `A note with the publicId '${newNote.publicId}' already exists.`,
+          'createNote',
+        );
+        throw new AlreadyInDBError(
+          `A note with the publicId '${newNote.publicId}' already exists.`,
+        );
       } else {
         throw e;
       }

test/private-api/notes.e2e-spec.ts

@@ -8,6 +8,7 @@ import { join } from 'path';
 import request from 'supertest';
 
 import { NotInDBError } from '../../src/errors/errors';
+import * as utils from '../../src/notes/utils';
 import { User } from '../../src/users/user.entity';
 import { TestSetup, TestSetupBuilder } from '../test-setup';
 
@@ -55,21 +56,55 @@ describe('Notes', () => {
     await testSetup.cleanup();
   });
 
-  it('POST /notes', async () => {
-    const response = await agent
-      .post('/api/private/notes')
-      .set('Content-Type', 'text/markdown')
-      .send(content)
-      .expect('Content-Type', /json/)
-      .expect(201);
-    expect(response.body.metadata?.id).toBeDefined();
-    expect(
-      await testSetup.notesService.getNoteContent(
-        await testSetup.notesService.getNoteByIdOrAlias(
-          response.body.metadata.id,
+  describe('POST /notes', () => {
+    it('creates a note', async () => {
+      const response = await agent
+        .post('/api/private/notes')
+        .set('Content-Type', 'text/markdown')
+        .send(content)
+        .expect('Content-Type', /json/)
+        .expect(201);
+      expect(response.body.metadata?.id).toBeDefined();
+      expect(
+        await testSetup.notesService.getNoteContent(
+          await testSetup.notesService.getNoteByIdOrAlias(
+            response.body.metadata.id,
+          ),
         ),
-      ),
-    ).toEqual(content);
+      ).toEqual(content);
+    });
+    describe('does not create a note', () => {
+      it('if content exceeds maxDocumentLength', async () => {
+        const content = 'x'.repeat(
+          (testSetup.configService.get('noteConfig')
+            .maxDocumentLength as number) + 1,
+        );
+        await agent
+          .post('/api/private/notes')
+          .set('Content-Type', 'text/markdown')
+          .send(content)
+          .expect('Content-Type', /json/)
+          .expect(413);
+      });
+      it('if publicId already exists', async () => {
+        // This should not happen, but you at least theoretical it's possible
+        const response = await agent
+          .post('/api/private/notes')
+          .set('Content-Type', 'text/markdown')
+          .send(content)
+          .expect('Content-Type', /json/)
+          .expect(201);
+        jest
+          .spyOn(utils, 'generatePublicId')
+          .mockReturnValueOnce(response.body.metadata?.id);
+        await agent
+          .post('/api/private/notes')
+          .set('Content-Type', 'text/markdown')
+          .send(content)
+          .expect('Content-Type', /json/)
+          .expect(409);
+      });
+    });
   });
 
   describe('GET /notes/{note}', () => {
@@ -126,6 +161,19 @@ describe('Notes', () => {
         .expect('Content-Type', /json/)
         .expect(409);
     });
+
+    it('fails with a content, that is too long', async () => {
+      const content = 'x'.repeat(
+        (testSetup.configService.get('noteConfig')
+          .maxDocumentLength as number) + 1,
+      );
+      await agent
+        .post('/api/private/notes/test2')
+        .set('Content-Type', 'text/markdown')
+        .send(content)
+        .expect('Content-Type', /json/)
+        .expect(413);
+    });
   });
 
   describe('DELETE /notes/{note}', () => {