Fix/enum validation (#2863)

* fix: SELECT enum can have a color key

* fix: "findOneOrFail" of undefined

* feat: alter column migration store previous metadata informations

* fix: enum validation extra keys
This commit is contained in:
Jérémy M
2023-12-07 17:04:49 +01:00
committed by GitHub
parent 145b432dc6
commit d70f8deeec
13 changed files with 176 additions and 109 deletions

View File

@@ -26,7 +26,6 @@ const bootstrap = async () => {
// Apply validation pipes globally
app.useGlobalPipes(
new ValidationPipe({
// whitelist: true,
transform: true,
}),
);

View File

@@ -16,6 +16,7 @@ import { TypeORMModule } from 'src/database/typeorm/typeorm.module';
import { IsFieldMetadataDefaultValue } from 'src/metadata/field-metadata/validators/is-field-metadata-default-value.validator';
import { FieldMetadataResolver } from 'src/metadata/field-metadata/field-metadata.resolver';
import { FieldMetadataDTO } from 'src/metadata/field-metadata/dtos/field-metadata.dto';
import { IsFieldMetadataOptions } from 'src/metadata/field-metadata/validators/is-field-metadata-options.validator';
import { FieldMetadataService } from './field-metadata.service';
import { FieldMetadataEntity } from './field-metadata.entity';
@@ -65,6 +66,7 @@ import { UpdateFieldInput } from './dtos/update-field.input';
],
providers: [
IsFieldMetadataDefaultValue,
IsFieldMetadataOptions,
FieldMetadataService,
FieldMetadataResolver,
],

View File

@@ -57,7 +57,13 @@ export const validateDefaultValueForType = (
FieldMetadataDefaultValue
>(validator, defaultValue);
return validateSync(defaultValueInstance).length === 0;
return (
validateSync(defaultValueInstance, {
whitelist: true,
forbidNonWhitelisted: true,
forbidUnknownValues: true,
}).length === 0
);
});
return isValid;

View File

@@ -11,7 +11,7 @@ import {
export const optionsValidatorsMap = {
[FieldMetadataType.RATING]: [FieldMetadataDefaultOptions],
[FieldMetadataType.SELECT]: [FieldMetadataDefaultOptions],
[FieldMetadataType.SELECT]: [FieldMetadataComplexOptions],
[FieldMetadataType.MULTI_SELECT]: [FieldMetadataComplexOptions],
};
@@ -31,12 +31,18 @@ export const validateOptionsForType = (
const isValid = options.every((option) => {
return validators.some((validator) => {
const optionsInstance = plainToInstance<any, FieldMetadataDefaultOptions>(
validator,
option,
);
const optionsInstance = plainToInstance<
any,
FieldMetadataDefaultOptions | FieldMetadataComplexOptions
>(validator, option);
return validateSync(optionsInstance).length === 0;
return (
validateSync(optionsInstance, {
whitelist: true,
forbidNonWhitelisted: true,
forbidUnknownValues: true,
}).length === 0
);
});
});

View File

@@ -5,7 +5,10 @@ import { ValidationArguments, ValidatorConstraint } from 'class-validator';
import { FieldMetadataOptions } from 'src/metadata/field-metadata/interfaces/field-metadata-options.interface';
import { FieldMetadataService } from 'src/metadata/field-metadata/field-metadata.service';
import { FieldMetadataType } from 'src/metadata/field-metadata/field-metadata.entity';
import {
FieldMetadataEntity,
FieldMetadataType,
} from 'src/metadata/field-metadata/field-metadata.entity';
import { validateOptionsForType } from 'src/metadata/field-metadata/utils/validate-options-for-type.util';
@Injectable()
@@ -28,7 +31,13 @@ export class IsFieldMetadataOptions {
return false;
}
const fieldMetadata = await this.fieldMetadataService.findOneOrFail(id);
let fieldMetadata: FieldMetadataEntity;
try {
fieldMetadata = await this.fieldMetadataService.findOneOrFail(id);
} catch {
return false;
}
type = fieldMetadata.type;
}

View File

@@ -47,21 +47,31 @@ export class BasicColumnActionFactory extends ColumnActionAbstractFactory<BasicF
}
protected handleAlterAction(
previousFieldMetadata: FieldMetadataInterface<BasicFieldMetadataType>,
nextFieldMetadata: FieldMetadataInterface<BasicFieldMetadataType>,
currentFieldMetadata: FieldMetadataInterface<BasicFieldMetadataType>,
alteredFieldMetadata: FieldMetadataInterface<BasicFieldMetadataType>,
options?: WorkspaceColumnActionOptions,
): WorkspaceMigrationColumnAlter {
const defaultValue =
this.getDefaultValue(nextFieldMetadata.defaultValue) ??
this.getDefaultValue(alteredFieldMetadata.defaultValue) ??
options?.defaultValue;
const serializedDefaultValue = serializeDefaultValue(defaultValue);
return {
action: WorkspaceMigrationColumnActionType.ALTER,
columnName: nextFieldMetadata.targetColumnMap.value,
columnType: fieldMetadataTypeToColumnType(nextFieldMetadata.type),
isNullable: nextFieldMetadata.isNullable,
defaultValue: serializedDefaultValue,
currentColumnDefinition: {
columnName: currentFieldMetadata.targetColumnMap.value,
columnType: fieldMetadataTypeToColumnType(currentFieldMetadata.type),
isNullable: currentFieldMetadata.isNullable,
defaultValue: serializeDefaultValue(
this.getDefaultValue(currentFieldMetadata.defaultValue),
),
},
alteredColumnDefinition: {
columnName: alteredFieldMetadata.targetColumnMap.value,
columnType: fieldMetadataTypeToColumnType(alteredFieldMetadata.type),
isNullable: alteredFieldMetadata.isNullable,
defaultValue: serializedDefaultValue,
},
};
}

View File

@@ -23,21 +23,21 @@ export class ColumnActionAbstractFactory<
action:
| WorkspaceMigrationColumnActionType.CREATE
| WorkspaceMigrationColumnActionType.ALTER,
previousFieldMetadata: FieldMetadataInterface<T> | undefined,
nextFieldMetadata: FieldMetadataInterface<T>,
currentFieldMetadata: FieldMetadataInterface<T> | undefined,
alteredFieldMetadata: FieldMetadataInterface<T>,
options?: WorkspaceColumnActionOptions,
): WorkspaceMigrationColumnAction {
switch (action) {
case WorkspaceMigrationColumnActionType.CREATE:
return this.handleCreateAction(nextFieldMetadata, options);
return this.handleCreateAction(alteredFieldMetadata, options);
case WorkspaceMigrationColumnActionType.ALTER: {
if (!previousFieldMetadata) {
throw new Error('Previous field metadata is required for alter');
if (!currentFieldMetadata) {
throw new Error('current field metadata is required for alter');
}
return this.handleAlterAction(
previousFieldMetadata,
nextFieldMetadata,
currentFieldMetadata,
alteredFieldMetadata,
options,
);
}
@@ -57,8 +57,8 @@ export class ColumnActionAbstractFactory<
}
protected handleAlterAction(
_previousFieldMetadata: FieldMetadataInterface<T>,
_nextFieldMetadata: FieldMetadataInterface<T>,
_currentFieldMetadata: FieldMetadataInterface<T>,
_alteredFieldMetadata: FieldMetadataInterface<T>,
_options?: WorkspaceColumnActionOptions,
): WorkspaceMigrationColumnAlter {
throw new Error('handleAlterAction method not implemented.');

View File

@@ -45,24 +45,24 @@ export class EnumColumnActionFactory extends ColumnActionAbstractFactory<EnumFie
}
protected handleAlterAction(
previousFieldMetadata: FieldMetadataInterface<EnumFieldMetadataType>,
nextFieldMetadata: FieldMetadataInterface<EnumFieldMetadataType>,
currentFieldMetadata: FieldMetadataInterface<EnumFieldMetadataType>,
alteredFieldMetadata: FieldMetadataInterface<EnumFieldMetadataType>,
options: WorkspaceColumnActionOptions,
): WorkspaceMigrationColumnAlter {
const defaultValue =
nextFieldMetadata.defaultValue?.value ?? options?.defaultValue;
alteredFieldMetadata.defaultValue?.value ?? options?.defaultValue;
const serializedDefaultValue = serializeDefaultValue(defaultValue);
const enumOptions = nextFieldMetadata.options
const enumOptions = alteredFieldMetadata.options
? [
...nextFieldMetadata.options.map((option) => {
const previousOption = previousFieldMetadata.options?.find(
(previousOption) => previousOption.id === option.id,
...alteredFieldMetadata.options.map((option) => {
const currentOption = currentFieldMetadata.options?.find(
(currentOption) => currentOption.id === option.id,
);
// The id is the same, but the value is different, so we need to alter the enum
if (previousOption && previousOption.value !== option.value) {
if (currentOption && currentOption.value !== option.value) {
return {
from: previousOption.value,
from: currentOption.value,
to: option.value,
};
}
@@ -74,12 +74,26 @@ export class EnumColumnActionFactory extends ColumnActionAbstractFactory<EnumFie
return {
action: WorkspaceMigrationColumnActionType.ALTER,
columnName: nextFieldMetadata.targetColumnMap.value,
columnType: fieldMetadataTypeToColumnType(nextFieldMetadata.type),
enum: enumOptions,
isArray: nextFieldMetadata.type === FieldMetadataType.MULTI_SELECT,
isNullable: nextFieldMetadata.isNullable,
defaultValue: serializedDefaultValue,
currentColumnDefinition: {
columnName: currentFieldMetadata.targetColumnMap.value,
columnType: fieldMetadataTypeToColumnType(currentFieldMetadata.type),
enum: currentFieldMetadata.options
? [...currentFieldMetadata.options.map((option) => option.value)]
: undefined,
isArray: currentFieldMetadata.type === FieldMetadataType.MULTI_SELECT,
isNullable: currentFieldMetadata.isNullable,
defaultValue: serializeDefaultValue(
currentFieldMetadata.defaultValue?.value,
),
},
alteredColumnDefinition: {
columnName: alteredFieldMetadata.targetColumnMap.value,
columnType: fieldMetadataTypeToColumnType(alteredFieldMetadata.type),
enum: enumOptions,
isArray: alteredFieldMetadata.type === FieldMetadataType.MULTI_SELECT,
isNullable: alteredFieldMetadata.isNullable,
defaultValue: serializedDefaultValue,
},
};
}
}

View File

@@ -14,8 +14,8 @@ export interface WorkspaceColumnActionFactory<
action:
| WorkspaceMigrationColumnActionType.CREATE
| WorkspaceMigrationColumnActionType.ALTER,
previousFieldMetadata: FieldMetadataInterface<T> | undefined,
nextFieldMetadata: FieldMetadataInterface<T>,
currentFieldMetadata: FieldMetadataInterface<T> | undefined,
alteredFieldMetadata: FieldMetadataInterface<T>,
options?: WorkspaceColumnActionOptions,
): WorkspaceMigrationColumnAction;
}

View File

@@ -13,24 +13,24 @@ export enum WorkspaceMigrationColumnActionType {
export type WorkspaceMigrationEnum = string | { from: string; to: string };
export type WorkspaceMigrationColumnCreate = {
action: WorkspaceMigrationColumnActionType.CREATE;
export interface WorkspaceMigrationColumnDefinition {
columnName: string;
columnType: string;
enum?: WorkspaceMigrationEnum[];
isArray?: boolean;
isNullable?: boolean;
defaultValue?: any;
};
}
export interface WorkspaceMigrationColumnCreate
extends WorkspaceMigrationColumnDefinition {
action: WorkspaceMigrationColumnActionType.CREATE;
}
export type WorkspaceMigrationColumnAlter = {
action: WorkspaceMigrationColumnActionType.ALTER;
columnName: string;
columnType: string;
enum?: WorkspaceMigrationEnum[];
isArray?: boolean;
isNullable?: boolean;
defaultValue?: any;
currentColumnDefinition: WorkspaceMigrationColumnDefinition;
alteredColumnDefinition: WorkspaceMigrationColumnDefinition;
};
export type WorkspaceMigrationColumnRelation = {

View File

@@ -97,51 +97,51 @@ export class WorkspaceMigrationFactory {
createColumnActions(
action: WorkspaceMigrationColumnActionType.ALTER,
previousFieldMetadata: FieldMetadataInterface,
nextFieldMetadata: FieldMetadataInterface,
currentFieldMetadata: FieldMetadataInterface,
alteredFieldMetadata: FieldMetadataInterface,
): WorkspaceMigrationColumnAction[];
createColumnActions(
action:
| WorkspaceMigrationColumnActionType.CREATE
| WorkspaceMigrationColumnActionType.ALTER,
fieldMetadataOrPreviousFieldMetadata: FieldMetadataInterface,
undefinedOrnextFieldMetadata?: FieldMetadataInterface,
fieldMetadataOrCurrentFieldMetadata: FieldMetadataInterface,
undefinedOrAlteredFieldMetadata?: FieldMetadataInterface,
): WorkspaceMigrationColumnAction[] {
const previousFieldMetadata =
const currentFieldMetadata =
action === WorkspaceMigrationColumnActionType.ALTER
? fieldMetadataOrPreviousFieldMetadata
? fieldMetadataOrCurrentFieldMetadata
: undefined;
const nextFieldMetadata =
const alteredFieldMetadata =
action === WorkspaceMigrationColumnActionType.CREATE
? fieldMetadataOrPreviousFieldMetadata
: undefinedOrnextFieldMetadata;
? fieldMetadataOrCurrentFieldMetadata
: undefinedOrAlteredFieldMetadata;
if (!nextFieldMetadata) {
if (!alteredFieldMetadata) {
this.logger.error(
`No field metadata provided for action ${action}`,
fieldMetadataOrPreviousFieldMetadata,
undefinedOrAlteredFieldMetadata,
);
throw new Error(`No field metadata provided for action ${action}`);
}
// If it's a composite field type, we need to create a column action for each of the fields
if (isCompositeFieldMetadataType(nextFieldMetadata.type)) {
if (isCompositeFieldMetadataType(alteredFieldMetadata.type)) {
const fieldMetadataCollection = this.compositeDefinitions.get(
nextFieldMetadata.type,
alteredFieldMetadata.type,
);
if (!fieldMetadataCollection) {
this.logger.error(
`No composite definition found for type ${nextFieldMetadata.type}`,
`No composite definition found for type ${alteredFieldMetadata.type}`,
{
nextFieldMetadata,
alteredFieldMetadata,
},
);
throw new Error(
`No composite definition found for type ${nextFieldMetadata.type}`,
`No composite definition found for type ${alteredFieldMetadata.type}`,
);
}
@@ -153,8 +153,8 @@ export class WorkspaceMigrationFactory {
// Otherwise, we create a single column action
const columnAction = this.createColumnAction(
action,
previousFieldMetadata,
nextFieldMetadata,
currentFieldMetadata,
alteredFieldMetadata,
);
return [columnAction];
@@ -164,24 +164,27 @@ export class WorkspaceMigrationFactory {
action:
| WorkspaceMigrationColumnActionType.CREATE
| WorkspaceMigrationColumnActionType.ALTER,
previousFieldMetadata: FieldMetadataInterface | undefined,
nextFieldMetadata: FieldMetadataInterface,
currentFieldMetadata: FieldMetadataInterface | undefined,
alteredFieldMetadata: FieldMetadataInterface,
): WorkspaceMigrationColumnAction {
const { factory, options } =
this.factoriesMap.get(nextFieldMetadata.type) ?? {};
this.factoriesMap.get(alteredFieldMetadata.type) ?? {};
if (!factory) {
this.logger.error(`No factory found for type ${nextFieldMetadata.type}`, {
nextFieldMetadata,
});
this.logger.error(
`No factory found for type ${alteredFieldMetadata.type}`,
{
alteredFieldMetadata,
},
);
throw new Error(`No factory found for type ${nextFieldMetadata.type}`);
throw new Error(`No factory found for type ${alteredFieldMetadata.type}`);
}
return factory.create(
action,
previousFieldMetadata,
nextFieldMetadata,
currentFieldMetadata,
alteredFieldMetadata,
options,
);
}

View File

@@ -12,10 +12,11 @@ export class WorkspaceMigrationEnumService {
tableName: string,
migrationColumn: WorkspaceMigrationColumnAlter,
) {
const oldEnumTypeName = `${tableName}_${migrationColumn.columnName}_enum`;
const newEnumTypeName = `${tableName}_${migrationColumn.columnName}_enum_new`;
const columnDefinition = migrationColumn.alteredColumnDefinition;
const oldEnumTypeName = `${tableName}_${columnDefinition.columnName}_enum`;
const newEnumTypeName = `${tableName}_${columnDefinition.columnName}_enum_new`;
const enumValues =
migrationColumn.enum?.map((enumValue) => {
columnDefinition.enum?.map((enumValue) => {
if (typeof enumValue === 'string') {
return enumValue;
}
@@ -23,8 +24,8 @@ export class WorkspaceMigrationEnumService {
return enumValue.to;
}) ?? [];
if (!migrationColumn.isNullable && !migrationColumn.defaultValue) {
migrationColumn.defaultValue = migrationColumn.enum?.[0];
if (!columnDefinition.isNullable && !columnDefinition.defaultValue) {
columnDefinition.defaultValue = columnDefinition.enum?.[0];
}
// Create new enum type with new values
@@ -38,7 +39,7 @@ export class WorkspaceMigrationEnumService {
// Temporarily change column type to text
await queryRunner.query(`
ALTER TABLE "${schemaName}"."${tableName}"
ALTER COLUMN "${migrationColumn.columnName}" TYPE TEXT
ALTER COLUMN "${columnDefinition.columnName}" TYPE TEXT
`);
// Migrate existing values to new values
@@ -63,7 +64,7 @@ export class WorkspaceMigrationEnumService {
queryRunner,
schemaName,
tableName,
migrationColumn.columnName,
columnDefinition.columnName,
newEnumTypeName,
);
@@ -100,20 +101,21 @@ export class WorkspaceMigrationEnumService {
tableName: string,
migrationColumn: WorkspaceMigrationColumnAlter,
) {
if (!migrationColumn.enum) {
const columnDefinition = migrationColumn.alteredColumnDefinition;
if (!columnDefinition.enum) {
return;
}
for (const enumValue of migrationColumn.enum) {
for (const enumValue of columnDefinition.enum) {
// Skip string values
if (typeof enumValue === 'string') {
continue;
}
await queryRunner.query(`
UPDATE "${schemaName}"."${tableName}"
SET "${migrationColumn.columnName}" = '${enumValue.to}'
WHERE "${migrationColumn.columnName}" = '${enumValue.from}'
SET "${columnDefinition.columnName}" = '${enumValue.to}'
WHERE "${columnDefinition.columnName}" = '${enumValue.from}'
`);
}
}
@@ -125,23 +127,25 @@ export class WorkspaceMigrationEnumService {
migrationColumn: WorkspaceMigrationColumnAlter,
enumValues: string[],
) {
const columnDefinition = migrationColumn.alteredColumnDefinition;
// Set missing values to null or default value
let defaultValue = 'NULL';
if (migrationColumn.defaultValue) {
if (Array.isArray(migrationColumn.defaultValue)) {
defaultValue = `ARRAY[${migrationColumn.defaultValue
if (columnDefinition.defaultValue) {
if (Array.isArray(columnDefinition.defaultValue)) {
defaultValue = `ARRAY[${columnDefinition.defaultValue
.map((e) => `'${e}'`)
.join(', ')}]`;
} else {
defaultValue = `'${migrationColumn.defaultValue}'`;
defaultValue = `'${columnDefinition.defaultValue}'`;
}
}
await queryRunner.query(`
UPDATE "${schemaName}"."${tableName}"
SET "${migrationColumn.columnName}" = ${defaultValue}
WHERE "${migrationColumn.columnName}" NOT IN (${enumValues
SET "${columnDefinition.columnName}" = ${defaultValue}
WHERE "${columnDefinition.columnName}" NOT IN (${enumValues
.map((e) => `'${e}'`)
.join(', ')})
`);

View File

@@ -237,7 +237,7 @@ export class WorkspaceMigrationRunnerService {
tableName: string,
migrationColumn: WorkspaceMigrationColumnAlter,
) {
const enumValues = migrationColumn.enum;
const enumValues = migrationColumn.alteredColumnDefinition.enum;
// TODO: Maybe we can do something better if we can recreate the old `TableColumn` object
if (enumValues) {
@@ -248,18 +248,32 @@ export class WorkspaceMigrationRunnerService {
tableName,
migrationColumn,
);
} else {
await queryRunner.changeColumn(
`${schemaName}.${tableName}`,
migrationColumn.columnName,
new TableColumn({
name: migrationColumn.columnName,
type: migrationColumn.columnType,
default: migrationColumn.defaultValue,
isNullable: migrationColumn.isNullable,
}),
);
}
await queryRunner.changeColumn(
`${schemaName}.${tableName}`,
new TableColumn({
name: migrationColumn.currentColumnDefinition.columnName,
type: migrationColumn.currentColumnDefinition.columnType,
default: migrationColumn.currentColumnDefinition.defaultValue,
enum: migrationColumn.currentColumnDefinition.enum?.filter(
(value): value is string => typeof value === 'string',
),
isArray: migrationColumn.currentColumnDefinition.isArray,
isNullable: migrationColumn.currentColumnDefinition.isNullable,
}),
new TableColumn({
name: migrationColumn.alteredColumnDefinition.columnName,
type: migrationColumn.alteredColumnDefinition.columnType,
default: migrationColumn.alteredColumnDefinition.defaultValue,
enum: migrationColumn.currentColumnDefinition.enum?.filter(
(value): value is string => typeof value === 'string',
),
isArray: migrationColumn.alteredColumnDefinition.isArray,
isNullable: migrationColumn.alteredColumnDefinition.isNullable,
}),
);
// }
}
private async createForeignKey(