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

[ON HOLD] MONGOID-5703: [Monkey Patch Removal] Extract symbol query macros into a gem #5747

Closed

Conversation

johnnyshields
Copy link
Contributor

@johnnyshields johnnyshields commented Nov 14, 2023

Fixes MONGOID-5703.

This PR extracts Symbol query-related monkey patches such as :symbol.eq, :symbol.in, etc. into a separate directory, which will be planned to be moved to a gem in the future.

This removes 35 different Monkey Patches on the Symbol class in total.

  • TODO: Make separate PR which deprecates the query operators. Including the gem should override (i.e. undo) the deprecations.
  • TODO: Tests (unit and integration)
  • TODO: Remove usages of Symbol monkey patches from the main app tests.

Reasons:

  • Refer to MONGOID-5660 for a litany of reasons why Monkey Patches are bad (conflicts, security, magic, user surprise, etc.)
  • In addition, these symbols are a sort of query DSL which is not native to MongoDB itself. It is much better for users to become used to writing MQL in Ruby (e.g. { name: { '$in' => %w[Alice Bob] } } rather than this Symbol hack { :name.in => %w[Alice Bob] }
  • The extraction itself is straightforward and low-risk.
  • There is no equivalent in ActiveRecord to this Symbol DSL.
  • This can be moved to a gem (we can host it in Github Mongoid org where other previously-extracted gems such as Mongoid::Paranoia are) so it won't incur any migration cost for users who still need it when upgrading.

…Mongoid entirely in the future).

This removes 35 different Monkey Patches on the symbol class.
@johnnyshields johnnyshields changed the title Extract symbol query macros into a gem MONGOID-5703: [Monkey Patch Removal] Extract symbol query macros into a gem Nov 14, 2023
@johnnyshields
Copy link
Contributor Author

@jamis @alexbevi please confirm this approach is acceptable--moving these methods to a gem--and I will work to complete this PR.

@jamis
Copy link
Contributor

jamis commented Nov 14, 2023

Please hold off on that, Johnny. I don't think we have appetite right now for removing the Symbol monkeypatches.

@johnnyshields johnnyshields changed the title MONGOID-5703: [Monkey Patch Removal] Extract symbol query macros into a gem [ON HOLD] MONGOID-5703: [Monkey Patch Removal] Extract symbol query macros into a gem Nov 15, 2023
@johnnyshields johnnyshields marked this pull request as draft November 15, 2023 11:32
@alexbevi
Copy link
Contributor

Closing this out as this is not the path we're choosing to pursue presently. As this is a draft PR the decision can be revisited if conditions change.

@alexbevi alexbevi closed this Mar 27, 2024
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.

3 participants