From d87980ae6d3666233898900ac6232555d722d0ad Mon Sep 17 00:00:00 2001
From: David Mehren <git@herrmehren.de>
Date: Thu, 29 Apr 2021 15:03:44 +0200
Subject: [PATCH] RevisionsService: Throw NotInDBError on empty DB result

This adds error handling to various getters, so they throw a
NotInDBError instead of (illegally, according to the type) returning
null.

Signed-off-by: David Mehren <git@herrmehren.de>
---
 src/revisions/revisions.service.spec.ts | 24 ++++++++++++++++++++++--
 src/revisions/revisions.service.ts      | 21 ++++++++++++++++++---
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/src/revisions/revisions.service.spec.ts b/src/revisions/revisions.service.spec.ts
index c509d5fd7..2d1226856 100644
--- a/src/revisions/revisions.service.spec.ts
+++ b/src/revisions/revisions.service.spec.ts
@@ -6,6 +6,8 @@
 
 import { Test, TestingModule } from '@nestjs/testing';
 import { getRepositoryToken } from '@nestjs/typeorm';
+import { Repository } from 'typeorm';
+import { NotInDBError } from '../errors/errors';
 import { LoggerModule } from '../logger/logger.module';
 import { AuthorColor } from '../notes/author-color.entity';
 import { Note } from '../notes/note.entity';
@@ -25,6 +27,7 @@ import appConfigMock from '../config/mock/app.config.mock';
 
 describe('RevisionsService', () => {
   let service: RevisionsService;
+  let revisionRepo: Repository<Revision>;
 
   beforeEach(async () => {
     const module: TestingModule = await Test.createTestingModule({
@@ -32,7 +35,7 @@ describe('RevisionsService', () => {
         RevisionsService,
         {
           provide: getRepositoryToken(Revision),
-          useValue: {},
+          useClass: Repository,
         },
       ],
       imports: [
@@ -57,7 +60,7 @@ describe('RevisionsService', () => {
       .overrideProvider(getRepositoryToken(Note))
       .useValue({})
       .overrideProvider(getRepositoryToken(Revision))
-      .useValue({})
+      .useClass(Repository)
       .overrideProvider(getRepositoryToken(Tag))
       .useValue({})
       .overrideProvider(getRepositoryToken(NoteGroupPermission))
@@ -69,9 +72,26 @@ describe('RevisionsService', () => {
       .compile();
 
     service = module.get<RevisionsService>(RevisionsService);
+    revisionRepo = module.get<Repository<Revision>>(
+      getRepositoryToken(Revision),
+    );
   });
 
   it('should be defined', () => {
     expect(service).toBeDefined();
   });
+
+  describe('getRevision', () => {
+    it('returns a revision', async () => {
+      const revision = Revision.create('', '');
+      jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(revision);
+      expect(await service.getRevision({} as Note, 1)).toEqual(revision);
+    });
+    it('throws if the revision is not in the databse', async () => {
+      jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(undefined);
+      await expect(service.getRevision({} as Note, 1)).rejects.toThrow(
+        NotInDBError,
+      );
+    });
+  });
 });
diff --git a/src/revisions/revisions.service.ts b/src/revisions/revisions.service.ts
index 22774b945..0655a441a 100644
--- a/src/revisions/revisions.service.ts
+++ b/src/revisions/revisions.service.ts
@@ -13,6 +13,7 @@ import { RevisionMetadataDto } from './revision-metadata.dto';
 import { RevisionDto } from './revision.dto';
 import { Revision } from './revision.entity';
 import { Note } from '../notes/note.entity';
+import { NotInDBError } from '../errors/errors';
 
 @Injectable()
 export class RevisionsService {
@@ -34,16 +35,22 @@ export class RevisionsService {
   }
 
   async getRevision(note: Note, revisionId: number): Promise<Revision> {
-    return await this.revisionRepository.findOne({
+    const revision = await this.revisionRepository.findOne({
       where: {
         id: revisionId,
         note: note,
       },
     });
+    if (revision === undefined) {
+      throw new NotInDBError(
+        `Revision with ID ${revisionId} for note ${note.id} not found.`,
+      );
+    }
+    return revision;
   }
 
   async getLatestRevision(noteId: string): Promise<Revision> {
-    return await this.revisionRepository.findOne({
+    const revision = await this.revisionRepository.findOne({
       where: {
         note: noteId,
       },
@@ -52,10 +59,14 @@ export class RevisionsService {
         id: 'DESC',
       },
     });
+    if (revision === undefined) {
+      throw new NotInDBError(`Revision for note ${noteId} not found.`);
+    }
+    return revision;
   }
 
   async getFirstRevision(noteId: string): Promise<Revision> {
-    return await this.revisionRepository.findOne({
+    const revision = await this.revisionRepository.findOne({
       where: {
         note: noteId,
       },
@@ -63,6 +74,10 @@ export class RevisionsService {
         createdAt: 'ASC',
       },
     });
+    if (revision === undefined) {
+      throw new NotInDBError(`Revision for note ${noteId} not found.`);
+    }
+    return revision;
   }
 
   toRevisionMetadataDto(revision: Revision): RevisionMetadataDto {