From f3beb7f8ceb943309ed35075e1a021627cf7634e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Gonz=C3=A1lez?= <carlos.rgn@gmail.com>
Date: Tue, 8 Dec 2020 01:55:37 -0600
Subject: [PATCH] fix(knex): reject in `commit()` method if commit statement
 fails (#1177)

Closes #1176
---
 packages/knex/src/AbstractSqlConnection.ts |   3 +-
 tests/issues/GH1176.test.ts                | 220 +++++++++++++++++++++
 2 files changed, 222 insertions(+), 1 deletion(-)
 create mode 100644 tests/issues/GH1176.test.ts

diff --git a/packages/knex/src/AbstractSqlConnection.ts b/packages/knex/src/AbstractSqlConnection.ts
index d784435cfb0d..04c8fc572f82 100644
--- a/packages/knex/src/AbstractSqlConnection.ts
+++ b/packages/knex/src/AbstractSqlConnection.ts
@@ -39,7 +39,8 @@ export abstract class AbstractSqlConnection extends Connection {
   }
 
   async commit(ctx: KnexTransaction): Promise<void> {
-    return ctx.commit();
+    ctx.commit();
+    return ctx.executionPromise; // https://github.com/knex/knex/issues/3847#issuecomment-626330453
   }
 
   async rollback(ctx: KnexTransaction): Promise<void> {
diff --git a/tests/issues/GH1176.test.ts b/tests/issues/GH1176.test.ts
new file mode 100644
index 000000000000..b3924753d651
--- /dev/null
+++ b/tests/issues/GH1176.test.ts
@@ -0,0 +1,220 @@
+import {
+  EntityManager,
+  Entity,
+  MikroORM,
+  PrimaryKey,
+  Property,
+} from '@mikro-orm/core';
+import { PostgreSqlDriver } from '@mikro-orm/postgresql';
+import { v4 as uuid } from 'uuid';
+
+@Entity({ tableName: 'users' })
+class User {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property()
+  readonly username: string;
+
+  constructor(username: string) {
+    this.username = username;
+  }
+
+}
+
+async function getOrmInstance(): Promise<MikroORM<PostgreSqlDriver>> {
+  const orm = await MikroORM.init({
+    entities: [User],
+    dbName: 'mikro_orm_test_gh_1176',
+    type: 'postgresql',
+  });
+
+  return orm as MikroORM<PostgreSqlDriver>;
+}
+
+describe('GH issue 1176', () => {
+  let orm: MikroORM<PostgreSqlDriver>;
+  let em: EntityManager;
+
+  beforeAll(async () => {
+    orm = await getOrmInstance();
+    await orm.getSchemaGenerator().dropDatabase('mikro_orm_test_gh_1176');
+    await orm.getSchemaGenerator().createDatabase('mikro_orm_test_gh_1176');
+  });
+
+  afterAll(async () => {
+    await orm.close();
+  });
+
+  beforeEach(() => {
+    em = orm.em.fork();
+  });
+
+  describe('immediate constraint (failures on insert)', () => {
+    beforeAll(async () => {
+      const orm = await getOrmInstance();
+      await orm.em.getConnection().execute(
+        `
+          drop table if exists users;
+          create table users (
+            id serial primary key,
+            username varchar(50) not null unique deferrable initially immediate
+          );
+          `
+      );
+      await orm.close();
+    });
+    describe('implicit transactions', () => {
+      let username: string;
+      it('creating a new user succeeds', async () => {
+        username = uuid();
+        const user = new User(username);
+        em.persist(user);
+
+        await expect(em.flush()).resolves.toBeUndefined();
+      });
+      it('flush fails when a database constraint fails', async () => {
+        const user = new User(username);
+        em.persist(user);
+
+        await expect(em.flush()).rejects.toThrowError(
+          /^insert.+duplicate key value/
+        );
+      });
+    });
+
+    describe('explicit transactions with "transactional()"', () => {
+      let username: string;
+      it('creating a new user succeeds', async () => {
+        const work = em.transactional(async em => {
+          username = uuid();
+          const user = new User(username);
+          em.persist(user);
+        });
+
+        await expect(work).resolves.toBeUndefined();
+      });
+      it('transactional throws when a database constraint fails', async () => {
+        const work = em.transactional(async em => {
+          const user = new User(username);
+          em.persist(user);
+        });
+
+        await expect(work).rejects.toThrowError(/^insert.+duplicate key value/);
+      });
+    });
+
+    describe('explicit transactions with explicit begin/commit method calls', () => {
+      let username: string;
+      it('creating a new user succeeds', async () => {
+        await em.begin();
+        username = uuid();
+        const user = new User(username);
+        em.persist(user);
+
+        await expect(em.commit()).resolves.toBeUndefined();
+      });
+      it('commit throws when a database constraint fails', async () => {
+        await em.begin();
+        const work = async () => {
+          try {
+            const user = new User(username);
+            em.persist(user);
+            await em.commit();
+          } catch (error) {
+            await em.rollback();
+            throw error;
+          }
+        };
+
+        await expect(work).rejects.toThrowError(/^insert.+duplicate key value/);
+      });
+    });
+  });
+
+  describe('deferred constraints (failures on commit)', () => {
+    beforeAll(async () => {
+      const orm = await getOrmInstance();
+      await orm.em.getConnection().execute(
+        `
+        drop table if exists users;
+        create table users (
+          id serial primary key,
+          username varchar(50) not null unique deferrable initially deferred
+        );
+          `
+      );
+      await orm.close();
+    });
+
+    describe('implicit transactions', () => {
+      let username: string;
+      it('creating a new user succeeds', async () => {
+        username = uuid();
+        const user = new User(username);
+        em.persist(user);
+
+        await expect(em.flush()).resolves.toBeUndefined();
+      });
+      it('flush throws when a database constraint ffails', async () => {
+        const user = new User(username);
+        em.persist(user);
+
+        await expect(em.flush()).rejects.toThrowError(
+          /^COMMIT.+duplicate key value/
+        );
+      });
+    });
+
+    describe('explicit transactions with "transactional()"', () => {
+      let username: string;
+      it('creating a new user succeeds', async () => {
+        const work = em.transactional(async em => {
+          username = uuid();
+          const user = new User(username);
+          em.persist(user);
+        });
+
+        await expect(work).resolves.toBeUndefined();
+      });
+      it('transactional throws when a database constraint fails', async () => {
+        const work = async () => {
+          await em.transactional(async em => {
+            const user = new User(username);
+            em.persist(user);
+          });
+        };
+
+        await expect(work).rejects.toThrowError(/^COMMIT.+duplicate key value/);
+      });
+    });
+
+    describe('explicit transactions with explicit begin/commit/rollback method calls', () => {
+      let username: string;
+      it('creating a new user succeeds', async () => {
+        await em.begin();
+        username = uuid();
+        const user = new User(username);
+        em.persist(user);
+
+        await expect(em.commit()).resolves.toBeUndefined();
+      });
+      it('commit throws when a database constraint fails', async () => {
+        await em.begin();
+        const work = async () => {
+          try {
+            const user = new User(username);
+            em.persist(user);
+            await em.commit();
+          } catch (error) {
+            await em.rollback();
+            throw error;
+          }
+        };
+
+        await expect(work).rejects.toThrowError(/^COMMIT.+duplicate key value/);
+      });
+    });
+  });
+});