Merge branch 'master' of git@github.com:rails/rails

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *Edge*
 
+* before_save, before_validation and before_destroy callbacks that return false will now ROLLBACK the transaction.  Previously this would have been committed before the processing was aborted. #891 [Xavier Noria]
+
 * Transactional migrations for databases which support them.  #834 [divoxx, Adam Wiggins, Tarmo Tänav]
 
 * Set config.active_record.timestamped_migrations = false to have migrations with numeric prefix instead of UTC timestamp. #446. [Andrew Stone, Nik Wakelin]

activerecord/lib/active_record/callbacks.rb

@@ -169,6 +169,18 @@ module ActiveRecord
   # If a <tt>before_*</tt> callback returns +false+, all the later callbacks and the associated action are cancelled. If an <tt>after_*</tt> callback returns
   # +false+, all the later callbacks are cancelled. Callbacks are generally run in the order they are defined, with the exception of callbacks
   # defined as methods on the model, which are called last.
+  #
+  # == Transactions
+  #
+  # The entire callback chain of a +save+, <tt>save!</tt>, or +destroy+ call runs
+  # within a transaction. That includes <tt>after_*</tt> hooks. If everything
+  # goes fine a COMMIT is executed once the chain has been completed.
+  #
+  # If a <tt>before_*</tt> callback cancels the action a ROLLBACK is issued. You
+  # can also trigger a ROLLBACK raising an exception in any of the callbacks,
+  # including <tt>after_*</tt> hooks. Note, however, that in that case the client
+  # needs to be aware of it because an ordinary +save+ will raise such exception
+  # instead of quietly returning +false+.
   module Callbacks
     CALLBACKS = %w(
       after_find after_initialize before_save after_save before_create after_create before_update after_update before_validation

activerecord/lib/active_record/transactions.rb

@@ -91,11 +91,11 @@ module ActiveRecord
     end
 
     def destroy_with_transactions #:nodoc:
-      transaction { destroy_without_transactions }
+      with_transaction_returning_status(:destroy_without_transactions)
     end
 
     def save_with_transactions(perform_validation = true) #:nodoc:
-      rollback_active_record_state! { transaction { save_without_transactions(perform_validation) } }
+      rollback_active_record_state! { with_transaction_returning_status(:save_without_transactions, perform_validation) }
     end
 
     def save_with_transactions! #:nodoc:
@@ -118,5 +118,17 @@ module ActiveRecord
       end
       raise
     end
+
+    # Executes +method+ within a transaction and captures its return value as a
+    # status flag. If the status is true the transaction is committed, otherwise
+    # a ROLLBACK is issued. In any case the status flag is returned.
+    def with_transaction_returning_status(method, *args)
+      status = nil
+      transaction do
+        status = send(method, *args)
+        raise ActiveRecord::Rollback unless status
+      end
+      status
+    end
   end
 end

activerecord/test/cases/transactions_test.rb

@@ -2,6 +2,7 @@ require "cases/helper"
 require 'models/topic'
 require 'models/reply'
 require 'models/developer'
+require 'models/book'
 
 class TransactionTest < ActiveRecord::TestCase
   self.use_transactional_fixtures = false
@@ -86,8 +87,7 @@ class TransactionTest < ActiveRecord::TestCase
     assert Topic.find(2).approved?, "Second should still be approved"
   end
 
-
-  def test_callback_rollback_in_save
+  def test_raising_exception_in_callback_rollbacks_in_save
     add_exception_raising_after_save_callback_to_topic
 
     begin
@@ -102,6 +102,54 @@ class TransactionTest < ActiveRecord::TestCase
     end
   end
 
+  def test_cancellation_from_before_destroy_rollbacks_in_destroy
+    add_cancelling_before_destroy_with_db_side_effect_to_topic
+    begin
+      nbooks_before_destroy = Book.count
+      status = @first.destroy
+      assert !status
+      assert_nothing_raised(ActiveRecord::RecordNotFound) { @first.reload }
+      assert_equal nbooks_before_destroy, Book.count
+    ensure
+      remove_cancelling_before_destroy_with_db_side_effect_to_topic
+    end
+  end
+
+  def test_cancellation_from_before_filters_rollbacks_in_save
+    %w(validation save).each do |filter|
+      send("add_cancelling_before_#{filter}_with_db_side_effect_to_topic")
+      begin
+        nbooks_before_save = Book.count
+        original_author_name = @first.author_name
+        @first.author_name += '_this_should_not_end_up_in_the_db'
+        status = @first.save
+        assert !status
+        assert_equal original_author_name, @first.reload.author_name
+        assert_equal nbooks_before_save, Book.count
+      ensure
+        send("remove_cancelling_before_#{filter}_with_db_side_effect_to_topic")
+      end
+    end
+  end
+
+  def test_cancellation_from_before_filters_rollbacks_in_save!
+    %w(validation save).each do |filter|
+      send("add_cancelling_before_#{filter}_with_db_side_effect_to_topic")
+      begin
+        nbooks_before_save = Book.count
+        original_author_name = @first.author_name
+        @first.author_name += '_this_should_not_end_up_in_the_db'
+        @first.save!
+        flunk
+      rescue => e
+        assert_equal original_author_name, @first.reload.author_name
+        assert_equal nbooks_before_save, Book.count
+      ensure
+        send("remove_cancelling_before_#{filter}_with_db_side_effect_to_topic")
+      end
+    end
+  end
+
   def test_callback_rollback_in_create
     new_topic = Topic.new(
       :title => "A new topic",
@@ -221,6 +269,16 @@ class TransactionTest < ActiveRecord::TestCase
     def remove_exception_raising_after_create_callback_to_topic
       Topic.class_eval { remove_method :after_create }
     end
+
+    %w(validation save destroy).each do |filter|
+      define_method("add_cancelling_before_#{filter}_with_db_side_effect_to_topic") do
+        Topic.class_eval "def before_#{filter}() Book.create; false end"
+      end
+
+      define_method("remove_cancelling_before_#{filter}_with_db_side_effect_to_topic") do
+        Topic.class_eval "remove_method :before_#{filter}"
+      end
+    end
 end
 
 if current_adapter?(:PostgreSQLAdapter)

ci/ci_build.rb

@@ -0,0 +1,117 @@
+#!/usr/bin/env ruby
+require 'fileutils'
+
+include FileUtils
+
+puts "[CruiseControl] Rails build"
+
+build_results = {}
+root_dir = File.expand_path(File.dirname(__FILE__) + "/..")
+
+# Requires gem home and path to be writeable and/or overridden to be ~/.gem,
+# Will enable when RubyGems supports this properly (in a coming release)
+# build_results[:geminstaller] = system 'geminstaller --exceptions'
+
+# for now, use the no-passwd sudoers approach (documented in ci_setup_notes.txt)
+# A security hole, but there is nothing valuable on rails CI box anyway.
+build_results[:geminstaller] = system 'sudo geminstaller --exceptions'
+
+cd "#{root_dir}/activesupport" do
+  puts
+  puts "[CruiseControl] Building ActiveSupport"
+  puts
+  build_results[:activesupport] = system 'rake'
+end
+
+cd "#{root_dir}/activerecord" do
+  puts
+  puts "[CruiseControl] Building ActiveRecord with MySQL"
+  puts
+  build_results[:activerecord_mysql] = system 'rake test_mysql'
+end
+
+# Postgres is disabled until tests are fixed
+# cd "#{root_dir}/activerecord" do
+#   puts
+#   puts "[CruiseControl] Building ActiveRecord with PostgreSQL"
+#   puts
+#   build_results[:activerecord_postgresql8] = system 'rake test_postgresql'
+# end
+
+# Sqlite2 is disabled until tests are fixed
+# cd "#{root_dir}/activerecord" do
+#  puts
+#  puts "[CruiseControl] Building ActiveRecord with SQLite 2"
+#  puts
+#  build_results[:activerecord_sqlite] = system 'rake test_sqlite'
+# end
+
+cd "#{root_dir}/activerecord" do
+  puts
+  puts "[CruiseControl] Building ActiveRecord with SQLite 3"
+  puts
+  build_results[:activerecord_sqlite3] = system 'rake test_sqlite3'
+end
+
+cd "#{root_dir}/activemodel" do
+  puts
+  puts "[CruiseControl] Building ActiveModel"
+  puts
+  build_results[:activemodel] = system 'rake'
+end
+
+cd "#{root_dir}/activeresource" do
+  puts
+  puts "[CruiseControl] Building ActiveResource"
+  puts
+  build_results[:activeresource] = system 'rake'
+end
+
+cd "#{root_dir}/actionpack" do
+  puts
+  puts "[CruiseControl] Building ActionPack"
+  puts
+  build_results[:actionpack] = system 'rake'
+end
+
+cd "#{root_dir}/actionmailer" do
+  puts
+  puts "[CruiseControl] Building ActionMailer"
+  puts
+  build_results[:actionmailer] = system 'rake'
+end
+
+cd "#{root_dir}/railties" do
+  puts
+  puts "[CruiseControl] Building RailTies"
+  puts
+  build_results[:railties] = system 'rake'
+end
+
+
+puts
+puts "[CruiseControl] Build environment:"
+puts "[CruiseControl]   #{`cat /etc/issue`}"
+puts "[CruiseControl]   #{`uname -a`}"
+puts "[CruiseControl]   #{`ruby -v`}"
+puts "[CruiseControl]   #{`mysql --version`}"
+puts "[CruiseControl]   #{`pg_config --version`}"
+puts "[CruiseControl]   SQLite2: #{`sqlite -version`}"
+puts "[CruiseControl]   SQLite3: #{`sqlite3 -version`}"
+`gem env`.each {|line| print "[CruiseControl]   #{line}"}
+puts "[CruiseControl]   Local gems:"
+`gem list`.each {|line| print "[CruiseControl]     #{line}"}
+
+failures = build_results.select { |key, value| value == false }
+
+if failures.empty?
+  puts
+  puts "[CruiseControl] Rails build finished sucessfully"
+  exit(0)
+else
+  puts
+  puts "[CruiseControl] Rails build FAILED"
+  puts "[CruiseControl] Failed components: #{failures.map { |component| component.first }.join(', ')}"
+  exit(-1)
+end
+

ci/ci_setup_notes.txt

@@ -0,0 +1,120 @@
+# Rails Continuous Integration Server Setup Notes
+# This procedure was used to set up http://ci.rubyonrails.org on Ubuntu 8.04
+# It can be used as a guideline for setting up your own CI server against your local rails branches
+
+* Set up ci user:
+# log in as root
+$ adduser ci
+enter user info and password
+$ visudo
+# give ci user same sudo rights as root
+
+* Disable root login:
+# log in as ci
+$ sudo vi /etc/shadow
+# overwrite and disable encrypted root password to disable root login:
+root:*:14001:0:99999:7:::
+
+* Change Hostname:
+$ sudo vi /etc/hostname
+change to 'ci'
+$ sudo vi /etc/hosts
+replace old hostname with 'ci'
+# reboot to use new hostname (and test reboot)
+$ sudo shutdown -r now
+
+* Update aptitude:
+$ sudo aptitude update
+
+* Use cinabox to perform rest of ruby/ccrb setup: 
+* http://github.com/thewoolleyman/cinabox/tree/master/README.txt
+
+# This is not yet properly supported by RubyGems...
+# * Configure RubyGems to not require root access for gem installation
+# $ vi ~/.profile
+# # add this line at bottom:
+# PATH="$HOME/.gem/ruby/1.8/bin:$PATH"
+# $ sudo vi /etc/init.d/cruise
+# # edit the start_cruise line to source CRUISE_USER/.profile:
+#   start_cruise "cd #{CRUISE_HOME} && source /home/#{CRUISE_USER}/.profile && ./cruise start -d"
+# $ vi ~/.gemrc
+# # add these lines:
+# ---
+# gemhome: /home/ci/.gem/ruby/1.8
+# gempath: 
+# - /home/ci/.gem/ruby/1.8
+
+* If you did not configure no-root-gem installation via ~/.gemrc as shown above, then allow no-password sudo for gem installation:
+$ sudo visudo
+# add this line to bottom:
+ci      ALL=NOPASSWD: /usr/local/bin/geminstaller, /usr/local/bin/ruby, /usr/local/bin/gem
+
+* Start ccrb via init script and check for default homepage at port 3333
+
+* Install/setup nginx:
+$ sudo aptitude install nginx
+$ sudo vi /etc/nginx/sites-available/default
+# comment two lines and add one to proxy to ccrb:
+#               root   /var/www/nginx-default;
+#               index  index.html index.htm;
+                proxy_pass      http://127.0.0.1:3333;
+$ sudo /etc/init.d/nginx start
+
+* Add project to cruise (It will still fail until everything is set up):
+$ cd ~/ccrb
+$ ./cruise add rails -s git -r git://github.com/rails/rails.git  # or the URI of your branch
+
+* Copy and configure cruise site config file:
+$ cp ~/.cruise/projects/rails/work/ci/site_config.rb ~/.cruise/site_config.rb
+# Edit ~/.cruise/site_config.rb as desired, for example:
+ActionMailer::Base.smtp_settings = {
+  :address =>        "localhost",
+  :domain =>         "ci.yourdomain.com",
+}
+Configuration.dashboard_refresh_interval = 60.seconds
+Configuration.dashboard_url = 'http://ci.yourdomain.com/'
+Configuration.serialize_builds = true
+Configuration.serialized_build_timeout = 1.hours
+BuildReaper.number_of_builds_to_keep = 100
+  
+* Copy and configure cruise project config file
+$ cp ~/.cruise/projects/rails/work/ci/cruise_config.rb ~/.cruise/projects/rails
+$ vi ~/.cruise/projects/rails/cruise_config.rb:
+# Edit ~/.cruise/projects/rails/cruise_config.rb as desired, for example:
+Project.configure do |project|
+ project.build_command = 'ruby ci/ci_build.rb'
+ project.email_notifier.emails = ['recipient@yourdomain.com']
+ project.email_notifier.from = 'sender@yourdomain.com'
+end
+
+* Set up mysql
+$ sudo aptitude install mysql-server-5.0 libmysqlclient-dev
+# no password for mysql root user
+
+* setup sqlite
+$ sudo aptitude install sqlite sqlite3 libsqlite-dev libsqlite3-dev
+# Note: there's some installation bugs with sqlite3-ruby 1.2.2 gem file permissions:
+#   http://www.icoretech.org/2008/07/06/no-such-file-to-load-sqlite3-database
+#   cd /usr/local/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.2.2 && sudo find . -perm 0662 -exec chmod 664 {} \;
+
+* setup postgres
+$ sudo aptitude install postgresql postgresql-server-dev-8.3
+$ sudo su - postgres -c 'createuser -s ci'
+
+* Install and run GemInstaller to get all dependency gems
+$ sudo gem install geminstaller
+$ cd ~/.cruise/projects/rails/work
+$ sudo geminstaller --config=ci/geminstaller.yml # turn up debugging with these options: --geminstaller-output=all --rubygems-output=all
+
+* Create ActiveRecord test databases for mysql
+$ mysql -uroot -e 'grant all on *.* to rails@localhost;'
+$ mysql -urails -e 'create database activerecord_unittest;'
+$ mysql -urails -e 'create database activerecord_unittest2;'
+
+* Create ActiveRecord test databases for postgres
+# cd to rails activerecord dir
+$ rake postgresql:build_databases
+
+* Reboot and make sure everything is working
+$ sudo shutdown -r now
+$ http://ci.yourdomain.com

ci/cruise_config.rb

@@ -0,0 +1,5 @@
+Project.configure do |project|
+ project.build_command = 'ruby ci/ci_build.rb'
+ project.email_notifier.emails = ['thewoolleyman@gmail.com','michael@koziarski.com']
+ project.email_notifier.from = 'thewoolleyman+railsci@gmail.com'
+end

ci/geminstaller.yml

@@ -0,0 +1,17 @@
+---
+gems:
+- name: geminstaller
+  version: >= 0.4.3
+- name: mocha
+  version: >= 0.9.0
+- name: mysql
+  #version: >= 2.7
+  version: = 2.7
+- name: postgres
+  version: >= 0.7.9.2008.01.28
+- name: rake
+  version: >= 0.8.1
+- name: sqlite-ruby
+  version: >= 2.2.3
+- name: sqlite3-ruby
+  version: >= 1.2.2

ci/site.css

@@ -0,0 +1,13 @@
+/* this is a copy of /home/ci/.cruise/site.css, please make any changes to it there */
+
+/* this is a copy of /home/ci/.cruise/site.css, please make any changes to it there */
+
+/* if you'd like to add custom styles to cruise, add them here */
+/* the following will make successful builds green */
+a.success, a.success:visited {
+  color: #0A0;
+}
+
+.build_success {
+  background-image: url(/images/green_gradient.png);
+}

ci/site_config.rb

@@ -0,0 +1,72 @@
+# site_config.rb contains examples of various configuration options for the local installation
+# of CruiseControl.rb.
+
+# YOU MUST RESTART YOUR CRUISE CONTROL SERVER FOR ANY CHANGES MADE HERE TO TAKE EFFECT!!!
+
+# EMAIL NOTIFICATION
+# ------------------
+
+# CruiseControl.rb can notify you about build status via email. It uses ActionMailer component of Ruby on Rails 
+# framework. Obviously, ActionMailer needs to know how to send out email messages. 
+# If you have an SMTP server on your network, and it needs no authentication, write this in your site_config.rb:
+# 
+ActionMailer::Base.smtp_settings = {
+  :address =>        "localhost",
+  :domain =>         "ci.rubyonrails.org",
+}
+#
+# If you have no SMTP server at hand, you can configure email notification to use GMail SMTP server, as follows
+# (of course, you'll need to create a GMail account):
+#
+# ActionMailer::Base.smtp_settings = {
+#   :address =>        "smtp.gmail.com",
+#   :port =>           587,
+#   :domain =>         "yourdomain.com",
+#   :authentication => :plain,
+#   :user_name =>      "yourgmailaccount",
+#   :password =>       "yourgmailpassword"
+# }
+# 
+# The same approach works for other SMTP servers thet require authentication. Note that GMail's SMTP server runs on a 
+# non-standard port 587 (standard port for SMTP is 25).
+#
+# For further details about configuration of outgoing email, see Ruby On Rails documentation for ActionMailer::Base.
+
+# Other site-wide options are available through Configuration class:
+
+# Change how often CC.rb pings Subversion for new requests. Default is 10.seconds, which should be OK for a local
+# SVN repository, but probably isn't very polite for a public repository, such as RubyForge. This can also be set for
+# each project individually, through project.scheduler.polling_interval option:
+# Configuration.default_polling_interval = 1.minute
+
+# How often the dashboard page refreshes itself. If you have more than 10-20 dashboards open,
+# it is advisable to set it to something higher than the default 5 seconds:
+Configuration.dashboard_refresh_interval = 60.seconds
+
+# Site-wide setting for the email "from" field. This can also be set on per-project basis,
+# through project.email.notifier.from attribute
+Configuration.email_from = 'thewoolleyman+railsci@gmail.com'
+
+# Root URL of the dashboard application. Setting this attribute allows various notifiers to include a link to the
+# build page in the notification message.
+Configuration.dashboard_url = 'http://ci.rubyonrails.org/'
+
+# If you don't want to allow triggering builds through dashboard Build Now button. Useful when you host CC.rb as a
+# public web site (such as http://cruisecontrolrb.thoughtworks.com/projects - try clicking on Build Now button there
+# and see what happens):
+# Configuration.disable_build_now = true
+
+# If you want to only allow one project to build at a time, uncomment this line
+# by default, cruise allows multiple projects to build at a time
+Configuration.serialize_builds = true
+
+# Amount of time a project will wait to build before failing when build serialization is on
+Configuration.serialized_build_timeout = 3.hours
+
+# To delete build when there are more than a certain number present, uncomment this line - it will make the dashboard 
+# perform better
+BuildReaper.number_of_builds_to_keep = 100
+
+# any files that you'd like to override in cruise, keep in ~/.cruise, and copy over when this file is loaded like this
+site_css = CRUISE_DATA_ROOT + "/site.css"
+FileUtils.cp site_css, RAILS_ROOT + "/public/stylesheets/site.css" if File.exists? site_css

railties/lib/commands/dbconsole.rb

@@ -47,7 +47,7 @@ when "mysql"
 
   args << config['database']
 
-  exec(find_cmd('mysql5', 'mysql'), *args)
+  exec(find_cmd('mysql', 'mysql5'), *args)
 
 when "postgresql"
   ENV['PGUSER']     = config["username"] if config["username"]