diff options
23 files changed, 185 insertions, 43 deletions
diff --git a/.travis.yml b/.travis.yml index 265124a3af..9a0ace81bb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ env: - "GEM=ar:mysql" - "GEM=ar:mysql2" - "GEM=ar:sqlite3" + - "GEM=ar:postgresql" notifications: email: false irc: @@ -57,10 +57,10 @@ platforms :ruby do gem "nokogiri", ">= 1.4.5" # AR - gem "sqlite3", "~> 1.3.4" + gem "sqlite3", "~> 1.3.5" group :db do - gem "pg", ">= 0.11.0" unless ENV['TRAVIS'] # once pg is on travis this can be removed + gem "pg", ">= 0.11.0" gem "mysql", ">= 2.8.1" gem "mysql2", ">= 0.3.10" end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 5da21ddd3d..417701ea54 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -15,6 +15,7 @@ module ActionDispatch begin response = @app.call(env) + # TODO: Maybe this should be in the router itself if response[1]['X-Cascade'] == 'pass' raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" end @@ -22,7 +23,7 @@ module ActionDispatch raise exception if env['action_dispatch.show_exceptions'] == false end - response ? response : render_exception(env, exception) + exception ? render_exception(env, exception) : response end private diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 9efe90a670..0c95248611 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -40,7 +40,7 @@ module ActionDispatch raise exception if env['action_dispatch.show_exceptions'] == false end - response ? response : render_exception_with_failsafe(env, exception) + response || render_exception_with_failsafe(env, exception) end private diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index e6c0a06878..f7411c7729 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -11,6 +11,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest env['action_dispatch.show_detailed_exceptions'] = @detailed req = ActionDispatch::Request.new(env) case req.path + when "/pass" + [404, { "X-Cascade" => "pass" }, []] when "/not_found" raise ActionController::UnknownAction when "/runtime_error" @@ -46,6 +48,13 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end end + test 'raise an exception on cascade pass' do + @app = ProductionApp + assert_raise ActionController::RoutingError do + get "/pass", {}, {'action_dispatch.show_exceptions' => true} + end + end + test "rescue with diagnostics message" do @app = DevelopmentApp diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 9ffd085925..888bc43ec9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,8 @@ ## Rails 3.2.0 (unreleased) ## +* Implements `AR::Base.silence_auto_explain`. This method allows the user to + selectively disable automatic EXPLAINs within a block. *fxn* + * Implements automatic EXPLAIN logging for slow queries. A new configuration parameter `config.active_record.auto_explain_threshold_in_seconds` diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 6c878f0f00..827b01c5ac 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -184,7 +184,7 @@ module ActiveRecord macro = join_part.reflection.macro if macro == :has_one - return if record.association_cache.key?(join_part.reflection.name) + return record.association(join_part.reflection.name).target if record.association_cache.key?(join_part.reflection.name) association = join_part.instantiate(row) unless row[join_part.aliased_primary_key].nil? set_target_and_inverse(join_part, association, record) else 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 236681096b..17cf34cdf6 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -16,22 +16,16 @@ module ActiveRecord module ClassMethods protected - # Defined for all +datetime+ and +timestamp+ attributes when +time_zone_aware_attributes+ are enabled. - # This enhanced read method automatically converts the UTC time stored in the database to the time + # The enhanced read method automatically converts the UTC time stored in the database to the time # zone stored in Time.zone. - def internal_attribute_access_code(attr_name, cast_code) + def attribute_cast_code(attr_name) column = columns_hash[attr_name] if create_time_zone_conversion_attribute?(attr_name, column) - super(attr_name, "(v=#{column.type_cast_code('v')}) && #{cast_code}") - else - super - end - end + typecast = "v = #{super}" + time_zone_conversion = "v.acts_like?(:time) ? v.in_time_zone : v" - def attribute_cast_code(attr_name) - if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) - "(v.acts_like?(:time) ? v.in_time_zone : v)" + "((#{typecast}) && (#{time_zone_conversion}))" else super end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 3d55729318..9bc0023539 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2028,7 +2028,7 @@ MSG attribute_names.each do |name| if (column = column_for_attribute(name)) && (include_primary_key || !column.primary) - if include_readonly_attributes || (!include_readonly_attributes && !self.class.readonly_attributes.include?(name)) + if include_readonly_attributes || !self.class.readonly_attributes.include?(name) value = if klass.serialized_attributes.include?(name) @attributes[name].serialized_value @@ -2043,6 +2043,7 @@ MSG end end end + attrs 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 7f5ddf77d6..4d2c80356d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -499,9 +499,14 @@ module ActiveRecord # Returns a table's primary key and belonging sequence. def pk_and_sequence_for(table) - execute_and_free("SHOW INDEX FROM #{quote_table_name(table)} WHERE Key_name = 'PRIMARY'", 'SCHEMA') do |result| - keys = each_hash(result).map { |row| row[:Column_name] } - keys.length == 1 ? [keys.first, nil] : nil + execute_and_free("SHOW CREATE TABLE #{quote_table_name(table)}", 'SCHEMA') do |result| + create_table = each_hash(result).first[:"Create Table"] + if create_table.to_s =~ /PRIMARY KEY\s+\((.+)\)/ + keys = $1.split(",").map { |key| key.gsub(/`/, "") } + keys.length == 1 ? [keys.first, nil] : nil + else + nil + 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 0a0da0b5d3..386b3f7465 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -1,6 +1,6 @@ require 'active_record/connection_adapters/sqlite_adapter' -gem 'sqlite3', '~> 1.3.4' +gem 'sqlite3', '~> 1.3.5' require 'sqlite3' module ActiveRecord diff --git a/activerecord/lib/active_record/explain.rb b/activerecord/lib/active_record/explain.rb index 4d013f0ef4..abe6fff5d5 100644 --- a/activerecord/lib/active_record/explain.rb +++ b/activerecord/lib/active_record/explain.rb @@ -1,5 +1,5 @@ module ActiveRecord - module Explain # :nodoc: + module Explain # logging_query_plan calls could appear nested in the call stack. In # particular this happens when a relation fetches its records, since # that results in find_by_sql calls downwards. @@ -13,33 +13,34 @@ module ActiveRecord # whole. That is, the threshold is not checked against each individual # query, but against the duration of the entire block. This approach is # convenient for relations. - def logging_query_plan(&block) - if (t = auto_explain_threshold_in_seconds) && !Thread.current[LOGGING_QUERY_PLAN] + def logging_query_plan(&block) # :nodoc: + threshold = auto_explain_threshold_in_seconds + if threshold && !Thread.current[LOGGING_QUERY_PLAN] && !Thread.current[SILENCED] begin Thread.current[LOGGING_QUERY_PLAN] = true start = Time.now result, sqls, binds = collecting_sqls_for_explain(&block) - logger.warn(exec_explain(sqls, binds)) if Time.now - start > t + logger.warn(exec_explain(sqls, binds)) if Time.now - start > threshold result ensure Thread.current[LOGGING_QUERY_PLAN] = false end else - block.call + yield end end # SCHEMA queries cannot be EXPLAINed, also we do not want to run EXPLAIN on # our own EXPLAINs now matter how loopingly beautiful that would be. SKIP_EXPLAIN_FOR = %w(SCHEMA EXPLAIN) - def ignore_explain_notification?(payload) + def ignore_explain_notification?(payload) # :nodoc: payload[:exception] || SKIP_EXPLAIN_FOR.include?(payload[:name]) end # Collects all queries executed while the passed block runs. Returns an # array with three elements, the result of the block, the strings with the # queries, and their respective bindings. - def collecting_sqls_for_explain(&block) + def collecting_sqls_for_explain # :nodoc: sqls = [] binds = [] callback = lambda do |*args| @@ -52,7 +53,7 @@ module ActiveRecord result = nil ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do - result = block.call + result = yield end [result, sqls, binds] @@ -60,7 +61,7 @@ module ActiveRecord # Makes the adapter execute EXPLAIN for the given queries and bindings. # Returns a formatted string ready to be logged. - def exec_explain(sqls, binds) + def exec_explain(sqls, binds) # :nodoc: sqls.zip(binds).map do |sql, bind| [].tap do |msg| msg << "EXPLAIN for: #{sql}" @@ -72,5 +73,24 @@ module ActiveRecord end.join("\n") end.join("\n") end + + SILENCED = :silence_explain + + # Silences automatic EXPLAIN logging for the duration of the block. + # + # This has high priority, no EXPLAINs will be run even if downwards + # the threshold is set to 0. + # + # As the name of the method suggests this only applies to automatic + # EXPLAINs, manual calls to +ActiveRecord::Relation#explain+ run. + def silence_auto_explain + # Implemented as a flag rather that setting the threshold to nil + # because we should not depend on a value that may be changed + # downwards. + Thread.current[SILENCED] = true + yield + ensure + Thread.current[SILENCED] = false + end end end diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index d08f0b324d..18670b4177 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -68,11 +68,15 @@ class SchemaTest < ActiveRecord::TestCase end def test_schema_change_with_prepared_stmt + altered = false @connection.exec_query "select * from developers where id = $1", 'sql', [[nil, 1]] @connection.exec_query "alter table developers add column zomg int", 'sql', [] + altered = true @connection.exec_query "select * from developers where id = $1", 'sql', [[nil, 1]] ensure - @connection.exec_query "alter table developers drop column if exists zomg", 'sql', [] + # We are not using DROP COLUMN IF EXISTS because that syntax is only + # supported by pg 9.X + @connection.exec_query("alter table developers drop column zomg", 'sql', []) if altered end def test_table_exists? diff --git a/activerecord/test/cases/adapters/postgresql/utils_test.rb b/activerecord/test/cases/adapters/postgresql/utils_test.rb index 5f08f79171..9e7b08ef34 100644 --- a/activerecord/test/cases/adapters/postgresql/utils_test.rb +++ b/activerecord/test/cases/adapters/postgresql/utils_test.rb @@ -1,3 +1,5 @@ +require 'cases/helper' + class PostgreSQLUtilsTest < ActiveSupport::TestCase include ActiveRecord::ConnectionAdapters::PostgreSQLAdapter::Utils diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index c6e451fc57..1dc71ac4cc 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -252,6 +252,41 @@ class EagerAssociationTest < ActiveRecord::TestCase end end + def test_nested_loading_through_has_one_association + aa = AuthorAddress.find(author_addresses(:david_address).id, :include => {:author => :posts}) + assert_equal aa.author.posts.count, aa.author.posts.length + end + + def test_nested_loading_through_has_one_association_with_order + aa = AuthorAddress.find(author_addresses(:david_address).id, :include => {:author => :posts}, :order => 'author_addresses.id') + assert_equal aa.author.posts.count, aa.author.posts.length + end + + def test_nested_loading_through_has_one_association_with_order_on_association + aa = AuthorAddress.find(author_addresses(:david_address).id, :include => {:author => :posts}, :order => 'authors.id') + assert_equal aa.author.posts.count, aa.author.posts.length + end + + def test_nested_loading_through_has_one_association_with_order_on_nested_association + aa = AuthorAddress.find(author_addresses(:david_address).id, :include => {:author => :posts}, :order => 'posts.id') + assert_equal aa.author.posts.count, aa.author.posts.length + end + + def test_nested_loading_through_has_one_association_with_conditions + aa = AuthorAddress.find(author_addresses(:david_address).id, :include => {:author => :posts}, :conditions => "author_addresses.id > 0") + assert_equal aa.author.posts.count, aa.author.posts.length + end + + def test_nested_loading_through_has_one_association_with_conditions_on_association + aa = AuthorAddress.find(author_addresses(:david_address).id, :include => {:author => :posts}, :conditions => "authors.id > 0") + assert_equal aa.author.posts.count, aa.author.posts.length + end + + def test_nested_loading_through_has_one_association_with_conditions_on_nested_association + aa = AuthorAddress.find(author_addresses(:david_address).id, :include => {:author => :posts}, :conditions => "posts.id > 0") + assert_equal aa.author.posts.count, aa.author.posts.length + end + def test_eager_association_loading_with_belongs_to_and_foreign_keys pets = Pet.find(:all, :include => :owner) assert_equal 3, pets.length diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index a7cad329e8..cec5822acb 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -553,6 +553,17 @@ class AttributeMethodsTest < ActiveRecord::TestCase end end + def test_setting_time_zone_aware_read_attribute + utc_time = Time.utc(2008, 1, 1) + cst_time = utc_time.in_time_zone("Central Time (US & Canada)") + in_time_zone "Pacific Time (US & Canada)" do + record = @target.create(:written_on => cst_time).reload + assert_equal utc_time, record[:written_on] + assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], record[:written_on].time_zone + assert_equal Time.utc(2007, 12, 31, 16), record[:written_on].time + end + end + def test_setting_time_zone_aware_attribute_with_string utc_time = Time.utc(2008, 1, 1) (-11..13).each do |timezone_offset| @@ -572,6 +583,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase record = @target.new record.written_on = ' ' assert_nil record.written_on + assert_nil record[:written_on] end end diff --git a/activerecord/test/cases/explain_test.rb b/activerecord/test/cases/explain_test.rb index d3eb9c2cb2..0284cca920 100644 --- a/activerecord/test/cases/explain_test.rb +++ b/activerecord/test/cases/explain_test.rb @@ -80,6 +80,14 @@ if ActiveRecord::Base.connection.supports_explain? assert_equal expected, base.exec_explain(sqls, binds) end + def test_silence_auto_explain + base.expects(:collecting_sqls_for_explain).never + base.logger.expects(:warn).never + base.silence_auto_explain do + with_threshold(0) { Car.all } + end + end + def with_threshold(threshold) current_threshold = base.auto_explain_threshold_in_seconds base.auto_explain_threshold_in_seconds = threshold diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb index 5d7f74bb65..7b359a039b 100644 --- a/activesupport/lib/active_support/core_ext/string/output_safety.rb +++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb @@ -20,7 +20,7 @@ class ERB if s.html_safe? s else - s.gsub(/&/, "&").gsub(/\"/, """).gsub(/>/, ">").gsub(/</, "<").html_safe + s.gsub(/[&"><]/n) { |special| HTML_ESCAPE[special] }.html_safe end end diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb index ade09efc56..47b9f68ed0 100644 --- a/activesupport/test/core_ext/string_ext_test.rb +++ b/activesupport/test/core_ext/string_ext_test.rb @@ -21,12 +21,6 @@ class StringInflectionsTest < Test::Unit::TestCase include InflectorTestCases include ConstantizeTestCases - def test_erb_escape - string = [192, 60].pack('CC') - expected = 192.chr + "<" - assert_equal expected, ERB::Util.html_escape(string) - end - def test_strip_heredoc_on_an_empty_string assert_equal '', ''.strip_heredoc end @@ -497,6 +491,23 @@ class OutputSafetyTest < ActiveSupport::TestCase assert string.html_safe? assert !string.to_param.html_safe? end + + test "ERB::Util.html_escape should escape unsafe characters" do + string = '<>&"' + expected = '<>&"' + assert_equal expected, ERB::Util.html_escape(string) + end + + test "ERB::Util.html_escape should correctly handle invalid UTF-8 strings" do + string = [192, 60].pack('CC') + expected = 192.chr + "<" + assert_equal expected, ERB::Util.html_escape(string) + end + + test "ERB::Util.html_escape should not escape safe strings" do + string = "<b>hello</b>".html_safe + assert_equal string, ERB::Util.html_escape(string) + end end class StringExcludeTest < ActiveSupport::TestCase diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 64abff6cb3..2841996b56 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,7 +1,7 @@ ## Rails 3.2.0 (unreleased) ## * New applications get a flag - `config.active_record.auto_explain_threshold_in_seconds` in the evironments + `config.active_record.auto_explain_threshold_in_seconds` in the environments configuration files. With a value of 0.5 in development.rb, and commented out in production.rb. No mention in test.rb. *fxn* diff --git a/railties/guides/source/active_record_querying.textile b/railties/guides/source/active_record_querying.textile index 352f23dc01..0cbabd71a1 100644 --- a/railties/guides/source/active_record_querying.textile +++ b/railties/guides/source/active_record_querying.textile @@ -1369,6 +1369,42 @@ EXPLAIN for: SELECT `posts`.* FROM `posts` WHERE `posts`.`user_id` IN (1) under MySQL. +h4. Automatic EXPLAIN + +Active Record is able to run EXPLAIN automatically on slow queries and log its +output. This feature is controlled by the configuration parameter + +<ruby> +config.active_record.auto_explain_threshold_in_seconds +</ruby> + +If set to a number, any query exceeding those many seconds will have its EXPLAIN +automatically triggered and logged. In the case of relations, the threshold is +compared to the total time needed to fetch records. So, a relation is seen as a +unit of work, no matter whether the implementation of eager loading involves +several queries under the hood. + +A threshold of +nil+ disables automatic EXPLAINs. + +The default threshold in development mode is 0.5 seconds, and +nil+ in test and +production modes. + +h5. Disabling Automatic EXPLAIN + +Automatic EXPLAIN can be selectively silenced with +ActiveRecord::Base.silence_auto_explain+: + +<ruby> +ActiveRecord::Base.silence_auto_explain do + # no automatic EXPLAIN is triggered here +end +</ruby> + +That may be useful for queries you know are slow but fine, like a heavyweight +report of an admin interface. + +As its name suggests, +silence_auto_explain+ only silences automatic EXPLAINs. +Explicit calls to +ActiveRecord::Relation#explain+ run. + h4. Interpreting EXPLAIN Interpretation of the output of EXPLAIN is beyond the scope of this guide. The diff --git a/railties/guides/source/active_support_core_extensions.textile b/railties/guides/source/active_support_core_extensions.textile index 76d757f6a7..6646e9cd05 100644 --- a/railties/guides/source/active_support_core_extensions.textile +++ b/railties/guides/source/active_support_core_extensions.textile @@ -2055,7 +2055,7 @@ NOTE: Defined in +active_support/core_ext/enumerable.rb+. h4. +pluck+ -Plucks the value of the passed method for each element and returns the result as an array +The +pluck+ method collects the value of the passed method for each element and returns the result as an array. <ruby> people.pluck(:name) # => [ "David Heinemeier Hansson", "Jamie Heinemeier Hansson" ] diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 41bb4d8b20..baa80419a6 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -329,8 +329,8 @@ class AppGeneratorTest < Rails::Generators::TestCase def test_generated_environments_file_for_sanitizer run_generator [destination_root, "--skip-active-record"] - ["config/environments/development.rb", "config/environments/test.rb"].each do |env_file| - assert_file env_file do |file| + %w(development test).each do |env| + assert_file "config/environments/#{env}.rb" do |file| assert_no_match(/config.active_record.mass_assignment_sanitizer = :strict/, file) end end @@ -338,7 +338,7 @@ class AppGeneratorTest < Rails::Generators::TestCase def test_generated_environments_file_for_auto_explain run_generator [destination_root, "--skip-active-record"] - %w(development test production).each do |env| + %w(development production).each do |env| assert_file "config/environments/#{env}.rb" do |file| assert_no_match %r(auto_explain_threshold_in_seconds), file end |