From 938f94bded92b272bdcecc04534ffb879186dc44 Mon Sep 17 00:00:00 2001 From: ertl Date: Tue, 9 May 2023 12:49:22 +0200 Subject: [PATCH] fix: add onDelete option validation for oracle (#9786) * fix: add onDelete option validation for oracle Closes: #9189 * refactor: move fk validation logic to EntityMetadataValidator.ts * fix: skip assertion for other databases * fix: styles --------- Co-authored-by: ke --- src/driver/Driver.ts | 12 ++++++ src/driver/oracle/OracleDriver.ts | 19 ++++++++++ src/driver/oracle/OracleQueryRunner.ts | 8 ++-- .../EntityMetadataValidator.ts | 22 +++++++++++ test/github-issues/9189/entity/GroupEntity.ts | 12 ++++++ test/github-issues/9189/entity/UserEntity.ts | 12 ++++++ test/github-issues/9189/issue-9189.ts | 38 +++++++++++++++++++ 7 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 test/github-issues/9189/entity/GroupEntity.ts create mode 100644 test/github-issues/9189/entity/UserEntity.ts create mode 100644 test/github-issues/9189/issue-9189.ts diff --git a/src/driver/Driver.ts b/src/driver/Driver.ts index 6edec44f8..90a6fec01 100644 --- a/src/driver/Driver.ts +++ b/src/driver/Driver.ts @@ -14,6 +14,8 @@ import { Table } from "../schema-builder/table/Table" import { View } from "../schema-builder/view/View" import { TableForeignKey } from "../schema-builder/table/TableForeignKey" import { UpsertType } from "./types/UpsertType" +import { OnDeleteType } from "../metadata/types/OnDeleteType" +import { OnUpdateType } from "../metadata/types/OnUpdateType" export type ReturningType = "insert" | "update" | "delete" @@ -68,6 +70,16 @@ export interface Driver { */ supportedUpsertTypes: UpsertType[] + /** + * Returns list of supported onDelete types by driver + */ + supportedOnDeleteTypes?: OnDeleteType[] + + /** + * Returns list of supported onUpdate types by driver + */ + supportedOnUpdateTypes?: OnUpdateType[] + /** * Default values of length, precision and scale depends on column data type. * Used in the cases when length/precision/scale is not specified by user. diff --git a/src/driver/oracle/OracleDriver.ts b/src/driver/oracle/OracleDriver.ts index c0b0d4c03..2eafc1e47 100644 --- a/src/driver/oracle/OracleDriver.ts +++ b/src/driver/oracle/OracleDriver.ts @@ -26,6 +26,8 @@ import { TableForeignKey } from "../../schema-builder/table/TableForeignKey" import { TypeORMError } from "../../error" import { InstanceChecker } from "../../util/InstanceChecker" import { UpsertType } from "../types/UpsertType" +import { OnDeleteType } from "../../metadata/types/OnDeleteType" +import { OnUpdateType } from "../../metadata/types/OnUpdateType" /** * Organizes communication with Oracle RDBMS. @@ -133,6 +135,23 @@ export class OracleDriver implements Driver { */ supportedUpsertTypes: UpsertType[] = [] + /** + * Returns list of supported onDelete types by driver. + * https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/sql-language-reference.pdf + * Oracle does not support NO ACTION, but NO ACTION is set by default in EntityMetadata + */ + supportedOnDeleteTypes: OnDeleteType[] = [ + "CASCADE", + "SET NULL", + "NO ACTION", + ] + + /** + * Returns list of supported onUpdate types by driver. + * Oracle does not have onUpdate option, but we allow NO ACTION since it is set by default in EntityMetadata + */ + supportedOnUpdateTypes: OnUpdateType[] = ["NO ACTION"] + /** * Gets list of spatial column data types. */ diff --git a/src/driver/oracle/OracleQueryRunner.ts b/src/driver/oracle/OracleQueryRunner.ts index 5d0024e85..8cb4d3dd2 100644 --- a/src/driver/oracle/OracleQueryRunner.ts +++ b/src/driver/oracle/OracleQueryRunner.ts @@ -2785,10 +2785,10 @@ export class OracleQueryRunner extends BaseQueryRunner implements QueryRunner { }" FOREIGN KEY (${columnNames}) REFERENCES ${this.escapePath( this.getTablePath(fk), )} (${referencedColumnNames})` - if (fk.onDelete && fk.onDelete !== "NO ACTION") + if (fk.onDelete && fk.onDelete !== "NO ACTION") { // Oracle does not support NO ACTION, but we set NO ACTION by default in EntityMetadata constraint += ` ON DELETE ${fk.onDelete}` - + } return constraint }) .join(", ") @@ -3038,9 +3038,9 @@ export class OracleQueryRunner extends BaseQueryRunner implements QueryRunner { this.getTablePath(foreignKey), )} (${referencedColumnNames})` // Oracle does not support NO ACTION, but we set NO ACTION by default in EntityMetadata - if (foreignKey.onDelete && foreignKey.onDelete !== "NO ACTION") + if (foreignKey.onDelete && foreignKey.onDelete !== "NO ACTION") { sql += ` ON DELETE ${foreignKey.onDelete}` - + } return new Query(sql) } diff --git a/src/metadata-builder/EntityMetadataValidator.ts b/src/metadata-builder/EntityMetadataValidator.ts index 8862e275a..6d78c49b0 100644 --- a/src/metadata-builder/EntityMetadataValidator.ts +++ b/src/metadata-builder/EntityMetadataValidator.ts @@ -226,6 +226,28 @@ export class EntityMetadataValidator { // validate relations entityMetadata.relations.forEach((relation) => { + // check OnDeleteTypes + if ( + driver.supportedOnDeleteTypes && + relation.onDelete && + !driver.supportedOnDeleteTypes.includes(relation.onDelete) + ) { + throw new TypeORMError( + `OnDeleteType "${relation.onDelete}" is not supported for ${driver.options.type}!`, + ) + } + + // check OnUpdateTypes + if ( + driver.supportedOnUpdateTypes && + relation.onUpdate && + !driver.supportedOnUpdateTypes.includes(relation.onUpdate) + ) { + throw new TypeORMError( + `OnUpdateType "${relation.onUpdate}" is not valid for ${driver.options.type}!`, + ) + } + // check join tables: // using JoinTable is possible only on one side of the many-to-many relation // todo(dima): fix diff --git a/test/github-issues/9189/entity/GroupEntity.ts b/test/github-issues/9189/entity/GroupEntity.ts new file mode 100644 index 000000000..5395734e5 --- /dev/null +++ b/test/github-issues/9189/entity/GroupEntity.ts @@ -0,0 +1,12 @@ +import { Entity, ManyToOne } from "../../../../src" +import { PrimaryGeneratedColumn } from "../../../../src" +import type { UserEntity } from "./UserEntity" + +@Entity() +export class GroupEntity { + @PrimaryGeneratedColumn() + id: number + + @ManyToOne("UserEntity", "group", { onDelete: "RESTRICT" }) + user: UserEntity +} diff --git a/test/github-issues/9189/entity/UserEntity.ts b/test/github-issues/9189/entity/UserEntity.ts new file mode 100644 index 000000000..0d54a4cb5 --- /dev/null +++ b/test/github-issues/9189/entity/UserEntity.ts @@ -0,0 +1,12 @@ +import { Entity, OneToMany } from "../../../../src" +import { PrimaryGeneratedColumn } from "../../../../src" +import { GroupEntity } from "./GroupEntity" + +@Entity() +export class UserEntity { + @PrimaryGeneratedColumn() + id: number + + @OneToMany("GroupEntity", "user") + group: GroupEntity +} diff --git a/test/github-issues/9189/issue-9189.ts b/test/github-issues/9189/issue-9189.ts new file mode 100644 index 000000000..6f3664315 --- /dev/null +++ b/test/github-issues/9189/issue-9189.ts @@ -0,0 +1,38 @@ +import "reflect-metadata" +import { + closeTestingConnections, + createTestingConnections, +} from "../../utils/test-utils" +import { DataSource, TypeORMError } from "../../../src" +import { expect } from "chai" + +describe("github issues > #9189 check invalid constraint options", () => { + let dataSources: DataSource[] = [] + + after(() => closeTestingConnections(dataSources)) + + it("should throw an exception, when invalid option is configured", async () => { + let err + try { + await Promise.all( + (dataSources = await createTestingConnections({ + entities: [__dirname + "/entity/*{.js,.ts}"], + schemaCreate: false, + dropSchema: true, + enabledDrivers: ["oracle"], + })), + ) + } catch (e) { + err = e + } + if (err) + // skip for other databases + expect(err).to.eql( + new TypeORMError( + 'OnDeleteType "RESTRICT" is not supported for oracle!', + ), + ) + }) + + // you can add additional tests if needed +})