diff options
56 files changed, 450 insertions, 198 deletions
diff --git a/actionpack/lib/action_dispatch/routing/routes_proxy.rb b/actionpack/lib/action_dispatch/routing/routes_proxy.rb index 3592683136..ee847eaeed 100644 --- a/actionpack/lib/action_dispatch/routing/routes_proxy.rb +++ b/actionpack/lib/action_dispatch/routing/routes_proxy.rb @@ -19,7 +19,7 @@ module ActionDispatch end end - def respond_to?(method, include_private = false) + def respond_to_missing?(method, include_private = false) super || @helpers.respond_to?(method) end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index f5ec28a9c2..0082b3d8cb 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -502,7 +502,7 @@ module ActionDispatch integration_session.default_url_options = options end - def respond_to?(method, include_private = false) + def respond_to_missing?(method, include_private = false) integration_session.respond_to?(method, include_private) || super end diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb index 6cac4bc0ad..2b9c1a8ceb 100644 --- a/actionview/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb @@ -56,7 +56,7 @@ module ActionView # # => <script src="http://www.example.com/xmlhr.js"></script> def javascript_include_tag(*sources) options = sources.extract_options!.stringify_keys - path_options = options.extract!("protocol", "extname", "host").symbolize_keys + path_options = options.extract!("protocol", "extname", "host", "skip_pipeline").symbolize_keys sources.uniq.map { |source| tag_options = { "src" => path_to_javascript(source, path_options) @@ -92,8 +92,7 @@ module ActionView # # <link href="/css/stylish.css" media="screen" rel="stylesheet" /> def stylesheet_link_tag(*sources) options = sources.extract_options!.stringify_keys - path_options = options.extract!("protocol", "host").symbolize_keys - + path_options = options.extract!("protocol", "host", "skip_pipeline").symbolize_keys sources.uniq.map { |source| tag_options = { "rel" => "stylesheet", @@ -174,7 +173,7 @@ module ActionView tag("link", { rel: "shortcut icon", type: "image/x-icon", - href: path_to_image(source) + href: path_to_image(source, skip_pipeline: options.delete(:skip_pipeline)) }.merge!(options.symbolize_keys)) end @@ -212,7 +211,7 @@ module ActionView options = options.symbolize_keys check_for_image_tag_errors(options) - src = options[:src] = path_to_image(source) + src = options[:src] = path_to_image(source, skip_pipeline: options.delete(:skip_pipeline)) unless src.start_with?("cid:") || src.start_with?("data:") || src.blank? options[:alt] = options.fetch(:alt) { image_alt(src) } @@ -258,6 +257,8 @@ module ActionView # * <tt>:size</tt> - Supplied as "{Width}x{Height}" or "{Number}", so "30x45" becomes # width="30" and height="45", and "50" becomes width="50" and height="50". # <tt>:size</tt> will be ignored if the value is not in the correct format. + # * <tt>:poster_skip_pipeline</tt> will bypass the asset pipeline when using + # the <tt>:poster</tt> option instead using an asset in the public folder. # # ==== Examples # @@ -269,6 +270,8 @@ module ActionView # # => <video preload="none" controls="controls" src="/videos/trailer.ogg" ></video> # video_tag("trailer.m4v", size: "16x10", poster: "screenshot.png") # # => <video src="/videos/trailer.m4v" width="16" height="10" poster="/assets/screenshot.png"></video> + # video_tag("trailer.m4v", size: "16x10", poster: "screenshot.png", poster_skip_pipeline: true) + # # => <video src="/videos/trailer.m4v" width="16" height="10" poster="screenshot.png"></video> # video_tag("/trailers/hd.avi", size: "16x16") # # => <video src="/trailers/hd.avi" width="16" height="16"></video> # video_tag("/trailers/hd.avi", size: "16") @@ -282,8 +285,11 @@ module ActionView # video_tag(["trailer.ogg", "trailer.flv"], size: "160x120") # # => <video height="120" width="160"><source src="/videos/trailer.ogg" /><source src="/videos/trailer.flv" /></video> def video_tag(*sources) - multiple_sources_tag("video", sources) do |options| - options[:poster] = path_to_image(options[:poster]) if options[:poster] + options = sources.extract_options!.symbolize_keys + public_poster_folder = options.delete(:poster_skip_pipeline) + sources << options + multiple_sources_tag_builder("video", sources) do |options| + options[:poster] = path_to_image(options[:poster], skip_pipeline: public_poster_folder) if options[:poster] options[:width], options[:height] = extract_dimensions(options.delete(:size)) if options[:size] end end @@ -301,22 +307,23 @@ module ActionView # audio_tag("sound.wav", "sound.mid") # # => <audio><source src="/audios/sound.wav" /><source src="/audios/sound.mid" /></audio> def audio_tag(*sources) - multiple_sources_tag("audio", sources) + multiple_sources_tag_builder("audio", sources) end private - def multiple_sources_tag(type, sources) - options = sources.extract_options!.symbolize_keys + def multiple_sources_tag_builder(type, sources) + options = sources.extract_options!.symbolize_keys + skip_pipeline = options.delete(:skip_pipeline) sources.flatten! yield options if block_given? if sources.size > 1 content_tag(type, options) do - safe_join sources.map { |source| tag("source", src: send("path_to_#{type}", source)) } + safe_join sources.map { |source| tag("source", src: send("path_to_#{type}", source, skip_pipeline: skip_pipeline)) } end else - options[:src] = send("path_to_#{type}", sources.first) + options[:src] = send("path_to_#{type}", sources.first, skip_pipeline: skip_pipeline) content_tag(type, nil, options) end end diff --git a/actionview/lib/action_view/helpers/asset_url_helper.rb b/actionview/lib/action_view/helpers/asset_url_helper.rb index 76a4893f2e..0967245855 100644 --- a/actionview/lib/action_view/helpers/asset_url_helper.rb +++ b/actionview/lib/action_view/helpers/asset_url_helper.rb @@ -118,16 +118,67 @@ module ActionView module AssetUrlHelper URI_REGEXP = %r{^[-a-z]+://|^(?:cid|data):|^//}i - # Computes the path to asset in public directory. If :type - # options is set, a file extension will be appended and scoped - # to the corresponding public directory. + # This is the entry point for all assets. + # When using the asset pipeline (i.e. sprockets and sprockets-rails), the + # behavior is "enhanced". You can bypass the asset pipeline by passing in + # <tt>skip_pipeline: true</tt> to the options. # # All other asset *_path helpers delegate through this method. # - # asset_path "application.js" # => /assets/application.js - # asset_path "application", type: :javascript # => /assets/application.js - # asset_path "application", type: :stylesheet # => /assets/application.css - # asset_path "http://www.example.com/js/xmlhr.js" # => http://www.example.com/js/xmlhr.js + # === With the asset pipeline + # + # All options passed to +asset_path+ will be passed to +compute_asset_path+ + # which is implemented by sprockets-rails. + # + # asset_path("application.js") # => "/assets/application-60aa4fdc5cea14baf5400fba1abf4f2a46a5166bad4772b1effe341570f07de9.js" + # + # === Without the asset pipeline (<tt>skip_pipeline: true</tt>) + # + # Accepts a <tt>type</tt> option that can specify the asset's extension. No error + # checking is done to verify the source passed into +asset_path+ is valid + # and that the file exists on disk. + # + # asset_path("application.js", skip_pipeline: true) # => "application.js" + # asset_path("filedoesnotexist.png", skip_pipeline: true) # => "filedoesnotexist.png" + # asset_path("application", type: :javascript, skip_pipeline: true) # => "/javascripts/application.js" + # asset_path("application", type: :stylesheet, skip_pipeline: true) # => "/stylesheets/application.css" + # + # === Options applying to all assets + # + # Below lists scenarios that apply to +asset_path+ whether or not you're + # using the asset pipeline. + # + # - All fully qualified urls are returned immediately. This bypasses the + # asset pipeline and all other behavior described. + # + # asset_path("http://www.example.com/js/xmlhr.js") # => "http://www.example.com/js/xmlhr.js" + # + # - All assets that begin with a forward slash are assumed to be full + # urls and will not be expanded. This will bypass the asset pipeline. + # + # asset_path("/foo.png") # => "/foo.png" + # + # - All blank strings will be returned immediately. This bypasses the + # asset pipeline and all other behavior described. + # + # asset_path("") # => "" + # + # - If <tt>config.relative_url_root</tt> is specified, all assets will have that + # root prepended. + # + # Rails.application.config.relative_url_root = "bar" + # asset_path("foo.js", skip_pipeline: true) # => "bar/foo.js" + # + # - A different asset host can be specified via <tt>config.action_controller.asset_host</tt> + # this is commonly used in conjunction with a CDN. + # + # Rails.application.config.action_controller.asset_host = "assets.example.com" + # asset_path("foo.js", skip_pipeline: true) # => "http://assets.example.com/foo.js" + # + # - An extension name can be specified manually with <tt>extname</tt>. + # + # asset_path("foo", skip_pipeline: true, extname: ".js") # => "/foo.js" + # asset_path("foo.css", skip_pipeline: true, extname: ".js") # => "/foo.css.js" def asset_path(source, options = {}) raise ArgumentError, "nil is not a valid asset source" if source.nil? @@ -142,7 +193,11 @@ module ActionView end if source[0] != ?/ - source = compute_asset_path(source, options) + if options[:skip_pipeline] + source = public_compute_asset_path(source, options) + else + source = compute_asset_path(source, options) + end end relative_url_root = defined?(config.relative_url_root) && config.relative_url_root @@ -203,6 +258,7 @@ module ActionView dir = ASSET_PUBLIC_DIRECTORIES[options[:type]] || "" File.join(dir, source) end + alias :public_compute_asset_path :compute_asset_path # Pick an asset host for this source. Returns +nil+ if no host is set, # the host if no wildcard is set, the host interpolated with the diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4aa1853bde..3eac34d65e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,22 @@ +* PostgreSQL array columns will now respect the encoding of strings contained + in the array. + + Fixes #26326. + + *Sean Griffin* + +* Inverse association instances will now be set before `after_find` or + `after_initialize` callbacks are run. + + Fixes #26320. + + *Sean Griffin* + +* Remove unnecessarily association load when a `belongs_to` association has already been + loaded then the foreign key is changed directly and the record saved. + + *James Coleman* + * Remove standardized column types/arguments spaces in schema dump. *Tim Petricola* diff --git a/activerecord/lib/active_record/association_relation.rb b/activerecord/lib/active_record/association_relation.rb index 2da2d968b9..de2d03cd0b 100644 --- a/activerecord/lib/active_record/association_relation.rb +++ b/activerecord/lib/active_record/association_relation.rb @@ -29,7 +29,10 @@ module ActiveRecord private def exec_queries - super.each { |r| @association.set_inverse_instance r } + super do |r| + @association.set_inverse_instance r + yield r if block_given? + end end end end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index dc68b01386..7759b0a044 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1792,12 +1792,7 @@ module ActiveRecord # has_and_belongs_to_many :nations, class_name: "Country" # has_and_belongs_to_many :categories, join_table: "prods_cats" # has_and_belongs_to_many :categories, -> { readonly } - def has_and_belongs_to_many(name, scope = nil, options = {}, &extension) - if scope.is_a?(Hash) - options = scope - scope = nil - end - + def has_and_belongs_to_many(name, scope = nil, **options, &extension) habtm_reflection = ActiveRecord::Reflection::HasAndBelongsToManyReflection.new(name, scope, options, self) builder = Builder::HasAndBelongsToMany.new name, self, options diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 0f51b35164..0c911a5396 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -390,9 +390,9 @@ module ActiveRecord end binds = AssociationScope.get_bind_values(owner, reflection.chain) - records = sc.execute(binds, klass, conn) - records.each { |record| set_inverse_instance(record) } - records + sc.execute(binds, klass, conn) do |record| + set_inverse_instance(record) + end end # We have some records loaded from the database (persisted) and some that are diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 62acad0eda..02f0721bed 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -286,17 +286,19 @@ module ActiveRecord end def construct_model(record, node, row, model_cache, id, aliases) - model = model_cache[node][id] ||= node.instantiate(row, - aliases.column_aliases(node)) other = record.association(node.reflection.name) + model = model_cache[node][id] ||= + node.instantiate(row, aliases.column_aliases(node)) do |m| + other.set_inverse_instance(m) + end + if node.reflection.collection? other.target.push(model) else other.target = model end - other.set_inverse_instance(model) model end end diff --git a/activerecord/lib/active_record/associations/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/join_dependency/join_part.rb index 551087f822..61cec5403a 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_part.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_part.rb @@ -62,8 +62,8 @@ module ActiveRecord hash end - def instantiate(row, aliases) - base_klass.instantiate(extract_record(row, aliases)) + def instantiate(row, aliases, &block) + base_klass.instantiate(extract_record(row, aliases), &block) end end end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 4bb627f399..07407700cd 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -62,7 +62,12 @@ module ActiveRecord private def associated_records_by_owner(preloader) - records = load_records + records = load_records do |record| + owner = owners_by_key[convert_key(record[association_key_name])] + association = owner.association(reflection.name) + association.set_inverse_instance(record) + end + owners.each_with_object({}) do |owner, result| result[owner] = records[convert_key(owner[owner_key_name])] || [] end @@ -79,6 +84,15 @@ module ActiveRecord @owner_keys end + def owners_by_key + unless defined?(@owners_by_key) + @owners_by_key = owners.each_with_object({}) do |owner, h| + h[convert_key(owner[owner_key_name])] = owner + end + end + @owners_by_key + end + def key_conversion_required? @key_conversion_required ||= association_key_type != owner_key_type end @@ -99,13 +113,13 @@ module ActiveRecord @model.type_for_attribute(owner_key_name.to_s).type end - def load_records + def load_records(&block) return {} if owner_keys.empty? # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) # Make several smaller queries if necessary or make one query if the adapter supports it slices = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) @preloaded_records = slices.flat_map do |slice| - records_for(slice) + records_for(slice).load(&block) end @preloaded_records.group_by do |record| convert_key(record[association_key_name]) diff --git a/activerecord/lib/active_record/associations/preloader/collection_association.rb b/activerecord/lib/active_record/associations/preloader/collection_association.rb index 24b8e01029..26690bf16d 100644 --- a/activerecord/lib/active_record/associations/preloader/collection_association.rb +++ b/activerecord/lib/active_record/associations/preloader/collection_association.rb @@ -9,7 +9,6 @@ module ActiveRecord association = owner.association(reflection.name) association.loaded! association.target.concat(records) - records.each { |record| association.set_inverse_instance(record) } end end end diff --git a/activerecord/lib/active_record/associations/preloader/singular_association.rb b/activerecord/lib/active_record/associations/preloader/singular_association.rb index 0888d383a6..5c5828262e 100644 --- a/activerecord/lib/active_record/associations/preloader/singular_association.rb +++ b/activerecord/lib/active_record/associations/preloader/singular_association.rb @@ -10,7 +10,6 @@ module ActiveRecord association = owner.association(reflection.name) association.target = record - association.set_inverse_instance(record) if record end end end diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb index 380593e809..0b08c2a39b 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activerecord/lib/active_record/attribute.rb @@ -187,7 +187,7 @@ module ActiveRecord class Null < Attribute # :nodoc: def initialize(name) - super(name, nil, Type::Value.new) + super(name, nil, Type.default_value) end def type_cast(*) diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 6e6620aad5..db84876b0a 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -457,7 +457,9 @@ module ActiveRecord # In addition, it will destroy the association if it was marked for destruction. def save_belongs_to_association(reflection) association = association_instance_get(reflection.name) - record = association && association.load_target + return unless association && association.loaded? && !association.stale_target? + + record = association.load_target if record && !record.destroyed? autosave = reflection.options[:autosave] diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 4dde525ebc..0c7197a002 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -433,16 +433,16 @@ module ActiveRecord @connection end - def case_sensitive_comparison(attribute, column, value) # :nodoc: - attribute.eq(value) + def case_sensitive_comparison(table, attribute, column, value) + table[attribute].eq(Arel::Nodes::BindParam.new) end - def case_insensitive_comparison(attribute, column, value) # :nodoc: + def case_insensitive_comparison(table, attribute, column, value) if can_perform_case_insensitive_comparison_for?(column) - value = attribute.relation.lower(value) - attribute = attribute.lower + table[attribute].lower.eq(table.lower(Arel::Nodes::BindParam.new)) + else + table[attribute].eq(Arel::Nodes::BindParam.new) end - attribute.eq(value) end def can_perform_case_insensitive_comparison_for?(column) 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 3a28879c15..4333cd1f57 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -613,11 +613,12 @@ module ActiveRecord SQL end - def case_sensitive_comparison(attribute, column, value) # :nodoc: + def case_sensitive_comparison(table, attribute, column, value) if column.collation && !column.case_sensitive? - value = Arel::Nodes::Bin.new(value) + table[attribute].eq(Arel::Nodes::Bin.new(Arel::Nodes::BindParam.new)) + else + super end - attribute.eq(value) end def can_perform_case_insensitive_comparison_for?(column) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb index 8c318886cf..0e526f6201 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb @@ -12,7 +12,7 @@ require "active_record/connection_adapters/postgresql/oid/json" require "active_record/connection_adapters/postgresql/oid/jsonb" require "active_record/connection_adapters/postgresql/oid/money" require "active_record/connection_adapters/postgresql/oid/point" -require "active_record/connection_adapters/postgresql/oid/rails_5_1_point" +require "active_record/connection_adapters/postgresql/oid/legacy_point" require "active_record/connection_adapters/postgresql/oid/range" require "active_record/connection_adapters/postgresql/oid/specialized_string" require "active_record/connection_adapters/postgresql/oid/uuid" diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb index 1a66afb23a..b969503178 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/array.rb @@ -33,7 +33,11 @@ module ActiveRecord def serialize(value) if value.is_a?(::Array) - @pg_encoder.encode(type_cast_array(value, :serialize)) + result = @pg_encoder.encode(type_cast_array(value, :serialize)) + if encoding = determine_encoding_of_strings(value) + result.encode!(encoding) + end + result else super end @@ -63,6 +67,13 @@ module ActiveRecord @subtype.public_send(method, value) end end + + def determine_encoding_of_strings(value) + case value + when ::Array then determine_encoding_of_strings(value.first) + when ::String then value.encoding + end + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/rails_5_1_point.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/legacy_point.rb index 845ff5b6a9..775eecaf85 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/rails_5_1_point.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/legacy_point.rb @@ -1,10 +1,8 @@ module ActiveRecord - Point = Struct.new(:x, :y) - module ConnectionAdapters module PostgreSQL module OID # :nodoc: - class Rails51Point < Type::Value # :nodoc: + class LegacyPoint < Type::Value # :nodoc: include Type::Helpers::Mutable def type @@ -14,23 +12,20 @@ module ActiveRecord def cast(value) case value when ::String - return if value.blank? - if value[0] == "(" && value[-1] == ")" value = value[1...-1] end - x, y = value.split(",") - build_point(x, y) + cast(value.split(",")) when ::Array - build_point(*value) + value.map { |v| Float(v) } else value end end def serialize(value) - if value.is_a?(ActiveRecord::Point) - "(#{number_for_point(value.x)},#{number_for_point(value.y)})" + if value.is_a?(::Array) + "(#{number_for_point(value[0])},#{number_for_point(value[1])})" else super end @@ -41,10 +36,6 @@ module ActiveRecord def number_for_point(number) number.to_s.gsub(/\.0$/, "") end - - def build_point(x, y) - ActiveRecord::Point.new(Float(x), Float(y)) - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/point.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/point.rb index bb4db2564c..7c764e7287 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/point.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/point.rb @@ -1,4 +1,6 @@ module ActiveRecord + Point = Struct.new(:x, :y) + module ConnectionAdapters module PostgreSQL module OID # :nodoc: @@ -12,20 +14,34 @@ module ActiveRecord def cast(value) case value when ::String + return if value.blank? + if value[0] == "(" && value[-1] == ")" value = value[1...-1] end - cast(value.split(",")) + x, y = value.split(",") + build_point(x, y) when ::Array - value.map { |v| Float(v) } + build_point(*value) else value end end def serialize(value) - if value.is_a?(::Array) - "(#{number_for_point(value[0])},#{number_for_point(value[1])})" + case value + when ActiveRecord::Point + "(#{number_for_point(value.x)},#{number_for_point(value.y)})" + when ::Array + serialize(build_point(*value)) + else + super + end + end + + def type_cast_for_schema(value) + if ActiveRecord::Point === value + [value.x, value.y] else super end @@ -36,6 +52,10 @@ module ActiveRecord def number_for_point(number) number.to_s.gsub(/\.0$/, "") end + + def build_point(x, y) + ActiveRecord::Point.new(Float(x), Float(y)) + end end 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 bd53123511..03ee0eec5b 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -437,7 +437,7 @@ module ActiveRecord type_map.fetch(oid, fmod, sql_type) { warn "unknown OID #{oid}: failed to recognize type of '#{column_name}'. It will be treated as String." - Type::Value.new.tap do |cast_type| + Type.default_value.tap do |cast_type| type_map.register_type(oid, cast_type) end } @@ -838,8 +838,8 @@ module ActiveRecord ActiveRecord::Type.register(:json, OID::Json, adapter: :postgresql) ActiveRecord::Type.register(:jsonb, OID::Jsonb, adapter: :postgresql) ActiveRecord::Type.register(:money, OID::Money, adapter: :postgresql) - ActiveRecord::Type.register(:point, OID::Rails51Point, adapter: :postgresql) - ActiveRecord::Type.register(:legacy_point, OID::Point, adapter: :postgresql) + ActiveRecord::Type.register(:point, OID::Point, adapter: :postgresql) + ActiveRecord::Type.register(:legacy_point, OID::LegacyPoint, adapter: :postgresql) ActiveRecord::Type.register(:uuid, OID::Uuid, adapter: :postgresql) ActiveRecord::Type.register(:vector, OID::Vector, adapter: :postgresql) ActiveRecord::Type.register(:xml, OID::Xml, adapter: :postgresql) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index aef4761be4..4628d7fb16 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -366,6 +366,8 @@ module ActiveRecord self.class.define_attribute_methods + yield self if block_given? + _run_find_callbacks _run_initialize_callbacks @@ -498,7 +500,7 @@ module ActiveRecord # We check defined?(@attributes) not to issue warnings if the object is # allocated but not initialized. inspection = if defined?(@attributes) && @attributes - self.class.column_names.collect do |name| + self.class.attribute_names.collect do |name| if has_attribute?(name) "#{name}: #{attribute_for_inspect(name)}" end diff --git a/activerecord/lib/active_record/dynamic_matchers.rb b/activerecord/lib/active_record/dynamic_matchers.rb index fd2fa7410a..55490e27bc 100644 --- a/activerecord/lib/active_record/dynamic_matchers.rb +++ b/activerecord/lib/active_record/dynamic_matchers.rb @@ -2,7 +2,7 @@ require "active_support/core_ext/regexp" module ActiveRecord module DynamicMatchers #:nodoc: - def respond_to?(name, include_private = false) + def respond_to_missing?(name, include_private = false) if self == Base super else diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 6b2c5d8da5..7e60aabc2d 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -92,7 +92,7 @@ module ActiveRecord send(method, args, &block) end - def respond_to?(*args) # :nodoc: + def respond_to_missing?(*args) # :nodoc: super || delegate.respond_to?(*args) end diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 5718e7fdd0..f49f8da2ed 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -268,7 +268,7 @@ module ActiveRecord def attribute_types # :nodoc: load_schema - @attribute_types ||= Hash.new(Type::Value.new) + @attribute_types ||= Hash.new(Type.default_value) end def yaml_encoder # :nodoc: @@ -370,7 +370,6 @@ module ActiveRecord def load_schema! @columns_hash = connection.schema_cache.columns_hash(table_name).except(*ignored_columns) @columns_hash.each do |name, column| - warn_if_deprecated_type(column) define_attribute( name, connection.lookup_cast_type_from_column(column), @@ -421,28 +420,6 @@ module ActiveRecord base.table_name end end - - def warn_if_deprecated_type(column) - return if attributes_to_define_after_schema_loads.key?(column.name) - if column.respond_to?(:oid) && column.sql_type.start_with?("point") - if column.array? - array_arguments = ", array: true" - else - array_arguments = "" - end - ActiveSupport::Deprecation.warn(<<-WARNING.strip_heredoc) - The behavior of the `:point` type will be changing in Rails 5.1 to - return a `Point` object, rather than an `Array`. If you'd like to - keep the old behavior, you can add this line to #{name}: - - attribute :#{column.name}, :legacy_point#{array_arguments} - - If you'd like the new behavior today, you can add this line: - - attribute :#{column.name}, :point#{array_arguments} - WARNING - end - end end end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index dc4af4f390..a04ef2e263 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -63,10 +63,10 @@ module ActiveRecord # # See <tt>ActiveRecord::Inheritance#discriminate_class_for_record</tt> to see # how this "single-table" inheritance mapping is implemented. - def instantiate(attributes, column_types = {}) + def instantiate(attributes, column_types = {}, &block) klass = discriminate_class_for_record(attributes) attributes = klass.attributes_builder.build_from_database(attributes, column_types) - klass.allocate.init_with("attributes" => attributes, "new_record" => false) + klass.allocate.init_with("attributes" => attributes, "new_record" => false, &block) end private @@ -178,7 +178,7 @@ module ActiveRecord # and #destroy returns +false+. # See ActiveRecord::Callbacks for further details. def destroy - raise ReadOnlyRecord, "#{self.class} is marked as readonly" if readonly? + _raise_readonly_record_error if readonly? destroy_associations self.class.connection.add_transaction_record(self) destroy_row if persisted? @@ -535,7 +535,7 @@ module ActiveRecord end def create_or_update(*args) - raise ReadOnlyRecord, "#{self.class} is marked as readonly" if readonly? + _raise_readonly_record_error if readonly? result = new_record? ? _create_record : _update_record(*args) result != false end @@ -577,5 +577,9 @@ module ActiveRecord def belongs_to_touch_method :touch end + + def _raise_readonly_record_error + raise ReadOnlyRecord, "#{self.class} is marked as readonly" + end end end diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index dd7d650207..36689f6559 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -35,7 +35,7 @@ module ActiveRecord # # Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date] # Post.find_by_sql ["SELECT body FROM comments WHERE author = :user_id OR approved_by = :user_id", { :user_id => user_id }] - def find_by_sql(sql, binds = [], preparable: nil) + def find_by_sql(sql, binds = [], preparable: nil, &block) result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable) column_types = result_set.column_types.dup columns_hash.each_key { |k| column_types.delete k } @@ -47,7 +47,7 @@ module ActiveRecord } message_bus.instrument("instantiation.active_record", payload) do - result_set.map { |record| instantiate(record, column_types) } + result_set.map { |record| instantiate(record, column_types, &block) } end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index d7de1032b6..6d571cf026 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -562,8 +562,8 @@ module ActiveRecord # return value is the relation itself, not the records. # # Post.where(published: true).load # => #<ActiveRecord::Relation> - def load - exec_queries unless loaded? + def load(&block) + exec_queries(&block) unless loaded? self end @@ -678,8 +678,8 @@ module ActiveRecord private - def exec_queries - @records = eager_loading? ? find_with_associations.freeze : @klass.find_by_sql(arel, bound_attributes).freeze + def exec_queries(&block) + @records = eager_loading? ? find_with_associations.freeze : @klass.find_by_sql(arel, bound_attributes, &block).freeze preload = preload_values preload += includes_values unless eager_loading? diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index b569abc7a8..a796e35261 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -312,7 +312,7 @@ module ActiveRecord key = group_columns.map { |aliaz, col_name| column = type_for(col_name) do calculated_data.column_types.fetch(aliaz) do - Type::Value.new + Type.default_value end end type_cast_calculated_value(row[aliaz], column) diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index e1c36982dd..d16de4b06c 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -108,7 +108,7 @@ module ActiveRecord end end - def respond_to?(method, include_private = false) + def respond_to_missing?(method, include_private = false) super || @klass.respond_to?(method, include_private) || arel.respond_to?(method, include_private) end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 29422bf131..780a1ee422 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -4,7 +4,6 @@ module ActiveRecord require "active_record/relation/predicate_builder/association_query_handler" require "active_record/relation/predicate_builder/base_handler" require "active_record/relation/predicate_builder/basic_object_handler" - require "active_record/relation/predicate_builder/case_sensitive_handler" require "active_record/relation/predicate_builder/class_handler" require "active_record/relation/predicate_builder/polymorphic_array_handler" require "active_record/relation/predicate_builder/range_handler" @@ -17,7 +16,6 @@ module ActiveRecord @handlers = [] register_handler(BasicObject, BasicObjectHandler.new) - register_handler(CaseSensitiveHandler::Value, CaseSensitiveHandler.new) register_handler(Class, ClassHandler.new(self)) register_handler(Base, BaseHandler.new(self)) register_handler(Range, RangeHandler.new) @@ -33,9 +31,9 @@ module ActiveRecord expand_from_hash(attributes) end - def create_binds(attributes, options) + def create_binds(attributes) attributes = convert_dot_notation_to_hash(attributes) - create_binds_for_hash(attributes, options) + create_binds_for_hash(attributes) end def self.references(attributes) @@ -84,14 +82,14 @@ module ActiveRecord end end - def create_binds_for_hash(attributes, options) + def create_binds_for_hash(attributes) result = attributes.dup binds = [] attributes.each do |column_name, value| case when value.is_a?(Hash) && !table.has_column?(column_name) - attrs, bvs = associated_predicate_builder(column_name).create_binds_for_hash(value, options) + attrs, bvs = associated_predicate_builder(column_name).create_binds_for_hash(value) result[column_name] = attrs binds += bvs next @@ -110,15 +108,11 @@ module ActiveRecord end result[column_name] = RangeHandler::RangeWithBinds.new(first, last, value.exclude_end?) - when can_be_bound?(column_name, value) - result[column_name] = - if perform_case_sensitive?(options) - CaseSensitiveHandler::Value.new( - Arel::Nodes::BindParam.new, table, options[:case_sensitive]) - else - Arel::Nodes::BindParam.new - end - binds << build_bind_param(column_name, value) + else + if can_be_bound?(column_name, value) + result[column_name] = Arel::Nodes::BindParam.new + binds << build_bind_param(column_name, value) + end end # Find the foreign key when using queries such as: @@ -170,10 +164,6 @@ module ActiveRecord end end - def perform_case_sensitive?(options) - options.key?(:case_sensitive) - end - def build_bind_param(column_name, value) Relation::QueryAttribute.new(column_name.to_s, value, table.type(column_name)) end diff --git a/activerecord/lib/active_record/relation/predicate_builder/case_sensitive_handler.rb b/activerecord/lib/active_record/relation/predicate_builder/case_sensitive_handler.rb deleted file mode 100644 index acf0bbd829..0000000000 --- a/activerecord/lib/active_record/relation/predicate_builder/case_sensitive_handler.rb +++ /dev/null @@ -1,21 +0,0 @@ -module ActiveRecord - class PredicateBuilder - class CaseSensitiveHandler # :nodoc: - def call(attribute, value) - value.call(attribute) - end - - class Value < Struct.new(:value, :table, :case_sensitive?) # :nodoc: - def call(attribute) - klass = table.send(:klass) - column = klass.column_for_attribute(attribute.name) - if case_sensitive? - klass.connection.case_sensitive_comparison(attribute, column, value) - else - klass.connection.case_insensitive_comparison(attribute, column, value) - end - end - end - end - end -end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index bd41653df0..5a31f61d6d 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -80,14 +80,14 @@ module ActiveRecord limit_bind = Attribute.with_cast_value( "LIMIT".freeze, connection.sanitize_limit(limit_value), - Type::Value.new, + Type.default_value, ) end if offset_value offset_bind = Attribute.with_cast_value( "OFFSET".freeze, offset_value.to_i, - Type::Value.new, + Type.default_value, ) end connection.combine_bind_parameters( diff --git a/activerecord/lib/active_record/relation/where_clause_factory.rb b/activerecord/lib/active_record/relation/where_clause_factory.rb index 122ab04c00..dc00149130 100644 --- a/activerecord/lib/active_record/relation/where_clause_factory.rb +++ b/activerecord/lib/active_record/relation/where_clause_factory.rb @@ -17,7 +17,7 @@ module ActiveRecord attributes = klass.send(:expand_hash_conditions_for_aggregates, attributes) attributes.stringify_keys! - attributes, binds = predicate_builder.create_binds(attributes, other.last || {}) + attributes, binds = predicate_builder.create_binds(attributes) parts = predicate_builder.build_from_hash(attributes) when Arel::Nodes::Node diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb index f5383f4c14..9ed70a9c2b 100644 --- a/activerecord/lib/active_record/result.rb +++ b/activerecord/lib/active_record/result.rb @@ -32,8 +32,6 @@ module ActiveRecord class Result include Enumerable - IDENTITY_TYPE = Type::Value.new # :nodoc: - attr_reader :columns, :rows, :column_types def initialize(columns, rows, column_types = {}) @@ -105,7 +103,7 @@ module ActiveRecord def column_type(name, type_overrides = {}) type_overrides.fetch(name) do - column_types.fetch(name, IDENTITY_TYPE) + column_types.fetch(name, Type.default_value) end end diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index fd67032235..d19bb96ede 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -99,12 +99,12 @@ module ActiveRecord @bind_map = bind_map end - def execute(params, klass, connection) + def execute(params, klass, connection, &block) bind_values = bind_map.bind params sql = query_builder.sql_for bind_values, connection - klass.find_by_sql(sql, bind_values, preparable: true) + klass.find_by_sql(sql, bind_values, preparable: true, &block) end alias :call :execute end diff --git a/activerecord/lib/active_record/table_metadata.rb b/activerecord/lib/active_record/table_metadata.rb index a2cb3ea1be..58184f3872 100644 --- a/activerecord/lib/active_record/table_metadata.rb +++ b/activerecord/lib/active_record/table_metadata.rb @@ -31,7 +31,7 @@ module ActiveRecord if klass klass.type_for_attribute(column_name.to_s) else - Type::Value.new + Type.default_value end end diff --git a/activerecord/lib/active_record/type.rb b/activerecord/lib/active_record/type.rb index 1b2fc1b034..0b48d2186a 100644 --- a/activerecord/lib/active_record/type.rb +++ b/activerecord/lib/active_record/type.rb @@ -37,6 +37,10 @@ module ActiveRecord registry.lookup(*args, adapter: adapter, **kwargs) end + def default_value # :nodoc: + @default_value ||= Value.new + end + private def current_adapter_name diff --git a/activerecord/lib/active_record/type/type_map.rb b/activerecord/lib/active_record/type/type_map.rb index 9618ff8787..7bce82a1ff 100644 --- a/activerecord/lib/active_record/type/type_map.rb +++ b/activerecord/lib/active_record/type/type_map.rb @@ -11,7 +11,7 @@ module ActiveRecord end def lookup(lookup_key, *args) - fetch(lookup_key, *args) { default_value } + fetch(lookup_key, *args) { Type.default_value } end def fetch(lookup_key, *args, &block) @@ -55,10 +55,6 @@ module ActiveRecord yield lookup_key, *args end end - - def default_value - @default_value ||= ActiveModel::Type::Value.new - end end end end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 08c4b01439..8c4930a81d 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -50,7 +50,37 @@ module ActiveRecord end def build_relation(klass, attribute, value) # :nodoc: - klass.unscoped.where!({ attribute => value }, options) + if reflection = klass._reflect_on_association(attribute) + attribute = reflection.foreign_key + value = value.attributes[reflection.klass.primary_key] unless value.nil? + end + + if value.nil? + return klass.unscoped.where!(attribute => value) + end + + # the attribute may be an aliased attribute + if klass.attribute_alias?(attribute) + attribute = klass.attribute_alias(attribute) + end + + attribute_name = attribute.to_s + + table = klass.arel_table + column = klass.columns_hash[attribute_name] + cast_type = klass.type_for_attribute(attribute_name) + + comparison = if !options[:case_sensitive] + # will use SQL LOWER function before comparison, unless it detects a case insensitive collation + klass.connection.case_insensitive_comparison(table, attribute, column, value) + else + klass.connection.case_sensitive_comparison(table, attribute, column, value) + end + klass.unscoped.tap do |scope| + parts = [comparison] + binds = [Relation::QueryAttribute.new(attribute_name, value, cast_type)] + scope.where_clause += Relation::WhereClause.new(parts, binds) + end end def scope_relation(record, relation) diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 60da9d8859..97960b6c51 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -311,6 +311,12 @@ class PostgresqlArrayTest < ActiveRecord::PostgreSQLTestCase assert_equal ["has already been taken"], e2.errors[:tags], "Should have uniqueness message for tags" end + def test_encoding_arrays_of_utf8_strings + string_with_utf8 = "nový" + assert_equal [string_with_utf8], @type.deserialize(@type.serialize([string_with_utf8])) + assert_equal [[string_with_utf8]], @type.deserialize(@type.serialize([[string_with_utf8]])) + end + private def assert_cycle(field, array) # test creation diff --git a/activerecord/test/cases/adapters/postgresql/geometric_test.rb b/activerecord/test/cases/adapters/postgresql/geometric_test.rb index 27a7467326..a65d4d1ad9 100644 --- a/activerecord/test/cases/adapters/postgresql/geometric_test.rb +++ b/activerecord/test/cases/adapters/postgresql/geometric_test.rb @@ -27,22 +27,10 @@ class PostgresqlPointTest < ActiveRecord::PostgreSQLTestCase t.point :legacy_y, default: [12.2, 13.3] t.point :legacy_z, default: "(14.4,15.5)" end - @connection.create_table("deprecated_points") do |t| - t.point :x - end end teardown do @connection.drop_table "postgresql_points", if_exists: true - @connection.drop_table "deprecated_points", if_exists: true - end - - class DeprecatedPoint < ActiveRecord::Base; end - - def test_deprecated_legacy_type - assert_deprecated do - DeprecatedPoint.new - end end def test_column diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 3f42cb9b9d..2418346d1b 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -626,6 +626,12 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_queries(0) { tagging.super_tag } end + def test_dont_find_target_when_saving_foreign_key_after_stale_association_loaded + client = Client.create!(name: "Test client", firm_with_basic_id: Firm.find(1)) + client.firm_id = Firm.create!(name: "Test firm").id + assert_queries(1) { client.save! } + end + def test_field_name_same_as_foreign_key computer = Computer.find(1) assert_not_nil computer.developer, ":foreign key == attribute didn't lock up" # ' diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 0b23cea420..6fe6ee6783 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -494,6 +494,33 @@ class InverseHasManyTests < ActiveRecord::TestCase assert !man.persisted? end + + def test_inverse_instance_should_be_set_before_find_callbacks_are_run + reset_callbacks(Interest, :find) do + Interest.after_find { raise unless association(:man).loaded? && man.present? } + + assert Man.first.interests.reload.any? + assert Man.includes(:interests).first.interests.any? + assert Man.joins(:interests).includes(:interests).first.interests.any? + end + end + + def test_inverse_instance_should_be_set_before_initialize_callbacks_are_run + reset_callbacks(Interest, :initialize) do + Interest.after_initialize { raise unless association(:man).loaded? && man.present? } + + assert Man.first.interests.reload.any? + assert Man.includes(:interests).first.interests.any? + assert Man.joins(:interests).includes(:interests).first.interests.any? + end + end + + def reset_callbacks(target, type) + old_callbacks = target.send(:get_callbacks, type).deep_dup + yield + ensure + target.send(:set_callbacks, type, old_callbacks) if old_callbacks + end end class InverseBelongsToTests < ActiveRecord::TestCase diff --git a/activerecord/test/cases/attributes_test.rb b/activerecord/test/cases/attributes_test.rb index 7ec1f6f18c..c18851257e 100644 --- a/activerecord/test/cases/attributes_test.rb +++ b/activerecord/test/cases/attributes_test.rb @@ -249,5 +249,11 @@ module ActiveRecord model.foo = "lol" refute model.changed? end + + test "attributes not backed by database columns appear in inspect" do + inspection = OverloadedType.new.inspect + + assert inspection.include?("non_existent_decimal") + end end end diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index 8c9ea2c0e8..28cf2953bf 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -159,7 +159,7 @@ module ActiveSupport error = "#{message}.\n#{error}" if message assert_not_equal before, after, error else - message = "#{expression.inspect} didn't change to #{to}" + error = "#{expression.inspect} didn't change to #{to}" error = "#{message}.\n#{error}" if message assert to === after, error end diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb index dfbb8aa41d..d769a8c145 100644 --- a/activesupport/test/test_case_test.rb +++ b/activesupport/test/test_case_test.rb @@ -211,6 +211,15 @@ class AssertDifferenceTest < ActiveSupport::TestCase end end + def test_assert_changes_with_message + error = assert_raises Minitest::Assertion do + assert_changes "@object.num", "@object.num should 1", to: 1 do + end + end + + assert_equal "@object.num should 1.\n\"@object.num\" didn't change to 1", error.message + end + def test_assert_no_changes_pass assert_no_changes "@object.num" do # ... diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 701304acde..41dfeea84d 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -586,6 +586,19 @@ in your application are included in the `config.assets.precompile` list. If `config.assets.digest` is also true, the asset pipeline will require that all requests for assets include digests. +### Raise an Error When an Asset is Not Found + +If you are using sprockets-rails >= 3.2.0 you can configure what happens +when an asset lookup is performed and nothing is found. If you turn off "asset fallback" +then an error will be raised when an asset cannot be found. + +```ruby +config.assets.unknown_asset_fallback = false +``` + +If "asset fallback" is enabled then when an asset cannot be found the path will be +output instead and no error raised. The asset fallback behavior is enabled by default. + ### Turning Digests Off You can turn off digests by updating `config/environments/development.rb` to diff --git a/guides/source/configuring.md b/guides/source/configuring.md index c938edd8f7..a115683134 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -169,6 +169,8 @@ pipeline is enabled. It is set to `true` by default. * `config.assets.precompile` allows you to specify additional assets (other than `application.css` and `application.js`) which are to be precompiled when `rake assets:precompile` is run. +* `config.assets.unknown_asset_fallback` allows you to modify the behavior of the asset pipeline when an asset is not in the pipeline, if you use sprockets-rails 3.2.0 or newer. Defaults to `true`. + * `config.assets.prefix` defines the prefix where assets are served from. Defaults to `/assets`. * `config.assets.manifest` defines the full path to be used for the asset precompiler's manifest file. Defaults to a file named `manifest-<random>.json` in the `config.assets.prefix` directory within the public folder. diff --git a/guides/source/credits.html.erb b/guides/source/credits.html.erb index 511d76041b..fa81b27036 100644 --- a/guides/source/credits.html.erb +++ b/guides/source/credits.html.erb @@ -32,7 +32,7 @@ Ruby on Rails Guides: Credits <% end %> <%= author('Oscar Del Ben', 'oscardelben', 'oscardelben.jpg') do %> -Oscar Del Ben is a software engineer at <a href="http://www.wildfireapp.com/">Wildfire</a>. He's a regular open source contributor (<a href="https://github.com/oscardelben">GitHub account</a>) and tweets regularly at <a href="https://twitter.com/oscardelben">@oscardelben</a>. +Oscar Del Ben is a software engineer at <a href="http://www.businessinsider.com/google-buys-wildfire-2012-8">Wildfire</a>. He's a regular open source contributor (<a href="https://github.com/oscardelben">GitHub account</a>) and tweets regularly at <a href="https://twitter.com/oscardelben">@oscardelben</a>. <% end %> <%= author('Frederick Cheung', 'fcheung') do %> diff --git a/guides/source/i18n.md b/guides/source/i18n.md index 1565008a69..887774961a 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -1123,7 +1123,7 @@ I18n support in Ruby on Rails was introduced in the release 2.2 and is still evo Thus we encourage everybody to experiment with new ideas and features in gems or other libraries and make them available to the community. (Don't forget to announce your work on our [mailing list](http://groups.google.com/group/rails-i18n)!) -If you find your own locale (language) missing from our [example translations data](https://github.com/svenfuchs/rails-i18n/tree/master/rails/locale) repository for Ruby on Rails, please [_fork_](https://github.com/guides/fork-a-project-and-submit-your-modifications) the repository, add your data and send a [pull request](https://github.com/guides/pull-requests). +If you find your own locale (language) missing from our [example translations data](https://github.com/svenfuchs/rails-i18n/tree/master/rails/locale) repository for Ruby on Rails, please [_fork_](https://github.com/guides/fork-a-project-and-submit-your-modifications) the repository, add your data and send a [pull request](https://help.github.com/articles/about-pull-requests/). Resources diff --git a/guides/source/security.md b/guides/source/security.md index 36eb61be8b..5c3d465220 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -287,7 +287,7 @@ Another class of security vulnerabilities surrounds the use of redirection and f WARNING: _Redirection in a web application is an underestimated cracker tool: Not only can the attacker forward the user to a trap web site, they may also create a self-contained attack._ -Whenever the user is allowed to pass (parts of) the URL for redirection, it is possibly vulnerable. The most obvious attack would be to redirect users to a fake web application which looks and feels exactly as the original one. This so-called phishing attack works by sending an unsuspicious link in an email to the users, injecting the link by XSS in the web application or putting the link into an external site. It is unsuspicious, because the link starts with the URL to the web application and the URL to the malicious site is hidden in the redirection parameter: http://www.example.com/site/redirect?to= www.attacker.com. Here is an example of a legacy action: +Whenever the user is allowed to pass (parts of) the URL for redirection, it is possibly vulnerable. The most obvious attack would be to redirect users to a fake web application which looks and feels exactly as the original one. This so-called phishing attack works by sending an unsuspicious link in an email to the users, injecting the link by XSS in the web application or putting the link into an external site. It is unsuspicious, because the link starts with the URL to the web application and the URL to the malicious site is hidden in the redirection parameter: http://www.example.com/site/redirect?to=www.attacker.com. Here is an example of a legacy action: ```ruby def legacy diff --git a/guides/source/testing.md b/guides/source/testing.md index 4de32d9d77..8f9246dea2 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -239,8 +239,8 @@ Run options: --seed 1808 Error: ArticleTest#test_should_report_error: -NameError: undefined local variable or method `some_undefined_variable' for #<ArticleTest:0x007fee3aa71798> - test/models/article_test.rb:11:in `block in <class:ArticleTest>' +NameError: undefined local variable or method 'some_undefined_variable' for #<ArticleTest:0x007fee3aa71798> + test/models/article_test.rb:11:in 'block in <class:ArticleTest>' bin/rails test test/models/article_test.rb:9 @@ -369,7 +369,7 @@ documentation](http://docs.seattlerb.org/minitest). We can run all of our tests at once by using the `bin/rails test` command. -Or we can run a single test by passing the `bin/rails test` command the filename containing the test cases. +Or we can run a single test file by passing the `bin/rails test` command the filename containing the test cases. ```bash $ bin/rails test test/models/article_test.rb @@ -763,16 +763,11 @@ The `get` method kicks off the web request and populates the results into the `@ * The action of the controller you are requesting. This can be in the form of a string or a route (i.e. `articles_url`). - * `params`: option with a hash of request parameters to pass into the action (e.g. query string parameters or article variables). - * `headers`: for setting the headers that will be passed with the request. - * `env`: for customizing the request environment as needed. - * `xhr`: whether the request is Ajax request or not. Can be set to true for marking the request as Ajax. - * `as`: for encoding the request with different content type. Supports `:json` by default. All of these keyword arguments are optional. @@ -865,8 +860,8 @@ class ArticlesControllerTest < ActionDispatch::IntegrationTest test "should get index" do get articles_url - assert_equal "index", @controller.action_name - assert_equal "application/x-www-form-urlencoded", @request.media_type + assert_equal "index", @controller.action_name + assert_equal "application/x-www-form-urlencoded", @request.media_type assert_match "Articles", @response.body end end @@ -1056,7 +1051,7 @@ To avoid code duplication, you can add your own test helpers. Sign in helper can be a good example: ```ruby -#test/test_helper.rb +# test/test_helper.rb module SignInHelper def sign_in_as(user) @@ -1362,7 +1357,7 @@ Here is an example using the [`travel_to`](http://api.rubyonrails.org/classes/Ac user = User.create(name: 'Gaurish', activation_date: Date.new(2004, 10, 24)) assert_not user.applicable_for_gifting? travel_to Date.new(2004, 11, 24) do - assert_equal Date.new(2004, 10, 24), user.activation_date # inside the travel_to block `Date.current` is mocked + assert_equal Date.new(2004, 10, 24), user.activation_date # inside the `travel_to` block `Date.current` is mocked assert user.applicable_for_gifting? end assert_equal Date.new(2004, 10, 24), user.activation_date # The change was visible only inside the `travel_to` block. diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults.rb.tt index e539f4c457..5ad18cc5ad 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults.rb.tt @@ -32,3 +32,7 @@ ActiveSupport.halt_callback_chains_on_return_false = <%= options[:update] ? true # Configure SSL options to enable HSTS with subdomains. Previous versions had false. Rails.application.config.ssl_options = { hsts: { subdomains: true } } <%- end -%> + +# Unknown asset fallback will return the path passed in when the given +# asset is not present in the asset pipeline. +Rails.application.config.assets.unknown_asset_fallback = <%= options[:update] ? true : false %> diff --git a/railties/test/application/asset_debugging_test.rb b/railties/test/application/asset_debugging_test.rb index a732869d62..3e17a1efa5 100644 --- a/railties/test/application/asset_debugging_test.rb +++ b/railties/test/application/asset_debugging_test.rb @@ -68,5 +68,102 @@ module ApplicationTests assert_match(/<script src="\/assets\/application(\.self)?-([0-z]+)\.js\?body=1"><\/script>/, last_response.body) assert_match(/<script src="\/assets\/xmlhr(\.self)?-([0-z]+)\.js\?body=1"><\/script>/, last_response.body) end + + test "public path and tag methods are not over-written by the asset pipeline" do + contents = "doesnotexist" + cases = { + asset_path: %r{/#{contents}}, + image_path: %r{/images/#{contents}}, + video_path: %r{/videos/#{contents}}, + audio_path: %r{/audios/#{contents}}, + font_path: %r{/fonts/#{contents}}, + javascript_path: %r{/javascripts/#{contents}}, + stylesheet_path: %r{/stylesheets/#{contents}}, + image_tag: %r{<img src="/images/#{contents}"}, + favicon_link_tag: %r{<link rel="shortcut icon" type="image/x-icon" href="/images/#{contents}" />}, + stylesheet_link_tag: %r{<link rel="stylesheet" media="screen" href="/stylesheets/#{contents}.css" />}, + javascript_include_tag: %r{<script src="/javascripts/#{contents}.js">}, + audio_tag: %r{<audio src="/audios/#{contents}"></audio>}, + video_tag: %r{<video src="/videos/#{contents}"></video>} + } + + cases.each do |(view_method, tag_match)| + app_file "app/views/posts/index.html.erb", "<%= #{view_method} '#{contents}', skip_pipeline: true %>" + + app "development" + + class ::PostsController < ActionController::Base ; end + + get "/posts?debug_assets=true" + + body = last_response.body + assert_match(tag_match, body, "Expected `#{view_method}` to produce a match to #{tag_match}, but did not: #{body}") + end + end + + test "public url methods are not over-written by the asset pipeline" do + contents = "doesnotexist" + cases = { + asset_url: %r{http://example.org/#{contents}}, + image_url: %r{http://example.org/images/#{contents}}, + video_url: %r{http://example.org/videos/#{contents}}, + audio_url: %r{http://example.org/audios/#{contents}}, + font_url: %r{http://example.org/fonts/#{contents}}, + javascript_url: %r{http://example.org/javascripts/#{contents}}, + stylesheet_url: %r{http://example.org/stylesheets/#{contents}}, + } + + cases.each do |(view_method, tag_match)| + app_file "app/views/posts/index.html.erb", "<%= #{view_method} '#{contents}', skip_pipeline: true %>" + + app "development" + + class ::PostsController < ActionController::Base ; end + + get "/posts?debug_assets=true" + + body = last_response.body + assert_match(tag_match, body, "Expected `#{view_method}` to produce a match to #{tag_match}, but did not: #{body}") + end + end + + test "{ skip_pipeline: true } does not use the asset pipeline" do + cases = { + /\/assets\/application-.*.\.js/ => {}, + /application.js/ => { skip_pipeline: true }, + } + cases.each do |(tag_match, options_hash)| + app_file "app/views/posts/index.html.erb", "<%= asset_path('application.js', #{options_hash}) %>" + + app "development" + + class ::PostsController < ActionController::Base ; end + + get "/posts?debug_assets=true" + + body = last_response.body.strip + assert_match(tag_match, body, "Expected `asset_path` with `#{options_hash}` to produce a match to #{tag_match}, but did not: #{body}") + end + end + + test "public_compute_asset_path does not use the asset pipeline" do + cases = { + compute_asset_path: /\/assets\/application-.*.\.js/, + public_compute_asset_path: /application.js/, + } + + cases.each do |(view_method, tag_match)| + app_file "app/views/posts/index.html.erb", "<%= #{ view_method } 'application.js' %>" + + app "development" + + class ::PostsController < ActionController::Base ; end + + get "/posts?debug_assets=true" + + body = last_response.body.strip + assert_match(tag_match, body, "Expected `#{view_method}` to produce a match to #{ tag_match }, but did not: #{ body }") + end + end end end |