Skip to content
This repository
Browse code

Clean up scope delegation

  • Loading branch information...
commit 5aad6ae7a9f5a2c430e89ae345e7dde588802128 1 parent d119911
Ben Johnson authored February 06, 2010
2  lib/searchlogic/active_record/consistency.rb
@@ -40,7 +40,7 @@ def merge_joins_with_consistent_conditions(*args)
40 40
       
41 41
       
42 42
       def merge_joins_with_merged_duplicates(*args)
43  
-        args << ""
  43
+        args << "" if !Thread.current["searchlogic_delegation"]
44 44
         joins = merge_joins_without_merged_duplicates(*args)
45 45
       end
46 46
     end
21  lib/searchlogic/active_record/named_scope_tools.rb
@@ -38,6 +38,27 @@ def named_scope_arity(name)
38 38
         options.respond_to?(:arity) ? options.arity : nil
39 39
       end
40 40
       
  41
+      # When searchlogic calls a named_scope on a foreigh model it will execute that scope and then call scope(:find).
  42
+      # When we get these options we want this to be in an exclusive scope, especially if we are calling a condition on
  43
+      # the same originating model:
  44
+      #
  45
+      #   Company.users_company_name_equals("name")
  46
+      #
  47
+      # If we aren't in an exclusive scope we will get unexpected results for the :joins option. Lastly, we want the named_scopes
  48
+      # generated by searchlogic to be symbols whenever possible. The reason for this is so that we can allow
  49
+      # ActiveRecord to leverage its joins library that automatically aliases joins if they appear more than once in a query.
  50
+      # If the joins are strings, AtiveRecord can't do anything. Because the code that does this in ActiveRecord is pretty bad
  51
+      # when it comes to being consisitent, searchlogic had to fix this in Searchloigc::ActiveRecord::Consistency. That said,
  52
+      # 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.
  53
+      # Only when we are performing an action on it. This is what the searchlogic_delegation thread variable is all about. A
  54
+      # flag to let search logic know not to convert joins to strings.
  55
+      def in_searchlogic_delegation(&block)
  56
+        old = Thread.current["searchlogic_delegation"]
  57
+        Thread.current["searchlogic_delegation"] = true
  58
+        with_exclusive_scope(&block)
  59
+        Thread.current["searchlogic_delegation"] = old
  60
+      end
  61
+      
41 62
       # A convenience method for creating inner join sql to that your inner joins
42 63
       # are consistent with how Active Record creates them. Basically a tool for
43 64
       # you to use when writing your own named scopes. This way you know for sure
11  lib/searchlogic/named_scopes/association_conditions.rb
@@ -66,9 +66,7 @@ def association_condition_options(association, association_condition, args, poly
66 66
             # The underlying condition doesn't require any parameters, so let's just create a simple
67 67
             # named scope that is based on a hash.
68 68
             options = {}
69  
-            with_exclusive_scope do
70  
-              options = scope.scope(:find)
71  
-            end
  69
+            in_searchlogic_delegation { options = scope.scope(:find) }
72 70
             prepare_named_scope_options(options, association, poly_class)
73 71
             options
74 72
           else
@@ -78,10 +76,13 @@ def association_condition_options(association, association_condition, args, poly
78 76
             eval <<-"end_eval"
79 77
               searchlogic_lambda(:#{arg_type}) { |#{proc_args.join(",")}|
80 78
                 options = {}
81  
-                with_exclusive_scope do
  79
+                
  80
+                in_searchlogic_delegation do
82 81
                   scope = klass.send(association_condition, #{proc_args.join(",")})
83  
-                  options = scope ? scope.scope(:find) : {}
  82
+                  options = scope.scope(:find) if scope
84 83
                 end
  84
+                
  85
+                
85 86
                 prepare_named_scope_options(options, association, poly_class)
86 87
                 options
87 88
               }
3  lib/searchlogic/named_scopes/or_conditions.rb
@@ -131,7 +131,8 @@ def merge_scopes_with_or(scopes)
131 131
           scopes_options = scopes.collect { |scope| scope.scope(:find) }
132 132
           conditions = scopes_options.reject { |o| o[:conditions].nil? }.collect { |o| sanitize_sql(o[:conditions]) }
133 133
           scope = scopes_options.inject(scoped({})) { |current_scope, options| current_scope.scoped(options) }
134  
-          options = scope.scope(:find)
  134
+          options = {}
  135
+          in_searchlogic_delegation { options = scope.scope(:find) }
135 136
           options.delete(:readonly) unless scopes.any? { |scope| scope.proxy_options.key?(:readonly) }
136 137
           options.merge(:conditions => "(" + conditions.join(") OR (") + ")")
137 138
         end
8  spec/active_record/consistency_spec.rb
... ...
@@ -1,7 +1,7 @@
1 1
 require File.expand_path(File.dirname(__FILE__) + "/../spec_helper")
2 2
 
3 3
 describe "Consistency" do
4  
-  it "should merege joins with consistent conditions" do
  4
+  it "should merge joins with consistent conditions" do
5 5
     user_group = UserGroup.create
6 6
     user_group.users.user_groups_name_like("name").user_groups_id_gt(10).scope(:find)[:joins].should == [
7 7
       "INNER JOIN \"user_groups_users\" ON \"user_groups_users\".user_id = \"users\".id",
@@ -19,4 +19,10 @@
19 19
     
20 20
     Company.users_company_name_like("name").users_company_description_like("description").users_company_created_at_after(Time.now).scope(:find).should == {}
21 21
   end
  22
+  
  23
+  it "shuold not convert joins to strings when delegating via associations" do
  24
+    User.alias_scope :has_id_gt, lambda { User.id_gt(10).has_name.orders_id_gt(10) }
  25
+    User.alias_scope :has_name, lambda { User.orders_created_at_after(Time.now).name_equals("ben").username_equals("ben") }
  26
+    Company.users_has_id_gt.proxy_options[:joins].should == {:users=>[:orders]}
  27
+  end
22 28
 end
21  spec/named_scopes/association_conditions_spec.rb
@@ -86,25 +86,38 @@
86 86
   
87 87
   it "should implement exclusive scoping" do
88 88
     scope = Company.users_company_name_like("name").users_company_description_like("description")
89  
-    scope.scope(:find)[:joins].should == [{:users => :company}]
  89
+    scope.scope(:find)[:joins].should == [
  90
+      "INNER JOIN \"users\" ON companies.id = users.company_id",
  91
+      "INNER JOIN \"companies\" companies_users ON \"companies_users\".id = \"users\".company_id"
  92
+    ]
90 93
     lambda { scope.all }.should_not raise_error
91 94
   end
92 95
   
93 96
   it "should not create the same join twice" do
94 97
     scope = Company.users_orders_total_gt(10).users_orders_taxes_lt(5).ascend_by_users_orders_total
95  
-    scope.scope(:find)[:joins].should == [{:users => :orders}]
  98
+    scope.scope(:find)[:joins].should == [
  99
+      "INNER JOIN \"users\" ON companies.id = users.company_id",
  100
+      "INNER JOIN \"orders\" ON orders.user_id = users.id"
  101
+    ]
96 102
     lambda { scope.count }.should_not raise_error
97 103
   end
98 104
   
99 105
   it "should not create the same join twice when traveling through the duplicate join" do
100 106
     scope = Company.users_username_like("bjohnson").users_orders_total_gt(100)
101  
-    scope.scope(:find)[:joins].should == [{:users => :orders}, :users]
  107
+    scope.scope(:find)[:joins].should == [
  108
+      "INNER JOIN \"users\" ON companies.id = users.company_id",
  109
+      "INNER JOIN \"orders\" ON orders.user_id = users.id"
  110
+    ]
102 111
     lambda { scope.count }.should_not raise_error
103 112
   end
104 113
   
105 114
   it "should not create the same join twice when traveling through the deep duplicate join" do
106 115
     scope = Company.users_orders_total_gt(100).users_orders_line_items_price_gt(20)
107  
-    scope.scope(:find)[:joins].should == [{:users => {:orders => :line_items}}, {:users => :orders}]
  116
+    scope.scope(:find)[:joins].should == [
  117
+      "INNER JOIN \"users\" ON companies.id = users.company_id",
  118
+      "INNER JOIN \"orders\" ON orders.user_id = users.id",
  119
+      "INNER JOIN \"line_items\" ON line_items.order_id = orders.id"
  120
+    ]
108 121
     lambda { scope.all }.should_not raise_error
109 122
   end
110 123
   

0 notes on commit 5aad6ae

Please sign in to comment.
Something went wrong with that request. Please try again.