aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionview/lib/action_view/helpers/form_helper.rb2
-rw-r--r--actionview/test/actionpack/abstract/helper_test.rb2
-rw-r--r--activerecord/CHANGELOG.md9
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb10
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb36
-rw-r--r--activerecord/lib/active_record/reflection.rb4
-rw-r--r--activerecord/lib/active_record/relation/finder_methods.rb8
-rw-r--r--activerecord/test/cases/adapters/postgresql/uuid_test.rb31
-rw-r--r--activerecord/test/cases/associations/has_many_through_associations_test.rb17
-rw-r--r--activerecord/test/cases/connection_pool_test.rb6
-rw-r--r--activesupport/lib/active_support/dependencies.rb2
-rw-r--r--activesupport/lib/active_support/logger.rb4
-rw-r--r--activesupport/test/broadcast_logger_test.rb22
-rw-r--r--railties/test/code_statistics_calculator_test.rb2
14 files changed, 99 insertions, 56 deletions
diff --git a/actionview/lib/action_view/helpers/form_helper.rb b/actionview/lib/action_view/helpers/form_helper.rb
index f1eefe1b8e..e7ea267211 100644
--- a/actionview/lib/action_view/helpers/form_helper.rb
+++ b/actionview/lib/action_view/helpers/form_helper.rb
@@ -1619,7 +1619,7 @@ module ActionView
@index = options[:index] || options[:child_index]
end
- (field_helpers - [:label, :check_box, :radio_button, :fields_for, :hidden_field, :file_field]).each do |selector|
+ (field_helpers - [:label, :check_box, :radio_button, :fields_for, :fields, :hidden_field, :file_field]).each do |selector|
class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
def #{selector}(method, options = {}) # def text_field(method, options = {})
@template.send( # @template.send(
diff --git a/actionview/test/actionpack/abstract/helper_test.rb b/actionview/test/actionpack/abstract/helper_test.rb
index 5a2f0839e5..83237518d7 100644
--- a/actionview/test/actionpack/abstract/helper_test.rb
+++ b/actionview/test/actionpack/abstract/helper_test.rb
@@ -109,7 +109,7 @@ module AbstractController
class InvalidHelpersTest < ActiveSupport::TestCase
def test_controller_raise_error_about_real_require_problem
e = assert_raise(LoadError) { AbstractInvalidHelpers.helper(:invalid_require) }
- assert_equal "No such file to load -- very_invalid_file_name", e.message
+ assert_equal "No such file to load -- very_invalid_file_name.rb", e.message
end
def test_controller_raise_error_about_missing_helper
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 0b9a2f9943..35f7c158a6 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,12 @@
+* Raise ActiveRecord::RecordNotFound from collection `*_ids` setters
+ for unknown IDs with a better error message.
+
+ Changes the collection `*_ids` setters to cast provided IDs the data
+ type of the primary key set in the association, not the model
+ primary key.
+
+ *Dominic Cleal*
+
* For PostgreSQL >= 9.4 use `pgcrypto`'s `gen_random_uuid()` instead of
`uuid-ossp`'s UUID generation function.
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index b2cf4713bb..3d23fa1e46 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -68,13 +68,17 @@ module ActiveRecord
# Implements the ids writer method, e.g. foo.item_ids= for Foo.has_many :items
def ids_writer(ids)
- pk_type = reflection.primary_key_type
+ pk_type = reflection.association_primary_key_type
ids = Array(ids).reject(&:blank?)
ids.map! { |i| pk_type.cast(i) }
records = klass.where(reflection.association_primary_key => ids).index_by do |r|
r.send(reflection.association_primary_key)
- end.values_at(*ids)
- replace(records)
+ end.values_at(*ids).compact
+ if records.size != ids.size
+ scope.raise_record_not_found_exception!(ids, records.size, ids.size, reflection.association_primary_key)
+ else
+ replace(records)
+ end
end
def reset
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index c9f907b281..d17722adec 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -422,7 +422,6 @@ module ActiveRecord
conn.disconnect!
end
@connections = []
- @available.clear
end
end
end
@@ -445,8 +444,6 @@ module ActiveRecord
# connections in the pool within a timeout interval (default duration is
# <tt>spec.config[:checkout_timeout] * 2</tt> seconds).
def clear_reloadable_connections(raise_on_acquisition_timeout = true)
- num_new_conns_required = 0
-
with_exclusively_acquired_all_connections(raise_on_acquisition_timeout) do
synchronize do
@connections.each do |conn|
@@ -457,24 +454,8 @@ module ActiveRecord
conn.disconnect! if conn.requires_reloading?
end
@connections.delete_if(&:requires_reloading?)
-
- @available.clear
-
- if @connections.size < @size
- # because of the pruning done by this method, we might be running
- # low on connections, while threads stuck in queue are helpless
- # (not being able to establish new connections for themselves),
- # see also more detailed explanation in +remove+
- num_new_conns_required = num_waiting_in_queue - @connections.size
- end
-
- @connections.each do |conn|
- @available.add conn
- end
end
end
-
- bulk_make_new_connections(num_new_conns_required) if num_new_conns_required > 0
end
# Clears the cache which maps classes and re-connects connections that
@@ -705,9 +686,26 @@ module ActiveRecord
yield
ensure
+ num_new_conns_required = 0
+
synchronize do
@threads_blocking_new_connections -= 1
+
+ if @threads_blocking_new_connections.zero?
+ @available.clear
+
+ num_new_conns_required = num_waiting_in_queue
+
+ @connections.each do |conn|
+ next if conn.in_use?
+
+ @available.add conn
+ num_new_conns_required -= 1
+ end
+ end
end
+
+ bulk_make_new_connections(num_new_conns_required) if num_new_conns_required > 0
end
# Acquire a connection by one of 1) immediately removing one
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index ef3c3bfae8..17751c9571 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -397,6 +397,10 @@ module ActiveRecord
options[:primary_key] || primary_key(klass || self.klass)
end
+ def association_primary_key_type
+ klass.type_for_attribute(association_primary_key)
+ end
+
def active_record_primary_key
@active_record_primary_key ||= options[:primary_key] || primary_key(active_record)
end
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index 55ded4c6d0..93c8722aa3 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -345,7 +345,7 @@ module ActiveRecord
# of results obtained should be provided in the +result_size+ argument and
# the expected number of results should be provided in the +expected_size+
# argument.
- def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil) # :nodoc:
+ def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil, key = primary_key) # :nodoc:
conditions = arel.where_sql(@klass.arel_engine)
conditions = " [#{conditions}]" if conditions
name = @klass.name
@@ -355,10 +355,10 @@ module ActiveRecord
error << " with#{conditions}" if conditions
raise RecordNotFound.new(error, name)
elsif Array(ids).size == 1
- error = "Couldn't find #{name} with '#{primary_key}'=#{ids}#{conditions}"
- raise RecordNotFound.new(error, name, primary_key, ids)
+ error = "Couldn't find #{name} with '#{key}'=#{ids}#{conditions}"
+ raise RecordNotFound.new(error, name, key, ids)
else
- error = "Couldn't find all #{name.pluralize} with '#{primary_key}': "
+ error = "Couldn't find all #{name.pluralize} with '#{key}': "
error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})"
raise RecordNotFound.new(error, name, primary_key, ids)
diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb
index ab0815631f..4604c2eb3b 100644
--- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb
@@ -43,20 +43,20 @@ class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase
column = UUIDType.columns_hash["thingy"]
assert_equal "gen_random_uuid()", column.default_function
end
- else
- def test_change_column_default
- connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v1()"
- UUIDType.reset_column_information
- column = UUIDType.columns_hash["thingy"]
- assert_equal "uuid_generate_v1()", column.default_function
+ end
- connection.change_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v4()"
- UUIDType.reset_column_information
- column = UUIDType.columns_hash["thingy"]
- assert_equal "uuid_generate_v4()", column.default_function
- ensure
- UUIDType.reset_column_information
- end
+ def test_change_column_default
+ connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v1()"
+ UUIDType.reset_column_information
+ column = UUIDType.columns_hash["thingy"]
+ assert_equal "uuid_generate_v1()", column.default_function
+
+ connection.change_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v4()"
+ UUIDType.reset_column_information
+ column = UUIDType.columns_hash["thingy"]
+ assert_equal "uuid_generate_v4()", column.default_function
+ ensure
+ UUIDType.reset_column_information
end
def test_data_type_of_uuid_types
@@ -236,6 +236,9 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase
if ActiveRecord::Base.connection.supports_pgcrypto_uuid?
def test_schema_dumper_for_uuid_primary_key_default_in_legacy_migration
+ @verbose_was = ActiveRecord::Migration.verbose
+ ActiveRecord::Migration.verbose = false
+
migration = Class.new(ActiveRecord::Migration[4.2]) do
def version; 101 end
def migrate(x)
@@ -248,8 +251,8 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase
assert_match(/\bcreate_table "pg_uuids_4", id: :uuid, default: -> { "uuid_generate_v4\(\)" }/, schema)
ensure
drop_table "pg_uuids_4"
+ ActiveRecord::Migration.verbose = @verbose_was
end
- else
end
end
end
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index c2239ac03a..8defb09db7 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -883,10 +883,25 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
end
+ def test_collection_singular_ids_setter_with_changed_primary_key
+ company = companies(:first_firm)
+ client = companies(:first_client)
+ company.clients_using_primary_key_ids = [client.name]
+ assert_equal [client], company.clients_using_primary_key
+ end
+
def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set
company = companies(:rails_core)
ids = [Developer.first.id, -9999]
- assert_raises(ActiveRecord::AssociationTypeMismatch) { company.developer_ids = ids }
+ e = assert_raises(ActiveRecord::RecordNotFound) { company.developer_ids = ids }
+ assert_match(/Couldn't find all Developers with 'id'/, e.message)
+ end
+
+ def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set_with_changed_primary_key
+ company = companies(:first_firm)
+ ids = [Client.first.name, "unknown client"]
+ e = assert_raises(ActiveRecord::RecordNotFound) { company.clients_using_primary_key_ids = ids }
+ assert_match(/Couldn't find all Clients with 'name'/, e.message)
end
def test_build_a_model_from_hm_through_association_with_where_clause
diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb
index b08e4f603c..2437c99621 100644
--- a/activerecord/test/cases/connection_pool_test.rb
+++ b/activerecord/test/cases/connection_pool_test.rb
@@ -487,12 +487,6 @@ module ActiveRecord
#--- post test clean up start
second_thread_done.count_down if failed
- # after `pool.disconnect()` the first thread will be left stuck in queue, no need to wait for
- # it to timeout with ConnectionTimeoutError
- if (group_action_method == :disconnect || group_action_method == :disconnect!) && pool.num_waiting_in_queue > 0
- pool.with_connection {} # create a new connection in case there are threads still stuck in a queue
- end
-
first_thread.join
second_thread.join
#--- post test clean up end
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 0146401abe..e125b657f2 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -242,7 +242,7 @@ module ActiveSupport #:nodoc:
# resolution deterministic for constants with the same relative name in
# different namespaces whose evaluation would depend on load order
# otherwise.
- def require_dependency(file_name, message = "No such file to load -- %s")
+ def require_dependency(file_name, message = "No such file to load -- %s.rb")
file_name = file_name.to_path if file_name.respond_to?(:to_path)
unless file_name.is_a?(String)
raise ArgumentError, "the file name must either be a String or implement #to_path -- you passed #{file_name.inspect}"
diff --git a/activesupport/lib/active_support/logger.rb b/activesupport/lib/active_support/logger.rb
index 3ba6461b57..ea09d7d2df 100644
--- a/activesupport/lib/active_support/logger.rb
+++ b/activesupport/lib/active_support/logger.rb
@@ -59,14 +59,14 @@ module ActiveSupport
define_method(:silence) do |level = Logger::ERROR, &block|
if logger.respond_to?(:silence)
logger.silence(level) do
- if respond_to?(:silence)
+ if defined?(super)
super(level, &block)
else
block.call(self)
end
end
else
- if respond_to?(:silence)
+ if defined?(super)
super(level, &block)
else
block.call(self)
diff --git a/activesupport/test/broadcast_logger_test.rb b/activesupport/test/broadcast_logger_test.rb
index d003a33471..184d0ebddd 100644
--- a/activesupport/test/broadcast_logger_test.rb
+++ b/activesupport/test/broadcast_logger_test.rb
@@ -69,6 +69,20 @@ module ActiveSupport
assert_equal ::Logger::FATAL, log2.local_level
end
+ test "#silence does not break custom loggers" do
+ new_logger = FakeLogger.new
+ custom_logger = CustomLogger.new
+ custom_logger.extend(Logger.broadcast(new_logger))
+
+ custom_logger.silence do
+ custom_logger.error "from error"
+ custom_logger.unknown "from unknown"
+ end
+
+ assert_equal [[::Logger::ERROR, "from error", nil], [::Logger::UNKNOWN, "from unknown", nil]], custom_logger.adds
+ assert_equal [[::Logger::ERROR, "from error", nil], [::Logger::UNKNOWN, "from unknown", nil]], new_logger.adds
+ end
+
test "#silence silences all loggers below the default level of ERROR" do
logger.silence do
logger.debug "test"
@@ -98,9 +112,7 @@ module ActiveSupport
assert_equal [[::Logger::FATAL, "seen", nil]], log2.adds
end
- class FakeLogger
- include LoggerSilence
-
+ class CustomLogger
attr_reader :adds, :closed, :chevrons
attr_accessor :level, :progname, :formatter, :local_level
@@ -150,5 +162,9 @@ module ActiveSupport
@closed = true
end
end
+
+ class FakeLogger < CustomLogger
+ include LoggerSilence
+ end
end
end
diff --git a/railties/test/code_statistics_calculator_test.rb b/railties/test/code_statistics_calculator_test.rb
index 8a2f0294d0..1bd4225f34 100644
--- a/railties/test/code_statistics_calculator_test.rb
+++ b/railties/test/code_statistics_calculator_test.rb
@@ -24,7 +24,7 @@ class CodeStatisticsCalculatorTest < ActiveSupport::TestCase
end
end
- test "count number of methods in MiniTest file" do
+ test "count number of methods in Minitest file" do
code = <<-RUBY
class FooTest < ActionController::TestCase
test 'expectation' do