split-up the update script and add unit tests for it

apps/files_sharing/appinfo/update.php

@@ -2,6 +2,21 @@
 
 $installedVersion = OCP\Config::getAppValue('files_sharing', 'installed_version');
 if (version_compare($installedVersion, '0.4', '<')) {
+	removeSharedFolder();
+}
+
+// clean up oc_share table from files which are no longer exists
+if (version_compare($installedVersion, '0.3.5.6', '<')) {
+	\OC\Files\Cache\Shared_Updater::fixBrokenSharesOnAppUpdate();
+}
+
+
+/**
+ * update script for the removal of the logical "Shared" folder, we create physical "Shared" folder and
+ * update the users file_target so that it doesn't make any difference for the user
+ * @note parameters are just for testing, please ignore them
+ */
+function removeSharedFolder($mkdirs = true, $chunkSize = 99) {
 	$query = OCP\DB::prepare('SELECT * FROM `*PREFIX*share`');
 	$result = $query->execute();
 	$view = new \OC\Files\View('/');
@@ -14,14 +29,14 @@ if (version_compare($installedVersion, '0.4', '<')) {
 	//we need to set up user backends, otherwise creating the shares will fail with "because user does not exist"
 	while ($row = $result->fetchRow()) {
 		//collect all user shares
-		if ($row['share_type'] === "0" && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) {
+		if ((int)$row['share_type'] === 0 && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) {
 			$users[] = $row['share_with'];
 			$shares[$row['id']] = $row['file_target'];
-		} else if ($row['share_type'] === "1" && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) {
+		} else if ((int)$row['share_type'] === 1 && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) {
 			//collect all group shares
 			$users = array_merge($users, \OC_group::usersInGroup($row['share_with']));
 			$shares[$row['id']] = $row['file_target'];
-		} else if ($row['share_type'] === "2") {
+		} else if ((int)$row['share_type'] === 2) {
 			$shares[$row['id']] = $row['file_target'];
 		}
 	}
@@ -32,30 +47,31 @@ if (version_compare($installedVersion, '0.4', '<')) {
 
 		// create folder Shared for each user
 
-		foreach ($unique_users as $user) {
+		if ($mkdirs) {
-			\OC\Files\Filesystem::initMountPoints($user);
+			foreach ($unique_users as $user) {
-			if (!$view->file_exists('/' . $user . '/files/Shared')) {
+				\OC\Files\Filesystem::initMountPoints($user);
-				$view->mkdir('/' . $user . '/files/Shared');
+				if (!$view->file_exists('/' . $user . '/files/Shared')) {
+					$view->mkdir('/' . $user . '/files/Shared');
+				}
 			}
 		}
 
-		$statement = "UPDATE `*PREFIX*share` SET `file_target` = CASE id ";
+		$chunkedShareList = array_chunk($shares, $chunkSize, true);
-		//update share table
+
-		$ids = implode(',', array_keys($shares));
+		foreach ($chunkedShareList as $subList) {
-		foreach ($shares as $id => $target) {
+
-			$statement .= "WHEN " . $id . " THEN '/Shared" . $target . "' ";
+			$statement = "UPDATE `*PREFIX*share` SET `file_target` = CASE `id` ";
+			//update share table
+			$ids = implode(',', array_keys($subList));
+			foreach ($subList as $id => $target) {
+				$statement .= "WHEN " . $id . " THEN '/Shared" . $target . "' ";
+			}
+			$statement .= ' END WHERE `id` IN (' . $ids . ')';
+
+			$query = OCP\DB::prepare($statement);
+
+			$query->execute(array());
 		}
-		$statement .= ' END WHERE `id` IN (' . $ids . ')';
-
-		$query = OCP\DB::prepare($statement);
-
-		$query->execute(array());
 
 	}
-
-}
-
-// clean up oc_share table from files which are no longer exists
-if (version_compare($installedVersion, '0.3.5.6', '<')) {
-	\OC\Files\Cache\Shared_Updater::fixBrokenSharesOnAppUpdate();
 }

apps/files_sharing/tests/updater.php

@@ -20,6 +20,8 @@
  *
  */
 
+require_once __DIR__ . '/../appinfo/update.php';
+
 /**
  * Class Test_Files_Sharing_Updater
  */
@@ -88,4 +90,57 @@ class Test_Files_Sharing_Updater extends \PHPUnit_Framework_TestCase {
 		$result = $countItems->execute()->fetchOne();
 		$this->assertEquals(2, $result);
 	}
+
+	/**
+	 * test update for the removal of the logical "Shared" folder. It should update
+	 * the file_target for every share and create a physical "Shared" folder for each user
+	 */
+	function testRemoveSharedFolder() {
+		self::prepareDB();
+		// run the update routine to remove the shared folder and replace it with a real folder
+		removeSharedFolder(false, 2);
+
+		// verify results
+		$query = \OC_DB::prepare('SELECT * FROM `*PREFIX*share`');
+		$result = $query->execute(array());
+
+		$newDBContent = $result->fetchAll();
+
+		foreach ($newDBContent as $row) {
+			if ((int)$row['share_type'] === \OCP\Share::SHARE_TYPE_USER) {
+				$this->assertSame('/Shared', substr($row['file_target'], 0, strlen('/Shared')));
+			} else {
+				$this->assertSame('/ShouldNotChange', $row['file_target']);
+			}
+		}
+
+		$this->cleanupSharedTable();
+
+	}
+
+	private function cleanupSharedTable() {
+		$query = \OC_DB::prepare('DELETE FROM `*PREFIX*share`');
+		$query->execute();
+	}
+
+	private function prepareDB() {
+		$this->cleanupSharedTable();
+		// add items except one - because this is the test case for the broken share table
+		$addItems = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`share_type`, `item_type`, ' .
+			'`share_with`, `uid_owner` , `file_target`) ' .
+			'VALUES (?, ?, ?, ?, ?)');
+		$items = array(
+			array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user1', 'admin' , '/foo'),
+			array(\OCP\Share::SHARE_TYPE_USER, 'folder', 'user2', 'admin', '/foo2'),
+			array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user3', 'admin', '/foo3'),
+			array(\OCP\Share::SHARE_TYPE_USER, 'folder', 'user4', 'admin', '/foo4'),
+			array(\OCP\Share::SHARE_TYPE_LINK, 'file', 'user1', 'admin', '/ShouldNotChange'),
+			array(\OCP\Share::SHARE_TYPE_CONTACT, 'contact', 'admin', 'user1', '/ShouldNotChange'),
+
+			);
+		foreach($items as $item) {
+			// the number is used as path_hash
+			$addItems->execute($item);
+		}
+	}
 }