Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merging filters with array conditional operators into $or clauses whe…

…n necessary
  • Loading branch information...
commit d4f941a76a802ae71e5a56ddbfe1eb2e808f0469 1 parent 0effd61
@vicentemundim authored
View
2  Gemfile.lock
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
- mongoid_query_string_interface (0.2.4)
+ mongoid_query_string_interface (0.2.5)
json (>= 1.4.6)
mongoid (~> 2.0.0.rc)
View
62 lib/mongoid/parsers/filter_parser.rb
@@ -37,6 +37,40 @@ def operator
@operator ||= operator_from(raw_attribute)
end
+ def or_attribute?
+ raw_attribute == 'or'
+ end
+
+ def include?(other_filter)
+ if or_attribute?
+ json_value.any? do |filters|
+ filters.filter_parsers.any? do |filter_parser|
+ filter_parser.attribute == other_filter.attribute &&
+ conditional_array_operators.include?(filter_parser.operator) &&
+ filter_parser.operator == other_filter.operator
+ end
+ end
+ end
+ end
+
+ def merge(other_filter)
+ if or_attribute?
+ @value = json_value.map do |filters|
+ filters.filter_parsers << other_filter
+ filters.parse
+ end
+ elsif conditional_array_operators.include?(other_filter.operator) && operator == other_filter.operator
+ @value = value.inject({}) do |result, filter|
+ filter_operation, filter_value = filter
+ filter_value = filter_value.concat(other_filter.value[filter_operation]) if other_filter.value[filter_operation]
+ result[filter_operation] = filter_value
+ result
+ end
+ else
+ @value = value.merge(other_filter.value)
+ end
+ end
+
private
def parsed_attribute
if or_attribute?
@@ -52,8 +86,6 @@ def expanded_value
if operator
if or_attribute?
parsed_json_value
- elsif all_operator? && parsed_value.size == 1
- { '$in' => parsed_value }
else
{ operator => parsed_value }
end
@@ -76,18 +108,22 @@ def parsed_value
def parsed_json_value
if unescaped_raw_value.is_a?(String)
- raw_or_data = ::JSON.parse(unescaped_raw_value)
-
- raise "$or query filters must be given as an array of hashes" unless valid_or_filters?(raw_or_data)
-
- raw_or_data.map do |filters|
- FiltersParser.new(filters).parse
- end
+ json_value.map(&:parse)
else
unescaped_raw_value
end
end
+ def json_value
+ raw_or_data = ::JSON.parse(unescaped_raw_value)
+
+ raise "$or query filters must be given as an array of hashes" unless valid_or_filters?(raw_or_data)
+
+ raw_or_data.map do |filters|
+ FiltersParser.new(filters)
+ end
+ end
+
def valid_or_filters?(raw_or_data)
raw_or_data.is_a?(Array) and raw_or_data.all? { |item| item.is_a?(Hash) }
end
@@ -96,12 +132,8 @@ def unescaped_raw_value
@unescaped_raw_value ||= raw_value.is_a?(String) ? CGI.unescape(raw_value) : raw_value
end
- def all_operator?
- operator == '$all'
- end
-
- def or_attribute?
- raw_attribute == 'or'
+ def conditional_array_operators
+ Mongoid::QueryStringInterface::ARRAY_CONDITIONAL_OPERATORS.map { |o| "$#{o}" }
end
def operator_from(attribute)
View
42 lib/mongoid/parsers/filters_parser.rb
@@ -13,25 +13,55 @@ def parse
default_filters.merge(parsed_filters)
end
+ def filter_parsers
+ @filter_parsers ||= filters.map do |raw_attribute, raw_value|
+ FilterParser.new(raw_attribute, raw_value)
+ end
+ end
+
private
def parsed_filters
- filter_parsers.inject({}) do |result, filter_parser|
+ filter_parsers_hash.inject({}) do |result, item|
+ attribute, filter_parser = item
+
+ result[attribute] = filter_parser.value
+
+ result
+ end
+ end
+
+ def filter_parsers_hash
+ optimized_filter_parsers.inject({}) do |result, filter_parser|
if result.has_key?(filter_parser.attribute)
- result[filter_parser.attribute].merge!(filter_parser.value)
+ result[filter_parser.attribute].merge(filter_parser)
else
- result[filter_parser.attribute] = filter_parser.value
+ result[filter_parser.attribute] = filter_parser
end
result
end
end
- def filter_parsers
- filters.map do |raw_attribute, raw_value|
- FilterParser.new(raw_attribute, raw_value)
+ def optimized_filter_parsers
+ if or_filter_parser
+ filter_parsers.inject([]) do |result, filter_parser|
+ if filter_parser != or_filter_parser && or_filter_parser.include?(filter_parser)
+ or_filter_parser.merge(filter_parser)
+ else
+ result << filter_parser
+ end
+
+ result
+ end
+ else
+ filter_parsers
end
end
+
+ def or_filter_parser
+ @or_filter_parser ||= filter_parsers.select { |filter_parser| filter_parser.or_attribute? }.first
+ end
end
end
end
View
2  lib/version.rb
@@ -1,6 +1,6 @@
# encoding: utf-8
module Mongoid #:nodoc
module QueryStringInterface #:nodoc
- VERSION = "0.2.4"
+ VERSION = "0.2.5"
end
end
View
70 spec/mongoid/parsers/filter_parser_spec.rb
@@ -326,33 +326,17 @@
Mongoid::QueryStringInterface::ARRAY_CONDITIONAL_OPERATORS.each do |operator|
context "with array operator $#{operator}" do
- if operator == :all
- context "with a single value" do
- subject do
- described_class.new("tags.all", 'Some Value')
- end
-
- it "should return the field name as attribute" do
- subject.attribute.should == 'tags'
- end
-
- it "should return the value as an array, using the operator $in instead of all" do
- subject.value.should == { "$in" => ['Some Value'] }
- end
+ context "with a single value" do
+ subject do
+ described_class.new("tags.#{operator}", 'Some Value')
end
- else
- context "with a single value" do
- subject do
- described_class.new("tags.#{operator}", 'Some Value')
- end
-
- it "should return the field name as attribute" do
- subject.attribute.should == 'tags'
- end
-
- it "should return the value as an array, using the operator" do
- subject.value.should == { "$#{operator}" => ['Some Value'] }
- end
+
+ it "should return the field name as attribute" do
+ subject.attribute.should == 'tags'
+ end
+
+ it "should return the value as an array, using the operator" do
+ subject.value.should == { "$#{operator}" => ['Some Value'] }
end
end
@@ -382,4 +366,38 @@
end
end
end
+
+ describe "include?" do
+ describe "or filters" do
+ subject do
+ described_class.new("or", '[{"title": "Some Title"}, {"count.gte": "1", "count.lt": "10"}, {"tags.all": "Some tag|Other tag", "tags.nin": ["A tag", "Another tag"]}]')
+ end
+
+ let :other_filter do
+ described_class.new("tags.all", 'Other filter tag')
+ end
+
+ it "should include other filter if they have an array conditional operator and their attribute is used in one of the or clauses" do
+ subject.should include(other_filter)
+ end
+
+ it "should merge other filter if they have an array conditional operator and their attribute is used in one of the or clauses" do
+ subject.merge(other_filter).should == [{'title' => 'Some Title', 'tags' => {'$all' => ['Other filter tag']}}, {'count' => { '$gte' => 1, '$lt' => 10 }, 'tags' => {'$all' => ['Other filter tag']}}, {'tags' => { '$all' => ['Some tag', 'Other tag', 'Other filter tag'], '$nin' => ["A tag", "Another tag"] }}]
+ end
+ end
+
+ describe "normal filters" do
+ subject do
+ described_class.new("tags.all", 'Some Value|Some Other Value|Another value')
+ end
+
+ let :other_filter do
+ described_class.new("tags.all", 'Some tag')
+ end
+
+ it "should not include other filters" do
+ subject.should_not include(other_filter)
+ end
+ end
+ end
end
View
32 spec/mongoid/parsers/filters_parser_spec.rb
@@ -68,38 +68,6 @@
subject.parse.should == default_filters.merge({ 'count' => { '$gte' => 1, '$lt' => 10 } })
end
end
-
- describe "optimizations for array operators" do
- context "when only one tag is given to $all" do
- let :filters do
- { 'tags.all' => 'esportes' }.with_indifferent_access
- end
-
- it "should convert to a $in parameter" do
- subject.parse.should == default_filters.merge('tags' => { '$in' => ['esportes'] })
- end
-
- context "and there is another tags parameter" do
- let :filters do
- { 'tags.all' => 'esportes', 'tags.nin' => 'futebol' }.with_indifferent_access
- end
-
- it "should convert to a $in parameter" do
- subject.parse.should == default_filters.merge('tags' => { '$in' => ['esportes'], '$nin' => ['futebol'] })
- end
- end
- end
-
- context "when more than one tag is given to $all" do
- let :filters do
- { 'tags.all' => 'esportes|Flamengo' }.with_indifferent_access
- end
-
- it "should not modify the $all parameter value" do
- subject.parse.should == default_filters.merge('tags' => { '$all' => ['esportes', 'Flamengo'] })
- end
- end
- end
end
describe "with $or operator" do
View
65 spec/mongoid/query_string_interface_spec.rb
@@ -204,6 +204,18 @@ class EmbeddedDocument
end
context 'with conditional operators' do
+ let :default_parameters do
+ Document.default_filtering_options.inject({}) { |r, i| k, v = i; r[k.to_s] = v; r }
+ end
+
+ let :criteria do
+ criteria = mock(Mongoid::Criteria)
+ criteria.stub!(:where).and_return(criteria)
+ criteria.stub!(:order_by).and_return(criteria)
+ criteria.stub!(:paginate).and_return(criteria)
+ criteria
+ end
+
it 'should use it when given as the last portion of attribute name' do
Document.filter_by('title.ne' => 'Some Other Title').should == [document]
end
@@ -284,18 +296,6 @@ class EmbeddedDocument
:created_at => 5.days.ago.to_time, :tags => ['esportes', 'basquete', 'flamengo', 'rede globo', 'esporte espetacular']
end
- let :default_parameters do
- Document.default_filtering_options.inject({}) { |r, i| k, v = i; r[k.to_s] = v; r }
- end
-
- let :criteria do
- criteria = mock(Mongoid::Criteria)
- criteria.stub!(:where).and_return(criteria)
- criteria.stub!(:order_by).and_return(criteria)
- criteria.stub!(:paginate).and_return(criteria)
- criteria
- end
-
it 'should convert values into arrays for operator $all' do
Document.filter_by('tags.all' => document.tags.join('|')).should == [document]
end
@@ -325,33 +325,6 @@ class EmbeddedDocument
document_with_similar_tags
Document.filter_by('tags.all' => 'esportes|basquete', 'tags.nin' => 'rede globo|esporte espetacular').should == [document]
end
-
- context "when only one tag is given to $all" do
- it "should convert to a $in parameter for optimization" do
- Document.should_receive(:where)
- .with(default_parameters.merge('tags' => { '$in' => ['esportes'] }))
- .and_return(criteria)
- Document.filter_by('tags.all' => 'esportes')
- end
-
- context "and there is another tags parameter" do
- it "should convert to a $in parameter for optimization" do
- Document.should_receive(:where)
- .with(default_parameters.merge('tags' => { '$in' => ['esportes'], '$nin' => ['futebol'] }))
- .and_return(criteria)
- Document.filter_by('tags.all' => 'esportes', 'tags.nin' => 'futebol')
- end
- end
- end
-
- context "when more than one tag is given to $all" do
- it "should not modify the $all parameter value" do
- Document.should_receive(:where)
- .with(default_parameters.merge('tags' => { '$all' => ['esportes', 'Flamengo'] }))
- .and_return(criteria)
- Document.filter_by('tags.all' => 'esportes|Flamengo')
- end
- end
end
context "with 'or' attribute" do
@@ -366,6 +339,20 @@ class EmbeddedDocument
it "should accept any valid mongodb query" do
Document.filter_by('or' => '[{"tags.all": ["flamengo", "basquete"]}, {"tags": {"$all" : ["flamengo", "jabulani"]}}]').should == [other_document, document]
end
+
+ context "with other parameters outside $or" do
+ context "that use array conditional operators" do
+ context "with single values" do
+ it "should merge outside parameters into $or clauses" do
+ Document.should_receive(:where)
+ .with(default_parameters.merge('$or' => [{'tags' => { "$all" => ['basquete', 'flamengo'] }}, {'tags' => { "$all" => ['jabulani', 'flamengo'] }}]))
+ .and_return(criteria)
+
+ Document.filter_by('tags.all' => 'flamengo', 'or' => '[{"tags.all": ["basquete"]}, {"tags.all" : ["jabulani"]}]')
+ end
+ end
+ end
+ end
end
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.