diff options
author | Mihai Andrei <mihai.andrei@10gen.com> | 2020-10-21 13:36:21 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-10-22 14:24:59 +0000 |
commit | 587288e528cd82f1e455412828b545aee1852764 (patch) | |
tree | d1ece16160adb358fc85e50a411b56b075f3358f | |
parent | 6670266e11305c2ba2ab63fae6e4db49378abf62 (diff) | |
download | mongo-587288e528cd82f1e455412828b545aee1852764.tar.gz |
SERVER-51781 $sampleRate and $regex edge cases should have error annotations
-rw-r--r-- | src/mongo/db/matcher/doc_validation_error.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/matcher/doc_validation_error_test.cpp | 53 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser.cpp | 26 |
3 files changed, 71 insertions, 12 deletions
diff --git a/src/mongo/db/matcher/doc_validation_error.cpp b/src/mongo/db/matcher/doc_validation_error.cpp index 68c8e88df6c..f794eb029e3 100644 --- a/src/mongo/db/matcher/doc_validation_error.cpp +++ b/src/mongo/db/matcher/doc_validation_error.cpp @@ -732,8 +732,8 @@ public: } } void visit(const ExprMatchExpression* expr) final { - static constexpr auto kNormalReason = "$expr did not match"; - static constexpr auto kInvertedReason = "$expr did match"; + static constexpr auto kNormalReason = "expression did not match"; + static constexpr auto kInvertedReason = "expression did match"; _context->pushNewFrame(*expr); if (_context->shouldGenerateError(*expr)) { appendErrorDetails(*expr); diff --git a/src/mongo/db/matcher/doc_validation_error_test.cpp b/src/mongo/db/matcher/doc_validation_error_test.cpp index ba43dc3ea4c..adcc738f772 100644 --- a/src/mongo/db/matcher/doc_validation_error_test.cpp +++ b/src/mongo/db/matcher/doc_validation_error_test.cpp @@ -900,7 +900,7 @@ TEST(MiscellaneousMatchExpression, BasicExpr) { BSONObj expectedError = BSON("operatorName" << "$expr" << "specifiedAs" << query << "reason" - << "$expr did not match" + << "expression did not match" << "expressionResult" << false); doc_validation_error::verifyGeneratedError(query, document, expectedError); } @@ -918,7 +918,7 @@ TEST(MiscellaneousMatchExpression, NorExpr) { << BSON("operatorName" << "$expr" << "specifiedAs" << failingQuery << "reason" - << "$expr did match" + << "expression did match" << "expressionResult" << true)))); doc_validation_error::verifyGeneratedError(query, document, expectedError); } @@ -929,10 +929,33 @@ TEST(MiscellaneousMatchExpression, ExprImplicitArrayTraversal) { BSONObj expectedError = BSON("operatorName" << "$expr" << "specifiedAs" << query << "reason" - << "$expr did not match" + << "expression did not match" << "expressionResult" << false); doc_validation_error::verifyGeneratedError(query, document, expectedError); } +// $sampleRate +TEST(MiscellaneousMatchExpression, SampleRateAlwaysFalse) { + BSONObj query = fromjson("{$sampleRate: 0}"); + BSONObj document = fromjson("{'will this always fail?': 'yes!'}"); + BSONObj expectedError = BSON("operatorName" + << "$sampleRate" + << "specifiedAs" << query << "reason" + << "expression did not match" + << "expressionResult" << false); + doc_validation_error::verifyGeneratedError(query, document, expectedError); +} +TEST(MiscellaneousMatchExpression, SampleRateAlwaysTrue) { + BSONObj query = fromjson("{$nor: [{$sampleRate: 1}]}"); + BSONObj document = fromjson("{'will this always succeed?': 'yes!'}"); + BSONObj expectedError = fromjson( + "{operatorName: '$nor', 'clausesSatisfied': [" + " {index: 0, details:" + " {operatorName: '$sampleRate'," + " specifiedAs: {$sampleRate: 1}," + " reason: 'expression did match'," + " expressionResult: true}}]}"); + doc_validation_error::verifyGeneratedError(query, document, expectedError); +} // $mod TEST(MiscellaneousMatchExpression, BasicMod) { BSONObj query = BSON("a" << BSON("$mod" << BSON_ARRAY(2 << 1))); @@ -1137,6 +1160,30 @@ TEST(MiscellaneousMatchExpression, RegexImplicitArrayTraversalMixedTypes) { doc_validation_error::verifyGeneratedError(query, document, expectedError); } +TEST(MiscellaneousMatchExpression, RegexNoExplicitOperator) { + BSONObj query = BSON("a" << BSONRegEx("^S")); + BSONObj document = BSON("a" + << "so sorry; not capitalized"); + BSONObj expectedError = fromjson( + "{operatorName: '$regex'," + "specifiedAs: {a: /^S/}, " + "reason: 'regular expression did not match'," + "consideredValue: 'so sorry; not capitalized'}"); + doc_validation_error::verifyGeneratedError(query, document, expectedError); +} + +TEST(MiscellaneousMatchExpression, NotOverRegexNoExplicitOperator) { + BSONObj query = BSON("a" << BSON("$not" << BSONRegEx("^S"))); + BSONObj document = BSON("a" + << "S"); + BSONObj expectedError = fromjson( + "{operatorName: '$not', 'details': " + " {operatorName: '$regex'," + " specifiedAs: {a: /^S/}, " + " reason: 'regular expression did match'," + " consideredValue: 'S'}}"); + doc_validation_error::verifyGeneratedError(query, document, expectedError); +} // Verifies that $bitsAllClear expression with numeric bitmask correctly generates a validation // error. TEST(BitTestMatchExpression, GeneratesValidationErrorBitsAllClearNumeric) { diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index 1077c1ef8ec..2b21f86d75d 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -140,11 +140,17 @@ std::function<StatusWithMatchExpression(StringData, DocumentParseLevel)> retrievePathlessParser(StringData name); -StatusWithMatchExpression parseRegexElement(StringData name, BSONElement e) { +StatusWithMatchExpression parseRegexElement(StringData name, + BSONElement e, + const boost::intrusive_ptr<ExpressionContext>& expCtx) { if (e.type() != BSONType::RegEx) return {Status(ErrorCodes::BadValue, "not a regex")}; - return {std::make_unique<RegexMatchExpression>(name, e.regex(), e.regexFlags())}; + return {std::make_unique<RegexMatchExpression>( + name, + e.regex(), + e.regexFlags(), + doc_validation_error::createAnnotation(expCtx, "$regex", BSON(name << e)))}; } StatusWithMatchExpression parseComparison( @@ -297,7 +303,7 @@ StatusWithMatchExpression parse(const BSONObj& obj, } if (e.type() == BSONType::RegEx) { - auto result = parseRegexElement(e.fieldNameStringData(), e); + auto result = parseRegexElement(e.fieldNameStringData(), e, expCtx); if (!result.isOK()) return result; root->add(result.getValue().release()); @@ -382,10 +388,14 @@ StatusWithMatchExpression parseSampleRate(StringData name, return {Status(ErrorCodes::BadValue, "numeric argument to $sampleRate must be in [0, 1]")}; } else if (x == kRandomMinValue) { return std::make_unique<ExprMatchExpression>( - ExpressionConstant::create(expCtx.get(), Value(false)), expCtx); + ExpressionConstant::create(expCtx.get(), Value(false)), + expCtx, + doc_validation_error::createAnnotation(expCtx, "$sampleRate", elem.wrap())); } else if (x == kRandomMaxValue) { return std::make_unique<ExprMatchExpression>( - ExpressionConstant::create(expCtx.get(), Value(true)), expCtx); + ExpressionConstant::create(expCtx.get(), Value(true)), + expCtx, + doc_validation_error::createAnnotation(expCtx, "$sampleRate", elem.wrap())); } else { return std::make_unique<ExprMatchExpression>( Expression::parseExpression(expCtx.get(), @@ -1382,11 +1392,13 @@ StatusWithMatchExpression parseNot(StringData name, MatchExpressionParser::AllowedFeatureSet allowedFeatures, DocumentParseLevel currentLevel) { if (elem.type() == BSONType::RegEx) { - auto regex = parseRegexElement(name, elem); + auto regex = parseRegexElement(name, elem, expCtx); if (!regex.isOK()) { return regex; } - return {std::make_unique<NotMatchExpression>(regex.getValue().release())}; + return {std::make_unique<NotMatchExpression>( + regex.getValue().release(), + doc_validation_error::createAnnotation(expCtx, "$not", BSONObj()))}; } if (elem.type() != BSONType::Object) { |