Merge pull request #24440 from...

Merge pull request #24440 from nextcloud/techdebt/noid/replace-static-unshare-call-in-test-with-sharemanager

Replace static call to Share::unshare with ShareManager->deleteShare in tests

apps/files_sharing/tests/SharedStorageTest.php

@@ -302,9 +302,7 @@ class SharedStorageTest extends TestCase {
 
 		//cleanup
 		self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
-		$result = \OC\Share\Share::unshare('folder', $fileinfoFolder['fileid'], IShare::TYPE_USER,
-			self::TEST_FILES_SHARING_API_USER2);
-		$this->assertTrue($result);
+		$this->shareManager->deleteShare($share);
 	}
 
 	public function testFopenWithUpdateOnlyPermission() {

lib/private/Share/Helper.php

@@ -30,94 +30,8 @@
 
 namespace OC\Share;
 
-use OC\HintException;
-use OCP\DB\QueryBuilder\IQueryBuilder;
-
 class Helper extends \OC\Share\Constants {
 
-	/**
-	 * Delete all reshares and group share children of an item
-	 * @param int $parent Id of item to delete
-	 * @param bool $excludeParent If true, exclude the parent from the delete (optional)
-	 * @param string $uidOwner The user that the parent was shared with (optional)
-	 * @param int $newParent new parent for the childrens
-	 * @param bool $excludeGroupChildren exclude group children elements
-	 */
-	public static function delete($parent, $excludeParent = false, $uidOwner = null, $newParent = null, $excludeGroupChildren = false) {
-		$ids = [$parent];
-		$deletedItems = [];
-		$changeParent = [];
-		$parents = [$parent];
-		while (!empty($parents)) {
-			$query = \OC::$server->getDatabaseConnection()->getQueryBuilder();
-			$query->select(
-				'id', 'share_with', 'item_type', 'share_type',
-				'item_target', 'file_target', 'parent'
-				)
-				->from('share')
-				->where($query->expr()->in('parent', $query->createNamedParameter(
-					$parents, IQueryBuilder::PARAM_INT_ARRAY
-				)));
-
-			if (count($ids) === 1 && isset($uidOwner)) {
-				// Check the owner on the first search of reshares, useful for
-				// finding and deleting the reshares by a single user of a group share
-				$query->andWhere($query->expr()->eq('uid_owner', $uidOwner));
-			}
-
-			if ($excludeGroupChildren) {
-				$query->andWhere($query->expr()->eq('share_type', self::$shareTypeGroupUserUnique));
-			}
-
-			$result = $query->execute();
-			// Reset parents array, only go through loop again if items are found
-			$parents = [];
-			while ($item = $result->fetch()) {
-				$tmpItem = [
-					'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'];
-				}
-				// if we have a new parent for the child we remember the child
-				// to update the parent, if not we add it to the list of items
-				// which should be deleted
-				if ($newParent !== null) {
-					$changeParent[] = $item['id'];
-				} else {
-					$deletedItems[] = $tmpItem;
-					$ids[] = $item['id'];
-					$parents[] = $item['id'];
-				}
-			}
-			$result->closeCursor();
-		}
-		if ($excludeParent) {
-			unset($ids[0]);
-		}
-
-		if (!empty($changeParent)) {
-			$query = \OC::$server->getDatabaseConnection()->getQueryBuilder();
-			$query->update('share')
-				->set('parent', $query->createNamedParameter($newParent, IQueryBuilder::PARAM_INT))
-				->where($query->expr()->in('id', $query->createNamedParameter($changeParent, IQueryBuilder::PARAM_INT_ARRAY)));
-			$query->execute();
-		}
-
-		if (!empty($ids)) {
-			$query = \OC::$server->getDatabaseConnection()->getQueryBuilder();
-			$query->delete('share')
-				->where($query->expr()->in('id', $query->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY)));
-			$query->execute();
-		}
-
-		return $deletedItems;
-	}
-
 	/**
 	 * get default expire settings defined by the admin
 	 * @return array contains 'defaultExpireDateSet', 'enforceExpireDate', 'expireAfterDays'
@@ -203,24 +117,6 @@ class Helper extends \OC\Share\Constants {
 		return $remote;
 	}
 
-	/**
-	 * split user and remote from federated cloud id
-	 *
-	 * @param string $id
-	 * @return string[]
-	 * @throws HintException
-	 */
-	public static function splitUserRemote($id) {
-		try {
-			$cloudId = \OC::$server->getCloudIdManager()->resolveCloudId($id);
-			return [$cloudId->getUser(), $cloudId->getRemote()];
-		} catch (\InvalidArgumentException $e) {
-			$l = \OC::$server->getL10N('core');
-			$hint = $l->t('Invalid Federated Cloud ID');
-			throw new HintException('Invalid Federated Cloud ID', $hint, 0, $e);
-		}
-	}
-
 	/**
 	 * check if two federated cloud IDs refer to the same user
 	 *

lib/private/Share/Share.php

@@ -251,85 +251,6 @@ class Share extends Constants {
 			null, -1, $includeCollections);
 	}
 
-	/**
-	 * Unshare an item from a user, group, or delete a private link
-	 * @param string $itemType
-	 * @param string $itemSource
-	 * @param int $shareType SHARE_TYPE_USER, SHARE_TYPE_GROUP, or SHARE_TYPE_LINK
-	 * @param string $shareWith User or group the item is being shared with
-	 * @param string $owner owner of the share, if null the current user is used
-	 * @return boolean true on success or false on failure
-	 */
-	public static function unshare($itemType, $itemSource, $shareType, $shareWith, $owner = null) {
-
-		// check if it is a valid itemType
-		self::getBackend($itemType);
-
-		$items = self::getItemSharedWithUser($itemType, $itemSource, $shareWith, $owner, $shareType);
-
-		$toDelete = [];
-		$newParent = null;
-		$currentUser = $owner ? $owner : \OC_User::getUser();
-		foreach ($items as $item) {
-			// delete the item with the expected share_type and owner
-			if ((int)$item['share_type'] === (int)$shareType && $item['uid_owner'] === $currentUser) {
-				$toDelete = $item;
-			// if there is more then one result we don't have to delete the children
-				// but update their parent. For group shares the new parent should always be
-				// the original group share and not the db entry with the unique name
-			} elseif ((int)$item['share_type'] === self::$shareTypeGroupUserUnique) {
-				$newParent = $item['parent'];
-			} else {
-				$newParent = $item['id'];
-			}
-		}
-
-		if (!empty($toDelete)) {
-			self::unshareItem($toDelete, $newParent);
-			return true;
-		}
-		return false;
-	}
-
-	/**
-	 * Unshares a share given a share data array
-	 * @param array $item Share data (usually database row)
-	 * @param int $newParent parent ID
-	 * @return null
-	 */
-	protected static function unshareItem(array $item, $newParent = null) {
-		$shareType = (int)$item['share_type'];
-		$shareWith = null;
-		if ($shareType !== IShare::TYPE_LINK) {
-			$shareWith = $item['share_with'];
-		}
-
-		// Pass all the vars we have for now, they may be useful
-		$hookParams = [
-			'id' => $item['id'],
-			'itemType' => $item['item_type'],
-			'itemSource' => $item['item_source'],
-			'shareType' => $shareType,
-			'shareWith' => $shareWith,
-			'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::class, 'pre_unshare', $hookParams);
-		$deletedShares = Helper::delete($item['id'], false, null, $newParent);
-		$deletedShares[] = $hookParams;
-		$hookParams['deletedShares'] = $deletedShares;
-		\OC_Hook::emit(\OCP\Share::class, 'post_unshare', $hookParams);
-		if ((int)$item['share_type'] === IShare::TYPE_REMOTE && \OC::$server->getUserSession()->getUser()) {
-			list(, $remote) = Helper::splitUserRemote($item['share_with']);
-			self::sendRemoteUnshare($remote, $item['id'], $item['token']);
-		}
-	}
-
 	/**
 	 * Get the backend class for the specified item type
 	 * @param string $itemType
@@ -964,65 +885,6 @@ class Share extends Constants {
 		return $url;
 	}
 
-	/**
-	 * try http post first with https and then with http as a fallback
-	 *
-	 * @param string $remoteDomain
-	 * @param string $urlSuffix
-	 * @param array $fields post parameters
-	 * @return array
-	 */
-	private static function tryHttpPostToShareEndpoint($remoteDomain, $urlSuffix, array $fields) {
-		$protocol = 'https://';
-		$result = [
-			'success' => false,
-			'result' => '',
-		];
-		$try = 0;
-		$discoveryService = \OC::$server->query(\OCP\OCS\IDiscoveryService::class);
-		while ($result['success'] === false && $try < 2) {
-			$federationEndpoints = $discoveryService->discover($protocol . $remoteDomain, 'FEDERATED_SHARING');
-			$endpoint = isset($federationEndpoints['share']) ? $federationEndpoints['share'] : '/ocs/v2.php/cloud/shares';
-			$client = \OC::$server->getHTTPClientService()->newClient();
-
-			try {
-				$response = $client->post(
-					$protocol . $remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT,
-					[
-						'body' => $fields,
-						'connect_timeout' => 10,
-					]
-				);
-
-				$result = ['success' => true, 'result' => $response->getBody()];
-			} catch (\Exception $e) {
-				$result = ['success' => false, 'result' => $e->getMessage()];
-			}
-
-			$try++;
-			$protocol = 'http://';
-		}
-
-		return $result;
-	}
-
-	/**
-	 * send server-to-server unshare to remote server
-	 *
-	 * @param string $remote url
-	 * @param int $id share id
-	 * @param string $token
-	 * @return bool
-	 */
-	private static function sendRemoteUnshare($remote, $id, $token) {
-		$url = rtrim($remote, '/');
-		$fields = ['token' => $token, 'format' => 'json'];
-		$url = self::removeProtocolFromUrl($url);
-		$result = self::tryHttpPostToShareEndpoint($url, '/'.$id.'/unshare', $fields);
-		$status = json_decode($result['result'], true);
-
-		return ($result['success'] && ($status['ocs']['meta']['statuscode'] === 100 || $status['ocs']['meta']['statuscode'] === 200));
-	}
 
 	/**
 	 * @return int

tests/lib/Files/ViewTest.php

@@ -1674,6 +1674,8 @@ class ViewTest extends \Test\TestCase {
 			->setSharedBy($this->user)
 			->setShareType(IShare::TYPE_USER)
 			->setPermissions(\OCP\Constants::PERMISSION_READ)
+			->setId(42)
+			->setProviderId('foo')
 			->setNode($shareDir);
 		$shareManager->createShare($share);
 
@@ -1681,7 +1683,7 @@ class ViewTest extends \Test\TestCase {
 		$this->assertFalse($view->rename('mount1', 'shareddir/sub'), 'Cannot move mount point into shared folder');
 		$this->assertFalse($view->rename('mount1', 'shareddir/sub/sub2'), 'Cannot move mount point into shared subfolder');
 
-		$this->assertTrue(\OC\Share\Share::unshare('folder', $fileId, IShare::TYPE_USER, 'test2'));
+		$shareManager->deleteShare($share);
 		$userObject->delete();
 	}
 

tests/lib/Share/HelperTest.php

@@ -55,79 +55,6 @@ class HelperTest extends \Test\TestCase {
 		$this->assertSame($expected, $result);
 	}
 
-	public function dataTestSplitUserRemote() {
-		$userPrefix = ['user@name', 'username'];
-		$protocols = ['', 'http://', 'https://'];
-		$remotes = [
-			'localhost',
-			'local.host',
-			'dev.local.host',
-			'dev.local.host/path',
-			'dev.local.host/at@inpath',
-			'127.0.0.1',
-			'::1',
-			'::192.0.2.128',
-			'::192.0.2.128/at@inpath',
-		];
-
-		$testCases = [];
-		foreach ($userPrefix as $user) {
-			foreach ($remotes as $remote) {
-				foreach ($protocols as $protocol) {
-					$baseUrl = $user . '@' . $protocol . $remote;
-
-					$testCases[] = [$baseUrl, $user, $protocol . $remote];
-					$testCases[] = [$baseUrl . '/', $user, $protocol . $remote];
-					$testCases[] = [$baseUrl . '/index.php', $user, $protocol . $remote];
-					$testCases[] = [$baseUrl . '/index.php/s/token', $user, $protocol . $remote];
-				}
-			}
-		}
-		return $testCases;
-	}
-
-	/**
-	 * @dataProvider dataTestSplitUserRemote
-	 *
-	 * @param string $remote
-	 * @param string $expectedUser
-	 * @param string $expectedUrl
-	 */
-	public function testSplitUserRemote($remote, $expectedUser, $expectedUrl) {
-		list($remoteUser, $remoteUrl) = \OC\Share\Helper::splitUserRemote($remote);
-		$this->assertSame($expectedUser, $remoteUser);
-		$this->assertSame($expectedUrl, $remoteUrl);
-	}
-
-	public function dataTestSplitUserRemoteError() {
-		return [
-			// Invalid path
-			['user@'],
-
-			// Invalid user
-			['@server'],
-			['us/er@server'],
-			['us:er@server'],
-
-			// Invalid splitting
-			['user'],
-			[''],
-			['us/erserver'],
-			['us:erserver'],
-		];
-	}
-
-	/**
-	 * @dataProvider dataTestSplitUserRemoteError
-	 *
-	 * @param string $id
-	 */
-	public function testSplitUserRemoteError($id) {
-		$this->expectException(\OC\HintException::class);
-
-		\OC\Share\Helper::splitUserRemote($id);
-	}
-
 	/**
 	 * @dataProvider dataTestCompareServerAddresses
 	 *