diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index eac70cc622..2ffdac2ca3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,20 @@ +* Support merging option `:rewhere` to allow mergee side condition to be replaced exactly. + + ```ruby + david_and_mary = Author.where(id: david.id..mary.id) + + # both conflict conditions exists + david_and_mary.merge(Author.where(id: bob)) # => [] + + # mergee side condition is replaced by rewhere + david_and_mary.merge(Author.rewhere(id: bob)) # => [bob] + + # mergee side condition is replaced by rewhere option + david_and_mary.merge(Author.where(id: bob), rewhere: true) # => [bob] + ``` + + *Ryuta Kamizono* + * Add support for finding records based on signed ids, which are tamper-proof, verified ids that can be set to expire and scoped with a purpose. This is particularly useful for things like password reset or email verification, where you want the bearer of the signed id to be able to interact with the diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 27ecd4b25e..b3909f3a27 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -7,15 +7,16 @@ module ActiveRecord class HashMerger # :nodoc: attr_reader :relation, :hash - def initialize(relation, hash) + def initialize(relation, hash, rewhere = nil) hash.assert_valid_keys(*Relation::VALUE_METHODS) @relation = relation @hash = hash + @rewhere = rewhere end - def merge #:nodoc: - Merger.new(relation, other).merge + def merge + Merger.new(relation, other, @rewhere).merge end # Applying values to a relation has some side effects. E.g. @@ -48,10 +49,11 @@ module ActiveRecord class Merger # :nodoc: attr_reader :relation, :values, :other - def initialize(relation, other) + def initialize(relation, other, rewhere = nil) @relation = relation @values = other.values @other = other + @rewhere = rewhere end NORMAL_VALUES = Relation::VALUE_METHODS - @@ -172,7 +174,7 @@ module ActiveRecord def merge_clauses relation.from_clause = other.from_clause if replace_from_clause? - where_clause = relation.where_clause.merge(other.where_clause) + where_clause = relation.where_clause.merge(other.where_clause, @rewhere) relation.where_clause = where_clause unless where_clause.empty? having_clause = relation.having_clause.merge(other.having_clause) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 4089f184ee..9be4e82033 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -702,15 +702,10 @@ module ActiveRecord # This is short-hand for unscope(where: conditions.keys).where(conditions). # Note that unlike reorder, we're only unscoping the named conditions -- not the entire where statement. def rewhere(conditions) - attrs = [] scope = spawn - where_clause = scope.build_where_clause(conditions) - where_clause.each_attribute do |attr| - attrs << attr - end - scope.unscope!(where: attrs) + scope.unscope!(where: where_clause.extract_attributes) scope.where_clause += where_clause scope end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 3f6dd50139..f44f3e9d01 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -28,21 +28,22 @@ module ActiveRecord # # => Post.where(published: true).joins(:comments) # # This is mainly intended for sharing common conditions between multiple associations. - def merge(other) + def merge(other, *rest) if other.is_a?(Array) records & other elsif other - spawn.merge!(other) + spawn.merge!(other, *rest) else raise ArgumentError, "invalid argument: #{other.inspect}." end end - def merge!(other) # :nodoc: + def merge!(other, *rest) # :nodoc: + options = rest.extract_options! if other.is_a?(Hash) - Relation::HashMerger.new(self, other).merge + Relation::HashMerger.new(self, other, options[:rewhere]).merge elsif other.is_a?(Relation) - Relation::Merger.new(self, other).merge + Relation::Merger.new(self, other, options[:rewhere]).merge elsif other.respond_to?(:to_proc) instance_exec(&other) else diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index b2c4f3e033..24e2fc3665 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -10,21 +10,21 @@ module ActiveRecord end def +(other) - WhereClause.new( - predicates + other.predicates, - ) + WhereClause.new(predicates + other.predicates) end def -(other) - WhereClause.new( - predicates - other.predicates, - ) + WhereClause.new(predicates - other.predicates) end - def merge(other) - WhereClause.new( - predicates_unreferenced_by(other) + other.predicates, - ) + def merge(other, rewhere = nil) + predicates = if rewhere + except_predicates(other.extract_attributes) + else + predicates_unreferenced_by(other) + end + + WhereClause.new(predicates + other.predicates) end def except(*columns) @@ -98,10 +98,12 @@ module ActiveRecord end end - def each_attribute(&block) + def extract_attributes + attrs = [] predicates.each do |node| - Arel.fetch_attribute(node, &block) + Arel.fetch_attribute(node) { |attr| attrs << attr } end + attrs end protected diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 849a97ee71..10c3d17433 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -13,6 +13,81 @@ require "models/rating" class RelationMergingTest < ActiveRecord::TestCase fixtures :developers, :comments, :authors, :author_addresses, :posts + def test_merge_in_clause + david, mary, bob = authors(:david, :mary, :bob) + + david_and_mary = Author.where(id: [david, mary]).order(:id) + mary_and_bob = Author.where(id: [mary, bob]).order(:id) + + assert_equal [david, mary], david_and_mary + assert_equal [mary, bob], mary_and_bob + + assert_equal [mary], david_and_mary.merge(Author.where(id: mary)) + assert_equal [bob], david_and_mary.merge(Author.where(id: bob)) + + assert_equal [mary], david_and_mary.merge(Author.rewhere(id: mary)) + assert_equal [bob], david_and_mary.merge(Author.rewhere(id: bob)) + + assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true) + assert_equal [bob], david_and_mary.merge(Author.where(id: bob), rewhere: true) + + assert_equal [mary, bob], david_and_mary.merge(mary_and_bob) + assert_equal [david, mary], mary_and_bob.merge(david_and_mary) + + assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true) + assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true) + end + + def test_merge_between_clause + david, mary, bob = authors(:david, :mary, :bob) + + david_and_mary = Author.where(id: david.id..mary.id).order(:id) + mary_and_bob = Author.where(id: mary.id..bob.id).order(:id) + + assert_equal [david, mary], david_and_mary + assert_equal [mary, bob], mary_and_bob + + assert_equal [mary], david_and_mary.merge(Author.where(id: mary)) + assert_equal [], david_and_mary.merge(Author.where(id: bob)) + + assert_equal [mary], david_and_mary.merge(Author.rewhere(id: mary)) + assert_equal [bob], david_and_mary.merge(Author.rewhere(id: bob)) + + assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true) + assert_equal [bob], david_and_mary.merge(Author.where(id: bob), rewhere: true) + + assert_equal [mary], david_and_mary.merge(mary_and_bob) + assert_equal [mary], mary_and_bob.merge(david_and_mary) + + assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true) + assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true) + end + + def test_merge_or_clause + david, mary, bob = authors(:david, :mary, :bob) + + david_and_mary = Author.where(id: david).or(Author.where(id: mary)).order(:id) + mary_and_bob = Author.where(id: mary).or(Author.where(id: bob)).order(:id) + + assert_equal [david, mary], david_and_mary + assert_equal [mary, bob], mary_and_bob + + assert_equal [mary], david_and_mary.merge(Author.where(id: mary)) + assert_equal [], david_and_mary.merge(Author.where(id: bob)) + + assert_equal [mary], david_and_mary.merge(Author.rewhere(id: mary)) + assert_equal [bob], david_and_mary.merge(Author.rewhere(id: bob)) + + assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true) + assert_equal [bob], david_and_mary.merge(Author.where(id: bob), rewhere: true) + + assert_equal [mary], david_and_mary.merge(mary_and_bob) + assert_equal [mary], mary_and_bob.merge(david_and_mary) + + assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true) + assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true) + end + def test_relation_merging devs = Developer.where("salary >= 80000").merge(Developer.limit(2)).merge(Developer.order("id ASC").where("id < 3")) assert_equal [developers(:david), developers(:jamis)], devs.to_a