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

Refactor scoping and make default_scopes thread-safe #683

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

a2ikm
Copy link
Contributor

@a2ikm a2ikm commented Mar 5, 2021

We've used active_scopes and default_scopes to store scopes in the current context. However, it has been thread-unsafe because we clear default_scopes to unscope.
Moreover, default_scopes is the master data of default scopes, and we shouldn't clear it.

Therefore, this commit introduces current_context as a thread local variable to store the current scoped context in it:

  • When unscoped { ... } is called, current_context stores a plain query object
  • When with_scope(...) { ... } is called, current_context stores a query object scoped with default_scopes and the method's arguments
  • Otherwise, it stores nothing - nil, so we need to build a new query with default_scopes

Then, we don't have to clear default_scopes to unscope.current_context includes active scopes naturally, so this commit removes the method.

We've used `active_scopes` and `default_scopes` to store scopes in the current
context. However, it has been thread-unsafe because we clear `default_scopes`
to unscope.
Moreover, `default_scopes` is the master data of default scopes, and we shouldn't
clear it.

Therefore, this commit introduces `current_context` as a thread local variable to
store the current scoped context in it:

- When `unscoped { ... }` is called, `current_context` stores a plain query object
- When `with_scope(...) { ... }` is called, `current_context` stores a query object
  scoped with `default_scopes` and the method's arguments
- Otherwise, it stores nothing - nil, so we need to build a new query with
  `default_scopes`

`current_context` includes active scopes naturally, so this commit removes the method.
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.

None yet

1 participant