prefer DirectoryIterator for readability, also test jbo translation, log deletion errors

lib/Data/Database.php

@@ -400,7 +400,9 @@ class Database extends AbstractData
                 $fs    = new Filesystem(array('dir' => 'data'));
                 $value = $fs->getValue('salt');
                 $this->setValue($value, 'salt');
-                unlink($file);
+                if (!unlink($file)) {
+                    error_log('Error deleting migrated salt: ' . $file);
+                }
                 return $value;
             }
         }

lib/Data/Filesystem.php

@@ -11,6 +11,7 @@
 
 namespace PrivateBin\Data;
 
+use DirectoryIterator;
 use GlobIterator;
 use PrivateBin\Exception\JsonException;
 use PrivateBin\Json;
@@ -121,21 +122,24 @@ class Filesystem extends AbstractData
         $pastedir = $this->_dataid2path($pasteid);
         if (is_dir($pastedir)) {
             // Delete the paste itself.
-            if (is_file($pastedir . $pasteid . '.php')) {
-                unlink($pastedir . $pasteid . '.php');
+            $pastefile = $pastedir . $pasteid . '.php';
+            if (is_file($pastefile)) {
+                if (!unlink($pastefile)) {
+                    error_log('Error deleting paste: ' . $pastefile);
+                }
             }
 
             // Delete discussion if it exists.
             $discdir = $this->_dataid2discussionpath($pasteid);
             if (is_dir($discdir)) {
                 // Delete all files in discussion directory
-                $dir = dir($discdir);
-                while (false !== ($filename = $dir->read())) {
-                    if (is_file($discdir . $filename)) {
-                        unlink($discdir . $filename);
+                foreach (new DirectoryIterator($discdir) as $file) {
+                    if ($file->isFile()) {
+                        if (!unlink($file->getPathname())) {
+                            error_log('Error deleting comment: ' . $file->getPathname());
+                        }
                     }
                 }
-                $dir->close();
                 rmdir($discdir);
             }
         }
@@ -159,14 +163,11 @@ class Filesystem extends AbstractData
             // convert comments, too
             $discdir = $this->_dataid2discussionpath($pasteid);
             if (is_dir($discdir)) {
-                $dir = dir($discdir);
-                while (false !== ($filename = $dir->read())) {
-                    if (substr($filename, -4) !== '.php' && strlen($filename) >= 16) {
-                        $commentFilename = $discdir . $filename . '.php';
-                        $this->_prependRename($discdir . $filename, $commentFilename);
+                foreach (new DirectoryIterator($discdir) as $file) {
+                    if ($file->getExtension() !== 'php' && strlen($file->getFilename()) >= 16) {
+                        $this->_prependRename($file->getPathname(), $file->getPathname() . '.php');
                     }
                 }
-                $dir->close();
             }
         }
         return is_readable($pastePath);
@@ -207,15 +208,14 @@ class Filesystem extends AbstractData
         $comments = array();
         $discdir  = $this->_dataid2discussionpath($pasteid);
         if (is_dir($discdir)) {
-            $dir = dir($discdir);
-            while (false !== ($filename = $dir->read())) {
+            foreach (new DirectoryIterator($discdir) as $file) {
                 // Filename is in the form pasteid.commentid.parentid.php:
                 // - pasteid is the paste this reply belongs to.
                 // - commentid is the comment identifier itself.
                 // - parentid is the comment this comment replies to (It can be pasteid)
-                if (is_file($discdir . $filename)) {
-                    $comment = $this->_get($discdir . $filename);
-                    $items   = explode('.', $filename);
+                if ($file->isFile()) {
+                    $comment = $this->_get($file->getPathname());
+                    $items   = explode('.', $file->getBasename('.php'));
                     // Add some meta information not contained in file.
                     $comment['id']       = $items[1];
                     $comment['parentid'] = $items[2];
@@ -228,7 +228,6 @@ class Filesystem extends AbstractData
                     $comments[$key] = $comment;
                 }
             }
-            $dir->close();
 
             // Sort comments by date, oldest first.
             ksort($comments);
@@ -522,6 +521,8 @@ class Filesystem extends AbstractData
             file_put_contents($destFile, $handle, FILE_APPEND);
             fclose($handle);
         }
-        unlink($srcFile);
+        if (!unlink($srcFile)) {
+            error_log('Error deleting converted document: ' . $srcFile);
+        }
     }
 }

tst/I18nTest.php

@@ -2,6 +2,7 @@
 
 use PHPUnit\Framework\TestCase;
 use PrivateBin\I18n;
+use PrivateBin\Json;
 
 class I18nMock extends I18n
 {
@@ -221,19 +222,19 @@ class I18nTest extends TestCase
     {
         $messageIds = array();
         $languages  = array();
-        $dir        = dir(PATH . 'i18n');
-        while (false !== ($file = $dir->read())) {
-            if (strlen($file) === 7) {
-                $language            = substr($file, 0, 2);
-                $languageMessageIds  = array_keys(
-                    json_decode(
-                        file_get_contents(PATH . 'i18n' . DIRECTORY_SEPARATOR . $file),
-                        true
-                    )
-                );
-                $messageIds           = array_unique(array_merge($messageIds, $languageMessageIds));
-                $languages[$language] = $languageMessageIds;
+        foreach (new DirectoryIterator(PATH . 'i18n') as $file) {
+            $fileNameLength = strlen($file->getFilename());
+            if ($fileNameLength === 7) {       // xx.json
+                $language = substr($file->getFilename(), 0, 2);
+            } elseif ($fileNameLength === 8) { // jbo.json
+                $language = substr($file->getFilename(), 0, 3);
+            } else {
+                continue;
             }
+            $languageJson         = file_get_contents($file->getPathname());
+            $languageMessageIds   = array_keys(Json::decode($languageJson));
+            $messageIds           = array_unique(array_merge($messageIds, $languageMessageIds));
+            $languages[$language] = $languageMessageIds;
         }
         foreach ($messageIds as $messageId) {
             foreach (array_keys($languages) as $language) {

tst/ViewTest.php

@@ -67,10 +67,9 @@ class ViewTest extends TestCase
         $page->assign('CSPHEADER', 'default-src \'none\'');
         $page->assign('SRI', array());
 
-        $dir = dir(PATH . 'tpl');
-        while (false !== ($file = $dir->read())) {
-            if (substr($file, -4) === '.php') {
-                $template = substr($file, 0, -4);
+        foreach (new DirectoryIterator(PATH . 'tpl') as $file) {
+            if ($file->getExtension() === 'php') {
+                $template = $file->getBasename('.php');
                 ob_start();
                 $page->draw($template);
                 $this->_content[$template] = ob_get_contents();