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.

diff --git a/apps/files_sharing/lib/updater.php b/apps/files_sharing/lib/updater.php
index aac4ed196deec85655617b93feedf92edc34288d..a34140f5a353c866ac0ed48e736fa1b2ef3b26c1 100644 (file)
--- 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 b0215d681765718f64a72f317d37870459394911..86b92b6961650c7d8594d1b758d15aa4daef7d0b 100644 (file)
--- 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));
        }
 
 }