[Share 2.0] Use full share id (providerId:shareId)

Now that we support multiple managers we communicate shares to the
outside as 'providerId:shareId'. This makes sures that id's are unique
when references from the OCS API.

However, since we do not want to break the OCS API v1 we need to
somewhat hack around this.

When we switch to OCS API v2 (which we should when we support more
custom providers). We will change the id to always be the fullShareId.

apps/files_sharing/api/share20ocs.php

@@ -142,10 +142,20 @@ class Share20OCS {
 	 * @return \OC_OCS_Result
 	 */
 	public function getShare($id) {
+		// Try both our default, and our federated provider..
+		$share = null;
+
+		// First check if it is an internal share.
 		try {
-			$share = $this->shareManager->getShareById($id);
+			$share = $this->shareManager->getShareById('ocinternal:'.$id);
 		} catch (\OC\Share20\Exception\ShareNotFound $e) {
-			return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.');
+			// Ignore for now
+			//return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.');
+		}
+
+		if ($share === null) {
+			//For now federated shares are handled by the old endpoint.
+			return \OCA\Files_Sharing\API\Local::getShare(['id' => $id]);
 		}
 
 		if ($this->canAccessShare($share)) {
@@ -163,18 +173,19 @@ class Share20OCS {
 	 * @return \OC_OCS_Result
 	 */
 	public function deleteShare($id) {
+		// Try both our default and our federated provider
+		$share = null;
+
 		try {
-			$share = $this->shareManager->getShareById($id);
+			$share = $this->shareManager->getShareById('ocinternal:' . $id);
 		} catch (\OC\Share20\Exception\ShareNotFound $e) {
-			return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.');
+			//Ignore for now
+			//return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.');
 		}
 
-		/*
-		 * FIXME
-		 * User the old code path for remote shares until we have our remoteshareprovider
-		 */
-		if ($share->getShareType() === \OCP\Share::SHARE_TYPE_REMOTE) {
-			\OCA\Files_Sharing\API\Local::deleteShare(['id' => $id]);
+		// Could not find the share as internal share... maybe it is a federated share
+		if ($share === null) {
+			return \OCA\Files_Sharing\API\Local::deleteShare(['id' => $id]);
 		}
 
 		if (!$this->canAccessShare($share)) {

apps/files_sharing/tests/api/share20ocstest.php

@@ -82,7 +82,7 @@ class Share20OCSTest extends \Test\TestCase {
 		$this->shareManager
 			->expects($this->once())
 			->method('getShareById')
-			->with(42)
+			->with('ocinternal:42')
 			->will($this->throwException(new \OC\Share20\Exception\ShareNotFound()));
 
 		$expected = new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.');
@@ -95,7 +95,7 @@ class Share20OCSTest extends \Test\TestCase {
 		$this->shareManager
 			->expects($this->once())
 			->method('getShareById')
-			->with(42)
+			->with('ocinternal:42')
 			->willReturn($share);
 		$this->shareManager
 			->expects($this->once())
@@ -114,7 +114,7 @@ class Share20OCSTest extends \Test\TestCase {
 		$this->shareManager
 			->expects($this->once())
 			->method('getShareById')
-			->with(42)
+			->with('ocinternal:42')
 			->willReturn($share);
 		$this->shareManager
 			->expects($this->once())
@@ -125,16 +125,20 @@ class Share20OCSTest extends \Test\TestCase {
 		$this->assertEquals($expected, $this->ocs->deleteShare(42));
 	}
 
+	/*
+	 * FIXME: Enable once we have a federated Share Provider
+
 	public function testGetGetShareNotExists() {
 		$this->shareManager
 			->expects($this->once())
 			->method('getShareById')
-			->with(42)
+			->with('ocinternal:42')
 			->will($this->throwException(new \OC\Share20\Exception\ShareNotFound()));
 
 		$expected = new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.');
 		$this->assertEquals($expected, $this->ocs->getShare(42));
 	}
+	*/
 
 	public function createShare($id, $shareType, $sharedWith, $sharedBy, $shareOwner, $path, $permissions,
 								$shareTime, $expiration, $parent, $target, $mail_send, $token=null,
@@ -155,6 +159,12 @@ class Share20OCSTest extends \Test\TestCase {
 		$share->method('getToken')->willReturn($token);
 		$share->method('getPassword')->willReturn($password);
 
+		if ($shareType === \OCP\Share::SHARE_TYPE_USER  ||
+			$shareType === \OCP\Share::SHARE_TYPE_GROUP ||
+			$shareType === \OCP\Share::SHARE_TYPE_LINK) {
+			$share->method('getFullId')->willReturn('ocinternal:'.$id);
+		}
+
 		return $share;
 	}
 
@@ -345,7 +355,7 @@ class Share20OCSTest extends \Test\TestCase {
 		$this->shareManager
 			->expects($this->once())
 			->method('getShareById')
-			->with($share->getId())
+			->with($share->getFullId())
 			->willReturn($share);
 
 		$userFolder = $this->getMock('OCP\Files\Folder');

lib/private/share20/defaultshareprovider.php

@@ -29,6 +29,11 @@ use OCP\Files\IRootFolder;
 use OCP\IDBConnection;
 use OCP\Files\Node;
 
+/**
+ * Class DefaultShareProvider
+ *
+ * @package OC\Share20
+ */
 class DefaultShareProvider implements IShareProvider {
 
 	/** @var IDBConnection */
@@ -75,6 +80,15 @@ class DefaultShareProvider implements IShareProvider {
 		];
 	}
 
+	/**
+	 * Return the identifier of this provider.
+	 *
+	 * @return string Containing only [a-zA-Z0-9]
+	 */
+	public function identifier() {
+		return 'ocinternal';
+	}
+
 	/**
 	 * Share a path
 	 *
@@ -355,6 +369,8 @@ class DefaultShareProvider implements IShareProvider {
 			$share->setExpirationDate($expiration);
 		}
 
+		$share->setProviderId($this->identifier());
+
 		return $share;
 	}
 

lib/private/share20/ishare.php

@@ -35,11 +35,34 @@ interface IShare {
 	 */
 	public function getId();
 
+	/**
+	 * Set the id of the share
+	 *
+	 * @param string $id
+	 * @return IShare The modified share object
+	 */
+	public function setId($id);
+
+	/**
+	 * Get the full share id
+	 *
+	 * @return string
+	 */
+	public function getFullId();
+
+	/**
+	 * Set the provider id
+	 *
+	 * @param string $id
+	 * @return IShare The modified share object
+	 */
+	public function setProviderId($id);
+
 	/**
 	 * Set the path of this share
 	 *
 	 * @param Node $path
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setPath(Node $path);
 
@@ -54,7 +77,7 @@ interface IShare {
 	 * Set the shareType
 	 *
 	 * @param int $shareType
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setShareType($shareType);
 
@@ -69,7 +92,7 @@ interface IShare {
 	 * Set the receiver of this share
 	 *
 	 * @param IUser|IGroup|string
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setSharedWith($sharedWith);
 
@@ -84,7 +107,7 @@ interface IShare {
 	 * Set the permissions
 	 *
 	 * @param int $permissions
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setPermissions($permissions);
 
@@ -99,7 +122,7 @@ interface IShare {
 	 * Set the expiration date
 	 *
 	 * @param \DateTime $expireDate
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setExpirationDate($expireDate);
 
@@ -114,7 +137,7 @@ interface IShare {
 	 * Set the sharer of the path
 	 *
 	 * @param IUser|string $sharedBy
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setSharedBy($sharedBy);
 
@@ -130,7 +153,7 @@ interface IShare {
 	 *
 	 * @param IUser|string
 	 *
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setShareOwner($shareOwner);
 
@@ -146,7 +169,7 @@ interface IShare {
 	 *
 	 * @param string $password
 	 *
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setPassword($password);
 
@@ -161,7 +184,7 @@ interface IShare {
 	 * Set the token
 	 *
 	 * @param string $token
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setToken($token);
 
@@ -183,7 +206,7 @@ interface IShare {
 	 * Set the target of this share
 	 *
 	 * @param string $target
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setTarget($target);
 

lib/private/share20/ishareprovider.php

@@ -33,6 +33,13 @@ interface IShareProvider {
 	 */
 	public function shareTypes();
 
+	/**
+	 * Return the identifier of this provider.
+	 *
+	 * @return string Containing only [a-zA-Z0-9]
+	 */
+	public function identifier();
+
 	/**
 	 * Share a path
 	 * 

lib/private/share20/manager.php

@@ -21,10 +21,12 @@
 namespace OC\Share20;
 
 
+use OC\Share20\Exception\BackendError;
 use OC\Share20\Exception\ProviderException;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\ILogger;
+use OCP\Preview\IProvider;
 use OCP\Security\ISecureRandom;
 use OCP\Security\IHasher;
 use OCP\Files\Mount\IMountManager;
@@ -112,40 +114,45 @@ class Manager {
 	 */
 	private function addProviders(array $providers) {
 		foreach ($providers as $provider) {
-			$name = get_class($provider);
+			$id = $provider->identifier();
+			$class = get_class($provider);
 
 			if (!($provider instanceof IShareProvider)) {
-				throw new ProviderException($name . ' is not an instance of IShareProvider');
+				throw new ProviderException($class . ' is not an instance of IShareProvider');
 			}
 
-			if (isset($this->providers[$name])) {
-				throw new ProviderException($name . ' is already registered');
+			if (isset($this->providers[$id])) {
+				throw new ProviderException($id . ' is already registered');
 			}
 
-			$this->providers[$name] = $provider;
+			$this->providers[$id] = $provider;
 			$types = $provider->shareTypes();
 
 			if ($types === []) {
-				throw new ProviderException('Provider ' . $name . ' must supply share types it can handle');
+				throw new ProviderException('Provider ' . $id . ' must supply share types it can handle');
 			}
 
 			foreach ($types as $type) {
 				if (isset($this->type2provider[$type])) {
-					throw new ProviderException($name . ' tried to register for share type ' . $type . ' but this type is already handled by ' . $this->type2provider[$type]);
+					throw new ProviderException($id . ' tried to register for share type ' . $type . ' but this type is already handled by ' . $this->type2provider[$type]);
 				}
 
-				$this->type2provider[$type] = $name;
+				$this->type2provider[$type] = $id;
 			}
 		}
 	}
 
+	/**
+	 * Run the factory if this is not done yet
+	 *
+	 * @throws ProviderException
+	 */
 	private function runFactory() {
 		if (!empty($this->providers)) {
 			return;
 		}
 
 		$this->addProviders($this->factory->getProviders());
-		$this->factoryDone = true;
 	}
 
 	/**
@@ -187,6 +194,16 @@ class Manager {
 		return $this->providers;
 	}
 
+	/**
+	 * Convert from a full share id to a tuple (providerId, shareId)
+	 *
+	 * @param string $id
+	 * @return string[]
+	 */
+	private function splitFullId($id) {
+		return explode(':', $id, 2);
+	}
+
 	/**
 	 * Verify if a password meets all requirements
 	 *
@@ -335,7 +352,6 @@ class Manager {
 		return $expireDate;
 	}
 
-
 	/**
 	 * Check for pre share requirements for user shares
 	 *
@@ -549,6 +565,7 @@ class Manager {
 
 		$provider = $this->getProviderForType($share->getShareType());
 		$share = $provider->create($share);
+		$share->setProviderId($provider->identifier());
 
 		// Post share hook
 		$postHookData = [
@@ -585,16 +602,14 @@ class Manager {
 	protected function deleteChildren(IShare $share) {
 		$deletedShares = [];
 
-		$providers = $this->getProviders();
+		$provider = $this->getProviderForType($share->getShareType());
 
-		foreach($providers as $provider) {
-			foreach ($provider->getChildren($share) as $child) {
-				$deletedChildren = $this->deleteChildren($child);
-				$deletedShares = array_merge($deletedShares, $deletedChildren);
+		foreach ($provider->getChildren($share) as $child) {
+			$deletedChildren = $this->deleteChildren($child);
+			$deletedShares = array_merge($deletedShares, $deletedChildren);
 
-				$provider->delete($child);
-				$deletedShares[] = $child;
-			}
+			$provider->delete($child);
+			$deletedShares[] = $child;
 		}
 
 		return $deletedShares;
@@ -605,11 +620,12 @@ class Manager {
 	 *
 	 * @param IShare $share
 	 * @throws ShareNotFound
-	 * @throws \OC\Share20\Exception\BackendError
+	 * @throws BackendError
+	 * @throws ShareNotFound
 	 */
 	public function deleteShare(IShare $share) {
 		// Just to make sure we have all the info
-		$share = $this->getShareById($share->getId());
+		$share = $this->getShareById($share->getFullId());
 
 		$formatHookParams = function(IShare $share) {
 			// Prepare hook
@@ -686,24 +702,11 @@ class Manager {
 			throw new ShareNotFound();
 		}
 
-		//FIXME ids need to become proper providerid:shareid eventually
-
-		$providers = $this->getProviders();
-
-		$share = null;
-		foreach ($providers as $provider) {
-			try {
-				$share = $provider->getShareById($id);
-				$found = true;
-				break;
-			} catch (ShareNotFound $e) {
-				// Ignore
-			}
-		}
+		list($providerId, $id) = $this->splitFullId($id);
+		$provider = $this->getProvider($providerId);
 
-		if ($share === null) {
-			throw new ShareNotFound();
-		}
+		$share = $provider->getShareById($id);
+		$share->setProviderId($provider->identifier());
 
 		return $share;
 	}

lib/private/share20/share.php

@@ -28,6 +28,8 @@ class Share implements IShare {
 
 	/** @var string */
 	private $id;
+	/** @var string */
+	private $providerId;
 	/** @var Node */
 	private $path;
 	/** @var int */
@@ -59,7 +61,7 @@ class Share implements IShare {
 	 * Set the id of the share
 	 *
 	 * @param string $id
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setId($id) {
 		$this->id = $id;
@@ -75,11 +77,26 @@ class Share implements IShare {
 		return $this->id;
 	}
 
+	/**
+	 * @inheritdoc
+	 */
+	public function getFullId() {
+		return $this->providerId . ':' . $this->id;
+	}
+
+	/**
+	 * @inheritdoc
+	 */
+	public function setProviderId($id) {
+		$this->providerId = $id;
+		return $this;
+	}
+
 	/**
 	 * Set the path of this share
 	 *
 	 * @param Node $path
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setPath(Node $path) {
 		$this->path = $path;
@@ -99,7 +116,7 @@ class Share implements IShare {
 	 * Set the shareType
 	 *
 	 * @param int $shareType
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setShareType($shareType) {
 		$this->shareType = $shareType;
@@ -119,7 +136,7 @@ class Share implements IShare {
 	 * Set the receiver of this share
 	 *
 	 * @param IUser|IGroup|string
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setSharedWith($sharedWith) {
 		$this->sharedWith = $sharedWith;
@@ -139,7 +156,7 @@ class Share implements IShare {
 	 * Set the permissions
 	 *
 	 * @param int $permissions
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setPermissions($permissions) {
 		//TODO checkes
@@ -161,7 +178,7 @@ class Share implements IShare {
 	 * Set the expiration date
 	 *
 	 * @param \DateTime $expireDate
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setExpirationDate($expireDate) {
 		//TODO checks
@@ -183,7 +200,7 @@ class Share implements IShare {
 	 * Set the sharer of the path
 	 *
 	 * @param IUser|string $sharedBy
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setSharedBy($sharedBy) {
 		//TODO checks
@@ -207,7 +224,7 @@ class Share implements IShare {
 	 *
 	 * @param IUser|string
 	 *
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setShareOwner($shareOwner) {
 		//TODO checks
@@ -231,7 +248,7 @@ class Share implements IShare {
 	 *
 	 * @param string $password
 	 *
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setPassword($password) {
 		//TODO verify
@@ -253,7 +270,7 @@ class Share implements IShare {
 	 * Set the token
 	 *
 	 * @param string $token
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setToken($token) {
 		$this->token = $token;
@@ -273,7 +290,7 @@ class Share implements IShare {
 	 * Set the parent id of this share
 	 *
 	 * @param int $parent
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setParent($parent) {
 		$this->parent = $parent;
@@ -293,7 +310,7 @@ class Share implements IShare {
 	 * Set the target of this share
 	 *
 	 * @param string $target
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setTarget($target) {
 		$this->target = $target;
@@ -313,7 +330,7 @@ class Share implements IShare {
 	 * Set the time this share was created
 	 *
 	 * @param int $shareTime
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setShareTime($shareTime) {
 		$this->shareTime = $shareTime;
@@ -333,7 +350,7 @@ class Share implements IShare {
 	 * Set mailSend
 	 *
 	 * @param bool $mailSend
-	 * @return Share The modified object
+	 * @return IShare The modified object
 	 */
 	public function setMailSend($mailSend) {
 		$this->mailSend = $mailSend;

tests/lib/share20/defaultshareprovidertest.php

@@ -589,6 +589,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
 		$share2 = $this->provider->create($share);
 
 		$this->assertNotNull($share2->getId());
+		$this->assertSame('ocinternal:'.$share2->getId(), $share2->getFullId());
 		$this->assertSame(\OCP\Share::SHARE_TYPE_USER, $share2->getShareType());
 		$this->assertSame($sharedWith, $share2->getSharedWith());
 		$this->assertSame($sharedBy, $share2->getSharedBy());
@@ -651,6 +652,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
 		$share2 = $this->provider->create($share);
 
 		$this->assertNotNull($share2->getId());
+		$this->assertSame('ocinternal:'.$share2->getId(), $share2->getFullId());
 		$this->assertSame(\OCP\Share::SHARE_TYPE_GROUP, $share2->getShareType());
 		$this->assertSame($sharedWith, $share2->getSharedWith());
 		$this->assertSame($sharedBy, $share2->getSharedBy());
@@ -710,6 +712,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
 		$share2 = $this->provider->create($share);
 
 		$this->assertNotNull($share2->getId());
+		$this->assertSame('ocinternal:'.$share2->getId(), $share2->getFullId());
 		$this->assertSame(\OCP\Share::SHARE_TYPE_LINK, $share2->getShareType());
 		$this->assertSame($sharedBy, $share2->getSharedBy());
 		$this->assertSame($shareOwner, $share2->getShareOwner());

tests/lib/share20/managertest.php

@@ -109,6 +109,7 @@ class ManagerTest extends \Test\TestCase {
 				\OCP\Share::SHARE_TYPE_GROUP,
 				\OCP\Share::SHARE_TYPE_LINK,
 			]);
+		$this->defaultProvider->method('identifier')->willReturn('default');
 	}
 
 	/**
@@ -155,6 +156,7 @@ class ManagerTest extends \Test\TestCase {
 		$provider = $provider->getMock();
 
 		$provider->method('shareTypes')->willReturn($shareTypes);
+		$provider->method('identifier')->willReturn(get_class($provider));
 
 		return $provider;
 	}
@@ -169,7 +171,7 @@ class ManagerTest extends \Test\TestCase {
 
 		$share
 			->expects($this->once())
-			->method('getId')
+			->method('getFullId')
 			->with()
 			->willReturn(null);
 
@@ -204,7 +206,7 @@ class ManagerTest extends \Test\TestCase {
 			\OCP\Share::SHARE_TYPE_GROUP,
 			\OCP\Share::SHARE_TYPE_LINK,
 			\OCP\Share::SHARE_TYPE_REMOTE,
-		]);
+		], 'prov');
 		$this->factoryProviders([$provider]);
 
 		$sharedBy = $this->getMock('\OCP\IUser');
@@ -215,13 +217,14 @@ class ManagerTest extends \Test\TestCase {
 
 		$share = $this->getMock('\OC\Share20\IShare');
 		$share->method('getId')->willReturn(42);
+		$share->method('getFullId')->willReturn('prov:42');
 		$share->method('getShareType')->willReturn($shareType);
 		$share->method('getSharedWith')->willReturn($sharedWith);
 		$share->method('getSharedBy')->willReturn($sharedBy);
 		$share->method('getPath')->willReturn($path);
 		$share->method('getTarget')->willReturn('myTarget');
 
-		$manager->expects($this->once())->method('getShareById')->with(42)->willReturn($share);
+		$manager->expects($this->once())->method('getShareById')->with('prov:42')->willReturn($share);
 		$manager->expects($this->once())->method('deleteChildren')->with($share);
 
 		$provider
@@ -288,7 +291,11 @@ class ManagerTest extends \Test\TestCase {
 			->setMethods(['getShareById'])
 			->getMock();
 
-		$provider = $this->createShareProvider([\OCP\Share::SHARE_TYPE_USER]);
+		$provider = $this->createShareProvider([
+			\OCP\Share::SHARE_TYPE_USER,
+			\OCP\Share::SHARE_TYPE_GROUP,
+			\OCP\Share::SHARE_TYPE_LINK,
+		], 'prov');
 		$this->factoryProviders([$provider]);
 
 		$sharedBy1 = $this->getMock('\OCP\IUser');
@@ -308,6 +315,7 @@ class ManagerTest extends \Test\TestCase {
 
 		$share1 = $this->getMock('\OC\Share20\IShare');
 		$share1->method('getId')->willReturn(42);
+		$share1->method('getFullId')->willReturn('prov:42');
 		$share1->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER);
 		$share1->method('getSharedWith')->willReturn($sharedWith1);
 		$share1->method('getSharedBy')->willReturn($sharedBy1);
@@ -316,6 +324,7 @@ class ManagerTest extends \Test\TestCase {
 
 		$share2 = $this->getMock('\OC\Share20\IShare');
 		$share2->method('getId')->willReturn(43);
+		$share2->method('getFullId')->willReturn('prov:43');
 		$share2->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_GROUP);
 		$share2->method('getSharedWith')->willReturn($sharedWith2);
 		$share2->method('getSharedBy')->willReturn($sharedBy2);
@@ -325,13 +334,14 @@ class ManagerTest extends \Test\TestCase {
 
 		$share3 = $this->getMock('\OC\Share20\IShare');
 		$share3->method('getId')->willReturn(44);
+		$share3->method('getFullId')->willReturn('prov:44');
 		$share3->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);
 		$share3->method('getSharedBy')->willReturn($sharedBy3);
 		$share3->method('getPath')->willReturn($path);
 		$share3->method('getTarget')->willReturn('myTarget3');
 		$share3->method('getParent')->willReturn(43);
 
-		$manager->expects($this->once())->method('getShareById')->with(42)->willReturn($share1);
+		$manager->expects($this->once())->method('getShareById')->with('prov:42')->willReturn($share1);
 
 		$provider
 			->method('getChildren')
@@ -408,7 +418,6 @@ class ManagerTest extends \Test\TestCase {
 			],
 		];
 
-
 		$hookListner
 			->expects($this->exactly(1))
 			->method('pre')
@@ -430,10 +439,14 @@ class ManagerTest extends \Test\TestCase {
 		$this->factoryProviders([$provider]);
 
 		$share = $this->getMock('\OC\Share20\IShare');
+		$share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER);
 
 		$child1 = $this->getMock('\OC\Share20\IShare');
+		$child1->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER);
 		$child2 = $this->getMock('\OC\Share20\IShare');
+		$child2->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER);
 		$child3 = $this->getMock('\OC\Share20\IShare');
+		$child3->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER);
 
 		$shares = [
 			$child1,
@@ -471,7 +484,7 @@ class ManagerTest extends \Test\TestCase {
 			->with(42)
 			->willReturn($share);
 
-		$this->assertEquals($share, $this->manager->getShareById(42));
+		$this->assertEquals($share, $this->manager->getShareById('default:42'));
 	}
 
 	/**