From 22f7d1080fcdd86eb9992bf8e7154379edc14ca4 Mon Sep 17 00:00:00 2001 From: Jan Arends Date: Tue, 3 Sep 2024 13:53:37 +0200 Subject: [PATCH] Fix error on nullable update within Oracle DB (#23436) * more differentiation between creation and update and some comments * added db helper for nullable update * integrated db helper * comments * changeset * update comment, autoformat * make new fields nullable by default we dont transmit unchanged values in the advanced creation form which means `is_nullable` will be undefined because thats the default before this pr it worked because we fell back due to `... ?? true` --------- Co-authored-by: Daniel Biegler --- .changeset/brave-snails-smell.md | 5 +++ api/src/database/helpers/index.ts | 2 + .../nullable-update/dialects/default.ts | 3 ++ .../nullable-update/dialects/oracle.ts | 19 +++++++++ .../database/helpers/nullable-update/index.ts | 7 ++++ .../database/helpers/nullable-update/types.ts | 16 ++++++++ api/src/services/fields.ts | 40 ++++++++++++++----- 7 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 .changeset/brave-snails-smell.md create mode 100644 api/src/database/helpers/nullable-update/dialects/default.ts create mode 100644 api/src/database/helpers/nullable-update/dialects/oracle.ts create mode 100644 api/src/database/helpers/nullable-update/index.ts create mode 100644 api/src/database/helpers/nullable-update/types.ts diff --git a/.changeset/brave-snails-smell.md b/.changeset/brave-snails-smell.md new file mode 100644 index 0000000000..9095a16e96 --- /dev/null +++ b/.changeset/brave-snails-smell.md @@ -0,0 +1,5 @@ +--- +'@directus/api': minor +--- + +Fixed OracleDB error when updating nullable option with the same value as before diff --git a/api/src/database/helpers/index.ts b/api/src/database/helpers/index.ts index 5a01e6cb11..47543e074b 100644 --- a/api/src/database/helpers/index.ts +++ b/api/src/database/helpers/index.ts @@ -8,6 +8,7 @@ import * as geometryHelpers from './geometry/index.js'; import * as schemaHelpers from './schema/index.js'; import * as sequenceHelpers from './sequence/index.js'; import * as numberHelpers from './number/index.js'; +import * as nullableUpdateHelper from './nullable-update/index.js'; export function getHelpers(database: Knex) { const client = getDatabaseClient(database); @@ -18,6 +19,7 @@ export function getHelpers(database: Knex) { schema: new schemaHelpers[client](database), sequence: new sequenceHelpers[client](database), number: new numberHelpers[client](database), + nullableUpdate: new nullableUpdateHelper[client](database), }; } diff --git a/api/src/database/helpers/nullable-update/dialects/default.ts b/api/src/database/helpers/nullable-update/dialects/default.ts new file mode 100644 index 0000000000..ffcddab620 --- /dev/null +++ b/api/src/database/helpers/nullable-update/dialects/default.ts @@ -0,0 +1,3 @@ +import { NullableFieldUpdateHelper } from '../types.js'; + +export class NullableFieldUpdateHelperDefault extends NullableFieldUpdateHelper {} diff --git a/api/src/database/helpers/nullable-update/dialects/oracle.ts b/api/src/database/helpers/nullable-update/dialects/oracle.ts new file mode 100644 index 0000000000..895efee0f6 --- /dev/null +++ b/api/src/database/helpers/nullable-update/dialects/oracle.ts @@ -0,0 +1,19 @@ +import type { Column } from '@directus/schema'; +import type { Field, RawField } from '@directus/types'; +import type { Knex } from 'knex'; +import { NullableFieldUpdateHelper } from '../types.js'; + +/** + * Oracle throws an error when overwriting the nullable option with same value. + * Therefore we need to check if the nullable option has changed and only then apply it. + * The default value can be set regardless of the previous value. + */ +export class NullableFieldUpdateHelperOracle extends NullableFieldUpdateHelper { + override updateNullableValue(column: Knex.ColumnBuilder, field: RawField | Field, existing: Column): void { + if (field.schema?.is_nullable === false && existing.is_nullable === true) { + column.notNullable(); + } else if (field.schema?.is_nullable === true && existing.is_nullable === false) { + column.nullable(); + } + } +} diff --git a/api/src/database/helpers/nullable-update/index.ts b/api/src/database/helpers/nullable-update/index.ts new file mode 100644 index 0000000000..1055f085c9 --- /dev/null +++ b/api/src/database/helpers/nullable-update/index.ts @@ -0,0 +1,7 @@ +export { NullableFieldUpdateHelperOracle as oracle } from './dialects/oracle.js'; +export { NullableFieldUpdateHelperDefault as postgres } from './dialects/default.js'; +export { NullableFieldUpdateHelperDefault as mysql } from './dialects/default.js'; +export { NullableFieldUpdateHelperDefault as cockroachdb } from './dialects/default.js'; +export { NullableFieldUpdateHelperDefault as redshift } from './dialects/default.js'; +export { NullableFieldUpdateHelperDefault as sqlite } from './dialects/default.js'; +export { NullableFieldUpdateHelperDefault as mssql } from './dialects/default.js'; diff --git a/api/src/database/helpers/nullable-update/types.ts b/api/src/database/helpers/nullable-update/types.ts new file mode 100644 index 0000000000..d84ee2d0fe --- /dev/null +++ b/api/src/database/helpers/nullable-update/types.ts @@ -0,0 +1,16 @@ +import type { Knex } from 'knex'; +import { DatabaseHelper } from '../types.js'; +import type { Column } from '@directus/schema'; +import type { Field, RawField } from '@directus/types'; + +export class NullableFieldUpdateHelper extends DatabaseHelper { + updateNullableValue(column: Knex.ColumnBuilder, field: RawField | Field, existing: Column): void { + const isNullable = field.schema?.is_nullable ?? existing?.is_nullable ?? true; + + if (isNullable) { + column.nullable(); + } else { + column.notNullable(); + } + } +} diff --git a/api/src/services/fields.ts b/api/src/services/fields.ts index 6bd44bb2d4..597cb2c4ea 100644 --- a/api/src/services/fields.ts +++ b/api/src/services/fields.ts @@ -859,10 +859,7 @@ export class FieldsService { throw new InvalidPayloadError({ reason: `Illegal type passed: "${field.type}"` }); } - const defaultValue = - field.schema?.default_value !== undefined ? field.schema?.default_value : existing?.default_value; - - if (defaultValue) { + const setDefaultValue = (defaultValue: string | number | boolean | null) => { const newDefaultValueIsString = typeof defaultValue === 'string'; const newDefaultIsNowFunction = newDefaultValueIsString && defaultValue.toLowerCase() === 'now()'; const newDefaultIsCurrentTimestamp = newDefaultValueIsString && defaultValue === 'CURRENT_TIMESTAMP'; @@ -882,16 +879,37 @@ export class FieldsService { } else { column.defaultTo(defaultValue); } - } else { - column.defaultTo(null); - } + }; - const isNullable = field.schema?.is_nullable ?? existing?.is_nullable ?? true; + // for a new item, set the default value and nullable as provided without any further considerations + if (!existing) { + if (field.schema?.default_value !== undefined) { + setDefaultValue(field.schema.default_value); + } - if (isNullable) { - column.nullable(); + if (field.schema?.is_nullable || field.schema?.is_nullable === undefined) { + column.nullable(); + } else { + column.notNullable(); + } } else { - column.notNullable(); + // for an existing item: if nullable option changed, we have to provide the default values as well and actually vice versa + // see https://knexjs.org/guide/schema-builder.html#alter + // To overwrite a nullable option with the same value this is not possible for Oracle though, hence the DB helper + + if (field.schema?.default_value !== undefined || field.schema?.is_nullable !== undefined) { + this.helpers.nullableUpdate.updateNullableValue(column, field, existing); + + let defaultValue = null; + + if (field.schema?.default_value !== undefined) { + defaultValue = field.schema.default_value; + } else if (existing.default_value !== undefined) { + defaultValue = existing.default_value; + } + + setDefaultValue(defaultValue); + } } if (field.schema?.is_primary_key) {