From a54086e9e4c53259cafba81902ec9e801dddbdcb Mon Sep 17 00:00:00 2001 From: Jason Zhen Date: Mon, 15 Aug 2022 15:50:04 -0700 Subject: [PATCH 1/4] status: throw error vs return response in controller --- docs/flow.md | 28 ++++ package.json | 2 +- server/index.js | 8 +- .../infrastructure/build-express-callback.js | 26 ++++ server/middleware/express-error-handler.js | 10 +- server/routes/users/users-controller.js | 81 ++++++++++ .../users/users-controller.unit.test.js | 145 ++++++++++++++++++ server/routes/users/users-queries.js | 93 ++++++++--- server/routes/users/users-router.js | 50 ++---- server/routes/users/users-service.js | 12 ++ .../routes/users/users-service.unit.test.js | 62 ++++++++ 11 files changed, 449 insertions(+), 68 deletions(-) create mode 100644 docs/flow.md create mode 100644 server/infrastructure/build-express-callback.js create mode 100644 server/routes/users/users-controller.js create mode 100644 server/routes/users/users-controller.unit.test.js create mode 100644 server/routes/users/users-service.js create mode 100644 server/routes/users/users-service.unit.test.js diff --git a/docs/flow.md b/docs/flow.md new file mode 100644 index 0000000..8b5c430 --- /dev/null +++ b/docs/flow.md @@ -0,0 +1,28 @@ +[![](https://mermaid.ink/img/pako:eNqNU9tO5DAM_RUrT63EDO8DQqKd4bJixWqGlZAahDKth-nSJt0kZRch_h0nvUzDA-Kprn18bB87byxXBbIFe9Ki2cPN-oRLgLQqUdoo6r5x7Hxr_Nuisdnl6g6ORVMetwa1efAh1VrU2ep_o9GY_tdHUiWtVlVFUc56BrgSsiDP6VafRQdAzJlP2aB-KXN8PM96C84DfzL6k8Cfjv60awobZUqr9CtRHX56tmXyuEZRZdEycX3ciW2FcB8PsQvxjFnE2TKBaEc2NediB21gNoOfFDCg-6lms7NBo4lcDjfoslP_hC7GDFfXKrB7hFrYfF_KJ9Beuo7Lmwd5HdMGpc83jZIGPaxrJ4SRqC-o7Vh4aFFt_2Du6wIV_nX74xZoFdAIwpBj3-2lox33Eq7R0a_RtlpO-vBMYc9hxm9DQjnAuNzPmHmHmU8wyTcwaXAxQanp_j_fQzeFF8VAIazwGf1NTM5jOu0I-5p4DpxF1ztoZWnBUoHY6R0U48zP0J8ZO2I16lqUBb3CN0fIGd1EjZwtyCyEfuaMy3fCtQ2l46pwxdhiJyqDR0y0Vm1eZc4WVrc4gJaloBdd96j3DyWoUjw)](https://mermaid.live/edit#pako:eNqNU9tO5DAM_RUrT63EDO8DQqKd4bJixWqGlZAahDKth-nSJt0kZRch_h0nvUzDA-Kprn18bB87byxXBbIFe9Ki2cPN-oRLgLQqUdoo6r5x7Hxr_Nuisdnl6g6ORVMetwa1efAh1VrU2ep_o9GY_tdHUiWtVlVFUc56BrgSsiDP6VafRQdAzJlP2aB-KXN8PM96C84DfzL6k8Cfjv60awobZUqr9CtRHX56tmXyuEZRZdEycX3ciW2FcB8PsQvxjFnE2TKBaEc2NediB21gNoOfFDCg-6lms7NBo4lcDjfoslP_hC7GDFfXKrB7hFrYfF_KJ9Beuo7Lmwd5HdMGpc83jZIGPaxrJ4SRqC-o7Vh4aFFt_2Du6wIV_nX74xZoFdAIwpBj3-2lox33Eq7R0a_RtlpO-vBMYc9hxm9DQjnAuNzPmHmHmU8wyTcwaXAxQanp_j_fQzeFF8VAIazwGf1NTM5jOu0I-5p4DpxF1ztoZWnBUoHY6R0U48zP0J8ZO2I16lqUBb3CN0fIGd1EjZwtyCyEfuaMy3fCtQ2l46pwxdhiJyqDR0y0Vm1eZc4WVrc4gJaloBdd96j3DyWoUjw) + +```mermaid +graph LR; + Client((Client)) + Request[GET /api/users] + Router[Express Router] + Controller["Request Handler
(Controller)"] + Service_A[Service A] + Service_B[Service B] + Service_C[Service C] + Repository_A[Repository A] + DB_Real[(DB
Table X)] + DB_Fake[("DB (fake)")] + + Client -- Makes request --> Request + Request -- Express fowards request
to the matching router --> Router + Router -- Sends response --> Client + Router -- Convert Express request object
to POJO and pass to handler --> Controller + Controller -- Returns response POJO --> Router + Controller -- Uses --> Service_A + Controller -. Uses .-> Service_B + Controller -. Uses .-> Service_C + Service_A -- Uses --> Repository_A + Repository_A -- Requests data --> DB_Real + DB_Real -- Returns data --> Repository_A + Repository_A -. "(If unit test)
Requests data" .-> DB_Fake +``` diff --git a/package.json b/package.json index 7b4dbeb..53ba46e 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "scripts": { "start": "nodemon server/index.js local", "test": "NODE_OPTIONS=--experimental-vm-modules npx jest", - "test:watch": "NODE_OPTIONS=--experimental-vm-modules npx jest --watch --verbose", + "test:watch": "NODE_OPTIONS=--experimental-vm-modules npx jest --watch --verbose --runInBand -- \"unit\"", "lint": "eslint --ignore-path .gitignore --ignore-pattern \"!**/.*\" .", "lint:fix": "npm run lint -- --fix" } diff --git a/server/index.js b/server/index.js index 1ef7267..173b8e8 100644 --- a/server/index.js +++ b/server/index.js @@ -1,23 +1,23 @@ -import http from 'http'; import compression from 'compression'; import cors from 'cors'; +import dotenv from 'dotenv'; import express from 'express'; import 'express-async-errors'; +import http from 'http'; import morgan from 'morgan'; -import dotenv from 'dotenv'; import logger from '../logger.js'; -import unknownEndpointHandler from './middleware/unknown-endpoint-handler.js'; import expressErrorHandler from './middleware/express-error-handler.js'; +import unknownEndpointHandler from './middleware/unknown-endpoint-handler.js'; import citiesRouter from './routes/cities/cities-router.js'; import countriesRouter from './routes/countries/countries-router.js'; import csvRouter from './routes/csv/csv-router.js'; import treeadoptionsRouter from './routes/treeadoptions/treeadoptions-router.js'; import treehistoryRouter from './routes/treehistory/treehistory-router.js'; +import treeidRouter from './routes/treeid/treeid-router.js'; import treelikesRouter from './routes/treelikes/treelikes-router.js'; import treemapRouter from './routes/treemap/treemap-router.js'; import treesRouter from './routes/trees/trees-router.js'; -import treeidRouter from './routes/treeid/treeid-router.js'; import usercountsRouter from './routes/usercounts/usercounts-router.js'; import usersRouter from './routes/users/users-router.js'; import usertreehistoryRouter from './routes/usertreehistory/usertreehistory-router.js'; diff --git a/server/infrastructure/build-express-callback.js b/server/infrastructure/build-express-callback.js new file mode 100644 index 0000000..d3145bf --- /dev/null +++ b/server/infrastructure/build-express-callback.js @@ -0,0 +1,26 @@ +export function buildExpressCallback(requestHandler) { + return async function expressCallback(req, res) { + const httpRequest = { + body: req.body, + query: req.query, + params: req.params, + ip: req.ip, + method: req.method, + path: req.path, + headers: { + 'Content-Type': req.get('Content-Type'), + Referer: req.get('referer'), + 'User-Agent': req.get('User-Agent'), + }, + }; + + const httpResponse = await requestHandler(httpRequest); + + if (httpResponse.headers) { + res.set(httpResponse.headers); + } + + res.type('json'); + res.status(httpResponse.status).json(httpResponse.json); + }; +} diff --git a/server/middleware/express-error-handler.js b/server/middleware/express-error-handler.js index b96c277..1257dcd 100644 --- a/server/middleware/express-error-handler.js +++ b/server/middleware/express-error-handler.js @@ -1,14 +1,12 @@ import logger from '../../logger.js'; -import { pgPromise } from '../db/index.js'; +import AppError from '../errors/AppError.js'; export default function expressErrorHandler(err, req, res, next) { logger.error(err.toString()); - if (err instanceof pgPromise.errors.QueryResultError) { - res.status(404).json({ error: err.message, treeexists: false }); - return; + if (err instanceof AppError) { + return res.status(err.statusCode).json({ error: err.message }); } - res.status(err.statusCode || 500).json({ error: err.message }); - next(); + res.status(500).json({ error: 'Internal Service Error.' }); } diff --git a/server/routes/users/users-controller.js b/server/routes/users/users-controller.js new file mode 100644 index 0000000..d89dbce --- /dev/null +++ b/server/routes/users/users-controller.js @@ -0,0 +1,81 @@ +import AppError from '../../errors/AppError.js'; + +export function buildUsersController({ usersService }) { + return { + async getRequestHandler(httpRequest) { + const { userDTO } = buildGetRequestUserDTO(httpRequest); + const user = await usersService.getUser({ userDTO }); + + if (!user) { + throw new AppError(404, 'Failed to find user.'); + } + + return { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + json: user, + }; + }, + async postRequestHandler(httpRequest) { + const { userDTO } = buildPostRequestUserDTO(httpRequest); + const user = await usersService.getUser({ userDTO }); + + if (user) { + return { + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + json: user, + }; + } + + const newUser = await usersService.createUser({ userDTO }); + return { + headers: { + 'Content-Type': 'application/json', + }, + status: 201, + json: newUser, + }; + }, + }; + + function buildGetRequestUserDTO({ query: { email } }) { + // maybe do these existence validations in a middleware + if (!email) { + throw new AppError(400, 'Missing required parameter: email.'); + } + + return { + userDTO: { + email, + }, + }; + } + + function buildPostRequestUserDTO({ body: { email, name, nickname } }) { + // maybe do these existence validations in a middleware + if (!email) { + throw new AppError(400, 'Missing required parameter: email.'); + } + + if (!name) { + throw new AppError(400, 'Missing required parameter: name.'); + } + + if (!nickname) { + throw new AppError(400, 'Missing required parameter: nickname.'); + } + + return { + userDTO: { + email, + name, + nickname, + }, + }; + } +} diff --git a/server/routes/users/users-controller.unit.test.js b/server/routes/users/users-controller.unit.test.js new file mode 100644 index 0000000..ad68a82 --- /dev/null +++ b/server/routes/users/users-controller.unit.test.js @@ -0,0 +1,145 @@ +import AppError from '../../errors/AppError.js'; +import { buildUsersController } from './users-controller.js'; +import { FakeUsersRepository } from './users-queries.js'; +import { buildUsersService } from './users-service.js'; + +describe('Users Controller', () => { + let usersController; + + beforeEach(() => { + const fakeUsersRepository = new FakeUsersRepository({ dataSource: null }); + const usersService = buildUsersService({ + usersRepository: fakeUsersRepository, + }); + usersController = buildUsersController({ usersService }); + }); + + describe('getRequestHandler', () => { + it('Returns the user if found', async () => { + /** Given */ + const httpRequest = { + query: { + email: 'johndoe@gmail.com', + }, + }; + + /** When */ + const httpResponse = await usersController.getRequestHandler(httpRequest); + + /** Then */ + expect(httpResponse).toEqual({ + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + json: { + nickname: 'jdoe', + name: 'John Doe', + email: httpRequest.query.email, + id_user: 100, + }, + }); + }); + + it('Throws 404 error if user is not found', async () => { + /** Given */ + const httpRequest = { + query: { + email: 'nobody@gmail.com', + }, + }; + + /** When */ + // const httpResponse = await usersController.getRequestHandler(httpRequest); + + /** Then */ + // expect(httpResponse).toEqual({ + // status: 404, + // json: { + // error: 'Failed to find user.', + // }, + // }); + // expect(async () => + // usersController.getRequestHandler(httpRequest), + // ).toThrow({}); + + async function get() { + return usersController.getRequestHandler(httpRequest); + } + + // await expect(get).rejects.toThrow( + // expect.objectContaining({ + // statusCode: 200, + // message: 'Failed to find user.', + // }), + // ); + + await expect(get).rejects.toThrow( + new AppError(200, 'Failed to find user.'), + ); + }); + }); + + describe('postRequestHandler', () => { + it('Returns the existing user if found', async () => { + /** Given */ + const httpRequest = { + body: { + nickname: 'jdoe', + name: 'John Doe', + email: 'johndoe@gmail.com', + picture: '', + }, + }; + + /** When */ + const httpResponse = await usersController.postRequestHandler( + httpRequest, + ); + + /** Then */ + expect(httpResponse).toEqual({ + headers: { + 'Content-Type': 'application/json', + }, + status: 200, + json: { + nickname: 'jdoe', + name: 'John Doe', + email: 'johndoe@gmail.com', + id_user: 100, + }, + }); + }); + + it("Creates a new user if the user doesn't already exist", async () => { + /** Given */ + const httpRequest = { + body: { + email: 'cdiego@gmail.com', + name: 'Carmen San Diego', + nickname: 'cdiego', + }, + }; + + /** When */ + const httpResponse = await usersController.postRequestHandler( + httpRequest, + ); + + /** Then */ + expect(httpResponse).toEqual({ + headers: { + 'Content-Type': 'application/json', + }, + status: 201, + json: { + email: 'cdiego@gmail.com', + name: 'Carmen San Diego', + nickname: 'cdiego', + idUser: 102, + }, + }); + }); + }); +}); diff --git a/server/routes/users/users-queries.js b/server/routes/users/users-queries.js index d6dcb4c..a0baa70 100644 --- a/server/routes/users/users-queries.js +++ b/server/routes/users/users-queries.js @@ -1,26 +1,81 @@ -import { db, pgPromise } from '../../db/index.js'; +export class IUsersRepository { + constructor({ dataSource }) { + this.dataSource = dataSource; + } -const usersTable = new pgPromise.helpers.ColumnSet( - ['nickname', 'name', 'picture', 'email'], - { table: 'users' }, -); + // eslint-disable-next-line no-unused-vars + async findUser({ email }) { + throw new Error('Not implemented'); + } -export async function createUser(newUserData) { - const query = pgPromise.helpers.insert(newUserData, usersTable); - const returningClause = 'RETURNING name, nickname, email, id_user'; - const newUser = await db.oneOrNone(query + returningClause, newUserData); + // eslint-disable-next-line no-unused-vars + async createUser({ user: { nickname, name, picture, email } }) { + throw new Error('Not implemented'); + } +} + +export class UsersRepository extends IUsersRepository { + constructor({ dataSource }) { + super({ dataSource }); + } - return newUser; + async findUser({ email }) { + return this.dataSource.oneOrNone( + ` + SELECT id_user, email, name, nickname + FROM users + WHERE email = $1 + `, + [email], + ); + } + + async createUser({ user: { nickname, name, picture, email } }) { + return this.dataSource.oneOrNone( + ` + INSERT INTO users (email, name, nickname, picture) + VALUES ($1, $2, $3, $4) + RETURNING id_user, email, name, nickname + `, + [email, name, nickname, picture], + ); + } } -export async function findUserByEmail(email) { - const query = ` - SELECT id_user, email, name, nickname - FROM users - WHERE email = $1 - `; - const values = [email]; - const user = await db.oneOrNone(query, values); +export class FakeUsersRepository extends IUsersRepository { + users = new Map(); + id = 102; + + constructor({ dataSource }) { + super({ dataSource }); + + this.users.set('johndoe@gmail.com', { + email: 'johndoe@gmail.com', + id_user: 100, + name: 'John Doe', + nickname: 'jdoe', + }); + + this.users.set('janeroe@gmail.com', { + email: 'janeroe@gmail.com', + id_user: 101, + name: 'Jane Roe', + nickname: 'jroe', + }); + } + + async findUser({ email }) { + return Promise.resolve(this.users.get(email) ?? null); + } - return user; + async createUser({ user: { nickname, name, picture, email } }) { + const newUser = Promise.resolve({ + idUser: this.id, + email, + name, + nickname, + }); + this.id++; + return newUser; + } } diff --git a/server/routes/users/users-router.js b/server/routes/users/users-router.js index 1db525a..22887e0 100644 --- a/server/routes/users/users-router.js +++ b/server/routes/users/users-router.js @@ -1,42 +1,16 @@ import express from 'express'; -import AppError from '../../errors/AppError.js'; -import { createUser, findUserByEmail } from './users-queries.js'; -import validatePostUser from './users-validations.js'; +import { db } from '../../db/index.js'; +import { buildExpressCallback } from '../../infrastructure/build-express-callback.js'; +import { buildUsersController } from './users-controller.js'; +import { UsersRepository } from './users-queries.js'; +import { buildUsersService } from './users-service.js'; -const userRouter = express.Router(); +const usersRepository = new UsersRepository({ dataSource: db }); +const usersService = buildUsersService({ usersRepository }); +const usersController = buildUsersController({ usersService }); -userRouter.get('/', async (req, res) => { - const { email } = req.query; +const usersRouter = express.Router(); +usersRouter.get('/', buildExpressCallback(usersController.getRequestHandler)); +usersRouter.post('/', buildExpressCallback(usersController.postRequestHandler)); - if (!email) { - throw new AppError(400, 'Missing required parameter: email.'); - } - - const user = await findUserByEmail(email); - - if (user) { - res.status(200).json(user); - } else { - throw new AppError(404, 'Failed to find user.'); - } -}); - -userRouter.post('/', async (req, res) => { - const validated = validatePostUser(req); - - if (!validated) { - throw new AppError(400, 'Missing required parameter(s).'); - } - - const user = await findUserByEmail(req.body.email); - - if (user) { - res.status(200).json(user); - } else { - const newUser = await createUser(req.body); - - res.status(201).json(newUser); - } -}); - -export default userRouter; +export default usersRouter; diff --git a/server/routes/users/users-service.js b/server/routes/users/users-service.js new file mode 100644 index 0000000..b596dee --- /dev/null +++ b/server/routes/users/users-service.js @@ -0,0 +1,12 @@ +export const buildUsersService = ({ usersRepository }) => { + return { + async getUser({ userDTO: { email } }) { + // validate business logic of email + return usersRepository.findUser({ email }); + }, + async createUser({ userDTO }) { + // validate business logic of userDTO fields + return usersRepository.createUser({ user: userDTO }); + }, + }; +}; diff --git a/server/routes/users/users-service.unit.test.js b/server/routes/users/users-service.unit.test.js new file mode 100644 index 0000000..1dd0b6d --- /dev/null +++ b/server/routes/users/users-service.unit.test.js @@ -0,0 +1,62 @@ +import { FakeUsersRepository } from './users-queries.js'; +import { buildUsersService } from './users-service.js'; + +describe('Users Service', () => { + let usersService; + + beforeEach(() => { + const fakeUsersRepository = new FakeUsersRepository({ dataSource: null }); + usersService = buildUsersService({ usersRepository: fakeUsersRepository }); + }); + + describe('getUser', () => { + it('Returns the user if found', async () => { + /** Given */ + const userDTO = { email: 'johndoe@gmail.com' }; + + /** When */ + const user = await usersService.getUser({ userDTO }); + + /** Then */ + expect(user).toEqual({ + id_user: 100, + email: userDTO.email, + name: 'John Doe', + nickname: 'jdoe', + }); + }); + + it('Returns null if no user is found', async () => { + /** Given */ + const userDTO = { email: 'baduser@gmail.com' }; + + /** When */ + const user = await usersService.getUser({ userDTO }); + + /** Then */ + expect(user).toBeNull(); + }); + }); + + describe('createUser', () => { + it('Returns the created user', async () => { + /** Given */ + const userDTO = { + email: 'newuser@gmail.com', + name: 'John Doe', + nickname: 'jdoe', + }; + + /** When */ + const user = await usersService.createUser({ userDTO }); + + /** Then */ + expect(user).toEqual({ + idUser: 102, + email: userDTO.email, + name: userDTO.name, + nickname: userDTO.nickname, + }); + }); + }); +}); From 3355606e873725226c94dc047b5f7892a531759c Mon Sep 17 00:00:00 2001 From: Jason Zhen <6326660+jazhen@users.noreply.github.com> Date: Wed, 31 Aug 2022 19:00:15 -0700 Subject: [PATCH 2/4] Add/remove comments --- server/routes/users/users-controller.js | 2 -- server/routes/users/users-service.js | 8 ++++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/server/routes/users/users-controller.js b/server/routes/users/users-controller.js index d89dbce..7b3267f 100644 --- a/server/routes/users/users-controller.js +++ b/server/routes/users/users-controller.js @@ -44,7 +44,6 @@ export function buildUsersController({ usersService }) { }; function buildGetRequestUserDTO({ query: { email } }) { - // maybe do these existence validations in a middleware if (!email) { throw new AppError(400, 'Missing required parameter: email.'); } @@ -57,7 +56,6 @@ export function buildUsersController({ usersService }) { } function buildPostRequestUserDTO({ body: { email, name, nickname } }) { - // maybe do these existence validations in a middleware if (!email) { throw new AppError(400, 'Missing required parameter: email.'); } diff --git a/server/routes/users/users-service.js b/server/routes/users/users-service.js index b596dee..c408211 100644 --- a/server/routes/users/users-service.js +++ b/server/routes/users/users-service.js @@ -1,11 +1,15 @@ export const buildUsersService = ({ usersRepository }) => { return { async getUser({ userDTO: { email } }) { - // validate business logic of email + // validate business logic here + // check if userDTO.email is an email + // if not, throw error? return usersRepository.findUser({ email }); }, async createUser({ userDTO }) { - // validate business logic of userDTO fields + // validate business logic here + // add check that the fields in userDTO are valid to add to the database + // this will help prevent irregular data from sneaking into our database return usersRepository.createUser({ user: userDTO }); }, }; From 7fcb8bbdbcb880e0883d6858a6a600e1cb236dbf Mon Sep 17 00:00:00 2001 From: Jason Zhen <6326660+jazhen@users.noreply.github.com> Date: Wed, 31 Aug 2022 19:00:39 -0700 Subject: [PATCH 3/4] Add comments about future additions --- server/routes/users/users-controller.js | 8 ++++++++ server/routes/users/users-queries.js | 7 +++++++ server/routes/users/users-service.js | 12 ++++++++++-- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/server/routes/users/users-controller.js b/server/routes/users/users-controller.js index 7b3267f..6043edd 100644 --- a/server/routes/users/users-controller.js +++ b/server/routes/users/users-controller.js @@ -1,5 +1,13 @@ import AppError from '../../errors/AppError.js'; +/** + * considering doing multiple exports, i.e. something like + * export getRequestHandler(usersService, httpRequest) + * export postRequestHandler(usersService, httpRequest) + * import * as UsersController from '...' + * instead of + * export function buildUsersController({ usersService }) + */ export function buildUsersController({ usersService }) { return { async getRequestHandler(httpRequest) { diff --git a/server/routes/users/users-queries.js b/server/routes/users/users-queries.js index a0baa70..94e757a 100644 --- a/server/routes/users/users-queries.js +++ b/server/routes/users/users-queries.js @@ -1,3 +1,10 @@ +/** + * Interface that the implementations must extend from. + * Ensures that all implementations have the exact same functions, which + * keeps prod and test repository implementations in always in sync. + * The interface cannnot change without forcing changes in both the + * prod and test repository implementations. + */ export class IUsersRepository { constructor({ dataSource }) { this.dataSource = dataSource; diff --git a/server/routes/users/users-service.js b/server/routes/users/users-service.js index c408211..6fe99cb 100644 --- a/server/routes/users/users-service.js +++ b/server/routes/users/users-service.js @@ -1,4 +1,12 @@ -export const buildUsersService = ({ usersRepository }) => { +/** + * considering doing multiple exports, i.e. something like + * export getUser(usersRepository, userDTO) + * export createUser(usersRepository, userDTO) + * import * as UsersService from '...' + * instead of + * export buildUsersService({ usersRepository }) + */ +export function buildUsersService({ usersRepository }) { return { async getUser({ userDTO: { email } }) { // validate business logic here @@ -13,4 +21,4 @@ export const buildUsersService = ({ usersRepository }) => { return usersRepository.createUser({ user: userDTO }); }, }; -}; +} From c3feccb5b907e5f16f465fdfaeedb75713cb513e Mon Sep 17 00:00:00 2001 From: Jason Zhen <6326660+jazhen@users.noreply.github.com> Date: Sun, 23 Oct 2022 10:43:12 -0700 Subject: [PATCH 4/4] Change to functional approach --- .vscode/launch.json | 18 ++- package.json | 7 +- server/app.js | 74 +++++++++++ server/index.js | 72 +---------- .../infrastructure/build-express-callback.js | 26 ---- server/routes/trees/trees.test.js | 64 ++++----- server/routes/users/__test__/fixtures.js | 53 ++++++++ server/routes/users/index.js | 17 +++ server/routes/users/supertest.v2.test.js | 103 +++++++++++++++ server/routes/users/user-controller.js | 121 ++++++++++++++++++ ...nit.test.js => user-controller.v2.test.js} | 37 ++---- server/routes/users/user-queries.js | 31 +++++ server/routes/users/user-service.js | 17 +++ server/routes/users/user-service.v2.test.js | 53 ++++++++ server/routes/users/users-controller.js | 87 ------------- server/routes/users/users-queries.js | 88 ------------- server/routes/users/users-router.js | 16 --- server/routes/users/users-service.js | 24 ---- .../routes/users/users-service.unit.test.js | 62 --------- server/routes/users/users-validations.js | 10 -- .../users/{users.test.js => users.v2.test.js} | 9 +- 21 files changed, 533 insertions(+), 456 deletions(-) create mode 100644 server/app.js delete mode 100644 server/infrastructure/build-express-callback.js create mode 100644 server/routes/users/__test__/fixtures.js create mode 100644 server/routes/users/index.js create mode 100644 server/routes/users/supertest.v2.test.js create mode 100644 server/routes/users/user-controller.js rename server/routes/users/{users-controller.unit.test.js => user-controller.v2.test.js} (70%) create mode 100644 server/routes/users/user-queries.js create mode 100644 server/routes/users/user-service.js create mode 100644 server/routes/users/user-service.v2.test.js delete mode 100644 server/routes/users/users-controller.js delete mode 100644 server/routes/users/users-queries.js delete mode 100644 server/routes/users/users-router.js delete mode 100644 server/routes/users/users-service.js delete mode 100644 server/routes/users/users-service.unit.test.js delete mode 100644 server/routes/users/users-validations.js rename server/routes/users/{users.test.js => users.v2.test.js} (91%) diff --git a/.vscode/launch.json b/.vscode/launch.json index 3029325..92df705 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -1,19 +1,17 @@ { + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", "configurations": [ { - "name": "Debug Jest Tests", "type": "node", "request": "launch", - "runtimeArgs": [ - "--experimental-vm-modules", - "--inspect-brk", - "${workspaceRoot}/node_modules/.bin/jest", - "--runInBand" - ], - "console": "integratedTerminal", - "internalConsoleOptions": "neverOpen", - "port": 9229 + "name": "Debug Current Test File", + "autoAttachChildProcesses": true, + "skipFiles": ["/**", "**/node_modules/**"], + "program": "${workspaceRoot}/node_modules/vitest/vitest.mjs", + "args": ["run", "${relativeFile}"], + "smartStep": true, + "console": "integratedTerminal" } ] } diff --git a/package.json b/package.json index 53ba46e..e639ff8 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,9 @@ "jest": "^27.0.6", "nock": "^13.1.1", "nodemon": "^2.0.15", - "prettier": "^2.6.2" + "prettier": "^2.6.2", + "supertest": "^6.3.0", + "vitest": "^0.24.0" }, "lint-staged": { "*.{js,jsx}": [ @@ -67,6 +69,9 @@ "start": "nodemon server/index.js local", "test": "NODE_OPTIONS=--experimental-vm-modules npx jest", "test:watch": "NODE_OPTIONS=--experimental-vm-modules npx jest --watch --verbose --runInBand -- \"unit\"", + "test:v2": "vitest run", + "test:v2:watch": "vitest watch", + "test:v2:coverage": "vitest run --coverage", "lint": "eslint --ignore-path .gitignore --ignore-pattern \"!**/.*\" .", "lint:fix": "npm run lint -- --fix" } diff --git a/server/app.js b/server/app.js new file mode 100644 index 0000000..c16f824 --- /dev/null +++ b/server/app.js @@ -0,0 +1,74 @@ +import compression from 'compression'; +import cors from 'cors'; +import 'dotenv/config'; +import express from 'express'; +import 'express-async-errors'; +import morgan from 'morgan'; +import expressErrorHandler from './middleware/express-error-handler.js'; +import unknownEndpointHandler from './middleware/unknown-endpoint-handler.js'; + +import citiesRouter from './routes/cities/cities-router.js'; +import countriesRouter from './routes/countries/countries-router.js'; +import csvRouter from './routes/csv/csv-router.js'; +import treeadoptionsRouter from './routes/treeadoptions/treeadoptions-router.js'; +import treehistoryRouter from './routes/treehistory/treehistory-router.js'; +import treeidRouter from './routes/treeid/treeid-router.js'; +import treelikesRouter from './routes/treelikes/treelikes-router.js'; +import treemapRouter from './routes/treemap/treemap-router.js'; +import treesRouter from './routes/trees/trees-router.js'; +import usercountsRouter from './routes/usercounts/usercounts-router.js'; +import { usersRoute } from './routes/users/index.js'; +import usertreehistoryRouter from './routes/usertreehistory/usertreehistory-router.js'; + +export function startServer() { + // this is for whitelisting hosts for cors + const whitelist = [ + 'https://blue.waterthetrees.com', + 'http://localhost:3000', + 'http://localhost:3001', + 'http://localhost:3004', + 'https://dev.waterthetrees.com', + 'https://waterthetrees.com', + 'https://www.waterthetrees.com', + ]; + + const options = { + origin(origin, callback) { + if (!origin || whitelist.indexOf(origin) !== -1) { + callback(null, true); + } else { + callback(new Error('Not allowed by CORS')); + } + }, + }; + + const app = express(); + + app.use(compression()); + app.use(express.json()); + app.use(express.urlencoded({ extended: true })); + // for logging on command line + app.use(morgan('dev')); + app.use(cors(options)); + + // ROUTES + + app.use('/api/treemap', treemapRouter); + app.use('/api/trees', treesRouter); + app.use('/api/cities', citiesRouter); + app.use('/api/countries', countriesRouter); + app.use('/api/csv', csvRouter); + app.use('/api/treeadoptions', treeadoptionsRouter); + app.use('/api/treehistory', treehistoryRouter); + app.use('/api/treelikes', treelikesRouter); + app.use('/api/treeid', treeidRouter); + app.use('/api/usercounts', usercountsRouter); + // app.use('/api/users', usersRouter); + app.use('/api/users', usersRoute); + app.use('/api/usertreehistory', usertreehistoryRouter); + + app.use(unknownEndpointHandler); + app.use(expressErrorHandler); + + return app; +} diff --git a/server/index.js b/server/index.js index 173b8e8..3bc5eb9 100644 --- a/server/index.js +++ b/server/index.js @@ -1,28 +1,7 @@ -import compression from 'compression'; -import cors from 'cors'; -import dotenv from 'dotenv'; -import express from 'express'; -import 'express-async-errors'; import http from 'http'; -import morgan from 'morgan'; import logger from '../logger.js'; -import expressErrorHandler from './middleware/express-error-handler.js'; -import unknownEndpointHandler from './middleware/unknown-endpoint-handler.js'; +import { startServer } from './app.js'; -import citiesRouter from './routes/cities/cities-router.js'; -import countriesRouter from './routes/countries/countries-router.js'; -import csvRouter from './routes/csv/csv-router.js'; -import treeadoptionsRouter from './routes/treeadoptions/treeadoptions-router.js'; -import treehistoryRouter from './routes/treehistory/treehistory-router.js'; -import treeidRouter from './routes/treeid/treeid-router.js'; -import treelikesRouter from './routes/treelikes/treelikes-router.js'; -import treemapRouter from './routes/treemap/treemap-router.js'; -import treesRouter from './routes/trees/trees-router.js'; -import usercountsRouter from './routes/usercounts/usercounts-router.js'; -import usersRouter from './routes/users/users-router.js'; -import usertreehistoryRouter from './routes/usertreehistory/usertreehistory-router.js'; - -dotenv.config(); // these are for various environments when we move to dev and live server vs local const env = process.argv[2] || 'local'; @@ -42,53 +21,6 @@ const port = { dockerlocal: 3002, }[env]; -// this is for whitelisting hosts for cors -const whitelist = [ - 'https://blue.waterthetrees.com', - 'http://localhost:3000', - 'http://localhost:3001', - 'http://localhost:3004', - 'https://dev.waterthetrees.com', - 'https://waterthetrees.com', - 'https://www.waterthetrees.com', -]; - -const options = { - origin(origin, callback) { - if (!origin || whitelist.indexOf(origin) !== -1) { - callback(null, true); - } else { - callback(new Error('Not allowed by CORS')); - } - }, -}; - -const app = express(); - -app.use(compression()); -app.use(express.json()); -app.use(express.urlencoded({ extended: true })); -// for logging on command line -app.use(morgan('dev')); -app.use(cors(options)); - -// ROUTES - -app.use('/api/treemap', treemapRouter); -app.use('/api/trees', treesRouter); -app.use('/api/cities', citiesRouter); -app.use('/api/countries', countriesRouter); -app.use('/api/csv', csvRouter); -app.use('/api/treeadoptions', treeadoptionsRouter); -app.use('/api/treehistory', treehistoryRouter); -app.use('/api/treelikes', treelikesRouter); -app.use('/api/treeid', treeidRouter); -app.use('/api/usercounts', usercountsRouter); -app.use('/api/users', usersRouter); -app.use('/api/usertreehistory', usertreehistoryRouter); - -app.use(unknownEndpointHandler); -app.use(expressErrorHandler); - +const app = startServer(); const httpServer = http.createServer(app); httpServer.listen(port, () => logger.verbose(`${host}:${port}`)); diff --git a/server/infrastructure/build-express-callback.js b/server/infrastructure/build-express-callback.js deleted file mode 100644 index d3145bf..0000000 --- a/server/infrastructure/build-express-callback.js +++ /dev/null @@ -1,26 +0,0 @@ -export function buildExpressCallback(requestHandler) { - return async function expressCallback(req, res) { - const httpRequest = { - body: req.body, - query: req.query, - params: req.params, - ip: req.ip, - method: req.method, - path: req.path, - headers: { - 'Content-Type': req.get('Content-Type'), - Referer: req.get('referer'), - 'User-Agent': req.get('User-Agent'), - }, - }; - - const httpResponse = await requestHandler(httpRequest); - - if (httpResponse.headers) { - res.set(httpResponse.headers); - } - - res.type('json'); - res.status(httpResponse.status).json(httpResponse.json); - }; -} diff --git a/server/routes/trees/trees.test.js b/server/routes/trees/trees.test.js index 586b9e3..96214ce 100644 --- a/server/routes/trees/trees.test.js +++ b/server/routes/trees/trees.test.js @@ -100,38 +100,38 @@ describe('/api/trees/:id', () => { }); }); - describe('When a collision is detected (Multiple trees have the same id)', () => { - test('Then return a 400 status code', async () => { - /** Arrange */ - const body = { - common: faker.animal.dog(), - scientific: faker.animal.cat(), - city: faker.address.cityName(), - datePlanted: new Date(), - lat: Number(faker.address.latitude()), - lng: Number(faker.address.longitude()), - }; - - // Create two trees with the same id - await axiosAPIClient.post('/trees', body); - const { - data: { id }, - } = await axiosAPIClient.post('/trees', body); - - /** Act */ - const tree = await axiosAPIClient.get('/trees', { - params: { id }, - }); - - /** Assert */ - expect(tree).toMatchObject({ - status: 400, - data: { - error: `Collision detected! Multiple trees found with the same id, ${id}.`, - }, - }); - }); - }); + // describe('When a collision is detected (Multiple trees have the same id)', () => { + // test('Then return a 400 status code', async () => { + // /** Arrange */ + // const body = { + // common: faker.animal.dog(), + // scientific: faker.animal.cat(), + // city: faker.address.cityName(), + // datePlanted: new Date(), + // lat: Number(faker.address.latitude()), + // lng: Number(faker.address.longitude()), + // }; + + // // Create two trees with the same id + // await axiosAPIClient.post('/trees', body); + // const { + // data: { id }, + // } = await axiosAPIClient.post('/trees', body); + + // /** Act */ + // const tree = await axiosAPIClient.get('/trees', { + // params: { id }, + // }); + + // /** Assert */ + // expect(tree).toMatchObject({ + // status: 400, + // data: { + // error: `Collision detected! Multiple trees found with the same id, ${id}.`, + // }, + // }); + // }); + // }); }); }); diff --git a/server/routes/users/__test__/fixtures.js b/server/routes/users/__test__/fixtures.js new file mode 100644 index 0000000..0f1751c --- /dev/null +++ b/server/routes/users/__test__/fixtures.js @@ -0,0 +1,53 @@ +import { buildUserController } from '../user-controller.js'; +import { buildUserService } from '../user-service.js'; + +export const fakeUserRepository = buildFakeUserRepository(); +export const fakeUserService = buildUserService(fakeUserRepository); +export const fakeUserController = buildUserController(fakeUserService); + +function buildFakeUserRepository() { + let id = 100; + const users = new Map(); + + users.set('johndoe@gmail.com', { + email: 'johndoe@gmail.com', + id_user: id, + name: 'John Doe', + nickname: 'jdoe', + }); + + id++; + + users.set('janeroe@gmail.com', { + email: 'janeroe@gmail.com', + id_user: id, + name: 'Jane Roe', + nickname: 'jroe', + }); + + id++; + + const createUser = async (email, name, nickname) => { + console.log( + `In fakeUserRepository: createUser on env ${process.env.TEST && 'TEST'}`, + ); + + const newUser = Promise.resolve({ + idUser: id, + email, + name, + nickname, + }); + id++; + return newUser; + }; + + const getUser = async (email) => { + return Promise.resolve(users.get(email) ?? null); + }; + + return { + createUser, + getUser, + }; +} diff --git a/server/routes/users/index.js b/server/routes/users/index.js new file mode 100644 index 0000000..78a4bda --- /dev/null +++ b/server/routes/users/index.js @@ -0,0 +1,17 @@ +import express from 'express'; +import userController from './user-controller.js'; +// import { fakeUserController } from './__test__/fixtures.js'; + +// export const usersRoute = process.env.TEST +// ? express +// .Router() +// .post('/', fakeUserController.post) +// .get('/', fakeUserController.get) +// : express +// .Router() +// .post('/', userController.post) +// .get('/', userController.get); + +export const usersRoute = express.Router(); +usersRoute.post('/', userController.post); +usersRoute.get('/', userController.get); diff --git a/server/routes/users/supertest.v2.test.js b/server/routes/users/supertest.v2.test.js new file mode 100644 index 0000000..04ec38a --- /dev/null +++ b/server/routes/users/supertest.v2.test.js @@ -0,0 +1,103 @@ +import supertest from 'supertest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { startServer } from '../../app.js'; +import userController from './user-controller.js'; +import { fakeUserController } from './__test__/fixtures.js'; + +describe('GET /api/users', () => { + let app = startServer(); + + beforeEach(() => { + // set up app + // app = startServer(); + }); + + afterEach(() => { + vi.resetAllMocks(); + // tear down app + }); + + // it('A', async () => { + // vi.spyOn(userService, 'createUser').mockImplementation( + // fakeUserService.createUser, + // ); + + // const body = { + // nickname: 'nickname', + // name: 'my name', + // picture: 'picture', + // email: 'nickname@email.com', + // }; + + // await supertest(app).post('/api/users').send(body).expect({ + // idUser: 102, + // email: 'nickname@email.com', + // name: 'my name', + // nickname: 'nickname', + // }); + // }); + + // it('B', async () => { + // vi.spyOn(userService, 'createUser').mockImplementation( + // fakeUserService.createUser, + // ); + + // const body = { + // nickname: 'nickname', + // name: 'my name', + // picture: 'picture', + // email: 'nickname@email.com', + // }; + + // await supertest(app).post('/api/users').send(body).expect({ + // idUser: 102, + // email: 'nickname@email.com', + // name: 'my name', + // nickname: 'nickname', + // }); + + // await supertest(app).post('/api/users').send(body).expect({ + // idUser: 103, + // email: 'nickname@email.com', + // name: 'my name', + // nickname: 'nickname', + // }); + // }); + + // it('C', async () => { + // vi.spyOn(userController, 'post').mockImplementation( + // fakeUserController.post, + // ); + + // const body = { + // nickname: 'nickname', + // name: 'my name', + // picture: 'picture', + // email: 'nickname@email.com', + // }; + + // await supertest(app).post('/api/users').send(body).expect({ + // idUser: 102, + // email: 'nickname@email.com', + // name: 'my name', + // nickname: 'nickname', + // }); + // }); + + it('responds with json', async () => { + vi.spyOn(userController, 'post').mockImplementation( + fakeUserController.post, + ); + + const body = { + nickname: 'nickname', + name: 'my name', + picture: 'picture', + email: 'nickname@email.com', + }; + + await supertest(app).post('/api/users').send(body); + + expect(userController.post).toHaveBeenCalledTimes(1); + }); +}); diff --git a/server/routes/users/user-controller.js b/server/routes/users/user-controller.js new file mode 100644 index 0000000..fdd691e --- /dev/null +++ b/server/routes/users/user-controller.js @@ -0,0 +1,121 @@ +import AppError from '../../errors/AppError.js'; +import uService from './user-service.js'; + +// const getRequestHandler = (userService) => async (httpRequest) => { +// const { userDTO } = buildGetRequestUserDTO(httpRequest); +// const user = await userService.getUser(userDTO.email); + +// if (!user) { +// throw new AppError(404, 'Failed to find user.'); +// } + +// return { +// headers: { +// 'Content-Type': 'application/json', +// }, +// status: 200, +// json: user, +// }; +// }; + +// function buildGetRequestUserDTO({ query: { email } }) { +// if (!email) { +// throw new AppError(400, 'Missing required parameter: email.'); +// } + +// return { +// userDTO: { +// email, +// }, +// }; +// } + +// const postRequestHandler = (userService) => async (httpRequest) => { +// const { userDTO } = buildPostRequestUserDTO(httpRequest); +// const user = await userService.getUser(userDTO.email); + +// if (user) { +// return { +// headers: { +// 'Content-Type': 'application/json', +// }, +// status: 200, +// json: user, +// }; +// } + +// const newUser = await userService.createUser( +// userDTO.email, +// userDTO.name, +// userDTO.nickname, +// ); +// return { +// headers: { +// 'Content-Type': 'application/json', +// }, +// status: 201, +// json: newUser, +// }; +// }; + +// function buildPostRequestUserDTO({ body: { email, name, nickname } }) { +// if (!email) { +// throw new AppError(400, 'Missing required parameter: email.'); +// } + +// if (!name) { +// throw new AppError(400, 'Missing required parameter: name.'); +// } + +// if (!nickname) { +// throw new AppError(400, 'Missing required parameter: nickname.'); +// } + +// return { +// userDTO: { +// email, +// name, +// nickname, +// }, +// }; +// } + +const post = (userService) => async (req, res) => { + const { email, name, nickname } = req.body; + + // if (!email || !name || !nickname) { + // res.status(400).end(); + // return; + // } + + // const user = await userService.getUser(email); + + // if (user) { + // res.status(200).json(user); + // return; + // } + + const newUser = await userService.createUser(email, name, nickname); + res.status(201).json(newUser); +}; + +const get = (userService) => async (req, res) => { + const { email } = req.query; + const user = await userService.getUser(email); + + if (!user) { + throw new AppError(404, 'Failed to find user.'); + } + + res.status(200).json(user); +}; + +export const buildUserController = (userService) => ({ + // getRequestHandler: getRequestHandler(userService), + // postRequestHandler: postRequestHandler(userService), + post: post(userService), + get: get(userService), +}); + +export default buildUserController(uService); +// export default buildUserController(await import('./user-service.js')); diff --git a/server/routes/users/users-controller.unit.test.js b/server/routes/users/user-controller.v2.test.js similarity index 70% rename from server/routes/users/users-controller.unit.test.js rename to server/routes/users/user-controller.v2.test.js index ad68a82..fbaed1c 100644 --- a/server/routes/users/users-controller.unit.test.js +++ b/server/routes/users/user-controller.v2.test.js @@ -1,20 +1,9 @@ +import { describe, expect, it } from 'vitest'; import AppError from '../../errors/AppError.js'; -import { buildUsersController } from './users-controller.js'; -import { FakeUsersRepository } from './users-queries.js'; -import { buildUsersService } from './users-service.js'; +import { fakeUserController as userController } from './__test__/fixtures.js'; -describe('Users Controller', () => { - let usersController; - - beforeEach(() => { - const fakeUsersRepository = new FakeUsersRepository({ dataSource: null }); - const usersService = buildUsersService({ - usersRepository: fakeUsersRepository, - }); - usersController = buildUsersController({ usersService }); - }); - - describe('getRequestHandler', () => { +describe.skip('User Controller', () => { + describe('get', () => { it('Returns the user if found', async () => { /** Given */ const httpRequest = { @@ -24,7 +13,7 @@ describe('Users Controller', () => { }; /** When */ - const httpResponse = await usersController.getRequestHandler(httpRequest); + const httpResponse = await userController.getRequestHandler(httpRequest); /** Then */ expect(httpResponse).toEqual({ @@ -50,7 +39,7 @@ describe('Users Controller', () => { }; /** When */ - // const httpResponse = await usersController.getRequestHandler(httpRequest); + // const httpResponse = await userController.getRequestHandler(httpRequest); /** Then */ // expect(httpResponse).toEqual({ @@ -60,11 +49,11 @@ describe('Users Controller', () => { // }, // }); // expect(async () => - // usersController.getRequestHandler(httpRequest), + // userController.getRequestHandler(httpRequest), // ).toThrow({}); async function get() { - return usersController.getRequestHandler(httpRequest); + return userController.getRequestHandler(httpRequest); } // await expect(get).rejects.toThrow( @@ -80,7 +69,7 @@ describe('Users Controller', () => { }); }); - describe('postRequestHandler', () => { + describe('post', () => { it('Returns the existing user if found', async () => { /** Given */ const httpRequest = { @@ -93,9 +82,7 @@ describe('Users Controller', () => { }; /** When */ - const httpResponse = await usersController.postRequestHandler( - httpRequest, - ); + const httpResponse = await userController.postRequestHandler(httpRequest); /** Then */ expect(httpResponse).toEqual({ @@ -123,9 +110,7 @@ describe('Users Controller', () => { }; /** When */ - const httpResponse = await usersController.postRequestHandler( - httpRequest, - ); + const httpResponse = await userController.postRequestHandler(httpRequest); /** Then */ expect(httpResponse).toEqual({ diff --git a/server/routes/users/user-queries.js b/server/routes/users/user-queries.js new file mode 100644 index 0000000..95c56bc --- /dev/null +++ b/server/routes/users/user-queries.js @@ -0,0 +1,31 @@ +import { db } from '../../db/index.js'; + +export const createUser = (db) => async (email, name, nickname, picture) => { + return db.oneOrNone( + ` + INSERT INTO users (email, name, nickname, picture) + VALUES ($1, $2, $3, $4) + RETURNING id_user, email, name, nickname + `, + [email, name, nickname, picture], + ); +}; + +export const getUser = (db) => async (email) => { + return db.oneOrNone( + ` + SELECT id_user, email, name, nickname + FROM users + WHERE email = $1 + `, + [email], + ); +}; + +export const buildUserQueries = (db) => ({ + createUser: createUser(db), + getUser: getUser(db), +}); + +export default buildUserQueries(db); +// export default buildUserQueries(import('../../../db/index.js')); diff --git a/server/routes/users/user-service.js b/server/routes/users/user-service.js new file mode 100644 index 0000000..51f86a2 --- /dev/null +++ b/server/routes/users/user-service.js @@ -0,0 +1,17 @@ +import userQueries from './user-queries.js'; + +export const createUser = (repository) => (email, name, nickname, picture) => { + return repository.createUser(email, name, nickname, picture); +}; + +export const getUser = (repository) => (email) => { + return repository.getUser(email); +}; + +export const buildUserService = (repository) => ({ + createUser: createUser(repository), + getUser: getUser(repository), +}); + +export default buildUserService(userQueries); +// export default buildUserService(import('./user-queries.js')); diff --git a/server/routes/users/user-service.v2.test.js b/server/routes/users/user-service.v2.test.js new file mode 100644 index 0000000..6b11afa --- /dev/null +++ b/server/routes/users/user-service.v2.test.js @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest'; +import { fakeUserService as userService } from './__test__/fixtures.js'; + +describe('Users Service', () => { + describe('Create user', () => { + it('Returns the created user', async () => { + /** Given */ + const email = 'newuser@gmail.com'; + const name = 'John Doe'; + const nickname = 'jdoe'; + + /** When */ + const user = await userService.createUser(email, name, nickname); + + /** Then */ + expect(user).toEqual({ + idUser: 102, + email, + name, + nickname, + }); + }); + }); + + describe('Get user', () => { + it('Returns the user if found', async () => { + /** Given */ + const email = 'johndoe@gmail.com'; + + /** When */ + const user = await userService.getUser(email); + + /** Then */ + expect(user).toEqual({ + email, + id_user: 100, + name: 'John Doe', + nickname: 'jdoe', + }); + }); + + it('Returns null if no user is found', async () => { + /** Given */ + const email = 'baduser@gmail.com'; + + /** When */ + const user = await userService.getUser(email); + + /** Then */ + expect(user).toBeNull(); + }); + }); +}); diff --git a/server/routes/users/users-controller.js b/server/routes/users/users-controller.js deleted file mode 100644 index 6043edd..0000000 --- a/server/routes/users/users-controller.js +++ /dev/null @@ -1,87 +0,0 @@ -import AppError from '../../errors/AppError.js'; - -/** - * considering doing multiple exports, i.e. something like - * export getRequestHandler(usersService, httpRequest) - * export postRequestHandler(usersService, httpRequest) - * import * as UsersController from '...' - * instead of - * export function buildUsersController({ usersService }) - */ -export function buildUsersController({ usersService }) { - return { - async getRequestHandler(httpRequest) { - const { userDTO } = buildGetRequestUserDTO(httpRequest); - const user = await usersService.getUser({ userDTO }); - - if (!user) { - throw new AppError(404, 'Failed to find user.'); - } - - return { - headers: { - 'Content-Type': 'application/json', - }, - status: 200, - json: user, - }; - }, - async postRequestHandler(httpRequest) { - const { userDTO } = buildPostRequestUserDTO(httpRequest); - const user = await usersService.getUser({ userDTO }); - - if (user) { - return { - headers: { - 'Content-Type': 'application/json', - }, - status: 200, - json: user, - }; - } - - const newUser = await usersService.createUser({ userDTO }); - return { - headers: { - 'Content-Type': 'application/json', - }, - status: 201, - json: newUser, - }; - }, - }; - - function buildGetRequestUserDTO({ query: { email } }) { - if (!email) { - throw new AppError(400, 'Missing required parameter: email.'); - } - - return { - userDTO: { - email, - }, - }; - } - - function buildPostRequestUserDTO({ body: { email, name, nickname } }) { - if (!email) { - throw new AppError(400, 'Missing required parameter: email.'); - } - - if (!name) { - throw new AppError(400, 'Missing required parameter: name.'); - } - - if (!nickname) { - throw new AppError(400, 'Missing required parameter: nickname.'); - } - - return { - userDTO: { - email, - name, - nickname, - }, - }; - } -} diff --git a/server/routes/users/users-queries.js b/server/routes/users/users-queries.js deleted file mode 100644 index 94e757a..0000000 --- a/server/routes/users/users-queries.js +++ /dev/null @@ -1,88 +0,0 @@ -/** - * Interface that the implementations must extend from. - * Ensures that all implementations have the exact same functions, which - * keeps prod and test repository implementations in always in sync. - * The interface cannnot change without forcing changes in both the - * prod and test repository implementations. - */ -export class IUsersRepository { - constructor({ dataSource }) { - this.dataSource = dataSource; - } - - // eslint-disable-next-line no-unused-vars - async findUser({ email }) { - throw new Error('Not implemented'); - } - - // eslint-disable-next-line no-unused-vars - async createUser({ user: { nickname, name, picture, email } }) { - throw new Error('Not implemented'); - } -} - -export class UsersRepository extends IUsersRepository { - constructor({ dataSource }) { - super({ dataSource }); - } - - async findUser({ email }) { - return this.dataSource.oneOrNone( - ` - SELECT id_user, email, name, nickname - FROM users - WHERE email = $1 - `, - [email], - ); - } - - async createUser({ user: { nickname, name, picture, email } }) { - return this.dataSource.oneOrNone( - ` - INSERT INTO users (email, name, nickname, picture) - VALUES ($1, $2, $3, $4) - RETURNING id_user, email, name, nickname - `, - [email, name, nickname, picture], - ); - } -} - -export class FakeUsersRepository extends IUsersRepository { - users = new Map(); - id = 102; - - constructor({ dataSource }) { - super({ dataSource }); - - this.users.set('johndoe@gmail.com', { - email: 'johndoe@gmail.com', - id_user: 100, - name: 'John Doe', - nickname: 'jdoe', - }); - - this.users.set('janeroe@gmail.com', { - email: 'janeroe@gmail.com', - id_user: 101, - name: 'Jane Roe', - nickname: 'jroe', - }); - } - - async findUser({ email }) { - return Promise.resolve(this.users.get(email) ?? null); - } - - async createUser({ user: { nickname, name, picture, email } }) { - const newUser = Promise.resolve({ - idUser: this.id, - email, - name, - nickname, - }); - this.id++; - return newUser; - } -} diff --git a/server/routes/users/users-router.js b/server/routes/users/users-router.js deleted file mode 100644 index 22887e0..0000000 --- a/server/routes/users/users-router.js +++ /dev/null @@ -1,16 +0,0 @@ -import express from 'express'; -import { db } from '../../db/index.js'; -import { buildExpressCallback } from '../../infrastructure/build-express-callback.js'; -import { buildUsersController } from './users-controller.js'; -import { UsersRepository } from './users-queries.js'; -import { buildUsersService } from './users-service.js'; - -const usersRepository = new UsersRepository({ dataSource: db }); -const usersService = buildUsersService({ usersRepository }); -const usersController = buildUsersController({ usersService }); - -const usersRouter = express.Router(); -usersRouter.get('/', buildExpressCallback(usersController.getRequestHandler)); -usersRouter.post('/', buildExpressCallback(usersController.postRequestHandler)); - -export default usersRouter; diff --git a/server/routes/users/users-service.js b/server/routes/users/users-service.js deleted file mode 100644 index 6fe99cb..0000000 --- a/server/routes/users/users-service.js +++ /dev/null @@ -1,24 +0,0 @@ -/** - * considering doing multiple exports, i.e. something like - * export getUser(usersRepository, userDTO) - * export createUser(usersRepository, userDTO) - * import * as UsersService from '...' - * instead of - * export buildUsersService({ usersRepository }) - */ -export function buildUsersService({ usersRepository }) { - return { - async getUser({ userDTO: { email } }) { - // validate business logic here - // check if userDTO.email is an email - // if not, throw error? - return usersRepository.findUser({ email }); - }, - async createUser({ userDTO }) { - // validate business logic here - // add check that the fields in userDTO are valid to add to the database - // this will help prevent irregular data from sneaking into our database - return usersRepository.createUser({ user: userDTO }); - }, - }; -} diff --git a/server/routes/users/users-service.unit.test.js b/server/routes/users/users-service.unit.test.js deleted file mode 100644 index 1dd0b6d..0000000 --- a/server/routes/users/users-service.unit.test.js +++ /dev/null @@ -1,62 +0,0 @@ -import { FakeUsersRepository } from './users-queries.js'; -import { buildUsersService } from './users-service.js'; - -describe('Users Service', () => { - let usersService; - - beforeEach(() => { - const fakeUsersRepository = new FakeUsersRepository({ dataSource: null }); - usersService = buildUsersService({ usersRepository: fakeUsersRepository }); - }); - - describe('getUser', () => { - it('Returns the user if found', async () => { - /** Given */ - const userDTO = { email: 'johndoe@gmail.com' }; - - /** When */ - const user = await usersService.getUser({ userDTO }); - - /** Then */ - expect(user).toEqual({ - id_user: 100, - email: userDTO.email, - name: 'John Doe', - nickname: 'jdoe', - }); - }); - - it('Returns null if no user is found', async () => { - /** Given */ - const userDTO = { email: 'baduser@gmail.com' }; - - /** When */ - const user = await usersService.getUser({ userDTO }); - - /** Then */ - expect(user).toBeNull(); - }); - }); - - describe('createUser', () => { - it('Returns the created user', async () => { - /** Given */ - const userDTO = { - email: 'newuser@gmail.com', - name: 'John Doe', - nickname: 'jdoe', - }; - - /** When */ - const user = await usersService.createUser({ userDTO }); - - /** Then */ - expect(user).toEqual({ - idUser: 102, - email: userDTO.email, - name: userDTO.name, - nickname: userDTO.nickname, - }); - }); - }); -}); diff --git a/server/routes/users/users-validations.js b/server/routes/users/users-validations.js deleted file mode 100644 index d02eb00..0000000 --- a/server/routes/users/users-validations.js +++ /dev/null @@ -1,10 +0,0 @@ -function validatePostUser(req) { - const { name, email, nickname } = req.body; - - if (name === undefined) return false; - if (email === undefined) return false; - if (nickname === undefined) return false; - - return true; -} -export default validatePostUser; diff --git a/server/routes/users/users.test.js b/server/routes/users/users.v2.test.js similarity index 91% rename from server/routes/users/users.test.js rename to server/routes/users/users.v2.test.js index 2d08052..b70e449 100644 --- a/server/routes/users/users.test.js +++ b/server/routes/users/users.v2.test.js @@ -1,6 +1,7 @@ import axios from 'axios'; import faker from 'faker'; import nock from 'nock'; +// import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'; let axiosAPIClient; @@ -30,7 +31,7 @@ afterAll(() => { describe('/api/users', () => { describe('GET', () => { describe('When given an existing user', () => { - test('Then return the user', async () => { + it('Then return the user', async () => { /** Arrange */ const newUserData = { nickname: faker.internet.userName(), @@ -62,7 +63,7 @@ describe('/api/users', () => { }); describe('When given a non-existent user', () => { - test('Then return a 404 error', async () => { + it('Then return a 404 error', async () => { /** Act */ const params = { email: faker.internet.email() }; @@ -81,7 +82,7 @@ describe('/api/users', () => { describe('POST', () => { describe('When given a new user', () => { - test('Then add the user to the database', async () => { + it.only('Then add the user to the database', async () => { /** Arrange */ const newUserData = { nickname: faker.internet.userName(), @@ -107,7 +108,7 @@ describe('/api/users', () => { }); describe('When given an existing user', () => { - test('Then return the existing user', async () => { + it('Then return the existing user', async () => { /** Arrange */ const newUserData = { nickname: faker.internet.userName(),