MemoryLimit: dedicated class, tests

fixes #1220

library/Director/Application/MemoryLimit.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace Icinga\Module\Director\Application;
+
+class MemoryLimit
+{
+    public static function raiseTo($string)
+    {
+        $current = static::getBytes();
+        $desired = static::parsePhpIniByteString($string);
+        if ($current !== -1 && $current < $desired) {
+            ini_set('memory_limit', $string);
+        }
+    }
+
+    public static function getBytes()
+    {
+        return static::parsePhpIniByteString((string) ini_get('memory_limit'));
+    }
+
+    /**
+     * Return Bytes from PHP shorthand bytes notation
+     *
+     * http://www.php.net/manual/en/faq.using.php#faq.using.shorthandbytes
+     *
+     * > The available options are K (for Kilobytes), M (for Megabytes) and G
+     * > (for Gigabytes), and are all case-insensitive. Anything else assumes
+     * > bytes.
+     *
+     * @param $string
+     * @return int
+     */
+    public static function parsePhpIniByteString($string)
+    {
+        $val = trim($string);
+        $last = strtoupper(substr($val, -1, 1));
+
+        switch ($last) {
+            case 'G':
+                $val *= 1024;
+                // Intentional fall-through
+            case 'M':
+                $val *= 1024;
+                // Intentional fall-through
+            case 'K':
+                $val *= 1024;
+        }
+
+        return intval($val);
+    }
+}

library/Director/Cli/Command.php

@@ -3,6 +3,7 @@
 namespace Icinga\Module\Director\Cli;
 
 use Icinga\Cli\Command as CliCommand;
+use Icinga\Module\Director\Application\MemoryLimit;
 use Icinga\Module\Director\Core\CoreApi;
 use Icinga\Module\Director\Db;
 use Icinga\Module\Director\Objects\IcingaEndpoint;
@@ -82,9 +83,7 @@ class Command extends CliCommand
      */
     protected function raiseLimits()
     {
-        if ((string) ini_get('memory_limit') !== '-1') {
+        MemoryLimit::raiseTo('1024M');
-            ini_set('memory_limit', '1024M');
-        }
 
         ini_set('max_execution_time', 0);
         if (version_compare(PHP_VERSION, '7.0.0') < 0) {

library/Director/IcingaConfig/IcingaConfig.php

@@ -9,6 +9,7 @@ use Icinga\Exception\ConfigurationError;
 use Icinga\Exception\IcingaException;
 use Icinga\Exception\NotFoundError;
 use Icinga\Exception\ProgrammingError;
+use Icinga\Module\Director\Application\MemoryLimit;
 use Icinga\Module\Director\Db\Cache\PrefetchCache;
 use Icinga\Module\Director\Db;
 use Icinga\Module\Director\Hook\ShipConfigFilesHook;
@@ -447,11 +448,7 @@ class IcingaConfig
 
         $start = microtime(true);
 
-        // Raise limits. TODO: do this in a failsafe way, and only if necessary
+        MemoryLimit::raiseTo('1024M');
-        if ((string) ini_get('memory_limit') !== '-1') {
-            ini_set('memory_limit', '1024M');
-        }
-
         ini_set('max_execution_time', 0);
         // Workaround for https://bugs.php.net/bug.php?id=68606 or similar
         ini_set('zend.enable_gc', 0);

library/Director/Import/Sync.php

@@ -4,6 +4,7 @@ namespace Icinga\Module\Director\Import;
 
 use Exception;
 use Icinga\Data\Filter\Filter;
+use Icinga\Module\Director\Application\MemoryLimit;
 use Icinga\Module\Director\Data\Db\DbObject;
 use Icinga\Module\Director\Db;
 use Icinga\Module\Director\Db\Cache\PrefetchCache;
@@ -151,15 +152,11 @@ class Sync
     /**
      * Raise PHP resource limits
      *
-     * TODO: do this in a failsafe way, and only if necessary
-     *
      * @return self;
      */
     protected function raiseLimits()
     {
-        if ((string) ini_get('memory_limit') !== '-1') {
+        MemoryLimit::raiseTo('1024M');
-            ini_set('memory_limit', '1024M');
-        }
         ini_set('max_execution_time', 0);
 
         return $this;

library/Director/Objects/ObjectApplyMatches.php

@@ -6,6 +6,7 @@ use Icinga\Application\Benchmark;
 use Icinga\Data\Filter\Filter;
 use Icinga\Data\Filter\FilterExpression;
 use Icinga\Exception\ProgrammingError;
+use Icinga\Module\Director\Application\MemoryLimit;
 use Icinga\Module\Director\Db;
 use Icinga\Module\Director\Db\Cache\PrefetchCache;
 use stdClass;
@@ -83,11 +84,9 @@ abstract class ObjectApplyMatches
 
     protected static function raiseLimits()
     {
-        // Raise limits. TODO: do this in a failsafe way, and only if necessary
+        // Note: IcingaConfig also raises the limit for generation, **but** we
-        // Note: IcingaConfig also raises the limit for generation, **but** we need the higher limit for preview.
+        // need the higher limit for preview.
-        if ((string) ini_get('memory_limit') !== '-1') {
+        MemoryLimit::raiseTo('1024M');
-            ini_set('memory_limit', '1024M');
-        }
     }
 
     protected static function fetchFlatObjects(Db $db)

test/php/library/Director/Application/MemoryLimitTest.php

@@ -0,0 +1,67 @@
+<?php
+
+namespace Tests\Icinga\Module\Director\Application;
+
+use Icinga\Module\Director\Application\MemoryLimit;
+use Icinga\Module\Director\Test\BaseTestCase;
+
+class MemoryLimitTest extends BaseTestCase
+{
+    public function testBytesValuesAreHandled()
+    {
+        $this->assertTrue(is_int(MemoryLimit::parsePhpIniByteString('1073741824')));
+        $this->assertEquals(
+            1073741824,
+            MemoryLimit::parsePhpIniByteString('1073741824')
+        );
+    }
+
+    public function testIntegersAreAccepted()
+    {
+        $this->assertEquals(
+            MemoryLimit::parsePhpIniByteString(1073741824),
+            1073741824
+        );
+    }
+
+    public function testNoLimitGivesMinusOne()
+    {
+        $this->assertTrue(is_int(MemoryLimit::parsePhpIniByteString('-1')));
+        $this->assertEquals(
+            -1,
+            MemoryLimit::parsePhpIniByteString('-1')
+        );
+    }
+
+    public function testInvalidStringGivesBytes()
+    {
+        $this->assertEquals(
+            1024,
+            MemoryLimit::parsePhpIniByteString('1024MB')
+        );
+    }
+
+    public function testHandlesKiloBytes()
+    {
+        $this->assertEquals(
+            45 * 1024,
+            MemoryLimit::parsePhpIniByteString('45K')
+        );
+    }
+
+    public function testHandlesMegaBytes()
+    {
+        $this->assertEquals(
+            512 * 1024 * 1024,
+            MemoryLimit::parsePhpIniByteString('512M')
+        );
+    }
+
+    public function testHandlesGigaBytes()
+    {
+        $this->assertEquals(
+            2 * 1024 * 1024 * 1024,
+            MemoryLimit::parsePhpIniByteString('2G')
+        );
+    }
+}