diff options
48 files changed, 1025 insertions, 670 deletions
diff --git a/src/mongo/db/update/SConscript b/src/mongo/db/update/SConscript index b390689f796..e17a2076b2e 100644 --- a/src/mongo/db/update/SConscript +++ b/src/mongo/db/update/SConscript @@ -8,9 +8,9 @@ env.Library( target='update_common', source=[ 'field_checker.cpp', - 'log_builder.cpp', 'path_support.cpp', 'storage_validation.cpp', + 'v1_log_builder.cpp', ], LIBDEPS=[ '$BUILD_DIR/mongo/base', @@ -119,7 +119,6 @@ env.CppUnitTest( 'document_diff_applier_test.cpp', 'document_diff_test.cpp', 'field_checker_test.cpp', - 'log_builder_test.cpp', 'modifier_table_test.cpp', 'object_replace_executor_test.cpp', 'path_support_test.cpp', @@ -136,6 +135,7 @@ env.CppUnitTest( 'update_driver_test.cpp', 'update_object_node_test.cpp', 'update_serialization_test.cpp', + 'v1_log_builder_test.cpp', ], LIBDEPS=[ '$BUILD_DIR/mongo/bson/mutable/mutable_bson', diff --git a/src/mongo/db/update/addtoset_node.cpp b/src/mongo/db/update/addtoset_node.cpp index b12c4ceeb9b..f7b821815d1 100644 --- a/src/mongo/db/update/addtoset_node.cpp +++ b/src/mongo/db/update/addtoset_node.cpp @@ -104,8 +104,8 @@ void AddToSetNode::setCollator(const CollatorInterface* collator) { deduplicate(_elements, _collator); } -ModifierNode::ModifyResult AddToSetNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { +ModifierNode::ModifyResult AddToSetNode::updateExistingElement(mutablebson::Element* element, + const FieldRef& elementPath) const { uassert(ErrorCodes::BadValue, str::stream() << "Cannot apply $addToSet to non-array field. Field named '" << element->getFieldName() << "' has non-array type " diff --git a/src/mongo/db/update/addtoset_node.h b/src/mongo/db/update/addtoset_node.h index 74564c91c47..240cf3b3484 100644 --- a/src/mongo/db/update/addtoset_node.h +++ b/src/mongo/db/update/addtoset_node.h @@ -57,7 +57,7 @@ public: protected: ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void setValueForNewElement(mutablebson::Element* element) const final; bool allowCreation() const final { diff --git a/src/mongo/db/update/addtoset_node_test.cpp b/src/mongo/db/update/addtoset_node_test.cpp index 9c3cac056be..c9b691126ee 100644 --- a/src/mongo/db/update/addtoset_node_test.cpp +++ b/src/mongo/db/update/addtoset_node_test.cpp @@ -113,7 +113,7 @@ TEST_F(AddToSetNodeTest, ApplyFailsOnNonArray) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 2}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), AssertionException, @@ -128,7 +128,7 @@ TEST_F(AddToSetNodeTest, ApplyNonEach) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -146,7 +146,7 @@ TEST_F(AddToSetNodeTest, ApplyNonEachArray) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -164,7 +164,7 @@ TEST_F(AddToSetNodeTest, ApplyEach) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -182,7 +182,7 @@ TEST_F(AddToSetNodeTest, ApplyToEmptyArray) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -200,7 +200,7 @@ TEST_F(AddToSetNodeTest, ApplyDeduplicateElementsToAdd) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -218,7 +218,7 @@ TEST_F(AddToSetNodeTest, ApplyDoNotAddExistingElements) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -236,7 +236,7 @@ TEST_F(AddToSetNodeTest, ApplyDoNotDeduplicateExistingElements) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -254,7 +254,7 @@ TEST_F(AddToSetNodeTest, ApplyNoElementsToAdd) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -272,7 +272,7 @@ TEST_F(AddToSetNodeTest, ApplyNoNonDuplicateElementsToAdd) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -329,7 +329,7 @@ TEST_F(AddToSetNodeTest, ApplyDeduplicationOfElementsToAddRespectsCollation) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -350,7 +350,7 @@ TEST_F(AddToSetNodeTest, ApplyComparisonToExistingElementsRespectsCollation) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: ['ABC']}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -372,7 +372,7 @@ TEST_F(AddToSetNodeTest, ApplyRespectsCollationFromSetCollator) { node.setCollator(&caseInsensitiveCollator); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -415,7 +415,7 @@ TEST_F(AddToSetNodeTest, ApplyNestedArray) { ASSERT_OK(node.init(update["$addToSet"]["a.1"], expCtx)); mutablebson::Document doc(fromjson("{ _id : 1, a : [ 1, [ ] ] }")); - setPathTaken("a.1"); + setPathTaken(makeRuntimeUpdatePathForTest("a.1")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][1]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -433,7 +433,7 @@ TEST_F(AddToSetNodeTest, ApplyIndexesNotAffected) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -450,7 +450,7 @@ TEST_F(AddToSetNodeTest, ApplyNoIndexDataOrLogBuilder) { ASSERT_OK(node.init(update["$addToSet"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); diff --git a/src/mongo/db/update/arithmetic_node.cpp b/src/mongo/db/update/arithmetic_node.cpp index 58c0d4a27ab..7b8f53efacf 100644 --- a/src/mongo/db/update/arithmetic_node.cpp +++ b/src/mongo/db/update/arithmetic_node.cpp @@ -63,7 +63,7 @@ Status ArithmeticNode::init(BSONElement modExpr, } ModifierNode::ModifyResult ArithmeticNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { + mutablebson::Element* element, const FieldRef& elementPath) const { if (!element->isNumeric()) { auto idElem = mutablebson::findFirstChildNamed(element->getDocument().root(), "_id"); uasserted(ErrorCodes::TypeMismatch, diff --git a/src/mongo/db/update/arithmetic_node.h b/src/mongo/db/update/arithmetic_node.h index c450e6dcdcc..4b25a52946c 100644 --- a/src/mongo/db/update/arithmetic_node.h +++ b/src/mongo/db/update/arithmetic_node.h @@ -59,7 +59,7 @@ public: protected: ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void setValueForNewElement(mutablebson::Element* element) const final; bool allowCreation() const final { diff --git a/src/mongo/db/update/arithmetic_node_test.cpp b/src/mongo/db/update/arithmetic_node_test.cpp index 85ce7e8b83c..03941282e4a 100644 --- a/src/mongo/db/update/arithmetic_node_test.cpp +++ b/src/mongo/db/update/arithmetic_node_test.cpp @@ -120,7 +120,7 @@ TEST_F(ArithmeticNodeTest, ApplyIncNoOp) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -138,7 +138,7 @@ TEST_F(ArithmeticNodeTest, ApplyMulNoOp) { ASSERT_OK(node.init(update["$mul"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -156,7 +156,7 @@ TEST_F(ArithmeticNodeTest, ApplyRoundingNoOp) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 6.022e23}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -174,7 +174,7 @@ TEST_F(ArithmeticNodeTest, ApplyEmptyPathToCreate) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -193,7 +193,7 @@ TEST_F(ArithmeticNodeTest, ApplyCreatePath) { mutablebson::Document doc(fromjson("{a: {d: 5}}")); setPathToCreate("b.c"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -212,7 +212,7 @@ TEST_F(ArithmeticNodeTest, ApplyExtendPath) { mutablebson::Document doc(fromjson("{a: {c: 1}}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -247,7 +247,7 @@ TEST_F(ArithmeticNodeTest, ApplyPositional) { ASSERT_OK(node.init(update["$inc"]["a.$"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2]}")); - setPathTaken("a.1"); + setPathTaken(makeRuntimeUpdatePathForTest("a.1")); setMatchedField("1"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][1]), getUpdateNodeApplyParams()); @@ -267,7 +267,7 @@ TEST_F(ArithmeticNodeTest, ApplyNonViablePathToInc) { mutablebson::Document doc(fromjson("{a: 5}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -284,7 +284,7 @@ TEST_F(ArithmeticNodeTest, ApplyNonViablePathToCreateFromReplicationIsNoOp) { mutablebson::Document doc(fromjson("{a: 5}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); setFromOplogApplication(true); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); @@ -303,7 +303,7 @@ TEST_F(ArithmeticNodeTest, ApplyNoIndexDataNoLogBuilder) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -320,7 +320,7 @@ TEST_F(ArithmeticNodeTest, ApplyDoesNotAffectIndexes) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -337,7 +337,7 @@ TEST_F(ArithmeticNodeTest, IncTypePromotionIsNotANoOp) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: NumberInt(2)}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -354,7 +354,7 @@ TEST_F(ArithmeticNodeTest, MulTypePromotionIsNotANoOp) { ASSERT_OK(node.init(update["$mul"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: NumberInt(2)}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -371,7 +371,7 @@ TEST_F(ArithmeticNodeTest, TypePromotionFromIntToDecimalIsNotANoOp) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: NumberInt(5)}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -389,7 +389,7 @@ TEST_F(ArithmeticNodeTest, TypePromotionFromLongToDecimalIsNotANoOp) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: NumberLong(5)}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -407,7 +407,7 @@ TEST_F(ArithmeticNodeTest, TypePromotionFromDoubleToDecimalIsNotANoOp) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5.25}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -425,7 +425,7 @@ TEST_F(ArithmeticNodeTest, ApplyPromoteToFloatingPoint) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: NumberLong(1)}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -442,7 +442,7 @@ TEST_F(ArithmeticNodeTest, IncrementedDecimalStaysDecimal) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: NumberDecimal(\"6.25\")}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -462,7 +462,7 @@ TEST_F(ArithmeticNodeTest, OverflowIntToLong) { const int initialValue = std::numeric_limits<int>::max(); mutablebson::Document doc(BSON("a" << initialValue)); ASSERT_EQUALS(mongo::NumberInt, doc.root()["a"].getType()); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -482,7 +482,7 @@ TEST_F(ArithmeticNodeTest, UnderflowIntToLong) { const int initialValue = std::numeric_limits<int>::min(); mutablebson::Document doc(BSON("a" << initialValue)); ASSERT_EQUALS(mongo::NumberInt, doc.root()["a"].getType()); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -501,7 +501,7 @@ TEST_F(ArithmeticNodeTest, IncModeCanBeReused) { mutablebson::Document doc1(fromjson("{a: 1}")); mutablebson::Document doc2(fromjson("{a: 2}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc1.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -511,7 +511,7 @@ TEST_F(ArithmeticNodeTest, IncModeCanBeReused) { ASSERT_EQUALS(getModifiedPaths(), "{a}"); resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); result = node.apply(getApplyParams(doc2.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -579,7 +579,7 @@ TEST_F(ArithmeticNodeTest, ApplyIncToObjectFails) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{_id: 'test_object', a: {b: 1}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -596,7 +596,7 @@ TEST_F(ArithmeticNodeTest, ApplyIncToArrayFails) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{_id: 'test_object', a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -613,7 +613,7 @@ TEST_F(ArithmeticNodeTest, ApplyIncToStringFails) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{_id: 'test_object', a: \"foo\"}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -630,7 +630,7 @@ TEST_F(ArithmeticNodeTest, OverflowingOperationFails) { ASSERT_OK(node.init(update["$mul"]["a"], expCtx)); mutablebson::Document doc(fromjson("{_id: 'test_object', a: NumberLong(9223372036854775807)}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -665,7 +665,7 @@ TEST_F(ArithmeticNodeTest, ApplyEmptyIndexData) { ASSERT_OK(node.init(update["$inc"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_EQUALS(fromjson("{a: 3}"), doc); ASSERT_TRUE(doc.isInPlaceModeEnabled()); @@ -681,7 +681,7 @@ TEST_F(ArithmeticNodeTest, ApplyNoOpDottedPath) { ASSERT_OK(node.init(update["$inc"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -698,7 +698,7 @@ TEST_F(ArithmeticNodeTest, TypePromotionOnDottedPathIsNotANoOp) { ASSERT_OK(node.init(update["$inc"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: NumberInt(2)}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -716,7 +716,7 @@ TEST_F(ArithmeticNodeTest, ApplyPathNotViableArray) { mutablebson::Document doc(fromjson("{a:[{b:1}]}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), AssertionException, @@ -731,7 +731,7 @@ TEST_F(ArithmeticNodeTest, ApplyInPlaceDottedPath) { ASSERT_OK(node.init(update["$inc"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 1}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -748,7 +748,7 @@ TEST_F(ArithmeticNodeTest, ApplyPromotionDottedPath) { ASSERT_OK(node.init(update["$inc"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: NumberInt(3)}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -799,7 +799,7 @@ TEST_F(ArithmeticNodeTest, ApplyNoOpArrayIndex) { ASSERT_OK(node.init(update["$inc"]["a.2.b"], expCtx)); mutablebson::Document doc(fromjson("{a: [{b: 0},{b: 1},{b: 2}]}")); - setPathTaken("a.2.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.2.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][2]["b"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -817,7 +817,7 @@ TEST_F(ArithmeticNodeTest, TypePromotionInArrayIsNotANoOp) { mutablebson::Document doc( fromjson("{a: [{b: NumberInt(0)},{b: NumberInt(1)},{b: NumberInt(2)}]}")); - setPathTaken("a.2.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.2.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][2]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -835,7 +835,7 @@ TEST_F(ArithmeticNodeTest, ApplyNonViablePathThroughArray) { mutablebson::Document doc(fromjson("{a: 0}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), AssertionException, @@ -850,7 +850,7 @@ TEST_F(ArithmeticNodeTest, ApplyInPlaceArrayIndex) { ASSERT_OK(node.init(update["$inc"]["a.2.b"], expCtx)); mutablebson::Document doc(fromjson("{a: [{b: 0},{b: 1},{b: 1}]}")); - setPathTaken("a.2.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.2.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][2]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -868,7 +868,7 @@ TEST_F(ArithmeticNodeTest, ApplyAppendArray) { mutablebson::Document doc(fromjson("{a: [{b: 0},{b: 1}]}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -886,7 +886,7 @@ TEST_F(ArithmeticNodeTest, ApplyPaddingArray) { mutablebson::Document doc(fromjson("{a: [{b: 0}]}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -904,7 +904,7 @@ TEST_F(ArithmeticNodeTest, ApplyNumericObject) { mutablebson::Document doc(fromjson("{a: {b: 0}}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -921,7 +921,10 @@ TEST_F(ArithmeticNodeTest, ApplyNumericField) { ASSERT_OK(node.init(update["$inc"]["a.2.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {'2': {b: 1}}}")); - setPathTaken("a.2.b"); + setPathTaken(RuntimeUpdatePath(FieldRef("a.2.b"), + {RuntimeUpdatePath::ComponentType::kFieldName, + RuntimeUpdatePath::ComponentType::kFieldName, + RuntimeUpdatePath::ComponentType::kFieldName})); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["2"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -939,7 +942,9 @@ TEST_F(ArithmeticNodeTest, ApplyExtendNumericField) { mutablebson::Document doc(fromjson("{a: {'2': {c: 1}}}")); setPathToCreate("b"); - setPathTaken("a.2"); + setPathTaken(RuntimeUpdatePath(FieldRef("a.2"), + {RuntimeUpdatePath::ComponentType::kFieldName, + RuntimeUpdatePath::ComponentType::kFieldName})); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["2"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -957,7 +962,7 @@ TEST_F(ArithmeticNodeTest, ApplyNumericFieldToEmptyObject) { mutablebson::Document doc(fromjson("{a: {}}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -975,7 +980,7 @@ TEST_F(ArithmeticNodeTest, ApplyEmptyArray) { mutablebson::Document doc(fromjson("{a: []}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -993,7 +998,7 @@ TEST_F(ArithmeticNodeTest, ApplyLogDottedPath) { mutablebson::Document doc(fromjson("{a: [{b:0}, {b:1}]}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_EQUALS(fromjson("{a: [{b:0}, {b:1}, {b:2}]}"), doc); ASSERT_FALSE(doc.isInPlaceModeEnabled()); @@ -1010,7 +1015,7 @@ TEST_F(ArithmeticNodeTest, LogEmptyArray) { mutablebson::Document doc(fromjson("{a: []}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_EQUALS(fromjson("{a: [null, null, {b:2}]}"), doc); ASSERT_FALSE(doc.isInPlaceModeEnabled()); @@ -1027,7 +1032,7 @@ TEST_F(ArithmeticNodeTest, LogEmptyObject) { mutablebson::Document doc(fromjson("{a: {}}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_EQUALS(fromjson("{a: {'2': {b: 2}}}"), doc); ASSERT_FALSE(doc.isInPlaceModeEnabled()); @@ -1066,7 +1071,7 @@ TEST_F(ArithmeticNodeTest, ApplyToDeserializedDocNoOp) { // De-serialize the int. doc.root()["a"].setValueInt(2).transitional_ignore(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -1086,7 +1091,7 @@ TEST_F(ArithmeticNodeTest, ApplyToDeserializedDocNestedNoop) { // De-serialize the int. doc.root().appendObject("a", BSON("b" << static_cast<int>(1))).transitional_ignore(); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -1106,7 +1111,7 @@ TEST_F(ArithmeticNodeTest, ApplyToDeserializedDocNestedNotNoop) { // De-serialize the int. doc.root().appendObject("a", BSON("b" << static_cast<int>(1))).transitional_ignore(); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); diff --git a/src/mongo/db/update/array_culling_node.cpp b/src/mongo/db/update/array_culling_node.cpp index c9afb985178..00a442a8d5f 100644 --- a/src/mongo/db/update/array_culling_node.cpp +++ b/src/mongo/db/update/array_culling_node.cpp @@ -34,7 +34,7 @@ namespace mongo { ModifierNode::ModifyResult ArrayCullingNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { + mutablebson::Element* element, const FieldRef& elementPath) const { invariant(element->ok()); uassert(ErrorCodes::BadValue, "Cannot apply $pull to a non-array value", diff --git a/src/mongo/db/update/array_culling_node.h b/src/mongo/db/update/array_culling_node.h index dd59033d3b7..b6f2c1f2edb 100644 --- a/src/mongo/db/update/array_culling_node.h +++ b/src/mongo/db/update/array_culling_node.h @@ -46,7 +46,7 @@ namespace mongo { class ArrayCullingNode : public ModifierNode { public: ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void validateUpdate(mutablebson::ConstElement updatedElement, mutablebson::ConstElement leftSibling, diff --git a/src/mongo/db/update/bit_node.cpp b/src/mongo/db/update/bit_node.cpp index 19f7a560846..98c91ada662 100644 --- a/src/mongo/db/update/bit_node.cpp +++ b/src/mongo/db/update/bit_node.cpp @@ -86,8 +86,8 @@ Status BitNode::init(BSONElement modExpr, const boost::intrusive_ptr<ExpressionC return Status::OK(); } -ModifierNode::ModifyResult BitNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { +ModifierNode::ModifyResult BitNode::updateExistingElement(mutablebson::Element* element, + const FieldRef& elementPath) const { if (!element->isIntegral()) { mutablebson::Element idElem = mutablebson::findFirstChildNamed(element->getDocument().root(), "_id"); diff --git a/src/mongo/db/update/bit_node.h b/src/mongo/db/update/bit_node.h index a2d51dadb4d..a81b98ee5b5 100644 --- a/src/mongo/db/update/bit_node.h +++ b/src/mongo/db/update/bit_node.h @@ -56,7 +56,7 @@ public: protected: ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void setValueForNewElement(mutablebson::Element* element) const final; bool allowCreation() const final { diff --git a/src/mongo/db/update/bit_node_test.cpp b/src/mongo/db/update/bit_node_test.cpp index 488ad971e5d..b8e652b1acf 100644 --- a/src/mongo/db/update/bit_node_test.cpp +++ b/src/mongo/db/update/bit_node_test.cpp @@ -203,7 +203,7 @@ TEST_F(BitNodeTest, ApplyAndLogSimpleDocumentAnd) { ASSERT_OK(node.init(update["$bit"]["a"], expCtx)); mutablebson::Document doc(BSON("a" << 0b0101)); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_EQUALS(BSON("a" << 0b0100), doc); @@ -218,7 +218,7 @@ TEST_F(BitNodeTest, ApplyAndLogSimpleDocumentOr) { ASSERT_OK(node.init(update["$bit"]["a"], expCtx)); mutablebson::Document doc(BSON("a" << 0b0101)); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_EQUALS(BSON("a" << 0b0111), doc); @@ -233,7 +233,7 @@ TEST_F(BitNodeTest, ApplyAndLogSimpleDocumentXor) { ASSERT_OK(node.init(update["$bit"]["a"], expCtx)); mutablebson::Document doc(BSON("a" << 0b0101)); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_EQUALS(BSON("a" << 0b0011), doc); @@ -248,7 +248,7 @@ TEST_F(BitNodeTest, ApplyShouldReportNoOp) { ASSERT_OK(node.init(update["$bit"]["a"], expCtx)); mutablebson::Document doc(BSON("a" << 1)); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); ASSERT_EQUALS(BSON("a" << static_cast<int>(1)), doc); @@ -268,7 +268,7 @@ TEST_F(BitNodeTest, ApplyMultipleBitOps) { ASSERT_OK(node.init(update["$bit"]["a"], expCtx)); mutablebson::Document doc(BSON("a" << 0b1111111100000000)); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_EQUALS(BSON("a" << 0b0101011001100110), doc); @@ -283,7 +283,7 @@ TEST_F(BitNodeTest, ApplyRepeatedBitOps) { ASSERT_OK(node.init(update["$bit"]["a"], expCtx)); mutablebson::Document doc(BSON("a" << 0b11110000)); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_EQUALS(BSON("a" << 0b10010110), doc); diff --git a/src/mongo/db/update/compare_node.cpp b/src/mongo/db/update/compare_node.cpp index f54cc8a2e57..5381d2e1d70 100644 --- a/src/mongo/db/update/compare_node.cpp +++ b/src/mongo/db/update/compare_node.cpp @@ -48,8 +48,8 @@ void CompareNode::setCollator(const CollatorInterface* collator) { _collator = collator; } -ModifierNode::ModifyResult CompareNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { +ModifierNode::ModifyResult CompareNode::updateExistingElement(mutablebson::Element* element, + const FieldRef& elementPath) const { const auto compareVal = element->compareWithBSONElement(_val, _collator, false); if ((compareVal == 0) || ((_mode == CompareMode::kMax) ? (compareVal > 0) : (compareVal < 0))) { return ModifyResult::kNoOp; diff --git a/src/mongo/db/update/compare_node.h b/src/mongo/db/update/compare_node.h index a1f25f96203..210d91b3a7c 100644 --- a/src/mongo/db/update/compare_node.h +++ b/src/mongo/db/update/compare_node.h @@ -59,7 +59,7 @@ public: protected: ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void setValueForNewElement(mutablebson::Element* element) const final; bool allowCreation() const final { diff --git a/src/mongo/db/update/compare_node_test.cpp b/src/mongo/db/update/compare_node_test.cpp index 246b933f7f7..a04c84f852f 100644 --- a/src/mongo/db/update/compare_node_test.cpp +++ b/src/mongo/db/update/compare_node_test.cpp @@ -63,7 +63,7 @@ TEST_F(CompareNodeTest, ApplyMaxSameNumber) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -80,7 +80,7 @@ TEST_F(CompareNodeTest, ApplyMinSameNumber) { ASSERT_OK(node.init(update["$min"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -97,7 +97,7 @@ TEST_F(CompareNodeTest, ApplyMaxNumberIsLess) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -114,7 +114,7 @@ TEST_F(CompareNodeTest, ApplyMinNumberIsMore) { ASSERT_OK(node.init(update["$min"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -131,7 +131,7 @@ TEST_F(CompareNodeTest, ApplyMaxSameValInt) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1.0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -148,7 +148,7 @@ TEST_F(CompareNodeTest, ApplyMaxSameValIntZero) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0.0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -165,7 +165,7 @@ TEST_F(CompareNodeTest, ApplyMinSameValIntZero) { ASSERT_OK(node.init(update["$min"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0.0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -199,7 +199,7 @@ TEST_F(CompareNodeTest, ApplyExistingNumberMinNumber) { ASSERT_OK(node.init(update["$min"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -233,7 +233,7 @@ TEST_F(CompareNodeTest, ApplyExistingNumberMaxNumber) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -250,7 +250,7 @@ TEST_F(CompareNodeTest, ApplyExistingDateMaxDate) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {$date: 0}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -267,7 +267,7 @@ TEST_F(CompareNodeTest, ApplyExistingEmbeddedDocMaxDoc) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -284,7 +284,7 @@ TEST_F(CompareNodeTest, ApplyExistingEmbeddedDocMaxNumber) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -304,7 +304,7 @@ TEST_F(CompareNodeTest, ApplyMinRespectsCollation) { ASSERT_OK(node.init(update["$min"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 'cbc'}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -325,7 +325,7 @@ TEST_F(CompareNodeTest, ApplyMinRespectsCollationFromSetCollator) { node.setCollator(&reverseStringCollator); mutablebson::Document doc(fromjson("{a: 'cbc'}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -346,7 +346,7 @@ TEST_F(CompareNodeTest, ApplyMaxRespectsCollationFromSetCollator) { node.setCollator(&reverseStringCollator); mutablebson::Document doc(fromjson("{a: 'cbc'}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -389,7 +389,7 @@ TEST_F(CompareNodeTest, ApplyIndexesNotAffected) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -406,7 +406,7 @@ TEST_F(CompareNodeTest, ApplyNoIndexDataOrLogBuilder) { ASSERT_OK(node.init(update["$max"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); diff --git a/src/mongo/db/update/current_date_node.cpp b/src/mongo/db/update/current_date_node.cpp index 7b8b9d4854b..aa6cb3ec663 100644 --- a/src/mongo/db/update/current_date_node.cpp +++ b/src/mongo/db/update/current_date_node.cpp @@ -97,7 +97,7 @@ Status CurrentDateNode::init(BSONElement modExpr, } ModifierNode::ModifyResult CurrentDateNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { + mutablebson::Element* element, const FieldRef& elementPath) const { setValue(_service, element, _typeIsDate); return ModifyResult::kNormalUpdate; } diff --git a/src/mongo/db/update/current_date_node.h b/src/mongo/db/update/current_date_node.h index 2c3e4492f86..f9ec59acb21 100644 --- a/src/mongo/db/update/current_date_node.h +++ b/src/mongo/db/update/current_date_node.h @@ -57,7 +57,7 @@ public: protected: ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void setValueForNewElement(mutablebson::Element* element) const final; bool allowCreation() const final { diff --git a/src/mongo/db/update/current_date_node_test.cpp b/src/mongo/db/update/current_date_node_test.cpp index 71622333925..a466de92157 100644 --- a/src/mongo/db/update/current_date_node_test.cpp +++ b/src/mongo/db/update/current_date_node_test.cpp @@ -132,7 +132,7 @@ TEST_F(CurrentDateNodeTest, ApplyTrue) { ASSERT_OK(node.init(update["$currentDate"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -156,7 +156,7 @@ TEST_F(CurrentDateNodeTest, ApplyFalse) { ASSERT_OK(node.init(update["$currentDate"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -180,7 +180,7 @@ TEST_F(CurrentDateNodeTest, ApplyDate) { ASSERT_OK(node.init(update["$currentDate"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -204,7 +204,7 @@ TEST_F(CurrentDateNodeTest, ApplyTimestamp) { ASSERT_OK(node.init(update["$currentDate"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -252,7 +252,7 @@ TEST_F(CurrentDateNodeTest, ApplyIndexesNotAffected) { ASSERT_OK(node.init(update["$currentDate"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -276,7 +276,7 @@ TEST_F(CurrentDateNodeTest, ApplyNoIndexDataOrLogBuilder) { ASSERT_OK(node.init(update["$currentDate"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); diff --git a/src/mongo/db/update/log_builder_interface.h b/src/mongo/db/update/log_builder_interface.h new file mode 100644 index 00000000000..961a224ec90 --- /dev/null +++ b/src/mongo/db/update/log_builder_interface.h @@ -0,0 +1,81 @@ +/** + * Copyright (C) 2020-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * 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 + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * 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 Server Side 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/base/status.h" +#include "mongo/base/string_data.h" +#include "mongo/bson/mutable/document.h" + +namespace mongo { +class RuntimeUpdatePath; + +/** + * Interface for building oplog entries for an update. Provides methods for logging updates, + * creations and deletes. + * + * The caller is expected to make sure all of the paths provided to the log*Field() methods are + * valid. For example, a sequence of calls which updates field 'a' to the value "foo" and then + * attempts to update field 'a.b' is a programming error. + */ +class LogBuilderInterface { +public: + virtual ~LogBuilderInterface() = default; + + /** + * These methods are used to log a modification to an existing field at given path. The field + * name provided in the 'elt' argument is ignored. + */ + virtual Status logUpdatedField(const RuntimeUpdatePath& path, mutablebson::Element elt) = 0; + + /** + * This method is used to log creation of a new field at the given path. The + * 'idxOfFirstNewComponent' argument indicates where the _first_ new component in the path + * is. For example, if an update operating on the document {a: {}} sets field 'a.b.c.d', the + * first new component would be at index 1 ('b'). + * + * The field name in the 'elt' argument is ignored. + */ + virtual Status logCreatedField(const RuntimeUpdatePath& path, + int idxOfFirstNewComponent, + mutablebson::Element elt) = 0; + virtual Status logCreatedField(const RuntimeUpdatePath& path, + int idxOfFirstNewComponent, + BSONElement elt) = 0; + /** + * Logs deletion of a field. + */ + virtual Status logDeletedField(const RuntimeUpdatePath& path) = 0; + + /** + * Serializes to a BSONObj which can be put into the 'o' section of an update oplog entry. + */ + virtual BSONObj serialize() const = 0; +}; +} // namespace mongo diff --git a/src/mongo/db/update/log_builder_test.cpp b/src/mongo/db/update/log_builder_test.cpp deleted file mode 100644 index a6cdf6421b6..00000000000 --- a/src/mongo/db/update/log_builder_test.cpp +++ /dev/null @@ -1,149 +0,0 @@ -/** - * Copyright (C) 2018-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * 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 - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * 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 Server Side 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/log_builder.h" - -#include "mongo/base/status.h" -#include "mongo/bson/mutable/mutable_bson_test_utils.h" -#include "mongo/db/json.h" -#include "mongo/unittest/unittest.h" -#include "mongo/util/safe_num.h" - -namespace { - -namespace mmb = mongo::mutablebson; -using mongo::LogBuilder; - -TEST(LogBuilder, Initialization) { - mmb::Document doc; - LogBuilder lb(doc.root()); - ASSERT_EQUALS(&doc, &lb.getDocument()); -} - -TEST(LogBuilder, AddOneToSet) { - mmb::Document doc; - LogBuilder lb(doc.root()); - - const mmb::Element elt_ab = doc.makeElementInt("a.b", 1); - ASSERT_TRUE(elt_ab.ok()); - ASSERT_OK(lb.addToSets(elt_ab)); - - ASSERT_EQUALS(mongo::fromjson("{ $set : { 'a.b' : 1 } }"), doc); -} - -TEST(LogBuilder, AddElementToSet) { - mmb::Document doc; - LogBuilder lb(doc.root()); - - const mmb::Element elt_ab = doc.makeElementInt("", 1); - ASSERT_TRUE(elt_ab.ok()); - ASSERT_OK(lb.addToSetsWithNewFieldName("a.b", elt_ab)); - - ASSERT_EQUALS(mongo::fromjson("{ $set : { 'a.b' : 1 } }"), doc); -} - -TEST(LogBuilder, AddBSONElementToSet) { - mmb::Document doc; - LogBuilder lb(doc.root()); - - mongo::BSONObj obj = mongo::fromjson("{'':1}"); - - ASSERT_OK(lb.addToSetsWithNewFieldName("a.b", obj.firstElement())); - - ASSERT_EQUALS(mongo::fromjson("{ $set : { 'a.b' : 1 } }"), doc); -} - -TEST(LogBuilder, AddSafeNumToSet) { - mmb::Document doc; - LogBuilder lb(doc.root()); - - mongo::BSONObj obj = mongo::fromjson("{'':1}"); - - ASSERT_OK(lb.addToSets("a.b", mongo::SafeNum(1))); - - ASSERT_EQUALS(mongo::fromjson("{ $set : { 'a.b' : 1 } }"), doc); -} - -TEST(LogBuilder, AddOneToUnset) { - mmb::Document doc; - LogBuilder lb(doc.root()); - ASSERT_OK(lb.addToUnsets("x.y")); - ASSERT_EQUALS(mongo::fromjson("{ $unset : { 'x.y' : true } }"), doc); -} - -TEST(LogBuilder, AddOneToEach) { - mmb::Document doc; - LogBuilder lb(doc.root()); - - const mmb::Element elt_ab = doc.makeElementInt("a.b", 1); - ASSERT_TRUE(elt_ab.ok()); - ASSERT_OK(lb.addToSets(elt_ab)); - - ASSERT_OK(lb.addToUnsets("x.y")); - - ASSERT_EQUALS(mongo::fromjson("{ " - " $set : { 'a.b' : 1 }, " - " $unset : { 'x.y' : true } " - "}"), - doc); -} - -TEST(LogBuilder, VerifySetsAreGrouped) { - mmb::Document doc; - LogBuilder lb(doc.root()); - - const mmb::Element elt_ab = doc.makeElementInt("a.b", 1); - ASSERT_TRUE(elt_ab.ok()); - ASSERT_OK(lb.addToSets(elt_ab)); - - const mmb::Element elt_xy = doc.makeElementInt("x.y", 1); - ASSERT_TRUE(elt_xy.ok()); - ASSERT_OK(lb.addToSets(elt_xy)); - - ASSERT_EQUALS(mongo::fromjson("{ $set : {" - " 'a.b' : 1, " - " 'x.y' : 1 " - "} }"), - doc); -} - -TEST(LogBuilder, VerifyUnsetsAreGrouped) { - mmb::Document doc; - LogBuilder lb(doc.root()); - - ASSERT_OK(lb.addToUnsets("a.b")); - ASSERT_OK(lb.addToUnsets("x.y")); - - ASSERT_EQUALS(mongo::fromjson("{ $unset : {" - " 'a.b' : true, " - " 'x.y' : true " - "} }"), - doc); -} -} // namespace diff --git a/src/mongo/db/update/modifier_node.cpp b/src/mongo/db/update/modifier_node.cpp index 9d5e48bcff9..28af3da1f4a 100644 --- a/src/mongo/db/update/modifier_node.cpp +++ b/src/mongo/db/update/modifier_node.cpp @@ -54,18 +54,18 @@ namespace { * already checked the update is not a noop. */ void checkImmutablePathsNotModifiedFromOriginal(mutablebson::Element element, - FieldRef* pathTaken, + const FieldRef& pathTaken, const FieldRefSet& immutablePaths, BSONObj original) { for (auto immutablePath = immutablePaths.begin(); immutablePath != immutablePaths.end(); ++immutablePath) { - auto prefixSize = pathTaken->commonPrefixSize(**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 " + str::stream() << "Updating the path '" << pathTaken.dottedField() << "' to " << element.toString() << " would modify the immutable field '" << (*immutablePath)->dottedField() << "'"); } @@ -73,7 +73,7 @@ void checkImmutablePathsNotModifiedFromOriginal(mutablebson::Element element, // 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()) { + if (prefixSize == pathTaken.numParts()) { auto oldElem = dotted_path_support::extractElementAtPath( original, (*immutablePath)->dottedField()); @@ -83,7 +83,7 @@ void checkImmutablePathsNotModifiedFromOriginal(mutablebson::Element element, } auto newElem = element; - for (size_t i = pathTaken->numParts(); i < (*immutablePath)->numParts(); ++i) { + 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 '" @@ -127,16 +127,16 @@ void checkImmutablePathsNotModifiedFromOriginal(mutablebson::Element element, * assume that we have already checked the update is not a noop. */ void checkImmutablePathsNotModified(mutablebson::Element element, - FieldRef* pathTaken, + const FieldRef& pathTaken, const FieldRefSet& immutablePaths) { for (auto immutablePath = immutablePaths.begin(); immutablePath != immutablePaths.end(); ++immutablePath) { uassert(ErrorCodes::ImmutableField, - str::stream() << "Performing an update on the path '" << pathTaken->dottedField() + str::stream() << "Performing an update on the path '" << pathTaken.dottedField() << "' would modify the immutable field '" << (*immutablePath)->dottedField() << "'", - pathTaken->commonPrefixSize(**immutablePath) < - std::min(pathTaken->numParts(), (*immutablePath)->numParts())); + pathTaken.commonPrefixSize(**immutablePath) < + std::min(pathTaken.numParts(), (*immutablePath)->numParts())); } } @@ -156,7 +156,7 @@ UpdateExecutor::ApplyResult ModifierNode::applyToExistingElement( for (auto immutablePath = applyParams.immutablePaths.begin(); immutablePath != applyParams.immutablePaths.end(); ++immutablePath) { - if (updateNodeApplyParams.pathTaken->isPrefixOf(**immutablePath)) { + if (updateNodeApplyParams.pathTaken->fieldRef().isPrefixOf(**immutablePath)) { compareWithOriginal = true; break; } @@ -169,42 +169,47 @@ UpdateExecutor::ApplyResult ModifierNode::applyToExistingElement( ModifyResult updateResult; if (compareWithOriginal) { BSONObj original = applyParams.element.getDocument().getObject(); - updateResult = updateExistingElement(&applyParams.element, updateNodeApplyParams.pathTaken); + updateResult = updateExistingElement(&applyParams.element, + updateNodeApplyParams.pathTaken->fieldRef()); if (updateResult == ModifyResult::kNoOp) { return ApplyResult::noopResult(); } checkImmutablePathsNotModifiedFromOriginal(applyParams.element, - updateNodeApplyParams.pathTaken.get(), + updateNodeApplyParams.pathTaken->fieldRef(), applyParams.immutablePaths, original); } else { - updateResult = updateExistingElement(&applyParams.element, updateNodeApplyParams.pathTaken); + updateResult = updateExistingElement(&applyParams.element, + updateNodeApplyParams.pathTaken->fieldRef()); if (updateResult == ModifyResult::kNoOp) { return ApplyResult::noopResult(); } - checkImmutablePathsNotModified( - applyParams.element, updateNodeApplyParams.pathTaken.get(), applyParams.immutablePaths); + checkImmutablePathsNotModified(applyParams.element, + updateNodeApplyParams.pathTaken->fieldRef(), + applyParams.immutablePaths); } invariant(updateResult != ModifyResult::kCreated); ApplyResult applyResult; if (!applyParams.indexData || - !applyParams.indexData->mightBeIndexed(*updateNodeApplyParams.pathTaken)) { + !applyParams.indexData->mightBeIndexed(updateNodeApplyParams.pathTaken->fieldRef())) { applyResult.indexesAffected = false; } if (applyParams.validateForStorage) { - const uint32_t recursionLevel = updateNodeApplyParams.pathTaken->numParts(); + const uint32_t recursionLevel = updateNodeApplyParams.pathTaken->size(); validateUpdate( applyParams.element, leftSibling, rightSibling, recursionLevel, updateResult); } if (auto logBuilder = updateNodeApplyParams.logBuilder) { logUpdate(logBuilder, - updateNodeApplyParams.pathTaken->dottedField(), + *updateNodeApplyParams.pathTaken, applyParams.element, - updateResult); + updateResult, + boost::none // No path was created. + ); } return applyResult; @@ -212,6 +217,8 @@ UpdateExecutor::ApplyResult ModifierNode::applyToExistingElement( UpdateExecutor::ApplyResult ModifierNode::applyToNonexistentElement( ApplyParams applyParams, UpdateNodeApplyParams updateNodeApplyParams) const { + invariant(!updateNodeApplyParams.pathToCreate->empty()); + if (allowCreation()) { auto newElementFieldName = updateNodeApplyParams.pathToCreate->getPart( updateNodeApplyParams.pathToCreate->numParts() - 1); @@ -242,7 +249,7 @@ UpdateExecutor::ApplyResult ModifierNode::applyToNonexistentElement( } if (applyParams.validateForStorage) { - const uint32_t recursionLevel = updateNodeApplyParams.pathTaken->numParts() + 1; + const uint32_t recursionLevel = updateNodeApplyParams.pathTaken->size() + 1; mutablebson::ConstElement elementForValidation = statusWithFirstCreatedElem.getValue(); validateUpdate(elementForValidation, elementForValidation.leftSibling(), @@ -261,28 +268,41 @@ UpdateExecutor::ApplyResult ModifierNode::applyToNonexistentElement( // because we just created this element.) uassert(ErrorCodes::ImmutableField, str::stream() << "Updating the path '" - << updateNodeApplyParams.pathTaken->dottedField() << "' to " - << applyParams.element.toString() + << updateNodeApplyParams.pathTaken->fieldRef().dottedField() + << "' to " << applyParams.element.toString() << " would modify the immutable field '" << (*immutablePath)->dottedField() << "'", - updateNodeApplyParams.pathTaken->commonPrefixSize(**immutablePath) != + updateNodeApplyParams.pathTaken->fieldRef().commonPrefixSize(**immutablePath) != (*immutablePath)->numParts()); } - invariant(!updateNodeApplyParams.pathToCreate->empty()); - FieldRef fullPath; + FieldRef fullPathFr; + std::vector<RuntimeUpdatePath::ComponentType> fullPathTypes; if (updateNodeApplyParams.pathTaken->empty()) { - fullPath = *updateNodeApplyParams.pathToCreate; + fullPathFr = *updateNodeApplyParams.pathToCreate; + // Newly created paths consist only of objects. + fullPathTypes.assign(fullPathFr.numParts(), + RuntimeUpdatePath::ComponentType::kFieldName); } else { - fullPath = - FieldRef(str::stream() << updateNodeApplyParams.pathTaken->dottedField() << "." - << updateNodeApplyParams.pathToCreate->dottedField()); + fullPathFr = + updateNodeApplyParams.pathTaken->fieldRef() + *updateNodeApplyParams.pathToCreate; + fullPathTypes = updateNodeApplyParams.pathTaken->types(); + const bool isCreatingArrayElem = applyParams.element.getType() == BSONType::Array; + + fullPathTypes.push_back(isCreatingArrayElem + ? RuntimeUpdatePath::ComponentType::kArrayIndex + : RuntimeUpdatePath::ComponentType::kFieldName); + for (auto i = 0; i < updateNodeApplyParams.pathToCreate->numParts() - 1; ++i) { + fullPathTypes.push_back(RuntimeUpdatePath::ComponentType::kFieldName); + } // If adding an element to an array, only mark the path to the array itself as modified. - if (applyParams.modifiedPaths && applyParams.element.getType() == BSONType::Array) { - applyParams.modifiedPaths->keepShortest(*updateNodeApplyParams.pathTaken); + if (applyParams.modifiedPaths && isCreatingArrayElem) { + applyParams.modifiedPaths->keepShortest( + updateNodeApplyParams.pathTaken->fieldRef()); } } + invariant(fullPathTypes.size() == fullPathFr.numParts()); ApplyResult applyResult; @@ -293,14 +313,19 @@ UpdateExecutor::ApplyResult ModifierNode::applyToNonexistentElement( // then we may need to add a null key to the index, even though "a.1.c" does not appear to // affect the index. if (!applyParams.indexData || - !applyParams.indexData->mightBeIndexed(applyParams.element.getType() != BSONType::Array - ? fullPath - : *updateNodeApplyParams.pathTaken)) { + !applyParams.indexData->mightBeIndexed( + applyParams.element.getType() != BSONType::Array + ? fullPathFr + : updateNodeApplyParams.pathTaken->fieldRef())) { applyResult.indexesAffected = false; } if (auto logBuilder = updateNodeApplyParams.logBuilder) { - logUpdate(logBuilder, fullPath.dottedField(), newElement, ModifyResult::kCreated); + logUpdate(logBuilder, + RuntimeUpdatePath(std::move(fullPathFr), std::move(fullPathTypes)), + newElement, + ModifyResult::kCreated, + updateNodeApplyParams.pathTaken->size()); } return applyResult; @@ -312,7 +337,7 @@ UpdateExecutor::ApplyResult ModifierNode::applyToNonexistentElement( // "non-viable," meaning it couldn't be created even if we intended to. UpdateLeafNode::checkViability(applyParams.element, *(updateNodeApplyParams.pathToCreate), - *(updateNodeApplyParams.pathTaken)); + (updateNodeApplyParams.pathTaken->fieldRef())); } return ApplyResult::noopResult(); @@ -331,7 +356,7 @@ UpdateExecutor::ApplyResult ModifierNode::apply(ApplyParams applyParams, } if (applyParams.modifiedPaths) { - applyParams.modifiedPaths->keepShortest(*updateNodeApplyParams.pathTaken + + applyParams.modifiedPaths->keepShortest(updateNodeApplyParams.pathTaken->fieldRef() + *updateNodeApplyParams.pathToCreate); } @@ -347,14 +372,20 @@ void ModifierNode::validateUpdate(mutablebson::ConstElement updatedElement, storage_validation::storageValid(updatedElement, doRecursiveCheck, recursionLevel); } -void ModifierNode::logUpdate(LogBuilder* logBuilder, - StringData pathTaken, +void ModifierNode::logUpdate(LogBuilderInterface* logBuilder, + const RuntimeUpdatePath& pathTaken, mutablebson::Element element, - ModifyResult modifyResult) const { + ModifyResult modifyResult, + boost::optional<int> createdFieldIdx) const { invariant(logBuilder); invariant(modifyResult == ModifyResult::kNormalUpdate || modifyResult == ModifyResult::kCreated); - uassertStatusOK(logBuilder->addToSetsWithNewFieldName(pathTaken, element)); + if (modifyResult == ModifyResult::kCreated) { + invariant(createdFieldIdx); + uassertStatusOK(logBuilder->logCreatedField(pathTaken, *createdFieldIdx, element)); + } else { + uassertStatusOK(logBuilder->logUpdatedField(pathTaken, element)); + } } } // namespace mongo diff --git a/src/mongo/db/update/modifier_node.h b/src/mongo/db/update/modifier_node.h index 4c26735400d..d9063768151 100644 --- a/src/mongo/db/update/modifier_node.h +++ b/src/mongo/db/update/modifier_node.h @@ -84,7 +84,7 @@ protected: * what kind of update was performed in its return value. */ virtual ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const = 0; + const FieldRef& elementPath) const = 0; /** * ModifierNode::apply() calls this method when applying an update to a path that does not yet @@ -135,11 +135,15 @@ protected: * setValueForNewElement(). * - 'modifyResult' is either the value returned by updateExistingElement() or the value * ModifyResult::kCreated. + * - 'createdFieldIdx' indicates what the first component in 'pathTaken' is that was created as + * part of this update. If the update did not add any new fields, boost::none should be + * provided. */ - virtual void logUpdate(LogBuilder* logBuilder, - StringData pathTaken, + virtual void logUpdate(LogBuilderInterface* logBuilder, + const RuntimeUpdatePath& pathTaken, mutablebson::Element element, - ModifyResult modifyResult) const; + ModifyResult modifyResult, + boost::optional<int> createdFieldIdx) const; /** * ModifierNode::apply() calls this method to determine what to do when applying an update to a diff --git a/src/mongo/db/update/pop_node.cpp b/src/mongo/db/update/pop_node.cpp index 3d4355793f1..dd18e8e768b 100644 --- a/src/mongo/db/update/pop_node.cpp +++ b/src/mongo/db/update/pop_node.cpp @@ -48,11 +48,11 @@ Status PopNode::init(BSONElement modExpr, const boost::intrusive_ptr<ExpressionC return Status::OK(); } -ModifierNode::ModifyResult PopNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { +ModifierNode::ModifyResult PopNode::updateExistingElement(mutablebson::Element* element, + const FieldRef& elementPath) const { invariant(element->ok()); uassert(ErrorCodes::TypeMismatch, - str::stream() << "Path '" << elementPath->dottedField() + str::stream() << "Path '" << elementPath.dottedField() << "' contains an element of non-array type '" << typeName(element->getType()) << "'", element->getType() == BSONType::Array); diff --git a/src/mongo/db/update/pop_node.h b/src/mongo/db/update/pop_node.h index e731a1956fb..3f0f8a55f15 100644 --- a/src/mongo/db/update/pop_node.h +++ b/src/mongo/db/update/pop_node.h @@ -41,7 +41,7 @@ public: Status init(BSONElement modExpr, const boost::intrusive_ptr<ExpressionContext>& expCtx) final; ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void validateUpdate(mutablebson::ConstElement updatedElement, mutablebson::ConstElement leftSibling, diff --git a/src/mongo/db/update/pop_node_test.cpp b/src/mongo/db/update/pop_node_test.cpp index d27931dde80..1349670365c 100644 --- a/src/mongo/db/update/pop_node_test.cpp +++ b/src/mongo/db/update/pop_node_test.cpp @@ -127,7 +127,7 @@ TEST_F(PopNodeTest, NoopWhenPathPartiallyExists) { mmb::Document doc(fromjson("{a: {}}")); setPathToCreate("b.c"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.b.c"); auto result = popNode.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -145,7 +145,7 @@ TEST_F(PopNodeTest, NoopWhenNumericalPathComponentExceedsArrayLength) { mmb::Document doc(fromjson("{a: []}")); setPathToCreate("0"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.0"); auto result = popNode.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -163,7 +163,7 @@ TEST_F(PopNodeTest, ThrowsWhenPathIsBlockedByAScalar) { mmb::Document doc(fromjson("{a: 'foo'}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( popNode.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -181,7 +181,7 @@ DEATH_TEST_REGEX_F(PopNodeTest, ASSERT_OK(popNode.init(update["$pop"]["a.b"], expCtx)); mmb::Document doc(fromjson("{a: {b: [1, 2, 3]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); popNode.apply(getApplyParams(doc.end()), getUpdateNodeApplyParams()); } @@ -193,7 +193,7 @@ TEST_F(PopNodeTest, ThrowsWhenPathExistsButDoesNotContainAnArray) { ASSERT_OK(popNode.init(update["$pop"]["a.b"], expCtx)); mmb::Document doc(fromjson("{a: {b: 'foo'}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()), @@ -209,7 +209,7 @@ TEST_F(PopNodeTest, NoopWhenPathContainsAnEmptyArray) { ASSERT_OK(popNode.init(update["$pop"]["a.b"], expCtx)); mmb::Document doc(fromjson("{a: {b: []}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -227,7 +227,7 @@ TEST_F(PopNodeTest, PopsSingleElementFromTheBack) { ASSERT_FALSE(popNode.popFromFront()); mmb::Document doc(fromjson("{a: {b: [1]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -245,7 +245,7 @@ TEST_F(PopNodeTest, PopsSingleElementFromTheFront) { ASSERT_TRUE(popNode.popFromFront()); mmb::Document doc(fromjson("{a: {b: [[1]]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a"); auto result = popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -263,7 +263,7 @@ TEST_F(PopNodeTest, PopsFromTheBackOfMultiElementArray) { ASSERT_FALSE(popNode.popFromFront()); mmb::Document doc(fromjson("{a: {b: [1, 2, 3]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b.c"); auto result = popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -281,7 +281,7 @@ TEST_F(PopNodeTest, PopsFromTheFrontOfMultiElementArray) { ASSERT_TRUE(popNode.popFromFront()); mmb::Document doc(fromjson("{a: {b: [1, 2, 3]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -299,7 +299,7 @@ TEST_F(PopNodeTest, PopsFromTheFrontOfMultiElementArrayWithoutAffectingIndexes) ASSERT_TRUE(popNode.popFromFront()); mmb::Document doc(fromjson("{a: {b: [1, 2, 3]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("unrelated.path"); auto result = popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -317,7 +317,7 @@ TEST_F(PopNodeTest, SucceedsWithNullUpdateIndexData) { ASSERT_FALSE(popNode.popFromFront()); mmb::Document doc(fromjson("{a: {b: [1, 2, 3]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); auto result = popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -334,7 +334,7 @@ TEST_F(PopNodeTest, SucceedsWithNullLogBuilder) { ASSERT_FALSE(popNode.popFromFront()); mmb::Document doc(fromjson("{a: {b: [1, 2, 3]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b.c"); setLogBuilderToNull(); auto result = popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); @@ -351,7 +351,7 @@ TEST_F(PopNodeTest, ThrowsWhenPathIsImmutable) { ASSERT_OK(popNode.init(update["$pop"]["a.b"], expCtx)); mmb::Document doc(fromjson("{a: {b: [0]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addImmutablePath("a.b"); addIndexedPath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( @@ -373,7 +373,7 @@ TEST_F(PopNodeTest, ThrowsWhenPathIsPrefixOfImmutable) { ASSERT_OK(popNode.init(update["$pop"]["a"], expCtx)); mmb::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addImmutablePath("a.0"); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( @@ -390,7 +390,7 @@ TEST_F(PopNodeTest, ThrowsWhenPathIsSuffixOfImmutable) { ASSERT_OK(popNode.init(update["$pop"]["a.b"], expCtx)); mmb::Document doc(fromjson("{a: {b: [0]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addImmutablePath("a"); addIndexedPath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( @@ -407,7 +407,7 @@ TEST_F(PopNodeTest, NoopOnImmutablePathSucceeds) { ASSERT_OK(popNode.init(update["$pop"]["a.b"], expCtx)); mmb::Document doc(fromjson("{a: {b: []}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addImmutablePath("a.b"); addIndexedPath("a.b"); auto result = popNode.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); diff --git a/src/mongo/db/update/pull_node_test.cpp b/src/mongo/db/update/pull_node_test.cpp index bca1b93a18f..4a178b0709d 100644 --- a/src/mongo/db/update/pull_node_test.cpp +++ b/src/mongo/db/update/pull_node_test.cpp @@ -154,7 +154,7 @@ TEST_F(PullNodeTest, ApplyToStringFails) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 'foo'}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -170,7 +170,7 @@ TEST_F(PullNodeTest, ApplyToObjectFails) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {foo: 'bar'}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -187,7 +187,7 @@ TEST_F(PullNodeTest, ApplyToNonViablePathFails) { mutablebson::Document doc(fromjson("{a: 1}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -204,7 +204,7 @@ TEST_F(PullNodeTest, ApplyToMissingElement) { mutablebson::Document doc(fromjson("{a: {b: {c: {}}}}")); setPathToCreate("d"); - setPathTaken("a.b.c"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b.c")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]["c"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -221,7 +221,7 @@ TEST_F(PullNodeTest, ApplyToEmptyArray) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -238,7 +238,7 @@ TEST_F(PullNodeTest, ApplyToArrayMatchingNone) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [2, 3, 4, 5]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -255,7 +255,7 @@ TEST_F(PullNodeTest, ApplyToArrayMatchingOne) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2, 3]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -272,7 +272,7 @@ TEST_F(PullNodeTest, ApplyToArrayMatchingSeveral) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 0, 2, 0, 3, 0, 4, 0, 5]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -289,7 +289,7 @@ TEST_F(PullNodeTest, ApplyToArrayMatchingAll) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, -1, -2, -3, -4, -5]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -306,7 +306,7 @@ TEST_F(PullNodeTest, ApplyNoIndexDataNoLogBuilder) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2, 3]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -327,7 +327,7 @@ TEST_F(PullNodeTest, ApplyWithCollation) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: ['zaa', 'zcc', 'zbb', 'zee']}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -347,7 +347,7 @@ TEST_F(PullNodeTest, ApplyWithCollationDoesNotAffectNonStringMatches) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [2, 1, 0, -1, -2, -3]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -367,7 +367,7 @@ TEST_F(PullNodeTest, ApplyWithCollationDoesNotAffectRegexMatches) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: ['b', 'a', 'aab', 'cb', 'bba']}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -387,7 +387,7 @@ TEST_F(PullNodeTest, ApplyStringLiteralMatchWithCollation) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: ['b', 'a', 'aab', 'cb', 'bba']}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -407,7 +407,7 @@ TEST_F(PullNodeTest, ApplyCollationDoesNotAffectNumberLiteralMatches) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: ['a', 99, 'b', 2, 'c', 99, 'd']}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -425,7 +425,7 @@ TEST_F(PullNodeTest, ApplyStringMatchAfterSetCollator) { // First without a collator. mutablebson::Document doc(fromjson("{ a : ['a', 'b', 'c', 'd'] }")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -437,7 +437,7 @@ TEST_F(PullNodeTest, ApplyStringMatchAfterSetCollator) { node.setCollator(&mockCollator); mutablebson::Document doc2(fromjson("{ a : ['a', 'b', 'c', 'd'] }")); resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); result = node.apply(getApplyParams(doc2.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -453,7 +453,7 @@ TEST_F(PullNodeTest, ApplyElementMatchAfterSetCollator) { // First without a collator. mutablebson::Document doc(fromjson("{ a : ['a', 'b', 'c', 'd'] }")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -465,7 +465,7 @@ TEST_F(PullNodeTest, ApplyElementMatchAfterSetCollator) { node.setCollator(&mockCollator); mutablebson::Document doc2(fromjson("{ a : ['a', 'b', 'c', 'd'] }")); resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); result = node.apply(getApplyParams(doc2.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -481,7 +481,7 @@ TEST_F(PullNodeTest, ApplyObjectMatchAfterSetCollator) { // First without a collator. mutablebson::Document doc(fromjson("{a : [{b: 'w'}, {b: 'x'}, {b: 'y'}, {b: 'z'}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -493,7 +493,7 @@ TEST_F(PullNodeTest, ApplyObjectMatchAfterSetCollator) { node.setCollator(&mockCollator); mutablebson::Document doc2(fromjson("{a : [{b: 'w'}, {b: 'x'}, {b: 'y'}, {b: 'z'}]}")); resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); result = node.apply(getApplyParams(doc2.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -514,7 +514,7 @@ TEST_F(PullNodeTest, SetCollatorDoesNotAffectClone) { // The original node should now have collation. mutablebson::Document doc(fromjson("{ a : ['a', 'b', 'c', 'd'] }")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -524,7 +524,7 @@ TEST_F(PullNodeTest, SetCollatorDoesNotAffectClone) { // The clone should have exact string matches (no collation). mutablebson::Document doc2(fromjson("{ a : ['a', 'b', 'c', 'd'] }")); resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); result = cloneNode->apply(getApplyParams(doc2.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); @@ -543,7 +543,7 @@ TEST_F(PullNodeTest, ApplyComplexDocAndMatching1) { ASSERT_OK(node.init(update["$pull"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: [{x: 1}, {y: 'y'}, {x: 2}, {z: 'z'}]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -560,7 +560,7 @@ TEST_F(PullNodeTest, ApplyComplexDocAndMatching2) { ASSERT_OK(node.init(update["$pull"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: [{x: 1}, {y: 'y'}, {x: 2}, {z: 'z'}]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -577,7 +577,7 @@ TEST_F(PullNodeTest, ApplyComplexDocAndMatching3) { ASSERT_OK(node.init(update["$pull"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: [{x: 1}, {y: 'y'}, {x: 2}, {z: 'z'}]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -598,7 +598,7 @@ TEST_F(PullNodeTest, ApplyFullPredicateWithCollation) { mutablebson::Document doc( fromjson("{a: {b: [{x: 'foo', y: 1}, {x: 'bar', y: 2}, {x: 'baz', y: 3}]}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -615,7 +615,7 @@ TEST_F(PullNodeTest, ApplyScalarValueMod) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [1, 2, 1, 2, 1, 2]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -632,7 +632,7 @@ TEST_F(PullNodeTest, ApplyObjectValueMod) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [{x: 1}, {y: 2}, {x: 1}, {y: 2}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -650,7 +650,7 @@ TEST_F(PullNodeTest, DocumentationExample1) { mutablebson::Document doc( fromjson("{flags: ['vme', 'de', 'pse', 'tsc', 'msr', 'pae', 'mce']}")); - setPathTaken("flags"); + setPathTaken(makeRuntimeUpdatePathForTest("flags")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["flags"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -668,7 +668,7 @@ TEST_F(PullNodeTest, DocumentationExample2a) { ASSERT_OK(node.init(update["$pull"]["votes"], expCtx)); mutablebson::Document doc(fromjson("{votes: [3, 5, 6, 7, 7, 8]}")); - setPathTaken("votes"); + setPathTaken(makeRuntimeUpdatePathForTest("votes")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["votes"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -685,7 +685,7 @@ TEST_F(PullNodeTest, DocumentationExample2b) { ASSERT_OK(node.init(update["$pull"]["votes"], expCtx)); mutablebson::Document doc(fromjson("{votes: [3, 5, 6, 7, 7, 8]}")); - setPathTaken("votes"); + setPathTaken(makeRuntimeUpdatePathForTest("votes")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["votes"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -702,7 +702,7 @@ TEST_F(PullNodeTest, ApplyPullWithObjectValueToArrayWithNonObjectValue) { ASSERT_OK(node.init(update["$pull"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [{x: 1}, 2]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -719,7 +719,7 @@ TEST_F(PullNodeTest, CannotModifyImmutableField) { ASSERT_OK(node.init(update["$pull"]["_id.a"], expCtx)); mutablebson::Document doc(fromjson("{_id: {a: [0, 1, 2]}}")); - setPathTaken("_id.a"); + setPathTaken(makeRuntimeUpdatePathForTest("_id.a")); addImmutablePath("_id"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["_id"]["a"]), getUpdateNodeApplyParams()), @@ -735,7 +735,7 @@ TEST_F(PullNodeTest, SERVER_3988) { ASSERT_OK(node.init(update["$pull"]["y"], expCtx)); mutablebson::Document doc(fromjson("{x: 1, y: [2, 3, 4, 'abc', 'xyz']}")); - setPathTaken("y"); + setPathTaken(makeRuntimeUpdatePathForTest("y")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["y"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); diff --git a/src/mongo/db/update/pullall_node_test.cpp b/src/mongo/db/update/pullall_node_test.cpp index 6ca896d45b0..8309b96e8aa 100644 --- a/src/mongo/db/update/pullall_node_test.cpp +++ b/src/mongo/db/update/pullall_node_test.cpp @@ -108,7 +108,7 @@ TEST_F(PullAllNodeTest, TargetArrayElementNotFound) { mutablebson::Document doc(fromjson("{a: [1, 2]}")); setPathToCreate("2"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -125,7 +125,7 @@ TEST_F(PullAllNodeTest, ApplyToNonArrayFails) { ASSERT_OK(node.init(update["$pullAll"]["a.0"], expCtx)); mutablebson::Document doc(fromjson("{a: [1, 2]}")); - setPathTaken("a.0"); + setPathTaken(makeRuntimeUpdatePathForTest("a.0")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"][0]), getUpdateNodeApplyParams()), @@ -141,7 +141,7 @@ TEST_F(PullAllNodeTest, ApplyWithSingleNumber) { ASSERT_OK(node.init(update["$pullAll"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [1, 'a', {r: 1, b: 2}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -158,7 +158,7 @@ TEST_F(PullAllNodeTest, ApplyNoIndexDataNoLogBuilder) { ASSERT_OK(node.init(update["$pullAll"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [1, 'a', {r: 1, b: 2}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -174,7 +174,7 @@ TEST_F(PullAllNodeTest, ApplyWithElementNotPresentInArray) { ASSERT_OK(node.init(update["$pullAll"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [1, 'a', {r: 1, b: 2}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -191,7 +191,7 @@ TEST_F(PullAllNodeTest, ApplyWithWithTwoElements) { ASSERT_OK(node.init(update["$pullAll"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [1, 'a', {r: 1, b: 2}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -208,7 +208,7 @@ TEST_F(PullAllNodeTest, ApplyWithAllArrayElements) { ASSERT_OK(node.init(update["$pullAll"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [1, 'a', {r: 1, b: 2}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -225,7 +225,7 @@ TEST_F(PullAllNodeTest, ApplyWithAllArrayElementsButOutOfOrder) { ASSERT_OK(node.init(update["$pullAll"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [1, 'a', {r: 1, b: 2}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -242,7 +242,7 @@ TEST_F(PullAllNodeTest, ApplyWithAllArrayElementsAndThenSome) { ASSERT_OK(node.init(update["$pullAll"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [1, 'a', {r: 1, b: 2}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -262,7 +262,7 @@ TEST_F(PullAllNodeTest, ApplyWithCollator) { ASSERT_OK(node.init(update["$pullAll"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: ['foo', 'bar', 'baz']}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -280,7 +280,7 @@ TEST_F(PullAllNodeTest, ApplyAfterSetCollator) { // First without a collator. mutablebson::Document doc(fromjson("{a: ['foo', 'bar', 'baz']}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); ASSERT_EQUALS(fromjson("{a: ['foo', 'bar', 'baz']}"), doc); @@ -291,7 +291,7 @@ TEST_F(PullAllNodeTest, ApplyAfterSetCollator) { node.setCollator(&mockCollator); mutablebson::Document doc2(fromjson("{a: ['foo', 'bar', 'baz']}")); resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); result = node.apply(getApplyParams(doc2.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); ASSERT_FALSE(result.indexesAffected); diff --git a/src/mongo/db/update/push_node.cpp b/src/mongo/db/update/push_node.cpp index a4a79fb6e5a..93769c666f3 100644 --- a/src/mongo/db/update/push_node.cpp +++ b/src/mongo/db/update/push_node.cpp @@ -285,7 +285,7 @@ ModifierNode::ModifyResult PushNode::insertElementsWithPosition( } ModifierNode::ModifyResult PushNode::performPush(mutablebson::Element* element, - FieldRef* elementPath) const { + const FieldRef* elementPath) const { if (element->getType() != BSONType::Array) { invariant(elementPath); // We can only hit this error if we are updating an existing path. auto idElem = mutablebson::findFirstChildNamed(element->getDocument().root(), "_id"); @@ -321,32 +321,41 @@ ModifierNode::ModifyResult PushNode::performPush(mutablebson::Element* element, return result; } -ModifierNode::ModifyResult PushNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { - return performPush(element, elementPath.get()); +ModifierNode::ModifyResult PushNode::updateExistingElement(mutablebson::Element* element, + const FieldRef& elementPath) const { + return performPush(element, &elementPath); } -void PushNode::logUpdate(LogBuilder* logBuilder, - StringData pathTaken, +void PushNode::logUpdate(LogBuilderInterface* logBuilder, + const RuntimeUpdatePath& pathTaken, mutablebson::Element element, - ModifyResult modifyResult) const { + ModifyResult modifyResult, + boost::optional<int> createdFieldIdx) const { invariant(logBuilder); - if (modifyResult == ModifyResult::kNormalUpdate || modifyResult == ModifyResult::kCreated) { - // Simple case: log the entires contents of the updated array. - uassertStatusOK(logBuilder->addToSetsWithNewFieldName(pathTaken, element)); + if (modifyResult == ModifyResult::kNormalUpdate) { + uassertStatusOK(logBuilder->logUpdatedField(pathTaken, element)); + } else if (modifyResult == ModifyResult::kCreated) { + invariant(createdFieldIdx); + uassertStatusOK(logBuilder->logCreatedField(pathTaken, *createdFieldIdx, element)); } else if (modifyResult == ModifyResult::kArrayAppendUpdate) { // This update only modified the array by appending entries to the end. Rather than writing // out the entire contents of the array, we create oplog entries for the newly appended // elements. - auto numAppended = _valuesToPush.size(); - auto arraySize = countChildren(element); + const auto numAppended = _valuesToPush.size(); + const auto arraySize = countChildren(element); + // We have to copy the field ref provided in order to use RuntimeUpdatePathTempAppend. + RuntimeUpdatePath pathTakenCopy = pathTaken; invariant(arraySize > numAppended); auto position = arraySize - numAppended; for (const auto& valueToLog : _valuesToPush) { - std::string pathToArrayElement(str::stream() << pathTaken << "." << position); - uassertStatusOK(logBuilder->addToSetsWithNewFieldName(pathToArrayElement, valueToLog)); + const std::string positionAsString = std::to_string(position); + + RuntimeUpdatePathTempAppend tempAppend( + pathTakenCopy, positionAsString, RuntimeUpdatePath::ComponentType::kArrayIndex); + uassertStatusOK( + logBuilder->logCreatedField(pathTakenCopy, pathTakenCopy.size() - 1, valueToLog)); ++position; } diff --git a/src/mongo/db/update/push_node.h b/src/mongo/db/update/push_node.h index 33677ae6652..38b4b00bc66 100644 --- a/src/mongo/db/update/push_node.h +++ b/src/mongo/db/update/push_node.h @@ -61,12 +61,13 @@ public: protected: ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void setValueForNewElement(mutablebson::Element* element) const final; - void logUpdate(LogBuilder* logBuilder, - StringData pathTaken, + void logUpdate(LogBuilderInterface* logBuilder, + const RuntimeUpdatePath& pathTaken, mutablebson::Element element, - ModifyResult modifyResult) const final; + ModifyResult modifyResult, + boost::optional<int> createdFieldIdx) const final; bool allowCreation() const final { return true; @@ -99,7 +100,7 @@ private: * inserted at the beginning or in the middle of the array, or a slice or sort gets * performed. */ - ModifyResult performPush(mutablebson::Element* element, FieldRef* elementPath) const; + ModifyResult performPush(mutablebson::Element* element, const FieldRef* elementPath) const; static const StringData kEachClauseName; static const StringData kSliceClauseName; diff --git a/src/mongo/db/update/push_node_test.cpp b/src/mongo/db/update/push_node_test.cpp index b6fa5e59aaa..92e81014846 100644 --- a/src/mongo/db/update/push_node_test.cpp +++ b/src/mongo/db/update/push_node_test.cpp @@ -264,7 +264,7 @@ TEST_F(PushNodeTest, ApplyToNonArrayFails) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{_id: 'test_object', a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -280,7 +280,7 @@ TEST_F(PushNodeTest, ApplyToEmptyArray) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -316,7 +316,7 @@ TEST_F(PushNodeTest, ApplyToArrayWithOneElement) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -340,7 +340,7 @@ TEST_F(PushNodeTest, ApplyToDottedPathElement) { " second: { choice: 'c'}}" "}")); setPathToCreate("votes"); - setPathTaken("choices.first"); + setPathTaken(makeRuntimeUpdatePathForTest("choices.first")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["choices"]["first"]), getUpdateNodeApplyParams()); @@ -364,7 +364,7 @@ TEST_F(PushNodeTest, ApplySimpleEachToEmptyArray) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -418,7 +418,7 @@ TEST_F(PushNodeTest, ApplySimpleEachToArrayWithOneElement) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -436,7 +436,7 @@ TEST_F(PushNodeTest, ApplyMultipleEachToArrayWithOneElement) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -454,7 +454,7 @@ TEST_F(PushNodeTest, ApplyEmptyEachToEmptyArray) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -490,7 +490,7 @@ TEST_F(PushNodeTest, ApplyEmptyEachToArrayWithOneElement) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -508,7 +508,7 @@ TEST_F(PushNodeTest, ApplyToArrayWithSlice) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [3]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -526,7 +526,7 @@ TEST_F(PushNodeTest, ApplyWithNumericSort) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [3]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -544,7 +544,7 @@ TEST_F(PushNodeTest, ApplyWithReverseNumericSort) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [3]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -562,7 +562,7 @@ TEST_F(PushNodeTest, ApplyWithMixedSort) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [3, 't', {b: 1}, {a: 1}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -580,7 +580,7 @@ TEST_F(PushNodeTest, ApplyWithReverseMixedSort) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [3, 't', {b: 1}, {a: 1}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -598,7 +598,7 @@ TEST_F(PushNodeTest, ApplyWithEmbeddedFieldSort) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [3, 't', {b: 1}, {a: 1}]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -619,7 +619,7 @@ TEST_F(PushNodeTest, ApplySortWithCollator) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: ['dd', 'fc', 'gb']}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -637,7 +637,7 @@ TEST_F(PushNodeTest, ApplySortAfterSetCollator) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: ['dd', 'fc', 'gb']}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -647,7 +647,7 @@ TEST_F(PushNodeTest, ApplySortAfterSetCollator) { // Now with a collator. resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); CollatorInterfaceMock mockCollator(CollatorInterfaceMock::MockType::kReverseString); node.setCollator(&mockCollator); @@ -701,7 +701,7 @@ TEST_F(PushNodeTest, ApplyToEmptyArrayWithSliceValues) { mutablebson::Document doc(fromjson("{a: []}")); resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); checkDocumentAndResult(update, data.resultingDoc, doc, result); @@ -734,7 +734,7 @@ TEST_F(PushNodeTest, ApplyToPopulatedArrayWithSliceValues) { mutablebson::Document doc(fromjson("{a: [2, 3]}")); resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); checkDocumentAndResult(update, data.resultingDoc, doc, result); @@ -834,7 +834,7 @@ TEST_F(PushNodeTest, ApplyToPopulatedArrayWithSortAndSliceValues) { mutablebson::Document doc(fromjson("{a: [{a: 2, b: 3}, {a: 3, b: 1}]}")); resetApplyParams(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); checkDocumentAndResult(update, data.resultingDoc, doc, result); @@ -848,7 +848,7 @@ TEST_F(PushNodeTest, ApplyToEmptyArrayWithPositionZero) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -866,7 +866,7 @@ TEST_F(PushNodeTest, ApplyToEmptyArrayWithPositionOne) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -884,7 +884,7 @@ TEST_F(PushNodeTest, ApplyToEmptyArrayWithLargePosition) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -902,7 +902,7 @@ TEST_F(PushNodeTest, ApplyToSingletonArrayWithPositionZero) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -920,7 +920,7 @@ TEST_F(PushNodeTest, ApplyToSingletonArrayWithLargePosition) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -938,7 +938,7 @@ TEST_F(PushNodeTest, ApplyToEmptyArrayWithNegativePosition) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: []}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -956,7 +956,7 @@ TEST_F(PushNodeTest, ApplyToSingletonArrayWithNegativePosition) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -974,7 +974,7 @@ TEST_F(PushNodeTest, ApplyToPopulatedArrayWithNegativePosition) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2, 3, 4]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -992,7 +992,7 @@ TEST_F(PushNodeTest, ApplyToPopulatedArrayWithOutOfBoundsNegativePosition) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2, 3, 4]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -1010,7 +1010,7 @@ TEST_F(PushNodeTest, ApplyMultipleElementsPushWithNegativePosition) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2, 3, 4]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -1030,7 +1030,7 @@ TEST_F(PushNodeTest, PushWithMinIntAsPosition) { ASSERT_OK(node.init(update["$push"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2, 3, 4]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); diff --git a/src/mongo/db/update/rename_node.cpp b/src/mongo/db/update/rename_node.cpp index 6f9119d41ac..7537925851c 100644 --- a/src/mongo/db/update/rename_node.cpp +++ b/src/mongo/db/update/rename_node.cpp @@ -81,8 +81,8 @@ public: } protected: - ModifierNode::ModifyResult updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const final { + ModifierNode::ModifyResult updateExistingElement(mutablebson::Element* element, + const FieldRef& elementPath) const final { invariant(element->setValueElement(_elemToSet)); return ModifyResult::kNormalUpdate; } @@ -166,7 +166,7 @@ UpdateExecutor::ApplyResult RenameNode::apply(ApplyParams applyParams, UpdateNodeApplyParams updateNodeApplyParams) const { // It would make sense to store fromFieldRef and toFieldRef as members during // RenameNode::init(), but FieldRef is not copyable. - auto fromFieldRef = std::make_shared<FieldRef>(_val.fieldName()); + FieldRef fromFieldRef(_val.fieldName()); FieldRef toFieldRef(_val.valueStringData()); mutablebson::Document& document = applyParams.element.getDocument(); @@ -174,9 +174,9 @@ UpdateExecutor::ApplyResult RenameNode::apply(ApplyParams applyParams, FieldIndex fromIdxFound; mutablebson::Element fromElement(document.end()); auto status = - pathsupport::findLongestPrefix(*fromFieldRef, document.root(), &fromIdxFound, &fromElement); + pathsupport::findLongestPrefix(fromFieldRef, document.root(), &fromIdxFound, &fromElement); - if (!status.isOK() || !fromElement.ok() || fromIdxFound != (fromFieldRef->numParts() - 1)) { + if (!status.isOK() || !fromElement.ok() || fromIdxFound != (fromFieldRef.numParts() - 1)) { // We could safely remove this restriction (thereby treating a rename with a non-viable // source path as a no-op), but most updates fail on an attempt to update a non-viable path, // so we throw an error for consistency. @@ -188,7 +188,7 @@ UpdateExecutor::ApplyResult RenameNode::apply(ApplyParams applyParams, // The element we want to rename does not exist. When that happens, we treat the operation // as a no-op. The attempted from/to paths are still considered modified. if (applyParams.modifiedPaths) { - applyParams.modifiedPaths->keepShortest(*fromFieldRef); + applyParams.modifiedPaths->keepShortest(fromFieldRef); applyParams.modifiedPaths->keepShortest(toFieldRef); } return ApplyResult::noopResult(); @@ -203,7 +203,7 @@ UpdateExecutor::ApplyResult RenameNode::apply(ApplyParams applyParams, auto idElem = mutablebson::findFirstChildNamed(document.root(), "_id"); uasserted(ErrorCodes::BadValue, str::stream() << "The source field cannot be an array element, '" - << fromFieldRef->dottedField() << "' in doc with " + << fromFieldRef.dottedField() << "' in doc with " << (idElem.ok() ? idElem.toString() : "no id") << " has an array field called '" << currentElement.getFieldName() << "'"); @@ -240,8 +240,18 @@ UpdateExecutor::ApplyResult RenameNode::apply(ApplyParams applyParams, ApplyParams unsetParams(applyParams); unsetParams.element = fromElement; + // Renames never "go through" arrays, so we're guaranteed all of the parts of the path are + // of type kFieldName. + auto pathTaken = std::make_shared<RuntimeUpdatePath>( + fromFieldRef, + std::vector<RuntimeUpdatePath::ComponentType>( + fromFieldRef.numParts(), RuntimeUpdatePath::ComponentType::kFieldName)); + UpdateNodeApplyParams unsetUpdateNodeApplyParams{ - std::make_shared<FieldRef>(), fromFieldRef, updateNodeApplyParams.logBuilder}; + std::make_shared<FieldRef>(), + pathTaken, + updateNodeApplyParams.logBuilder, + }; UnsetNode unsetElement; auto unsetElementApplyResult = unsetElement.apply(unsetParams, unsetUpdateNodeApplyParams); diff --git a/src/mongo/db/update/rename_node_test.cpp b/src/mongo/db/update/rename_node_test.cpp index 6eec4d8f498..bc52481b036 100644 --- a/src/mongo/db/update/rename_node_test.cpp +++ b/src/mongo/db/update/rename_node_test.cpp @@ -134,7 +134,7 @@ TEST_F(RenameNodeTest, ToExistsAtSameLevel) { ASSERT_OK(node.init(update["$rename"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 2, b: 1}")); - setPathTaken("b"); + setPathTaken(makeRuntimeUpdatePathForTest("b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -151,7 +151,7 @@ TEST_F(RenameNodeTest, ToAndFromHaveSameValue) { ASSERT_OK(node.init(update["$rename"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 2, b: 2}")); - setPathTaken("b"); + setPathTaken(makeRuntimeUpdatePathForTest("b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -168,7 +168,7 @@ TEST_F(RenameNodeTest, RenameToFieldWithSameValueButDifferentType) { ASSERT_OK(node.init(update["$rename"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1, b: NumberLong(1)}")); - setPathTaken("b"); + setPathTaken(makeRuntimeUpdatePathForTest("b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -185,7 +185,7 @@ TEST_F(RenameNodeTest, FromDottedElement) { ASSERT_OK(node.init(update["$rename"]["a.c"], expCtx)); mutablebson::Document doc(fromjson("{a: {c: {d: 6}}, b: 1}")); - setPathTaken("b"); + setPathTaken(makeRuntimeUpdatePathForTest("b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -202,7 +202,7 @@ TEST_F(RenameNodeTest, RenameToExistingNestedFieldDoesNotReorderFields) { ASSERT_OK(node.init(update["$rename"]["c.d"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: {c: 1, d: 2}}, b: 3, c: {d: 4}}")); - setPathTaken("a.b.c"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b.c")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]["c"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -220,7 +220,7 @@ TEST_F(RenameNodeTest, MissingCompleteTo) { mutablebson::Document doc(fromjson("{a: 2, b: 1, c: {}}")); setPathToCreate("r.d"); - setPathTaken("c"); + setPathTaken(makeRuntimeUpdatePathForTest("c")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["c"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -272,7 +272,7 @@ TEST_F(RenameNodeTest, MoveIntoArray) { mutablebson::Document doc(fromjson("{_id: 'test_object', a: [1, 2], b: 2}")); setPathToCreate("2"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -290,7 +290,7 @@ TEST_F(RenameNodeTest, MoveIntoArrayNoId) { mutablebson::Document doc(fromjson("{a: [1, 2], b: 2}")); setPathToCreate("2"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -307,7 +307,7 @@ TEST_F(RenameNodeTest, MoveToArrayElement) { ASSERT_OK(node.init(update["$rename"]["b"], expCtx)); mutablebson::Document doc(fromjson("{_id: 'test_object', a: [1, 2], b: 2}")); - setPathTaken("a.1"); + setPathTaken(makeRuntimeUpdatePathForTest("a.1")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"][1]), getUpdateNodeApplyParams()), @@ -372,7 +372,7 @@ TEST_F(RenameNodeTest, ReplaceArrayField) { ASSERT_OK(node.init(update["$rename"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 2, b: []}")); - setPathTaken("b"); + setPathTaken(makeRuntimeUpdatePathForTest("b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -389,7 +389,7 @@ TEST_F(RenameNodeTest, ReplaceWithArrayField) { ASSERT_OK(node.init(update["$rename"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [], b: 2}")); - setPathTaken("b"); + setPathTaken(makeRuntimeUpdatePathForTest("b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -437,7 +437,7 @@ TEST_F(RenameNodeTest, RenameFromNonExistentPathIsNoOp) { ASSERT_OK(node.init(update["$rename"]["a"], expCtx)); mutablebson::Document doc(fromjson("{b: 2}")); - setPathTaken("b"); + setPathTaken(makeRuntimeUpdatePathForTest("b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["b"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -572,7 +572,7 @@ TEST_F(RenameNodeTest, ApplyCannotOverwriteImmutablePath) { ASSERT_OK(node.init(update["$rename"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 0, b: 1}")); - setPathTaken("b"); + setPathTaken(makeRuntimeUpdatePathForTest("b")); addImmutablePath("b"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["b"]), getUpdateNodeApplyParams()), diff --git a/src/mongo/db/update/runtime_update_path.h b/src/mongo/db/update/runtime_update_path.h new file mode 100644 index 00000000000..529496834cc --- /dev/null +++ b/src/mongo/db/update/runtime_update_path.h @@ -0,0 +1,134 @@ +/** + * Copyright (C) 2020-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * 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 + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * 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 Server Side 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 <vector> + +#include "mongo/db/field_ref.h" + +namespace mongo { +/** + * This class represents a path which can be taken through a specific document. In addition to the + * path components, which are stored as a FieldRef, RuntimeUpdatePath also stores the "type" of + * each component, which can be either kFieldName or kArrayIndex. Non-numeric components are always + * of type kFieldName. Numeric components may either be field names (in the case of an object like + * {"123": "foo"}) or array indexes. + * + * Unlike other path representations in our codebase, RuntimeUpdatePath is tied to a particular + * document. It does not represent a path that a user provides as part of an update. + * + * Example: + * Document: {a: {0: {b: ["foo", "bar"]}}}. + * + * The path to "bar" would be represented as "a.0.b.1" where "a" "0" and "b" are of type kFieldName + * while "1" is of type kArrayIndex. + */ +class RuntimeUpdatePath { +public: + enum ComponentType { kFieldName, kArrayIndex }; + using ComponentTypeVector = std::vector<ComponentType>; + + RuntimeUpdatePath() = default; + RuntimeUpdatePath(FieldRef fr, ComponentTypeVector types) + : _fieldRef(std::move(fr)), _types(std::move(types)) { + invariant(good()); + } + + /** + * Returns the underlying FieldRef. + */ + const FieldRef& fieldRef() const { + invariant(good()); + return _fieldRef; + } + + /** + * Returns the type array, which is guaranteed to be the same length as the backing FieldRef. + */ + const ComponentTypeVector& types() const { + invariant(good()); + return _types; + } + + bool empty() const { + invariant(good()); + return _fieldRef.empty(); + } + + size_t size() const { + invariant(good()); + return _fieldRef.numParts(); + } + + void append(StringData fieldName, ComponentType type) { + invariant(good()); + _fieldRef.appendPart(fieldName); + _types.push_back(type); + } + + void popBack() { + invariant(good()); + invariant(_fieldRef.numParts() > 0); + _fieldRef.removeLastPart(); + _types.pop_back(); + } + +private: + // Returns whether the RuntimeUpdatePath is in a valid state. + bool good() const { + return _fieldRef.numParts() == _types.size(); + } + + // There is an invariant that the number of components in field ref is equal to the number of + // types in _types. + FieldRef _fieldRef; + ComponentTypeVector _types; +}; + +/** + * RAII class for temporarily appending to a RuntimeUpdatePath. + */ +class RuntimeUpdatePathTempAppend { +public: + RuntimeUpdatePathTempAppend(RuntimeUpdatePath& typedFieldRef, + StringData part, + RuntimeUpdatePath::ComponentType type) + : _typedFieldRef(typedFieldRef) { + _typedFieldRef.append(part, type); + } + + ~RuntimeUpdatePathTempAppend() { + _typedFieldRef.popBack(); + } + +private: + RuntimeUpdatePath& _typedFieldRef; +}; +} // namespace mongo diff --git a/src/mongo/db/update/set_node.cpp b/src/mongo/db/update/set_node.cpp index a2ee3562dfe..3175cb38f6b 100644 --- a/src/mongo/db/update/set_node.cpp +++ b/src/mongo/db/update/set_node.cpp @@ -43,8 +43,8 @@ Status SetNode::init(BSONElement modExpr, const boost::intrusive_ptr<ExpressionC return Status::OK(); } -ModifierNode::ModifyResult SetNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { +ModifierNode::ModifyResult SetNode::updateExistingElement(mutablebson::Element* element, + const FieldRef& elementPath) const { // If 'element' is deserialized, then element.getValue() will be EOO, which will never equal // val. if (element->getValue().binaryEqualValues(val)) { diff --git a/src/mongo/db/update/set_node.h b/src/mongo/db/update/set_node.h index 4261f3e736b..e9920826f29 100644 --- a/src/mongo/db/update/set_node.h +++ b/src/mongo/db/update/set_node.h @@ -59,7 +59,7 @@ public: protected: ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void setValueForNewElement(mutablebson::Element* element) const final; bool allowCreation() const final { diff --git a/src/mongo/db/update/set_node_test.cpp b/src/mongo/db/update/set_node_test.cpp index 8cd2b60243e..45198f6e179 100644 --- a/src/mongo/db/update/set_node_test.cpp +++ b/src/mongo/db/update/set_node_test.cpp @@ -67,7 +67,7 @@ TEST_F(SetNodeTest, ApplyNoOp) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -85,7 +85,7 @@ TEST_F(SetNodeTest, ApplyEmptyPathToCreate) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -104,7 +104,7 @@ TEST_F(SetNodeTest, ApplyCreatePath) { mutablebson::Document doc(fromjson("{a: {d: 5}}")); setPathToCreate("b.c"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -140,7 +140,7 @@ TEST_F(SetNodeTest, ApplyPositional) { ASSERT_OK(node.init(update["$set"]["a.$"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2]}")); - setPathTaken("a.1"); + setPathTaken(makeRuntimeUpdatePathForTest("a.1")); setMatchedField("1"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][1]), getUpdateNodeApplyParams()); @@ -160,7 +160,7 @@ TEST_F(SetNodeTest, ApplyNonViablePathToCreate) { mutablebson::Document doc(fromjson("{a: 5}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -177,7 +177,7 @@ TEST_F(SetNodeTest, ApplyNonViablePathToCreateFromReplicationIsNoOp) { mutablebson::Document doc(fromjson("{a: 5}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); setFromOplogApplication(true); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); @@ -196,7 +196,7 @@ TEST_F(SetNodeTest, ApplyNoIndexDataNoLogBuilder) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -213,7 +213,7 @@ TEST_F(SetNodeTest, ApplyDoesNotAffectIndexes) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -230,7 +230,7 @@ TEST_F(SetNodeTest, TypeChangeIsNotANoop) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: NumberInt(2)}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -251,7 +251,7 @@ TEST_F(SetNodeTest, IdentityOpOnDeserializedIsNotANoOp) { // Apply a mutation to the document that will make it non-serialized. doc.root()["a"]["b"].setValueInt(2).transitional_ignore(); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -285,7 +285,7 @@ TEST_F(SetNodeTest, ApplyInPlace) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -302,7 +302,7 @@ TEST_F(SetNodeTest, ApplyOverridePath) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 1}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -319,7 +319,7 @@ TEST_F(SetNodeTest, ApplyChangeType) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 'str'}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -353,7 +353,7 @@ TEST_F(SetNodeTest, ApplyLog) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_EQUALS(fromjson("{a: 2}"), doc); ASSERT_TRUE(doc.isInPlaceModeEnabled()); @@ -369,7 +369,7 @@ TEST_F(SetNodeTest, ApplyNoOpDottedPath) { ASSERT_OK(node.init(update["$set"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -386,7 +386,7 @@ TEST_F(SetNodeTest, TypeChangeOnDottedPathIsNotANoOp) { ASSERT_OK(node.init(update["$set"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: NumberLong(2)}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -404,7 +404,7 @@ TEST_F(SetNodeTest, ApplyPathNotViable) { mutablebson::Document doc(fromjson("{a:1}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), AssertionException, @@ -420,7 +420,7 @@ TEST_F(SetNodeTest, ApplyPathNotViableArrray) { mutablebson::Document doc(fromjson("{a:[{b:1}]}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), AssertionException, @@ -435,7 +435,7 @@ TEST_F(SetNodeTest, ApplyInPlaceDottedPath) { ASSERT_OK(node.init(update["$set"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 1}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -452,7 +452,7 @@ TEST_F(SetNodeTest, ApplyChangeTypeDottedPath) { ASSERT_OK(node.init(update["$set"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 'str'}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -469,7 +469,7 @@ TEST_F(SetNodeTest, ApplyChangePath) { ASSERT_OK(node.init(update["$set"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: {c: 1}}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -487,7 +487,7 @@ TEST_F(SetNodeTest, ApplyExtendPath) { mutablebson::Document doc(fromjson("{a: {c: 1}}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -555,7 +555,7 @@ TEST_F(SetNodeTest, ApplyNoOpArrayIndex) { ASSERT_OK(node.init(update["$set"]["a.2.b"], expCtx)); mutablebson::Document doc(fromjson("{a: [{b: 0},{b: 1},{b: 2}]}")); - setPathTaken("a.2.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.2.b")); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"][2]["b"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -572,7 +572,7 @@ TEST_F(SetNodeTest, TypeChangeInArrayIsNotANoOp) { ASSERT_OK(node.init(update["$set"]["a.2.b"], expCtx)); mutablebson::Document doc(fromjson("{a: [{b: 0},{b: 1},{b: 2.0}]}")); - setPathTaken("a.2.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.2.b")); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"][2]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -590,7 +590,7 @@ TEST_F(SetNodeTest, ApplyNonViablePath) { mutablebson::Document doc(fromjson("{a: 0}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), AssertionException, @@ -605,7 +605,7 @@ TEST_F(SetNodeTest, ApplyInPlaceArrayIndex) { ASSERT_OK(node.init(update["$set"]["a.2.b"], expCtx)); mutablebson::Document doc(fromjson("{a: [{b: 0},{b: 1},{b: 1}]}")); - setPathTaken("a.2.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.2.b")); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"][2]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -623,7 +623,7 @@ TEST_F(SetNodeTest, ApplyNormalArray) { mutablebson::Document doc(fromjson("{a: [{b: 0},{b: 1}]}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -641,7 +641,7 @@ TEST_F(SetNodeTest, ApplyPaddingArray) { mutablebson::Document doc(fromjson("{a: [{b: 0}]}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -659,7 +659,7 @@ TEST_F(SetNodeTest, ApplyNumericObject) { mutablebson::Document doc(fromjson("{a: {b: 0}}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -676,7 +676,10 @@ TEST_F(SetNodeTest, ApplyNumericField) { ASSERT_OK(node.init(update["$set"]["a.2.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {'2': {b: 1}}}")); - setPathTaken("a.2.b"); + setPathTaken(RuntimeUpdatePath(FieldRef("a.2.b"), + {RuntimeUpdatePath::ComponentType::kFieldName, + RuntimeUpdatePath::ComponentType::kFieldName, + RuntimeUpdatePath::ComponentType::kFieldName})); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["2"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -694,7 +697,9 @@ TEST_F(SetNodeTest, ApplyExtendNumericField) { mutablebson::Document doc(fromjson("{a: {'2': {c: 1}}}")); setPathToCreate("b"); - setPathTaken("a.2"); + setPathTaken(RuntimeUpdatePath(FieldRef("a.2"), + {RuntimeUpdatePath::ComponentType::kFieldName, + RuntimeUpdatePath::ComponentType::kFieldName})); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"]["2"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -712,7 +717,7 @@ TEST_F(SetNodeTest, ApplyEmptyObject) { mutablebson::Document doc(fromjson("{a: {}}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -730,7 +735,7 @@ TEST_F(SetNodeTest, ApplyEmptyArray) { mutablebson::Document doc(fromjson("{a: []}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.2.b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -748,7 +753,7 @@ TEST_F(SetNodeTest, ApplyLogDottedPath) { mutablebson::Document doc(fromjson("{a: [{b:0}, {b:1}]}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_EQUALS(fromjson("{a: [{b:0}, {b:1}, {b:2}]}"), doc); ASSERT_FALSE(doc.isInPlaceModeEnabled()); @@ -765,7 +770,7 @@ TEST_F(SetNodeTest, LogEmptyArray) { mutablebson::Document doc(fromjson("{a: []}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_EQUALS(fromjson("{a: [null, null, {b:2}]}"), doc); ASSERT_FALSE(doc.isInPlaceModeEnabled()); @@ -782,7 +787,7 @@ TEST_F(SetNodeTest, LogEmptyObject) { mutablebson::Document doc(fromjson("{a: {}}")); setPathToCreate("2.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_EQUALS(fromjson("{a: {'2': {b: 2}}}"), doc); ASSERT_FALSE(doc.isInPlaceModeEnabled()); @@ -798,7 +803,7 @@ TEST_F(SetNodeTest, ApplyNoOpComplex) { ASSERT_OK(node.init(update["$set"]["a.1.b"], expCtx)); mutablebson::Document doc(fromjson("{a: [{b: {c: 0, d: 0}}, {b: {c: 1, d: 1}}]}}")); - setPathTaken("a.1.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.1.b")); addIndexedPath("a.1.b"); auto result = node.apply(getApplyParams(doc.root()["a"][1]["b"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -815,7 +820,7 @@ TEST_F(SetNodeTest, ApplySameStructure) { ASSERT_OK(node.init(update["$set"]["a.1.b"], expCtx)); mutablebson::Document doc(fromjson("{a: [{b: {c: 0, d: 0}}, {b: {c: 1, xxx: 1}}]}}")); - setPathTaken("a.1.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.1.b")); addIndexedPath("a.1.b"); auto result = node.apply(getApplyParams(doc.root()["a"][1]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -833,7 +838,7 @@ TEST_F(SetNodeTest, NonViablePathWithoutRepl) { mutablebson::Document doc(fromjson("{a: 1}")); setPathToCreate("1.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), AssertionException, @@ -849,7 +854,7 @@ TEST_F(SetNodeTest, SingleFieldFromReplication) { mutablebson::Document doc(fromjson("{_id:1, a: 1}")); setPathToCreate("1.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.1.b"); setFromOplogApplication(true); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); @@ -868,7 +873,7 @@ TEST_F(SetNodeTest, SingleFieldNoIdFromReplication) { mutablebson::Document doc(fromjson("{a: 1}")); setPathToCreate("1.b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.1.b"); setFromOplogApplication(true); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); @@ -887,7 +892,7 @@ TEST_F(SetNodeTest, NestedFieldFromReplication) { mutablebson::Document doc(fromjson("{_id:1, a: {a: 1}}")); setPathToCreate("1.b"); - setPathTaken("a.a"); + setPathTaken(makeRuntimeUpdatePathForTest("a.a")); addIndexedPath("a.a.1.b"); setFromOplogApplication(true); auto result = node.apply(getApplyParams(doc.root()["a"]["a"]), getUpdateNodeApplyParams()); @@ -906,7 +911,7 @@ TEST_F(SetNodeTest, DoubleNestedFieldFromReplication) { mutablebson::Document doc(fromjson("{_id:1, a: {b: {c: 1}}}")); setPathToCreate("d"); - setPathTaken("a.b.c"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b.c")); addIndexedPath("a.b.c.d"); setFromOplogApplication(true); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]["c"]), getUpdateNodeApplyParams()); @@ -925,7 +930,7 @@ TEST_F(SetNodeTest, NestedFieldNoIdFromReplication) { mutablebson::Document doc(fromjson("{a: {a: 1}}")); setPathToCreate("1.b"); - setPathTaken("a.a"); + setPathTaken(makeRuntimeUpdatePathForTest("a.a")); addIndexedPath("a.a.1.b"); setFromOplogApplication(true); auto result = node.apply(getApplyParams(doc.root()["a"]["a"]), getUpdateNodeApplyParams()); @@ -944,7 +949,7 @@ TEST_F(SetNodeTest, ReplayArrayFieldNotAppendedIntermediateFromReplication) { mutablebson::Document doc(fromjson("{_id: 0, a: [1, {b: [1]}]}")); setPathToCreate("b"); - setPathTaken("a.0"); + setPathTaken(makeRuntimeUpdatePathForTest("a.0")); addIndexedPath("a.1.b"); setFromOplogApplication(true); auto result = node.apply(getApplyParams(doc.root()["a"][0]), getUpdateNodeApplyParams()); @@ -962,7 +967,7 @@ TEST_F(SetNodeTest, Set6) { ASSERT_OK(node.init(update["$set"]["r.a"], expCtx)); mutablebson::Document doc(fromjson("{_id: 1, r: {a:1, b:2}}")); - setPathTaken("r.a"); + setPathTaken(makeRuntimeUpdatePathForTest("r.a")); addIndexedPath("r.a"); auto result = node.apply(getApplyParams(doc.root()["r"]["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -981,7 +986,7 @@ TEST_F(SetNodeTest, Set6FromRepl) { ASSERT_OK(node.init(update["$set"]["r.a"], expCtx)); mutablebson::Document doc(fromjson("{_id: 1, r: {a:1, b:2}}")); - setPathTaken("r.a"); + setPathTaken(makeRuntimeUpdatePathForTest("r.a")); addIndexedPath("r.a"); setFromOplogApplication(true); auto result = node.apply(getApplyParams(doc.root()["r"]["a"]), getUpdateNodeApplyParams()); @@ -1009,7 +1014,7 @@ TEST_F(SetNodeTest, ApplySetModToEphemeralDocument) { Element a = doc.makeElementInt("a", 100); x.pushBack(a).transitional_ignore(); - setPathTaken("x"); + setPathTaken(makeRuntimeUpdatePathForTest("x")); addIndexedPath("x"); auto result = node.apply(getApplyParams(doc.root()["x"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -1025,7 +1030,7 @@ TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldInsideSetElement) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), AssertionException, @@ -1105,7 +1110,7 @@ TEST_F(SetNodeTest, ApplyCannotOverwriteImmutablePath) { ASSERT_OK(node.init(update["$set"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addImmutablePath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()), @@ -1121,7 +1126,7 @@ TEST_F(SetNodeTest, ApplyCanPerformNoopOnImmutablePath) { ASSERT_OK(node.init(update["$set"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addImmutablePath("a.b"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); @@ -1141,7 +1146,7 @@ TEST_F(SetNodeTest, ApplyCannotOverwritePrefixToRemoveImmutablePath) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addImmutablePath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -1157,7 +1162,7 @@ TEST_F(SetNodeTest, ApplyCannotOverwritePrefixToModifyImmutablePath) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addImmutablePath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -1174,7 +1179,7 @@ TEST_F(SetNodeTest, ApplyCanPerformNoopOnPrefixOfImmutablePath) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addImmutablePath("a.b"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); @@ -1194,7 +1199,7 @@ TEST_F(SetNodeTest, ApplyCanOverwritePrefixToCreateImmutablePath) { ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addImmutablePath("a.b"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); @@ -1214,7 +1219,7 @@ TEST_F(SetNodeTest, ApplyCanOverwritePrefixOfImmutablePathIfNoopOnImmutablePath) ASSERT_OK(node.init(update["$set"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 2}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addImmutablePath("a.b"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); @@ -1234,7 +1239,7 @@ TEST_F(SetNodeTest, ApplyCannotOverwriteSuffixOfImmutablePath) { ASSERT_OK(node.init(update["$set"]["a.b.c"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: {c: 2}}}")); - setPathTaken("a.b.c"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b.c")); addImmutablePath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]["b"]["c"]), getUpdateNodeApplyParams()), @@ -1250,7 +1255,7 @@ TEST_F(SetNodeTest, ApplyCanPerformNoopOnSuffixOfImmutablePath) { ASSERT_OK(node.init(update["$set"]["a.b.c"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: {c: 2}}}")); - setPathTaken("a.b.c"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b.c")); addImmutablePath("a.b"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]["c"]), getUpdateNodeApplyParams()); @@ -1271,7 +1276,7 @@ TEST_F(SetNodeTest, ApplyCannotCreateFieldAtEndOfImmutablePath) { mutablebson::Document doc(fromjson("{a: {b: {}}}")); setPathToCreate("c"); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addImmutablePath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()), @@ -1288,7 +1293,7 @@ TEST_F(SetNodeTest, ApplyCannotCreateFieldBeyondEndOfImmutablePath) { mutablebson::Document doc(fromjson("{a: {b: {}}}")); setPathToCreate("c"); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addImmutablePath("a"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()), @@ -1305,7 +1310,7 @@ TEST_F(SetNodeTest, ApplyCanCreateImmutablePath) { mutablebson::Document doc(fromjson("{a: {}}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addImmutablePath("a.b"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); @@ -1346,7 +1351,7 @@ TEST_F(SetNodeTest, ApplySetFieldInNonExistentArrayElementAffectsIndexOnSiblingF mutablebson::Document doc(fromjson("{a: [{b: 0}]}")); setPathToCreate("1.c"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -1366,7 +1371,7 @@ TEST_F(SetNodeTest, ApplySetFieldInExistingArrayElementDoesNotAffectIndexOnSibli mutablebson::Document doc(fromjson("{a: [{b: 0}]}")); setPathToCreate("c"); - setPathTaken("a.0"); + setPathTaken(makeRuntimeUpdatePathForTest("a.0")); addIndexedPath("a.b"); auto result = node.apply(getApplyParams(doc.root()["a"][0]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -1386,7 +1391,7 @@ TEST_F(SetNodeTest, ApplySetFieldInNonExistentNumericFieldDoesNotAffectIndexOnSi mutablebson::Document doc(fromjson("{a: {'0': {b: 0}}}")); setPathToCreate("1.c"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a.b"); addIndexedPath("a.1.b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); @@ -1443,7 +1448,7 @@ TEST_F(SetNodeTest, ApplySetOnInsertExistingPath) { ASSERT_OK(node.init(update["$setOnInsert"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 1}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setInsert(true); addIndexedPath("a"); setLogBuilderToNull(); // The log builder is null for inserts. diff --git a/src/mongo/db/update/unset_node.cpp b/src/mongo/db/update/unset_node.cpp index 44a450c2a29..d49b5492cd3 100644 --- a/src/mongo/db/update/unset_node.cpp +++ b/src/mongo/db/update/unset_node.cpp @@ -41,8 +41,8 @@ Status UnsetNode::init(BSONElement modExpr, const boost::intrusive_ptr<Expressio return Status::OK(); } -ModifierNode::ModifyResult UnsetNode::updateExistingElement( - mutablebson::Element* element, std::shared_ptr<FieldRef> elementPath) const { +ModifierNode::ModifyResult UnsetNode::updateExistingElement(mutablebson::Element* element, + const FieldRef& elementPath) const { auto parent = element->parent(); invariant(parent.ok()); @@ -78,13 +78,15 @@ void UnsetNode::validateUpdate(mutablebson::ConstElement updatedElement, } } -void UnsetNode::logUpdate(LogBuilder* logBuilder, - StringData pathTaken, +void UnsetNode::logUpdate(LogBuilderInterface* logBuilder, + const RuntimeUpdatePath& pathTaken, mutablebson::Element element, - ModifyResult modifyResult) const { + ModifyResult modifyResult, + boost::optional<int> createdFieldIdx) const { invariant(logBuilder); invariant(modifyResult == ModifyResult::kNormalUpdate); - uassertStatusOK(logBuilder->addToUnsets(pathTaken)); + invariant(!createdFieldIdx); + uassertStatusOK(logBuilder->logDeletedField(pathTaken)); } } // namespace mongo diff --git a/src/mongo/db/update/unset_node.h b/src/mongo/db/update/unset_node.h index 8fc466fef4f..1aab11bc1b8 100644 --- a/src/mongo/db/update/unset_node.h +++ b/src/mongo/db/update/unset_node.h @@ -50,7 +50,7 @@ public: void setCollator(const CollatorInterface* collator) final {} ModifyResult updateExistingElement(mutablebson::Element* element, - std::shared_ptr<FieldRef> elementPath) const final; + const FieldRef& elementPath) const final; void validateUpdate(mutablebson::ConstElement updatedElement, mutablebson::ConstElement leftSibling, @@ -58,10 +58,11 @@ public: std::uint32_t recursionLevel, ModifyResult modifyResult) const final; - void logUpdate(LogBuilder* logBuilder, - StringData pathTaken, + void logUpdate(LogBuilderInterface* logBuilder, + const RuntimeUpdatePath& pathTaken, mutablebson::Element element, - ModifyResult modifyResult) const final; + ModifyResult modifyResult, + boost::optional<int> createdFieldIdx) const final; bool allowNonViablePath() const final { return true; diff --git a/src/mongo/db/update/unset_node_test.cpp b/src/mongo/db/update/unset_node_test.cpp index fc2c04e9932..67cb1140856 100644 --- a/src/mongo/db/update/unset_node_test.cpp +++ b/src/mongo/db/update/unset_node_test.cpp @@ -57,7 +57,7 @@ DEATH_TEST_REGEX(UnsetNodeTest, DEATH_TEST_REGEX_F(UnsetNodeTest, ApplyToRootFails, - R"#(Invariant failure.*!updateNodeApplyParams.pathTaken->empty\(\))#") { + R"#(Invariant failure.*!updateNodeApplyParams.pathTaken.*empty\(\))#") { auto update = fromjson("{$unset: {}}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); UnsetNode node; @@ -101,7 +101,7 @@ TEST_F(UnsetNodeTest, UnsetNoOpDottedPath) { mutablebson::Document doc(fromjson("{a: 5}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -120,7 +120,7 @@ TEST_F(UnsetNodeTest, UnsetNoOpThroughArray) { mutablebson::Document doc(fromjson("{a:[{b:1}]}")); setPathToCreate("b"); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_TRUE(result.noop); @@ -156,7 +156,7 @@ TEST_F(UnsetNodeTest, UnsetTopLevelPath) { ASSERT_OK(node.init(update["$unset"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -174,7 +174,7 @@ TEST_F(UnsetNodeTest, UnsetNestedPath) { ASSERT_OK(node.init(update["$unset"]["a.b.c"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: {c: 6}}}}")); - setPathTaken("a.b.c"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b.c")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]["c"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -192,7 +192,7 @@ TEST_F(UnsetNodeTest, UnsetObject) { ASSERT_OK(node.init(update["$unset"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: {c: 6}}}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -210,7 +210,7 @@ TEST_F(UnsetNodeTest, UnsetArrayElement) { ASSERT_OK(node.init(update["$unset"]["a.0"], expCtx)); mutablebson::Document doc(fromjson("{a:[1], b:1}")); - setPathTaken("a.0"); + setPathTaken(makeRuntimeUpdatePathForTest("a.0")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][0]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -228,7 +228,7 @@ TEST_F(UnsetNodeTest, UnsetPositional) { ASSERT_OK(node.init(update["$unset"]["a.$"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2]}")); - setPathTaken("a.1"); + setPathTaken(makeRuntimeUpdatePathForTest("a.1")); setMatchedField("1"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][1]), getUpdateNodeApplyParams()); @@ -247,7 +247,7 @@ TEST_F(UnsetNodeTest, UnsetEntireArray) { ASSERT_OK(node.init(update["$unset"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: [0, 1, 2]}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -265,7 +265,7 @@ TEST_F(UnsetNodeTest, UnsetFromObjectInArray) { ASSERT_OK(node.init(update["$unset"]["a.0.b"], expCtx)); mutablebson::Document doc(fromjson("{a: [{b: 1}]}")); - setPathTaken("a.0.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.0.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][0]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -283,7 +283,7 @@ TEST_F(UnsetNodeTest, CanUnsetInvalidField) { ASSERT_OK(node.init(update["$unset"]["a.$.$b"], expCtx)); mutablebson::Document doc(fromjson("{b: 1, a: [{$b: 1}]}")); - setPathTaken("a.0.$b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.0.$b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"][0]["$b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -301,7 +301,7 @@ TEST_F(UnsetNodeTest, ApplyNoIndexDataNoLogBuilder) { ASSERT_OK(node.init(update["$unset"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); setLogBuilderToNull(); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -318,7 +318,7 @@ TEST_F(UnsetNodeTest, ApplyDoesNotAffectIndexes) { ASSERT_OK(node.init(update["$unset"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: 5}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addIndexedPath("b"); auto result = node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -336,7 +336,7 @@ TEST_F(UnsetNodeTest, ApplyFieldWithDot) { ASSERT_OK(node.init(update["$unset"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{'a.b':4, a: {b: 2}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); ASSERT_FALSE(result.noop); @@ -354,7 +354,7 @@ TEST_F(UnsetNodeTest, ApplyCannotRemoveRequiredPartOfDBRef) { ASSERT_OK(node.init(update["$unset"]["a.$id"], expCtx)); mutablebson::Document doc(fromjson("{a: {$ref: 'c', $id: 0}}")); - setPathTaken("a.$id"); + setPathTaken(makeRuntimeUpdatePathForTest("a.$id")); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]["$id"]), getUpdateNodeApplyParams()), AssertionException, @@ -369,7 +369,7 @@ TEST_F(UnsetNodeTest, ApplyCanRemoveRequiredPartOfDBRefIfValidateForStorageIsFal ASSERT_OK(node.init(update["$unset"]["a.$id"], expCtx)); mutablebson::Document doc(fromjson("{a: {$ref: 'c', $id: 0}}")); - setPathTaken("a.$id"); + setPathTaken(makeRuntimeUpdatePathForTest("a.$id")); addIndexedPath("a"); setValidateForStorage(false); auto result = node.apply(getApplyParams(doc.root()["a"]["$id"]), getUpdateNodeApplyParams()); @@ -390,7 +390,7 @@ TEST_F(UnsetNodeTest, ApplyCannotRemoveImmutablePath) { ASSERT_OK(node.init(update["$unset"]["a.b"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 1}}")); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addImmutablePath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()), @@ -406,7 +406,7 @@ TEST_F(UnsetNodeTest, ApplyCannotRemovePrefixOfImmutablePath) { ASSERT_OK(node.init(update["$unset"]["a"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: 1}}")); - setPathTaken("a"); + setPathTaken(makeRuntimeUpdatePathForTest("a")); addImmutablePath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()), @@ -422,7 +422,7 @@ TEST_F(UnsetNodeTest, ApplyCannotRemoveSuffixOfImmutablePath) { ASSERT_OK(node.init(update["$unset"]["a.b.c"], expCtx)); mutablebson::Document doc(fromjson("{a: {b: {c: 1}}}")); - setPathTaken("a.b.c"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b.c")); addImmutablePath("a.b"); ASSERT_THROWS_CODE_AND_WHAT( node.apply(getApplyParams(doc.root()["a"]["b"]["c"]), getUpdateNodeApplyParams()), @@ -439,7 +439,7 @@ TEST_F(UnsetNodeTest, ApplyCanRemoveImmutablePathIfNoop) { mutablebson::Document doc(fromjson("{a: {b: 1}}")); setPathToCreate("c"); - setPathTaken("a.b"); + setPathTaken(makeRuntimeUpdatePathForTest("a.b")); addImmutablePath("a.b"); addIndexedPath("a"); auto result = node.apply(getApplyParams(doc.root()["a"]["b"]), getUpdateNodeApplyParams()); diff --git a/src/mongo/db/update/update_array_node.cpp b/src/mongo/db/update/update_array_node.cpp index 46d4f88828d..c7c1ea904f6 100644 --- a/src/mongo/db/update/update_array_node.cpp +++ b/src/mongo/db/update/update_array_node.cpp @@ -50,12 +50,12 @@ std::unique_ptr<UpdateNode> UpdateArrayNode::createUpdateNodeByMerging( UpdateExecutor::ApplyResult UpdateArrayNode::apply( ApplyParams applyParams, UpdateNodeApplyParams updateNodeApplyParams) const { if (!updateNodeApplyParams.pathToCreate->empty()) { + FieldRef pathTakenCopy(updateNodeApplyParams.pathTaken->fieldRef()); for (size_t i = 0; i < updateNodeApplyParams.pathToCreate->numParts(); ++i) { - updateNodeApplyParams.pathTaken->appendPart( - updateNodeApplyParams.pathToCreate->getPart(i)); + pathTakenCopy.appendPart(updateNodeApplyParams.pathToCreate->getPart(i)); } uasserted(ErrorCodes::BadValue, - str::stream() << "The path '" << updateNodeApplyParams.pathTaken->dottedField() + str::stream() << "The path '" << pathTakenCopy.dottedField() << "' must exist in the document in order to apply array updates."); } @@ -118,8 +118,10 @@ UpdateExecutor::ApplyResult UpdateArrayNode::apply( // Merge all of the updates for this array element. invariant(updates->second.size() > 0); auto mergedChild = updates->second[0]; - FieldRef::FieldRefTempAppend tempAppend(*(updateNodeApplyParams.pathTaken), - childElement.getFieldName()); + RuntimeUpdatePathTempAppend tempAppend(*updateNodeApplyParams.pathTaken, + childElement.getFieldName(), + RuntimeUpdatePath::ComponentType::kArrayIndex); + for (size_t j = 1; j < updates->second.size(); ++j) { // Use the cached merge result, if it is available. @@ -129,11 +131,15 @@ UpdateExecutor::ApplyResult UpdateArrayNode::apply( continue; } + // UpdateNode::createUpdateNodeByMerging() requires a mutable field path + FieldRef pathTakenFieldRefCopy(updateNodeApplyParams.pathTaken->fieldRef()); + + // The cached merge result is not available, so perform the merge and cache the // result. _mergedChildrenCache[mergedChild][updates->second[j]] = UpdateNode::createUpdateNodeByMerging( - *mergedChild, *updates->second[j], updateNodeApplyParams.pathTaken.get()); + *mergedChild, *updates->second[j], &pathTakenFieldRefCopy); mergedChild = _mergedChildrenCache[mergedChild][updates->second[j]].get(); } @@ -162,29 +168,29 @@ UpdateExecutor::ApplyResult UpdateArrayNode::apply( // If no elements match the array filter, report the path to the array itself as modified. if (applyParams.modifiedPaths && matchingElements.size() == 0) { - applyParams.modifiedPaths->keepShortest(*updateNodeApplyParams.pathTaken); + applyParams.modifiedPaths->keepShortest(updateNodeApplyParams.pathTaken->fieldRef()); } // If the child updates have not been logged, log the updated array elements. auto* const logBuilder = updateNodeApplyParams.logBuilder; if (!childrenShouldLogThemselves && logBuilder) { - if (nModified > 1) { + // Earlier we should have checked that the path already exists. + invariant(updateNodeApplyParams.pathToCreate->empty()); + if (nModified > 1) { // Log the entire array. - auto logElement = logBuilder->getDocument().makeElementWithNewFieldName( - updateNodeApplyParams.pathTaken->dottedField(), applyParams.element); - invariant(logElement.ok()); - uassertStatusOK(logBuilder->addToSets(logElement)); + uassertStatusOK( + logBuilder->logUpdatedField(*updateNodeApplyParams.pathTaken, applyParams.element)); } else if (nModified == 1) { - // Log the modified array element. invariant(modifiedElement); - FieldRef::FieldRefTempAppend tempAppend(*(updateNodeApplyParams.pathTaken), - modifiedElement->getFieldName()); - auto logElement = logBuilder->getDocument().makeElementWithNewFieldName( - updateNodeApplyParams.pathTaken->dottedField(), *modifiedElement); - invariant(logElement.ok()); - uassertStatusOK(logBuilder->addToSets(logElement)); + + // Temporarily append the array index. + RuntimeUpdatePathTempAppend tempAppend(*updateNodeApplyParams.pathTaken, + modifiedElement->getFieldName(), + RuntimeUpdatePath::ComponentType::kArrayIndex); + uassertStatusOK( + logBuilder->logUpdatedField(*updateNodeApplyParams.pathTaken, *modifiedElement)); } } diff --git a/src/mongo/db/update/update_driver.cpp b/src/mongo/db/update/update_driver.cpp index 5e0fbbf2332..d82a2b1978c 100644 --- a/src/mongo/db/update/update_driver.cpp +++ b/src/mongo/db/update/update_driver.cpp @@ -45,6 +45,7 @@ #include "mongo/db/update/object_replace_executor.h" #include "mongo/db/update/path_support.h" #include "mongo/db/update/storage_validation.h" +#include "mongo/db/update/update_oplog_entry_version.h" #include "mongo/stdx/variant.h" #include "mongo/util/embedded_builder.h" #include "mongo/util/str.h" diff --git a/src/mongo/db/update/update_node.h b/src/mongo/db/update/update_node.h index 59b8e513902..0f0e76fde09 100644 --- a/src/mongo/db/update/update_node.h +++ b/src/mongo/db/update/update_node.h @@ -38,7 +38,8 @@ #include "mongo/bson/bsonelement.h" #include "mongo/bson/mutable/element.h" #include "mongo/db/field_ref_set.h" -#include "mongo/db/update/log_builder.h" +#include "mongo/db/update/log_builder_interface.h" +#include "mongo/db/update/runtime_update_path.h" #include "mongo/db/update/update_executor.h" #include "mongo/db/update/update_node_visitor.h" #include "mongo/db/update_index_data.h" @@ -81,11 +82,11 @@ public: // The path through the root document to 'element', ending with the field name of 'element'. // For example, if the update is {$set: {'a.b.c': 5}}, and the document is {a: {}}, then at // the leaf node, 'pathTaken'="a". - std::shared_ptr<FieldRef> pathTaken = std::make_shared<FieldRef>(); + std::shared_ptr<RuntimeUpdatePath> pathTaken = std::make_shared<RuntimeUpdatePath>(); // Builder object used for constructing an oplog entry. A value of nullptr indicates that // no oplog entry needs to be constructed. - LogBuilder* logBuilder = nullptr; + LogBuilderInterface* logBuilder = nullptr; }; explicit UpdateNode(Type type, Context context = Context::kAll) diff --git a/src/mongo/db/update/update_node_test_fixture.h b/src/mongo/db/update/update_node_test_fixture.h index 28bc2cb9721..aa4721894b4 100644 --- a/src/mongo/db/update/update_node_test_fixture.h +++ b/src/mongo/db/update/update_node_test_fixture.h @@ -33,6 +33,7 @@ #include "mongo/db/service_context.h" #include "mongo/db/service_context_test_fixture.h" #include "mongo/db/update/update_node.h" +#include "mongo/db/update/v1_log_builder.h" #include "mongo/unittest/unittest.h" namespace mongo { @@ -42,6 +43,22 @@ public: ~UpdateNodeTest() override = default; protected: + // Creates a RuntimeUpdatePath from a string, assuming that all numeric path components are + // array indexes. Tests which use numeric field names in objects must manually create a + // RuntimeUpdatePath. + static RuntimeUpdatePath makeRuntimeUpdatePathForTest(StringData path) { + FieldRef fr(path); + std::vector<RuntimeUpdatePath::ComponentType> types; + + for (int i = 0; i < fr.numParts(); ++i) { + types.push_back(fr.isNumericPathComponentStrict(i) + ? RuntimeUpdatePath::ComponentType::kArrayIndex + : RuntimeUpdatePath::ComponentType::kFieldName); + } + + return RuntimeUpdatePath(fr, std::move(types)); + } + void setUp() override { resetApplyParams(); @@ -55,14 +72,14 @@ protected: _immutablePathsVector.clear(); _immutablePaths.clear(); _pathToCreate = std::make_shared<FieldRef>(); - _pathTaken = std::make_shared<FieldRef>(); + _pathTaken = std::make_shared<RuntimeUpdatePath>(); _matchedField = StringData(); _insert = false; _fromOplogApplication = false; _validateForStorage = true; _indexData.reset(); _logDoc.reset(); - _logBuilder = std::make_unique<LogBuilder>(_logDoc.root()); + _logBuilder = std::make_unique<V1LogBuilder>(_logDoc.root()); _modifiedPaths.clear(); } @@ -97,9 +114,8 @@ protected: _pathToCreate->parse(path); } - void setPathTaken(StringData path) { - _pathTaken->clear(); - _pathTaken->parse(path); + void setPathTaken(const RuntimeUpdatePath& pathTaken) { + *_pathTaken = pathTaken; } void setMatchedField(StringData matchedField) { @@ -141,14 +157,14 @@ private: std::vector<std::unique_ptr<FieldRef>> _immutablePathsVector; FieldRefSet _immutablePaths; std::shared_ptr<FieldRef> _pathToCreate; - std::shared_ptr<FieldRef> _pathTaken; + std::shared_ptr<RuntimeUpdatePath> _pathTaken; StringData _matchedField; bool _insert; bool _fromOplogApplication; bool _validateForStorage; std::unique_ptr<UpdateIndexData> _indexData; mutablebson::Document _logDoc; - std::unique_ptr<LogBuilder> _logBuilder; + std::unique_ptr<LogBuilderInterface> _logBuilder; FieldRefSetWithStorage _modifiedPaths; }; diff --git a/src/mongo/db/update/update_object_node.cpp b/src/mongo/db/update/update_object_node.cpp index 62c901a1ae8..b27a9fb18e9 100644 --- a/src/mongo/db/update/update_object_node.cpp +++ b/src/mongo/db/update/update_object_node.cpp @@ -108,7 +108,7 @@ void applyChild(const UpdateNode& child, UpdateNode::UpdateNodeApplyParams* updateNodeApplyParams, UpdateExecutor::ApplyResult* applyResult) { - auto pathTakenSizeBefore = updateNodeApplyParams->pathTaken->numParts(); + auto pathTakenSizeBefore = updateNodeApplyParams->pathTaken->fieldRef().numParts(); // A non-ok value for childElement will indicate that we need to append 'field' to the // 'pathToCreate' FieldRef. @@ -125,7 +125,11 @@ void applyChild(const UpdateNode& child, // The path we've traversed so far already exists in our document, and 'childElement' // represents the Element indicated by the 'field' name or index, which we indicate by // updating the 'pathTaken' FieldRef. - updateNodeApplyParams->pathTaken->appendPart(field); + updateNodeApplyParams->pathTaken->append( + field, + applyParams->element.getType() == BSONType::Array + ? RuntimeUpdatePath::ComponentType::kArrayIndex + : RuntimeUpdatePath::ComponentType::kFieldName); } else { // We are traversing path components that do not exist in our document. Any update modifier // that creates new path components (i.e., any modifiers that return true for @@ -148,15 +152,15 @@ void applyChild(const UpdateNode& child, if (!updateNodeApplyParams->pathToCreate->empty()) { updateNodeApplyParams->pathToCreate->removeLastPart(); } else { - updateNodeApplyParams->pathTaken->removeLastPart(); + updateNodeApplyParams->pathTaken->popBack(); } // If the child is an internal node, it may have created 'pathToCreate' and moved 'pathToCreate' // to the end of 'pathTaken'. We should advance 'element' to the end of 'pathTaken'. - if (updateNodeApplyParams->pathTaken->numParts() > pathTakenSizeBefore) { - for (auto i = pathTakenSizeBefore; i < updateNodeApplyParams->pathTaken->numParts(); ++i) { - applyParams->element = - getChild(applyParams->element, updateNodeApplyParams->pathTaken->getPart(i)); + if (updateNodeApplyParams->pathTaken->size() > pathTakenSizeBefore) { + for (auto i = pathTakenSizeBefore; i < updateNodeApplyParams->pathTaken->size(); ++i) { + applyParams->element = getChild( + applyParams->element, updateNodeApplyParams->pathTaken->fieldRef().getPart(i)); invariant(applyParams->element.ok()); } } else if (!updateNodeApplyParams->pathToCreate->empty()) { @@ -168,16 +172,22 @@ void applyChild(const UpdateNode& child, getChild(applyParams->element, updateNodeApplyParams->pathToCreate->getPart(0)); if (childElement.ok()) { applyParams->element = childElement; - updateNodeApplyParams->pathTaken->appendPart( - updateNodeApplyParams->pathToCreate->getPart(0)); + updateNodeApplyParams->pathTaken->append( + updateNodeApplyParams->pathToCreate->getPart(0), + applyParams->element.getType() == BSONType::Array + ? RuntimeUpdatePath::ComponentType::kArrayIndex + : RuntimeUpdatePath::ComponentType::kFieldName); // Either the path was fully created or not created at all. for (size_t i = 1; i < updateNodeApplyParams->pathToCreate->numParts(); ++i) { + const BSONType parentType = applyParams->element.getType(); applyParams->element = getChild(applyParams->element, updateNodeApplyParams->pathToCreate->getPart(i)); invariant(applyParams->element.ok()); - updateNodeApplyParams->pathTaken->appendPart( - updateNodeApplyParams->pathToCreate->getPart(i)); + updateNodeApplyParams->pathTaken->append( + updateNodeApplyParams->pathToCreate->getPart(i), + parentType == BSONType::Array ? RuntimeUpdatePath::ComponentType::kArrayIndex + : RuntimeUpdatePath::ComponentType::kFieldName); } updateNodeApplyParams->pathToCreate->clear(); @@ -405,7 +415,6 @@ UpdateExecutor::ApplyResult UpdateObjectNode::apply( auto applyResult = ApplyResult::noopResult(); for (const auto& pair : _children) { - // If this child has the same field name as the positional child, they must be merged and // applied. if (applyPositional && pair.first == applyParams.matchedField) { @@ -413,20 +422,21 @@ UpdateExecutor::ApplyResult UpdateObjectNode::apply( // Check if we have stored the result of merging the positional child with this child. auto mergedChild = _mergedChildrenCache.find(pair.first); if (mergedChild == _mergedChildrenCache.end()) { - - // The full path to the merged field is required for error reporting. + // The full path to the merged field is required for error reporting. In order to + // modify the 'pathTaken' FieldRef, we need a (mutable) copy of it. + FieldRef pathTakenFieldRefCopy(updateNodeApplyParams.pathTaken->fieldRef()); for (size_t i = 0; i < updateNodeApplyParams.pathToCreate->numParts(); ++i) { - updateNodeApplyParams.pathTaken->appendPart( + pathTakenFieldRefCopy.appendPart( updateNodeApplyParams.pathToCreate->getPart(i)); } - updateNodeApplyParams.pathTaken->appendPart(applyParams.matchedField); - auto insertResult = _mergedChildrenCache.emplace(std::make_pair( - pair.first, - UpdateNode::createUpdateNodeByMerging( - *_positionalChild, *pair.second, updateNodeApplyParams.pathTaken.get()))); + pathTakenFieldRefCopy.appendPart(applyParams.matchedField); + auto insertResult = _mergedChildrenCache.emplace( + std::make_pair(pair.first, + UpdateNode::createUpdateNodeByMerging( + *_positionalChild, *pair.second, &pathTakenFieldRefCopy))); for (FieldIndex i = 0; i < updateNodeApplyParams.pathToCreate->numParts() + 1; ++i) { - updateNodeApplyParams.pathTaken->removeLastPart(); + pathTakenFieldRefCopy.removeLastPart(); } invariant(insertResult.second); mergedChild = insertResult.first; diff --git a/src/mongo/db/update/update_tree_executor.h b/src/mongo/db/update/update_tree_executor.h index e04eef0a6d4..fff31d4935f 100644 --- a/src/mongo/db/update/update_tree_executor.h +++ b/src/mongo/db/update/update_tree_executor.h @@ -33,6 +33,7 @@ #include "mongo/db/update/update_node.h" #include "mongo/db/update/update_object_node.h" +#include "mongo/db/update/v1_log_builder.h" namespace mongo { @@ -43,19 +44,11 @@ public: ApplyResult applyUpdate(ApplyParams applyParams) const final { mutablebson::Document logDocument; - boost::optional<LogBuilder> optLogBuilder; - const bool generateOplogEntry = - applyParams.logMode != ApplyParams::LogMode::kDoNotGenerateOplogEntry; - if (generateOplogEntry) { - optLogBuilder.emplace(logDocument.root()); - } + boost::optional<V1LogBuilder> optLogBuilder; UpdateNode::UpdateNodeApplyParams updateNodeApplyParams; - updateNodeApplyParams.logBuilder = optLogBuilder.get_ptr(); - - auto ret = _updateTree->apply(applyParams, updateNodeApplyParams); - if (generateOplogEntry) { + if (applyParams.logMode != ApplyParams::LogMode::kDoNotGenerateOplogEntry) { // In versions since 3.6, the absence of a $v field indicates either a // replacement-style update or a "classic" modifier-style update. // @@ -66,10 +59,17 @@ public: // it because: // (a) It avoids an unnecessary oplog format change. // (b) It is easy to distinguish from $v: 2 delta-style oplog entries. - invariant(optLogBuilder->setVersion(UpdateOplogEntryVersion::kUpdateNodeV1)); + const bool includeVersionField = true; + + optLogBuilder.emplace(logDocument.root(), includeVersionField); + updateNodeApplyParams.logBuilder = optLogBuilder.get_ptr(); + } + + auto ret = _updateTree->apply(applyParams, updateNodeApplyParams); - invariant(ret.oplogEntry.isEmpty()); - ret.oplogEntry = logDocument.getObject(); + invariant(ret.oplogEntry.isEmpty()); + if (auto logBuilder = updateNodeApplyParams.logBuilder) { + ret.oplogEntry = logBuilder->serialize(); } return ret; diff --git a/src/mongo/db/update/log_builder.cpp b/src/mongo/db/update/v1_log_builder.cpp index e148f2df4be..ef3ff88abff 100644 --- a/src/mongo/db/update/log_builder.cpp +++ b/src/mongo/db/update/v1_log_builder.cpp @@ -27,7 +27,10 @@ * it in the license file. */ -#include "mongo/db/update/log_builder.h" +#include "mongo/db/update/v1_log_builder.h" + +#include "mongo/db/update/runtime_update_path.h" +#include "mongo/db/update/update_oplog_entry_serialization.h" #include "mongo/util/str.h" namespace mongo { @@ -39,7 +42,22 @@ const char kSet[] = "$set"; const char kUnset[] = "$unset"; } // namespace -inline Status LogBuilder::addToSection(Element newElt, Element* section, const char* sectionName) { +V1LogBuilder::V1LogBuilder(mutablebson::Element logRoot, bool includeVersionField) + : _logRoot(logRoot), + _setAccumulator(_logRoot.getDocument().end()), + _unsetAccumulator(_setAccumulator) { + invariant(logRoot.isType(mongo::Object)); + invariant(!logRoot.hasChildren()); + + if (includeVersionField) { + auto version = logRoot.getDocument().makeElementInt( + kUpdateOplogEntryVersionFieldName, + static_cast<int>(UpdateOplogEntryVersion::kUpdateNodeV1)); + invariant(_logRoot.pushFront(version).isOK()); + } +} + +Status V1LogBuilder::addToSection(Element newElt, Element* section, const char* sectionName) { // If we don't already have this section, try to create it now. if (!section->ok()) { mutablebson::Document& doc = _logRoot.getDocument(); @@ -51,7 +69,7 @@ inline Status LogBuilder::addToSection(Element newElt, Element* section, const c const Element newElement = doc.makeElementObject(sectionName); if (!newElement.ok()) return Status(ErrorCodes::InternalError, - "LogBuilder: failed to construct Object Element for $set/$unset"); + "V1LogBuilder: failed to construct Object Element for $set/$unset"); // Enqueue the new section under the root, and record it as our out parameter. Status result = _logRoot.pushBack(newElement); @@ -68,11 +86,11 @@ inline Status LogBuilder::addToSection(Element newElt, Element* section, const c return section->pushBack(newElt); } -Status LogBuilder::addToSets(Element elt) { +Status V1LogBuilder::addToSets(Element elt) { return addToSection(elt, &_setAccumulator, kSet); } -Status LogBuilder::addToSetsWithNewFieldName(StringData name, const mutablebson::Element val) { +Status V1LogBuilder::addToSetsWithNewFieldName(StringData name, const mutablebson::Element val) { mutablebson::Element elemToSet = _logRoot.getDocument().makeElementWithNewFieldName(name, val); if (!elemToSet.ok()) return Status(ErrorCodes::InternalError, @@ -83,7 +101,7 @@ Status LogBuilder::addToSetsWithNewFieldName(StringData name, const mutablebson: return addToSets(elemToSet); } -Status LogBuilder::addToSetsWithNewFieldName(StringData name, const BSONElement& val) { +Status V1LogBuilder::addToSetsWithNewFieldName(StringData name, const BSONElement& val) { mutablebson::Element elemToSet = _logRoot.getDocument().makeElementWithNewFieldName(name, val); if (!elemToSet.ok()) return Status(ErrorCodes::InternalError, @@ -94,17 +112,7 @@ Status LogBuilder::addToSetsWithNewFieldName(StringData name, const BSONElement& return addToSets(elemToSet); } -Status LogBuilder::addToSets(StringData name, const SafeNum& val) { - mutablebson::Element elemToSet = _logRoot.getDocument().makeElementSafeNum(name, val); - if (!elemToSet.ok()) - return Status(ErrorCodes::InternalError, - str::stream() << "Could not create new '" << name << "' SafeNum from " - << val.debugString()); - - return addToSets(elemToSet); -} - -Status LogBuilder::addToUnsets(StringData path) { +Status V1LogBuilder::addToUnsets(StringData path) { mutablebson::Element logElement = _logRoot.getDocument().makeElementBool(path, true); if (!logElement.ok()) return Status(ErrorCodes::InternalError, @@ -113,17 +121,23 @@ Status LogBuilder::addToUnsets(StringData path) { return addToSection(logElement, &_unsetAccumulator, kUnset); } -Status LogBuilder::setVersion(UpdateOplogEntryVersion oplogVersion) { - if (_version.ok()) { - return Status(ErrorCodes::IllegalOperation, "LogBuilder: Invalid attempt to set $v twice."); - } +Status V1LogBuilder::logUpdatedField(const RuntimeUpdatePath& path, mutablebson::Element elt) { + return addToSetsWithNewFieldName(path.fieldRef().dottedField(), elt); +} - mutablebson::Document& doc = _logRoot.getDocument(); - _version = - doc.makeElementInt(kUpdateOplogEntryVersionFieldName, static_cast<int>(oplogVersion)); +Status V1LogBuilder::logCreatedField(const RuntimeUpdatePath& path, + int idxOfFirstNewComponent, + mutablebson::Element elt) { + return addToSetsWithNewFieldName(path.fieldRef().dottedField(), elt); +} - dassert(_logRoot[kUpdateOplogEntryVersionFieldName] == doc.end()); +Status V1LogBuilder::logCreatedField(const RuntimeUpdatePath& path, + int idxOfFirstNewComponent, + BSONElement elt) { + return addToSetsWithNewFieldName(path.fieldRef().dottedField(), elt); +} - return _logRoot.pushFront(_version); +Status V1LogBuilder::logDeletedField(const RuntimeUpdatePath& path) { + return addToUnsets(path.fieldRef().dottedField()); } } // namespace mongo diff --git a/src/mongo/db/update/log_builder.h b/src/mongo/db/update/v1_log_builder.h index 8f01f9e0f4a..c532b007ab8 100644 --- a/src/mongo/db/update/log_builder.h +++ b/src/mongo/db/update/v1_log_builder.h @@ -31,48 +31,69 @@ #include "mongo/base/status.h" #include "mongo/bson/mutable/document.h" -#include "mongo/db/update/update_oplog_entry_version.h" +#include "mongo/db/update/log_builder_interface.h" namespace mongo { +class RuntimeUpdatePath; /** * LogBuilder abstracts away some of the details of producing a properly constructed oplog $v:1 * modifier-style update entry. It manages separate regions into which it accumulates $set and * $unset operations. */ -class LogBuilder { +class V1LogBuilder : public LogBuilderInterface { public: - /** Construct a new LogBuilder. Log entries will be recorded as new children under the - * 'logRoot' Element, which must be of type mongo::Object and have no children. + /** + * Construct a new LogBuilder. Log entries will be recorded as new children under the + * 'logRoot' Element, which must be of type mongo::Object and have no children. + * + * The 'includeVersionField' indicates whether the generated log entry should include a $v + * (version) field. */ - inline LogBuilder(mutablebson::Element logRoot) - : _logRoot(logRoot), - _setAccumulator(_logRoot.getDocument().end()), - _unsetAccumulator(_setAccumulator), - _version(_setAccumulator) { - dassert(logRoot.isType(mongo::Object)); - dassert(!logRoot.hasChildren()); - } + V1LogBuilder(mutablebson::Element logRoot, bool includeVersionField = false); - /** Return the Document to which the logging root belongs. */ + /** + * Overloads from LogBuilderInterface. Each of these methods logs a modification to the document + * in _logRoot. The field name given in the mutablebson element or BSONElement is ignored + * and the 'path' argument is used instead. + */ + Status logUpdatedField(const RuntimeUpdatePath& path, mutablebson::Element elt) override; + + /** + * Logs the creation of a new field. The 'idxOfFirstNewComponent' parameter is unused in this + * implementation. + */ + Status logCreatedField(const RuntimeUpdatePath& path, + int idxOfFirstNewComponent, + mutablebson::Element elt) override; + Status logCreatedField(const RuntimeUpdatePath& path, + int idxOfFirstNewComponent, + BSONElement elt) override; + + Status logDeletedField(const RuntimeUpdatePath& path) override; + + /** + * Return the Document to which the logging root belongs. + */ inline mutablebson::Document& getDocument() { return _logRoot.getDocument(); } - /** Add the given Element as a new entry in the '$set' section of the log. If a $set - * section does not yet exist, it will be created. If this LogBuilder is currently - * configured to contain an object replacement, the request to add to the $set section - * will return an Error. + /** + * Produces a BSON object representing this update using the modifier syntax which can be + * stored in the oplog. */ - Status addToSets(mutablebson::Element elt); + BSONObj serialize() const override { + return _logRoot.getDocument().getObject(); + } +private: /** - * Convenience method which calls addToSets after - * creating a new Element to wrap the SafeNum value. - * - * If any problem occurs then the operation will stop and return that error Status. + * Add the given Element as a new entry in the '$set' section of the log. If a $set section + * does not yet exist, it will be created. If this LogBuilder is currently configured to + * contain an object replacement, the request to add to the $set section will return an Error. */ - Status addToSets(StringData name, const SafeNum& val); + Status addToSets(mutablebson::Element elt); /** * Convenience method which calls addToSets after @@ -90,27 +111,20 @@ public: */ Status addToSetsWithNewFieldName(StringData name, const BSONElement& val); - /** Add the given path as a new entry in the '$unset' section of the log. If an - * '$unset' section does not yet exist, it will be created. If this LogBuilder is - * currently configured to contain an object replacement, the request to add to the - * $unset section will return an Error. - */ - Status addToUnsets(StringData path); - /** - * Add a "$v" field to the log. Fails if there is already a "$v" field. + * Add the given path as a new entry in the '$unset' section of the log. If an '$unset' section + * does not yet exist, it will be created. If this LogBuilder is currently configured to + * contain an object replacement, the request to add to the $unset section will return an + * Error. */ - Status setVersion(UpdateOplogEntryVersion); + Status addToUnsets(StringData path); -private: - inline Status addToSection(mutablebson::Element newElt, - mutablebson::Element* section, - const char* sectionName); + Status addToSection(mutablebson::Element newElt, + mutablebson::Element* section, + const char* sectionName); mutablebson::Element _logRoot; mutablebson::Element _setAccumulator; mutablebson::Element _unsetAccumulator; - mutablebson::Element _version; }; - } // namespace mongo diff --git a/src/mongo/db/update/v1_log_builder_test.cpp b/src/mongo/db/update/v1_log_builder_test.cpp new file mode 100644 index 00000000000..b760c7bbc68 --- /dev/null +++ b/src/mongo/db/update/v1_log_builder_test.cpp @@ -0,0 +1,159 @@ +/** + * Copyright (C) 2018-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * 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 + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * 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 Server Side 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/v1_log_builder.h" + +#include "mongo/base/status.h" +#include "mongo/bson/bsonobj.h" +#include "mongo/bson/mutable/mutable_bson_test_utils.h" +#include "mongo/db/json.h" +#include "mongo/db/update/runtime_update_path.h" +#include "mongo/unittest/unittest.h" +#include "mongo/util/safe_num.h" + +namespace mongo { +namespace { +namespace mmb = mongo::mutablebson; + +/** + * Given a FieldRef, creates a RuntimeUpdatePath based on it, assuming that every component is a + * field name. This is safe to do while testing the V1 log builder, since it ignores the types of + * the path given entirely. + */ +RuntimeUpdatePath makeRuntimeUpdatePathAssumeAllComponentsFieldNames(StringData path) { + FieldRef fieldRef(path); + return RuntimeUpdatePath( + std::move(fieldRef), + std::vector<RuntimeUpdatePath::ComponentType>( + fieldRef.numParts(), RuntimeUpdatePath::ComponentType::kFieldName)); +} + +TEST(V1LogBuilder, UpdateFieldMutableBson) { + mmb::Document doc; + V1LogBuilder lb(doc.root()); + + const mmb::Element elt_ab = doc.makeElementInt("a.b", 1); + ASSERT_TRUE(elt_ab.ok()); + ASSERT_OK( + lb.logUpdatedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("a.b"), elt_ab)); + + ASSERT_BSONOBJ_BINARY_EQ(mongo::fromjson("{ $set : { 'a.b' : 1 } }"), lb.serialize()); +} + +TEST(V1LogBuilder, CreateField) { + mmb::Document doc; + V1LogBuilder lb(doc.root()); + + const mmb::Element elt_ab = doc.makeElementInt("a.b", 1); + ASSERT_TRUE(elt_ab.ok()); + ASSERT_OK(lb.logCreatedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("a.b"), + 0, // idxOfFirstNewComponent (unused) + elt_ab)); + + ASSERT_BSONOBJ_BINARY_EQ(mongo::fromjson("{ $set : { 'a.b' : 1 } }"), lb.serialize()); +} + +TEST(V1LogBuilder, CreateFieldBSONElt) { + mmb::Document doc; + V1LogBuilder lb(doc.root()); + + BSONObj storage = BSON("a" << 1); + ASSERT_OK(lb.logCreatedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("a.b"), + 0, // idxOfFirstNewComponent (unused) + storage.firstElement())); + + ASSERT_BSONOBJ_BINARY_EQ(mongo::fromjson("{ $set : { 'a.b' : 1 } }"), lb.serialize()); +} + +TEST(V1LogBuilder, AddOneToUnset) { + mmb::Document doc; + V1LogBuilder lb(doc.root()); + ASSERT_OK(lb.logDeletedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("x.y"))); + ASSERT_EQUALS(mongo::fromjson("{ $unset : { 'x.y' : true } }"), doc); +} +TEST(V1LogBuilder, AddOneToEach) { + mmb::Document doc; + V1LogBuilder lb(doc.root()); + + const mmb::Element elt_ab = doc.makeElementInt("", 1); + ASSERT_TRUE(elt_ab.ok()); + ASSERT_OK( + lb.logUpdatedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("a.b"), elt_ab)); + + const mmb::Element elt_cd = doc.makeElementInt("", 2); + ASSERT_TRUE(elt_cd.ok()); + + ASSERT_OK(lb.logCreatedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("c.d"), + 0, // idxOfCreatedComponent (unused) + elt_cd)); + + ASSERT_OK(lb.logDeletedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("x.y"))); + + ASSERT_EQUALS(mongo::fromjson("{ " + " $set : { 'a.b' : 1, 'c.d': 2 }, " + " $unset : { 'x.y' : true } " + "}"), + doc); +} +TEST(V1LogBuilder, VerifySetsAreGrouped) { + mmb::Document doc; + V1LogBuilder lb(doc.root()); + + const mmb::Element elt_ab = doc.makeElementInt("a.b", 1); + ASSERT_TRUE(elt_ab.ok()); + ASSERT_OK( + lb.logUpdatedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("a.b"), elt_ab)); + + const mmb::Element elt_xy = doc.makeElementInt("x.y", 1); + ASSERT_TRUE(elt_xy.ok()); + ASSERT_OK( + lb.logUpdatedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("x.y"), elt_xy)); + + ASSERT_EQUALS(mongo::fromjson("{ $set : {" + " 'a.b' : 1, " + " 'x.y' : 1 " + "} }"), + doc); +} + +TEST(V1LogBuilder, VerifyUnsetsAreGrouped) { + mmb::Document doc; + V1LogBuilder lb(doc.root()); + + ASSERT_OK(lb.logDeletedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("a.b"))); + ASSERT_OK(lb.logDeletedField(makeRuntimeUpdatePathAssumeAllComponentsFieldNames("x.y"))); + + ASSERT_EQUALS(mongo::fromjson("{ $unset : {" + " 'a.b' : true, " + " 'x.y' : true " + "} }"), + doc); +} +} // namespace +} // namespace mongo |