Add API request validation #3

Dizzzmas · 2021-01-12T12:37:56Z

Main changes

Due to quite a few changes to Yeoman generators in this PR it's a bit lengthy.

Main concepts regarding request validation are represented primarily in:

generators/app/templates/packages/backend/src/domain/game.ts
generators/app/templates/packages/backend/src/api/game/crud.ts
generators/app/templates/packages/backend/src/util/serialization.ts

What does this PR do?

Request bodies will now be validated using class-validator and lambda-middleware class-validator

We can't add validation as decorators. Instead we need to take an existing function and generate the Lambda handler from it.
My idea is to have all the functions for API in domain and the actual API views have only exports of modified functions.

Here's how an API view would look like with this setup:

import { Game, PaginatedResponse } from "demo-core"
import { GameSchemaLite } from "demo-core"
import { applyErrorHandlingAndValidation } from "../../util/serialization";
import { getById, create, updateById, deleteById, list } from "../../domain/game";


export const createHandler = applyErrorHandlingAndValidation<Game>(GameSchemaLite, create)

export const listHandler = applyErrorHandlingAndValidation<Game[]>(Game, list)

export const getByIdHandler = applyErrorHandlingAndValidation<Game>(Game, getById)

export const updateByIdHandler = applyErrorHandlingAndValidation<Game>(GameSchemaLite, updateById)

export const deleteByIdHandler = applyErrorHandlingAndValidation<Game>(Game, deleteById)

The PR also includes related changes to Yeoman generator and Jest tests.

Dizzzmas · 2021-01-12T12:39:37Z

generators/app/templates/packages/backend/src/api/game/crud.test.ts

@@ -1,12 +1,8 @@
 import { db } from "../../db"


When trying to use absolute paths. Deployed code was failing with unable to find module error.

Hence backend uses relative paths now

I suspect it's this issue serverless/serverless-plugin-typescript#180

Dizzzmas · 2021-01-12T12:42:40Z

generators/app/templates/packages/backend/src/api/game/crud.test.ts

@@ -89,7 +90,7 @@ describe('Test entity API', () => {
            entityToUpdate = await repo.createQueryBuilder("game").getOneOrFail()

            expect(entityToUpdate.name).not.toEqual(updatedEntity.name)
-            await updateById({ pathParameters: { gameId: entityToUpdate.id }, body: JSON.stringify(updatedEntity) } as unknown as APIGatewayProxyEventV2) as Game
+            await updateByIdHandler({ pathParameters: { gameId: entityToUpdate.id }, body: JSON.stringify(updatedEntity) } as unknown as APIGatewayProxyEventV2) as Game


Type casting with as is kinda ugly here, but had to use it because of type signatures for lambdas looking like this:

(event: APIGatewayProxyEventV2): Promise<APIGatewayProxyResultV2<Game>>

When testing we don't need and don't have things featured on APIGatewayProxyEventV2 and APIGatewayProxyResultV2 interfaces.

Can updateByIdHandler have a return type of Game?
Should we have a helper that builds a fake APIGatewayProxyEventV2 object so we don't need to type cast?

Dizzzmas · 2021-01-12T12:44:37Z

generators/app/templates/packages/backend/cloudformation/serverlessFunctions/app.yml

@@ -1,44 +1,39 @@
 listGames:
-    handler: src/api/game/crud.list
+    handler: src/api/game/crud.listHandler


In case you wonder what those Game models and API endpoints are, they're just examples the project is gonna get generated with.

I think it's gonna turn out helpful for whoever's using our template.

generators/app/templates/packages/backend/src/util/query.ts

revmischa

Nice progress! Let's try to think about how to keep things simple like this, without requiring too much boilerplate or repeating ourselves for CRUD. It's a tricky balance.
Are there no existing libraries or frameworks we can be leveraging here?
If not, this would be a great project to try to promote as its own framework

revmischa · 2021-01-12T13:27:44Z

generators/app/templates/packages/backend/src/api/game/crud.test.ts

@@ -89,7 +90,7 @@ describe('Test entity API', () => {
            entityToUpdate = await repo.createQueryBuilder("game").getOneOrFail()

            expect(entityToUpdate.name).not.toEqual(updatedEntity.name)
-            await updateById({ pathParameters: { gameId: entityToUpdate.id }, body: JSON.stringify(updatedEntity) } as unknown as APIGatewayProxyEventV2) as Game
+            await updateByIdHandler({ pathParameters: { gameId: entityToUpdate.id }, body: JSON.stringify(updatedEntity) } as unknown as APIGatewayProxyEventV2) as Game


Can updateByIdHandler have a return type of Game?
Should we have a helper that builds a fake APIGatewayProxyEventV2 object so we don't need to type cast?

generators/app/templates/packages/backend/src/api/game/crud.ts

generators/app/templates/packages/backend/src/domain/game.ts

revmischa · 2021-01-12T13:32:09Z

generators/app/templates/packages/backend/src/domain/game.ts

+
+    return {
+        items: games,
+        paginationData: getPaginationData(totalCount, pagesData),


I like putting pagination metadata in headers
Don't name identifiers with "data"

revmischa · 2021-01-12T13:34:08Z

generators/app/templates/packages/frontend/src/api/game.ts


 const apiName = process.env.REACT_APP_USE_APP_WITHOUT_AUTH ? undefined : process.env.REACT_APP_API_NAME // name of the API Gateway API

 const myInit = {
+  // OPTIONAL


A bit confused what this is

generators/model/templates/domain.ts

revmischa · 2021-01-12T13:36:08Z

generators/model/templates/domain.ts

+export const create = async (event: { body: <%= modelSchemaName %> }): Promise < APIGatewayProxyResultV2 <<%= capitalizedModelName %>>> => {
+
+    const conn = await db.getConnection()
+


Don't think we need all these newlines

Will add ESLint and Prettier this week, which will hopefully handle spacing

valeriikundas

Good job!

generators/app/templates/packages/backend/cloudformation/bastion.yml

generators/app/templates/packages/backend/index.d.ts

generators/model/index.js

generators/app/templates/packages/backend/src/domain/game.ts

generators/app/templates/packages/backend/src/util/pagination.ts

generators/app/templates/packages/backend/src/util/query.ts

Dizzzmas added 4 commits January 12, 2021 14:21

Add API request validation to templates

027a98a

Fix naming in templates

0b5d864

'Add .env.local to .gitignore

a66550e

Cleanup frontend code

d8a87eb

Dizzzmas commented Jan 12, 2021

View reviewed changes

Dizzzmas changed the title ~~Add API request validation to templates~~ Add API request validation Jan 12, 2021

Dizzzmas requested a review from revmischa January 12, 2021 13:10

Dizzzmas self-assigned this Jan 12, 2021

Dizzzmas requested a review from adamszeptycki January 12, 2021 13:13

revmischa approved these changes Jan 12, 2021

View reviewed changes

revmischa requested review from a team and Jorjon and removed request for a team January 12, 2021 13:42

Dizzzmas added 2 commits January 13, 2021 11:28

Make domain functions not have API-view related code

0f6e4d6

finished

53d1307

valeriikundas self-requested a review January 13, 2021 14:16

valeriikundas reviewed Jan 13, 2021

View reviewed changes

Template changes after code review

21ffc34

Dizzzmas merged commit 86d7cb7 into main Jan 13, 2021

Dizzzmas deleted the api-serializaion-and-validation branch January 13, 2021 16:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API request validation #3

Add API request validation #3

Dizzzmas commented Jan 12, 2021 •

edited

Loading

Dizzzmas Jan 12, 2021

revmischa Jan 12, 2021

Dizzzmas Jan 12, 2021

revmischa Jan 12, 2021

Dizzzmas Jan 12, 2021

revmischa left a comment

revmischa Jan 12, 2021

revmischa Jan 12, 2021

revmischa Jan 12, 2021

revmischa Jan 12, 2021

Dizzzmas Jan 13, 2021

valeriikundas left a comment

		export const create = async (event: { body: <%= modelSchemaName %> }): Promise < APIGatewayProxyResultV2 <<%= capitalizedModelName %>>> => {

		const conn = await db.getConnection()

Add API request validation #3

Add API request validation #3

Conversation

Dizzzmas commented Jan 12, 2021 • edited Loading

Main changes

What does this PR do?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

revmischa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valeriikundas left a comment

Choose a reason for hiding this comment

Dizzzmas commented Jan 12, 2021 •

edited

Loading