Skip to content
This repository
Browse code

[#366] Cleaned up the Update API support and added an integration tes…

…t suite

This patch expands karmi/retire@f620b36.

* Polished the `curl` output from the API call
* Added the ability to pass URL parameters (timeout, refresh, etc)
* Cleaned up the unit tests
* Added a separate integration test for resilience and documentation

Example usage
-------------

Update a counter:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ruby
  Tire.index('articles-with-tags') { update( 'article', '1', :script => "ctx._source.views += 1" ) and refresh }
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Add a tag to the document:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ruby
    Tire.index('articles-with-tags') do
      update 'article', '1', :script => "ctx._source.tags += tag",
                             :params => { :tag => 'new-tag' }
      refresh
    end
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Set a title of the document:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ruby
  @title = 'Foo Is Now Bar!'

  Tire.index('articles-with-tags') do |index|

    index.update 'article', '1', :script => "ctx._source.title = title",
                                 :params => { :title => @title }
    index.refresh
  end
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

See <http://www.elasticsearch.org/guide/reference/api/update.html> for more documentation.

---

Closes #366.
  • Loading branch information...
commit ff00015244230f796304c39fff3985804af9b636 1 parent f620b36
Karel Minarik authored June 24, 2012
29  lib/tire/index.rb
@@ -198,6 +198,22 @@ def retrieve(type, id)
198 198
       logged(id, curl)
199 199
     end
200 200
 
  201
+    def update(type, id, payload={}, options={})
  202
+      raise ArgumentError, "Please pass a document type" unless type
  203
+      raise ArgumentError, "Please pass a document ID"   unless id
  204
+      raise ArgumentError, "Please pass a script in the payload hash" unless payload[:script]
  205
+
  206
+      type      = Utils.escape(type)
  207
+      url       = "#{self.url}/#{type}/#{id}/_update"
  208
+      url      += "?#{options.to_param}" unless options.keys.empty?
  209
+      @response = Configuration.client.post url, MultiJson.encode(payload)
  210
+      MultiJson.decode(@response.body)
  211
+
  212
+    ensure
  213
+      curl = %Q|curl -X POST "#{url}" -d '#{MultiJson.encode(payload)}'|
  214
+      logged(id, curl)
  215
+    end
  216
+
201 217
     def refresh
202 218
       @response = Configuration.client.post "#{url}/_refresh", ''
203 219
 
@@ -275,19 +291,6 @@ def percolate(*args, &block)
275 291
       logged('_percolate', curl)
276 292
     end
277 293
 
278  
-    def update(type, id, options)
279  
-      raise ArgumentError, "Please pass a document ID" unless id
280  
-      raise ArgumentError, "Missing script in options hash" unless options[:script]
281  
-
282  
-      type      = Utils.escape(type)
283  
-      url       = "#{self.url}/#{type}/#{id}/_update"
284  
-      @response = Configuration.client.post url, MultiJson.encode(options)
285  
-      MultiJson.decode(@response.body)['ok']
286  
-    ensure
287  
-      curl = %Q|curl -X POST "#{url}"|
288  
-      logged(id, curl)
289  
-    end
290  
-
291 294
     def logged(endpoint='/', curl='')
292 295
       if Configuration.logger
293 296
         error = $!
2  lib/tire/logger.rb
@@ -9,7 +9,7 @@ def initialize(device, options={})
9 9
       end
10 10
       @device.sync = true if @device.respond_to?(:sync)
11 11
       @options = options
12  
-      at_exit { @device.close unless @device.closed? } if @device.respond_to?(:closed?) && @device.respond_to?(:close)
  12
+      # at_exit { @device.close unless @device.closed? } if @device.respond_to?(:closed?) && @device.respond_to?(:close)
13 13
     end
14 14
 
15 15
     def level
111  test/integration/index_update_document_test.rb
... ...
@@ -0,0 +1,111 @@
  1
+require 'test_helper'
  2
+
  3
+module Tire
  4
+
  5
+  class IndexUpdateDocumentIntegrationTest < Test::Unit::TestCase
  6
+    include Test::Integration
  7
+
  8
+    context "Updating a document" do
  9
+
  10
+      setup do
  11
+        Tire.index 'articles-with-tags' do
  12
+          delete
  13
+          create
  14
+
  15
+          store :type => 'article', :id => 1, :title => 'One',   :tags => ['foo'],        :views => 0
  16
+          store :type => 'article', :id => 2, :title => 'Two',   :tags => ['foo', 'bar'], :views => 10
  17
+          store :type => 'article', :id => 3, :title => 'Three', :tags => ['foobar']
  18
+
  19
+          refresh
  20
+        end
  21
+      end
  22
+
  23
+      teardown { Tire.index('articles-with-tags').delete }
  24
+
  25
+      should "increment a counter" do
  26
+        Tire.index('articles-with-tags') { update( 'article', '1', :script => "ctx._source.views += 1" ) and refresh }
  27
+
  28
+        document = Tire.search('articles-with-tags') { query { string 'title:one' } }.results.first
  29
+        assert_equal 1, document.views, document.inspect
  30
+
  31
+        Tire.index('articles-with-tags') { update( 'article', '2', :script => "ctx._source.views += 1" ) and refresh }
  32
+
  33
+        document = Tire.search('articles-with-tags') { query { string 'title:two' } }.results.first
  34
+        assert_equal 11, document.views, document.inspect
  35
+      end
  36
+
  37
+      should "add a tag to document" do
  38
+        Tire.index('articles-with-tags') do
  39
+          update 'article', '1', :script => "ctx._source.tags += tag",
  40
+                                 :params => { :tag => 'new' }
  41
+          refresh
  42
+        end
  43
+
  44
+        document = Tire.search('articles-with-tags') { query { string 'title:one' } }.results.first
  45
+        assert_equal ['foo', 'new'], document.tags, document.inspect
  46
+      end
  47
+
  48
+      should "remove a tag from document" do
  49
+        Tire.index('articles-with-tags') do
  50
+          update 'article', '1', :script => "ctx._source.tags = tags",
  51
+                                 :params => { :tags => [] }
  52
+          refresh
  53
+        end
  54
+
  55
+        document = Tire.index('articles-with-tags').retrieve 'article', '1'
  56
+        assert_equal [], document.tags, document.inspect
  57
+      end
  58
+
  59
+      should "remove the entire document if specific condition is met" do
  60
+        Tire.index('articles-with-tags') do
  61
+          # Remove document when it contains tag 'foobar'
  62
+          update 'article', '3', :script => "ctx._source.tags.contains(tag) ? ctx.op = 'delete' : 'none'",
  63
+                                 :params => { :tag => 'foobar' }
  64
+          refresh
  65
+        end
  66
+
  67
+        assert_nil Tire.index('articles-with-tags').retrieve 'article', '3'
  68
+      end
  69
+
  70
+      should "pass the operation parameters to the API" do
  71
+        Tire.index('articles-with-tags').update 'article', '2', { :script => "ctx._source.tags += tag",
  72
+                                                                  :params => { :tag => 'new' }
  73
+                                                                },
  74
+                                                                {
  75
+                                                                  :refresh => true
  76
+                                                                }
  77
+
  78
+        document = Tire.search('articles-with-tags') { query { string 'title:two' } }.results.first
  79
+        assert_equal 3, document.tags.size, document.inspect
  80
+      end
  81
+
  82
+      should "access variables from the outer scope" do
  83
+        $t = self
  84
+
  85
+        class Updater
  86
+          @tags = ['foo', 'bar', 'baz']
  87
+
  88
+          def self.perform_update!
  89
+            $t.assert_not_nil @tags
  90
+
  91
+            Tire.index('articles-with-tags') do |index|
  92
+              $t.assert_not_nil @tags
  93
+
  94
+              index.update 'article', '3', :script => "ctx._source.tags = tags",
  95
+                                     :params => { :tags => @tags }
  96
+              index.refresh
  97
+            end
  98
+          end
  99
+        end
  100
+
  101
+        Updater.perform_update!
  102
+
  103
+        document = Tire.search('articles-with-tags') { query { string 'title:three' } }.results.first
  104
+        assert_equal 3, document.tags.size, document.inspect
  105
+      end
  106
+
  107
+    end
  108
+
  109
+  end
  110
+
  111
+end
61  test/unit/index_test.rb
@@ -420,6 +420,48 @@ class MyDocument;end; document = MyDocument.new
420 420
 
421 421
       end
422 422
 
  423
+      context "when updating" do
  424
+
  425
+        should "send payload" do
  426
+          Configuration.client.expects(:post).with do |url,payload|
  427
+                                payload = MultiJson.decode(payload)
  428
+                                # p [url, payload]
  429
+                                assert_equal( "#{@index.url}/document/42/_update", url ) &&
  430
+                                assert_not_nil( payload['script'] ) &&
  431
+                                assert_not_nil( payload['params'] ) &&
  432
+                                assert_equal( '21', payload['params']['bar'] )
  433
+                              end.
  434
+                              returns(
  435
+                                mock_response('{"ok":"true","_index":"dummy","_type":"document","_id":"42","_version":"2"}'))
  436
+
  437
+          assert @index.update('document', '42', {:script => "ctx._source.foo = bar;", :params => { :bar => '21' }})
  438
+        end
  439
+
  440
+        should "send options" do
  441
+          Configuration.client.expects(:post).with do |url,payload|
  442
+                                payload = MultiJson.decode(payload)
  443
+                                # p [url, payload]
  444
+                                assert_equal( "#{@index.url}/document/42/_update?timeout=1000", url ) &&
  445
+                                assert_nil( payload['timeout'] )
  446
+                              end.
  447
+                              returns(
  448
+                                mock_response('{"ok":"true","_index":"dummy","_type":"document","_id":"42","_version":"2"}'))
  449
+          assert @index.update('document', '42', {:script => "ctx._source.foo = 'bar'"}, {:timeout => 1000})
  450
+        end
  451
+
  452
+        should "raise error when no type or ID is passed" do
  453
+          assert_raise(ArgumentError) { @index.update('article', nil, :script => 'foobar') }
  454
+          assert_raise(ArgumentError) { @index.update(nil, '123', :script => 'foobar') }
  455
+        end
  456
+
  457
+        should "raise an error when no script is passed" do
  458
+          assert_raise ArgumentError do
  459
+            @index.update('article', "42", {:params => {"foo" => "bar"}})
  460
+          end
  461
+        end
  462
+
  463
+      end
  464
+
423 465
       context "when storing in bulk" do
424 466
         # The expected JSON looks like this:
425 467
         #
@@ -750,24 +792,6 @@ def self.count;          DATA.size;        end
750 792
           assert_equal ["alerts"], matches
751 793
         end
752 794
 
753  
-        should "run update script against a given document" do
754  
-          Configuration.client.expects(:post).with do |url,payload|
755  
-            payload = MultiJson.decode(payload)
756  
-            # p [url, payload]
757  
-            url == "#{@index.url}/document/42/_update" &&
758  
-            payload['script'] != nil &&
759  
-            payload['params'] != nil &&
760  
-            payload['params']['x'] == '21' &&
761  
-            payload['params']['y'] == [2,4,6]
762  
-          end.returns(mock_response('{"ok":"true","_index":"dummy","_type":"document","_id":"42","_version":"2"}'))
763  
-          assert @index.update('document', '42', {:script => "ctx._source.test = 'youpi';", :params => { :x => '21', :y => [2,4,6] }})
764  
-        end
765  
-
766  
-        should "raise error when running update without a script or an id" do
767  
-          assert_raise(ArgumentError) { @index.update('lol', "42", {:params => {"foo" => "bar"}}) }
768  
-          assert_raise(ArgumentError) { @index.update('lol', nil, {:script => "ctx._source.test = 'test';"}) }
769  
-        end
770  
-
771 795
         context "while storing document" do
772 796
 
773 797
           should "percolate document against all registered queries" do
@@ -862,6 +886,7 @@ def index_something
862 886
         end
863 887
 
864 888
       end
  889
+
865 890
     end
866 891
 
867 892
   end

0 notes on commit ff00015

Please sign in to comment.
Something went wrong with that request. Please try again.