From 0463855223100028e62f7cb2e84319770f54449e Mon Sep 17 00:00:00 2001
From: James Ward <james@notjam.es>
Date: Wed, 30 Jun 2021 01:11:31 -0400
Subject: [PATCH] fix: handle postgres default when tableColumn.default is not
 string (#7816)

there was an edge case where the tableColumn.default was not a string
and thus caused a TypeError to be emitted when we tried to `substring`
it.  instead, we check the type first and only if that type is a
string do we try to decode it
---
 src/driver/postgres/PostgresDriver.ts | 25 +++++++++++------------
 test/functional/json/entity/Record.ts |  7 ++++++-
 test/functional/json/jsonb.ts         | 29 +++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/src/driver/postgres/PostgresDriver.ts b/src/driver/postgres/PostgresDriver.ts
index cc3ef3a289..10c5fd616b 100644
--- a/src/driver/postgres/PostgresDriver.ts
+++ b/src/driver/postgres/PostgresDriver.ts
@@ -766,21 +766,20 @@ export class PostgresDriver implements Driver {
      * Compares "default" value of the column.
      * Postgres sorts json values before it is saved, so in that case a deep comparison has to be performed to see if has changed.
      */
-    defaultEqual(columnMetadata: ColumnMetadata, tableColumn: TableColumn): boolean {
-      if (Array<ColumnType>("json", "jsonb").indexOf(columnMetadata.type) >= 0
-       && typeof columnMetadata.default !== "function"
-       && columnMetadata.default !== undefined) {
-        let columnDefault = columnMetadata.default;
-        if (typeof columnDefault !== "object") {
-          columnDefault = JSON.parse(columnMetadata.default);
+    private defaultEqual(columnMetadata: ColumnMetadata, tableColumn: TableColumn): boolean {
+        if (
+            ["json", "jsonb"].includes(columnMetadata.type as string)
+            && !["function", "undefined"].includes(typeof columnMetadata.default)
+        ) {
+            const tableColumnDefault = typeof tableColumn.default === "string" ?
+                JSON.parse(tableColumn.default.substring(1, tableColumn.default.length-1)) :
+                tableColumn.default;
+
+            return OrmUtils.deepCompare(columnMetadata.default, tableColumnDefault);
         }
-        let tableDefault = JSON.parse(tableColumn.default.substring(1, tableColumn.default.length-1));
-        return OrmUtils.deepCompare(columnDefault, tableDefault);
-      }
-      else {
+
         const columnDefault = this.lowerDefaultValueIfNecessary(this.normalizeDefault(columnMetadata));
-        return (columnDefault === tableColumn.default);
-      }
+        return columnDefault === tableColumn.default;
     }
 
     /**
diff --git a/test/functional/json/entity/Record.ts b/test/functional/json/entity/Record.ts
index 72503bd35b..4247054063 100644
--- a/test/functional/json/entity/Record.ts
+++ b/test/functional/json/entity/Record.ts
@@ -17,4 +17,9 @@ export class Record {
     @Column({ type: "jsonb", nullable: true })
     data: any;
 
-}
\ No newline at end of file
+    @Column({ type: "jsonb", nullable: true, default: { hello: "world", foo: "bar" } })
+    dataWithDefaultObject: any;
+
+    @Column({ type: "jsonb", nullable: true, default: null })
+    dataWithDefaultNull: any;
+}
diff --git a/test/functional/json/jsonb.ts b/test/functional/json/jsonb.ts
index 052ba816fe..6379287873 100644
--- a/test/functional/json/jsonb.ts
+++ b/test/functional/json/jsonb.ts
@@ -22,6 +22,8 @@ describe("jsonb type", () => {
         expect(schema).not.to.be.undefined;
         expect(schema!.columns.find(tableColumn => tableColumn.name === "config" && tableColumn.type === "json")).to.be.not.empty;
         expect(schema!.columns.find(tableColumn => tableColumn.name === "data" && tableColumn.type === "jsonb")).to.be.not.empty;
+        expect(schema!.columns.find(tableColumn => tableColumn.name === "dataWithDefaultObject" && tableColumn.type === "jsonb")).to.be.not.empty;
+        expect(schema!.columns.find(tableColumn => tableColumn.name === "dataWithDefaultNull" && tableColumn.type === "jsonb")).to.be.not.empty;
     })));
 
     it("should persist jsonb correctly", () => Promise.all(connections.map(async connection => {
@@ -33,6 +35,8 @@ describe("jsonb type", () => {
         let foundRecord = await recordRepo.findOne(persistedRecord.id);
         expect(foundRecord).to.be.not.undefined;
         expect(foundRecord!.data.foo).to.eq("bar");
+        expect(foundRecord!.dataWithDefaultNull).to.be.null;
+        expect(foundRecord!.dataWithDefaultObject).to.eql({ hello: "world", foo: "bar" });
     })));
 
     it("should persist jsonb string correctly", () => Promise.all(connections.map(async connection => {
@@ -55,4 +59,29 @@ describe("jsonb type", () => {
         expect(foundRecord).to.be.not.undefined;
         expect(foundRecord!.data).to.deep.include.members([1, "2", { a: 3 }]);
     })));
+
+    it("should create updates when changing object", () => Promise.all(connections.map(async connection => {
+        await connection.query(`ALTER TABLE record ALTER COLUMN "dataWithDefaultObject" SET DEFAULT '{"foo":"baz","hello": "earth"}';`)
+
+        const sqlInMemory = await connection.driver.createSchemaBuilder().log();
+
+        expect(sqlInMemory.upQueries).not.to.eql([]);
+        expect(sqlInMemory.downQueries).not.to.eql([]);
+    })))
+
+    it("should not create updates when resorting object", () => Promise.all(connections.map(async connection => {
+        await connection.query(`ALTER TABLE record ALTER COLUMN "dataWithDefaultObject" SET DEFAULT '{"foo":"bar", "hello": "world"}';`)
+
+        const sqlInMemory = await connection.driver.createSchemaBuilder().log();
+
+        expect(sqlInMemory.upQueries).to.eql([]);
+        expect(sqlInMemory.downQueries).to.eql([]);
+    })));
+
+    it("should not create new migrations when everything is equivalent", () => Promise.all(connections.map(async connection => {
+        const sqlInMemory = await connection.driver.createSchemaBuilder().log();
+
+        expect(sqlInMemory.upQueries).to.eql([]);
+        expect(sqlInMemory.downQueries).to.eql([]);
+    })));
 });