This project is archived and is in readonly mode.
default_scope can't take procs
Reported by Brian Mitchell | January 28th, 2009 @ 10:24 PM | in 3.1
The primary problem I have with the current implementation is that it only supports a hash as the argument. I'd like to be able to pass in a proc as well similar to how named and anonymous scopes currently work.
My current work-around involves pushing a lambda into default_scoping and implementing a simple argument selector to emulate [] on a hash but there are other problems later if named scopes are used in conjunction (I have to convert to a hash at that point). I've browsed through the code and not found a quick fix. I'll search more later but if someone else has an idea for fixing this I'd be grateful.
Comments and changes to this ticket
-
-
-
-
Adrian Mugnolo February 20th, 2009 @ 06:21 PM
+1
I've found that this would be very useful for simple model i18n like in:
default_scope :ordered, lambda { {:order => column_translated(:name)} } -
Adrian Mugnolo February 20th, 2009 @ 06:22 PM
Er, sorry. I meant:
default_scope lambda { {:order => column_translated(:name)} } -
Peter Wagenet March 16th, 2009 @ 09:15 PM
This surprised me too. From looking at the code it looks like the quick and easy solution to adding a default scope was taken. I'll poke around a bit and see if I can't figure something out.
-
Josh N. Abbott March 16th, 2009 @ 09:18 PM
Agreed. If you look at the implementation of default_scope vs. named_scope, you'll see a lot more time and thought went into named_scope.
I think that if we're going to have a default scope option, we should at least consider having it accept the same options as named_scope (such as Proc).
-
Peter Wagenet March 17th, 2009 @ 02:29 AM
Attached is a patch that adds support for setting a block for default scope as such:
default_scope do { :conditions => ["created_at < ?", Time.now] } endOther than this it behaves almost the same as the previous implementation. In the few places I had to change it, I tried to mimic the behavior of named_scopes. The most notable of these is that the order param now overwrites previous ones. For instance
class Person < ActiveRecord::Base default_scope :order => "name DESC" named_scope :by_age, :order => "age DESC" end Person.by_agewill produce the following order clause: "age DESC" where previously it would have produced "name DESC, age DESC".
Also, defining multiple default_scopes will stack effects in the same way that multiple nested_scopes would. This way you could define a sitewide default_scope and add on to it in subclasses.
Another small caveat is that, while the condition will be used in the create scope if it is a hash, it will be ignored if it is a string. So { :name => "Peter" } would be used whereas "name = 'Peter'" would not be. Personally, I don't think default scope should affect the create scope, but since that was already there I was hesitant to remove that functionality entirely.
Finally, to completely override the effects of default_scope, use #with_exclusive_scope.
Please let me know if you have any questions or suggestions.
-
Peter Wagenet March 17th, 2009 @ 02:30 AM
- Tag changed from “2.3, activerecord, default, scope” to “2.3, activerecord, default, patch, scope, tested”
Sorry about the formatting. Not sure what happened there. You should still be able to make sense of it.
-
Peter Wagenet March 17th, 2009 @ 03:09 PM
- Tag changed from “2.3, activerecord, default, patch, scope, tested” to “2.3, activerecord, default_scope, enhancement, named_scope, patch, scope, tested”
Updated the patch with a slightly cleaner version.
-
Peter Wagenet March 18th, 2009 @ 03:28 PM
Here's an even better version. Default scope isn't merged in until #scope is actually called which means that you can override the default scope from within a #with_scope block. I also added a #without_default_scope wrapper that temporarily disables all default_scopes for that class.
Person.with_scope(:find => { :conditions => { :active => true }}) do Person.without_default_scope do Person.find(:all) # => Returns all active people, ignoring any default_scope end end -
Peter Wagenet March 18th, 2009 @ 03:34 PM
Also, if you want to use this functionality in a Rails 2.3 app without patching, check out http://gist.github.com/81187
-
-
-
Pratik May 18th, 2009 @ 08:56 PM
Hey Peter,
Any reason to go with block instead of procs ? Why not the following :
default_scope proc { { :conditions => ["created_at < ?", Time.now] } } -
Peter Wagenet May 18th, 2009 @ 09:25 PM
Pratik,
I thought blocks looked nicer, but using procs would be more consistent with named_scope.
-
Pratik May 18th, 2009 @ 09:29 PM
Yeah. I think it should be consistent with named_scopes. Rest looks nice. Thanks for working on this !
-
runa August 3rd, 2009 @ 08:33 PM
- Tag changed from “2.3, activerecord, default_scope, enhancement, named_scope, patch, scope, tested” to “2.3, activerecord, bug, default_scope, enhancement, named_scope, patch, scope, tested”
Guys, I'm using the monkeypatch from "March 18th, 2009 @ 03:34 PM" and found the following problem: if you find() and object and include it's relationships, the related table default_scope is not set:
@@@ruby Loading development environment (Rails 2.3.2)
Employer.send(:default_scope){{:conditions => {:id => 33}}} job=Job.find(:first,:include => [:employer]) => #
(see, no scope in Employer)And runs: @@@ruby DEBUG Job Load (0.3ms) SELECT * FROMjobs LIMIT 1 DEBUG Employer Load (0.4ms) SELECT * FROMemployersWHERE (employers.id= 2424)if I now reload job's data and ask for the related employer object:
@@@rubyjob.reload DEBUG Job Load (0.4ms) SELECT * FROM
jobsWHERE (jobs.id= 56)
job.employer DEBUG Employer Load (0.4ms) SELECT * FROMemployersWHERE (employers.id= 2424) AND (employers.id= 33)(see the scope applied to Employer) -
runa August 3rd, 2009 @ 08:35 PM
Ok, I screwed the formatting before (no preview button? :)
Loading development environment (Rails 2.3.2) Employer.send(:default_scope){{:conditions => {:id => 33}}} job=Job.find(:first,:include => [:employer]) => #And runs:
DEBUG Job Load (0.3ms) SELECT * FROMjobs LIMIT 1 DEBUG Employer Load (0.4ms) SELECT * FROMemployersWHERE (employers.id= 2424)(see, no scope in Employer)
if I now reload job's data and ask for the related employer object: @@@ruby
Job.reload DEBUG Job Load (0.4ms) SELECT * FROM jobs WHERE (jobs.id = 56) job.employer DEBUG Employer Load (0.4ms) SELECT * FROM employers WHERE (employers.id = 2424) AND (employers.id = 33)(see the scope applied to Employer)
-
Brian Mitchell August 3rd, 2009 @ 08:41 PM
Right. I actually don't use that patch. Instead I create an object that poses as a Hash which seems to work well (a hash with a proc passed is not enough since you can't merge or iterate properly -- I basically have a proc subclass that generates proxied new hashes on demand).
-
runa August 3rd, 2009 @ 09:26 PM
Brian: could you give me an example? I would love to see a simpler solution!
-
Brian Mitchell August 3rd, 2009 @ 09:38 PM
I use something like this code: http://gist.github.com/160809
A hash poser can take defaults or a block (default is used when the block returns nothing) which allows you to conditionally provide something dynamic.
Let me know if you have trouble using it or not but it has proven to work consistently enough for one of my projects.
-
Steffen Bartsch October 2nd, 2009 @ 09:42 AM
Brian, is there any reason not to use the default-by-block function of Ruby's Hash implementation? I.e.
Hash.new {|hash, key| key == :conditions ? {:a_column => "a_value"} : nil }In my case, I needed to directly set self.default_scoping as default_scope copies the :condition value for the :create scope. Then, it worked perfectly.
-
Brian Mitchell October 2nd, 2009 @ 04:28 PM
@Steffen There are issues that still won't work on that method. Specifically creation scopes (it uses a has_key? rather than a nil check last time I looked) as well as proper scope merging as they use other keys and/or require iteration.
So if it is working for you, it is probably limited to very specific cases. I would not recommend trying to use that method in more general code.
-
-
Neeraj Singh June 29th, 2010 @ 05:19 AM
- State changed from “new” to “open”
- Importance changed from “” to “”
I am looking at rails3 code and looks like I am stuck.
def construct_finder_arel(options = {}, scope = nil) + options = options.call if options.is_a?(Proc) relation = options.is_a?(Hash) ? unscoped.apply_finder_options(options) : unscoped.merge(options) relation = scope.merge(relation) if scope relation endNotice the line with + sign. I added that line and now default_scope can take proc. However since the default_scope finally builds a relation object, the lazy loading behavior of default_scope is lost. It means if you define
default_scope proc { { :conditions => ["created_at < ?", Time.now] } }then every single time you will get the same value for Time.now which defeats the whole point of having a proc.
Please correct me but to the extent I know relation object doest not support laziness the way named_scope does.
I am marking this ticket as open so that we can get more eyes on this ticket.
-
Neeraj Singh August 13th, 2010 @ 02:25 PM
- Tag changed from “2.3, activerecord, bug, default_scope, enhancement, named_scope, patch, scope, tested” to “activerecord, default_scope, enhancement, named_scope, scope”
-
Neeraj Singh August 13th, 2010 @ 02:26 PM
- Tag changed from “activerecord, default_scope, enhancement, named_scope, scope” to “rails 3, activerecord, default_scope, enhancement, named_scope, scope”
-
Tanel Suurhans September 24th, 2010 @ 08:35 AM
The issue is that default scopes should be stored as procs and evaluated on the last moment. Evaluating the proc when appending the scope will not work as expected.
Its not too complicated to get the default_scope to accept procs with hash conditions, but supporting arel conditions is a bit more complex.
I managed to get some concept code working, supporting all possible default_scope conditions with the exception of arel conditions needing a unscoped prefix.As the scopes are merged and evaluated at the last possible moment, i.e the current_scoped_methods method, and arel conditions are delegated to the scoped method, it will end up in an endless loop (as scoped will access the current_scoped_methods method, which in turn then makes a call to scoped etc).
def default_scope(scope_options = {}) # wrap the scope into a lambda, evaluating the actual given conditions first self.default_scoping << lambda { construct_finder_arel(scope_options.is_a?(Proc) ? scope_options.call : scope_options) } end def current_scoped_methods # merge all scopes scoped_methods.inject(relation) do |relation, scope| relation.merge(scope.call) end end # model class Category < ActiveRecord::Base default_scope order(:name) default_scope lambda { { :conditions => ["created_at < ?", Time.now] } } default_scope lambda { where("created_at < ?", Time.now) } endNot sure if these changes would yield any side effects, haven't had time to run the test suite just yet. Plus the endless call chain would still also need solving.
-
Tim Morgan October 15th, 2010 @ 04:56 AM
This seems to make it work:
def current_scoped_methods #:nodoc: if m = scoped_methods.last and m.is_a?(Proc) unscoped { m.call } else scoped_methods.last end endThe commit is available (with unit test) in my fork here: http://github.com/seven1m/rails/commit/d0251fd1adac2dc2d20a7e4b4f0c...
Patch is also attached.
Let me know what you think!
-Tim
-
-
Tim Morgan October 19th, 2010 @ 03:38 PM
I rebased my fork, so the commit is now: http://github.com/seven1m/rails/commit/282f5bb6a6dd89d810e67f7bb864...
Can someone take a look at this?
-
Neeraj Singh October 19th, 2010 @ 05:33 PM
Looks good to me.
I would replace "and" with "&&" in line if m = scoped_methods.last and m.is_a?(Proc).
-
Tim Morgan October 19th, 2010 @ 05:41 PM
Replacing and with && would cause m to = true inside the conditional. "and" has lower operator precedence than = in Ruby.
-
Jon Leighton October 19th, 2010 @ 05:49 PM
Nice one for doing a patch.
Personally I prefer:
def current_scoped_methods #:nodoc: last = scoped_methods.last if last.is_a?(Proc) unscoped(&last) else last end endI dislike assignment in conditional tests, and I think you can use the
&operator to pass the block directly tounscoped. (Or did you discover some reason why this shouldn't be done?) -
Tim Morgan October 19th, 2010 @ 05:56 PM
I just forgot about the & operator for passing the block -- that should work. I'll be happy to remove the assignment from the conditional if you think it will get this accepted.
-
Neeraj Singh October 19th, 2010 @ 06:00 PM
Building on what Jon suggested, it can be reduced to two lines.
def current_scoped_methods #:nodoc: last = scoped_methods.last last.is_a?(Proc) ? unscoped(&last) : last end -
Jon Leighton October 19th, 2010 @ 06:05 PM
Heh, we're really grasping at straws now!
I have my head in my own patch at the moment, but will try to run the tests on this later and add a +1.
-
Tim Morgan October 19th, 2010 @ 10:14 PM
Here is the updated commit (updated patch is attached):
http://github.com/seven1m/rails/commit/e0b73b64c30e81e5b4279ce55dd2...
-
Aaron Patterson October 19th, 2010 @ 11:30 PM
- State changed from “open” to “committed”
- Milestone changed from “3.x” to “3.1”
- Tag set to “rails 3.1”
I've pushed this to master. As it's a new feature, I think it should go in the 3.1.0 release.
-
-
Adam Wróbel October 27th, 2010 @ 08:18 PM
Hey guys,
I'm a little confused. I've just cloned rails/master and built a gem only to find out this feature still isn't working. Tim's patch with new current_scoped_methods and test cases is merged, but Tanel's modification of default_scope method is not. The lambda is immediately passed to construct_finder_arel and produces a familiar error.
Also, Tim, ActiveRecord::Relation responds to :call so maybe current_scoped_methods should use is_a?(Proc) like in Neeraj's version.
I've just hopped into this conversation, so a huge thanks to you guys for looking into this issue.
Adam
-
Tim Morgan October 27th, 2010 @ 08:35 PM
Adam, I know the tests on edge rails pass for me as of Oct 20th (haven't tested since then), but I haven't tried it in a real app yet.
Since I am trying to upgrade a 2.x app, I am still working in 3.0 land, and the following monkeypatch gets me what I need for now:
module ActiveRecord class Base class << self protected def current_scoped_methods #:nodoc: method = scoped_methods.last if method.respond_to?(:call) unscoped(&method) else method end end end end end -
Draiken October 29th, 2010 @ 04:53 PM
Hi !
I need to set a conditional default_scope and after implementing Tim Morgan's solution on an initializer I still did not manage to pass a Proc to default_scopeI recieve a
NoMethodError: undefined methodincludes_values' for #<Proc:0xb5e7b214>on the merge method in the SpawnMethods module
Is there any other way to set a conditional default_scope other than passing a Proc? I really need this :/
-
Adam Wróbel October 29th, 2010 @ 05:19 PM
Hi Draiken,
That's exactly the same error I was getting. Just like I described in my last post I had to merge Tim's and Tanel's modifications. Attached is my git diff patch. Good luck!
-
Tim Morgan October 29th, 2010 @ 05:24 PM
Are you using latest checkout of 3-0-stable or the 3.0.1 gem?
I believe my patch will only work with 3-0-stable, i.e. upcoming 3.0.2 gem.
In your Gemfile, you can specify the following:
gem 'rails', :git => 'git://github.com/rails/rails.git', :branch => '3-0-stable'Let me know if that works.
-
Draiken October 29th, 2010 @ 06:29 PM
Adam Wróbel's patch worked perfectly.
I had only implemented Tim Morgan's snippet and not the previous one.
If anyone else have this problem, that patch works :]Thank you all very much.
-
Tanel Suurhans November 24th, 2010 @ 08:37 PM
It seems that with Tim Morgans patch the proc for the default scope is not evaluated properly. In production mode the value inside the proc (Time.now for example) stays the same for every invocation.
-
Tim Morgan November 24th, 2010 @ 09:13 PM
I just updated to Rails 3.0.3 and tested this in production mode. It works for me.
# irb require 'rubygems' require 'active_record' require 'logger' ActiveRecord::Base.establish_connection(:adapter => 'sqlite3', :database => ':memory:') ActiveRecord::Migration.create_table :widgets do |t| t.string :name t.timestamps end class Widget < ActiveRecord::Base default_scope lambda { where(['created_at >= ?', 1.minute.ago]) } end module ActiveRecord class Base class << self protected def current_scoped_methods #:nodoc: method = scoped_methods.last if method.respond_to?(:call) unscoped(&method) else method end end end end end Widget.create!(:name => 'foo') # => #<Widget id: 1, name: "foo", created_at: "2010-11-24 15:05:39", updated_at: "2010-11-24 15:05:39"> ActiveRecord::Base.logger = Logger.new(STDOUT) Widget.all # Widget Load (0.4ms) SELECT "widgets".* FROM "widgets" WHERE (created_at >= '2010-11-24 15:05:10.456719') Widget.all # Widget Load (0.3ms) SELECT "widgets".* FROM "widgets" WHERE (created_at >= '2010-11-24 15:05:16.256514') Widget.all # Widget Load (0.3ms) SELECT "widgets".* FROM "widgets" WHERE (created_at >= '2010-11-24 15:05:17.373986') Widget.all # Widget Load (0.4ms) SELECT "widgets".* FROM "widgets" WHERE (created_at >= '2010-11-24 15:05:20.403725') -
Adam Wróbel January 19th, 2011 @ 07:17 PM
Tim, sorry for late reply. I've tried your example and it indeed worked. But when I switched my project to 3.0.3 gem and applied your patch I was still getting errors. After some digging I've found out that the problem is with models that use multiple calls to default_scope. This is allowed by rails and the exceptions raise when default_scope tries to merge scopes provided with multiple calls.
To see those errors change the model definition in your example to:
class Widget < ActiveRecord::Base default_scope order(:created_at) default_scope lambda { where(['created_at >= ?', 1.minute.ago]) } endThis looks stupid, but something similar happens when you monkey patch foreign models to add your own default_scope in addition to the one they already use.
The patch I've uploaded previously was a diff of ActiveRecord source code. A monkey patch that gets me going in my project is:
module ActiveRecord class Base
class << self def default_scope(options = {}) reset_scoped_methods self.default_scoping << lambda { construct_finder_arel(options.is_a?(Proc) ? options.call : options) } end protected def current_scoped_methods method = scoped_methods.last if method.respond_to?(:call) unscoped(&method) else method end end end
end endCheers
-
-
Adam Wróbel January 20th, 2011 @ 12:29 AM
Actually, I've just found out that my previous patch and all other patches in this ticket were just ignoring the previous default_scope definitions instead of merging them with the current.
I've updated my gist to take previous default_scope declarations into account. This is consistent with how default_scope works for non-lambda arguments. This patch allows you to have declarations like this one:
class Widget < ActiveRecord::Base default_scope order(:created_at) default_scope limit(5) default_scope lambda { where(['created_at >= ?', 10.minute.ago]) } default_scope lambda { where(['created_at <= ?', 1.minute.ago]) } endThe resulting query for Widget.all looks like this:
SELECT "widgets".* FROM "widgets" WHERE (created_at >= '2011-01-19 23:40:11.181327') AND (created_at <= '2011-01-19 23:49:11.181152') ORDER BY created_at LIMIT 5Of course usually you wouldn't put those declarations next to each other, but rather in different monkey patches for the some engine.
A monkey patch for 3.0.x
https://gist.github.com/786674I've also created a pull request at github. Hopefully we'll see this in 3.1:
https://github.com/rails/rails/pull/169 -
-
Adam Wróbel January 20th, 2011 @ 05:27 PM
I was reminded to use respond_to?(:call) instead of kind_of?(Proc) on github. Here's a corrected patch.
-
Tim Morgan January 20th, 2011 @ 08:07 PM
Adam, thanks for your work on this. I had noticed a problem in the console, whenever a model using default_scope would get reloaded, this bug would cause an exception.
I think I worked around it my app with some ugly hack, but your fix looks like the right way to handle this.
I tested the monkeypatch against my 3.0.3 system and it works as advertised. So a +1 on that.
I did NOT test your patch against 3.1 though.
Hopefully we can get @tenderlove back on here so he can commit.
-
Julien Guimont January 22nd, 2011 @ 03:35 PM
Hello,
The monkey patch for 3.0.x from adamwrobel works for most of the cases, but it seems it doesn't scope the validation for uniqueness. You need to add it to each validates.
Any pointers as to why?
Thanks,
Julien. -
Adam Wróbel January 22nd, 2011 @ 04:24 PM
This is unrelated to the lambda support. Uniqueness validator just ignores the default_scope regardless of whether it's lambda or arel finder.
activerecord/lib/active_record/validations/uniqueness.rb, line 24:
relation = finder_class.unscoped.where(sql, *params)If you feel this is a bug you can open a new ticket. But supporting it would be a bit tricky. Uniqueness validator expects column names which it compares with simple = sql operator. Named and default scopes are much more powerful. Beside allowing all the order(), limit(), offset() etc. finders they allow more complex where() conditions.
You might need to add the :scope option explicitly to your validations or if that's not possible you can do something similar to what I did in a project at my company, but I have to warn you - it's a really ugly hack:
https://gist.github.com/791214This fits my project, but you might want to modify this to read scope column names from model's class variables.
-
Aaron Patterson January 22nd, 2011 @ 09:59 PM
- State changed from “committed” to “open”
- Assigned user set to “Aaron Patterson”
reopening and assigning to myself.
-
Ivan Ukhov January 28th, 2011 @ 06:58 PM
Adam Wróbel,
about your patch, try to change the order of the default_scope calls in the model like:
class Widget < ActiveRecord::Base default_scope lambda { where(['created_at >= ?', 10.minute.ago]) } default_scope lambda { where(['created_at <= ?', 1.minute.ago]) } default_scope order(:created_at) default_scope limit(5) endInternally order and limit (as well as where and others) are delegated to scoped that calls current_scope_methods, therefore, the last two calls will calculate lambdas immediately, save the result, and keep appending it to the final query (thanks to construct_finder_arel). If one of the first two default_scope lambdas passes a hash to where like where(:locale => I18n.locale.to_s), the last two scopes will completely overwrite the locale attribute in the final query.
-
Ivan Ukhov January 28th, 2011 @ 07:30 PM
Actually, this is a problem with all defined scopes in the model using scope and new arel syntax. All regular scopes will overwrite default ones.
-
Adam Wróbel January 29th, 2011 @ 02:05 PM
Thanks for finding this issue.
You're right that those latter calls calculate previously defined lambdas and append their results to the final query. A test with where() other than equality condition given in hash:
class Widget < ActiveRecord::Base default_scope lambda { where(['created_at >= ?', 10.minute.ago]) } default_scope order(:created_at) default_scope where(:name => "new") default_scope limit(5) end Widget.all #SELECT "widgets".* FROM "widgets" WHERE #(created_at >= '2011-01-28 19:03:40.789990') AND #(created_at >= '2011-01-28 19:03:40.787447') AND #(created_at >= '2011-01-28 19:03:40.788207') AND #(created_at >= '2011-01-28 19:03:40.788917') ORDER BY created_at LIMIT 5The lambda was called four times - once with each non-lambda default_scope call after it was defined and once during the final fetch. Results of all those calls were merged in the final query.
Test with varying where({}):
class Widget < ActiveRecord::Base default_scope lambda do tmpGlobal ||= "a" $tmpGlobal.next! where(:name => $tmpGlobal.dup ) end default_scope order(:created_at) default_scope limit(5) end Widget.all # Widget Load (0.2ms) SELECT "widgets".* FROM "widgets" WHERE "widgets"."name" = 'c' ORDER BY created_at LIMIT 5 Widget.all # Widget Load (0.2ms) SELECT "widgets".* FROM "widgets" WHERE "widgets"."name" = 'c' ORDER BY created_at LIMIT 5The lambda value that is stored inside the last default_scope call overwrites what is generated later.
Although this is an issue that should be remedied the current implementation is still better than the current state of master branch where one just gets exceptions.
If you have a control over the last calls to default_scope (like when inheriting a class from gem or monkey patching it in your application) then you can invoke your call like this:
default_scope unscoped.limit(5)
This will produce correct results. Of course it's not "the Rails way". I'll try to find a workaround, but from my investigation so far it doesn't look easy. Maybe Aaron can help.
I've got a feeling that if we find a solution to this, then we could also make calls like this work:
Post.unscoped.publishedCurrently it's the same as:
Post.published -
Ivan Ukhov January 29th, 2011 @ 02:20 PM
Please also try with regular scopes + where, order, etc. Again they will just overwrite default scopes. My workaround is a module called DynamicDefaultScoping, I include it in classes where I need dynamic default scopes after all calls to regular default_scope and scope and then user already dynamic default_scope.
In that module I've also implemented the possibility to name default scopes like:
default_scope :language, lambda { |*args| locale = (args[0].presence || I18n.locale).to_s case locale = (args[0].presence || I18n.locale).to_s when 'any', 'all' where :locale => I18n.available_locales.map(&:to_s) else where :locale => locale end }... and the possibility to later overwrite the default value by calling those named scopes. It works with different combinations with other scopes:
Post.recent Post.language(:en).recent Post.recent.language(:en)I can share if somebody's interested.
-
Ivan Ukhov January 30th, 2011 @ 08:05 AM
So, here is the gem: https://github.com/UVSoft/dynamic_default_scoping.
-
Adam Wróbel January 31st, 2011 @ 06:07 AM
- Tag changed from “rails 3.1” to “rails 3.1, default_scope, named_scope, patch”
This problem can also surface when using named scopes.
class Widget < ActiveRecord::Base default_scope lambda { where(['created_at >= ?', 10.minute.ago]) } scope :red, order(:name => "red") endThat call to order will evaluate default scope and save it's current state in :red named scope. The same thing causes the Post.unscoped.published == Post.published absurd. Also calling multiple named scopes like Post.published.recent.by_author("Mike") pollutes the final query with multiple default_scope nodes.
I see three options for working around this:
.1. Change scope and default_scope syntax to require blocks.
Like this:
default_scope { where(:name => :red) } # or default_scope do { :conditions => {...} } end # or scope :published { where( :published => 1 ) }This way we can execute them inside unscoped.scoping {} and apply clean default/named scopes when necessary. Cons: API change; application code is a bit ugly; scope could no longer accept extensions like it does now - through blocks.
.2. Remove definitions from existing default scope from relations passed to default_scope and scope methods.
It's a general idea, but one specific implementation I thought about is: With each created ActiveRecord::Relation keep it's second copy that would be free of definitions from default scope. Let's say that the copy is called r.without_default. Relations returned from default scope would have the without_default cleared to unscoped. Then every scope merge or modification would affect both copies equally. Named scopes would be merged into current relation using their without_default copy.
Cons: rails code gets a bit ugly; we're wasting resources
.3. Keep the default scope separate from the current scope stack and only merge it into the current relation when we're about to hit database.
Cons: also some ugly rails code; not as simple as the above; might brake 3rd party gems that use methods like where_values, order_values, because those wouldn't contain data from default scope.
I've started implementing 3 and then thought 2 would be much quicker. You can find it on my pull request on github. I'm also attaching the patch.
I don't like that code, but it works. Some testing follows:
class Widget < ActiveRecord::Base default_scope lambda { $tmpGlobal ||= "a"; $tmpGlobal.next! where(:name => $tmpGlobal.dup ) } default_scope order(:created_at) default_scope limit(5) scope :red, where(:name => "red") scope :blue, where(:name => "blue") end Widget.all #SELECT "widgets".* FROM "widgets" WHERE "widgets"."name" = 'k' ORDER BY created_at LIMIT 5 Widget.all #SELECT "widgets".* FROM "widgets" WHERE "widgets"."name" = 'm' ORDER BY created_at LIMIT 5 Widget.red #SELECT "widgets".* FROM "widgets" WHERE "widgets"."name" = 'red' ORDER BY created_at LIMIT 5 Widget.unscoped.blue #SELECT "widgets".* FROM "widgets" WHERE "widgets"."name" = 'blue' -
2kan February 2nd, 2011 @ 02:40 PM
I think that Adam Wróbel is correct in his general idea: AR::Relation should be refactored because there are some problems with current code. I think we should find some new way to store and apply default_scopes.
Known issues: this ticket, ticket #6011 (which as I understand difficult to fix in edge without ugly fixes) also we have problems with except (ticket #6290, hey can anybody review it???). Anything else?
I think that the best solution for now is to prevent merging default_scopes with procs. It looks very standoffish for all users and especially for new users to Rails3 and Rails at all when rails raises some internal exception like
undefined methodmerge' for #<Proc:0x108af8058></code> </pre>
orundefined method `includes_values' for #<Proc:0x108af63c0>
I think that it is not acceptable. So right now we can raise an exception (see my patch at https://rails.lighthouseapp.com/projects/8994/tickets/6297, also I will attach it here) and start working on refactoring this code. I think we should find as many issues about AR::Realtion as possible for now to design new great solution. -
Adam Wróbel March 17th, 2011 @ 01:17 PM
I've started a discussion on core with suggestion to change scope syntax. I think the syntax change is certainly a better way of resolving this issue than a hacky work around like the one I've posted here.
http://groups.google.com/group/rubyonrails-core/browse_thread/threa...
-
Jon Leighton April 18th, 2011 @ 11:39 PM
- State changed from “open” to “resolved”
Hi,
I'm closing this ticket because I think the issue is resolved now, see:
https://github.com/rails/rails/commit/8572ae6671c6ec7c2524f327cee82...
If anyone disagrees please shout.
Jon
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
-
2kan
-
Aaron Patterson
-
Arik Fraimovich
-
Brian Mitchell
-
Chris Mear
-
Claudio Poli
-
Diego Rocha
-
Evan
-
Evgeniy Dolzhenko
-
Falk Pauser
-
Gerrit Kaiser
-
Ivan Ukhov
-
Jan Xie
-
Jeremy Kemper
-
Johannes Fahrenkrug
-
Jon Leighton
-
jonnie (at cleverna)
-
Josh N. Abbott
-
Levin Alexander
-
Manfred Stienstra
-
Marcin Michałowski
-
Neeraj Singh
-
Peter Leonhardt
-
Peter Wagenet
-
Piotr Sarnacki
-
runa
-
Rusty Burchfield
-
Ryan Bigg
-
S
-
Selva
-
Steffen Bartsch
-
Stephan Kaag
-
Tanel Suurhans
-
Tim Morgan
-
Tim Trautmann
-
Yaroslav Markin
Attachments
Referenced by
-
6297
[PATCH] Default scopes can't be merged with something that responds_to call
https://rails.lighthouseapp.com/projects/8994/tickets/18...