From ab9622ee62e4ae6372fab69b9b148875be573a22 Mon Sep 17 00:00:00 2001 From: oleg dashevskii Date: Wed, 25 Aug 2010 15:52:00 +0700 Subject: [PATCH 01/85] Tests proving #5441 --- activerecord/test/cases/calculations_test.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index afef31396e056..3f963fc89abb4 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -275,6 +275,17 @@ def test_count_with_column_and_options_parameter assert_equal 2, Account.count(:firm_id, :conditions => "credit_limit = 50 AND firm_id IS NOT NULL") end + def test_should_count_field_in_joined_table + assert_equal 5, Account.count('companies.id', :joins => :firm) + assert_equal 4, Account.count('companies.id', :joins => :firm, :distinct => true) + end + + def test_should_count_field_in_joined_table_with_group_by + c = Account.count('companies.id', :group => :firm_id, :joins => :firm) + + [1,6,2,9].each { |firm_id| assert c.keys.include?(firm_id) } + end + def test_count_with_no_parameters_isnt_deprecated assert_not_deprecated { Account.count } end @@ -335,5 +346,4 @@ def test_from_option_with_specified_index def test_from_option_with_table_different_than_class assert_equal Account.count(:all), Company.count(:all, :from => 'accounts') end - end From cc18034c8c182df51a4fcb1a0a9b662f97d1f53d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 09:37:17 -0700 Subject: [PATCH 02/85] group clause must be more specific --- activerecord/test/cases/calculations_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 3f963fc89abb4..7ec40906d4636 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -281,7 +281,7 @@ def test_should_count_field_in_joined_table end def test_should_count_field_in_joined_table_with_group_by - c = Account.count('companies.id', :group => :firm_id, :joins => :firm) + c = Account.count('companies.id', :group => 'accounts.firm_id', :joins => :firm) [1,6,2,9].each { |firm_id| assert c.keys.include?(firm_id) } end From a8a62f87f6777b1bad8f38cc4b0239b74a4f8a7a Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 10:16:35 -0700 Subject: [PATCH 03/85] [#5441 state:resolved] refactoring code to determine aggregate column --- .../active_record/relation/calculations.rb | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 03862c78e4743..d79ef78b4d0c5 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -183,12 +183,16 @@ def perform_calculation(operation, column_name, options = {}) end end - def execute_simple_calculation(operation, column_name, distinct) #:nodoc: - column = if @klass.column_names.include?(column_name.to_s) + def aggregate_column(column_name) + if @klass.column_names.include?(column_name.to_s) Arel::Attribute.new(@klass.unscoped.table, column_name) else - Arel::SqlLiteral.new(column_name == :all ? "*" : column_name.to_s) + Arel.sql(column_name == :all ? "*" : column_name.to_s) end + end + + def execute_simple_calculation(operation, column_name, distinct) #:nodoc: + column = aggregate_column(column_name) # Postgresql doesn't like ORDER BY when there are no GROUP BY relation = except(:order) @@ -209,18 +213,17 @@ def execute_grouped_calculation(operation, column_name) #:nodoc: group = @klass.connection.adapter_name == 'FrontBase' ? group_alias : group_field - aggregate_alias = column_alias_for(operation, column_name) - - select_statement = if operation == 'count' && column_name == :all - ["COUNT(*) AS count_all"] + if operation == 'count' && column_name == :all + aggregate_alias = 'count_all' else - [Arel::Attribute.new(@klass.unscoped.table, column_name).send(operation).as(aggregate_alias)] + aggregate_alias = column_alias_for(operation, column_name) end - select_statement << "#{group_field} AS #{group_alias}" - relation = except(:group).group(group) - relation.select_values = select_statement + relation.select_values = [ + aggregate_column(column_name).send(operation).as(aggregate_alias), + "#{group_field} AS #{group_alias}" + ] calculated_data = @klass.connection.select_all(relation.to_sql) From ef8ce78ba1d37c7d4246f0c6b2cf13140c2898f8 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 11:00:14 -0700 Subject: [PATCH 04/85] changing map and include to find --- activeresource/test/cases/format_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activeresource/test/cases/format_test.rb b/activeresource/test/cases/format_test.rb index bed95ef5248ed..fc1a7b8c6fd2b 100644 --- a/activeresource/test/cases/format_test.rb +++ b/activeresource/test/cases/format_test.rb @@ -33,7 +33,7 @@ def test_formats_on_collection ActiveResource::HttpMock.respond_to.get "/people.#{format}", {'Accept' => ActiveResource::Formats[format].mime_type}, ActiveResource::Formats[format].encode(@programmers) remote_programmers = Person.find(:all) assert_equal 2, remote_programmers.size - assert remote_programmers.map { |p| p.name }.include? 'David' + assert remote_programmers.find { |p| p.name == 'David' } end end end From 505b532605bd3b9b8a444be10911f0864f1d42ba Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 12:13:12 -0700 Subject: [PATCH 05/85] speeding up object instantiation by eliminating instance_eval --- activerecord/lib/active_record/base.rb | 28 +++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2157a0adedc64..9204295d8c5cf 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -884,13 +884,9 @@ def relation #:nodoc: # single-table inheritance model that makes it possible to create # objects of different types from the same table. def instantiate(record) - find_sti_class(record[inheritance_column]).allocate.instance_eval do - @attributes, @attributes_cache, @previously_changed, @changed_attributes = record, {}, {}, {} - @new_record = @readonly = @destroyed = @marked_for_destruction = false - _run_find_callbacks - _run_initialize_callbacks - self - end + model = find_sti_class(record[inheritance_column]).allocate + model.init_with('attributes' => record) + model end def find_sti_class(type_name) @@ -1416,6 +1412,24 @@ def initialize_copy(other) populate_with_current_scope_attributes end + # Initialize an empty model object from +coder+. +coder+ must contain + # the attributes necessary for initializing an empty model object. For + # example: + # + # class Post < ActiveRecord::Base + # end + # + # post = Post.allocate + # post.init_with('attributes' => { 'title' => 'hello world' }) + # post.title # => 'hello world' + def init_with(coder) + @attributes = coder['attributes'] + @attributes_cache, @previously_changed, @changed_attributes = {}, {}, {} + @new_record = @readonly = @destroyed = @marked_for_destruction = false + _run_find_callbacks + _run_initialize_callbacks + end + # Returns a String, which Action Pack uses for constructing an URL to this # object. The default implementation returns this record's id as a String, # or nil if this record's unsaved. From ef6df93a8ddb675f1298973bb347825c554e95f9 Mon Sep 17 00:00:00 2001 From: Marcelo Giorgi Date: Thu, 30 Sep 2010 14:56:11 -0300 Subject: [PATCH 06/85] AssociationCollection#include? working properly for objects added with build method [#3472 state:resolved] --- .../associations/association_collection.rb | 13 +++++++++++++ .../has_and_belongs_to_many_associations_test.rb | 6 ++++++ .../associations/has_many_associations_test.rb | 6 ++++++ .../has_many_through_associations_test.rb | 14 ++++++++++++++ 4 files changed, 39 insertions(+) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 91e0a9f2f83c2..a617a0fb36f29 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -363,6 +363,7 @@ def replace(other_array) def include?(record) return false unless record.is_a?(@reflection.klass) + return include_in_memory?(record) if record.new_record? load_target if @reflection.options[:finder_sql] && !loaded? return @target.include?(record) if loaded? exists?(record) @@ -554,6 +555,18 @@ def fetch_first_or_last_using_find?(args) args.first.kind_of?(Hash) || !(loaded? || @owner.new_record? || @reflection.options[:finder_sql] || @target.any? { |record| record.new_record? } || args.first.kind_of?(Integer)) end + + def include_in_memory?(record) + if @reflection.is_a?(ActiveRecord::Reflection::ThroughReflection) + @owner.send(proxy_reflection.through_reflection.name.to_sym).map do |source| + source_reflection_target = source.send(proxy_reflection.source_reflection.name) + return true if source_reflection_target.respond_to?(:include?) ? source_reflection_target.include?(record) : source_reflection_target == record + end + false + else + @target.include?(record) + end + end end end end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index c0be7dfdcc163..7e070e17469e9 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -858,4 +858,10 @@ def test_attributes_are_being_set_when_initialized_from_habm_association_with_mu assert_equal new_developer.name, "Marcelo" assert_equal new_developer.salary, 90_000 end + + def test_include_method_in_has_and_belongs_to_many_association_should_return_true_for_instance_added_with_build + project = Project.new + developer = project.developers.build + assert project.developers.include?(developer) + end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 720b7fc38656f..c9f00fd737474 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1267,4 +1267,10 @@ def test_attributes_are_being_set_when_initialized_from_has_many_association_wit assert_equal new_comment.type, "SpecialComment" assert_equal new_comment.post_id, posts(:welcome).id end + + def test_include_method_in_has_many_association_should_return_true_for_instance_added_with_build + post = Post.new + comment = post.comments.build + assert post.comments.include?(comment) + end end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 0dac633852ad1..4b9f49f1ecddc 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -435,4 +435,18 @@ def test_attributes_are_being_set_when_initialized_from_hm_through_association_w assert_equal new_subscriber.nick, "marklazz" assert_equal new_subscriber.name, "Marcelo Giorgi" end + + def test_include_method_in_association_through_should_return_true_for_instance_added_with_build + person = Person.new + reference = person.references.build + job = reference.build_job + assert person.jobs.include?(job) + end + + def test_include_method_in_association_through_should_return_true_for_instance_added_with_nested_builds + author = Author.new + post = author.posts.build + comment = post.comments.build + assert author.comments.include?(comment) + end end From fb4ee9c7e5da783d84d3a633297830ceac58ee48 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 14:22:09 -0700 Subject: [PATCH 07/85] type_name is never a blank string, so use faster .nil? call --- activerecord/lib/active_record/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 9204295d8c5cf..aed4eff5659a1 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -890,7 +890,7 @@ def instantiate(record) end def find_sti_class(type_name) - if type_name.blank? || !columns_hash.include?(inheritance_column) + if type_name.nil? || !columns_hash.include?(inheritance_column) self else begin From 15419a5dc692be997d7637e40435a3d0d0fa4c1c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 14:35:36 -0700 Subject: [PATCH 08/85] build_where should be private --- activerecord/lib/active_record/relation/query_methods.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 2e0a2effc2999..c8174b5f455fb 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -196,6 +196,8 @@ def build_arel arel end + private + def build_where(opts, other = []) case opts when String, Array @@ -208,8 +210,6 @@ def build_where(opts, other = []) end end - private - def build_joins(relation, joins) association_joins = [] From 0238228e5d3ce1747a6f1d618693ffd44f947561 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 16:02:49 -0700 Subject: [PATCH 09/85] type_name should check for blank because people may have messed up databases --- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/cases/inheritance_test.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index aed4eff5659a1..9204295d8c5cf 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -890,7 +890,7 @@ def instantiate(record) end def find_sti_class(type_name) - if type_name.nil? || !columns_hash.include?(inheritance_column) + if type_name.blank? || !columns_hash.include?(inheritance_column) self else begin diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 8c09fc4d59d0b..31679b2efe655 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -14,6 +14,20 @@ def test_class_with_store_full_sti_class_returns_full_name ActiveRecord::Base.store_full_sti_class = old end + def test_class_with_blank_sti_name + company = Company.find(:first) + company = company.clone + company.extend(Module.new { + def read_attribute(name) + return ' ' if name == 'type' + super + end + }) + company.save! + company = Company.find(:all).find { |x| x.id == company.id } + assert_equal ' ', company.type + end + def test_class_without_store_full_sti_class_returns_demodulized_name old = ActiveRecord::Base.store_full_sti_class ActiveRecord::Base.store_full_sti_class = false From 45edeed1ee5148e837ea9680fa0c40e4b151d6d7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 16:28:12 -0700 Subject: [PATCH 10/85] Arel::Sql::Engine.new does not do anything anymore --- activerecord/lib/active_record.rb | 2 +- activerecord/lib/active_record/base.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index e2f2508ae80c9..f692e5ac89739 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -118,7 +118,7 @@ module ConnectionAdapters end ActiveSupport.on_load(:active_record) do - Arel::Table.engine = Arel::Sql::Engine.new(self) + Arel::Table.engine = self end I18n.load_path << File.dirname(__FILE__) + '/active_record/locale/en.yml' diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 9204295d8c5cf..80ddd5e7ab6f1 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -834,7 +834,7 @@ def arel_engine if self == ActiveRecord::Base Arel::Table.engine else - connection_handler.connection_pools[name] ? Arel::Sql::Engine.new(self) : superclass.arel_engine + connection_handler.connection_pools[name] ? self : superclass.arel_engine end end end From 542ddd8c891e27278cdac26763b6ff8aadbfdf68 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Fri, 1 Oct 2010 23:16:05 +0200 Subject: [PATCH 11/85] brings csrf_meta_tags back to the generated layout After more discussion, it has be agreed that this kind of changes within reasonable margins are OK for 3.1. That is, it is fine to change a little bit the generators even if that means examples in existing books won't be exact. (Note that the singular csrf_meta_tag exists as an alias and thus those outdated examples will run, same for existing applications.) --- railties/guides/source/getting_started.textile | 2 +- .../app/templates/app/views/layouts/application.html.erb.tt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/railties/guides/source/getting_started.textile b/railties/guides/source/getting_started.textile index 92b9131b596c1..9eae712a936fb 100644 --- a/railties/guides/source/getting_started.textile +++ b/railties/guides/source/getting_started.textile @@ -559,7 +559,7 @@ The view is only part of the story of how HTML is displayed in your web browser. Blog <%= stylesheet_link_tag :all %> <%= javascript_include_tag :defaults %> - <%= csrf_meta_tag %> + <%= csrf_meta_tags %> diff --git a/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt b/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt index 1dd112b4a66e2..1de78eecae12d 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt +++ b/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt @@ -4,7 +4,7 @@ <%= app_const_base %> <%%= stylesheet_link_tag :all %> <%%= javascript_include_tag :defaults %> - <%%= csrf_meta_tag %> + <%%= csrf_meta_tags %> From ff2fdcc52b391514cb62c2a1ef29827ac94914c6 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:22:42 -0700 Subject: [PATCH 12/85] removing AS::Testing::Default in favor of just undefing default_test --- .../lib/action_dispatch/testing/performance_test.rb | 3 +-- activesupport/lib/active_support/test_case.rb | 3 +-- activesupport/lib/active_support/testing/default.rb | 9 --------- activesupport/test/test_test.rb | 7 ------- 4 files changed, 2 insertions(+), 20 deletions(-) delete mode 100644 activesupport/lib/active_support/testing/default.rb diff --git a/actionpack/lib/action_dispatch/testing/performance_test.rb b/actionpack/lib/action_dispatch/testing/performance_test.rb index 33a5c68b9d696..d6c98b4db7be2 100644 --- a/actionpack/lib/action_dispatch/testing/performance_test.rb +++ b/actionpack/lib/action_dispatch/testing/performance_test.rb @@ -11,9 +11,8 @@ module ActionDispatch # formats are written, so you'll have two output files per test method. class PerformanceTest < ActionDispatch::IntegrationTest include ActiveSupport::Testing::Performance - include ActiveSupport::Testing::Default end end rescue NameError $stderr.puts "Specify ruby-prof as application's dependency in Gemfile to run benchmarks." -end \ No newline at end of file +end diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index ed8c02ba3e803..fb52fc708358d 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -24,8 +24,7 @@ class TestCase < ::Test::Unit::TestCase else Assertion = Test::Unit::AssertionFailedError - require 'active_support/testing/default' - include ActiveSupport::Testing::Default + undef :default_test end $tags = {} diff --git a/activesupport/lib/active_support/testing/default.rb b/activesupport/lib/active_support/testing/default.rb deleted file mode 100644 index a0bd6303c77c9..0000000000000 --- a/activesupport/lib/active_support/testing/default.rb +++ /dev/null @@ -1,9 +0,0 @@ -module ActiveSupport - module Testing - module Default #:nodoc: - # Placeholder so test/unit ignores test cases without any tests. - def default_test - end - end - end -end diff --git a/activesupport/test/test_test.rb b/activesupport/test/test_test.rb index cdaf63961aa85..ee5a20c789fd7 100644 --- a/activesupport/test/test_test.rb +++ b/activesupport/test/test_test.rb @@ -126,13 +126,6 @@ def test_assert_blank_false end end -# These should always pass -if ActiveSupport::Testing.const_defined?(:Default) - class NotTestingThingsTest < Test::Unit::TestCase - include ActiveSupport::Testing::Default - end -end - class AlsoDoingNothingTest < ActiveSupport::TestCase end From 61e8b23fe599ac54382251b44d9690f08a759fe1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:37:34 -0700 Subject: [PATCH 13/85] speed up index_by by removing a lolinject --- activesupport/lib/active_support/core_ext/enumerable.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index f1103029c8858..6ecedc26ef7ef 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -90,10 +90,7 @@ def each_with_object(memo, &block) # => { "Chade- Fowlersburg-e" => , "David Heinemeier Hansson" => , ...} # def index_by - inject({}) do |accum, elem| - accum[yield(elem)] = elem - accum - end + Hash[map { |elem| [yield(elem), elem] }] end # Returns true if the collection has more than 1 element. Functionally equivalent to collection.size > 1. From dfa331ae154c0475dfc631528071bdb06947acc2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:54:50 -0700 Subject: [PATCH 14/85] use a method that actually exists --- actionpack/test/controller/integration_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 4ff39fb76ca18..f0d62b0b1372d 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -167,7 +167,7 @@ def test_xml_http_request_override_accept class IntegrationTestTest < Test::Unit::TestCase def setup - @test = ::ActionDispatch::IntegrationTest.new(:default_test) + @test = ::ActionDispatch::IntegrationTest.new(:app) @test.class.stubs(:fixture_table_names).returns([]) @session = @test.open_session end From 6e1df2ca4631b8da6dae348af9d791f6e9aee1c5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:55:39 -0700 Subject: [PATCH 15/85] remove another lolinject --- activesupport/lib/active_support/cache/mem_cache_store.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index f32b562368483..9eee3fc5e36a5 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -73,7 +73,7 @@ def initialize(*addresses) def read_multi(*names) options = names.extract_options! options = merged_options(options) - keys_to_names = names.inject({}){|map, name| map[escape_key(namespaced_key(name, options))] = name; map} + keys_to_names = Hash[names.map{|name| [escape_key(namespaced_key(name, options)), name]}] raw_values = @data.get_multi(keys_to_names.keys, :raw => true) values = {} raw_values.each do |key, value| From 44f85678e967f1eccfaf448f82ca81111c9584af Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:01:34 -0700 Subject: [PATCH 16/85] delete repeated code --- actionpack/test/abstract_unit.rb | 18 ++++++++++++++++++ actionpack/test/controller/redirect_test.rb | 18 ------------------ actionpack/test/template/url_helper_test.rb | 18 ------------------ 3 files changed, 18 insertions(+), 36 deletions(-) diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 3540af13aca58..bf6d43da08455 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -308,3 +308,21 @@ class TestCase end end end + +class Workshop + extend ActiveModel::Naming + include ActiveModel::Conversion + attr_accessor :id + + def initialize(id) + @id = id + end + + def persisted? + id.present? + end + + def to_s + id.to_s + end +end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index b00142c92dfba..92d4a6d98b04b 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -3,24 +3,6 @@ class WorkshopsController < ActionController::Base end -class Workshop - extend ActiveModel::Naming - include ActiveModel::Conversion - attr_accessor :id - - def initialize(id) - @id = id - end - - def persisted? - id.present? - end - - def to_s - id.to_s - end -end - class RedirectController < ActionController::Base def simple_redirect redirect_to :action => "hello_world" diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index db8fd82aebbbb..98276da559268 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -547,24 +547,6 @@ def test_link_to_unless_current_shows_link end end -class Workshop - extend ActiveModel::Naming - include ActiveModel::Conversion - attr_accessor :id - - def initialize(id) - @id = id - end - - def persisted? - id.present? - end - - def to_s - id.to_s - end -end - class Session extend ActiveModel::Naming include ActiveModel::Conversion From ffbcb84c215bb615a3db4bb8bf8dcb977e72e32b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:05:59 -0700 Subject: [PATCH 17/85] removing more duplicate code --- actionpack/test/abstract_unit.rb | 17 +++++++++++++++++ actionpack/test/controller/rescue_test.rb | 16 ---------------- .../test/dispatch/show_exceptions_test.rb | 14 -------------- 3 files changed, 17 insertions(+), 30 deletions(-) diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index bf6d43da08455..470b36dbe2c52 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -326,3 +326,20 @@ def to_s id.to_s end end + +module ActionDispatch + class ShowExceptions + private + remove_method :public_path + def public_path + "#{FIXTURE_LOAD_PATH}/public" + end + + remove_method :logger + # Silence logger + def logger + nil + end + end +end + diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index a2418bb7c05db..c445285538f65 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -1,21 +1,5 @@ require 'abstract_unit' -module ActionDispatch - class ShowExceptions - private - remove_method :public_path - def public_path - "#{FIXTURE_LOAD_PATH}/public" - end - - remove_method :logger - # Silence logger - def logger - nil - end - end -end - class RescueController < ActionController::Base class NotAuthorized < StandardError end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 4ede1ab47ced9..ce6c397e32c40 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -1,19 +1,5 @@ require 'abstract_unit' -module ActionDispatch - class ShowExceptions - private - def public_path - "#{FIXTURE_LOAD_PATH}/public" - end - - # Silence logger - def logger - nil - end - end -end - class ShowExceptionsTest < ActionDispatch::IntegrationTest Boomer = lambda do |env| req = ActionDispatch::Request.new(env) From 50cf5c11a1b33cd082bbdf4f253581109955797c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:07:19 -0700 Subject: [PATCH 18/85] fixing warnings with regexps on assert_match --- actionpack/test/controller/log_subscriber_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 10873708fe580..90c944d8907ff 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -151,8 +151,8 @@ def test_with_fragment_cache_and_percent_in_key wait assert_equal 4, logs.size - assert_match /Exist fragment\? views\/foo%bar/, logs[1] - assert_match /Write fragment views\/foo%bar/, logs[2] + assert_match(/Exist fragment\? views\/foo%bar/, logs[1]) + assert_match(/Write fragment views\/foo%bar/, logs[2]) ensure @controller.config.perform_caching = true end From 3eb7f9adee4f606ac65e8e3d25098411b5a787b7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:09:37 -0700 Subject: [PATCH 19/85] removing more duplicate code. :'( --- .../test/controller/record_identifier_test.rb | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/actionpack/test/controller/record_identifier_test.rb b/actionpack/test/controller/record_identifier_test.rb index 835a0e970bb54..e46e317e31b80 100644 --- a/actionpack/test/controller/record_identifier_test.rb +++ b/actionpack/test/controller/record_identifier_test.rb @@ -1,17 +1,5 @@ require 'abstract_unit' - -class Comment - extend ActiveModel::Naming - include ActiveModel::Conversion - - attr_reader :id - def to_key; id ? [id] : nil end - def save; @id = 1 end - def new_record?; @id.nil? end - def name - @id.nil? ? 'new comment' : "comment ##{@id}" - end -end +require 'controller/fake_models' class Sheep extend ActiveModel::Naming From b95152201871f076a0d5c95e9e6387f68feab94e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 2 Oct 2010 08:36:55 +0200 Subject: [PATCH 20/85] Revert "Perf: refactor _assign method to avoid inject and defining unneeded local var." _assigns must return a hash. This reverts commit e66c1cee86aba1c81152f3d0872313e65cec85a9. --- actionpack/lib/action_view/test_case.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 915c2f90d7648..0eb4a663de35f 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -192,7 +192,11 @@ def _instance_variables end def _assigns - _instance_variables.map { |var| [var[1..-1].to_sym, instance_variable_get(var)] } + _instance_variables.inject({}) do |hash, var| + name = var[1..-1].to_sym + hash[name] = instance_variable_get(var) + hash + end end def _routes From 4e93179ed3f44825c157b54517e5a256f5725a55 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Thu, 30 Sep 2010 20:28:22 -0300 Subject: [PATCH 21/85] Refactor AssociationCollection#include? with objects in memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- .../active_record/associations/association_collection.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index a617a0fb36f29..cb2d9e0a796de 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -558,11 +558,10 @@ def fetch_first_or_last_using_find?(args) def include_in_memory?(record) if @reflection.is_a?(ActiveRecord::Reflection::ThroughReflection) - @owner.send(proxy_reflection.through_reflection.name.to_sym).map do |source| - source_reflection_target = source.send(proxy_reflection.source_reflection.name) - return true if source_reflection_target.respond_to?(:include?) ? source_reflection_target.include?(record) : source_reflection_target == record + @owner.send(proxy_reflection.through_reflection.name.to_sym).any? do |source| + target = source.send(proxy_reflection.source_reflection.name) + target.respond_to?(:include?) ? target.include?(record) : target == record end - false else @target.include?(record) end From 609849a0f10ce37d96444f0359ce325b01d916ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 2 Oct 2010 17:42:36 +0200 Subject: [PATCH 22/85] Fix a routing test. Reorganize middleware tests. --- .../middleware/best_practices_test.rb | 26 ++++++ .../test/application/middleware/cache_test.rb | 12 --- .../application/middleware/remote_ip_test.rb | 63 +++++++++++++ .../application/middleware/sendfile_test.rb | 56 ++++++++++++ railties/test/application/middleware_test.rb | 91 ------------------- railties/test/application/routing_test.rb | 56 ++---------- railties/test/isolation/abstract_unit.rb | 27 ++++++ 7 files changed, 180 insertions(+), 151 deletions(-) create mode 100644 railties/test/application/middleware/best_practices_test.rb create mode 100644 railties/test/application/middleware/remote_ip_test.rb create mode 100644 railties/test/application/middleware/sendfile_test.rb diff --git a/railties/test/application/middleware/best_practices_test.rb b/railties/test/application/middleware/best_practices_test.rb new file mode 100644 index 0000000000000..5b722e75103e7 --- /dev/null +++ b/railties/test/application/middleware/best_practices_test.rb @@ -0,0 +1,26 @@ +require 'isolation/abstract_unit' + +module ApplicationTests + class BestPracticesTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation + + def setup + build_app + boot_rails + require 'rack/test' + extend Rack::Test::Methods + simple_controller + end + + test "simple controller in production mode returns best standards" do + get '/foo' + assert_equal "IE=Edge,chrome=1", last_response.headers["X-UA-Compatible"] + end + + test "simple controller in development mode leaves out Chrome" do + app("development") + get "/foo" + assert_equal "IE=Edge", last_response.headers["X-UA-Compatible"] + end + end +end diff --git a/railties/test/application/middleware/cache_test.rb b/railties/test/application/middleware/cache_test.rb index 5675cebfd97ad..f582ed0e42f24 100644 --- a/railties/test/application/middleware/cache_test.rb +++ b/railties/test/application/middleware/cache_test.rb @@ -11,18 +11,6 @@ def setup extend Rack::Test::Methods end - def app(env = "production") - old_env = ENV["RAILS_ENV"] - - @app ||= begin - ENV["RAILS_ENV"] = env - require "#{app_path}/config/environment" - Rails.application - end - ensure - ENV["RAILS_ENV"] = old_env - end - def simple_controller controller :expires, <<-RUBY class ExpiresController < ApplicationController diff --git a/railties/test/application/middleware/remote_ip_test.rb b/railties/test/application/middleware/remote_ip_test.rb new file mode 100644 index 0000000000000..f28302d70a0eb --- /dev/null +++ b/railties/test/application/middleware/remote_ip_test.rb @@ -0,0 +1,63 @@ +require 'isolation/abstract_unit' + +module ApplicationTests + class RemoteIpTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation + + def setup + build_app + boot_rails + FileUtils.rm_rf "#{app_path}/config/environments" + end + + def app + @app ||= Rails.application + end + + def remote_ip(env = {}) + remote_ip = nil + env = Rack::MockRequest.env_for("/").merge(env).merge!( + 'action_dispatch.show_exceptions' => false, + 'action_dispatch.secret_token' => 'b3c631c314c0bbca50c1b2843150fe33' + ) + + endpoint = Proc.new do |e| + remote_ip = ActionDispatch::Request.new(e).remote_ip + [200, {}, ["Hello"]] + end + + Rails.application.middleware.build(endpoint).call(env) + remote_ip + end + + test "remote_ip works" do + make_basic_app + assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "1.1.1.1") + end + + test "checks IP spoofing by default" do + make_basic_app + assert_raises(ActionDispatch::RemoteIp::IpSpoofAttackError) do + remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") + end + end + + test "can disable IP spoofing check" do + make_basic_app do |app| + app.config.action_dispatch.ip_spoofing_check = false + end + + assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do + assert_equal "1.1.1.2", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") + end + end + + test "the user can set trusted proxies" do + make_basic_app do |app| + app.config.action_dispatch.trusted_proxies = /^4\.2\.42\.42$/ + end + + assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "4.2.42.42,1.1.1.1") + end + end +end diff --git a/railties/test/application/middleware/sendfile_test.rb b/railties/test/application/middleware/sendfile_test.rb new file mode 100644 index 0000000000000..0128261cd4d9f --- /dev/null +++ b/railties/test/application/middleware/sendfile_test.rb @@ -0,0 +1,56 @@ +require 'isolation/abstract_unit' + +module ApplicationTests + class SendfileTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation + + def setup + build_app + boot_rails + FileUtils.rm_rf "#{app_path}/config/environments" + end + + def app + @app ||= Rails.application + end + + define_method :simple_controller do + class ::OmgController < ActionController::Base + def index + send_file __FILE__ + end + end + end + + # x_sendfile_header middleware + test "config.action_dispatch.x_sendfile_header defaults to ''" do + make_basic_app + simple_controller + + get "/" + assert_equal File.read(__FILE__), last_response.body + end + + test "config.action_dispatch.x_sendfile_header can be set" do + make_basic_app do |app| + app.config.action_dispatch.x_sendfile_header = "X-Sendfile" + end + + simple_controller + + get "/" + assert_equal File.expand_path(__FILE__), last_response.headers["X-Sendfile"] + end + + test "config.action_dispatch.x_sendfile_header is sent to Rack::Sendfile" do + make_basic_app do |app| + app.config.action_dispatch.x_sendfile_header = 'X-Lighttpd-Send-File' + end + + simple_controller + + get "/" + assert_equal File.expand_path(__FILE__), last_response.headers["X-Lighttpd-Send-File"] + end + end +end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index f9b594eb33b31..2372ad85b8ae2 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -129,81 +129,6 @@ def app assert_equal "Rack::Config", middleware.first end - # x_sendfile_header middleware - test "config.action_dispatch.x_sendfile_header defaults to ''" do - make_basic_app - - class ::OmgController < ActionController::Base - def index - send_file __FILE__ - end - end - - get "/" - assert_equal File.read(__FILE__), last_response.body - end - - test "config.action_dispatch.x_sendfile_header can be set" do - make_basic_app do |app| - app.config.action_dispatch.x_sendfile_header = "X-Sendfile" - end - - class ::OmgController < ActionController::Base - def index - send_file __FILE__ - end - end - - get "/" - assert_equal File.expand_path(__FILE__), last_response.headers["X-Sendfile"] - end - - test "config.action_dispatch.x_sendfile_header is sent to Rack::Sendfile" do - make_basic_app do |app| - app.config.action_dispatch.x_sendfile_header = 'X-Lighttpd-Send-File' - end - - class ::OmgController < ActionController::Base - def index - send_file __FILE__ - end - end - - get "/" - assert_equal File.expand_path(__FILE__), last_response.headers["X-Lighttpd-Send-File"] - end - - # remote_ip tests - test "remote_ip works" do - make_basic_app - assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "1.1.1.1") - end - - test "checks IP spoofing by default" do - make_basic_app - assert_raises(ActionDispatch::RemoteIp::IpSpoofAttackError) do - remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") - end - end - - test "can disable IP spoofing check" do - make_basic_app do |app| - app.config.action_dispatch.ip_spoofing_check = false - end - - assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do - assert_equal "1.1.1.2", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") - end - end - - test "the user can set trusted proxies" do - make_basic_app do |app| - app.config.action_dispatch.trusted_proxies = /^4\.2\.42\.42$/ - end - - assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "4.2.42.42,1.1.1.1") - end - test "show exceptions middleware filter backtrace before logging" do my_middleware = Struct.new(:app) do def call(env) @@ -232,21 +157,5 @@ def boot! def middleware AppTemplate::Application.middleware.map(&:klass).map(&:name) end - - def remote_ip(env = {}) - remote_ip = nil - env = Rack::MockRequest.env_for("/").merge(env).merge!( - 'action_dispatch.show_exceptions' => false, - 'action_dispatch.secret_token' => 'b3c631c314c0bbca50c1b2843150fe33' - ) - - endpoint = Proc.new do |e| - remote_ip = ActionDispatch::Request.new(e).remote_ip - [200, {}, ["Hello"]] - end - - Rails.application.middleware.build(endpoint).call(env) - remote_ip - end end end diff --git a/railties/test/application/routing_test.rb b/railties/test/application/routing_test.rb index 416a5de5b0df1..e42f5720a9df8 100644 --- a/railties/test/application/routing_test.rb +++ b/railties/test/application/routing_test.rb @@ -11,34 +11,6 @@ def setup extend Rack::Test::Methods end - def app(env = "production") - old_env = ENV["RAILS_ENV"] - - @app ||= begin - ENV["RAILS_ENV"] = env - require "#{app_path}/config/environment" - Rails.application - end - ensure - ENV["RAILS_ENV"] = old_env - end - - def simple_controller - controller :foo, <<-RUBY - class FooController < ApplicationController - def index - render :text => "foo" - end - end - RUBY - - app_file 'config/routes.rb', <<-RUBY - AppTemplate::Application.routes.draw do - match ':controller(/:action)' - end - RUBY - end - test "rails/info/properties in development" do app("development") get "/rails/info/properties" @@ -58,21 +30,6 @@ def index assert_equal 'foo', last_response.body end - test "simple controller in production mode returns best standards" do - simple_controller - - get '/foo' - assert_equal "IE=Edge,chrome=1", last_response.headers["X-UA-Compatible"] - end - - test "simple controller in development mode leaves out Chrome" do - simple_controller - app("development") - - get "/foo" - assert_equal "IE=Edge", last_response.headers["X-UA-Compatible"] - end - test "simple controller with helper" do controller :foo, <<-RUBY class FooController < ApplicationController @@ -177,7 +134,7 @@ def index assert_equal 'admin::foo', last_response.body end - def test_reloads_appended_route_blocks + test "routes appending blocks" do app_file 'config/routes.rb', <<-RUBY AppTemplate::Application.routes.draw do match ':controller#:action' @@ -246,10 +203,13 @@ def baz test 'routes are loaded just after initialization' do require "#{app_path}/config/application" - app_file 'config/routes.rb', <<-RUBY - InitializeRackApp = lambda { |env| [200, {}, ["InitializeRackApp"]] } + # Create the rack app just inside after initialize callback + ActiveSupport.on_load(:after_initialize) do + ::InitializeRackApp = lambda { |env| [200, {}, ["InitializeRackApp"]] } + end - AppTemplate::Application.routes.draw do + app_file 'config/routes.rb', <<-RUBY + AppTemplate::Application.routes.draw do |map| match 'foo', :to => ::InitializeRackApp end RUBY @@ -258,7 +218,7 @@ def baz assert_equal "InitializeRackApp", last_response.body end - test 'resource routing with irrigular inflection' do + test 'resource routing with irregular inflection' do app_file 'config/initializers/inflection.rb', <<-RUBY ActiveSupport::Inflector.inflections do |inflect| inflect.irregular 'yazi', 'yazilar' diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 79c77350195aa..3b03e4eb3dbcc 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -45,6 +45,17 @@ def rails_root end module Rack + def app(env = "production") + old_env = ENV["RAILS_ENV"] + @app ||= begin + ENV["RAILS_ENV"] = env + require "#{app_path}/config/environment" + Rails.application + end + ensure + ENV["RAILS_ENV"] = old_env + end + def extract_body(response) "".tap do |body| response[2].each {|chunk| body << chunk } @@ -124,6 +135,22 @@ def make_basic_app extend ::Rack::Test::Methods end + def simple_controller + controller :foo, <<-RUBY + class FooController < ApplicationController + def index + render :text => "foo" + end + end + RUBY + + app_file 'config/routes.rb', <<-RUBY + AppTemplate::Application.routes.draw do + match ':controller(/:action)' + end + RUBY + end + class Bukkit attr_reader :path From 7b0c592e38e4b1d370e04d856d245f825dfa9cfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 2 Oct 2010 17:45:26 +0200 Subject: [PATCH 23/85] reload_routes! is part of the public API and should not be removed. --- railties/lib/rails/application.rb | 4 ++++ railties/lib/rails/application/routes_reloader.rb | 6 ++++-- railties/test/application/routing_test.rb | 9 ++++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index aafbbc29ee3e7..4d04184b2008a 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -81,6 +81,10 @@ def eager_load! #:nodoc: super end + def reload_routes! + routes_reloader.reload! + end + def routes_reloader @routes_reloader ||= RoutesReloader.new end diff --git a/railties/lib/rails/application/routes_reloader.rb b/railties/lib/rails/application/routes_reloader.rb index 23b72a0ec6bcc..6da903c1acd0c 100644 --- a/railties/lib/rails/application/routes_reloader.rb +++ b/railties/lib/rails/application/routes_reloader.rb @@ -8,7 +8,7 @@ def initialize def blocks @blocks ||= {} end - private + def reload! clear! load_blocks @@ -18,6 +18,8 @@ def reload! revert end + protected + def clear! routers.each do |routes| routes.disable_clear_and_finalize = true @@ -32,7 +34,7 @@ def load_blocks end def load_paths - paths.each { |path| load(path) } + paths.each { |path| load(path) } end def finalize! diff --git a/railties/test/application/routing_test.rb b/railties/test/application/routing_test.rb index e42f5720a9df8..62589c998d9f0 100644 --- a/railties/test/application/routing_test.rb +++ b/railties/test/application/routing_test.rb @@ -209,7 +209,7 @@ def baz end app_file 'config/routes.rb', <<-RUBY - AppTemplate::Application.routes.draw do |map| + AppTemplate::Application.routes.draw do match 'foo', :to => ::InitializeRackApp end RUBY @@ -218,6 +218,13 @@ def baz assert_equal "InitializeRackApp", last_response.body end + test 'reload_routes! is part of Rails.application API' do + app("development") + assert_nothing_raised do + Rails.application.reload_routes! + end + end + test 'resource routing with irregular inflection' do app_file 'config/initializers/inflection.rb', <<-RUBY ActiveSupport::Inflector.inflections do |inflect| From 8d1df887d32643f0aee2d71f7216ec98fdfe4fdb Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Wed, 29 Sep 2010 18:15:36 +0530 Subject: [PATCH 24/85] Fixing search_field to remove object attribute from options hash [#5730 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_view/helpers/form_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 1836baaf1275f..3cd8b02bc487b 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -791,7 +791,7 @@ def search_field(object_name, method, options = {}) options["incremental"] = true unless options.has_key?("incremental") end - InstanceTag.new(object_name, method, self, options.delete(:object)).to_input_field_tag("search", options) + InstanceTag.new(object_name, method, self, options.delete("object")).to_input_field_tag("search", options) end # Returns a text_field of type "tel". From 297cf0b26658ff9c7f19fc703de5bd8939e31d06 Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Sat, 2 Oct 2010 17:39:12 +0530 Subject: [PATCH 25/85] added test for form_for with search_field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/test/template/form_helper_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index abc98ebe69f8d..8809e510fb33b 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -761,6 +761,20 @@ def test_form_for_with_method assert_dom_equal expected, output_buffer end + + def test_form_for_with_search_field + # Test case for bug which would emit an "object" attribute + # when used with form_for using a search_field form helper + form_for(Post.new, :url => "/search", :html => { :id => 'search-post' }) do |f| + concat f.search_field(:title) + end + + expected = whole_form("/search", "search-post", "new_post") do + "" + end + + assert_dom_equal expected, output_buffer + end def test_form_for_with_remote form_for(@post, :url => '/', :remote => true, :html => { :id => 'create-post', :method => :put }) do |f| @@ -1737,4 +1751,5 @@ def test_fields_for_returns_block_result def protect_against_forgery? false end + end From 757bbd540cadc47b8f9d6b9d6bc7bb3cb638d022 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 2 Oct 2010 18:30:38 +0200 Subject: [PATCH 26/85] :'' is not valid ruby. --- railties/test/generators/generated_attribute_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/test/generators/generated_attribute_test.rb b/railties/test/generators/generated_attribute_test.rb index 272e179fe37b3..064546a3f3b87 100644 --- a/railties/test/generators/generated_attribute_test.rb +++ b/railties/test/generators/generated_attribute_test.rb @@ -117,7 +117,7 @@ def test_nil_type_raises_exception def test_missing_type_raises_exception assert_raise Thor::Error do - create_generated_attribute(:'', 'title') + create_generated_attribute('', 'title') end end end From 04cbabb0a0206553d7b474ee7a191d19957048fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 2 Oct 2010 18:38:23 +0200 Subject: [PATCH 27/85] Deprecate generators in Railties. You should use app_generators instead. --- railties/lib/rails/engine/configuration.rb | 28 ++++++++++++++++-- railties/lib/rails/railtie.rb | 2 +- railties/lib/rails/railtie/configuration.rb | 32 ++++----------------- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/railties/lib/rails/engine/configuration.rb b/railties/lib/rails/engine/configuration.rb index d4d87be527266..b69c0e1c533b6 100644 --- a/railties/lib/rails/engine/configuration.rb +++ b/railties/lib/rails/engine/configuration.rb @@ -4,16 +4,38 @@ module Rails class Engine class Configuration < ::Rails::Railtie::Configuration attr_reader :root - attr_writer :eager_load_paths, :autoload_once_paths, :autoload_paths - attr_accessor :middleware, :plugins, :asset_path + attr_writer :middleware, :eager_load_paths, :autoload_once_paths, :autoload_paths + attr_accessor :plugins, :asset_path def initialize(root=nil) super() @root = root - @middleware = Rails::Configuration::MiddlewareStackProxy.new @helpers_paths = [] end + # Returns the middleware stack for the engine. + def middleware + @middleware ||= Rails::Configuration::MiddlewareStackProxy.new + end + + # Holds generators configuration: + # + # config.generators do |g| + # g.orm :datamapper, :migration => true + # g.template_engine :haml + # g.test_framework :rspec + # end + # + # If you want to disable color in console, do: + # + # config.generators.colorize_logging = false + # + def generators #:nodoc + @generators ||= Rails::Configuration::Generators.new + yield(@generators) if block_given? + @generators + end + def paths @paths ||= begin paths = Rails::Paths::Root.new(@root) diff --git a/railties/lib/rails/railtie.rb b/railties/lib/rails/railtie.rb index 09650789acae7..2b68a3c45397b 100644 --- a/railties/lib/rails/railtie.rb +++ b/railties/lib/rails/railtie.rb @@ -83,7 +83,7 @@ module Rails # # class MyRailtie < Rails::Railtie # # Customize the ORM - # config.generators.orm :my_railtie_orm + # config.app_generators.orm :my_railtie_orm # # # Add a to_prepare block which is executed once in production # # and before each request in development diff --git a/railties/lib/rails/railtie/configuration.rb b/railties/lib/rails/railtie/configuration.rb index e0e4324a4a8e5..3d0af185a2d4a 100644 --- a/railties/lib/rails/railtie/configuration.rb +++ b/railties/lib/rails/railtie/configuration.rb @@ -5,7 +5,6 @@ class Railtie class Configuration def initialize @@options ||= {} - @@static_asset_paths = ActiveSupport::OrderedHash.new end # This allows you to modify the application's middlewares from Engines. @@ -23,32 +22,13 @@ def app_middleware # application overwrites them. def app_generators @@app_generators ||= Rails::Configuration::Generators.new - if block_given? - yield @@app_generators - else - @@app_generators - end + yield(@@app_generators) if block_given? + @@app_generators end - # Holds generators configuration: - # - # config.generators do |g| - # g.orm :datamapper, :migration => true - # g.template_engine :haml - # g.test_framework :rspec - # end - # - # If you want to disable color in console, do: - # - # config.generators.colorize_logging = false - # - def generators - @generators ||= Rails::Configuration::Generators.new - if block_given? - yield @generators - else - @generators - end + def generators(&block) #:nodoc + ActiveSupport::Deprecation.warn "config.generators is deprecated. Please use config.app_generators instead." + app_generators(&block) end def before_configuration(&block) @@ -83,7 +63,7 @@ def respond_to?(name) # with associated public folders, like: # { "/" => "/app/public", "/my_engine" => "app/engines/my_engine/public" } def static_asset_paths - @@static_asset_paths + @@static_asset_paths ||= ActiveSupport::OrderedHash.new end private From 49cc01002e82208596439bb94d04805b85b75d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 2 Oct 2010 09:40:54 -0700 Subject: [PATCH 28/85] Be more explicit about what is deprecated. --- railties/lib/rails/railtie/configuration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/railtie/configuration.rb b/railties/lib/rails/railtie/configuration.rb index 3d0af185a2d4a..afeceafb67e83 100644 --- a/railties/lib/rails/railtie/configuration.rb +++ b/railties/lib/rails/railtie/configuration.rb @@ -27,7 +27,7 @@ def app_generators end def generators(&block) #:nodoc - ActiveSupport::Deprecation.warn "config.generators is deprecated. Please use config.app_generators instead." + ActiveSupport::Deprecation.warn "config.generators in Rails::Railtie is deprecated. Please use config.app_generators instead." app_generators(&block) end From f656796d05715174568536cfe119a3959a020f23 Mon Sep 17 00:00:00 2001 From: David Chelimsky Date: Sat, 2 Oct 2010 12:35:17 -0500 Subject: [PATCH 29/85] Rename _assigns to view_assigns in AV::TC - also add tests - also deprecate _assigns [#5751 state:resolved] Signed-off-by: Santiago Pastorino --- actionpack/lib/action_view/test_case.rb | 30 ++++++++++++++------- actionpack/test/template/test_case_test.rb | 31 ++++++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 0eb4a663de35f..ac59c16d7c10f 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -103,7 +103,7 @@ def config end def render(options = {}, local_assigns = {}, &block) - view.assign(_assigns) + view.assign(view_assigns) @rendered << output = view.render(options, local_assigns, &block) output end @@ -169,15 +169,19 @@ def view alias_method :_view, :view - EXCLUDE_IVARS = %w{ + INTERNAL_IVARS = %w{ + @__name__ @_assertion_wrapped + @_assertions @_result + @_routes @controller @layouts @locals @method_name @output_buffer @partials + @passed @rendered @request @routes @@ -187,18 +191,26 @@ def view @view_context_class } - def _instance_variables - instance_variables.map(&:to_s) - EXCLUDE_IVARS + def _user_defined_ivars + instance_variables.map(&:to_s) - INTERNAL_IVARS end - def _assigns - _instance_variables.inject({}) do |hash, var| - name = var[1..-1].to_sym - hash[name] = instance_variable_get(var) - hash + # Returns a Hash of instance variables and their values, as defined by + # the user in the test case, which are then assigned to the view being + # rendered. This is generally intended for internal use and extension + # frameworks. + def view_assigns + _user_defined_ivars.inject({}) do |hash, var| + hash.merge(var.sub('@','').to_sym => instance_variable_get(var)) end end + def _assigns + ActiveSupport::Deprecation.warn "ActionView::TestCase#_assigns is deprecated and will be removed in future versions. " << + "Please use view_assigns instead." + view_assigns + end + def _routes @controller._routes if @controller.respond_to?(:_routes) end diff --git a/actionpack/test/template/test_case_test.rb b/actionpack/test/template/test_case_test.rb index 8526db61cc798..a74599962291d 100644 --- a/actionpack/test/template/test_case_test.rb +++ b/actionpack/test/template/test_case_test.rb @@ -116,6 +116,37 @@ def render_from_helper end end + class AssignsTest < ActionView::TestCase + setup do + ActiveSupport::Deprecation.stubs(:warn) + end + + test "_assigns delegates to user_defined_ivars" do + self.expects(:view_assigns) + _assigns + end + + test "_assigns is deprecated" do + ActiveSupport::Deprecation.expects(:warn) + _assigns + end + end + + class ViewAssignsTest < ActionView::TestCase + test "view_assigns returns a Hash of user defined ivars" do + @a = 'b' + @c = 'd' + assert_equal({:a => 'b', :c => 'd'}, view_assigns) + end + + test "view_assigns excludes internal ivars" do + INTERNAL_IVARS.each do |ivar| + assert defined?(ivar), "expected #{ivar} to be defined" + assert !view_assigns.keys.include?(ivar.sub('@','').to_sym), "expected #{ivar} to be excluded from view_assigns" + end + end + end + class HelperExposureTest < ActionView::TestCase helper(Module.new do def render_from_helper From c28bebef13b8a0e497fc7bbb83f542e9400e07e5 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 13:34:34 -0200 Subject: [PATCH 30/85] PERF: Hash[] + map is faster than this silly inject, and var[1..-1] is faster than var.sub('@', '') --- actionpack/lib/action_view/test_case.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index ac59c16d7c10f..731f91df308b9 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -200,9 +200,9 @@ def _user_defined_ivars # rendered. This is generally intended for internal use and extension # frameworks. def view_assigns - _user_defined_ivars.inject({}) do |hash, var| - hash.merge(var.sub('@','').to_sym => instance_variable_get(var)) - end + Hash[_user_defined_ivars.map do |var| + [var[1..-1].to_sym, instance_variable_get(var)] + end] end def _assigns From 10249d7b7728525fb7660409e3bbc9e0286dbf0f Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 14:52:46 -0200 Subject: [PATCH 31/85] PERF: change inject({}) with Hash + map --- actionmailer/lib/action_mailer/old_api.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionmailer/lib/action_mailer/old_api.rb b/actionmailer/lib/action_mailer/old_api.rb index b8c15df263fce..a9cdf6c0caee3 100644 --- a/actionmailer/lib/action_mailer/old_api.rb +++ b/actionmailer/lib/action_mailer/old_api.rb @@ -247,7 +247,7 @@ def parse_content_type(defaults=nil) [ nil, {} ] else ctype, *attrs = @content_type.split(/;\s*/) - attrs = attrs.inject({}) { |h,s| k,v = s.split(/\=/, 2); h[k] = v; h } + attrs = Hash[attrs.map { |attr| attr.split(/\=/, 2) }] [ctype, {"charset" => @charset}.merge(attrs)] end end From 49203126430c6dbaf9ce47c21c58c4081fb53e82 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 14:58:18 -0200 Subject: [PATCH 32/85] PERF: Don't create unnecessary objects --- actionmailer/lib/action_mailer/old_api.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionmailer/lib/action_mailer/old_api.rb b/actionmailer/lib/action_mailer/old_api.rb index a9cdf6c0caee3..0396f0775e49d 100644 --- a/actionmailer/lib/action_mailer/old_api.rb +++ b/actionmailer/lib/action_mailer/old_api.rb @@ -248,7 +248,7 @@ def parse_content_type(defaults=nil) else ctype, *attrs = @content_type.split(/;\s*/) attrs = Hash[attrs.map { |attr| attr.split(/\=/, 2) }] - [ctype, {"charset" => @charset}.merge(attrs)] + [ctype, {"charset" => @charset}.merge!(attrs)] end end end From 42fad8c82b3ba6f86c84f06645cc1e39a8a776dd Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 15:03:59 -0200 Subject: [PATCH 33/85] PERF: more changes from inject({}) to Hash + map --- activeresource/lib/active_resource/validations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activeresource/lib/active_resource/validations.rb b/activeresource/lib/active_resource/validations.rb index d3b19ee560bd1..a373e53f112bd 100644 --- a/activeresource/lib/active_resource/validations.rb +++ b/activeresource/lib/active_resource/validations.rb @@ -13,7 +13,7 @@ class Errors < ActiveModel::Errors # or not (by passing true) def from_array(messages, save_cache = false) clear unless save_cache - humanized_attributes = @base.attributes.keys.inject({}) { |h, attr_name| h.update(attr_name.humanize => attr_name) } + humanized_attributes = Hash[@base.attributes.keys.map { |attr_name| [attr_name.humanize, attr_name] }] messages.each do |message| attr_message = humanized_attributes.keys.detect do |attr_name| if message[0, attr_name.size + 1] == "#{attr_name} " From 5836af8f8b0eb3c569c66792abf50a0485bb6f22 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 16:33:46 -0200 Subject: [PATCH 34/85] PERF: more Hash + map changes --- .../active_support/core_ext/class/inheritable_attributes.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/class/inheritable_attributes.rb b/activesupport/lib/active_support/core_ext/class/inheritable_attributes.rb index 7a6a0be69d182..af30bfc13a840 100644 --- a/activesupport/lib/active_support/core_ext/class/inheritable_attributes.rb +++ b/activesupport/lib/active_support/core_ext/class/inheritable_attributes.rb @@ -158,9 +158,9 @@ def inherited_with_inheritable_attributes(child) if inheritable_attributes.equal?(EMPTY_INHERITABLE_ATTRIBUTES) new_inheritable_attributes = EMPTY_INHERITABLE_ATTRIBUTES else - new_inheritable_attributes = inheritable_attributes.inject({}) do |memo, (key, value)| - memo.update(key => value.duplicable? ? value.dup : value) - end + new_inheritable_attributes = Hash[inheritable_attributes.map do |(key, value)| + [key, value.duplicable? ? value.dup : value] + end] end child.instance_variable_set('@inheritable_attributes', new_inheritable_attributes) From 50215f9525b6b5e3bfe703724b9f68177ed8565d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 20 Sep 2010 10:18:44 +0200 Subject: [PATCH 35/85] Rely on Rack::Session stores API for more compatibility across the Ruby world. --- Gemfile | 1 + actionpack/CHANGELOG | 6 +- actionpack/lib/action_controller/test_case.rb | 8 +- actionpack/lib/action_dispatch/http/url.rb | 5 - .../middleware/session/abstract_store.rb | 280 +++--------------- .../middleware/session/cookie_store.rb | 64 ++-- .../middleware/session/mem_cache_store.rb | 53 +--- .../dispatch/session/cookie_store_test.rb | 12 - .../lib/active_record/session_store.rb | 7 +- 9 files changed, 90 insertions(+), 346 deletions(-) diff --git a/Gemfile b/Gemfile index 9d29c42e1c6d5..7fe1c3ad2210f 100644 --- a/Gemfile +++ b/Gemfile @@ -6,6 +6,7 @@ else gem "arel", :git => "git://github.com/rails/arel.git" end +gem "rack", :git => "git://github.com/rack/rack.git" gem "rails", :path => File.dirname(__FILE__) gem "rake", ">= 0.8.7" diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 6f8314109b3dd..6352b97a6ba16 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,12 +1,12 @@ *Rails 3.1.0 (unreleased)* +* Rely on Rack::Session stores API for more compatibility across the Ruby world. This is backwards incompatible since Rack::Session expects #get_session to accept 4 arguments and requires #destroy_session instead of simply #destroy. [José Valim] + * file_field automatically adds :multipart => true to the enclosing form. [Santiago Pastorino] * Renames csrf_meta_tag -> csrf_meta_tags, and aliases csrf_meta_tag for backwards compatibility. [fxn] -* Add Rack::Cache to the default stack. Create a Rails store that delegates to the Rails cache, so by default, whatever caching layer you are using will be used - for HTTP caching. Note that Rack::Cache will be used if you use #expires_in, #fresh_when or #stale with :public => true. Otherwise, the caching rules will apply - to the browser only. +* Add Rack::Cache to the default stack. Create a Rails store that delegates to the Rails cache, so by default, whatever caching layer you are using will be used for HTTP caching. Note that Rack::Cache will be used if you use #expires_in, #fresh_when or #stale with :public => true. Otherwise, the caching rules will apply to the browser only. [Yehuda Katz, Carl Lerche] *Rails 3.0.0 (August 29, 2010)* diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 1af75fc2d7c33..d7b54c2abc298 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -187,15 +187,17 @@ def recycle! end end - class TestSession < ActionDispatch::Session::AbstractStore::SessionHash #:nodoc: - DEFAULT_OPTIONS = ActionDispatch::Session::AbstractStore::DEFAULT_OPTIONS + class TestSession < Rack::Session::Abstract::SessionHash #:nodoc: + DEFAULT_OPTIONS = Rack::Session::Abstract::ID::DEFAULT_OPTIONS def initialize(session = {}) replace(session.stringify_keys) @loaded = true end - def exists?; true; end + def exists? + true + end end # Superclass for ActionController functional tests. Functional tests allow you to diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 3e5cd6a2f9c4a..cfee95eb4b66c 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -18,11 +18,6 @@ def protocol @protocol ||= ssl? ? 'https://' : 'http://' end - # Is this an SSL request? - def ssl? - @ssl ||= @env['HTTPS'] == 'on' || @env['HTTP_X_FORWARDED_PROTO'] == 'https' - end - # Returns the \host for this request, such as "example.com". def raw_host_with_port if forwarded = env["HTTP_X_FORWARDED_HOST"] diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index db0187c015991..679ba7fc8ed30 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -1,5 +1,6 @@ require 'rack/utils' require 'rack/request' +require 'rack/session/abstract/id' require 'action_dispatch/middleware/cookies' require 'active_support/core_ext/object/blank' @@ -8,252 +9,69 @@ module Session class SessionRestoreError < StandardError #:nodoc: end - class AbstractStore - ENV_SESSION_KEY = 'rack.session'.freeze - ENV_SESSION_OPTIONS_KEY = 'rack.session.options'.freeze - - # thin wrapper around Hash that allows us to lazily - # load session id into session_options - class OptionsHash < Hash - def initialize(by, env, default_options) - @by = by - @env = env - @session_id_loaded = false - merge!(default_options) - end - - def [](key) - if key == :id - load_session_id! unless key?(:id) || has_session_id? - end - super - end - - private - - def has_session_id? - @session_id_loaded - end - - def load_session_id! - self[:id] = @by.send(:extract_session_id, @env) - @session_id_loaded = true - end - end - - class SessionHash < Hash - def initialize(by, env) - super() - @by = by - @env = env - @loaded = false - end - - def [](key) - load_for_read! - super(key.to_s) - end - - def has_key?(key) - load_for_read! - super(key.to_s) - end - - def []=(key, value) - load_for_write! - super(key.to_s, value) - end - - def clear - load_for_write! - super - end - - def to_hash - load_for_read! - h = {}.replace(self) - h.delete_if { |k,v| v.nil? } - h - end - - def update(hash) - load_for_write! - super(hash.stringify_keys) - end - - def delete(key) - load_for_write! - super(key.to_s) - end - - def inspect - load_for_read! - super - end - - def exists? - return @exists if instance_variable_defined?(:@exists) - @exists = @by.send(:exists?, @env) - end - - def loaded? - @loaded - end - - def destroy - clear - @by.send(:destroy, @env) if defined?(@by) && @by - @env[ENV_SESSION_OPTIONS_KEY][:id] = nil if defined?(@env) && @env && @env[ENV_SESSION_OPTIONS_KEY] - @loaded = false - end - - private - - def load_for_read! - load! if !loaded? && exists? - end - - def load_for_write! - load! unless loaded? - end - - def load! - id, session = @by.send(:load_session, @env) - @env[ENV_SESSION_OPTIONS_KEY][:id] = id - replace(session.stringify_keys) - @loaded = true - end - + module DestroyableSession + def destroy + clear + options = @env[Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY] if @env + options ||= {} + @by.send(:destroy_session, @env, options[:id], options) if @by + options[:id] = nil + @loaded = false end + end - DEFAULT_OPTIONS = { - :key => '_session_id', - :path => '/', - :domain => nil, - :expire_after => nil, - :secure => false, - :httponly => true, - :cookie_only => true - } + ::Rack::Session::Abstract::SessionHash.send :include, DestroyableSession + module Compatibility def initialize(app, options = {}) - @app = app - @default_options = DEFAULT_OPTIONS.merge(options) - @key = @default_options.delete(:key).freeze - @cookie_only = @default_options.delete(:cookie_only) - ensure_session_key! + options[:key] ||= '_session_id' + super end - def call(env) - prepare!(env) - response = @app.call(env) - - session_data = env[ENV_SESSION_KEY] - options = env[ENV_SESSION_OPTIONS_KEY] - - if !session_data.is_a?(AbstractStore::SessionHash) || session_data.loaded? || options[:expire_after] - request = ActionDispatch::Request.new(env) - - return response if (options[:secure] && !request.ssl?) - - session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.loaded? - - sid = options[:id] || generate_sid - session_data = session_data.to_hash - - value = set_session(env, sid, session_data) - return response unless value - - cookie = { :value => value } - if options[:expire_after] - cookie[:expires] = Time.now + options.delete(:expire_after) - end - - set_cookie(request, cookie.merge!(options)) - end - - response + def generate_sid + ActiveSupport::SecureRandom.hex(16) end + end - private - - def prepare!(env) - env[ENV_SESSION_KEY] = SessionHash.new(self, env) - env[ENV_SESSION_OPTIONS_KEY] = OptionsHash.new(self, env, @default_options) - end - - def generate_sid - ActiveSupport::SecureRandom.hex(16) - end - - def set_cookie(request, options) - if request.cookie_jar[@key] != options[:value] || !options[:expires].nil? - request.cookie_jar[@key] = options - end - end - - def load_session(env) - stale_session_check! do - sid = current_session_id(env) - sid, session = get_session(env, sid) - [sid, session] - end - end - - def extract_session_id(env) - stale_session_check! do - request = ActionDispatch::Request.new(env) - sid = request.cookies[@key] - sid ||= request.params[@key] unless @cookie_only - sid - end - end - - def current_session_id(env) - env[ENV_SESSION_OPTIONS_KEY][:id] - end + module StaleSessionCheck + def load_session(env) + stale_session_check! { super } + end - def ensure_session_key! - if @key.blank? - raise ArgumentError, 'A key is required to write a ' + - 'cookie containing the session data. Use ' + - 'config.session_store SESSION_STORE, { :key => ' + - '"_myapp_session" } in config/application.rb' - end - end + def extract_session_id(env) + stale_session_check! { super } + end - def stale_session_check! - yield - rescue ArgumentError => argument_error - if argument_error.message =~ %r{undefined class/module ([\w:]*\w)} - begin - # Note that the regexp does not allow $1 to end with a ':' - $1.constantize - rescue LoadError, NameError => const_error - raise ActionDispatch::Session::SessionRestoreError, "Session contains objects whose class definition isn't available.\nRemember to require the classes for all objects kept in the session.\n(Original exception: #{const_error.message} [#{const_error.class}])\n" - end - retry - else - raise + def stale_session_check! + yield + rescue ArgumentError => argument_error + if argument_error.message =~ %r{undefined class/module ([\w:]*\w)} + begin + # Note that the regexp does not allow $1 to end with a ':' + $1.constantize + rescue LoadError, NameError => const_error + raise ActionDispatch::Session::SessionRestoreError, "Session contains objects whose class definition isn't available.\nRemember to require the classes for all objects kept in the session.\n(Original exception: #{const_error.message} [#{const_error.class}])\n" end + retry + else + raise end + end + end - def exists?(env) - current_session_id(env).present? - end - - def get_session(env, sid) - raise '#get_session needs to be implemented.' - end + class AbstractStore < Rack::Session::Abstract::ID + include Compatibility + include StaleSessionCheck - def set_session(env, sid, session_data) - raise '#set_session needs to be implemented and should return ' << - 'the value to be stored in the cookie (usually the sid)' - end + def destroy_session(env, sid, options) + ActiveSupport::Deprecation.warn "Implementing #destroy in session stores is deprecated. " << + "Please implement destroy_session(env, session_id, options) instead." + destroy(env) + end - def destroy(env) - raise '#destroy needs to be implemented.' - end + def destroy(env) + raise '#destroy needs to be implemented.' + end end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index ca1494425f5ac..9c9ccc62f56a9 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -1,5 +1,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/object/blank' +require 'action_dispatch/middleware/session/abstract_store' +require 'rack/session/cookie' module ActionDispatch module Session @@ -38,58 +40,32 @@ module Session # "rake secret" and set the key in config/initializers/secret_token.rb. # # Note that changing digest or secret invalidates all existing sessions! - class CookieStore < AbstractStore - - def initialize(app, options = {}) - super(app, options.merge!(:cookie_only => true)) - freeze - end + class CookieStore < Rack::Session::Cookie + include Compatibility + include StaleSessionCheck private - def load_session(env) - data = unpacked_cookie_data(env) - data = persistent_session_id!(data) - [data["session_id"], data] - end - - def extract_session_id(env) - if data = unpacked_cookie_data(env) - data["session_id"] - else - nil - end - end - - def unpacked_cookie_data(env) - env["action_dispatch.request.unsigned_session_cookie"] ||= begin - stale_session_check! do - request = ActionDispatch::Request.new(env) - if data = request.cookie_jar.signed[@key] - data.stringify_keys! - end - data || {} + def unpacked_cookie_data(env) + env["action_dispatch.request.unsigned_session_cookie"] ||= begin + stale_session_check! do + request = ActionDispatch::Request.new(env) + if data = request.cookie_jar.signed[@key] + data.stringify_keys! end + data || {} end end + end - def set_cookie(request, options) - request.cookie_jar.signed[@key] = options - end - - def set_session(env, sid, session_data) - persistent_session_id!(session_data, sid) - end - - def destroy(env) - # session data is stored on client; nothing to do here - end + def set_session(env, sid, session_data, options) + persistent_session_id!(session_data, sid) + end - def persistent_session_id!(data, sid=nil) - data ||= {} - data["session_id"] ||= sid || generate_sid - data - end + def set_cookie(env, session_id, cookie) + request = ActionDispatch::Request.new(env) + request.cookie_jar.signed[@key] = cookie + end end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb b/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb index 28e3dbd732aac..4dd9a946c2e60 100644 --- a/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb @@ -1,56 +1,17 @@ +require 'action_dispatch/middleware/session/abstract_store' +require 'rack/session/memcache' + module ActionDispatch module Session - class MemCacheStore < AbstractStore + class MemCacheStore < Rack::Session::Memcache + include Compatibility + include StaleSessionCheck + def initialize(app, options = {}) require 'memcache' - - # Support old :expires option options[:expire_after] ||= options[:expires] - - super - - @default_options = { - :namespace => 'rack:session', - :memcache_server => 'localhost:11211' - }.merge(@default_options) - - @pool = options[:cache] || MemCache.new(@default_options[:memcache_server], @default_options) - unless @pool.servers.any? { |s| s.alive? } - raise "#{self} unable to find server during initialization." - end - @mutex = Mutex.new - super end - - private - def get_session(env, sid) - sid ||= generate_sid - begin - session = @pool.get(sid) || {} - rescue MemCache::MemCacheError, Errno::ECONNREFUSED - session = {} - end - [sid, session] - end - - def set_session(env, sid, session_data) - options = env['rack.session.options'] - expiry = options[:expire_after] || 0 - @pool.set(sid, session_data, expiry) - sid - rescue MemCache::MemCacheError, Errno::ECONNREFUSED - false - end - - def destroy(env) - if sid = current_session_id(env) - @pool.delete(sid) - end - rescue MemCache::MemCacheError, Errno::ECONNREFUSED - false - end - end end end diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 3489f628ede0a..256d0781c7e20 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -53,18 +53,6 @@ def change_session_id def rescue_action(e) raise end end - def test_raises_argument_error_if_missing_session_key - assert_raise(ArgumentError, nil.inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => nil, :secret => SessionSecret) - } - - assert_raise(ArgumentError, ''.inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => '', :secret => SessionSecret) - } - end - def test_setting_session_value with_test_route_set do get '/set_session_value' diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb index 01cc14b8d6fee..3fc596e02a0ce 100644 --- a/activerecord/lib/active_record/session_store.rb +++ b/activerecord/lib/active_record/session_store.rb @@ -288,6 +288,7 @@ def destroy self.session_class = Session SESSION_RECORD_KEY = 'rack.session.record' + ENV_SESSION_OPTIONS_KEY = Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY private def get_session(env, sid) @@ -299,7 +300,7 @@ def get_session(env, sid) end end - def set_session(env, sid, session_data) + def set_session(env, sid, session_data, options) Base.silence do record = get_session_model(env, sid) record.data = session_data @@ -316,12 +317,14 @@ def set_session(env, sid, session_data) sid end - def destroy(env) + def destroy_session(env, session_id, options) if sid = current_session_id(env) Base.silence do get_session_model(env, sid).destroy end end + + generate_sid unless options[:drop] end def get_session_model(env, sid) From 74dd8a3681c6984ea35c879f88c6a87521b58ec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 22 Sep 2010 21:40:14 +0200 Subject: [PATCH 36/85] Move ETag and ConditionalGet logic from AD::Response to the middleware stack. --- actionpack/lib/action_dispatch/http/cache.rb | 22 +--- .../lib/action_dispatch/http/response.rb | 2 +- .../test/controller/new_base/etag_test.rb | 46 ------- actionpack/test/controller/render_test.rb | 118 ------------------ actionpack/test/dispatch/response_test.rb | 11 +- railties/lib/rails/application.rb | 4 +- railties/test/application/middleware_test.rb | 64 ++++++---- 7 files changed, 51 insertions(+), 216 deletions(-) delete mode 100644 actionpack/test/controller/new_base/etag_test.rb diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 047fab006e78c..4061222d11676 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -50,8 +50,7 @@ def initialize(*) if cache_control = self["Cache-Control"] cache_control.split(/,\s*/).each do |segment| first, last = segment.split("=") - last ||= true - @cache_control[first.to_sym] = last + @cache_control[first.to_sym] = last || true end end end @@ -88,28 +87,9 @@ def etag=(etag) def handle_conditional_get! if etag? || last_modified? || !@cache_control.empty? set_conditional_cache_control! - elsif nonempty_ok_response? - self.etag = body - - if request && request.etag_matches?(etag) - self.status = 304 - self.body = [] - end - - set_conditional_cache_control! - else - headers["Cache-Control"] = "no-cache" end end - def nonempty_ok_response? - @status == 200 && string_body? - end - - def string_body? - !@blank && @body.respond_to?(:all?) && @body.all? { |part| part.is_a?(String) } - end - DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate" def set_conditional_cache_control! diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 151c90167b6d3..72871e328a207 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -132,7 +132,7 @@ def location=(url) # information. attr_accessor :charset, :content_type - CONTENT_TYPE = "Content-Type" + CONTENT_TYPE = "Content-Type" cattr_accessor(:default_charset) { "utf-8" } diff --git a/actionpack/test/controller/new_base/etag_test.rb b/actionpack/test/controller/new_base/etag_test.rb deleted file mode 100644 index 2bca5aec6a927..0000000000000 --- a/actionpack/test/controller/new_base/etag_test.rb +++ /dev/null @@ -1,46 +0,0 @@ -require 'abstract_unit' - -module Etags - class BasicController < ActionController::Base - self.view_paths = [ActionView::FixtureResolver.new( - "etags/basic/base.html.erb" => "Hello from without_layout.html.erb", - "layouts/etags.html.erb" => "teh <%= yield %> tagz" - )] - - def without_layout - render :action => "base" - end - - def with_layout - render :action => "base", :layout => "etags" - end - end - - class EtagTest < Rack::TestCase - describe "Rendering without any special etag options returns an etag that is an MD5 hash of its text" - - test "an action without a layout" do - get "/etags/basic/without_layout" - - body = "Hello from without_layout.html.erb" - assert_body body - assert_header "Etag", etag_for(body) - assert_status 200 - end - - test "an action with a layout" do - get "/etags/basic/with_layout" - - body = "teh Hello from without_layout.html.erb tagz" - assert_body body - assert_header "Etag", etag_for(body) - assert_status 200 - end - - private - - def etag_for(text) - %("#{Digest::MD5.hexdigest(text)}") - end - end -end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 7ca784c4675f8..fca8de60bc42a 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -99,11 +99,6 @@ def render_hello_world_with_last_modified_set render :template => "test/hello_world" end - def render_hello_world_with_etag_set - response.etag = "hello_world" - render :template => "test/hello_world" - end - # :ported: compatibility def render_hello_world_with_forward_slash render :template => "/test/hello_world" @@ -1386,119 +1381,6 @@ def test_expires_now end end - -class EtagRenderTest < ActionController::TestCase - tests TestController - - def setup - super - @request.host = "www.nextangle.com" - @expected_bang_etag = etag_for(expand_key([:foo, 123])) - end - - def test_render_blank_body_shouldnt_set_etag - get :blank_response - assert !@response.etag? - end - - def test_render_200_should_set_etag - get :render_hello_world_from_variable - assert_equal etag_for("hello david"), @response.headers['ETag'] - assert_equal "max-age=0, private, must-revalidate", @response.headers['Cache-Control'] - end - - def test_render_against_etag_request_should_304_when_match - @request.if_none_match = etag_for("hello david") - get :render_hello_world_from_variable - assert_equal 304, @response.status.to_i - assert @response.body.empty? - end - - def test_render_against_etag_request_should_have_no_content_length_when_match - @request.if_none_match = etag_for("hello david") - get :render_hello_world_from_variable - assert !@response.headers.has_key?("Content-Length") - end - - def test_render_against_etag_request_should_200_when_no_match - @request.if_none_match = etag_for("hello somewhere else") - get :render_hello_world_from_variable - assert_equal 200, @response.status.to_i - assert !@response.body.empty? - end - - def test_render_should_not_set_etag_when_last_modified_has_been_specified - get :render_hello_world_with_last_modified_set - assert_equal 200, @response.status.to_i - assert_not_nil @response.last_modified - assert_nil @response.etag - assert_present @response.body - end - - def test_render_with_etag - get :render_hello_world_from_variable - expected_etag = etag_for('hello david') - assert_equal expected_etag, @response.headers['ETag'] - @response = ActionController::TestResponse.new - - @request.if_none_match = expected_etag - get :render_hello_world_from_variable - assert_equal 304, @response.status.to_i - - @response = ActionController::TestResponse.new - @request.if_none_match = "\"diftag\"" - get :render_hello_world_from_variable - assert_equal 200, @response.status.to_i - end - - def render_with_404_shouldnt_have_etag - get :render_custom_code - assert_nil @response.headers['ETag'] - end - - def test_etag_should_not_be_changed_when_already_set - get :render_hello_world_with_etag_set - assert_equal etag_for("hello_world"), @response.headers['ETag'] - end - - def test_etag_should_govern_renders_with_layouts_too - get :builder_layout_test - assert_equal "\n\n

Hello

\n

This is grand!

\n\n\n", @response.body - assert_equal etag_for("\n\n

Hello

\n

This is grand!

\n\n\n"), @response.headers['ETag'] - end - - def test_etag_with_bang_should_set_etag - get :conditional_hello_with_bangs - assert_equal @expected_bang_etag, @response.headers["ETag"] - assert_response :success - end - - def test_etag_with_bang_should_obey_if_none_match - @request.if_none_match = @expected_bang_etag - get :conditional_hello_with_bangs - assert_response :not_modified - end - - def test_etag_with_public_true_should_set_header - get :conditional_hello_with_public_header - assert_equal "public", @response.headers['Cache-Control'] - end - - def test_etag_with_public_true_should_set_header_and_retain_other_headers - get :conditional_hello_with_public_header_and_expires_at - assert_equal "max-age=60, public", @response.headers['Cache-Control'] - end - - protected - def etag_for(text) - %("#{Digest::MD5.hexdigest(text)}") - end - - def expand_key(args) - ActiveSupport::Cache.expand_cache_key(args) - end -end - class LastModifiedRenderTest < ActionController::TestCase tests TestController diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index cd0418c338087..be6398feadaa6 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -11,9 +11,7 @@ def setup status, headers, body = @response.to_a assert_equal 200, status assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "max-age=0, private, must-revalidate", - "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"' + "Content-Type" => "text/html; charset=utf-8" }, headers) parts = [] @@ -27,9 +25,7 @@ def setup status, headers, body = @response.to_a assert_equal 200, status assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "max-age=0, private, must-revalidate", - "ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"' + "Content-Type" => "text/html; charset=utf-8" }, headers) end @@ -41,8 +37,7 @@ def setup status, headers, body = @response.to_a assert_equal 200, status assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "no-cache" + "Content-Type" => "text/html; charset=utf-8" }, headers) parts = [] diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 4d04184b2008a..075e3c5692fd8 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -145,8 +145,8 @@ def default_middleware_stack rack_cache = config.action_controller.perform_caching && config.action_dispatch.rack_cache require "action_dispatch/http/rack_cache" if rack_cache + middleware.use ::Rack::Cache, rack_cache if rack_cache - middleware.use ::Rack::Cache, rack_cache if rack_cache middleware.use ::ActionDispatch::Static, config.static_asset_paths if config.serve_static_assets middleware.use ::Rack::Lock if !config.allow_concurrency middleware.use ::Rack::Runtime @@ -165,6 +165,8 @@ def default_middleware_stack middleware.use ::ActionDispatch::ParamsParser middleware.use ::Rack::MethodOverride middleware.use ::ActionDispatch::Head + middleware.use ::Rack::ConditionalGet + middleware.use ::Rack::ETag, "no-cache" middleware.use ::ActionDispatch::BestStandardsSupport, config.action_dispatch.best_standards_support if config.action_dispatch.best_standards_support end end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 2372ad85b8ae2..173ac40b12245 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -36,6 +36,8 @@ def app "ActionDispatch::ParamsParser", "Rack::MethodOverride", "ActionDispatch::Head", + "Rack::ConditionalGet", + "Rack::ETag", "ActionDispatch::BestStandardsSupport" ], middleware end @@ -45,27 +47,7 @@ def app boot! - assert_equal [ - "Rack::Cache", - "ActionDispatch::Static", - "Rack::Lock", - "ActiveSupport::Cache::Strategy::LocalCache", - "Rack::Runtime", - "Rails::Rack::Logger", - "ActionDispatch::ShowExceptions", - "ActionDispatch::RemoteIp", - "Rack::Sendfile", - "ActionDispatch::Callbacks", - "ActiveRecord::ConnectionAdapters::ConnectionManagement", - "ActiveRecord::QueryCache", - "ActionDispatch::Cookies", - "ActionDispatch::Session::CookieStore", - "ActionDispatch::Flash", - "ActionDispatch::ParamsParser", - "Rack::MethodOverride", - "ActionDispatch::Head", - "ActionDispatch::BestStandardsSupport" - ], middleware + assert_equal "Rack::Cache", middleware.first end test "removing Active Record omits its middleware" do @@ -129,6 +111,46 @@ def app assert_equal "Rack::Config", middleware.first end + # ConditionalGet + Etag + test "conditional get + etag middlewares handle http caching based on body" do + make_basic_app + + class ::OmgController < ActionController::Base + def index + if params[:nothing] + render :text => "" + else + render :text => "OMG" + end + end + end + + etag = "5af83e3196bf99f440f31f2e1a6c9afe".inspect + + get "/" + assert_equal 200, last_response.status + assert_equal "OMG", last_response.body + assert_equal "text/html; charset=utf-8", last_response.headers["Content-Type"] + assert_equal "max-age=0, private, must-revalidate", last_response.headers["Cache-Control"] + assert_equal etag, last_response.headers["Etag"] + + get "/", {}, "HTTP_IF_NONE_MATCH" => etag + assert_equal 304, last_response.status + assert_equal "", last_response.body + assert_equal nil, last_response.headers["Content-Type"] + assert_equal "max-age=0, private, must-revalidate", last_response.headers["Cache-Control"] + assert_equal etag, last_response.headers["Etag"] + + get "/?nothing=true" + puts last_response.body + assert_equal 200, last_response.status + assert_equal "", last_response.body + assert_equal "text/html; charset=utf-8", last_response.headers["Content-Type"] + assert_equal "no-cache", last_response.headers["Cache-Control"] + assert_equal nil, last_response.headers["Etag"] + end + + # Show exceptions middleware test "show exceptions middleware filter backtrace before logging" do my_middleware = Struct.new(:app) do def call(env) From 653acac069e66f53b791caa4838a1e25de905f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 3 Oct 2010 21:45:27 +0200 Subject: [PATCH 37/85] Solve some warnings and a failing test. --- actionpack/lib/action_controller/test_case.rb | 1 + actionpack/lib/action_dispatch/http/request.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index d7b54c2abc298..6061945622850 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -191,6 +191,7 @@ class TestSession < Rack::Session::Abstract::SessionHash #:nodoc: DEFAULT_OPTIONS = Rack::Session::Abstract::ID::DEFAULT_OPTIONS def initialize(session = {}) + @env, @by = nil, nil replace(session.stringify_keys) @loaded = true end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 09d6ba82237ed..bbcdefb190ec1 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -199,7 +199,7 @@ def body_stream #:nodoc: # TODO This should be broken apart into AD::Request::Session and probably # be included by the session middleware. def reset_session - session.destroy if session + session.destroy if session && session.respond_to?(:destroy) self.session = {} @env['action_dispatch.request.flash_hash'] = nil end From adfd43a4daf08cc9a801a0b6a039dd109ce4e81f Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Thu, 30 Sep 2010 19:17:36 +0200 Subject: [PATCH 38/85] Add documentation on app_generators --- railties/lib/rails/engine.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 9ae235b81866a..ba89bce928869 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -47,6 +47,26 @@ module Rails # end # end # + # == Generators + # + # You can set up generators for engine with config.generators method: + # + # class MyEngine < Rails::Engine + # config.generators do |g| + # g.orm :active_record + # g.template_engine :erb + # g.test_framework :test_unit + # end + # end + # + # You can also set generators for application by using config.app_generators: + # + # class MyEngine < Rails::Engine + # # note that you can also pass block to app_generators in the same way you + # # can pass it to generators method + # config.app_generators.orm :datamapper + # end + # # == Paths # # Since Rails 3.0, both your Application and Engines do not have hardcoded paths. From 18a7b767e8ad545702c1025fc9cc7a1cc3c64f28 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 14:11:34 -0700 Subject: [PATCH 39/85] moving fake model to the correct file --- .../test/controller/record_identifier_test.rb | 13 ------------- actionpack/test/lib/controller/fake_models.rb | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/actionpack/test/controller/record_identifier_test.rb b/actionpack/test/controller/record_identifier_test.rb index e46e317e31b80..f3e5ff8a47aec 100644 --- a/actionpack/test/controller/record_identifier_test.rb +++ b/actionpack/test/controller/record_identifier_test.rb @@ -1,19 +1,6 @@ require 'abstract_unit' require 'controller/fake_models' -class Sheep - extend ActiveModel::Naming - include ActiveModel::Conversion - - attr_reader :id - def to_key; id ? [id] : nil end - def save; @id = 1 end - def new_record?; @id.nil? end - def name - @id.nil? ? 'new sheep' : "sheep ##{@id}" - end -end - class RecordIdentifierTest < Test::Unit::TestCase include ActionController::RecordIdentifier diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index 8cb3b4940a2a4..cf3f2f7fa4360 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -130,6 +130,20 @@ def value end end +class Sheep + extend ActiveModel::Naming + include ActiveModel::Conversion + + attr_reader :id + def to_key; id ? [id] : nil end + def save; @id = 1 end + def new_record?; @id.nil? end + def name + @id.nil? ? 'new sheep' : "sheep ##{@id}" + end +end + + class TagRelevance extend ActiveModel::Naming include ActiveModel::Conversion From 83633b807a3b08aaf5554c0fc793583a064c2472 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 14:12:06 -0700 Subject: [PATCH 40/85] avoid creating objects when we can --- activerecord/lib/active_record/base.rb | 3 +-- .../active_record/relation/predicate_builder.rb | 16 +++++----------- .../lib/active_record/relation/query_methods.rb | 2 +- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 80ddd5e7ab6f1..f6d9050828ee1 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1270,8 +1270,7 @@ def sanitize_sql_hash_for_conditions(attrs, default_table_name = self.table_name attrs = expand_hash_conditions_for_aggregates(attrs) table = Arel::Table.new(self.table_name, :engine => arel_engine, :as => default_table_name) - builder = PredicateBuilder.new(arel_engine) - builder.build_from_hash(attrs, table).map{ |b| b.to_sql }.join(' AND ') + PredicateBuilder.build_from_hash(arel_engine, attrs, table).map{ |b| b.to_sql }.join(' AND ') end alias_method :sanitize_sql_hash, :sanitize_sql_hash_for_conditions diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 0d1307d87eb8e..c5428dccd61c2 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -1,23 +1,18 @@ module ActiveRecord - class PredicateBuilder - - def initialize(engine) - @engine = engine - end - - def build_from_hash(attributes, default_table) + class PredicateBuilder # :nodoc: + def self.build_from_hash(engine, attributes, default_table) predicates = attributes.map do |column, value| table = default_table if value.is_a?(Hash) - table = Arel::Table.new(column, :engine => @engine) - build_from_hash(value, table) + table = Arel::Table.new(column, :engine => engine) + build_from_hash(engine, value, table) else column = column.to_s if column.include?('.') table_name, column = column.split('.', 2) - table = Arel::Table.new(table_name, :engine => @engine) + table = Arel::Table.new(table_name, :engine => engine) end attribute = table[column] || Arel::Attribute.new(table, column) @@ -38,6 +33,5 @@ def build_from_hash(attributes, default_table) predicates.flatten end - end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index c8174b5f455fb..001207514d68a 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -204,7 +204,7 @@ def build_where(opts, other = []) [@klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))] when Hash attributes = @klass.send(:expand_hash_conditions_for_aggregates, opts) - PredicateBuilder.new(table.engine).build_from_hash(attributes, table) + PredicateBuilder.build_from_hash(table.engine, attributes, table) else [opts] end From bd78d24bd8168b43ceb167e8d8b3e542486a1bba Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 14:27:54 -0700 Subject: [PATCH 41/85] be kind to the garbage collector and reuse our visitor object --- activerecord/lib/active_record/base.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index f6d9050828ee1..ff6be4ff19911 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1270,7 +1270,10 @@ def sanitize_sql_hash_for_conditions(attrs, default_table_name = self.table_name attrs = expand_hash_conditions_for_aggregates(attrs) table = Arel::Table.new(self.table_name, :engine => arel_engine, :as => default_table_name) - PredicateBuilder.build_from_hash(arel_engine, attrs, table).map{ |b| b.to_sql }.join(' AND ') + viz = Arel::Visitors.for(arel_engine) + PredicateBuilder.build_from_hash(arel_engine, attrs, table).map { |b| + viz.accept b + }.join(' AND ') end alias_method :sanitize_sql_hash, :sanitize_sql_hash_for_conditions From 7836616a6473527f1f42672e54cf6971c05f6fdf Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 15:18:32 -0700 Subject: [PATCH 42/85] remove a few function calls --- activerecord/lib/active_record/schema_dumper.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 6faa88ab78325..437f01b657a66 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -176,9 +176,11 @@ def default_string(value) def indexes(table, stream) if (indexes = @connection.indexes(table)).any? add_index_statements = indexes.map do |index| - statement_parts = [ ('add_index ' + index.table.inspect) ] - statement_parts << index.columns.inspect - statement_parts << (':name => ' + index.name.inspect) + statement_parts = [ + ('add_index ' + index.table.inspect), + index.columns.inspect, + (':name => ' + index.name.inspect), + ] statement_parts << ':unique => true' if index.unique index_lengths = index.lengths.compact if index.lengths.is_a?(Array) From 5154a464cc405438336739ba0d563f87d9fc2d96 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 15:35:56 -0700 Subject: [PATCH 43/85] lengths will be nil or an array --- activerecord/lib/active_record/schema_dumper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 437f01b657a66..f5331bb8a947a 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -183,8 +183,8 @@ def indexes(table, stream) ] statement_parts << ':unique => true' if index.unique - index_lengths = index.lengths.compact if index.lengths.is_a?(Array) - statement_parts << (':length => ' + Hash[*index.columns.zip(index.lengths).flatten].inspect) if index_lengths.present? + index_lengths = (index.lengths || []).compact + statement_parts << (':length => ' + Hash[index.columns.zip(index.lengths)].inspect) unless index_lengths.empty? ' ' + statement_parts.join(', ') end From a6c42c82678f11271fe71923a200eb135f2f1c0e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 15:38:17 -0700 Subject: [PATCH 44/85] two argument String#slice is faster than single argument, also avoid creating a Range object --- actionpack/lib/action_view/test_case.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 731f91df308b9..4026f7a40ec04 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -201,7 +201,7 @@ def _user_defined_ivars # frameworks. def view_assigns Hash[_user_defined_ivars.map do |var| - [var[1..-1].to_sym, instance_variable_get(var)] + [var[1, var.length].to_sym, instance_variable_get(var)] end] end From 8beda11fd3cbd2db21a7a7a7ae9823edbbc2dd5e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:13:45 -0700 Subject: [PATCH 45/85] no need to differentiate between nil and false in this case --- activerecord/lib/active_record/migration.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index e708b3fbcf319..b075632de6a75 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -570,7 +570,7 @@ def migrate current = migrations.detect { |m| m.version == current_version } target = migrations.detect { |m| m.version == @target_version } - if target.nil? && !@target_version.nil? && @target_version > 0 + if target.nil? && @target_version && @target_version > 0 raise UnknownMigrationVersionError.new(@target_version) end @@ -579,7 +579,7 @@ def migrate runnable = migrations[start..finish] # skip the last migration if we're headed down, but not ALL the way down - runnable.pop if down? && !target.nil? + runnable.pop if down? && target runnable.each do |migration| Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger From 341e71a1b9a3d0ad367f79ff89b1c97fe6890905 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:23:07 -0700 Subject: [PATCH 46/85] dry up some migration logic --- activerecord/lib/active_record/migration.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index b075632de6a75..6744a0783a791 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -584,11 +584,13 @@ def migrate runnable.each do |migration| Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger + seen = migrated.include?(migration.version.to_i) + # On our way up, we skip migrating the ones we've already migrated - next if up? && migrated.include?(migration.version.to_i) + next if up? && seen # On our way down, we skip reverting the ones we've never migrated - if down? && !migrated.include?(migration.version.to_i) + if down? && !seen migration.announce 'never migrated, skipping'; migration.write next end From e6583901e58afe58aeb07f1ba1bd9f103492b98b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:32:27 -0700 Subject: [PATCH 47/85] convertion MigrationProxy to a Struct, initialize instance variables --- activerecord/lib/active_record/migration.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 6744a0783a791..5458bba51f109 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -419,9 +419,12 @@ def next_migration_number(number) # MigrationProxy is used to defer loading of the actual migration classes # until they are needed - class MigrationProxy + class MigrationProxy < Struct.new(:name, :version, :filename, :scope) - attr_accessor :name, :version, :filename, :scope + def initialize(name, version, filename, scope) + super + @migration = nil + end delegate :migrate, :announce, :write, :to=>:migration @@ -518,11 +521,7 @@ def migrations(path) raise DuplicateMigrationNameError.new(name.camelize) end - migration = MigrationProxy.new - migration.name = name.camelize - migration.version = version - migration.filename = file - migration.scope = scope + migration = MigrationProxy.new(name.camelize, version, file, scope) klasses << migration end From 40761c4bf39826254b7a5266210f91742591f58c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:36:43 -0700 Subject: [PATCH 48/85] reduce the number of calls to camelize --- activerecord/lib/active_record/migration.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 5458bba51f109..3bfd1851b55c3 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -509,6 +509,7 @@ def migrations(path) migrations = files.inject([]) do |klasses, file| version, name, scope = file.scan(/([0-9]+)_([_a-z0-9]*)\.?([_a-z0-9]*)?.rb/).first + name = name.camelize raise IllegalMigrationNameError.new(file) unless version version = version.to_i @@ -517,11 +518,11 @@ def migrations(path) raise DuplicateMigrationVersionError.new(version) end - if klasses.detect { |m| m.name == name.camelize && m.scope == scope } - raise DuplicateMigrationNameError.new(name.camelize) + if klasses.detect { |m| m.name == name && m.scope == scope } + raise DuplicateMigrationNameError.new(name) end - migration = MigrationProxy.new(name.camelize, version, file, scope) + migration = MigrationProxy.new(name, version, file, scope) klasses << migration end From 365c93b7cdaa161c77c157c7cc385221ebbc33c7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:39:48 -0700 Subject: [PATCH 49/85] speed up duplicate migration detection --- activerecord/lib/active_record/migration.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 3bfd1851b55c3..7a1504430bc97 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -507,6 +507,8 @@ def migrations_path def migrations(path) files = Dir["#{path}/[0-9]*_*.rb"] + seen = Hash.new false + migrations = files.inject([]) do |klasses, file| version, name, scope = file.scan(/([0-9]+)_([_a-z0-9]*)\.?([_a-z0-9]*)?.rb/).first name = name.camelize @@ -514,13 +516,10 @@ def migrations(path) raise IllegalMigrationNameError.new(file) unless version version = version.to_i - if klasses.detect { |m| m.version == version } - raise DuplicateMigrationVersionError.new(version) - end + raise DuplicateMigrationVersionError.new(version) if seen[version] + raise DuplicateMigrationNameError.new(name) if seen[[name, scope]] - if klasses.detect { |m| m.name == name && m.scope == scope } - raise DuplicateMigrationNameError.new(name) - end + seen[version] = seen[[name, scope]] = true migration = MigrationProxy.new(name, version, file, scope) klasses << migration From 69a2c6b0419177448d9811745cf4035d46b68bbd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:41:59 -0700 Subject: [PATCH 50/85] converting inject([]) to map --- activerecord/lib/active_record/migration.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 7a1504430bc97..9ac18f993955d 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -509,20 +509,19 @@ def migrations(path) seen = Hash.new false - migrations = files.inject([]) do |klasses, file| + migrations = files.map do |file| version, name, scope = file.scan(/([0-9]+)_([_a-z0-9]*)\.?([_a-z0-9]*)?.rb/).first - name = name.camelize raise IllegalMigrationNameError.new(file) unless version version = version.to_i + name = name.camelize raise DuplicateMigrationVersionError.new(version) if seen[version] raise DuplicateMigrationNameError.new(name) if seen[[name, scope]] seen[version] = seen[[name, scope]] = true - migration = MigrationProxy.new(name, version, file, scope) - klasses << migration + MigrationProxy.new(name, version, file, scope) end migrations.sort_by(&:version) From 3986fcb935c8d5b89ecb86b2f1cbb463460182de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 4 Oct 2010 08:47:36 +0200 Subject: [PATCH 51/85] Initialize sid should just skip instance variables. --- .../action_dispatch/middleware/session/abstract_store.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index 679ba7fc8ed30..64d3a87fd0443 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -31,6 +31,13 @@ def initialize(app, options = {}) def generate_sid ActiveSupport::SecureRandom.hex(16) end + + protected + + def initialize_sid + @default_options.delete(:sidbits) + @default_options.delete(:secure_random) + end end module StaleSessionCheck From 10d014acb875c8201b33f5ffe7b08dbcd81b2875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 4 Oct 2010 13:46:37 +0200 Subject: [PATCH 52/85] Update to Thor 0.14.3. --- railties/railties.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/railties.gemspec b/railties/railties.gemspec index 73acb73dec331..d26c1bcdbcf5b 100644 --- a/railties/railties.gemspec +++ b/railties/railties.gemspec @@ -20,7 +20,7 @@ Gem::Specification.new do |s| s.has_rdoc = false s.add_dependency('rake', '>= 0.8.7') - s.add_dependency('thor', '~> 0.14.2') + s.add_dependency('thor', '~> 0.14.3') s.add_dependency('activesupport', version) s.add_dependency('actionpack', version) end From 0b51f3cc73ac21ed56b45a15fcce1d31beb7170c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 4 Oct 2010 18:06:04 +0200 Subject: [PATCH 53/85] Ensure the proper content type is returned for static files. --- .../lib/action_dispatch/middleware/static.rb | 6 +-- actionpack/test/dispatch/static_test.rb | 47 +++++++++++-------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index cf139383315b3..913b899e20cbd 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -6,13 +6,13 @@ def initialize(at, root) @at, @root = at.chomp('/'), root.chomp('/') @compiled_at = (Regexp.compile(/^#{Regexp.escape(at)}/) unless @at.blank?) @compiled_root = Regexp.compile(/^#{Regexp.escape(root)}/) - @file_server = ::Rack::File.new(root) + @file_server = ::Rack::File.new(@root) end def match?(path) path = path.dup - if @compiled_at.blank? || path.sub!(@compiled_at, '') - full_path = File.join(@root, ::Rack::Utils.unescape(path)) + if !@compiled_at || path.sub!(@compiled_at, '') + full_path = path.empty? ? @root : File.join(@root, ::Rack::Utils.unescape(path)) paths = "#{full_path}#{ext}" matches = Dir[paths] diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 2eb82fc5d897f..655745a848e66 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -2,30 +2,37 @@ module StaticTests def test_serves_dynamic_content - assert_equal "Hello, World!", get("/nofile") + assert_equal "Hello, World!", get("/nofile").body end def test_serves_static_index_at_root - assert_equal "/index.html", get("/index.html") - assert_equal "/index.html", get("/index") - assert_equal "/index.html", get("/") + assert_html "/index.html", get("/index.html") + assert_html "/index.html", get("/index") + assert_html "/index.html", get("/") + assert_html "/index.html", get("") end def test_serves_static_file_in_directory - assert_equal "/foo/bar.html", get("/foo/bar.html") - assert_equal "/foo/bar.html", get("/foo/bar/") - assert_equal "/foo/bar.html", get("/foo/bar") + assert_html "/foo/bar.html", get("/foo/bar.html") + assert_html "/foo/bar.html", get("/foo/bar/") + assert_html "/foo/bar.html", get("/foo/bar") end def test_serves_static_index_file_in_directory - assert_equal "/foo/index.html", get("/foo/index.html") - assert_equal "/foo/index.html", get("/foo/") - assert_equal "/foo/index.html", get("/foo") + assert_html "/foo/index.html", get("/foo/index.html") + assert_html "/foo/index.html", get("/foo/") + assert_html "/foo/index.html", get("/foo") end private + + def assert_html(body, response) + assert_equal body, response.body + assert_equal "text/html", response.headers["Content-Type"] + end + def get(path) - Rack::MockRequest.new(@app).request("GET", path).body + Rack::MockRequest.new(@app).request("GET", path) end end @@ -59,16 +66,16 @@ def setup include StaticTests test "serves files from other mounted directories" do - assert_equal "/blog/index.html", get("/blog/index.html") - assert_equal "/blog/index.html", get("/blog/index") - assert_equal "/blog/index.html", get("/blog/") + assert_html "/blog/index.html", get("/blog/index.html") + assert_html "/blog/index.html", get("/blog/index") + assert_html "/blog/index.html", get("/blog/") - assert_equal "/blog/blog.html", get("/blog/blog/") - assert_equal "/blog/blog.html", get("/blog/blog.html") - assert_equal "/blog/blog.html", get("/blog/blog") + assert_html "/blog/blog.html", get("/blog/blog/") + assert_html "/blog/blog.html", get("/blog/blog.html") + assert_html "/blog/blog.html", get("/blog/blog") - assert_equal "/blog/subdir/index.html", get("/blog/subdir/index.html") - assert_equal "/blog/subdir/index.html", get("/blog/subdir/") - assert_equal "/blog/subdir/index.html", get("/blog/subdir") + assert_html "/blog/subdir/index.html", get("/blog/subdir/index.html") + assert_html "/blog/subdir/index.html", get("/blog/subdir/") + assert_html "/blog/subdir/index.html", get("/blog/subdir") end end From 848e48ec9c20b6de15768c7bc33a5c54b242afac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 4 Oct 2010 18:08:03 +0200 Subject: [PATCH 54/85] Link to rack from github for this while. --- railties/lib/rails/generators/rails/app/templates/Gemfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 1dbf27d978aac..40213b1261827 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -3,15 +3,18 @@ source 'http://rubygems.org' <%- if options.dev? -%> gem 'rails', :path => '<%= Rails::Generators::RAILS_DEV_PATH %>' gem 'arel', :git => 'git://github.com/rails/arel.git' +gem "rack", :git => "git://github.com/rack/rack.git" <%- elsif options.edge? -%> gem 'rails', :git => 'git://github.com/rails/rails.git' gem 'arel', :git => 'git://github.com/rails/arel.git' +gem "rack", :git => "git://github.com/rack/rack.git" <%- else -%> gem 'rails', '<%= Rails::VERSION::STRING %>' # Bundle edge Rails instead: # gem 'rails', :git => 'git://github.com/rails/rails.git' # gem 'arel', :git => 'git://github.com/rails/arel.git' +# gem "rack", :git => "git://github.com/rack/rack.git" <%- end -%> <% unless options[:skip_active_record] -%> From ccf228b0276afb8bb00d14f1b861015b1ef5ab76 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 09:28:21 -0700 Subject: [PATCH 55/85] [#5406 state:resolved] calling the correct method on minitest to obtain the test name --- activesupport/test/test_case_test.rb | 38 ++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 activesupport/test/test_case_test.rb diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb new file mode 100644 index 0000000000000..8a5006cdf229b --- /dev/null +++ b/activesupport/test/test_case_test.rb @@ -0,0 +1,38 @@ +require 'abstract_unit' + +module ActiveSupport + class TestCaseTest < ActiveSupport::TestCase + class FakeRunner + attr_reader :puked + + def initialize + @puked = [] + end + + def puke(klass, name, e) + @puked << [klass, name, e] + end + end + + if defined?(MiniTest::Assertions) && TestCase < MiniTest::Assertions + def test_callback_with_exception + tc = Class.new(TestCase) do + setup :bad_callback + def bad_callback; raise 'oh noes' end + def test_true; assert true end + end + + test_name = 'test_true' + fr = FakeRunner.new + + test = tc.new test_name + test.run fr + klass, name, exception = *fr.puked.first + + assert_equal tc, klass + assert_equal test_name, name + assert_equal 'oh noes', exception.message + end + end + end +end From b7c49cedba4a9acd001df65a102d79611598f472 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 09:29:37 -0700 Subject: [PATCH 56/85] calling correct method on minitest for test name when teardown callback fails --- activesupport/test/test_case_test.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb index 8a5006cdf229b..7e65c63062c3b 100644 --- a/activesupport/test/test_case_test.rb +++ b/activesupport/test/test_case_test.rb @@ -33,6 +33,25 @@ def test_true; assert true end assert_equal test_name, name assert_equal 'oh noes', exception.message end + + def test_teardown_callback_with_exception + tc = Class.new(TestCase) do + teardown :bad_callback + def bad_callback; raise 'oh noes' end + def test_true; assert true end + end + + test_name = 'test_true' + fr = FakeRunner.new + + test = tc.new test_name + test.run fr + klass, name, exception = *fr.puked.first + + assert_equal tc, klass + assert_equal test_name, name + assert_equal 'oh noes', exception.message + end end end end From 33aaa15f6208d65ca043ffcc5dc161d410963e9d Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Mon, 4 Oct 2010 13:35:38 -0400 Subject: [PATCH 57/85] Convert to model before calling model_name on a record in ActiveModel::Naming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activemodel/lib/active_model/naming.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index 2d580fd325927..adb71f788f715 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -129,7 +129,11 @@ def self.param_key(record_or_class) private def self.model_name_from_record_or_class(record_or_class) - (record_or_class.is_a?(Class) ? record_or_class : record_or_class.class).model_name + (record_or_class.is_a?(Class) ? record_or_class : convert_to_model(record_or_class).class).model_name + end + + def self.convert_to_model(object) + object.respond_to?(:to_model) ? object.to_model : object end end From 21cb1d40b9c57ab814c69d01a242b50e27e8283b Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Mon, 4 Oct 2010 15:04:39 -0400 Subject: [PATCH 58/85] Test to_model being called in ActiveModel::Naming helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activemodel/test/cases/naming_test.rb | 4 ++++ activemodel/test/models/track_back.rb | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/activemodel/test/cases/naming_test.rb b/activemodel/test/cases/naming_test.rb index 40ce4c0e2d0fb..a7dde2c4330ec 100644 --- a/activemodel/test/cases/naming_test.rb +++ b/activemodel/test/cases/naming_test.rb @@ -125,6 +125,10 @@ def setup @param_key = 'contact' end + def test_to_model_called_on_record + assert_equal 'post_named_track_backs', plural(Post::TrackBack.new) + end + def test_singular assert_equal @singular, singular(@record) end diff --git a/activemodel/test/models/track_back.rb b/activemodel/test/models/track_back.rb index d137e4ff8f97e..545acd1655a88 100644 --- a/activemodel/test/models/track_back.rb +++ b/activemodel/test/models/track_back.rb @@ -1,4 +1,11 @@ class Post class TrackBack + def to_model + NamedTrackBack.new(self) + end + end + + class NamedTrackBack + extend ActiveModel::Naming end end \ No newline at end of file From 232e56ce872383ef1805d1a7ef0fbf5475e5cfb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 4 Oct 2010 22:32:29 +0200 Subject: [PATCH 59/85] No need to pass self as parameter here. --- activemodel/test/models/track_back.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activemodel/test/models/track_back.rb b/activemodel/test/models/track_back.rb index 545acd1655a88..768c96ecf0c54 100644 --- a/activemodel/test/models/track_back.rb +++ b/activemodel/test/models/track_back.rb @@ -1,7 +1,7 @@ class Post class TrackBack def to_model - NamedTrackBack.new(self) + NamedTrackBack.new end end From d8135eb452bfb956e093c61cf12adb9b0da6e28b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 15:19:27 -0700 Subject: [PATCH 60/85] * + flatten is not required in >= Ruby 1.8.7 --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 194842a9a09ce..95b6a8137d1d2 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -975,7 +975,7 @@ def last_insert_id(table, sequence_name) #:nodoc: def select(sql, name = nil) fields, rows = select_raw(sql, name) rows.map do |row| - Hash[*fields.zip(row).flatten] + Hash[fields.zip(row)] end end From e7d860c6bed99b0d44680097fcc1cfd7c1fa07ef Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 15:25:20 -0700 Subject: [PATCH 61/85] create fewer objects, call fewer methods in extract_pg_identifier_from_name --- .../active_record/connection_adapters/postgresql_adapter.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 95b6a8137d1d2..5f14284615da3 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -1017,11 +1017,11 @@ def column_definitions(table_name) #:nodoc: end def extract_pg_identifier_from_name(name) - match_data = name[0,1] == '"' ? name.match(/\"([^\"]+)\"/) : name.match(/([^\.]+)/) + match_data = name.start_with?('"') ? name.match(/\"([^\"]+)\"/) : name.match(/([^\.]+)/) if match_data - rest = name[match_data[0].length..-1] - rest = rest[1..-1] if rest[0,1] == "." + rest = name[match_data[0].length, name.length] + rest = rest[1, rest.length] if rest.start_with? "." [match_data[1], (rest.length > 0 ? rest : nil)] end end From 28bb1885f5a35d0adecd35d38b73751d737891c4 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:08:01 -0700 Subject: [PATCH 62/85] avoid method call to compact --- actionpack/lib/action_dispatch/routing/mapper.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 47aed0273cea8..bf10f811272ea 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -171,13 +171,13 @@ def default_controller_and_action(to_shorthand=nil) end def blocks + block = @scope[:blocks] || [] + if @options[:constraints].present? && !@options[:constraints].is_a?(Hash) - block = @options[:constraints] - else - block = nil + block << @options[:constraints] end - ((@scope[:blocks] || []) + [ block ]).compact + block end def constraints From f9734f2b0f5326c399d1c1cccba8b6d8e7d9bdd4 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:30:16 -0700 Subject: [PATCH 63/85] adding tests for uploaded file --- .../test/dispatch/uploaded_file_test.rb | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 actionpack/test/dispatch/uploaded_file_test.rb diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb new file mode 100644 index 0000000000000..def6289fb37f1 --- /dev/null +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -0,0 +1,25 @@ +require 'abstract_unit' + +module ActionDispatch + class UploadedFileTest < ActiveSupport::TestCase + def test_original_filename + uf = Http::UploadedFile.new(:filename => 'foo') + assert_equal 'foo', uf.original_filename + end + + def test_content_type + uf = Http::UploadedFile.new(:type => 'foo') + assert_equal 'foo', uf.content_type + end + + def test_headers + uf = Http::UploadedFile.new(:head => 'foo') + assert_equal 'foo', uf.headers + end + + def test_tempfile + uf = Http::UploadedFile.new(:tempfile => 'foo') + assert_equal 'foo', uf.tempfile + end + end +end From 2a3022db7f2ddc0fc0e678ea757f97902c5f5c01 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:56:45 -0700 Subject: [PATCH 64/85] delegate to the @tempfile instance variable --- actionpack/lib/action_dispatch/http/upload.rb | 18 +++++------------- actionpack/test/dispatch/uploaded_file_test.rb | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 49465d820e938..bfbe7c530559a 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -2,27 +2,19 @@ module ActionDispatch module Http - class UploadedFile < Tempfile + class UploadedFile attr_accessor :original_filename, :content_type, :tempfile, :headers def initialize(hash) @original_filename = hash[:filename] @content_type = hash[:type] @headers = hash[:head] - - # To the untrained eye, this may appear as insanity. Given the alternatives, - # such as busting the method cache on every request or breaking backwards - # compatibility with is_a?(Tempfile), this solution is the best available - # option. - # - # TODO: Deprecate is_a?(Tempfile) and define a real API for this parameter - tempfile = hash[:tempfile] - tempfile.instance_variables.each do |ivar| - instance_variable_set(ivar, tempfile.instance_variable_get(ivar)) - end + @tempfile = hash[:tempfile] end - alias local_path path + def method_missing(name, *args, &block) + @tempfile.send(name, *args, &block) + end end module Upload diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index def6289fb37f1..d04d5a8650bc6 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -21,5 +21,23 @@ def test_tempfile uf = Http::UploadedFile.new(:tempfile => 'foo') assert_equal 'foo', uf.tempfile end + + def test_delegates_to_tempfile + tf = Class.new { def tenderlove; 'thunderhorse' end } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert_equal 'thunderhorse', uf.tenderlove + end + + def test_delegates_to_tempfile_with_params + tf = Class.new { def tenderlove *args; args end } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert_equal %w{ thunder horse }, uf.tenderlove(*%w{ thunder horse }) + end + + def test_delegates_to_tempfile_with_block + tf = Class.new { def tenderlove; yield end } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert_equal('thunderhorse', uf.tenderlove { 'thunderhorse' }) + end end end From 876acf001a32887679e259f83a2fbad750ac2e67 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:57:11 -0700 Subject: [PATCH 65/85] if it walks like a duck and talks like a duck, it must be a duck --- .../test/dispatch/request/multipart_params_parsing_test.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 073dd3ddad1f9..3ff558ec5a53c 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -36,7 +36,6 @@ def teardown assert_equal 'bar', params['foo'] file = params['file'] - assert_kind_of Tempfile, file assert_equal 'file.txt', file.original_filename assert_equal "text/plain", file.content_type assert_equal 'contents', file.read @@ -49,8 +48,6 @@ def teardown file = params['file'] foo = params['foo'] - assert_kind_of Tempfile, file - assert_equal 'file.txt', file.original_filename assert_equal "text/plain", file.content_type @@ -64,8 +61,6 @@ def teardown file = params['file'] - assert_kind_of Tempfile, file - assert_equal 'file.txt', file.original_filename assert_equal "text/plain", file.content_type assert_equal(('a' * 20480), file.read) @@ -77,13 +72,11 @@ def teardown assert_equal 'bar', params['foo'] file = params['file'] - assert_kind_of Tempfile, file assert_equal 'file.csv', file.original_filename assert_nil file.content_type assert_equal 'contents', file.read file = params['flowers'] - assert_kind_of Tempfile, file assert_equal 'flowers.jpg', file.original_filename assert_equal "image/jpeg", file.content_type assert_equal 19512, file.size From 8a9747021085c569f0118db1093bc12cfa2ba915 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 17:08:25 -0700 Subject: [PATCH 66/85] raising an argument error if tempfile is not provided --- actionpack/lib/action_dispatch/http/upload.rb | 1 + actionpack/test/dispatch/uploaded_file_test.rb | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index bfbe7c530559a..d4fabe1eaf1d1 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -10,6 +10,7 @@ def initialize(hash) @content_type = hash[:type] @headers = hash[:head] @tempfile = hash[:tempfile] + raise(ArgumentError, ':tempfile is required') unless @tempfile end def method_missing(name, *args, &block) diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index d04d5a8650bc6..4173ce0a44225 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -2,18 +2,24 @@ module ActionDispatch class UploadedFileTest < ActiveSupport::TestCase + def test_constructor_with_argument_error + assert_raises(ArgumentError) do + Http::UploadedFile.new({}) + end + end + def test_original_filename - uf = Http::UploadedFile.new(:filename => 'foo') + uf = Http::UploadedFile.new(:filename => 'foo', :tempfile => Object.new) assert_equal 'foo', uf.original_filename end def test_content_type - uf = Http::UploadedFile.new(:type => 'foo') + uf = Http::UploadedFile.new(:type => 'foo', :tempfile => Object.new) assert_equal 'foo', uf.content_type end def test_headers - uf = Http::UploadedFile.new(:head => 'foo') + uf = Http::UploadedFile.new(:head => 'foo', :tempfile => Object.new) assert_equal 'foo', uf.headers end From 3370ad0b1e883c9ec24c771f6c52b296a71eff40 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 17:11:50 -0700 Subject: [PATCH 67/85] making sure respond_to? works properly --- actionpack/lib/action_dispatch/http/upload.rb | 5 +++++ actionpack/test/dispatch/uploaded_file_test.rb | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index d4fabe1eaf1d1..53f8039121cad 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -13,7 +13,12 @@ def initialize(hash) raise(ArgumentError, ':tempfile is required') unless @tempfile end + def respond_to?(name) + super || @tempfile.respond_to?(name) + end + def method_missing(name, *args, &block) + return super unless respond_to?(name) @tempfile.send(name, *args, &block) end end diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index 4173ce0a44225..c81797a73fadc 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -45,5 +45,20 @@ def test_delegates_to_tempfile_with_block uf = Http::UploadedFile.new(:tempfile => tf.new) assert_equal('thunderhorse', uf.tenderlove { 'thunderhorse' }) end + + def test_delegate_respects_respond_to? + tf = Class.new { def tenderlove; yield end; private :tenderlove } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert_raises(NoMethodError) do + uf.tenderlove + end + end + + def test_respond_to? + tf = Class.new { def tenderlove; yield end } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert uf.respond_to?(:headers), 'responds to headers' + assert uf.respond_to?(:tenderlove), 'responds to tenderlove' + end end end From 12173396163616e077f761e190c13beb43d536bd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:28:40 -0700 Subject: [PATCH 68/85] only forwarding enough methods to work. People should grab the delegate tempfile if they really need to do hard work --- actionpack/lib/action_dispatch/http/upload.rb | 13 ++++++----- .../test/dispatch/uploaded_file_test.rb | 22 +++++++------------ 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 53f8039121cad..84e58d7d6a308 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -13,13 +13,16 @@ def initialize(hash) raise(ArgumentError, ':tempfile is required') unless @tempfile end - def respond_to?(name) - super || @tempfile.respond_to?(name) + def read(*args) + @tempfile.read(*args) end - def method_missing(name, *args, &block) - return super unless respond_to?(name) - @tempfile.send(name, *args, &block) + def rewind + @tempfile.rewind + end + + def size + @tempfile.size end end diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index c81797a73fadc..b51697b9307d9 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -29,36 +29,30 @@ def test_tempfile end def test_delegates_to_tempfile - tf = Class.new { def tenderlove; 'thunderhorse' end } + tf = Class.new { def read; 'thunderhorse' end } uf = Http::UploadedFile.new(:tempfile => tf.new) - assert_equal 'thunderhorse', uf.tenderlove + assert_equal 'thunderhorse', uf.read end def test_delegates_to_tempfile_with_params - tf = Class.new { def tenderlove *args; args end } + tf = Class.new { def read *args; args end } uf = Http::UploadedFile.new(:tempfile => tf.new) - assert_equal %w{ thunder horse }, uf.tenderlove(*%w{ thunder horse }) - end - - def test_delegates_to_tempfile_with_block - tf = Class.new { def tenderlove; yield end } - uf = Http::UploadedFile.new(:tempfile => tf.new) - assert_equal('thunderhorse', uf.tenderlove { 'thunderhorse' }) + assert_equal %w{ thunder horse }, uf.read(*%w{ thunder horse }) end def test_delegate_respects_respond_to? - tf = Class.new { def tenderlove; yield end; private :tenderlove } + tf = Class.new { def read; yield end; private :read } uf = Http::UploadedFile.new(:tempfile => tf.new) assert_raises(NoMethodError) do - uf.tenderlove + uf.read end end def test_respond_to? - tf = Class.new { def tenderlove; yield end } + tf = Class.new { def read; yield end } uf = Http::UploadedFile.new(:tempfile => tf.new) assert uf.respond_to?(:headers), 'responds to headers' - assert uf.respond_to?(:tenderlove), 'responds to tenderlove' + assert uf.respond_to?(:read), 'responds to read' end end end From 5769636663c2849fd132fd593c0f6d29702b7cf1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:32:49 -0700 Subject: [PATCH 69/85] fixing a few test warnings --- actionpack/test/controller/filters_test.rb | 4 ++-- actionpack/test/lib/controller/fake_models.rb | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index dfc90943e1fc3..3a8a37d967fc9 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -758,12 +758,12 @@ def raise_after def without_exception # Do stuff... - 1 + 1 + wtf = 1 + 1 yield # Do stuff... - 1 + 1 + wtf += 1 end end diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index cf3f2f7fa4360..dba632e6df501 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -55,6 +55,11 @@ class Post < Struct.new(:title, :author_name, :body, :secret, :written_on, :cost alias_method :secret?, :secret + def initialize(*args) + super + @persisted = false + end + def persisted=(boolean) @persisted = boolean end From 333a5659e8663049618386e3fa45248d388070fd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:46:38 -0700 Subject: [PATCH 70/85] dry up some crazy codes --- actionpack/lib/action_view/helpers/url_helper.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 1c3ca78d28f78..440e0162cdcf5 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -476,18 +476,16 @@ def mail_to(email_address, name = nil, html_options = {}) html_options = html_options.stringify_keys encode = html_options.delete("encode").to_s - cc, bcc, subject, body = html_options.delete("cc"), html_options.delete("bcc"), html_options.delete("subject"), html_options.delete("body") - extras = [] - extras << "cc=#{Rack::Utils.escape(cc).gsub("+", "%20")}" unless cc.nil? - extras << "bcc=#{Rack::Utils.escape(bcc).gsub("+", "%20")}" unless bcc.nil? - extras << "body=#{Rack::Utils.escape(body).gsub("+", "%20")}" unless body.nil? - extras << "subject=#{Rack::Utils.escape(subject).gsub("+", "%20")}" unless subject.nil? + extras = %w{ cc bcc body subject }.map { |item| + option = html_options.delete(item) || next + "#{item}=#{Rack::Utils.escape(option).gsub("+", "%20")}" + }.compact extras = extras.empty? ? '' : '?' + html_escape(extras.join('&')) email_address_obfuscated = email_address.dup - email_address_obfuscated.gsub!(/@/, html_options.delete("replace_at")) if html_options.has_key?("replace_at") - email_address_obfuscated.gsub!(/\./, html_options.delete("replace_dot")) if html_options.has_key?("replace_dot") + email_address_obfuscated.gsub!(/@/, html_options.delete("replace_at")) if html_options.key?("replace_at") + email_address_obfuscated.gsub!(/\./, html_options.delete("replace_dot")) if html_options.key?("replace_dot") string = '' From 714fea4540c96f70d044e2bd1be92b504d1f8fa3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:52:17 -0700 Subject: [PATCH 71/85] deleting more crazy --- actionpack/lib/action_view/helpers/url_helper.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 440e0162cdcf5..0eed3ea25984b 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -495,13 +495,11 @@ def mail_to(email_address, name = nil, html_options = {}) end "".html_safe elsif encode == "hex" - email_address_encoded = '' - email_address_obfuscated.each_byte do |c| - email_address_encoded << sprintf("&#%d;", c) - end + email_address_encoded = email_address_obfuscated.unpack('C*').map {|c| + sprintf("&#%d;", c) + }.join - protocol = 'mailto:' - protocol.each_byte { |c| string << sprintf("&#%d;", c) } + string += 'mailto:'.unpack('C*').map { |c| sprintf("&#%d;", c) }.join email_address.each_byte do |c| char = c.chr From 839e2f96647d5b29cc2865555a6f615c31429109 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 20:00:55 -0700 Subject: [PATCH 72/85] cleaning up more crazy! --- actionpack/lib/action_view/helpers/url_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 0eed3ea25984b..651240a1227d9 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -501,10 +501,10 @@ def mail_to(email_address, name = nil, html_options = {}) string += 'mailto:'.unpack('C*').map { |c| sprintf("&#%d;", c) }.join - email_address.each_byte do |c| + string += email_address.unpack('C*').map do |c| char = c.chr - string << (char =~ /\w/ ? sprintf("%%%x", c) : char) - end + char =~ /\w/ ? sprintf("%%%x", c) : char + end.join content_tag "a", name || email_address_encoded.html_safe, html_options.merge("href" => "#{string}#{extras}".html_safe) else content_tag "a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe) From e3acdcfbf349e416c63f4c56170e9d82f7b1b1d0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 20:05:56 -0700 Subject: [PATCH 73/85] refactoring to use fewer intermediate variables --- .../lib/action_view/helpers/url_helper.rb | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 651240a1227d9..da42d943183f9 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -487,24 +487,25 @@ def mail_to(email_address, name = nil, html_options = {}) email_address_obfuscated.gsub!(/@/, html_options.delete("replace_at")) if html_options.key?("replace_at") email_address_obfuscated.gsub!(/\./, html_options.delete("replace_dot")) if html_options.key?("replace_dot") - string = '' - - if encode == "javascript" - "document.write('#{content_tag("a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe))}');".each_byte do |c| - string << sprintf("%%%x", c) - end + case encode + when "javascript" + string = + "document.write('#{content_tag("a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe))}');".unpack('C*').map { |c| + sprintf("%%%x", c) + }.join "".html_safe - elsif encode == "hex" + when "hex" email_address_encoded = email_address_obfuscated.unpack('C*').map {|c| sprintf("&#%d;", c) }.join - string += 'mailto:'.unpack('C*').map { |c| sprintf("&#%d;", c) }.join - - string += email_address.unpack('C*').map do |c| + string = 'mailto:'.unpack('C*').map { |c| + sprintf("&#%d;", c) + }.join + email_address.unpack('C*').map { |c| char = c.chr char =~ /\w/ ? sprintf("%%%x", c) : char - end.join + }.join + content_tag "a", name || email_address_encoded.html_safe, html_options.merge("href" => "#{string}#{extras}".html_safe) else content_tag "a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe) From 4a4ff148ff85a148b5313dcc429e21b2d32057d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 5 Oct 2010 09:48:32 +0200 Subject: [PATCH 74/85] Use RbConfig instead of Config for 1.9.3 compatibility. --- railties/lib/rails/engine.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index ba89bce928869..3981e8dfd5899 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -535,7 +535,7 @@ def find_root_with_flag(flag, default=nil) root = File.exist?("#{root_path}/#{flag}") ? root_path : default raise "Could not find root path for #{self}" unless root - Config::CONFIG['host_os'] =~ /mswin|mingw/ ? + RbConfig::CONFIG['host_os'] =~ /mswin|mingw/ ? Pathname.new(root).expand_path : Pathname.new(root).realpath end From 68d75c3365e1d0e1af21caab01e192223c371511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 5 Oct 2010 09:55:51 +0200 Subject: [PATCH 75/85] Don't expect an AD::Response object back from the app. --- railties/test/railties/engine_test.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index c75639d740198..89262aeaae435 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -89,7 +89,7 @@ class Engine < ::Rails::Engine env = Rack::MockRequest.env_for("/bukkits") response = Rails.application.call(env) - assert_equal "HELLO WORLD", response[2] + assert_equal ["HELLO WORLD"], response[2] end test "it provides routes as default endpoint" do @@ -116,8 +116,7 @@ class Engine < ::Rails::Engine env = Rack::MockRequest.env_for("/bukkits/foo") response = Rails.application.call(env) - - assert_equal "foo", response[2] + assert_equal ["foo"], response[2] end test "engine can load its own plugins" do @@ -379,15 +378,15 @@ def bar env = Rack::MockRequest.env_for("/foo") response = Rails.application.call(env) - assert_equal "Something... Something... Something...", response[2].body + assert_equal ["Something... Something... Something..."], response[2] env = Rack::MockRequest.env_for("/foo/show") response = Rails.application.call(env) - assert_equal "/foo", response[2].body + assert_equal ["/foo"], response[2] env = Rack::MockRequest.env_for("/foo/bar") response = Rails.application.call(env) - assert_equal "It's a bar.", response[2].body + assert_equal ["It's a bar."], response[2] end test "isolated engine should include only its own routes and helpers" do @@ -488,23 +487,23 @@ class MyMailer < ActionMailer::Base env = Rack::MockRequest.env_for("/bukkits/from_app") response = AppTemplate::Application.call(env) - assert_equal "false", response[2].body + assert_equal ["false"], response[2] env = Rack::MockRequest.env_for("/bukkits/foo/show") response = AppTemplate::Application.call(env) - assert_equal "/bukkits/foo", response[2].body + assert_equal ["/bukkits/foo"], response[2] env = Rack::MockRequest.env_for("/bukkits/foo") response = AppTemplate::Application.call(env) - assert_equal "Helped.", response[2].body + assert_equal ["Helped."], response[2] env = Rack::MockRequest.env_for("/bukkits/routes_helpers_in_view") response = AppTemplate::Application.call(env) - assert_equal "/bukkits/foo, /bar", response[2].body + assert_equal ["/bukkits/foo, /bar"], response[2] env = Rack::MockRequest.env_for("/bukkits/polymorphic_path_without_namespace") response = AppTemplate::Application.call(env) - assert_equal "/bukkits/posts/1", response[2].body + assert_equal ["/bukkits/posts/1"], response[2] end test "isolated engine should avoid namespace in names if that's possible" do From 360a8780706cc95144bef429d6fe42e9475a5400 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 5 Oct 2010 19:34:27 +0200 Subject: [PATCH 76/85] new guide: Ruby on Rails Guides Guidelines --- railties/guides/source/index.html.erb | 4 + railties/guides/source/layout.html.erb | 1 + .../ruby_on_rails_guides_guidelines.textile | 78 +++++++++++++++++++ 3 files changed, 83 insertions(+) create mode 100644 railties/guides/source/ruby_on_rails_guides_guidelines.textile diff --git a/railties/guides/source/index.html.erb b/railties/guides/source/index.html.erb index e6d327168ae2b..d28ea24a76f56 100644 --- a/railties/guides/source/index.html.erb +++ b/railties/guides/source/index.html.erb @@ -161,6 +161,10 @@ Ruby on Rails Guides <%= guide('API Documentation Guidelines', 'api_documentation_guidelines.html') do %>

This guide documents the Ruby on Rails API documentation guidelines.

<% end %> + + <%= guide('Ruby on Rails Guides Guidelines', 'ruby_on_rails_guides_guidelines.html') do %> +

This guide documentes the Ruby on Rails guides Guidelines.

+ <% end %>

Release Notes

diff --git a/railties/guides/source/layout.html.erb b/railties/guides/source/layout.html.erb index 2039c76213ef9..f0aa227c7e406 100644 --- a/railties/guides/source/layout.html.erb +++ b/railties/guides/source/layout.html.erb @@ -81,6 +81,7 @@
Contributing to Rails
Contributing to Rails
API Documentation Guidelines
+
Ruby on Rails Guides Guidelines
Release Notes
Ruby on Rails 3.0 Release Notes
diff --git a/railties/guides/source/ruby_on_rails_guides_guidelines.textile b/railties/guides/source/ruby_on_rails_guides_guidelines.textile new file mode 100644 index 0000000000000..818d96da7989e --- /dev/null +++ b/railties/guides/source/ruby_on_rails_guides_guidelines.textile @@ -0,0 +1,78 @@ +h2. Ruby on Rails Guides Guidelines + +This guide documents guidelines for writing guides. This guide follows itself in a gracile loop. + +endprologue. + +h3. Textile + +Guides are written in "Textile":http://www.textism.com/tools/textile/. There's comprehensive documentation "here":http://redcloth.org/hobix.com/textile/ and a cheatsheet for markup "here":http://redcloth.org/hobix.com/textile/quick.html. + +h3. Prologue + +Each guide should start with motivational text at the top. That's the little introduction in the blue area. The prologue should tell the readers what's the guide about, and what will they learn. See for example the "Routing Guide":routing.html. + +h3. Titles + +The title of every guide uses +h2+, guide sections use +h3+, subsections +h4+, etc. + +Capitalize all words except for internal articles, prepositions, conjuctions, and forms of the verb to be: + + +h5. Middleware Stack is an Array +h5. When are Objects Saved? + + +Use same typography as in regular text: + + +h3. Brief Note About +Test::Unit+ +h6. The +:content_type+ Option + + +h3. API Documentation Guidelides + +The guides and the API should be coherent where appropriate. Please have a look at these particular sections of the "API Documentation Guidelines":api_documentation_guidelines.html: + +* "Wording":api_documentation_guidelines.html#wording +* "Example Code":api_documentation_guidelines.html#example-code +* "Filenames":api_documentation_guidelines.html#filenames +* "Fonts":api_documentation_guidelines.html#fonts + +Those guidelines apply also to guides. + +h3. HTML Generation + +To generate all the guides just cd into the +railties+ directory and execute + + +rake generate_guides + + +You'll need the gems erubis, i18n, and RedCloth. + +To process +my_guide.textile+ and nothing else use the +ONLY+ environment variable: + + +rake generate_guides ONLY=my_guide + + +Although by default guides that have not been modified are not processed, so +ONLY+ is rarely needed in practice. + +To force process of al the guides pass +ALL=1+. + +It is also recommended that you work with +WARNINGS=1+, this detects duplicate IDs and warns about broken internal links. + +h3. HTML validation + +Please do validate the generated HTML with + + +rake validate_guides + + +Particularly, titles get an ID generated from their content and this often leads to duplicates. Please set +WARNINGS=1+ when generating guides to detect them. The warning messages suggest a way to fix them. + +h3. Changelog + +* October 5, 2010: ported from the docrails wiki and revised by "Xavier Noria":credits.html#fxn From da34ee423f3d43c368e33d834bb70d59af16273b Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 5 Oct 2010 19:38:05 +0200 Subject: [PATCH 77/85] aaaaannnddd, your beloved typo only spotted in the github colored diff no matter how many passes you did before pushing --- railties/guides/source/index.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/guides/source/index.html.erb b/railties/guides/source/index.html.erb index d28ea24a76f56..6b897e3a6a295 100644 --- a/railties/guides/source/index.html.erb +++ b/railties/guides/source/index.html.erb @@ -163,7 +163,7 @@ Ruby on Rails Guides <% end %> <%= guide('Ruby on Rails Guides Guidelines', 'ruby_on_rails_guides_guidelines.html') do %> -

This guide documentes the Ruby on Rails guides Guidelines.

+

This guide documents the Ruby on Rails guides guidelines.

<% end %> From 3b7d48cc455a04a40328e8f29b4200fdfb4f73b7 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 5 Oct 2010 22:52:43 +0200 Subject: [PATCH 78/85] a couple of touches to the guides guidelines --- railties/guides/source/ruby_on_rails_guides_guidelines.textile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/railties/guides/source/ruby_on_rails_guides_guidelines.textile b/railties/guides/source/ruby_on_rails_guides_guidelines.textile index 818d96da7989e..0bc409cbdad56 100644 --- a/railties/guides/source/ruby_on_rails_guides_guidelines.textile +++ b/railties/guides/source/ruby_on_rails_guides_guidelines.textile @@ -26,11 +26,10 @@ h5. When are Objects Saved? Use same typography as in regular text: -h3. Brief Note About +Test::Unit+ h6. The +:content_type+ Option -h3. API Documentation Guidelides +h3. API Documentation Guidelines The guides and the API should be coherent where appropriate. Please have a look at these particular sections of the "API Documentation Guidelines":api_documentation_guidelines.html: From 19a5f99685ab9f0a60bb8c1ed2a3fdb40b2e762e Mon Sep 17 00:00:00 2001 From: Erik Michaels-Ober Date: Tue, 5 Oct 2010 19:21:07 -0700 Subject: [PATCH 79/85] Fix copy/paste bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activesupport/lib/active_support/xml_mini.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/xml_mini.rb b/activesupport/lib/active_support/xml_mini.rb index 352172027b4c4..b6a8cf3caf54d 100644 --- a/activesupport/lib/active_support/xml_mini.rb +++ b/activesupport/lib/active_support/xml_mini.rb @@ -25,7 +25,7 @@ def content_type DEFAULT_ENCODINGS = { "binary" => "base64" - } unless defined?(TYPE_NAMES) + } unless defined?(DEFAULT_ENCODINGS) TYPE_NAMES = { "Symbol" => "symbol", From 990719bb59b89c614ec81aa25f2fc83f3323c9ea Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Tue, 5 Oct 2010 21:01:01 +0530 Subject: [PATCH 80/85] mailer comment should use namespace in comment --- actionmailer/lib/rails/generators/mailer/templates/mailer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionmailer/lib/rails/generators/mailer/templates/mailer.rb b/actionmailer/lib/rails/generators/mailer/templates/mailer.rb index 4d21c65101432..370a508cad906 100644 --- a/actionmailer/lib/rails/generators/mailer/templates/mailer.rb +++ b/actionmailer/lib/rails/generators/mailer/templates/mailer.rb @@ -6,7 +6,7 @@ class <%= class_name %> < ActionMailer::Base # Subject can be set in your I18n file at config/locales/en.yml # with the following lookup: # - # en.<%= file_name %>.<%= action %>.subject + # en.<%= file_path.gsub("/",".") %>.<%= action %>.subject # def <%= action %> @greeting = "Hi" From ae812e9fc9232a56daebd7e20f56a18981480d8c Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Wed, 6 Oct 2010 11:59:11 +0530 Subject: [PATCH 81/85] adding test for namedspaced mailers --- railties/test/generators/mailer_generator_test.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/railties/test/generators/mailer_generator_test.rb b/railties/test/generators/mailer_generator_test.rb index 450dec77165a6..f4fdc46328810 100644 --- a/railties/test/generators/mailer_generator_test.rb +++ b/railties/test/generators/mailer_generator_test.rb @@ -59,6 +59,15 @@ def test_logs_if_the_template_engine_cannot_be_found assert_match /haml \[not found\]/, content end + def test_mailer_with_namedspaced_mailer + run_generator ["Farm::Animal", "moos"] + assert_file "app/mailers/farm/animal.rb" do |mailer| + assert_match /class Farm::Animal < ActionMailer::Base/, mailer + assert_match /en\.farm\.animal\.moos\.subject/, mailer + end + assert_file "app/views/farm/animal/moos.text.erb" + end + def test_actions_are_turned_into_methods run_generator From 6774e12afa0f29442aa612ddf6e51d5a1b7a4356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 6 Oct 2010 11:07:09 +0200 Subject: [PATCH 82/85] Clean up the builder abstraction in AppGenerator. This commit may be reverted once documentation and a proper way to handle actions are added. --- .../generators/rails/app/app_generator.rb | 247 ++++-------------- .../test/generators/app_generator_test.rb | 68 +---- 2 files changed, 55 insertions(+), 260 deletions(-) diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 2715483914b55..be0969878783e 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -6,152 +6,12 @@ require 'uri' module Rails - module ActionMethods - attr_reader :options - - def initialize(generator) - @generator = generator - @options = generator.options - end - - private - %w(template copy_file directory empty_directory inside - empty_directory_with_gitkeep create_file chmod shebang).each do |method| - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method}(*args, &block) - @generator.send(:#{method}, *args, &block) - end - RUBY - end - - # TODO: Remove once this is fully in place - def method_missing(meth, *args, &block) - STDERR.puts "Calling #{meth} with #{args.inspect} with #{block}" - @generator.send(meth, *args, &block) - end - end - - class AppBuilder - def rakefile - template "Rakefile" - end - - def readme - copy_file "README" - end - - def gemfile - template "Gemfile" - end - - def configru - template "config.ru" - end - - def gitignore - copy_file "gitignore", ".gitignore" - end - - def app - directory 'app' - end - - def config - empty_directory "config" - - inside "config" do - template "routes.rb" - template "application.rb" - template "environment.rb" - - directory "environments" - directory "initializers" - directory "locales" - end - end - - def database_yml - template "config/databases/#{@options[:database]}.yml", "config/database.yml" - end - - def db - directory "db" - end - - def doc - directory "doc" - end - - def lib - empty_directory "lib" - empty_directory_with_gitkeep "lib/tasks" - end - - def log - empty_directory "log" - - inside "log" do - %w( server production development test ).each do |file| - create_file "#{file}.log" - chmod "#{file}.log", 0666, :verbose => false - end - end - end - - def public_directory - directory "public", "public", :recursive => false - end - - def images - directory "public/images" - end - - def stylesheets - empty_directory_with_gitkeep "public/stylesheets" - end - - def javascripts - unless options[:skip_prototype] - directory "public/javascripts" - else - empty_directory_with_gitkeep "public/javascripts" - create_file "public/javascripts/application.js" - end - end - - def script - directory "script" do |content| - "#{shebang}\n" + content - end - chmod "script", 0755, :verbose => false - end - - def test - directory "test" - end - - def tmp - empty_directory "tmp" - - inside "tmp" do - %w(sessions sockets cache pids).each do |dir| - empty_directory(dir) - end - end - end - - def vendor_plugins - empty_directory_with_gitkeep "vendor/plugins" - end - end - module Generators # We need to store the RAILS_DEV_PATH in a constant, otherwise the path # can change in Ruby 1.8.7 when we FileUtils.cd. RAILS_DEV_PATH = File.expand_path("../../../../../..", File.dirname(__FILE__)) - RESERVED_NAMES = %w[application destroy benchmarker profiler - plugin runner test] + RESERVED_NAMES = %w(application destroy benchmarker profiler plugin runner test) class AppGenerator < Base DATABASES = %w( mysql oracle postgresql sqlite3 frontbase ibm_db ) @@ -164,9 +24,6 @@ class AppGenerator < Base class_option :database, :type => :string, :aliases => "-d", :default => "sqlite3", :desc => "Preconfigure for selected database (options: #{DATABASES.join('/')})" - class_option :builder, :type => :string, :aliases => "-b", - :desc => "Path to an application builder (can be a filesystem path or URL)" - class_option :template, :type => :string, :aliases => "-m", :desc => "Path to an application template (can be a filesystem path or URL)" @@ -200,7 +57,6 @@ class AppGenerator < Base def initialize(*args) raise Error, "Options should be given after the application name. For details run: rails --help" if args[0].blank? - @original_wd = Dir.pwd super @@ -220,19 +76,29 @@ def create_root end def create_root_files - build(:readme) - build(:rakefile) - build(:configru) - build(:gitignore) unless options[:skip_git] - build(:gemfile) unless options[:skip_gemfile] + copy_file "README" + template "Rakefile" + template "config.ru" + copy_file "gitignore", ".gitignore" unless options[:skip_git] + template "Gemfile" unless options[:skip_gemfile] end def create_app_files - build(:app) + directory 'app' end def create_config_files - build(:config) + empty_directory "config" + + inside "config" do + template "routes.rb" + template "application.rb" + template "environment.rb" + + directory "environments" + directory "initializers" + directory "locales" + end end def create_boot_file @@ -241,59 +107,77 @@ def create_boot_file def create_active_record_files return if options[:skip_active_record] - build(:database_yml) + template "config/databases/#{@options[:database]}.yml", "config/database.yml" end def create_db_files - build(:db) + directory "db" end def create_doc_files - build(:doc) + directory "doc" end def create_lib_files - build(:lib) + empty_directory "lib" + empty_directory_with_gitkeep "lib/tasks" end def create_log_files - build(:log) + empty_directory "log" + + inside "log" do + %w( server production development test ).each do |file| + create_file "#{file}.log" + chmod "#{file}.log", 0666, :verbose => false + end + end end def create_public_files - build(:public_directory) + directory "public", "public", :recursive => false end def create_public_image_files - build(:images) + directory "public/images" end def create_public_stylesheets_files - build(:stylesheets) + empty_directory_with_gitkeep "public/stylesheets" end - def create_prototype_files - build(:javascripts) + def create_public_javascripts_files + unless options[:skip_prototype] + directory "public/javascripts" + else + empty_directory_with_gitkeep "public/javascripts" + create_file "public/javascripts/application.js" + end end def create_script_files - build(:script) + directory "script" do |content| + "#{shebang}\n" + content + end + chmod "script", 0755, :verbose => false end def create_test_files - build(:test) unless options[:skip_test_unit] + directory "test" unless options[:skip_test_unit] end def create_tmp_files - build(:tmp) - end + empty_directory "tmp" - def create_vendor_files - build(:vendor_plugins) + inside "tmp" do + %w(sessions sockets cache pids).each do |dir| + empty_directory(dir) + end + end end - def finish_template - build(:leftovers) + def create_vendor_files + empty_directory_with_gitkeep "vendor/plugins" end def apply_rails_template @@ -313,29 +197,6 @@ def self.banner "rails new #{self.arguments.map(&:usage).join(' ')} [options]" end - def builder - @builder ||= begin - if path = options[:builder] - if URI(path).is_a?(URI::HTTP) - contents = open(path, "Accept" => "application/x-thor-template") {|io| io.read } - else - contents = open(File.expand_path(path, @original_wd)) {|io| io.read } - end - - prok = eval("proc { #{contents} }", TOPLEVEL_BINDING, path, 1) - instance_eval(&prok) - end - - builder_class = defined?(::AppBuilder) ? ::AppBuilder : Rails::AppBuilder - builder_class.send(:include, ActionMethods) - builder_class.new(self) - end - end - - def build(meth, *args) - builder.send(meth, *args) if builder.respond_to?(meth) - end - def set_default_accessors! self.rails_template = case options[:template] when /^http:\/\// diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 3653b067c8acd..3bd8770710a9e 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -261,70 +261,4 @@ def action(*args, &block) silence(:stdout){ generator.send(*args, &block) } end -end - -class CustomAppGeneratorTest < Rails::Generators::TestCase - include GeneratorsTestHelper - tests Rails::Generators::AppGenerator - - arguments [destination_root] - - def setup - Rails.application = TestApp::Application - super - Rails::Generators::AppGenerator.instance_variable_set('@desc', nil) - @bundle_command = File.basename(Thor::Util.ruby_command).sub(/ruby/, 'bundle') - end - - def teardown - super - Rails::Generators::AppGenerator.instance_variable_set('@desc', nil) - Object.class_eval { remove_const :AppBuilder if const_defined?(:AppBuilder) } - Rails.application = TestApp::Application.instance - end - - def test_builder_option_with_empty_app_builder - FileUtils.cd(Rails.root) - run_generator([destination_root, "-b", "#{Rails.root}/lib/empty_builder.rb"]) - DEFAULT_APP_FILES.each{ |path| assert_no_file path } - end - - def test_builder_option_with_simple_app_builder - FileUtils.cd(Rails.root) - run_generator([destination_root, "-b", "#{Rails.root}/lib/simple_builder.rb"]) - (DEFAULT_APP_FILES - ['config.ru']).each{ |path| assert_no_file path } - assert_file "config.ru", %[run proc { |env| [200, { "Content-Type" => "text/html" }, ["Hello World"]] }] - end - - def test_builder_option_with_relative_path - here = File.expand_path(File.dirname(__FILE__)) - FileUtils.cd(here) - run_generator([destination_root, "-b", "../fixtures/lib/simple_builder.rb"]) - (DEFAULT_APP_FILES - ['config.ru']).each{ |path| assert_no_file path } - assert_file "config.ru", %[run proc { |env| [200, { "Content-Type" => "text/html" }, ["Hello World"]] }] - end - - def test_builder_option_with_tweak_app_builder - FileUtils.cd(Rails.root) - run_generator([destination_root, "-b", "#{Rails.root}/lib/tweak_builder.rb"]) - DEFAULT_APP_FILES.each{ |path| assert_file path } - assert_file "config.ru", %[run proc { |env| [200, { "Content-Type" => "text/html" }, ["Hello World"]] }] - end - - def test_builder_option_with_http - path = "http://gist.github.com/103208.txt" - template = "class AppBuilder; end" - template.instance_eval "def read; self; end" # Make the string respond to read - - generator([destination_root], :builder => path).expects(:open).with(path, 'Accept' => 'application/x-thor-template').returns(template) - capture(:stdout) { generator.invoke_all } - - DEFAULT_APP_FILES.each{ |path| assert_no_file path } - end - -protected - - def action(*args, &block) - silence(:stdout){ generator.send(*args, &block) } - end -end +end \ No newline at end of file From 848eb0d777d2f2907c399c2fe073fedffad14c63 Mon Sep 17 00:00:00 2001 From: wycats Date: Wed, 6 Oct 2010 02:53:00 -0700 Subject: [PATCH 83/85] Revert "Clean up the builder abstraction in AppGenerator." The phrase "clean up" misrepresents the fact that this removes a feature that ships with Rails 3.0. This reverts commit 6774e12afa0f29442aa612ddf6e51d5a1b7a4356. --- .../generators/rails/app/app_generator.rb | 247 ++++++++++++++---- .../test/generators/app_generator_test.rb | 68 ++++- 2 files changed, 260 insertions(+), 55 deletions(-) diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index be0969878783e..2715483914b55 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -6,12 +6,152 @@ require 'uri' module Rails + module ActionMethods + attr_reader :options + + def initialize(generator) + @generator = generator + @options = generator.options + end + + private + %w(template copy_file directory empty_directory inside + empty_directory_with_gitkeep create_file chmod shebang).each do |method| + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{method}(*args, &block) + @generator.send(:#{method}, *args, &block) + end + RUBY + end + + # TODO: Remove once this is fully in place + def method_missing(meth, *args, &block) + STDERR.puts "Calling #{meth} with #{args.inspect} with #{block}" + @generator.send(meth, *args, &block) + end + end + + class AppBuilder + def rakefile + template "Rakefile" + end + + def readme + copy_file "README" + end + + def gemfile + template "Gemfile" + end + + def configru + template "config.ru" + end + + def gitignore + copy_file "gitignore", ".gitignore" + end + + def app + directory 'app' + end + + def config + empty_directory "config" + + inside "config" do + template "routes.rb" + template "application.rb" + template "environment.rb" + + directory "environments" + directory "initializers" + directory "locales" + end + end + + def database_yml + template "config/databases/#{@options[:database]}.yml", "config/database.yml" + end + + def db + directory "db" + end + + def doc + directory "doc" + end + + def lib + empty_directory "lib" + empty_directory_with_gitkeep "lib/tasks" + end + + def log + empty_directory "log" + + inside "log" do + %w( server production development test ).each do |file| + create_file "#{file}.log" + chmod "#{file}.log", 0666, :verbose => false + end + end + end + + def public_directory + directory "public", "public", :recursive => false + end + + def images + directory "public/images" + end + + def stylesheets + empty_directory_with_gitkeep "public/stylesheets" + end + + def javascripts + unless options[:skip_prototype] + directory "public/javascripts" + else + empty_directory_with_gitkeep "public/javascripts" + create_file "public/javascripts/application.js" + end + end + + def script + directory "script" do |content| + "#{shebang}\n" + content + end + chmod "script", 0755, :verbose => false + end + + def test + directory "test" + end + + def tmp + empty_directory "tmp" + + inside "tmp" do + %w(sessions sockets cache pids).each do |dir| + empty_directory(dir) + end + end + end + + def vendor_plugins + empty_directory_with_gitkeep "vendor/plugins" + end + end + module Generators # We need to store the RAILS_DEV_PATH in a constant, otherwise the path # can change in Ruby 1.8.7 when we FileUtils.cd. RAILS_DEV_PATH = File.expand_path("../../../../../..", File.dirname(__FILE__)) - RESERVED_NAMES = %w(application destroy benchmarker profiler plugin runner test) + RESERVED_NAMES = %w[application destroy benchmarker profiler + plugin runner test] class AppGenerator < Base DATABASES = %w( mysql oracle postgresql sqlite3 frontbase ibm_db ) @@ -24,6 +164,9 @@ class AppGenerator < Base class_option :database, :type => :string, :aliases => "-d", :default => "sqlite3", :desc => "Preconfigure for selected database (options: #{DATABASES.join('/')})" + class_option :builder, :type => :string, :aliases => "-b", + :desc => "Path to an application builder (can be a filesystem path or URL)" + class_option :template, :type => :string, :aliases => "-m", :desc => "Path to an application template (can be a filesystem path or URL)" @@ -57,6 +200,7 @@ class AppGenerator < Base def initialize(*args) raise Error, "Options should be given after the application name. For details run: rails --help" if args[0].blank? + @original_wd = Dir.pwd super @@ -76,29 +220,19 @@ def create_root end def create_root_files - copy_file "README" - template "Rakefile" - template "config.ru" - copy_file "gitignore", ".gitignore" unless options[:skip_git] - template "Gemfile" unless options[:skip_gemfile] + build(:readme) + build(:rakefile) + build(:configru) + build(:gitignore) unless options[:skip_git] + build(:gemfile) unless options[:skip_gemfile] end def create_app_files - directory 'app' + build(:app) end def create_config_files - empty_directory "config" - - inside "config" do - template "routes.rb" - template "application.rb" - template "environment.rb" - - directory "environments" - directory "initializers" - directory "locales" - end + build(:config) end def create_boot_file @@ -107,77 +241,59 @@ def create_boot_file def create_active_record_files return if options[:skip_active_record] - template "config/databases/#{@options[:database]}.yml", "config/database.yml" + build(:database_yml) end def create_db_files - directory "db" + build(:db) end def create_doc_files - directory "doc" + build(:doc) end def create_lib_files - empty_directory "lib" - empty_directory_with_gitkeep "lib/tasks" + build(:lib) end def create_log_files - empty_directory "log" - - inside "log" do - %w( server production development test ).each do |file| - create_file "#{file}.log" - chmod "#{file}.log", 0666, :verbose => false - end - end + build(:log) end def create_public_files - directory "public", "public", :recursive => false + build(:public_directory) end def create_public_image_files - directory "public/images" + build(:images) end def create_public_stylesheets_files - empty_directory_with_gitkeep "public/stylesheets" + build(:stylesheets) end - def create_public_javascripts_files - unless options[:skip_prototype] - directory "public/javascripts" - else - empty_directory_with_gitkeep "public/javascripts" - create_file "public/javascripts/application.js" - end + def create_prototype_files + build(:javascripts) end def create_script_files - directory "script" do |content| - "#{shebang}\n" + content - end - chmod "script", 0755, :verbose => false + build(:script) end def create_test_files - directory "test" unless options[:skip_test_unit] + build(:test) unless options[:skip_test_unit] end def create_tmp_files - empty_directory "tmp" - - inside "tmp" do - %w(sessions sockets cache pids).each do |dir| - empty_directory(dir) - end - end + build(:tmp) end def create_vendor_files - empty_directory_with_gitkeep "vendor/plugins" + build(:vendor_plugins) + end + + def finish_template + build(:leftovers) end def apply_rails_template @@ -197,6 +313,29 @@ def self.banner "rails new #{self.arguments.map(&:usage).join(' ')} [options]" end + def builder + @builder ||= begin + if path = options[:builder] + if URI(path).is_a?(URI::HTTP) + contents = open(path, "Accept" => "application/x-thor-template") {|io| io.read } + else + contents = open(File.expand_path(path, @original_wd)) {|io| io.read } + end + + prok = eval("proc { #{contents} }", TOPLEVEL_BINDING, path, 1) + instance_eval(&prok) + end + + builder_class = defined?(::AppBuilder) ? ::AppBuilder : Rails::AppBuilder + builder_class.send(:include, ActionMethods) + builder_class.new(self) + end + end + + def build(meth, *args) + builder.send(meth, *args) if builder.respond_to?(meth) + end + def set_default_accessors! self.rails_template = case options[:template] when /^http:\/\// diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 3bd8770710a9e..3653b067c8acd 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -261,4 +261,70 @@ def action(*args, &block) silence(:stdout){ generator.send(*args, &block) } end -end \ No newline at end of file +end + +class CustomAppGeneratorTest < Rails::Generators::TestCase + include GeneratorsTestHelper + tests Rails::Generators::AppGenerator + + arguments [destination_root] + + def setup + Rails.application = TestApp::Application + super + Rails::Generators::AppGenerator.instance_variable_set('@desc', nil) + @bundle_command = File.basename(Thor::Util.ruby_command).sub(/ruby/, 'bundle') + end + + def teardown + super + Rails::Generators::AppGenerator.instance_variable_set('@desc', nil) + Object.class_eval { remove_const :AppBuilder if const_defined?(:AppBuilder) } + Rails.application = TestApp::Application.instance + end + + def test_builder_option_with_empty_app_builder + FileUtils.cd(Rails.root) + run_generator([destination_root, "-b", "#{Rails.root}/lib/empty_builder.rb"]) + DEFAULT_APP_FILES.each{ |path| assert_no_file path } + end + + def test_builder_option_with_simple_app_builder + FileUtils.cd(Rails.root) + run_generator([destination_root, "-b", "#{Rails.root}/lib/simple_builder.rb"]) + (DEFAULT_APP_FILES - ['config.ru']).each{ |path| assert_no_file path } + assert_file "config.ru", %[run proc { |env| [200, { "Content-Type" => "text/html" }, ["Hello World"]] }] + end + + def test_builder_option_with_relative_path + here = File.expand_path(File.dirname(__FILE__)) + FileUtils.cd(here) + run_generator([destination_root, "-b", "../fixtures/lib/simple_builder.rb"]) + (DEFAULT_APP_FILES - ['config.ru']).each{ |path| assert_no_file path } + assert_file "config.ru", %[run proc { |env| [200, { "Content-Type" => "text/html" }, ["Hello World"]] }] + end + + def test_builder_option_with_tweak_app_builder + FileUtils.cd(Rails.root) + run_generator([destination_root, "-b", "#{Rails.root}/lib/tweak_builder.rb"]) + DEFAULT_APP_FILES.each{ |path| assert_file path } + assert_file "config.ru", %[run proc { |env| [200, { "Content-Type" => "text/html" }, ["Hello World"]] }] + end + + def test_builder_option_with_http + path = "http://gist.github.com/103208.txt" + template = "class AppBuilder; end" + template.instance_eval "def read; self; end" # Make the string respond to read + + generator([destination_root], :builder => path).expects(:open).with(path, 'Accept' => 'application/x-thor-template').returns(template) + capture(:stdout) { generator.invoke_all } + + DEFAULT_APP_FILES.each{ |path| assert_no_file path } + end + +protected + + def action(*args, &block) + silence(:stdout){ generator.send(*args, &block) } + end +end From 0904e8256864239f673bf91fce1cfffb9345ee61 Mon Sep 17 00:00:00 2001 From: wycats Date: Wed, 6 Oct 2010 02:58:49 -0700 Subject: [PATCH 84/85] Delegate everything to the generator --- .../generators/rails/app/app_generator.rb | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 2715483914b55..d2ab0988850cb 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -14,21 +14,11 @@ def initialize(generator) @options = generator.options end - private - %w(template copy_file directory empty_directory inside - empty_directory_with_gitkeep create_file chmod shebang).each do |method| - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method}(*args, &block) - @generator.send(:#{method}, *args, &block) - end - RUBY - end + private - # TODO: Remove once this is fully in place - def method_missing(meth, *args, &block) - STDERR.puts "Calling #{meth} with #{args.inspect} with #{block}" - @generator.send(meth, *args, &block) - end + def method_missing(meth, *args, &block) + @generator.send(meth, *args, &block) + end end class AppBuilder From d40ca9cce241a8083756c993d6c99a79e62e050e Mon Sep 17 00:00:00 2001 From: wycats Date: Wed, 6 Oct 2010 03:06:12 -0700 Subject: [PATCH 85/85] Some initial docs --- railties/lib/rails/generators/rails/app/app_generator.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index d2ab0988850cb..7907191c741c8 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -21,6 +21,13 @@ def method_missing(meth, *args, &block) end end + # The application builder allows you to override elements of the application + # generator without being forced to reverse the operations of the default + # generator. + # + # This allows you to override entire operations, like the creation of the + # Gemfile, README, or javascript files, without needing to know exactly + # what those operations do so you can create another template action. class AppBuilder def rakefile template "Rakefile"