fix(history-entry): remove composite primary keys

TypeORM promises to support composite primary keys,
but that does not work in reality.
This replaces the composite key used in the permission entities with
a single generated primary key and
a unique index on the relation columns.

See https://github.com/typeorm/typeorm/issues/8513

Signed-off-by: David Mehren <git@herrmehren.de>

src/history/history-entry.entity.ts

@@ -6,8 +6,9 @@
 import {
   Column,
   Entity,
+  Index,
   ManyToOne,
-  PrimaryColumn,
+  PrimaryGeneratedColumn,
   UpdateDateColumn,
 } from 'typeorm';
 
@@ -15,28 +16,22 @@ import { Note } from '../notes/note.entity';
 import { User } from '../users/user.entity';
 
 @Entity()
+@Index(['note', 'user'], { unique: true })
 export class HistoryEntry {
-  /**
+  @PrimaryGeneratedColumn()
-   * The `user` and `note` properties cannot be converted to promises
+  id: number;
-   * (to lazy-load them), as TypeORM gets confused about lazy composite
-   * primary keys.
-   * See https://github.com/typeorm/typeorm/issues/6908
-   */
-  @PrimaryColumn()
-  userId: string;
 
   @ManyToOne((_) => User, (user) => user.historyEntries, {
     onDelete: 'CASCADE',
+    orphanedRowAction: 'delete', // This ensures the whole row is deleted when the Entry stops being referenced
   })
-  user: User;
+  user: Promise<User>;
-
-  @PrimaryColumn()
-  noteId: string;
 
   @ManyToOne((_) => Note, (note) => note.historyEntries, {
     onDelete: 'CASCADE',
+    orphanedRowAction: 'delete', // This ensures the whole row is deleted when the Entry stops being referenced
   })
-  note: Note;
+  note: Promise<Note>;
 
   @Column()
   pinStatus: boolean;
@@ -56,8 +51,8 @@ export class HistoryEntry {
     pinStatus = false,
   ): Omit<HistoryEntry, 'updatedAt'> {
     const newHistoryEntry = new HistoryEntry();
-    newHistoryEntry.user = user;
+    newHistoryEntry.user = Promise.resolve(user);
-    newHistoryEntry.note = note;
+    newHistoryEntry.note = Promise.resolve(note);
     newHistoryEntry.pinStatus = pinStatus;
     return newHistoryEntry;
   }

src/history/history.service.spec.ts

@@ -178,10 +178,12 @@ describe('HistoryService', () => {
           Note.create(user, alias) as Note,
           user,
         );
-        expect(await createHistoryEntry.note.aliases).toHaveLength(1);
+        expect(await (await createHistoryEntry.note).aliases).toHaveLength(1);
-        expect((await createHistoryEntry.note.aliases)[0].name).toEqual(alias);
+        expect((await (await createHistoryEntry.note).aliases)[0].name).toEqual(
-        expect(await createHistoryEntry.note.owner).toEqual(user);
+          alias,
-        expect(createHistoryEntry.user).toEqual(user);
+        );
+        expect(await (await createHistoryEntry.note).owner).toEqual(user);
+        expect(await createHistoryEntry.user).toEqual(user);
         expect(createHistoryEntry.pinStatus).toEqual(false);
       });
 
@@ -196,10 +198,12 @@ describe('HistoryService', () => {
           Note.create(user, alias) as Note,
           user,
         );
-        expect(await createHistoryEntry.note.aliases).toHaveLength(1);
+        expect(await (await createHistoryEntry.note).aliases).toHaveLength(1);
-        expect((await createHistoryEntry.note.aliases)[0].name).toEqual(alias);
+        expect((await (await createHistoryEntry.note).aliases)[0].name).toEqual(
-        expect(await createHistoryEntry.note.owner).toEqual(user);
+          alias,
-        expect(createHistoryEntry.user).toEqual(user);
+        );
+        expect(await (await createHistoryEntry.note).owner).toEqual(user);
+        expect(await createHistoryEntry.user).toEqual(user);
         expect(createHistoryEntry.pinStatus).toEqual(false);
         expect(createHistoryEntry.updatedAt.getTime()).toBeGreaterThanOrEqual(
           historyEntry.updatedAt.getTime(),
@@ -231,10 +235,12 @@ describe('HistoryService', () => {
             pinStatus: true,
           },
         );
-        expect(await updatedHistoryEntry.note.aliases).toHaveLength(1);
+        expect(await (await updatedHistoryEntry.note).aliases).toHaveLength(1);
-        expect((await updatedHistoryEntry.note.aliases)[0].name).toEqual(alias);
+        expect(
-        expect(await updatedHistoryEntry.note.owner).toEqual(user);
+          (await (await updatedHistoryEntry.note).aliases)[0].name,
-        expect(updatedHistoryEntry.user).toEqual(user);
+        ).toEqual(alias);
+        expect(await (await updatedHistoryEntry.note).owner).toEqual(user);
+        expect(await updatedHistoryEntry.user).toEqual(user);
         expect(updatedHistoryEntry.pinStatus).toEqual(true);
       });
 
@@ -357,13 +363,13 @@ describe('HistoryService', () => {
         remove: jest
           .fn()
           .mockImplementationOnce(async (entry: HistoryEntry) => {
-            expect(await entry.note.aliases).toHaveLength(1);
+            expect(await (await entry.note).aliases).toHaveLength(1);
-            expect((await entry.note.aliases)[0].name).toEqual(alias);
+            expect((await (await entry.note).aliases)[0].name).toEqual(alias);
             expect(entry.pinStatus).toEqual(false);
           }),
-        save: jest.fn().mockImplementationOnce((entry: HistoryEntry) => {
+        save: jest.fn().mockImplementationOnce(async (entry: HistoryEntry) => {
-          expect(entry.note.aliases).toEqual(
+          expect((await entry.note).aliases).toEqual(
-            newlyCreatedHistoryEntry.note.aliases,
+            (await newlyCreatedHistoryEntry.note).aliases,
           );
           expect(entry.pinStatus).toEqual(newlyCreatedHistoryEntry.pinStatus);
           expect(entry.updatedAt).toEqual(newlyCreatedHistoryEntry.updatedAt);

src/history/history.service.ts

@@ -177,8 +177,8 @@ export class HistoryService {
     return {
       identifier: await getIdentifier(entry),
       lastVisitedAt: entry.updatedAt,
-      tags: await this.notesService.toTagList(entry.note),
+      tags: await this.notesService.toTagList(await entry.note),
-      title: entry.note.title ?? '',
+      title: (await entry.note).title ?? '',
       pinStatus: entry.pinStatus,
     };
   }

src/history/utils.ts

@@ -7,13 +7,13 @@ import { getPrimaryAlias } from '../notes/utils';
 import { HistoryEntry } from './history-entry.entity';
 
 export async function getIdentifier(entry: HistoryEntry): Promise<string> {
-  const aliases = await entry.note.aliases;
+  const aliases = await (await entry.note).aliases;
   if (!aliases || aliases.length === 0) {
-    return entry.note.publicId;
+    return (await entry.note).publicId;
   }
-  const primaryAlias = await getPrimaryAlias(entry.note);
+  const primaryAlias = await getPrimaryAlias(await entry.note);
   if (primaryAlias === undefined) {
-    return entry.note.publicId;
+    return (await entry.note).publicId;
   }
   return primaryAlias;
 }

src/notes/notes.service.spec.ts

@@ -313,7 +313,7 @@ describe('NotesService', () => {
         expect(revisions).toHaveLength(1);
         expect(revisions[0].content).toEqual(content);
         expect(await newNote.historyEntries).toHaveLength(1);
-        expect((await newNote.historyEntries)[0].user).toEqual(user);
+        expect(await (await newNote.historyEntries)[0].user).toEqual(user);
         expect(await newNote.userPermissions).toHaveLength(0);
         expect(await newNote.groupPermissions).toHaveLength(0);
         expect(await newNote.tags).toHaveLength(0);
@@ -338,7 +338,7 @@ describe('NotesService', () => {
         expect(revisions).toHaveLength(1);
         expect(revisions[0].content).toEqual(content);
         expect(await newNote.historyEntries).toHaveLength(1);
-        expect((await newNote.historyEntries)[0].user).toEqual(user);
+        expect(await (await newNote.historyEntries)[0].user).toEqual(user);
         expect(await newNote.userPermissions).toHaveLength(0);
         expect(await newNote.groupPermissions).toHaveLength(0);
         expect(await newNote.tags).toHaveLength(0);

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

@@ -101,16 +101,16 @@ describe('History', () => {
         .expect(201);
       const userEntries = await testSetup.historyService.getEntriesByUser(user);
       expect(userEntries.length).toEqual(1);
-      expect((await userEntries[0].note.aliases)[0].name).toEqual(
+      expect((await (await userEntries[0].note).aliases)[0].name).toEqual(
         (await note2.aliases)[0].name,
       );
-      expect((await userEntries[0].note.aliases)[0].primary).toEqual(
+      expect((await (await userEntries[0].note).aliases)[0].primary).toEqual(
         (await note2.aliases)[0].primary,
       );
-      expect((await userEntries[0].note.aliases)[0].id).toEqual(
+      expect((await (await userEntries[0].note).aliases)[0].id).toEqual(
         (await note2.aliases)[0].id,
       );
-      expect(userEntries[0].user.username).toEqual(user.username);
+      expect((await userEntries[0].user).username).toEqual(user.username);
       expect(userEntries[0].pinStatus).toEqual(pinStatus);
       expect(userEntries[0].updatedAt).toEqual(lastVisited);
     });
@@ -161,11 +161,13 @@ describe('History', () => {
           user,
         );
         expect(historyEntries).toHaveLength(1);
-        expect(await historyEntries[0].note.aliases).toEqual(
+        expect(await (await historyEntries[0].note).aliases).toEqual(
-          await prevEntry.note.aliases,
+          await (
+            await prevEntry.note
+          ).aliases,
         );
-        expect(historyEntries[0].user.username).toEqual(
+        expect((await historyEntries[0].user).username).toEqual(
-          prevEntry.user.username,
+          (await prevEntry.user).username,
         );
         expect(historyEntries[0].pinStatus).toEqual(prevEntry.pinStatus);
         expect(historyEntries[0].updatedAt).toEqual(prevEntry.updatedAt);
@@ -189,8 +191,9 @@ describe('History', () => {
       user,
     );
     expect(entry.pinStatus).toBeFalsy();
-    const alias = (await entry.note.aliases).filter((alias) => alias.primary)[0]
+    const alias = (await (await entry.note).aliases).filter(
-      .name;
+      (alias) => alias.primary,
+    )[0].name;
     await agent
       .put(`/api/private/me/history/${alias || 'null'}`)
       .send({ pinStatus: true })
@@ -203,8 +206,9 @@ describe('History', () => {
 
   it('DELETE /me/history/:note', async () => {
     const entry = await historyService.updateHistoryEntryTimestamp(note2, user);
-    const alias = (await entry.note.aliases).filter((alias) => alias.primary)[0]
+    const alias = (await (await entry.note).aliases).filter(
-      .name;
+      (alias) => alias.primary,
+    )[0].name;
     const entry2 = await historyService.updateHistoryEntryTimestamp(note, user);
     const entryDto = await historyService.toHistoryEntryDto(entry2);
     await agent

test/public-api/me.e2e-spec.ts

@@ -118,7 +118,7 @@ describe('Me', () => {
       let theEntry: HistoryEntryDto;
       for (const entry of history) {
         if (
-          (await entry.note.aliases).find(
+          (await (await entry.note).aliases).find(
             (element) => element.name === noteName,
           )
         ) {
@@ -147,7 +147,7 @@ describe('Me', () => {
       const history = await testSetup.historyService.getEntriesByUser(user);
       for (const entry of history) {
         if (
-          (await entry.note.aliases).find(
+          (await (await entry.note).aliases).find(
             (element) => element.name === noteName,
           )
         ) {