diff options
author | Tess Avitabile <tess.avitabile@mongodb.com> | 2017-06-27 13:06:18 -0400 |
---|---|---|
committer | Tess Avitabile <tess.avitabile@mongodb.com> | 2017-06-30 17:10:38 -0400 |
commit | 07baac065147381842a172726a5f80d7e57a6ef8 (patch) | |
tree | f358a00798025d4b848ddb5231ccd2974e3db501 | |
parent | acc6b704793fc37d5439b32b64a186a500436a36 (diff) | |
download | mongo-07baac065147381842a172726a5f80d7e57a6ef8.tar.gz |
SERVER-29162 UpdateNode implementation should only validate modified fields
43 files changed, 3177 insertions, 573 deletions
diff --git a/src/mongo/db/auth/authz_manager_external_state_mock.cpp b/src/mongo/db/auth/authz_manager_external_state_mock.cpp index 8bdd5e71191..e735ab1757d 100644 --- a/src/mongo/db/auth/authz_manager_external_state_mock.cpp +++ b/src/mongo/db/auth/authz_manager_external_state_mock.cpp @@ -189,8 +189,16 @@ Status AuthzManagerExternalStateMock::updateOne(OperationContext* opCtx, mmb::Document document; if (status.isOK()) { document.reset(*iter, mmb::Document::kInPlaceDisabled); + const BSONObj emptyOriginal; + const bool validateForStorage = false; + const FieldRefSet emptyImmutablePaths; BSONObj logObj; - status = driver.update(StringData(), &document, &logObj); + status = driver.update(StringData(), + emptyOriginal, + &document, + validateForStorage, + emptyImmutablePaths, + &logObj); if (!status.isOK()) return status; BSONObj newObj = document.getObject().copy(); @@ -206,11 +214,21 @@ Status AuthzManagerExternalStateMock::updateOne(OperationContext* opCtx, if (query.hasField("_id")) { document.root().appendElement(query["_id"]).transitional_ignore(); } - status = driver.populateDocumentWithQueryFields(opCtx, query, NULL, document); + const FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + invariant(immutablePaths.insert(&idFieldRef)); + status = driver.populateDocumentWithQueryFields(opCtx, query, immutablePaths, document); if (!status.isOK()) { return status; } - status = driver.update(StringData(), &document); + + // The original document can be empty because it is only needed for validation of immutable + // paths. + const BSONObj emptyOriginal; + const bool validateForStorage = false; + const FieldRefSet emptyImmutablePaths; + status = driver.update( + StringData(), emptyOriginal, &document, validateForStorage, emptyImmutablePaths); if (!status.isOK()) { return status; } diff --git a/src/mongo/db/auth/role_graph_update.cpp b/src/mongo/db/auth/role_graph_update.cpp index d8148f56132..12296e50286 100644 --- a/src/mongo/db/auth/role_graph_update.cpp +++ b/src/mongo/db/auth/role_graph_update.cpp @@ -186,12 +186,22 @@ Status handleOplogUpdate(OperationContext* opCtx, status = AuthorizationManager::getBSONForRole(roleGraph, roleToUpdate, roleDocument.root()); if (status == ErrorCodes::RoleNotFound) { // The query pattern will only contain _id, no other immutable fields are present - status = driver.populateDocumentWithQueryFields(opCtx, queryPattern, NULL, roleDocument); + const FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + invariant(immutablePaths.insert(&idFieldRef)); + status = driver.populateDocumentWithQueryFields( + opCtx, queryPattern, immutablePaths, roleDocument); } if (!status.isOK()) return status; - status = driver.update(StringData(), &roleDocument); + // The original document can be empty because it is only needed for validation of immutable + // paths. + const BSONObj emptyOriginal; + const bool validateForStorage = false; + const FieldRefSet emptyImmutablePaths; + status = driver.update( + StringData(), emptyOriginal, &roleDocument, validateForStorage, emptyImmutablePaths); if (!status.isOK()) return status; diff --git a/src/mongo/db/exec/update.cpp b/src/mongo/db/exec/update.cpp index 87145233e3d..c3dae9389fb 100644 --- a/src/mongo/db/exec/update.cpp +++ b/src/mongo/db/exec/update.cpp @@ -35,9 +35,7 @@ #include <algorithm> #include "mongo/base/status_with.h" -#include "mongo/bson/bson_depth.h" #include "mongo/bson/mutable/algorithm.h" -#include "mongo/db/bson/dotted_path_support.h" #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/exec/scoped_timer.h" #include "mongo/db/exec/working_set_common.h" @@ -49,6 +47,7 @@ #include "mongo/db/s/collection_metadata.h" #include "mongo/db/s/collection_sharding_state.h" #include "mongo/db/service_context.h" +#include "mongo/db/update/storage_validation.h" #include "mongo/stdx/memory.h" #include "mongo/util/log.h" #include "mongo/util/scopeguard.h" @@ -62,416 +61,12 @@ using std::vector; using stdx::make_unique; namespace mb = mutablebson; -namespace dps = ::mongo::dotted_path_support; namespace { const char idFieldName[] = "_id"; const FieldRef idFieldRef(idFieldName); -StatusWith<std::uint32_t> storageValid(const mb::Document&, - bool deep, - std::uint32_t recursionLevel); -StatusWith<std::uint32_t> storageValid(const mb::ConstElement&, - bool deep, - std::uint32_t recursionLevel); -StatusWith<std::uint32_t> storageValidChildren(const mb::ConstElement&, - bool deep, - std::uint32_t recursionLevel); - -/** - * Validates that the MutableBSON document 'doc' is acceptable for storage in a collection. If - * 'deep' is true, the check is performed recursively on subdocuments. - * - * An error is returned if the validation fails or if 'recursionLevel' exceeds the maximum allowable - * depth. On success, an integer is returned that represents the nesting depth of this document. - */ -StatusWith<std::uint32_t> storageValid(const mb::Document& doc, - bool deep, - std::uint32_t recursionLevel) { - if (recursionLevel > BSONDepth::getMaxDepthForUserStorage()) { - return Status(ErrorCodes::Overflow, - str::stream() << "Document exceeds maximum nesting depth of " - << BSONDepth::getMaxDepthForUserStorage()); - } - - mb::ConstElement currElem = doc.root().leftChild(); - std::uint32_t greatestDepth = recursionLevel; - while (currElem.ok()) { - if (currElem.getFieldName() == idFieldName) { - switch (currElem.getType()) { - case RegEx: - case Array: - case Undefined: - return Status(ErrorCodes::InvalidIdField, - str::stream() << "The '_id' value cannot be of type " - << typeName(currElem.getType())); - default: - break; - } - } - - // Get the nesting depth of this child element. - auto depth = storageValid(currElem, deep, recursionLevel + 1); - if (!depth.isOK()) { - return depth; - } - - // The depth of this document is the depth of its deepest child, so we only keep track of - // the maximum depth seen so far. - greatestDepth = std::max(greatestDepth, depth.getValue()); - currElem = currElem.rightSibling(); - } - - return greatestDepth; -} - -/** - * Validates an element that has a field name which starts with a dollar sign ($). - * In the case of a DBRef field ($id, $ref, [$db]) these fields may be valid in - * the correct order/context only. - */ -Status validateDollarPrefixElement(const mb::ConstElement elem, const bool deep) { - mb::ConstElement curr = elem; - StringData currName = elem.getFieldName(); - LOG(5) << "validateDollarPrefixElement -- validating field '" << currName << "'"; - // Found a $db field - if (currName == "$db") { - if (curr.getType() != String) { - return Status(ErrorCodes::InvalidDBRef, - str::stream() << "The DBRef $db field must be a String, not a " - << typeName(curr.getType())); - } - curr = curr.leftSibling(); - - if (!curr.ok() || (curr.getFieldName() != "$id")) - return Status(ErrorCodes::InvalidDBRef, - "Found $db field without a $id before it, which is invalid."); - - currName = curr.getFieldName(); - } - - // Found a $id field - if (currName == "$id") { - // We don't care about the recursion level being accurate, as the validate() command will - // perform full validation of the updated object. - const uint32_t recursionLevel = 0; - auto depth = storageValidChildren(curr, deep, recursionLevel); - if (!depth.isOK()) { - return depth.getStatus(); - } - - curr = curr.leftSibling(); - if (!curr.ok() || (curr.getFieldName() != "$ref")) { - return Status(ErrorCodes::InvalidDBRef, - "Found $id field without a $ref before it, which is invalid."); - } - - currName = curr.getFieldName(); - } - - if (currName == "$ref") { - if (curr.getType() != String) { - return Status(ErrorCodes::InvalidDBRef, - str::stream() << "The DBRef $ref field must be a String, not a " - << typeName(curr.getType())); - } - - if (!curr.rightSibling().ok() || curr.rightSibling().getFieldName() != "$id") - return Status(ErrorCodes::InvalidDBRef, - str::stream() << "The DBRef $ref field must be " - "following by a $id field"); - } else { - // not an okay, $ prefixed field name. - return Status(ErrorCodes::DollarPrefixedFieldName, - str::stream() << "The dollar ($) prefixed field '" << elem.getFieldName() - << "' in '" - << mb::getFullName(elem) - << "' is not valid for storage."); - } - - return Status::OK(); -} - -/** - * Checks that all of the parents of the MutableBSON element 'elem' are valid for storage. Note that - * 'elem' must be in a valid state when using this function. - * - * An error is returned if the validation fails, or if 'recursionLevel' exceeds the maximum - * allowable depth. On success, an integer is returned that represents the number of steps from this - * element to the root through ancestor nodes. - */ -StatusWith<std::uint32_t> storageValidParents(const mb::ConstElement& elem, - std::uint32_t recursionLevel) { - if (recursionLevel > BSONDepth::getMaxDepthForUserStorage()) { - return Status(ErrorCodes::Overflow, - str::stream() << "Document exceeds maximum nesting depth of " - << BSONDepth::getMaxDepthForUserStorage()); - } - - const mb::ConstElement& root = elem.getDocument().root(); - if (elem != root) { - const mb::ConstElement& parent = elem.parent(); - if (parent.ok() && parent != root) { - const bool doRecursiveCheck = false; - const uint32_t parentsRecursionLevel = 0; - auto height = storageValid(parent, doRecursiveCheck, parentsRecursionLevel); - if (height.isOK()) { - height = storageValidParents(parent, recursionLevel + 1); - } - return height; - } - return recursionLevel + 1; - } - return recursionLevel; -} - -StatusWith<std::uint32_t> storageValid(const mb::ConstElement& elem, - const bool deep, - std::uint32_t recursionLevel) { - if (!elem.ok()) - return Status(ErrorCodes::BadValue, "Invalid elements cannot be stored."); - - if (recursionLevel > BSONDepth::getMaxDepthForUserStorage()) { - return Status(ErrorCodes::Overflow, - str::stream() << "Document exceeds maximum nesting depth of " - << BSONDepth::getMaxDepthForUserStorage()); - } - - // Field names of elements inside arrays are not meaningful in mutable bson, - // so we do not want to validate them. - // - // TODO: Revisit how mutable handles array field names. We going to need to make - // this better if we ever want to support ordered updates that can alter the same - // element repeatedly; see SERVER-12848. - const mb::ConstElement& parent = elem.parent(); - const bool childOfArray = parent.ok() ? (parent.getType() == mongo::Array) : false; - - if (!childOfArray) { - StringData fieldName = elem.getFieldName(); - // Cannot start with "$", unless dbref - if (fieldName[0] == '$') { - Status status = validateDollarPrefixElement(elem, deep); - if (!status.isOK()) - return status; - } - } - - if (deep) { - // Check children if there are any. - auto depth = storageValidChildren(elem, deep, recursionLevel); - if (!depth.isOK()) { - return depth; - } - invariant(depth.getValue() >= recursionLevel); - return depth.getValue(); - } - - return recursionLevel; -} - -StatusWith<std::uint32_t> storageValidChildren(const mb::ConstElement& elem, - const bool deep, - std::uint32_t recursionLevel) { - if (!elem.hasChildren()) { - return recursionLevel; - } - - std::uint32_t greatestDepth = recursionLevel; - mb::ConstElement curr = elem.leftChild(); - while (curr.ok()) { - auto depth = storageValid(curr, deep, recursionLevel + 1); - if (!depth.isOK()) { - return depth.getStatus(); - } - - // Find the maximum depth amongst all of the children of 'elem'. - greatestDepth = std::max(greatestDepth, depth.getValue()); - curr = curr.rightSibling(); - } - - return greatestDepth; -} - -/** - * This will verify that all updated fields are - * 1.) Valid for storage (checking parent to make sure things like DBRefs are valid) - * 2.) Compare updated immutable fields do not change values - * - * If updateFields is empty then it was replacement and/or we need to check all fields - */ -inline Status validate(const BSONObj& original, - const FieldRefSet& updatedFields, - const mb::Document& updated, - const std::vector<std::unique_ptr<FieldRef>>* immutableAndSingleValueFields, - const ModifierInterface::Options& opts) { - LOG(3) << "update validate options -- " - << " updatedFields: " << updatedFields << " immutableAndSingleValueFields.size:" - << (immutableAndSingleValueFields ? immutableAndSingleValueFields->size() : 0) - << " validate:" << opts.enforceOkForStorage; - - // 1.) Loop through each updated field and validate for storage - // and detect immutable field updates - - // The set of possibly changed immutable fields -- we will need to check their vals - FieldRefSet changedImmutableFields; - - // Check to see if there were no fields specified or if we are not validating - // The case if a range query, or query that didn't result in saved fields - if (updatedFields.empty() || !opts.enforceOkForStorage) { - if (opts.enforceOkForStorage) { - // No specific fields were updated so the whole doc must be checked - const bool doRecursiveCheck = true; - const std::uint32_t recursionLevel = 0; - auto documentDepth = storageValid(updated, doRecursiveCheck, recursionLevel); - if (!documentDepth.isOK()) { - return documentDepth.getStatus(); - } - } - - // Check all immutable fields - if (immutableAndSingleValueFields) { - changedImmutableFields.fillFrom( - transitional_tools_do_not_use::unspool_vector(*immutableAndSingleValueFields)); - } - } else { - // TODO: Change impl so we don't need to create a new FieldRefSet - // -- move all conflict logic into static function on FieldRefSet? - FieldRefSet immutableFieldRef; - if (immutableAndSingleValueFields) { - immutableFieldRef.fillFrom( - transitional_tools_do_not_use::unspool_vector(*immutableAndSingleValueFields)); - } - - FieldRefSet::const_iterator where = updatedFields.begin(); - const FieldRefSet::const_iterator end = updatedFields.end(); - for (; where != end; ++where) { - const FieldRef& current = **where; - - // Find the updated field in the updated document. - mutablebson::ConstElement newElem = updated.root(); - size_t currentPart = 0; - while (newElem.ok() && currentPart < current.numParts()) - newElem = newElem[current.getPart(currentPart++)]; - - // newElem might be missing if $unset/$renamed-away - if (newElem.ok()) { - // Check element, and its children - const bool doRecursiveCheck = true; - const std::uint32_t recursionLevel = 0; - auto newElemDepth = storageValid(newElem, doRecursiveCheck, recursionLevel); - if (!newElemDepth.isOK()) { - return newElemDepth.getStatus(); - } - - // Check parents to make sure they are valid as well. - auto parentsDepth = storageValidParents(newElem, recursionLevel); - if (!parentsDepth.isOK()) { - return parentsDepth.getStatus(); - } - - // Ensure that the combined depths of both the new element and its parents do not - // exceed the maximum BSON depth. - if (newElemDepth.getValue() + parentsDepth.getValue() > - BSONDepth::getMaxDepthForUserStorage()) { - return {ErrorCodes::Overflow, - str::stream() << "Update operation causes document to exceed maximum " - "nesting depth of " - << BSONDepth::getMaxDepthForUserStorage()}; - } - } - // Check if the updated field conflicts with immutable fields - immutableFieldRef.findConflicts(¤t, &changedImmutableFields); - } - } - - const bool checkIdField = (updatedFields.empty() && !original.isEmpty()) || - updatedFields.findConflicts(&idFieldRef, NULL); - - // Add _id to fields to check since it too is immutable - if (checkIdField) - changedImmutableFields.keepShortest(&idFieldRef); - else if (changedImmutableFields.empty()) { - // Return early if nothing changed which is immutable - return Status::OK(); - } - - LOG(4) << "Changed immutable fields: " << changedImmutableFields; - // 2.) Now compare values of the changed immutable fields (to make sure they haven't) - - const mutablebson::ConstElement newIdElem = updated.root()[idFieldName]; - - FieldRefSet::const_iterator where = changedImmutableFields.begin(); - const FieldRefSet::const_iterator end = changedImmutableFields.end(); - for (; where != end; ++where) { - const FieldRef& current = **where; - - // Find the updated field in the updated document. - mutablebson::ConstElement newElem = updated.root(); - size_t currentPart = 0; - while (newElem.ok() && currentPart < current.numParts()) - newElem = newElem[current.getPart(currentPart++)]; - - if (!newElem.ok()) { - if (original.isEmpty()) { - // If the _id is missing and not required, then skip this check - if (!(current.dottedField() == idFieldName)) - return Status(ErrorCodes::NoSuchKey, - mongoutils::str::stream() << "After applying the update, the new" - << " document was missing the '" - << current.dottedField() - << "' (required and immutable) field."); - - } else { - if (current.dottedField() != idFieldName) - return Status(ErrorCodes::ImmutableField, - mongoutils::str::stream() - << "After applying the update to the document with " - << newIdElem.toString() - << ", the '" - << current.dottedField() - << "' (required and immutable) field was " - "found to have been removed --" - << original); - } - } else { - // Find the potentially affected field in the original document. - const BSONElement oldElem = dps::extractElementAtPath(original, current.dottedField()); - const BSONElement oldIdElem = original.getField(idFieldName); - - // Ensure no arrays since neither _id nor shard keys can be in an array, or one. - mb::ConstElement currElem = newElem; - while (currElem.ok()) { - if (currElem.getType() == Array) { - return Status( - ErrorCodes::NotSingleValueField, - mongoutils::str::stream() - << "After applying the update to the document {" - << (oldIdElem.ok() ? oldIdElem.toString() : newIdElem.toString()) - << " , ...}, the (immutable) field '" - << current.dottedField() - << "' was found to be an array or array descendant."); - } - currElem = currElem.parent(); - } - - // If we have both (old and new), compare them. If we just have new we are good - if (oldElem.ok() && newElem.compareWithBSONElement(oldElem, nullptr, false) != 0) { - return Status(ErrorCodes::ImmutableField, - mongoutils::str::stream() - << "After applying the update to the document {" - << oldElem.toString() - << " , ...}, the (immutable) field '" - << current.dottedField() - << "' was found to have been altered to " - << newElem.toString()); - } - } - } - - return Status::OK(); -} - Status ensureIdFieldIsFirst(mb::Document* doc) { mb::Element idElem = mb::findFirstChildNamed(doc->root(), idFieldName); @@ -505,6 +100,31 @@ Status addObjectIDIdField(mb::Document* doc) { } /** + * Uasserts if any of the paths in 'immutablePaths' are not present in 'document', or if they are + * arrays or array descendants. + */ +void checkImmutablePathsPresent(const mb::Document& document, const FieldRefSet& immutablePaths) { + for (auto path = immutablePaths.begin(); path != immutablePaths.end(); ++path) { + auto elem = document.root(); + for (size_t i = 0; i < (*path)->numParts(); ++i) { + elem = elem[(*path)->getPart(i)]; + uassert(ErrorCodes::NoSuchKey, + str::stream() << "After applying the update, the new document was missing the " + "required field '" + << (*path)->dottedField() + << "'", + elem.ok()); + uassert( + ErrorCodes::NotSingleValueField, + str::stream() << "After applying the update to the document, the required field '" + << (*path)->dottedField() + << "' was found to be an array or array descendant.", + elem.getType() != BSONType::Array); + } + } +} + +/** * Returns true if we should throw a WriteConflictException in order to retry the operation in the * case of a conflict. Returns false if we should skip the document and keep going. */ @@ -575,13 +195,32 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco BSONObj logObj; - FieldRefSet updatedFields; bool docWasModified = false; Status status = Status::OK(); + const bool validateForStorage = getOpCtx()->writesAreReplicated() && + !request->isFromMigration() && driver->modOptions().enforceOkForStorage; + FieldRefSet immutablePaths; + if (getOpCtx()->writesAreReplicated() && !request->isFromMigration()) { + if (lifecycle) { + auto immutablePathsVector = + getImmutableFields(getOpCtx(), request->getNamespaceString()); + if (immutablePathsVector) { + immutablePaths.fillFrom( + transitional_tools_do_not_use::unspool_vector(*immutablePathsVector)); + } + } + immutablePaths.keepShortest(&idFieldRef); + } if (!driver->needMatchDetails()) { // If we don't need match details, avoid doing the rematch - status = driver->update(StringData(), &_doc, &logObj, &updatedFields, &docWasModified); + status = driver->update(StringData(), + oldObj.value(), + &_doc, + validateForStorage, + immutablePaths, + &logObj, + &docWasModified); } else { // If there was a matched field, obtain it. MatchDetails matchDetails; @@ -594,12 +233,13 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco if (matchDetails.hasElemMatchKey()) matchedField = matchDetails.elemMatchKey(); - // TODO: Right now, each mod checks in 'prepare' that if it needs positional - // data, that a non-empty StringData() was provided. In principle, we could do - // that check here in an else clause to the above conditional and remove the - // checks from the mods. - - status = driver->update(matchedField, &_doc, &logObj, &updatedFields, &docWasModified); + status = driver->update(matchedField, + oldObj.value(), + &_doc, + validateForStorage, + immutablePaths, + &logObj, + &docWasModified); } if (!status.isOK()) { @@ -636,16 +276,6 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco } if (docWasModified) { - // Verify that no immutable fields were changed and data is valid for storage. - - if (!(!getOpCtx()->writesAreReplicated() || request->isFromMigration())) { - const std::vector<std::unique_ptr<FieldRef>>* immutableFields = nullptr; - if (lifecycle) - immutableFields = getImmutableFields(getOpCtx(), request->getNamespaceString()); - - uassertStatusOK(validate( - oldObj.value(), updatedFields, _doc, immutableFields, driver->modOptions())); - } // Prepare to write back the modified document WriteUnitOfWork wunit(getOpCtx()); @@ -748,20 +378,21 @@ Status UpdateStage::applyUpdateOpsForInsert(OperationContext* opCtx, driver->setLogOp(false); driver->setContext(ModifierInterface::ExecInfo::INSERT_CONTEXT); - const std::vector<std::unique_ptr<FieldRef>>* immutablePaths = nullptr; - if (!isInternalRequest) - immutablePaths = getImmutableFields(opCtx, ns); + FieldRefSet immutablePaths; + if (!isInternalRequest) { + auto immutablePathsVector = getImmutableFields(opCtx, ns); + if (immutablePathsVector) { + immutablePaths.fillFrom( + transitional_tools_do_not_use::unspool_vector(*immutablePathsVector)); + } + } + immutablePaths.keepShortest(&idFieldRef); // The original document we compare changes to - immutable paths must not change BSONObj original; if (cq) { - std::vector<FieldRef*> fields; - if (immutablePaths) { - fields = transitional_tools_do_not_use::unspool_vector(*immutablePaths); - } - - Status status = driver->populateDocumentWithQueryFields(*cq, &fields, *doc); + Status status = driver->populateDocumentWithQueryFields(*cq, immutablePaths, *doc); if (!status.isOK()) { return status; } @@ -776,8 +407,14 @@ Status UpdateStage::applyUpdateOpsForInsert(OperationContext* opCtx, fassert(17352, doc->root().appendElement(idElt)); } - // Apply the update modifications here. - Status updateStatus = driver->update(StringData(), doc); + // Apply the update modifications here. Do not validate for storage, since we will validate the + // entire document after the update. However, we ensure that no immutable fields are updated. + const bool validateForStorage = false; + if (isInternalRequest) { + immutablePaths.clear(); + } + Status updateStatus = + driver->update(StringData(), original, doc, validateForStorage, immutablePaths); if (!updateStatus.isOK()) { return Status(updateStatus.code(), updateStatus.reason(), 16836); } @@ -796,13 +433,10 @@ Status UpdateStage::applyUpdateOpsForInsert(OperationContext* opCtx, // that contains all the immutable keys and can be stored if it isn't coming // from a migration or via replication. if (!isInternalRequest) { - FieldRefSet noFields; - // This will only validate the modified fields if not a replacement. - Status validateStatus = - validate(original, noFields, *doc, immutablePaths, driver->modOptions()); - if (!validateStatus.isOK()) { - return validateStatus; + if (driver->modOptions().enforceOkForStorage) { + storage_validation::storageValid(*doc); } + checkImmutablePathsPresent(*doc, immutablePaths); } BSONObj newObj = doc->getObject(); diff --git a/src/mongo/db/ops/modifier_add_to_set.cpp b/src/mongo/db/ops/modifier_add_to_set.cpp index 23825863b39..acf34b2f544 100644 --- a/src/mongo/db/ops/modifier_add_to_set.cpp +++ b/src/mongo/db/ops/modifier_add_to_set.cpp @@ -290,8 +290,10 @@ Status ModifierAddToSet::apply() const { } // createPathAt() will complete the path and attach 'elemToSet' at the end of it. - Status status = pathsupport::createPathAt( - _fieldRef, _preparedState->idxFound, _preparedState->elemFound, baseArray); + Status status = + pathsupport::createPathAt( + _fieldRef, _preparedState->idxFound, _preparedState->elemFound, baseArray) + .getStatus(); if (!status.isOK()) { return status; } diff --git a/src/mongo/db/ops/modifier_bit.cpp b/src/mongo/db/ops/modifier_bit.cpp index 2e59b8a2f4e..51bfed75a64 100644 --- a/src/mongo/db/ops/modifier_bit.cpp +++ b/src/mongo/db/ops/modifier_bit.cpp @@ -256,7 +256,8 @@ Status ModifierBit::apply() const { // createPathAt() will complete the path and attach 'elemToSet' at the end of it. return pathsupport::createPathAt( - _fieldRef, _preparedState->idxFound, _preparedState->elemFound, elemToSet); + _fieldRef, _preparedState->idxFound, _preparedState->elemFound, elemToSet) + .getStatus(); } Status ModifierBit::log(LogBuilder* logBuilder) const { diff --git a/src/mongo/db/ops/modifier_compare.cpp b/src/mongo/db/ops/modifier_compare.cpp index fdf73982adf..5f579e65cc6 100644 --- a/src/mongo/db/ops/modifier_compare.cpp +++ b/src/mongo/db/ops/modifier_compare.cpp @@ -167,7 +167,8 @@ Status ModifierCompare::apply() const { // createPathAt() will complete the path and attach 'elemToSet' at the end of it. return pathsupport::createPathAt( - _updatePath, _preparedState->idxFound, _preparedState->elemFound, elemToSet); + _updatePath, _preparedState->idxFound, _preparedState->elemFound, elemToSet) + .getStatus(); } Status ModifierCompare::log(LogBuilder* logBuilder) const { diff --git a/src/mongo/db/ops/modifier_current_date.cpp b/src/mongo/db/ops/modifier_current_date.cpp index a7d301bbe86..babf1a2b82a 100644 --- a/src/mongo/db/ops/modifier_current_date.cpp +++ b/src/mongo/db/ops/modifier_current_date.cpp @@ -214,7 +214,8 @@ Status ModifierCurrentDate::apply() const { // createPathAt() will complete the path and attach 'elemToSet' at the end of it. Status s = pathsupport::createPathAt( - _updatePath, _preparedState->idxFound, _preparedState->elemFound, elemToSet); + _updatePath, _preparedState->idxFound, _preparedState->elemFound, elemToSet) + .getStatus(); if (!s.isOK()) return s; } diff --git a/src/mongo/db/ops/modifier_inc.cpp b/src/mongo/db/ops/modifier_inc.cpp index 1692d71260f..1edca18a4fa 100644 --- a/src/mongo/db/ops/modifier_inc.cpp +++ b/src/mongo/db/ops/modifier_inc.cpp @@ -243,7 +243,8 @@ Status ModifierInc::apply() const { // createPathAt() will complete the path and attach 'elemToSet' at the end of it. return pathsupport::createPathAt( - _fieldRef, _preparedState->idxFound, _preparedState->elemFound, elemToSet); + _fieldRef, _preparedState->idxFound, _preparedState->elemFound, elemToSet) + .getStatus(); } Status ModifierInc::log(LogBuilder* logBuilder) const { diff --git a/src/mongo/db/ops/modifier_push.cpp b/src/mongo/db/ops/modifier_push.cpp index 2e8acb4de53..d09006a983c 100644 --- a/src/mongo/db/ops/modifier_push.cpp +++ b/src/mongo/db/ops/modifier_push.cpp @@ -538,7 +538,8 @@ Status ModifierPush::apply() const { // createPathAt() will complete the path and attach 'elemToSet' at the end of it. status = pathsupport::createPathAt( - _fieldRef, _preparedState->idxFound, _preparedState->elemFound, baseArray); + _fieldRef, _preparedState->idxFound, _preparedState->elemFound, baseArray) + .getStatus(); if (!status.isOK()) { return status; } diff --git a/src/mongo/db/ops/modifier_rename.cpp b/src/mongo/db/ops/modifier_rename.cpp index 20ca8d67e49..375edd4d812 100644 --- a/src/mongo/db/ops/modifier_rename.cpp +++ b/src/mongo/db/ops/modifier_rename.cpp @@ -255,7 +255,8 @@ Status ModifierRename::apply() const { return pathsupport::createPathAt(_toFieldRef, tempElem == doc.end() ? 0 : tempIdx + 1, tempElem == doc.end() ? doc.root() : tempElem, - elemToSet); + elemToSet) + .getStatus(); } Status ModifierRename::log(LogBuilder* logBuilder) const { diff --git a/src/mongo/db/ops/modifier_set.cpp b/src/mongo/db/ops/modifier_set.cpp index 77575fc76a9..86796a31314 100644 --- a/src/mongo/db/ops/modifier_set.cpp +++ b/src/mongo/db/ops/modifier_set.cpp @@ -244,7 +244,8 @@ Status ModifierSet::apply() const { // createPathAt() will complete the path and attach 'elemToSet' at the end of it. return pathsupport::createPathAt( - _fieldRef, _preparedState->idxFound, _preparedState->elemFound, elemToSet); + _fieldRef, _preparedState->idxFound, _preparedState->elemFound, elemToSet) + .getStatus(); } Status ModifierSet::log(LogBuilder* logBuilder) const { diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp index 2aa57637773..078d2f30f25 100644 --- a/src/mongo/db/ops/update.cpp +++ b/src/mongo/db/ops/update.cpp @@ -136,7 +136,14 @@ BSONObj applyUpdateOperators(const BSONObj& from, const BSONObj& operators) { } mutablebson::Document doc(from, mutablebson::Document::kInPlaceDisabled); - status = driver.update(StringData(), &doc); + + // The original document can be empty because it is only needed for validation of immutable + // paths. + const BSONObj emptyOriginal; + const bool validateForStorage = false; + const FieldRefSet emptyImmutablePaths; + status = + driver.update(StringData(), emptyOriginal, &doc, validateForStorage, emptyImmutablePaths); if (!status.isOK()) { uasserted(16839, status.reason()); } diff --git a/src/mongo/db/update/SConscript b/src/mongo/db/update/SConscript index 6a51bc49669..0ee72535349 100644 --- a/src/mongo/db/update/SConscript +++ b/src/mongo/db/update/SConscript @@ -10,6 +10,7 @@ env.Library( 'field_checker.cpp', 'log_builder.cpp', 'path_support.cpp', + 'storage_validation.cpp', ], LIBDEPS=[ '$BUILD_DIR/mongo/base', diff --git a/src/mongo/db/update/addtoset_node_test.cpp b/src/mongo/db/update/addtoset_node_test.cpp index 419540d621d..3d9fd8aefb0 100644 --- a/src/mongo/db/update/addtoset_node_test.cpp +++ b/src/mongo/db/update/addtoset_node_test.cpp @@ -112,6 +112,8 @@ TEST(AddToSetNodeTest, ApplyFailsOnNonArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -122,6 +124,8 @@ TEST(AddToSetNodeTest, ApplyFailsOnNonArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -142,6 +146,8 @@ TEST(AddToSetNodeTest, ApplyNonEach) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -153,6 +159,8 @@ TEST(AddToSetNodeTest, ApplyNonEach) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -175,6 +183,8 @@ TEST(AddToSetNodeTest, ApplyNonEachArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -186,6 +196,8 @@ TEST(AddToSetNodeTest, ApplyNonEachArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -208,6 +220,8 @@ TEST(AddToSetNodeTest, ApplyEach) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -219,6 +233,8 @@ TEST(AddToSetNodeTest, ApplyEach) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -241,6 +257,8 @@ TEST(AddToSetNodeTest, ApplyToEmptyArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -252,6 +270,8 @@ TEST(AddToSetNodeTest, ApplyToEmptyArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -274,6 +294,8 @@ TEST(AddToSetNodeTest, ApplyDeduplicateElementsToAdd) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -285,6 +307,8 @@ TEST(AddToSetNodeTest, ApplyDeduplicateElementsToAdd) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -307,6 +331,8 @@ TEST(AddToSetNodeTest, ApplyDoNotAddExistingElements) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -318,6 +344,8 @@ TEST(AddToSetNodeTest, ApplyDoNotAddExistingElements) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -340,6 +368,8 @@ TEST(AddToSetNodeTest, ApplyDoNotDeduplicateExistingElements) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -351,6 +381,8 @@ TEST(AddToSetNodeTest, ApplyDoNotDeduplicateExistingElements) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -373,6 +405,8 @@ TEST(AddToSetNodeTest, ApplyNoElementsToAdd) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -384,6 +418,8 @@ TEST(AddToSetNodeTest, ApplyNoElementsToAdd) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -406,6 +442,8 @@ TEST(AddToSetNodeTest, ApplyNoNonDuplicateElementsToAdd) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -417,6 +455,8 @@ TEST(AddToSetNodeTest, ApplyNoNonDuplicateElementsToAdd) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -439,6 +479,8 @@ TEST(AddToSetNodeTest, ApplyCreateArray) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -450,6 +492,8 @@ TEST(AddToSetNodeTest, ApplyCreateArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -472,6 +516,8 @@ TEST(AddToSetNodeTest, ApplyCreateEmptyArrayIsNotNoop) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -483,6 +529,8 @@ TEST(AddToSetNodeTest, ApplyCreateEmptyArrayIsNotNoop) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -505,6 +553,8 @@ TEST(AddToSetNodeTest, ApplyDeduplicationOfElementsToAddRespectsCollation) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -516,6 +566,8 @@ TEST(AddToSetNodeTest, ApplyDeduplicationOfElementsToAddRespectsCollation) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -538,6 +590,8 @@ TEST(AddToSetNodeTest, ApplyComparisonToExistingElementsRespectsCollation) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -549,6 +603,8 @@ TEST(AddToSetNodeTest, ApplyComparisonToExistingElementsRespectsCollation) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -575,6 +631,8 @@ TEST(AddToSetNodeTest, ApplyRespectsCollationFromSetCollator) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -586,6 +644,8 @@ TEST(AddToSetNodeTest, ApplyRespectsCollationFromSetCollator) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -629,6 +689,8 @@ TEST(AddToSetNodeTest, ApplyNestedArray) { FieldRef pathTaken("a.1"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -640,6 +702,8 @@ TEST(AddToSetNodeTest, ApplyNestedArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -662,6 +726,8 @@ TEST(AddToSetNodeTest, ApplyIndexesNotAffected) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("b"); Document logDoc; @@ -673,6 +739,8 @@ TEST(AddToSetNodeTest, ApplyIndexesNotAffected) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -695,6 +763,8 @@ TEST(AddToSetNodeTest, ApplyNoIndexDataOrLogBuilder) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -704,6 +774,8 @@ TEST(AddToSetNodeTest, ApplyNoIndexDataOrLogBuilder) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, diff --git a/src/mongo/db/update/arithmetic_node_test.cpp b/src/mongo/db/update/arithmetic_node_test.cpp index 924d66a8076..9626125c29f 100644 --- a/src/mongo/db/update/arithmetic_node_test.cpp +++ b/src/mongo/db/update/arithmetic_node_test.cpp @@ -119,6 +119,8 @@ TEST(ArithmeticNodeTest, ApplyIncNoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -130,6 +132,8 @@ TEST(ArithmeticNodeTest, ApplyIncNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -152,6 +156,8 @@ TEST(ArithmeticNodeTest, ApplyMulNoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -163,6 +169,8 @@ TEST(ArithmeticNodeTest, ApplyMulNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -185,6 +193,8 @@ TEST(ArithmeticNodeTest, ApplyRoundingNoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -196,6 +206,8 @@ TEST(ArithmeticNodeTest, ApplyRoundingNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -218,6 +230,8 @@ TEST(ArithmeticNodeTest, ApplyEmptyPathToCreate) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -229,6 +243,8 @@ TEST(ArithmeticNodeTest, ApplyEmptyPathToCreate) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -251,6 +267,8 @@ TEST(ArithmeticNodeTest, ApplyCreatePath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -262,6 +280,8 @@ TEST(ArithmeticNodeTest, ApplyCreatePath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -284,6 +304,8 @@ TEST(ArithmeticNodeTest, ApplyExtendPath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -294,6 +316,8 @@ TEST(ArithmeticNodeTest, ApplyExtendPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -315,6 +339,8 @@ TEST(ArithmeticNodeTest, ApplyCreatePathFromRoot) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -326,6 +352,8 @@ TEST(ArithmeticNodeTest, ApplyCreatePathFromRoot) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -348,6 +376,8 @@ TEST(ArithmeticNodeTest, ApplyPositional) { FieldRef pathTaken("a.1"); StringData matchedField("1"); auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -359,6 +389,8 @@ TEST(ArithmeticNodeTest, ApplyPositional) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -381,6 +413,8 @@ TEST(ArithmeticNodeTest, ApplyNonViablePathToInc) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -392,6 +426,8 @@ TEST(ArithmeticNodeTest, ApplyNonViablePathToInc) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -412,6 +448,8 @@ TEST(ArithmeticNodeTest, ApplyNonViablePathToCreateFromReplicationIsNoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -423,6 +461,8 @@ TEST(ArithmeticNodeTest, ApplyNonViablePathToCreateFromReplicationIsNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -445,6 +485,8 @@ TEST(ArithmeticNodeTest, ApplyNoIndexDataNoLogBuilder) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -454,6 +496,8 @@ TEST(ArithmeticNodeTest, ApplyNoIndexDataNoLogBuilder) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -475,6 +519,8 @@ TEST(ArithmeticNodeTest, ApplyDoesNotAffectIndexes) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("b"); LogBuilder* logBuilder = nullptr; @@ -485,6 +531,8 @@ TEST(ArithmeticNodeTest, ApplyDoesNotAffectIndexes) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -506,6 +554,8 @@ TEST(ArithmeticNodeTest, IncTypePromotionIsNotANoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -516,6 +566,8 @@ TEST(ArithmeticNodeTest, IncTypePromotionIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -537,6 +589,8 @@ TEST(ArithmeticNodeTest, MulTypePromotionIsNotANoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -547,6 +601,8 @@ TEST(ArithmeticNodeTest, MulTypePromotionIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -568,6 +624,8 @@ TEST(ArithmeticNodeTest, TypePromotionFromIntToDecimalIsNotANoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -579,6 +637,8 @@ TEST(ArithmeticNodeTest, TypePromotionFromIntToDecimalIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -601,6 +661,8 @@ TEST(ArithmeticNodeTest, TypePromotionFromLongToDecimalIsNotANoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -612,6 +674,8 @@ TEST(ArithmeticNodeTest, TypePromotionFromLongToDecimalIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -634,6 +698,8 @@ TEST(ArithmeticNodeTest, TypePromotionFromDoubleToDecimalIsNotANoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -645,6 +711,8 @@ TEST(ArithmeticNodeTest, TypePromotionFromDoubleToDecimalIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -667,6 +735,8 @@ TEST(ArithmeticNodeTest, ApplyPromoteToFloatingPoint) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -677,6 +747,8 @@ TEST(ArithmeticNodeTest, ApplyPromoteToFloatingPoint) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -698,6 +770,8 @@ TEST(ArithmeticNodeTest, IncrementedDecimalStaysDecimal) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -709,6 +783,8 @@ TEST(ArithmeticNodeTest, IncrementedDecimalStaysDecimal) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -733,6 +809,8 @@ TEST(ArithmeticNodeTest, OverflowIntToLong) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -744,6 +822,8 @@ TEST(ArithmeticNodeTest, OverflowIntToLong) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -768,6 +848,8 @@ TEST(ArithmeticNodeTest, UnderflowIntToLong) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -779,6 +861,8 @@ TEST(ArithmeticNodeTest, UnderflowIntToLong) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -802,6 +886,8 @@ TEST(ArithmeticNodeTest, IncModeCanBeReused) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -813,6 +899,8 @@ TEST(ArithmeticNodeTest, IncModeCanBeReused) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -829,6 +917,8 @@ TEST(ArithmeticNodeTest, IncModeCanBeReused) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -850,6 +940,8 @@ TEST(ArithmeticNodeTest, CreatedNumberHasSameTypeAsInc) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -860,6 +952,8 @@ TEST(ArithmeticNodeTest, CreatedNumberHasSameTypeAsInc) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -881,6 +975,8 @@ TEST(ArithmeticNodeTest, CreatedNumberHasSameTypeAsMul) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -891,6 +987,8 @@ TEST(ArithmeticNodeTest, CreatedNumberHasSameTypeAsMul) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -912,6 +1010,8 @@ TEST(ArithmeticNodeTest, ApplyEmptyDocument) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -922,6 +1022,8 @@ TEST(ArithmeticNodeTest, ApplyEmptyDocument) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -943,6 +1045,8 @@ TEST(ArithmeticNodeTest, ApplyIncToObjectFails) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -953,6 +1057,8 @@ TEST(ArithmeticNodeTest, ApplyIncToObjectFails) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -974,6 +1080,8 @@ TEST(ArithmeticNodeTest, ApplyIncToArrayFails) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -984,6 +1092,8 @@ TEST(ArithmeticNodeTest, ApplyIncToArrayFails) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1005,6 +1115,8 @@ TEST(ArithmeticNodeTest, ApplyIncToStringFails) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1015,6 +1127,8 @@ TEST(ArithmeticNodeTest, ApplyIncToStringFails) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1036,6 +1150,8 @@ TEST(ArithmeticNodeTest, ApplyNewPath) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1046,6 +1162,8 @@ TEST(ArithmeticNodeTest, ApplyNewPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1067,6 +1185,8 @@ TEST(ArithmeticNodeTest, ApplyEmptyIndexData) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1077,6 +1197,8 @@ TEST(ArithmeticNodeTest, ApplyEmptyIndexData) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1098,6 +1220,8 @@ TEST(ArithmeticNodeTest, ApplyNoOpDottedPath) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -1108,6 +1232,8 @@ TEST(ArithmeticNodeTest, ApplyNoOpDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1129,6 +1255,8 @@ TEST(ArithmeticNodeTest, TypePromotionOnDottedPathIsNotANoOp) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -1139,6 +1267,8 @@ TEST(ArithmeticNodeTest, TypePromotionOnDottedPathIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1160,6 +1290,8 @@ TEST(ArithmeticNodeTest, ApplyPathNotViableArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -1169,6 +1301,8 @@ TEST(ArithmeticNodeTest, ApplyPathNotViableArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -1189,6 +1323,8 @@ TEST(ArithmeticNodeTest, ApplyInPlaceDottedPath) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -1199,6 +1335,8 @@ TEST(ArithmeticNodeTest, ApplyInPlaceDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1220,6 +1358,8 @@ TEST(ArithmeticNodeTest, ApplyPromotionDottedPath) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -1230,6 +1370,8 @@ TEST(ArithmeticNodeTest, ApplyPromotionDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1251,6 +1393,8 @@ TEST(ArithmeticNodeTest, ApplyDottedPathEmptyDoc) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -1261,6 +1405,8 @@ TEST(ArithmeticNodeTest, ApplyDottedPathEmptyDoc) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1282,6 +1428,8 @@ TEST(ArithmeticNodeTest, ApplyFieldWithDot) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -1292,6 +1440,8 @@ TEST(ArithmeticNodeTest, ApplyFieldWithDot) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1313,6 +1463,8 @@ TEST(ArithmeticNodeTest, ApplyNoOpArrayIndex) { FieldRef pathTaken("a.2.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1323,6 +1475,8 @@ TEST(ArithmeticNodeTest, ApplyNoOpArrayIndex) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1344,6 +1498,8 @@ TEST(ArithmeticNodeTest, TypePromotionInArrayIsNotANoOp) { FieldRef pathTaken("a.2.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1354,6 +1510,8 @@ TEST(ArithmeticNodeTest, TypePromotionInArrayIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1375,6 +1533,8 @@ TEST(ArithmeticNodeTest, ApplyNonViablePathThroughArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -1384,6 +1544,8 @@ TEST(ArithmeticNodeTest, ApplyNonViablePathThroughArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -1404,6 +1566,8 @@ TEST(ArithmeticNodeTest, ApplyInPlaceArrayIndex) { FieldRef pathTaken("a.2.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1414,6 +1578,8 @@ TEST(ArithmeticNodeTest, ApplyInPlaceArrayIndex) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1435,6 +1601,8 @@ TEST(ArithmeticNodeTest, ApplyAppendArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1445,6 +1613,8 @@ TEST(ArithmeticNodeTest, ApplyAppendArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1466,6 +1636,8 @@ TEST(ArithmeticNodeTest, ApplyPaddingArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1476,6 +1648,8 @@ TEST(ArithmeticNodeTest, ApplyPaddingArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1497,6 +1671,8 @@ TEST(ArithmeticNodeTest, ApplyNumericObject) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1507,6 +1683,8 @@ TEST(ArithmeticNodeTest, ApplyNumericObject) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1528,6 +1706,8 @@ TEST(ArithmeticNodeTest, ApplyNumericField) { FieldRef pathTaken("a.2.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1538,6 +1718,8 @@ TEST(ArithmeticNodeTest, ApplyNumericField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1559,6 +1741,8 @@ TEST(ArithmeticNodeTest, ApplyExtendNumericField) { FieldRef pathTaken("a.2"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1569,6 +1753,8 @@ TEST(ArithmeticNodeTest, ApplyExtendNumericField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1590,6 +1776,8 @@ TEST(ArithmeticNodeTest, ApplyNumericFieldToEmptyObject) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1600,6 +1788,8 @@ TEST(ArithmeticNodeTest, ApplyNumericFieldToEmptyObject) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1621,6 +1811,8 @@ TEST(ArithmeticNodeTest, ApplyEmptyArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -1631,6 +1823,8 @@ TEST(ArithmeticNodeTest, ApplyEmptyArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1652,6 +1846,8 @@ TEST(ArithmeticNodeTest, ApplyLogDottedPath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1662,6 +1858,8 @@ TEST(ArithmeticNodeTest, ApplyLogDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1683,6 +1881,8 @@ TEST(ArithmeticNodeTest, LogEmptyArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1693,6 +1893,8 @@ TEST(ArithmeticNodeTest, LogEmptyArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1714,6 +1916,8 @@ TEST(ArithmeticNodeTest, LogEmptyObject) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1724,6 +1928,8 @@ TEST(ArithmeticNodeTest, LogEmptyObject) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1748,6 +1954,8 @@ TEST(ArithmeticNodeTest, ApplyDeserializedDocNotNoOp) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1758,6 +1966,8 @@ TEST(ArithmeticNodeTest, ApplyDeserializedDocNotNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1783,6 +1993,8 @@ TEST(ArithmeticNodeTest, ApplyToDeserializedDocNoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1793,6 +2005,8 @@ TEST(ArithmeticNodeTest, ApplyToDeserializedDocNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1818,6 +2032,8 @@ TEST(ArithmeticNodeTest, ApplyToDeserializedDocNestedNoop) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1828,6 +2044,8 @@ TEST(ArithmeticNodeTest, ApplyToDeserializedDocNestedNoop) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1853,6 +2071,8 @@ TEST(ArithmeticNodeTest, ApplyToDeserializedDocNestedNotNoop) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1863,6 +2083,8 @@ TEST(ArithmeticNodeTest, ApplyToDeserializedDocNestedNotNoop) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, diff --git a/src/mongo/db/update/conflict_placeholder_node.h b/src/mongo/db/update/conflict_placeholder_node.h index 02acc6b94e9..4684226e409 100644 --- a/src/mongo/db/update/conflict_placeholder_node.h +++ b/src/mongo/db/update/conflict_placeholder_node.h @@ -59,6 +59,8 @@ public: FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, diff --git a/src/mongo/db/update/path_creating_node.cpp b/src/mongo/db/update/path_creating_node.cpp index 9bc6ee793c6..97feb3fb17e 100644 --- a/src/mongo/db/update/path_creating_node.cpp +++ b/src/mongo/db/update/path_creating_node.cpp @@ -30,15 +30,90 @@ #include "mongo/db/update/path_creating_node.h" +#include "mongo/db/bson/dotted_path_support.h" #include "mongo/db/update/path_support.h" +#include "mongo/db/update/storage_validation.h" namespace mongo { +namespace { + +/** + * Checks that no immutable paths were modified in the case where we are modifying an existing path + * in the document. 'element' should be the modified element. 'pathTaken' is the path to the + * modified element. If 'pathTaken' is a strict prefix of any immutable path, 'original' should be + * provided as the preimage of the whole document. We assume that we have already checked the update + * is not a noop. + */ +void checkImmutablePathsNotModified(mutablebson::Element element, + FieldRef* pathTaken, + const FieldRefSet& immutablePaths, + BSONObj original) { + for (auto immutablePath = immutablePaths.begin(); immutablePath != immutablePaths.end(); + ++immutablePath) { + auto prefixSize = pathTaken->commonPrefixSize(**immutablePath); + + // If 'immutablePath' is a (strict or non-strict) prefix of 'pathTaken', and the update is + // not a noop, then we have modified 'immutablePath', which is immutable. + if (prefixSize == (*immutablePath)->numParts()) { + uasserted(ErrorCodes::ImmutableField, + str::stream() << "Updating the path '" << pathTaken->dottedField() << "' to " + << element.toString() + << " would modify the immutable field '" + << (*immutablePath)->dottedField() + << "'"); + } + + // If 'pathTaken' is a strict prefix of 'immutablePath', then we may have modified + // 'immutablePath'. We already know that 'pathTaken' is not equal to 'immutablePath', or we + // would have uasserted. + if (prefixSize == pathTaken->numParts()) { + auto oldElem = dotted_path_support::extractElementAtPath( + original, (*immutablePath)->dottedField()); + + // We are allowed to modify immutable paths that do not yet exist. + if (!oldElem.ok()) { + continue; + } + + auto newElem = element; + for (size_t i = pathTaken->numParts(); i < (*immutablePath)->numParts(); ++i) { + uassert(ErrorCodes::NotSingleValueField, + str::stream() + << "After applying the update to the document, the immutable field '" + << (*immutablePath)->dottedField() + << "' was found to be an array or array descendant.", + newElem.getType() != BSONType::Array); + newElem = newElem[(*immutablePath)->getPart(i)]; + if (!newElem.ok()) { + break; + } + } + + uassert(ErrorCodes::ImmutableField, + str::stream() << "After applying the update, the immutable field '" + << (*immutablePath)->dottedField() + << "' was found to have been removed.", + newElem.ok()); + uassert(ErrorCodes::ImmutableField, + str::stream() << "After applying the update, the immutable field '" + << (*immutablePath)->dottedField() + << "' was found to have been altered to " + << newElem.toString(), + newElem.compareWithBSONElement(oldElem, nullptr, false) == 0); + } + } +} + +} // namespace + void PathCreatingNode::apply(mutablebson::Element element, FieldRef* pathToCreate, FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, @@ -50,12 +125,32 @@ void PathCreatingNode::apply(mutablebson::Element element, mutablebson::Element valueToLog = element.getDocument().end(); if (pathToCreate->empty()) { + + // If 'pathTaken' is a strict prefix of any immutable path, store the original document to + // ensure the immutable path does not change. + BSONObj original; + for (auto immutablePath = immutablePaths.begin(); immutablePath != immutablePaths.end(); + ++immutablePath) { + if (pathTaken->isPrefixOf(**immutablePath)) { + original = element.getDocument().getObject(); + break; + } + } + // We found an existing element at the update path. updateExistingElement(&element, noop); if (*noop) { return; // Successful no-op update. } + if (validateForStorage) { + const bool doRecursiveCheck = true; + const uint32_t recursionLevel = pathTaken->numParts(); + storage_validation::storageValid(element, doRecursiveCheck, recursionLevel); + } + + checkImmutablePathsNotModified(element, pathTaken, immutablePaths, original); + valueToLog = element; } else { // We did not find an element at the update path. Create one. @@ -64,8 +159,9 @@ void PathCreatingNode::apply(mutablebson::Element element, setValueForNewElement(&newElement); invariant(newElement.ok()); - auto status = pathsupport::createPathAt(*pathToCreate, 0, element, newElement); - if (!status.isOK()) { + auto statusWithFirstCreatedElem = + pathsupport::createPathAt(*pathToCreate, 0, element, newElement); + if (!statusWithFirstCreatedElem.isOK()) { // $sets on non-viable paths are ignored when the update came from replication. We do // not error because idempotency requires that any other update modifiers must still be // applied. For example, consider applying the following updates twice to an initially @@ -77,14 +173,36 @@ void PathCreatingNode::apply(mutablebson::Element element, // (There are modifiers besides $set that use this code path, but they are not used for // replication, so we are not concerned with their behavior when "fromReplication" is // true.) - if (status.code() == ErrorCodes::PathNotViable && fromReplication) { + if (statusWithFirstCreatedElem.getStatus().code() == ErrorCodes::PathNotViable && + fromReplication) { *noop = true; return; } - uassertStatusOK(status); + uassertStatusOK(statusWithFirstCreatedElem); MONGO_UNREACHABLE; // The previous uassertStatusOK should always throw. } + if (validateForStorage) { + const bool doRecursiveCheck = true; + const uint32_t recursionLevel = pathTaken->numParts() + 1; + storage_validation::storageValid( + statusWithFirstCreatedElem.getValue(), doRecursiveCheck, recursionLevel); + } + + for (auto immutablePath = immutablePaths.begin(); immutablePath != immutablePaths.end(); + ++immutablePath) { + + // If 'immutablePath' is a (strict or non-strict) prefix of 'pathTaken', then we are + // modifying 'immutablePath'. For example, adding '_id.x' will illegally modify '_id'. + uassert(ErrorCodes::ImmutableField, + str::stream() << "Updating the path '" << pathTaken->dottedField() << "' to " + << element.toString() + << " would modify the immutable field '" + << (*immutablePath)->dottedField() + << "'", + pathTaken->commonPrefixSize(**immutablePath) != (*immutablePath)->numParts()); + } + valueToLog = newElement; } diff --git a/src/mongo/db/update/path_creating_node.h b/src/mongo/db/update/path_creating_node.h index 1c9406c947d..adbf770022c 100644 --- a/src/mongo/db/update/path_creating_node.h +++ b/src/mongo/db/update/path_creating_node.h @@ -46,6 +46,8 @@ public: FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, diff --git a/src/mongo/db/update/path_support.cpp b/src/mongo/db/update/path_support.cpp index 5538f581385..73a7b5528db 100644 --- a/src/mongo/db/update/path_support.cpp +++ b/src/mongo/db/update/path_support.cpp @@ -156,11 +156,12 @@ Status findLongestPrefix(const FieldRef& prefix, } } -Status createPathAt(const FieldRef& prefix, - size_t idxFound, - mutablebson::Element elemFound, - mutablebson::Element newElem) { +StatusWith<mutablebson::Element> createPathAt(const FieldRef& prefix, + size_t idxFound, + mutablebson::Element elemFound, + mutablebson::Element newElem) { Status status = Status::OK(); + auto firstNewElem = elemFound.getDocument().end(); if (elemFound.getType() != BSONType::Object && elemFound.getType() != BSONType::Array) { return Status(ErrorCodes::PathNotViable, @@ -228,11 +229,17 @@ Status createPathAt(const FieldRef& prefix, return status; } inArray = false; + if (!firstNewElem.ok()) { + firstNewElem = arrayObj; + } } else { status = elemFound.pushBack(elem); if (!status.isOK()) { return status; } + if (!firstNewElem.ok()) { + firstNewElem = elem; + } } elemFound = elem; @@ -257,14 +264,21 @@ Status createPathAt(const FieldRef& prefix, return status; } + if (!firstNewElem.ok()) { + firstNewElem = arrayObj; + } + } else { status = elemFound.pushBack(newElem); if (!status.isOK()) { return status; } + if (!firstNewElem.ok()) { + firstNewElem = newElem; + } } - return Status::OK(); + return firstNewElem; } Status setElementAtPath(const FieldRef& path, @@ -299,7 +313,7 @@ Status setElementAtPath(const FieldRef& path, StringData leafFieldName = path.getPart(path.numParts() - 1); mutablebson::Element leafElem = doc->makeElementWithNewFieldName(leafFieldName, value); dassert(leafElem.ok()); - return createPathAt(path, deepestElemPathPart, deepestElem, leafElem); + return createPathAt(path, deepestElemPathPart, deepestElem, leafElem).getStatus(); } } diff --git a/src/mongo/db/update/path_support.h b/src/mongo/db/update/path_support.h index 69106ea5010..a27be4b93f6 100644 --- a/src/mongo/db/update/path_support.h +++ b/src/mongo/db/update/path_support.h @@ -86,18 +86,18 @@ Status findLongestPrefix(const FieldRef& prefix, mutablebson::Element* elemFound); /** - * Creates the parts 'prefix[idxRoot]', 'prefix[idxRoot+1]', ..., - * 'prefix[<numParts>-1]' under 'elemFound' and adds 'newElem' as a child of that - * path. Returns OK, if successful, or an error code describing why not, otherwise. + * Creates the parts 'prefix[idxRoot]', 'prefix[idxRoot+1]', ..., 'prefix[<numParts>-1]' under + * 'elemFound' and adds 'newElem' as a child of that path. Returns the first element that was + * created along the path, if successful, or an error code describing why not, otherwise. * * createPathAt is designed to work with 'findLongestPrefix' in that it can create the * field parts in 'prefix' that are missing from a given document. 'elemFound' points * to the element in the doc that is the parent of prefix[idxRoot]. */ -Status createPathAt(const FieldRef& prefix, - size_t idxRoot, - mutablebson::Element elemFound, - mutablebson::Element newElem); +StatusWith<mutablebson::Element> createPathAt(const FieldRef& prefix, + size_t idxRoot, + mutablebson::Element elemFound, + mutablebson::Element newElem); /** * Uses the above methods to set the given value at the specified path in a mutable diff --git a/src/mongo/db/update/path_support_test.cpp b/src/mongo/db/update/path_support_test.cpp index 49675d09ac3..4a915bdf7d5 100644 --- a/src/mongo/db/update/path_support_test.cpp +++ b/src/mongo/db/update/path_support_test.cpp @@ -106,10 +106,28 @@ TEST_F(EmptyDoc, NewField) { Element newElem = doc().makeElementInt("a", 1); ASSERT_TRUE(newElem.ok()); - ASSERT_OK(createPathAt(field(), 0, root(), newElem)); + auto firstNewElem = createPathAt(field(), 0, root(), newElem); + ASSERT_OK(firstNewElem); + ASSERT_EQUALS(firstNewElem.getValue().compareWithElement(root()["a"], nullptr), 0); ASSERT_EQUALS(fromjson("{a: 1}"), doc()); } +TEST_F(EmptyDoc, NewPath) { + setField("a.b.c"); + + size_t idxFound; + Element elemFound = root(); + Status status = findLongestPrefix(field(), root(), &idxFound, &elemFound); + ASSERT_EQUALS(status, ErrorCodes::NonExistentPath); + + Element newElem = doc().makeElementInt("c", 1); + ASSERT_TRUE(newElem.ok()); + auto firstNewElem = createPathAt(field(), 0, root(), newElem); + ASSERT_OK(firstNewElem); + ASSERT_EQUALS(firstNewElem.getValue().compareWithElement(root()["a"], nullptr), 0); + ASSERT_EQUALS(fromjson("{a: {b: {c: 1}}}"), doc()); +} + class SimpleDoc : public mongo::unittest::Test { public: SimpleDoc() : _doc() {} @@ -184,7 +202,9 @@ TEST_F(SimpleDoc, NotCommonPrefix) { ASSERT_TRUE(newElem.ok()); ASSERT_EQUALS(countChildren(root()), 1u); - ASSERT_OK(createPathAt(field(), 0, root(), newElem)); + auto firstNewElem = createPathAt(field(), 0, root(), newElem); + ASSERT_OK(firstNewElem); + ASSERT_EQUALS(firstNewElem.getValue().compareWithElement(root()["b"], nullptr), 0); ASSERT_EQUALS(newElem.getFieldName(), "b"); ASSERT_EQUALS(newElem.getType(), NumberInt); ASSERT_TRUE(newElem.hasValue()); @@ -206,8 +226,8 @@ TEST_F(SimpleDoc, CreatePathAtFailsIfElemFoundIsNonObjectNonArray) { ASSERT_TRUE(newElem.ok()); auto result = createPathAt(field(), 0, elemFound, newElem); ASSERT_NOT_OK(result); - ASSERT_EQ(result.code(), ErrorCodes::PathNotViable); - ASSERT_EQ(result.reason(), "Cannot create field 'b' in element {a: 1}"); + ASSERT_EQ(result.getStatus().code(), ErrorCodes::PathNotViable); + ASSERT_EQ(result.getStatus().reason(), "Cannot create field 'b' in element {a: 1}"); } class NestedDoc : public mongo::unittest::Test { @@ -307,7 +327,9 @@ TEST_F(NestedDoc, NewFieldNested) { ASSERT_TRUE(newElem.ok()); ASSERT_EQUALS(countChildren(elemFound), 1u); // 'c' is a child of 'b' - ASSERT_OK(createPathAt(field(), idxFound + 1, elemFound, newElem)); + auto firstNewElem = createPathAt(field(), idxFound + 1, elemFound, newElem); + ASSERT_OK(firstNewElem); + ASSERT_EQUALS(firstNewElem.getValue().compareWithElement(root()["a"]["b"]["d"], nullptr), 0); ASSERT_EQUALS(fromjson("{a: {b: {c: 1, d: 1}}}"), doc()); } @@ -410,7 +432,10 @@ TEST_F(ArrayDoc, ExtendingExistingObject) { ASSERT_TRUE(newElem.ok()); ASSERT_EQUALS(countChildren(elemFound), 1u); // '{c:1}' is a child of b.0 - ASSERT_OK(createPathAt(field(), idxFound + 1, elemFound, newElem)); + auto firstNewElem = createPathAt(field(), idxFound + 1, elemFound, newElem); + ASSERT_OK(firstNewElem); + ASSERT_EQUALS( + firstNewElem.getValue().compareWithElement(root()["b"].findNthChild(0)["d"], nullptr), 0); ASSERT_EQUALS(fromjson("{a: [], b: [{c:1, d:1}]}"), doc()); } @@ -429,7 +454,10 @@ TEST_F(ArrayDoc, NewObjectInsideArray) { ASSERT_TRUE(newElem.ok()); ASSERT_EQUALS(countChildren(elemFound), 1u); // '{c:1}' is a child of 'b' - ASSERT_OK(createPathAt(field(), idxFound + 1, elemFound, newElem)); + auto firstNewElem = createPathAt(field(), idxFound + 1, elemFound, newElem); + ASSERT_OK(firstNewElem); + ASSERT_EQUALS(firstNewElem.getValue().compareWithElement(root()["b"].findNthChild(1), nullptr), + 0); ASSERT_EQUALS(fromjson("{a: [], b: [{c:1},{c:2}]}"), doc()); } @@ -448,7 +476,10 @@ TEST_F(ArrayDoc, NewNestedObjectInsideArray) { ASSERT_TRUE(newElem.ok()); ASSERT_EQUALS(countChildren(elemFound), 1u); // '{c:1}' is a child of 'b' - ASSERT_OK(createPathAt(field(), idxFound + 1, elemFound, newElem)); + auto firstNewElem = createPathAt(field(), idxFound + 1, elemFound, newElem); + ASSERT_OK(firstNewElem); + ASSERT_EQUALS(firstNewElem.getValue().compareWithElement(root()["b"].findNthChild(1), nullptr), + 0); ASSERT_EQUALS(fromjson("{a: [], b: [{c:1},{c:{d:2}}]}"), doc()); } @@ -467,7 +498,10 @@ TEST_F(ArrayDoc, ArrayPaddingNecessary) { ASSERT_TRUE(newElem.ok()); ASSERT_EQUALS(countChildren(elemFound), 1u); // '{c:1}' is a child of 'b' - ASSERT_OK(createPathAt(field(), idxFound + 1, elemFound, newElem)); + auto firstNewElem = createPathAt(field(), idxFound + 1, elemFound, newElem); + ASSERT_OK(firstNewElem); + ASSERT_EQUALS(firstNewElem.getValue().compareWithElement(root()["b"].findNthChild(5), nullptr), + 0); ASSERT_EQUALS(fromjson("{a: [], b: [{c:1},null,null,null,null,1]}"), doc()); } @@ -486,7 +520,7 @@ TEST_F(ArrayDoc, ExcessivePaddingRequested) { ASSERT_TRUE(newElem.ok()); ASSERT_EQUALS(countChildren(elemFound), 1u); // '{c:1}' is a child of 'b' - Status status = createPathAt(field(), idxFound + 1, elemFound, newElem); + Status status = createPathAt(field(), idxFound + 1, elemFound, newElem).getStatus(); ASSERT_EQUALS(status.code(), ErrorCodes::CannotBackfillArray); } @@ -512,7 +546,12 @@ TEST_F(ArrayDoc, ExcessivePaddingNotRequestedIfArrayAlreadyPadded) { Element newElem = doc().makeElementInt("", 99); ASSERT_TRUE(newElem.ok()); - ASSERT_OK(createPathAt(field(), idxFound + 1, elemFound, newElem)); + auto firstNewElem = createPathAt(field(), idxFound + 1, elemFound, newElem); + ASSERT_OK(firstNewElem); + ASSERT_EQUALS( + firstNewElem.getValue().compareWithElement( + root()["a"].findNthChild(mongo::pathsupport::kMaxPaddingAllowed + 5), nullptr), + 0); // Array should now have maxPadding + 6 elements, since the highest array index is maxPadding + // 5. maxPadding of these elements are nulls adding as padding, 5 were appended at the @@ -540,8 +579,8 @@ TEST_F(ArrayDoc, CreatePathAtFailsIfElemFoundIsArrayAndIdxFoundFieldIsNonNumeric ASSERT_TRUE(newElem.ok()); auto result = createPathAt(field(), 0, elemFound, newElem); ASSERT_NOT_OK(result); - ASSERT_EQ(result.code(), ErrorCodes::PathNotViable); - ASSERT_EQ(result.reason(), "Cannot create field 'b' in element {a: []}"); + ASSERT_EQ(result.getStatus().code(), ErrorCodes::PathNotViable); + ASSERT_EQ(result.getStatus().reason(), "Cannot create field 'b' in element {a: []}"); } // diff --git a/src/mongo/db/update/pop_node.cpp b/src/mongo/db/update/pop_node.cpp index c0a81132118..38a36157382 100644 --- a/src/mongo/db/update/pop_node.cpp +++ b/src/mongo/db/update/pop_node.cpp @@ -44,6 +44,8 @@ void PopNode::apply(mutablebson::Element element, FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, @@ -113,6 +115,21 @@ void PopNode::apply(mutablebson::Element element, auto elementToRemove = _popFromFront ? element.leftChild() : element.rightChild(); invariantOK(elementToRemove.remove()); + // No need to validate for storage, since we cannot have increased the BSON depth or interfered + // with a DBRef. + + // Ensure we are not changing any immutable paths. + for (auto immutablePath = immutablePaths.begin(); immutablePath != immutablePaths.end(); + ++immutablePath) { + uassert(ErrorCodes::ImmutableField, + str::stream() << "Performing a $pop on the path '" << pathTaken->dottedField() + << "' would modify the immutable field '" + << (*immutablePath)->dottedField() + << "'", + pathTaken->commonPrefixSize(**immutablePath) < + std::min(pathTaken->numParts(), (*immutablePath)->numParts())); + } + if (logBuilder) { uassertStatusOK(logBuilder->addToSetsWithNewFieldName(pathTaken->dottedField(), element)); } diff --git a/src/mongo/db/update/pop_node.h b/src/mongo/db/update/pop_node.h index 99f6f6fbb32..7d7a2d13f14 100644 --- a/src/mongo/db/update/pop_node.h +++ b/src/mongo/db/update/pop_node.h @@ -42,6 +42,8 @@ public: FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, diff --git a/src/mongo/db/update/pop_node_test.cpp b/src/mongo/db/update/pop_node_test.cpp index 733f4082112..9866aeff435 100644 --- a/src/mongo/db/update/pop_node_test.cpp +++ b/src/mongo/db/update/pop_node_test.cpp @@ -99,6 +99,8 @@ TEST(PopNodeTest, NoopWhenFirstPathComponentDoesNotExist) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); mmb::Document logDoc; @@ -110,6 +112,8 @@ TEST(PopNodeTest, NoopWhenFirstPathComponentDoesNotExist) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -131,6 +135,8 @@ TEST(PopNodeTest, NoopWhenPathPartiallyExists) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b.c"); mmb::Document logDoc; @@ -142,6 +148,8 @@ TEST(PopNodeTest, NoopWhenPathPartiallyExists) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -163,6 +171,8 @@ TEST(PopNodeTest, NoopWhenNumericalPathComponentExceedsArrayLength) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.0"); mmb::Document logDoc; @@ -174,6 +184,8 @@ TEST(PopNodeTest, NoopWhenNumericalPathComponentExceedsArrayLength) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -195,6 +207,8 @@ TEST(PopNodeTest, ThrowsWhenPathIsBlockedByAScalar) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); mmb::Document logDoc; @@ -207,6 +221,8 @@ TEST(PopNodeTest, ThrowsWhenPathIsBlockedByAScalar) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -227,6 +243,8 @@ DEATH_TEST(PopNodeTest, NonOkElementWhenPathExistsIsFatal, "Invariant failure el FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); mmb::Document logDoc; @@ -238,6 +256,8 @@ DEATH_TEST(PopNodeTest, NonOkElementWhenPathExistsIsFatal, "Invariant failure el &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -255,6 +275,8 @@ TEST(PopNodeTest, ThrowsWhenPathExistsButDoesNotContainAnArray) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); mmb::Document logDoc; @@ -266,6 +288,8 @@ TEST(PopNodeTest, ThrowsWhenPathExistsButDoesNotContainAnArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -286,6 +310,8 @@ TEST(PopNodeTest, NoopWhenPathContainsAnEmptyArray) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); mmb::Document logDoc; @@ -297,6 +323,8 @@ TEST(PopNodeTest, NoopWhenPathContainsAnEmptyArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -319,6 +347,8 @@ TEST(PopNodeTest, PopsSingleElementFromTheBack) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); mmb::Document logDoc; @@ -330,6 +360,8 @@ TEST(PopNodeTest, PopsSingleElementFromTheBack) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -352,6 +384,8 @@ TEST(PopNodeTest, PopsSingleElementFromTheFront) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); mmb::Document logDoc; @@ -363,6 +397,8 @@ TEST(PopNodeTest, PopsSingleElementFromTheFront) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -385,6 +421,8 @@ TEST(PopNodeTest, PopsFromTheBackOfMultiElementArray) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b.c"); mmb::Document logDoc; @@ -396,6 +434,8 @@ TEST(PopNodeTest, PopsFromTheBackOfMultiElementArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -418,6 +458,8 @@ TEST(PopNodeTest, PopsFromTheFrontOfMultiElementArray) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); mmb::Document logDoc; @@ -429,6 +471,8 @@ TEST(PopNodeTest, PopsFromTheFrontOfMultiElementArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -451,6 +495,8 @@ TEST(PopNodeTest, PopsFromTheFrontOfMultiElementArrayWithoutAffectingIndexes) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("unrelated.path"); mmb::Document logDoc; @@ -462,6 +508,8 @@ TEST(PopNodeTest, PopsFromTheFrontOfMultiElementArrayWithoutAffectingIndexes) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -484,6 +532,8 @@ TEST(PopNodeTest, SucceedsWithNullUpdateIndexData) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; mmb::Document logDoc; LogBuilder logBuilder(logDoc.root()); bool indexesAffected; @@ -493,6 +543,8 @@ TEST(PopNodeTest, SucceedsWithNullUpdateIndexData) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, nullptr, &logBuilder, &indexesAffected, @@ -515,6 +567,8 @@ TEST(PopNodeTest, SucceedsWithNullLogBuilder) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b.c"); bool indexesAffected; @@ -524,6 +578,8 @@ TEST(PopNodeTest, SucceedsWithNullLogBuilder) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, nullptr, &indexesAffected, @@ -533,5 +589,162 @@ TEST(PopNodeTest, SucceedsWithNullLogBuilder) { ASSERT_EQUALS(fromjson("{a: {b: [1, 2]}}"), doc); } +TEST(PopNodeTest, ThrowsWhenPathIsImmutable) { + auto update = fromjson("{$pop: {'a.b': 1}}"); + const CollatorInterface* collator = nullptr; + PopNode popNode; + ASSERT_OK(popNode.init(update["$pop"]["a.b"], collator)); + + mmb::Document doc(fromjson("{a: {b: [0]}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a.b"); + mmb::Document logDoc; + LogBuilder logBuilder(logDoc.root()); + bool indexesAffected; + bool noop; + ASSERT_THROWS_CODE_AND_WHAT( + popNode.apply(doc.root()["a"]["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Performing a $pop on the path 'a.b' would modify the immutable field 'a.b'"); +} + +TEST(PopNodeTest, ThrowsWhenPathIsPrefixOfImmutable) { + + // This is only possible for an upsert, since it is not legal to have an array in an immutable + // path. If this update did not fail, we would fail later for storing an immutable path with an + // array in it. + + auto update = fromjson("{$pop: {'a': 1}}"); + const CollatorInterface* collator = nullptr; + PopNode popNode; + ASSERT_OK(popNode.init(update["$pop"]["a"], collator)); + + mmb::Document doc(fromjson("{a: [0]}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.0"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + mmb::Document logDoc; + LogBuilder logBuilder(logDoc.root()); + bool indexesAffected; + bool noop; + ASSERT_THROWS_CODE_AND_WHAT( + popNode.apply(doc.root()["a"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Performing a $pop on the path 'a' would modify the immutable field 'a.0'"); +} + +TEST(PopNodeTest, ThrowsWhenPathIsSuffixOfImmutable) { + auto update = fromjson("{$pop: {'a.b': 1}}"); + const CollatorInterface* collator = nullptr; + PopNode popNode; + ASSERT_OK(popNode.init(update["$pop"]["a.b"], collator)); + + mmb::Document doc(fromjson("{a: {b: [0]}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a.b"); + mmb::Document logDoc; + LogBuilder logBuilder(logDoc.root()); + bool indexesAffected; + bool noop; + ASSERT_THROWS_CODE_AND_WHAT( + popNode.apply(doc.root()["a"]["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Performing a $pop on the path 'a.b' would modify the immutable field 'a'"); +} + +TEST(PopNodeTest, NoopOnImmutablePathSucceeds) { + auto update = fromjson("{$pop: {'a.b': 1}}"); + const CollatorInterface* collator = nullptr; + PopNode popNode; + ASSERT_OK(popNode.init(update["$pop"]["a.b"], collator)); + + mmb::Document doc(fromjson("{a: {b: []}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a.b"); + mmb::Document logDoc; + LogBuilder logBuilder(logDoc.root()); + bool indexesAffected; + bool noop; + popNode.apply(doc.root()["a"]["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_TRUE(noop); + ASSERT_FALSE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: {b: []}}"), doc); + ASSERT_EQUALS(fromjson("{}"), logDoc); +} + } // namespace } // namespace mongo diff --git a/src/mongo/db/update/rename_node.cpp b/src/mongo/db/update/rename_node.cpp index cd0248706ba..b12810e2636 100644 --- a/src/mongo/db/update/rename_node.cpp +++ b/src/mongo/db/update/rename_node.cpp @@ -34,6 +34,7 @@ #include "mongo/db/update/field_checker.h" #include "mongo/db/update/path_creating_node.h" #include "mongo/db/update/path_support.h" +#include "mongo/db/update/storage_validation.h" namespace mongo { @@ -142,6 +143,8 @@ void RenameNode::apply(mutablebson::Element element, FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, @@ -222,11 +225,16 @@ void RenameNode::apply(mutablebson::Element element, pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &setAffectedIndexes, &setWasNoop); + auto leftSibling = fromElement.leftSibling(); + auto rightSibling = fromElement.rightSibling(); + invariant(fromElement.parent().ok()); invariantOK(fromElement.remove()); @@ -238,6 +246,34 @@ void RenameNode::apply(mutablebson::Element element, // *indexedAffected remains false } + if (validateForStorage) { + + // Validate the left and right sibling, in case this element was part of a DBRef. + if (leftSibling.ok()) { + const bool doRecursiveCheck = false; + const uint32_t recursionLevel = 0; + storage_validation::storageValid(leftSibling, doRecursiveCheck, recursionLevel); + } + + if (rightSibling.ok()) { + const bool doRecursiveCheck = false; + const uint32_t recursionLevel = 0; + storage_validation::storageValid(rightSibling, doRecursiveCheck, recursionLevel); + } + } + + // Ensure we are not changing any immutable paths. + for (auto immutablePath = immutablePaths.begin(); immutablePath != immutablePaths.end(); + ++immutablePath) { + uassert(ErrorCodes::ImmutableField, + str::stream() << "Unsetting the path '" << fromFieldRef.dottedField() + << "' using $rename would modify the immutable field '" + << (*immutablePath)->dottedField() + << "'", + fromFieldRef.commonPrefixSize(**immutablePath) < + std::min(fromFieldRef.numParts(), (*immutablePath)->numParts())); + } + // Log the $unset. The $set was already logged by SetElementNode::apply(). if (logBuilder) { uassertStatusOK(logBuilder->addToUnsets(fromFieldRef.dottedField())); diff --git a/src/mongo/db/update/rename_node.h b/src/mongo/db/update/rename_node.h index 79ef4520529..096740e1907 100644 --- a/src/mongo/db/update/rename_node.h +++ b/src/mongo/db/update/rename_node.h @@ -56,6 +56,8 @@ public: FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, diff --git a/src/mongo/db/update/rename_node_test.cpp b/src/mongo/db/update/rename_node_test.cpp index 431f2ecaa47..95a6d337d4a 100644 --- a/src/mongo/db/update/rename_node_test.cpp +++ b/src/mongo/db/update/rename_node_test.cpp @@ -118,6 +118,8 @@ TEST(RenameNodeTest, SimpleNumberAtRoot) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -129,6 +131,8 @@ TEST(RenameNodeTest, SimpleNumberAtRoot) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -150,6 +154,8 @@ TEST(RenameNodeTest, ToExistsAtSameLevel) { FieldRef pathTaken("b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -161,6 +167,8 @@ TEST(RenameNodeTest, ToExistsAtSameLevel) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -182,6 +190,8 @@ TEST(RenameNodeTest, ToAndFromHaveSameValue) { FieldRef pathTaken("b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -193,6 +203,8 @@ TEST(RenameNodeTest, ToAndFromHaveSameValue) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -214,6 +226,8 @@ TEST(RenameNodeTest, FromDottedElement) { FieldRef pathTaken("b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -225,6 +239,8 @@ TEST(RenameNodeTest, FromDottedElement) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -246,6 +262,8 @@ TEST(RenameNodeTest, RenameToExistingNestedFieldDoesNotReorderFields) { FieldRef pathTaken("a.b.c"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -257,6 +275,8 @@ TEST(RenameNodeTest, RenameToExistingNestedFieldDoesNotReorderFields) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -278,6 +298,8 @@ TEST(RenameNodeTest, MissingCompleteTo) { FieldRef pathTaken("c"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -289,6 +311,8 @@ TEST(RenameNodeTest, MissingCompleteTo) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -310,6 +334,8 @@ TEST(RenameNodeTest, ToIsCompletelyMissing) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -321,6 +347,8 @@ TEST(RenameNodeTest, ToIsCompletelyMissing) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -342,6 +370,8 @@ TEST(RenameNodeTest, ToMissingDottedField) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -353,6 +383,8 @@ TEST(RenameNodeTest, ToMissingDottedField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -374,6 +406,8 @@ TEST(RenameNodeTest, MoveIntoArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -385,6 +419,8 @@ TEST(RenameNodeTest, MoveIntoArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -406,6 +442,8 @@ TEST(RenameNodeTest, MoveIntoArrayNoId) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -417,6 +455,8 @@ TEST(RenameNodeTest, MoveIntoArrayNoId) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -438,6 +478,8 @@ TEST(RenameNodeTest, MoveToArrayElement) { FieldRef pathTaken("a.1"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -449,6 +491,8 @@ TEST(RenameNodeTest, MoveToArrayElement) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -470,6 +514,8 @@ TEST(RenameNodeTest, MoveOutOfArray) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -481,6 +527,8 @@ TEST(RenameNodeTest, MoveOutOfArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -502,6 +550,8 @@ TEST(RenameNodeTest, MoveNonexistentEmbeddedFieldOut) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -514,6 +564,8 @@ TEST(RenameNodeTest, MoveNonexistentEmbeddedFieldOut) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -534,6 +586,8 @@ TEST(RenameNodeTest, MoveEmbeddedFieldOutWithElementNumber) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -545,6 +599,8 @@ TEST(RenameNodeTest, MoveEmbeddedFieldOutWithElementNumber) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -566,6 +622,8 @@ TEST(RenameNodeTest, ReplaceArrayField) { FieldRef pathTaken("b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -577,6 +635,8 @@ TEST(RenameNodeTest, ReplaceArrayField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -598,6 +658,8 @@ TEST(RenameNodeTest, ReplaceWithArrayField) { FieldRef pathTaken("b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -609,6 +671,8 @@ TEST(RenameNodeTest, ReplaceWithArrayField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -630,6 +694,8 @@ TEST(RenameNodeTest, CanRenameFromInvalidFieldName) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -641,6 +707,8 @@ TEST(RenameNodeTest, CanRenameFromInvalidFieldName) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -662,6 +730,8 @@ TEST(RenameNodeTest, RenameWithoutLogBuilderOrIndexData) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -671,6 +741,8 @@ TEST(RenameNodeTest, RenameWithoutLogBuilderOrIndexData) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -690,6 +762,8 @@ TEST(RenameNodeTest, RenameFromNonExistentPathIsNoOp) { FieldRef pathTaken("b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -701,6 +775,8 @@ TEST(RenameNodeTest, RenameFromNonExistentPathIsNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -711,5 +787,296 @@ TEST(RenameNodeTest, RenameFromNonExistentPathIsNoOp) { ASSERT_EQUALS(fromjson("{}"), logDoc); } +TEST(RenameNodeTest, ApplyCannotRemoveRequiredPartOfDBRef) { + auto update = fromjson("{$rename: {'a.$id': 'b'}}"); + const CollatorInterface* collator = nullptr; + RenameNode node; + ASSERT_OK(node.init(update["$rename"]["a.$id"], collator)); + + Document doc(fromjson("{a: {$ref: 'c', $id: 0}}")); + FieldRef pathToCreate("b"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT(node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::InvalidDBRef, + "The DBRef $ref field must be followed by a $id field"); +} + +TEST(RenameNodeTest, ApplyCanRemoveRequiredPartOfDBRefIfValidateForStorageIsFalse) { + auto update = fromjson("{$rename: {'a.$id': 'b'}}"); + const CollatorInterface* collator = nullptr; + RenameNode node; + ASSERT_OK(node.init(update["$rename"]["a.$id"], collator)); + + Document doc(fromjson("{a: {$ref: 'c', $id: 0}}")); + FieldRef pathToCreate("b"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = false; + FieldRefSet immutablePaths; + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_FALSE(noop); + ASSERT_TRUE(indexesAffected); + auto updated = BSON("a" << BSON("$ref" + << "c") + << "b" + << 0); + ASSERT_EQUALS(updated, doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(fromjson("{$set: {'b': 0}, $unset: {'a.$id': true}}"), logDoc); +} + +TEST(RenameNodeTest, ApplyCannotRemoveImmutablePath) { + auto update = fromjson("{$rename: {'a.b': 'c'}}"); + const CollatorInterface* collator = nullptr; + RenameNode node; + ASSERT_OK(node.init(update["$rename"]["a.b"], collator)); + + Document doc(fromjson("{a: {b: 1}}")); + FieldRef pathToCreate("c"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Unsetting the path 'a.b' using $rename would modify the immutable field 'a.b'"); +} + +TEST(RenameNodeTest, ApplyCannotRemovePrefixOfImmutablePath) { + auto update = fromjson("{$rename: {a: 'c'}}"); + const CollatorInterface* collator = nullptr; + RenameNode node; + ASSERT_OK(node.init(update["$rename"]["a"], collator)); + + Document doc(fromjson("{a: {b: 1}}")); + FieldRef pathToCreate("c"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Unsetting the path 'a' using $rename would modify the immutable field 'a.b'"); +} + +TEST(RenameNodeTest, ApplyCannotRemoveSuffixOfImmutablePath) { + auto update = fromjson("{$rename: {'a.b.c': 'd'}}"); + const CollatorInterface* collator = nullptr; + RenameNode node; + ASSERT_OK(node.init(update["$rename"]["a.b.c"], collator)); + + Document doc(fromjson("{a: {b: {c: 1}}}")); + FieldRef pathToCreate("d"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Unsetting the path 'a.b.c' using $rename would modify the immutable field 'a.b'"); +} + +TEST(RenameNodeTest, ApplyCanRemoveImmutablePathIfNoop) { + auto update = fromjson("{$rename: {'a.b.c': 'd'}}"); + const CollatorInterface* collator = nullptr; + RenameNode node; + ASSERT_OK(node.init(update["$rename"]["a.b.c"], collator)); + + Document doc(fromjson("{a: {b: {}}}")); + FieldRef pathToCreate("d"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = false; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_TRUE(noop); + ASSERT_FALSE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: {b: {}}}"), doc); + ASSERT_TRUE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(fromjson("{}"), logDoc); +} + +TEST(RenameNodeTest, ApplyCannotCreateDollarPrefixedField) { + auto update = fromjson("{$rename: {a: '$bad'}}"); + const CollatorInterface* collator = nullptr; + RenameNode node; + ASSERT_OK(node.init(update["$rename"]["a"], collator)); + + Document doc(fromjson("{a: 0}")); + FieldRef pathToCreate("$bad"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::DollarPrefixedFieldName, + "The dollar ($) prefixed field '$bad' in '$bad' is not valid for storage."); +} + +TEST(RenameNodeTest, ApplyCannotOverwriteImmutablePath) { + auto update = fromjson("{$rename: {a: 'b'}}"); + const CollatorInterface* collator = nullptr; + RenameNode node; + ASSERT_OK(node.init(update["$rename"]["a"], collator)); + + Document doc(fromjson("{a: 0, b: 1}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root()["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Updating the path 'b' to b: 0 would modify the immutable field 'b'"); +} + } // namespace } // namespace mongo diff --git a/src/mongo/db/update/set_node_test.cpp b/src/mongo/db/update/set_node_test.cpp index 5b0b8b29398..0df17390b0b 100644 --- a/src/mongo/db/update/set_node_test.cpp +++ b/src/mongo/db/update/set_node_test.cpp @@ -68,6 +68,8 @@ TEST(SetNodeTest, ApplyNoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -79,6 +81,8 @@ TEST(SetNodeTest, ApplyNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -101,6 +105,8 @@ TEST(SetNodeTest, ApplyEmptyPathToCreate) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -112,6 +118,8 @@ TEST(SetNodeTest, ApplyEmptyPathToCreate) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -134,6 +142,8 @@ TEST(SetNodeTest, ApplyCreatePath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -145,6 +155,8 @@ TEST(SetNodeTest, ApplyCreatePath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -167,6 +179,8 @@ TEST(SetNodeTest, ApplyCreatePathFromRoot) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -178,6 +192,8 @@ TEST(SetNodeTest, ApplyCreatePathFromRoot) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -200,6 +216,8 @@ TEST(SetNodeTest, ApplyPositional) { FieldRef pathTaken("a.1"); StringData matchedField("1"); auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -211,6 +229,8 @@ TEST(SetNodeTest, ApplyPositional) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -233,6 +253,8 @@ TEST(SetNodeTest, ApplyNonViablePathToCreate) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -244,6 +266,8 @@ TEST(SetNodeTest, ApplyNonViablePathToCreate) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -264,6 +288,8 @@ TEST(SetNodeTest, ApplyNonViablePathToCreateFromReplicationIsNoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -275,6 +301,8 @@ TEST(SetNodeTest, ApplyNonViablePathToCreateFromReplicationIsNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -297,6 +325,8 @@ TEST(SetNodeTest, ApplyNoIndexDataNoLogBuilder) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -306,6 +336,8 @@ TEST(SetNodeTest, ApplyNoIndexDataNoLogBuilder) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -327,6 +359,8 @@ TEST(SetNodeTest, ApplyDoesNotAffectIndexes) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("b"); LogBuilder* logBuilder = nullptr; @@ -337,6 +371,8 @@ TEST(SetNodeTest, ApplyDoesNotAffectIndexes) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -358,6 +394,8 @@ TEST(SetNodeTest, TypeChangeIsNotANoop) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -368,6 +406,8 @@ TEST(SetNodeTest, TypeChangeIsNotANoop) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -393,6 +433,8 @@ TEST(SetNodeTest, IdentityOpOnDeserializedIsNotANoOp) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -403,6 +445,8 @@ TEST(SetNodeTest, IdentityOpOnDeserializedIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -424,6 +468,8 @@ TEST(SetNodeTest, ApplyEmptyDocument) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -434,6 +480,8 @@ TEST(SetNodeTest, ApplyEmptyDocument) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -455,6 +503,8 @@ TEST(SetNodeTest, ApplyInPlace) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -465,6 +515,8 @@ TEST(SetNodeTest, ApplyInPlace) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -486,6 +538,8 @@ TEST(SetNodeTest, ApplyOverridePath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -496,6 +550,8 @@ TEST(SetNodeTest, ApplyOverridePath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -517,6 +573,8 @@ TEST(SetNodeTest, ApplyChangeType) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -527,6 +585,8 @@ TEST(SetNodeTest, ApplyChangeType) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -548,6 +608,8 @@ TEST(SetNodeTest, ApplyNewPath) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); LogBuilder* logBuilder = nullptr; @@ -558,6 +620,8 @@ TEST(SetNodeTest, ApplyNewPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -579,6 +643,8 @@ TEST(SetNodeTest, ApplyLog) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -589,6 +655,8 @@ TEST(SetNodeTest, ApplyLog) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -610,6 +678,8 @@ TEST(SetNodeTest, ApplyNoOpDottedPath) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -620,6 +690,8 @@ TEST(SetNodeTest, ApplyNoOpDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -641,6 +713,8 @@ TEST(SetNodeTest, TypeChangeOnDottedPathIsNotANoOp) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -651,6 +725,8 @@ TEST(SetNodeTest, TypeChangeOnDottedPathIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -672,6 +748,8 @@ TEST(SetNodeTest, ApplyPathNotViable) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -681,6 +759,8 @@ TEST(SetNodeTest, ApplyPathNotViable) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -701,6 +781,8 @@ TEST(SetNodeTest, ApplyPathNotViableArrray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -710,6 +792,8 @@ TEST(SetNodeTest, ApplyPathNotViableArrray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -730,6 +814,8 @@ TEST(SetNodeTest, ApplyInPlaceDottedPath) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -740,6 +826,8 @@ TEST(SetNodeTest, ApplyInPlaceDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -761,6 +849,8 @@ TEST(SetNodeTest, ApplyChangeTypeDottedPath) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -771,6 +861,8 @@ TEST(SetNodeTest, ApplyChangeTypeDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -792,6 +884,8 @@ TEST(SetNodeTest, ApplyChangePath) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -802,6 +896,8 @@ TEST(SetNodeTest, ApplyChangePath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -823,6 +919,8 @@ TEST(SetNodeTest, ApplyExtendPath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -833,6 +931,8 @@ TEST(SetNodeTest, ApplyExtendPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -854,6 +954,8 @@ TEST(SetNodeTest, ApplyNewDottedPath) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -864,6 +966,8 @@ TEST(SetNodeTest, ApplyNewDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -885,6 +989,8 @@ TEST(SetNodeTest, ApplyEmptyDoc) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -895,6 +1001,8 @@ TEST(SetNodeTest, ApplyEmptyDoc) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -916,6 +1024,8 @@ TEST(SetNodeTest, ApplyFieldWithDot) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b"); LogBuilder* logBuilder = nullptr; @@ -926,6 +1036,8 @@ TEST(SetNodeTest, ApplyFieldWithDot) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -947,6 +1059,8 @@ TEST(SetNodeTest, ApplyNoOpArrayIndex) { FieldRef pathTaken("a.2.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -957,6 +1071,8 @@ TEST(SetNodeTest, ApplyNoOpArrayIndex) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -978,6 +1094,8 @@ TEST(SetNodeTest, TypeChangeInArrayIsNotANoOp) { FieldRef pathTaken("a.2.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -988,6 +1106,8 @@ TEST(SetNodeTest, TypeChangeInArrayIsNotANoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1009,6 +1129,8 @@ TEST(SetNodeTest, ApplyNonViablePath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -1018,6 +1140,8 @@ TEST(SetNodeTest, ApplyNonViablePath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -1038,6 +1162,8 @@ TEST(SetNodeTest, ApplyInPlaceArrayIndex) { FieldRef pathTaken("a.2.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -1048,6 +1174,8 @@ TEST(SetNodeTest, ApplyInPlaceArrayIndex) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1069,6 +1197,8 @@ TEST(SetNodeTest, ApplyNormalArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -1079,6 +1209,8 @@ TEST(SetNodeTest, ApplyNormalArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1100,6 +1232,8 @@ TEST(SetNodeTest, ApplyPaddingArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -1110,6 +1244,8 @@ TEST(SetNodeTest, ApplyPaddingArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1131,6 +1267,8 @@ TEST(SetNodeTest, ApplyNumericObject) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -1141,6 +1279,8 @@ TEST(SetNodeTest, ApplyNumericObject) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1162,6 +1302,8 @@ TEST(SetNodeTest, ApplyNumericField) { FieldRef pathTaken("a.2.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -1172,6 +1314,8 @@ TEST(SetNodeTest, ApplyNumericField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1193,6 +1337,8 @@ TEST(SetNodeTest, ApplyExtendNumericField) { FieldRef pathTaken("a.2"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -1203,6 +1349,8 @@ TEST(SetNodeTest, ApplyExtendNumericField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1224,6 +1372,8 @@ TEST(SetNodeTest, ApplyEmptyObject) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -1234,6 +1384,8 @@ TEST(SetNodeTest, ApplyEmptyObject) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1255,6 +1407,8 @@ TEST(SetNodeTest, ApplyEmptyArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.2.b"); LogBuilder* logBuilder = nullptr; @@ -1265,6 +1419,8 @@ TEST(SetNodeTest, ApplyEmptyArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1286,6 +1442,8 @@ TEST(SetNodeTest, ApplyLogDottedPath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1296,6 +1454,8 @@ TEST(SetNodeTest, ApplyLogDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1317,6 +1477,8 @@ TEST(SetNodeTest, LogEmptyArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1327,6 +1489,8 @@ TEST(SetNodeTest, LogEmptyArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1348,6 +1512,8 @@ TEST(SetNodeTest, LogEmptyObject) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -1358,6 +1524,8 @@ TEST(SetNodeTest, LogEmptyObject) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, @@ -1379,6 +1547,8 @@ TEST(SetNodeTest, ApplyNoOpComplex) { FieldRef pathTaken("a.1.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.1.b"); LogBuilder* logBuilder = nullptr; @@ -1389,6 +1559,8 @@ TEST(SetNodeTest, ApplyNoOpComplex) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1410,6 +1582,8 @@ TEST(SetNodeTest, ApplySameStructure) { FieldRef pathTaken("a.1.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.1.b"); LogBuilder* logBuilder = nullptr; @@ -1420,6 +1594,8 @@ TEST(SetNodeTest, ApplySameStructure) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1441,6 +1617,8 @@ TEST(SetNodeTest, NonViablePathWithoutRepl) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -1450,6 +1628,8 @@ TEST(SetNodeTest, NonViablePathWithoutRepl) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -1470,6 +1650,8 @@ TEST(SetNodeTest, SingleFieldFromReplication) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.1.b"); LogBuilder* logBuilder = nullptr; @@ -1480,6 +1662,8 @@ TEST(SetNodeTest, SingleFieldFromReplication) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1501,6 +1685,8 @@ TEST(SetNodeTest, SingleFieldNoIdFromReplication) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.1.b"); LogBuilder* logBuilder = nullptr; @@ -1511,6 +1697,8 @@ TEST(SetNodeTest, SingleFieldNoIdFromReplication) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1532,6 +1720,8 @@ TEST(SetNodeTest, NestedFieldFromReplication) { FieldRef pathTaken("a.a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.a.1.b"); LogBuilder* logBuilder = nullptr; @@ -1542,6 +1732,8 @@ TEST(SetNodeTest, NestedFieldFromReplication) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1563,6 +1755,8 @@ TEST(SetNodeTest, DoubleNestedFieldFromReplication) { FieldRef pathTaken("a.b.c"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.b.c.d"); LogBuilder* logBuilder = nullptr; @@ -1573,6 +1767,8 @@ TEST(SetNodeTest, DoubleNestedFieldFromReplication) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1594,6 +1790,8 @@ TEST(SetNodeTest, NestedFieldNoIdFromReplication) { FieldRef pathTaken("a.a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.a.1.b"); LogBuilder* logBuilder = nullptr; @@ -1604,6 +1802,8 @@ TEST(SetNodeTest, NestedFieldNoIdFromReplication) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1625,6 +1825,8 @@ TEST(SetNodeTest, ReplayArrayFieldNotAppendedIntermediateFromReplication) { FieldRef pathTaken("a.0"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.0.b"); LogBuilder* logBuilder = nullptr; @@ -1635,6 +1837,8 @@ TEST(SetNodeTest, ReplayArrayFieldNotAppendedIntermediateFromReplication) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1656,6 +1860,8 @@ TEST(SetNodeTest, Set6) { FieldRef pathTaken("r.a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("r.a"); Document logDoc; @@ -1667,6 +1873,8 @@ TEST(SetNodeTest, Set6) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1690,6 +1898,8 @@ TEST(SetNodeTest, Set6FromRepl) { FieldRef pathTaken("r.a"); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("r.a"); Document logDoc; @@ -1701,6 +1911,8 @@ TEST(SetNodeTest, Set6FromRepl) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1732,6 +1944,8 @@ TEST(SetNodeTest, ApplySetModToEphemeralDocument) { FieldRef pathTaken("x"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("x"); LogBuilder* logBuilder = nullptr; @@ -1742,6 +1956,8 @@ TEST(SetNodeTest, ApplySetModToEphemeralDocument) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, logBuilder, &indexesAffected, @@ -1752,4 +1968,674 @@ TEST(SetNodeTest, ApplySetModToEphemeralDocument) { ASSERT_FALSE(doc.isInPlaceModeEnabled()); } +TEST(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldInsideSetElement) { + auto update = fromjson("{$set: {a: {$bad: 1}}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a"], collator)); + + Document doc(fromjson("{a: 5}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root()["a"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::DollarPrefixedFieldName, + "The dollar ($) prefixed field '$bad' in 'a.$bad' is not valid for storage."); +} + +TEST(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldAtStartOfPath) { + auto update = fromjson("{$set: {'$bad.a': 1}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["$bad.a"], collator)); + + Document doc(fromjson("{}")); + FieldRef pathToCreate("$bad.a"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::DollarPrefixedFieldName, + "The dollar ($) prefixed field '$bad' in '$bad' is not valid for storage."); +} + +TEST(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldInMiddleOfPath) { + auto update = fromjson("{$set: {'a.$bad.b': 1}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a.$bad.b"], collator)); + + Document doc(fromjson("{}")); + FieldRef pathToCreate("a.$bad.b"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::DollarPrefixedFieldName, + "The dollar ($) prefixed field '$bad' in 'a.$bad' is not valid for storage."); +} + +TEST(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldAtEndOfPath) { + auto update = fromjson("{$set: {'a.$bad': 1}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a.$bad"], collator)); + + Document doc(fromjson("{}")); + FieldRef pathToCreate("a.$bad"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::DollarPrefixedFieldName, + "The dollar ($) prefixed field '$bad' in 'a.$bad' is not valid for storage."); +} + +TEST(SetNodeTest, ApplyCanCreateDollarPrefixedFieldNameWhenValidateForStorageIsFalse) { + auto update = fromjson("{$set: {$bad: 1}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["$bad"], collator)); + + Document doc(fromjson("{}")); + FieldRef pathToCreate("$bad"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = false; + FieldRefSet immutablePaths; + UpdateIndexData indexData; + indexData.addPath("$bad"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_FALSE(noop); + ASSERT_TRUE(indexesAffected); + ASSERT_EQUALS(fromjson("{$bad: 1}"), doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(countChildren(logDoc.root()), 1u); + ASSERT_EQUALS(fromjson("{$set: {$bad: 1}}"), logDoc); +} + +TEST(SetNodeTest, ApplyCannotOverwriteImmutablePath) { + auto update = fromjson("{$set: {'a.b': 1}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a.b"], collator)); + + Document doc(fromjson("{a: {b: 2}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root()["a"]["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Updating the path 'a.b' to b: 1 would modify the immutable field 'a.b'"); +} + +TEST(SetNodeTest, ApplyCanPerformNoopOnImmutablePath) { + auto update = fromjson("{$set: {'a.b': 2}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a.b"], collator)); + + Document doc(fromjson("{a: {b: 2}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root()["a"]["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_TRUE(noop); + ASSERT_FALSE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: {b: 2}}"), doc); + ASSERT_TRUE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(countChildren(logDoc.root()), 0u); + ASSERT_EQUALS(fromjson("{}"), logDoc); +} + +TEST(SetNodeTest, ApplyCannotOverwritePrefixToRemoveImmutablePath) { + auto update = fromjson("{$set: {a: 1}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a"], collator)); + + Document doc(fromjson("{a: {b: 2}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root()["a"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "After applying the update, the immutable field 'a.b' was found to have been removed."); +} + +TEST(SetNodeTest, ApplyCannotOverwritePrefixToModifyImmutablePath) { + auto update = fromjson("{$set: {a: {b: 1}}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a"], collator)); + + Document doc(fromjson("{a: {b: 2}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT(node.apply(doc.root()["a"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "After applying the update, the immutable field 'a.b' was found to " + "have been altered to b: 1"); +} + +TEST(SetNodeTest, ApplyCanPerformNoopOnPrefixOfImmutablePath) { + auto update = fromjson("{$set: {a: {b: 2}}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a"], collator)); + + Document doc(fromjson("{a: {b: 2}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root()["a"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_TRUE(noop); + ASSERT_FALSE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: {b: 2}}"), doc); + ASSERT_TRUE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(countChildren(logDoc.root()), 0u); + ASSERT_EQUALS(fromjson("{}"), logDoc); +} + +TEST(SetNodeTest, ApplyCanOverwritePrefixToCreateImmutablePath) { + auto update = fromjson("{$set: {a: {b: 2}}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a"], collator)); + + Document doc(fromjson("{a: 1}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root()["a"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_FALSE(noop); + ASSERT_TRUE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: {b: 2}}"), doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(countChildren(logDoc.root()), 1u); + ASSERT_EQUALS(fromjson("{$set: {a: {b: 2}}}"), logDoc); +} + +TEST(SetNodeTest, ApplyCanOverwritePrefixOfImmutablePathIfNoopOnImmutablePath) { + auto update = fromjson("{$set: {a: {b: 2, c: 3}}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a"], collator)); + + Document doc(fromjson("{a: {b: 2}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root()["a"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_FALSE(noop); + ASSERT_TRUE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: {b: 2, c: 3}}"), doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(countChildren(logDoc.root()), 1u); + ASSERT_EQUALS(fromjson("{$set: {a: {b: 2, c: 3}}}"), logDoc); +} + +TEST(SetNodeTest, ApplyCannotOverwriteSuffixOfImmutablePath) { + auto update = fromjson("{$set: {'a.b.c': 1}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a.b.c"], collator)); + + Document doc(fromjson("{a: {b: {c: 2}}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.b.c"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root()["a"]["b"]["c"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Updating the path 'a.b.c' to c: 1 would modify the immutable field 'a.b'"); +} + +TEST(SetNodeTest, ApplyCanPerformNoopOnSuffixOfImmutablePath) { + auto update = fromjson("{$set: {'a.b.c': 2}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a.b.c"], collator)); + + Document doc(fromjson("{a: {b: {c: 2}}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.b.c"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root()["a"]["b"]["c"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_TRUE(noop); + ASSERT_FALSE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: {b: {c: 2}}}"), doc); + ASSERT_TRUE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(countChildren(logDoc.root()), 0u); + ASSERT_EQUALS(fromjson("{}"), logDoc); +} + +TEST(SetNodeTest, ApplyCannotCreateFieldAtEndOfImmutablePath) { + auto update = fromjson("{$set: {'a.b.c': 1}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a.b.c"], collator)); + + Document doc(fromjson("{a: {b: {}}}")); + FieldRef pathToCreate("c"); + FieldRef pathTaken("a.b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root()["a"]["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Updating the path 'a.b' to b: { c: 1 } would modify the immutable field 'a.b'"); +} + +TEST(SetNodeTest, ApplyCannotCreateFieldBeyondEndOfImmutablePath) { + auto update = fromjson("{$set: {'a.b.c': 1}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a.b.c"], collator)); + + Document doc(fromjson("{a: {b: {}}}")); + FieldRef pathToCreate("c"); + FieldRef pathTaken("a.b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root()["a"]["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Updating the path 'a.b' to b: { c: 1 } would modify the immutable field 'a'"); +} + +TEST(SetNodeTest, ApplyCanCreateImmutablePath) { + auto update = fromjson("{$set: {'a.b': 2}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a.b"], collator)); + + Document doc(fromjson("{a: {}}")); + FieldRef pathToCreate("b"); + FieldRef pathTaken("a"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root()["a"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_FALSE(noop); + ASSERT_TRUE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: {b: 2}}"), doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(countChildren(logDoc.root()), 1u); + ASSERT_EQUALS(fromjson("{$set: {'a.b': 2}}"), logDoc); +} + +TEST(SetNodeTest, ApplyCanCreatePrefixOfImmutablePath) { + auto update = fromjson("{$set: {a: 2}}"); + const CollatorInterface* collator = nullptr; + SetNode node; + ASSERT_OK(node.init(update["$set"]["a"], collator)); + + Document doc(fromjson("{}")); + FieldRef pathToCreate("a"); + FieldRef pathTaken(""); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root(), + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_FALSE(noop); + ASSERT_TRUE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: 2}"), doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(countChildren(logDoc.root()), 1u); + ASSERT_EQUALS(fromjson("{$set: {a: 2}}"), logDoc); +} + } // namespace diff --git a/src/mongo/db/update/storage_validation.cpp b/src/mongo/db/update/storage_validation.cpp new file mode 100644 index 00000000000..8b5be0d95e5 --- /dev/null +++ b/src/mongo/db/update/storage_validation.cpp @@ -0,0 +1,188 @@ +/** + * Copyright (C) 2017 MongoDB Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the GNU Affero General Public License in all respects + * for all of the code used other than as permitted herein. If you modify + * file(s) with this exception, you may extend this exception to your + * version of the file(s), but you are not obligated to do so. If you do not + * wish to do so, delete this exception statement from your version. If you + * delete this exception statement from all source files in the program, + * then also delete it in the license file. + */ + +#include "mongo/db/update/storage_validation.h" + +#include "mongo/bson/bson_depth.h" +#include "mongo/bson/mutable/algorithm.h" +#include "mongo/bson/mutable/document.h" + +namespace mongo { + +namespace storage_validation { + +namespace { + +const StringData idFieldName = "_id"_sd; + +void storageValidChildren(mutablebson::ConstElement elem, + const bool deep, + std::uint32_t recursionLevel) { + if (!elem.hasChildren()) { + return; + } + + auto curr = elem.leftChild(); + while (curr.ok()) { + storageValid(curr, deep, recursionLevel + 1); + curr = curr.rightSibling(); + } +} + +/** + * Validates an element that has a field name which starts with a dollar sign ($). + * In the case of a DBRef field ($id, $ref, [$db]) these fields may be valid in + * the correct order/context only. + */ +void validateDollarPrefixElement(mutablebson::ConstElement elem) { + auto curr = elem; + auto currName = elem.getFieldName(); + + // Found a $db field. + if (currName == "$db") { + uassert(ErrorCodes::InvalidDBRef, + str::stream() << "The DBRef $db field must be a String, not a " + << typeName(curr.getType()), + curr.getType() == BSONType::String); + curr = curr.leftSibling(); + + uassert(ErrorCodes::InvalidDBRef, + "Found $db field without a $id before it, which is invalid.", + curr.ok() && (curr.getFieldName() == "$id")); + + currName = curr.getFieldName(); + } + + // Found a $id field. + if (currName == "$id") { + curr = curr.leftSibling(); + uassert(ErrorCodes::InvalidDBRef, + "Found $id field without a $ref before it, which is invalid.", + curr.ok() && (curr.getFieldName() == "$ref")); + + currName = curr.getFieldName(); + } + + // Found a $ref field. + if (currName == "$ref") { + uassert(ErrorCodes::InvalidDBRef, + str::stream() << "The DBRef $ref field must be a String, not a " + << typeName(curr.getType()), + curr.getType() == BSONType::String); + + uassert(ErrorCodes::InvalidDBRef, + "The DBRef $ref field must be followed by a $id field", + curr.rightSibling().ok() && curr.rightSibling().getFieldName() == "$id"); + } else { + + // Not an okay, $ prefixed field name. + uasserted(ErrorCodes::DollarPrefixedFieldName, + str::stream() << "The dollar ($) prefixed field '" << elem.getFieldName() + << "' in '" + << mutablebson::getFullName(elem) + << "' is not valid for storage."); + } +} +} // namespace + +void storageValid(const mutablebson::Document& doc) { + auto currElem = doc.root().leftChild(); + while (currElem.ok()) { + if (currElem.getFieldName() == idFieldName) { + switch (currElem.getType()) { + case BSONType::RegEx: + case BSONType::Array: + case BSONType::Undefined: + uasserted(ErrorCodes::InvalidIdField, + str::stream() << "The '_id' value cannot be of type " + << typeName(currElem.getType())); + default: + break; + } + } + + // Validate this child element. + const auto deep = true; + const uint32_t recursionLevel = 1; + storageValid(currElem, deep, recursionLevel); + + currElem = currElem.rightSibling(); + } +} + +void storageValid(mutablebson::ConstElement elem, const bool deep, std::uint32_t recursionLevel) { + uassert(ErrorCodes::BadValue, "Invalid elements cannot be stored.", elem.ok()); + + uassert(ErrorCodes::Overflow, + str::stream() << "Document exceeds maximum nesting depth of " + << BSONDepth::getMaxDepthForUserStorage(), + recursionLevel <= BSONDepth::getMaxDepthForUserStorage()); + + // Field names of elements inside arrays are not meaningful in mutable bson, + // so we do not want to validate them. + const mutablebson::ConstElement& parent = elem.parent(); + const bool childOfArray = parent.ok() ? (parent.getType() == BSONType::Array) : false; + + if (!childOfArray) { + auto fieldName = elem.getFieldName(); + + // Cannot start with "$", unless dbref. + if (fieldName[0] == '$') { + validateDollarPrefixElement(elem); + } + } + + if (deep) { + + // Check children if there are any. + storageValidChildren(elem, deep, recursionLevel); + } +} + +std::uint32_t storageValidParents(mutablebson::ConstElement elem, std::uint32_t recursionLevel) { + uassert(ErrorCodes::Overflow, + str::stream() << "Document exceeds maximum nesting depth of " + << BSONDepth::getMaxDepthForUserStorage(), + recursionLevel <= BSONDepth::getMaxDepthForUserStorage()); + + auto root = elem.getDocument().root(); + if (elem != root) { + auto parent = elem.parent(); + if (parent.ok() && parent != root) { + const bool doRecursiveCheck = false; + const uint32_t parentsRecursionLevel = 0; + storageValid(parent, doRecursiveCheck, parentsRecursionLevel); + return storageValidParents(parent, recursionLevel + 1); + } + return recursionLevel + 1; + } + return recursionLevel; +} + +} // namespace storage_validation +} // namespace mongo diff --git a/src/mongo/db/update/storage_validation.h b/src/mongo/db/update/storage_validation.h new file mode 100644 index 00000000000..d47197e8d17 --- /dev/null +++ b/src/mongo/db/update/storage_validation.h @@ -0,0 +1,61 @@ +/** + * Copyright (C) 2017 MongoDB Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the GNU Affero General Public License in all respects + * for all of the code used other than as permitted herein. If you modify + * file(s) with this exception, you may extend this exception to your + * version of the file(s), but you are not obligated to do so. If you do not + * wish to do so, delete this exception statement from your version. If you + * delete this exception statement from all source files in the program, + * then also delete it in the license file. + */ + +#pragma once + +#include "mongo/bson/mutable/element.h" + +namespace mongo { + +namespace storage_validation { + +/** + * Validates that the MutableBSON document 'doc' is acceptable for storage in a collection. The + * check is performed recursively on subdocuments. Uasserts if the validation fails or if the depth + * exceeds the maximum allowable depth. + */ +void storageValid(const mutablebson::Document& doc); + +/** + * Validates that the MutableBSON element 'elem' is acceptable for storage in a collection. If + * 'deep' is true, the check is performed recursively on subdocuments. Uasserts if the validation + * fails or if 'recursionLevel' exceeds the maximum allowable depth. + */ +void storageValid(mutablebson::ConstElement elem, const bool deep, std::uint32_t recursionLevel); + +/** + * Checks that all of the parents of the MutableBSON element 'elem' are valid for storage. Note that + * 'elem' must be in a valid state when using this function. Uasserts if the validation fails, or if + * 'recursionLevel' exceeds the maximum allowable depth. On success, an integer is returned that + * represents the number of steps from this element to the root through ancestor nodes. + */ +std::uint32_t storageValidParents(mutablebson::ConstElement elem, std::uint32_t recursionLevel); + +} // namespace storage_validation + +} // namespace mongo diff --git a/src/mongo/db/update/unset_node.cpp b/src/mongo/db/update/unset_node.cpp index 642c9d0937b..c7266ecb2d0 100644 --- a/src/mongo/db/update/unset_node.cpp +++ b/src/mongo/db/update/unset_node.cpp @@ -30,6 +30,8 @@ #include "mongo/db/update/unset_node.h" +#include "mongo/db/update/storage_validation.h" + namespace mongo { Status UnsetNode::init(BSONElement modExpr, const CollatorInterface* collator) { @@ -43,6 +45,8 @@ void UnsetNode::apply(mutablebson::Element element, FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, @@ -64,6 +68,9 @@ void UnsetNode::apply(mutablebson::Element element, } auto parent = element.parent(); + auto leftSibling = element.leftSibling(); + auto rightSibling = element.rightSibling(); + invariant(parent.ok()); if (!parent.isType(BSONType::Array)) { invariantOK(element.remove()); @@ -73,6 +80,34 @@ void UnsetNode::apply(mutablebson::Element element, invariantOK(element.setValueNull()); } + if (validateForStorage) { + + // Validate the left and right sibling, in case this element was part of a DBRef. + if (leftSibling.ok()) { + const bool doRecursiveCheck = false; + const uint32_t recursionLevel = 0; + storage_validation::storageValid(leftSibling, doRecursiveCheck, recursionLevel); + } + + if (rightSibling.ok()) { + const bool doRecursiveCheck = false; + const uint32_t recursionLevel = 0; + storage_validation::storageValid(rightSibling, doRecursiveCheck, recursionLevel); + } + } + + // Ensure we are not changing any immutable paths. + for (auto immutablePath = immutablePaths.begin(); immutablePath != immutablePaths.end(); + ++immutablePath) { + uassert(ErrorCodes::ImmutableField, + str::stream() << "Unsetting the path '" << pathTaken->dottedField() + << "' would modify the immutable field '" + << (*immutablePath)->dottedField() + << "'", + pathTaken->commonPrefixSize(**immutablePath) < + std::min(pathTaken->numParts(), (*immutablePath)->numParts())); + } + // Log the unset. if (logBuilder) { uassertStatusOK(logBuilder->addToUnsets(pathTaken->dottedField())); diff --git a/src/mongo/db/update/unset_node.h b/src/mongo/db/update/unset_node.h index 6e21fe8eced..973eb300bef 100644 --- a/src/mongo/db/update/unset_node.h +++ b/src/mongo/db/update/unset_node.h @@ -51,6 +51,8 @@ public: FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, diff --git a/src/mongo/db/update/unset_node_test.cpp b/src/mongo/db/update/unset_node_test.cpp index 0dcbf3f4776..8c64cf5201c 100644 --- a/src/mongo/db/update/unset_node_test.cpp +++ b/src/mongo/db/update/unset_node_test.cpp @@ -61,6 +61,8 @@ DEATH_TEST(UnsetNodeTest, ApplyToRootFails, "Invariant failure parent.ok()") { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -70,6 +72,8 @@ DEATH_TEST(UnsetNodeTest, ApplyToRootFails, "Invariant failure parent.ok()") { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -95,6 +99,8 @@ TEST(UnsetNodeTest, UnsetNoOp) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -106,6 +112,8 @@ TEST(UnsetNodeTest, UnsetNoOp) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -128,6 +136,8 @@ TEST(UnsetNodeTest, UnsetNoOpDottedPath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -139,6 +149,8 @@ TEST(UnsetNodeTest, UnsetNoOpDottedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -161,6 +173,8 @@ TEST(UnsetNodeTest, UnsetNoOpThroughArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -172,6 +186,8 @@ TEST(UnsetNodeTest, UnsetNoOpThroughArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -194,6 +210,8 @@ TEST(UnsetNodeTest, UnsetNoOpEmptyDoc) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -205,6 +223,8 @@ TEST(UnsetNodeTest, UnsetNoOpEmptyDoc) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -227,6 +247,8 @@ TEST(UnsetNodeTest, UnsetTopLevelPath) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -238,6 +260,8 @@ TEST(UnsetNodeTest, UnsetTopLevelPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -260,6 +284,8 @@ TEST(UnsetNodeTest, UnsetNestedPath) { FieldRef pathTaken("a.b.c"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -271,6 +297,8 @@ TEST(UnsetNodeTest, UnsetNestedPath) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -293,6 +321,8 @@ TEST(UnsetNodeTest, UnsetObject) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -304,6 +334,8 @@ TEST(UnsetNodeTest, UnsetObject) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -326,6 +358,8 @@ TEST(UnsetNodeTest, UnsetArrayElement) { FieldRef pathTaken("a.0"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -337,6 +371,8 @@ TEST(UnsetNodeTest, UnsetArrayElement) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -359,6 +395,8 @@ TEST(UnsetNodeTest, UnsetPositional) { FieldRef pathTaken("a.1"); StringData matchedField = "1"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -370,6 +408,8 @@ TEST(UnsetNodeTest, UnsetPositional) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -392,6 +432,8 @@ TEST(UnsetNodeTest, UnsetEntireArray) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -403,6 +445,8 @@ TEST(UnsetNodeTest, UnsetEntireArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -425,6 +469,8 @@ TEST(UnsetNodeTest, UnsetFromObjectInArray) { FieldRef pathTaken("a.0.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -436,6 +482,8 @@ TEST(UnsetNodeTest, UnsetFromObjectInArray) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -458,6 +506,8 @@ TEST(UnsetNodeTest, CanUnsetInvalidField) { FieldRef pathTaken("a.0.$b"); StringData matchedField = "0"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -469,6 +519,8 @@ TEST(UnsetNodeTest, CanUnsetInvalidField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -491,6 +543,8 @@ TEST(UnsetNodeTest, ApplyNoIndexDataNoLogBuilder) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -500,6 +554,8 @@ TEST(UnsetNodeTest, ApplyNoIndexDataNoLogBuilder) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, @@ -521,6 +577,8 @@ TEST(UnsetNodeTest, ApplyDoesNotAffectIndexes) { FieldRef pathTaken("a"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("b"); Document logDoc; @@ -532,6 +590,8 @@ TEST(UnsetNodeTest, ApplyDoesNotAffectIndexes) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -554,6 +614,8 @@ TEST(UnsetNodeTest, ApplyFieldWithDot) { FieldRef pathTaken("a.b"); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -565,6 +627,8 @@ TEST(UnsetNodeTest, ApplyFieldWithDot) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -576,4 +640,221 @@ TEST(UnsetNodeTest, ApplyFieldWithDot) { ASSERT_EQUALS(fromjson("{$unset: {'a.b': true}}"), logDoc); } +TEST(UnsetNodeTest, ApplyCannotRemoveRequiredPartOfDBRef) { + auto update = fromjson("{$unset: {'a.$id': true}}"); + const CollatorInterface* collator = nullptr; + UnsetNode node; + ASSERT_OK(node.init(update["$unset"]["a.$id"], collator)); + + Document doc(fromjson("{a: {$ref: 'c', $id: 0}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.$id"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT(node.apply(doc.root()["a"]["$id"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::InvalidDBRef, + "The DBRef $ref field must be followed by a $id field"); +} + +TEST(UnsetNodeTest, ApplyCanRemoveRequiredPartOfDBRefIfValidateForStorageIsFalse) { + auto update = fromjson("{$unset: {'a.$id': true}}"); + const CollatorInterface* collator = nullptr; + UnsetNode node; + ASSERT_OK(node.init(update["$unset"]["a.$id"], collator)); + + Document doc(fromjson("{a: {$ref: 'c', $id: 0}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.$id"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = false; + FieldRefSet immutablePaths; + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root()["a"]["$id"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_FALSE(noop); + ASSERT_TRUE(indexesAffected); + auto updated = BSON("a" << BSON("$ref" + << "c")); + ASSERT_EQUALS(updated, doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(fromjson("{$unset: {'a.$id': true}}"), logDoc); +} + +TEST(UnsetNodeTest, ApplyCannotRemoveImmutablePath) { + auto update = fromjson("{$unset: {'a.b': true}}"); + const CollatorInterface* collator = nullptr; + UnsetNode node; + ASSERT_OK(node.init(update["$unset"]["a.b"], collator)); + + Document doc(fromjson("{a: {b: 1}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT(node.apply(doc.root()["a"]["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Unsetting the path 'a.b' would modify the immutable field 'a.b'"); +} + +TEST(UnsetNodeTest, ApplyCannotRemovePrefixOfImmutablePath) { + auto update = fromjson("{$unset: {a: true}}"); + const CollatorInterface* collator = nullptr; + UnsetNode node; + ASSERT_OK(node.init(update["$unset"]["a"], collator)); + + Document doc(fromjson("{a: {b: 1}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT(node.apply(doc.root()["a"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Unsetting the path 'a' would modify the immutable field 'a.b'"); +} + +TEST(UnsetNodeTest, ApplyCannotRemoveSuffixOfImmutablePath) { + auto update = fromjson("{$unset: {'a.b.c': true}}"); + const CollatorInterface* collator = nullptr; + UnsetNode node; + ASSERT_OK(node.init(update["$unset"]["a.b.c"], collator)); + + Document doc(fromjson("{a: {b: {c: 1}}}")); + FieldRef pathToCreate(""); + FieldRef pathTaken("a.b.c"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + const UpdateIndexData* indexData = nullptr; + LogBuilder* logBuilder = nullptr; + auto indexesAffected = false; + auto noop = false; + ASSERT_THROWS_CODE_AND_WHAT( + node.apply(doc.root()["a"]["b"]["c"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + indexData, + logBuilder, + &indexesAffected, + &noop), + UserException, + ErrorCodes::ImmutableField, + "Unsetting the path 'a.b.c' would modify the immutable field 'a.b'"); +} + +TEST(UnsetNodeTest, ApplyCanRemoveImmutablePathIfNoop) { + auto update = fromjson("{$unset: {'a.b.c': true}}"); + const CollatorInterface* collator = nullptr; + UnsetNode node; + ASSERT_OK(node.init(update["$unset"]["a.b.c"], collator)); + + Document doc(fromjson("{a: {b: 1}}")); + FieldRef pathToCreate("c"); + FieldRef pathTaken("a.b"); + StringData matchedField; + auto fromReplication = false; + auto validateForStorage = false; + FieldRefSet immutablePaths; + FieldRef path("a.b"); + immutablePaths.insert(&path); + UpdateIndexData indexData; + indexData.addPath("a"); + Document logDoc; + LogBuilder logBuilder(logDoc.root()); + auto indexesAffected = false; + auto noop = false; + node.apply(doc.root()["a"]["b"], + &pathToCreate, + &pathTaken, + matchedField, + fromReplication, + validateForStorage, + immutablePaths, + &indexData, + &logBuilder, + &indexesAffected, + &noop); + ASSERT_TRUE(noop); + ASSERT_FALSE(indexesAffected); + ASSERT_EQUALS(fromjson("{a: {b: 1}}"), doc); + ASSERT_TRUE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(fromjson("{}"), logDoc); +} + } // namespace diff --git a/src/mongo/db/update/update_array_node.cpp b/src/mongo/db/update/update_array_node.cpp index ded095236d7..e82e5d8aef3 100644 --- a/src/mongo/db/update/update_array_node.cpp +++ b/src/mongo/db/update/update_array_node.cpp @@ -51,6 +51,8 @@ void UpdateArrayNode::apply(mutablebson::Element element, FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, @@ -151,6 +153,8 @@ void UpdateArrayNode::apply(mutablebson::Element element, pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, childrenShouldLogThemselves ? logBuilder : nullptr, &childAffectsIndexes, diff --git a/src/mongo/db/update/update_array_node.h b/src/mongo/db/update/update_array_node.h index 6a14829e8fd..d99f24518bd 100644 --- a/src/mongo/db/update/update_array_node.h +++ b/src/mongo/db/update/update_array_node.h @@ -76,6 +76,8 @@ public: FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, diff --git a/src/mongo/db/update/update_array_node_test.cpp b/src/mongo/db/update/update_array_node_test.cpp index 1eb5d657ae1..00e9f174fc6 100644 --- a/src/mongo/db/update/update_array_node_test.cpp +++ b/src/mongo/db/update/update_array_node_test.cpp @@ -65,6 +65,8 @@ TEST(UpdateArrayNodeTest, ApplyCreatePathFails) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -77,6 +79,8 @@ TEST(UpdateArrayNodeTest, ApplyCreatePathFails) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -107,6 +111,8 @@ TEST(UpdateArrayNodeTest, ApplyToNonArrayFails) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -118,6 +124,8 @@ TEST(UpdateArrayNodeTest, ApplyToNonArrayFails) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -148,6 +156,8 @@ TEST(UpdateArrayNodeTest, UpdateIsAppliedToAllMatchingElements) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -159,6 +169,8 @@ TEST(UpdateArrayNodeTest, UpdateIsAppliedToAllMatchingElements) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -195,6 +207,8 @@ DEATH_TEST(UpdateArrayNodeTest, FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -206,6 +220,8 @@ DEATH_TEST(UpdateArrayNodeTest, &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -230,6 +246,8 @@ TEST(UpdateArrayNodeTest, UpdateForEmptyIdentifierIsAppliedToAllArrayElements) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -241,6 +259,8 @@ TEST(UpdateArrayNodeTest, UpdateForEmptyIdentifierIsAppliedToAllArrayElements) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -291,6 +311,8 @@ TEST(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElement) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -302,6 +324,8 @@ TEST(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElement) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -342,6 +366,8 @@ TEST(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElementsUsingMergedChildren FieldRef pathToCreate(""); FieldRef pathTaken(""); StringData matchedField; + auto validateForStorage = true; + FieldRefSet immutablePaths; auto fromReplication = false; UpdateIndexData indexData; indexData.addPath("a"); @@ -354,6 +380,8 @@ TEST(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElementsUsingMergedChildren &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -404,6 +432,8 @@ TEST(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElementsWithoutMergedChildr FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -415,6 +445,8 @@ TEST(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElementsWithoutMergedChildr &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -450,6 +482,8 @@ TEST(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElementWithEmptyIdentifiers FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -461,6 +495,8 @@ TEST(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElementWithEmptyIdentifiers &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -508,6 +544,8 @@ TEST(UpdateArrayNodeTest, ApplyNestedArrayUpdates) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -519,6 +557,8 @@ TEST(UpdateArrayNodeTest, ApplyNestedArrayUpdates) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -560,6 +600,8 @@ TEST(UpdateArrayNodeTest, ApplyUpdatesWithMergeConflictToArrayElementFails) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -571,6 +613,8 @@ TEST(UpdateArrayNodeTest, ApplyUpdatesWithMergeConflictToArrayElementFails) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -610,6 +654,8 @@ TEST(UpdateArrayNodeTest, ApplyUpdatesWithEmptyIdentifiersWithMergeConflictToArr FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -621,6 +667,8 @@ TEST(UpdateArrayNodeTest, ApplyUpdatesWithEmptyIdentifiersWithMergeConflictToArr &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -666,6 +714,8 @@ TEST(UpdateArrayNodeTest, ApplyNestedArrayUpdatesWithMergeConflictFails) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -677,6 +727,8 @@ TEST(UpdateArrayNodeTest, ApplyNestedArrayUpdatesWithMergeConflictFails) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -707,6 +759,8 @@ TEST(UpdateArrayNodeTest, NoArrayElementsMatch) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -718,6 +772,8 @@ TEST(UpdateArrayNodeTest, NoArrayElementsMatch) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -750,6 +806,8 @@ TEST(UpdateArrayNodeTest, UpdatesToAllArrayElementsAreNoops) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -761,6 +819,8 @@ TEST(UpdateArrayNodeTest, UpdatesToAllArrayElementsAreNoops) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -793,6 +853,8 @@ TEST(UpdateArrayNodeTest, NoArrayElementAffectsIndexes) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a.c"); Document logDoc; @@ -804,6 +866,8 @@ TEST(UpdateArrayNodeTest, NoArrayElementAffectsIndexes) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -836,6 +900,8 @@ TEST(UpdateArrayNodeTest, WhenOneElementIsMatchedLogElementUpdateDirectly) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -847,6 +913,8 @@ TEST(UpdateArrayNodeTest, WhenOneElementIsMatchedLogElementUpdateDirectly) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -879,6 +947,8 @@ TEST(UpdateArrayNodeTest, WhenOneElementIsModifiedLogElement) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -890,6 +960,8 @@ TEST(UpdateArrayNodeTest, WhenOneElementIsModifiedLogElement) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -919,6 +991,8 @@ TEST(UpdateArrayNodeTest, ArrayUpdateOnEmptyArrayIsANoop) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -930,6 +1004,8 @@ TEST(UpdateArrayNodeTest, ArrayUpdateOnEmptyArrayIsANoop) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -962,6 +1038,8 @@ TEST(UpdateArrayNodeTest, ApplyPositionalInsideArrayUpdate) { FieldRef pathTaken(""); StringData matchedField = "1"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -973,6 +1051,8 @@ TEST(UpdateArrayNodeTest, ApplyPositionalInsideArrayUpdate) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1005,6 +1085,8 @@ TEST(UpdateArrayNodeTest, ApplyArrayUpdateFromReplication) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -1016,6 +1098,8 @@ TEST(UpdateArrayNodeTest, ApplyArrayUpdateFromReplication) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1048,6 +1132,8 @@ TEST(UpdateArrayNodeTest, ApplyArrayUpdateNotFromReplication) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -1059,6 +1145,8 @@ TEST(UpdateArrayNodeTest, ApplyArrayUpdateNotFromReplication) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1089,6 +1177,8 @@ TEST(UpdateArrayNodeTest, ApplyArrayUpdateWithoutLogBuilderOrIndexData) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; LogBuilder* logBuilder = nullptr; auto indexesAffected = false; @@ -1098,6 +1188,8 @@ TEST(UpdateArrayNodeTest, ApplyArrayUpdateWithoutLogBuilderOrIndexData) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &indexesAffected, diff --git a/src/mongo/db/update/update_driver.cpp b/src/mongo/db/update/update_driver.cpp index 44e7dd73ea7..48ae3780d29 100644 --- a/src/mongo/db/update/update_driver.cpp +++ b/src/mongo/db/update/update_driver.cpp @@ -33,6 +33,7 @@ #include "mongo/base/string_data.h" #include "mongo/bson/mutable/algorithm.h" #include "mongo/bson/mutable/document.h" +#include "mongo/db/bson/dotted_path_support.h" #include "mongo/db/field_ref.h" #include "mongo/db/matcher/expression_leaf.h" #include "mongo/db/matcher/extensions_callback_noop.h" @@ -41,6 +42,7 @@ #include "mongo/db/update/log_builder.h" #include "mongo/db/update/modifier_table.h" #include "mongo/db/update/path_support.h" +#include "mongo/db/update/storage_validation.h" #include "mongo/util/embedded_builder.h" #include "mongo/util/mongoutils/str.h" @@ -240,7 +242,7 @@ inline Status UpdateDriver::addAndParse(const modifiertable::ModifierType type, Status UpdateDriver::populateDocumentWithQueryFields(OperationContext* opCtx, const BSONObj& query, - const vector<FieldRef*>* immutablePaths, + const FieldRefSet& immutablePaths, mutablebson::Document& doc) const { // We canonicalize the query to collapse $and/$or, and the namespace is not needed. Also, // because this is for the upsert case, where we insert a new document if one was not found, the @@ -258,26 +260,16 @@ Status UpdateDriver::populateDocumentWithQueryFields(OperationContext* opCtx, } Status UpdateDriver::populateDocumentWithQueryFields(const CanonicalQuery& query, - const vector<FieldRef*>* immutablePathsPtr, + const FieldRefSet& immutablePaths, mutablebson::Document& doc) const { EqualityMatches equalities; Status status = Status::OK(); if (isDocReplacement()) { - FieldRefSet pathsToExtract; - - // TODO: Refactor update logic, make _id just another immutable field - static const FieldRef idPath("_id"); - static const vector<FieldRef*> emptyImmutablePaths; - const vector<FieldRef*>& immutablePaths = - immutablePathsPtr ? *immutablePathsPtr : emptyImmutablePaths; - - pathsToExtract.fillFrom(immutablePaths); - pathsToExtract.insert(&idPath); // Extract only immutable fields from replacement-style status = - pathsupport::extractFullEqualityMatches(*query.root(), pathsToExtract, &equalities); + pathsupport::extractFullEqualityMatches(*query.root(), immutablePaths, &equalities); } else { // Extract all fields from op-style status = pathsupport::extractEqualityMatches(*query.root(), &equalities); @@ -291,23 +283,14 @@ Status UpdateDriver::populateDocumentWithQueryFields(const CanonicalQuery& query } Status UpdateDriver::update(StringData matchedField, + BSONObj original, mutablebson::Document* doc, + bool validateForStorage, + const FieldRefSet& immutablePaths, BSONObj* logOpRec, - FieldRefSet* updatedFields, bool* docWasModified) { // TODO: assert that update() is called at most once in a !_multi case. - // Use the passed in FieldRefSet - FieldRefSet* targetFields = updatedFields; - - // If we didn't get a FieldRefSet* from the caller, allocate storage and use - // the unique_ptr for lifecycle management - unique_ptr<FieldRefSet> targetFieldScopedPtr; - if (!targetFields) { - targetFieldScopedPtr.reset(new FieldRefSet()); - targetFields = targetFieldScopedPtr.get(); - } - _affectIndices = (isDocReplacement() && (_indexedFields != NULL)); _logDoc.reset(); @@ -325,6 +308,8 @@ Status UpdateDriver::update(StringData matchedField, &pathTaken, matchedField, _modOptions.fromReplication, + validateForStorage, + immutablePaths, _indexedFields, (_logOp && logOpRec) ? &logBuilder : nullptr, &indexesAffected, @@ -342,6 +327,8 @@ Status UpdateDriver::update(StringData matchedField, // We parsed using the old ModifierInterface implementation. // Ask each of the mods to type check whether they can operate over the current document // and, if so, to change that document accordingly. + FieldRefSet updatedPaths; + for (vector<ModifierInterface*>::iterator it = _mods.begin(); it != _mods.end(); ++it) { ModifierInterface::ExecInfo execInfo; Status status = (*it)->prepare(doc->root(), matchedField, &execInfo); @@ -372,7 +359,7 @@ Status UpdateDriver::update(StringData matchedField, // Record each field being updated but check for conflicts first const FieldRef* other; - if (!targetFields->insert(execInfo.fieldRef[i], &other)) { + if (!updatedPaths.insert(execInfo.fieldRef[i], &other)) { return Status(ErrorCodes::ConflictingUpdateOperators, str::stream() << "Cannot update '" << other->dottedField() << "' and '" @@ -412,6 +399,80 @@ Status UpdateDriver::update(StringData matchedField, } } } + + // Check for BSON depth and DBRef constraint violations. + if (validateForStorage) { + if (updatedPaths.empty()) { + + // In the case of full document replacement, check the whole document. + storage_validation::storageValid(*doc); + } else { + for (auto path = updatedPaths.begin(); path != updatedPaths.end(); ++path) { + + // Find the updated field in the updated document. + auto newElem = doc->root(); + for (size_t i = 0; i < (*path)->numParts(); ++i) { + newElem = newElem[(*path)->getPart(i)]; + if (!newElem.ok()) { + break; + } + } + + // newElem might be missing if $unset/$renamed-away. + if (newElem.ok()) { + + // Check parents. + const std::uint32_t recursionLevel = 0; + auto parentsDepth = + storage_validation::storageValidParents(newElem, recursionLevel); + + // Check element and its children. + const bool doRecursiveCheck = true; + storage_validation::storageValid(newElem, doRecursiveCheck, parentsDepth); + } + } + } + } + + for (auto path = immutablePaths.begin(); path != immutablePaths.end(); ++path) { + + if (!updatedPaths.empty() && !updatedPaths.findConflicts(*path, nullptr)) { + continue; + } + + // Find the updated field in the updated document. + auto newElem = doc->root(); + for (size_t i = 0; i < (*path)->numParts(); ++i) { + newElem = newElem[(*path)->getPart(i)]; + if (!newElem.ok()) { + break; + } + uassert(ErrorCodes::NotSingleValueField, + str::stream() + << "After applying the update to the document, the (immutable) field '" + << (*path)->dottedField() + << "' was found to be an array or array descendant.", + newElem.getType() != BSONType::Array); + } + + auto oldElem = + dotted_path_support::extractElementAtPath(original, (*path)->dottedField()); + + uassert(ErrorCodes::ImmutableField, + str::stream() << "After applying the update, the '" << (*path)->dottedField() + << "' (required and immutable) field was " + "found to have been removed --" + << original, + newElem.ok() || !oldElem.ok()); + if (newElem.ok() && oldElem.ok()) { + uassert(ErrorCodes::ImmutableField, + str::stream() << "After applying the update, the (immutable) field '" + << (*path)->dottedField() + << "' was found to have been altered to " + << newElem.toString(), + newElem.compareWithBSONElement(oldElem, nullptr, false) == 0); + } + } } if (_logOp && logOpRec) diff --git a/src/mongo/db/update/update_driver.h b/src/mongo/db/update/update_driver.h index 65785542414..9e91e3be3c6 100644 --- a/src/mongo/db/update/update_driver.h +++ b/src/mongo/db/update/update_driver.h @@ -78,14 +78,17 @@ public: * * Returns Status::OK() if the document can be used. If there are any error or * conflicts along the way then those errors will be returned. + * + * If the current update is a document replacement, only the 'immutablePaths' are extracted from + * 'query' and used to populate 'doc'. */ Status populateDocumentWithQueryFields(OperationContext* opCtx, const BSONObj& query, - const std::vector<FieldRef*>* immutablePaths, + const FieldRefSet& immutablePaths, mutablebson::Document& doc) const; Status populateDocumentWithQueryFields(const CanonicalQuery& query, - const std::vector<FieldRef*>* immutablePaths, + const FieldRefSet& immutablePaths, mutablebson::Document& doc) const; /** @@ -95,23 +98,26 @@ public: BSONObj makeOplogEntryQuery(const BSONObj& doc, bool multi) const; /** - * Returns OK and executes '_mods' over 'doc', generating 'newObj'. If any mod is - * positional, use 'matchedField' (index of the array item matched). If doc allows - * mods to be applied in place and no index updating is involved, then the mods may - * be applied "in place" over 'doc'. + * Executes the update over 'doc'. If any modifier is positional, use 'matchedField' (index of + * the array item matched). If 'doc' allows the modifiers to be applied in place and no index + * updating is involved, then the modifiers may be applied "in place" over 'doc'. * - * If the driver's '_logOp' mode is turned on, and if 'logOpRec' is not NULL, fills in - * the latter with the oplog entry corresponding to the update. If '_mods' can't be - * applied, returns an error status with a corresponding description. + * If the driver's '_logOp' mode is turned on, and if 'logOpRec' is not null, fills in the + * latter with the oplog entry corresponding to the update. If the modifiers can't be applied, + * returns an error status or uasserts with a corresponding description. * - * If a non-NULL updatedField vector* is supplied, - * then all updated fields will be added to it. + * If 'validateForStorage' is true, ensures that modified elements do not violate depth or DBRef + * constraints. Ensures that no paths in 'immutablePaths' are modified (though they may be + * created, if they do not yet exist). The original document 'original' is needed for checking + * immutable paths for the ModifierInterface implementation. */ Status update(StringData matchedField, + BSONObj original, mutablebson::Document* doc, - BSONObj* logOpRec = NULL, - FieldRefSet* updatedFields = NULL, - bool* docWasModified = NULL); + bool validateForStorage, + const FieldRefSet& immutablePaths, + BSONObj* logOpRec = nullptr, + bool* docWasModified = nullptr); // // Accessors diff --git a/src/mongo/db/update/update_driver_test.cpp b/src/mongo/db/update/update_driver_test.cpp index 8f4bdfc25df..e7db2df426e 100644 --- a/src/mongo/db/update/update_driver_test.cpp +++ b/src/mongo/db/update/update_driver_test.cpp @@ -159,10 +159,21 @@ TEST(Collator, SetCollationUpdatesModifierInterfaces) { ASSERT_OK(driver.parse(updateDocument, arrayFilters)); ASSERT_EQUALS(driver.numMods(), 1U); + const BSONObj original; + const bool validateForStorage = true; + const FieldRefSet emptyImmutablePaths; bool modified = false; Document doc(fromjson("{a: 'cba'}")); driver.setCollator(&collator); - driver.update(StringData(), &doc, nullptr, nullptr, &modified).transitional_ignore(); + driver + .update(StringData(), + original, + &doc, + validateForStorage, + emptyImmutablePaths, + nullptr, + &modified) + .transitional_ignore(); ASSERT_TRUE(modified); } @@ -270,178 +281,265 @@ static void assertSameFields(const BSONObj& docA, const BSONObj& docB) { TEST_F(CreateFromQuery, BasicOp) { BSONObj query = fromjson("{a:1,b:2}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(query, doc().getObject()); } TEST_F(CreateFromQuery, BasicOpEq) { BSONObj query = fromjson("{a:{$eq:1}}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{a:1}"), doc().getObject()); } TEST_F(CreateFromQuery, BasicOpWithId) { BSONObj query = fromjson("{_id:1,a:1,b:2}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(query, doc().getObject()); } TEST_F(CreateFromQuery, BasicRepl) { BSONObj query = fromjson("{a:1,b:2}"); - ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{}"), doc().getObject()); } TEST_F(CreateFromQuery, BasicReplWithId) { BSONObj query = fromjson("{_id:1,a:1,b:2}"); - ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{_id:1}"), doc().getObject()); } TEST_F(CreateFromQuery, BasicReplWithIdEq) { BSONObj query = fromjson("{_id:{$eq:1},a:1,b:2}"); - ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{_id:1}"), doc().getObject()); } TEST_F(CreateFromQuery, NoRootIdOp) { BSONObj query = fromjson("{'_id.a':1,'_id.b':2}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{_id:{a:1,b:2}}"), doc().getObject()); } TEST_F(CreateFromQuery, NoRootIdRepl) { BSONObj query = fromjson("{'_id.a':1,'_id.b':2}"); - ASSERT_NOT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_NOT_OK( + driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); } TEST_F(CreateFromQuery, NestedSharedRootOp) { BSONObj query = fromjson("{'a.c':1,'a.b':{$eq:2}}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{a:{c:1,b:2}}"), doc().getObject()); } TEST_F(CreateFromQuery, OrQueryOp) { BSONObj query = fromjson("{$or:[{a:1}]}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{a:1}"), doc().getObject()); } TEST_F(CreateFromQuery, OrQueryIdRepl) { BSONObj query = fromjson("{$or:[{_id:1}]}"); - ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{_id:1}"), doc().getObject()); } TEST_F(CreateFromQuery, OrQueryNoExtractOps) { BSONObj query = fromjson("{$or:[{a:1}, {b:2}]}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(BSONObj(), doc().getObject()); } TEST_F(CreateFromQuery, OrQueryNoExtractIdRepl) { BSONObj query = fromjson("{$or:[{_id:1}, {_id:2}]}"); - ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(BSONObj(), doc().getObject()); } TEST_F(CreateFromQuery, AndQueryOp) { BSONObj query = fromjson("{$and:[{'a.c':1},{'a.b':{$eq:2}}]}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{a:{c:1,b:2}}"), doc().getObject()); } TEST_F(CreateFromQuery, AndQueryIdRepl) { BSONObj query = fromjson("{$and:[{_id:1},{a:{$eq:2}}]}"); - ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{_id:1}"), doc().getObject()); } TEST_F(CreateFromQuery, AllArrayOp) { BSONObj query = fromjson("{a:{$all:[1]}}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{a:1}"), doc().getObject()); } TEST_F(CreateFromQuery, AllArrayIdRepl) { BSONObj query = fromjson("{_id:{$all:[1]}, b:2}"); - ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{_id:1}"), doc().getObject()); } TEST_F(CreateFromQuery, ConflictFieldsFailOp) { BSONObj query = fromjson("{a:1,'a.b':1}"); - ASSERT_NOT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_NOT_OK( + driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); } TEST_F(CreateFromQuery, ConflictFieldsFailSameValueOp) { BSONObj query = fromjson("{a:{b:1},'a.b':1}"); - ASSERT_NOT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_NOT_OK( + driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); } TEST_F(CreateFromQuery, ConflictWithIdRepl) { BSONObj query = fromjson("{_id:1,'_id.a':1}"); - ASSERT_NOT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_NOT_OK( + driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); } TEST_F(CreateFromQuery, ConflictAndQueryOp) { BSONObj query = fromjson("{$and:[{a:{b:1}},{'a.b':{$eq:1}}]}"); - ASSERT_NOT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_NOT_OK( + driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); } TEST_F(CreateFromQuery, ConflictAllMultipleValsOp) { BSONObj query = fromjson("{a:{$all:[1, 2]}}"); - ASSERT_NOT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_NOT_OK( + driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); } TEST_F(CreateFromQuery, NoConflictOrQueryOp) { BSONObj query = fromjson("{$or:[{a:{b:1}},{'a.b':{$eq:1}}]}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(BSONObj(), doc().getObject()); } TEST_F(CreateFromQuery, ImmutableFieldsOp) { BSONObj query = fromjson("{$or:[{a:{b:1}},{'a.b':{$eq:1}}]}"); - ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, NULL, doc())); + FieldRef idFieldRef("_id"); + FieldRefSet immutablePaths; + immutablePaths.insert(&idFieldRef); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(BSONObj(), doc().getObject()); } TEST_F(CreateFromQuery, ShardKeyRepl) { BSONObj query = fromjson("{a:{$eq:1}}, b:2}"); - OwnedPointerVector<FieldRef> immutablePaths; - immutablePaths.push_back(new FieldRef("a")); - ASSERT_OK(driverRepl().populateDocumentWithQueryFields( - opCtx(), query, &immutablePaths.vector(), doc())); + OwnedPointerVector<FieldRef> immutablePathsVector; + immutablePathsVector.push_back(new FieldRef("a")); + immutablePathsVector.push_back(new FieldRef("_id")); + FieldRefSet immutablePaths; + immutablePaths.fillFrom(immutablePathsVector.vector()); + ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{a:1}"), doc().getObject()); } TEST_F(CreateFromQuery, NestedShardKeyRepl) { BSONObj query = fromjson("{a:{$eq:1},'b.c':2},d:2}"); - OwnedPointerVector<FieldRef> immutablePaths; - immutablePaths.push_back(new FieldRef("a")); - immutablePaths.push_back(new FieldRef("b.c")); - ASSERT_OK(driverRepl().populateDocumentWithQueryFields( - opCtx(), query, &immutablePaths.vector(), doc())); + OwnedPointerVector<FieldRef> immutablePathsVector; + immutablePathsVector.push_back(new FieldRef("a")); + immutablePathsVector.push_back(new FieldRef("b.c")); + immutablePathsVector.push_back(new FieldRef("_id")); + FieldRefSet immutablePaths; + immutablePaths.fillFrom(immutablePathsVector.vector()); + ASSERT_OK(driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{a:1,b:{c:2}}"), doc().getObject()); } TEST_F(CreateFromQuery, NestedShardKeyOp) { BSONObj query = fromjson("{a:{$eq:1},'b.c':2,d:{$all:[3]}},e:2}"); - OwnedPointerVector<FieldRef> immutablePaths; - immutablePaths.push_back(new FieldRef("a")); - immutablePaths.push_back(new FieldRef("b.c")); - ASSERT_OK(driverOps().populateDocumentWithQueryFields( - opCtx(), query, &immutablePaths.vector(), doc())); + OwnedPointerVector<FieldRef> immutablePathsVector; + immutablePathsVector.push_back(new FieldRef("a")); + immutablePathsVector.push_back(new FieldRef("b.c")); + immutablePathsVector.push_back(new FieldRef("_id")); + FieldRefSet immutablePaths; + immutablePaths.fillFrom(immutablePathsVector.vector()); + ASSERT_OK(driverOps().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); assertSameFields(fromjson("{a:1,b:{c:2},d:3}"), doc().getObject()); } TEST_F(CreateFromQuery, NotFullShardKeyRepl) { BSONObj query = fromjson("{a:{$eq:1}, 'b.c':2}, d:2}"); - OwnedPointerVector<FieldRef> immutablePaths; - immutablePaths.push_back(new FieldRef("a")); - immutablePaths.push_back(new FieldRef("b")); - ASSERT_NOT_OK(driverRepl().populateDocumentWithQueryFields( - opCtx(), query, &immutablePaths.vector(), doc())); + OwnedPointerVector<FieldRef> immutablePathsVector; + immutablePathsVector.push_back(new FieldRef("a")); + immutablePathsVector.push_back(new FieldRef("b")); + immutablePathsVector.push_back(new FieldRef("_id")); + FieldRefSet immutablePaths; + immutablePaths.fillFrom(immutablePathsVector.vector()); + ASSERT_NOT_OK( + driverRepl().populateDocumentWithQueryFields(opCtx(), query, immutablePaths, doc())); } } // namespace diff --git a/src/mongo/db/update/update_node.h b/src/mongo/db/update/update_node.h index f134c6272e8..a9e093008a3 100644 --- a/src/mongo/db/update/update_node.h +++ b/src/mongo/db/update/update_node.h @@ -32,7 +32,7 @@ #include "mongo/bson/bsonelement.h" #include "mongo/bson/mutable/element.h" -#include "mongo/db/field_ref.h" +#include "mongo/db/field_ref_set.h" #include "mongo/db/update/log_builder.h" #include "mongo/db/update_index_data.h" #include "mongo/util/assert_util.h" @@ -87,13 +87,17 @@ public: * when the update is from replication. Uses the index information in 'indexData' to determine * whether indexes are affected. If a LogBuilder is provided, logs the update. Outputs whether * the operation was a no-op. Trips a uassert (which throws UserException) if the update node - * cannot be applied to the document. + * cannot be applied to the document. If 'validateForStorage' is true, ensures that modified + * elements do not violate depth or DBRef constraints. Ensures that no paths in 'immutablePaths' + * are modified (though they may be created, if they do not yet exist). */ virtual void apply(mutablebson::Element element, FieldRef* pathToCreate, FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, diff --git a/src/mongo/db/update/update_object_node.cpp b/src/mongo/db/update/update_object_node.cpp index 3274c46d50f..f73ac0a5fdb 100644 --- a/src/mongo/db/update/update_object_node.cpp +++ b/src/mongo/db/update/update_object_node.cpp @@ -91,6 +91,8 @@ void applyChild(const UpdateNode& child, FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, @@ -116,6 +118,8 @@ void applyChild(const UpdateNode& child, pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, &childAffectsIndexes, @@ -360,6 +364,8 @@ void UpdateObjectNode::apply(mutablebson::Element element, FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, @@ -375,7 +381,8 @@ void UpdateObjectNode::apply(mutablebson::Element element, } // Capture arguments to applyChild() to avoid code duplication. - auto applyChildClosure = [=, &element](const UpdateNode& child, StringData field) { + auto applyChildClosure = [=, &element, &immutablePaths](const UpdateNode& child, + StringData field) { applyChild(child, field, &element, @@ -383,6 +390,8 @@ void UpdateObjectNode::apply(mutablebson::Element element, pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, logBuilder, indexesAffected, diff --git a/src/mongo/db/update/update_object_node.h b/src/mongo/db/update/update_object_node.h index 3256988a766..d43fc09026e 100644 --- a/src/mongo/db/update/update_object_node.h +++ b/src/mongo/db/update/update_object_node.h @@ -96,6 +96,8 @@ public: FieldRef* pathTaken, StringData matchedField, bool fromReplication, + bool validateForStorage, + const FieldRefSet& immutablePaths, const UpdateIndexData* indexData, LogBuilder* logBuilder, bool* indexesAffected, diff --git a/src/mongo/db/update/update_object_node_test.cpp b/src/mongo/db/update/update_object_node_test.cpp index 63dabbb402a..524b5ec8295 100644 --- a/src/mongo/db/update/update_object_node_test.cpp +++ b/src/mongo/db/update/update_object_node_test.cpp @@ -1705,6 +1705,8 @@ TEST(UpdateObjectNodeTest, ApplyCreateField) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("b"); Document logDoc; @@ -1716,6 +1718,8 @@ TEST(UpdateObjectNodeTest, ApplyCreateField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1745,6 +1749,8 @@ TEST(UpdateObjectNodeTest, ApplyExistingField) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -1756,6 +1762,8 @@ TEST(UpdateObjectNodeTest, ApplyExistingField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1803,6 +1811,8 @@ TEST(UpdateObjectNodeTest, ApplyExistingAndNonexistingFields) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -1814,6 +1824,8 @@ TEST(UpdateObjectNodeTest, ApplyExistingAndNonexistingFields) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1861,6 +1873,8 @@ TEST(UpdateObjectNodeTest, ApplyExistingNestedPaths) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -1872,6 +1886,8 @@ TEST(UpdateObjectNodeTest, ApplyExistingNestedPaths) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1920,6 +1936,8 @@ TEST(UpdateObjectNodeTest, ApplyCreateNestedPaths) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -1931,6 +1949,8 @@ TEST(UpdateObjectNodeTest, ApplyCreateNestedPaths) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -1973,6 +1993,8 @@ TEST(UpdateObjectNodeTest, ApplyCreateDeeplyNestedPaths) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -1984,6 +2006,8 @@ TEST(UpdateObjectNodeTest, ApplyCreateDeeplyNestedPaths) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2038,6 +2062,8 @@ TEST(UpdateObjectNodeTest, ChildrenShouldBeAppliedInAlphabeticalOrder) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2049,6 +2075,8 @@ TEST(UpdateObjectNodeTest, ChildrenShouldBeAppliedInAlphabeticalOrder) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2084,6 +2112,8 @@ TEST(UpdateObjectNodeTest, CollatorShouldNotAffectUpdateOrder) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("abc"); Document logDoc; @@ -2095,6 +2125,8 @@ TEST(UpdateObjectNodeTest, CollatorShouldNotAffectUpdateOrder) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2136,6 +2168,8 @@ TEST(UpdateObjectNodeTest, ApplyNoop) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); indexData.addPath("b"); @@ -2149,6 +2183,8 @@ TEST(UpdateObjectNodeTest, ApplyNoop) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2190,6 +2226,8 @@ TEST(UpdateObjectNodeTest, ApplySomeChildrenNoops) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); indexData.addPath("b"); @@ -2203,6 +2241,8 @@ TEST(UpdateObjectNodeTest, ApplySomeChildrenNoops) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2232,6 +2272,8 @@ TEST(UpdateObjectNodeTest, ApplyBlockingElement) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2243,6 +2285,8 @@ TEST(UpdateObjectNodeTest, ApplyBlockingElement) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2276,6 +2320,8 @@ TEST(UpdateObjectNodeTest, ApplyBlockingElementFromReplication) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = true; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2287,6 +2333,8 @@ TEST(UpdateObjectNodeTest, ApplyBlockingElementFromReplication) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2316,6 +2364,8 @@ TEST(UpdateObjectNodeTest, ApplyPositionalMissingMatchedField) { FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2328,6 +2378,8 @@ TEST(UpdateObjectNodeTest, ApplyPositionalMissingMatchedField) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2361,6 +2413,8 @@ TEST(UpdateObjectNodeTest, ApplyMergePositionalChild) { FieldRef pathTaken(""); StringData matchedField = "0"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2372,6 +2426,8 @@ TEST(UpdateObjectNodeTest, ApplyMergePositionalChild) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2419,6 +2475,8 @@ TEST(UpdateObjectNodeTest, ApplyOrderMergedPositionalChild) { FieldRef pathTaken(""); StringData matchedField = "1"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2430,6 +2488,8 @@ TEST(UpdateObjectNodeTest, ApplyOrderMergedPositionalChild) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2466,6 +2526,8 @@ TEST(UpdateObjectNodeTest, ApplyMergeConflictWithPositionalChild) { FieldRef pathTaken(""); StringData matchedField = "0"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2477,6 +2539,8 @@ TEST(UpdateObjectNodeTest, ApplyMergeConflictWithPositionalChild) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2516,6 +2580,8 @@ TEST(UpdateObjectNodeTest, ApplyDoNotMergePositionalChild) { FieldRef pathTaken(""); StringData matchedField = "1"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2527,6 +2593,8 @@ TEST(UpdateObjectNodeTest, ApplyDoNotMergePositionalChild) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2568,6 +2636,8 @@ TEST(UpdateObjectNodeTest, ApplyPositionalChildLast) { FieldRef pathTaken(""); StringData matchedField = "2"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2579,6 +2649,8 @@ TEST(UpdateObjectNodeTest, ApplyPositionalChildLast) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2614,6 +2686,8 @@ TEST(UpdateObjectNodeTest, ApplyUseStoredMergedPositional) { FieldRef pathTaken(""); StringData matchedField = "0"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2625,6 +2699,8 @@ TEST(UpdateObjectNodeTest, ApplyUseStoredMergedPositional) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2643,6 +2719,8 @@ TEST(UpdateObjectNodeTest, ApplyUseStoredMergedPositional) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder2, &indexesAffected, @@ -2684,6 +2762,8 @@ TEST(UpdateObjectNodeTest, ApplyDoNotUseStoredMergedPositional) { FieldRef pathTaken(""); StringData matchedField = "0"; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; UpdateIndexData indexData; indexData.addPath("a"); Document logDoc; @@ -2695,6 +2775,8 @@ TEST(UpdateObjectNodeTest, ApplyDoNotUseStoredMergedPositional) { &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder, &indexesAffected, @@ -2714,6 +2796,8 @@ TEST(UpdateObjectNodeTest, ApplyDoNotUseStoredMergedPositional) { &pathTaken, matchedField2, fromReplication, + validateForStorage, + immutablePaths, &indexData, &logBuilder2, &indexesAffected, @@ -2750,6 +2834,8 @@ TEST(UpdateObjectNodeTest, SetAndPopModifiersWithCommonPrefixApplySuccessfully) FieldRef pathTaken(""); StringData matchedField; auto fromReplication = false; + auto validateForStorage = true; + FieldRefSet immutablePaths; const UpdateIndexData* indexData = nullptr; Document logDoc; LogBuilder logBuilder(logDoc.root()); @@ -2760,6 +2846,8 @@ TEST(UpdateObjectNodeTest, SetAndPopModifiersWithCommonPrefixApplySuccessfully) &pathTaken, matchedField, fromReplication, + validateForStorage, + immutablePaths, indexData, &logBuilder, &indexesAffected, |