Merge pull request #37 from cheald/compound_or

Rewrite CriteriaHash#merge to be less gnarly. Add $or merging.

lib/plucky/criteria_hash.rb

@@ -77,58 +77,9 @@ def to_hash
       @source
     end
 
-    # Public and completely disgusting
+    # Public
     def merge(other)
-      target = @source.dup
-      other.source.each_key do |key|
-        value, other_value = target[key], other[key]
-        target[key] =
-          if target.key?(key)
-            value_is_hash = value.is_a?(Hash)
-            other_is_hash = other_value.is_a?(Hash)
-
-            if value_is_hash && other_is_hash
-              value.update(other_value) do |key, old_value, new_value|
-                if old_value.is_a?(Hash) && new_value.is_a?(Hash)
-                  self.class.new(old_value).merge(self.class.new(new_value)).to_hash
-                else
-                  merge_values_into_array(old_value, new_value)
-                end
-              end
-            elsif value_is_hash && !other_is_hash
-              if modifier_key = value.keys.detect { |k| Plucky.modifier?(k) }
-                current_value = value[modifier_key]
-                value[modifier_key] = current_value.concat(array(other_value)).uniq
-              else
-                # kaboom! Array(value).concat(Array(other_value)).uniq
-              end
-            elsif other_is_hash && !value_is_hash
-              if modifier_key = other_value.keys.detect { |k| Plucky.modifier?(k) }
-                current_value = other_value[modifier_key]
-                other_value[modifier_key] = current_value.concat(array(value)).uniq
-              else
-                # kaboom! Array(value).concat(Array(other_value)).uniq
-              end
-            else
-              merge_values_into_array(value, other_value)
-            end
-          else
-            other_value
-          end
-      end
-      self.class.new(target)
-    end
-
-    # Private
-    def merge_values_into_array(value, other_value)
-      array(value).concat(array(other_value)).uniq
-    end
-
-    # Private: Array(BSON::ObjectId) returns the byte array or what not instead
-    # of the object id. This makes sure it is an array of object ids, not the
-    # guts of the object id.
-    def array(value)
-      value.is_a?(BSON::ObjectId) ? [value] : Array(value)
+      self.class.new hash_merge(@source, other.source)
     end
 
     # Public
@@ -139,17 +90,6 @@ def merge!(other)
       self
     end
 
-    # Private
-    def object_ids
-      @options[:object_ids] ||= []
-    end
-
-    # Private
-    def object_ids=(value)
-      raise ArgumentError unless value.is_a?(Array)
-      @options[:object_ids] = value.flatten
-    end
-
     # Public: The definition of simple is querying by only _id or _id and _type.
     # If this is the case, you can use IdentityMap in library to not perform
     # query and instead just return from map.
@@ -165,6 +105,75 @@ def object_id?(key)
       object_ids.include?(key.to_sym)
     end
 
+    # Private
+    def object_ids
+      @options[:object_ids] ||= []
+    end
+
+    # Private
+    def object_ids=(value)
+      raise ArgumentError unless value.is_a?(Array)
+      @options[:object_ids] = value.flatten
+    end
+
+    private
+
+    # Private
+    def hash_merge(oldhash, newhash)
+      merge_compound_or_clauses!(oldhash, newhash)
+      oldhash.merge(newhash) do |key, oldval, newval|
+        old_is_hash = oldval.instance_of? Hash
+        new_is_hash = newval.instance_of? Hash
+
+        if old_is_hash && new_is_hash
+          hash_merge(oldval, newval)
+        elsif old_is_hash
+          modifier_merge(oldval, newval)
+        elsif new_is_hash
+          modifier_merge(newval, oldval)
+        else
+          merge_values_into_array(oldval, newval)
+        end
+      end
+    end
+
+    def merge_compound_or_clauses!(oldhash, newhash)
+      old_or = oldhash[:$or]
+      new_or = newhash[:$or]
+      if old_or && new_or
+        oldhash[:$and] ||= []
+        oldhash[:$and] << {:$or => oldhash.delete(:$or)}
+        oldhash[:$and] << {:$or => newhash.delete(:$or)}
+      elsif new_or && oldhash[:$and]
+        if oldhash[:$and].any? {|v| v.key? :$or }
+          oldhash[:$and] << {:$or => newhash.delete(:$or)}
+        end
+      elsif old_or && newhash[:$and]
+        if newhash[:$and].any? {|v| v.key? :$or }
+          newhash[:$and] << {:$or => oldhash.delete(:$or)}
+        end
+      end
+    end
+
+    # Private
+    def modifier_merge(hash, value)
+      if modifier_key = hash.keys.detect { |k| Plucky.modifier?(k) }
+        hash[modifier_key].concat( array(value) ).uniq
+      end
+    end
+
+    # Private
+    def merge_values_into_array(value, other_value)
+      array(value).concat(array(other_value)).uniq
+    end
+
+    # Private: Array(BSON::ObjectId) returns the byte array or what not instead
+    # of the object id. This makes sure it is an array of object ids, not the
+    # guts of the object id.
+    def array(value)
+      value.is_a?(BSON::ObjectId) ? [value] : Array(value)
+    end
+
     # Private
     def normalized_key(key)
       key_normalizer.call(key)

spec/plucky/criteria_hash_spec.rb

@@ -147,6 +147,40 @@
       c1.merge(c2).should_not equal(c1)
       c1[:foo].should == 'bar'
     end
+
+    context "given multiple $or clauses" do
+      before do
+        @c1 = described_class.new(:$or => [{:a => 1}, {:b => 2}])
+        @c2 = described_class.new(:$or => [{:a => 3}, {:b => 4}])
+        @c3 = described_class.new(:$or => [{:a => 4}, {:b => 4}])
+      end
+
+      it "merges two $ors into a compound $and" do
+        merged = @c1.merge(@c2)
+        merged[:$and].should == [
+          {:$or => [{:a => 1}, {:b => 2}]},
+          {:$or => [{:a => 3}, {:b => 4}]}
+        ]
+      end
+
+      it "merges an $and and a $or into a compound $and" do
+        merged = @c1.merge(@c2).merge(@c3)
+        merged[:$and].should == [
+          {:$or => [{:a => 1}, {:b => 2}]},
+          {:$or => [{:a => 3}, {:b => 4}]},
+          {:$or => [{:a => 4}, {:b => 4}]}
+        ]
+      end
+
+      it "merges an $or and an $and into a compound $and" do
+        merged = @c3.merge @c1.merge(@c2)
+        merged[:$and].should == [
+          {:$or => [{:a => 1}, {:b => 2}]},
+          {:$or => [{:a => 3}, {:b => 4}]},
+          {:$or => [{:a => 4}, {:b => 4}]}
+        ]
+      end
+    end
   end
 
   context "#merge!" do