Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MONGOID-5665 [Monkey Patch Removal] Remove Object#__find_args__ #5706

Merged
merged 6 commits into from
Nov 6, 2023

Conversation

johnnyshields
Copy link
Contributor

@johnnyshields johnnyshields commented Sep 3, 2023

Fixes MONGOID-5665

Object#__find_args__ is a kernel monkey patch intended for use in Criteria::Findable. It prepare arguments for the Criteria.find method (it's somewhat beguiling name is intended to mean "args for the find method".)

Please merge #5702 before this one.

This PR does the following:

  1. Move Object#__find_args__ to a private method in Criteria::Findable (now named #prepare_ids_for_find)
  2. The PR does not add specs for the private method, as per standard.
  3. __find_args__ was mistakenly called in Atomic#unset method. Although __find_args__ does a similar thing as what #unset requires, it is pure coincidence and its usage here is a kludge. So I've inlined the code and made a new private method.
  4. __find_args__ for Range previously called to_a. In the case of String, this is actually a bug / DOS risk, because ranges of strings tend to explode when you use to_a. As an example, ('64e0'..'64ff').to_a.size => 9320, and it gets exponentially worse for 24-char BSON ids! Interestingly however, the MongoDB server itself can handle ranges of id strings gracefully, so I am now just passing them into the DB as-is and it magically works--I've added tests. (Range<Numeric, Numeric> is still converted to_a)
  5. The new code also supports the case of using a Set of IDs as an arg.

Overall progress is tracked here: http://tinyurl.com/mongoid-monkey. Refer to MONGOID-5660 for context.

…e where it is now a private method.

2 points to note:
- __find_args__ was mistakenly called in Atomic#unset method. Although technically __find_args__ does a similar thing as what #unset requires for its argument preparation, this is pure coincidence and its a kludge to use find_args here. So I've inlined the code and made a new private method.
- __find_args__ for Range previously called to_a. This is actually a bug / DOS risk, because ranges of strings tend to explode when you use to_a. As an example, ('64e0'..'64ff').to_a.size => 9320, and it gets much worse for 24-char BSON ids! Interestingly however, MongoDB itself can handle ranges of ids gracefully, so I am now just passing them into the DB as-is and it magically works--I've added tests.
@johnnyshields johnnyshields changed the title Remove Object#__find_args__This primarily used in Criteria::Findabl… Remove Object#__find_args__ Sep 3, 2023
@johnnyshields johnnyshields changed the title Remove Object#__find_args__ MONGOID-5665 [Monkey Patch Removal] Remove Object#__find_args__ Sep 3, 2023
@jamis jamis merged commit 67c8035 into mongodb:master Nov 6, 2023
15 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants