diff options
52 files changed, 555 insertions, 345 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 0af79e83cb..48ba1518e0 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,10 @@ ## Rails 4.0.0 (unreleased) ## +* `date_select` helper accepts `with_css_classes: true` to add css classes similar with type + of generated select tags. + + *Pavel Nikitin* + * Only non-js/css under app/assets path will be included in default config.assets.precompile. *Josh Peek* diff --git a/actionpack/lib/action_controller/metal/hide_actions.rb b/actionpack/lib/action_controller/metal/hide_actions.rb index 420b22cf56..2aa6b7adaf 100644 --- a/actionpack/lib/action_controller/metal/hide_actions.rb +++ b/actionpack/lib/action_controller/metal/hide_actions.rb @@ -26,20 +26,14 @@ module ActionController self.hidden_actions = hidden_actions.dup.merge(args.map(&:to_s)).freeze end - def inherited(klass) - klass.class_eval { @visible_actions = {} } - super - end - def visible_action?(action_name) - return @visible_actions[action_name] if @visible_actions.key?(action_name) - @visible_actions[action_name] = !hidden_actions.include?(action_name) + action_methods.include?(action_name) end # Overrides AbstractController::Base#action_methods to remove any methods # that are listed as hidden methods. def action_methods - @action_methods ||= Set.new(super.reject { |name| hidden_actions.include?(name) }) + @action_methods ||= Set.new(super.reject { |name| hidden_actions.include?(name) }).freeze end end end diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 47cf41cfa3..4a7df6b657 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -1,3 +1,4 @@ +require 'mutex_m' require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/object/duplicable' @@ -20,9 +21,18 @@ module ActionDispatch # end # => reverses the value to all keys matching /secret/i module FilterParameters - extend ActiveSupport::Concern + @@parameter_filter_for = {}.extend(Mutex_m) - @@parameter_filter_for = {} + ENV_MATCH = [/RAW_POST_DATA/, "rack.request.form_vars"] # :nodoc: + NULL_PARAM_FILTER = ParameterFilter.new # :nodoc: + NULL_ENV_FILTER = ParameterFilter.new ENV_MATCH # :nodoc: + + def initialize(env) + super + @filtered_parameters = nil + @filtered_env = nil + @filtered_path = nil + end # Return a hash of parameters with all sensitive data replaced. def filtered_parameters @@ -42,15 +52,24 @@ module ActionDispatch protected def parameter_filter - parameter_filter_for(@env["action_dispatch.parameter_filter"]) + parameter_filter_for @env.fetch("action_dispatch.parameter_filter") { + return NULL_PARAM_FILTER + } end def env_filter - parameter_filter_for(Array(@env["action_dispatch.parameter_filter"]) + [/RAW_POST_DATA/, "rack.request.form_vars"]) + user_key = @env.fetch("action_dispatch.parameter_filter") { + return NULL_ENV_FILTER + } + parameter_filter_for(Array(user_key) + ENV_MATCH) end def parameter_filter_for(filters) - @@parameter_filter_for[filters] ||= ParameterFilter.new(filters) + @@parameter_filter_for.synchronize do + # Do we *actually* need this cache? Constructing ParameterFilters + # doesn't seem too expensive. + @@parameter_filter_for[filters] ||= ParameterFilter.new(filters) + end end KV_RE = '[^&;=]+' diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index a3bb25f75a..dc04d4577b 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -1,32 +1,39 @@ module ActionDispatch module Http - class Headers < ::Hash - @@env_cache = Hash.new { |h,k| h[k] = "HTTP_#{k.upcase.gsub(/-/, '_')}" } + class Headers + include Enumerable - def initialize(*args) - - if args.size == 1 && args[0].is_a?(Hash) - super() - update(args[0]) - else - super - end + def initialize(env = {}) + @headers = env end def [](header_name) - super env_name(header_name) + @headers[env_name(header_name)] + end + + def []=(k,v); @headers[k] = v; end + def key?(k); @headers.key? k; end + alias :include? :key? + + def fetch(header_name, *args, &block) + @headers.fetch env_name(header_name), *args, &block end - def fetch(header_name, default=nil, &block) - super env_name(header_name), default, &block + def each(&block) + @headers.each(&block) end private - # Converts a HTTP header name to an environment variable name if it is - # not contained within the headers hash. - def env_name(header_name) - include?(header_name) ? header_name : @@env_cache[header_name] - end + + # Converts a HTTP header name to an environment variable name if it is + # not contained within the headers hash. + def env_name(header_name) + @headers.include?(header_name) ? header_name : cgi_name(header_name) + end + + def cgi_name(k) + "HTTP_#{k.upcase.gsub(/-/, '_')}" + end end end end diff --git a/actionpack/lib/action_dispatch/http/parameter_filter.rb b/actionpack/lib/action_dispatch/http/parameter_filter.rb index 490b46c990..b655a54865 100644 --- a/actionpack/lib/action_dispatch/http/parameter_filter.rb +++ b/actionpack/lib/action_dispatch/http/parameter_filter.rb @@ -1,74 +1,72 @@ module ActionDispatch module Http class ParameterFilter + FILTERED = '[FILTERED]'.freeze # :nodoc: - def initialize(filters) + def initialize(filters = []) @filters = filters end def filter(params) - if enabled? - compiled_filter.call(params) - else - params.dup - end + compiled_filter.call(params) end private - def enabled? - @filters.present? + def compiled_filter + @compiled_filter ||= CompiledFilter.compile(@filters) end - FILTERED = '[FILTERED]'.freeze + class CompiledFilter # :nodoc: + def self.compile(filters) + return lambda { |params| params.dup } if filters.empty? - def compiled_filter - @compiled_filter ||= begin - regexps, blocks = compile_filter + strings, regexps, blocks = [], [], [] - lambda do |original_params| - filtered_params = {} + filters.each do |item| + case item + when Proc + blocks << item + when Regexp + regexps << item + else + strings << item.to_s + end + end - original_params.each do |key, value| - if regexps.find { |r| key =~ r } - value = FILTERED - elsif value.is_a?(Hash) - value = filter(value) - elsif value.is_a?(Array) - value = value.map { |v| v.is_a?(Hash) ? filter(v) : v } - elsif blocks.present? - key = key.dup - value = value.dup if value.duplicable? - blocks.each { |b| b.call(key, value) } - end + regexps << Regexp.new(strings.join('|'), true) unless strings.empty? + new regexps, blocks + end - filtered_params[key] = value - end + attr_reader :regexps, :blocks - filtered_params - end + def initialize(regexps, blocks) + @regexps = regexps + @blocks = blocks end - end - def compile_filter - strings, regexps, blocks = [], [], [] + def call(original_params) + filtered_params = {} + + original_params.each do |key, value| + if regexps.any? { |r| key =~ r } + value = FILTERED + elsif value.is_a?(Hash) + value = call(value) + elsif value.is_a?(Array) + value = value.map { |v| v.is_a?(Hash) ? call(v) : v } + elsif blocks.any? + key = key.dup + value = value.dup if value.duplicable? + blocks.each { |b| b.call(key, value) } + end - @filters.each do |item| - case item - when NilClass - when Proc - blocks << item - when Regexp - regexps << item - else - strings << item.to_s + filtered_params[key] = value end - end - regexps << Regexp.new(strings.join('|'), true) unless strings.empty? - [regexps, blocks] + filtered_params + end end - end end end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index b8ebeb408f..fc8825d6d9 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -70,7 +70,13 @@ module ActionDispatch RFC5789 = %w(PATCH) HTTP_METHODS = RFC2616 + RFC2518 + RFC3253 + RFC3648 + RFC3744 + RFC5323 + RFC5789 - HTTP_METHOD_LOOKUP = Hash.new { |h, m| h[m] = m.underscore.to_sym if HTTP_METHODS.include?(m) } + + HTTP_METHOD_LOOKUP = {} + + # Populate the HTTP method lookup cache + HTTP_METHODS.each { |method| + HTTP_METHOD_LOOKUP[method] = method.underscore.to_sym + } # Returns the HTTP \method that the application should see. # In the case where the \method was overridden by a middleware diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 387dfeab17..f43d20c6ed 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -73,13 +73,17 @@ module ActionView options[:include_seconds] ||= !!include_seconds_or_options end + options = { + scope: :'datetime.distance_in_words' + }.merge!(options) + from_time = from_time.to_time if from_time.respond_to?(:to_time) to_time = to_time.to_time if to_time.respond_to?(:to_time) from_time, to_time = to_time, from_time if from_time > to_time distance_in_minutes = ((to_time - from_time)/60.0).round distance_in_seconds = (to_time - from_time).round - I18n.with_options :locale => options[:locale], :scope => :'datetime.distance_in_words' do |locale| + I18n.with_options :locale => options[:locale], :scope => options[:scope] do |locale| case distance_in_minutes when 0..1 return distance_in_minutes == 0 ? @@ -196,6 +200,8 @@ module ActionView # for <tt>:year</tt>, <tt>:month</tt>, <tt>:day</tt>, <tt>:hour</tt>, <tt>:minute</tt> and <tt>:second</tt>. # Setting this option prepends a select option with a generic prompt (Day, Month, Year, Hour, Minute, Seconds) # or the given prompt string. + # * <tt>:with_css_classes</tt> - Set to true if you want assign different styles for 'select' tags. This option + # automatically set classes 'year', 'month', 'day', 'hour', 'minute' and 'second' for your 'select' tags. # # If anything is passed in the +html_options+ hash it will be applied to every select tag in the set. # @@ -937,6 +943,7 @@ module ActionView :name => input_name_from_type(type) }.merge!(@html_options) select_options[:disabled] = 'disabled' if @options[:disabled] + select_options[:class] = type if @options[:with_css_classes] select_html = "\n" select_html << content_tag(:option, '', :value => '') + "\n" if @options[:include_blank] diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index bc7cad8db5..42432510c3 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -7,6 +7,26 @@ class HeaderTest < ActiveSupport::TestCase ) end + def test_each + headers = [] + @headers.each { |pair| headers << pair } + assert_equal [["HTTP_CONTENT_TYPE", "text/plain"]], headers + end + + def test_setter + @headers['foo'] = "bar" + assert_equal "bar", @headers['foo'] + end + + def test_key? + assert @headers.key?('HTTP_CONTENT_TYPE') + assert @headers.include?('HTTP_CONTENT_TYPE') + end + + def test_fetch_with_block + assert_equal 'omg', @headers.fetch('notthere') { 'omg' } + end + test "content type" do assert_equal "text/plain", @headers["Content-Type"] assert_equal "text/plain", @headers["content-type"] diff --git a/actionpack/test/template/date_helper_i18n_test.rb b/actionpack/test/template/date_helper_i18n_test.rb index 63066d40cd..495a9d3f9d 100644 --- a/actionpack/test/template/date_helper_i18n_test.rb +++ b/actionpack/test/template/date_helper_i18n_test.rb @@ -36,16 +36,13 @@ class DateHelperDistanceOfTimeInWordsI18nTests < ActiveSupport::TestCase end end - def assert_distance_of_time_in_words_translates_key(passed, expected) - diff, passed_options = *passed - key, count = *expected - to = @from + diff - - options = {:locale => 'en', :scope => :'datetime.distance_in_words'} - options[:count] = count if count - - I18n.expects(:t).with(key, options) - distance_of_time_in_words(@from, to, passed_options.merge(:locale => 'en')) + def test_distance_of_time_in_words_calls_i18n_with_custom_scope + { + [30.days, { scope: :'datetime.distance_in_words_ago' }] => [:'about_x_months', 1], + [60.days, { scope: :'datetime.distance_in_words_ago' }] => [:'x_months', 2], + }.each do |passed, expected| + assert_distance_of_time_in_words_translates_key(passed, expected, scope: :'datetime.distance_in_words_ago') + end end def test_time_ago_in_words_passes_locale @@ -74,6 +71,18 @@ class DateHelperDistanceOfTimeInWordsI18nTests < ActiveSupport::TestCase assert_equal expected, I18n.t(key, :count => count, :scope => 'datetime.distance_in_words') end end + + def assert_distance_of_time_in_words_translates_key(passed, expected, expected_options = {}) + diff, passed_options = *passed + key, count = *expected + to = @from + diff + + options = { locale: 'en', scope: :'datetime.distance_in_words' }.merge!(expected_options) + options[:count] = count if count + + I18n.expects(:t).with(key, options) + distance_of_time_in_words(@from, to, passed_options.merge(locale: 'en')) + end end class DateHelperSelectTagsI18nTests < ActiveSupport::TestCase diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index a4da7cd4b0..8bd8eff3c0 100644 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -1007,6 +1007,22 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, select_date(Time.mktime(2003, 8, 16), { :date_separator => " / ", :prefix => "date[first]", :use_hidden => true }) end + def test_select_date_with_css_classes_option + expected = %(<select id="date_first_year" name="date[first][year]" class="year">\n) + expected << %(<option value="2003" selected="selected">2003</option>\n<option value="2004">2004</option>\n<option value="2005">2005</option>\n) + expected << "</select>\n" + + expected << %(<select id="date_first_month" name="date[first][month]" class="month">\n) + expected << %(<option value="1">January</option>\n<option value="2">February</option>\n<option value="3">March</option>\n<option value="4">April</option>\n<option value="5">May</option>\n<option value="6">June</option>\n<option value="7">July</option>\n<option value="8" selected="selected">August</option>\n<option value="9">September</option>\n<option value="10">October</option>\n<option value="11">November</option>\n<option value="12">December</option>\n) + expected << "</select>\n" + + expected << %(<select id="date_first_day" name="date[first][day]" class="day">\n) + expected << %(<option value="1">1</option>\n<option value="2">2</option>\n<option value="3">3</option>\n<option value="4">4</option>\n<option value="5">5</option>\n<option value="6">6</option>\n<option value="7">7</option>\n<option value="8">8</option>\n<option value="9">9</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16" selected="selected">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n<option value="24">24</option>\n<option value="25">25</option>\n<option value="26">26</option>\n<option value="27">27</option>\n<option value="28">28</option>\n<option value="29">29</option>\n<option value="30">30</option>\n<option value="31">31</option>\n) + expected << "</select>\n" + + assert_dom_equal expected, select_date(Time.mktime(2003, 8, 16), {:start_year => 2003, :end_year => 2005, :prefix => "date[first]", :with_css_classes => true}) + end + def test_select_datetime expected = %(<select id="date_first_year" name="date[first][year]">\n) expected << %(<option value="2003" selected="selected">2003</option>\n<option value="2004">2004</option>\n<option value="2005">2005</option>\n) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 2c966943ee..aa42bf762f 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -76,6 +76,6 @@ * Trim down Active Model API by removing `valid?` and `errors.full_messages` *José Valim* -* When `^` or `$` are used in the regular expression provided to `validates_format_of` and the :multiline option is not set to true, an exception will be raised. This is to prevent security vulnerabilities when using `validates_format_of`. The problem is described in detail in the Rails security guide. +* When `^` or `$` are used in the regular expression provided to `validates_format_of` and the :multiline option is not set to true, an exception will be raised. This is to prevent security vulnerabilities when using `validates_format_of`. The problem is described in detail in the Rails security guide *Jan Berdajs + Egor Homakov* Please check [3-2-stable](https://github.com/rails/rails/blob/3-2-stable/activemodel/CHANGELOG.md) for previous changes. diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 243d911f71..9e6ec17ce1 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -3,6 +3,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/hash/except' require 'active_model/errors' require 'active_model/validations/callbacks' +require 'active_model/validator' module ActiveModel @@ -233,7 +234,7 @@ module ActiveModel # # person = Person.new # person.valid? # => false - # person.errors # => #<ActiveModel::Errors:0x007fe603816640 @messages={:name=>["can't be blank"]}> + # person.errors # => #<ActiveModel::Errors:0x007fe603816640 @messages={:name=>["can't be blank"]}> def errors @errors ||= Errors.new(self) end @@ -250,7 +251,7 @@ module ActiveModel # # person = Person.new # person.name = '' - # person.valid? # => false + # person.valid? # => false # person.name = 'david' # person.valid? # => true # @@ -265,7 +266,7 @@ module ActiveModel # end # # person = Person.new - # person.valid? # => true + # person.valid? # => true # person.valid?(:new) # => false def valid?(context = nil) current_context, self.validation_context = validation_context, context @@ -287,7 +288,7 @@ module ActiveModel # # person = Person.new # person.name = '' - # person.invalid? # => true + # person.invalid? # => true # person.name = 'david' # person.invalid? # => false # @@ -302,7 +303,7 @@ module ActiveModel # end # # person = Person.new - # person.invalid? # => false + # person.invalid? # => false # person.invalid?(:new) # => true def invalid?(context = nil) !valid?(context) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 186c7bf244..b4831c1fac 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,43 @@ ## Rails 4.0.0 (unreleased) ## +* Add `find_or_create_by`, `find_or_create_by!` and + `find_or_initialize_by` methods to `Relation`. + + These are similar to the `first_or_create` family of methods, but + the behaviour when a record is created is slightly different: + + User.where(first_name: 'Penélope').first_or_create + + will execute: + + User.where(first_name: 'Penélope').create + + Causing all the `create` callbacks to execute within the context of + the scope. This could affect queries that occur within callbacks. + + User.find_or_create_by(first_name: 'Penélope') + + will execute: + + User.create(first_name: 'Penélope') + + Which obviously does not affect the scoping of queries within + callbacks. + + The `find_or_create_by` version also reads better, frankly. + + If you need to add extra attributes during create, you can do one of: + + User.create_with(active: true).find_or_create_by(first_name: 'Jon') + User.find_or_create_by(first_name: 'Jon') { |u| u.active = true } + + The `first_or_create` family of methods have been nodoc'ed in favour + of this API. They may be deprecated in the future but their + implementation is very small and it's probably not worth putting users + through lots of annoying deprecation warnings. + + *Jon Leighton* + * Fix bug with presence validation of associations. Would incorrectly add duplicated errors when the association was blank. Bug introduced in 1fab518c6a75dac5773654646eb724a59741bc13. @@ -74,6 +112,10 @@ app processes (so long as the code in those processes doesn't contain any references to the removed column). + The `partial_updates` configuration option is now renamed to + `partial_writes` to reflect the fact that it now impacts both inserts + and updates. + *Jon Leighton* * Allow before and after validations to take an array of lifecycle events @@ -607,9 +649,9 @@ * `find_or_initialize_by_...` can be rewritten using `where(...).first_or_initialize` * `find_or_create_by_...` can be rewritten using - `where(...).first_or_create` + `find_or_create_by(...)` or where(...).first_or_create` * `find_or_create_by_...!` can be rewritten using - `where(...).first_or_create!` + `find_or_create_by!(...) or `where(...).first_or_create!` The implementation of the deprecated dynamic finders has been moved to the `activerecord-deprecated_finders` gem. See below for details. @@ -903,7 +945,14 @@ * Plugins & libraries etc that add methods to `ActiveRecord::Base` will not be compatible with `ActiveRecord::Model`. Those libraries should add to `ActiveRecord::Model` instead (which is included in - `Base`), or better still, avoid monkey-patching AR and instead + `Base`). This should be done using the `:active_record_model` + load hook, which executes before `ActiveRecord::Base` loads: + + ActiveSupport.on_load(:active_record_model) do + include MyPlugin + end + + Or better still, avoid monkey-patching AR and instead provide a module that users can include where they need it. * To minimise the risk of conflicts with other code, it is diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 7a5bb9e863..b4d227eeee 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -1,9 +1,10 @@ require 'active_support/core_ext/module/attribute_accessors' +require 'active_support/deprecation' module ActiveRecord - ActiveSupport.on_load(:active_record_config) do - mattr_accessor :partial_updates, instance_accessor: false - self.partial_updates = true + ActiveSupport.on_load(:active_record_model) do + mattr_accessor :partial_writes, instance_accessor: false + self.partial_writes = true end module AttributeMethods @@ -17,7 +18,18 @@ module ActiveRecord raise "You cannot include Dirty after Timestamp" end - config_attribute :partial_updates + config_attribute :partial_writes + + def self.partial_updates=(v); self.partial_writes = v; end + def self.partial_updates?; partial_writes?; end + def self.partial_updates; partial_writes; end + + ActiveSupport::Deprecation.deprecate_methods( + singleton_class, + :partial_updates= => :partial_writes=, + :partial_updates? => :partial_writes?, + :partial_updates => :partial_writes + ) end # Attempts to +save+ the record and clears changed attributes if successful. @@ -64,26 +76,16 @@ module ActiveRecord end def update(*) - partial_updates? ? super(keys_for_partial_update) : super + partial_writes? ? super(keys_for_partial_write) : super end def create(*) - if partial_updates? - keys = keys_for_partial_update - - # This is an extremely bloody annoying necessity to work around mysql being crap. - # See test_mysql_text_not_null_defaults - keys.concat self.class.columns.select(&:explicit_default?).map(&:name) - - super keys - else - super - end + partial_writes? ? super(keys_for_partial_write) : super end # Serialized attributes should always be written in case they've been # changed in place. - def keys_for_partial_update + def keys_for_partial_write changed | (attributes.keys & self.class.serialized_attributes.keys) end diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 46fd6ebfb3..1b1e732eb1 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -1,5 +1,5 @@ module ActiveRecord - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do mattr_accessor :attribute_types_cached_by_default, instance_accessor: false end 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 f36a5806a9..95bbf68433 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -1,6 +1,6 @@ module ActiveRecord - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do mattr_accessor :time_zone_aware_attributes, instance_accessor: false self.time_zone_aware_attributes = false diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index a4705b24ca..258c222f7b 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -323,6 +323,6 @@ module ActiveRecord #:nodoc: class Base include ActiveRecord::Model end -end -ActiveSupport.run_load_hooks(:active_record, ActiveRecord::Model::DeprecationProxy.new) + ActiveSupport.run_load_hooks(:active_record, Base) +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 8c83c4f5db..b0e7bd7e82 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -4,17 +4,19 @@ module ActiveRecord module ConnectionAdapters class AbstractMysqlAdapter < AbstractAdapter class Column < ConnectionAdapters::Column # :nodoc: - attr_reader :collation + attr_reader :collation, :strict - def initialize(name, default, sql_type = nil, null = true, collation = nil) - super(name, default, sql_type, null) + def initialize(name, default, sql_type = nil, null = true, collation = nil, strict = false) + @strict = strict @collation = collation + + super(name, default, sql_type, null) end def extract_default(default) if sql_type =~ /blob/i || type == :text if default.blank? - return null ? nil : '' + null || strict ? nil : '' else raise ArgumentError, "#{type} columns cannot have a default value: #{default.inspect}" end @@ -30,10 +32,6 @@ module ActiveRecord super end - def explicit_default? - !null && (sql_type =~ /blob/i || type == :text) - end - # Must return the relevant concrete adapter def adapter raise NotImplementedError @@ -571,6 +569,10 @@ module ActiveRecord where_sql end + def strict_mode? + @config.fetch(:strict, true) + end + protected # MySQL is too stupid to create a temporary table for use subquery, so we have diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index 2028abf6f0..816b5e17c1 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -53,10 +53,6 @@ module ActiveRecord !default.nil? end - def explicit_default? - false - end - # Returns the Ruby class that corresponds to the abstract data type. def klass case type diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 328d080687..879eec7fcf 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -53,7 +53,7 @@ module ActiveRecord end def new_column(field, default, type, null, collation) # :nodoc: - Column.new(field, default, type, null, collation) + Column.new(field, default, type, null, collation, strict_mode?) end def error_number(exception) @@ -259,9 +259,7 @@ module ActiveRecord # Make MySQL reject illegal values rather than truncating or # blanking them. See # http://dev.mysql.com/doc/refman/5.5/en/server-sql-mode.html#sqlmode_strict_all_tables - if @config.fetch(:strict, true) - variable_assignments << "SQL_MODE='STRICT_ALL_TABLES'" - end + variable_assignments << "SQL_MODE='STRICT_ALL_TABLES'" if strict_mode? encoding = @config[:encoding] diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 0b936bbf39..76667616a1 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -150,7 +150,7 @@ module ActiveRecord end def new_column(field, default, type, null, collation) # :nodoc: - Column.new(field, default, type, null, collation) + Column.new(field, default, type, null, collation, strict_mode?) end def error_number(exception) # :nodoc: @@ -546,9 +546,7 @@ module ActiveRecord # Make MySQL reject illegal values rather than truncating or # blanking them. See # http://dev.mysql.com/doc/refman/5.5/en/server-sql-mode.html#sqlmode_strict_all_tables - if @config.fetch(:strict, true) - execute("SET SQL_MODE='STRICT_ALL_TABLES'", :skip_logging) - end + execute("SET SQL_MODE='STRICT_ALL_TABLES'", :skip_logging) if strict_mode? end def select(sql, name = nil, binds = []) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index f97c363871..f1e65f538b 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -3,7 +3,7 @@ require 'active_support/core_ext/object/duplicable' require 'thread' module ActiveRecord - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do ## # :singleton-method: # diff --git a/activerecord/lib/active_record/explain.rb b/activerecord/lib/active_record/explain.rb index 9e0390bed1..72e09945b5 100644 --- a/activerecord/lib/active_record/explain.rb +++ b/activerecord/lib/active_record/explain.rb @@ -1,7 +1,7 @@ require 'active_support/lazy_load_hooks' module ActiveRecord - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do mattr_accessor :auto_explain_threshold_in_seconds, instance_accessor: false end diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 35273b0d81..68e5b1dafa 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -1,6 +1,6 @@ module ActiveRecord - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do # Determine whether to store the full constant name including namespace when using STI mattr_accessor :store_full_sti_class, instance_accessor: false self.store_full_sti_class = true diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml index 896132d566..b1fbd38622 100644 --- a/activerecord/lib/active_record/locale/en.yml +++ b/activerecord/lib/active_record/locale/en.yml @@ -4,11 +4,15 @@ en: #created_at: "Created at" #updated_at: "Updated at" + # Default error messages + errors: + messages: + taken: "has already been taken" + # Active Record models configuration activerecord: errors: messages: - taken: "has already been taken" record_invalid: "Validation failed: %{errors}" restrict_dependent_destroy: one: "Cannot delete record because a dependent %{record} exists" diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index e96ed00f9c..81195cb355 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -1,5 +1,5 @@ module ActiveRecord - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do mattr_accessor :lock_optimistically, instance_accessor: false self.lock_optimistically = true end diff --git a/activerecord/lib/active_record/model.rb b/activerecord/lib/active_record/model.rb index f059840f4d..5fcb0359c5 100644 --- a/activerecord/lib/active_record/model.rb +++ b/activerecord/lib/active_record/model.rb @@ -115,52 +115,21 @@ module ActiveRecord 'type' end end - - class DeprecationProxy < BasicObject #:nodoc: - def initialize(model = Model, base = Base) - @model = model - @base = base - end - - def method_missing(name, *args, &block) - if @model.respond_to?(name, true) - @model.send(name, *args, &block) - else - ::ActiveSupport::Deprecation.warn( - "The object passed to the active_record load hook was previously ActiveRecord::Base " \ - "(a Class). Now it is ActiveRecord::Model (a Module). You have called `#{name}' which " \ - "is only defined on ActiveRecord::Base. Please change your code so that it works with " \ - "a module rather than a class. (Model is included in Base, so anything added to Model " \ - "will be available on Base as well.)" - ) - @base.send(name, *args, &block) - end - end - - alias send method_missing - - def extend(*mods) - ::ActiveSupport::Deprecation.warn( - "The object passed to the active_record load hook was previously ActiveRecord::Base " \ - "(a Class). Now it is ActiveRecord::Model (a Module). You have called `extend' which " \ - "would add singleton methods to Model. This is presumably not what you want, since the " \ - "methods would not be inherited down to Base. Rather than using extend, please use " \ - "ActiveSupport::Concern + include, which will ensure that your class methods are " \ - "inherited." - ) - @base.extend(*mods) - end - end end - # This hook is where config accessors on Model get defined. + # This hook is where config accessors on Model should be defined. # # We don't want to just open the Model module and add stuff to it in other files, because # that would cause Model to load, which causes all sorts of loading order issues. # # We need this hook rather than just using the :active_record one, because users of the # :active_record hook may need to use config options. - ActiveSupport.run_load_hooks(:active_record_config, Model) + # + # Users who wish to include a module in Model that they want to also + # get inherited by Base should do so using this load hook. After Base + # has included Model, any modules subsequently included in Model won't + # be inherited by Base. + ActiveSupport.run_load_hooks(:active_record_model, Model) # Load Base at this point, because the active_record load hook is run in that file. Base diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 99de16cd33..93bd21d7e9 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -1,6 +1,6 @@ module ActiveRecord - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do mattr_accessor :primary_key_prefix_type, instance_accessor: false mattr_accessor :table_name_prefix, instance_accessor: false diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 2e7fb3bbb3..4d1ff98eb3 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -3,7 +3,7 @@ require 'active_support/core_ext/object/try' require 'active_support/core_ext/hash/indifferent_access' module ActiveRecord - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do mattr_accessor :nested_attributes_options, instance_accessor: false self.nested_attributes_options = {} end diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 13e09eda53..45f6a78428 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -3,6 +3,7 @@ module ActiveRecord module Querying delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, :any?, :many?, :to => :all delegate :first_or_create, :first_or_create!, :first_or_initialize, :to => :all + delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, :to => :all delegate :find_by, :find_by!, :to => :all delegate :destroy, :destroy_all, :delete, :delete_all, :update, :update_all, :to => :all delegate :find_each, :find_in_batches, :to => :all diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index ecce7c703b..2e2286e4fd 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -129,46 +129,53 @@ module ActiveRecord scoping { @klass.create!(*args, &block) } end - # Tries to load the first record; if it fails, then <tt>create</tt> is called with the same arguments as this method. - # - # Expects arguments in the same format as +Base.create+. + def first_or_create(attributes = nil, &block) # :nodoc: + first || create(attributes, &block) + end + + def first_or_create!(attributes = nil, &block) # :nodoc: + first || create!(attributes, &block) + end + + def first_or_initialize(attributes = nil, &block) # :nodoc: + first || new(attributes, &block) + end + + # Finds the first record with the given attributes, or creates a record with the attributes + # if one is not found. # # ==== Examples # # Find the first user named Penélope or create a new one. - # User.where(:first_name => 'Penélope').first_or_create + # User.find_or_create_by(first_name: 'Penélope') # # => <User id: 1, first_name: 'Penélope', last_name: nil> # # # Find the first user named Penélope or create a new one. # # We already have one so the existing record will be returned. - # User.where(:first_name => 'Penélope').first_or_create + # User.find_or_create_by(first_name: 'Penélope') # # => <User id: 1, first_name: 'Penélope', last_name: nil> # # # Find the first user named Scarlett or create a new one with a particular last name. - # User.where(:first_name => 'Scarlett').first_or_create(:last_name => 'Johansson') + # User.create_with(last_name: 'Johansson').find_or_create_by(first_name: 'Scarlett') # # => <User id: 2, first_name: 'Scarlett', last_name: 'Johansson'> # # # Find the first user named Scarlett or create a new one with a different last name. # # We already have one so the existing record will be returned. - # User.where(:first_name => 'Scarlett').first_or_create do |user| + # User.find_or_create_by(first_name: 'Scarlett') do |user| # user.last_name = "O'Hara" # end # # => <User id: 2, first_name: 'Scarlett', last_name: 'Johansson'> - def first_or_create(attributes = nil, &block) - first || create(attributes, &block) + def find_or_create_by(attributes, &block) + find_by(attributes) || create(attributes, &block) end - # Like <tt>first_or_create</tt> but calls <tt>create!</tt> so an exception is raised if the created record is invalid. - # - # Expects arguments in the same format as <tt>Base.create!</tt>. - def first_or_create!(attributes = nil, &block) - first || create!(attributes, &block) + # Like <tt>find_or_create_by</tt>, but calls <tt>create!</tt> so an exception is raised if the created record is invalid. + def find_or_create_by!(attributes, &block) + find_by(attributes) || create!(attributes, &block) end - # Like <tt>first_or_create</tt> but calls <tt>new</tt> instead of <tt>create</tt>. - # - # Expects arguments in the same format as <tt>Base.new</tt>. - def first_or_initialize(attributes = nil, &block) - first || new(attributes, &block) + # Like <tt>find_or_create_by</tt>, but calls <tt>new</tt> instead of <tt>create</tt>. + def find_or_initialize_by(attributes, &block) + find_by(attributes) || new(attributes, &block) end # Runs EXPLAIN on the query or queries triggered by this relation and diff --git a/activerecord/lib/active_record/serialization.rb b/activerecord/lib/active_record/serialization.rb index e8dd312a47..6bdf0db95a 100644 --- a/activerecord/lib/active_record/serialization.rb +++ b/activerecord/lib/active_record/serialization.rb @@ -1,5 +1,5 @@ module ActiveRecord #:nodoc: - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do mattr_accessor :include_root_in_json, instance_accessor: false self.include_root_in_json = true end diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index dd08a6f4f5..9fb5be7861 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -1,6 +1,6 @@ module ActiveRecord - ActiveSupport.on_load(:active_record_config) do + ActiveSupport.on_load(:active_record_model) do mattr_accessor :record_timestamps, instance_accessor: false self.record_timestamps = true end @@ -75,7 +75,7 @@ module ActiveRecord end def should_record_timestamps? - self.record_timestamps && (!partial_updates? || changed? || (attributes.keys & self.class.serialized_attributes.keys).present?) + self.record_timestamps && (!partial_writes? || changed? || (attributes.keys & self.class.serialized_attributes.keys).present?) end def timestamp_attributes_for_create_in_model diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 4afd33f273..16ce150396 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -144,7 +144,7 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas firm = Firm.first firm.account = Account.first - assert_queries(Firm.partial_updates? ? 0 : 1) { firm.save! } + assert_queries(Firm.partial_writes? ? 0 : 1) { firm.save! } firm = Firm.first.dup firm.account = Account.first diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index 72f1c99ca0..0df872ff10 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -51,11 +51,60 @@ if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter) # We don't want that to happen, so we disable transactional fixtures here. self.use_transactional_fixtures = false - # MySQL 5 and higher is quirky with not null text/blob columns. - # With MySQL Text/blob columns cannot have defaults. If the column is not - # null MySQL will report that the column has a null default - # but it behaves as though the column had a default of '' - def test_mysql_text_not_null_defaults + def using_strict(strict) + connection = ActiveRecord::Model.remove_connection + ActiveRecord::Model.establish_connection connection.merge(strict: strict) + yield + ensure + ActiveRecord::Model.remove_connection + ActiveRecord::Model.establish_connection connection + end + + # MySQL cannot have defaults on text/blob columns. It reports the + # default value as null. + # + # Despite this, in non-strict mode, MySQL will use an empty string + # as the default value of the field, if no other value is + # specified. + # + # Therefore, in non-strict mode, we want column.default to report + # an empty string as its default, to be consistent with that. + # + # In strict mode, column.default should be nil. + def test_mysql_text_not_null_defaults_non_strict + using_strict(false) do + with_text_blob_not_null_table do |klass| + assert_equal '', klass.columns_hash['non_null_blob'].default + assert_equal '', klass.columns_hash['non_null_text'].default + + assert_nil klass.columns_hash['null_blob'].default + assert_nil klass.columns_hash['null_text'].default + + instance = klass.create! + + assert_equal '', instance.non_null_text + assert_equal '', instance.non_null_blob + + assert_nil instance.null_text + assert_nil instance.null_blob + end + end + end + + def test_mysql_text_not_null_defaults_strict + using_strict(true) do + with_text_blob_not_null_table do |klass| + assert_nil klass.columns_hash['non_null_blob'].default + assert_nil klass.columns_hash['non_null_text'].default + assert_nil klass.columns_hash['null_blob'].default + assert_nil klass.columns_hash['null_text'].default + + assert_raises(ActiveRecord::StatementInvalid) { klass.create } + end + end + end + + def with_text_blob_not_null_table klass = Class.new(ActiveRecord::Base) klass.table_name = 'test_mysql_text_not_null_defaults' klass.connection.create_table klass.table_name do |t| @@ -64,19 +113,8 @@ if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter) t.column :null_text, :text, :null => true t.column :null_blob, :blob, :null => true end - assert_equal '', klass.columns_hash['non_null_blob'].default - assert_equal '', klass.columns_hash['non_null_text'].default - - assert_nil klass.columns_hash['null_blob'].default - assert_nil klass.columns_hash['null_text'].default - assert_nothing_raised do - instance = klass.create! - assert_equal '', instance.non_null_text - assert_equal '', instance.non_null_blob - assert_nil instance.null_text - assert_nil instance.null_blob - end + yield klass ensure klass.connection.drop_table(klass.table_name) rescue nil end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 7334514f9a..40f1dbccde 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -311,12 +311,12 @@ class DirtyTest < ActiveRecord::TestCase pirate = Pirate.new(:catchphrase => 'foo') old_updated_on = 1.hour.ago.beginning_of_day - with_partial_updates Pirate, false do + with_partial_writes Pirate, false do assert_queries(2) { 2.times { pirate.save! } } Pirate.where(id: pirate.id).update_all(:updated_on => old_updated_on) end - with_partial_updates Pirate, true do + with_partial_writes Pirate, true do assert_queries(0) { 2.times { pirate.save! } } assert_equal old_updated_on, pirate.reload.updated_on @@ -329,12 +329,12 @@ class DirtyTest < ActiveRecord::TestCase person = Person.new(:first_name => 'foo') old_lock_version = 1 - with_partial_updates Person, false do + with_partial_writes Person, false do assert_queries(2) { 2.times { person.save! } } Person.where(id: person.id).update_all(:first_name => 'baz') end - with_partial_updates Person, true do + with_partial_writes Person, true do assert_queries(0) { 2.times { person.save! } } assert_equal old_lock_version, person.reload.lock_version @@ -408,8 +408,8 @@ class DirtyTest < ActiveRecord::TestCase assert !pirate.catchphrase_changed? end - def test_save_should_store_serialized_attributes_even_with_partial_updates - with_partial_updates(Topic) do + def test_save_should_store_serialized_attributes_even_with_partial_writes + with_partial_writes(Topic) do topic = Topic.create!(:content => {:a => "a"}) topic.content[:b] = "b" #assert topic.changed? # Known bug, will fail @@ -421,7 +421,7 @@ class DirtyTest < ActiveRecord::TestCase end def test_save_always_should_update_timestamps_when_serialized_attributes_are_present - with_partial_updates(Topic) do + with_partial_writes(Topic) do topic = Topic.create!(:content => {:a => "a"}) topic.save! @@ -434,8 +434,8 @@ class DirtyTest < ActiveRecord::TestCase end end - def test_save_should_not_save_serialized_attribute_with_partial_updates_if_not_present - with_partial_updates(Topic) do + def test_save_should_not_save_serialized_attribute_with_partial_writes_if_not_present + with_partial_writes(Topic) do Topic.create!(:author_name => 'Bill', :content => {:a => "a"}) topic = Topic.select('id, author_name').first topic.update_columns author_name: 'John' @@ -552,7 +552,7 @@ class DirtyTest < ActiveRecord::TestCase end test "partial insert" do - with_partial_updates Person do + with_partial_writes Person do jon = nil assert_sql(/first_name/i) do jon = Person.create! first_name: 'Jon' @@ -568,20 +568,34 @@ class DirtyTest < ActiveRecord::TestCase end test "partial insert with empty values" do - with_partial_updates Aircraft do + with_partial_writes Aircraft do a = Aircraft.create! a.reload assert_not_nil a.id end end + test "partial_updates config attribute is deprecated" do + klass = Class.new(ActiveRecord::Base) + + assert klass.partial_writes? + assert_deprecated { assert klass.partial_updates? } + assert_deprecated { assert klass.partial_updates } + + assert_deprecated { klass.partial_updates = false } + + assert !klass.partial_writes? + assert_deprecated { assert !klass.partial_updates? } + assert_deprecated { assert !klass.partial_updates } + end + private - def with_partial_updates(klass, on = true) - old = klass.partial_updates? - klass.partial_updates = on + def with_partial_writes(klass, on = true) + old = klass.partial_writes? + klass.partial_writes = on yield ensure - klass.partial_updates = old + klass.partial_writes = old end def check_pirate_after_save_failure(pirate) diff --git a/activerecord/test/cases/inclusion_test.rb b/activerecord/test/cases/inclusion_test.rb index 8f095e4953..297c7a9923 100644 --- a/activerecord/test/cases/inclusion_test.rb +++ b/activerecord/test/cases/inclusion_test.rb @@ -82,42 +82,6 @@ class InclusionUnitTest < ActiveRecord::TestCase def test_included_twice @klass.send :include, ActiveRecord::Model end - - def test_deprecation_proxy - proxy = ActiveRecord::Model::DeprecationProxy.new - - assert_equal ActiveRecord::Model.name, proxy.name - assert_equal ActiveRecord::Base.superclass, assert_deprecated { proxy.superclass } - - sup, sup2 = nil, nil - ActiveSupport.on_load(:__test_active_record_model_deprecation) do - sup = superclass - sup2 = send(:superclass) - end - assert_deprecated do - ActiveSupport.run_load_hooks(:__test_active_record_model_deprecation, proxy) - end - assert_equal ActiveRecord::Base.superclass, sup - assert_equal ActiveRecord::Base.superclass, sup2 - end - - test "including in deprecation proxy" do - model, base = ActiveRecord::Model.dup, ActiveRecord::Base.dup - proxy = ActiveRecord::Model::DeprecationProxy.new(model, base) - - mod = Module.new - proxy.include mod - assert model < mod - end - - test "extending in deprecation proxy" do - model, base = ActiveRecord::Model.dup, ActiveRecord::Base.dup - proxy = ActiveRecord::Model::DeprecationProxy.new(model, base) - - mod = Module.new - assert_deprecated { proxy.extend mod } - assert base.singleton_class < mod - end end class InclusionFixturesTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 3c0d2b18d9..c155f29973 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -344,11 +344,7 @@ class MigrationTest < ActiveRecord::TestCase columns = Person.connection.columns(:binary_testings) data_column = columns.detect { |c| c.name == "data" } - if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter) - assert_equal '', data_column.default - else - assert_nil data_column.default - end + assert_nil data_column.default Person.connection.drop_table :binary_testings rescue nil end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index fdbc0a3fdb..5f96145b47 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1058,6 +1058,39 @@ class RelationTest < ActiveRecord::TestCase assert_equal 'parrot', parrot.name end + def test_find_or_create_by + assert_nil Bird.find_by(name: 'bob') + + bird = Bird.find_or_create_by(name: 'bob') + assert bird.persisted? + + assert_equal bird, Bird.find_or_create_by(name: 'bob') + end + + def test_find_or_create_by_with_create_with + assert_nil Bird.find_by(name: 'bob') + + bird = Bird.create_with(color: 'green').find_or_create_by(name: 'bob') + assert bird.persisted? + assert_equal 'green', bird.color + + assert_equal bird, Bird.create_with(color: 'blue').find_or_create_by(name: 'bob') + end + + def test_find_or_create_by! + assert_raises(ActiveRecord::RecordInvalid) { Bird.find_or_create_by!(color: 'green') } + end + + def test_find_or_initialize_by + assert_nil Bird.find_by(name: 'bob') + + bird = Bird.find_or_initialize_by(name: 'bob') + assert bird.new_record? + bird.save! + + assert_equal bird, Bird.find_or_initialize_by(name: 'bob') + end + def test_explicit_create_scope hens = Bird.where(:name => 'hen') assert_equal 'hen', hens.new.name diff --git a/activerecord/test/cases/validations/i18n_generate_message_validation_test.rb b/activerecord/test/cases/validations/i18n_generate_message_validation_test.rb index 2f5ee32538..174d96aa4e 100644 --- a/activerecord/test/cases/validations/i18n_generate_message_validation_test.rb +++ b/activerecord/test/cases/validations/i18n_generate_message_validation_test.rb @@ -54,4 +54,9 @@ class I18nGenerateMessageValidationTest < ActiveRecord::TestCase end end + test "translation for 'taken' can be overridden" do + I18n.backend.store_translations "en", {errors: {attributes: {title: {taken: "Custom taken message" }}}} + assert_equal "Custom taken message", @topic.errors.generate_message(:title, :taken, :value => 'title') + end + end diff --git a/activesupport/lib/active_support/descendants_tracker.rb b/activesupport/lib/active_support/descendants_tracker.rb index e2a8b4d4e3..27861e01d0 100644 --- a/activesupport/lib/active_support/descendants_tracker.rb +++ b/activesupport/lib/active_support/descendants_tracker.rb @@ -2,35 +2,50 @@ module ActiveSupport # This module provides an internal implementation to track descendants # which is faster than iterating through ObjectSpace. module DescendantsTracker - @@direct_descendants = Hash.new { |h, k| h[k] = [] } + @@direct_descendants = {} - def self.direct_descendants(klass) - @@direct_descendants[klass] - end + class << self + def direct_descendants(klass) + @@direct_descendants[klass] || [] + end - def self.descendants(klass) - @@direct_descendants[klass].inject([]) do |descendants, _klass| - descendants << _klass - descendants.concat _klass.descendants + def descendants(klass) + arr = [] + accumulate_descendants(klass, arr) + arr end - end - def self.clear - if defined? ActiveSupport::Dependencies - @@direct_descendants.each do |klass, descendants| - if ActiveSupport::Dependencies.autoloaded?(klass) - @@direct_descendants.delete(klass) - else - descendants.reject! { |v| ActiveSupport::Dependencies.autoloaded?(v) } + def clear + if defined? ActiveSupport::Dependencies + @@direct_descendants.each do |klass, descendants| + if ActiveSupport::Dependencies.autoloaded?(klass) + @@direct_descendants.delete(klass) + else + descendants.reject! { |v| ActiveSupport::Dependencies.autoloaded?(v) } + end end + else + @@direct_descendants.clear + end + end + + # This is the only method that is not thread safe, but is only ever called + # during the eager loading phase. + def store_inherited(klass, descendant) + (@@direct_descendants[klass] ||= []) << descendant + end + + private + def accumulate_descendants(klass, acc) + if direct_descendants = @@direct_descendants[klass] + acc.concat(direct_descendants) + direct_descendants.each { |direct_descendant| accumulate_descendants(direct_descendant, acc) } end - else - @@direct_descendants.clear end end def inherited(base) - self.direct_descendants << base + DescendantsTracker.store_inherited(self, base) super end diff --git a/activesupport/lib/active_support/testing/performance/ruby.rb b/activesupport/lib/active_support/testing/performance/ruby.rb index 12aef0d7fe..7c149df1e4 100644 --- a/activesupport/lib/active_support/testing/performance/ruby.rb +++ b/activesupport/lib/active_support/testing/performance/ruby.rb @@ -2,7 +2,7 @@ begin require 'ruby-prof' rescue LoadError $stderr.puts 'Specify ruby-prof as application\'s dependency in Gemfile to run benchmarks.' - exit + raise end module ActiveSupport diff --git a/activesupport/test/descendants_tracker_test_cases.rb b/activesupport/test/descendants_tracker_test_cases.rb index 066ec8549b..69e046998e 100644 --- a/activesupport/test/descendants_tracker_test_cases.rb +++ b/activesupport/test/descendants_tracker_test_cases.rb @@ -1,3 +1,5 @@ +require 'set' + module DescendantsTrackerTestCases class Parent extend ActiveSupport::DescendantsTracker @@ -18,15 +20,15 @@ module DescendantsTrackerTestCases ALL = [Parent, Child1, Child2, Grandchild1, Grandchild2] def test_descendants - assert_equal [Child1, Grandchild1, Grandchild2, Child2], Parent.descendants - assert_equal [Grandchild1, Grandchild2], Child1.descendants - assert_equal [], Child2.descendants + assert_equal_sets [Child1, Grandchild1, Grandchild2, Child2], Parent.descendants + assert_equal_sets [Grandchild1, Grandchild2], Child1.descendants + assert_equal_sets [], Child2.descendants end def test_direct_descendants - assert_equal [Child1, Child2], Parent.direct_descendants - assert_equal [Grandchild1, Grandchild2], Child1.direct_descendants - assert_equal [], Child2.direct_descendants + assert_equal_sets [Child1, Child2], Parent.direct_descendants + assert_equal_sets [Grandchild1, Grandchild2], Child1.direct_descendants + assert_equal_sets [], Child2.direct_descendants end def test_clear @@ -40,6 +42,10 @@ module DescendantsTrackerTestCases protected + def assert_equal_sets(expected, actual) + assert_equal Set.new(expected), Set.new(actual) + end + def mark_as_autoloaded(*klasses) # If ActiveSupport::Dependencies is not loaded, forget about autoloading. # This allows using AS::DescendantsTracker without AS::Dependencies. @@ -56,4 +62,4 @@ module DescendantsTrackerTestCases ActiveSupport::Dependencies.autoloaded_constants = old_autoloaded if defined? ActiveSupport::Dependencies ActiveSupport::DescendantsTracker.class_eval("@@direct_descendants").replace(old_descendants) end -end
\ No newline at end of file +end diff --git a/activesupport/test/descendants_tracker_with_autoloading_test.rb b/activesupport/test/descendants_tracker_with_autoloading_test.rb index 9180f1f977..ab1be296f8 100644 --- a/activesupport/test/descendants_tracker_with_autoloading_test.rb +++ b/activesupport/test/descendants_tracker_with_autoloading_test.rb @@ -18,17 +18,17 @@ class DescendantsTrackerWithAutoloadingTest < ActiveSupport::TestCase def test_clear_with_autoloaded_children_and_granchildren mark_as_autoloaded Child1, Grandchild1, Grandchild2 do ActiveSupport::DescendantsTracker.clear - assert_equal [Child2], Parent.descendants - assert_equal [], Child2.descendants + assert_equal_sets [Child2], Parent.descendants + assert_equal_sets [], Child2.descendants end end def test_clear_with_autoloaded_granchildren mark_as_autoloaded Grandchild1, Grandchild2 do ActiveSupport::DescendantsTracker.clear - assert_equal [Child1, Child2], Parent.descendants - assert_equal [], Child1.descendants - assert_equal [], Child2.descendants + assert_equal_sets [Child1, Child2], Parent.descendants + assert_equal_sets [], Child1.descendants + assert_equal_sets [], Child2.descendants end end end diff --git a/activesupport/test/testing/performance_test.rb b/activesupport/test/testing/performance_test.rb index 53073cb8db..6918110cce 100644 --- a/activesupport/test/testing/performance_test.rb +++ b/activesupport/test/testing/performance_test.rb @@ -1,10 +1,19 @@ require 'abstract_unit' -require 'active_support/testing/performance' - module ActiveSupport module Testing class PerformanceTest < ActiveSupport::TestCase + begin + require 'active_support/testing/performance' + HAVE_RUBYPROF = true + rescue LoadError + HAVE_RUBYPROF = false + end + + def setup + skip "no rubyprof" unless HAVE_RUBYPROF + end + def test_amount_format amount_metric = ActiveSupport::Testing::Performance::Metrics[:amount].new assert_equal "0", amount_metric.format(0) @@ -56,4 +65,4 @@ module ActiveSupport end end end -end
\ No newline at end of file +end diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 66e6390f67..76548a3397 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -1225,17 +1225,17 @@ WARNING: Up to and including Rails 3.1, when the number of arguments passed to a Find or build a new object -------------------------- -It's common that you need to find a record or create it if it doesn't exist. You can do that with the `first_or_create` and `first_or_create!` methods. +It's common that you need to find a record or create it if it doesn't exist. You can do that with the `find_or_create_by` and `find_or_create_by!` methods. -### `first_or_create` +### `find_or_create_by` -The `first_or_create` method checks whether `first` returns `nil` or not. If it does return `nil`, then `create` is called. This is very powerful when coupled with the `where` method. Let's see an example. +The `find_or_create_by` method checks whether a record with the attributes exists. If it doesn't, then `create` is called. Let's see an example. -Suppose you want to find a client named 'Andy', and if there's none, create one and additionally set his `locked` attribute to false. You can do so by running: +Suppose you want to find a client named 'Andy', and if there's none, create one. You can do so by running: ```ruby -Client.where(:first_name => 'Andy').first_or_create(:locked => false) -# => #<Client id: 1, first_name: "Andy", orders_count: 0, locked: false, created_at: "2011-08-30 06:09:27", updated_at: "2011-08-30 06:09:27"> +Client.find_or_create_by(first_name: 'Andy') +# => #<Client id: 1, first_name: "Andy", orders_count: 0, locked: true, created_at: "2011-08-30 06:09:27", updated_at: "2011-08-30 06:09:27"> ``` The SQL generated by this method looks like this: @@ -1243,27 +1243,39 @@ The SQL generated by this method looks like this: ```sql SELECT * FROM clients WHERE (clients.first_name = 'Andy') LIMIT 1 BEGIN -INSERT INTO clients (created_at, first_name, locked, orders_count, updated_at) VALUES ('2011-08-30 05:22:57', 'Andy', 0, NULL, '2011-08-30 05:22:57') +INSERT INTO clients (created_at, first_name, locked, orders_count, updated_at) VALUES ('2011-08-30 05:22:57', 'Andy', 1, NULL, '2011-08-30 05:22:57') COMMIT ``` -`first_or_create` returns either the record that already exists or the new record. In our case, we didn't already have a client named Andy so the record is created and returned. +`find_or_create_by` returns either the record that already exists or the new record. In our case, we didn't already have a client named Andy so the record is created and returned. The new record might not be saved to the database; that depends on whether validations passed or not (just like `create`). -It's also worth noting that `first_or_create` takes into account the arguments of the `where` method. In the example above we didn't explicitly pass a `:first_name => 'Andy'` argument to `first_or_create`. However, that was used when creating the new record because it was already passed before to the `where` method. +Suppose we want to set the 'locked' attribute to true if we're +creating a new record, but we don't want to include it in the query. So +we want to find the client named "Andy", or if that client doesn't +exist, create a client named "Andy" which is not locked. -You can do the same with the `find_or_create_by` method: +We can achive this in two ways. The first is to use `create_with`: ```ruby -Client.find_or_create_by_first_name(:first_name => "Andy", :locked => false) +Client.create_with(locked: false).find_or_create_by(first_name: 'Andy') ``` -This method still works, but it's encouraged to use `first_or_create` because it's more explicit on which arguments are used to _find_ the record and which are used to _create_, resulting in less confusion overall. +The second way is using a block: -### `first_or_create!` +```ruby +Client.find_or_create_by(first_name: 'Andy') do |c| + c.locked = false +end +``` -You can also use `first_or_create!` to raise an exception if the new record is invalid. Validations are not covered on this guide, but let's assume for a moment that you temporarily add +The block will only be executed if the client is being created. The +second time we run this code, the block will be ignored. + +### `find_or_create_by!` + +You can also use `find_or_create_by!` to raise an exception if the new record is invalid. Validations are not covered on this guide, but let's assume for a moment that you temporarily add ```ruby validates :orders_count, :presence => true @@ -1272,19 +1284,21 @@ validates :orders_count, :presence => true to your `Client` model. If you try to create a new `Client` without passing an `orders_count`, the record will be invalid and an exception will be raised: ```ruby -Client.where(:first_name => 'Andy').first_or_create!(:locked => false) +Client.find_or_create_by!(first_name: 'Andy') # => ActiveRecord::RecordInvalid: Validation failed: Orders count can't be blank ``` -As with `first_or_create` there is a `find_or_create_by!` method but the `first_or_create!` method is preferred for clarity. - -### `first_or_initialize` +### `find_or_initialize_by` -The `first_or_initialize` method will work just like `first_or_create` but it will not call `create` but `new`. This means that a new model instance will be created in memory but won't be saved to the database. Continuing with the `first_or_create` example, we now want the client named 'Nick': +The `find_or_initialize_by` method will work just like +`find_or_create_by` but it will call `new` instead of `create`. This +means that a new model instance will be created in memory but won't be +saved to the database. Continuing with the `find_or_create_by` example, we +now want the client named 'Nick': ```ruby -nick = Client.where(:first_name => 'Nick').first_or_initialize(:locked => false) -# => <Client id: nil, first_name: "Nick", orders_count: 0, locked: false, created_at: "2011-08-30 06:09:27", updated_at: "2011-08-30 06:09:27"> +nick = Client.find_or_initialize_by(first_name: 'Nick') +# => <Client id: nil, first_name: "Nick", orders_count: 0, locked: true, created_at: "2011-08-30 06:09:27", updated_at: "2011-08-30 06:09:27"> nick.persisted? # => false diff --git a/rails.gemspec b/rails.gemspec index db2f4cc9da..97f6dfeac8 100644 --- a/rails.gemspec +++ b/rails.gemspec @@ -25,5 +25,5 @@ Gem::Specification.new do |s| s.add_dependency('actionmailer', version) s.add_dependency('railties', version) s.add_dependency('bundler', '~> 1.2') - s.add_dependency('sprockets-rails', '~> 2.0') + s.add_dependency('sprockets-rails', '~> 2.0.0.rc1') end diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index eff5bbbdd6..9ef001c7d0 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -286,7 +286,7 @@ module Rails def default_middleware_stack #:nodoc: ActionDispatch::MiddlewareStack.new.tap do |middleware| app = self - if rack_cache = config.action_controller.perform_caching && config.action_dispatch.rack_cache + if rack_cache = config.action_dispatch.rack_cache begin require 'rack/cache' rescue LoadError => error diff --git a/railties/test/application/middleware/cache_test.rb b/railties/test/application/middleware/cache_test.rb index 9b6c76e7aa..c99666d7e4 100644 --- a/railties/test/application/middleware/cache_test.rb +++ b/railties/test/application/middleware/cache_test.rb @@ -49,8 +49,6 @@ module ApplicationTests get ':controller(/:action)' end RUBY - - add_to_config "config.action_dispatch.rack_cache = true" end def test_cache_keeps_if_modified_since @@ -80,6 +78,8 @@ module ApplicationTests def test_cache_works_with_expires simple_controller + add_to_config "config.action_dispatch.rack_cache = true" + get "/expires/expires_header" assert_equal "miss, store", last_response.headers["X-Rack-Cache"] assert_equal "max-age=10, public", last_response.headers["Cache-Control"] @@ -96,6 +96,8 @@ module ApplicationTests def test_cache_works_with_expires_private simple_controller + add_to_config "config.action_dispatch.rack_cache = true" + get "/expires/expires_header", private: true assert_equal "miss", last_response.headers["X-Rack-Cache"] assert_equal "private, max-age=10", last_response.headers["Cache-Control"] @@ -110,6 +112,8 @@ module ApplicationTests def test_cache_works_with_etags simple_controller + add_to_config "config.action_dispatch.rack_cache = true" + get "/expires/expires_etag" assert_equal "miss, store", last_response.headers["X-Rack-Cache"] assert_equal "public", last_response.headers["Cache-Control"] @@ -125,6 +129,8 @@ module ApplicationTests def test_cache_works_with_etags_private simple_controller + add_to_config "config.action_dispatch.rack_cache = true" + get "/expires/expires_etag", private: true assert_equal "miss", last_response.headers["X-Rack-Cache"] assert_equal "must-revalidate, private, max-age=0", last_response.headers["Cache-Control"] @@ -140,6 +146,8 @@ module ApplicationTests def test_cache_works_with_last_modified simple_controller + add_to_config "config.action_dispatch.rack_cache = true" + get "/expires/expires_last_modified" assert_equal "miss, store", last_response.headers["X-Rack-Cache"] assert_equal "public", last_response.headers["Cache-Control"] @@ -155,6 +163,8 @@ module ApplicationTests def test_cache_works_with_last_modified_private simple_controller + add_to_config "config.action_dispatch.rack_cache = true" + get "/expires/expires_last_modified", private: true assert_equal "miss", last_response.headers["X-Rack-Cache"] assert_equal "must-revalidate, private, max-age=0", last_response.headers["Cache-Control"] diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index c03d35e926..c5a0d02fe1 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -57,15 +57,12 @@ module ApplicationTests end test "Rack::Cache is not included by default" do - add_to_config "config.action_controller.perform_caching = true" - boot! assert !middleware.include?("Rack::Cache"), "Rack::Cache is not included in the default stack unless you set config.action_dispatch.rack_cache" end - test "Rack::Cache is present when action_controller.perform_caching is set and action_dispatch.rack_cache is set" do - add_to_config "config.action_controller.perform_caching = true" + test "Rack::Cache is present when action_dispatch.rack_cache is set" do add_to_config "config.action_dispatch.rack_cache = true" boot! diff --git a/railties/test/application/url_generation_test.rb b/railties/test/application/url_generation_test.rb index 4ecb94b65e..2a48adae5c 100644 --- a/railties/test/application/url_generation_test.rb +++ b/railties/test/application/url_generation_test.rb @@ -17,6 +17,7 @@ module ApplicationTests config.secret_token = "3b7cd727ee24e8444053437c36cc66c4" config.session_store :cookie_store, key: "_myapp_session" config.active_support.deprecation = :log + config.eager_load = false end MyApp.initialize! diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 3d1252505a..62e1f3531e 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -223,7 +223,7 @@ class AppGeneratorTest < Rails::Generators::TestCase def test_generator_if_skip_sprockets_is_given run_generator [destination_root, "--skip-sprockets"] assert_file "config/application.rb" do |content| - assert_match(/#\s+require\s+["']sprockets\/rails\/railtie["']/, content) + assert_match(/#\s+require\s+["']sprockets\/railtie["']/, content) assert_no_match(/config\.assets\.enabled = true/, content) end assert_file "Gemfile" do |content| |