From 8142b6f843b29c01d822d51320423db9ad47cc81 Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Fri, 25 Mar 2016 11:58:08 +0900 Subject: [PATCH 1/7] Change serialization syntax string as following In previous version: string_param1 string_value string_param2 string value In this version: string_param1 "string_value" string_param2 "string\nvalue" I don't change serialization format for other types. --- lib/fluent/config/element.rb | 24 +++++++++++++++++++++++- test/config/test_configurable.rb | 10 +++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lib/fluent/config/element.rb b/lib/fluent/config/element.rb index 6daff414c2..124e976a5c 100644 --- a/lib/fluent/config/element.rb +++ b/lib/fluent/config/element.rb @@ -111,7 +111,17 @@ def to_s(nest = 0) if secret_param?(k) out << "#{nindent}#{k} xxxxxx\n" else - out << "#{nindent}#{k} #{v}\n" + case param_type(k) + when :string + out << "#{nindent}#{k} \"#{self.class.unescape_parameter(v)}\"\n" + when :enum, :integer, :float, :size, :bool, :time + out << "#{nindent}#{k} #{v}\n" + when :hash, :array + out << "#{nindent}#{k} #{v}\n" + else + # Unknown type + out << "#{nindent}#{k} #{v}\n" + end end } @elements.each { |e| @@ -144,6 +154,18 @@ def secret_param?(key) false end + def param_type(key) + return nil if @corresponding_proxies.empty? + + param_key = key.to_sym + proxy = @corresponding_proxies.detect do |_proxy| + _proxy.params.has_key?(param_key) + end + return nil unless proxy + _block, opts = proxy.params[param_key] + opts[:type] + end + def self.unescape_parameter(v) result = '' v.each_char { |c| result << LiteralParser.unescape_char(c) } diff --git a/test/config/test_configurable.rb b/test/config/test_configurable.rb index b1a416f36f..9a43d9d689 100644 --- a/test/config/test_configurable.rb +++ b/test/config/test_configurable.rb @@ -926,7 +926,7 @@ class TestConfigurable < ::Test::Unit::TestCase test 'to_s hides secret config_param' do @conf.to_s.each_line { |line| key, value = line.strip.split(' ', 2) - assert_secret_param(key, value) + assert_secret_param(key, value, use_to_s: true) } end @@ -956,10 +956,14 @@ class TestConfigurable < ::Test::Unit::TestCase } end - def assert_secret_param(key, value) + def assert_secret_param(key, value, use_to_s: false) case key when 'normal_param', 'normal_param2' - assert_equal 'normal', value + if use_to_s + assert_equal '"normal"', value + else + assert_equal 'normal', value + end when 'secret_param', 'secret_param2' assert_equal 'xxxxxx', value end From bc020cd4882dc303540e32c1143940f6433a37a5 Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Fri, 25 Mar 2016 12:51:51 +0900 Subject: [PATCH 2/7] Extract dump_value --- lib/fluent/config/element.rb | 40 ++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/lib/fluent/config/element.rb b/lib/fluent/config/element.rb index 124e976a5c..86ccb8c426 100644 --- a/lib/fluent/config/element.rb +++ b/lib/fluent/config/element.rb @@ -108,21 +108,7 @@ def to_s(nest = 0) out << "#{indent}<#{@name} #{@arg}>\n" end each_pair { |k, v| - if secret_param?(k) - out << "#{nindent}#{k} xxxxxx\n" - else - case param_type(k) - when :string - out << "#{nindent}#{k} \"#{self.class.unescape_parameter(v)}\"\n" - when :enum, :integer, :float, :size, :bool, :time - out << "#{nindent}#{k} #{v}\n" - when :hash, :array - out << "#{nindent}#{k} #{v}\n" - else - # Unknown type - out << "#{nindent}#{k} #{v}\n" - end - end + out << dump_value(k, v, indent, nindent) } @elements.each { |e| out << e.to_s(nest + 1) @@ -166,6 +152,30 @@ def param_type(key) opts[:type] end + def dump_value(k, v, indent, nindent) + out = "" + if secret_param?(k) + out << "#{nindent}#{k} xxxxxx\n" + else + if @v1_config + case param_type(k) + when :string + out << "#{nindent}#{k} \"#{self.class.unescape_parameter(v)}\"\n" + when :enum, :integer, :float, :size, :bool, :time + out << "#{nindent}#{k} #{v}\n" + when :hash, :array + out << "#{nindent}#{k} #{v}\n" + else + # Unknown type + out << "#{nindent}#{k} #{v}\n" + end + else + out << "#{nindent}#{k} #{v}\n" + end + end + out + end + def self.unescape_parameter(v) result = '' v.each_char { |c| result << LiteralParser.unescape_char(c) } From afa27c1d06617d993e1e7de4ec048ac972192cd8 Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Fri, 25 Mar 2016 14:05:03 +0900 Subject: [PATCH 3/7] Set v1_config properly --- lib/fluent/config/element.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/fluent/config/element.rb b/lib/fluent/config/element.rb index 86ccb8c426..664451fff3 100644 --- a/lib/fluent/config/element.rb +++ b/lib/fluent/config/element.rb @@ -120,6 +120,7 @@ def to_s(nest = 0) def to_masked_element new_elems = @elements.map { |e| e.to_masked_element } new_elem = Element.new(@name, @arg, {}, new_elems, @unused) + new_elem.v1_config = @v1_config each_pair { |k, v| new_elem[k] = secret_param?(k) ? 'xxxxxx' : v } From bc4460284782fe54df69054196c589618fc4f7da Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Fri, 25 Mar 2016 14:10:04 +0900 Subject: [PATCH 4/7] Set corresponding_proxies properly --- lib/fluent/config/element.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/fluent/config/element.rb b/lib/fluent/config/element.rb index 664451fff3..8c9808bfc8 100644 --- a/lib/fluent/config/element.rb +++ b/lib/fluent/config/element.rb @@ -121,6 +121,7 @@ def to_masked_element new_elems = @elements.map { |e| e.to_masked_element } new_elem = Element.new(@name, @arg, {}, new_elems, @unused) new_elem.v1_config = @v1_config + new_elem.corresponding_proxies = @corresponding_proxies each_pair { |k, v| new_elem[k] = secret_param?(k) ? 'xxxxxx' : v } From 7a8a9877ef6e793745c910913f77ee2dbab69f76 Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Fri, 25 Mar 2016 14:20:17 +0900 Subject: [PATCH 5/7] Remove needless condition Because we use `element.v1_config == false` always. --- test/config/test_configurable.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/config/test_configurable.rb b/test/config/test_configurable.rb index 9a43d9d689..b1a416f36f 100644 --- a/test/config/test_configurable.rb +++ b/test/config/test_configurable.rb @@ -926,7 +926,7 @@ class TestConfigurable < ::Test::Unit::TestCase test 'to_s hides secret config_param' do @conf.to_s.each_line { |line| key, value = line.strip.split(' ', 2) - assert_secret_param(key, value, use_to_s: true) + assert_secret_param(key, value) } end @@ -956,14 +956,10 @@ class TestConfigurable < ::Test::Unit::TestCase } end - def assert_secret_param(key, value, use_to_s: false) + def assert_secret_param(key, value) case key when 'normal_param', 'normal_param2' - if use_to_s - assert_equal '"normal"', value - else - assert_equal 'normal', value - end + assert_equal 'normal', value when 'secret_param', 'secret_param2' assert_equal 'xxxxxx', value end From b885cead81ea1cf174c42ab58948b20c3bd975fa Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Fri, 25 Mar 2016 14:21:30 +0900 Subject: [PATCH 6/7] Add test for `Fluent::Config::Element#to_s` --- test/config/test_config_parser.rb | 82 +++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/test/config/test_config_parser.rb b/test/config/test_config_parser.rb index e1529515c4..4210d3e6ed 100644 --- a/test/config/test_config_parser.rb +++ b/test/config/test_config_parser.rb @@ -22,6 +22,20 @@ def e(name, arg='', attrs={}, elements=[]) end end + class AllTypes + include Fluent::Configurable + + config_param :param_string, :string + config_param :param_enum, :enum, list: [:foo, :bar, :baz] + config_param :param_integer, :integer + config_param :param_float, :float + config_param :param_size, :size + config_param :param_bool, :bool + config_param :param_time, :time + config_param :param_hash, :hash + config_param :param_array, :array + end + class TestV1Parser < ::Test::Unit::TestCase def read_config(path) path = File.expand_path(path) @@ -384,6 +398,74 @@ def prepare_config conf2 = parse_text(conf.to_s) # use dumpped configuration to check unescape assert_equal(expected, conf2.elements.first['k1']) end + + test 'all types' do + conf = parse_text(%[ + param_string "value" + param_enum foo + param_integer 999 + param_float 55.55 + param_size 4k + param_bool true + param_time 10m + param_hash { "key1": "value1", "key2": 2 } + param_array ["value1", "value2", 100] + ]) + target = AllTypes.new.configure(conf) + assert_equal(conf.to_s, target.config.to_s) + expected = < + param_string "value" + param_enum foo + param_integer 999 + param_float 55.55 + param_size 4k + param_bool true + param_time 10m + param_hash {"key1":"value1","key2":2} + param_array ["value1","value2",100] + +DUMP + assert_equal(expected, conf.to_s) + end + end + end + + class TestV0Parser < ::Test::Unit::TestCase + def parse_text(text) + basepath = File.expand_path(File.dirname(__FILE__) + '/../../') + Fluent::Config::Parser.parse(StringIO.new(text), '(test)', basepath) + end + + sub_test_case "Fluent::Config::Element#to_s" do + test 'all types' do + conf = parse_text(%[ + param_string value + param_enum foo + param_integer 999 + param_float 55.55 + param_size 4k + param_bool true + param_time 10m + param_hash { "key1": "value1", "key2": 2 } + param_array ["value1", "value2", 100] + ]) + target = AllTypes.new.configure(conf) + assert_equal(conf.to_s, target.config.to_s) + expected = < + param_string value + param_enum foo + param_integer 999 + param_float 55.55 + param_size 4k + param_bool true + param_time 10m + param_hash { "key1": "value1", "key2": 2 } + param_array ["value1", "value2", 100] + +DUMP + end end end end From ad8d8efe3252b2f4e0bf8713949f6aec7dc7943b Mon Sep 17 00:00:00 2001 From: Kenji Okimoto Date: Tue, 29 Mar 2016 09:23:03 +0900 Subject: [PATCH 7/7] Remove redundant assignment --- lib/fluent/config/element.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/fluent/config/element.rb b/lib/fluent/config/element.rb index 8c9808bfc8..a974e02662 100644 --- a/lib/fluent/config/element.rb +++ b/lib/fluent/config/element.rb @@ -155,27 +155,25 @@ def param_type(key) end def dump_value(k, v, indent, nindent) - out = "" if secret_param?(k) - out << "#{nindent}#{k} xxxxxx\n" + "#{nindent}#{k} xxxxxx\n" else if @v1_config case param_type(k) when :string - out << "#{nindent}#{k} \"#{self.class.unescape_parameter(v)}\"\n" + "#{nindent}#{k} \"#{self.class.unescape_parameter(v)}\"\n" when :enum, :integer, :float, :size, :bool, :time - out << "#{nindent}#{k} #{v}\n" + "#{nindent}#{k} #{v}\n" when :hash, :array - out << "#{nindent}#{k} #{v}\n" + "#{nindent}#{k} #{v}\n" else # Unknown type - out << "#{nindent}#{k} #{v}\n" + "#{nindent}#{k} #{v}\n" end else - out << "#{nindent}#{k} #{v}\n" + "#{nindent}#{k} #{v}\n" end end - out end def self.unescape_parameter(v)