Skip to content

Commit

Permalink
Clean up scope delegation
Browse files Browse the repository at this point in the history
  • Loading branch information
binarylogic committed Feb 6, 2010
1 parent d119911 commit 5aad6ae
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 12 deletions.
2 changes: 1 addition & 1 deletion lib/searchlogic/active_record/consistency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def merge_joins_with_consistent_conditions(*args)


def merge_joins_with_merged_duplicates(*args)
args << ""
args << "" if !Thread.current["searchlogic_delegation"]
joins = merge_joins_without_merged_duplicates(*args)
end
end
Expand Down
21 changes: 21 additions & 0 deletions lib/searchlogic/active_record/named_scope_tools.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,27 @@ def named_scope_arity(name)
options.respond_to?(:arity) ? options.arity : nil
end

# When searchlogic calls a named_scope on a foreigh model it will execute that scope and then call scope(:find).
# When we get these options we want this to be in an exclusive scope, especially if we are calling a condition on
# the same originating model:
#
# Company.users_company_name_equals("name")
#
# If we aren't in an exclusive scope we will get unexpected results for the :joins option. Lastly, we want the named_scopes
# generated by searchlogic to be symbols whenever possible. The reason for this is so that we can allow
# ActiveRecord to leverage its joins library that automatically aliases joins if they appear more than once in a query.
# If the joins are strings, AtiveRecord can't do anything. Because the code that does this in ActiveRecord is pretty bad
# when it comes to being consisitent, searchlogic had to fix this in Searchloigc::ActiveRecord::Consistency. That said,
# part of this fix is to normalize joins into strings. We do not want to do this if we are calling scopes on foreigh models.
# Only when we are performing an action on it. This is what the searchlogic_delegation thread variable is all about. A
# flag to let search logic know not to convert joins to strings.
def in_searchlogic_delegation(&block)
old = Thread.current["searchlogic_delegation"]
Thread.current["searchlogic_delegation"] = true
with_exclusive_scope(&block)
Thread.current["searchlogic_delegation"] = old
end

# A convenience method for creating inner join sql to that your inner joins
# are consistent with how Active Record creates them. Basically a tool for
# you to use when writing your own named scopes. This way you know for sure
Expand Down
11 changes: 6 additions & 5 deletions lib/searchlogic/named_scopes/association_conditions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ def association_condition_options(association, association_condition, args, poly
# The underlying condition doesn't require any parameters, so let's just create a simple
# named scope that is based on a hash.
options = {}
with_exclusive_scope do
options = scope.scope(:find)
end
in_searchlogic_delegation { options = scope.scope(:find) }
prepare_named_scope_options(options, association, poly_class)
options
else
Expand All @@ -78,10 +76,13 @@ def association_condition_options(association, association_condition, args, poly
eval <<-"end_eval"
searchlogic_lambda(:#{arg_type}) { |#{proc_args.join(",")}|
options = {}
with_exclusive_scope do
in_searchlogic_delegation do
scope = klass.send(association_condition, #{proc_args.join(",")})
options = scope ? scope.scope(:find) : {}
options = scope.scope(:find) if scope
end
prepare_named_scope_options(options, association, poly_class)
options
}
Expand Down
3 changes: 2 additions & 1 deletion lib/searchlogic/named_scopes/or_conditions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ def merge_scopes_with_or(scopes)
scopes_options = scopes.collect { |scope| scope.scope(:find) }
conditions = scopes_options.reject { |o| o[:conditions].nil? }.collect { |o| sanitize_sql(o[:conditions]) }
scope = scopes_options.inject(scoped({})) { |current_scope, options| current_scope.scoped(options) }
options = scope.scope(:find)
options = {}
in_searchlogic_delegation { options = scope.scope(:find) }
options.delete(:readonly) unless scopes.any? { |scope| scope.proxy_options.key?(:readonly) }
options.merge(:conditions => "(" + conditions.join(") OR (") + ")")
end
Expand Down
8 changes: 7 additions & 1 deletion spec/active_record/consistency_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require File.expand_path(File.dirname(__FILE__) + "/../spec_helper")

describe "Consistency" do
it "should merege joins with consistent conditions" do
it "should merge joins with consistent conditions" do
user_group = UserGroup.create
user_group.users.user_groups_name_like("name").user_groups_id_gt(10).scope(:find)[:joins].should == [
"INNER JOIN \"user_groups_users\" ON \"user_groups_users\".user_id = \"users\".id",
Expand All @@ -19,4 +19,10 @@

Company.users_company_name_like("name").users_company_description_like("description").users_company_created_at_after(Time.now).scope(:find).should == {}
end

it "shuold not convert joins to strings when delegating via associations" do
User.alias_scope :has_id_gt, lambda { User.id_gt(10).has_name.orders_id_gt(10) }
User.alias_scope :has_name, lambda { User.orders_created_at_after(Time.now).name_equals("ben").username_equals("ben") }
Company.users_has_id_gt.proxy_options[:joins].should == {:users=>[:orders]}
end
end
21 changes: 17 additions & 4 deletions spec/named_scopes/association_conditions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,25 +86,38 @@

it "should implement exclusive scoping" do
scope = Company.users_company_name_like("name").users_company_description_like("description")
scope.scope(:find)[:joins].should == [{:users => :company}]
scope.scope(:find)[:joins].should == [
"INNER JOIN \"users\" ON companies.id = users.company_id",
"INNER JOIN \"companies\" companies_users ON \"companies_users\".id = \"users\".company_id"
]
lambda { scope.all }.should_not raise_error
end

it "should not create the same join twice" do
scope = Company.users_orders_total_gt(10).users_orders_taxes_lt(5).ascend_by_users_orders_total
scope.scope(:find)[:joins].should == [{:users => :orders}]
scope.scope(:find)[:joins].should == [
"INNER JOIN \"users\" ON companies.id = users.company_id",
"INNER JOIN \"orders\" ON orders.user_id = users.id"
]
lambda { scope.count }.should_not raise_error
end

it "should not create the same join twice when traveling through the duplicate join" do
scope = Company.users_username_like("bjohnson").users_orders_total_gt(100)
scope.scope(:find)[:joins].should == [{:users => :orders}, :users]
scope.scope(:find)[:joins].should == [
"INNER JOIN \"users\" ON companies.id = users.company_id",
"INNER JOIN \"orders\" ON orders.user_id = users.id"
]
lambda { scope.count }.should_not raise_error
end

it "should not create the same join twice when traveling through the deep duplicate join" do
scope = Company.users_orders_total_gt(100).users_orders_line_items_price_gt(20)
scope.scope(:find)[:joins].should == [{:users => {:orders => :line_items}}, {:users => :orders}]
scope.scope(:find)[:joins].should == [
"INNER JOIN \"users\" ON companies.id = users.company_id",
"INNER JOIN \"orders\" ON orders.user_id = users.id",
"INNER JOIN \"line_items\" ON line_items.order_id = orders.id"
]
lambda { scope.all }.should_not raise_error
end

Expand Down

0 comments on commit 5aad6ae

Please sign in to comment.