diff --git a/lib/fluent/config/configure_proxy.rb b/lib/fluent/config/configure_proxy.rb index 9f47790cf2..09f1326ea1 100644 --- a/lib/fluent/config/configure_proxy.rb +++ b/lib/fluent/config/configure_proxy.rb @@ -41,7 +41,7 @@ def initialize(name, param_name: nil, final: nil, init: nil, required: nil, mult @name = name.to_sym @final = final - @param_name = (param_name || @name).to_sym + @param_name = param_name && param_name.to_sym @init = init @required = required @multi = multi @@ -63,6 +63,10 @@ def initialize(name, param_name: nil, final: nil, init: nil, required: nil, mult @current_description = nil end + def variable_name + @param_name || @name + end + def init? @init.nil? ? false : @init end @@ -82,26 +86,17 @@ 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 - if overwrite?(other, :multi) - raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: multi" - end - if overwrite?(other, :alias) - raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: alias" - end - if overwrite?(other, :configured_in_section) - raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: configured_in" + [:param_name, :required, :multi, :alias, :configured_in_section].each do |prohibited_name| + if overwrite?(other, prohibited_name) + raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: #{prohibited_name}" + end end options = {} - # param_name is used not to ovewrite plugin's instance - # varible, so this should be able to be overwritten - options[:param_name] = other.param_name + # param_name affects instance variable name, which is just "internal" of each plugins. + # so it must not be changed. base class's name (or param_name) is always used. + options[:param_name] = @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 @@ -110,7 +105,7 @@ def merge(other) # self is base class, other is subclass options[:final] = @final || other.final options[:type_lookup] = @type_lookup - merged = self.class.new(other.name, options) + merged = self.class.new(@name, options) # configured_in MUST be kept merged.configured_in_section = self.configured_in_section @@ -137,45 +132,36 @@ def merge(other) # self is base class, other is subclass def merge_for_finalized(other) # list what subclass can do for finalized section - # * overwrite param_name to escape duplicated name of instance variable # * append params/defaults/sections which are missing in superclass + # * change default values of superclass + # * overwrite init to make it enable to instantiate section objects with added default values if other.final == false && overwrite?(other, :final) 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 - if overwrite?(other, :multi) - raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: multi" - end - if overwrite?(other, :alias) - raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: alias" - end - if overwrite?(other, :configured_in_section) - raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: configured_in" + [:param_name, :required, :multi, :alias, :configured_in_section].each do |prohibited_name| + if overwrite?(other, prohibited_name) + raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: #{prohibited_name}" + end end options = {} - options[:param_name] = other.param_name - options[:init] = @init.nil? ? other.init : self.init + options[:param_name] = @param_name + options[:init] = @init || other.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 options[:final] = true options[:type_lookup] = @type_lookup - merged = self.class.new(other.name, options) + merged = self.class.new(@name, options) merged.configured_in_section = self.configured_in_section merged.argument = self.argument || other.argument merged.params = other.params.merge(self.params) - merged.defaults = other.defaults.merge(self.defaults) + merged.defaults = self.defaults.merge(other.defaults) merged.sections = {} (self.sections.keys + other.sections.keys).uniq.each do |section_key| self_section = self.sections[section_key] diff --git a/lib/fluent/config/section.rb b/lib/fluent/config/section.rb index 5467e3e927..dd423c0d58 100644 --- a/lib/fluent/config/section.rb +++ b/lib/fluent/config/section.rb @@ -144,7 +144,7 @@ def self.generate(proxy, conf, logger, plugin_class, stack = []) check_unused_section(proxy, conf, plugin_class) proxy.sections.each do |name, subproxy| - varname = subproxy.param_name.to_sym + varname = subproxy.variable_name 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, '', {}, []) diff --git a/lib/fluent/configurable.rb b/lib/fluent/configurable.rb index 304f7f4b3d..6b7c531aea 100644 --- a/lib/fluent/configurable.rb +++ b/lib/fluent/configurable.rb @@ -41,9 +41,9 @@ def initialize next if name.to_s.start_with?('@') subproxy = proxy.sections[name] if subproxy.multi? - instance_variable_set("@#{subproxy.param_name}".to_sym, []) + instance_variable_set("@#{subproxy.variable_name}".to_sym, []) else - instance_variable_set("@#{subproxy.param_name}".to_sym, nil) + instance_variable_set("@#{subproxy.variable_name}".to_sym, nil) end end end @@ -145,7 +145,7 @@ def config_set_desc(name, desc) def config_section(name, **kwargs, &block) configure_proxy(self.name).config_section(name, **kwargs, &block) - attr_accessor configure_proxy(self.name).sections[name].param_name + attr_accessor configure_proxy(self.name).sections[name].variable_name end def desc(description) diff --git a/test/config/test_configurable.rb b/test/config/test_configurable.rb index a8dff79b87..f87f370905 100644 --- a/test/config/test_configurable.rb +++ b/test/config/test_configurable.rb @@ -215,74 +215,101 @@ class AddParamOverwriteAddress < Base end module Final + # Show what is allowed in finalized sections + # InheritsFinalized < Finalized < Base class Base include Fluent::Configurable config_section :appendix, multi: false, final: false do - config_param :name, :string, default: "x" + config_param :code, :string + config_param :name, :string + config_param :address, :string, default: "" end end class Finalized < Base + # to non-finalized section + # subclass can change type (code) + # add default value (name) + # change default value (address) + # add field (age) config_section :appendix, final: true do - config_param :name, :string, default: "y" + config_param :code, :integer + config_set_default :name, "y" + config_set_default :address, "-" config_param :age, :integer, default: 10 end end class InheritsFinalized < Finalized + # to finalized section + # subclass can add default value (code) + # change default value (age) + # add field (phone_no) config_section :appendix do - config_param :name, :string, default: "z" - config_param :age, :integer, default: 20 + config_set_default :code, 2 + config_set_default :age, 0 config_param :phone_no, :string end end + # Show what is allowed/prohibited for finalized sections class FinalizedBase include Fluent::Configurable - config_section :appendix, required: true, multi: false, alias: "options", final: true do - config_param :name, :string, default: "x" + config_section :appendix, param_name: :apd, init: false, required: true, multi: false, alias: "options", final: true do + config_param :name, :string end end - class InheritsFinalized2 < FinalizedBase - config_section :appendix do - config_param :name, :string, default: "y" - config_param :age, :integer, default: 10 - config_param :phone_no, :string + class FinalizedBase2 + include Fluent::Configurable + config_section :appendix, param_name: :apd, init: false, required: false, multi: false, alias: "options", final: true do + config_param :name, :string end end - class InheritsFinalized3 < InheritsFinalized2 + # subclass can change init with adding default values + class OverwriteInit < FinalizedBase2 + config_section :appendix, init: true do + config_set_default :name, "moris" + config_param :code, :integer, default: 0 + end + end + + # subclass cannot change type (name) + class Subclass < FinalizedBase config_section :appendix do - config_param :name, :string, default: "y" - config_param :age, :integer, default: 20 - config_param :phone_no, :string + config_param :name, :integer + end + end + + # subclass cannot change param_name + class OverwriteParamName < FinalizedBase + config_section :appendix, param_name: :adx do end end - # Error - class InheritsFinalized4 < FinalizedBase + # subclass cannot change final (section) + class OverwriteFinal < FinalizedBase config_section :appendix, final: false do - config_param :age, :integer, default: 10 - config_param :phone_no, :string + config_param :name, :integer end end + # subclass cannot change required class OverwriteRequired < FinalizedBase config_section :appendix, required: false do - config_param :phone_no, :string end end + # subclass cannot change multi class OverwriteMulti < FinalizedBase config_section :appendix, multi: true do - config_param :phone_no, :string end end + # subclass cannot change alias class OverwriteAlias < FinalizedBase config_section :appendix, alias: "options2" do - config_param :phone_no, :string end end end @@ -862,60 +889,96 @@ class TestConfigurable < ::Test::Unit::TestCase end sub_test_case 'final' do - CONF = config_element('ROOT', '', {}, - [config_element('appendix', '', {"phone_no" => "+81-0000-0000"}, [])]) - test 'subclass can overwrite appendix.name, appendix.age w/ level 2' do - finalized = ConfigurableSpec::Final::Finalized.new - finalized.configure(CONF) - expected = { name: "y", age: 10 } - actual = { name: finalized.appendix.name, age: finalized.appendix.age } - assert_equal(expected, actual) + test 'base class has designed params and default values' do + b = ConfigurableSpec::Final::Base.new + appendix_conf = config_element('appendix', '', {"code" => "b", "name" => "base"}) + b.configure(config_element('ROOT', '', {}, [appendix_conf])) + + assert_equal "b", b.appendix.code + assert_equal "base", b.appendix.name + assert_equal "", b.appendix.address end - test 'subclass cannot overwrite appendix.name, appendix.age w/ level 3' do - level3 = ConfigurableSpec::Final::InheritsFinalized.new - level3.configure(CONF) - expected = { name: "y", age: 10 } - actual = { name: level3.appendix.name, age: level3.appendix.age } - assert_equal(expected, actual) + test 'subclass can change type, add default value, change default value of parameters, and add parameters to non-finalized section' do + f = ConfigurableSpec::Final::Finalized.new + appendix_conf = config_element('appendix', '', {"code" => 1}) + f.configure(config_element('ROOT', '', {}, [appendix_conf])) + + assert_equal 1, f.appendix.code + assert_equal 'y', f.appendix.name + assert_equal "-", f.appendix.address + assert_equal 10, f.appendix.age end - test 'inherit finalized base' do - target1 = ConfigurableSpec::Final::InheritsFinalized2.new - target1.configure(CONF) - expected = { name: "x", age: 10 } - actual1 = { name: target1.appendix.name, age: target1.appendix.age } - assert_equal(expected, actual1) - - target2 = ConfigurableSpec::Final::InheritsFinalized3.new - target2.configure(CONF) - actual2 = { name: target2.appendix.name, age: target2.appendix.age } - assert_equal(expected, actual2) + test 'subclass can add default value, change default value of parameters, and add parameters to finalized section' do + i = ConfigurableSpec::Final::InheritsFinalized.new + appendix_conf = config_element('appendix', '', {"phone_no" => "00-0000-0000"}) + i.configure(config_element('ROOT', '', {}, [appendix_conf])) + + assert_equal 2, i.appendix.code + assert_equal 0, i.appendix.age + assert_equal "00-0000-0000", i.appendix.phone_no end - test 'failed to overwrite finalized base' do + test 'finalized base class works as designed' do + b = ConfigurableSpec::Final::FinalizedBase.new + appendix_conf = config_element('options', '', {"name" => "moris"}) + + assert_nothing_raised do + b.configure(config_element('ROOT', '', {}, [appendix_conf])) + end + assert b.apd + assert_equal "moris", b.apd.name + end + + test 'subclass can change init' do + n = ConfigurableSpec::Final::OverwriteInit.new + + assert_nothing_raised do + n.configure(config_element('ROOT', '')) + end + assert n.apd + assert_equal "moris", n.apd.name + assert_equal 0, n.apd.code + end + + test 'subclass cannot change parameter types in finalized sections' do + s = ConfigurableSpec::Final::Subclass.new + appendix_conf = config_element('options', '', {"name" => "1"}) + + assert_nothing_raised do + s.configure(config_element('ROOT', '', {}, [appendix_conf])) + end + assert_equal "1", s.apd.name + end + + test 'subclass cannot change param_name of finalized section' do + assert_raise(Fluent::ConfigError.new("BUG: subclass cannot overwrite base class's config_section: param_name")) do + ConfigurableSpec::Final::OverwriteParamName.new + end + end + + test 'subclass cannot change final of finalized section' do assert_raise(Fluent::ConfigError.new("BUG: subclass cannot overwrite finalized base class's config_section")) do - ConfigurableSpec::Final::InheritsFinalized4.new.configure(CONF) + ConfigurableSpec::Final::OverwriteFinal.new end end - sub_test_case 'overwrite' do - test 'required' do - assert_raise(Fluent::ConfigError.new("BUG: subclass cannot overwrite base class's config_section: required")) do - ConfigurableSpec::Final::OverwriteRequired.new.configure(CONF) - end + test 'subclass cannot change required of finalized section' do + assert_raise(Fluent::ConfigError.new("BUG: subclass cannot overwrite base class's config_section: required")) do + ConfigurableSpec::Final::OverwriteRequired.new end + end - test 'multi' do - assert_raise(Fluent::ConfigError.new("BUG: subclass cannot overwrite base class's config_section: multi")) do - ConfigurableSpec::Final::OverwriteMulti.new.configure(CONF) - end + test 'subclass cannot change multi of finalized section' do + assert_raise(Fluent::ConfigError.new("BUG: subclass cannot overwrite base class's config_section: multi")) do + ConfigurableSpec::Final::OverwriteMulti.new end + end - test 'alias' do - assert_raise(Fluent::ConfigError.new("BUG: subclass cannot overwrite base class's config_section: alias")) do - ConfigurableSpec::Final::OverwriteAlias.new.configure(CONF) - end + test 'subclass cannot change alias of finalized section' do + assert_raise(Fluent::ConfigError.new("BUG: subclass cannot overwrite base class's config_section: alias")) do + ConfigurableSpec::Final::OverwriteAlias.new end end end diff --git a/test/config/test_configure_proxy.rb b/test/config/test_configure_proxy.rb index 34ca037118..f0c01e4ebc 100644 --- a/test/config/test_configure_proxy.rb +++ b/test/config/test_configure_proxy.rb @@ -15,7 +15,8 @@ class TestConfigureProxy < ::Test::Unit::TestCase proxy = Fluent::Config::ConfigureProxy.new(:section, type_lookup: @type_lookup) assert_equal(:section, proxy.name) - assert_equal(:section, proxy.param_name) + assert_nil(proxy.param_name) + assert_equal(:section, proxy.variable_name) assert_nil(proxy.init) assert_nil(proxy.required) assert_false(proxy.required?) @@ -27,6 +28,7 @@ class TestConfigureProxy < ::Test::Unit::TestCase proxy = Fluent::Config::ConfigureProxy.new(:section, param_name: 'sections', init: true, required: false, multi: true, type_lookup: @type_lookup) assert_equal(:section, proxy.name) assert_equal(:sections, proxy.param_name) + assert_equal(:sections, proxy.variable_name) assert_false(proxy.required) assert_false(proxy.required?) assert_true(proxy.multi) @@ -35,6 +37,7 @@ class TestConfigureProxy < ::Test::Unit::TestCase proxy = Fluent::Config::ConfigureProxy.new(:section, param_name: :sections, init: false, required: true, multi: false, type_lookup: @type_lookup) assert_equal(:section, proxy.name) assert_equal(:sections, proxy.param_name) + assert_equal(:sections, proxy.variable_name) assert_true(proxy.required) assert_true(proxy.required?) assert_false(proxy.multi) @@ -51,7 +54,8 @@ class TestConfigureProxy < ::Test::Unit::TestCase test 'generate a new instance which values are overwritten by the argument object' do proxy = p1 = Fluent::Config::ConfigureProxy.new(:section, type_lookup: @type_lookup) assert_equal(:section, proxy.name) - assert_equal(:section, proxy.param_name) + assert_nil(proxy.param_name) + assert_equal(:section, proxy.variable_name) assert_nil(proxy.init) assert_nil(proxy.required) assert_false(proxy.required?) @@ -59,10 +63,11 @@ class TestConfigureProxy < ::Test::Unit::TestCase assert_true(proxy.multi?) assert_nil(proxy.configured_in_section) - p2 = Fluent::Config::ConfigureProxy.new(:section, param_name: :sections, init: false, required: true, multi: false, type_lookup: @type_lookup) + p2 = Fluent::Config::ConfigureProxy.new(:section, init: false, required: true, multi: false, type_lookup: @type_lookup) proxy = p1.merge(p2) assert_equal(:section, proxy.name) - assert_equal(:sections, proxy.param_name) + assert_nil(proxy.param_name) + assert_equal(:section, proxy.variable_name) assert_false(proxy.init) assert_false(proxy.init?) assert_true(proxy.required) @@ -73,13 +78,14 @@ class TestConfigureProxy < ::Test::Unit::TestCase end test 'does not overwrite with argument object without any specifications of required/multi' do - p1 = Fluent::Config::ConfigureProxy.new(:section1, type_lookup: @type_lookup) + p1 = Fluent::Config::ConfigureProxy.new(:section1, param_name: :sections, type_lookup: @type_lookup) p1.configured_in_section = :subsection - p2 = Fluent::Config::ConfigureProxy.new(:section2, param_name: :sections, init: false, required: true, multi: false, type_lookup: @type_lookup) + p2 = Fluent::Config::ConfigureProxy.new(:section2, init: false, required: true, multi: false, type_lookup: @type_lookup) p3 = Fluent::Config::ConfigureProxy.new(:section3, type_lookup: @type_lookup) proxy = p1.merge(p2).merge(p3) - assert_equal(:section3, proxy.name) - assert_equal(:section3, proxy.param_name) + assert_equal(:section1, proxy.name) + assert_equal(:sections, proxy.param_name) + assert_equal(:sections, proxy.variable_name) assert_false(proxy.init) assert_false(proxy.init?) assert_true(proxy.required)