diff options
112 files changed, 1654 insertions, 578 deletions
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 16fcf112b7..cd76383931 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -4,6 +4,7 @@ require 'action_mailer/collector' require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/proc' +require 'active_support/core_ext/string/inflections' require 'action_mailer/log_subscriber' module ActionMailer #:nodoc: @@ -349,9 +350,6 @@ module ActionMailer #:nodoc: helper ActionMailer::MailHelper include ActionMailer::OldApi - delegate :register_observer, :to => Mail - delegate :register_interceptor, :to => Mail - private_class_method :new #:nodoc: class_attribute :default_params @@ -363,6 +361,32 @@ module ActionMailer #:nodoc: }.freeze class << self + # Register one or more Observers which will be notified when mail is delivered. + def register_observers(*observers) + observers.flatten.compact.each { |observer| register_observer(observer) } + end + + # Register one or more Interceptors which will be called before mail is sent. + def register_interceptors(*interceptors) + interceptors.flatten.compact.each { |interceptor| register_interceptor(interceptor) } + end + + # Register an Observer which will be notified when mail is delivered. + # Either a class or a string can be passed in as the Observer. If a string is passed in + # it will be <tt>constantize</tt>d. + def register_observer(observer) + delivery_observer = (observer.is_a?(String) ? observer.constantize : observer) + Mail.register_observer(delivery_observer) + end + + # Register an Inteceptor which will be called before mail is sent. + # Either a class or a string can be passed in as the Observer. If a string is passed in + # it will be <tt>constantize</tt>d. + def register_interceptor(interceptor) + delivery_interceptor = (interceptor.is_a?(String) ? interceptor.constantize : interceptor) + Mail.register_interceptor(delivery_interceptor) + end + def mailer_name @mailer_name ||= name.underscore end diff --git a/actionmailer/lib/action_mailer/railtie.rb b/actionmailer/lib/action_mailer/railtie.rb index 4ec478067f..444754d0e9 100644 --- a/actionmailer/lib/action_mailer/railtie.rb +++ b/actionmailer/lib/action_mailer/railtie.rb @@ -19,13 +19,17 @@ module ActionMailer options.stylesheets_dir ||= paths["public/stylesheets"].first # make sure readers methods get compiled - options.asset_path ||= app.config.asset_path - options.asset_host ||= app.config.asset_host + options.asset_path ||= app.config.asset_path + options.asset_host ||= app.config.asset_host ActiveSupport.on_load(:action_mailer) do include AbstractController::UrlFor extend ::AbstractController::Railties::RoutesHelpers.with(app.routes) include app.routes.mounted_helpers + + register_interceptors(options.delete(:interceptors)) + register_observers(options.delete(:observers)) + options.each { |k,v| send("#{k}=", v) } end end diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 6a7931da8c..9fdd0e1ced 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -478,6 +478,11 @@ class BaseTest < ActiveSupport::TestCase end end + class MySecondObserver + def self.delivered_email(mail) + end + end + test "you can register an observer to the mail object that gets informed on email delivery" do ActionMailer::Base.register_observer(MyObserver) mail = BaseMailer.welcome @@ -485,11 +490,31 @@ class BaseTest < ActiveSupport::TestCase mail.deliver end + test "you can register an observer using its stringified name to the mail object that gets informed on email delivery" do + ActionMailer::Base.register_observer("BaseTest::MyObserver") + mail = BaseMailer.welcome + MyObserver.expects(:delivered_email).with(mail) + mail.deliver + end + + test "you can register multiple observers to the mail object that both get informed on email delivery" do + ActionMailer::Base.register_observers("BaseTest::MyObserver", MySecondObserver) + mail = BaseMailer.welcome + MyObserver.expects(:delivered_email).with(mail) + MySecondObserver.expects(:delivered_email).with(mail) + mail.deliver + end + class MyInterceptor def self.delivering_email(mail) end end + class MySecondInterceptor + def self.delivering_email(mail) + end + end + test "you can register an interceptor to the mail object that gets passed the mail object before delivery" do ActionMailer::Base.register_interceptor(MyInterceptor) mail = BaseMailer.welcome @@ -497,6 +522,21 @@ class BaseTest < ActiveSupport::TestCase mail.deliver end + test "you can register an interceptor using its stringified name to the mail object that gets passed the mail object before delivery" do + ActionMailer::Base.register_interceptor("BaseTest::MyInterceptor") + mail = BaseMailer.welcome + MyInterceptor.expects(:delivering_email).with(mail) + mail.deliver + end + + test "you can register multiple interceptors to the mail object that both get passed the mail object before delivery" do + ActionMailer::Base.register_interceptors("BaseTest::MyInterceptor", MySecondInterceptor) + mail = BaseMailer.welcome + MyInterceptor.expects(:delivering_email).with(mail) + MySecondInterceptor.expects(:delivering_email).with(mail) + mail.deliver + end + test "being able to put proc's into the defaults hash and they get evaluated on mail sending" do mail1 = ProcMailer.welcome yesterday = 1.day.ago diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 0f6bc10dd5..618a6cdb7d 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -1,5 +1,6 @@ require 'abstract_controller/collector' require 'active_support/core_ext/class/attribute' +require 'active_support/core_ext/object/inclusion' module ActionController #:nodoc: module MimeResponds @@ -248,9 +249,9 @@ module ActionController #:nodoc: config = self.class.mimes_for_respond_to[mime] if config[:except] - !config[:except].include?(action) + !action.in?(config[:except]) elsif config[:only] - config[:only].include?(action) + action.in?(config[:only]) else true end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 49971fc9f8..7f972fc281 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -60,6 +60,7 @@ module ActionDispatch autoload :Static end + autoload :ClosedError, 'action_dispatch/middleware/closed_error' autoload :MiddlewareStack, 'action_dispatch/middleware/stack' autoload :Routing diff --git a/actionpack/lib/action_dispatch/middleware/closed_error.rb b/actionpack/lib/action_dispatch/middleware/closed_error.rb new file mode 100644 index 0000000000..0a4db47f4b --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/closed_error.rb @@ -0,0 +1,7 @@ +module ActionDispatch + class ClosedError < StandardError #:nodoc: + def initialize(kind) + super "Cannot modify #{kind} because it was closed. This means it was already streamed back to the client or converted to HTTP headers." + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 7ac608f0a8..24ebb8fed7 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -83,7 +83,7 @@ module ActionDispatch # Raised when storing more than 4K of session data. class CookieOverflow < StandardError; end - class CookieJar < Hash #:nodoc: + class CookieJar #:nodoc: # This regular expression is used to split the levels of a domain. # The top level domain can be any string without a period or @@ -115,13 +115,22 @@ module ActionDispatch @delete_cookies = {} @host = host @secure = secure - - super() + @closed = false + @cookies = {} end + attr_reader :closed + alias :closed? :closed + def close!; @closed = true end + # Returns the value of the cookie by +name+, or +nil+ if no such cookie exists. def [](name) - super(name.to_s) + @cookies[name.to_s] + end + + def update(other_hash) + @cookies.update other_hash + self end def handle_options(options) #:nodoc: @@ -145,6 +154,7 @@ module ActionDispatch # Sets the cookie named +name+. The second argument may be the very cookie # value, or a hash of options as documented above. def []=(key, options) + raise ClosedError, :cookies if closed? if options.is_a?(Hash) options.symbolize_keys! value = options[:value] @@ -153,7 +163,7 @@ module ActionDispatch options = { :value => value } end - value = super(key.to_s, value) + value = @cookies[key.to_s] = value handle_options(options) @@ -170,7 +180,7 @@ module ActionDispatch handle_options(options) - value = super(key.to_s) + value = @cookies.delete(key.to_s) @delete_cookies[key] = options value end @@ -225,6 +235,7 @@ module ActionDispatch end def []=(key, options) + raise ClosedError, :cookies if closed? if options.is_a?(Hash) options.symbolize_keys! else @@ -263,6 +274,7 @@ module ActionDispatch end def []=(key, options) + raise ClosedError, :cookies if closed? if options.is_a?(Hash) options.symbolize_keys! options[:value] = @verifier.generate(options[:value]) @@ -305,6 +317,7 @@ module ActionDispatch end def call(env) + cookie_jar = nil status, headers, body = @app.call(env) if cookie_jar = env['action_dispatch.cookies'] @@ -315,6 +328,9 @@ module ActionDispatch end [status, headers, body] + ensure + cookie_jar = ActionDispatch::Request.new(env).cookie_jar unless cookie_jar + cookie_jar.close! end end end diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 21aeeb217a..027ff7f8ac 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -43,9 +43,15 @@ module ActionDispatch class FlashNow #:nodoc: def initialize(flash) @flash = flash + @closed = false end + attr_reader :closed + alias :closed? :closed + def close!; @closed = true end + def []=(k, v) + raise ClosedError, :flash if closed? @flash[k] = v @flash.discard(k) v @@ -66,27 +72,70 @@ module ActionDispatch end end - class FlashHash < Hash + class FlashHash + include Enumerable + def initialize #:nodoc: - super - @used = Set.new + @used = Set.new + @closed = false + @flashes = {} end + attr_reader :closed + alias :closed? :closed + def close!; @closed = true end + def []=(k, v) #:nodoc: + raise ClosedError, :flash if closed? keep(k) - super + @flashes[k] = v + end + + def [](k) + @flashes[k] end def update(h) #:nodoc: h.keys.each { |k| keep(k) } - super + @flashes.update h + self + end + + def keys + @flashes.keys + end + + def key?(name) + @flashes.key? name + end + + def delete(key) + @flashes.delete key + self + end + + def to_hash + @flashes.dup + end + + def empty? + @flashes.empty? + end + + def clear + @flashes.clear + end + + def each(&block) + @flashes.each(&block) end alias :merge! :update def replace(h) #:nodoc: @used = Set.new - super + @flashes.replace h + self end # Sets a flash that will not be available to the next action, only to the current. @@ -100,7 +149,7 @@ module ActionDispatch # # Entries set via <tt>now</tt> are accessed the same way as standard entries: <tt>flash['my-key']</tt>. def now - FlashNow.new(self) + @now ||= FlashNow.new(self) end # Keeps either the entire current flash or a specific flash entry available for the next action: @@ -184,8 +233,11 @@ module ActionDispatch session = env['rack.session'] || {} flash_hash = env['action_dispatch.request.flash_hash'] - if flash_hash && (!flash_hash.empty? || session.key?('flash')) + if flash_hash + if !flash_hash.empty? || session.key?('flash') session["flash"] = flash_hash + end + flash_hash.close! end if session.key?('flash') && session['flash'].empty? diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 35be0b3a27..13aef18ee1 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1,6 +1,7 @@ require 'erb' require 'active_support/core_ext/hash/except' require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/object/inclusion' require 'active_support/inflector' require 'action_dispatch/routing/redirection' @@ -1345,11 +1346,11 @@ module ActionDispatch end def resource_scope? #:nodoc: - [:resource, :resources].include?(@scope[:scope_level]) + @scope[:scope_level].among?(:resource, :resources) end def resource_method_scope? #:nodoc: - [:collection, :member, :new].include?(@scope[:scope_level]) + @scope[:scope_level].among?(:collection, :member, :new) end def with_exclusive_scope diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 77a15f3e97..16a6b93ce8 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + module ActionDispatch module Assertions # A small suite of assertions that test responses from \Rails applications. @@ -33,7 +35,7 @@ module ActionDispatch def assert_response(type, message = nil) validate_request! - if [ :success, :missing, :redirect, :error ].include?(type) && @response.send("#{type}?") + if type.among?(:success, :missing, :redirect, :error) && @response.send("#{type}?") assert_block("") { true } # to count the assertion elsif type.is_a?(Fixnum) && @response.response_code == type assert_block("") { true } # to count the assertion diff --git a/actionpack/lib/action_dispatch/testing/assertions/selector.rb b/actionpack/lib/action_dispatch/testing/assertions/selector.rb index 2b862fb7d6..f41d3e5ddb 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/selector.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/selector.rb @@ -1,4 +1,5 @@ require 'action_controller/vendor/html-scanner' +require 'active_support/core_ext/object/inclusion' #-- # Copyright (c) 2006 Assaf Arkin (http://labnotes.org) @@ -441,7 +442,7 @@ module ActionDispatch if matches assert_block("") { true } # to count the assertion - if block_given? && !([:remove, :show, :hide, :toggle].include? rjs_type) + if block_given? && !rjs_type.among?(:remove, :show, :hide, :toggle) begin @selected ||= nil in_scope, @selected = @selected, matches diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 4706112a06..174babb9aa 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -1,6 +1,7 @@ require 'stringio' require 'uri' require 'active_support/core_ext/kernel/singleton_class' +require 'active_support/core_ext/object/inclusion' require 'active_support/core_ext/object/try' require 'rack/test' require 'test/unit/assertions' @@ -243,7 +244,8 @@ module ActionDispatch end # Performs the actual request. - def process(method, path, parameters = nil, rack_environment = nil) + def process(method, path, parameters = nil, env = nil) + env ||= {} if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme @@ -259,7 +261,7 @@ module ActionDispatch hostname, port = host.split(':') - env = { + default_env = { :method => method, :params => parameters, @@ -277,9 +279,7 @@ module ActionDispatch session = Rack::Test::Session.new(_mock_session) - (rack_environment || {}).each do |key, value| - env[key] = value - end + env.reverse_merge!(default_env) # NOTE: rack-test v0.5 doesn't build a default uri correctly # Make sure requested path is always a full uri @@ -321,7 +321,7 @@ module ActionDispatch define_method(method) do |*args| reset! unless integration_session # reset the html_document variable, but only for new get/post calls - @html_document = nil unless %w(cookies assigns).include?(method) + @html_document = nil unless method.among?("cookies", "assigns") integration_session.__send__(method, *args).tap do copy_session_variables! end diff --git a/actionpack/lib/action_view/helpers/csrf_helper.rb b/actionpack/lib/action_view/helpers/csrf_helper.rb index 65c8debc76..1f2bc28cac 100644 --- a/actionpack/lib/action_view/helpers/csrf_helper.rb +++ b/actionpack/lib/action_view/helpers/csrf_helper.rb @@ -17,10 +17,12 @@ module ActionView # Note that regular forms generate hidden fields, and that Ajax calls are whitelisted, # so they do not use these tags. def csrf_meta_tags - <<-METAS.strip_heredoc.chomp.html_safe if protect_against_forgery? - <meta name="csrf-param" content="#{Rack::Utils.escape_html(request_forgery_protection_token)}"/> - <meta name="csrf-token" content="#{Rack::Utils.escape_html(form_authenticity_token)}"/> - METAS + if protect_against_forgery? + [ + tag('meta', :name => 'csrf-param', :content => request_forgery_protection_token), + tag('meta', :name => 'csrf-token', :content => form_authenticity_token) + ].join("\n").html_safe + end end # For backwards compatibility. diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index 2d3c5fe7e7..bdda1df437 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -303,7 +303,7 @@ module ActionView # # => "Welcome to my new blog at <a href=\"http://www.myblog.com/\" target=\"_blank\">http://www.myblog.com</a>. # Please e-mail me at <a href=\"mailto:me@email.com\">me@email.com</a>." def auto_link(text, *args, &block)#link = :all, html = {}, &block) - return ''.html_safe if text.blank? + return '' if text.blank? options = args.size == 2 ? {} : args.extract_options! # this is necessary because the old auto_link API has a Hash as its last parameter unless args.empty? @@ -507,7 +507,7 @@ module ActionView end content_tag(:a, link_text, link_attributes.merge('href' => href), !!options[:sanitize]) + punctuation.reverse.join('') end - end.html_safe + end end # Turns all email addresses into clickable links. If a block is given, diff --git a/actionpack/test/controller/flash_hash_test.rb b/actionpack/test/controller/flash_hash_test.rb new file mode 100644 index 0000000000..9b69a2648e --- /dev/null +++ b/actionpack/test/controller/flash_hash_test.rb @@ -0,0 +1,90 @@ +require 'abstract_unit' + +module ActionDispatch + class FlashHashTest < ActiveSupport::TestCase + def setup + @hash = Flash::FlashHash.new + end + + def test_set_get + @hash[:foo] = 'zomg' + assert_equal 'zomg', @hash[:foo] + end + + def test_keys + assert_equal [], @hash.keys + + @hash['foo'] = 'zomg' + assert_equal ['foo'], @hash.keys + + @hash['bar'] = 'zomg' + assert_equal ['foo', 'bar'].sort, @hash.keys.sort + end + + def test_update + @hash['foo'] = 'bar' + @hash.update('foo' => 'baz', 'hello' => 'world') + + assert_equal 'baz', @hash['foo'] + assert_equal 'world', @hash['hello'] + end + + def test_delete + @hash['foo'] = 'bar' + @hash.delete 'foo' + + assert !@hash.key?('foo') + assert_nil @hash['foo'] + end + + def test_to_hash + @hash['foo'] = 'bar' + assert_equal({'foo' => 'bar'}, @hash.to_hash) + + @hash.to_hash['zomg'] = 'aaron' + assert !@hash.key?('zomg') + assert_equal({'foo' => 'bar'}, @hash.to_hash) + end + + def test_empty? + assert @hash.empty? + @hash['zomg'] = 'bears' + assert !@hash.empty? + @hash.clear + assert @hash.empty? + end + + def test_each + @hash['hello'] = 'world' + @hash['foo'] = 'bar' + + things = [] + @hash.each do |k,v| + things << [k,v] + end + + assert_equal([%w{ hello world }, %w{ foo bar }].sort, things.sort) + end + + def test_replace + @hash['hello'] = 'world' + @hash.replace('omg' => 'aaron') + assert_equal({'omg' => 'aaron'}, @hash.to_hash) + end + + def test_discard_no_args + @hash['hello'] = 'world' + @hash.discard + @hash.sweep + assert_equal({}, @hash.to_hash) + end + + def test_discard_one_arg + @hash['hello'] = 'world' + @hash['omg'] = 'world' + @hash.discard 'hello' + @hash.sweep + assert_equal({'omg' => 'world'}, @hash.to_hash) + end + end +end diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index 3569a2f213..9c89f1334d 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -174,13 +174,13 @@ class FlashTest < ActionController::TestCase assert_equal(:foo_indeed, flash.discard(:foo)) # valid key passed assert_nil flash.discard(:unknown) # non existant key passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.discard()) # nothing passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.discard(nil)) # nothing passed + assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.discard().to_hash) # nothing passed + assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.discard(nil).to_hash) # nothing passed assert_equal(:foo_indeed, flash.keep(:foo)) # valid key passed assert_nil flash.keep(:unknown) # non existant key passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.keep()) # nothing passed - assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.keep(nil)) # nothing passed + assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.keep().to_hash) # nothing passed + assert_equal({:foo => :foo_indeed, :bar => :bar_indeed}, flash.keep(nil).to_hash) # nothing passed end def test_redirect_to_with_alert @@ -214,11 +214,20 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest SessionSecret = 'b3c631c314c0bbca50c1b2843150fe33' class TestController < ActionController::Base + def dont_set_flash + head :ok + end + def set_flash flash["that"] = "hello" head :ok end + def set_flash_now + flash.now["that"] = "hello" + head :ok + end + def use_flash render :inline => "flash: #{flash["that"]}" end @@ -245,6 +254,47 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest end end + def test_setting_flash_raises_after_stream_back_to_client + with_test_route_set do + env = { 'action_dispatch.request.flash_hash' => ActionDispatch::Flash::FlashHash.new } + get '/set_flash', nil, env + assert_raise(ActionDispatch::ClosedError) { + @request.flash['alert'] = 'alert' + } + end + end + + def test_setting_flash_raises_after_stream_back_to_client_even_with_an_empty_flash + with_test_route_set do + env = { 'action_dispatch.request.flash_hash' => ActionDispatch::Flash::FlashHash.new } + get '/dont_set_flash', nil, env + assert_raise(ActionDispatch::ClosedError) { + @request.flash['alert'] = 'alert' + } + end + end + + def test_setting_flash_now_raises_after_stream_back_to_client + with_test_route_set do + env = { 'action_dispatch.request.flash_hash' => ActionDispatch::Flash::FlashHash.new } + get '/set_flash_now', nil, env + assert_raise(ActionDispatch::ClosedError) { + @request.flash.now['alert'] = 'alert' + } + end + end + + def test_setting_flash_now_raises_after_stream_back_to_client_even_with_an_empty_flash + with_test_route_set do + env = { 'action_dispatch.request.flash_hash' => ActionDispatch::Flash::FlashHash.new } + get '/dont_set_flash', nil, env + assert_raise(ActionDispatch::ClosedError) { + @request.flash.now['alert'] = 'alert' + } + end + end + + private # Overwrite get to send SessionSecret in env hash diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index f0d62b0b13..01dc2f2091 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -521,4 +521,12 @@ class ApplicationIntegrationTest < ActionDispatch::IntegrationTest get '/foo' assert_raise(NameError) { missing_path } end + + test "process reuse the env we pass as argument" do + env = { :SERVER_NAME => 'server', 'action_dispatch.custom' => 'custom' } + get '/foo', nil, env + assert_equal :get, env[:method] + assert_equal 'server', env[:SERVER_NAME] + assert_equal 'custom', env['action_dispatch.custom'] + end end diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 41f80d0784..be5866e5aa 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -1,6 +1,7 @@ require 'abstract_unit' require 'controller/fake_models' require 'active_support/core_ext/hash/conversions' +require 'active_support/core_ext/object/inclusion' class StarStarMimeController < ActionController::Base layout nil @@ -158,7 +159,7 @@ class RespondToController < ActionController::Base protected def set_layout - if ["all_types_with_layout", "iphone_with_html_response_type"].include?(action_name) + if action_name.among?("all_types_with_layout", "iphone_with_html_response_type") "respond_to/layouts/standard" elsif action_name == "iphone_with_html_response_type_without_layout" "respond_to/layouts/missing" diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index d520b5e512..31f4bf3a76 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -172,13 +172,11 @@ end class RequestForgeryProtectionControllerTest < ActionController::TestCase include RequestForgeryProtectionTests - test 'should emit a csrf-token meta tag' do + test 'should emit a csrf-param meta tag and a csrf-token meta tag' do ActiveSupport::SecureRandom.stubs(:base64).returns(@token + '<=?') get :meta - assert_equal <<-METAS.strip_heredoc.chomp, @response.body - <meta name="csrf-param" content="authenticity_token"/> - <meta name="csrf-token" content="cf50faa3fe97702ca1ae<=?"/> - METAS + assert_select 'meta[name=?][content=?]', 'csrf-param', 'authenticity_token' + assert_select 'meta[name=?][content=?]', 'csrf-token', 'cf50faa3fe97702ca1ae<=?' end end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index acb4617a60..6ea492cf8b 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -1,6 +1,7 @@ require 'abstract_unit' require 'active_support/core_ext/object/try' require 'active_support/core_ext/object/with_options' +require 'active_support/core_ext/object/inclusion' class ResourcesController < ActionController::Base def index() render :nothing => true end @@ -1292,7 +1293,7 @@ class ResourcesTest < ActionController::TestCase def assert_resource_methods(expected, resource, action_method, method) assert_equal expected.length, resource.send("#{action_method}_methods")[method].size, "#{resource.send("#{action_method}_methods")[method].inspect}" expected.each do |action| - assert resource.send("#{action_method}_methods")[method].include?(action), + assert action.in?(resource.send("#{action_method}_methods")[method]) "#{method} not in #{action_method} methods: #{resource.send("#{action_method}_methods")[method].inspect}" end end @@ -1329,9 +1330,9 @@ class ResourcesTest < ActionController::TestCase options = options.merge(:action => action.to_s) path_options = { :path => path, :method => method } - if Array(allowed).include?(action) + if action.in?(Array(allowed)) assert_recognizes options, path_options - elsif Array(not_allowed).include?(action) + elsif action.in?(Array(not_allowed)) assert_not_recognizes options, path_options end end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 39159fd629..ebc16694db 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -495,3 +495,99 @@ class CookiesTest < ActionController::TestCase end end end + +class CookiesIntegrationTest < ActionDispatch::IntegrationTest + SessionKey = '_myapp_session' + SessionSecret = 'b3c631c314c0bbca50c1b2843150fe33' + + class TestController < ActionController::Base + def dont_set_cookies + head :ok + end + + def set_cookies + cookies["that"] = "hello" + head :ok + end + end + + def test_setting_cookies_raises_after_stream_back_to_client + with_test_route_set do + get '/set_cookies' + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar['alert'] = 'alert' + cookies['alert'] = 'alert' + } + end + end + + def test_setting_cookies_raises_after_stream_back_to_client_even_without_cookies + with_test_route_set do + get '/dont_set_cookies' + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar['alert'] = 'alert' + } + end + end + + def test_setting_permanent_cookies_raises_after_stream_back_to_client + with_test_route_set do + get '/set_cookies' + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar.permanent['alert'] = 'alert' + cookies['alert'] = 'alert' + } + end + end + + def test_setting_permanent_cookies_raises_after_stream_back_to_client_even_without_cookies + with_test_route_set do + get '/dont_set_cookies' + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar.permanent['alert'] = 'alert' + } + end + end + + def test_setting_signed_cookies_raises_after_stream_back_to_client + with_test_route_set do + get '/set_cookies' + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar.signed['alert'] = 'alert' + cookies['alert'] = 'alert' + } + end + end + + def test_setting_signed_cookies_raises_after_stream_back_to_client_even_without_cookies + with_test_route_set do + get '/dont_set_cookies' + assert_raise(ActionDispatch::ClosedError) { + request.cookie_jar.signed['alert'] = 'alert' + } + end + end + + private + + # Overwrite get to send SessionSecret in env hash + def get(path, parameters = nil, env = {}) + env["action_dispatch.secret_token"] ||= SessionSecret + super + end + + def with_test_route_set + with_routing do |set| + set.draw do + match ':action', :to => CookiesIntegrationTest::TestController + end + + @app = self.class.build_app(set) do |middleware| + middleware.use ActionDispatch::Cookies + middleware.delete "ActionDispatch::ShowExceptions" + end + + yield + end + end +end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 5e5758a60e..cf7a5aaa3b 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1,6 +1,7 @@ require 'erb' require 'abstract_unit' require 'controller/fake_controllers' +require 'active_support/core_ext/object/inclusion' class TestRoutingMapper < ActionDispatch::IntegrationTest SprocketsApp = lambda { |env| @@ -495,7 +496,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest resources :todos, :id => /\d+/ end - scope '/countries/:country', :constraints => lambda { |params, req| %[all France].include?(params[:country]) } do + scope '/countries/:country', :constraints => lambda { |params, req| params[:country].among?("all", "France") } do match '/', :to => 'countries#index' match '/cities', :to => 'countries#cities' end diff --git a/actionpack/test/template/erb_util_test.rb b/actionpack/test/template/erb_util_test.rb index d1891094e8..30f6d1a213 100644 --- a/actionpack/test/template/erb_util_test.rb +++ b/actionpack/test/template/erb_util_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'active_support/core_ext/object/inclusion' class ErbUtilTest < Test::Unit::TestCase include ERB::Util @@ -29,7 +30,7 @@ class ErbUtilTest < Test::Unit::TestCase def test_rest_in_ascii (0..127).to_a.map {|int| int.chr }.each do |chr| - next if %w(& " < >).include?(chr) + next if chr.in?('&"<>') assert_equal chr, html_escape(chr) end end diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index ff183d097d..435ce81238 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -1,5 +1,6 @@ require 'abstract_unit' require 'controller/fake_models' +require 'active_support/core_ext/object/inclusion' class FormHelperTest < ActionView::TestCase tests ActionView::Helpers::FormHelper @@ -1743,7 +1744,7 @@ class FormHelperTest < ActionView::TestCase def snowman(method = nil) txt = %{<div style="margin:0;padding:0;display:inline">} txt << %{<input name="utf8" type="hidden" value="✓" />} - if (method && !['get','post'].include?(method.to_s)) + if method && !method.to_s.among?('get', 'post') txt << %{<input name="_method" type="hidden" value="#{method}" />} end txt << %{</div>} diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb index 93ff7ba0fd..76236aee41 100644 --- a/actionpack/test/template/form_options_helper_test.rb +++ b/actionpack/test/template/form_options_helper_test.rb @@ -1,5 +1,6 @@ require 'abstract_unit' require 'tzinfo' +require 'active_support/core_ext/object/inclusion' class Map < Hash def category @@ -82,7 +83,7 @@ class FormOptionsHelperTest < ActionView::TestCase def test_collection_options_with_proc_for_disabled assert_dom_equal( "<option value=\"<Abe>\"><Abe> went home</option>\n<option value=\"Babe\" disabled=\"disabled\">Babe went home</option>\n<option value=\"Cabe\" disabled=\"disabled\">Cabe went home</option>", - options_from_collection_for_select(dummy_posts, "author_name", "title", :disabled => lambda{|p| %w(Babe Cabe).include? p.author_name }) + options_from_collection_for_select(dummy_posts, "author_name", "title", :disabled => lambda{|p| p.author_name.among?("Babe", "Cabe") }) ) end diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index f8671f2980..aa2114ff79 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'active_support/core_ext/object/inclusion' class FormTagHelperTest < ActionView::TestCase tests ActionView::Helpers::FormTagHelper @@ -13,7 +14,7 @@ class FormTagHelperTest < ActionView::TestCase txt = %{<div style="margin:0;padding:0;display:inline">} txt << %{<input name="utf8" type="hidden" value="✓" />} - if (method && !['get','post'].include?(method.to_s)) + if method && !method.to_s.among?('get','post') txt << %{<input name="_method" type="hidden" value="#{method}" />} end txt << %{</div>} diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb index d0d4286393..a4fcff5167 100644 --- a/actionpack/test/template/text_helper_test.rb +++ b/actionpack/test/template/text_helper_test.rb @@ -315,14 +315,20 @@ class TextHelperTest < ActionView::TestCase end end - def test_auto_link_should_be_html_safe + def test_auto_link_should_not_be_html_safe email_raw = 'santiago@wyeworks.com' link_raw = 'http://www.rubyonrails.org' - assert auto_link(nil).html_safe? - assert auto_link('').html_safe? - assert auto_link("#{link_raw} #{link_raw} #{link_raw}").html_safe? - assert auto_link("hello #{email_raw}").html_safe? + assert !auto_link(nil).html_safe?, 'should not be html safe' + assert !auto_link('').html_safe?, 'should not be html safe' + assert !auto_link("#{link_raw} #{link_raw} #{link_raw}").html_safe?, 'should not be html safe' + assert !auto_link("hello #{email_raw}").html_safe?, 'should not be html safe' + end + + def test_auto_link_email_address + email_raw = 'aaron@tenderlovemaking.com' + email_result = %{<a href="mailto:#{email_raw}">#{email_raw}</a>} + assert !auto_link_email_addresses(email_result).html_safe?, 'should not be html safe' end def test_auto_link diff --git a/activemodel/CHANGELOG b/activemodel/CHANGELOG index 3082c7186a..03287fac7a 100644 --- a/activemodel/CHANGELOG +++ b/activemodel/CHANGELOG @@ -1,5 +1,14 @@ *Rails 3.1.0 (unreleased)* +* Add support for proc or lambda as an option for InclusionValidator, + ExclusionValidator, and FormatValidator [Prem Sichanugrist] + + You can now supply Proc, lambda, or anything that respond to #call in those + validations, and it will be called with current record as an argument. + That given proc or lambda must returns an object which respond to #include? for + InclusionValidator and ExclusionValidator, and returns a regular expression + object for FormatValidator. + * Added ActiveModel::SecurePassword to encapsulate dead-simple password usage with BCrypt encryption and salting [DHH] * ActiveModel::AttributeMethods allows attributes to be defined on demand [Alexander Uvarov] diff --git a/activemodel/lib/active_model/validations/exclusion.rb b/activemodel/lib/active_model/validations/exclusion.rb index e38e565d09..a85c23f725 100644 --- a/activemodel/lib/active_model/validations/exclusion.rb +++ b/activemodel/lib/active_model/validations/exclusion.rb @@ -1,18 +1,35 @@ +require 'active_support/core_ext/range.rb' + module ActiveModel # == Active Model Exclusion Validator module Validations class ExclusionValidator < EachValidator + ERROR_MESSAGE = "An object with the method #include? or a proc or lambda is required, " << + "and must be supplied as the :in option of the configuration hash" + def check_validity! - raise ArgumentError, "An object with the method include? is required must be supplied as the " << - ":in option of the configuration hash" unless options[:in].respond_to?(:include?) + unless [:include?, :call].any? { |method| options[:in].respond_to?(method) } + raise ArgumentError, ERROR_MESSAGE + end end def validate_each(record, attribute, value) - if options[:in].include?(value) + delimiter = options[:in] + exclusions = delimiter.respond_to?(:call) ? delimiter.call(record) : delimiter + if exclusions.send(inclusion_method(exclusions), value) record.errors.add(attribute, :exclusion, options.except(:in).merge!(:value => value)) end end + + private + + # In Ruby 1.9 <tt>Range#include?</tt> on non-numeric ranges checks all possible values in the + # range for equality, so it may be slow for large ranges. The new <tt>Range#cover?</tt> + # uses the previous logic of comparing a value with the range endpoints. + def inclusion_method(enumerable) + enumerable.is_a?(Range) ? :cover? : :include? + end end module HelperMethods @@ -22,10 +39,14 @@ module ActiveModel # validates_exclusion_of :username, :in => %w( admin superuser ), :message => "You don't belong here" # validates_exclusion_of :age, :in => 30..60, :message => "This site is only for under 30 and over 60" # validates_exclusion_of :format, :in => %w( mov avi ), :message => "extension %{value} is not allowed" + # validates_exclusion_of :password, :in => lambda { |p| [p.username, p.first_name] }, :message => "should not be the same as your username or first name" # end # # Configuration options: # * <tt>:in</tt> - An enumerable object of items that the value shouldn't be part of. + # This can be supplied as a proc or lambda which returns an enumerable. If the enumerable + # is a range the test is performed with <tt>Range#cover?</tt> + # (backported in Active Support for 1.8), otherwise with <tt>include?</tt>. # * <tt>:message</tt> - Specifies a custom error message (default is: "is reserved"). # * <tt>:allow_nil</tt> - If set to true, skips this validation if the attribute is +nil+ (default is +false+). # * <tt>:allow_blank</tt> - If set to true, skips this validation if the attribute is blank (default is +false+). diff --git a/activemodel/lib/active_model/validations/format.rb b/activemodel/lib/active_model/validations/format.rb index 541f53a834..6f23d492eb 100644 --- a/activemodel/lib/active_model/validations/format.rb +++ b/activemodel/lib/active_model/validations/format.rb @@ -4,10 +4,12 @@ module ActiveModel module Validations class FormatValidator < EachValidator def validate_each(record, attribute, value) - if options[:with] && value.to_s !~ options[:with] - record.errors.add(attribute, :invalid, options.except(:with).merge!(:value => value)) - elsif options[:without] && value.to_s =~ options[:without] - record.errors.add(attribute, :invalid, options.except(:without).merge!(:value => value)) + if options[:with] + regexp = option_call(record, :with) + record_error(record, attribute, :with, value) if value.to_s !~ regexp + elsif options[:without] + regexp = option_call(record, :without) + record_error(record, attribute, :without, value) if value.to_s =~ regexp end end @@ -16,12 +18,25 @@ module ActiveModel raise ArgumentError, "Either :with or :without must be supplied (but not both)" end - if options[:with] && !options[:with].is_a?(Regexp) - raise ArgumentError, "A regular expression must be supplied as the :with option of the configuration hash" - end + check_options_validity(options, :with) + check_options_validity(options, :without) + end + + private - if options[:without] && !options[:without].is_a?(Regexp) - raise ArgumentError, "A regular expression must be supplied as the :without option of the configuration hash" + def option_call(record, name) + option = options[name] + option.respond_to?(:call) ? option.call(record) : option + end + + def record_error(record, attribute, name, value) + record.errors.add(attribute, :invalid, options.except(name).merge!(:value => value)) + end + + def check_options_validity(options, name) + option = options[name] + if option && !option.is_a?(Regexp) && !option.respond_to?(:call) + raise ArgumentError, "A regular expression or a proc or lambda must be supplied as :#{name}" end end end @@ -40,17 +55,26 @@ module ActiveModel # validates_format_of :email, :without => /NOSPAM/ # end # + # You can also provide a proc or lambda which will determine the regular expression that will be used to validate the attribute + # + # class Person < ActiveRecord::Base + # # Admin can have number as a first letter in their screen name + # validates_format_of :screen_name, :with => lambda{ |person| person.admin? ? /\A[a-z0-9][a-z0-9_\-]*\Z/i : /\A[a-z][a-z0-9_\-]*\Z/i } + # end + # # Note: use <tt>\A</tt> and <tt>\Z</tt> to match the start and end of the string, <tt>^</tt> and <tt>$</tt> match the start/end of a line. # - # You must pass either <tt>:with</tt> or <tt>:without</tt> as an option. In addition, both must be a regular expression, - # or else an exception will be raised. + # You must pass either <tt>:with</tt> or <tt>:without</tt> as an option. In addition, both must be a regular expression + # or a proc or lambda, or else an exception will be raised. # # Configuration options: # * <tt>:message</tt> - A custom error message (default is: "is invalid"). # * <tt>:allow_nil</tt> - If set to true, skips this validation if the attribute is +nil+ (default is +false+). # * <tt>:allow_blank</tt> - If set to true, skips this validation if the attribute is blank (default is +false+). # * <tt>:with</tt> - Regular expression that if the attribute matches will result in a successful validation. + # This can be provided as a proc or lambda returning regular expression which will be called at runtime. # * <tt>:without</tt> - Regular expression that if the attribute does not match will result in a successful validation. + # This can be provided as a proc or lambda returning regular expression which will be called at runtime. # * <tt>:on</tt> - Specifies when this validation is active. Runs in all # validation contexts by default (+nil+), other options are <tt>:create</tt> # and <tt>:update</tt>. diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb index 92ac940f36..d32aebeb88 100644 --- a/activemodel/lib/active_model/validations/inclusion.rb +++ b/activemodel/lib/active_model/validations/inclusion.rb @@ -5,20 +5,30 @@ module ActiveModel # == Active Model Inclusion Validator module Validations class InclusionValidator < EachValidator + ERROR_MESSAGE = "An object with the method #include? or a proc or lambda is required, " << + "and must be supplied as the :in option of the configuration hash" + def check_validity! - raise ArgumentError, "An object with the method include? is required must be supplied as the " << - ":in option of the configuration hash" unless options[:in].respond_to?(:include?) + unless [:include?, :call].any?{ |method| options[:in].respond_to?(method) } + raise ArgumentError, ERROR_MESSAGE + end end def validate_each(record, attribute, value) - record.errors.add(attribute, :inclusion, options.except(:in).merge!(:value => value)) unless options[:in].send(include?, value) + delimiter = options[:in] + exclusions = delimiter.respond_to?(:call) ? delimiter.call(record) : delimiter + unless exclusions.send(inclusion_method(exclusions), value) + record.errors.add(attribute, :inclusion, options.except(:in).merge!(:value => value)) + end end + private + # In Ruby 1.9 <tt>Range#include?</tt> on non-numeric ranges checks all possible values in the # range for equality, so it may be slow for large ranges. The new <tt>Range#cover?</tt> # uses the previous logic of comparing a value with the range endpoints. - def include? - options[:in].is_a?(Range) ? :cover? : :include? + def inclusion_method(enumerable) + enumerable.is_a?(Range) ? :cover? : :include? end end @@ -29,11 +39,13 @@ module ActiveModel # validates_inclusion_of :gender, :in => %w( m f ) # validates_inclusion_of :age, :in => 0..99 # validates_inclusion_of :format, :in => %w( jpg gif png ), :message => "extension %{value} is not included in the list" + # validates_inclusion_of :states, :in => lambda{ |person| STATES[person.country] } # end # # Configuration options: - # * <tt>:in</tt> - An enumerable object of available items. - # If the enumerable is a range the test is performed with <tt>Range#cover?</tt> + # * <tt>:in</tt> - An enumerable object of available items. This can be + # supplied as a proc or lambda which returns an enumerable. If the enumerable + # is a range the test is performed with <tt>Range#cover?</tt> # (backported in Active Support for 1.8), otherwise with <tt>include?</tt>. # * <tt>:message</tt> - Specifies a custom error message (default is: "is not included in the list"). # * <tt>:allow_nil</tt> - If set to true, skips this validation if the attribute is +nil+ (default is +false+). diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb index c5ed8d22d3..5f7f7a6ad6 100644 --- a/activemodel/lib/active_model/validator.rb +++ b/activemodel/lib/active_model/validator.rb @@ -1,6 +1,7 @@ require 'active_support/core_ext/array/wrap' require "active_support/core_ext/module/anonymous" require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/object/inclusion' module ActiveModel #:nodoc: @@ -67,7 +68,7 @@ module ActiveModel #:nodoc: # # class TitleValidator < ActiveModel::EachValidator # def validate_each(record, attribute, value) - # record.errors[attribute] << 'must be Mr. Mrs. or Dr.' unless ['Mr.', 'Mrs.', 'Dr.'].include?(value) + # record.errors[attribute] << 'must be Mr. Mrs. or Dr.' unless value.among?('Mr.', 'Mrs.', 'Dr.') # end # end # diff --git a/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb b/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb index 015153ec7c..9a73a5ad91 100644 --- a/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb +++ b/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb @@ -1,5 +1,6 @@ require "cases/helper" require 'logger' +require 'active_support/core_ext/object/inclusion' class SanitizerTest < ActiveModel::TestCase @@ -9,7 +10,7 @@ class SanitizerTest < ActiveModel::TestCase attr_accessor :logger def deny?(key) - [ 'admin' ].include?(key) + key.in?(['admin']) end end diff --git a/activemodel/test/cases/serializeration/json_serialization_test.rb b/activemodel/test/cases/serializers/json_serialization_test.rb index 500a5c575f..500a5c575f 100644 --- a/activemodel/test/cases/serializeration/json_serialization_test.rb +++ b/activemodel/test/cases/serializers/json_serialization_test.rb diff --git a/activemodel/test/cases/serializeration/xml_serialization_test.rb b/activemodel/test/cases/serializers/xml_serialization_test.rb index b6a2f88667..b6a2f88667 100644 --- a/activemodel/test/cases/serializeration/xml_serialization_test.rb +++ b/activemodel/test/cases/serializers/xml_serialization_test.rb diff --git a/activemodel/test/cases/validations/exclusion_validation_test.rb b/activemodel/test/cases/validations/exclusion_validation_test.rb index be9d98d644..72a383f128 100644 --- a/activemodel/test/cases/validations/exclusion_validation_test.rb +++ b/activemodel/test/cases/validations/exclusion_validation_test.rb @@ -42,4 +42,16 @@ class ExclusionValidationTest < ActiveModel::TestCase ensure Person.reset_callbacks(:validate) end + + def test_validates_exclusion_of_with_lambda + Topic.validates_exclusion_of :title, :in => lambda{ |topic| topic.author_name == "sikachu" ? %w( monkey elephant ) : %w( abe wasabi ) } + + p = Topic.new + p.title = "elephant" + p.author_name = "sikachu" + assert p.invalid? + + p.title = "wasabi" + assert p.valid? + end end diff --git a/activemodel/test/cases/validations/format_validation_test.rb b/activemodel/test/cases/validations/format_validation_test.rb index 6c4fb36d52..73647efea5 100644 --- a/activemodel/test/cases/validations/format_validation_test.rb +++ b/activemodel/test/cases/validations/format_validation_test.rb @@ -98,6 +98,30 @@ class PresenceValidationTest < ActiveModel::TestCase assert_raise(ArgumentError) { Topic.validates_format_of(:title, :without => "clearly not a regexp") } end + def test_validates_format_of_with_lambda + Topic.validates_format_of :content, :with => lambda{ |topic| topic.title == "digit" ? /\A\d+\Z/ : /\A\S+\Z/ } + + p = Topic.new + p.title = "digit" + p.content = "Pixies" + assert p.invalid? + + p.content = "1234" + assert p.valid? + end + + def test_validates_format_of_without_lambda + Topic.validates_format_of :content, :without => lambda{ |topic| topic.title == "characters" ? /\A\d+\Z/ : /\A\S+\Z/ } + + p = Topic.new + p.title = "characters" + p.content = "1234" + assert p.invalid? + + p.content = "Pixies" + assert p.valid? + end + def test_validates_format_of_for_ruby_class Person.validates_format_of :karma, :with => /\A\d+\Z/ diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb index 62f2ec785d..92c473de0e 100644 --- a/activemodel/test/cases/validations/inclusion_validation_test.rb +++ b/activemodel/test/cases/validations/inclusion_validation_test.rb @@ -74,4 +74,16 @@ class InclusionValidationTest < ActiveModel::TestCase ensure Person.reset_callbacks(:validate) end + + def test_validates_inclusion_of_with_lambda + Topic.validates_inclusion_of :title, :in => lambda{ |topic| topic.author_name == "sikachu" ? %w( monkey elephant ) : %w( abe wasabi ) } + + p = Topic.new + p.title = "wasabi" + p.author_name = "sikachu" + assert p.invalid? + + p.title = "elephant" + assert p.valid? + end end diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index e536d2b408..88c2562619 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.1.0 (unreleased)* +* PostgreSQL adapter only supports PostgreSQL version 8.2 and higher. + * ConnectionManagement middleware is changed to clean up the connection pool after the rack body has been flushed. diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 27c446b12c..66f6477ec5 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -1,4 +1,5 @@ require 'active_support/core_ext/array/wrap' +require 'active_support/core_ext/object/inclusion' module ActiveRecord module Associations @@ -163,7 +164,7 @@ module ActiveRecord def creation_attributes attributes = {} - if [:has_one, :has_many].include?(reflection.macro) && !options[:through] + if reflection.macro.among?(:has_one, :has_many) && !options[:through] attributes[reflection.foreign_key] = owner[reflection.active_record_primary_key] if reflection.options[:as] diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 964e7fddc8..763b12f708 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + module ActiveRecord::Associations::Builder class BelongsTo < SingularAssociation #:nodoc: self.macro = :belongs_to @@ -65,7 +67,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless [:destroy, :delete].include?(options[:dependent]) + unless options[:dependent].among?(:destroy, :delete) raise ArgumentError, "The :dependent option expects either :destroy or :delete (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 77bb66228d..67ce339380 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + module ActiveRecord::Associations::Builder class HasMany < CollectionAssociation #:nodoc: self.macro = :has_many @@ -14,7 +16,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless [:destroy, :delete_all, :nullify, :restrict].include?(options[:dependent]) + unless options[:dependent].among?(:destroy, :delete_all, :nullify, :restrict) raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, " \ ":nullify or :restrict (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 07ba5d088e..18d7117bb7 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + module ActiveRecord::Associations::Builder class HasOne < SingularAssociation #:nodoc: self.macro = :has_one @@ -27,7 +29,7 @@ module ActiveRecord::Associations::Builder def configure_dependency if options[:dependent] - unless [:destroy, :delete, :nullify, :restrict].include?(options[:dependent]) + unless options[:dependent].among?(:destroy, :delete, :nullify, :restrict) raise ArgumentError, "The :dependent option expects either :destroy, :delete, " \ ":nullify or :restrict (#{options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 1d2e8667e4..aaa8b97389 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + module ActiveRecord # = Active Record Belongs To Has One Association module Associations @@ -50,7 +52,7 @@ module ActiveRecord end def remove_target!(method) - if [:delete, :destroy].include?(method) + if method.among?(:delete, :destroy) target.send(method) else nullify_owner_attributes(target) diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 6aac96df6f..67b0e77a25 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -1,4 +1,5 @@ require 'active_support/core_ext/class/attribute' +require 'active_support/core_ext/object/inclusion' module ActiveRecord module AttributeMethods @@ -58,7 +59,7 @@ module ActiveRecord private def create_time_zone_conversion_attribute?(name, column) - time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) && [:datetime, :timestamp].include?(column.type) + time_zone_aware_attributes && !self.skip_time_zone_conversion_for_attributes.include?(name.to_sym) && column.type.among?(:datetime, :timestamp) end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 0f44baa2fe..b0e6136e12 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -203,6 +203,10 @@ module ActiveRecord def release_savepoint end + def case_sensitive_modifier(node) + node + end + def current_savepoint_name "active_record_#{open_transactions}" end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 7bad511c64..1f7e527eeb 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -528,6 +528,11 @@ module ActiveRecord def case_sensitive_equality_operator "= BINARY" end + deprecate :case_sensitive_equality_operator + + def case_sensitive_modifier(node) + Arel::Nodes::Bin.new(node) + end def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key) where_sql @@ -587,8 +592,26 @@ module ActiveRecord # Returns an array of record hashes with the column names as keys and # column values as values. - def select(sql, name = nil) - execute(sql, name).each(:as => :hash) + def select(sql, name = nil, binds = []) + exec_query(sql, name, binds).to_a + end + + def exec_query(sql, name = 'SQL', binds = []) + @connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone + + log(sql, name, binds) do + begin + result = @connection.query(sql) + rescue ActiveRecord::StatementInvalid => exception + if exception.message.split(":").first =~ /Packets out of order/ + raise ActiveRecord::StatementInvalid, "'Packets out of order' error was received from the database. Please update your mysql bindings (gem install mysql) and read http://dev.mysql.com/doc/mysql/en/password-hashing.html for more information. If you're on Windows, use the Instant Rails installer to get the updated mysql bindings." + else + raise + end + end + + ActiveRecord::Result.new(result.fields, result.to_a) + end end def supports_views? diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index e1186209d3..d88075fdea 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -196,6 +196,7 @@ module ActiveRecord @connection_options, @config = connection_options, config @quoted_column_names, @quoted_table_names = {}, {} @statements = {} + @client_encoding = nil connect end @@ -330,6 +331,63 @@ module ActiveRecord @statements.clear end + if "<3".respond_to?(:encode) + # Taken from here: + # https://github.com/tmtm/ruby-mysql/blob/master/lib/mysql/charset.rb + # Author: TOMITA Masahiro <tommy@tmtm.org> + ENCODINGS = { + "armscii8" => nil, + "ascii" => Encoding::US_ASCII, + "big5" => Encoding::Big5, + "binary" => Encoding::ASCII_8BIT, + "cp1250" => Encoding::Windows_1250, + "cp1251" => Encoding::Windows_1251, + "cp1256" => Encoding::Windows_1256, + "cp1257" => Encoding::Windows_1257, + "cp850" => Encoding::CP850, + "cp852" => Encoding::CP852, + "cp866" => Encoding::IBM866, + "cp932" => Encoding::Windows_31J, + "dec8" => nil, + "eucjpms" => Encoding::EucJP_ms, + "euckr" => Encoding::EUC_KR, + "gb2312" => Encoding::EUC_CN, + "gbk" => Encoding::GBK, + "geostd8" => nil, + "greek" => Encoding::ISO_8859_7, + "hebrew" => Encoding::ISO_8859_8, + "hp8" => nil, + "keybcs2" => nil, + "koi8r" => Encoding::KOI8_R, + "koi8u" => Encoding::KOI8_U, + "latin1" => Encoding::ISO_8859_1, + "latin2" => Encoding::ISO_8859_2, + "latin5" => Encoding::ISO_8859_9, + "latin7" => Encoding::ISO_8859_13, + "macce" => Encoding::MacCentEuro, + "macroman" => Encoding::MacRoman, + "sjis" => Encoding::SHIFT_JIS, + "swe7" => nil, + "tis620" => Encoding::TIS_620, + "ucs2" => Encoding::UTF_16BE, + "ujis" => Encoding::EucJP_ms, + "utf8" => Encoding::UTF_8, + "utf8mb4" => Encoding::UTF_8, + } + else + ENCODINGS = Hash.new { |h,k| h[k] = k } + end + + # Get the client encoding for this database + def client_encoding + return @client_encoding if @client_encoding + + result = exec_query( + "SHOW VARIABLES WHERE Variable_name = 'character_set_client'", + 'SCHEMA') + @client_encoding = ENCODINGS[result.rows.last.last] + end + def exec_query(sql, name = 'SQL', binds = []) log(sql, name, binds) do result = nil @@ -363,6 +421,10 @@ module ActiveRecord end end + def exec_insert(sql, name, binds) + exec_query(sql, name, binds) + end + def exec_without_stmt(sql, name = 'SQL') # :nodoc: # Some queries, like SHOW CREATE TABLE don't work through the prepared # statement API. For those queries, we need to use this method. :'( @@ -506,7 +568,7 @@ module ActiveRecord def tables(name = nil, database = nil) #:nodoc: tables = [] - result = execute(["SHOW TABLES", database].compact.join(' IN '), name) + result = execute(["SHOW TABLES", database].compact.join(' IN '), 'SCHEMA') result.each { |field| tables << field[0] } result.free tables @@ -551,7 +613,7 @@ module ActiveRecord def columns(table_name, name = nil)#:nodoc: sql = "SHOW FIELDS FROM #{quote_table_name(table_name)}" columns = [] - result = execute(sql) + result = execute(sql, 'SCHEMA') result.each { |field| columns << MysqlColumn.new(field[0], field[4], field[1], field[2] == "YES") } result.free columns @@ -638,7 +700,7 @@ module ActiveRecord # Returns a table's primary key and belonging sequence. def pk_and_sequence_for(table) #:nodoc: keys = [] - result = execute("describe #{quote_table_name(table)}") + result = execute("describe #{quote_table_name(table)}", 'SCHEMA') result.each_hash do |h| keys << h["Field"]if h["Key"] == "PRI" end @@ -655,6 +717,11 @@ module ActiveRecord def case_sensitive_equality_operator "= BINARY" end + deprecate :case_sensitive_equality_operator + + def case_sensitive_modifier(node) + Arel::Nodes::Bin.new(node) + end def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key) where_sql diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 5a830a50fb..1c2cc7d5fa 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -229,7 +229,12 @@ module ActiveRecord @statements = {} connect - @local_tz = execute('SHOW TIME ZONE').first["TimeZone"] + + if postgresql_version < 80200 + raise "Your version of PostgreSQL (#{postgresql_version}) is too old, please upgrade!" + end + + @local_tz = execute('SHOW TIME ZONE', 'SCHEMA').first["TimeZone"] end def clear_cache! @@ -293,13 +298,13 @@ module ActiveRecord # 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 + execute('SET standard_conforming_strings = on', 'SCHEMA') rescue nil ensure self.client_min_messages = old end def supports_insert_with_returning? - postgresql_version >= 80200 + true end def supports_ddl_transactions? @@ -310,10 +315,9 @@ module ActiveRecord true end - # Returns the configured supported identifier length supported by PostgreSQL, - # or report the default of 63 on PostgreSQL 7.x. + # Returns the configured supported identifier length supported by PostgreSQL def table_alias_length - @table_alias_length ||= (postgresql_version >= 80000 ? query('SHOW max_identifier_length')[0][0].to_i : 63) + @table_alias_length ||= query('SHOW max_identifier_length')[0][0].to_i end # QUOTING ================================================== @@ -404,7 +408,7 @@ module ActiveRecord # REFERENTIAL INTEGRITY ==================================== def supports_disable_referential_integrity?() #:nodoc: - postgresql_version >= 80100 + true end def disable_referential_integrity #:nodoc: @@ -427,34 +431,16 @@ module ActiveRecord end # Executes an INSERT query and returns the new record's ID - def insert(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) + def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) # Extract the table from the insert sql. Yuck. - table = sql.split(" ", 4)[2].gsub('"', '') - - # Try an insert with 'returning id' if available (PG >= 8.2) - if supports_insert_with_returning? - pk, sequence_name = *pk_and_sequence_for(table) unless pk - if pk - id = select_value("#{sql} RETURNING #{quote_column_name(pk)}") - clear_query_cache - return id - end - end + _, table = extract_schema_and_table(sql.split(" ", 4)[2]) - # Otherwise, insert then grab last_insert_id. - if insert_id = super - insert_id - else - # If neither pk nor sequence name is given, look them up. - unless pk || sequence_name - pk, sequence_name = *pk_and_sequence_for(table) - end + pk ||= primary_key(table) - # If a pk is given, fallback to default sequence name. - # Don't fetch last insert id for a table without a pk. - if pk && sequence_name ||= default_sequence_name(table, pk) - last_insert_id(sequence_name) - end + if pk + select_value("#{sql} RETURNING #{quote_column_name(pk)}") + else + super end end alias :create :insert @@ -554,6 +540,10 @@ module ActiveRecord end end + def exec_insert(sql, name, binds) + exec_query(sql, name, binds) + end + # Executes an UPDATE query and returns the number of affected tuples. def update_sql(sql, name = nil) super.cmd_tuples @@ -632,20 +622,12 @@ module ActiveRecord # Example: # drop_database 'matt_development' def drop_database(name) #:nodoc: - if postgresql_version >= 80200 - execute "DROP DATABASE IF EXISTS #{quote_table_name(name)}" - else - begin - execute "DROP DATABASE #{quote_table_name(name)}" - rescue ActiveRecord::StatementInvalid - @logger.warn "#{name} database doesn't exist." if @logger - end - end + execute "DROP DATABASE IF EXISTS #{quote_table_name(name)}" end # Returns the list of all tables in the schema search path or a specified schema. def tables(name = nil) - query(<<-SQL, name).map { |row| row[0] } + query(<<-SQL, 'SCHEMA').map { |row| row[0] } SELECT tablename FROM pg_tables WHERE schemaname = ANY (current_schemas(false)) @@ -653,7 +635,21 @@ module ActiveRecord end def table_exists?(name) - name = name.to_s + schema, table = extract_schema_and_table(name.to_s) + + binds = [[nil, table.gsub(/(^"|"$)/,'')]] + binds << [nil, schema] if schema + + exec_query(<<-SQL, 'SCHEMA', binds).rows.first[0].to_i > 0 + SELECT COUNT(*) + FROM pg_tables + WHERE tablename = $1 + #{schema ? "AND schemaname = $2" : ''} + SQL + end + + # Extracts the table and schema name from +name+ + def extract_schema_and_table(name) schema, table = name.split('.', 2) unless table # A table was provided without a schema @@ -665,13 +661,7 @@ module ActiveRecord table = name schema = nil end - - query(<<-SQL).first[0].to_i > 0 - SELECT COUNT(*) - FROM pg_tables - WHERE tablename = '#{table.gsub(/(^"|"$)/,'')}' - #{schema ? "AND schemaname = '#{schema}'" : ''} - SQL + [schema, table] end # Returns the list of all indexes for a table. @@ -748,37 +738,47 @@ module ActiveRecord # Returns the current client message level. def client_min_messages - query('SHOW client_min_messages')[0][0] + query('SHOW client_min_messages', 'SCHEMA')[0][0] end # Set the client message level. def client_min_messages=(level) - execute("SET client_min_messages TO '#{level}'") + execute("SET client_min_messages TO '#{level}'", 'SCHEMA') end # Returns the sequence name for a table's primary key or some other specified key. def default_sequence_name(table_name, pk = nil) #:nodoc: - default_pk, default_seq = pk_and_sequence_for(table_name) - default_seq || "#{table_name}_#{pk || default_pk || 'id'}_seq" + serial_sequence(table_name, pk || 'id').split('.').last + rescue ActiveRecord::StatementInvalid + "#{table_name}_#{pk || 'id'}_seq" + end + + def serial_sequence(table, column) + result = exec_query(<<-eosql, 'SCHEMA', [[nil, table], [nil, column]]) + SELECT pg_get_serial_sequence($1, $2) + eosql + result.rows.first.first end # Resets the sequence of a table's primary key to the maximum value. def reset_pk_sequence!(table, pk = nil, sequence = nil) #:nodoc: unless pk and sequence default_pk, default_sequence = pk_and_sequence_for(table) + pk ||= default_pk sequence ||= default_sequence end - if pk - if sequence - quoted_sequence = quote_column_name(sequence) - select_value <<-end_sql, 'Reset sequence' - SELECT setval('#{quoted_sequence}', (SELECT COALESCE(MAX(#{quote_column_name pk})+(SELECT increment_by FROM #{quoted_sequence}), (SELECT min_value FROM #{quoted_sequence})) FROM #{quote_table_name(table)}), false) - end_sql - else - @logger.warn "#{table} has primary key #{pk} with no default sequence" if @logger - end + if @logger && pk && !sequence + @logger.warn "#{table} has primary key #{pk} with no default sequence" + end + + if pk && sequence + quoted_sequence = quote_column_name(sequence) + + select_value <<-end_sql, 'Reset sequence' + SELECT setval('#{quoted_sequence}', (SELECT COALESCE(MAX(#{quote_column_name pk})+(SELECT increment_by FROM #{quoted_sequence}), (SELECT min_value FROM #{quoted_sequence})) FROM #{quote_table_name(table)}), false) + end_sql end end @@ -786,7 +786,7 @@ module ActiveRecord def pk_and_sequence_for(table) #:nodoc: # First try looking for a sequence with a dependency on the # given table's primary key. - result = exec_query(<<-end_sql, 'PK and serial sequence').rows.first + result = exec_query(<<-end_sql, 'SCHEMA').rows.first SELECT attr.attname, seq.relname FROM pg_class seq, pg_attribute attr, @@ -803,28 +803,6 @@ module ActiveRecord AND dep.refobjid = '#{quote_table_name(table)}'::regclass end_sql - if result.nil? or result.empty? - # If that fails, try parsing the primary key's default value. - # Support the 7.x and 8.0 nextval('foo'::text) as well as - # the 8.1+ nextval('foo'::regclass). - result = query(<<-end_sql, 'PK and custom sequence')[0] - SELECT attr.attname, - CASE - WHEN split_part(def.adsrc, '''', 2) ~ '.' THEN - substr(split_part(def.adsrc, '''', 2), - strpos(split_part(def.adsrc, '''', 2), '.')+1) - ELSE split_part(def.adsrc, '''', 2) - END - FROM pg_class t - JOIN pg_attribute attr ON (t.oid = attrelid) - JOIN pg_attrdef def ON (adrelid = attrelid AND adnum = attnum) - JOIN pg_constraint cons ON (conrelid = adrelid AND adnum = conkey[1]) - WHERE t.oid = '#{quote_table_name(table)}'::regclass - AND cons.contype = 'p' - AND def.adsrc ~* 'nextval' - end_sql - end - # [primary_key, sequence] [result.first, result.last] rescue @@ -833,8 +811,21 @@ module ActiveRecord # Returns just a table's primary key def primary_key(table) - pk_and_sequence = pk_and_sequence_for(table) - pk_and_sequence && pk_and_sequence.first + row = exec_query(<<-end_sql, 'SCHEMA', [[nil, table]]).rows.first + SELECT DISTINCT(attr.attname) + FROM pg_attribute attr, + pg_depend dep, + pg_namespace name, + pg_constraint cons + WHERE attr.attrelid = dep.refobjid + AND attr.attnum = dep.refobjsubid + AND attr.attrelid = cons.conrelid + AND attr.attnum = cons.conkey[1] + AND cons.contype = 'p' + AND dep.refobjid = $1::regclass + end_sql + + row && row.first end # Renames a table. @@ -848,38 +839,14 @@ module ActiveRecord add_column_sql = "ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" add_column_options!(add_column_sql, options) - begin - execute add_column_sql - rescue ActiveRecord::StatementInvalid => e - raise e if postgresql_version > 80000 - - execute("ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}") - change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) - change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) - end + execute add_column_sql end # Changes the column of a table. def change_column(table_name, column_name, type, options = {}) quoted_table_name = quote_table_name(table_name) - begin - execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" - rescue ActiveRecord::StatementInvalid => e - raise e if postgresql_version > 80000 - # This is PostgreSQL 7.x, so we have to use a more arcane way of doing it. - begin - begin_db_transaction - tmp_column_name = "#{column_name}_ar_tmp" - add_column(table_name, tmp_column_name, type, options) - execute "UPDATE #{quoted_table_name} SET #{quote_column_name(tmp_column_name)} = CAST(#{quote_column_name(column_name)} AS #{type_to_sql(type, options[:limit], options[:precision], options[:scale])})" - remove_column(table_name, column_name) - rename_column(table_name, tmp_column_name, column_name) - commit_db_transaction - rescue - rollback_db_transaction - end - end + execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) @@ -1031,9 +998,9 @@ module ActiveRecord # If using Active Record's time zone support configure the connection to return # TIMESTAMP WITH ZONE types in UTC. if ActiveRecord::Base.default_timezone == :utc - execute("SET time zone 'UTC'") + execute("SET time zone 'UTC'", 'SCHEMA') elsif @local_tz - execute("SET time zone '#{@local_tz}'") + execute("SET time zone '#{@local_tz}'", 'SCHEMA') end end @@ -1076,7 +1043,7 @@ module ActiveRecord # - format_type includes the column size constraint, e.g. varchar(50) # - ::regclass is a function that gives the id for a table name def column_definitions(table_name) #:nodoc: - exec_query(<<-end_sql).rows + exec_query(<<-end_sql, 'SCHEMA').rows SELECT a.attname, format_type(a.atttypid, a.atttypmod), d.adsrc, a.attnotnull FROM pg_attribute a LEFT JOIN pg_attrdef d ON a.attrelid = d.adrelid AND a.attnum = d.adnum diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 32229a8410..8ef286b473 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -173,6 +173,10 @@ module ActiveRecord end end + def exec_insert(sql, name, binds) + exec_query(sql, name, binds) + end + def execute(sql, name = nil) #:nodoc: log(sql, name) { @connection.execute(sql) } end @@ -188,7 +192,8 @@ module ActiveRecord end def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) #:nodoc: - super || @connection.last_insert_row_id + super + id_value || @connection.last_insert_row_id end alias :create :insert_sql diff --git a/activerecord/lib/active_record/identity_map.rb b/activerecord/lib/active_record/identity_map.rb index d18b2b0a54..95a8e5cff7 100644 --- a/activerecord/lib/active_record/identity_map.rb +++ b/activerecord/lib/active_record/identity_map.rb @@ -50,7 +50,14 @@ module ActiveRecord def get(klass, primary_key) obj = repository[klass.symbolized_base_class][primary_key] - obj.is_a?(klass) ? obj : nil + if obj.is_a?(klass) + if ActiveRecord::Base.logger + ActiveRecord::Base.logger.debug "#{klass} with ID = #{primary_key} loaded from Identity Map" + end + obj + else + nil + end end def add(record) diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 522c0cfc9f..08b27b6a8e 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -403,12 +403,6 @@ module ActiveRecord unless reject_new_record?(association_name, attributes) association.build(attributes.except(*UNASSIGNABLE_KEYS)) end - elsif existing_records.count == 0 #Existing record but not yet associated - existing_record = self.class.reflect_on_association(association_name).klass.find(attributes['id']) - if !call_reject_if(association_name, attributes) - association.send(:add_record_to_target_with_callbacks, existing_record) if !association.loaded? - assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) - end elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s } unless association.loaded? || call_reject_if(association_name, attributes) # Make sure we are operating on the actual object which is in the association's @@ -452,6 +446,7 @@ module ActiveRecord end def call_reject_if(association_name, attributes) + return false if has_destroy_flag?(attributes) case callback = self.nested_attributes_options[association_name][:reject_if] when Symbol method(callback).arity == 0 ? send(callback) : send(callback, attributes) diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index ff36814684..944a29a3e6 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + db_namespace = namespace :db do task :load_config => :rails_env do require 'active_record' @@ -135,7 +137,7 @@ db_namespace = namespace :db do end def local_database?(config, &block) - if %w( 127.0.0.1 localhost ).include?(config['host']) || config['host'].blank? + if config['host'].among?("127.0.0.1", "localhost") || config['host'].blank? yield else $stderr.puts "This task only modifies local databases. #{config['database']} is on a remote host." diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index e801bc4afa..eac6a5dcad 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/module/deprecation' +require 'active_support/core_ext/object/inclusion' module ActiveRecord # = Active Record Reflection @@ -163,7 +164,7 @@ module ActiveRecord def initialize(macro, name, options, active_record) super - @collection = [:has_many, :has_and_belongs_to_many].include?(macro) + @collection = macro.among?(:has_many, :has_and_belongs_to_many) end # Returns a new, unsaved instance of the associated class. +options+ will diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 896daf516e..75269c85d9 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -194,6 +194,7 @@ module ActiveRecord # # The same idea applies to limit and order # Book.where('title LIKE ?', '%Rails%').order(:created_at).limit(5).update_all(:author => 'David') def update_all(updates, conditions = nil, options = {}) + IdentityMap.repository[symbolized_base_class].clear if IdentityMap.enabled? if conditions || options.present? where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates) else @@ -322,6 +323,7 @@ module ActiveRecord # If you need to destroy dependent associations or call your <tt>before_*</tt> or # +after_destroy+ callbacks, use the +destroy_all+ method instead. def delete_all(conditions = nil) + IdentityMap.repository[symbolized_base_class] = {} if IdentityMap.enabled? if conditions where(conditions).delete_all else @@ -353,6 +355,7 @@ module ActiveRecord # # Delete multiple rows # Todo.delete([2,3,4]) def delete(id_or_array) + IdentityMap.remove_by_id(self.symbolized_base_class, id_or_array) if IdentityMap.enabled? where(primary_key => id_or_array).delete_all end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 8fa315bdf3..673e47942b 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -309,6 +309,15 @@ module ActiveRecord def find_one(id) id = id.id if ActiveRecord::Base === id + if IdentityMap.enabled? && where_values.blank? && + limit_value.blank? && order_values.blank? && + includes_values.blank? && preload_values.blank? && + readonly_value.nil? && joins_values.blank? && + !@klass.locking_enabled? && + record = IdentityMap.get(@klass, id) + return record + end + column = columns_hash[primary_key] substitute = connection.substitute_for(column, @bind_values) @@ -343,8 +352,8 @@ module ActiveRecord if result.size == expected_size result else - conditions = arel.wheres.map { |x| x.value }.join(', ') - conditions = " [WHERE #{conditions}]" if conditions.present? + conditions = arel.where_sql + conditions = " [#{conditions}]" if conditions error = "Couldn't find all #{@klass.name.pluralize} with IDs " error << "(#{ids.join(", ")})#{conditions} (found #{result.size} results, but was looking for #{expected_size})" diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb index 0465b21e88..243012f88c 100644 --- a/activerecord/lib/active_record/result.rb +++ b/activerecord/lib/active_record/result.rb @@ -1,9 +1,9 @@ module ActiveRecord ### - # This class encapsulates a Result returned from calling +exec+ on any + # This class encapsulates a Result returned from calling +exec_query+ on any # database connection adapter. For example: # - # x = ActiveRecord::Base.connection.exec('SELECT * FROM foo') + # x = ActiveRecord::Base.connection.exec_query('SELECT * FROM foo') # x # => #<ActiveRecord::Result:0xdeadbeef> class Result include Enumerable diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 60d4c256c4..d363f36108 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -270,6 +270,7 @@ module ActiveRecord def rolledback!(force_restore_state = false) #:nodoc: run_callbacks :rollback ensure + IdentityMap.remove(self) if IdentityMap.enabled? restore_transaction_record_state(force_restore_state) end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 9cd6c26322..d1225a9ed9 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -14,6 +14,7 @@ module ActiveRecord def validate_each(record, attribute, value) finder_class = find_finder_class_for(record) + table = finder_class.arel_table coder = record.class.serialized_attributes[attribute.to_s] @@ -21,21 +22,15 @@ module ActiveRecord value = coder.dump value end - sql, params = mount_sql_and_params(finder_class, record.class.quoted_table_name, attribute, value) - - relation = finder_class.unscoped.where(sql, *params) + relation = build_relation(finder_class, table, attribute, value) + relation = relation.and(table[finder_class.primary_key.to_sym].not_eq(record.send(:id))) if record.persisted? Array.wrap(options[:scope]).each do |scope_item| scope_value = record.send(scope_item) - relation = relation.where(scope_item => scope_value) - end - - if record.persisted? - # TODO : This should be in Arel - relation = relation.where("#{record.class.quoted_table_name}.#{record.class.primary_key} <> ?", record.send(:id)) + relation = relation.and(table[scope_item].eq(scope_value)) end - if relation.exists? + if finder_class.unscoped.where(relation).exists? record.errors.add(attribute, :taken, options.except(:case_sensitive, :scope).merge(:value => value)) end end @@ -57,27 +52,18 @@ module ActiveRecord class_hierarchy.detect { |klass| !klass.abstract_class? } end - def mount_sql_and_params(klass, table_name, attribute, value) #:nodoc: + def build_relation(klass, table, attribute, value) #:nodoc: column = klass.columns_hash[attribute.to_s] + value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s if column.text? - operator = if value.nil? - "IS ?" - elsif column.text? - value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s - "#{klass.connection.case_sensitive_equality_operator} ?" - else - "= ?" - end - - sql_attribute = "#{table_name}.#{klass.connection.quote_column_name(attribute)}" - - if value.nil? || (options[:case_sensitive] || !column.text?) - sql = "#{sql_attribute} #{operator}" + if !options[:case_sensitive] && column.text? + relation = table[attribute].matches(value) else - sql = "LOWER(#{sql_attribute}) = LOWER(?)" + value = klass.connection.case_sensitive_modifier(value) + relation = table[attribute].eq(value) end - [sql, [value]] + relation end end diff --git a/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb b/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb index afcda2a98a..d80c8ba996 100644 --- a/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb +++ b/activerecord/lib/rails/generators/active_record/session_migration/session_migration_generator.rb @@ -1,4 +1,5 @@ require 'rails/generators/active_record' +require 'active_support/core_ext/object/inclusion' module ActiveRecord module Generators @@ -13,7 +14,7 @@ module ActiveRecord def session_table_name current_table_name = ActiveRecord::SessionStore::Session.table_name - if ["sessions", "session"].include?(current_table_name) + if current_table_name.among?("sessions", "session") current_table_name = (ActiveRecord::Base.pluralize_table_names ? 'session'.pluralize : 'session') end current_table_name diff --git a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb new file mode 100644 index 0000000000..146b77a95c --- /dev/null +++ b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb @@ -0,0 +1,69 @@ +# encoding: utf-8 + +require "cases/helper" + +module ActiveRecord + module ConnectionAdapters + class MysqlAdapterTest < ActiveRecord::TestCase + def setup + @conn = ActiveRecord::Base.connection + @conn.exec_query('drop table if exists ex') + @conn.exec_query(<<-eosql) + CREATE TABLE `ex` ( + `id` int(11) DEFAULT NULL auto_increment PRIMARY KEY, + `number` integer, + `data` varchar(255)) + eosql + end + + def test_client_encoding + if "<3".respond_to?(:encoding) + assert_equal Encoding::UTF_8, @conn.client_encoding + else + assert_equal 'utf8', @conn.client_encoding + end + end + + def test_exec_insert_number + insert(@conn, 'number' => 10) + + result = @conn.exec_query('SELECT number FROM ex WHERE number = 10') + + assert_equal 1, result.rows.length + assert_equal 10, result.rows.last.last + end + + def test_exec_insert_string + str = 'いただきます！' + insert(@conn, 'number' => 10, 'data' => str) + + result = @conn.exec_query('SELECT number, data FROM ex WHERE number = 10') + + value = result.rows.last.last + + if "<3".respond_to?(:encoding) + # FIXME: this should probably be inside the mysql AR adapter? + value.force_encoding(@conn.client_encoding) + + # The strings in this file are utf-8, so transcode to utf-8 + value.encode!(Encoding::UTF_8) + end + + assert_equal str, value + end + + private + def insert(ctx, data) + binds = data.map { |name, value| + [ctx.columns('ex').find { |x| x.name == name }, value] + } + columns = binds.map(&:first).map(&:name) + + sql = "INSERT INTO ex (#{columns.join(", ")}) + VALUES (#{(['?'] * columns.length).join(', ')})" + + ctx.exec_insert(sql, 'SQL', binds) + end + end + end +end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index b0a4a4e39d..2d412a6e2a 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -1,3 +1,4 @@ +# encoding: utf-8 require "cases/helper" module ActiveRecord @@ -6,7 +7,52 @@ module ActiveRecord def setup @connection = ActiveRecord::Base.connection @connection.exec_query('drop table if exists ex') - @connection.exec_query('create table ex(id serial primary key, data character varying(255))') + @connection.exec_query('create table ex(id serial primary key, number integer, data character varying(255))') + end + + def test_serial_sequence + assert_equal 'public.accounts_id_seq', + @connection.serial_sequence('accounts', 'id') + + assert_raises(ActiveRecord::StatementInvalid) do + @connection.serial_sequence('zomg', 'id') + end + end + + def test_default_sequence_name + assert_equal 'accounts_id_seq', + @connection.default_sequence_name('accounts', 'id') + + assert_equal 'accounts_id_seq', + @connection.default_sequence_name('accounts') + end + + def test_default_sequence_name_bad_table + assert_equal 'zomg_id_seq', + @connection.default_sequence_name('zomg', 'id') + + assert_equal 'zomg_id_seq', + @connection.default_sequence_name('zomg') + end + + def test_exec_insert_number + insert(@connection, 'number' => 10) + + result = @connection.exec_query('SELECT number FROM ex WHERE number = 10') + + assert_equal 1, result.rows.length + assert_equal "10", result.rows.last.last + end + + def test_exec_insert_string + str = 'いただきます！' + insert(@connection, 'number' => 10, 'data' => str) + + result = @connection.exec_query('SELECT number, data FROM ex WHERE number = 10') + + value = result.rows.last.last + + assert_equal str, value end def test_table_alias_length @@ -63,6 +109,21 @@ module ActiveRecord bind = @connection.substitute_for(nil, [nil]) assert_equal Arel.sql('$2'), bind end + + private + def insert(ctx, data) + binds = data.map { |name, value| + [ctx.columns('ex').find { |x| x.name == name }, value] + } + columns = binds.map(&:first).map(&:name) + + bind_subs = columns.length.times.map { |x| "$#{x + 1}" } + + sql = "INSERT INTO ex (#{columns.join(", ")}) + VALUES (#{bind_subs.join(', ')})" + + ctx.exec_insert(sql, 'SQL', binds) + end end end end diff --git a/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb b/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb deleted file mode 100644 index d1fc470907..0000000000 --- a/activerecord/test/cases/adapters/sqlite/sqlite_adapter_test.rb +++ /dev/null @@ -1,229 +0,0 @@ -# encoding: utf-8 -require "cases/helper" -require 'models/binary' - -module ActiveRecord - module ConnectionAdapters - class SQLiteAdapterTest < ActiveRecord::TestCase - class DualEncoding < ActiveRecord::Base - end - - def setup - @ctx = Base.sqlite3_connection :database => ':memory:', - :adapter => 'sqlite3', - :timeout => nil - @ctx.execute <<-eosql - CREATE TABLE items ( - id integer PRIMARY KEY AUTOINCREMENT, - number integer - ) - eosql - end - - def test_quote_binary_column_escapes_it - DualEncoding.connection.execute(<<-eosql) - CREATE TABLE dual_encodings ( - id integer PRIMARY KEY AUTOINCREMENT, - name string, - data binary - ) - eosql - str = "\x80".force_encoding("ASCII-8BIT") - binary = DualEncoding.new :name => 'いただきます！', :data => str - binary.save! - assert_equal str, binary.data - end - - def test_execute - @ctx.execute "INSERT INTO items (number) VALUES (10)" - records = @ctx.execute "SELECT * FROM items" - assert_equal 1, records.length - - record = records.first - assert_equal 10, record['number'] - assert_equal 1, record['id'] - end - - def test_quote_string - assert_equal "''", @ctx.quote_string("'") - end - - def test_insert_sql - 2.times do |i| - rv = @ctx.insert_sql "INSERT INTO items (number) VALUES (#{i})" - assert_equal(i + 1, rv) - end - - records = @ctx.execute "SELECT * FROM items" - assert_equal 2, records.length - end - - def test_insert_sql_logged - sql = "INSERT INTO items (number) VALUES (10)" - name = "foo" - - assert_logged([[sql, name]]) do - @ctx.insert_sql sql, name - end - end - - def test_insert_id_value_returned - sql = "INSERT INTO items (number) VALUES (10)" - idval = 'vuvuzela' - id = @ctx.insert_sql sql, nil, nil, idval - assert_equal idval, id - end - - def test_select_rows - 2.times do |i| - @ctx.create "INSERT INTO items (number) VALUES (#{i})" - end - rows = @ctx.select_rows 'select number, id from items' - assert_equal [[0, 1], [1, 2]], rows - end - - def test_select_rows_logged - sql = "select * from items" - name = "foo" - - assert_logged([[sql, name]]) do - @ctx.select_rows sql, name - end - end - - def test_transaction - count_sql = 'select count(*) from items' - - @ctx.begin_db_transaction - @ctx.create "INSERT INTO items (number) VALUES (10)" - - assert_equal 1, @ctx.select_rows(count_sql).first.first - @ctx.rollback_db_transaction - assert_equal 0, @ctx.select_rows(count_sql).first.first - end - - def test_tables - assert_equal %w{ items }, @ctx.tables - - @ctx.execute <<-eosql - CREATE TABLE people ( - id integer PRIMARY KEY AUTOINCREMENT, - number integer - ) - eosql - assert_equal %w{ items people }.sort, @ctx.tables.sort - end - - def test_tables_logs_name - name = "hello" - assert_logged [[name]] do - @ctx.tables(name) - assert_not_nil @ctx.logged.first.shift - end - end - - def test_columns - columns = @ctx.columns('items').sort_by { |x| x.name } - assert_equal 2, columns.length - assert_equal %w{ id number }.sort, columns.map { |x| x.name } - assert_equal [nil, nil], columns.map { |x| x.default } - assert_equal [true, true], columns.map { |x| x.null } - end - - def test_columns_with_default - @ctx.execute <<-eosql - CREATE TABLE columns_with_default ( - id integer PRIMARY KEY AUTOINCREMENT, - number integer default 10 - ) - eosql - column = @ctx.columns('columns_with_default').find { |x| - x.name == 'number' - } - assert_equal 10, column.default - end - - def test_columns_with_not_null - @ctx.execute <<-eosql - CREATE TABLE columns_with_default ( - id integer PRIMARY KEY AUTOINCREMENT, - number integer not null - ) - eosql - column = @ctx.columns('columns_with_default').find { |x| - x.name == 'number' - } - assert !column.null, "column should not be null" - end - - def test_indexes_logs - intercept_logs_on @ctx - assert_difference('@ctx.logged.length') do - @ctx.indexes('items') - end - assert_match(/items/, @ctx.logged.last.first) - end - - def test_no_indexes - assert_equal [], @ctx.indexes('items') - end - - def test_index - @ctx.add_index 'items', 'id', :unique => true, :name => 'fun' - index = @ctx.indexes('items').find { |idx| idx.name == 'fun' } - - assert_equal 'items', index.table - assert index.unique, 'index is unique' - assert_equal ['id'], index.columns - end - - def test_non_unique_index - @ctx.add_index 'items', 'id', :name => 'fun' - index = @ctx.indexes('items').find { |idx| idx.name == 'fun' } - assert !index.unique, 'index is not unique' - end - - def test_compound_index - @ctx.add_index 'items', %w{ id number }, :name => 'fun' - index = @ctx.indexes('items').find { |idx| idx.name == 'fun' } - assert_equal %w{ id number }.sort, index.columns.sort - end - - def test_primary_key - assert_equal 'id', @ctx.primary_key('items') - - @ctx.execute <<-eosql - CREATE TABLE foos ( - internet integer PRIMARY KEY AUTOINCREMENT, - number integer not null - ) - eosql - assert_equal 'internet', @ctx.primary_key('foos') - end - - def test_no_primary_key - @ctx.execute 'CREATE TABLE failboat (number integer not null)' - assert_nil @ctx.primary_key('failboat') - end - - private - - def assert_logged logs - intercept_logs_on @ctx - yield - assert_equal logs, @ctx.logged - end - - def intercept_logs_on ctx - @ctx.extend(Module.new { - attr_accessor :logged - def log sql, name - @logged << [sql, name] - yield - end - }) - @ctx.logged = [] - end - end - end -end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index b8abdface4..0e2f468908 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -1,12 +1,34 @@ +# encoding: utf-8 require "cases/helper" module ActiveRecord module ConnectionAdapters class SQLite3AdapterTest < ActiveRecord::TestCase + class DualEncoding < ActiveRecord::Base + end + def setup @conn = Base.sqlite3_connection :database => ':memory:', :adapter => 'sqlite3', :timeout => 100 + @conn.execute <<-eosql + CREATE TABLE items ( + id integer PRIMARY KEY AUTOINCREMENT, + number integer + ) + eosql + end + + def test_exec_insert + column = @conn.columns('items').find { |col| col.name == 'number' } + vals = [[column, 10]] + @conn.exec_insert('insert into items (number) VALUES (?)', 'SQL', vals) + + result = @conn.exec_query( + 'select number from items where number = ?', 'SQL', vals) + + assert_equal 1, result.rows.length + assert_equal 10, result.rows.first.first end def test_primary_key_returns_nil_for_no_pk @@ -102,6 +124,213 @@ module ActiveRecord assert_equal [[1, 'foo']], result.rows end + + def test_quote_binary_column_escapes_it + return unless "<3".respond_to?(:encode) + + DualEncoding.connection.execute(<<-eosql) + CREATE TABLE dual_encodings ( + id integer PRIMARY KEY AUTOINCREMENT, + name string, + data binary + ) + eosql + str = "\x80".force_encoding("ASCII-8BIT") + binary = DualEncoding.new :name => 'いただきます！', :data => str + binary.save! + assert_equal str, binary.data + end + + def test_execute + @conn.execute "INSERT INTO items (number) VALUES (10)" + records = @conn.execute "SELECT * FROM items" + assert_equal 1, records.length + + record = records.first + assert_equal 10, record['number'] + assert_equal 1, record['id'] + end + + def test_quote_string + assert_equal "''", @conn.quote_string("'") + end + + def test_insert_sql + 2.times do |i| + rv = @conn.insert_sql "INSERT INTO items (number) VALUES (#{i})" + assert_equal(i + 1, rv) + end + + records = @conn.execute "SELECT * FROM items" + assert_equal 2, records.length + end + + def test_insert_sql_logged + sql = "INSERT INTO items (number) VALUES (10)" + name = "foo" + + assert_logged([[sql, name, []]]) do + @conn.insert_sql sql, name + end + end + + def test_insert_id_value_returned + sql = "INSERT INTO items (number) VALUES (10)" + idval = 'vuvuzela' + id = @conn.insert_sql sql, nil, nil, idval + assert_equal idval, id + end + + def test_select_rows + 2.times do |i| + @conn.create "INSERT INTO items (number) VALUES (#{i})" + end + rows = @conn.select_rows 'select number, id from items' + assert_equal [[0, 1], [1, 2]], rows + end + + def test_select_rows_logged + sql = "select * from items" + name = "foo" + + assert_logged([[sql, name, []]]) do + @conn.select_rows sql, name + end + end + + def test_transaction + count_sql = 'select count(*) from items' + + @conn.begin_db_transaction + @conn.create "INSERT INTO items (number) VALUES (10)" + + assert_equal 1, @conn.select_rows(count_sql).first.first + @conn.rollback_db_transaction + assert_equal 0, @conn.select_rows(count_sql).first.first + end + + def test_tables + assert_equal %w{ items }, @conn.tables + + @conn.execute <<-eosql + CREATE TABLE people ( + id integer PRIMARY KEY AUTOINCREMENT, + number integer + ) + eosql + assert_equal %w{ items people }.sort, @conn.tables.sort + end + + def test_tables_logs_name + name = "hello" + assert_logged [[name, []]] do + @conn.tables(name) + assert_not_nil @conn.logged.first.shift + end + end + + def test_columns + columns = @conn.columns('items').sort_by { |x| x.name } + assert_equal 2, columns.length + assert_equal %w{ id number }.sort, columns.map { |x| x.name } + assert_equal [nil, nil], columns.map { |x| x.default } + assert_equal [true, true], columns.map { |x| x.null } + end + + def test_columns_with_default + @conn.execute <<-eosql + CREATE TABLE columns_with_default ( + id integer PRIMARY KEY AUTOINCREMENT, + number integer default 10 + ) + eosql + column = @conn.columns('columns_with_default').find { |x| + x.name == 'number' + } + assert_equal 10, column.default + end + + def test_columns_with_not_null + @conn.execute <<-eosql + CREATE TABLE columns_with_default ( + id integer PRIMARY KEY AUTOINCREMENT, + number integer not null + ) + eosql + column = @conn.columns('columns_with_default').find { |x| + x.name == 'number' + } + assert !column.null, "column should not be null" + end + + def test_indexes_logs + intercept_logs_on @conn + assert_difference('@conn.logged.length') do + @conn.indexes('items') + end + assert_match(/items/, @conn.logged.last.first) + end + + def test_no_indexes + assert_equal [], @conn.indexes('items') + end + + def test_index + @conn.add_index 'items', 'id', :unique => true, :name => 'fun' + index = @conn.indexes('items').find { |idx| idx.name == 'fun' } + + assert_equal 'items', index.table + assert index.unique, 'index is unique' + assert_equal ['id'], index.columns + end + + def test_non_unique_index + @conn.add_index 'items', 'id', :name => 'fun' + index = @conn.indexes('items').find { |idx| idx.name == 'fun' } + assert !index.unique, 'index is not unique' + end + + def test_compound_index + @conn.add_index 'items', %w{ id number }, :name => 'fun' + index = @conn.indexes('items').find { |idx| idx.name == 'fun' } + assert_equal %w{ id number }.sort, index.columns.sort + end + + def test_primary_key + assert_equal 'id', @conn.primary_key('items') + + @conn.execute <<-eosql + CREATE TABLE foos ( + internet integer PRIMARY KEY AUTOINCREMENT, + number integer not null + ) + eosql + assert_equal 'internet', @conn.primary_key('foos') + end + + def test_no_primary_key + @conn.execute 'CREATE TABLE failboat (number integer not null)' + assert_nil @conn.primary_key('failboat') + end + + private + + def assert_logged logs + intercept_logs_on @conn + yield + assert_equal logs, @conn.logged + end + + def intercept_logs_on ctx + @conn.extend(Module.new { + attr_accessor :logged + def log sql, name, binds = [] + @logged << [sql, name, binds] + yield + end + }) + @conn.logged = [] + end end end end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 5a7b139030..afc830cae9 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'active_support/core_ext/object/inclusion' require 'models/tag' require 'models/tagging' require 'models/post' @@ -453,7 +454,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert saved_post.tags.include?(new_tag) assert new_tag.persisted? - assert saved_post.reload.tags(true).include?(new_tag) + assert new_tag.in?(saved_post.reload.tags(true)) new_post = Post.new(:title => "Association replacmenet works!", :body => "You best believe it.") @@ -466,7 +467,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase new_post.save! assert new_post.persisted? - assert new_post.reload.tags(true).include?(saved_tag) + assert saved_tag.in?(new_post.reload.tags(true)) assert !posts(:thinking).tags.build.persisted? assert !posts(:thinking).tags.new.persisted? diff --git a/activerecord/test/cases/attribute_methods/read_test.rb b/activerecord/test/cases/attribute_methods/read_test.rb index d0a9028264..3641031d12 100644 --- a/activerecord/test/cases/attribute_methods/read_test.rb +++ b/activerecord/test/cases/attribute_methods/read_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'active_support/core_ext/object/inclusion' module ActiveRecord module AttributeMethods @@ -41,13 +42,13 @@ module ActiveRecord instance = @klass.new @klass.column_names.each do |name| - assert ! instance.methods.map(&:to_s).include?(name) + assert !name.in?(instance.methods.map(&:to_s)) end @klass.define_attribute_methods @klass.column_names.each do |name| - assert(instance.methods.map(&:to_s).include?(name), "#{name} is not defined") + assert name.in?(instance.methods.map(&:to_s)), "#{name} is not defined" end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 84f75cc803..a3c5c14758 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'active_support/core_ext/object/inclusion' require 'models/minimalistic' require 'models/developer' require 'models/auto_id' @@ -638,7 +639,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase end def time_related_columns_on_topic - Topic.columns.select { |c| [:time, :date, :datetime, :timestamp].include?(c.type) } + Topic.columns.select { |c| c.type.among?(:time, :date, :datetime, :timestamp) } end def serialized_columns_on_topic diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index aeb0b28bab..b3facf50b8 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -172,7 +172,7 @@ class BasicsTest < ActiveRecord::TestCase with_active_record_default_timezone :utc do time = Time.local(2000) topic = Topic.create('written_on' => time) - saved_time = Topic.find(topic.id).written_on + saved_time = Topic.find(topic.id).reload.written_on assert_equal time, saved_time assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "EST"], time.to_a assert_equal [0, 0, 5, 1, 1, 2000, 6, 1, false, "UTC"], saved_time.to_a @@ -186,7 +186,7 @@ class BasicsTest < ActiveRecord::TestCase Time.use_zone 'Central Time (US & Canada)' do time = Time.zone.local(2000) topic = Topic.create('written_on' => time) - saved_time = Topic.find(topic.id).written_on + saved_time = Topic.find(topic.id).reload.written_on assert_equal time, saved_time assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "CST"], time.to_a assert_equal [0, 0, 6, 1, 1, 2000, 6, 1, false, "UTC"], saved_time.to_a @@ -199,7 +199,7 @@ class BasicsTest < ActiveRecord::TestCase with_env_tz 'America/New_York' do time = Time.utc(2000) topic = Topic.create('written_on' => time) - saved_time = Topic.find(topic.id).written_on + saved_time = Topic.find(topic.id).reload.written_on assert_equal time, saved_time assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "UTC"], time.to_a assert_equal [0, 0, 19, 31, 12, 1999, 5, 365, false, "EST"], saved_time.to_a @@ -212,7 +212,7 @@ class BasicsTest < ActiveRecord::TestCase Time.use_zone 'Central Time (US & Canada)' do time = Time.zone.local(2000) topic = Topic.create('written_on' => time) - saved_time = Topic.find(topic.id).written_on + saved_time = Topic.find(topic.id).reload.written_on assert_equal time, saved_time assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, "CST"], time.to_a assert_equal [0, 0, 1, 1, 1, 2000, 6, 1, false, "EST"], saved_time.to_a @@ -1048,7 +1048,7 @@ class BasicsTest < ActiveRecord::TestCase topic = Topic.new(:content => myobj) assert topic.save Topic.serialize(:content, Hash) - assert_raise(ActiveRecord::SerializationTypeMismatch) { Topic.find(topic.id).content } + assert_raise(ActiveRecord::SerializationTypeMismatch) { Topic.find(topic.id).reload.content } ensure Topic.serialize(:content) end diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index deaf5252db..42b14bd25c 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'active_support/core_ext/object/inclusion' require 'models/default' require 'models/entrant' @@ -94,7 +95,7 @@ if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter) assert_equal 0, klass.columns_hash['zero'].default assert !klass.columns_hash['zero'].null # 0 in MySQL 4, nil in 5. - assert [0, nil].include?(klass.columns_hash['omit'].default) + assert klass.columns_hash['omit'].default.among?(0, nil) assert !klass.columns_hash['omit'].null assert_raise(ActiveRecord::StatementInvalid) { klass.create! } diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb index 89f7b92d09..199e59657d 100644 --- a/activerecord/test/cases/identity_map_test.rb +++ b/activerecord/test/cases/identity_map_test.rb @@ -90,6 +90,13 @@ class IdentityMapTest < ActiveRecord::TestCase ) end + def test_queries_are_not_executed_when_finding_by_id + Post.find 1 + assert_no_queries do + Post.find 1 + end + end + ############################################################################## # Tests checking if IM is functioning properly on more advanced finds # # and associations # @@ -144,7 +151,7 @@ class IdentityMapTest < ActiveRecord::TestCase s = Subscriber.find('swistak') - assert_equal({'name' => ["Raczkowski Marcin", "Swistak Sreberkowiec"]}, swistak.changes) + assert_equal({"name"=>["Marcin Raczkowski", "Swistak Sreberkowiec"]}, swistak.changes) assert_equal("Swistak Sreberkowiec", swistak.name) end @@ -159,8 +166,8 @@ class IdentityMapTest < ActiveRecord::TestCase s = Subscriber.find('swistak') assert_equal("Swistak Sreberkowiec", swistak.name) - assert_equal({}, swistak.changes) - assert !swistak.name_changed? + assert_equal({"name"=>["Marcin Raczkowski", "Swistak Sreberkowiec"]}, swistak.changes) + assert swistak.name_changed? end def test_has_many_associations @@ -381,6 +388,15 @@ class IdentityMapTest < ActiveRecord::TestCase assert_not_nil post.title end + def test_log + log = StringIO.new + ActiveRecord::Base.logger = Logger.new(log) + ActiveRecord::Base.logger.level = Logger::DEBUG + Post.find 1 + Post.find 1 + assert_match(/Post with ID = 1 loaded from Identity Map/, log.string) + end + # Currently AR is not allowing changing primary key (see Persistence#update) # So we ignore it. If this changes, this test needs to be uncommented. # def test_updating_of_pkey diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index 8ebde933b4..5f55299065 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -8,6 +8,8 @@ class LogSubscriberTest < ActiveRecord::TestCase def setup @old_logger = ActiveRecord::Base.logger + @using_identity_map = ActiveRecord::IdentityMap.enabled? + ActiveRecord::IdentityMap.enabled = false super ActiveRecord::LogSubscriber.attach_to(:active_record) end @@ -16,6 +18,7 @@ class LogSubscriberTest < ActiveRecord::TestCase super ActiveRecord::LogSubscriber.log_subscribers.pop ActiveRecord::Base.logger = @old_logger + ActiveRecord::IdentityMap.enabled = @using_identity_map end def set_logger(logger) diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index c57ab7ed28..6568eb1d18 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -131,6 +131,14 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase assert_equal 'photography', interest.reload.topic end + def test_destroy_works_independent_of_reject_if + Man.accepts_nested_attributes_for :interests, :reject_if => proc {|attributes| true }, :allow_destroy => true + man = Man.create(:name => "Jon") + interest = man.interests.create(:topic => 'the ladies') + man.update_attributes({:interests_attributes => { :_destroy => "1", :id => interest.id } }) + assert man.reload.interests.empty? + end + def test_has_many_association_updating_a_single_record Man.accepts_nested_attributes_for(:interests) man = Man.create(:name => 'John') diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 53aefc7b58..287f7e255b 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -13,7 +13,7 @@ class QueryCacheTest < ActiveRecord::TestCase end def test_find_queries - assert_queries(2) { Task.find(1); Task.find(1) } + assert_queries(ActiveRecord::IdentityMap.enabled? ? 1 : 2) { Task.find(1); Task.find(1) } end def test_find_queries_with_cache diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb index 480f2fbecb..e286362f87 100644 --- a/activeresource/lib/active_resource/connection.rb +++ b/activeresource/lib/active_resource/connection.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/benchmark' require 'active_support/core_ext/uri' +require 'active_support/core_ext/object/inclusion' require 'net/https' require 'date' require 'time' @@ -277,7 +278,7 @@ module ActiveResource def legitimize_auth_type(auth_type) return :basic if auth_type.nil? auth_type = auth_type.to_sym - [:basic, :digest].include?(auth_type) ? auth_type : :basic + auth_type.among?(:basic, :digest) ? auth_type : :basic end end end diff --git a/activeresource/lib/active_resource/http_mock.rb b/activeresource/lib/active_resource/http_mock.rb index 75649053d0..e085a05f6d 100644 --- a/activeresource/lib/active_resource/http_mock.rb +++ b/activeresource/lib/active_resource/http_mock.rb @@ -1,4 +1,5 @@ require 'active_support/core_ext/kernel/reporting' +require 'active_support/core_ext/object/inclusion' module ActiveResource class InvalidRequestError < StandardError; end #:nodoc: @@ -299,7 +300,7 @@ module ActiveResource end def success? - (200..299).include?(code) + code.in?(200..299) end def [](key) diff --git a/activeresource/test/cases/http_mock_test.rb b/activeresource/test/cases/http_mock_test.rb index 43cf5f5ef0..dfb097315e 100644 --- a/activeresource/test/cases/http_mock_test.rb +++ b/activeresource/test/cases/http_mock_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'active_support/core_ext/object/inclusion' class HttpMockTest < ActiveSupport::TestCase setup do @@ -192,7 +193,7 @@ class HttpMockTest < ActiveSupport::TestCase end def request(method, path, headers = {}, body = nil) - if [:put, :post].include? method + if method.among?(:put, :post) @http.send(method, path, body, headers) else @http.send(method, path, headers) diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 373236ce9a..eb027795a8 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.1.0 (unreleased)* +* Add Object#in? to test if an object is included in another object, and Object#among? to test if an object is included in a list of objects which will be passed as arguments. [Prem Sichanugrist, Brian Morearty, John Reitano] + * LocalCache strategy is now a real middleware class, not an anonymous class posing for pictures. diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 18182bbb40..bdef47a237 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/file/atomic' require 'active_support/core_ext/string/conversions' +require 'active_support/core_ext/object/inclusion' require 'rack/utils' module ActiveSupport @@ -20,7 +21,7 @@ module ActiveSupport end def clear(options = nil) - root_dirs = Dir.entries(cache_path).reject{|f| ['.', '..'].include?(f)} + root_dirs = Dir.entries(cache_path).reject{|f| f.among?('.', '..')} FileUtils.rm_r(root_dirs.collect{|f| File.join(cache_path, f)}) end @@ -161,7 +162,7 @@ module ActiveSupport # Delete empty directories in the cache. def delete_empty_directories(dir) return if dir == cache_path - if Dir.entries(dir).reject{|f| ['.', '..'].include?(f)}.empty? + if Dir.entries(dir).reject{|f| f.among?('.', '..')}.empty? File.delete(dir) rescue nil delete_empty_directories(File.dirname(dir)) end diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 418102352f..6a0bffb6d7 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -4,6 +4,7 @@ require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/kernel/reporting' require 'active_support/core_ext/kernel/singleton_class' +require 'active_support/core_ext/object/inclusion' module ActiveSupport # \Callbacks are code hooks that are run at key points in an object's lifecycle. @@ -412,7 +413,7 @@ module ActiveSupport # CallbackChain. # def __update_callbacks(name, filters = [], block = nil) #:nodoc: - type = [:before, :after, :around].include?(filters.first) ? filters.shift : :before + type = filters.first.among?(:before, :after, :around) ? filters.shift : :before options = filters.last.is_a?(Hash) ? filters.pop : {} filters.unshift(block) if block diff --git a/activesupport/lib/active_support/core_ext/date_time/zones.rb b/activesupport/lib/active_support/core_ext/date_time/zones.rb index 82a4f7ac5a..6fa55a9255 100644 --- a/activesupport/lib/active_support/core_ext/date_time/zones.rb +++ b/activesupport/lib/active_support/core_ext/date_time/zones.rb @@ -16,6 +16,6 @@ class DateTime def in_time_zone(zone = ::Time.zone) return self unless zone - ActiveSupport::TimeWithZone.new(utc? ? self : getutc, ::Time.__send__(:get_zone, zone)) + ActiveSupport::TimeWithZone.new(utc? ? self : getutc, ::Time.find_zone!(zone)) end end diff --git a/activesupport/lib/active_support/core_ext/object.rb b/activesupport/lib/active_support/core_ext/object.rb index 790a26f5c1..9ad1e12699 100644 --- a/activesupport/lib/active_support/core_ext/object.rb +++ b/activesupport/lib/active_support/core_ext/object.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/object/acts_like' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/object/duplicable' require 'active_support/core_ext/object/try' +require 'active_support/core_ext/object/inclusion' require 'active_support/core_ext/object/conversions' require 'active_support/core_ext/object/instance_variables' diff --git a/activesupport/lib/active_support/core_ext/object/inclusion.rb b/activesupport/lib/active_support/core_ext/object/inclusion.rb new file mode 100644 index 0000000000..cf89288aed --- /dev/null +++ b/activesupport/lib/active_support/core_ext/object/inclusion.rb @@ -0,0 +1,20 @@ +class Object + # Returns true if this object is included in the argument. Argument must be + # any object which respond to +#include?+. Usage: + # + # characters = ["Konata", "Kagami", "Tsukasa"] + # "Konata".in?(characters) # => true + # + def in?(another_object) + another_object.include?(self) + end + + # Returns true if this object is included in the argument list. Usage: + # + # username = "sikachu" + # username.among?("josevalim", "dhh", "wycats") # => false + # + def among?(*objects) + objects.include?(self) + end +end diff --git a/activesupport/lib/active_support/core_ext/time/zones.rb b/activesupport/lib/active_support/core_ext/time/zones.rb index ff90d7ca58..0c5962858e 100644 --- a/activesupport/lib/active_support/core_ext/time/zones.rb +++ b/activesupport/lib/active_support/core_ext/time/zones.rb @@ -34,29 +34,36 @@ class Time # end # end def zone=(time_zone) - Thread.current[:time_zone] = get_zone(time_zone) + Thread.current[:time_zone] = find_zone!(time_zone) end # Allows override of <tt>Time.zone</tt> locally inside supplied block; resets <tt>Time.zone</tt> to existing value when done. def use_zone(time_zone) - old_zone, ::Time.zone = ::Time.zone, get_zone(time_zone) - yield - ensure - ::Time.zone = old_zone + new_zone = find_zone!(time_zone) + begin + old_zone, ::Time.zone = ::Time.zone, new_zone + yield + ensure + ::Time.zone = old_zone + end end - private - def get_zone(time_zone) - return time_zone if time_zone.nil? || time_zone.is_a?(ActiveSupport::TimeZone) - # lookup timezone based on identifier (unless we've been passed a TZInfo::Timezone) - unless time_zone.respond_to?(:period_for_local) - time_zone = ActiveSupport::TimeZone[time_zone] || TZInfo::Timezone.get(time_zone) rescue nil - end - # Return if a TimeZone instance, or wrap in a TimeZone instance if a TZInfo::Timezone - if time_zone - time_zone.is_a?(ActiveSupport::TimeZone) ? time_zone : ActiveSupport::TimeZone.create(time_zone.name, nil, time_zone) - end + # Returns a TimeZone instance or nil, or raises an ArgumentError for invalid timezones. + def find_zone!(time_zone) + return time_zone if time_zone.nil? || time_zone.is_a?(ActiveSupport::TimeZone) + # lookup timezone based on identifier (unless we've been passed a TZInfo::Timezone) + unless time_zone.respond_to?(:period_for_local) + time_zone = ActiveSupport::TimeZone[time_zone] || TZInfo::Timezone.get(time_zone) end + # Return if a TimeZone instance, or wrap in a TimeZone instance if a TZInfo::Timezone + time_zone.is_a?(ActiveSupport::TimeZone) ? time_zone : ActiveSupport::TimeZone.create(time_zone.name, nil, time_zone) + rescue TZInfo::InvalidTimezoneIdentifier + raise ArgumentError, "Invalid Timezone: #{time_zone}" + end + + def find_zone(time_zone) + find_zone!(time_zone) rescue nil + end end # Returns the simultaneous time in <tt>Time.zone</tt>. @@ -74,6 +81,6 @@ class Time def in_time_zone(zone = ::Time.zone) return self unless zone - ActiveSupport::TimeWithZone.new(utc? ? self : getutc, ::Time.__send__(:get_zone, zone)) + ActiveSupport::TimeWithZone.new(utc? ? self : getutc, ::Time.find_zone!(zone)) end end diff --git a/activesupport/lib/active_support/railtie.rb b/activesupport/lib/active_support/railtie.rb index c2deba3b1b..04df2ea562 100644 --- a/activesupport/lib/active_support/railtie.rb +++ b/activesupport/lib/active_support/railtie.rb @@ -46,7 +46,7 @@ module ActiveSupport # If assigned value cannot be matched to a TimeZone, an exception will be raised. initializer "active_support.initialize_time_zone" do |app| require 'active_support/core_ext/time/zones' - zone_default = Time.__send__(:get_zone, app.config.time_zone) + zone_default = Time.find_zone!(app.config.time_zone) unless zone_default raise \ diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb index c66aa78ce8..6b9120e51f 100644 --- a/activesupport/lib/active_support/time_with_zone.rb +++ b/activesupport/lib/active_support/time_with_zone.rb @@ -1,5 +1,6 @@ require "active_support/values/time_zone" require 'active_support/core_ext/object/acts_like' +require 'active_support/core_ext/object/inclusion' module ActiveSupport # A Time-like class that can represent a time in any time zone. Necessary because standard Ruby Time instances are @@ -309,7 +310,7 @@ module ActiveSupport end def marshal_load(variables) - initialize(variables[0].utc, ::Time.__send__(:get_zone, variables[1]), variables[2].utc) + initialize(variables[0].utc, ::Time.find_zone(variables[1]), variables[2].utc) end # Ensure proxy class responds to all methods that underlying time instance responds to. @@ -344,7 +345,7 @@ module ActiveSupport end def duration_of_variable_length?(obj) - ActiveSupport::Duration === obj && obj.parts.any? {|p| [:years, :months, :days].include? p[0] } + ActiveSupport::Duration === obj && obj.parts.any? {|p| p[0].among?(:years, :months, :days) } end end end diff --git a/activesupport/test/core_ext/object/inclusion_test.rb b/activesupport/test/core_ext/object/inclusion_test.rb new file mode 100644 index 0000000000..25120deb81 --- /dev/null +++ b/activesupport/test/core_ext/object/inclusion_test.rb @@ -0,0 +1,52 @@ +require 'abstract_unit' +require 'active_support/core_ext/object/inclusion' + +class InTest < Test::Unit::TestCase + def test_in_array + assert 1.in?([1,2]) + assert !3.in?([1,2]) + end + + def test_in_hash + h = { "a" => 100, "b" => 200 } + assert "a".in?(h) + assert !"z".in?(h) + end + + def test_in_string + assert "lo".in?("hello") + assert !"ol".in?("hello") + assert ?h.in?("hello") + end + + def test_in_range + assert 25.in?(1..50) + assert !75.in?(1..50) + end + + def test_in_set + s = Set.new([1,2]) + assert 1.in?(s) + assert !3.in?(s) + end + + def test_either + assert 1.among?(1,2,3) + assert !5.among?(1,2,3) + assert [1,2,3].among?([1,2,3], 2, [3,4,5]) + end + + module A + end + class B + include A + end + class C < B + end + + def test_in_module + assert A.in?(B) + assert A.in?(C) + assert !A.in?(A) + end +end diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb index bafa335a09..72b55183ba 100644 --- a/activesupport/test/core_ext/time_with_zone_test.rb +++ b/activesupport/test/core_ext/time_with_zone_test.rb @@ -36,6 +36,12 @@ class TimeWithZoneTest < Test::Unit::TestCase assert_equal @twz.object_id, @twz.in_time_zone(ActiveSupport::TimeZone['Eastern Time (US & Canada)']).object_id end + def test_in_time_zone_with_bad_argument + assert_raise(ArgumentError) { @twz.in_time_zone('No such timezone exists') } + assert_raise(ArgumentError) { @twz.in_time_zone(-15.hours) } + assert_raise(ArgumentError) { @twz.in_time_zone(Object.new) } + end + def test_localtime assert_equal @twz.localtime, @twz.utc.getlocal end @@ -760,6 +766,15 @@ class TimeWithZoneMethodsForTimeAndDateTimeTest < Test::Unit::TestCase end end + def test_in_time_zone_with_invalid_argument + assert_raise(ArgumentError) { @t.in_time_zone("No such timezone exists") } + assert_raise(ArgumentError) { @dt.in_time_zone("No such timezone exists") } + assert_raise(ArgumentError) { @t.in_time_zone(-15.hours) } + assert_raise(ArgumentError) { @dt.in_time_zone(-15.hours) } + assert_raise(ArgumentError) { @t.in_time_zone(Object.new) } + assert_raise(ArgumentError) { @dt.in_time_zone(Object.new) } + end + def test_in_time_zone_with_time_local_instance with_env_tz 'US/Eastern' do time = Time.local(1999, 12, 31, 19) # == Time.utc(2000) @@ -790,6 +805,14 @@ class TimeWithZoneMethodsForTimeAndDateTimeTest < Test::Unit::TestCase assert_equal ActiveSupport::TimeZone['Alaska'], Time.zone end + def test_use_zone_raises_on_invalid_timezone + Time.zone = 'Alaska' + assert_raise ArgumentError do + Time.use_zone("No such timezone exists") { } + end + assert_equal ActiveSupport::TimeZone['Alaska'], Time.zone + end + def test_time_zone_getter_and_setter Time.zone = ActiveSupport::TimeZone['Alaska'] assert_equal ActiveSupport::TimeZone['Alaska'], Time.zone @@ -843,11 +866,27 @@ class TimeWithZoneMethodsForTimeAndDateTimeTest < Test::Unit::TestCase end def test_time_zone_setter_with_invalid_zone - Time.zone = 'foo' - assert_nil Time.zone + assert_raise(ArgumentError){ Time.zone = "No such timezone exists" } + assert_raise(ArgumentError){ Time.zone = -15.hours } + assert_raise(ArgumentError){ Time.zone = Object.new } + end + + def test_find_zone_without_bang_returns_nil_if_time_zone_can_not_be_found + assert_nil Time.find_zone('No such timezone exists') + assert_nil Time.find_zone(-15.hours) + assert_nil Time.find_zone(Object.new) + end + + def test_find_zone_with_bang_raises_if_time_zone_can_not_be_found + assert_raise(ArgumentError) { Time.find_zone!('No such timezone exists') } + assert_raise(ArgumentError) { Time.find_zone!(-15.hours) } + assert_raise(ArgumentError) { Time.find_zone!(Object.new) } + end - Time.zone = -15.hours - assert_nil Time.zone + def test_time_zone_setter_with_find_zone_without_bang + assert_nil Time.zone = Time.find_zone('No such timezone exists') + assert_nil Time.zone = Time.find_zone(-15.hours) + assert_nil Time.zone = Time.find_zone(Object.new) end def test_current_returns_time_now_when_zone_not_set diff --git a/activesupport/test/transliterate_test.rb b/activesupport/test/transliterate_test.rb index b054855d08..90b40b5478 100644 --- a/activesupport/test/transliterate_test.rb +++ b/activesupport/test/transliterate_test.rb @@ -1,6 +1,7 @@ # encoding: utf-8 require 'abstract_unit' require 'active_support/inflector/transliterate' +require 'active_support/core_ext/object/inclusion' class TransliterateTest < Test::Unit::TestCase @@ -15,7 +16,7 @@ class TransliterateTest < Test::Unit::TestCase # create string with range of Unicode"s western characters with # diacritics, excluding the division and multiplication signs which for # some reason or other are floating in the middle of all the letters. - string = (0xC0..0x17E).to_a.reject {|c| [0xD7, 0xF7].include? c}.pack("U*") + string = (0xC0..0x17E).to_a.reject {|c| c.among?(0xD7, 0xF7)}.pack("U*") string.each_char do |char| assert_match %r{^[a-zA-Z']*$}, ActiveSupport::Inflector.transliterate(string) end @@ -1,9 +1,7 @@ #!/usr/bin/env ruby -begin - require "rails/cli" -rescue LoadError +if File.exists?(File.join(File.expand_path('../..', __FILE__), '.git')) railties_path = File.expand_path('../../railties/lib', __FILE__) $:.unshift(railties_path) - require "rails/cli" end +require "rails/cli" diff --git a/railties/CHANGELOG b/railties/CHANGELOG index f159247308..90faad6131 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,5 +1,19 @@ *Rails 3.1.0 (unreleased)* +* Changed scaffold and app generator to create Ruby 1.9 style hash when running on Ruby 1.9 [Prem Sichanugrist] + + So instead of creating something like: + + redirect_to users_path, :notice => "User has been created" + + it will now be like this: + + redirect_to users_path, notice: "User has been created" + + You can also passing `--old-style-hash` to make Rails generate old style hash even you're on Ruby 1.9 + +* Changed scaffold_controller generator to create format block for JSON instead of XML [Prem Sichanugrist] + * Add using Turn with natural language test case names for test_help.rb when running with minitest (Ruby 1.9.2+) [DHH] * Direct logging of Active Record to STDOUT so it's shown inline with the results in the console [DHH] diff --git a/railties/guides/rails_guides/textile_extensions.rb b/railties/guides/rails_guides/textile_extensions.rb index affef1ccb0..9beeefb551 100644 --- a/railties/guides/rails_guides/textile_extensions.rb +++ b/railties/guides/rails_guides/textile_extensions.rb @@ -1,9 +1,11 @@ +require 'active_support/core_ext/object/inclusion' + module RailsGuides module TextileExtensions def notestuff(body) body.gsub!(/^(IMPORTANT|CAUTION|WARNING|NOTE|INFO)[.:](.*)$/) do |m| css_class = $1.downcase - css_class = 'warning' if ['caution', 'important'].include?(css_class) + css_class = 'warning' if css_class.among?('caution', 'important') result = "<div class='#{css_class}'><p>" result << $2.strip @@ -33,7 +35,7 @@ module RailsGuides def code(body) body.gsub!(%r{<(yaml|shell|ruby|erb|html|sql|plain)>(.*?)</\1>}m) do |m| es = ERB::Util.h($2) - css_class = ['erb', 'shell'].include?($1) ? 'html' : $1 + css_class = $1.among?('erb', 'shell') ? 'html' : $1 %{<notextile><div class="code_container"><code class="#{css_class}">#{es}</code></div></notextile>} end end diff --git a/railties/lib/rails/commands.rb b/railties/lib/rails/commands.rb index 02ccdf8060..7108d4157e 100644 --- a/railties/lib/rails/commands.rb +++ b/railties/lib/rails/commands.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/inclusion' + ARGV << '--help' if ARGV.empty? aliases = { @@ -69,7 +71,7 @@ when '--version', '-v' require 'rails/commands/application' else - puts "Error: Command not recognized" unless %w(-h --help).include?(command) + puts "Error: Command not recognized" unless command.among?('-h', '--help') puts <<-EOT Usage: rails COMMAND [ARGS] diff --git a/railties/lib/rails/commands/application.rb b/railties/lib/rails/commands/application.rb index 3b57b925ba..f3fa1fd54d 100644 --- a/railties/lib/rails/commands/application.rb +++ b/railties/lib/rails/commands/application.rb @@ -1,5 +1,6 @@ require 'rails/version' -if %w(--version -v).include? ARGV.first + +if ['--version', '-v'].include?(ARGV.first) puts "Rails #{Rails::VERSION::STRING}" exit(0) end diff --git a/railties/lib/rails/commands/benchmarker.rb b/railties/lib/rails/commands/benchmarker.rb index 0432261802..e10ec8fd38 100644 --- a/railties/lib/rails/commands/benchmarker.rb +++ b/railties/lib/rails/commands/benchmarker.rb @@ -1,4 +1,6 @@ -if [nil, "-h", "--help"].include?(ARGV.first) +require 'active_support/core_ext/object/inclusion' + +if ARGV.first.among?(nil, "-h", "--help") puts "Usage: rails benchmarker [times] 'Person.expensive_way' 'Person.another_expensive_way' ..." exit 1 end diff --git a/railties/lib/rails/commands/destroy.rb b/railties/lib/rails/commands/destroy.rb index db59cd8ad9..d082ba592c 100644 --- a/railties/lib/rails/commands/destroy.rb +++ b/railties/lib/rails/commands/destroy.rb @@ -1,7 +1,9 @@ require 'rails/generators' +require 'active_support/core_ext/object/inclusion' + Rails::Generators.configure! -if [nil, "-h", "--help"].include?(ARGV.first) +if ARGV.first.among?(nil, "-h", "--help") Rails::Generators.help 'destroy' exit end diff --git a/railties/lib/rails/commands/generate.rb b/railties/lib/rails/commands/generate.rb index 1b3eef504a..789e5ebedb 100644 --- a/railties/lib/rails/commands/generate.rb +++ b/railties/lib/rails/commands/generate.rb @@ -1,7 +1,9 @@ require 'rails/generators' +require 'active_support/core_ext/object/inclusion' + Rails::Generators.configure! -if [nil, "-h", "--help"].include?(ARGV.first) +if ARGV.first.among?(nil, "-h", "--help") Rails::Generators.help 'generate' exit end diff --git a/railties/lib/rails/commands/profiler.rb b/railties/lib/rails/commands/profiler.rb index 6d9717b5cd..d3195a204e 100644 --- a/railties/lib/rails/commands/profiler.rb +++ b/railties/lib/rails/commands/profiler.rb @@ -1,4 +1,6 @@ -if [nil, "-h", "--help"].include?(ARGV.first) +require 'active_support/core_ext/object/inclusion' + +if ARGV.first.among?(nil, "-h", "--help") $stderr.puts "Usage: rails profiler 'Person.expensive_method(10)' [times] [flat|graph|graph_html]" exit(1) end diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index a2eaf7a6fb..93db5106ce 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -53,6 +53,9 @@ module Rails class_option :help, :type => :boolean, :aliases => "-h", :group => :rails, :desc => "Show this help message and quit" + + class_option :old_style_hash, :type => :boolean, :default => false, + :desc => "Force using old style hash (:foo => 'bar') on Ruby >= 1.9" end def initialize(*args) @@ -174,6 +177,15 @@ module Rails create_file("#{destination}/.gitkeep") unless options[:skip_git] end + # Returns Ruby 1.9 style key-value pair if current code is running on + # Ruby 1.9.x. Returns the old-style (with hash rocket) otherwise. + def key_value(key, value) + if options[:old_style_hash] || RUBY_VERSION < '1.9' + ":#{key} => #{value}" + else + "#{key}: #{value}" + end + end end end end diff --git a/railties/lib/rails/generators/base.rb b/railties/lib/rails/generators/base.rb index dfecd2a6e4..2b0eaea3a4 100644 --- a/railties/lib/rails/generators/base.rb +++ b/railties/lib/rails/generators/base.rb @@ -8,6 +8,7 @@ rescue LoadError end require 'rails/generators/actions' +require 'active_support/core_ext/object/inclusion' module Rails module Generators @@ -164,7 +165,7 @@ module Rails names.each do |name| defaults = if options[:type] == :boolean { } - elsif [true, false].include?(default_value_for_option(name, options)) + elsif default_value_for_option(name, options).among?(true, false) { :banner => "" } else { :desc => "#{name.to_s.humanize} to be invoked", :banner => "NAME" } diff --git a/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb b/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb index 4c46db4d67..a7393cfe18 100644 --- a/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb +++ b/railties/lib/rails/generators/erb/scaffold/templates/index.html.erb @@ -17,7 +17,7 @@ <% end -%> <td><%%= link_to 'Show', <%= singular_table_name %> %></td> <td><%%= link_to 'Edit', edit_<%= singular_table_name %>_path(<%= singular_table_name %>) %></td> - <td><%%= link_to 'Destroy', <%= singular_table_name %>, :confirm => 'Are you sure?', :method => :delete %></td> + <td><%%= link_to 'Destroy', <%= singular_table_name %>, <%= key_value :confirm, "'Are you sure?'" %>, <%= key_value :method, ":delete" %> %></td> </tr> <%% end %> </table> diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb index 64273e4ca4..f85375b2a3 100644 --- a/railties/lib/rails/generators/generated_attribute.rb +++ b/railties/lib/rails/generators/generated_attribute.rb @@ -1,4 +1,5 @@ require 'active_support/time' +require 'active_support/core_ext/object/inclusion' module Rails module Generators @@ -44,7 +45,7 @@ module Rails end def reference? - [ :references, :belongs_to ].include?(self.type) + self.type.among?(:references, :belongs_to) end end end diff --git a/railties/lib/rails/generators/named_base.rb b/railties/lib/rails/generators/named_base.rb index 2af7f85463..36bc9e055c 100644 --- a/railties/lib/rails/generators/named_base.rb +++ b/railties/lib/rails/generators/named_base.rb @@ -8,6 +8,9 @@ module Rails class_option :skip_namespace, :type => :boolean, :default => false, :desc => "Skip namespace (affects only isolated applications)" + class_option :old_style_hash, :type => :boolean, :default => false, + :desc => "Force using old style hash (:foo => 'bar') on Ruby >= 1.9" + def initialize(args, *options) #:nodoc: # Unfreeze name in case it's given as a frozen string args[0] = args[0].dup if args[0].is_a?(String) && args[0].frozen? @@ -181,6 +184,16 @@ module Rails class_collisions "#{options[:prefix]}#{name}#{options[:suffix]}" end end + + # Returns Ruby 1.9 style key-value pair if current code is running on + # Ruby 1.9.x. Returns the old-style (with hash rocket) otherwise. + def key_value(key, value) + if options[:old_style_hash] || RUBY_VERSION < '1.9' + ":#{key} => #{value}" + else + "#{key}: #{value}" + end + end end end end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index c383d4842f..61daefef90 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -23,9 +23,13 @@ source 'http://rubygems.org' # Bundle gems for the local environment. Make sure to # put test-only gems in this group so their generators # and rake tasks are available in development mode: -# group :development, :test do -# gem 'webrat' -# end + +group :development, :test do + # Depend "turn" for pretty printing test output, but disable autorequire. + gem 'turn', :require => false + + # gem 'webrat' +end # Needed for guides generation # gem "RedCloth", "~> 4.2" diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/session_store.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/session_store.rb.tt index 62aa06dc3e..ddfe4ba1e1 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/session_store.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/session_store.rb.tt @@ -1,6 +1,6 @@ # Be sure to restart your server when you modify this file. -<%= app_const %>.config.session_store :cookie_store, :key => '_<%= app_name %>_session' +<%= app_const %>.config.session_store :cookie_store, <%= key_value :key, "'_#{app_name}_session'" %> # Use the database for sessions instead of the cookie-based default, # which shouldn't be used to store highly confidential information diff --git a/railties/lib/rails/generators/rails/app/templates/db/seeds.rb b/railties/lib/rails/generators/rails/app/templates/db/seeds.rb index 664d8c74c8..9a2efa68a7 100644 --- a/railties/lib/rails/generators/rails/app/templates/db/seeds.rb +++ b/railties/lib/rails/generators/rails/app/templates/db/seeds.rb @@ -3,5 +3,5 @@ # # Examples: # -# cities = City.create([{ :name => 'Chicago' }, { :name => 'Copenhagen' }]) -# Mayor.create(:name => 'Daley', :city => cities.first) +# cities = City.create([{ <%= key_value :name, "'Chicago'" %> }, { <%= key_value :name, "'Copenhagen'" %> }]) +# Mayor.create(<%= key_value :name, "'Daley'" %>, <%= key_value :city, "cities.first" %>) diff --git a/railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb b/railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb index b5317a055b..32b961d9fc 100644 --- a/railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb +++ b/railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb @@ -1,35 +1,35 @@ <% module_namespacing do -%> class <%= controller_class_name %>Controller < ApplicationController # GET <%= route_url %> - # GET <%= route_url %>.xml + # GET <%= route_url %>.json def index @<%= plural_table_name %> = <%= orm_class.all(class_name) %> respond_to do |format| format.html # index.html.erb - format.xml { render :xml => @<%= plural_table_name %> } + format.json { render <%= key_value :json, "@#{plural_table_name}" %> } end end # GET <%= route_url %>/1 - # GET <%= route_url %>/1.xml + # GET <%= route_url %>/1.json def show @<%= singular_table_name %> = <%= orm_class.find(class_name, "params[:id]") %> respond_to do |format| format.html # show.html.erb - format.xml { render :xml => @<%= singular_table_name %> } + format.json { render <%= key_value :json, "@#{singular_table_name}" %> } end end # GET <%= route_url %>/new - # GET <%= route_url %>/new.xml + # GET <%= route_url %>/new.json def new @<%= singular_table_name %> = <%= orm_class.build(class_name) %> respond_to do |format| format.html # new.html.erb - format.xml { render :xml => @<%= singular_table_name %> } + format.json { render <%= key_value :json, "@#{singular_table_name}" %> } end end @@ -39,46 +39,46 @@ class <%= controller_class_name %>Controller < ApplicationController end # POST <%= route_url %> - # POST <%= route_url %>.xml + # POST <%= route_url %>.json def create @<%= singular_table_name %> = <%= orm_class.build(class_name, "params[:#{singular_table_name}]") %> respond_to do |format| if @<%= orm_instance.save %> - format.html { redirect_to(@<%= singular_table_name %>, :notice => '<%= human_name %> was successfully created.') } - format.xml { render :xml => @<%= singular_table_name %>, :status => :created, :location => @<%= singular_table_name %> } + format.html { redirect_to @<%= singular_table_name %>, <%= key_value :notice, "'#{human_name} was successfully created.'" %> } + format.json { render <%= key_value :json, "@#{singular_table_name}" %>, <%= key_value :status, ':created' %>, <%= key_value :location, "@#{singular_table_name}" %> } else - format.html { render :action => "new" } - format.xml { render :xml => @<%= orm_instance.errors %>, :status => :unprocessable_entity } + format.html { render <%= key_value :action, '"new"' %> } + format.json { render <%= key_value :json, "@#{orm_instance.errors}" %>, <%= key_value :status, ':unprocessable_entity' %> } end end end # PUT <%= route_url %>/1 - # PUT <%= route_url %>/1.xml + # PUT <%= route_url %>/1.json def update @<%= singular_table_name %> = <%= orm_class.find(class_name, "params[:id]") %> respond_to do |format| if @<%= orm_instance.update_attributes("params[:#{singular_table_name}]") %> - format.html { redirect_to(@<%= singular_table_name %>, :notice => '<%= human_name %> was successfully updated.') } - format.xml { head :ok } + format.html { redirect_to @<%= singular_table_name %>, <%= key_value :notice, "'#{human_name} was successfully updated.'" %> } + format.json { head :ok } else - format.html { render :action => "edit" } - format.xml { render :xml => @<%= orm_instance.errors %>, :status => :unprocessable_entity } + format.html { render <%= key_value :action, '"edit"' %> } + format.json { render <%= key_value :json, "@#{orm_instance.errors}" %>, <%= key_value :status, ':unprocessable_entity' %> } end end end # DELETE <%= route_url %>/1 - # DELETE <%= route_url %>/1.xml + # DELETE <%= route_url %>/1.json def destroy @<%= singular_table_name %> = <%= orm_class.find(class_name, "params[:id]") %> @<%= orm_instance.destroy %> respond_to do |format| - format.html { redirect_to(<%= index_helper %>_url) } - format.xml { head :ok } + format.html { redirect_to <%= index_helper %>_url } + format.json { head :ok } end end end diff --git a/railties/lib/rails/generators/test_unit/scaffold/templates/functional_test.rb b/railties/lib/rails/generators/test_unit/scaffold/templates/functional_test.rb index 964d59d84c..01fe6dda7a 100644 --- a/railties/lib/rails/generators/test_unit/scaffold/templates/functional_test.rb +++ b/railties/lib/rails/generators/test_unit/scaffold/templates/functional_test.rb @@ -19,30 +19,30 @@ class <%= controller_class_name %>ControllerTest < ActionController::TestCase test "should create <%= singular_table_name %>" do assert_difference('<%= class_name %>.count') do - post :create, :<%= singular_table_name %> => @<%= singular_table_name %>.attributes + post :create, <%= key_value singular_table_name, "@#{singular_table_name}.attributes" %> end assert_redirected_to <%= singular_table_name %>_path(assigns(:<%= singular_table_name %>)) end test "should show <%= singular_table_name %>" do - get :show, :id => @<%= singular_table_name %>.to_param + get :show, <%= key_value :id, "@#{singular_table_name}.to_param" %> assert_response :success end test "should get edit" do - get :edit, :id => @<%= singular_table_name %>.to_param + get :edit, <%= key_value :id, "@#{singular_table_name}.to_param" %> assert_response :success end test "should update <%= singular_table_name %>" do - put :update, :id => @<%= singular_table_name %>.to_param, :<%= singular_table_name %> => @<%= singular_table_name %>.attributes + put :update, <%= key_value :id, "@#{singular_table_name}.to_param" %>, <%= key_value singular_table_name, "@#{singular_table_name}.attributes" %> assert_redirected_to <%= singular_table_name %>_path(assigns(:<%= singular_table_name %>)) end test "should destroy <%= singular_table_name %>" do assert_difference('<%= class_name %>.count', -1) do - delete :destroy, :id => @<%= singular_table_name %>.to_param + delete :destroy, <%= key_value :id, "@#{singular_table_name}.to_param" %> end assert_redirected_to <%= index_helper %>_path diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index b9f7bdc2eb..41485c8bac 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -14,10 +14,14 @@ if defined?(Test::Unit::Util::BacktraceFilter) && ENV['BACKTRACE'].nil? end if defined?(MiniTest) - require 'turn' + # Enable turn if it is available + begin + require 'turn' - if MiniTest::Unit.respond_to?(:use_natural_language_case_names=) - MiniTest::Unit.use_natural_language_case_names = true + if MiniTest::Unit.respond_to?(:use_natural_language_case_names=) + MiniTest::Unit.use_natural_language_case_names = true + end + rescue LoadError end end diff --git a/railties/railties.gemspec b/railties/railties.gemspec index b1eda71c7f..cd0646b8ed 100644 --- a/railties/railties.gemspec +++ b/railties/railties.gemspec @@ -21,7 +21,6 @@ Gem::Specification.new do |s| s.add_dependency('rake', '>= 0.8.7') s.add_dependency('thor', '~> 0.14.4') s.add_dependency('rack-ssl', '~> 1.3.2') - s.add_dependency('turn', '~> 0.8.2') s.add_dependency('activesupport', version) s.add_dependency('actionpack', version) end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 044fd2a278..62697b1bf9 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1,5 +1,18 @@ require "isolation/abstract_unit" +class ::MyMailInterceptor + def self.delivering_email(email); email; end +end + +class ::MyOtherMailInterceptor < ::MyMailInterceptor; end + +class ::MyMailObserver + def self.delivered_email(email); email; end +end + +class ::MyOtherMailObserver < ::MyMailObserver; end + + module ApplicationTests class ConfigurationTest < Test::Unit::TestCase include ActiveSupport::Testing::Isolation @@ -245,6 +258,80 @@ module ApplicationTests assert_equal res, last_response.body # value should be unchanged end + test "registers interceptors with ActionMailer" do + add_to_config <<-RUBY + config.action_mailer.interceptors = MyMailInterceptor + RUBY + + require "#{app_path}/config/environment" + require "mail" + + ActionMailer::Base + + assert_equal [::MyMailInterceptor], ::Mail.send(:class_variable_get, "@@delivery_interceptors") + end + + test "registers multiple interceptors with ActionMailer" do + add_to_config <<-RUBY + config.action_mailer.interceptors = [MyMailInterceptor, "MyOtherMailInterceptor"] + RUBY + + require "#{app_path}/config/environment" + require "mail" + + ActionMailer::Base + + assert_equal [::MyMailInterceptor, ::MyOtherMailInterceptor], ::Mail.send(:class_variable_get, "@@delivery_interceptors") + end + + test "registers observers with ActionMailer" do + add_to_config <<-RUBY + config.action_mailer.observers = MyMailObserver + RUBY + + require "#{app_path}/config/environment" + require "mail" + + ActionMailer::Base + + assert_equal [::MyMailObserver], ::Mail.send(:class_variable_get, "@@delivery_notification_observers") + end + + test "registers multiple observers with ActionMailer" do + add_to_config <<-RUBY + config.action_mailer.observers = [MyMailObserver, "MyOtherMailObserver"] + RUBY + + require "#{app_path}/config/environment" + require "mail" + + ActionMailer::Base + + assert_equal [::MyMailObserver, ::MyOtherMailObserver], ::Mail.send(:class_variable_get, "@@delivery_notification_observers") + end + + test "valid timezone is setup correctly" do + add_to_config <<-RUBY + config.root = "#{app_path}" + config.time_zone = "Wellington" + RUBY + + require "#{app_path}/config/environment" + + assert_equal "Wellington", Rails.application.config.time_zone + end + + test "raises when an invalid timezone is defined in the config" do + add_to_config <<-RUBY + config.root = "#{app_path}" + config.time_zone = "That big hill over yonder hill" + RUBY + + assert_raise(ArgumentError) do + require "#{app_path}/config/environment" + end + end + test "config.action_controller.perform_caching = false" do make_basic_app do |app| app.config.action_controller.perform_caching = false diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 018c2fa6bf..43f2fbd71c 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -216,6 +216,24 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_new_hash_style + run_generator [destination_root] + assert_file "config/initializers/session_store.rb" do |file| + if RUBY_VERSION < "1.9" + assert_match /config.session_store :cookie_store, :key => '_.+_session'/, file + else + assert_match /config.session_store :cookie_store, key: '_.+_session'/, file + end + end + end + + def test_force_old_style_hash + run_generator [destination_root, "--old-style-hash"] + assert_file "config/initializers/session_store.rb" do |file| + assert_match /config.session_store :cookie_store, :key => '_.+_session'/, file + end + end + protected def action(*args, &block) diff --git a/railties/test/generators/scaffold_controller_generator_test.rb b/railties/test/generators/scaffold_controller_generator_test.rb index d55ed22975..c7f45a807d 100644 --- a/railties/test/generators/scaffold_controller_generator_test.rb +++ b/railties/test/generators/scaffold_controller_generator_test.rb @@ -122,4 +122,22 @@ class ScaffoldControllerGeneratorTest < Rails::Generators::TestCase ensure Unknown::Generators.send :remove_const, :ActiveModel end + + def test_new_hash_style + run_generator + assert_file "app/controllers/users_controller.rb" do |content| + if RUBY_VERSION < "1.9" + assert_match /\{ render :action => "new" \}/, content + else + assert_match /\{ render action: "new" \}/, content + end + end + end + + def test_force_old_style_hash + run_generator ["User", "--old-style-hash"] + assert_file "app/controllers/users_controller.rb" do |content| + assert_match /\{ render :action => "new" \}/, content + end + end end |