Add new :list property to configuration parameters.

If set to try this will allow the user to specify one or more values.
This generally replaces the :array type, which had fewer type checks.

The array type is still needed for lists of complex objects, e.g. hashes.

Fixes #5453

docs/static/configuration.asciidoc

@@ -83,25 +83,38 @@ The settings you can configure vary according to the plugin type. For informatio
 === Value Types
 
 A plugin can require that the value for a setting be a
-certain type, such as boolean or hash. The following value
+certain type, such as boolean, list, or hash. The following value
 types are supported.
 
 [[array]]
-[float]
 ==== Array
 
-An array can be a single string value or multiple values. If you specify the same
-setting multiple times, it appends to the array.
+This type is now mostly deprecated in favor of using a standard type like `string` with the plugin defining the `:list => true` property for better type checking. It is still needed to handle lists of hashes or mixed types where type checking is not desired.
+
+Example:
+
+[source,js]
+----------------------------------
+  users => [ {id => 1, name => bob}, {id => 2, name => jane} ]
+----------------------------------
+
+[[list]]
+[float]
+==== Lists
+
+Not a type in and of itself, but a property types can have.
+This makes it possible to type check multiple values.
+Plugin authors can enable list checking by specifying `:list => true` when declaring an argument.
 
 Example:
 
 [source,js]
 ----------------------------------
   path => [ "/var/log/messages", "/var/log/*.log" ]
-  path => "/data/mysql/mysql.log"
+  uris => [ "http://elastic.co", "http://example.net" ]
 ----------------------------------
 
-This example configures `path` to be an array that contains an element for each of the three strings.
+This example configures `path`, which is a `string` to be a list that contains an element for each of the three strings. It also will configure the `uris` parameter to be a list of URIs, failing if any of the URIs provided are not valid.
 
 
 [[boolean]]

docs/static/include/pluginbody.asciidoc

@@ -474,13 +474,14 @@ There are several configuration attributes:
 
 * `:validate` - allows you to enforce passing a particular data type to Logstash
 for this configuration option, such as `:string`, `:password`, `:boolean`,
-`:number`, `:array`, `:hash`, `:path` (a file-system path), `:codec` (since
+`:number`, `:array`, `:hash`, `:path` (a file-system path), `uri`, `:codec` (since
 1.2.0), `:bytes` (starting in 1.5.0).  Note that this also works as a coercion
 in that if I specify "true" for boolean (even though technically a string), it
 will become a valid boolean in the config.  This coercion works for the
 `:number` type as well where "1.2" becomes a float and "22" is an integer.
 * `:default` - lets you specify a default value for a parameter
 * `:required` - whether or not this parameter is mandatory (a Boolean `true` or
+* `:list` - whether or not this value should be a list of values. Will typecheck the list members, and convert scalars to one element lists. Note that this mostly obviates the array type, though if you need lists of complex objects that will be more suitable.
 `false`)
 * `:deprecated` - informational (also a Boolean `true` or `false`)
 * `:obsolete` - used to declare that a given setting has been removed and is no longer functioning. The idea is to provide an informed upgrade path to users who are still using a now-removed setting.

logstash-core/lib/logstash/config/mixin.rb

@@ -328,58 +328,84 @@ module LogStash::Config::Mixin
       return true
     end # def validate_check_invalid_parameter_names
 
+    def validate_check_required_parameter(config_key, config_opts, k, v)
+      if config_key.is_a?(Regexp)
+        (k =~ config_key && v)
+      elsif config_key.is_a?(String)
+        k && v
+      end
+    end
+
     def validate_check_required_parameter_names(params)
       is_valid = true
 
       @config.each do |config_key, config|
         next unless config[:required]
 
-        if config_key.is_a?(Regexp)
-          next if params.keys.select { |k| k =~ config_key }.length > 0
-        elsif config_key.is_a?(String)
-          next if params.keys.member?(config_key)
-        end
-        @logger.error(I18n.t("logstash.runner.configuration.setting_missing",
-                             :setting => config_key, :plugin => @plugin_name,
-                             :type => @plugin_type))
-        is_valid = false
+        value = params[config_key]
+        if value.nil? || (config[:list] && Array(value).empty?)
+          @logger.error(I18n.t("logstash.runner.configuration.setting_missing",
+                               :setting => config_key, :plugin => @plugin_name,
+                               :type => @plugin_type))
+          is_valid = false
+        end        
       end
 
       return is_valid
     end
 
+    def process_parameter_value(value, config_settings)
+      config_val = config_settings[:validate]
+      
+      if config_settings[:list]
+        value = Array(value) # coerce scalars to lists
+        # Empty lists are converted to nils
+        return true, nil if value.empty?
+          
+        validated_items = value.map {|v| validate_value(v, config_val)}
+        is_valid = validated_items.all? {|sr| sr[0] }
+        processed_value = validated_items.map {|sr| sr[1]}
+      else
+        is_valid, processed_value = validate_value(value, config_val)
+      end
+      
+      return [is_valid, processed_value]
+    end
+
     def validate_check_parameter_values(params)
       # Filter out parametrs that match regexp keys.
       # These are defined in plugins like this:
       #   config /foo.*/ => ... 
-      is_valid = true
+      all_params_valid = true
 
       params.each do |key, value|
         @config.keys.each do |config_key|
           next unless (config_key.is_a?(Regexp) && key =~ config_key) \
                       || (config_key.is_a?(String) && key == config_key)
-          config_val = @config[config_key][:validate]
-          #puts "  Key matches."
-          success, result = validate_value(value, config_val)
-          if success 
-            # Accept coerced value if success
+
+          config_settings = @config[config_key]          
+
+          is_valid, processed_value = process_parameter_value(value, config_settings)
+          
+          if is_valid
+            # Accept coerced value if valid
             # Used for converting values in the config to proper objects.
-            params[key] = result if !result.nil?
+            params[key] = processed_value
           else
             @logger.error(I18n.t("logstash.runner.configuration.setting_invalid",
                                  :plugin => @plugin_name, :type => @plugin_type,
                                  :setting => key, :value => value.inspect,
-                                 :value_type => config_val,
-                                 :note => result))
+                                 :value_type => config_settings[:validate],
+                                 :note => processed_value))
           end
-          #puts "Result: #{key} / #{result.inspect} / #{success}"
-          is_valid &&= success
+          
+          all_params_valid &&= is_valid
 
           break # done with this param key
         end # config.each
       end # params.each
 
-      return is_valid
+      return all_params_valid
     end # def validate_check_parameter_values
 
     def validator_find(key)
@@ -555,12 +581,9 @@ module LogStash::Config::Mixin
 
     def secure_params!(params)
       params.each do |key, value|
-        if @config[key][:validate] == :password && !value.is_a?(::LogStash::Util::Password)
-          params[key] = ::LogStash::Util::Password.new(value)
-        end
-
-        if @config[key][:validate] == :uri && !value.is_a?(::LogStash::Util::SafeURI)
-          params[key] = ::LogStash::Util::SafeURI.new(value)
+        if [:uri, :password].include? @config[key][:validate]
+          is_valid, processed_value = process_parameter_value(value, @config[key])
+          params[key] = processed_value
         end
       end
     end

logstash-core/lib/logstash/util/safe_uri.rb

@@ -40,5 +40,9 @@ class LogStash::Util::SafeURI
     safe.password = PASS_PLACEHOLDER
     safe
   end
+
+  def ==(other)
+    other.is_a?(::LogStash::Util::SafeURI) ? @uri == other.uri : false
+  end
 end
 

logstash-core/spec/logstash/config/mixin_spec.rb

@@ -68,6 +68,74 @@ describe LogStash::Config::Mixin do
     end
   end
 
+  context "when validating lists of items" do
+    let(:klass) do
+      Class.new(LogStash::Filters::Base)  do
+        config_name "multiuri"
+        config :uris, :validate => :uri, :list => true
+        config :strings, :validate => :string, :list => true
+        config :required_strings, :validate => :string, :list => true, :required => true
+      end
+    end
+
+    let(:uris) { ["http://example.net/1", "http://example.net/2"] }
+    let(:safe_uris) { uris.map {|str| ::LogStash::Util::SafeURI.new(str) } }
+    let(:strings) { ["I am a", "modern major general"] }
+    let(:required_strings) { ["required", "strings"] }
+
+    subject { klass.new("uris" => uris, "strings" => strings, "required_strings" => required_strings) }
+
+    it "a URI list should return an array of URIs" do
+      expect(subject.uris).to match_array(safe_uris)
+    end
+
+    it "a string list should return an array of strings" do
+      expect(subject.strings).to match_array(strings)
+    end
+
+    context "with a scalar value" do
+      let(:strings) { "foo" }
+
+      it "should return the scalar value as a single element array" do
+        expect(subject.strings).to match_array([strings])
+      end
+    end
+
+    context "with an empty list" do
+      let(:strings) { [] }
+
+      it "should return nil" do
+        expect(subject.strings).to be_nil
+      end
+    end
+
+    describe "with required => true" do
+      context "and a single element" do
+        let(:required_strings) { ["foo"] }
+
+        it "should return the single value" do
+          expect(subject.required_strings).to eql(required_strings)
+        end
+      end
+
+      context "with an empty list" do
+        let (:required_strings) { [] }
+
+        it "should raise a configuration error" do
+          expect { subject.required_strings }.to raise_error(LogStash::ConfigurationError)
+        end        
+      end
+
+      context "with no value specified" do
+        let (:required_strings) { nil }
+
+        it "should raise a configuration error" do
+          expect { subject.required_strings }.to raise_error(LogStash::ConfigurationError)
+        end
+      end          
+    end
+  end
+
   context "when validating :password" do
     let(:klass) do
       Class.new(LogStash::Filters::Base)  do