Merge pull request #19 from bcgsc/release/v3.13.1

Release/v3.13.1
bcgsc · Mar 18, 2021 · 4e13930 · 4e13930
2 parents 7452b0f + 6460882
commit 4e13930
Show file tree

Hide file tree

Showing 12 changed files with 144 additions and 54 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,
 };