Skip to content

Commit

Permalink
Bring back "database already exists" messages when running rake tasks
Browse files Browse the repository at this point in the history
When running tasks such "rake db:setup", instead of showing messages
like "db_development already exists", it was showing a big stack trace
and a message "Couldn't create database for ..." with the configuration
options, a very confusing message with a big trace.

This brings back the functionality present in 3-2, showing the same
message.
  • Loading branch information
carlosantoniodasilva committed Jan 12, 2013
1 parent c1d7225 commit 9b636dc
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 20 deletions.
4 changes: 4 additions & 0 deletions activerecord/lib/active_record/tasks/database_tasks.rb
Original file line number Original file line Diff line number Diff line change
@@ -1,5 +1,7 @@
module ActiveRecord module ActiveRecord
module Tasks # :nodoc: module Tasks # :nodoc:
class DatabaseAlreadyExists < StandardError; end # :nodoc:

module DatabaseTasks # :nodoc: module DatabaseTasks # :nodoc:
extend self extend self


Expand Down Expand Up @@ -32,6 +34,8 @@ def current_config(options = {})
def create(*arguments) def create(*arguments)
configuration = arguments.first configuration = arguments.first
class_for_adapter(configuration['adapter']).new(*arguments).create class_for_adapter(configuration['adapter']).new(*arguments).create
rescue DatabaseAlreadyExists
$stderr.puts "#{configuration['database']} already exists"
rescue Exception => error rescue Exception => error
$stderr.puts error, *(error.backtrace) $stderr.puts error, *(error.backtrace)
$stderr.puts "Couldn't create database for #{configuration.inspect}" $stderr.puts "Couldn't create database for #{configuration.inspect}"
Expand Down
18 changes: 9 additions & 9 deletions activerecord/lib/active_record/tasks/mysql_database_tasks.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ def create
connection.create_database configuration['database'], creation_options connection.create_database configuration['database'], creation_options
connection.execute grant_statement.gsub(/\s+/, ' ').strip connection.execute grant_statement.gsub(/\s+/, ' ').strip
establish_connection configuration establish_connection configuration
rescue error_class => error rescue error_class, ActiveRecord::StatementInvalid => error
$stderr.puts error.error if /database exists/ === error.message
$stderr.puts "Couldn't create database for #{configuration.inspect}, #{creation_options.inspect}" raise DatabaseAlreadyExists
$stderr.puts "(If you set the charset manually, make sure you have a matching collation)" if configuration['encoding'] else
$stderr.puts "Couldn't create database for #{configuration.inspect}, #{creation_options.inspect}"
$stderr.puts "(If you set the charset manually, make sure you have a matching collation)" if configuration['encoding']
end
end end


def drop def drop
Expand Down Expand Up @@ -87,14 +90,11 @@ def creation_options
end end


def error_class def error_class
case configuration['adapter'] if configuration['adapter'] =~ /jdbc/
when /jdbc/
require 'active_record/railties/jdbcmysql_error' require 'active_record/railties/jdbcmysql_error'
ArJdbcMySQL::Error ArJdbcMySQL::Error
when /mysql2/
Mysql2::Error
else else
Mysql::Error defined?(Mysql2) ? Mysql2::Error : Mysql::Error
end end
end end


Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ def create(master_established = false)
connection.create_database configuration['database'], connection.create_database configuration['database'],
configuration.merge('encoding' => encoding) configuration.merge('encoding' => encoding)
establish_connection configuration establish_connection configuration
rescue ActiveRecord::StatementInvalid => error
if /database .* already exists/ === error.message
raise DatabaseAlreadyExists
else
raise
end
end end


def drop def drop
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@ def initialize(configuration, root = Rails.root)
end end


def create def create
if File.exist?(configuration['database']) raise DatabaseAlreadyExists if File.exist?(configuration['database'])
$stderr.puts "#{configuration['database']} already exists"
return
end


establish_connection configuration establish_connection configuration
connection connection
Expand Down
14 changes: 7 additions & 7 deletions activerecord/test/cases/tasks/database_tasks_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ def setup
end end


ADAPTERS_TASKS = { ADAPTERS_TASKS = {
:mysql => :mysql_tasks, mysql: :mysql_tasks,
:mysql2 => :mysql_tasks, mysql2: :mysql_tasks,
:postgresql => :postgresql_tasks, postgresql: :postgresql_tasks,
:sqlite3 => :sqlite_tasks sqlite3: :sqlite_tasks
} }


class DatabaseTasksRegisterTask < ActiveRecord::TestCase class DatabaseTasksRegisterTask < ActiveRecord::TestCase
Expand All @@ -32,7 +32,7 @@ def structure_dump(filename); end
ActiveRecord::Tasks::DatabaseTasks.structure_dump({'adapter' => :foo}, "awesome-file.sql") ActiveRecord::Tasks::DatabaseTasks.structure_dump({'adapter' => :foo}, "awesome-file.sql")
end end
end end

class DatabaseTasksCreateTest < ActiveRecord::TestCase class DatabaseTasksCreateTest < ActiveRecord::TestCase
include DatabaseTasksSetupper include DatabaseTasksSetupper


Expand Down Expand Up @@ -258,7 +258,7 @@ class DatabaseTasksPurgeTest < ActiveRecord::TestCase


class DatabaseTasksCharsetTest < ActiveRecord::TestCase class DatabaseTasksCharsetTest < ActiveRecord::TestCase
include DatabaseTasksSetupper include DatabaseTasksSetupper

ADAPTERS_TASKS.each do |k, v| ADAPTERS_TASKS.each do |k, v|
define_method("test_#{k}_charset") do define_method("test_#{k}_charset") do
eval("@#{v}").expects(:charset) eval("@#{v}").expects(:charset)
Expand All @@ -269,7 +269,7 @@ class DatabaseTasksCharsetTest < ActiveRecord::TestCase


class DatabaseTasksCollationTest < ActiveRecord::TestCase class DatabaseTasksCollationTest < ActiveRecord::TestCase
include DatabaseTasksSetupper include DatabaseTasksSetupper

ADAPTERS_TASKS.each do |k, v| ADAPTERS_TASKS.each do |k, v|
define_method("test_#{k}_collation") do define_method("test_#{k}_collation") do
eval("@#{v}").expects(:collation) eval("@#{v}").expects(:collation)
Expand Down
10 changes: 10 additions & 0 deletions activerecord/test/cases/tasks/mysql_rake_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ def test_establishes_connection_to_database


ActiveRecord::Tasks::DatabaseTasks.create @configuration ActiveRecord::Tasks::DatabaseTasks.create @configuration
end end

def test_create_when_database_exists_outputs_info_to_stderr
$stderr.expects(:puts).with("my-app-db already exists").once

ActiveRecord::Base.connection.stubs(:create_database).raises(
ActiveRecord::StatementInvalid.new("Can't create database 'dev'; database exists:")
)

ActiveRecord::Tasks::DatabaseTasks.create @configuration
end
end end


class MysqlDBCreateAsRootTest < ActiveRecord::TestCase class MysqlDBCreateAsRootTest < ActiveRecord::TestCase
Expand Down
10 changes: 10 additions & 0 deletions activerecord/test/cases/tasks/postgresql_rake_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ def test_db_create_with_error_prints_message


ActiveRecord::Tasks::DatabaseTasks.create @configuration ActiveRecord::Tasks::DatabaseTasks.create @configuration
end end

def test_create_when_database_exists_outputs_info_to_stderr
$stderr.expects(:puts).with("my-app-db already exists").once

ActiveRecord::Base.connection.stubs(:create_database).raises(
ActiveRecord::StatementInvalid.new('database "my-app-db" already exists')
)

ActiveRecord::Tasks::DatabaseTasks.create @configuration
end
end end


class PostgreSQLDBDropTest < ActiveRecord::TestCase class PostgreSQLDBDropTest < ActiveRecord::TestCase
Expand Down

0 comments on commit 9b636dc

Please sign in to comment.