diff options
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/Rakefile | 4 | ||||
-rwxr-xr-x | activerecord/bin/test | 2 | ||||
-rw-r--r-- | activerecord/lib/active_record/associations.rb | 12 | ||||
-rw-r--r-- | activerecord/lib/active_record/attribute_methods/primary_key.rb | 17 | ||||
-rw-r--r-- | activerecord/lib/active_record/attribute_methods/write.rb | 21 | ||||
-rw-r--r-- | activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb | 2 | ||||
-rw-r--r-- | activerecord/lib/active_record/relation/query_methods.rb | 2 | ||||
-rw-r--r-- | activerecord/test/cases/adapters/postgresql/uuid_test.rb | 3 | ||||
-rw-r--r-- | activerecord/test/cases/errors_test.rb | 2 | ||||
-rw-r--r-- | activerecord/test/cases/migration/compatibility_test.rb | 15 | ||||
-rw-r--r-- | activerecord/test/cases/primary_keys_test.rb | 23 | ||||
-rw-r--r-- | activerecord/test/cases/relation/or_test.rb | 25 | ||||
-rw-r--r-- | activerecord/test/cases/schema_dumper_test.rb | 8 | ||||
-rw-r--r-- | activerecord/test/cases/transactions_test.rb | 46 | ||||
-rw-r--r-- | activerecord/test/schema/schema.rb | 15 |
15 files changed, 145 insertions, 52 deletions
diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 2d0d5bd657..fe5f9d1071 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -1,7 +1,7 @@ require "rake/testtask" -require File.expand_path("test/config", __dir__) -require File.expand_path("test/support/config", __dir__) +require_relative "test/config" +require_relative "test/support/config" def run_without_aborting(*tasks) errors = [] diff --git a/activerecord/bin/test b/activerecord/bin/test index 3a9547e5c1..ab69f4f603 100755 --- a/activerecord/bin/test +++ b/activerecord/bin/test @@ -1,7 +1,7 @@ #!/usr/bin/env ruby COMPONENT_ROOT = File.expand_path("..", __dir__) -require File.expand_path("../tools/test", COMPONENT_ROOT) +require_relative "../../tools/test" module Minitest def self.plugin_active_record_options(opts, options) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 7c37132d3a..f05a122544 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -342,7 +342,7 @@ module ActiveRecord # | | belongs_to | # generated methods | belongs_to | :polymorphic | has_one # ----------------------------------+------------+--------------+--------- - # other(force_reload=false) | X | X | X + # other | X | X | X # other=(other) | X | X | X # build_other(attributes={}) | X | | X # create_other(attributes={}) | X | | X @@ -352,7 +352,7 @@ module ActiveRecord # | | | has_many # generated methods | habtm | has_many | :through # ----------------------------------+-------+----------+---------- - # others(force_reload=false) | X | X | X + # others | X | X | X # others=(other,other,...) | X | X | X # other_ids | X | X | X # other_ids=(id,id,...) | X | X | X @@ -1187,7 +1187,7 @@ module ActiveRecord # +collection+ is a placeholder for the symbol passed as the +name+ argument, so # <tt>has_many :clients</tt> would add among others <tt>clients.empty?</tt>. # - # [collection(force_reload = false)] + # [collection] # Returns an array of all the associated objects. # An empty array is returned if none are found. # [collection<<(object, ...)] @@ -1407,7 +1407,7 @@ module ActiveRecord # +association+ is a placeholder for the symbol passed as the +name+ argument, so # <tt>has_one :manager</tt> would add among others <tt>manager.nil?</tt>. # - # [association(force_reload = false)] + # [association] # Returns the associated object. +nil+ is returned if none is found. # [association=(associate)] # Assigns the associate object, extracts the primary key, sets it as the foreign key, @@ -1539,7 +1539,7 @@ module ActiveRecord # +association+ is a placeholder for the symbol passed as the +name+ argument, so # <tt>belongs_to :author</tt> would add among others <tt>author.nil?</tt>. # - # [association(force_reload = false)] + # [association] # Returns the associated object. +nil+ is returned if none is found. # [association=(associate)] # Assigns the associate object, extracts the primary key, and sets it as the foreign key. @@ -1701,7 +1701,7 @@ module ActiveRecord # +collection+ is a placeholder for the symbol passed as the +name+ argument, so # <tt>has_and_belongs_to_many :categories</tt> would add among others <tt>categories.empty?</tt>. # - # [collection(force_reload = false)] + # [collection] # Returns an array of all the associated objects. # An empty array is returned if none are found. # [collection<<(object, ...)] diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 2f32caa257..b9b2acff37 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -8,17 +8,14 @@ module ActiveRecord # Returns this record's primary key value wrapped in an array if one is # available. def to_key - sync_with_transaction_state key = id [key] if key end # Returns the primary key value. def id - if pk = self.class.primary_key - sync_with_transaction_state - _read_attribute(pk) - end + sync_with_transaction_state + _read_attribute(self.class.primary_key) if self.class.primary_key end # Sets the primary key value. @@ -57,16 +54,12 @@ module ActiveRecord end module ClassMethods - def define_method_attribute(attr_name) - super + ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was id_in_database).to_set - if attr_name == primary_key && attr_name != "id" - generated_attribute_methods.send(:alias_method, :id, primary_key) - end + def instance_method_already_implemented?(method_name) + super || primary_key && ID_ATTRIBUTE_METHODS.include?(method_name) end - ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was id_in_database).to_set - def dangerous_attribute_method?(method_name) super && !ID_ATTRIBUTE_METHODS.include?(method_name) end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index fe0e01db28..75c5a1a600 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -35,11 +35,15 @@ module ActiveRecord attr_name.to_s end - write_attribute_with_type_cast(name, value, true) + name = self.class.primary_key if name == "id".freeze && self.class.primary_key + @attributes.write_from_user(name, value) + value end def raw_write_attribute(attr_name, value) # :nodoc: - write_attribute_with_type_cast(attr_name, value, false) + name = attr_name.to_s + @attributes.write_cast_value(name, value) + value end private @@ -47,19 +51,6 @@ module ActiveRecord def attribute=(attribute_name, value) write_attribute(attribute_name, value) end - - def write_attribute_with_type_cast(attr_name, value, should_type_cast) - attr_name = attr_name.to_s - attr_name = self.class.primary_key if attr_name == "id" && self.class.primary_key - - if should_type_cast - @attributes.write_from_user(attr_name, value) - else - @attributes.write_cast_value(attr_name, value) - end - - value - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index a1031ccbf5..22d7791dec 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1010,7 +1010,7 @@ module ActiveRecord def dump_schema_information #:nodoc: versions = ActiveRecord::SchemaMigration.all_versions - insert_versions_sql(versions) + insert_versions_sql(versions) if versions.any? end def initialize_schema_migrations_table # :nodoc: diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 79e65baae5..6ccdd7adcb 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1167,7 +1167,7 @@ module ActiveRecord end end - STRUCTURAL_OR_METHODS = Relation::VALUE_METHODS - [:extending, :where, :having] + STRUCTURAL_OR_METHODS = Relation::VALUE_METHODS - [:extending, :where, :having, :unscope] def structurally_incompatible_values_for_or(other) STRUCTURAL_OR_METHODS.reject do |method| get_value(method) == other.get_value(method) diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index d124b64861..8eddd81c38 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -40,7 +40,8 @@ class PostgresqlUUIDTest < ActiveRecord::PostgreSQLTestCase drop_table "uuid_data_type" end - if ActiveRecord::Base.connection.supports_pgcrypto_uuid? + if ActiveRecord::Base.connection.respond_to?(:supports_pgcrypto_uuid?) && + 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 diff --git a/activerecord/test/cases/errors_test.rb b/activerecord/test/cases/errors_test.rb index 73feb831d0..e90669e0c7 100644 --- a/activerecord/test/cases/errors_test.rb +++ b/activerecord/test/cases/errors_test.rb @@ -1,4 +1,4 @@ -require_relative "../cases/helper" +require "cases/helper" class ErrorsTest < ActiveRecord::TestCase def test_can_be_instantiated_with_no_args diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 7a80bfb899..596a21dcbc 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -90,6 +90,21 @@ module ActiveRecord connection.drop_table :more_testings rescue nil end + def test_timestamps_have_null_constraints_if_not_present_in_migration_of_change_table + migration = Class.new(ActiveRecord::Migration[4.2]) { + def migrate(x) + change_table :testings do |t| + t.timestamps + end + end + }.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + assert connection.columns(:testings).find { |c| c.name == "created_at" }.null + assert connection.columns(:testings).find { |c| c.name == "updated_at" }.null + end + def test_timestamps_have_null_constraints_if_not_present_in_migration_for_adding_timestamps_to_existing_table migration = Class.new(ActiveRecord::Migration[4.2]) { def migrate(x) diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 5ded619716..56229b70bc 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -46,7 +46,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase topic = Topic.new topic.title = "New Topic" assert_nil topic.id - assert_nothing_raised { topic.save! } + topic.save! id = topic.id topicReloaded = Topic.find(id) @@ -56,23 +56,36 @@ class PrimaryKeysTest < ActiveRecord::TestCase def test_customized_primary_key_auto_assigns_on_save Keyboard.delete_all keyboard = Keyboard.new(name: "HHKB") - assert_nothing_raised { keyboard.save! } + keyboard.save! assert_equal keyboard.id, Keyboard.find_by_name("HHKB").id end def test_customized_primary_key_can_be_get_before_saving keyboard = Keyboard.new assert_nil keyboard.id - assert_nothing_raised { assert_nil keyboard.key_number } + assert_nil keyboard.key_number end def test_customized_string_primary_key_settable_before_save subscriber = Subscriber.new - assert_nothing_raised { subscriber.id = "webster123" } + subscriber.id = "webster123" assert_equal "webster123", subscriber.id assert_equal "webster123", subscriber.nick end + def test_update_with_non_primary_key_id_column + subscriber = Subscriber.first + subscriber.update(update_count: 1) + subscriber.reload + assert_equal 1, subscriber.update_count + end + + def test_update_columns_with_non_primary_key_id_column + subscriber = Subscriber.first + subscriber.update_columns(id: 1) + assert_not_equal 1, subscriber.nick + end + def test_string_key subscriber = Subscriber.find(subscribers(:first).nick) assert_equal(subscribers(:first).name, subscriber.name) @@ -83,7 +96,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase subscriber.id = "jdoe" assert_equal("jdoe", subscriber.id) subscriber.name = "John Doe" - assert_nothing_raised { subscriber.save! } + subscriber.save! assert_equal("jdoe", subscriber.id) subscriberReloaded = Subscriber.find("jdoe") diff --git a/activerecord/test/cases/relation/or_test.rb b/activerecord/test/cases/relation/or_test.rb index abb7ca72dd..61b6601580 100644 --- a/activerecord/test/cases/relation/or_test.rb +++ b/activerecord/test/cases/relation/or_test.rb @@ -59,6 +59,31 @@ module ActiveRecord assert_equal "Relation passed to #or must be structurally compatible. Incompatible values: [:order]", error.message end + def test_or_with_unscope_where + expected = Post.where("id = 1 or id = 2") + partial = Post.where("id = 1 and id != 2") + assert_equal expected, partial.or(partial.unscope(:where).where("id = 2")).to_a + end + + def test_or_with_unscope_where_column + expected = Post.where("id = 1 or id = 2") + partial = Post.where(id: 1).where.not(id: 2) + assert_equal expected, partial.or(partial.unscope(where: :id).where("id = 2")).to_a + end + + def test_or_with_unscope_order + expected = Post.where("id = 1 or id = 2") + assert_equal expected, Post.order("body asc").where("id = 1").unscope(:order).or(Post.where("id = 2")).to_a + end + + def test_or_with_incompatible_unscope + error = assert_raises ArgumentError do + Post.order("body asc").where("id = 1").or(Post.order("body asc").where("id = 2").unscope(:order)).to_a + end + + assert_equal "Relation passed to #or must be structurally compatible. Incompatible values: [:order]", error.message + end + def test_or_when_grouping groups = Post.where("id < 10").group("body").select("body, COUNT(*) AS c") expected = groups.having("COUNT(*) > 1 OR body like 'Such%'").to_a.map { |o| [o.body, o.c] } diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 417c6f4832..4c81e825fa 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -17,6 +17,12 @@ class SchemaDumperTest < ActiveRecord::TestCase dump_all_table_schema [] end + def test_dump_schema_information_with_empty_versions + ActiveRecord::SchemaMigration.delete_all + schema_info = ActiveRecord::Base.connection.dump_schema_information + assert_no_match(/INSERT INTO/, schema_info) + end + def test_dump_schema_information_outputs_lexically_ordered_versions versions = %w{ 20100101010101 20100201010101 20100301010101 } versions.reverse_each do |v| @@ -320,7 +326,7 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dump_keeps_id_false_when_id_is_false_and_unique_not_null_column_added output = standard_dump - assert_match %r{create_table "subscribers", id: false}, output + assert_match %r{create_table "string_key_objects", id: false}, output end if ActiveRecord::Base.connection.supports_foreign_keys? diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 5c6d78b574..79ba306ef5 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -595,6 +595,52 @@ class TransactionTest < ActiveRecord::TestCase assert_not topic.frozen? end + def test_restore_id_after_rollback + topic = Topic.new + + Topic.transaction do + topic.save! + raise ActiveRecord::Rollback + end + + assert_nil topic.id + end + + def test_restore_custom_primary_key_after_rollback + movie = Movie.new(name: "foo") + + Movie.transaction do + movie.save! + raise ActiveRecord::Rollback + end + + assert_nil movie.id + end + + def test_assign_id_after_rollback + topic = Topic.create! + + Topic.transaction do + topic.save! + raise ActiveRecord::Rollback + end + + topic.id = nil + assert_nil topic.id + end + + def test_assign_custom_primary_key_after_rollback + movie = Movie.create!(name: "foo") + + Movie.transaction do + movie.save! + raise ActiveRecord::Rollback + end + + movie.id = nil + assert_nil movie.id + end + def test_rollback_of_frozen_records topic = Topic.create.freeze Topic.transaction do diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 50f1d9bfe7..dc8bdb98ff 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -807,16 +807,19 @@ ActiveRecord::Schema.define do t.string :sponsorable_type end - create_table :string_key_objects, id: false, primary_key: :id, force: true do |t| - t.string :id - t.string :name - t.integer :lock_version, null: false, default: 0 + create_table :string_key_objects, id: false, force: true do |t| + t.string :id, null: false + t.string :name + t.integer :lock_version, null: false, default: 0 + t.index :id, unique: true end - create_table :subscribers, force: true, id: false do |t| + create_table :subscribers, id: false, force: true do |t| t.string :nick, null: false t.string :name - t.column :books_count, :integer, null: false, default: 0 + t.integer :id + t.integer :books_count, null: false, default: 0 + t.integer :update_count, null: false, default: 0 t.index :nick, unique: true end |