fix: Performance issue while using plainToClassFromExist in combination with @IsOptional

[jokesterfr]

Description

While using this library, I encountered a memory issue and a blocked Node.js eventloop, which lead my server to crash a lot of times. Digging, I found out this was my usage of inheritance and the non-decorator usage of IsOptional() which lead me to this issue in combination with plainToClassFromExist() calls.

I made a complete test file to stress this out, and isolate the issue from my running code. So you can easily reproduce it yourselves:

import { Expose, plainToClassFromExist } from 'class-transformer';
import { IsNumber, IsOptional, IsString, validate } from 'class-validator';
import { v4 as uuidv4 } from 'uuid';

/**
 * Generate a payload of your requested itemCount
 *
 * @param {function} modifier function to modify the output payload
 * @param {number} items the number of items to get in the resulting array
 * @return any[]
 */
const payloadGenerator = function (
  modifier: (payload) => any,
  items: number = 10000,
) {
  modifier = modifier || ((idem) => idem);
  return new Array(items).fill(null).map((p, i) =>
    modifier({
      id: uuidv4(),
      value: i,
    }),
  );
};

class MyDto {
  @Expose()
  @IsString()
  id: string;

  @Expose()
  @IsNumber()
  value: number;

  @Expose()
  @IsString()
  extraThing: string;
}

class MyFullClass {
  @Expose()
  @IsString()
  id: string;

  @Expose()
  @IsNumber()
  value: number;

  @IsOptional()
  extraThing: string;

  constructor(data?: Partial<MyFullClass>) {
    if (data) Object.assign(this, data);
  }
}

class MyInheritedClass extends MyDto {
  constructor(data?: Partial<MyInheritedClass>) {
    super();
    IsOptional()(this, 'extraThing');
    if (data) Object.assign(this, data);
  }
}

class MyInheritedClassWithoutIsOptional extends MyDto {
  constructor(data?: Partial<MyInheritedClass>) {
    super();
    if (data) Object.assign(this, data);
  }
}

describe('test validate()', () => {
  it('[test 1] takes <3s to validate the payload with ** Simple Dto **', async () => {
    const modifier = (payload) =>
      plainToClassFromExist(new MyFullClass(), payload, {
        excludeExtraneousValues: true,
      });
    const startTime = process.hrtime();
    await Promise.all(payloadGenerator(modifier).map((p) => validate(p)));
    const diff = process.hrtime(startTime);
    console.log(`test typestack/class-transformer#1 took ${diff[0] * 1000 + diff[1] / 1000000} ms`);
    expect(diff[0]).toBeLessThanOrEqual(3);
  });

  it('[test 2] takes <3s to validate the payload of ** Inherited Dto without IsOptional **', async () => {
    const modifier = (payload) =>
      plainToClassFromExist(
        new MyInheritedClassWithoutIsOptional(),
        { ...payload, extraThing: 'thing' },
        {
          excludeExtraneousValues: true,
        },
      );
    const startTime = process.hrtime();
    await Promise.all(payloadGenerator(modifier).map((p) => validate(p)));
    const diff = process.hrtime(startTime);
    console.log(`test typestack/class-transformer#2 took ${diff[0] * 1000 + diff[1] / 1000000} ms`);
    expect(diff[0]).toBeLessThanOrEqual(0);
  });

  it('[test 3] takes <3s to validate the payload of ** Inherited Dto with IsOptional **', async () => {
    const modifier = (payload) =>
      plainToClassFromExist(new MyInheritedClass(), payload, {
        excludeExtraneousValues: true,
      });
    const startTime = process.hrtime();
    await Promise.all(payloadGenerator(modifier).map((p) => validate(p)));
    const diff = process.hrtime(startTime);
    console.log(`test typestack/class-transformer#3 took ${diff[0] * 1000 + diff[1] / 1000000} ms`);
    expect(diff[0]).toBeLessThanOrEqual(0);
  });

  it('[test 1 bis] takes <3s to validate the payload with ** Simple Dto **', async () => {
    const modifier = (payload) =>
      plainToClassFromExist(new MyFullClass(), payload, {
        excludeExtraneousValues: true,
      });
    const startTime = process.hrtime();
    await Promise.all(payloadGenerator(modifier).map((p) => validate(p)));
    const diff = process.hrtime(startTime);
    console.log(`test typestack/class-transformer#1 bis took ${diff[0] * 1000 + diff[1] / 1000000} ms`);
    expect(diff[0]).toBeLessThanOrEqual(3);
  });

  it('[test 2 bis] takes <3s to validate the payload of ** Inherited Dto without IsOptional **', async () => {
    const modifier = (payload) =>
      plainToClassFromExist(
        new MyInheritedClassWithoutIsOptional(),
        { ...payload, extraThing: 'thing' },
        {
          excludeExtraneousValues: true,
        },
      );
    const startTime = process.hrtime();
    await Promise.all(payloadGenerator(modifier).map((p) => validate(p)));
    const diff = process.hrtime(startTime);
    console.log(`test typestack/class-transformer#2 bis took ${diff[0] * 1000 + diff[1] / 1000000} ms`);
    expect(diff[0]).toBeLessThanOrEqual(0);
  });

  it('[test 3 bis] takes <3s to validate the payload of ** Inherited Dto with IsOptional **', async () => {
    const modifier = (payload) =>
      plainToClassFromExist(new MyInheritedClass(), payload, {
        excludeExtraneousValues: true,
      });
    const startTime = process.hrtime();
    await Promise.all(payloadGenerator(modifier).map((p) => validate(p)));
    const diff = process.hrtime(startTime);
    console.log(`test typestack/class-transformer#3 bis took ${diff[0] * 1000 + diff[1] / 1000000} ms`);
    expect(diff[0]).toBeLessThanOrEqual(0);
  });
});

Calling the test give these results:

  test validate()
    ✓ [test 1] takes <3s to validate the payload with ** Simple Dto ** (488 ms)
    ✓ [test 2] takes <3s to validate the payload of ** Inherited Dto without IsOptional ** (406 ms)
    ✕ [test 3] takes <3s to validate the payload of ** Inherited Dto with IsOptional ** (16080 ms)
    ✕ [test 1 bis] takes <3s to validate the payload with ** Simple Dto ** (30074 ms)
    ✕ [test 2 bis] takes <3s to validate the payload of ** Inherited Dto without IsOptional ** (27930 ms)
    ✕ [test 3 bis] takes <3s to validate the payload of ** Inherited Dto with IsOptional ** (62250 ms)

Which can be interpreted this way:

• Only test 3 use the combination of IsOptional and plainToClassFromExist, and clearly point out a failure for this usage
• Test 3 is introducing a low level issue in the node.js running process, so 1 & 2 fails next time they run
• Time to proceed the validation is logarithmic in case 3.

I tested out this with various items length:

Items in payload | 100 | 500 | 1000 | 5000 | 10000 | 50000 | 100000
-- | -- | -- | -- | -- | -- | -- | --
Direct object validation (ms) | 7 | 31 | 63 | 251 | 367 | 1389 | 7000
Inherited object validation (ms) | 13 | 212 | 374 | 4153 | 14344 | 590338 | 3632000

Hence the logarithmic graph of these measures:

Minimal code-snippet showcasing the problem

class MyDto {
  @Expose()
  @IsString()
  id: string;

  @Expose()
  @IsNumber()
  value: number;

  @Expose()
  @IsString()
  extraThing: string;
}
class MyInheritedClass extends MyDto {
  constructor(data?: Partial<MyInheritedClass>) {
    super();
    IsOptional()(this, 'extraThing');
    if (data) Object.assign(this, data);
  }
}
const o = plainToClassFromExist(new MyInheritedClass(), {id: "42", value: 72 }, {
  excludeExtraneousValues: true,
});
await o.validate(); // this works, but as the number of validate calls increase you're having serious troubles and hard time to debug it

Expected behavior

No performance difference between test 1, 2 and 3.

Actual behavior

Crashing the process after many iterations on class-validator validate() calls.

[Rush]

I just started using this library. I hope it does not contain any more surprises like this!

[jokesterfr]

This is the only edge case I had so far, same for my colleagues. This lib is pretty reliable, but for sure perfs should be carefully tested to avoid anything like this.

[NoNameProvided]

Thanks for the detailed example @jokesterfr!

[NoNameProvided]

I just started using this library. I hope it does not contain any more surprises like this!

I am sure as with any code in the world, this lib contains more surprises like this. However, feel free to send a fix for every problem you find. :)

[sualko]

@jokesterfr that's one of the nicest bug reports I have ever seen.

If someone finds this issue, I do not think that it is only related to the IsOptional decorator, because I also see the issue without using them. We use the standalone validate function and only simple validators. We are at a stage where we can't use class-validator any longer, due to this bug.

[jokesterfr]

Same here. We moved to AJV and json-schema, in cunjunction with OpenApi v3.1 spec first process. Types are generated on the fly from it.