update unit tests

Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>

diff --git a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php
index 61f87e9ec67d628eecd49e213e3872b72458ff03..233395dec9f9e1a47ad033a3383d031a2dfd10b3 100644 (file)
--- a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php
+++ b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php
@@ -67,7 +67,7 @@ class RequestHandlerControllerTest extends TestCase {
 
        /** @var  \OCA\FederatedFileSharing\AddressHandler|\PHPUnit_Framework_MockObject_MockObject */
        private $addressHandler;
-       
+
        /** @var  IUserManager|\PHPUnit_Framework_MockObject_MockObject */
        private $userManager;
 
@@ -107,7 +107,7 @@ class RequestHandlerControllerTest extends TestCase {
                $this->userManager = $this->getMockBuilder('OCP\IUserManager')->getMock();
 
                $this->cloudIdManager = new CloudIdManager();
-               
+
                $this->registerHttpHelper($httpHelperMock);
 
                $this->s2s = new RequestHandlerController(
@@ -384,6 +384,7 @@ class RequestHandlerControllerTest extends TestCase {
                        'parent' => null,
                        'accepted' => '0',
                        'expiration' => null,
+                       'password' => null,
                        'mail_send' => '0'
                ];
 

diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index b810a5150822afc5deb17bb446fef0af81ee4ec6..bc525b6ef8218ced2139c38f01763fd049f530db 100644 (file)
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -669,13 +669,14 @@ class ShareAPIController extends OCSController {
                        throw new OCSNotFoundException($this->l->t('Wrong share ID, share doesn\'t exist'));
                }
 
+               if ($permissions === null && $password === null && $publicUpload === null && $expireDate === null) {
+                       throw new OCSBadRequestException($this->l->t('Wrong or no update parameter given'));
+               }
+
                /*
                 * expirationdate, password and publicUpload only make sense for link shares
                 */
                if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
-                       if ($permissions === null && $password === null && $publicUpload === null && $expireDate === null) {
-                               throw new OCSBadRequestException($this->l->t('Wrong or no update parameter given'));
-                       }
 
                        $newPermissions = null;
                        if ($publicUpload === 'true') {

diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php
index 540905a7dc9067c646ffb29846cd7b353184365d..046ede1f83e6a5ca520824d6bd6cd37e561376e0 100644 (file)
--- a/apps/files_sharing/tests/ApiTest.php
+++ b/apps/files_sharing/tests/ApiTest.php
@@ -936,36 +936,6 @@ class ApiTest extends TestCase {
                $this->shareManager->deleteShare($share2);
        }
 
-       /**
-        * @medium
-        * @depends testCreateShareUserFile
-        */
-       public function testUpdateShareInvalidPermissions() {
-               $node1 = $this->userFolder->get($this->filename);
-               $share1 = $this->shareManager->newShare();
-               $share1->setNode($node1)
-                       ->setSharedBy(self::TEST_FILES_SHARING_API_USER1)
-                       ->setSharedWith(self::TEST_FILES_SHARING_API_USER2)
-                       ->setShareType(\OCP\Share::SHARE_TYPE_USER)
-                       ->setPermissions(19);
-               $share1 = $this->shareManager->createShare($share1);
-
-               $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1);
-               try {
-                       $ocs->updateShare($share1->getId());
-                       $this->fail();
-               } catch (OCSBadRequestException $e) {
-
-               }
-               $ocs->cleanup();
-
-               //Permissions should not have changed!
-               $share1 = $this->shareManager->getShareById('ocinternal:' . $share1->getId());
-               $this->assertEquals(19, $share1->getPermissions());
-
-               $this->shareManager->deleteShare($share1);
-       }
-
        /**
         * @medium
         */

diff --git a/apps/files_sharing/tests/MigrationTest.php b/apps/files_sharing/tests/MigrationTest.php
index a357e814f25a2281de68ba4993d48ee99d6933b8..708de1c0eca5b4cd0f88c3116fe66bca0db5675f 100644 (file)
--- a/apps/files_sharing/tests/MigrationTest.php
+++ b/apps/files_sharing/tests/MigrationTest.php
@@ -429,11 +429,11 @@ class MigrationTest extends TestCase {
 
                foreach ($allShares as $share) {
                        if ((int)$share['share_type'] === Share::SHARE_TYPE_LINK) {
-                               $this->assertSame(null, $share['share_with']);
+                               $this->assertNull( $share['share_with']);
                                $this->assertSame('shareWith', $share['password']);
                        } else {
                                $this->assertSame('shareWith', $share['share_with']);
-                               $this->assertSame(null, $share['password']);
+                               $this->assertNull($share['password']);
                        }
                }
        }

diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php
index 4ec62dc1a0302acac64d7f0eda01e3bb36a54a01..288ebb4bb452a250e78e312940fe3a3432f2248b 100644 (file)
--- a/apps/sharebymail/tests/ShareByMailProviderTest.php
+++ b/apps/sharebymail/tests/ShareByMailProviderTest.php
@@ -23,7 +23,6 @@
 namespace OCA\ShareByMail\Tests;
 
 
-use OC\HintException;
 use OCA\ShareByMail\Settings\SettingsManager;
 use OCA\ShareByMail\ShareByMailProvider;
 use OCP\Files\IRootFolder;
@@ -33,9 +32,7 @@ use OCP\ILogger;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\Mail\IMailer;
-use OCP\Security\IHasher;
 use OCP\Security\ISecureRandom;
-use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
 use Test\TestCase;
@@ -127,7 +124,8 @@ class ShareByMailProviderTest extends TestCase {
                                        $this->logger,
                                        $this->mailer,
                                        $this->urlGenerator,
-                                       $this->activityManager
+                                       $this->activityManager,
+                                       $this->settingsManager
                                ]
                        );
 
@@ -318,7 +316,7 @@ class ShareByMailProviderTest extends TestCase {
                $this->share->expects($this->once())->method('getPermissions')->willReturn($permissions + 1);
                $this->share->expects($this->once())->method('getShareOwner')->willReturn($uidOwner);
                $this->share->expects($this->once())->method('getSharedBy')->willReturn($sharedBy);
-               $this->share->expects($this->once())->method('getId')->willReturn($id);
+               $this->share->expects($this->atLeastOnce())->method('getId')->willReturn($id);
 
                $this->assertSame($this->share,
                        $instance->update($this->share)

diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js
index 3c234f54665e897e6cd8093316f7805d44e81802..573d594911809bd957f80f88a0a635b2d5458224 100644 (file)
--- a/core/js/sharedialogshareelistview.js
+++ b/core/js/sharedialogshareelistview.js
@@ -510,7 +510,7 @@
                                passwordField.attr('value', '');
                                passwordField.attr('placeholder', PASSWORD_PLACEHOLDER_MESSAGE);
                        } else {
-                               var passwordField = '#passwordField-' + this.cid + '-' + shareId;
+                               passwordField = '#passwordField-' + this.cid + '-' + shareId;
                                this.$(passwordField).focus();
                        }
                },
@@ -623,10 +623,9 @@
                        var $li = $element.closest('li[data-share-id]');
                        var shareId = $li.data('share-id');
 
+                       var permissions = OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE | OC.PERMISSION_READ;
                        if ($element.is(':checked')) {
-                               var permissions = OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE;
-                       } else {
-                               var permissions = OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE | OC.PERMISSION_READ;
+                               permissions = OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE;
                        }
 
                        /** disable checkboxes during save operation to avoid race conditions **/

diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php
index 0d02123e00135bc8990ef7161658fa5d2f1cbe23..bf8bcc9c6d99042dbd18f490787ab2b46104c72b 100644 (file)
--- a/lib/private/Share20/DefaultShareProvider.php
+++ b/lib/private/Share20/DefaultShareProvider.php
@@ -116,7 +116,7 @@ class DefaultShareProvider implements IShareProvider {
 
                        //If a password is set store it
                        if ($share->getPassword() !== null) {
-                               $qb->setValue('share_with', $qb->createNamedParameter($share->getPassword()));
+                               $qb->setValue('password', $qb->createNamedParameter($share->getPassword()));
                        }
 
                        //If an expiration date is set store it

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index 0dade2f7126c6f08a25f8ec0f2922294936f5df0..292b07d28d562a7f15ea90ecb92d3278b9de3e3d 100644 (file)
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -266,8 +266,8 @@ class Manager implements IManager {
 
                // Check that read permissions are always set
                // Link shares are allowed to have no read permissions to allow upload to hidden folders
-               $noReadPermissionRequired = $share->getShareType() !== \OCP\Share::SHARE_TYPE_LINK
-                       || $share->getShareType() !== \OCP\Share::SHARE_TYPE_EMAIL;
+               $noReadPermissionRequired = $share->getShareType() === \OCP\Share::SHARE_TYPE_LINK
+                       || $share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL;
                if (!$noReadPermissionRequired &&
                        ($share->getPermissions() & \OCP\Constants::PERMISSION_READ) === 0) {
                        throw new \InvalidArgumentException('Shares need at least read permissions');
@@ -936,59 +936,49 @@ class Manager implements IManager {
                 * Work around so we don't return expired shares but still follow
                 * proper pagination.
                 */
-               if ($shareType === \OCP\Share::SHARE_TYPE_LINK) {
-                       $shares2 = [];
 
-                       while(true) {
-                               $added = 0;
-                               foreach ($shares as $share) {
+               $shares2 = [];
 
-                                       $added++;
-                                       $shares2[] = $share;
+               while(true) {
+                       $added = 0;
+                       foreach ($shares as $share) {
 
-                                       if (count($shares2) === $limit) {
-                                               break;
-                                       }
+                               try {
+                                       $this->checkExpireDate($share);
+                               } catch (ShareNotFound $e) {
+                                       //Ignore since this basically means the share is deleted
+                                       continue;
                                }
 
-                               if (count($shares2) === $limit) {
-                                       break;
-                               }
+                               $added++;
+                               $shares2[] = $share;
 
-                               // If there was no limit on the select we are done
-                               if ($limit === -1) {
+                               if (count($shares2) === $limit) {
                                        break;
                                }
+                       }
 
-                               $offset += $added;
-
-                               // Fetch again $limit shares
-                               $shares = $provider->getSharesBy($userId, $shareType, $path, $reshares, $limit, $offset);
+                       if (count($shares2) === $limit) {
+                               break;
+                       }
 
-                               // No more shares means we are done
-                               if (empty($shares)) {
-                                       break;
-                               }
+                       // If there was no limit on the select we are done
+                       if ($limit === -1) {
+                               break;
                        }
 
-                       $shares = $shares2;
-               }
+                       $offset += $added;
 
+                       // Fetch again $limit shares
+                       $shares = $provider->getSharesBy($userId, $shareType, $path, $reshares, $limit, $offset);
 
-               // remove all shares which are already expired
-               foreach ($shares as $key => $share) {
-                       try {
-                               $this->checkExpireDate($share);
-                       } catch (ShareNotFound $e) {
-                               unset($shares[$key]);
-                               try {
-                                       $this->deleteShare($share);
-                               } catch (NotFoundException $e) {
-                                       //Ignore since this basically means the share is deleted
-                               }
+                       // No more shares means we are done
+                       if (empty($shares)) {
+                               break;
                        }
                }
 
+               $shares = $shares2;
 
                return $shares;
        }
@@ -1011,11 +1001,6 @@ class Manager implements IManager {
                                $this->checkExpireDate($share);
                        } catch (ShareNotFound $e) {
                                unset($shares[$key]);
-                               try {
-                                       $this->deleteShare($share);
-                               } catch (NotFoundException $e) {
-                                       //Ignore since this basically means the share is deleted
-                               }
                        }
                }
 

diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php
index 5b552b51c3c8520c3a798a026e06949876678f3a..8deec573c1ba1de48426846551b53442c4533897 100644 (file)
--- a/lib/public/Share/IShare.php
+++ b/lib/public/Share/IShare.php
@@ -189,7 +189,7 @@ interface IShare {
        /**
         * Set the expiration date
         *
-        * @param \DateTime $expireDate
+        * @param null|\DateTime $expireDate
         * @return \OCP\Share\IShare The modified object
         * @since 9.0.0
         */

diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php
index a1e783135589d3818e66542598844eec6a385ba2..fce5668440d3aebf9978a7ed76699f2b04a63876 100644 (file)
--- a/tests/lib/Share20/DefaultShareProviderTest.php
+++ b/tests/lib/Share20/DefaultShareProviderTest.php
@@ -338,7 +338,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
                $qb->insert('share')
                        ->values([
                                'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_LINK),
-                               'share_with' => $qb->expr()->literal('sharedWith'),
+                               'password' => $qb->expr()->literal('password'),
                                'uid_owner' => $qb->expr()->literal('shareOwner'),
                                'uid_initiator' => $qb->expr()->literal('sharedBy'),
                                'item_type'   => $qb->expr()->literal('file'),
@@ -366,7 +366,8 @@ class DefaultShareProviderTest extends \Test\TestCase {
 
                $this->assertEquals($id, $share->getId());
                $this->assertEquals(\OCP\Share::SHARE_TYPE_LINK, $share->getShareType());
-               $this->assertEquals('sharedWith', $share->getPassword());
+               $this->assertNull($share->getSharedWith());
+               $this->assertEquals('password', $share->getPassword());
                $this->assertEquals('sharedBy', $share->getSharedBy());
                $this->assertEquals('shareOwner', $share->getShareOwner());
                $this->assertEquals($ownerPath, $share->getNode());
@@ -752,7 +753,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
                $qb->insert('share')
                        ->values([
                                'share_type'    => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_LINK),
-                               'share_with'    => $qb->expr()->literal('password'),
+                               'password'    => $qb->expr()->literal('password'),
                                'uid_owner'     => $qb->expr()->literal('shareOwner'),
                                'uid_initiator' => $qb->expr()->literal('sharedBy'),
                                'item_type'     => $qb->expr()->literal('file'),
@@ -814,7 +815,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
                        ['home::shareOwner', 'files/test.txt', 'files/test2.txt'],
                        // regular file on external storage
                        ['smb::whatever', 'files/test.txt', 'files/test2.txt'],
-                       // regular file on external storage in trashbin-like folder, 
+                       // regular file on external storage in trashbin-like folder,
                        ['smb::whatever', 'files_trashbin/files/test.txt', 'files_trashbin/files/test2.txt'],
                ];
        }
@@ -2353,9 +2354,11 @@ class DefaultShareProviderTest extends \Test\TestCase {
                        $rootFolder
                );
 
-               $u1 = $userManager->createUser('testShare1', 'test');
-               $u2 = $userManager->createUser('testShare2', 'test');
-               $u3 = $userManager->createUser('testShare3', 'test');
+               $password = md5(time());
+
+               $u1 = $userManager->createUser('testShare1', $password);
+               $u2 = $userManager->createUser('testShare2', $password);
+               $u3 = $userManager->createUser('testShare3', $password);
 
                $g1 = $groupManager->createGroup('group1');