diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 0ecbcdefc5..25fe6b2542 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,17 @@ +* Add option to run `default_scope` on all queries. + + Previously, a `default_scope` would only run on select or insert queries. In some cases, like non-Rails tenant sharding solutions, it may be desirable to run `default_scope` on all queries in order to ensure queries are including a foreign key for the shard (ie `blog_id`). + + Now applications can add an option to run on all queries including select, insert, delete, and update by adding an `all_queries` option to the default scope definition. + + ```ruby + class Article < ApplicationRecord + default_scope -> { where(blog_id: Current.blog.id) }, all_queries: true + end + ``` + + *Eileen M. Uchitelle* + * Add `where.associated` to check for the presence of an association. ```ruby diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 011b7359fe..2a22284bff 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -378,6 +378,10 @@ module ActiveRecord def _update_record(values, constraints) # :nodoc: constraints = _substitute_values(constraints).map { |attr, bind| attr.eq(bind) } + if default_scopes.any? && !default_scoped(all_queries: true).where_clause.empty? + constraints << default_scoped(all_queries: true).where_clause.ast + end + um = arel_table.where( constraints.reduce(&:and) ).compile_update(_substitute_values(values), primary_key) @@ -388,6 +392,10 @@ module ActiveRecord def _delete_record(constraints) # :nodoc: constraints = _substitute_values(constraints).map { |attr, bind| attr.eq(bind) } + if default_scopes.any? && !default_scoped(all_queries: true).where_clause.empty? + constraints << default_scoped(all_queries: true).where_clause.ast + end + dm = Arel::DeleteManager.new dm.from(arel_table) dm.wheres = constraints diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index 151eef362b..e3faf1513d 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -2,6 +2,15 @@ module ActiveRecord module Scoping + class DefaultScope # :nodoc: + attr_reader :scope, :all_queries + + def initialize(scope, all_queries = nil) + @scope = scope + @all_queries = all_queries + end + end + module Default extend ActiveSupport::Concern @@ -54,11 +63,26 @@ module ActiveRecord # Article.all # => SELECT * FROM articles WHERE published = true # # The #default_scope is also applied while creating/building a record. - # It is not applied while updating a record. + # It is not applied while updating or deleting a record. # # Article.new.published # => true # Article.create.published # => true # + # To apply a #default_scope when updating or deleting a record, add + # `all_queries: true`: + # + # class Article < ActiveRecord::Base + # default_scope { where(blog_id: 1) }, all_queries: true + # end + # + # Applying a default scope to all queries will ensure that records + # are always queried by the additional conditions. Note that only + # where clauses apply, as it does not make sense to add order to + # queries that return a single object by primary key. + # + # Article.find(1).destroy + # => DELETE ... FROM `articles` where ID = 1 AND blog_id = 1; + # # (You can also pass any object which responds to +call+ to the # +default_scope+ macro, and it will be called when building the # default scope.) @@ -85,7 +109,7 @@ module ActiveRecord # # Should return a scope, you can call 'super' here etc. # end # end - def default_scope(scope = nil, &block) # :doc: + def default_scope(scope = nil, all_queries: nil, &block) # :doc: scope = block if block_given? if scope.is_a?(Relation) || !scope.respond_to?(:call) @@ -96,10 +120,12 @@ module ActiveRecord "self.default_scope.)" end - self.default_scopes += [scope] + default_scope = DefaultScope.new(scope, all_queries) + + self.default_scopes += [default_scope] end - def build_default_scope(relation = relation()) + def build_default_scope(relation = relation(), all_queries: nil) return if abstract_class? if default_scope_override.nil? @@ -115,14 +141,26 @@ module ActiveRecord end elsif default_scopes.any? evaluate_default_scope do - default_scopes.inject(relation) do |default_scope, scope| - scope = scope.respond_to?(:to_proc) ? scope : scope.method(:call) - default_scope.instance_exec(&scope) || default_scope + default_scopes.inject(relation) do |default_scope, scope_obj| + if execute_scope?(all_queries, scope_obj) + scope = scope_obj.scope.respond_to?(:to_proc) ? scope_obj.scope : scope_obj.scope.method(:call) + + default_scope.instance_exec(&scope) || default_scope + end end end end end + # If all_queries is nil, only execute on select and insert queries. + # + # If all_queries is true, check if the default_scope object has + # all_queries set, then execute on all queries; select, insert, update + # and delete. + def execute_scope?(all_queries, default_scope_obj) + all_queries.nil? || all_queries && default_scope_obj.all_queries + end + def ignore_default_scope? ScopeRegistry.value_for(:ignore_default_scope, base_class) end diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index ea1c1d3e76..bdf33d8310 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -42,8 +42,8 @@ module ActiveRecord end # Returns a scope for the model with default scopes. - def default_scoped(scope = relation) - build_default_scope(scope) || scope + def default_scoped(scope = relation, all_queries: nil) + build_default_scope(scope, all_queries: all_queries) || scope end def default_extensions # :nodoc: diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index 6b54c98736..c763a37c53 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -378,7 +378,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase prev_default_scope = Club.default_scopes [:includes, :preload, :joins, :eager_load].each do |q| - Club.default_scopes = [proc { Club.public_send(q, :category) }] + Club.default_scopes = [ActiveRecord::Scoping::DefaultScope.new(proc { Club.public_send(q, :category) })] assert_equal categories(:general), members(:groucho).reload.club_category end ensure diff --git a/activerecord/test/cases/dup_test.rb b/activerecord/test/cases/dup_test.rb index 192f391227..ed10d7bd35 100644 --- a/activerecord/test/cases/dup_test.rb +++ b/activerecord/test/cases/dup_test.rb @@ -145,7 +145,7 @@ module ActiveRecord def test_dup_with_default_scope prev_default_scopes = Topic.default_scopes - Topic.default_scopes = [proc { Topic.where(approved: true) }] + Topic.default_scopes = [ActiveRecord::Scoping::DefaultScope.new(proc { Topic.where(approved: true) })] topic = Topic.new(approved: false) assert_not topic.dup.approved?, "should not be overridden by default scopes" ensure diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index 5e9f97cb3a..77ee745204 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -7,6 +7,7 @@ require "models/developer" require "models/project" require "models/computer" require "models/cat" +require "models/mentor" require "concurrent/atomic/cyclic_barrier" class DefaultScopingTest < ActiveRecord::TestCase @@ -79,6 +80,84 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal 50000, wheres["salary"] end + def test_default_scope_runs_on_create + Mentor.create! + create_sql = capture_sql { DeveloperwithDefaultMentorScopeNot.create!(name: "Eileen") }.first + + assert_match(/mentor_id/, create_sql) + end + + def test_default_scope_with_all_queries_runs_on_create + Mentor.create! + create_sql = capture_sql { DeveloperWithDefaultMentorScopeAllQueries.create!(name: "Eileen") }.first + + assert_match(/mentor_id/, create_sql) + end + + def test_default_scope_runs_on_select + Mentor.create! + DeveloperwithDefaultMentorScopeNot.create!(name: "Eileen") + select_sql = capture_sql { DeveloperwithDefaultMentorScopeNot.find_by(name: "Eileen") }.first + + assert_match(/mentor_id/, select_sql) + end + + def test_default_scope_with_all_queries_runs_on_select + Mentor.create! + DeveloperWithDefaultMentorScopeAllQueries.create!(name: "Eileen") + select_sql = capture_sql { DeveloperWithDefaultMentorScopeAllQueries.find_by(name: "Eileen") }.first + + assert_match(/mentor_id/, select_sql) + end + + def test_default_scope_doesnt_run_on_update + Mentor.create! + dev = DeveloperwithDefaultMentorScopeNot.create!(name: "Eileen") + update_sql = capture_sql { dev.update!(name: "Not Eileen") }.first + + assert_no_match(/mentor_id/, update_sql) + end + + def test_default_scope_with_all_queries_runs_on_update + Mentor.create! + dev = DeveloperWithDefaultMentorScopeAllQueries.create!(name: "Eileen") + update_sql = capture_sql { dev.update!(name: "Not Eileen") }.first + + assert_match(/mentor_id/, update_sql) + end + + def test_default_scope_doesnt_run_on_update_columns + Mentor.create! + dev = DeveloperwithDefaultMentorScopeNot.create!(name: "Eileen") + update_sql = capture_sql { dev.update_columns(name: "Not Eileen") }.first + + assert_no_match(/mentor_id/, update_sql) + end + + def test_default_scope_with_all_queries_runs_on_update_columns + Mentor.create! + dev = DeveloperWithDefaultMentorScopeAllQueries.create!(name: "Eileen") + update_sql = capture_sql { dev.update_columns(name: "Not Eileen") }.first + + assert_match(/mentor_id/, update_sql) + end + + def test_default_scope_doesnt_run_on_destroy + Mentor.create! + dev = DeveloperwithDefaultMentorScopeNot.create!(name: "Eileen") + destroy_sql = capture_sql { dev.destroy }.first + + assert_no_match(/mentor_id/, destroy_sql) + end + + def test_default_scope_with_all_queries_runs_on_destroy + Mentor.create! + dev = DeveloperWithDefaultMentorScopeAllQueries.create!(name: "Eileen") + destroy_sql = capture_sql { dev.destroy }.first + + assert_match(/mentor_id/, destroy_sql) + end + def test_scope_overwrites_default expected = Developer.all.merge!(order: "salary DESC, name DESC").to_a.collect(&:name) received = DeveloperOrderedBySalary.by_name.to_a.collect(&:name) diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index d239b629bd..f4c051e691 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -140,6 +140,16 @@ class DeveloperWithSelect < ActiveRecord::Base default_scope { select("name") } end +class DeveloperwithDefaultMentorScopeNot < ActiveRecord::Base + self.table_name = "developers" + default_scope -> { where(mentor_id: 1) } +end + +class DeveloperWithDefaultMentorScopeAllQueries < ActiveRecord::Base + self.table_name = "developers" + default_scope -> { where(mentor_id: 1) }, all_queries: true +end + class DeveloperWithIncludes < ActiveRecord::Base self.table_name = "developers" has_many :audit_logs, foreign_key: :developer_id