[REFACTORING] Deprecated the old sort { somefield, 'DESC' } interfa…

…ce, use the more declarative `sort { by :somefield, 'DESC' }` (=> Sort.new.by(:somefield, 'desc'))

lib/tire/model/search.rb

@@ -35,15 +35,14 @@ module ClassMethods
         def search(query=nil, options={}, &block)
           old_wrapper = Tire::Configuration.wrapper
           Tire::Configuration.wrapper self
-          sort  = options[:order] || options[:sort]
-          sort  = Array(sort)
+          sort  = Array( options[:order] || options[:sort] )
           unless block_given?
             s = Tire::Search::Search.new(elasticsearch_index.name, options)
             s.query { string query }
             s.sort do
               sort.each do |t|
                 field_name, direction = t.split(' ')
-                field_name.include?('.') ? field(field_name, direction) : send(field_name, direction)
+                by field_name, direction
               end
             end unless sort.empty?
             s.size( options[:per_page].to_i ) if options[:per_page]

lib/tire/search/sort.rb

@@ -4,20 +4,19 @@ module Search
     class Sort
       def initialize(&block)
         @value = []
-        self.instance_eval(&block) if block_given?
+        block.arity < 1 ? self.instance_eval(&block) : block.call(self) if block_given?
       end
 
-      def field(name, direction=nil)
+      def by(name, direction=nil)
         @value << ( direction ? { name => direction } : name )
         self
       end
 
       def method_missing(id, *args, &block)
-        case arg = args.shift
-          when String, Symbol, Hash then @value << { id => arg }
-          else @value << id
-        end
-        self
+        Tire.warn "Using methods when sorting has been deprecated, please use the `by` method: " +
+                  "sort { by :#{id}#{ args.empty? ? '' : ', ' + args.first.inspect } }"
+
+        by id, args.shift
       end
 
       def to_ary

test/integration/active_record_searchable_test.rb

@@ -132,7 +132,7 @@ def teardown
           should "find first page with five results" do
             results = ActiveRecordArticle.search do |search|
               search.query { |query| query.string @q }
-              search.sort  { title }
+              search.sort  { by :title }
               search.from 0
               search.size 5
             end
@@ -149,7 +149,7 @@ def teardown
           should "find next page with five results" do
             results = ActiveRecordArticle.search do |search|
               search.query { |query| query.string @q }
-              search.sort  { title }
+              search.sort  { by :title }
               search.from 5
               search.size 5
             end
@@ -166,7 +166,7 @@ def teardown
           should "not find a missing page" do
             results = ActiveRecordArticle.search do |search|
               search.query { |query| query.string @q }
-              search.sort  { title }
+              search.sort  { by :title }
               search.from 10
               search.size 5
             end

test/integration/sort_test.rb

@@ -11,7 +11,7 @@ class SortIntegrationTest < Test::Unit::TestCase
         q = '*'
         s = Tire.search('articles-test') do
           query { string q }
-          sort  { title }
+          sort  { by :title }
         end
 
         assert_equal 5, s.results.count
@@ -22,7 +22,7 @@ class SortIntegrationTest < Test::Unit::TestCase
         q = '*'
         s = Tire.search('articles-test') do
           query { string q }
-          sort  { title :desc }
+          sort  { by :title, :desc }
         end
 
         assert_equal 5, s.results.count

test/unit/model_search_test.rb

@@ -139,26 +139,26 @@ class SearchTest < Test::Unit::TestCase
           end
 
           should "allow to pass :order option" do
-            Tire::Search::Sort.any_instance.expects(:title)
+            Tire::Search::Sort.any_instance.expects(:by).with('title', nil)
 
             ActiveModelArticle.search @q, :order => 'title'
           end
 
           should "allow to pass :sort option as :order option" do
-            Tire::Search::Sort.any_instance.expects(:title)
+            Tire::Search::Sort.any_instance.expects(:by).with('title', nil)
 
             ActiveModelArticle.search @q, :sort => 'title'
           end
 
           should "allow to specify sort direction" do
-            Tire::Search::Sort.any_instance.expects(:title).with('DESC')
+            Tire::Search::Sort.any_instance.expects(:by).with('title', 'DESC')
 
             ActiveModelArticle.search @q, :order => 'title DESC'
           end
 
           should "allow to specify more fields to sort on" do
-            Tire::Search::Sort.any_instance.expects(:title).with('DESC')
-            Tire::Search::Sort.any_instance.expects(:field).with('author.name', nil)
+            Tire::Search::Sort.any_instance.expects(:by).with('title', 'DESC')
+            Tire::Search::Sort.any_instance.expects(:by).with('author.name', nil)
 
             ActiveModelArticle.search @q, :order => ['title DESC', 'author.name']
           end

test/unit/search_sort_test.rb

@@ -6,41 +6,74 @@ class SortTest < Test::Unit::TestCase
 
     context "Sort" do
 
-      should "be serialized to JSON" do
-        assert_respond_to Sort.new, :to_json
-      end
+      context "with the old interface" do
 
-      should "encode simple strings" do
-        assert_equal [:foo].to_json, Sort.new.foo.to_json
-      end
+        should "encode simple strings" do
+          assert_equal [:foo].to_json, Sort.new.foo.to_json
+        end
 
-      should "encode method arguments" do
-        assert_equal [:foo => 'desc'].to_json, Sort.new.foo('desc').to_json
-      end
+        should "encode method arguments" do
+          assert_equal [:foo => 'desc'].to_json, Sort.new.foo('desc').to_json
+        end
 
-      should "encode hash" do
-        assert_equal [ :foo => { :reverse => true } ].to_json, Sort.new.foo(:reverse => true).to_json
-      end
+        should "encode hash" do
+          assert_equal [ :foo => { :reverse => true } ].to_json, Sort.new.foo(:reverse => true).to_json
+        end
 
-      should "encode multiple sort fields" do
-        assert_equal [:foo, :bar].to_json, Sort.new.foo.bar.to_json
-      end
+        should "encode multiple sort fields" do
+          assert_equal [:foo, :bar].to_json, Sort.new.foo.bar.to_json
+        end
 
-      should "encode fields when passed as a block to constructor" do
-        s = Sort.new do
-          foo
-          bar 'desc'
-          _score
+        should "encode fields when passed as a block to constructor with deprecated interface" do
+          s = Sort.new do
+            foo
+            bar 'desc'
+            _score
+          end
+          assert_equal [ :foo, {:bar => 'desc'}, :_score ].to_json, s.to_json
         end
-        assert_equal [ :foo, {:bar => 'desc'}, :_score ].to_json, s.to_json
+
       end
 
-      should "encode fields deeper in json" do
-        s = Sort.new { field 'author.name' }
-        assert_equal [ 'author.name' ].to_json, s.to_json
+      context "with the new interface" do
+
+        should "be serialized to JSON" do
+          assert_respond_to Sort.new, :to_json
+        end
+
+        should "encode simple strings" do
+          assert_equal [:foo].to_json, Sort.new.by(:foo).to_json
+        end
+
+        should "encode method arguments" do
+          assert_equal [:foo => 'desc'].to_json, Sort.new.by(:foo, 'desc').to_json
+        end
+
+        should "encode hash" do
+          assert_equal [ :foo => { :reverse => true } ].to_json, Sort.new.by(:foo, :reverse => true).to_json
+        end
+
+        should "encode multiple sort fields in chain" do
+          assert_equal [:foo, :bar].to_json, Sort.new.by(:foo).by(:bar).to_json
+        end
+
+        should "encode fields when passed as a block to constructor" do
+          s = Sort.new do
+            by :foo
+            by :bar, 'desc'
+            by :_score
+          end
+          assert_equal [ :foo, {:bar => 'desc'}, :_score ].to_json, s.to_json
+        end
+
+        should "encode fields deeper in json" do
+          s = Sort.new { by 'author.name' }
+          assert_equal [ 'author.name' ].to_json, s.to_json
+
+          s = Sort.new { by 'author.name', 'desc' }
+          assert_equal [ {'author.name' => 'desc'} ].to_json, s.to_json
+        end
 
-        s = Sort.new { field 'author.name', :desc }
-        assert_equal [ {'author.name' => 'desc'} ].to_json, s.to_json
       end
 
     end

test/unit/search_test.rb

@@ -67,7 +67,11 @@ def foo; 'bar'; end
 
       should "allow chaining" do
         assert_nothing_raised do
-          Search::Search.new('index').query { }.sort { title 'desc' }.size(5).sort { name 'asc' }.from(1)
+          Search::Search.new('index').query { }.
+                                      sort { by :title, 'desc' }.
+                                      size(5).
+                                      sort { by :name, 'asc' }.
+                                      from(1)
         end
       end
 
@@ -108,8 +112,8 @@ def foo; 'bar'; end
         should "allow sorting by multiple fields" do
           s = Search::Search.new('index') do
             sort do
-              title 'desc'
-              _score
+              by :title, 'desc'
+              by :_score
             end
           end
           hash = MultiJson.decode( s.to_json )