From 320b0f19ef4b1cbe4fffaba5488a4a2d518313fd Mon Sep 17 00:00:00 2001
From: Thomas Steur <thomas.steur@googlemail.com>
Date: Thu, 13 Nov 2014 02:19:20 +0100
Subject: [PATCH] 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 | 8 ++
core/CliMulti/Output.php | 16 ++-
core/CliMulti/Process.php | 19 ++++
core/Filesystem.php | 34 ++++++
.../ExamplePlugin/tests/Unit/SimpleTest.php | 4 +-
tests/PHPUnit/Framework/Mock/File.php | 62 +++++++++++
.../Framework/TestCase/UnitTestCase.php | 31 ++++++
tests/PHPUnit/Integration/FilesystemTest.php | 33 ++++++
tests/PHPUnit/System/CliMultiTest.php | 1 +
tests/PHPUnit/Unit/CliMulti/OutputTest.php | 31 +++++-
tests/PHPUnit/Unit/CliMulti/ProcessTest.php | 27 ++++-
tests/PHPUnit/Unit/FilesystemTest.php | 102 +++++++++++++++++-
12 files changed, 363 insertions(+), 5 deletions(-)
create mode 100644 tests/PHPUnit/Framework/Mock/File.php
create mode 100755 tests/PHPUnit/Framework/TestCase/UnitTestCase.php
create mode 100644 tests/PHPUnit/Integration/FilesystemTest.php
diff --git a/core/CliMulti.php b/core/CliMulti.php
index 992a4e1dd4..26befbc8ef 100644
--- a/core/CliMulti.php
+++ b/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]);
diff --git a/core/CliMulti/Output.php b/core/CliMulti/Output.php
index 67ecf3543b..e4feb1a805 100644
--- a/core/CliMulti/Output.php
+++ b/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);
diff --git a/core/CliMulti/Process.php b/core/CliMulti/Process.php
index 15dc539e59..cd75a1e6ef 100644
--- a/core/CliMulti/Process.php
+++ b/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);
diff --git a/core/Filesystem.php b/core/Filesystem.php
index 8f34c6a1c8..159c518cc9 100644
--- a/core/Filesystem.php
+++ b/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
diff --git a/plugins/ExamplePlugin/tests/Unit/SimpleTest.php b/plugins/ExamplePlugin/tests/Unit/SimpleTest.php
index 4b5341096f..4e0958d239 100644
--- a/plugins/ExamplePlugin/tests/Unit/SimpleTest.php
+++ b/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
{
/**
diff --git a/tests/PHPUnit/Framework/Mock/File.php b/tests/PHPUnit/Framework/Mock/File.php
new file mode 100644
index 0000000000..aa17325a94
--- /dev/null
+++ b/tests/PHPUnit/Framework/Mock/File.php
@@ -0,0 +1,62 @@
+<?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;
+ }
+}
diff --git a/tests/PHPUnit/Framework/TestCase/UnitTestCase.php b/tests/PHPUnit/Framework/TestCase/UnitTestCase.php
new file mode 100755
index 0000000000..e12bec330f
--- /dev/null
+++ b/tests/PHPUnit/Framework/TestCase/UnitTestCase.php
@@ -0,0 +1,31 @@
+<?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();
+ }
+}
diff --git a/tests/PHPUnit/Integration/FilesystemTest.php b/tests/PHPUnit/Integration/FilesystemTest.php
new file mode 100644
index 0000000000..f3879927e5
--- /dev/null
+++ b/tests/PHPUnit/Integration/FilesystemTest.php
@@ -0,0 +1,33 @@
+<?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
diff --git a/tests/PHPUnit/System/CliMultiTest.php b/tests/PHPUnit/System/CliMultiTest.php
index 0802a7cc67..8797eeec7e 100644
--- a/tests/PHPUnit/System/CliMultiTest.php
+++ b/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
{
diff --git a/tests/PHPUnit/Unit/CliMulti/OutputTest.php b/tests/PHPUnit/Unit/CliMulti/OutputTest.php
index 6dbcf12179..16c5081a00 100644
--- a/tests/PHPUnit/Unit/CliMulti/OutputTest.php
+++ b/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');
diff --git a/tests/PHPUnit/Unit/CliMulti/ProcessTest.php b/tests/PHPUnit/Unit/CliMulti/ProcessTest.php
index 3db93081c6..68dbd41c43 100644
--- a/tests/PHPUnit/Unit/CliMulti/ProcessTest.php
+++ b/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();
diff --git a/tests/PHPUnit/Unit/FilesystemTest.php b/tests/PHPUnit/Unit/FilesystemTest.php
index 263a268bbd..4725516a84 100644
--- a/tests/PHPUnit/Unit/FilesystemTest.php
+++ b/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
--
GitLab