diff options
author | Blake Oler <blake.oler@10gen.com> | 2017-10-09 11:48:26 -0400 |
---|---|---|
committer | Blake Oler <blake.oler@10gen.com> | 2017-10-09 11:48:40 -0400 |
commit | 95fedb251673c87b5172269e1f8c116b5e05fb16 (patch) | |
tree | c0fbd29a89addfb6bf54d4630c76772abf6c1564 | |
parent | d7a30a716243db13644a16618a939df6bc1344fc (diff) | |
download | mongo-95fedb251673c87b5172269e1f8c116b5e05fb16.tar.gz |
SERVER-31029 Add support for top-level $expr within $or and $and
-rw-r--r-- | jstests/aggregation/sources/graphLookup/filter.js | 28 | ||||
-rw-r--r-- | jstests/aggregation/sources/match/expr_match.js | 10 | ||||
-rw-r--r-- | jstests/core/expr_valid_positions.js | 23 | ||||
-rw-r--r-- | jstests/core/update_arrayFilters.js | 20 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser.cpp | 440 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser.h | 78 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser_test.cpp | 44 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser_tree.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_with_placeholder.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_with_placeholder.h | 14 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_with_placeholder_test.cpp | 173 | ||||
-rw-r--r-- | src/mongo/db/matcher/schema/expression_parser_schema_test.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/ops/parsed_update.cpp | 26 | ||||
-rw-r--r-- | src/mongo/db/update/update_array_node_test.cpp | 164 | ||||
-rw-r--r-- | src/mongo/db/update/update_object_node_test.cpp | 91 |
15 files changed, 725 insertions, 414 deletions
diff --git a/jstests/aggregation/sources/graphLookup/filter.js b/jstests/aggregation/sources/graphLookup/filter.js index b6eaff41b25..69027500aae 100644 --- a/jstests/aggregation/sources/graphLookup/filter.js +++ b/jstests/aggregation/sources/graphLookup/filter.js @@ -77,18 +77,18 @@ assert.eq(res.results.length, 1); // $expr is allowed inside the 'restrictSearchWithMatch' match expression. - // TODO SERVER-31029: This should not throw once support is added for top-level $expr within $or - // and $and. - assert.throws(function() { - local.aggregate({ - $graphLookup: { - from: "foreign", - startWith: "$starting", - connectFromField: "to", - connectToField: "from", - as: "results", - restrictSearchWithMatch: {$expr: {$eq: ["$shouldBeIncluded", true]}} - } - }); - }); + res = local + .aggregate({ + $graphLookup: { + from: "foreign", + startWith: "$starting", + connectFromField: "to", + connectToField: "from", + as: "results", + restrictSearchWithMatch: {$expr: {$eq: ["$shouldBeIncluded", true]}} + } + }) + .toArray()[0]; + + assert.eq(res.results.length, 1); })(); diff --git a/jstests/aggregation/sources/match/expr_match.js b/jstests/aggregation/sources/match/expr_match.js index 3f2056daf74..b2627c963cc 100644 --- a/jstests/aggregation/sources/match/expr_match.js +++ b/jstests/aggregation/sources/match/expr_match.js @@ -36,4 +36,14 @@ // $match with constant expression and no field path. assert.eq(4, coll.aggregate([{$match: {$expr: {$gte: [10, 5]}}}]).itcount()); assert.eq(0, coll.aggregate([{$match: {$expr: {$gte: [5, 10]}}}]).itcount()); + + // $match with $expr works inside a $or. + assert.eq(4, + coll.aggregate([{$match: {$or: [{$expr: {$eq: ["$foo", "$bar"]}}, {b: {$gt: 3}}]}}]) + .itcount()); + + // $match with $expr works inside a $and. + assert.eq(2, + coll.aggregate([{$match: {$and: [{$expr: {$eq: ["$foo", "$bar"]}}, {x: {$lt: 2}}]}}]) + .itcount()); })(); diff --git a/jstests/core/expr_valid_positions.js b/jstests/core/expr_valid_positions.js new file mode 100644 index 00000000000..df1f2470261 --- /dev/null +++ b/jstests/core/expr_valid_positions.js @@ -0,0 +1,23 @@ +// Verify that $expr can be used in the top-level position, but not in subdocuments. + +(function() { + "use strict"; + + const coll = db.expr_valid_positions; + + // Works at the BSON root level. + assert.eq(0, coll.find({$expr: {$eq: ["$foo", "$bar"]}}).itcount()); + + // Works inside a $or. + assert.eq(0, coll.find({$or: [{$expr: {$eq: ["$foo", "$bar"]}}, {b: {$gt: 3}}]}).itcount()); + + // Fails inside an elemMatch. + assert.throws(function() { + coll.find({a: {$elemMatch: {$expr: {$eq: ["$foo", "$bar"]}}}}).itcount(); + }); + + // Fails inside an _internalSchemaObjectMatch. + assert.throws(function() { + coll.find({a: {$_internalSchemaObjectMatch: {$expr: {$eq: ["$foo", "$bar"]}}}}).itcount(); + }); +}());
\ No newline at end of file diff --git a/jstests/core/update_arrayFilters.js b/jstests/core/update_arrayFilters.js index ce015fba4f9..fd6b5d15e4a 100644 --- a/jstests/core/update_arrayFilters.js +++ b/jstests/core/update_arrayFilters.js @@ -62,6 +62,26 @@ "Cannot use an expression without a top-level field name in arrayFilters"), "update failed for a reason other than missing a top-level field name in arrayFilter"); + // Array filter with $text inside fails to parse. + res = coll.update( + {_id: 0}, {$set: {"a.$[i]": 5}}, {arrayFilters: [{$text: {$search: "foo"}}]}); + assert.writeErrorWithCode(res, ErrorCodes.BadValue); + + // Array filter with $where inside fails to parse. + res = + coll.update({_id: 0}, {$set: {"a.$[i]": 5}}, {arrayFilters: [{$where: "this.a == 2"}]}); + assert.writeErrorWithCode(res, ErrorCodes.BadValue); + + // Array filter with $geoNear inside fails to parse. + res = coll.update( + {_id: 0}, {$set: {"a.$[i]": 5}}, {arrayFilters: [{loc: {$geoNear: [50, 50]}}]}); + assert.writeErrorWithCode(res, ErrorCodes.BadValue); + + // Array filter with $expr inside fails to parse. + res = coll.update( + {_id: 0}, {$set: {"a.$[i]": 5}}, {arrayFilters: [{$expr: {$eq: ["$foo", "$bar"]}}]}); + assert.writeErrorWithCode(res, ErrorCodes.QueryFeatureNotAllowed); + // Good value for arrayFilters succeeds. assert.writeOK(coll.update( {_id: 0}, {$set: {"a.$[i]": 5, "a.$[j]": 6}}, {arrayFilters: [{i: 0}, {j: 0}]})); diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index ddb6491739b..d235e36e964 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -89,15 +89,24 @@ bool hasNode(const MatchExpression* root, MatchExpression::MatchType type) { return false; } -const boost::container::flat_set<StringData> topLevelOperators{"$_internalSchemaAllowedProperties", - "$_internalSchemaMaxProperties", - "$_internalSchemaMinProperties", - "$_internalSchemaXor", - "$and", - "$nor", - "$or", - "$where"}; - +const boost::container::flat_set<StringData> kPathlessOperators{"$_internalSchemaAllowedProperties", + "$_internalSchemaCond", + "$_internalSchemaMaxProperties", + "$_internalSchemaMinProperties", + "$_internalSchemaRootDocEq", + "$_internalSchemaXor", + "$alwaysFalse", + "$alwaysTrue", + "$and", + "$atomic", + "$comment", + "$expr", + "$isolated" + "$jsonSchema", + "$nor", + "$or", + "$text", + "$where"}; } // namespace namespace mongo { @@ -145,23 +154,25 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField( const BSONElement& e, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel) { + DocumentParseLevel currentLevel) { if (mongoutils::str::equals("$eq", e.fieldName())) return _parseComparison(name, new EqualityMatchExpression(), e, expCtx, allowedFeatures); if (mongoutils::str::equals("$not", e.fieldName())) { - return _parseNot(name, e, expCtx, allowedFeatures, topLevel); + return _parseNot(name, e, expCtx, allowedFeatures, currentLevel); } auto parseExpMatchType = MatchExpressionParser::parsePathAcceptingKeyword(e); if (!parseExpMatchType) { - // $where cannot be a sub-expression because it works on top-level documents only. - if (mongoutils::str::equals("$where", e.fieldName())) { - return {Status(ErrorCodes::BadValue, "$where cannot be applied to a field")}; + if (kPathlessOperators.find(e.fieldNameStringData()) != kPathlessOperators.end()) { + return {Status(ErrorCodes::FailedToParse, + mongoutils::str::stream() << e.fieldNameStringData() + << " cannot be applied to a field path.")}; } - return {Status(ErrorCodes::BadValue, - mongoutils::str::stream() << "unknown operator: " << e.fieldName())}; + return { + Status(ErrorCodes::BadValue, + mongoutils::str::stream() << "unknown operator: " << e.fieldNameStringData())}; } switch (*parseExpMatchType) { @@ -299,10 +310,10 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField( } case PathAcceptingKeyword::ELEM_MATCH: - return _parseElemMatch(name, e, expCtx, allowedFeatures, topLevel); + return _parseElemMatch(name, e, expCtx, allowedFeatures); case PathAcceptingKeyword::ALL: - return _parseAll(name, e, expCtx, allowedFeatures, topLevel); + return _parseAll(name, e, expCtx, allowedFeatures); case PathAcceptingKeyword::WITHIN: case PathAcceptingKeyword::GEO_INTERSECTS: @@ -349,7 +360,8 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField( str::stream() << "$_internalSchemaObjectMatch must be an object"); } - auto parsedSubObjExpr = _parse(e.Obj(), expCtx, allowedFeatures, topLevel); + auto parsedSubObjExpr = + _parse(e.Obj(), expCtx, allowedFeatures, DocumentParseLevel::kUserSubDocument); if (!parsedSubObjExpr.isOK()) { return parsedSubObjExpr; } @@ -387,7 +399,8 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField( } case PathAcceptingKeyword::INTERNAL_SCHEMA_MATCH_ARRAY_INDEX: { - return _parseInternalSchemaMatchArrayIndex(name, e, expCtx); + return _parseInternalSchemaMatchArrayIndex( + name, e, expCtx, allowedFeatures, currentLevel); } case PathAcceptingKeyword::INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX: { @@ -435,8 +448,17 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField( << "must be an object"); } + auto filter = _parse(second.embeddedObject(), + expCtx, + kBanAllSpecialFeatures, + DocumentParseLevel::kUserSubDocument); + + if (!filter.isOK()) { + return filter.getStatus(); + } + auto exprWithPlaceholder = - ExpressionWithPlaceholder::parse(second.embeddedObject(), expCtx); + ExpressionWithPlaceholder::make(std::move(filter.getValue())); if (!exprWithPlaceholder.isOK()) { return exprWithPlaceholder.getStatus(); } @@ -472,10 +494,12 @@ StatusWithMatchExpression MatchExpressionParser::_parse( const BSONObj& obj, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel) { + DocumentParseLevel currentLevel) { std::unique_ptr<AndMatchExpression> root = stdx::make_unique<AndMatchExpression>(); - const bool childIsTopLevel = false; + const DocumentParseLevel nextLevel = (currentLevel == DocumentParseLevel::kPredicateTopLevel) + ? DocumentParseLevel::kUserDocumentTopLevel + : currentLevel; BSONObjIterator i(obj); while (i.more()) { BSONElement e = i.next(); @@ -486,8 +510,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse( if (e.type() != Array) return {Status(ErrorCodes::BadValue, "$or must be an array")}; std::unique_ptr<OrMatchExpression> temp = stdx::make_unique<OrMatchExpression>(); - Status s = - _parseTreeList(e.Obj(), temp.get(), expCtx, allowedFeatures, childIsTopLevel); + Status s = _parseTreeList(e.Obj(), temp.get(), expCtx, allowedFeatures, nextLevel); if (!s.isOK()) return s; root->add(temp.release()); @@ -495,8 +518,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse( if (e.type() != Array) return {Status(ErrorCodes::BadValue, "$and must be an array")}; std::unique_ptr<AndMatchExpression> temp = stdx::make_unique<AndMatchExpression>(); - Status s = - _parseTreeList(e.Obj(), temp.get(), expCtx, allowedFeatures, childIsTopLevel); + Status s = _parseTreeList(e.Obj(), temp.get(), expCtx, allowedFeatures, nextLevel); if (!s.isOK()) return s; root->add(temp.release()); @@ -504,16 +526,16 @@ StatusWithMatchExpression MatchExpressionParser::_parse( if (e.type() != Array) return {Status(ErrorCodes::BadValue, "$nor must be an array")}; std::unique_ptr<NorMatchExpression> temp = stdx::make_unique<NorMatchExpression>(); - Status s = - _parseTreeList(e.Obj(), temp.get(), expCtx, allowedFeatures, childIsTopLevel); + Status s = _parseTreeList(e.Obj(), temp.get(), expCtx, allowedFeatures, nextLevel); if (!s.isOK()) return s; root->add(temp.release()); } else if (mongoutils::str::equals("atomic", rest) || mongoutils::str::equals("isolated", rest)) { - if (!topLevel) - return {Status(ErrorCodes::BadValue, + if (currentLevel != DocumentParseLevel::kPredicateTopLevel) { + return {Status(ErrorCodes::FailedToParse, "$atomic/$isolated has to be at the top level")}; + } // Don't do anything with the expression; CanonicalQuery::init() will look through // the BSONObj again for a $atomic/$isolated. } else if (mongoutils::str::equals("where", rest)) { @@ -521,13 +543,19 @@ StatusWithMatchExpression MatchExpressionParser::_parse( return {Status(ErrorCodes::BadValue, "$where is not allowed in this context")}; } + if (currentLevel == DocumentParseLevel::kUserSubDocument) { + return {Status(ErrorCodes::FailedToParse, + "$where can only be applied to the top-level document")}; + } + StatusWithMatchExpression s = _extensionsCallback->parseWhere(e); if (!s.isOK()) return s; root->add(s.getValue().release()); } else if (mongoutils::str::equals("expr", rest)) { - if (!topLevel) { - return {Status(ErrorCodes::BadValue, "$expr has to be at the top level")}; + if (currentLevel == DocumentParseLevel::kUserSubDocument) { + return {Status(ErrorCodes::FailedToParse, + "$expr can only be applied to the top-level document")}; } auto status = _parseExpr(e, allowedFeatures, expCtx); @@ -558,7 +586,8 @@ StatusWithMatchExpression MatchExpressionParser::_parse( root->add(eq.release()); } else if (mongoutils::str::equals("_internalSchemaAllowedProperties", rest)) { - auto allowedProperties = _parseInternalSchemaAllowedProperties(e, expCtx); + auto allowedProperties = + _parseInternalSchemaAllowedProperties(e, expCtx, allowedFeatures, nextLevel); if (!allowedProperties.isOK()) { return allowedProperties.getStatus(); } @@ -566,7 +595,11 @@ StatusWithMatchExpression MatchExpressionParser::_parse( } else if (mongoutils::str::equals("_internalSchemaCond", rest)) { auto condExpr = _parseInternalSchemaFixedArityArgument<InternalSchemaCondMatchExpression>( - InternalSchemaCondMatchExpression::kName, e, expCtx, allowedFeatures); + InternalSchemaCondMatchExpression::kName, + e, + expCtx, + allowedFeatures, + nextLevel); if (!condExpr.isOK()) { return condExpr.getStatus(); } @@ -577,8 +610,8 @@ StatusWithMatchExpression MatchExpressionParser::_parse( return { Status(ErrorCodes::TypeMismatch, "$_internalSchemaXor must be an array")}; auto xorExpr = stdx::make_unique<InternalSchemaXorMatchExpression>(); - Status s = _parseTreeList( - e.Obj(), xorExpr.get(), expCtx, allowedFeatures, childIsTopLevel); + Status s = + _parseTreeList(e.Obj(), xorExpr.get(), expCtx, allowedFeatures, nextLevel); if (!s.isOK()) return s; root->add(xorExpr.release()); @@ -639,7 +672,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse( auto alwaysTrueExpr = stdx::make_unique<AlwaysTrueMatchExpression>(); root->add(alwaysTrueExpr.release()); } else if (mongoutils::str::equals("_internalSchemaRootDocEq", rest)) { - if (!topLevel) { + if (currentLevel != DocumentParseLevel::kPredicateTopLevel) { return {Status(ErrorCodes::FailedToParse, str::stream() << InternalSchemaRootDocEqMatchExpression::kName << " must be at the top level")}; @@ -664,8 +697,8 @@ StatusWithMatchExpression MatchExpressionParser::_parse( } if (_isExpressionDocument(e, false)) { - Status s = _parseSub( - e.fieldName(), e.Obj(), root.get(), expCtx, allowedFeatures, childIsTopLevel); + Status s = + _parseSub(e.fieldName(), e.Obj(), root.get(), expCtx, allowedFeatures, nextLevel); if (!s.isOK()) return s; continue; @@ -701,7 +734,7 @@ Status MatchExpressionParser::_parseSub(const char* name, AndMatchExpression* root, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel) { + DocumentParseLevel currentLevel) { // The one exception to {field : {fully contained argument} } is, of course, geo. Example: // sub == { field : {$near[Sphere]: [0,0], $maxDistance: 1000, $minDistance: 10 } } // We peek inside of 'sub' to see if it's possibly a $near. If so, we can't iterate over @@ -733,9 +766,8 @@ Status MatchExpressionParser::_parseSub(const char* name, while (j.more()) { BSONElement deep = j.next(); - const bool childIsTopLevel = false; StatusWithMatchExpression s = - _parseSubField(sub, root, name, deep, expCtx, allowedFeatures, childIsTopLevel); + _parseSubField(sub, root, name, deep, expCtx, allowedFeatures, currentLevel); if (!s.isOK()) return s.getStatus(); @@ -945,21 +977,15 @@ StatusWithMatchExpression MatchExpressionParser::_parseElemMatch( const char* name, const BSONElement& e, const boost::intrusive_ptr<ExpressionContext>& expCtx, - AllowedFeatureSet allowedFeatures, - bool topLevel) { + AllowedFeatureSet allowedFeatures) { if (e.type() != Object) return {Status(ErrorCodes::BadValue, "$elemMatch needs an Object")}; BSONObj obj = e.Obj(); - // $elemMatch value case applies when the children all - // work on the field 'name'. - // This is the case when: - // 1) the argument is an expression document; and - // 2) expression is not a AND/NOR/OR logical operator. Children of - // these logical operators are initialized with field names. - // 3) expression is not a WHERE operator. WHERE works on objects instead - // of specific field. + // We distinguish here between $elemMatch value and $elemMatch object. The $elemMatch can only + // be $elemMatch value if the argument is an expression document whose operator is + // path-accepting. bool isElemMatchValue = false; if (_isExpressionDocument(e, true)) { BSONObj o = e.Obj(); @@ -967,14 +993,15 @@ StatusWithMatchExpression MatchExpressionParser::_parseElemMatch( invariant(!elt.eoo()); isElemMatchValue = - topLevelOperators.find(elt.fieldNameStringData()) == topLevelOperators.end(); + kPathlessOperators.find(elt.fieldNameStringData()) == kPathlessOperators.end(); } if (isElemMatchValue) { // value case AndMatchExpression theAnd; - Status s = _parseSub("", obj, &theAnd, expCtx, allowedFeatures, topLevel); + Status s = _parseSub( + "", obj, &theAnd, expCtx, allowedFeatures, DocumentParseLevel::kUserSubDocument); if (!s.isOK()) return s; @@ -998,17 +1025,12 @@ StatusWithMatchExpression MatchExpressionParser::_parseElemMatch( // object case - StatusWithMatchExpression subRaw = _parse(obj, expCtx, allowedFeatures, topLevel); + StatusWithMatchExpression subRaw = + _parse(obj, expCtx, allowedFeatures, DocumentParseLevel::kUserSubDocument); if (!subRaw.isOK()) return subRaw; std::unique_ptr<MatchExpression> sub = std::move(subRaw.getValue()); - // $where is not supported under $elemMatch because $where - // applies to top-level document, not array elements in a field. - if (hasNode(sub.get(), MatchExpression::WHERE)) { - return {Status(ErrorCodes::BadValue, "$elemMatch cannot contain $where expression")}; - } - std::unique_ptr<ElemMatchObjectMatchExpression> temp = stdx::make_unique<ElemMatchObjectMatchExpression>(); Status status = temp->init(name, sub.release()); @@ -1022,8 +1044,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseAll( const char* name, const BSONElement& e, const boost::intrusive_ptr<ExpressionContext>& expCtx, - AllowedFeatureSet allowedFeatures, - bool topLevel) { + AllowedFeatureSet allowedFeatures) { if (e.type() != Array) return {Status(ErrorCodes::BadValue, "$all needs an array")}; @@ -1051,12 +1072,8 @@ StatusWithMatchExpression MatchExpressionParser::_parseAll( return {Status(ErrorCodes::BadValue, "$all/$elemMatch has to be consistent")}; } - const bool childIsTopLevel = false; - StatusWithMatchExpression inner = _parseElemMatch(name, - hopefullyElemMatchObj.firstElement(), - expCtx, - allowedFeatures, - childIsTopLevel); + StatusWithMatchExpression inner = _parseElemMatch( + name, hopefullyElemMatchObj.firstElement(), expCtx, allowedFeatures); if (!inner.isOK()) return inner; myAnd->add(inner.getValue().release()); @@ -1267,6 +1284,128 @@ StatusWith<long long> MatchExpressionParser::parseIntegerElementToLong(BSONEleme return number; } +StatusWith<std::unique_ptr<ExpressionWithPlaceholder>> +MatchExpressionParser::_parseExprWithPlaceholder( + const BSONObj& containingObject, + StringData exprWithPlaceholderFieldName, + StringData expressionName, + StringData expectedPlaceholder, + const boost::intrusive_ptr<ExpressionContext>& expCtx, + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel) { + auto exprWithPlaceholderElem = containingObject[exprWithPlaceholderFieldName]; + if (!exprWithPlaceholderElem) { + return {ErrorCodes::FailedToParse, + str::stream() << expressionName << " requires '" << exprWithPlaceholderFieldName + << "'"}; + } else if (exprWithPlaceholderElem.type() != BSONType::Object) { + return {ErrorCodes::TypeMismatch, + str::stream() << expressionName << " found '" << exprWithPlaceholderFieldName + << "', which is an incompatible type: " + << exprWithPlaceholderElem.type()}; + } + + auto filter = _parse( + exprWithPlaceholderElem.embeddedObject(), expCtx, kBanAllSpecialFeatures, currentLevel); + + if (!filter.isOK()) { + return filter.getStatus(); + } + + auto result = ExpressionWithPlaceholder::make(std::move(filter.getValue())); + if (!result.isOK()) { + return result.getStatus(); + } + + auto placeholder = result.getValue()->getPlaceholder(); + if (placeholder && (*placeholder != expectedPlaceholder)) { + return {ErrorCodes::FailedToParse, + str::stream() << expressionName << " expected a name placeholder of " + << expectedPlaceholder + << ", but '" + << exprWithPlaceholderElem.fieldName() + << "' has a mismatching placeholder '" + << *placeholder + << "'"}; + } + return result; +} + +StatusWith<std::vector<InternalSchemaAllowedPropertiesMatchExpression::PatternSchema>> +MatchExpressionParser::_parsePatternProperties( + BSONElement patternPropertiesElem, + StringData expectedPlaceholder, + const boost::intrusive_ptr<ExpressionContext>& expCtx, + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel) { + if (!patternPropertiesElem) { + return {ErrorCodes::FailedToParse, + str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName + << " requires 'patternProperties'"}; + } else if (patternPropertiesElem.type() != BSONType::Array) { + return {ErrorCodes::TypeMismatch, + str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName + << " requires 'patternProperties' to be an array, not " + << patternPropertiesElem.type()}; + } + + std::vector<InternalSchemaAllowedPropertiesMatchExpression::PatternSchema> patternProperties; + for (auto&& constraintElem : patternPropertiesElem.embeddedObject()) { + if (constraintElem.type() != BSONType::Object) { + return {ErrorCodes::TypeMismatch, + str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName + << " requires 'patternProperties' to be an array of objects"}; + } + + auto constraint = constraintElem.embeddedObject(); + if (constraint.nFields() != 2) { + return {ErrorCodes::FailedToParse, + str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName + << " requires 'patternProperties' to be an array of objects " + "containing exactly two fields, 'regex' and 'expression'"}; + } + + auto expressionWithPlaceholder = + _parseExprWithPlaceholder(constraint, + "expression"_sd, + InternalSchemaAllowedPropertiesMatchExpression::kName, + expectedPlaceholder, + expCtx, + allowedFeatures, + currentLevel); + if (!expressionWithPlaceholder.isOK()) { + return expressionWithPlaceholder.getStatus(); + } + + auto regexElem = constraint["regex"]; + if (!regexElem) { + return { + ErrorCodes::FailedToParse, + str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName + << " requires each object in 'patternProperties' to have a 'regex'"}; + } + if (regexElem.type() != BSONType::RegEx) { + return {ErrorCodes::TypeMismatch, + str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName + << " requires 'patternProperties' to be an array of objects, " + "where 'regex' is a regular expression"}; + } + if (*regexElem.regexFlags() != '\0') { + return { + ErrorCodes::BadValue, + str::stream() + << InternalSchemaAllowedPropertiesMatchExpression::kName + << " does not accept regex flags for pattern schemas in 'patternProperties'"}; + } + + patternProperties.emplace_back( + InternalSchemaAllowedPropertiesMatchExpression::Pattern(regexElem.regex()), + std::move(expressionWithPlaceholder.getValue())); + } + + return std::move(patternProperties); +} + StatusWithMatchExpression MatchExpressionParser::_parseInternalSchemaFmod(const char* name, const BSONElement& elem) { StringData path(name); @@ -1307,7 +1446,8 @@ StatusWithMatchExpression MatchExpressionParser::_parseInternalSchemaFixedArityA StringData name, const BSONElement& input, const boost::intrusive_ptr<ExpressionContext>& expCtx, - AllowedFeatureSet allowedFeatures) { + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel) { constexpr auto arity = T::arity(); if (input.type() != BSONType::Array) { return {ErrorCodes::FailedToParse, @@ -1335,8 +1475,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseInternalSchemaFixedArityA << elem.type()}; } - const bool isTopLevel = false; - auto subexpr = _parse(elem.embeddedObject(), expCtx, allowedFeatures, isTopLevel); + auto subexpr = _parse(elem.embeddedObject(), expCtx, allowedFeatures, currentLevel); if (!subexpr.isOK()) { return subexpr.getStatus(); } @@ -1404,120 +1543,6 @@ StatusWith<StringData> parseNamePlaceholder(const BSONObj& containingObject, return {namePlaceholderElem.valueStringData()}; } -/** - * Looks at the field named 'exprWithPlaceholderFieldName' within 'containingObject' and parses an - * ExpressionWithPlaceholder from that element. Fails if an error occurs during parsing, or if the - * ExpressionWithPlaceholder has a different name placeholder than 'expectedPlaceholder'. - * 'expressionName' is the name of the expression that requires the ExpressionWithPlaceholder and is - * used to generate helpful error messages. - */ -StatusWith<std::unique_ptr<ExpressionWithPlaceholder>> parseExprWithPlaceholder( - const BSONObj& containingObject, - StringData exprWithPlaceholderFieldName, - StringData expressionName, - StringData expectedPlaceholder, - const boost::intrusive_ptr<ExpressionContext>& expCtx) { - auto exprWithPlaceholderElem = containingObject[exprWithPlaceholderFieldName]; - if (!exprWithPlaceholderElem) { - return {ErrorCodes::FailedToParse, - str::stream() << expressionName << " requires '" << exprWithPlaceholderFieldName - << "'"}; - } else if (exprWithPlaceholderElem.type() != BSONType::Object) { - return {ErrorCodes::TypeMismatch, - str::stream() << expressionName << " found '" << exprWithPlaceholderFieldName - << "', which is an incompatible type: " - << exprWithPlaceholderElem.type()}; - } - - auto result = - ExpressionWithPlaceholder::parse(exprWithPlaceholderElem.embeddedObject(), expCtx); - if (!result.isOK()) { - return result.getStatus(); - } - - auto placeholder = result.getValue()->getPlaceholder(); - if (placeholder && (*placeholder != expectedPlaceholder)) { - return {ErrorCodes::FailedToParse, - str::stream() << expressionName << " expected a name placeholder of " - << expectedPlaceholder - << ", but '" - << exprWithPlaceholderElem.fieldName() - << "' has a mismatching placeholder '" - << *placeholder - << "'"}; - } - return result; -} - -StatusWith<std::vector<InternalSchemaAllowedPropertiesMatchExpression::PatternSchema>> -parsePatternProperties(BSONElement patternPropertiesElem, - StringData expectedPlaceholder, - const boost::intrusive_ptr<ExpressionContext>& expCtx) { - if (!patternPropertiesElem) { - return {ErrorCodes::FailedToParse, - str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName - << " requires 'patternProperties'"}; - } else if (patternPropertiesElem.type() != BSONType::Array) { - return {ErrorCodes::TypeMismatch, - str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName - << " requires 'patternProperties' to be an array, not " - << patternPropertiesElem.type()}; - } - - std::vector<InternalSchemaAllowedPropertiesMatchExpression::PatternSchema> patternProperties; - for (auto&& constraintElem : patternPropertiesElem.embeddedObject()) { - if (constraintElem.type() != BSONType::Object) { - return {ErrorCodes::TypeMismatch, - str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName - << " requires 'patternProperties' to be an array of objects"}; - } - - auto constraint = constraintElem.embeddedObject(); - if (constraint.nFields() != 2) { - return {ErrorCodes::FailedToParse, - str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName - << " requires 'patternProperties' to be an array of objects " - "containing exactly two fields, 'regex' and 'expression'"}; - } - - auto expressionWithPlaceholder = - parseExprWithPlaceholder(constraint, - "expression"_sd, - InternalSchemaAllowedPropertiesMatchExpression::kName, - expectedPlaceholder, - expCtx); - if (!expressionWithPlaceholder.isOK()) { - return expressionWithPlaceholder.getStatus(); - } - - auto regexElem = constraint["regex"]; - if (!regexElem) { - return { - ErrorCodes::FailedToParse, - str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName - << " requires each object in 'patternProperties' to have a 'regex'"}; - } - if (regexElem.type() != BSONType::RegEx) { - return {ErrorCodes::TypeMismatch, - str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName - << " requires 'patternProperties' to be an array of objects, " - "where 'regex' is a regular expression"}; - } else if (*regexElem.regexFlags() != '\0') { - return { - ErrorCodes::BadValue, - str::stream() - << InternalSchemaAllowedPropertiesMatchExpression::kName - << " does not accept regex flags for pattern schemas in 'patternProperties'"}; - } - - patternProperties.emplace_back( - InternalSchemaAllowedPropertiesMatchExpression::Pattern(regexElem.regex()), - std::move(expressionWithPlaceholder.getValue())); - } - - return std::move(patternProperties); -} - StatusWith<boost::container::flat_set<StringData>> parseProperties(BSONElement propertiesElem) { if (!propertiesElem) { return {ErrorCodes::FailedToParse, @@ -1549,7 +1574,9 @@ StatusWith<boost::container::flat_set<StringData>> parseProperties(BSONElement p StatusWithMatchExpression MatchExpressionParser::_parseInternalSchemaMatchArrayIndex( const char* path, const BSONElement& elem, - const boost::intrusive_ptr<ExpressionContext>& expCtx) { + const boost::intrusive_ptr<ExpressionContext>& expCtx, + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel) { if (elem.type() != BSONType::Object) { return {ErrorCodes::TypeMismatch, str::stream() << InternalSchemaMatchArrayIndexMatchExpression::kName @@ -1576,11 +1603,13 @@ StatusWithMatchExpression MatchExpressionParser::_parseInternalSchemaMatchArrayI } auto expressionWithPlaceholder = - parseExprWithPlaceholder(subobj, - "expression"_sd, - InternalSchemaMatchArrayIndexMatchExpression::kName, - namePlaceholder.getValue(), - expCtx); + _parseExprWithPlaceholder(subobj, + "expression"_sd, + InternalSchemaMatchArrayIndexMatchExpression::kName, + namePlaceholder.getValue(), + expCtx, + allowedFeatures, + currentLevel); if (!expressionWithPlaceholder.isOK()) { return expressionWithPlaceholder.getStatus(); } @@ -1595,7 +1624,10 @@ StatusWithMatchExpression MatchExpressionParser::_parseInternalSchemaMatchArrayI } StatusWithMatchExpression MatchExpressionParser::_parseInternalSchemaAllowedProperties( - const BSONElement& elem, const boost::intrusive_ptr<ExpressionContext>& expCtx) { + const BSONElement& elem, + const boost::intrusive_ptr<ExpressionContext>& expCtx, + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel) { if (elem.type() != BSONType::Object) { return {ErrorCodes::TypeMismatch, str::stream() << InternalSchemaAllowedPropertiesMatchExpression::kName @@ -1616,17 +1648,23 @@ StatusWithMatchExpression MatchExpressionParser::_parseInternalSchemaAllowedProp return namePlaceholder.getStatus(); } - auto patternProperties = - parsePatternProperties(subobj["patternProperties"], namePlaceholder.getValue(), expCtx); + auto patternProperties = _parsePatternProperties(subobj["patternProperties"], + namePlaceholder.getValue(), + expCtx, + allowedFeatures, + currentLevel); if (!patternProperties.isOK()) { return patternProperties.getStatus(); } - auto otherwise = parseExprWithPlaceholder(subobj, - "otherwise"_sd, - InternalSchemaAllowedPropertiesMatchExpression::kName, - namePlaceholder.getValue(), - expCtx); + auto otherwise = + _parseExprWithPlaceholder(subobj, + "otherwise"_sd, + InternalSchemaAllowedPropertiesMatchExpression::kName, + namePlaceholder.getValue(), + expCtx, + allowedFeatures, + currentLevel); if (!otherwise.isOK()) { return otherwise.getStatus(); } diff --git a/src/mongo/db/matcher/expression_parser.h b/src/mongo/db/matcher/expression_parser.h index 1adb7640ed1..6091d3f1426 100644 --- a/src/mongo/db/matcher/expression_parser.h +++ b/src/mongo/db/matcher/expression_parser.h @@ -36,8 +36,10 @@ #include "mongo/db/matcher/expression_leaf.h" #include "mongo/db/matcher/expression_tree.h" #include "mongo/db/matcher/expression_type.h" +#include "mongo/db/matcher/expression_with_placeholder.h" #include "mongo/db/matcher/extensions_callback.h" #include "mongo/db/matcher/extensions_callback_noop.h" +#include "mongo/db/matcher/schema/expression_internal_schema_allowed_properties.h" #include "mongo/db/pipeline/expression.h" #include "mongo/db/pipeline/expression_context.h" #include "mongo/stdx/functional.h" @@ -127,9 +129,8 @@ public: const ExtensionsCallback& extensionsCallback = ExtensionsCallbackNoop(), AllowedFeatureSet allowedFeatures = kDefaultSpecialFeatures) { invariant(expCtx.get()); - const bool topLevelCall = true; return MatchExpressionParser(&extensionsCallback) - ._parse(obj, expCtx, allowedFeatures, topLevelCall); + ._parse(obj, expCtx, allowedFeatures, DocumentParseLevel::kPredicateTopLevel); } /** @@ -154,6 +155,23 @@ public: static StatusWith<long long> parseIntegerElementToLong(BSONElement elem); private: + /** + * 'DocumentParseLevel' refers to the current position of the parser as it descends a + * MatchExpression tree. + */ + enum class DocumentParseLevel { + // Indicates that the parser is looking at the root level of the BSON object containing the + // user's query predicate. + kPredicateTopLevel, + // Indicates that match expression nodes in this position will match against the complete + // user document, as opposed to matching against a nested document or a subdocument inside + // an array. + kUserDocumentTopLevel, + // Indicates that match expression nodes in this position will match against a nested + // document or a subdocument inside an array. + kUserSubDocument, + }; + MatchExpressionParser(const ExtensionsCallback* extensionsCallback) : _extensionsCallback(extensionsCallback) {} @@ -179,26 +197,23 @@ private: /** * Parse 'obj' and return either a MatchExpression or an error. - * - * 'topLevel' indicates whether or not the we are at the top level of the tree across recursive - * class to this function. This is used to apply special logic at the top level. */ StatusWithMatchExpression _parse(const BSONObj& obj, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel); + DocumentParseLevel currentLevel); /** * parses a field in a sub expression * if the query is { x : { $gt : 5, $lt : 8 } } - * e is { $gt : 5, $lt : 8 } + * obj is { $gt : 5, $lt : 8 } */ Status _parseSub(const char* name, const BSONObj& obj, AndMatchExpression* root, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel); + DocumentParseLevel currentLevel); /** * parses a single field in a sub expression @@ -211,7 +226,7 @@ private: const BSONElement& e, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel); + DocumentParseLevel currentLevel); StatusWithMatchExpression _parseComparison( const char* name, @@ -247,14 +262,12 @@ private: StatusWithMatchExpression _parseElemMatch(const char* name, const BSONElement& e, const boost::intrusive_ptr<ExpressionContext>& expCtx, - AllowedFeatureSet allowedFeatures, - bool topLevel); + AllowedFeatureSet allowedFeatures); StatusWithMatchExpression _parseAll(const char* name, const BSONElement& e, const boost::intrusive_ptr<ExpressionContext>& expCtx, - AllowedFeatureSet allowedFeatures, - bool topLevel); + AllowedFeatureSet allowedFeatures); // tree @@ -262,13 +275,13 @@ private: ListOfMatchExpression* out, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel); + DocumentParseLevel currentLevel); StatusWithMatchExpression _parseNot(const char* name, const BSONElement& e, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel); + DocumentParseLevel currentLevel); /** * Parses 'e' into a BitTestMatchExpression. @@ -284,6 +297,29 @@ private: StatusWithMatchExpression _parseInternalSchemaFmod(const char* name, const BSONElement& e); /** + * Looks at the field named 'exprWithPlaceholderFieldName' within 'containingObject' and parses + * an ExpressionWithPlaceholder from that element. Fails if an error occurs during parsing, or + * if the ExpressionWithPlaceholder has a different name placeholder than 'expectedPlaceholder'. + * 'expressionName' is the name of the expression that requires the ExpressionWithPlaceholder + * and is used to generate helpful error messages. + */ + StatusWith<std::unique_ptr<ExpressionWithPlaceholder>> _parseExprWithPlaceholder( + const BSONObj& containingObject, + StringData exprWithPlaceholderFieldName, + StringData expressionName, + StringData expectedPlaceholder, + const boost::intrusive_ptr<ExpressionContext>& expCtx, + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel); + + StatusWith<std::vector<InternalSchemaAllowedPropertiesMatchExpression::PatternSchema>> + _parsePatternProperties(BSONElement patternPropertiesElem, + StringData expectedPlaceholder, + const boost::intrusive_ptr<ExpressionContext>& expCtx, + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel); + + /** * Parses a MatchExpression which takes a fixed-size array of MatchExpressions as arguments. */ template <class T> @@ -291,7 +327,8 @@ private: StringData name, const BSONElement& elem, const boost::intrusive_ptr<ExpressionContext>& expCtx, - AllowedFeatureSet allowedFeatures); + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel); /** * Parses the given BSONElement into a single integer argument and creates a MatchExpression @@ -315,10 +352,15 @@ private: StatusWithMatchExpression _parseInternalSchemaMatchArrayIndex( const char* path, const BSONElement& elem, - const boost::intrusive_ptr<ExpressionContext>& expCtx); + const boost::intrusive_ptr<ExpressionContext>& expCtx, + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel); StatusWithMatchExpression _parseInternalSchemaAllowedProperties( - const BSONElement& elem, const boost::intrusive_ptr<ExpressionContext>& expCtx); + const BSONElement& elem, + const boost::intrusive_ptr<ExpressionContext>& expCtx, + AllowedFeatureSet allowedFeatures, + DocumentParseLevel currentLevel); // Performs parsing for the match extensions. We do not own this pointer - it has to live // as long as the parser is active. diff --git a/src/mongo/db/matcher/expression_parser_test.cpp b/src/mongo/db/matcher/expression_parser_test.cpp index 3c8796833b6..e25a322248f 100644 --- a/src/mongo/db/matcher/expression_parser_test.cpp +++ b/src/mongo/db/matcher/expression_parser_test.cpp @@ -385,9 +385,49 @@ TEST(MatchExpressionParserTest, ExprParsesSuccessfullyWithAdditionalTopLevelPred ASSERT_OK(MatchExpressionParser::parse(query, expCtx).getStatus()); } -TEST(MatchExpressionParserTest, ExprFailsToParseWithinTopLevelOr) { +TEST(MatchExpressionParserTest, ExprParsesSuccessfullyWithinTopLevelOr) { auto query = fromjson("{$or: [{x: 1}, {$expr: {$eq: ['$a', 5]}}]}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - ASSERT_NOT_OK(MatchExpressionParser::parse(query, expCtx).getStatus()); + ASSERT_OK( + MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kExpr) + .getStatus()); +} + +TEST(MatchExpressionParserTest, ExprParsesSuccessfullyWithinTopLevelAnd) { + auto query = fromjson("{$and: [{x: 1}, {$expr: {$eq: ['$a', 5]}}]}"); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + ASSERT_OK( + MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kExpr) + .getStatus()); +} + +TEST(MatchExpressionParserTest, ExprFailsToParseWithinElemMatch) { + auto query = fromjson("{a: {$elemMatch: {$expr: {$eq: ['$foo', '$bar']}}}}"); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + ASSERT_NOT_OK( + MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kExpr) + .getStatus()); +} + +TEST(MatchExpressionParserTest, ExprNestedFailsToParseWithinElemMatch) { + auto query = + fromjson("{a: {$elemMatch: {b: 1, $or: [{$expr: {$eq: ['$foo', '$bar']}}, {c: 1}]}}}"); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + ASSERT_NOT_OK( + MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kExpr) + .getStatus()); +} + +TEST(MatchExpressionParserTest, ExprFailsToParseWithinInternalSchemaObjectMatch) { + auto query = fromjson("{a: {$_internalSchemaObjectMatch: {$expr: {$eq: ['$foo', '$bar']}}}}"); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + ASSERT_NOT_OK( + MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kExpr) + .getStatus()); } } diff --git a/src/mongo/db/matcher/expression_parser_tree.cpp b/src/mongo/db/matcher/expression_parser_tree.cpp index 3491d0ff734..04680e16532 100644 --- a/src/mongo/db/matcher/expression_parser_tree.cpp +++ b/src/mongo/db/matcher/expression_parser_tree.cpp @@ -43,7 +43,7 @@ Status MatchExpressionParser::_parseTreeList(const BSONObj& arr, ListOfMatchExpression* out, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel) { + DocumentParseLevel currentLevel) { if (arr.isEmpty()) return Status(ErrorCodes::BadValue, "$and/$or/$nor must be a nonempty array"); @@ -54,7 +54,7 @@ Status MatchExpressionParser::_parseTreeList(const BSONObj& arr, if (e.type() != Object) return Status(ErrorCodes::BadValue, "$or/$and/$nor entries need to be full objects"); - StatusWithMatchExpression sub = _parse(e.Obj(), expCtx, allowedFeatures, topLevel); + StatusWithMatchExpression sub = _parse(e.Obj(), expCtx, allowedFeatures, currentLevel); if (!sub.isOK()) return sub.getStatus(); @@ -68,7 +68,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseNot( const BSONElement& e, const boost::intrusive_ptr<ExpressionContext>& expCtx, AllowedFeatureSet allowedFeatures, - bool topLevel) { + DocumentParseLevel currentLevel) { if (e.type() == RegEx) { StatusWithMatchExpression s = _parseRegexElement(name, e); if (!s.isOK()) @@ -88,7 +88,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseNot( return StatusWithMatchExpression(ErrorCodes::BadValue, "$not cannot be empty"); std::unique_ptr<AndMatchExpression> theAnd = stdx::make_unique<AndMatchExpression>(); - Status s = _parseSub(name, notObject, theAnd.get(), expCtx, allowedFeatures, topLevel); + Status s = _parseSub(name, notObject, theAnd.get(), expCtx, allowedFeatures, currentLevel); if (!s.isOK()) return StatusWithMatchExpression(s); diff --git a/src/mongo/db/matcher/expression_with_placeholder.cpp b/src/mongo/db/matcher/expression_with_placeholder.cpp index bb878e05cfa..86287c18bf8 100644 --- a/src/mongo/db/matcher/expression_with_placeholder.cpp +++ b/src/mongo/db/matcher/expression_with_placeholder.cpp @@ -30,6 +30,8 @@ #include "mongo/db/matcher/expression_with_placeholder.h" +#include "mongo/db/matcher/expression_parser.h" + #include <regex> namespace mongo { @@ -88,16 +90,8 @@ bool ExpressionWithPlaceholder::equivalent(const ExpressionWithPlaceholder* othe const std::regex ExpressionWithPlaceholder::placeholderRegex("^[a-z][a-zA-Z0-9]*$"); // static -StatusWith<std::unique_ptr<ExpressionWithPlaceholder>> ExpressionWithPlaceholder::parse( - BSONObj rawFilter, const boost::intrusive_ptr<ExpressionContext>& expCtx) { - StatusWithMatchExpression statusWithFilter = MatchExpressionParser::parse( - rawFilter, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::kBanAllSpecialFeatures); - - if (!statusWithFilter.isOK()) { - return statusWithFilter.getStatus(); - } - auto filter = std::move(statusWithFilter.getValue()); - +StatusWith<std::unique_ptr<ExpressionWithPlaceholder>> ExpressionWithPlaceholder::make( + std::unique_ptr<MatchExpression> filter) { auto statusWithId = parseTopLevelFieldName(filter.get()); if (!statusWithId.isOK()) { return statusWithId.getStatus(); diff --git a/src/mongo/db/matcher/expression_with_placeholder.h b/src/mongo/db/matcher/expression_with_placeholder.h index a60b55c0bc2..885b7d9145e 100644 --- a/src/mongo/db/matcher/expression_with_placeholder.h +++ b/src/mongo/db/matcher/expression_with_placeholder.h @@ -31,7 +31,9 @@ #include <boost/optional.hpp> #include <regex> -#include "mongo/db/matcher/expression_parser.h" +#include "mongo/base/status_with.h" +#include "mongo/db/matcher/expression.h" +#include "mongo/db/pipeline/expression_context.h" namespace mongo { @@ -45,13 +47,11 @@ public: static const std::regex placeholderRegex; /** - * Parses 'rawFilter' to an ExpressionWithPlaceholder. This succeeds if 'rawFilter' is a - * filter over a single top-level field, which begins with a lowercase letter and contains - * no special characters. Otherwise, a non-OK status is returned. Callers must maintain - * ownership of 'rawFilter'. + * Constructs an ExpressionWithPlaceholder from an existing match expression. Returns a non-OK + * status if the paths inside the match expression do not name a consistent placeholder string. */ - static StatusWith<std::unique_ptr<ExpressionWithPlaceholder>> parse( - BSONObj rawFilter, const boost::intrusive_ptr<ExpressionContext>& expCtx); + static StatusWith<std::unique_ptr<ExpressionWithPlaceholder>> make( + std::unique_ptr<MatchExpression> filter); /** * Construct a new ExpressionWithPlaceholder. 'filter' must point to a valid MatchExpression. diff --git a/src/mongo/db/matcher/expression_with_placeholder_test.cpp b/src/mongo/db/matcher/expression_with_placeholder_test.cpp index 27a2a1d549c..5644f3e7ac6 100644 --- a/src/mongo/db/matcher/expression_with_placeholder_test.cpp +++ b/src/mongo/db/matcher/expression_with_placeholder_test.cpp @@ -30,6 +30,7 @@ #include "mongo/db/json.h" #include "mongo/db/matcher/expression_always_boolean.h" +#include "mongo/db/matcher/expression_parser.h" #include "mongo/db/matcher/expression_with_placeholder.h" #include "mongo/db/pipeline/expression_context_for_test.h" #include "mongo/db/query/collation/collator_interface_mock.h" @@ -44,7 +45,8 @@ using unittest::assertGet; TEST(ExpressionWithPlaceholderTest, ParseBasic) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{i: 0}"); - auto filter = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto filter = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT(filter->getPlaceholder()); ASSERT_EQ(*filter->getPlaceholder(), "i"); ASSERT_TRUE(filter->getFilter()->matchesBSON(fromjson("{i: 0}"))); @@ -54,7 +56,8 @@ TEST(ExpressionWithPlaceholderTest, ParseBasic) { TEST(ExpressionWithPlaceholderTest, ParseDottedField) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{'i.a': 0, 'i.b': 1}"); - auto filter = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto filter = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT(filter->getPlaceholder()); ASSERT_EQ(*filter->getPlaceholder(), "i"); ASSERT_TRUE(filter->getFilter()->matchesBSON(fromjson("{i: {a: 0, b: 1}}"))); @@ -64,7 +67,8 @@ TEST(ExpressionWithPlaceholderTest, ParseDottedField) { TEST(ExpressionWithPlaceholderTest, ParseLogicalQuery) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{$and: [{i: {$gte: 0}}, {i: {$lte: 0}}]}"); - auto filter = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto filter = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT(filter->getPlaceholder()); ASSERT_EQ(*filter->getPlaceholder(), "i"); ASSERT_TRUE(filter->getFilter()->matchesBSON(fromjson("{i: 0}"))); @@ -74,7 +78,8 @@ TEST(ExpressionWithPlaceholderTest, ParseLogicalQuery) { TEST(ExpressionWithPlaceholderTest, ParseElemMatch) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{i: {$elemMatch: {a: 0}}}"); - auto filter = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto filter = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT(filter->getPlaceholder()); ASSERT_EQ(*filter->getPlaceholder(), "i"); ASSERT_TRUE(filter->getFilter()->matchesBSON(fromjson("{i: [{a: 0}]}"))); @@ -86,7 +91,8 @@ TEST(ExpressionWithPlaceholderTest, ParseCollation) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); expCtx->setCollator(&collator); auto rawFilter = fromjson("{i: 'abc'}"); - auto filter = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto filter = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT(filter->getPlaceholder()); ASSERT_EQ(*filter->getPlaceholder(), "i"); ASSERT_TRUE(filter->getFilter()->matchesBSON(fromjson("{i: 'cba'}"))); @@ -96,31 +102,27 @@ TEST(ExpressionWithPlaceholderTest, ParseCollation) { TEST(ExpressionWithPlaceholderTest, ParseIdContainsNumbersAndCapitals) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{iA3: 0}"); - auto filter = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto filter = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT(filter->getPlaceholder()); ASSERT_EQ(*filter->getPlaceholder(), "iA3"); ASSERT_TRUE(filter->getFilter()->matchesBSON(fromjson("{'iA3': 0}"))); ASSERT_FALSE(filter->getFilter()->matchesBSON(fromjson("{'iA3': 1}"))); } -TEST(ExpressionWithPlaceholderTest, BadMatchExpressionFailsToParse) { - boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - auto rawFilter = fromjson("{$and: 0}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); - ASSERT_NOT_OK(status.getStatus()); -} - TEST(ExpressionWithPlaceholderTest, EmptyMatchExpressionParsesSuccessfully) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{}"); - auto result = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto result = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT_FALSE(result->getPlaceholder()); } TEST(ExpressionWithPlaceholderTest, NestedEmptyMatchExpressionParsesSuccessfully) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{$or: [{$and: [{}]}]}"); - auto result = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto result = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT_FALSE(result->getPlaceholder()); } @@ -128,7 +130,8 @@ TEST(ExpressionWithPlaceholderTest, NestedMatchExpressionParsesSuccessfullyWhenSomeClausesHaveNoFieldName) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{$or: [{$and: [{}]}, {i: 0}, {i: 1}, {$and: [{}]}]}"); - auto result = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto result = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT(result->getPlaceholder()); ASSERT_EQ(*result->getPlaceholder(), "i"_sd); } @@ -137,133 +140,115 @@ TEST(ExpressionWithPlaceholderTest, SuccessfullyParsesExpressionsWithTypeOther) boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{a: {$_internalSchemaObjectMatch: {$_internalSchemaMinProperties: 5}}}"); - auto result = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto result = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT(result->getPlaceholder()); ASSERT_EQ(*result->getPlaceholder(), "a"_sd); rawFilter = fromjson("{a: {$_internalSchemaType: 'string'}}"); - result = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + result = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT(result->getPlaceholder()); ASSERT_EQ(*result->getPlaceholder(), "a"_sd); rawFilter = fromjson("{$_internalSchemaMinProperties: 1}}"); - result = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + result = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT_FALSE(result->getPlaceholder()); rawFilter = fromjson("{$_internalSchemaCond: [{a: {$exists: true}}, {b: 1}, {c: 1}]}"); - result = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + result = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT_FALSE(result->getPlaceholder()); } TEST(ExpressionWithPlaceholderTest, SuccessfullyParsesAlwaysTrue) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = BSON(AlwaysTrueMatchExpression::kName << 1); - auto result = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = + assertGet(MatchExpressionParser::parse(rawFilter, + expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::kBanAllSpecialFeatures)); + auto result = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT_FALSE(result->getPlaceholder()); } TEST(ExpressionWithPlaceholderTest, SuccessfullyParsesAlwaysFalse) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = BSON(AlwaysFalseMatchExpression::kName << 1); - auto result = assertGet(ExpressionWithPlaceholder::parse(rawFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto result = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); ASSERT_FALSE(result->getPlaceholder()); } TEST(ExpressionWithPlaceholderTest, EmptyFieldNameFailsToParse) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{'': 0}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto status = ExpressionWithPlaceholder::make(std::move(parsedFilter)); ASSERT_NOT_OK(status.getStatus()); } TEST(ExpressionWithPlaceholderTest, EmptyElemMatchFieldNameFailsToParse) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{'': {$elemMatch: {a: 0}}}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto status = ExpressionWithPlaceholder::make(std::move(parsedFilter)); ASSERT_NOT_OK(status.getStatus()); } TEST(ExpressionWithPlaceholderTest, EmptyTopLevelFieldNameFailsToParse) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{'.i': 0}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto status = ExpressionWithPlaceholder::make(std::move(parsedFilter)); ASSERT_NOT_OK(status.getStatus()); } TEST(ExpressionWithPlaceholderTest, MultipleTopLevelFieldsFailsToParse) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{$and: [{i: 0}, {j: 0}]}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto status = ExpressionWithPlaceholder::make(std::move(parsedFilter)); ASSERT_NOT_OK(status.getStatus()); } TEST(ExpressionWithPlaceholderTest, SpecialCharactersInFieldNameFailsToParse) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{'i&': 0}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto status = ExpressionWithPlaceholder::make(std::move(parsedFilter)); ASSERT_NOT_OK(status.getStatus()); } TEST(ExpressionWithPlaceholderTest, FieldNameStartingWithNumberFailsToParse) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{'3i': 0}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto status = ExpressionWithPlaceholder::make(std::move(parsedFilter)); ASSERT_NOT_OK(status.getStatus()); } TEST(ExpressionWithPlaceholderTest, FieldNameStartingWithCapitalFailsToParse) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter = fromjson("{'Ai': 0}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); - ASSERT_NOT_OK(status.getStatus()); -} - -TEST(ExpressionWithPlaceholderTest, TextSearchExpressionFailsToParse) { - boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - auto rawFilter = fromjson("{$text: {$search: 'search terms'}}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); + auto parsedFilter = assertGet(MatchExpressionParser::parse(rawFilter, expCtx)); + auto status = ExpressionWithPlaceholder::make(std::move(parsedFilter)); ASSERT_NOT_OK(status.getStatus()); -} - -TEST(ExpressionWithPlaceholderTest, WhereExpressionFailsToParse) { - boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - auto rawFilter = fromjson("{$where: 'sleep(100)'}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); - ASSERT_NOT_OK(status.getStatus()); -} - -TEST(ExpressionWithPlaceholderTest, GeoNearExpressionFailsToParse) { - boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - auto rawFilter = - fromjson("{i: {$nearSphere: {$geometry: {type: 'Point', coordinates: [0, 0]}}}}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); - ASSERT_NOT_OK(status.getStatus()); -} - -TEST(ExpressionWithPlaceholderTest, ExprExpressionFailsToParse) { - boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - auto rawFilter = fromjson("{$expr: {$eq: ['$i', 5]}}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); - ASSERT_NOT_OK(status.getStatus()); - ASSERT_EQ(status.getStatus().code(), ErrorCodes::QueryFeatureNotAllowed); -} - -TEST(ExpressionWithPlaceholderTest, JSONSchemaExpressionFailsToParse) { - boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - auto rawFilter = fromjson("{$jsonSchema: {}}"); - auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); - ASSERT_NOT_OK(status.getStatus()); - ASSERT_EQ(status.getStatus().code(), ErrorCodes::QueryFeatureNotAllowed); + ASSERT_EQ(status.getStatus().code(), ErrorCodes::BadValue); } TEST(ExpressionWithPlaceholderTest, EquivalentIfPlaceholderAndExpressionMatch) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter1 = fromjson("{i: 5}}"); - auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::parse(rawFilter1, expCtx); + auto parsedFilter1 = assertGet(MatchExpressionParser::parse(rawFilter1, expCtx)); + auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::make(std::move(parsedFilter1)); ASSERT_OK(expressionWithPlaceholder1.getStatus()); auto rawFilter2 = fromjson("{i: 5}"); - auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::parse(rawFilter2, expCtx); + auto parsedFilter2 = assertGet(MatchExpressionParser::parse(rawFilter2, expCtx)); + auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::make(std::move(parsedFilter2)); ASSERT_OK(expressionWithPlaceholder2.getStatus()); ASSERT_TRUE(expressionWithPlaceholder1.getValue()->equivalent( expressionWithPlaceholder2.getValue().get())); @@ -272,26 +257,30 @@ TEST(ExpressionWithPlaceholderTest, EquivalentIfPlaceholderAndExpressionMatch) { TEST(ExpressionWithPlaceholderTest, EmptyMatchExpressionsAreEquivalent) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter1 = fromjson("{}"); - auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::parse(rawFilter1, expCtx); + auto parsedFilter1 = assertGet(MatchExpressionParser::parse(rawFilter1, expCtx)); + auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::make(std::move(parsedFilter1)); ASSERT_OK(expressionWithPlaceholder1.getStatus()); auto rawFilter2 = fromjson("{}"); - auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::parse(rawFilter2, expCtx); + auto parsedFilter2 = assertGet(MatchExpressionParser::parse(rawFilter2, expCtx)); + auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::make(std::move(parsedFilter2)); ASSERT_OK(expressionWithPlaceholder2.getStatus()); - ASSERT(expressionWithPlaceholder1.getValue()->equivalent( + ASSERT_TRUE(expressionWithPlaceholder1.getValue()->equivalent( expressionWithPlaceholder2.getValue().get())); } TEST(ExpressionWithPlaceholderTest, NestedEmptyMatchExpressionsAreEquivalent) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter1 = fromjson("{$or: [{$and: [{}]}]}"); - auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::parse(rawFilter1, expCtx); + auto parsedFilter1 = assertGet(MatchExpressionParser::parse(rawFilter1, expCtx)); + auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::make(std::move(parsedFilter1)); ASSERT_OK(expressionWithPlaceholder1.getStatus()); auto rawFilter2 = fromjson("{$or: [{$and: [{}]}]}"); - auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::parse(rawFilter2, expCtx); + auto parsedFilter2 = assertGet(MatchExpressionParser::parse(rawFilter2, expCtx)); + auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::make(std::move(parsedFilter2)); ASSERT_OK(expressionWithPlaceholder2.getStatus()); - ASSERT(expressionWithPlaceholder1.getValue()->equivalent( + ASSERT_TRUE(expressionWithPlaceholder1.getValue()->equivalent( expressionWithPlaceholder2.getValue().get())); } @@ -299,38 +288,44 @@ TEST(ExpressionWithPlaceholderTest, SameObjectMatchesAreEquivalent) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter1 = fromjson("{a: {$_internalSchemaObjectMatch: {$_internalSchemaMaxProperties: 2}}}"); - auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::parse(rawFilter1, expCtx); + auto parsedFilter1 = assertGet(MatchExpressionParser::parse(rawFilter1, expCtx)); + auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::make(std::move(parsedFilter1)); ASSERT_OK(expressionWithPlaceholder1.getStatus()); auto rawFilter2 = fromjson("{a: {$_internalSchemaObjectMatch: {$_internalSchemaMaxProperties: 2}}}"); - auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::parse(rawFilter2, expCtx); + auto parsedFilter2 = assertGet(MatchExpressionParser::parse(rawFilter2, expCtx)); + auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::make(std::move(parsedFilter2)); ASSERT_OK(expressionWithPlaceholder2.getStatus()); - ASSERT(expressionWithPlaceholder1.getValue()->equivalent( + ASSERT_TRUE(expressionWithPlaceholder1.getValue()->equivalent( expressionWithPlaceholder2.getValue().get())); } TEST(ExpressionWithPlaceholderTest, AlwaysTruesAreEquivalent) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter1 = BSON(AlwaysTrueMatchExpression::kName << 1); - auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::parse(rawFilter1, expCtx); + auto parsedFilter1 = assertGet(MatchExpressionParser::parse(rawFilter1, expCtx)); + auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::make(std::move(parsedFilter1)); ASSERT_OK(expressionWithPlaceholder1.getStatus()); auto rawFilter2 = BSON(AlwaysTrueMatchExpression::kName << 1); - auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::parse(rawFilter2, expCtx); + auto parsedFilter2 = assertGet(MatchExpressionParser::parse(rawFilter2, expCtx)); + auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::make(std::move(parsedFilter2)); ASSERT_OK(expressionWithPlaceholder2.getStatus()); - ASSERT(expressionWithPlaceholder1.getValue()->equivalent( + ASSERT_TRUE(expressionWithPlaceholder1.getValue()->equivalent( expressionWithPlaceholder2.getValue().get())); } TEST(ExpressionWithPlaceholderTest, NotEquivalentIfPlaceholderDoesNotMatch) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter1 = fromjson("{i: {$type: 'array'}}"); - auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::parse(rawFilter1, expCtx); + auto parsedFilter1 = assertGet(MatchExpressionParser::parse(rawFilter1, expCtx)); + auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::make(std::move(parsedFilter1)); ASSERT_OK(expressionWithPlaceholder1.getStatus()); auto rawFilter2 = fromjson("{j: {$type: 'array'}}"); - auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::parse(rawFilter2, expCtx); + auto parsedFilter2 = assertGet(MatchExpressionParser::parse(rawFilter2, expCtx)); + auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::make(std::move(parsedFilter2)); ASSERT_OK(expressionWithPlaceholder2.getStatus()); ASSERT_FALSE(expressionWithPlaceholder1.getValue()->equivalent( expressionWithPlaceholder2.getValue().get())); @@ -339,11 +334,13 @@ TEST(ExpressionWithPlaceholderTest, NotEquivalentIfPlaceholderDoesNotMatch) { TEST(ExpressionWithPlaceholder, NotEquivalentIfOnePlaceholderIsEmpty) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter1 = fromjson("{}"); - auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::parse(rawFilter1, expCtx); + auto parsedFilter1 = assertGet(MatchExpressionParser::parse(rawFilter1, expCtx)); + auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::make(std::move(parsedFilter1)); ASSERT_OK(expressionWithPlaceholder1.getStatus()); auto rawFilter2 = fromjson("{i: 5}"); - auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::parse(rawFilter2, expCtx); + auto parsedFilter2 = assertGet(MatchExpressionParser::parse(rawFilter2, expCtx)); + auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::make(std::move(parsedFilter2)); ASSERT_OK(expressionWithPlaceholder2.getStatus()); ASSERT_FALSE(expressionWithPlaceholder1.getValue()->equivalent( expressionWithPlaceholder2.getValue().get())); @@ -352,11 +349,13 @@ TEST(ExpressionWithPlaceholder, NotEquivalentIfOnePlaceholderIsEmpty) { TEST(ExpressionWithPlaceholderTest, NotEquivalentIfExpressionDoesNotMatch) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rawFilter1 = fromjson("{i: {$lte: 5}}"); - auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::parse(rawFilter1, expCtx); + auto parsedFilter1 = assertGet(MatchExpressionParser::parse(rawFilter1, expCtx)); + auto expressionWithPlaceholder1 = ExpressionWithPlaceholder::make(std::move(parsedFilter1)); ASSERT_OK(expressionWithPlaceholder1.getStatus()); auto rawFilter2 = fromjson("{i: {$gte: 5}}"); - auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::parse(rawFilter2, expCtx); + auto parsedFilter2 = assertGet(MatchExpressionParser::parse(rawFilter2, expCtx)); + auto expressionWithPlaceholder2 = ExpressionWithPlaceholder::make(std::move(parsedFilter2)); ASSERT_OK(expressionWithPlaceholder2.getStatus()); ASSERT_FALSE(expressionWithPlaceholder1.getValue()->equivalent( expressionWithPlaceholder2.getValue().get())); diff --git a/src/mongo/db/matcher/schema/expression_parser_schema_test.cpp b/src/mongo/db/matcher/schema/expression_parser_schema_test.cpp index ff3aa8beac4..c05af09c4f3 100644 --- a/src/mongo/db/matcher/schema/expression_parser_schema_test.cpp +++ b/src/mongo/db/matcher/schema/expression_parser_schema_test.cpp @@ -238,7 +238,7 @@ TEST(MatchExpressionParserSchemaTest, ObjectMatchSubExprRejectsTopLevelOperators "}}}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto result = MatchExpressionParser::parse(query, expCtx); - ASSERT_EQ(result.getStatus(), ErrorCodes::BadValue); + ASSERT_EQ(result.getStatus(), ErrorCodes::FailedToParse); } // @@ -853,7 +853,7 @@ TEST(MatchExpressionParserSchemaTest, RootDocEqMustBeTopLevel) { auto query = fromjson("{a: {$_internalSchemaRootDocEq: 1}}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto rootDocEq = MatchExpressionParser::parse(query, expCtx); - ASSERT_EQ(rootDocEq.getStatus(), ErrorCodes::BadValue); + ASSERT_EQ(rootDocEq.getStatus(), ErrorCodes::FailedToParse); query = fromjson("{$or: [{a: 1}, {$_internalSchemaRootDocEq: 1}]}"); rootDocEq = MatchExpressionParser::parse(query, expCtx); @@ -861,7 +861,7 @@ TEST(MatchExpressionParserSchemaTest, RootDocEqMustBeTopLevel) { query = fromjson("{a: {$elemMatch: {$_internalSchemaRootDocEq: 1}}}"); rootDocEq = MatchExpressionParser::parse(query, expCtx); - ASSERT_EQ(rootDocEq.getStatus(), ErrorCodes::BadValue); + ASSERT_EQ(rootDocEq.getStatus(), ErrorCodes::FailedToParse); } TEST(MatchExpressionParserSchemaTest, RootDocEqParsesSuccessfully) { diff --git a/src/mongo/db/ops/parsed_update.cpp b/src/mongo/db/ops/parsed_update.cpp index 7944a16a545..aecf7c30794 100644 --- a/src/mongo/db/ops/parsed_update.cpp +++ b/src/mongo/db/ops/parsed_update.cpp @@ -171,15 +171,25 @@ Status ParsedUpdate::parseArrayFilters() { for (auto rawArrayFilter : _request->getArrayFilters()) { boost::intrusive_ptr<ExpressionContext> expCtx( new ExpressionContext(_opCtx, _collator.get())); - auto arrayFilterStatus = - ExpressionWithPlaceholder::parse(rawArrayFilter, std::move(expCtx)); - if (!arrayFilterStatus.isOK()) { - return Status(arrayFilterStatus.getStatus().code(), + auto parsedArrayFilter = + MatchExpressionParser::parse(rawArrayFilter, + std::move(expCtx), + ExtensionsCallbackNoop(), + MatchExpressionParser::kBanAllSpecialFeatures); + if (!parsedArrayFilter.isOK()) { + return Status(parsedArrayFilter.getStatus().code(), str::stream() << "Error parsing array filter: " - << arrayFilterStatus.getStatus().reason()); + << parsedArrayFilter.getStatus().reason()); } - auto arrayFilter = std::move(arrayFilterStatus.getValue()); - auto fieldName = arrayFilter->getPlaceholder(); + auto parsedArrayFilterWithPlaceholder = + ExpressionWithPlaceholder::make(std::move(parsedArrayFilter.getValue())); + if (!parsedArrayFilterWithPlaceholder.isOK()) { + return Status(parsedArrayFilterWithPlaceholder.getStatus().code(), + str::stream() << "Error parsing array filter: " + << parsedArrayFilterWithPlaceholder.getStatus().reason()); + } + auto finalArrayFilter = std::move(parsedArrayFilterWithPlaceholder.getValue()); + auto fieldName = finalArrayFilter->getPlaceholder(); if (!fieldName) { return Status( ErrorCodes::FailedToParse, @@ -192,7 +202,7 @@ Status ParsedUpdate::parseArrayFilters() { << *fieldName); } - _arrayFilters[*fieldName] = std::move(arrayFilter); + _arrayFilters[*fieldName] = std::move(finalArrayFilter); } return Status::OK(); diff --git a/src/mongo/db/update/update_array_node_test.cpp b/src/mongo/db/update/update_array_node_test.cpp index 38004cd39f9..59204998b81 100644 --- a/src/mongo/db/update/update_array_node_test.cpp +++ b/src/mongo/db/update/update_array_node_test.cpp @@ -33,6 +33,7 @@ #include "mongo/bson/mutable/algorithm.h" #include "mongo/bson/mutable/mutable_bson_test_utils.h" #include "mongo/db/json.h" +#include "mongo/db/matcher/expression_parser.h" #include "mongo/db/pipeline/expression_context_for_test.h" #include "mongo/db/update/update_node_test_fixture.h" #include "mongo/db/update/update_object_node.h" @@ -44,13 +45,15 @@ namespace { using UpdateArrayNodeTest = UpdateNodeTest; using mongo::mutablebson::Element; +using unittest::assertGet; TEST_F(UpdateArrayNodeTest, ApplyCreatePathFails) { auto update = fromjson("{$set: {'a.b.$[i]': 0}}"); auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -74,7 +77,8 @@ TEST_F(UpdateArrayNodeTest, ApplyToNonArrayFails) { auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -97,7 +101,8 @@ TEST_F(UpdateArrayNodeTest, UpdateIsAppliedToAllMatchingElements) { auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -124,7 +129,8 @@ DEATH_TEST_F(UpdateArrayNodeTest, auto arrayFilter = fromjson("{'i.c': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -171,9 +177,15 @@ TEST_F(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElement) { auto arrayFilterK = fromjson("{'k.d': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); - arrayFilters["k"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterK, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx)); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx)); + auto parsedFilterK = assertGet(MatchExpressionParser::parse(arrayFilterK, expCtx)); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + arrayFilters["k"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterK))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -211,8 +223,13 @@ TEST_F(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElementsUsingMergedChildr auto arrayFilterJ = fromjson("{'j.c': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx)); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx)); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -245,9 +262,15 @@ TEST_F(UpdateArrayNodeTest, ApplyMultipleUpdatesToArrayElementsWithoutMergedChil auto arrayFilterK = fromjson("{'k.d': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); - arrayFilters["k"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterK, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx)); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx)); + auto parsedFilterK = assertGet(MatchExpressionParser::parse(arrayFilterK, expCtx)); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + arrayFilters["k"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterK))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -316,10 +339,25 @@ TEST_F(UpdateArrayNodeTest, ApplyNestedArrayUpdates) { auto arrayFilterL = fromjson("{'l.d': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); - arrayFilters["k"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterK, expCtx)); - arrayFilters["l"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterL, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx + + )); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx + + )); + auto parsedFilterK = assertGet(MatchExpressionParser::parse(arrayFilterK, expCtx + + )); + auto parsedFilterL = assertGet(MatchExpressionParser::parse(arrayFilterL, expCtx + + )); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + arrayFilters["k"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterK))); + arrayFilters["l"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterL))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -351,8 +389,17 @@ TEST_F(UpdateArrayNodeTest, ApplyUpdatesWithMergeConflictToArrayElementFails) { auto arrayFilterJ = fromjson("{'j': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx + + )); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx + + )); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -382,8 +429,17 @@ TEST_F(UpdateArrayNodeTest, ApplyUpdatesWithEmptyIdentifiersWithMergeConflictToA auto arrayFilterJ = fromjson("{'j': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx + + )); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx + + )); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -415,10 +471,25 @@ TEST_F(UpdateArrayNodeTest, ApplyNestedArrayUpdatesWithMergeConflictFails) { auto arrayFilterL = fromjson("{l: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); - arrayFilters["k"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterK, expCtx)); - arrayFilters["l"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterL, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx + + )); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx + + )); + auto parsedFilterK = assertGet(MatchExpressionParser::parse(arrayFilterK, expCtx + + )); + auto parsedFilterL = assertGet(MatchExpressionParser::parse(arrayFilterL, expCtx + + )); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + arrayFilters["k"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterK))); + arrayFilters["l"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterL))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -447,7 +518,10 @@ TEST_F(UpdateArrayNodeTest, NoArrayElementsMatch) { auto arrayFilter = fromjson("{'i': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx + + )); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -472,7 +546,10 @@ TEST_F(UpdateArrayNodeTest, UpdatesToAllArrayElementsAreNoops) { auto arrayFilter = fromjson("{'i': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx + + )); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -497,7 +574,10 @@ TEST_F(UpdateArrayNodeTest, NoArrayElementAffectsIndexes) { auto arrayFilter = fromjson("{'i.c': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx + + )); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -522,7 +602,10 @@ TEST_F(UpdateArrayNodeTest, WhenOneElementIsMatchedLogElementUpdateDirectly) { auto arrayFilter = fromjson("{'i.c': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx + + )); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -547,7 +630,10 @@ TEST_F(UpdateArrayNodeTest, WhenOneElementIsModifiedLogElement) { auto arrayFilter = fromjson("{'i.c': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx + + )); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -595,7 +681,10 @@ TEST_F(UpdateArrayNodeTest, ApplyPositionalInsideArrayUpdate) { auto arrayFilter = fromjson("{'i.c': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx + + )); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -621,7 +710,10 @@ TEST_F(UpdateArrayNodeTest, ApplyArrayUpdateFromReplication) { auto arrayFilter = fromjson("{'i': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx + + )); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -647,7 +739,10 @@ TEST_F(UpdateArrayNodeTest, ApplyArrayUpdateNotFromReplication) { auto arrayFilter = fromjson("{'i': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx + + )); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -670,7 +765,10 @@ TEST_F(UpdateArrayNodeTest, ApplyArrayUpdateWithoutLogBuilderOrIndexData) { auto arrayFilter = fromjson("{'i': 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx + + )); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, diff --git a/src/mongo/db/update/update_object_node_test.cpp b/src/mongo/db/update/update_object_node_test.cpp index c410553eb9d..9bd9ff81d52 100644 --- a/src/mongo/db/update/update_object_node_test.cpp +++ b/src/mongo/db/update/update_object_node_test.cpp @@ -33,6 +33,7 @@ #include "mongo/bson/mutable/algorithm.h" #include "mongo/bson/mutable/mutable_bson_test_utils.h" #include "mongo/db/json.h" +#include "mongo/db/matcher/expression_parser.h" #include "mongo/db/pipeline/expression_context_for_test.h" #include "mongo/db/query/collation/collator_interface_mock.h" #include "mongo/db/update/conflict_placeholder_node.h" @@ -47,6 +48,7 @@ namespace { using UpdateObjectNodeTest = UpdateNodeTest; using mongo::mutablebson::Element; +using unittest::assertGet; TEST(UpdateObjectNodeTest, InvalidPathFailsToParse) { auto update = fromjson("{$set: {'': 5}}"); @@ -784,7 +786,8 @@ TEST(UpdateObjectNodeTest, IdentifierWithArrayFilterParsesSuccessfully) { auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -802,7 +805,8 @@ TEST(UpdateObjectNodeTest, IdentifierWithArrayFilterInMiddleOfPathParsesSuccessf auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -820,7 +824,8 @@ TEST(UpdateObjectNodeTest, IdentifierInFirstPositionFailsToParse) { auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; auto result = UpdateObjectNode::parseAndMerge(&root, @@ -841,7 +846,8 @@ TEST(UpdateObjectNodeTest, IdentifierInFirstPositionWithSuffixFailsToParse) { auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; auto result = UpdateObjectNode::parseAndMerge(&root, @@ -862,7 +868,8 @@ TEST(UpdateObjectNodeTest, CreateObjectNodeInSamePositionAsArrayNodeFailsToParse auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -890,7 +897,8 @@ TEST(UpdateObjectNodeTest, CreateArrayNodeInSamePositionAsObjectNodeFailsToParse auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -916,7 +924,8 @@ TEST(UpdateObjectNodeTest, CreateLeafNodeInSamePositionAsArrayNodeFailsToParse) auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -943,7 +952,8 @@ TEST(UpdateObjectNodeTest, CreateArrayNodeInSamePositionAsLeafNodeFailsToParse) auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -970,8 +980,13 @@ TEST(UpdateObjectNodeTest, CreateTwoChildrenOfArrayNodeParsesSuccessfully) { auto arrayFilterJ = fromjson("{j: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx)); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx)); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -996,10 +1011,11 @@ TEST(UpdateObjectNodeTest, CreateTwoChildrenOfArrayNodeParsesSuccessfully) { TEST(UpdateObjectNodeTest, ConflictAtArrayNodeChildFailsToParse) { auto update1 = fromjson("{$set: {'a.$[i]': 5}}"); auto update2 = fromjson("{$set: {'a.$[i]': 6}}"); - auto arrayFilterI = fromjson("{i: 0}"); + auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -1024,10 +1040,11 @@ TEST(UpdateObjectNodeTest, ConflictAtArrayNodeChildFailsToParse) { TEST(UpdateObjectNodeTest, ConflictThroughArrayNodeChildFailsToParse) { auto update = fromjson("{$set: {'a.$[i].b': 5, 'a.$[i].b.c': 6}}"); - auto arrayFilterI = fromjson("{i: 0}"); + auto arrayFilter = fromjson("{i: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -1056,8 +1073,12 @@ TEST(UpdateObjectNodeTest, NoConflictDueToDifferentArrayNodeChildrenParsesSucces auto arrayFilterJ = fromjson("{j: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx)); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx)); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -1085,8 +1106,13 @@ TEST(UpdateObjectNodeTest, MultipleArrayNodesAlongPathParsesSuccessfully) { auto arrayFilterJ = fromjson("{j: 0}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx)); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx)); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + std::set<std::string> foundIdentifiers; UpdateObjectNode root; ASSERT_OK(UpdateObjectNode::parseAndMerge(&root, @@ -1516,11 +1542,13 @@ DEATH_TEST(UpdateObjectNodeTest, FieldRef fakeFieldRef("root"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto arrayFilterI = fromjson("{i: 0}"); - std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters1; - arrayFilters1["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); auto arrayFilterJ = fromjson("{j: 0}"); + std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters1; std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters2; - arrayFilters2["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx)); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx)); + arrayFilters1["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters2["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); std::set<std::string> foundIdentifiers; UpdateObjectNode setRoot1, setRoot2; ASSERT_OK(UpdateObjectNode::parseAndMerge(&setRoot1, @@ -1546,7 +1574,8 @@ TEST(UpdateObjectNodeTest, MergingArrayNodeWithObjectNodeFails) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto arrayFilter = fromjson("{i: 0}"); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode setRoot1, setRoot2; ASSERT_OK(UpdateObjectNode::parseAndMerge(&setRoot1, @@ -1577,7 +1606,8 @@ TEST(UpdateObjectNodeTest, MergingArrayNodeWithLeafNodeFails) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto arrayFilter = fromjson("{i: 0}"); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode setRoot1, setRoot2; ASSERT_OK(UpdateObjectNode::parseAndMerge(&setRoot1, @@ -1609,8 +1639,13 @@ TEST(UpdateObjectNodeTest, MergingTwoArrayNodesSucceeds) { auto arrayFilterI = fromjson("{i: 0}"); auto arrayFilterJ = fromjson("{j: 0}"); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterI, expCtx)); - arrayFilters["j"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilterJ, expCtx)); + + auto parsedFilterI = assertGet(MatchExpressionParser::parse(arrayFilterI, expCtx)); + auto parsedFilterJ = assertGet(MatchExpressionParser::parse(arrayFilterJ, expCtx)); + + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterI))); + arrayFilters["j"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilterJ))); + std::set<std::string> foundIdentifiers; UpdateObjectNode setRoot1, setRoot2; ASSERT_OK(UpdateObjectNode::parseAndMerge(&setRoot1, @@ -1648,7 +1683,8 @@ TEST(UpdateObjectNodeTest, MergeConflictThroughArrayNodesFails) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto arrayFilter = fromjson("{i: 0}"); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode setRoot1, setRoot2; ASSERT_OK(UpdateObjectNode::parseAndMerge(&setRoot1, @@ -1679,7 +1715,8 @@ TEST(UpdateObjectNodeTest, NoMergeConflictThroughArrayNodesSucceeds) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto arrayFilter = fromjson("{i: 0}"); std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; - arrayFilters["i"] = uassertStatusOK(ExpressionWithPlaceholder::parse(arrayFilter, expCtx)); + auto parsedFilter = assertGet(MatchExpressionParser::parse(arrayFilter, expCtx)); + arrayFilters["i"] = assertGet(ExpressionWithPlaceholder::make(std::move(parsedFilter))); std::set<std::string> foundIdentifiers; UpdateObjectNode setRoot1, setRoot2; ASSERT_OK(UpdateObjectNode::parseAndMerge(&setRoot1, |