kotovalexarian-likes-github/rails--rails
1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Code Releases Activity

Use SET CONSTRAINTS for disable_referential_integrity without superuser privileges (take 2)

Browse source 
Re-create https://github.com/rails/rails/pull/21233

eeac6151a5 was reverted (127509c071) because it breaks tests.

----------------

ref: 72c1557254

- We must use `authors` fixture with `author_addresses` because of its foreign key constraint.
- Tests require PostgreSQL >= 9.4.2 because it had a bug about `ALTER CONSTRAINTS` and fixed in 9.4.2.
This commit is contained in:
Fumiaki MATSUSHIMA 2015-12-13 12:46:59 +09:00
parent d96dde82b7
commit 2a129380e8
24 changed files with 205 additions and 112 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
.travis.yml
View file

@ -15,6 +15,9 @@ services:
addons:
postgresql: "9.4"
apt:
packages:
- postgresql-9.4
bundler_args: --without test --jobs 3 --retry 3
before_install:
@ -101,6 +104,12 @@ matrix:
jdk: oraclejdk8
env:
- "GEM=am,amo,aj"
# Test with old (< 9.4.2) postgresql
- rvm: 2.4.0
env:
- "GEM=ar:postgresql"
addons:
postgresql: "9.4"
allow_failures:
- rvm: ruby-head
- rvm: jruby-9.1.8.0

47
activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb
View file

@ -6,8 +6,50 @@ module ActiveRecord
true
end
def disable_referential_integrity # :nodoc:
def disable_referential_integrity(&block) # :nodoc:
if supports_disable_referential_integrity?
if supports_alter_constraint?
disable_referential_integrity_with_alter_constraint(&block)
else
disable_referential_integrity_with_disable_trigger(&block)
end
else
yield
end
end
private
def disable_referential_integrity_with_alter_constraint
tables_constraints = execute(<<-SQL).values
SELECT table_name, constraint_name
FROM information_schema.table_constraints
WHERE constraint_type = 'FOREIGN KEY'
AND is_deferrable = 'NO'
SQL
execute(
tables_constraints.collect { |table, constraint|
"ALTER TABLE #{quote_table_name(table)} ALTER CONSTRAINT #{constraint} DEFERRABLE"
}.join(";")
)
begin
transaction do
execute("SET CONSTRAINTS ALL DEFERRED")
yield
end
ensure
execute(
tables_constraints.collect { |table, constraint|
"ALTER TABLE #{quote_table_name(table)} ALTER CONSTRAINT #{constraint} NOT DEFERRABLE"
}.join(";")
)
end
end
def disable_referential_integrity_with_disable_trigger
original_exception = nil
begin
@ -39,10 +81,7 @@ Rails needs superuser privileges to disable referential integrity.
end
rescue ActiveRecord::ActiveRecordError
end
else
yield
end
end
end
end
end

6
activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
View file

@ -314,6 +314,12 @@ module ActiveRecord
postgresql_version >= 90400
end
def supports_alter_constraint?
# PostgreSQL 9.4 introduces ALTER TABLE ... ALTER CONSTRAINT but it has a bug and fixed in 9.4.2
# https://www.postgresql.org/docs/9.4/static/release-9-4-2.html
postgresql_version >= 90402
end
def get_advisory_lock(lock_id) # :nodoc:
unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63
raise(ArgumentError, "Postgres requires advisory lock ids to be a signed 64 bit integer")

205
activerecord/test/cases/adapters/postgresql/referential_integrity_test.rb
View file

@ -1,111 +1,150 @@
require "cases/helper"
require "support/connection_helper"
class PostgreSQLReferentialIntegrityTest < ActiveRecord::PostgreSQLTestCase
self.use_transactional_tests = false
if ActiveRecord::Base.connection.respond_to?(:supports_alter_constraint?) &&
ActiveRecord::Base.connection.supports_alter_constraint?
class PostgreSQLReferentialIntegrityWithAlterConstraintTest < ActiveRecord::PostgreSQLTestCase
self.use_transactional_tests = false
include ConnectionHelper
include ConnectionHelper
IS_REFERENTIAL_INTEGRITY_SQL = lambda do |sql|
sql.match(/DISABLE TRIGGER ALL/) || sql.match(/ENABLE TRIGGER ALL/)
end
module MissingSuperuserPrivileges
def execute(sql)
if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
super "BROKEN;" rescue nil # put transaction in broken state
raise ActiveRecord::StatementInvalid, "PG::InsufficientPrivilege"
else
super
end
IS_REFERENTIAL_INTEGRITY_SQL = lambda do |sql|
sql.match(/SET CONSTRAINTS ALL DEFERRED/)
end
end
module ProgrammerMistake
def execute(sql)
if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
raise ArgumentError, "something is not right."
else
super
end
end
end
def setup
@connection = ActiveRecord::Base.connection
end
def teardown
reset_connection
if ActiveRecord::Base.connection.is_a?(MissingSuperuserPrivileges)
raise "MissingSuperuserPrivileges patch was not removed"
end
end
def test_should_reraise_invalid_foreign_key_exception_and_show_warning
@connection.extend MissingSuperuserPrivileges
warning = capture(:stderr) do
e = assert_raises(ActiveRecord::InvalidForeignKey) do
@connection.disable_referential_integrity do
raise ActiveRecord::InvalidForeignKey, "Should be re-raised"
module ProgrammerMistake
def execute(sql)
if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
raise ArgumentError, "something is not right."
else
super
end
end
assert_equal "Should be re-raised", e.message
end
assert_match (/WARNING: Rails was not able to disable referential integrity/), warning
assert_match (/cause: PG::InsufficientPrivilege/), warning
def setup
@connection = ActiveRecord::Base.connection
end
def teardown
reset_connection
end
def test_errors_bubble_up
@connection.extend ProgrammerMistake
assert_raises ArgumentError do
@connection.disable_referential_integrity {}
end
end
end
else
class PostgreSQLReferentialIntegrityWithDisableTriggerTest < ActiveRecord::PostgreSQLTestCase
self.use_transactional_tests = false
def test_does_not_print_warning_if_no_invalid_foreign_key_exception_was_raised
@connection.extend MissingSuperuserPrivileges
include ConnectionHelper
warning = capture(:stderr) do
e = assert_raises(ActiveRecord::StatementInvalid) do
@connection.disable_referential_integrity do
raise ActiveRecord::StatementInvalid, "Should be re-raised"
IS_REFERENTIAL_INTEGRITY_SQL = lambda do |sql|
sql.match(/DISABLE TRIGGER ALL/) || sql.match(/ENABLE TRIGGER ALL/)
end
module MissingSuperuserPrivileges
def execute(sql)
if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
super "BROKEN;" rescue nil # put transaction in broken state
raise ActiveRecord::StatementInvalid, "PG::InsufficientPrivilege"
else
super
end
end
assert_equal "Should be re-raised", e.message
end
assert warning.blank?, "expected no warnings but got:\n#{warning}"
end
def test_does_not_break_transactions
@connection.extend MissingSuperuserPrivileges
@connection.transaction do
@connection.disable_referential_integrity do
assert_transaction_is_not_broken
module ProgrammerMistake
def execute(sql)
if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
raise ArgumentError, "something is not right."
else
super
end
end
assert_transaction_is_not_broken
end
end
def test_does_not_break_nested_transactions
@connection.extend MissingSuperuserPrivileges
def setup
@connection = ActiveRecord::Base.connection
end
@connection.transaction do
@connection.transaction(requires_new: true) do
def teardown
reset_connection
if ActiveRecord::Base.connection.is_a?(MissingSuperuserPrivileges)
raise "MissingSuperuserPrivileges patch was not removed"
end
end
def test_should_reraise_invalid_foreign_key_exception_and_show_warning
@connection.extend MissingSuperuserPrivileges
warning = capture(:stderr) do
e = assert_raises(ActiveRecord::InvalidForeignKey) do
@connection.disable_referential_integrity do
raise ActiveRecord::InvalidForeignKey, "Should be re-raised"
end
end
assert_equal "Should be re-raised", e.message
end
assert_match (/WARNING: Rails was not able to disable referential integrity/), warning
assert_match (/cause: PG::InsufficientPrivilege/), warning
end
def test_does_not_print_warning_if_no_invalid_foreign_key_exception_was_raised
@connection.extend MissingSuperuserPrivileges
warning = capture(:stderr) do
e = assert_raises(ActiveRecord::StatementInvalid) do
@connection.disable_referential_integrity do
raise ActiveRecord::StatementInvalid, "Should be re-raised"
end
end
assert_equal "Should be re-raised", e.message
end
assert warning.blank?, "expected no warnings but got:\n#{warning}"
end
def test_does_not_break_transactions
@connection.extend MissingSuperuserPrivileges
@connection.transaction do
@connection.disable_referential_integrity do
assert_transaction_is_not_broken
end
assert_transaction_is_not_broken
end
assert_transaction_is_not_broken
end
def test_does_not_break_nested_transactions
@connection.extend MissingSuperuserPrivileges
@connection.transaction do
@connection.transaction(requires_new: true) do
@connection.disable_referential_integrity do
assert_transaction_is_not_broken
end
end
assert_transaction_is_not_broken
end
end
def test_only_catch_active_record_errors_others_bubble_up
@connection.extend ProgrammerMistake
assert_raises ArgumentError do
@connection.disable_referential_integrity {}
end
end
private
def assert_transaction_is_not_broken
assert_equal 1, @connection.select_value("SELECT 1")
end
end
def test_only_catch_active_record_errors_others_bubble_up
@connection.extend ProgrammerMistake
assert_raises ArgumentError do
@connection.disable_referential_integrity {}
end
end
private
def assert_transaction_is_not_broken
assert_equal 1, @connection.select_value("SELECT 1")
end
end

2
activerecord/test/cases/associations/callbacks_test.rb
View file

@ -7,7 +7,7 @@ require "models/computer"
require "models/company"
class AssociationCallbacksTest < ActiveRecord::TestCase
fixtures :posts, :authors, :projects, :developers
fixtures :posts, :authors, :author_addresses, :projects, :developers
def setup
@david = authors(:david)

2
activerecord/test/cases/associations/cascaded_eager_loading_test.rb
View file

@ -12,7 +12,7 @@ require "models/vertex"
require "models/edge"
class CascadedEagerLoadingTest < ActiveRecord::TestCase
fixtures :authors, :mixins, :companies, :posts, :topics, :accounts, :comments,
fixtures :authors, :author_addresses, :mixins, :companies, :posts, :topics, :accounts, :comments,
:categorizations, :people, :categories, :edges, :vertices
def test_eager_association_loading_with_cascaded_two_levels

6
activerecord/test/cases/associations/has_many_associations_test.rb
View file

@ -40,7 +40,7 @@ require "models/zine"
require "models/interest"
class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase
fixtures :authors, :posts, :comments
fixtures :authors, :author_addresses, :posts, :comments
def test_should_generate_valid_sql
author = authors(:david)
@ -51,7 +51,7 @@ class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCa
end
class HasManyAssociationsTestPrimaryKeys < ActiveRecord::TestCase
fixtures :authors, :essays, :subscribers, :subscriptions, :people
fixtures :authors, :author_addresses, :essays, :subscribers, :subscriptions, :people
def test_custom_primary_key_on_new_record_should_fetch_with_query
subscriber = Subscriber.new(nick: "webster132")
@ -100,7 +100,7 @@ end
class HasManyAssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :categories, :companies, :developers, :projects,
:developers_projects, :topics, :authors, :comments,
:developers_projects, :topics, :authors, :author_addresses, :comments,
:posts, :readers, :taggings, :cars, :jobs, :tags,
:categorizations, :zines, :interests

2
activerecord/test/cases/associations/has_one_through_associations_test.rb
View file

@ -23,7 +23,7 @@ require "models/customer_carrier"
class HasOneThroughAssociationsTest < ActiveRecord::TestCase
fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans,
:dashboards, :speedometers, :authors, :posts, :comments, :categories, :essays, :owners
:dashboards, :speedometers, :authors, :author_addresses, :posts, :comments, :categories, :essays, :owners
def setup
@member = members(:groucho)

2
activerecord/test/cases/associations/inner_join_association_test.rb
View file

@ -10,7 +10,7 @@ require "models/tagging"
require "models/tag"
class InnerJoinAssociationTest < ActiveRecord::TestCase
fixtures :authors, :essays, :posts, :comments, :categories, :categories_posts, :categorizations,
fixtures :authors, :author_addresses, :essays, :posts, :comments, :categories, :categories_posts, :categorizations,
:taggings, :tags
def test_construct_finder_sql_applies_aliases_tables_on_association_conditions

2
activerecord/test/cases/associations/join_model_test.rb
View file

@ -19,7 +19,7 @@ require "models/car"
class AssociationsJoinModelTest < ActiveRecord::TestCase
self.use_transactional_tests = false unless supports_savepoints?
fixtures :posts, :authors, :categories, :categorizations, :comments, :tags, :taggings, :author_favorites, :vertices, :items, :books,
fixtures :posts, :authors, :author_addresses, :categories, :categorizations, :comments, :tags, :taggings, :author_favorites, :vertices, :items, :books,
# Reload edges table from fixtures as otherwise repeated test was failing
:edges

2
activerecord/test/cases/associations/nested_through_associations_test.rb
View file

@ -24,7 +24,7 @@ require "models/membership"
require "models/essay"
class NestedThroughAssociationsTest < ActiveRecord::TestCase
fixtures :authors, :books, :posts, :subscriptions, :subscribers, :tags, :taggings,
fixtures :authors, :author_addresses, :books, :posts, :subscriptions, :subscribers, :tags, :taggings,
:people, :readers, :references, :jobs, :ratings, :comments, :members, :member_details,
:member_types, :sponsors, :clubs, :organizations, :categories, :categories_posts,
:categorizations, :memberships, :essays

4
activerecord/test/cases/associations_test.rb
View file

@ -22,7 +22,7 @@ require "models/interest"
class AssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :companies, :developers, :projects, :developers_projects,
:computers, :people, :readers, :authors, :author_favorites
:computers, :people, :readers, :authors, :author_addresses, :author_favorites
def test_eager_loading_should_not_change_count_of_children
liquid = Liquid.create(name: "salty")
@ -111,7 +111,7 @@ class AssociationsTest < ActiveRecord::TestCase
end
class AssociationProxyTest < ActiveRecord::TestCase
fixtures :authors, :posts, :categorizations, :categories, :developers, :projects, :developers_projects
fixtures :authors, :author_addresses, :posts, :categorizations, :categories, :developers, :projects, :developers_projects
def test_push_does_not_load_target
david = authors(:david)

2
activerecord/test/cases/base_test.rb
View file

@ -64,7 +64,7 @@ class LintTest < ActiveRecord::TestCase
end
class BasicsTest < ActiveRecord::TestCase
fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, "warehouse-things", :authors, :categorizations, :categories, :posts
fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, "warehouse-things", :authors, :author_addresses, :categorizations, :categories, :posts
def test_column_names_are_escaped
conn = ActiveRecord::Base.connection

2
activerecord/test/cases/bind_parameter_test.rb
View file

@ -7,7 +7,7 @@ if ActiveRecord::Base.connection.supports_statement_cache? &&
ActiveRecord::Base.connection.prepared_statements
module ActiveRecord
class BindParameterTest < ActiveRecord::TestCase
fixtures :topics, :authors, :posts
fixtures :topics, :authors, :author_addresses, :posts
class LogListener
attr_accessor :calls

2
activerecord/test/cases/fixtures_test.rb
View file

@ -766,7 +766,7 @@ end
class FasterFixturesTest < ActiveRecord::TestCase
self.use_transactional_tests = false
fixtures :categories, :authors
fixtures :categories, :authors, :author_addresses
def load_extra_fixture(name)
fixture = create_fixtures(name).first

2
activerecord/test/cases/json_serialization_test.rb
View file

@ -168,7 +168,7 @@ class JsonSerializationTest < ActiveRecord::TestCase
end
class DatabaseConnectedJsonEncodingTest < ActiveRecord::TestCase
fixtures :authors, :posts, :comments, :tags, :taggings
fixtures :authors, :author_addresses, :posts, :comments, :tags, :taggings
include JsonSerializationHelpers

2
activerecord/test/cases/readonly_test.rb
View file

@ -10,7 +10,7 @@ require "models/person"
require "models/ship"
class ReadOnlyTest < ActiveRecord::TestCase
fixtures :authors, :posts, :comments, :developers, :projects, :developers_projects, :people, :readers
fixtures :authors, :author_addresses, :posts, :comments, :developers, :projects, :developers_projects, :people, :readers
def test_cant_save_readonly_record
dev = Developer.find(1)

4
activerecord/test/cases/relation/merging_test.rb
View file

@ -8,7 +8,7 @@ require "models/project"
require "models/rating"
class RelationMergingTest < ActiveRecord::TestCase
fixtures :developers, :comments, :authors, :posts
fixtures :developers, :comments, :authors, :author_addresses, :posts
def test_relation_merging
devs = Developer.where("salary >= 80000").merge(Developer.limit(2)).merge(Developer.order("id ASC").where("id < 3"))
@ -114,7 +114,7 @@ class RelationMergingTest < ActiveRecord::TestCase
end
class MergingDifferentRelationsTest < ActiveRecord::TestCase
fixtures :posts, :authors, :developers
fixtures :posts, :authors, :author_addresses, :developers
test "merging where relations" do
hello_by_bob = Post.where(body: "hello").joins(:author).

2
activerecord/test/cases/relation/where_test.rb
View file

@ -15,7 +15,7 @@ require "models/vertex"
module ActiveRecord
class WhereTest < ActiveRecord::TestCase
fixtures :posts, :edges, :authors, :binaries, :essays, :cars, :treasures, :price_estimates
fixtures :posts, :edges, :authors, :author_addresses, :binaries, :essays, :cars, :treasures, :price_estimates
def test_where_copies_bind_params
author = authors(:david)

2
activerecord/test/cases/relation_test.rb
View file

@ -6,7 +6,7 @@ require "models/rating"
module ActiveRecord
class RelationTest < ActiveRecord::TestCase
fixtures :posts, :comments, :authors
fixtures :posts, :comments, :authors, :author_addresses
FakeKlass = Struct.new(:table_name, :name) do
extend ActiveRecord::Delegation::DelegateCache

2
activerecord/test/cases/relations_test.rb
View file

@ -22,7 +22,7 @@ require "models/categorization"
require "models/edge"
class RelationTest < ActiveRecord::TestCase
fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments,
fixtures :authors, :author_addresses, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments,
:tags, :taggings, :cars, :minivans
class TopicWithCallbacks < ActiveRecord::Base

4
activerecord/test/cases/scoping/relation_scoping_test.rb
View file

@ -10,7 +10,7 @@ require "models/person"
require "models/reference"
class RelationScopingTest < ActiveRecord::TestCase
fixtures :authors, :developers, :projects, :comments, :posts, :developers_projects
fixtures :authors, :author_addresses, :developers, :projects, :comments, :posts, :developers_projects
setup do
developers(:david)
@ -238,7 +238,7 @@ class RelationScopingTest < ActiveRecord::TestCase
end
class NestedRelationScopingTest < ActiveRecord::TestCase
fixtures :authors, :developers, :projects, :comments, :posts
fixtures :authors, :author_addresses, :developers, :projects, :comments, :posts
def test_merge_options
Developer.where("salary = 80000").scoping do

2
activerecord/test/cases/transactions_test.rb
View file

@ -10,7 +10,7 @@ require "models/movie"
class TransactionTest < ActiveRecord::TestCase
self.use_transactional_tests = false
fixtures :topics, :developers, :authors, :posts
fixtures :topics, :developers, :authors, :author_addresses, :posts
def setup
@first, @second = Topic.find(1, 2).sort_by(&:id)

2
activerecord/test/cases/yaml_serialization_test.rb
View file

@ -5,7 +5,7 @@ require "models/post"
require "models/author"
class YamlSerializationTest < ActiveRecord::TestCase
fixtures :topics, :authors, :posts
fixtures :topics, :authors, :author_addresses, :posts
def test_to_yaml_with_time_with_zone_should_not_raise_exception
with_timezone_config aware_attributes: true, zone: "Pacific Time (US & Canada)" do