From 097b2101897af447591d00fb2809d91894572b87 Mon Sep 17 00:00:00 2001 From: Stefan Kanev Date: Thu, 31 Jul 2014 12:05:07 +0200 Subject: [PATCH 01/25] Add an after_bundle callback in Rails templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The template runs before the generation of binstubs – this does not allow to write one, that makes an initial commit to version control. It is solvable by adding an after_bundle callback. --- guides/source/rails_application_templates.md | 24 ++++++++++++++++--- railties/CHANGELOG.md | 7 ++++++ railties/lib/rails/generators/actions.rb | 11 +++++++++ .../generators/rails/app/app_generator.rb | 6 +++++ .../test/generators/app_generator_test.rb | 15 ++++++++++++ 5 files changed, 60 insertions(+), 3 deletions(-) diff --git a/guides/source/rails_application_templates.md b/guides/source/rails_application_templates.md index 0bd608c007..7a3bd35a22 100644 --- a/guides/source/rails_application_templates.md +++ b/guides/source/rails_application_templates.md @@ -38,9 +38,11 @@ generate(:scaffold, "person name:string") route "root to: 'people#index'" rake("db:migrate") -git :init -git add: "." -git commit: %Q{ -m 'Initial commit' } +after_bundle do + git :init + git add: "." + git commit: %Q{ -m 'Initial commit' } +end ``` The following sections outline the primary methods provided by the API: @@ -228,6 +230,22 @@ git add: "." git commit: "-a -m 'Initial commit'" ``` +### after_bundle(&block) + +Registers a callback to be executed after bundler and binstub generation have +run. Useful for adding the binstubs to version control: + +```ruby +after_bundle do + git :init + git add: '.' + git commit: "-a -m 'Initial commit'" +end +``` + +The callbacks gets executed even if `--skip-bundle` and/or `--skip-spring` has +been passed. + Advanced Usage -------------- diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 651f40007e..9a57604f48 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,10 @@ +* Add `after_bundle` callbacks in Rails templates. Useful for allowing the + generated binstubs to be added to version control. + + Fixes #16292. + + *Stefan Kanev* + * Scaffold generator `_form` partial adds `class="field"` for password confirmation fields. diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index a239874df0..4709914947 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -7,6 +7,7 @@ module Rails def initialize(*) # :nodoc: super @in_group = nil + @after_bundle_callbacks = [] end # Adds an entry into +Gemfile+ for the supplied gem. @@ -232,6 +233,16 @@ module Rails log File.read(find_in_source_paths(path)) end + # Registers a callback to be executed after bundle and spring binstubs + # have run. + # + # after_bundle do + # git add: '.' + # end + def after_bundle(&block) + @after_bundle_callbacks << block + end + protected # Define log for backwards compatibility. If just one argument is sent, diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 188e62b6c8..9110c129d1 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -259,6 +259,12 @@ module Rails public_task :apply_rails_template, :run_bundle public_task :generate_spring_binstubs + def run_after_bundle_callbacks + @after_bundle_callbacks.each do |callback| + callback.call + end + end + protected def self.banner diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 184cfc2220..3f31f89473 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -501,6 +501,21 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_after_bundle_callback + path = 'http://example.org/rails_template' + template = %{ after_bundle { run 'echo ran after_bundle' } } + template.instance_eval "def read; self; end" # Make the string respond to read + + generator([destination_root], template: path).expects(:open).with(path, 'Accept' => 'application/x-thor-template').returns(template) + + bundler_first = sequence('bundle, binstubs, after_bundle') + generator.expects(:bundle_command).with('install').once.in_sequence(bundler_first) + generator.expects(:bundle_command).with('exec spring binstub --all').in_sequence(bundler_first) + generator.expects(:run).with('echo ran after_bundle').in_sequence(bundler_first) + + quietly { generator.invoke_all } + end + protected def action(*args, &block) From 7a0c2ba48b0145e5031e51d316407ba28d769905 Mon Sep 17 00:00:00 2001 From: Bogdan Gusiev Date: Tue, 5 Aug 2014 17:23:00 +0300 Subject: [PATCH 02/25] Fixed #select form builder helper to support block with html output --- actionview/lib/action_view/helpers/tags/select.rb | 2 +- .../test/template/form_options_helper_test.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/actionview/lib/action_view/helpers/tags/select.rb b/actionview/lib/action_view/helpers/tags/select.rb index 00881d9978..180900cc8d 100644 --- a/actionview/lib/action_view/helpers/tags/select.rb +++ b/actionview/lib/action_view/helpers/tags/select.rb @@ -3,7 +3,7 @@ module ActionView module Tags # :nodoc: class Select < Base # :nodoc: def initialize(object_name, method_name, template_object, choices, options, html_options) - @choices = block_given? ? template_object.capture { yield } : choices + @choices = block_given? ? template_object.capture { yield || "" } : choices @choices = @choices.to_a if @choices.is_a?(Range) @html_options = html_options diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index fbafb7aa08..d25fa3706f 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -591,6 +591,19 @@ class FormOptionsHelperTest < ActionView::TestCase ) end + def test_select_under_fields_for_with_block_without_options + @post = Post.new + + output_buffer = fields_for :post, @post do |f| + concat(f.select(:category) {}) + end + + assert_dom_equal( + "", + output_buffer + ) + end + def test_select_with_multiple_to_add_hidden_input output_buffer = select(:post, :category, "", {}, :multiple => true) assert_dom_equal( From c294e91d00696e910e05ad1428ba3ce4884bc6a3 Mon Sep 17 00:00:00 2001 From: Stefan Kanev Date: Tue, 5 Aug 2014 19:38:20 +0300 Subject: [PATCH 03/25] Add after_bundle to the release notes and upgrade guide --- guides/source/4_2_release_notes.md | 3 +++ guides/source/upgrading_ruby_on_rails.md | 32 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/guides/source/4_2_release_notes.md b/guides/source/4_2_release_notes.md index 12db528b91..a39dd9ace0 100644 --- a/guides/source/4_2_release_notes.md +++ b/guides/source/4_2_release_notes.md @@ -85,6 +85,9 @@ Please refer to the [Changelog][railties] for detailed changes. * Introduced `Rails.gem_version` as a convenience method to return `Gem::Version.new(Rails.version)`. ([Pull Request](https://github.com/rails/rails/pull/14101)) +* Introduced an `after_bundle` callback in the Rails templates. + ([Pull Request](https://github.com/rails/rails/pull/16359)) + Action Pack ----------- diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index b3e4505fc0..62e4071935 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -58,6 +58,38 @@ When assigning `nil` to a serialized attribute, it will be saved to the database as `NULL` instead of passing the `nil` value through the coder (e.g. `"null"` when using the `JSON` coder). +### `after_bundle` in Rails templates + +If you have a Rails template that adds all the files in version control, it +fails to add the generated binstubs because it gets executed before Bundler: + +```ruby +# template.rb +generate(:scaffold, "person name:string") +route "root to: 'people#index'" +rake("db:migrate") + +git :init +git add: "." +git commit: %Q{ -m 'Initial commit' } +``` + +You can now wrap the `git` calls in an `after_bundle` block. It will be run +after the binstubs have been generated. + +```ruby +# template.rb +generate(:scaffold, "person name:string") +route "root to: 'people#index'" +rake("db:migrate") + +after_bundle do + git :init + git add: "." + git commit: %Q{ -m 'Initial commit' } +end +``` + Upgrading from Rails 4.0 to Rails 4.1 ------------------------------------- From 70673759a036746987eda58e8b77cf723938d82c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 5 Aug 2014 16:28:43 -0700 Subject: [PATCH 04/25] no reason to lazily instantiate the routes especially if you're just going to add a call two lines down that populates the cache. common. --- .../test/dispatch/prefix_generation_test.rb | 83 +++++++++---------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index 7cf3559e5d..eda92beae1 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -25,34 +25,33 @@ module TestGenerationPrefix include Rack::Test::Methods class BlogEngine < Rails::Engine - def self.routes - @routes ||= begin - routes = ActionDispatch::Routing::RouteSet.new - routes.draw do - get "/posts/:id", :to => "inside_engine_generating#show", :as => :post - get "/posts", :to => "inside_engine_generating#index", :as => :posts - get "/url_to_application", :to => "inside_engine_generating#url_to_application" - get "/polymorphic_path_for_engine", :to => "inside_engine_generating#polymorphic_path_for_engine" - get "/conflicting_url", :to => "inside_engine_generating#conflicting" - get "/foo", :to => "never#invoked", :as => :named_helper_that_should_be_invoked_only_in_respond_to_test + @routes = ActionDispatch::Routing::RouteSet.new.tap { |routes| + routes.draw do + get "/posts/:id", :to => "inside_engine_generating#show", :as => :post + get "/posts", :to => "inside_engine_generating#index", :as => :posts + get "/url_to_application", :to => "inside_engine_generating#url_to_application" + get "/polymorphic_path_for_engine", :to => "inside_engine_generating#polymorphic_path_for_engine" + get "/conflicting_url", :to => "inside_engine_generating#conflicting" + get "/foo", :to => "never#invoked", :as => :named_helper_that_should_be_invoked_only_in_respond_to_test - get "/relative_path_root", :to => redirect("") - get "/relative_path_redirect", :to => redirect("foo") - get "/relative_option_root", :to => redirect(:path => "") - get "/relative_option_redirect", :to => redirect(:path => "foo") - get "/relative_custom_root", :to => redirect { |params, request| "" } - get "/relative_custom_redirect", :to => redirect { |params, request| "foo" } + get "/relative_path_root", :to => redirect("") + get "/relative_path_redirect", :to => redirect("foo") + get "/relative_option_root", :to => redirect(:path => "") + get "/relative_option_redirect", :to => redirect(:path => "foo") + get "/relative_custom_root", :to => redirect { |params, request| "" } + get "/relative_custom_redirect", :to => redirect { |params, request| "foo" } - get "/absolute_path_root", :to => redirect("/") - get "/absolute_path_redirect", :to => redirect("/foo") - get "/absolute_option_root", :to => redirect(:path => "/") - get "/absolute_option_redirect", :to => redirect(:path => "/foo") - get "/absolute_custom_root", :to => redirect { |params, request| "/" } - get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" } - end - - routes + get "/absolute_path_root", :to => redirect("/") + get "/absolute_path_redirect", :to => redirect("/foo") + get "/absolute_option_root", :to => redirect(:path => "/") + get "/absolute_option_redirect", :to => redirect(:path => "/foo") + get "/absolute_custom_root", :to => redirect { |params, request| "/" } + get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" } end + } + + def self.routes + @routes end def self.call(env) @@ -62,25 +61,24 @@ module TestGenerationPrefix end class RailsApplication - def self.routes - @routes ||= begin - routes = ActionDispatch::Routing::RouteSet.new - routes.draw do - scope "/:omg", :omg => "awesome" do - mount BlogEngine => "/blog", :as => "blog_engine" - end - get "/posts/:id", :to => "outside_engine_generating#post", :as => :post - get "/generate", :to => "outside_engine_generating#index" - get "/polymorphic_path_for_app", :to => "outside_engine_generating#polymorphic_path_for_app" - get "/polymorphic_path_for_engine", :to => "outside_engine_generating#polymorphic_path_for_engine" - get "/polymorphic_with_url_for", :to => "outside_engine_generating#polymorphic_with_url_for" - get "/conflicting_url", :to => "outside_engine_generating#conflicting" - get "/ivar_usage", :to => "outside_engine_generating#ivar_usage" - root :to => "outside_engine_generating#index" + @routes = ActionDispatch::Routing::RouteSet.new.tap { |routes| + routes.draw do + scope "/:omg", :omg => "awesome" do + mount BlogEngine => "/blog", :as => "blog_engine" end - - routes + get "/posts/:id", :to => "outside_engine_generating#post", :as => :post + get "/generate", :to => "outside_engine_generating#index" + get "/polymorphic_path_for_app", :to => "outside_engine_generating#polymorphic_path_for_app" + get "/polymorphic_path_for_engine", :to => "outside_engine_generating#polymorphic_path_for_engine" + get "/polymorphic_with_url_for", :to => "outside_engine_generating#polymorphic_with_url_for" + get "/conflicting_url", :to => "outside_engine_generating#conflicting" + get "/ivar_usage", :to => "outside_engine_generating#ivar_usage" + root :to => "outside_engine_generating#index" end + } + + def self.routes + @routes end def self.call(env) @@ -90,7 +88,6 @@ module TestGenerationPrefix end # force draw - RailsApplication.routes RailsApplication.routes.define_mounted_helper(:main_app) class ::InsideEngineGeneratingController < ActionController::Base From 9a36fac810246dad32fb4b88619dda805315d5e7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 5 Aug 2014 16:30:08 -0700 Subject: [PATCH 05/25] a rails application should be an engine subclass --- actionpack/test/dispatch/prefix_generation_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index eda92beae1..060d76748a 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -60,7 +60,7 @@ module TestGenerationPrefix end end - class RailsApplication + class RailsApplication < Rails::Engine @routes = ActionDispatch::Routing::RouteSet.new.tap { |routes| routes.draw do scope "/:omg", :omg => "awesome" do From acb371ca3fbbe3123d8d5b411ae2f2fb0b51ed7b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 5 Aug 2014 16:50:50 -0700 Subject: [PATCH 06/25] call the routes method on engines if we access the instance, we can free up lots of codes --- .../test/dispatch/prefix_generation_test.rb | 107 ++++++------------ 1 file changed, 35 insertions(+), 72 deletions(-) diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index 060d76748a..f90d5499d7 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -25,65 +25,43 @@ module TestGenerationPrefix include Rack::Test::Methods class BlogEngine < Rails::Engine - @routes = ActionDispatch::Routing::RouteSet.new.tap { |routes| - routes.draw do - get "/posts/:id", :to => "inside_engine_generating#show", :as => :post - get "/posts", :to => "inside_engine_generating#index", :as => :posts - get "/url_to_application", :to => "inside_engine_generating#url_to_application" - get "/polymorphic_path_for_engine", :to => "inside_engine_generating#polymorphic_path_for_engine" - get "/conflicting_url", :to => "inside_engine_generating#conflicting" - get "/foo", :to => "never#invoked", :as => :named_helper_that_should_be_invoked_only_in_respond_to_test + routes.draw do + get "/posts/:id", :to => "inside_engine_generating#show", :as => :post + get "/posts", :to => "inside_engine_generating#index", :as => :posts + get "/url_to_application", :to => "inside_engine_generating#url_to_application" + get "/polymorphic_path_for_engine", :to => "inside_engine_generating#polymorphic_path_for_engine" + get "/conflicting_url", :to => "inside_engine_generating#conflicting" + get "/foo", :to => "never#invoked", :as => :named_helper_that_should_be_invoked_only_in_respond_to_test - get "/relative_path_root", :to => redirect("") - get "/relative_path_redirect", :to => redirect("foo") - get "/relative_option_root", :to => redirect(:path => "") - get "/relative_option_redirect", :to => redirect(:path => "foo") - get "/relative_custom_root", :to => redirect { |params, request| "" } - get "/relative_custom_redirect", :to => redirect { |params, request| "foo" } + get "/relative_path_root", :to => redirect("") + get "/relative_path_redirect", :to => redirect("foo") + get "/relative_option_root", :to => redirect(:path => "") + get "/relative_option_redirect", :to => redirect(:path => "foo") + get "/relative_custom_root", :to => redirect { |params, request| "" } + get "/relative_custom_redirect", :to => redirect { |params, request| "foo" } - get "/absolute_path_root", :to => redirect("/") - get "/absolute_path_redirect", :to => redirect("/foo") - get "/absolute_option_root", :to => redirect(:path => "/") - get "/absolute_option_redirect", :to => redirect(:path => "/foo") - get "/absolute_custom_root", :to => redirect { |params, request| "/" } - get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" } - end - } - - def self.routes - @routes - end - - def self.call(env) - env['action_dispatch.routes'] = routes - routes.call(env) + get "/absolute_path_root", :to => redirect("/") + get "/absolute_path_redirect", :to => redirect("/foo") + get "/absolute_option_root", :to => redirect(:path => "/") + get "/absolute_option_redirect", :to => redirect(:path => "/foo") + get "/absolute_custom_root", :to => redirect { |params, request| "/" } + get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" } end end class RailsApplication < Rails::Engine - @routes = ActionDispatch::Routing::RouteSet.new.tap { |routes| - routes.draw do - scope "/:omg", :omg => "awesome" do - mount BlogEngine => "/blog", :as => "blog_engine" - end - get "/posts/:id", :to => "outside_engine_generating#post", :as => :post - get "/generate", :to => "outside_engine_generating#index" - get "/polymorphic_path_for_app", :to => "outside_engine_generating#polymorphic_path_for_app" - get "/polymorphic_path_for_engine", :to => "outside_engine_generating#polymorphic_path_for_engine" - get "/polymorphic_with_url_for", :to => "outside_engine_generating#polymorphic_with_url_for" - get "/conflicting_url", :to => "outside_engine_generating#conflicting" - get "/ivar_usage", :to => "outside_engine_generating#ivar_usage" - root :to => "outside_engine_generating#index" + routes.draw do + scope "/:omg", :omg => "awesome" do + mount BlogEngine => "/blog", :as => "blog_engine" end - } - - def self.routes - @routes - end - - def self.call(env) - env['action_dispatch.routes'] = routes - routes.call(env) + get "/posts/:id", :to => "outside_engine_generating#post", :as => :post + get "/generate", :to => "outside_engine_generating#index" + get "/polymorphic_path_for_app", :to => "outside_engine_generating#polymorphic_path_for_app" + get "/polymorphic_path_for_engine", :to => "outside_engine_generating#polymorphic_path_for_engine" + get "/polymorphic_with_url_for", :to => "outside_engine_generating#polymorphic_with_url_for" + get "/conflicting_url", :to => "outside_engine_generating#conflicting" + get "/ivar_usage", :to => "outside_engine_generating#ivar_usage" + root :to => "outside_engine_generating#index" end end @@ -159,7 +137,7 @@ module TestGenerationPrefix end def app - RailsApplication + RailsApplication.instance end attr_reader :engine_object, :app_object @@ -389,27 +367,12 @@ module TestGenerationPrefix end end - class RailsApplication - def self.routes - @routes ||= begin - routes = ActionDispatch::Routing::RouteSet.new - routes.draw do - mount BlogEngine => "/" - end - - routes - end - end - - def self.call(env) - env['action_dispatch.routes'] = routes - routes.call(env) + class RailsApplication < Rails::Engine + routes.draw do + mount BlogEngine => "/" end end - # force draw - RailsApplication.routes - class ::PostsController < ActionController::Base include BlogEngine.routes.url_helpers include RailsApplication.routes.mounted_helpers @@ -420,7 +383,7 @@ module TestGenerationPrefix end def app - RailsApplication + RailsApplication.instance end test "generating path inside engine" do From 53dba73dc17502b3d3f71d4672e86a14b995a732 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Thu, 24 Jul 2014 14:46:25 +0200 Subject: [PATCH 07/25] Revert "Revert "Merge pull request #15394 from morgoth/fix-automatic-maintaining-test-schema-for-sql-format"" This reverts commit 5c87b5c5248154cf8aa76cce9a24a88769de022d. --- activerecord/CHANGELOG.md | 9 +- .../lib/active_record/tasks/database_tasks.rb | 2 + .../tasks/sqlite_database_tasks.rb | 6 +- railties/test/application/test_test.rb | 91 ++++++++++++++++++- 4 files changed, 105 insertions(+), 3 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 9043542fda..103f657306 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -297,7 +297,14 @@ Fixes #8328. - *Sean Griffin* + Sean Griffin + +* Fixed automatic maintaining test schema to properly handle sql structure + schema format. + + Fixes #15394. + + *Wojciech Wnętrzak* * Pluck now works when selecting columns from different tables with the same name. diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 72e0cf3723..659110c066 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -188,10 +188,12 @@ module ActiveRecord when :ruby file ||= File.join(db_dir, "schema.rb") check_schema_file(file) + purge(current_config) load(file) when :sql file ||= File.join(db_dir, "structure.sql") check_schema_file(file) + purge(current_config) structure_load(current_config, file) else raise ArgumentError, "unknown format #{format.inspect}" diff --git a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb index 5688931db2..9ab64d0325 100644 --- a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb @@ -21,7 +21,11 @@ module ActiveRecord FileUtils.rm(file) if File.exist?(file) end - alias :purge :drop + + def purge + drop + create + end def charset connection.encoding diff --git a/railties/test/application/test_test.rb b/railties/test/application/test_test.rb index a223180169..c724c867ec 100644 --- a/railties/test/application/test_test.rb +++ b/railties/test/application/test_test.rb @@ -67,7 +67,7 @@ module ApplicationTests assert_match %r{/app/test/unit/failing_test\.rb}, output end - test "migrations" do + test "ruby schema migrations" do output = script('generate model user name:string') version = output.match(/(\d+)_create_users\.rb/)[1] @@ -104,6 +104,95 @@ module ApplicationTests assert !result.include?("create_table(:users)") end + test "sql structure migrations" do + output = script('generate model user name:string') + version = output.match(/(\d+)_create_users\.rb/)[1] + + app_file 'test/models/user_test.rb', <<-RUBY + require 'test_helper' + + class UserTest < ActiveSupport::TestCase + test "user" do + User.create! name: "Jon" + end + end + RUBY + + app_file 'db/structure.sql', '' + app_file 'config/initializers/enable_sql_schema_format.rb', <<-RUBY + Rails.application.config.active_record.schema_format = :sql + RUBY + + assert_unsuccessful_run "models/user_test.rb", "Migrations are pending" + + app_file 'db/structure.sql', <<-SQL + CREATE TABLE "schema_migrations" ("version" varchar(255) NOT NULL); + CREATE UNIQUE INDEX "unique_schema_migrations" ON "schema_migrations" ("version"); + CREATE TABLE "users" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar(255)); + INSERT INTO schema_migrations (version) VALUES ('#{version}'); + SQL + + app_file 'config/initializers/disable_maintain_test_schema.rb', <<-RUBY + Rails.application.config.active_record.maintain_test_schema = false + RUBY + + assert_unsuccessful_run "models/user_test.rb", "Could not find table 'users'" + + File.delete "#{app_path}/config/initializers/disable_maintain_test_schema.rb" + + assert_successful_test_run('models/user_test.rb') + end + + test "sql structure migrations when adding column to existing table" do + output_1 = script('generate model user name:string') + version_1 = output_1.match(/(\d+)_create_users\.rb/)[1] + + app_file 'test/models/user_test.rb', <<-RUBY + require 'test_helper' + class UserTest < ActiveSupport::TestCase + test "user" do + User.create! name: "Jon" + end + end + RUBY + + app_file 'config/initializers/enable_sql_schema_format.rb', <<-RUBY + Rails.application.config.active_record.schema_format = :sql + RUBY + + app_file 'db/structure.sql', <<-SQL + CREATE TABLE "schema_migrations" ("version" varchar(255) NOT NULL); + CREATE UNIQUE INDEX "unique_schema_migrations" ON "schema_migrations" ("version"); + CREATE TABLE "users" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar(255)); + INSERT INTO schema_migrations (version) VALUES ('#{version_1}'); + SQL + + assert_successful_test_run('models/user_test.rb') + + output_2 = script('generate migration add_email_to_users') + version_2 = output_2.match(/(\d+)_add_email_to_users\.rb/)[1] + + app_file 'test/models/user_test.rb', <<-RUBY + require 'test_helper' + + class UserTest < ActiveSupport::TestCase + test "user" do + User.create! name: "Jon", email: "jon@doe.com" + end + end + RUBY + + app_file 'db/structure.sql', <<-SQL + CREATE TABLE "schema_migrations" ("version" varchar(255) NOT NULL); + CREATE UNIQUE INDEX "unique_schema_migrations" ON "schema_migrations" ("version"); + CREATE TABLE "users" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar(255), "email" varchar(255)); + INSERT INTO schema_migrations (version) VALUES ('#{version_1}'); + INSERT INTO schema_migrations (version) VALUES ('#{version_2}'); + SQL + + assert_successful_test_run('models/user_test.rb') + end + private def assert_unsuccessful_run(name, message) result = run_test_file(name) From f15cef67f75e4b52fd45655d7c6ab6b35623c608 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Fri, 25 Jul 2014 17:49:27 +0200 Subject: [PATCH 08/25] schema rake tasks are specific about the configuration to act on. The rake tasks and the `DatabaseTakss` adapter classes used to assume a configuration at some places. This forced the rake tasks to establish a specific connection before calling into `load_schema`. After #15394 this started to cause issues because it could `purge` the wrong database before loading the schema. --- activerecord/CHANGELOG.md | 22 +++++++++++------- activerecord/lib/active_record/migration.rb | 2 +- .../lib/active_record/railties/databases.rake | 14 ++++------- .../lib/active_record/tasks/database_tasks.rb | 23 ++++++++++++++++--- 4 files changed, 39 insertions(+), 22 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 103f657306..c7e8b264fb 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,16 @@ +* Deprecate `DatabaseTasks.load_schema` to act on the current connection. + Use `.load_schema_current` instead. In the future `load_schema` will + require the `configuration` to act on as an argument. + + *Yves Senn* + +* Fixed automatic maintaining test schema to properly handle sql structure + schema format. + + Fixes #15394. + + *Wojciech Wnętrzak* + * Fix type casting to Decimal from Float with large precision. *Tomohiro Hashidate* @@ -297,14 +310,7 @@ Fixes #8328. - Sean Griffin - -* Fixed automatic maintaining test schema to properly handle sql structure - schema format. - - Fixes #15394. - - *Wojciech Wnętrzak* + *Sean Griffin* * Pluck now works when selecting columns from different tables with the same name. diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 7c4dad21a0..a6847e28c2 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -399,7 +399,7 @@ module ActiveRecord def load_schema_if_pending! if ActiveRecord::Migrator.needs_migration? - ActiveRecord::Tasks::DatabaseTasks.load_schema + ActiveRecord::Tasks::DatabaseTasks.load_schema_current check_pending! end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index ac385817e4..458862a538 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -240,7 +240,7 @@ db_namespace = namespace :db do desc 'Load a schema.rb file into the database' task :load => [:environment, :load_config] do - ActiveRecord::Tasks::DatabaseTasks.load_schema(:ruby, ENV['SCHEMA']) + ActiveRecord::Tasks::DatabaseTasks.load_schema_current(:ruby, ENV['SCHEMA']) end task :load_if_ruby => ['db:create', :environment] do @@ -286,7 +286,7 @@ db_namespace = namespace :db do desc "Recreate the databases from the structure.sql file" task :load => [:environment, :load_config] do - ActiveRecord::Tasks::DatabaseTasks.load_schema(:sql, ENV['DB_STRUCTURE']) + ActiveRecord::Tasks::DatabaseTasks.load_schema_current(:sql, ENV['DB_STRUCTURE']) end task :load_if_sql => ['db:create', :environment] do @@ -317,9 +317,8 @@ db_namespace = namespace :db do task :load_schema => %w(db:test:deprecated db:test:purge) do begin should_reconnect = ActiveRecord::Base.connection_pool.active_connection? - ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations['test']) ActiveRecord::Schema.verbose = false - db_namespace["schema:load"].invoke + ActiveRecord::Tasks::DatabaseTasks.load_schema_for ActiveRecord::Base.configurations['test'], :ruby, ENV['SCHEMA'] ensure if should_reconnect ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations[ActiveRecord::Tasks::DatabaseTasks.env]) @@ -329,12 +328,7 @@ db_namespace = namespace :db do # desc "Recreate the test database from an existent structure.sql file" task :load_structure => %w(db:test:deprecated db:test:purge) do - begin - ActiveRecord::Tasks::DatabaseTasks.current_config(:config => ActiveRecord::Base.configurations['test']) - db_namespace["structure:load"].invoke - ensure - ActiveRecord::Tasks::DatabaseTasks.current_config(:config => nil) - end + ActiveRecord::Tasks::DatabaseTasks.load_schema_for ActiveRecord::Base.configurations['test'], :sql, ENV['SCHEMA'] end # desc "Recreate the test database from a fresh schema" diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 659110c066..892c78e479 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -184,22 +184,39 @@ module ActiveRecord end def load_schema(format = ActiveRecord::Base.schema_format, file = nil) + ActiveSupport::Deprecation.warn(<<-MESSAGE.strip_heredoc) + This method will act on a specific connection in the future. + To act on the current connection, use `load_schema_current` instead. + MESSAGE + load_schema_current(format, file) + end + + # This method is the successor of +load_schema+. We should rename it + # after +load_schema+ went through a deprecation cycle. (Rails > 4.2) + def load_schema_for(configuration, format = ActiveRecord::Base.schema_format, file = nil) # :nodoc: case format when :ruby file ||= File.join(db_dir, "schema.rb") check_schema_file(file) - purge(current_config) + purge(configuration) + ActiveRecord::Base.establish_connection(configuration) load(file) when :sql file ||= File.join(db_dir, "structure.sql") check_schema_file(file) - purge(current_config) - structure_load(current_config, file) + purge(configuration) + structure_load(configuration, file) else raise ArgumentError, "unknown format #{format.inspect}" end end + def load_schema_current(format = ActiveRecord::Base.schema_format, file = nil, environment = env) + each_current_configuration(environment) { |configuration| + load_schema_for configuration, format, file + } + end + def check_schema_file(filename) unless File.exist?(filename) message = %{#{filename} doesn't exist yet. Run `rake db:migrate` to create it, then try again.} From 3dfcae6afa24b641bd838b9044c5ce9aa2a1a6db Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Thu, 7 Aug 2014 01:57:55 +0900 Subject: [PATCH 09/25] defined? should actually work in current implementation So this trick is not needed to be documented anymore. --- actionview/lib/action_view/base.rb | 9 --------- 1 file changed, 9 deletions(-) diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index 900f96255e..86c55ffb51 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -66,15 +66,6 @@ module ActionView #:nodoc: # Headline: <%= headline %> # First name: <%= person.first_name %> # - # If you need to find out whether a certain local variable has been assigned a value in a particular render call, - # you need to use the following pattern: - # - # <% if local_assigns.has_key? :headline %> - # Headline: <%= headline %> - # <% end %> - # - # Testing using defined? headline will not work. This is an implementation restriction. - # # === Template caching # # By default, Rails will compile each template to a method in order to render it. When you alter a template, From f8dcb365dfb8506c60297a4434f70f41b5259250 Mon Sep 17 00:00:00 2001 From: Yevhene Shemet Date: Wed, 6 Aug 2014 17:41:36 +0300 Subject: [PATCH 10/25] Allow password to contain spaces only. --- activemodel/CHANGELOG.md | 6 ++++++ activemodel/lib/active_model/secure_password.rb | 4 ++-- activemodel/test/cases/secure_password_test.rb | 10 ++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 8d22e3ac46..b1a3d660d6 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,9 @@ +* Passwords with spaces only allowed in `ActiveModel::SecurePassword`. + + Presence validation can be used to resore old behavior. + + *Yevhene Shemet* + * Validate options passed to `ActiveModel::Validations.validate`. Preventing, in many cases, the simple mistake of using `validate` instead of `validates`. diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 7e179cf4b7..f6ad35769f 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -105,7 +105,7 @@ module ActiveModel attr_reader :password # Encrypts the password into the +password_digest+ attribute, only if the - # new password is not blank. + # new password is not empty. # # class User < ActiveRecord::Base # has_secure_password validations: false @@ -119,7 +119,7 @@ module ActiveModel def password=(unencrypted_password) if unencrypted_password.nil? self.password_digest = nil - elsif unencrypted_password.present? + elsif !unencrypted_password.empty? @password = unencrypted_password cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine.cost self.password_digest = BCrypt::Password.create(unencrypted_password, cost: cost) diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index 6b21bc68fa..6d56c8344a 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -40,6 +40,11 @@ class SecurePasswordTest < ActiveModel::TestCase assert @user.valid?(:create), 'user should be valid' end + test "create a new user with validation and a spaces only password" do + @user.password = ' ' * 72 + assert @user.valid?(:create), 'user should be valid' + end + test "create a new user with validation and a blank password" do @user.password = '' assert !@user.valid?(:create), 'user should be invalid' @@ -105,6 +110,11 @@ class SecurePasswordTest < ActiveModel::TestCase assert @existing_user.valid?(:update), 'user should be valid' end + test "updating an existing user with validation and a spaces only password" do + @user.password = ' ' * 72 + assert @user.valid?(:update), 'user should be valid' + end + test "updating an existing user with validation and a blank password and password_confirmation" do @existing_user.password = '' @existing_user.password_confirmation = '' From 2090615d39c071c9eb25e715275eb79f3f9b6266 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 6 Aug 2014 14:17:57 -0700 Subject: [PATCH 11/25] refactor Redirecting so we do not need a controller instance --- actionpack/lib/action_controller/metal/redirecting.rb | 8 +++++--- .../lib/action_dispatch/testing/assertions/response.rb | 9 ++------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index ca8c0278d0..acaa8227c9 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -72,11 +72,11 @@ module ActionController raise AbstractController::DoubleRenderError if response_body self.status = _extract_redirect_to_status(options, response_status) - self.location = _compute_redirect_to_location(options) + self.location = _compute_redirect_to_location(request, options) self.response_body = "You are being redirected." end - def _compute_redirect_to_location(options) #:nodoc: + def _compute_redirect_to_location(request, options) #:nodoc: case options # The scheme name consist of a letter followed by any combination of # letters, digits, and the plus ("+"), period ("."), or hyphen ("-") @@ -90,11 +90,13 @@ module ActionController when :back request.headers["Referer"] or raise RedirectBackError when Proc - _compute_redirect_to_location options.call + _compute_redirect_to_location request, options.call else url_for(options) end.delete("\0\r\n") end + module_function :_compute_redirect_to_location + public :_compute_redirect_to_location private def _extract_redirect_to_status(options, response_status) diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 0adc6c84ff..13a72220b3 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -73,13 +73,8 @@ module ActionDispatch if Regexp === fragment fragment else - handle = @controller || Class.new(ActionController::Metal) do - include ActionController::Redirecting - def initialize(request) - @_request = request - end - end.new(@request) - handle._compute_redirect_to_location(fragment) + handle = @controller || ActionController::Redirecting + handle._compute_redirect_to_location(@request, fragment) end end end From d25fe31c40928712b5e08fe0afb567c3bc88eddf Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 6 Aug 2014 18:27:16 -0700 Subject: [PATCH 12/25] lazily instantiate application subclasses this means we can meaningfully override methods in the subclass --- railties/lib/rails.rb | 8 +++++++- railties/lib/rails/application.rb | 4 +--- railties/test/engine_test.rb | 10 ++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/railties/lib/rails.rb b/railties/lib/rails.rb index ecd8c22dd8..e7172e491f 100644 --- a/railties/lib/rails.rb +++ b/railties/lib/rails.rb @@ -29,7 +29,13 @@ module Rails autoload :WelcomeController class << self - attr_accessor :application, :cache, :logger + @application = @app_class = nil + + attr_writer :application + attr_accessor :app_class, :cache, :logger + def application + @application ||= (app_class.instance if app_class) + end delegate :initialize!, :initialized?, to: :application diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index c5fd08e743..292986b475 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -87,7 +87,7 @@ module Rails class << self def inherited(base) super - base.instance + Rails.app_class = base end # Makes the +new+ method public. @@ -117,8 +117,6 @@ module Rails @railties = nil @message_verifiers = {} - Rails.application ||= self - add_lib_to_load_path! ActiveSupport.run_load_hooks(:before_configuration, self) diff --git a/railties/test/engine_test.rb b/railties/test/engine_test.rb index 7970913d21..8401494bd2 100644 --- a/railties/test/engine_test.rb +++ b/railties/test/engine_test.rb @@ -11,4 +11,14 @@ class EngineTest < ActiveSupport::TestCase assert !engine.routes? end + + def test_application_can_be_subclassed + klass = Class.new(Rails::Application) do + attr_reader :hello + def initialize + @hello = "world" + end + end + assert_equal "world", klass.instance.hello + end end From ca9736e78ca9348e785a5c78c8cc085c0c2d4731 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 14 Jan 2013 03:32:34 -0800 Subject: [PATCH 13/25] Fix DateTime comparison with DateTime::Infinity object Fixes #16406 --- activesupport/CHANGELOG.md | 4 ++++ .../lib/active_support/core_ext/date_time/calculations.rb | 4 +++- activesupport/test/core_ext/range_ext_test.rb | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 7514faabb2..03e8d73ea7 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix DateTime comparison with DateTime::Infinity object. + + *Rafael Mendonça França* + * Added Object#itself which returns the object itself. Useful when dealing with a chaining scenario, like Active Record scopes: Event.public_send(state.presence_in([ :trashed, :drafted ]) || :itself).order(:created_at) diff --git a/activesupport/lib/active_support/core_ext/date_time/calculations.rb b/activesupport/lib/active_support/core_ext/date_time/calculations.rb index 289ca12b5e..dc4e767e9d 100644 --- a/activesupport/lib/active_support/core_ext/date_time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/date_time/calculations.rb @@ -161,7 +161,9 @@ class DateTime # Layers additional behavior on DateTime#<=> so that Time and # ActiveSupport::TimeWithZone instances can be compared with a DateTime. def <=>(other) - if other.respond_to? :to_datetime + if other.kind_of?(Infinity) + super + elsif other.respond_to? :to_datetime super other.to_datetime else nil diff --git a/activesupport/test/core_ext/range_ext_test.rb b/activesupport/test/core_ext/range_ext_test.rb index cfe31b75e8..98c4ec6b5e 100644 --- a/activesupport/test/core_ext/range_ext_test.rb +++ b/activesupport/test/core_ext/range_ext_test.rb @@ -16,6 +16,7 @@ class RangeTest < ActiveSupport::TestCase def test_date_range assert_instance_of Range, DateTime.new..DateTime.new assert_instance_of Range, DateTime::Infinity.new..DateTime::Infinity.new + assert_instance_of Range, DateTime.new..DateTime::Infinity.new end def test_overlaps_last_inclusive From ee255794b348e066bdaf60ccb583a1de9c84db6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 6 Aug 2014 22:48:22 -0300 Subject: [PATCH 14/25] Test using turbolinks master See https://github.com/rails/turbolinks/commit/153f1b0f04c718442cfd73365a2778dfe1a1c5c7 --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 49a68c7d9d..04cef17458 100644 --- a/Gemfile +++ b/Gemfile @@ -10,7 +10,7 @@ gem 'mocha', '~> 0.14', require: false gem 'rack', github: 'rack/rack' gem 'rack-cache', '~> 1.2' gem 'jquery-rails', '~> 3.1.0' -gem 'turbolinks' +gem 'turbolinks', github: 'rails/turbolinks', branch: 'master' gem 'coffee-rails', '~> 4.0.0' gem 'arel', github: 'rails/arel', branch: 'master' gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'master' From e6e81f856e969efda49166af82c27594f30ea592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 6 Aug 2014 23:44:00 -0300 Subject: [PATCH 15/25] Define id_was to get the previous value of the primary key Currently when we call id_was and we have a custom primary key name Active Record will return the current value of the primary key. This make impossible to correctly do an update operation if you change the id. Fixes #16413 --- activerecord/CHANGELOG.md | 11 +++++++++++ .../active_record/attribute_methods/primary_key.rb | 8 +++++++- activerecord/test/cases/primary_keys_test.rb | 10 ++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c7e8b264fb..4b70d883fe 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,14 @@ +* Define `id_was` to get the previous value of the primary key. + + Currently when we call id_was and we have a custom primary key name + Active Record will return the current value of the primary key. This + make impossible to correctly do an update operation if you change the + id. + + Fixes #16413. + + *Rafael Mendonça França* + * Deprecate `DatabaseTasks.load_schema` to act on the current connection. Use `.load_schema_current` instead. In the future `load_schema` will require the `configuration` to act on as an argument. diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index cadad60ddd..9bd333bbac 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -39,6 +39,12 @@ module ActiveRecord read_attribute_before_type_cast(self.class.primary_key) end + # Returns the primary key previous value. + def id_was + sync_with_transaction_state + attribute_was(self.class.primary_key) + end + protected def attribute_method?(attr_name) @@ -54,7 +60,7 @@ module ActiveRecord end end - ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast).to_set + ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was).to_set def dangerous_attribute_method?(method_name) super && !ID_ATTRIBUTE_METHODS.include?(method_name) diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index b04df7ce43..f19a6ea5e3 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -5,6 +5,7 @@ require 'models/subscriber' require 'models/movie' require 'models/keyboard' require 'models/mixed_case_monkey' +require 'models/dashboard' class PrimaryKeysTest < ActiveRecord::TestCase fixtures :topics, :subscribers, :movies, :mixed_case_monkeys @@ -164,6 +165,15 @@ class PrimaryKeysTest < ActiveRecord::TestCase MixedCaseMonkey.reset_primary_key assert_equal "monkeyID", MixedCaseMonkey.primary_key end + + def test_primary_key_update_with_custom_key_name + dashboard = Dashboard.create!(dashboard_id: '1') + dashboard.id = '2' + dashboard.save! + + dashboard = Dashboard.first + assert_equal '2', dashboard.id + end end class PrimaryKeyWithNoConnectionTest < ActiveRecord::TestCase From 14508aec701a366713572908de76fec134f6a2c3 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 7 Aug 2014 17:20:17 +0800 Subject: [PATCH 16/25] Remove ActionController::RaiseActionExceptions. The latest modification to the code was done in https://github.com/rails/rails/commit/5e3517ea. In Rails 3.2, `ActionController#rescue_action` was deprecated and `rescue_action_without_handler` is no longer being used. --- actionpack/lib/action_controller/test_case.rb | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 9d0ec6f4de..eb5d824cbc 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -456,7 +456,6 @@ module ActionController end def controller_class=(new_class) - prepare_controller_class(new_class) if new_class self._controller_class = new_class end @@ -473,11 +472,6 @@ module ActionController Class === constant && constant < ActionController::Metal end end - - def prepare_controller_class(new_class) - new_class.send :include, ActionController::TestCase::RaiseActionExceptions - end - end # Simulate a GET request with the given parameters. @@ -713,34 +707,6 @@ module ActionController end end - # When the request.remote_addr remains the default for testing, which is 0.0.0.0, the exception is simply raised inline - # (skipping the regular exception handling from rescue_action). If the request.remote_addr is anything else, the regular - # rescue_action process takes place. This means you can test your rescue_action code by setting remote_addr to something else - # than 0.0.0.0. - # - # The exception is stored in the exception accessor for further inspection. - module RaiseActionExceptions - def self.included(base) #:nodoc: - unless base.method_defined?(:exception) && base.method_defined?(:exception=) - base.class_eval do - attr_accessor :exception - protected :exception, :exception= - end - end - end - - protected - def rescue_action_without_handler(e) - self.exception = e - - if request.remote_addr == "0.0.0.0" - raise(e) - else - super(e) - end - end - end - include Behavior end end From 9a0e0594ab919b5b1a3a85bbe90f4a44922b1d84 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Thu, 7 Aug 2014 09:17:12 -0300 Subject: [PATCH 17/25] Fix typo [ci skip] --- activemodel/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index b1a3d660d6..c14b0688c7 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,6 +1,6 @@ * Passwords with spaces only allowed in `ActiveModel::SecurePassword`. - Presence validation can be used to resore old behavior. + Presence validation can be used to restore old behavior. *Yevhene Shemet* From 8aead812baaacb962c15d0ab27bdaf4c388b7d59 Mon Sep 17 00:00:00 2001 From: tsukasaoishi Date: Thu, 7 Aug 2014 22:41:23 +0900 Subject: [PATCH 18/25] Tables existence check query is executed in large quantities When Rails starts, tables existence check query is executed number of models. In case of mysql, SHOW TABLES LIKE 'table1'; SHOW TABLES LIKE 'table2'; SHOW TABLES LIKE 'table3'; ... SHOW TABLES LIKE 'table999'; Add process to get the names of all tables by one query. --- .../lib/active_record/connection_adapters/schema_cache.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/activerecord/lib/active_record/connection_adapters/schema_cache.rb b/activerecord/lib/active_record/connection_adapters/schema_cache.rb index 3116bed596..726810535d 100644 --- a/activerecord/lib/active_record/connection_adapters/schema_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/schema_cache.rb @@ -19,6 +19,7 @@ module ActiveRecord # A cached lookup for table existence. def table_exists?(name) + prepare_tables if @tables.blank? return @tables[name] if @tables.key? name @tables[name] = connection.table_exists?(name) @@ -82,6 +83,12 @@ module ActiveRecord def marshal_load(array) @version, @columns, @columns_hash, @primary_keys, @tables = array end + + private + + def prepare_tables + connection.tables.each { |table| @tables[table] = true } + end end end end From 4c30b4fc215585b93f87ac68e466ca477b10e7e0 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 7 Aug 2014 23:09:29 +0800 Subject: [PATCH 19/25] Fix spelling. --- actionpack/test/controller/test_case_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 3b3b15c061..a9ed5021bb 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -221,7 +221,7 @@ XML assert_equal 200, @response.status end - def test_head_params_as_sting + def test_head_params_as_string assert_raise(NoMethodError) { head :test_params, "document body", :id => 10 } end From 4e83815ce991efce2b84aa570f6673227ff0bb0d Mon Sep 17 00:00:00 2001 From: tsukasaoishi Date: Fri, 8 Aug 2014 01:03:09 +0900 Subject: [PATCH 20/25] change to empty? from blank? --- .../lib/active_record/connection_adapters/schema_cache.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/connection_adapters/schema_cache.rb b/activerecord/lib/active_record/connection_adapters/schema_cache.rb index 726810535d..a10ce330c7 100644 --- a/activerecord/lib/active_record/connection_adapters/schema_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/schema_cache.rb @@ -19,7 +19,7 @@ module ActiveRecord # A cached lookup for table existence. def table_exists?(name) - prepare_tables if @tables.blank? + prepare_tables if @tables.empty? return @tables[name] if @tables.key? name @tables[name] = connection.table_exists?(name) From 399f5f6346829210b4d58ed5f7c6d4d73ef67737 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 7 Aug 2014 12:10:31 -0700 Subject: [PATCH 21/25] use the uri parser so that newer version of Ruby work --- .../connection_adapters/connection_specification.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 5693031053..d28a54b8f9 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -32,7 +32,7 @@ module ActiveRecord # } def initialize(url) raise "Database URL cannot be empty" if url.blank? - @uri = URI.parse(url) + @uri = uri_parser.parse(url) @adapter = @uri.scheme.gsub('-', '_') @adapter = "postgresql" if @adapter == "postgres" From 6c9669e8f0553a6d5d67cf64ba2273f527a2072a Mon Sep 17 00:00:00 2001 From: Gregory Ostermayr Date: Thu, 7 Aug 2014 16:19:20 -0400 Subject: [PATCH 22/25] remove dead file_watcher code --- .../lib/active_support/file_watcher.rb | 36 ------------------- 1 file changed, 36 deletions(-) delete mode 100644 activesupport/lib/active_support/file_watcher.rb diff --git a/activesupport/lib/active_support/file_watcher.rb b/activesupport/lib/active_support/file_watcher.rb deleted file mode 100644 index 81e63e76a7..0000000000 --- a/activesupport/lib/active_support/file_watcher.rb +++ /dev/null @@ -1,36 +0,0 @@ -module ActiveSupport - class FileWatcher - class Backend - def initialize(path, watcher) - @watcher = watcher - @path = path - end - - def trigger(files) - @watcher.trigger(files) - end - end - - def initialize - @regex_matchers = {} - end - - def watch(pattern, &block) - @regex_matchers[pattern] = block - end - - def trigger(files) - trigger_files = Hash.new { |h,k| h[k] = Hash.new { |h2,k2| h2[k2] = [] } } - - files.each do |file, state| - @regex_matchers.each do |pattern, block| - trigger_files[block][state] << file if pattern === file - end - end - - trigger_files.each do |block, payload| - block.call payload - end - end - end -end From 22969898262b8c92bea1ac93b668a817c7511158 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 7 Aug 2014 15:28:31 -0700 Subject: [PATCH 23/25] defer running after_config hooks until after the object is allocated --- railties/lib/rails/application.rb | 33 ++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 292986b475..796c068a10 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -90,6 +90,10 @@ module Rails Rails.app_class = base end + def instance + super.run_load_hooks! + end + # Makes the +new+ method public. # # Note that Rails::Application inherits from Rails::Engine, which @@ -116,17 +120,13 @@ module Rails @ordered_railties = nil @railties = nil @message_verifiers = {} + @ran_load_hooks = false + + # are these actually used? + @initial_variable_values = initial_variable_values + @block = block add_lib_to_load_path! - ActiveSupport.run_load_hooks(:before_configuration, self) - - initial_variable_values.each do |variable_name, value| - if INITIAL_VARIABLES.include?(variable_name) - instance_variable_set("@#{variable_name}", value) - end - end - - instance_eval(&block) if block_given? end # Returns true if the application is initialized. @@ -134,6 +134,21 @@ module Rails @initialized end + def run_load_hooks! # :nodoc: + return self if @ran_load_hooks + @ran_load_hooks = true + ActiveSupport.run_load_hooks(:before_configuration, self) + + @initial_variable_values.each do |variable_name, value| + if INITIAL_VARIABLES.include?(variable_name) + instance_variable_set("@#{variable_name}", value) + end + end + + instance_eval(&@block) if @block + self + end + # Implements call according to the Rack API. It simply # dispatches the request to the underlying middleware stack. def call(env) From 8121eefc222fbd8979ab70d89725a2e330f5a9b2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 7 Aug 2014 15:50:46 -0700 Subject: [PATCH 24/25] add a new constructor that runs load hooks --- railties/lib/rails/application.rb | 4 ++++ .../test/application/multiple_applications_test.rb | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 796c068a10..61639be7c6 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -94,6 +94,10 @@ module Rails super.run_load_hooks! end + def create(initial_variable_values = {}, &block) + new(initial_variable_values, &block).run_load_hooks! + end + # Makes the +new+ method public. # # Note that Rails::Application inherits from Rails::Engine, which diff --git a/railties/test/application/multiple_applications_test.rb b/railties/test/application/multiple_applications_test.rb index 98707d22e4..9ebf163671 100644 --- a/railties/test/application/multiple_applications_test.rb +++ b/railties/test/application/multiple_applications_test.rb @@ -36,23 +36,23 @@ module ApplicationTests end def test_initialization_of_application_with_previous_config - application1 = AppTemplate::Application.new(config: Rails.application.config) - application2 = AppTemplate::Application.new + application1 = AppTemplate::Application.create(config: Rails.application.config) + application2 = AppTemplate::Application.create assert_equal Rails.application.config, application1.config, "Creating a new application while setting an initial config should result in the same config" assert_not_equal Rails.application.config, application2.config, "New applications without setting an initial config should not have the same config" end def test_initialization_of_application_with_previous_railties - application1 = AppTemplate::Application.new(railties: Rails.application.railties) - application2 = AppTemplate::Application.new + application1 = AppTemplate::Application.create(railties: Rails.application.railties) + application2 = AppTemplate::Application.create assert_equal Rails.application.railties, application1.railties assert_not_equal Rails.application.railties, application2.railties end def test_initialize_new_application_with_all_previous_initialization_variables - application1 = AppTemplate::Application.new( + application1 = AppTemplate::Application.create( config: Rails.application.config, railties: Rails.application.railties, routes_reloader: Rails.application.routes_reloader, From e81453ef92b37156dafc092093106c8e8b87b268 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 7 Aug 2014 16:03:09 -0700 Subject: [PATCH 25/25] need to call super --- railties/test/engine_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/railties/test/engine_test.rb b/railties/test/engine_test.rb index 8401494bd2..f46fb748f5 100644 --- a/railties/test/engine_test.rb +++ b/railties/test/engine_test.rb @@ -17,6 +17,7 @@ class EngineTest < ActiveSupport::TestCase attr_reader :hello def initialize @hello = "world" + super end end assert_equal "world", klass.instance.hello