diff options
author | Eileen M. Uchitelle <eileencodes@users.noreply.github.com> | 2018-11-30 10:06:02 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-11-30 10:06:02 -0500 |
commit | f6e1061c915a92389fee2819d29b13764942c985 (patch) | |
tree | 30f91b6801492c0e040b76ee330f53b3e4b957e4 /activerecord | |
parent | 5c6316dbb88075e27169f49a22e59357efd1a967 (diff) | |
parent | f39d72d5267baed1000932831cda98503d1e1047 (diff) | |
download | rails-f6e1061c915a92389fee2819d29b13764942c985.tar.gz rails-f6e1061c915a92389fee2819d29b13764942c985.tar.bz2 rails-f6e1061c915a92389fee2819d29b13764942c985.zip |
Merge pull request #34505 from eileencodes/add-readonly-mode
Add ability to block writes to a database
Diffstat (limited to 'activerecord')
10 files changed, 286 insertions, 1 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 5b07d63684..d9488bd0f0 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,19 @@ +* Add the ability to prevent writes to a database for the duration of a block. + + Allows the application to prevent writes to a database. This can be useful when + you're building out multiple databases and want to make sure you're not sending + writes when you want a read. + + If `while_preventing_writes` is called and the query is considered a write + query the database will raise an exception regardless of whether the database + user is able to write. + + This is not meant to be a catch-all for write queries but rather a way to enforce + read-only queries without opening a second connection. One purpose of this is to + catch accidental writes, not all writes. + + *Eileen M. Uchitelle* + * Allow aliased attributes to be used in `#update_columns` and `#update`. *Gannon McGibbon* diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 0059f0b773..2299fc0214 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -98,6 +98,11 @@ module ActiveRecord exec_query(sql, name).rows end + # Determines whether the SQL statement is a write query. + def write_query?(sql) + raise NotImplementedError + end + # Executes the SQL statement in the context of this connection and returns # the raw result from the connection adapter. # Note: depending on your database connector, the result returned by this diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index f73369ceef..31a994292e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -76,7 +76,7 @@ module ActiveRecord SIMPLE_INT = /\A\d+\z/ - attr_accessor :visitor, :pool + attr_accessor :visitor, :pool, :prevent_writes attr_reader :schema_cache, :owner, :logger, :prepared_statements, :lock alias :in_use? :owner @@ -100,6 +100,13 @@ module ActiveRecord end end + def self.build_read_query_regexp(*parts) # :nodoc: + lambda do |*parts| + parts = parts.map { |part| /\A\s*#{part}/i } + Regexp.union(*parts) + end + end + def initialize(connection, logger = nil, config = {}) # :nodoc: super() @@ -133,6 +140,27 @@ module ActiveRecord @config[:replica] || false end + # Determines whether writes are currently being prevents. + # + # Returns true if the connection is a replica, or if +prevent_writes+ + # is set to true. + def preventing_writes? + replica? || prevent_writes + end + + # Prevent writing to the database regardless of role. + # + # In some cases you may want to prevent writes to the database + # even if you are on a database that can write. `while_preventing_writes` + # will prevent writes to the database for the duration of the block. + def while_preventing_writes + original = self.prevent_writes + self.prevent_writes = true + yield + ensure + self.prevent_writes = original + end + def migrations_paths # :nodoc: @config[:migrations_paths] || Migrator.migrations_paths end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb index 9ea237dd81..0ec9581c42 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb @@ -19,8 +19,18 @@ module ActiveRecord execute(sql, name).to_a end + READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp.call(:begin, :select, :set, :show, :release, :savepoint) # :nodoc: + + def write_query?(sql) # :nodoc: + !READ_QUERY.match?(sql) + end + # Executes the SQL statement in the context of this connection. def execute(sql, name = nil) + if preventing_writes? && write_query?(sql) + raise ActiveRecord::StatementInvalid, "Write query attempted while in readonly mode: #{sql}" + end + # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been # made since we established the connection @connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 6bd6b67165..c8bc339e61 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -67,11 +67,21 @@ module ActiveRecord end end + READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp.call(:select, :show, :set) # :nodoc: + + def write_query?(sql) # :nodoc: + !READ_QUERY.match?(sql) + end + # Executes an SQL statement, returning a PG::Result object on success # or raising a PG::Error exception otherwise. # Note: the PG::Result object is manually memory managed; if you don't # need it specifically, you may want consider the <tt>exec_query</tt> wrapper. def execute(sql, name = nil) + if preventing_writes? && write_query?(sql) + raise ActiveRecord::StatementInvalid, "Write query attempted while in readonly mode: #{sql}" + end + materialize_transactions log(sql, name) do diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index b41bf2fc66..1bec8fbabd 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -209,6 +209,12 @@ module ActiveRecord # DATABASE STATEMENTS ====================================== #++ + READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp.call(:select) # :nodoc: + + def write_query?(sql) # :nodoc: + !READ_QUERY.match?(sql) + end + def explain(arel, binds = []) sql = "EXPLAIN QUERY PLAN #{to_sql(arel, binds)}" SQLite3::ExplainPrettyPrinter.new.pp(exec_query(sql, "EXPLAIN", [])) @@ -257,6 +263,10 @@ module ActiveRecord end def execute(sql, name = nil) #:nodoc: + if preventing_writes? && write_query?(sql) + raise ActiveRecord::StatementInvalid, "Write query attempted while in readonly mode: #{sql}" + end + materialize_transactions log(sql, name) do diff --git a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb index 0719baaa23..bddcd309cd 100644 --- a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb @@ -69,6 +69,64 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase @conn.exec_query("ALTER TABLE engines DROP COLUMN old_car_id") end + def test_errors_when_an_insert_query_is_called_while_preventing_writes + assert_raises(ActiveRecord::StatementInvalid) do + @conn.while_preventing_writes do + @conn.insert("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')") + end + end + end + + def test_errors_when_an_update_query_is_called_while_preventing_writes + @conn.insert("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')") + + assert_raises(ActiveRecord::StatementInvalid) do + @conn.while_preventing_writes do + @conn.update("UPDATE `engines` SET `engines`.`car_id` = '9989' WHERE `engines`.`car_id` = '138853948594'") + end + end + end + + def test_errors_when_a_delete_query_is_called_while_preventing_writes + @conn.execute("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')") + + assert_raises(ActiveRecord::StatementInvalid) do + @conn.while_preventing_writes do + @conn.execute("DELETE FROM `engines` where `engines`.`car_id` = '138853948594'") + end + end + end + + def test_errors_when_a_replace_query_is_called_while_preventing_writes + @conn.execute("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')") + + assert_raises(ActiveRecord::StatementInvalid) do + @conn.while_preventing_writes do + @conn.execute("REPLACE INTO `engines` SET `engines`.`car_id` = '249823948'") + end + end + end + + def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes + @conn.execute("INSERT INTO `engines` (`car_id`) VALUES ('138853948594')") + + @conn.while_preventing_writes do + assert_equal 1, @conn.execute("SELECT `engines`.* FROM `engines` WHERE `engines`.`car_id` = '138853948594'").entries.count + end + end + + def test_doesnt_error_when_a_show_query_is_called_while_preventing_writes + @conn.while_preventing_writes do + assert_equal 2, @conn.execute("SHOW FULL FIELDS FROM `engines`").entries.count + end + end + + def test_doesnt_error_when_a_set_query_is_called_while_preventing_writes + @conn.while_preventing_writes do + assert_nil @conn.execute("SET NAMES utf8") + end + end + private def with_example_table(definition = "id int auto_increment primary key, number int, data varchar(255)", &block) diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index cbb6cd42b5..59ddb8457e 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -376,6 +376,62 @@ module ActiveRecord end end + def test_errors_when_an_insert_query_is_called_while_preventing_writes + with_example_table do + assert_raises(ActiveRecord::StatementInvalid) do + @connection.while_preventing_writes do + @connection.execute("INSERT INTO ex (data) VALUES ('138853948594')") + end + end + end + end + + def test_errors_when_an_update_query_is_called_while_preventing_writes + with_example_table do + @connection.execute("INSERT INTO ex (data) VALUES ('138853948594')") + + assert_raises(ActiveRecord::StatementInvalid) do + @connection.while_preventing_writes do + @connection.execute("UPDATE ex SET data = '9989' WHERE data = '138853948594'") + end + end + end + end + + def test_errors_when_a_delete_query_is_called_while_preventing_writes + with_example_table do + @connection.execute("INSERT INTO ex (data) VALUES ('138853948594')") + + assert_raises(ActiveRecord::StatementInvalid) do + @connection.while_preventing_writes do + @connection.execute("DELETE FROM ex where data = '138853948594'") + end + end + end + end + + def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes + with_example_table do + @connection.execute("INSERT INTO ex (data) VALUES ('138853948594')") + + @connection.while_preventing_writes do + assert_equal 1, @connection.execute("SELECT * FROM ex WHERE data = '138853948594'").entries.count + end + end + end + + def test_doesnt_error_when_a_show_query_is_called_while_preventing_writes + @connection.while_preventing_writes do + assert_equal 1, @connection.execute("SHOW TIME ZONE").entries.count + end + end + + def test_doesnt_error_when_a_set_query_is_called_while_preventing_writes + @connection.while_preventing_writes do + assert_equal [], @connection.execute("SET standard_conforming_strings = on").entries + end + end + private def with_example_table(definition = "id serial primary key, number integer, data character varying(255)", &block) diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 8536dcf183..3b7a28b757 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -573,6 +573,62 @@ module ActiveRecord end end + def test_errors_when_an_insert_query_is_called_while_preventing_writes + with_example_table "id int, data string" do + assert_raises(ActiveRecord::StatementInvalid) do + @conn.while_preventing_writes do + @conn.execute("INSERT INTO ex (data) VALUES ('138853948594')") + end + end + end + end + + def test_errors_when_an_update_query_is_called_while_preventing_writes + with_example_table "id int, data string" do + @conn.execute("INSERT INTO ex (data) VALUES ('138853948594')") + + assert_raises(ActiveRecord::StatementInvalid) do + @conn.while_preventing_writes do + @conn.execute("UPDATE ex SET data = '9989' WHERE data = '138853948594'") + end + end + end + end + + def test_errors_when_a_delete_query_is_called_while_preventing_writes + with_example_table "id int, data string" do + @conn.execute("INSERT INTO ex (data) VALUES ('138853948594')") + + assert_raises(ActiveRecord::StatementInvalid) do + @conn.while_preventing_writes do + @conn.execute("DELETE FROM ex where data = '138853948594'") + end + end + end + end + + def test_errors_when_a_replace_query_is_called_while_preventing_writes + with_example_table "id int, data string" do + @conn.execute("INSERT INTO ex (data) VALUES ('138853948594')") + + assert_raises(ActiveRecord::StatementInvalid) do + @conn.while_preventing_writes do + @conn.execute("REPLACE INTO ex (data) VALUES ('249823948')") + end + end + end + end + + def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes + with_example_table "id int, data string" do + @conn.execute("INSERT INTO ex (data) VALUES ('138853948594')") + + @conn.while_preventing_writes do + assert_equal 1, @conn.execute("SELECT data from ex WHERE data = '138853948594'").count + end + end + end + private def assert_logged(logs) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 63a73101c4..6c7f3a9c56 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1488,4 +1488,40 @@ class BasicsTest < ActiveRecord::TestCase ensure ActiveRecord::Base.protected_environments = previous_protected_environments end + + test "creating a record raises if preventing writes" do + assert_raises ActiveRecord::StatementInvalid do + ActiveRecord::Base.connection.while_preventing_writes do + bird = Bird.create! name: "Bluejay" + end + end + end + + test "updating a record raises if preventing writes" do + bird = Bird.create! name: "Bluejay" + + assert_raises ActiveRecord::StatementInvalid do + ActiveRecord::Base.connection.while_preventing_writes do + bird.update! name: "Robin" + end + end + end + + test "deleting a record raises if preventing writes" do + bird = Bird.create! name: "Bluejay" + + assert_raises ActiveRecord::StatementInvalid do + ActiveRecord::Base.connection.while_preventing_writes do + bird.destroy! + end + end + end + + test "selecting a record does not raise if preventing writes" do + bird = Bird.create! name: "Bluejay" + + ActiveRecord::Base.connection.while_preventing_writes do + assert_equal bird, Bird.where(name: "Bluejay").first + end + end end |