From ab9622ee62e4ae6372fab69b9b148875be573a22 Mon Sep 17 00:00:00 2001 From: oleg dashevskii Date: Wed, 25 Aug 2010 15:52:00 +0700 Subject: 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 afef31396e..3f963fc89a 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -275,6 +275,17 @@ class CalculationsTest < ActiveRecord::TestCase 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 @@ class CalculationsTest < ActiveRecord::TestCase def test_from_option_with_table_different_than_class assert_equal Account.count(:all), Company.count(:all, :from => 'accounts') end - end -- cgit v1.2.3 From cc18034c8c182df51a4fcb1a0a9b662f97d1f53d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 09:37:17 -0700 Subject: 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 3f963fc89a..7ec40906d4 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -281,7 +281,7 @@ class CalculationsTest < ActiveRecord::TestCase 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 -- cgit v1.2.3 From a8a62f87f6777b1bad8f38cc4b0239b74a4f8a7a Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 10:16:35 -0700 Subject: [#5441 state:resolved] refactoring code to determine aggregate column --- .../lib/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 03862c78e4..d79ef78b4d 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -183,12 +183,16 @@ module ActiveRecord 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 @@ module ActiveRecord 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) -- cgit v1.2.3 From ef8ce78ba1d37c7d4246f0c6b2cf13140c2898f8 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 11:00:14 -0700 Subject: 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 bed95ef524..fc1a7b8c6f 100644 --- a/activeresource/test/cases/format_test.rb +++ b/activeresource/test/cases/format_test.rb @@ -33,7 +33,7 @@ class FormatTest < Test::Unit::TestCase 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 -- cgit v1.2.3 From 505b532605bd3b9b8a444be10911f0864f1d42ba Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 12:13:12 -0700 Subject: 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 2157a0aded..9204295d8c 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -884,13 +884,9 @@ module ActiveRecord #: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 @@ MSG 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. -- cgit v1.2.3 From ef6df93a8ddb675f1298973bb347825c554e95f9 Mon Sep 17 00:00:00 2001 From: Marcelo Giorgi Date: Thu, 30 Sep 2010 14:56:11 -0300 Subject: AssociationCollection#include? working properly for objects added with build method [#3472 state:resolved] --- .../active_record/associations/association_collection.rb | 13 +++++++++++++ .../has_and_belongs_to_many_associations_test.rb | 6 ++++++ .../test/cases/associations/has_many_associations_test.rb | 6 ++++++ .../associations/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 91e0a9f2f8..a617a0fb36 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -363,6 +363,7 @@ module ActiveRecord 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 @@ module ActiveRecord 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 c0be7dfdcc..7e070e1746 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 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase 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 720b7fc386..c9f00fd737 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 @@ class HasManyAssociationsTest < ActiveRecord::TestCase 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 0dac633852..4b9f49f1ec 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 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase 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 -- cgit v1.2.3 From fb4ee9c7e5da783d84d3a633297830ceac58ee48 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 14:22:09 -0700 Subject: 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 9204295d8c..aed4eff565 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -890,7 +890,7 @@ module ActiveRecord #:nodoc: 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 -- cgit v1.2.3 From 15419a5dc692be997d7637e40435a3d0d0fa4c1c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 14:35:36 -0700 Subject: 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 2e0a2effc2..c8174b5f45 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -196,6 +196,8 @@ module ActiveRecord arel end + private + def build_where(opts, other = []) case opts when String, Array @@ -208,8 +210,6 @@ module ActiveRecord end end - private - def build_joins(relation, joins) association_joins = [] -- cgit v1.2.3 From 0238228e5d3ce1747a6f1d618693ffd44f947561 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 16:02:49 -0700 Subject: 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 aed4eff565..9204295d8c 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -890,7 +890,7 @@ module ActiveRecord #:nodoc: 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 8c09fc4d59..31679b2efe 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -14,6 +14,20 @@ class InheritanceTest < ActiveRecord::TestCase 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 -- cgit v1.2.3 From 45edeed1ee5148e837ea9680fa0c40e4b151d6d7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 30 Sep 2010 16:28:12 -0700 Subject: 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 e2f2508ae8..f692e5ac89 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -118,7 +118,7 @@ module ActiveRecord 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 9204295d8c..80ddd5e7ab 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -834,7 +834,7 @@ module ActiveRecord #:nodoc: 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 -- cgit v1.2.3 From 542ddd8c891e27278cdac26763b6ff8aadbfdf68 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Fri, 1 Oct 2010 23:16:05 +0200 Subject: 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 +- .../rails/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 92b9131b59..9eae712a93 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 1dd112b4a6..1de78eecae 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 %> -- cgit v1.2.3 From ff2fdcc52b391514cb62c2a1ef29827ac94914c6 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:22:42 -0700 Subject: removing AS::Testing::Default in favor of just undefing default_test --- actionpack/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 33a5c68b9d..d6c98b4db7 100644 --- a/actionpack/lib/action_dispatch/testing/performance_test.rb +++ b/actionpack/lib/action_dispatch/testing/performance_test.rb @@ -11,9 +11,8 @@ begin # 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 ed8c02ba3e..fb52fc7083 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -24,8 +24,7 @@ module ActiveSupport 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 a0bd6303c7..0000000000 --- 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 cdaf63961a..ee5a20c789 100644 --- a/activesupport/test/test_test.rb +++ b/activesupport/test/test_test.rb @@ -126,13 +126,6 @@ class AssertPresentTest < ActiveSupport::TestCase 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 -- cgit v1.2.3 From 61e8b23fe599ac54382251b44d9690f08a759fe1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:37:34 -0700 Subject: 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 f1103029c8..6ecedc26ef 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -90,10 +90,7 @@ module Enumerable # => { "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. -- cgit v1.2.3 From dfa331ae154c0475dfc631528071bdb06947acc2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:54:50 -0700 Subject: 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 4ff39fb76c..f0d62b0b13 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -167,7 +167,7 @@ end 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 -- cgit v1.2.3 From 6e1df2ca4631b8da6dae348af9d791f6e9aee1c5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:55:39 -0700 Subject: 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 f32b562368..9eee3fc5e3 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 @@ module ActiveSupport 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| -- cgit v1.2.3 From 44f85678e967f1eccfaf448f82ca81111c9584af Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:01:34 -0700 Subject: 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 3540af13ac..bf6d43da08 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -308,3 +308,21 @@ module ActionView 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 b00142c92d..92d4a6d98b 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -3,24 +3,6 @@ require 'abstract_unit' 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 db8fd82aeb..98276da559 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -547,24 +547,6 @@ class LinkToUnlessCurrentWithControllerTest < ActionController::TestCase 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 -- cgit v1.2.3 From ffbcb84c215bb615a3db4bb8bf8dcb977e72e32b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:05:59 -0700 Subject: removing more duplicate code --- actionpack/test/abstract_unit.rb | 17 +++++++++++++++++ actionpack/test/controller/rescue_test.rb | 16 ---------------- actionpack/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 bf6d43da08..470b36dbe2 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -326,3 +326,20 @@ class Workshop 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 a2418bb7c0..c445285538 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 4ede1ab47c..ce6c397e32 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) -- cgit v1.2.3 From 50cf5c11a1b33cd082bbdf4f253581109955797c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:07:19 -0700 Subject: 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 10873708fe..90c944d890 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -151,8 +151,8 @@ class ACLogSubscriberTest < ActionController::TestCase 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 -- cgit v1.2.3 From 3eb7f9adee4f606ac65e8e3d25098411b5a787b7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:09:37 -0700 Subject: removing more duplicate code. :'( --- actionpack/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 835a0e970b..e46e317e31 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 -- cgit v1.2.3 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: 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 915c2f90d7..0eb4a663de 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -192,7 +192,11 @@ module ActionView 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 -- cgit v1.2.3 From 4e93179ed3f44825c157b54517e5a256f5725a55 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Thu, 30 Sep 2010 20:28:22 -0300 Subject: 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 --- .../lib/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 a617a0fb36..cb2d9e0a79 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -558,11 +558,10 @@ module ActiveRecord 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 -- cgit v1.2.3 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: Fix a routing test. Reorganize middleware tests. --- .../application/middleware/best_practices_test.rb | 26 +++++++ railties/test/application/middleware/cache_test.rb | 12 --- .../test/application/middleware/remote_ip_test.rb | 63 +++++++++++++++ .../test/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 0000000000..5b722e7510 --- /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 5675cebfd9..f582ed0e42 100644 --- a/railties/test/application/middleware/cache_test.rb +++ b/railties/test/application/middleware/cache_test.rb @@ -11,18 +11,6 @@ module ApplicationTests 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 0000000000..f28302d70a --- /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 0000000000..0128261cd4 --- /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 f9b594eb33..2372ad85b8 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -129,81 +129,6 @@ module ApplicationTests 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 @@ module ApplicationTests 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 416a5de5b0..e42f5720a9 100644 --- a/railties/test/application/routing_test.rb +++ b/railties/test/application/routing_test.rb @@ -11,34 +11,6 @@ module ApplicationTests 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 @@ module ApplicationTests 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 @@ module ApplicationTests 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 @@ module ApplicationTests 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 @@ module ApplicationTests 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 79c7735019..3b03e4eb3d 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -45,6 +45,17 @@ module TestHelpers 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 @@ module TestHelpers 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 -- cgit v1.2.3 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: 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 aafbbc29ee..4d04184b20 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -81,6 +81,10 @@ module Rails 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 23b72a0ec6..6da903c1ac 100644 --- a/railties/lib/rails/application/routes_reloader.rb +++ b/railties/lib/rails/application/routes_reloader.rb @@ -8,7 +8,7 @@ module Rails def blocks @blocks ||= {} end - private + def reload! clear! load_blocks @@ -18,6 +18,8 @@ module Rails revert end + protected + def clear! routers.each do |routes| routes.disable_clear_and_finalize = true @@ -32,7 +34,7 @@ module Rails 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 e42f5720a9..62589c998d 100644 --- a/railties/test/application/routing_test.rb +++ b/railties/test/application/routing_test.rb @@ -209,7 +209,7 @@ module ApplicationTests 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 @@ module ApplicationTests 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| -- cgit v1.2.3 From 8d1df887d32643f0aee2d71f7216ec98fdfe4fdb Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Wed, 29 Sep 2010 18:15:36 +0530 Subject: 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 1836baaf12..3cd8b02bc4 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -791,7 +791,7 @@ module ActionView 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". -- cgit v1.2.3 From 297cf0b26658ff9c7f19fc703de5bd8939e31d06 Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Sat, 2 Oct 2010 17:39:12 +0530 Subject: 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 abc98ebe69..8809e510fb 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -761,6 +761,20 @@ class FormHelperTest < ActionView::TestCase 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 @@ class FormHelperTest < ActionView::TestCase def protect_against_forgery? false end + end -- cgit v1.2.3 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: :'' 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 272e179fe3..064546a3f3 100644 --- a/railties/test/generators/generated_attribute_test.rb +++ b/railties/test/generators/generated_attribute_test.rb @@ -117,7 +117,7 @@ class GeneratedAttributeTest < Rails::Generators::TestCase def test_missing_type_raises_exception assert_raise Thor::Error do - create_generated_attribute(:'', 'title') + create_generated_attribute('', 'title') end end end -- cgit v1.2.3 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: 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 d4d87be527..b69c0e1c53 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 09650789ac..2b68a3c453 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 e0e4324a4a..3d0af185a2 100644 --- a/railties/lib/rails/railtie/configuration.rb +++ b/railties/lib/rails/railtie/configuration.rb @@ -5,7 +5,6 @@ module Rails 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 @@ module Rails # 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 @@ module Rails # 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 -- cgit v1.2.3 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: 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 3d0af185a2..afeceafb67 100644 --- a/railties/lib/rails/railtie/configuration.rb +++ b/railties/lib/rails/railtie/configuration.rb @@ -27,7 +27,7 @@ module Rails 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 -- cgit v1.2.3 From f656796d05715174568536cfe119a3959a020f23 Mon Sep 17 00:00:00 2001 From: David Chelimsky Date: Sat, 2 Oct 2010 12:35:17 -0500 Subject: 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 0eb4a663de..ac59c16d7c 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -103,7 +103,7 @@ module ActionView 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 @@ module ActionView 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 @@ module ActionView @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 8526db61cc..a745999622 100644 --- a/actionpack/test/template/test_case_test.rb +++ b/actionpack/test/template/test_case_test.rb @@ -116,6 +116,37 @@ module ActionView 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 -- cgit v1.2.3 From c28bebef13b8a0e497fc7bbb83f542e9400e07e5 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 13:34:34 -0200 Subject: 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 ac59c16d7c..731f91df30 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -200,9 +200,9 @@ module ActionView # 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 -- cgit v1.2.3 From 10249d7b7728525fb7660409e3bbc9e0286dbf0f Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 14:52:46 -0200 Subject: 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 b8c15df263..a9cdf6c0ca 100644 --- a/actionmailer/lib/action_mailer/old_api.rb +++ b/actionmailer/lib/action_mailer/old_api.rb @@ -247,7 +247,7 @@ module ActionMailer [ 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 -- cgit v1.2.3 From 49203126430c6dbaf9ce47c21c58c4081fb53e82 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 14:58:18 -0200 Subject: 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 a9cdf6c0ca..0396f0775e 100644 --- a/actionmailer/lib/action_mailer/old_api.rb +++ b/actionmailer/lib/action_mailer/old_api.rb @@ -248,7 +248,7 @@ module ActionMailer 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 -- cgit v1.2.3 From 42fad8c82b3ba6f86c84f06645cc1e39a8a776dd Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 15:03:59 -0200 Subject: 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 d3b19ee560..a373e53f11 100644 --- a/activeresource/lib/active_resource/validations.rb +++ b/activeresource/lib/active_resource/validations.rb @@ -13,7 +13,7 @@ module ActiveResource # 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} " -- cgit v1.2.3 From 5836af8f8b0eb3c569c66792abf50a0485bb6f22 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 16:33:46 -0200 Subject: PERF: more Hash + map changes --- .../lib/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 7a6a0be69d..af30bfc13a 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 @@ class Class # :nodoc: 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) -- cgit v1.2.3 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: 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 +--- .../test/dispatch/session/cookie_store_test.rb | 12 - activerecord/lib/active_record/session_store.rb | 7 +- 9 files changed, 90 insertions(+), 346 deletions(-) diff --git a/Gemfile b/Gemfile index 9d29c42e1c..7fe1c3ad22 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 6f8314109b..6352b97a6b 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 1af75fc2d7..d7b54c2abc 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -187,15 +187,17 @@ module ActionController 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 3e5cd6a2f9..cfee95eb4b 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -18,11 +18,6 @@ module ActionDispatch @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 db0187c015..679ba7fc8e 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 ActionDispatch 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 ca1494425f..9c9ccc62f5 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 ActionDispatch # "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 28e3dbd732..4dd9a946c2 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 3489f628ed..256d0781c7 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -53,18 +53,6 @@ class CookieStoreTest < ActionDispatch::IntegrationTest 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 01cc14b8d6..3fc596e02a 100644 --- a/activerecord/lib/active_record/session_store.rb +++ b/activerecord/lib/active_record/session_store.rb @@ -288,6 +288,7 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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) -- cgit v1.2.3 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: Move ETag and ConditionalGet logic from AD::Response to the middleware stack. --- actionpack/lib/action_dispatch/http/cache.rb | 22 +---- actionpack/lib/action_dispatch/http/response.rb | 2 +- actionpack/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 047fab006e..4061222d11 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -50,8 +50,7 @@ module ActionDispatch 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 @@ module ActionDispatch 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 151c90167b..72871e328a 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -132,7 +132,7 @@ module ActionDispatch # :nodoc: # 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 2bca5aec6a..0000000000 --- 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 7ca784c467..fca8de60bc 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -99,11 +99,6 @@ class TestController < ActionController::Base 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 @@ class ExpiresInRenderTest < ActionController::TestCase 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 cd0418c338..be6398fead 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -11,9 +11,7 @@ class ResponseTest < ActiveSupport::TestCase 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 @@ class ResponseTest < ActiveSupport::TestCase 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 @@ class ResponseTest < ActiveSupport::TestCase 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 4d04184b20..075e3c5692 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -145,8 +145,8 @@ module Rails 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 @@ module Rails 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 2372ad85b8..173ac40b12 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -36,6 +36,8 @@ module ApplicationTests "ActionDispatch::ParamsParser", "Rack::MethodOverride", "ActionDispatch::Head", + "Rack::ConditionalGet", + "Rack::ETag", "ActionDispatch::BestStandardsSupport" ], middleware end @@ -45,27 +47,7 @@ module ApplicationTests 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 @@ module ApplicationTests 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) -- cgit v1.2.3 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: 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 d7b54c2abc..6061945622 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -191,6 +191,7 @@ module ActionController 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 09d6ba8223..bbcdefb190 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -199,7 +199,7 @@ module ActionDispatch # 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 -- cgit v1.2.3 From adfd43a4daf08cc9a801a0b6a039dd109ce4e81f Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Thu, 30 Sep 2010 19:17:36 +0200 Subject: 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 9ae235b818..ba89bce928 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. -- cgit v1.2.3 From 18a7b767e8ad545702c1025fc9cc7a1cc3c64f28 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 14:11:34 -0700 Subject: moving fake model to the correct file --- actionpack/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 e46e317e31..f3e5ff8a47 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 8cb3b4940a..cf3f2f7fa4 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -130,6 +130,20 @@ class CommentRelevance 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 -- cgit v1.2.3 From 83633b807a3b08aaf5554c0fc793583a064c2472 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 14:12:06 -0700 Subject: avoid creating objects when we can --- activerecord/lib/active_record/base.rb | 3 +-- .../lib/active_record/relation/predicate_builder.rb | 16 +++++----------- activerecord/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 80ddd5e7ab..f6d9050828 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1270,8 +1270,7 @@ MSG 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 0d1307d87e..c5428dccd6 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 @@ module ActiveRecord 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 c8174b5f45..001207514d 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -204,7 +204,7 @@ module ActiveRecord [@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 -- cgit v1.2.3 From bd78d24bd8168b43ceb167e8d8b3e542486a1bba Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 14:27:54 -0700 Subject: 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 f6d9050828..ff6be4ff19 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1270,7 +1270,10 @@ MSG 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 -- cgit v1.2.3 From 7836616a6473527f1f42672e54cf6971c05f6fdf Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 15:18:32 -0700 Subject: 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 6faa88ab78..437f01b657 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -176,9 +176,11 @@ HEADER 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) -- cgit v1.2.3 From 5154a464cc405438336739ba0d563f87d9fc2d96 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 15:35:56 -0700 Subject: 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 437f01b657..f5331bb8a9 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -183,8 +183,8 @@ HEADER ] 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 -- cgit v1.2.3 From a6c42c82678f11271fe71923a200eb135f2f1c0e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 15:38:17 -0700 Subject: 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 731f91df30..4026f7a40e 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -201,7 +201,7 @@ module ActionView # 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 -- cgit v1.2.3 From 8beda11fd3cbd2db21a7a7a7ae9823edbbc2dd5e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:13:45 -0700 Subject: 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 e708b3fbcf..b075632de6 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -570,7 +570,7 @@ module ActiveRecord 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 @@ module ActiveRecord 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 -- cgit v1.2.3 From 341e71a1b9a3d0ad367f79ff89b1c97fe6890905 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:23:07 -0700 Subject: 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 b075632de6..6744a0783a 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -584,11 +584,13 @@ module ActiveRecord 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 -- cgit v1.2.3 From e6583901e58afe58aeb07f1ba1bd9f103492b98b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:32:27 -0700 Subject: 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 6744a0783a..5458bba51f 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -419,9 +419,12 @@ module ActiveRecord # 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 @@ module ActiveRecord 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 -- cgit v1.2.3 From 40761c4bf39826254b7a5266210f91742591f58c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:36:43 -0700 Subject: 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 5458bba51f..3bfd1851b5 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -509,6 +509,7 @@ module ActiveRecord 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 @@ module ActiveRecord 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 -- cgit v1.2.3 From 365c93b7cdaa161c77c157c7cc385221ebbc33c7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:39:48 -0700 Subject: 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 3bfd1851b5..7a1504430b 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -507,6 +507,8 @@ module ActiveRecord 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 @@ module ActiveRecord 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 -- cgit v1.2.3 From 69a2c6b0419177448d9811745cf4035d46b68bbd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 16:41:59 -0700 Subject: 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 7a1504430b..9ac18f9939 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -509,20 +509,19 @@ module ActiveRecord 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) -- cgit v1.2.3 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: Initialize sid should just skip instance variables. --- .../lib/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 679ba7fc8e..64d3a87fd0 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 @@ module ActionDispatch 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 -- cgit v1.2.3 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: 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 73acb73dec..d26c1bcdbc 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 -- cgit v1.2.3 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: 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 cf13938331..913b899e20 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -6,13 +6,13 @@ module ActionDispatch @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 2eb82fc5d8..655745a848 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -2,30 +2,37 @@ require 'abstract_unit' 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 @@ class MultipleDirectorisStaticTest < ActiveSupport::TestCase 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 -- cgit v1.2.3 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: 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 1dbf27d978..40213b1261 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] -%> -- cgit v1.2.3 From ccf228b0276afb8bb00d14f1b861015b1ef5ab76 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 09:28:21 -0700 Subject: [#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 0000000000..8a5006cdf2 --- /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 -- cgit v1.2.3 From b7c49cedba4a9acd001df65a102d79611598f472 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 09:29:37 -0700 Subject: 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 8a5006cdf2..7e65c63062 100644 --- a/activesupport/test/test_case_test.rb +++ b/activesupport/test/test_case_test.rb @@ -33,6 +33,25 @@ module ActiveSupport 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 -- cgit v1.2.3 From 33aaa15f6208d65ca043ffcc5dc161d410963e9d Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Mon, 4 Oct 2010 13:35:38 -0400 Subject: 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 2d580fd325..adb71f788f 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -129,7 +129,11 @@ module ActiveModel 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 -- cgit v1.2.3 From 21cb1d40b9c57ab814c69d01a242b50e27e8283b Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Mon, 4 Oct 2010 15:04:39 -0400 Subject: 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 40ce4c0e2d..a7dde2c433 100644 --- a/activemodel/test/cases/naming_test.rb +++ b/activemodel/test/cases/naming_test.rb @@ -125,6 +125,10 @@ class NamingHelpersTest < Test::Unit::TestCase @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 d137e4ff8f..545acd1655 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 -- cgit v1.2.3 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: 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 545acd1655..768c96ecf0 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 -- cgit v1.2.3 From d8135eb452bfb956e093c61cf12adb9b0da6e28b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 15:19:27 -0700 Subject: * + 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 194842a9a0..95b6a8137d 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 @@ module ActiveRecord 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 -- cgit v1.2.3 From e7d860c6bed99b0d44680097fcc1cfd7c1fa07ef Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 15:25:20 -0700 Subject: create fewer objects, call fewer methods in extract_pg_identifier_from_name --- .../lib/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 95b6a8137d..5f14284615 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 @@ module ActiveRecord 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 -- cgit v1.2.3 From 28bb1885f5a35d0adecd35d38b73751d737891c4 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:08:01 -0700 Subject: 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 47aed0273c..bf10f81127 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -171,13 +171,13 @@ module ActionDispatch 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 -- cgit v1.2.3 From f9734f2b0f5326c399d1c1cccba8b6d8e7d9bdd4 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:30:16 -0700 Subject: adding tests for uploaded file --- actionpack/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 0000000000..def6289fb3 --- /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 -- cgit v1.2.3 From 2a3022db7f2ddc0fc0e678ea757f97902c5f5c01 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:56:45 -0700 Subject: 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 49465d820e..bfbe7c5305 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -2,27 +2,19 @@ require 'active_support/core_ext/object/blank' 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 def6289fb3..d04d5a8650 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -21,5 +21,23 @@ module ActionDispatch 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 -- cgit v1.2.3 From 876acf001a32887679e259f83a2fbad750ac2e67 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:57:11 -0700 Subject: if it walks like a duck and talks like a duck, it must be a duck --- actionpack/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 073dd3ddad..3ff558ec5a 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 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest 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 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest 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 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest 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 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest 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 -- cgit v1.2.3 From 8a9747021085c569f0118db1093bc12cfa2ba915 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 17:08:25 -0700 Subject: 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 bfbe7c5305..d4fabe1eaf 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -10,6 +10,7 @@ module ActionDispatch @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 d04d5a8650..4173ce0a44 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -2,18 +2,24 @@ require 'abstract_unit' 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 -- cgit v1.2.3 From 3370ad0b1e883c9ec24c771f6c52b296a71eff40 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 17:11:50 -0700 Subject: 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 d4fabe1eaf..53f8039121 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -13,7 +13,12 @@ module ActionDispatch 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 4173ce0a44..c81797a73f 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -45,5 +45,20 @@ module ActionDispatch 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 -- cgit v1.2.3 From 12173396163616e077f761e190c13beb43d536bd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:28:40 -0700 Subject: 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 ++++++++----- actionpack/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 53f8039121..84e58d7d6a 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -13,13 +13,16 @@ module ActionDispatch 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 c81797a73f..b51697b930 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -29,36 +29,30 @@ module ActionDispatch 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 -- cgit v1.2.3 From 5769636663c2849fd132fd593c0f6d29702b7cf1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:32:49 -0700 Subject: 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 dfc90943e1..3a8a37d967 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -758,12 +758,12 @@ class ControllerWithSymbolAsFilter < PostsController 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 cf3f2f7fa4..dba632e6df 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 -- cgit v1.2.3 From 333a5659e8663049618386e3fa45248d388070fd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:46:38 -0700 Subject: 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 1c3ca78d28..440e0162cd 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -476,18 +476,16 @@ module ActionView 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 = '' -- cgit v1.2.3 From 714fea4540c96f70d044e2bd1be92b504d1f8fa3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:52:17 -0700 Subject: 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 440e0162cd..0eed3ea259 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -495,13 +495,11 @@ module ActionView 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 -- cgit v1.2.3 From 839e2f96647d5b29cc2865555a6f615c31429109 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 20:00:55 -0700 Subject: 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 0eed3ea259..651240a122 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -501,10 +501,10 @@ module ActionView 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) -- cgit v1.2.3 From e3acdcfbf349e416c63f4c56170e9d82f7b1b1d0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 20:05:56 -0700 Subject: refactoring to use fewer intermediate variables --- actionpack/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 651240a122..da42d94318 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -487,24 +487,25 @@ module ActionView 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) -- cgit v1.2.3 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: 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 ba89bce928..3981e8dfd5 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -535,7 +535,7 @@ module Rails 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 -- cgit v1.2.3 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: 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 c75639d740..89262aeaae 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -89,7 +89,7 @@ module RailtiesTest 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 @@ module RailtiesTest 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 @@ module RailtiesTest 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 @@ module RailtiesTest 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 -- cgit v1.2.3 From 360a8780706cc95144bef429d6fe42e9475a5400 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 5 Oct 2010 19:34:27 +0200 Subject: new guide: Ruby on Rails Guides Guidelines --- railties/guides/source/index.html.erb | 4 ++ railties/guides/source/layout.html.erb | 1 + .../source/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 e6d327168a..d28ea24a76 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 2039c76213..f0aa227c7e 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 0000000000..818d96da79 --- /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 -- cgit v1.2.3 From da34ee423f3d43c368e33d834bb70d59af16273b Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 5 Oct 2010 19:38:05 +0200 Subject: 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 d28ea24a76..6b897e3a6a 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 %> -- cgit v1.2.3 From 3b7d48cc455a04a40328e8f29b4200fdfb4f73b7 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 5 Oct 2010 22:52:43 +0200 Subject: 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 818d96da79..0bc409cbda 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: -- cgit v1.2.3 From 19a5f99685ab9f0a60bb8c1ed2a3fdb40b2e762e Mon Sep 17 00:00:00 2001 From: Erik Michaels-Ober Date: Tue, 5 Oct 2010 19:21:07 -0700 Subject: 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 352172027b..b6a8cf3caf 100644 --- a/activesupport/lib/active_support/xml_mini.rb +++ b/activesupport/lib/active_support/xml_mini.rb @@ -25,7 +25,7 @@ module ActiveSupport DEFAULT_ENCODINGS = { "binary" => "base64" - } unless defined?(TYPE_NAMES) + } unless defined?(DEFAULT_ENCODINGS) TYPE_NAMES = { "Symbol" => "symbol", -- cgit v1.2.3 From 990719bb59b89c614ec81aa25f2fc83f3323c9ea Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Tue, 5 Oct 2010 21:01:01 +0530 Subject: 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 4d21c65101..370a508cad 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" -- cgit v1.2.3 From ae812e9fc9232a56daebd7e20f56a18981480d8c Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Wed, 6 Oct 2010 11:59:11 +0530 Subject: 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 450dec7716..f4fdc46328 100644 --- a/railties/test/generators/mailer_generator_test.rb +++ b/railties/test/generators/mailer_generator_test.rb @@ -59,6 +59,15 @@ class MailerGeneratorTest < Rails::Generators::TestCase 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 -- cgit v1.2.3 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: Clean up the builder abstraction in AppGenerator. This commit may be reverted once documentation and a proper way to handle actions are added. --- .../rails/generators/rails/app/app_generator.rb | 247 +++++---------------- railties/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 2715483914..be09698787 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 'open-uri' 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 @@ module Rails 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 @@ module Rails 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 @@ module Rails 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 @@ module Rails 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 @@ module Rails "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 3653b067c8..3bd8770710 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -261,70 +261,4 @@ protected 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 -- cgit v1.2.3 From 848eb0d777d2f2907c399c2fe073fedffad14c63 Mon Sep 17 00:00:00 2001 From: wycats Date: Wed, 6 Oct 2010 02:53:00 -0700 Subject: 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. --- .../rails/generators/rails/app/app_generator.rb | 247 ++++++++++++++++----- railties/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 be09698787..2715483914 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 'open-uri' 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 @@ module Rails 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 @@ module Rails 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 @@ module Rails 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 @@ module Rails 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 @@ module Rails "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 3bd8770710..3653b067c8 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -261,4 +261,70 @@ protected 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 -- cgit v1.2.3 From 0904e8256864239f673bf91fce1cfffb9345ee61 Mon Sep 17 00:00:00 2001 From: wycats Date: Wed, 6 Oct 2010 02:58:49 -0700 Subject: Delegate everything to the generator --- .../lib/rails/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 2715483914..d2ab098885 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 @@ module Rails @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 -- cgit v1.2.3 From d40ca9cce241a8083756c993d6c99a79e62e050e Mon Sep 17 00:00:00 2001 From: wycats Date: Wed, 6 Oct 2010 03:06:12 -0700 Subject: 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 d2ab098885..7907191c74 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 @@ module Rails 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" -- cgit v1.2.3