[inversify-express-utils] controllerMethod metadata should be more generally extendable

[bowd]

At the moment when defining a httpMethod in the controller the metadata is pushed onto an array. This works fine for most cases but it provides two disadvantages in my opinion:

1. If we want to add custom decorators that alter the metadata (for example by adding a middleware) we need to first make sure our custom decorator is above the httpMethod one, and then we need to look inside the list for a matching key. The only problem with this is readability, I'd like the httpMethod decorator to be at the top personally.

Also the metadata reflect keys aren't exported which makes this a bit of a hack now.

2. It is impossible to alter the metadata from a method parameter decorator, because these are evaluated before the method decorator and thus no metadata exists at the time.

Skip to context for a better understanding of my usecase.

Expected Behavior / Possible Solution

Maybe the controller method metadata should be an object indexed by key instead of an array (just like the method parameters metadata created by the parameter decorators). And more importantly the httpMethod decorator should merge with any existing metadata for that method.

If my proposal makes sense I'm happy to hammer out a PR

Current Behavior

Covered in the summary. Currently you need to jump through a few loopholes to extend the metadata when using method decorators, and it's virtually impossible from the method param decorators.

Context

What I'm trying to accomplish is to validate request payloads using class-validator. Request payloads can be either params, body or query. The way I'm validating is by unshifting a middleware that checks the payload against a class definition and replaces the payload on the request object with the class instance or executes next with the errors found.

Here is some example code and usage:

// Decorator definition

export function withParams<PayloadType>(type: Constructor<PayloadType>) {
  return withPayloadType(type, PayloadSource.params);
}

export function withBody<PayloadType>(type: Constructor<PayloadType>) {
  return withPayloadType(type, PayloadSource.body);
}

export function withQuery<PayloadType>(type: Constructor<PayloadType>) {
  return withPayloadType(type, PayloadSource.query);
}

function withPayloadType<PayloadType>(
  type: Constructor<PayloadType>,
  source: PayloadSource
) {
  return function(target: any, key: string, value: any) {
    let metadataList: interfaces.ControllerMethodMetadata[] = Reflect.getMetadata(
      // https://github.com/inversify/inversify-express-utils/blob/master/src/constants.ts
      "inversify-express-utils:controller-method",
      target.constructor
    );
    let methodMetadata = metadataList.find(metadata => metadata.key === key);
    if (methodMetadata === undefined) {
      throw `Could not find method definition for ${key} on ${
        target.constructor.name
      } ` + `when defining ${source} type. Check the order of decorators.`;
    } else {
      let validateMiddleware = validate(type, source);
      methodMetadata.middleware.unshift(validateMiddleware);
    }
  };
}

let validator = new Validator();
export function validate<PayloadType>(
  type: Constructor<PayloadType>,
  source: PayloadSource
): express.RequestHandler {
  return (req: express.Request, res: express.Response, next) => {
    let payload = plainToClass(type, req[source]);
    let errors = validator.validateSync(payload);
    if (errors.length > 0) {
      next(errors);
    } else {
      req[source] = payload;
      next();
    }
  };
}

// Usage in Controller

export class GetByIdParams {
  @IsNumberString()
  id: string;

  get ID() {
    return parseInt(this.id, 10);
  }
}

@controller("/people")
export class PeopleController {
  constructor(@inject("PeopleService") private peopleService: PeopleService) {}

  @withParams(GetByIdParams)
  @httpGet("/:id")
  private async handleGet(
    @requestParam() params: GetByIdParams,
    res: Response
  ) {
    let person = await this.peopleService.findById(params.ID);
    res.json(classToPlain(person));
  }
}

This works but it feels like repetition. Theoretically I could only need the param injection decorator to also include the validation middleware, similarly to how NestJS does it.

Unrelated but the @requestParam actually doesn't work as advertised, i.e. it doesn't provide the full params if the attribute name is omitted. The attribute name can't actually be omitted, opening separate issue for this.

[dcavanagh]

@bogdan-dumitru this is definitely interesting have you looked into implementing this already? Can we discuss it further?

[bowd]

@dcavanagh somehow this reply got lost in the weeds for me. Is this still of interest? I'll try to bang out a proof of concept, I have to reacquaint myself with the issue/solution a bit though it felt pretty straightforward if I remember correctly.