diff options
44 files changed, 915 insertions, 176 deletions
@@ -52,7 +52,7 @@ end gem "dalli", ">= 2.2.1" gem "listen", ">= 3.0.5", "< 3.2", require: false gem "libxml-ruby", platforms: :ruby -gem "connection_pool" +gem "connection_pool", require: false # for railties app_generator_test gem "bootsnap", ">= 1.1.0", require: false diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 2836f0cfbc..04bbae8495 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,9 @@ +* Bring back proc with arity of 1 in `ActionMailer::Base.default` proc + since it was supported in Rails 5.0 but not deprecated. + + *Jimmy Bourassa* + + ## Rails 5.2.0.beta2 (November 28, 2017) ## * No changes. diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index eb8ae59533..3af95081ee 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -889,7 +889,7 @@ module ActionMailer default_values = self.class.default.map do |key, value| [ key, - value.is_a?(Proc) ? instance_exec(&value) : value + compute_default(value) ] end.to_h @@ -898,6 +898,16 @@ module ActionMailer headers_with_defaults end + def compute_default(value) + return value unless value.is_a?(Proc) + + if value.arity == 1 + instance_exec(self, &value) + else + instance_exec(&value) + end + end + def assign_headers_to_message(message, headers) assignable = headers.except(:parts_order, :content_type, :body, :template_name, :template_path, :delivery_method, :delivery_method_options) diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 977e0e201e..ac90809518 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -725,6 +725,15 @@ class BaseTest < ActiveSupport::TestCase assert(ProcMailer.welcome["x-has-to-proc"].to_s == "symbol") end + test "proc default values can have arity of 1 where arg is a mailer instance" do + assert_equal(ProcMailer.welcome["X-Lambda-Arity-1-arg"].to_s, "complex_value") + assert_equal(ProcMailer.welcome["X-Lambda-Arity-1-self"].to_s, "complex_value") + end + + test "proc default values with fixed arity of 0 can be called" do + assert_equal("0", ProcMailer.welcome["X-Lambda-Arity-0"].to_s) + end + test "we can call other defined methods on the class as needed" do mail = ProcMailer.welcome assert_equal("Thanks for signing up this afternoon", mail.subject) diff --git a/actionmailer/test/mailers/proc_mailer.rb b/actionmailer/test/mailers/proc_mailer.rb index b7cf53eb4a..76e730bb79 100644 --- a/actionmailer/test/mailers/proc_mailer.rb +++ b/actionmailer/test/mailers/proc_mailer.rb @@ -4,12 +4,19 @@ class ProcMailer < ActionMailer::Base default to: "system@test.lindsaar.net", "X-Proc-Method" => Proc.new { Time.now.to_i.to_s }, subject: Proc.new { give_a_greeting }, - "x-has-to-proc" => :symbol + "x-has-to-proc" => :symbol, + "X-Lambda-Arity-0" => ->() { "0" }, + "X-Lambda-Arity-1-arg" => ->(arg) { arg.computed_value }, + "X-Lambda-Arity-1-self" => ->(_) { self.computed_value } def welcome mail end + def computed_value + "complex_value" + end + private def give_a_greeting diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 4453f845f4..ff8375e690 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,3 +1,20 @@ +* Allow block to be passed to `ActiveJob::Base.discard_on` to allow custom handling of discard jobs. + + Example: + + class RemoteServiceJob < ActiveJob::Base + discard_on(CustomAppException) do |job, exception| + ExceptionNotifier.caught(exception) + end + + def perform(*args) + # Might raise CustomAppException for something domain specific + end + end + + *Aidan Haran* + + ## Rails 5.2.0.beta2 (November 28, 2017) ## * No changes. diff --git a/activejob/lib/active_job/exceptions.rb b/activejob/lib/active_job/exceptions.rb index 8b4a88ba6a..ae700848d0 100644 --- a/activejob/lib/active_job/exceptions.rb +++ b/activejob/lib/active_job/exceptions.rb @@ -61,18 +61,28 @@ module ActiveJob # Discard the job with no attempts to retry, if the exception is raised. This is useful when the subject of the job, # like an Active Record, is no longer available, and the job is thus no longer relevant. # + # You can also pass a block that'll be invoked. This block is yielded with the job instance as the first and the error instance as the second parameter. + # # ==== Example # # class SearchIndexingJob < ActiveJob::Base # discard_on ActiveJob::DeserializationError + # discard_on(CustomAppException) do |job, exception| + # ExceptionNotifier.caught(exception) + # end # # def perform(record) # # Will raise ActiveJob::DeserializationError if the record can't be deserialized + # # Might raise CustomAppException for something domain specific # end # end def discard_on(exception) rescue_from exception do |error| - logger.error "Discarded #{self.class} due to a #{exception}. The original exception was #{error.cause.inspect}." + if block_given? + yield self, exception + else + logger.error "Discarded #{self.class} due to a #{exception}. The original exception was #{error.cause.inspect}." + end end end end diff --git a/activejob/test/cases/exceptions_test.rb b/activejob/test/cases/exceptions_test.rb index 22fed0a808..bc33d79f61 100644 --- a/activejob/test/cases/exceptions_test.rb +++ b/activejob/test/cases/exceptions_test.rb @@ -58,6 +58,13 @@ class ExceptionsTest < ActiveJob::TestCase end end + test "custom handling of discarded job" do + perform_enqueued_jobs do + RetryJob.perform_later "CustomDiscardableError", 2 + assert_equal "Dealt with a job that was discarded in a custom way", JobBuffer.last_value + end + end + test "custom handling of job that exceeds retry attempts" do perform_enqueued_jobs do RetryJob.perform_later "CustomCatchError", 6 diff --git a/activejob/test/jobs/retry_job.rb b/activejob/test/jobs/retry_job.rb index 9aa99d9a21..82b80fe12b 100644 --- a/activejob/test/jobs/retry_job.rb +++ b/activejob/test/jobs/retry_job.rb @@ -10,6 +10,7 @@ class ExponentialWaitTenAttemptsError < StandardError; end class CustomWaitTenAttemptsError < StandardError; end class CustomCatchError < StandardError; end class DiscardableError < StandardError; end +class CustomDiscardableError < StandardError; end class RetryJob < ActiveJob::Base retry_on DefaultsError @@ -19,6 +20,7 @@ class RetryJob < ActiveJob::Base retry_on CustomWaitTenAttemptsError, wait: ->(executions) { executions * 2 }, attempts: 10 retry_on(CustomCatchError) { |job, exception| JobBuffer.add("Dealt with a job that failed to retry in a custom way after #{job.arguments.second} attempts. Message: #{exception.message}") } discard_on DiscardableError + discard_on(CustomDiscardableError) { |job, exception| JobBuffer.add("Dealt with a job that was discarded in a custom way") } def perform(raising, attempts) if executions < attempts diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index b67a803b9d..86353674d9 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,7 +1,14 @@ +* Models using the attributes API with a proc default can now be marshalled. + + Fixes #31216. + + *Sean Griffin* + * Fix to working before/after validation callbacks on multiple contexts. *Yoshiyuki Hirano* + ## Rails 5.2.0.beta2 (November 28, 2017) ## * No changes. diff --git a/activemodel/lib/active_model/attribute/user_provided_default.rb b/activemodel/lib/active_model/attribute/user_provided_default.rb index f274b687d4..a5dc6188d2 100644 --- a/activemodel/lib/active_model/attribute/user_provided_default.rb +++ b/activemodel/lib/active_model/attribute/user_provided_default.rb @@ -22,6 +22,28 @@ module ActiveModel self.class.new(name, user_provided_value, type, original_attribute) end + def marshal_dump + result = [ + name, + value_before_type_cast, + type, + original_attribute, + ] + result << value if defined?(@value) + result + end + + def marshal_load(values) + name, user_provided_value, type, original_attribute, value = values + @name = name + @user_provided_value = user_provided_value + @type = type + @original_attribute = original_attribute + if values.length == 5 + @value = value + end + end + protected attr_reader :user_provided_value diff --git a/activemodel/test/cases/attributes_test.rb b/activemodel/test/cases/attributes_test.rb index e43bf15335..7c1d813ce0 100644 --- a/activemodel/test/cases/attributes_test.rb +++ b/activemodel/test/cases/attributes_test.rb @@ -64,5 +64,14 @@ module ActiveModel assert_equal "4.4", data.integer_field end + + test "attributes with proc defaults can be marshalled" do + data = ModelForAttributesTest.new + attributes = data.instance_variable_get(:@attributes) + round_tripped = Marshal.load(Marshal.dump(data)) + new_attributes = round_tripped.instance_variable_get(:@attributes) + + assert_equal attributes, new_attributes + end end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index efe555374a..e2dc8045e2 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Support for PostgreSQL foreign tables. + + *fatkodima* + * Fix relation merger issue with `left_outer_joins`. *Mehmet Emin İNAÇ* diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 24766dd315..59929b8c4e 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -8,9 +8,7 @@ module ActiveRecord def initialize(owner, reflection) super - - @through_records = {} - @through_association = nil + @through_records = {} end def concat(*records) @@ -50,11 +48,6 @@ module ActiveRecord end private - - def through_association - @through_association ||= owner.association(through_reflection.name) - end - # The through record (built with build_record) is temporarily cached # so that it may be reused if insert_record is subsequently called. # diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb index 36746f9115..491282adf7 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -6,17 +6,16 @@ module ActiveRecord class HasOneThroughAssociation < HasOneAssociation #:nodoc: include ThroughAssociation - def replace(record) - create_through_record(record) + def replace(record, save = true) + create_through_record(record, save) self.target = record end private - - def create_through_record(record) + def create_through_record(record, save) ensure_not_nested - through_proxy = owner.association(through_reflection.name) + through_proxy = through_association through_record = through_proxy.load_target if through_record && !record @@ -30,7 +29,7 @@ module ActiveRecord if through_record through_record.update(attributes) - elsif owner.new_record? + elsif owner.new_record? || !save through_proxy.build(attributes) else through_proxy.create(attributes) diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index 54673b74f7..5afb0bc068 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -19,6 +19,10 @@ module ActiveRecord end end + def through_association + @through_association ||= owner.association(through_reflection.name) + end + # We merge in these scopes for two reasons: # # 1. To get the default_scope conditions for any of the other reflections in the chain 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 36048bee03..d663b59444 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -356,35 +356,33 @@ module ActiveRecord # Inserts a set of fixtures into the table. Overridden in adapters that require # something beyond a simple insert (eg. Oracle). def insert_fixtures(fixtures, table_name) + ActiveSupport::Deprecation.warn(<<-MSG.squish) + `insert_fixtures` is deprecated and will be removed in the next version of Rails. + Consider using `insert_fixtures_set` for performance improvement. + MSG return if fixtures.empty? - columns = schema_cache.columns_hash(table_name) + execute(build_fixture_sql(fixtures, table_name), "Fixtures Insert") + end - values = fixtures.map do |fixture| - fixture = fixture.stringify_keys + def insert_fixtures_set(fixture_set, tables_to_delete = []) + fixture_inserts = fixture_set.map do |table_name, fixtures| + next if fixtures.empty? - unknown_columns = fixture.keys - columns.keys - if unknown_columns.any? - raise Fixture::FixtureError, %(table "#{table_name}" has no columns named #{unknown_columns.map(&:inspect).join(', ')}.) - end + build_fixture_sql(fixtures, table_name) + end.compact - columns.map do |name, column| - if fixture.key?(name) - type = lookup_cast_type_from_column(column) - bind = Relation::QueryAttribute.new(name, fixture[name], type) - with_yaml_fallback(bind.value_for_database) - else - Arel.sql("DEFAULT") + table_deletes = tables_to_delete.map { |table| "DELETE FROM #{quote_table_name table}".dup } + total_sql = Array.wrap(combine_multi_statements(table_deletes + fixture_inserts)) + + disable_referential_integrity do + transaction(requires_new: true) do + total_sql.each do |sql| + execute sql, "Fixtures Load" + yield if block_given? end end end - - table = Arel::Table.new(table_name) - manager = Arel::InsertManager.new - manager.into(table) - columns.each_key { |column| manager.columns << table[column] } - manager.values = manager.create_values_list(values) - execute manager.to_sql, "Fixtures Insert" end def empty_insert_statement_value @@ -417,6 +415,41 @@ module ActiveRecord private + def build_fixture_sql(fixtures, table_name) + columns = schema_cache.columns_hash(table_name) + + values = fixtures.map do |fixture| + fixture = fixture.stringify_keys + + unknown_columns = fixture.keys - columns.keys + if unknown_columns.any? + raise Fixture::FixtureError, %(table "#{table_name}" has no columns named #{unknown_columns.map(&:inspect).join(', ')}.) + end + + columns.map do |name, column| + if fixture.key?(name) + type = lookup_cast_type_from_column(column) + bind = Relation::QueryAttribute.new(name, fixture[name], type) + with_yaml_fallback(bind.value_for_database) + else + Arel.sql("DEFAULT") + end + end + end + + table = Arel::Table.new(table_name) + manager = Arel::InsertManager.new + manager.into(table) + columns.each_key { |column| manager.columns << table[column] } + manager.values = manager.create_values_list(values) + + manager.to_sql + end + + def combine_multi_statements(total_sql) + total_sql.join(";\n") + end + # Returns a subquery for the given key using the join information. def subquery_for(key, select) subselect = select.clone diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index b394a8969f..d1a3b6de40 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -530,8 +530,56 @@ module ActiveRecord without_sql_mode("NO_AUTO_VALUE_ON_ZERO") { super } end + def insert_fixtures_set(fixture_set, tables_to_delete = []) + iterate_over_results = -> { while raw_connection.next_result; end; } + + with_multi_statements do + without_sql_mode("NO_AUTO_VALUE_ON_ZERO") do + super(fixture_set, tables_to_delete, &iterate_over_results) + end + end + end + private + def combine_multi_statements(total_sql) + total_sql.each_with_object([]) do |sql, total_sql_chunks| + previous_packet = total_sql_chunks.last + sql << ";\n" + if max_allowed_packet_reached?(sql, previous_packet) || total_sql_chunks.empty? + total_sql_chunks << sql + else + previous_packet << sql + end + end + end + + def max_allowed_packet_reached?(current_packet, previous_packet) + if current_packet.bytesize > max_allowed_packet + raise ActiveRecordError, "Fixtures set is too large #{current_packet.bytesize}. Consider increasing the max_allowed_packet variable." + elsif previous_packet.nil? + false + else + (current_packet.bytesize + previous_packet.bytesize) > max_allowed_packet + end + end + + def max_allowed_packet + bytes_margin = 2 + @max_allowed_packet ||= (show_variable("max_allowed_packet") - bytes_margin) + end + + def with_multi_statements + previous_flags = @config[:flags] + @config[:flags] = Mysql2::Client::MULTI_STATEMENTS + reconnect! + + yield + ensure + @config[:flags] = previous_flags + reconnect! + end + def without_sql_mode(mode) result = execute("SELECT @@SESSION.sql_mode") current_mode = result.first[0] diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index c6e5122daf..8678fab2ac 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -375,7 +375,7 @@ module ActiveRecord if respond_to?(method, true) sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) } sql_fragments << sqls - non_combinable_operations << procs if procs.present? + non_combinable_operations.concat(procs) else execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? non_combinable_operations.each(&:call) @@ -527,6 +527,14 @@ module ActiveRecord end end + def foreign_tables + query_values(data_source_sql(type: "FOREIGN TABLE"), "SCHEMA") + end + + def foreign_table_exists?(table_name) + query_values(data_source_sql(table_name, type: "FOREIGN TABLE"), "SCHEMA").any? if table_name.present? + end + # Maps logical Rails types to PostgreSQL-specific data types. def type_to_sql(type, limit: nil, precision: nil, scale: nil, array: nil, **) # :nodoc: sql = \ @@ -739,7 +747,7 @@ module ActiveRecord def data_source_sql(name = nil, type: nil) scope = quoted_scope(name, type: type) - scope[:type] ||= "'r','v','m'" # (r)elation/table, (v)iew, (m)aterialized view + scope[:type] ||= "'r','v','m','f'" # (r)elation/table, (v)iew, (m)aterialized view, (f)oreign table sql = "SELECT c.relname FROM pg_class c LEFT JOIN pg_namespace n ON n.oid = c.relnamespace".dup sql << " WHERE n.nspname = #{scope[:schema]}" @@ -756,6 +764,8 @@ module ActiveRecord "'r'" when "VIEW" "'v','m'" + when "FOREIGN TABLE" + "'f'" end scope = {} scope[:schema] = schema ? quote(schema) : "ANY (current_schemas(false))" diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 9ac5a8760e..dc6287e32c 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -281,7 +281,7 @@ module ActiveRecord end def discard! # :nodoc: - @connection.socket_io.reopen(IO::NULL) + @connection.socket_io.reopen(IO::NULL) rescue nil @connection = nil end @@ -318,6 +318,10 @@ module ActiveRecord postgresql_version >= 90300 end + def supports_foreign_tables? + postgresql_version >= 90300 + end + def supports_pgcrypto_uuid? postgresql_version >= 90400 end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index d4f5bd16ac..c66cada07a 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -101,7 +101,7 @@ module ActiveRecord def initialize(connection, logger, connection_options, config) super(connection, logger, config) - @active = nil + @active = true @statements = StatementPool.new(self.class.type_cast_config_to_integer(config[:statement_limit])) configure_connection @@ -144,7 +144,7 @@ module ActiveRecord end def active? - @active != false + @active end # Disconnects from the database if already connected. Otherwise, this @@ -372,6 +372,18 @@ module ActiveRecord end end + def insert_fixtures_set(fixture_set, tables_to_delete = []) + disable_referential_integrity do + transaction(requires_new: true) do + tables_to_delete.each { |table| delete "DELETE FROM #{quote_table_name(table)}", "Fixture Delete" } + + fixture_set.each do |table_name, rows| + rows.each { |row| insert_fixture(row, table_name) } + end + end + end + end + private def initialize_type_map(m = type_map) super diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 86f13d75d5..896d51c0fe 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -540,47 +540,38 @@ module ActiveRecord } unless files_to_read.empty? - connection.disable_referential_integrity do - fixtures_map = {} - - fixture_sets = files_to_read.map do |fs_name| - klass = class_names[fs_name] - conn = klass ? klass.connection : connection - fixtures_map[fs_name] = new( # ActiveRecord::FixtureSet.new - conn, - fs_name, - klass, - ::File.join(fixtures_directory, fs_name)) - end - - update_all_loaded_fixtures fixtures_map - - connection.transaction(requires_new: true) do - deleted_tables = Hash.new { |h, k| h[k] = Set.new } - fixture_sets.each do |fs| - conn = fs.model_class.respond_to?(:connection) ? fs.model_class.connection : connection - table_rows = fs.table_rows + fixtures_map = {} + + fixture_sets = files_to_read.map do |fs_name| + klass = class_names[fs_name] + conn = klass ? klass.connection : connection + fixtures_map[fs_name] = new( # ActiveRecord::FixtureSet.new + conn, + fs_name, + klass, + ::File.join(fixtures_directory, fs_name)) + end - table_rows.each_key do |table| - unless deleted_tables[conn].include? table - conn.delete "DELETE FROM #{conn.quote_table_name(table)}", "Fixture Delete" - end - deleted_tables[conn] << table - end + update_all_loaded_fixtures fixtures_map + fixture_sets_by_connection = fixture_sets.group_by { |fs| fs.model_class ? fs.model_class.connection : connection } - table_rows.each do |fixture_set_name, rows| - conn.insert_fixtures(rows, fixture_set_name) - end + fixture_sets_by_connection.each do |conn, set| + table_rows_for_connection = Hash.new { |h, k| h[k] = [] } - # Cap primary key sequences to max(pk). - if conn.respond_to?(:reset_pk_sequence!) - conn.reset_pk_sequence!(fs.table_name) - end + set.each do |fs| + fs.table_rows.each do |table, rows| + table_rows_for_connection[table].unshift(*rows) end end + conn.insert_fixtures_set(table_rows_for_connection, table_rows_for_connection.keys) - cache_fixtures(connection, fixtures_map) + # Cap primary key sequences to max(pk). + if conn.respond_to?(:reset_pk_sequence!) + set.each { |fs| conn.reset_pk_sequence!(fs.table_name) } + end end + + cache_fixtures(connection, fixtures_map) end cached_fixtures(connection, fixture_set_names) end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index cdd54cc502..a45d011d75 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -359,7 +359,8 @@ module ActiveRecord # Any change to the attributes on either instance will affect both instances. # If you want to change the sti column as well, use #becomes! instead. def becomes(klass) - became = klass.new + became = klass.allocate + became.send(:initialize) became.instance_variable_set("@attributes", @attributes) became.instance_variable_set("@mutations_from_database", @mutations_from_database) if defined?(@mutations_from_database) became.instance_variable_set("@new_record", new_record?) diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 0e9e86f425..7eecd5bc35 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -228,7 +228,9 @@ class PostgresqlArrayTest < ActiveRecord::PostgreSQLTestCase def test_insert_fixtures tag_values = ["val1", "val2", "val3_with_'_multiple_quote_'_chars"] - @connection.insert_fixtures([{ "tags" => tag_values }], "pg_arrays") + assert_deprecated do + @connection.insert_fixtures([{ "tags" => tag_values }], "pg_arrays") + end assert_equal(PgArray.last.tags, tag_values) end diff --git a/activerecord/test/cases/adapters/postgresql/foreign_table_test.rb b/activerecord/test/cases/adapters/postgresql/foreign_table_test.rb new file mode 100644 index 0000000000..4fa315ad23 --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/foreign_table_test.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/professor" + +if ActiveRecord::Base.connection.supports_foreign_tables? + class ForeignTableTest < ActiveRecord::TestCase + self.use_transactional_tests = false + + class ForeignProfessor < ActiveRecord::Base + self.table_name = "foreign_professors" + end + + class ForeignProfessorWithPk < ForeignProfessor + self.primary_key = "id" + end + + def setup + @professor = Professor.create(name: "Nicola") + + @connection = ActiveRecord::Base.connection + enable_extension!("postgres_fdw", @connection) + + foreign_db_config = ARTest.connection_config["arunit2"] + @connection.execute <<-SQL + CREATE SERVER foreign_server + FOREIGN DATA WRAPPER postgres_fdw + OPTIONS (dbname '#{foreign_db_config["database"]}') + SQL + + @connection.execute <<-SQL + CREATE USER MAPPING FOR CURRENT_USER + SERVER foreign_server + SQL + + @connection.execute <<-SQL + CREATE FOREIGN TABLE foreign_professors ( + id int, + name character varying NOT NULL + ) SERVER foreign_server OPTIONS ( + table_name 'professors' + ) + SQL + end + + def teardown + disable_extension!("postgres_fdw", @connection) + @connection.execute <<-SQL + DROP SERVER IF EXISTS foreign_server CASCADE + SQL + end + + def test_table_exists + table_name = ForeignProfessor.table_name + assert_not ActiveRecord::Base.connection.table_exists?(table_name) + end + + def test_foreign_tables_are_valid_data_sources + table_name = ForeignProfessor.table_name + assert @connection.data_source_exists?(table_name), "'#{table_name}' should be a data source" + end + + def test_foreign_tables + assert_equal ["foreign_professors"], @connection.foreign_tables + end + + def test_foreign_table_exists + assert @connection.foreign_table_exists?("foreign_professors") + assert @connection.foreign_table_exists?(:foreign_professors) + assert_not @connection.foreign_table_exists?("nonexistingtable") + assert_not @connection.foreign_table_exists?("'") + assert_not @connection.foreign_table_exists?(nil) + end + + def test_attribute_names + assert_equal ["id", "name"], ForeignProfessor.attribute_names + end + + def test_attributes + professor = ForeignProfessorWithPk.find(@professor.id) + assert_equal @professor.attributes, professor.attributes + end + + def test_does_not_have_a_primary_key + assert_nil ForeignProfessor.primary_key + end + + def test_insert_record + # Explicit `id` here to avoid complex configurations to implicitly work with remote table + ForeignProfessorWithPk.create!(id: 100, name: "Leonardo") + + professor = ForeignProfessorWithPk.last + assert_equal "Leonardo", professor.name + end + + def test_update_record + professor = ForeignProfessorWithPk.find(@professor.id) + professor.name = "Albert" + professor.save! + professor.reload + assert_equal "Albert", professor.name + end + + def test_delete_record + professor = ForeignProfessorWithPk.find(@professor.id) + assert_difference("ForeignProfessor.count", -1) { professor.destroy } + end + end +end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index 1d37457464..8bbd4134fa 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -42,6 +42,18 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase assert_not_nil new_member.club end + def test_creating_association_builds_through_record + new_member = Member.create(name: "Chris") + new_club = new_member.association(:club).build + assert new_member.current_membership + assert_equal new_club, new_member.club + assert new_club.new_record? + assert new_member.current_membership.new_record? + assert new_member.save + assert new_club.persisted? + assert new_member.current_membership.persisted? + end + def test_creating_association_builds_through_record_for_new new_member = Member.new(name: "Jane") new_member.club = clubs(:moustache_club) diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index 603ed63a8c..ef0b00b36c 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -171,6 +171,46 @@ module ActiveRecord assert_equal 3, ActiveRecord::Base.connection.select_value("SELECT COUNT(*) FROM people") end + def test_forked_child_recovers_from_disconnected_parent + object_id = ActiveRecord::Base.connection.object_id + assert ActiveRecord::Base.connection.active? + + rd, wr = IO.pipe + rd.binmode + wr.binmode + + outer_pid = fork { + ActiveRecord::Base.connection.disconnect! + + pid = fork { + rd.close + if ActiveRecord::Base.connection.active? + pair = [ActiveRecord::Base.connection.object_id, + ActiveRecord::Base.connection.select_value("SELECT COUNT(*) FROM people")] + wr.write Marshal.dump pair + end + wr.close + + exit # allow finalizers to run + } + + Process.waitpid pid + } + + wr.close + + Process.waitpid outer_pid + child_id, child_count = Marshal.load(rd.read) + + assert_not_equal object_id, child_id + rd.close + + assert_equal 3, child_count + + # Outer connection is unaffected + assert_equal 6, ActiveRecord::Base.connection.select_value("SELECT 2 * COUNT(*) FROM people") + end + def test_retrieve_connection_pool_copies_schema_cache_from_ancestor_pool @pool.schema_cache = @pool.connection.schema_cache @pool.schema_cache.add("posts") diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index d692c0eccb..d6b22e0a79 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -79,6 +79,124 @@ class FixturesTest < ActiveRecord::TestCase ActiveSupport::Notifications.unsubscribe(subscription) end end + + def test_bulk_insert_multiple_table_with_a_multi_statement_query + subscriber = InsertQuerySubscriber.new + subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber) + + create_fixtures("bulbs", "authors", "computers") + + expected_sql = <<-EOS.strip_heredoc.chop + INSERT INTO #{ActiveRecord::Base.connection.quote_table_name("bulbs")} .* + INSERT INTO #{ActiveRecord::Base.connection.quote_table_name("authors")} .* + INSERT INTO #{ActiveRecord::Base.connection.quote_table_name("computers")} .* + EOS + assert_equal 1, subscriber.events.size + assert_match(/#{expected_sql}/, subscriber.events.first) + ensure + ActiveSupport::Notifications.unsubscribe(subscription) + end + + def test_bulk_insert_with_a_multi_statement_query_raises_an_exception_when_any_insert_fails + require "models/aircraft" + + assert_equal false, Aircraft.columns_hash["wheels_count"].null + fixtures = { + "aircraft" => [ + { "name" => "working_aircrafts", "wheels_count" => 2 }, + { "name" => "broken_aircrafts", "wheels_count" => nil }, + ] + } + + assert_no_difference "Aircraft.count" do + assert_raises(ActiveRecord::NotNullViolation) do + ActiveRecord::Base.connection.insert_fixtures_set(fixtures) + end + end + end + end + + if current_adapter?(:Mysql2Adapter) + def test_insert_fixtures_set_raises_an_error_when_max_allowed_packet_is_smaller_than_fixtures_set_size + conn = ActiveRecord::Base.connection + mysql_margin = 2 + packet_size = 1024 + bytes_needed_to_have_a_1024_bytes_fixture = 855 + fixtures = { + "traffic_lights" => [ + { "location" => "US", "state" => ["NY"], "long_state" => ["a" * bytes_needed_to_have_a_1024_bytes_fixture] }, + ] + } + + conn.stubs(:max_allowed_packet).returns(packet_size - mysql_margin) + + error = assert_raises(ActiveRecord::ActiveRecordError) { conn.insert_fixtures_set(fixtures) } + assert_match(/Fixtures set is too large #{packet_size}\./, error.message) + end + + def test_insert_fixture_set_when_max_allowed_packet_is_bigger_than_fixtures_set_size + conn = ActiveRecord::Base.connection + packet_size = 1024 + fixtures = { + "traffic_lights" => [ + { "location" => "US", "state" => ["NY"], "long_state" => ["a" * 51] }, + ] + } + + conn.stubs(:max_allowed_packet).returns(packet_size) + + assert_difference "TrafficLight.count" do + conn.insert_fixtures_set(fixtures) + end + end + + def test_insert_fixtures_set_split_the_total_sql_into_two_chunks_smaller_than_max_allowed_packet + subscriber = InsertQuerySubscriber.new + subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber) + conn = ActiveRecord::Base.connection + packet_size = 1024 + fixtures = { + "traffic_lights" => [ + { "location" => "US", "state" => ["NY"], "long_state" => ["a" * 450] }, + ], + "comments" => [ + { "post_id" => 1, "body" => "a" * 450 }, + ] + } + + conn.stubs(:max_allowed_packet).returns(packet_size) + + conn.insert_fixtures_set(fixtures) + assert_equal 2, subscriber.events.size + assert_operator subscriber.events.first.bytesize, :<, packet_size + assert_operator subscriber.events.second.bytesize, :<, packet_size + ensure + ActiveSupport::Notifications.unsubscribe(subscription) + end + + def test_insert_fixtures_set_concat_total_sql_into_a_single_packet_smaller_than_max_allowed_packet + subscriber = InsertQuerySubscriber.new + subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber) + conn = ActiveRecord::Base.connection + packet_size = 1024 + fixtures = { + "traffic_lights" => [ + { "location" => "US", "state" => ["NY"], "long_state" => ["a" * 200] }, + ], + "comments" => [ + { "post_id" => 1, "body" => "a" * 200 }, + ] + } + + conn.stubs(:max_allowed_packet).returns(packet_size) + + assert_difference ["TrafficLight.count", "Comment.count"], +1 do + conn.insert_fixtures_set(fixtures) + end + assert_equal 1, subscriber.events.size + ensure + ActiveSupport::Notifications.unsubscribe(subscription) + end end def test_no_auto_value_on_zero_is_disabled diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index ffcf6a588e..2ef055fb01 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -881,7 +881,7 @@ if ActiveRecord::Base.connection.supports_bulk_alter? classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] expected_query_count = { "Mysql2Adapter" => 3, # one query for columns, one query for primary key, one query to do the bulk change - "PostgreSQLAdapter" => 2, # one query for columns, one for bulk change + "PostgreSQLAdapter" => 3, # one query for columns, one for bulk change, one for comment }.fetch(classname) { raise "need an expected query count for #{classname}" } @@ -889,12 +889,13 @@ if ActiveRecord::Base.connection.supports_bulk_alter? assert_queries(expected_query_count, ignore_none: true) do with_bulk_change_table do |t| t.change :name, :string, default: "NONAME" - t.change :birthdate, :datetime + t.change :birthdate, :datetime, comment: "This is a comment" end end assert_equal "NONAME", column(:name).default assert_equal :datetime, column(:birthdate).type + assert_equal "This is a comment", column(:birthdate).comment end private diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 0edca96cf5..d242fff442 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -473,6 +473,22 @@ class PersistenceTest < ActiveRecord::TestCase assert_instance_of Reply, Reply.find(reply.id) end + def test_becomes_default_sti_subclass + original_type = Topic.columns_hash["type"].default + ActiveRecord::Base.connection.change_column_default :topics, :type, "Reply" + Topic.reset_column_information + + reply = topics(:second) + assert_instance_of Reply, reply + + topic = reply.becomes(Topic) + assert_instance_of Topic, topic + + ensure + ActiveRecord::Base.connection.change_column_default :topics, :type, original_type + Topic.reset_column_information + end + def test_update_after_create klass = Class.new(Topic) do def self.name; "Topic"; end diff --git a/activestorage/lib/active_storage/downloading.rb b/activestorage/lib/active_storage/downloading.rb index a57fda49b4..295289c1e7 100644 --- a/activestorage/lib/active_storage/downloading.rb +++ b/activestorage/lib/active_storage/downloading.rb @@ -1,10 +1,12 @@ # frozen_string_literal: true +require "tmpdir" + module ActiveStorage module Downloading private # Opens a new tempfile in #tempdir and copies blob data into it. Yields the tempfile. - def download_blob_to_tempfile # :doc: + def download_blob_to_tempfile #:doc: Tempfile.open([ "ActiveStorage", blob.filename.extension_with_delimiter ], tempdir) do |file| download_blob_to file yield file @@ -12,14 +14,14 @@ module ActiveStorage end # Efficiently downloads blob data into the given file. - def download_blob_to(file) # :doc: + def download_blob_to(file) #:doc: file.binmode blob.download { |chunk| file.write(chunk) } file.rewind end # Returns the directory in which tempfiles should be opened. Defaults to +Dir.tmpdir+. - def tempdir # :doc: + def tempdir #:doc: Dir.tmpdir end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index acff6367f2..29d6119113 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add support for connection pooling on RedisCacheStore. + + *fatkodima* + * Support hash as first argument in `assert_difference`. This allows to specify multiple numeric differences in the same assertion. diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 2d038dba77..d221b36365 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -160,6 +160,23 @@ module ActiveSupport attr_reader :silence, :options alias :silence? :silence + class << self + private + def retrieve_pool_options(options) + {}.tap do |pool_options| + pool_options[:size] = options[:pool_size] if options[:pool_size] + pool_options[:timeout] = options[:pool_timeout] if options[:pool_timeout] + end + end + + def ensure_connection_pool_added! + require "connection_pool" + rescue LoadError => e + $stderr.puts "You don't have connection_pool installed in your application. Please add it to your Gemfile and run bundle install" + raise e + end + end + # Creates a new cache. The options will be passed to any write method calls # except for <tt>:namespace</tt> which can be used to set the global # namespace for the cache. diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index cae0d44e7d..2840781dde 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -63,21 +63,12 @@ module ActiveSupport addresses = addresses.flatten options = addresses.extract_options! addresses = ["localhost:11211"] if addresses.empty? - - pool_options = {} - pool_options[:size] = options[:pool_size] if options[:pool_size] - pool_options[:timeout] = options[:pool_timeout] if options[:pool_timeout] + pool_options = retrieve_pool_options(options) if pool_options.empty? Dalli::Client.new(addresses, options) else - begin - require "connection_pool" - rescue LoadError => e - $stderr.puts "You don't have connection_pool installed in your application. Please add it to your Gemfile and run bundle install" - raise e - end - + ensure_connection_pool_added! ConnectionPool.new(pool_options) { Dalli::Client.new(addresses, options.merge(threadsafe: false)) } end end diff --git a/activesupport/lib/active_support/cache/redis_cache_store.rb b/activesupport/lib/active_support/cache/redis_cache_store.rb index 0368423dad..c4cd9c4761 100644 --- a/activesupport/lib/active_support/cache/redis_cache_store.rb +++ b/activesupport/lib/active_support/cache/redis_cache_store.rb @@ -20,6 +20,31 @@ require "active_support/core_ext/marshal" module ActiveSupport module Cache + module ConnectionPoolLike + def with + yield self + end + end + + ::Redis.include(ConnectionPoolLike) + + class RedisDistributedWithConnectionPool < ::Redis::Distributed + def add_node(options) + pool_options = {} + pool_options[:size] = options[:pool_size] if options[:pool_size] + pool_options[:timeout] = options[:pool_timeout] if options[:pool_timeout] + + if pool_options.empty? + super + else + options = { url: options } if options.is_a?(String) + options = @default_options.merge(options) + pool = ConnectionPool.new(pool_options) { ::Redis.new(options) } + @ring.add_node(pool) + end + end + end + # Redis cache store. # # Deployment note: Take care to use a *dedicated Redis cache* rather @@ -122,7 +147,7 @@ module ActiveSupport private def build_redis_distributed_client(urls:, **redis_options) - ::Redis::Distributed.new([], DEFAULT_REDIS_OPTIONS.merge(redis_options)).tap do |dist| + RedisDistributedWithConnectionPool.new([], DEFAULT_REDIS_OPTIONS.merge(redis_options)).tap do |dist| urls.each { |u| dist.add_node url: u } end end @@ -172,7 +197,7 @@ module ActiveSupport end def redis - @redis ||= self.class.build_redis(**redis_options) + @redis ||= wrap_in_connection_pool(self.class.build_redis(**redis_options)) end def inspect @@ -211,7 +236,7 @@ module ActiveSupport instrument :delete_matched, matcher do case matcher when String - redis.eval DELETE_GLOB_LUA, [], [namespace_key(matcher, options)] + redis.with { |c| c.eval DELETE_GLOB_LUA, [], [namespace_key(matcher, options)] } else raise ArgumentError, "Only Redis glob strings are supported: #{matcher.inspect}" end @@ -228,7 +253,9 @@ module ActiveSupport # Failsafe: Raises errors. def increment(name, amount = 1, options = nil) instrument :increment, name, amount: amount do - redis.incrby normalize_key(name, options), amount + failsafe :increment do + redis.with { |c| c.incrby normalize_key(name, options), amount } + end end end @@ -242,7 +269,9 @@ module ActiveSupport # Failsafe: Raises errors. def decrement(name, amount = 1, options = nil) instrument :decrement, name, amount: amount do - redis.decrby normalize_key(name, options), amount + failsafe :decrement do + redis.with { |c| c.decrby normalize_key(name, options), amount } + end end end @@ -263,7 +292,7 @@ module ActiveSupport if namespace = merged_options(options)[namespace] delete_matched "*", namespace: namespace else - redis.flushdb + redis.with { |c| c.flushdb } end end end @@ -279,6 +308,21 @@ module ActiveSupport end private + def wrap_in_connection_pool(redis_connection) + if redis_connection.is_a?(::Redis) + pool_options = self.class.send(:retrieve_pool_options, redis_options) + + if pool_options.empty? + redis_connection + else + self.class.send(:ensure_connection_pool_added!) + ConnectionPool.new(pool_options) { redis_connection } + end + else + redis_connection + end + end + def set_redis_capabilities case redis when Redis::Distributed @@ -294,7 +338,7 @@ module ActiveSupport # Read an entry from the cache. def read_entry(key, options = nil) failsafe :read_entry do - deserialize_entry redis.get(key) + deserialize_entry redis.with { |c| c.get(key) } end end @@ -303,7 +347,10 @@ module ActiveSupport options = merged_options(options) keys = names.map { |name| normalize_key(name, options) } - values = redis.mget(*keys) + + values = failsafe(:read_multi_mget, returning: {}) do + redis.with { |c| c.mget(*keys) } + end names.zip(values).each_with_object({}) do |(name, value), results| if value @@ -328,15 +375,15 @@ module ActiveSupport expires_in += 5.minutes end - failsafe :write_entry do + failsafe :write_entry, returning: false do if unless_exist || expires_in modifiers = {} modifiers[:nx] = unless_exist modifiers[:px] = (1000 * expires_in.to_f).ceil if expires_in - redis.set key, value, modifiers + redis.with { |c| c.set key, value, modifiers } else - redis.set key, value + redis.with { |c| c.set key, value } end end end @@ -344,7 +391,7 @@ module ActiveSupport # Delete an entry from the cache. def delete_entry(key, options) failsafe :delete_entry, returning: false do - redis.del key + redis.with { |c| c.del key } end end @@ -353,7 +400,7 @@ module ActiveSupport if entries.any? if mset_capable? && expires_in.nil? failsafe :write_multi_entries do - redis.mapped_mset(entries) + redis.with { |c| c.mapped_mset(entries) } end else super diff --git a/activesupport/test/cache/behaviors.rb b/activesupport/test/cache/behaviors.rb index cb08a10bba..745dc09e2c 100644 --- a/activesupport/test/cache/behaviors.rb +++ b/activesupport/test/cache/behaviors.rb @@ -5,5 +5,7 @@ require_relative "behaviors/cache_delete_matched_behavior" require_relative "behaviors/cache_increment_decrement_behavior" require_relative "behaviors/cache_store_behavior" require_relative "behaviors/cache_store_version_behavior" +require_relative "behaviors/connection_pool_behavior" require_relative "behaviors/encoded_key_cache_behavior" +require_relative "behaviors/failure_safety_behavior" require_relative "behaviors/local_cache_behavior" diff --git a/activesupport/test/cache/behaviors/connection_pool_behavior.rb b/activesupport/test/cache/behaviors/connection_pool_behavior.rb new file mode 100644 index 0000000000..500d51a134 --- /dev/null +++ b/activesupport/test/cache/behaviors/connection_pool_behavior.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module ConnectionPoolBehavior + def test_connection_pool + emulating_latency do + begin + cache = ActiveSupport::Cache.lookup_store(store, pool_size: 2, pool_timeout: 1) + cache.clear + + threads = [] + + assert_raises Timeout::Error do + # One of the three threads will fail in 1 second because our pool size + # is only two. + 3.times do + threads << Thread.new do + cache.read("latency") + end + end + + threads.each(&:join) + end + ensure + threads.each(&:kill) + end + end + end + + def test_no_connection_pool + emulating_latency do + begin + cache = ActiveSupport::Cache.lookup_store(store) + cache.clear + + threads = [] + + assert_nothing_raised do + # Default connection pool size is 5, assuming 10 will make sure that + # the connection pool isn't used at all. + 10.times do + threads << Thread.new do + cache.read("latency") + end + end + + threads.each(&:join) + end + ensure + threads.each(&:kill) + end + end + end +end diff --git a/activesupport/test/cache/behaviors/failure_safety_behavior.rb b/activesupport/test/cache/behaviors/failure_safety_behavior.rb new file mode 100644 index 0000000000..53bda4f942 --- /dev/null +++ b/activesupport/test/cache/behaviors/failure_safety_behavior.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +module FailureSafetyBehavior + def test_fetch_read_failure_returns_nil + @cache.write("foo", "bar") + + emulating_unavailability do |cache| + assert_nil cache.fetch("foo") + end + end + + def test_fetch_read_failure_does_not_attempt_to_write + end + + def test_read_failure_returns_nil + @cache.write("foo", "bar") + + emulating_unavailability do |cache| + assert_nil cache.read("foo") + end + end + + def test_read_multi_failure_returns_empty_hash + @cache.write_multi("foo" => "bar", "baz" => "quux") + + emulating_unavailability do |cache| + assert_equal Hash.new, cache.read_multi("foo", "baz") + end + end + + def test_write_failure_returns_false + emulating_unavailability do |cache| + assert_equal false, cache.write("foo", "bar") + end + end + + def test_write_multi_failure_not_raises + emulating_unavailability do |cache| + assert_nothing_raised do + cache.write_multi("foo" => "bar", "baz" => "quux") + end + end + end + + def test_fetch_multi_failure_returns_fallback_results + @cache.write_multi("foo" => "bar", "baz" => "quux") + + emulating_unavailability do |cache| + fetched = cache.fetch_multi("foo", "baz") { |k| "unavailable" } + assert_equal Hash["foo" => "unavailable", "baz" => "unavailable"], fetched + end + end + + def test_delete_failure_returns_false + @cache.write("foo", "bar") + + emulating_unavailability do |cache| + assert_equal false, cache.delete("foo") + end + end + + def test_exist_failure_returns_false + @cache.write("foo", "bar") + + emulating_unavailability do |cache| + assert !cache.exist?("foo") + end + end + + def test_increment_failure_returns_nil + @cache.write("foo", 1, raw: true) + + emulating_unavailability do |cache| + assert_nil cache.increment("foo") + end + end + + def test_decrement_failure_returns_nil + @cache.write("foo", 1, raw: true) + + emulating_unavailability do |cache| + assert_nil cache.decrement("foo") + end + end + + def test_clear_failure_returns_nil + emulating_unavailability do |cache| + assert_nil cache.clear + end + end +end diff --git a/activesupport/test/cache/stores/mem_cache_store_test.rb b/activesupport/test/cache/stores/mem_cache_store_test.rb index 7f537c3bbf..3e2316f217 100644 --- a/activesupport/test/cache/stores/mem_cache_store_test.rb +++ b/activesupport/test/cache/stores/mem_cache_store_test.rb @@ -17,6 +17,12 @@ class SlowDalliClient < Dalli::Client end end +class UnavailableDalliServer < Dalli::Server + def alive? + false + end +end + class MemCacheStoreTest < ActiveSupport::TestCase begin ss = Dalli::Client.new("localhost:11211").stats @@ -45,56 +51,8 @@ class MemCacheStoreTest < ActiveSupport::TestCase include CacheIncrementDecrementBehavior include EncodedKeyCacheBehavior include AutoloadingCacheBehavior - - def test_connection_pool - emulating_latency do - begin - cache = ActiveSupport::Cache.lookup_store(:mem_cache_store, pool_size: 2, pool_timeout: 1) - cache.clear - - threads = [] - - assert_raises Timeout::Error do - # One of the three threads will fail in 1 second because our pool size - # is only two. - 3.times do - threads << Thread.new do - cache.read("latency") - end - end - - threads.each(&:join) - end - ensure - threads.each(&:kill) - end - end - end - - def test_no_connection_pool - emulating_latency do - begin - cache = ActiveSupport::Cache.lookup_store(:mem_cache_store) - cache.clear - - threads = [] - - assert_nothing_raised do - # Default connection pool size is 5, assuming 10 will make sure that - # the connection pool isn't used at all. - 10.times do - threads << Thread.new do - cache.read("latency") - end - end - - threads.each(&:join) - end - ensure - threads.each(&:kill) - end - end - end + include ConnectionPoolBehavior + include FailureSafetyBehavior def test_raw_values cache = ActiveSupport::Cache.lookup_store(:mem_cache_store, raw: true) @@ -154,6 +112,10 @@ class MemCacheStoreTest < ActiveSupport::TestCase private + def store + :mem_cache_store + end + def emulating_latency old_client = Dalli.send(:remove_const, :Client) Dalli.const_set(:Client, SlowDalliClient) @@ -163,4 +125,14 @@ class MemCacheStoreTest < ActiveSupport::TestCase Dalli.send(:remove_const, :Client) Dalli.const_set(:Client, old_client) end + + def emulating_unavailability + old_server = Dalli.send(:remove_const, :Server) + Dalli.const_set(:Server, UnavailableDalliServer) + + yield ActiveSupport::Cache::MemCacheStore.new + ensure + Dalli.send(:remove_const, :Server) + Dalli.const_set(:Server, old_server) + end end diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index 3635703002..7c1286a115 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -8,6 +8,18 @@ require_relative "../behaviors" module ActiveSupport::Cache::RedisCacheStoreTests DRIVER = %w[ ruby hiredis ].include?(ENV["REDIS_DRIVER"]) ? ENV["REDIS_DRIVER"] : "hiredis" + # Emulates a latency on Redis's back-end for the key latency to facilitate + # connection pool testing. + class SlowRedis < Redis + def get(key, options = {}) + if key =~ /latency/ + sleep 3 + else + super + end + end + end + class LookupTest < ActiveSupport::TestCase test "may be looked up as :redis_cache_store" do assert_kind_of ActiveSupport::Cache::RedisCacheStore, @@ -110,6 +122,26 @@ module ActiveSupport::Cache::RedisCacheStoreTests include AutoloadingCacheBehavior end + class RedisCacheStoreConnectionPoolBehaviourTest < StoreTest + include ConnectionPoolBehavior + + private + + def store + :redis_cache_store + end + + def emulating_latency + old_redis = Object.send(:remove_const, :Redis) + Object.const_set(:Redis, SlowRedis) + + yield + ensure + Object.send(:remove_const, :Redis) + Object.const_set(:Redis, old_redis) + end + end + # Separate test class so we can omit the namespace which causes expected, # appropriate complaints about incompatible string encodings. class KeyEncodingSafetyTest < StoreTest @@ -124,15 +156,26 @@ module ActiveSupport::Cache::RedisCacheStoreTests class StoreAPITest < StoreTest end - class FailureSafetyTest < StoreTest - test "fetch read failure returns nil" do + class UnavailableRedisClient < Redis::Client + def ensure_connected + raise Redis::BaseConnectionError end + end - test "fetch read failure does not attempt to write" do - end + class FailureSafetyTest < StoreTest + include FailureSafetyBehavior - test "write failure returns nil" do - end + private + + def emulating_unavailability + old_client = Redis.send(:remove_const, :Client) + Redis.const_set(:Client, UnavailableRedisClient) + + yield ActiveSupport::Cache::RedisCacheStore.new + ensure + Redis.send(:remove_const, :Client) + Redis.const_set(:Client, old_client) + end end class DeleteMatchedTest < StoreTest diff --git a/guides/source/active_record_basics.md b/guides/source/active_record_basics.md index 9be9c6c7b7..2f85b765a3 100644 --- a/guides/source/active_record_basics.md +++ b/guides/source/active_record_basics.md @@ -45,6 +45,8 @@ relationships of the objects in an application can be easily stored and retrieved from a database without writing SQL statements directly and with less overall database access code. +NOTE: If you are not familiar enough with relational database management systems (RDBMS) or structured query language (SQL), please go through [this tutorial](https://www.w3schools.com/sql/default.asp) (or [this one](http://www.sqlcourse.com/)) or study them by other means. Understanding how relational databases work is crucial to understanding Active Records and Rails in general. + ### Active Record as an ORM Framework Active Record gives us several mechanisms, the most important being the ability @@ -142,7 +144,7 @@ end This will create a `Product` model, mapped to a `products` table at the database. By doing this you'll also have the ability to map the columns of each row in that table with the attributes of the instances of your model. Suppose -that the `products` table was created using an SQL statement like: +that the `products` table was created using an SQL (or one of its extensions) statement like: ```sql CREATE TABLE products ( @@ -152,8 +154,9 @@ CREATE TABLE products ( ); ``` -Following the table schema above, you would be able to write code like the -following: +Schema above declares a table with two columns: `id` and `name`. Each row of +this table represents a certain product with these two parameters. Thus, you +would be able to write code like the following: ```ruby p = Product.new diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index 967c992c05..c1668f989b 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -134,7 +134,8 @@ learn about Ruby on Rails, and the API, which serves as a reference. You can help improve the Rails guides by making them more coherent, consistent or readable, adding missing information, correcting factual errors, fixing typos, or bringing them up to date with the latest edge Rails. -To do so, open a pull request to [Rails](https://github.com/rails/rails) on GitHub. +To do so, make changes to Rails guides source files (located [here](https://github.com/rails/rails/tree/master/guides/source) on GitHub). Then open a pull request to apply your +changes to master branch. When working with documentation, please take into account the [API Documentation Guidelines](api_documentation_guidelines.html) and the [Ruby on Rails Guides Guidelines](ruby_on_rails_guides_guidelines.html). diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index b007baea87..6cf99a7e5c 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -462,8 +462,7 @@ You're getting this error now because Rails expects plain actions like this one to have views associated with them to display their information. With no view available, Rails will raise an exception. -In the above image, the bottom line has been truncated. Let's see what the full -error message looks like: +Let's look at the full error message again: >ArticlesController#new is missing a template for this request format and variant. request.formats: ["text/html"] request.variant: [] NOTE! For XHR/Ajax or API requests, this action would normally respond with 204 No Content: an empty white screen. Since you're loading it in a web browser, we assume that you expected to actually render a template, not… nothing, so we're showing an error to be extra-clear. If you expect 204 No Content, carry on. That's what you'll get from an XHR or API request. Give it a shot. diff --git a/railties/lib/rails/templates/rails/mailers/email.html.erb b/railties/lib/rails/templates/rails/mailers/email.html.erb index 44c400a7e6..2a41c29602 100644 --- a/railties/lib/rails/templates/rails/mailers/email.html.erb +++ b/railties/lib/rails/templates/rails/mailers/email.html.erb @@ -95,14 +95,16 @@ </dd> <% end %> + <dt>Format:</dt> <% if @email.multipart? %> - <dt>Format:</dt> <dd> <select id="part" onchange="refreshBody();"> <option <%= request.format == Mime[:html] ? 'selected' : '' %> value="<%= part_query('text/html') %>">View as HTML email</option> <option <%= request.format == Mime[:text] ? 'selected' : '' %> value="<%= part_query('text/plain') %>">View as plain-text email</option> </select> </dd> + <% else %> + <dd id="mime_type" data-mime-type="<%= part_query(@email.mime_type) %>"><%= @email.mime_type == 'text/html' ? 'HTML email' : 'plain-text email' %></dd> <% end %> <% if I18n.available_locales.count > 1 %> @@ -131,15 +133,24 @@ function refreshBody() { var part_select = document.querySelector('select#part'); var locale_select = document.querySelector('select#locale'); - var part_param = part_select.options[part_select.selectedIndex].value; - var locale_param = locale_select.options[locale_select.selectedIndex].value; var iframe = document.getElementsByName('messageBody')[0]; - iframe.contentWindow.location = '?' + part_param + '&' + locale_param; + var part_param = part_select ? + part_select.options[part_select.selectedIndex].value : + document.querySelector('#mime_type').dataset.mimeType; + var locale_param = locale_select ? locale_select.options[locale_select.selectedIndex].value : null; + var fresh_location; + if (locale_param) { + fresh_location = '?' + part_param + '&' + locale_param; + } else { + fresh_location = '?' + part_param; + } + iframe.contentWindow.location = fresh_location; if (history.replaceState) { var url = location.pathname.replace(/\.(txt|html)$/, ''); var format = /html/.test(part_param) ? '.html' : '.txt'; - window.history.replaceState({}, '', url + format + '?' + locale_param); + var state_to_replace = locale_param ? (url + format + '?' + locale_param) : (url + format); + window.history.replaceState({}, '', state_to_replace); } } </script> |