diff --git a/src/persistence/SubjectExecutor.ts b/src/persistence/SubjectExecutor.ts index 2345b7bfdef..c2c6db2f67b 100644 --- a/src/persistence/SubjectExecutor.ts +++ b/src/persistence/SubjectExecutor.ts @@ -387,7 +387,7 @@ export class SubjectExecutor { // for mongodb we have a bit different updation logic if (this.queryRunner instanceof MongoQueryRunner) { - const partialEntity = OrmUtils.mergeDeep({}, subject.entity!); + const partialEntity = this.cloneSubjectEntity(subject); if (subject.metadata.objectIdColumn && subject.metadata.objectIdColumn.propertyName) { delete partialEntity[subject.metadata.objectIdColumn.propertyName]; } @@ -540,6 +540,18 @@ export class SubjectExecutor { } } + private cloneSubjectEntity(subject: Subject): ObjectLiteral { + const target: ObjectLiteral = {}; + + if (subject.entity) { + for (const column of subject.metadata.columns) { + OrmUtils.mergeDeep(target, column.getEntityValueMap(subject.entity)); + } + } + + return target; + } + /** * Soft-removes all given subjects in the database. */ @@ -551,7 +563,7 @@ export class SubjectExecutor { // for mongodb we have a bit different updation logic if (this.queryRunner instanceof MongoQueryRunner) { - const partialEntity = OrmUtils.mergeDeep({}, subject.entity!); + const partialEntity = this.cloneSubjectEntity(subject); if (subject.metadata.objectIdColumn && subject.metadata.objectIdColumn.propertyName) { delete partialEntity[subject.metadata.objectIdColumn.propertyName]; } @@ -631,7 +643,7 @@ export class SubjectExecutor { // for mongodb we have a bit different updation logic if (this.queryRunner instanceof MongoQueryRunner) { - const partialEntity = OrmUtils.mergeDeep({}, subject.entity!); + const partialEntity = this.cloneSubjectEntity(subject); if (subject.metadata.objectIdColumn && subject.metadata.objectIdColumn.propertyName) { delete partialEntity[subject.metadata.objectIdColumn.propertyName]; } diff --git a/src/util/OrmUtils.ts b/src/util/OrmUtils.ts index 4687b2e963f..32cd470f626 100644 --- a/src/util/OrmUtils.ts +++ b/src/util/OrmUtils.ts @@ -58,41 +58,64 @@ export class OrmUtils { }, [] as T[]); } - static isObject(item: any) { - return (item && typeof item === "object" && !Array.isArray(item)); + // Checks if it's an object made by Object.create(null), {} or new Object() + private static isPlainObject(item: any) { + if (item === null || item === undefined) { + return false; + } + + return !item.constructor || item.constructor === Object; + } + + private static mergeKey(target: any, key: string, value: any, memo: Map) { + // Have we seen this before? Prevent infinite recursion. + if (memo.has(value)) { + Object.assign(target, { [key]: memo.get(value) }); + return; + } + + let newValue = value; + + if (this.isPlainObject(value)) { + const newValue = Object.create(Object.getPrototypeOf(value)); + + if (!target[key]) { + Object.assign(target, { [key]: newValue }); + } + + memo.set(value, newValue); + this.merge(target[key], value); + memo.delete(value); + } + + Object.assign(target, { [key]: newValue }); + } + + private static merge(target: any, source: any, memo: Map = new Map()): any { + let keys = []; + + if (this.isPlainObject(target) && this.isPlainObject(source)) { + keys.push(...Object.keys(source)); + } + + for (const key of keys) { + this.mergeKey(target, key, source[key], memo); + } } /** * Deep Object.assign. - * - * @see http://stackoverflow.com/a/34749873 */ static mergeDeep(target: any, ...sources: any[]): any { - if (!sources.length) return target; - const source = sources.shift(); - - if (this.isObject(target) && this.isObject(source)) { - for (const key in source) { - const value = source[key]; - if (key === "__proto__" || value instanceof Promise) - continue; - - if (this.isObject(value) - && !(value instanceof Map) - && !(value instanceof Set) - && !(value instanceof Date) - && !(value instanceof Buffer) - && !(value instanceof RegExp)) { - if (!target[key]) - Object.assign(target, { [key]: Object.create(Object.getPrototypeOf(value)) }); - this.mergeDeep(target[key], value); - } else { - Object.assign(target, { [key]: value }); - } - } + if (!sources.length) { + return target; + } + + for (const source of sources) { + OrmUtils.merge(target, source); } - return this.mergeDeep(target, ...sources); + return target; } /** diff --git a/test/functional/columns/value-transformer/entity/Post.ts b/test/functional/columns/value-transformer/entity/Post.ts index 5077452d66f..3daca45fca4 100644 --- a/test/functional/columns/value-transformer/entity/Post.ts +++ b/test/functional/columns/value-transformer/entity/Post.ts @@ -15,6 +15,38 @@ class TagTransformer implements ValueTransformer { } +export class Complex { + x: number; + y: number; + circularReferenceToMySelf: { + complex: Complex + }; + + constructor(from: String) { + this.circularReferenceToMySelf = { complex: this }; + const [x, y] = from.split(" "); + this.x = +x; + this.y = +y; + } + + toString() { + return `${this.x} ${this.y}`; + } +} + +class ComplexTransformer implements ValueTransformer { + + to (value: Complex | null): string | null { + if (value == null) { return value; } + return value.toString(); + } + + from (value: string | null): Complex | null { + if (value == null) { return value; } + return new Complex(value); + } +} + @Entity() export class Post { @@ -27,4 +59,6 @@ export class Post { @Column({ type: String, transformer: new TagTransformer() }) tags: string[]; + @Column({ type: String, transformer: new ComplexTransformer(), nullable: true }) + complex: Complex | null; } diff --git a/test/functional/columns/value-transformer/value-transformer.ts b/test/functional/columns/value-transformer/value-transformer.ts index 02f5d49bc90..9e49b166aab 100644 --- a/test/functional/columns/value-transformer/value-transformer.ts +++ b/test/functional/columns/value-transformer/value-transformer.ts @@ -4,7 +4,7 @@ import {closeTestingConnections, createTestingConnections, reloadTestingDatabase import {Connection} from "../../../../src/connection/Connection"; import {PhoneBook} from "./entity/PhoneBook"; -import {Post} from "./entity/Post"; +import {Complex, Post} from "./entity/Post"; import {User} from "./entity/User"; importĀ {Category} from "./entity/Category"; import {View} from "./entity/View"; @@ -95,4 +95,60 @@ describe("columns > value-transformer functionality", () => { dbView && dbView.title.should.be.eql(title); }))); + + it("should marshal data using a complex value-transformer", () => Promise.all(connections.map(async connection => { + + const postRepository = connection.getRepository(Post); + + // create and save a post first + const post = new Post(); + post.title = "Complex transformers!"; + post.tags = ["complex", "transformer"]; + await postRepository.save(post); + + let loadedPost = await postRepository.findOne(post.id); + expect(loadedPost!.complex).to.eq(null); + + // then update all its properties and save again + post.title = "Complex transformers2!"; + post.tags = ["very", "complex", "actually"]; + post.complex = new Complex("3 2.5"); + await postRepository.save(post); + + // check if all columns are updated except for readonly columns + loadedPost = await postRepository.findOne(post.id); + expect(loadedPost!.title).to.be.equal("Complex transformers2!"); + expect(loadedPost!.tags).to.deep.eq(["very", "complex", "actually"]); + expect(loadedPost!.complex!.x).to.eq(3); + expect(loadedPost!.complex!.y).to.eq(2.5); + + // then update all its properties and save again + post.title = "Complex transformers3!"; + post.tags = ["very", "lacking", "actually"]; + post.complex = null; + await postRepository.save(post); + + loadedPost = await postRepository.findOne(post.id); + expect(loadedPost!.complex).to.eq(null); + + // then update all its properties and save again + post.title = "Complex transformers4!"; + post.tags = ["very", "here", "again!"]; + post.complex = new Complex("0.5 0.5"); + await postRepository.save(post); + + loadedPost = await postRepository.findOne(post.id); + expect(loadedPost!.complex!.x).to.eq(0.5); + expect(loadedPost!.complex!.y).to.eq(0.5); + + // then update all its properties and save again + post.title = "Complex transformers5!"; + post.tags = ["now", "really", "lacking!"]; + post.complex = new Complex("1.05 2.3"); + await postRepository.save(post); + + loadedPost = await postRepository.findOne(post.id); + expect(loadedPost!.complex!.x).to.eq(1.05); + expect(loadedPost!.complex!.y).to.eq(2.3); + }))); }); diff --git a/test/functional/mongodb/basic/mongo-repository/mongo-repository.ts b/test/functional/mongodb/basic/mongo-repository/mongo-repository.ts index 3d229cd2368..e030381147f 100644 --- a/test/functional/mongodb/basic/mongo-repository/mongo-repository.ts +++ b/test/functional/mongodb/basic/mongo-repository/mongo-repository.ts @@ -87,5 +87,29 @@ describe("mongodb > MongoRepository", () => { }))); // todo: cover other methods as well + it("should be able to save and update mongo entities", () => Promise.all(connections.map(async connection => { + const postRepository = connection.getMongoRepository(Post); + + // save few posts + const firstPost = new Post(); + firstPost.title = "Post #1"; + firstPost.text = "Everything about post #1"; + await postRepository.save(firstPost); + + const secondPost = new Post(); + secondPost.title = "Post #2"; + secondPost.text = "Everything about post #2"; + await postRepository.save(secondPost); + // save few posts + firstPost.text = "Everything and more about post #1"; + await postRepository.save(firstPost); + + const loadedPosts = await postRepository.find(); + + loadedPosts.length.should.be.equal(2); + loadedPosts[0].text.should.be.equal("Everything and more about post #1"); + loadedPosts[0].text.should.be.equal("Everything about post #2"); + + }))); }); diff --git a/test/functional/mongodb/basic/repository-actions/entity/Counters.ts b/test/functional/mongodb/basic/repository-actions/entity/Counters.ts new file mode 100644 index 00000000000..90572b94871 --- /dev/null +++ b/test/functional/mongodb/basic/repository-actions/entity/Counters.ts @@ -0,0 +1,19 @@ +import {Column} from "../../../../../../src/decorator/columns/Column"; +import {Entity} from "../../../../../../src/decorator/entity/Entity"; +import {User} from "./User"; + +@Entity() +export class Counters { + + @Column({ name: "_likes" }) + likes: number; + + @Column({ name: "_comments" }) + comments: number; + + @Column({ name: "_favorites" }) + favorites: number; + + @Column(() => User) + viewer: User; +} diff --git a/test/functional/mongodb/basic/repository-actions/entity/Post.ts b/test/functional/mongodb/basic/repository-actions/entity/Post.ts index 281d15c25fe..5475cfa5d38 100644 --- a/test/functional/mongodb/basic/repository-actions/entity/Post.ts +++ b/test/functional/mongodb/basic/repository-actions/entity/Post.ts @@ -1,4 +1,5 @@ import {Entity} from "../../../../../../src/decorator/entity/Entity"; +import {Counters} from "./Counters"; import {Column} from "../../../../../../src/decorator/columns/Column"; import {ObjectIdColumn} from "../../../../../../src/decorator/columns/ObjectIdColumn"; import {ObjectID} from "../../../../../../src/driver/mongodb/typings"; @@ -18,7 +19,6 @@ export class Post { @Column() index: number; - // @Column(() => Counters) - // counters: Counters; - + @Column(() => Counters) + counters: Counters; } diff --git a/test/functional/mongodb/basic/repository-actions/entity/User.ts b/test/functional/mongodb/basic/repository-actions/entity/User.ts new file mode 100644 index 00000000000..79290ca9775 --- /dev/null +++ b/test/functional/mongodb/basic/repository-actions/entity/User.ts @@ -0,0 +1,9 @@ +import {Column} from "../../../../../../src/decorator/columns/Column"; +import {Entity} from "../../../../../../src/decorator/entity/Entity"; + +@Entity() +export class User { + + @Column() + name: string; +} diff --git a/test/functional/mongodb/basic/repository-actions/mongodb-repository-actions.ts b/test/functional/mongodb/basic/repository-actions/mongodb-repository-actions.ts index 7385ceeaa9a..7d42fa036f5 100644 --- a/test/functional/mongodb/basic/repository-actions/mongodb-repository-actions.ts +++ b/test/functional/mongodb/basic/repository-actions/mongodb-repository-actions.ts @@ -3,6 +3,8 @@ import {expect} from "chai"; import {Connection} from "../../../../../src/connection/Connection"; import {closeTestingConnections, createTestingConnections, reloadTestingDatabases} from "../../../../utils/test-utils"; import {Post} from "./entity/Post"; +import {Counters} from "./entity/Counters"; +import {User} from "./entity/User"; describe("mongodb > basic repository actions", () => { @@ -46,6 +48,32 @@ describe("mongodb > basic repository actions", () => { mergedPost.text.should.be.equal("And its text is updated as well"); }))); + it("merge should merge all given recursive partial objects into given source entity", () => Promise.all(connections.map(async connection => { + const postRepository = connection.getRepository(Post); + const counter1 = new Counters(); + counter1.likes = 5; + const counter2 = new Counters(); + counter2.likes = 2; + counter2.viewer = new User(); + counter2.viewer.name = "Hello World"; + const post = postRepository.create({ + title: "This is created post", + text: "All about this post", + counters: counter1 + }); + const mergedPost = postRepository.merge(post, + { title: "This is updated post" }, + { text: "And its text is updated as well" }, + { counters: counter2 } + ); + mergedPost.should.be.instanceOf(Post); + mergedPost.should.be.equal(post); + mergedPost.title.should.be.equal("This is updated post"); + mergedPost.text.should.be.equal("And its text is updated as well"); + mergedPost.counters.likes.should.be.equal(2); + mergedPost.counters.viewer.name.should.be.equal("Hello World"); + }))); + it("target should be valid", () => Promise.all(connections.map(async connection => { const postRepository = connection.getRepository(Post); expect(postRepository.target).not.to.be.undefined; diff --git a/test/functional/util/OrmUtils.ts b/test/functional/util/OrmUtils.ts new file mode 100644 index 00000000000..9c1ade28636 --- /dev/null +++ b/test/functional/util/OrmUtils.ts @@ -0,0 +1,82 @@ +import { expect } from "chai"; +import { OrmUtils } from "../../../src/util/OrmUtils"; + +describe("OrmUtils.mergeDeep", () => { + const mergeDeep = OrmUtils.mergeDeep.bind(OrmUtils); + + it("should handle simple values.", () => { + expect(mergeDeep(1, 2)).to.equal(1); + expect(mergeDeep(2, 1)).to.equal(2); + expect(mergeDeep(2, 1, 1)).to.equal(2); + expect(mergeDeep(1, 2, 1)).to.equal(1); + expect(mergeDeep(1, 1, 2)).to.equal(1); + expect(mergeDeep(2, 1, 2)).to.equal(2); + }); + + it("should handle ordering and indempotence.", () => { + const a = { a: 1 }; + const b = { a: 2 }; + expect(mergeDeep(a, b)).to.deep.equal(b); + expect(mergeDeep(b, a)).to.deep.equal(a); + expect(mergeDeep(b, a, a)).to.deep.equal(a); + expect(mergeDeep(a, b, a)).to.deep.equal(a); + expect(mergeDeep(a, a, b)).to.deep.equal(b); + expect(mergeDeep(b, a, b)).to.deep.equal(b); + const c = { a: 3 }; + expect(mergeDeep(a, b, c)).to.deep.equal(c); + expect(mergeDeep(b, c, b)).to.deep.equal(b); + expect(mergeDeep(c, a, a)).to.deep.equal(a); + expect(mergeDeep(c, b, a)).to.deep.equal(a); + expect(mergeDeep(a, c, b)).to.deep.equal(b); + expect(mergeDeep(b, a, c)).to.deep.equal(c); + }); + + it("should skip nested promises in sources.", () => { + expect(mergeDeep({}, { p: Promise.resolve() })).to.deep.equal({}); + expect(mergeDeep({}, { p: { p: Promise.resolve() }})).to.deep.equal({ p: {} }); + const a = { p: Promise.resolve(0) }; + const b = { p: Promise.resolve(1) }; + expect(mergeDeep(a, {})).to.deep.equal(a); + expect(mergeDeep(a, b)).to.deep.equal(a); + expect(mergeDeep(b, a)).to.deep.equal(b); + expect(mergeDeep(b, {})).to.deep.equal(b); + }); + + it("should merge moderately deep objects correctly.", () => { + const a = { a: { b: { c: { d: { e: 123, h: { i: 23 } } } } }, g: 19 }; + const b = { a: { b: { c: { d: { f: 99 } }, f: 31 } } }; + const c = { a: { b: { c: { d: { e: 123, f: 99, h: { i: 23 } } }, f: 31 } }, g: 19 }; + expect(mergeDeep(a, b)).to.deep.equal(c); + expect(mergeDeep(b, a)).to.deep.equal(c); + expect(mergeDeep(b, a, a)).to.deep.equal(c); + expect(mergeDeep(a, b, a)).to.deep.equal(c); + expect(mergeDeep(a, a, b)).to.deep.equal(c); + expect(mergeDeep(b, a, b)).to.deep.equal(c); + }); + + it("should merge recursively deep objects correctly", () => { + let a: Record = {}; + let b: Record = {}; + + a['b'] = b; + a['a'] = a; + b['a'] = a; + + expect(mergeDeep({}, a)); + }); + + it("should reference copy complex instances of classes.", () => { + class Foo { + recursive: Foo; + + constructor() { + this.recursive = this; + } + } + + const foo = new Foo(); + const result = mergeDeep({}, { foo }); + expect(result).to.have.property("foo"); + expect(result.foo).to.equal(foo); + }); + }); diff --git a/test/github-issues/5762/entity/User.ts b/test/github-issues/5762/entity/User.ts new file mode 100644 index 00000000000..a4dc4b0bddf --- /dev/null +++ b/test/github-issues/5762/entity/User.ts @@ -0,0 +1,24 @@ +import {Entity} from "../../../../src/decorator/entity/Entity"; +import {PrimaryColumn, Column} from "../../../../src"; +import { URL } from "url"; + +@Entity() +export class User { + + @PrimaryColumn() + id: number; + + @Column("varchar", { + // marshall + transformer: { + from(value: string): URL { + return new URL(value); + }, + to(value: URL): string { + return value.toString(); + }, + }, + }) + url: URL; + +} diff --git a/test/github-issues/5762/issue-5762.ts b/test/github-issues/5762/issue-5762.ts new file mode 100644 index 00000000000..d3ca010f712 --- /dev/null +++ b/test/github-issues/5762/issue-5762.ts @@ -0,0 +1,39 @@ +import "reflect-metadata"; +import {expect} from "chai"; +import {Connection} from "../../../src"; +import {User} from "./entity/User"; +import {createTestingConnections, reloadTestingDatabases, closeTestingConnections} from "../../utils/test-utils"; +import { URL } from "url"; + +describe("github issues > #5762 `Using URL as a rich column type breaks", () => { + + let connections: Connection[]; + + before(async () => { + connections = await createTestingConnections({ + entities: [User], + schemaCreate: true, + dropSchema: true + }); + }); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should allow assigning URL as a field value", () => + Promise.all(connections.map(async (connection) => { + const userRepository = connection.getRepository(User); + + const url = new URL("https://typeorm.io"); + + const user = new User(); + user.id = 1; + user.url = url; + + const promise = userRepository.save(user); + + return expect(promise).to.eventually.be.deep.equal(user) + .and.to.have.property("url").be.instanceOf(URL) + .and.to.have.nested.property("href").equal(url.href); + }))); + +});