From 36ca627d1bf751c07cdcb1e96d846bc7d9f111c8 Mon Sep 17 00:00:00 2001 From: VLE2FE Date: Mon, 31 Aug 2020 09:54:57 +0200 Subject: [PATCH 1/4] model now get deleted from user --- src/db.ts | 24 ++++++++++++++++++++---- src/models/changelog.ts | 4 ++-- src/routes/model.spec.ts | 18 ++++++++++++++++++ src/routes/model.ts | 40 +++++++++++++++++++++++----------------- 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/src/db.ts b/src/db.ts index cfebbbe..5f4face 100644 --- a/src/db.ts +++ b/src/db.ts @@ -137,31 +137,34 @@ export default class db { // changelog entry, expects (req, this (from query helper)) or (req, collection, conditions, data) static log(req, thisOrCollection, conditions = null, data = null) { if (! (conditions || data)) { // (req, this) + console.log(11); data = thisOrCollection._update ? _.cloneDeep(thisOrCollection._update) : {}; // replace undefined with {} + // replace keys with a leading $ Object.keys(data).forEach(key => { if (key[0] === '$') { data[key.substr(1)] = data[key]; delete data[key]; } }); - new ChangelogModel({ + console.log(thisOrCollection._conditions); + new ChangelogModel(this.logEscape(_.cloneDeep({ action: req.method + ' ' + req.url, collection_name: thisOrCollection._collection.collectionName, conditions: thisOrCollection._conditions, data: data, user_id: req.authDetails.id ? req.authDetails.id : null - }).save(err => { + }))).save({validateBeforeSave: false}, err => { if (err) console.error(err); }); } else { // (req, collection, conditions, data) - new ChangelogModel({ + new ChangelogModel(this.logEscape(_.cloneDeep({ action: req.method + ' ' + req.url, collection_name: thisOrCollection, conditions: conditions, data: data, user_id: req.authDetails.id ? req.authDetails.id : null - }).save(err => { + }))).save(err => { if (err) console.error(err); }); } @@ -178,4 +181,17 @@ export default class db { }); return object; } + + private static logEscape(obj) { // replace MongoDB control characters in keys + if (Object(obj) === obj && Object.keys(obj).length > 0) { + Object.keys(obj).forEach(key => { + const safeKey = key.replace(/[$.]/g, ''); + obj[safeKey] = this.logEscape(obj[key]); + if (key !== safeKey) { + delete obj[key]; + } + }); + } + return obj; + } }; diff --git a/src/models/changelog.ts b/src/models/changelog.ts index b26bd16..57701a9 100644 --- a/src/models/changelog.ts +++ b/src/models/changelog.ts @@ -3,9 +3,9 @@ import mongoose from 'mongoose'; const ChangelogSchema = new mongoose.Schema({ action: String, collection_name: String, - conditions: Object, + conditions: mongoose.Schema.Types.Mixed, data: Object, user_id: mongoose.Schema.Types.ObjectId -}, {minimize: false}); +}, {minimize: false, strict: false}); export default mongoose.model>('changelog', ChangelogSchema); \ No newline at end of file diff --git a/src/routes/model.spec.ts b/src/routes/model.spec.ts index 5be66b7..88a963e 100644 --- a/src/routes/model.spec.ts +++ b/src/routes/model.spec.ts @@ -2,6 +2,8 @@ import should from 'should/as-function'; import ModelFileModel from '../models/model_file'; import TestHelper from "../test/helper"; import ModelModel from '../models/model'; +import UserModel from '../models/user'; +import mongoose from 'mongoose'; describe('/model', () => { @@ -232,6 +234,22 @@ describe('/model', () => { }); }); }); + it ('removes the model_id from all user.models', done => { + TestHelper.request(server, done, { + method: 'delete', + url: '/model/VN/Model%20A', + auth: {basic: 'admin'}, + httpStatus: 200 + }).end((err, res) => { + if (err) return done(err); + should(res.body).be.eql({status: 'OK'}); + UserModel.find({models: mongoose.Types.ObjectId("120000000000000000000001")}).lean().exec((err, res) => { + if (err) return done(err); + should(res).have.lengthOf(0); + done(); + }); + }); + }); it('returns 404 for an unknown group', done => { TestHelper.request(server, done, { method: 'delete', diff --git a/src/routes/model.ts b/src/routes/model.ts index c771ff9..70bf5ac 100644 --- a/src/routes/model.ts +++ b/src/routes/model.ts @@ -3,6 +3,7 @@ import bodyParser from 'body-parser'; import ModelFileModel from '../models/model_file'; import ModelModel from '../models/model'; +import UserModel from '../models/user'; import _ from 'lodash'; import ModelValidate from './validate/model'; import res400 from './validate/res400'; @@ -54,7 +55,7 @@ router.post('/model/:group', (req, res, next) => { {$push: {models: model as never}} ).log(req).lean().exec(err => { if (err) return next(err); - res.json({status: 'OK'}) + res.json({status: 'OK'}); }); } } @@ -62,7 +63,7 @@ router.post('/model/:group', (req, res, next) => { new ModelModel({group: req.params.group, models: [model]}).save((err, data) => { if (err) return next(err); db.log(req, 'models', {_id: data._id}, data.toObject()); - res.json({status: 'OK'}) + res.json({status: 'OK'}); }); } }); @@ -77,21 +78,26 @@ router.delete('/model/:group(((?!file)[^\\/]+?))/:name', (req, res, next) => { if (!data || !data.models.find(e => e.name === req.params.name)) { return res.status(404).json({status: 'Not found'}); } - if (data.models.length > 1) { // only remove model - ModelModel.findOneAndUpdate( - {group: req.params.group}, - {$pull: {models: data.models.find(e => e.name === req.params.name) as never}} - ).log(req).lean().exec(err => { - if (err) return next(err); - res.json({status: 'OK'}) - }); - } - else { // remove document - ModelModel.findOneAndDelete({group: req.params.group}).log(req).lean().exec(err => { - if (err) return next(err); - res.json({status: 'OK'}) - }); - } + // delete all references in user.models + UserModel.updateMany({}, {$pull: {models: data.models.find(e => e.name === req.params.name)._id as never}}, + { multi: true }).log(req).lean().exec(err => { + if (err) return next(err); + if (data.models.length > 1) { // only remove model + ModelModel.findOneAndUpdate( + {group: req.params.group}, + {$pull: {models: data.models.find(e => e.name === req.params.name) as never}} + ).log(req).lean().exec(err => { + if (err) return next(err); + res.json({status: 'OK'}) + }); + } + else { // remove document + ModelModel.findOneAndDelete({group: req.params.group}).log(req).lean().exec(err => { + if (err) return next(err); + res.json({status: 'OK'}) + }); + } + }); }); }); From b59b3ea9eaa10ec974cbb2076b8493c7cd082e9f Mon Sep 17 00:00:00 2001 From: VLE2FE Date: Mon, 31 Aug 2020 13:47:12 +0200 Subject: [PATCH 2/4] small material changes for easier UI implementation --- api/material.yaml | 8 +++++- src/routes/material.spec.ts | 47 ++++++++++++++++++++++++++++----- src/routes/material.ts | 8 +++--- src/routes/validate/material.ts | 23 +++++++++++----- 4 files changed, 70 insertions(+), 16 deletions(-) diff --git a/api/material.yaml b/api/material.yaml index 548bce8..0bdd996 100644 --- a/api/material.yaml +++ b/api/material.yaml @@ -20,7 +20,13 @@ schema: type: array items: - $ref: 'api.yaml#/components/schemas/Material' + allOf: + - $ref: 'api.yaml#/components/schemas/Material' + properties: + status: + type: string + description: can be deleted/new/validated + example: new 401: $ref: 'api.yaml#/components/responses/401' 500: diff --git a/src/routes/material.spec.ts b/src/routes/material.spec.ts index 1bbae13..657cb7c 100644 --- a/src/routes/material.spec.ts +++ b/src/routes/material.spec.ts @@ -24,11 +24,12 @@ describe('/material', () => { const json = require('../test/db.json'); should(res.body).have.lengthOf(json.collections.materials.filter(e => e.status === 'validated').length); should(res.body).matchEach(material => { - should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers'); + should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers', 'status'); should(material).have.property('_id').be.type('string'); should(material).have.property('name').be.type('string'); should(material).have.property('supplier').be.type('string'); should(material).have.property('group').be.type('string'); + should(material).have.property('status', 'validated'); should(material.properties).have.property('material_template').be.type('string'); should(material.numbers).be.instanceof(Array); }); @@ -46,11 +47,12 @@ describe('/material', () => { const json = require('../test/db.json'); should(res.body).have.lengthOf(json.collections.materials.filter(e => e.status === 'validated').length); should(res.body).matchEach(material => { - should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers'); + should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers', 'status'); should(material).have.property('_id').be.type('string'); should(material).have.property('name').be.type('string'); should(material).have.property('supplier').be.type('string'); should(material).have.property('group').be.type('string'); + should(material).have.property('status', 'validated'); should(material.properties).have.property('material_template').be.type('string'); should(material.numbers).be.instanceof(Array); }); @@ -60,7 +62,7 @@ describe('/material', () => { it('allows filtering by state', done => { TestHelper.request(server, done, { method: 'get', - url: '/materials?status=new', + url: '/materials?status[]=new', auth: {basic: 'janedoe'}, httpStatus: 200 }).end((err, res) => { @@ -68,24 +70,57 @@ describe('/material', () => { const json = require('../test/db.json'); should(res.body).have.lengthOf(json.collections.materials.filter(e => e.status === 'new').length); should(res.body).matchEach(material => { - should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers'); + should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers', 'status'); should(material).have.property('_id').be.type('string'); should(material).have.property('name').be.type('string'); should(material).have.property('supplier').be.type('string'); should(material).have.property('group').be.type('string'); + should(material).have.property('status', 'new'); should(material.properties).have.property('material_template').be.type('string'); should(material.numbers).be.instanceof(Array); }); done(); }); }); + it('allows filtering by deleted state for admins', done => { + TestHelper.request(server, done, { + method: 'get', + url: '/materials?status[]=deleted', + auth: {basic: 'admin'}, + httpStatus: 200 + }).end((err, res) => { + if (err) return done(err); + const json = require('../test/db.json'); + should(res.body).have.lengthOf(json.collections.materials.filter(e => e.status === 'deleted').length); + should(res.body).matchEach(material => { + should(material).have.only.keys('_id', 'name', 'supplier', 'group', 'properties', 'numbers', 'status'); + should(material).have.property('_id').be.type('string'); + should(material).have.property('name').be.type('string'); + should(material).have.property('supplier').be.type('string'); + should(material).have.property('group').be.type('string'); + should(material).have.property('status', 'deleted'); + should(material.properties).have.property('material_template').be.type('string'); + should(material.numbers).be.instanceof(Array); + }); + done(); + }); + }); + it('rejects filtering by deleted state for a write user', done => { + TestHelper.request(server, done, { + method: 'get', + url: '/materials?status[]=deleted', + auth: {basic: 'janedoe'}, + httpStatus: 400, + res: {status: 'Invalid body format', details: '"status[0]" must be one of [validated, new]'} + }); + }); it('rejects an invalid state name', done => { TestHelper.request(server, done, { method: 'get', - url: '/materials?status=xxx', + url: '/materials?status[]=xxx', auth: {basic: 'janedoe'}, httpStatus: 400, - res: {status: 'Invalid body format', details: '"status" must be one of [validated, new, all]'} + res: {status: 'Invalid body format', details: '"status[0]" must be one of [validated, new]'} }); }); it('rejects unauthorized requests', done => { diff --git a/src/routes/material.ts b/src/routes/material.ts index a1cf1e5..4fb4bad 100644 --- a/src/routes/material.ts +++ b/src/routes/material.ts @@ -21,7 +21,8 @@ const router = express.Router(); router.get('/materials', (req, res, next) => { if (!req.auth(res, ['read', 'write', 'dev', 'admin'], 'all')) return; - const {error, value: filters} = MaterialValidate.query(req.query); + const {error, value: filters} = + MaterialValidate.query(req.query, ['dev', 'admin'].indexOf(req.authDetails.level) >= 0); if (error) return res400(error, res); let conditions; @@ -38,11 +39,12 @@ router.get('/materials', (req, res, next) => { conditions = {status: globals.status.val}; } - MaterialModel.find(conditions).populate('group_id').populate('supplier_id').lean().exec((err, data) => { + MaterialModel.find(conditions).sort({name: 1}).populate('group_id').populate('supplier_id') + .lean().exec((err, data) => { if (err) return next(err); // validate all and filter null values from validation errors - res.json(_.compact(data.map(e => MaterialValidate.output(e)))); + res.json(_.compact(data.map(e => MaterialValidate.output(e, true)))); }); }); diff --git a/src/routes/validate/material.ts b/src/routes/validate/material.ts index b4532e8..cf5782d 100644 --- a/src/routes/validate/material.ts +++ b/src/routes/validate/material.ts @@ -20,7 +20,10 @@ export default class MaterialValidate { // validate input for material .items( Joi.string() .max(64) - ) + ), + + status: Joi.string() + .valid(...Object.values(globals.status)) }; static input (data, param) { // validate input, set param to 'new' to make all attributes required @@ -47,18 +50,22 @@ export default class MaterialValidate { // validate input for material } } - static output (data) { // validate output and strip unwanted properties, returns null if not valid + static output (data, status = false) { // validate output and strip unwanted properties, returns null if not valid data = IdValidate.stringify(data); data.group = data.group_id.name; data.supplier = data.supplier_id.name; - const {value, error} = Joi.object({ + const validate: any = { _id: IdValidate.get(), name: this.material.name, supplier: this.material.supplier, group: this.material.group, properties: this.material.properties, numbers: this.material.numbers - }).validate(data, {stripUnknown: true}); + }; + if (status) { + validate.status = this.material.status; + } + const {value, error} = Joi.object(validate).validate(data, {stripUnknown: true}); return error !== undefined? null : value; } @@ -83,9 +90,13 @@ export default class MaterialValidate { // validate input for material }); } - static query (data) { + static query (data, dev = false) { + const acceptedStatuses = [globals.status.val, globals.status.new]; + if (dev) { // dev and admin can also access deleted samples + acceptedStatuses.push(globals.status.del) + } return Joi.object({ - status: Joi.string().valid(globals.status.val, globals.status.new, 'all') + status: Joi.array().items(Joi.string().valid(...acceptedStatuses)).default([globals.status.val]) }).validate(data); } } \ No newline at end of file From 8f88023da24308f31acf62438ca37615f30e510f Mon Sep 17 00:00:00 2001 From: VLE2FE Date: Mon, 31 Aug 2020 14:51:43 +0200 Subject: [PATCH 3/4] fixed deleting materials --- src/routes/material.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/routes/material.ts b/src/routes/material.ts index 4fb4bad..a75f73d 100644 --- a/src/routes/material.ts +++ b/src/routes/material.ts @@ -122,7 +122,8 @@ router.delete('/material/' + IdValidate.parameter(), (req, res, next) => { if (!req.auth(res, ['write', 'dev', 'admin'], 'basic')) return; // check if there are still samples referencing this material - SampleModel.find({'material_id': new mongoose.Types.ObjectId(req.params.id)}).lean().exec((err, data) => { + SampleModel.find({'material_id': new mongoose.Types.ObjectId(req.params.id), status: {$ne: globals.status.del}}) + .lean().exec((err, data) => { if (err) return next(err); if (data.length) { return res.status(400).json({status: 'Material still in use'}); From d28e9a1cc92c3c4dc1f00bfd762ec077b6c34472 Mon Sep 17 00:00:00 2001 From: VLE2FE Date: Mon, 31 Aug 2020 16:20:22 +0200 Subject: [PATCH 4/4] changelog delete --- package-lock.json | 32 +++++++++++++++++++------------- package.json | 1 + src/db.ts | 12 ++++++++++++ 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/package-lock.json b/package-lock.json index 210f9a5..c72166e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -385,14 +385,6 @@ "integrity": "sha1-n7OjzzEyMoFR81PeRjLgHlIQK+o=", "dev": true }, - "@szmarczak/http-timer": { - "version": "4.0.5", - "resolved": "https://r.cnpmjs.org/@szmarczak/http-timer/download/@szmarczak/http-timer-4.0.5.tgz", - "integrity": "sha1-v71QIR6d+lG6B9pYoUzf0zMgUVI=", - "requires": { - "defer-to-connect": "^2.0.0" - } - }, "@types/bcrypt": { "version": "3.0.0", "resolved": "https://r.cnpmjs.org/@types/bcrypt/download/@types/bcrypt-3.0.0.tgz", @@ -1297,11 +1289,6 @@ "strip-bom": "^4.0.0" } }, - "defer-to-connect": { - "version": "2.0.0", - "resolved": "https://r.cnpmjs.org/defer-to-connect/download/defer-to-connect-2.0.0.tgz", - "integrity": "sha1-g9axmdsEFZOshNeBtSIjCMz0wsE=" - }, "define-properties": { "version": "1.1.3", "resolved": "https://r.cnpmjs.org/define-properties/download/define-properties-1.1.3.tgz", @@ -2603,6 +2590,15 @@ "resolved": "https://r.cnpmjs.org/nocache/download/nocache-2.1.0.tgz", "integrity": "sha1-Egyf/sQ7Vymx1d6IzXGqdaC6SR8=" }, + "node-cron": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/node-cron/-/node-cron-2.0.3.tgz", + "integrity": "sha512-eJI+QitXlwcgiZwNNSRbqsjeZMp5shyajMR81RZCqeW0ZDEj4zU9tpd4nTh/1JsBiKbF8d08FCewiipDmVIYjg==", + "requires": { + "opencollective-postinstall": "^2.0.0", + "tz-offset": "0.0.1" + } + }, "node-environment-flags": { "version": "1.0.6", "resolved": "https://r.cnpmjs.org/node-environment-flags/download/node-environment-flags-1.0.6.tgz", @@ -2958,6 +2954,11 @@ "resolved": "https://r.cnpmjs.org/openapi-types/download/openapi-types-1.3.5.tgz", "integrity": "sha1-ZxjPvIV/5sbxRx9lsyveu5wQzkA=" }, + "opencollective-postinstall": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/opencollective-postinstall/-/opencollective-postinstall-2.0.3.tgz", + "integrity": "sha512-8AV/sCtuzUeTo8gQK5qDZzARrulB3egtLzFgteqB2tcT4Mw7B8Kt7JcDHmltjz6FOAHsvTevk70gZEbhM4ZS9Q==" + }, "os-tmpdir": { "version": "1.0.2", "resolved": "https://r.cnpmjs.org/os-tmpdir/download/os-tmpdir-1.0.2.tgz", @@ -3936,6 +3937,11 @@ "integrity": "sha1-mNYApevcOPQMsndSLxLcgA6eJfo=", "dev": true }, + "tz-offset": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/tz-offset/-/tz-offset-0.0.1.tgz", + "integrity": "sha512-kMBmblijHJXyOpKzgDhKx9INYU4u4E1RPMB0HqmKSgWG8vEcf3exEfLh4FFfzd3xdQOw9EuIy/cP0akY6rHopQ==" + }, "undefsafe": { "version": "2.0.3", "resolved": "https://r.cnpmjs.org/undefsafe/download/undefsafe-2.0.3.tgz", diff --git a/package.json b/package.json index 2097e66..baed92e 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "json2csv": "^5.0.1", "lodash": "^4.17.15", "mongoose": "^5.8.7", + "node-cron": "^2.0.3", "swagger-ui-dist": "^3.30.2" }, "devDependencies": { diff --git a/src/db.ts b/src/db.ts index 5f4face..2e8592d 100644 --- a/src/db.ts +++ b/src/db.ts @@ -2,12 +2,14 @@ import mongoose from 'mongoose'; import cfenv from 'cfenv'; import _ from 'lodash'; import ChangelogModel from './models/changelog'; +import cron from 'node-cron'; // database urls, prod db url is retrieved automatically const TESTING_URL = 'mongodb://localhost/dfopdb_test'; const DEV_URL = 'mongodb://localhost/dfopdb'; const debugging = true; +const changelogKeepDays = 30; // days to keep the changelog if (process.env.NODE_ENV !== 'production' && debugging) { mongoose.set('debug', true); // enable mongoose debug @@ -78,6 +80,16 @@ export default class db { this.state.db = mongoose.connection; done(); }); + + if (mode !== 'test') { // clear old changelog regularly + cron.schedule('0 0 * * *', () => { + ChangelogModel.deleteMany({_id: {$lt: // id from time + Math.floor(new Date().getTime() / 1000 - changelogKeepDays * 24 * 60 * 60).toString(16) + '0000000000000000' + }}).log({method: 'scheduled changelog delete', url: '', authDetails: {}}).lean().exec(err => { + if (err) console.error(err); + }); + }); + } } static disconnect (done) {