From 8df3a3f8de86be919c6ff9f18d2e22f0761610c3 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 12 Oct 2020 16:37:08 +0100 Subject: [PATCH 1/3] Fixes for deleting records when a table is deleted. --- packages/server/src/api/controllers/model.js | 8 ++++---- packages/server/src/db/utils.js | 21 ++++++++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/packages/server/src/api/controllers/model.js b/packages/server/src/api/controllers/model.js index 87a2449f76..74e3494eba 100644 --- a/packages/server/src/api/controllers/model.js +++ b/packages/server/src/api/controllers/model.js @@ -105,11 +105,8 @@ exports.save = async function(ctx) { exports.destroy = async function(ctx) { const instanceId = ctx.user.instanceId const db = new CouchDB(instanceId) - const modelToDelete = await db.get(ctx.params.modelId) - await db.remove(modelToDelete) - // Delete all records for that model const records = await db.allDocs( getRecordParams(ctx.params.modelId, null, { @@ -117,7 +114,7 @@ exports.destroy = async function(ctx) { }) ) await db.bulkDocs( - records.rows.map(record => ({ _id: record.id, _deleted: true })) + records.rows.map(record => ({ ...record.doc, _deleted: true })) ) // update linked records @@ -127,6 +124,9 @@ exports.destroy = async function(ctx) { model: modelToDelete, }) + // don't remove the table itself until very end + await db.remove(modelToDelete) + ctx.eventEmitter && ctx.eventEmitter.emitModel(`model:delete`, instanceId, modelToDelete) ctx.status = 200 diff --git a/packages/server/src/db/utils.js b/packages/server/src/db/utils.js index a167966a4e..56572e1170 100644 --- a/packages/server/src/db/utils.js +++ b/packages/server/src/db/utils.js @@ -57,21 +57,26 @@ exports.generateModelID = () => { /** * Gets the DB allDocs/query params for retrieving a record. - * @param {string} modelId The model in which the records have been stored. + * @param {string|null} modelId The model in which the records have been stored. * @param {string|null} recordId The ID of the record which is being specifically queried for. This can be * left null to get all the records in the model. * @param {object} otherProps Any other properties to add to the request. * @returns {object} Parameters which can then be used with an allDocs request. */ -exports.getRecordParams = (modelId, recordId = null, otherProps = {}) => { +exports.getRecordParams = ( + modelId = null, + recordId = null, + otherProps = {} +) => { if (modelId == null) { - throw "Cannot build params for records without a model ID" + return getDocParams(DocumentTypes.RECORD, null, otherProps) + } else { + const endOfKey = + recordId == null + ? `${modelId}${SEPARATOR}` + : `${modelId}${SEPARATOR}${recordId}` + return getDocParams(DocumentTypes.RECORD, endOfKey, otherProps) } - const endOfKey = - recordId == null - ? `${modelId}${SEPARATOR}` - : `${modelId}${SEPARATOR}${recordId}` - return getDocParams(DocumentTypes.RECORD, endOfKey, otherProps) } /** From 352ff82885ea40efa296a3b19a8ba7abc905fc22 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 12 Oct 2020 16:51:41 +0100 Subject: [PATCH 2/3] Improving consistency of model saving, making sure that any validation which could fail happens before any updates are carried out. --- packages/server/src/api/controllers/model.js | 17 ++++++++++------- .../src/db/linkedRecords/LinkController.js | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/server/src/api/controllers/model.js b/packages/server/src/api/controllers/model.js index 74e3494eba..d9277c2f3d 100644 --- a/packages/server/src/api/controllers/model.js +++ b/packages/server/src/api/controllers/model.js @@ -33,6 +33,7 @@ exports.save = async function(ctx) { views: {}, ...rest, } + let renameDocs = [] // if the model obj had an _id then it will have been retrieved const oldModel = ctx.preExisting @@ -49,14 +50,11 @@ exports.save = async function(ctx) { include_docs: true, }) ) - - const docs = records.rows.map(({ doc }) => { + renameDocs = records.rows.map(({ doc }) => { doc[_rename.updated] = doc[_rename.old] delete doc[_rename.old] return doc }) - - await db.bulkDocs(docs) delete modelToSave._rename } @@ -69,9 +67,6 @@ exports.save = async function(ctx) { modelView.schema = modelToSave.schema } - const result = await db.post(modelToSave) - modelToSave._rev = result.rev - // update linked records await linkRecords.updateLinks({ instanceId, @@ -82,6 +77,14 @@ exports.save = async function(ctx) { oldModel: oldModel, }) + // don't perform any updates until relationships have been + // checked by the updateLinks function + if (renameDocs.length !== 0) { + await db.bulkDocs(renameDocs) + } + const result = await db.post(modelToSave) + modelToSave._rev = result.rev + ctx.eventEmitter && ctx.eventEmitter.emitModel(`model:save`, instanceId, modelToSave) diff --git a/packages/server/src/db/linkedRecords/LinkController.js b/packages/server/src/db/linkedRecords/LinkController.js index b006e798c2..fc28320d70 100644 --- a/packages/server/src/db/linkedRecords/LinkController.js +++ b/packages/server/src/db/linkedRecords/LinkController.js @@ -161,7 +161,7 @@ class LinkController { }) // now add the docs to be deleted to the bulk operation operations.push(...toDeleteDocs) - // replace this field with a simple entry to denote there are links + // remove the field from this row, link doc will be added to record on way out delete record[fieldName] } } From 1dea4f24b730fdddc5390d1b3f25764438500116 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 12 Oct 2020 17:02:52 +0100 Subject: [PATCH 3/3] Handling empty relationship column the same way other columns are handled, it won't do anything until it is valid - but doesn't error. --- .../src/db/linkedRecords/LinkController.js | 10 +++++++++- packages/server/src/db/linkedRecords/index.js | 17 +++++++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/server/src/db/linkedRecords/LinkController.js b/packages/server/src/db/linkedRecords/LinkController.js index fc28320d70..48e459a1cb 100644 --- a/packages/server/src/db/linkedRecords/LinkController.js +++ b/packages/server/src/db/linkedRecords/LinkController.js @@ -234,8 +234,16 @@ class LinkController { for (let fieldName of Object.keys(schema)) { const field = schema[fieldName] if (field.type === "link") { + // handle this in a separate try catch, want + // the put to bubble up as an error, if can't update + // table for some reason + let linkedModel + try { + linkedModel = await this._db.get(field.modelId) + } catch (err) { + continue + } // create the link field in the other model - const linkedModel = await this._db.get(field.modelId) linkedModel.schema[field.fieldName] = { name: field.fieldName, type: "link", diff --git a/packages/server/src/db/linkedRecords/index.js b/packages/server/src/db/linkedRecords/index.js index ba93f3d2e8..09a58c1062 100644 --- a/packages/server/src/db/linkedRecords/index.js +++ b/packages/server/src/db/linkedRecords/index.js @@ -42,6 +42,7 @@ exports.updateLinks = async function({ model, oldModel, }) { + const baseReturnObj = record == null ? model : record if (instanceId == null) { throw "Cannot operate without an instance ID." } @@ -50,12 +51,16 @@ exports.updateLinks = async function({ arguments[0].modelId = model._id } let linkController = new LinkController(arguments[0]) - if ( - !(await linkController.doesModelHaveLinkedFields()) && - (oldModel == null || - !(await linkController.doesModelHaveLinkedFields(oldModel))) - ) { - return record + try { + if ( + !(await linkController.doesModelHaveLinkedFields()) && + (oldModel == null || + !(await linkController.doesModelHaveLinkedFields(oldModel))) + ) { + return baseReturnObj + } + } catch (err) { + return baseReturnObj } switch (eventType) { case EventType.RECORD_SAVE: