Skip to content

Commit

Permalink
Fix overwriting config_section definitions:
Browse files Browse the repository at this point in the history
 * "name" and "param_name" is strongly connected with plugin's code (even in base classes)
   so these MUST be not changed by subclasses (subclass MUST use same names)
 * separate "param_name" from "name" to detect overwriting "param_name" by subclasses to raise errors
   and add "variable_name" method as internal utility
 * make it possible to change default values in finalized sections
   because it's very popular that 3rd party plugin prefer different default value for built-in predefined parameters
 * make it possible to change "init" specification of finalized sections
   because newly added/changed default values might be able to init sections
  • Loading branch information
tagomoris committed May 23, 2016
1 parent ce372dd commit 6c913e8
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 112 deletions.
58 changes: 20 additions & 38 deletions lib/fluent/config/configure_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -82,26 +86,15 @@ 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|
raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: #{prohibited_name}"
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
Expand All @@ -110,7 +103,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
Expand All @@ -137,45 +130,34 @@ 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|
raise ConfigError, "BUG: subclass cannot overwrite base class's config_section: #{prohibited_name}"
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]
Expand Down
2 changes: 1 addition & 1 deletion lib/fluent/config/section.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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, '', {}, [])
Expand Down
6 changes: 3 additions & 3 deletions lib/fluent/configurable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 6c913e8

Please sign in to comment.