[HTTP] Added HTTP client agnostic implementation

* Get rid of dependency on RestClient implementation
* Implemented Tire::HTTP::Response
* Made default implementation(RestClient) to use Tire::HTTP::Response
* All API calls returns false on failure instead of raising exceptions

Closes karmi#98.

.gitignore

@@ -9,4 +9,5 @@ coverage/
 scratch/
 examples/*.html
 *.log
-.rvmrc
+.rvmrc
+tags

lib/tire.rb

@@ -6,6 +6,7 @@
 require 'tire/rubyext/symbol'
 require 'tire/logger'
 require 'tire/configuration'
+require 'tire/http/response'
 require 'tire/client'
 require 'tire/search'
 require 'tire/search/query'

lib/tire/client.rb

@@ -3,20 +3,33 @@ module Tire
   module Client
 
     class RestClient
+
       def self.get(url, data=nil)
-        ::RestClient::Request.new(:method => :get, :url => url, :payload => data).execute
+        tire_http_response ::RestClient::Request.new(:method => :get, :url => url, :payload => data).execute(&just_response)
       end
+
       def self.post(url, data)
-        ::RestClient.post url, data
+        tire_http_response ::RestClient.post(url, data, &just_response)
       end
+
       def self.put(url, data)
-        ::RestClient.put url, data
+        tire_http_response ::RestClient.put(url, data, &just_response)
       end
+
       def self.delete(url)
-        ::RestClient.delete url rescue nil
+        tire_http_response ::RestClient.delete(url, &just_response)
       end
+
       def self.head(url)
-        ::RestClient.head url
+        tire_http_response ::RestClient.head(url, &just_response)
+      end
+
+      def self.tire_http_response(response)
+        Tire::HTTP::Response.new response.body, response.code, response.headers
+      end
+
+      def self.just_response
+        lambda {|response, request, result| response }
       end
     end
 

lib/tire/http/response.rb

@@ -0,0 +1,10 @@
+module Tire
+  module HTTP
+    class Response
+      attr_reader :body, :code, :headers
+      def initialize(body, code, headers = {})
+        @body, @code, @headers = body, code, headers
+      end
+    end
+  end
+end

lib/tire/index.rb

@@ -9,30 +9,24 @@ def initialize(name, &block)
     end
 
     def exists?
-      !!Configuration.client.head("#{Configuration.url}/#{@name}")
-    rescue Exception => error
-      false
+      Configuration.client.head("#{Configuration.url}/#{@name}").code == 200
     end
 
     def delete
-      # FIXME: RestClient does not return response for DELETE requests?
       @response = Configuration.client.delete "#{Configuration.url}/#{@name}"
-      return @response.body =~ /error/ ? false : true
-    rescue Exception => error
-      false
+      return true if @response.code == 200
     ensure
       curl = %Q|curl -X DELETE "#{Configuration.url}/#{@name}"|
-      logged(error, 'DELETE', curl)
+      logged(@response.body, 'DELETE', curl)
     end
 
     def create(options={})
       @options = options
       @response = Configuration.client.post "#{Configuration.url}/#{@name}", MultiJson.encode(options)
-    rescue Exception => error
-      false
+      @response.code == 200 ? @response : false
     ensure
       curl = %Q|curl -X POST "#{Configuration.url}/#{@name}" -d '#{MultiJson.encode(options)}'|
-      logged(error, 'CREATE', curl)
+      logged(@response.body, 'CREATE', curl)
     end
 
     def mapping
@@ -83,16 +77,17 @@ def bulk_store documents
       count = 0
 
       begin
-        Configuration.client.post("#{Configuration.url}/_bulk", payload.join("\n"))
+        response = Configuration.client.post("#{Configuration.url}/_bulk", payload.join("\n"))
+        raise response.body if response.code != 200
+        response
       rescue Exception => error
         if count < tries
           count += 1
-          STDERR.puts "[ERROR] #{error.message}:#{error.http_body rescue nil}, retrying (#{count})..."
+          STDERR.puts "[ERROR] #{error.message}, retrying (#{count})..."
           retry
         else
           STDERR.puts "[ERROR] Too many exceptions occured, giving up..."
-          STDERR.puts "Response: #{error.http_body rescue nil}"
-          raise
+          STDERR.puts "Response: #{error.message}"
         end
       ensure
         curl = %Q|curl -X POST "#{Configuration.url}/_bulk" -d '{... data omitted ...}'|
@@ -135,7 +130,7 @@ def remove(*args)
       raise ArgumentError, "Please pass a document ID" unless id
 
       result = Configuration.client.delete "#{Configuration.url}/#{@name}/#{type}/#{id}"
-      MultiJson.decode(result) if result
+      MultiJson.decode(result.body) if result.code == 200
     end
 
     def retrieve(type, id)

lib/tire/search.rb

@@ -68,14 +68,15 @@ def fields(*fields)
 
       def perform
         @response = Configuration.client.get(@url, self.to_json)
+        if(@response.code != 200)
+          STDERR.puts "[REQUEST FAILED] #{self.to_curl}\n"
+          return false
+        end
         @json     = MultiJson.decode(@response.body)
         @results  = Results::Collection.new(@json, @options)
-        self
-      rescue Exception => error
-        STDERR.puts "[REQUEST FAILED] #{self.to_curl}\n"
-        raise
+        return self
       ensure
-        logged(error)
+        logged(@response.body)
       end
 
       def to_curl

test/integration/index_store_test.rb

@@ -7,10 +7,7 @@ class IndexStoreIntegrationTest < Test::Unit::TestCase
 
     context "Storing the documents in index" do
 
-      teardown { Tire.index('articles-test-ids').delete }
-
-      should "store hashes under their IDs" do
-
+      setup do
         Tire.index 'articles-test-ids' do
           delete
           create
@@ -23,7 +20,16 @@ class IndexStoreIntegrationTest < Test::Unit::TestCase
 
           refresh
         end
+      end
 
+      teardown { Tire.index('articles-test-ids').delete }
+
+      should "exists if looked up by the index" do
+        assert Tire.index("articles-test-ids").exists?
+        assert !Tire.index("invalid-index").exists?
+      end
+
+      should "store hashes under their IDs" do
         s = Tire.search('articles-test-ids') { query { string '*' } }
 
         assert_equal 4, s.results.count

test/integration/percolator_test.rb

@@ -43,9 +43,7 @@ class PercolatorIntegrationTest < Test::Unit::TestCase
           assert @index.unregister_percolator_query('alert')
           Tire.index('_percolator').refresh
 
-          assert_raise(RestClient::ResourceNotFound) do
-            Configuration.client.get("#{Configuration.url}/_percolator/percolator-test/alert")
-          end
+          assert Configuration.client.get("#{Configuration.url}/_percolator/percolator-test/alert").code != 200
         end
 
       end

test/unit/index_test.rb

@@ -20,7 +20,7 @@ class IndexTest < Test::Unit::TestCase
       end
 
       should "return false when does not exist" do
-        Configuration.client.expects(:head).raises(RestClient::ResourceNotFound)
+        Configuration.client.expects(:head).returns(mock_response('', 404))
         assert ! @index.exists?
       end
 
@@ -30,7 +30,7 @@ class IndexTest < Test::Unit::TestCase
       end
 
       should "not raise exception and just return false when trying to create existing index" do
-        Configuration.client.expects(:post).raises(RestClient::BadRequest)
+        Configuration.client.expects(:post).returns(mock_response('{"error":"IndexAlreadyExistsException[\'dummy\']"}', 400))
         assert_nothing_raised { assert ! @index.create }
       end
 
@@ -40,9 +40,7 @@ class IndexTest < Test::Unit::TestCase
       end
 
       should "not raise exception and just return false when deleting non-existing index" do
-        Configuration.client.expects(:delete).returns(mock_response('{"error":"[articles] missing"}'))
-        assert_nothing_raised { assert ! @index.delete }
-        Configuration.client.expects(:delete).raises(RestClient::BadRequest)
+        Configuration.client.expects(:delete).returns(mock_response('{"error":"[articles] missing"}', 404))
         assert_nothing_raised { assert ! @index.delete }
       end
 
@@ -229,33 +227,33 @@ class MyDocument;end; document = MyDocument.new
 
         should "get type from document" do
           Configuration.client.expects(:delete).with("#{Configuration.url}/dummy/article/1").
-                                                returns('{"ok":true,"_id":"1"}').twice
+                                                returns(mock_response('{"ok":true,"_id":"1"}')).twice
           @index.remove :id => 1, :type => 'article', :title => 'Test'
           @index.remove :id => 1, :type => 'article', :title => 'Test'
         end
 
         should "set default type" do
           Configuration.client.expects(:delete).with("#{Configuration.url}/dummy/document/1").
-                                                returns('{"ok":true,"_id":"1"}')
+                                                returns(mock_response('{"ok":true,"_id":"1"}'))
           @index.remove :id => 1, :title => 'Test'
         end
 
         should "get ID from hash" do
           Configuration.client.expects(:delete).with("#{Configuration.url}/dummy/document/1").
-                                                returns('{"ok":true,"_id":"1"}')
+                                                returns(mock_response('{"ok":true,"_id":"1"}'))
           @index.remove :id => 1
         end
 
         should "get ID from method" do
           document = stub('document', :id => 1)
           Configuration.client.expects(:delete).with("#{Configuration.url}/dummy/document/1").
-                                                returns('{"ok":true,"_id":"1"}')
+                                                returns(mock_response('{"ok":true,"_id":"1"}'))
           @index.remove document
         end
 
         should "get type and ID from arguments" do
           Configuration.client.expects(:delete).with("#{Configuration.url}/dummy/article/1").
-                                                returns('{"ok":true,"_id":"1"}')
+                                                returns(mock_response('{"ok":true,"_id":"1"}'))
           @index.remove :article, 1
         end
 
@@ -288,7 +286,7 @@ class MyDocument;end; document = MyDocument.new
             json =~ /"id":"2"/ &&
             json =~ /"title":"One"/ &&
             json =~ /"title":"Two"/
-          end.returns('{}')
+          end.returns(mock_response('{}'), 200)
 
           @index.bulk_store [ {:id => '1', :title => 'One'}, {:id => '2', :title => 'Two'} ]
 
@@ -305,7 +303,7 @@ class MyDocument;end; document = MyDocument.new
             json =~ /"id":"2"/ &&
             json =~ /"title":"One"/ &&
             json =~ /"title":"Two"/
-          end.returns('{}')
+          end.returns(mock_response('{}', 200))
 
           one = ActiveModelArticle.new 'title' => 'One'; one.id = '1'
           two = ActiveModelArticle.new 'title' => 'Two'; two.id = '2'
@@ -315,15 +313,13 @@ class MyDocument;end; document = MyDocument.new
         end
 
         should "try again when an exception occurs" do
-          Configuration.client.expects(:post).raises(RestClient::RequestFailed).at_least(2)
+          Configuration.client.expects(:post).returns(mock_response('', 404)).at_least(2)
 
-          assert_raise(RestClient::RequestFailed) do
-            @index.bulk_store [ {:id => '1', :title => 'One'}, {:id => '2', :title => 'Two'} ]
-          end
+          assert !@index.bulk_store([ {:id => '1', :title => 'One'}, {:id => '2', :title => 'Two'} ])
         end
 
         should "display error message when collection item does not have ID" do
-          Configuration.client.expects(:post).with { |url, json| url  == "#{Configuration.url}/_bulk" }
+          Configuration.client.expects(:post).with{ |url, json| url  == "#{Configuration.url}/_bulk" }.returns(mock_response('success', 200))
           STDERR.expects(:puts).once
 
           documents = [ { :title => 'Bogus' }, { :title => 'Real', :id => 1 } ]

test/unit/model_persistence_test.rb

@@ -411,7 +411,8 @@ class << a
                                  end.returns(mock_response('{"ok":true,"_id":"123"}'))
 
             Configuration.client.expects(:delete).
-                                 with("#{Configuration.url}/persistent_articles/persistent_article/123")
+                                 with("#{Configuration.url}/persistent_articles/persistent_article/123").
+                                 returns(mock_response('{"ok":true,"acknowledged":true}', 200))
 
             article = PersistentArticle.new :id => '123', :title => 'Test'
             article.save

test/unit/search_test.rb

@@ -110,12 +110,12 @@ def foo; 'bar'; end
         assert_not_nil s.response
       end
 
-      should "print debugging information on exception and re-raise it" do
-        Configuration.client.expects(:get).raises(RestClient::InternalServerError)
+      should "print debugging information on exception and retun false" do
+        Configuration.client.expects(:get).returns(mock_response('failed', '404'))
         STDERR.expects(:puts)
 
         s = Search::Search.new('index')
-        assert_raise(RestClient::InternalServerError) { s.perform }
+        assert !s.perform
       end
 
       should "log request, but not response, when logger is set" do

test/unit/tire_http_response_test.rb

@@ -0,0 +1,19 @@
+require 'test_helper'
+
+module Tire
+  module HTTP
+    class ResponseTest < Test::Unit::TestCase
+      context "construction" do
+        should "take response body, code and headers" do
+          response = Response.new "http response body",
+                                  200,
+                                  :content_length => 20,
+                                  :content_encoding => 'gzip'
+
+          assert_equal "http response body", response.body
+          assert_equal 200, response.code
+        end
+      end
+    end
+  end
+end