Enhance validation error handling and improve trigger value transformation logic in lifecycle DTO

Author: tacxou

src/management/lifecycle/_dto/config.dto.ts

@@ -1,8 +1,127 @@
 import { ApiProperty } from '@nestjs/swagger';
-import { Type } from 'class-transformer';
-import { IsArray, IsEnum, IsNegative, IsNotEmpty, IsNumber, IsObject, IsOptional, ValidateNested } from 'class-validator';
+import { Type, Transform } from 'class-transformer';
+import { IsArray, IsEnum, IsNegative, IsNotEmpty, IsNumber, IsObject, IsOptional, ValidateNested, registerDecorator, ValidationOptions, ValidationArguments, isString, isNumber } from 'class-validator';
 import { IdentityLifecycle } from '~/management/identities/_enums/lifecycle.enum';
 
+/**
+ * Transform trigger values to seconds.
+ * - Numbers are interpreted as days and converted to seconds
+ * - Strings with 'd' suffix are interpreted as days and converted to seconds
+ * - Strings with 'm' suffix are interpreted as minutes and converted to seconds
+ * - Strings with 's' suffix are already in seconds
+ *
+ * @param value The trigger value to transform
+ * @returns The value converted to seconds
+ */
+function transformTriggerToSeconds(value: number | string): number | undefined {
+  let isValid = false;
+
+  if (value === undefined || value === null) {
+    return undefined;
+  }
+
+  /**
+   * Check if the value is a negative number.
+   * If it's a number, we check if it's less than 0.
+   * If it's a string, we check if it matches the regex for negative time strings.
+   */
+  if (isNumber(value)) {
+    isValid = value < 0;
+  } else if (isString(value)) {
+    const timeRegex = /^-?\d+[dms]$/;
+    if (timeRegex.test(value)) {
+      // Extract the number part and check if it's negative
+      const numberPart = value.replace(/[dms]$/, '');
+      const num = parseInt(numberPart, 10);
+      isValid = num < 0;
+    }
+  }
+
+  if (!isValid) {
+    throw new Error('Trigger must be a negative number (days) or a negative time string with units (e.g., "-90d", "-10m", "-45s")');
+  }
+
+  /**
+   * If the value is a number, we assume it's in days and convert it to seconds.
+   * We multiply by 24 (hours) * 60 (minutes) * 60 (seconds) to get the total seconds.
+   * This conversion preserves the sign of the number,
+   * so if the input is negative, the output will also be negative.
+   */
+  if (isNumber(value)) {
+    return value * 24 * 60 * 60; // Convert days to seconds, preserving sign
+  }
+
+  /**
+   * If the value is a string, we check if it matches the regex for negative time strings.
+   * If it does, we extract the number and unit, then convert it to seconds.
+   * - 'd' is converted to seconds by multiplying by 24 * 60 * 60
+   * - 'm' is converted to seconds by multiplying by 60
+   * - 's' is already in seconds
+   * This conversion preserves the sign of the number,
+   * so if the input is negative, the output will also be negative.
+   */
+  if (isString(value)) {
+    const match = value.match(/^(-?\d+)([dms])$/);
+    if (match) {
+      const numValue = parseInt(match[1], 10);
+      const unit = match[2];
+
+      switch (unit) {
+        case 'd': // days
+          return numValue * 24 * 60 * 60;
+
+        case 'm': // minutes
+          return numValue * 60;
+
+        case 's': // seconds
+          return numValue;
+
+        default:
+          throw new Error(`Unsupported time unit: ${unit}`);
+      }
+    }
+  }
+
+  // If we can't parse it, try to convert to number
+  return Number(value) || undefined;
+}
+
+/**
+ * Custom decorator to validate that at least one of the properties 'rules' or 'trigger' is defined and not empty.
+ * This decorator can be applied to a class to enforce this validation rule.
+ *
+ * @param validationOptions
+ * @returns
+ */
+function ValidateRulesOrTrigger(validationOptions?: ValidationOptions) {
+  return function (constructor: Function) {
+    registerDecorator({
+      name: 'validateRulesOrTrigger',
+      target: constructor,
+      propertyName: undefined,
+      options: validationOptions,
+      validator: {
+        validate(_: any, args: ValidationArguments) {
+          const obj = args.object as ConfigObjectIdentitiesDTO;
+
+          /**
+           * Check if either 'rules' or 'trigger' is defined and not empty.
+           * 'rules' should be an object with at least one key-value pair,
+           * and 'trigger' should be a number that is not null.
+           */
+          const hasRules = obj.rules !== undefined && obj.rules !== null && (typeof obj.rules === 'object' && Object.keys(obj.rules).length > 0);
+          const hasTrigger = obj.trigger !== undefined && obj.trigger !== null;
+          return hasRules || hasTrigger;
+        },
+        defaultMessage(_: ValidationArguments) {
+          return 'Either rules or trigger must be provided';
+        }
+      }
+    });
+  };
+}
+
+@ValidateRulesOrTrigger({ message: 'Either rules or trigger must be provided' })
 export class ConfigObjectIdentitiesDTO {
   @IsEnum(IdentityLifecycle, { each: true })
   @ApiProperty({
@@ -19,10 +138,17 @@ export class ConfigObjectIdentitiesDTO {
   public rules: object;
 
   @IsOptional()
+  @Transform(({ value }) => transformTriggerToSeconds(value))
   @IsNumber()
-  @IsNegative()
-  @Type(() => Number)
-  @ApiProperty({ type: Number, required: false })
+  @ApiProperty({
+    oneOf: [
+      { type: 'number', description: 'Negative number representing days' },
+      { type: 'string', description: 'Negative time string with units (d=days, m=minutes, s=seconds)' }
+    ],
+    required: false,
+    description: 'Trigger time as negative number (days) or negative time string with units (converted to negative seconds internally)',
+    examples: [-90, '-90d', '-10m', '-45s']
+  })
   public trigger: number;
 
   @IsNotEmpty()

src/management/lifecycle/lifecycle.service.ts

@@ -10,7 +10,7 @@ import { readdirSync, readFileSync, writeFileSync } from 'node:fs';
 import { parse } from 'yaml';
 import { plainToInstance } from 'class-transformer';
 import { ConfigObjectIdentitiesDTO, ConfigObjectSchemaDTO } from './_dto/config.dto';
-import { validateOrReject } from 'class-validator';
+import { validateOrReject, ValidationError } from 'class-validator';
 import { omit } from 'radash';
 import { IdentitiesCrudService } from '../identities/identities-crud.service';
 
@@ -144,9 +144,9 @@ export class LifecycleService extends AbstractServiceSchema implements OnApplica
         })
         this.logger.debug(`Validated schema for file: ${file}`);
       } catch (errors) {
-        const err = new Error(`Validation failed for schema in file: ${file}`);
-        err.message = errors.map((e) => e.toString()).join(', ') //TODO: improve error message
-        throw err
+        const formattedErrors = this.formatValidationErrors(errors, file);
+        const err = new Error(`Validation errors in file '${file}':\n${formattedErrors}`);
+        throw err;
       }
 
       lifecycleRules.push(schema);
@@ -157,6 +157,69 @@ export class LifecycleService extends AbstractServiceSchema implements OnApplica
     return lifecycleRules;
   }
 
+  /**
+   * Format validation errors for better readability
+   *
+   * @param errors - Array of ValidationError objects from class-validator
+   * @param file - The file name where the validation failed
+   * @returns A formatted error message string
+   */
+  private formatValidationErrors(errors: ValidationError[], file: string, basePath: string = '', isInArrayContext: boolean = false): string {
+    const formatError = (error: ValidationError, currentPath: string, inArrayContext: boolean): string[] => {
+      let propertyPath = currentPath;
+
+      /**
+       * Check if error.property is defined, not null, not empty, and not the string 'undefined'.
+       * If it is, we construct the property path based on whether we are in an array context or not.
+       * If it is an array context, we use the index notation; otherwise, we use dot notation.
+       */
+      if (error.property !== undefined &&
+        error.property !== null &&
+        error.property !== '' &&
+        error.property !== 'undefined') {
+        if (inArrayContext && !isNaN(Number(error.property))) {
+          // C'est un index d'array
+          propertyPath = currentPath ? `${currentPath}[${error.property}]` : `[${error.property}]`;
+        } else {
+          // C'est une propriété normale
+          propertyPath = currentPath ? `${currentPath}.${error.property}` : error.property;
+        }
+      }
+
+      const errorMessages: string[] = [];
+
+      /**
+       * Check if error.constraints is defined and not empty.
+       * If it is, we iterate over each constraint and format the error message.
+       */
+      if (error.constraints) {
+        Object.entries(error.constraints).forEach(([constraintKey, message]) => {
+          errorMessages.push(`Property '${propertyPath}': ${message} (constraint: ${constraintKey})`);
+        });
+      }
+
+      /**
+       * If the error has children, we recursively format each child error.
+       * We check if the error has children and if they are defined.
+       */
+      if (error.children && error.children.length > 0) {
+        const isNextLevelArray = Array.isArray(error.value);
+        error.children.forEach(childError => {
+          errorMessages.push(...formatError(childError, propertyPath, isNextLevelArray));
+        });
+      }
+
+      return errorMessages;
+    };
+
+    const allErrorMessages: string[] = [];
+    errors.forEach(error => {
+      allErrorMessages.push(...formatError(error, basePath, isInArrayContext));
+    });
+
+    return allErrorMessages.map(msg => `• ${msg}`).join('\n');
+  }
+
   /**
    * Handle identity update events
    * This method listens for events emitted after an identity is updated.