diff options
author | Mindaugas Malinauskas <mindaugas.malinauskas@mongodb.com> | 2020-08-19 16:41:55 +0300 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-08-24 12:16:37 +0000 |
commit | 8988d49ad6e5ebbbaa1d7b19a6e5b44d1c4dd87e (patch) | |
tree | d2fed1f99e936f50cb8d3ec43ff377a139f9b173 | |
parent | c986f14ab59177da4927429e41aa76f49a3079ff (diff) | |
download | mongo-8988d49ad6e5ebbbaa1d7b19a6e5b44d1c4dd87e.tar.gz |
SERVER-49234 Add '_id' field to generated errors for updates
-rw-r--r-- | jstests/core/doc_validation.js | 59 | ||||
-rw-r--r-- | jstests/core/doc_validation_options.js | 9 | ||||
-rw-r--r-- | src/mongo/db/matcher/doc_validation_error.cpp | 11 | ||||
-rw-r--r-- | src/mongo/db/matcher/doc_validation_error_test.cpp | 41 |
4 files changed, 108 insertions, 12 deletions
diff --git a/jstests/core/doc_validation.js b/jstests/core/doc_validation.js index 93deff56da9..51a3dddc953 100644 --- a/jstests/core/doc_validation.js +++ b/jstests/core/doc_validation.js @@ -287,4 +287,63 @@ assert.commandWorked(db.createCollection( {validator: {$expr: {$eq: ["$a", "foobar"]}}, collation: {locale: "en", strength: 2}})); assert.commandWorked(coll.insert({a: "foobar"})); assert.commandWorked(coll.insert({a: "fooBAR"})); + +// Recreate a collection with a simple validator. +coll.drop(); +assert.commandWorked(db.createCollection(collName, {validator: {a: 1}})); + +// Insert a document that fails validation. +res = coll.insert({_id: 1, a: 2}); + +// Verify that the document validation error attribute 'failingDocumentId' equals to the document +// '_id' attribute. +assertDocumentValidationFailure(res, coll); +if (coll.getMongo().writeMode() === 'commands') { + const errorInfo = res.getWriteError().errInfo; + const expectedError = { + failingDocumentId: 1, + details: { + operatorName: "$eq", + specifiedAs: {a: 1}, + reason: "comparison failed", + consideredValue: 2 + } + }; + assert.docEq(errorInfo, expectedError, tojson(res)); +} + +// Insert a valid document. +assert.commandWorked(coll.insert({_id: 1, a: 1})); + +// Issues the update command and returns the response. +function updateCommand(coll, query, update) { + return coll.update(query, update); +} + +// Issues the findAndModify command and returns the response. +function findAndModifyCommand(coll, query, update) { + return coll.runCommand("findAndModify", {query: query, update: update}); +} + +for (const command of [updateCommand, findAndModifyCommand]) { + // Attempt to update the document by replacing it with a document that does not pass validation. + const res = command(coll, {}, {a: 2}); + + // Verify that the document validation error attribute 'failingDocumentId' equals to the + // document '_id' attribute. + assertDocumentValidationFailure(res, coll); + if (coll.getMongo().writeMode() === 'commands') { + const errorInfo = (res instanceof WriteResult ? res.getWriteError() : res).errInfo; + const expectedError = { + failingDocumentId: 1, + details: { + operatorName: "$eq", + specifiedAs: {a: 1}, + reason: "comparison failed", + consideredValue: 2 + } + }; + assert.docEq(errorInfo, expectedError, tojson(res)); + } +} })(); diff --git a/jstests/core/doc_validation_options.js b/jstests/core/doc_validation_options.js index 0ec26697032..4b8e7e5b8e2 100644 --- a/jstests/core/doc_validation_options.js +++ b/jstests/core/doc_validation_options.js @@ -23,7 +23,7 @@ t.drop(); assert.commandWorked(db.createCollection(t.getName(), {validator: {a: 1}})); assertFailsValidation(t.insert({a: 2})); -t.insert({a: 1}); +t.insert({_id: 1, a: 1}); assert.eq(1, t.count()); // test default to strict @@ -42,10 +42,9 @@ assert.eq(1, t.find({a: 2}).itcount()); const conn = FixtureHelpers.getPrimaryForNodeHostingDatabase(db); const logId = 20294; const errInfo = { - "operatorName": "$eq", - "specifiedAs": {a: 1}, - "reason": "comparison failed", - "consideredValue": 2 + failingDocumentId: 1, + details: + {operatorName: "$eq", specifiedAs: {a: 1}, reason: "comparison failed", consideredValue: 2} }; checkLog.containsJson(conn, logId, { "errInfo": function(obj) { diff --git a/src/mongo/db/matcher/doc_validation_error.cpp b/src/mongo/db/matcher/doc_validation_error.cpp index f3e84fef1ae..c5320f88bc5 100644 --- a/src/mongo/db/matcher/doc_validation_error.cpp +++ b/src/mongo/db/matcher/doc_validation_error.cpp @@ -1003,7 +1003,16 @@ BSONObj generateError(const MatchExpression& validatorExpr, const BSONObj& doc) // There should be no frames when error generation is complete as the finished error will be // stored in 'context'. invariant(context.frames.empty()); - return context.getLatestCompleteError(); + BSONObjBuilder objBuilder; + + // Add document id to the error object. + BSONElement objectIdElement; + invariant(doc.getObjectID(objectIdElement)); + objBuilder.appendAs(objectIdElement, "failingDocumentId"_sd); + + // Add errors from match expressions. + objBuilder.append("details"_sd, context.getLatestCompleteError()); + return objBuilder.obj(); } } // namespace mongo::doc_validation_error diff --git a/src/mongo/db/matcher/doc_validation_error_test.cpp b/src/mongo/db/matcher/doc_validation_error_test.cpp index a36786412e5..666b8df6bf1 100644 --- a/src/mongo/db/matcher/doc_validation_error_test.cpp +++ b/src/mongo/db/matcher/doc_validation_error_test.cpp @@ -32,25 +32,54 @@ #include "mongo/db/matcher/doc_validation_error_test.h" namespace mongo::doc_validation_error { -void verifyGeneratedError(const BSONObj& query, - const BSONObj& document, - const BSONObj& expectedError) { +namespace { +/** + * Parses a MatchExpression from 'query' and returns a validation error generated by the parsed + * MatchExpression against document 'document'. + */ +BSONObj generateValidationError(const BSONObj& query, const BSONObj& document) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); expCtx->isParsingCollectionValidator = true; StatusWithMatchExpression result = MatchExpressionParser::parse(query, expCtx); ASSERT_OK(result.getStatus()); MatchExpression* expr = result.getValue().get(); - BSONObj generatedError = doc_validation_error::generateError(*expr, document); // Verify that the document fails to match against the query. ASSERT_FALSE(expr->matchesBSON(document)); - // Verify that the generated error matches the expected error. - ASSERT_BSONOBJ_EQ(generatedError, expectedError); + return doc_validation_error::generateError( + *expr, + document.hasField("_id") ? document : document.addField(BSON("_id" << 1).firstElement())); +} +} // namespace + +void verifyGeneratedError(const BSONObj& query, + const BSONObj& document, + const BSONObj& expectedError) { + auto generatedError = generateValidationError(query, document); + + // Verify that the generated error details match the expected error. + ASSERT_TRUE(generatedError.hasField("details")); + ASSERT_TRUE(generatedError["details"].isABSONObj()); + ASSERT_BSONOBJ_EQ(generatedError["details"].Obj(), expectedError); } namespace { +// Verifies that 'failingDocumentId' attribute is correctly set in a document validation error. +TEST(GenerateValidationError, FailingDocumentId) { + BSONObj query = BSON("a" << BSON("$eq" << 2)); + BSONObj document = BSON("a" << 1 << "_id" << 10); + BSONObj expectedError = BSON("failingDocumentId" << 10 << "details" + << BSON("operatorName" + << "$eq" + << "specifiedAs" << query << "reason" + << "comparison failed" + << "consideredValue" << 1)); + ASSERT_BSONOBJ_EQ(doc_validation_error::generateValidationError(query, document), + expectedError); +} + // Comparison operators. // $eq TEST(ComparisonMatchExpression, BasicEq) { |