API contract reviewing

[felipeneuhauss]

Problem description

The API nowadays contains a lot of resources and we need to fix and improve some points in terms of best practices for RESTful APIs.

These are the points identified and require our attention:

1. To fix status codes applying 200 for PUT, POST, 201 for entity creations, 204 when no body content is required, and 202 when an async process is executed in the background.
2. To Apply validations for body data, path, and query requests, which means returning 422 for the user, with clear messages as well.
3. When saving new data, check if it's indispensable to return some response. Maybe one id attribute is enough.
4. Return 500 only for exceptions. Validation error is not an exception.
5. To improve the route names, like applying plural names for resources keeping the conventions.
6. Avoid returns like res.json('null') because it will generate an unexpected response for the consumer. Use res.json({}) or res.json([]) instead.
7. Avoid the use of res.end(). The res.send() checks the structure of your output and sets header information accordingly.
8. For deletions 204 status code for most cases is enough.
9. Some endpoints contain params that are not used, such us profiles/:username. In this case for example, the data managed comes from the logged user and the :username param does not influence in the result.

Furthermore, we need to improve the strategy to manage all validations. As mentioned previously some policies and schema validations are not being appropriately treated.

We need to create a standard way to organize and manage any validation rules and place its logic in a different function or class in order to improve maintenance e.g.

The route should process only the functions it was made for. As practiced in the Express framework, middleware filters, sanitize, or even logs data. So we can use the same place to manage validation rules.

More information here

Requirements

• Fix all status code
• Fix routes name conventions
• Apply validations to provide better feedback

Furthermore this items need to be fixed.

• Validations were applied through a new middleware.
• Route names were changed to plural like /artifact to /artifacts
• The way the errors were handled was changed to be more user-friendly not only returning 500 but also 422, 404, 403, with the specific errors.
• The error handler was improved to centralize all request errors
• Duplicated codes were removed in one only call
• Some unnecessary casts were removed
• Some unreachable conditions were removed.
• Unnecessary conditions to check URL params were removed because once it's not present it automatically returns 404.
• The hpp express middleware was added to protect against HTTP Parameter Pollution attacks

Definition of done

All the API attend the RESTful standard.

Acceptance criteria

All the API attend the RESTful standard.

[mattsmithies]

Please consider using an alternative status code other then 500, server error.

[mattsmithies]

Although this is somewhat opinionated, the Laravel way of handling validation is really nice, you are able to put a similar approach to JS which I will explain below.

I believe it isn't necessary to have a middleware for validation, however, to have separate classes that the controller can call through a given route can validate and then handle the subsequent error -- if you are strictly looking for bad payload input you can use status code is 422 pretty much across the board.

Have a look here to understand the overview/concept of Laraval Validators, regards validation in JS this is what I use for my Hedera API.

---

This is my primary validation folder that holds all validations for controllers or states that I wish to verify.

If I want to create a token I use this validation method

The validation is called within the controller for creating a token thus on the very first line of the controller I can return the required status code/body through the response object, already present on the controller.

Ultimately, feel free to follow any solution you want but I have found that this approach mirroring Laraval is easier to handle and maintain.

[felipeneuhauss]

@mattsmithies thank you so much for your contribution. I'm a huge fan of the Laravel framework and it's a good strategy that we can apply. I've suggested using middlewares to validate entry data because it's common in express to use middlewares to filter, log, or even sanitize data.

Of course, we need to create functions or classes to easily maintain all validation rules. This strategy is also clean and useful if you want to reuse similar or reusable validations:

I like this mechanism to organize validations rules:

https://github.com/felipeneuhauss/mappab-telemedicine-app/blob/main/src/shared/utils/validations/index.ts

The way you validated the serverless API is simple and clean as well.

So the way we going to do that is totally discussable. But once we need to improve other aspects of validations in this process about policies and schemas, we definitely need to extract it from the routes and organize it better.

[mattsmithies]

Hey @felipeneuhauss,

I'm glad my comments are useful, I would also add that the way we handle middleware at a route-level is dreadfully simple but effective.

For our guardian-middleware-api, each of our routes on our NextJS API user the following format this approach allows us to validate and inject dependencies into the state of the request, in turn, this makes testing simpler due to the purity of the given middleware or functions.

This approach also allows us to inject context into a request so if communication with the guardian (or another third-party service) is needed to satisfy the required behavior of a request, Typescript types make this a breeze to prove.

The higher-order prepare function enables us to explicitly describe the middleware functions that need to pass before reaching the handler class.

You may ask or wonder why payload validation isn't included as a middleware, while many frameworks may use that approach, I feel that the validation itself should only have the context to the payload without the entire request body.

Once again, hope you have success resolving many of these REST standardization and architectural issues on the guardian.

[voycey]

On board with all of the comments on here.

I will just point out that MikroORM is in use in the project which is very similar to Laravel & CakePHP in how it can handle validations - this would be the ideal place to implement any validation logic:

https://mikro-orm.io/api/next/core/class/EntityValidator

https://mikro-orm.io/docs/property-validation#runtime-validation