diff --git a/package-lock.json b/package-lock.json index 24c454d6..c6f2226b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@bcgsc-pori/graphkb-api", - "version": "3.13.0", + "version": "3.13.1", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -1778,9 +1778,9 @@ "dev": true }, "clipboard": { - "version": "2.0.6", - "resolved": "https://registry.npmjs.org/clipboard/-/clipboard-2.0.6.tgz", - "integrity": "sha512-g5zbiixBRk/wyKakSwCKd7vQXDjFnAMGHoEyBogG/bw9kTD9GvdAvaoRR1ALcEzt3pVKxZR0pViekPMIS0QyGg==", + "version": "2.0.8", + "resolved": "https://registry.npmjs.org/clipboard/-/clipboard-2.0.8.tgz", + "integrity": "sha512-Y6WO0unAIQp5bLmk1zdThRhgJt/x3ks6f30s3oE3H1mgIEU33XyQjEf8gsf6DxC7NPX8Y1SsNWjUjL/ywLnnbQ==", "dev": true, "optional": true, "requires": { @@ -2412,18 +2412,18 @@ "dev": true }, "docsify": { - "version": "4.11.4", - "resolved": "https://registry.npmjs.org/docsify/-/docsify-4.11.4.tgz", - "integrity": "sha512-Qwt98y6ddM2Wb46gRH/zQpEAvw70AlIpzVlB9Wi2u2T2REg9O+bXMpJ27F5TaRTn2bD6SF1VyZYNUfimpihZwQ==", + "version": "4.12.1", + "resolved": "https://registry.npmjs.org/docsify/-/docsify-4.12.1.tgz", + "integrity": "sha512-7v4UlCYLTmb83leJLIlheQlQ8kDTbTxcpMttRg0Uf92Nl//m0AcKFHoLLo5HHS4UhnO0KhDV8SKCdTR279zI9A==", "dev": true, "requires": { - "dompurify": "^2.0.8", - "marked": "^0.7.0", - "medium-zoom": "^1.0.5", + "dompurify": "^2.2.6", + "marked": "^1.2.9", + "medium-zoom": "^1.0.6", "opencollective-postinstall": "^2.0.2", - "prismjs": "^1.19.0", + "prismjs": "^1.23.0", "strip-indent": "^3.0.0", - "tinydate": "^1.0.0", + "tinydate": "^1.3.0", "tweezer.js": "^1.4.0" } }, @@ -6375,9 +6375,9 @@ } }, "marked": { - "version": "0.7.0", - "resolved": "https://registry.npmjs.org/marked/-/marked-0.7.0.tgz", - "integrity": "sha512-c+yYdCZJQrsRjTPhUx7VKkApw9bwDkNbHUKo1ovgcfDjb2kc8rLuRbIFyXL5WOEUwzSSKo3IXpph2K6DqB/KZg==", + "version": "1.2.9", + "resolved": "https://registry.npmjs.org/marked/-/marked-1.2.9.tgz", + "integrity": "sha512-H8lIX2SvyitGX+TRdtS06m1jHMijKN/XjfH6Ooii9fvxMlh8QdqBfBDkGUpMWH2kQNrtixjzYUa3SH8ROTgRRw==", "dev": true }, "media-typer": { @@ -7245,9 +7245,9 @@ } }, "prismjs": { - "version": "1.20.0", - "resolved": "https://registry.npmjs.org/prismjs/-/prismjs-1.20.0.tgz", - "integrity": "sha512-AEDjSrVNkynnw6A+B1DsFkd6AVdTnp+/WoUixFRULlCLZVRZlVQMVWio/16jv7G1FscUxQxOQhWwApgbnxr6kQ==", + "version": "1.23.0", + "resolved": "https://registry.npmjs.org/prismjs/-/prismjs-1.23.0.tgz", + "integrity": "sha512-c29LVsqOaLbBHuIbsTxaKENh1N2EQBOHaWv7gkHN4dgRbxSREqDnDbtFJYdpPauS4YCplMSNCABQ6Eeor69bAA==", "dev": true, "requires": { "clipboard": "^2.0.0" diff --git a/package.json b/package.json index 94cae2c4..b19f7468 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@bcgsc-pori/graphkb-api", "main": "server.js", - "version": "3.13.0", + "version": "3.13.1", "private": true, "repository": { "type": "git", @@ -42,7 +42,7 @@ "chai-http": "^4.3.0", "commitizen": "^4.2.2", "cz-conventional-changelog": "^3.1.0", - "docsify": "^4.11.4", + "docsify": "^4.12.1", "docsify-cli": "^4.4.1", "eslint": "^5.16.0", "eslint-config-airbnb": "^17.0.0", diff --git a/src/repo/commands/create.js b/src/repo/commands/create.js index b4e65ec5..b5787ddc 100644 --- a/src/repo/commands/create.js +++ b/src/repo/commands/create.js @@ -6,7 +6,7 @@ const { const { logger } = require('../logging'); const { parseRecord } = require('../query_builder'); const { - RecordExistsError, PermissionError, + RecordConflictError, PermissionError, } = require('../error'); const { select, getUserByName, fetchDisplayName } = require('./select'); const { wrapIfTypeError, omitDBAttributes } = require('./util'); @@ -103,20 +103,6 @@ const create = async (db, opt) => { if (model.isEdge) { return createEdge(db, opt); - } if (model.getActiveProperties()) { - // try select before create if active properties are defined (as they may not be db enforceable) - try { - const query = parseRecord(model, content, { activeIndexOnly: true }); - - const records = await select(db, query); - - if (records.length) { - throw new RecordExistsError(`Cannot create the record. Violates the unique constraint (${model.name}.active)`); - } - } catch (err) { - logger.error(err); - throw wrapIfTypeError(err); - } } const newRecordContent = { ...content, createdBy: user['@rid'] }; @@ -136,6 +122,22 @@ const create = async (db, opt) => { } } + if (model.getActiveProperties()) { + // try select before create if active properties are defined (as they may not be db enforceable) + try { + const query = parseRecord(model, record, { activeIndexOnly: true }); + + const records = await select(db, query); + + if (records.length) { + throw new RecordConflictError(`Cannot create the record. Violates the unique constraint (${model.name}.active)`); + } + } catch (err) { + logger.error(err); + throw wrapIfTypeError(err); + } + } + try { if (!content.displayName && model.properties.displayName) { // displayName exists but has not been filled diff --git a/src/repo/commands/update.js b/src/repo/commands/update.js index 7ff467f9..1333fcb2 100644 --- a/src/repo/commands/update.js +++ b/src/repo/commands/update.js @@ -19,12 +19,14 @@ const { logger } = require('./../logging'); const { NotImplementedError, PermissionError, + RecordConflictError, } = require('./../error'); const { omitDBAttributes, wrapIfTypeError, hasRecordAccess, } = require('./util'); const { select, fetchDisplayName } = require('./select'); const { nestedProjection } = require('../query_builder/projection'); +const { parse } = require('../query_builder'); const { checkUserAccessFor } = require('../../middleware/auth'); @@ -331,6 +333,80 @@ const deleteNodeTx = async (db, opt) => { return commit.commit(); }; + +/** + * Check if the record to be deleted is used by some links + * + * @param {orientjs.Db} db database connection object + * @param {ClassModel} model the model of the current record + * @param {string} ridToDelete the recordId being deleted + */ +const deletionLinkChecks = async (db, model, ridToDelete) => { + if (model.name === 'Vocabulary') { + // check variants + let [{ count }] = await select(db, parse({ + count: true, + filters: { + type: ridToDelete, + }, + target: 'Variant', + })); + + if (count > 0) { + throw new RecordConflictError(`Cannot delete ${ridToDelete} since it is used by ${count} Variant records`); + } + // check statements + [{ count }] = await select(db, parse({ + count: true, + filters: { + OR: [ + { conditions: ridToDelete, operator: 'CONTAINS' }, + { evidence: ridToDelete, operator: 'CONTAINS' }, + { subject: ridToDelete }, + ], + }, + target: 'Statement', + })); + + if (count > 0) { + throw new RecordConflictError(`Cannot delete ${ridToDelete} since it is used by ${count} Statement records`); + } + } else if (model.inherits.includes('Ontology')) { + // check variants + let [{ count }] = await select(db, parse({ + count: true, + filters: { + OR: [ + { reference1: ridToDelete }, + { reference2: ridToDelete }, + ], + }, + target: 'Variant', + })); + + if (count > 0) { + throw new RecordConflictError(`Cannot delete ${ridToDelete} since it is used by ${count} Variant records`); + } + // check statements + [{ count }] = await select(db, parse({ + count: true, + filters: { + OR: [ + { conditions: ridToDelete, operator: 'CONTAINS' }, + { evidence: ridToDelete, operator: 'CONTAINS' }, + { subject: ridToDelete }, + ], + }, + target: 'Statement', + })); + + if (count > 0) { + throw new RecordConflictError(`Cannot delete ${ridToDelete} since it is used by ${count} Statement records`); + } + } +}; + + /** * uses a transaction to copy the current record into a new record * then update the actual current record (to preserve links) @@ -364,6 +440,13 @@ const modify = async (db, opt) => { if (!hasRecordAccess(user, original)) { throw new PermissionError(`The user '${user.name}' does not have sufficient permissions to interact with record ${original['@rid']}`); } + + // check for outstanding links before deleting + if (opt.changes === null) { + await deletionLinkChecks(db, model, original['@rid'].toString()); + } + + // now delete the record const changes = opt.changes === null ? null : Object.assign({}, model.formatRecord(opt.changes, { diff --git a/src/repo/commands/util.js b/src/repo/commands/util.js index 7c6473a8..5a9642a0 100644 --- a/src/repo/commands/util.js +++ b/src/repo/commands/util.js @@ -6,7 +6,7 @@ const { util: { castToRID } } = require('@bcgsc-pori/graphkb-schema'); const { NoRecordFoundError, - RecordExistsError, + RecordConflictError, DatabaseConnectionError, DatabaseRequestError, } = require('../error'); @@ -22,7 +22,7 @@ const wrapIfTypeError = (err) => { const type = err.type.toLowerCase(); if (type.includes('orecordduplicatedexception')) { - return new RecordExistsError(err); + return new RecordConflictError(err); } if (type.includes('orecordnotfoundexception')) { return new NoRecordFoundError(err); } if (type.includes('odatabaseexception')) { diff --git a/src/repo/error.js b/src/repo/error.js index f7e5d12f..444e84fa 100644 --- a/src/repo/error.js +++ b/src/repo/error.js @@ -22,7 +22,7 @@ class AuthenticationError extends ErrorMixin {} class MultipleRecordsFoundError extends ErrorMixin {} -class RecordExistsError extends ErrorMixin {} +class RecordConflictError extends ErrorMixin {} class NotImplementedError extends ErrorMixin {} @@ -46,5 +46,5 @@ module.exports = { NotImplementedError, ParsingError, PermissionError, - RecordExistsError, + RecordConflictError, }; diff --git a/src/repo/index.js b/src/repo/index.js index 40ecf67b..a3dcddee 100644 --- a/src/repo/index.js +++ b/src/repo/index.js @@ -6,7 +6,7 @@ const { logger } = require('./logging'); const { loadSchema, createSchema } = require('./schema'); const { migrate } = require('./migrate'); const { createUser, update, getUserByName } = require('./commands'); -const { RecordExistsError } = require('./error'); +const { RecordConflictError } = require('./error'); const { parseRecord } = require('./query_builder'); @@ -140,7 +140,7 @@ const connectDB = async ({ userName: process.env.USER, }); } catch (err) { - if (!(err instanceof RecordExistsError)) { + if (!(err instanceof RecordConflictError)) { logger.log('error', `Error in creating the current user ${err}`); } } diff --git a/src/repo/query_builder/index.js b/src/repo/query_builder/index.js index 352eb209..b4b2998e 100644 --- a/src/repo/query_builder/index.js +++ b/src/repo/query_builder/index.js @@ -136,7 +136,12 @@ class WrapperQuery { } } - +/** + * Given some input record, create a query to find it + * + * @param {object} query JSON query object + * + */ const parse = query => WrapperQuery.parse(query); /** diff --git a/src/routes/error.js b/src/routes/error.js index 4b75891a..6bb5a31e 100644 --- a/src/routes/error.js +++ b/src/routes/error.js @@ -5,7 +5,7 @@ const { error: { AttributeError } } = require('@bcgsc-pori/graphkb-schema'); const { DatabaseConnectionError, NoRecordFoundError, - RecordExistsError, + RecordConflictError, AuthenticationError, PermissionError, } = require('./../repo/error'); @@ -32,7 +32,7 @@ const addErrorRoute = (app) => { code = HTTP_STATUS.BAD_REQUEST; } else if (err instanceof NoRecordFoundError) { code = HTTP_STATUS.NOT_FOUND; - } else if (err instanceof RecordExistsError) { + } else if (err instanceof RecordConflictError) { code = HTTP_STATUS.CONFLICT; } else if (err instanceof DatabaseConnectionError) { logger.warn('connection error, attempting to restart the database connection'); diff --git a/src/routes/openapi/index.js b/src/routes/openapi/index.js index 9c7ea873..752127d0 100644 --- a/src/routes/openapi/index.js +++ b/src/routes/openapi/index.js @@ -213,7 +213,7 @@ const describePost = (model) => { 400: { $ref: '#/components/responses/BadInput' }, 401: { $ref: '#/components/responses/NotAuthorized' }, 403: { $ref: '#/components/responses/Forbidden' }, - 409: { $ref: '#/components/responses/RecordExistsError' }, + 409: { $ref: '#/components/responses/RecordConflictError' }, }, summary: `create a new ${model.name} record`, tags: [model.name], @@ -265,7 +265,7 @@ const describeOperationByID = (model, operation = 'delete') => { }; if (operation !== 'delete') { - description.responses[409] = { $ref: '#/components/responses/RecordExistsError' }; + description.responses[409] = { $ref: '#/components/responses/RecordConflictError' }; } if (operation === 'get') { description.parameters.push({ $ref: '#/components/parameters/neighbors' }); diff --git a/src/routes/openapi/responses.js b/src/routes/openapi/responses.js index a11b2e8d..7727ea7d 100644 --- a/src/routes/openapi/responses.js +++ b/src/routes/openapi/responses.js @@ -26,12 +26,12 @@ const NotAuthorized = { }, description: 'Authorization failed or insufficient permissions were found', }; -const RecordExistsError = { +const RecordConflictError = { content: { 'application/json': { schema: { $ref: '#/components/schemas/Error', - properties: { name: { example: 'RecordExistsError' } }, + properties: { name: { example: 'RecordConflictError' } }, }, }, }, @@ -61,5 +61,5 @@ const RecordNotFound = { }; module.exports = { - BadInput, Forbidden, NotAuthorized, RecordExistsError, RecordNotFound, + BadInput, Forbidden, NotAuthorized, RecordConflictError, RecordNotFound, }; diff --git a/test/db_integration/crud.test.js b/test/db_integration/crud.test.js index 4a0508a7..d1754ffc 100644 --- a/test/db_integration/crud.test.js +++ b/test/db_integration/crud.test.js @@ -7,7 +7,7 @@ const { select, } = require('../../src/repo/commands'); const { - RecordExistsError, AttributeError, NotImplementedError, + RecordConflictError, AttributeError, NotImplementedError, } = require('../../src/repo/error'); const { parseRecord, @@ -81,7 +81,7 @@ describeWithAuth('CRUD operations', () => { try { await create(session, { content: { name: 'alice' }, model: schema.User, user: db.admin }); } catch (err) { - expect(err).toBeInstanceOf(RecordExistsError); + expect(err).toBeInstanceOf(RecordConflictError); return; } throw new Error('Did not throw expected error'); @@ -105,7 +105,7 @@ describeWithAuth('CRUD operations', () => { { content: { name: original.name }, model: schema.User, user: db.admin }, ); } catch (err) { - expect(err).toBeInstanceOf(RecordExistsError); + expect(err).toBeInstanceOf(RecordConflictError); return; } throw new Error('Did not throw expected error'); @@ -532,7 +532,7 @@ describeWithAuth('CRUD operations', () => { }, ); - // throws RecordExistsError on next create call + // throws RecordConflictError on next create call try { await create( session, @@ -548,7 +548,7 @@ describeWithAuth('CRUD operations', () => { }, ); } catch (err) { - expect(err).toBeInstanceOf(RecordExistsError); + expect(err).toBeInstanceOf(RecordConflictError); return; } throw new Error('Did not throw the expected error');