Skip to content

Refactor scoping and make default_scopes thread-safe#683

Open
a2ikm wants to merge 1 commit into
mongomapper:masterfrom
a2ikm:make-default-scope-thread-safe
Open

Refactor scoping and make default_scopes thread-safe#683
a2ikm wants to merge 1 commit into
mongomapper:masterfrom
a2ikm:make-default-scope-thread-safe

Conversation

@a2ikm

@a2ikm a2ikm commented Mar 5, 2021

Copy link
Copy Markdown
Contributor

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.

1 participant