[MODEL] Refactored the MyModel.search method to use the SearchRequest class

Author: karmi

elasticsearch-model/lib/elasticsearch/model/response.rb

@@ -10,16 +10,17 @@ module Response
       # Implements Enumerable and forwards its methods to the {#results} object.
       #
       class Response
-        attr_reader :klass, :response,
+        attr_reader :klass, :search, :response,
                     :took, :timed_out, :shards
 
         include Enumerable
         extend  Support::Forwardable
 
         forward :results, :each, :empty?, :size, :slice, :[], :to_ary
 
-        def initialize(klass, response)
+        def initialize(klass, search, response)
           @klass     = klass
+          @search    = search
           @response  = response
           @took      = response['took']
           @timed_out = response['timed_out']
@@ -29,13 +30,15 @@ def initialize(klass, response)
         # Return the collection of "hits" from Elasticsearch
         #
         def results
-          @results ||= Results.new(klass, response)
+          @response ||= search.execute!
+          @results  ||= Results.new(klass, response)
         end
 
         # Return the collection of records from the database
         #
         def records
-          @records ||= Records.new(klass, response, results)
+          @response ||= search.execute!
+          @records  ||= Records.new(klass, response, results)
         end
 
       end

elasticsearch-model/lib/elasticsearch/model/searching.rb

@@ -5,13 +5,60 @@ module Model
     #
     module Searching
 
+      # Wraps a search request definition
+      #
+      class SearchRequest
+        attr_reader :klass, :definition
+
+        # @param klass [Class] The class of the model
+        # @param query_or_payload [String,Hash,Object] The search request definition
+        #                                              (string, JSON, Hash, or object responding to `to_hash`)
+        # @param options [Hash] Optional parameters to be passed to the Elasticsearch client
+        #
+        def initialize(klass, query_or_payload, options={})
+          @klass   = klass
+
+          __index_name    = options[:index] || klass.index_name
+          __document_type = options[:type]  || klass.document_type
+
+          case
+            # search query: ...
+            when query_or_payload.respond_to?(:to_hash)
+              body = query_or_payload.to_hash
+
+            # search '{ "query" : ... }'
+            when query_or_payload.is_a?(String) && query_or_payload =~ /^\s*{/
+              body = query_or_payload
+
+            # search '...'
+            else
+              q = query_or_payload
+          end
+
+          if body
+            @definition = { index: __index_name, type: __document_type, body: body }
+          else
+            @definition = { index: __index_name, type: __document_type, q: q }
+          end
+        end
+
+        # Performs the request and returns the response from client
+        #
+        # @return [Hash] The response from Elasticsearch
+        #
+        def execute!
+          klass.client.search(@definition)
+        end
+      end
+
       module ClassMethods
 
-        # Provide a `search` method for the model to easily search within an index/type
+        # Provides a `search` method for the model to easily search within an index/type
         # corresponding to the model settings.
         #
-        # @param query_or_payload [String,Hash] Query, Hash or JSON payload to use as search definition
-        # @param options          [Hash]        Optional parameters to be passed to the Elasticsearch client
+        # @param query_or_payload [String,Hash,Object] The search request definition
+        #                                              (string, JSON, Hash, or object responding to `to_hash`)
+        # @param options [Hash] Optional parameters to be passed to the Elasticsearch client
         #
         # @return [Elasticsearch::Model::Response::Response]
         #
@@ -48,24 +95,10 @@ module ClassMethods
         #     Article.search '{"query" : { "match_all" : {} }}'
         #
         def search(query_or_payload, options={})
-          __index_name    = options[:index] || index_name
-          __document_type = options[:type]  || document_type
-
-          case
-            # search query: ...
-            when query_or_payload.respond_to?(:to_hash)
-              response = client.search index: __index_name, type: __document_type, body: query_or_payload.to_hash
-
-            # search '{ "query" : ... }'
-            when query_or_payload.is_a?(String) && query_or_payload =~ /^\s*{/
-              response = client.search index: __index_name, type: __document_type, body: query_or_payload
-
-            # search '...'
-            else
-              response = client.search index: __index_name, type: __document_type, q: query_or_payload
-          end
+          search   = SearchRequest.new(self, query_or_payload, options={})
+          response = search.execute!
 
-          Response::Response.new(self, response)
+          Response::Response.new(self, search, response)
         end
 
       end

elasticsearch-model/test/unit/response_test.rb

@@ -2,32 +2,37 @@
 
 class Elasticsearch::Model::ResponseTest < Test::Unit::TestCase
   context "Response" do
-    class OriginClass; end
+    class OriginClass
+      def self.index_name;    'foo'; end
+      def self.document_type; 'bar'; end
+    end
 
+    SEARCH   = Elasticsearch::Model::Searching::SearchRequest.new OriginClass, '*'
     RESPONSE = { 'took' => '5', 'timed_out' => false, '_shards' => {'one' => 'OK'}, 'hits' => { 'hits' => [] } }
 
     should "access klass, response, took, timed_out, shards" do
-      response = Elasticsearch::Model::Response::Response.new OriginClass, RESPONSE
+      response = Elasticsearch::Model::Response::Response.new OriginClass, SEARCH, RESPONSE
 
       assert_equal OriginClass, response.klass
+      assert_equal SEARCH,   response.search
       assert_equal RESPONSE, response.response
       assert_equal '5',      response.took
       assert_equal false,    response.timed_out
       assert_equal 'OK',     response.shards.one
     end
 
     should "initialize and access the results" do
-      response = Elasticsearch::Model::Response::Response.new OriginClass, RESPONSE
+      response = Elasticsearch::Model::Response::Response.new OriginClass, SEARCH, RESPONSE
       assert_instance_of Elasticsearch::Model::Response::Results, response.results
     end
 
     should "initialize and access the records" do
-      response = Elasticsearch::Model::Response::Response.new OriginClass, RESPONSE
+      response = Elasticsearch::Model::Response::Response.new OriginClass, SEARCH, RESPONSE
       assert_instance_of Elasticsearch::Model::Response::Records, response.records
     end
 
     should "delegate Enumerable methods to results" do
-      response = Elasticsearch::Model::Response::Response.new OriginClass, RESPONSE
+      response = Elasticsearch::Model::Response::Response.new OriginClass, SEARCH, RESPONSE
 
       assert response.empty?
     end

elasticsearch-model/test/unit/searching_search_request_test.rb

@@ -0,0 +1,62 @@
+require 'test_helper'
+
+class Elasticsearch::Model::SearchRequestTest < Test::Unit::TestCase
+  context "SearchRequest class" do
+    class ::DummySearchingModel
+      extend Elasticsearch::Model::Searching::ClassMethods
+
+      def self.index_name;    'foo'; end
+      def self.document_type; 'bar'; end
+
+    end
+
+    setup do
+      @client = mock('client')
+      DummySearchingModel.stubs(:client).returns(@client)
+    end
+
+    should "pass the search definition as a simple query" do
+      @client.expects(:search).with do |params|
+        assert_equal 'foo', params[:q]
+      end
+      .returns({})
+
+      s = Elasticsearch::Model::Searching::SearchRequest.new ::DummySearchingModel, 'foo'
+      s.execute!
+    end
+
+    should "pass the search definition as a Hash" do
+      @client.expects(:search).with do |params|
+        assert_equal( {foo: 'bar'}, params[:body] )
+      end
+      .returns({})
+
+      s = Elasticsearch::Model::Searching::SearchRequest.new ::DummySearchingModel, foo: 'bar'
+      s.execute!
+    end
+
+    should "pass the search definition as a JSON string" do
+      @client.expects(:search).with do |params|
+        assert_equal( '{"foo":"bar"}', params[:body] )
+      end
+      .returns({})
+
+      s = Elasticsearch::Model::Searching::SearchRequest.new ::DummySearchingModel, '{"foo":"bar"}'
+      s.execute!
+    end
+
+    should "pass the search definition as an object which responds to to_hash" do
+      class MySpecialQueryBuilder
+        def to_hash; {foo: 'bar'}; end
+      end
+
+      @client.expects(:search).with do |params|
+        assert_equal( {foo: 'bar'}, params[:body] )
+      end
+      .returns({})
+
+      s = Elasticsearch::Model::Searching::SearchRequest.new ::DummySearchingModel, MySpecialQueryBuilder.new
+      s.execute!
+    end
+  end
+end

elasticsearch-model/test/unit/searching_test.rb

@@ -1,6 +1,6 @@
 require 'test_helper'
 
-class Elasticsearch::Model::SearchTest < Test::Unit::TestCase
+class Elasticsearch::Model::SearchingTest < Test::Unit::TestCase
   context "Searching module" do
     class ::DummySearchingModel
       extend Elasticsearch::Model::Searching::ClassMethods
@@ -19,44 +19,15 @@ def self.document_type; 'bar'; end
       assert_respond_to DummySearchingModel, :search
     end
 
-    should "pass the search definition as simple query" do
-      @client.expects(:search).with do |params|
-        assert_equal 'foo', params[:q]
-      end
-      .returns({})
+    should "execute the search" do
+      Elasticsearch::Model::Searching::SearchRequest
+        .expects(:new).with do |klass, query, options|
+          assert_equal DummySearchingModel, klass
+          assert_equal 'foo', query
+        end
+        .returns( mock('search', execute!: {}) )
 
       DummySearchingModel.search 'foo'
     end
-
-    should "pass the search definition as a Hash" do
-      @client.expects(:search).with do |params|
-        assert_equal( {foo: 'bar'}, params[:body] )
-      end
-      .returns({})
-
-      DummySearchingModel.search foo: 'bar'
-    end
-
-    should "pass the search definition as a JSON string" do
-      @client.expects(:search).with do |params|
-        assert_equal( '{"foo":"bar"}', params[:body] )
-      end
-      .returns({})
-
-      DummySearchingModel.search '{"foo":"bar"}'
-    end
-
-    should "pass the search definition as an object which responds to to_hash" do
-      class MySpecialQueryBuilder
-        def to_hash; {foo: 'bar'}; end
-      end
-
-      @client.expects(:search).with do |params|
-        assert_equal( {foo: 'bar'}, params[:body] )
-      end
-      .returns({})
-
-      DummySearchingModel.search MySpecialQueryBuilder.new
-    end
   end
 end