Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from May 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/persistence/SubjectChangedColumnsComputer.ts
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions 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;
}
42 changes: 42 additions & 0 deletions 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;
})));
});
32 changes: 32 additions & 0 deletions 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 = [];
}
}
14 changes: 14 additions & 0 deletions 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;
}
};
8 changes: 8 additions & 0 deletions test/utils/test-utils.ts
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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)
Expand Down