diff --git a/src/lazy-loading/LazyRelationsWrapper.ts b/src/lazy-loading/LazyRelationsWrapper.ts index 5aa011c70..4d3bc3524 100644 --- a/src/lazy-loading/LazyRelationsWrapper.ts +++ b/src/lazy-loading/LazyRelationsWrapper.ts @@ -27,13 +27,15 @@ export class LazyRelationsWrapper { if (relation.hasInverseSide) { // if we don't have inverse side then we can't select and join by relation from inverse side qb.select(relation.propertyName) .from(relation.inverseRelation.entityMetadata.target, relation.propertyName) - .innerJoin(`${relation.propertyName}.${relation.inverseRelation.propertyName}`, relation.entityMetadata.targetName); + .innerJoin(`${relation.propertyName}.${relation.inverseRelation.propertyName}`, relation.entityMetadata.targetName) + .andWhereInIds([relation.entityMetadata.getEntityIdMixedMap(this)]); } else { qb.select(relation.propertyName) .from(relation.type, relation.propertyName) .innerJoin(relation.junctionEntityMetadata.table.name, relation.junctionEntityMetadata.name, `${relation.junctionEntityMetadata.name}.${relation.name}=:${relation.propertyName}Id`) - .setParameter(relation.propertyName + "Id", this[relation.referencedColumnName]); + .setParameter(relation.propertyName + "Id", this[relation.referencedColumnName]) + .andWhereInIds([relation.entityMetadata.getEntityIdMixedMap(this)]); } this[loadIndex] = qb.getMany().then(results => { @@ -51,18 +53,21 @@ export class LazyRelationsWrapper { if (relation.hasInverseSide) { qb.select(relation.propertyName) .from(relation.inverseRelation.entityMetadata.target, relation.propertyName) - .innerJoin(`${relation.propertyName}.${relation.inverseRelation.propertyName}`, relation.entityMetadata.targetName); + .innerJoin(`${relation.propertyName}.${relation.inverseRelation.propertyName}`, relation.entityMetadata.targetName) + .andWhereInIds([relation.entityMetadata.getEntityIdMixedMap(this)]); } else { // (ow) post.category<=>category.post // loaded: category from post + // example: SELECT category.id AS category_id, category.name AS category_name FROM category category + // INNER JOIN post Post ON Post.category=category.id WHERE Post.id=1 qb.select(relation.propertyName) // category .from(relation.type, relation.propertyName) // Category, category .innerJoin(relation.entityMetadata.target as Function, relation.entityMetadata.name, - `${relation.entityMetadata.name}.${relation.propertyName}=:${relation.propertyName}Id`) // Post, post, post.category = categoryId - .setParameter(relation.propertyName + "Id", this[relation.referencedColumnName]); + `${relation.entityMetadata.name}.${relation.propertyName}=${relation.propertyName}.${relation.referencedColumn.propertyName}`) + .andWhereInIds([relation.entityMetadata.getEntityIdMixedMap(this)]); } - // console.log(qb.getSql()); + this[loadIndex] = qb.getOne().then(result => { this[index] = result; this[resolveIndex] = true; diff --git a/src/metadata/RelationMetadata.ts b/src/metadata/RelationMetadata.ts index 5b30cc37f..0eb6226cb 100644 --- a/src/metadata/RelationMetadata.ts +++ b/src/metadata/RelationMetadata.ts @@ -417,6 +417,13 @@ export class RelationMetadata { return this.isLazy ? entity["__" + this.propertyName + "__"] : entity[this.propertyName]; } + /** + * Checks if given entity has a value in a relation. + */ + hasEntityValue(entity: ObjectLiteral): boolean { + return this.isLazy ? entity["__" + this.propertyName + "__"] : entity[this.propertyName]; + } + /** * todo: lazy relations are not supported here? implement logic? * diff --git a/src/persistence/SubjectBuilder.ts b/src/persistence/SubjectBuilder.ts index ca9447301..a9daa1a46 100644 --- a/src/persistence/SubjectBuilder.ts +++ b/src/persistence/SubjectBuilder.ts @@ -83,6 +83,9 @@ export class SubjectBuilder { // Public Methods // ------------------------------------------------------------------------- + /** + * Builds operations for entity that is being inserted/updated. + */ async persist(entity: Entity, metadata: EntityMetadata): Promise { // create subject for currently persisted entity and mark that it can be inserted and updated @@ -111,6 +114,9 @@ export class SubjectBuilder { await this.buildJunctionOperations({ insert: true, remove: true }); } + /** + * Builds only remove operations for entity that is being removed. + */ async remove(entity: Entity, metadata: EntityMetadata): Promise { // create subject for currently removed entity and mark that it must be removed diff --git a/src/persistence/SubjectOperationExecutor.ts b/src/persistence/SubjectOperationExecutor.ts index d2d77ec74..4373ba85e 100644 --- a/src/persistence/SubjectOperationExecutor.ts +++ b/src/persistence/SubjectOperationExecutor.ts @@ -180,7 +180,7 @@ export class SubjectOperationExecutor { const updateOptions: ObjectLiteral = {}; subject.metadata.relationsWithJoinColumns.forEach(relation => { const referencedColumn = relation.joinColumn.referencedColumn; - const relatedEntity = subject.entity[relation.propertyName]; + const relatedEntity = relation.getEntityValue(subject.entity); // if relation value is not set then nothing to do here if (!relatedEntity) @@ -373,7 +373,7 @@ export class SubjectOperationExecutor { /** * Collects columns and values for the insert operation. */ - private collectColumnsAndValues(metadata: EntityMetadata, entity: any, date: Date, parentIdColumnValue: any, discriminatorValue: any, alreadyInsertedSubjects: Subject[]): ObjectLiteral { + private collectColumnsAndValues(metadata: EntityMetadata, entity: ObjectLiteral, date: Date, parentIdColumnValue: any, discriminatorValue: any, alreadyInsertedSubjects: Subject[]): ObjectLiteral { // extract all columns const columns = metadata.columns.filter(column => { @@ -381,7 +381,7 @@ export class SubjectOperationExecutor { }); const relationColumns = metadata.relationsWithJoinColumns - .filter(relation => entity.hasOwnProperty(relation.propertyName)); + .filter(relation => relation.hasEntityValue(entity)); const columnNames = columns.map(column => column.name); const relationColumnNames = relationColumns.map(relation => relation.name); @@ -394,8 +394,6 @@ export class SubjectOperationExecutor { // extract all values const relationValues = relationColumns.map(relation => { const value = relation.getEntityValue(entity); - if (value === null || value === undefined) - return value; // if relation value is stored in the entity itself then use it from there const relationId = relation.getInverseEntityRelationId(value); // todo: check it diff --git a/test/issues/47/entity/Category.ts b/test/issues/47/entity/Category.ts new file mode 100644 index 000000000..02ae73ada --- /dev/null +++ b/test/issues/47/entity/Category.ts @@ -0,0 +1,19 @@ +import {Table} from "../../../../src/decorator/tables/Table"; +import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn"; +import {Column} from "../../../../src/decorator/columns/Column"; +import {Post} from "./Post"; +import {OneToMany} from "../../../../src/decorator/relations/OneToMany"; + +@Table() +export class Category { + + @PrimaryGeneratedColumn() + id: number; + + @Column() + name: string; + + @OneToMany(() => Post, post => post.category) + posts: Promise; + +} \ No newline at end of file diff --git a/test/issues/47/entity/Post.ts b/test/issues/47/entity/Post.ts new file mode 100644 index 000000000..55ccba52f --- /dev/null +++ b/test/issues/47/entity/Post.ts @@ -0,0 +1,21 @@ +import {Table} from "../../../../src/decorator/tables/Table"; +import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn"; +import {Column} from "../../../../src/decorator/columns/Column"; +import {Category} from "./Category"; +import {ManyToOne} from "../../../../src/decorator/relations/ManyToOne"; + +@Table() +export class Post { + + @PrimaryGeneratedColumn() + id: number; + + @Column() + title: string; + + @ManyToOne(() => Category, category => category.posts, { + cascadeInsert: true + }) + category: Promise; + +} \ No newline at end of file diff --git a/test/issues/47/issue-47.ts b/test/issues/47/issue-47.ts new file mode 100644 index 000000000..87a1a00cb --- /dev/null +++ b/test/issues/47/issue-47.ts @@ -0,0 +1,79 @@ +import "reflect-metadata"; +import {setupTestingConnections, closeConnections, reloadDatabases} from "../../utils/test-utils"; +import {Connection} from "../../../src/connection/Connection"; +import {Post} from "./entity/Post"; +import {expect} from "chai"; +import {Category} from "./entity/Category"; + +describe("github issues > #47 wrong sql syntax when loading lazy relation", () => { + + let connections: Connection[]; + before(async () => connections = await setupTestingConnections({ + entities: [__dirname + "/entity/*{.js,.ts}"], + schemaCreate: true, + dropSchemaOnConnection: true, + })); + beforeEach(() => reloadDatabases(connections)); + after(() => closeConnections(connections)); + + it("should persist successfully and return persisted entity", () => Promise.all(connections.map(async connection => { + + // create objects to save + const category1 = new Category(); + category1.name = "category #1"; + + const post1 = new Post(); + post1.title = "Hello Post #1"; + post1.category = Promise.resolve(category1); + + const category2 = new Category(); + category2.name = "category #2"; + + const post2 = new Post(); + post2.title = "Hello Post #2"; + post2.category = Promise.resolve(category2); + + // persist + await connection.entityManager.persist(category1); + await connection.entityManager.persist(post1); + await connection.entityManager.persist(category2); + await connection.entityManager.persist(post2); + + // check that all persisted objects exist + const loadedPost = await connection.entityManager + .createQueryBuilder(Post, "post") + .getMany(); + + const loadedCategory1 = await loadedPost[0].category; + expect(loadedCategory1!).not.to.be.empty; + loadedCategory1!.should.be.eql({ + id: 1, + name: "category #1" + }); + + const loadedCategory2 = await loadedPost[1].category; + expect(loadedCategory2!).not.to.be.empty; + loadedCategory2!.should.be.eql({ + id: 2, + name: "category #2" + }); + + const loadedPosts1 = await loadedCategory1.posts; + expect(loadedPosts1!).not.to.be.empty; + loadedPosts1!.should.be.eql([{ + id: 1, + title: "Hello Post #1" + }]); + + const loadedPosts2 = await loadedCategory2.posts; + expect(loadedPosts2!).not.to.be.empty; + loadedPosts2!.should.be.eql([{ + id: 2, + title: "Hello Post #2" + }]); + + // todo: need to test somehow how query is being generated, or how many raw data is returned + + }))); + +}); \ No newline at end of file