Skip to content

Commit

Permalink
fix: support combination of many-to-one/cacade/composte PK (#6417)
Browse files Browse the repository at this point in the history
handle setting values deeply in entities with relations in ColumnMetadata,
also check for a virtual relationship column in rawsqlresultstoentitytransformer

this allows us to handle a case where `many-to-one` with explicit composite PKs
columns were failing to persist a second time- instead of correctly updating the
field they would cause an insert to occur leading to a unique PK constraint error
  • Loading branch information
imnotjames committed Oct 21, 2020
1 parent 7ec1b75 commit 9a0497b
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 3 deletions.
13 changes: 12 additions & 1 deletion src/metadata/ColumnMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,18 @@ export class ColumnMetadata {
return extractEmbeddedColumnValue([...this.embeddedMetadata.embeddedMetadataTree], entity);

} else {
entity[this.propertyName] = value;
// we write a deep object in this entity only if the column is virtual
// because if its not virtual it means the user defined a real column for this relation
// also we don't do it if column is inside a junction table
if (!this.entityMetadata.isJunction && this.isVirtual && this.referencedColumn && this.referencedColumn.propertyName !== this.propertyName) {
if (!(this.propertyName in entity)) {
entity[this.propertyName] = {};
}

entity[this.propertyName][this.referencedColumn.propertyName] = value;
} else {
entity[this.propertyName] = value;
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/persistence/Subject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,8 @@ export class Subject {
if (this.parentSubject) {
this.metadata.primaryColumns.forEach(primaryColumn => {
if (primaryColumn.relationMetadata && primaryColumn.relationMetadata.inverseEntityMetadata === this.parentSubject!.metadata) {
primaryColumn.setEntityValue(this.entityWithFulfilledIds!, this.parentSubject!.entity);
const value = primaryColumn.referencedColumn!.getEntityValue(this.parentSubject!.entity!);
primaryColumn.setEntityValue(this.entityWithFulfilledIds!, value);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ export class RawSqlResultsToEntityTransformer {
const idMap = columns.reduce((idMap, column) => {
let value = result[column.databaseName];
if (relation.isOneToMany || relation.isOneToOneNotOwner) {
if (column.referencedColumn) // if column is a relation
if (column.isVirtual && column.referencedColumn && column.referencedColumn.propertyName !== column.propertyName) // if column is a relation
value = column.referencedColumn.createValueMap(value);

return OrmUtils.mergeDeep(idMap, column.createValueMap(value));
Expand Down
52 changes: 52 additions & 0 deletions test/github-issues/6416/entity/Post.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { EntitySchema } from "../../../../src";

import PostTag from "./PostTag";
import PostAttachment from "./PostAttachment";

let id = 0;

export default class Post {
postId: number;

otherId: number;

tags: PostTag[];

attachments: PostAttachment[];

constructor() {
this.postId = id++;
this.otherId = id++;
}
}

export const PostSchema = new EntitySchema<Post>({
name: "Post",
target: Post,
columns: {
otherId: {
type: Number,
primary: true,
nullable: false
},
postId: {
type: Number,
primary: true,
nullable: false
}
},
relations: {
tags: {
target: () => PostTag,
type: "one-to-many",
inverseSide: "post",
cascade: true
},
attachments: {
target: () => PostAttachment,
type: "one-to-many",
inverseSide: "post",
cascade: true
}
}
});
35 changes: 35 additions & 0 deletions test/github-issues/6416/entity/PostAttachment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { EntitySchema } from "../../../../src";

import Post from "./Post";

let id = 0;

export default class PostAttachment {
attachmentId: number;

post: Post;

constructor () {
this.attachmentId = id++;
}
}

export const PostAttachmentSchema = new EntitySchema<PostAttachment>({
name: "PostAttachment",
target: PostAttachment,
columns: {
attachmentId: {
type: Number,
primary: true,
nullable: false
}
},
relations: {
post: {
primary: true,
nullable: false,
target: () => Post,
type: "many-to-one"
}
}
});
51 changes: 51 additions & 0 deletions test/github-issues/6416/entity/PostTag.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { EntitySchema } from "../../../../src";

import Post from "./Post";

let id = 0;

export default class PostTag {
tagId: number;

tagOtherId: string;

tagPostId: string;

post: Post;

constructor () {
this.tagId = id++;
}
}

export const PostTagSchema = new EntitySchema<PostTag>({
name: "PostTag",
target: PostTag,
columns: {
tagOtherId: {
type: Number,
primary: true
},
tagPostId: {
type: Number,
primary: true
},
tagId: {
type: Number,
primary: true,
nullable: false
}
},
relations: {
post: {
primary: true,
nullable: false,
target: () => Post,
type: "many-to-one",
joinColumn: [
{ name: "tagPostId", referencedColumnName: "postId" },
{ name: "tagOtherId", referencedColumnName: "otherId" }
]
}
}
});
66 changes: 66 additions & 0 deletions test/github-issues/6416/issue-6416.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import {closeTestingConnections, createTestingConnections, reloadTestingDatabases} from "../../utils/test-utils";
import {Connection} from "../../../src";

import { assert } from "chai";

import Post, { PostSchema } from "./entity/Post";
import PostTag, { PostTagSchema } from "./entity/PostTag";
import PostAttachment, { PostAttachmentSchema } from "./entity/PostAttachment";

describe("github issues > #6399 Combining ManyToOne, Cascade, & Composite Primary Key causes Unique Constraint issues", () => {

let connections: Connection[];
before(async () => connections = await createTestingConnections({
entities: [PostSchema, PostTagSchema, PostAttachmentSchema],
enabledDrivers: ["sqlite"],
}));
beforeEach(() => reloadTestingDatabases(connections));
after(() => closeTestingConnections(connections));

it("persisting the cascading entities should succeed", () => Promise.all(connections.map(async connection => {

const post = new Post();
const postTag = new PostTag();
post.tags = [postTag];

await connection.manager.save(post, { reload: true });

try {
await connection.manager.save(post);
} catch (e) {
assert.fail(e.toString(), null, "Second save had an exception");
}
})));

it("persisting the cascading entities without JoinColumn should succeed", () => Promise.all(connections.map(async connection => {

const post = new Post();
const postAttachment = new PostAttachment();
post.attachments = [postAttachment];

await connection.manager.save(post, { reload: true });

try {
await connection.manager.save(post);
} catch (e) {
assert.fail(e.toString(), null, "Second save had an exception");
}
})));

it("persisting the child entity should succeed", () => Promise.all(connections.map(async connection => {
const post = new Post();

await connection.manager.save<Post>(post);

const postTag = new PostTag();
postTag.post = post;

await connection.manager.save(postTag, { reload: true });

try {
await connection.manager.save(postTag);
} catch (e) {
assert.fail(e.toString(), null, "Second save had an exception");
}
})));
});

0 comments on commit 9a0497b

Please sign in to comment.