diff options
74 files changed, 617 insertions, 337 deletions
@@ -8,7 +8,7 @@ gemspec gem 'mocha', '~> 0.14', require: false gem 'rack-cache', '~> 1.2' -gem 'bcrypt-ruby', '~> 3.0.0' +gem 'bcrypt-ruby', '~> 3.1.0' gem 'jquery-rails', '~> 2.2.0' gem 'turbolinks' gem 'coffee-rails', '~> 4.0.0' @@ -56,7 +56,7 @@ platforms :ruby do group :db do gem 'pg', '>= 0.11.0' gem 'mysql', '>= 2.9.0' - gem 'mysql2', '>= 0.3.10' + gem 'mysql2', '>= 0.3.13' end end diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 28597451ba..eac3488a62 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,10 @@ +* Skip routes pointing to a redirect or mounted application when generating urls + using an options hash as they aren't relevant and generate incorrect urls. + + Fixes #8018 + + *Andrew White* + * Move `MissingHelperError` out of the `ClassMethods` module. *Yves Senn* diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 97ab30ada0..748df26033 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -1,6 +1,8 @@ require 'rake/testtask' require 'rubygems/package_task' +test_files = Dir.glob('test/{abstract,controller,dispatch,assertions,journey,routing}/**/*_test.rb') + desc "Default Task" task :default => :test @@ -10,7 +12,7 @@ Rake::TestTask.new do |t| # make sure we include the tests in alphabetical order as on some systems # this will not happen automatically and the tests (as a whole) will error - t.test_files = Dir.glob('test/{abstract,controller,dispatch,assertions,journey}/**/*_test.rb').sort + t.test_files = test_files.sort t.warning = true t.verbose = true @@ -19,7 +21,7 @@ end namespace :test do task :isolated do ruby = File.join(*RbConfig::CONFIG.values_at('bindir', 'RUBY_INSTALL_NAME')) - Dir.glob("test/{abstract,controller,dispatch,assertions,journey}/**/*_test.rb").all? do |file| + test_files.all? do |file| sh(ruby, '-w', '-Ilib:test', file) end or raise "Failures" end diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index e288f026c7..7764763791 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -18,7 +18,11 @@ module ActionDispatch match_route(name, constraints) do |route| parameterized_parts = extract_parameterized_parts(route, options, recall, parameterize) - next if !name && route.requirements.empty? && route.parts.empty? + + # Skip this route unless a name has been provided or it is a + # standard Rails route since we can't determine whether an options + # hash passed to url_for matches a Rack application or a redirect. + next unless name || route.dispatcher? missing_keys = missing_keys(route, parameterized_parts) next unless missing_keys.empty? diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 50e1853094..c8eb0f6f2d 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -16,6 +16,14 @@ module ActionDispatch @app = app @path = path + # Unwrap any constraints so we can see what's inside for route generation. + # This allows the formatter to skip over any mounted applications or redirects + # that shouldn't be matched when using a url_for without a route name. + while app.is_a?(Routing::Mapper::Constraints) do + app = app.app + end + @dispatcher = app.is_a?(Routing::RouteSet::Dispatcher) + @constraints = constraints @defaults = defaults @required_defaults = nil @@ -93,6 +101,10 @@ module ActionDispatch end end + def dispatcher? + @dispatcher + end + def matches?(request) constraints.all? do |method, value| next true unless request.respond_to?(method) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 3ae9f92c0b..0e5dc1fc6c 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -186,10 +186,16 @@ module ActionDispatch klass = Journey::Router::Utils @path_parts.zip(args) do |part, arg| + parameterized_arg = arg.to_param + + if parameterized_arg.nil? || parameterized_arg.empty? + raise_generation_error(args) + end + # Replace each route parameter # e.g. :id for regular parameter or *path for globbing # with ruby string interpolation code - path.gsub!(/(\*|:)#{part}/, klass.escape_fragment(arg.to_param)) + path.gsub!(/(\*|:)#{part}/, klass.escape_fragment(parameterized_arg)) end path end @@ -197,6 +203,25 @@ module ActionDispatch def optimize_routes_generation?(t) t.send(:optimize_routes_generation?) end + + def raise_generation_error(args) + parts, missing_keys = [], [] + + @path_parts.zip(args) do |part, arg| + parameterized_arg = arg.to_param + + if parameterized_arg.nil? || parameterized_arg.empty? + missing_keys << part + end + + parts << [part, arg] + end + + message = "No route matches #{Hash[parts].inspect}" + message << " missing required keys: #{missing_keys.inspect}" + + raise ActionController::UrlGenerationError, message + end end def initialize(route, options) diff --git a/actionpack/test/active_record_unit.rb b/actionpack/test/active_record_unit.rb deleted file mode 100644 index 95fbb112c0..0000000000 --- a/actionpack/test/active_record_unit.rb +++ /dev/null @@ -1,91 +0,0 @@ -require 'abstract_unit' - -# Define the essentials -class ActiveRecordTestConnector - cattr_accessor :able_to_connect - cattr_accessor :connected - - # Set our defaults - self.connected = false - self.able_to_connect = true -end - -# Try to grab AR -unless defined?(ActiveRecord) && defined?(FixtureSet) - begin - PATH_TO_AR = "#{File.dirname(__FILE__)}/../../activerecord/lib" - raise LoadError, "#{PATH_TO_AR} doesn't exist" unless File.directory?(PATH_TO_AR) - $LOAD_PATH.unshift PATH_TO_AR - require 'active_record' - rescue LoadError => e - $stderr.print "Failed to load Active Record. Skipping Active Record assertion tests: #{e}" - ActiveRecordTestConnector.able_to_connect = false - end -end -$stderr.flush - - -# Define the rest of the connector -class ActiveRecordTestConnector - class << self - def setup - unless self.connected || !self.able_to_connect - setup_connection - load_schema - require_fixture_models - self.connected = true - end - rescue Exception => e # errors from ActiveRecord setup - $stderr.puts "\nSkipping ActiveRecord assertion tests: #{e}" - #$stderr.puts " #{e.backtrace.join("\n ")}\n" - self.able_to_connect = false - end - - private - def setup_connection - if Object.const_defined?(:ActiveRecord) - defaults = { :database => ':memory:' } - adapter = defined?(JRUBY_VERSION) ? 'jdbcsqlite3' : 'sqlite3' - options = defaults.merge :adapter => adapter, :timeout => 500 - ActiveRecord::Base.establish_connection(options) - ActiveRecord::Base.configurations = { 'sqlite3_ar_integration' => options } - ActiveRecord::Base.connection - - Object.send(:const_set, :QUOTED_TYPE, ActiveRecord::Base.connection.quote_column_name('type')) unless Object.const_defined?(:QUOTED_TYPE) - else - raise "Can't setup connection since ActiveRecord isn't loaded." - end - end - - # Load actionpack sqlite tables - def load_schema - File.read(File.dirname(__FILE__) + "/fixtures/db_definitions/sqlite.sql").split(';').each do |sql| - ActiveRecord::Base.connection.execute(sql) unless sql.blank? - end - end - - def require_fixture_models - Dir.glob(File.dirname(__FILE__) + "/fixtures/*.rb").each {|f| require f} - end - end -end - -class ActiveRecordTestCase < ActionController::TestCase - include ActiveRecord::TestFixtures - - # Set our fixture path - if ActiveRecordTestConnector.able_to_connect - self.fixture_path = [FIXTURE_LOAD_PATH] - self.use_transactional_fixtures = false - end - - def self.fixtures(*args) - super if ActiveRecordTestConnector.connected - end - - def run(*args) - super if ActiveRecordTestConnector.connected - end -end - -ActiveRecordTestConnector.setup diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index f7ec6d71b3..e851cc6a63 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -277,7 +277,7 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end def redirect - redirect_to :action => "get" + redirect_to action_url('get') end end @@ -511,8 +511,8 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end set.draw do - match ':action', :to => controller, :via => [:get, :post] - get 'get/:action', :to => controller + match ':action', :to => controller, :via => [:get, :post], :as => :action + get 'get/:action', :to => controller, :as => :get_action end self.singleton_class.send(:include, set.url_helpers) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 8e4339aa0c..e4c3ddd3f9 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3620,3 +3620,56 @@ class TestRouteDefaults < ActionDispatch::IntegrationTest assert_equal '/projects/1', url_for(controller: 'projects', action: 'show', id: 1, only_path: true) end end + +class TestRackAppRouteGeneration < ActionDispatch::IntegrationTest + stub_controllers do |routes| + Routes = routes + Routes.draw do + rack_app = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] } + mount rack_app, at: '/account', as: 'account' + mount rack_app, at: '/:locale/account', as: 'localized_account' + end + end + + def app + Routes + end + + include Routes.url_helpers + + def test_mounted_application_doesnt_match_unnamed_route + assert_raise(ActionController::UrlGenerationError) do + assert_equal '/account?controller=products', url_for(controller: 'products', action: 'index', only_path: true) + end + + assert_raise(ActionController::UrlGenerationError) do + assert_equal '/de/account?controller=products', url_for(controller: 'products', action: 'index', :locale => 'de', only_path: true) + end + end +end + +class TestRedirectRouteGeneration < ActionDispatch::IntegrationTest + stub_controllers do |routes| + Routes = routes + Routes.draw do + get '/account', to: redirect('/myaccount'), as: 'account' + get '/:locale/account', to: redirect('/%{locale}/myaccount'), as: 'localized_account' + end + end + + def app + Routes + end + + include Routes.url_helpers + + def test_redirect_doesnt_match_unnamed_route + assert_raise(ActionController::UrlGenerationError) do + assert_equal '/account?controller=products', url_for(controller: 'products', action: 'index', only_path: true) + end + + assert_raise(ActionController::UrlGenerationError) do + assert_equal '/de/account?controller=products', url_for(controller: 'products', action: 'index', :locale => 'de', only_path: true) + end + end +end diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 3d52b2e9ee..a286f77633 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -4,9 +4,13 @@ require 'abstract_unit' module ActionDispatch module Journey class TestRouter < ActiveSupport::TestCase + # TODO : clean up routing tests so we don't need this hack + class StubDispatcher < Routing::RouteSet::Dispatcher; end + attr_reader :routes def setup + @app = StubDispatcher.new @routes = Routes.new @router = Router.new(@routes, {}) @formatter = Formatter.new(@routes) @@ -306,7 +310,7 @@ module ActionDispatch def test_nil_path_parts_are_ignored path = Path::Pattern.new "/:controller(/:action(.:format))" - @router.routes.add_route nil, path, {}, {}, {} + @router.routes.add_route @app, path, {}, {}, {} params = { :controller => "tasks", :format => nil } extras = { :action => 'lol' } @@ -321,7 +325,7 @@ module ActionDispatch str = Router::Strexp.new("/", Hash[params], ['/', '.', '?'], true) path = Path::Pattern.new str - @router.routes.add_route nil, path, {}, {}, {} + @router.routes.add_route @app, path, {}, {}, {} path, _ = @formatter.generate(:path_info, nil, Hash[params], {}) assert_equal '/', path @@ -329,7 +333,7 @@ module ActionDispatch def test_generate_calls_param_proc path = Path::Pattern.new '/:controller(/:action)' - @router.routes.add_route nil, path, {}, {}, {} + @router.routes.add_route @app, path, {}, {}, {} parameterized = [] params = [ [:controller, "tasks"], @@ -347,7 +351,7 @@ module ActionDispatch def test_generate_id path = Path::Pattern.new '/:controller(/:action)' - @router.routes.add_route nil, path, {}, {}, {} + @router.routes.add_route @app, path, {}, {}, {} path, params = @formatter.generate( :path_info, nil, {:id=>1, :controller=>"tasks", :action=>"show"}, {}) @@ -357,7 +361,7 @@ module ActionDispatch def test_generate_escapes path = Path::Pattern.new '/:controller(/:action)' - @router.routes.add_route nil, path, {}, {}, {} + @router.routes.add_route @app, path, {}, {}, {} path, _ = @formatter.generate(:path_info, nil, { :controller => "tasks", @@ -368,7 +372,7 @@ module ActionDispatch def test_generate_extra_params path = Path::Pattern.new '/:controller(/:action)' - @router.routes.add_route nil, path, {}, {}, {} + @router.routes.add_route @app, path, {}, {}, {} path, params = @formatter.generate(:path_info, nil, { :id => 1, @@ -382,7 +386,7 @@ module ActionDispatch def test_generate_uses_recall_if_needed path = Path::Pattern.new '/:controller(/:action(/:id))' - @router.routes.add_route nil, path, {}, {}, {} + @router.routes.add_route @app, path, {}, {}, {} path, params = @formatter.generate(:path_info, nil, @@ -394,7 +398,7 @@ module ActionDispatch def test_generate_with_name path = Path::Pattern.new '/:controller(/:action)' - @router.routes.add_route nil, path, {}, {}, {} + @router.routes.add_route @app, path, {}, {}, {} path, params = @formatter.generate(:path_info, "tasks", @@ -541,7 +545,7 @@ module ActionDispatch def add_routes router, paths paths.each do |path| path = Path::Pattern.new path - router.routes.add_route nil, path, {}, {}, {} + router.routes.add_route @app, path, {}, {}, {} end end diff --git a/actionpack/test/routing/helper_test.rb b/actionpack/test/routing/helper_test.rb index a5588d95fa..0028aaa629 100644 --- a/actionpack/test/routing/helper_test.rb +++ b/actionpack/test/routing/helper_test.rb @@ -22,7 +22,7 @@ module ActionDispatch x = Class.new { include rs.url_helpers } - assert_raises ActionController::RoutingError do + assert_raises ActionController::UrlGenerationError do x.new.pond_duck_path Duck.new end end diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 2981e76ec5..60ddace7e4 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix default rendered format problem when calling `render` without :content_type option. + It should return :html. Fix #11393. + + *Gleb Mazovetskiy* *Oleg* *kennyj* + * Fix `link_to` with block and url hashes. Before: @@ -8,7 +13,7 @@ After: link_to(action: 'bar', controller: 'foo') { content_tag(:span, 'Example site') } - # => "<a href=\"/\"><span>Example site</span></a>" + # => "<a href=\"/foo/bar\"><span>Example site</span></a>" *Murahashi Sanemat Kenichi* diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index daa9a393b3..a4f04b0b3b 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -16,7 +16,7 @@ module ActionView # provided here will only work in the context of a request # (link_to_unless_current, for instance), which must be provided # as a method called #request on the context. - + BUTTON_TAG_METHOD_VERBS = %w{patch put delete} extend ActiveSupport::Concern include TagHelper @@ -289,7 +289,7 @@ module ActionView remote = html_options.delete('remote') method = html_options.delete('method').to_s - method_tag = %w{patch put delete}.include?(method) ? method_tag(method) : ''.html_safe + method_tag = BUTTON_TAG_METHOD_VERBS.include?(method) ? method_tag(method) : ''.html_safe form_method = method == 'get' ? 'get' : 'post' form_options = html_options.delete('form') || {} diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index 4d5c5db80c..668831dff3 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -11,7 +11,7 @@ module ActionView prepend_formats(template.formats) unless context.rendered_format - context.rendered_format = template.formats.first || formats.last + context.rendered_format = template.formats.first || formats.first end render_template(template, options[:layout], options[:locals]) diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index 07f8d36d93..67e3775f28 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -100,7 +100,7 @@ class TemplateDigestorTest < ActionView::TestCase assert_not_nil digest("level/recursion") # assert digest is stored end - def test_chaning_the_top_templete_on_recursion + def test_chaining_the_top_template_on_recursion assert digest("level/recursion") # assert recursion is possible assert_digest_difference("level/recursion") do @@ -110,7 +110,7 @@ class TemplateDigestorTest < ActionView::TestCase assert_not_nil digest("level/recursion") # assert digest is stored end - def test_chaning_the_partial_templete_on_recursion + def test_chaining_the_partial_template_on_recursion assert digest("level/recursion") # assert recursion is possible assert_digest_difference("level/recursion") do diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 8cffe73cce..928dfb092d 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -41,6 +41,11 @@ module RenderTestCases assert_match "<error>No Comment</error>", @view.render(:template => "comments/empty", :formats => [:xml]) end + def test_rendered_format_without_format + @view.render(:inline => "test") + assert_equal :html, @view.lookup_context.rendered_format + end + def test_render_partial_implicitly_use_format_of_the_rendered_template @view.lookup_context.formats = [:json] assert_equal "Hello world", @view.render(:template => "test/one", :formats => [:html]) diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 7156f1bb30..3d6de33e1e 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -18,9 +18,9 @@ module ActiveModel # value to the password_confirmation attribute and the validation # will not be triggered. # - # You need to add bcrypt-ruby (~> 3.0.0) to Gemfile to use #has_secure_password: + # You need to add bcrypt-ruby (~> 3.1.0) to Gemfile to use #has_secure_password: # - # gem 'bcrypt-ruby', '~> 3.0.0' + # gem 'bcrypt-ruby', '~> 3.1.0' # # Example using Active Record (which automatically includes ActiveModel::SecurePassword): # @@ -44,7 +44,7 @@ module ActiveModel # This is to avoid ActiveModel (and by extension the entire framework) # being dependent on a binary library. begin - gem 'bcrypt-ruby', '~> 3.0.0' + gem 'bcrypt-ruby', '~> 3.1.0' require 'bcrypt' rescue LoadError $stderr.puts "You don't have bcrypt-ruby installed in your application. Please add it to your Gemfile and run bundle install" diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb index 2803f69b6f..2864c2ba11 100644 --- a/activemodel/lib/active_model/serializers/xml.rb +++ b/activemodel/lib/active_model/serializers/xml.rb @@ -205,7 +205,7 @@ module ActiveModel Serializer.new(self, options).serialize(&block) end - # Sets the model +attributes+ from a JSON string. Returns +self+. + # Sets the model +attributes+ from an XML string. Returns +self+. # # class Person # include ActiveModel::Serializers::Xml diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1eb2f2d130..8e262b5fd7 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,30 @@ +* `change_column` for PostgreSQL adapter respects the `:array` option. + + *Yves Senn* + +* Remove deprecation warning from `attribute_missing` for attributes that are columns. + + *Arun Agrawal* + +* Remove extra decrement of transaction deep level. + + Fixes: #4566 + + *Paul Nikitochkin* + +* Reset @column_defaults when assigning `locking_column`. + We had a potential problem. For example: + + class Post < ActiveRecord::Base + self.column_defaults # if we call this unintentionally before setting locking_column ... + self.locking_column = 'my_locking_column' + end + + Post.column_defaults["my_locking_column"] + => nil # expected value is 0 ! + + *kennyj* + * Remove extra select and update queries on save/touch/destroy ActiveRecord model with belongs to reflection with option `touch: true`. diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index f1bec5787a..bdfafa5066 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -119,7 +119,7 @@ module ActiveRecord # the owner klass.table_name else - reflection.table_name + super end end diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index e0715d9dd1..8c528af399 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -25,11 +25,8 @@ module ActiveRecord attr_reader :tables delegate :options, :through_reflection, :source_reflection, :chain, :to => :reflection - delegate :table, :table_name, :to => :parent, :prefix => :parent delegate :alias_tracker, :to => :join_dependency - alias :alias_suffix :parent_table_name - def initialize(reflection, join_dependency, parent = nil) reflection.check_validity! @@ -47,6 +44,9 @@ module ActiveRecord @tables = construct_tables.reverse end + def parent_table_name; parent.table_name; end + alias :alias_suffix :parent_table_name + def ==(other) other.class == self.class && other.reflection == reflection && @@ -64,15 +64,20 @@ module ActiveRecord end end - def join_to(manager) + def join_constraints + joins = [] tables = @tables.dup - foreign_table = parent_table + + foreign_table = parent.table foreign_klass = parent.base_klass + scope_chain_iter = reflection.scope_chain.reverse_each + # The chain starts with the target table, but we want to end with it here (makes # more sense in this context), so we reverse - chain.reverse.each_with_index do |reflection, i| + chain.reverse_each do |reflection| table = tables.shift + klass = reflection.klass case reflection.source_macro when :belongs_to @@ -80,11 +85,10 @@ module ActiveRecord foreign_key = reflection.foreign_key when :has_and_belongs_to_many # Join the join table first... - manager.from(join( + joins << join( table, table[reflection.foreign_key]. - eq(foreign_table[reflection.active_record_primary_key]) - )) + eq(foreign_table[reflection.active_record_primary_key])) foreign_table, table = table, tables.shift @@ -95,38 +99,39 @@ module ActiveRecord foreign_key = reflection.active_record_primary_key end - constraint = build_constraint(reflection, table, key, foreign_table, foreign_key) + constraint = build_constraint(klass, table, key, foreign_table, foreign_key) - scope_chain_items = scope_chain[i] + scope_chain_items = scope_chain_iter.next.map do |item| + if item.is_a?(Relation) + item + else + ActiveRecord::Relation.new(klass, table).instance_exec(self, &item) + end + end if reflection.type - scope_chain_items += [ - ActiveRecord::Relation.new(reflection.klass, table) + scope_chain_items << + ActiveRecord::Relation.new(klass, table) .where(reflection.type => foreign_klass.base_class.name) - ] end - scope_chain_items += [reflection.klass.send(:build_default_scope)].compact + scope_chain_items.concat [klass.send(:build_default_scope)].compact - constraint = scope_chain_items.inject(constraint) do |chain, item| - unless item.is_a?(Relation) - item = ActiveRecord::Relation.new(reflection.klass, table).instance_exec(self, &item) - end + rel = scope_chain_items.inject(scope_chain_items.shift) do |left, right| + left.merge right + end - if item.arel.constraints.empty? - chain - else - chain.and(item.arel.constraints) - end + if rel && !rel.arel.constraints.empty? + constraint = constraint.and rel.arel.constraints end - manager.from(join(table, constraint)) + joins << join(table, constraint) # The current table in this iteration becomes the foreign table in the next - foreign_table, foreign_klass = table, reflection.klass + foreign_table, foreign_klass = table, klass end - manager + joins end # Builds equality condition. @@ -144,13 +149,13 @@ module ActiveRecord # foreign_table #=> #<Arel::Table @name="physicians" ...> # foreign_key #=> id # - def build_constraint(reflection, table, key, foreign_table, foreign_key) + def build_constraint(klass, table, key, foreign_table, foreign_key) constraint = table[key].eq(foreign_table[foreign_key]) - if reflection.klass.finder_needs_type_condition? + if klass.finder_needs_type_condition? constraint = table.create_and([ constraint, - reflection.klass.send(:type_condition, table) + klass.send(:type_condition, table) ]) end @@ -169,11 +174,6 @@ module ActiveRecord def aliased_table_name table.table_alias || table.name end - - def scope_chain - @scope_chain ||= reflection.scope_chain.reverse - end - end end end diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 0b96f88197..208da2cb77 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -176,20 +176,6 @@ module ActiveRecord end end - def attribute_missing(match, *args, &block) # :nodoc: - if self.class.columns_hash[match.attr_name] - ActiveSupport::Deprecation.warn( - "The method `#{match.method_name}', matching the attribute `#{match.attr_name}' has " \ - "dispatched through method_missing. This shouldn't happen, because `#{match.attr_name}' " \ - "is a column of the table. If this error has happened through normal usage of Active " \ - "Record (rather than through your own code or external libraries), please report it as " \ - "a bug." - ) - end - - super - end - # A Person object with a name attribute can ask <tt>person.respond_to?(:name)</tt>, # <tt>person.respond_to?(:name=)</tt>, and <tt>person.respond_to?(:name?)</tt> # which will all return +true+. It also define the attribute methods if they have diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index a8a1847554..c991c870ed 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -343,6 +343,7 @@ module ActiveRecord end records.each do |record| + next if record.destroyed? saved = true diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 41e07fbda9..e6b3c8ec9f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -20,6 +20,12 @@ module ActiveRecord attr_reader :query_cache, :query_cache_enabled + def initialize(*) + super + @query_cache = Hash.new { |h,sql| h[sql] = {} } + @query_cache_enabled = false + end + # Enable the query cache within the block. def cache old, @query_cache_enabled = @query_cache_enabled, true diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 75333765aa..b82d0fb872 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -95,8 +95,6 @@ module ActiveRecord @last_use = false @logger = logger @pool = pool - @query_cache = Hash.new { |h,sql| h[sql] = {} } - @query_cache_enabled = false @schema_cache = SchemaCache.new self @visitor = nil end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index a651b6c32e..3fce8de1ba 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -384,8 +384,9 @@ module ActiveRecord def change_column(table_name, column_name, type, options = {}) clear_cache! quoted_table_name = quote_table_name(table_name) - - execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" + sql_type = type_to_sql(type, options[:limit], options[:precision], options[:scale]) + sql_type << "[]" if options[:array] + execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{sql_type}" change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 209de78898..2a7996c4e7 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -138,6 +138,7 @@ module ActiveRecord # Set the column to use for optimistic locking. Defaults to +lock_version+. def locking_column=(value) + @column_defaults = nil @locking_column = value.to_s end diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index ac2d2f2712..44ea8610f2 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -253,19 +253,6 @@ module ActiveRecord @content_columns ||= columns.reject { |c| c.primary || c.name =~ /(_id|_count)$/ || c.name == inheritance_column } end - # Returns a hash of all the methods added to query each of the columns in the table with the name of the method as the key - # and true as the value. This makes it possible to do O(1) lookups in respond_to? to check if a given method for attribute - # is available. - def column_methods_hash #:nodoc: - @dynamic_methods_hash ||= column_names.each_with_object(Hash.new(false)) do |attr, methods| - attr_name = attr.to_s - methods[attr.to_sym] = attr_name - methods["#{attr}=".to_sym] = attr_name - methods["#{attr}?".to_sym] = attr_name - methods["#{attr}_before_type_cast".to_sym] = attr_name - end - end - # Resets all the cached information about columns, which will cause them # to be reloaded on the next request. # diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 8a9488656b..f470946da5 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -527,7 +527,9 @@ module ActiveRecord # def chain @chain ||= begin - chain = source_reflection.chain + through_reflection.chain + a = source_reflection.chain + b = through_reflection.chain + chain = a + b chain[0] = self # Use self so we don't lose the information from :source_type chain end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 3b24ed6754..52a538e5b5 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -189,11 +189,7 @@ module ActiveRecord distinct = self.distinct_value if operation == "count" - if select_values.present? - column_name ||= select_values.join(", ") - else - column_name ||= :all - end + column_name ||= select_for_count unless arel.ast.grep(Arel::Nodes::OuterJoin).empty? distinct = true @@ -361,6 +357,15 @@ module ActiveRecord column ? column.type_cast(value) : value end + # TODO: refactor to allow non-string `select_values` (eg. Arel nodes). + def select_for_count + if select_values.present? + select_values.join(", ") + else + :all + end + end + def build_count_subquery(relation, column_name, distinct) column_alias = Arel.sql('count_column') subquery_alias = Arel.sql('subquery_for_count') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 6d047e0331..da13152e01 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -97,19 +97,35 @@ module ActiveRecord def merge_multi_values lhs_wheres = relation.where_values rhs_wheres = values[:where] || [] + lhs_binds = relation.bind_values rhs_binds = values[:bind] || [] removed, kept = partition_overwrites(lhs_wheres, rhs_wheres) - relation.where_values = kept + rhs_wheres - relation.bind_values = filter_binds(lhs_binds, removed) + rhs_binds + where_values = kept + rhs_wheres + bind_values = filter_binds(lhs_binds, removed) + rhs_binds + + conn = relation.klass.connection + bviter = bind_values.each.with_index + where_values.map! do |node| + if Arel::Nodes::Equality === node && Arel::Nodes::BindParam === node.right + (column, _), i = bviter.next + substitute = conn.substitute_at column, i + Arel::Nodes::Equality.new(node.left, substitute) + else + node + end + end + + relation.where_values = where_values + relation.bind_values = bind_values if values[:reordering] # override any order specified in the original relation relation.reorder! values[:order] elsif values[:order] - # merge in order_values from r + # merge in order_values from relation relation.order! values[:order] end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 1327cc3c34..d0c56ac3d0 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -100,6 +100,14 @@ module ActiveRecord # firing an additional query. This will often result in a # performance improvement over a simple +join+. # + # You can also specify multiple relationships, like this: + # + # users = User.includes(:address, :friends) + # + # Loading nested relationships is possible using a Hash: + # + # users = User.includes(:address, friends: [:address, :followers]) + # # === conditions # # If you want to add conditions to your included models you'll have @@ -280,16 +288,18 @@ module ActiveRecord args.flatten! validate_order_args args - references = args.reject { |arg| Arel::Node === arg } + references = args.grep(String) references.map! { |arg| arg =~ /^([a-zA-Z]\w*)\.(\w+)/ && $1 }.compact! references!(references) if references.any? # if a symbol is given we prepend the quoted table name - args = args.map { |arg| - arg.is_a?(Symbol) ? "#{quoted_table_name}.#{arg} ASC" : arg + args.map! { |arg| + arg.is_a?(Symbol) ? + Arel::Nodes::Ascending.new(klass.arel_table[arg]) : + arg } - self.order_values = args + self.order_values + self.order_values = args.concat self.order_values self end @@ -946,10 +956,11 @@ module ActiveRecord join_dependency.graft(*stashed_association_joins) - # FIXME: refactor this to build an AST - join_dependency.join_associations.each do |association| - association.join_to(manager) - end + joins = join_dependency.join_associations.map { |association| + association.join_constraints + }.flatten + + joins.each { |join| manager.from join } manager.join_sources.concat join_list diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 62920d936f..dcbf38a89f 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -304,6 +304,7 @@ module ActiveRecord run_callbacks :rollback ensure restore_transaction_record_state(force_restore_state) + clear_transaction_record_state end # Add the record to the current transaction so that the +after_rollback+ and +after_commit+ callbacks @@ -360,8 +361,8 @@ module ActiveRecord # Restore the new record state and id of a record that was previously saved by a call to save_record_state. def restore_transaction_record_state(force = false) #:nodoc: unless @_start_transaction_state.empty? - @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1 - if @_start_transaction_state[:level] < 1 || force + transaction_level = (@_start_transaction_state[:level] || 0) - 1 + if transaction_level < 1 || force restore_state = @_start_transaction_state was_frozen = restore_state[:frozen?] @attributes = @attributes.dup if @attributes.frozen? @@ -374,7 +375,6 @@ module ActiveRecord @attributes_cache.delete(self.class.primary_key) end @attributes.freeze if was_frozen - @_start_transaction_state.clear end end end diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 61a3a2ba0f..e5e877f0b0 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -27,6 +27,18 @@ class PostgresqlArrayTest < ActiveRecord::TestCase assert @column.array end + def test_change_column_with_array + @connection.add_column :pg_arrays, :snippets, :string, array: true, default: [] + @connection.change_column :pg_arrays, :snippets, :text, array: true, default: "{}" + + PgArray.reset_column_information + column = PgArray.columns.find { |c| c.name == 'snippets' } + + assert_equal :text, column.type + assert_equal [], column.default + assert column.array + end + def test_type_cast_array assert @column diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 170c4cd9cd..baba55ea43 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -759,21 +759,6 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert subklass.method_defined?(:id), "subklass is missing id method" end - def test_dispatching_column_attributes_through_method_missing_deprecated - Topic.define_attribute_methods - - topic = Topic.new(:id => 5) - topic.id = 5 - - topic.method(:id).owner.send(:undef_method, :id) - - assert_deprecated do - assert_equal 5, topic.id - end - ensure - Topic.undefine_attribute_methods - end - def test_read_attribute_with_nil_should_not_asplode assert_equal nil, Topic.new.read_attribute(nil) end diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 580aa96ecd..635278abc1 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -705,6 +705,13 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase ids.each { |id| assert_nil klass.find_by_id(id) } end + def test_should_not_resave_destroyed_association + @pirate.birds.create!(name: :parrot) + @pirate.birds.first.destroy + @pirate.save! + assert @pirate.reload.birds.empty? + end + def test_should_skip_validation_on_has_many_if_marked_for_destruction 2.times { |i| @pirate.birds.create!(:name => "birds_#{i}") } diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 827fcf3d50..db7d3b80ab 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -17,6 +17,7 @@ class LockWithoutDefault < ActiveRecord::Base; end class LockWithCustomColumnWithoutDefault < ActiveRecord::Base self.table_name = :lock_without_defaults_cust + self.column_defaults # to test @column_defaults caching. self.locking_column = :custom_lock_version end diff --git a/activerecord/test/cases/migration/column_attributes_test.rb b/activerecord/test/cases/migration/column_attributes_test.rb index 6f5f29f0da..aa606ac8bb 100644 --- a/activerecord/test/cases/migration/column_attributes_test.rb +++ b/activerecord/test/cases/migration/column_attributes_test.rb @@ -16,32 +16,23 @@ module ActiveRecord end def test_add_remove_single_field_using_string_arguments - assert_not TestModel.column_methods_hash.key?(:last_name) + assert_no_column TestModel, :last_name add_column 'test_models', 'last_name', :string - - TestModel.reset_column_information - - assert TestModel.column_methods_hash.key?(:last_name) + assert_column TestModel, :last_name remove_column 'test_models', 'last_name' - - TestModel.reset_column_information - assert_not TestModel.column_methods_hash.key?(:last_name) + assert_no_column TestModel, :last_name end def test_add_remove_single_field_using_symbol_arguments - assert_not TestModel.column_methods_hash.key?(:last_name) + assert_no_column TestModel, :last_name add_column :test_models, :last_name, :string - - TestModel.reset_column_information - assert TestModel.column_methods_hash.key?(:last_name) + assert_column TestModel, :last_name remove_column :test_models, :last_name - - TestModel.reset_column_information - assert_not TestModel.column_methods_hash.key?(:last_name) + assert_no_column TestModel, :last_name end def test_unabstracted_database_dependent_types diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index e99312c245..ed080b2995 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -177,20 +177,18 @@ class MigrationTest < ActiveRecord::TestCase end def test_filtering_migrations - assert !Person.column_methods_hash.include?(:last_name) + assert_no_column Person, :last_name assert !Reminder.table_exists? name_filter = lambda { |migration| migration.name == "ValidPeopleHaveLastNames" } ActiveRecord::Migrator.up(MIGRATIONS_ROOT + "/valid", &name_filter) - Person.reset_column_information - assert Person.column_methods_hash.include?(:last_name) + assert_column Person, :last_name assert_raise(ActiveRecord::StatementInvalid) { Reminder.first } ActiveRecord::Migrator.down(MIGRATIONS_ROOT + "/valid", &name_filter) - Person.reset_column_information - assert !Person.column_methods_hash.include?(:last_name) + assert_no_column Person, :last_name assert_raise(ActiveRecord::StatementInvalid) { Reminder.first } end @@ -237,7 +235,7 @@ class MigrationTest < ActiveRecord::TestCase skip "not supported on #{ActiveRecord::Base.connection.class}" end - assert_not Person.column_methods_hash.include?(:last_name) + assert_no_column Person, :last_name migration = Class.new(ActiveRecord::Migration) { def version; 100 end @@ -253,8 +251,7 @@ 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), + assert_no_column Person, :last_name, "On error, the Migrator should revert schema changes but it did not." end @@ -263,7 +260,7 @@ class MigrationTest < ActiveRecord::TestCase skip "not supported on #{ActiveRecord::Base.connection.class}" end - assert_not Person.column_methods_hash.include?(:last_name) + assert_no_column Person, :last_name migration = Class.new(ActiveRecord::Migration) { def version; 100 end @@ -279,8 +276,7 @@ class MigrationTest < ActiveRecord::TestCase assert_equal "An error has occurred, this migration was canceled:\n\nSomething broke", e.message - Person.reset_column_information - assert_not Person.column_methods_hash.include?(:last_name), + assert_no_column Person, :last_name, "On error, the Migrator should revert schema changes but it did not." end @@ -289,7 +285,7 @@ class MigrationTest < ActiveRecord::TestCase skip "not supported on #{ActiveRecord::Base.connection.class}" end - assert_not Person.column_methods_hash.include?(:last_name) + assert_no_column Person, :last_name migration = Class.new(ActiveRecord::Migration) { self.disable_ddl_transaction! @@ -305,12 +301,11 @@ class MigrationTest < ActiveRecord::TestCase 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), + assert_column Person, :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) + if Person.column_names.include?('last_name') Person.connection.remove_column('people', 'last_name') end end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 693b36f35c..7c90f54343 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -9,6 +9,9 @@ module ActiveRecord fixtures :posts, :comments, :authors class FakeKlass < Struct.new(:table_name, :name) + def self.connection + Post.connection + end end def test_construction @@ -201,8 +204,12 @@ module ActiveRecord class RelationMutationTest < ActiveSupport::TestCase class FakeKlass < Struct.new(:table_name, :name) - def quoted_table_name - %{"#{table_name}"} + def arel_table + Post.arel_table + end + + def connection + Post.connection end end @@ -224,7 +231,17 @@ module ActiveRecord test "#order! with symbol prepends the table name" do assert relation.order!(:name).equal?(relation) - assert_equal ['"posts".name ASC'], relation.order_values + node = relation.order_values.first + assert node.ascending? + assert_equal :name, node.expr.name + assert_equal "posts", node.expr.relation.name + end + + test "#order! on non-string does not attempt regexp match for references" do + obj = Object.new + obj.expects(:=~).never + assert relation.order!(obj) + assert_equal [obj], relation.order_values end test '#references!' do @@ -285,7 +302,7 @@ module ActiveRecord assert_equal({foo: 'bar'}, relation.create_with_value) end - test 'merge!' do + def test_merge! assert relation.merge!(where: :foo).equal?(relation) assert_equal [:foo], relation.where_values end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index b205472cf5..d0f8731b9a 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1556,4 +1556,21 @@ class RelationTest < ActiveRecord::TestCase merged = left.merge(right) assert_equal binds, merged.bind_values end + + def test_merging_reorders_bind_params + post = Post.first + id_column = Post.columns_hash['id'] + title_column = Post.columns_hash['title'] + + bv = Post.connection.substitute_at id_column, 0 + + right = Post.where(id: bv) + right.bind_values += [[id_column, post.id]] + + left = Post.where(title: bv) + left.bind_values += [[title_column, post.title]] + + merged = left.merge(right) + assert_equal post, merged.first + end end diff --git a/activerecord/test/cases/test_case.rb b/activerecord/test/cases/test_case.rb index 70b970639a..c33e5cdc00 100644 --- a/activerecord/test/cases/test_case.rb +++ b/activerecord/test/cases/test_case.rb @@ -49,6 +49,18 @@ module ActiveRecord assert_queries(0, :ignore_none => true, &block) end + def assert_column(model, column_name, msg=nil) + assert has_column?(model, column_name), msg + end + + def assert_no_column(model, column_name, msg=nil) + assert_not has_column?(model, column_name), msg + end + + def has_column?(model, column_name) + model.reset_column_information + model.column_names.include?(column_name.to_s) + end end class SQLCounter diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index c0cad52b22..9c5f2e4724 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -117,6 +117,20 @@ class TransactionTest < ActiveRecord::TestCase assert !Topic.find(1).approved? end + def test_raising_exception_in_nested_transaction_restore_state_in_save + topic = Topic.new + + def topic.after_save_for_transaction + raise 'Make the transaction rollback' + end + + assert_raises(RuntimeError) do + Topic.transaction { topic.save } + end + + assert topic.new_record?, "#{topic.inspect} should be new record" + end + def test_update_should_rollback_on_failure author = Author.find(1) posts_count = author.posts.size diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index 414960d2b1..54cff6d394 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -244,14 +244,14 @@ module ActiveSupport # # ==== Examples # - # number_to_percentage(100) # => 100.000% - # number_to_percentage('98') # => 98.000% - # number_to_percentage(100, precision: 0) # => 100% - # number_to_percentage(1000, delimiter: '.', separator: ,') # => 1.000,000% - # number_to_percentage(302.24398923423, precision: 5) # => 302.24399% - # number_to_percentage(1000, locale: :fr) # => 1 000,000% - # number_to_percentage('98a') # => 98a% - # number_to_percentage(100, format: '%n %') # => 100 % + # number_to_percentage(100) # => 100.000% + # number_to_percentage('98') # => 98.000% + # number_to_percentage(100, precision: 0) # => 100% + # number_to_percentage(1000, delimiter: '.', separator: ',') # => 1.000,000% + # number_to_percentage(302.24398923423, precision: 5) # => 302.24399% + # number_to_percentage(1000, locale: :fr) # => 1 000,000% + # number_to_percentage('98a') # => 98a% + # number_to_percentage(100, format: '%n %') # => 100 % def number_to_percentage(number, options = {}) return unless number options = options.symbolize_keys diff --git a/activesupport/test/concern_test.rb b/activesupport/test/concern_test.rb index 8e2c298fc6..9b0a2ff7c9 100644 --- a/activesupport/test/concern_test.rb +++ b/activesupport/test/concern_test.rb @@ -68,12 +68,6 @@ class ConcernTest < ActiveSupport::TestCase assert_equal ConcernTest::Baz::ClassMethods, (class << @klass; self.included_modules; end)[0] end - def test_instance_methods_are_included - @klass.send(:include, Baz) - assert_equal "baz", @klass.new.baz - assert @klass.included_modules.include?(ConcernTest::Baz) - end - def test_included_block_is_ran @klass.send(:include, Baz) assert_equal true, @klass.included_ran diff --git a/guides/code/getting_started/Gemfile b/guides/code/getting_started/Gemfile index acd2ed5160..3d57012901 100644 --- a/guides/code/getting_started/Gemfile +++ b/guides/code/getting_started/Gemfile @@ -31,7 +31,7 @@ end gem 'jbuilder', '~> 1.2' # To use ActiveModel has_secure_password -# gem 'bcrypt-ruby', '~> 3.0.0' +# gem 'bcrypt-ruby', '~> 3.1.0' # Use unicorn as the app server # gem 'unicorn' diff --git a/guides/source/4_0_release_notes.md b/guides/source/4_0_release_notes.md index 66e26c63cb..006d0cda92 100644 --- a/guides/source/4_0_release_notes.md +++ b/guides/source/4_0_release_notes.md @@ -50,10 +50,47 @@ $ ruby /path/to/rails/railties/bin/rails new myapp --dev Major Features -------------- -TODO. Give a list and then talk about each of them briefly. We can point to relevant code commits or documentation from these sections. - [![Rails 4.0](images/rails4_features.png)](http://guides.rubyonrails.org/images/rails4_features.png) +### Upgrade + + * **Ruby 1.9.3** ([commit](https://github.com/rails/rails/commit/a0380e808d3dbd2462df17f5d3b7fcd8bd812496)) - Ruby 2.0 preferred; 1.9.3+ required + * **[New deprecation policy](http://www.youtube.com/watch?v=z6YgD6tVPQs)** - Deprecated features are warnings in Rails 4.0 and will be removed in Rails 4.1. + * **ActionPack page and action caching** ([commit](https://github.com/rails/rails/commit/b0a7068564f0c95e7ef28fc39d0335ed17d93e90)) - Page and action caching are extracted to a separate gem. Page and action caching requires too much manual intervention (manually expiring caches when the underlying model objects are updated). Instead, use Russian doll caching. + * **ActiveRecord observers** ([commit](https://github.com/rails/rails/commit/ccecab3ba950a288b61a516bf9b6962e384aae0b)) - Observers are extracted to a separate gem. Observers are only needed for page and action caching, and can lead to spaghetti code. + * **ActiveRecord session store** ([commit](https://github.com/rails/rails/commit/0ffe19056c8e8b2f9ae9d487b896cad2ce9387ad)) - The ActiveRecord session store is extracted to a separate gem. Storing sessions in SQL is costly. Instead, use cookie sessions, memcache sessions, or a custom session store. + * **ActiveModel mass assignment protection** ([commit](https://github.com/rails/rails/commit/f8c9a4d3e88181cee644f91e1342bfe896ca64c6)) - Rails 3 mass assignment protection is deprecated. Instead, use strong parameters. + * **ActiveResource** ([commit](https://github.com/rails/rails/commit/f1637bf2bb00490203503fbd943b73406e043d1d)) - ActiveResource is extracted to a separate gem. ActiveResource was not widely used. + * **vendor/plugins removed** ([commit](https://github.com/rails/rails/commit/853de2bd9ac572735fa6cf59fcf827e485a231c3)) - Use a Gemfile to manage installed gems. + +### ActionPack + + * **Strong parameters** ([commit](https://github.com/rails/rails/commit/a8f6d5c6450a7fe058348a7f10a908352bb6c7fc)) - Only allow whitelisted parameters to update model objects (`params.permit(:title, :text)`). + * **Routing concerns** ([commit](https://github.com/rails/rails/commit/0dd24728a088fcb4ae616bb5d62734aca5276b1b)) - In the routing DSL, factor out common subroutes (`comments` from `/posts/1/comments` and `/videos/1/comments`). + * **ActionController::Live** ([commit](https://github.com/rails/rails/commit/af0a9f9eefaee3a8120cfd8d05cbc431af376da3)) - Stream JSON with `response.stream`. + * **Declarative ETags** ([commit](https://github.com/rails/rails/commit/ed5c938fa36995f06d4917d9543ba78ed506bb8d)) - Set ETag and Last-Modified headers using `etag` and `fresh_when`. + * **[Russian doll caching](http://37signals.com/svn/posts/3113-how-key-based-cache-expiration-works)** ([commit](https://github.com/rails/rails/commit/4154bf012d2bec2aae79e4a49aa94a70d3e91d49)) - Cache nested fragments of views. Each fragment expires based on a set of dependencies (a cache key). The cache key is usually a template version number and a model object. + * **Turbolinks** ([commit](https://github.com/rails/rails/commit/e35d8b18d0649c0ecc58f6b73df6b3c8d0c6bb74)) - Serve only one initial HTML page. When the user navigates to another page, use pushState to update the URL and use AJAX to update the title and body. + * **Decouple ActionView from ActionController** ([commit](https://github.com/rails/rails/commit/78b0934dd1bb84e8f093fb8ef95ca99b297b51cd)) - ActionView is moved outside of ActionPack. + * **Do not depend on ActiveModel** ([commit](https://github.com/rails/rails/commit/166dbaa7526a96fdf046f093f25b0a134b277a68)) - ActionPack no longer depends on ActiveModel. + +### General + + * **ActiveModel::Model** ([commit](https://github.com/rails/rails/commit/3b822e91d1a6c4eab0064989bbd07aae3a6d0d08)) - `ActiveModel::Model` is extracted from ActiveRecord. `ActiveModel::Model` provides validations and `form_for` for normal Ruby objects. + * **New scope API** ([commit](https://github.com/rails/rails/commit/50cbc03d18c5984347965a94027879623fc44cce)) - Scopes must always use callables. + * **Schema cache dump** ([commit](https://github.com/rails/rails/commit/5ca4fc95818047108e69e22d200e7a4a22969477)) - To improve Rails boot time, instead of loading the schema directly from the database, load the schema from a dump file. + * **Support for specifying transaction isolation level** ([commit](https://github.com/rails/rails/commit/392eeecc11a291e406db927a18b75f41b2658253)) - Choose whether repeatable reads or improved performance (less locking) is more important. + * **Dalli** ([commit](https://github.com/rails/rails/commit/82663306f428a5bbc90c511458432afb26d2f238)) - For the memcache session store, use the Dalli memcache client. + * **Notifications start & finish** ([commit](https://github.com/rails/rails/commit/f08f8750a512f741acb004d0cebe210c5f949f28)) - Active Support instrumentation reports start and finish notifications to subscribers. + * **Thread safe by default** ([commit](https://github.com/rails/rails/commit/5d416b907864d99af55ebaa400fff217e17570cd)) - Rails can run in threaded app servers without additional configuration. Note: Check that the gems you are using are threadsafe. + * **PATCH verb** ([commit](https://github.com/rails/rails/commit/eed9f2539e3ab5a68e798802f464b8e4e95e619e)) - In Rails, PATCH replaces PUT. PATCH is used for partial updates of resources. + +### Security + + * **match do not catch all** ([commit](https://github.com/rails/rails/commit/90d2802b71a6e89aedfe40564a37bd35f777e541)) - In the routing DSL, match requires the HTTP verb or verbs to be specified. + * **html entities escaped by default** ([commit](https://github.com/rails/rails/commit/5f189f41258b83d49012ec5a0678d827327e7543)) - Strings rendered in erb are escaped unless wrapped with `raw` or `html_safe` is called. + * **New security headers** ([commit](https://github.com/rails/rails/commit/6794e92b204572d75a07bd6413bdae6ae22d5a82)) - Rails sends the following headers with every HTTP request: `X-Frame-Options` (prevents clickjacking by forbidding the browser from embedding the page in a frame), `X-XSS-Protection` (asks the browser to halt script injection) and `X-Content-Type-Options` (prevents the browser from opening a jpeg as an exe). + Extraction of features to gems --------------------------- diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index f2abd833aa..ecaee02cce 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -907,6 +907,92 @@ Now the user can request to get a PDF version of a client just by adding ".pdf" GET /clients/1.pdf ``` +### Live Streaming of Arbitrary Data + +Rails allows you to stream more than just files. In fact, you can stream anything +you would like in a response object. The `ActionController::Live` module allows +you to create a persistent connection with a browser. Using this module, you will +be able to send arbitrary data to the browser at specific points in time. + +#### Incorporating Live Streaming + +Including `ActionController::Live` inside of your controller class will provide +all actions inside of the controller the ability to stream data. You can mix in +the module like so: + +```ruby +class MyController < ActionController::Base + include ActionController::Live + + def stream + response.headers['Content-Type'] = 'text/event-stream' + 100.times { + response.stream.write "hello world\n" + sleep 1 + } + ensure + response.stream.close + end +end +``` + +The above code will keep a persistent connection with the browser and send 100 +messages of `"hello world\n"`, each one second apart. + +There are a couple of things to notice in the above example. We need to make +sure to close the response stream. Forgetting to close the stream will leave +the socket open forever. We also have to set the content type to `text/event-stream` +before we write to the response stream. This is because headers cannot be written +after the response has been committed (when `response.committed` returns a truthy +value), which occurs when you `write` or `commit` the response stream. + +#### Example Usage + +Let's suppose that you were making a Karaoke machine and a user wants to get the +lyrics for a particular song. Each `Song` has a particular number of lines and +each line takes time `num_beats` to finish singing. + +If we wanted to return the lyrics in Karaoke fashion (only sending the line when +the singer has finished the previous line), then we could use `ActionController::Live` +as follows: + +```ruby +class LyricsController < ActionController::Base + include ActionController::Live + + def show + response.headers['Content-Type'] = 'text/event-stream' + song = Song.find(params[:id]) + + song.each do |line| + response.stream.write line.lyrics + sleep line.num_beats + end + ensure + response.stream.close + end +end +``` + +The above code sends the next line only after the singer has completed the previous +line. + +#### Streaming Considerations + +Streaming arbitrary data is an extremely powerful tool. As shown in the previous +examples, you can choose when and what to send across a response stream. However, +you should also note the following things: + +* Each response stream creates a new thread and copies over the thread local + variables from the original thread. Having too many thread local variables can + negatively impact performance. Similarly, a large number of threads can also + hinder performance. +* Failing to close the response stream will leave the corresponding socket open + forever. Make sure to call `close` whenever you are using a response stream. +* WEBrick servers buffer all responses, and so including `ActionController::Live` + will not work. You must use a web server which does not automatically buffer + responses. + Log Filtering ------------- diff --git a/guides/source/active_record_callbacks.md b/guides/source/active_record_callbacks.md index 2e70e64b39..df1dd22971 100644 --- a/guides/source/active_record_callbacks.md +++ b/guides/source/active_record_callbacks.md @@ -49,8 +49,8 @@ The macro-style class methods can also receive a block. Consider using this styl class User < ActiveRecord::Base validates :login, :email, presence: true - before_create do |user| - user.name = user.login.capitalize if user.name.blank? + before_create do + self.name = login.capitalize if name.blank? end end ``` diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 18a5342a2f..0592821a14 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -711,7 +711,7 @@ Post.order('id DESC').limit(20).unscope(:order, :limit) = Post.all You can additionally unscope specific where clauses. For example: ```ruby -Post.where(id: 10).limit(1).unscope(:where, :limit).order('id DESC') = Post.order('id DESC') +Post.where(id: 10).limit(1).unscope({ where: :id }, :limit).order('id DESC') = Post.order('id DESC') ``` ### `only` diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index 3bfc600e7c..d95b587e78 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -121,7 +121,7 @@ database only if the object is valid: The bang versions (e.g. `save!`) raise an exception if the record is invalid. The non-bang versions don't, `save` and `update` return `false`, -`create` just return the objects. +`create` just returns the object. ### Skipping Validations diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index a6e3d3b0ac..6c4d7fe255 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -606,7 +606,7 @@ Customizing the Pipeline ### CSS Compression -There is currently one option for compressing CSS, YUI. The [YUI CSS compressor](http://developer.yahoo.com/yui/compressor/css.html) provides minification. +There is currently one option for compressing CSS, YUI. The [YUI CSS compressor](http://yui.github.io/yuicompressor/css.html) provides minification. The following line enables YUI compression, and requires the `yui-compressor` gem. @@ -620,7 +620,7 @@ The `config.assets.compress` must be set to `true` to enable CSS compression. Possible options for JavaScript compression are `:closure`, `:uglifier` and `:yui`. These require the use of the `closure-compiler`, `uglifier` or `yui-compressor` gems, respectively. -The default Gemfile includes [uglifier](https://github.com/lautis/uglifier). This gem wraps [UglifierJS](https://github.com/mishoo/UglifyJS) (written for NodeJS) in Ruby. It compresses your code by removing white space. It also includes other optimizations such as changing your `if` and `else` statements to ternary operators where possible. +The default Gemfile includes [uglifier](https://github.com/lautis/uglifier). This gem wraps [UglifyJS](https://github.com/mishoo/UglifyJS) (written for NodeJS) in Ruby. It compresses your code by removing white space and comments, shortening local variable names, and performing other micro-optimizations such as changing `if` and `else` statements to ternary operators where possible. The following line invokes `uglifier` for JavaScript compression. diff --git a/guides/source/initialization.md b/guides/source/initialization.md index 11c736585f..26259408b4 100644 --- a/guides/source/initialization.md +++ b/guides/source/initialization.md @@ -350,7 +350,7 @@ end The interesting part for a Rails app is the last line, `server.run`. Here we encounter the `wrapped_app` method again, which this time we're going to explore more (even though it was executed before, and -thus memorized by now). +thus memoized by now). ```ruby @wrapped_app ||= build_app app diff --git a/guides/source/plugins.md b/guides/source/plugins.md index 695f25f8a9..f5b4b0167c 100644 --- a/guides/source/plugins.md +++ b/guides/source/plugins.md @@ -68,7 +68,7 @@ In this example you will add a method to String named `to_squawk`. To begin, cre require 'test_helper' -class CoreExtTest < Test::Unit::TestCase +class CoreExtTest < ActiveSupport::TestCase def test_to_squawk_prepends_the_word_squawk assert_equal "squawk! Hello World", "Hello World".to_squawk end diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index 5992e94cd8..b7ec747a77 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -151,11 +151,13 @@ Rails 4.0 no longer supports loading plugins from `vendor/plugins`. You must rep * The `delete` method in collection associations can now receive `Fixnum` or `String` arguments as record ids, besides records, pretty much like the `destroy` method does. Previously it raised `ActiveRecord::AssociationTypeMismatch` for such arguments. From Rails 4.0 on `delete` automatically tries to find the records matching the given ids before deleting them. +* In Rails 4.0 when a column or a table is renamed the related indexes are also renamed. If you have migrations which rename the indexes, they are no longer needed. + * Rails 4.0 has changed how orders get stacked in `ActiveRecord::Relation`. In previous versions of Rails, the new order was applied after the previously defined order. But this is no longer true. Check [Active Record Query guide](active_record_querying.html#ordering) for more information. * Rails 4.0 has changed `serialized_attributes` and `attr_readonly` to class methods only. You shouldn't use instance methods since it's now deprecated. You should change them to use class methods, e.g. `self.serialized_attributes` to `self.class.serialized_attributes`. -* Rails 4.0 has removed `attr_accessible` and `attr_protected` feature in favor of Strong Parameters. You can use the [Protected Attributes gem](https://github.com/rails/protected_attributes) to a smoothly upgrade path. +* Rails 4.0 has removed `attr_accessible` and `attr_protected` feature in favor of Strong Parameters. You can use the [Protected Attributes gem](https://github.com/rails/protected_attributes) for a smooth upgrade path. * If you are not using Protected Attributes, you can remove any options related to this gem such as `whitelist_attributes` or `mass_assignment_sanitizer` options. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 13a3301fbd..7e7b157483 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,12 @@ +* Added generated unit test for generator generator using new + `test:generators` rake task. + + *Josef Šimánek* + +* Removed `update:application_controller` rake task. + + *Josef Šimánek* + * Fix `rake environment` to do not eager load modules *Paul Nikitochkin* diff --git a/railties/RDOC_MAIN.rdoc b/railties/RDOC_MAIN.rdoc index aa67005b24..eccdee7b07 100644 --- a/railties/RDOC_MAIN.rdoc +++ b/railties/RDOC_MAIN.rdoc @@ -55,7 +55,7 @@ can read more about Action Pack in its {README}[link:files/actionpack/README_rdo 5. Follow the guidelines to start developing your application. You may find the following resources handy: -* The README file created within your application. +* The \README file created within your application. * {Getting Started with \Rails}[http://guides.rubyonrails.org/getting_started.html]. * {Ruby on \Rails Tutorial}[http://ruby.railstutorial.org/ruby-on-rails-tutorial-book]. * {Ruby on \Rails Guides}[http://guides.rubyonrails.org]. diff --git a/railties/Rakefile b/railties/Rakefile index 5728c774bd..a899d069b5 100644 --- a/railties/Rakefile +++ b/railties/Rakefile @@ -1,10 +1,6 @@ require 'rake/testtask' require 'rubygems/package_task' -require 'date' -require 'rbconfig' - - task :default => :test desc "Run all unit tests" @@ -35,15 +31,6 @@ Rake::TestTask.new('test:regular') do |t| t.verbose = true end -# Update spinoffs ------------------------------------------------------------------- - -desc "Updates application README to the latest version Railties README" -task :update_readme do - readme = "lib/rails/generators/rails/app/templates/README" - rm readme - cp "./README.rdoc", readme -end - # Generate GEM ---------------------------------------------------------------------------- spec = eval(File.read('railties.gemspec')) diff --git a/railties/lib/rails/api/task.rb b/railties/lib/rails/api/task.rb index edd2283182..1f9568fb5f 100644 --- a/railties/lib/rails/api/task.rb +++ b/railties/lib/rails/api/task.rb @@ -7,7 +7,6 @@ module Rails 'activesupport' => { :include => %w( README.rdoc - CHANGELOG.md lib/active_support/**/*.rb ), :exclude => 'lib/active_support/vendor/*' @@ -16,7 +15,6 @@ module Rails 'activerecord' => { :include => %w( README.rdoc - CHANGELOG.md lib/active_record/**/*.rb ), :exclude => 'lib/active_record/vendor/*' @@ -25,7 +23,6 @@ module Rails 'activemodel' => { :include => %w( README.rdoc - CHANGELOG.md lib/active_model/**/*.rb ) }, @@ -33,19 +30,23 @@ module Rails 'actionpack' => { :include => %w( README.rdoc - CHANGELOG.md lib/abstract_controller/**/*.rb lib/action_controller/**/*.rb lib/action_dispatch/**/*.rb - lib/action_view/**/*.rb ), :exclude => 'lib/action_controller/vendor/*' }, + 'actionview' => { + :include => %w( + README.rdoc + lib/action_view/**/*.rb + ) + }, + 'actionmailer' => { :include => %w( README.rdoc - CHANGELOG.md lib/action_mailer/**/*.rb ), :exclude => 'lib/action_mailer/vendor/*' @@ -54,8 +55,6 @@ module Rails 'railties' => { :include => %w( README.rdoc - CHANGELOG.md - MIT-LICENSE lib/**/*.rb ), :exclude => 'lib/rails/generators/rails/**/templates/**/*.rb' diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index f761bef664..4faaf5ef1e 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -289,7 +289,6 @@ module Rails railties.each { |r| r.run_tasks_blocks(app) } super require "rails/tasks" - config = self.config task :environment do ActiveSupport.on_load(:before_initialize) { config.eager_load = false } diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 370c906086..570ff02c83 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -20,9 +20,7 @@ module Rails middleware.use ::ActionDispatch::SSL, config.ssl_options end - if config.action_dispatch.x_sendfile_header.present? - middleware.use ::Rack::Sendfile, config.action_dispatch.x_sendfile_header - end + middleware.use ::Rack::Sendfile, config.action_dispatch.x_sendfile_header if config.serve_static_assets middleware.use ::ActionDispatch::Static, paths["public"].first, config.static_cache_control diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 577ff651e5..adc83353b4 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -16,7 +16,7 @@ group :doc do end # Use ActiveModel has_secure_password -# gem 'bcrypt-ruby', '~> 3.0.0' +# gem 'bcrypt-ruby', '~> 3.1.0' # Use unicorn as the app server # gem 'unicorn' diff --git a/railties/lib/rails/generators/rails/generator/USAGE b/railties/lib/rails/generators/rails/generator/USAGE index d28eb3d7d8..799383050c 100644 --- a/railties/lib/rails/generators/rails/generator/USAGE +++ b/railties/lib/rails/generators/rails/generator/USAGE @@ -10,3 +10,4 @@ Example: lib/generators/awesome/awesome_generator.rb lib/generators/awesome/USAGE lib/generators/awesome/templates/ + test/lib/generators/awesome_generator_test.rb diff --git a/railties/lib/rails/generators/rails/generator/generator_generator.rb b/railties/lib/rails/generators/rails/generator/generator_generator.rb index 9a7a516b5b..15d88f06ac 100644 --- a/railties/lib/rails/generators/rails/generator/generator_generator.rb +++ b/railties/lib/rails/generators/rails/generator/generator_generator.rb @@ -10,6 +10,8 @@ module Rails directory '.', generator_dir end + hook_for :test_framework + protected def generator_dir diff --git a/railties/lib/rails/generators/test_unit/generator/generator_generator.rb b/railties/lib/rails/generators/test_unit/generator/generator_generator.rb new file mode 100644 index 0000000000..d7307398ce --- /dev/null +++ b/railties/lib/rails/generators/test_unit/generator/generator_generator.rb @@ -0,0 +1,26 @@ +require 'rails/generators/test_unit' + +module TestUnit # :nodoc: + module Generators # :nodoc: + class GeneratorGenerator < Base # :nodoc: + check_class_collision suffix: "GeneratorTest" + + class_option :namespace, type: :boolean, default: true, + desc: "Namespace generator under lib/generators/name" + + def create_generator_files + template 'generator_test.rb', File.join('test/lib/generators', class_path, "#{file_name}_generator_test.rb") + end + + protected + + def generator_path + if options[:namespace] + File.join("generators", regular_class_path, file_name, "#{file_name}_generator") + else + File.join("generators", regular_class_path, "#{file_name}_generator") + end + end + end + end +end diff --git a/railties/lib/rails/generators/test_unit/generator/templates/generator_test.rb b/railties/lib/rails/generators/test_unit/generator/templates/generator_test.rb new file mode 100644 index 0000000000..a7f1fc4fba --- /dev/null +++ b/railties/lib/rails/generators/test_unit/generator/templates/generator_test.rb @@ -0,0 +1,16 @@ +require 'test_helper' +require '<%= generator_path %>' + +<% module_namespacing do -%> +class <%= class_name %>GeneratorTest < Rails::Generators::TestCase + tests <%= class_name %>Generator + destination Rails.root.join('tmp/generators') + setup :prepare_destination + + # test "generator runs without errors" do + # assert_nothing_raised do + # run_generator ["arguments"] + # end + # end +end +<% end -%> diff --git a/railties/lib/rails/tasks/framework.rake b/railties/lib/rails/tasks/framework.rake index 76a95304c3..bd4e7c33e0 100644 --- a/railties/lib/rails/tasks/framework.rake +++ b/railties/lib/rails/tasks/framework.rake @@ -1,6 +1,6 @@ namespace :rails do - desc "Update configs and some other initially generated files (or use just update:configs, update:bin, or update:application_controller)" - task update: [ "update:configs", "update:bin", "update:application_controller" ] + desc "Update configs and some other initially generated files (or use just update:configs or update:bin)" + task update: [ "update:configs", "update:bin" ] desc "Applies the template supplied by LOCATION=(/path/to/template) or URL" task :template do @@ -62,15 +62,5 @@ namespace :rails do task :bin do invoke_from_app_generator :create_bin_files end - - # desc "Rename application.rb to application_controller.rb" - task :application_controller do - old_style = Rails.root + '/app/controllers/application.rb' - new_style = Rails.root + '/app/controllers/application_controller.rb' - if File.exists?(old_style) && !File.exists?(new_style) - FileUtils.mv(old_style, new_style) - puts "#{old_style} has been renamed to #{new_style}, update your SCM as necessary" - end - end end end diff --git a/railties/lib/rails/templates/rails/welcome/index.html.erb b/railties/lib/rails/templates/rails/welcome/index.html.erb index 4c4c80ecda..eb620caa00 100644 --- a/railties/lib/rails/templates/rails/welcome/index.html.erb +++ b/railties/lib/rails/templates/rails/welcome/index.html.erb @@ -232,8 +232,8 @@ </li> <li> - <h2>Create your database</h2> - <p>Run <code>rake db:create</code> to create your database. If you're not using SQLite (the default), edit <span class="filename">config/database.yml</span> with your username and password.</p> + <h2>Configure your database</h2> + <p>If you're not using SQLite (the default), edit <span class="filename">config/database.yml</span> with your username and password.</p> </li> </ol> </div> diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index 739e4ad992..46f7466551 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -6,6 +6,7 @@ require 'active_support/testing/autorun' require 'active_support/test_case' require 'action_controller/test_case' require 'action_dispatch/testing/integration' +require 'rails/generators/test_case' # Config Rails backtrace in tests. require 'rails/backtrace_cleaner' diff --git a/railties/lib/rails/test_unit/testing.rake b/railties/lib/rails/test_unit/testing.rake index 3aa33d1bba..547846c833 100644 --- a/railties/lib/rails/test_unit/testing.rake +++ b/railties/lib/rails/test_unit/testing.rake @@ -14,7 +14,7 @@ namespace :test do # Placeholder task for other Railtie and plugins to enhance. See Active Record for an example. end - task :run => ['test:units', 'test:functionals', 'test:integration'] + task :run => ['test:units', 'test:functionals', 'test:generators', 'test:integration'] # Inspired by: http://ngauthier.com/2012/02/quick-tests-with-bash.html desc "Run tests quickly by merging all types and not resetting db" @@ -35,6 +35,10 @@ namespace :test do end end + Rails::TestTask.new(generators: "test:prepare") do |t| + t.pattern = "test/lib/generators/**/*_test.rb" + end + Rails::TestTask.new(units: "test:prepare") do |t| t.pattern = 'test/{models,helpers,unit}/**/*_test.rb' end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 251fe02bc5..31a35a09bb 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -19,7 +19,7 @@ module ApplicationTests end test "default middleware stack" do - add_to_config "config.action_dispatch.x_sendfile_header = 'X-Sendfile'" + add_to_config "config.active_record.migration_error = :page_load" boot! @@ -37,6 +37,7 @@ module ApplicationTests "ActionDispatch::RemoteIp", "ActionDispatch::Reloader", "ActionDispatch::Callbacks", + "ActiveRecord::Migration::CheckPending", "ActiveRecord::ConnectionAdapters::ConnectionManagement", "ActiveRecord::QueryCache", "ActionDispatch::Cookies", @@ -49,12 +50,6 @@ module ApplicationTests ], middleware end - test "Rack::Sendfile is not included by default" do - boot! - - assert !middleware.include?("Rack::Sendfile"), "Rack::Sendfile is not included in the default stack unless you set config.action_dispatch.x_sendfile_header" - end - test "Rack::Cache is not included by default" do boot! @@ -96,6 +91,7 @@ module ApplicationTests boot! assert !middleware.include?("ActiveRecord::ConnectionAdapters::ConnectionManagement") assert !middleware.include?("ActiveRecord::QueryCache") + assert !middleware.include?("ActiveRecord::Migration::CheckPending") end test "removes lock if cache classes is set" do @@ -143,7 +139,7 @@ module ApplicationTests end test "insert middleware after" do - add_to_config "config.middleware.insert_after ActionDispatch::Static, Rack::Config" + add_to_config "config.middleware.insert_after Rack::Sendfile, Rack::Config" boot! assert_equal "Rack::Config", middleware.second end @@ -151,16 +147,16 @@ module ApplicationTests test "Rails.cache does not respond to middleware" do add_to_config "config.cache_store = :memory_store" boot! - assert_equal "Rack::Runtime", middleware.third + assert_equal "Rack::Runtime", middleware.fourth end test "Rails.cache does respond to middleware" do boot! - assert_equal "Rack::Runtime", middleware.fourth + assert_equal "Rack::Runtime", middleware.fifth end test "insert middleware before" do - add_to_config "config.middleware.insert_before ActionDispatch::Static, Rack::Config" + add_to_config "config.middleware.insert_before Rack::Sendfile, Rack::Config" boot! assert_equal "Rack::Config", middleware.first end diff --git a/railties/test/generators/generator_generator_test.rb b/railties/test/generators/generator_generator_test.rb index f4c975fc18..41181aee85 100644 --- a/railties/test/generators/generator_generator_test.rb +++ b/railties/test/generators/generator_generator_test.rb @@ -16,6 +16,10 @@ class GeneratorGeneratorTest < Rails::Generators::TestCase assert_file "lib/generators/awesome/awesome_generator.rb", /class AwesomeGenerator < Rails::Generators::NamedBase/ + assert_file "test/lib/generators/awesome_generator_test.rb", + /class AwesomeGeneratorTest < Rails::Generators::TestCase/ + assert_file "test/lib/generators/awesome_generator_test.rb", + /require 'generators\/awesome\/awesome_generator'/ end def test_namespaced_generator_skeleton @@ -29,6 +33,10 @@ class GeneratorGeneratorTest < Rails::Generators::TestCase assert_file "lib/generators/rails/awesome/awesome_generator.rb", /class Rails::AwesomeGenerator < Rails::Generators::NamedBase/ + assert_file "test/lib/generators/rails/awesome_generator_test.rb", + /class Rails::AwesomeGeneratorTest < Rails::Generators::TestCase/ + assert_file "test/lib/generators/rails/awesome_generator_test.rb", + /require 'generators\/rails\/awesome\/awesome_generator'/ end def test_generator_skeleton_is_created_without_file_name_namespace @@ -42,6 +50,10 @@ class GeneratorGeneratorTest < Rails::Generators::TestCase assert_file "lib/generators/awesome_generator.rb", /class AwesomeGenerator < Rails::Generators::NamedBase/ + assert_file "test/lib/generators/awesome_generator_test.rb", + /class AwesomeGeneratorTest < Rails::Generators::TestCase/ + assert_file "test/lib/generators/awesome_generator_test.rb", + /require 'generators\/awesome_generator'/ end def test_namespaced_generator_skeleton_without_file_name_namespace @@ -55,5 +67,9 @@ class GeneratorGeneratorTest < Rails::Generators::TestCase assert_file "lib/generators/rails/awesome_generator.rb", /class Rails::AwesomeGenerator < Rails::Generators::NamedBase/ + assert_file "test/lib/generators/rails/awesome_generator_test.rb", + /class Rails::AwesomeGeneratorTest < Rails::Generators::TestCase/ + assert_file "test/lib/generators/rails/awesome_generator_test.rb", + /require 'generators\/rails\/awesome_generator'/ end end diff --git a/tasks/release.rb b/tasks/release.rb index 0c22f812fc..66da67bfd0 100644 --- a/tasks/release.rb +++ b/tasks/release.rb @@ -1,4 +1,4 @@ -FRAMEWORKS = %w( activesupport activemodel activerecord actionpack actionmailer railties ) +FRAMEWORKS = %w( activesupport activemodel activerecord actionpack actionview actionmailer railties ) root = File.expand_path('../../', __FILE__) version = File.read("#{root}/RAILS_VERSION").strip diff --git a/tools/profile b/tools/profile index 665efe049d..fbea67492b 100755 --- a/tools/profile +++ b/tools/profile @@ -4,7 +4,6 @@ ENV['NO_RELOAD'] ||= '1' ENV['RAILS_ENV'] ||= 'development' -Gem.source_index require 'benchmark' module RequireProfiler |