Merge pull request #6667 from piwik/6617

CliMulti: If process or output size is unexpected large, declare it as finished

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,13 +8,26 @@
 
 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
 {
+    public function setUp()
+    {
+        parent::setUp();
+        // set up here if needed
+    }
+    
+    public function tearDown()
+    {
+        // tear down here if needed
+        parent::tearDown();
+    }
 
     /**
      * All your actual test methods should start with the name "test"

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
@@ -23,6 +26,7 @@ class OutputTest extends \PHPUnit_Framework_TestCase
 
     public function setUp()
     {
+        parent::setup();
         Url::setHost(false);
         $this->output = new Output('myid');
     }
@@ -30,6 +34,7 @@ class OutputTest extends \PHPUnit_Framework_TestCase
     public function tearDown()
     {
         $this->output->destroy();
+        parent::tearDown();
     }
 
     /**
@@ -41,6 +46,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 +64,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