From 8f650d6db83e95a385aeb18571cab34328089cf6 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 15 Jul 2017 11:59:50 +0900 Subject: Remove outdated `test_scoped_responds_to_delegated_methods` This test was added at 74ed123 to ensure `respond_to?` delegate methods to `Array` and `arel_table`. But array method delegation was removed at 9d79334 in favor of including `Enumerable`. And now `Relation` has `insert`, `update`, and `delete` methods (63480d2, 8d31c9f, d5f9173). So this delegation test is already outdated. --- activerecord/test/cases/relations_test.rb | 8 -------- 1 file changed, 8 deletions(-) (limited to 'activerecord/test/cases/relations_test.rb') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index eb3449b331..c77048a756 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -586,14 +586,6 @@ class RelationTest < ActiveRecord::TestCase assert_nothing_raised { Topic.reorder([]) } end - def test_scoped_responds_to_delegated_methods - relation = Topic.all - - ["map", "uniq", "sort", "insert", "delete", "update"].each do |method| - assert_respond_to relation, method, "Topic.all should respond to #{method.inspect}" - end - end - def test_respond_to_delegates_to_arel relation = Topic.all fake_arel = Struct.new(:responds) { -- cgit v1.2.3 From f73742455ffd3db0462b18d3d686dbe13a07ee56 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 15 Jul 2017 22:39:17 +0900 Subject: Extract `NullRelationTest` from `RelationTest` `test/cases/relations_test.rb` file has too much lines (2000 over). So I extracted `NullRelationTest` to the dedicated file. --- activerecord/test/cases/relations_test.rb | 118 ------------------------------ 1 file changed, 118 deletions(-) (limited to 'activerecord/test/cases/relations_test.rb') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index eb3449b331..b91ba8341c 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -15,7 +15,6 @@ require "models/car" require "models/engine" require "models/tyre" require "models/minivan" -require "models/aircraft" require "models/possession" require "models/reader" require "models/categorization" @@ -420,123 +419,6 @@ class RelationTest < ActiveRecord::TestCase assert_equal [2, 4, 6, 8, 10], even_ids.sort end - def test_none - assert_no_queries(ignore_none: false) do - assert_equal [], Developer.none - assert_equal [], Developer.all.none - end - end - - def test_none_chainable - assert_no_queries(ignore_none: false) do - assert_equal [], Developer.none.where(name: "David") - end - end - - def test_none_chainable_to_existing_scope_extension_method - assert_no_queries(ignore_none: false) do - assert_equal 1, Topic.anonymous_extension.none.one - end - end - - def test_none_chained_to_methods_firing_queries_straight_to_db - assert_no_queries(ignore_none: false) do - assert_equal [], Developer.none.pluck(:id, :name) - assert_equal 0, Developer.none.delete_all - assert_equal 0, Developer.none.update_all(name: "David") - assert_equal 0, Developer.none.delete(1) - assert_equal false, Developer.none.exists?(1) - end - end - - def test_null_relation_content_size_methods - assert_no_queries(ignore_none: false) do - assert_equal 0, Developer.none.size - assert_equal 0, Developer.none.count - assert_equal true, Developer.none.empty? - assert_equal true, Developer.none.none? - assert_equal false, Developer.none.any? - assert_equal false, Developer.none.one? - assert_equal false, Developer.none.many? - end - end - - def test_null_relation_calculations_methods - assert_no_queries(ignore_none: false) do - assert_equal 0, Developer.none.count - assert_equal 0, Developer.none.calculate(:count, nil) - assert_nil Developer.none.calculate(:average, "salary") - end - end - - def test_null_relation_metadata_methods - assert_equal "", Developer.none.to_sql - assert_equal({}, Developer.none.where_values_hash) - end - - def test_null_relation_where_values_hash - assert_equal({ "salary" => 100_000 }, Developer.none.where(salary: 100_000).where_values_hash) - end - - def test_null_relation_sum - ac = Aircraft.new - assert_equal Hash.new, ac.engines.group(:id).sum(:id) - assert_equal 0, ac.engines.count - ac.save - assert_equal Hash.new, ac.engines.group(:id).sum(:id) - assert_equal 0, ac.engines.count - end - - def test_null_relation_count - ac = Aircraft.new - assert_equal Hash.new, ac.engines.group(:id).count - assert_equal 0, ac.engines.count - ac.save - assert_equal Hash.new, ac.engines.group(:id).count - assert_equal 0, ac.engines.count - end - - def test_null_relation_size - ac = Aircraft.new - assert_equal Hash.new, ac.engines.group(:id).size - assert_equal 0, ac.engines.size - ac.save - assert_equal Hash.new, ac.engines.group(:id).size - assert_equal 0, ac.engines.size - end - - def test_null_relation_average - ac = Aircraft.new - assert_equal Hash.new, ac.engines.group(:car_id).average(:id) - assert_nil ac.engines.average(:id) - ac.save - assert_equal Hash.new, ac.engines.group(:car_id).average(:id) - assert_nil ac.engines.average(:id) - end - - def test_null_relation_minimum - ac = Aircraft.new - assert_equal Hash.new, ac.engines.group(:car_id).minimum(:id) - assert_nil ac.engines.minimum(:id) - ac.save - assert_equal Hash.new, ac.engines.group(:car_id).minimum(:id) - assert_nil ac.engines.minimum(:id) - end - - def test_null_relation_maximum - ac = Aircraft.new - assert_equal Hash.new, ac.engines.group(:car_id).maximum(:id) - assert_nil ac.engines.maximum(:id) - ac.save - assert_equal Hash.new, ac.engines.group(:car_id).maximum(:id) - assert_nil ac.engines.maximum(:id) - end - - def test_null_relation_in_where_condition - assert_operator Comment.count, :>, 0 # precondition, make sure there are comments. - assert_equal 0, Comment.where(post_id: Post.none).to_a.size - end - def test_joins_with_nil_argument assert_nothing_raised { DependentFirm.joins(nil).first } end -- cgit v1.2.3 From 01c85097d4977b0c141b7c89df15c0750f37c62d Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 13 Jul 2017 07:14:59 +0900 Subject: Fix `create_with` using both string and symbol This is related with #27680. Since `where_values_hash` keys constructed by `where` are string, so we need `stringify_keys` to `create_with_value` before merging it. --- activerecord/test/cases/relations_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/test/cases/relations_test.rb') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index eb3449b331..6c35940c2e 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1482,7 +1482,7 @@ class RelationTest < ActiveRecord::TestCase assert_equal bird, Bird.find_or_initialize_by(name: "bob") end - def test_explicit_create_scope + def test_explicit_create_with hens = Bird.where(name: "hen") assert_equal "hen", hens.new.name -- cgit v1.2.3 From 26ba655f36c20315390497a0963c23014cadeb91 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 15 Jul 2017 08:38:29 +0900 Subject: Fix `where` with a custom table Without this fix, SELECT clause doesn't use a custom table alias name: ``` % ARCONN=sqlite3 be ruby -w -Itest test/cases/relations_test.rb -n test_using_a_custom_table_affects_the_wheres Using sqlite3 Run options: -n test_using_a_custom_table_affects_the_wheres --seed 31818 E Error: RelationTest#test_using_a_custom_table_affects_the_wheres: ActiveRecord::StatementInvalid: SQLite3::SQLException: no such table: posts: SELECT "posts".* FROM "posts" "omg_posts" WHERE "omg_posts"."title" = ? LIMIT ? ``` --- activerecord/test/cases/relations_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'activerecord/test/cases/relations_test.rb') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 6c35940c2e..ca15c95432 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1915,11 +1915,11 @@ class RelationTest < ActiveRecord::TestCase table_metadata = ActiveRecord::TableMetadata.new(Post, table_alias) predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata) - relation = ActiveRecord::Relation.new(Post, table_alias, predicate_builder) - relation.where!(foo: "bar") + relation = ActiveRecord::Relation.create(Post, table_alias, predicate_builder) - node = relation.arel.constraints.first.grep(Arel::Attributes::Attribute).first - assert_equal table_alias, node.relation + post = posts(:welcome) + + assert_equal post, relation.where!(title: post.title).take end test "#load" do -- cgit v1.2.3 From ea09bf5419ec752dd020d26b03533afa457d09d6 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 18 Jul 2017 04:15:45 +0900 Subject: Fix `JoinDependency` with using a custom table Without this fix, `JoinDependency` doesn't use a custom table alias: ``` % ARCONN=sqlite3 be ruby -w -Itest test/cases/relations_test.rb -n test_using_a_custom_table_with_joins_affects_the_wheres Using sqlite3 Run options: -n test_using_a_custom_table_with_joins_affects_the_wheres --seed 14531 E Error:RelationTest#test_using_a_custom_table_with_joins_affects_the_wheres: ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: posts.author_id: SELECT "omg_posts".* FROM "posts" "omg_posts" INNER JOIN "authors" ON "authors"."id" = "posts"."author_id" WHERE "omg_posts"."title" = ? LIMIT ? ``` --- activerecord/test/cases/relations_test.rb | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) (limited to 'activerecord/test/cases/relations_test.rb') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 82e1077e87..316ea75e36 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1785,15 +1785,15 @@ class RelationTest < ActiveRecord::TestCase end test "using a custom table affects the wheres" do - table_alias = Post.arel_table.alias("omg_posts") + post = posts(:welcome) - table_metadata = ActiveRecord::TableMetadata.new(Post, table_alias) - predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata) - relation = ActiveRecord::Relation.create(Post, table_alias, predicate_builder) + assert_equal post, custom_post_relation.where!(title: post.title).take + end + test "using a custom table with joins affects the joins" do post = posts(:welcome) - assert_equal post, relation.where!(title: post.title).take + assert_equal post, custom_post_relation.joins(:author).where!(title: post.title).take end test "#load" do @@ -1950,4 +1950,13 @@ class RelationTest < ActiveRecord::TestCase end end end + + private + def custom_post_relation + table_alias = Post.arel_table.alias("omg_posts") + table_metadata = ActiveRecord::TableMetadata.new(Post, table_alias) + predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata) + + ActiveRecord::Relation.create(Post, table_alias, predicate_builder) + end end -- cgit v1.2.3 From 831be98f9a6685da3410b0c9bf4143bd8dd647af Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Sun, 9 Jul 2017 20:41:28 +0300 Subject: Use frozen-string-literal in ActiveRecord --- activerecord/test/cases/relations_test.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activerecord/test/cases/relations_test.rb') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 316ea75e36..e1b2f794ac 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "cases/helper" require "models/tag" require "models/tagging" -- cgit v1.2.3 From d0c4e9496dd7527eb4954d1362b410129ae740ec Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 20 Jul 2017 18:35:07 +0900 Subject: Fix a failued AR test when using OracleAdapter --- activerecord/test/cases/relations_test.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'activerecord/test/cases/relations_test.rb') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 316ea75e36..d57bd4cc62 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1592,7 +1592,11 @@ class RelationTest < ActiveRecord::TestCase assert_equal ["comments"], scope.references_values scope = Post.order("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}") - assert_equal ["comments"], scope.references_values + if current_adapter?(:OracleAdapter) + assert_equal ["COMMENTS"], scope.references_values + else + assert_equal ["comments"], scope.references_values + end scope = Post.order("comments.body", "yaks.body") assert_equal ["comments", "yaks"], scope.references_values @@ -1613,7 +1617,11 @@ class RelationTest < ActiveRecord::TestCase assert_equal %w(comments), scope.references_values scope = Post.reorder("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}") - assert_equal ["comments"], scope.references_values + if current_adapter?(:OracleAdapter) + assert_equal ["COMMENTS"], scope.references_values + else + assert_equal ["comments"], scope.references_values + end scope = Post.reorder("comments.body", "yaks.body") assert_equal %w(comments yaks), scope.references_values -- cgit v1.2.3 From 93c9a95e013ba7c77cf381bab9ead4a0de37e128 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 21 Jul 2017 13:55:24 -0400 Subject: Revert "Extract `bind_param` and `bind_attribute` into `ActiveRecord::TestCase`" This reverts commit b6ad4052d18e4b29b8a092526c2beef013e2bf4f. This is not something that the majority of Active Record should be testing or care about. We should look at having fewer places rely on these details, not make it easier to rely on them. --- activerecord/test/cases/relations_test.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'activerecord/test/cases/relations_test.rb') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 316ea75e36..d294f8f628 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1853,8 +1853,7 @@ class RelationTest < ActiveRecord::TestCase def test_unscope_removes_binds left = Post.where(id: 20) - binds = [bind_attribute("id", 20, Post.type_for_attribute("id"))] - assert_equal binds, left.bound_attributes + assert_equal 1, left.bound_attributes.length relation = left.unscope(where: :id) assert_equal [], relation.bound_attributes @@ -1864,21 +1863,18 @@ class RelationTest < ActiveRecord::TestCase left = Post.where(id: 20) right = Post.where(id: [1, 2, 3, 4]) - binds = [bind_attribute("id", 20, Post.type_for_attribute("id"))] - assert_equal binds, left.bound_attributes + assert_equal 1, left.bound_attributes.length merged = left.merge(right) assert_equal [], merged.bound_attributes end def test_merging_keeps_lhs_binds - binds = [bind_attribute("id", 20, Post.type_for_attribute("id"))] - right = Post.where(id: 20) left = Post.where(id: 10) merged = left.merge(right) - assert_equal binds, merged.bound_attributes + assert_equal [20], merged.bound_attributes.map(&:value) end def test_locked_should_not_build_arel -- cgit v1.2.3 From 213796fb4936dce1da2f0c097a054e1af5c25c2c Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 24 Jul 2017 08:19:35 -0400 Subject: Refactor Active Record to let Arel manage bind params A common source of bugs and code bloat within Active Record has been the need for us to maintain the list of bind values separately from the AST they're associated with. This makes any sort of AST manipulation incredibly difficult, as any time we want to potentially insert or remove an AST node, we need to traverse the entire tree to find where the associated bind parameters are. With this change, the bind parameters now live on the AST directly. Active Record does not need to know or care about them until the final AST traversal for SQL construction. Rather than returning just the SQL, the Arel collector will now return both the SQL and the bind parameters. At this point the connection adapter will have all the values that it had before. A bit of this code is janky and something I'd like to refactor later. In particular, I don't like how we're handling associations in the predicate builder, the special casing of `StatementCache::Substitute` in `QueryAttribute`, or generally how we're handling bind value replacement in the statement cache when prepared statements are disabled. This also mostly reverts #26378, as it moved all the code into a location that I wanted to delete. /cc @metaskills @yahonda, this change will affect the adapters Fixes #29766. Fixes #29804. Fixes #26541. Close #28539. Close #24769. Close #26468. Close #26202. There are probably other issues/PRs that can be closed because of this commit, but that's all I could find on the first few pages. --- activerecord/test/cases/relations_test.rb | 47 +------------------------------ 1 file changed, 1 insertion(+), 46 deletions(-) (limited to 'activerecord/test/cases/relations_test.rb') diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index a9154294ad..ae1dc35bff 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1761,7 +1761,7 @@ class RelationTest < ActiveRecord::TestCase test "relations with cached arel can't be mutated [internal API]" do relation = Post.all - relation.count + relation.arel assert_raises(ActiveRecord::ImmutableRelation) { relation.limit!(5) } assert_raises(ActiveRecord::ImmutableRelation) { relation.where!("1 = 2") } @@ -1860,33 +1860,6 @@ class RelationTest < ActiveRecord::TestCase assert_equal 1, posts.unscope(where: :body).count end - def test_unscope_removes_binds - left = Post.where(id: 20) - - assert_equal 1, left.bound_attributes.length - - relation = left.unscope(where: :id) - assert_equal [], relation.bound_attributes - end - - def test_merging_removes_rhs_binds - left = Post.where(id: 20) - right = Post.where(id: [1, 2, 3, 4]) - - assert_equal 1, left.bound_attributes.length - - merged = left.merge(right) - assert_equal [], merged.bound_attributes - end - - def test_merging_keeps_lhs_binds - right = Post.where(id: 20) - left = Post.where(id: 10) - - merged = left.merge(right) - assert_equal [20], merged.bound_attributes.map(&:value) - end - def test_locked_should_not_build_arel posts = Post.locked assert posts.locked? @@ -1897,24 +1870,6 @@ class RelationTest < ActiveRecord::TestCase assert_equal "Thank you for the welcome,Thank you again for the welcome", Post.first.comments.join(",") end - def test_connection_adapters_can_reorder_binds - posts = Post.limit(1).offset(2) - - stubbed_connection = Post.connection.dup - def stubbed_connection.combine_bind_parameters(**kwargs) - offset = kwargs[:offset] - kwargs[:offset] = kwargs[:limit] - kwargs[:limit] = offset - super(**kwargs) - end - - posts.define_singleton_method(:connection) do - stubbed_connection - end - - assert_equal 2, posts.to_a.length - end - test "#skip_query_cache!" do Post.cache do assert_queries(1) do -- cgit v1.2.3