diff options
64 files changed, 799 insertions, 200 deletions
@@ -30,7 +30,7 @@ instance_eval File.read local_gemfile if File.exists? local_gemfile platforms :mri do group :test do gem 'ruby-prof', '~> 0.11.2' if RUBY_VERSION < '2.0' - gem 'debugger' if !ENV['TRAVIS'] && RUBY_VERSION < '2.0' + gem 'debugger' if !ENV['TRAVIS'] end end @@ -53,7 +53,7 @@ end platforms :jruby do gem 'json' - gem 'activerecord-jdbcsqlite3-adapter', '>= 1.2.0' + gem 'activerecord-jdbcsqlite3-adapter', '>= 1.2.7' # This is needed by now to let tests work on JRuby # TODO: When the JRuby guys merge jruby-openssl in @@ -61,8 +61,8 @@ platforms :jruby do gem 'jruby-openssl' group :db do - gem 'activerecord-jdbcmysql-adapter', '>= 1.2.0' - gem 'activerecord-jdbcpostgresql-adapter', '>= 1.2.0' + gem 'activerecord-jdbcmysql-adapter', '>= 1.2.7' + gem 'activerecord-jdbcpostgresql-adapter', '>= 1.2.7' end end diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3fc3e06160..f757911c23 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,26 @@ ## Rails 4.0.0 (unreleased) ## +* Fix incorrectly appended square brackets to a multiple select box + if an explicit name has been given and it already ends with "[]" + + Before: + + select(:category, [], {}, multiple: true, name: "post[category][]") + # => <select name="post[category][][]" ...> + + After: + + select(:category, [], {}, multiple: true, name: "post[category][]") + # => <select name="post[category][]" ...> + + *Olek Janiszewski* + +* Fixed regression when using `assert_template` to verify files sent using + `render file: 'README.md'`. + Fixes #9464. + + *Justin Coyne* + * Fixed `ActionView::Helpers::CaptureHelper#content_for` regression when trying to use it in a boolean statement. Fixes #9360. diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 32e5afa335..9d628c916f 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -14,6 +14,7 @@ module ActionController # response.stream.write "hello world\n" # sleep 1 # } + # ensure # response.stream.close # end # end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index bba1f1e201..e12bf0a1c6 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -16,6 +16,7 @@ module ActionController @_partials = Hash.new(0) @_templates = Hash.new(0) @_layouts = Hash.new(0) + @_files = Hash.new(0) ActiveSupport::Notifications.subscribe("render_template.action_view") do |name, start, finish, id, payload| path = payload[:layout] @@ -39,6 +40,16 @@ module ActionController @_templates[path] += 1 end + + ActiveSupport::Notifications.subscribe("!render_template.action_view") do |name, start, finish, id, payload| + path = payload[:identifier] + next if payload[:virtual_path] # files don't have virtual path + if path + @_files[path] += 1 + @_files[path.split("/").last] += 1 + end + + end end def teardown_subscriptions @@ -106,7 +117,7 @@ module ActionController end assert matches_template, msg when Hash - options.assert_valid_keys(:layout, :partial, :locals, :count) + options.assert_valid_keys(:layout, :partial, :locals, :count, :file) if options.key?(:layout) expected_layout = options[:layout] @@ -123,6 +134,10 @@ module ActionController end end + if options[:file] + assert_includes @_files.keys, options[:file] + end + if expected_partial = options[:partial] if expected_locals = options[:locals] if defined?(@_rendered_views) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb index 6b1f233930..550f4dbd0d 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb @@ -10,7 +10,7 @@ clean_params.delete("action") clean_params.delete("controller") - request_dump = clean_params.empty? ? 'None' : clean_params.inspect.tr(',', ",\n") + request_dump = clean_params.empty? ? 'None' : clean_params.inspect.gsub(',', ",\n") def debug_hash(object) object.to_hash.sort_by { |k, v| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") @@ -31,4 +31,4 @@ </div> <h2 style="margin-top: 30px">Response</h2> -<p><b>Headers</b>:</p> <pre><%= defined?(@response) ? @response.headers.inspect.tr(',', ",\n") : 'None' %></pre> +<p><b>Headers</b>:</p> <pre><%= defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %></pre> diff --git a/actionpack/lib/action_view/helpers/debug_helper.rb b/actionpack/lib/action_view/helpers/debug_helper.rb index db7f9f3494..c29c1b1eea 100644 --- a/actionpack/lib/action_view/helpers/debug_helper.rb +++ b/actionpack/lib/action_view/helpers/debug_helper.rb @@ -28,7 +28,7 @@ module ActionView # </pre> def debug(object) Marshal::dump(object) - object = ERB::Util.html_escape(object.to_yaml).tr(" ", " ").html_safe + object = ERB::Util.html_escape(object.to_yaml).gsub(" ", " ").html_safe content_tag(:pre, object, :class => "debug_dump") rescue Exception # errors from Marshal or YAML # Object couldn't be dumped, perhaps because of singleton methods -- this is the fallback diff --git a/actionpack/lib/action_view/helpers/tags/base.rb b/actionpack/lib/action_view/helpers/tags/base.rb index 3d597079c4..aef1572290 100644 --- a/actionpack/lib/action_view/helpers/tags/base.rb +++ b/actionpack/lib/action_view/helpers/tags/base.rb @@ -84,7 +84,7 @@ module ActionView options["id"] = options.fetch("id"){ tag_id } end - options["name"] += "[]" if options["multiple"] + options["name"] += "[]" if options["multiple"] && !options["name"].ends_with?("[]") options["id"] = [options.delete('namespace'), options["id"]].compact.join("_").presence end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index f73d14c79b..720890eeb9 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -138,7 +138,7 @@ module ActionView # we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. def render(view, locals, buffer=nil, &block) - ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do + ActiveSupport::Notifications.instrument("!render_template.action_view", virtual_path: @virtual_path, identifier: @identifier) do compile!(view) view.send(method_name, locals, buffer, &block) end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 1a1083bf00..47bd011d5e 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -109,7 +109,7 @@ module ActionView @cache.clear end - # Normalizes the arguments and passes it on to find_template. + # Normalizes the arguments and passes it on to find_templates. def find_all(name, prefix=nil, partial=false, details={}, key=nil, locals=[]) cached(key, [name, prefix, partial], details, locals) do find_templates(name, prefix, partial, details) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 463f192d0c..10b487f37a 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -219,6 +219,7 @@ module ActionView :@_routes, :@controller, :@_layouts, + :@_files, :@_rendered_views, :@method_name, :@output_buffer, diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 5d727b3811..22a410db94 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -96,6 +96,14 @@ class ActionPackAssertionsController < ActionController::Base raise "post" if request.post? render :text => "request method: #{request.env['REQUEST_METHOD']}" end + + def render_file_absolute_path + render :file => File.expand_path('../../../README.rdoc', __FILE__) + end + + def render_file_relative_path + render :file => 'README.rdoc' + end end # Used to test that assert_response includes the exception message @@ -142,6 +150,16 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase assert_tag :content => "/action_pack_assertions/flash_me" end + def test_render_file_absolute_path + get :render_file_absolute_path + assert_match(/\A= Action Pack/, @response.body) + end + + def test_render_file_relative_path + get :render_file_relative_path + assert_match(/\A= Action Pack/, @response.body) + end + def test_get_request assert_raise(RuntimeError) { get :raise_exception_on_get } get :raise_exception_on_post @@ -441,6 +459,23 @@ class AssertTemplateTest < ActionController::TestCase assert_template :partial => '_partial' end + def test_file_with_absolute_path_success + get :render_file_absolute_path + assert_template :file => File.expand_path('../../../README.rdoc', __FILE__) + end + + def test_file_with_relative_path_success + get :render_file_relative_path + assert_template :file => 'README.rdoc' + end + + def test_with_file_failure + get :render_file_absolute_path + assert_raise(ActiveSupport::TestCase::Assertion) do + assert_template :file => 'test/hello_world' + end + end + def test_with_nil_passes_when_no_template_rendered get :nothing assert_template nil diff --git a/actionpack/test/template/debug_helper_test.rb b/actionpack/test/template/debug_helper_test.rb new file mode 100644 index 0000000000..42d06bd9ff --- /dev/null +++ b/actionpack/test/template/debug_helper_test.rb @@ -0,0 +1,8 @@ +require 'active_record_unit' + +class DebugHelperTest < ActionView::TestCase + def test_debug + company = Company.new(name: "firebase") + assert_match " name: firebase", debug(company) + end +end diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb index 04cdd068c8..29d63d9653 100644 --- a/actionpack/test/template/form_options_helper_test.rb +++ b/actionpack/test/template/form_options_helper_test.rb @@ -575,6 +575,14 @@ class FormOptionsHelperTest < ActionView::TestCase ) end + def test_select_with_multiple_and_with_explicit_name_ending_with_brackets + output_buffer = select(:post, :category, [], {include_hidden: false}, multiple: true, name: 'post[category][]') + assert_dom_equal( + "<select multiple=\"multiple\" id=\"post_category\" name=\"post[category][]\"></select>", + output_buffer + ) + end + def test_select_with_multiple_and_disabled_to_add_disabled_hidden_input output_buffer = select(:post, :category, "", {}, :multiple => true, :disabled => true) assert_dom_equal( diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 1fe6dbd4d9..c6d7b0b5d3 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,5 +1,32 @@ ## Rails 4.0.0 (unreleased) ## +* `has_secure_password` does not fail the confirmation validation + when assigning empty String to `password` and `password_confirmation`. + + Example: + + # Given User has_secure_password. + @user.password = "" + @user.password_confirmation = "" + @user.valid?(:update) # used to be false + + *Yves Senn* + +* `validates_confirmation_of` does not override writer methods for + the confirmation attribute if no reader is defined. + + Example: + + class Blog + def title=(new_title) + @title = new_title.downcase + end + + # previously this would override the setter above. + validates_confirmation_of :title + end + + *Yves Senn* ## Rails 4.0.0.beta1 (February 25, 2013) ## diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 6644b60609..9324a1ad0a 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -48,6 +48,8 @@ module ActiveModel attr_reader :password + include InstanceMethodsOnActivation + if options.fetch(:validations, true) validates_confirmation_of :password validates_presence_of :password, :on => :create @@ -55,8 +57,6 @@ module ActiveModel before_create { raise "Password digest missing on new record" if password_digest.blank? } end - include InstanceMethodsOnActivation - if respond_to?(:attributes_protected_by_default) def self.attributes_protected_by_default #:nodoc: super + ['password_digest'] @@ -99,6 +99,12 @@ module ActiveModel self.password_digest = BCrypt::Password.create(unencrypted_password, cost: cost) end end + + def password_confirmation=(unencrypted_password) + unless unencrypted_password.blank? + @password_confirmation = unencrypted_password + end + end end end end diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb index 3a3abce364..d14fb4dc53 100644 --- a/activemodel/lib/active_model/validations/confirmation.rb +++ b/activemodel/lib/active_model/validations/confirmation.rb @@ -10,9 +10,13 @@ module ActiveModel end def setup(klass) - klass.send(:attr_accessor, *attributes.map do |attribute| + klass.send(:attr_reader, *attributes.map do |attribute| :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation") end.compact) + + klass.send(:attr_writer, *attributes.map do |attribute| + :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation=") + end.compact) end end diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index 7783bb25d5..02cd3b8a93 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -88,4 +88,10 @@ class SecurePasswordTest < ActiveModel::TestCase @user.password = "secret" assert_equal BCrypt::Engine::MIN_COST, @user.password_digest.cost end + + test "blank password_confirmation does not result in a confirmation error" do + @user.password = "" + @user.password_confirmation = "" + assert @user.valid?(:update), "user should be valid" + end end diff --git a/activemodel/test/cases/validations/confirmation_validation_test.rb b/activemodel/test/cases/validations/confirmation_validation_test.rb index f7556a249f..814eec3f59 100644 --- a/activemodel/test/cases/validations/confirmation_validation_test.rb +++ b/activemodel/test/cases/validations/confirmation_validation_test.rb @@ -71,4 +71,35 @@ class ConfirmationValidationTest < ActiveModel::TestCase I18n.backend = @old_backend end + test "does not override confirmation reader if present" do + klass = Class.new do + include ActiveModel::Validations + + def title_confirmation + "expected title" + end + + validates_confirmation_of :title + end + + assert_equal "expected title", klass.new.title_confirmation, + "confirmation validation should not override the reader" + end + + test "does not override confirmation writer if present" do + klass = Class.new do + include ActiveModel::Validations + + def title_confirmation=(value) + @title_confirmation = "expected title" + end + + validates_confirmation_of :title + end + + model = klass.new + model.title_confirmation = "new title" + assert_equal "expected title", model.title_confirmation, + "confirmation validation should not override the writer" + end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 97616ffc58..49f7b03464 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,124 @@ ## Rails 4.0.0 (unreleased) ## +* When copying migrations, preserve their magic comments and content encoding. + + *OZAWA Sakuro* + +* Fix ActiveRecord `subclass_from_attrs` when `eager_load` is false. + It cannot find subclass because all classes are loaded automatically + when it needs. + + *Dmitry Vorotilin* + +* When `:name` option is provided to `remove_index`, use it if there is no + index by the conventional name. + + For example, previously if an index was removed like so + `remove_index :values, column: :value, name: 'a_different_name'` + the generated SQL would not contain the specified index name, + and hence the migration would fail. + Fixes #8858. + + *Ezekiel Smithburg* + +* Created block to by-pass the prepared statement bindings. + This will allow to compose fragments of large SQL statements to + avoid multiple round-trips between Ruby and the DB. + + Example: + + sql = Post.connection.unprepared_statement do + Post.first.comments.to_sql + end + + *Cédric Fabianski* + +* Change the semantics of combining scopes to be the same as combining + class methods which return scopes. For example: + + class User < ActiveRecord::Base + scope :active, -> { where state: 'active' } + scope :inactive, -> { where state: 'inactive' } + end + + class Post < ActiveRecord::Base + def self.active + where state: 'active' + end + + def self.inactive + where state: 'inactive' + end + end + + ### BEFORE ### + + User.where(state: 'active').where(state: 'inactive') + # => SELECT * FROM users WHERE state = 'active' AND state = 'inactive' + + User.active.inactive + # => SELECT * FROM users WHERE state = 'inactive' + + Post.active.inactive + # => SELECT * FROM posts WHERE state = 'active' AND state = 'inactive' + + ### AFTER ### + + User.active.inactive + # => SELECT * FROM posts WHERE state = 'active' AND state = 'inactive' + + Before this change, invoking a scope would merge it into the current + scope and return the result. `Relation#merge` applies "last where + wins" logic to de-duplicate the conditions, but this lead to + confusing and inconsistent behaviour. This fixes that. + + If you really do want the "last where wins" logic, you can opt-in to + it like so: + + User.active.merge(User.inactive) + + Fixes #7365. + + *Neeraj Singh* and *Jon Leighton* + +* Expand `#cache_key` to consult all relevant updated timestamps. + + Previously only `updated_at` column was checked, now it will + consult other columns that received updated timestamps on save, + such as `updated_on`. When multiple columns are present it will + use the most recent timestamp. + Fixes #9033. + + *Brendon Murphy* + +* Throw `NotImplementedError` when trying to instantiate `ActiveRecord::Base` or an abstract class. + + *Aaron Weiner* + +* Warn when `rake db:structure:dump` with a mysl database and + `mysqldump` is not in the PATH or fails. + Fixes #9518. + + *Yves Senn* + +* Remove `connection#structure_dump`, which is no longer used. *Yves Senn* + +* Make it possible to execute migrations without a transaction even + if the database adapter supports DDL transactions. + Fixes #9483. + + Example: + + class ChangeEnum < ActiveRecord::Migration + disable_ddl_transaction! + + def up + execute "ALTER TYPE model_size ADD VALUE 'new_value'" + end + end + + *Yves Senn* + * Assigning "0.0" to a nullable numeric column does not make it dirty. Fix #9034. 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 c3d15ca929..c64b542286 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -125,7 +125,8 @@ module ActiveRecord # In order to get around this problem, #transaction will emulate the effect # of nested transactions, by using savepoints: # http://dev.mysql.com/doc/refman/5.0/en/savepoint.html - # Savepoints are supported by MySQL and PostgreSQL, but not SQLite3. + # Savepoints are supported by MySQL and PostgreSQL. SQLite3 version >= '3.6.8' + # supports savepoints. # # It is safe to call this method if a database transaction is already open, # i.e. if #transaction is called within another #transaction block. In case diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 0cce8c7596..e2deb6bfcd 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -516,11 +516,6 @@ module ActiveRecord end alias :remove_belongs_to :remove_reference - # Returns a string of <tt>CREATE TABLE</tt> SQL statement(s) for recreating the - # entire structure of the database. - def structure_dump - end - def dump_schema_information #:nodoc: sm_table = ActiveRecord::Migrator.schema_migrations_table_name @@ -692,6 +687,14 @@ module ActiveRecord index_name = index_name(table_name, options) unless index_name_exists?(table_name, index_name, true) + if options.is_a?(Hash) && options.has_key?(:name) + options_without_column = options.dup + options_without_column.delete :column + index_name_without_column = index_name(table_name, options_without_column) + + return index_name_without_column if index_name_exists?(table_name, index_name_without_column, false) + end + raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' does not exist" end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index ff9de712bc..7949bcb5ce 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -118,6 +118,17 @@ module ActiveRecord @in_use = false end + def unprepared_visitor + self.class::BindSubstitution.new self + end + + def unprepared_statement + old, @visitor = @visitor, unprepared_visitor + yield + ensure + @visitor = old + end + # Returns the human-readable name of the adapter. Use mixed case - one # can always use downcase if needed. def adapter_name diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 9826b18053..f88f5742a8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -143,7 +143,7 @@ module ActiveRecord if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @visitor = Arel::Visitors::MySQL.new self else - @visitor = BindSubstitution.new self + @visitor = unprepared_visitor end end @@ -332,20 +332,6 @@ module ActiveRecord # SCHEMA STATEMENTS ======================================== - def structure_dump #:nodoc: - if supports_views? - sql = "SHOW FULL TABLES WHERE Table_type = 'BASE TABLE'" - else - sql = "SHOW TABLES" - end - - select_all(sql, 'SCHEMA').map { |table| - table.delete('Table_type') - sql = "SHOW CREATE TABLE #{quote_table_name(table.to_a.first.last)}" - exec_query(sql, 'SCHEMA').first['Create Table'] + ";\n\n" - }.join - end - # Drops the database specified on the +name+ attribute # and creates it again using the provided +options+. def recreate_database(name, options = {}) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index c91e1b3fb9..691b2ab37f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -489,7 +489,7 @@ module ActiveRecord if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @visitor = Arel::Visitors::PostgreSQL.new self else - @visitor = BindSubstitution.new self + @visitor = unprepared_visitor end @connection_parameters, @config = connection_parameters, config diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 981c4c96a0..84fa1c7d5a 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -113,7 +113,7 @@ module ActiveRecord if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true }) @visitor = Arel::Visitors::SQLite.new self else - @visitor = BindSubstitution.new self + @visitor = unprepared_visitor end end diff --git a/activerecord/lib/active_record/explain.rb b/activerecord/lib/active_record/explain.rb index b2a9a54af1..3135465dfe 100644 --- a/activerecord/lib/active_record/explain.rb +++ b/activerecord/lib/active_record/explain.rb @@ -6,7 +6,8 @@ module ActiveRecord def collecting_queries_for_explain # :nodoc: current = Thread.current original, current[:available_queries_for_explain] = current[:available_queries_for_explain], [] - return yield, current[:available_queries_for_explain] + yield + return current[:available_queries_for_explain] ensure # Note that the return value above does not depend on this assigment. current[:available_queries_for_explain] = original diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index e630897a4b..f54865c86e 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -15,6 +15,9 @@ module ActiveRecord # and if the inheritance column is attr accessible, it initializes an # instance of the given subclass instead of the base class def new(*args, &block) + if abstract_class? || self == Base + raise NotImplementedError, "#{self} is an abstract class and can not be instantiated." + end if (attrs = args.first).is_a?(Hash) if subclass = subclass_from_attrs(attrs) return subclass.new(*args, &block) @@ -167,11 +170,16 @@ module ActiveRecord # this will ignore the inheritance column and return nil def subclass_from_attrs(attrs) subclass_name = attrs.with_indifferent_access[inheritance_column] - return nil if subclass_name.blank? || subclass_name == self.name - unless subclass = subclasses.detect { |sub| sub.name == subclass_name } - raise ActiveRecord::SubclassNotFound.new("Invalid single-table inheritance type: #{subclass_name} is not a subclass of #{name}") + + if subclass_name.present? && subclass_name != self.name + subclass = subclass_name.safe_constantize + + unless subclasses.include?(subclass) + raise ActiveRecord::SubclassNotFound.new("Invalid single-table inheritance type: #{subclass_name} is not a subclass of #{name}") + end + + subclass end - subclass end end diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index 32d35f0ec1..48c73d7781 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -49,7 +49,7 @@ module ActiveRecord case when new_record? "#{self.class.model_name.cache_key}/new" - when timestamp = self[:updated_at] + when timestamp = max_updated_column_timestamp timestamp = timestamp.utc.to_s(cache_timestamp_format) "#{self.class.model_name.cache_key}/#{id}-#{timestamp}" else diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 823595a128..5d7762ec3a 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -330,6 +330,24 @@ module ActiveRecord # # For a list of commands that are reversible, please see # <tt>ActiveRecord::Migration::CommandRecorder</tt>. + # + # == Transactional Migrations + # + # If the database adapter supports DDL transactions, all migrations will + # automatically be wrapped in a transaction. There are queries that you + # can't execute inside a transaction though, and for these situations + # you can turn the automatic transactions off. + # + # class ChangeEnum < ActiveRecord::Migration + # disable_ddl_transaction! + # + # def up + # execute "ALTER TYPE model_size ADD VALUE 'new_value'" + # end + # end + # + # Remember that you can still open your own transactions, even if you + # are in a Migration with <tt>self.disable_ddl_transaction!</tt>. class Migration autoload :CommandRecorder, 'active_record/migration/command_recorder' @@ -351,6 +369,7 @@ module ActiveRecord class << self attr_accessor :delegate # :nodoc: + attr_accessor :disable_ddl_transaction # :nodoc: end def self.check_pending! @@ -365,8 +384,16 @@ module ActiveRecord new.migrate direction end - cattr_accessor :verbose + # Disable DDL transactions for this migration. + def self.disable_ddl_transaction! + @disable_ddl_transaction = true + end + + def disable_ddl_transaction # :nodoc: + self.class.disable_ddl_transaction + end + cattr_accessor :verbose attr_accessor :name, :version def initialize(name = self.class.name, version = nil) @@ -375,8 +402,8 @@ module ActiveRecord @connection = nil end + self.verbose = true # instantiate the delegate object after initialize is defined - self.verbose = true self.delegate = new # Reverses the migration commands for the given block and @@ -607,8 +634,17 @@ module ActiveRecord source_migrations = ActiveRecord::Migrator.migrations(path) source_migrations.each do |migration| - source = File.read(migration.filename) - source = "# This migration comes from #{scope} (originally #{migration.version})\n#{source}" + source = File.binread(migration.filename) + inserted_comment = "# This migration comes from #{scope} (originally #{migration.version})\n" + if /\A#.*\b(?:en)?coding:\s*\S+/ =~ source + # If we have a magic comment in the original migration, + # insert our comment after the first newline(end of the magic comment line) + # so the magic keep working. + # Note that magic comments must be at the first line(except sh-bang). + source[/\n/] = "\n#{inserted_comment}" + else + source = "#{inserted_comment}#{source}" + end if duplicate = destination_migrations.detect { |m| m.name == migration.name } if options[:on_skip] && duplicate.scope != scope.to_s @@ -622,7 +658,7 @@ module ActiveRecord old_path, migration.filename = migration.filename, new_path last = migration - File.open(migration.filename, "w") { |f| f.write source } + File.binwrite(migration.filename, source) copied << migration options[:on_copy].call(scope, migration, old_path) if options[:on_copy] destination_migrations << migration @@ -663,7 +699,7 @@ module ActiveRecord File.basename(filename) end - delegate :migrate, :announce, :write, :to => :migration + delegate :migrate, :announce, :write, :disable_ddl_transaction, to: :migration private @@ -856,12 +892,12 @@ module ActiveRecord Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger begin - ddl_transaction do + ddl_transaction(migration) do migration.migrate(@direction) record_version_state_after_migrating(migration.version) end rescue => e - canceled_msg = Base.connection.supports_ddl_transactions? ? "this and " : "" + canceled_msg = use_transaction?(migration) ? "this and " : "" raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace end end @@ -935,12 +971,16 @@ module ActiveRecord end # Wrap the migration in a transaction only if supported by the adapter. - def ddl_transaction - if Base.connection.supports_ddl_transactions? + def ddl_transaction(migration) + if use_transaction?(migration) Base.transaction { yield } else yield end end + + def use_transaction?(migration) + !migration.disable_ddl_transaction && Base.connection.supports_ddl_transactions? + end end end diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index c5bd11edbf..602ab9e2f4 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -269,23 +269,36 @@ module ActiveRecord self.nested_attributes_options = nested_attributes_options type = (reflection.collection? ? :collection : :one_to_one) - - # def pirate_attributes=(attributes) - # assign_nested_attributes_for_one_to_one_association(:pirate, attributes, mass_assignment_options) - # end - generated_feature_methods.module_eval <<-eoruby, __FILE__, __LINE__ + 1 - if method_defined?(:#{association_name}_attributes=) - remove_method(:#{association_name}_attributes=) - end - def #{association_name}_attributes=(attributes) - assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes) - end - eoruby + generate_association_writer(association_name, type) else raise ArgumentError, "No association found for name `#{association_name}'. Has it been defined yet?" end end end + + private + + # Generates a writer method for this association. Serves as a point for + # accessing the objects in the association. For example, this method + # could generate the following: + # + # def pirate_attributes=(attributes) + # assign_nested_attributes_for_one_to_one_association(:pirate, attributes) + # end + # + # This redirects the attempts to write objects in an association through + # the helper methods defined below. Makes it seem like the nested + # associations are just regular associations. + def generate_association_writer(association_name, type) + generated_feature_methods.module_eval <<-eoruby, __FILE__, __LINE__ + 1 + if method_defined?(:#{association_name}_attributes=) + remove_method(:#{association_name}_attributes=) + end + def #{association_name}_attributes=(attributes) + assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes) + end + eoruby + end end # Returns ActiveRecord::AutosaveAssociation::marked_for_destruction? It's @@ -371,20 +384,7 @@ module ActiveRecord raise ArgumentError, "Hash or Array expected, got #{attributes_collection.class.name} (#{attributes_collection.inspect})" end - if limit = options[:limit] - limit = case limit - when Symbol - send(limit) - when Proc - limit.call - else - limit - end - - if limit && attributes_collection.size > limit - raise TooManyRecords, "Maximum #{limit} records are allowed. Got #{attributes_collection.size} records instead." - end - end + check_record_limit!(options[:limit], attributes_collection) if attributes_collection.is_a? Hash keys = attributes_collection.keys @@ -433,6 +433,29 @@ module ActiveRecord end end + # Takes in a limit and checks if the attributes_collection has too many + # records. The method will take limits in the form of symbols, procs, and + # number-like objects (anything that can be compared with an integer). + # + # Will raise an TooManyRecords error if the attributes_collection is + # larger than the limit. + def check_record_limit!(limit, attributes_collection) + if limit + limit = case limit + when Symbol + send(limit) + when Proc + limit.call + else + limit + end + + if limit && attributes_collection.size > limit + raise TooManyRecords, "Maximum #{limit} records are allowed. Got #{attributes_collection.size} records instead." + end + end + end + # Updates a record with the +attributes+ or marks it for destruction if # +allow_destroy+ is +true+ and has_destroy_flag? returns +true+. def assign_to_or_mark_for_destruction(record, attributes, allow_destroy) @@ -452,6 +475,11 @@ module ActiveRecord has_destroy_flag?(attributes) || call_reject_if(association_name, attributes) end + # Determines if a record with the particular +attributes+ should be + # rejected by calling the reject_if Symbol or Proc (if defined). + # The reject_if option is defined by +accepts_nested_attributes_for+. + # + # Returns false if there is a +destroy_flag+ on the attributes. def call_reject_if(association_name, attributes) return false if has_destroy_flag?(attributes) case callback = self.nested_attributes_options[association_name][:reject_if] diff --git a/activerecord/lib/active_record/railties/console_sandbox.rb b/activerecord/lib/active_record/railties/console_sandbox.rb index 90b462fad6..604a220303 100644 --- a/activerecord/lib/active_record/railties/console_sandbox.rb +++ b/activerecord/lib/active_record/railties/console_sandbox.rb @@ -1,4 +1,5 @@ -ActiveRecord::Base.connection.begin_db_transaction +ActiveRecord::Base.connection.begin_transaction(joinable: false) + at_exit do - ActiveRecord::Base.connection.rollback_db_transaction + ActiveRecord::Base.connection.rollback_transaction end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index efbae108b9..ad54ba55b6 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -188,8 +188,7 @@ module ActiveRecord # Please see further details in the # {Active Record Query Interface guide}[http://guides.rubyonrails.org/active_record_querying.html#running-explain]. def explain - _, queries = collecting_queries_for_explain { exec_queries } - exec_explain(queries) + exec_explain(collecting_queries_for_explain { exec_queries }) end # Converts relation objects to Array. diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 01fbb96b8e..12317601b6 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -159,10 +159,19 @@ module ActiveRecord end singleton_class.send(:define_method, name) do |*args| - options = body.respond_to?(:call) ? unscoped { body.call(*args) } : body - relation = all.merge(options) + if body.respond_to?(:call) + scope = extension ? body.call(*args).extending(extension) : body.call(*args) - extension ? relation.extending(extension) : relation + if scope + default_scoped = scope.default_scoped + scope = relation.merge(scope) + scope.default_scoped = default_scoped + end + else + scope = body + end + + scope || all end end end diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 17378969a5..10696258c9 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -57,7 +57,10 @@ module ActiveRecord args.concat(["--result-file", "#{filename}"]) args.concat(["--no-data"]) args.concat(["#{configuration['database']}"]) - Kernel.system(*args) + unless Kernel.system(*args) + $stderr.puts "Could not dump the database structure. "\ + "Make sure `mysqldump` is in your PATH and check the command output for warnings." + end end def structure_load(filename) diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index 8ded6d4a86..ae99cff35e 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -98,6 +98,12 @@ module ActiveRecord timestamp_attributes_for_create + timestamp_attributes_for_update end + def max_updated_column_timestamp + if (timestamps = timestamp_attributes_for_update.map { |attr| self[attr] }.compact).present? + timestamps.map { |ts| ts.to_time }.max + end + end + def current_time_from_proper_timezone self.class.default_timezone == :utc ? Time.now.utc : Time.now end diff --git a/activerecord/test/cases/adapters/mysql/reserved_word_test.rb b/activerecord/test/cases/adapters/mysql/reserved_word_test.rb index 5164acf77f..4cf4bc4c61 100644 --- a/activerecord/test/cases/adapters/mysql/reserved_word_test.rb +++ b/activerecord/test/cases/adapters/mysql/reserved_word_test.rb @@ -63,11 +63,6 @@ class MysqlReservedWordTest < ActiveRecord::TestCase assert_nothing_raised { @connection.rename_column(:group, :order, :values) } end - # dump structure of table with reserved word name - def test_structure_dump - assert_nothing_raised { @connection.structure_dump } - end - # introspect table with reserved word name def test_introspect assert_nothing_raised { @connection.columns(:group) } diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index 1265cb927e..fedd9f603c 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -70,12 +70,6 @@ class MysqlConnectionTest < ActiveRecord::TestCase end end - def test_logs_name_structure_dump - @connection.structure_dump - assert_equal "SCHEMA", @connection.logged[0][1] - assert_equal "SCHEMA", @connection.logged[2][1] - end - def test_logs_name_show_variable @connection.show_variable 'foo' assert_equal "SCHEMA", @connection.logged[0][1] diff --git a/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb b/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb index 1017b0758d..98596a7f62 100644 --- a/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb +++ b/activerecord/test/cases/adapters/mysql2/reserved_word_test.rb @@ -63,11 +63,6 @@ class MysqlReservedWordTest < ActiveRecord::TestCase assert_nothing_raised { @connection.rename_column(:group, :order, :values) } end - # dump structure of table with reserved word name - def test_structure_dump - assert_nothing_raised { @connection.structure_dump } - end - # introspect table with reserved word name def test_introspect assert_nothing_raised { @connection.columns(:group) } diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 3a6da0e59f..f5316952b8 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -21,9 +21,9 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase :posts, :tags, :taggings, :comments, :sponsors, :members def test_belongs_to - Client.find(3).firm.name - assert_equal companies(:first_firm).name, Client.find(3).firm.name - assert_not_nil Client.find(3).firm, "Microsoft should have a firm" + firm = Client.find(3).firm + assert_not_nil firm + assert_equal companies(:first_firm).name, firm.name end def test_belongs_to_with_primary_key @@ -232,15 +232,15 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end def test_belongs_to_counter_with_assigning_nil - p = Post.find(1) - c = Comment.find(1) + post = Post.find(1) + comment = Comment.find(1) - assert_equal p.id, c.post_id - assert_equal 2, Post.find(p.id).comments.size + assert_equal post.id, comment.post_id + assert_equal 2, Post.find(post.id).comments.size - c.post = nil + comment.post = nil - assert_equal 1, Post.find(p.id).comments.size + assert_equal 1, Post.find(post.id).comments.size end def test_belongs_to_with_primary_key_counter @@ -263,56 +263,56 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end def test_belongs_to_counter_with_reassigning - t1 = Topic.create("title" => "t1") - t2 = Topic.create("title" => "t2") - r1 = Reply.new("title" => "r1", "content" => "r1") - r1.topic = t1 + topic1 = Topic.create("title" => "t1") + topic2 = Topic.create("title" => "t2") + reply1 = Reply.new("title" => "r1", "content" => "r1") + reply1.topic = topic1 - assert r1.save - assert_equal 1, Topic.find(t1.id).replies.size - assert_equal 0, Topic.find(t2.id).replies.size + assert reply1.save + assert_equal 1, Topic.find(topic1.id).replies.size + assert_equal 0, Topic.find(topic2.id).replies.size - r1.topic = Topic.find(t2.id) + reply1.topic = Topic.find(topic2.id) assert_no_queries do - r1.topic = t2 + reply1.topic = topic2 end - assert r1.save - assert_equal 0, Topic.find(t1.id).replies.size - assert_equal 1, Topic.find(t2.id).replies.size + assert reply1.save + assert_equal 0, Topic.find(topic1.id).replies.size + assert_equal 1, Topic.find(topic2.id).replies.size - r1.topic = nil + reply1.topic = nil - assert_equal 0, Topic.find(t1.id).replies.size - assert_equal 0, Topic.find(t2.id).replies.size + assert_equal 0, Topic.find(topic1.id).replies.size + assert_equal 0, Topic.find(topic2.id).replies.size - r1.topic = t1 + reply1.topic = topic1 - assert_equal 1, Topic.find(t1.id).replies.size - assert_equal 0, Topic.find(t2.id).replies.size + assert_equal 1, Topic.find(topic1.id).replies.size + assert_equal 0, Topic.find(topic2.id).replies.size - r1.destroy + reply1.destroy - assert_equal 0, Topic.find(t1.id).replies.size - assert_equal 0, Topic.find(t2.id).replies.size + assert_equal 0, Topic.find(topic1.id).replies.size + assert_equal 0, Topic.find(topic2.id).replies.size end def test_belongs_to_reassign_with_namespaced_models_and_counters - t1 = Web::Topic.create("title" => "t1") - t2 = Web::Topic.create("title" => "t2") - r1 = Web::Reply.new("title" => "r1", "content" => "r1") - r1.topic = t1 + topic1 = Web::Topic.create("title" => "t1") + topic2 = Web::Topic.create("title" => "t2") + reply1 = Web::Reply.new("title" => "r1", "content" => "r1") + reply1.topic = topic1 - assert r1.save - assert_equal 1, Web::Topic.find(t1.id).replies.size - assert_equal 0, Web::Topic.find(t2.id).replies.size + assert reply1.save + assert_equal 1, Web::Topic.find(topic1.id).replies.size + assert_equal 0, Web::Topic.find(topic2.id).replies.size - r1.topic = Web::Topic.find(t2.id) + reply1.topic = Web::Topic.find(topic2.id) - assert r1.save - assert_equal 0, Web::Topic.find(t1.id).replies.size - assert_equal 1, Web::Topic.find(t2.id).replies.size + assert reply1.save + assert_equal 0, Web::Topic.find(topic1.id).replies.size + assert_equal 1, Web::Topic.find(topic2.id).replies.size end def test_belongs_to_counter_after_save @@ -367,9 +367,9 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end def test_new_record_with_foreign_key_but_no_object - c = Client.new("firm_id" => 1) + client = Client.new("firm_id" => 1) # sometimes tests on Oracle fail if ORDER BY is not provided therefore add always :order with :first - assert_equal Firm.all.merge!(:order => "id").first, c.firm_with_basic_id + assert_equal Firm.all.merge!(:order => "id").first, client.firm_with_basic_id end def test_setting_foreign_key_after_nil_target_loaded diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index af1845c937..e5880a6b7b 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -307,6 +307,20 @@ class BasicsTest < ActiveRecord::TestCase assert_equal("last_read", ex.errors[0].attribute) end + def test_initialize_abstract_class + e = assert_raises(NotImplementedError) do + FirstAbstractClass.new + end + assert_equal("FirstAbstractClass is an abstract class and can not be instantiated.", e.message) + end + + def test_initialize_base + e = assert_raises(NotImplementedError) do + ActiveRecord::Base.new + end + assert_equal("ActiveRecord::Base is an abstract class and can not be instantiated.", e.message) + end + def test_create_after_initialize_without_block cb = CustomBulb.create(:name => 'Dude') assert_equal('Dude', cb.name) @@ -1441,12 +1455,30 @@ class BasicsTest < ActiveRecord::TestCase assert_not_equal key, car.cache_key end - def test_cache_key_format_for_existing_record_with_nil_updated_at + def test_cache_key_format_for_existing_record_with_nil_updated_timestamps dev = Developer.first - dev.update_columns(updated_at: nil) + dev.update_columns(updated_at: nil, updated_on: nil) assert_match(/\/#{dev.id}$/, dev.cache_key) end + def test_cache_key_for_updated_on + dev = Developer.first + dev.updated_at = nil + assert_equal "developers/#{dev.id}-#{dev.updated_on.utc.to_s(:nsec)}", dev.cache_key + end + + def test_cache_key_for_newer_updated_at + dev = Developer.first + dev.updated_at += 3600 + assert_equal "developers/#{dev.id}-#{dev.updated_at.utc.to_s(:nsec)}", dev.cache_key + end + + def test_cache_key_for_newer_updated_on + dev = Developer.first + dev.updated_on += 3600 + assert_equal "developers/#{dev.id}-#{dev.updated_on.utc.to_s(:nsec)}", dev.cache_key + end + def test_touch_should_raise_error_on_a_new_object company = Company.new(:rating => 1, :name => "37signals", :firm_name => "37signals") assert_raises(ActiveRecord::ActiveRecordError) do diff --git a/activerecord/test/cases/explain_test.rb b/activerecord/test/cases/explain_test.rb index b1d276f9eb..6dac5db111 100644 --- a/activerecord/test/cases/explain_test.rb +++ b/activerecord/test/cases/explain_test.rb @@ -20,7 +20,7 @@ if ActiveRecord::Base.connection.supports_explain? end def test_collecting_queries_for_explain - result, queries = ActiveRecord::Base.collecting_queries_for_explain do + queries = ActiveRecord::Base.collecting_queries_for_explain do Car.where(:name => 'honda').to_a end @@ -28,7 +28,6 @@ if ActiveRecord::Base.connection.supports_explain? assert_match "SELECT", sql assert_match "honda", sql assert_equal [], binds - assert_equal [cars(:honda)], result end def test_exec_explain_with_no_binds diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 189066eb41..b91146db4e 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -179,6 +179,17 @@ class InheritanceTest < ActiveRecord::TestCase assert_raise(ActiveRecord::SubclassNotFound) { Company.new(:type => 'Account') } end + def test_new_with_autoload_paths + path = File.expand_path('../../models/autoloadable', __FILE__) + ActiveSupport::Dependencies.autoload_paths << path + + firm = Company.new(:type => 'ExtraFirm') + assert_equal ExtraFirm, firm.class + ensure + ActiveSupport::Dependencies.autoload_paths.reject! { |p| p == path } + ActiveSupport::Dependencies.clear + end + def test_inheritance_condition assert_equal 10, Company.count assert_equal 2, Firm.count diff --git a/activerecord/test/cases/migration/logger_test.rb b/activerecord/test/cases/migration/logger_test.rb index ee0c20747e..97efb94b66 100644 --- a/activerecord/test/cases/migration/logger_test.rb +++ b/activerecord/test/cases/migration/logger_test.rb @@ -7,6 +7,7 @@ module ActiveRecord self.use_transactional_fixtures = false Migration = Struct.new(:name, :version) do + def disable_ddl_transaction; false end def migrate direction # do nothing end @@ -34,4 +35,3 @@ module ActiveRecord end end end - diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index fa8dec0e15..f8afb7c591 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -239,9 +239,13 @@ class MigrationTest < ActiveRecord::TestCase assert_not Person.column_methods_hash.include?(:last_name) - migration = Struct.new(:name, :version) { - def migrate(x); raise 'Something broke'; end - }.new('zomg', 100) + migration = Class.new(ActiveRecord::Migration) { + def version; 100 end + def migrate(x) + add_column "people", "last_name", :string + raise 'Something broke' + end + }.new migrator = ActiveRecord::Migrator.new(:up, [migration], 100) @@ -250,7 +254,39 @@ class MigrationTest < ActiveRecord::TestCase assert_equal "An error has occurred, this and all later migrations canceled:\n\nSomething broke", e.message Person.reset_column_information + assert_not Person.column_methods_hash.include?(:last_name), + "On error, the Migrator should revert schema changes but it did not." + end + + def test_migration_without_transaction + unless ActiveRecord::Base.connection.supports_ddl_transactions? + skip "not supported on #{ActiveRecord::Base.connection.class}" + end + assert_not Person.column_methods_hash.include?(:last_name) + + migration = Class.new(ActiveRecord::Migration) { + self.disable_ddl_transaction! + + def version; 101 end + def migrate(x) + add_column "people", "last_name", :string + raise 'Something broke' + end + }.new + + migrator = ActiveRecord::Migrator.new(:up, [migration], 101) + e = assert_raise(StandardError) { migrator.migrate } + assert_equal "An error has occurred, all later migrations canceled:\n\nSomething broke", e.message + + Person.reset_column_information + assert Person.column_methods_hash.include?(:last_name), + "without ddl transactions, the Migrator should not rollback on error but it did." + ensure + Person.reset_column_information + if Person.column_methods_hash.include?(:last_name) + Person.connection.remove_column('people', 'last_name') + end end def test_schema_migrations_table_name @@ -426,6 +462,22 @@ class ReservedWordsMigrationTest < ActiveRecord::TestCase end end +class ExplicitlyNamedIndexMigrationTest < ActiveRecord::TestCase + def test_drop_index_by_name + connection = Person.connection + connection.create_table :values, force: true do |t| + t.integer :value + end + + assert_nothing_raised ArgumentError do + connection.add_index :values, :value, name: 'a_different_name' + connection.remove_index :values, column: :value, name: 'a_different_name' + end + + connection.drop_table :values rescue nil + end +end + if ActiveRecord::Base.connection.supports_bulk_alter? class BulkAlterTableMigrationsTest < ActiveRecord::TestCase def setup @@ -686,6 +738,26 @@ class CopyMigrationsTest < ActiveRecord::TestCase clear end + def test_copying_migrations_preserving_magic_comments + ActiveRecord::Base.timestamped_migrations = false + @migrations_path = MIGRATIONS_ROOT + "/valid" + @existing_migrations = Dir[@migrations_path + "/*.rb"] + + copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/magic"}) + assert File.exists?(@migrations_path + "/4_currencies_have_symbols.bukkits.rb") + assert_equal [@migrations_path + "/4_currencies_have_symbols.bukkits.rb"], copied.map(&:filename) + + expected = "# coding: ISO-8859-15\n# This migration comes from bukkits (originally 1)" + assert_equal expected, IO.readlines(@migrations_path + "/4_currencies_have_symbols.bukkits.rb")[0..1].join.chomp + + files_count = Dir[@migrations_path + "/*.rb"].length + copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/magic"}) + assert_equal files_count, Dir[@migrations_path + "/*.rb"].length + assert copied.empty? + ensure + clear + end + def test_skipping_migrations @migrations_path = MIGRATIONS_ROOT + "/valid_with_timestamps" @existing_migrations = Dir[@migrations_path + "/*.rb"] diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index bd121126e7..b593270352 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -309,7 +309,7 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal post.comments.size, Post.joins(join).joins(join).where("posts.id = #{post.id}").size end - def test_chaining_should_use_latest_conditions_when_creating + def test_chaining_applies_last_conditions_when_creating post = Topic.rejected.new assert !post.approved? @@ -323,13 +323,13 @@ class NamedScopeTest < ActiveRecord::TestCase assert post.approved? end - def test_chaining_should_use_latest_conditions_when_searching + def test_chaining_combines_conditions_when_searching # Normal hash conditions - assert_equal Topic.where(:approved => true).to_a, Topic.rejected.approved.to_a - assert_equal Topic.where(:approved => false).to_a, Topic.approved.rejected.to_a + assert_equal Topic.where(approved: false).where(approved: true).to_a, Topic.rejected.approved.to_a + assert_equal Topic.where(approved: true).where(approved: false).to_a, Topic.approved.rejected.to_a # Nested hash conditions with same keys - assert_equal [posts(:sti_comments)], Post.with_special_comments.with_very_special_comments.to_a + assert_equal [], Post.with_special_comments.with_very_special_comments.to_a # Nested hash conditions with different keys assert_equal [posts(:sti_comments)], Post.with_special_comments.with_post(4).to_a.uniq diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 8156f99037..b936cca875 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -399,14 +399,6 @@ class PersistencesTest < ActiveRecord::TestCase assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } end - def test_string_ids - # FIXME: Fix this failing test - skip "Failing test. We need this fixed before 4.0.0" - mv = Minivan.where(:minivan_id => 1234).first_or_initialize - assert mv.new_record? - assert_equal '1234', mv.minivan_id - end - def test_update_attribute_with_one_updated t = Topic.first t.update_attribute(:title, 'super_title') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 8298d7534c..26cbb03892 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -714,6 +714,13 @@ class RelationTest < ActiveRecord::TestCase assert_equal [developers(:poor_jamis)], dev_with_count.to_a end + def test_relation_to_sql + sql = Post.connection.unprepared_statement do + Post.first.comments.to_sql + end + assert_no_match(/\?/, sql) + end + def test_relation_merging_with_arel_equalities_keeps_last_equality devs = Developer.where(Developer.arel_table[:salary].eq(80000)).merge( Developer.where(Developer.arel_table[:salary].eq(9000)) diff --git a/activerecord/test/cases/serialized_attribute_test.rb b/activerecord/test/cases/serialized_attribute_test.rb index 295c7e13fa..726338db14 100644 --- a/activerecord/test/cases/serialized_attribute_test.rb +++ b/activerecord/test/cases/serialized_attribute_test.rb @@ -1,6 +1,7 @@ require 'cases/helper' require 'models/topic' require 'models/person' +require 'models/traffic_light' require 'bcrypt' class SerializedAttributeTest < ActiveRecord::TestCase @@ -234,4 +235,10 @@ class SerializedAttributeTest < ActiveRecord::TestCase person = person.reload assert_equal(insures, person.insures) end + + def test_regression_serialized_default_on_text_column_with_null_false + light = TrafficLight.new + assert_equal [], light.state + assert_equal [], light.long_state + end end diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index 38b9dd02f0..dadcca5b7f 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -249,10 +249,21 @@ module ActiveRecord def test_structure_dump filename = "awesome-file.sql" - Kernel.expects(:system).with("mysqldump", "--result-file", filename, "--no-data", "test-db") + Kernel.expects(:system).with("mysqldump", "--result-file", filename, "--no-data", "test-db").returns(true) ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, filename) end + + def test_warn_when_external_structure_dump_fails + filename = "awesome-file.sql" + Kernel.expects(:system).with("mysqldump", "--result-file", filename, "--no-data", "test-db").returns(false) + + warnings = capture(:stderr) do + ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, filename) + end + + assert_match(/Could not dump the database structure/, warnings) + end end class MySQLStructureLoadTest < ActiveRecord::TestCase diff --git a/activerecord/test/fixtures/traffic_lights.yml b/activerecord/test/fixtures/traffic_lights.yml index 6dabd53474..81b4e47959 100644 --- a/activerecord/test/fixtures/traffic_lights.yml +++ b/activerecord/test/fixtures/traffic_lights.yml @@ -4,3 +4,7 @@ uk: - Green - Red - Orange + long_state: + - "Green, go ahead" + - "Red, wait" + - "Orange, caution light is about to switch"
\ No newline at end of file diff --git a/activerecord/test/migrations/magic/1_currencies_have_symbols.rb b/activerecord/test/migrations/magic/1_currencies_have_symbols.rb new file mode 100644 index 0000000000..c066c068c2 --- /dev/null +++ b/activerecord/test/migrations/magic/1_currencies_have_symbols.rb @@ -0,0 +1,12 @@ +# coding: ISO-8859-15 + +class CurrenciesHaveSymbols < ActiveRecord::Migration + def self.up + # We use ¤ for default currency symbol + add_column "currencies", "symbol", :string, :default => "¤" + end + + def self.down + remove_column "currencies", "symbol" + end +end diff --git a/activerecord/test/models/autoloadable/extra_firm.rb b/activerecord/test/models/autoloadable/extra_firm.rb new file mode 100644 index 0000000000..5578ba0d9b --- /dev/null +++ b/activerecord/test/models/autoloadable/extra_firm.rb @@ -0,0 +1,2 @@ +class ExtraFirm < Company +end diff --git a/activerecord/test/models/traffic_light.rb b/activerecord/test/models/traffic_light.rb index 228f3f7bd4..a6b7edb882 100644 --- a/activerecord/test/models/traffic_light.rb +++ b/activerecord/test/models/traffic_light.rb @@ -1,3 +1,4 @@ class TrafficLight < ActiveRecord::Base serialize :state, Array + serialize :long_state, Array end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index cd9835259a..ae779c702a 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -219,6 +219,8 @@ ActiveRecord::Schema.define do t.integer :salary, :default => 70000 t.datetime :created_at t.datetime :updated_at + t.datetime :created_on + t.datetime :updated_on end create_table :developers_projects, :force => true, :id => false do |t| @@ -685,6 +687,7 @@ ActiveRecord::Schema.define do create_table :traffic_lights, :force => true do |t| t.string :location t.string :state + t.text :long_state, :null => false t.datetime :created_at t.datetime :updated_at end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index c47cb75274..9d26b8ba3e 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,11 +1,24 @@ ## Rails 4.0.0 (unreleased) ## * Fix deletion of empty directories in ActiveSupport::Cache::FileStore. - + *Charles Jones* ## Rails 4.0.0.beta1 (February 25, 2013) ## +* Improve singularizing a singular for multiple cases. + Fixes #2608 #1825 #2395. + + Example: + + # Before + 'address'.singularize # => 'addres' + + # After + 'address'.singularize # => 'address' + + *Mark McSpadden* + * Prevent `DateTime#change` from truncating the second fraction, when seconds do not need to be changed. diff --git a/guides/code/getting_started/app/controllers/comments_controller.rb b/guides/code/getting_started/app/controllers/comments_controller.rb index 0082e9c8ec..0e3d2a6dde 100644 --- a/guides/code/getting_started/app/controllers/comments_controller.rb +++ b/guides/code/getting_started/app/controllers/comments_controller.rb @@ -1,7 +1,7 @@ class CommentsController < ApplicationController -Â Â http_basic_authenticate_with name: "dhh", password: "secret", only: :destroy - + http_basic_authenticate_with name: "dhh", password: "secret", only: :destroy + def create @post = Post.find(params[:post_id]) @comment = @post.comments.create(params[:comment].permit(:commenter, :body)) diff --git a/guides/code/getting_started/app/controllers/posts_controller.rb b/guides/code/getting_started/app/controllers/posts_controller.rb index 0398395200..6aa1409170 100644 --- a/guides/code/getting_started/app/controllers/posts_controller.rb +++ b/guides/code/getting_started/app/controllers/posts_controller.rb @@ -1,7 +1,7 @@ class PostsController < ApplicationController -Â Â http_basic_authenticate_with name: "dhh", password: "secret", except: [:index, :show] - + http_basic_authenticate_with name: "dhh", password: "secret", except: [:index, :show] + def index @posts = Post.all end diff --git a/guides/source/migrations.md b/guides/source/migrations.md index 89ae564c24..bd63970bea 100644 --- a/guides/source/migrations.md +++ b/guides/source/migrations.md @@ -61,6 +61,10 @@ migrations are wrapped in a transaction. If the database does not support this then when a migration fails the parts of it that succeeded will not be rolled back. You will have to rollback the changes that were made by hand. +NOTE: There are certain queries that can't run inside a transaction. If your +adapter supports DDL transactions you can use `disable_ddl_transaction!` to +disable them for a single migration. + If you wish for a migration to do something that Active Record doesn't know how to reverse, you can use `reversible`: @@ -180,7 +184,7 @@ end ``` If the migration name is of the form "CreateXXX" and is -followed by a list of column names and types then a migration creating the table +followed by a list of column names and types then a migration creating the table XXX with the columns listed will be generated. For example: ```bash diff --git a/railties/Rakefile b/railties/Rakefile index 8576275aea..eb068fc526 100644 --- a/railties/Rakefile +++ b/railties/Rakefile @@ -6,6 +6,8 @@ require 'rbconfig' task :default => :test + +desc "Run all unit tests" task :test => 'test:isolated' namespace :test do diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 25cc36ff5d..2417bdca21 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -46,10 +46,10 @@ module Rails # 6) Run config.before_initialize callbacks # 7) Run Railtie#initializer defined by railties, engines and application. # One by one, each engine sets up its load paths, routes and runs its config/initializers/* files. - # 9) Custom Railtie#initializers added by railties, engines and applications are executed - # 10) Build the middleware stack and run to_prepare callbacks - # 11) Run config.before_eager_load and eager_load! if eager_load is true - # 12) Run config.after_initialize callbacks + # 8) Custom Railtie#initializers added by railties, engines and applications are executed + # 9) Build the middleware stack and run to_prepare callbacks + # 10) Run config.before_eager_load and eager_load! if eager_load is true + # 11) Run config.after_initialize callbacks # class Application < Engine autoload :Bootstrap, 'rails/application/bootstrap' diff --git a/railties/lib/rails/commands/console.rb b/railties/lib/rails/commands/console.rb index 86ab1aabbf..96229bb4f6 100644 --- a/railties/lib/rails/commands/console.rb +++ b/railties/lib/rails/commands/console.rb @@ -46,7 +46,10 @@ module Rails def initialize(app, options={}) @app = app @options = options + + app.sandbox = sandbox? app.load_console + @console = app.config.console || IRB end @@ -71,7 +74,6 @@ module Rails end def start - app.sandbox = sandbox? require_debugger if debugger? set_environment! if environment? diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt index 5669fe6d64..c40eef145f 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt @@ -24,10 +24,10 @@ <%- unless options.skip_sprockets? -%> # Compress JavaScripts and CSS. - config.assets.js_compressor = :uglifier + config.assets.js_compressor = :uglifier # config.assets.css_compressor = :sass - # Whether to fallback to assets pipeline if a precompiled asset is missed. + # Do not fallback to assets pipeline if a precompiled asset is missed. config.assets.compile = false # Generate digests for assets URLs. diff --git a/railties/test/application/console_test.rb b/railties/test/application/console_test.rb index 3cb3643e3a..f4450c8a3c 100644 --- a/railties/test/application/console_test.rb +++ b/railties/test/application/console_test.rb @@ -81,18 +81,82 @@ class ConsoleTest < ActiveSupport::TestCase assert_equal 'Once upon a time in a world...', helper.truncate('Once upon a time in a world far far away') end +end + +begin + require "pty" +rescue LoadError +end + +class FullStackConsoleTest < ActiveSupport::TestCase + def setup + skip "PTY unavailable" unless defined?(PTY) && PTY.respond_to?(:open) + + build_app + app_file 'app/models/post.rb', <<-CODE + class Post < ActiveRecord::Base + end + CODE + system "#{app_path}/bin/rails runner 'Post.connection.create_table :posts'" + + @master, @slave = PTY.open + end + + def teardown + teardown_app + end - def test_with_sandbox - require 'rails/all' - value = false + def assert_output(expected, timeout = 5) + timeout = Time.now + timeout - Class.new(Rails::Railtie) do - console do |app| - value = app.sandbox? + output = "" + until output.include?(expected) || Time.now > timeout + if IO.select([@master], [], [], 0.1) + output << @master.readpartial(100) end end - load_environment(true) - assert value + assert output.include?(expected), "#{expected.inspect} expected, but got:\n\n#{output}" + end + + def write_prompt(command, expected_output = nil) + @master.puts command + assert_output command + assert_output expected_output if expected_output + assert_output "> " + end + + def kill(pid) + Process.kill('QUIT', pid) + Process.wait(pid) + rescue Errno::ESRCH + end + + def spawn_console + pid = Process.spawn( + "#{app_path}/bin/rails console --sandbox", + in: @slave, out: @slave, err: @slave + ) + + assert_output "> ", 30 + pid + end + + def test_sandbox + pid = spawn_console + + write_prompt "Post.count", "=> 0" + write_prompt "Post.create" + write_prompt "Post.count", "=> 1" + + kill pid + + pid = spawn_console + + write_prompt "Post.count", "=> 0" + write_prompt "Post.transaction { Post.create; raise }" + write_prompt "Post.count", "=> 0" + ensure + kill pid if pid end end diff --git a/railties/test/commands/console_test.rb b/railties/test/commands/console_test.rb index 6be4a5fe89..3c784b43be 100644 --- a/railties/test/commands/console_test.rb +++ b/railties/test/commands/console_test.rb @@ -58,10 +58,7 @@ class Rails::ConsoleTest < ActiveSupport::TestCase end def test_console_defaults_to_IRB - config = mock("config", console: nil) - app = mock("app", config: config) - app.expects(:load_console).returns(nil) - + app = build_app(console: nil) assert_equal IRB, Rails::Console.new(app).console end @@ -127,13 +124,15 @@ class Rails::ConsoleTest < ActiveSupport::TestCase end def app - @app ||= begin - config = mock("config", console: FakeConsole) - app = mock("app", config: config) - app.stubs(:sandbox=).returns(nil) - app.expects(:load_console) - app - end + @app ||= build_app(console: FakeConsole) + end + + def build_app(config) + config = mock("config", config) + app = mock("app", config: config) + app.stubs(:sandbox=).returns(nil) + app.expects(:load_console) + app end def parse_arguments(args) |