Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

question: validate objects as if they're maps #1378

Open
flisboac opened this issue Nov 2, 2021 · 2 comments · May be fixed by #2066
Open

question: validate objects as if they're maps #1378

flisboac opened this issue Nov 2, 2021 · 2 comments · May be fixed by #2066
Labels
type: question Questions about the usage of the library.

Comments

@flisboac
Copy link

flisboac commented Nov 2, 2021

I was trying to...
Validate objects as if they're maps.

For the same reasons discussed in #447 (i.e. lack of Map in my specific environment).

The problem:

Based on one of the examples in the original issue (#447). Would this work?

class B {
  @IsString()
  @IsNotEmpty()
  name: string;
}

class A {
  @IsNotEmpty()
  @ValidateNested({each: true})
  @Type(() => B)
  aMap: Record<string, B>; //< Note the use of Record instead of Map!
}

Also, if anyone knows (and I know this is class-transformer stuff), and considering I do use class-transformer to instantiate the whole plain object into a class instance: does the @Type there properly converts all object values to their corresponding B instances? (As I understand, being an instance of a class is a precondition for validation (including nested validation) to work)

Another example (based on the previous, to clarify my use case):

const construct = async <T>(Cls: Constructor<T>, plain: Record<string, any>): T => {
  // Assuming all values of `plain` will be converted to instances of `B`...
  const obj = plainToClass(Cls, plain);

  // Does this validate correctly?
  const violations = await validate(obj);

  if (violations.length > 0) {
    throw new Error('Validation error!');
  }
  
  return obj;
};

//
// Then, somewhere else...
const obj = await construct(A, {
  aMap: {
    ABCD: { name: 'MyName' },
  },
});
@flisboac flisboac added the type: question Questions about the usage of the library. label Nov 2, 2021
@flisboac
Copy link
Author

flisboac commented Nov 2, 2021

Well, by the looks of this, what I initially proposed won't work.

if (value instanceof Array || value instanceof Set || value instanceof Map) {
// Treats Set as an array - as index of Set value is value itself and it is common case to have Object as value
const arrayLikeValue = value instanceof Set ? Array.from(value) : value;
arrayLikeValue.forEach((subValue: any, index: any) => {
this.performValidations(value, subValue, index.toString(), [], metadatas, errors);
});
} else if (value instanceof Object) {
const targetSchema = typeof metadata.target === 'string' ? metadata.target : metadata.target.name;
this.execute(value, targetSchema, errors);
} else {

Because a plain objects are just instanceof Object; and by the Executor's logic, it will be assumed that the value is a class instance, which is not the case (i.e. the values DO have a specific class constructor, unlike the "plain map/record" itself, which is, well, just new Object, effectively.


What about extending the definition of @ValidateNested to contemplate what I'm now calling plain maps (or, more in line with what TypeScript provides, records)? Please, consider the following suggestion:

In ValidationOptions.ts we could have a new option:

export interface ValidationOptions {
  /**
   * Indicates that an object is to be considered a plain record.
   * 
   * For an object-valued property marked as plain, the object the property holds may neither
   * be specifically class-typed nor validated, but all of the child values of said object MUST be.
   * Effectively, this declares a "plain record", which will be validated the same way any other 
   * JavaScript collection does (Array, Map, Set, etc).
   * The default is `false`; that is, an object-value must be an instance of a class.
   */
  plain?: boolean;
}

Then, in ValidationExecutor.ts we can introduce a different code path for when a property is plain-typed:

export class ValidationExecutor {

  // Not changed
  // ...

  private nestedValidations(value: any, metadatas: ValidationMetadata[], errors: ValidationError[]): void {
      // Not changed
      // ...

      // At line 346
      if (value instanceof Array || value instanceof Set || value instanceof Map) {
        // Not changed
        //  ...
      } else if (value instanceof Object) {
        if (metadata.validationOptions.plain) {
          // Here, we do essentially what would be done with a Map-typed property
          Object.values(value).forEach((subValue: any, index: any) => {
            this.performValidations(value, subValue, index.toString(), [], metadatas, errors);
          });
        } else {
          const targetSchema = typeof metadata.target === 'string' ? metadata.target : metadata.target.name;
          this.execute(value, targetSchema, errors);
        }
      } else {
        // Not changed
        //  ...
      }
    });
  }

I suggest this change because it's not easy to map this "map" concept in terms of classes. The idiom { [K: string]: unknown } means nothing at runtime, and it's perhaps impossible to enumerate all possible keys for such a mapping in all cases. class-validator should be prepared to handle said cases. The lack of a Map implementation only aggravates this problem, but even if one is present, the transformation may be somewhat unnecessary, if not detrimental (in terms of processing time, and development time (because you need to manually convert it all)).

christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 27, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 27, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 27, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 27, 2023
@christian-forgacs christian-forgacs linked a pull request Apr 27, 2023 that will close this issue
6 tasks
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 28, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 28, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 28, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 28, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 28, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 28, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 28, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 28, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 28, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Apr 29, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue May 2, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue May 3, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue May 12, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue May 16, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Jun 21, 2023
@proudman
Copy link

proudman commented Jun 29, 2023

Hi, here is my solution with constraint

import {
    ValidatorConstraint,
    ValidatorConstraintInterface,
    validate,
    ValidationArguments,
    ValidationError,
} from 'class-validator';
import {
    ClassConstructor,
    plainToClass,
} from 'class-transformer';

@ValidatorConstraint({ async: true })
export class RecordValidator<T extends Record<string, unknown>> implements ValidatorConstraintInterface {
    validationErrors: ValidationError[][] = [];

    async validate(record: Record<string, T>, validationArguments: ValidationArguments) {
        const ClassTransformer = validationArguments.constraints?.[0] as ClassConstructor<T>;

        if (!ClassTransformer) {
            return false;
        }

        const values = Object.values(record);

        this.validationErrors = await Promise.all(values.map((value) => {
            const transformedValue = plainToClass(ClassTransformer, value);

            return validate(transformedValue);
        }));

        return this.validationErrors.every((validationError) => isEmpty(validationError));
    }

    defaultMessage() {
        return this.validationErrors.map(
            (errors) => errors.reduce(
                (acc, error) => error.constraints ? [...acc, ...Object.values(error.constraints)] : acc,
                [],
            ).join(', '),
        ).join(', ');
    }
}

And usage like this:

class  ButtonDto {
    @IsString()
    id: sting;

    @IsOptional()
    @IsString()
    name?: string;
}

export class SomeDto {
    @Validate(RecordValidator, [ButtonDto])
    buttons: Record<string, ButtonDto>;
}

christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Jul 27, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Sep 14, 2023
christian-forgacs pushed a commit to christian-forgacs/class-validator that referenced this issue Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question Questions about the usage of the library.
Development

Successfully merging a pull request may close this issue.

2 participants