Skip to content

Commit

Permalink
RuleBasedEditor: additional comment cleanups and clarifications
Browse files Browse the repository at this point in the history 
- Also includes some code reformatting
  • Loading branch information
@jouvin
jouvin committed May 9, 2016
1 parent f0c06fb commit a3a4a03
Showing 1 changed file with 55 additions and 65 deletions.
120 changes: 55 additions & 65 deletions src/main/perl/RuleBasedEditor.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,34 +70,9 @@ hash).
=item line_fmt
Defines the format used to represent the key/value pair. The following formats are
supported (see LINE_FORMAT_xxx constants below):
=over
=item *
A SH shell environment variable definition (export key=val).
=item *
A SH shell variable definition (key=val).
=item *
A 'keyword value' line, as used by Apache config files (similar to Config::General).
=item *
A 'setenv keyword=value' line, as used by Xrootd config files mainly. It doesn't work in a CSH shell script (C<=> present).
=item *
A 'set keyword=value' line, as used in a a CSH shell script to define a shell variable.
=back
Inline comments are not supported in 'keyword value' family of formats.
Defines the format used to represent the keyword/value pair. Several format are supported covering
the most usual ones (SH shell script, Apache, ...). For the exact list, see the definition of
LINE_FORMAT_xxx constants and the associated documentation below.
=item value_fmt
Expand Down Expand Up @@ -184,6 +159,8 @@ LINE_FORMAT_KEY_VAL_SET: set key=val (CSH shell variable)
=back
Inline comments are not supported for the LINE_FORMAT_KEY_VAL_xxx formats.
=cut

use enum qw(
Expand Down Expand Up @@ -253,7 +230,7 @@ LINE_VALUE_OPT_SINGLE: each value must be a separate instance of the keyword (mu
=item
LINE_VALUE_OPT_UNIQUE: each values are concataneted as a space-separated string
LINE_VALUE_OPT_UNIQUE: each values are concatenated as a space-separated string
=item
Expand Down Expand Up @@ -303,6 +280,14 @@ our %EXPORT_TAGS;
push @EXPORT_OK, @RULE_CONSTANTS;
$EXPORT_TAGS{rule_constants} = \@RULE_CONSTANTS;

=pod
$FILE_INTRO_xxx: constants defining the expected header lines in the configuration file
=cut

Readonly my $FILE_INTRO_PATTERN => "# This file is managed by Quattor";
Readonly my $FILE_INTRO_TXT => "# This file is managed by Quattor - DO NOT EDIT lines generated by Quattor";

# Backup file extension
Readonly my $BACKUP_FILE_EXT => ".old";
Expand All @@ -318,8 +303,8 @@ Readonly my $BACKUP_FILE_EXT => ".old";
Update configuration file contents, applying configuration rules.
Arguments :
config_rules: config rules corresponding to the file to build
config_options: configuration parameters used to build actual configuration
config_rules: a hashref containing config rules corresponding to the file to build
config_options: a hashref for configuration parameters used to build actual configuration
options: a hashref defining options to modify the behaviour of this function
Supported entries for options hash:
Expand All @@ -329,7 +314,7 @@ Supported entries for options hash:
Return value
sucess: 1
argument error: undef
argument error or error duing rule processing: undef
=cut

Expand All @@ -354,12 +339,10 @@ sub updateFile
$self->seek_begin();

# Check that config file has an appropriate header
Readonly my $INTRO_PATTERN => "# This file is managed by Quattor";
my $intro = "# This file is managed by Quattor - DO NOT EDIT lines generated by Quattor";
$self->add_or_replace_lines(
qr/^$INTRO_PATTERN/,
qr/^$intro$/,
$intro . "\n#\n",
qr/^$FILE_INTRO_PATTERN/,
qr/^$FILE_INTRO_TXT$/,
$FILE_INTRO_TXT . "\n#\n",
BEGINNING_OF_FILE,
);

Expand Down Expand Up @@ -433,7 +416,7 @@ sub _formatAttributeValue
# LINE_VALUE_INSTANCE_PARAMS is a value format specific to XrootD (http://xrootd.org).
# The value is a hash containing 3 keys that are used to construct a command option line.
$formatted_value = ''; # Don't return if no matching attributes is found
# Instance parameters are described in a nlist
# Instance parameters are described in a hash
$formatted_value .= " -l $attr_value->{logFile}" if $attr_value->{logFile};
$formatted_value .= " -c $attr_value->{configFile}" if $attr_value->{configFile};
$formatted_value .= " -k $attr_value->{logKeep}" if $attr_value->{logKeep};
Expand Down Expand Up @@ -663,6 +646,7 @@ sub _commentConfigLine
# The code used is a customized version of FileEditor::replace() that lacks support for backreferences
# in the replacement value (here we want to rewrite the same line commented out but we don't know the
# current line contents, only a regexp matching it).
# FIXME: should be updated when https://github.com/quattor/CAF/issues/125 is fixed.
my @lns;
my $line_count = 0;
$self->seek_begin();
Expand Down Expand Up @@ -914,7 +898,10 @@ Apply configuration rules. This method is the real workhorse of the rule-based e
Arguments :
config_rules: config rules corresponding to the file to build
config_options: configuration parameters used to build actual configuration
config_options: configuration parameters used to build actual configuration. Note that keys in the
config_options hash are interpreted as escaped (generally harmless if they are not as the
killing sequence, '_'+ 2 hex digit, is unlikely to occur in this context. Use camel case
for keys to prevent problems).
parser_options: a hash setting options to modify the behaviour of this function
Supported entries for options hash:
Expand Down Expand Up @@ -974,7 +961,7 @@ sub _apply_rules
$rule_id++;

# Initialize parser_options for this rule according the default for this file
my $rule_parsing_options = {%{$parser_options}};
my $rule_parsing_options = {%$parser_options};

# Check if the keyword is prefixed by:
# - a '-': in this case the corresponding line must be unconditionally
Expand Down Expand Up @@ -1010,37 +997,43 @@ sub _apply_rules

# If the keyword was "negated", remove (comment out) configuration line if present and enabled
if ($comment_line) {
*$self->{LOG}->debug(1, "$function_name: keyword '$keyword' negated, removing configuration line");
$self->_commentConfigLine($keyword, $line_fmt);
*$self->{LOG}->debug(1, "$function_name: keyword '$keyword' negated, removing/commenting configuration line");
unless ( $self->_commentConfigLine($keyword, $line_fmt) ) {
*$self->{LOG}->error("$function_name: _commentConfigLine() encountered an internal error, lines matching '$keyword' not removed");
}
next;
}


# Parse rule if it is non empty
my $rule_info;
if ($rule ne '') {
*$self->{LOG}
->debug(1, "$function_name: processing rule $rule_id (variable=>>>$keyword<<<, rule=>>>$rule<<<, fmt=$line_fmt)");
*$self->{LOG}->debug(1, "$function_name: processing rule $rule_id (variable=>>>$keyword<<<, rule=>>>$rule<<<, fmt=$line_fmt)");
$rule_info = $self->_parse_rule($rule, $config_options, $rule_parsing_options);
next unless $rule_info;
*$self->{LOG}->debug(2, "$function_name: information returned by rule parser: " . join(" ", sort(keys(%$rule_info))));

if (exists($rule_info->{error_msg})) {
# FIXME: decide whether an invalid rule is considered an error or just a warning. The latter would
# allow the caller to decide what to do exactly rather than impose an error (meaning a
# potential dependency failure)
*$self->{LOG}->error("Error parsing rule >>>$rule<<<: " . $rule_info->{error_msg});
# An invalid rule is just ignored
next;
} elsif ($rule_info->{remove_matching_lines}) {
if ($rule_parsing_options->{remove_if_undef}) {
*$self->{LOG}->debug(1, "$function_name: removing configuration lines for keyword '$keyword'");
$self->_commentConfigLine($keyword, $line_fmt);
*$self->{LOG}->debug(1, "$function_name: removing/commenting configuration lines for keyword '$keyword'");
unless ( $self->_commentConfigLine($keyword, $line_fmt) ) {
*$self->{LOG}->error("$function_name: _commentConfigLine() encountered an internal error, lines matching '$keyword' not removed");
}
} else {
*$self->{LOG}->debug(1, "$function_name: remove_if_undef not set, ignoring line to remove");
}
next;
}
}

# Build the value to be substitued for each option set specified.
# Build the value to be substituted for each option set specified.
# option_set=GLOBAL is a special case indicating a global option instead of an
# attribute in a specific option set.
my $config_value = "";
Expand All @@ -1050,14 +1043,12 @@ sub _apply_rules
if ($rule_info->{attribute}) {
foreach my $option_set (@{$rule_info->{option_sets}}) {
my $attr_value;
*$self->{LOG}
->debug(1, "$function_name: retrieving '" . $rule_info->{attribute} . "' value in option set $option_set");
*$self->{LOG}->debug(1, "$function_name: retrieving '" . $rule_info->{attribute} . "' value in option set $option_set");
if ($option_set eq $RULE_OPTION_SET_GLOBAL) {
if (exists($config_options->{$rule_info->{attribute}})) {
if ( $config_options->{$rule_info->{attribute}} ) {
$attr_value = $config_options->{$rule_info->{attribute}};
} else {
*$self->{LOG}
->debug(1, "$function_name: attribute '" . $rule_info->{attribute} . "' not found in global option set");
*$self->{LOG}->debug(1, "$function_name: attribute '" . $rule_info->{attribute} . "' not found in global option set");
$attribute_present = 0;
}
} else {
Expand Down Expand Up @@ -1090,21 +1081,19 @@ sub _apply_rules
next;
}

# Instance parameters are specific, as this is a nlist of instance
# with the value being a nlist of parameters for the instance.
# Instance parameters are specific, as this is a hash of instances
# with the value being a hash of parameters for the instance.
# Also the variable name must be updated to contain the instance name.
# One configuration line must be written/updated for each instance.
if ($value_fmt == LINE_VALUE_INSTANCE_PARAMS) {
foreach my $instance (sort keys %{$attr_value}) {
my $params = $attr_value->{$instance};
*$self->{LOG}->debug(1, "$function_name: formatting instance '$instance' parameters ($params)");
$config_value =
$self->_formatAttributeValue(
$params,
$line_fmt,
$value_fmt,
$value_opt,
);
$config_value = $self->_formatAttributeValue($params,
$line_fmt,
$value_fmt,
$value_opt,
);
my $config_param = $keyword;
my $instance_uc = uc($instance);
$config_param =~ s/%%INSTANCE%%/$instance_uc/;
Expand All @@ -1114,12 +1103,13 @@ sub _apply_rules
$config_updated = 1;
} elsif ($value_fmt == LINE_VALUE_STRING_HASH) {
# With this value format, several lines with the same keyword are generated,
# one for each key/value pair.
# one for each keyword/value pair.
foreach my $k (sort keys %$attr_value) {
my $v = $attr_value->{$k};
# Value is made by joining key and value as a string
# Keys may be escaped if they contain characters like '/': unescaping a non-escaped
# string is generally harmless.
# Value is made by joining key and value as a string.
# Keys may be escaped if they contain characters like '/'. Generally harmless if
# they are not, except if the unescaped key as a sequence '_' + 2 hex digits.
# Unlikely in this context: to prevent problems use camel case for keys.
my $tmp = unescape($k) . " $v";
*$self->{LOG}->debug(1,
"$function_name: formatting (string hash) attribute '"
Expand Down

0 comments on commit a3a4a03

Please sign in to comment.