Resolution of merge conflicts for inherited classes PR

README.md

@@ -368,7 +368,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 both from that 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 both from that 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';
@@ -726,6 +726,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,

package.json

@@ -36,30 +36,30 @@
     "test:ci": "jest --runInBand --no-cache --coverage --verbose"
   },
   "dependencies": {
-    "libphonenumber-js": "^1.9.43",
+    "libphonenumber-js": "^1.9.51",
     "validator": "^13.7.0"
   },
   "devDependencies": {
-    "@rollup/plugin-commonjs": "^21.0.1",
-    "@rollup/plugin-node-resolve": "^13.0.6",
-    "@types/jest": "^27.0.3",
-    "@types/node": "^16.11.9",
-    "@types/validator": "^13.7.0",
+    "@rollup/plugin-commonjs": "^21.0.3",
+    "@rollup/plugin-node-resolve": "^13.2.0",
+    "@types/jest": "^27.4.1",
+    "@types/node": "^17.0.24",
+    "@types/validator": "^13.7.2",
     "@typescript-eslint/eslint-plugin": "^4.33.0",
     "@typescript-eslint/parser": "^4.33.0",
     "eslint": "^7.32.0",
-    "eslint-config-prettier": "^8.3.0",
-    "eslint-plugin-jest": "^25.2.4",
+    "eslint-config-prettier": "^8.5.0",
+    "eslint-plugin-jest": "^26.1.4",
     "husky": "^4.3.8",
     "jest": "^26.6.3",
-    "lint-staged": "^12.0.3",
-    "prettier": "^2.2.1",
+    "lint-staged": "^12.3.7",
+    "prettier": "^2.6.2",
     "reflect-metadata": "0.1.13",
     "rimraf": "3.0.2",
-    "rollup": "^2.60.0",
+    "rollup": "^2.70.1",
     "rollup-plugin-terser": "^7.0.2",
     "ts-jest": "^26.5.6",
-    "ts-node": "^10.4.0",
-    "typescript": "^4.2.4"
+    "ts-node": "^10.7.0",
+    "typescript": "^4.6.3"
   }
 }

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

@@ -119,10 +119,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

@@ -9,6 +9,11 @@ export class ValidationMetadata {
   // Properties
   // -------------------------------------------------------------------------
 
+  /**
+   * Validation name. Used to uniquely identify this validator.
+   */
+  name?: string;
+
   /**
    * Validation type.
    */
@@ -69,6 +74,7 @@ export class ValidationMetadata {
   // -------------------------------------------------------------------------
 
   constructor(args: ValidationMetadataArgs) {
+    this.name = args.name;
     this.type = args.type;
     this.target = args.target;
     this.propertyName = args.propertyName;

src/metadata/ValidationMetadataArgs.ts

@@ -4,6 +4,11 @@ import { ValidationOptions } from '../decorator/ValidationOptions';
  * Constructor arguments for ValidationMetadata class.
  */
 export interface ValidationMetadataArgs {
+  /**
+   * Validation name. Used to uniquely identify this validator.
+   */
+  name?: string;
+
   /**
    * Validation type.
    */

src/register-decorator.ts

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

src/validation-schema/ValidationSchema.ts

@@ -16,6 +16,11 @@ export interface ValidationSchema {
      * Name of the object's property to be validated which holds an array of validation constraints.
      */
     [propertyName: string]: {
+      /**
+       * Validation name. Used to uniquely identify this validator.
+       */
+      name?: string;
+
       /**
        * Validation type. Should be one of the ValidationTypes value.
        */

src/validation-schema/ValidationSchemaToMetadataTransformer.ts

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

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',
+      });
+    });
+  });
 });