default_scope can't take procs

+1 on this. i was definitely surprised by this behavior

+1

+1

+1

I've found that this would be very useful for simple model i18n like in:

default_scope :ordered, lambda { {:order => column_translated(:name)} }

Er, sorry. I meant:

default_scope lambda { {:order => column_translated(:name)} }

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.

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).

Attached is a patch that adds support for setting a block for default scope as such:

default_scope do
  { :conditions => ["created_at < ?", Time.now] }
end

Other 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_age

will 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.

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

Also, if you want to use this functionality in a Rails 2.3 app without patching, check out http://gist.github.com/81187

More minor bug fixes.

I've applied Peters patch and it works for me, thanks.

Hey Peter,

Any reason to go with block instead of procs ? Why not the following :

default_scope proc { { :conditions => ["created_at < ?", Time.now] } }

Pratik,

I thought blocks looked nicer, but using procs would be more consistent with named_scope.

Yeah. I think it should be consistent with named_scopes. Rest looks nice. Thanks for working on this !

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 * FROM jobs LIMIT 1
DEBUG      Employer Load (0.4ms)   SELECT * FROM employers WHERE (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)

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).

Brian: could you give me an example? I would love to see a simpler solution!

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.

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.

@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.

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) }
  end

Not 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.

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
    end

The 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

I rebased my fork, so the commit is now: http://github.com/seven1m/rails/commit/282f5bb6a6dd89d810e67f7bb864...

Can someone take a look at this?

Looks good to me.

I would replace "and" with "&&" in line if m = scoped_methods.last and m.is_a?(Proc).

Replacing and with && would cause m to = true inside the conditional. "and" has lower operator precedence than = in Ruby.

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
    end

I dislike assignment in conditional tests, and I think you can use the & operator to pass the block directly to unscoped. (Or did you discover some reason why this shouldn't be done?)

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.

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

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.

Here is the updated commit (updated patch is attached):

http://github.com/seven1m/rails/commit/e0b73b64c30e81e5b4279ce55dd2...

Thanks!

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

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

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_scope

I 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 :/

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!

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.

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.

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.

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')

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]) }
end

This 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
end

Cheers

Problems with code formatting?

https://gist.github.com/786674

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]) }
end

The 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 5

Of 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/786674

I've also created a pull request at github. Hopefully we'll see this in 3.1:
https://github.com/rails/rails/pull/169

Patch for 3.1 for those who prefer it over github fork.

I was reminded to use respond_to?(:call) instead of kind_of?(Proc) on github. Here's a corrected patch.

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.

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.

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/791214

This fits my project, but you might want to modify this to read scope column names from model's class variables.

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)
end

Internally 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.

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.

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 5

The 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 5

The 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.published

Currently it's the same as:
Post.published

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.

So, here is the gem: https://github.com/UVSoft/dynamic_default_scoping.

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 method merge' for #&lt;Proc:0x108af8058&gt;</code>
</pre>



or 

undefined 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.

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...