Skip to content

chore: move rmdirr implementation from OC legacy to OCP\Files #52812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
skjnldsv merged 1 commit into from
May 14, 2025
Merged

chore: move rmdirr implementation from OC legacy to OCP\Files #52812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\QueuedJob;
use OCP\Files;
use OCP\IConfig;
use Psr\Log\LoggerInterface;

Expand Down Expand Up @@ -70,7 +71,7 @@ public function run($argument): void {

foreach ($dirList as $dir) {
$this->log->info("Removing $dir ...");
$result = \OC_Helper::rmdirr($dir);
$result = Files::rmdirr($dir);
if (!$result) {
$this->log->error('Could not remove updater backup folder $dir');
}
Expand Down
8 changes: 6 additions & 2 deletions lib/private/Files/Storage/Temporary.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,20 @@
*/
namespace OC\Files\Storage;

use OCP\Files;
use OCP\ITempManager;
use OCP\Server;

/**
* local storage backend in temporary folder for testing purpose
*/
class Temporary extends Local {
public function __construct(array $parameters = []) {
parent::__construct(['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]);
parent::__construct(['datadir' => Server::get(ITempManager::class)->getTemporaryFolder()]);
}

public function cleanUp(): void {
\OC_Helper::rmdirr($this->datadir);
Files::rmdirr($this->datadir);
}

public function __destruct() {
Expand Down
7 changes: 4 additions & 3 deletions lib/private/Installer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use OC_App;
use OC_Helper;
use OCP\App\IAppManager;
use OCP\Files;
use OCP\HintException;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
Expand Down Expand Up @@ -324,14 +325,14 @@ public function downloadApp(string $appId, bool $allowUnstable = false): void {

$baseDir = OC_App::getInstallPath() . '/' . $appId;
// Remove old app with the ID if existent
OC_Helper::rmdirr($baseDir);
Files::rmdirr($baseDir);
// Move to app folder
if (@mkdir($baseDir)) {
$extractDir .= '/' . $folders[0];
OC_Helper::copyr($extractDir, $baseDir);
}
OC_Helper::copyr($extractDir, $baseDir);
OC_Helper::rmdirr($extractDir);
Files::rmdirr($extractDir);
return;
}
// Signature does not match
Expand Down Expand Up @@ -450,7 +451,7 @@ public function removeApp(string $appId): bool {
return false;
}
$appDir = OC_App::getInstallPath() . '/' . $appId;
OC_Helper::rmdirr($appDir);
Files::rmdirr($appDir);
return true;
} else {
$this->logger->error('can\'t remove app ' . $appId . '. It is not installed.');
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Repair/MoveUpdaterStepFile.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
namespace OC\Repair;

use OCP\Files;
use OCP\Migration\IOutput;
use OCP\Migration\IRepairStep;

Expand Down Expand Up @@ -40,7 +41,7 @@ public function run(IOutput $output) {

// cleanup
if (file_exists($previousStepFile)) {
if (\OC_Helper::rmdirr($previousStepFile)) {
if (Files::rmdirr($previousStepFile)) {
$output->info('.step-previous-update removed');
} else {
$output->info('.step-previous-update can\'t be removed - abort move of .step file');
Expand Down
3 changes: 2 additions & 1 deletion lib/private/TempManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace OC;

use bantu\IniGetWrapper\IniGetWrapper;
use OCP\Files;
use OCP\IConfig;
use OCP\ITempManager;
use OCP\Security\ISecureRandom;
Expand Down Expand Up @@ -99,7 +100,7 @@ protected function cleanFiles($files) {
foreach ($files as $file) {
if (file_exists($file)) {
try {
\OC_Helper::rmdirr($file);
Files::rmdirr($file);
} catch (\UnexpectedValueException $ex) {
$this->log->warning(
'Error deleting temporary file/folder: {file} - Reason: {error}',
Expand Down
31 changes: 2 additions & 29 deletions lib/private/legacy/OC_Helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,37 +144,10 @@ public static function copyr($src, $dest) {
* @param string $dir path to the folder
* @param bool $deleteSelf if set to false only the content of the folder will be deleted
* @return bool
* @deprecated 5.0.0 use \OCP\Files::rmdirr instead
*/
public static function rmdirr($dir, $deleteSelf = true) {
if (is_dir($dir)) {
$files = new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($dir, RecursiveDirectoryIterator::SKIP_DOTS),
RecursiveIteratorIterator::CHILD_FIRST
);

foreach ($files as $fileInfo) {
/** @var SplFileInfo $fileInfo */
if ($fileInfo->isLink()) {
unlink($fileInfo->getPathname());
} elseif ($fileInfo->isDir()) {
rmdir($fileInfo->getRealPath());
} else {
unlink($fileInfo->getRealPath());
}
}
if ($deleteSelf) {
rmdir($dir);
}
} elseif (file_exists($dir)) {
if ($deleteSelf) {
unlink($dir);
}
}
if (!$deleteSelf) {
return true;
}

return !file_exists($dir);
return \OCP\Files::rmdirr($dir, $deleteSelf);
}

/**
Expand Down
36 changes: 34 additions & 2 deletions lib/public/Files.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,44 @@
class Files {
/**
* Recursive deletion of folders
*
* @param string $dir path to the folder
* @param bool $deleteSelf if set to false only the content of the folder will be deleted
* @return bool
* @since 5.0.0
* @since 32.0.0 added the $deleteSelf parameter
* @deprecated 14.0.0
Copy link
Contributor Author

@susnux susnux May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this one already exists in OCP @icewind1991.
I only moved the implementation of it.

Copy link
Member

@icewind1991 icewind1991 May 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, missed that part 🙈

My point about having a ~7y old deprecated method that is apparently still useful enough to keep around still stands. So I would still like to see a cleaned up version of the method.

Copy link
Contributor Author

@susnux susnux May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we need it in public scope, its only used in private stuff.
Also accessing local files only makes sense in private stuff, public / apps should use app data or similar.

*/
public static function rmdirr($dir) {
return \OC_Helper::rmdirr($dir);
public static function rmdirr($dir, bool $deleteSelf = true) {
if (is_dir($dir)) {
$files = new \RecursiveIteratorIterator(
new \RecursiveDirectoryIterator($dir, \RecursiveDirectoryIterator::SKIP_DOTS),
\RecursiveIteratorIterator::CHILD_FIRST
);

foreach ($files as $fileInfo) {
/** @var \SplFileInfo $fileInfo */
if ($fileInfo->isLink()) {
unlink($fileInfo->getPathname());
} elseif ($fileInfo->isDir()) {
rmdir($fileInfo->getRealPath());
} else {
unlink($fileInfo->getRealPath());
}
}
if ($deleteSelf) {
rmdir($dir);
}
} elseif (file_exists($dir)) {
if ($deleteSelf) {
unlink($dir);
}
}
if (!$deleteSelf) {
return true;
}

return !file_exists($dir);
}

/**
Expand Down
Loading