From 4163ccec2343ee66e2488f067eab2a15260e1219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 22 Apr 2010 12:00:13 +0200 Subject: Clean up the config object in ActionPack. Create config_accessor which just delegates to the config object, reducing the number of deprecations and add specific tests. --- actionpack/lib/abstract_controller.rb | 1 - actionpack/lib/abstract_controller/assigns.rb | 21 ---- actionpack/lib/abstract_controller/base.rb | 17 +-- actionpack/lib/abstract_controller/helpers.rb | 1 - actionpack/lib/abstract_controller/logger.rb | 2 +- actionpack/lib/action_controller/base.rb | 5 +- actionpack/lib/action_controller/caching.rb | 6 +- actionpack/lib/action_controller/caching/pages.rb | 8 +- .../lib/action_controller/deprecated/base.rb | 48 ++------- .../lib/action_controller/metal/compatibility.rb | 4 +- actionpack/lib/action_controller/metal/helpers.rb | 4 +- .../metal/request_forgery_protection.rb | 118 ++++++++------------- actionpack/lib/action_controller/railtie.rb | 65 +++++------- actionpack/lib/action_view/test_case.rb | 2 +- actionpack/test/template/form_tag_helper_test.rb | 3 - activesupport/lib/active_support/configurable.rb | 37 +++---- activesupport/test/configurable_test.rb | 42 ++++++++ railties/lib/rails/application/configuration.rb | 4 +- 18 files changed, 162 insertions(+), 226 deletions(-) delete mode 100644 actionpack/lib/abstract_controller/assigns.rb create mode 100644 activesupport/test/configurable_test.rb diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb index de95f935c2..2da4dc052c 100644 --- a/actionpack/lib/abstract_controller.rb +++ b/actionpack/lib/abstract_controller.rb @@ -12,7 +12,6 @@ require 'active_support/i18n' module AbstractController extend ActiveSupport::Autoload - autoload :Assigns autoload :Base autoload :Callbacks autoload :Collector diff --git a/actionpack/lib/abstract_controller/assigns.rb b/actionpack/lib/abstract_controller/assigns.rb deleted file mode 100644 index 21459c6d51..0000000000 --- a/actionpack/lib/abstract_controller/assigns.rb +++ /dev/null @@ -1,21 +0,0 @@ -module AbstractController - module Assigns - # This method should return a hash with assigns. - # You can overwrite this configuration per controller. - # :api: public - def view_assigns - hash = {} - variables = instance_variable_names - variables -= protected_instance_variables if respond_to?(:protected_instance_variables) - variables.each { |name| hash[name] = instance_variable_get(name) } - hash - end - - # This method assigns the hash specified in _assigns_hash to the given object. - # :api: private - # TODO Ideally, this should be done on AV::Base.new initialization. - def _evaluate_assigns(object) - view_assigns.each { |k,v| object.instance_variable_set(k, v) } - end - end -end \ No newline at end of file diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index c12b584144..8500cbd7f2 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -1,4 +1,4 @@ -require 'active_support/ordered_options' +require 'active_support/configurable' module AbstractController class Error < StandardError; end @@ -8,6 +8,8 @@ module AbstractController attr_internal :response_body attr_internal :action_name + include ActiveSupport::Configurable + class << self attr_reader :abstract alias_method :abstract?, :abstract @@ -29,14 +31,6 @@ module AbstractController @descendants ||= [] end - def config - @config ||= ActiveSupport::InheritableOptions.new(superclass < Base ? superclass.config : {}) - end - - def configure - yield config - end - # A list of all internal methods for a controller. This finds the first # abstract superclass of a controller, and gets a list of all public # instance methods on that abstract class. Public instance methods of @@ -99,10 +93,6 @@ module AbstractController abstract! - def config - @config ||= ActiveSupport::InheritableOptions.new(self.class.config) - end - # Calls the action going through the entire action dispatch stack. # # The actual method that is called is determined by calling @@ -133,6 +123,7 @@ module AbstractController end private + # Returns true if the name can be considered an action. This can # be overridden in subclasses to modify the semantics of what # can be considered an action. diff --git a/actionpack/lib/abstract_controller/helpers.rb b/actionpack/lib/abstract_controller/helpers.rb index 53cf6b3931..4374b439d0 100644 --- a/actionpack/lib/abstract_controller/helpers.rb +++ b/actionpack/lib/abstract_controller/helpers.rb @@ -8,7 +8,6 @@ module AbstractController included do class_attribute :_helpers - delegate :_helpers, :to => :'self.class' self._helpers = Module.new end diff --git a/actionpack/lib/abstract_controller/logger.rb b/actionpack/lib/abstract_controller/logger.rb index 9318f5e369..0b196119f4 100644 --- a/actionpack/lib/abstract_controller/logger.rb +++ b/actionpack/lib/abstract_controller/logger.rb @@ -6,7 +6,7 @@ module AbstractController extend ActiveSupport::Concern included do - cattr_accessor :logger + config_accessor :logger extend ActiveSupport::Benchmarkable end end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 2e94a20d9d..092fe98588 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -65,8 +65,11 @@ module ActionController @subclasses ||= [] end + # TODO Move this to the appropriate module + config_accessor :assets_dir, :asset_path, :javascripts_dir, :stylesheets_dir + ActiveSupport.run_load_hooks(:action_controller, self) end end -require "action_controller/deprecated/base" +require "action_controller/deprecated/base" \ No newline at end of file diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index 0da0ca1893..4105f9e14f 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -63,12 +63,10 @@ module ActionController #:nodoc: included do extend ConfigMethods - delegate :perform_caching, :perform_caching=, :to => :config - singleton_class.delegate :perform_caching, :perform_caching=, :to => :config - self.perform_caching = true + config_accessor :perform_caching + self.perform_caching = true if perform_caching.nil? end - def caching_allowed? request.get? && response.status == 200 end diff --git a/actionpack/lib/action_controller/caching/pages.rb b/actionpack/lib/action_controller/caching/pages.rb index 20df3a1a69..cefd1f48c0 100644 --- a/actionpack/lib/action_controller/caching/pages.rb +++ b/actionpack/lib/action_controller/caching/pages.rb @@ -44,8 +44,8 @@ module ActionController #:nodoc: # For Rails, this directory has already been set to Rails.public_path (which is usually set to Rails.root + "/public"). Changing # this setting can be useful to avoid naming conflicts with files in public/, but doing so will likely require configuring your # web server to look in the new location for cached files. - singleton_class.delegate :page_cache_directory, :page_cache_directory=, :to => :config - self.page_cache_directory = '' + config_accessor :page_cache_directory + self.page_cache_directory ||= '' ## # :singleton-method: @@ -53,8 +53,8 @@ module ActionController #:nodoc: # order to make it easy for the cached files to be picked up properly by the web server. By default, this cache extension is .html. # If you want something else, like .php or .shtml, just set Base.page_cache_extension. In cases where a request already has an # extension, such as .xml or .rss, page caching will not add an extension. This allows it to work well with RESTful apps. - singleton_class.delegate :page_cache_extension, :page_cache_extension=, :to => :config - self.page_cache_extension = '.html' + config_accessor :page_cache_extension + self.page_cache_extension ||= '.html' end module ClassMethods diff --git a/actionpack/lib/action_controller/deprecated/base.rb b/actionpack/lib/action_controller/deprecated/base.rb index 57203ce95f..3975afcaf0 100644 --- a/actionpack/lib/action_controller/deprecated/base.rb +++ b/actionpack/lib/action_controller/deprecated/base.rb @@ -1,33 +1,16 @@ module ActionController class Base - class << self - def deprecated_config_accessor(option, message = nil) - deprecated_config_reader(option, message) - deprecated_config_writer(option, message) + # Deprecated methods. Wrap them in a module so they can be overwritten by plugins + # (like the verify method.) + module DeprecatedBehavior #:nodoc: + def relative_url_root + ActiveSupport::Deprecation.warn "ActionController::Base.relative_url_root is ineffective. " << + "Please stop using it.", caller end - def deprecated_config_reader(option, message = nil) - message ||= "Reading #{option} directly from ActionController::Base is deprecated. " \ - "Please read it from config.#{option}" - - self.class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{option} - ActiveSupport::Deprecation.warn #{message.inspect}, caller - config.#{option} - end - RUBY - end - - def deprecated_config_writer(option, message = nil) - message ||= "Setting #{option} directly on ActionController::Base is deprecated. " \ - "Please set it on config.action_controller.#{option}" - - self.class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{option}=(val) - ActiveSupport::Deprecation.warn #{message.inspect}, caller - config.#{option} = val - end - RUBY + def relative_url_root= + ActiveSupport::Deprecation.warn "ActionController::Base.relative_url_root= is ineffective. " << + "Please stop using it.", caller end def consider_all_requests_local @@ -125,9 +108,7 @@ module ActionController def use_accept_header=(val) use_accept_header end - end - module DeprecatedBehavior # This method has been moved to ActionDispatch::Request.filter_parameters def filter_parameter_logging(*args, &block) ActiveSupport::Deprecation.warn("Setting filter_parameter_logging in ActionController is deprecated and has no longer effect, please set 'config.filter_parameters' in config/application.rb instead", caller) @@ -146,17 +127,6 @@ module ActionController extend DeprecatedBehavior - deprecated_config_writer :session_options - deprecated_config_writer :session_store - - deprecated_config_accessor :assets_dir - deprecated_config_accessor :asset_path - deprecated_config_accessor :helpers_path - deprecated_config_accessor :javascripts_dir - deprecated_config_accessor :page_cache_directory - deprecated_config_accessor :relative_url_root, "relative_url_root is ineffective. Please stop using it" - deprecated_config_accessor :stylesheets_dir - delegate :consider_all_requests_local, :consider_all_requests_local=, :allow_concurrency, :allow_concurrency=, :to => :"self.class" end diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb index 02722360f1..d49465fa0b 100644 --- a/actionpack/lib/action_controller/metal/compatibility.rb +++ b/actionpack/lib/action_controller/metal/compatibility.rb @@ -21,8 +21,8 @@ module ActionController delegate :default_charset=, :to => "ActionDispatch::Response" end - # cattr_reader :protected_instance_variables - cattr_accessor :protected_instance_variables + # TODO: Update protected instance variables list + config_accessor :protected_instance_variables self.protected_instance_variables = %w(@assigns @performed_redirect @performed_render @variables_added @request_origin @url @parent_controller @action_name diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index 82bedc3fad..1110d7c117 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -52,8 +52,8 @@ module ActionController include AbstractController::Helpers included do - class_attribute :helpers_path - self.helpers_path = [] + config_accessor :helpers_path + self.helpers_path ||= [] end module ClassMethods diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 39a809657b..2ba0d6e5cd 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -4,6 +4,45 @@ module ActionController #:nodoc: class InvalidAuthenticityToken < ActionControllerError #:nodoc: end + # Protecting controller actions from CSRF attacks by ensuring that all forms are coming from the current + # web application, not a forged link from another site, is done by embedding a token based on a random + # string stored in the session (which an attacker wouldn't know) in all forms and Ajax requests generated + # by Rails and then verifying the authenticity of that token in the controller. Only HTML/JavaScript + # requests are checked, so this will not protect your XML API (presumably you'll have a different + # authentication scheme there anyway). Also, GET requests are not protected as these should be + # idempotent anyway. + # + # This is turned on with the protect_from_forgery method, which will check the token and raise an + # ActionController::InvalidAuthenticityToken if it doesn't match what was expected. You can customize the + # error message in production by editing public/422.html. A call to this method in ApplicationController is + # generated by default in post-Rails 2.0 applications. + # + # The token parameter is named authenticity_token by default. If you are generating an HTML form + # manually (without the use of Rails' form_for, form_tag or other helpers), you have to + # include a hidden field named like that and set its value to what is returned by + # form_authenticity_token. + # + # Request forgery protection is disabled by default in test environment. If you are upgrading from Rails + # 1.x, add this to config/environments/test.rb: + # + # # Disable request forgery protection in test environment + # config.action_controller.allow_forgery_protection = false + # + # == Learn more about CSRF (Cross-Site Request Forgery) attacks + # + # Here are some resources: + # * http://isc.sans.org/diary.html?storyid=1750 + # * http://en.wikipedia.org/wiki/Cross-site_request_forgery + # + # Keep in mind, this is NOT a silver-bullet, plug 'n' play, warm security blanket for your rails application. + # There are a few guidelines you should follow: + # + # * Keep your GET requests safe and idempotent. More reading material: + # * http://www.xml.com/pub/a/2002/04/24/deviant.html + # * http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.1.1 + # * Make sure the session cookies that Rails creates are non-persistent. Check in Firefox and look + # for "Expires: at end of session" + # module RequestForgeryProtection extend ActiveSupport::Concern @@ -12,54 +51,17 @@ module ActionController #:nodoc: included do # Sets the token parameter name for RequestForgery. Calling +protect_from_forgery+ # sets it to :authenticity_token by default. - config.request_forgery_protection_token ||= :authenticity_token + config_accessor :request_forgery_protection_token + self.request_forgery_protection_token ||= :authenticity_token # Controls whether request forgergy protection is turned on or not. Turned off by default only in test mode. - config.allow_forgery_protection ||= true + config_accessor :allow_forgery_protection + self.allow_forgery_protection = true if allow_forgery_protection.nil? helper_method :form_authenticity_token helper_method :protect_against_forgery? end - # Protecting controller actions from CSRF attacks by ensuring that all forms are coming from the current - # web application, not a forged link from another site, is done by embedding a token based on a random - # string stored in the session (which an attacker wouldn't know) in all forms and Ajax requests generated - # by Rails and then verifying the authenticity of that token in the controller. Only HTML/JavaScript - # requests are checked, so this will not protect your XML API (presumably you'll have a different - # authentication scheme there anyway). Also, GET requests are not protected as these should be - # idempotent anyway. - # - # This is turned on with the protect_from_forgery method, which will check the token and raise an - # ActionController::InvalidAuthenticityToken if it doesn't match what was expected. You can customize the - # error message in production by editing public/422.html. A call to this method in ApplicationController is - # generated by default in post-Rails 2.0 applications. - # - # The token parameter is named authenticity_token by default. If you are generating an HTML form - # manually (without the use of Rails' form_for, form_tag or other helpers), you have to - # include a hidden field named like that and set its value to what is returned by - # form_authenticity_token. - # - # Request forgery protection is disabled by default in test environment. If you are upgrading from Rails - # 1.x, add this to config/environments/test.rb: - # - # # Disable request forgery protection in test environment - # config.action_controller.allow_forgery_protection = false - # - # == Learn more about CSRF (Cross-Site Request Forgery) attacks - # - # Here are some resources: - # * http://isc.sans.org/diary.html?storyid=1750 - # * http://en.wikipedia.org/wiki/Cross-site_request_forgery - # - # Keep in mind, this is NOT a silver-bullet, plug 'n' play, warm security blanket for your rails application. - # There are a few guidelines you should follow: - # - # * Keep your GET requests safe and idempotent. More reading material: - # * http://www.xml.com/pub/a/2002/04/24/deviant.html - # * http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.1.1 - # * Make sure the session cookies that Rails creates are non-persistent. Check in Firefox and look - # for "Expires: at end of session" - # module ClassMethods # Turn on request forgery protection. Bear in mind that only non-GET, HTML/JavaScript requests are checked. # @@ -79,22 +81,6 @@ module ActionController #:nodoc: self.request_forgery_protection_token ||= :authenticity_token before_filter :verify_authenticity_token, options end - - def request_forgery_protection_token - config.request_forgery_protection_token - end - - def request_forgery_protection_token=(val) - config.request_forgery_protection_token = val - end - - def allow_forgery_protection - config.allow_forgery_protection - end - - def allow_forgery_protection=(val) - config.allow_forgery_protection = val - end end protected @@ -104,22 +90,6 @@ module ActionController #:nodoc: before_filter :verify_authenticity_token, options end - def request_forgery_protection_token - config.request_forgery_protection_token - end - - def request_forgery_protection_token=(val) - config.request_forgery_protection_token = val - end - - def allow_forgery_protection - config.allow_forgery_protection - end - - def allow_forgery_protection=(val) - config.allow_forgery_protection = val - end - # The actual before_filter that is used. Modify this to change how you handle unverified requests. def verify_authenticity_token verified_request? || raise(ActionController::InvalidAuthenticityToken) @@ -146,7 +116,7 @@ module ActionController #:nodoc: end def protect_against_forgery? - config.allow_forgery_protection + allow_forgery_protection end end end diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index b029434004..0e3cdffadc 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -13,64 +13,51 @@ module ActionController class Railtie < Rails::Railtie config.action_controller = ActiveSupport::OrderedOptions.new - ad = config.action_dispatch - config.action_controller.singleton_class.send(:define_method, :session) do - ActiveSupport::Deprecation.warn "config.action_controller.session has been " \ - "renamed to config.action_dispatch.session.", caller - ad.session - end + config.action_controller.singleton_class.tap do |d| + d.send(:define_method, :session) do + ActiveSupport::Deprecation.warn "config.action_controller.session has been deprecated. " << + "Please use Rails.application.config.session_store instead.", caller + end - config.action_controller.singleton_class.send(:define_method, :session=) do |val| - ActiveSupport::Deprecation.warn "config.action_controller.session has been " \ - "renamed to config.action_dispatch.session.", caller - ad.session = val - end + d.send(:define_method, :session=) do |val| + ActiveSupport::Deprecation.warn "config.action_controller.session= has been deprecated. " << + "Please use config.session_store(name, options) instead.", caller + end - config.action_controller.singleton_class.send(:define_method, :session_store) do - ActiveSupport::Deprecation.warn "config.action_controller.session_store has been " \ - "renamed to config.action_dispatch.session_store.", caller - ad.session_store - end + d.send(:define_method, :session_store) do + ActiveSupport::Deprecation.warn "config.action_controller.session_store has been deprecated. " << + "Please use Rails.application.config.session_store instead.", caller + end - config.action_controller.singleton_class.send(:define_method, :session_store=) do |val| - ActiveSupport::Deprecation.warn "config.action_controller.session_store has been " \ - "renamed to config.action_dispatch.session_store.", caller - ad.session_store = val + d.send(:define_method, :session_store=) do |val| + ActiveSupport::Deprecation.warn "config.action_controller.session_store= has been deprecated. " << + "Please use config.session_store(name, options) instead.", caller + end end log_subscriber :action_controller, ActionController::Railties::LogSubscriber.new - initializer "action_controller.logger" do - ActiveSupport.on_load(:action_controller) { self.logger ||= Rails.logger } - end - - initializer "action_controller.page_cache_directory" do - ActiveSupport.on_load(:action_controller) do - self.page_cache_directory = Rails.public_path - end - end - initializer "action_controller.set_configs" do |app| paths = app.config.paths ac = app.config.action_controller - ac.assets_dir = paths.public.to_a.first - ac.javascripts_dir = paths.public.javascripts.to_a.first - ac.stylesheets_dir = paths.public.stylesheets.to_a.first + ac.assets_dir ||= paths.public.to_a.first + ac.javascripts_dir ||= paths.public.javascripts.to_a.first + ac.stylesheets_dir ||= paths.public.stylesheets.to_a.first + ac.page_cache_directory ||= paths.public.to_a.first + ac.helpers_path ||= paths.app.helpers.to_a ActiveSupport.on_load(:action_controller) do self.config.merge!(ac) end end - initializer "action_controller.initialize_framework_caches" do - ActiveSupport.on_load(:action_controller) { self.cache_store ||= RAILS_CACHE } + initializer "action_controller.logger" do + ActiveSupport.on_load(:action_controller) { self.logger ||= Rails.logger } end - initializer "action_controller.set_helpers_path" do |app| - ActiveSupport.on_load(:action_controller) do - self.helpers_path = app.config.paths.app.helpers.to_a - end + initializer "action_controller.initialize_framework_caches" do + ActiveSupport.on_load(:action_controller) { self.cache_store ||= RAILS_CACHE } end initializer "action_controller.url_helpers" do |app| diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index beda7743bf..f6417d5c2c 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -31,8 +31,8 @@ module ActionView include ActionController::PolymorphicRoutes include ActionController::RecordIdentifier + include AbstractController::Helpers include ActionView::Helpers - include ActionController::Helpers class_inheritable_accessor :helper_class attr_accessor :controller, :output_buffer, :rendered diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index ef612b879b..8756bd310f 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -3,9 +3,6 @@ require 'abstract_unit' class FormTagHelperTest < ActionView::TestCase tests ActionView::Helpers::FormTagHelper - # include ActiveSupport::Configurable - # DEFAULT_CONFIG = ActionView::DEFAULT_CONFIG - def setup super @controller = BasicController.new diff --git a/activesupport/lib/active_support/configurable.rb b/activesupport/lib/active_support/configurable.rb index 890f465ce1..f562e17c75 100644 --- a/activesupport/lib/active_support/configurable.rb +++ b/activesupport/lib/active_support/configurable.rb @@ -1,35 +1,36 @@ -require "active_support/concern" +require 'active_support/concern' +require 'active_support/ordered_options' +require 'active_support/core_ext/kernel/singleton_class' +require 'active_support/core_ext/module/delegation' module ActiveSupport module Configurable extend ActiveSupport::Concern module ClassMethods - def get_config - module_parts = name.split("::") - modules = [Object] - module_parts.each {|name| modules.push modules.last.const_get(name) } - modules.reverse_each do |mod| - return mod.const_get(:DEFAULT_CONFIG) if const_defined?(:DEFAULT_CONFIG) - end - {} - end - def config - self.config = get_config unless @config - @config + @config ||= ActiveSupport::InheritableOptions.new(superclass.respond_to?(:config) ? superclass.config : {}) end - def config=(hash) - @config = ActiveSupport::OrderedOptions.new - hash.each do |key, value| - @config[key] = value + def configure + yield config + end + + def config_accessor(*names) + names.each do |name| + code, line = <<-RUBY, __LINE__ + 1 + def #{name}; config.#{name}; end + def #{name}=(value); config.#{name} = value; end + RUBY + + singleton_class.class_eval code, __FILE__, line + class_eval code, __FILE__, line end end end def config - self.class.config + @config ||= ActiveSupport::InheritableOptions.new(self.class.config) end end end \ No newline at end of file diff --git a/activesupport/test/configurable_test.rb b/activesupport/test/configurable_test.rb new file mode 100644 index 0000000000..cef67e3cf9 --- /dev/null +++ b/activesupport/test/configurable_test.rb @@ -0,0 +1,42 @@ +require 'abstract_unit' +require 'active_support/configurable' + +class ConfigurableActiveSupport < ActiveSupport::TestCase + class Parent + include ActiveSupport::Configurable + config_accessor :foo + end + + class Child < Parent + end + + setup do + Parent.config.clear + Parent.config.foo = :bar + + Child.config.clear + end + + test "adds a configuration hash" do + assert_equal({ :foo => :bar }, Parent.config) + end + + test "configuration hash is inheritable" do + assert_equal :bar, Child.config.foo + assert_equal :bar, Parent.config.foo + + Child.config.foo = :baz + assert_equal :baz, Child.config.foo + assert_equal :bar, Parent.config.foo + end + + test "configuration hash is available on instance" do + instance = Parent.new + assert_equal :bar, instance.config.foo + assert_equal :bar, Parent.config.foo + + instance.config.foo = :baz + assert_equal :baz, instance.config.foo + assert_equal :bar, Parent.config.foo + end +end \ No newline at end of file diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 11bf6a6e72..874b3a78b6 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -128,13 +128,13 @@ module Rails end end - protected - def session_options return @session_options unless @session_store == :cookie_store @session_options.merge(:secret => @secret_token) end + protected + def default_middleware_stack ActionDispatch::MiddlewareStack.new.tap do |middleware| middleware.use('::ActionDispatch::Static', lambda { paths.public.to_a.first }, :if => lambda { serve_static_assets }) -- cgit v1.2.3 From 81fb742488e696ee7a7a8b1a620bc93deb1fad77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 22 Apr 2010 12:12:38 +0200 Subject: Always downstream given options in :json, :xml and :js renderers and add tests for it. --- .../lib/action_controller/metal/renderers.rb | 4 ++-- actionpack/test/controller/render_json_test.rb | 18 ++++++++++++++++ actionpack/test/controller/render_xml_test.rb | 24 +++++++++++++++------- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index 47759aa2fe..0be07cd1fc 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -79,12 +79,12 @@ module ActionController add :js do |js, options| self.content_type ||= Mime::JS - self.response_body = js.respond_to?(:to_js) ? js.to_js : js + self.response_body = js.respond_to?(:to_js) ? js.to_js(options) : js end add :xml do |xml, options| self.content_type ||= Mime::XML - self.response_body = xml.respond_to?(:to_xml) ? xml.to_xml : xml + self.response_body = xml.respond_to?(:to_xml) ? xml.to_xml(options) : xml end add :update do |proc, options| diff --git a/actionpack/test/controller/render_json_test.rb b/actionpack/test/controller/render_json_test.rb index 2580ada88b..5958b18d80 100644 --- a/actionpack/test/controller/render_json_test.rb +++ b/actionpack/test/controller/render_json_test.rb @@ -3,6 +3,14 @@ require 'controller/fake_models' require 'pathname' class RenderJsonTest < ActionController::TestCase + class JsonRenderable + def as_json(options={}) + hash = { :a => :b, :c => :d, :e => :f } + hash.except!(*options[:except]) if options[:except] + hash + end + end + class TestController < ActionController::Base protect_from_forgery @@ -37,6 +45,10 @@ class RenderJsonTest < ActionController::TestCase def render_json_with_render_to_string render :json => {:hello => render_to_string(:partial => 'partial')} end + + def render_json_with_extra_options + render :json => JsonRenderable.new, :except => [:c, :e] + end end tests TestController @@ -91,4 +103,10 @@ class RenderJsonTest < ActionController::TestCase assert_equal '{"hello":"partial html"}', @response.body assert_equal 'application/json', @response.content_type end + + def test_render_json_forwards_extra_options + get :render_json_with_extra_options + assert_equal '{"a":"b"}', @response.body + assert_equal 'application/json', @response.content_type + end end diff --git a/actionpack/test/controller/render_xml_test.rb b/actionpack/test/controller/render_xml_test.rb index 4da6c954cf..518842c34c 100644 --- a/actionpack/test/controller/render_xml_test.rb +++ b/actionpack/test/controller/render_xml_test.rb @@ -3,6 +3,13 @@ require 'controller/fake_models' require 'pathname' class RenderXmlTest < ActionController::TestCase + class XmlRenderable + def to_xml(options) + options[:root] ||= "i-am-xml" + "<#{options[:root]}/>" + end + end + class TestController < ActionController::Base protect_from_forgery @@ -20,13 +27,7 @@ class RenderXmlTest < ActionController::TestCase end def render_with_to_xml - to_xmlable = Class.new do - def to_xml - "" - end - end.new - - render :xml => to_xmlable + render :xml => XmlRenderable.new end def formatted_xml_erb @@ -35,6 +36,10 @@ class RenderXmlTest < ActionController::TestCase def render_xml_with_custom_content_type render :xml => "", :content_type => "application/atomsvc+xml" end + + def render_xml_with_custom_options + render :xml => XmlRenderable.new, :root => "i-am-THE-xml" + end end tests TestController @@ -58,6 +63,11 @@ class RenderXmlTest < ActionController::TestCase assert_equal "", @response.body end + def test_rendering_xml_should_call_to_xml_with_extra_options + get :render_with_to_xml + assert_equal "", @response.body + end + def test_rendering_with_object_location_should_set_header_with_url_for with_routing do |set| set.draw do |map| -- cgit v1.2.3 From 9476daa829f7dd0681121e042dd1356af0d50ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 22 Apr 2010 12:29:08 +0200 Subject: Speed up xml serializer by computing values just once and remove unecessary code duplication. --- activemodel/lib/active_model/serializers/xml.rb | 35 +++++++++++----------- .../active_record/serializers/xml_serializer.rb | 11 ------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb index c226359ea7..ee3e0eab06 100644 --- a/activemodel/lib/active_model/serializers/xml.rb +++ b/activemodel/lib/active_model/serializers/xml.rb @@ -1,6 +1,7 @@ require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/class/attribute_accessors' require 'active_support/core_ext/hash/conversions' +require 'active_support/core_ext/hash/slice' module ActiveModel module Serializers @@ -12,8 +13,10 @@ module ActiveModel class Attribute #:nodoc: attr_reader :name, :value, :type - def initialize(name, serializable) + def initialize(name, serializable, raw_value=nil) @name, @serializable = name, serializable + @raw_value = raw_value || @serializable.send(name) + @type = compute_type @value = compute_value end @@ -51,20 +54,17 @@ module ActiveModel protected def compute_type - value = @serializable.send(name) - type = Hash::XML_TYPE_NAMES[value.class.name] - type ||= :string if value.respond_to?(:to_str) + type = Hash::XML_TYPE_NAMES[@raw_value.class.name] + type ||= :string if @raw_value.respond_to?(:to_str) type ||= :yaml type end def compute_value - value = @serializable.send(name) - if formatter = Hash::XML_FORMATTING[type.to_s] - value ? formatter.call(value) : nil + @raw_value ? formatter.call(@raw_value) : nil else - value + @raw_value end end end @@ -72,7 +72,7 @@ module ActiveModel class MethodAttribute < Attribute #:nodoc: protected def compute_type - Hash::XML_TYPE_NAMES[@serializable.send(name).class.name] || :string + Hash::XML_TYPE_NAMES[@raw_value.class.name] || :string end end @@ -92,25 +92,24 @@ module ActiveModel # then because :except is set to a default value, the second # level model can have both :except and :only set. So if # :only is set, always delete :except. - def serializable_attribute_names - attribute_names = @serializable.attributes.keys.sort - + def serializable_attributes_hash + attributes = @serializable.attributes if options[:only].any? - attribute_names &= options[:only] + attributes.slice(*options[:only]) elsif options[:except].any? - attribute_names -= options[:except] + attributes.except(*options[:except]) + else + attributes end - - attribute_names end def serializable_attributes - serializable_attribute_names.collect { |name| Attribute.new(name, @serializable) } + serializable_attributes_hash.map { |name, value| self.class::Attribute.new(name, @serializable, value) } end def serializable_method_attributes Array.wrap(options[:methods]).inject([]) do |methods, name| - methods << MethodAttribute.new(name.to_s, @serializable) if @serializable.respond_to?(name.to_s) + methods << self.class::MethodAttribute.new(name.to_s, @serializable) if @serializable.respond_to?(name.to_s) methods end end diff --git a/activerecord/lib/active_record/serializers/xml_serializer.rb b/activerecord/lib/active_record/serializers/xml_serializer.rb index 2e85959b1e..255b03433d 100644 --- a/activerecord/lib/active_record/serializers/xml_serializer.rb +++ b/activerecord/lib/active_record/serializers/xml_serializer.rb @@ -182,17 +182,6 @@ module ActiveRecord #:nodoc: options[:except] |= Array.wrap(@serializable.class.inheritance_column) end - def serializable_attributes - serializable_attribute_names.collect { |name| Attribute.new(name, @serializable) } - end - - def serializable_method_attributes - Array.wrap(options[:methods]).inject([]) do |method_attributes, name| - method_attributes << MethodAttribute.new(name.to_s, @serializable) if @serializable.respond_to?(name.to_s) - method_attributes - end - end - def add_associations(association, records, opts) if records.is_a?(Enumerable) tag = reformat_name(association.to_s) -- cgit v1.2.3 From 275e839b8dfa3cf2bdedf1b31302dec20ac96a46 Mon Sep 17 00:00:00 2001 From: J Smith Date: Wed, 21 Apr 2010 21:37:51 -0400 Subject: Ensure that url_for uses symbolized keys in the controller. [#4391] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_dispatch/routing/url_for.rb | 2 +- actionpack/test/controller/url_for_test.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index fb236dce53..394d5a6248 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -128,7 +128,7 @@ module ActionDispatch when String options when nil, Hash - _router.url_for(url_options.merge(options || {})) + _router.url_for(url_options.merge(options || {}).symbolize_keys) else polymorphic_url(options) end diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index fc7773dffe..501f928e05 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -257,6 +257,16 @@ module AbstractController assert_equal second_class.default_url_options[:host], second_host end + def test_with_stringified_keys + assert_equal("/c/a", W.new.url_for('controller' => 'c', 'action' => 'a', 'only_path' => true)) + assert_equal("/c", W.new.url_for('controller' => 'c', 'only_path' => true)) + end + + def test_with_hash_with_indifferent_access + assert_equal("/c/a", W.new.url_for(HashWithIndifferentAccess.new('controller' => 'c', 'action' => 'a', 'only_path' => true))) + assert_equal("/c", W.new.url_for(HashWithIndifferentAccess.new('controller' => 'c', 'only_path' => true))) + end + private def extract_params(url) url.split('?', 2).last.split('&').sort -- cgit v1.2.3 From 2472f1026a7d3b3309937568204a5b46b60c2c3c Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 22 Apr 2010 13:02:55 -0300 Subject: HWIA delegates to to_hash symbolize_keys and stringify_keys and bang methods are not in the api Signed-off-by: Jeremy Kemper --- .../active_support/hash_with_indifferent_access.rb | 6 ++-- activesupport/test/core_ext/hash_ext_test.rb | 38 ++++++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 8241b69c8b..4de399a21a 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -111,8 +111,10 @@ module ActiveSupport super(convert_key(key)) end - def stringify_keys!; self end - def symbolize_keys!; self end + undef :stringify_keys! + def stringify_keys; to_hash.stringify_keys end + undef :symbolize_keys! + def symbolize_keys; to_hash.symbolize_keys end def to_options!; self end # Convert to a Hash with String keys. diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 86272a28c1..bac6d16ac5 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -60,6 +60,40 @@ class HashExtTest < Test::Unit::TestCase assert_equal @strings, @mixed.dup.stringify_keys! end + def test_symbolize_keys_for_hash_with_indifferent_access + assert_equal @symbols, @symbols.with_indifferent_access.symbolize_keys + assert_equal @symbols, @strings.with_indifferent_access.symbolize_keys + assert_equal @symbols, @mixed.with_indifferent_access.symbolize_keys + end + + def test_symbolize_keys_bang_for_hash_with_indifferent_access + assert_raise(NoMethodError) { @symbols.with_indifferent_access.dup.symbolize_keys! } + assert_raise(NoMethodError) { @strings.with_indifferent_access.dup.symbolize_keys! } + assert_raise(NoMethodError) { @mixed.with_indifferent_access.dup.symbolize_keys! } + end + + def test_symbolize_keys_preserves_keys_that_cant_be_symbolized_for_hash_with_indifferent_access + assert_equal @illegal_symbols, @illegal_symbols.with_indifferent_access.symbolize_keys + assert_raise(NoMethodError) { @illegal_symbols.with_indifferent_access.dup.symbolize_keys! } + end + + def test_symbolize_keys_preserves_fixnum_keys_for_hash_with_indifferent_access + assert_equal @fixnums, @fixnums.with_indifferent_access.symbolize_keys + assert_raise(NoMethodError) { @fixnums.with_indifferent_access.dup.symbolize_keys! } + end + + def test_stringify_keys_for_hash_with_indifferent_access + assert_equal @strings, @symbols.with_indifferent_access.stringify_keys + assert_equal @strings, @strings.with_indifferent_access.stringify_keys + assert_equal @strings, @mixed.with_indifferent_access.stringify_keys + end + + def test_stringify_keys_bang_for_hash_with_indifferent_access + assert_raise(NoMethodError) { @symbols.with_indifferent_access.dup.stringify_keys! } + assert_raise(NoMethodError) { @strings.with_indifferent_access.dup.stringify_keys! } + assert_raise(NoMethodError) { @mixed.with_indifferent_access.dup.stringify_keys! } + end + def test_indifferent_assorted @strings = @strings.with_indifferent_access @symbols = @symbols.with_indifferent_access @@ -213,11 +247,11 @@ class HashExtTest < Test::Unit::TestCase def test_stringify_and_symbolize_keys_on_indifferent_preserves_hash h = HashWithIndifferentAccess.new h[:first] = 1 - h.stringify_keys! + h = h.stringify_keys assert_equal 1, h['first'] h = HashWithIndifferentAccess.new h['first'] = 1 - h.symbolize_keys! + h = h.symbolize_keys assert_equal 1, h[:first] end -- cgit v1.2.3 From 920df0a4758d003dc284c13e439e0b9f60a6e6f0 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 22 Apr 2010 12:33:54 -0300 Subject: Make ActionDispatch url_for use HWIA symbolize_keys Signed-off-by: Jeremy Kemper --- actionpack/lib/action_dispatch/routing/url_for.rb | 2 +- actionpack/test/controller/url_for_test.rb | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 394d5a6248..db17615d92 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -128,7 +128,7 @@ module ActionDispatch when String options when nil, Hash - _router.url_for(url_options.merge(options || {}).symbolize_keys) + _router.url_for(url_options.merge((options || {}).symbolize_keys)) else polymorphic_url(options) end diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index 501f928e05..907acf9573 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -258,13 +258,17 @@ module AbstractController end def test_with_stringified_keys - assert_equal("/c/a", W.new.url_for('controller' => 'c', 'action' => 'a', 'only_path' => true)) assert_equal("/c", W.new.url_for('controller' => 'c', 'only_path' => true)) + assert_equal("/c/a", W.new.url_for('controller' => 'c', 'action' => 'a', 'only_path' => true)) end def test_with_hash_with_indifferent_access - assert_equal("/c/a", W.new.url_for(HashWithIndifferentAccess.new('controller' => 'c', 'action' => 'a', 'only_path' => true))) + W.default_url_options[:controller] = 'd' + W.default_url_options[:only_path] = false assert_equal("/c", W.new.url_for(HashWithIndifferentAccess.new('controller' => 'c', 'only_path' => true))) + + W.default_url_options[:action] = 'b' + assert_equal("/c/a", W.new.url_for(HashWithIndifferentAccess.new('controller' => 'c', 'action' => 'a', 'only_path' => true))) end private @@ -273,4 +277,4 @@ module AbstractController end end end -end \ No newline at end of file +end -- cgit v1.2.3 From ebf9820f7eb83476d200c71915dd0e8a056bcbab Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 22 Apr 2010 12:35:44 -0300 Subject: HWIA symbolize_keys now returns a hash so no need to do this anymore Signed-off-by: Jeremy Kemper --- actionpack/lib/action_view/helpers/url_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 13a54b0c21..4ffc5ea127 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -102,7 +102,7 @@ module ActionView escape = true options when Hash - options = { :only_path => options[:host].nil? }.update(options.to_hash.symbolize_keys) + options = { :only_path => options[:host].nil? }.update(options.symbolize_keys) escape = options.key?(:escape) ? options.delete(:escape) : false super when :back -- cgit v1.2.3 From d692e6be3091dd114afa0cce2778787d3af93e83 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 22 Apr 2010 09:47:40 -0700 Subject: Restore HWIA#stringify_keys! and update changelog --- activerecord/lib/active_record/base.rb | 3 +-- activesupport/CHANGELOG | 2 ++ activesupport/lib/active_support/hash_with_indifferent_access.rb | 4 ++-- activesupport/test/core_ext/hash_ext_test.rb | 6 +++--- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index fd24dcddc3..2d7cfad80d 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1844,8 +1844,7 @@ module ActiveRecord #:nodoc: # user.is_admin? # => true def attributes=(new_attributes, guard_protected_attributes = true) return if new_attributes.nil? - attributes = new_attributes.dup - attributes.stringify_keys! + attributes = new_attributes.stringify_keys multi_parameter_attributes = [] attributes = remove_attributes_protected_from_mass_assignment(attributes) if guard_protected_attributes diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index da370303c6..5ada5a1e9b 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.0.0 [beta 3] (April 13th, 2010)* +* HashWithIndifferentAccess: remove inherited symbolize_keys! since its keys are always strings. [Santiago Pastorino] + * Improve transliteration quality. #4374 [Norman Clarke] * Speed up and add Ruby 1.9 support for ActiveSupport::Multibyte::Chars#tidy_bytes. #4350 [Norman Clarke] diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 4de399a21a..21407327cd 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -111,8 +111,8 @@ module ActiveSupport super(convert_key(key)) end - undef :stringify_keys! - def stringify_keys; to_hash.stringify_keys end + def stringify_keys!; self end + def stringify_keys; to_hash end undef :symbolize_keys! def symbolize_keys; to_hash.symbolize_keys end def to_options!; self end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index bac6d16ac5..1dcde4b665 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -89,9 +89,9 @@ class HashExtTest < Test::Unit::TestCase end def test_stringify_keys_bang_for_hash_with_indifferent_access - assert_raise(NoMethodError) { @symbols.with_indifferent_access.dup.stringify_keys! } - assert_raise(NoMethodError) { @strings.with_indifferent_access.dup.stringify_keys! } - assert_raise(NoMethodError) { @mixed.with_indifferent_access.dup.stringify_keys! } + assert_equal @strings, @symbols.with_indifferent_access.dup.stringify_keys! + assert_equal @strings, @strings.with_indifferent_access.dup.stringify_keys! + assert_equal @strings, @mixed.with_indifferent_access.dup.stringify_keys! end def test_indifferent_assorted -- cgit v1.2.3 From c976784381d694df8090ad8a16c2648e93a2bd12 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 22 Apr 2010 10:07:33 -0700 Subject: Change HWIA#stringify_keys to return a HWIA not a Hash --- activesupport/lib/active_support/hash_with_indifferent_access.rb | 2 +- activesupport/test/core_ext/hash_ext_test.rb | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 21407327cd..6b7a897cf2 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -112,7 +112,7 @@ module ActiveSupport end def stringify_keys!; self end - def stringify_keys; to_hash end + def stringify_keys; dup end undef :symbolize_keys! def symbolize_keys; to_hash.symbolize_keys end def to_options!; self end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 1dcde4b665..b2a9731578 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -61,6 +61,7 @@ class HashExtTest < Test::Unit::TestCase end def test_symbolize_keys_for_hash_with_indifferent_access + assert_instance_of Hash, @symbols.with_indifferent_access.symbolize_keys assert_equal @symbols, @symbols.with_indifferent_access.symbolize_keys assert_equal @symbols, @strings.with_indifferent_access.symbolize_keys assert_equal @symbols, @mixed.with_indifferent_access.symbolize_keys @@ -83,12 +84,14 @@ class HashExtTest < Test::Unit::TestCase end def test_stringify_keys_for_hash_with_indifferent_access + assert_instance_of ActiveSupport::HashWithIndifferentAccess, @symbols.with_indifferent_access.stringify_keys assert_equal @strings, @symbols.with_indifferent_access.stringify_keys assert_equal @strings, @strings.with_indifferent_access.stringify_keys assert_equal @strings, @mixed.with_indifferent_access.stringify_keys end def test_stringify_keys_bang_for_hash_with_indifferent_access + assert_instance_of ActiveSupport::HashWithIndifferentAccess, @symbols.with_indifferent_access.dup.stringify_keys! assert_equal @strings, @symbols.with_indifferent_access.dup.stringify_keys! assert_equal @strings, @strings.with_indifferent_access.dup.stringify_keys! assert_equal @strings, @mixed.with_indifferent_access.dup.stringify_keys! -- cgit v1.2.3 From 19cecc907f2c97458519f103cbb967cf8dda5716 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 22 Apr 2010 14:50:46 -0300 Subject: HWIA relies on Hash#symbolize_keys and #stringify_keys extensions. Signed-off-by: Jeremy Kemper --- activesupport/lib/active_support/hash_with_indifferent_access.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 6b7a897cf2..f64f0f44cc 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/hash/keys' + # This class has dubious semantics and we only have it so that # people can write params[:key] instead of params['key'] # and they get the same value for both keys. -- cgit v1.2.3 From aaaa1782b44c620cecb97238534a2bd2be5d365a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 24 Apr 2010 11:48:47 +0200 Subject: =?UTF-8?q?Fix=20render=20:xml=20test=20(ht=20Simo=20Niemel=C3=A4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- actionpack/test/controller/render_xml_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionpack/test/controller/render_xml_test.rb b/actionpack/test/controller/render_xml_test.rb index 518842c34c..4bf867fa41 100644 --- a/actionpack/test/controller/render_xml_test.rb +++ b/actionpack/test/controller/render_xml_test.rb @@ -64,8 +64,8 @@ class RenderXmlTest < ActionController::TestCase end def test_rendering_xml_should_call_to_xml_with_extra_options - get :render_with_to_xml - assert_equal "", @response.body + get :render_xml_with_custom_options + assert_equal "", @response.body end def test_rendering_with_object_location_should_set_header_with_url_for -- cgit v1.2.3 From 577034decbe790789eaadef493582d164d9431c2 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 24 Apr 2010 10:26:09 -0700 Subject: Ensure require and load are private - h/t apeiros --- tools/profile | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/profile b/tools/profile index 0ccef6c26c..f02f1b5057 100755 --- a/tools/profile +++ b/tools/profile @@ -13,6 +13,7 @@ Gem.source_index require 'benchmark' module RequireProfiler + private def require(file, *args) RequireProfiler.profile(file) { super } end def load(file, *args) RequireProfiler.profile(file) { super } end -- cgit v1.2.3 From 70625badcf3c3f1be8d14181040099d6ca5f7a2b Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 24 Apr 2010 12:27:56 -0700 Subject: Drop support for postgres driver. Use pg >= 0.9.0. --- activerecord/CHANGELOG | 2 ++ .../connection_adapters/postgresql_adapter.rb | 18 ++---------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index c0c4df5035..f7c6d1dc77 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.0.0 [beta 4/release candidate] (unreleased)* +* PostgreSQL: drop support for old postgres driver. Use pg 0.9.0 or later. [Jeremy Kemper] + * Observers can prevent records from saving by returning false, just like before_save and friends. #4087 [Mislav Marohnić] diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index ceb1adc9e0..de89318eff 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -2,26 +2,12 @@ require 'active_record/connection_adapters/abstract_adapter' require 'active_support/core_ext/kernel/requires' require 'active_support/core_ext/object/blank' -begin - require_library_or_gem 'pg' -rescue LoadError => e - begin - require_library_or_gem 'postgres' - class PGresult - alias_method :nfields, :num_fields unless self.method_defined?(:nfields) - alias_method :ntuples, :num_tuples unless self.method_defined?(:ntuples) - alias_method :ftype, :type unless self.method_defined?(:ftype) - alias_method :cmd_tuples, :cmdtuples unless self.method_defined?(:cmd_tuples) - end - rescue LoadError - raise e - end -end - module ActiveRecord class Base # Establishes a connection to the database that's used by all Active Record objects def self.postgresql_connection(config) # :nodoc: + require 'pg' + config = config.symbolize_keys host = config[:host] port = config[:port] || 5432 -- cgit v1.2.3 From 2538ef0d09954340701354e8fcf57e76c4210416 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 24 Apr 2010 13:12:07 -0700 Subject: Use Array.wrap to quiet 1.8.8 deprecation --- activerecord/lib/active_record/fixtures.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 22f0e60083..cd3188675b 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -3,8 +3,9 @@ require 'yaml' require 'csv' require 'zlib' require 'active_support/dependencies' -require 'active_support/core_ext/logger' +require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/logger' if RUBY_VERSION < '1.9' module YAML #:nodoc: @@ -868,8 +869,8 @@ module ActiveRecord end def setup_fixture_accessors(table_names = nil) - table_names = [table_names] if table_names && !table_names.respond_to?(:each) - (table_names || fixture_table_names).each do |table_name| + table_names = Array.wrap(table_names || fixture_table_names) + table_names.each do |table_name| table_name = table_name.to_s.tr('.', '_') define_method(table_name) do |*fixtures| -- cgit v1.2.3 From 72a3e4b77b0e6e249a534d41ae48acdedd2098cc Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 24 Apr 2010 14:57:49 -0700 Subject: Rename fieldWithErrors style to field_with_errors. Remove unused alert style. --- actionpack/CHANGELOG | 2 ++ actionpack/lib/action_view/helpers/active_model_helper.rb | 2 +- actionpack/test/template/active_model_helper_test.rb | 8 ++++---- .../lib/rails/generators/erb/scaffold/templates/show.html.erb | 2 +- .../lib/rails/generators/rails/stylesheets/templates/scaffold.css | 8 ++------ 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 064e06bf92..04e44be291 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.0.0 [beta 4/release candidate] (unreleased)* +* Renamed the field error CSS class from fieldWithErrors to field_with_errors for consistency. [Jeremy Kemper] + * Add support for shorthand routes like /projects/status(.:format) #4423 [Diego Carrion] * Changed translate helper so that it doesn’t mark every translation as safe HTML. Only keys with a "_html" suffix and keys named "html" are considered to be safe HTML. All other translations are left untouched. [Craig Davey] diff --git a/actionpack/lib/action_view/helpers/active_model_helper.rb b/actionpack/lib/action_view/helpers/active_model_helper.rb index a7650c0050..d9f09b5dc5 100644 --- a/actionpack/lib/action_view/helpers/active_model_helper.rb +++ b/actionpack/lib/action_view/helpers/active_model_helper.rb @@ -6,7 +6,7 @@ require 'active_support/core_ext/object/blank' module ActionView ActiveSupport.on_load(:action_view) do class ActionView::Base - @@field_error_proc = Proc.new{ |html_tag, instance| "
#{html_tag}
".html_safe } + @@field_error_proc = Proc.new{ |html_tag, instance| "
#{html_tag}
".html_safe } cattr_accessor :field_error_proc end end diff --git a/actionpack/test/template/active_model_helper_test.rb b/actionpack/test/template/active_model_helper_test.rb index 8deeb78eab..b1705072c2 100644 --- a/actionpack/test/template/active_model_helper_test.rb +++ b/actionpack/test/template/active_model_helper_test.rb @@ -27,14 +27,14 @@ class ActiveModelHelperTest < ActionView::TestCase def test_text_area_with_errors assert_dom_equal( - %(
), + %(
), text_area("post", "body") ) end def test_text_field_with_errors assert_dom_equal( - %(
), + %(
), text_field("post", "author_name") ) end @@ -42,11 +42,11 @@ class ActiveModelHelperTest < ActionView::TestCase def test_field_error_proc old_proc = ActionView::Base.field_error_proc ActionView::Base.field_error_proc = Proc.new do |html_tag, instance| - %(
#{html_tag} #{[instance.error_message].join(', ')}
).html_safe + %(
#{html_tag} #{[instance.error_message].join(', ')}
).html_safe end assert_dom_equal( - %(
can't be empty
), + %(
can't be empty
), text_field("post", "author_name") ) ensure diff --git a/railties/lib/rails/generators/erb/scaffold/templates/show.html.erb b/railties/lib/rails/generators/erb/scaffold/templates/show.html.erb index 4dd2e6bf8c..6b3518717a 100644 --- a/railties/lib/rails/generators/erb/scaffold/templates/show.html.erb +++ b/railties/lib/rails/generators/erb/scaffold/templates/show.html.erb @@ -1,4 +1,4 @@ -

<%%= notice %>

+

<%%= notice %>

<% for attribute in attributes -%>

diff --git a/railties/lib/rails/generators/rails/stylesheets/templates/scaffold.css b/railties/lib/rails/generators/rails/stylesheets/templates/scaffold.css index 9f2056a702..1ae7000299 100644 --- a/railties/lib/rails/generators/rails/stylesheets/templates/scaffold.css +++ b/railties/lib/rails/generators/rails/stylesheets/templates/scaffold.css @@ -20,15 +20,11 @@ div.field, div.actions { margin-bottom: 10px; } -.notice { +#notice { color: green; } -.alert { - color: red; -} - -.fieldWithErrors { +.field_with_errors { padding: 2px; background-color: red; display: table; -- cgit v1.2.3 From dac80f779d357b2df6fabdda33dac56c69c2a6f9 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 24 Apr 2010 15:27:18 -0700 Subject: PostgreSQL: use standard-conforming strings if possible --- .../connection_adapters/postgresql_adapter.rb | 44 +++++++--------------- 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index de89318eff..3d6b37268f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -263,20 +263,12 @@ module ActiveRecord true end - # Does PostgreSQL support standard conforming strings? - def supports_standard_conforming_strings? - # Temporarily set the client message level above error to prevent unintentional - # error messages in the logs when working on a PostgreSQL database server that - # does not support standard conforming strings. - client_min_messages_old = client_min_messages - self.client_min_messages = 'panic' - - # postgres-pr does not raise an exception when client_min_messages is set higher - # than error and "SHOW standard_conforming_strings" fails, but returns an empty - # PGresult instead. - has_support = query('SHOW standard_conforming_strings')[0][0] rescue false - self.client_min_messages = client_min_messages_old - has_support + # Enable standard-conforming strings if available. + def set_standard_conforming_strings + old, self.client_min_messages = client_min_messages, 'panic' + execute('SET standard_conforming_strings = on') rescue nil + ensure + self.client_min_messages = old end def supports_insert_with_returning? @@ -376,9 +368,9 @@ module ActiveRecord # Quotes PostgreSQL-specific data types for SQL input. def quote(value, column = nil) #:nodoc: if value.kind_of?(String) && column && column.type == :binary - "#{quoted_string_prefix}'#{escape_bytea(value)}'" + "'#{escape_bytea(value)}'" elsif value.kind_of?(String) && column && column.sql_type == 'xml' - "xml E'#{quote_string(value)}'" + "xml '#{quote_string(value)}'" elsif value.kind_of?(Numeric) && column && column.sql_type == 'money' # Not truly string input, so doesn't require (or allow) escape string syntax. "'#{value.to_s}'" @@ -991,22 +983,11 @@ module ActiveRecord # Ignore async_exec and async_query when using postgres-pr. @async = @config[:allow_concurrency] && @connection.respond_to?(:async_exec) - # Use escape string syntax if available. We cannot do this lazily when encountering - # the first string, because that could then break any transactions in progress. - # See: http://www.postgresql.org/docs/current/static/runtime-config-compatible.html - # If PostgreSQL doesn't know the standard_conforming_strings parameter then it doesn't - # support escape string syntax. Don't override the inherited quoted_string_prefix. - if supports_standard_conforming_strings? - self.class.instance_eval do - define_method(:quoted_string_prefix) { 'E' } - end - end - # Money type has a fixed precision of 10 in PostgreSQL 8.2 and below, and as of # PostgreSQL 8.3 it has a fixed precision of 19. PostgreSQLColumn.extract_precision # should know about this but can't detect it there, so deal with it here. - PostgreSQLColumn.money_precision = - (postgresql_version >= 80300) ? 19 : 10 + PostgreSQLColumn.money_precision = (postgresql_version >= 80300) ? 19 : 10 + configure_connection end @@ -1022,7 +1003,10 @@ module ActiveRecord end self.client_min_messages = @config[:min_messages] if @config[:min_messages] self.schema_search_path = @config[:schema_search_path] || @config[:schema_order] - + + # Use standard-conforming strings if available so we don't have to do the E'...' dance. + set_standard_conforming_strings + # If using ActiveRecord's time zone support configure the connection to return # TIMESTAMP WITH ZONE types in UTC. execute("SET time zone 'UTC'") if ActiveRecord::Base.default_timezone == :utc -- cgit v1.2.3 From 426f93b7510630308a8a9594836b3c4a84aedcce Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 24 Apr 2010 15:27:21 -0700 Subject: PostgreSQL: always rely on pg driver for escape/unescape and quoting duties --- .../connection_adapters/postgresql_adapter.rb | 95 ++-------------------- 1 file changed, 7 insertions(+), 88 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 3d6b37268f..74fed4ad62 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -292,77 +292,15 @@ module ActiveRecord # QUOTING ================================================== # Escapes binary strings for bytea input to the database. - def escape_bytea(original_value) - if @connection.respond_to?(:escape_bytea) - self.class.instance_eval do - define_method(:escape_bytea) do |value| - @connection.escape_bytea(value) if value - end - end - elsif PGconn.respond_to?(:escape_bytea) - self.class.instance_eval do - define_method(:escape_bytea) do |value| - PGconn.escape_bytea(value) if value - end - end - else - self.class.instance_eval do - define_method(:escape_bytea) do |value| - if value - result = '' - value.each_byte { |c| result << sprintf('\\\\%03o', c) } - result - end - end - end - end - escape_bytea(original_value) + def escape_bytea(value) + @connection.escape_bytea(value) if value end # Unescapes bytea output from a database to the binary string it represents. # NOTE: This is NOT an inverse of escape_bytea! This is only to be used # on escaped binary output from database drive. - def unescape_bytea(original_value) - # In each case, check if the value actually is escaped PostgreSQL bytea output - # or an unescaped Active Record attribute that was just written. - if PGconn.respond_to?(:unescape_bytea) - self.class.instance_eval do - define_method(:unescape_bytea) do |value| - if value =~ /\\\d{3}/ - PGconn.unescape_bytea(value) - else - value - end - end - end - else - self.class.instance_eval do - define_method(:unescape_bytea) do |value| - if value =~ /\\\d{3}/ - result = '' - i, max = 0, value.size - while i < max - char = value[i] - if char == ?\\ - if value[i+1] == ?\\ - char = ?\\ - i += 1 - else - char = value[i+1..i+3].oct - i += 3 - end - end - result << char - i += 1 - end - result - else - value - end - end - end - end - unescape_bytea(original_value) + def unescape_bytea(value) + @connection.unescape_bytea(value) if value end # Quotes PostgreSQL-specific data types for SQL input. @@ -386,28 +324,9 @@ module ActiveRecord end end - # Quotes strings for use in SQL input in the postgres driver for better performance. - def quote_string(original_value) #:nodoc: - if @connection.respond_to?(:escape) - self.class.instance_eval do - define_method(:quote_string) do |s| - @connection.escape(s) - end - end - elsif PGconn.respond_to?(:escape) - self.class.instance_eval do - define_method(:quote_string) do |s| - PGconn.escape(s) - end - end - else - # There are some incorrectly compiled postgres drivers out there - # that don't define PGconn.escape. - self.class.instance_eval do - remove_method(:quote_string) - end - end - quote_string(original_value) + # Quotes strings for use in SQL input. + def quote_string(s) #:nodoc: + @connection.escape(s) end # Checks the following cases: -- cgit v1.2.3 From 5c9d23f870bc59a42a2c5994f4e6b11d3e4e5bdc Mon Sep 17 00:00:00 2001 From: Matthew Rudy Jacobs Date: Sat, 24 Apr 2010 19:15:14 -0300 Subject: Make assert_recognizes work in IntegrationTest [#4390 state:committed] Signed-off-by: Santiago Pastorino Signed-off-by: Jeremy Kemper --- railties/lib/rails/test_help.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index 3ce4e2c780..ec5e4a357c 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -35,7 +35,9 @@ class ActionController::TestCase end class ActionDispatch::IntegrationTest - include Rails.application.routes.url_helpers + setup do + @routes = Rails.application.routes + end end begin -- cgit v1.2.3 From ed0ca5db9ea247ceae67f22a750cbd49006f35ed Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sat, 24 Apr 2010 19:17:30 -0300 Subject: Add a test for assert_recognizes on ActionDispatch::IntegrationTest [#4390 state:committed] Signed-off-by: Jeremy Kemper --- actionpack/test/abstract_unit.rb | 8 +++++++- actionpack/test/dispatch/routing_test.rb | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index acf887ee4a..f3ff258016 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -143,6 +143,12 @@ class BasicController end end +class ActionDispatch::IntegrationTest < ActiveSupport::TestCase + setup do + @routes = SharedTestRoutes + end +end + class ActionController::IntegrationTest < ActiveSupport::TestCase def self.build_app(routes = nil) RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware| @@ -275,4 +281,4 @@ module ActionController class Base include SharedTestRoutes.url_helpers end -end \ No newline at end of file +end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 5bca476b27..f67e403330 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1000,6 +1000,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_assert_recognizes_account_overview + with_test_routes do + assert_recognizes({:controller => "account", :action => "overview"}, "/account/overview") + end + end + private def with_test_routes yield -- cgit v1.2.3 From bd3cc6bfffd02302414f1558be7d6105ac47e67a Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 24 Apr 2010 16:27:20 -0700 Subject: Remove quoted_string_prefix entirely since PostgreSQL was the only database adapter relying on it. --- .../lib/active_record/connection_adapters/abstract/quoting.rb | 10 +++------- activerecord/test/cases/finder_test.rb | 4 ++-- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index 8649f96498..d7b5bf8e31 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -13,12 +13,12 @@ module ActiveRecord when String, ActiveSupport::Multibyte::Chars value = value.to_s if column && column.type == :binary && column.class.respond_to?(:string_to_binary) - "#{quoted_string_prefix}'#{quote_string(column.class.string_to_binary(value))}'" # ' (for ruby-mode) + "'#{quote_string(column.class.string_to_binary(value))}'" # ' (for ruby-mode) elsif column && [:integer, :float].include?(column.type) value = column.type == :integer ? value.to_i : value.to_f value.to_s else - "#{quoted_string_prefix}'#{quote_string(value)}'" # ' (for ruby-mode) + "'#{quote_string(value)}'" # ' (for ruby-mode) end when NilClass then "NULL" when TrueClass then (column && column.type == :integer ? '1' : quoted_true) @@ -30,7 +30,7 @@ module ActiveRecord if value.acts_like?(:date) || value.acts_like?(:time) "'#{quoted_date(value)}'" else - "#{quoted_string_prefix}'#{quote_string(value.to_yaml)}'" + "'#{quote_string(value.to_yaml)}'" end end end @@ -67,10 +67,6 @@ module ActiveRecord value end.to_s(:db) end - - def quoted_string_prefix - '' - end end end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 9e88ec8016..77b2b748b1 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -566,8 +566,8 @@ class FinderTest < ActiveRecord::TestCase end def test_string_sanitation - assert_not_equal "#{ActiveRecord::Base.connection.quoted_string_prefix}'something ' 1=1'", ActiveRecord::Base.sanitize("something ' 1=1") - assert_equal "#{ActiveRecord::Base.connection.quoted_string_prefix}'something; select table'", ActiveRecord::Base.sanitize("something; select table") + assert_not_equal "'something ' 1=1'", ActiveRecord::Base.sanitize("something ' 1=1") + assert_equal "'something; select table'", ActiveRecord::Base.sanitize("something; select table") end def test_count -- cgit v1.2.3 From 403752e28993493d87ae4e69f9aca1858eb133f7 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 24 Apr 2010 18:35:12 -0700 Subject: Explicit source encoding --- actionpack/test/template/text_helper_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb index 9962b7af3f..8b6c107a21 100644 --- a/actionpack/test/template/text_helper_test.rb +++ b/actionpack/test/template/text_helper_test.rb @@ -1,3 +1,4 @@ +# encoding: us-ascii require 'abstract_unit' require 'testing_sandbox' begin -- cgit v1.2.3 From df886c4c89f3b8a95ef2456d6893393376b330a9 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 24 Apr 2010 18:52:51 -0700 Subject: Missed commit: explicit source encoding --- actionpack/test/controller/assert_select_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/test/controller/assert_select_test.rb b/actionpack/test/controller/assert_select_test.rb index cb3e848dfa..4ef6fa4000 100644 --- a/actionpack/test/controller/assert_select_test.rb +++ b/actionpack/test/controller/assert_select_test.rb @@ -1,3 +1,4 @@ +# encoding: utf-8 #-- # Copyright (c) 2006 Assaf Arkin (http://labnotes.org) # Under MIT and/or CC By license. @@ -347,7 +348,6 @@ class AssertSelectTest < ActionController::TestCase assert_select str, :text => "\343\203\201\343\202\261\343\203\203\343\203\210" assert_select str, "\343\203\201\343\202\261\343\203\203\343\203\210" if str.respond_to?(:force_encoding) - str.force_encoding(Encoding::UTF_8) assert_select str, /\343\203\201..\343\203\210/u assert_raise(Assertion) { assert_select str, /\343\203\201.\343\203\210/u } else -- cgit v1.2.3 From 864bd9c21fdc8ce265c48fdfa2d4d7917032e717 Mon Sep 17 00:00:00 2001 From: David Chelimsky Date: Sat, 24 Apr 2010 21:12:02 -0500 Subject: allow unsubscribe by name or subscription [#4433 state:resolved] Signed-off-by: Jeremy Kemper --- actionpack/lib/action_controller/test_case.rb | 1 + .../lib/active_support/notifications/fanout.rb | 20 +++++++++++++++----- activesupport/test/notifications_test.rb | 22 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index b3758218a2..27939e3434 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -36,6 +36,7 @@ module ActionController end def teardown_subscriptions + ActiveSupport::Notifications.unsubscribe("action_view.render_template") ActiveSupport::Notifications.unsubscribe("action_view.render_template!") end diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index a3ddc7705a..300ec842a9 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -19,8 +19,8 @@ module ActiveSupport end def unsubscribe(subscriber) - @subscribers.delete(subscriber) @listeners_for.clear + @subscribers.reject! {|s| s.matches?(subscriber)} end def publish(name, *args) @@ -60,7 +60,7 @@ module ActiveSupport end def publish(*args) - return unless matches?(args.first) + return unless subscribed_to?(args.first) push(*args) true end @@ -69,10 +69,20 @@ module ActiveSupport true end - private - def matches?(name) - !@pattern || @pattern =~ name.to_s + def subscribed_to?(name) + !@pattern || @pattern =~ name.to_s + end + + def matches?(subscriber_or_name) + case subscriber_or_name + when String + @pattern && @pattern =~ subscriber_or_name + when self + true end + end + + private def push(*args) @block.call(*args) diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index 92fbe5b92f..c2e1c588f0 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -6,7 +6,9 @@ module Notifications ActiveSupport::Notifications.notifier = nil @notifier = ActiveSupport::Notifications.notifier @events = [] + @named_events = [] @subscription = @notifier.subscribe { |*args| @events << event(*args) } + @named_subscription = @notifier.subscribe("named.subscription") { |*args| @named_events << event(*args) } end private @@ -30,6 +32,26 @@ module Notifications assert_equal [[:foo]], @events end + def test_unsubscribing_by_name_removes_a_subscription + @notifier.publish "named.subscription", :foo + @notifier.wait + assert_equal [["named.subscription", :foo]], @named_events + @notifier.unsubscribe("named.subscription") + @notifier.publish "named.subscription", :foo + @notifier.wait + assert_equal [["named.subscription", :foo]], @named_events + end + + def test_unsubscribing_by_name_leaves_the_other_subscriptions + @notifier.publish "named.subscription", :foo + @notifier.wait + assert_equal [["named.subscription", :foo]], @events + @notifier.unsubscribe("named.subscription") + @notifier.publish "named.subscription", :foo + @notifier.wait + assert_equal [["named.subscription", :foo], ["named.subscription", :foo]], @events + end + private def event(*args) args -- cgit v1.2.3 From 77c099c231f2efb36a2a77a32138ed5c6761ec19 Mon Sep 17 00:00:00 2001 From: reu Date: Sat, 24 Apr 2010 14:17:14 -0300 Subject: Fix validates_numericaly_of only integer error message [#4406 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activemodel/lib/active_model/locale/en.yml | 1 + .../lib/active_model/validations/numericality.rb | 29 ++++++++++++++-------- .../test/cases/validations/i18n_validation_test.rb | 18 +++++++------- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/activemodel/lib/active_model/locale/en.yml b/activemodel/lib/active_model/locale/en.yml index ea58021767..d05c04967c 100644 --- a/activemodel/lib/active_model/locale/en.yml +++ b/activemodel/lib/active_model/locale/en.yml @@ -17,6 +17,7 @@ en: too_short: "is too short (minimum is {{count}} characters)" wrong_length: "is the wrong length (should be {{count}} characters)" not_a_number: "is not a number" + not_an_integer: "must be an integer" greater_than: "must be greater than {{count}}" greater_than_or_equal_to: "must be greater than or equal to {{count}}" equal_to: "must be equal to {{count}}" diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index c6d84c5312..f974999bef 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -25,11 +25,18 @@ module ActiveModel return if options[:allow_nil] && raw_value.nil? - unless value = parse_raw_value(raw_value, options) + unless value = parse_raw_value_as_a_number(raw_value) record.errors.add(attr_name, :not_a_number, :value => raw_value, :default => options[:message]) return end + if options[:only_integer] + unless value = parse_raw_value_as_an_integer(raw_value) + record.errors.add(attr_name, :not_an_integer, :value => raw_value, :default => options[:message]) + return + end + end + options.slice(*CHECKS.keys).each do |option, option_value| case option when :odd, :even @@ -44,23 +51,23 @@ module ActiveModel record.errors.add(attr_name, option, :default => options[:message], :value => value, :count => option_value) end end - end + end end protected - def parse_raw_value(raw_value, options) - if options[:only_integer] - raw_value.to_i if raw_value.to_s =~ /\A[+-]?\d+\Z/ - else - begin - Kernel.Float(raw_value) - rescue ArgumentError, TypeError - nil - end + def parse_raw_value_as_a_number(raw_value) + begin + Kernel.Float(raw_value) + rescue ArgumentError, TypeError + nil end end + def parse_raw_value_as_an_integer(raw_value) + raw_value.to_i if raw_value.to_s =~ /\A[+-]?\d+\Z/ + end + end module ClassMethods diff --git a/activemodel/test/cases/validations/i18n_validation_test.rb b/activemodel/test/cases/validations/i18n_validation_test.rb index 7dd82e711d..1b4c1699ae 100644 --- a/activemodel/test/cases/validations/i18n_validation_test.rb +++ b/activemodel/test/cases/validations/i18n_validation_test.rb @@ -217,15 +217,15 @@ class I18nValidationTest < ActiveModel::TestCase def test_validates_numericality_of_only_integer_generates_message Person.validates_numericality_of :title, :only_integer => true - @person.title = 'a' - @person.errors.expects(:generate_message).with(:title, :not_a_number, {:value => 'a', :default => nil}) + @person.title = '0.0' + @person.errors.expects(:generate_message).with(:title, :not_an_integer, {:value => '0.0', :default => nil}) @person.valid? end def test_validates_numericality_of_only_integer_generates_message_with_custom_default_message Person.validates_numericality_of :title, :only_integer => true, :message => 'custom' - @person.title = 'a' - @person.errors.expects(:generate_message).with(:title, :not_a_number, {:value => 'a', :default => 'custom'}) + @person.title = '0.0' + @person.errors.expects(:generate_message).with(:title, :not_an_integer, {:value => '0.0', :default => 'custom'}) @person.valid? end @@ -441,20 +441,20 @@ class I18nValidationTest < ActiveModel::TestCase # validates_numericality_of with :only_integer w/o mocha def test_validates_numericality_of_only_integer_finds_custom_model_key_translation - I18n.backend.store_translations 'en', :activemodel => {:errors => {:models => {:person => {:attributes => {:title => {:not_a_number => 'custom message'}}}}}} - I18n.backend.store_translations 'en', :errors => {:messages => {:not_a_number => 'global message'}} + I18n.backend.store_translations 'en', :activemodel => {:errors => {:models => {:person => {:attributes => {:title => {:not_an_integer => 'custom message'}}}}}} + I18n.backend.store_translations 'en', :errors => {:messages => {:not_an_integer => 'global message'}} Person.validates_numericality_of :title, :only_integer => true - @person.title = 'a' + @person.title = '1.0' @person.valid? assert_equal ['custom message'], @person.errors[:title] end def test_validates_numericality_of_only_integer_finds_global_default_translation - I18n.backend.store_translations 'en', :errors => {:messages => {:not_a_number => 'global message'}} + I18n.backend.store_translations 'en', :errors => {:messages => {:not_an_integer => 'global message'}} Person.validates_numericality_of :title, :only_integer => true - @person.title = 'a' + @person.title = '1.0' @person.valid? assert_equal ['global message'], @person.errors[:title] end -- cgit v1.2.3 From 8ec085bf1804770a547894967fcdee24087fda87 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Wed, 14 Apr 2010 18:15:27 +0100 Subject: Support fixtures for namespaced models [#2965 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/lib/active_record/fixtures.rb | 9 +++++---- activerecord/lib/active_record/railties/databases.rake | 4 ++-- activerecord/test/cases/fixtures_test.rb | 10 +++++++++- activerecord/test/fixtures/admin/accounts.yml | 2 ++ activerecord/test/fixtures/admin/users.yml | 7 +++++++ activerecord/test/models/admin.rb | 5 +++++ activerecord/test/models/admin/account.rb | 3 +++ activerecord/test/models/admin/user.rb | 3 +++ activerecord/test/schema/schema.rb | 9 +++++++++ 9 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 activerecord/test/fixtures/admin/accounts.yml create mode 100644 activerecord/test/fixtures/admin/users.yml create mode 100644 activerecord/test/models/admin.rb create mode 100644 activerecord/test/models/admin/account.rb create mode 100644 activerecord/test/models/admin/user.rb diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index cd3188675b..0bc49c1daa 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -493,6 +493,7 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) def self.create_fixtures(fixtures_directory, table_names, class_names = {}) table_names = [table_names].flatten.map { |n| n.to_s } + table_names.each { |n| class_names[n.tr('/', '_').to_sym] = n.classify if n.include?('/') } connection = block_given? ? yield : ActiveRecord::Base.connection table_names_to_fetch = table_names.reject { |table_name| fixture_is_cached?(connection, table_name) } @@ -503,7 +504,7 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) fixtures_map = {} fixtures = table_names_to_fetch.map do |table_name| - fixtures_map[table_name] = Fixtures.new(connection, File.split(table_name.to_s).last, class_names[table_name.to_sym], File.join(fixtures_directory, table_name.to_s)) + fixtures_map[table_name] = Fixtures.new(connection, table_name.tr('/', '_'), class_names[table_name.tr('/', '_').to_sym], File.join(fixtures_directory, table_name)) end all_loaded_fixtures.update(fixtures_map) @@ -837,8 +838,8 @@ module ActiveRecord def fixtures(*table_names) if table_names.first == :all - table_names = Dir["#{fixture_path}/*.yml"] + Dir["#{fixture_path}/*.csv"] - table_names.map! { |f| File.basename(f).split('.')[0..-2].join('.') } + table_names = Dir["#{fixture_path}/**/*.{yml,csv}"] + table_names.map! { |f| f[(fixture_path.size + 1)..-5] } else table_names = table_names.flatten.map { |n| n.to_s } end @@ -871,7 +872,7 @@ module ActiveRecord def setup_fixture_accessors(table_names = nil) table_names = Array.wrap(table_names || fixture_table_names) table_names.each do |table_name| - table_name = table_name.to_s.tr('.', '_') + table_name = table_name.to_s.tr('./', '_') define_method(table_name) do |*fixtures| force_reload = fixtures.pop if fixtures.last == true || fixtures.last == :reload diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 2b53afc68b..cb7eade0ab 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -258,8 +258,8 @@ namespace :db do base_dir = ENV['FIXTURES_PATH'] ? File.join(Rails.root, ENV['FIXTURES_PATH']) : File.join(Rails.root, 'test', 'fixtures') fixtures_dir = ENV['FIXTURES_DIR'] ? File.join(base_dir, ENV['FIXTURES_DIR']) : base_dir - (ENV['FIXTURES'] ? ENV['FIXTURES'].split(/,/).map {|f| File.join(fixtures_dir, f) } : Dir.glob(File.join(fixtures_dir, '*.{yml,csv}'))).each do |fixture_file| - Fixtures.create_fixtures(File.dirname(fixture_file), File.basename(fixture_file, '.*')) + (ENV['FIXTURES'] ? ENV['FIXTURES'].split(/,/).map {|f| File.join(fixtures_dir, f) } : Dir["#{fixtures_dir}/**/*.{yml,csv}"]).each do |fixture_file| + Fixtures.create_fixtures(fixtures_dir, fixture_file[(fixtures_dir.size + 1)..-5]) end end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index d24283fe4e..3ce23209cc 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -16,6 +16,9 @@ require 'models/treasure' require 'models/matey' require 'models/ship' require 'models/book' +require 'models/admin' +require 'models/admin/account' +require 'models/admin/user' class FixturesTest < ActiveRecord::TestCase self.use_instantiated_fixtures = true @@ -507,7 +510,7 @@ class FasterFixturesTest < ActiveRecord::TestCase end class FoxyFixturesTest < ActiveRecord::TestCase - fixtures :parrots, :parrots_pirates, :pirates, :treasures, :mateys, :ships, :computers, :developers + fixtures :parrots, :parrots_pirates, :pirates, :treasures, :mateys, :ships, :computers, :developers, :"admin/accounts", :"admin/users" def test_identifies_strings assert_equal(Fixtures.identify("foo"), Fixtures.identify("foo")) @@ -629,6 +632,11 @@ class FoxyFixturesTest < ActiveRecord::TestCase assert_kind_of DeadParrot, parrots(:polly) assert_equal pirates(:blackbeard), parrots(:polly).killer end + + def test_namespaced_models + assert admin_accounts(:signals37).users.include?(admin_users(:david)) + assert_equal 2, admin_accounts(:signals37).users.size + end end class ActiveSupportSubclassWithFixturesTest < ActiveRecord::TestCase diff --git a/activerecord/test/fixtures/admin/accounts.yml b/activerecord/test/fixtures/admin/accounts.yml new file mode 100644 index 0000000000..9e341a15af --- /dev/null +++ b/activerecord/test/fixtures/admin/accounts.yml @@ -0,0 +1,2 @@ +signals37: + name: 37signals diff --git a/activerecord/test/fixtures/admin/users.yml b/activerecord/test/fixtures/admin/users.yml new file mode 100644 index 0000000000..6f11f2509e --- /dev/null +++ b/activerecord/test/fixtures/admin/users.yml @@ -0,0 +1,7 @@ +david: + name: David + account: signals37 + +jamis: + name: Jamis + account: signals37 diff --git a/activerecord/test/models/admin.rb b/activerecord/test/models/admin.rb new file mode 100644 index 0000000000..00e69fbed8 --- /dev/null +++ b/activerecord/test/models/admin.rb @@ -0,0 +1,5 @@ +module Admin + def self.table_name_prefix + 'admin_' + end +end \ No newline at end of file diff --git a/activerecord/test/models/admin/account.rb b/activerecord/test/models/admin/account.rb new file mode 100644 index 0000000000..46de28aae1 --- /dev/null +++ b/activerecord/test/models/admin/account.rb @@ -0,0 +1,3 @@ +class Admin::Account < ActiveRecord::Base + has_many :users +end \ No newline at end of file diff --git a/activerecord/test/models/admin/user.rb b/activerecord/test/models/admin/user.rb new file mode 100644 index 0000000000..74bb21551e --- /dev/null +++ b/activerecord/test/models/admin/user.rb @@ -0,0 +1,3 @@ +class Admin::User < ActiveRecord::Base + belongs_to :account +end \ No newline at end of file diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 7a0cf550e0..f5fba2f87d 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -26,6 +26,15 @@ ActiveRecord::Schema.define do t.integer :credit_limit end + create_table :admin_accounts, :force => true do |t| + t.string :name + end + + create_table :admin_users, :force => true do |t| + t.string :name + t.references :account + end + create_table :audit_logs, :force => true do |t| t.column :message, :string, :null=>false t.column :developer_id, :integer, :null=>false -- cgit v1.2.3 From 490a3335d56c124c8113aac0b63ad367f81bb450 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 25 Apr 2010 19:48:40 +0200 Subject: Action Pack: fix tests with -K*, work around Ruby 1.9.1 constant lookup. [#4473 state:committed] Signed-off-by: Jeremy Kemper --- actionpack/test/controller/send_file_test.rb | 4 ++-- actionpack/test/dispatch/routing_test.rb | 4 ++-- activesupport/lib/active_support/multibyte/chars.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 30c9a65b7c..36b8055810 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -25,7 +25,7 @@ class SendFileController < ActionController::Base end def multibyte_text_data - send_data("Кирилица\n祝您好運", options) + send_data("Кирилица\n祝您好運.", options) end end @@ -128,7 +128,7 @@ class SendFileTest < ActionController::TestCase assert_equal 'image/png', @controller.content_type end - + def test_send_file_headers_with_bad_symbol options = { diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index f67e403330..b508996467 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -237,8 +237,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest AltRoutes = ActionDispatch::Routing::RouteSet.new(AltRequest) AltRoutes.draw do - get "/" => XHeader.new, :constraints => {:x_header => /HEADER/} - get "/" => AltApp.new + get "/" => TestRoutingMapper::TestAltApp::XHeader.new, :constraints => {:x_header => /HEADER/} + get "/" => TestRoutingMapper::TestAltApp::AltApp.new end def app diff --git a/activesupport/lib/active_support/multibyte/chars.rb b/activesupport/lib/active_support/multibyte/chars.rb index 38007fd4e7..4ade1158fd 100644 --- a/activesupport/lib/active_support/multibyte/chars.rb +++ b/activesupport/lib/active_support/multibyte/chars.rb @@ -72,8 +72,8 @@ module ActiveSupport #:nodoc: def self.codepoints_to_pattern(array_of_codepoints) #:nodoc: array_of_codepoints.collect{ |e| [e].pack 'U*' }.join('|') end - UNICODE_TRAILERS_PAT = /(#{codepoints_to_pattern(UNICODE_LEADERS_AND_TRAILERS)})+\Z/ - UNICODE_LEADERS_PAT = /\A(#{codepoints_to_pattern(UNICODE_LEADERS_AND_TRAILERS)})+/ + UNICODE_TRAILERS_PAT = /(#{codepoints_to_pattern(UNICODE_LEADERS_AND_TRAILERS)})+\Z/u + UNICODE_LEADERS_PAT = /\A(#{codepoints_to_pattern(UNICODE_LEADERS_AND_TRAILERS)})+/u UTF8_PAT = ActiveSupport::Multibyte::VALID_CHARACTER['UTF-8'] -- cgit v1.2.3 From 2a6e0f34ad6b48dcf41989e0d7555cda46492b34 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sun, 25 Apr 2010 21:02:35 -0700 Subject: Revert "create option to include_root_in_json for ActiveResource [#2584 state:committed]" This reverts commits 72f89b5d971b48a133c4c0af56fbeda35d738dae, 137d8e0b2fe9fcc4fdac6cbbd44ca010784e5972. Should reuse Active Model. [#2584 state:incomplete] --- activeresource/lib/active_resource/base.rb | 9 --------- activeresource/test/cases/base_test.rb | 29 ++++++----------------------- 2 files changed, 6 insertions(+), 32 deletions(-) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 1dd5af8098..bf775a14c8 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -251,9 +251,6 @@ module ActiveResource # The logger for diagnosing and tracing Active Resource calls. cattr_accessor :logger - # Controls the top-level behavior of JSON serialization - cattr_accessor :include_root_in_json, :instance_writer => false - class << self # Creates a schema for this resource - setting the attributes that are # known prior to fetching an instance from the remote system. @@ -1243,12 +1240,6 @@ module ActiveResource case self.class.format when ActiveResource::Formats::XmlFormat self.class.format.encode(attributes, {:root => self.class.element_name}.merge(options)) - when ActiveResource::Formats::JsonFormat - if ActiveResource::Base.include_root_in_json - self.class.format.encode({self.class.element_name => attributes}, options) - else - self.class.format.encode(attributes, options) - end else self.class.format.encode(attributes, options) end diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb index a424b2baf3..0f10a0e4d7 100644 --- a/activeresource/test/cases/base_test.rb +++ b/activeresource/test/cases/base_test.rb @@ -4,22 +4,20 @@ require "fixtures/customer" require "fixtures/street_address" require "fixtures/beast" require "fixtures/proxy" -require 'active_support/json' require 'active_support/core_ext/hash/conversions' require 'mocha' class BaseTest < Test::Unit::TestCase def setup + @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') + @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person') + @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') + @addy = { :id => 1, :street => '12345 Street', :country => 'Australia' }.to_xml(:root => 'address') @default_request_headers = { 'Content-Type' => 'application/xml' } - @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') - @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person') - @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') - @addy = { :id => 1, :street => '12345 Street', :country => 'Australia' }.to_xml(:root => 'address') - @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person") - @joe = { 'person' => { :id => 6, :name => 'Joe' }}.to_json + @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person") @people = [{ :id => 1, :name => 'Matz' }, { :id => 2, :name => 'David' }].to_xml(:root => 'people') @people_david = [{ :id => 2, :name => 'David' }].to_xml(:root => 'people') - @addresses = [{ :id => 1, :street => '12345 Street', :country => 'Australia' }].to_xml(:root => 'addresses') + @addresses = [{ :id => 1, :street => '12345 Street', :country => 'Australia' }].to_xml(:root => 'addresses') # - deep nested resource - # - Luis (Customer) @@ -68,7 +66,6 @@ class BaseTest < Test::Unit::TestCase ActiveResource::HttpMock.respond_to do |mock| mock.get "/people/1.xml", {}, @matz mock.get "/people/2.xml", {}, @david - mock.get "/people/6.json", {}, @joe mock.get "/people/5.xml", {}, @marty mock.get "/people/Greg.xml", {}, @greg mock.get "/people/4.xml", {'key' => 'value'}, nil, 404 @@ -1015,20 +1012,6 @@ class BaseTest < Test::Unit::TestCase assert xml.include?('1') end - - def test_to_json_including_root - Person.include_root_in_json = true - Person.format = :json - joe = Person.find(6) - json = joe.encode - Person.format = :xml - - assert_match %r{^\{"person":\{"person":\{}, json - assert_match %r{"id":6}, json - assert_match %r{"name":"Joe"}, json - assert_match %r{\}\}\}$}, json - end - def test_to_param_quacks_like_active_record new_person = Person.new assert_nil new_person.to_param -- cgit v1.2.3 From c9132c149cb9fe5628c2e947434e8c58acdaa709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 26 Apr 2010 09:04:04 +0200 Subject: Refactor tests by moving all middleware tests to the same place. --- railties/test/application/configuration_test.rb | 71 ++--------------- .../application/middleware_stack_defaults_test.rb | 54 ------------- railties/test/application/middleware_test.rb | 93 ++++++++++++++++++++++ railties/test/isolation/abstract_unit.rb | 19 +++++ 4 files changed, 117 insertions(+), 120 deletions(-) delete mode 100644 railties/test/application/middleware_stack_defaults_test.rb diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 4f1fc3c299..dfc4e2359b 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -109,7 +109,7 @@ module ApplicationTests end end - test "Frameworks are not preloaded by default" do + test "frameworks are not preloaded by default" do require "#{app_path}/config/environment" assert ActionController.autoload?(:RecordIdentifier) @@ -193,71 +193,10 @@ module ApplicationTests assert_equal File.join(app_path, "somewhere"), Rails.public_path end - def make_basic_app - require "rails" - require "action_controller/railtie" - - app = Class.new(Rails::Application) - - yield app if block_given? - - app.config.session_store :disabled - app.initialize! - - app.routes.draw do - match "/" => "omg#index" - end - - require 'rack/test' - extend Rack::Test::Methods - end - - test "config.action_dispatch.x_sendfile_header defaults to ''" do - make_basic_app - - class ::OmgController < ActionController::Base - def index - send_file __FILE__ - end - end - - get "/" - assert_equal File.read(__FILE__), last_response.body - end - - test "config.action_dispatch.x_sendfile_header can be set" do - make_basic_app do |app| - app.config.action_dispatch.x_sendfile_header = "X-Sendfile" - end - - class ::OmgController < ActionController::Base - def index - send_file __FILE__ - end - end - - get "/" - assert_equal File.expand_path(__FILE__), last_response.headers["X-Sendfile"] - end - - test "config.action_dispatch.x_sendfile_header is sent to Rack::Sendfile" do - make_basic_app do |app| - app.config.action_dispatch.x_sendfile_header = 'X-Lighttpd-Send-File' - end - - class ::OmgController < ActionController::Base - def index - send_file __FILE__ - end - end - - get "/" - assert_equal File.expand_path(__FILE__), last_response.headers["X-Lighttpd-Send-File"] - end - test "config.secret_token is sent in env" do make_basic_app do |app| app.config.secret_token = 'ThisIsASECRET123' + app.config.session_store :disabled end class ::OmgController < ActionController::Base @@ -287,9 +226,9 @@ module ApplicationTests end test "config.action_controller.perform_caching = true" do - make_basic_app do |app| - app.config.action_controller.perform_caching = true - end + make_basic_app do |app| + app.config.action_controller.perform_caching = true + end class ::OmgController < ActionController::Base @@count = 0 diff --git a/railties/test/application/middleware_stack_defaults_test.rb b/railties/test/application/middleware_stack_defaults_test.rb deleted file mode 100644 index f31ca01fbf..0000000000 --- a/railties/test/application/middleware_stack_defaults_test.rb +++ /dev/null @@ -1,54 +0,0 @@ -require 'isolation/abstract_unit' - -class MiddlewareStackDefaultsTest < Test::Unit::TestCase - include ActiveSupport::Testing::Isolation - - def setup - boot_rails - require "rails" - require "action_controller/railtie" - - Object.const_set(:MyApplication, Class.new(Rails::Application)) - MyApplication.class_eval do - config.secret_token = "3b7cd727ee24e8444053437c36cc66c4" - config.session_store :cookie_store, :key => "_myapp_session" - end - end - - def remote_ip(env = {}) - remote_ip = nil - env = Rack::MockRequest.env_for("/").merge(env).merge('action_dispatch.show_exceptions' => false) - - endpoint = Proc.new do |e| - remote_ip = ActionDispatch::Request.new(e).remote_ip - [200, {}, ["Hello"]] - end - - out = MyApplication.middleware.build(endpoint).call(env) - remote_ip - end - - test "remote_ip works" do - assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "1.1.1.1") - end - - test "checks IP spoofing by default" do - assert_raises(ActionDispatch::RemoteIp::IpSpoofAttackError) do - remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") - end - end - - test "can disable IP spoofing check" do - MyApplication.config.action_dispatch.ip_spoofing_check = false - - assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do - assert_equal "1.1.1.2", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") - end - end - - test "the user can set trusted proxies" do - MyApplication.config.action_dispatch.trusted_proxies = /^4\.2\.42\.42$/ - - assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "4.2.42.42,1.1.1.1") - end -end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 7f72881d55..27374dcb28 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -10,6 +10,10 @@ module ApplicationTests FileUtils.rm_rf "#{app_path}/config/environments" end + def app + @app ||= Rails.application + end + test "default middleware stack" do boot! @@ -83,7 +87,83 @@ module ApplicationTests assert middleware.include?("ActionDispatch::Cascade") end + # x_sendfile_header middleware + test "config.action_dispatch.x_sendfile_header defaults to ''" do + make_basic_app + + class ::OmgController < ActionController::Base + def index + send_file __FILE__ + end + end + + get "/" + assert_equal File.read(__FILE__), last_response.body + end + + test "config.action_dispatch.x_sendfile_header can be set" do + make_basic_app do |app| + app.config.action_dispatch.x_sendfile_header = "X-Sendfile" + end + + class ::OmgController < ActionController::Base + def index + send_file __FILE__ + end + end + + get "/" + assert_equal File.expand_path(__FILE__), last_response.headers["X-Sendfile"] + end + + test "config.action_dispatch.x_sendfile_header is sent to Rack::Sendfile" do + make_basic_app do |app| + app.config.action_dispatch.x_sendfile_header = 'X-Lighttpd-Send-File' + end + + class ::OmgController < ActionController::Base + def index + send_file __FILE__ + end + end + + get "/" + assert_equal File.expand_path(__FILE__), last_response.headers["X-Lighttpd-Send-File"] + end + + # remote_ip tests + test "remote_ip works" do + make_basic_app + assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "1.1.1.1") + end + + test "checks IP spoofing by default" do + make_basic_app + assert_raises(ActionDispatch::RemoteIp::IpSpoofAttackError) do + remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") + end + end + + test "can disable IP spoofing check" do + make_basic_app do |app| + app.config.action_dispatch.ip_spoofing_check = false + end + + assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do + assert_equal "1.1.1.2", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") + end + end + + test "the user can set trusted proxies" do + make_basic_app do |app| + app.config.action_dispatch.trusted_proxies = /^4\.2\.42\.42$/ + end + + assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "4.2.42.42,1.1.1.1") + end + private + def boot! require "#{app_path}/config/environment" end @@ -91,5 +171,18 @@ module ApplicationTests def middleware AppTemplate::Application.middleware.active.map(&:klass).map(&:name) end + + def remote_ip(env = {}) + remote_ip = nil + env = Rack::MockRequest.env_for("/").merge(env).merge('action_dispatch.show_exceptions' => false) + + endpoint = Proc.new do |e| + remote_ip = ActionDispatch::Request.new(e).remote_ip + [200, {}, ["Hello"]] + end + + Rails.application.middleware.build(endpoint).call(env) + remote_ip + end end end diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index f0c64b92ba..6f4c5d77f3 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -103,6 +103,25 @@ module TestHelpers add_to_config 'config.secret_token = "3b7cd727ee24e8444053437c36cc66c4"; config.session_store :cookie_store, :key => "_myapp_session"' end + def make_basic_app + require "rails" + require "action_controller/railtie" + + app = Class.new(Rails::Application) + app.config.secret_token = "3b7cd727ee24e8444053437c36cc66c4" + app.config.session_store :cookie_store, :key => "_myapp_session" + + yield app if block_given? + app.initialize! + + app.routes.draw do + match "/" => "omg#index" + end + + require 'rack/test' + extend ::Rack::Test::Methods + end + class Bukkit attr_reader :path -- cgit v1.2.3 From e461e1bc0ec0a5c365840031309b83143e12955a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 26 Apr 2010 09:36:13 +0200 Subject: Ensure application rake tasks and generators are loaded after the ones specified in railties/engines/rails. [#4471 state:resolved] --- railties/lib/rails/application.rb | 4 ++-- railties/lib/rails/tasks/documentation.rake | 2 ++ railties/lib/rails/test_unit/testing.rake | 2 ++ railties/test/application/rake_test.rb | 23 +++++++++++++++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 railties/test/application/rake_test.rb diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 38a5aa8ca3..7cec14c738 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -112,15 +112,15 @@ module Rails def load_tasks initialize_tasks - super railties.all { |r| r.load_tasks } + super self end def load_generators initialize_generators - super railties.all { |r| r.load_generators } + super self end diff --git a/railties/lib/rails/tasks/documentation.rake b/railties/lib/rails/tasks/documentation.rake index 957c375f6a..19d1fd2354 100644 --- a/railties/lib/rails/tasks/documentation.rake +++ b/railties/lib/rails/tasks/documentation.rake @@ -1,3 +1,5 @@ +require 'rake/rdoctask' + namespace :doc do def gem_path(gem_name) path = $LOAD_PATH.grep(/#{gem_name}[\w.-]*\/lib$/).first diff --git a/railties/lib/rails/test_unit/testing.rake b/railties/lib/rails/test_unit/testing.rake index 83f25506cb..f69a1169d9 100644 --- a/railties/lib/rails/test_unit/testing.rake +++ b/railties/lib/rails/test_unit/testing.rake @@ -1,3 +1,5 @@ +require 'rake/testtask' + TEST_CHANGES_SINCE = Time.now - 600 # Look up tests for recently modified sources. diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb new file mode 100644 index 0000000000..bf2da866f4 --- /dev/null +++ b/railties/test/application/rake_test.rb @@ -0,0 +1,23 @@ +require "isolation/abstract_unit" + +module ApplicationTests + class RakeTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation + + def setup + build_app + boot_rails + FileUtils.rm_rf("#{app_path}/config/environments") + end + + def test_gems_tasks_are_loaded_first_than_application_ones + app_file "lib/tasks/app.rake", <<-RUBY + $task_loaded = Rake::Task.task_defined?("db:create:all") + RUBY + + require "#{app_path}/config/environment" + ::Rails.application.load_tasks + assert $task_loaded + end + end +end \ No newline at end of file -- cgit v1.2.3 From 9b5a1f7ac19cdb7e4b661dd1603ceeede0d75d7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 26 Apr 2010 09:37:15 +0200 Subject: No need to require test and rdoc tasks. --- railties/lib/rails/generators/rails/app/templates/Rakefile | 3 --- 1 file changed, 3 deletions(-) diff --git a/railties/lib/rails/generators/rails/app/templates/Rakefile b/railties/lib/rails/generators/rails/app/templates/Rakefile index 9cb2046439..13f1f9fa41 100755 --- a/railties/lib/rails/generators/rails/app/templates/Rakefile +++ b/railties/lib/rails/generators/rails/app/templates/Rakefile @@ -2,9 +2,6 @@ # for example lib/tasks/capistrano.rake, and they will automatically be available to Rake. require File.expand_path('../config/application', __FILE__) - require 'rake' -require 'rake/testtask' -require 'rake/rdoctask' Rails::Application.load_tasks -- cgit v1.2.3 From 176fbfd66fe500ddfd04ffeb08ed014e705a82f9 Mon Sep 17 00:00:00 2001 From: David Chelimsky Date: Sun, 25 Apr 2010 16:04:22 -0500 Subject: extract ActionController::TestCase::Behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - this makes it possible for other test frameworks to hook into testing facilities provided by Rails without having to subclass AC::TestCase. [#4474 state:resolved] Signed-off-by: José Valim --- actionpack/lib/action_controller/test_case.rb | 259 +++++++++++++------------- 1 file changed, 133 insertions(+), 126 deletions(-) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 27939e3434..2b9b34961e 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -283,165 +283,143 @@ module ActionController # # assert_redirected_to page_url(:title => 'foo') class TestCase < ActiveSupport::TestCase - include ActionDispatch::TestProcess - include ActionController::TemplateAssertions + module Behavior + extend ActiveSupport::Concern + include ActionDispatch::TestProcess - attr_reader :response, :request + attr_reader :response, :request - # Executes a request simulating GET HTTP method and set/volley the response - def get(action, parameters = nil, session = nil, flash = nil) - process(action, parameters, session, flash, "GET") - end + module ClassMethods - # Executes a request simulating POST HTTP method and set/volley the response - def post(action, parameters = nil, session = nil, flash = nil) - process(action, parameters, session, flash, "POST") - end + # Sets the controller class name. Useful if the name can't be inferred from test class. + # Expects +controller_class+ as a constant. Example: tests WidgetController. + def tests(controller_class) + self.controller_class = controller_class + end + + def controller_class=(new_class) + prepare_controller_class(new_class) if new_class + write_inheritable_attribute(:controller_class, new_class) + end - # Executes a request simulating PUT HTTP method and set/volley the response - def put(action, parameters = nil, session = nil, flash = nil) - process(action, parameters, session, flash, "PUT") - end + def controller_class + if current_controller_class = read_inheritable_attribute(:controller_class) + current_controller_class + else + self.controller_class = determine_default_controller_class(name) + end + end - # Executes a request simulating DELETE HTTP method and set/volley the response - def delete(action, parameters = nil, session = nil, flash = nil) - process(action, parameters, session, flash, "DELETE") - end + def determine_default_controller_class(name) + name.sub(/Test$/, '').constantize + rescue NameError + nil + end - # Executes a request simulating HEAD HTTP method and set/volley the response - def head(action, parameters = nil, session = nil, flash = nil) - process(action, parameters, session, flash, "HEAD") - end + def prepare_controller_class(new_class) + new_class.send :include, ActionController::TestCase::RaiseActionExceptions + end - def xml_http_request(request_method, action, parameters = nil, session = nil, flash = nil) - @request.env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' - @request.env['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') - returning __send__(request_method, action, parameters, session, flash) do - @request.env.delete 'HTTP_X_REQUESTED_WITH' - @request.env.delete 'HTTP_ACCEPT' end - end - alias xhr :xml_http_request - - def process(action, parameters = nil, session = nil, flash = nil, http_method = 'GET') - # Sanity check for required instance variables so we can give an - # understandable error message. - %w(@routes @controller @request @response).each do |iv_name| - if !(instance_variable_names.include?(iv_name) || instance_variable_names.include?(iv_name.to_sym)) || instance_variable_get(iv_name).nil? - raise "#{iv_name} is nil: make sure you set it in your test's setup method." - end + + # Executes a request simulating GET HTTP method and set/volley the response + def get(action, parameters = nil, session = nil, flash = nil) + process(action, parameters, session, flash, "GET") end - @request.recycle! - @response.recycle! - @controller.response_body = nil - @controller.formats = nil - @controller.params = nil - - @html_document = nil - @request.env['REQUEST_METHOD'] = http_method - - parameters ||= {} - @request.assign_parameters(@routes, @controller.class.name.underscore.sub(/_controller$/, ''), action.to_s, parameters) - - @request.session = ActionController::TestSession.new(session) unless session.nil? - @request.session["flash"] = @request.flash.update(flash || {}) - @request.session["flash"].sweep - - @controller.request = @request - @controller.params.merge!(parameters) - build_request_uri(action, parameters) - Base.class_eval { include Testing } - @controller.process_with_new_base_test(@request, @response) - @request.session.delete('flash') if @request.session['flash'].blank? - @response - end + # Executes a request simulating POST HTTP method and set/volley the response + def post(action, parameters = nil, session = nil, flash = nil) + process(action, parameters, session, flash, "POST") + end - include ActionDispatch::Assertions + # Executes a request simulating PUT HTTP method and set/volley the response + def put(action, parameters = nil, session = nil, flash = nil) + process(action, parameters, session, flash, "PUT") + end - # When the request.remote_addr remains the default for testing, which is 0.0.0.0, the exception is simply raised inline - # (bystepping the regular exception handling from rescue_action). If the request.remote_addr is anything else, the regular - # rescue_action process takes place. This means you can test your rescue_action code by setting remote_addr to something else - # than 0.0.0.0. - # - # The exception is stored in the exception accessor for further inspection. - module RaiseActionExceptions - def self.included(base) - base.class_eval do - attr_accessor :exception - protected :exception, :exception= - end + # Executes a request simulating DELETE HTTP method and set/volley the response + def delete(action, parameters = nil, session = nil, flash = nil) + process(action, parameters, session, flash, "DELETE") end - protected - def rescue_action_without_handler(e) - self.exception = e + # Executes a request simulating HEAD HTTP method and set/volley the response + def head(action, parameters = nil, session = nil, flash = nil) + process(action, parameters, session, flash, "HEAD") + end - if request.remote_addr == "0.0.0.0" - raise(e) - else - super(e) + def xml_http_request(request_method, action, parameters = nil, session = nil, flash = nil) + @request.env['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest' + @request.env['HTTP_ACCEPT'] ||= [Mime::JS, Mime::HTML, Mime::XML, 'text/xml', Mime::ALL].join(', ') + returning __send__(request_method, action, parameters, session, flash) do + @request.env.delete 'HTTP_X_REQUESTED_WITH' + @request.env.delete 'HTTP_ACCEPT' + end + end + alias xhr :xml_http_request + + def process(action, parameters = nil, session = nil, flash = nil, http_method = 'GET') + # Sanity check for required instance variables so we can give an + # understandable error message. + %w(@routes @controller @request @response).each do |iv_name| + if !(instance_variable_names.include?(iv_name) || instance_variable_names.include?(iv_name.to_sym)) || instance_variable_get(iv_name).nil? + raise "#{iv_name} is nil: make sure you set it in your test's setup method." end end - end - setup :setup_controller_request_and_response + @request.recycle! + @response.recycle! + @controller.response_body = nil + @controller.formats = nil + @controller.params = nil - @@controller_class = nil + @html_document = nil + @request.env['REQUEST_METHOD'] = http_method - class << self - # Sets the controller class name. Useful if the name can't be inferred from test class. - # Expects +controller_class+ as a constant. Example: tests WidgetController. - def tests(controller_class) - self.controller_class = controller_class - end + parameters ||= {} + @request.assign_parameters(@routes, @controller.class.name.underscore.sub(/_controller$/, ''), action.to_s, parameters) - def controller_class=(new_class) - prepare_controller_class(new_class) if new_class - write_inheritable_attribute(:controller_class, new_class) - end + @request.session = ActionController::TestSession.new(session) unless session.nil? + @request.session["flash"] = @request.flash.update(flash || {}) + @request.session["flash"].sweep - def controller_class - if current_controller_class = read_inheritable_attribute(:controller_class) - current_controller_class - else - self.controller_class = determine_default_controller_class(name) - end + @controller.request = @request + @controller.params.merge!(parameters) + build_request_uri(action, parameters) + Base.class_eval { include Testing } + @controller.process_with_new_base_test(@request, @response) + @request.session.delete('flash') if @request.session['flash'].blank? + @response end - def determine_default_controller_class(name) - name.sub(/Test$/, '').constantize - rescue NameError - nil - end + def setup_controller_request_and_response + @request = TestRequest.new + @response = TestResponse.new - def prepare_controller_class(new_class) - new_class.send :include, RaiseActionExceptions - end - end + if klass = self.class.controller_class + @controller ||= klass.new rescue nil + end - def setup_controller_request_and_response - @request = TestRequest.new - @response = TestResponse.new + @request.env.delete('PATH_INFO') - if klass = self.class.controller_class - @controller ||= klass.new rescue nil + if @controller + @controller.request = @request + @controller.params = {} + end end - @request.env.delete('PATH_INFO') - - if @controller - @controller.request = @request - @controller.params = {} + # Cause the action to be rescued according to the regular rules for rescue_action when the visitor is not local + def rescue_action_in_public! + @request.remote_addr = '208.77.188.166' # example.com end - end - # Cause the action to be rescued according to the regular rules for rescue_action when the visitor is not local - def rescue_action_in_public! - @request.remote_addr = '208.77.188.166' # example.com - end + included do + include ActionController::TemplateAssertions + include ActionDispatch::Assertions + setup :setup_controller_request_and_response + end private + def build_request_uri(action, parameters) unless @request.env["PATH_INFO"] options = @controller.__send__(:url_options).merge(parameters) @@ -459,4 +437,33 @@ module ActionController end end end + + # When the request.remote_addr remains the default for testing, which is 0.0.0.0, the exception is simply raised inline + # (bystepping the regular exception handling from rescue_action). If the request.remote_addr is anything else, the regular + # rescue_action process takes place. This means you can test your rescue_action code by setting remote_addr to something else + # than 0.0.0.0. + # + # The exception is stored in the exception accessor for further inspection. + module RaiseActionExceptions + def self.included(base) + base.class_eval do + attr_accessor :exception + protected :exception, :exception= + end + end + + protected + def rescue_action_without_handler(e) + self.exception = e + + if request.remote_addr == "0.0.0.0" + raise(e) + else + super(e) + end + end + end + + include Behavior + end end -- cgit v1.2.3 From e813ad2a42549e308a69fd9473f1b9ed531a0d7e Mon Sep 17 00:00:00 2001 From: unknown Date: Sun, 25 Apr 2010 17:26:36 +1000 Subject: Make db console work for all versions of ruby on Windows. [#3999 state:committed] Signed-off-by: Jeremy Kemper --- railties/lib/rails/commands/dbconsole.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/railties/lib/rails/commands/dbconsole.rb b/railties/lib/rails/commands/dbconsole.rb index 68982b9f52..8957f11724 100644 --- a/railties/lib/rails/commands/dbconsole.rb +++ b/railties/lib/rails/commands/dbconsole.rb @@ -1,6 +1,7 @@ require 'erb' require 'yaml' require 'optparse' +require 'rbconfig' module Rails class DBConsole @@ -41,7 +42,7 @@ module Rails def find_cmd(*commands) dirs_on_path = ENV['PATH'].to_s.split(File::PATH_SEPARATOR) - commands += commands.map{|cmd| "#{cmd}.exe"} if RUBY_PLATFORM =~ /win32/ + commands += commands.map{|cmd| "#{cmd}.exe"} if Config::CONFIG['host_os'] =~ /mswin|mingw/ full_path_command = nil found = commands.detect do |cmd| -- cgit v1.2.3 From 53c13f1acaa2eb05e7f418b53f6156a4f5a773e0 Mon Sep 17 00:00:00 2001 From: Anil Wadghule Date: Mon, 26 Apr 2010 20:55:07 +0530 Subject: Use Config::CONFIG['host_os'] instead of RUBY_PLATFORM [#4477 state:resolved] Signed-off-by: Jeremy Kemper --- actionpack/test/controller/layout_test.rb | 3 ++- activesupport/lib/active_support/core_ext/kernel/reporting.rb | 3 ++- activesupport/lib/active_support/testing/isolation.rb | 3 ++- railties/Rakefile | 1 + railties/lib/rails/commands/runner.rb | 3 ++- railties/lib/rails/engine.rb | 3 ++- railties/lib/rails/generators/actions.rb | 5 +++-- railties/lib/rails/generators/rails/app/app_generator.rb | 3 ++- railties/lib/rails/test_unit/testing.rake | 2 +- 9 files changed, 17 insertions(+), 9 deletions(-) diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index e1c1128753..48be7571ea 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'rbconfig' # The view_paths array must be set on Base and not LayoutTest so that LayoutTest's inherited # method has access to the view_paths array when looking for a layout to automatically assign. @@ -209,7 +210,7 @@ class LayoutStatusIsRenderedTest < ActionController::TestCase end end -unless RUBY_PLATFORM =~ /mswin|mingw/ +unless Config::CONFIG['host_os'] =~ /mswin|mingw/ class LayoutSymlinkedTest < LayoutTest layout "symlinked/symlinked_layout" end diff --git a/activesupport/lib/active_support/core_ext/kernel/reporting.rb b/activesupport/lib/active_support/core_ext/kernel/reporting.rb index ac35db6ab6..7c455f66d5 100644 --- a/activesupport/lib/active_support/core_ext/kernel/reporting.rb +++ b/activesupport/lib/active_support/core_ext/kernel/reporting.rb @@ -1,3 +1,4 @@ +require 'rbconfig' module Kernel # Sets $VERBOSE to nil for the duration of the block and back to its original value afterwards. # @@ -37,7 +38,7 @@ module Kernel # puts 'But this will' def silence_stream(stream) old_stream = stream.dup - stream.reopen(RUBY_PLATFORM =~ /mswin|mingw/ ? 'NUL:' : '/dev/null') + stream.reopen(Config::CONFIG['host_os'] =~ /mswin|mingw/ ? 'NUL:' : '/dev/null') stream.sync = true yield ensure diff --git a/activesupport/lib/active_support/testing/isolation.rb b/activesupport/lib/active_support/testing/isolation.rb index 9507dbf473..69df399cde 100644 --- a/activesupport/lib/active_support/testing/isolation.rb +++ b/activesupport/lib/active_support/testing/isolation.rb @@ -1,3 +1,4 @@ +require 'rbconfig' module ActiveSupport module Testing class RemoteError < StandardError @@ -33,7 +34,7 @@ module ActiveSupport module Isolation def self.forking_env? - !ENV["NO_FORK"] && RUBY_PLATFORM !~ /mswin|mingw|java/ + !ENV["NO_FORK"] && ((Config::CONFIG['host_os'] !~ /mswin|mingw/) && (RUBY_PLATFORM !~ /java/)) end def self.included(base) diff --git a/railties/Rakefile b/railties/Rakefile index d88036f829..daffd8ce30 100644 --- a/railties/Rakefile +++ b/railties/Rakefile @@ -6,6 +6,7 @@ require 'rake/gempackagetask' require 'date' require 'rbconfig' + task :default => :test task :test => 'test:isolated' diff --git a/railties/lib/rails/commands/runner.rb b/railties/lib/rails/commands/runner.rb index 5634ee0f69..1dd11e1241 100644 --- a/railties/lib/rails/commands/runner.rb +++ b/railties/lib/rails/commands/runner.rb @@ -1,4 +1,5 @@ require 'optparse' +require 'rbconfig' options = { :environment => (ENV['RAILS_ENV'] || "development").dup } code_or_file = nil @@ -18,7 +19,7 @@ ARGV.clone.options do |opts| opts.on("-h", "--help", "Show this help message.") { $stderr.puts opts; exit } - if RUBY_PLATFORM !~ /mswin|mingw/ + if Config::CONFIG['host_os'] !~ /mswin|mingw/ opts.separator "" opts.separator "You can also use runner as a shebang line for your scripts like this:" opts.separator "-------------------------------------------------------------" diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 98da7e2b4a..36fcc896ae 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -1,6 +1,7 @@ require 'rails/railtie' require 'active_support/core_ext/module/delegation' require 'pathname' +require 'rbconfig' module Rails # Rails::Engine allows you to wrap a specific Rails application and share it accross @@ -119,7 +120,7 @@ module Rails root = File.exist?("#{root_path}/#{flag}") ? root_path : default raise "Could not find root path for #{self}" unless root - RUBY_PLATFORM =~ /mswin|mingw/ ? + Config::CONFIG['host_os'] =~ /mswin|mingw/ ? Pathname.new(root).expand_path : Pathname.new(root).realpath end end diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 7dec4d446a..a31932906d 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -1,5 +1,6 @@ require 'open-uri' require 'active_support/deprecation' +require 'rbconfig' module Rails module Generators @@ -240,7 +241,7 @@ module Rails def rake(command, options={}) log :rake, command env = options[:env] || 'development' - sudo = options[:sudo] && RUBY_PLATFORM !~ /mswin|mingw/ ? 'sudo ' : '' + sudo = options[:sudo] && Config::CONFIG['host_os'] !~ /mswin|mingw/ ? 'sudo ' : '' in_root { run("#{sudo}#{extify(:rake)} #{command} RAILS_ENV=#{env}", :verbose => false) } end @@ -307,7 +308,7 @@ module Rails # Add an extension to the given name based on the platform. # def extify(name) - if RUBY_PLATFORM =~ /mswin|mingw/ + if Config::CONFIG['host_os'] =~ /mswin|mingw/ "#{name}.bat" else name diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 6818fafbe9..aa066fe3c4 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -1,6 +1,7 @@ require 'digest/md5' require 'active_support/secure_random' require 'rails/version' unless defined?(Rails::VERSION) +require 'rbconfig' module Rails::Generators # We need to store the RAILS_DEV_PATH in a constant, otherwise the path @@ -265,7 +266,7 @@ module Rails::Generators "/opt/local/var/run/mysql4/mysqld.sock", # mac + darwinports + mysql4 "/opt/local/var/run/mysql5/mysqld.sock", # mac + darwinports + mysql5 "/opt/lampp/var/mysql/mysql.sock" # xampp for linux - ].find { |f| File.exist?(f) } unless RUBY_PLATFORM =~ /mswin|mingw/ + ].find { |f| File.exist?(f) } unless Config::CONFIG['host_os'] =~ /mswin|mingw/ end def empty_directory_with_gitkeep(destination, config = {}) diff --git a/railties/lib/rails/test_unit/testing.rake b/railties/lib/rails/test_unit/testing.rake index f69a1169d9..79fa667ed1 100644 --- a/railties/lib/rails/test_unit/testing.rake +++ b/railties/lib/rails/test_unit/testing.rake @@ -32,7 +32,7 @@ end module Kernel def silence_stderr old_stderr = STDERR.dup - STDERR.reopen(RUBY_PLATFORM =~ /mswin|mingw/ ? 'NUL:' : '/dev/null') + STDERR.reopen(Config::CONFIG['host_os'] =~ /mswin|mingw/ ? 'NUL:' : '/dev/null') STDERR.sync = true yield ensure -- cgit v1.2.3 From 76d6a993647f3bbe39f31faa0b8ed8767a228301 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 26 Apr 2010 11:50:18 -0300 Subject: Use explicit source encoding rather than forced UTF-8 from US-ASCII. Signed-off-by: Jeremy Kemper --- actionmailer/test/old_base/mail_service_test.rb | 2 -- activesupport/lib/active_support/core_ext/uri.rb | 3 ++- activesupport/test/core_ext/uri_ext_test.rb | 2 +- activesupport/test/multibyte_chars_test.rb | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/actionmailer/test/old_base/mail_service_test.rb b/actionmailer/test/old_base/mail_service_test.rb index db2db59cc7..9da9132fe1 100644 --- a/actionmailer/test/old_base/mail_service_test.rb +++ b/actionmailer/test/old_base/mail_service_test.rb @@ -866,7 +866,6 @@ EOF regex = Regexp.escape('Subject: Foo =?UTF-8?Q?=C3=A1=C3=AB=C3=B4=?= =?UTF-8?Q?_=C3=AE=C3=BC=?=') assert_match(/#{regex}/, mail.encoded) string = "Foo áëô îü" - string.force_encoding('UTF-8') if string.respond_to?(:force_encoding) assert_match(string, mail.subject) end @@ -875,7 +874,6 @@ EOF regex = Regexp.escape('Subject: Foo =?UTF-8?Q?=C3=A1=C3=AB=C3=B4=?= =?UTF-8?Q?_=C3=AE=C3=BC=?=') assert_match(/#{regex}/, mail.encoded) string = "Foo áëô îü" - string.force_encoding('UTF-8') if string.respond_to?(:force_encoding) assert_match(string, mail.subject) end diff --git a/activesupport/lib/active_support/core_ext/uri.rb b/activesupport/lib/active_support/core_ext/uri.rb index ca1be8b7e9..28eabd2111 100644 --- a/activesupport/lib/active_support/core_ext/uri.rb +++ b/activesupport/lib/active_support/core_ext/uri.rb @@ -1,8 +1,9 @@ +# encoding: utf-8 + if RUBY_VERSION >= '1.9' require 'uri' str = "\xE6\x97\xA5\xE6\x9C\xAC\xE8\xAA\x9E" # Ni-ho-nn-go in UTF-8, means Japanese. - str.force_encoding(Encoding::UTF_8) if str.respond_to?(:force_encoding) parser = URI::Parser.new unless str == parser.unescape(parser.escape(str)) diff --git a/activesupport/test/core_ext/uri_ext_test.rb b/activesupport/test/core_ext/uri_ext_test.rb index e4a242abc4..d988837603 100644 --- a/activesupport/test/core_ext/uri_ext_test.rb +++ b/activesupport/test/core_ext/uri_ext_test.rb @@ -1,3 +1,4 @@ +# encoding: utf-8 require 'abstract_unit' require 'uri' require 'active_support/core_ext/uri' @@ -5,7 +6,6 @@ require 'active_support/core_ext/uri' class URIExtTest < Test::Unit::TestCase def test_uri_decode_handle_multibyte str = "\xE6\x97\xA5\xE6\x9C\xAC\xE8\xAA\x9E" # Ni-ho-nn-go in UTF-8, means Japanese. - str.force_encoding(Encoding::UTF_8) if str.respond_to?(:force_encoding) if URI.const_defined?(:Parser) parser = URI::Parser.new diff --git a/activesupport/test/multibyte_chars_test.rb b/activesupport/test/multibyte_chars_test.rb index 1b8d13c024..caf50aa1c9 100644 --- a/activesupport/test/multibyte_chars_test.rb +++ b/activesupport/test/multibyte_chars_test.rb @@ -105,7 +105,7 @@ class MultibyteCharsUTF8BehaviourTest < Test::Unit::TestCase @whitespace = "\n\t#{[32, 8195].pack('U*')}" else # Ruby 1.9 only supports basic whitespace - @whitespace = "\n\t ".force_encoding(Encoding::UTF_8) + @whitespace = "\n\t " end @byte_order_mark = [65279].pack('U') -- cgit v1.2.3 From 7cd1d37a51f5f53f8fc1360f886d26cabf12d969 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 26 Apr 2010 18:27:16 -0300 Subject: Reuse Active Model serialization in Active Resource. [#2584 state:committed] Signed-off-by: Jeremy Kemper --- activeresource/lib/active_resource/base.rb | 66 ++---------------------------- activeresource/test/cases/base_test.rb | 28 ++++++++++--- 2 files changed, 25 insertions(+), 69 deletions(-) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index bf775a14c8..ad994214f6 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -1176,73 +1176,11 @@ module ActiveResource !new? && self.class.exists?(to_param, :params => prefix_options) end - # Converts the resource to an XML string representation. - # - # ==== Options - # The +options+ parameter is handed off to the +to_xml+ method on each - # attribute, so it has the same options as the +to_xml+ methods in - # Active Support. - # - # * :indent - Set the indent level for the XML output (default is +2+). - # * :dasherize - Boolean option to determine whether or not element names should - # replace underscores with dashes (default is false). - # * :skip_instruct - Toggle skipping the +instruct!+ call on the XML builder - # that generates the XML declaration (default is false). - # - # ==== Examples - # my_group = SubsidiaryGroup.find(:first) - # my_group.to_xml - # # => - # # [...] - # - # my_group.to_xml(:dasherize => true) - # # => - # # [...] - # - # my_group.to_xml(:skip_instruct => true) - # # => [...] - def to_xml(options={}) - attributes.to_xml({:root => self.class.element_name}.merge(options)) - end - - # Coerces to a hash for JSON encoding. - # - # ==== Options - # The +options+ are passed to the +to_json+ method on each - # attribute, so the same options as the +to_json+ methods in - # Active Support. - # - # * :only - Only include the specified attribute or list of - # attributes in the serialized output. Attribute names must be specified - # as strings. - # * :except - Do not include the specified attribute or list of - # attributes in the serialized output. Attribute names must be specified - # as strings. - # - # ==== Examples - # person = Person.new(:first_name => "Jim", :last_name => "Smith") - # person.to_json - # # => {"first_name": "Jim", "last_name": "Smith"} - # - # person.to_json(:only => ["first_name"]) - # # => {"first_name": "Jim"} - # - # person.to_json(:except => ["first_name"]) - # # => {"last_name": "Smith"} - def as_json(options = nil) - attributes.as_json(options) - end - # Returns the serialized string representation of the resource in the configured # serialization format specified in ActiveResource::Base.format. The options # applicable depend on the configured encoding format. def encode(options={}) - case self.class.format - when ActiveResource::Formats::XmlFormat - self.class.format.encode(attributes, {:root => self.class.element_name}.merge(options)) - else - self.class.format.encode(attributes, options) - end + send("to_#{self.class.format.extension}", options) end # A method to \reload the attributes of this object from the remote web service. @@ -1466,5 +1404,7 @@ module ActiveResource class Base extend ActiveModel::Naming include CustomMethods, Observing, Validations + include ActiveModel::Serializers::JSON + include ActiveModel::Serializers::Xml end end diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb index 0f10a0e4d7..2ed7e1c95f 100644 --- a/activeresource/test/cases/base_test.rb +++ b/activeresource/test/cases/base_test.rb @@ -4,20 +4,22 @@ require "fixtures/customer" require "fixtures/street_address" require "fixtures/beast" require "fixtures/proxy" +require 'active_support/json' require 'active_support/core_ext/hash/conversions' require 'mocha' class BaseTest < Test::Unit::TestCase def setup - @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') - @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person') - @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') - @addy = { :id => 1, :street => '12345 Street', :country => 'Australia' }.to_xml(:root => 'address') @default_request_headers = { 'Content-Type' => 'application/xml' } - @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person") + @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') + @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person') + @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') + @addy = { :id => 1, :street => '12345 Street', :country => 'Australia' }.to_xml(:root => 'address') + @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person") + @joe = { 'person' => { :id => 6, :name => 'Joe' }}.to_json @people = [{ :id => 1, :name => 'Matz' }, { :id => 2, :name => 'David' }].to_xml(:root => 'people') @people_david = [{ :id => 2, :name => 'David' }].to_xml(:root => 'people') - @addresses = [{ :id => 1, :street => '12345 Street', :country => 'Australia' }].to_xml(:root => 'addresses') + @addresses = [{ :id => 1, :street => '12345 Street', :country => 'Australia' }].to_xml(:root => 'addresses') # - deep nested resource - # - Luis (Customer) @@ -66,6 +68,7 @@ class BaseTest < Test::Unit::TestCase ActiveResource::HttpMock.respond_to do |mock| mock.get "/people/1.xml", {}, @matz mock.get "/people/2.xml", {}, @david + mock.get "/people/6.json", {}, @joe mock.get "/people/5.xml", {}, @marty mock.get "/people/Greg.xml", {}, @greg mock.get "/people/4.xml", {'key' => 'value'}, nil, 404 @@ -1012,6 +1015,19 @@ class BaseTest < Test::Unit::TestCase assert xml.include?('1') end + def test_to_json + Person.include_root_in_json = true + Person.format = :json + joe = Person.find(6) + json = joe.encode + Person.format = :xml + + assert_match %r{^\{"person":\{"person":\{}, json + assert_match %r{"id":6}, json + assert_match %r{"name":"Joe"}, json + assert_match %r{\}\}\}$}, json + end + def test_to_param_quacks_like_active_record new_person = Person.new assert_nil new_person.to_param -- cgit v1.2.3 From 43e2fd93b4fa92ca23d8bc8e68e1bf5a94038461 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 26 Apr 2010 15:21:04 -0700 Subject: Update CHANGELOG for include_root_in_json. --- activeresource/CHANGELOG | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/activeresource/CHANGELOG b/activeresource/CHANGELOG index 91dccb9671..bd97b2d549 100644 --- a/activeresource/CHANGELOG +++ b/activeresource/CHANGELOG @@ -1,3 +1,8 @@ +*Rails 3.0.0 [beta 4/release candidate] (unreleased)* + +* JSON: set Base.include_root_in_json = true to include a root value in the JSON: {"post": {"title": ...}}. Mirrors the Active Record option. [Santiago Pastorino] + + *Rails 3.0.0 [beta 3] (April 13th, 2010)* * No changes -- cgit v1.2.3 From c1d73270717f30498f8f4d55d6695509107c2834 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 26 Apr 2010 19:32:23 -0700 Subject: JSON: encode objects that don't have a native JSON representation using to_hash, if available, instead of instance_values (the old fallback) or to_s (other encoders' default). Encode BigDecimal and Regexp encode as strings to conform with other encoders. Try to transcode non-UTF-8 strings. --- activesupport/CHANGELOG | 5 +++++ activesupport/lib/active_support/json/encoding.rb | 24 ++++++++++++++++++----- activesupport/test/json/encoding_test.rb | 21 ++++++++++++++++++-- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 5ada5a1e9b..c47839b001 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,3 +1,8 @@ +*Rails 3.0.0 [beta 4/release candidate] (unreleased)* + +* JSON: encode objects that don't have a native JSON representation using to_hash, if available, instead of instance_values (the old fallback) or to_s (other encoders' default). Encode BigDecimal and Regexp encode as strings to conform with other encoders. Try to transcode non-UTF-8 strings. [Jeremy Kemper] + + *Rails 3.0.0 [beta 3] (April 13th, 2010)* * HashWithIndifferentAccess: remove inherited symbolize_keys! since its keys are always strings. [Santiago Pastorino] diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb index 8ba45f7ea2..0f38fd0e89 100644 --- a/activesupport/lib/active_support/json/encoding.rb +++ b/activesupport/lib/active_support/json/encoding.rb @@ -1,4 +1,5 @@ # encoding: utf-8 +require 'bigdecimal' require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/hash/except' require 'active_support/core_ext/hash/slice' @@ -102,7 +103,9 @@ module ActiveSupport end def escape(string) - string = string.dup.force_encoding(::Encoding::BINARY) if string.respond_to?(:force_encoding) + if string.respond_to?(:force_encoding) + string = string.encode(::Encoding::UTF_8, undef: :replace).force_encoding(::Encoding::BINARY) + end json = string. gsub(escape_regex) { |s| ESCAPED_CHARS[s] }. gsub(/([\xC0-\xDF][\x80-\xBF]| @@ -110,7 +113,9 @@ module ActiveSupport [\xF0-\xF7][\x80-\xBF]{3})+/nx) { |s| s.unpack("U*").pack("n*").unpack("H*")[0].gsub(/.{4}/n, '\\\\u\&') } - %("#{json}") + json = %("#{json}") + json.force_encoding(::Encoding::UTF_8) if json.respond_to?(:force_encoding) + json end end @@ -128,7 +133,13 @@ class Object ActiveSupport::JSON.encode(self, options) end - def as_json(options = nil) instance_values end #:nodoc: + def as_json(options = nil) #:nodoc: + if respond_to?(:to_hash) + to_hash + else + instance_values + end + end end # A string that returns itself as its JSON-encoded form. @@ -166,9 +177,12 @@ class Numeric def encode_json(encoder) to_s end #:nodoc: end +class BigDecimal + def as_json(options = nil) to_s end #:nodoc: +end + class Regexp - def as_json(options = nil) self end #:nodoc: - def encode_json(encoder) inspect end #:nodoc: + def as_json(options = nil) to_s end #:nodoc: end module Enumerable diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index 188b799f3f..ff95c0ca18 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -9,6 +9,12 @@ class TestJSONEncoding < Test::Unit::TestCase end end + class Hashlike + def to_hash + { :a => 1 } + end + end + class Custom def as_json(options) 'custom' @@ -19,7 +25,8 @@ class TestJSONEncoding < Test::Unit::TestCase FalseTests = [[ false, %(false) ]] NilTests = [[ nil, %(null) ]] NumericTests = [[ 1, %(1) ], - [ 2.5, %(2.5) ]] + [ 2.5, %(2.5) ], + [ BigDecimal('2.5'), %("#{BigDecimal('2.5').to_s}") ]] StringTests = [[ 'this is the ', %("this is the \\u003Cstring\\u003E")], [ 'a "string" with quotes & an ampersand', %("a \\"string\\" with quotes \\u0026 an ampersand") ], @@ -35,11 +42,12 @@ class TestJSONEncoding < Test::Unit::TestCase [ :"a b", %("a b") ]] ObjectTests = [[ Foo.new(1, 2), %({\"a\":1,\"b\":2}) ]] + HashlikeTests = [[ Hashlike.new, %({\"a\":1}) ]] CustomTests = [[ Custom.new, '"custom"' ]] VariableTests = [[ ActiveSupport::JSON::Variable.new('foo'), 'foo'], [ ActiveSupport::JSON::Variable.new('alert("foo")'), 'alert("foo")']] - RegexpTests = [[ /^a/, '/^a/' ], [/^\w{1,2}[a-z]+/ix, '/^\\w{1,2}[a-z]+/ix']] + RegexpTests = [[ /^a/, '"(?-mix:^a)"' ], [/^\w{1,2}[a-z]+/ix, '"(?ix-m:^\\\\w{1,2}[a-z]+)"']] DateTests = [[ Date.new(2005,2,1), %("2005/02/01") ]] TimeTests = [[ Time.utc(2005,2,1,15,15,10), %("2005/02/01 15:15:10 +0000") ]] @@ -91,6 +99,15 @@ class TestJSONEncoding < Test::Unit::TestCase end end + if '1.9'.respond_to?(:force_encoding) + def test_non_utf8_string_transcodes + s = '二'.encode('Shift_JIS') + result = ActiveSupport::JSON.encode(s) + assert_equal '"\\u4e8c"', result + assert_equal Encoding::UTF_8, result.encoding + end + end + def test_exception_raised_when_encoding_circular_reference a = [1] a << a -- cgit v1.2.3