Improve field validation for documents by plaan · Pull Request #769 · mongomock/mongomock

Improve field validation for documents by plaan · Pull Request #769 · mongomock/mongomock

Starting from mongodb 3.6 field names cannot contain the null character and top-field names cannot start with the dollar sign. Apart form those cases mongodb does permit storage of the dots and dollar signs (mongodb v3.6). Newer versions of mongodb add improved support (mongodb v4.0, mongodb v5.0).

Fixes: #720

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your contribution.

Please add a test that illustrates that recent versions accept weirder names.


		recursive_none_key_check(data)
		for key in data.keys():
		if len(key) > 0 and key[0] == "$":

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use key.startswith

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot about that one, thanks!



		def _validate_data_fields(data):
		def recursive_none_key_check(data):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function isn't using any closure, so I'd rather not have it as a nested function

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it out of the function

		def _validate_data_fields(data):
		def recursive_none_key_check(data):
		for key, value in data.items():
		if "\0" in key:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use single quotes for literal strings. '\0' (same in all your changes).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to using all single quotes

Hi @pcorpet, wondering if you have the time for a second review. I made sure to add your suggested changes!

Hi @pcorpet @plaan do we have any update on this? We are encountering with this issue, and we would like to solve it.

This would be a great one so the mocking gets closer to the validation of the actual mongo. Any updates on getting this one merged?

Thanks for pinging folks, it's working. I'll take a look now.