dbartholomae / middy-middleware-class-validator

A middy validation middleware using class-validator.
MIT License
13 stars 5 forks source link

Is this repo maintained? #14

Closed pzmudzinski closed 4 years ago

pzmudzinski commented 4 years ago

Want to add some PRs but not sure if it's still maintained.

dbartholomae commented 4 years ago

Sure is :) What kind of functionality would you like to add?

pzmudzinski commented 4 years ago

Let me show you my example:

import "reflect-metadata";
import middy from "@middy/core";
import ClassValidatorMiddleware, {
  WithBody
} from "middy-middleware-class-validator";
import JSONErrorHandlerMiddleware from "middy-middleware-json-error-handler";
import { APIGatewayProxyEvent, Context } from "aws-lambda";
import { getConnection } from "../connection/database";
import { Client } from "../entities/Client";
import { IsString, IsNotEmpty, validate } from "class-validator";

class ClientBody {
  @IsString()
  @IsNotEmpty()
  public name: string;
}

const create = async (
  event: WithBody<APIGatewayProxyEvent, ClientBody>,
  serverlessContext: Context
) => {
  let connection = await getConnection();

  let client = new Client();
  client.name = event.body.name;

  let clientRepo = connection.getRepository(Client);

  const savedClient = await clientRepo.save(client);
  return {
    statusCode: 201,
    body: JSON.stringify({
      record: savedClient
    })
  };
};

export const handler = middy(create)
  .use(
    ClassValidatorMiddleware({
      classType: ClientBody
    })
  )
  .use(JSONErrorHandlerMiddleware());

Now I look at it I see it might be related to JSONErrorHandlerMiddleware - just starting playing with your middleware :)

So when I sent an empty body ({}) this is response I am getting:

{
    "0": {
        "target": {},
        "property": "name",
        "children": [],
        "constraints": {
            "isNotEmpty": "name should not be empty",
            "isString": "name must be a string"
        }
    },
    "statusCode": 400
}

See "0" property? Probably it comes from stringify-ing array. I guess it should be configurable:

pzmudzinski commented 4 years ago

Second thing - link to "additional docs" in README.md isn't working. Third thing - if there's WithBody shouldn't there be WithPath so you can easily get strongly typed event.pathParameters?

dbartholomae commented 4 years ago

For the way the error gets serialized, this is actually happening in the error middleware, so a PR there would make more sense, or adding different middleware altogether if you want to achieve e. g. xml error handling. Additional Docs should be solved now, thanks! I forgot to enable the option in github to host the documentation. Validating the pathParameters also sounds nice. We could go from classType to bodyType and paramType to achieve something like this.

In total, if you haven't used this middleware yet and can wait a bit more: I am currently writing a new set of middlewares that does not rely on middy, mostly for easier type checking. You can find it here: https://github.com/dbartholomae/lambda-middleware No guarantees, but I hope to have the first version online end of next week.

pzmudzinski commented 4 years ago

Yeah, I am just playing to see if it's good match for my requirements, so can wait a bit.

Would you like me to help you there?

I see some potential middlewares like:

dbartholomae commented 4 years ago

I'm always happy about help. For lambda-middleware there is a bunch of open issues, specifically also some which I haven't assigned to myself and might be good starting points.

dbartholomae commented 4 years ago

Closing as there hasn't been any more discussion.