From 7722ef8d52a2d6964cc6b93ec62993198cd5e51e Mon Sep 17 00:00:00 2001 From: Ryan Shea Date: Mon, 16 Mar 2020 23:06:05 -0500 Subject: [PATCH 1/5] fix: columns with transformer should be normalized for update Closes: #2703 --- .../SubjectChangedColumnsComputer.ts | 7 ++- test/github-issues/2703/entity/Dummy.ts | 13 ++++++ test/github-issues/2703/issue-2703.ts | 44 +++++++++++++++++++ test/github-issues/2703/memory-logger.ts | 32 ++++++++++++++ test/github-issues/2703/wrapped-number.ts | 14 ++++++ test/utils/test-utils.ts | 8 ++++ 6 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 test/github-issues/2703/entity/Dummy.ts create mode 100644 test/github-issues/2703/issue-2703.ts create mode 100644 test/github-issues/2703/memory-logger.ts create mode 100644 test/github-issues/2703/wrapped-number.ts diff --git a/src/persistence/SubjectChangedColumnsComputer.ts b/src/persistence/SubjectChangedColumnsComputer.ts index 9a55c0386d..970e0866c2 100644 --- a/src/persistence/SubjectChangedColumnsComputer.ts +++ b/src/persistence/SubjectChangedColumnsComputer.ts @@ -3,6 +3,7 @@ import {DateUtils} from "../util/DateUtils"; import {ObjectLiteral} from "../common/ObjectLiteral"; import {EntityMetadata} from "../metadata/EntityMetadata"; import {OrmUtils} from "../util/OrmUtils"; +import {ApplyValueTransformers} from "../util/ApplyValueTransformers"; /** * Finds what columns are changed in the subject entities. @@ -62,7 +63,7 @@ export class SubjectChangedColumnsComputer { if (subject.databaseEntity) { // get database value of the column - let databaseValue = column.getEntityValue(subject.databaseEntity); + let databaseValue = column.getEntityValue(subject.databaseEntity, true); // filter out "relational columns" only in the case if there is a relation object in entity if (column.relationMetadata) { @@ -118,6 +119,10 @@ export class SubjectChangedColumnsComputer { databaseValue = DateUtils.simpleJsonToString(databaseValue); break; } + + if (column.transformer) { + normalizedValue = ApplyValueTransformers.transformTo(column.transformer, entityValue); + } } // if value is not changed - then do nothing diff --git a/test/github-issues/2703/entity/Dummy.ts b/test/github-issues/2703/entity/Dummy.ts new file mode 100644 index 0000000000..f99159d3f8 --- /dev/null +++ b/test/github-issues/2703/entity/Dummy.ts @@ -0,0 +1,13 @@ +import {Entity} from "../../../../src/decorator/entity/Entity"; +import {Column} from "../../../../src/decorator/columns/Column"; +import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn"; +import {WrappedNumber, wrappedNumberTransformer} from "../wrapped-number"; + +@Entity() +export class Dummy { + @PrimaryGeneratedColumn() + id: number; + + @Column("int", {transformer: wrappedNumberTransformer}) + value: WrappedNumber; +} diff --git a/test/github-issues/2703/issue-2703.ts b/test/github-issues/2703/issue-2703.ts new file mode 100644 index 0000000000..3eeee0284d --- /dev/null +++ b/test/github-issues/2703/issue-2703.ts @@ -0,0 +1,44 @@ +import {expect} from "chai"; +import {Connection} from "../../../src/connection/Connection"; +import {createTestingConnections, reloadTestingDatabases, closeTestingConnections} from "../../utils/test-utils"; +import {Dummy} from "./entity/Dummy"; +import {WrappedNumber} from "./wrapped-number"; +import {MemoryLogger} from "./memory-logger"; + +describe("github issues > #2703 Column with transformer is not normalized for update", () => { + let connections: Connection[]; + let logger: MemoryLogger; + + before(async () => { + logger = new MemoryLogger(false); + connections = await createTestingConnections({ + entities: [`${__dirname}/entity/*{.js,.ts}`], + schemaCreate: true, + dropSchema: true, + logger, + }); + }); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + afterEach(() => { + logger.enabled = false; + logger.clear(); + }); + + it("should transform values when computing changed columns", () => Promise.all(connections.map(async connection => { + const repository = connection.getRepository(Dummy); + + const dummy = repository.create({ + value: new WrappedNumber(1), + }); + await repository.save(dummy); + + logger.enabled = true; + + await repository.save(dummy); + + const updateQueries = logger.queries.filter(q => q.startsWith("UPDATE")); + + expect(updateQueries).to.be.empty; + }))); +}); diff --git a/test/github-issues/2703/memory-logger.ts b/test/github-issues/2703/memory-logger.ts new file mode 100644 index 0000000000..3ec36711fc --- /dev/null +++ b/test/github-issues/2703/memory-logger.ts @@ -0,0 +1,32 @@ +import {Logger} from "../../../src/logger/Logger"; + +export class MemoryLogger implements Logger { + constructor(public enabled = true) {} + + private _queries: string[] = []; + get queries() { return this._queries; } + + logQuery(query: string) { + if (this.enabled) { + this._queries.push(query); + } + } + + logQueryError(error: string, query: string) {} + + logQuerySlow(time: number, query: string) {} + + logSchemaBuild(message: string) {} + + logMigration(message: string) {} + + log(level: "log" | "info" | "warn", message: any) {} + + writeToConsole() { + this.queries.forEach(q => console.log(`query: ${q}`)); + } + + clear() { + this._queries = []; + } +} diff --git a/test/github-issues/2703/wrapped-number.ts b/test/github-issues/2703/wrapped-number.ts new file mode 100644 index 0000000000..f222c296ff --- /dev/null +++ b/test/github-issues/2703/wrapped-number.ts @@ -0,0 +1,14 @@ +import {ValueTransformer} from "../../../src/decorator/options/ValueTransformer"; + +export class WrappedNumber { + constructor(readonly value: number) {} +} + +export const wrappedNumberTransformer: ValueTransformer = { + from(value: number): WrappedNumber { + return new WrappedNumber(value); + }, + to(value: WrappedNumber): number { + return value.value; + } +}; diff --git a/test/utils/test-utils.ts b/test/utils/test-utils.ts index 9d7298a6b1..9d0235bb34 100644 --- a/test/utils/test-utils.ts +++ b/test/utils/test-utils.ts @@ -8,6 +8,7 @@ import {createConnections} from "../../src/index"; import {NamingStrategyInterface} from "../../src/naming-strategy/NamingStrategyInterface"; import {PromiseUtils} from "../../src/util/PromiseUtils"; import {QueryResultCache} from "../../src/cache/QueryResultCache"; +import {Logger} from "../../src/logger/Logger"; /** * Interface in which data is stored in ormconfig.json of the project. @@ -133,6 +134,11 @@ export interface TestingOptions { * They are passed down to the enabled drivers. */ driverSpecific?: Object; + + /** + * Logger instance used to log queries and events in the ORM. + */ + logger?: "advanced-console"|"simple-console"|"file"|"debug"|Logger; } /** @@ -219,6 +225,8 @@ export function setupTestingConnections(options?: TestingOptions): ConnectionOpt newOptions.schema = options.schema; if (options && options.logging !== undefined) newOptions.logging = options.logging; + if (options && options.logger !== undefined) + newOptions.logger = options.logger; if (options && options.__dirname) newOptions.entities = [options.__dirname + "/entity/*{.js,.ts}"]; if (options && options.__dirname) From 8fdadcf3b503f1405f960f2f1087e553c7b55544 Mon Sep 17 00:00:00 2001 From: Ryan Shea Date: Tue, 17 Mar 2020 09:43:14 -0500 Subject: [PATCH 2/5] fix: test case to use separate logger per connection --- test/github-issues/2703/issue-2703.ts | 26 ++++++++++++-------------- test/utils/test-utils.ts | 8 ++++---- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/test/github-issues/2703/issue-2703.ts b/test/github-issues/2703/issue-2703.ts index 3eeee0284d..73498dc7a1 100644 --- a/test/github-issues/2703/issue-2703.ts +++ b/test/github-issues/2703/issue-2703.ts @@ -5,25 +5,22 @@ import {Dummy} from "./entity/Dummy"; import {WrappedNumber} from "./wrapped-number"; import {MemoryLogger} from "./memory-logger"; -describe("github issues > #2703 Column with transformer is not normalized for update", () => { +describe.only("github issues > #2703 Column with transformer is not normalized for update", () => { let connections: Connection[]; - let logger: MemoryLogger; - - before(async () => { - logger = new MemoryLogger(false); - connections = await createTestingConnections({ - entities: [`${__dirname}/entity/*{.js,.ts}`], - schemaCreate: true, - dropSchema: true, - logger, - }); - }); + + before(async () => connections = await createTestingConnections({ + entities: [`${__dirname}/entity/*{.js,.ts}`], + schemaCreate: true, + dropSchema: true, + createLogger: () => new MemoryLogger(false), + })); beforeEach(() => reloadTestingDatabases(connections)); after(() => closeTestingConnections(connections)); - afterEach(() => { + afterEach(() => connections.forEach(connection => { + const logger = connection.logger as MemoryLogger; logger.enabled = false; logger.clear(); - }); + })); it("should transform values when computing changed columns", () => Promise.all(connections.map(async connection => { const repository = connection.getRepository(Dummy); @@ -33,6 +30,7 @@ describe("github issues > #2703 Column with transformer is not normalized for up }); await repository.save(dummy); + const logger = connection.logger as MemoryLogger; logger.enabled = true; await repository.save(dummy); diff --git a/test/utils/test-utils.ts b/test/utils/test-utils.ts index 9d0235bb34..093df0c691 100644 --- a/test/utils/test-utils.ts +++ b/test/utils/test-utils.ts @@ -136,9 +136,9 @@ export interface TestingOptions { driverSpecific?: Object; /** - * Logger instance used to log queries and events in the ORM. + * Factory to create a logger for each test connection. */ - logger?: "advanced-console"|"simple-console"|"file"|"debug"|Logger; + createLogger?: () => "advanced-console"|"simple-console"|"file"|"debug"|Logger; } /** @@ -225,8 +225,8 @@ export function setupTestingConnections(options?: TestingOptions): ConnectionOpt newOptions.schema = options.schema; if (options && options.logging !== undefined) newOptions.logging = options.logging; - if (options && options.logger !== undefined) - newOptions.logger = options.logger; + if (options && options.createLogger !== undefined) + newOptions.logger = options.createLogger(); if (options && options.__dirname) newOptions.entities = [options.__dirname + "/entity/*{.js,.ts}"]; if (options && options.__dirname) From 3d2e1ed8ebe899e70eef6cbd554cc246dbb10c61 Mon Sep 17 00:00:00 2001 From: Ryan Shea Date: Tue, 17 Mar 2020 13:50:12 -0500 Subject: [PATCH 3/5] fix: test dummy column type int means int8 on cockroachdb. Explicitly specify int4. --- test/github-issues/2703/entity/Dummy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/github-issues/2703/entity/Dummy.ts b/test/github-issues/2703/entity/Dummy.ts index f99159d3f8..5e83ef6128 100644 --- a/test/github-issues/2703/entity/Dummy.ts +++ b/test/github-issues/2703/entity/Dummy.ts @@ -8,6 +8,6 @@ export class Dummy { @PrimaryGeneratedColumn() id: number; - @Column("int", {transformer: wrappedNumberTransformer}) + @Column("int4", {transformer: wrappedNumberTransformer}) value: WrappedNumber; } From f00f7f4583409f44fbb2deb01ff089e26e56c3a1 Mon Sep 17 00:00:00 2001 From: Ryan Shea Date: Tue, 17 Mar 2020 14:07:33 -0500 Subject: [PATCH 4/5] fix: use string instead of number for test int4 doesn't work for all dbs. Use string because it's universal. --- test/github-issues/2703/entity/Dummy.ts | 6 +++--- test/github-issues/2703/issue-2703.ts | 4 ++-- test/github-issues/2703/wrapped-number.ts | 14 -------------- test/github-issues/2703/wrapped-string.ts | 14 ++++++++++++++ 4 files changed, 19 insertions(+), 19 deletions(-) delete mode 100644 test/github-issues/2703/wrapped-number.ts create mode 100644 test/github-issues/2703/wrapped-string.ts diff --git a/test/github-issues/2703/entity/Dummy.ts b/test/github-issues/2703/entity/Dummy.ts index 5e83ef6128..d6bae2e0f7 100644 --- a/test/github-issues/2703/entity/Dummy.ts +++ b/test/github-issues/2703/entity/Dummy.ts @@ -1,13 +1,13 @@ import {Entity} from "../../../../src/decorator/entity/Entity"; import {Column} from "../../../../src/decorator/columns/Column"; import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn"; -import {WrappedNumber, wrappedNumberTransformer} from "../wrapped-number"; +import {WrappedString, wrappedStringTransformer} from "../wrapped-string"; @Entity() export class Dummy { @PrimaryGeneratedColumn() id: number; - @Column("int4", {transformer: wrappedNumberTransformer}) - value: WrappedNumber; + @Column("string", {transformer: wrappedStringTransformer}) + value: WrappedString; } diff --git a/test/github-issues/2703/issue-2703.ts b/test/github-issues/2703/issue-2703.ts index 73498dc7a1..4316807143 100644 --- a/test/github-issues/2703/issue-2703.ts +++ b/test/github-issues/2703/issue-2703.ts @@ -2,7 +2,7 @@ import {expect} from "chai"; import {Connection} from "../../../src/connection/Connection"; import {createTestingConnections, reloadTestingDatabases, closeTestingConnections} from "../../utils/test-utils"; import {Dummy} from "./entity/Dummy"; -import {WrappedNumber} from "./wrapped-number"; +import {WrappedString} from "./wrapped-string"; import {MemoryLogger} from "./memory-logger"; describe.only("github issues > #2703 Column with transformer is not normalized for update", () => { @@ -26,7 +26,7 @@ describe.only("github issues > #2703 Column with transformer is not normalized f const repository = connection.getRepository(Dummy); const dummy = repository.create({ - value: new WrappedNumber(1), + value: new WrappedString("test"), }); await repository.save(dummy); diff --git a/test/github-issues/2703/wrapped-number.ts b/test/github-issues/2703/wrapped-number.ts deleted file mode 100644 index f222c296ff..0000000000 --- a/test/github-issues/2703/wrapped-number.ts +++ /dev/null @@ -1,14 +0,0 @@ -import {ValueTransformer} from "../../../src/decorator/options/ValueTransformer"; - -export class WrappedNumber { - constructor(readonly value: number) {} -} - -export const wrappedNumberTransformer: ValueTransformer = { - from(value: number): WrappedNumber { - return new WrappedNumber(value); - }, - to(value: WrappedNumber): number { - return value.value; - } -}; diff --git a/test/github-issues/2703/wrapped-string.ts b/test/github-issues/2703/wrapped-string.ts new file mode 100644 index 0000000000..96b91d90cd --- /dev/null +++ b/test/github-issues/2703/wrapped-string.ts @@ -0,0 +1,14 @@ +import {ValueTransformer} from "../../../src/decorator/options/ValueTransformer"; + +export class WrappedString { + constructor(readonly value: string) {} +} + +export const wrappedStringTransformer: ValueTransformer = { + from(value: string): WrappedString { + return new WrappedString(value); + }, + to(value: WrappedString): string { + return value.value; + } +}; From d0f4b94a404e0a2e5e9306628698ad703888b26d Mon Sep 17 00:00:00 2001 From: Ryan Shea Date: Tue, 17 Mar 2020 14:21:18 -0500 Subject: [PATCH 5/5] fix: let typeorm infer proper test column type --- test/github-issues/2703/entity/Dummy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/github-issues/2703/entity/Dummy.ts b/test/github-issues/2703/entity/Dummy.ts index d6bae2e0f7..a2354cd51c 100644 --- a/test/github-issues/2703/entity/Dummy.ts +++ b/test/github-issues/2703/entity/Dummy.ts @@ -8,6 +8,6 @@ export class Dummy { @PrimaryGeneratedColumn() id: number; - @Column("string", {transformer: wrappedStringTransformer}) + @Column({type: String, transformer: wrappedStringTransformer}) value: WrappedString; }