diff options
14 files changed, 183 insertions, 24 deletions
diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 3794e277f6..c7827309dd 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -253,7 +253,7 @@ module ActionController #:nodoc: end def display_errors - controller.render format => resource.errors, :status => :unprocessable_entity + controller.render format => resource_errors, :status => :unprocessable_entity end # Check whether the resource has errors. @@ -286,5 +286,13 @@ module ActionController #:nodoc: def empty_json_resource "{}" end + + def resource_errors + respond_to?("#{format}_resource_errors") ? send("#{format}_resource_errors") : resource.errors + end + + def json_resource_errors + {:errors => resource.errors} + end end end diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index afb2d39955..afdab30eaf 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -745,6 +745,20 @@ class RespondWithControllerTest < ActionController::TestCase end end + def test_using_resource_for_post_with_json_yields_unprocessable_entity_on_failure + with_test_route_set do + @request.accept = "application/json" + errors = { :name => :invalid } + Customer.any_instance.stubs(:errors).returns(errors) + post :using_resource + assert_equal "application/json", @response.content_type + assert_equal 422, @response.status + errors = {:errors => errors} + assert_equal errors.to_json, @response.body + assert_nil @response.location + end + end + def test_using_resource_for_put_with_html_redirects_on_success with_test_route_set do put :using_resource @@ -808,6 +822,18 @@ class RespondWithControllerTest < ActionController::TestCase assert_nil @response.location end + def test_using_resource_for_put_with_json_yields_unprocessable_entity_on_failure + @request.accept = "application/json" + errors = { :name => :invalid } + Customer.any_instance.stubs(:errors).returns(errors) + put :using_resource + assert_equal "application/json", @response.content_type + assert_equal 422, @response.status + errors = {:errors => errors} + assert_equal errors.to_json, @response.body + assert_nil @response.location + end + def test_using_resource_for_delete_with_html_redirects_on_success with_test_route_set do Customer.any_instance.stubs(:destroyed?).returns(true) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 10ad35ae3c..50203608c2 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,10 +1,25 @@ -*Rails 3.1.1 (unreleased)* +*Rails 3.2.0 (unreleased)* + +* MySQL: case-insensitive uniqueness validation avoids calling LOWER when + the column already uses a case-insensitive collation. Fixes #561. + + [Joseph Palermo] * Transactional fixtures enlist all active database connections. You can test models on different connections without disabling transactional fixtures. [Jeremy Kemper] +* Add first_or_create, first_or_create!, first_or_initialize methods to Active Record. This is a + better approach over the old find_or_create_by dynamic methods because it's clearer which + arguments are used to find the record and which are used to create it: + + User.where(:first_name => "Scarlett").first_or_create!(:last_name => "Johansson") + + [Andrés Mejía] + +*Rails 3.1.1 (October 7, 2011)* + * Add deprecation for the preload_associations method. Fixes #3022. [Jon Leighton] @@ -33,13 +48,7 @@ keys are per process id. * lib/active_record/connection_adapters/sqlite_adapter.rb: ditto -* Add first_or_create, first_or_create!, first_or_initialize methods to Active Record. This is a - better approach over the old find_or_create_by dynamic methods because it's clearer which - arguments are used to find the record and which are used to create it: - - User.where(:first_name => "Scarlett").first_or_create!(:last_name => "Johansson") - - [Andrés Mejía] + [Aaron Patterson] * Support bulk change_table in mysql2 adapter, as well as the mysql one. [Jon Leighton] @@ -65,6 +74,7 @@ a URI that specifies the connection configuration. For example: [Prem Sichanugrist] + *Rails 3.1.0 (August 30, 2011)* * Add a proxy_association method to association proxies, which can be called by association diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 443e61b527..4c3a8f7233 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -238,6 +238,10 @@ module ActiveRecord node end + def case_insensitive_comparison(table, attribute, column, value) + table[attribute].lower.eq(table.lower(value)) + end + def current_savepoint_name "active_record_#{open_transactions}" 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 4b7c74e0b8..dd573ba569 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -4,6 +4,13 @@ module ActiveRecord module ConnectionAdapters class AbstractMysqlAdapter < AbstractAdapter class Column < ConnectionAdapters::Column # :nodoc: + attr_reader :collation + + def initialize(name, default, sql_type = nil, null = true, collation = nil) + super(name, default, sql_type, null) + @collation = collation + end + def extract_default(default) if sql_type =~ /blob/i || type == :text if default.blank? @@ -28,6 +35,10 @@ module ActiveRecord raise NotImplementedError end + def case_sensitive? + collation && !collation.match(/_ci$/) + end + private def simplified_type(field_type) @@ -157,8 +168,8 @@ module ActiveRecord end # Overridden by the adapters to instantiate their specific Column type. - def new_column(field, default, type, null) # :nodoc: - Column.new(field, default, type, null) + def new_column(field, default, type, null, collation) # :nodoc: + Column.new(field, default, type, null, collation) end # Must return the Mysql error number from the exception, if the exception has an @@ -393,10 +404,10 @@ module ActiveRecord # Returns an array of +Column+ objects for the table specified by +table_name+. def columns(table_name, name = nil)#:nodoc: - sql = "SHOW FIELDS FROM #{quote_table_name(table_name)}" + sql = "SHOW FULL FIELDS FROM #{quote_table_name(table_name)}" execute_and_free(sql, 'SCHEMA') do |result| each_hash(result).map do |field| - new_column(field[:Field], field[:Default], field[:Type], field[:Null] == "YES") + new_column(field[:Field], field[:Default], field[:Type], field[:Null] == "YES", field[:Collation]) end end end @@ -501,6 +512,14 @@ module ActiveRecord Arel::Nodes::Bin.new(node) end + def case_insensitive_comparison(table, attribute, column, value) + if column.case_sensitive? + super + else + table[attribute].eq(value) + end + end + def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key) where_sql end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 8b574518e5..971f3c35f3 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -47,8 +47,8 @@ module ActiveRecord end end - def new_column(field, default, type, null) # :nodoc: - Column.new(field, default, type, null) + def new_column(field, default, type, null, collation) # :nodoc: + Column.new(field, default, type, null, collation) end def error_number(exception) diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index a1824fe396..f092edecda 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -150,8 +150,8 @@ module ActiveRecord end end - def new_column(field, default, type, null) # :nodoc: - Column.new(field, default, type, null) + def new_column(field, default, type, null, collation) # :nodoc: + Column.new(field, default, type, null, collation) end def error_number(exception) # :nodoc: diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 484b1d369b..2e2ea8c42b 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -57,8 +57,8 @@ module ActiveRecord value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s if column.text? if !options[:case_sensitive] && value && column.text? - # will use SQL LOWER function before comparison - relation = table[attribute].lower.eq(table.lower(value)) + # will use SQL LOWER function before comparison, unless it detects a case insensitive collation + relation = klass.connection.case_insensitive_comparison(table, attribute, column, value) else value = klass.connection.case_sensitive_modifier(value) relation = table[attribute].eq(value) diff --git a/activerecord/test/cases/adapters/mysql/case_sensitivity_test.rb b/activerecord/test/cases/adapters/mysql/case_sensitivity_test.rb new file mode 100644 index 0000000000..5ffd886dab --- /dev/null +++ b/activerecord/test/cases/adapters/mysql/case_sensitivity_test.rb @@ -0,0 +1,35 @@ +require "cases/helper" +require 'models/person' + +class MysqlCaseSensitivityTest < ActiveRecord::TestCase + class CollationTest < ActiveRecord::Base + validates_uniqueness_of :string_cs_column, :case_sensitive => false + validates_uniqueness_of :string_ci_column, :case_sensitive => false + end + + def test_columns_include_collation_different_from_table + assert_equal 'utf8_bin', CollationTest.columns_hash['string_cs_column'].collation + assert_equal 'utf8_general_ci', CollationTest.columns_hash['string_ci_column'].collation + end + + def test_case_sensitive + assert !CollationTest.columns_hash['string_ci_column'].case_sensitive? + assert CollationTest.columns_hash['string_cs_column'].case_sensitive? + end + + def test_case_insensitive_comparison_for_ci_column + CollationTest.create!(:string_ci_column => 'A') + invalid = CollationTest.new(:string_ci_column => 'a') + queries = assert_sql { invalid.save } + ci_uniqueness_query = queries.detect { |q| q.match /string_ci_column/ } + assert_no_match(/lower/i, ci_uniqueness_query) + end + + def test_case_insensitive_comparison_for_cs_column + CollationTest.create!(:string_cs_column => 'A') + invalid = CollationTest.new(:string_cs_column => 'a') + queries = assert_sql { invalid.save } + cs_uniqueness_query = queries.detect { |q| q.match /string_cs_column/ } + assert_match(/lower/i, cs_uniqueness_query) + end +end diff --git a/activerecord/test/cases/adapters/mysql2/case_sensitivity_test.rb b/activerecord/test/cases/adapters/mysql2/case_sensitivity_test.rb new file mode 100644 index 0000000000..6bcc113482 --- /dev/null +++ b/activerecord/test/cases/adapters/mysql2/case_sensitivity_test.rb @@ -0,0 +1,35 @@ +require "cases/helper" +require 'models/person' + +class Mysql2CaseSensitivityTest < ActiveRecord::TestCase + class CollationTest < ActiveRecord::Base + validates_uniqueness_of :string_cs_column, :case_sensitive => false + validates_uniqueness_of :string_ci_column, :case_sensitive => false + end + + def test_columns_include_collation_different_from_table + assert_equal 'utf8_bin', CollationTest.columns_hash['string_cs_column'].collation + assert_equal 'utf8_general_ci', CollationTest.columns_hash['string_ci_column'].collation + end + + def test_case_sensitive + assert !CollationTest.columns_hash['string_ci_column'].case_sensitive? + assert CollationTest.columns_hash['string_cs_column'].case_sensitive? + end + + def test_case_insensitive_comparison_for_ci_column + CollationTest.create!(:string_ci_column => 'A') + invalid = CollationTest.new(:string_ci_column => 'a') + queries = assert_sql { invalid.save } + ci_uniqueness_query = queries.detect { |q| q.match(/string_ci_column/) } + assert_no_match(/lower/i, ci_uniqueness_query) + end + + def test_case_insensitive_comparison_for_cs_column + CollationTest.create!(:string_cs_column => 'A') + invalid = CollationTest.new(:string_cs_column => 'a') + queries = assert_sql { invalid.save } + cs_uniqueness_query = queries.detect { |q| q.match(/string_cs_column/)} + assert_match(/lower/i, cs_uniqueness_query) + end +end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 1166c45843..7e2dafcd01 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -48,11 +48,11 @@ class FixturesTest < ActiveRecord::TestCase def test_broken_yaml_exception badyaml = Tempfile.new ['foo', '.yml'] - badyaml.write 'a: !ruby.yaml.org,2002:str |\nfoo' + badyaml.write 'a: : ' badyaml.flush dir = File.dirname badyaml.path - name =File.basename badyaml.path, '.yml' + name = File.basename badyaml.path, '.yml' assert_raises(ActiveRecord::Fixture::FormatError) do ActiveRecord::Fixtures.create_fixtures(dir, name) end diff --git a/activerecord/test/schema/mysql2_specific_schema.rb b/activerecord/test/schema/mysql2_specific_schema.rb index c78d99f4af..ab2c7ccc10 100644 --- a/activerecord/test/schema/mysql2_specific_schema.rb +++ b/activerecord/test/schema/mysql2_specific_schema.rb @@ -21,4 +21,15 @@ BEGIN END SQL -end + ActiveRecord::Base.connection.execute <<-SQL +DROP TABLE IF EXISTS collation_tests; +SQL + + ActiveRecord::Base.connection.execute <<-SQL +CREATE TABLE collation_tests ( + string_cs_column VARCHAR(1) COLLATE utf8_bin, + string_ci_column VARCHAR(1) COLLATE utf8_general_ci +) CHARACTER SET utf8 COLLATE utf8_general_ci +SQL + +end
\ No newline at end of file diff --git a/activerecord/test/schema/mysql_specific_schema.rb b/activerecord/test/schema/mysql_specific_schema.rb index 30e1c5a167..a0adfe3752 100644 --- a/activerecord/test/schema/mysql_specific_schema.rb +++ b/activerecord/test/schema/mysql_specific_schema.rb @@ -32,4 +32,15 @@ BEGIN END SQL + ActiveRecord::Base.connection.execute <<-SQL +DROP TABLE IF EXISTS collation_tests; +SQL + + ActiveRecord::Base.connection.execute <<-SQL +CREATE TABLE collation_tests ( + string_cs_column VARCHAR(1) COLLATE utf8_bin, + string_ci_column VARCHAR(1) COLLATE utf8_general_ci +) CHARACTER SET utf8 COLLATE utf8_general_ci +SQL + end diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb index 2c9ebbf327..d4ffbe3d66 100644 --- a/railties/test/application/assets_test.rb +++ b/railties/test/application/assets_test.rb @@ -291,12 +291,12 @@ module ApplicationTests test "precompile should handle utf8 filenames" do filename = "レイルズ.png" app_file "app/assets/images/#{filename}", "not a image really" - add_to_config "config.assets.precompile = [ /\.png$$/, /application.(css|js)$/ ]" + add_to_config "config.assets.precompile = [ /\.png$/, /application.(css|js)$/ ]" precompile! require "#{app_path}/config/environment" - get "/assets/#{URI.escape(filename)}" + get "/assets/#{URI.parser.escape(filename)}" assert_match "not a image really", last_response.body assert File.exists?("#{app_path}/public/assets/#{filename}") end |