Make the share object lazy

Share providers can now just pass in a fileId. And the node will only be
created once needed.

diff --git a/lib/private/server.php b/lib/private/server.php
index 69406c2203f616d367e4e2f15e42ef27ad478d7c..55ac64a0c2d152505ca9dd063406f9ff996cd35c 100644 (file)
--- a/lib/private/server.php
+++ b/lib/private/server.php
@@ -618,7 +618,8 @@ class Server extends ServerContainer implements IServerContainer {
                                $c->getGroupManager(),
                                $c->getL10N('core'),
                                $factory,
-                               $c->getUserManager()
+                               $c->getUserManager(),
+                               $c->getRootFolder()
                        );
 
                        return $manager;

diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php
index 38d1dae316d7999bb88bf39dce5c1aeede0419cb..48164a5d419cdc582eb3c0bdf41b7a3c26513b53 100644 (file)
--- a/lib/private/share20/defaultshareprovider.php
+++ b/lib/private/share20/defaultshareprovider.php
@@ -711,7 +711,7 @@ class DefaultShareProvider implements IShareProvider {
         * @throws InvalidShare
         */
        private function createShare($data) {
-               $share = new Share();
+               $share = new Share($this->rootFolder);
                $share->setId((int)$data['id'])
                        ->setShareType((int)$data['share_type'])
                        ->setPermissions((int)$data['permissions'])
@@ -744,8 +744,7 @@ class DefaultShareProvider implements IShareProvider {
                        $share->setShareOwner($data['uid_owner']);
                }
 
-               $path = $this->getNode($share->getShareOwner(), (int)$data['file_source']);
-               $share->setNode($path);
+               $share->setNodeId((int)$data['file_source']);
 
                if ($data['expiration'] !== null) {
                        $expiration = \DateTime::createFromFormat('Y-m-d H:i:s', $data['expiration']);

diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php
index db42f76d7a2f698ce9704053799f963b09db22dc..3db76d92ac7ec79553e09c5b8cc5a2dd48e67fa5 100644 (file)
--- a/lib/private/share20/manager.php
+++ b/lib/private/share20/manager.php
@@ -21,6 +21,7 @@
 
 namespace OC\Share20;
 
+use OCP\Files\IRootFolder;
 use OCP\IUserManager;
 use OCP\Share\IManager;
 use OCP\Share\IProviderFactory;
@@ -45,30 +46,24 @@ class Manager implements IManager {
 
        /** @var IProviderFactory */
        private $factory;
-
        /** @var ILogger */
        private $logger;
-
        /** @var IConfig */
        private $config;
-
        /** @var ISecureRandom */
        private $secureRandom;
-
        /** @var IHasher */
        private $hasher;
-
        /** @var IMountManager */
        private $mountManager;
-
        /** @var IGroupManager */
        private $groupManager;
-
        /** @var IL10N */
        private $l;
-
        /** @var IUserManager */
        private $userManager;
+       /** @var IRootFolder */
+       private $rootFolder;
 
        /**
         * Manager constructor.
@@ -92,7 +87,8 @@ class Manager implements IManager {
                        IGroupManager $groupManager,
                        IL10N $l,
                        IProviderFactory $factory,
-                       IUserManager $userManager
+                       IUserManager $userManager,
+                       IRootFolder $rootFolder
        ) {
                $this->logger = $logger;
                $this->config = $config;
@@ -103,6 +99,7 @@ class Manager implements IManager {
                $this->l = $l;
                $this->factory = $factory;
                $this->userManager = $userManager;
+               $this->rootFolder = $rootFolder;
        }
 
        /**
@@ -888,7 +885,7 @@ class Manager implements IManager {
         * @return \OCP\Share\IShare;
         */
        public function newShare() {
-               return new \OC\Share20\Share();
+               return new \OC\Share20\Share($this->rootFolder);
        }
 
        /**

diff --git a/lib/private/share20/share.php b/lib/private/share20/share.php
index db91ad4a91d30d1e36644f60fdf5f52275e0947f..5b7945b1da8b04ef12fabad94ffceeec03b21462 100644 (file)
--- a/lib/private/share20/share.php
+++ b/lib/private/share20/share.php
@@ -20,7 +20,9 @@
  */
 namespace OC\Share20;
 
+use OCP\Files\IRootFolder;
 use OCP\Files\Node;
+use OCP\Files\NotFoundException;
 use OCP\IUser;
 use OCP\IGroup;
 
@@ -31,14 +33,16 @@ class Share implements \OCP\Share\IShare {
        /** @var string */
        private $providerId;
        /** @var Node */
-       private $path;
+       private $node;
+       /** @var int */
+       private $fileId;
        /** @var int */
        private $shareType;
-       /** @var IUser|IGroup */
+       /** @var string */
        private $sharedWith;
-       /** @var IUser */
+       /** @var string */
        private $sharedBy;
-       /** @var IUser */
+       /** @var string */
        private $shareOwner;
        /** @var int */
        private $permissions;
@@ -57,6 +61,13 @@ class Share implements \OCP\Share\IShare {
        /** @var bool */
        private $mailSend;
 
+       /** @var IRootFolder */
+       private $rootFolder;
+
+       public function __construct(IRootFolder $rootFolder) {
+               $this->rootFolder = $rootFolder;
+       }
+
        /**
         * @inheritdoc
         */
@@ -90,8 +101,8 @@ class Share implements \OCP\Share\IShare {
        /**
         * @inheritdoc
         */
-       public function setNode(Node $path) {
-               $this->path = $path;
+       public function setNode(Node $node) {
+               $this->node = $node;
                return $this;
        }
 
@@ -99,7 +110,32 @@ class Share implements \OCP\Share\IShare {
         * @inheritdoc
         */
        public function getNode() {
-               return $this->path;
+               if ($this->node === null) {
+
+                       if ($this->shareOwner === null || $this->fileId === null) {
+                               throw new NotFoundException();
+                       }
+
+                       $userFolder = $this->rootFolder->getUserFolder($this->shareOwner);
+
+                       $nodes = $userFolder->getById($this->fileId);
+                       if (empty($nodes)) {
+                               throw new NotFoundException();
+                       }
+
+                       $this->node = $nodes[0];
+               }
+
+               return $this->node;
+       }
+
+       /**
+        * @inheritdoc
+        */
+       public function setNodeId($fileId) {
+               $this->node = null;
+               $this->fileId = $fileId;
+               return $this;
        }
 
        /**

diff --git a/lib/public/share/ishare.php b/lib/public/share/ishare.php
index 5a82436c720e743487583fc58ebcd7b8e093cc09..074f411758466903203936a9685a50bf0698cccd 100644 (file)
--- a/lib/public/share/ishare.php
+++ b/lib/public/share/ishare.php
@@ -24,8 +24,7 @@ namespace OCP\Share;
 use OCP\Files\File;
 use OCP\Files\Folder;
 use OCP\Files\Node;
-use OCP\IUser;
-use OCP\IGroup;
+use OCP\Files\NotFoundException;
 
 /**
  * Interface IShare
@@ -55,20 +54,29 @@ interface IShare {
        /**
         * Set the node of the file/folder that is shared
         *
-        * @param File|Folder $path
+        * @param Node $node
         * @return \OCP\Share\IShare The modified object
         * @since 9.0.0
         */
-       public function setNode(Node $path);
+       public function setNode(Node $node);
 
        /**
         * Get the node of the file/folder that is shared
         *
         * @return File|Folder
         * @since 9.0.0
+        * @throws NotFoundException
         */
        public function getNode();
 
+       /**
+        * Set file id for lazy evaluation of the node
+        * @param int $fileId
+        * @return \OCP\Share\IShare The modified object
+        * @since 9.0.0
+        */
+       public function setNodeId($fileId);
+
        /**
         * Set the shareType
         *

diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php
index 19f21fba7b1d9b7591eb9bb01701c811359b180b..1f3fa9af32e8533748eaad9563529e4c02e64a78 100644 (file)
--- a/tests/lib/share20/defaultshareprovidertest.php
+++ b/tests/lib/share20/defaultshareprovidertest.php
@@ -561,7 +561,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
        }
 
        public function testCreateUserShare() {
-               $share = new \OC\Share20\Share();
+               $share = new \OC\Share20\Share($this->rootFolder);
 
                $shareOwner = $this->getMock('OCP\IUser');
                $shareOwner->method('getUID')->WillReturn('shareOwner');
@@ -609,7 +609,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
        }
 
        public function testCreateGroupShare() {
-               $share = new \OC\Share20\Share();
+               $share = new \OC\Share20\Share($this->rootFolder);
 
                $shareOwner = $this->getMock('\OCP\IUser');
                $shareOwner->method('getUID')->willReturn('shareOwner');
@@ -657,7 +657,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
        }
 
        public function testCreateLinkShare() {
-               $share = new \OC\Share20\Share();
+               $share = new \OC\Share20\Share($this->rootFolder);
 
                $shareOwner = $this->getMock('\OCP\IUser');
                $shareOwner->method('getUID')->willReturn('shareOwner');

diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php
index aa286cc4719bf5b3159fc02285299958c3db888d..01e5283fcf7ee553cc1701f1ff5e369a47f0cdb6 100644 (file)
--- a/tests/lib/share20/managertest.php
+++ b/tests/lib/share20/managertest.php
@@ -20,6 +20,7 @@
  */
 namespace Test\Share20;
 
+use OCP\Files\IRootFolder;
 use OCP\IUserManager;
 use OCP\Share\IProviderFactory;
 use OCP\Share\IShare;
@@ -35,7 +36,6 @@ use OCP\Security\ISecureRandom;
 use OCP\Security\IHasher;
 use OCP\Files\Mount\IMountManager;
 use OCP\IGroupManager;
-use Sabre\VObject\Property\VCard\DateTime;
 
 /**
  * Class ManagerTest
@@ -47,36 +47,28 @@ class ManagerTest extends \Test\TestCase {
 
        /** @var Manager */
        protected $manager;
-
        /** @var ILogger */
        protected $logger;
-
        /** @var IConfig */
        protected $config;
-
        /** @var ISecureRandom */
        protected $secureRandom;
-
        /** @var IHasher */
        protected $hasher;
-
        /** @var IShareProvider | \PHPUnit_Framework_MockObject_MockObject */
        protected $defaultProvider;
-
        /** @var  IMountManager */
        protected $mountManager;
-
        /** @var  IGroupManager */
        protected $groupManager;
-
        /** @var IL10N */
        protected $l;
-
        /** @var DummyFactory */
        protected $factory;
-
        /** @var IUserManager */
        protected $userManager;
+       /** @var IRootFolder | \PHPUnit_Framework_MockObject_MockObject */
+       protected $rootFolder;
 
        public function setUp() {
                
@@ -87,6 +79,7 @@ class ManagerTest extends \Test\TestCase {
                $this->mountManager = $this->getMock('\OCP\Files\Mount\IMountManager');
                $this->groupManager = $this->getMock('\OCP\IGroupManager');
                $this->userManager = $this->getMock('\OCP\IUserManager');
+               $this->rootFolder = $this->getMock('\OCP\Files\IRootFolder');
 
                $this->l = $this->getMock('\OCP\IL10N');
                $this->l->method('t')
@@ -105,7 +98,8 @@ class ManagerTest extends \Test\TestCase {
                        $this->groupManager,
                        $this->l,
                        $this->factory,
-                       $this->userManager
+                       $this->userManager,
+                       $this->rootFolder
                );
 
                $this->defaultProvider = $this->getMockBuilder('\OC\Share20\DefaultShareProvider')
@@ -131,7 +125,8 @@ class ManagerTest extends \Test\TestCase {
                                $this->groupManager,
                                $this->l,
                                $this->factory,
-                               $this->userManager
+                               $this->userManager,
+                               $this->rootFolder
                        ]);
        }
 
@@ -792,7 +787,7 @@ class ManagerTest extends \Test\TestCase {
         * @expectedExceptionMessage Only sharing with group members is allowed
         */
        public function testUserCreateChecksShareWithGroupMembersOnlyDifferentGroups() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $sharedBy = $this->getMock('\OCP\IUser');
                $sharedWith = $this->getMock('\OCP\IUser');
@@ -822,7 +817,7 @@ class ManagerTest extends \Test\TestCase {
        }
 
        public function testUserCreateChecksShareWithGroupMembersOnlySharedGroup() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $sharedBy = $this->getMock('\OCP\IUser');
                $sharedWith = $this->getMock('\OCP\IUser');
@@ -904,8 +899,7 @@ class ManagerTest extends \Test\TestCase {
                        ->setProviderId('foo')
                        ->setId('bar');
 
-               $share2 = new \OC\Share20\Share();
-               $owner2 = $this->getMock('\OCP\IUser');
+               $share2 = $this->manager->newShare();
                $share2->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
                        ->setShareOwner('shareOwner2')
                        ->setProviderId('foo')
@@ -928,7 +922,7 @@ class ManagerTest extends \Test\TestCase {
        }
 
        public function testUserCreateChecksIdenticalPathNotSharedWithUser() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
                $sharedWith = $this->getMock('\OCP\IUser');
                $path = $this->getMock('\OCP\Files\Node');
                $share->setSharedWith('sharedWith')
@@ -939,7 +933,7 @@ class ManagerTest extends \Test\TestCase {
 
                $this->userManager->method('get')->with('sharedWith')->willReturn($sharedWith);
 
-               $share2 = new \OC\Share20\Share();
+               $share2 = $this->manager->newShare();
                $share2->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
                        ->setShareOwner('shareOwner2')
                        ->setProviderId('foo')
@@ -967,7 +961,7 @@ class ManagerTest extends \Test\TestCase {
         * @expectedExceptionMessage Only sharing within your own groups is allowed
         */
        public function testGroupCreateChecksShareWithGroupMembersOnlyNotInGroup() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $user = $this->getMock('\OCP\IUser');
                $group = $this->getMock('\OCP\IGroup');
@@ -988,7 +982,7 @@ class ManagerTest extends \Test\TestCase {
        }
 
        public function testGroupCreateChecksShareWithGroupMembersOnlyInGroup() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $user = $this->getMock('\OCP\IUser');
                $group = $this->getMock('\OCP\IGroup');
@@ -1028,7 +1022,7 @@ class ManagerTest extends \Test\TestCase {
                        ->setProviderId('foo')
                        ->setId('bar');
 
-               $share2 = new \OC\Share20\Share();
+               $share2 = $this->manager->newShare();
                $share2->setSharedWith('sharedWith')
                        ->setProviderId('foo')
                        ->setId('baz');
@@ -1041,14 +1035,14 @@ class ManagerTest extends \Test\TestCase {
        }
 
        public function testGroupCreateChecksPathAlreadySharedWithDifferentGroup() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $share->setSharedWith('sharedWith');
 
                $path = $this->getMock('\OCP\Files\Node');
                $share->setNode($path);
 
-               $share2 = new \OC\Share20\Share();
+               $share2 = $this->manager->newShare();
                $share2->setSharedWith('sharedWith2');
 
                $this->defaultProvider->method('getSharesByPath')
@@ -1063,7 +1057,7 @@ class ManagerTest extends \Test\TestCase {
         * @expectedExceptionMessage Link sharing not allowed
         */
        public function testLinkCreateChecksNoLinkSharesAllowed() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $this->config
                        ->method('getAppValue')
@@ -1079,7 +1073,7 @@ class ManagerTest extends \Test\TestCase {
         * @expectedExceptionMessage Link shares can't have reshare permissions
         */
        public function testLinkCreateChecksSharePermissions() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $share->setPermissions(\OCP\Constants::PERMISSION_SHARE);
 
@@ -1097,7 +1091,7 @@ class ManagerTest extends \Test\TestCase {
         * @expectedExceptionMessage Link shares can't have delete permissions
         */
        public function testLinkCreateChecksDeletePermissions() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $share->setPermissions(\OCP\Constants::PERMISSION_DELETE);
 
@@ -1115,7 +1109,7 @@ class ManagerTest extends \Test\TestCase {
         * @expectedExceptionMessage Public upload not allowed
         */
        public function testLinkCreateChecksNoPublicUpload() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $share->setPermissions(\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);
 
@@ -1130,7 +1124,7 @@ class ManagerTest extends \Test\TestCase {
        }
 
        public function testLinkCreateChecksPublicUpload() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $share->setPermissions(\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE);
 
@@ -1145,7 +1139,7 @@ class ManagerTest extends \Test\TestCase {
        }
 
        public function testLinkCreateChecksReadOnly() {
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
 
                $share->setPermissions(\OCP\Constants::PERMISSION_READ);
 
@@ -1316,7 +1310,7 @@ class ManagerTest extends \Test\TestCase {
                        ->getMock();
 
                $manager->expects($this->once())->method('canShare')->willReturn(false);
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
                $manager->createShare($share);
        }
 
@@ -1610,7 +1604,8 @@ class ManagerTest extends \Test\TestCase {
                        $this->groupManager,
                        $this->l,
                        $factory,
-                       $this->userManager
+                       $this->userManager,
+                       $this->rootFolder
                );
 
                $share = $this->getMock('\OCP\Share\IShare');
@@ -1695,7 +1690,7 @@ class ManagerTest extends \Test\TestCase {
                        ->getMock();
 
                $manager->expects($this->once())->method('canShare')->willReturn(false);
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
                $manager->updateShare($share);
        }
 
@@ -1711,13 +1706,13 @@ class ManagerTest extends \Test\TestCase {
                        ])
                        ->getMock();
 
-               $originalShare = new \OC\Share20\Share();
+               $originalShare = $this->manager->newShare();
                $originalShare->setShareType(\OCP\Share::SHARE_TYPE_GROUP);
 
                $manager->expects($this->once())->method('canShare')->willReturn(true);
                $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
 
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
                $share->setProviderId('foo')
                        ->setId('42')
                        ->setShareType(\OCP\Share::SHARE_TYPE_USER);
@@ -1737,14 +1732,14 @@ class ManagerTest extends \Test\TestCase {
                        ])
                        ->getMock();
 
-               $originalShare = new \OC\Share20\Share();
+               $originalShare = $this->manager->newShare();
                $originalShare->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
                        ->setSharedWith('origGroup');
 
                $manager->expects($this->once())->method('canShare')->willReturn(true);
                $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
 
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
                $share->setProviderId('foo')
                        ->setId('42')
                        ->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
@@ -1765,14 +1760,14 @@ class ManagerTest extends \Test\TestCase {
                        ])
                        ->getMock();
 
-               $originalShare = new \OC\Share20\Share();
+               $originalShare = $this->manager->newShare();
                $originalShare->setShareType(\OCP\Share::SHARE_TYPE_USER)
                        ->setSharedWith('sharedWith');
 
                $manager->expects($this->once())->method('canShare')->willReturn(true);
                $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
 
-               $share = new \OC\Share20\Share();
+               $share = $this->manager->newShare();
                $share->setProviderId('foo')
                        ->setId('42')
                        ->setShareType(\OCP\Share::SHARE_TYPE_USER)
@@ -1793,19 +1788,22 @@ class ManagerTest extends \Test\TestCase {
                        ])
                        ->getMock();
 
-               $originalShare = new \OC\Share20\Share();
+               $originalShare = $this->manager->newShare();
                $originalShare->setShareType(\OCP\Share::SHARE_TYPE_USER)
                        ->setSharedWith('origUser');
 
                $manager->expects($this->once())->method('canShare')->willReturn(true);
                $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
 
-               $share = new \OC\Share20\Share();
+               $node = $this->getMock('\OCP\Files\File');
+
+               $share = $this->manager->newShare();
                $share->setProviderId('foo')
                        ->setId('42')
                        ->setShareType(\OCP\Share::SHARE_TYPE_USER)
                        ->setSharedWith('origUser')
-                       ->setShareOwner('newUser');
+                       ->setShareOwner('newUser')
+                       ->setNode($node);
 
                $this->defaultProvider->expects($this->once())
                        ->method('update')
@@ -1831,19 +1829,22 @@ class ManagerTest extends \Test\TestCase {
                        ])
                        ->getMock();
 
-               $originalShare = new \OC\Share20\Share();
+               $originalShare = $this->manager->newShare();
                $originalShare->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
                        ->setSharedWith('origUser');
 
                $manager->expects($this->once())->method('canShare')->willReturn(true);
                $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
 
-               $share = new \OC\Share20\Share();
+               $node = $this->getMock('\OCP\Files\File');
+
+               $share = $this->manager->newShare();
                $share->setProviderId('foo')
                        ->setId('42')
                        ->setShareType(\OCP\Share::SHARE_TYPE_GROUP)
                        ->setSharedWith('origUser')
-                       ->setShareOwner('owner');
+                       ->setShareOwner('owner')
+                       ->setNode($node);
 
                $this->defaultProvider->expects($this->once())
                        ->method('update')
@@ -1871,7 +1872,7 @@ class ManagerTest extends \Test\TestCase {
                        ])
                        ->getMock();
 
-               $originalShare = new \OC\Share20\Share();
+               $originalShare = $this->manager->newShare();
                $originalShare->setShareType(\OCP\Share::SHARE_TYPE_LINK);
 
                $tomorrow = new \DateTime();