diff --git a/library/Director/CustomVariable/CustomVariableString.php b/library/Director/CustomVariable/CustomVariableString.php index 530ca4ce..16b2f210 100644 --- a/library/Director/CustomVariable/CustomVariableString.php +++ b/library/Director/CustomVariable/CustomVariableString.php @@ -46,7 +46,7 @@ class CustomVariableString extends CustomVariable public function toConfigString($renderExpressions = false) { if ($renderExpressions) { - return c::renderStringWithVariables($this->getValue()); + return c::renderStringWithVariables($this->getValue(), ['config']); } else { return c::renderString($this->getValue()); } diff --git a/library/Director/IcingaConfig/IcingaConfigHelper.php b/library/Director/IcingaConfig/IcingaConfigHelper.php index 6f5e246f..61da6f32 100644 --- a/library/Director/IcingaConfig/IcingaConfigHelper.php +++ b/library/Director/IcingaConfig/IcingaConfigHelper.php @@ -2,8 +2,7 @@ namespace Icinga\Module\Director\IcingaConfig; -use Icinga\Exception\IcingaException; -use Icinga\Exception\ProgrammingError; +use InvalidArgumentException; class IcingaConfigHelper { @@ -11,7 +10,7 @@ class IcingaConfigHelper * Reserved words according to * https://docs.icinga.com/icinga2/snapshot/doc/module/icinga2/chapter/language-reference#reserved-keywords */ - protected static $reservedWords = array( + protected static $reservedWords = [ 'object', 'template', 'include', @@ -39,7 +38,7 @@ class IcingaConfigHelper 'in', 'current_filename', 'current_line', - ); + ]; public static function renderKeyValue($key, $value, $prefix = ' ') { @@ -69,7 +68,10 @@ class IcingaConfigHelper } elseif ($value === 'n' || $value === false) { return 'false'; } else { - throw new ProgrammingError('%s is not a valid boolean', $value); + throw new InvalidArgumentException(sprintf( + '%s is not a valid boolean', + $value + )); } } @@ -98,7 +100,7 @@ class IcingaConfigHelper // Parameter? Dedicated method? Always if \n is found? public static function renderString($string) { - $special = array( + $special = [ '/\\\/', '/"/', '/\$/', @@ -106,10 +108,10 @@ class IcingaConfigHelper '/\r/', '/\n/', // '/\b/', -> doesn't work - '/\f/' - ); + '/\f/', + ]; - $replace = array( + $replace = [ '\\\\\\', '\\"', '\\$', @@ -118,7 +120,7 @@ class IcingaConfigHelper '\\n', // '\\b', '\\f', - ); + ]; $string = preg_replace($special, $replace, $string); @@ -144,7 +146,10 @@ class IcingaConfigHelper } elseif (is_string($value)) { return static::renderString($value); } else { - throw new IcingaException('Unexpected type %s', var_export($value, 1)); + throw new InvalidArgumentException(sprintf( + 'Unexpected type %s', + var_export($value, 1) + )); } } @@ -160,7 +165,7 @@ class IcingaConfigHelper // Requires an array public static function renderArray($array) { - $data = array(); + $data = []; foreach ($array as $entry) { if ($entry instanceof IcingaConfigRenderer) { $data[] = $entry; @@ -186,7 +191,7 @@ class IcingaConfigHelper public static function renderDictionary($dictionary) { - $vals = array(); + $vals = []; foreach ($dictionary as $key => $value) { $vals[$key] = rtrim( self::renderKeyValue( @@ -259,11 +264,12 @@ class IcingaConfigHelper $value = 0; foreach ($parts as $part) { if (! preg_match('/^(\d+)([dhms]?)$/', $part, $m)) { - throw new ProgrammingError( + throw new InvalidArgumentException(sprintf( '"%s" is not a valid time (duration) definition', $interval - ); + )); } + switch ($m[2]) { case 'd': $value += $m[1] * 86400; @@ -290,11 +296,11 @@ class IcingaConfigHelper return '0s'; } - $steps = array( + $steps = [ 'd' => 86400, 'h' => 3600, 'm' => 60, - ); + ]; foreach ($steps as $unit => $duration) { if ($seconds % $duration === 0) { @@ -305,37 +311,89 @@ class IcingaConfigHelper return $seconds . 's'; } - public static function stringHasMacro($string) + public static function stringHasMacro($string, $macroName = null) { - return preg_match('/(? $offset) { + $parts[] = static::renderString( + substr($string, $offset, $start - $offset) + ); + } + $parts[] = $macroName; + $offset = $i + 1; + } + } - if (substr($string, 0, 5) === '"" + ') { - $string = substr($string, 5); - } - if (substr($string, -5) === ' + ""') { - $string = substr($string, 0, -5); + $start = false; + } + } + } } - return $string; + if ($offset < $i) { + $parts[] = static::renderString(substr($string, $offset, $i - $offset)); + } + + return implode(' + ', $parts); } } diff --git a/library/Director/IcingaConfig/IcingaConfigRendered.php b/library/Director/IcingaConfig/IcingaConfigRendered.php index 10649648..90b710e3 100644 --- a/library/Director/IcingaConfig/IcingaConfigRendered.php +++ b/library/Director/IcingaConfig/IcingaConfigRendered.php @@ -2,7 +2,7 @@ namespace Icinga\Module\Director\IcingaConfig; -use Icinga\Exception\ProgrammingError; +use InvalidArgumentException; class IcingaConfigRendered implements IcingaConfigRenderer { @@ -11,7 +11,7 @@ class IcingaConfigRendered implements IcingaConfigRenderer public function __construct($string) { if (! is_string($string)) { - throw new ProgrammingError('IcingaConfigRendered accepts only strings'); + throw new InvalidArgumentException('IcingaConfigRendered accepts only strings'); } $this->rendered = $string; diff --git a/test/php/library/Director/CustomVariable/CustomVariablesTest.php b/test/php/library/Director/CustomVariable/CustomVariablesTest.php index 2a32ea30..c5ba9f67 100644 --- a/test/php/library/Director/CustomVariable/CustomVariablesTest.php +++ b/test/php/library/Director/CustomVariable/CustomVariablesTest.php @@ -15,15 +15,12 @@ class CustomVariablesTest extends BaseTestCase $vars->bla = 'da'; $vars->{'aBc'} = 'normal'; $vars->{'a-0'} = 'special'; - $expected = $this->indentVarsList(array( + $expected = $this->indentVarsList([ 'vars["a-0"] = "special"', 'vars.aBc = "normal"', 'vars.bla = "da"' - )); - $this->assertEquals( - $vars->toConfigString(), - $expected - ); + ]); + $this->assertEquals($expected, $vars->toConfigString()); } public function testVarsCanBeUnsetAndSetAgain() @@ -33,24 +30,21 @@ class CustomVariablesTest extends BaseTestCase unset($vars->one); $vars->one = 'three'; - $res = array(); + $res = []; foreach ($vars as $k => $v) { $res[$k] = $v->getValue(); } - $this->assertEquals( - array('one' => 'three'), - $res - ); + $this->assertEquals(['one' => 'three'], $res); } public function testNumericKeysAreRenderedWithArraySyntax() { $vars = $this->newVars(); $vars->{'1'} = 1; - $expected = $this->indentVarsList(array( + $expected = $this->indentVarsList([ 'vars["1"] = 1' - )); + ]); $this->assertEquals( $expected, @@ -63,14 +57,11 @@ class CustomVariablesTest extends BaseTestCase $vars = $this->newVars(); $vars->bla = 'da'; $vars->abc = '$val$'; - $expected = $this->indentVarsList(array( - 'vars.abc = val', + $expected = $this->indentVarsList([ + 'vars.abc = "$val$"', 'vars.bla = "da"' - )); - $this->assertEquals( - $vars->toConfigString(true), - $expected - ); + ]); + $this->assertEquals($expected, $vars->toConfigString(true)); } protected function indentVarsList($vars) diff --git a/test/php/library/Director/IcingaConfig/IcingaConfigHelperTest.php b/test/php/library/Director/IcingaConfig/IcingaConfigHelperTest.php index bb5955a0..506f3b8f 100644 --- a/test/php/library/Director/IcingaConfig/IcingaConfigHelperTest.php +++ b/test/php/library/Director/IcingaConfig/IcingaConfigHelperTest.php @@ -19,7 +19,7 @@ class IcingaConfigHelperTest extends BaseTestCase } /** - * @expectedException \Icinga\Exception\ProgrammingError + * @expectedException \InvalidArgumentException */ public function testWhetherInvalidIntervalStringRaisesException() { @@ -53,12 +53,12 @@ class IcingaConfigHelperTest extends BaseTestCase public function testWhetherDictionaryRendersCorrectly() { - $dict = (object) array( + $dict = (object) [ 'key1' => 'bla', 'include' => 'reserved', 'spe cial' => 'value', '0' => 'numeric', - ); + ]; $this->assertEquals( c::renderDictionary($dict), rtrim($this->loadRendered('dict1')) @@ -81,26 +81,50 @@ class IcingaConfigHelperTest extends BaseTestCase $this->assertEquals(c::renderString('\f'), '"\\\\f"'); } + public function testMacrosAreDetected() + { + $this->assertFalse(c::stringHasMacro('$$vars$')); + $this->assertFalse(c::stringHasMacro('$$')); + $this->assertTrue(c::stringHasMacro('$vars$$')); + $this->assertTrue(c::stringHasMacro('$multiple$$vars.nested.name$$vars$ is here')); + $this->assertTrue(c::stringHasMacro('some $vars.nested.name$ is here')); + $this->assertTrue(c::stringHasMacro('some $vars.nested.name$$vars.even.more$')); + $this->assertTrue(c::stringHasMacro('$vars.nested.name$$a$$$$not$')); + $this->assertTrue(c::stringHasMacro('MSSQL$$$config$')); + $this->assertTrue(c::stringHasMacro('MSSQL$$$config$', 'config')); + $this->assertTrue(c::stringHasMacro('MSSQL$$$nix$ and $config$', 'config')); + $this->assertFalse(c::stringHasMacro('MSSQL$$$nix$config$ and $$', 'config')); + $this->assertFalse(c::stringHasMacro('MSSQL$$$nix$ and $$config$', 'config')); + $this->assertFalse(c::stringHasMacro('MSSQL$$$config$', 'conf')); + } + public function testRenderStringWithVariables() { - $this->assertEquals(c::renderStringWithVariables('Before $var$'), '"Before " + var'); + $this->assertEquals('"Before " + var', c::renderStringWithVariables('Before $var$')); $this->assertEquals(c::renderStringWithVariables('$var$ After'), 'var + " After"'); $this->assertEquals(c::renderStringWithVariables('$var$'), 'var'); $this->assertEquals(c::renderStringWithVariables('$$var$$'), '"$$var$$"'); $this->assertEquals(c::renderStringWithVariables('Before $$var$$ After'), '"Before $$var$$ After"'); $this->assertEquals( - c::renderStringWithVariables('Before $name$ $name$ After'), - '"Before " + name + " " + name + " After"' + '"Before " + name1 + " " + name2 + " After"', + c::renderStringWithVariables('Before $name1$ $name2$ After') + ); + } + + public function testRenderStringWithVariablesX() + { + $this->assertEquals( + '"Before " + var1 + " " + var2 + " After"', + c::renderStringWithVariables('Before $var1$ $var2$ After') ); $this->assertEquals( - c::renderStringWithVariables('Before $var1$ $var2$ After'), - '"Before " + var1 + " " + var2 + " After"' + 'host.vars.custom', + c::renderStringWithVariables('$host.vars.custom$') ); - $this->assertEquals(c::renderStringWithVariables('$host.vars.custom$'), 'host.vars.custom'); - $this->assertEquals(c::renderStringWithVariables('$var"$'), '"$var\"$"'); + $this->assertEquals('"$var\"$"', c::renderStringWithVariables('$var"$')); $this->assertEquals( - c::renderStringWithVariables('\tI am\rrendering\nproperly\fand I $support$ "multiple" $variables$\$'), - '"\\\\tI am\\\\rrendering\\\\nproperly\\\\fand I " + support + " \"multiple\" " + variables + "\\\\$"' + '"\\\\tI am\\\\rrendering\\\\nproperly\\\\fand I " + support + " \"multiple\" " + variables + "\\\\$"', + c::renderStringWithVariables('\tI am\rrendering\nproperly\fand I $support$ "multiple" $variables$\$') ); } }