Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config_param options to specify deprecated/obsoleted parameters #1186

Merged
merged 7 commits into from
Aug 29, 2016
Prev Previous commit
Next Next commit
add validators for config_param options, and its tests
  • Loading branch information
tagomoris committed Aug 29, 2016
commit dd8e8091922969c58ffac7612d118a8ac268348f
17 changes: 17 additions & 0 deletions lib/fluent/config/configure_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ def overwrite_defaults(other) # other is owner plugin's corresponding proxy
end
end

def option_value_type!(name, opts, key, klass)
if opts.has_key?(key) && !opts[key].is_a?(klass)
raise ArgumentError, "#{name}: #{key} must be a #{klass}, but #{opts[key].class}"
end
end

def parameter_configuration(name, type = nil, **kwargs, &block)
name = name.to_sym

Expand All @@ -227,6 +233,17 @@ def parameter_configuration(name, type = nil, **kwargs, &block)
raise ArgumentError, "#{name}: unknown config_argument type `#{type}'"
end

option_value_type!(name, opts, :desc, String)
option_value_type!(name, opts, :alias, Symbol)
option_value_type!(name, opts, :deprecated, String)
option_value_type!(name, opts, :obsoleted, String)
if type == :enum
if !opts.has_key?(:list) || !opts[:list].all?{|v| v.is_a?(Symbol) }
raise ArgumentError, "#{name}: enum parameter requires :list of Symbols"
end
end
option_value_type!(name, opts, :value_type, Symbol) # hash, array

if opts.has_key?(:default)
config_set_default(name, opts[:default])
end
Expand Down
68 changes: 58 additions & 10 deletions test/config/test_configurable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,8 @@ class BufferChild
class UnRecommended
include Fluent::Configurable
attr_accessor :log
config_param :key1, :string, default: 'deprecated', deprecated: true
config_param :key2, :string, default: 'obsoleted', obsoleted: true
config_param :key1, :string, default: 'deprecated', deprecated: "key1 will be removed."
config_param :key2, :string, default: 'obsoleted', obsoleted: "key2 has been removed."
end
end

Expand Down Expand Up @@ -1108,17 +1108,65 @@ def assert_secret_param(key, value)
end
end
end
# class UnRecommended
# include Fluent::Configurable
# config_param :key1, :string, default: 'deprecated', deprecated: true
# config_param :key2, :string, default: 'obsoleted', obsoleted: true
# end
sub_test_case 'non-required options for config_param' do
test 'desc must be a string if specified' do
assert_raise ArgumentError.new("key: desc must be a String, but Symbol") do
class InvalidDescClass
include Fluent::Configurable
config_param :key, :string, default: '', desc: :invalid_description
end
end
end
test 'alias must be a symbol if specified' do
assert_raise ArgumentError.new("key: alias must be a Symbol, but String") do
class InvalidAliasClass
include Fluent::Configurable
config_param :key, :string, default: '', alias: 'yay'
end
end
end
test 'deprecated must be a string if specified' do
assert_raise ArgumentError.new("key: deprecated must be a String, but TrueClass") do
class InvalidDeprecatedClass
include Fluent::Configurable
config_param :key, :string, default: '', deprecated: true
end
end
end
test 'obsoleted must be a string if specified' do
assert_raise ArgumentError.new("key: obsoleted must be a String, but TrueClass") do
class InvalidObsoletedClass
include Fluent::Configurable
config_param :key, :string, default: '', obsoleted: true
end
end
end
test 'value_type for hash must be a symbol' do
assert_raise ArgumentError.new("key: value_type must be a Symbol, but String") do
class InvalidValueTypeOfHashClass
include Fluent::Configurable
config_param :key, :hash, value_type: 'yay'
end
end
end
test 'value_type for array must be a symbol' do
assert_raise ArgumentError.new("key: value_type must be a Symbol, but String") do
class InvalidValueTypeOfArrayClass
include Fluent::Configurable
config_param :key, :array, value_type: 'yay'
end
end
end
end
sub_test_case 'enum parameters' do
test 'list must be specified as an array of symbols'
end
sub_test_case 'deprecated/obsoleted parameters' do
test 'both cannot be specified at once' do
assert_raise ArgumentError.new("param1: both of deprecated and obsoleted cannot be specified at once") do
class Buggy1
include Fluent::Configurable
config_param :param1, :string, default: '', deprecated: true, obsoleted: true
config_param :param1, :string, default: '', deprecated: 'yay', obsoleted: 'foo!'
end
end
end
Expand All @@ -1130,14 +1178,14 @@ class Buggy1

assert_equal 'yay', obj.key1
first_log = obj.log.logs.first
assert{ first_log && first_log.include?("[warn]") && first_log.include?("'key1' paramenter isn't recommended to use now.") }
assert{ first_log && first_log.include?("[warn]") && first_log.include?("'key1' parameter is deprecated: key1 will be removed.") }
end

test 'error raised if obsoleted parameter is configured' do
obj = ConfigurableSpec::UnRecommended.new
obj.log = Fluent::Test::TestLogger.new

assert_raise Fluent::ObsoletedParameterError.new("'key2' parameter is already removed. Don't use it.") do
assert_raise Fluent::ObsoletedParameterError.new("'key2' parameter is already removed: key2 has been removed.") do
obj.configure(config_element('ROOT', '', {'key2' => 'yay'}, []))
end
end
Expand Down
10 changes: 7 additions & 3 deletions test/config/test_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ class TestConfigTypes < ::Test::Unit::TestCase
assert_equal :val, Config::ENUM_TYPE.call('val', {list: [:val, :value, :v]})
assert_equal :v, Config::ENUM_TYPE.call('v', {list: [:val, :value, :v]})
assert_equal :value, Config::ENUM_TYPE.call('value', {list: [:val, :value, :v]})
assert_raises(Fluent::ConfigError){ Config::ENUM_TYPE.call('x', {list: [:val, :value, :v]}) }
assert_raises(RuntimeError){ Config::ENUM_TYPE.call('val', {}) }
assert_raises(RuntimeError){ Config::ENUM_TYPE.call('val', {list: ["val", "value", "v"]}) }
assert_raises(Fluent::ConfigError.new("valid options are val,value,v but got x")){ Config::ENUM_TYPE.call('x', {list: [:val, :value, :v]}) }
assert_raises(RuntimeError.new("Plugin BUG: config type 'enum' requires :list of symbols")){ Config::ENUM_TYPE.call('val', {}) }
assert_raises(RuntimeError.new("Plugin BUG: config type 'enum' requires :list of symbols")){ Config::ENUM_TYPE.call('val', {list: ["val", "value", "v"]}) }
end

test 'integer' do
Expand Down Expand Up @@ -138,6 +138,8 @@ class TestConfigTypes < ::Test::Unit::TestCase

assert_equal({"x"=>1,"y"=>60,"z"=>3600}, Config::HASH_TYPE.call('{"x":"1s","y":"1m","z":"1h"}', {value_type: :time}))
assert_equal({"x"=>1,"y"=>60,"z"=>3600}, Config::HASH_TYPE.call('x:1s,y:1m,z:1h', {value_type: :time}))

assert_raise(RuntimeError.new("unknown type in REFORMAT: foo")){ Config::HASH_TYPE.call("x:1,y:2", {value_type: :foo}) }
end

test 'array' do
Expand All @@ -162,6 +164,8 @@ class TestConfigTypes < ::Test::Unit::TestCase
}
assert_equal(["1","2"], Config::ARRAY_TYPE.call('["1","2"]', array_options))
assert_equal(["3"], Config::ARRAY_TYPE.call('["3"]', array_options))

assert_raise(RuntimeError.new("unknown type in REFORMAT: foo")){ Config::ARRAY_TYPE.call("1,2", {value_type: :foo}) }
end
end
end