diff --git a/src/metadata/EntityMetadata.ts b/src/metadata/EntityMetadata.ts index 257dde2712..bc31f090f0 100644 --- a/src/metadata/EntityMetadata.ts +++ b/src/metadata/EntityMetadata.ts @@ -664,16 +664,20 @@ export class EntityMetadata { * Iterates through entity and finds and extracts all values from relations in the entity. * If relation value is an array its being flattened. */ - extractRelationValuesFromEntity(entity: ObjectLiteral, relations: RelationMetadata[]): [RelationMetadata, any, EntityMetadata][] { + async extractRelationValuesFromEntity(entity: ObjectLiteral, relations: RelationMetadata[]): Promise<[RelationMetadata, any, EntityMetadata][]> { const relationsAndValues: [RelationMetadata, any, EntityMetadata][] = []; - relations.forEach(relation => { - const value = relation.getEntityValue(entity); + await Promise.all(relations.map(async relation => { + let value = relation.getEntityValue(entity); + if (value instanceof Promise) { + // Promise will only be returned if it is user-supplied (ie. value is dirty) + value = await value; + } if (value instanceof Array) { value.forEach(subValue => relationsAndValues.push([relation, subValue, relation.inverseEntityMetadata])); } else if (value) { relationsAndValues.push([relation, value, relation.inverseEntityMetadata]); } - }); + })); return relationsAndValues; } @@ -830,4 +834,4 @@ export class EntityMetadata { return this.database && !(this.connection.driver instanceof PostgresDriver) ? this.database + "." + this.schema : this.schema; } -} \ No newline at end of file +} diff --git a/src/metadata/RelationMetadata.ts b/src/metadata/RelationMetadata.ts index 7b82664b90..d9b21ff044 100644 --- a/src/metadata/RelationMetadata.ts +++ b/src/metadata/RelationMetadata.ts @@ -348,7 +348,7 @@ export class RelationMetadata { if (embeddedObject["__" + this.propertyName + "__"] !== undefined) return embeddedObject["__" + this.propertyName + "__"]; - if (getLazyRelationsPromiseValue === true) + if (entity["__dirty_" + this.propertyName + "__"] || getLazyRelationsPromiseValue === true) return embeddedObject[this.propertyName]; return undefined; @@ -360,7 +360,7 @@ export class RelationMetadata { if (entity["__" + this.propertyName + "__"] !== undefined) return entity["__" + this.propertyName + "__"]; - if (getLazyRelationsPromiseValue === true) + if (entity["__dirty_" + this.propertyName + "__"] || getLazyRelationsPromiseValue === true) return entity[this.propertyName]; return undefined; @@ -376,7 +376,7 @@ export class RelationMetadata { * If merge is set to true, it merges given value into currently */ setEntityValue(entity: ObjectLiteral, value: any): void { - const propertyName = this.isLazy ? "__" + this.propertyName + "__" : this.propertyName; + const propertyName = (this.isLazy && !(value instanceof Promise)) ? "__" + this.propertyName + "__" : this.propertyName; if (this.embeddedMetadata) { @@ -515,4 +515,4 @@ export class RelationMetadata { return this.embeddedMetadata.parentPropertyNames.join(".") + "." + this.propertyName; } -} \ No newline at end of file +} diff --git a/src/persistence/EntityPersistExecutor.ts b/src/persistence/EntityPersistExecutor.ts index 7f295c3cb3..bb94e76759 100644 --- a/src/persistence/EntityPersistExecutor.ts +++ b/src/persistence/EntityPersistExecutor.ts @@ -85,11 +85,11 @@ export class EntityPersistExecutor { // console.time("building cascades..."); // go through each entity with metadata and create subjects and subjects by cascades for them const cascadesSubjectBuilder = new CascadesSubjectBuilder(subjects); - subjects.forEach(subject => { + await Promise.all(subjects.map(async subject => { // next step we build list of subjects we will operate with // these subjects are subjects that we need to insert or update alongside with main persisted entity - cascadesSubjectBuilder.build(subject); - }); + await cascadesSubjectBuilder.build(subject); + })); // console.timeEnd("building cascades..."); // load database entities for all subjects we have @@ -171,4 +171,4 @@ export class EntityPersistExecutor { }); } -} \ No newline at end of file +} diff --git a/src/persistence/subject-builder/CascadesSubjectBuilder.ts b/src/persistence/subject-builder/CascadesSubjectBuilder.ts index 0e4a8c2713..17cac1f261 100644 --- a/src/persistence/subject-builder/CascadesSubjectBuilder.ts +++ b/src/persistence/subject-builder/CascadesSubjectBuilder.ts @@ -21,11 +21,11 @@ export class CascadesSubjectBuilder { /** * Builds a cascade subjects tree and pushes them in into the given array of subjects. */ - build(subject: Subject) { + async build(subject: Subject) { - subject.metadata - .extractRelationValuesFromEntity(subject.entity!, subject.metadata.relations) // todo: we can create EntityMetadata.cascadeRelations - .forEach(([relation, relationEntity, relationEntityMetadata]) => { + const relationValues = await subject.metadata + .extractRelationValuesFromEntity(subject.entity!, subject.metadata.relations); // todo: we can create EntityMetadata.cascadeRelations + await Promise.all(relationValues.map(async ([relation, relationEntity, relationEntityMetadata]) => { // we need only defined values and insert or update cascades of the relation should be set if (relationEntity === undefined || @@ -60,8 +60,8 @@ export class CascadesSubjectBuilder { this.allSubjects.push(relationEntitySubject); // go recursively and find other entities we need to insert/update - this.build(relationEntitySubject); - }); + await this.build(relationEntitySubject); + })); } // --------------------------------------------------------------------- @@ -84,4 +84,4 @@ export class CascadesSubjectBuilder { }); } -} \ No newline at end of file +} diff --git a/src/query-builder/RelationLoader.ts b/src/query-builder/RelationLoader.ts index 70da45f677..d3f7b6acfe 100644 --- a/src/query-builder/RelationLoader.ts +++ b/src/query-builder/RelationLoader.ts @@ -188,6 +188,7 @@ export class RelationLoader { const dataIndex = "__" + relation.propertyName + "__"; // in what property of the entity loaded data will be stored const promiseIndex = "__promise_" + relation.propertyName + "__"; // in what property of the entity loading promise will be stored const resolveIndex = "__has_" + relation.propertyName + "__"; // indicates if relation data already was loaded or not, we need this flag if loaded data is empty + const dirtyIndex = "__dirty_" + relation.propertyName + "__"; // indicates if relation Promise is "dirty" - ie a user-supplied promise set via the property setter Object.defineProperty(entity, relation.propertyName, { get: function() { @@ -209,11 +210,14 @@ export class RelationLoader { }, set: function(value: any|Promise) { if (value instanceof Promise) { // if set data is a promise then wait for its resolve and save in the object - value.then(result => { + this[dirtyIndex] = true; + this[promiseIndex] = value.then(result => { this[dataIndex] = result; this[resolveIndex] = true; + delete this[promiseIndex]; + delete this[dirtyIndex]; + return this[dataIndex]; }); - } else { // if its direct data set (non promise, probably not safe-typed) this[dataIndex] = value; this[resolveIndex] = true; @@ -223,4 +227,4 @@ export class RelationLoader { }); } -} \ No newline at end of file +} diff --git a/src/query-builder/transformer/PlainObjectToDatabaseEntityTransformer.ts b/src/query-builder/transformer/PlainObjectToDatabaseEntityTransformer.ts index 120db7acbd..ffcf6052be 100644 --- a/src/query-builder/transformer/PlainObjectToDatabaseEntityTransformer.ts +++ b/src/query-builder/transformer/PlainObjectToDatabaseEntityTransformer.ts @@ -94,16 +94,17 @@ export class PlainObjectToDatabaseEntityTransformer { // create a special load map that will hold all metadata that will be used to operate with entities easily const loadMap = new LoadMap(); - const fillLoadMap = (entity: ObjectLiteral, entityMetadata: EntityMetadata, parentLoadMapItem?: LoadMapItem, relation?: RelationMetadata) => { + const fillLoadMap = async (entity: ObjectLiteral, entityMetadata: EntityMetadata, parentLoadMapItem?: LoadMapItem, relation?: RelationMetadata) => { const item = new LoadMapItem(entity, entityMetadata, parentLoadMapItem, relation); loadMap.addLoadMap(item); - entityMetadata - .extractRelationValuesFromEntity(entity, metadata.relations) + const relationValues = await entityMetadata + .extractRelationValuesFromEntity(entity, metadata.relations); + await Promise.all(relationValues .filter(value => value !== null && value !== undefined) - .forEach(([relation, value, inverseEntityMetadata]) => fillLoadMap(value, inverseEntityMetadata, item, relation)); + .map(async ([relation, value, inverseEntityMetadata]) => await fillLoadMap(value, inverseEntityMetadata, item, relation))); }; - fillLoadMap(plainObject, metadata); + await fillLoadMap(plainObject, metadata); // load all entities and store them in the load map await Promise.all(loadMap.groupByTargetIds().map(targetWithIds => { // todo: fix type hinting return this.manager @@ -132,4 +133,4 @@ export class PlainObjectToDatabaseEntityTransformer { return loadMap.mainLoadMapItem ? loadMap.mainLoadMapItem.entity : undefined; } -} \ No newline at end of file +} diff --git a/src/query-builder/transformer/PlainObjectToNewEntityTransformer.ts b/src/query-builder/transformer/PlainObjectToNewEntityTransformer.ts index 4184544ba0..9baf5ca2e9 100644 --- a/src/query-builder/transformer/PlainObjectToNewEntityTransformer.ts +++ b/src/query-builder/transformer/PlainObjectToNewEntityTransformer.ts @@ -1,5 +1,6 @@ import {EntityMetadata} from "../../metadata/EntityMetadata"; import {ObjectLiteral} from "../../common/ObjectLiteral"; +import {RelationMetadata} from "../../metadata/RelationMetadata"; /** * Transforms plain old javascript object @@ -48,52 +49,67 @@ export class PlainObjectToNewEntityTransformer { if (objectRelatedValue === undefined) return; - if (relation.isOneToMany || relation.isManyToMany) { - if (!(objectRelatedValue instanceof Array)) - return; - - if (!entityRelatedValue) { - entityRelatedValue = []; - relation.setEntityValue(entity, entityRelatedValue); - } + if (relation.isLazy && objectRelatedValue instanceof Promise) { + // Set lazy entity value to a Promise which resolves to the transformed value + relation.setEntityValue( + entity, + objectRelatedValue.then(value => this.transformRelatedValue(entityRelatedValue, value, relation, getLazyRelationsPromiseValue)) + ); + } else { + relation.setEntityValue(entity, this.transformRelatedValue(entityRelatedValue, objectRelatedValue, relation, getLazyRelationsPromiseValue)); + } + }); + } + } - objectRelatedValue.forEach(objectRelatedValueItem => { + /** + * Transform and return an Entity for the provided relation value + */ + private transformRelatedValue(entityRelatedValue: ObjectLiteral, objectRelatedValue: ObjectLiteral, relation: RelationMetadata, getLazyRelationsPromiseValue: boolean) { - // check if we have this item from the merging object in the original entity we merge into - let objectRelatedValueEntity = (entityRelatedValue as any[]).find(entityRelatedValueItem => { - return relation.inverseEntityMetadata.compareEntities(objectRelatedValueItem, entityRelatedValueItem); - }); + if (relation.isOneToMany || relation.isManyToMany) { + if (!(objectRelatedValue instanceof Array)) + return; - // if such item already exist then merge new data into it, if its not we create a new entity and merge it into the array - if (!objectRelatedValueEntity) { - objectRelatedValueEntity = relation.inverseEntityMetadata.create(); - entityRelatedValue.push(objectRelatedValueEntity); - } + if (!entityRelatedValue) { + entityRelatedValue = []; + } - this.groupAndTransform(objectRelatedValueEntity, objectRelatedValueItem, relation.inverseEntityMetadata, getLazyRelationsPromiseValue); - }); + objectRelatedValue.forEach(objectRelatedValueItem => { - } else { + // check if we have this item from the merging object in the original entity we merge into + let objectRelatedValueEntity = (entityRelatedValue as any[]).find(entityRelatedValueItem => { + return relation.inverseEntityMetadata.compareEntities(objectRelatedValueItem, entityRelatedValueItem); + }); - // if related object isn't an object (direct relation id for example) - // we just set it to the entity relation, we don't need anything more from it - // however we do it only if original entity does not have this relation set to object - // to prevent full overriding of objects - if (!(objectRelatedValue instanceof Object)) { - if (!(entityRelatedValue instanceof Object)) - relation.setEntityValue(entity, objectRelatedValue); - return; - } - - if (!entityRelatedValue) { - entityRelatedValue = relation.inverseEntityMetadata.create(); - relation.setEntityValue(entity, entityRelatedValue); - } - - this.groupAndTransform(entityRelatedValue, objectRelatedValue, relation.inverseEntityMetadata, getLazyRelationsPromiseValue); + // if such item already exist then merge new data into it, if its not we create a new entity and merge it into the array + if (!objectRelatedValueEntity) { + objectRelatedValueEntity = relation.inverseEntityMetadata.create(); + entityRelatedValue.push(objectRelatedValueEntity); } + + this.groupAndTransform(objectRelatedValueEntity, objectRelatedValueItem, relation.inverseEntityMetadata, getLazyRelationsPromiseValue); }); + + } else { + + // if related object isn't an object (direct relation id for example) + // we just set it to the entity relation, we don't need anything more from it + // however we do it only if original entity does not have this relation set to object + // to prevent full overriding of objects + if (!(objectRelatedValue instanceof Object)) { + if (!(entityRelatedValue instanceof Object)) + entityRelatedValue = objectRelatedValue; + return; + } + + if (!entityRelatedValue) { + entityRelatedValue = relation.inverseEntityMetadata.create(); + } + + this.groupAndTransform(entityRelatedValue, objectRelatedValue, relation.inverseEntityMetadata, getLazyRelationsPromiseValue); } - } -} \ No newline at end of file + return entityRelatedValue; + } +} diff --git a/test/github-issues/2729/entity/Bar.ts b/test/github-issues/2729/entity/Bar.ts new file mode 100644 index 0000000000..801ebd6862 --- /dev/null +++ b/test/github-issues/2729/entity/Bar.ts @@ -0,0 +1,15 @@ +import { Column, Entity, OneToMany, PrimaryGeneratedColumn } from "../../../../src"; +import { Foo } from "./Foo"; + +@Entity() +export class Bar { + + @PrimaryGeneratedColumn() + id: number; + + @Column() + name: string; + + @OneToMany(() => Foo, foo => foo.bar, { lazy: true }) + foos: Promise; +} diff --git a/test/github-issues/2729/entity/Baz.ts b/test/github-issues/2729/entity/Baz.ts new file mode 100644 index 0000000000..11d3284bfd --- /dev/null +++ b/test/github-issues/2729/entity/Baz.ts @@ -0,0 +1,14 @@ +import { Column, Entity, OneToMany, PrimaryGeneratedColumn } from "../../../../src"; +import { Foo } from "./Foo"; + +@Entity() +export class Baz { + @PrimaryGeneratedColumn() + id: number; + + @Column() + name: string; + + @OneToMany(() => Foo, foo => foo.baz, { lazy: true }) + foos: Promise; +} diff --git a/test/github-issues/2729/entity/Foo.ts b/test/github-issues/2729/entity/Foo.ts new file mode 100644 index 0000000000..8643c1a988 --- /dev/null +++ b/test/github-issues/2729/entity/Foo.ts @@ -0,0 +1,18 @@ +import { Column, Entity, ManyToOne, PrimaryGeneratedColumn } from "../../../../src"; +import { Bar } from "./Bar"; +import { Baz } from "./Baz"; + +@Entity() +export class Foo { + @PrimaryGeneratedColumn() + id: number; + + @Column() + name: string; + + @ManyToOne(() => Bar, bar => bar.foos, { lazy: true }) + bar: Promise; + + @ManyToOne(() => Baz, baz => baz.foos, { lazy: true }) + baz: Promise; +} diff --git a/test/github-issues/2729/issue-2729.ts b/test/github-issues/2729/issue-2729.ts new file mode 100644 index 0000000000..f94f78b653 --- /dev/null +++ b/test/github-issues/2729/issue-2729.ts @@ -0,0 +1,76 @@ +import "reflect-metadata"; + +// import { expect } from "chai"; + +import { Connection } from "../../../src/connection/Connection"; +import { closeTestingConnections, createTestingConnections, reloadTestingDatabases } from "../../utils/test-utils"; +import { Foo } from "./entity/Foo"; +import { Bar } from "./entity/Bar"; +import { Baz } from "./entity/Baz"; + +// import { expect } from "chai"; +describe("github issues > #2729 creating entities with lazy relationships fails to handle Promises in input", () => { + + let connections: Connection[]; + before(async () => { + connections = await createTestingConnections({ + entities: [__dirname + "/entity/*{.js,.ts}"], + schemaCreate: true, + dropSchema: true, + }); + }); + + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("saves instance with unresolved Promise on \"one\" end lazy relation property", () => Promise.all(connections.map(async connection => { + + const fooRepo = connection.manager.getRepository(Foo); + const barRepo = connection.manager.getRepository(Bar); + + const barPromise = barRepo.save(barRepo.create({ name: "Bar" })); + + const foo = fooRepo.create({ name: "Foo", bar: barPromise }); + + await fooRepo.save(foo); + + foo.id.should.not.be.null; + + }))); + + it("saves instance with unresolved Promise on \"many\" end of lazy relation property", () => Promise.all(connections.map(async connection => { + + const fooRepo = connection.manager.getRepository(Foo); + const bazRepo = connection.manager.getRepository(Baz); + + const fooValues = [ + { name: "Foo1" }, + { name: "Foo2" }, + { name: "Foo3" }, + ]; + + // Create baz with Promise.resolve of foo values + const baz1 = bazRepo.create({ + name: "Baz1", + foos: Promise.resolve(fooValues), + }); + await bazRepo.save(baz1); + + // Create baz with resolved promise of actual entities + const savedFoosPromise = fooRepo.save(fooValues.map(values => fooRepo.create(values))); + + const baz2 = await bazRepo.save(bazRepo.create({ + name: "Baz2", + foos: savedFoosPromise, + })); + + baz1.id.should.not.be.null; + const baz1Foos = await baz1.foos; + baz1Foos.length.should.equal(3); + + baz2.id.should.not.be.null; + const baz2Foos = await baz2.foos; + baz2Foos.length.should.equal(3); + }))); + +});