Rewrote parenting organize methods to avoid rebuilds

This commit is contained in:
ThaumRystra
2023-12-20 20:02:29 +02:00
parent 4a349ea906
commit 4c778fa282
6 changed files with 355 additions and 72 deletions

View File

@@ -2,6 +2,7 @@ import { buildComputationFromProps } from '/imports/api/engine/computation/build
import { assert } from 'chai';
import computeCreatureComputation from '../../computeCreatureComputation';
import clean from '../../utility/cleanProp.testFn';
import { propsFromForest } from '/imports/api/properties/tests/propTestBuilder.testFn';
export default function () {
const computation = buildComputationFromProps(testProperties);

View File

@@ -1,5 +1,6 @@
import computeAction from './computeAction.testFn';
import computeAttribute from './computeAttribute.testFn';
import computeCalculations from './computeCalculations.testFn';
import computeClasses from './computeClasses.testFn';
import computeConstants from './computeConstants.testFn';
import computeInventory from './computeInventory.testFn';
@@ -7,6 +8,7 @@ import computeDamageMultipliers from './computeDamageMultipliers.testFn';
import computeEffects from './computeEffects.testFn';
import computeSkills from './computeSkills.testFn';
import computeProficiencies from './computeProficiencies.testFn';
import computePointBuys from './computePointBuys.testFn';
export default [{
text: 'Computes actions',

View File

@@ -2,18 +2,72 @@ import SimpleSchema from 'simpl-schema';
import { ValidatedMethod } from 'meteor/mdg:validated-method';
import { RateLimiterMixin } from 'ddp-rate-limiter-mixin';
import { RefSchema } from '/imports/api/parenting/ChildSchema';
import { assertDocEditPermission } from '/imports/api/sharing/sharingPermissions.js';
import { assertDocEditPermission, assertEditPermission } from '/imports/api/sharing/sharingPermissions.js';
import { compact } from 'lodash';
import Creatures from '/imports/api/creature/creatures/Creatures.js';
import { changeParent, fetchDocByRefAsync, getCollectionByName, rebuildNestedSets } from '/imports/api/parenting/parentingFunctions';
import { fetchDocByRefAsync, getCollectionByName, moveDocBetweenRoots, moveDocWithinRoot } from '/imports/api/parenting/parentingFunctions';
const organizeDoc = new ValidatedMethod({
name: 'organize.organizeDoc',
const moveBetweenRoots = new ValidatedMethod({
name: 'organize.moveDocBetweenRoots',
validate: new SimpleSchema({
docRef: RefSchema,
parentRef: RefSchema,
order: {
type: Number,
// Should end in 0.5 to place it reliably between two existing documents
newPosition: {
type: Number, // Must end in .5
},
newRootRef: RefSchema,
skipRecompute: {
type: Boolean,
optional: true,
},
skipClient: {
type: Boolean,
optional: true,
},
}).validator(),
mixins: [RateLimiterMixin],
rateLimit: {
numRequests: 5,
timeInterval: 5000,
},
async run({ docRef, newPosition, newRootRef, skipRecompute, skipClient }) {
if (skipClient && this.isSimulation) {
return;
}
let doc = await fetchDocByRefAsync(docRef);
let collection = getCollectionByName(docRef.collection);
// The user must be able to edit both the doc and its parent to move it
// successfully
assertDocEditPermission(doc, this.userId);
const newRoot = await fetchDocByRefAsync(newRootRef);
assertEditPermission(newRoot, this.userId);
// Move the doc
await moveDocBetweenRoots(doc, collection, newRootRef, newPosition);
// Figure out which creatures need to be recalculated after this move
const creatureIdsToRecalculate = compact([
getCreatureAncestorId(doc),
getCreatureAncestorId(newRoot),
]);
// Mark the creatures for recompute
if (!skipRecompute && creatureIdsToRecalculate.length) {
Creatures.updateAsync({
_id: { $in: creatureIdsToRecalculate },
}, {
$set: { dirty: true },
});
}
},
});
const moveWithinRoot = new ValidatedMethod({
name: 'organize.moveDocWithinRoot',
validate: new SimpleSchema({
docRef: RefSchema,
newPosition: {
type: Number, // Must end in .5
},
skipRecompute: {
type: Boolean,
@@ -29,76 +83,26 @@ const organizeDoc = new ValidatedMethod({
numRequests: 5,
timeInterval: 5000,
},
async run({ docRef, parentRef, order, skipRecompute, skipClient }) {
async run({ docRef, newPosition, skipRecompute, skipClient }) {
if (skipClient && this.isSimulation) {
return;
}
let doc = await fetchDocByRefAsync(docRef);
let collection = getCollectionByName(docRef.collection);
// The user must be able to edit both the doc and its parent to move it
// successfully
// The user must be able to edit the doc
assertDocEditPermission(doc, this.userId);
let parent;
parent = await fetchDocByRefAsync(parentRef);
assertDocEditPermission(parent, this.userId);
// Moving the doc to the root level means changing its parent to undefined
if (doc.root.id === parent._id) {
parent = null;
}
// Change the doc's parent
await changeParent(doc, parent, collection, order);
// Move the doc
await moveDocWithinRoot(doc, collection, newPosition);
// Figure out which creature needs to be recalculated after this move
const creatureIdToRecalculate = getCreatureAncestorId(doc);
// Figure out which creatures need to be recalculated after this move
const docCreature = getCreatureAncestorId(doc);
const parentCreature = getCreatureAncestorId(parent);
// Mark the creatures for recompute
if (!skipRecompute) {
if (docCreature) {
Creatures.updateAsync({
_id: docCreature,
}, {
$set: { dirty: true },
});
}
if (parentCreature && parentCreature !== docCreature) {
Creatures.updateAsync({
_id: parentCreature,
}, {
$set: { dirty: true },
});
}
}
},
});
const reorderDoc = new ValidatedMethod({
name: 'organize.reorderDoc',
validate: new SimpleSchema({
docRef: RefSchema,
order: {
type: Number,
// Should end in 0.5 to place it reliably between two existing documents
},
}).validator(),
mixins: [RateLimiterMixin],
rateLimit: {
numRequests: 5,
timeInterval: 5000,
},
async run({ docRef, order }) {
const doc = await fetchDocByRefAsync(docRef);
assertDocEditPermission(doc, this.userId);
let collection = getCollectionByName(docRef.collection);
console.log('setting doc left to ', order);
await collection.updateAsync(doc._id, { $set: { left: order } });
// Recompute the affected creatures
const creatureId = getCreatureAncestorId(doc);
if (creatureId) {
return Creatures.updateAsync({
_id: creatureId
if (!skipRecompute && creatureIdToRecalculate) {
Creatures.updateAsync({
_id: creatureIdToRecalculate,
}, {
$set: { dirty: true },
});
@@ -115,4 +119,4 @@ function getCreatureAncestorId(doc) {
}
}
export { organizeDoc, reorderDoc };
export { moveBetweenRoots, moveWithinRoot };

View File

@@ -1,9 +1,12 @@
import { docsToForest, calculateNestedSetOperations, getFilter } from '/imports/api/parenting/parentingFunctions'
import '/imports/api/simpleSchemaConfig';
import { docsToForest, calculateNestedSetOperations, getFilter, moveDocWithinRoot } from '/imports/api/parenting/parentingFunctions'
import { TreeDoc } from '/imports/api/parenting/ChildSchema';
import { assert } from 'chai';
function doc(_id, left, right, parentId?): TreeDoc {
return { _id, root: { id: 'root', collection: 'col' }, left, right, parentId };
const doc = { _id, root: { id: 'root', collection: 'col' }, left, right, parentId };
if (!parentId) delete doc.parentId;
return doc;
}
function op(_id, left, right) {
@@ -184,3 +187,108 @@ describe('Document tree filters can fetch other documents based on their positio
assert.sameMembers(parentIds['dbm'], ['Databases']);
});
});
describe('Document can be moved without breaking the tree', function () {
/**
* Test the following structure
*
* 1 Books 12 13 Videos 24
* ┃ ┃
* 2 Programming 11 14 Cooking 23
* ┏━━━━━━━━┻━━━━━━━━━┓ ┏━━━━━━━━┻━━━━━━━━━┓
* 3 Languages 4 5 Databases 10 15 Meat 16 17 Vegetarian 22
* ┏━━━━━━━┻━━━━━━━┓ ┏━━━━━━━┻━━━━━━━┓
* 6 MongoDB 7 8 dbm 9 18 Pasta 19 20 Mains 21
*/
const treeCollection: Mongo.Collection<TreeDoc> = new Mongo.Collection('treeDocsMove');
beforeEach(function () {
treeCollection.remove({});
[
doc('Books', 1, 12, undefined),
doc('Programming', 2, 11, 'Books'),
doc('Languages', 3, 4, 'Programming'),
doc('Databases', 5, 10, 'Programming'),
doc('MongoDB', 6, 7, 'Databases'),
doc('dbm', 8, 9, 'Databases'),
doc('Videos', 13, 24, undefined),
doc('Cooking', 14, 23, 'Videos'),
doc('Meat', 15, 16, 'Cooking'),
doc('Vegetarian', 17, 22, 'Cooking'),
doc('Pasta', 18, 19, 'Vegetarian'),
doc('Mains', 20, 21, 'Vegetarian'),
].map(doc => {
return treeCollection.insert(doc);
});
});
it('can move a document within its parent', async function () {
const languagesDoc = await treeCollection.findOneAsync({ _id: 'Languages' });
if (!languagesDoc) throw new Error('Languages doc not found');
await moveDocWithinRoot(languagesDoc, treeCollection, 10.5);
/**
* Expected resulting structure
* 1 Books 12 13 Videos 24
* ┃ ┃
* 2 Programming 11 14 Cooking 23
* ┏━━━━━━━━┻━━━━━━━━━┓ ┏━━━━━━━━┻━━━━━━━━━┓
* 3 Databases 8 9 Languages 10 15 Meat 16 17 Vegetarian 22
* ┏━━━━━━━┻━━━━━━━┓ ┏━━━━━━━┻━━━━━━━┓
* 4 MongoDB 5 6 dbm 7 18 Pasta 19 20 Mains 21
*/
const docs = await treeCollection.find({}, { sort: { left: 1 } }).fetchAsync();
assert.deepEqual(docs, [
doc('Books', 1, 12, undefined),
doc('Programming', 2, 11, 'Books'),
doc('Databases', 3, 8, 'Programming'),
doc('MongoDB', 4, 5, 'Databases'),
doc('dbm', 6, 7, 'Databases'),
doc('Languages', 9, 10, 'Programming'),
doc('Videos', 13, 24, undefined),
doc('Cooking', 14, 23, 'Videos'),
doc('Meat', 15, 16, 'Cooking'),
doc('Vegetarian', 17, 22, 'Cooking'),
doc('Pasta', 18, 19, 'Vegetarian'),
doc('Mains', 20, 21, 'Vegetarian'),
]);
});
it('can move a document to a whole new parent', async function () {
const videos = await treeCollection.findOneAsync({ _id: 'Videos' });
if (!videos) throw new Error('Videos doc not found');
await moveDocWithinRoot(videos, treeCollection, 6.5);
/**
* Expected resulting structure
* 1 Books 24
* ┃
* 2 Programming 23
* ┏━━━━━━━━┻━━━━━━━━━┓
* 3 Languages 4 5 Databases 22
* ┏━━━━━━━┻━━━━━━━┓
* 6 MongoDB 19 20 dbm 21
* ┃
* 7 Videos 18
* ┃
* 8 Cooking 17
* ┏━━━━━━━━┻━━━━━━━━━┓
* 9 Meat 10 11 Vegetarian 16
* ┏━━━━━━━┻━━━━━━━┓
* 12 Pasta 13 14 Mains 15
*/
const docs = await treeCollection.find({}, { sort: { left: 1 } }).fetchAsync();
assert.deepEqual(docs, [
doc('Books', 1, 24, undefined),
doc('Programming', 2, 23, 'Books'),
doc('Languages', 3, 4, 'Programming'),
doc('Databases', 5, 22, 'Programming'),
doc('MongoDB', 6, 19, 'Databases'),
doc('Videos', 7, 18, 'MongoDB'),
doc('Cooking', 8, 17, 'Videos'),
doc('Meat', 9, 10, 'Cooking'),
doc('Vegetarian', 11, 16, 'Cooking'),
doc('Pasta', 12, 13, 'Vegetarian'),
doc('Mains', 14, 15, 'Vegetarian'),
doc('dbm', 20, 21, 'Databases'),
]);
});
});
// TODO test moving between roots

View File

@@ -341,9 +341,176 @@ export function renewDocIds({ docArray, collectionMap = {}, idMap = {} }) {
});
}
/**
* Moves a document within its root. The destination must be halfway between two positions (n.5)
* @param doc
* @param collection
*/
export async function moveDocWithinRoot(doc: TreeDoc, collection: Mongo.Collection<TreeDoc>, newPosition: number) {
let move: number;
let includedRange;
// Ensure the destination is at a midway point
if (newPosition % 1 !== 0.5) {
throw new Meteor.Error('invalid-move', 'Destination must be halfway between two positions (n.5)');
}
// Get the distance to move and the range between the current document and the destination
const docSize = doc.right - doc.left + 1;
let shiftDistance;
if (newPosition < doc.left) {
move = newPosition - doc.left + 0.5;
includedRange = { left: newPosition, right: doc.left - 0.5 };
shiftDistance = docSize;
} else if (newPosition > doc.right) {
move = newPosition - doc.right - 0.5;
includedRange = { left: doc.right + 0.5, right: newPosition };
shiftDistance = -docSize;
} else {
throw new Meteor.Error('invalid-move', 'Destination must be outside the doc\'s current location');
}
console.log(`Moving ${doc._id} ${move} spaces, while everything between ${includedRange.left} and ${includedRange.right} is shifted by ${shiftDistance}`);
// If the move isn't meaningfully changing the doc's location, skip
if (Math.abs(move) < 1) {
return;
}
// Get the new parent of the doc after the move
const newParent = await collection.findOneAsync({
'root.id': doc.root.id,
left: { $lt: newPosition },
right: { $gt: newPosition },
}, {
sort: { left: -1 }, // Many ancestors match, taking the right most one gets the immediate parent
fields: { _id: 1 },
});
const newParentId = newParent?._id;
// get ids of the doc and its children, so we can move them even after other docs have been
// moved around to potentially overlap the moving doc
const movedIds = await collection.find({
'root.id': doc.root.id,
left: { $gte: doc.left },
right: { $lte: doc.right },
}, {
fields: { _id: 1 }
}).mapAsync(doc => doc._id);
// Move all the lefts and rights of documents between the current doc edge and the destination
const moveIncludedLeft = collection.updateAsync({
'root.id': doc.root.id,
left: { $gt: includedRange.left, $lt: includedRange.right },
}, {
$inc: { left: shiftDistance }
}, {
multi: true
});
const moveIncludedRight = collection.updateAsync({
'root.id': doc.root.id,
right: { $gt: includedRange.left, $lt: includedRange.right },
}, {
$inc: { right: shiftDistance }
}, {
multi: true
});
// Move the doc and its children to the new location
const moveDocAndChildren = collection.updateAsync({
_id: { $in: movedIds }
}, {
$inc: {
left: move,
right: move,
}
}, {
multi: true,
});
// Set the doc's parent to the new parentId
let changeParent: Promise<number> | undefined;
if (newParentId !== doc.parentId) {
let update;
if (newParentId) {
update = { $set: { parentId: newParentId } };
} else {
update = { $unset: { parentId: 1 } };
}
changeParent = collection.updateAsync(doc._id, update);
}
return Promise.all([moveIncludedLeft, moveIncludedRight, moveDocAndChildren, changeParent]);
}
export async function moveDocBetweenRoots(doc: TreeDoc, collection: Mongo.Collection<TreeDoc>, newRoot: Reference, newPosition: number) {
if (newRoot.id === doc.root.id) {
throw new Meteor.Error('invalid-move', 'Document is already in the given root')
}
// get ids of the doc and its children, so we can move them even after other docs have been
// moved around to potentially overlap the moving doc
const movedIds = await collection.find({
'root.id': doc.root.id,
left: { $gte: doc.left },
right: { $lte: doc.right },
}, {
fields: { _id: 1 }
}).mapAsync(doc => doc._id);
// Close the gap in the root we are leaving
const docSize = doc.right - doc.left + 1;
const closeGapLeft = collection.updateAsync({
'root.id': doc.root.id,
left: { $gt: doc.right },
}, {
$inc: { left: -docSize },
}, {
multi: true,
});
const closeGapRight = collection.updateAsync({
'root.id': doc.root.id,
right: { $gt: doc.right },
}, {
$inc: { right: -docSize },
}, {
multi: true,
});
// Open a gap in the root we are moving to at the new location
const openGapLeft = collection.updateAsync({
'root.id': newRoot.id,
left: { $gt: newPosition },
}, {
$inc: { left: -docSize },
}, {
multi: true,
});
const openGapRight = collection.updateAsync({
'root.id': newRoot.id,
right: { $gt: newPosition },
}, {
$inc: { right: -docSize },
}, {
multi: true,
});
// Move the docs to the new root and update their left and right positions to land in the gap
const moveDistance = newPosition + 0.5 - doc.left;
const moveDocs = collection.updateAsync({
_id: { $in: movedIds },
}, {
$set: { root: newRoot },
$inc: { left: moveDistance, right: moveDistance },
}, {
multi: true,
});
return Promise.all([closeGapLeft, closeGapRight, openGapLeft, openGapRight, moveDocs]);
}
/**
* Changes the doc to be a child of the parent, and then rebuilds the nested sets of the roots
* of both doc and parent
* @deprecated Use moveDocWithinRoot or moveDocBetweenRoots instead
* @param doc The doc to move
* @param parent The new parent of the doc, null to move the doc to the root of the tree
* @param collection

View File

@@ -1,3 +1,4 @@
import '/imports/api/simpleSchemaConfig';
import SimpleSchema from 'simpl-schema';
import { ComputedActionSchema } from '/imports/api/properties/Actions';
import { ComputedAdjustmentSchema } from '/imports/api/properties/Adjustments';