Skip to content

Refactor scoping and make default_scopes thread-safe #683

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.

Sign up for GitHub

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

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

Refactor scoping and make default_scopes thread-safe #683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 29 additions & 25 deletions lib/mongo_mapper/plugins/scopes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ def scopes
self._scopes ||= {}
end

def active_scopes
Thread.current["mongo_mapper_#{name}_active_scopes"] ||= []
end

def default_scopes
@default_scopes ||= begin
superclass.respond_to?(:default_scopes) ?
Expand All @@ -42,16 +38,26 @@ def default_scopes
end
end

# @param Hash options
# @options options [Boolean] :unscoped Specify to return plain query object
def query(options = {})
res = super(options)

all_anonymous_scopes.each do |scope|
unscoped do
res = process_scope(res, scope)
if options.delete(:unscoped)
context = super(options)
else
context = current_context

if context
context = process_scope(context, options)
else
context = super(options)

default_scopes.flatten.each do |scope|
context = process_scope(context, scope)
end
end
end

res
context
end

def default_scope(*args, &block)
Expand All @@ -66,28 +72,30 @@ def default_scope(*args, &block)
default_scopes
end

def with_scope(query = {})
active_scopes.push(query)
def with_scope(scope = {})
prev, self.current_context = current_context, process_scope(query, scope)
yield
ensure
active_scopes.pop
self.current_context = prev
end

def unscoped
old_default_scopes = default_scopes.dup
old_active_scopes = active_scopes.dup

@default_scopes = []
active_scopes.clear

prev, self.current_context = current_context, query(unscoped: true)
yield
ensure
@default_scopes = old_default_scopes
active_scopes.concat(old_active_scopes)
self.current_context = prev
end

private

def current_context
Thread.current["mongo_mapper_#{name}_current_context"]
end

def current_context=(context)
Thread.current["mongo_mapper_#{name}_current_context"] = context
end

def process_scope(context, scope, *args)
if scope.is_a?(Proc)
scope = context.instance_exec(*args, &scope)
Expand All @@ -98,10 +106,6 @@ def process_scope(context, scope, *args)
scope
end

def all_anonymous_scopes
[default_scopes + active_scopes].flatten
end

RESTRICTED_CLASS_METHODS = %w(private public protected allocate new name parent superclass)
private_constant :RESTRICTED_CLASS_METHODS

Expand Down
135 changes: 131 additions & 4 deletions spec/functional/scopes_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,17 @@
docs.first.age.should == 50
end

it "should work with scoped queries swapped" do
u1 = @document.create(:name => 'John', :age => 60)
u2 = @document.create(:name => 'John', :age => 50)
u3 = @document.create(:name => 'Bill', :age => 50)

q1 = @document.by_name('John')
q2 = @document.by_age(50)
q1.all.should == [u1, u2]
q2.all.should == [u2, u3]
end

context "with model methods" do
it "should work if method returns a query" do
@document.create(:name => 'John', :age => 10)
Expand Down Expand Up @@ -253,6 +264,84 @@ class ::Blog < ::Item
@klass.all.should include(u2)
end

it "should work in nested" do
u1 = @klass.create!(:first_name => "Scott", :age => 10)
u2 = @klass.create!(:first_name => "Scott", :age => 20)
u3 = @klass.create!(:first_name => "Andrew", :age => 20)

run = false

@klass.with_scope(:first_name => "Scott") do
@klass.with_scope(:age => 20) do
run = true
@klass.all.should == [u2]
end

@klass.all.should == [u1, u2]
end

run.should == true

@klass.all.should == [u1, u2, u3]
end

it "should work with nested two class scopes" do
other = Doc("Other") do
end

u1 = @klass.create!(:first_name => "Scott")
u2 = @klass.create!(:first_name => "Andrew")

a1 = other.create!(:first_name => "Scott")
a2 = other.create!(:first_name => "Andrew")

run = false

@klass.with_scope(:first_name => "Scott") do
other.with_scope(:first_name => "Andrew") do
run = true
@klass.all.should == [u1]
other.all.should == [a2]
end

@klass.all.should == [u1]
other.all.should == [a1, a2]
end

run.should == true

@klass.all.should == [u1, u2]
other.all.should == [a1, a2]
end

it "should work with default scope" do
u1 = @klass.create!(:first_name => "Scott", :age => 10)
u2 = @klass.create!(:first_name => "Scott", :age => 20)
u3 = @klass.create!(:first_name => "Andrew", :age => 20)

@klass.default_scope { where(age: 20) }

result = @klass.with_scope(:first_name => "Scott") do
@klass.all
end

result.should == [u2]
end

it "should work with scope chain" do
u1 = @klass.create!(:first_name => "Scott", :age => 10)
u2 = @klass.create!(:first_name => "Scott", :age => 20)
u3 = @klass.create!(:first_name => "Andrew", :age => 20)

@klass.scope :by_age, lambda { |age| where(age: age) }

result = @klass.by_age(20).with_scope(:first_name => "Scott") do
@klass.all
end

result.should == [u2]
end

it "should be able to use an unscoped query" do
u1 = @klass.create!(:first_name => "Scott")
u2 = @klass.create!(:first_name => "Andrew")
Expand Down Expand Up @@ -368,6 +457,19 @@ class ::Blog < ::Item
@subclass.default_scopes.length.should == 2
@klass.default_scopes.length.should == 1
end

it "should be cleared in unscoped" do
@klass.key :active, Boolean, default: true

normal = @klass.create!
inactive = @klass.create!(active: false)

@klass.default_scope { where(active: true) }

@klass.unscoped do
@klass.all.should == [normal, inactive]
end
end
end
end

Expand Down Expand Up @@ -473,8 +575,8 @@ def wait_for_threads!
@threads.each { |t| t.join }
end

it "should not taint another thread's active scopes" do
count_active_scopes = nil
it "should not taint another thread's current_context" do
is_current_context_nil = nil
second_thread_run = false

make_thread do
Expand All @@ -487,13 +589,13 @@ def wait_for_threads!
end

make_thread do
count_active_scopes = @klass.active_scopes.length
is_current_context_nil = @klass.send(:current_context).nil?
second_thread_run = true
end

wait_for_threads!

count_active_scopes.should == 0
is_current_context_nil.should == true
end

it "should not taint another thread's count method" do
Expand Down Expand Up @@ -521,5 +623,30 @@ def wait_for_threads!

klass_count.should == 2
end

it "should not taint another thread's default_scopes" do
@klass.default_scope { where }

count_default_scopes = nil
second_thread_run = false

make_thread do
@klass.unscoped do
loop do
sleep Float::EPSILON
break if second_thread_run
end
end
end

make_thread do
count_default_scopes = @klass.default_scopes.length
second_thread_run = true
end

wait_for_threads!

count_default_scopes.should == 1
end
end
end