Fix failing tests

[franmomu]

Q	A
Type	task
BC Break	no
Fixed issues

Summary

These tests started to fail using the 1.14 version of the driver: https://github.com/doctrine/mongodb-odm/runs/7379626257?check_suite_focus=true.

Digging into these fails I've ended up in mongodb/mongo-c-driver#988 which seems to be the origin.

IIUC #2238 (comment), the data in the tests is from https://www.mongodb.com/docs/manual/core/schema-validation/#other-query-expressions, so I've updated it to simplify it.

Should we add another field and validation using $regularExpression?

The remaining fail is because of doctrine/annotations#438, maybe we should remove that tests since it's up to the reader to throw errors or bump doctrine/annotations to the latest version (1.13.3).

[malarzm]

Re $regex change - do you know if these two formats were OK before and we were just suggesting wrong one, or is it more problematic change that maybe could be raised to the MongoDB team?

The remaining fail is because of doctrine/annotations#438, maybe we should remove that tests since it's up to the reader to throw errors or bump doctrine/annotations to the latest version (1.13.3).

Feel free to remove the test. It'd have value without types in the ctor 👍

[franmomu]

Re $regex change - do you know if these two formats were OK before and we were just suggesting wrong one, or is it more problematic change that maybe could be raised to the MongoDB team?

I have no idea to be honest, I couldn't find an example like the format we had in the documentation:

• https://www.mongodb.com/docs/manual/reference/operator/query/regex/
• https://www.mongodb.com/docs/manual/reference/mongodb-extended-json/#mongodb-bsontype-Regular-Expression

maybe @alexandre-abrioux can help here?

[alexandre-abrioux]

Hey! Nice find for the mongo-c-driver change. The tested format is documented here, so it should still work, it looks like a regression in the driver. I'm short on time right now, but I can dig deeper this WE or next week.

[malarzm]

cc @jmikola we might need your awesome detective skills :)

[jmikola]

I wasn't aware of these examples, but I see no reason we should have ever nested $regularExpression within a $regex. $regularExpression is the extended JSON syntax for a BSON regex type. $regex in extended JSON was legacy syntax that predated that spec, and it wasn't kept because of the conflict with the $regex query operator. The only example I found of nesting $regularExpression within $regex is within a note in the Extended JSON spec about parsing legacy syntax.

To clarify, were you seeing a "Unexpected nested object value for..." error on the latest version of the PHP driver (with libmongoc 1.22)? I think the code path we're seeing here relates to this bit of the libmongoc PR @franmomu dug up. I'm not familiar with the PR but it looks like some of the legacy parsing might have been removed. That might have been unintended, so I'll raise a comment in CDRIVER-3380 to confirm.

Assuming these code examples were depending on the legacy syntax parser, I think the direct change would be to drop $regex and just use the $regularExpression object as the value of email. That would continue to match an email field against a BSON regex type.

That said, matching email with the query operator works just as well. IMO, there isn't much reason to use a BSON regex type when writing queries. It's more useful when you actually need to store a regex type for some reason.

Outside of extended JSON parsing, I don't know offhand if a $regex query operator would accept a BSON regex type; however, that wouldn't apply here since the entire $jsonSchema string is likely being run through the driver's extended JSON parser. I suppose this is more an academic question were you to write something like a find query that used $regex and an instance of MongoDB\BSON\Regex. Not worth thinking about further, but I wan't to mention it :)

[alexandre-abrioux]

Thank you, @jmikola, for all the details! I think I used $regularExpression in the tests and documentations of the @Validator feature (here and here) to showcase how to add modifiers to the regex, since this is not possible with the query notation in this context. However, using the legacy notation was a mistake from my end. Honestly, I don't have much experience with the extended JSON spec 😆 Let's switch back to the recommended notation, I also suggest we update the previously linked docs:

{ "email": { "$regularExpression" : { "pattern": "@mongodb\\.com$", "options": ""  } } },

[jmikola]

I think I used $regularExpression in the tests and documentations of the @Validator feature (here and here) to showcase how to add modifiers to the regex, since this is not possible with the query notation in this context.

@alexandre-abrioux: Just to clarify, I think modifiers would still have been possible by using $options as a sibling field of $regex, as demonstrated in the $regex docs. Since the MongoDB manual is written with the JavaScript shell in mind, they refer to /pattern/ instead of "BSON regex type"; however, the two are interchangeable for most intents and purposes. /pattern/ is JavaScript's native regex type, which is going to serialize to a BSON regex type. Since PHP has no native regex type, we only use the BSON regex class.

$regex vs. /pattern/ Syntax is basically explaining the subtle differences between the two and why you might need to use the $regex query operator in some cases (mostly alongside other operators such as $in).

[alexandre-abrioux]

@jmikola Great! Thanks for clarifying!

[malarzm]

I'm gonna merge this with failing SA/CS jobs so the history of this PR (especially conversation) is easily accessed through git blame. Failing jobs will be fixed soon in a subsequent PR. Thanks @franmomu!