From eaedda2116ddde4b0a571ccdf96437eea37fe380 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 28 Nov 2013 13:17:19 +0100 Subject: [PATCH 1/3] make sure that we don't try to access an already deleted files, fixes some file source not found warnings --- apps/files_sharing/lib/updater.php | 23 +++++++++---- apps/files_versions/appinfo/app.php | 1 + apps/files_versions/lib/hooks.php | 15 +++++++-- apps/files_versions/lib/versions.php | 50 ++++++++++++++++++++-------- lib/private/files/cache/updater.php | 8 +++-- 5 files changed, 72 insertions(+), 25 deletions(-) diff --git a/apps/files_sharing/lib/updater.php b/apps/files_sharing/lib/updater.php index 0c35b18c42..44ebb5cd3c 100644 --- a/apps/files_sharing/lib/updater.php +++ b/apps/files_sharing/lib/updater.php @@ -23,6 +23,9 @@ namespace OC\Files\Cache; class Shared_Updater { + // shares which can be removed from oc_share after the delete operation was successful + static private $toRemove = array(); + /** * Correct the parent folders' ETags for all users shared the file at $target * @@ -58,15 +61,17 @@ class Shared_Updater { * @param string $path */ private static function removeShare($path) { - $fileInfo = \OC\Files\Filesystem::getFileInfo($path); - $fileSource = $fileInfo['fileid']; + $fileSource = self::$toRemove[$path]; - $query = \OC_DB::prepare('DELETE FROM `*PREFIX*share` WHERE `file_source`=?'); - try { - \OC_DB::executeAudited($query, array($fileSource)); - } catch (\Exception $e) { - \OCP\Util::writeLog('files_sharing', "can't remove share: " . $e->getMessage(), \OCP\Util::WARN); + if (!\OC\Files\Filesystem::file_exists($path)) { + $query = \OC_DB::prepare('DELETE FROM `*PREFIX*share` WHERE `file_source`=?'); + try { + \OC_DB::executeAudited($query, array($fileSource)); + } catch (\Exception $e) { + \OCP\Util::writeLog('files_sharing', "can't remove share: " . $e->getMessage(), \OCP\Util::WARN); + } } + unset(self::$toRemove[$path]); } /** @@ -89,6 +94,10 @@ class Shared_Updater { */ static public function deleteHook($params) { self::correctFolders($params['path']); + $fileInfo = \OC\Files\Filesystem::getFileInfo($params['path']); + // mark file as deleted so that we can clean up the share table if + // the file was deleted successfully + self::$toRemove[$params['path']] = $fileInfo['fileid']; } /** diff --git a/apps/files_versions/appinfo/app.php b/apps/files_versions/appinfo/app.php index 5b1e464ba6..8f2071dd7b 100644 --- a/apps/files_versions/appinfo/app.php +++ b/apps/files_versions/appinfo/app.php @@ -12,6 +12,7 @@ OCP\Util::addStyle('files_versions', 'versions'); OCP\Util::connectHook('OC_Filesystem', 'write', "OCA\Files_Versions\Hooks", "write_hook"); // Listen to delete and rename signals OCP\Util::connectHook('OC_Filesystem', 'post_delete', "OCA\Files_Versions\Hooks", "remove_hook"); +OCP\Util::connectHook('OC_Filesystem', 'delete', "OCA\Files_Versions\Hooks", "pre_remove_hook"); OCP\Util::connectHook('OC_Filesystem', 'rename', "OCA\Files_Versions\Hooks", "rename_hook"); //Listen to delete user signal OCP\Util::connectHook('OC_User', 'pre_deleteUser', "OCA\Files_Versions\Hooks", "deleteUser_hook"); diff --git a/apps/files_versions/lib/hooks.php b/apps/files_versions/lib/hooks.php index 81ee3c8b3c..5d3882cc3e 100644 --- a/apps/files_versions/lib/hooks.php +++ b/apps/files_versions/lib/hooks.php @@ -45,6 +45,17 @@ class Hooks { } } + /** + * @brief mark file as "deleted" so that we can clean up the versions if the file is gone + * @param array $params + */ + public static function pre_remove_hook($params) { + $path = $params[\OC\Files\Filesystem::signal_param_path]; + if($path<>'') { + Storage::markDeletedFile($path); + } + } + /** * @brief rename/move versions of renamed/moved files * @param array with oldpath and newpath @@ -53,7 +64,7 @@ class Hooks { * of the stored versions along the actual file */ public static function rename_hook($params) { - + if (\OCP\App::isEnabled('files_versions')) { $oldpath = $params['oldpath']; $newpath = $params['newpath']; @@ -71,7 +82,7 @@ class Hooks { * to remove the used space for versions stored in the database */ public static function deleteUser_hook($params) { - + if (\OCP\App::isEnabled('files_versions')) { $uid = $params['uid']; Storage::deleteUser($uid); diff --git a/apps/files_versions/lib/versions.php b/apps/files_versions/lib/versions.php index 3ad73284ef..65ed33fe2a 100644 --- a/apps/files_versions/lib/versions.php +++ b/apps/files_versions/lib/versions.php @@ -21,6 +21,9 @@ class Storage { const DEFAULTMAXSIZE=50; // unit: percentage; 50% of available disk space/quota const VERSIONS_ROOT = 'files_versions/'; + // files for which we can remove the versions after the delete operation was successful + private static $deletedFiles = array(); + private static $max_versions_per_interval = array( //first 10sec, one version every 2sec 1 => array('intervalEndsAfter' => 10, 'step' => 2), @@ -141,26 +144,45 @@ class Storage { } + /** + * @brief mark file as deleted so that we can remove the versions if the file is gone + * @param string $path + */ + public static function markDeletedFile($path) { + list($uid, $filename) = self::getUidAndFilename($path); + self::$deletedFiles[$path] = array( + 'uid' => $uid, + 'filename' => $filename); + } + /** * Delete versions of a file */ - public static function delete($filename) { - list($uid, $filename) = self::getUidAndFilename($filename); - $versions_fileview = new \OC\Files\View('/'.$uid .'/files_versions'); + public static function delete($path) { - $abs_path = $versions_fileview->getLocalFile($filename.'.v'); - if( ($versions = self::getVersions($uid, $filename)) ) { - $versionsSize = self::getVersionsSize($uid); - if ( $versionsSize === false || $versionsSize < 0 ) { - $versionsSize = self::calculateSize($uid); + $deletedFile = self::$deletedFiles[$path]; + $uid = $deletedFile['uid']; + $filename = $deletedFile['filename']; + + if (!\OC\Files\Filesystem::file_exists($path)) { + + $versions_fileview = new \OC\Files\View('/' . $uid . '/files_versions'); + + $abs_path = $versions_fileview->getLocalFile($filename . '.v'); + if (($versions = self::getVersions($uid, $filename))) { + $versionsSize = self::getVersionsSize($uid); + if ($versionsSize === false || $versionsSize < 0) { + $versionsSize = self::calculateSize($uid); + } + foreach ($versions as $v) { + unlink($abs_path . $v['version']); + \OC_Hook::emit('\OCP\Versions', 'delete', array('path' => $abs_path . $v['version'])); + $versionsSize -= $v['size']; + } + self::setVersionsSize($uid, $versionsSize); } - foreach ($versions as $v) { - unlink($abs_path . $v['version']); - \OC_Hook::emit('\OCP\Versions', 'delete', array('path' => $abs_path . $v['version'])); - $versionsSize -= $v['size']; - } - self::setVersionsSize($uid, $versionsSize); } + unset(self::$deletedFiles[$path]); } /** diff --git a/lib/private/files/cache/updater.php b/lib/private/files/cache/updater.php index d45c5e17fc..9c182c3c3c 100644 --- a/lib/private/files/cache/updater.php +++ b/lib/private/files/cache/updater.php @@ -59,10 +59,14 @@ class Updater { */ list($storage, $internalPath) = self::resolvePath($path); if ($storage) { + $parent = dirname($internalPath); + if ($parent === '.') { + $parent = ''; + } $cache = $storage->getCache($internalPath); $cache->remove($internalPath); - $cache->correctFolderSize($internalPath); - self::correctFolder($path, time()); + $cache->correctFolderSize($parent); + self::correctFolder($parent, time()); self::correctParentStorageMtime($storage, $internalPath); } } From 466ed01e5d9dc0f7c813a04bea09adcbe1f25024 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 28 Nov 2013 16:31:01 +0100 Subject: [PATCH 2/3] correctFolder() already resolves the dirname internally --- lib/private/files/cache/updater.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/files/cache/updater.php b/lib/private/files/cache/updater.php index 9c182c3c3c..1fb47cd218 100644 --- a/lib/private/files/cache/updater.php +++ b/lib/private/files/cache/updater.php @@ -66,7 +66,7 @@ class Updater { $cache = $storage->getCache($internalPath); $cache->remove($internalPath); $cache->correctFolderSize($parent); - self::correctFolder($parent, time()); + self::correctFolder($path, time()); self::correctParentStorageMtime($storage, $internalPath); } } From 348706854cd47f3fd104fbc8f5af7595980f5084 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 28 Nov 2013 19:31:35 +0100 Subject: [PATCH 3/3] use better coding style --- apps/files_versions/lib/versions.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_versions/lib/versions.php b/apps/files_versions/lib/versions.php index 65ed33fe2a..42a15612d6 100644 --- a/apps/files_versions/lib/versions.php +++ b/apps/files_versions/lib/versions.php @@ -169,7 +169,8 @@ class Storage { $versions_fileview = new \OC\Files\View('/' . $uid . '/files_versions'); $abs_path = $versions_fileview->getLocalFile($filename . '.v'); - if (($versions = self::getVersions($uid, $filename))) { + $versions = self::getVersions($uid, $filename); + if (!empty($versions)) { $versionsSize = self::getVersionsSize($uid); if ($versionsSize === false || $versionsSize < 0) { $versionsSize = self::calculateSize($uid);