From 9214529284c4cc3308f5f14144785f7c6896336a Mon Sep 17 00:00:00 2001 From: Thaum Rystra Date: Mon, 18 May 2020 02:03:14 +0200 Subject: [PATCH] rewrote entire ordering structure for ancestor trees --- app/imports/api/parenting/ChildSchema.js | 1 - .../getDescendantsInDepthFirstOrder.js | 27 ++++ app/imports/api/parenting/order.js | 128 ++++++------------ app/imports/api/parenting/organizeMethods.js | 30 ++-- app/imports/api/parenting/parenting.js | 22 +-- 5 files changed, 103 insertions(+), 105 deletions(-) create mode 100644 app/imports/api/parenting/getDescendantsInDepthFirstOrder.js diff --git a/app/imports/api/parenting/ChildSchema.js b/app/imports/api/parenting/ChildSchema.js index b4cfbc3e..dd532219 100644 --- a/app/imports/api/parenting/ChildSchema.js +++ b/app/imports/api/parenting/ChildSchema.js @@ -14,7 +14,6 @@ const RefSchema = new SimpleSchema({ let ChildSchema = new SimpleSchema({ order: { type: Number, - min: 0, }, parent: { type: RefSchema, diff --git a/app/imports/api/parenting/getDescendantsInDepthFirstOrder.js b/app/imports/api/parenting/getDescendantsInDepthFirstOrder.js new file mode 100644 index 00000000..cc308f73 --- /dev/null +++ b/app/imports/api/parenting/getDescendantsInDepthFirstOrder.js @@ -0,0 +1,27 @@ +import { nodesToTree } from '/imports/api/parenting/parenting.js'; + +export default function getDescendantsInDepthFirstOrder({ + collection, + ancestorId, + filter, + options = {fields: {order: 1, ancestors: 1}}, +}){ + let forest = nodesToTree({collection, ancestorId, filter, options}); + let orderMemo = getDocsInDepthFirstOrder(forest); + return orderMemo; +} + +export function getDocsInDepthFirstOrder(forest){ + let docs = []; + forest.forEach(node => { + addNodeAndTraverse(node, docs) + }); + return docs; +} + +function addNodeAndTraverse(node, docs){ + docs.push(node.node); + node.children.forEach(child => { + addNodeAndTraverse(child, docs) + }); +} diff --git a/app/imports/api/parenting/order.js b/app/imports/api/parenting/order.js index 1a43258b..cf520a7f 100644 --- a/app/imports/api/parenting/order.js +++ b/app/imports/api/parenting/order.js @@ -1,53 +1,24 @@ import fetchDocByRef from '/imports/api/parenting/fetchDocByRef.js'; import getCollectionByName from '/imports/api/parenting/getCollectionByName.js'; +import getDescendantsInDepthFirstOrder from '/imports/api/parenting/getDescendantsInDepthFirstOrder.js' -// Docs keep track of their order amongst their siblings and keep a copy of the -// order of their ancestors. Order is first compared between oldest non-shared -// ancestors, then by ancestors before children, then between order of siblings. +// Docs keep track of their depth-first order amongst their entire ancestor tree export function compareOrder(docA, docB){ // < 0 if A comes before B // = 0 if A and B are the same order // > 0 if B comes before A - // Documents are equal order to themselves - if (docA._id && docB._id && docA._id === docB._id){ - return 0; - } - - // If they are siblings, just compare order - if (docA.parent.id === docB.parent.id){ - return docA.order - docB.order; - } - // They must share a root ancestor to be meaningfully sorted if (docA.ancestors[0].id !== docB.ancestors[0].id){ return 0; + } else { + return docA.order - docB.order; } - - // Go through their ancestors after the root, and find the first order - // difference - // TODO ancestors don't store order yet - let i, difference; - const length = Math.min(docA.ancestors.length, docB.ancestors.length); - for (i = 1; i < length; i++){ - difference = docA.ancestors[i].order - docB.ancestors[i].order; - if (difference){ - return difference; - } else if (docA.ancestors[i].id !== docB.ancestors[i].id) { - throw new Meteor.Error('Sibling order clash', - 'Sibling docs share the same order, sort failed'); - } - } - - // We haven't returned yet, all ancestors up to this point are shared and one - // doc has no more ancestors implying one is an ancestor of the other, - // return the difference in their ancestor list lengths, shorter comes first - return docA.ancestors.length - docB.ancestors.length } -export function getHighestOrder({collection, parentId}){ +export function getHighestOrder({collection, ancestorId}){ const highestOrderedDoc = collection.findOne({ - 'parent.id': parentId, + 'ancestors.id': ancestorId, }, { fields: {order: 1}, sort: {order: -1}, @@ -62,9 +33,9 @@ export function setDocToLastOrder({collection, doc}){ }) + 1; } -// update the order of a doc, and shift the siblings around to suit the new +// update the order of a doc, and shift the related docs around to suit the new // order -export function updateDocOrder({docRef, order}){ +function cheapUpdateDocOrder({docRef, order}){ let doc = fetchDocByRef(docRef, {fields: { order: 1, parent: 1, @@ -92,7 +63,7 @@ export function updateDocOrder({docRef, order}){ increment = 1; } collection.update({ - 'parent.id': doc.parent.id, + 'ancestors.id': doc.ancestors[0].id, order: inBetweenSelector, }, { $inc: {order: increment}, @@ -105,10 +76,10 @@ export function updateDocOrder({docRef, order}){ } } -export function removedDocAtOrder({collection, doc}){ +export function cheapRemovedDocAtOrder({collection, doc}){ // Decrement the order of all docs after the removed doc collection.update({ - 'parent.id': doc.parent.id, + 'ancestors.id': doc.ancestors[0].id, order: {$gt: doc.order}, }, { $inc: {order: -1}, @@ -118,10 +89,10 @@ export function removedDocAtOrder({collection, doc}){ }); } -export function insertedDocAtOrder({collection, parentId, order}){ +export function cheapInsertedDocAtOrder({collection, ancestorId, order}){ // Increment the order of all docs after the inserted doc collection.update({ - 'parent.id': parentId, + 'ancestors.id': ancestorId, order: {$gte: order}, }, { $inc: {order: 1}, @@ -131,54 +102,28 @@ export function insertedDocAtOrder({collection, parentId, order}){ }); } -// Update the order a single doc and re-order the entire sibling list +// Update the order a single doc and re-order the entire related doc list // with the change export function safeUpdateDocOrder({docRef, order}){ let collection = getCollectionByName(docRef.collection); - let movedDoc = fetchDocByRef(docRef, {fields: { - parent: 1, name: 1 - }}); - let parentId = movedDoc.parent.id; - let bulkWrite = []; - let docs = collection.find({ - 'parent.id': parentId, - '_id': {$ne: movedDoc._id}, + // Put the new doc half a step in front of its new order + // to ensure it's in front of whichever doc was there before + collection.update(docRef.id, { + $set: {order} }, { - fields: {order: 1, name: 1}, - sort: {order: 1} - }).fetch(); - docs.splice(order, 0, movedDoc); - docs.forEach((doc, index) => { - if (doc.order !== index){ - bulkWrite.push({ - updateOne: { - filter: {_id: doc._id}, - update: {$set: {order: index}}, - }, - }); - } + selector: {type: 'any'} }); - if (Meteor.isServer){ - collection.rawCollection().bulkWrite(bulkWrite); - } else { - bulkWrite.forEach(op => { - collection.update( - op.updateOne.filter, - op.updateOne.update, - {selector: {type: 'any'}} - ); - }); - } -}; + // reorder all related docs so that order is back to being a continous + // set of whole numbers + let movedDoc = fetchDocByRef(docRef, {fields: {ancestors: 1}}); + let ancestorId = movedDoc.ancestors[0].id; + reorderDocs({collection, ancestorId}); +} -export function reorderDocs({collection, parentId}){ +export function reorderDocs({collection, ancestorId}){ + let orderedDocs = getDescendantsInDepthFirstOrder({collection, ancestorId}); let bulkWrite = []; - collection.find({ - 'parent.id': parentId, - }, { - fields: {order: 1}, - sort: {order: 1} - }).forEach((doc, index) => { + orderedDocs.forEach((doc, index) => { if (doc.order !== index){ bulkWrite.push({ updateOne : { @@ -189,10 +134,23 @@ export function reorderDocs({collection, parentId}){ } }); if (Meteor.isServer){ - collection.rawCollection().bulkWrite(bulkWrite); + collection.rawCollection().bulkWrite( + bulkWrite, + {ordered : false}, + function(e){ + if (e) { + console.error('Bulk write failed: '); + console.error(e); + } + } + ); } else { bulkWrite.forEach(op => { - collection.update(op.updateOne.filter, op.updateOne.update); + collection.update( + op.updateOne.filter, + op.updateOne.update, + {selector: {type: 'any'}} + ); }); } } diff --git a/app/imports/api/parenting/organizeMethods.js b/app/imports/api/parenting/organizeMethods.js index bce2f889..fed8540a 100644 --- a/app/imports/api/parenting/organizeMethods.js +++ b/app/imports/api/parenting/organizeMethods.js @@ -1,6 +1,7 @@ import SimpleSchema from 'simpl-schema'; +import { ValidatedMethod } from 'meteor/mdg:validated-method'; import { updateParent } from '/imports/api/parenting/parenting.js'; -import { insertedDocAtOrder, removedDocAtOrder, safeUpdateDocOrder } from '/imports/api/parenting/order.js'; +import { reorderDocs, safeUpdateDocOrder } from '/imports/api/parenting/order.js'; import { RefSchema } from '/imports/api/parenting/ChildSchema.js'; import { assertDocEditPermission } from '/imports/api/sharing/sharingPermissions.js'; import fetchDocByRef from '/imports/api/parenting/fetchDocByRef.js'; @@ -13,7 +14,7 @@ const organizeDoc = new ValidatedMethod({ parentRef: RefSchema, order: { type: Number, - min: 0, + // Should end in 0.5 to place it reliably between two existing documents }, }).validator(), run({docRef, parentRef, order}) { @@ -25,14 +26,19 @@ const organizeDoc = new ValidatedMethod({ let parent = fetchDocByRef(parentRef); assertDocEditPermission(parent, this.userId); - // Reorder the documents in the doc's old parent - removedDocAtOrder({collection, doc}); - // Reorder the docs in the destination parent - insertedDocAtOrder({collection, parentId: parentRef.id, order}); // Change the doc's parent updateParent({docRef, parentRef}); - // Change the doc's order + // Change the doc's order to be a half step ahead of its target location collection.update(doc._id, {$set: {order}}, {selector: {type: 'any'}}); + + // Reorder both ancestors' documents + let oldAncestorId = doc.ancestors[0].id; + reorderDocs({collection, ancestorId: oldAncestorId}); + + let newAncestorId = getRootId(parent); + if (newAncestorId !== oldAncestorId){ + reorderDocs({collection, ancestorId: newAncestorId}); + } }, }); @@ -42,7 +48,7 @@ const reorderDoc = new ValidatedMethod({ docRef: RefSchema, order: { type: Number, - min: 0, + // Should end in 0.5 to place it reliably between two existing documents }, }).validator(), run({docRef, order}) { @@ -52,4 +58,12 @@ const reorderDoc = new ValidatedMethod({ }, }); +function getRootId(doc){ + if (doc.ancestors && doc.ancestors.length && doc.ancestors[0]){ + return doc.ancestors[0].id; + } else { + return doc._id; + } +} + export { organizeDoc, reorderDoc }; diff --git a/app/imports/api/parenting/parenting.js b/app/imports/api/parenting/parenting.js index 64a80b5b..58bd8130 100644 --- a/app/imports/api/parenting/parenting.js +++ b/app/imports/api/parenting/parenting.js @@ -48,38 +48,38 @@ export function fetchParent({id, collection}){ } export function fetchChildren({ collection, parentId, filter = {}, options = {sort: {order: 1}} }){ - filter["parent.id"] = parentId; + filter['parent.id'] = parentId; let children = []; children.push( ...collection.find({ - "parent.id": parentId + 'parent.id': parentId }, options).fetch() ); return children; } export function updateChildren({collection, parentId, filter = {}, modifier, options={}}){ - filter["parent.id"] = parentId; + filter['parent.id'] = parentId; options.multi = true; collection.update(filter, modifier, options); } export function fetchDescendants({ collection, ancestorId, filter = {}, options}){ - filter["ancestors.id"] = ancestorId; + filter['ancestors.id'] = ancestorId; let descendants = []; descendants.push(...collection.find(filter, options).fetch()); return descendants; } export function updateDescendants({collection, ancestorId, filter = {}, modifier, options={}}){ - filter["ancestors.id"] = ancestorId; + filter['ancestors.id'] = ancestorId; options.multi = true; options.selector = {type: 'any'}; collection.update(filter, modifier, options); } export function forEachDescendant({collection, ancestorId, filter = {}, options}, callback){ - filter["ancestors.id"] = ancestorId; + filter['ancestors.id'] = ancestorId; collection.find(filter, options).forEach(callback); } @@ -200,21 +200,21 @@ export function getName(doc){ if (doc.name) return name; var i = doc.ancestors.length; while(i--) { - if (ancestors[i].name) return ancestors[i].name; + if (doc.ancestors[i].name) return doc.ancestors[i].name; } } -export function nodesToTree({collection, ancestorId, filter}){ +export function nodesToTree({collection, ancestorId, filter, options}){ // Store a dict of all the nodes let nodeIndex = {}; let nodeList = []; + if (!options) options = {}; + options.sort = {order: 1}; collection.find({ 'ancestors.id': ancestorId, removed: {$ne: true}, ...filter, - }, { - sort: {order: 1} - }).forEach( node => { + }, options).forEach( node => { let treeNode = { node: node, children: [],