diff options
Diffstat (limited to 'activerecord')
| -rw-r--r-- | activerecord/CHANGELOG.md | 20 | ||||
| -rw-r--r-- | activerecord/lib/active_record/connection_adapters/abstract/transaction.rb | 9 | ||||
| -rw-r--r-- | activerecord/lib/active_record/gem_version.rb | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/internal_metadata.rb | 13 | ||||
| -rw-r--r-- | activerecord/lib/active_record/model_schema.rb | 4 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/finder_methods.rb | 39 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_test.rb | 32 | ||||
| -rw-r--r-- | activerecord/test/cases/migration_test.rb | 25 | ||||
| -rw-r--r-- | activerecord/test/cases/schema_dumper_test.rb | 8 | ||||
| -rw-r--r-- | activerecord/test/cases/transaction_callbacks_test.rb | 26 |
10 files changed, 105 insertions, 73 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index faab0f9554..3b67c6f495 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,22 +1,4 @@ -* Rework `ActiveRecord::Relation#last` - - 1. Always find last with ruby if relation is loaded - 2. Always use SQL instead if relation is not loaded. - 3. Deprecated relation loading when SQL order can not be automatically reversed - - Topic.order("title").load.last(3) - # before: SELECT ... - # after: No SQL - - Topic.order("title").last - # before: SELECT * FROM `topics` - # after: SELECT * FROM `topics` ORDER BY `topics`.`title` DESC LIMIT 1 - - Topic.order("coalesce(author, title)").last - # before: SELECT * FROM `topics` - # after: Deprecation Warning for irreversible order - - *Bogdan Gusiev* +## Rails 5.0.0.beta2 (February 01, 2016) ## * `ActiveRecord::Relation#reverse_order` throws `ActiveRecord::IrreversibleOrderError` when the order can not be reversed using current trivial algorithm. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 14d04a6388..6ecdab6eb0 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -167,8 +167,13 @@ module ActiveRecord def commit_transaction transaction = @stack.last - transaction.before_commit_records - @stack.pop + + begin + transaction.before_commit_records + ensure + @stack.pop + end + transaction.commit transaction.commit_records end diff --git a/activerecord/lib/active_record/gem_version.rb b/activerecord/lib/active_record/gem_version.rb index b4f2f66e1c..aa1f5c4fb4 100644 --- a/activerecord/lib/active_record/gem_version.rb +++ b/activerecord/lib/active_record/gem_version.rb @@ -8,7 +8,7 @@ module ActiveRecord MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta1.1" + PRE = "beta2" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/activerecord/lib/active_record/internal_metadata.rb b/activerecord/lib/active_record/internal_metadata.rb index cb4b1fc47c..81db96bffd 100644 --- a/activerecord/lib/active_record/internal_metadata.rb +++ b/activerecord/lib/active_record/internal_metadata.rb @@ -14,6 +14,10 @@ module ActiveRecord "#{table_name_prefix}#{ActiveRecord::Base.internal_metadata_table_name}#{table_name_suffix}" end + def original_table_name + "#{table_name_prefix}active_record_internal_metadatas#{table_name_suffix}" + end + def []=(key, value) first_or_initialize(key: key).update_attributes!(value: value) end @@ -26,8 +30,17 @@ module ActiveRecord ActiveSupport::Deprecation.silence { connection.table_exists?(table_name) } end + def original_table_exists? + # This method will be removed in Rails 5.1 + # Since it is only necessary when `active_record_internal_metadatas` could exist + ActiveSupport::Deprecation.silence { connection.table_exists?(original_table_name) } + end + # Creates an internal metadata table with columns +key+ and +value+ def create_table + if original_table_exists? + connection.rename_table(original_table_name, table_name) + end unless table_exists? key_options = connection.internal_string_options_for_primary_key diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 722d7b5fce..ee52c3ae02 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -44,9 +44,9 @@ module ActiveRecord ## # :singleton-method: - # Accessor for the name of the internal metadata table. By default, the value is "active_record_internal_metadatas" + # Accessor for the name of the internal metadata table. By default, the value is "ar_internal_metadata" class_attribute :internal_metadata_table_name, instance_accessor: false - self.internal_metadata_table_name = "active_record_internal_metadatas" + self.internal_metadata_table_name = "ar_internal_metadata" ## # :singleton-method: diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 5d4a045097..3f5d6de78a 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -145,23 +145,15 @@ module ActiveRecord # # [#<Person id:4>, #<Person id:3>, #<Person id:2>] def last(limit = nil) - return find_last(limit) if loaded? - - result = order_values.empty? && primary_key ? order(arel_table[primary_key].desc) : reverse_order - result = result.limit!(limit || 1) - limit ? result.reverse : result.first - rescue ActiveRecord::IrreversibleOrderError - ActiveSupport::Deprecation.warn(<<-WARNING.squish) - Finding a last element by loading the relation when SQL ORDER - can not be reversed is deprecated. - Rails 5.1 will raise ActiveRecord::IrreversibleOrderError in this case. - Please call `to_a.last` if you still want to load the relation. - WARNING - find_last(limit) - end - - def find_last(limit) - limit ? to_a.last(limit) : to_a.last + if limit + if order_values.empty? && primary_key + order(arel_table[primary_key].desc).limit(limit).reverse + else + to_a.last(limit) + end + else + find_last + end end # Same as #last but raises ActiveRecord::RecordNotFound if no record @@ -531,6 +523,19 @@ module ActiveRecord relation.limit(limit).to_a end + def find_last + if loaded? + @records.last + else + @last ||= + if limit_value + to_a.last + else + reverse_order.limit(1).to_a.first + end + end + end + private def find_nth_with_limit_and_offset(index, limit, offset:) # :nodoc: diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index cbeb37dd22..75a74c052d 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -506,7 +506,7 @@ class FinderTest < ActiveRecord::TestCase end end - def test_take_and_first_and_last_with_integer_should_use_sql + def test_take_and_first_and_last_with_integer_should_use_sql_limit assert_sql(/LIMIT|ROWNUM <=/) { Topic.take(3).entries } assert_sql(/LIMIT|ROWNUM <=/) { Topic.first(2).entries } assert_sql(/LIMIT|ROWNUM <=/) { Topic.last(5).entries } @@ -516,30 +516,16 @@ class FinderTest < ActiveRecord::TestCase assert_equal Topic.order("title").to_a.last(2), Topic.order("title").last(2) end - def test_last_with_integer_and_order_should_use_sql - relation = Topic.order("title") - assert_queries(1) { relation.last(5) } - assert !relation.loaded? + def test_last_with_integer_and_order_should_not_use_sql_limit + query = assert_sql { Topic.order("title").last(5).entries } + assert_equal 1, query.length + assert_no_match(/LIMIT/, query.first) end - def test_last_with_integer_and_reorder_should_use_sql - relation = Topic.reorder("title") - assert_queries(1) { relation.last(5) } - assert !relation.loaded? - end - - def test_last_on_loaded_relation_should_not_use_sql - relation = Topic.limit(10).load - assert_no_queries do - relation.last - relation.last(2) - end - end - - def test_last_with_irreversible_order - assert_deprecated do - Topic.order("coalesce(author_name, title)").last - end + def test_last_with_integer_and_reorder_should_not_use_sql_limit + query = assert_sql { Topic.reorder("title").last(5).entries } + assert_equal 1, query.length + assert_no_match(/LIMIT/, query.first) end def test_take_and_first_and_last_with_integer_should_return_an_array diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index f51e366b1d..9b4394377f 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -357,14 +357,14 @@ class MigrationTest < ActiveRecord::TestCase def test_internal_metadata_table_name original_internal_metadata_table_name = ActiveRecord::Base.internal_metadata_table_name - assert_equal "active_record_internal_metadatas", ActiveRecord::InternalMetadata.table_name - ActiveRecord::Base.table_name_prefix = "prefix_" - ActiveRecord::Base.table_name_suffix = "_suffix" + assert_equal "ar_internal_metadata", ActiveRecord::InternalMetadata.table_name + ActiveRecord::Base.table_name_prefix = "p_" + ActiveRecord::Base.table_name_suffix = "_s" Reminder.reset_table_name - assert_equal "prefix_active_record_internal_metadatas_suffix", ActiveRecord::InternalMetadata.table_name + assert_equal "p_ar_internal_metadata_s", ActiveRecord::InternalMetadata.table_name ActiveRecord::Base.internal_metadata_table_name = "changed" Reminder.reset_table_name - assert_equal "prefix_changed_suffix", ActiveRecord::InternalMetadata.table_name + assert_equal "p_changed_s", ActiveRecord::InternalMetadata.table_name ActiveRecord::Base.table_name_prefix = "" ActiveRecord::Base.table_name_suffix = "" Reminder.reset_table_name @@ -426,6 +426,21 @@ class MigrationTest < ActiveRecord::TestCase ENV["RACK_ENV"] = original_rack_env end + def test_rename_internal_metadata_table + original_internal_metadata_table_name = ActiveRecord::Base.internal_metadata_table_name + + ActiveRecord::Base.internal_metadata_table_name = "active_record_internal_metadatas" + Reminder.reset_table_name + + ActiveRecord::Base.internal_metadata_table_name = original_internal_metadata_table_name + Reminder.reset_table_name + + assert_equal "ar_internal_metadata", ActiveRecord::InternalMetadata.table_name + ensure + ActiveRecord::Base.internal_metadata_table_name = original_internal_metadata_table_name + Reminder.reset_table_name + end + def test_proper_table_name_on_migration reminder_class = new_isolated_reminder_class migration = ActiveRecord::Migration.new diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 7b93d20e05..25f4a69ad1 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -38,7 +38,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_match %r{create_table "accounts"}, output assert_match %r{create_table "authors"}, output assert_no_match %r{create_table "schema_migrations"}, output - assert_no_match %r{create_table "active_record_internal_metadatas"}, output + assert_no_match %r{create_table "ar_internal_metadata"}, output end def test_schema_dump_uses_force_cascade_on_create_table @@ -159,7 +159,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_no_match %r{create_table "accounts"}, output assert_match %r{create_table "authors"}, output assert_no_match %r{create_table "schema_migrations"}, output - assert_no_match %r{create_table "active_record_internal_metadatas"}, output + assert_no_match %r{create_table "ar_internal_metadata"}, output end def test_schema_dump_with_regexp_ignored_table @@ -167,7 +167,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_no_match %r{create_table "accounts"}, output assert_match %r{create_table "authors"}, output assert_no_match %r{create_table "schema_migrations"}, output - assert_no_match %r{create_table "active_record_internal_metadatas"}, output + assert_no_match %r{create_table "ar_internal_metadata"}, output end def test_schema_dumps_index_columns_in_right_order @@ -345,7 +345,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_no_match %r{create_table "foo_.+_bar"}, output assert_no_match %r{add_index "foo_.+_bar"}, output assert_no_match %r{create_table "schema_migrations"}, output - assert_no_match %r{create_table "active_record_internal_metadatas"}, output + assert_no_match %r{create_table "ar_internal_metadata"}, output if ActiveRecord::Base.connection.supports_foreign_keys? assert_no_match %r{add_foreign_key "foo_.+_bar"}, output diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 637f89196e..8a7f19293d 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -34,6 +34,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase has_many :replies, class_name: "ReplyWithCallbacks", foreign_key: "parent_id" + before_commit { |record| record.do_before_commit(nil) } after_commit { |record| record.do_after_commit(nil) } after_create_commit { |record| record.do_after_commit(:create) } after_update_commit { |record| record.do_after_commit(:update) } @@ -47,6 +48,12 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @history ||= [] end + def before_commit_block(on = nil, &block) + @before_commit ||= {} + @before_commit[on] ||= [] + @before_commit[on] << block + end + def after_commit_block(on = nil, &block) @after_commit ||= {} @after_commit[on] ||= [] @@ -59,6 +66,11 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @after_rollback[on] << block end + def do_before_commit(on) + blocks = @before_commit[on] if defined?(@before_commit) + blocks.each{|b| b.call(self)} if blocks + end + def do_after_commit(on) blocks = @after_commit[on] if defined?(@after_commit) blocks.each{|b| b.call(self)} if blocks @@ -74,6 +86,20 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @first = TopicWithCallbacks.find(1) end + # FIXME: Test behavior, not implementation. + def test_before_commit_exception_should_pop_transaction_stack + @first.before_commit_block { raise 'better pop this txn from the stack!' } + + original_txn = @first.class.connection.current_transaction + + begin + @first.save! + fail + rescue + assert_equal original_txn, @first.class.connection.current_transaction + end + end + def test_call_after_commit_after_transaction_commits @first.after_commit_block{|r| r.history << :after_commit} @first.after_rollback_block{|r| r.history << :after_rollback} |