[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.

diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php
index f81eb071a37c6c07570eb881fdce7b6dce487011..c698dccefb8ae3a33f8da48f7084d654f019c23d 100644 (file)
--- a/apps/files_sharing/api/share20ocs.php
+++ b/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)) {

diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php
index b594d253eb29898fbbaf93f04f6c9b895ee9c54e..81166b9e3c41c47dcb50022e7539c18fbc1906b0 100644 (file)
--- a/apps/files_sharing/tests/api/share20ocstest.php
+++ b/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');

diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php
index f961bc5202fc577b9c04cae7cc18a1939de7bf78..60c3c4390fc6751b98ed4e335b3abd3e1207b8b2 100644 (file)
--- a/lib/private/share20/defaultshareprovider.php
+++ b/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;
        }
 

diff --git a/lib/private/share20/ishare.php b/lib/private/share20/ishare.php
index 53cfe76916df4a41c260e3561cb65922f3e8d9a2..34d1dfa4d3d632c0bb4a9c301d87f289de2fc2d0 100644 (file)
--- a/lib/private/share20/ishare.php
+++ b/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);
 

diff --git a/lib/private/share20/ishareprovider.php b/lib/private/share20/ishareprovider.php
index a7e9c59a97243cfaedaa44c76ff225944a31d123..e3a8e3c55b069073a83dae090a0a872c83de2d2f 100644 (file)
--- a/lib/private/share20/ishareprovider.php
+++ b/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
         * 

diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php
index e35b69f0031458c823172f6b898d37032c25e32a..0784a53ecc2997bfead9f2b604a9d26d14cb3c8b 100644 (file)
--- a/lib/private/share20/manager.php
+++ b/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;
        }

diff --git a/lib/private/share20/share.php b/lib/private/share20/share.php
index 2cd685b79638274dcaee0e1ef0117cdba8e66305..ee43725d9bcacf7ee1b0f3e9f0d45538d806d946 100644 (file)
--- a/lib/private/share20/share.php
+++ b/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;

diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php
index beef4c9ef535c7edd607b7be21102370a03b30ef..166fbd25f1764c08b45a32d44d2a8909dbb2476e 100644 (file)
--- a/tests/lib/share20/defaultshareprovidertest.php
+++ b/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());

diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php
index dd1028c78b477d2310be2e5b443d73fad670b8a0..93e4bbb2f6e32e98a2baeedeaa9107744f940077 100644 (file)
--- a/tests/lib/share20/managertest.php
+++ b/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'));
        }
 
        /**