fix: overwrite validation decorators from parent class when defined in child

README.md

@@ -371,7 +371,7 @@ export class Post {
 
 ## Inheriting Validation decorators
 
-When you define a subclass which extends from another one, the subclass will automatically inherit the parent's decorators. If a property is redefined in the descendant, class decorators will be applied on it from both its own class and the base class.
+When you define a subclass which extends from another one, the subclass will automatically inherit the parent's decorators. If a property is redefined in the descendant, class decorators will be applied on it from both its own class and the base class. If a property-decorator pair is defined in both the sub-class and parent-class, the decorator from the sub-class will be used instead of the parent-class.
 
 ```typescript
 import { validate } from 'class-validator';
@@ -733,6 +733,7 @@ Lets create another custom validation decorator called `IsUserAlreadyExist`:
    export function IsUserAlreadyExist(validationOptions?: ValidationOptions) {
      return function (object: Object, propertyName: string) {
        registerDecorator({
+         name: 'IsUserAlreadyExist',
          target: object.constructor,
          propertyName: propertyName,
          options: validationOptions,

sample/sample6-custom-decorator/IsLongerThan.ts

@@ -7,6 +7,7 @@ import { ValidationArguments } from '../../src/validation/ValidationArguments';
 export function IsLongerThan(property: string, validationOptions?: ValidationOptions) {
   return function (object: Object, propertyName: string) {
     registerDecorator({
+      name: 'IsLongerThan',
       target: object.constructor,
       propertyName: propertyName,
       options: validationOptions,

src/decorator/common/Allow.ts

@@ -10,6 +10,7 @@ import { getMetadataStorage } from '../../metadata/MetadataStorage';
 export function Allow(validationOptions?: ValidationOptions): PropertyDecorator {
   return function (object: object, propertyName: string): void {
     const args: ValidationMetadataArgs = {
+      name: 'Allow',
       type: ValidationTypes.WHITELIST,
       target: object.constructor,
       propertyName: propertyName,

src/decorator/common/IsOptional.ts

@@ -10,6 +10,7 @@ import { getMetadataStorage } from '../../metadata/MetadataStorage';
 export function IsOptional(validationOptions?: ValidationOptions): PropertyDecorator {
   return function (object: object, propertyName: string): void {
     const args: ValidationMetadataArgs = {
+      name: 'IsOptional',
       type: ValidationTypes.CONDITIONAL_VALIDATION,
       target: object.constructor,
       propertyName: propertyName,

src/decorator/common/Validate.ts

@@ -26,6 +26,9 @@ export function ValidatorConstraint(options?: { name?: string; async?: boolean }
 /**
  * Performs validation based on the given custom validation class.
  * Validation class must be decorated with ValidatorConstraint decorator.
+ *
+ * TODO: allow passing in a `name` so the validator instance created can be uniquely identified
+ * until then, this validator will be overwritten by properties decorated with `Validate` on subclasses
  */
 export function Validate(constraintClass: Function, validationOptions?: ValidationOptions): PropertyDecorator;
 export function Validate(
@@ -40,6 +43,7 @@ export function Validate(
 ): PropertyDecorator {
   return function (object: object, propertyName: string): void {
     const args: ValidationMetadataArgs = {
+      name: 'Validate',
       type: ValidationTypes.CUSTOM_VALIDATION,
       target: object.constructor,
       propertyName: propertyName,

src/decorator/common/ValidateIf.ts

@@ -6,13 +6,17 @@ import { getMetadataStorage } from '../../metadata/MetadataStorage';
 
 /**
  * Ignores the other validators on a property when the provided condition function returns false.
+ *
+ * TODO: allow passing in a `name` so the validator instance created can be uniquely identified
+ * until then, this validator will be overwritten by properties decorated with `Validate` on subclasses
  */
 export function ValidateIf(
   condition: (object: any, value: any) => boolean,
   validationOptions?: ValidationOptions
 ): PropertyDecorator {
   return function (object: object, propertyName: string): void {
     const args: ValidationMetadataArgs = {
+      name: 'ValidateIf',
       type: ValidationTypes.CONDITIONAL_VALIDATION,
       target: object.constructor,
       propertyName: propertyName,

src/decorator/common/ValidateNested.ts

@@ -14,6 +14,7 @@ export function ValidateNested(validationOptions?: ValidationOptions): PropertyD
 
   return function (object: object, propertyName: string): void {
     const args: ValidationMetadataArgs = {
+      name: 'ValidateNested',
       type: ValidationTypes.NESTED_VALIDATION,
       target: object.constructor,
       propertyName: propertyName,

src/decorator/common/ValidatePromise.ts

@@ -10,6 +10,7 @@ import { getMetadataStorage } from '../../metadata/MetadataStorage';
 export function ValidatePromise(validationOptions?: ValidationOptions): PropertyDecorator {
   return function (object: object, propertyName: string): void {
     const args: ValidationMetadataArgs = {
+      name: 'ValidatePromise',
       type: ValidationTypes.PROMISE_VALIDATION,
       target: object.constructor,
       propertyName: propertyName,

src/metadata/MetadataStorage.ts

@@ -138,10 +138,25 @@ export class MetadataStorage {
     // filter out duplicate metadatas, prefer original metadatas instead of inherited metadatas
     const uniqueInheritedMetadatas = inheritedMetadatas.filter(inheritedMetadata => {
       return !originalMetadatas.find(originalMetadata => {
-        return (
-          originalMetadata.propertyName === inheritedMetadata.propertyName &&
-          originalMetadata.type === inheritedMetadata.type
-        );
+        // We have no clean way to determine if 2 validators are the same, and thus can't easily determine
+        // which validators have been overwritten by a subclass
+        //  - Can't use `validatorCls` object/function: it's recreated on a per-usage basis so two decorators will give different instances
+        //  - Can't use `ValidationTypes`: this was useable until 11a7b8bb59c83d55bc723ebb236fdca912f49d88,
+        //    after which 90% of ValidationTypes were removed in favour of type "customValidation". Note that
+        //    some validators, including any custom validators, still had type "customValidation" before this, and therefore
+        //    did not work with inherited validation
+        //  - `name`: can be used to uniquely identify a validator, but is optional to not break backwards compatability
+        //    in a future release, it should be made required
+        const isSameProperty = originalMetadata.propertyName === inheritedMetadata.propertyName;
+        const isSameValidator =
+          originalMetadata.name && inheritedMetadata.name
+            ? // TODO: when names becomes required, ONLY compare by name
+              originalMetadata.name === inheritedMetadata.name
+            : // 95% of decorators are of type "customValidation", despite being different decorators
+              // therefore this equality comparison introduces lots of false positives
+              originalMetadata.type === inheritedMetadata.type;
+
+        return isSameProperty && isSameValidator;
       });
     });
 

src/metadata/ValidationMetadata.ts

@@ -10,14 +10,14 @@ export class ValidationMetadata {
   // -------------------------------------------------------------------------
 
   /**
-   * Validation type.
+   * Validation name. Used to uniquely identify this validator.
    */
-  type: string;
+  name?: string;
 
   /**
-   * Validator name.
+   * Validation type.
    */
-  name?: string;
+  type: string;
 
   /**
    * Target class to which this validation is applied.
@@ -74,8 +74,8 @@ export class ValidationMetadata {
   // -------------------------------------------------------------------------
 
   constructor(args: ValidationMetadataArgs) {
-    this.type = args.type;
     this.name = args.name;
+    this.type = args.type;
     this.target = args.target;
     this.propertyName = args.propertyName;
     this.constraints = args?.constraints;

src/metadata/ValidationMetadataArgs.ts

@@ -5,14 +5,14 @@ import { ValidationOptions } from '../decorator/ValidationOptions';
  */
 export interface ValidationMetadataArgs {
   /**
-   * Validation type.
+   * Validation name. Used to uniquely identify this validator.
    */
-  type: string;
+  name?: string;
 
   /**
-   * Validator name.
+   * Validation type.
    */
-  name?: string;
+  type: string;
 
   /**
    * Object that is used to be validated.

src/register-decorator.ts

@@ -75,8 +75,8 @@ export function registerDecorator(options: ValidationDecoratorOptions): void {
   }
 
   const validationMetadataArgs: ValidationMetadataArgs = {
-    type: options.name && ValidationTypes.isValid(options.name) ? options.name : ValidationTypes.CUSTOM_VALIDATION,
     name: options.name,
+    type: options.name && ValidationTypes.isValid(options.name) ? options.name : ValidationTypes.CUSTOM_VALIDATION,
     target: options.target,
     propertyName: options.propertyName,
     validationOptions: options.options,

src/validation-schema/ValidationSchema.ts

@@ -17,14 +17,14 @@ export interface ValidationSchema {
      */
     [propertyName: string]: {
       /**
-       * Validation type. Should be one of the ValidationTypes value.
+       * Validation name. Used to uniquely identify this validator.
        */
-      type: string;
+      name?: string;
 
       /**
-       * Validator name.
+       * Validation type. Should be one of the ValidationTypes value.
        */
-      name?: string;
+      type: string;
 
       /**
        * Constraints set by validation type.

src/validation-schema/ValidationSchemaToMetadataTransformer.ts

@@ -18,8 +18,8 @@ export class ValidationSchemaToMetadataTransformer {
           each: validation.each,
         };
         const args: ValidationMetadataArgs = {
-          type: validation.type,
           name: validation.name,
+          type: validation.type,
           target: schema.name,
           propertyName: property,
           constraints: validation.constraints,

test/functional/custom-decorators.spec.ts

@@ -11,11 +11,11 @@ describe('decorator with inline validation', () => {
   function IsLongerThan(property: string, validationOptions?: ValidationOptions) {
     return function (object: object, propertyName: string): void {
       registerDecorator({
+        name: 'isLongerThan',
         target: object.constructor,
         propertyName: propertyName,
         options: validationOptions,
         constraints: [property],
-        name: 'isLongerThan',
         validator: {
           validate(value: any, args: ValidationArguments): Promise<boolean> | boolean {
             const [relatedPropertyName] = args.constraints;
@@ -109,11 +109,11 @@ describe('decorator with default message', () => {
   function IsLonger(property: string, validationOptions?: ValidationOptions) {
     return function (object: object, propertyName: string): void {
       registerDecorator({
+        name: 'isLonger',
         target: object.constructor,
         propertyName: propertyName,
         options: validationOptions,
         constraints: [property],
-        name: 'isLonger',
         validator: {
           validate(value: any, args: ValidationArguments): boolean {
             const [relatedPropertyName] = args.constraints;
@@ -183,6 +183,7 @@ describe('decorator with separate validation constraint class', () => {
   function IsShorterThan(property: string, validationOptions?: ValidationOptions) {
     return function (object: object, propertyName: string): void {
       registerDecorator({
+        name: 'IsShorterThan',
         target: object.constructor,
         propertyName: propertyName,
         options: validationOptions,

test/functional/inherited-validation.spec.ts

@@ -1,4 +1,4 @@
-import { Contains, MinLength } from '../../src/decorator/decorators';
+import { Contains, MinLength, Equals, Min } from '../../src/decorator/decorators';
 import { Validator } from '../../src/validation/Validator';
 
 const validator = new Validator();
@@ -34,4 +34,131 @@ describe('inherited validation', () => {
       expect(errors[1].value).toEqual('helo world');
     });
   });
+
+  it('should use validators from parent and child classes', () => {
+    expect.assertions(5);
+
+    class MyClass {
+      @Contains('hello')
+      title: string;
+    }
+
+    class MySubClass extends MyClass {
+      @MinLength(5)
+      title: string;
+    }
+
+    const model = new MySubClass();
+    model.title = 'helo';
+    return validator.validate(model).then(errors => {
+      expect(errors.length).toEqual(1);
+      expect(errors[0].target).toEqual(model);
+      expect(errors[0].property).toEqual('title');
+      expect(errors[0].constraints).toEqual({
+        minLength: 'title must be longer than or equal to 5 characters',
+        contains: 'title must contain a hello string',
+      });
+      expect(errors[0].value).toEqual('helo');
+    });
+  });
+
+  it('should override inherited validators in sub classes', () => {
+    expect.assertions(9);
+
+    class MyClass {
+      @Min(30)
+      age: number;
+
+      @Equals('validator')
+      first_name: string;
+
+      @Equals('class')
+      last_name: string;
+    }
+
+    class MySubClass extends MyClass {
+      @Min(40)
+      age: number;
+
+      @Equals('class')
+      first_name: string;
+
+      @Equals('validator')
+      last_name: string;
+    }
+
+    const model = new MySubClass();
+    model.age = 20; // fail validation (using sub classes constraint)
+    model.first_name = 'class'; // pass validation (overriding fail from parent)
+    model.last_name = 'class'; // fail validation (overriding pass from parent)
+
+    return validator.validate(model).then(errors => {
+      expect(errors.length).toEqual(2);
+      expect(errors[0].target).toEqual(model);
+      expect(errors[0].property).toEqual('age');
+      expect(errors[0].constraints).toEqual({
+        min: 'age must not be less than 40',
+      });
+      expect(errors[0].value).toEqual(20);
+
+      expect(errors[1].target).toEqual(model);
+      expect(errors[1].property).toEqual('last_name');
+      expect(errors[1].constraints).toEqual({
+        equals: 'last_name must be equal to validator',
+      });
+      expect(errors[1].value).toEqual('class');
+    });
+  });
+
+  it('should not override different validators of inherited properties in the parent class', () => {
+    expect.assertions(4);
+
+    class MyClass {
+      @Contains('parent-class')
+      title: string;
+    }
+
+    class MySubClass extends MyClass {
+      @Equals('sub-class')
+      title: string;
+    }
+
+    const model = new MySubClass();
+    model.title = 'sub-class';
+
+    return validator.validate(model).then(errors => {
+      expect(errors.length).toEqual(1);
+      expect(errors[0].target).toEqual(model);
+      expect(errors[0].property).toEqual('title');
+      expect(errors[0].constraints).toEqual({
+        contains: 'title must contain a parent-class string',
+      });
+    });
+  });
+
+  it('should not override different validators of inherited properties in the sub class', () => {
+    expect.assertions(4);
+
+    class MyClass {
+      @Contains('parent-class')
+      title: string;
+    }
+
+    class MySubClass extends MyClass {
+      @Equals('sub-class')
+      title: string;
+    }
+
+    const model = new MySubClass();
+    model.title = 'parent-class';
+
+    return validator.validate(model).then(errors => {
+      expect(errors.length).toEqual(1);
+      expect(errors[0].target).toEqual(model);
+      expect(errors[0].property).toEqual('title');
+      expect(errors[0].constraints).toEqual({
+        equals: 'title must be equal to sub-class',
+      });
+    });
+  });
 });

test/functional/sync-validation.spec.ts

@@ -18,12 +18,12 @@ describe('sync validation should ignore async validation constraints', () => {
   function IsLonger(property: string, validationOptions?: ValidationOptions) {
     return function (object: object, propertyName: string): void {
       registerDecorator({
+        name: 'isLonger',
         target: object.constructor,
         propertyName: propertyName,
         options: validationOptions,
         constraints: [property],
         async: true,
-        name: 'isLonger',
         validator: {
           validate(value: any, args: ValidationArguments): Promise<boolean> {
             return Promise.resolve(false);