From 1f30bdb9ff67d4c2345603ae95f34749037a686e Mon Sep 17 00:00:00 2001 From: TAGOMORI Satoshi Date: Wed, 2 Mar 2016 13:00:46 -0800 Subject: [PATCH 1/2] make types code testable --- lib/fluent/config/types.rb | 86 +++++++++++++++++--------------------- test/config/test_types.rb | 69 ++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 48 deletions(-) diff --git a/lib/fluent/config/types.rb b/lib/fluent/config/types.rb index 13af0b8264..44fe93f30b 100644 --- a/lib/fluent/config/types.rb +++ b/lib/fluent/config/types.rb @@ -64,54 +64,44 @@ def self.bool_value(str) nil end end - end - - Configurable.register_type(:string, Proc.new { |val, opts| - val - }) - - Configurable.register_type(:enum, Proc.new { |val, opts| - s = val.to_sym - raise "Plugin BUG: config type 'enum' requires :list argument" unless opts[:list].is_a?(Array) - unless opts[:list].include?(s) - raise ConfigError, "valid options are #{opts[:list].join(',')} but got #{val}" - end - s - }) - - Configurable.register_type(:integer, Proc.new { |val, opts| - val.to_i - }) - - Configurable.register_type(:float, Proc.new { |val, opts| - val.to_f - }) - - Configurable.register_type(:size, Proc.new { |val, opts| - Config.size_value(val) - }) - - Configurable.register_type(:bool, Proc.new { |val, opts| - Config.bool_value(val) - }) - Configurable.register_type(:time, Proc.new { |val, opts| - Config.time_value(val) - }) - - Configurable.register_type(:hash, Proc.new { |val, opts| - param = val.is_a?(String) ? JSON.load(val) : val - if param.class != Hash - raise ConfigError, "hash required but got #{val.inspect}" - end - param - }) + STRING_TYPE = Proc.new { |val, opts| val } + ENUM_TYPE = Proc.new { |val, opts| + s = val.to_sym + raise "Plugin BUG: config type 'enum' requires :list argument" unless opts[:list].is_a?(Array) + unless opts[:list].include?(s) + raise ConfigError, "valid options are #{opts[:list].join(',')} but got #{val}" + end + s + } + INTEGER_TYPE = Proc.new { |val, opts| val.to_i } + FLOAT_TYPE = Proc.new { |val, opts| val.to_f } + SIZE_TYPE = Proc.new { |val, opts| Config.size_value(val) } + BOOL_TYPE = Proc.new { |val, opts| Config.bool_value(val) } + TIME_TYPE = Proc.new { |val, opts| Config.time_value(val) } + HASH_TYPE = Proc.new { |val, opts| + param = val.is_a?(String) ? JSON.load(val) : val + if param.class != Hash + raise ConfigError, "hash required but got #{val.inspect}" + end + param + } + ARRAY_TYPE = Proc.new { |val, opts| + param = val.is_a?(String) ? JSON.load(val) : val + if param.class != Array + raise ConfigError, "array required but got #{val.inspect}" + end + param + } + end - Configurable.register_type(:array, Proc.new { |val, opts| - param = val.is_a?(String) ? JSON.load(val) : val - if param.class != Array - raise ConfigError, "array required but got #{val.inspect}" - end - param - }) + Configurable.register_type(:string, Config::STRING_TYPE) + Configurable.register_type(:enum, Config::ENUM_TYPE) + Configurable.register_type(:integer, Config::INTEGER_TYPE) + Configurable.register_type(:float, Config::FLOAT_TYPE) + Configurable.register_type(:size, Config::SIZE_TYPE) + Configurable.register_type(:bool, Config::BOOL_TYPE) + Configurable.register_type(:time, Config::TIME_TYPE) + Configurable.register_type(:hash, Config::HASH_TYPE) + Configurable.register_type(:array, Config::ARRAY_TYPE) end diff --git a/test/config/test_types.rb b/test/config/test_types.rb index be240dbf52..7bb020631f 100644 --- a/test/config/test_types.rb +++ b/test/config/test_types.rb @@ -60,4 +60,73 @@ class TestConfigTypes < ::Test::Unit::TestCase assert_nil Config.bool_value(10) end end + + sub_test_case 'type converters for config_param definitions' do + test 'string' do + assert_equal 'test', Config::STRING_TYPE.call('test', {}) + assert_equal '1', Config::STRING_TYPE.call('1', {}) + assert_equal ' ', Config::STRING_TYPE.call(' ', {}) + end + + test 'enum' do + 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', {}) } + end + + test 'integer' do + assert_equal 1, Config::INTEGER_TYPE.call('1', {}) + assert_equal 1, Config::INTEGER_TYPE.call('1.0', {}) + assert_equal 1000, Config::INTEGER_TYPE.call('1_000', {}) + assert_equal 1, Config::INTEGER_TYPE.call('1x', {}) + end + + test 'float' do + assert_equal 1.0, Config::FLOAT_TYPE.call('1', {}) + assert_equal 1.0, Config::FLOAT_TYPE.call('1.0', {}) + assert_equal 1.0, Config::FLOAT_TYPE.call('1.00', {}) + assert_equal 1.0, Config::FLOAT_TYPE.call('1e0', {}) + end + + test 'size' do + assert_equal 1000, Config::SIZE_TYPE.call('1000', {}) + assert_equal 1024, Config::SIZE_TYPE.call('1k', {}) + assert_equal 1024*1024, Config::SIZE_TYPE.call('1m', {}) + end + + test 'bool' do + assert_equal true, Config::BOOL_TYPE.call('true', {}) + assert_equal true, Config::BOOL_TYPE.call('yes', {}) + assert_equal false, Config::BOOL_TYPE.call('no', {}) + assert_equal false, Config::BOOL_TYPE.call('false', {}) + + assert_equal nil, Config::BOOL_TYPE.call('TRUE', {}) + assert_equal nil, Config::BOOL_TYPE.call('True', {}) + assert_equal nil, Config::BOOL_TYPE.call('Yes', {}) + assert_equal nil, Config::BOOL_TYPE.call('No', {}) + + assert_equal true, Config::BOOL_TYPE.call('', {}) + assert_equal nil, Config::BOOL_TYPE.call('unexpected_string', {}) + end + + test 'time' do + assert_equal 0, Config::TIME_TYPE.call('0', {}) + assert_equal 1.0, Config::TIME_TYPE.call('1', {}) + assert_equal 1.01, Config::TIME_TYPE.call('1.01', {}) + assert_equal 1, Config::TIME_TYPE.call('1s', {}) + assert_equal 60, Config::TIME_TYPE.call('1m', {}) + assert_equal 3600, Config::TIME_TYPE.call('1h', {}) + assert_equal 86400, Config::TIME_TYPE.call('1d', {}) + end + + test 'hash' do + assert_equal({"x"=>"v","k"=>1}, Config::HASH_TYPE.call('{"x":"v","k":1}', {})) + end + + test 'array' do + assert_equal(["1","2",1], Config::ARRAY_TYPE.call('["1","2",1]', {})) + end + end end From 43d081afbfe437eba6d2169454e28065ac99d18f Mon Sep 17 00:00:00 2001 From: TAGOMORI Satoshi Date: Wed, 2 Mar 2016 13:02:12 -0800 Subject: [PATCH 2/2] fix to check :list argument contains Symbol only (non-symbol members cannot be fetched) --- lib/fluent/config/types.rb | 7 ++++--- test/config/test_types.rb | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/fluent/config/types.rb b/lib/fluent/config/types.rb index 44fe93f30b..b295ef5816 100644 --- a/lib/fluent/config/types.rb +++ b/lib/fluent/config/types.rb @@ -68,9 +68,10 @@ def self.bool_value(str) STRING_TYPE = Proc.new { |val, opts| val } ENUM_TYPE = Proc.new { |val, opts| s = val.to_sym - raise "Plugin BUG: config type 'enum' requires :list argument" unless opts[:list].is_a?(Array) - unless opts[:list].include?(s) - raise ConfigError, "valid options are #{opts[:list].join(',')} but got #{val}" + list = opts[:list] + raise "Plugin BUG: config type 'enum' requires :list of symbols" unless list.is_a?(Array) && list.all?{|v| v.is_a? Symbol } + unless list.include?(s) + raise ConfigError, "valid options are #{list.join(',')} but got #{val}" end s } diff --git a/test/config/test_types.rb b/test/config/test_types.rb index 7bb020631f..95c544bc63 100644 --- a/test/config/test_types.rb +++ b/test/config/test_types.rb @@ -74,6 +74,7 @@ class TestConfigTypes < ::Test::Unit::TestCase 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"]}) } end test 'integer' do