From 2e70cc5d99123ae1f20badadefe6000e32295516 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 25 Oct 2013 14:48:23 +1100 Subject: [PATCH 1/2] Update queryable interface so that it doesn't mutate --- middleman-core/features/queryable.feature | 6 ++++- .../step_definitions/queryable_steps.rb | 14 ++++++++++- .../lib/middleman-core/sitemap/queryable.rb | 25 +++++++++++++------ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/middleman-core/features/queryable.feature b/middleman-core/features/queryable.feature index b4a7294c..1382db3f 100644 --- a/middleman-core/features/queryable.feature +++ b/middleman-core/features/queryable.feature @@ -28,4 +28,8 @@ Feature: Queryable Selector Then should support ordering by attribute ascending Then should support ordering by attribute descending Then should order by attribute ascending by default - Then should exclude documents that do not own the attribute \ No newline at end of file + Then should exclude documents that do not own the attribute + Scenario: Passing queries around + Given a simple 'where' query + When I chain a where clause onto that query + Then the original query should remain unchanged diff --git a/middleman-core/features/step_definitions/queryable_steps.rb b/middleman-core/features/step_definitions/queryable_steps.rb index 788625ab..c14fcd6c 100644 --- a/middleman-core/features/step_definitions/queryable_steps.rb +++ b/middleman-core/features/step_definitions/queryable_steps.rb @@ -1,3 +1,15 @@ +Given /^a simple 'where' query$/ do + @query = Middleman::Sitemap::Queryable::Query.new({}).where(:foo => 'bar') +end + +When /^I chain a where clause onto that query$/ do + @new_query = @query.where(:baz => 'foo') +end + +Then /^the original query should remain unchanged$/ do + @query.should_not eql @new_query +end + Then /^should initialize with an attribute and an operator$/ do selector = ::Middleman::Sitemap::Queryable::Selector.new :attribute => :author, :operator => 'equal' :author.should == selector.attribute @@ -120,4 +132,4 @@ end Then /^should exclude documents that do not own the attribute$/ do found_documents = @server_inst.sitemap.order_by(:status).all found_documents.map { |r| r.raw_data[:id] }.to_set.should == [1,2].to_set -end \ No newline at end of file +end diff --git a/middleman-core/lib/middleman-core/sitemap/queryable.rb b/middleman-core/lib/middleman-core/sitemap/queryable.rb index 0597bd22..fbe83ac1 100644 --- a/middleman-core/lib/middleman-core/sitemap/queryable.rb +++ b/middleman-core/lib/middleman-core/sitemap/queryable.rb @@ -65,9 +65,12 @@ module Middleman end class Query - def initialize(model) + def initialize(model, opts={}) @model = model - @where = {} + @where = opts[:where] || {} + @order_by = opts[:order_by] + @offset = opts[:offset] + @limit = opts[:limit] end def where(constraints_hash) @@ -78,22 +81,30 @@ module Middleman selector_hash.update({ selector => value }) end @where.merge! selector_hash - self + Query.new @model, opts + end + + def opts + { :where => @where, + :order_by => @order_by, + :offset => @offset, + :limit => @limit + } end def order_by(field) @order_by = field.is_a?(Symbol) ? {field => :asc} : field - self + Query.new @model, opts end def offset(number) @offset = number - self + Query.new @model, opts end def limit(number) @limit = number - self + Query.new @model, opts end def first @@ -145,4 +156,4 @@ class Symbol self.to_s <=> other.to_s end end -end \ No newline at end of file +end From e8de5907fafa5d7488e4224eaf463872be0250d6 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 25 Oct 2013 18:09:36 +1100 Subject: [PATCH 2/2] Actually fix the issue, rather than pretending to --- .../step_definitions/queryable_steps.rb | 2 +- .../lib/middleman-core/sitemap/queryable.rb | 18 +++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/middleman-core/features/step_definitions/queryable_steps.rb b/middleman-core/features/step_definitions/queryable_steps.rb index c14fcd6c..995c3d33 100644 --- a/middleman-core/features/step_definitions/queryable_steps.rb +++ b/middleman-core/features/step_definitions/queryable_steps.rb @@ -7,7 +7,7 @@ When /^I chain a where clause onto that query$/ do end Then /^the original query should remain unchanged$/ do - @query.should_not eql @new_query + @query.opts({}).should_not eql @new_query.opts({}) end Then /^should initialize with an attribute and an operator$/ do diff --git a/middleman-core/lib/middleman-core/sitemap/queryable.rb b/middleman-core/lib/middleman-core/sitemap/queryable.rb index fbe83ac1..59ce45ce 100644 --- a/middleman-core/lib/middleman-core/sitemap/queryable.rb +++ b/middleman-core/lib/middleman-core/sitemap/queryable.rb @@ -80,31 +80,27 @@ module Middleman selector = Selector.new(:attribute => attribute, :operator => 'equal') selector_hash.update({ selector => value }) end - @where.merge! selector_hash - Query.new @model, opts + Query.new @model, opts(:where => @where.merge(selector_hash)) end - def opts - { :where => @where, + def opts new_opts + { :where => {}.merge(@where), :order_by => @order_by, :offset => @offset, :limit => @limit - } + }.merge(new_opts) end def order_by(field) - @order_by = field.is_a?(Symbol) ? {field => :asc} : field - Query.new @model, opts + Query.new @model, opts(:order_by => field.is_a?(Symbol) ? {field => :asc} : field) end def offset(number) - @offset = number - Query.new @model, opts + Query.new @model, opts(:offset => number) end def limit(number) - @limit = number - Query.new @model, opts + Query.new @model, opts(:limit => number) end def first