fix: composite key save issue (#10672)

* chore(TypeORM): Create test case to uncover TypeORM composite key save issue

* chore(TypeORM): Revert package.json test script alteration

* fix the issue

* fixed regression in 1551

* fixed test to prevent issues with auto increment when inserting rows in a different order

* fixed test to prevent issues with auto increment when inserting rows in a different order

---------

Co-authored-by: Umed Khudoiberdiev <pleerock.me@gmail.com>

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "typeorm",
-  "version": "0.3.19",
+  "version": "0.3.20",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "typeorm",
-      "version": "0.3.19",
+      "version": "0.3.20",
       "license": "MIT",
       "dependencies": {
         "@sqltools/formatter": "^1.2.5",

src/metadata/ColumnMetadata.ts

@@ -694,19 +694,29 @@ export class ColumnMetadata {
                 entity[this.relationMetadata.propertyName] &&
                 ObjectUtils.isObject(entity[this.relationMetadata.propertyName])
             ) {
-                const map = this.relationMetadata.joinColumns.reduce(
-                    (map, joinColumn) => {
-                        const value =
-                            joinColumn.referencedColumn!.getEntityValueMap(
-                                entity[this.relationMetadata!.propertyName],
-                            )
-                        if (value === undefined) return map
-                        return OrmUtils.mergeDeep(map, value)
-                    },
-                    {},
-                )
-                if (Object.keys(map).length > 0)
-                    return { [this.propertyName]: map }
+                if (this.relationMetadata.joinColumns.length > 1) {
+                    const map = this.relationMetadata.joinColumns.reduce(
+                        (map, joinColumn) => {
+                            const value =
+                                joinColumn.referencedColumn!.getEntityValueMap(
+                                    entity[this.relationMetadata!.propertyName],
+                                )
+                            if (value === undefined) return map
+                            return OrmUtils.mergeDeep(map, value)
+                        },
+                        {},
+                    )
+                    if (Object.keys(map).length > 0)
+                        return { [this.propertyName]: map }
+                } else {
+                    const value =
+                        this.relationMetadata.joinColumns[0].referencedColumn!.getEntityValue(
+                            entity[this.relationMetadata!.propertyName],
+                        )
+                    if (value) {
+                        return { [this.propertyName]: value }
+                    }
+                }
 
                 return undefined
             } else {
@@ -714,8 +724,9 @@ export class ColumnMetadata {
                     entity[this.propertyName] !== undefined &&
                     (returnNulls === false ||
                         entity[this.propertyName] !== null)
-                )
+                ) {
                     return { [this.propertyName]: entity[this.propertyName] }
+                }
 
                 return undefined
             }

test/functional/tree-tables/nested-set/nested-set.ts

@@ -54,16 +54,18 @@ describe("tree tables > nested-set", () => {
                 ])
 
                 const a11Parent = await categoryRepository.findAncestors(a11)
-                a11Parent.length.should.be.equal(2)
-                a11Parent.should.deep.include({ id: 1, name: "a1" })
-                a11Parent.should.deep.include({ id: 2, name: "a11" })
+                const a11ParentNames = a11Parent.map((i) => i.name)
+                a11ParentNames.length.should.be.equal(2)
+                a11ParentNames.should.deep.include("a1")
+                a11ParentNames.should.deep.include("a11")
 
                 const a1Children = await categoryRepository.findDescendants(a1)
-                a1Children.length.should.be.equal(4)
-                a1Children.should.deep.include({ id: 1, name: "a1" })
-                a1Children.should.deep.include({ id: 2, name: "a11" })
-                a1Children.should.deep.include({ id: 3, name: "a111" })
-                a1Children.should.deep.include({ id: 4, name: "a12" })
+                const a1ChildrenNames = a1Children.map((i) => i.name)
+                a1ChildrenNames.length.should.be.equal(4)
+                a1ChildrenNames.should.deep.include("a1")
+                a1ChildrenNames.should.deep.include("a11")
+                a1ChildrenNames.should.deep.include("a111")
+                a1ChildrenNames.should.deep.include("a12")
             }),
         ))
 
@@ -95,15 +97,17 @@ describe("tree tables > nested-set", () => {
                 ])
 
                 const a11Parent = await categoryRepository.findAncestors(a11)
-                a11Parent.length.should.be.equal(2)
-                a11Parent.should.deep.include({ id: 1, name: "a1" })
-                a11Parent.should.deep.include({ id: 2, name: "a11" })
+                const a11ParentNames = a11Parent.map((i) => i.name)
+                a11ParentNames.length.should.be.equal(2)
+                a11ParentNames.should.deep.include("a1")
+                a11ParentNames.should.deep.include("a11")
 
                 const a1Children = await categoryRepository.findDescendants(a1)
-                a1Children.length.should.be.equal(3)
-                a1Children.should.deep.include({ id: 1, name: "a1" })
-                a1Children.should.deep.include({ id: 2, name: "a11" })
-                a1Children.should.deep.include({ id: 3, name: "a12" })
+                const a1ChildrenNames = a1Children.map((i) => i.name)
+                a1ChildrenNames.length.should.be.equal(3)
+                a1ChildrenNames.should.deep.include("a1")
+                a1ChildrenNames.should.deep.include("a11")
+                a1ChildrenNames.should.deep.include("a12")
             }),
         ))
 
@@ -135,15 +139,17 @@ describe("tree tables > nested-set", () => {
                 ])
 
                 const a11Parent = await categoryRepository.findAncestors(a11)
-                a11Parent.length.should.be.equal(2)
-                a11Parent.should.deep.include({ id: 1, name: "a1" })
-                a11Parent.should.deep.include({ id: 2, name: "a11" })
+                const a11ParentNames = a11Parent.map((i) => i.name)
+                a11ParentNames.length.should.be.equal(2)
+                a11ParentNames.should.deep.include("a1")
+                a11ParentNames.should.deep.include("a11")
 
                 const a1Children = await categoryRepository.findDescendants(a1)
-                a1Children.length.should.be.equal(3)
-                a1Children.should.deep.include({ id: 1, name: "a1" })
-                a1Children.should.deep.include({ id: 2, name: "a11" })
-                a1Children.should.deep.include({ id: 3, name: "a12" })
+                const a1ChildrenNames = a1Children.map((i) => i.name)
+                a1ChildrenNames.length.should.be.equal(3)
+                a1ChildrenNames.should.deep.include("a1")
+                a1ChildrenNames.should.deep.include("a11")
+                a1ChildrenNames.should.deep.include("a12")
             }),
         ))
 
@@ -181,9 +187,10 @@ describe("tree tables > nested-set", () => {
                 ])
 
                 const a11Parent = await categoryRepository.findAncestors(a11)
-                a11Parent.length.should.be.equal(2)
-                a11Parent.should.deep.include({ id: 1, name: "a1" })
-                a11Parent.should.deep.include({ id: 2, name: "a11" })
+                const a11ParentNames = a11Parent.map((child) => child.name)
+                a11ParentNames.length.should.be.equal(2)
+                a11ParentNames.should.deep.include("a1")
+                a11ParentNames.should.deep.include("a11")
 
                 const a1Children = await categoryRepository.findDescendants(a1)
                 const a1ChildrenNames = a1Children.map((child) => child.name)

test/github-issues/1551/issue-1551.ts

@@ -16,7 +16,7 @@ describe("github issues > #1551 complex example of cascades + multiple primary k
         async () =>
             (connections = await createTestingConnections({
                 __dirname,
-                enabledDrivers: ["mysql"],
+                // enabledDrivers: ["mysql"],
             })),
     )
     beforeEach(() => reloadTestingDatabases(connections))

test/github-issues/7068/issue-7068.ts

@@ -1,4 +1,4 @@
-import "reflect-metadata"
+import "../../utils/test-setup"
 import { Category } from "./entity/Category"
 import { DataSource } from "../../../src/data-source/DataSource"
 import {
@@ -47,15 +47,17 @@ describe("github issues > #7068", () => {
                 ])
 
                 const a11Parent = await categoryRepository.findAncestors(a11)
-                a11Parent.length.should.be.equal(2)
-                a11Parent.should.deep.include({ id: 1, name: "a1" })
-                a11Parent.should.deep.include({ id: 2, name: "a11" })
+                const names1 = a11Parent.map((child) => child.name)
+                names1.length.should.be.equal(2)
+                names1.should.deep.include("a1")
+                names1.should.deep.include("a11")
 
                 const a1Children = await categoryRepository.findDescendants(a1)
-                a1Children.length.should.be.equal(3)
-                a1Children.should.deep.include({ id: 1, name: "a1" })
-                a1Children.should.deep.include({ id: 2, name: "a11" })
-                a1Children.should.deep.include({ id: 3, name: "a12" })
+                const names2 = a1Children.map((child) => child.name)
+                names2.length.should.be.equal(3)
+                names2.should.deep.include("a1")
+                names2.should.deep.include("a11")
+                names2.should.deep.include("a12")
             }),
         ))
 
@@ -87,15 +89,17 @@ describe("github issues > #7068", () => {
                 ])
 
                 const a11Parent = await categoryRepository.findAncestors(a11)
-                a11Parent.length.should.be.equal(2)
-                a11Parent.should.deep.include({ id: 1, name: "a1" })
-                a11Parent.should.deep.include({ id: 2, name: "a11" })
+                const names1 = a11Parent.map((child) => child.name)
+                names1.length.should.be.equal(2)
+                names1.should.deep.include("a1")
+                names1.should.deep.include("a11")
 
                 const a1Children = await categoryRepository.findDescendants(a1)
-                a1Children.length.should.be.equal(3)
-                a1Children.should.deep.include({ id: 1, name: "a1" })
-                a1Children.should.deep.include({ id: 2, name: "a11" })
-                a1Children.should.deep.include({ id: 3, name: "a12" })
+                const names2 = a1Children.map((child) => child.name)
+                names2.length.should.be.equal(3)
+                names2.should.deep.include("a1")
+                names2.should.deep.include("a11")
+                names2.should.deep.include("a12")
             }),
         ))
 
@@ -133,9 +137,10 @@ describe("github issues > #7068", () => {
                 ])
 
                 const a11Parent = await categoryRepository.findAncestors(a11)
-                a11Parent.length.should.be.equal(2)
-                a11Parent.should.deep.include({ id: 1, name: "a1" })
-                a11Parent.should.deep.include({ id: 2, name: "a11" })
+                const names1 = a11Parent.map((child) => child.name)
+                names1.length.should.be.equal(2)
+                names1.should.deep.include("a1")
+                names1.should.deep.include("a11")
 
                 const a1Children = await categoryRepository.findDescendants(a1)
                 const a1ChildrenNames = a1Children.map((child) => child.name)

test/other-issues/composite-keys/composite-keys.ts

@@ -0,0 +1,109 @@
+import "reflect-metadata"
+import {
+    closeTestingConnections,
+    createTestingConnections,
+    reloadTestingDatabases,
+} from "../../utils/test-utils"
+import { DataSource } from "../../../src/data-source/DataSource"
+import { Policy } from "./entity/Policy"
+import { Group } from "./entity/Group"
+import { PolicyGroup } from "./entity/PolicyGroup"
+
+describe("other issues > composite keys doesn't work as expected in 0.3 compared to 0.2", () => {
+    let connections: DataSource[]
+    before(
+        async () =>
+            (connections = await createTestingConnections({
+                entities: [__dirname + "/entity/*{.js,.ts}"],
+            })),
+    )
+    beforeEach(() => reloadTestingDatabases(connections))
+    after(() => closeTestingConnections(connections))
+
+    it("should properly save new relation items", () =>
+        Promise.all(
+            connections.map(async function (connection) {
+                const group1 = new Group()
+                group1.id = 1
+                const group2 = new Group()
+                group2.id = 2
+
+                const policy1 = new Policy()
+                policy1.id = 1
+                const policy2 = new Policy()
+                policy2.id = 2
+
+                await connection.manager.save([
+                    group1,
+                    group2,
+                    policy1,
+                    policy2,
+                ])
+
+                const policyGroup1 = new PolicyGroup()
+                policyGroup1.groupId = group1.id
+                policyGroup1.policyId = policy1.id
+
+                await connection.manager.save([policyGroup1])
+
+                // re-load policy
+                const loadedPolicy = await connection.manager
+                    .getRepository(Policy)
+                    .findOneOrFail({
+                        where: { id: 1 },
+                        loadEagerRelations: false,
+                    })
+
+                const loadedPolicyGroups = await connection.manager
+                    .getRepository(PolicyGroup)
+                    .find({
+                        where: {
+                            policyId: loadedPolicy.id,
+                        },
+                        loadEagerRelations: false,
+                    })
+
+                const policyGroups2 = new PolicyGroup()
+                policyGroups2.groupId = group2.id
+                policyGroups2.policyId = policy1.id
+
+                loadedPolicy.groups = [...loadedPolicyGroups, policyGroups2]
+
+                await connection.manager.save(loadedPolicy)
+            }),
+        ))
+
+    it("should properly save new relation items - Drata", async () => {
+        for (const connection of connections) {
+            let policy = new Policy()
+            policy.id = 1
+
+            const group = new Group()
+            group.id = 1
+
+            let policyGroup = new PolicyGroup()
+            policyGroup.policy = policy
+            policyGroup.group = group
+
+            await connection.manager.save(policy)
+            await connection.manager.save(group)
+            await connection.manager.save(policyGroup)
+
+            /**
+             * Query everything back out to start fresh
+             */
+            policy = await connection.manager.findOneByOrFail(Policy, {})
+            policyGroup = await connection.manager.findOneByOrFail(
+                PolicyGroup,
+                {},
+            )
+
+            policy.groups = [policyGroup]
+
+            /**
+             * This save fails
+             */
+            await connection.manager.save(policy)
+        }
+    })
+})

test/other-issues/composite-keys/entity/Group.ts

@@ -0,0 +1,15 @@
+import { OneToMany, PrimaryColumn } from "../../../../src"
+import { Entity } from "../../../../src/decorator/entity/Entity"
+import { PolicyGroup } from "./PolicyGroup"
+
+@Entity()
+export class Group {
+    @PrimaryColumn()
+    id: number
+
+    @OneToMany(() => PolicyGroup, (policyGroup) => policyGroup.policy)
+    groups: PolicyGroup[]
+
+    @OneToMany(() => PolicyGroup, (policyGroup) => policyGroup.group)
+    policies: PolicyGroup[]
+}

test/other-issues/composite-keys/entity/Policy.ts

@@ -0,0 +1,15 @@
+import { Entity } from "../../../../src/decorator/entity/Entity"
+import { OneToMany, PrimaryColumn } from "../../../../src/index"
+import { PolicyGroup } from "./PolicyGroup"
+
+@Entity()
+export class Policy {
+    @PrimaryColumn()
+    id: number
+
+    @OneToMany(() => PolicyGroup, (policyGroup) => policyGroup.policy)
+    groups: PolicyGroup[]
+
+    @OneToMany(() => PolicyGroup, (policyGroup) => policyGroup.group)
+    policies: PolicyGroup[]
+}

test/other-issues/composite-keys/entity/PolicyGroup.ts

@@ -0,0 +1,26 @@
+import { Entity } from "../../../../src/decorator/entity/Entity"
+import { JoinColumn, ManyToOne, PrimaryColumn } from "../../../../src/index"
+import { Policy } from "./Policy"
+import { Group } from "./Group"
+
+@Entity()
+export class PolicyGroup {
+    @PrimaryColumn()
+    policyId: number
+
+    @ManyToOne(() => Policy, (policy) => policy.id, {
+        eager: true,
+        nullable: false,
+    })
+    @JoinColumn()
+    policy: Policy
+
+    @PrimaryColumn()
+    groupId: number
+
+    @ManyToOne(() => Group, (group) => group.id, {
+        eager: true,
+        nullable: false,
+    })
+    group: Group
+}