From f4f52cf24261f9a6015bf9f2dd721151a8a7e785 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 8 Jul 2014 12:51:32 +0200 Subject: [PATCH] Fix update cleanup to only affect file and folders Fix bug in the SQL query that cleans up stray shares for removed files/folders, which is now correctly limited to that item type instead of also removing all other share types. --- apps/files_sharing/lib/updater.php | 4 ++-- apps/files_sharing/tests/update.php | 30 +++++++++++++++++++++-------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/apps/files_sharing/lib/updater.php b/apps/files_sharing/lib/updater.php index aac4ed196d..a34140f5a3 100644 --- a/apps/files_sharing/lib/updater.php +++ b/apps/files_sharing/lib/updater.php @@ -204,8 +204,8 @@ class Shared_Updater { static public function fixBrokenSharesOnAppUpdate() { // delete all shares where the original file no longer exists $findAndRemoveShares = \OC_DB::prepare('DELETE FROM `*PREFIX*share` ' . - 'WHERE `file_source` NOT IN ( ' . - 'SELECT `fileid` FROM `*PREFIX*filecache` WHERE `item_type` IN (\'file\', \'folder\'))' + 'WHERE `item_type` IN (\'file\', \'folder\') ' . + 'AND `file_source` NOT IN (SELECT `fileid` FROM `*PREFIX*filecache`)' ); $findAndRemoveShares->execute(array()); } diff --git a/apps/files_sharing/tests/update.php b/apps/files_sharing/tests/update.php index b0215d6817..86b92b6961 100644 --- a/apps/files_sharing/tests/update.php +++ b/apps/files_sharing/tests/update.php @@ -87,13 +87,18 @@ class Test_Files_Sharing_Update_Routine extends Test_Files_Sharing_Base { /** * @medium */ - function testRemoveBrokenShares() { + function testRemoveBrokenFileShares() { $this->prepareFileCache(); - // check if there are just 3 shares (see setUp - precondition: empty table) - $countShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share`'); - $result = $countShares->execute()->fetchOne(); + // check if there are just 4 shares (see setUp - precondition: empty table) + $countAllShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share`'); + $result = $countAllShares->execute()->fetchOne(); + $this->assertEquals(4, $result); + + // check if there are just 3 file shares (see setUp - precondition: empty table) + $countFileShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share` WHERE `item_type` IN (\'file\', \'folder\')'); + $result = $countFileShares->execute()->fetchOne(); $this->assertEquals(3, $result); // check if there are just 2 items (see setUp - precondition: empty table) @@ -105,19 +110,23 @@ class Test_Files_Sharing_Update_Routine extends Test_Files_Sharing_Base { \OC\Files\Cache\Shared_Updater::fixBrokenSharesOnAppUpdate(); // check if there are just 2 shares (one gets killed by the code as there is no filecache entry for this) - $countShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share`'); - $result = $countShares->execute()->fetchOne(); + $result = $countFileShares->execute()->fetchOne(); $this->assertEquals(2, $result); // check if the share of file '200' is removed as there is no entry for this in filecache table - $countShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share` WHERE `file_source` = 200'); - $result = $countShares->execute()->fetchOne(); + $countFileShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share` WHERE `file_source` = 200'); + $result = $countFileShares->execute()->fetchOne(); $this->assertEquals(0, $result); // check if there are just 2 items $countItems = \OC_DB::prepare('SELECT COUNT(`fileid`) FROM `*PREFIX*filecache`'); $result = $countItems->execute()->fetchOne(); $this->assertEquals(2, $result); + + // the calendar share survived + $countOtherShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share` WHERE `item_source` = \'999\''); + $result = $countOtherShares->execute()->fetchOne(); + $this->assertEquals(1, $result); } /** @@ -228,6 +237,11 @@ class Test_Files_Sharing_Update_Routine extends Test_Files_Sharing_Base { $addShares->execute(array($fileIds[0])); $addShares->execute(array(200)); // id of "deleted" file $addShares->execute(array($fileIds[1])); + + // add a few unrelated shares, calendar share that must be left untouched + $addShares = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`item_source`, `item_type`, `uid_owner`) VALUES (?, \'calendar\', 1)'); + // the number is used as item_source + $addShares->execute(array(999)); } }