diff options
47 files changed, 344 insertions, 142 deletions
diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 606b01893e..a2d639cd56 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -60,11 +60,11 @@ module ActionDispatch assert_response(:redirect, message) return true if options == @response.location - redirected_to_after_normalisation = normalize_argument_to_redirection(@response.location) - options_after_normalisation = normalize_argument_to_redirection(options) + redirected_to_after_normalization = normalize_argument_to_redirection(@response.location) + options_after_normalization = normalize_argument_to_redirection(options) - if redirected_to_after_normalisation != options_after_normalisation - flunk "Expected response to be a redirect to <#{options_after_normalisation}> but was a redirect to <#{redirected_to_after_normalisation}>" + if redirected_to_after_normalization != options_after_normalization + flunk "Expected response to be a redirect to <#{options_after_normalization}> but was a redirect to <#{redirected_to_after_normalization}>" end end diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index a67b61c1ef..78eddb7530 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -71,11 +71,6 @@ module ActionView autoload :TemplateError autoload :WrongEncodingError end - - autoload_at "action_view/template" do - autoload :TemplateHandler - autoload :TemplateHandlers - end end ENCODING_FLAG = '#.*coding[:=]\s*(\S+)[ \t]*' diff --git a/actionpack/lib/action_view/helpers/controller_helper.rb b/actionpack/lib/action_view/helpers/controller_helper.rb index e22331cb3c..db59bca159 100644 --- a/actionpack/lib/action_view/helpers/controller_helper.rb +++ b/actionpack/lib/action_view/helpers/controller_helper.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/module/attr_internal' + module ActionView module Helpers # This module keeps all methods and behavior in ActionView diff --git a/actionpack/test/controller/new_base/base_test.rb b/actionpack/test/controller/new_base/base_test.rb index 8fa5d20372..ed244513a5 100644 --- a/actionpack/test/controller/new_base/base_test.rb +++ b/actionpack/test/controller/new_base/base_test.rb @@ -40,7 +40,7 @@ module Dispatching class ContainedEmptyController < ActionController::Base ; end class ContainedSubEmptyController < ContainedEmptyController ; end class ContainedNonDefaultPathController < ActionController::Base - def self.controller_path; "i_am_extremly_not_default"; end + def self.controller_path; "i_am_extremely_not_default"; end end end @@ -89,7 +89,7 @@ module Dispatching end test "namespaced non-default controller path" do - assert_equal 'i_am_extremly_not_default', Submodule::ContainedNonDefaultPathController.controller_path + assert_equal 'i_am_extremely_not_default', Submodule::ContainedNonDefaultPathController.controller_path assert_equal Submodule::ContainedNonDefaultPathController.controller_path, Submodule::ContainedNonDefaultPathController.new.controller_path end diff --git a/activemodel/CHANGELOG b/activemodel/CHANGELOG index be4de2e53c..b1ad315c46 100644 --- a/activemodel/CHANGELOG +++ b/activemodel/CHANGELOG @@ -1,3 +1,5 @@ +* Provide mass_assignment_sanitizer as an easy API to replace the sanitizer behavior. Also support both :logger (default) and :strict sanitizer behavior [Bogdan Gusiev] + *Rails 3.1.0 (unreleased)* * attr_accessible and friends now accepts :as as option to specify a role [Josh Kalderimis] diff --git a/activemodel/lib/active_model/mass_assignment_security.rb b/activemodel/lib/active_model/mass_assignment_security.rb index cc30609f2b..a7b4706906 100644 --- a/activemodel/lib/active_model/mass_assignment_security.rb +++ b/activemodel/lib/active_model/mass_assignment_security.rb @@ -1,4 +1,5 @@ -require 'active_support/core_ext/class/attribute.rb' +require 'active_support/core_ext/class/attribute' +require 'active_support/core_ext/string/inflections' require 'active_model/mass_assignment_security/permission_set' require 'active_model/mass_assignment_security/sanitizer' @@ -11,7 +12,9 @@ module ActiveModel class_attribute :_accessible_attributes class_attribute :_protected_attributes class_attribute :_active_authorizer - class_attribute :mass_assignment_sanitizer + + class_attribute :_mass_assignment_sanitizer + self.mass_assignment_sanitizer = :logger end # Mass assignment security provides an interface for protecting attributes @@ -43,6 +46,16 @@ module ActiveModel # # end # + # = Configuration options + # + # * <tt>mass_assignment_sanitizer</tt> - Defines sanitize method. Possible values are: + # * <tt>:logger</tt> (default) - writes filtered attributes to logger + # * <tt>:strict</tt> - raise <tt>ActiveModel::MassAssignmentSecurity::Error</tt> on any protected attribute update + # + # You can specify your own sanitizer object eg. MySanitizer.new. + # See <tt>ActiveModel::MassAssignmentSecurity::LoggerSanitizer</tt> for example implementation. + # + # module ClassMethods # Attributes named in this macro are protected from mass-assignment # whenever attributes are sanitized before assignment. A role for the @@ -156,7 +169,7 @@ module ActiveModel options = args.extract_options! role = options[:as] || :default - self._accessible_attributes = accessible_attributes_configs.dup + self._accessible_attributes = accessible_attributes_configs.dup self._accessible_attributes[role] = self.accessible_attributes(role) + args self._active_authorizer = self._accessible_attributes @@ -179,19 +192,25 @@ module ActiveModel [] end + def mass_assignment_sanitizer=(value) + self._mass_assignment_sanitizer = if value.is_a?(Symbol) + const_get(:"#{value.to_s.camelize}Sanitizer").new(self) + else + value + end + end + private def protected_attributes_configs self._protected_attributes ||= begin - default_black_list = BlackList.new(attributes_protected_by_default) - Hash.new(default_black_list) + Hash.new { |h,k| h[k] = BlackList.new(attributes_protected_by_default) } end end def accessible_attributes_configs self._accessible_attributes ||= begin - default_white_list = WhiteList.new - Hash.new(default_white_list) + Hash.new { |h,k| h[k] = WhiteList.new } end end end @@ -199,11 +218,7 @@ module ActiveModel protected def sanitize_for_mass_assignment(attributes, role = :default) - (mass_assignment_sanitizer || default_mass_assignment_sanitizer).sanitize(attributes, mass_assignment_authorizer(role)) - end - - def default_mass_assignment_sanitizer - DefaultSanitizer.new(self.respond_to?(:logger) && self.logger) + _mass_assignment_sanitizer.sanitize(attributes, mass_assignment_authorizer(role)) end def mass_assignment_authorizer(role = :default) diff --git a/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb b/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb index 5dbcf473bd..ee43a6694f 100644 --- a/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb +++ b/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb @@ -1,6 +1,11 @@ +require 'active_support/core_ext/module/delegation' + module ActiveModel module MassAssignmentSecurity class Sanitizer + def initialize(target=nil) + end + # Returns all attributes not denied by the authorizer. def sanitize(attributes, authorizer) sanitized_attributes = attributes.reject { |key, value| authorizer.deny?(key) } @@ -18,20 +23,32 @@ module ActiveModel def process_removed_attributes(attrs) raise NotImplementedError, "#process_removed_attributes(attrs) suppose to be overwritten" end - end - class DefaultSanitizer < Sanitizer - attr_accessor :logger + class LoggerSanitizer < Sanitizer + delegate :logger, :to => :@target - def initialize(logger = nil) - self.logger = logger - super() + def initialize(target) + @target = target + super end - + + def logger? + @target.respond_to?(:logger) && @target.logger + end + def process_removed_attributes(attrs) - self.logger.debug "WARNING: Can't mass-assign protected attributes: #{attrs.join(', ')}" if self.logger + logger.debug "WARNING: Can't mass-assign protected attributes: #{attrs.join(', ')}" if logger? end end + + class StrictSanitizer < Sanitizer + def process_removed_attributes(attrs) + raise ActiveModel::MassAssignmentSecurity::Error, "Can't mass-assign protected attributes: #{attrs.join(', ')}" + end + end + + class Error < StandardError + end end end diff --git a/activemodel/lib/active_model/validations/exclusion.rb b/activemodel/lib/active_model/validations/exclusion.rb index ab4de3c459..d3b8d31502 100644 --- a/activemodel/lib/active_model/validations/exclusion.rb +++ b/activemodel/lib/active_model/validations/exclusion.rb @@ -1,4 +1,4 @@ -require 'active_support/core_ext/range.rb' +require 'active_support/core_ext/range' module ActiveModel diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb index bfb65d2f3f..9a9270d615 100644 --- a/activemodel/lib/active_model/validations/inclusion.rb +++ b/activemodel/lib/active_model/validations/inclusion.rb @@ -1,4 +1,4 @@ -require 'active_support/core_ext/range.rb' +require 'active_support/core_ext/range' module ActiveModel diff --git a/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb b/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb index 8547694c24..62a6ec9c9b 100644 --- a/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb +++ b/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb @@ -3,7 +3,7 @@ require 'logger' require 'active_support/core_ext/object/inclusion' class SanitizerTest < ActiveModel::TestCase - + attr_accessor :logger class Authorizer < ActiveModel::MassAssignmentSecurity::PermissionSet def deny?(key) @@ -12,24 +12,32 @@ class SanitizerTest < ActiveModel::TestCase end def setup - @sanitizer = ActiveModel::MassAssignmentSecurity::DefaultSanitizer.new + @logger_sanitizer = ActiveModel::MassAssignmentSecurity::LoggerSanitizer.new(self) + @strict_sanitizer = ActiveModel::MassAssignmentSecurity::StrictSanitizer.new(self) @authorizer = Authorizer.new end test "sanitize attributes" do original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } - attributes = @sanitizer.sanitize(original_attributes, @authorizer) + attributes = @logger_sanitizer.sanitize(original_attributes, @authorizer) assert attributes.key?('first_name'), "Allowed key shouldn't be rejected" assert !attributes.key?('admin'), "Denied key should be rejected" end - test "debug mass assignment removal" do + test "debug mass assignment removal with LoggerSanitizer" do original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } log = StringIO.new - @sanitizer.logger = Logger.new(log) - @sanitizer.sanitize(original_attributes, @authorizer) + self.logger = Logger.new(log) + @logger_sanitizer.sanitize(original_attributes, @authorizer) assert_match(/admin/, log.string, "Should log removed attributes: #{log.string}") end + test "debug mass assignment removal with StrictSanitizer" do + original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } + assert_raise ActiveModel::MassAssignmentSecurity::Error do + @strict_sanitizer.sanitize(original_attributes, @authorizer) + end + end + end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index a0a1ff23db..3acfa5f729 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -860,7 +860,7 @@ module ActiveRecord # the inverse of each other and the inverse of the +dungeon+ association on +EvilWizard+ # is the +evil_wizard+ association on +Dungeon+ (and vice-versa). By default, # Active Record doesn't know anything about these inverse relationships and so no object - # loading optimisation is possible. For example: + # loading optimization is possible. For example: # # d = Dungeon.first # t = d.traps.first diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 8a028c8996..02cc455a4e 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -104,26 +104,11 @@ module ActiveRecord end def create(attributes = {}, options = {}, &block) - unless owner.persisted? - raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" - end - - if attributes.is_a?(Array) - attributes.collect { |attr| create(attr, options, &block) } - else - transaction do - add_to_target(build_record(attributes, options)) do |record| - yield(record) if block_given? - insert_record(record) - end - end - end + create_record(attributes, options, &block) end - def create!(attrs = {}, options = {}, &block) - record = create(attrs, options, &block) - Array.wrap(record).each(&:save!) - record + def create!(attributes = {}, options = {}, &block) + create_record(attributes, options, true, &block) end # Add +records+ to this association. Returns +self+ so method calls may be chained. @@ -402,9 +387,13 @@ module ActiveRecord return memory if persisted.empty? persisted.map! do |record| - mem_record = memory.delete(record) + # Unfortunately we cannot simply do memory.delete(record) since on 1.8 this returns + # record rather than memory.at(memory.index(record)). The behaviour is fixed in 1.9. + mem_index = memory.index(record) + + if mem_index + mem_record = memory.delete_at(mem_index) - if mem_record (record.attribute_names - mem_record.changes.keys).each do |name| mem_record[name] = record[name] end @@ -418,8 +407,25 @@ module ActiveRecord persisted + memory end + def create_record(attributes, options, raise = false, &block) + unless owner.persisted? + raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" + end + + if attributes.is_a?(Array) + attributes.collect { |attr| create_record(attr, options, raise, &block) } + else + transaction do + add_to_target(build_record(attributes, options)) do |record| + yield(record) if block_given? + insert_record(record, true, raise) + end + end + end + end + # Do the relevant stuff to insert the given record into the association collection. - def insert_record(record, validate = true) + def insert_record(record, validate = true, raise = false) raise NotImplementedError end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index adfc71d435..81b4a26b04 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -56,6 +56,8 @@ module ActiveRecord Array.wrap(association.options[:extend]).each { |ext| proxy_extend(ext) } end + alias_method :new, :build + def respond_to?(*args) super || (load_target && target.respond_to?(*args)) || @@ -115,14 +117,6 @@ module ActiveRecord @association.reload self end - - def new(*args, &block) - if @association.is_a?(HasManyThroughAssociation) - @association.build(*args, &block) - else - method_missing(:new, *args, &block) - end - end end end end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 217213808b..f7ce70db1a 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -9,8 +9,14 @@ module ActiveRecord super end - def insert_record(record, validate = true) - return if record.new_record? && !record.save(:validate => validate) + def insert_record(record, validate = true, raise = false) + if record.new_record? + if raise + record.save!(:validate => validate) + else + return unless record.save(:validate => validate) + end + end if options[:insert_sql] owner.connection.insert(interpolate(options[:insert_sql], record)) diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 7172e89a05..50ee60284c 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -7,9 +7,14 @@ module ActiveRecord # is provided by its child HasManyThroughAssociation. class HasManyAssociation < CollectionAssociation #:nodoc: - def insert_record(record, validate = true) + def insert_record(record, validate = true, raise = false) set_owner_attributes(record) - record.save(:validate => validate) + + if raise + record.save!(:validate => validate) + else + record.save(:validate => validate) + end end private 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 7708228d23..2e818dca5d 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -6,8 +6,6 @@ module ActiveRecord class HasManyThroughAssociation < HasManyAssociation #:nodoc: include ThroughAssociation - alias_method :new, :build - # 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 @@ -33,9 +31,16 @@ module ActiveRecord super end - def insert_record(record, validate = true) + def insert_record(record, validate = true, raise = false) ensure_not_nested - return if record.new_record? && !record.save(:validate => validate) + + if record.new_record? + if raise + record.save!(:validate => validate) + else + return unless record.save(:validate => validate) + end + end through_record(record).save! update_counter(1) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index f74810720d..08cc282d09 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1308,7 +1308,6 @@ MSG rescue NameError => e # We don't want to swallow NoMethodError < NameError errors raise e unless e.instance_of?(NameError) - rescue ArgumentError end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 8b48c055ac..3e390ba994 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -805,8 +805,8 @@ module ActiveRecord end if pk && sequence - quoted_sequence = quote_column_name(sequence) - + quoted_sequence = quote_table_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 @@ -818,18 +818,25 @@ module ActiveRecord # First try looking for a sequence with a dependency on the # given table's primary key. result = exec_query(<<-end_sql, 'SCHEMA').rows.first - SELECT attr.attname, seq.relname + SELECT attr.attname, ns.nspname, seq.relname FROM pg_class seq INNER JOIN pg_depend dep ON seq.oid = dep.objid INNER JOIN pg_attribute attr ON attr.attrelid = dep.refobjid AND attr.attnum = dep.refobjsubid INNER JOIN pg_constraint cons ON attr.attrelid = cons.conrelid AND attr.attnum = cons.conkey[1] + INNER JOIN pg_namespace ns ON seq.relnamespace = ns.oid WHERE seq.relkind = 'S' AND cons.contype = 'p' AND dep.refobjid = '#{quote_table_name(table)}'::regclass end_sql # [primary_key, sequence] - [result.first, result.last] + if result.second == 'public' then + sequence = result.last + else + sequence = result.second+'.'+result.last + end + + [result.first, sequence] rescue nil end diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 588f52be44..14db7a6cd6 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -17,7 +17,7 @@ module ActiveRecord # posts.each {|p| puts p.name } # Fires "select * from posts" and loads post objects # # fruits = Fruit.scoped - # fruits = fruits.where(:colour => 'red') if options[:red_only] + # fruits = fruits.where(:color => 'red') if options[:red_only] # fruits = fruits.limit(10) if limited? # # Anonymous \scopes tend to be useful when procedurally generating complex diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index e852f50d86..317af8a15d 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -6,7 +6,7 @@ module ActiveRecord JoinOperation = Struct.new(:relation, :join_class, :on) ASSOCIATION_METHODS = [:includes, :eager_load, :preload] MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having, :bind] - SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from, :reorder] + SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from, :reorder, :reverse_order] include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index a785f38e89..aabe5c269b 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -196,7 +196,7 @@ module ActiveRecord def execute_simple_calculation(operation, column_name, distinct) #:nodoc: # Postgresql doesn't like ORDER BY when there are no GROUP BY - relation = reorder(nil) + relation = with_default_scope.reorder(nil) if operation == "count" && (relation.limit_value || relation.offset_value) # Shortcut when limit is zero. @@ -245,7 +245,7 @@ module ActiveRecord "#{field} AS #{aliaz}" } - relation = except(:group).group(group.join(',')) + relation = with_default_scope.except(:group).group(group.join(',')) relation.select_values = select_values calculated_data = @klass.connection.select_all(relation.to_sql) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 94aa999715..739363415c 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -9,7 +9,7 @@ module ActiveRecord :select_values, :group_values, :order_values, :joins_values, :where_values, :having_values, :bind_values, :limit_value, :offset_value, :lock_value, :readonly_value, :create_with_value, - :from_value, :reorder_value + :from_value, :reorder_value, :reverse_order_value def includes(*args) args.reject! {|a| a.blank? } @@ -158,13 +158,9 @@ module ActiveRecord end def reverse_order - order_clause = arel.order_clauses - - order = order_clause.empty? ? - "#{table_name}.#{primary_key} DESC" : - reverse_sql_order(order_clause).join(', ') - - except(:order).order(Arel.sql(order)) + relation = clone + relation.reverse_order_value = !relation.reverse_order_value + relation end def arel @@ -186,6 +182,7 @@ module ActiveRecord arel.group(*@group_values.uniq.reject{|g| g.blank?}) unless @group_values.empty? order = @reorder_value ? @reorder_value : @order_values + order = reverse_sql_order(order) if @reverse_order_value arel.order(*order.uniq.reject{|o| o.blank?}) unless order.empty? build_select(arel, @select_values.uniq) @@ -306,6 +303,8 @@ module ActiveRecord end def reverse_sql_order(order_query) + order_query = ["#{quoted_table_name}.#{quoted_primary_key} ASC"] if order_query.empty? + order_query.join(', ').split(',').collect do |s| s.gsub!(/\sasc\Z/i, ' DESC') || s.gsub!(/\sdesc\Z/i, ' ASC') || s.concat(' DESC') end diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 62e6999736..19585f6214 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -106,7 +106,7 @@ HEADER spec = {} spec[:name] = column.name.inspect - # AR has an optimisation which handles zero-scale decimals as integers. This + # AR has an optimization which handles zero-scale decimals as integers. This # code ensures that the dumper still dumps the column as a decimal. spec[:type] = if column.type == :integer && [/^numeric/, /^decimal/].any? { |e| e.match(column.sql_type) } 'decimal' diff --git a/activerecord/test/cases/adapters/postgresql/timestamp_test.rb b/activerecord/test/cases/adapters/postgresql/timestamp_test.rb new file mode 100644 index 0000000000..337f43c421 --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/timestamp_test.rb @@ -0,0 +1,30 @@ +require 'cases/helper' +require 'models/developer' + +class TimestampTest < ActiveRecord::TestCase + def test_load_infinity_and_beyond + unless current_adapter?(:PostgreSQLAdapter) + return skip("only tested on postgresql") + end + + d = Developer.find_by_sql("select 'infinity'::timestamp as updated_at") + assert d.first.updated_at.infinite?, 'timestamp should be infinite' + + d = Developer.find_by_sql("select '-infinity'::timestamp as updated_at") + time = d.first.updated_at + assert time.infinite?, 'timestamp should be infinite' + assert_operator time, :<, 0 + end + + def test_save_infinity_and_beyond + unless current_adapter?(:PostgreSQLAdapter) + return skip("only tested on postgresql") + end + + d = Developer.create!(:name => 'aaron', :updated_at => 1.0 / 0.0) + assert_equal(1.0 / 0.0, d.updated_at) + + d = Developer.create!(:name => 'aaron', :updated_at => -1.0 / 0.0) + assert_equal(-1.0 / 0.0, d.updated_at) + end +end diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 49d8722aff..ff376a68d8 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -8,10 +8,12 @@ require 'models/company' require 'models/topic' require 'models/reply' require 'models/person' +require 'models/vertex' +require 'models/edge' class CascadedEagerLoadingTest < ActiveRecord::TestCase fixtures :authors, :mixins, :companies, :posts, :topics, :accounts, :comments, - :categorizations, :people, :categories + :categorizations, :people, :categories, :edges, :vertices def test_eager_association_loading_with_cascaded_two_levels authors = Author.find(:all, :include=>{:posts=>:comments}, :order=>"authors.id") @@ -164,12 +166,6 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase authors[2].post_about_thinking.comments.first end end -end - -require 'models/vertex' -require 'models/edge' -class CascadedEagerLoadingTest < ActiveRecord::TestCase - fixtures :edges, :vertices def test_eager_association_loading_with_recursive_cascading_four_levels_has_many_through source = Vertex.find(:first, :include=>{:sinks=>{:sinks=>{:sinks=>:sinks}}}, :order => 'vertices.id') diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 839a7852fc..e5735988d0 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -245,6 +245,21 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal Developer.find(1).projects.sort_by(&:id).last, proj # prove join table is updated end + def test_new_aliased_to_build + devel = Developer.find(1) + proj = assert_no_queries { devel.projects.new("name" => "Projekt") } + assert !devel.projects.loaded? + + assert_equal devel.projects.last, proj + assert devel.projects.loaded? + + assert !proj.persisted? + devel.save + assert proj.persisted? + assert_equal devel.projects.last, proj + assert_equal Developer.find(1).projects.sort_by(&:id).last, proj # prove join table is updated + end + def test_build_by_new_record devel = Developer.new(:name => "Marcel", :salary => 75000) devel.projects.build(:name => "Make bed") diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 43974fd895..49999630b6 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2,6 +2,7 @@ require "cases/helper" require 'models/developer' require 'models/project' require 'models/company' +require 'models/contract' require 'models/topic' require 'models/reply' require 'models/category' @@ -536,6 +537,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 3, companies(:first_firm).clients_of_firm(true).size end + def test_new_aliased_to_build + company = companies(:first_firm) + new_client = assert_no_queries { company.clients_of_firm.new("name" => "Another Client") } + assert !company.clients_of_firm.loaded? + + assert_equal "Another Client", new_client.name + assert !new_client.persisted? + assert_equal new_client, company.clients_of_firm.last + end + def test_build company = companies(:first_firm) new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") } @@ -1475,4 +1486,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase tagging = Tagging.create! :taggable => post assert_equal [tagging], post.taggings end + + def test_dont_call_save_callbacks_twice_on_has_many + firm = companies(:first_firm) + contract = firm.contracts.create! + + assert_equal 1, contract.hi_count + assert_equal 1, contract.bye_count + end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 54c4d4ae90..2712fa8e1d 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -134,7 +134,12 @@ class AttributeMethodsTest < ActiveRecord::TestCase if current_adapter?(:MysqlAdapter) def test_read_attributes_before_type_cast_on_boolean bool = Boolean.create({ "value" => false }) - assert_equal 0, bool.reload.attributes_before_type_cast["value"] + if RUBY_PLATFORM =~ /java/ + #JRuby will returns the value before typecast as integer + assert_equal 0, bool.reload.attributes_before_type_cast["value"] + else + assert_equal "0", bool.reload.attributes_before_type_cast["value"] + end end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 39043447fc..2224097f04 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1766,6 +1766,13 @@ class BasicsTest < ActiveRecord::TestCase end end + def test_compute_type_argument_error + ActiveSupport::Dependencies.stubs(:constantize).raises(ArgumentError) + assert_raises ArgumentError do + ActiveRecord::Base.send :compute_type, 'InvalidModel' + end + end + def test_clear_cache! # preheat cache c1 = Post.columns diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index 8f8e72f052..a6e08f95d0 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -11,6 +11,14 @@ require 'models/reference' class RelationScopingTest < ActiveRecord::TestCase fixtures :authors, :developers, :projects, :comments, :posts, :developers_projects + def test_reverse_order + assert_equal Developer.order("id DESC").to_a.reverse, Developer.order("id DESC").reverse_order + end + + def test_double_reverse_order_produces_original_order + assert_equal Developer.order("name DESC"), Developer.order("name DESC").reverse_order.reverse_order + end + def test_scoped_find Developer.where("name = 'David'").scoping do assert_nothing_raised { Developer.find(1) } @@ -471,7 +479,23 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal 10, DeveloperCalledJamis.unscoped.poor.length end + def test_default_scope_select_ignored_by_aggregations + assert_equal DeveloperWithSelect.all.count, DeveloperWithSelect.count + end + + def test_default_scope_select_ignored_by_grouped_aggregations + assert_equal Hash[Developer.all.group_by(&:salary).map { |s, d| [s, d.count] }], + DeveloperWithSelect.group(:salary).count + end + def test_default_scope_order_ignored_by_aggregations assert_equal DeveloperOrderedBySalary.all.count, DeveloperOrderedBySalary.count end + + def test_default_scope_find_last + assert DeveloperOrderedBySalary.count > 1, "need more than one row for test" + + lowest_salary_dev = DeveloperOrderedBySalary.find(developers(:poor_jamis).id) + assert_equal lowest_salary_dev, DeveloperOrderedBySalary.last + end end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 6874bd18f8..12e58bd340 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -20,7 +20,7 @@ module ActiveRecord end def test_single_values - assert_equal [:limit, :offset, :lock, :readonly, :create_with, :from, :reorder].map(&:to_s).sort, + assert_equal [:limit, :offset, :lock, :readonly, :create_with, :from, :reorder, :reverse_order].map(&:to_s).sort, Relation::SINGLE_VALUE_METHODS.map(&:to_s).sort end diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 22d4cac422..ceb1452afd 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -14,32 +14,6 @@ class TimestampTest < ActiveRecord::TestCase @previously_updated_at = @developer.updated_at end - def test_load_infinity_and_beyond - unless current_adapter?(:PostgreSQLAdapter) - return skip("only tested on postgresql") - end - - d = Developer.find_by_sql("select 'infinity'::timestamp as updated_at") - assert d.first.updated_at.infinite?, 'timestamp should be infinite' - - d = Developer.find_by_sql("select '-infinity'::timestamp as updated_at") - time = d.first.updated_at - assert time.infinite?, 'timestamp should be infinite' - assert_operator time, :<, 0 - end - - def test_save_infinity_and_beyond - unless current_adapter?(:PostgreSQLAdapter) - return skip("only tested on postgresql") - end - - d = Developer.create!(:name => 'aaron', :updated_at => 1.0 / 0.0) - assert_equal(1.0 / 0.0, d.updated_at) - - d = Developer.create!(:name => 'aaron', :updated_at => -1.0 / 0.0) - assert_equal(-1.0 / 0.0, d.updated_at) - end - def test_saving_a_changed_record_updates_its_timestamp @developer.name = "Jack Bauer" @developer.save! diff --git a/activerecord/test/models/contract.rb b/activerecord/test/models/contract.rb index 94fd48e12a..2cf5aa7a85 100644 --- a/activerecord/test/models/contract.rb +++ b/activerecord/test/models/contract.rb @@ -1,4 +1,19 @@ class Contract < ActiveRecord::Base belongs_to :company belongs_to :developer + + before_save :hi + after_save :bye + + attr_accessor :hi_count, :bye_count + + def hi + @hi_count ||= 0 + @hi_count += 1 + end + + def bye + @bye_count ||= 0 + @bye_count += 1 + end end diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 41c52f7df0..98d6aa22f7 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -86,6 +86,11 @@ class DeveloperWithBeforeDestroyRaise < ActiveRecord::Base end end +class DeveloperWithSelect < ActiveRecord::Base + self.table_name = 'developers' + default_scope select('name') +end + class DeveloperOrderedBySalary < ActiveRecord::Base self.table_name = 'developers' default_scope :order => 'salary DESC' diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 0c272fa093..74730ca01f 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -283,7 +283,7 @@ module ActiveResource # attribute 'name', :string # # # or use the convenience methods and pass >=1 attribute names - # string 'eye_colour', 'hair_colour' + # string 'eye_color', 'hair_color' # integer 'age' # float 'height', 'weight' # diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index bf48306684..6b7044aeae 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -7,6 +7,8 @@ *Rails 3.1.0 (unreleased)* +* ActiveSupport::Dependencies now raises NameError if it finds an existing constant in load_missing_constant. This better reflects the nature of the error which is usually caused by calling constantize on a nested constant. [Andrew White] + * Deprecated ActiveSupport::SecureRandom in favour of SecureRandom from the standard library [Jon Leighton] * New reporting method Kernel#quietly. [fxn] diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index 7ef1497ac2..64d2c3bff6 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -31,7 +31,7 @@ module ActiveSupport DELETED = "DELETED\r\n" end - ESCAPE_KEY_CHARS = /[\x00-\x20%\x7F-\xFF]/ + ESCAPE_KEY_CHARS = /[\x00-\x20%\x7F-\xFF]/n def self.build_mem_cache(*addresses) addresses = addresses.flatten diff --git a/activesupport/lib/active_support/core_ext/array/access.rb b/activesupport/lib/active_support/core_ext/array/access.rb index 2df4fd1da1..6162f7af27 100644 --- a/activesupport/lib/active_support/core_ext/array/access.rb +++ b/activesupport/lib/active_support/core_ext/array/access.rb @@ -16,7 +16,7 @@ class Array # %w( a b c d ).to(10) # => %w( a b c d ) # %w().to(0) # => %w() def to(position) - self[0..position] + self.first position + 1 end # Equal to <tt>self[1]</tt>. diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index cae68c3c95..26c5c157cb 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -473,7 +473,7 @@ module ActiveSupport #:nodoc: raise ArgumentError, "A copy of #{from_mod} has been removed from the module tree but is still active!" end - raise ArgumentError, "#{from_mod} is not missing constant #{const_name}!" if local_const_defined?(from_mod, const_name) + raise NameError, "#{from_mod} is not missing constant #{const_name}!" if local_const_defined?(from_mod, const_name) qualified_name = qualified_name_for from_mod, const_name path_suffix = qualified_name.underscore diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 2ddbce5150..b4edf0f51d 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -441,7 +441,7 @@ class DependenciesTest < Test::Unit::TestCase with_autoloading_fixtures do require_dependency '././counting_loader' assert_equal 1, $counting_loaded_times - assert_raise(ArgumentError) { ActiveSupport::Dependencies.load_missing_constant Object, :CountingLoader } + assert_raise(NameError) { ActiveSupport::Dependencies.load_missing_constant Object, :CountingLoader } assert_equal 1, $counting_loaded_times end end diff --git a/railties/guides/source/api_documentation_guidelines.textile b/railties/guides/source/api_documentation_guidelines.textile index e22ffa4c04..e046bc5cbb 100644 --- a/railties/guides/source/api_documentation_guidelines.textile +++ b/railties/guides/source/api_documentation_guidelines.textile @@ -33,6 +33,10 @@ Spell names correctly: Arel, Test::Unit, RSpec, HTML, MySQL, JavaScript, ERB. Wh Use the article "an" for "SQL", as in "an SQL statement". Also "an SQLite database". +h3. English + +Please use American English (_color_, _center_, _modularize_, etc.). See "a list of American and British English spelling differences here":http://en.wikipedia.org/wiki/American_and_British_English_spelling_differences. + h3. Example Code Choose meaningful examples that depict and cover the basics as well as interesting points or gotchas. diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt index 19294b3478..f33a7f4ac2 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/javascripts/application.js.tt @@ -1,9 +1,15 @@ -// This is a manifest file that'll be compiled into including all the files listed below. -// Add new JavaScript/Coffee code in separate files in this directory and they'll automatically -// be included in the compiled file accessible from http://example.com/assets/application.js +// This is a manifest file that'll be compiled into application.js, which will include all the files +// listed below. +// +// Any JavaScript/Coffee file within this directory, lib/assets/javascripts, vendor/assets/javascripts, +// or vendor/assets/javascripts of plugins, if any, can be referenced here using a relative path. +// // It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the // the compiled file. // +// WARNING: THE FIRST BLANK LINE MARKS THE END OF WHAT'S TO BE PROCESSED, ANY BLANK LINE SHOULD +// GO AFTER THE REQUIRES BELOW. +// <% unless options[:skip_javascript] -%> //= require <%= options[:javascript] %> //= require <%= options[:javascript] %>_ujs diff --git a/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css b/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css index fc25b5723f..9e07c7d9a9 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css +++ b/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css @@ -1,7 +1,13 @@ /* - * This is a manifest file that'll automatically include all the stylesheets available in this directory - * and any sub-directories. You're free to add application-wide styles to this file and they'll appear at - * the top of the compiled file, but it's generally better to create a new file per style scope. + * This is a manifest file that'll be compiled into application.css, which will include all the files + * listed below. + * + * Any CSS and SCSS file within this directory, lib/assets/stylesheets, vendor/assets/stylesheets, + * or vendor/assets/stylesheets of plugins, if any, can be referenced here using a relative path. + * + * You're free to add application-wide styles to this file and they'll appear at the top of the + * compiled file, but it's generally better to create a new file per style scope. + * *= require_self *= require_tree . */
\ No newline at end of file diff --git a/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb b/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb index 00f90f9498..11867a4cd7 100644 --- a/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb +++ b/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb @@ -118,7 +118,7 @@ task :default => :test return if options.skip_javascript? if mountable? - copy_file "#{app_templates_dir}/app/assets/javascripts/application.js.tt", + template "#{app_templates_dir}/app/assets/javascripts/application.js.tt", "app/assets/javascripts/application.js" elsif full? empty_directory_with_gitkeep "app/assets/javascripts" diff --git a/railties/lib/rails/generators/rails/plugin_new/templates/Gemfile b/railties/lib/rails/generators/rails/plugin_new/templates/Gemfile index 29900c93dc..c28e568711 100644 --- a/railties/lib/rails/generators/rails/plugin_new/templates/Gemfile +++ b/railties/lib/rails/generators/rails/plugin_new/templates/Gemfile @@ -6,6 +6,10 @@ source "http://rubygems.org" <%= database_gemfile_entry -%> <% end -%> +<% if mountable? -%> +<%= gem_for_javascript -%> +<% end -%> + if RUBY_VERSION < '1.9' gem "ruby-debug", ">= 0.10.3" end diff --git a/railties/lib/rails/tasks/assets.rake b/railties/lib/rails/tasks/assets.rake index 5d2f02af13..5f87edc9e2 100644 --- a/railties/lib/rails/tasks/assets.rake +++ b/railties/lib/rails/tasks/assets.rake @@ -7,4 +7,12 @@ namespace :assets do assets = Rails.application.config.assets.precompile Rails.application.assets.precompile(*assets) end + + desc "Remove compiled assets" + task :clean => :environment do + assets = Rails.application.config.assets + public_asset_path = Rails.public_path + assets.prefix + file_list = FileList.new("#{public_asset_path}/*.js", "#{public_asset_path}/*.css") + file_list.each{ |file| rm file } + end end diff --git a/railties/test/generators/plugin_new_generator_test.rb b/railties/test/generators/plugin_new_generator_test.rb index 528c73ffaf..b272638026 100644 --- a/railties/test/generators/plugin_new_generator_test.rb +++ b/railties/test/generators/plugin_new_generator_test.rb @@ -112,6 +112,28 @@ class PluginNewGeneratorTest < Rails::Generators::TestCase assert_file "app/assets/javascripts/application.js" end + def test_jquery_is_the_default_javascript_library + run_generator [destination_root, "--mountable"] + assert_file "app/assets/javascripts/application.js" do |contents| + assert_match %r{^//= require jquery}, contents + assert_match %r{^//= require jquery_ujs}, contents + end + assert_file 'Gemfile' do |contents| + assert_match(/^gem 'jquery-rails'/, contents) + end + end + + def test_other_javascript_libraries + run_generator [destination_root, "--mountable", '-j', 'prototype'] + assert_file "app/assets/javascripts/application.js" do |contents| + assert_match %r{^//= require prototype}, contents + assert_match %r{^//= require prototype_ujs}, contents + end + assert_file 'Gemfile' do |contents| + assert_match(/^gem 'prototype-rails'/, contents) + end + end + def test_skip_javascripts run_generator [destination_root, "--skip-javascript", "--mountable"] assert_no_file "app/assets/javascripts/application.js" |