fix: columns with transformer should be normalized for update (#5700)

* fix: columns with transformer should be normalized for update

Closes: #2703

* fix: test case to use separate logger per connection

* fix: test dummy column type

int means int8 on cockroachdb. Explicitly specify int4.

* fix: use string instead of number for test

int4 doesn't work for all dbs. Use string because it's universal.

* fix: let typeorm infer proper test column type

Co-authored-by: Ryan Shea <ryan.shea@alphaledger.com>

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.
@@ -63,7 +64,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) {
@@ -119,6 +120,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

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 {WrappedString, wrappedStringTransformer} from "../wrapped-string";
+
+@Entity()
+export class Dummy {
+    @PrimaryGeneratedColumn()
+    id: number;
+
+    @Column({type: String, transformer: wrappedStringTransformer})
+    value: WrappedString;
+}

test/github-issues/2703/issue-2703.ts

@@ -0,0 +1,42 @@
+import {expect} from "chai";
+import {Connection} from "../../../src/connection/Connection";
+import {createTestingConnections, reloadTestingDatabases, closeTestingConnections} from "../../utils/test-utils";
+import {Dummy} from "./entity/Dummy";
+import {WrappedString} from "./wrapped-string";
+import {MemoryLogger} from "./memory-logger";
+
+describe.only("github issues > #2703 Column with transformer is not normalized for update", () => {
+    let connections: Connection[];
+
+    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(() => 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);
+
+        const dummy = repository.create({
+            value: new WrappedString("test"),
+        });
+        await repository.save(dummy);
+
+        const logger = connection.logger as MemoryLogger;
+        logger.enabled = true;
+
+        await repository.save(dummy);
+
+        const updateQueries = logger.queries.filter(q => q.startsWith("UPDATE"));
+
+        expect(updateQueries).to.be.empty;
+    })));
+});

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 = [];
+    }
+}

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;
+    }
+};

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;
+
+    /**
+     * Factory to create a logger for each test connection.
+     */
+    createLogger?: () => "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.createLogger !== undefined)
+                newOptions.logger = options.createLogger();
             if (options && options.__dirname)
                 newOptions.entities = [options.__dirname + "/entity/*{.js,.ts}"];
             if (options && options.__dirname)