From febb65a513bc282221c0554825aac8fa9a26474d Mon Sep 17 00:00:00 2001 From: Stefan Zermatten Date: Fri, 8 Mar 2019 13:57:24 +0200 Subject: [PATCH] Re-wrote parenting, should be significantly faster, more maintainable --- .../api/creature/properties/Actions.js | 18 +- .../api/creature/subSchemas/PropertySchema.js | 16 ++ app/imports/api/parenting.js | 251 ------------------ app/imports/api/parenting/ChildSchema.js | 37 +++ .../api/parenting/SoftRemovableSchema.js | 22 ++ app/imports/api/parenting/fetchDocByRef.js | 12 + .../api/parenting/getCollectionByName.js | 11 + .../parenting/getInheritPropertiesSchema.js | 16 ++ app/imports/api/parenting/parenting.js | 165 ++++++++++++ app/imports/api/parenting/softRemove.js | 55 ++++ 10 files changed, 339 insertions(+), 264 deletions(-) create mode 100644 app/imports/api/creature/subSchemas/PropertySchema.js delete mode 100644 app/imports/api/parenting.js create mode 100644 app/imports/api/parenting/ChildSchema.js create mode 100644 app/imports/api/parenting/SoftRemovableSchema.js create mode 100644 app/imports/api/parenting/fetchDocByRef.js create mode 100644 app/imports/api/parenting/getCollectionByName.js create mode 100644 app/imports/api/parenting/getInheritPropertiesSchema.js create mode 100644 app/imports/api/parenting/parenting.js create mode 100644 app/imports/api/parenting/softRemove.js diff --git a/app/imports/api/creature/properties/Actions.js b/app/imports/api/creature/properties/Actions.js index f134768f..3813ec5e 100644 --- a/app/imports/api/creature/properties/Actions.js +++ b/app/imports/api/creature/properties/Actions.js @@ -1,8 +1,8 @@ -import SimpleSchema from 'simpl-schema'; import schema from '/imports/api/schema.js'; -import {makeChild} from '/imports/api/parenting.js'; import AdjustmentSchema from '/imports/api/creature/subSchemas/AdjustmentSchema.js'; import DamageSchema from '/imports/api/creature/subSchemas/DamageSchema.js'; +import PropertySchema from '/imports/api/creature/subSchemas/PropertySchema.js'; +import ChildSchema from '/imports/api/parenting/ChildSchema.js'; let Actions = new Mongo.Collection('actions'); @@ -10,20 +10,11 @@ let Actions = new Mongo.Collection('actions'); * Actions are given to a character by items and features */ let actionSchema = schema({ - charId: { - type: String, - regEx: SimpleSchema.RegEx.Id, - index: 1, - }, name: { type: String, optional: true, trim: false, }, - enabled: { - type: Boolean, - defaultValue: true, - }, description: { type: String, optional: true, @@ -52,8 +43,9 @@ let actionSchema = schema({ }); Actions.attachSchema(actionSchema); +Actions.attachSchema(PropertySchema); +Actions.attachSchema(ChildSchema); -// Actions.attachBehaviour('softRemovable'); -makeChild(Actions, ["name", "enabled"]); +// makeChild(Actions, ["name", "enabled"]); export default Actions diff --git a/app/imports/api/creature/subSchemas/PropertySchema.js b/app/imports/api/creature/subSchemas/PropertySchema.js new file mode 100644 index 00000000..c4727e57 --- /dev/null +++ b/app/imports/api/creature/subSchemas/PropertySchema.js @@ -0,0 +1,16 @@ +import SimpleSchema from 'simpl-schema'; +import schema from '/imports/api/schema.js'; + +const PropertySchema = schema({ + charId: { + type: String, + regEx: SimpleSchema.RegEx.Id, + index: 1, + }, + enabled: { + type: Boolean, + defaultValue: true, + }, +}); + +export default PropertySchema; diff --git a/app/imports/api/parenting.js b/app/imports/api/parenting.js deleted file mode 100644 index aff34587..00000000 --- a/app/imports/api/parenting.js +++ /dev/null @@ -1,251 +0,0 @@ -import SimpleSchema from 'simpl-schema'; -import schema from '/imports/api/schema.js'; -import { ValidatedMethod } from 'meteor/mdg:validated-method'; -import { _ } from 'meteor/underscore'; - -let childSchema = schema({ - parent: {type: Object}, - "parent.collection": {type: String}, - "parent.id": {type: String, regEx: SimpleSchema.RegEx.Id, index: 1}, - "parent.group": {type: String, optional: true}, - "removedWith": { - optional: true, - type: String, - regEx: SimpleSchema.RegEx.Id, - }, -}); - -let joinWithDefaultKeys = function(keys){ - let defaultKeys = [ - "charId", - ]; - return _.union(keys, defaultKeys); -}; - -let limitModifierToKeys = function(modifier, keys){ - if (!modifier) return; - modifier = _.pick(modifier, ["$set", "$unset"]); - if (modifier.$set) modifier.$set = _.pick(modifier.$set, keys); - if (modifier.$unset) modifier.$unset = _.pick(modifier.$unset, keys); - if (_.isEmpty(modifier.$set)) delete modifier.$set; - if (_.isEmpty(modifier.$unset)) delete modifier.$unset; - return modifier; -}; - -let getParent = function(doc){ - if (!doc || !doc.parent) return; - let parentCol = Meteor.isClient ? - window[doc.parent.collection] : global[doc.parent.collection]; - if (parentCol) - return parentCol.findOne(doc.parent.id, {removed: true}); -}; - -let inheritParentProperties = function(doc, collection){ - let parent = getParent(doc); - if (!parent) throw new Meteor.Error( - "Parenting Error", - "Document's parent does not exist" - ); - let handMeDowns = _.pick(parent, collection.inheritedKeys); - if ( - _.contains(collection.inheritedKeys, "charId") && - doc.parent.collection === "Characters" - ){ - handMeDowns.charId = doc.parent.id; - } - if (_.isEmpty(handMeDowns)) return; - collection.update(doc._id, {$set: handMeDowns}); -}; - -let childCollections = []; - -let makeChild = function(collection, inheritedKeys){ - inheritedKeys = inheritedKeys || []; - if (inheritedKeys) { - collection.inheritedKeys = joinWithDefaultKeys(inheritedKeys); - } - collection.helpers({ - //returns the parent even if it's removed - getParent: function(){ - return getParent(this); - }, - getParentCollection: function(){ - return Meteor.isClient ? - window[this.parent.collection] : global[this.parent.collection]; - }, - }); - - //when created, inherit parent properties - collection.after.insert(function(userId, doc){ - inheritParentProperties(doc, collection); - }); - - collection.before.update(function(userId, doc, fieldNames, modifier, options){ - //if we are restoring this asset, unmark that it was removed with its parent, we no longer care - if (modifier && modifier.$unset && modifier.$unset.removed) { - modifier.$unset.removedWith = ""; - } - }); - - collection.after.update(function(userId, doc, fieldNames, modifier, options) { - if (modifier && modifier.$set && modifier.$set["parent.id"]){ - //when we change parents, inherit its properties - inheritParentProperties(doc, collection); - } - }); - - collection.softRemoveNode = collection.softRemoveNode || function(id){ - collection.softRemove(id); - }; - - collection.restoreNode = collection.restoreNode || function(id){ - collection.restore(id); - }; - - collection.attachSchema(childSchema); - - childCollections.push(collection); -}; - -let makeParent = function(collection, donatedKeys){ - donatedKeys = joinWithDefaultKeys(donatedKeys); - let collectionName = collection._collection.name; - //after changing, push the changes to all children - collection.after.update(function(userId, doc, fieldNames, modifier, options) { - modifier = limitModifierToKeys(modifier, donatedKeys); - doc = _.pick(doc, ["_id", "charId"]); - if (!modifier) return; - Meteor.call("updateChildren", doc, modifier, true); - }); - collection.softRemoveNode = function(id){ - Meteor.call("softRemoveNode", collectionName, id); - }; - - collection.restoreNode = function(id){ - Meteor.call("restoreNode", collectionName, id); - }; - - if (Meteor.isServer) collection.after.remove(function(userId, doc) { - _.each(childCollections, function(collection){ - collection.remove( - {"parent.id": doc._id} - ); - }); - }); -}; - -let checkPermission = function(userId, charId){ - let char = Characters.findOne(charId, {fields: {owner: 1, writers: 1}}); - if (!char) - throw new Meteor.Error("Access Denied, no charId", - "Character " + charId + " does not exist"); - if (!userId) - throw new Meteor.Error("Access Denied, no userId", - "No UserId set when trying to update character asset."); - if (char.owner !== userId && !_.contains(char.writers, userId)) - throw new Meteor.Error("Access Denied, not permitted", - "Not permitted to update assets of this character."); - return true; -}; - -let cascadeSoftRemove = function(id, removedWithId){ - _.each(childCollections, function(treeCollection){ - treeCollection.update( - {"parent.id": id}, - {$set: { - removed: true, - removedWith: removedWithId, - }}, - {multi: true} - ); - treeCollection.find({"parent.id": id}).forEach(function(doc){ - cascadeSoftRemove(doc._id, removedWithId); - }); - }); -}; - -let checkRemovePermission = function(collectionName, id, self){ - check(collectionName, String); - check(id, String); - let collection = Mongo.Collection.get(collectionName); - let node = collection.findOne(id); - let charId = node && node.charId; - checkPermission(self.userId, charId); -}; - -const softRemoveNode = new ValidatedMethod({ - name: "parenting.methods.softRemoveNode", - - validate: schema({ - collectionName: {type: String,}, - id: { - type: String, - regEx: SimpleSchema.RegEx.Id, - }, - }).validator(), // argument validation - - run({collectionName, id}){ - checkRemovePermission(collectionName, id, this); - let collection = Mongo.Collection.get(collectionName); - collection.softRemove(id); - cascadeSoftRemove(id, id); - }, -}); - -const restoreNode = new ValidatedMethod({ - name: "parenting.methods.restoreNode", - validate: null, - run(collectionName, id){ - checkRemovePermission(collectionName, id, this); - let collection = Mongo.Collection.get(collectionName); - collection.restore(id); - _.each(childCollections, function(treeCollection){ - treeCollection.update( - {removedWith: id, removed: true}, - {$unset: {removed: true, removedWith: ""}}, - {multi: true} - ); - }); - }, -}); - -const updateChildren = new ValidatedMethod({ - name: "parenting.methods.updateChildren", - validate: null, - run({parent, modifier, limitToInheritance}){ - check(parent, {_id: String, charId: String}); - check(modifier, Object); - checkPermission(this.userId, parent.charId); - let selector = {"parent.id": parent._id}; - _.each(childCollections, function(collection){ - let thisModifier; - if (limitToInheritance){ - thisModifier = limitModifierToKeys(modifier, collection.inheritedKeys); - } else { - thisModifier = _.clone(modifier); - } - if (_.isEmpty(thisModifier)) return; - collection.update(selector, thisModifier, {multi: true, removed: true}); - }); - }, -}); - -const cloneChildren = new ValidatedMethod({ - name: "parenting.methods.cloneChildren", - validate: null, - run({objectId, newParent}){ - check(objectId, String); - check(newParent, {id: String, collection: String}); - - _.each(childCollections, function(collection){ - let keys = collection.simpleSchema().objectKeys(); - collection.find({"parent.id": objectId}).forEach(function(doc){ - let newDoc = _.pick(doc, keys); - newDoc.parent = newParent; - collection.insert(newDoc); - }); - }); - } -}) - -export {makeChild, makeParent, softRemoveNode}; diff --git a/app/imports/api/parenting/ChildSchema.js b/app/imports/api/parenting/ChildSchema.js new file mode 100644 index 00000000..9bf0d971 --- /dev/null +++ b/app/imports/api/parenting/ChildSchema.js @@ -0,0 +1,37 @@ +import schema from '/imports/api/schema.js'; + +const refSchema = new SimpleSchema({ + id: { + type: String, + regEx: SimpleSchema.RegEx.Id, + index: 1 + }, + collection: { + type: String + }, + name: { + type: String, + optional: true, + }, + enabled: { + type: Boolean, + optional: true, + index: 1, + }, +}); + +let childSchema = schema({ + parent: { + type: refSchema, + optional: true, + }, + ancestors: { + type: Array, + defaultValue: [], + }, + 'ancestors.$': { + type: refSchema, + }, +}); + +export default childSchema; diff --git a/app/imports/api/parenting/SoftRemovableSchema.js b/app/imports/api/parenting/SoftRemovableSchema.js new file mode 100644 index 00000000..15634ebb --- /dev/null +++ b/app/imports/api/parenting/SoftRemovableSchema.js @@ -0,0 +1,22 @@ +import schema from '/imports/api/schema.js'; + +let SoftRemovableSchema = schema({ + "removed": { + type: Boolean, + optional: true, + index: 1, + }, + "removedAt": { + type: Date, + optional: true, + index: 1, + }, + "removedWith": { + optional: true, + type: String, + regEx: SimpleSchema.RegEx.Id, + index: 1, + }, +}); + +export default SoftRemovableSchema; diff --git a/app/imports/api/parenting/fetchDocByRef.js b/app/imports/api/parenting/fetchDocByRef.js new file mode 100644 index 00000000..23766707 --- /dev/null +++ b/app/imports/api/parenting/fetchDocByRef.js @@ -0,0 +1,12 @@ +import getCollectionByName from '/imports/api/parenting/getCollectionByName.js'; + +const docNotFoundError = function({id, collection}){ + throw new Meteor.Error('document-not-found', + `No document could be found with id: ${id} in ${collection}` + ); +}; + +export default function fetchDocByRef({id, collection}, options){ + return getCollectionByName(collection).findOne(id, options) || + docNotFoundError({id, collection}); +}; diff --git a/app/imports/api/parenting/getCollectionByName.js b/app/imports/api/parenting/getCollectionByName.js new file mode 100644 index 00000000..bc9fe4e0 --- /dev/null +++ b/app/imports/api/parenting/getCollectionByName.js @@ -0,0 +1,11 @@ +const collectionDoesntExistError = function(collectionName){ + throw new Meteor.Error('bad-collection-reference', + `Parent references collection ${collectionName}, which does not exist` + ); +}; + +const getCollectionByName = function(name){ + return Mongo.Collection.get(name) || collectionDoesntExistError(name); +}; + +export default getCollectionByName; diff --git a/app/imports/api/parenting/getInheritPropertiesSchema.js b/app/imports/api/parenting/getInheritPropertiesSchema.js new file mode 100644 index 00000000..41f53b4d --- /dev/null +++ b/app/imports/api/parenting/getInheritPropertiesSchema.js @@ -0,0 +1,16 @@ +import schema from '/imports/api/schema.js'; + +let getInheritPropertiesSchema = function(keys){ + let options = { + 'parent.properties': { + type: Object, + optional: true, + }, + }; + for (let key in keys){ + options[`parent.properties.${key}`] = keys[key]; + } + return schema(options); +}; + +export default getInheritPropertiesSchema; diff --git a/app/imports/api/parenting/parenting.js b/app/imports/api/parenting/parenting.js new file mode 100644 index 00000000..682d4661 --- /dev/null +++ b/app/imports/api/parenting/parenting.js @@ -0,0 +1,165 @@ +import fetchDocByRef from '/imports/api/parenting/fetchDocByRef.js'; +import getCollectionByName from 'app/imports/api/parenting/getCollectionByName.js'; + +// n = collections.length +let collections = []; + +export function registerCollection(collectionName){ + collections.push(collectionName); +}; + +// 1 database hit to get the parent by reference +export function fetchParent({id, collection}){ + return fetchDocByRef({id, collection}); +}; + +// n database hits to get the children by parent id +export function fetchChildren({parentId, filter = {}, options}){ + filter["parent.id"] = parentId; + let children = []; + collections.forEach(collection => { + children.push( + ...collection.find({ + "parent.id": parentId + }, options).fetch() + ); + }); + return children; +} + +// n database hits to update the decendents +export function updateChildren({parentId, filter = {}, modifier, options={}}){ + filter["parent.id"] = parentId; + options.multi = true; + collections.forEach(collection => { + collection.update(filter, modifier, options); + }); +}; + +// n database hits to fetch the decendents by ancestor id, in no particular order +export function fetchDecendents({ancestorId, filter = {}, options}){ + filter["ancestors.id"] = ancestorId; + let decendents = []; + collections.forEach(collection => { + decendents.push(...collection.find(filter, options).fetch()); + }); + return decendents; +}; + +// n database hits to update the decendents +export function updateDecendents({ancestorId, filter = {}, modifier, options={}}){ + filter["ancestors.id"] = ancestorId; + options.multi = true; + collections.forEach(collection => { + collection.update(filter, modifier, options); + }); +}; + +// n database hits to get decendents to act on +export function forEachDecendent({ancestorId, filter = {}, options}, callback){ + filter["ancestors.id"] = ancestorId; + collections.forEach(collection => { + collection.find(filter, options).forEach(callback); + }); +}; + +// 1 database read +export function getParenting({id, collection}){ + + // Get the parent ref + let parentDoc = fetchDocByRef({id, collection}, {fields: { + name: 1, + enabled: 1, + ancestors: 1, + }}); + let parent = { + id, + collection, + name: parentDoc.name, + enabled: parentDoc.enabled, + }; + + // Ancestors is [...parent's ancestors, parent ref] + let ancestors = parentDoc.ancestors; + ancestors.push(parent); + + return {parent, ancestors}; +} + +export function updateParent(docRef, parentRef){ + let collection = getCollectionByName(docRef.collection); + let oldDoc = fetchDocByRef(docRef, {fields: { + parent: 1, + ancestors: 1, + }}); + + // Skip if we aren't changing the parent id + if (oldDoc.parent.id === parentRef.id) return; + + // update the document's parenting + let {parent, ancestors} = getParenting(parentRef); + collection.update(docRef.id, {$set: {parent, ancestors}}); + + // Remove the old ancestors from the decendents + updateDecendents({ + ancestorId: docRef.id, + modifier: {$pullAll: { + ancestors: oldDoc.ancestors, + }}, + }); + + // Add the new ancestors to the decendents + updateDecendents({ + ancestorId: docRef.id, + modifier: {$push: { + ancestors: { + $each: ancestors, + $position: 0, + }, + }}, + }); +}; + +export function setInheritedField({id, collection, fieldName, fieldValue}){ + + // Update the doc + let collection = getCollectionByName(collection); + collection.update(id, {$set: { + [`${fieldName}`]: fieldValue, + }}); + + // Update the parent object of its children + updateChildren({ + parentId: id, + modifier: {$set: { + [`parent.${fieldName}`]: fieldValue, + }}, + }); + + // Update the ancestors object of its decendents + updateDecendents({ + ancestorId: id, + modifier: {$set: { + [`ancestors.$.${fieldName}`]: fieldValue, + }}, + }); + +}; + +export function setEnabled({id, collection, enabled}){ + setInheritedField({ + id, + collection, + fieldName: 'enabled', + fieldValue: enabled, + }); +}; + +export function setName({id, collection, name}){ + setInheritedField({ + id, + collection, + fieldName: 'name', + fieldValue: name, + }); +}; diff --git a/app/imports/api/parenting/softRemove.js b/app/imports/api/parenting/softRemove.js new file mode 100644 index 00000000..a64053de --- /dev/null +++ b/app/imports/api/parenting/softRemove.js @@ -0,0 +1,55 @@ +import getCollectionByName from '/imports/api/parenting/getCollectionByName.js'; +import updateDecendents from '/imports/api/parenting/parenting.js'; + +// 1 + n database hits +export function softRemove({id, collection}){ + let removalDate = new Date(); + // Remove this document + collection = getCollectionByName(collection); + collection.update(id, {$set: { + removed: true, + removedAt: removalDate, + }, $unset: { + removedWith: 1, + }}); + // Remove all the decendents that have not yet been removed, and set them to be + // removed with this document + updateDecendents({ + ancestorId: id, + filter: {removed: {$ne: true}}, + modifier: {$set: { + removed: true, + removedAt: removalDate, + removedWith: id, + }}, + }); +}; + +const restoreError = function(){ + throw new Meteor.Error('restore-failed', + 'Could not restore this document, maybe it was removed by a parent?' + ); +}; + +export function restore({id, collection}){ + collection = getCollectionByName(collection); + let numUpdated = collection.update({ + _id: id, + removedWith: {$exists: false} + }, { $unset: { + removed: 1, + removedAt: 1, + }}); + if (numUpdated === 0) restoreError(); + updateDecendents({ + ancestorId: id, + filter: { + removedWith: id, + }, + modifier: { $unset: { + removed: 1, + removedAt: 1, + removedWith: 1, + }}, + }); +}