From 82343859d568799a4151facbde1f8c711ecb7a3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Thu, 31 Jul 2008 23:59:53 +0300 Subject: Added missing fixtures for tests which fail to run independently if run after schema reset Signed-off-by: Michael Koziarski --- activerecord/test/cases/associations/cascaded_eager_loading_test.rb | 2 +- activerecord/test/cases/associations/eager_test.rb | 2 +- .../cases/associations/has_and_belongs_to_many_associations_test.rb | 2 +- activerecord/test/cases/associations/has_many_associations_test.rb | 2 +- activerecord/test/cases/base_test.rb | 2 +- activerecord/test/cases/lifecycle_test.rb | 2 +- activerecord/test/cases/method_scoping_test.rb | 2 +- activerecord/test/cases/query_cache_test.rb | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 3631be76a0..1f8a1090eb 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -9,7 +9,7 @@ require 'models/topic' require 'models/reply' class CascadedEagerLoadingTest < ActiveRecord::TestCase - fixtures :authors, :mixins, :companies, :posts, :topics + fixtures :authors, :mixins, :companies, :posts, :topics, :accounts, :comments, :categorizations def test_eager_association_loading_with_cascaded_two_levels authors = Author.find(:all, :include=>{:posts=>:comments}, :order=>"authors.id") diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index f65ada550b..58506574f8 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -21,7 +21,7 @@ class EagerAssociationTest < ActiveRecord::TestCase fixtures :posts, :comments, :authors, :categories, :categories_posts, :companies, :accounts, :tags, :taggings, :people, :readers, :owners, :pets, :author_favorites, :jobs, :references, :subscribers, :subscriptions, :books, - :developers, :projects + :developers, :projects, :developers_projects def test_loading_with_one_association posts = Post.find(:all, :include => :comments) 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 b29df68d22..1ed0522e53 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 @@ -70,7 +70,7 @@ end class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :categories, :posts, :categories_posts, :developers, :projects, :developers_projects, - :parrots, :pirates, :treasures, :price_estimates + :parrots, :pirates, :treasures, :price_estimates, :tags, :taggings def test_has_and_belongs_to_many david = Developer.find(1) diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 47e4b3527d..b806e885e1 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -14,7 +14,7 @@ require 'models/reader' class HasManyAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :categories, :companies, :developers, :projects, :developers_projects, :topics, :authors, :comments, :author_addresses, - :people, :posts + :people, :posts, :readers def setup Client.destroyed_client_ids.clear diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index e6d1b5ddfd..0f9eda4d09 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -76,7 +76,7 @@ class TopicWithProtectedContentAndAccessibleAuthorName < ActiveRecord::Base end class BasicsTest < ActiveRecord::TestCase - fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations + fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories def test_table_exists assert !NonExistentTable.table_exists? diff --git a/activerecord/test/cases/lifecycle_test.rb b/activerecord/test/cases/lifecycle_test.rb index ab005c6b00..54fb3d8c39 100644 --- a/activerecord/test/cases/lifecycle_test.rb +++ b/activerecord/test/cases/lifecycle_test.rb @@ -74,7 +74,7 @@ class MultiObserver < ActiveRecord::Observer end class LifecycleTest < ActiveRecord::TestCase - fixtures :topics, :developers + fixtures :topics, :developers, :minimalistics def test_before_destroy original_count = Topic.count diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index d6b3e341df..ee66ac948d 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -6,7 +6,7 @@ require 'models/post' require 'models/category' class MethodScopingTest < ActiveRecord::TestCase - fixtures :developers, :projects, :comments, :posts + fixtures :developers, :projects, :comments, :posts, :developers_projects def test_set_conditions Developer.with_scope(:find => { :conditions => 'just a test...' }) do diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index dc9eeec281..eae2104531 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -58,7 +58,7 @@ end uses_mocha 'QueryCacheExpiryTest' do class QueryCacheExpiryTest < ActiveRecord::TestCase - fixtures :tasks + fixtures :tasks, :posts, :categories, :categories_posts def test_find Task.connection.expects(:clear_query_cache).times(1) -- cgit v1.2.3 From 61842d97c5269af8a36a33115f50543a155f1608 Mon Sep 17 00:00:00 2001 From: Ben Sandofsky Date: Fri, 1 Aug 2008 17:01:10 -0700 Subject: Make requiring gems optional. Signed-off-by: Michael Koziarski [#743 state:resolved] --- railties/lib/initializer.rb | 6 +++++- railties/lib/rails/gem_dependency.rb | 2 +- railties/test/gem_dependency_test.rb | 9 +++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index a2d08e2938..e876481cf1 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -688,13 +688,17 @@ Run `rake gems:install` to install the missing gems. # You can add gems with the #gem method. attr_accessor :gems - # Adds a single Gem dependency to the rails application. + # Adds a single Gem dependency to the rails application. By default, it will require + # the library with the same name as the gem. Use :lib to specify a different name. # # # gem 'aws-s3', '>= 0.4.0' # # require 'aws/s3' # config.gem 'aws-s3', :lib => 'aws/s3', :version => '>= 0.4.0', \ # :source => "http://code.whytheluckystiff.net" # + # To require a library be installed, but not attempt to load it, pass :lib => false + # + # config.gem 'qrp', :version => '0.4.1', :lib => false def gem(name, options = {}) @gems << Rails::GemDependency.new(name, options) end diff --git a/railties/lib/rails/gem_dependency.rb b/railties/lib/rails/gem_dependency.rb index f8d97840c1..471e03fa5f 100644 --- a/railties/lib/rails/gem_dependency.rb +++ b/railties/lib/rails/gem_dependency.rb @@ -58,7 +58,7 @@ module Rails def load return if @loaded || @load_paths_added == false - require(@lib || @name) + require(@lib || @name) unless @lib == false @loaded = true rescue LoadError puts $!.to_s diff --git a/railties/test/gem_dependency_test.rb b/railties/test/gem_dependency_test.rb index b5946aa7b8..964ca50992 100644 --- a/railties/test/gem_dependency_test.rb +++ b/railties/test/gem_dependency_test.rb @@ -11,6 +11,7 @@ uses_mocha "Plugin Tests" do @gem_with_source = Rails::GemDependency.new "hpricot", :source => "http://code.whytheluckystiff.net" @gem_with_version = Rails::GemDependency.new "hpricot", :version => "= 0.6" @gem_with_lib = Rails::GemDependency.new "aws-s3", :lib => "aws/s3" + @gem_without_load = Rails::GemDependency.new "hpricot", :lib => false end def test_configuration_adds_gem_dependency @@ -62,5 +63,13 @@ uses_mocha "Plugin Tests" do @gem_with_lib.add_load_paths @gem_with_lib.load end + + def test_gem_without_lib_loading + @gem_without_load.expects(:gem).with(@gem_without_load.name) + @gem_without_load.expects(:require).with(@gem_without_load.lib).never + @gem_without_load.add_load_paths + @gem_without_load.load + end + end end -- cgit v1.2.3 From ddd552504bd682d64aa63bd06aa3f74818d48493 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 4 Aug 2008 18:37:53 -0700 Subject: Expose Routing::Segment::SAFE_PCHAR list of path characters that don't need escaping --- actionpack/lib/action_controller/routing/segments.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/routing/segments.rb b/actionpack/lib/action_controller/routing/segments.rb index 75784c3b78..9d4b740a44 100644 --- a/actionpack/lib/action_controller/routing/segments.rb +++ b/actionpack/lib/action_controller/routing/segments.rb @@ -2,7 +2,8 @@ module ActionController module Routing class Segment #:nodoc: RESERVED_PCHAR = ':@&=+$,;' - UNSAFE_PCHAR = Regexp.new("[^#{URI::REGEXP::PATTERN::UNRESERVED}#{RESERVED_PCHAR}]", false, 'N').freeze + SAFE_PCHAR = "#{URI::REGEXP::PATTERN::UNRESERVED}#{RESERVED_PCHAR}" + UNSAFE_PCHAR = Regexp.new("[^#{SAFE_PCHAR}]", false, 'N').freeze # TODO: Convert :is_optional accessor to read only attr_accessor :is_optional -- cgit v1.2.3 From 177a35e711e3b21eac0eb19f03aeae7626e490f5 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 2 Aug 2008 00:42:32 -0500 Subject: Added config.threadsafe! to toggle allow concurrency settings and disable the dependency loader --- railties/CHANGELOG | 2 ++ railties/environments/production.rb | 3 +++ railties/lib/initializer.rb | 12 ++++++++++++ 3 files changed, 17 insertions(+) diff --git a/railties/CHANGELOG b/railties/CHANGELOG index 6df7c568dc..3a276d5aad 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Added config.threadsafe! to toggle allow concurrency settings and disable the dependency loader [Josh Peek] + * Turn cache_classes on by default [Josh Peek] * Added configurable eager load paths. Defaults to app/models, app/controllers, and app/helpers [Josh Peek] diff --git a/railties/environments/production.rb b/railties/environments/production.rb index e915e8be73..ec5b7bc865 100644 --- a/railties/environments/production.rb +++ b/railties/environments/production.rb @@ -4,6 +4,9 @@ # Code is not reloaded between requests config.cache_classes = true +# Enable threaded mode +# config.threadsafe! + # Use a different logger for distributed setups # config.logger = SyslogLogger.new diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index e876481cf1..f8b3a78dff 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -768,6 +768,18 @@ Run `rake gems:install` to install the missing gems. ::RAILS_ROOT.replace @root_path end + # Enable threaded mode. Allows concurrent requests to controller actions and + # multiple database connections. Also disables automatic dependency loading + # after boot + def threadsafe! + self.cache_classes = true + self.dependency_loading = false + self.active_record.allow_concurrency = true + self.action_controller.allow_concurrency = true + self.to_prepare { Rails.cache.threadsafe! } + self + end + # Loads and returns the contents of the #database_configuration_file. The # contents of the file are processed via ERB before being sent through # YAML::load. -- cgit v1.2.3 From dc66469e6464bbb6d7bd6f242731395b6574aca2 Mon Sep 17 00:00:00 2001 From: Clemens Kofler Date: Mon, 4 Aug 2008 22:26:14 -0500 Subject: Fixed i18n bulk translate issues in NumberHelper Signed-off-by: Joshua Peek --- .../lib/action_view/helpers/number_helper.rb | 56 ++++++++++++---------- .../test/template/number_helper_i18n_test.rb | 26 +++++----- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index 8c1dea2186..77f19b36a6 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -71,9 +71,9 @@ module ActionView def number_to_currency(number, options = {}) options.symbolize_keys! - defaults, currency = I18n.translate([:'number.format', :'number.currency.format'], - :locale => options[:locale]) || [{},{}] - defaults = defaults.merge(currency) + defaults = I18n.translate(:'number.format', :locale => options[:locale], :raise => true) rescue {} + currency = I18n.translate(:'number.currency.format', :locale => options[:locale], :raise => true) rescue {} + defaults = defaults.merge(currency) precision = options[:precision] || defaults[:precision] unit = options[:unit] || defaults[:unit] @@ -109,9 +109,9 @@ module ActionView def number_to_percentage(number, options = {}) options.symbolize_keys! - defaults, percentage = I18n.translate([:'number.format', :'number.percentage.format'], - :locale => options[:locale]) || [{},{}] - defaults = defaults.merge(percentage) + defaults = I18n.translate(:'number.format', :locale => options[:locale], :raise => true) rescue {} + percentage = I18n.translate(:'number.percentage.format', :locale => options[:locale], :raise => true) rescue {} + defaults = defaults.merge(percentage) precision = options[:precision] || defaults[:precision] separator = options[:separator] || defaults[:separator] @@ -151,7 +151,7 @@ module ActionView options = args.extract_options! options.symbolize_keys! - defaults = I18n.translate(:'number.format', :locale => options[:locale]) || {} + defaults = I18n.translate(:'number.format', :locale => options[:locale], :raise => true) rescue {} unless args.empty? ActiveSupport::Deprecation.warn('number_with_delimiter takes an option hash ' + @@ -195,9 +195,10 @@ module ActionView options = args.extract_options! options.symbolize_keys! - defaults, precision_defaults = I18n.translate([:'number.format', :'number.precision.format'], - :locale => options[:locale]) || [{},{}] - defaults = defaults.merge(precision_defaults) + defaults = I18n.translate(:'number.format', :locale => options[:locale], :raise => true) rescue {} + precision_defaults = I18n.translate(:'number.precision.format', :locale => options[:locale], + :raise => true) rescue {} + defaults = defaults.merge(precision_defaults) unless args.empty? ActiveSupport::Deprecation.warn('number_with_precision takes an option hash ' + @@ -209,12 +210,14 @@ module ActionView separator ||= (options[:separator] || defaults[:separator]) delimiter ||= (options[:delimiter] || defaults[:delimiter]) - rounded_number = (Float(number) * (10 ** precision)).round.to_f / 10 ** precision - number_with_delimiter("%01.#{precision}f" % rounded_number, - :separator => separator, - :delimiter => delimiter) - rescue - number + begin + rounded_number = (Float(number) * (10 ** precision)).round.to_f / 10 ** precision + number_with_delimiter("%01.#{precision}f" % rounded_number, + :separator => separator, + :delimiter => delimiter) + rescue + number + end end STORAGE_UNITS = %w( Bytes KB MB GB TB ).freeze @@ -251,8 +254,8 @@ module ActionView options = args.extract_options! options.symbolize_keys! - defaults, human = I18n.translate([:'number.format', :'number.human.format'], - :locale => options[:locale]) || [{},{}] + defaults = I18n.translate(:'number.format', :locale => options[:locale], :raise => true) rescue {} + human = I18n.translate(:'number.human.format', :locale => options[:locale], :raise => true) rescue {} defaults = defaults.merge(human) unless args.empty? @@ -272,13 +275,16 @@ module ActionView number /= 1024 ** exponent unit = STORAGE_UNITS[exponent] - number_with_precision(number, - :precision => precision, - :separator => separator, - :delimiter => delimiter - ).sub(/(\d)(#{Regexp.escape(separator)}[1-9]*)?0+\z/, '\1') + " #{unit}" - rescue - number + begin + escaped_separator = Regexp.escape(separator) + number_with_precision(number, + :precision => precision, + :separator => separator, + :delimiter => delimiter + ).sub(/(\d)(#{escaped_separator}[1-9]*)?0+\z/, '\1\2').sub(/#{escaped_separator}\z/, '') + " #{unit}" + rescue + number + end end end end diff --git a/actionpack/test/template/number_helper_i18n_test.rb b/actionpack/test/template/number_helper_i18n_test.rb index ce0da398cc..2ee7f43a65 100644 --- a/actionpack/test/template/number_helper_i18n_test.rb +++ b/actionpack/test/template/number_helper_i18n_test.rb @@ -18,35 +18,35 @@ class NumberHelperI18nTests < Test::Unit::TestCase end def test_number_to_currency_translates_currency_formats - I18n.expects(:translate).with( - [:'number.format', :'number.currency.format'], :locale => 'en-US' - ).returns([@number_defaults, @currency_defaults]) + I18n.expects(:translate).with(:'number.format', :locale => 'en-US', :raise => true).returns(@number_defaults) + I18n.expects(:translate).with(:'number.currency.format', :locale => 'en-US', + :raise => true).returns(@currency_defaults) number_to_currency(1, :locale => 'en-US') end def test_number_with_precision_translates_number_formats - I18n.expects(:translate).with( - [:'number.format', :'number.precision.format'], :locale => 'en-US' - ).returns([@number_defaults, @precision_defaults]) + I18n.expects(:translate).with(:'number.format', :locale => 'en-US', :raise => true).returns(@number_defaults) + I18n.expects(:translate).with(:'number.precision.format', :locale => 'en-US', + :raise => true).returns(@precision_defaults) number_with_precision(1, :locale => 'en-US') end def test_number_with_delimiter_translates_number_formats - I18n.expects(:translate).with(:'number.format', :locale => 'en-US').returns(@number_defaults) + I18n.expects(:translate).with(:'number.format', :locale => 'en-US', :raise => true).returns(@number_defaults) number_with_delimiter(1, :locale => 'en-US') end def test_number_to_percentage_translates_number_formats - I18n.expects(:translate).with( - [:'number.format', :'number.percentage.format'], :locale => 'en-US' - ).returns([@number_defaults, @percentage_defaults]) + I18n.expects(:translate).with(:'number.format', :locale => 'en-US', :raise => true).returns(@number_defaults) + I18n.expects(:translate).with(:'number.percentage.format', :locale => 'en-US', + :raise => true).returns(@percentage_defaults) number_to_percentage(1, :locale => 'en-US') end def test_number_to_human_size_translates_human_formats - I18n.expects(:translate).with( - [:'number.format', :'number.human.format'], :locale => 'en-US' - ).returns([@number_defaults, @human_defaults]) + I18n.expects(:translate).with(:'number.format', :locale => 'en-US', :raise => true).returns(@number_defaults) + I18n.expects(:translate).with(:'number.human.format', :locale => 'en-US', + :raise => true).returns(@human_defaults) # can't be called with 1 because this directly returns without calling I18n.translate number_to_human_size(1025, :locale => 'en-US') end -- cgit v1.2.3 From 8d72b82b8da99eda9ba654b3d3a4da847a212406 Mon Sep 17 00:00:00 2001 From: Jeffrey Hardy Date: Tue, 5 Aug 2008 16:29:56 -0500 Subject: Make assert_template failure message more friendly Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/assertions/response_assertions.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller/assertions/response_assertions.rb b/actionpack/lib/action_controller/assertions/response_assertions.rb index 765225ae24..e2e8bbdc71 100644 --- a/actionpack/lib/action_controller/assertions/response_assertions.rb +++ b/actionpack/lib/action_controller/assertions/response_assertions.rb @@ -87,11 +87,11 @@ module ActionController # def assert_template(expected = nil, message=nil) clean_backtrace do - rendered = @response.rendered_template + rendered = @response.rendered_template.to_s msg = build_message(message, "expecting but rendering with ", expected, rendered) assert_block(msg) do if expected.nil? - @response.rendered_template.nil? + @response.rendered_template.blank? else rendered.to_s.match(expected) end -- cgit v1.2.3 From 69e9cbb99ab111ad6b9cb2c9d8ba823ad8600bd6 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 5 Aug 2008 17:33:51 -0700 Subject: Ensure public superclass methods don't shadow public controller methods. Case in point, ruby-debug's Kernel#start shadowing a controller's start action. --- actionpack/lib/action_controller/base.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 5689a9825e..c50e255c98 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -1199,7 +1199,7 @@ module ActionController #:nodoc: end def perform_action - if self.class.action_methods.include?(action_name) + if action_methods.include?(action_name) send(action_name) default_render unless performed? elsif respond_to? :method_missing @@ -1208,7 +1208,7 @@ module ActionController #:nodoc: elsif template_exists? && template_public? default_render else - raise UnknownAction, "No action responded to #{action_name}. Actions: #{action_methods.to_a.sort.to_sentence}", caller + raise UnknownAction, "No action responded to #{action_name}. Actions: #{action_methods.sort.to_sentence}", caller end end @@ -1234,7 +1234,7 @@ module ActionController #:nodoc: end def self.action_methods - @action_methods ||= Set.new(public_instance_methods.map { |m| m.to_s }) - hidden_actions + @action_methods ||= Set.new(public_instance_methods.map { |m| m.to_s }) - hidden_actions + public_instance_methods(false).map { |m| m.to_s } end def add_variables_to_assigns -- cgit v1.2.3 From 29a06f10e8600bbda333c30bf294e7896f35b8d5 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 5 Aug 2008 19:28:52 -0700 Subject: Strip newlines from cookie session data --- actionpack/lib/action_controller/session/cookie_store.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/session/cookie_store.rb b/actionpack/lib/action_controller/session/cookie_store.rb index b477c1f7da..5bf7503f04 100644 --- a/actionpack/lib/action_controller/session/cookie_store.rb +++ b/actionpack/lib/action_controller/session/cookie_store.rb @@ -129,7 +129,7 @@ class CGI::Session::CookieStore private # Marshal a session hash into safe cookie data. Include an integrity hash. def marshal(session) - data = ActiveSupport::Base64.encode64(Marshal.dump(session)).chop + data = ActiveSupport::Base64.encode64s(Marshal.dump(session)) "#{data}--#{generate_digest(data)}" end -- cgit v1.2.3 From 080974784582e1e289c2948227b446bc56d404a1 Mon Sep 17 00:00:00 2001 From: Nik Wakelin Date: Sat, 2 Aug 2008 17:44:02 +1200 Subject: Added MigrationProxy to defer loading of Migration classes until they are actually required by the migrator Signed-off-by: Michael Koziarski [#747 state:resolved] --- activerecord/lib/active_record/migration.rb | 32 +++++++++++++++++++++++------ activerecord/test/cases/migration_test.rb | 20 ++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 731a350854..fd77f27b77 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -349,6 +349,27 @@ module ActiveRecord end end + # MigrationProxy is used to defer loading of the actual migration classes + # until they are needed + class MigrationProxy + + attr_accessor :name, :version, :filename + + delegate :migrate, :announce, :write, :to=>:migration + + private + + def migration + @migration ||= load_migration + end + + def load_migration + load(filename) + name.constantize + end + + end + class Migrator#:nodoc: class << self def migrate(migrations_path, target_version = nil) @@ -437,7 +458,7 @@ module ActiveRecord runnable.pop if down? && !target.nil? runnable.each do |migration| - Base.logger.info "Migrating to #{migration} (#{migration.version})" + Base.logger.info "Migrating to #{migration.name} (#{migration.version})" # On our way up, we skip migrating the ones we've already migrated # On our way down, we skip reverting the ones we've never migrated @@ -470,11 +491,10 @@ module ActiveRecord raise DuplicateMigrationNameError.new(name.camelize) end - load(file) - - klasses << returning(name.camelize.constantize) do |klass| - class << klass; attr_accessor :version end - klass.version = version + klasses << returning(MigrationProxy.new) do |migration| + migration.name = name.camelize + migration.version = version + migration.filename = file end end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 7ecf755ef8..920f719995 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -922,6 +922,26 @@ if ActiveRecord::Base.connection.supports_migrations? migrations[0].name == 'innocent_jointable' end + def test_only_loads_pending_migrations + # migrate up to 1 + ActiveRecord::Migrator.up(MIGRATIONS_ROOT + "/valid", 1) + + # now unload the migrations that have been defined + PeopleHaveLastNames.unloadable + ActiveSupport::Dependencies.remove_unloadable_constants! + + ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/valid", nil) + + assert !defined? PeopleHaveLastNames + + %w(WeNeedReminders, InnocentJointable).each do |migration| + assert defined? migration + end + + ensure + load(MIGRATIONS_ROOT + "/valid/1_people_have_last_names.rb") + end + def test_migrator_interleaved_migrations ActiveRecord::Migrator.up(MIGRATIONS_ROOT + "/interleaved/pass_1") -- cgit v1.2.3 From 73056500f88d569fa497d846dfe6b501a9e03739 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 6 Aug 2008 14:39:13 -0500 Subject: Make File.atomic_write copy the original permissions or use the directories default. --- activesupport/lib/active_support/core_ext/file.rb | 24 ++--------- .../lib/active_support/core_ext/file/atomic.rb | 46 ++++++++++++++++++++ activesupport/test/core_ext/file_test.rb | 50 +++++++++++++++++++--- 3 files changed, 94 insertions(+), 26 deletions(-) create mode 100644 activesupport/lib/active_support/core_ext/file/atomic.rb diff --git a/activesupport/lib/active_support/core_ext/file.rb b/activesupport/lib/active_support/core_ext/file.rb index 45d93b220f..e03f8ac44e 100644 --- a/activesupport/lib/active_support/core_ext/file.rb +++ b/activesupport/lib/active_support/core_ext/file.rb @@ -1,21 +1,5 @@ -require 'tempfile' +require 'active_support/core_ext/file/atomic' -# Write to a file atomically. Useful for situations where you don't -# want other processes or threads to see half-written files. -# -# File.atomic_write("important.file") do |file| -# file.write("hello") -# end -# -# If your temp directory is not on the same filesystem as the file you're -# trying to write, you can provide a different temporary directory. -# -# File.atomic_write("/data/something.important", "/data/tmp") do |f| -# file.write("hello") -# end -def File.atomic_write(file_name, temp_dir = Dir.tmpdir) - temp_file = Tempfile.new(File.basename(file_name), temp_dir) - yield temp_file - temp_file.close - File.rename(temp_file.path, file_name) -end \ No newline at end of file +class File #:nodoc: + extend ActiveSupport::CoreExtensions::File::Atomic +end diff --git a/activesupport/lib/active_support/core_ext/file/atomic.rb b/activesupport/lib/active_support/core_ext/file/atomic.rb new file mode 100644 index 0000000000..4d3cf5423f --- /dev/null +++ b/activesupport/lib/active_support/core_ext/file/atomic.rb @@ -0,0 +1,46 @@ +require 'tempfile' + +module ActiveSupport #:nodoc: + module CoreExtensions #:nodoc: + module File #:nodoc: + module Atomic + # Write to a file atomically. Useful for situations where you don't + # want other processes or threads to see half-written files. + # + # File.atomic_write("important.file") do |file| + # file.write("hello") + # end + # + # If your temp directory is not on the same filesystem as the file you're + # trying to write, you can provide a different temporary directory. + # + # File.atomic_write("/data/something.important", "/data/tmp") do |f| + # file.write("hello") + # end + def atomic_write(file_name, temp_dir = Dir.tmpdir) + temp_file = Tempfile.new(basename(file_name), temp_dir) + yield temp_file + temp_file.close + + begin + # Get original file permissions + old_stat = stat(file_name) + rescue Errno::ENOENT + # No old permissions, write a temp file to determine the defaults + check_name = ".permissions_check.#{Thread.current.object_id}.#{Process.pid}.#{rand(1000000)}" + new(check_name, "w") + old_stat = stat(check_name) + unlink(check_name) + end + + # Overwrite original file with temp file + rename(temp_file.path, file_name) + + # Set correct permissions on new file + chown(old_stat.uid, old_stat.gid, file_name) + chmod(old_stat.mode, file_name) + end + end + end + end +end diff --git a/activesupport/test/core_ext/file_test.rb b/activesupport/test/core_ext/file_test.rb index 5efe357e9f..63b470684f 100644 --- a/activesupport/test/core_ext/file_test.rb +++ b/activesupport/test/core_ext/file_test.rb @@ -1,9 +1,8 @@ require 'abstract_unit' class AtomicWriteTest < Test::Unit::TestCase - def test_atomic_write_without_errors - contents = "Atomic Text" + contents = "Atomic Text" File.atomic_write(file_name, Dir.pwd) do |file| file.write(contents) assert !File.exist?(file_name) @@ -13,7 +12,7 @@ class AtomicWriteTest < Test::Unit::TestCase ensure File.unlink(file_name) rescue nil end - + def test_atomic_write_doesnt_write_when_block_raises File.atomic_write(file_name) do |file| file.write("testing") @@ -22,8 +21,47 @@ class AtomicWriteTest < Test::Unit::TestCase rescue assert !File.exist?(file_name) end - - def file_name - "atomic.file" + + def test_atomic_write_preserves_file_permissions + contents = "Atomic Text" + File.open(file_name, "w", 0755) do |file| + file.write(contents) + assert File.exist?(file_name) + end + assert File.exist?(file_name) + assert_equal "100755", file_mode + assert_equal contents, File.read(file_name) + + File.atomic_write(file_name, Dir.pwd) do |file| + file.write(contents) + assert File.exist?(file_name) + end + assert File.exist?(file_name) + assert_equal "100755", file_mode + assert_equal contents, File.read(file_name) + ensure + File.unlink(file_name) rescue nil + end + + def test_atomic_write_preserves_default_file_permissions + contents = "Atomic Text" + File.atomic_write(file_name, Dir.pwd) do |file| + file.write(contents) + assert !File.exist?(file_name) + end + assert File.exist?(file_name) + assert_equal "100644", file_mode + assert_equal contents, File.read(file_name) + ensure + File.unlink(file_name) rescue nil end + + private + def file_name + "atomic.file" + end + + def file_mode + sprintf("%o", File.stat(file_name).mode) + end end -- cgit v1.2.3 From e5b1ab7cc39ff57f9789ffda75fb33f72187775d Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 6 Aug 2008 14:50:02 -0500 Subject: MemoryStore is the only "unsafe" store. Make it threadsafe by default. --- .../lib/action_view/helpers/asset_tag_helper.rb | 2 +- activesupport/lib/active_support/cache.rb | 18 ------ .../lib/active_support/cache/memory_store.rb | 55 ++++++++++++++---- activesupport/test/caching_test.rb | 67 ---------------------- railties/lib/initializer.rb | 1 - 5 files changed, 44 insertions(+), 99 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 769eada120..1e44b166d9 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -463,7 +463,7 @@ module ActionView end private - COMPUTED_PUBLIC_PATHS = ActiveSupport::Cache::MemoryStore.new.silence!.threadsafe! + COMPUTED_PUBLIC_PATHS = ActiveSupport::Cache::MemoryStore.new.silence! # Add the the extension +ext+ if not present. Return full URLs otherwise untouched. # Prefix with /dir/ if lacking a leading +/+. Account for relative URL diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 5a064f8bea..95eae3a77e 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -39,10 +39,6 @@ module ActiveSupport class Store cattr_accessor :logger - def threadsafe! - extend ThreadSafety - end - def silence! @silence = true self @@ -115,20 +111,6 @@ module ActiveSupport logger.debug("Cache #{operation}: #{key}#{options ? " (#{options.inspect})" : ""}") if logger && !@silence && !@logger_off end end - - module ThreadSafety #:nodoc: - def self.extended(object) #:nodoc: - object.instance_variable_set(:@mutex, Mutex.new) - end - - %w(read write delete delete_matched exist? increment decrement).each do |method| - module_eval <<-EOS, __FILE__, __LINE__ - def #{method}(*args) - @mutex.synchronize { super } - end - EOS - end - end end end diff --git a/activesupport/lib/active_support/cache/memory_store.rb b/activesupport/lib/active_support/cache/memory_store.rb index 6f114273e4..9332d50f24 100644 --- a/activesupport/lib/active_support/cache/memory_store.rb +++ b/activesupport/lib/active_support/cache/memory_store.rb @@ -3,36 +3,67 @@ module ActiveSupport class MemoryStore < Store def initialize @data = {} + @mutex = Mutex.new + end + + def fetch(key, options = {}) + @mutex.synchronize do + super + end end def read(name, options = nil) - super - @data[name] + @mutex.synchronize do + super + @data[name] + end end def write(name, value, options = nil) - super - @data[name] = value + @mutex.synchronize do + super + @data[name] = value + end end def delete(name, options = nil) - super - @data.delete(name) + @mutex.synchronize do + super + @data.delete(name) + end end def delete_matched(matcher, options = nil) - super - @data.delete_if { |k,v| k =~ matcher } + @mutex.synchronize do + super + @data.delete_if { |k,v| k =~ matcher } + end end def exist?(name,options = nil) - super - @data.has_key?(name) + @mutex.synchronize do + super + @data.has_key?(name) + end + end + + def increment(key, amount = 1) + @mutex.synchronize do + super + end + end + + def decrement(key, amount = 1) + @mutex.synchronize do + super + end end def clear - @data.clear + @mutex.synchronize do + @data.clear + end end end end -end \ No newline at end of file +end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 0af4251962..f3220d27aa 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -70,70 +70,3 @@ uses_mocha 'high-level cache store tests' do end end end - -class ThreadSafetyCacheStoreTest < Test::Unit::TestCase - def setup - @cache = ActiveSupport::Cache.lookup_store(:memory_store).threadsafe! - @cache.write('foo', 'bar') - - # No way to have mocha proxy to the original method - @mutex = @cache.instance_variable_get(:@mutex) - @mutex.instance_eval %( - def calls; @calls; end - def synchronize - @calls ||= 0 - @calls += 1 - yield - end - ) - end - - def test_read_is_synchronized - assert_equal 'bar', @cache.read('foo') - assert_equal 1, @mutex.calls - end - - def test_write_is_synchronized - @cache.write('foo', 'baz') - assert_equal 'baz', @cache.read('foo') - assert_equal 2, @mutex.calls - end - - def test_delete_is_synchronized - assert_equal 'bar', @cache.read('foo') - @cache.delete('foo') - assert_equal nil, @cache.read('foo') - assert_equal 3, @mutex.calls - end - - def test_delete_matched_is_synchronized - assert_equal 'bar', @cache.read('foo') - @cache.delete_matched(/foo/) - assert_equal nil, @cache.read('foo') - assert_equal 3, @mutex.calls - end - - def test_fetch_is_synchronized - assert_equal 'bar', @cache.fetch('foo') { 'baz' } - assert_equal 'fu', @cache.fetch('bar') { 'fu' } - assert_equal 3, @mutex.calls - end - - def test_exist_is_synchronized - assert @cache.exist?('foo') - assert !@cache.exist?('bar') - assert_equal 2, @mutex.calls - end - - def test_increment_is_synchronized - @cache.write('foo_count', 1) - assert_equal 2, @cache.increment('foo_count') - assert_equal 4, @mutex.calls - end - - def test_decrement_is_synchronized - @cache.write('foo_count', 1) - assert_equal 0, @cache.decrement('foo_count') - assert_equal 4, @mutex.calls - end -end diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index f8b3a78dff..4036b14f19 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -776,7 +776,6 @@ Run `rake gems:install` to install the missing gems. self.dependency_loading = false self.active_record.allow_concurrency = true self.action_controller.allow_concurrency = true - self.to_prepare { Rails.cache.threadsafe! } self end -- cgit v1.2.3 From dfc83566b3f52b4b84db52312c01fcc3b8847059 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 6 Aug 2008 14:52:39 -0500 Subject: Make FileStore use atomic writes --- activesupport/lib/active_support/cache/file_store.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 5b771b1da0..0bdca49388 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -15,7 +15,7 @@ module ActiveSupport def write(name, value, options = nil) super ensure_cache_path(File.dirname(real_file_path(name))) - File.open(real_file_path(name), "wb+") { |f| f.write(value) } + File.atomic_write(real_file_path(name)) { |f| f.write(value) } rescue => e RAILS_DEFAULT_LOGGER.error "Couldn't create cache directory: #{name} (#{e.message})" if RAILS_DEFAULT_LOGGER end -- cgit v1.2.3 From fbc6129acd9ecb6b7435931b472d3226985ba4c4 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 6 Aug 2008 17:03:42 -0500 Subject: Treat single C operations in MemoryStore as atomic --- .../lib/active_support/cache/memory_store.rb | 31 ++++++---------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/activesupport/lib/active_support/cache/memory_store.rb b/activesupport/lib/active_support/cache/memory_store.rb index 9332d50f24..a44f877414 100644 --- a/activesupport/lib/active_support/cache/memory_store.rb +++ b/activesupport/lib/active_support/cache/memory_store.rb @@ -13,38 +13,25 @@ module ActiveSupport end def read(name, options = nil) - @mutex.synchronize do - super - @data[name] - end + super + @data[name] end def write(name, value, options = nil) - @mutex.synchronize do - super - @data[name] = value - end + super + @data[name] = value end def delete(name, options = nil) - @mutex.synchronize do - super - @data.delete(name) - end + @data.delete(name) end def delete_matched(matcher, options = nil) - @mutex.synchronize do - super - @data.delete_if { |k,v| k =~ matcher } - end + @data.delete_if { |k,v| k =~ matcher } end def exist?(name,options = nil) - @mutex.synchronize do - super - @data.has_key?(name) - end + @data.has_key?(name) end def increment(key, amount = 1) @@ -60,9 +47,7 @@ module ActiveSupport end def clear - @mutex.synchronize do - @data.clear - end + @data.clear end end end -- cgit v1.2.3 From 165120a606577dd81fe1a267899c4415fe25f1fb Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 6 Aug 2008 15:23:39 -0700 Subject: Be more careful about deducing action_methods --- actionpack/lib/action_controller/base.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index c50e255c98..0fdbcbd26f 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -428,11 +428,7 @@ module ActionController #:nodoc: # By default, all methods defined in ActionController::Base and included modules are hidden. # More methods can be hidden using hide_actions. def hidden_actions - unless read_inheritable_attribute(:hidden_actions) - write_inheritable_attribute(:hidden_actions, ActionController::Base.public_instance_methods.map { |m| m.to_s }) - end - - read_inheritable_attribute(:hidden_actions) + read_inheritable_attribute(:hidden_actions) || write_inheritable_attribute(:hidden_actions, []) end # Hide each of the given methods from being callable as actions. @@ -1234,7 +1230,15 @@ module ActionController #:nodoc: end def self.action_methods - @action_methods ||= Set.new(public_instance_methods.map { |m| m.to_s }) - hidden_actions + public_instance_methods(false).map { |m| m.to_s } + @action_methods ||= + # All public instance methods of this class, including ancestors + public_instance_methods(true).map { |m| m.to_s }.to_set - + # Except for public instance methods of Base and its ancestors + Base.public_instance_methods(true).map { |m| m.to_s } + + # Be sure to include shadowed public instance methods of this class + public_instance_methods(false).map { |m| m.to_s } - + # And always exclude explicitly hidden actions + hidden_actions end def add_variables_to_assigns -- cgit v1.2.3 From c6b7d0f34472ee7ef03d602c8923fd0ba8dab833 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 6 Aug 2008 17:22:58 -0500 Subject: Ensure file atomic write uses the cache directory as its tmp folder --- activesupport/lib/active_support/cache/file_store.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 0bdca49388..7b6ca39091 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -15,7 +15,7 @@ module ActiveSupport def write(name, value, options = nil) super ensure_cache_path(File.dirname(real_file_path(name))) - File.atomic_write(real_file_path(name)) { |f| f.write(value) } + File.atomic_write(real_file_path(name), cache_path) { |f| f.write(value) } rescue => e RAILS_DEFAULT_LOGGER.error "Couldn't create cache directory: #{name} (#{e.message})" if RAILS_DEFAULT_LOGGER end -- cgit v1.2.3 From f5bcbde1e387020c7f4968515921a3ccee3dcda2 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 6 Aug 2008 17:40:03 -0500 Subject: Make sure ActionView is loaded inorder to build view paths --- railties/lib/initializer.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 4036b14f19..6576cd368b 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -340,9 +340,11 @@ Run `rake gems:install` to install the missing gems. end def load_view_paths - ActionView::PathSet::Path.eager_load_templates! if configuration.cache_classes - ActionMailer::Base.template_root.load if configuration.frameworks.include?(:action_mailer) - ActionController::Base.view_paths.load if configuration.frameworks.include?(:action_controller) + if configuration.frameworks.include?(:action_view) + ActionView::PathSet::Path.eager_load_templates! if configuration.cache_classes + ActionController::Base.view_paths.load if configuration.frameworks.include?(:action_controller) + ActionMailer::Base.template_root.load if configuration.frameworks.include?(:action_mailer) + end end # Eager load application classes @@ -440,9 +442,11 @@ Run `rake gems:install` to install the missing gems. # paths have already been set, it is not changed, otherwise it is # set to use Configuration#view_path. def initialize_framework_views - view_path = ActionView::PathSet::Path.new(configuration.view_path, false) - ActionMailer::Base.template_root ||= view_path if configuration.frameworks.include?(:action_mailer) - ActionController::Base.view_paths = view_path if configuration.frameworks.include?(:action_controller) && ActionController::Base.view_paths.empty? + if configuration.frameworks.include?(:action_view) + view_path = ActionView::PathSet::Path.new(configuration.view_path, false) + ActionMailer::Base.template_root ||= view_path if configuration.frameworks.include?(:action_mailer) + ActionController::Base.view_paths = view_path if configuration.frameworks.include?(:action_controller) && ActionController::Base.view_paths.empty? + end end # If Action Controller is not one of the loaded frameworks (Configuration#frameworks) -- cgit v1.2.3 From ed8a882e47e07b470b71cacd8cd50e251dca4d27 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 6 Aug 2008 17:31:57 -0700 Subject: JRuby: improve constantize performance. [#410 state:resolved] --- activesupport/lib/active_support/inflector.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/activesupport/lib/active_support/inflector.rb b/activesupport/lib/active_support/inflector.rb index 6651569d33..c2738b39fc 100644 --- a/activesupport/lib/active_support/inflector.rb +++ b/activesupport/lib/active_support/inflector.rb @@ -291,11 +291,14 @@ module ActiveSupport # NameError is raised when the name is not in CamelCase or the constant is # unknown. def constantize(camel_cased_word) - unless /\A(?:::)?([A-Z]\w*(?:::[A-Z]\w*)*)\z/ =~ camel_cased_word - raise NameError, "#{camel_cased_word.inspect} is not a valid constant name!" - end + names = camel_cased_word.split('::') + names.shift if names.empty? || names.first.empty? - Object.module_eval("::#{$1}", __FILE__, __LINE__) + constant = Object + names.each do |name| + constant = constant.const_defined?(name) ? constant.const_get(name) : constant.const_missing(name) + end + constant end # Turns a number into an ordinal string used to denote the position in an @@ -326,4 +329,4 @@ require 'active_support/inflections' require 'active_support/core_ext/string/inflections' unless String.included_modules.include?(ActiveSupport::CoreExtensions::String::Inflections) String.send :include, ActiveSupport::CoreExtensions::String::Inflections -end \ No newline at end of file +end -- cgit v1.2.3 From b2504f8ba0f9baadb9298647fd58ef2c136f9aae Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 6 Aug 2008 20:08:27 -0500 Subject: Tidy up ActionMailer rendering logic to take advantage of view path cache instead of using file system lookups --- actionmailer/lib/action_mailer/base.rb | 49 ++++++++++-------------- actionmailer/test/mail_service_test.rb | 31 ++++++--------- actionpack/lib/action_view/base.rb | 5 ++- actionpack/lib/action_view/template.rb | 10 ++++- actionpack/test/controller/assert_select_test.rb | 33 ++++------------ 5 files changed, 53 insertions(+), 75 deletions(-) diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index fa29ae2446..1583fb4066 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -216,7 +216,7 @@ module ActionMailer #:nodoc: # * :domain - If you need to specify a HELO domain, you can do it here. # * :user_name - If your mail server requires authentication, set the username in this setting. # * :password - If your mail server requires authentication, set the password in this setting. - # * :authentication - If your mail server requires authentication, you need to specify the authentication type here. + # * :authentication - If your mail server requires authentication, you need to specify the authentication type here. # This is a symbol and one of :plain, :login, :cram_md5. # # * sendmail_settings - Allows you to override options for the :sendmail delivery method. @@ -233,10 +233,10 @@ module ActionMailer #:nodoc: # * deliveries - Keeps an array of all the emails sent out through the Action Mailer with delivery_method :test. Most useful # for unit and functional testing. # - # * default_charset - The default charset used for the body and to encode the subject. Defaults to UTF-8. You can also + # * default_charset - The default charset used for the body and to encode the subject. Defaults to UTF-8. You can also # pick a different charset from inside a method with +charset+. # * default_content_type - The default content type used for the main part of the message. Defaults to "text/plain". You - # can also pick a different content type from inside a method with +content_type+. + # can also pick a different content type from inside a method with +content_type+. # * default_mime_version - The default mime version used for the message. Defaults to 1.0. You # can also pick a different value from inside a method with +mime_version+. # * default_implicit_parts_order - When a message is built implicitly (i.e. multiple parts are assembled from templates @@ -253,9 +253,6 @@ module ActionMailer #:nodoc: class_inheritable_accessor :view_paths cattr_accessor :logger - cattr_accessor :template_extensions - @@template_extensions = ['erb', 'builder', 'rhtml', 'rxml'] - @@smtp_settings = { :address => "localhost", :port => 25, @@ -414,15 +411,10 @@ module ActionMailer #:nodoc: new.deliver!(mail) end - # Register a template extension so mailer templates written in a - # templating language other than rhtml or rxml are supported. - # To use this, include in your template-language plugin's init - # code or on a per-application basis, this can be invoked from - # config/environment.rb: - # - # ActionMailer::Base.register_template_extension('haml') def register_template_extension(extension) - template_extensions << extension + ActiveSupport::Deprecation.warn( + "ActionMailer::Base.register_template_extension has been deprecated." + + "Use ActionView::Base.register_template_extension instead", caller) end def template_root @@ -455,16 +447,18 @@ module ActionMailer #:nodoc: # "the_template_file.text.html.erb", etc.). Only do this if parts # have not already been specified manually. if @parts.empty? - templates = Dir.glob("#{template_path}/#{@template}.*") - templates.each do |path| - basename = File.basename(path) - template_regex = Regexp.new("^([^\\\.]+)\\\.([^\\\.]+\\\.[^\\\.]+)\\\.(" + template_extensions.join('|') + ")$") - next unless md = template_regex.match(basename) - template_name = basename - content_type = md.captures[1].gsub('.', '/') - @parts << Part.new(:content_type => content_type, - :disposition => "inline", :charset => charset, - :body => render_message(template_name, @body)) + Dir.glob("#{template_path}/#{@template}.*").each do |path| + template = template_root["#{mailer_name}/#{File.basename(path)}"] + + # Skip unless template has a multipart format + next unless template.multipart? + + @parts << Part.new( + :content_type => template.content_type, + :disposition => "inline", + :charset => charset, + :body => render_message(template, @body) + ) end unless @parts.empty? @content_type = "multipart/alternative" @@ -476,9 +470,8 @@ module ActionMailer #:nodoc: # also render a "normal" template (without the content type). If a # normal template exists (or if there were no implicit parts) we render # it. - template_exists = @parts.empty? - template_exists ||= Dir.glob("#{template_path}/#{@template}.*").any? { |i| File.basename(i).split(".").length == 2 } - @body = render_message(@template, @body) if template_exists + template = template_root["#{mailer_name}/#{@template}"] + @body = render_message(@template, @body) if template # Finally, if there are other message parts and a textual body exists, # we shift it onto the front of the parts and set the body to nil (so @@ -538,7 +531,7 @@ module ActionMailer #:nodoc: def render(opts) body = opts.delete(:body) - if opts[:file] && opts[:file] !~ /\// + if opts[:file] && (opts[:file] !~ /\// && !opts[:file].respond_to?(:render)) opts[:file] = "#{mailer_name}/#{opts[:file]}" end initialize_template_class(body).render(opts) diff --git a/actionmailer/test/mail_service_test.rb b/actionmailer/test/mail_service_test.rb index e5ecb0e254..882b07d675 100644 --- a/actionmailer/test/mail_service_test.rb +++ b/actionmailer/test/mail_service_test.rb @@ -219,7 +219,7 @@ class TestMailer < ActionMailer::Base end attachment :content_type => "application/octet-stream",:filename => "test.txt", :body => "test abcdefghijklmnopqstuvwxyz" end - + def nested_multipart_with_body(recipient) recipients recipient subject "nested multipart with body" @@ -321,7 +321,7 @@ class ActionMailerTest < Test::Unit::TestCase assert_nothing_raised { created = TestMailer.create_nested_multipart(@recipient)} assert_equal 2,created.parts.size assert_equal 2,created.parts.first.parts.size - + assert_equal "multipart/mixed", created.content_type assert_equal "multipart/alternative", created.parts.first.content_type assert_equal "bar", created.parts.first.header['foo'].to_s @@ -366,7 +366,7 @@ class ActionMailerTest < Test::Unit::TestCase assert_not_nil ActionMailer::Base.deliveries.first assert_equal expected.encoded, ActionMailer::Base.deliveries.first.encoded end - + def test_custom_template expected = new_mail expected.to = @recipient @@ -382,7 +382,6 @@ class ActionMailerTest < Test::Unit::TestCase end def test_custom_templating_extension - # # N.b., custom_templating_extension.text.plain.haml is expected to be in fixtures/test_mailer directory expected = new_mail expected.to = @recipient @@ -390,18 +389,10 @@ class ActionMailerTest < Test::Unit::TestCase expected.body = "Hello there, \n\nMr. #{@recipient}" expected.from = "system@loudthinking.com" expected.date = Time.local(2004, 12, 12) - + # Stub the render method so no alternative renderers need be present. ActionView::Base.any_instance.stubs(:render).returns("Hello there, \n\nMr. #{@recipient}") - - # If the template is not registered, there should be no parts. - created = nil - assert_nothing_raised { created = TestMailer.create_custom_templating_extension(@recipient) } - assert_not_nil created - assert_equal 0, created.parts.length - - ActionMailer::Base.register_template_extension('haml') - + # Now that the template is registered, there should be one part. The text/plain part. created = nil assert_nothing_raised { created = TestMailer.create_custom_templating_extension(@recipient) } @@ -428,7 +419,7 @@ class ActionMailerTest < Test::Unit::TestCase assert_not_nil ActionMailer::Base.deliveries.first assert_equal expected.encoded, ActionMailer::Base.deliveries.first.encoded end - + def test_cc_bcc expected = new_mail expected.to = @recipient @@ -550,7 +541,7 @@ class ActionMailerTest < Test::Unit::TestCase TestMailer.deliver_signed_up(@recipient) assert_equal 1, ActionMailer::Base.deliveries.size end - + def test_doesnt_raise_errors_when_raise_delivery_errors_is_false ActionMailer::Base.raise_delivery_errors = false TestMailer.any_instance.expects(:perform_delivery_test).raises(Exception) @@ -670,7 +661,7 @@ EOF assert_not_nil ActionMailer::Base.deliveries.first assert_equal expected.encoded, ActionMailer::Base.deliveries.first.encoded end - + def test_utf8_body_is_not_quoted @recipient = "Foo áëô îü " expected = new_mail "utf-8" @@ -760,7 +751,7 @@ EOF mail = TestMailer.create_multipart_with_mime_version(@recipient) assert_equal "1.1", mail.mime_version end - + def test_multipart_with_utf8_subject mail = TestMailer.create_multipart_with_utf8_subject(@recipient) assert_match(/\nSubject: =\?utf-8\?Q\?Foo_.*?\?=/, mail.encoded) @@ -825,7 +816,7 @@ EOF mail = TestMailer.create_implicitly_multipart_example(@recipient, 'iso-8859-1') assert_equal "multipart/alternative", mail.header['content-type'].body - + assert_equal 'iso-8859-1', mail.parts[0].sub_header("content-type", "charset") assert_equal 'iso-8859-1', mail.parts[1].sub_header("content-type", "charset") assert_equal 'iso-8859-1', mail.parts[2].sub_header("content-type", "charset") @@ -852,7 +843,7 @@ EOF assert_equal "line #1\nline #2\nline #3\nline #4\n\n", mail.parts[0].body assert_equal "

line #1

\n

line #2

\n

line #3

\n

line #4

\n\n", mail.parts[1].body end - + def test_headers_removed_on_smtp_delivery ActionMailer::Base.delivery_method = :smtp TestMailer.deliver_cc_bcc(@recipient) diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index bdcb1dc246..ad59d92086 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -300,6 +300,8 @@ module ActionView #:nodoc: # # => 'users/legacy.rhtml' # def pick_template(template_path) + return template_path if template_path.respond_to?(:render) + path = template_path.sub(/^\//, '') if m = path.match(/(.*)\.(\w+)$/) template_file_name, template_file_extension = m[1], m[2] @@ -343,7 +345,8 @@ module ActionView #:nodoc: ActiveSupport::Deprecation.warn("use_full_path option has been deprecated and has no affect.", caller) end - if defined?(ActionMailer) && defined?(ActionMailer::Base) && controller.is_a?(ActionMailer::Base) && !template_path.include?("/") + if defined?(ActionMailer) && defined?(ActionMailer::Base) && controller.is_a?(ActionMailer::Base) && + template_path.is_a?(String) && !template_path.include?("/") raise ActionViewError, <<-END_ERROR Due to changes in ActionMailer, you need to provide the mailer_name along with the template name. diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index b281ff61f2..5dc6708431 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -22,6 +22,14 @@ module ActionView #:nodoc: end memoize :format_and_extension + def multipart? + format && format.include?('.') + end + + def content_type + format.gsub('.', '/') + end + def mime_type Mime::Type.lookup_by_extension(format) if format end @@ -84,7 +92,7 @@ module ActionView #:nodoc: # [base_path, name, format, extension] def split(file) if m = file.match(/^(.*\/)?([^\.]+)\.?(\w+)?\.?(\w+)?\.?(\w+)?$/) - if m[5] # Mulipart formats + if m[5] # Multipart formats [m[1], m[2], "#{m[3]}.#{m[4]}", m[5]] elsif m[4] # Single format [m[1], m[2], m[3], m[4]] diff --git a/actionpack/test/controller/assert_select_test.rb b/actionpack/test/controller/assert_select_test.rb index 5af579f3e3..1531e7c21a 100644 --- a/actionpack/test/controller/assert_select_test.rb +++ b/actionpack/test/controller/assert_select_test.rb @@ -17,6 +17,8 @@ unless defined?(ActionMailer) end end +ActionMailer::Base.template_root = FIXTURE_LOAD_PATH + class AssertSelectTest < Test::Unit::TestCase class AssertSelectController < ActionController::Base def response_with=(content) @@ -69,11 +71,10 @@ class AssertSelectTest < Test::Unit::TestCase ActionMailer::Base.deliveries = [] end - def teardown ActionMailer::Base.deliveries.clear end - + def assert_failure(message, &block) e = assert_raises(AssertionFailedError, &block) assert_match(message, e.message) if Regexp === message @@ -91,7 +92,6 @@ class AssertSelectTest < Test::Unit::TestCase assert_failure(/Expected at least 1 element matching \"p\", found 0/) { assert_select "p" } end - def test_equality_true_false render_html %Q{
} assert_nothing_raised { assert_select "div" } @@ -102,7 +102,6 @@ class AssertSelectTest < Test::Unit::TestCase assert_nothing_raised { assert_select "p", false } end - def test_equality_string_and_regexp render_html %Q{
foo
foo
} assert_nothing_raised { assert_select "div", "foo" } @@ -116,7 +115,6 @@ class AssertSelectTest < Test::Unit::TestCase assert_raises(AssertionFailedError) { assert_select "p", :text=>/foobar/ } end - def test_equality_of_html render_html %Q{

\n"This is not a big problem," he said.\n

} text = "\"This is not a big problem,\" he said." @@ -135,7 +133,6 @@ class AssertSelectTest < Test::Unit::TestCase assert_raises(AssertionFailedError) { assert_select "pre", :html=>text } end - def test_counts render_html %Q{
foo
foo
} assert_nothing_raised { assert_select "div", 2 } @@ -166,7 +163,6 @@ class AssertSelectTest < Test::Unit::TestCase end end - def test_substitution_values render_html %Q{
foo
foo
} assert_select "div#?", /\d+/ do |elements| @@ -181,7 +177,6 @@ class AssertSelectTest < Test::Unit::TestCase end end - def test_nested_assert_select render_html %Q{
foo
foo
} assert_select "div" do |elements| @@ -200,7 +195,7 @@ class AssertSelectTest < Test::Unit::TestCase assert_select "#3", false end end - + assert_failure(/Expected at least 1 element matching \"#4\", found 0\./) do assert_select "div" do assert_select "#4" @@ -208,7 +203,6 @@ class AssertSelectTest < Test::Unit::TestCase end end - def test_assert_select_text_match render_html %Q{
foo
bar
} assert_select "div" do @@ -225,7 +219,6 @@ class AssertSelectTest < Test::Unit::TestCase end end - # With single result. def test_assert_select_from_rjs_with_single_result render_rjs do |page| @@ -255,19 +248,16 @@ class AssertSelectTest < Test::Unit::TestCase end end - # # Test css_select. # - def test_css_select render_html %Q{
} assert 2, css_select("div").size assert 0, css_select("p").size end - def test_nested_css_select render_html %Q{
foo
foo
} assert_select "div#?", /\d+/ do |elements| @@ -286,7 +276,6 @@ class AssertSelectTest < Test::Unit::TestCase end end - # With one result. def test_css_select_from_rjs_with_single_result render_rjs do |page| @@ -309,12 +298,10 @@ class AssertSelectTest < Test::Unit::TestCase assert_equal 1, css_select("#2").size end - # # Test assert_select_rjs. # - # Test that we can pick up all statements in the result. def test_assert_select_rjs_picks_up_all_statements render_rjs do |page| @@ -381,7 +368,6 @@ class AssertSelectTest < Test::Unit::TestCase assert_raises(AssertionFailedError) { assert_select_rjs "test4" } end - def test_assert_select_rjs_for_replace render_rjs do |page| page.replace "test1", "
foo
" @@ -479,7 +465,7 @@ class AssertSelectTest < Test::Unit::TestCase end end end - + # Simple hide def test_assert_select_rjs_for_hide render_rjs do |page| @@ -500,7 +486,7 @@ class AssertSelectTest < Test::Unit::TestCase end end end - + # Simple toggle def test_assert_select_rjs_for_toggle render_rjs do |page| @@ -521,7 +507,7 @@ class AssertSelectTest < Test::Unit::TestCase end end end - + # Non-positioned insert. def test_assert_select_rjs_for_nonpositioned_insert render_rjs do |page| @@ -568,7 +554,7 @@ class AssertSelectTest < Test::Unit::TestCase assert_select "div", 4 end end - + # Simple selection from a single result. def test_nested_assert_select_rjs_with_single_result render_rjs do |page| @@ -600,7 +586,6 @@ class AssertSelectTest < Test::Unit::TestCase end end - def test_feed_item_encoded render_xml <<-EOF @@ -654,7 +639,6 @@ EOF end end - # # Test assert_select_email # @@ -670,7 +654,6 @@ EOF end end - protected def render_html(html) @controller.response_with = html -- cgit v1.2.3 From be0d235a3b82e72de49592913753a1f291a98f86 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 6 Aug 2008 20:21:15 -0500 Subject: Optimize memoized method if there are no arguments --- activesupport/lib/active_support/memoizable.rb | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/activesupport/lib/active_support/memoizable.rb b/activesupport/lib/active_support/memoizable.rb index 23dd96e4df..7e2e28712c 100644 --- a/activesupport/lib/active_support/memoizable.rb +++ b/activesupport/lib/active_support/memoizable.rb @@ -29,14 +29,24 @@ module ActiveSupport raise "Already memoized #{symbol}" if method_defined?(:#{original_method}) alias #{original_method} #{symbol} - def #{symbol}(*args) - #{memoized_ivar} ||= {} - reload = args.pop if args.last == true || args.last == :reload + if instance_method(:#{symbol}).arity == 0 + def #{symbol}(reload = false) + if !reload && defined? #{memoized_ivar} + #{memoized_ivar} + else + #{memoized_ivar} = #{original_method}.freeze + end + end + else + def #{symbol}(*args) + #{memoized_ivar} ||= {} + reload = args.pop if args.last == true || args.last == :reload - if !reload && #{memoized_ivar} && #{memoized_ivar}.has_key?(args) - #{memoized_ivar}[args] - else - #{memoized_ivar}[args] = #{original_method}(*args).freeze + if !reload && #{memoized_ivar} && #{memoized_ivar}.has_key?(args) + #{memoized_ivar}[args] + else + #{memoized_ivar}[args] = #{original_method}(*args).freeze + end end end EOS -- cgit v1.2.3 From 105093f90728f81268367bd52581fccfa165f170 Mon Sep 17 00:00:00 2001 From: Clemens Kofler Date: Thu, 7 Aug 2008 13:13:47 -0500 Subject: Refactor DateHelper to use DateTimeSelector presenter pattern Signed-off-by: Joshua Peek --- actionpack/lib/action_view/helpers/date_helper.rb | 576 ++++++++++++++-------- actionpack/test/template/date_helper_i18n_test.rb | 22 +- actionpack/test/template/date_helper_test.rb | 299 +++++++++-- 3 files changed, 638 insertions(+), 259 deletions(-) diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index c7a1d40ff2..953a2a9f86 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -13,9 +13,6 @@ module ActionView # the select_month method would use simply "date" (which can be overwritten using :prefix) instead of # "date[month]". module DateHelper - include ActionView::Helpers::TagHelper - DEFAULT_PREFIX = 'date' unless const_defined?('DEFAULT_PREFIX') - # Reports the approximate distance in time between two Time or Date objects or integers as seconds. # Set include_seconds to true if you want more detailed approximations when distance < 1 min, 29 secs # Distances are reported based on the following table: @@ -52,7 +49,7 @@ module ActionView # distance_of_time_in_words(from_time, from_time - 45.seconds, true) # => less than a minute # distance_of_time_in_words(from_time, 76.seconds.from_now) # => 1 minute # distance_of_time_in_words(from_time, from_time + 1.year + 3.days) # => about 1 year - # distance_of_time_in_words(from_time, from_time + 4.years + 15.days + 30.minutes + 5.seconds) # => over 4 years + # distance_of_time_in_words(from_time, from_time + 4.years + 9.days + 30.minutes + 5.seconds) # => over 4 years # # to_time = Time.now + 6.years + 19.days # distance_of_time_in_words(from_time, to_time, true) # => over 6 years @@ -109,19 +106,36 @@ module ActionView alias_method :distance_of_time_in_words_to_now, :time_ago_in_words # Returns a set of select tags (one for year, month, and day) pre-selected for accessing a specified date-based - # attribute (identified by +method+) on an object assigned to the template (identified by +object+). It's - # possible to tailor the selects through the +options+ hash, which accepts all the keys that each of the - # individual select builders do (like :use_month_numbers for select_month) as well as a range of discard - # options. The discard options are :discard_year, :discard_month and :discard_day. Set - # to true, they'll drop the respective select. Discarding the month select will also automatically discard the - # day select. It's also possible to explicitly set the order of the tags using the :order option with an - # array of symbols :year, :month and :day in the desired order. Symbols may be omitted - # and the respective select is not included. - # - # Pass the :default option to set the default date. Use a Time object or a Hash of :year, - # :month, :day, :hour, :minute, and :second. - # - # Passing :disabled => true as part of the +options+ will make elements inaccessible for change. + # attribute (identified by +method+) on an object assigned to the template (identified by +object+). You can + # the output in the +options+ hash. + # + # ==== Options + # * :use_month_numbers - Set to true if you want to use month numbers rather than month names (e.g. + # "2" instead of "February"). + # * :use_short_month - Set to true if you want to use the abbreviated month name instead of the full + # name (e.g. "Feb" instead of "February"). + # * :add_month_number - Set to true if you want to show both, the month's number and name (e.g. + # "2 - February" instead of "February"). + # * :use_month_names - Set to an array with 12 month names if you want to customize month names. + # Note: You can also use Rails' new i18n functionality for this. + # * :date_separator - Specifies a string to separate the date fields. Default is "" (i.e. nothing). + # * :start_year - Set the start year for the year select. Default is Time.now.year - 5. + # * :end_year - Set the end year for the year select. Default is Time.now.year + 5. + # * :discard_day - Set to true if you don't want to show a day select. This includes the day + # as a hidden field instead of showing a select field. Also note that this implicitly sets the day to be the + # first of the given month in order to not create invalid dates like 31 February. + # * :discard_month - Set to true if you don't want to show a month select. This includes the month + # as a hidden field instead of showing a select field. Also note that this implicitly sets :discard_day to true. + # * :discard_year - Set to true if you don't want to show a year select. This includes the year + # as a hidden field instead of showing a select field. + # * :order - Set to an array containing :day, :month and :year do + # customize the order in which the select fields are shown. If you leave out any of the symbols, the respective + # select will not be shown (like when you set :discard_xxx => true. Defaults to the order defined in + # the respective locale (e.g. [:year, :month, :day] in the en-US locale that ships with Rails). + # * :include_blank - Include a blank option in every select field so it's possible to set empty + # dates. + # * :default - Set a default date if the affected date isn't set or is nil. + # * :disabled - Set to true if you want show the select fields as disabled. # # If anything is passed in the +html_options+ hash it will be applied to every select tag in the set. # @@ -165,9 +179,9 @@ module ActionView InstanceTag.new(object_name, method, self, options.delete(:object)).to_date_select_tag(options, html_options) end - # Returns a set of select tags (one for hour, minute and optionally second) pre-selected for accessing a specified - # time-based attribute (identified by +method+) on an object assigned to the template (identified by +object+). - # You can include the seconds with :include_seconds. + # Returns a set of select tags (one for hour, minute and optionally second) pre-selected for accessing a + # specified time-based attribute (identified by +method+) on an object assigned to the template (identified by + # +object+). You can include the seconds with :include_seconds. # # This method will also generate 3 input hidden tags, for the actual year, month and day unless the option # :ignore_date is set to +true+. @@ -178,7 +192,8 @@ module ActionView # # Creates a time select tag that, when POSTed, will be stored in the post variable in the sunrise attribute # time_select("post", "sunrise") # - # # Creates a time select tag that, when POSTed, will be stored in the order variable in the submitted attribute + # # Creates a time select tag that, when POSTed, will be stored in the order variable in the submitted + # # attribute # time_select("order", "submitted") # # # Creates a time select tag that, when POSTed, will be stored in the mail variable in the sent_at attribute @@ -210,7 +225,8 @@ module ActionView # If anything is passed in the html_options hash it will be applied to every select tag in the set. # # ==== Examples - # # Generates a datetime select that, when POSTed, will be stored in the post variable in the written_on attribute + # # Generates a datetime select that, when POSTed, will be stored in the post variable in the written_on + # # attribute # datetime_select("post", "written_on") # # # Generates a datetime select with a year select that starts at 1995 that, when POSTed, will be stored in the @@ -230,12 +246,12 @@ module ActionView InstanceTag.new(object_name, method, self, options.delete(:object)).to_datetime_select_tag(options, html_options) end - # Returns a set of html select-tags (one for year, month, day, hour, and minute) pre-selected with the +datetime+. - # It's also possible to explicitly set the order of the tags using the :order option with an array of - # symbols :year, :month and :day in the desired order. If you do not supply a Symbol, - # it will be appended onto the :order passed in. You can also add :date_separator, - # :datetime_separator and :time_separator keys to the +options+ to control visual display of - # the elements. + # Returns a set of html select-tags (one for year, month, day, hour, and minute) pre-selected with the + # +datetime+. It's also possible to explicitly set the order of the tags using the :order option with + # an array of symbols :year, :month and :day in the desired order. If you do not + # supply a Symbol, it will be appended onto the :order passed in. You can also add + # :date_separator, :datetime_separator and :time_separator keys to the +options+ to + # control visual display of the elements. # # If anything is passed in the html_options hash it will be applied to every select tag in the set. # @@ -270,14 +286,13 @@ module ActionView # select_datetime(my_date_time, :prefix => 'payday') # def select_datetime(datetime = Time.current, options = {}, html_options = {}) - separator = options[:datetime_separator] || '' - select_date(datetime, options, html_options) + separator + select_time(datetime, options, html_options) + DateTimeSelector.new(datetime, options, html_options).select_datetime end # Returns a set of html select-tags (one for year, month, and day) pre-selected with the +date+. # It's possible to explicitly set the order of the tags using the :order option with an array of - # symbols :year, :month and :day in the desired order. If you do not supply a Symbol, it - # will be appended onto the :order passed in. + # symbols :year, :month and :day in the desired order. If you do not supply a Symbol, + # it will be appended onto the :order passed in. # # If anything is passed in the html_options hash it will be applied to every select tag in the set. # @@ -307,12 +322,7 @@ module ActionView # select_date(my_date, :prefix => 'payday') # def select_date(date = Date.current, options = {}, html_options = {}) - options.reverse_merge!(:order => [], :date_separator => '') - [:year, :month, :day].each { |o| options[:order].push(o) unless options[:order].include?(o) } - - options[:order].inject([]) { |s, o| - s << self.send("select_#{o}", date, options, html_options) - }.join(options[:date_separator]) + DateTimeSelector.new(date, options, html_options).select_date end # Returns a set of html select-tags (one for hour and minute) @@ -343,9 +353,7 @@ module ActionView # select_time(my_time, :time_separator => ':', :include_seconds => true) # def select_time(datetime = Time.current, options = {}, html_options = {}) - separator = options[:time_separator] || '' - select_hour(datetime, options, html_options) + separator + select_minute(datetime, options, html_options) + - (options[:include_seconds] ? separator + select_second(datetime, options, html_options) : '') + DateTimeSelector.new(datetime, options, html_options).select_time end # Returns a select tag with options for each of the seconds 0 through 59 with the current second selected. @@ -366,15 +374,12 @@ module ActionView # select_second(my_time, :field_name => 'interval') # def select_second(datetime, options = {}, html_options = {}) - val = datetime ? (datetime.kind_of?(Fixnum) ? datetime : datetime.sec) : '' - options[:use_hidden] ? - (options[:include_seconds] ? _date_hidden_html(options[:field_name] || 'second', val, options) : '') : - _date_select_html(options[:field_name] || 'second', _date_build_options(val), options, html_options) + DateTimeSelector.new(datetime, options, html_options).select_second end # Returns a select tag with options for each of the minutes 0 through 59 with the current minute selected. - # Also can return a select tag with options by minute_step from 0 through 59 with the 00 minute selected - # The minute can also be substituted for a minute number. + # Also can return a select tag with options by minute_step from 0 through 59 with the 00 minute + # selected. The minute can also be substituted for a minute number. # Override the field name using the :field_name option, 'minute' by default. # # ==== Examples @@ -391,11 +396,7 @@ module ActionView # select_minute(my_time, :field_name => 'stride') # def select_minute(datetime, options = {}, html_options = {}) - val = datetime ? (datetime.kind_of?(Fixnum) ? datetime : datetime.min) : '' - options[:use_hidden] ? - _date_hidden_html(options[:field_name] || 'minute', val, options) : - _date_select_html(options[:field_name] || 'minute', - _date_build_options(val, :step => options[:minute_step]), options, html_options) + DateTimeSelector.new(datetime, options, html_options).select_minute end # Returns a select tag with options for each of the hours 0 through 23 with the current hour selected. @@ -416,9 +417,7 @@ module ActionView # select_minute(my_time, :field_name => 'stride') # def select_hour(datetime, options = {}, html_options = {}) - val = datetime ? (datetime.kind_of?(Fixnum) ? datetime : datetime.hour) : '' - options[:use_hidden] ? _date_hidden_html(options[:field_name] || 'hour', val, options) : - _date_select_html(options[:field_name] || 'hour', _date_build_options(val, :end => 23), options, html_options) + DateTimeSelector.new(datetime, options, html_options).select_hour end # Returns a select tag with options for each of the days 1 through 31 with the current day selected. @@ -439,11 +438,7 @@ module ActionView # select_day(my_time, :field_name => 'due') # def select_day(date, options = {}, html_options = {}) - val = date ? (date.kind_of?(Fixnum) ? date : date.day) : '' - options[:use_hidden] ? _date_hidden_html(options[:field_name] || 'day', val, options) : - _date_select_html(options[:field_name] || 'day', - _date_build_options(val, :start => 1, :end => 31, :leading_zeros => false), - options, html_options) + DateTimeSelector.new(date, options, html_options).select_day end # Returns a select tag with options for each of the months January through December with the current month @@ -481,36 +476,7 @@ module ActionView # select_month(Date.today, :use_month_names => %w(Januar Februar Marts ...)) # def select_month(date, options = {}, html_options = {}) - locale = options[:locale] - - val = date ? (date.kind_of?(Fixnum) ? date : date.month) : '' - if options[:use_hidden] - _date_hidden_html(options[:field_name] || 'month', val, options) - else - month_options = [] - month_names = options[:use_month_names] || begin - key = options[:use_short_month] ? :'date.abbr_month_names' : :'date.month_names' - I18n.translate key, :locale => locale - end - month_names.unshift(nil) if month_names.size < 13 - - 1.upto(12) do |month_number| - month_name = if options[:use_month_numbers] - month_number - elsif options[:add_month_numbers] - month_number.to_s + ' - ' + month_names[month_number] - else - month_names[month_number] - end - - month_options << ((val == month_number) ? - content_tag(:option, month_name, :value => month_number, :selected => "selected") : - content_tag(:option, month_name, :value => month_number) - ) - month_options << "\n" - end - _date_select_html(options[:field_name] || 'month', month_options.join, options, html_options) - end + DateTimeSelector.new(date, options, html_options).select_month end # Returns a select tag with options for each of the five years on each side of the current, which is selected. @@ -537,158 +503,369 @@ module ActionView # select_year(2006, :start_year => 2000, :end_year => 2010) # def select_year(date, options = {}, html_options = {}) - if !date || date == 0 + DateTimeSelector.new(date, options, html_options).select_year + end + end + + class DateTimeSelector #:nodoc: + extend ActiveSupport::Memoizable + include ActionView::Helpers::TagHelper + + DEFAULT_PREFIX = 'date'.freeze unless const_defined?('DEFAULT_PREFIX') + POSITION = { + :year => 1, :month => 2, :day => 3, :hour => 4, :minute => 5, :second => 6 + }.freeze unless const_defined?('POSITION') + + def initialize(datetime, options = {}, html_options = {}) + @options = options.dup + @html_options = html_options.dup + @datetime = datetime + end + + def select_datetime + # TODO: Remove tag conditional + # Ideally we could just join select_date and select_date for the tag case + if @options[:tag] && @options[:ignore_date] + select_time + elsif @options[:tag] + order = date_order.dup + order -= [:hour, :minute, :second] + + @options[:discard_year] ||= true unless order.include?(:year) + @options[:discard_month] ||= true unless order.include?(:month) + @options[:discard_day] ||= true if @options[:discard_month] || !order.include?(:day) + @options[:discard_minute] ||= true if @options[:discard_hour] + @options[:discard_second] ||= true unless @options[:include_seconds] && !@options[:discard_minute] + + # If the day is hidden and the month is visible, the day should be set to the 1st so all month choices are + # valid (otherwise it could be 31 and february wouldn't be a valid date) + if @options[:discard_day] && !@options[:discard_month] + @datetime = @datetime.change(:day => 1) + end + + [:day, :month, :year].each { |o| order.unshift(o) unless order.include?(o) } + order += [:hour, :minute, :second] unless @options[:discard_hour] + + build_selects_from_types(order) + else + "#{select_date}#{@options[:datetime_separator]}#{select_time}" + end + end + + def select_date + order = date_order.dup + + # TODO: Remove tag conditional + if @options[:tag] + @options[:discard_hour] = true + @options[:discard_minute] = true + @options[:discard_second] = true + + @options[:discard_year] ||= true unless order.include?(:year) + @options[:discard_month] ||= true unless order.include?(:month) + @options[:discard_day] ||= true if @options[:discard_month] || !order.include?(:day) + + # If the day is hidden and the month is visible, the day should be set to the 1st so all month choices are + # valid (otherwise it could be 31 and february wouldn't be a valid date) + if @options[:discard_day] && !@options[:discard_month] + @datetime = @datetime.change(:day => 1) + end + end + + [:day, :month, :year].each { |o| order.unshift(o) unless order.include?(o) } + + build_selects_from_types(order) + end + + def select_time + order = [] + + # TODO: Remove tag conditional + if @options[:tag] + @options[:discard_month] = true + @options[:discard_year] = true + @options[:discard_day] = true + @options[:discard_second] ||= true unless @options[:include_seconds] + + order += [:year, :month, :day] unless @options[:ignore_date] + end + + order += [:hour, :minute] + order << :second if @options[:include_seconds] + + build_selects_from_types(order) + end + + def select_second + if @options[:use_hidden] || @options[:discard_second] + build_hidden(:second, sec) if @options[:include_seconds] + else + build_options_and_select(:second, sec) + end + end + + def select_minute + if @options[:use_hidden] || @options[:discard_minute] + build_hidden(:minute, min) + else + build_options_and_select(:minute, min, :step => @options[:minute_step]) + end + end + + def select_hour + if @options[:use_hidden] || @options[:discard_hour] + build_hidden(:hour, hour) + else + build_options_and_select(:hour, hour, :end => 23) + end + end + + def select_day + if @options[:use_hidden] || @options[:discard_day] + build_hidden(:day, day) + else + build_options_and_select(:day, day, :start => 1, :end => 31, :leading_zeros => false) + end + end + + def select_month + if @options[:use_hidden] || @options[:discard_month] + build_hidden(:month, month) + else + month_options = [] + 1.upto(12) do |month_number| + options = { :value => month_number } + options[:selected] = "selected" if month == month_number + month_options << content_tag(:option, month_name(month_number), options) + "\n" + end + build_select(:month, month_options.join) + end + end + + def select_year + if !@datetime || @datetime == 0 val = '' middle_year = Date.today.year - elsif date.kind_of?(Fixnum) - val = middle_year = date else - val = middle_year = date.year + val = middle_year = year end - if options[:use_hidden] - _date_hidden_html(options[:field_name] || 'year', val, options) + if @options[:use_hidden] || @options[:discard_year] + build_hidden(:year, val) else - options[:start_year] ||= middle_year - 5 - options[:end_year] ||= middle_year + 5 - step = options[:start_year] < options[:end_year] ? 1 : -1 - - _date_select_html(options[:field_name] || 'year', - _date_build_options(val, - :start => options[:start_year], - :end => options[:end_year], - :step => step, - :leading_zeros => false - ), options, html_options) + options = {} + options[:start] = @options[:start_year] || middle_year - 5 + options[:end] = @options[:end_year] || middle_year + 5 + options[:step] = options[:start] < options[:end] ? 1 : -1 + options[:leading_zeros] = false + + build_options_and_select(:year, val, options) end end private - def _date_build_options(selected, options={}) - options.reverse_merge!(:start => 0, :end => 59, :step => 1, :leading_zeros => true) + %w( sec min hour day month year ).each do |method| + define_method(method) do + @datetime.kind_of?(Fixnum) ? @datetime : @datetime.send(method) if @datetime + end + end + + # Returns translated month names, but also ensures that a custom month + # name array has a leading nil element + def month_names + month_names = @options[:use_month_names] || translated_month_names + month_names.unshift(nil) if month_names.size < 13 + month_names + end + memoize :month_names + + # Returns translated month names + # => [nil, "January", "February", "March", + # "April", "May", "June", "July", + # "August", "September", "October", + # "November", "December"] + # + # If :use_short_month option is set + # => [nil, "Jan", "Feb", "Mar", "Apr", "May", "Jun", + # "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"] + def translated_month_names + begin + key = @options[:use_short_month] ? :'date.abbr_month_names' : :'date.month_names' + I18n.translate(key, :locale => @options[:locale]) + end + end + + # Lookup month name for number + # month_name(1) => "January" + # + # If :use_month_numbers option is passed + # month_name(1) => 1 + # + # If :add_month_numbers option is passed + # month_name(1) => "1 - January" + def month_name(number) + if @options[:use_month_numbers] + number + elsif @options[:add_month_numbers] + "#{number} - #{month_names[number]}" + else + month_names[number] + end + end + + def date_order + @options[:order] || translated_date_order + end + memoize :date_order + + def translated_date_order + begin + I18n.translate(:'date.order', :locale => @options[:locale]) || [] + end + end + + # Build full select tag from date type and options + def build_options_and_select(type, selected, options = {}) + build_select(type, build_options(selected, options)) + end + + # Build select option html from date value and options + # build_options(15, :start => 1, :end => 31) + # => " + # + # ..." + def build_options(selected, options = {}) + start = options.delete(:start) || 0 + stop = options.delete(:end) || 59 + step = options.delete(:step) || 1 + leading_zeros = options.delete(:leading_zeros).nil? ? true : false select_options = [] - (options[:start] || 0).step((options[:end] || 59), options[:step] || 1) do |i| - value = options[:leading_zeros] ? sprintf("%02d", i) : i + start.step(stop, step) do |i| + value = leading_zeros ? sprintf("%02d", i) : i tag_options = { :value => value } tag_options[:selected] = "selected" if selected == i - select_options << content_tag(:option, value, tag_options) end select_options.join("\n") + "\n" end - def _date_select_html(type, html_options, options, select_tag_options = {}) - _date_name_and_id_from_options(options, type) - select_options = {:id => options[:id], :name => options[:name]} - select_options.merge!(:disabled => 'disabled') if options[:disabled] - select_options.merge!(select_tag_options) unless select_tag_options.empty? + # Builds select tag from date type and html select options + # build_select(:month, "...") + # => "" + def build_select(type, select_options_as_html) + select_options = { + :id => input_id_from_type(type), + :name => input_name_from_type(type) + }.merge(@html_options) + select_options.merge!(:disabled => 'disabled') if @options[:disabled] + select_html = "\n" - select_html << content_tag(:option, '', :value => '') + "\n" if options[:include_blank] - select_html << html_options.to_s + select_html << content_tag(:option, '', :value => '') + "\n" if @options[:include_blank] + select_html << select_options_as_html.to_s + content_tag(:select, select_html, select_options) + "\n" end - def _date_hidden_html(type, value, options) - _date_name_and_id_from_options(options, type) - hidden_html = tag(:input, :type => "hidden", :id => options[:id], :name => options[:name], :value => value) + "\n" + # Builds hidden input tag for date part and value + # build_hidden(:year, 2008) + # => "" + def build_hidden(type, value) + tag(:input, { + :type => "hidden", + :id => input_id_from_type(type), + :name => input_name_from_type(type), + :value => value + }) + "\n" end - def _date_name_and_id_from_options(options, type) - options[:name] = (options[:prefix] || DEFAULT_PREFIX) + (options[:discard_type] ? '' : "[#{type}]") - options[:id] = options[:name].gsub(/([\[\(])|(\]\[)/, '_').gsub(/[\]\)]/, '') + # Returns the name attribute for the input tag + # => post[written_on(1i)] + def input_name_from_type(type) + prefix = @options[:prefix] || ActionView::Helpers::DateTimeSelector::DEFAULT_PREFIX + prefix += "[#{@options[:index]}]" if @options[:index] + + field_name = @options[:field_name] || type + if @options[:include_position] + field_name += "(#{ActionView::Helpers::DateTimeSelector::POSITION[type]}i)" + end + + @options[:discard_type] ? prefix : "#{prefix}[#{field_name}]" + end + + # Returns the id attribute for the input tag + # => "post_written_on_1i" + def input_id_from_type(type) + input_name_from_type(type).gsub(/([\[\(])|(\]\[)/, '_').gsub(/[\]\)]/, '') + end + + # Given an ordering of datetime components, create the selection html + # and join them with their appropriate seperators + def build_selects_from_types(order) + select = '' + order.reverse.each do |type| + separator = separator(type) unless type == order.first # don't add on last field + select.insert(0, separator.to_s + send("select_#{type}").to_s) + end + select + end + + # Returns the separator for a given datetime component + def separator(type) + case type + when :month, :day + @options[:date_separator] + when :hour + (@options[:discard_year] && @options[:discard_day]) ? "" : @options[:datetime_separator] + when :minute + @options[:time_separator] + when :second + @options[:include_seconds] ? @options[:time_separator] : "" + end end end class InstanceTag #:nodoc: - include DateHelper - def to_date_select_tag(options = {}, html_options = {}) - date_or_time_select(options.merge(:discard_hour => true), html_options) + datetime_selector(options, html_options).select_date end def to_time_select_tag(options = {}, html_options = {}) - date_or_time_select(options.merge(:discard_year => true, :discard_month => true), html_options) + datetime_selector(options, html_options).select_time end def to_datetime_select_tag(options = {}, html_options = {}) - date_or_time_select(options, html_options) + datetime_selector(options, html_options).select_datetime end private - def date_or_time_select(options, html_options = {}) - locale = options[:locale] - - defaults = { :discard_type => true } - options = defaults.merge(options) - datetime = value(object) - datetime ||= default_time_from_options(options[:default]) unless options[:include_blank] - - position = { :year => 1, :month => 2, :day => 3, :hour => 4, :minute => 5, :second => 6 } - - order = options[:order] ||= I18n.translate(:'date.order', :locale => locale) - - # Discard explicit and implicit by not being included in the :order - discard = {} - discard[:year] = true if options[:discard_year] or !order.include?(:year) - discard[:month] = true if options[:discard_month] or !order.include?(:month) - discard[:day] = true if options[:discard_day] or discard[:month] or !order.include?(:day) - discard[:hour] = true if options[:discard_hour] - discard[:minute] = true if options[:discard_minute] or discard[:hour] - discard[:second] = true unless options[:include_seconds] && !discard[:minute] - - # If the day is hidden and the month is visible, the day should be set to the 1st so all month choices are valid - # (otherwise it could be 31 and february wouldn't be a valid date) - if datetime && discard[:day] && !discard[:month] - datetime = datetime.change(:day => 1) - end - - # Maintain valid dates by including hidden fields for discarded elements - [:day, :month, :year].each { |o| order.unshift(o) unless order.include?(o) } - - # Ensure proper ordering of :hour, :minute and :second - [:hour, :minute, :second].each { |o| order.delete(o); order.push(o) } - - date_or_time_select = '' - order.reverse.each do |param| - # Send hidden fields for discarded elements once output has started - # This ensures AR can reconstruct valid dates using ParseDate - next if discard[param] && (date_or_time_select.empty? || options[:ignore_date]) - - date_or_time_select.insert(0, - self.send("select_#{param}", - datetime, - options_with_prefix(position[param], options.merge(:use_hidden => discard[param])), - html_options)) - date_or_time_select.insert(0, - case param - when :hour then (discard[:year] && discard[:day] ? "" : " — ") - when :minute then " : " - when :second then options[:include_seconds] ? " : " : "" - else "" - end) - end - - date_or_time_select + def datetime_selector(options, html_options) + datetime = value(object) || default_datetime(options) + + options = options.dup + options[:field_name] = @method_name + options[:include_position] = true + options[:prefix] ||= @object_name + options[:index] ||= @auto_index + options[:datetime_separator] ||= ' — ' + options[:time_separator] ||= ' : ' + + DateTimeSelector.new(datetime, options.merge(:tag => true), html_options) end - def options_with_prefix(position, options) - prefix = "#{@object_name}" - if options[:index] - prefix << "[#{options[:index]}]" - elsif @auto_index - prefix << "[#{@auto_index}]" - end - options.merge(:prefix => "#{prefix}[#{@method_name}(#{position}i)]") - end + def default_datetime(options) + return if options[:include_blank] - def default_time_from_options(default) - case default + case options[:default] when nil Time.current when Date, Time - default + options[:default] else + default = options[:default].dup + # Rename :minute and :second to :min and :sec default[:min] ||= default[:minute] default[:sec] ||= default[:second] @@ -699,8 +876,11 @@ module ActionView default[key] ||= time.send(key) end - Time.utc_time(default[:year], default[:month], default[:day], default[:hour], default[:min], default[:sec]) - end + Time.utc_time( + default[:year], default[:month], default[:day], + default[:hour], default[:min], default[:sec] + ) + end end end diff --git a/actionpack/test/template/date_helper_i18n_test.rb b/actionpack/test/template/date_helper_i18n_test.rb index aca3593921..2b40074498 100644 --- a/actionpack/test/template/date_helper_i18n_test.rb +++ b/actionpack/test/template/date_helper_i18n_test.rb @@ -3,22 +3,22 @@ require 'abstract_unit' class DateHelperDistanceOfTimeInWordsI18nTests < Test::Unit::TestCase include ActionView::Helpers::DateHelper attr_reader :request - + def setup @from = Time.mktime(2004, 6, 6, 21, 45, 0) end - + uses_mocha 'date_helper_distance_of_time_in_words_i18n_test' do # distance_of_time_in_words def test_distance_of_time_in_words_calls_i18n { # with include_seconds - [2.seconds, true] => [:'less_than_x_seconds', 5], - [9.seconds, true] => [:'less_than_x_seconds', 10], - [19.seconds, true] => [:'less_than_x_seconds', 20], - [30.seconds, true] => [:'half_a_minute', nil], - [59.seconds, true] => [:'less_than_x_minutes', 1], - [60.seconds, true] => [:'x_minutes', 1], + [2.seconds, true] => [:'less_than_x_seconds', 5], + [9.seconds, true] => [:'less_than_x_seconds', 10], + [19.seconds, true] => [:'less_than_x_seconds', 20], + [30.seconds, true] => [:'half_a_minute', nil], + [59.seconds, true] => [:'less_than_x_minutes', 1], + [60.seconds, true] => [:'x_minutes', 1], # without include_seconds [29.seconds, false] => [:'less_than_x_minutes', 1], @@ -38,7 +38,7 @@ class DateHelperDistanceOfTimeInWordsI18nTests < Test::Unit::TestCase def assert_distance_of_time_in_words_translates_key(passed, expected) diff, include_seconds = *passed - key, count = *expected + key, count = *expected to = @from + diff options = {:locale => 'en-US', :scope => :'datetime.distance_in_words'} @@ -49,11 +49,11 @@ class DateHelperDistanceOfTimeInWordsI18nTests < Test::Unit::TestCase end end end - + class DateHelperSelectTagsI18nTests < Test::Unit::TestCase include ActionView::Helpers::DateHelper attr_reader :request - + uses_mocha 'date_helper_select_tags_i18n_tests' do def setup I18n.stubs(:translate).with(:'date.month_names', :locale => 'en-US').returns Date::MONTHNAMES diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index d8c07e731b..1a645bccc6 100644 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -557,11 +557,8 @@ class DateHelperTest < ActionView::TestCase end def test_select_date_with_incomplete_order - expected = %(\n" - - expected << %(\n) expected << %(\n\n\n) expected << "\n" @@ -569,6 +566,10 @@ class DateHelperTest < ActionView::TestCase expected << %(\n\n\n\n\n\n\n\n\n\n\n\n) expected << "\n" + expected << %(\n" + assert_dom_equal expected, select_date(Time.mktime(2003, 8, 16), :start_year => 2003, :end_year => 2005, :prefix => "date[first]", :order => [:day]) end @@ -909,6 +910,10 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, select_datetime(Time.mktime(2003, 8, 16, 8, 4, 18), { :datetime_separator => "—", :date_separator => "/", :time_separator => ":", :start_year => 2003, :end_year => 2005, :prefix => "date[first]"}, :class => 'selector') end + def test_select_datetime_should_work_with_date + assert_nothing_raised { select_datetime(Date.today) } + end + def test_select_time expected = %(\n} + expected << %{\n\n\n\n\n\n\n\n\n\n\n} + expected << "\n" + + expected << " / " + + expected << %{\n" + + expected << " / " + + expected << %{\n" + + assert_dom_equal expected, date_select("post", "written_on", { :date_separator => " / " }) + end + def test_time_select @post = Post.new @post.written_on = Time.local(2004, 6, 15, 15, 16, 35) @@ -1330,6 +1336,33 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + def test_time_select_with_separator + @post = Post.new + @post.written_on = Time.local(2004, 6, 15, 15, 16, 35) + + expected = %{\n} + expected << %{\n} + expected << %{\n} + + expected << %(\n" + + expected << " - " + + expected << %(\n" + + expected << " - " + + expected << %(\n" + + assert_dom_equal expected, time_select("post", "written_on", { :time_separator => " - ", :include_seconds => true }) + end + def test_datetime_select @post = Post.new @post.updated_at = Time.local(2004, 6, 15, 16, 35) @@ -1412,6 +1445,47 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + def test_datetime_select_with_separators + @post = Post.new + @post.updated_at = Time.local(2004, 6, 15, 15, 16, 35) + + expected = %{\n" + + expected << " / " + + expected << %{\n" + + expected << " / " + + expected << %{\n" + + expected << " , " + + expected << %(\n" + + expected << " - " + + expected << %(\n" + + expected << " - " + + expected << %(\n" + + assert_dom_equal expected, datetime_select("post", "updated_at", { :date_separator => " / ", :datetime_separator => " , ", :time_separator => " - ", :include_seconds => true }) + end + def test_date_select_with_zero_value_and_no_start_year expected = %(