From bdcf70cca89df906a3510464ef46a44646fd29a3 Mon Sep 17 00:00:00 2001 From: David Chelimsky Date: Tue, 8 Jun 2010 15:18:02 -0400 Subject: [PATCH 01/35] Memoize the object returned by _view in ActionView::TestCase::Behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#4799 state:resolved] Signed-off-by: José Valim --- actionpack/lib/action_view/test_case.rb | 14 ++++++++------ actionpack/test/template/test_case_test.rb | 4 ++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 4dbbd2eb6a..15d424be74 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -131,12 +131,14 @@ module ActionView end def _view - view = ActionView::Base.new(ActionController::Base.view_paths, _assigns, @controller) - view.singleton_class.send :include, _helpers - view.singleton_class.send :include, @controller._router.url_helpers - view.singleton_class.send :delegate, :alert, :notice, :to => "request.flash" - view.output_buffer = self.output_buffer - view + @_view ||= begin + view = ActionView::Base.new(ActionController::Base.view_paths, _assigns, @controller) + view.singleton_class.send :include, _helpers + view.singleton_class.send :include, @controller._router.url_helpers + view.singleton_class.send :delegate, :alert, :notice, :to => "request.flash" + view.output_buffer = self.output_buffer + view + end end EXCLUDE_IVARS = %w{ diff --git a/actionpack/test/template/test_case_test.rb b/actionpack/test/template/test_case_test.rb index 16e5ee4f72..9b50ea8a42 100644 --- a/actionpack/test/template/test_case_test.rb +++ b/actionpack/test/template/test_case_test.rb @@ -37,6 +37,10 @@ module ActionView include SharedTests test_case = self + test "memoizes the _view" do + assert_same _view, _view + end + test "works without testing a helper module" do assert_equal 'Eloy', render('developers/developer', :developer => stub(:name => 'Eloy')) end From 0e9b9d59859efa46a82b56e0715784fa52656650 Mon Sep 17 00:00:00 2001 From: Andrew Bloomgarden Date: Tue, 1 Jun 2010 22:47:34 -0700 Subject: [PATCH 02/35] Fix ActiveRecord::Base.compute_type swallowing NoMethodError. [#4751 state:resolved] Signed-off-by: David Heinemeier Hansson --- activerecord/CHANGELOG | 2 ++ activerecord/lib/active_record/base.rb | 4 +++- activerecord/test/cases/base_test.rb | 17 +++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 348248e849..e47d291eb3 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.0.0 [beta 4] (June 8th, 2010)* +* Fixed that ActiveRecord::Base.compute_type would swallow NoMethodError #4751 [Andrew Bloomgarden, Andrew White] + * Add index length support for MySQL. #1852 [Emili Parreno, Pratik Naik] Example: diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index aa2826fb33..7cff6d9f1a 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1219,7 +1219,9 @@ module ActiveRecord #:nodoc: begin constant = candidate.constantize return constant if candidate == constant.to_s - rescue NameError + rescue NameError => e + # We don't want to swallow NoMethodError < NameError errors + raise e unless e.instance_of?(NameError) rescue ArgumentError end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 36c572b5e7..5c175de6d4 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -2334,6 +2334,23 @@ class BasicsTest < ActiveRecord::TestCase assert !Minimalistic.new.freeze.dup.frozen? end + def test_compute_type_success + assert_equal Author, ActiveRecord::Base.send(:compute_type, 'Author') + end + + def test_compute_type_nonexistent_constant + assert_raises NameError do + ActiveRecord::Base.send :compute_type, 'NonexistentModel' + end + end + + def test_compute_type_no_method_error + String.any_instance.stubs(:constantize).raises(NoMethodError) + assert_raises NoMethodError do + ActiveRecord::Base.send :compute_type, 'InvalidModel' + end + end + protected def with_env_tz(new_tz = 'US/Eastern') old_tz, ENV['TZ'] = ENV['TZ'], new_tz From 03be27092bf51e78d0d3f3ee7fd0213023d43e0d Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 8 Jun 2010 16:20:46 -0400 Subject: [PATCH 03/35] Revert "Add shallow routes to the new router" for now. Needs more work. This reverts commit 67a60ee314f53abcde78f8ecd2a1f7c9ef8264e1. --- actionpack/CHANGELOG | 10 ----- .../lib/action_dispatch/routing/mapper.rb | 29 ++------------ actionpack/test/dispatch/routing_test.rb | 39 ------------------- 3 files changed, 3 insertions(+), 75 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index c3609958bc..1796d11dcd 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,15 +1,5 @@ *Rails 3.0.0 [beta 4] (June 8th, 2010)* -* Add shallow routes back to the new router [Diego Carrion] - - resources :posts do - shallow do - resources :comments - end - end - - You can now use comment_path for /comments/1 instead of post_comment_path for /posts/1/comments/1. - * Remove middleware laziness [José Valim] * Make session stores rely on request.cookie_jar and change set_session semantics to return the cookie value instead of a boolean. [José Valim] diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index e91a72cbe5..6db8d1aacc 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -350,10 +350,6 @@ module ActionDispatch scope(:constraints => constraints) { yield } end - def shallow - scope(:shallow => true) { yield } - end - def defaults(defaults = {}) scope(:defaults => defaults) { yield } end @@ -378,21 +374,12 @@ module ActionDispatch @scope_options ||= private_methods.grep(/^merge_(.+)_scope$/) { $1.to_sym } end - def merge_shallow_scope(parent, child) - parent or child - end - def merge_path_scope(parent, child) - parent_path = (@scope[:shallow] and child.eql?(':id')) ? parent.split('/').last : parent - Mapper.normalize_path "#{parent_path}/#{child}" + Mapper.normalize_path("#{parent}/#{child}") end def merge_name_prefix_scope(parent, child) - if @scope[:shallow] - child - else - parent ? "#{parent}_#{child}" : child - end + parent ? "#{parent}_#{child}" : child end def merge_module_scope(parent, child) @@ -535,10 +522,6 @@ module ActionDispatch options["#{singular}_id".to_sym] = id_constraint if id_constraint? options end - - def shallow? - options[:shallow] - end end class SingletonResource < Resource #:nodoc: @@ -620,12 +603,8 @@ module ActionDispatch resource = Resource.new(resources.pop, options) - scope(:path => resource.path, :controller => resource.controller, :shallow => resource.shallow?) do + scope(:path => resource.path, :controller => resource.controller) do with_scope_level(:resources, resource) do - if @scope[:shallow] && @scope[:name_prefix] - @scope[:path] = "/#{@scope[:name_prefix].pluralize}/:#{@scope[:name_prefix]}_id/#{resource.path}" - end - yield if block_given? with_scope_level(:collection) do @@ -639,8 +618,6 @@ module ActionDispatch with_scope_level(:member) do scope(':id') do scope(resource.options) do - @scope[:name_prefix] = nil if @scope[:shallow] - get :show if resource.actions.include?(:show) put :update if resource.actions.include?(:update) delete :destroy if resource.actions.include?(:destroy) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index a294535e88..5c46f9b971 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -34,33 +34,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - resources :users do - shallow do - resources :photos do - resources :types do - member do - post :preview - end - collection do - delete :erase - end - end - end - end - end - - shallow do - resources :teams do - resources :players - end - - resources :countries do - resources :cities do - resources :places - end - end - end - match 'account/logout' => redirect("/logout"), :as => :logout_redirect match 'account/login', :to => redirect("/login") @@ -779,18 +752,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - def test_shallow_routes - with_test_routes do - assert_equal '/photos/4', photo_path(4) - assert_equal '/types/10/edit', edit_type_path(10) - assert_equal '/types/5/preview', preview_type_path(5) - assert_equal '/photos/2/types', photo_types_path(2) - assert_equal '/cities/1/places', url_for(:controller => :places, :action => :index, :city_id => 1, :only_path => true) - assert_equal '/teams/new', url_for(:controller => :teams, :action => :new, :only_path => true) - assert_equal '/photos/11/types/erase', url_for(:controller => :types, :action => :erase, :photo_id => 11, :only_path => true) - end - end - def test_update_project_person with_test_routes do get '/projects/1/people/2/update' From 4b4a548a60236ccfb68c91f53386459bcb1751ab Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Tue, 8 Jun 2010 16:32:10 -0400 Subject: [PATCH 04/35] Avoid PostgreSQL and MySQL tests warnings. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/test/cases/active_schema_test_mysql.rb | 8 +++++--- activerecord/test/cases/active_schema_test_postgresql.rb | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/activerecord/test/cases/active_schema_test_mysql.rb b/activerecord/test/cases/active_schema_test_mysql.rb index 3526f49afd..d7431e5158 100644 --- a/activerecord/test/cases/active_schema_test_mysql.rb +++ b/activerecord/test/cases/active_schema_test_mysql.rb @@ -4,6 +4,7 @@ class ActiveSchemaTest < ActiveRecord::TestCase def setup ActiveRecord::ConnectionAdapters::MysqlAdapter.class_eval do alias_method :execute_without_stub, :execute + remove_method :execute def execute(sql, name = nil) return sql end end end @@ -66,7 +67,7 @@ class ActiveSchemaTest < ActiveRecord::TestCase assert_equal "DROP TABLE `otherdb`.`people`", drop_table('otherdb.people') end - def test_add_timestamps + def test_add_timestamps with_real_execute do begin ActiveRecord::Base.connection.create_table :delete_me do |t| @@ -79,8 +80,8 @@ class ActiveSchemaTest < ActiveRecord::TestCase end end end - - def test_remove_timestamps + + def test_remove_timestamps with_real_execute do begin ActiveRecord::Base.connection.create_table :delete_me do |t| @@ -106,6 +107,7 @@ class ActiveSchemaTest < ActiveRecord::TestCase ensure #before finishing, we restore the alias to the mock-up method ActiveRecord::ConnectionAdapters::MysqlAdapter.class_eval do + remove_method :execute alias_method :execute, :execute_with_stub end end diff --git a/activerecord/test/cases/active_schema_test_postgresql.rb b/activerecord/test/cases/active_schema_test_postgresql.rb index af80f724f2..4f04c6735c 100644 --- a/activerecord/test/cases/active_schema_test_postgresql.rb +++ b/activerecord/test/cases/active_schema_test_postgresql.rb @@ -4,6 +4,7 @@ class PostgresqlActiveSchemaTest < Test::Unit::TestCase def setup ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.class_eval do alias_method :real_execute, :execute + remove_method :execute def execute(sql, name = nil) sql end end end From e50bf67ffadc03c2ee175764f8017296313a4fcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 8 Jun 2010 22:36:35 +0200 Subject: [PATCH 05/35] Bring pg back. --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 6d878404d6..acee230d23 100644 --- a/Gemfile +++ b/Gemfile @@ -32,7 +32,7 @@ if mri || RUBY_ENGINE == "rbx" gem "sqlite3-ruby", "~> 1.3.0", :require => 'sqlite3' group :db do - # gem "pg", ">= 0.9.0" + gem "pg", ">= 0.9.0" gem "mysql", ">= 2.8.1" end elsif RUBY_ENGINE == "jruby" From 87cc3d556948f4cb644091c98969c03e319c864e Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 8 Jun 2010 16:57:46 -0400 Subject: [PATCH 06/35] Clarify Fixture#key_ and value_list --- activerecord/lib/active_record/fixtures.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 8099aaa7f7..82270c56b3 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -787,16 +787,14 @@ class Fixture #:nodoc: end def key_list - columns = @fixture.keys.collect{ |column_name| @connection.quote_column_name(column_name) } - columns.join(", ") + @fixture.keys.map { |column_name| @connection.quote_column_name(column_name) }.join(', ') end def value_list - list = @fixture.inject([]) do |fixtures, (key, value)| - col = model_class.columns_hash[key] if model_class.respond_to?(:ancestors) && model_class.ancestors.include?(ActiveRecord::Base) - fixtures << @connection.quote(value, col).gsub('[^\]\\n', "\n").gsub('[^\]\\r', "\r") - end - list * ', ' + cols = (model_class && model_class < ActiveRecord::Base) ? model_class.columns_hash : {} + @fixture.map do |key, value| + @connection.quote(value, cols[key]).gsub('[^\]\\n', "\n").gsub('[^\]\\r', "\r") + end.join(', ') end def find From b07073924002fd56ac5b63b24cb9318b3dee45c4 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 8 Jun 2010 16:59:06 -0400 Subject: [PATCH 07/35] Revert "Temporarily revert "Update after_commit and after_rollback docs and tests to use new style API with an :on options instead of on_* suffix." and "Add after_commit and after_rollback callbacks to ActiveRecord that are called after transactions either commit or rollback on all records saved or destroyed in the transaction."" This reverts commit 1b2941cba1165b0721f57524645fe378bee2a950. [#2991] --- activerecord/CHANGELOG | 2 + .../abstract/database_statements.rb | 56 ++++ .../lib/active_record/transactions.rb | 120 ++++++++- .../test/cases/transaction_callbacks_test.rb | 240 ++++++++++++++++++ activerecord/test/cases/transactions_test.rb | 33 +++ 5 files changed, 441 insertions(+), 10 deletions(-) create mode 100644 activerecord/test/cases/transaction_callbacks_test.rb diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index e47d291eb3..7d5e550a7c 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -14,6 +14,8 @@ * find_or_create_by_attr(value, ...) works when attr is protected. #4457 [Santiago Pastorino, Marc-André Lafortune] +* New callbacks: after_commit and after_rollback. Do expensive operations like image thumbnailing after_commit instead of after_save. #2991 [Brian Durand] + * Serialized attributes are not converted to YAML if they are any of the formats that can be serialized to XML (like Hash, Array and Strings). [José Valim] * Destroy uses optimistic locking. If lock_version on the record you're destroying doesn't match lock_version in the database, a StaleObjectError is raised. #1966 [Curtis Hawthorne] 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 0c87e052c4..b9fb452eee 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -122,6 +122,8 @@ module ActiveRecord requires_new = options[:requires_new] || !last_transaction_joinable transaction_open = false + @_current_transaction_records ||= [] + begin if block_given? if requires_new || open_transactions == 0 @@ -132,6 +134,7 @@ module ActiveRecord end increment_open_transactions transaction_open = true + @_current_transaction_records.push([]) end yield end @@ -141,8 +144,10 @@ module ActiveRecord decrement_open_transactions if open_transactions == 0 rollback_db_transaction + rollback_transaction_records(true) else rollback_to_savepoint + rollback_transaction_records(false) end end raise unless database_transaction_rollback.is_a?(ActiveRecord::Rollback) @@ -157,20 +162,35 @@ module ActiveRecord begin if open_transactions == 0 commit_db_transaction + commit_transaction_records else release_savepoint + save_point_records = @_current_transaction_records.pop + unless save_point_records.blank? + @_current_transaction_records.push([]) if @_current_transaction_records.empty? + @_current_transaction_records.last.concat(save_point_records) + end end rescue Exception => database_transaction_rollback if open_transactions == 0 rollback_db_transaction + rollback_transaction_records(true) else rollback_to_savepoint + rollback_transaction_records(false) end raise end end end + # Register a record with the current transaction so that its after_commit and after_rollback callbacks + # can be called. + def add_transaction_record(record) + last_batch = @_current_transaction_records.last + last_batch << record if last_batch + end + # Begins the transaction (and turns off auto-committing). def begin_db_transaction() end @@ -268,6 +288,42 @@ module ActiveRecord limit.to_i end end + + # Send a rollback message to all records after they have been rolled back. If rollback + # is false, only rollback records since the last save point. + def rollback_transaction_records(rollback) #:nodoc + if rollback + records = @_current_transaction_records.flatten + @_current_transaction_records.clear + else + records = @_current_transaction_records.pop + end + + unless records.blank? + records.uniq.each do |record| + begin + record.rolledback!(rollback) + rescue Exception => e + record.logger.error(e) if record.respond_to?(:logger) + end + end + end + end + + # Send a commit message to all records after they have been committed. + def commit_transaction_records #:nodoc + records = @_current_transaction_records.flatten + @_current_transaction_records.clear + unless records.blank? + records.uniq.each do |record| + begin + record.committed! + rescue Exception => e + record.logger.error(e) if record.respond_to?(:logger) + end + end + end + end end end end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 3f2c1911e7..5a8e2ce880 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -8,6 +8,11 @@ module ActiveRecord class TransactionError < ActiveRecordError # :nodoc: end + included do + define_model_callbacks :commit, :commit_on_update, :commit_on_create, :commit_on_destroy, :only => :after + define_model_callbacks :rollback, :rollback_on_update, :rollback_on_create, :rollback_on_destroy + end + # Transactions are protective blocks where SQL statements are only permanent # if they can all succeed as one atomic action. The classic example is a # transfer between two accounts where you can only have a deposit if the @@ -158,6 +163,21 @@ module ActiveRecord # http://dev.mysql.com/doc/refman/5.0/en/savepoints.html # for more information about savepoints. # + # === Callbacks + # + # There are two types of callbacks associated with committing and rolling back transactions: + # +after_commit+ and +after_rollback+. + # + # +after_commit+ callbacks are called on every record saved or destroyed within a + # transaction immediately after the transaction is committed. +after_rollback+ callbacks + # are called on every record saved or destroyed within a transaction immediately after the + # transaction or savepoint is rolled back. + # + # These callbacks are useful for interacting with other systems since you will be guaranteed + # that the callback is only executed when the database is in a permanent state. For example, + # +after_commit+ is a good spot to put in a hook to clearing a cache since clearing it from + # within a transaction could trigger the cache to be regenerated before the database is updated. + # # === Caveats # # If you're on MySQL, then do not use DDL operations in nested transactions @@ -205,19 +225,50 @@ module ActiveRecord # Reset id and @new_record if the transaction rolls back. def rollback_active_record_state! - id_present = has_attribute?(self.class.primary_key) - previous_id = id - previous_new_record = new_record? + remember_transaction_record_state yield rescue Exception - @new_record = previous_new_record - if id_present - self.id = previous_id - else - @attributes.delete(self.class.primary_key) - @attributes_cache.delete(self.class.primary_key) - end + restore_transaction_record_state raise + ensure + clear_transaction_record_state + end + + # Call the after_commit callbacks + def committed! #:nodoc: + if transaction_record_state(:new_record) + _run_commit_on_create_callbacks + elsif transaction_record_state(:destroyed) + _run_commit_on_destroy_callbacks + else + _run_commit_on_update_callbacks + end + _run_commit_callbacks + ensure + clear_transaction_record_state + end + + # Call the after rollback callbacks. The restore_state argument indicates if the record + # state should be rolled back to the beginning or just to the last savepoint. + def rolledback!(force_restore_state = false) #:nodoc: + if transaction_record_state(:new_record) + _run_rollback_on_create_callbacks + elsif transaction_record_state(:destroyed) + _run_rollback_on_destroy_callbacks + else + _run_rollback_on_update_callbacks + end + _run_rollback_callbacks + ensure + restore_transaction_record_state(force_restore_state) + end + + # Add the record to the current transaction so that the :after_rollback and :after_commit callbacks + # can be called. + def add_to_transaction + if self.class.connection.add_transaction_record(self) + remember_transaction_record_state + end end # Executes +method+ within a transaction and captures its return value as a @@ -229,10 +280,59 @@ module ActiveRecord def with_transaction_returning_status status = nil self.class.transaction do + add_to_transaction status = yield raise ActiveRecord::Rollback unless status end status end + + protected + + # Save the new record state and id of a record so it can be restored later if a transaction fails. + def remember_transaction_record_state #:nodoc + @_start_transaction_state ||= {} + unless @_start_transaction_state.include?(:new_record) + @_start_transaction_state[:id] = id if has_attribute?(self.class.primary_key) + @_start_transaction_state[:new_record] = @new_record + end + unless @_start_transaction_state.include?(:destroyed) + @_start_transaction_state[:destroyed] = @new_record + end + @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1 + end + + # Clear the new record state and id of a record. + def clear_transaction_record_state #:nodoc + if defined?(@_start_transaction_state) + @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1 + remove_instance_variable(:@_start_transaction_state) if @_start_transaction_state[:level] < 1 + end + end + + # Restore the new record state and id of a record that was previously saved by a call to save_record_state. + def restore_transaction_record_state(force = false) #:nodoc + if defined?(@_start_transaction_state) + @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1 + if @_start_transaction_state[:level] < 1 + restore_state = remove_instance_variable(:@_start_transaction_state) + if restore_state + @new_record = restore_state[:new_record] + @destroyed = restore_state[:destroyed] + if restore_state[:id] + self.id = restore_state[:id] + else + @attributes.delete(self.class.primary_key) + @attributes_cache.delete(self.class.primary_key) + end + end + end + end + end + + # Determine if a record was created or destroyed in a transaction. State should be one of :new_record or :destroyed. + def transaction_record_state(state) #:nodoc + @_start_transaction_state[state] if defined?(@_start_transaction_state) + end end end diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb new file mode 100644 index 0000000000..ebc16653cb --- /dev/null +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -0,0 +1,240 @@ +require "cases/helper" +require 'models/topic' +require 'models/reply' + +class TransactionCallbacksTest < ActiveRecord::TestCase + self.use_transactional_fixtures = false + fixtures :topics + + class TopicWithCallbacks < ActiveRecord::Base + set_table_name :topics + + after_commit{|record| record.send(:do_after_commit, nil)} + after_commit(:on => :create){|record| record.send(:do_after_commit, :create)} + after_commit(:on => :update){|record| record.send(:do_after_commit, :update)} + after_commit(:on => :destroy){|record| record.send(:do_after_commit, :destroy)} + after_rollback{|record| record.send(:do_after_rollback, nil)} + after_rollback(:on => :create){|record| record.send(:do_after_rollback, :create)} + after_rollback(:on => :update){|record| record.send(:do_after_rollback, :update)} + after_rollback(:on => :destroy){|record| record.send(:do_after_rollback, :destroy)} + + def history + @history ||= [] + end + + def after_commit_block(on = nil, &block) + @after_commit ||= {} + @after_commit[on] ||= [] + @after_commit[on] << block + end + + def after_rollback_block(on = nil, &block) + @after_rollback ||= {} + @after_rollback[on] ||= [] + @after_rollback[on] << block + end + + def do_after_commit(on) + blocks = @after_commit[on] if defined?(@after_commit) + blocks.each{|b| b.call(self)} if blocks + end + + def do_after_rollback(on) + blocks = @after_rollback[on] if defined?(@after_rollback) + blocks.each{|b| b.call(self)} if blocks + end + end + + def setup + @first, @second = TopicWithCallbacks.find(1, 3).sort_by { |t| t.id } + 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} + + @first.save! + assert_equal [:after_commit], @first.history + end + + def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record + @first.after_commit_block(:create){|r| r.history << :commit_on_create} + @first.after_commit_block(:update){|r| r.history << :commit_on_update} + @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} + @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} + @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} + @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + + @first.save! + assert_equal [:commit_on_update], @first.history + end + + def test_only_call_after_commit_on_destroy_after_transaction_commits_for_destroyed_record + @first.after_commit_block(:create){|r| r.history << :commit_on_create} + @first.after_commit_block(:update){|r| r.history << :commit_on_update} + @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} + @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} + @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} + @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + + @first.destroy + assert_equal [:commit_on_destroy], @first.history + end + + def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record + @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today) + @new_record.after_commit_block(:create){|r| r.history << :commit_on_create} + @new_record.after_commit_block(:update){|r| r.history << :commit_on_update} + @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} + @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create} + @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update} + @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + + @new_record.save! + assert_equal [:commit_on_create], @new_record.history + end + + def test_call_after_rollback_after_transaction_rollsback + @first.after_commit_block{|r| r.history << :after_commit} + @first.after_rollback_block{|r| r.history << :after_rollback} + + Topic.transaction do + @first.save! + raise ActiveRecord::Rollback + end + + assert_equal [:after_rollback], @first.history + end + + def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record + @first.after_commit_block(:create){|r| r.history << :commit_on_create} + @first.after_commit_block(:update){|r| r.history << :commit_on_update} + @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} + @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} + @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} + @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + + Topic.transaction do + @first.save! + raise ActiveRecord::Rollback + end + + assert_equal [:rollback_on_update], @first.history + end + + def test_only_call_after_rollback_on_destroy_after_transaction_rollsback_for_destroyed_record + @first.after_commit_block(:create){|r| r.history << :commit_on_create} + @first.after_commit_block(:update){|r| r.history << :commit_on_update} + @first.after_commit_block(:destroy){|r| r.history << :commit_on_update} + @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} + @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} + @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + + Topic.transaction do + @first.destroy + raise ActiveRecord::Rollback + end + + assert_equal [:rollback_on_destroy], @first.history + end + + def test_only_call_after_rollback_on_create_after_transaction_rollsback_for_new_record + @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today) + @new_record.after_commit_block(:create){|r| r.history << :commit_on_create} + @new_record.after_commit_block(:update){|r| r.history << :commit_on_update} + @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} + @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create} + @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update} + @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + + Topic.transaction do + @new_record.save! + raise ActiveRecord::Rollback + end + + assert_equal [:rollback_on_create], @new_record.history + end + + def test_call_after_rollback_when_commit_fails + @first.connection.class.send(:alias_method, :real_method_commit_db_transaction, :commit_db_transaction) + begin + @first.connection.class.class_eval do + def commit_db_transaction; raise "boom!"; end + end + + @first.after_commit_block{|r| r.history << :after_commit} + @first.after_rollback_block{|r| r.history << :after_rollback} + + assert !@first.save rescue nil + assert_equal [:after_rollback], @first.history + ensure + @first.connection.class.send(:remove_method, :commit_db_transaction) + @first.connection.class.send(:alias_method, :commit_db_transaction, :real_method_commit_db_transaction) + end + end + + def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint + def @first.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end + def @first.commits(i=0); @commits ||= 0; @commits += i if i; end + @first.after_rollback_block{|r| r.rollbacks(1)} + @first.after_commit_block{|r| r.commits(1)} + + def @second.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end + def @second.commits(i=0); @commits ||= 0; @commits += i if i; end + @second.after_rollback_block{|r| r.rollbacks(1)} + @second.after_commit_block{|r| r.commits(1)} + + Topic.transaction do + @first.save! + Topic.transaction(:requires_new => true) do + @second.save! + raise ActiveRecord::Rollback + end + end + + assert_equal 1, @first.commits + assert_equal 0, @first.rollbacks + assert_equal 0, @second.commits + assert_equal 1, @second.rollbacks + end + + def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint_when_release_savepoint_fails + def @first.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end + def @first.commits(i=0); @commits ||= 0; @commits += i if i; end + + @first.after_rollback_block{|r| r.rollbacks(1)} + @first.after_commit_block{|r| r.commits(1)} + + Topic.transaction do + @first.save + Topic.transaction(:requires_new => true) do + @first.save! + raise ActiveRecord::Rollback + end + Topic.transaction(:requires_new => true) do + @first.save! + raise ActiveRecord::Rollback + end + end + + assert_equal 1, @first.commits + assert_equal 2, @first.rollbacks + end + + def test_after_transaction_callbacks_should_not_raise_errors + def @first.last_after_transaction_error=(e); @last_transaction_error = e; end + def @first.last_after_transaction_error; @last_transaction_error; end + @first.after_commit_block{|r| r.last_after_transaction_error = :commit; raise "fail!";} + @first.after_rollback_block{|r| r.last_after_transaction_error = :rollback; raise "fail!";} + + @first.save! + assert_equal :commit, @first.last_after_transaction_error + + Topic.transaction do + @first.save! + raise ActiveRecord::Rollback + end + + assert_equal :rollback, @first.last_after_transaction_error + end +end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 00f3b527d7..958a4e4f94 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -320,6 +320,33 @@ class TransactionTest < ActiveRecord::TestCase end end + def test_restore_active_record_state_for_all_records_in_a_transaction + topic_1 = Topic.new(:title => 'test_1') + topic_2 = Topic.new(:title => 'test_2') + Topic.transaction do + assert topic_1.save + assert topic_2.save + @first.save + @second.destroy + assert_equal false, topic_1.new_record? + assert_not_nil topic_1.id + assert_equal false, topic_2.new_record? + assert_not_nil topic_2.id + assert_equal false, @first.new_record? + assert_not_nil @first.id + assert_equal true, @second.destroyed? + raise ActiveRecord::Rollback + end + + assert_equal true, topic_1.new_record? + assert_nil topic_1.id + assert_equal true, topic_2.new_record? + assert_nil topic_2.id + assert_equal false, @first.new_record? + assert_not_nil @first.id + assert_equal false, @second.destroyed? + end + if current_adapter?(:PostgreSQLAdapter) && defined?(PGconn::PQTRANS_IDLE) def test_outside_transaction_works assert Topic.connection.outside_transaction? @@ -382,6 +409,12 @@ class TransactionTest < ActiveRecord::TestCase end private + def define_callback_method(callback_method) + define_method(callback_method) do + self.history << [callback_method, :method] + end + end + def add_exception_raising_after_save_callback_to_topic Topic.class_eval <<-eoruby, __FILE__, __LINE__ + 1 remove_method(:after_save_for_transaction) From 2500e6af6634c984f7d2b567403de1c4c544ff91 Mon Sep 17 00:00:00 2001 From: Brian Durand Date: Tue, 8 Jun 2010 14:41:42 -0500 Subject: [PATCH 08/35] Make logic for after_commit and after_rollback :on option work like it does for validation callbacks. [#2991 state:committed] Signed-off-by: Jeremy Kemper --- .../lib/active_record/transactions.rb | 51 ++++++++++++------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 5a8e2ce880..620758f5af 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -9,8 +9,7 @@ module ActiveRecord end included do - define_model_callbacks :commit, :commit_on_update, :commit_on_create, :commit_on_destroy, :only => :after - define_model_callbacks :rollback, :rollback_on_update, :rollback_on_create, :rollback_on_destroy + define_callbacks :commit, :rollback, :terminator => "result == false", :scope => [:kind, :name] end # Transactions are protective blocks where SQL statements are only permanent @@ -77,7 +76,7 @@ module ActiveRecord # # Both +save+ and +destroy+ come wrapped in a transaction that ensures # that whatever you do in validations or callbacks will happen under its - # protected cover. So you can use validations to check for values that + # protected cover. So you can use validations to check for values that # the transaction depends on or you can raise exceptions in the callbacks # to rollback, including after_* callbacks. # @@ -202,6 +201,24 @@ module ActiveRecord # See the ConnectionAdapters::DatabaseStatements#transaction API docs. connection.transaction(options, &block) end + + def after_commit(*args, &block) + options = args.last + if options.is_a?(Hash) && options[:on] + options[:if] = Array.wrap(options[:if]) + options[:if] << "transaction_include_action?(:#{options[:on]})" + end + set_callback(:commit, :after, *args, &block) + end + + def after_rollback(*args, &block) + options = args.last + if options.is_a?(Hash) && options[:on] + options[:if] = Array.wrap(options[:if]) + options[:if] << "transaction_include_action?(:#{options[:on]})" + end + set_callback(:rollback, :after, *args, &block) + end end # See ActiveRecord::Transactions::ClassMethods for detailed documentation. @@ -236,13 +253,6 @@ module ActiveRecord # Call the after_commit callbacks def committed! #:nodoc: - if transaction_record_state(:new_record) - _run_commit_on_create_callbacks - elsif transaction_record_state(:destroyed) - _run_commit_on_destroy_callbacks - else - _run_commit_on_update_callbacks - end _run_commit_callbacks ensure clear_transaction_record_state @@ -251,13 +261,6 @@ module ActiveRecord # Call the after rollback callbacks. The restore_state argument indicates if the record # state should be rolled back to the beginning or just to the last savepoint. def rolledback!(force_restore_state = false) #:nodoc: - if transaction_record_state(:new_record) - _run_rollback_on_create_callbacks - elsif transaction_record_state(:destroyed) - _run_rollback_on_destroy_callbacks - else - _run_rollback_on_update_callbacks - end _run_rollback_callbacks ensure restore_transaction_record_state(force_restore_state) @@ -297,7 +300,7 @@ module ActiveRecord @_start_transaction_state[:new_record] = @new_record end unless @_start_transaction_state.include?(:destroyed) - @_start_transaction_state[:destroyed] = @new_record + @_start_transaction_state[:destroyed] = @destroyed end @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1 end @@ -334,5 +337,17 @@ module ActiveRecord def transaction_record_state(state) #:nodoc @_start_transaction_state[state] if defined?(@_start_transaction_state) end + + # Determine if a transaction included an action for :create, :update, or :destroy. Used in filtering callbacks. + def transaction_include_action?(action) #:nodoc + case action + when :create + transaction_record_state(:new_record) + when :destroy + destroyed? + when :update + !(transaction_record_state(:new_record) || destroyed?) + end + end end end From db23a95a616860e4fefa4ef83b396abe7ec0ea71 Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Fri, 7 May 2010 01:01:47 -0400 Subject: [PATCH 09/35] cache_sweeper yields blank output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#3914 state:open] Signed-off-by: José Valim --- actionpack/lib/action_controller/caching/sweeping.rb | 1 + actionpack/test/abstract_unit.rb | 3 +++ actionpack/test/controller/filters_test.rb | 6 ++++++ 3 files changed, 10 insertions(+) diff --git a/actionpack/lib/action_controller/caching/sweeping.rb b/actionpack/lib/action_controller/caching/sweeping.rb index cf16417e84..e9db0d97b6 100644 --- a/actionpack/lib/action_controller/caching/sweeping.rb +++ b/actionpack/lib/action_controller/caching/sweeping.rb @@ -57,6 +57,7 @@ module ActionController #:nodoc: def before(controller) self.controller = controller callback(:before) if controller.perform_caching + true # before method from sweeper should always return true end def after(controller) diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index d2e5d2e965..84c5395610 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -24,6 +24,9 @@ require 'action_view/testing/resolvers' require 'action_dispatch' require 'active_support/dependencies' require 'active_model' +require 'active_record' +require 'action_controller/caching' +require 'action_controller/caching/sweeping' begin require 'ruby-debug' diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index d5704eba78..25b78124e3 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -445,6 +445,12 @@ class FilterTest < ActionController::TestCase end + + def test_before_method_of_sweeper_should_always_return_true + sweeper = ActionController::Caching::Sweeper.send(:new) + assert sweeper.before(TestController.new) + end + def test_non_yielding_around_filters_not_returning_false_do_not_raise controller = NonYieldingAroundFilterController.new controller.instance_variable_set "@filter_return_value", true From 4560385fa4fa6925aa220bbaf6b2608c2bbd7039 Mon Sep 17 00:00:00 2001 From: Jan De Poorter Date: Wed, 14 Apr 2010 10:34:42 +0200 Subject: [PATCH 10/35] Make sure namespaces are nested within resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_dispatch/routing/mapper.rb | 8 ++++++++ actionpack/test/dispatch/routing_test.rb | 15 +++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 6db8d1aacc..0f267caefe 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -679,6 +679,14 @@ module ActionDispatch end end + def namespace(path) + if @scope[:scope_level] == :resources + nested { super } + else + super + end + end + def match(*args) options = args.extract_options! diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 5c46f9b971..c4e402afd3 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -144,6 +144,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest resources :sheep + resources :clients do + namespace :google do + resource :account + end + end + match 'sprockets.js' => ::TestRoutingMapper::SprocketsApp match 'people/:id/update', :to => 'people#update', :as => :update_person @@ -813,6 +819,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/account/admin/subscription', account_admin_subscription_path end end + + def test_namespace_nested_in_resources + with_test_routes do + get '/clients/1/google/account' + assert_equal '/clients/1/google/account', client_google_account_path(1) + + assert_equal 'google/accounts#show', @response.body + end + end def test_articles_with_id with_test_routes do From 5c9f27abaabba0d008ccd710ed1af5f6caa4e371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 8 Jun 2010 23:26:51 +0200 Subject: [PATCH 11/35] Add more cases to previous commit [#4394 state:resolved] --- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- actionpack/test/dispatch/routing_test.rb | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 0f267caefe..7b79b6bde3 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -680,7 +680,7 @@ module ActionDispatch end def namespace(path) - if @scope[:scope_level] == :resources + if resource_scope? nested { super } else super diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index c4e402afd3..e13960e0dc 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -146,7 +146,11 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest resources :clients do namespace :google do - resource :account + resource :account do + namespace :secret do + resource :info + end + end end end @@ -824,8 +828,11 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest with_test_routes do get '/clients/1/google/account' assert_equal '/clients/1/google/account', client_google_account_path(1) - assert_equal 'google/accounts#show', @response.body + + get '/clients/1/google/account/secret/info' + assert_equal '/clients/1/google/account/secret/info', client_google_account_secret_info_path(1) + assert_equal 'google/secret/infos#show', @response.body end end From 6ebc7c8ee6de0f3f441a68baa6351416a6ac0a59 Mon Sep 17 00:00:00 2001 From: wycats Date: Tue, 8 Jun 2010 18:10:18 -0400 Subject: [PATCH 12/35] Update bundler dependency --- rails.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rails.gemspec b/rails.gemspec index 3b1dfe9456..236203c626 100644 --- a/rails.gemspec +++ b/rails.gemspec @@ -25,5 +25,5 @@ Gem::Specification.new do |s| s.add_dependency('activeresource', version) s.add_dependency('actionmailer', version) s.add_dependency('railties', version) - s.add_dependency('bundler', '>= 0.9.19') + s.add_dependency('bundler', '>= 0.9.26') end From f48aa14bf43fb103e5d128151061549ba6bb8c23 Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Tue, 8 Jun 2010 23:25:55 -0400 Subject: [PATCH 13/35] Better test for ticket [#3914 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/test/controller/filters_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 25b78124e3..14f1bd797a 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -445,6 +445,17 @@ class FilterTest < ActionController::TestCase end + class ::AppSweeper < ActionController::Caching::Sweeper; end + class SweeperTestController < ActionController::Base + cache_sweeper :app_sweeper + def show + render :text => 'hello world' + end + end + def test_sweeper_should_not_block_rendering + response = test_process(SweeperTestController) + assert_equal 'hello world', response.body + end def test_before_method_of_sweeper_should_always_return_true sweeper = ActionController::Caching::Sweeper.send(:new) From 0919c0dbca3df02f5cfff7dde4f61b85ef16d886 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 9 Jun 2010 00:28:42 -0300 Subject: [PATCH 14/35] Removed textilize, textilize_without_paragraph and markdown helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- Gemfile | 4 - actionpack/CHANGELOG | 2 + .../lib/action_view/helpers/text_helper.rb | 83 -------------- actionpack/test/template/text_helper_test.rb | 108 ------------------ 4 files changed, 2 insertions(+), 195 deletions(-) diff --git a/Gemfile b/Gemfile index acee230d23..19e532bc0b 100644 --- a/Gemfile +++ b/Gemfile @@ -44,10 +44,6 @@ elsif RUBY_ENGINE == "jruby" end end -# AP -gem "RedCloth", ">= 4.2.2" -gem "bluecloth", ">= 2.0.7" - group :documentation do gem 'rdoc', '2.1' end diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 1796d11dcd..0ca2f86dae 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,3 +1,5 @@ +* Removed textilize, textilize_without_paragraph and markdown helpers. [Santiago Pastorino] + *Rails 3.0.0 [beta 4] (June 8th, 2010)* * Remove middleware laziness [José Valim] diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index 8f63845d49..a06073ce66 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -220,89 +220,6 @@ module ActionView end * "\n" end - # Returns the text with all the Textile[http://www.textism.com/tools/textile] codes turned into HTML tags. - # - # You can learn more about Textile's syntax at its website[http://www.textism.com/tools/textile]. - # This method is only available if RedCloth[http://redcloth.org/] is available. - # - # ==== Examples - # textilize("*This is Textile!* Rejoice!") - # # => "

This is Textile! Rejoice!

" - # - # textilize("I _love_ ROR(Ruby on Rails)!") - # # => "

I love ROR!

" - # - # textilize("h2. Textile makes markup -easy- simple!") - # # => "

Textile makes markup easy simple!

" - # - # textilize("Visit the Rails website "here":http://www.rubyonrails.org/.) - # # => "

Visit the Rails website here.

" - # - # textilize("This is worded strongly") - # # => "

This is worded strongly

" - # - # textilize("This is worded strongly", :filter_html) - # # => "

This is worded <strong>strongly</strong>

" - # - def textilize(text, *options) - options ||= [:hard_breaks] - text = sanitize(text) unless text.html_safe? || options.delete(:safe) - - if text.blank? - "" - else - textilized = RedCloth.new(text, options) - textilized.to_html - end.html_safe - end - - # Returns the text with all the Textile codes turned into HTML tags, - # but without the bounding

tag that RedCloth adds. - # - # You can learn more about Textile's syntax at its website[http://www.textism.com/tools/textile]. - # This method is only available if RedCloth[http://redcloth.org/] is available. - # - # ==== Examples - # textilize_without_paragraph("*This is Textile!* Rejoice!") - # # => "This is Textile! Rejoice!" - # - # textilize_without_paragraph("I _love_ ROR(Ruby on Rails)!") - # # => "I love ROR!" - # - # textilize_without_paragraph("h2. Textile makes markup -easy- simple!") - # # => "

Textile makes markup easy simple!

" - # - # textilize_without_paragraph("Visit the Rails website "here":http://www.rubyonrails.org/.) - # # => "Visit the Rails website here." - def textilize_without_paragraph(text, *options) - textiled = textilize(text, *options) - if textiled[0..2] == "

" then textiled = textiled[3..-1] end - if textiled[-4..-1] == "

" then textiled = textiled[0..-5] end - return textiled - end - - # Returns the text with all the Markdown codes turned into HTML tags. - # This method requires BlueCloth[http://www.deveiate.org/projects/BlueCloth] - # to be available. - # - # ==== Examples - # markdown("We are using __Markdown__ now!") - # # => "

We are using Markdown now!

" - # - # markdown("We like to _write_ `code`, not just _read_ it!") - # # => "

We like to write code, not just read it!

" - # - # markdown("The [Markdown website](http://daringfireball.net/projects/markdown/) has more information.") - # # => "

The Markdown website - # # has more information.

" - # - # markdown('![The ROR logo](http://rubyonrails.com/images/rails.png "Ruby on Rails")') - # # => '

The ROR logo

' - def markdown(text, *options) - text = sanitize(text) unless text.html_safe? || options.delete(:safe) - (text.blank? ? "" : BlueCloth.new(text).to_html).html_safe - end - # Returns +text+ transformed into HTML using simple formatting rules. # Two or more consecutive newlines(\n\n) are considered as a # paragraph and wrapped in

tags. One newline (\n) is diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb index 64f1d46413..17fc8b6edd 100644 --- a/actionpack/test/template/text_helper_test.rb +++ b/actionpack/test/template/text_helper_test.rb @@ -1,17 +1,6 @@ # encoding: us-ascii require 'abstract_unit' require 'testing_sandbox' -begin - require 'redcloth' -rescue LoadError - $stderr.puts "Skipping textilize tests. `gem install RedCloth` to enable." -end - -begin - require 'bluecloth' -rescue LoadError - $stderr.puts "Skipping markdown tests. 'gem install bluecloth' to enable." -end class TextHelperTest < ActionView::TestCase tests ActionView::Helpers::TextHelper @@ -665,101 +654,4 @@ class TextHelperTest < ActionView::TestCase assert_equal("red", cycle("red", "blue")) assert_equal(%w{Specialized Fuji Giant}, @cycles) end - - # TODO test textilize_without_paragraph and markdown - if defined? RedCloth - def test_textilize_should_be_html_safe - assert textilize("*This is Textile!* Rejoice!").html_safe? - end - - def test_textilize - assert_equal("

This is Textile! Rejoice!

", textilize("*This is Textile!* Rejoice!")) - end - - def test_textilize_with_blank - assert_equal("", textilize("")) - end - - def test_textilize_with_options - assert_equal("

This is worded <strong>strongly</strong>

", textilize("This is worded strongly", :filter_html)) - end - - def test_textilize_should_sanitize_unsafe_input - assert_equal("

This is worded strongly

", textilize("This is worded strongly")) - end - - def test_textilize_should_not_sanitize_input_if_safe_option - assert_equal("

This is worded strongly

", textilize("This is worded strongly", :safe)) - end - - def test_textilize_should_not_sanitize_safe_input - assert_equal("

This is worded strongly

", textilize("This is worded strongly".html_safe)) - end - - def test_textilize_with_hard_breaks - assert_equal("

This is one scary world.
\n True.

", textilize("This is one scary world.\n True.")) - end - - def test_textilize_without_paragraph_should_be_html_safe - textilize_without_paragraph("*This is Textile!* Rejoice!").html_safe? - end - - def test_textilize_without_paragraph - assert_equal("This is Textile! Rejoice!", textilize_without_paragraph("*This is Textile!* Rejoice!")) - end - - def test_textilize_without_paragraph_with_blank - assert_equal("", textilize_without_paragraph("")) - end - - def test_textilize_without_paragraph_with_options - assert_equal("This is worded <strong>strongly</strong>", textilize_without_paragraph("This is worded strongly", :filter_html)) - end - - def test_textilize_without_paragraph_should_sanitize_unsafe_input - assert_equal("This is worded strongly", textilize_without_paragraph("This is worded strongly")) - end - - def test_textilize_without_paragraph_should_not_sanitize_input_if_safe_option - assert_equal("This is worded strongly", textilize_without_paragraph("This is worded strongly", :safe)) - end - - def test_textilize_without_paragraph_should_not_sanitize_safe_input - assert_equal("This is worded strongly", textilize_without_paragraph("This is worded strongly".html_safe)) - end - - def test_textilize_without_paragraph_with_hard_breaks - assert_equal("This is one scary world.
\n True.", textilize_without_paragraph("This is one scary world.\n True.")) - end - end - - if defined? BlueCloth - def test_markdown_should_be_html_safe - assert markdown("We are using __Markdown__ now!").html_safe? - end - - def test_markdown - assert_equal("

We are using Markdown now!

", markdown("We are using __Markdown__ now!")) - end - - def test_markdown_with_blank - assert_equal("", markdown("")) - end - - def test_markdown_should_sanitize_unsafe_input - assert_equal("

This is worded strongly

", markdown("This is worded strongly")) - end - - def test_markdown_should_not_sanitize_input_if_safe_option - assert_equal("

This is worded strongly

", markdown("This is worded strongly", :safe)) - end - - def test_markdown_should_not_sanitize_safe_input - assert_equal("

This is worded strongly

", markdown("This is worded strongly".html_safe)) - end - - def test_markdown_with_hard_breaks - assert_equal("

This is one scary world.

\n\n

True.

", markdown("This is one scary world.\n\nTrue.")) - end - end end From cfacae1a7d85849f0b4f71f0cfd4f42ad3ff90a4 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 9 Jun 2010 03:03:45 -0300 Subject: [PATCH 15/35] SQLite2Adapter doesn't exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/test/cases/migration_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 1edec66c25..ddadde8dcf 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -1019,7 +1019,7 @@ if ActiveRecord::Base.connection.supports_migrations? # This one is fun. The 'value_of_e' field is defined as 'DECIMAL' with # precision/scale explicitly left out. By the SQL standard, numbers # assigned to this field should be truncated but that's seldom respected. - if current_adapter?(:PostgreSQLAdapter, :SQLite2Adapter) + if current_adapter?(:PostgreSQLAdapter) # - PostgreSQL changes the SQL spec on columns declared simply as # "decimal" to something more useful: instead of being given a scale # of 0, they take on the compile-time limit for precision and scale, From 5b42acdadd0da164ede9e3c5f9afbd9ef18fb8e9 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 9 Jun 2010 03:37:24 -0300 Subject: [PATCH 16/35] Should call configure! to initiliaze the application MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- railties/test/application/generators_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/railties/test/application/generators_test.rb b/railties/test/application/generators_test.rb index 712c8bef36..cbf0decd07 100644 --- a/railties/test/application/generators_test.rb +++ b/railties/test/application/generators_test.rb @@ -64,6 +64,7 @@ module ApplicationTests # Initialize the application require "#{app_path}/config/environment" require "rails/generators" + Rails::Generators.configure! assert_equal :rspec, Rails::Generators.options[:rails][:test_framework] assert_equal "-w", Rails::Generators.aliases[:rails][:test_framework] From 6898c167c3906b87dcf52cf6c088ae9f08f3ad22 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 9 Jun 2010 04:25:06 -0300 Subject: [PATCH 17/35] Make sure about which is the first element of the query, fixes a postgresql 8.4 failing test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- .../test/cases/associations/cascaded_eager_loading_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 7a4a33d177..050b730dda 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -41,9 +41,9 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase def test_eager_association_loading_grafts_stashed_associations_to_correct_parent assert_nothing_raised do - Person.eager_load(:primary_contact => :primary_contact).where('primary_contacts_people_2.first_name = ?', 'Susan').all + Person.eager_load(:primary_contact => :primary_contact).where('primary_contacts_people_2.first_name = ?', 'Susan').order('primary_contacts_people_2.id').all end - assert_equal people(:michael), Person.eager_load(:primary_contact => :primary_contact).where('primary_contacts_people_2.first_name = ?', 'Susan').first + assert_equal people(:michael), Person.eager_load(:primary_contact => :primary_contact).where('primary_contacts_people_2.first_name = ?', 'Susan').order('primary_contacts_people_2.id').first end def test_eager_association_loading_with_cascaded_two_levels_with_two_has_many_associations From 0265c708b9696c3943518ad5f3dabdc22c5eba11 Mon Sep 17 00:00:00 2001 From: James Le Cuirot Date: Wed, 9 Jun 2010 12:33:40 +0100 Subject: [PATCH 18/35] Don't overwrite unsaved updates when loading an association but preserve the order of the loaded records. [#4642 state:resolved] Signed-off-by: Pratik Naik --- .../associations/association_collection.rb | 2 +- activerecord/test/cases/nested_attributes_test.rb | 14 ++++++++++++++ activerecord/test/models/pirate.rb | 4 ++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index d9903243ce..5b68ca2edb 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -388,7 +388,7 @@ module ActiveRecord begin if !loaded? if @target.is_a?(Array) && @target.any? - @target = find_target + @target.find_all {|t| t.new_record? } + @target = find_target.map { |f| i = @target.index(f); i ? @target.delete_at(i) : f } + @target else @target = find_target end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 685b11cb03..a87683bd97 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -466,6 +466,20 @@ module NestedAttributesOnACollectionAssociationTests assert_equal 'Grace OMalley', @child_1.reload.name end + def test_should_not_overwrite_unsaved_updates_when_loading_association + @pirate.reload + @pirate.send(association_setter, [{ :id => @child_1.id, :name => 'Grace OMalley' }]) + @pirate.send(@association_name).send(:load_target) + assert_equal 'Grace OMalley', @pirate.send(@association_name).target.find { |r| r.id == @child_1.id }.name + end + + def test_should_preserve_order_when_not_overwriting_unsaved_updates + @pirate.reload + @pirate.send(association_setter, [{ :id => @child_1.id, :name => 'Grace OMalley' }]) + @pirate.send(@association_name).send(:load_target) + assert_equal @pirate.send(@association_name).target.first.id, @child_1.id + end + def test_should_take_a_hash_with_composite_id_keys_and_assign_the_attributes_to_the_associated_models @child_1.stubs(:id).returns('ABC1X') @child_2.stubs(:id).returns('ABC2X') diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index f1dbe32c6e..d89c8cf381 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -1,7 +1,7 @@ class Pirate < ActiveRecord::Base belongs_to :parrot, :validate => true belongs_to :non_validated_parrot, :class_name => 'Parrot' - has_and_belongs_to_many :parrots, :validate => true + has_and_belongs_to_many :parrots, :validate => true, :order => 'parrots.id ASC' has_and_belongs_to_many :non_validated_parrots, :class_name => 'Parrot' has_and_belongs_to_many :parrots_with_method_callbacks, :class_name => "Parrot", :before_add => :log_before_add, @@ -21,7 +21,7 @@ class Pirate < ActiveRecord::Base has_one :ship has_one :update_only_ship, :class_name => 'Ship' has_one :non_validated_ship, :class_name => 'Ship' - has_many :birds + has_many :birds, :order => 'birds.id ASC' has_many :birds_with_method_callbacks, :class_name => "Bird", :before_add => :log_before_add, :after_add => :log_after_add, From 5c5b73518a5b346a080bb29c676eac901bd49776 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 9 Jun 2010 15:38:06 -0400 Subject: [PATCH 19/35] You dont have to manually mention the application layout, its automatically used --- .../app/templates/app/controllers/application_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb b/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb index f2569b3a77..e8065d9505 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb +++ b/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb @@ -1,4 +1,3 @@ class ApplicationController < ActionController::Base protect_from_forgery - layout 'application' end From 2f398f23d68da41c058a9b8b27b2bd3994f9a8de Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 9 Jun 2010 15:47:43 -0400 Subject: [PATCH 20/35] Remove needless links and search box and update doc links --- .../rails/app/templates/public/index.html | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/railties/lib/rails/generators/rails/app/templates/public/index.html b/railties/lib/rails/generators/rails/app/templates/public/index.html index 9fb304a66b..c65593e8bc 100644 --- a/railties/lib/rails/generators/rails/app/templates/public/index.html +++ b/railties/lib/rails/generators/rails/app/templates/public/index.html @@ -210,23 +210,6 @@