From 23fbef3daebc0837803e98340a0f9d5e40ef3a24 Mon Sep 17 00:00:00 2001
From: Laurent Destailleur <eldy@destailleur.fr>
Date: Thu, 13 Jul 2017 00:18:58 +0200
Subject: [PATCH] Fix the delete dir functions does not return real nb of
deleted files.
---
htdocs/admin/tools/purge.php | 2 +-
htdocs/core/class/utils.class.php | 26 +++++++++++++----
htdocs/core/lib/files.lib.php | 20 ++++++-------
htdocs/langs/en_US/admin.lang | 1 +
test/phpunit/FilesLibTest.php | 48 ++++++++++++++++++++-----------
5 files changed, 65 insertions(+), 32 deletions(-)
diff --git a/htdocs/admin/tools/purge.php b/htdocs/admin/tools/purge.php
index 7a5cb30ae75..bc328e951d4 100644
--- a/htdocs/admin/tools/purge.php
+++ b/htdocs/admin/tools/purge.php
@@ -31,7 +31,7 @@ if (! $user->admin)
$action=GETPOST('action','alpha');
$confirm=GETPOST('confirm','alpha');
-$choice=GETPOST('choice');
+$choice=GETPOST('choice','aZ09');
// Define filelog to discard it from purge
diff --git a/htdocs/core/class/utils.class.php b/htdocs/core/class/utils.class.php
index 2081e662abb..dee63987d45 100644
--- a/htdocs/core/class/utils.class.php
+++ b/htdocs/core/class/utils.class.php
@@ -112,21 +112,33 @@ class Utils
}
$count=0;
+ $countdeleted=0;
+ $counterror=0;
if (count($filesarray))
{
foreach($filesarray as $key => $value)
{
- //print "x ".$filesarray[$key]['fullname']."<br>\n";
+ //print "x ".$filesarray[$key]['fullname']."-".$filesarray[$key]['type']."<br>\n";
if ($filesarray[$key]['type'] == 'dir')
{
- $count+=dol_delete_dir_recursive($filesarray[$key]['fullname']);
+ $startcount=0;
+ $tmpcountdeleted=0;
+ $result=dol_delete_dir_recursive($filesarray[$key]['fullname'], $startcount, 1, 0, $tmpcountdeleted);
+ $count+=$result;
+ $countdeleted+=$tmpcountdeleted;
}
elseif ($filesarray[$key]['type'] == 'file')
{
// If (file that is not logfile) or (if logfile with option logfile)
if ($filesarray[$key]['fullname'] != $filelog || $choice=='logfile')
{
- $count+=(dol_delete_file($filesarray[$key]['fullname'])?1:0);
+ $result=dol_delete_file($filesarray[$key]['fullname'], 1, 1);
+ if ($result)
+ {
+ $count++;
+ $countdeleted++;
+ }
+ else $counterror++;
}
}
}
@@ -140,8 +152,12 @@ class Utils
}
}
- if ($count > 0) $this->output=$langs->trans("PurgeNDirectoriesDeleted", $count);
- else $this->output=$langs->trans("PurgeNothingToDelete");
+ if ($count > 0)
+ {
+ $this->output=$langs->trans("PurgeNDirectoriesDeleted", $countdeleted);
+ if ($count > $countdeleted) $this->output.='<br>'.$langs->trans("PurgeNDirectoriesFailed", ($count - $countdeleted));
+ }
+ else $this->output=$langs->trans("PurgeNothingToDelete").($choice == 'tempfilesold' ? ' (older than 24h)':'');
//return $count;
return 0; // This function can be called by cron so must return 0 if OK
diff --git a/htdocs/core/lib/files.lib.php b/htdocs/core/lib/files.lib.php
index dd21a1345e7..074b38bfc2d 100644
--- a/htdocs/core/lib/files.lib.php
+++ b/htdocs/core/lib/files.lib.php
@@ -1113,12 +1113,13 @@ function dol_delete_dir($dir,$nophperrors=0)
* Remove a directory $dir and its subdirectories (or only files and subdirectories)
*
* @param string $dir Dir to delete
- * @param int $count Counter to count nb of deleted elements
+ * @param int $count Counter to count nb of elements found to delete
* @param int $nophperrors Disable all PHP output errors
* @param int $onlysub Delete only files and subdir, not main directory
- * @return int Number of files and directory removed
+ * @param int $countdeleted Counter to count nb of elements found really deleted
+ * @return int Number of files and directory we try to remove. NB really removed is returned into $countdeleted.
*/
-function dol_delete_dir_recursive($dir,$count=0,$nophperrors=0,$onlysub=0)
+function dol_delete_dir_recursive($dir, $count=0, $nophperrors=0, $onlysub=0, &$countdeleted=0)
{
dol_syslog("functions.lib:dol_delete_dir_recursive ".$dir,LOG_DEBUG);
if (dol_is_dir($dir))
@@ -1134,13 +1135,13 @@ function dol_delete_dir_recursive($dir,$count=0,$nophperrors=0,$onlysub=0)
{
if (is_dir(dol_osencode("$dir/$item")))
{
- $count=dol_delete_dir_recursive("$dir/$item",$count,$nophperrors);
+ $count=dol_delete_dir_recursive("$dir/$item", $count, $nophperrors, 0, $countdeleted);
}
else
{
- dol_delete_file("$dir/$item",1,$nophperrors);
+ $result=dol_delete_file("$dir/$item", 1, $nophperrors);
$count++;
- //echo " removing $dir/$item<br>\n";
+ if ($result) $countdeleted++;
}
}
}
@@ -1148,14 +1149,13 @@ function dol_delete_dir_recursive($dir,$count=0,$nophperrors=0,$onlysub=0)
if (empty($onlysub))
{
- dol_delete_dir($dir,$nophperrors);
- $count++;
- //echo "removing $dir<br>\n";
+ $result=dol_delete_dir($dir, $nophperrors);
+ $count++;
+ if ($result) $countdeleted++;
}
}
}
- //echo "return=".$count;
return $count;
}
diff --git a/htdocs/langs/en_US/admin.lang b/htdocs/langs/en_US/admin.lang
index a7032d3a945..cd3aa50c439 100644
--- a/htdocs/langs/en_US/admin.lang
+++ b/htdocs/langs/en_US/admin.lang
@@ -147,6 +147,7 @@ PurgeDeleteAllFilesInDocumentsDir=Delete all files in directory <b>%s</b>. Tempo
PurgeRunNow=Purge now
PurgeNothingToDelete=No directory or files to delete.
PurgeNDirectoriesDeleted=<b>%s</b> files or directories deleted.
+PurgeNDirectoriesFailed=Failed to delete <b>%s</b> files or directories.
PurgeAuditEvents=Purge all security events
ConfirmPurgeAuditEvents=Are you sure you want to purge all security events? All security logs will be deleted, no other data will be removed.
GenerateBackup=Generate backup
diff --git a/test/phpunit/FilesLibTest.php b/test/phpunit/FilesLibTest.php
index c0a4ce845a9..feac8b212ed 100644
--- a/test/phpunit/FilesLibTest.php
+++ b/test/phpunit/FilesLibTest.php
@@ -286,11 +286,27 @@ class FilesLibTest extends PHPUnit_Framework_TestCase
$db=$this->savdb;
$dirout=$conf->admin->dir_temp.'/test';
+ $dirout2=$conf->admin->dir_temp.'/test2';
$count=0;
- $result=dol_delete_dir_recursive($dirout,$count,1); // If it has no permission to delete, it will fails as if dir does not exists, so we can't test it
+ $result=dol_delete_dir_recursive($dirout,$count); // If it has no permission to delete, it will fails as if dir does not exists, so we can't test it
print __METHOD__." result=".$result."\n";
$this->assertGreaterThanOrEqual(0,$result);
+
+ $count=0;
+ $countdeleted=0;
+ $result=dol_delete_dir_recursive($dirout,$count,1,0,$countdeleted); // If it has no permission to delete, it will fails as if dir does not exists, so we can't test it
+ print __METHOD__." result=".$result."\n";
+ $this->assertGreaterThanOrEqual(0,$result);
+ $this->assertGreaterThanOrEqual(0,$countdeleted);
+
+ dol_mkdir($dirout2);
+ $count=0;
+ $countdeleted=0;
+ $result=dol_delete_dir_recursive($dirout2,$count,1,0,$countdeleted); // If it has no permission to delete, it will fails as if dir does not exists, so we can't test it
+ print __METHOD__." result=".$result."\n";
+ $this->assertGreaterThanOrEqual(1,$result);
+ $this->assertGreaterThanOrEqual(1,$countdeleted);
}
@@ -399,7 +415,7 @@ class FilesLibTest extends PHPUnit_Framework_TestCase
print __METHOD__." result=".join(',',$result)."\n";
$this->assertEquals(0,count($result));
}
-
+
/**
* testDolDirList
*
@@ -411,7 +427,7 @@ class FilesLibTest extends PHPUnit_Framework_TestCase
public function testDolDirList()
{
global $conf,$user,$langs,$db;
-
+
// Scan dir to guaruante we on't have library jquery twice (we accept exception of duplicte into ckeditor because all dir is removed for debian package, so there is no duplicate).
$founddirs=dol_dir_list(DOL_DOCUMENT_ROOT.'/includes/', 'files', 1, '^jquery\.js', array('ckeditor'));
print __METHOD__." count(founddirs)=".count($founddirs)."\n";
@@ -431,60 +447,60 @@ class FilesLibTest extends PHPUnit_Framework_TestCase
$user=$this->savuser;
$langs=$this->savlangs;
$db=$this->savdb;
-
-
+
+
//$dummyuser=new User($db);
//$result=restrictedArea($dummyuser,'societe');
// We save user properties
$savpermlire = $user->rights->facture->lire;
$savpermcreer = $user->rights->facture->creer;
-
-
+
+
// Check access to SPECIMEN
$user->rights->facture->lire = 0;
$user->rights->facture->creer = 0;
$filename='SPECIMEN.pdf'; // Filename relative to module part
$result=dol_check_secure_access_document('facture', $filename, 0, '', '', 'read');
$this->assertEquals(1,$result['accessallowed']);
-
-
+
+
// Check read permission
$user->rights->facture->lire = 1;
$user->rights->facture->creer = 1;
$filename='FA010101/FA010101.pdf'; // Filename relative to module part
$result=dol_check_secure_access_document('facture', $filename, 0, '', '', 'read');
$this->assertEquals(1,$result['accessallowed']);
-
+
$user->rights->facture->lire = 0;
$user->rights->facture->creer = 0;
$filename='FA010101/FA010101.pdf'; // Filename relative to module part
$result=dol_check_secure_access_document('facture', $filename, 0, '', '', 'read');
$this->assertEquals(0,$result['accessallowed']);
-
+
// Check write permission
$user->rights->facture->lire = 0;
$user->rights->facture->creer = 0;
$filename='FA010101/FA010101.pdf'; // Filename relative to module part
$result=dol_check_secure_access_document('facture', $filename, 0, '', '', 'write');
$this->assertEquals(0,$result['accessallowed']);
-
+
$user->rights->facture->lire = 1;
$user->rights->facture->creer = 1;
$filename='FA010101/FA010101.pdf'; // Filename relative to module part
$result=dol_check_secure_access_document('facture', $filename, 0, '', '', 'write');
$this->assertEquals(1,$result['accessallowed']);
-
+
$user->rights->facture->lire = 1;
$user->rights->facture->creer = 0;
$filename='FA010101/FA010101.pdf'; // Filename relative to module part
$result=dol_check_secure_access_document('facture', $filename, 0, '', '', 'write');
$this->assertEquals(0,$result['accessallowed']);
-
-
+
+
// We restore user properties
$user->rights->facture->lire = $savpermlire;
$user->rights->facture->creer = $savpermcreer;
- }
+ }
}
--
GitLab