diff options
Diffstat (limited to 'activerecord')
20 files changed, 217 insertions, 56 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a8bed82e19..35f7c158a6 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,36 @@ +* 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. + + *Yuji Yaginuma*, *Yaw Boakye* + +* Introduce `Model#reload_<association>` to bring back the behavior + of `Article.category(true)` where `category` is a singular + association. + + The force reloading of the association reader was deprecated in + #20888. Unfortunately the suggested alternative of + `article.reload.category` does not expose the same behavior. + + This patch adds a reader method with the prefix `reload_` for + singular associations. This method has the same semantics as + passing true to the association reader used to have. + + *Yves Senn* + +* Make sure eager loading `ActiveRecord::Associations` also loads + constants defined in `ActiveRecord::Associations::Preloader`. + + *Yves Senn* + * Allow `ActionController::Parameters`-like objects to be passed as values for Postgres HStore columns. diff --git a/activerecord/bin/test b/activerecord/bin/test index 23add35d45..3a9547e5c1 100755 --- a/activerecord/bin/test +++ b/activerecord/bin/test @@ -17,5 +17,3 @@ module Minitest end Minitest.extensions.unshift "active_record" - -exit Minitest.run(ARGV) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 3c94c4bd7f..19308643f3 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -224,6 +224,11 @@ module ActiveRecord autoload :AliasTracker end + def self.eager_load! + super + Preloader.eager_load! + end + # Returns the association instance for the given name, instantiating it if it doesn't already exist def association(name) #:nodoc: association = association_instance_get(name) diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index bb96202a22..7732b63af6 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -8,7 +8,16 @@ module ActiveRecord::Associations::Builder # :nodoc: def self.define_accessors(model, reflection) super - define_constructors(model.generated_association_methods, reflection.name) if reflection.constructable? + mixin = model.generated_association_methods + name = reflection.name + + define_constructors(mixin, name) if reflection.constructable? + + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 + def reload_#{name} + association(:#{name}).force_reload_reader + end + CODE end # Defines the (build|create)_association methods for belongs_to or has_one association 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/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index e386cc0e4c..1953cc6a72 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -30,6 +30,13 @@ module ActiveRecord record end + # Implements the reload reader method, e.g. foo.reload_bar for + # Foo.has_one :bar + def force_reload_reader + klass.uncached { reload } + target + end + private def create_scope diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb index a11dbe7dce..5d689c2dc3 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb @@ -11,11 +11,12 @@ module ActiveRecord # t.timestamps # end # - # By default, this will use the +uuid_generate_v4()+ function from the - # +uuid-ossp+ extension, which MUST be enabled on your database. To enable - # the +uuid-ossp+ extension, you can use the +enable_extension+ method in your - # migrations. To use a UUID primary key without +uuid-ossp+ enabled, you can - # set the +:default+ option to +nil+: + # By default, this will use the +gen_random_uuid()+ function from the + # +pgcrypto+ extension (only PostgreSQL >= 9.4), or +uuid_generate_v4()+ + # function from the +uuid-ossp+ extension. To enable the appropriate + # extension, which is a requirement, you can use the +enable_extension+ + # method in your migrations. To use a UUID primary key without any of + # of extensions, you can set the +:default+ option to +nil+: # # create_table :stuffs, id: false do |t| # t.primary_key :id, :uuid, default: nil @@ -23,15 +24,15 @@ module ActiveRecord # t.timestamps # end # - # You may also pass a different UUID generation function from +uuid-ossp+ - # or another library. + # You may also pass a custom stored procedure that returns a UUID or use a + # different UUID generation function from another library. # # Note that setting the UUID primary key default value to +nil+ will # require you to assure that you always provide a UUID value before saving # a record (as primary keys cannot be +nil+). This might be done via the # +SecureRandom.uuid+ method and a +before_save+ callback, for instance. def primary_key(name, type = :primary_key, **options) - options[:default] = options.fetch(:default, "uuid_generate_v4()") if type == :uuid + options[:default] = options.fetch(:default, "gen_random_uuid()") if type == :uuid super end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 710b5cd887..140ad4827a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -315,6 +315,10 @@ module ActiveRecord postgresql_version >= 90300 end + def supports_pgcrypto_uuid? + postgresql_version >= 90400 + end + def get_advisory_lock(lock_id) # :nodoc: unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63 raise(ArgumentError, "Postgres requires advisory lock ids to be a signed 64 bit integer") diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 04e538baa5..ae45ac7157 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -103,6 +103,14 @@ module ActiveRecord end class V5_0 < V5_1 + def create_table(table_name, options = {}) + if ActiveRecord::Base.connection.adapter_name == "PostgreSQL" + if options[:id] == :uuid && !options[:default] + options[:default] = "uuid_generate_v4()" + end + end + super + end end class V4_2 < V5_0 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/prepared_statements_disabled_test.rb b/activerecord/test/cases/adapters/postgresql/prepared_statements_disabled_test.rb new file mode 100644 index 0000000000..8c62690866 --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/prepared_statements_disabled_test.rb @@ -0,0 +1,25 @@ +require "cases/helper" +require "models/computer" +require "models/developer" + +class PreparedStatementsDisabledTest < ActiveRecord::PostgreSQLTestCase + fixtures :developers + + def setup + @conn = ActiveRecord::Base.establish_connection :arunit_without_prepared_statements + end + + def teardown + @conn.release_connection + ActiveRecord::Base.establish_connection :arunit + end + + def test_select_query_works_even_when_prepared_statements_are_disabled + assert_not Developer.connection.prepared_statements + + david = developers(:david) + + assert_equal david, Developer.where(name: "David").last # With Binds + assert_operator Developer.count, :>, 0 # Without Binds + end +end diff --git a/activerecord/test/cases/adapters/postgresql/prepared_statements_test.rb b/activerecord/test/cases/adapters/postgresql/prepared_statements_test.rb deleted file mode 100644 index 181c1a097c..0000000000 --- a/activerecord/test/cases/adapters/postgresql/prepared_statements_test.rb +++ /dev/null @@ -1,22 +0,0 @@ -require "cases/helper" -require "models/computer" -require "models/developer" - -class PreparedStatementsTest < ActiveRecord::PostgreSQLTestCase - fixtures :developers - - def setup - @default_prepared_statements = ActiveRecord::Base.connection.instance_variable_get("@prepared_statements") - ActiveRecord::Base.connection.instance_variable_set("@prepared_statements", false) - end - - def teardown - ActiveRecord::Base.connection.instance_variable_set("@prepared_statements", @default_prepared_statements) - end - - def test_nothing_raised_with_falsy_prepared_statements - assert_nothing_raised do - Developer.where(id: 1) - end - end -end diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index 9a59691737..ab0815631f 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -9,6 +9,10 @@ module PostgresqlUUIDHelper def drop_table(name) connection.drop_table name, if_exists: true end + + def uuid_function + connection.supports_pgcrypto_uuid? ? "gen_random_uuid()" : "uuid_generate_v4()" + end end class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase @@ -21,6 +25,7 @@ class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase setup do enable_extension!("uuid-ossp", connection) + enable_extension!("pgcrypto", connection) if connection.supports_pgcrypto_uuid? connection.create_table "uuid_data_type" do |t| t.uuid "guid" @@ -31,19 +36,27 @@ class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase drop_table "uuid_data_type" 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 + if ActiveRecord::Base.connection.supports_pgcrypto_uuid? + def test_uuid_column_default + connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "gen_random_uuid()" + UUIDType.reset_column_information + 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 + + 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 end def test_data_type_of_uuid_types @@ -155,7 +168,7 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase # to test dumping tables which columns have defaults with custom functions connection.execute <<-SQL CREATE OR REPLACE FUNCTION my_uuid_generator() RETURNS uuid - AS $$ SELECT * FROM uuid_generate_v4() $$ + AS $$ SELECT * FROM #{uuid_function} $$ LANGUAGE SQL VOLATILE; SQL @@ -164,11 +177,16 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase t.string "name" t.uuid "other_uuid_2", default: "my_uuid_generator()" end + + connection.create_table("pg_uuids_3", id: :uuid) do |t| + t.string "name" + end end teardown do drop_table "pg_uuids" drop_table "pg_uuids_2" + drop_table "pg_uuids_3" connection.execute "DROP FUNCTION IF EXISTS my_uuid_generator();" end @@ -206,6 +224,33 @@ class PostgresqlUUIDGenerationTest < ActiveRecord::PostgreSQLTestCase assert_match(/\bcreate_table "pg_uuids_2", id: :uuid, default: -> { "my_uuid_generator\(\)" }/, schema) assert_match(/t\.uuid "other_uuid_2", default: -> { "my_uuid_generator\(\)" }/, schema) end + + def test_schema_dumper_for_uuid_primary_key_default + schema = dump_table_schema "pg_uuids_3" + if connection.supports_pgcrypto_uuid? + assert_match(/\bcreate_table "pg_uuids_3", id: :uuid, default: -> { "gen_random_uuid\(\)" }/, schema) + else + assert_match(/\bcreate_table "pg_uuids_3", id: :uuid, default: -> { "uuid_generate_v4\(\)" }/, schema) + end + end + + if ActiveRecord::Base.connection.supports_pgcrypto_uuid? + def test_schema_dumper_for_uuid_primary_key_default_in_legacy_migration + migration = Class.new(ActiveRecord::Migration[4.2]) do + def version; 101 end + def migrate(x) + create_table("pg_uuids_4", id: :uuid) + end + end.new + ActiveRecord::Migrator.new(:up, [migration]).migrate + + schema = dump_table_schema "pg_uuids_4" + assert_match(/\bcreate_table "pg_uuids_4", id: :uuid, default: -> { "uuid_generate_v4\(\)" }/, schema) + ensure + drop_table "pg_uuids_4" + end + else + end end end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 6b7e4fee56..72f1b3b125 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -291,6 +291,16 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert client.account.new_record? end + def test_reloading_the_belonging_object + odegy_account = accounts(:odegy_account) + + assert_equal "Odegy", odegy_account.firm.name + Company.where(id: odegy_account.firm_id).update_all(name: "ODEGY") + assert_equal "Odegy", odegy_account.firm.name + + assert_equal "ODEGY", odegy_account.reload_firm.name + end + def test_natural_assignment_to_nil client = Client.find(3) client.firm = nil 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/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 862f33a1a0..48fbc5990c 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -326,6 +326,16 @@ class HasOneAssociationsTest < ActiveRecord::TestCase end end + def test_reload_association + odegy = companies(:odegy) + + assert_equal 53, odegy.account.credit_limit + Account.where(id: odegy.account.id).update_all(credit_limit: 80) + assert_equal 53, odegy.account.credit_limit + + assert_equal 80, odegy.reload_account.credit_limit + end + def test_build firm = Firm.new("name" => "GlobalMegaCorp") firm.save diff --git a/activerecord/test/config.example.yml b/activerecord/test/config.example.yml index 58e2d45748..4bcb2aeea6 100644 --- a/activerecord/test/config.example.yml +++ b/activerecord/test/config.example.yml @@ -77,6 +77,9 @@ connections: postgresql: arunit: min_messages: warning + arunit_without_prepared_statements: + min_messages: warning + prepared_statements: false arunit2: min_messages: warning diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index f00b858ea6..15ba2d67ab 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -1,6 +1,7 @@ ActiveRecord::Schema.define do enable_extension!("uuid-ossp", ActiveRecord::Base.connection) + enable_extension!("pgcrypto", ActiveRecord::Base.connection) if ActiveRecord::Base.connection.supports_pgcrypto_uuid? create_table :uuid_parents, id: :uuid, force: true do |t| t.string :name diff --git a/activerecord/test/support/config.rb b/activerecord/test/support/config.rb index 5817e427e3..aaff408b41 100644 --- a/activerecord/test/support/config.rb +++ b/activerecord/test/support/config.rb @@ -26,7 +26,8 @@ module ARTest def expand_config(config) config["connections"].each do |adapter, connection| - dbs = [["arunit", "activerecord_unittest"], ["arunit2", "activerecord_unittest2"]] + dbs = [["arunit", "activerecord_unittest"], ["arunit2", "activerecord_unittest2"], + ["arunit_without_prepared_statements", "activerecord_unittest"]] dbs.each do |name, dbname| unless connection[name].is_a?(Hash) connection[name] = { "database" => connection[name] } |