diff options
30 files changed, 278 insertions, 61 deletions
@@ -6,6 +6,7 @@ gem 'arel', github: 'rails/arel', branch: 'master' gem 'mocha', '>= 0.11.2', :require => false gem 'rack-test', github: 'brynary/rack-test' +gem 'rack-cache', "~> 1.2" gem 'bcrypt-ruby', '~> 3.0.0' gem 'jquery-rails' diff --git a/actionmailer/lib/action_mailer/log_subscriber.rb b/actionmailer/lib/action_mailer/log_subscriber.rb index a6c163832e..3fe64759ac 100644 --- a/actionmailer/lib/action_mailer/log_subscriber.rb +++ b/actionmailer/lib/action_mailer/log_subscriber.rb @@ -1,13 +1,15 @@ module ActionMailer class LogSubscriber < ActiveSupport::LogSubscriber def deliver(event) + return unless logger.info? recipients = Array(event.payload[:to]).join(', ') - info("\nSent mail to #{recipients} (%1.fms)" % event.duration) + info("\nSent mail to #{recipients} (#{event.duration.round(1)}ms)") debug(event.payload[:mail]) end def receive(event) - info("\nReceived mail (%.1fms)" % event.duration) + return unless logger.info? + info("\nReceived mail (#{event.duration.round(1)}ms)") debug(event.payload[:mail]) end diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 7fd6cfa859..5a5c4b33f1 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,23 @@ ## Rails 4.0.0 (unreleased) ## +* Failsafe exception returns text/plain. *Steve Klabnik* + +* Remove actionpack's rack-cache dependency and declare the + dependency in the Gemfile. + + *Guillermo Iguarán* + +* Rename internal variables on ActionController::TemplateAssertions to prevent + naming collisions. @partials, @templates and @layouts are now prefixed with an underscore. + Fix #7459 + + *Yves Senn* + +* `resource` and `resources` don't modify the passed options hash + Fix #7777 + + *Yves Senn* + * Precompiled assets include aliases from foo.js to foo/index.js and vice versa. # Precompiles phone-<digest>.css and aliases phone/index.css to phone.css. diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index fd09d3b55b..7d292ac17c 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -18,7 +18,6 @@ Gem::Specification.new do |s| s.requirements << 'none' s.add_dependency('activesupport', version) - s.add_dependency('rack-cache', '~> 1.2') s.add_dependency('builder', '~> 3.1.0') s.add_dependency('rack', '~> 1.4.1') s.add_dependency('rack-test', '~> 0.6.1') diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index f41d1bb4b9..3d274e7dd7 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -4,6 +4,8 @@ module ActionController INTERNAL_PARAMS = %w(controller action format _method only_path) def start_processing(event) + return unless logger.info? + payload = event.payload params = payload[:params].except(*INTERNAL_PARAMS) format = payload[:format] @@ -14,6 +16,8 @@ module ActionController end def process_action(event) + return unless logger.info? + payload = event.payload additions = ActionController::Base.log_process_action(payload) @@ -22,35 +26,36 @@ module ActionController exception_class_name = payload[:exception].first status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name) end - message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in %.0fms" % event.duration + message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms" message << " (#{additions.join(" | ")})" unless additions.blank? info(message) end def halted_callback(event) - info "Filter chain halted as #{event.payload[:filter]} rendered or redirected" + info("Filter chain halted as #{event.payload[:filter]} rendered or redirected") end def send_file(event) - info("Sent file %s (%.1fms)" % [event.payload[:path], event.duration]) + info("Sent file #{event.payload[:path]} (#{event.duration.round(1)}ms)") end def redirect_to(event) - info "Redirected to #{event.payload[:location]}" + info("Redirected to #{event.payload[:location]}") end def send_data(event) - info("Sent data %s (%.1fms)" % [event.payload[:filename], event.duration]) + info("Sent data #{event.payload[:filename]} (#{event.duration.round(1)}ms)") end %w(write_fragment read_fragment exist_fragment? expire_fragment expire_page write_page).each do |method| class_eval <<-METHOD, __FILE__, __LINE__ + 1 def #{method}(event) + return unless logger.info? key_or_path = event.payload[:key] || event.payload[:path] human_name = #{method.to_s.humanize.inspect} - info("\#{human_name} \#{key_or_path} \#{"(%.1fms)" % event.duration}") + info("\#{human_name} \#{key_or_path} (\#{event.duration.round(1)}ms)") end METHOD end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 20c6e2bf53..3af378173a 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -12,16 +12,16 @@ module ActionController end def setup_subscriptions - @partials = Hash.new(0) - @templates = Hash.new(0) - @layouts = Hash.new(0) + @_partials = Hash.new(0) + @_templates = Hash.new(0) + @_layouts = Hash.new(0) ActiveSupport::Notifications.subscribe("render_template.action_view") do |name, start, finish, id, payload| path = payload[:layout] if path - @layouts[path] += 1 + @_layouts[path] += 1 if path =~ /^layouts\/(.*)/ - @layouts[$1] += 1 + @_layouts[$1] += 1 end end end @@ -32,11 +32,11 @@ module ActionController partial = path =~ /^.*\/_[^\/]*$/ if partial - @partials[path] += 1 - @partials[path.split("/").last] += 1 + @_partials[path] += 1 + @_partials[path.split("/").last] += 1 end - @templates[path] += 1 + @_templates[path] += 1 end end @@ -46,9 +46,9 @@ module ActionController end def process(*args) - @partials = Hash.new(0) - @templates = Hash.new(0) - @layouts = Hash.new(0) + @_partials = Hash.new(0) + @_templates = Hash.new(0) + @_layouts = Hash.new(0) super end @@ -88,7 +88,7 @@ module ActionController case options when NilClass, Regexp, String, Symbol options = options.to_s if Symbol === options - rendered = @templates + rendered = @_templates msg = message || sprintf("expecting <%s> but rendering with <%s>", options.inspect, rendered.keys) matches_template = @@ -109,15 +109,15 @@ module ActionController if options.key?(:layout) expected_layout = options[:layout] msg = message || sprintf("expecting layout <%s> but action rendered <%s>", - expected_layout, @layouts.keys) + expected_layout, @_layouts.keys) case expected_layout when String, Symbol - assert_includes @layouts.keys, expected_layout.to_s, msg + assert_includes @_layouts.keys, expected_layout.to_s, msg when Regexp - assert(@layouts.keys.any? {|l| l =~ expected_layout }, msg) + assert(@_layouts.keys.any? {|l| l =~ expected_layout }, msg) when nil, false - assert(@layouts.empty?, msg) + assert(@_layouts.empty?, msg) end end @@ -128,17 +128,17 @@ module ActionController assert_equal(v, actual_locals[k]) end elsif expected_count = options[:count] - actual_count = @partials[expected_partial] + actual_count = @_partials[expected_partial] msg = message || sprintf("expecting %s to be rendered %s time(s) but rendered %s time(s)", expected_partial, expected_count, actual_count) assert(actual_count == expected_count.to_i, msg) else msg = message || sprintf("expecting partial <%s> but action rendered <%s>", - options[:partial], @partials.keys) - assert_includes @partials, expected_partial, msg + options[:partial], @_partials.keys) + assert_includes @_partials, expected_partial, msg end elsif options.key?(:partial) - assert @partials.empty?, + assert @_partials.empty?, "Expected no partials to be rendered" end else diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index ab740a0190..402f29cd76 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -15,11 +15,11 @@ module ActionDispatch # If any exception happens inside the exceptions app, this middleware # catches the exceptions and returns a FAILSAFE_RESPONSE. class ShowExceptions - FAILSAFE_RESPONSE = [500, {'Content-Type' => 'text/html'}, - ["<html><body><h1>500 Internal Server Error</h1>" << + FAILSAFE_RESPONSE = [500, { 'Content-Type' => 'text/plain' }, + ["500 Internal Server Error\n" << "If you are the administrator of this website, then please read this web " << "application's log file and/or the web server's log file to find out what " << - "went wrong.</body></html>"]] + "went wrong."]] def initialize(app, exceptions_app) @app = app diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 49afa01d25..c5cf413c8f 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1038,7 +1038,7 @@ module ActionDispatch # === Options # Takes same options as +resources+. def resource(*resources, &block) - options = resources.extract_options! + options = resources.extract_options!.dup if apply_common_behavior_for(:resource, resources, options, &block) return self @@ -1204,7 +1204,7 @@ module ActionDispatch # # resource actions are at /admin/posts. # resources :posts, :path => "admin/posts" def resources(*resources, &block) - options = resources.extract_options! + options = resources.extract_options!.dup if apply_common_behavior_for(:resources, resources, options, &block) return self diff --git a/actionpack/lib/action_view/log_subscriber.rb b/actionpack/lib/action_view/log_subscriber.rb index cc3a871576..fd9a543e0a 100644 --- a/actionpack/lib/action_view/log_subscriber.rb +++ b/actionpack/lib/action_view/log_subscriber.rb @@ -3,10 +3,13 @@ module ActionView # # Provides functionality so that Rails can output logs from Action View. class LogSubscriber < ActiveSupport::LogSubscriber + VIEWS_PATTERN = /^app\/views\//.freeze + def render_template(event) + return unless logger.info? message = " Rendered #{from_rails_root(event.payload[:identifier])}" message << " within #{from_rails_root(event.payload[:layout])}" if event.payload[:layout] - message << (" (%.1fms)" % event.duration) + message << " (#{event.duration.round(1)}ms)" info(message) end alias :render_partial :render_template @@ -19,7 +22,7 @@ module ActionView protected def from_rails_root(string) - string.sub("#{Rails.root}/", "").sub(/^app\/views\//, "") + string.sub("#{Rails.root}/", "").sub(VIEWS_PATTERN, "") end end end diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index b7fd6ec7e1..5434b3421e 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -196,17 +196,17 @@ module ActionView :@_result, :@_routes, :@controller, - :@layouts, + :@_layouts, :@locals, :@method_name, :@output_buffer, - :@partials, + :@_partials, :@passed, :@rendered, :@request, :@routes, :@tagged_logger, - :@templates, + :@_templates, :@options, :@test_passed, :@view, diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb index 351b9c4cfa..ab1bd0e3b6 100644 --- a/actionpack/test/controller/show_exceptions_test.rb +++ b/actionpack/test/controller/show_exceptions_test.rb @@ -93,4 +93,20 @@ module ShowExceptions assert_equal 'text/html', response.content_type.to_s end end + + class ShowFailsafeExceptionsTest < ActionDispatch::IntegrationTest + def test_render_failsafe_exception + @app = ShowExceptionsOverridenController.action(:boom) + @exceptions_app = @app.instance_variable_get(:@exceptions_app) + @app.instance_variable_set(:@exceptions_app, nil) + $stderr = StringIO.new + + get '/', {}, 'HTTP_ACCEPT' => 'text/json' + assert_response :internal_server_error + assert_equal 'text/plain', response.content_type.to_s + + @app.instance_variable_set(:@exceptions_app, @exceptions_app) + $stderr = STDERR + end + end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 4e83ad16d7..93d89f7568 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1124,6 +1124,26 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/sheep/1/_it', _it_sheep_path(1) end + def test_resource_does_not_modify_passed_options + options = {:id => /.+?/, :format => /json|xml/} + self.class.stub_controllers do |routes| + routes.draw do + resource :user, options + end + end + assert_equal({:id => /.+?/, :format => /json|xml/}, options) + end + + def test_resources_does_not_modify_passed_options + options = {:id => /.+?/, :format => /json|xml/} + self.class.stub_controllers do |routes| + routes.draw do + resources :users, options + end + end + assert_equal({:id => /.+?/, :format => /json|xml/}, options) + end + def test_path_names get '/pt/projetos' assert_equal 'projects#index', @response.body diff --git a/actionpack/test/dispatch/session/cache_store_test.rb b/actionpack/test/dispatch/session/cache_store_test.rb index a74e165826..b8479e8836 100644 --- a/actionpack/test/dispatch/session/cache_store_test.rb +++ b/actionpack/test/dispatch/session/cache_store_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'fixtures/session_autoload_test/session_autoload_test/foo' class CacheStoreTest < ActionDispatch::IntegrationTest class TestController < ActionController::Base diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 67bec5f38e..8ff4c4706c 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,10 @@ ## Rails 4.0.0 (unreleased) ## +* Fix `reset_counters` crashing on `has_many :through` associations. + Fix #7822. + + *lulalala* + * Support for partial inserts. When inserting new records, only the fields which have been changed diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index c877079b25..d28cd560d9 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -22,8 +22,13 @@ module ActiveRecord counters.each do |association| has_many_association = reflect_on_association(association.to_sym) - foreign_key = has_many_association.foreign_key.to_s - child_class = has_many_association.klass + if has_many_association.is_a? ActiveRecord::Reflection::ThroughReflection + foreign_key = has_many_association.through_reflection.foreign_key.to_s + child_class = has_many_association.through_reflection.klass + else + foreign_key = has_many_association.foreign_key.to_s + child_class = has_many_association.klass + end belongs_to = child_class.reflect_on_all_associations(:belongs_to) reflection = belongs_to.find { |e| e.foreign_key.to_s == foreign_key && e.options[:counter_cache].present? } counter_name = reflection.counter_cache_column diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index a25f2c7bca..ca79950049 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -1,11 +1,13 @@ module ActiveRecord class LogSubscriber < ActiveSupport::LogSubscriber + IGNORE_PAYLOAD_NAMES = ["SCHEMA", "EXPLAIN"] + def self.runtime=(value) - Thread.current["active_record_sql_runtime"] = value + Thread.current[:active_record_sql_runtime] = value end def self.runtime - Thread.current["active_record_sql_runtime"] ||= 0 + Thread.current[:active_record_sql_runtime] ||= 0 end def self.reset_runtime @@ -24,9 +26,9 @@ module ActiveRecord payload = event.payload - return if 'SCHEMA' == payload[:name] + return if IGNORE_PAYLOAD_NAMES.include?(payload[:name]) - name = '%s (%.1fms)' % [payload[:name], event.duration] + name = "#{payload[:name]} (#{event.duration.round(1)}ms)" sql = payload[:sql].squeeze(' ') binds = nil diff --git a/activerecord/test/cases/counter_cache_test.rb b/activerecord/test/cases/counter_cache_test.rb index ee443741ca..fc46a249c8 100644 --- a/activerecord/test/cases/counter_cache_test.rb +++ b/activerecord/test/cases/counter_cache_test.rb @@ -10,9 +10,12 @@ require 'models/dog' require 'models/dog_lover' require 'models/person' require 'models/friendship' +require 'models/subscriber' +require 'models/subscription' +require 'models/book' class CounterCacheTest < ActiveRecord::TestCase - fixtures :topics, :categories, :categorizations, :cars, :dogs, :dog_lovers, :people, :friendships + fixtures :topics, :categories, :categorizations, :cars, :dogs, :dog_lovers, :people, :friendships, :subscribers, :subscriptions, :books class ::SpecialTopic < ::Topic has_many :special_replies, :foreign_key => 'parent_id' @@ -118,4 +121,14 @@ class CounterCacheTest < ActiveRecord::TestCase Person.reset_counters(michael.id, :followers) end end + + test "reset counter of has_many :through association" do + subscriber = subscribers('second') + Subscriber.reset_counters(subscriber.id, 'books') + Subscriber.increment_counter('books_count', subscriber.id) + + assert_difference 'subscriber.reload.books_count', -1 do + Subscriber.reset_counters(subscriber.id, 'books') + end + end end diff --git a/activerecord/test/cases/store_test.rb b/activerecord/test/cases/store_test.rb index dc47d40f41..562ca8d9ff 100644 --- a/activerecord/test/cases/store_test.rb +++ b/activerecord/test/cases/store_test.rb @@ -136,7 +136,7 @@ class StoreTest < ActiveRecord::TestCase end test "all stored attributes are returned" do - assert_equal [:color, :homepage, :favorite_food, :phone_number], Admin::User.stored_attributes[:settings] + assert_equal [:color, :homepage, :favorite_food], Admin::User.stored_attributes[:settings] end test "stores_attributes are class level settings" do diff --git a/activerecord/test/models/admin/user.rb b/activerecord/test/models/admin/user.rb index 35170faa76..467f3ccd39 100644 --- a/activerecord/test/models/admin/user.rb +++ b/activerecord/test/models/admin/user.rb @@ -1,7 +1,7 @@ class Admin::User < ActiveRecord::Base belongs_to :account store :settings, :accessors => [ :color, :homepage ] - store_accessor :settings, :favorite_food, :phone_number + store_accessor :settings, :favorite_food store :preferences, :accessors => [ :remember_login ] store :json_data, :accessors => [ :height, :weight ], :coder => JSON store :json_data_empty, :accessors => [ :is_a_good_guy ], :coder => JSON diff --git a/activerecord/test/models/subscription.rb b/activerecord/test/models/subscription.rb index 4bdb36ea46..bcac4738a3 100644 --- a/activerecord/test/models/subscription.rb +++ b/activerecord/test/models/subscription.rb @@ -1,4 +1,4 @@ class Subscription < ActiveRecord::Base - belongs_to :subscriber + belongs_to :subscriber, :counter_cache => :books_count belongs_to :book end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 798ea20efc..2e4ec96933 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -621,6 +621,7 @@ ActiveRecord::Schema.define do create_table :subscribers, :force => true, :id => false do |t| t.string :nick, :null => false t.string :name + t.column :books_count, :integer, :null => false, :default => 0 end add_index :subscribers, :nick, :unique => true diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 70d9ec5613..0a12ba6cdd 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,5 +1,7 @@ ## Rails 4.0.0 (unreleased) ## +* Optimize ActiveSupport::Cache::Entry to reduce memory and processing overhead. *Brian Durand* + * Tests tag the Rails log with the current test class and test case: [SessionsControllerTest] [test_0002_sign in] Processing by SessionsController#create as HTML @@ -237,6 +239,6 @@ * Remove deprecated ActiveSupport::JSON::Variable. *Erich Menge* -* Optimize ActiveSupport::Cache::Entry to reduce memory and processing overhead. *Brian Durand* +* Optimize log subscribers to check log level before doing any processing. *Brian Durand* Please check [3-2-stable](https://github.com/rails/rails/blob/3-2-stable/activesupport/CHANGELOG.md) for previous changes. diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index a681a8d6a9..690e5ce194 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -591,9 +591,9 @@ module ActiveSupport when NilClass 0 when String - value.bytesize + @v.bytesize else - @s = Marshal.dump(value).bytesize + @s = Marshal.dump(@v).bytesize end end end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index f8e4dd6349..febf0eeeff 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -616,7 +616,7 @@ end class MemoryStoreTest < ActiveSupport::TestCase def setup - @record_size = Marshal.dump("aaaaaaaaaa").bytesize + @record_size = ActiveSupport::Cache::Entry.new("aaaaaaaaaa").size @cache = ActiveSupport::Cache.lookup_store(:memory_store, :expires_in => 60, :size => @record_size * 10) end @@ -855,7 +855,7 @@ class CacheEntryTest < ActiveSupport::TestCase value = "value" * 100 entry = ActiveSupport::Cache::Entry.new(value, :compress => true, :compress_threshold => 1) assert_equal value, entry.value - assert (value.bytesize > entry.size), "value is compressed" + assert(value.bytesize > entry.size, "value is compressed") end def test_non_compress_values diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index b92b65214f..c8a6600da9 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -291,6 +291,12 @@ module Rails ActionDispatch::MiddlewareStack.new.tap do |middleware| app = self if rack_cache = config.action_controller.perform_caching && config.action_dispatch.rack_cache + begin + require 'rack/cache' + rescue LoadError => error + error.message << ' Be sure to add rack-cache to your Gemfile' + raise + end require "action_dispatch/http/rack_cache" middleware.use ::Rack::Cache, rack_cache end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 55a6b3f4f2..69027f2903 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -9,6 +9,10 @@ source 'https://rubygems.org' <%= assets_gemfile_entry %> <%= javascript_gemfile_entry %> +# Puts a simple HTTP cache in front of your app. +# For large-scale production use, consider using a caching reverse proxy like nginx, varnish, or squid. +gem 'rack-cache', '~> 1.2' + # To use ActiveModel has_secure_password # gem 'bcrypt-ruby', '~> 3.0.0' diff --git a/railties/lib/rails/generators/rails/resource_route/resource_route_generator.rb b/railties/lib/rails/generators/rails/resource_route/resource_route_generator.rb index 6a5d62803c..f33d56b564 100644 --- a/railties/lib/rails/generators/rails/resource_route/resource_route_generator.rb +++ b/railties/lib/rails/generators/rails/resource_route/resource_route_generator.rb @@ -1,13 +1,50 @@ module Rails module Generators class ResourceRouteGenerator < NamedBase + + # Properly nests namespaces passed into a generator + # + # $ rails generate resource admin/users/products + # + # should give you + # + # namespace :admin do + # namespace :users + # resources :products + # end + # end def add_resource_route return if options[:actions].present? - route_config = regular_class_path.collect{ |namespace| "namespace :#{namespace} do " }.join(" ") - route_config << "resources :#{file_name.pluralize}" - route_config << " end" * regular_class_path.size - route route_config + + # iterates over all namespaces and opens up blocks + regular_class_path.each_with_index do |namespace, index| + write("namespace :#{namespace} do", index + 1) + end + + # inserts the primary resource + write("resources :#{file_name.pluralize}", route_length + 1) + + # ends blocks + regular_class_path.each_index do |index| + write("end", route_length - index) + end + + # route prepends two spaces onto the front of the string that is passed, this corrects that + route route_string[2..-1] end + + private + def route_string + @route_string ||= "" + end + + def write(str, indent) + route_string << "#{" " * indent}#{str}\n" + end + + def route_length + regular_class_path.length + end end end end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 856a0e163b..7a646626c6 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -300,6 +300,11 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "Gemfile", /# gem 'debugger'/ end + def test_inclusion_of_rack_cache + run_generator + assert_file "Gemfile", /gem 'rack-cache'/ + end + def test_template_from_dir_pwd FileUtils.cd(Rails.root) assert_match(/It works from file!/, run_generator([destination_root, "-m", "lib/template.rb"])) diff --git a/railties/test/generators/namespaced_generators_test.rb b/railties/test/generators/namespaced_generators_test.rb index db2b8af217..ede779ea59 100644 --- a/railties/test/generators/namespaced_generators_test.rb +++ b/railties/test/generators/namespaced_generators_test.rb @@ -304,7 +304,7 @@ class NamespacedScaffoldGeneratorTest < NamespacedGeneratorTestCase # Route assert_file "config/routes.rb" do |route| - assert_match(/namespace :admin do resources :roles end$/, route) + assert_match(/^ namespace :admin do\n resources :roles\n end$/, route) end # Controller @@ -346,7 +346,7 @@ class NamespacedScaffoldGeneratorTest < NamespacedGeneratorTestCase # Route assert_file "config/routes.rb" do |route| - assert_no_match(/namespace :admin do resources :roles end$/, route) + assert_no_match(/^ namespace :admin do\n resources :roles\n end$$/, route) end # Controller @@ -364,4 +364,76 @@ class NamespacedScaffoldGeneratorTest < NamespacedGeneratorTestCase # Stylesheets (should not be removed) assert_file "app/assets/stylesheets/scaffold.css" end + + def test_scaffold_with_nested_namespace_on_invoke + run_generator [ "admin/user/special/role", "name:string", "description:string" ] + + # Model + assert_file "app/models/test_app/admin/user/special.rb", /module TestApp\n module Admin/ + assert_file "app/models/test_app/admin/user/special/role.rb", /module TestApp\n class Admin::User::Special::Role < ActiveRecord::Base/ + assert_file "test/unit/test_app/admin/user/special/role_test.rb", /module TestApp\n class Admin::User::Special::RoleTest < ActiveSupport::TestCase/ + assert_file "test/fixtures/test_app/admin/user/special/roles.yml" + assert_migration "db/migrate/create_test_app_admin_user_special_roles.rb" + + # Route + assert_file "config/routes.rb" do |route| + assert_match(/^ namespace :admin do\n namespace :user do\n namespace :special do\n resources :roles\n end\n end\n end$/, route) + end + + # Controller + assert_file "app/controllers/test_app/admin/user/special/roles_controller.rb" do |content| + assert_match(/module TestApp\n class Admin::User::Special::RolesController < ApplicationController/, content) + end + + assert_file "test/functional/test_app/admin/user/special/roles_controller_test.rb", + /module TestApp\n class Admin::User::Special::RolesControllerTest < ActionController::TestCase/ + + # Views + %w( + index + edit + new + show + _form + ).each { |view| assert_file "app/views/test_app/admin/user/special/roles/#{view}.html.erb" } + assert_no_file "app/views/layouts/admin/user/special/roles.html.erb" + + # Helpers + assert_file "app/helpers/test_app/admin/user/special/roles_helper.rb" + assert_file "test/unit/helpers/test_app/admin/user/special/roles_helper_test.rb" + + # Stylesheets + assert_file "app/assets/stylesheets/scaffold.css" + end + + def test_scaffold_with_nested_namespace_on_revoke + run_generator [ "admin/user/special/role", "name:string", "description:string" ] + run_generator [ "admin/user/special/role" ], :behavior => :revoke + + # Model + assert_file "app/models/test_app/admin/user/special.rb" # ( should not be remove ) + assert_no_file "app/models/test_app/admin/user/special/role.rb" + assert_no_file "test/unit/test_app/admin/user/special/role_test.rb" + assert_no_file "test/fixtures/test_app/admin/user/special/roles.yml" + assert_no_migration "db/migrate/create_test_app_admin_user_special_roles.rb" + + # Route + assert_file "config/routes.rb" do |route| + assert_no_match(/^ namespace :admin do\n namespace :user do\n namespace :special do\n resources :roles\n end\n end\n end$/, route) + end + + # Controller + assert_no_file "app/controllers/test_app/admin/user/special/roles_controller.rb" + assert_no_file "test/functional/test_app/admin/user/special/roles_controller_test.rb" + + # Views + assert_no_file "app/views/test_app/admin/user/special/roles" + + # Helpers + assert_no_file "app/helpers/test_app/admin/user/special/roles_helper.rb" + assert_no_file "test/unit/helpers/test_app/admin/user/special/roles_helper_test.rb" + + # Stylesheets (should not be removed) + assert_file "app/assets/stylesheets/scaffold.css" + end end diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb index 40c5188042..f802f3c4ad 100644 --- a/railties/test/generators/scaffold_generator_test.rb +++ b/railties/test/generators/scaffold_generator_test.rb @@ -142,7 +142,7 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase # Route assert_file "config/routes.rb" do |route| - assert_match(/namespace :admin do resources :roles end$/, route) + assert_match(/^ namespace :admin do\n resources :roles\n end$/, route) end # Controller |