diff options
19 files changed, 362 insertions, 160 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 6a50565de5..fb633f11d5 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Routes specifying 'to:' must be a string that contains a "#" or a rack + application. Use of a symbol should be replaced with `action: symbol`. + Use of a string without a "#" should be replaced with `controller: string`. + * Deprecate all *_filter callbacks in favor of *_action callbacks. *Rafael Mendonça França* diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 3bc578b379..a32e4ee0d1 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -7,6 +7,7 @@ require 'active_support/core_ext/module/remove_method' require 'active_support/inflector' require 'action_dispatch/routing/redirection' require 'action_dispatch/routing/endpoint' +require 'active_support/deprecation' module ActionDispatch module Routing @@ -60,32 +61,73 @@ module ActionDispatch end class Mapping #:nodoc: - IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix, :format] ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} - attr_reader :scope, :options, :requirements, :conditions, :defaults - attr_reader :to, :default_controller, :default_action - - def initialize(scope, path, options) - @scope = scope - @requirements, @conditions, @defaults = {}, {}, {} + attr_reader :requirements, :conditions, :defaults + attr_reader :to, :default_controller, :default_action, :as, :anchor + def self.build(scope, path, options) options = scope[:options].merge(options) if scope[:options] - @to = options[:to] - @default_controller = options[:controller] || scope[:controller] - @default_action = options[:action] || scope[:action] - path = normalize_path! path, options[:format] + options.delete :only + options.delete :except + options.delete :shallow_path + options.delete :shallow_prefix + options.delete :shallow + + defaults = (scope[:defaults] || {}).merge options.delete(:defaults) || {} + + new scope, path, defaults, options + end + + def initialize(scope, path, defaults, options) + @requirements, @conditions = {}, {} + @defaults = defaults + + @to = options.delete :to + @default_controller = options.delete(:controller) || scope[:controller] + @default_action = options.delete(:action) || scope[:action] + @as = options.delete :as + @anchor = options.delete :anchor + + formatted = options.delete :format + via = Array(options.delete(:via) { [] }) + options_constraints = options.delete :constraints + + path = normalize_path! path, formatted ast = path_ast path path_params = path_params ast - @options = normalize_options!(options, path_params, ast) - normalize_requirements!(path_params) - normalize_conditions!(path_params, path, ast) - normalize_defaults! + + options = normalize_options!(options, formatted, path_params, ast, scope[:module]) + + + split_constraints(path_params, scope[:constraints]) if scope[:constraints] + constraints = constraints(options, path_params) + + split_constraints path_params, constraints + + @blocks = blocks(options_constraints, scope[:blocks]) + + if options_constraints.is_a?(Hash) + split_constraints path_params, options_constraints + options_constraints.each do |key, default| + if URL_OPTIONS.include?(key) && (String === default || Fixnum === default) + @defaults[key] ||= default + end + end + end + + normalize_format!(formatted) + + @conditions[:path_info] = path + @conditions[:parsed_path_info] = ast + + add_request_method(via, @conditions) + normalize_defaults!(options) end def to_route - [ app, conditions, requirements, defaults, options[:as], options[:anchor] ] + [ app(@blocks), conditions, requirements, defaults, as, anchor ] end private @@ -106,17 +148,17 @@ module ActionDispatch format != false && !path.include?(':format') && !path.end_with?('/') end - def normalize_options!(options, path_params, path_ast) + def normalize_options!(options, formatted, path_params, path_ast, modyoule) # Add a constraint for wildcard route to make it non-greedy and match the # optional format part of the route by default - if options[:format] != false + if formatted != false path_ast.grep(Journey::Nodes::Star) do |node| options[node.name.to_sym] ||= /.+?/ end end if path_params.include?(:controller) - raise ArgumentError, ":controller segment is not allowed within a namespace block" if scope[:module] + raise ArgumentError, ":controller segment is not allowed within a namespace block" if modyoule # Add a default constraint for :controller path segments that matches namespaced # controllers with default routes like :controller/:action/:id(.:format), e.g: @@ -128,23 +170,30 @@ module ActionDispatch if to.respond_to? :call options else - options.merge!(default_controller_and_action(path_params)) + options.merge!(default_controller_and_action(path_params, modyoule)) end end - def normalize_requirements!(path_params) - constraints.each do |key, requirement| - next unless path_params.include?(key) || key == :controller - verify_regexp_requirement(requirement) if requirement.is_a?(Regexp) - @requirements[key] = requirement + def split_constraints(path_params, constraints) + constraints.each_pair do |key, requirement| + if path_params.include?(key) || key == :controller + verify_regexp_requirement(requirement) if requirement.is_a?(Regexp) + @requirements[key] = requirement + else + @conditions[key] = requirement + end end + end - if options[:format] == true + def normalize_format!(formatted) + if formatted == true @requirements[:format] ||= /.+/ - elsif Regexp === options[:format] - @requirements[:format] = options[:format] - elsif String === options[:format] - @requirements[:format] = Regexp.compile(options[:format]) + elsif Regexp === formatted + @requirements[:format] = formatted + @defaults[:format] = nil + elsif String === formatted + @requirements[:format] = Regexp.compile(formatted) + @defaults[:format] = formatted end end @@ -158,31 +207,12 @@ module ActionDispatch end end - def normalize_defaults! - @defaults.merge!(scope[:defaults]) if scope[:defaults] - @defaults.merge!(options[:defaults]) if options[:defaults] - - options.each do |key, default| - unless Regexp === default || IGNORE_OPTIONS.include?(key) + def normalize_defaults!(options) + options.each_pair do |key, default| + unless Regexp === default @defaults[key] = default end end - - if options[:constraints].is_a?(Hash) - options[:constraints].each do |key, default| - if URL_OPTIONS.include?(key) && (String === default || Fixnum === default) - @defaults[key] ||= default - end - end - elsif options[:constraints] - verify_callable_constraint(options[:constraints]) - end - - if Regexp === options[:format] - @defaults[:format] = nil - elsif String === options[:format] - @defaults[:format] = options[:format] - end end def verify_callable_constraint(callable_constraint) @@ -191,41 +221,22 @@ module ActionDispatch end end - def normalize_conditions!(path_params, path, ast) - @conditions[:path_info] = path - @conditions[:parsed_path_info] = ast - - constraints.each do |key, condition| - unless path_params.include?(key) || key == :controller - @conditions[key] = condition - end - end - - required_defaults = [] - options.each do |key, required_default| - unless path_params.include?(key) || IGNORE_OPTIONS.include?(key) || Regexp === required_default - required_defaults << key - end - end - @conditions[:required_defaults] = required_defaults - - via_all = options.delete(:via) if options[:via] == :all + def add_request_method(via, conditions) + return if via == [:all] - if !via_all && options[:via].blank? + if via.empty? msg = "You should not use the `match` method in your router without specifying an HTTP method.\n" \ "If you want to expose your action to both GET and POST, add `via: [:get, :post]` option.\n" \ "If you want to expose your action to GET, use `get` in the router:\n" \ " Instead of: match \"controller#action\"\n" \ " Do: get \"controller#action\"" - raise msg + raise ArgumentError, msg end - if via = options[:via] - @conditions[:request_method] = Array(via).map { |m| m.to_s.dasherize.upcase } - end + conditions[:request_method] = via.map { |m| m.to_s.dasherize.upcase } end - def app + def app(blocks) return to if Redirect === to if to.respond_to?(:call) @@ -239,11 +250,11 @@ module ActionDispatch end end - def default_controller_and_action(path_params) + def default_controller_and_action(path_params, modyoule) controller, action = get_controller_and_action(default_controller, default_action, to, - @scope[:module] + modyoule ) hash = check_part(:controller, controller, path_params, {}) do |part| @@ -274,9 +285,13 @@ module ActionDispatch def get_controller_and_action(controller, action, to, modyoule) case to - when Symbol then action = to.to_s + when Symbol + ActiveSupport::Deprecation.warn "defining a route where `to` is a symbol is deprecated. Please change \"to: :#{to}\" to \"action: :#{to}\"" + action = to.to_s when /#/ then controller, action = to.split('#') - when String then controller = to + when String + ActiveSupport::Deprecation.warn "defining a route where `to` is a controller without an action is deprecated. Please change \"to: :#{to}\" to \"controller: :#{to}\"" + controller = to end if modyoule && !controller.is_a?(Regexp) @@ -296,24 +311,27 @@ module ActionDispatch yield end - def blocks - if options[:constraints].present? && !options[:constraints].is_a?(Hash) - [options[:constraints]] + def blocks(options_constraints, scope_blocks) + if options_constraints && !options_constraints.is_a?(Hash) + verify_callable_constraint(options_constraints) + [options_constraints] else - scope[:blocks] || [] + scope_blocks || [] end end - def constraints - @constraints ||= {}.tap do |constraints| - constraints.merge!(scope[:constraints]) if scope[:constraints] - - options.except(*IGNORE_OPTIONS).each do |key, option| - constraints[key] = option if Regexp === option + def constraints(options, path_params) + constraints = {} + required_defaults = [] + options.each_pair do |key, option| + if Regexp === option + constraints[key] = option + else + required_defaults << key unless path_params.include?(key) end - - constraints.merge!(options[:constraints]) if options[:constraints].is_a?(Hash) end + @conditions[:required_defaults] = required_defaults + constraints end def path_params(ast) @@ -1445,7 +1463,20 @@ module ActionDispatch if rest.empty? && Hash === path options = path path, to = options.find { |name, _value| name.is_a?(String) } - options[:to] = to + + case to + when Symbol + options[:action] = to + when String + if to =~ /#/ + options[:to] = to + else + options[:controller] = to + end + else + options[:to] = to + end + options.delete(path) paths = [path] else @@ -1515,7 +1546,7 @@ module ActionDispatch options[:as] = name_for_action(options[:as], action) end - mapping = Mapping.new(@scope, URI.parser.escape(path), options) + mapping = Mapping.build(@scope, URI.parser.escape(path), options) app, conditions, requirements, defaults, as, anchor = mapping.to_route @set.add_route(app, conditions, requirements, defaults, as, anchor) end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 9dc6d77012..660589a86e 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -243,6 +243,33 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal 'clients', get(URI('http://clients.example.org/')) end + def test_scoped_lambda + scope_called = false + rs.draw do + scope '/foo', :constraints => lambda { |req| scope_called = true } do + get '/', :to => lambda { |env| [200, {}, %w{default}] } + end + end + + assert_equal 'default', get(URI('http://www.example.org/foo/')) + assert scope_called, "scope constraint should be called" + end + + def test_scoped_lambda_with_get_lambda + scope_called = false + inner_called = false + + rs.draw do + scope '/foo', :constraints => lambda { |req| flunk "should not be called" } do + get '/', :constraints => lambda { |req| inner_called = true }, + :to => lambda { |env| [200, {}, %w{default}] } + end + end + + assert_equal 'default', get(URI('http://www.example.org/foo/')) + assert inner_called, "inner constraint should be called" + end + def test_empty_string_match rs.draw do get '/:username', :constraints => { :username => /[^\/]+/ }, diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index bf82e09f39..d8d3209dac 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -38,7 +38,7 @@ module ActionDispatch def test_mapping_requirements options = { :controller => 'foo', :action => 'bar', :via => :get } - m = Mapper::Mapping.new({}, '/store/:name(*rest)', options) + m = Mapper::Mapping.build({}, '/store/:name(*rest)', options) _, _, requirements, _ = m.to_route assert_equal(/.+?/, requirements[:rest]) end diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 2a2f92b5b3..2db3fee6bb 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -145,7 +145,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest test "does not raise EOFError on GET request with multipart content-type" do with_routing do |set| set.draw do - get ':action', to: 'multipart_params_parsing_test/test' + get ':action', controller: 'multipart_params_parsing_test/test' end headers = { "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x" } get "/parse", {}, headers @@ -174,7 +174,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest def with_test_routing with_routing do |set| set.draw do - post ':action', :to => 'multipart_params_parsing_test/test' + post ':action', :controller => 'multipart_params_parsing_test/test' end yield end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index d6477e19bb..778dbfc74d 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -361,8 +361,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest draw do controller(:global) do get 'global/hide_notice' - get 'global/export', :to => :export, :as => :export_request - get '/export/:id/:file', :to => :export, :as => :export_download, :constraints => { :file => /.*/ } + get 'global/export', :action => :export, :as => :export_request + get '/export/:id/:file', :action => :export, :as => :export_download, :constraints => { :file => /.*/ } get 'global/:action' end end @@ -730,8 +730,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest draw do resources :replies do member do - put :answer, :to => :mark_as_answer - delete :answer, :to => :unmark_as_answer + put :answer, :action => :mark_as_answer + delete :answer, :action => :unmark_as_answer end end end @@ -1188,7 +1188,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest controller :articles do scope '/articles', :as => 'article' do scope :path => '/:title', :title => /[a-z]+/, :as => :with_title do - get '/:id', :to => :with_id, :as => "" + get '/:id', :action => :with_id, :as => "" end end end @@ -1435,7 +1435,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_scoped_controller_with_namespace_and_action draw do namespace :account do - get ':action/callback', :action => /twitter|github/, :to => "callbacks", :as => :callback + get ':action/callback', :action => /twitter|github/, :controller => "callbacks", :as => :callback end end @@ -1492,7 +1492,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_normalize_namespaced_matches draw do namespace :account do - get 'description', :to => :description, :as => "description" + get 'description', :action => :description, :as => "description" end end @@ -2154,7 +2154,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end resources :invoices do get "outstanding" => "invoices#outstanding", :on => :collection - get "overdue", :to => :overdue, :on => :collection + get "overdue", :action => :overdue, :on => :collection get "print" => "invoices#print", :as => :print, :on => :member post "preview" => "invoices#preview", :as => :preview, :on => :new end @@ -2242,6 +2242,22 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/api/1.0/users/first.last.xml', api_user_path(:version => '1.0', :id => 'first.last', :format => :xml) end + def test_match_without_via + assert_raises(ArgumentError) do + draw do + match '/foo/bar', :to => 'files#show' + end + end + end + + def test_match_with_empty_via + assert_raises(ArgumentError) do + draw do + match '/foo/bar', :to => 'files#show', :via => [] + end + end + end + def test_glob_parameter_accepts_regexp draw do get '/:locale/*file.:format', :to => 'files#show', :file => /path\/to\/existing\/file/ @@ -2980,7 +2996,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end assert_raise(ArgumentError) do - draw { controller("/feeds") { get '/feeds/:service', :to => :show } } + assert_deprecated do + draw { controller("/feeds") { get '/feeds/:service', :to => :show } } + end end assert_raise(ArgumentError) do @@ -3239,6 +3257,58 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/admin/posts/1/comments', admin_post_comments_path('1') end + def test_mix_string_to_controller_action + draw do + get '/projects', controller: 'project_files', + action: 'index', + to: 'comments#index' + end + get '/projects' + assert_equal 'comments#index', @response.body + end + + def test_mix_string_to_controller + draw do + get '/projects', controller: 'project_files', + to: 'comments#index' + end + get '/projects' + assert_equal 'comments#index', @response.body + end + + def test_mix_string_to_action + draw do + get '/projects', action: 'index', + to: 'comments#index' + end + get '/projects' + assert_equal 'comments#index', @response.body + end + + def test_mix_symbol_to_controller_action + assert_deprecated do + draw do + get '/projects', controller: 'project_files', + action: 'index', + to: :show + end + end + get '/projects' + assert_equal 'project_files#show', @response.body + end + + def test_mix_string_to_controller_action_no_hash + assert_deprecated do + draw do + get '/projects', controller: 'project_files', + action: 'index', + to: 'show' + end + end + get '/projects' + assert_equal 'show#index', @response.body + end + def test_shallow_path_and_prefix_are_not_added_to_non_shallow_routes draw do scope shallow_path: 'projects', shallow_prefix: 'project' do @@ -3503,7 +3573,7 @@ class TestNamespaceWithControllerOption < ActionDispatch::IntegrationTest def test_missing_controller ex = assert_raises(ArgumentError) { draw do - get '/foo/bar', :to => :index + get '/foo/bar', :action => :index end } assert_match(/Missing :controller/, ex.message) @@ -3511,8 +3581,10 @@ class TestNamespaceWithControllerOption < ActionDispatch::IntegrationTest def test_missing_action ex = assert_raises(ArgumentError) { - draw do - get '/foo/bar', :to => 'foo' + assert_deprecated do + draw do + get '/foo/bar', :to => 'foo' + end end } assert_match(/Missing :action/, ex.message) @@ -4019,7 +4091,7 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest set.draw do get "/bar/:id", :to => redirect("/foo/show/%{id}") get "/foo/show(/:id)", :to => "test_invalid_urls/foo#show" - get "/foo(/:action(/:id))", :to => "test_invalid_urls/foo" + get "/foo(/:action(/:id))", :controller => "test_invalid_urls/foo" get "/:controller(/:action(/:id))" end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index f836e60988..04ae67234f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -18,21 +18,7 @@ module ActiveRecord value = column.type_cast_for_database(value) end - case value - when String, ActiveSupport::Multibyte::Chars - "'#{quote_string(value.to_s)}'" - when true then quoted_true - when false then quoted_false - when nil then "NULL" - # BigDecimals need to be put in a non-normalized form and quoted. - when BigDecimal then value.to_s('F') - when Numeric, ActiveSupport::Duration then value.to_s - when Date, Time then "'#{quoted_date(value)}'" - when Symbol then "'#{quote_string(value.to_s)}'" - when Class then "'#{value.to_s}'" - else - "'#{quote_string(YAML.dump(value))}'" - end + _quote(value) end # Cast a +value+ to a type that the database understands. For example, @@ -52,20 +38,10 @@ module ActiveRecord value = column.type_cast_for_database(value) end - case value - when Symbol, ActiveSupport::Multibyte::Chars - value.to_s - when true then unquoted_true - when false then unquoted_false - # BigDecimals need to be put in a non-normalized form and quoted. - when BigDecimal then value.to_s('F') - when Date, Time then quoted_date(value) - when *types_which_need_no_typecasting - value - else - to_type = column ? " to #{column.type}" : "" - raise TypeError, "can't cast #{value.class}#{to_type}" - end + _type_cast(value) + rescue TypeError + to_type = column ? " to #{column.type}" : "" + raise TypeError, "can't cast #{value.class}#{to_type}" end # Quotes a string, escaping any ' (single quote) and \ (backslash) @@ -129,6 +105,39 @@ module ActiveRecord def types_which_need_no_typecasting [nil, Numeric, String] end + + def _quote(value) + case value + when String, ActiveSupport::Multibyte::Chars, Type::Binary::Data + "'#{quote_string(value.to_s)}'" + when true then quoted_true + when false then quoted_false + when nil then "NULL" + # BigDecimals need to be put in a non-normalized form and quoted. + when BigDecimal then value.to_s('F') + when Numeric, ActiveSupport::Duration then value.to_s + when Date, Time then "'#{quoted_date(value)}'" + when Symbol then "'#{quote_string(value.to_s)}'" + when Class then "'#{value.to_s}'" + else + "'#{quote_string(YAML.dump(value))}'" + end + end + + def _type_cast(value) + case value + when Symbol, ActiveSupport::Multibyte::Chars, Type::Binary::Data + value.to_s + when true then unquoted_true + when false then unquoted_false + # BigDecimals need to be put in a non-normalized form and quoted. + when BigDecimal then value.to_s('F') + when Date, Time then quoted_date(value) + when *types_which_need_no_typecasting + value + else raise TypeError + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 2677b6ee83..759ac9943f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -218,10 +218,9 @@ module ActiveRecord # QUOTING ================================================== - def quote(value, column = nil) - if value.kind_of?(String) && column && column.type == :binary - s = value.unpack("H*")[0] - "x'#{s}'" + def _quote(value) # :nodoc: + if value.is_a?(Type::Binary::Data) + "x'#{value.hex}'" else super end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index c875bc5162..4c719b834f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -61,7 +61,6 @@ module ActiveRecord end when String case sql_type - when 'bytea' then "'#{escape_bytea(value)}'" when 'xml' then "xml '#{quote_string(value)}'" when /^bit/ case value @@ -105,15 +104,6 @@ module ActiveRecord super(value, column) end end - when String - if 'bytea' == column.sql_type - # Return a bind param hash with format as binary. - # See http://deveiate.org/code/pg/PGconn.html#method-i-exec_prepared-doc - # for more information - { value: value, format: 1 } - else - super(value, column) - end when Hash case column.sql_type when 'hstore' then PostgreSQLColumn.hstore_to_string(value, array_member) @@ -173,6 +163,27 @@ module ActiveRecord quote(value, column) end end + + private + + def _quote(value) + if value.is_a?(Type::Binary::Data) + "'#{escape_bytea(value.to_s)}'" + else + super + end + end + + def _type_cast(value) + if value.is_a?(Type::Binary::Data) + # Return a bind param hash with format as binary. + # See http://deveiate.org/code/pg/PGconn.html#method-i-exec_prepared-doc + # for more information + { value: value.to_s, format: 1 } + else + super + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index adf893d7e7..e6163771e8 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -219,10 +219,9 @@ module ActiveRecord # QUOTING ================================================== - def quote(value, column = nil) - if value.kind_of?(String) && column && column.type == :binary - s = value.unpack("H*")[0] - "x'#{s}'" + def _quote(value) # :nodoc: + if value.is_a?(Type::Binary::Data) + "x'#{value.hex}'" else super end diff --git a/activerecord/lib/active_record/type/binary.rb b/activerecord/lib/active_record/type/binary.rb index e34b7bb268..9d10c91fc1 100644 --- a/activerecord/lib/active_record/type/binary.rb +++ b/activerecord/lib/active_record/type/binary.rb @@ -12,6 +12,24 @@ module ActiveRecord def klass ::String end + + def type_cast_for_database(value) + Data.new(super) + end + + class Data + def initialize(value) + @value = value + end + + def to_s + @value + end + + def hex + @value.unpack('H*')[0] + end + end end end end diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 1ba4daba37..e03d83df59 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -38,6 +38,9 @@ class PostgresqlArrayTest < ActiveRecord::TestCase def test_default @connection.add_column 'pg_arrays', 'score', :integer, array: true, default: [4, 4, 2] PgArray.reset_column_information + column = PgArray.columns_hash["score"] + + assert_equal([4, 4, 2], column.default) assert_equal([4, 4, 2], PgArray.new.score) ensure PgArray.reset_column_information @@ -48,6 +51,7 @@ class PostgresqlArrayTest < ActiveRecord::TestCase PgArray.reset_column_information column = PgArray.columns_hash["names"] + assert_equal(["foo", "bar"], column.default) assert_equal(["foo", "bar"], PgArray.new.names) ensure PgArray.reset_column_information diff --git a/activerecord/test/cases/adapters/postgresql/bit_string_test.rb b/activerecord/test/cases/adapters/postgresql/bit_string_test.rb index 667468837f..3a9397bc26 100644 --- a/activerecord/test/cases/adapters/postgresql/bit_string_test.rb +++ b/activerecord/test/cases/adapters/postgresql/bit_string_test.rb @@ -11,13 +11,14 @@ class PostgresqlBitStringTest < ActiveRecord::TestCase def setup @connection = ActiveRecord::Base.connection - @connection.create_table('postgresql_bit_strings') do |t| + @connection.create_table('postgresql_bit_strings', :force => true) do |t| t.bit :a_bit, default: "00000011", limit: 8 t.bit_varying :a_bit_varying, default: "0011", limit: 4 end end - teardown do + def teardown + return unless @connection @connection.execute 'DROP TABLE IF EXISTS postgresql_bit_strings' end @@ -42,7 +43,12 @@ class PostgresqlBitStringTest < ActiveRecord::TestCase end def test_default + column = PostgresqlBitString.columns_hash["a_bit"] + assert_equal "00000011", column.default assert_equal "00000011", PostgresqlBitString.new.a_bit + + column = PostgresqlBitString.columns_hash["a_bit_varying"] + assert_equal "0011", column.default assert_equal "0011", PostgresqlBitString.new.a_bit_varying end diff --git a/activerecord/test/cases/adapters/postgresql/enum_test.rb b/activerecord/test/cases/adapters/postgresql/enum_test.rb index fda718ca81..b809f1a79c 100644 --- a/activerecord/test/cases/adapters/postgresql/enum_test.rb +++ b/activerecord/test/cases/adapters/postgresql/enum_test.rb @@ -44,6 +44,7 @@ class PostgresqlEnumTest < ActiveRecord::TestCase PostgresqlEnum.reset_column_information column = PostgresqlEnum.columns_hash["good_mood"] + assert_equal "happy", column.default assert_equal "happy", PostgresqlEnum.new.good_mood ensure PostgresqlEnum.reset_column_information diff --git a/activerecord/test/cases/adapters/postgresql/geometric_test.rb b/activerecord/test/cases/adapters/postgresql/geometric_test.rb index 0a05caa249..2f106ee664 100644 --- a/activerecord/test/cases/adapters/postgresql/geometric_test.rb +++ b/activerecord/test/cases/adapters/postgresql/geometric_test.rb @@ -35,7 +35,12 @@ class PostgresqlPointTest < ActiveRecord::TestCase end def test_default + column = PostgresqlPoint.columns_hash["y"] + assert_equal [12.2, 13.3], column.default assert_equal [12.2, 13.3], PostgresqlPoint.new.y + + column = PostgresqlPoint.columns_hash["z"] + assert_equal [14.4, 15.5], column.default assert_equal [14.4, 15.5], PostgresqlPoint.new.z end diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index 1f1a81808e..1fef4899be 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -64,6 +64,9 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase def test_default @connection.add_column 'hstores', 'permissions', :hstore, default: '"users"=>"read", "articles"=>"write"' Hstore.reset_column_information + column = Hstore.columns_hash["permissions"] + + assert_equal({"users"=>"read", "articles"=>"write"}, column.default) assert_equal({"users"=>"read", "articles"=>"write"}, Hstore.new.permissions) ensure Hstore.reset_column_information diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index 8971bcd1c6..03b546119d 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -43,6 +43,9 @@ class PostgresqlJSONTest < ActiveRecord::TestCase def test_default @connection.add_column 'json_data_type', 'permissions', :json, default: '{"users": "read", "posts": ["read", "write"]}' JsonDataType.reset_column_information + column = JsonDataType.columns_hash["permissions"] + + assert_equal({"users"=>"read", "posts"=>["read", "write"]}, column.default) assert_equal({"users"=>"read", "posts"=>["read", "write"]}, JsonDataType.new.permissions) ensure JsonDataType.reset_column_information diff --git a/activerecord/test/cases/adapters/postgresql/money_test.rb b/activerecord/test/cases/adapters/postgresql/money_test.rb index a3a2a5ff23..3e33477bff 100644 --- a/activerecord/test/cases/adapters/postgresql/money_test.rb +++ b/activerecord/test/cases/adapters/postgresql/money_test.rb @@ -32,6 +32,8 @@ class PostgresqlMoneyTest < ActiveRecord::TestCase end def test_default + column = PostgresqlMoney.columns_hash["depth"] + assert_equal BigDecimal.new("150.55"), column.default assert_equal BigDecimal.new("150.55"), PostgresqlMoney.new.depth end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index cb706d77c2..cd0cb1a144 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -70,6 +70,8 @@ class HashExtTest < ActiveSupport::TestCase assert_respond_to h, :to_options! assert_respond_to h, :compact assert_respond_to h, :compact! + assert_respond_to h, :except + assert_respond_to h, :except! end def test_transform_keys @@ -919,13 +921,19 @@ class HashExtTest < ActiveSupport::TestCase def test_except_with_more_than_one_argument original = { :a => 'x', :b => 'y', :c => 10 } expected = { :a => 'x' } + assert_equal expected, original.except(:b, :c) + + assert_equal expected, original.except!(:b, :c) + assert_equal expected, original end def test_except_with_original_frozen original = { :a => 'x', :b => 'y' } original.freeze assert_nothing_raised { original.except(:a) } + + assert_raise(RuntimeError) { original.except!(:a) } end def test_except_with_mocha_expectation_on_original |