Merge pull request #9193 from owncloud/sharing_etag_propagation

make sure that the etags get propagated correctly

apps/files_sharing/appinfo/app.php

@@ -14,20 +14,14 @@ OC::$CLASSPATH['OCA\Files\Share\Proxy'] = 'files_sharing/lib/proxy.php';
 
 \OCP\App::registerAdmin('files_sharing', 'settings-admin');
 
-OCP\Util::connectHook('OC_Filesystem', 'setup', '\OC\Files\Storage\Shared', 'setup');
+\OCA\Files_Sharing\Helper::registerHooks();
-OCP\Util::connectHook('OC_Filesystem', 'setup', '\OCA\Files_Sharing\External\Manager', 'setup');
+
 OCP\Share::registerBackend('file', 'OC_Share_Backend_File');
 OCP\Share::registerBackend('folder', 'OC_Share_Backend_Folder', 'file');
 
 OCP\Util::addScript('files_sharing', 'share');
 OCP\Util::addScript('files_sharing', 'external');
 
-\OC_Hook::connect('OC_Filesystem', 'post_write', '\OC\Files\Cache\Shared_Updater', 'writeHook');
-\OC_Hook::connect('OC_Filesystem', 'post_delete', '\OC\Files\Cache\Shared_Updater', 'postDeleteHook');
-\OC_Hook::connect('OC_Filesystem', 'delete', '\OC\Files\Cache\Shared_Updater', 'deleteHook');
-\OC_Hook::connect('OC_Filesystem', 'post_rename', '\OC\Files\Cache\Shared_Updater', 'renameHook');
-\OC_Hook::connect('OC_Appconfig', 'post_set_value', '\OCA\Files\Share\Maintainer', 'configChangeHook');
-
 OC_FileProxy::register(new OCA\Files\Share\Proxy());
 
 \OCA\Files\App::getNavigationManager()->add(

apps/files_sharing/lib/helper.php

@@ -7,6 +7,20 @@ use PasswordHash;
 
 class Helper {
 
+	public static function registerHooks() {
+		\OCP\Util::connectHook('OC_Filesystem', 'setup', '\OC\Files\Storage\Shared', 'setup');
+		\OCP\Util::connectHook('OC_Filesystem', 'setup', '\OCA\Files_Sharing\External\Manager', 'setup');
+		\OCP\Util::connectHook('OC_Filesystem', 'post_write', '\OC\Files\Cache\Shared_Updater', 'writeHook');
+		\OCP\Util::connectHook('OC_Filesystem', 'post_delete', '\OC\Files\Cache\Shared_Updater', 'postDeleteHook');
+		\OCP\Util::connectHook('OC_Filesystem', 'delete', '\OC\Files\Cache\Shared_Updater', 'deleteHook');
+		\OCP\Util::connectHook('OC_Filesystem', 'post_rename', '\OC\Files\Cache\Shared_Updater', 'renameHook');
+		\OCP\Util::connectHook('OC_Appconfig', 'post_set_value', '\OCA\Files\Share\Maintainer', 'configChangeHook');
+
+		\OCP\Util::connectHook('OCP\Share', 'post_shared', '\OC\Files\Cache\Shared_Updater', 'postShareHook');
+		\OCP\Util::connectHook('OCP\Share', 'post_unshare', '\OC\Files\Cache\Shared_Updater', 'postUnshareHook');
+		\OCP\Util::connectHook('OCP\Share', 'post_unshareFromSelf', '\OC\Files\Cache\Shared_Updater', 'postUnshareFromSelfHook');
+	}
+
 	/**
 	 * Sets up the filesystem and user for public sharing
 	 * @param string $token string share token

apps/files_sharing/lib/updater.php

@@ -132,6 +132,68 @@ class Shared_Updater {
 		self::removeShare($params['path']);
 	}
 
+	/**
+	 * update etags if a file was shared
+	 * @param array $params
+	 */
+	static public function postShareHook($params) {
+
+		if ($params['itemType'] === 'folder' || $params['itemType'] === 'file') {
+
+			$shareWith = $params['shareWith'];
+			$shareType = $params['shareType'];
+
+			if ($shareType === \OCP\Share::SHARE_TYPE_USER) {
+				self::correctUsersFolder($shareWith, '/');
+			} elseif ($shareType === \OCP\Share::SHARE_TYPE_GROUP) {
+				foreach (\OC_Group::usersInGroup($shareWith) as $user) {
+					self::correctUsersFolder($user, '/');
+				}
+			}
+		}
+	}
+
+	/**
+	 * update etags if a file was unshared
+	 *
+	 * @param array $params
+	 */
+	static public function postUnshareHook($params) {
+
+		if ($params['itemType'] === 'file' || $params['itemType'] === 'folder') {
+
+			$deletedShares = isset($params['deletedShares']) ? $params['deletedShares'] : array();
+
+			foreach ($deletedShares as $share) {
+				if ($share['shareType'] === \OCP\Share::SHARE_TYPE_GROUP) {
+					foreach (\OC_Group::usersInGroup($share['shareWith']) as $user) {
+						self::correctUsersFolder($user, dirname($share['fileTarget']));
+					}
+				} else {
+					self::correctUsersFolder($share['shareWith'], dirname($share['fileTarget']));
+				}
+			}
+		}
+	}
+
+	/**
+	 * update etags if file was unshared from self
+	 * @param array $params
+	 */
+	static public function postUnshareFromSelfHook($params) {
+		if ($params['itemType'] === 'file' || $params['itemType'] === 'folder') {
+			foreach ($params['unsharedItems'] as $item) {
+				if ($item['shareType'] === \OCP\Share::SHARE_TYPE_GROUP) {
+					foreach (\OC_Group::usersInGroup($item['shareWith']) as $user) {
+						self::correctUsersFolder($user, dirname($item['fileTarget']));
+					}
+				} else {
+					self::correctUsersFolder($item['shareWith'], dirname($item['fileTarget']));
+				}
+			}
+		}
+	}
+
 	/**
 	 * clean up oc_share table from files which are no longer exists
 	 *

apps/files_sharing/tests/updater.php

@@ -28,14 +28,19 @@ require_once __DIR__ . '/base.php';
  */
 class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base {
 
-	const TEST_FOLDER_NAME = '/folder_share_api_test';
+	const TEST_FOLDER_NAME = '/folder_share_updater_test';
+
+	public static function setUpBeforeClass() {
+		parent::setUpBeforeClass();
+		\OCA\Files_Sharing\Helper::registerHooks();
+	}
 
 	function setUp() {
 		parent::setUp();
 
 		$this->folder = self::TEST_FOLDER_NAME;
 
-		$this->filename = '/share-api-test.txt';
+		$this->filename = '/share-updater-test.txt';
 
 		// save file with content
 		$this->view->file_put_contents($this->filename, $this->data);
@@ -104,6 +109,112 @@ class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base {
 		if ($status === false) {
 			\OC_App::disable('files_trashbin');
 		}
+		// cleanup
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
+		$result = \OCP\Share::unshare('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2);
+		$this->assertTrue($result);
+	}
+
+	/**
+	 * if a file gets shared the etag for the recipients root should change
+	 */
+	function testShareFile() {
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
+
+		$beforeShare = \OC\Files\Filesystem::getFileInfo('');
+		$etagBeforeShare = $beforeShare->getEtag();
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
+		$fileinfo = \OC\Files\Filesystem::getFileInfo($this->folder);
+		$result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31);
+		$this->assertTrue($result);
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
+
+		$afterShare = \OC\Files\Filesystem::getFileInfo('');
+		$etagAfterShare = $afterShare->getEtag();
+
+		$this->assertTrue(is_string($etagBeforeShare));
+		$this->assertTrue(is_string($etagAfterShare));
+		$this->assertTrue($etagBeforeShare !== $etagAfterShare);
+
+		// cleanup
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
+		$result = \OCP\Share::unshare('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2);
+		$this->assertTrue($result);
+	}
+
+	/**
+	 * if a file gets unshared by the owner the etag for the recipients root should change
+	 */
+	function testUnshareFile() {
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
+		$fileinfo = \OC\Files\Filesystem::getFileInfo($this->folder);
+		$result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31);
+		$this->assertTrue($result);
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
+
+		$beforeUnshare = \OC\Files\Filesystem::getFileInfo('');
+		$etagBeforeUnshare = $beforeUnshare->getEtag();
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
+		$result = \OCP\Share::unshare('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2);
+		$this->assertTrue($result);
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
+
+		$afterUnshare = \OC\Files\Filesystem::getFileInfo('');
+		$etagAfterUnshare = $afterUnshare->getEtag();
+
+		$this->assertTrue(is_string($etagBeforeUnshare));
+		$this->assertTrue(is_string($etagAfterUnshare));
+		$this->assertTrue($etagBeforeUnshare !== $etagAfterUnshare);
+
+	}
+
+	/**
+	 * if a file gets unshared from self the etag for the recipients root should change
+	 */
+	function testUnshareFromSelfFile() {
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER1);
+		$fileinfo = \OC\Files\Filesystem::getFileInfo($this->folder);
+		$result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, 31);
+		$this->assertTrue($result);
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
+
+		$result = \OCP\Share::shareItem('folder', $fileinfo->getId(), \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER3, 31);
+
+		$beforeUnshareUser2 = \OC\Files\Filesystem::getFileInfo('');
+		$etagBeforeUnshareUser2 = $beforeUnshareUser2->getEtag();
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER3);
+
+		$beforeUnshareUser3 = \OC\Files\Filesystem::getFileInfo('');
+		$etagBeforeUnshareUser3 = $beforeUnshareUser3->getEtag();
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER2);
+
+		$result = \OC\Files\Filesystem::unlink($this->folder);
+		$this->assertTrue($result);
+
+		$afterUnshareUser2 = \OC\Files\Filesystem::getFileInfo('');
+		$etagAfterUnshareUser2 = $afterUnshareUser2->getEtag();
+
+		$this->loginHelper(self::TEST_FILES_SHARING_API_USER3);
+
+		$afterUnshareUser3 = \OC\Files\Filesystem::getFileInfo('');
+		$etagAfterUnshareUser3 = $afterUnshareUser3->getEtag();
+
+		$this->assertTrue(is_string($etagBeforeUnshareUser2));
+		$this->assertTrue(is_string($etagBeforeUnshareUser3));
+		$this->assertTrue(is_string($etagAfterUnshareUser2));
+		$this->assertTrue(is_string($etagAfterUnshareUser3));
+		$this->assertTrue($etagBeforeUnshareUser2 !== $etagAfterUnshareUser2);
+		$this->assertTrue($etagBeforeUnshareUser3 !== $etagAfterUnshareUser3);
+
 	}
 
 }

lib/private/share/helper.php

@@ -149,17 +149,18 @@ class Helper extends \OC\Share\Constants {
 	 */
 	public static function delete($parent, $excludeParent = false, $uidOwner = null) {
 		$ids = array($parent);
+		$deletedItems = array();
 		$parents = array($parent);
 		while (!empty($parents)) {
 			$parents = "'".implode("','", $parents)."'";
 			// Check the owner on the first search of reshares, useful for
 			// finding and deleting the reshares by a single user of a group share
 			if (count($ids) == 1 && isset($uidOwner)) {
-				$query = \OC_DB::prepare('SELECT `id`, `uid_owner`, `item_type`, `item_target`, `parent`'
+				$query = \OC_DB::prepare('SELECT `id`, `share_with`, `item_type`, `share_type`, `item_target`, `file_target`, `parent`'
 					.' FROM `*PREFIX*share` WHERE `parent` IN ('.$parents.') AND `uid_owner` = ?');
 				$result = $query->execute(array($uidOwner));
 			} else {
-				$query = \OC_DB::prepare('SELECT `id`, `item_type`, `item_target`, `parent`, `uid_owner`'
+				$query = \OC_DB::prepare('SELECT `id`, `share_with`, `item_type`, `share_type`, `item_target`, `file_target`, `parent`, `uid_owner`'
 					.' FROM `*PREFIX*share` WHERE `parent` IN ('.$parents.')');
 				$result = $query->execute();
 			}
@@ -168,16 +169,29 @@ class Helper extends \OC\Share\Constants {
 			while ($item = $result->fetchRow()) {
 				$ids[] = $item['id'];
 				$parents[] = $item['id'];
+				$tmpItem = array(
+					'id' => $item['id'],
+					'shareWith' => $item['share_with'],
+					'itemTarget' => $item['item_target'],
+					'itemType' => $item['item_type'],
+					'shareType' => (int)$item['share_type'],
+				);
+				if (isset($item['file_target'])) {
+					$tmpItem['fileTarget'] = $item['file_target'];
+				}
+				$deletedItems[] = $tmpItem;
 			}
 		}
 		if ($excludeParent) {
 			unset($ids[0]);
 		}
 		if (!empty($ids)) {
-			$ids = "'".implode("','", $ids)."'";
+			$idList = "'".implode("','", $ids)."'";
-			$query = \OC_DB::prepare('DELETE FROM `*PREFIX*share` WHERE `id` IN ('.$ids.')');
+			$query = \OC_DB::prepare('DELETE FROM `*PREFIX*share` WHERE `id` IN ('.$idList.')');
 			$query->execute();
 		}
+
+		return $deletedItems;
 	}
 
 	/**

lib/private/share/share.php

@@ -738,11 +738,24 @@ class Share extends \OC\Share\Constants {
 
 		$shares = $result->fetchAll();
 
+		$listOfUnsharedItems = array();
+
 		$itemUnshared = false;
 		foreach ($shares as $share) {
 			if ((int)$share['share_type'] === \OCP\Share::SHARE_TYPE_USER &&
 					$share['share_with'] === $uid) {
-				Helper::delete($share['id']);
+				$deletedShares = Helper::delete($share['id']);
+				$shareTmp = array(
+					'id' => $share['id'],
+					'shareWith' => $share['share_with'],
+					'itemTarget' => $share['item_target'],
+					'itemType' => $share['item_type'],
+					'shareType' => (int)$share['share_type'],
+				);
+				if (isset($share['file_target'])) {
+					$shareTmp['fileTarget'] = $share['file_target'];
+				}
+				$listOfUnsharedItems = array_merge($listOfUnsharedItems, $deletedShares, array($shareTmp));
 				$itemUnshared = true;
 				break;
 			} elseif ((int)$share['share_type'] === \OCP\Share::SHARE_TYPE_GROUP) {
@@ -764,13 +777,40 @@ class Share extends \OC\Share\Constants {
 				$groupShare['id'], self::$shareTypeGroupUserUnique,
 				\OC_User::getUser(), $groupShare['uid_owner'], 0, $groupShare['stime'], $groupShare['file_source'],
 				$groupShare['file_target']));
+			$shareTmp = array(
+				'id' => $groupShare['id'],
+				'shareWith' => $groupShare['share_with'],
+				'itemTarget' => $groupShare['item_target'],
+				'itemType' => $groupShare['item_type'],
+				'shareType' => (int)$groupShare['share_type'],
+				);
+			if (isset($groupShare['file_target'])) {
+				$shareTmp['fileTarget'] = $groupShare['file_target'];
+			}
+			$listOfUnsharedItems = array_merge($listOfUnsharedItems, array($groupShare));
 			$itemUnshared = true;
 		} elseif (!$itemUnshared && isset($uniqueGroupShare)) {
 			$query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = ? WHERE `id` = ?');
 			$query->execute(array(0, $uniqueGroupShare['id']));
+			$shareTmp = array(
+				'id' => $uniqueGroupShare['id'],
+				'shareWith' => $uniqueGroupShare['share_with'],
+				'itemTarget' => $uniqueGroupShare['item_target'],
+				'itemType' => $uniqueGroupShare['item_type'],
+				'shareType' => (int)$uniqueGroupShare['share_type'],
+				);
+			if (isset($uniqueGroupShare['file_target'])) {
+				$shareTmp['fileTarget'] = $uniqueGroupShare['file_target'];
+			}
+			$listOfUnsharedItems = array_merge($listOfUnsharedItems, array($uniqueGroupShare));
 			$itemUnshared = true;
 		}
 
+		if ($itemUnshared) {
+			\OC_Hook::emit('OCP\Share', 'post_unshareFromSelf',
+					array('unsharedItems' => $listOfUnsharedItems, 'itemType' => $itemType));
+		}
+
 		return $itemUnshared;
 	}
 
@@ -967,19 +1007,23 @@ class Share extends \OC\Share\Constants {
 	protected static function unshareItem(array $item) {
 		// Pass all the vars we have for now, they may be useful
 		$hookParams = array(
+			'id'            => $item['id'],
 			'itemType'      => $item['item_type'],
 			'itemSource'    => $item['item_source'],
-			'fileSource'    => $item['file_source'],
+			'shareType'     => (int)$item['share_type'],
-			'shareType'     => $item['share_type'],
 			'shareWith'     => $item['share_with'],
 			'itemParent'    => $item['parent'],
 			'uidOwner'      => $item['uid_owner'],
 		);
+		if($item['item_type'] === 'file' || $item['item_type'] === 'folder') {
+			$hookParams['fileSource'] = $item['file_source'];
+			$hookParams['fileTarget'] = $item['file_target'];
+		}
 
-		\OC_Hook::emit('OCP\Share', 'pre_unshare', $hookParams + array(
+		\OC_Hook::emit('OCP\Share', 'pre_unshare', $hookParams);
-			'fileSource'	=> $item['file_source'],
+		$deletedShares = Helper::delete($item['id']);
-		));
+		$deletedShares[] = $hookParams;
-		Helper::delete($item['id']);
+		$hookParams['deletedShares'] = $deletedShares;
 		\OC_Hook::emit('OCP\Share', 'post_unshare', $hookParams);
 	}
 
@@ -1788,7 +1832,7 @@ class Share extends \OC\Share\Constants {
 			if (isset($uidOwner)) {
 				if ($fileDependent) {
 					$select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`,'
-							. ' `share_type`, `share_with`, `file_source`, `path`, `*PREFIX*share`.`permissions`, `stime`,'
+							. ' `share_type`, `share_with`, `file_source`, `file_target`, `path`, `*PREFIX*share`.`permissions`, `stime`,'
 							. ' `expiration`, `token`, `storage`, `mail_send`, `uid_owner`';
 				} else {
 					$select = '`id`, `item_type`, `item_source`, `parent`, `share_type`, `share_with`, `*PREFIX*share`.`permissions`,'