Update action without document_id test case : - stop raising error wi…

…thin bulk and let elasticsearch return status (400 => no retry) - add check for configuration file error (document_id absent or empty) and raise error. Adapt test case for it.

Fixes elastic#235

lib/logstash/outputs/elasticsearch/http_client.rb

@@ -149,36 +149,33 @@ def template_put(name, template)
 
     # Build a bulk item for an elasticsearch update action
     def update_action_builder(args, source)
-      if args[:_id]
-        if args[:_script]
-          # Use the event as a hash from your script with variable name defined by script_var_name (default: "event")
-          # Ex: event["@timestamp"]
-          source = { 'script' => {'params' => { @options[:script_var_name] => source }} }
-          if @options[:scripted_upsert]
-            source['scripted_upsert'] = true
-            source['upsert'] = {}
-          else
-            source['upsert'] = args.delete(:_upsert) if args[:_upsert]
-          end    
-          case @options[:script_type]
-          when "indexed"
-            source['script']['id'] = args.delete(:_script)
-          when "file"               
-            source['script']['file'] = args.delete(:_script)
-          when "inline"
-            source['script']['inline'] = args.delete(:_script)
-          end
-          source['script']['lang'] = @options[:script_lang] if @options[:script_lang] != ''
+      if args[:_script]
+        # Use the event as a hash from your script with variable name defined
+        # by script_var_name (default: "event")
+        # Ex: event["@timestamp"]
+        source = { 'script' => {'params' => { @options[:script_var_name] => source }} }
+        if @options[:scripted_upsert]
+          source['scripted_upsert'] = true
+          source['upsert'] = {}
         else
-          source = { 'doc' => source }
-          if @options[:doc_as_upsert]
-            source['doc_as_upsert'] = true
-          else
-            source['upsert'] = args.delete(:_upsert) if args[:_upsert]
-          end
+          source['upsert'] = args.delete(:_upsert) if args[:_upsert]
+        end
+        case @options[:script_type]
+        when 'indexed'
+          source['script']['id'] = args.delete(:_script)
+        when 'file'
+          source['script']['file'] = args.delete(:_script)
+        when 'inline'
+          source['script']['inline'] = args.delete(:_script)
         end
+        source['script']['lang'] = @options[:script_lang] if @options[:script_lang] != ''
       else
-        raise(LogStash::ConfigurationError, "Specifying action => 'update' without a document '_id' is not supported.")
+        source = { 'doc' => source }
+        if @options[:doc_as_upsert]
+          source['doc_as_upsert'] = true
+        else
+          source['upsert'] = args.delete(:_upsert) if args[:_upsert]
+        end
       end
       [args, source]
     end

lib/logstash/outputs/elasticsearch/http_client_builder.rb

@@ -22,6 +22,11 @@ def self.build(logger, hosts, params)
         "doc_as_upsert and scripted_upsert are mutually exclusive."
       ) if params["doc_as_upsert"] and params["scripted_upsert"]
 
+      raise(
+        LogStash::ConfigurationError,
+        "Specifying action => 'update' needs a document_id."
+      ) if params['action'] == 'update' and params.fetch('document_id', '') == ''
+
       # Update API setup
       update_options = {
         :doc_as_upsert => params["doc_as_upsert"],

spec/integration/outputs/update_spec.rb

@@ -32,11 +32,8 @@ def get_es_output( options={} )
   end
 
   it "should fail without a document_id" do
-    subject = get_es_output()
-    subject.register
-    subject.receive(LogStash::Event.new("somevalue" => 100))
-    expect(subject.logger).to receive(:warn).at_least(1).with(anything,hash_including(:class => 'LogStash::ConfigurationError'))
-    subject.flush
+    subject = get_es_output
+    expect { subject.register }.to raise_error(LogStash::ConfigurationError)
   end
 
   context "when update only" do