From 628110d7eeb446fee7f9e043f113c083d24883c1 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 14 May 2009 17:42:20 -0700 Subject: [PATCH 01/11] Active Support dependencies --- actionpack/lib/action_view/helpers/active_record_helper.rb | 1 + actionpack/test/abstract_controller/layouts_test.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/actionpack/lib/action_view/helpers/active_record_helper.rb b/actionpack/lib/action_view/helpers/active_record_helper.rb index 7c0dfdab10..b4b9f6e34b 100644 --- a/actionpack/lib/action_view/helpers/active_record_helper.rb +++ b/actionpack/lib/action_view/helpers/active_record_helper.rb @@ -1,5 +1,6 @@ require 'cgi' require 'action_view/helpers/form_helper' +require 'active_support/core_ext/class/attribute_accessors' module ActionView class Base diff --git a/actionpack/test/abstract_controller/layouts_test.rb b/actionpack/test/abstract_controller/layouts_test.rb index 6e1c2bf9e8..d3440c3de0 100644 --- a/actionpack/test/abstract_controller/layouts_test.rb +++ b/actionpack/test/abstract_controller/layouts_test.rb @@ -1,4 +1,5 @@ require File.join(File.expand_path(File.dirname(__FILE__)), "test_helper") +require 'active_support/core_ext/class/removal' module AbstractControllerTests module Layouts From 195fadbfd31294d43634afb7bbf4f0ffc86b470a Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Mon, 18 May 2009 16:59:37 +0200 Subject: [PATCH 02/11] Ensure HTTP Digest auth uses appropriate HTTP method [#2490 state:resolved] [Steve Madsen] --- .../base/http_authentication.rb | 3 ++- .../http_digest_authentication_test.rb | 23 ++++++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_controller/base/http_authentication.rb b/actionpack/lib/action_controller/base/http_authentication.rb index fa8ecea408..2893290efb 100644 --- a/actionpack/lib/action_controller/base/http_authentication.rb +++ b/actionpack/lib/action_controller/base/http_authentication.rb @@ -194,9 +194,10 @@ module ActionController if valid_nonce && realm == credentials[:realm] && opaque == credentials[:opaque] password = password_procedure.call(credentials[:username]) + method = request.env['rack.methodoverride.original_method'] || request.env['REQUEST_METHOD'] [true, false].any? do |password_is_ha1| - expected = expected_response(request.env['REQUEST_METHOD'], request.env['REQUEST_URI'], credentials, password, password_is_ha1) + expected = expected_response(method, request.env['REQUEST_URI'], credentials, password, password_is_ha1) expected == credentials[:response] end end diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 7bebc8cd2a..b8a2205ce6 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -149,6 +149,16 @@ class HttpDigestAuthenticationTest < ActionController::TestCase assert_equal 'Definitely Maybe', @response.body end + test "authentication request with _method" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please', :method => :post) + @request.env['rack.methodoverride.original_method'] = 'POST' + put :display + + assert_response :success + assert assigns(:logged_in) + assert_equal 'Definitely Maybe', @response.body + end + private def encode_credentials(options) @@ -159,15 +169,22 @@ class HttpDigestAuthenticationTest < ActionController::TestCase # to prevent tampering of timestamp ActionController::Base.session_options[:secret] = "session_options_secret" - # Perform unauthenticated GET to retrieve digest parameters to use on subsequent request - get :index + # Perform unauthenticated request to retrieve digest parameters to use on subsequent request + method = options.delete(:method) || 'GET' + + case method.to_s.upcase + when 'GET' + get :index + when 'POST' + post :index + end assert_response :unauthorized credentials = decode_credentials(@response.headers['WWW-Authenticate']) credentials.merge!(options) credentials.reverse_merge!(:uri => "#{@request.env['REQUEST_URI']}") - ActionController::HttpAuthentication::Digest.encode_credentials("GET", credentials, password, options[:password_is_ha1]) + ActionController::HttpAuthentication::Digest.encode_credentials(method, credentials, password, options[:password_is_ha1]) end def decode_credentials(header) From 5e190ef138a034bf86419ce4f4c343ae16bfc77b Mon Sep 17 00:00:00 2001 From: Andy Stewart Date: Wed, 28 Jan 2009 16:45:28 +0000 Subject: [PATCH 03/11] Truncate helper accepts a :separator for a more legible result [#1807 state:resolved] Signed-off-by: Pratik Naik --- actionpack/lib/action_view/helpers/text_helper.rb | 7 ++++++- actionpack/test/template/text_helper_test.rb | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index 8136a1cb13..ad0733a7e1 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -34,12 +34,16 @@ module ActionView # Truncates a given +text+ after a given :length if +text+ is longer than :length # (defaults to 30). The last characters will be replaced with the :omission (defaults to "..."). + # Pass a :separator to truncate +text+ at a natural break. # # ==== Examples # # truncate("Once upon a time in a world far far away") # # => Once upon a time in a world f... # + # truncate("Once upon a time in a world far far away", :separator => ' ') + # # => Once upon a time in a world... + # # truncate("Once upon a time in a world far far away", :length => 14) # # => Once upon a... # @@ -71,7 +75,8 @@ module ActionView if text l = options[:length] - options[:omission].mb_chars.length chars = text.mb_chars - (chars.length > options[:length] ? chars[0...l] + options[:omission] : text).to_s + stop = options[:separator] ? (chars.rindex(options[:separator].mb_chars, l) || l) : l + (chars.length > options[:length] ? chars[0...stop] + options[:omission] : text).to_s end end diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb index a780bfc606..706b5085f4 100644 --- a/actionpack/test/template/text_helper_test.rb +++ b/actionpack/test/template/text_helper_test.rb @@ -49,6 +49,9 @@ class TextHelperTest < ActionView::TestCase assert_equal "This is a string that wil[...]", truncate("This is a string that will go longer then the default truncate length of 30", :omission => "[...]") assert_equal "Hello W...", truncate("Hello World!", :length => 10) assert_equal "Hello[...]", truncate("Hello World!", :omission => "[...]", :length => 10) + assert_equal "Hello[...]", truncate("Hello Big World!", :omission => "[...]", :length => 13, :separator => ' ') + assert_equal "Hello Big[...]", truncate("Hello Big World!", :omission => "[...]", :length => 14, :separator => ' ') + assert_equal "Hello Big[...]", truncate("Hello Big World!", :omission => "[...]", :length => 15, :separator => ' ') end if RUBY_VERSION < '1.9.0' From 37453e11e9c071f9bdec47b073939fd34402127d Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 18 May 2009 11:55:06 -0700 Subject: [PATCH 04/11] Revert "Get AR CI passing again by requiring the entire core_ext" This reverts commit 8e6a18d8672f7efe6ef79b49185e4a6a23e4e547. --- activerecord/lib/active_record.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index b5c17cb23b..2d98239052 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -25,9 +25,6 @@ activesupport_path = "#{File.dirname(__FILE__)}/../../activesupport/lib" $:.unshift(activesupport_path) if File.directory?(activesupport_path) require 'active_support' -# TODO: Figure out what parts of AS are *actually* required and use those -require 'active_support/core_ext' - module ActiveRecord # TODO: Review explicit loads to see if they will automatically be handled by the initilizer. def self.load_all! From c5e109bbe766b0407a909361f18d2b87e9bfc75c Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Mon, 18 May 2009 21:27:42 +0200 Subject: [PATCH 05/11] Make sure default_scope#create checks for options[:conditions] [#2181 state:resolved] [James Le Cuirot] --- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/cases/method_scoping_test.rb | 10 ++++++++++ activerecord/test/models/developer.rb | 10 ++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index ca4f4fa6b6..968b9bde5a 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2176,7 +2176,7 @@ module ActiveRecord #:nodoc: # default_scope :order => 'last_name, first_name' # end def default_scope(options = {}) - self.default_scoping << { :find => options, :create => (options.is_a?(Hash) && options.has_key?(:conditions)) ? options[:conditions] : {} } + self.default_scoping << { :find => options, :create => options[:conditions].is_a?(Hash) ? options[:conditions] : {} } end # Test whether the given method and optional key are scoped. diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index c479859ee0..d8246f49b8 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -591,6 +591,16 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal expected, received end + def test_default_scope_with_conditions_string + assert_equal Developer.find_all_by_name('David').map(&:id).sort, DeveloperCalledDavid.all.map(&:id).sort + assert_equal nil, DeveloperCalledDavid.create!.name + end + + def test_default_scope_with_conditions_hash + assert_equal Developer.find_all_by_name('Jamis').map(&:id).sort, DeveloperCalledJamis.all.map(&:id).sort + assert_equal 'Jamis', DeveloperCalledJamis.create!.name + end + def test_default_scoping_with_threads scope = [{ :create => {}, :find => { :order => 'salary DESC' } }] diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 92039a4f54..058970336b 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -89,3 +89,13 @@ class DeveloperOrderedBySalary < ActiveRecord::Base end end end + +class DeveloperCalledDavid < ActiveRecord::Base + self.table_name = 'developers' + default_scope :conditions => "name = 'David'" +end + +class DeveloperCalledJamis < ActiveRecord::Base + self.table_name = 'developers' + default_scope :conditions => { :name => 'Jamis' } +end From a69b28a8b10fdcbb6801051e3086228b56891dbb Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 18 May 2009 13:20:04 -0700 Subject: [PATCH 06/11] Missing 1.8.7 backport extensions --- activerecord/lib/active_record/base.rb | 1 + activerecord/lib/active_record/validations.rb | 2 ++ 2 files changed, 3 insertions(+) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index ca4f4fa6b6..e8e1746b4b 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -9,6 +9,7 @@ require 'active_support/core_ext/hash/deep_merge' require 'active_support/core_ext/hash/indifferent_access' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/string/behavior' +require 'active_support/core_ext/symbol' require 'active_support/core/time' module ActiveRecord #:nodoc: diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index b6e848fa79..a18fb3f426 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/integer/even_odd' + module ActiveRecord # Raised by save! and create! when the record is invalid. Use the # +record+ method to retrieve the record which did not validate. From cef76c8af4705dc60f85a721e3a14adb99418d33 Mon Sep 17 00:00:00 2001 From: Luca Guidi Date: Mon, 16 Mar 2009 13:30:30 +0100 Subject: [PATCH 07/11] Ensure HasManyThroughAssociation#destroy delete orphan records [#2251 state:resolved] Signed-off-by: Pratik Naik --- .../associations/has_many_through_association.rb | 7 +++++++ .../associations/has_many_through_associations_test.rb | 10 ++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 2dca84b911..e8dbae9011 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -17,6 +17,13 @@ module ActiveRecord end end + def destroy(*records) + transaction do + delete_records(flatten_deeper(records)) + super + end + end + # Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been loaded and # calling collection.size if it has. If it's more likely than not that the collection does have a size larger than zero, # and you need to fetch that collection afterwards, it'll take one fewer SELECT query if you use #length. diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 97efca7891..51c70b9819 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -93,7 +93,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_destroy_association - assert_difference "Person.count", -1 do + assert_difference ["Person.count", "Reader.count"], -1 do posts(:welcome).people.destroy(people(:michael)) end @@ -102,7 +102,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_destroy_all - assert_difference "Person.count", -1 do + assert_difference ["Person.count", "Reader.count"], -1 do posts(:welcome).people.destroy_all end @@ -110,6 +110,12 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:welcome).people(true).empty? end + def test_should_raise_exception_for_destroying_mismatching_records + assert_no_difference ["Person.count", "Reader.count"] do + assert_raise(ActiveRecord::AssociationTypeMismatch) { posts(:welcome).people.destroy(posts(:thinking)) } + end + end + def test_replace_association assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)} From 87adecfef59577be17a9731245cb201ecb1b477f Mon Sep 17 00:00:00 2001 From: Ken Collins Date: Mon, 11 May 2009 23:22:20 -0400 Subject: [PATCH 08/11] Reimplement Fixtures.identify so that it consistently generates identities across ruby versions. [#2633 state:committed] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/fixtures.rb | 9 +++++---- activerecord/test/cases/fixtures_test.rb | 5 +++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index e30fcf9a4f..721114d9d0 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -1,6 +1,7 @@ require 'erb' require 'yaml' require 'csv' +require 'zlib' require 'active_support/dependencies' require 'active_support/test_case' require 'active_support/core_ext/logger' @@ -434,6 +435,7 @@ end # Any fixture labeled "DEFAULTS" is safely ignored. class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) + MAX_ID = 2 ** 31 - 1 DEFAULT_FILTER_RE = /\.ya?ml$/ @@all_cached_fixtures = {} @@ -525,11 +527,10 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) cached_fixtures(connection, table_names) end - # Returns a consistent identifier for +label+. This will always - # be a positive integer, and will always be the same for a given - # label, assuming the same OS, platform, and version of Ruby. + # Returns a consistent, platform-independent identifier for +label+. + # Identifiers are positive integers less than 2^32. def self.identify(label) - label.to_s.hash.abs + Zlib.crc32(label.to_s) % MAX_ID end attr_reader :table_name, :name diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 252bf4ff61..b07d4f3521 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -518,6 +518,11 @@ class FoxyFixturesTest < ActiveRecord::TestCase assert_equal(Fixtures.identify(:foo), Fixtures.identify(:foo)) end + def test_identifies_consistently + assert_equal 1281023246, Fixtures.identify(:ruby) + assert_equal 2140105598, Fixtures.identify(:sapphire_2) + end + TIMESTAMP_COLUMNS = %w(created_at created_on updated_at updated_on) def test_populates_timestamp_columns From 29550cc91f8dcbfe3e4020986d719c77ddd2f14d Mon Sep 17 00:00:00 2001 From: Joe Van Dyk Date: Mon, 2 Feb 2009 16:43:14 -0800 Subject: [PATCH 09/11] Add ability to get multiple memcached keys at the same time (via MemCacheStore#read_multi). Signed-off-by: Jeremy Kemper --- .../lib/active_support/cache/mem_cache_store.rb | 5 +++++ activesupport/memcached_get_multi.diff | 0 activesupport/test/caching_test.rb | 9 +++++++++ 3 files changed, 14 insertions(+) create mode 100644 activesupport/memcached_get_multi.diff diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index 4d8e1fdd67..90b7526fe1 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -43,6 +43,11 @@ module ActiveSupport extend Strategy::LocalCache end + # Reads multiple keys from the cache. + def read_multi(*keys) + @data.get_multi keys + end + def read(key, options = nil) # :nodoc: super @data.get(key, raw?(options)) diff --git a/activesupport/memcached_get_multi.diff b/activesupport/memcached_get_multi.diff new file mode 100644 index 0000000000..e69de29bb2 diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 8bb0c155cf..b845796fe5 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -251,6 +251,15 @@ uses_memcached 'memcached backed store' do end end + def test_multi_get + @cache.with_local_cache do + @cache.write('foo', 1) + @cache.write('goo', 2) + result = @cache.read_multi('foo', 'goo') + assert_equal({'foo' => 1, 'goo' => 2}, result) + end + end + def test_middleware app = lambda { |env| result = @cache.write('foo', 'bar') From 5463823df38310d392a3f87d633dce9b4150259a Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Sat, 2 May 2009 13:56:10 -0300 Subject: [PATCH 10/11] Remove unnecessary condition and local variable [#2602 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 968b9bde5a..af800261e5 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1541,12 +1541,12 @@ module ActiveRecord #:nodoc: end def reverse_sql_order(order_query) - reversed_query = order_query.to_s.split(/,/).each { |s| + order_query.to_s.split(/,/).each { |s| if s.match(/\s(asc|ASC)$/) s.gsub!(/\s(asc|ASC)$/, ' DESC') elsif s.match(/\s(desc|DESC)$/) s.gsub!(/\s(desc|DESC)$/, ' ASC') - elsif !s.match(/\s(asc|ASC|desc|DESC)$/) + else s.concat(' DESC') end }.join(',') From 41e9414b60e8d042bacf2f8d61d7efc07111417a Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 18 May 2009 15:01:13 -0700 Subject: [PATCH 11/11] Include acts_as_string? --- activesupport/lib/active_support/multibyte/chars.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/activesupport/lib/active_support/multibyte/chars.rb b/activesupport/lib/active_support/multibyte/chars.rb index 60f082bcc1..96ed35f0e0 100644 --- a/activesupport/lib/active_support/multibyte/chars.rb +++ b/activesupport/lib/active_support/multibyte/chars.rb @@ -1,4 +1,5 @@ # encoding: utf-8 +require 'active_support/core_ext/string/behavior' module ActiveSupport #:nodoc: module Multibyte #:nodoc: