summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/mongo/db/update/document_diff_applier_test.cpp129
-rw-r--r--src/mongo/db/update/document_diff_calculator.cpp33
-rw-r--r--src/mongo/db/update/document_diff_calculator_test.cpp33
-rw-r--r--src/mongo/db/update/document_diff_serialization.cpp124
-rw-r--r--src/mongo/db/update/document_diff_serialization.h318
-rw-r--r--src/mongo/db/update/document_diff_serialization_test.cpp239
-rw-r--r--src/mongo/db/update/pipeline_executor_test.cpp23
7 files changed, 526 insertions, 373 deletions
diff --git a/src/mongo/db/update/document_diff_applier_test.cpp b/src/mongo/db/update/document_diff_applier_test.cpp
index 6f3913ddb2f..aeadcb47060 100644
--- a/src/mongo/db/update/document_diff_applier_test.cpp
+++ b/src/mongo/db/update/document_diff_applier_test.cpp
@@ -62,7 +62,7 @@ TEST(DiffApplierTest, DeleteSimple) {
builder.addDelete("f2");
builder.addDelete("f3");
- auto diff = builder.release();
+ auto diff = builder.serialize();
checkDiff(preImage, BSON("foo" << 2), diff);
}
@@ -71,11 +71,13 @@ TEST(DiffApplierTest, InsertSimple) {
const BSONObj preImage(BSON("f1" << 1 << "foo" << 2 << "f2" << 3));
const BSONObj storage(BSON("a" << 1 << "b" << 2));
+ StringData newField = "newField";
+
DocumentDiffBuilder builder;
builder.addInsert("f1", storage["a"]);
- builder.addInsert("newField", storage["b"]);
+ builder.addInsert(newField, storage["b"]);
- auto diff = builder.release();
+ auto diff = builder.serialize();
checkDiff(preImage, BSON("foo" << 2 << "f2" << 3 << "f1" << 1 << "newField" << 2), diff);
}
@@ -83,11 +85,13 @@ TEST(DiffApplierTest, UpdateSimple) {
const BSONObj preImage(BSON("f1" << 0 << "foo" << 2 << "f2" << 3));
const BSONObj storage(BSON("a" << 1 << "b" << 2));
+ StringData newField = "newField";
+
DocumentDiffBuilder builder;
builder.addUpdate("f1", storage["a"]);
- builder.addUpdate("newField", storage["b"]);
+ builder.addUpdate(newField, storage["b"]);
- auto diff = builder.release();
+ auto diff = builder.serialize();
checkDiff(preImage, BSON("f1" << 1 << "foo" << 2 << "f2" << 3 << "newField" << 2), diff);
}
@@ -98,13 +102,13 @@ TEST(DiffApplierTest, SubObjDiffSimple) {
const BSONObj storage(BSON("a" << 1 << "b" << 2));
DocumentDiffBuilder builder;
{
- DocumentDiffBuilder sub(builder.startSubObjDiff("obj"));
- sub.addDelete("dField");
- sub.addInsert("iField", storage["a"]);
- sub.addUpdate("uField", storage["b"]);
+ auto subBuilderGuard = builder.startSubObjDiff("obj");
+ subBuilderGuard.builder()->addDelete("dField");
+ subBuilderGuard.builder()->addInsert("iField", storage["a"]);
+ subBuilderGuard.builder()->addUpdate("uField", storage["b"]);
}
- auto diff = builder.release();
+ auto diff = builder.serialize();
checkDiff(
preImage, BSON("obj" << BSON("uField" << 2 << "iField" << 1) << "otherField" << 0), diff);
}
@@ -113,14 +117,16 @@ TEST(DiffApplierTest, SubArrayDiffSimpleWithAppend) {
const BSONObj preImage(BSON("arr" << BSON_ARRAY(999 << 999 << 999 << 999)));
const BSONObj storage(BSON("a" << 1 << "b" << 2));
+ StringData arr = "arr";
+
DocumentDiffBuilder builder;
{
- ArrayDiffBuilder sub(builder.startSubArrDiff("arr"));
- sub.addUpdate(1, storage["a"]);
- sub.addUpdate(4, storage["b"]);
+ auto subBuilderGuard = builder.startSubArrDiff(arr);
+ subBuilderGuard.builder()->addUpdate(1, storage["a"]);
+ subBuilderGuard.builder()->addUpdate(4, storage["b"]);
}
- auto diff = builder.release();
+ auto diff = builder.serialize();
checkDiff(preImage, BSON("arr" << BSON_ARRAY(999 << 1 << 999 << 999 << 2)), diff);
}
@@ -129,14 +135,16 @@ TEST(DiffApplierTest, SubArrayDiffSimpleWithTruncate) {
const BSONObj preImage(BSON("arr" << BSON_ARRAY(999 << 999 << 999 << 999)));
const BSONObj storage(BSON("a" << 1 << "b" << 2));
+ StringData arr = "arr";
+
DocumentDiffBuilder builder;
{
- ArrayDiffBuilder sub(builder.startSubArrDiff("arr"));
- sub.addUpdate(1, storage["a"]);
- sub.setResize(3);
+ auto subBuilderGuard = builder.startSubArrDiff(arr);
+ subBuilderGuard.builder()->addUpdate(1, storage["a"]);
+ subBuilderGuard.builder()->setResize(3);
}
- auto diff = builder.release();
+ auto diff = builder.serialize();
checkDiff(preImage, BSON("arr" << BSON_ARRAY(999 << 1 << 999)), diff);
}
@@ -144,29 +152,32 @@ TEST(DiffApplierTest, SubArrayDiffSimpleWithNullPadding) {
const BSONObj preImage(BSON("arr" << BSON_ARRAY(0)));
BSONObj storage(BSON("a" << 1));
+ StringData arr = "arr";
+
DocumentDiffBuilder builder;
{
- ArrayDiffBuilder sub(builder.startSubArrDiff("arr"));
- sub.addUpdate(3, storage["a"]);
+ auto subBuilderGuard = builder.startSubArrDiff(arr);
+ subBuilderGuard.builder()->addUpdate(3, storage["a"]);
}
- auto diff = builder.release();
+ auto diff = builder.serialize();
checkDiff(preImage, BSON("arr" << BSON_ARRAY(0 << NullLabeler{} << NullLabeler{} << 1)), diff);
}
TEST(DiffApplierTest, NestedSubObjUpdateScalar) {
BSONObj storage(BSON("a" << 1));
+ StringData subObj = "subObj";
DocumentDiffBuilder builder;
{
- DocumentDiffBuilder sub(builder.startSubObjDiff("subObj"));
+ auto subBuilderGuard = builder.startSubObjDiff(subObj);
{
- DocumentDiffBuilder subSub(sub.startSubObjDiff("subObj"));
- subSub.addUpdate("a", storage["a"]);
+ auto subSubBuilderGuard = subBuilderGuard.builder()->startSubObjDiff(subObj);
+ subSubBuilderGuard.builder()->addUpdate("a", storage["a"]);
}
}
- auto diff = builder.release();
+ auto diff = builder.serialize();
// Check the case where the object matches the structure we expect.
BSONObj preImage(fromjson("{subObj: {subObj: {a: 0}}}"));
@@ -200,33 +211,38 @@ TEST(DiffApplierTest, UpdateArrayOfObjectsSubDiff) {
BSONObj storage(
BSON("uFieldNew" << 999 << "newObj" << BSON("x" << 1) << "a" << 1 << "b" << 2 << "c" << 3));
+ StringData arr = "arr";
+ StringData dFieldA = "dFieldA";
+ StringData dFieldB = "dFieldB";
+ StringData uField = "uField";
+
DocumentDiffBuilder builder;
{
- builder.addDelete("dFieldA");
- builder.addDelete("dFieldB");
- builder.addUpdate("uField", storage["uFieldNew"]);
+ builder.addDelete(dFieldA);
+ builder.addDelete(dFieldB);
+ builder.addUpdate(uField, storage["uFieldNew"]);
- ArrayDiffBuilder subArr(builder.startSubArrDiff("arr"));
+ auto subBuilderGuard = builder.startSubArrDiff(arr);
{
{
- DocumentDiffBuilder subObjBuilder(subArr.startSubObjDiff(1));
- subObjBuilder.addUpdate("a", storage["a"]);
+ auto subSubBuilderGuard = subBuilderGuard.builder()->startSubObjDiff(1);
+ subSubBuilderGuard.builder()->addUpdate("a", storage["a"]);
}
{
- DocumentDiffBuilder subObjBuilder(subArr.startSubObjDiff(2));
- subObjBuilder.addUpdate("b", storage["b"]);
+ auto subSubBuilderGuard = subBuilderGuard.builder()->startSubObjDiff(2);
+ subSubBuilderGuard.builder()->addUpdate("b", storage["b"]);
}
{
- DocumentDiffBuilder subObjBuilder(subArr.startSubObjDiff(3));
- subObjBuilder.addUpdate("c", storage["c"]);
+ auto subSubBuilderGuard = subBuilderGuard.builder()->startSubObjDiff(3);
+ subSubBuilderGuard.builder()->addUpdate("c", storage["c"]);
}
- subArr.addUpdate(4, storage["newObj"]);
+ subBuilderGuard.builder()->addUpdate(4, storage["newObj"]);
}
}
- auto diff = builder.release();
+ auto diff = builder.serialize();
// Case where the object matches the structure we expect.
BSONObj preImage(
@@ -274,17 +290,18 @@ TEST(DiffApplierTest, UpdateArrayOfObjectsSubDiff) {
// Case where an array diff rewrites several non contiguous indices which are objects.
TEST(DiffApplierTest, UpdateArrayOfObjectsWithUpdateOperationNonContiguous) {
BSONObj storage(BSON("dummyA" << 997 << "dummyB" << BSON("newVal" << 998) << "dummyC" << 999));
+ StringData arr = "arr";
DocumentDiffBuilder builder;
{
- ArrayDiffBuilder subArr(builder.startSubArrDiff("arr"));
+ auto subBuilderGuard = builder.startSubArrDiff(arr);
{
- subArr.addUpdate(1, storage["dummyA"]);
- subArr.addUpdate(2, storage["dummyB"]);
- subArr.addUpdate(5, storage["dummyC"]);
+ subBuilderGuard.builder()->addUpdate(1, storage["dummyA"]);
+ subBuilderGuard.builder()->addUpdate(2, storage["dummyB"]);
+ subBuilderGuard.builder()->addUpdate(5, storage["dummyC"]);
}
}
- auto diff = builder.release();
+ auto diff = builder.serialize();
// Case where the object matches the structure we expect.
BSONObj preImage(fromjson("{arr: [null, {}, {}, {}, {}, {}]}"));
@@ -342,8 +359,34 @@ TEST(DiffApplierTest, EmptyDiff) {
}
TEST(DiffApplierTest, ArrayDiffAtTop) {
- BSONObj arrDiff = fromjson("{a: true, l: 5, '0': {d: false}}");
+ BSONObj arrDiff = fromjson("{a: true, l: 5, 'd0': false}");
ASSERT_THROWS_CODE(applyDiff(BSONObj(), arrDiff), DBException, 4770503);
}
+
+TEST(DiffApplierTest, DuplicateFieldNames) {
+ // Within the same update type.
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{d: {a: false, b: false, a: false}}")),
+ DBException,
+ 4728000);
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{u: {f1: 3, f1: 4}}")), DBException, 4728000);
+ ASSERT_THROWS_CODE(
+ applyDiff(BSONObj(), fromjson("{i: {a: {}, a: null}}")), DBException, 4728000);
+ ASSERT_THROWS_CODE(
+ applyDiff(BSONObj(), fromjson("{s: {a: {d: {p: false}}, a: {a: true, d: {p: false}}}}")),
+ DBException,
+ 4728000);
+
+ // Across update types.
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{d: {b: false}, i: {a: {}, b: null}}")),
+ DBException,
+ 4728000);
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{u: {b: false}, i: {a: {}, b: null}}")),
+ DBException,
+ 4728000);
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{u: {a: {}}, s: {a: {d : {k: false}}}}")),
+ DBException,
+ 4728000);
+}
+
} // namespace
} // namespace mongo::doc_diff
diff --git a/src/mongo/db/update/document_diff_calculator.cpp b/src/mongo/db/update/document_diff_calculator.cpp
index 7019d4a675c..512087d1a67 100644
--- a/src/mongo/db/update/document_diff_calculator.cpp
+++ b/src/mongo/db/update/document_diff_calculator.cpp
@@ -50,7 +50,7 @@ bool computeArrayDiff(const BSONObj& pre,
size_t nFieldsInPostArray = 0;
for (; preItr.more() && postItr.more(); ++preItr, ++postItr, ++nFieldsInPostArray) {
// Bailout if the generated diff so far is larger than the 'post' object.
- if (postObjSize < diffBuilder->computeApproxSize()) {
+ if (postObjSize < diffBuilder->getObjSize()) {
return false;
}
if (!(*preItr).binaryEqual(*postItr)) {
@@ -76,7 +76,7 @@ bool computeArrayDiff(const BSONObj& pre,
if (preItr.more()) {
diffBuilder->setResize(nFieldsInPostArray);
}
- return postObjSize > diffBuilder->computeApproxSize();
+ return postObjSize > diffBuilder->getObjSize();
}
bool computeDocDiff(const BSONObj& pre,
@@ -91,7 +91,7 @@ bool computeDocDiff(const BSONObj& pre,
auto postVal = *postItr;
// Bailout if the generated diff so far is larger than the 'post' object.
- if (postObjSize < diffBuilder->computeApproxSize()) {
+ if (postObjSize < diffBuilder->getObjSize()) {
return false;
}
if (preVal.fieldNameStringData() == postVal.fieldNameStringData()) {
@@ -134,7 +134,7 @@ bool computeDocDiff(const BSONObj& pre,
for (auto&& deleteField : deletes) {
diffBuilder->addDelete(deleteField);
}
- return postObjSize > diffBuilder->computeApproxSize();
+ return postObjSize > diffBuilder->getObjSize();
}
template <class DiffBuilder, class T>
@@ -143,19 +143,19 @@ void calculateSubDiffHelper(const BSONElement& preVal,
T fieldIdentifier,
DiffBuilder* builder) {
if (preVal.type() == BSONType::Object) {
- auto subDiffBuilder = builder->startSubObjDiff(fieldIdentifier);
- const auto hasSubDiff =
- computeDocDiff(preVal.embeddedObject(), postVal.embeddedObject(), &subDiffBuilder);
+ auto subDiffBuilderGuard = builder->startSubObjDiff(fieldIdentifier);
+ const auto hasSubDiff = computeDocDiff(
+ preVal.embeddedObject(), postVal.embeddedObject(), subDiffBuilderGuard.builder());
if (!hasSubDiff) {
- subDiffBuilder.abandon();
+ subDiffBuilderGuard.abandon();
builder->addUpdate(fieldIdentifier, postVal);
}
} else {
- auto subDiffBuilder = builder->startSubArrDiff(fieldIdentifier);
- const auto hasSubDiff =
- computeArrayDiff(preVal.embeddedObject(), postVal.embeddedObject(), &subDiffBuilder);
+ auto subDiffBuilderGuard = builder->startSubArrDiff(fieldIdentifier);
+ const auto hasSubDiff = computeArrayDiff(
+ preVal.embeddedObject(), postVal.embeddedObject(), subDiffBuilderGuard.builder());
if (!hasSubDiff) {
- subDiffBuilder.abandon();
+ subDiffBuilderGuard.abandon();
builder->addUpdate(fieldIdentifier, postVal);
}
}
@@ -164,7 +164,12 @@ void calculateSubDiffHelper(const BSONElement& preVal,
boost::optional<doc_diff::Diff> computeDiff(const BSONObj& pre, const BSONObj& post) {
doc_diff::DocumentDiffBuilder diffBuilder;
- bool hasDiff = computeDocDiff(pre, post, &diffBuilder);
- return hasDiff ? boost::optional<doc_diff::Diff>(diffBuilder.release()) : boost::none;
+ if (computeDocDiff(pre, post, &diffBuilder)) {
+ auto diff = diffBuilder.serialize();
+ if (diff.objsize() < post.objsize()) {
+ return diff;
+ }
+ }
+ return {};
}
} // namespace mongo::doc_diff
diff --git a/src/mongo/db/update/document_diff_calculator_test.cpp b/src/mongo/db/update/document_diff_calculator_test.cpp
index 707a2198a8e..055ecaa3d4d 100644
--- a/src/mongo/db/update/document_diff_calculator_test.cpp
+++ b/src/mongo/db/update/document_diff_calculator_test.cpp
@@ -128,7 +128,7 @@ TEST(DocumentDiffTest, SimpleArrayPush) {
auto diff = doc_diff::computeDiff(fromjson("{field1: 'abcd', field2: [1, 2, 3]}"),
fromjson("{field1: 'abcd', field2: [1, 2, 3, 4]}"));
ASSERT(diff);
- ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field2: {a: true, '3': {u: 4}}}}"));
+ ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field2: {a: true, 'u3': 4}}}"));
}
TEST(DocumentDiffTest, NestedArray) {
@@ -137,15 +137,15 @@ TEST(DocumentDiffTest, NestedArray) {
ASSERT(diff);
// When the sub-array delta is larger than the size of the sub-array, we record it as an update
// operation.
- ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field2: {a: true, '3': {u: [[4]]}}}}"));
+ ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field2: {a: true, 'u3': [[4]]}}}"));
diff = doc_diff::computeDiff(
fromjson("{field1: 'abcd', field2: [1, 2, 3, [1, 'longString', [2], 4, 5, 6], 5, 5, 5]}"),
fromjson("{field1: 'abcd', field2: [1, 2, 3, [1, 'longString', [4], 4], 5, 6]}"));
ASSERT(diff);
- ASSERT_BSONOBJ_BINARY_EQ(*diff,
- fromjson("{s: {field2: {a: true, l: 6, '3': {s: {a: true, l: 4, "
- "'2': {u: [4]}}}, '5': {u: 6}} }}"));
+ ASSERT_BSONOBJ_BINARY_EQ(
+ *diff,
+ fromjson("{s: {field2: {a: true, l: 6, 's3': {a: true, l: 4, 'u2': [4]}, 'u5': 6}}}"));
}
TEST(DocumentDiffTest, SubObjInSubArrayUpdateElements) {
@@ -157,9 +157,10 @@ TEST(DocumentDiffTest, SubObjInSubArrayUpdateElements) {
"5]}]}"));
ASSERT(diff);
- ASSERT_BSONOBJ_BINARY_EQ(*diff,
- fromjson("{s: {field2: {a: true, '3': {s: {s: {field3: {a: true, '2': "
- "{u: 4}, '3': {u: 3}, '4': {u: 5}} }} }} }}"));
+ ASSERT_BSONOBJ_BINARY_EQ(
+ *diff,
+ fromjson("{s: {field2: {a: true, 's3': {s: {field3: {a: true, 'u2': 4, 'u3': 3, "
+ "'u4': 5}} }} }}"));
}
TEST(DocumentDiffTest, SubObjInSubArrayDeleteElements) {
@@ -169,8 +170,7 @@ TEST(DocumentDiffTest, SubObjInSubArrayDeleteElements) {
ASSERT(diff);
ASSERT_BSONOBJ_BINARY_EQ(
*diff,
- fromjson("{s: {field2: {a: true, '3': {s: {s: {field3: {a: true, l: 4, '3': "
- "{u: 5}} }} }} }}"));
+ fromjson("{s: {field2: {a: true, 's3': {s: {field3: {a: true, l: 4, 'u3': 5}} }} }}"));
}
TEST(DocumentDiffTest, NestedSubObjs) {
@@ -192,7 +192,7 @@ TEST(DocumentDiffTest, SubArrayInSubArrayLargeDelta) {
fromjson("{field1: 'abcd', field2: [1, 2, 3, {field3: [1, 2, 3, 4, 5]}]}"));
ASSERT(diff);
ASSERT_BSONOBJ_BINARY_EQ(
- *diff, fromjson("{s: {field2: {a: true, '3': {u: {field3: [1, 2, 3, 4, 5]}}} }}"));
+ *diff, fromjson("{s: {field2: {a: true, 'u3': {field3: [1, 2, 3, 4, 5]}} }}"));
}
TEST(DocumentDiffTest, SubObjInSubArrayLargeDelta) {
@@ -200,20 +200,21 @@ TEST(DocumentDiffTest, SubObjInSubArrayLargeDelta) {
doc_diff::computeDiff(fromjson("{field1: [1, 2, 3, 4, 5, 6, {a: 1, b: 2, c: 3, d: 4}, 7]}"),
fromjson("{field1: [1, 2, 3, 4, 5, 6, {p: 1, q: 2}, 7]}"));
ASSERT(diff);
- ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field1: {a: true, '6': {u: {p: 1, q: 2} }} }}"));
+ ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field1: {a: true, 'u6': {p: 1, q: 2}} }}"));
}
TEST(DocumentDiffTest, SubObjInSubObjLargeDelta) {
auto diff = doc_diff::computeDiff(
- fromjson("{field: {p: 1, q: 2, r: {a: 1, b: 2, c: 3, 'd': 4}, s: 3}}"),
- fromjson("{field: {p: 1, q: 2, r: {p: 1, q: 2}, s: 3}}"));
+ fromjson("{field: {p: 'someString', q: 2, r: {a: 1, b: 2, c: 3, 'd': 4}, s: 3}}"),
+ fromjson("{field: {p: 'someString', q: 2, r: {p: 1, q: 2}, s: 3}}"));
ASSERT(diff);
ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field: {u: {r: {p: 1, q: 2} }} }}"));
}
TEST(DocumentDiffTest, SubArrayInSubObjLargeDelta) {
- auto diff = doc_diff::computeDiff(fromjson("{field: {p: 1, q: 2, r: [1, 3, 4, 5], s: 3}}"),
- fromjson("{field: {p: 1, q: 2, r: [1, 2, 3, 4], s: 3}}"));
+ auto diff =
+ doc_diff::computeDiff(fromjson("{field: {p: 'someString', q: 2, r: [1, 3, 4, 5], s: 3}}"),
+ fromjson("{field: {p: 'someString', q: 2, r: [1, 2, 3, 4], s: 3}}"));
ASSERT(diff);
ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field: {u: {r: [1, 2, 3, 4]}} }}"));
}
diff --git a/src/mongo/db/update/document_diff_serialization.cpp b/src/mongo/db/update/document_diff_serialization.cpp
index 1bec8a0313e..0449f700cb2 100644
--- a/src/mongo/db/update/document_diff_serialization.cpp
+++ b/src/mongo/db/update/document_diff_serialization.cpp
@@ -51,13 +51,6 @@ doc_diff::DiffType identifyType(const BSONObj& diff) {
assertDiffNonEmpty(it);
if ((*it).fieldNameStringData() == kArrayHeader) {
- uassert(470521,
- str::stream() << "Expected " << kArrayHeader << " field to be bool but got" << *it,
- (*it).type() == BSONType::Bool);
-
- uassert(470522,
- str::stream() << "Expected " << kArrayHeader << " field to be true but got" << *it,
- (*it).Bool());
return DiffType::kArray;
}
return DiffType::kDocument;
@@ -72,73 +65,90 @@ stdx::variant<DocumentDiffReader, ArrayDiffReader> getReader(const Diff& diff) {
}
} // namespace
-DocumentDiffBuilder ArrayDiffBuilder::startSubObjDiff(size_t idx) {
- invariant(!_childSubDiffIndex);
- _childSubDiffIndex = idx;
- return DocumentDiffBuilder(this);
+SubBuilderGuard<DocumentDiffBuilder> ArrayDiffBuilder::startSubObjDiff(size_t idx) {
+ auto subDiffBuilder = std::make_unique<DocumentDiffBuilder>(0);
+ DocumentDiffBuilder* subBuilderPtr = subDiffBuilder.get();
+ _modifications.push_back({std::to_string(idx), std::move(subDiffBuilder)});
+ return SubBuilderGuard<DocumentDiffBuilder>(this, subBuilderPtr);
}
-ArrayDiffBuilder ArrayDiffBuilder::startSubArrDiff(size_t idx) {
- invariant(!_childSubDiffIndex);
- _childSubDiffIndex = idx;
- return ArrayDiffBuilder(this);
+SubBuilderGuard<ArrayDiffBuilder> ArrayDiffBuilder::startSubArrDiff(size_t idx) {
+ auto subDiffBuilder = std::unique_ptr<ArrayDiffBuilder>(new ArrayDiffBuilder());
+ ArrayDiffBuilder* subBuilderPtr = subDiffBuilder.get();
+ _modifications.push_back({std::to_string(idx), std::move(subDiffBuilder)});
+ return SubBuilderGuard<ArrayDiffBuilder>(this, subBuilderPtr);
}
void ArrayDiffBuilder::addUpdate(size_t idx, BSONElement elem) {
- _modifications.push_back({idx, elem});
+ auto fieldName = std::to_string(idx);
+ sizeTracker.addEntry(fieldName.size() + kUpdateSectionFieldName.size(), elem.valuesize());
+ _modifications.push_back({std::move(fieldName), elem});
}
-void ArrayDiffBuilder::releaseTo(BSONObjBuilder* output) {
+void ArrayDiffBuilder::serializeTo(BSONObjBuilder* output) const {
output->append(kArrayHeader, true);
if (_newSize) {
output->append(kResizeSectionFieldName, *_newSize);
}
- for (auto&& [idx, modification] : _modifications) {
+ for (auto&& modificationEntry : _modifications) {
+ auto&& idx = modificationEntry.first;
+ auto&& modification = modificationEntry.second;
stdx::visit(
visit_helper::Overloaded{
- [idx = idx, output](const Diff& subDiff) {
- // TODO: SERVER-48602 Try to avoid using BSON macro here. Ideally we will just
- // need one BSONObjBuilder for serializing the diff, at the end.
- output->append(std::to_string(idx), BSON(kSubDiffSectionFieldName << subDiff));
+ [&idx, output](const std::unique_ptr<DiffBuilderBase>& subDiff) {
+ BSONObjBuilder subObjBuilder =
+ output->subobjStart(kSubDiffSectionFieldName + idx);
+ subDiff->serializeTo(&subObjBuilder);
},
- [idx = idx, output](BSONElement elt) {
- output->append(std::to_string(idx), elt.wrap(kUpdateSectionFieldName));
+ [&idx, output](BSONElement elt) {
+ output->appendAs(elt, kUpdateSectionFieldName + idx);
}},
modification);
}
}
-void DocumentDiffBuilder::releaseTo(BSONObjBuilder* output) {
- if (!_deletes.asTempObj().isEmpty()) {
+void DocumentDiffBuilder::serializeTo(BSONObjBuilder* output) const {
+ if (!_deletes.empty()) {
BSONObjBuilder subBob(output->subobjStart(kDeleteSectionFieldName));
- subBob.appendElements(_deletes.done());
+ for (auto&& del : _deletes) {
+ subBob.append(del, false);
+ }
}
- if (!_updates.asTempObj().isEmpty()) {
+ if (!_updates.empty()) {
BSONObjBuilder subBob(output->subobjStart(kUpdateSectionFieldName));
- subBob.appendElements(_updates.done());
+ for (auto&& update : _updates) {
+ subBob.appendAs(update.second, update.first);
+ }
}
- if (!_inserts.asTempObj().isEmpty()) {
+ if (!_inserts.empty()) {
BSONObjBuilder subBob(output->subobjStart(kInsertSectionFieldName));
- subBob.appendElements(_inserts.done());
+ for (auto&& insert : _inserts) {
+ subBob.appendAs(insert.second, insert.first);
+ }
}
- if (!_subDiffs.asTempObj().isEmpty()) {
+ if (!_subDiffs.empty()) {
BSONObjBuilder subBob(output->subobjStart(kSubDiffSectionFieldName));
- subBob.appendElements(_subDiffs.done());
+ for (auto&& subDiff : _subDiffs) {
+ BSONObjBuilder subDiffBuilder(subBob.subobjStart(subDiff.first));
+ subDiff.second->serializeTo(&subDiffBuilder);
+ }
}
}
-DocumentDiffBuilder DocumentDiffBuilder::startSubObjDiff(StringData field) {
- invariant(!_childSubDiffField);
- _childSubDiffField = field.toString();
- return DocumentDiffBuilder(this);
+SubBuilderGuard<DocumentDiffBuilder> DocumentDiffBuilder::startSubObjDiff(StringData field) {
+ auto subDiffBuilder = std::make_unique<DocumentDiffBuilder>(0);
+ DocumentDiffBuilder* subBuilderPtr = subDiffBuilder.get();
+ _subDiffs.push_back({field, std::move(subDiffBuilder)});
+ return SubBuilderGuard<DocumentDiffBuilder>(this, subBuilderPtr);
}
-ArrayDiffBuilder DocumentDiffBuilder::startSubArrDiff(StringData field) {
- invariant(!_childSubDiffField);
- _childSubDiffField = field.toString();
- return ArrayDiffBuilder(this);
+SubBuilderGuard<ArrayDiffBuilder> DocumentDiffBuilder::startSubArrDiff(StringData field) {
+ auto subDiffBuilder = std::unique_ptr<ArrayDiffBuilder>(new ArrayDiffBuilder());
+ ArrayDiffBuilder* subBuilderPtr = subDiffBuilder.get();
+ _subDiffs.push_back({field, std::move(subDiffBuilder)});
+ return SubBuilderGuard<ArrayDiffBuilder>(this, subBuilderPtr);
}
namespace {
@@ -186,36 +196,36 @@ boost::optional<std::pair<size_t, ArrayDiffReader::ArrayModification>> ArrayDiff
}
auto next = _it.next();
- const size_t idx = extractArrayIndex(next.fieldNameStringData());
- uassert(4770514,
- str::stream() << "expected object at index " << idx << " in array diff but got "
- << next,
- next.type() == BSONType::Object);
+ auto fieldName = next.fieldNameStringData();
- const auto nextAsObj = next.embeddedObject();
+ static_assert(kUpdateSectionFieldName.size() == 1 && kSubDiffSectionFieldName.size() == 1,
+ "The code below assumes that the field names used in the diff format are single "
+ "character long");
uassert(4770521,
- str::stream() << "expected single-field object at index " << idx
- << " in array diff but got " << nextAsObj,
- nextAsObj.nFields() == 1);
- if (nextAsObj.firstElementFieldNameStringData() == kUpdateSectionFieldName) {
+ str::stream() << "expected field name to be at least two characters long, but found: "
+ << fieldName,
+ fieldName.size() > 1);
+ const size_t idx = extractArrayIndex(fieldName.substr(1, fieldName.size()));
+
+ if (fieldName[0] == kUpdateSectionFieldName[0]) {
// It's an update.
- return {{idx, nextAsObj.firstElement()}};
- } else if (nextAsObj.firstElementFieldNameStringData() == kSubDiffSectionFieldName) {
+ return {{idx, next}};
+ } else if (fieldName[0] == kSubDiffSectionFieldName[0]) {
// It's a sub diff...But which type?
uassert(4770501,
- str::stream() << "expected sub diff at index " << idx << " but got " << nextAsObj,
- nextAsObj.firstElement().type() == BSONType::Object);
+ str::stream() << "expected sub diff at index " << idx << " but got " << next,
+ next.type() == BSONType::Object);
auto modification =
stdx::visit(visit_helper::Overloaded{[](const auto& reader) -> ArrayModification {
return {reader};
}},
- getReader(nextAsObj.firstElement().embeddedObject()));
+ getReader(next.embeddedObject()));
return {{idx, modification}};
} else {
uasserted(4770502,
str::stream() << "Expected either 'u' (update) or 's' (sub diff) at index " << idx
- << " but got " << nextAsObj);
+ << " but got " << next);
}
}
diff --git a/src/mongo/db/update/document_diff_serialization.h b/src/mongo/db/update/document_diff_serialization.h
index 03acb763aea..3e56b226648 100644
--- a/src/mongo/db/update/document_diff_serialization.h
+++ b/src/mongo/db/update/document_diff_serialization.h
@@ -46,41 +46,120 @@ using Diff = BSONObj;
enum DiffType : uint8_t { kDocument, kArray };
// Below are string constants used in the diff format.
-const static StringData kArrayHeader = "a"_sd;
-const static StringData kDeleteSectionFieldName = "d"_sd;
-const static StringData kInsertSectionFieldName = "i"_sd;
-const static StringData kUpdateSectionFieldName = "u"_sd;
+constexpr StringData kArrayHeader = "a"_sd;
+constexpr StringData kDeleteSectionFieldName = "d"_sd;
+constexpr StringData kInsertSectionFieldName = "i"_sd;
+constexpr StringData kUpdateSectionFieldName = "u"_sd;
// 'l' for length.
-const static StringData kResizeSectionFieldName = "l"_sd;
-const static StringData kSubDiffSectionFieldName = "s"_sd;
+constexpr StringData kResizeSectionFieldName = "l"_sd;
+constexpr StringData kSubDiffSectionFieldName = "s"_sd;
+
+// Below are constants used for computation of Diff size. Note that the computed size is supposed to
+// be an approximate value.
+constexpr size_t kAdditionalPaddingForObjectSize = 5;
+constexpr size_t kSizeOfEmptyDocumentDiffBuilder = 5;
+// Size of empty object(5) + kArrayHeader(1) + null terminator + type byte + bool size.
+constexpr size_t kSizeOfEmptyArrayDiffBuilder = 9;
+
+/**
+ * Internal helper class for BSON size tracking to be used by the DiffBuilder classes.
+ */
+struct ApproxBSONSizeTracker {
+ ApproxBSONSizeTracker(size_t initialSize) : _size(initialSize) {}
+
+ void addEntry(size_t fieldSize, size_t valueSize, bool needToCreateWrappingObject = false) {
+ if (needToCreateWrappingObject) {
+ // Type byte(1) + FieldName(1) + Null terminator(1) + empty BSON object size (5)
+ _size += 8;
+ }
+ _size += fieldSize + valueSize + 2 /* Type byte + null terminator for field name */;
+ }
+
+ size_t getSize() const {
+ return _size;
+ }
+
+private:
+ size_t _size;
+};
+
+template <class DiffBuilder>
+class SubBuilderGuard;
+class DocumentDiffBuilder;
+class ArrayDiffBuilder;
// Not meant to be used elsewhere.
class DiffBuilderBase {
public:
- // A child DiffBuilder calls this on its parent when has completed and wants the diff it
- // produced to be stored.
- virtual void completeSubDiff(Diff) = 0;
+ DiffBuilderBase(size_t size) : sizeTracker(size){};
+ virtual ~DiffBuilderBase(){};
+
+ // Serializes the diff to 'out'.
+ virtual void serializeTo(BSONObjBuilder* out) const = 0;
+ size_t getObjSize() const {
+ return sizeTracker.getSize();
+ }
+
+protected:
+ ApproxBSONSizeTracker sizeTracker;
- // When a child builder is abandoned, it calls this on its parent to indicate that the parent
- // should release any state it was holding for the child.
+private:
+ // When the current child builder is abandoned, we needs to call this to indicate that the
+ // DiffBuilder should release any state it was holding for the current child. After calling
+ // this the builder will *not* add anything to the child builder.
virtual void abandonChild() = 0;
-};
-class DocumentDiffBuilder;
+ // When the current child builder is finished, we needs to call this to indicate that the
+ // DiffBuilder should stop accepting further modifications to the current child. After calling
+ // this the builder will *not* add anything to the child builder.
+ virtual void finishChild() = 0;
+
+ friend class SubBuilderGuard<ArrayDiffBuilder>;
+ friend class SubBuilderGuard<DocumentDiffBuilder>;
+};
/**
- * Class for building array diffs.
+ * An RAII type class which provides access to a sub-diff builder. This can be used to let the
+ * parent builder know what to do with the current builder resource, i.e, either commit or destroy.
+ * Note that this class is not directly responsible for destroying the diff builders.
*/
-class ArrayDiffBuilder final : public DiffBuilderBase {
+template <class DiffBuilder>
+class SubBuilderGuard final {
public:
- ~ArrayDiffBuilder() {
+ SubBuilderGuard(DiffBuilderBase* parent, DiffBuilder* builder)
+ : _parent(parent), _builder(builder) {}
+ ~SubBuilderGuard() {
if (_parent) {
- BSONObjBuilder builder;
- releaseTo(&builder);
- _parent->completeSubDiff(builder.obj());
+ _parent->finishChild();
}
}
+ void abandon() {
+ _parent->abandonChild();
+ _parent = nullptr;
+ _builder = nullptr;
+ }
+
+ DiffBuilder* builder() {
+ return _builder;
+ }
+
+private:
+ // Both pointers are unowned.
+ DiffBuilderBase* _parent;
+ DiffBuilder* _builder;
+};
+
+/**
+ * Class for building array diffs. The diff builder does not take ownership of the BSONElement
+ * passed. It is the responsibility of the caller to ensure that data stays in scope until
+ * serializeTo() is called.
+ */
+class ArrayDiffBuilder final : public DiffBuilderBase {
+public:
+ using ArrayModification = stdx::variant<BSONElement, std::unique_ptr<DiffBuilderBase>>;
+ ~ArrayDiffBuilder() {}
+
/**
* Sets the new size of the array. Should only be used for array truncations.
*/
@@ -89,6 +168,7 @@ public:
// BSON only has signed 4 byte integers. The new size must fit into that type.
invariant(index <= static_cast<uint32_t>(std::numeric_limits<int32_t>::max()));
_newSize = static_cast<int32_t>(index);
+ sizeTracker.addEntry(kResizeSectionFieldName.size(), sizeof(uint32_t) /* size of value */);
}
/*
@@ -102,204 +182,120 @@ public:
*/
void addUpdate(size_t idx, BSONElement newValue);
- DocumentDiffBuilder startSubObjDiff(size_t idx);
- ArrayDiffBuilder startSubArrDiff(size_t idx);
-
/**
- * Indicates that the changes added to this diff builder should be discarded. After calling
- * this the builder will *not* add anything to its parent builder.
+ * Starts a sub-diff object and returns an unowned pointer to the sub-diff builder. The client
+ * should not try to destroy the object.
*/
- void abandon() {
- if (_parent) {
- _parent->abandonChild();
- _parent = nullptr;
- }
- }
-
- void abandonChild() override {
- invariant(_childSubDiffIndex);
- _childSubDiffIndex = {};
- }
+ SubBuilderGuard<DocumentDiffBuilder> startSubObjDiff(size_t idx);
+ SubBuilderGuard<ArrayDiffBuilder> startSubArrDiff(size_t idx);
- // Called by child builders when they've completed.
- void completeSubDiff(Diff childDiff) override {
- invariant(_childSubDiffIndex);
- _modifications.push_back({*_childSubDiffIndex, std::move(childDiff)});
- _childSubDiffIndex = {};
- }
-
- size_t computeApproxSize() const {
- // TODO SERVER-48602: We need to ensure that this function returns in O(1). Incrementally
- // computing this size might be one option.
- size_t size = sizeof(int) /* Size of object */ + 1 /* Type byte */ + kArrayHeader.size() +
- 1 /* null terminator */ + 1 /*type byte */ + 1 /* bool size*/;
-
- if (_newSize) {
- size += kResizeSectionFieldName.size() + 1 /* Null terminator */ + 1 /* Type byte */ +
- sizeof(int) /* size of value */;
- }
-
- for (auto&& [idx, modification] : _modifications) {
- stdx::visit(visit_helper::Overloaded{[idx = idx, &size](const Diff& subDiff) {
- size += sizeof(idx) +
- kSubDiffSectionFieldName.size() +
- subDiff.objsize() + 2;
- },
- [idx = idx, &size](BSONElement elt) {
- size += sizeof(idx) +
- kUpdateSectionFieldName.size() +
- elt.size() + 2;
- }},
- modification);
- }
-
- return size;
- }
+ // Dumps the diff into the given builder.
+ void serializeTo(BSONObjBuilder* out) const;
private:
// The top-level of a diff is never an array diff. One can only construct an ArrayDiffBuilder
// from a parent DocumentDiffBuilder.
- explicit ArrayDiffBuilder(DiffBuilderBase* parent) : _parent(parent) {
- invariant(_parent);
+ explicit ArrayDiffBuilder() : DiffBuilderBase(kSizeOfEmptyArrayDiffBuilder) {}
+
+ void abandonChild() override {
+ invariant(!_modifications.empty());
+ _modifications.pop_back();
}
- // Dumps the diff into the given builder.
- void releaseTo(BSONObjBuilder* out);
+ void finishChild() override {
+ invariant(!_modifications.empty());
+ sizeTracker.addEntry(
+ _modifications.back().first.size() + kSubDiffSectionFieldName.size(),
+ stdx::get<std::unique_ptr<DiffBuilderBase>>(_modifications.back().second)
+ ->getObjSize());
+ }
// Each element is either an update (BSONElement) or a sub diff.
- std::vector<std::pair<size_t, stdx::variant<BSONElement, Diff>>> _modifications;
- boost::optional<int32_t> _newSize;
+ std::vector<std::pair<std::string, ArrayModification>> _modifications;
- // If there is an outstanding child builder, which index it is under.
- boost::optional<size_t> _childSubDiffIndex;
-
- // Parent DiffBuilder, if any. When this diff builder is destroyed, it will write all of the
- // modifications into its parent (unless it was abandoned).
- DiffBuilderBase* _parent = nullptr;
+ boost::optional<int32_t> _newSize;
friend class DocumentDiffBuilder;
}; // namespace doc_diff
/**
- * Class for building document diffs.
+ * Class for building document diffs. The diff builder does not take ownership of either the field
+ * name or the BSONElement passed. It is the responsibility of the caller to ensure that data stays
+ * in scope until serialize() is called.
*/
class DocumentDiffBuilder final : public DiffBuilderBase {
public:
- DocumentDiffBuilder() = default;
- ~DocumentDiffBuilder() {
- if (_parent) {
- BSONObjBuilder builder;
- releaseTo(&builder);
- _parent->completeSubDiff(builder.obj());
- }
- }
+ DocumentDiffBuilder(size_t padding = kAdditionalPaddingForObjectSize)
+ : DiffBuilderBase(kSizeOfEmptyDocumentDiffBuilder + padding) {}
/**
* Produces an owned BSONObj representing the diff.
*/
- Diff release() {
+ Diff serialize() const {
BSONObjBuilder bob;
- releaseTo(&bob);
+ serializeTo(&bob);
return bob.obj();
}
/**
- * Similar to release() but using an existing BSONObjBuilder.
+ * Similar to serialize() but using an existing BSONObjBuilder.
*/
- void releaseTo(BSONObjBuilder* out);
+ void serializeTo(BSONObjBuilder* out) const;
/**
* Functions for adding pieces of the diff. These functions must be called no more than once
* for a given field. They make no attempt to check for duplicates.
*/
void addDelete(StringData field) {
- _deletes.append(field, false);
+ // Add the size of 'field' + 'value'.
+ sizeTracker.addEntry(field.size(), sizeof(char), _deletes.empty());
+
+ _deletes.push_back(field);
}
void addUpdate(StringData field, BSONElement value) {
- _updates.appendAs(value, field);
+ // Add the size of 'field' + 'value'.
+ sizeTracker.addEntry(field.size(), value.valuesize(), _updates.empty());
+
+ _updates.push_back({field, value});
}
void addInsert(StringData field, BSONElement value) {
- _inserts.appendAs(value, field);
+ // Add the size of 'field' + 'value'.
+ sizeTracker.addEntry(field.size(), value.valuesize(), _inserts.empty());
+
+ _inserts.push_back({field, value});
}
/**
* Methods for starting sub diffs. Must not be called more than once for a given field. The
* contents of the StringData passed in must live for the entire duration of the sub-builder's
- * life.
+ * life. Returns an unowned pointer to the sub-diff builder. The client should not try to
+ * destroy the object.
*/
- DocumentDiffBuilder startSubObjDiff(StringData field);
- ArrayDiffBuilder startSubArrDiff(StringData field);
-
- /**
- * Indicates that the diff being built will never be serialized. If this is a child builder,
- * calling this will ensure that the diff is not added to the parent's buffer on destruction.
- */
- void abandon() {
- if (_parent) {
- _parent->abandonChild();
- _parent = nullptr;
- }
- _deletes.abandon();
- _updates.abandon();
- _inserts.abandon();
- }
+ SubBuilderGuard<DocumentDiffBuilder> startSubObjDiff(StringData field);
+ SubBuilderGuard<ArrayDiffBuilder> startSubArrDiff(StringData field);
+private:
void abandonChild() override {
- invariant(_childSubDiffField);
- _childSubDiffField = {};
+ invariant(!_subDiffs.empty());
+ _subDiffs.pop_back();
}
- // Called by child builders when they are done.
- void completeSubDiff(Diff childDiff) override {
- invariant(_childSubDiffField);
- _subDiffs.append(*_childSubDiffField, std::move(childDiff));
- _childSubDiffField = {};
- }
+ void finishChild() override {
+ invariant(!_subDiffs.empty());
- size_t computeApproxSize() {
- // TODO SERVER-48602: We need to ensure that this function returns in O(1). Incrementally
- // computing this size might be one option.
- size_t size = sizeof(int) /* Size of object */ + 1 /* Type byte */;
-
- if (!_updates.asTempObj().isEmpty()) {
- size += 1 /* Type byte */ + kUpdateSectionFieldName.size() /* FieldName */ +
- 1 /* Null terminator */ + _updates.bb().len() + 1 /* Null terminator */;
- }
-
- if (!_inserts.asTempObj().isEmpty()) {
- size += 1 /* Type byte */ + kInsertSectionFieldName.size() /* FieldName */ +
- 1 /* Null terminator */ + _inserts.bb().len() + 1 /* Null terminator */;
- }
-
- if (!_deletes.asTempObj().isEmpty()) {
- size += 1 /* Type byte */ + kDeleteSectionFieldName.size() /* FieldName */ +
- 1 /* Null terminator */ + _deletes.bb().len() + 1 /* Null terminator */;
- }
-
- if (!_subDiffs.asTempObj().isEmpty()) {
- size += 1 /* Type byte */ + kSubDiffSectionFieldName.size() /* FieldName */ +
- 1 /* Null terminator */ + _subDiffs.bb().len() + 1 /* Null terminator */;
- }
- return size;
+ // Add the size of 'field' + 'value'.
+ sizeTracker.addEntry(_subDiffs.back().first.size(),
+ _subDiffs.back().second->getObjSize(),
+ _subDiffs.size() == 1);
}
-
-private:
- DocumentDiffBuilder(DiffBuilderBase* parent) : _parent(parent) {}
-
- // TODO SERVER-48602: By using a BSONObjBuilder here and again in release() we make two copies
- // of each part of the diff before serializing it. An optimized implementation should avoid
- // this by only creating a BSONObjBuilder when it is time to serialize.
- BSONObjBuilder _subDiffs;
- BSONObjBuilder _deletes;
- BSONObjBuilder _updates;
- BSONObjBuilder _inserts;
+ std::vector<std::pair<StringData, BSONElement>> _updates;
+ std::vector<std::pair<StringData, BSONElement>> _inserts;
+ std::vector<StringData> _deletes;
+ std::vector<std::pair<StringData, std::unique_ptr<DiffBuilderBase>>> _subDiffs;
// If there is an outstanding child diff builder, its field is stored here.
boost::optional<std::string> _childSubDiffField;
- // Only used by sub-builders.
- DiffBuilderBase* _parent = nullptr;
-
friend class ArrayDiffBuilder;
};
diff --git a/src/mongo/db/update/document_diff_serialization_test.cpp b/src/mongo/db/update/document_diff_serialization_test.cpp
index cbd0a722e87..fd793897715 100644
--- a/src/mongo/db/update/document_diff_serialization_test.cpp
+++ b/src/mongo/db/update/document_diff_serialization_test.cpp
@@ -32,6 +32,7 @@
#include "mongo/platform/basic.h"
#include "mongo/bson/json.h"
+#include "mongo/db/update/document_diff_applier.h"
#include "mongo/db/update/document_diff_serialization.h"
#include "mongo/unittest/unittest.h"
@@ -50,17 +51,20 @@ BSONElement withFieldName(BSONElement elem, StringData fieldName) {
TEST(DiffSerializationTest, DeleteSimple) {
DocumentDiffBuilder builder;
- builder.addDelete("f1"_sd);
- builder.addDelete("f2"_sd);
- builder.addDelete("f3"_sd);
-
- auto out = builder.release();
+ StringData fieldName1 = "f1";
+ StringData fieldName2 = "f2";
+ StringData fieldName3 = "f3";
+ builder.addDelete(fieldName1);
+ builder.addDelete(fieldName2);
+ builder.addDelete(fieldName3);
+
+ auto out = builder.serialize();
ASSERT_BSONOBJ_BINARY_EQ(fromjson("{d: {f1: false, f2: false, f3: false}}"), out);
DocumentDiffReader reader(out);
- ASSERT_EQ(*reader.nextDelete(), "f1");
- ASSERT_EQ(*reader.nextDelete(), "f2");
- ASSERT_EQ(*reader.nextDelete(), "f3");
+ ASSERT_EQ(*reader.nextDelete(), fieldName1);
+ ASSERT_EQ(*reader.nextDelete(), fieldName2);
+ ASSERT_EQ(*reader.nextDelete(), fieldName3);
ASSERT(reader.nextDelete() == boost::none);
ASSERT(reader.nextInsert() == boost::none);
@@ -73,10 +77,10 @@ TEST(DiffSerializationTest, InsertSimple) {
<< "foo"));
DocumentDiffBuilder builder;
- builder.addInsert("f1"_sd, kDummyObj["a"]);
- builder.addInsert("f2"_sd, kDummyObj["b"]);
+ builder.addInsert("f1", kDummyObj["a"]);
+ builder.addInsert("f2", kDummyObj["b"]);
- auto out = builder.release();
+ auto out = builder.serialize();
ASSERT_BSONOBJ_BINARY_EQ(out, fromjson("{'i': {f1: 1, f2: 'foo' }}"));
DocumentDiffReader reader(out);
@@ -94,10 +98,10 @@ TEST(DiffSerializationTest, UpdateSimple) {
<< "foo"));
DocumentDiffBuilder builder;
- builder.addUpdate("f1"_sd, kDummyObj["a"]);
- builder.addUpdate("f2"_sd, kDummyObj["b"]);
+ builder.addUpdate("f1", kDummyObj["a"]);
+ builder.addUpdate("f2", kDummyObj["b"]);
- auto out = builder.release();
+ auto out = builder.serialize();
ASSERT_BSONOBJ_BINARY_EQ(out, fromjson("{'u': { f1: 1, f2: 'foo'}}"));
DocumentDiffReader reader(out);
@@ -115,19 +119,18 @@ TEST(DiffSerializationTest, SubDiff) {
<< "foo"));
DocumentDiffBuilder builder;
-
{
- DocumentDiffBuilder sub(builder.startSubObjDiff("obj"));
- sub.addInsert("iField", kDummyObj["a"]);
- sub.addUpdate("uField", kDummyObj["b"]);
- sub.addDelete("dField");
+ auto subBuilderGuard = builder.startSubObjDiff("obj");
+
+ subBuilderGuard.builder()->addInsert("iField", kDummyObj["a"]);
+ subBuilderGuard.builder()->addUpdate("uField", kDummyObj["b"]);
+ subBuilderGuard.builder()->addDelete("dField");
}
- auto out = builder.release();
+ auto out = builder.serialize();
ASSERT_BSONOBJ_BINARY_EQ(
out,
- fromjson("{s :"
- "{obj: {d : { dField: false }, u : { uField: 'foo' }, i : { iField: 1}}}}"));
+ fromjson("{s :{obj: {d : { dField: false }, u : { uField: 'foo' }, i : { iField: 1}}}}"));
DocumentDiffReader reader(out);
ASSERT(reader.nextUpdate() == boost::none);
@@ -156,27 +159,25 @@ TEST(DiffSerializationTest, SubArrayWithSubDiff) {
const BSONObj kDummyObj(BSON("a" << 1 << "b"
<< BSON("foo"
<< "bar")));
-
DocumentDiffBuilder builder;
-
{
- ArrayDiffBuilder sub(builder.startSubArrDiff("arr"));
- sub.addUpdate(0, kDummyObj["b"]);
+ auto subBuilderGuard = builder.startSubArrDiff("arr");
+ subBuilderGuard.builder()->addUpdate(0, kDummyObj["b"]);
{
- DocumentDiffBuilder subSub(sub.startSubObjDiff(2));
- subSub.addDelete("dField");
+ auto subSubBuilderGuard = subBuilderGuard.builder()->startSubObjDiff(2);
+ subSubBuilderGuard.builder()->addDelete("dField");
}
- sub.addUpdate(5, kDummyObj["a"]);
- sub.setResize(6);
+ subBuilderGuard.builder()->addUpdate(5, kDummyObj["a"]);
+ subBuilderGuard.builder()->setResize(6);
}
- auto out = builder.release();
+ auto out = builder.serialize();
ASSERT_BSONOBJ_BINARY_EQ(out,
fromjson("{s: {arr: {a: true, l: 6,"
- "'0': {'u': {foo: 'bar'}}, "
- "'2': {s: {'d': {dField: false}}},"
- "'5': {u: 1}}}}"));
+ "'u0': {foo: 'bar'}, "
+ "'s2': {'d': {dField: false}},"
+ "'u5': 1}}}"));
DocumentDiffReader reader(out);
ASSERT(reader.nextUpdate() == boost::none);
@@ -225,44 +226,47 @@ TEST(DiffSerializationTest, SubArrayNestedObject) {
BSONObj storage(BSON("a" << 1 << "b" << 2 << "c" << 3));
DocumentDiffBuilder builder;
{
- ArrayDiffBuilder subArr(builder.startSubArrDiff("arr"));
+ auto subArrBuilderGuard = builder.startSubArrDiff("subArray");
{
{
- DocumentDiffBuilder subObjBuilder(subArr.startSubObjDiff(1));
- subObjBuilder.addUpdate("a", storage["a"]);
+ auto subBuilderGuard = subArrBuilderGuard.builder()->startSubObjDiff(1);
+ subBuilderGuard.builder()->addUpdate(storage["a"].fieldNameStringData(),
+ storage["a"]);
}
{
- DocumentDiffBuilder subObjBuilder(subArr.startSubObjDiff(2));
- subObjBuilder.addUpdate("b", storage["b"]);
+ auto subBuilderGuard = subArrBuilderGuard.builder()->startSubObjDiff(2);
+ subBuilderGuard.builder()->addUpdate(storage["b"].fieldNameStringData(),
+ storage["b"]);
}
{
- DocumentDiffBuilder subObjBuilder(subArr.startSubObjDiff(3));
- subObjBuilder.addUpdate("c", storage["c"]);
+ auto subBuilderGuard = subArrBuilderGuard.builder()->startSubObjDiff(3);
+ subBuilderGuard.builder()->addUpdate(storage["c"].fieldNameStringData(),
+ storage["c"]);
}
}
}
- const auto out = builder.release();
- ASSERT_BSONOBJ_BINARY_EQ(out,
- fromjson("{s: {arr: {a: true, '1': {s: {u: {a: 1}}},"
- "'2': {s: {u: {b: 2}}}, '3': {s: {u: {c: 3}}}}}}"));
+ const auto out = builder.serialize();
+ ASSERT_BSONOBJ_BINARY_EQ(
+ out,
+ fromjson(
+ "{s: {subArray: {a: true, 's1': {u: {a: 1}}, 's2': {u: {b: 2}}, 's3': {u: {c: 3}}}}}"));
}
TEST(DiffSerializationTest, SubArrayHighIndex) {
const BSONObj kDummyObj(BSON("a" << 1 << "b"
<< "foo"));
-
DocumentDiffBuilder builder;
{
- ArrayDiffBuilder sub(builder.startSubArrDiff("arr"));
- sub.addUpdate(254, kDummyObj["b"]);
+ auto subBuilderGuard = builder.startSubArrDiff("subArray");
+ subBuilderGuard.builder()->addUpdate(254, kDummyObj["b"]);
}
- auto out = builder.release();
- ASSERT_BSONOBJ_BINARY_EQ(out, fromjson("{s: {arr: {a: true, '254': {u: 'foo'}}}}"));
+ auto out = builder.serialize();
+ ASSERT_BSONOBJ_BINARY_EQ(out, fromjson("{s: {subArray: {a: true, 'u254': 'foo'}}}"));
DocumentDiffReader reader(out);
ASSERT(reader.nextUpdate() == boost::none);
@@ -271,7 +275,7 @@ TEST(DiffSerializationTest, SubArrayHighIndex) {
{
auto [name, subDiffVar] = *reader.nextSubDiff();
- ASSERT_EQ(name, "arr");
+ ASSERT_EQ(name, "subArray");
auto subReader = stdx::get<ArrayDiffReader>(subDiffVar);
@@ -294,19 +298,21 @@ TEST(DiffSerializationTest, SubDiffAbandon) {
builder.addUpdate("uField", kDummyObj["a"]);
{
- DocumentDiffBuilder sub(builder.startSubObjDiff("obj"));
- sub.abandon();
+
+ auto subBuilderGuard = builder.startSubObjDiff("obj");
+ subBuilderGuard.builder()->addDelete("dField3");
+ subBuilderGuard.abandon();
}
// Make sure that after we abandon something we can still use the parent builder.
builder.addDelete("dField2");
{
- DocumentDiffBuilder sub(builder.startSubObjDiff("obj2"));
- sub.addDelete("dField2");
+ auto subBuilderGuard = builder.startSubObjDiff("obj2");
+ subBuilderGuard.builder()->addDelete("dField2");
}
- auto out = builder.release();
+ auto out = builder.serialize();
ASSERT_BSONOBJ_BINARY_EQ(
out,
fromjson("{d : {dField1: false, dField2: false}, u: {uField: 1}, s: {obj2: {d: "
@@ -321,18 +327,19 @@ TEST(DiffSerializationTest, SubArrayDiffAbandon) {
builder.addUpdate("uField", kDummyObj["a"]);
{
- ArrayDiffBuilder sub(builder.startSubArrDiff("arr"));
- sub.abandon();
+ auto subBuilderGuard = builder.startSubArrDiff("subArray");
+ subBuilderGuard.builder()->addUpdate(4, kDummyObj.firstElement());
+ subBuilderGuard.abandon();
}
// Make sure that after we abandon something we can still use the parent builder.
builder.addDelete("dField2");
{
- ArrayDiffBuilder sub(builder.startSubArrDiff("arr2"));
- sub.setResize(5);
+ auto subBuilderGuard = builder.startSubArrDiff("arr2");
+ subBuilderGuard.builder()->setResize(5);
}
- auto out = builder.release();
+ auto out = builder.serialize();
ASSERT_BSONOBJ_BINARY_EQ(out,
fromjson("{d : {dField1: false, dField2: false}, u: {uField: 1},"
"s: {arr2: {a: true, l: 5}}}"));
@@ -357,26 +364,108 @@ TEST(DiffSerializationTest, ValidateComputeApproxSize) {
builder.addDelete("");
{
// Ensure size of the sub-array diff is included.
- auto subDiff = builder.startSubArrDiff("subArray");
- subDiff.setResize(5);
- subDiff.addUpdate(2, storage["num"]);
- subDiff.addUpdate(2, storage["str"]);
-
- auto subSubDiff = subDiff.startSubObjDiff(22);
- subSubDiff.addInsert("insert2", storage["emptyStr"]);
- subSubDiff.addUpdate("update3", storage["null"]);
+ auto subBuilderGuard = builder.startSubArrDiff("subArray");
+ subBuilderGuard.builder()->setResize(5);
+ subBuilderGuard.builder()->addUpdate(2, storage["num"]);
+ subBuilderGuard.builder()->addUpdate(2, storage["str"]);
+ {
+ auto subSubBuilderGuard = subBuilderGuard.builder()->startSubObjDiff(2);
+ subSubBuilderGuard.builder()->addInsert("insert2", storage["emptyStr"]);
+ subSubBuilderGuard.builder()->addUpdate("update3", storage["null"]);
+ }
+ {
+ auto subSubBuilderGuard = subBuilderGuard.builder()->startSubObjDiff(234);
+ subSubBuilderGuard.builder()->addInsert("subObj", storage["subObj"]);
+ subSubBuilderGuard.builder()->addUpdate("update3", storage["null"]);
+ }
+ {
+ auto subSubArrayBuilderGuard = subBuilderGuard.builder()->startSubArrDiff(2456);
+ subSubArrayBuilderGuard.builder()->addUpdate(0, storage["num"]);
+ subSubArrayBuilderGuard.abandon();
+ }
+ {
+ auto subSubArrayBuilderGuard = subBuilderGuard.builder()->startSubArrDiff(2456);
+ subSubArrayBuilderGuard.builder()->setResize(10000);
+ subSubArrayBuilderGuard.builder()->addUpdate(0, storage["array"]);
+ }
+ }
+ {
+ auto subArrayBuilderGuard = builder.startSubArrDiff("subArray2");
+ subArrayBuilderGuard.builder()->addUpdate(2, storage["str"]);
}
{
// Ensure size of the sub-object diff is included.
- auto subDiff = builder.startSubObjDiff("subObj");
- subDiff.addUpdate("setArray", storage["array"]);
+ auto subBuilderGuard = builder.startSubObjDiff("subObj");
+ subBuilderGuard.builder()->addUpdate("setArray", storage["array"]);
+ }
+ {
+ // Ensure size of the abandoned sub-object diff is non included.
+ auto subBuilderGuard = builder.startSubObjDiff("subObj1");
+ subBuilderGuard.builder()->addUpdate("setArray2", storage["array"]);
+ subBuilderGuard.abandon();
}
// Update with a sub-object.
- builder.addUpdate("update4", storage["subObj"]);
+ builder.addUpdate("update2", storage["subObj"]);
+
+ auto computedSize = builder.getObjSize();
+ auto out = builder.serialize();
+ ASSERT_EQ(computedSize, out.objsize() + kAdditionalPaddingForObjectSize);
+}
+
+TEST(DiffSerializationTest, ExecptionsWhileDiffBuildingDoesNotLeakMemory) {
+ try {
+ DocumentDiffBuilder builder;
+ auto subBuilderGuard = builder.startSubArrDiff("subArray");
+ subBuilderGuard.builder()->setResize(4);
+ auto subSubBuilderGuard = subBuilderGuard.builder()->startSubObjDiff(5);
+ throw std::exception();
+ } catch (const std::exception&) {
+ }
+}
+TEST(DiffSerializationTest, UnexpectedFieldsInObjDiff) {
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{p : 1}")), DBException, 4770503);
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{u : true}")), DBException, 4770507);
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{d : []}")), DBException, 4770507);
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{i : null}")), DBException, 4770507);
+
+ // If the order of the fields is not obeyed.
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{i : {}, d: {}}")), DBException, 4770513);
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{s : {}, i: {}}")), DBException, 4770513);
+ ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{s : {}, p: {}}")), DBException, 4770513);
+
+ // Empty deletes object is valid.
+ ASSERT_BSONOBJ_BINARY_EQ(applyDiff(fromjson("{a: 1}"), fromjson("{d : {}}")),
+ fromjson("{a: 1}"));
+}
- auto computedSize = builder.computeApproxSize();
- auto out = builder.release();
- ASSERT_EQ(computedSize, out.objsize());
+TEST(DiffSerializationTest, UnexpectedFieldsInArrayDiff) {
+ ASSERT_THROWS_CODE(
+ applyDiff(fromjson("{arr: []}"), fromjson("{s: {arr: {a: true, '3u' : 1}}}")),
+ DBException,
+ 4770512);
+ ASSERT_THROWS_CODE(
+ applyDiff(fromjson("{arr: []}"), fromjson("{s: {arr: {a: true, u : true}}}")),
+ DBException,
+ 4770521);
+ ASSERT_THROWS_CODE(
+ applyDiff(fromjson("{arr: []}"), fromjson("{s: {arr: {a: true, '5' : {}}}}")),
+ DBException,
+ 4770521);
+ ASSERT_THROWS_CODE(
+ applyDiff(fromjson("{arr: []}"), fromjson("{s: {arr: {a: false, 'u3' : 4}}}")),
+ DBException,
+ 4770520);
+ ASSERT_THROWS_CODE(applyDiff(fromjson("{arr: []}"), fromjson("{s: {arr: {a: 1, 'u3' : 4}}}")),
+ DBException,
+ 4770519);
+ ASSERT_THROWS_CODE(
+ applyDiff(fromjson("{arr: []}"), fromjson("{s: {arr: {a: true, 's3' : 4}}}")),
+ DBException,
+ 4770501);
+ ASSERT_THROWS_CODE(
+ applyDiff(fromjson("{arr: []}"), fromjson("{s: {arr: {a: true, 'd3' : 4}}}")),
+ DBException,
+ 4770502);
}
} // namespace
diff --git a/src/mongo/db/update/pipeline_executor_test.cpp b/src/mongo/db/update/pipeline_executor_test.cpp
index d9566bbd4f7..2cabd0f5119 100644
--- a/src/mongo/db/update/pipeline_executor_test.cpp
+++ b/src/mongo/db/update/pipeline_executor_test.cpp
@@ -100,16 +100,17 @@ TEST_F(PipelineExecutorTest, ShouldNotCreateIdIfNoIdExistsAndNoneIsSpecified) {
std::vector<BSONObj> pipeline{fromjson("{$addFields: {a: 1, b: 2}}")};
PipelineExecutor exec(expCtx, pipeline);
- mutablebson::Document doc(fromjson("{c: 1, d: 2}"));
+ mutablebson::Document doc(fromjson("{c: 1, d: 'largeStringValue'}"));
auto result = exec.applyUpdate(getApplyParams(doc.root()));
ASSERT_FALSE(result.noop);
ASSERT_TRUE(result.indexesAffected);
- ASSERT_EQUALS(fromjson("{c: 1, d: 2, a: 1, b: 2}"), doc);
+ ASSERT_EQUALS(fromjson("{c: 1, d: 'largeStringValue', a: 1, b: 2}"), doc);
ASSERT_FALSE(doc.isInPlaceModeEnabled());
if (deltaOplogEntryAllowed()) {
ASSERT_BSONOBJ_BINARY_EQ(fromjson("{$v: 2, diff: {i: {a: 1, b: 2}}}"), result.oplogEntry);
} else {
- ASSERT_BSONOBJ_BINARY_EQ(fromjson("{c: 1, d: 2, a: 1, b: 2}"), result.oplogEntry);
+ ASSERT_BSONOBJ_BINARY_EQ(fromjson("{c: 1, d: 'largeStringValue', a: 1, b: 2}"),
+ result.oplogEntry);
}
}
@@ -135,17 +136,18 @@ TEST_F(PipelineExecutorTest, ShouldSucceedWhenImmutableIdIsNotModified) {
std::vector<BSONObj> pipeline{fromjson("{$addFields: {_id: 0, a: 1, b: 2}}")};
PipelineExecutor exec(expCtx, pipeline);
- mutablebson::Document doc(fromjson("{_id: 0, c: 1, d: 2}"));
+ mutablebson::Document doc(fromjson("{_id: 0, c: 1, d: 'largeStringValue'}"));
addImmutablePath("_id");
auto result = exec.applyUpdate(getApplyParams(doc.root()));
ASSERT_FALSE(result.noop);
ASSERT_TRUE(result.indexesAffected);
- ASSERT_EQUALS(fromjson("{_id: 0, c: 1, d: 2, a: 1, b: 2}"), doc);
+ ASSERT_EQUALS(fromjson("{_id: 0, c: 1, d: 'largeStringValue', a: 1, b: 2}"), doc);
ASSERT_FALSE(doc.isInPlaceModeEnabled());
if (deltaOplogEntryAllowed()) {
ASSERT_BSONOBJ_BINARY_EQ(fromjson("{$v: 2, diff: {i: {a: 1, b: 2 }}}"), result.oplogEntry);
} else {
- ASSERT_BSONOBJ_BINARY_EQ(fromjson("{_id: 0, c: 1, d: 2, a: 1, b: 2}"), result.oplogEntry);
+ ASSERT_BSONOBJ_BINARY_EQ(fromjson("{_id: 0, c: 1, d: 'largeStringValue', a: 1, b: 2}"),
+ result.oplogEntry);
}
}
@@ -161,7 +163,14 @@ TEST_F(PipelineExecutorTest, ComplexDoc) {
ASSERT_TRUE(result.indexesAffected);
ASSERT_EQUALS(fromjson("{a: 1, b: [0, 1, 2], e: [], c: {d: 1}}"), doc);
ASSERT_FALSE(doc.isInPlaceModeEnabled());
- ASSERT_BSONOBJ_BINARY_EQ(fromjson("{a: 1, b: [0, 1, 2], e: [], c: {d: 1}}"), result.oplogEntry);
+ if (deltaOplogEntryAllowed()) {
+ ASSERT_BSONOBJ_BINARY_EQ(
+ fromjson("{$v: 2, diff: {i: {c: {d: 1}}, s: {b: {a: true, u1: 1} }}}"),
+ result.oplogEntry);
+ } else {
+ ASSERT_BSONOBJ_BINARY_EQ(fromjson("{a: 1, b: [0, 1, 2], e: [], c: {d: 1}}"),
+ result.oplogEntry);
+ }
}
TEST_F(PipelineExecutorTest, CannotRemoveImmutablePath) {
View Lorry Controller Status