diff options
-rw-r--r-- | jstests/replsets/v2_delta_oplog_entries.js | 36 | ||||
-rw-r--r-- | src/mongo/db/repl/idempotency_test.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/update/document_diff_applier_test.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/update/document_diff_calculator.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/update/document_diff_calculator.h | 7 | ||||
-rw-r--r-- | src/mongo/db/update/document_diff_calculator_test.cpp | 270 | ||||
-rw-r--r-- | src/mongo/db/update/document_diff_serialization.cpp | 112 | ||||
-rw-r--r-- | src/mongo/db/update/document_diff_serialization.h | 24 | ||||
-rw-r--r-- | src/mongo/db/update/document_diff_serialization_test.cpp | 94 | ||||
-rw-r--r-- | src/mongo/db/update/document_diff_test.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/update/pipeline_executor.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/update/pipeline_executor_test.cpp | 11 | ||||
-rw-r--r-- | src/mongo/db/update/update_oplog_entry_serialization.h | 2 |
13 files changed, 379 insertions, 204 deletions
diff --git a/jstests/replsets/v2_delta_oplog_entries.js b/jstests/replsets/v2_delta_oplog_entries.js index aa0d086aa04..3755dac8b2f 100644 --- a/jstests/replsets/v2_delta_oplog_entries.js +++ b/jstests/replsets/v2_delta_oplog_entries.js @@ -79,7 +79,7 @@ function checkOplogEntry(node, expectedOplogEntryType, expectedId) { // Do some cursory/weak checks about the format of the 'o' field. assert.eq(Object.keys(oplogEntry.o), ["$v", "diff"]); for (let key of Object.keys(oplogEntry.o.diff)) { - assert.contains(key, ["i", "u", "s", "d"]); + assert.contains(key[0], ["i", "u", "s", "d"]); } } else if (expectedOplogEntryType === kExpectReplacementEntry) { assert.eq(oplogEntry.op, "u"); @@ -119,18 +119,18 @@ testUpdateReplicates({ // Adding a field and updating an existing one. id = generateId(); testUpdateReplicates({ - preImage: {_id: id, x: 0, y: 0}, + preImage: {_id: id, x: "notSoLargeString", y: 0}, pipeline: [{$set: {a: "foo", y: 999}}], - postImage: {_id: id, x: 0, y: 999, a: "foo"}, + postImage: {_id: id, x: "notSoLargeString", y: 999, a: "foo"}, expectedOplogEntry: kExpectDeltaEntry }); // Updating a subfield to a string. id = generateId(); testUpdateReplicates({ - preImage: {_id: id, x: 4, subObj: {a: 1, b: 2}}, + preImage: {_id: id, x: "notSoLargeString", subObj: {a: 1, b: 2}}, pipeline: [{$set: {"subObj.a": "foo", y: 1}}], - postImage: {_id: id, x: 4, subObj: {a: "foo", b: 2}, y: 1}, + postImage: {_id: id, x: "notSoLargeString", subObj: {a: "foo", b: 2}, y: 1}, expectedOplogEntry: kExpectDeltaEntry }); @@ -139,18 +139,18 @@ testUpdateReplicates({ // than a weak BSON type insensitive comparison. id = generateId(); testUpdateReplicates({ - preImage: {_id: id, x: 4, subObj: {a: NumberLong(1), b: 2}}, + preImage: {_id: id, x: "notSoLargeString", subObj: {a: NumberLong(1), b: 2}}, pipeline: [{$set: {"subObj.a": 1, y: 1}}], - postImage: {_id: id, x: 4, subObj: {a: 1, b: 2}, y: 1}, + postImage: {_id: id, x: "notSoLargeString", subObj: {a: 1, b: 2}, y: 1}, expectedOplogEntry: kExpectDeltaEntry }); // Update a subfield to an object. id = generateId(); testUpdateReplicates({ - preImage: {_id: id, x: 4, subObj: {a: NumberLong(1), b: 2}}, + preImage: {_id: id, x: "notSoLargeString", subObj: {a: NumberLong(1), b: 2}}, pipeline: [{$set: {"subObj.a": {$const: {newObj: {subField: 1}}}, y: 1}}], - postImage: {_id: id, x: 4, subObj: {a: {newObj: {subField: 1}}, b: 2}, y: 1}, + postImage: {_id: id, x: "notSoLargeString", subObj: {a: {newObj: {subField: 1}}, b: 2}, y: 1}, expectedOplogEntry: kExpectDeltaEntry }); @@ -331,6 +331,24 @@ testUpdateReplicates({ expectedOplogEntry: kExpectDeltaEntry }); +function generateDeepObj(depth, maxDepth, value) { + return { + "padding": kGiantStr, + "subObj": (depth >= maxDepth) ? value : generateDeepObj(depth + 1, maxDepth, value) + }; +} + +// Verify that diffing the deepest objects allowed by the js client, can produce a delta op-log +// entries. +id = generateId(); +let path = "subObj.".repeat(146) + "subObj"; +testUpdateReplicates({ + preImage: Object.assign({_id: id}, generateDeepObj(1, 147, 1)), + pipeline: [{$set: {[path]: 2}}], + postImage: Object.assign({_id: id}, generateDeepObj(1, 147, 2)), + expectedOplogEntry: kExpectDeltaEntry +}); + // Don't drop any collections. At the end we want the DBHash checker will make sure there's no // corruption. diff --git a/src/mongo/db/repl/idempotency_test.cpp b/src/mongo/db/repl/idempotency_test.cpp index c841250218f..643148d6ca9 100644 --- a/src/mongo/db/repl/idempotency_test.cpp +++ b/src/mongo/db/repl/idempotency_test.cpp @@ -44,6 +44,7 @@ #include "mongo/db/server_options.h" #include "mongo/db/update/document_diff_calculator.h" #include "mongo/db/update/document_diff_test_helpers.h" +#include "mongo/db/update/update_oplog_entry_serialization.h" #include "mongo/logv2/log.h" #include "mongo/unittest/unittest.h" @@ -267,7 +268,8 @@ void RandomizedIdempotencyTest::runUpdateV2IdempotencyTestCase(double v2Probabil // computing the input objects) would break idempotency. So we do a dry run of what // the collection state would look like and compute diffs based on that. generatedDoc = generateDocWithId(kDocId); - auto diff = doc_diff::computeDiff(oldDoc, *generatedDoc); + auto diff = doc_diff::computeDiff( + oldDoc, *generatedDoc, update_oplog_entry::kSizeOfDeltaOplogEntryMetadata); ASSERT(diff); oplogDiff = BSON("$v" << 2 << "diff" << *diff); } else { diff --git a/src/mongo/db/update/document_diff_applier_test.cpp b/src/mongo/db/update/document_diff_applier_test.cpp index aeadcb47060..90ea71785b3 100644 --- a/src/mongo/db/update/document_diff_applier_test.cpp +++ b/src/mongo/db/update/document_diff_applier_test.cpp @@ -372,7 +372,7 @@ TEST(DiffApplierTest, DuplicateFieldNames) { 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}}}}")), + applyDiff(BSONObj(), fromjson("{sa: {d: {p: false}}, sa: {a: true, d: {p: false}}}")), DBException, 4728000); @@ -383,9 +383,8 @@ TEST(DiffApplierTest, DuplicateFieldNames) { 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); + ASSERT_THROWS_CODE( + applyDiff(BSONObj(), fromjson("{u: {a: {}}, sa: {d : {k: false}}}")), DBException, 4728000); } } // namespace diff --git a/src/mongo/db/update/document_diff_calculator.cpp b/src/mongo/db/update/document_diff_calculator.cpp index 512087d1a67..35935372241 100644 --- a/src/mongo/db/update/document_diff_calculator.cpp +++ b/src/mongo/db/update/document_diff_calculator.cpp @@ -162,8 +162,10 @@ void calculateSubDiffHelper(const BSONElement& preVal, } } // namespace -boost::optional<doc_diff::Diff> computeDiff(const BSONObj& pre, const BSONObj& post) { - doc_diff::DocumentDiffBuilder diffBuilder; +boost::optional<doc_diff::Diff> computeDiff(const BSONObj& pre, + const BSONObj& post, + size_t padding) { + doc_diff::DocumentDiffBuilder diffBuilder(padding); if (computeDocDiff(pre, post, &diffBuilder)) { auto diff = diffBuilder.serialize(); if (diff.objsize() < post.objsize()) { diff --git a/src/mongo/db/update/document_diff_calculator.h b/src/mongo/db/update/document_diff_calculator.h index 62436511252..e5329da24f9 100644 --- a/src/mongo/db/update/document_diff_calculator.h +++ b/src/mongo/db/update/document_diff_calculator.h @@ -37,8 +37,11 @@ namespace mongo::doc_diff { /** * Returns the delta between 'pre' and 'post' by recursively iterating the object. If the size * of the computed delta is larger than the 'post' object then the function returns - * 'boost::none'. + * 'boost::none'. The 'paddingForDiff' represents the additional size that needs be added to the + * size of the diff, while comparing whether the diff is viable. */ -boost::optional<doc_diff::Diff> computeDiff(const BSONObj& pre, const BSONObj& post); +boost::optional<doc_diff::Diff> computeDiff(const BSONObj& pre, + const BSONObj& post, + size_t paddingForDiff); }; // 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 055ecaa3d4d..a2b441ff310 100644 --- a/src/mongo/db/update/document_diff_calculator_test.cpp +++ b/src/mongo/db/update/document_diff_calculator_test.cpp @@ -29,6 +29,9 @@ #include "mongo/platform/basic.h" +#include <functional> + +#include "mongo/bson/bson_depth.h" #include "mongo/bson/json.h" #include "mongo/db/update/document_diff_calculator.h" #include "mongo/unittest/unittest.h" @@ -36,9 +39,9 @@ namespace mongo { namespace { -TEST(DocumentDiffTest, SameObjectsNoDiff) { +TEST(DocumentDiffCalculatorTest, SameObjectsNoDiff) { auto assertDiffEmpty = [](const BSONObj& doc) { - auto diff = doc_diff::computeDiff(doc, doc); + auto diff = doc_diff::computeDiff(doc, doc, 5); ASSERT(diff); ASSERT_BSONOBJ_BINARY_EQ(*diff, BSONObj()); }; @@ -51,172 +54,293 @@ TEST(DocumentDiffTest, SameObjectsNoDiff) { assertDiffEmpty(fromjson("{'0': [[{'0': [[{'0': [[]]} ]]}]]}")); } -TEST(DocumentDiffTest, LargeDelta) { +TEST(DocumentDiffCalculatorTest, LargeDelta) { ASSERT_FALSE( - doc_diff::computeDiff(fromjson("{a: 1, b: 2, c: 3}"), fromjson("{A: 1, B: 2, C: 3}"))); + doc_diff::computeDiff(fromjson("{a: 1, b: 2, c: 3}"), fromjson("{A: 1, B: 2, C: 3}"), 0)); // Inserting field at the beginning produces a large delta. - ASSERT_FALSE(doc_diff::computeDiff(fromjson("{b: 1, c: 1, d: 1}"), - fromjson("{a: 1, b: 1, c: 1, d: 1}"))); + ASSERT_FALSE(doc_diff::computeDiff( + fromjson("{b: 1, c: 1, d: 1}"), fromjson("{a: 1, b: 1, c: 1, d: 1}"), 0)); // Empty objects. - ASSERT_FALSE(doc_diff::computeDiff(BSONObj(), BSONObj())); + ASSERT_FALSE(doc_diff::computeDiff(BSONObj(), BSONObj(), 1)); // Full diff. - ASSERT_FALSE(doc_diff::computeDiff(fromjson("{b: 1}"), BSONObj())); - ASSERT_FALSE(doc_diff::computeDiff(BSONObj(), fromjson("{b: 1}"))); + ASSERT_FALSE(doc_diff::computeDiff(fromjson("{b: 1}"), BSONObj(), 0)); + ASSERT_FALSE(doc_diff::computeDiff(BSONObj(), fromjson("{b: 1}"), 0)); } -TEST(DocumentDiffTest, DeleteAndInsertFieldAtTheEnd) { - auto diff = doc_diff::computeDiff(fromjson("{a: 1, b: 2, c: 3, d: 4}"), - fromjson("{b: 2, c: 3, d: 4, a: 1}")); +TEST(DocumentDiffCalculatorTest, DeleteAndInsertFieldAtTheEnd) { + auto diff = doc_diff::computeDiff(fromjson("{a: 1, b: 'valueString', c: 3, d: 4}"), + fromjson("{b: 'valueString', c: 3, d: 4, a: 1}"), + 15); ASSERT(diff); ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{i: {a: 1}}")); } -TEST(DocumentDiffTest, DeletesRecordedInAscendingOrderOfFieldNames) { - auto diff = doc_diff::computeDiff(fromjson("{b: 1, a: 2, c: 3, d: 4, e: 'value'}"), - fromjson("{c: 3, d: 4, e: 'value'}")); +TEST(DocumentDiffCalculatorTest, DeletesRecordedInAscendingOrderOfFieldNames) { + auto diff = doc_diff::computeDiff(fromjson("{b: 1, a: 2, c: 3, d: 4, e: 'valueString'}"), + fromjson("{c: 3, d: 4, e: 'valueString'}"), + 15); ASSERT(diff); ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{d: {a: false, b: false}}")); // Delete at the end still follow the order. diff = doc_diff::computeDiff( - fromjson("{b: 1, a: 2, c: 'value', d: 'value', e: 'value', g: 1, f: 1}"), - fromjson("{c: 'value', d: 'value', e: 'value'}")); + fromjson("{b: 1, a: 2, c: 'value', d: 'valueString', e: 'valueString', g: 1, f: 1}"), + fromjson("{c: 'value', d: 'valueString', e: 'valueString'}"), + 15); ASSERT(diff); ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{d: {g: false, f: false, a: false, b: false}}")); } -TEST(DocumentDiffTest, DataTypeChangeRecorded) { +TEST(DocumentDiffCalculatorTest, DataTypeChangeRecorded) { const auto objWithDoubleValue = - fromjson("{a: 1, b: 2, c: {subField1: 1, subField2: 3.0}, d: 4}"); - const auto objWithIntValue = fromjson("{a: 1, b: 2, c: {subField1: 1, subField2: 3}, d: 4}"); + fromjson("{a: 'valueString', b: 2, c: {subField1: 1, subField2: 3.0}, d: 4}"); + const auto objWithIntValue = + fromjson("{a: 'valueString', b: 2, c: {subField1: 1, subField2: 3}, d: 4}"); const auto objWithLongValue = - fromjson("{a: 1, b: 2, c: {subField1: 1, subField2: NumberLong(3)}, d: 4}"); - auto diff = doc_diff::computeDiff(objWithDoubleValue, objWithIntValue); + fromjson("{a: 'valueString', b: 2, c: {subField1: 1, subField2: NumberLong(3)}, d: 4}"); + auto diff = doc_diff::computeDiff(objWithDoubleValue, objWithIntValue, 15); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {c: {u: {subField2: 3}} }}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{sc: {u: {subField2: 3}} }")); - diff = doc_diff::computeDiff(objWithIntValue, objWithLongValue); + diff = doc_diff::computeDiff(objWithIntValue, objWithLongValue, 15); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {c: {u: {subField2: NumberLong(3)}} }}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{sc: {u: {subField2: NumberLong(3)}} }")); - diff = doc_diff::computeDiff(objWithLongValue, objWithDoubleValue); + diff = doc_diff::computeDiff(objWithLongValue, objWithDoubleValue, 15); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {c: {u: {subField2: 3.0}} }}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{sc: {u: {subField2: 3.0}} }")); } -TEST(DocumentDiffTest, NullAndMissing) { - auto diff = doc_diff::computeDiff(fromjson("{a: null}"), fromjson("{}")); +TEST(DocumentDiffCalculatorTest, NullAndMissing) { + auto diff = doc_diff::computeDiff(fromjson("{a: null}"), fromjson("{}"), 15); ASSERT_FALSE(diff); - diff = doc_diff::computeDiff(fromjson("{a: null, b: undefined, c: null, d: 'someVal'}"), - fromjson("{a: null, b: undefined, c: undefined, d: 'someVal'}")); + diff = + doc_diff::computeDiff(fromjson("{a: null, b: undefined, c: null, d: 'someValueStr'}"), + fromjson("{a: null, b: undefined, c: undefined, d: 'someValueStr'}"), + 15); ASSERT(diff); ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{u: {c: undefined}}")); } -TEST(DocumentDiffTest, FieldOrder) { +TEST(DocumentDiffCalculatorTest, FieldOrder) { auto diff = doc_diff::computeDiff(fromjson("{a: 1, b: 2, c: {p: 1, q: 1, s: 1, r: 2}, d: 4}"), - fromjson("{a: 1, b: 2, c: {p: 1, q: 1, r: 2, s: 1}, d: 4}")); + fromjson("{a: 1, b: 2, c: {p: 1, q: 1, r: 2, s: 1}, d: 4}"), + 10); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {c: {i: {s: 1}} }}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{sc: {i: {s: 1}} }")); } -TEST(DocumentDiffTest, SimpleArrayPush) { +TEST(DocumentDiffCalculatorTest, SimpleArrayPush) { auto diff = doc_diff::computeDiff(fromjson("{field1: 'abcd', field2: [1, 2, 3]}"), - fromjson("{field1: 'abcd', field2: [1, 2, 3, 4]}")); + fromjson("{field1: 'abcd', field2: [1, 2, 3, 4]}"), + 5); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field2: {a: true, 'u3': 4}}}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{sfield2: {a: true, 'u3': 4}}")); } -TEST(DocumentDiffTest, NestedArray) { +TEST(DocumentDiffCalculatorTest, NestedArray) { auto diff = doc_diff::computeDiff(fromjson("{field1: 'abcd', field2: [1, 2, 3, [[2]]]}"), - fromjson("{field1: 'abcd', field2: [1, 2, 3, [[4]]]}")); + fromjson("{field1: 'abcd', field2: [1, 2, 3, [[4]]]}"), + 0); 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, 'u3': [[4]]}}}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{sfield2: {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]}")); + fromjson("{field1: 'abcd', field2: [1, 2, 3, [1, 'longString', [4], 4], 5, 6]}"), + 0); + ASSERT(diff); + ASSERT_BSONOBJ_BINARY_EQ( + *diff, fromjson("{sfield2: {a: true, l: 6, 's3': {a: true, l: 4, 'u2': [4]}, 'u5': 6}}")); +} + +TEST(DocumentDiffCalculatorTest, SubObjHasEmptyFieldName) { + auto diff = doc_diff::computeDiff( + fromjson("{'': '1', field2: [1, 2, 3, {'': {'': 1, padding: 'largeFieldValue'}}]}"), + fromjson("{'': '2', field2: [1, 2, 3, {'': {'': 2, padding: 'largeFieldValue'}}]}"), + 15); + ASSERT(diff); ASSERT_BSONOBJ_BINARY_EQ( - *diff, - fromjson("{s: {field2: {a: true, l: 6, 's3': {a: true, l: 4, 'u2': [4]}, 'u5': 6}}}")); + *diff, fromjson("{u: {'': '2'}, sfield2: {a: true, s3: {s: {u: {'': 2}}} }}")); } -TEST(DocumentDiffTest, SubObjInSubArrayUpdateElements) { +TEST(DocumentDiffCalculatorTest, SubObjInSubArrayUpdateElements) { auto diff = doc_diff::computeDiff( fromjson("{field1: 'abcd', field2: [1, 2, 3, " "{field3: ['veryLargeStringValueveryLargeStringValue', 2, 3, 4]}]}"), fromjson("{field1: 'abcd', field2: [1, 2, 3, {'field3': " - "['veryLargeStringValueveryLargeStringValue', 2, 4, 3, " - "5]}]}")); + "['veryLargeStringValueveryLargeStringValue', 2, 4, 3, 5]}]}"), + 0); ASSERT(diff); ASSERT_BSONOBJ_BINARY_EQ( - *diff, - fromjson("{s: {field2: {a: true, 's3': {s: {field3: {a: true, 'u2': 4, 'u3': 3, " - "'u4': 5}} }} }}")); + *diff, fromjson("{sfield2: {a: true, s3: {sfield3: {a: true, u2: 4, u3: 3, u4: 5}} }}")); } -TEST(DocumentDiffTest, SubObjInSubArrayDeleteElements) { +TEST(DocumentDiffCalculatorTest, SubObjInSubArrayDeleteElements) { auto diff = doc_diff::computeDiff( fromjson("{field1: 'abcd', field2: [1, 2, 3, {'field3': ['largeString', 2, 3, 4, 5]}]}"), - fromjson("{field1: 'abcd', field2: [1, 2, 3, {'field3': ['largeString', 2, 3, 5]}]}")); + fromjson("{field1: 'abcd', field2: [1, 2, 3, {'field3': ['largeString', 2, 3, 5]}]}"), + 15); ASSERT(diff); ASSERT_BSONOBJ_BINARY_EQ( - *diff, - fromjson("{s: {field2: {a: true, 's3': {s: {field3: {a: true, l: 4, 'u3': 5}} }} }}")); + *diff, fromjson("{sfield2: {a: true, 's3': {sfield3: {a: true, l: 4, 'u3': 5}} }}")); } -TEST(DocumentDiffTest, NestedSubObjs) { +TEST(DocumentDiffCalculatorTest, NestedSubObjs) { auto diff = doc_diff::computeDiff( fromjson("{level1Field1: 'abcd', level1Field2: {level2Field1: {level3Field1: {p: 1}, " - "level3Field2: {q: 1}}, level2Field2: 2}, level1Field3: 3}"), + "level3Field2: {q: 1}}, level2Field2: 2}, level1Field3: 3, level1Field4: " + "{level2Field1: {level3Field1: {p: 1}, level3Field2: {q: 1}}} }"), fromjson("{level1Field1: 'abcd', level1Field2: {level2Field1: {level3Field1: {q: 1}, " - "level3Field2: {q: 1}}, level2Field2: 2}, level1Field3: '3'}")); + "level3Field2: {q: 1}}, level2Field2: 2}, level1Field3: '3', level1Field4: " + "{level2Field1: {level3Field1: {q: 1}, level3Field2: {q: 1}}} }"), + 15); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ( - *diff, - fromjson("{u: {level1Field3: '3'}, s: {level1Field2: {s: {'level2Field1': " - "{u: {level3Field1: {q: 1}}} }} }}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, + fromjson("{u: {level1Field3: '3'}, slevel1Field2: {slevel2Field1: {u: " + "{level3Field1: {q: 1}}}}, slevel1Field4: {slevel2Field1: " + "{u: {level3Field1: {q: 1}}}} }")); } -TEST(DocumentDiffTest, SubArrayInSubArrayLargeDelta) { +TEST(DocumentDiffCalculatorTest, SubArrayInSubArrayLargeDelta) { auto diff = doc_diff::computeDiff( fromjson("{field1: 'abcd', field2: [1, 2, 3, {field3: [2, 3, 4, 5]}]}"), - fromjson("{field1: 'abcd', field2: [1, 2, 3, {field3: [1, 2, 3, 4, 5]}]}")); + fromjson("{field1: 'abcd', field2: [1, 2, 3, {field3: [1, 2, 3, 4, 5]}]}"), + 15); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ( - *diff, fromjson("{s: {field2: {a: true, 'u3': {field3: [1, 2, 3, 4, 5]}} }}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, + fromjson("{sfield2: {a: true, 'u3': {field3: [1, 2, 3, 4, 5]}} }")); } -TEST(DocumentDiffTest, SubObjInSubArrayLargeDelta) { +TEST(DocumentDiffCalculatorTest, SubObjInSubArrayLargeDelta) { auto diff = 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]}")); + fromjson("{field1: [1, 2, 3, 4, 5, 6, {p: 1, q: 2}, 7]}"), + 0); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field1: {a: true, 'u6': {p: 1, q: 2}} }}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{sfield1: {a: true, 'u6': {p: 1, q: 2}} }")); } -TEST(DocumentDiffTest, SubObjInSubObjLargeDelta) { +TEST(DocumentDiffCalculatorTest, SubObjInSubObjLargeDelta) { auto diff = doc_diff::computeDiff( 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}}")); + fromjson("{field: {p: 'someString', q: 2, r: {p: 1, q: 2}, s: 3}}"), + 0); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field: {u: {r: {p: 1, q: 2} }} }}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{sfield: {u: {r: {p: 1, q: 2} }} }")); } -TEST(DocumentDiffTest, SubArrayInSubObjLargeDelta) { +TEST(DocumentDiffCalculatorTest, SubArrayInSubObjLargeDelta) { 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}}")); + fromjson("{field: {p: 'someString', q: 2, r: [1, 2, 3, 4], s: 3}}"), + 0); ASSERT(diff); - ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{s: {field: {u: {r: [1, 2, 3, 4]}} }}")); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{sfield: {u: {r: [1, 2, 3, 4]}} }")); +} + +void buildDeepObj(BSONObjBuilder* builder, + StringData fieldName, + int depth, + int maxDepth, + std::function<void(BSONObjBuilder*, int, int)> function) { + if (depth >= maxDepth) { + return; + } + BSONObjBuilder subObj = builder->subobjStart(fieldName); + function(&subObj, depth, maxDepth); + + buildDeepObj(&subObj, fieldName, depth + 1, maxDepth, function); +} + +TEST(DocumentDiffCalculatorTest, DeeplyNestObjectGenerateDiff) { + const auto largeValue = std::string(50, 'a'); + const auto maxDepth = BSONDepth::getMaxDepthForUserStorage(); + auto functionToApply = [&largeValue](BSONObjBuilder* builder, int depth, int maxDepth) { + builder->append("largeField", largeValue); + }; + + BSONObjBuilder preBob; + preBob.append("largeField", largeValue); + buildDeepObj(&preBob, "subObj", 0, maxDepth, functionToApply); + auto preObj = preBob.done(); + ASSERT(preObj.valid()); + + BSONObjBuilder postBob; + postBob.append("largeField", largeValue); + auto diff = doc_diff::computeDiff(preObj, postBob.done(), 0); + + // Just deleting the deeply nested sub-object should give the post object. + ASSERT(diff); + ASSERT_BSONOBJ_BINARY_EQ(*diff, fromjson("{d: {subObj: false}}")); + + BSONObjBuilder postBob2; + postBob2.append("largeField", largeValue); + buildDeepObj(&postBob2, "subObj", 0, maxDepth - 1, functionToApply); + + // Deleting the deepest field should give the post object. + diff = doc_diff::computeDiff(preObj, postBob2.done(), 0); + ASSERT(diff); + ASSERT(diff->valid()); + + BSONObjBuilder expectedOutputBuilder; + buildDeepObj(&expectedOutputBuilder, + "ssubObj", + 0, + maxDepth - 1, + [](BSONObjBuilder* builder, int depth, int maxDepth) { + if (depth == maxDepth - 1) { + builder->append("d", BSON("subObj" << false)); + } + }); + ASSERT_BSONOBJ_BINARY_EQ(*diff, expectedOutputBuilder.obj()); +} + +TEST(DocumentDiffCalculatorTest, DeepestObjectSubDiff) { + BSONObjBuilder bob1; + const auto largeValue = std::string(50, 'a'); + int value; + auto functionToApply = [&value, &largeValue](BSONObjBuilder* builder, int depth, int maxDepth) { + builder->append("largeField", largeValue); + if (depth == maxDepth - 1) { + builder->append("field", value); + } + }; + + value = 1; + buildDeepObj(&bob1, "subObj", 0, BSONDepth::getMaxDepthForUserStorage(), functionToApply); + auto preObj = bob1.done(); + ASSERT(preObj.valid()); + + BSONObjBuilder postBob; + value = 2; + buildDeepObj(&postBob, "subObj", 0, BSONDepth::getMaxDepthForUserStorage(), functionToApply); + auto postObj = postBob.done(); + ASSERT(postObj.valid()); + + auto diff = doc_diff::computeDiff(preObj, postObj, 0); + ASSERT(diff); + ASSERT(diff->valid()); + + BSONObjBuilder expectedOutputBuilder; + buildDeepObj(&expectedOutputBuilder, + "ssubObj", + 0, + BSONDepth::getMaxDepthForUserStorage(), + [](BSONObjBuilder* builder, int depth, int maxDepth) { + if (depth == maxDepth - 1) { + builder->append("u", BSON("field" << 2)); + } + }); + ASSERT_BSONOBJ_BINARY_EQ(*diff, expectedOutputBuilder.obj()); } } // namespace diff --git a/src/mongo/db/update/document_diff_serialization.cpp b/src/mongo/db/update/document_diff_serialization.cpp index 0449f700cb2..f978eb23e9e 100644 --- a/src/mongo/db/update/document_diff_serialization.cpp +++ b/src/mongo/db/update/document_diff_serialization.cpp @@ -36,10 +36,6 @@ namespace mongo::doc_diff { namespace { -static const StringDataSet kDocumentDiffSections = {kInsertSectionFieldName, - kUpdateSectionFieldName, - kDeleteSectionFieldName, - kSubDiffSectionFieldName}; void assertDiffNonEmpty(const BSONObjIterator& it) { uassert(4770500, "Expected diff to be non-empty", it.more()); @@ -80,7 +76,7 @@ SubBuilderGuard<ArrayDiffBuilder> ArrayDiffBuilder::startSubArrDiff(size_t idx) void ArrayDiffBuilder::addUpdate(size_t idx, BSONElement elem) { auto fieldName = std::to_string(idx); - sizeTracker.addEntry(fieldName.size() + kUpdateSectionFieldName.size(), elem.valuesize()); + sizeTracker.addEntry(fieldName.size() + 1 /* kUpdateSectionFieldName */, elem.valuesize()); _modifications.push_back({std::move(fieldName), elem}); } @@ -97,7 +93,7 @@ void ArrayDiffBuilder::serializeTo(BSONObjBuilder* output) const { visit_helper::Overloaded{ [&idx, output](const std::unique_ptr<DiffBuilderBase>& subDiff) { BSONObjBuilder subObjBuilder = - output->subobjStart(kSubDiffSectionFieldName + idx); + output->subobjStart(kSubDiffSectionFieldPrefix + idx); subDiff->serializeTo(&subObjBuilder); }, [&idx, output](BSONElement elt) { @@ -109,30 +105,30 @@ void ArrayDiffBuilder::serializeTo(BSONObjBuilder* output) const { void DocumentDiffBuilder::serializeTo(BSONObjBuilder* output) const { if (!_deletes.empty()) { - BSONObjBuilder subBob(output->subobjStart(kDeleteSectionFieldName)); + BSONObjBuilder subBob(output->subobjStart(StringData(&kDeleteSectionFieldName, 1))); for (auto&& del : _deletes) { subBob.append(del, false); } } if (!_updates.empty()) { - BSONObjBuilder subBob(output->subobjStart(kUpdateSectionFieldName)); + BSONObjBuilder subBob(output->subobjStart(StringData(&kUpdateSectionFieldName, 1))); for (auto&& update : _updates) { subBob.appendAs(update.second, update.first); } } if (!_inserts.empty()) { - BSONObjBuilder subBob(output->subobjStart(kInsertSectionFieldName)); + BSONObjBuilder subBob(output->subobjStart(StringData(&kInsertSectionFieldName, 1))); for (auto&& insert : _inserts) { subBob.appendAs(insert.second, insert.first); } } if (!_subDiffs.empty()) { - BSONObjBuilder subBob(output->subobjStart(kSubDiffSectionFieldName)); for (auto&& subDiff : _subDiffs) { - BSONObjBuilder subDiffBuilder(subBob.subobjStart(subDiff.first)); + BSONObjBuilder subDiffBuilder( + output->subobjStart(std::string(1, kSubDiffSectionFieldPrefix) + subDiff.first)); subDiff.second->serializeTo(&subDiffBuilder); } } @@ -152,31 +148,31 @@ SubBuilderGuard<ArrayDiffBuilder> DocumentDiffBuilder::startSubArrDiff(StringDat } namespace { -void checkSection(BSONObjIterator* it, StringData sectionName, BSONType expectedType) { +void checkSection(BSONElement element, char sectionName, BSONType expectedType) { uassert(4770507, str::stream() << "Expected " << sectionName << " section to be type " << expectedType, - (**it).type() == expectedType); + element.type() == expectedType); } } // namespace ArrayDiffReader::ArrayDiffReader(const Diff& diff) : _diff(diff), _it(_diff) { assertDiffNonEmpty(_it); - + auto field = *_it; uassert(4770504, str::stream() << "Expected first field to be array header " << kArrayHeader << " but found " << (*_it).fieldNameStringData(), - (*_it).fieldNameStringData() == kArrayHeader); + field.fieldNameStringData() == kArrayHeader); uassert(4770519, str::stream() << "Expected array header to be bool but got " << (*_it), - (*_it).type() == BSONType::Bool); + field.type() == BSONType::Bool); uassert(4770520, str::stream() << "Expected array header to be value true but got " << (*_it), - (*_it).Bool()); + field.Bool()); ++_it; - - if (_it.more() && (*_it).fieldNameStringData() == kResizeSectionFieldName) { - checkSection(&_it, kResizeSectionFieldName, BSONType::NumberInt); - _newSize.emplace((*_it).numberInt()); + field = *_it; + if (_it.more() && field.fieldNameStringData() == kResizeSectionFieldName) { + checkSection(field, kResizeSectionFieldName[0], BSONType::NumberInt); + _newSize.emplace(field.numberInt()); ++_it; } } @@ -198,19 +194,16 @@ boost::optional<std::pair<size_t, ArrayDiffReader::ArrayModification>> ArrayDiff auto next = _it.next(); auto fieldName = next.fieldNameStringData(); - 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 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]) { + if (fieldName[0] == kUpdateSectionFieldName) { // It's an update. return {{idx, next}}; - } else if (fieldName[0] == kSubDiffSectionFieldName[0]) { + } else if (fieldName[0] == kSubDiffSectionFieldPrefix) { // It's a sub diff...But which type? uassert(4770501, str::stream() << "expected sub diff at index " << idx << " but got " << next, @@ -233,42 +226,49 @@ DocumentDiffReader::DocumentDiffReader(const Diff& diff) : _diff(diff) { BSONObjIterator it(diff); assertDiffNonEmpty(it); - // Find each seection of the diff and initialize an iterator. + // Find each section of the diff and initialize an iterator. struct Section { - StringData fieldName; boost::optional<BSONObjIterator>* outIterator; + int order; }; - const std::array<Section, 4> sections{ - Section{kDeleteSectionFieldName, &_deletes}, - Section{kUpdateSectionFieldName, &_updates}, - Section{kInsertSectionFieldName, &_inserts}, - Section{kSubDiffSectionFieldName, &_subDiffs}, - }; - - for (size_t i = 0; i < sections.size(); ++i) { - if (!it.more()) { - break; - } - - const auto fieldName = (*it).fieldNameStringData(); - if (fieldName != sections[i].fieldName) { - uassert(4770503, - str::stream() << "Unexpected section: " << fieldName << " in document diff", - kDocumentDiffSections.count(fieldName) != 0); - - continue; + const std::map<char, Section> sections{{kDeleteSectionFieldName, Section{&_deletes, 1}}, + {kUpdateSectionFieldName, Section{&_updates, 2}}, + {kInsertSectionFieldName, Section{&_inserts, 3}}, + {kSubDiffSectionFieldPrefix, Section{&_subDiffs, 4}}}; + + char prev = 0; + bool hasSubDiffSections = false; + for (; it.more(); ++it) { + const auto field = *it; + uassert(4770505, + str::stream() << "Expected sections field names in diff to be non-empty ", + field.fieldNameStringData().size()); + const auto sectionName = field.fieldNameStringData()[0]; + auto section = sections.find(sectionName); + if ((section != sections.end()) && (section->second.order > prev)) { + checkSection(field, sectionName, BSONType::Object); + + // Once we encounter a sub-diff section, we break and save the iterator for later use. + if (sectionName == kSubDiffSectionFieldPrefix) { + section->second.outIterator->emplace(it); + hasSubDiffSections = true; + break; + } else { + section->second.outIterator->emplace(field.embeddedObject()); + } + } else { + uasserted(4770503, + str::stream() + << "Unexpected section: " << sectionName << " in document diff"); } - - checkSection(&it, sections[i].fieldName, BSONType::Object); - sections[i].outIterator->emplace((*it).embeddedObject()); - ++it; + prev = section->second.order; } uassert(4770513, str::stream() << "Did not expect more sections in diff but found one: " << (*it).fieldNameStringData(), - !it.more()); + hasSubDiffSections || !it.more()); } boost::optional<StringData> DocumentDiffReader::nextDelete() { @@ -300,10 +300,16 @@ DocumentDiffReader::nextSubDiff() { } auto next = _subDiffs->next(); + const auto fieldName = next.fieldNameStringData(); + uassert(4770514, + str::stream() << "Did not expect more sections in diff but found one: " + << next.fieldNameStringData(), + fieldName.size() > 0 && fieldName[0] == kSubDiffSectionFieldPrefix); + uassert(470510, str::stream() << "Subdiffs should be objects, got " << next, next.type() == BSONType::Object); - return {{next.fieldNameStringData(), getReader(next.embeddedObject())}}; + return {{fieldName.substr(1, fieldName.size()), getReader(next.embeddedObject())}}; } } // namespace mongo::doc_diff diff --git a/src/mongo/db/update/document_diff_serialization.h b/src/mongo/db/update/document_diff_serialization.h index 3e56b226648..8ade179a875 100644 --- a/src/mongo/db/update/document_diff_serialization.h +++ b/src/mongo/db/update/document_diff_serialization.h @@ -47,16 +47,15 @@ enum DiffType : uint8_t { kDocument, kArray }; // Below are string constants used in the diff format. constexpr StringData kArrayHeader = "a"_sd; -constexpr StringData kDeleteSectionFieldName = "d"_sd; -constexpr StringData kInsertSectionFieldName = "i"_sd; -constexpr StringData kUpdateSectionFieldName = "u"_sd; +constexpr char kDeleteSectionFieldName = 'd'; +constexpr char kInsertSectionFieldName = 'i'; +constexpr char kUpdateSectionFieldName = 'u'; +constexpr char kSubDiffSectionFieldPrefix = 's'; // 'l' for length. 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; @@ -168,7 +167,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 */); + sizeTracker.addEntry(1 /* kResizeSectionFieldName */, sizeof(uint32_t) /* size of value */); } /* @@ -205,7 +204,7 @@ private: void finishChild() override { invariant(!_modifications.empty()); sizeTracker.addEntry( - _modifications.back().first.size() + kSubDiffSectionFieldName.size(), + _modifications.back().first.size() + 1 /* kSubDiffSectionFieldPrefix */, stdx::get<std::unique_ptr<DiffBuilderBase>>(_modifications.back().second) ->getObjSize()); } @@ -225,7 +224,11 @@ private: */ class DocumentDiffBuilder final : public DiffBuilderBase { public: - DocumentDiffBuilder(size_t padding = kAdditionalPaddingForObjectSize) + /** + * The 'padding' is the additional size that needs to be added to the diff builder while + * tracking the overall size of the diff. + */ + DocumentDiffBuilder(size_t padding = 0) : DiffBuilderBase(kSizeOfEmptyDocumentDiffBuilder + padding) {} /** @@ -284,10 +287,11 @@ private: invariant(!_subDiffs.empty()); // Add the size of 'field' + 'value'. - sizeTracker.addEntry(_subDiffs.back().first.size(), + sizeTracker.addEntry(1 /*kSubDiffSectionFieldPrefix */ + _subDiffs.back().first.size(), _subDiffs.back().second->getObjSize(), - _subDiffs.size() == 1); + false); } + std::vector<std::pair<StringData, BSONElement>> _updates; std::vector<std::pair<StringData, BSONElement>> _inserts; std::vector<StringData> _deletes; diff --git a/src/mongo/db/update/document_diff_serialization_test.cpp b/src/mongo/db/update/document_diff_serialization_test.cpp index fd793897715..97c2fcdf438 100644 --- a/src/mongo/db/update/document_diff_serialization_test.cpp +++ b/src/mongo/db/update/document_diff_serialization_test.cpp @@ -129,8 +129,7 @@ TEST(DiffSerializationTest, SubDiff) { auto out = builder.serialize(); ASSERT_BSONOBJ_BINARY_EQ( - out, - fromjson("{s :{obj: {d : { dField: false }, u : { uField: 'foo' }, i : { iField: 1}}}}")); + out, fromjson("{sobj: {d : { dField: false }, u : { uField: 'foo' }, i : { iField: 1}}}")); DocumentDiffReader reader(out); ASSERT(reader.nextUpdate() == boost::none); @@ -174,10 +173,10 @@ TEST(DiffSerializationTest, SubArrayWithSubDiff) { auto out = builder.serialize(); ASSERT_BSONOBJ_BINARY_EQ(out, - fromjson("{s: {arr: {a: true, l: 6," + fromjson("{sarr: {a: true, l: 6," "'u0': {foo: 'bar'}, " "'s2': {'d': {dField: false}}," - "'u5': 1}}}")); + "'u5': 1}}")); DocumentDiffReader reader(out); ASSERT(reader.nextUpdate() == boost::none); @@ -252,7 +251,7 @@ TEST(DiffSerializationTest, SubArrayNestedObject) { ASSERT_BSONOBJ_BINARY_EQ( out, fromjson( - "{s: {subArray: {a: true, 's1': {u: {a: 1}}, 's2': {u: {b: 2}}, 's3': {u: {c: 3}}}}}")); + "{ssubArray: {a: true, 's1': {u: {a: 1}}, 's2': {u: {b: 2}}, 's3': {u: {c: 3}}}}")); } TEST(DiffSerializationTest, SubArrayHighIndex) { @@ -266,7 +265,7 @@ TEST(DiffSerializationTest, SubArrayHighIndex) { } auto out = builder.serialize(); - ASSERT_BSONOBJ_BINARY_EQ(out, fromjson("{s: {subArray: {a: true, 'u254': 'foo'}}}")); + ASSERT_BSONOBJ_BINARY_EQ(out, fromjson("{ssubArray: {a: true, 'u254': 'foo'}}")); DocumentDiffReader reader(out); ASSERT(reader.nextUpdate() == boost::none); @@ -315,8 +314,8 @@ TEST(DiffSerializationTest, SubDiffAbandon) { auto out = builder.serialize(); ASSERT_BSONOBJ_BINARY_EQ( out, - fromjson("{d : {dField1: false, dField2: false}, u: {uField: 1}, s: {obj2: {d: " - "{dField2: false}}}}")); + fromjson("{d: {dField1: false, dField2: false}, u: {uField: 1}, sobj2: {d: " + "{dField2: false}}}")); } TEST(DiffSerializationTest, SubArrayDiffAbandon) { @@ -340,12 +339,13 @@ TEST(DiffSerializationTest, SubArrayDiffAbandon) { subBuilderGuard.builder()->setResize(5); } auto out = builder.serialize(); - ASSERT_BSONOBJ_BINARY_EQ(out, - fromjson("{d : {dField1: false, dField2: false}, u: {uField: 1}," - "s: {arr2: {a: true, l: 5}}}")); + ASSERT_BSONOBJ_BINARY_EQ( + out, + fromjson("{d : {dField1: false, dField2: false}, u: {uField: 1}, sarr2: {a: true, l: 5}}")); } TEST(DiffSerializationTest, ValidateComputeApproxSize) { + const size_t padding = 20; const auto storage = BSON("num" << 4 << "str" << "val" << "emptyStr" @@ -357,7 +357,7 @@ TEST(DiffSerializationTest, ValidateComputeApproxSize) { << BSON("" << "update")); - DocumentDiffBuilder builder; + DocumentDiffBuilder builder(padding); builder.addDelete("deleteField"); builder.addInsert("insert", storage["num"]); builder.addUpdate("update1", storage["subObj"]); @@ -409,7 +409,7 @@ TEST(DiffSerializationTest, ValidateComputeApproxSize) { auto computedSize = builder.getObjSize(); auto out = builder.serialize(); - ASSERT_EQ(computedSize, out.objsize() + kAdditionalPaddingForObjectSize); + ASSERT_EQ(computedSize, out.objsize() + padding); } TEST(DiffSerializationTest, ExecptionsWhileDiffBuildingDoesNotLeakMemory) { @@ -423,15 +423,33 @@ TEST(DiffSerializationTest, ExecptionsWhileDiffBuildingDoesNotLeakMemory) { } } TEST(DiffSerializationTest, UnexpectedFieldsInObjDiff) { + // Empty field names on top level. + ASSERT_THROWS_CODE( + applyDiff(BSONObj(), fromjson("{d: {f: false}, '' : {}}")), DBException, 4770505); + ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{'' : {}}")), DBException, 4770505); + + // Expected diff to be non-empty. + ASSERT_THROWS_CODE( + applyDiff(BSONObj(), fromjson("{d: {f: false}, s : {}}")), DBException, 4770500); + ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{sa : {}}")), DBException, 4770500); + 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); + ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{i : {}, d: {}}")), DBException, 4770503); + + ASSERT_THROWS_CODE( + applyDiff(BSONObj(), fromjson("{s : {i: {}}, i: {}}")), DBException, 4770514); + ASSERT_THROWS_CODE( + applyDiff(BSONObj(), fromjson("{sa : {u: {}}, d: {p: false}}")), DBException, 4770514); + ASSERT_THROWS_CODE(applyDiff(BSONObj(), fromjson("{sa : {u: {}}, sb : {u: {}}, i: {}}")), + DBException, + 4770514); + ASSERT_THROWS_CODE( + applyDiff(BSONObj(), fromjson("{sa : {u: {}}, p: {}}")), DBException, 4770514); // Empty deletes object is valid. ASSERT_BSONOBJ_BINARY_EQ(applyDiff(fromjson("{a: 1}"), fromjson("{d : {}}")), @@ -439,33 +457,27 @@ TEST(DiffSerializationTest, UnexpectedFieldsInObjDiff) { } 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}}}")), + ASSERT_THROWS_CODE(applyDiff(fromjson("{arr: []}"), fromjson("{sarr: {a: true, '3u' : 1}}")), + DBException, + 4770512); + ASSERT_THROWS_CODE(applyDiff(fromjson("{arr: []}"), fromjson("{sarr: {a: true, u : true}}")), + DBException, + 4770521); + ASSERT_THROWS_CODE(applyDiff(fromjson("{arr: []}"), fromjson("{sarr: {a: true, '5' : {}}}")), + DBException, + 4770521); + ASSERT_THROWS_CODE(applyDiff(fromjson("{arr: []}"), fromjson("{sarr: {a: false, 'u3' : 4}}")), + DBException, + 4770520); + ASSERT_THROWS_CODE(applyDiff(fromjson("{arr: []}"), fromjson("{sarr: {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); + ASSERT_THROWS_CODE(applyDiff(fromjson("{arr: []}"), fromjson("{sarr: {a: true, 's3' : 4}}")), + DBException, + 4770501); + ASSERT_THROWS_CODE(applyDiff(fromjson("{arr: []}"), fromjson("{sarr: {a: true, 'd3' : 4}}")), + DBException, + 4770502); } } // namespace diff --git a/src/mongo/db/update/document_diff_test.cpp b/src/mongo/db/update/document_diff_test.cpp index a928060a0e2..4b88e10eae5 100644 --- a/src/mongo/db/update/document_diff_test.cpp +++ b/src/mongo/db/update/document_diff_test.cpp @@ -36,6 +36,7 @@ #include "mongo/db/update/document_diff_applier.h" #include "mongo/db/update/document_diff_calculator.h" #include "mongo/db/update/document_diff_test_helpers.h" +#include "mongo/db/update/update_oplog_entry_serialization.h" #include "mongo/logv2/log.h" #include "mongo/platform/random.h" #include "mongo/unittest/bson_test_util.h" @@ -110,7 +111,8 @@ void runTest(std::vector<BSONObj> documents, size_t numSimulations) { std::vector<BSONObj> diffs; diffs.reserve(documents.size() - 1); for (size_t i = 1; i < documents.size(); ++i) { - const auto diff = computeDiff(preDoc, documents[i]); + const auto diff = computeDiff( + preDoc, documents[i], update_oplog_entry::kSizeOfDeltaOplogEntryMetadata); ASSERT(diff); diffs.push_back(*diff); diff --git a/src/mongo/db/update/pipeline_executor.cpp b/src/mongo/db/update/pipeline_executor.cpp index d12cfd7f292..fec64dccdf5 100644 --- a/src/mongo/db/update/pipeline_executor.cpp +++ b/src/mongo/db/update/pipeline_executor.cpp @@ -106,8 +106,10 @@ UpdateExecutor::ApplyResult PipelineExecutor::applyUpdate(ApplyParams applyParam if (applyParams.logMode != ApplyParams::LogMode::kDoNotGenerateOplogEntry && !ret.noop) { if (applyParams.logMode == ApplyParams::LogMode::kGenerateOplogEntry) { - // We're allowed to generate $v: 2 log entries. - const auto diff = doc_diff::computeDiff(originalDoc, transformedDoc); + // We're allowed to generate $v: 2 log entries. The $v:2 has certain meta-fields like + // '$v', 'diff'. So we pad some additional byte while computing diff. + const auto diff = doc_diff::computeDiff( + originalDoc, transformedDoc, update_oplog_entry::kSizeOfDeltaOplogEntryMetadata); if (diff) { ret.oplogEntry = update_oplog_entry::makeDeltaOplogEntry(*diff); return ret; diff --git a/src/mongo/db/update/pipeline_executor_test.cpp b/src/mongo/db/update/pipeline_executor_test.cpp index 2cabd0f5119..e1dfaf40fd8 100644 --- a/src/mongo/db/update/pipeline_executor_test.cpp +++ b/src/mongo/db/update/pipeline_executor_test.cpp @@ -157,18 +157,17 @@ TEST_F(PipelineExecutorTest, ComplexDoc) { std::vector<BSONObj> pipeline{fromjson("{$addFields: {a: 1, b: [0, 1, 2], c: {d: 1}}}")}; PipelineExecutor exec(expCtx, pipeline); - mutablebson::Document doc(fromjson("{a: 1, b: [0, 2, 2], e: []}")); + mutablebson::Document doc(fromjson("{a: 1, b: [0, 2, 2], e: ['val1', 'val2']}")); auto result = exec.applyUpdate(getApplyParams(doc.root())); ASSERT_FALSE(result.noop); ASSERT_TRUE(result.indexesAffected); - ASSERT_EQUALS(fromjson("{a: 1, b: [0, 1, 2], e: [], c: {d: 1}}"), doc); + ASSERT_EQUALS(fromjson("{a: 1, b: [0, 1, 2], e: ['val1', 'val2'], c: {d: 1}}"), doc); ASSERT_FALSE(doc.isInPlaceModeEnabled()); if (deltaOplogEntryAllowed()) { - ASSERT_BSONOBJ_BINARY_EQ( - fromjson("{$v: 2, diff: {i: {c: {d: 1}}, s: {b: {a: true, u1: 1} }}}"), - result.oplogEntry); + ASSERT_BSONOBJ_BINARY_EQ(fromjson("{$v: 2, diff: {i: {c: {d: 1}}, sb: {a: true, u1: 1} }}"), + result.oplogEntry); } else { - ASSERT_BSONOBJ_BINARY_EQ(fromjson("{a: 1, b: [0, 1, 2], e: [], c: {d: 1}}"), + ASSERT_BSONOBJ_BINARY_EQ(fromjson("{a: 1, b: [0, 1, 2], e: ['val1', 'val2'], c: {d: 1}}"), result.oplogEntry); } } diff --git a/src/mongo/db/update/update_oplog_entry_serialization.h b/src/mongo/db/update/update_oplog_entry_serialization.h index 286c6e3b756..cc751028efe 100644 --- a/src/mongo/db/update/update_oplog_entry_serialization.h +++ b/src/mongo/db/update/update_oplog_entry_serialization.h @@ -38,6 +38,8 @@ */ namespace mongo::update_oplog_entry { +constexpr size_t kSizeOfDeltaOplogEntryMetadata = 15; + /** * Given a diff, produce the contents for the 'o' field of a $v: 2 delta-style oplog entry. */ |