From 029c55f74c5e411558faf9f15e955d4c769fe905 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 10 Jun 2022 17:00:09 -0400 Subject: [PATCH] feat(hadron-document)!: handle nested fields and dots & dollars well COMPASS-5805 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Instead of fully replacing top-level properties only when editing documents, set only the individual nested values that have been edited. - This does not apply to array item removals, since there is no way to perform this (at least in MongoDB 4.0, but doing so on newer versions is also not straightforward). - Use `$getField` and `$setField` to modify fields whose names contain dots or start with a dollar sign. This is kind of a breaking change in that, previously, we would have queried for the field names containing dots, the server would have interpreted the field name as referring to nested documents, almost never found a document matching that (unless the doc happened to have a shape like `{ x: { y: 2 }, 'x.y': 2 }` with matching values), and we then prompted the user to force-update due to the assumption that no document having been found means that it has been updated in the background. After this change, we just inform users that doing this properly requires MongoDB 5.0 or above. Users can still perform edits through the JSON view, at least for now (potentially only until COMPASS-5753 is done). - Split the query/update document generation parts into two steps, and move these out of the `Document` class to `ObjectGenerator` (which is already a collection of static methods for recursively traversing a document/element tree). - In the first step, gather all the elements that were updated, their original paths and values and their new paths and values. - In the second step, build either a query out of those original paths and values, or build the update document out of the new paths and values. - Drive-by bugfixes: - When renaming elements, we previously did not check that the new name was not already added in the background. We check this as well now. - Handle the case in which elements are renamed circularly, e.g. key2 → key3, key1 → key2 (or similar add/remove situations). - Use string arrays instead of objects that are only being used to extract their keys in the hadron-document API for hopefully a bit more API clarity. - Clarify in the docs that dots inside field names refer to nested documents. This is the way that sharding behaves, which is why we use them in the first place, and is not a practical API to break. (And if it does, there is an upcoming server sharding project will hopefully already have made inclusion of the shard keys unnecessary.) This also addresses COMPASS-5528 (which is about adjusting the update document sent to the server, whereas COMPASS-5805 is about adjusting the query document). --- .../compass-crud/src/stores/crud-store.js | 17 +- packages/hadron-document/src/document.ts | 147 +---- packages/hadron-document/src/element.ts | 32 +- .../hadron-document/src/object-generator.ts | 320 ++++++++++ .../hadron-document/test/document.test.ts | 575 ++++++++++++++++-- 5 files changed, 893 insertions(+), 198 deletions(-) diff --git a/packages/compass-crud/src/stores/crud-store.js b/packages/compass-crud/src/stores/crud-store.js index d7523380a21..c303fc85fd4 100644 --- a/packages/compass-crud/src/stores/crud-store.js +++ b/packages/compass-crud/src/stores/crud-store.js @@ -23,7 +23,7 @@ import { import configureGridStore from './grid-store'; -const { log, mongoLogId, track } = createLoggerAndTelemetry('COMPASS-CRUD-UI'); +const { debug, log, mongoLogId, track } = createLoggerAndTelemetry('COMPASS-CRUD-UI'); function pickQueryProps({ filter, @@ -476,7 +476,8 @@ const configureStore = (options = {}) => { const { query, updateDoc - } = doc.generateUpdateUnlessChangedInBackgroundQuery(this.state.shardKeys); + } = doc.generateUpdateUnlessChangedInBackgroundQuery(Object.keys(this.state.shardKeys)); + debug('Performing findOneAndUpdate', { query, updateDoc }); if (Object.keys(updateDoc).length === 0) { doc.emit('update-error', EMPTY_UPDATE_ERROR.message); @@ -492,6 +493,10 @@ const configureStore = (options = {}) => { }); if (error) { + if (error.codeName === 'InvalidPipelineOperator' && error.message.match(/\$[gs]etField/)) { + const nbsp = '\u00a0'; + error.message += ` (Updating fields whose names contain dots or start with $ require MongoDB${nbsp}5.0 or above.)`; + } doc.emit('update-error', error.message); } else if (d) { doc.emit('update-success', d); @@ -518,10 +523,10 @@ const configureStore = (options = {}) => { try { doc.emit('update-start'); const object = doc.generateObject(); - const query = doc.getOriginalKeysAndValuesForSpecifiedKeys({ - _id: 1, - ...(this.state.shardKeys || {}) - }); + const query = doc.getQueryForOriginalKeysAndValuesForSpecifiedKeys([ + '_id', ...Object.keys(this.state.shardKeys || {}) + ]); + debug('Performing findOneAndReplace', { query, object }); if (!await this._verifyUpdateAllowed(this.state.ns, doc)) { // _verifyUpdateAllowed emitted update-error diff --git a/packages/hadron-document/src/document.ts b/packages/hadron-document/src/document.ts index f6e7436abb8..5cb4c689af6 100644 --- a/packages/hadron-document/src/document.ts +++ b/packages/hadron-document/src/document.ts @@ -5,7 +5,7 @@ import EventEmitter from 'eventemitter3'; import { EJSON, UUID } from 'bson'; import type { ObjectGeneratorOptions } from './object-generator'; import ObjectGenerator from './object-generator'; -import type { BSONObject, BSONValue } from './utils'; +import type { BSONArray, BSONObject, BSONValue } from './utils'; import { objectToIdiomaticEJSON } from './utils'; import type { HadronEJSONOptions } from './utils'; @@ -109,40 +109,40 @@ export class Document extends EventEmitter { * where the update only succeeds when the changed document's elements have * not been changed in the background. * + * `query` and `updateDoc` may use $getField and $setField if field names + * contain either `.` or start with `$`. These operators are only available + * on MongoDB 5.0+. (Note that field names starting with `$` are also only + * allowed in MongoDB 5.0+.) + * * @param {Object} alwaysIncludeKeys - An object whose keys are used as keys - * that are always included in the generated query. + * that are always included in the generated query. Dots inside key names + * are interpreted as referring to nested properties. * * @returns {Object} An object containing the `query` and `updateDoc` to be * used in an update operation. */ generateUpdateUnlessChangedInBackgroundQuery( - alwaysIncludeKeys: BSONObject | null = null + alwaysIncludeKeys: string[] = [] ): { query: BSONObject; - updateDoc: { $set?: BSONObject; $unset?: BSONObject }; + updateDoc: { $set?: BSONObject; $unset?: BSONObject } | BSONArray; } { // Build a query that will find the document to update only if it has the // values of elements that were changed with their original value. // This query won't find the document if an updated element's value isn't // the same value as it was when it was originally loaded. const originalFieldsThatWillBeUpdated = - this.getOriginalKeysAndValuesForFieldsThatWereUpdated(alwaysIncludeKeys); + ObjectGenerator.getQueryForOriginalKeysAndValuesForSpecifiedFields( + this, + alwaysIncludeKeys, + true + ); const query = { _id: this.getId(), ...originalFieldsThatWillBeUpdated, }; - // Build the update document to be used in an update operation with `$set` - // and `$unset` reflecting the changes that have occured in the document. - const setUpdateObject = this.getSetUpdateForDocumentChanges(); - const unsetUpdateObject = this.getUnsetUpdateForDocumentChanges(); - const updateDoc: { $set?: BSONObject; $unset?: BSONObject } = {}; - if (setUpdateObject && Object.keys(setUpdateObject).length > 0) { - updateDoc.$set = setUpdateObject; - } - if (unsetUpdateObject && Object.keys(unsetUpdateObject).length > 0) { - updateDoc.$unset = unsetUpdateObject; - } + const updateDoc = ObjectGenerator.generateUpdateDoc(this); return { query, @@ -197,98 +197,24 @@ export class Document extends EventEmitter { return element ? element.generateObject() : null; } - /** - * Generate the query javascript object reflecting the elements that - * were updated in this document. The values of this object are the original - * values, this can be used when querying for an update to see if the original - * document was changed in the background while it was being updated elsewhere. - * - * @param {Object} alwaysIncludeKeys - An object whose keys are used as keys - * that are always included in the generated query. - * - * @returns {Object} The javascript object. - */ - getOriginalKeysAndValuesForFieldsThatWereUpdated( - alwaysIncludeKeys: BSONObject | null = null - ): BSONObject { - const object: BSONObject = {}; - - if (this.elements) { - for (const element of this.elements) { - if ( - (element.isModified() && !element.isAdded()) || - (alwaysIncludeKeys && element.key in alwaysIncludeKeys) - ) { - // Using `.key` instead of `.currentKey` to ensure we look at - // the original field's value. - object[element.key] = element.generateOriginalObject(); - } - if (element.isAdded() && element.currentKey !== '') { - // When a new field is added, check if that field - // was already added in the background. - object[element.currentKey] = { $exists: false }; - } - } - } - - return object; - } - /** * Generate the query javascript object reflecting the elements that * are specified by the keys listed in `keys`. The values of this object are * the original values, this can be used when querying for an update based * on multiple criteria. * - * @param {Object} keys - An object whose keys are used as keys - * that are included in the generated query. + * @param keys - An array whose entries are used as keys + * that are included in the generated query. Dots inside key names + * are interpreted as referring to nested properties. * * @returns {Object} The javascript object. */ - getOriginalKeysAndValuesForSpecifiedKeys(keys: BSONObject): BSONObject { - const object: BSONObject = {}; - - if (this.elements) { - for (const element of this.elements) { - if (element.key in keys) { - // Using `.key` instead of `.currentKey` to ensure we look at - // the original field's value. - object[element.key] = element.generateOriginalObject(); - } - } - } - - return object; - } - - /** - * Generate an $set javascript object, that can be used in update operations to - * set the changes which have occured in the document since it was loaded. - * - * @returns {Object} The javascript update object. - **/ - getSetUpdateForDocumentChanges(): BSONObject { - const object: BSONObject = {}; - - if (this.elements) { - for (const element of this.elements) { - if ( - !element.isRemoved() && - element.currentKey !== '' && - element.isModified() - ) { - // Include the full modified element. - // We don't individually set nested fields because we can't guarantee a - // path to the element using '.' dot notation will update - // the correct field, because field names can contain dots as of 3.6. - // When a nested field has been altered (changed/added/removed) it is - // set at the top level field. This means we overwrite possible - // background changes that occur within sub documents. - object[element.currentKey] = element.generateObject(); - } - } - } - return object; + getQueryForOriginalKeysAndValuesForSpecifiedKeys(keys: string[]): BSONObject { + return ObjectGenerator.getQueryForOriginalKeysAndValuesForSpecifiedFields( + this, + keys, + false + ); } /** @@ -309,29 +235,6 @@ export class Document extends EventEmitter { return String(element.value); } - /** - * Generate an $unset javascript object, that can be used in update - * operations, with the removals from the document. - * - * @returns {Object} The javascript update object. - **/ - getUnsetUpdateForDocumentChanges(): BSONObject { - const object: BSONObject = {}; - - if (this.elements) { - for (const element of this.elements) { - if (!element.isAdded() && element.isRemoved() && element.key !== '') { - object[element.key] = true; - } - if (!element.isAdded() && element.isRenamed() && element.key !== '') { - // Remove the original field when an element is renamed. - object[element.key] = true; - } - } - } - return object; - } - /** * Insert a placeholder element at the end of the document. * diff --git a/packages/hadron-document/src/element.ts b/packages/hadron-document/src/element.ts index 8f89338a483..98f70f76c91 100644 --- a/packages/hadron-document/src/element.ts +++ b/packages/hadron-document/src/element.ts @@ -452,9 +452,9 @@ export class Element extends EventEmitter { } /** - * Determine if the element is renamed. + * Determine if the element was explicitly renamed by the user. * - * @returns If the element was renamed. + * @returns If the element was explicitly renamed by the user. */ isRenamed(): boolean { if ( @@ -468,7 +468,17 @@ export class Element extends EventEmitter { } /** - * Can changes to the elemnt be reverted? + * Determine if the element was renamed, potentially as part + * of moving array elements. + * + * @returns If the element was renamed, explicitly or implicitly. + */ + hasChangedKey(): boolean { + return this.key !== this.currentKey; + } + + /** + * Can changes to the element be reverted? * * @returns If the element can be reverted. */ @@ -631,6 +641,22 @@ export class Element extends EventEmitter { return this.removed; } + /** + * Are any immediate children of this element flagged for removal? + * + * @returns If any immediate children of this element are flagged for removal. + */ + hasAnyRemovedChild(): boolean { + if (this.elements) { + for (const element of this.elements) { + if (element.isRemoved()) { + return true; + } + } + } + return false; + } + /** * Elements themselves are not the root. * diff --git a/packages/hadron-document/src/object-generator.ts b/packages/hadron-document/src/object-generator.ts index cede4a6b528..d6cf8503536 100644 --- a/packages/hadron-document/src/object-generator.ts +++ b/packages/hadron-document/src/object-generator.ts @@ -1,4 +1,7 @@ import type { Element } from './element'; +import type { Document } from './document'; +import type { BSONArray, BSONObject, BSONValue } from './utils'; +import isEqual from 'lodash.isequal'; const DECRYPTED_KEYS = Symbol.for('@@mdb.decryptedKeys'); @@ -24,6 +27,19 @@ function maybeDecorateWithDecryptedKeys( } } +/** Used to represent missing values, i.e. non-existent fields. */ +const DoesNotExist = Symbol('DidNotExist'); + +/** + * Describe a field in a document, with its path and current value. + * For example, in the document `{ a: { b: 42 } }`, the nested property + * `b` of `a` would be described by `{ path: ['a', 'b'], value: 42 }`. + */ +type FieldDescription = { + path: string[]; + value: BSONValue | typeof DoesNotExist; +}; + /** * Generates javascript objects from elements. */ @@ -143,6 +159,310 @@ export class ObjectGenerator { } return elements; } + + /** + * As the first step in generating query and update documents for updated + * fields in a document, gather the original and new paths and values + * for those updated fields. + * + * @param target The target document, or, when recursing, element. + * @param alwaysIncludeOriginalKeys A list of fields whose original values + * are always included in `originalFields`. Dots inside key names + * are interpreted as referring to nested properties. + * @param includeUpdatedFields Whether to include original and new values + * of updated fields. If set to `false`, only fields included in + * @see alwaysIncludeOriginalKeys are included. + * @returns A pair `{ originalFields, newFields }`, each listing the + * original and new paths and values for updated fields, respectively. + */ + private static recursivelyGatherFieldsAndValuesForUpdate( + target: Document | Element, + alwaysIncludeOriginalKeys: string[], + includeUpdatedFields: boolean + ): { + originalFields: FieldDescription[]; + newFields: FieldDescription[]; + } { + const originalFields: FieldDescription[] = []; + const newFields: FieldDescription[] = []; + + for (const element of target.elements ?? []) { + // Recurse into an element if it either has been updated and we are looking + // for updated fields, or it is part of the set of keys that we should always + // include. + if ( + (includeUpdatedFields && + element.isModified() && + !element.isAdded() && + !element.hasChangedKey()) || + alwaysIncludeOriginalKeys.some( + (key) => + key === String(element.key) || key.startsWith(`${element.key}.`) + ) + ) { + // Two possible cases: Either we recurse into this element and change + // nested values, or we replace the element entirely. + // We can only recurse if: + // - This is a nested element with children, i.e. array or document + // - It was not explicitly requested via alwaysIncludeOriginalKeys to + // always include it in its entirety + // - Its type has not changed + // - It is not an array with removed elements, since MongoDB has + // no way to remove individual array elements (!!) prior to + // agg-pipeline-style updates added in 4.2, and even then it's complex + // to actually do so + if ( + element.elements && + !alwaysIncludeOriginalKeys.includes(String(element.key)) && + ((element.type === 'Object' && element.currentType === 'Object') || + (element.type === 'Array' && + element.currentType === 'Array' && + !element.hasAnyRemovedChild())) + ) { + // Nested case: Translate alwaysIncludeKeys to the nested keys, + // get the original keys and values for the nested element, + // then translate the result back to this level. + const nestedAlwaysIncludeKeys = alwaysIncludeOriginalKeys + .filter((key) => key.startsWith(`${element.key}.`)) + .map((key) => key.replace(`${element.key}.`, '')); + const nestedResult = + ObjectGenerator.recursivelyGatherFieldsAndValuesForUpdate( + element, + nestedAlwaysIncludeKeys, + includeUpdatedFields + ); + for (const { path, value } of nestedResult.originalFields) { + originalFields.push({ + path: [String(element.key), ...path], + value, + }); + } + for (const { path, value } of nestedResult.newFields) { + newFields.push({ + path: [String(element.currentKey), ...path], + value, + }); + } + } else { + // Using `.key` instead of `.currentKey` to ensure we look at + // the original field's value. + originalFields.push({ + path: [String(element.key)], + value: element.generateOriginalObject(), + }); + + if ( + includeUpdatedFields && + element.currentKey !== '' && + !element.isRemoved() + ) { + newFields.push({ + path: [String(element.currentKey)], + value: element.generateObject(), + }); + } + } + } + + if ( + includeUpdatedFields && + !element.isRemoved() && + (element.isAdded() || element.hasChangedKey()) && + element.currentKey !== '' + ) { + // When a new field is added, check if that field + // was already added in the background. + originalFields.push({ + path: [String(element.currentKey)], + value: DoesNotExist, + }); + newFields.push({ + path: [String(element.currentKey)], + value: element.generateObject(), + }); + } + + if ( + includeUpdatedFields && + !element.isAdded() && + (element.isRemoved() || element.hasChangedKey()) && + element.key !== '' + ) { + // Remove the original field when an element is removed or renamed. + originalFields.push({ + path: [String(element.key)], + value: element.generateOriginalObject(), + }); + newFields.push({ path: [String(element.key)], value: DoesNotExist }); + } + } + + // Sometimes elements are removed or renamed, and then another + // element is added or renamed to take its place. We filter out + // the DoesNotExist entry for that case. + for (let i = 0; i < newFields.length; ) { + const entry = newFields[i]; + if (entry.value === DoesNotExist) { + if ( + newFields.some( + (otherEntry) => + isEqual(otherEntry.path, entry.path) && entry !== otherEntry + ) + ) { + // Drop `entry`. + newFields.splice(i, 1); + continue; + } + } + i++; + } + + return { originalFields, newFields }; + } + + // Return a $getField expression that evaluates to the current value + // of the document at `path`. + private static createGetFieldExpr(path: string[]): BSONObject { + return path.reduce( + (input, key) => ({ + $getField: { + field: { $literal: key }, + input, + }, + }), + '$$ROOT' as any + ); + } + + // Return a $setField expression that writes the specified value + // to the document at `path`. + private static createSetFieldExpr( + path: string[], + value: BSONValue | typeof DoesNotExist + ): BSONValue { + return path.reduceRight( + (value, key, idx, array) => ({ + $setField: { + field: { $literal: key }, + input: ObjectGenerator.createGetFieldExpr(array.slice(0, idx)), + value, + }, + }), + (value === DoesNotExist ? '$$REMOVE' : { $literal: value }) as any + ); + } + + /** + * Generate the query javascript object reflecting original + * values of specific elements in this documents. This can include + * elements that were updated in this document. In that case, the + * values of this object are the original values, this can be used + * when querying for an update to see if the original document was + * changed in the background while it was being updated elsewhere. + * + * NOTE: `alwaysIncludeKeys` is currently used for sharding, since + * updates on sharded setups need to include the shard key in their + * find part. https://jira.mongodb.org/browse/PM-1632 will make + * this requirement go away for future MongoDB versions! + * + * @param target The target (sub-)document. + * @param alwaysIncludeOriginalKeys A list whose entries are used as keys + * that are always included in the generated query. Dots inside key names + * are interpreted as referring to nested properties. + * @param includeUpdatedFields Whether to include the original values for + * updated fields. + * + * @returns A pair of lists, one containing the original values for updated fields + * or those specified in the always-include list, and one containing new values + * of the updated fields. If includeUpdatedFields is not set, the second + * list will be empty. + */ + static getQueryForOriginalKeysAndValuesForSpecifiedFields( + target: Document | Element, + alwaysIncludeOriginalKeys: string[], + includeUpdatedFields: boolean + ): BSONObject { + const { originalFields } = + ObjectGenerator.recursivelyGatherFieldsAndValuesForUpdate( + target, + alwaysIncludeOriginalKeys, + includeUpdatedFields + ); + + const query: any = {}; + if ( + originalFields.some(({ path }) => + path.some((key) => key.includes('.') || key.startsWith('$')) + ) + ) { + // Some of the keys in this query are only accesible via $getField, + // which was introduced in MongoDB 5.0. + for (const { path, value } of originalFields) { + const getFieldExpr = ObjectGenerator.createGetFieldExpr(path); + const matchExpr = + value !== DoesNotExist + ? { + $eq: [getFieldExpr, { $literal: value }], + } + : { + $eq: [{ $type: getFieldExpr }, 'missing'], + }; + query.$expr ??= { $and: [] }; + query.$expr.$and.push(matchExpr); + } + } else { + for (const { path, value } of originalFields) { + const matchValue = value === DoesNotExist ? { $exists: false } : value; + query[path.join('.')] = matchValue; + } + } + return query; + } + + /** + * Generate an update document or pipeline which reflects the updates + * that have taken place for this document. A pipeline will be returned + * if the updates require changes to fields containing dots or prefixed + * with $. + * + * @param target The target (sub-)document. + */ + static generateUpdateDoc( + target: Document | Element + ): { $set?: BSONObject; $unset?: BSONObject } | BSONArray { + const { newFields } = + ObjectGenerator.recursivelyGatherFieldsAndValuesForUpdate( + target, + [], + true + ); + + if ( + newFields.some(({ path }) => + path.some((key) => key.includes('.') || key.startsWith('$')) + ) + ) { + // Some of the keys in this query are only writable via $setField/$unsetField, + // which was introduced in MongoDB 5.0. In this case we can use pipeline-style updates. + return newFields.map(({ path, value }) => { + return { + $replaceWith: ObjectGenerator.createSetFieldExpr(path, value), + }; + }); + } else { + const updateDoc: { $set?: BSONObject; $unset?: BSONObject } = {}; + for (const { path, value } of newFields) { + if (value === DoesNotExist) { + updateDoc.$unset ??= {}; + updateDoc.$unset[path.join('.')] = true; + } else { + updateDoc.$set ??= {}; + updateDoc.$set[path.join('.')] = value; + } + } + return updateDoc; + } + } } export default ObjectGenerator; diff --git a/packages/hadron-document/test/document.test.ts b/packages/hadron-document/test/document.test.ts index 6412c89ac38..307b5a7312e 100644 --- a/packages/hadron-document/test/document.test.ts +++ b/packages/hadron-document/test/document.test.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import Document from '../src/'; import SharedExamples from './shared-examples'; -import { ObjectId, Long } from 'bson'; +import { ObjectId, Long, Int32 } from 'bson'; describe('Document', function () { describe('#get', function () { @@ -325,7 +325,7 @@ describe('Document', function () { }); }); - describe('#getOriginalKeysAndValuesForSpecifiedKeys', function () { + describe('#getQueryForOriginalKeysAndValuesForSpecifiedFields', function () { context('when an element is removed', function () { const object = { name: 'test', ignored: 'ignored' }; const doc = new Document(object); @@ -336,7 +336,7 @@ describe('Document', function () { it('includes the element in the object', function () { expect( - doc.getOriginalKeysAndValuesForSpecifiedKeys({ name: 1 }) + doc.getQueryForOriginalKeysAndValuesForSpecifiedKeys(['name']) ).to.deep.equal({ name: 'test' }); }); }); @@ -347,7 +347,7 @@ describe('Document', function () { it('includes the element in the object', function () { expect( - doc.getOriginalKeysAndValuesForSpecifiedKeys({ name: 1 }) + doc.getQueryForOriginalKeysAndValuesForSpecifiedKeys(['name']) ).to.deep.equal({ name: 'test' }); }); }); @@ -362,7 +362,7 @@ describe('Document', function () { it('includes the element in the object', function () { expect( - doc.getOriginalKeysAndValuesForSpecifiedKeys({ name: 1 }) + doc.getQueryForOriginalKeysAndValuesForSpecifiedKeys(['name']) ).to.deep.equal({ name: 'test' }); }); }); @@ -377,7 +377,7 @@ describe('Document', function () { it('includes the element in the object', function () { expect( - doc.getOriginalKeysAndValuesForSpecifiedKeys({ name: 1 }) + doc.getQueryForOriginalKeysAndValuesForSpecifiedKeys(['name']) ).to.deep.equal({ name: 'test' }); }); }); @@ -392,7 +392,7 @@ describe('Document', function () { it('includes the element in the object', function () { expect( - doc.getOriginalKeysAndValuesForSpecifiedKeys({ name: 1 }) + doc.getQueryForOriginalKeysAndValuesForSpecifiedKeys(['name']) ).to.deep.equal({ name: 'test' }); }); }); @@ -693,7 +693,7 @@ describe('Document', function () { }); }); - describe('#generateUpdateUnlessChangedInBackgroundQuery', function () { + describe('#generateUpdateUnlessChangedInBackgroundQuery: query', function () { context('when called with an edited document', function () { const doc = { _id: 'testing', name: 'Beach Sand', yes: 'no' }; const hadronDoc = new Document(doc); @@ -711,7 +711,7 @@ describe('Document', function () { it('contains keys that were explicitly requested', function () { const { query } = - hadronDoc.generateUpdateUnlessChangedInBackgroundQuery({ yes: 1 }); + hadronDoc.generateUpdateUnlessChangedInBackgroundQuery(['yes']); expect(query).to.deep.equal({ _id: 'testing', @@ -745,6 +745,7 @@ describe('Document', function () { expect(query).to.deep.equal({ _id: 'testing', name: 'Beach Sand', + newname: { $exists: false }, }); }); @@ -779,7 +780,7 @@ describe('Document', function () { expect(query).to.deep.equal({ _id: 'testing', - a: { nestedField1: 'abc', nestedField2: 'aaa' }, + 'a.nestedField1': 'abc', }); }); @@ -789,7 +790,7 @@ describe('Document', function () { expect(updateDoc).to.deep.equal({ $set: { - a: { nestedField1: 'cba', nestedField2: 'aaa' }, + 'a.nestedField1': 'cba', }, }); }); @@ -809,7 +810,8 @@ describe('Document', function () { expect(query).to.deep.equal({ _id: 'testing', - a: { nestedField1: 'abc', bbb: 'vvv' }, + 'a.nestedField1': 'abc', + 'a.newname': { $exists: false }, }); }); @@ -819,7 +821,10 @@ describe('Document', function () { expect(updateDoc).to.deep.equal({ $set: { - a: { newname: 'abc', bbb: 'vvv' }, + 'a.newname': 'abc', + }, + $unset: { + 'a.nestedField1': true, }, }); }); @@ -842,7 +847,7 @@ describe('Document', function () { expect(query).to.deep.equal({ _id: 'testing', - a: { nestedField1: 'abc', nestedField2: 'aaa' }, + 'a.nestedField1': 'abc', }); }); @@ -851,8 +856,8 @@ describe('Document', function () { hadronDoc.generateUpdateUnlessChangedInBackgroundQuery(); expect(updateDoc).to.deep.equal({ - $set: { - a: { nestedField2: 'aaa' }, + $unset: { + 'a.nestedField1': true, }, }); }); @@ -892,8 +897,9 @@ describe('Document', function () { it('includes the key in the object', function () { expect( - doc.getOriginalKeysAndValuesForFieldsThatWereUpdated() + doc.generateUpdateUnlessChangedInBackgroundQuery().query ).to.deep.equal({ + _id: null, name: 'test', }); }); @@ -905,8 +911,10 @@ describe('Document', function () { it('returns an empty object', function () { expect( - doc.getOriginalKeysAndValuesForFieldsThatWereUpdated() - ).to.deep.equal({}); + doc.generateUpdateUnlessChangedInBackgroundQuery().query + ).to.deep.equal({ + _id: null, + }); }); }); @@ -920,8 +928,9 @@ describe('Document', function () { it('includes the original in the object', function () { expect( - doc.getOriginalKeysAndValuesForFieldsThatWereUpdated() + doc.generateUpdateUnlessChangedInBackgroundQuery().query ).to.deep.equal({ + _id: null, name: 'test', }); }); @@ -937,9 +946,11 @@ describe('Document', function () { it('includes the original in the object', function () { expect( - doc.getOriginalKeysAndValuesForFieldsThatWereUpdated() + doc.generateUpdateUnlessChangedInBackgroundQuery().query ).to.deep.equal({ + _id: null, name: 'test', + aa: { $exists: false }, }); }); }); @@ -960,12 +971,10 @@ describe('Document', function () { it('returns the original element in the object', function () { expect( - doc.getOriginalKeysAndValuesForFieldsThatWereUpdated() + doc.generateUpdateUnlessChangedInBackgroundQuery().query ).to.deep.equal({ - name: { - first: 'jimmy', - last: 'hendrix', - }, + _id: null, + 'name.last': 'hendrix', }); }); }); @@ -980,8 +989,9 @@ describe('Document', function () { it('includes the change in the object', function () { expect( - doc.getOriginalKeysAndValuesForFieldsThatWereUpdated() + doc.generateUpdateUnlessChangedInBackgroundQuery().query ).to.deep.equal({ + _id: null, name: 'test', }); }); @@ -998,8 +1008,10 @@ describe('Document', function () { it('does not have any element in the object', function () { expect( - doc.getOriginalKeysAndValuesForFieldsThatWereUpdated() - ).to.deep.equal({}); + doc.generateUpdateUnlessChangedInBackgroundQuery().query + ).to.deep.equal({ + _id: null, + }); }); }); @@ -1013,8 +1025,9 @@ describe('Document', function () { it('includes a check that the new element doesnt exist or exists with the same value', function () { expect( - doc.getOriginalKeysAndValuesForFieldsThatWereUpdated() + doc.generateUpdateUnlessChangedInBackgroundQuery().query ).to.deep.equal({ + _id: null, pineapple: { $exists: false, }, @@ -1038,18 +1051,16 @@ describe('Document', function () { it('returns the original element in the object', function () { expect( - doc.getOriginalKeysAndValuesForFieldsThatWereUpdated() + doc.generateUpdateUnlessChangedInBackgroundQuery().query ).to.deep.equal({ - name: { - first: 'jimmy', - last: 'hendrix', - }, + _id: null, + 'name.last': 'hendrix', }); }); }); }); - describe('#getSetUpdateForDocumentChanges', function () { + describe('#generateUpdateUnlessChangedInBackgroundQuery: $set', function () { context('when an element is removed', function () { const object = { name: 'test' }; const doc = new Document(object); @@ -1059,7 +1070,9 @@ describe('Document', function () { }); it('does not include the element in the object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({}); + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$set + ).to.equal(undefined); }); }); @@ -1068,7 +1081,9 @@ describe('Document', function () { const doc = new Document(object); it('returns an empty object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({}); + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$set + ).to.equal(undefined); }); }); @@ -1081,7 +1096,9 @@ describe('Document', function () { }); it('does not include the element in the object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({}); + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$set + ).to.equal(undefined); }); }); @@ -1094,7 +1111,9 @@ describe('Document', function () { }); it('includes the element in the object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({ + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$set + ).to.deep.equal({ aa: 'test', }); }); @@ -1114,11 +1133,10 @@ describe('Document', function () { }); it('includes the element in the object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({ - name: { - first: 'jimmy', - last: 'aa', - }, + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$set + ).to.deep.equal({ + 'name.last': 'aa', }); }); }); @@ -1137,10 +1155,84 @@ describe('Document', function () { }); it('includes the element in the object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({ - name: { - first: 'jimmy', - aa: 'hendrix', + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery() + ).to.deep.equal({ + query: { + _id: null, + 'name.aa': { $exists: false }, + 'name.last': 'hendrix', + }, + updateDoc: { + $set: { + 'name.aa': 'hendrix', + }, + $unset: { + 'name.last': true, + }, + }, + }); + }); + }); + + context('when an element is changed to a nested document', function () { + const object = { + name: 42, + }; + const doc = new Document(object); + + before(function () { + doc.get('name')?.changeType('Object'); + doc.get('name')?.insertEnd('first', 'jimmy'); + doc.get('name')?.insertEnd('last', 'hendrix'); + }); + + it('includes the element in the object', function () { + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery() + ).to.deep.equal({ + query: { + _id: null, + name: new Int32(42), + }, + updateDoc: { + $set: { + name: { first: 'jimmy', last: 'hendrix' }, + }, + }, + }); + }); + }); + + context('when an element is changed from nested document', function () { + const object = { + name: { + first: 'jimmy', + last: 'hendrix', + }, + }; + const doc = new Document(object); + + before(function () { + doc.get('name')?.changeType('Int32'); + doc.get('name')?.edit(new Int32(42)); + }); + + it('includes the element in the object', function () { + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery() + ).to.deep.equal({ + query: { + _id: null, + name: { + first: 'jimmy', + last: 'hendrix', + }, + }, + updateDoc: { + $set: { + name: new Int32(42), + }, }, }); }); @@ -1155,7 +1247,9 @@ describe('Document', function () { }); it('does not include the change in the object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({}); + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$set + ).to.equal(undefined); }); }); @@ -1169,7 +1263,9 @@ describe('Document', function () { }); it('does not include the change in the object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({}); + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$set + ).to.equal(undefined); }); }); @@ -1182,7 +1278,9 @@ describe('Document', function () { }); it('includes the change in the object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({ + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$set + ).to.deep.equal({ pineapple: 'hat', }); }); @@ -1202,16 +1300,22 @@ describe('Document', function () { }); it('does includes the top level element in the object', function () { - expect(doc.getSetUpdateForDocumentChanges()).to.deep.equal({ - name: { - first: 'jimmy', - }, + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$set + ).to.equal(undefined); + }); + + it('includes it in the unset part of the query', function () { + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$unset + ).to.deep.equal({ + 'name.last': true, }); }); }); }); - describe('#getUnsetUpdateForDocumentChanges', function () { + describe('#generateUpdateUnlessChangedInBackgroundQuery: $unset', function () { context('when an element is removed', function () { const object = { name: 'test' }; const doc = new Document(object); @@ -1221,7 +1325,9 @@ describe('Document', function () { }); it('includes the key in the object', function () { - expect(doc.getUnsetUpdateForDocumentChanges()).to.deep.equal({ + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$unset + ).to.deep.equal({ name: true, }); }); @@ -1231,8 +1337,10 @@ describe('Document', function () { const object = { name: 'test' }; const doc = new Document(object); - it('returns an empty object', function () { - expect(doc.getUnsetUpdateForDocumentChanges()).to.deep.equal({}); + it('returns undefined', function () { + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$unset + ).to.equal(undefined); }); }); @@ -1245,7 +1353,9 @@ describe('Document', function () { }); it('has the original key in the object', function () { - expect(doc.getUnsetUpdateForDocumentChanges()).to.deep.equal({ + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$unset + ).to.deep.equal({ name: true, }); }); @@ -1261,7 +1371,9 @@ describe('Document', function () { }); it('does not include the change in the object', function () { - expect(doc.getUnsetUpdateForDocumentChanges()).to.deep.equal({}); + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$unset + ).to.equal(undefined); }); }); @@ -1274,7 +1386,9 @@ describe('Document', function () { }); it('does not have any change in the object', function () { - expect(doc.getUnsetUpdateForDocumentChanges()).to.deep.equal({}); + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$unset + ).to.equal(undefined); }); }); @@ -1287,12 +1401,61 @@ describe('Document', function () { }); it('includes the original key in the object', function () { - expect(doc.getUnsetUpdateForDocumentChanges()).to.deep.equal({ + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$unset + ).to.deep.equal({ name: true, }); }); }); + context('when two elements is renamed in a circular manner', function () { + const object = { a: 'test1', b: 'test2' }; + const doc = new Document(object); + + before(function () { + doc.elements.get('b')?.rename('c'); + doc.elements.get('a')?.rename('b'); + }); + + it('generates the proper query and update for that situation', function () { + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery() + ).to.deep.equal({ + query: { _id: null, b: 'test2', a: 'test1', c: { $exists: false } }, + updateDoc: { + $set: { b: 'test1', c: 'test2' }, + $unset: { a: true }, + }, + }); + }); + }); + + context( + 'when an element is renamed to the name of a removed element', + function () { + const object = { a: 'test1', b: 'test2' }; + const doc = new Document(object); + + before(function () { + doc.elements.get('b')?.remove(); + doc.elements.get('a')?.rename('b'); + }); + + it('generates the proper query and update for that situation', function () { + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery() + ).to.deep.equal({ + query: { _id: null, b: 'test2', a: 'test1' }, + updateDoc: { + $set: { b: 'test1' }, + $unset: { a: true }, + }, + }); + }); + } + ); + context('when a nested element is edited', function () { const object = { name: { @@ -1306,8 +1469,10 @@ describe('Document', function () { doc.get('name')?.get('last')?.edit('aa'); }); - it('returns empty object', function () { - expect(doc.getUnsetUpdateForDocumentChanges()).to.deep.equal({}); + it('returns undefined', function () { + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$unset + ).to.equal(undefined); }); }); @@ -1325,7 +1490,283 @@ describe('Document', function () { }); it('does not include the element in the object', function () { - expect(doc.getUnsetUpdateForDocumentChanges()).to.deep.equal({}); + expect( + doc.generateUpdateUnlessChangedInBackgroundQuery().updateDoc.$unset + ).to.deep.equal({ + 'name.last': true, + }); + }); + }); + }); + + describe('array modifications', function () { + it('can add array elements', function () { + const doc = new Document({ + a: [1, 2, 3], + }); + doc.get('a')?.insertEnd(3, new Int32(4)); + expect(doc.generateUpdateUnlessChangedInBackgroundQuery()).to.deep.equal({ + query: { _id: null, 'a.3': { $exists: false } }, + updateDoc: { $set: { 'a.3': new Int32(4) } }, + }); + }); + it('can edit array elements', function () { + const doc = new Document({ + a: [1, 2, 3], + }); + doc.get('a')?.get(2)?.edit(new Int32(4)); + expect(doc.generateUpdateUnlessChangedInBackgroundQuery()).to.deep.equal({ + query: { _id: null, 'a.2': new Int32(3) }, + updateDoc: { $set: { 'a.2': new Int32(4) } }, + }); + }); + it('can insert array elements', function () { + const doc = new Document({ + a: [1, 2, 3], + }); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + doc.get('a')?.insertAfter(doc.get('a')!.get(1)!, 2, new Int32(4)); + expect(doc.generateUpdateUnlessChangedInBackgroundQuery()).to.deep.equal({ + query: { _id: null, 'a.2': new Int32(3), 'a.3': { $exists: false } }, + updateDoc: { $set: { 'a.2': new Int32(4), 'a.3': new Int32(3) } }, + }); + }); + it('can remove array elements in the middle of the array', function () { + const doc = new Document({ + a: [1, 2, 3], + }); + doc.get('a')?.get(1)?.remove(); + expect(doc.generateUpdateUnlessChangedInBackgroundQuery()).to.deep.equal({ + query: { + _id: null, + a: [new Int32(1), new Int32(2), new Int32(3)], + }, + updateDoc: { + $set: { a: [new Int32(1), new Int32(3)] }, + }, + }); + }); + it('can remove array elements at the end of the array', function () { + const doc = new Document({ + a: [1, 2, 3], + }); + doc.get('a')?.get(2)?.remove(); + expect(doc.generateUpdateUnlessChangedInBackgroundQuery()).to.deep.equal({ + query: { + _id: null, + a: [new Int32(1), new Int32(2), new Int32(3)], + }, + updateDoc: { + $set: { a: [new Int32(1), new Int32(2)] }, + }, + }); + }); + }); + + describe('dots & dollars', function () { + it('can perform updates on fields containing dots', function () { + const doc = new Document({ + 'a.b': { 'c.d': 'x' }, + }); + + doc.get('a.b')?.get('c.d')?.edit('y'); + + expect(doc.generateUpdateUnlessChangedInBackgroundQuery()).to.deep.equal({ + query: { + _id: null, + $expr: { + $and: [ + { + $eq: [ + { + $getField: { + field: { $literal: 'c.d' }, + input: { + $getField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + }, + }, + }, + }, + { $literal: 'x' }, + ], + }, + ], + }, + }, + updateDoc: [ + { + $replaceWith: { + $setField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + value: { + $setField: { + field: { $literal: 'c.d' }, + input: { + $getField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + }, + }, + value: { $literal: 'y' }, + }, + }, + }, + }, + }, + ], + }); + }); + + it('can perform updates on fields containing dollars', function () { + const doc = new Document({ + 'a.b': { $foo: 'x' }, + }); + + doc.get('a.b')?.get('$foo')?.edit('y'); + + expect(doc.generateUpdateUnlessChangedInBackgroundQuery()).to.deep.equal({ + query: { + _id: null, + $expr: { + $and: [ + { + $eq: [ + { + $getField: { + field: { $literal: '$foo' }, + input: { + $getField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + }, + }, + }, + }, + { $literal: 'x' }, + ], + }, + ], + }, + }, + updateDoc: [ + { + $replaceWith: { + $setField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + value: { + $setField: { + field: { $literal: '$foo' }, + input: { + $getField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + }, + }, + value: { $literal: 'y' }, + }, + }, + }, + }, + }, + ], + }); + }); + + it('can rename fields containing dots and dollars', function () { + const doc = new Document({ + 'a.b': { $foo: 'x' }, + }); + + doc.get('a.b')?.get('$foo')?.rename('$bar'); + + expect(doc.generateUpdateUnlessChangedInBackgroundQuery()).to.deep.equal({ + query: { + _id: null, + $expr: { + $and: [ + { + $eq: [ + { + $type: { + $getField: { + field: { $literal: '$bar' }, + input: { + $getField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + }, + }, + }, + }, + }, + 'missing', + ], + }, + { + $eq: [ + { + $getField: { + field: { $literal: '$foo' }, + input: { + $getField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + }, + }, + }, + }, + { $literal: 'x' }, + ], + }, + ], + }, + }, + updateDoc: [ + { + $replaceWith: { + $setField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + value: { + $setField: { + field: { $literal: '$bar' }, + input: { + $getField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + }, + }, + value: { $literal: 'x' }, + }, + }, + }, + }, + }, + { + $replaceWith: { + $setField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + value: { + $setField: { + field: { $literal: '$foo' }, + input: { + $getField: { + field: { $literal: 'a.b' }, + input: '$$ROOT', + }, + }, + value: '$$REMOVE', + }, + }, + }, + }, + }, + ], }); }); });