diff options
| -rw-r--r-- | jstests/core/json_schema.js | 23 | ||||
| -rw-r--r-- | src/mongo/db/matcher/schema/json_schema_parser.cpp | 51 | ||||
| -rw-r--r-- | src/mongo/db/matcher/schema/json_schema_parser_test.cpp | 89 |
3 files changed, 110 insertions, 53 deletions
diff --git a/jstests/core/json_schema.js b/jstests/core/json_schema.js index 8d458734d9a..bfebfb7ab90 100644 --- a/jstests/core/json_schema.js +++ b/jstests/core/json_schema.js @@ -12,6 +12,7 @@ assert.writeOK(coll.insert({_id: 5, num: NumberLong(-1)})); assert.writeOK(coll.insert({_id: 6, num: {}})); assert.writeOK(coll.insert({_id: 7, num: "str"})); + assert.writeOK(coll.insert({_id: 8})); // Test that $jsonSchema fails to parse if its argument is not an object. assert.throws(function() { @@ -53,57 +54,57 @@ }); // Test that the empty schema matches everything. - assert.eq(8, coll.find({$jsonSchema: {}}).itcount()); + assert.eq(9, coll.find({$jsonSchema: {}}).itcount()); // Test that a schema just checking that the type of stored documents is "object" is legal and // matches everything. - assert.eq(8, coll.find({$jsonSchema: {type: "object"}}).itcount()); + assert.eq(9, coll.find({$jsonSchema: {type: "object"}}).itcount()); // Test that schemas whose top-level type is not object matches nothing. assert.eq(0, coll.find({$jsonSchema: {type: "string"}}).itcount()); assert.eq(0, coll.find({$jsonSchema: {type: "long"}}).itcount()); assert.eq(0, coll.find({$jsonSchema: {type: "objectId"}}).itcount()); - // Test that type:"number" only matches numbers. - assert.eq([{_id: 0}, {_id: 1}, {_id: 2}, {_id: 3}, {_id: 4}, {_id: 5}], + // Test that type:"number" only matches numbers, or documents where the field is missing. + assert.eq([{_id: 0}, {_id: 1}, {_id: 2}, {_id: 3}, {_id: 4}, {_id: 5}, {_id: 8}], coll.find({$jsonSchema: {properties: {num: {type: "number"}}}}, {_id: 1}) .sort({_id: 1}) .toArray()); // Test that maximum restriction is enforced correctly. - assert.eq([{_id: 1}, {_id: 3}, {_id: 5}], + assert.eq([{_id: 1}, {_id: 3}, {_id: 5}, {_id: 8}], coll.find({$jsonSchema: {properties: {num: {type: "number", maximum: -1}}}}, {_id: 1}) .sort({_id: 1}) .toArray()); // Repeat the test, but include an explicit top-level type:"object". assert.eq( - [{_id: 1}, {_id: 3}, {_id: 5}], + [{_id: 1}, {_id: 3}, {_id: 5}, {_id: 8}], coll.find({$jsonSchema: {type: "object", properties: {num: {type: "number", maximum: -1}}}}, {_id: 1}) .sort({_id: 1}) .toArray()); - // Test that type:"long" only matches longs. - assert.eq([{_id: 4}, {_id: 5}], + // Test that type:"long" only matches longs, or documents where the field is missing. + assert.eq([{_id: 4}, {_id: 5}, {_id: 8}], coll.find({$jsonSchema: {properties: {num: {type: "long"}}}}, {_id: 1}) .sort({_id: 1}) .toArray()); // Test that maximum restriction is enforced correctly with type:"long". - assert.eq([{_id: 5}], + assert.eq([{_id: 5}, {_id: 8}], coll.find({$jsonSchema: {properties: {num: {type: "long", maximum: 0}}}}, {_id: 1}) .sort({_id: 1}) .toArray()); // Test that maximum restriction without a numeric type specified only applies to numbers. - assert.eq([{_id: 1}, {_id: 3}, {_id: 5}, {_id: 6}, {_id: 7}], + assert.eq([{_id: 1}, {_id: 3}, {_id: 5}, {_id: 6}, {_id: 7}, {_id: 8}], coll.find({$jsonSchema: {properties: {num: {maximum: 0}}}}, {_id: 1}) .sort({_id: 1}) .toArray()); // Test that maximum restriction does nothing if a non-numeric type is also specified. - assert.eq([{_id: 7}], + assert.eq([{_id: 7}, {_id: 8}], coll.find({$jsonSchema: {properties: {num: {type: "string", maximum: 0}}}}, {_id: 1}) .sort({_id: 1}) .toArray()); diff --git a/src/mongo/db/matcher/schema/json_schema_parser.cpp b/src/mongo/db/matcher/schema/json_schema_parser.cpp index c5fcb89cc1e..4418eb2f699 100644 --- a/src/mongo/db/matcher/schema/json_schema_parser.cpp +++ b/src/mongo/db/matcher/schema/json_schema_parser.cpp @@ -70,19 +70,24 @@ std::unique_ptr<MatchExpression> makeRestriction(TypeMatchExpression::Type restr (statedType->matchesAllNumbers() || isNumericBSONType(statedType->getBSONType())); const bool bsonTypesMatch = restrictionType.bsonType == statedType->getBSONType(); - if (bothNumeric || bsonTypesMatch) { - // This restriction applies only to the type that is already being enforced. We return - // the restriction unmodified. - return restrictionExpr; - } else { + if (!bothNumeric && !bsonTypesMatch) { // This restriction doesn't take any effect, since the type of the schema is different // from the type to which this retriction applies. return stdx::make_unique<AlwaysTrueMatchExpression>(); } } - invariant(!statedType); - + // Generate and return the following expression tree: + // + // OR + // / / + // NOT <restrictionExpr> + // / + // TYPE + // <restrictionType> + // + // We need to do this because restriction keywords do not apply when a field is either not + // present or of a different type. auto typeExprForNot = stdx::make_unique<TypeMatchExpression>(); invariantOK(typeExprForNot->init(restrictionExpr->path(), restrictionType)); @@ -94,6 +99,36 @@ std::unique_ptr<MatchExpression> makeRestriction(TypeMatchExpression::Type restr return std::move(orExpr); } +/** + * Constructs and returns the following expression tree: + * OR + * / \ + * NOT <typeExpr> + * / + * EXISTS + * <typeExpr field> + * + * This is needed because the JSON Schema 'type' keyword only applies if the corresponding field is + * present. + * + * 'typeExpr' must be non-null and must have a non-empty path. + */ +std::unique_ptr<MatchExpression> makeTypeRestriction( + std::unique_ptr<TypeMatchExpression> typeExpr) { + invariant(typeExpr); + invariant(!typeExpr->path().empty()); + + auto existsExpr = stdx::make_unique<ExistsMatchExpression>(); + invariantOK(existsExpr->init(typeExpr->path())); + + auto notExpr = stdx::make_unique<NotMatchExpression>(existsExpr.release()); + auto orExpr = stdx::make_unique<OrMatchExpression>(); + orExpr->add(notExpr.release()); + orExpr->add(typeExpr.release()); + + return std::move(orExpr); +} + StatusWith<std::unique_ptr<TypeMatchExpression>> parseType(StringData path, BSONElement typeElt) { if (!typeElt) { return {nullptr}; @@ -318,7 +353,7 @@ StatusWithMatchExpression JSONSchemaParser::_parse(StringData path, BSONObj sche } if (!path.empty() && typeExpr.getValue()) { - andExpr->add(typeExpr.getValue().release()); + andExpr->add(makeTypeRestriction(std::move(typeExpr.getValue())).release()); } return {std::move(andExpr)}; } diff --git a/src/mongo/db/matcher/schema/json_schema_parser_test.cpp b/src/mongo/db/matcher/schema/json_schema_parser_test.cpp index a39244e12f7..a0de359d855 100644 --- a/src/mongo/db/matcher/schema/json_schema_parser_test.cpp +++ b/src/mongo/db/matcher/schema/json_schema_parser_test.cpp @@ -102,8 +102,9 @@ TEST(JSONSchemaParserTest, NestedTypeObjectTranslatesCorrectly) { ASSERT_BSONOBJ_EQ( builder.obj(), fromjson("{$and: [{$and: [{$and: [" - "{a: {$_internalSchemaObjectMatch: {$and: [{$and:[{b: {$type:2}}]}]}}}," - "{a: {$type: 3}}]}]}]}")); + "{$or: [{$nor: [{a: {$type: 3}}]}, {a: {$_internalSchemaObjectMatch:" + "{$and: [{$and:[{$or: [{$nor: [{b: {$exists: true}}]}, {b: {$type: 2}}]}]}]}}}]}," + "{$or: [{$nor: [{a: {$exists: true}}]}, {a: {$type: 3}}]}]}]}]}")); } TEST(JSONSchemaParserTest, TopLevelNonObjectTypeTranslatesCorrectly) { @@ -121,8 +122,10 @@ TEST(JSONSchemaParserTest, TypeNumberTranslatesCorrectly) { ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ(builder.obj(), - fromjson("{$and: [{$and: [{$and: [{num: {$type: 'number'}}]}]}]}")); + ASSERT_BSONOBJ_EQ( + builder.obj(), + fromjson("{$and: [{$and: [{$and: [" + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 'number'}}]}]}]}]}")); } TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithTypeNumber) { @@ -131,9 +134,11 @@ TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithTypeNumber) { ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{num: {$lte: 0}}, {num: {$type: 'number'}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [" + "{$or: [{$nor: [{num: {$type: 'number'}}]}, {num: {$lte: 0}}]}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 'number'}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithTypeLong) { @@ -142,9 +147,11 @@ TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithTypeLong) { ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{num: {$lte: 0}}, {num: {$type: 18}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [" + "{$or: [{$nor: [{num: {$type: 'number'}}]}, {num: {$lte: 0}}]}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 18}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithTypeString) { @@ -153,9 +160,10 @@ TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithTypeString) { ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{$alwaysTrue: 1}, {num: {$type: 2}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [{$alwaysTrue: 1}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 2}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithNoType) { @@ -177,9 +185,11 @@ TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithExclusiveMaximumTrue) { ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{num: {$lt: 0}}, {num: {$type: 18}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [" + "{$or: [{$nor: [{num: {$type: 'number'}}]}, {num: {$lt: 0}}]}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 18}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithExclusiveMaximumFalse) { @@ -189,9 +199,11 @@ TEST(JSONSchemaParserTest, MaximumTranslatesCorrectlyWithExclusiveMaximumFalse) ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{num: {$lte: 0}}, {num: {$type: 18}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [" + "{$or: [{$nor: [{num: {$type: 'number'}}]}, {num: {$lte: 0}}]}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 18}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, FailsToParseIfMaximumIsNotANumber) { @@ -218,9 +230,11 @@ TEST(JSONSchemaParserTest, MinimumTranslatesCorrectlyWithTypeNumber) { ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{num: {$gte: 0}}, {num: {$type: 'number'}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [" + "{$or: [{$nor: [{num: {$type: 'number'}}]}, {num: {$gte: 0}}]}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 'number'}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, MinimumTranslatesCorrectlyWithTypeLong) { @@ -229,9 +243,11 @@ TEST(JSONSchemaParserTest, MinimumTranslatesCorrectlyWithTypeLong) { ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{num: {$gte: 0}}, {num: {$type: 18}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [" + "{$or: [{$nor: [{num: {$type: 'number'}}]}, {num: {$gte: 0}}]}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 18}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, MinimumTranslatesCorrectlyWithTypeString) { @@ -240,9 +256,10 @@ TEST(JSONSchemaParserTest, MinimumTranslatesCorrectlyWithTypeString) { ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{$alwaysTrue: 1}, {num: {$type: 2}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [{$alwaysTrue: 1}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 2}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, MinimumTranslatesCorrectlyWithNoType) { @@ -264,9 +281,11 @@ TEST(JSONSchemaParserTest, MinimumTranslatesCorrectlyWithExclusiveMinimumTrue) { ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{num: {$gt: 0}}, {num: {$type: 18}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [" + "{$or: [{$nor: [{num: {$type: 'number'}}]}, {num: {$gt: 0}}]}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 18}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, MinimumTranslatesCorrectlyWithExclusiveMinimumFalse) { @@ -276,9 +295,11 @@ TEST(JSONSchemaParserTest, MinimumTranslatesCorrectlyWithExclusiveMinimumFalse) ASSERT_OK(result.getStatus()); BSONObjBuilder builder; result.getValue()->serialize(&builder); - ASSERT_BSONOBJ_EQ( - builder.obj(), - fromjson("{$and: [{$and: [{$and: [{num: {$gte: 0}}, {num: {$type: 18}}]}]}]}")); + ASSERT_BSONOBJ_EQ(builder.obj(), + fromjson("{$and: [{$and: [{$and: [" + "{$or: [{$nor: [{num: {$type: 'number'}}]}, {num: {$gte: 0}}]}," + "{$or: [{$nor: [{num: {$exists: true}}]}, {num: {$type: 18}}]}" + "]}]}]}")); } TEST(JSONSchemaParserTest, FailsToParseIfMinimumIsNotANumber) { |