fix: add onDelete option validation for oracle (#9786)

ertl · web-flow · commit 938f94bded92 · 2023-05-09T15:49:22.000+05:00
* 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 <ke@sbs.co.at>
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
+})
