refs #6617 if process or output size is too large, declare it as finished

Declares a process as finished as soon as a PID file is > 500bytes
(contains only PID) and output file is > 100MB (which should usually
not be larger than 100KB or 1MB) to prevent files growing up to many
GBs. I added unit and integration tests for the file size detection. It is
now possible to mock the methods file_exists and filesize although very
simple so far. Later we can allow to define callbacks or to define different
return values for different files or we can use something like vfsStream

core/CliMulti.php

@@ -151,6 +151,14 @@ class CliMulti {
                 return false;
             }
 
+            $pid = $process->getPid();
+            foreach ($this->outputs as $output) {
+                if ($output->getOutputId() === $pid && $output->isAbnormal()) {
+                    $process->finishProcess();
+                    return true;
+                }
+            }
+
             if ($process->hasFinished()) {
                 // prevent from checking this process over and over again
                 unset($this->processes[$index]);

core/CliMulti/Output.php

@@ -13,6 +13,7 @@ use Piwik\Filesystem;
 class Output {
 
     private $tmpFile  = '';
+    private $outputId = null;
 
     public function __construct($outputId)
     {
@@ -23,7 +24,13 @@ class Output {
         $dir = CliMulti::getTmpPath();
         Filesystem::mkdir($dir);
 
-        $this->tmpFile = $dir . '/' . $outputId . '.output';
+        $this->tmpFile  = $dir . '/' . $outputId . '.output';
+        $this->outputId = $outputId;
+    }
+
+    public function getOutputId()
+    {
+        return $this->outputId;
     }
 
     public function write($content)
@@ -36,6 +43,13 @@ class Output {
         return $this->tmpFile;
     }
 
+    public function isAbnormal()
+    {
+        $size = Filesystem::getFileSize($this->tmpFile, 'MB');
+
+        return $size !== null && $size >= 100;
+    }
+
     public function exists()
     {
         return file_exists($this->tmpFile);

core/CliMulti/Process.php

@@ -24,6 +24,7 @@ class Process
     private $pidFile = '';
     private $timeCreation = null;
     private $isSupported = null;
+    private $pid = null;
 
     public function __construct($pid)
     {
@@ -37,10 +38,16 @@ class Process
         $this->isSupported  = self::isSupported();
         $this->pidFile      = $pidDir . '/' . $pid . '.pid';
         $this->timeCreation = time();
+        $this->pid = $pid;
 
         $this->markAsNotStarted();
     }
 
+    public function getPid()
+    {
+        return $this->pid;
+    }
+
     private function markAsNotStarted()
     {
         $content = $this->getPidFileContent();
@@ -97,6 +104,11 @@ class Process
             return false;
         }
 
+        if (!$this->pidFileSizeIsNormal()) {
+            $this->finishProcess();
+            return false;
+        }
+
         if ($this->isProcessStillRunning($content)) {
             return true;
         }
@@ -108,6 +120,13 @@ class Process
         return false;
     }
 
+    private function pidFileSizeIsNormal()
+    {
+        $size = Filesystem::getFileSize($this->pidFile);
+
+        return $size !== null && $size < 500;
+    }
+
     public function finishProcess()
     {
         Filesystem::deleteFileIfExists($this->pidFile);

core/Filesystem.php

@@ -373,6 +373,40 @@ class Filesystem
         return @unlink($pathToFile);
     }
 
+    /**
+     * Get the size of a file in the specified unit.
+     *
+     * @param string $pathToFile
+     * @param string $unit eg 'B' for Byte, 'KB', 'MB', 'GB', 'TB'.
+     *
+     * @return float|null Returns null if file does not exist or the size of the file in the specified unit
+     *
+     * @throws Exception In case the unit is invalid
+     */
+    public static function getFileSize($pathToFile, $unit = 'B')
+    {
+        $unit  = strtoupper($unit);
+        $units = array('TB' => pow(1024, 4),
+                       'GB' => pow(1024, 3),
+                       'MB' => pow(1024, 2),
+                       'KB' => 1024,
+                       'B' => 1);
+
+        if (!array_key_exists($unit, $units)) {
+            throw new Exception('Invalid unit given');
+        }
+
+        if (!file_exists($pathToFile)) {
+            return;
+        }
+
+        $filesize  = filesize($pathToFile);
+        $factor    = $units[$unit];
+        $converted = $filesize / $factor;
+
+        return $converted;
+    }
+
     /**
      * @param $path
      * @return int

plugins/ExamplePlugin/tests/Unit/SimpleTest.php

@@ -8,12 +8,14 @@
 
 namespace Piwik\Plugins\ExamplePlugin\tests\Unit;
 
+use Piwik\Tests\Framework\TestCase\UnitTestCase;
+
 /**
  * @group ExamplePlugin
  * @group SimpleTest
  * @group Plugins
  */
-class SimpleTest extends \PHPUnit_Framework_TestCase
+class SimpleTest extends UnitTestCase
 {
 
     /**

tests/PHPUnit/Framework/Mock/File.php

+<?php
+/**
+* Piwik - free/libre analytics platform
+*
+* @link http://piwik.org
+* @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+*/
+namespace Piwik;
+
+use Piwik\Tests\Framework\Mock\File;
+
+function filesize($filename)
+{
+    if (File::getFileSize() !== null) {
+        return File::getFileSize();
+    }
+
+    return \filesize($filename);
+}
+
+function file_exists($filename)
+{
+    if (File::getFileExists() !== null) {
+        return File::getFileExists();
+    }
+
+    return \file_exists($filename);
+}
+
+namespace Piwik\Tests\Framework\Mock;
+
+class File
+{
+    static $filesize = null;
+    static $fileExists = null;
+
+    public static function getFileSize()
+    {
+        return self::$filesize;
+    }
+
+    public static function setFileSize($filesize)
+    {
+        self::$filesize = $filesize;
+    }
+
+    public static function reset()
+    {
+        self::$filesize = null;
+        self::$fileExists = null;
+    }
+
+    public static function getFileExists()
+    {
+        return self::$fileExists;
+    }
+
+    public static function setFileExists($exists)
+    {
+        self::$fileExists = $exists;
+    }
+}

tests/PHPUnit/Framework/TestCase/UnitTestCase.php

+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Tests\Framework\TestCase;
+use Piwik\Tests\Framework\Mock\File;
+
+
+/**
+ * Base class for Unit tests.
+ *
+ * @since 2.10.0
+ */
+abstract class UnitTestCase extends \PHPUnit_Framework_TestCase
+{
+    public function setup()
+    {
+        parent::setUp();
+        File::reset();
+    }
+
+    public function tearDown()
+    {
+        parent::tearDown();
+        File::reset();
+    }
+}

tests/PHPUnit/Integration/FilesystemTest.php

+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Tests\Integration;
+
+use Piwik\Filesystem;
+
+/**
+ * @group Core
+ */
+class FilesystemTest extends \PHPUnit_Framework_TestCase
+{
+    public function test_getFileSize_ShouldRecognizeLowerUnits()
+    {
+        $size = Filesystem::getFileSize(__FILE__, 'b');
+
+        $this->assertGreaterThan(400, $size);
+        $this->assertLessThan(400000, $size);
+    }
+
+    public function test_getFileSize_ShouldReturnNull_IfFileDoesNotExists()
+    {
+        $size = Filesystem::getFileSize(PIWIK_INCLUDE_PATH . '/tests/NotExisting.File');
+
+        $this->assertNull($size);
+    }
+
+}
\ No newline at end of file

tests/PHPUnit/System/CliMultiTest.php

@@ -16,6 +16,7 @@ use Piwik\Tests\Framework\Fixture;
  *
  * @group Core
  * @group Core_CliMultiTest
+ * @group CliMulti
  */
 class Core_CliMultiTest extends SystemTestCase
 {

tests/PHPUnit/Unit/CliMulti/OutputTest.php

@@ -9,12 +9,15 @@
 namespace Piwik\Tests\Unit\CliMulti;
 
 use Piwik\CliMulti\Output;
+use Piwik\Tests\Framework\Mock\File;
+use Piwik\Tests\Framework\TestCase\UnitTestCase;
 use Piwik\Url;
 
 /**
  * @group Core
+ * @group CliMulti
  */
-class OutputTest extends \PHPUnit_Framework_TestCase
+class OutputTest extends UnitTestCase
 {
     /**
      * @var Output
@@ -41,6 +44,11 @@ class OutputTest extends \PHPUnit_Framework_TestCase
         new Output('../../');
     }
 
+    public function test_getOutputId()
+    {
+        $this->assertSame('myid', $this->output->getOutputId());
+    }
+
     public function test_exists_ShouldReturnsFalse_IfNothingWrittenYet()
     {
         $this->assertFalse($this->output->exists());
@@ -54,6 +62,27 @@ class OutputTest extends \PHPUnit_Framework_TestCase
         $this->assertGreaterThan(strlen($expectedEnd), strlen($this->output->getPathToFile()));
     }
 
+    public function test_isAbormal_ShouldReturnFalse_IfFileDoesNotExist()
+    {
+        $this->assertFalse($this->output->isAbnormal());
+    }
+
+    public function test_isAbormal_ShouldReturnTrue_IfFilesizeIsNotTooBig()
+    {
+        File::setFileSize(1024 * 1024 * 99);
+        File::setFileExists(true);
+
+        $this->assertFalse($this->output->isAbnormal());
+    }
+
+    public function test_isAbormal_ShouldReturnTrue_IfFilesizeIsTooBig()
+    {
+        File::setFileSize(1024 * 1024 * 101);
+        File::setFileExists(true);
+
+        $this->assertTrue($this->output->isAbnormal());
+    }
+
     public function test_exists_ShouldReturnTrue_IfSomethingIsWritten()
     {
         $this->output->write('test');

tests/PHPUnit/Unit/CliMulti/ProcessTest.php

@@ -9,12 +9,15 @@
 namespace Piwik\Tests\Unit\CliMulti;
 
 use Piwik\CliMulti\Process;
+use Piwik\Tests\Framework\Mock\File;
+use Piwik\Tests\Framework\TestCase\UnitTestCase;
 use ReflectionProperty;
 
 /**
  * @group Core
+ * @group CliMulti
  */
-class ProcessTest extends \PHPUnit_Framework_TestCase
+class ProcessTest extends UnitTestCase
 {
     /**
      * @var Process
@@ -23,6 +26,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
 
     public function setUp()
     {
+        parent::setup();
         $this->process = new Process('testPid');
     }
 
@@ -40,6 +44,11 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
         new Process('../../htaccess');
     }
 
+    public function test_getPid()
+    {
+        $this->assertSame('testPid', $this->process->getPid());
+    }
+
     public function test_construct_shouldBeNotStarted_IfPidJustCreated()
     {
         $this->assertFalse($this->process->hasStarted());
@@ -85,6 +94,22 @@ class ProcessTest extends \PHPUnit_Framework_TestCase
         $this->assertTrue($this->process->hasFinished());
     }
 
+    public function test_isRunning_ShouldMarkProcessAsFinished_IfPidFileIsTooBig()
+    {
+        if (! Process::isSupported()) {
+            $this->markTestSkipped('Not supported');
+        }
+
+        $this->process->startProcess();
+        $this->assertTrue($this->process->isRunning());
+        $this->assertFalse($this->process->hasFinished());
+
+        File::setFileSize(505);
+
+        $this->assertFalse($this->process->isRunning());
+        $this->assertTrue($this->process->hasFinished());
+    }
+
     public function test_finishProcess_ShouldNotThrowError_IfNotStartedBefore()
     {
         $this->process->finishProcess();

tests/PHPUnit/Unit/FilesystemTest.php

@@ -9,11 +9,13 @@
 namespace Piwik\Tests\Unit;
 
 use Piwik\Filesystem;
+use Piwik\Tests\Framework\Mock\File;
+use Piwik\Tests\Framework\TestCase\UnitTestCase;
 
 /**
  * @group Core
  */
-class FilesystemTest extends \PHPUnit_Framework_TestCase
+class FilesystemTest extends UnitTestCase
 {
     private $testPath;
 
@@ -245,4 +247,102 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase
         return $this->testPath . '/target';
     }
 
+    public function test_getFileSize_ZeroSize()
+    {
+        File::setFileSize(0);
+
+        $size = Filesystem::getFileSize(__FILE__);
+        $this->assertEquals(0, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'KB');
+        $this->assertEquals(0, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'MB');
+        $this->assertEquals(0, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'GB');
+        $this->assertEquals(0, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'TB');
+        $this->assertEquals(0, $size);
+    }
+
+    public function test_getFileSize_LowSize()
+    {
+        File::setFileSize(1024);
+
+        $size = Filesystem::getFileSize(__FILE__);
+        $this->assertEquals(1024, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'KB');
+        $this->assertEquals(1, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'MB');
+        $this->assertGreaterThanOrEqual(0.0009, $size);
+        $this->assertLessThanOrEqual(0.0011, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'GB');
+        $this->assertGreaterThanOrEqual(0.0000009, $size);
+        $this->assertLessThanOrEqual(0.0000011, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'TB');
+        $this->assertGreaterThanOrEqual(0.0000000009, $size);
+        $this->assertLessThanOrEqual(0.0000000011, $size);
+    }
+
+    public function test_getFileSize_HighSize()
+    {
+        File::setFileSize(1073741824);
+
+        $size = Filesystem::getFileSize(__FILE__, 'B');
+        $this->assertEquals(1073741824, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'KB');
+        $this->assertEquals(1048576, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'MB');
+        $this->assertEquals(1024, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'GB');
+        $this->assertEquals(1, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'TB');
+        $this->assertGreaterThanOrEqual(0.0009, $size);
+        $this->assertLessThanOrEqual(0.0011, $size);
+    }
+
+    public function test_getFileSize_ShouldRecognizeLowerUnits()
+    {
+        File::setFileSize(1073741824);
+
+        $size = Filesystem::getFileSize(__FILE__, 'b');
+        $this->assertEquals(1073741824, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'kb');
+        $this->assertEquals(1048576, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'mB');
+        $this->assertEquals(1024, $size);
+
+        $size = Filesystem::getFileSize(__FILE__, 'Gb');
+        $this->assertEquals(1, $size);
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage Invalid unit given
+     */
+    public function test_getFileSize_ShouldThrowException_IfInvalidUnit()
+    {
+        Filesystem::getFileSize(__FILE__, 'iV');
+    }
+
+    public function test_getFileSize_ShouldReturnNull_IfFileDoesNotExists()
+    {
+        File::setFileExists(false);
+        $size = Filesystem::getFileSize(__FILE__);
+
+        $this->assertNull($size);
+    }
+
 }
\ No newline at end of file