From f7853cf439df613838d7c3fe710f423956f943a2 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Fri, 24 Mar 2017 23:42:11 +0100 Subject: [PATCH] removing duplicate code, cleanup of temporary test files --- lib/Data/Filesystem.php | 74 +++++++++-------------------------- lib/Persistence/DataStore.php | 48 +++++++++++++++++++++++ tst/Bootstrap.php | 28 +++++++------ tst/Data/FilesystemTest.php | 30 -------------- tst/JsonApiTest.php | 2 + tst/RequestTest.php | 1 + 6 files changed, 85 insertions(+), 98 deletions(-) create mode 100644 lib/Persistence/DataStore.php diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 393d3d80..09844052 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -12,9 +12,8 @@ namespace PrivateBin\Data; -use Exception; -use PrivateBin\Json; use PrivateBin\Model\Paste; +use PrivateBin\Persistence\DataStore; /** * Filesystem @@ -23,15 +22,6 @@ use PrivateBin\Model\Paste; */ class Filesystem extends AbstractData { - /** - * directory where data is stored - * - * @access private - * @static - * @var string - */ - private static $_dir = 'data/'; - /** * get instance of singleton * @@ -51,8 +41,7 @@ class Filesystem extends AbstractData is_array($options) && array_key_exists('dir', $options) ) { - self::$_dir = $options['dir'] . DIRECTORY_SEPARATOR; - self::_init(); + DataStore::setPath($options['dir']); } return self::$_instance; } @@ -63,19 +52,19 @@ class Filesystem extends AbstractData * @access public * @param string $pasteid * @param array $paste - * @throws Exception * @return bool */ public function create($pasteid, $paste) { $storagedir = self::_dataid2path($pasteid); - if (is_file($storagedir . $pasteid)) { + $file = $storagedir . $pasteid; + if (is_file($file)) { return false; } if (!is_dir($storagedir)) { mkdir($storagedir, 0700, true); } - return (bool) file_put_contents($storagedir . $pasteid, Json::encode($paste)); + return DataStore::store($file, $paste); } /** @@ -156,20 +145,19 @@ class Filesystem extends AbstractData * @param string $parentid * @param string $commentid * @param array $comment - * @throws Exception * @return bool */ public function createComment($pasteid, $parentid, $commentid, $comment) { $storagedir = self::_dataid2discussionpath($pasteid); - $filename = $pasteid . '.' . $commentid . '.' . $parentid; - if (is_file($storagedir . $filename)) { + $file = $storagedir . $pasteid . '.' . $commentid . '.' . $parentid; + if (is_file($file)) { return false; } if (!is_dir($storagedir)) { mkdir($storagedir, 0700, true); } - return (bool) file_put_contents($storagedir . $filename, Json::encode($comment)); + return DataStore::store($file, $comment); } /** @@ -238,8 +226,9 @@ class Filesystem extends AbstractData protected function _getExpiredPastes($batchsize) { $pastes = array(); + $mainpath = DataStore::getPath(); $firstLevel = array_filter( - scandir(self::$_dir), + scandir($mainpath), 'self::_isFirstLevelDir' ); if (count($firstLevel) > 0) { @@ -247,7 +236,7 @@ class Filesystem extends AbstractData for ($i = 0, $max = $batchsize * 10; $i < $max; ++$i) { $firstKey = array_rand($firstLevel); $secondLevel = array_filter( - scandir(self::$_dir . $firstLevel[$firstKey]), + scandir($mainpath . DIRECTORY_SEPARATOR . $firstLevel[$firstKey]), 'self::_isSecondLevelDir' ); @@ -258,8 +247,9 @@ class Filesystem extends AbstractData } $secondKey = array_rand($secondLevel); - $path = self::$_dir . $firstLevel[$firstKey] . - DIRECTORY_SEPARATOR . $secondLevel[$secondKey]; + $path = $mainpath . DIRECTORY_SEPARATOR . + $firstLevel[$firstKey] . DIRECTORY_SEPARATOR . + $secondLevel[$secondKey]; if (!is_dir($path)) { continue; } @@ -293,34 +283,6 @@ class Filesystem extends AbstractData return $pastes; } - /** - * Initialize data store - * - * @access private - * @static - * @return void - */ - private static function _init() - { - // Create storage directory if it does not exist. - if (!is_dir(self::$_dir)) { - if (!@mkdir(self::$_dir, 0700)) { - throw new Exception('unable to create directory ' . self::$_dir, 10); - } - } - $file = self::$_dir . DIRECTORY_SEPARATOR . '.htaccess'; - if (!is_file($file)) { - $writtenBytes = @file_put_contents( - $file, - 'Require all denied' . PHP_EOL, - LOCK_EX - ); - if ($writtenBytes === false || $writtenBytes < 19) { - throw new Exception('unable to write to file ' . $file, 11); - } - } - } - /** * Convert paste id to storage path. * @@ -338,8 +300,10 @@ class Filesystem extends AbstractData */ private static function _dataid2path($dataid) { - return self::$_dir . substr($dataid, 0, 2) . DIRECTORY_SEPARATOR . - substr($dataid, 2, 2) . DIRECTORY_SEPARATOR; + return DataStore::getPath( + substr($dataid, 0, 2) . DIRECTORY_SEPARATOR . + substr($dataid, 2, 2) . DIRECTORY_SEPARATOR + ); } /** @@ -369,7 +333,7 @@ class Filesystem extends AbstractData private static function _isFirstLevelDir($element) { return self::_isSecondLevelDir($element) && - is_dir(self::$_dir . DIRECTORY_SEPARATOR . $element); + is_dir(DataStore::getPath($element)); } /** diff --git a/lib/Persistence/DataStore.php b/lib/Persistence/DataStore.php new file mode 100644 index 00000000..27c7131d --- /dev/null +++ b/lib/Persistence/DataStore.php @@ -0,0 +1,48 @@ +read())) { - if ($file != '.' && $file != '..') { - if (is_dir($path . $file)) { - self::rmDir($path . $file); - } elseif (is_file($path . $file)) { - if (!unlink($path . $file)) { - throw new Exception('Error deleting file "' . $path . $file . '".'); + if (is_dir($path)) { + $path .= DIRECTORY_SEPARATOR; + $dir = dir($path); + while (false !== ($file = $dir->read())) { + if ($file != '.' && $file != '..') { + if (is_dir($path . $file)) { + self::rmDir($path . $file); + } elseif (is_file($path . $file)) { + if (!unlink($path . $file)) { + throw new Exception('Error deleting file "' . $path . $file . '".'); + } } } } - } - $dir->close(); - if (!rmdir($path)) { - throw new Exception('Error deleting directory "' . $path . '".'); + $dir->close(); + if (!rmdir($path)) { + throw new Exception('Error deleting directory "' . $path . '".'); + } } } diff --git a/tst/Data/FilesystemTest.php b/tst/Data/FilesystemTest.php index fe012c41..e7e6dc82 100644 --- a/tst/Data/FilesystemTest.php +++ b/tst/Data/FilesystemTest.php @@ -110,10 +110,6 @@ class FilesystemTest extends PHPUnit_Framework_TestCase } } - /** - * @expectedException Exception - * @expectedExceptionCode 90 - */ public function testErrorDetection() { $this->_model->delete(Helper::getPasteId()); @@ -123,10 +119,6 @@ class FilesystemTest extends PHPUnit_Framework_TestCase $this->assertFalse($this->_model->exists(Helper::getPasteId()), 'paste does still not exist'); } - /** - * @expectedException Exception - * @expectedExceptionCode 90 - */ public function testCommentErrorDetection() { $this->_model->delete(Helper::getPasteId()); @@ -138,26 +130,4 @@ class FilesystemTest extends PHPUnit_Framework_TestCase $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment), 'unable to store broken comment'); $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does still not exist'); } - - /** - * @expectedException Exception - * @expectedExceptionCode 10 - */ - public function testPermissionShenanigans() - { - // try creating an invalid path - chmod($this->_invalidPath, 0000); - Filesystem::getInstance(array('dir' => $this->_invalidPath . DIRECTORY_SEPARATOR . 'baz')); - } - - /** - * @expectedException Exception - * @expectedExceptionCode 11 - */ - public function testPathShenanigans() - { - // try setting an invalid path - chmod($this->_invalidPath, 0000); - Filesystem::getInstance(array('dir' => $this->_invalidPath)); - } } diff --git a/tst/JsonApiTest.php b/tst/JsonApiTest.php index 5cf13608..a5928893 100644 --- a/tst/JsonApiTest.php +++ b/tst/JsonApiTest.php @@ -98,6 +98,7 @@ class JsonApiTest extends PHPUnit_Framework_TestCase new PrivateBin; $content = ob_get_contents(); ob_end_clean(); + unlink($file); $response = json_decode($content, true); $this->assertEquals(0, $response['status'], 'outputs status'); $this->assertEquals(Helper::getPasteId(), $response['id'], 'outputted paste ID matches input'); @@ -132,6 +133,7 @@ class JsonApiTest extends PHPUnit_Framework_TestCase new PrivateBin; $content = ob_get_contents(); ob_end_clean(); + unlink($file); $response = json_decode($content, true); $this->assertEquals(0, $response['status'], 'outputs status'); $this->assertFalse($this->_model->exists(Helper::getPasteId()), 'paste successfully deleted'); diff --git a/tst/RequestTest.php b/tst/RequestTest.php index f20209f5..29b0dade 100644 --- a/tst/RequestTest.php +++ b/tst/RequestTest.php @@ -63,6 +63,7 @@ class RequestTest extends PHPUnit_Framework_TestCase file_put_contents($file, 'data=foo'); Request::setInputStream($file); $request = new Request; + unlink($file); $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call'); $this->assertEquals('create', $request->getOperation()); $this->assertEquals('foo', $request->getParam('data'));