Skip to content

Commit

Permalink
fix: Only check for discriminator conflicts on STI entities (#2985)
Browse files Browse the repository at this point in the history
If an entity has the same discriminator value as one using `TableInheritance` this should be allowed as 
there is no way for them to conflict.

This commit also fixes the discriminatorColumn 
check to check all the entities it should be
checking. Only the parent in an STI setup has the
"STI" inheritancePattern, the children just have the 
"entity-child" tableType. Before two children with 
the same discriminatorValue would be
(incorrectly) allowed as they'd never even get 
checked.

Fixes #2984
  • Loading branch information
amaranth committed Oct 20, 2020
1 parent c0d691d commit 06903d1
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/metadata-builder/EntityMetadataValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,18 @@ export class EntityMetadataValidator {

// validate if table is using inheritance it has a discriminator
// also validate if discriminator values are not empty and not repeated
if (entityMetadata.inheritancePattern === "STI") {
if (entityMetadata.inheritancePattern === "STI" || entityMetadata.tableType === "entity-child") {
if (!entityMetadata.discriminatorColumn)
throw new Error(`Entity ${entityMetadata.name} using single-table inheritance, it should also have a discriminator column. Did you forget to put discriminator column options?`);

if (["", undefined, null].indexOf(entityMetadata.discriminatorValue) !== -1)
throw new Error(`Entity ${entityMetadata.name} has empty discriminator value. Discriminator value should not be empty.`);

const sameDiscriminatorValueEntityMetadata = allEntityMetadatas.find(metadata => {
return metadata !== entityMetadata && metadata.discriminatorValue === entityMetadata.discriminatorValue;
return metadata !== entityMetadata
&& (metadata.inheritancePattern === "STI" || metadata.tableType === "entity-child")
&& metadata.discriminatorValue === entityMetadata.discriminatorValue
&& metadata.inheritanceTree.some(parent => entityMetadata.inheritanceTree.indexOf(parent) !== -1);
});
if (sameDiscriminatorValueEntityMetadata)
throw new Error(`Entities ${entityMetadata.name} and ${sameDiscriminatorValueEntityMetadata.name} have the same discriminator values. Make sure they are different while using the @ChildEntity decorator.`);
Expand Down
9 changes: 9 additions & 0 deletions test/github-issues/2984/entity/commit/note.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import {Entity, PrimaryGeneratedColumn} from "../../../../../src";

@Entity({name: "commitNote"})
export class Note {

@PrimaryGeneratedColumn()
public id: number;

}
10 changes: 10 additions & 0 deletions test/github-issues/2984/entity/issue/note.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import {Entity, PrimaryGeneratedColumn, TableInheritance} from "../../../../../src";

@Entity({name: "issueNote"})
@TableInheritance({column: {type: "varchar", name: "type"}})
export class Note {

@PrimaryGeneratedColumn()
public id: number;

}
10 changes: 10 additions & 0 deletions test/github-issues/2984/entity/issue/ownernote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import {ChildEntity, Column} from "../../../../../src";
import {Note} from "./note";

@ChildEntity()
export class OwnerNote extends Note {

@Column()
public owner: string;

}
10 changes: 10 additions & 0 deletions test/github-issues/2984/entity/wiki/note.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import {Entity, PrimaryGeneratedColumn, TableInheritance} from "../../../../../src";

@Entity({name: "wikiNote"})
@TableInheritance({column: {type: "varchar", name: "type"}})
export class Note {

@PrimaryGeneratedColumn()
public id: number;

}
10 changes: 10 additions & 0 deletions test/github-issues/2984/entity/wiki/ownernote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import {ChildEntity, Column} from "../../../../../src";
import {Note} from "./note";

@ChildEntity()
export class OwnerNote extends Note {

@Column()
public owner: string;

}
21 changes: 21 additions & 0 deletions test/github-issues/2984/issue-2984.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import "reflect-metadata";

import {closeTestingConnections, createTestingConnections, reloadTestingDatabases} from "../../utils/test-utils";
import {Connection} from "../../../src";

describe("github issues > #2984 Discriminator conflict reported even for non-inherited tables", () => {

let connections: Connection[];
before(async () => connections = await createTestingConnections({
entities: [__dirname + "/entity/**/*{.js,.ts}"],
}));
beforeEach(() => reloadTestingDatabases(connections));
after(() => closeTestingConnections(connections));

it("should load entities even with the same discriminator", () => Promise.all(connections.map(async connection => {
connection.entityMetadatas.should.have.length(5);
connection.entityMetadatas.forEach(metadata =>
metadata.discriminatorValue!.should.be.oneOf(["Note", "OwnerNote"]));
})));

});

0 comments on commit 06903d1

Please sign in to comment.