diff options
46 files changed, 268 insertions, 154 deletions
diff --git a/.travis.yml b/.travis.yml index 847e11900a..91ac7e8e5e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,7 +11,6 @@ cache: services: - memcached - - redis addons: postgresql: "9.4" @@ -34,6 +33,7 @@ before_script: # Decodes to e.g. `export VARIABLE=VALUE` - $(base64 --decode <<< "ZXhwb3J0IFNBVUNFX0FDQ0VTU19LRVk9YTAzNTM0M2YtZTkyMi00MGIzLWFhM2MtMDZiM2VhNjM1YzQ4") - $(base64 --decode <<< "ZXhwb3J0IFNBVUNFX1VTRVJOQU1FPXJ1YnlvbnJhaWxz") + - redis-server --bind 127.0.0.1 --port 6379 --requirepass 'password' --daemonize yes script: 'ci/travis.rb' @@ -65,25 +65,21 @@ matrix: env: "GEM=aj:integration" services: - memcached - - redis - rabbitmq - rvm: 2.3.4 env: "GEM=aj:integration" services: - memcached - - redis - rabbitmq - rvm: 2.4.1 env: "GEM=aj:integration" services: - memcached - - redis - rabbitmq - rvm: ruby-head env: "GEM=aj:integration" services: - memcached - - redis - rabbitmq - rvm: 2.3.4 env: diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index 2bf11da463..b1408496a0 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -1,3 +1,12 @@ +* ActionCable's `redis` adapter allows for other common redis-rb options (`host`, `port`, `db`, `password`) in cable.yml. + + Previously, it accepts only a [redis:// url](https://www.iana.org/assignments/uri-schemes/prov/redis) as an option. + While we can add all of these options to the `url` itself, it is not explicitly documented. This alternative setup + is shown as the first example in the [Redis rubygem](https://github.com/redis/redis-rb#getting-started), which + makes this set of options as sensible as using just the `url`. + + *Marc Rendl Ignacio* + * ActionCable socket errors are now logged to the console Previously any socket errors were ignored and this made it hard to diagnose socket issues (e.g. as discussed in #28362). diff --git a/actioncable/lib/action_cable/subscription_adapter/redis.rb b/actioncable/lib/action_cable/subscription_adapter/redis.rb index a31ed33bdb..225609c236 100644 --- a/actioncable/lib/action_cable/subscription_adapter/redis.rb +++ b/actioncable/lib/action_cable/subscription_adapter/redis.rb @@ -10,7 +10,9 @@ module ActionCable # Overwrite this factory method for redis connections if you want to use a different Redis library than Redis. # This is needed, for example, when using Makara proxies for distributed Redis. - cattr_accessor :redis_connector, default: ->(config) { ::Redis.new(url: config[:url]) } + cattr_accessor :redis_connector, default: ->(config) do + ::Redis.new(config.slice(:url, :host, :port, :db, :password)) + end def initialize(*) super diff --git a/actioncable/test/subscription_adapter/evented_redis_test.rb b/actioncable/test/subscription_adapter/evented_redis_test.rb index 5453511549..1c99031ab0 100644 --- a/actioncable/test/subscription_adapter/evented_redis_test.rb +++ b/actioncable/test/subscription_adapter/evented_redis_test.rb @@ -54,6 +54,6 @@ class EventedRedisAdapterTest < ActionCable::TestCase end def cable_config - { adapter: "evented_redis", url: "redis://127.0.0.1:6379/12" } + { adapter: "evented_redis", url: "redis://:password@127.0.0.1:6379/12" } end end diff --git a/actioncable/test/subscription_adapter/redis_test.rb b/actioncable/test/subscription_adapter/redis_test.rb index 11120b3f85..60596dd205 100644 --- a/actioncable/test/subscription_adapter/redis_test.rb +++ b/actioncable/test/subscription_adapter/redis_test.rb @@ -7,7 +7,7 @@ class RedisAdapterTest < ActionCable::TestCase include ChannelPrefixTest def cable_config - { adapter: "redis", driver: "ruby", url: "redis://127.0.0.1:6379/12" } + { adapter: "redis", driver: "ruby", url: "redis://:password@127.0.0.1:6379/12" } end end @@ -16,3 +16,11 @@ class RedisAdapterTest::Hiredis < RedisAdapterTest super.merge(driver: "hiredis") end end + +class RedisAdapterTest::AlternateConfiguration < RedisAdapterTest + def cable_config + alt_cable_config = super.dup + alt_cable_config.delete(:url) + alt_cable_config.merge(host: "127.0.0.1", port: 6379, db: 12, password: "password") + end +end diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index 68881b8402..44151c9f71 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -282,7 +282,7 @@ module ActionController return false unless request.has_content_type? ref = request.content_mime_type.ref - _wrapper_formats.include?(ref) && _wrapper_key && !request.request_parameters[_wrapper_key] + _wrapper_formats.include?(ref) && _wrapper_key && !request.request_parameters.key?(_wrapper_key) end end end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 4a60e2eff2..cd6a0c0b98 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -245,7 +245,7 @@ module ActionController # oddity: "Heavy stone crab" # }) # params.to_h - # # => ActionController::UnfilteredParameters: unable to convert unfiltered parameters to hash + # # => ActionController::UnfilteredParameters: unable to convert unpermitted parameters to hash # # safe_params = params.permit(:name) # safe_params.to_h # => {"name"=>"Senjougahara Hitagi"} @@ -265,7 +265,7 @@ module ActionController # oddity: "Heavy stone crab" # }) # params.to_hash - # # => ActionController::UnfilteredParameters: unable to convert unfiltered parameters to hash + # # => ActionController::UnfilteredParameters: unable to convert unpermitted parameters to hash # # safe_params = params.permit(:name) # safe_params.to_hash # => {"name"=>"Senjougahara Hitagi"} diff --git a/actionpack/lib/action_dispatch/system_testing/server.rb b/actionpack/lib/action_dispatch/system_testing/server.rb index 4a214ef713..89ca6944d9 100644 --- a/actionpack/lib/action_dispatch/system_testing/server.rb +++ b/actionpack/lib/action_dispatch/system_testing/server.rb @@ -3,6 +3,12 @@ require "rack/handler/puma" module ActionDispatch module SystemTesting class Server # :nodoc: + class << self + attr_accessor :silence_puma + end + + self.silence_puma = false + def run register setup @@ -11,7 +17,12 @@ module ActionDispatch private def register Capybara.register_server :rails_puma do |app, port, host| - Rack::Handler::Puma.run(app, Port: port, Threads: "0:1") + Rack::Handler::Puma.run( + app, + Port: port, + Threads: "0:1", + Silent: self.class.silence_puma + ) end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 2416c58817..f16647fac8 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -338,8 +338,7 @@ module ActionDispatch @integration_session = nil end - %w(get post patch put head delete cookies assigns - xml_http_request xhr get_via_redirect post_via_redirect).each do |method| + %w(get post patch put head delete cookies assigns follow_redirect!).each do |method| define_method(method) do |*args| # reset the html_document variable, except for cookies/assigns calls unless method == "cookies" || method == "assigns" diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 72163ccd5e..cb282d4330 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -335,6 +335,18 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end end + def test_redirect_reset_html_document + with_test_route_set do + get "/redirect" + previous_html_document = html_document + + follow_redirect! + + assert_response :ok + refute_same previous_html_document, html_document + end + end + def test_xml_http_request_get with_test_route_set do get "/get", xhr: true diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb index 2a41d57b26..4cbb28ef60 100644 --- a/actionpack/test/controller/params_wrapper_test.rb +++ b/actionpack/test/controller/params_wrapper_test.rb @@ -170,6 +170,14 @@ class ParamsWrapperTest < ActionController::TestCase end end + def test_no_double_wrap_if_key_exists_and_value_is_nil + with_default_wrapper_options do + @request.env["CONTENT_TYPE"] = "application/json" + post :parse, params: { "user" => nil } + assert_parameters("user" => nil) + end + end + def test_nested_params with_default_wrapper_options do @request.env["CONTENT_TYPE"] = "application/json" diff --git a/activejob/.gitignore b/activejob/.gitignore deleted file mode 100644 index b3aaf55871..0000000000 --- a/activejob/.gitignore +++ /dev/null @@ -1 +0,0 @@ -test/dummy diff --git a/activejob/lib/active_job/core.rb b/activejob/lib/active_job/core.rb index 548ec89ee2..e3e63f227e 100644 --- a/activejob/lib/active_job/core.rb +++ b/activejob/lib/active_job/core.rb @@ -80,6 +80,7 @@ module ActiveJob { "job_class" => self.class.name, "job_id" => job_id, + "provider_job_id" => provider_job_id, "queue_name" => queue_name, "priority" => priority, "arguments" => serialize_arguments(arguments), diff --git a/activejob/test/cases/job_serialization_test.rb b/activejob/test/cases/job_serialization_test.rb index 3f2e300dfa..c737557ece 100644 --- a/activejob/test/cases/job_serialization_test.rb +++ b/activejob/test/cases/job_serialization_test.rb @@ -44,4 +44,12 @@ class JobSerializationTest < ActiveSupport::TestCase job.deserialize({}) assert_equal "en", job.locale end + + test "serialize stores provider_job_id" do + job = HelloJob.new + assert_nil job.serialize["provider_job_id"] + + job.provider_job_id = "some value set by adapter" + assert_equal job.provider_job_id, job.serialize["provider_job_id"] + end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c4ee75c9a2..d4f4041910 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,19 @@ +* Merging two relations representing nested joins no longer transforms the joins of + the merged relation into LEFT OUTER JOIN. Example to clarify: + + ``` + Author.joins(:posts).merge(Post.joins(:comments)) + # Before the change: + #=> SELECT ... FROM authors INNER JOIN posts ON ... LEFT OUTER JOIN comments ON... + + # After the change: + #=> SELECT ... FROM authors INNER JOIN posts ON ... INNER JOIN comments ON... + ``` + + TODO: Add to the Rails 5.2 upgrade guide + + *Maxime Handfield Lapointe* + * `ActiveRecord::Persistence#touch` does not work well when optimistic locking enabled and `locking_column`, without default value, is null in the database. diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb index 4a5c821607..104de4f69d 100644 --- a/activerecord/lib/active_record/associations/alias_tracker.rb +++ b/activerecord/lib/active_record/associations/alias_tracker.rb @@ -4,21 +4,21 @@ module ActiveRecord module Associations # Keeps track of table aliases for ActiveRecord::Associations::JoinDependency class AliasTracker # :nodoc: - def self.create(connection, initial_table, type_caster) + def self.create(connection, initial_table) aliases = Hash.new(0) aliases[initial_table] = 1 - new connection, aliases, type_caster + new(connection, aliases) end - def self.create_with_joins(connection, initial_table, joins, type_caster) + def self.create_with_joins(connection, initial_table, joins) if joins.empty? - create(connection, initial_table, type_caster) + create(connection, initial_table) else aliases = Hash.new { |h, k| h[k] = initial_count_for(connection, k, joins) } aliases[initial_table] = 1 - new connection, aliases, type_caster + new(connection, aliases) end end @@ -51,17 +51,16 @@ module ActiveRecord end # table_joins is an array of arel joins which might conflict with the aliases we assign here - def initialize(connection, aliases, type_caster) + def initialize(connection, aliases) @aliases = aliases @connection = connection - @type_caster = type_caster end - def aliased_table_for(table_name, aliased_name) + def aliased_table_for(table_name, aliased_name, type_caster) if aliases[table_name].zero? # If it's zero, we can have our table_name aliases[table_name] = 1 - Arel::Table.new(table_name, type_caster: @type_caster) + Arel::Table.new(table_name, type_caster: type_caster) else # Otherwise, we need to use an alias aliased_name = @connection.table_alias_for(aliased_name) @@ -74,7 +73,7 @@ module ActiveRecord else aliased_name end - Arel::Table.new(table_name, type_caster: @type_caster).alias(table_alias) + Arel::Table.new(table_name, type_caster: type_caster).alias(table_alias) end end diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 1593b94f0c..1744dc785f 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -21,7 +21,7 @@ module ActiveRecord reflection = association.reflection scope = klass.unscoped owner = association.owner - alias_tracker = AliasTracker.create connection, association.klass.table_name, klass.type_caster + alias_tracker = AliasTracker.create connection, association.klass.table_name chain_head, chain_tail = get_chain(reflection, association, alias_tracker) scope.extending! reflection.extensions @@ -112,7 +112,11 @@ module ActiveRecord runtime_reflection = Reflection::RuntimeReflection.new(reflection, association) previous_reflection = runtime_reflection reflection.chain.drop(1).each do |refl| - alias_name = tracker.aliased_table_for(refl.table_name, refl.alias_candidate(name)) + alias_name = tracker.aliased_table_for( + refl.table_name, + refl.alias_candidate(name), + refl.klass.type_caster + ) proxy = ReflectionProxy.new(refl, alias_name) previous_reflection.next = proxy previous_reflection = proxy @@ -138,7 +142,7 @@ module ActiveRecord # Exclude the scope of the association itself, because that # was already merged in the #scope method. reflection.constraints.each do |scope_chain_item| - item = eval_scope(reflection.klass, table, scope_chain_item, owner) + item = eval_scope(reflection, table, scope_chain_item, owner) if scope_chain_item == refl.scope scope.merge! item.except(:where, :includes) @@ -159,9 +163,8 @@ module ActiveRecord scope end - def eval_scope(klass, table, scope, owner) - predicate_builder = PredicateBuilder.new(TableMetadata.new(klass, table)) - ActiveRecord::Relation.create(klass, table, predicate_builder).instance_exec(owner, &scope) + def eval_scope(reflection, table, scope, owner) + reflection.build_scope(table).instance_exec(owner, &scope) end end end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 8cdb508c43..d77fcaf668 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -1114,6 +1114,7 @@ module ActiveRecord end def reset_scope # :nodoc: + @offsets = {} @scope = nil self end diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 643226267c..bc66194aef 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -93,7 +93,7 @@ module ActiveRecord # joins # => [] # def initialize(base, associations, joins, eager_loading: true) - @alias_tracker = AliasTracker.create_with_joins(base.connection, base.table_name, joins, base.type_caster) + @alias_tracker = AliasTracker.create_with_joins(base.connection, base.table_name, joins) @eager_loading = eager_loading tree = self.class.make_tree associations @join_root = JoinBase.new base, build(tree, base) @@ -104,17 +104,17 @@ module ActiveRecord join_root.drop(1).map!(&:reflection) end - def join_constraints(outer_joins, join_type) + def join_constraints(joins_to_add, join_type) joins = join_root.children.flat_map { |child| make_join_constraints(join_root, child, join_type) } - joins.concat outer_joins.flat_map { |oj| + joins.concat joins_to_add.flat_map { |oj| if join_root.match? oj.join_root walk join_root, oj.join_root else oj.join_root.children.flat_map { |child| - make_outer_joins oj.join_root, child + make_join_constraints(oj.join_root, child, join_type) } end } @@ -185,7 +185,8 @@ module ActiveRecord node.reflection.chain.map { |reflection| alias_tracker.aliased_table_for( reflection.table_name, - table_alias_for(reflection, parent, reflection != node.reflection) + table_alias_for(reflection, parent, reflection != node.reflection), + reflection.klass.type_caster ) } end diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 97cfec0302..005410d598 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -34,34 +34,10 @@ module ActiveRecord table = tables.shift klass = reflection.klass - join_keys = reflection.join_keys - key = join_keys.key - foreign_key = join_keys.foreign_key + join_scope = reflection.join_scope(table, foreign_table, foreign_klass) - constraint = build_constraint(klass, table, key, foreign_table, foreign_key) - - predicate_builder = PredicateBuilder.new(TableMetadata.new(klass, table)) - scope_chain_items = reflection.join_scopes(table, predicate_builder) - klass_scope = reflection.klass_join_scope(table, predicate_builder) - - scope_chain_items.concat [klass_scope].compact - - rel = scope_chain_items.inject(scope_chain_items.shift) do |left, right| - left.merge right - end - - if rel && !rel.arel.constraints.empty? - binds += rel.bound_attributes - constraint = constraint.and rel.arel.constraints - end - - if reflection.type - value = foreign_klass.base_class.name - column = klass.columns_hash[reflection.type.to_s] - - binds << Relation::QueryAttribute.new(column.name, value, klass.type_for_attribute(column.name)) - constraint = constraint.and klass.arel_attribute(reflection.type, table).eq(Arel::Nodes::BindParam.new) - end + binds.concat join_scope.bound_attributes + constraint = join_scope.arel.constraints joins << table.create_join(table, table.create_on(constraint), join_type) @@ -72,34 +48,6 @@ module ActiveRecord JoinInformation.new joins, binds end - # Builds equality condition. - # - # Example: - # - # class Physician < ActiveRecord::Base - # has_many :appointments - # end - # - # If I execute `Physician.joins(:appointments).to_a` then - # klass # => Physician - # table # => #<Arel::Table @name="appointments" ...> - # key # => physician_id - # foreign_table # => #<Arel::Table @name="physicians" ...> - # foreign_key # => id - # - def build_constraint(klass, table, key, foreign_table, foreign_key) - constraint = table[key].eq(foreign_table[foreign_key]) - - if klass.finder_needs_type_condition? - constraint = table.create_and([ - constraint, - klass.send(:type_condition, table) - ]) - end - - constraint - end - def table tables.first end diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 9f77f38b35..208d1b2670 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -147,7 +147,7 @@ module ActiveRecord def preloaders_for_one(association, records, scope) grouped_records(association, records).flat_map do |reflection, klasses| klasses.map do |rhs_klass, rs| - loader = preloader_for(reflection, rs, rhs_klass).new(rhs_klass, rs, reflection, scope) + loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope) loader.run self loader end @@ -159,6 +159,7 @@ module ActiveRecord records.each do |record| next unless record assoc = record.association(association) + next unless assoc.klass klasses = h[assoc.reflection] ||= {} (klasses[assoc.klass] ||= []) << record end @@ -180,20 +181,11 @@ module ActiveRecord end end - class NullPreloader # :nodoc: - def self.new(klass, owners, reflection, preload_scope); self; end - def self.run(preloader); end - def self.preloaded_records; []; end - def self.owners; []; end - end - # Returns a class containing the logic needed to load preload the data # and attach it to a relation. For example +Preloader::Association+ or # +Preloader::HasManyThrough+. The class returned implements a `run` method # that accepts a preloader. - def preloader_for(reflection, owners, rhs_klass) - return NullPreloader unless rhs_klass - + def preloader_for(reflection, owners) if owners.first.association(reflection.name).loaded? return AlreadyLoaded end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index dc766ce4fc..af5314c1d6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -425,12 +425,11 @@ module ActiveRecord # are not quotable. In this case we want to convert # the column value to YAML. def with_yaml_fallback(value) - begin - quote(value) - rescue TypeError - value = YAML.dump(value) + if value.is_a?(Hash) || value.is_a?(Array) + YAML.dump(value) + else + value end - value end end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index e8ee8279fd..3d3ec862a3 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -171,7 +171,7 @@ module ActiveRecord JoinKeys = Struct.new(:key, :foreign_key) # :nodoc: def join_keys - get_join_keys klass + @join_keys ||= get_join_keys(klass) end # Returns a list of scopes that should be applied for this Reflection @@ -185,10 +185,30 @@ module ActiveRecord end deprecate :scope_chain + def join_scope(table, foreign_table, foreign_klass) + predicate_builder = predicate_builder(table) + scope_chain_items = join_scopes(table, predicate_builder) + klass_scope = klass_join_scope(table, predicate_builder) + + key = join_keys.key + foreign_key = join_keys.foreign_key + + klass_scope.where!(table[key].eq(foreign_table[foreign_key])) + + if klass.finder_needs_type_condition? + klass_scope.where!(klass.send(:type_condition, table)) + end + + if type + klass_scope.where!(type => foreign_klass.base_class.name) + end + + scope_chain_items.inject(klass_scope, &:merge!) + end + def join_scopes(table, predicate_builder) # :nodoc: if scope - [ActiveRecord::Relation.create(klass, table, predicate_builder) - .instance_exec(&scope)] + [build_scope(table, predicate_builder).instance_exec(&scope)] else [] end @@ -200,12 +220,7 @@ module ActiveRecord scope.joins_values = scope.left_outer_joins_values = [].freeze } else - relation = ActiveRecord::Relation.create( - klass, - table, - predicate_builder, - ) - klass.send(:build_default_scope, relation) + klass.default_scoped(build_scope(table, predicate_builder)) end end @@ -287,12 +302,19 @@ module ActiveRecord JoinKeys.new(join_pk(association_klass), join_fk) end + def build_scope(table, predicate_builder = predicate_builder(table)) + Relation.create(klass, table, predicate_builder) + end + protected def actual_source_reflection # FIXME: this is a horrible name self end private + def predicate_builder(table) + PredicateBuilder.new(TableMetadata.new(klass, table)) + end def join_pk(_) foreign_key diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 1d661fa8ed..5ec2dfcfc9 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -147,8 +147,7 @@ module ActiveRecord def last(limit = nil) return find_last(limit) if loaded? || limit_value - result = limit(limit) - result.order!(arel_attribute(primary_key)) if order_values.empty? && primary_key + result = ordered_relation.limit(limit) result = result.reverse_order! limit ? result.reverse : result.first @@ -535,11 +534,7 @@ module ActiveRecord if loaded? records[index, limit] || [] else - relation = if order_values.empty? && primary_key - order(arel_attribute(primary_key).asc) - else - self - end + relation = ordered_relation if limit_value.nil? || index < limit_value relation = relation.offset(offset_index + index) unless index.zero? @@ -554,11 +549,7 @@ module ActiveRecord if loaded? records[-index] else - relation = if order_values.empty? && primary_key - order(arel_attribute(primary_key).asc) - else - self - end + relation = ordered_relation relation.to_a[-index] # TODO: can be made more performant on large result sets by @@ -572,5 +563,13 @@ module ActiveRecord def find_last(limit) limit ? records.last(limit) : records.last end + + def ordered_relation + if order_values.empty? && primary_key + order(arel_attribute(primary_key).asc) + else + self + 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 6ccdd7adcb..d44f6fd572 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1122,7 +1122,7 @@ module ActiveRecord validate_order_args(order_args) references = order_args.grep(String) - references.map! { |arg| arg =~ /^([a-zA-Z]\w*)\.(\w+)/ && $1 }.compact! + references.map! { |arg| arg =~ /^\W?(\w+)\W?\./ && $1 }.compact! references!(references) if references.any? # if a symbol is given we prepend the quoted table name diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index a61fdd6454..388f471bf5 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -29,8 +29,7 @@ module ActiveRecord end end - def default_scoped # :nodoc: - scope = relation + def default_scoped(scope = relation) # :nodoc: build_default_scope(scope) || scope end diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 541165b3d1..c25d87dd3e 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -59,7 +59,6 @@ module ActiveRecord args.concat(["--no-data"]) args.concat(["--routines"]) args.concat(["--skip-comments"]) - args.concat(Array(extra_flags)) if extra_flags ignore_tables = ActiveRecord::SchemaDumper.ignore_tables if ignore_tables.any? @@ -67,6 +66,7 @@ module ActiveRecord end args.concat(["#{configuration['database']}"]) + args.unshift(*extra_flags) if extra_flags run_cmd("mysqldump", args, "dumping") end @@ -75,7 +75,7 @@ module ActiveRecord args = prepare_command_options args.concat(["--execute", %{SET FOREIGN_KEY_CHECKS = 0; SOURCE #{filename}; SET FOREIGN_KEY_CHECKS = 1}]) args.concat(["--database", "#{configuration['database']}"]) - args.concat(Array(extra_flags)) if extra_flags + args.unshift(*extra_flags) if extra_flags run_cmd("mysql", args, "loading") end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 590d3642bd..169e1a5449 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -741,6 +741,41 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal client2, firm.clients.merge!(where: ["#{QUOTED_TYPE} = :type", { type: "Client" }], order: "id").first end + def test_find_first_after_reset_scope + firm = Firm.all.merge!(order: "id").first + collection = firm.clients + + original_object_id = collection.first.object_id + assert_equal original_object_id, collection.first.object_id, "Expected second call to #first to cache the same object" + + # It should return a different object, since the association has been reloaded + assert_not_equal original_object_id, firm.clients.first.object_id, "Expected #first to return a new object" + end + + def test_find_first_after_reset + firm = Firm.all.merge!(order: "id").first + collection = firm.clients + + original_object_id = collection.first.object_id + assert_equal original_object_id, collection.first.object_id, "Expected second call to #first to cache the same object" + collection.reset + + # It should return a different object, since the association has been reloaded + assert_not_equal original_object_id, collection.first.object_id, "Expected #first after #reload to return a new object" + end + + def test_find_first_after_reload + firm = Firm.all.merge!(order: "id").first + collection = firm.clients + + original_object_id = collection.first.object_id + assert_equal original_object_id, collection.first.object_id, "Expected second call to #first to cache the same object" + collection.reset + + # It should return a different object, since the association has been reloaded + assert_not_equal original_object_id, collection.first.object_id, "Expected #first after #reload to return a new object" + end + def test_find_all_with_include_and_conditions assert_nothing_raised do Developer.all.merge!(joins: :audit_logs, where: { "audit_logs.message" => nil, :name => "Smith" }).to_a diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 5fb32270b7..a403824f1a 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -6,7 +6,7 @@ require "models/rating" module ActiveRecord class RelationTest < ActiveRecord::TestCase - fixtures :posts, :comments, :authors, :author_addresses + fixtures :posts, :comments, :authors, :author_addresses, :ratings FakeKlass = Struct.new(:table_name, :name) do extend ActiveRecord::Delegation::DelegateCache @@ -224,7 +224,26 @@ module ActiveRecord def test_relation_merging_with_merged_joins_as_symbols special_comments_with_ratings = SpecialComment.joins(:ratings) posts_with_special_comments_with_ratings = Post.group("posts.id").joins(:special_comments).merge(special_comments_with_ratings) - assert_equal({ 2 => 1, 4 => 3, 5 => 1 }, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count) + assert_equal({ 4 => 2 }, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count) + end + + def test_relation_merging_with_merged_symbol_joins_keeps_inner_joins + queries = capture_sql { Author.joins(:posts).merge(Post.joins(:comments)).to_a } + + nb_inner_join = queries.sum { |sql| sql.scan(/INNER\s+JOIN/i).size } + assert_equal 2, nb_inner_join, "Wrong amount of INNER JOIN in query" + assert queries.none? { |sql| /LEFT\s+(OUTER)?\s+JOIN/i.match?(sql) }, "Shouldn't have any LEFT JOIN in query" + end + + def test_relation_merging_with_merged_symbol_joins_has_correct_size_and_count + # Has one entry per comment + merged_authors_with_commented_posts_relation = Author.joins(:posts).merge(Post.joins(:comments)) + + post_ids_with_author = Post.joins(:author).pluck(:id) + manual_comments_on_post_that_have_author = Comment.where(post_id: post_ids_with_author).pluck(:id) + + assert_equal manual_comments_on_post_that_have_author.size, merged_authors_with_commented_posts_relation.count + assert_equal manual_comments_on_post_that_have_author.size, merged_authors_with_commented_posts_relation.to_a.size end def test_relation_merging_with_joins_as_join_dependency_pick_proper_parent diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 86f3b0b962..dc81f13fe9 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1717,6 +1717,9 @@ class RelationTest < ActiveRecord::TestCase scope = Post.order("comments.body") assert_equal ["comments"], scope.references_values + scope = Post.order("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}") + assert_equal ["comments"], scope.references_values + scope = Post.order("comments.body", "yaks.body") assert_equal ["comments", "yaks"], scope.references_values @@ -1735,6 +1738,9 @@ class RelationTest < ActiveRecord::TestCase scope = Post.reorder("comments.body") assert_equal %w(comments), scope.references_values + scope = Post.reorder("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}") + assert_equal ["comments"], scope.references_values + scope = Post.reorder("comments.body", "yaks.body") assert_equal %w(comments yaks), scope.references_values diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index c22d974536..9c6fb14376 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -296,7 +296,7 @@ if current_adapter?(:Mysql2Adapter) def test_structure_dump_with_extra_flags filename = "awesome-file.sql" - expected_command = ["mysqldump", "--result-file", filename, "--no-data", "--routines", "--skip-comments", "--noop", "test-db"] + expected_command = ["mysqldump", "--noop", "--result-file", filename, "--no-data", "--routines", "--skip-comments", "test-db"] assert_called_with(Kernel, :system, expected_command, returns: true) do with_structure_dump_flags(["--noop"]) do @@ -364,7 +364,7 @@ if current_adapter?(:Mysql2Adapter) def test_structure_load filename = "awesome-file.sql" - expected_command = ["mysql", "--execute", %{SET FOREIGN_KEY_CHECKS = 0; SOURCE #{filename}; SET FOREIGN_KEY_CHECKS = 1}, "--database", "test-db", "--noop"] + expected_command = ["mysql", "--noop", "--execute", %{SET FOREIGN_KEY_CHECKS = 0; SOURCE #{filename}; SET FOREIGN_KEY_CHECKS = 1}, "--database", "test-db"] assert_called_with(Kernel, :system, expected_command, returns: true) do with_structure_load_flags(["--noop"]) do diff --git a/activerecord/test/models/membership.rb b/activerecord/test/models/membership.rb index 2c3ad230a7..0f8be0ad85 100644 --- a/activerecord/test/models/membership.rb +++ b/activerecord/test/models/membership.rb @@ -1,4 +1,5 @@ class Membership < ActiveRecord::Base + enum type: %i(Membership CurrentMembership SuperMembership SelectedMembership TenantMembership) belongs_to :member belongs_to :club end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 6da0ef26f6..d7ceb6d4ef 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -491,7 +491,7 @@ ActiveRecord::Schema.define do t.datetime :joined_on t.integer :club_id, :member_id t.boolean :favourite, default: false - t.string :type + t.integer :type end create_table :member_types, force: true do |t| diff --git a/guides/bug_report_templates/active_record_migrations_gem.rb b/guides/bug_report_templates/active_record_migrations_gem.rb index 0c398e334a..00ba3c1cd6 100644 --- a/guides/bug_report_templates/active_record_migrations_gem.rb +++ b/guides/bug_report_templates/active_record_migrations_gem.rb @@ -48,16 +48,14 @@ end class BugTest < Minitest::Test def test_migration_up - migrator = ActiveRecord::Migrator.new(:up, [ChangeAmountToAddScale]) - migrator.run + ChangeAmountToAddScale.migrate(:up) Payment.reset_column_information assert_equal "decimal(10,2)", Payment.columns.last.sql_type end def test_migration_down - migrator = ActiveRecord::Migrator.new(:down, [ChangeAmountToAddScale]) - migrator.run + ChangeAmountToAddScale.migrate(:down) Payment.reset_column_information assert_equal "decimal(10,0)", Payment.columns.last.sql_type diff --git a/guides/bug_report_templates/active_record_migrations_master.rb b/guides/bug_report_templates/active_record_migrations_master.rb index 84a4b71909..52c9028b0f 100644 --- a/guides/bug_report_templates/active_record_migrations_master.rb +++ b/guides/bug_report_templates/active_record_migrations_master.rb @@ -48,16 +48,14 @@ end class BugTest < Minitest::Test def test_migration_up - migrator = ActiveRecord::Migrator.new(:up, [ChangeAmountToAddScale]) - migrator.run + ChangeAmountToAddScale.migrate(:up) Payment.reset_column_information assert_equal "decimal(10,2)", Payment.columns.last.sql_type end def test_migration_down - migrator = ActiveRecord::Migrator.new(:down, [ChangeAmountToAddScale]) - migrator.run + ChangeAmountToAddScale.migrate(:down) Payment.reset_column_information assert_equal "decimal(10,0)", Payment.columns.last.sql_type diff --git a/guides/source/active_record_postgresql.md b/guides/source/active_record_postgresql.md index 041fdacbab..8543fcd20f 100644 --- a/guides/source/active_record_postgresql.md +++ b/guides/source/active_record_postgresql.md @@ -114,16 +114,21 @@ Profile.where("settings->'color' = ?", "yellow") # => #<ActiveRecord::Relation [#<Profile id: 1, settings: {"color"=>"yellow", "resolution"=>"1280x1024"}>]> ``` -### JSON +### JSON and JSONB * [type definition](http://www.postgresql.org/docs/current/static/datatype-json.html) * [functions and operators](http://www.postgresql.org/docs/current/static/functions-json.html) ```ruby # db/migrate/20131220144913_create_events.rb +# ... for json datatype: create_table :events do |t| t.json 'payload' end +# ... or for jsonb datatype: +create_table :events do |t| + t.jsonb 'payload' +end # app/models/event.rb class Event < ApplicationRecord diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 3676462788..215142223d 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -513,8 +513,6 @@ Article.where(author: author) Author.joins(:articles).where(articles: { author: author }) ``` -NOTE: The values cannot be symbols. For example, you cannot do `Client.where(status: :active)`. - #### Range Conditions ```ruby diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index caa3d21d23..c96cf61761 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -1171,7 +1171,7 @@ To pass a local variable to a partial in only specific cases use the `local_assi This way it is possible to use the partial without the need to declare all local variables. -Every partial also has a local variable with the same name as the partial (minus the underscore). You can pass an object in to this local variable via the `:object` option: +Every partial also has a local variable with the same name as the partial (minus the leading underscore). You can pass an object in to this local variable via the `:object` option: ```erb <%= render partial: "customer", object: @new_customer %> diff --git a/guides/source/security.md b/guides/source/security.md index 9b1f28a283..297680b176 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -1061,6 +1061,6 @@ Additional Resources The security landscape shifts and it is important to keep up to date, because missing a new vulnerability can be catastrophic. You can find additional resources about (Rails) security here: * Subscribe to the Rails security [mailing list.](http://groups.google.com/group/rubyonrails-security) -* [Brakeman - Rails Security Scanner](http://brakemanscanner.org/)- To perform static security analysis for Rails applications. +* [Brakeman - Rails Security Scanner](http://brakemanscanner.org/) - To perform static security analysis for Rails applications. * [Keep up to date on the other application layers.](http://secunia.com/) (they have a weekly newsletter, too) * A [good security blog](https://www.owasp.org) including the [Cross-Site scripting Cheat Sheet.](https://www.owasp.org/index.php/DOM_based_XSS_Prevention_Cheat_Sheet) diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 6cb8cbcd73..b9a530258d 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add `railtie.rb` to the plugin generator + + *Tsukuru Tanimichi* + * Deprecate `capify!` method in generators and templates. *Yuji Yaginuma* diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 8fe48feefb..63300ffef3 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -10,7 +10,7 @@ module Rails end def build_stack - ActionDispatch::MiddlewareStack.new.tap do |middleware| + ActionDispatch::MiddlewareStack.new do |middleware| if config.force_ssl middleware.use ::ActionDispatch::SSL, config.ssl_options end diff --git a/railties/lib/rails/generators/rails/app/templates/gitignore b/railties/lib/rails/generators/rails/app/templates/gitignore index 7221c26729..1e6b9afcd2 100644 --- a/railties/lib/rails/generators/rails/app/templates/gitignore +++ b/railties/lib/rails/generators/rails/app/templates/gitignore @@ -26,4 +26,8 @@ /yarn-error.log <% end -%> + +<% unless options[:api] -%> +/public/assets +<% end -%> .byebug_history diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index 118e44d9d0..445235852d 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -60,7 +60,12 @@ module Rails template "lib/%namespaced_name%.rb" template "lib/tasks/%namespaced_name%_tasks.rake" template "lib/%namespaced_name%/version.rb" - template "lib/%namespaced_name%/engine.rb" if engine? + + if engine? + template "lib/%namespaced_name%/engine.rb" + else + template "lib/%namespaced_name%/railtie.rb" + end end def config diff --git a/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%.rb b/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%.rb index 40b1c4cee7..3285055eb7 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%.rb +++ b/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%.rb @@ -1,5 +1,7 @@ <% if engine? -%> require "<%= namespaced_name %>/engine" - +<% else -%> +require "<%= namespaced_name %>/railtie" <% end -%> + <%= wrap_in_modules "# Your code goes here..." %> diff --git a/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%/railtie.rb b/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%/railtie.rb new file mode 100644 index 0000000000..7bdf4ee5fb --- /dev/null +++ b/railties/lib/rails/generators/rails/plugin/templates/lib/%namespaced_name%/railtie.rb @@ -0,0 +1,5 @@ +<%= wrap_in_modules <<-rb.strip_heredoc + class Railtie < ::Rails::Railtie + end +rb +%> diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 63aa079a12..f8512f9157 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -68,6 +68,8 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_match(/Minitest\.backtrace_filter = Minitest::BacktraceFilter\.new/, content) assert_match(/Rails::TestUnitReporter\.executable = 'bin\/test'/, content) end + assert_file "lib/bukkits/railtie.rb", /module Bukkits\n class Railtie < ::Rails::Railtie\n end\nend/ + assert_file "lib/bukkits.rb", /require "bukkits\/railtie"/ assert_file "test/bukkits_test.rb", /assert_kind_of Module, Bukkits/ assert_file "bin/test" assert_no_file "bin/rails" |