Add support for oneOf() to if()

Fixes #1170
express-validator · Mar 11, 2023 · dbddbdd · dbddbdd
1 parent 47da322
commit dbddbdd
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 62 deletions.
diff --git a/src/chain/context-handler-impl.ts b/src/chain/context-handler-impl.ts
@@ -4,7 +4,7 @@ import { ChainCondition, CustomCondition } from '../context-items';
 import { CustomValidator } from '../base';
 import { Bail } from '../context-items/bail';
 import { ContextHandler } from './context-handler';
-import { ValidationChain } from './validation-chain';
+import { ContextRunner } from './context-runner';
 
 export class ContextHandlerImpl<Chain> implements ContextHandler<Chain> {
   constructor(private readonly builder: ContextBuilder, private readonly chain: Chain) {}
@@ -14,7 +14,7 @@ export class ContextHandlerImpl<Chain> implements ContextHandler<Chain> {
     return this.chain;
   }
 
-  if(condition: CustomValidator | ValidationChain) {
+  if(condition: CustomValidator | ContextRunner) {
     if ('run' in condition) {
       this.builder.addItem(new ChainCondition(condition));
     } else if (typeof condition === 'function') {

diff --git a/src/chain/context-handler.ts b/src/chain/context-handler.ts
@@ -1,6 +1,6 @@
 import { CustomValidator } from '../base';
 import { Optional } from '../context';
-import { ValidationChain } from './validation-chain';
+import { ContextRunner } from './context-runner';
 
 export interface ContextHandler<Chain> {
   /**
@@ -40,7 +40,7 @@ export interface ContextHandler<Chain> {
    *    .if(body('oldPassword').notEmpty())
    * @returns the current validation chain
    */
-  if(condition: CustomValidator | ValidationChain): Chain;
+  if(condition: CustomValidator | ContextRunner): Chain;
 
   /**
    * Marks the field(s) of the validation chain as optional.

diff --git a/src/context-items/chain-condition.ts b/src/context-items/chain-condition.ts
@@ -1,10 +1,10 @@
-import { ValidationChain } from '../chain';
 import { Meta, ValidationHalt } from '../base';
+import { ContextRunner } from '../chain';
 import { Context } from '../context';
 import { ContextItem } from './context-item';
 
 export class ChainCondition implements ContextItem {
-  constructor(private readonly chain: ValidationChain) {}
+  constructor(private readonly chain: ContextRunner) {}
 
   async run(_context: Context, _value: any, meta: Meta) {
     const result = await this.chain.run(meta.req, { dryRun: true });

diff --git a/src/middlewares/one-of.spec.ts b/src/middlewares/one-of.spec.ts
@@ -29,13 +29,22 @@ it('runs surrogate context created internally', done => {
   oneOf([check('bar'), check('baz')])(req, {}, () => {
     expect(spy).toHaveBeenCalledTimes(3);
     // Calls 1 and 2 asserted by previous test
-    expect(spy).toHaveBeenNthCalledWith(3, req);
+    expect(spy).toHaveBeenNthCalledWith(3, req, undefined);
 
     spy.mockRestore();
     done();
   });
 });
 
+it('runs in dry-run mode', async () => {
+  const req = {};
+  const spy = jest.spyOn(ContextRunnerImpl.prototype, 'run');
+  await oneOf([check('bar'), check('baz')]).run(req, { dryRun: true });
+  // Calls 1 and 2 asserted by previous test
+  expect(spy).toHaveBeenNthCalledWith(3, req, { dryRun: true });
+  spy.mockRestore();
+});
+
 it('passes unexpected errors down to other middlewares', done => {
   const error = new Error();
   const spy = jest.spyOn(ContextRunnerImpl.prototype, 'run').mockRejectedValue(error);

diff --git a/src/middlewares/one-of.ts b/src/middlewares/one-of.ts
@@ -1,6 +1,6 @@
 import * as _ from 'lodash';
-import { ContextRunner, ContextRunnerImpl, ResultWithContext, ValidationChain } from '../chain';
-import { AlternativeMessageFactory, InternalRequest, Middleware, Request } from '../base';
+import { ContextRunner, ContextRunnerImpl, ValidationChain } from '../chain';
+import { AlternativeMessageFactory, Middleware, Request } from '../base';
 import { ContextBuilder } from '../context-builder';
 import { ContextItem } from '../context-items';
 
@@ -58,8 +58,7 @@ export function oneOf(
   chains: (ValidationChain | ValidationChain[])[],
   options: { message?: any; errorType?: OneOfErrorType } = {},
 ): Middleware & ContextRunner {
-  let result: ResultWithContext;
-  const middleware = async (req: InternalRequest, _res: any, next: (err?: any) => void) => {
+  const run = async (req: Request, opts?: { dryRun?: boolean }) => {
     const surrogateContext = new ContextBuilder().addItem(dummyItem).build();
 
     // Run each group of chains in parallel, and within each group, run each chain in parallel too.
@@ -80,64 +79,52 @@ export function oneOf(
       return groupErrors;
     });
 
-    try {
-      const allErrors = await Promise.all(promises);
-      const success = allErrors.some(groupErrors => groupErrors.length === 0);
+    const allErrors = await Promise.all(promises);
+    const success = allErrors.some(groupErrors => groupErrors.length === 0);
 
-      if (!success) {
-        const message = options.message || 'Invalid value(s)';
-        switch (options.errorType) {
-          case 'flat':
-            surrogateContext.addError({
-              type: 'alternative',
-              req,
-              message,
-              nestedErrors: _.flatMap(allErrors),
-            });
-            break;
-          case 'leastErroredOnly':
-            let leastErroredIndex = 0;
-            for (let i = 1; i < allErrors.length; i++) {
-              if (allErrors[i].length < allErrors[leastErroredIndex].length) {
-                leastErroredIndex = i;
-              }
+    if (!success) {
+      const message = options.message || 'Invalid value(s)';
+      switch (options.errorType) {
+        case 'flat':
+          surrogateContext.addError({
+            type: 'alternative',
+            req,
+            message,
+            nestedErrors: _.flatMap(allErrors),
+          });
+          break;
+        case 'leastErroredOnly':
+          let leastErroredIndex = 0;
+          for (let i = 1; i < allErrors.length; i++) {
+            if (allErrors[i].length < allErrors[leastErroredIndex].length) {
+              leastErroredIndex = i;
             }
-            surrogateContext.addError({
-              type: 'alternative',
-              req,
-              message,
-              nestedErrors: allErrors[leastErroredIndex],
-            });
-            break;
+          }
+          surrogateContext.addError({
+            type: 'alternative',
+            req,
+            message,
+            nestedErrors: allErrors[leastErroredIndex],
+          });
+          break;
 
-          case 'grouped':
-          default:
-            // grouped
-            surrogateContext.addError({
-              type: 'alternative_grouped',
-              req,
-              message,
-              nestedErrors: allErrors,
-            });
-            break;
-        }
+        case 'grouped':
+        default:
+          // grouped
+          surrogateContext.addError({
+            type: 'alternative_grouped',
+            req,
+            message,
+            nestedErrors: allErrors,
+          });
+          break;
       }
-
-      // Final context running pass to ensure contexts are added and values are modified properly
-      result = await new ContextRunnerImpl(surrogateContext).run(req);
-      next();
-    } catch (e) {
-      next(e);
     }
-  };
 
-  const run = async (req: Request) => {
-    return new Promise<ResultWithContext>((resolve, reject) => {
-      middleware(req, {}, (e?: any) => {
-        e ? reject(e) : resolve(result);
-      });
-    });
+    // Final context running pass to ensure contexts are added and values are modified properly
+    return await new ContextRunnerImpl(surrogateContext).run(req, opts);
   };
 
+  const middleware: Middleware = (req, _res, next) => run(req).then(() => next(), next);
   return Object.assign(middleware, { run });
 }