From f1f95b1199b4d951f4e464b1b9e3023a9ea26ce0 Mon Sep 17 00:00:00 2001 From: Santiago Faci Date: Mon, 24 Mar 2025 01:25:50 +0100 Subject: [PATCH] Mejoras varias --- package-lock.json | 45 ++++++++++++- package.json | 3 +- src/controller/cities.js | 58 ++++++++++++++--- src/route/cities.js | 6 +- src/service/cities.js | 17 ++--- src/test/unit/cities.test.js | 63 +++++++++++++++++++ .../unit/{utils.test.js => cityUtils.test.js} | 2 +- src/test/unit/dateUtils.test.js | 2 +- src/test/unit/mocks/cities.js | 15 +++++ src/{utils.js => utils/cityUtils.js} | 0 src/{ => utils}/dateUtils.js | 0 11 files changed, 189 insertions(+), 22 deletions(-) create mode 100644 src/test/unit/cities.test.js rename src/test/unit/{utils.test.js => cityUtils.test.js} (82%) create mode 100644 src/test/unit/mocks/cities.js rename src/{utils.js => utils/cityUtils.js} (100%) rename src/{ => utils}/dateUtils.js (100%) diff --git a/package-lock.json b/package-lock.json index aeb124e..4dde2a2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,7 +21,8 @@ "chai": "^4.4.1", "chai-http": "^4.4.0", "jest": "^29.7.0", - "mocha": "^10.4.0" + "mocha": "^10.4.0", + "node-mocks-http": "^1.0.0" } }, "node_modules/@ampproject/remapping": { @@ -4855,6 +4856,48 @@ "integrity": "sha512-O5lz91xSOeoXP6DulyHfllpq+Eg00MWitZIbtPfoSEvqIHdl5gfcY6hYzDWnj0qD5tz52PI08u9qUvSVeUBeHw==", "dev": true }, + "node_modules/node-mocks-http": { + "version": "1.16.2", + "resolved": "https://registry.npmjs.org/node-mocks-http/-/node-mocks-http-1.16.2.tgz", + "integrity": "sha512-2Sh6YItRp1oqewZNlck3LaFp5vbyW2u51HX2p1VLxQ9U/bG90XV8JY9O7Nk+HDd6OOn/oV3nA5Tx5k4Rki0qlg==", + "dev": true, + "dependencies": { + "accepts": "^1.3.7", + "content-disposition": "^0.5.3", + "depd": "^1.1.0", + "fresh": "^0.5.2", + "merge-descriptors": "^1.0.1", + "methods": "^1.1.2", + "mime": "^1.3.4", + "parseurl": "^1.3.3", + "range-parser": "^1.2.0", + "type-is": "^1.6.18" + }, + "engines": { + "node": ">=14" + }, + "peerDependencies": { + "@types/express": "^4.17.21 || ^5.0.0", + "@types/node": "*" + }, + "peerDependenciesMeta": { + "@types/express": { + "optional": true + }, + "@types/node": { + "optional": true + } + } + }, + "node_modules/node-mocks-http/node_modules/depd": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/depd/-/depd-1.1.2.tgz", + "integrity": "sha512-7emPTl6Dpo6JRXOXjLRxck+FlLRX5847cLKEn00PLAgc3g2hTZZgr+e4c2v6QpSmLeFP3n5yUo7ft6avBK/5jQ==", + "dev": true, + "engines": { + "node": ">= 0.6" + } + }, "node_modules/node-releases": { "version": "2.0.19", "resolved": "https://registry.npmjs.org/node-releases/-/node-releases-2.0.19.tgz", diff --git a/package.json b/package.json index 69c742a..1ac07d5 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "chai-http": "^4.4.0", "babel-eslint": "10.1.0", "jest": "^29.7.0", - "mocha": "^10.4.0" + "mocha": "^10.4.0", + "node-mocks-http": "^1.0.0" } } diff --git a/src/controller/cities.js b/src/controller/cities.js index 5c6359d..28bf674 100644 --- a/src/controller/cities.js +++ b/src/controller/cities.js @@ -1,4 +1,4 @@ -const { findCities, registerCity, modifyCity, removeCity } = require("../service/cities"); +const { findCities, findCity, registerCity, modifyCity, removeCity } = require("../service/cities"); const getCities = (async (req, res) => { const cityList = await findCities(); @@ -12,7 +12,17 @@ const getCities = (async (req, res) => { }); const getCity = (async (req, res) => { - const city = await findCity(req.params.city); + const cityId = parseInt(req.params.cityId); + + if (!Number.isInteger(cityId)) { + res.status(400).json({ + status: 'bad-request', + message: 'cityId is not a valid number' + }); + return; + } + + const city = await findCity(cityId); if (city === undefined) { res.status(404).json({ @@ -67,16 +77,50 @@ const postCity = (async (req, res) => { }); const putCity = (async (req, res) => { - await modifyCity(req.params.name, req.body.population, req.body.altitude); + const cityId = parseInt(req.params.cityId); + + if (!Number.isInteger(cityId)) { + res.status(400).json({ + status: 'bad-request', + message: 'cityId is not a valid number' + }); + return; + } + + const modified = await modifyCity(cityId, req.body.name, req.body.population, req.body.altitude); + + if (!modified) { + res.status(404).json({ + status: 'not-found', + message: 'city not found' + }); + return; + } res.status(204).json({}); }); const deleteCity = (async (req, res) => { - // TODO Validaciones y comprobaciones - await removeCity(req.params.name); - - res.status(204).json({}) + const cityId = parseInt(req.params.cityId); + + if (!Number.isInteger(cityId)) { + res.status(400).json({ + status: 'bad-request', + message: 'cityId is not a valid number' + }); + return; + } + + const removed = await removeCity(cityId); + if (!removed) { + res.status(404).json({ + status: 'not-found', + message: 'city not found' + }); + return; + } + + res.status(204).json({}); }); module.exports = { diff --git a/src/route/cities.js b/src/route/cities.js index 8c7be50..019d097 100644 --- a/src/route/cities.js +++ b/src/route/cities.js @@ -3,9 +3,9 @@ const { getCities, getCity, postCity, putCity, deleteCity } = require('../contro const router = express.Router(); router.get('/cities', getCities); -router.get('/cities/:city', getCity); +router.get('/cities/:cityId', getCity); router.post('/cities', postCity); -router.put('/cities/:city', putCity); -router.delete('/cities/:city', deleteCity); +router.put('/cities/:cityId', putCity); +router.delete('/cities/:cityId', deleteCity); module.exports = router; \ No newline at end of file diff --git a/src/service/cities.js b/src/service/cities.js index 8e2d999..5954a71 100644 --- a/src/service/cities.js +++ b/src/service/cities.js @@ -1,7 +1,7 @@ const knex = require('knex'); -const { getYearsFromNow } = require('../dateUtils'); -const { getDensity } = require('../utils'); +const { getYearsFromNow } = require('../utils/dateUtils'); +const { getDensity } = require('../utils/cityUtils'); const { config } = require('../config/configuration'); // Configuración de la base de datos: tipo, ubicación y otros parámetros @@ -23,8 +23,8 @@ const findCities = (async () => { return result; }); -const findCity = (async (name) => { - const result = await db('cities').select('*').where({name: name}).first(); +const findCity = (async (cityId) => { + const result = await db('cities').select('*').where({id: cityId}).first(); return result; }); @@ -60,16 +60,17 @@ const registerCity = (async (name, population, altitude, foundationDate, area) = return result; }); -const modifyCity = (async (name, population, altitude) => { +const modifyCity = (async (cityId, name, population, altitude) => { // TODO Modificar el resto de campos - await db('cities').where({ name: name }).update({ + return await db('cities').where({ id: cityId }).update({ + name: name, population: population, altitude: altitude }); }); -const removeCity = (async (name) => { - await db('cities').del().where({name: name}); +const removeCity = (async (cityId) => { + return await db('cities').del().where({id: cityId}); }); module.exports = { diff --git a/src/test/unit/cities.test.js b/src/test/unit/cities.test.js new file mode 100644 index 0000000..92e45f2 --- /dev/null +++ b/src/test/unit/cities.test.js @@ -0,0 +1,63 @@ +const httpMocks = require('node-mocks-http'); +const { describe, it, expect, afterEach } = require('@jest/globals'); + +jest.mock('../../service/cities'); + +const cityController = require('../../controller/cities'); + +const cityService = require('../../service/cities'); +const mockedFindCities = jest.spyOn(cityService, 'findCities'); +const mockedRegisterCity = jest.spyOn(cityService, "registerCity"); +const { mockCityArray, mockCityToPost, mockCityResponse, mockCityToRegister } = require('./mocks/cities'); + +afterEach(() => { + jest.clearAllMocks(); +}); + +describe('cities', () => { + it('GET /cities should get a city list', async () => { + const response = httpMocks.createResponse(); + const request = httpMocks.createRequest(); + request.app = {}; + request.app.conf = {}; + request.path = '/cities'; + + const mockedCityList = jest.fn(async () => { + return mockCityArray; + }); + mockedFindCities.mockImplementation(mockedCityList); + + await cityController.getCities(request, response); + expect(mockedFindCities).toHaveBeenCalledTimes(1); + expect(response.statusCode).toEqual(200); + expect(response._isEndCalled()).toBeTruthy(); + expect(response._getJSONData().length).toEqual(5); + }); + + it('POST /cities should register a new city', async () => { + const response = httpMocks.createResponse(); + const request = httpMocks.createRequest(); + request.app = {}; + request.app.conf = {}; + request.path = '/cities'; + request.body = mockCityToRegister; + + const mockedRegisterCityResponse = jest.fn(async () => { + return mockCityResponse; + }); + mockedRegisterCity.mockImplementation(mockedRegisterCityResponse); + + await cityController.postCity(request, response); + expect(mockedRegisterCity).toHaveBeenCalledTimes(1); + expect(response.statusCode).toEqual(201); + expect(response._isEndCalled()).toBeTruthy(); + expect(response._getJSONData().id).toEqual(1); + expect(response._getJSONData().name).toEqual('Zaragoza'); + expect(response._getJSONData().population).toEqual(700000); + expect(response._getJSONData().altitude).toEqual(200); + expect(response._getJSONData().age).toEqual(5); + expect(response._getJSONData().area).toEqual(734734); + expect(response._getJSONData().density).toEqual(2373); + expect(response._getJSONData().foundationDate).toEqual('2020-12-01'); + }); +}); \ No newline at end of file diff --git a/src/test/unit/utils.test.js b/src/test/unit/cityUtils.test.js similarity index 82% rename from src/test/unit/utils.test.js rename to src/test/unit/cityUtils.test.js index 03da570..0e74e08 100644 --- a/src/test/unit/utils.test.js +++ b/src/test/unit/cityUtils.test.js @@ -1,5 +1,5 @@ const expect = require('chai').expect; -const { getDensity} = require('../../utils'); +const { getDensity} = require('../../utils/cityUtils'); describe('utils', () => { it('getDensity', () => { diff --git a/src/test/unit/dateUtils.test.js b/src/test/unit/dateUtils.test.js index ac8ec7d..3fa1740 100644 --- a/src/test/unit/dateUtils.test.js +++ b/src/test/unit/dateUtils.test.js @@ -1,5 +1,5 @@ const expect = require('chai').expect; -const { getDaysFromNow, getDays} = require('../../dateUtils'); +const { getDaysFromNow, getDays} = require('../../utils/dateUtils'); beforeAll(() => { jest.useFakeTimers(); diff --git a/src/test/unit/mocks/cities.js b/src/test/unit/mocks/cities.js new file mode 100644 index 0000000..bed2c17 --- /dev/null +++ b/src/test/unit/mocks/cities.js @@ -0,0 +1,15 @@ +exports.mockCityArray = [ + {"id":1,"name":"Zaragoza","population":700000,"altitude":200,"age":1000,"area":734734,"density":2373,"foundationDate":"1000-01-01T00:14:44.000Z"}, + {"id":2,"name":"Madrid","population":5000000,"altitude":500,"age":800,"area":344734,"density":23473,"foundationDate":"1200-01-01T00:14:44.000Z"}, + {"id":3,"name":"cityName","population":100,"altitude":200,"age":20,"area":1000,"density":0.1,"foundationDate":"2005-10-09T22:00:00.000Z"}, + {"id":4,"name":"cityName","population":100,"altitude":200,"age":20,"area":1000,"density":0.1,"foundationDate":"2005-10-09T22:00:00.000Z"}, + {"id":5,"name":"cityName","population":100,"altitude":200,"age":20,"area":1000,"density":0.1,"foundationDate":"2005-10-09T22:00:00.000Z"} +]; + +exports.mockCityToRegister = { + "name":"Zaragoza","population":700000,"altitude":200,"area":734734,"foundationDate":"2020-12-01" +}; + +exports.mockCityResponse = { + "id": 1,"name":"Zaragoza","population":700000,"altitude":200,"age":5,"area":734734,"density":2373,"foundationDate":"2020-12-01" +}; \ No newline at end of file diff --git a/src/utils.js b/src/utils/cityUtils.js similarity index 100% rename from src/utils.js rename to src/utils/cityUtils.js diff --git a/src/dateUtils.js b/src/utils/dateUtils.js similarity index 100% rename from src/dateUtils.js rename to src/utils/dateUtils.js