Skip to content

Commit

Permalink
fix: handle postgres default when tableColumn.default is not string (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
imnotjames committed Jun 30, 2021
1 parent 87788bb commit 0463855
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 14 deletions.
25 changes: 12 additions & 13 deletions src/driver/postgres/PostgresDriver.ts
Expand Up @@ -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;
}

/**
Expand Down
7 changes: 6 additions & 1 deletion test/functional/json/entity/Record.ts
Expand Up @@ -17,4 +17,9 @@ export class Record {
@Column({ type: "jsonb", nullable: true })
data: any;

}
@Column({ type: "jsonb", nullable: true, default: { hello: "world", foo: "bar" } })
dataWithDefaultObject: any;

@Column({ type: "jsonb", nullable: true, default: null })
dataWithDefaultNull: any;
}
29 changes: 29 additions & 0 deletions test/functional/json/jsonb.ts
Expand Up @@ -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 => {
Expand All @@ -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 => {
Expand All @@ -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([]);
})));
});

0 comments on commit 0463855

Please sign in to comment.