From 33b2bd7acc55d6eb30bfe0681748d6b6abaff0b5 Mon Sep 17 00:00:00 2001 From: Ranjan Purbey Date: Sat, 26 Feb 2022 14:45:38 +0530 Subject: [PATCH] fix: fix entityManager.getId for custom join table (#8676) getId currently returns undefined for an entity with composite primary key if primary key columns also foreign keys with lazy relations, for e.g., in a custom join table. This commit tries to fix that Closes: #7736 (maybe) --- src/metadata/ColumnMetadata.ts | 7 ++- .../entity/Category.ts | 10 ++++ .../entity/Post.ts | 10 ++++ .../entity/PostCategory.ts | 24 ++++++++++ .../get-id-for-composite-primary-key.ts | 47 +++++++++++++++++++ 5 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 test/other-issues/get-id-for-composite-primary-key/entity/Category.ts create mode 100644 test/other-issues/get-id-for-composite-primary-key/entity/Post.ts create mode 100644 test/other-issues/get-id-for-composite-primary-key/entity/PostCategory.ts create mode 100644 test/other-issues/get-id-for-composite-primary-key/get-id-for-composite-primary-key.ts diff --git a/src/metadata/ColumnMetadata.ts b/src/metadata/ColumnMetadata.ts index 9e4f759f0e..0aaa78e8ef 100644 --- a/src/metadata/ColumnMetadata.ts +++ b/src/metadata/ColumnMetadata.ts @@ -570,7 +570,12 @@ export class ColumnMetadata { return Object.keys(map).length > 0 ? map : undefined; } else { // no embeds - no problems. Simply return column property name and its value of the entity - if (this.relationMetadata && entity[this.relationMetadata.propertyName] && entity[this.relationMetadata.propertyName] instanceof Object) { + /** + * Object.getOwnPropertyDescriptor checks if the relation is lazy, in which case value is a Promise + * DO NOT use `entity[this.relationMetadata.propertyName] instanceof Promise`, which will invoke property getter and make unwanted DB request + * refer: https://github.com/typeorm/typeorm/pull/8676#issuecomment-1049906331 + */ + if (this.relationMetadata && !Object.getOwnPropertyDescriptor(entity, this.relationMetadata.propertyName)?.get && entity[this.relationMetadata.propertyName] && entity[this.relationMetadata.propertyName] instanceof Object) { const map = this.relationMetadata.joinColumns.reduce((map, joinColumn) => { const value = joinColumn.referencedColumn!.getEntityValueMap(entity[this.relationMetadata!.propertyName]); if (value === undefined) return map; diff --git a/test/other-issues/get-id-for-composite-primary-key/entity/Category.ts b/test/other-issues/get-id-for-composite-primary-key/entity/Category.ts new file mode 100644 index 0000000000..61d7b87bfc --- /dev/null +++ b/test/other-issues/get-id-for-composite-primary-key/entity/Category.ts @@ -0,0 +1,10 @@ +import { Column, Entity, PrimaryGeneratedColumn } from "../../../../src"; + +@Entity() +export class Category { + @PrimaryGeneratedColumn("increment") + id!: number; + + @Column() + name!: string; +} diff --git a/test/other-issues/get-id-for-composite-primary-key/entity/Post.ts b/test/other-issues/get-id-for-composite-primary-key/entity/Post.ts new file mode 100644 index 0000000000..26c9fc7dce --- /dev/null +++ b/test/other-issues/get-id-for-composite-primary-key/entity/Post.ts @@ -0,0 +1,10 @@ +import { Column, Entity, PrimaryGeneratedColumn } from "../../../../src"; + +@Entity() +export class Post { + @PrimaryGeneratedColumn("increment") + id!: number; + + @Column() + content!: string; +} diff --git a/test/other-issues/get-id-for-composite-primary-key/entity/PostCategory.ts b/test/other-issues/get-id-for-composite-primary-key/entity/PostCategory.ts new file mode 100644 index 0000000000..0bb75df4d4 --- /dev/null +++ b/test/other-issues/get-id-for-composite-primary-key/entity/PostCategory.ts @@ -0,0 +1,24 @@ +import { + CreateDateColumn, + Entity, + ManyToOne, + PrimaryColumn, +} from "../../../../src"; +import { Category } from "./Category"; +import { Post } from "./Post"; + +@Entity() +export class PostCategory { + @ManyToOne(() => Post) + post!: Promise; + @PrimaryColumn() + postId!: Post["id"]; + + @ManyToOne(() => Category) + category!: Promise; + @PrimaryColumn() + categoryId!: Category["id"]; + + @CreateDateColumn() + added!: Date; +} diff --git a/test/other-issues/get-id-for-composite-primary-key/get-id-for-composite-primary-key.ts b/test/other-issues/get-id-for-composite-primary-key/get-id-for-composite-primary-key.ts new file mode 100644 index 0000000000..cc750dc5f5 --- /dev/null +++ b/test/other-issues/get-id-for-composite-primary-key/get-id-for-composite-primary-key.ts @@ -0,0 +1,47 @@ +import { expect } from "chai"; +import "reflect-metadata"; +import { Connection } from "../../../src/connection/Connection"; +import { + closeTestingConnections, + createTestingConnections, + reloadTestingDatabases, +} from "../../utils/test-utils"; +import { Category } from "./entity/Category"; +import { Post } from "./entity/Post"; +import { PostCategory } from "./entity/PostCategory"; + +describe("other issues > getId should not return undefined for composite primary keys with lazy relations", () => { + let connections: Connection[]; + before( + async () => + (connections = await createTestingConnections({ + entities: [__dirname + "/entity/*{.js,.ts}"], + })) + ); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("getId should not return undefined", () => + Promise.all( + connections.map(async ({ manager }) => { + const post = manager.create(Post, { + content: "Sample Post", + }); + await manager.save(post); + + const category = manager.create(Category, { + name: "javascript", + }); + await manager.save(category); + + const postCategory = manager.create(PostCategory, {}); + postCategory.post = Promise.resolve(post); + postCategory.category = Promise.resolve(category); + await manager.save(postCategory); + + expect(manager.getId(post)).not.to.be.undefined; + expect(manager.getId(category)).not.to.be.undefined; + expect(manager.getId(postCategory)).not.to.be.undefined; + }) + )); +});