Skip to content

Commit

Permalink
fix: allow falsey discriminator values (typeorm#6973)
Browse files Browse the repository at this point in the history
allow discriminator values in Single Table Inheritance
like 0, "", or `null`

fixes typeorm#3891
  • Loading branch information
imnotjames authored and Svetlozar committed Jan 12, 2021
1 parent 627a5dd commit 127ea14
Show file tree
Hide file tree
Showing 10 changed files with 138 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/decorator/entity/ChildEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function ChildEntity(discriminatorValue?: any): ClassDecorator {
} as TableMetadataArgs);

// register discriminator value if it was provided
if (discriminatorValue) {
if (typeof discriminatorValue !== 'undefined') {
getMetadataArgsStorage().discriminatorValues.push({
target: target,
value: discriminatorValue
Expand Down
7 changes: 6 additions & 1 deletion src/metadata-builder/EntityMetadataBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,12 @@ export class EntityMetadataBuilder {
const entityInheritance = this.metadataArgsStorage.findInheritanceType(entityMetadata.target);

const discriminatorValue = this.metadataArgsStorage.findDiscriminatorValue(entityMetadata.target);
entityMetadata.discriminatorValue = discriminatorValue ? discriminatorValue.value : (entityMetadata.target as any).name; // todo: pass this to naming strategy to generate a name

if (typeof discriminatorValue !== "undefined") {
entityMetadata.discriminatorValue = discriminatorValue.value;
} else {
entityMetadata.discriminatorValue = (entityMetadata.target as any).name;
}

// if single table inheritance is used, we need to mark all embedded columns as nullable
entityMetadata.embeddeds = this.createEmbeddedsRecursively(entityMetadata, this.metadataArgsStorage.filterEmbeddeds(entityMetadata.inheritanceTree))
Expand Down
4 changes: 2 additions & 2 deletions src/metadata-builder/EntityMetadataValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ export class EntityMetadataValidator {
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.`);
if (typeof entityMetadata.discriminatorValue === "undefined")
throw new Error(`Entity ${entityMetadata.name} has an undefined discriminator value. Discriminator value should be defined.`);

const sameDiscriminatorValueEntityMetadata = allEntityMetadatas.find(metadata => {
return metadata !== entityMetadata
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class RawSqlResultsToEntityTransformer {
if (metadata.discriminatorColumn) {
const discriminatorValues = rawResults.map(result => result[DriverUtils.buildColumnAlias(this.driver, alias.name, alias.metadata.discriminatorColumn!.databaseName)]);
const discriminatorMetadata = metadata.childEntityMetadatas.find(childEntityMetadata => {
return !!discriminatorValues.find(value => value === childEntityMetadata.discriminatorValue);
return typeof discriminatorValues.find(value => value === childEntityMetadata.discriminatorValue) !== 'undefined';
});
if (discriminatorMetadata)
metadata = discriminatorMetadata;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import {Column} from "../../../../../../src/decorator/columns/Column";
import {ChildEntity} from "../../../../../../src/decorator/entity/ChildEntity";
import {Person} from "./Person";

@ChildEntity("")
export class Other extends Person {

@Column()
mood: string;

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import {closeTestingConnections, createTestingConnections, reloadTestingDatabase
import {Connection} from "../../../../../src/connection/Connection";
import {Student} from "./entity/Student";
import {Employee} from "./entity/Employee";
import {Other} from "./entity/Other";
import {Person} from "./entity/Person";
import {OracleDriver} from "../../../../../src/driver/oracle/OracleDriver";

describe("table-inheritance > single-table > non-virtual-discriminator-column", () => {

Expand All @@ -30,6 +32,15 @@ describe("table-inheritance > single-table > non-virtual-discriminator-column",
employee.salary = 1000;
await connection.getRepository(Employee).save(employee);

if (!(connection.driver instanceof OracleDriver)) {
// In Oracle, empty string is a `null` so this isn't exactly possible there.

const other = new Other();
other.name = "Empty";
other.mood = "Happy"
await connection.getRepository(Other).save(other);
}

// -------------------------------------------------------------------------
// Select
// -------------------------------------------------------------------------
Expand All @@ -47,6 +58,15 @@ describe("table-inheritance > single-table > non-virtual-discriminator-column",
persons[1].type.should.be.equal("employee-type");
persons[1].name.should.be.equal("Roger");
(persons[1] as Employee).salary.should.be.equal(1000);

if (!(connection.driver instanceof OracleDriver)) {
// In Oracle, empty string is a `null` so this isn't exactly possible there.

persons[2].id.should.be.equal(3);
persons[2].type.should.be.equal("");
persons[2].name.should.be.equal("Empty");
(persons[2] as Other).mood.should.be.equal("Happy");
}
})));

});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import {Column} from "../../../../../../src/decorator/columns/Column";
import {TableInheritance} from "../../../../../../src/decorator/entity/TableInheritance";
import {Entity} from "../../../../../../src/decorator/entity/Entity";
import {PrimaryGeneratedColumn} from "../../../../../../src/decorator/columns/PrimaryGeneratedColumn";

@Entity()
@TableInheritance({ column: { name: "type", type: "int" } })
export class Person {

@PrimaryGeneratedColumn()
id: number;

@Column()
name: string;

@Column()
type: number;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import {Column} from "../../../../../../src/decorator/columns/Column";
import {ChildEntity} from "../../../../../../src/decorator/entity/ChildEntity";
import {Person} from "./Person";

@ChildEntity(0)
export class Student extends Person {

@Column()
faculty: string;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import {Column} from "../../../../../../src/decorator/columns/Column";
import {ChildEntity} from "../../../../../../src/decorator/entity/ChildEntity";
import {Person} from "./Person";

@ChildEntity(1)
export class Teacher extends Person {

@Column()
specialization: string;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import "reflect-metadata";
import {expect} from "chai";
import {closeTestingConnections, createTestingConnections, reloadTestingDatabases} from "../../../../utils/test-utils";
import {Connection} from "../../../../../src/connection/Connection";
import {Student} from "./entity/Student";
import {Teacher} from "./entity/Teacher";
import {Person} from "./entity/Person";
import {CockroachDriver} from "../../../../../src/driver/cockroachdb/CockroachDriver";

describe("table-inheritance > single-table > numeric types", () => {

let connections: Connection[];
before(async () => connections = await createTestingConnections({
entities: [Person, Student, Teacher]
}));
beforeEach(() => reloadTestingDatabases(connections));
after(() => closeTestingConnections(connections));

it("should allow numeric types for the discriminator, including 0", () => Promise.all(connections.map(async connection => {
if (connection.driver instanceof CockroachDriver) {
return;
}

// -------------------------------------------------------------------------
// Create
// -------------------------------------------------------------------------

const student = new Student();
student.name = "Alice";
student.faculty = "Economics";
await connection.getRepository(Student).save(student);

const teacher = new Teacher();
teacher.name = "Roger";
teacher.specialization = "Math";
await connection.getRepository(Teacher).save(teacher);

// -------------------------------------------------------------------------
// Select
// -------------------------------------------------------------------------

let persons = await connection.manager
.createQueryBuilder(Person, "person")
.getMany();

expect(persons[0].id).to.be.equal(1);
expect(persons[0].type).to.be.equal(0);
expect(persons[0].name).to.be.equal("Alice");
expect((persons[0] as Student).faculty).to.be.equal("Economics");

expect(persons[1].id).to.be.equal(2);
expect(persons[1].type).to.be.equal(1);
expect(persons[1].name).to.be.equal("Roger");
expect((persons[1] as Teacher).specialization).to.be.equal("Math");
})));

});

0 comments on commit 127ea14

Please sign in to comment.