From 67ecfe573430da46952f61dc0d6ef367c58edac0 Mon Sep 17 00:00:00 2001 From: TAGOMORI Satoshi Date: Thu, 31 Mar 2016 16:46:32 +0900 Subject: [PATCH] Add `config_section` option `init` to initialize section object if not specified: fixes #850 --- lib/fluent/config/configure_proxy.rb | 26 +++++++++++- lib/fluent/config/section.rb | 4 ++ test/config/test_configurable.rb | 19 +++++++++ test/config/test_configure_proxy.rb | 63 ++++++++++++++++++++++++++-- 4 files changed, 107 insertions(+), 5 deletions(-) diff --git a/lib/fluent/config/configure_proxy.rb b/lib/fluent/config/configure_proxy.rb index 3a41d9db61..3e23c3e8c0 100644 --- a/lib/fluent/config/configure_proxy.rb +++ b/lib/fluent/config/configure_proxy.rb @@ -19,7 +19,7 @@ module Fluent module Config class ConfigureProxy - attr_accessor :name, :final, :param_name, :required, :multi, :alias, :argument, :params, :defaults, :descriptions, :sections + attr_accessor :name, :final, :param_name, :init, :required, :multi, :alias, :argument, :params, :defaults, :descriptions, :sections # config_param :desc, :string, :default => '....' # config_set_default :buffer_type, :memory # @@ -43,10 +43,13 @@ def initialize(name, opts = {}) @final = opts[:final] @param_name = (opts[:param_name] || @name).to_sym + @init = opts[:init] @required = opts[:required] @multi = opts[:multi] @alias = opts[:alias] + raise "init and required are exclusive" if @init && @required + @argument = nil # nil: ignore argument @params = {} @defaults = {} @@ -55,6 +58,10 @@ def initialize(name, opts = {}) @current_description = nil end + def init? + @init.nil? ? false : @init + end + def required? @required.nil? ? false : @required end @@ -70,6 +77,9 @@ def final? def merge(other) # self is base class, other is subclass return merge_for_finalized(other) if self.final? + if overwrite?(other, :init) + raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: init" + end if overwrite?(other, :required) raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: required" end @@ -85,6 +95,7 @@ def merge(other) # self is base class, other is subclass # varible, so this should be able to be overwritten options[:param_name] = other.param_name # subclass cannot overwrite base class's definition + options[:init] = @init.nil? ? other.init : self.init options[:required] = @required.nil? ? other.required : self.required options[:multi] = @multi.nil? ? other.multi : self.multi options[:alias] = @alias.nil? ? other.alias : self.alias @@ -121,6 +132,9 @@ def merge_for_finalized(other) raise ConfigError, "BUG: subclass cannot overwrite finalized base class's config_section" end + if overwrite?(other, :init) + raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: init" + end if overwrite?(other, :required) raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: required" end @@ -133,6 +147,7 @@ def merge_for_finalized(other) options = {} options[:param_name] = other.param_name + options[:init] = @init.nil? ? other.init : self.init options[:required] = @required.nil? ? other.required : self.required options[:multi] = @multi.nil? ? other.multi : self.multi options[:alias] = @alias.nil? ? other.alias : self.alias @@ -261,6 +276,15 @@ def config_section(name, *args, &block) sub_proxy = ConfigureProxy.new(name, *args) sub_proxy.instance_exec(&block) + if sub_proxy.init? + if sub_proxy.argument && !sub_proxy.defaults.has_key?(sub_proxy.argument.first) + raise ArgumentError, "#{self.name}: init is specified, but default value of argument is missing" + end + if sub_proxy.params.keys.any?{|param_name| !sub_proxy.defaults.has_key?(param_name)} + raise ArgumentError, "#{self.name}: init is specified, but there're parameters without default values" + end + end + @params.delete(name) @sections[name] = sub_proxy diff --git a/lib/fluent/config/section.rb b/lib/fluent/config/section.rb index b1706e92d7..89b1274fbe 100644 --- a/lib/fluent/config/section.rb +++ b/lib/fluent/config/section.rb @@ -141,6 +141,10 @@ def self.generate(proxy, conf, logger, plugin_class, stack = []) proxy.sections.each do |name, subproxy| varname = subproxy.param_name.to_sym elements = (conf.respond_to?(:elements) ? conf.elements : []).select{ |e| e.name == subproxy.name.to_s || e.name == subproxy.alias.to_s } + if elements.empty? && subproxy.init? && !subproxy.multi? + elements << Fluent::Config::Element.new(subproxy.name.to_s, '', {}, []) + end + # set subproxy for secret option elements.each { |element| element.corresponding_proxies << subproxy diff --git a/test/config/test_configurable.rb b/test/config/test_configurable.rb index b1a416f36f..d1d7fe3411 100644 --- a/test/config/test_configurable.rb +++ b/test/config/test_configurable.rb @@ -89,6 +89,16 @@ def get_all end end + class Init0 + include Fluent::Configurable + config_section :sec1, init: true, multi: false do + config_param :name, :string, default: 'sec1' + end + config_section :sec2, init: true, multi: true do + config_param :name, :string, default: 'sec1' + end + end + class Example0 include Fluent::Configurable @@ -642,6 +652,15 @@ class TestConfigurable < ::Test::Unit::TestCase assert_raise(Fluent::ConfigError) { checker.call(complete.reject{|k,v| k == "arrayvalue" }) } end + test 'generates section with default values for init:true sections' do + conf = config_element('ROOT', '', {}, []) + init0 = ConfigurableSpec::Init0.new + assert_nothing_raised { init0.configure(conf) } + assert init0.sec1 + assert_equal "sec1", init0.sec1.name + assert_equal [], init0.sec2 + end + test 'accepts configuration values as string representation' do conf = config_element('ROOT', '', { "stringvalue" => "s1", "boolvalue" => "yes", "integervalue" => "10", diff --git a/test/config/test_configure_proxy.rb b/test/config/test_configure_proxy.rb index 63ee5c7809..8c6c4c5654 100644 --- a/test/config/test_configure_proxy.rb +++ b/test/config/test_configure_proxy.rb @@ -12,6 +12,7 @@ class TestConfigureProxy < ::Test::Unit::TestCase proxy = Fluent::Config::ConfigureProxy.new(:section) assert_equal(:section, proxy.name) assert_equal(:section, proxy.param_name) + assert_nil(proxy.init) assert_nil(proxy.required) assert_false(proxy.required?) assert_nil(proxy.multi) @@ -19,7 +20,7 @@ class TestConfigureProxy < ::Test::Unit::TestCase end test 'can specify param_name/required/multi with optional arguments' do - proxy = Fluent::Config::ConfigureProxy.new(:section, param_name: 'sections', required: false, multi: true) + proxy = Fluent::Config::ConfigureProxy.new(:section, param_name: 'sections', init: true, required: false, multi: true) assert_equal(:section, proxy.name) assert_equal(:sections, proxy.param_name) assert_false(proxy.required) @@ -27,7 +28,7 @@ class TestConfigureProxy < ::Test::Unit::TestCase assert_true(proxy.multi) assert_true(proxy.multi?) - proxy = Fluent::Config::ConfigureProxy.new(:section, param_name: :sections, required: true, multi: false) + proxy = Fluent::Config::ConfigureProxy.new(:section, param_name: :sections, init: false, required: true, multi: false) assert_equal(:section, proxy.name) assert_equal(:sections, proxy.param_name) assert_true(proxy.required) @@ -35,6 +36,11 @@ class TestConfigureProxy < ::Test::Unit::TestCase assert_false(proxy.multi) assert_false(proxy.multi?) end + test 'raise error if both of init and required are true' do + assert_raise "init and required are exclusive" do + Fluent::Config::ConfigureProxy.new(:section, init: true, required: true) + end + end end sub_test_case '#merge' do @@ -42,15 +48,18 @@ class TestConfigureProxy < ::Test::Unit::TestCase proxy = p1 = Fluent::Config::ConfigureProxy.new(:section) assert_equal(:section, proxy.name) assert_equal(:section, proxy.param_name) + assert_nil(proxy.init) assert_nil(proxy.required) assert_false(proxy.required?) assert_nil(proxy.multi) assert_true(proxy.multi?) - p2 = Fluent::Config::ConfigureProxy.new(:section, param_name: :sections, required: true, multi: false) + p2 = Fluent::Config::ConfigureProxy.new(:section, param_name: :sections, init: false, required: true, multi: false) proxy = p1.merge(p2) assert_equal(:section, proxy.name) assert_equal(:sections, proxy.param_name) + assert_false(proxy.init) + assert_false(proxy.init?) assert_true(proxy.required) assert_true(proxy.required?) assert_false(proxy.multi) @@ -59,11 +68,13 @@ class TestConfigureProxy < ::Test::Unit::TestCase test 'does not overwrite with argument object without any specifications of required/multi' do p1 = Fluent::Config::ConfigureProxy.new(:section1) - p2 = Fluent::Config::ConfigureProxy.new(:section2, param_name: :sections, required: true, multi: false) + p2 = Fluent::Config::ConfigureProxy.new(:section2, param_name: :sections, init: false, required: true, multi: false) p3 = Fluent::Config::ConfigureProxy.new(:section3) proxy = p1.merge(p2).merge(p3) assert_equal(:section3, proxy.name) assert_equal(:section3, proxy.param_name) + assert_false(proxy.init) + assert_false(proxy.init?) assert_true(proxy.required) assert_true(proxy.required?) assert_false(proxy.multi) @@ -95,6 +106,50 @@ class TestConfigureProxy < ::Test::Unit::TestCase end end + sub_test_case '#config_param without default values cause error if section is configured as init:true' do + test 'with simple config_param with default value' do + proxy = Fluent::Config::ConfigureProxy.new(:section) + assert_nothing_raised do + proxy.config_section :subsection, init: true do + config_param :param1, :integer, default: 1 + end + end + end + test 'with simple config_param without default value' do + proxy = Fluent::Config::ConfigureProxy.new(:section) + assert_raise ArgumentError do + proxy.config_section :subsection, init: true do + config_param :param1, :integer + end + end + end + test 'with config_param with config_set_default' do + proxy = Fluent::Config::ConfigureProxy.new(:section) + assert_nothing_raised do + proxy.config_section :subsection, init: true do + config_param :param1, :integer + config_set_default :param1, 1 + end + end + end + test 'with config_argument' do + proxy = Fluent::Config::ConfigureProxy.new(:section) + assert_raise ArgumentError do + proxy.config_section :subsection, init: true do + config_argument :param0, :string + end + end + end + test 'with config_argument with default value' do + proxy = Fluent::Config::ConfigureProxy.new(:section) + assert_nothing_raised do + proxy.config_section :subsection, init: true do + config_argument :param0, :string, default: '' + end + end + end + end + sub_test_case '#config_set_desc' do setup do @proxy = Fluent::Config::ConfigureProxy.new(:section)