diff options
26 files changed, 128 insertions, 61 deletions
diff --git a/activemodel/lib/active_model/validations/with.rb b/activemodel/lib/active_model/validations/with.rb index 7022f9bee5..ff41572105 100644 --- a/activemodel/lib/active_model/validations/with.rb +++ b/activemodel/lib/active_model/validations/with.rb @@ -53,7 +53,7 @@ module ActiveModel # # Configuration options: # * <tt>:on</tt> - Specifies when this validation is active - # (<tt>:create</tt> or <tt>:update</tt>. + # (<tt>:create</tt> or <tt>:update</tt>). # * <tt>:if</tt> - Specifies a method, proc or string to call to determine # if the validation should occur (e.g. <tt>if: :allow_validation</tt>, # or <tt>if: Proc.new { |user| user.signup_step > 2 }</tt>). diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index dce2692b0d..16f9db95fc 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -4,6 +4,21 @@ *Vladimir Sazhin*, *Toms Mikoss*, *Yves Senn* +* Make possible to have an association called `records`. + + Fixes #11645. + + *prathamesh-sonpatki* + +* `to_sql` on an association now matches the query that is actually executed, where it + could previously have incorrectly accrued additional conditions (e.g. as a result of + a previous query). CollectionProxy now always defers to the association scope's + `arel` method so the (incorrect) inherited one should be entirely concealed. + + Fixes #14003. + + *Jefferson Lai* + * Block a few default Class methods as scope name. For instance, this will raise: diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 80ae38b3fb..dee7e972c1 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -134,11 +134,11 @@ module ActiveRecord end def create(attributes = {}, &block) - create_record(attributes, &block) + _create_record(attributes, &block) end def create!(attributes = {}, &block) - create_record(attributes, true, &block) + _create_record(attributes, true, &block) end # Add +records+ to this association. Returns +self+ so method calls may @@ -449,13 +449,13 @@ module ActiveRecord persisted + memory end - def create_record(attributes, raise = false, &block) + def _create_record(attributes, raise = false, &block) unless owner.persisted? raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" end if attributes.is_a?(Array) - attributes.collect { |attr| create_record(attr, raise, &block) } + attributes.collect { |attr| _create_record(attr, raise, &block) } else transaction do add_to_target(build_record(attributes)) do |record| diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index eba688866c..5b71ed163e 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -860,6 +860,10 @@ module ActiveRecord !!@association.include?(record) end + def arel + scope.arel + end + def proxy_association @association end diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index 399aff378a..747bb5f1d6 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -18,11 +18,11 @@ module ActiveRecord end def create(attributes = {}, &block) - create_record(attributes, &block) + _create_record(attributes, &block) end def create!(attributes = {}, &block) - create_record(attributes, true, &block) + _create_record(attributes, true, &block) end def build(attributes = {}) @@ -52,7 +52,7 @@ module ActiveRecord replace(record) end - def create_record(attributes, raise_error = false) + def _create_record(attributes, raise_error = false) record = build_record(attributes) yield(record) if block_given? saved = record.save diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 8a1b199997..99070f127b 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -79,11 +79,11 @@ module ActiveRecord end end - def update_record(*) + def _update_record(*) partial_writes? ? super(keys_for_partial_write) : super end - def create_record(*) + def _create_record(*) partial_writes? ? super(keys_for_partial_write) : super end diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index 35f19f0bc0..5955673b42 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -302,11 +302,11 @@ module ActiveRecord run_callbacks(:save) { super } end - def create_record #:nodoc: + def _create_record #:nodoc: run_callbacks(:create) { super } end - def update_record(*) #:nodoc: + def _update_record(*) #:nodoc: run_callbacks(:update) { super } end end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 6f54729b3c..4d63b04d9f 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -66,7 +66,7 @@ module ActiveRecord send(lock_col + '=', previous_lock_value + 1) end - def update_record(attribute_names = @attributes.keys) #:nodoc: + def _update_record(attribute_names = @attributes.keys) #:nodoc: return super unless locking_enabled? return 0 if attribute_names.empty? diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 1a2581f579..13d7432773 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -484,24 +484,24 @@ module ActiveRecord def create_or_update raise ReadOnlyRecord if readonly? - result = new_record? ? create_record : update_record + result = new_record? ? _create_record : _update_record result != false end # Updates the associated record with values matching those of the instance attributes. # Returns the number of affected rows. - def update_record(attribute_names = @attributes.keys) + def _update_record(attribute_names = @attributes.keys) attributes_values = arel_attributes_with_values_for_update(attribute_names) if attributes_values.empty? 0 else - self.class.unscoped.update_record attributes_values, id, id_was + self.class.unscoped._update_record attributes_values, id, id_was end end # Creates a record with values matching those of the instance attributes # and returns its id. - def create_record(attribute_names = @attributes.keys) + def _create_record(attribute_names = @attributes.keys) attributes_values = arel_attributes_with_values_for_create(attribute_names) new_id = self.class.unscoped.insert attributes_values diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 9eaba4a655..4d37ac6e2b 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -70,7 +70,7 @@ module ActiveRecord binds) end - def update_record(values, id, id_was) # :nodoc: + def _update_record(values, id, id_was) # :nodoc: substitutes, binds = substitute_values values um = @klass.unscoped.where(@klass.arel_table[@klass.primary_key].eq(id_was || id)).arel.compile_update(substitutes, @klass.primary_key) diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index 7178bed560..6c30ccab72 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -43,7 +43,7 @@ module ActiveRecord private - def create_record + def _create_record if self.record_timestamps current_time = current_time_from_proper_timezone @@ -57,7 +57,7 @@ module ActiveRecord super end - def update_record(*args) + def _update_record(*args) if should_record_timestamps? current_time = current_time_from_proper_timezone diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index 5e5f653d17..11ec7599a3 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -271,13 +271,13 @@ class SchemaTest < ActiveRecord::TestCase end def test_with_uppercase_index_name - ActiveRecord::Base.connection.execute "CREATE INDEX \"things_Index\" ON #{SCHEMA_NAME}.things (name)" - assert_nothing_raised { ActiveRecord::Base.connection.remove_index! "things", "#{SCHEMA_NAME}.things_Index"} + @connection.execute "CREATE INDEX \"things_Index\" ON #{SCHEMA_NAME}.things (name)" + assert_nothing_raised { @connection.remove_index! "things", "#{SCHEMA_NAME}.things_Index"} + @connection.execute "CREATE INDEX \"things_Index\" ON #{SCHEMA_NAME}.things (name)" - ActiveRecord::Base.connection.execute "CREATE INDEX \"things_Index\" ON #{SCHEMA_NAME}.things (name)" - ActiveRecord::Base.connection.schema_search_path = SCHEMA_NAME - assert_nothing_raised { ActiveRecord::Base.connection.remove_index! "things", "things_Index"} - ActiveRecord::Base.connection.schema_search_path = "public" + with_schema_search_path SCHEMA_NAME do + assert_nothing_raised { @connection.remove_index! "things", "things_Index"} + end end def test_primary_key_with_schema_specified @@ -328,18 +328,17 @@ class SchemaTest < ActiveRecord::TestCase end def test_prepared_statements_with_multiple_schemas + [SCHEMA_NAME, SCHEMA2_NAME].each do |schema_name| + with_schema_search_path schema_name do + Thing5.create(:id => 1, :name => "thing inside #{SCHEMA_NAME}", :email => "thing1@localhost", :moment => Time.now) + end + end - @connection.schema_search_path = SCHEMA_NAME - Thing5.create(:id => 1, :name => "thing inside #{SCHEMA_NAME}", :email => "thing1@localhost", :moment => Time.now) - - @connection.schema_search_path = SCHEMA2_NAME - Thing5.create(:id => 1, :name => "thing inside #{SCHEMA2_NAME}", :email => "thing1@localhost", :moment => Time.now) - - @connection.schema_search_path = SCHEMA_NAME - assert_equal 1, Thing5.count - - @connection.schema_search_path = SCHEMA2_NAME - assert_equal 1, Thing5.count + [SCHEMA_NAME, SCHEMA2_NAME].each do |schema_name| + with_schema_search_path schema_name do + assert_equal 1, Thing5.count + end + end end def test_schema_exists? diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index a65f2da7a0..27f6fa575d 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -16,6 +16,8 @@ require 'models/essay' require 'models/toy' require 'models/invoice' require 'models/line_item' +require 'models/column' +require 'models/record' class BelongsToAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, :topics, @@ -885,4 +887,10 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end end end + + test 'belongs_to works with model called Record' do + record = Record.create! + Column.create! record: record + assert_equal 1, Column.count + end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index eb0ebc75ad..c79c0c87c5 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -69,7 +69,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_has_many_build_with_options college = College.create(name: 'UFMT') - student = Student.create(active: true, college_id: college.id, name: 'Sarah') + Student.create(active: true, college_id: college.id, name: 'Sarah') assert_equal college.students, Student.where(active: true, college_id: college.id) end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index fddb7c204a..049c5a0606 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -573,6 +573,12 @@ class RelationTest < ActiveRecord::TestCase assert_equal expected, actual end + def test_to_sql_on_scoped_proxy + auth = Author.first + Post.where("1=1").written_by(auth) + assert_not auth.posts.to_sql.include?("1=1") + end + def test_loading_with_one_association_with_non_preload posts = Post.eager_load(:last_comment).order('comments.id DESC') post = posts.find { |p| p.id == 1 } diff --git a/activerecord/test/models/column.rb b/activerecord/test/models/column.rb new file mode 100644 index 0000000000..499358b4cf --- /dev/null +++ b/activerecord/test/models/column.rb @@ -0,0 +1,3 @@ +class Column < ActiveRecord::Base + belongs_to :record +end diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index faf539a562..099e039255 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -149,6 +149,10 @@ class Post < ActiveRecord::Base ranked_by_comments.limit_by(limit) end + def self.written_by(author) + where(id: author.posts.pluck(:id)) + end + def self.reset_log @log = [] end diff --git a/activerecord/test/models/record.rb b/activerecord/test/models/record.rb new file mode 100644 index 0000000000..f77ac9fc03 --- /dev/null +++ b/activerecord/test/models/record.rb @@ -0,0 +1,2 @@ +class Record < ActiveRecord::Base +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index d79cce8cca..d9e1745029 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -161,6 +161,10 @@ ActiveRecord::Schema.define do t.integer :references, null: false end + create_table :columns, force: true do |t| + t.references :record + end + create_table :comments, force: true do |t| t.integer :post_id, null: false # use VARCHAR2(4000) instead of CLOB datatype as CLOB data type has many limitations in @@ -819,6 +823,8 @@ ActiveRecord::Schema.define do t.integer :department_id end + create_table :records, force: true do |t| + end except 'SQLite' do # fk_test_has_fk should be before fk_test_has_pk diff --git a/activesupport/test/core_ext/class/delegating_attributes_test.rb b/activesupport/test/core_ext/class/delegating_attributes_test.rb index 86193c2b07..447b1d10ad 100644 --- a/activesupport/test/core_ext/class/delegating_attributes_test.rb +++ b/activesupport/test/core_ext/class/delegating_attributes_test.rb @@ -33,7 +33,7 @@ class DelegatingAttributesTest < ActiveSupport::TestCase end def test_simple_accessor_declaration - ActiveSupport::Deprecation.silence do + assert_deprecated do single_class.superclass_delegating_accessor :both end @@ -48,7 +48,7 @@ class DelegatingAttributesTest < ActiveSupport::TestCase def test_simple_accessor_declaration_with_instance_reader_false _instance_methods = single_class.public_instance_methods - ActiveSupport::Deprecation.silence do + assert_deprecated do single_class.superclass_delegating_accessor :no_instance_reader, :instance_reader => false end @@ -60,7 +60,7 @@ class DelegatingAttributesTest < ActiveSupport::TestCase end def test_working_with_simple_attributes - ActiveSupport::Deprecation.silence do + assert_deprecated do single_class.superclass_delegating_accessor :both end @@ -79,7 +79,7 @@ class DelegatingAttributesTest < ActiveSupport::TestCase def test_child_class_delegates_to_parent_but_can_be_overridden parent = Class.new - ActiveSupport::Deprecation.silence do + assert_deprecated do parent.superclass_delegating_accessor :both end diff --git a/rails.gemspec b/rails.gemspec index d1c199a97a..4800df0df4 100644 --- a/rails.gemspec +++ b/rails.gemspec @@ -27,5 +27,5 @@ Gem::Specification.new do |s| s.add_dependency 'railties', version s.add_dependency 'bundler', '>= 1.3.0', '< 2.0' - s.add_dependency 'sprockets-rails', '~> 2.0.0' + s.add_dependency 'sprockets-rails', '~> 2.1' end diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index e37347b576..dd650e9631 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -332,6 +332,25 @@ module Rails config.helpers_paths end + console do + require "pp" + end + + console do + unless ::Kernel.private_method_defined?(:y) + if RUBY_VERSION >= '2.0' + require "psych/y" + else + module ::Kernel + def y(*objects) + puts ::Psych.dump_stream(*objects) + end + private :y + end + end + end + end + protected alias :build_middleware_stack :app diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index c38cc3b5e5..b36ab3d0d5 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -429,8 +429,6 @@ module Rails # Load console and invoke the registered hooks. # Check <tt>Rails::Railtie.console</tt> for more info. def load_console(app=self) - require "pp" - require "psych/y" require "rails/console/app" require "rails/console/helpers" run_console_blocks(app) diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 2fb49479fe..c066f748ee 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -249,7 +249,7 @@ module Rails 'Use SCSS for stylesheets') else gems << GemfileEntry.version('sass-rails', - '~> 4.0.2', + '~> 4.0.3', 'Use SCSS for stylesheets') end diff --git a/railties/lib/rails/railtie.rb b/railties/lib/rails/railtie.rb index 8d7e804bdc..2b33beaa2b 100644 --- a/railties/lib/rails/railtie.rb +++ b/railties/lib/rails/railtie.rb @@ -221,26 +221,28 @@ module Rails protected def run_console_blocks(app) #:nodoc: - self.class.console.each { |block| block.call(app) } + each_registered_block(:console) { |block| block.call(app) } end def run_generators_blocks(app) #:nodoc: - self.class.generators.each { |block| block.call(app) } + each_registered_block(:generators) { |block| block.call(app) } end def run_runner_blocks(app) #:nodoc: - self.class.runner.each { |block| block.call(app) } + each_registered_block(:runner) { |block| block.call(app) } end def run_tasks_blocks(app) #:nodoc: extend Rake::DSL - self.class.rake_tasks.each { |block| instance_exec(app, &block) } + each_registered_block(:rake_tasks) { |block| instance_exec(app, &block) } + end - # Load also tasks from all superclasses - klass = self.class.superclass + private - while klass.respond_to?(:rake_tasks) - klass.rake_tasks.each { |t| instance_exec(app, &t) } + def each_registered_block(type, &block) + klass = self.class + while klass.respond_to?(type) + klass.public_send(type).each(&block) klass = klass.superclass end end diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb index b235b51d90..6cc17ad176 100644 --- a/railties/test/application/assets_test.rb +++ b/railties/test/application/assets_test.rb @@ -42,7 +42,7 @@ module ApplicationTests test "assets routes have higher priority" do app_file "app/assets/images/rails.png", "notactuallyapng" - app_file "app/assets/javascripts/demo.js.erb", "a = <%= image_path('rails.png').inspect %>;" + app_file "app/assets/javascripts/demo.js.erb", "//= depend_on_asset 'rails.png'\na = <%= image_path('rails.png').inspect %>;" app_file 'config/routes.rb', <<-RUBY Rails.application.routes.draw do @@ -199,7 +199,8 @@ module ApplicationTests end test "precompile creates a manifest file with all the assets listed" do - app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>" + app_file "app/assets/images/rails.png", "notactuallyapng" + app_file "app/assets/stylesheets/application.css.erb", "//= depend_on_asset 'rails.png'\n <%= asset_path('rails.png') %>" app_file "app/assets/javascripts/application.js", "alert();" # digest is default in false, we must enable it for test environment add_to_config "config.assets.digest = true" @@ -279,7 +280,7 @@ module ApplicationTests test "precompile appends the md5 hash to files referenced with asset_path and run in production with digest true" do app_file "app/assets/images/rails.png", "notactuallyapng" - app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>" + app_file "app/assets/stylesheets/application.css.erb", "//= depend_on_asset 'rails.png'\n<%= asset_path('rails.png') %>" add_to_config "config.assets.compile = true" add_to_config "config.assets.digest = true" @@ -448,23 +449,23 @@ module ApplicationTests test "asset urls should be protocol-relative if no request is in scope" do app_file "app/assets/images/rails.png", "notreallyapng" - app_file "app/assets/javascripts/image_loader.js.erb", 'var src="<%= image_path("rails.png") %>";' + app_file "app/assets/javascripts/image_loader.js.erb", "//= depend_on_asset 'rails.png'\n\nvar src='<%= image_path('rails.png') %>';" add_to_config "config.assets.precompile = %w{image_loader.js}" add_to_config "config.asset_host = 'example.com'" precompile! - assert_match 'src="//example.com/assets/rails.png"', File.read(Dir["#{app_path}/public/assets/image_loader-*.js"].first) + assert_match "src='//example.com/assets/rails.png'", File.read(Dir["#{app_path}/public/assets/image_loader-*.js"].first) end test "asset paths should use RAILS_RELATIVE_URL_ROOT by default" do ENV["RAILS_RELATIVE_URL_ROOT"] = "/sub/uri" app_file "app/assets/images/rails.png", "notreallyapng" - app_file "app/assets/javascripts/app.js.erb", 'var src="<%= image_path("rails.png") %>";' + app_file "app/assets/javascripts/app.js.erb", "//= depend_on_asset 'rails.png'\n\nvar src='<%= image_path('rails.png') %>';" add_to_config "config.assets.precompile = %w{app.js}" precompile! - assert_match 'src="/sub/uri/assets/rails.png"', File.read(Dir["#{app_path}/public/assets/app-*.js"].first) + assert_match "src='/sub/uri/assets/rails.png'", File.read(Dir["#{app_path}/public/assets/app-*.js"].first) end test "assets:cache:clean should clean cache" do |