Skip to content

Commit cc7a3db

Browse files
committed
perf: fix issues detected in pipeline
1 parent 2f5154b commit cc7a3db

10 files changed

Lines changed: 503 additions & 28 deletions

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
"typescript"
2121
],
2222
"scripts": {
23+
"benchmark": "ts-node --transpile-only test/benchmark.ts",
2324
"build": "npm run build:cjs",
2425
"build:clean": "rimraf build",
2526
"build:es2015": "tsc --project tsconfig.prod.esm2015.json",

src/decorator/object/IsNotEmptyObject.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ export function isNotEmptyObject(value: unknown, options?: { nullable?: boolean
1414
}
1515

1616
if (options?.nullable === false) {
17-
for (const key in value as object) {
18-
if ((value as object).hasOwnProperty(key)) {
17+
for (const key in value ) {
18+
if ((value ).hasOwnProperty(key)) {
1919
const propertyValue = (value as any)[key];
2020
if (propertyValue !== null && propertyValue !== undefined) {
2121
return true;

src/metadata/MetadataStorage.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ export class MetadataStorage {
148148
}
149149
}
150150

151-
const customOnly = defined.length === 0 && nested.length === 0 && conditional.length === 0 && !hasPromiseValidation;
151+
const customOnly =
152+
defined.length === 0 && nested.length === 0 && conditional.length === 0 && !hasPromiseValidation;
152153
result[propertyName] = { defined, custom, nested, conditional, all, hasPromiseValidation, customOnly };
153154
}
154155

@@ -159,13 +160,7 @@ export class MetadataStorage {
159160
/**
160161
* Gets all validation metadatas for the given object with the given groups.
161162
*/
162-
buildCacheKey(
163-
target: Function,
164-
schema: string,
165-
always: boolean,
166-
strictGroups: boolean,
167-
groups?: string[]
168-
): string {
163+
buildCacheKey(target: Function, schema: string, always: boolean, strictGroups: boolean, groups?: string[]): string {
169164
const targetId = (target as any).__cv_id ?? ((target as any).__cv_id = ++MetadataStorage._nextId);
170165
const groupKey = groups?.length ? groups.slice().sort().join(',') : '';
171166
return `${targetId}|${schema || ''}|${always ? 1 : 0}|${strictGroups ? 1 : 0}|${groupKey}`;

src/metadata/ValidationMetadata.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ export class ValidationMetadata {
9090
*/
9191
inlineDefaultMessage?: (args?: any) => string;
9292

93-
9493
// -------------------------------------------------------------------------
9594
// Constructor
9695
// -------------------------------------------------------------------------

src/validation/ValidationExecutor.ts

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ export class ValidationExecutor {
7171
this.strictGroups = (validatorOptions && validatorOptions.strictGroups) || false;
7272
this.always = (validatorOptions && validatorOptions.always) || false;
7373
this.forbidUnknownValues =
74-
!validatorOptions || validatorOptions.forbidUnknownValues === undefined || validatorOptions.forbidUnknownValues !== false;
74+
!validatorOptions ||
75+
validatorOptions.forbidUnknownValues === undefined ||
76+
validatorOptions.forbidUnknownValues !== false;
7577
this.whitelist = (validatorOptions && validatorOptions.whitelist) || false;
7678
this.forbidNonWhitelisted = (validatorOptions && validatorOptions.forbidNonWhitelisted) || false;
7779
}
@@ -95,7 +97,13 @@ export class ValidationExecutor {
9597
);
9698
}
9799

98-
const cacheKey = this.metadataStorage.buildCacheKey(object.constructor, targetSchema, this.always, this.strictGroups, this.groups);
100+
const cacheKey = this.metadataStorage.buildCacheKey(
101+
object.constructor,
102+
targetSchema,
103+
this.always,
104+
this.strictGroups,
105+
this.groups
106+
);
99107
const targetMetadatas = this.metadataStorage.getTargetValidationMetadatas(
100108
object.constructor,
101109
targetSchema,
@@ -216,19 +224,25 @@ export class ValidationExecutor {
216224
for (const metadata of metadatas) {
217225
validationArguments.constraints = metadata.constraints;
218226

219-
if (metadata.inlineValidate && (!metadata.each || !(Array.isArray(value) || value instanceof Set || value instanceof Map))) {
227+
if (
228+
metadata.inlineValidate &&
229+
(!metadata.each || !(Array.isArray(value) || value instanceof Set || value instanceof Map))
230+
) {
220231
if (this.stopAtFirstError && validationError && hasConstraints(validationError.constraints)) continue;
221232

222233
const validatedValue = metadata.inlineValidate(value, validationArguments);
223234
if (validatedValue !== true && validatedValue !== false) {
224-
const promise = (validatedValue as Promise<boolean>).then(isValid => {
235+
const promise = (validatedValue ).then(isValid => {
225236
if (!isValid) {
226237
if (!validationError) validationError = this.generateValidationError(object, value, propertyName);
227238
const [type, message] = this.createValidationErrorInline(metadata, validationArguments);
228239
validationError.constraints[type] = message;
229240
if (metadata.context) {
230241
if (!validationError.contexts) validationError.contexts = {};
231-
validationError.contexts[type] = Object.assign(validationError.contexts[type] || {}, metadata.context);
242+
validationError.contexts[type] = Object.assign(
243+
validationError.contexts[type] || {},
244+
metadata.context
245+
);
232246
}
233247
}
234248
});
@@ -255,7 +269,11 @@ export class ValidationExecutor {
255269

256270
if (validationError) {
257271
const hasAsyncPending = this.awaitingPromises.length > asyncCountBefore;
258-
if (hasConstraints(validationError.constraints) || (validationError.children && validationError.children.length > 0) || hasAsyncPending) {
272+
if (
273+
hasConstraints(validationError.constraints) ||
274+
(validationError.children && validationError.children.length > 0) ||
275+
hasAsyncPending
276+
) {
259277
validationErrors.push(validationError);
260278
}
261279
} else if (this.awaitingPromises.length > asyncCountBefore) {
@@ -380,7 +398,10 @@ export class ValidationExecutor {
380398
if (metadata.validateIf && !metadata.validateIf(object, value)) continue;
381399

382400
// Fast path: inline validators (all built-in decorators) bypass constraint metadata dispatch
383-
if (metadata.inlineValidate && (!metadata.each || !(Array.isArray(value) || value instanceof Set || value instanceof Map))) {
401+
if (
402+
metadata.inlineValidate &&
403+
(!metadata.each || !(Array.isArray(value) || value instanceof Set || value instanceof Map))
404+
) {
384405
if (this.stopAtFirstError) {
385406
const error = getError();
386407
if (hasConstraints(error.constraints)) continue;
@@ -389,7 +410,7 @@ export class ValidationExecutor {
389410
const validatedValue = metadata.inlineValidate(value, validationArguments);
390411
if (validatedValue !== true && validatedValue !== false) {
391412
// Async result (Promise)
392-
const promise = (validatedValue as Promise<boolean>).then(isValid => {
413+
const promise = (validatedValue ).then(isValid => {
393414
if (!isValid) {
394415
const error = getError();
395416
const [type, message] = this.createValidationErrorInline(metadata, validationArguments);
@@ -417,7 +438,8 @@ export class ValidationExecutor {
417438
continue;
418439
}
419440

420-
const constraintMetadatas = metadata.resolvedConstraints ??
441+
const constraintMetadatas =
442+
metadata.resolvedConstraints ??
421443
(metadata.resolvedConstraints = this.metadataStorage.getTargetValidatorConstraints(metadata.constraintCls));
422444
for (const customConstraintMetadata of constraintMetadatas) {
423445
if (customConstraintMetadata.async && this.ignoreAsyncValidations) continue;
@@ -509,10 +531,7 @@ export class ValidationExecutor {
509531
for (const metadata of metadatas) {
510532
if (metadata.type !== ValidationTypes.NESTED_VALIDATION && metadata.type !== ValidationTypes.PROMISE_VALIDATION) {
511533
continue;
512-
} else if (
513-
this.stopAtFirstError &&
514-
Object.keys(error.constraints || {}).length > 0
515-
) {
534+
} else if (this.stopAtFirstError && Object.keys(error.constraints || {}).length > 0) {
516535
continue;
517536
}
518537

@@ -550,9 +569,12 @@ export class ValidationExecutor {
550569
// Inline validators: use metadata.name directly
551570
type = metadata.name || metadata.type;
552571
} else {
553-
const customConstraints = metadata.resolvedConstraints ??
554-
(metadata.resolvedConstraints = this.metadataStorage.getTargetValidatorConstraints(metadata.constraintCls));
555-
type = (customConstraints[0] && customConstraints[0].name) ? customConstraints[0].name : metadata.type;
572+
const customConstraints =
573+
metadata.resolvedConstraints ??
574+
(metadata.resolvedConstraints = this.metadataStorage.getTargetValidatorConstraints(
575+
metadata.constraintCls
576+
));
577+
type = customConstraints[0] && customConstraints[0].name ? customConstraints[0].name : metadata.type;
556578
}
557579
} else {
558580
type = metadata.type;

benchmark.ts renamed to test/benchmark.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,21 @@
1+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
2+
// @ts-nocheck
13
import 'reflect-metadata';
2-
import { validate, validateSync, IsString, IsInt, IsBoolean, IsEmail, IsOptional, MinLength, MaxLength, Min, Max, IsNotEmpty, ValidateNested } from './src';
4+
import {
5+
validate,
6+
validateSync,
7+
IsString,
8+
IsInt,
9+
IsBoolean,
10+
IsEmail,
11+
IsOptional,
12+
MinLength,
13+
MaxLength,
14+
Min,
15+
Max,
16+
IsNotEmpty,
17+
ValidateNested,
18+
} from '../src';
319

420
// --- Classes with inheritance and nesting ---
521
class BaseEntity {

test/functional/custom-decorators.spec.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,3 +274,61 @@ describe('decorator with symbol constraint', () => {
274274
});
275275
});
276276
});
277+
278+
describe('inline custom decorator fast-path behavior', () => {
279+
function InlineFailing(name: string, validationOptions?: ValidationOptions) {
280+
return function (object: object, propertyName: string): void {
281+
registerDecorator({
282+
target: object.constructor,
283+
propertyName,
284+
name,
285+
options: validationOptions,
286+
validator: {
287+
validate(): boolean {
288+
return false;
289+
},
290+
},
291+
});
292+
};
293+
}
294+
295+
function InlineAsyncPassing(name: string, validationOptions?: ValidationOptions) {
296+
return function (object: object, propertyName: string): void {
297+
registerDecorator({
298+
target: object.constructor,
299+
propertyName,
300+
name,
301+
options: validationOptions,
302+
validator: {
303+
validate(): Promise<boolean> {
304+
return Promise.resolve(true);
305+
},
306+
},
307+
});
308+
};
309+
}
310+
311+
it('should stop after first inline custom validator failure when stopAtFirstError is enabled', () => {
312+
class StopAtFirstErrorModel {
313+
@InlineFailing('firstError', { message: 'first message' })
314+
@InlineFailing('secondError', { message: 'second message' })
315+
value: string = 'x';
316+
}
317+
318+
return validator.validate(new StopAtFirstErrorModel(), { stopAtFirstError: true }).then(errors => {
319+
expect(errors.length).toEqual(1);
320+
expect(Object.keys(errors[0].constraints).length).toEqual(1);
321+
});
322+
});
323+
324+
it('should not create a visible error when inline async validator resolves true', () => {
325+
class AsyncPassModel {
326+
@InlineAsyncPassing('asyncPass')
327+
value: string = 'x';
328+
}
329+
330+
return validator.validate(new AsyncPassModel()).then(errors => {
331+
expect(errors.length).toEqual(0);
332+
});
333+
});
334+
});
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { MetadataStorage } from '../../src/metadata/MetadataStorage';
2+
import { ValidationMetadata } from '../../src/metadata/ValidationMetadata';
3+
import { ValidationTypes } from '../../src/validation/ValidationTypes';
4+
5+
describe('MetadataStorage PR-2665 coverage', () => {
6+
class TestTarget {}
7+
8+
function createMetadata(type: string, propertyName: string): ValidationMetadata {
9+
return new ValidationMetadata({
10+
type,
11+
target: TestTarget,
12+
propertyName,
13+
});
14+
}
15+
16+
it('should build cache keys independent from group order', () => {
17+
const storage = new MetadataStorage();
18+
19+
const first = storage.buildCacheKey(TestTarget, '', false, false, ['beta', 'alpha']);
20+
const second = storage.buildCacheKey(TestTarget, '', false, false, ['alpha', 'beta']);
21+
const strict = storage.buildCacheKey(TestTarget, '', false, true, ['alpha', 'beta']);
22+
23+
expect(first).toEqual(second);
24+
expect(first).not.toEqual(strict);
25+
});
26+
27+
it('should partition metadata and compute customOnly accurately', () => {
28+
const storage = new MetadataStorage();
29+
const cacheKey = storage.buildCacheKey(TestTarget, '', false, false, undefined);
30+
const grouped = {
31+
onlyCustom: [createMetadata(ValidationTypes.CUSTOM_VALIDATION, 'onlyCustom')],
32+
withPromise: [
33+
createMetadata(ValidationTypes.CUSTOM_VALIDATION, 'withPromise'),
34+
createMetadata(ValidationTypes.PROMISE_VALIDATION, 'withPromise'),
35+
],
36+
withNestedAndConditional: [
37+
createMetadata(ValidationTypes.CUSTOM_VALIDATION, 'withNestedAndConditional'),
38+
createMetadata(ValidationTypes.NESTED_VALIDATION, 'withNestedAndConditional'),
39+
createMetadata(ValidationTypes.CONDITIONAL_VALIDATION, 'withNestedAndConditional'),
40+
],
41+
withDefined: [
42+
createMetadata(ValidationTypes.IS_DEFINED, 'withDefined'),
43+
createMetadata(ValidationTypes.CUSTOM_VALIDATION, 'withDefined'),
44+
],
45+
withWhitelist: [
46+
createMetadata(ValidationTypes.WHITELIST, 'withWhitelist'),
47+
createMetadata(ValidationTypes.CUSTOM_VALIDATION, 'withWhitelist'),
48+
],
49+
};
50+
51+
const partitioned = storage.getPartitionedMetadata(grouped, cacheKey);
52+
const cached = storage.getPartitionedMetadata(grouped, cacheKey);
53+
54+
expect(cached).toBe(partitioned);
55+
expect(partitioned.onlyCustom.customOnly).toEqual(true);
56+
expect(partitioned.withPromise.customOnly).toEqual(false);
57+
expect(partitioned.withPromise.hasPromiseValidation).toEqual(true);
58+
expect(partitioned.withNestedAndConditional.customOnly).toEqual(false);
59+
expect(partitioned.withDefined.customOnly).toEqual(false);
60+
expect(partitioned.withDefined.defined.length).toEqual(1);
61+
expect(partitioned.withWhitelist.all.length).toEqual(1);
62+
});
63+
64+
it('should apply always/strictGroups metadata filtering consistently', () => {
65+
class GroupedTarget {}
66+
const storage = new MetadataStorage();
67+
const plain = new ValidationMetadata({
68+
type: ValidationTypes.CUSTOM_VALIDATION,
69+
target: GroupedTarget,
70+
propertyName: 'plain',
71+
});
72+
const grouped = new ValidationMetadata({
73+
type: ValidationTypes.CUSTOM_VALIDATION,
74+
target: GroupedTarget,
75+
propertyName: 'grouped',
76+
});
77+
grouped.groups = ['g1'];
78+
const explicitAlways = new ValidationMetadata({
79+
type: ValidationTypes.CUSTOM_VALIDATION,
80+
target: GroupedTarget,
81+
propertyName: 'always',
82+
});
83+
explicitAlways.groups = ['g2'];
84+
explicitAlways.always = true;
85+
86+
storage.addValidationMetadata(plain);
87+
storage.addValidationMetadata(grouped);
88+
storage.addValidationMetadata(explicitAlways);
89+
90+
const strictNoGroups = storage.getTargetValidationMetadatas(GroupedTarget, '', false, true);
91+
const strictProperties = strictNoGroups.map(metadata => metadata.propertyName);
92+
93+
expect(strictProperties).toContain('plain');
94+
expect(strictProperties).toContain('always');
95+
expect(strictProperties).not.toContain('grouped');
96+
});
97+
});

0 commit comments

Comments
 (0)