From ce5a01ca1299dd91a862dd573880a5ee1553fd53 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Tue, 17 Jan 2023 20:53:12 -0800 Subject: [PATCH] Moved history access to specialized service impl --- src/routers/config-strategy.js | 13 ++-- src/routers/config.js | 13 ++-- src/routers/domain.js | 11 ++-- src/routers/group-config.js | 13 ++-- src/services/history.js | 17 +++++ tests/fixtures/db_history.js | 24 +++++++ tests/services/history.test.js | 111 +++++++++++++++++++++++++++++++++ 7 files changed, 171 insertions(+), 31 deletions(-) create mode 100644 src/services/history.js create mode 100644 tests/fixtures/db_history.js create mode 100644 tests/services/history.test.js diff --git a/src/routers/config-strategy.js b/src/routers/config-strategy.js index f3339f1..70ad1c6 100644 --- a/src/routers/config-strategy.js +++ b/src/routers/config-strategy.js @@ -1,6 +1,5 @@ import express from 'express'; import { check, query } from 'express-validator'; -import History from '../models/history'; import { EnvType } from '../models/environment'; import { validate, verifyInputUpdateParameters } from '../middleware/validators'; import { strategyRequirements, StrategiesType } from '../models/config-strategy'; @@ -8,6 +7,7 @@ import { auth } from '../middleware/auth'; import { verifyOwnership, sortBy } from '../helpers'; import { ActionTypes, RouterTypes } from '../models/permission'; import { getConfigById } from '../services/config'; +import { getHistory, deleteHistory } from '../services/history'; import * as Services from '../services/config-strategy'; import { responseException } from '../exceptions'; @@ -75,12 +75,9 @@ router.get('/configstrategy/history/:id', auth, [ ], validate, async (req, res) => { try { const configStrategy = await Services.getStrategyById(req.params.id); - const history = await History.find({ domainId: configStrategy.domain, elementId: configStrategy._id }) - .select('oldValue newValue updatedBy date -_id') - .sort(sortBy(req.query)) - .limit(parseInt(req.query.limit || 10)) - .skip(parseInt(req.query.skip || 0)) - .exec(); + + const query = 'oldValue newValue updatedBy date -_id'; + const history = await getHistory(query, configStrategy.domain, configStrategy._id, req.query); await verifyOwnership(req.admin, configStrategy, configStrategy.domain, ActionTypes.READ, RouterTypes.STRATEGY); @@ -97,7 +94,7 @@ router.delete('/configstrategy/history/:id', auth, [ const configStrategy = await Services.getStrategyById(req.params.id); await verifyOwnership(req.admin, configStrategy, configStrategy.domain, ActionTypes.DELETE, RouterTypes.ADMIN); - await History.deleteMany({ domainId: configStrategy.domain, elementId: configStrategy._id }).exec(); + await deleteHistory(configStrategy.domain, configStrategy._id); res.send(configStrategy); } catch (e) { responseException(res, e, 500); diff --git a/src/routers/config.js b/src/routers/config.js index e63e089..cfd1ea5 100644 --- a/src/routers/config.js +++ b/src/routers/config.js @@ -1,7 +1,6 @@ import express from 'express'; import { check, query } from 'express-validator'; import { relayOptions } from '../models/config'; -import History from '../models/history'; import { auth } from '../middleware/auth'; import { ActionTypes, RouterTypes } from '../models/permission'; import { responseException } from '../exceptions'; @@ -10,6 +9,7 @@ import { verifyInputUpdateParameters } from '../middleware/validators'; import { sortBy, verifyOwnership } from '../helpers'; import * as Services from '../services/config'; +import { getHistory, deleteHistory } from '../services/history'; import { getGroupConfigById } from '../services/group-config'; import { SwitcherKeys } from '../external/switcher-api-facade'; @@ -80,12 +80,9 @@ router.get('/config/history/:id', auth, [ ], validate, async (req, res) => { try { const config = await Services.getConfigById(req.params.id); - const history = await History.find({ domainId: config.domain, elementId: config._id }) - .select('oldValue newValue updatedBy date -_id') - .sort(sortBy(req.query)) - .limit(parseInt(req.query.limit || 10)) - .skip(parseInt(req.query.skip || 0)) - .exec(); + + const query = 'oldValue newValue updatedBy date -_id'; + const history = await getHistory(query, config.domain, config._id, req.query); await verifyOwnership(req.admin, config, config.domain, ActionTypes.READ, RouterTypes.CONFIG); @@ -102,7 +99,7 @@ router.delete('/config/history/:id', auth, [ const config = await Services.getConfigById(req.params.id); await verifyOwnership(req.admin, config, config.domain, ActionTypes.DELETE, RouterTypes.ADMIN); - await History.deleteMany({ domainId: config.domain, elementId: config._id }).exec(); + await deleteHistory(config.domain, config._id); res.send(config); } catch (e) { responseException(res, e, 500); diff --git a/src/routers/domain.js b/src/routers/domain.js index 5348df1..9572568 100644 --- a/src/routers/domain.js +++ b/src/routers/domain.js @@ -1,5 +1,4 @@ import express from 'express'; -import History from '../models/history'; import { auth } from '../middleware/auth'; import { check } from 'express-validator'; import { validate, verifyInputUpdateParameters } from '../middleware/validators'; @@ -7,6 +6,7 @@ import { verifyOwnership, sortBy } from '../helpers'; import { ActionTypes, RouterTypes } from '../models/permission'; import { checkDomain, SwitcherKeys } from '../external/switcher-api-facade'; import * as Services from '../services/domain'; +import { getHistory } from '../services/history'; import { responseException } from '../exceptions'; const router = new express.Router(); @@ -58,12 +58,9 @@ router.get('/domain/history/:id', auth, [ ], validate, async (req, res) => { try { const domain = await Services.getDomainById(req.params.id); - const history = await History.find({ elementId: domain._id }) - .select('oldValue newValue updatedBy date -_id') - .sort(sortBy(req.query)) - .limit(parseInt(req.query.limit || 10)) - .skip(parseInt(req.query.skip || 0)) - .exec(); + + const query = 'oldValue newValue updatedBy date -_id'; + const history = await getHistory(query, domain._id, undefined, req.query); await verifyOwnership(req.admin, domain, domain._id, ActionTypes.READ, RouterTypes.DOMAIN); diff --git a/src/routers/group-config.js b/src/routers/group-config.js index 4fd9e52..ad641b8 100644 --- a/src/routers/group-config.js +++ b/src/routers/group-config.js @@ -1,12 +1,12 @@ import express from 'express'; import { check, query } from 'express-validator'; -import History from '../models/history'; import { auth } from '../middleware/auth'; import { validate, verifyInputUpdateParameters } from '../middleware/validators'; import { sortBy, verifyOwnership } from '../helpers'; import { responseException } from '../exceptions'; import { ActionTypes, RouterTypes } from '../models/permission'; import * as Services from '../services/group-config'; +import { getHistory, deleteHistory } from '../services/history'; import { getDomainById } from '../services/domain'; import { SwitcherKeys } from '../external/switcher-api-facade'; @@ -68,12 +68,9 @@ router.get('/groupconfig/history/:id', auth, [ ], validate, async (req, res) => { try { const groupconfig = await Services.getGroupConfigById(req.params.id); - const history = await History.find({ domainId: groupconfig.domain, elementId: groupconfig._id }) - .select('oldValue newValue updatedBy date -_id') - .sort(sortBy(req.query)) - .limit(parseInt(req.query.limit || 10)) - .skip(parseInt(req.query.skip || 0)) - .exec(); + + const query = 'oldValue newValue updatedBy date -_id'; + const history = await getHistory(query, groupconfig.domain, groupconfig._id, req.query); await verifyOwnership(req.admin, groupconfig, groupconfig.domain, ActionTypes.READ, RouterTypes.GROUP); @@ -90,7 +87,7 @@ router.delete('/groupconfig/history/:id', auth, [ const groupconfig = await Services.getGroupConfigById(req.params.id); await verifyOwnership(req.admin, groupconfig, groupconfig.domain, ActionTypes.DELETE, RouterTypes.ADMIN); - await History.deleteMany({ domainId: groupconfig.domain, elementId: groupconfig._id }).exec(); + await deleteHistory(groupconfig.domain, groupconfig._id); res.send(groupconfig); } catch (e) { responseException(res, e, 500); diff --git a/src/services/history.js b/src/services/history.js new file mode 100644 index 0000000..c599f22 --- /dev/null +++ b/src/services/history.js @@ -0,0 +1,17 @@ +import History from '../models/history'; +import { sortBy } from '../helpers'; + +export async function getHistory(query, domainId, elementId, specs = {}) { + const findQuery = elementId ? { domainId, elementId } : { domainId }; + + return History.find(findQuery) + .select(query) + .sort(sortBy(specs)) + .limit(parseInt(specs.limit || 10)) + .skip(parseInt(specs.skip || 0)) + .exec(); +} + +export async function deleteHistory(domainId, elementId) { + await History.deleteMany({ domainId, elementId }).exec(); +} \ No newline at end of file diff --git a/tests/fixtures/db_history.js b/tests/fixtures/db_history.js new file mode 100644 index 0000000..8e87eaa --- /dev/null +++ b/tests/fixtures/db_history.js @@ -0,0 +1,24 @@ +import mongoose from 'mongoose'; +import History from '../../src/models/history'; + +export const domainId = new mongoose.Types.ObjectId(); +export const element1Id = new mongoose.Types.ObjectId(); +export const element2Id = new mongoose.Types.ObjectId(); + +export const addHistory = async (domainId, elementId, oldValue, newValue, date) => { + const history = { + id: new mongoose.Types.ObjectId(), + domainId, + elementId, + oldValue, + newValue, + date, + updatedBy: 'FIXTURE' + }; + + await new History(history).save(); +}; + +export const setupDatabase = async () => { + await History.deleteMany().exec(); +}; \ No newline at end of file diff --git a/tests/services/history.test.js b/tests/services/history.test.js new file mode 100644 index 0000000..a59b0bf --- /dev/null +++ b/tests/services/history.test.js @@ -0,0 +1,111 @@ +require('../../src/db/mongoose'); + +import mongoose from 'mongoose'; +import { getHistory, deleteHistory } from '../../src/services/history'; +import { + setupDatabase, + addHistory, + domainId, + element1Id, + element2Id +} from '../fixtures/db_history'; + +afterAll(async () => { + await new Promise(resolve => setTimeout(resolve, 1000)); + await mongoose.disconnect(); +}); + +describe('Testing history services', () => { + beforeEach(setupDatabase); + + test('HISTORY_SERVICE - Should get history', async () => { + // given + const timestamp = Date.now(); + await addHistory(domainId, element1Id, { value: 1 }, { value: 2 }, timestamp); + + // test + const history = await getHistory('oldValue newValue updatedBy', domainId, element1Id); + expect(history[0].toJSON()).toEqual( + expect.objectContaining({ + oldValue: { value: 1 }, + newValue: { value: 2 }, + updatedBy: 'FIXTURE' + }) + ); + }); + + test('HISTORY_SERVICE - Should get history - single entry', async () => { + // given + const timestamp = Date.now(); + await addHistory(domainId, element1Id, { value: 1 }, { value: 2 }, timestamp); + await addHistory(domainId, element1Id, { value: 2 }, { value: 3 }, timestamp); + await addHistory(domainId, element2Id, { value: 4 }, { value: 5 }, timestamp); + + // test + const history = await getHistory('elementId', domainId, element1Id, { + limit: 1 + }); + + expect(history.length).toBe(1); + expect(history[0].elementId).toMatchObject(element1Id); + }); + + test('HISTORY_SERVICE - Should get history - skip first entry', async () => { + // given + const timestamp = Date.now(); + await addHistory(domainId, element1Id, { value: 1 }, { value: 2 }, timestamp); + await addHistory(domainId, element1Id, { value: 3 }, { value: 4 }, timestamp); + + // test + const history = await getHistory('oldValue', domainId, element1Id, { + skip: 1 + }); + + expect(history[0].oldValue.toJSON()).toMatchObject({ value: 3 }); + }); + + test('HISTORY_SERVICE - Should get history entries sorted by asc', async () => { + // given + const timestamp = Date.now(); + await addHistory(domainId, element1Id, { value: 1 }, { value: 2 }, timestamp); + await addHistory(domainId, element1Id, { value: 3 }, { value: 4 }, timestamp); + + // test + const history = await getHistory('oldValue', domainId, element1Id, { + sortBy: 'oldValue:asc' + }); + + expect(history[0].oldValue.toJSON()).toMatchObject({ value: 1 }); + }); + + test('HISTORY_SERVICE - Should get history entries sorted by desc', async () => { + // given + const timestamp = Date.now(); + await addHistory(domainId, element1Id, { value: 1 }, { value: 2 }, timestamp); + await addHistory(domainId, element1Id, { value: 3 }, { value: 4 }, timestamp); + + // test + const history = await getHistory('oldValue', domainId, element1Id, { + sortBy: 'oldValue:desc' + }); + + expect(history[0].oldValue.toJSON()).toMatchObject({ value: 3 }); + }); + + test('HISTORY_SERVICE - Should delete element history entries', async () => { + // given + const timestamp = Date.now(); + await addHistory(domainId, element1Id, { value: 1 }, { value: 2 }, timestamp); + await addHistory(domainId, element1Id, { value: 3 }, { value: 4 }, timestamp); + + // that + let history = await getHistory('oldValue', domainId, element1Id); + expect(history.length).toBe(2); + + // test + await deleteHistory(domainId, element1Id); + history = await getHistory('oldValue', domainId, element1Id); + expect(history.length).toBe(0); + }); + +}); \ No newline at end of file