From b52154b306f13327a593fdc1231fcd454720e0a6 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 1 Aug 2014 16:24:19 +0200 Subject: [PATCH] unit tests for grouping of shares pointing to the same source --- apps/files_sharing/tests/base.php | 9 +++ apps/files_sharing/tests/share.php | 100 +++++++++++++++++++---------- lib/private/share/share.php | 2 +- tests/lib/share/share.php | 92 ++++++++++++++++++++++---- 4 files changed, 155 insertions(+), 48 deletions(-) diff --git a/apps/files_sharing/tests/base.php b/apps/files_sharing/tests/base.php index 34ec4a36ede..60159d84e7e 100644 --- a/apps/files_sharing/tests/base.php +++ b/apps/files_sharing/tests/base.php @@ -35,6 +35,8 @@ abstract class Test_Files_Sharing_Base extends \PHPUnit_Framework_TestCase { const TEST_FILES_SHARING_API_USER2 = "test-share-user2"; const TEST_FILES_SHARING_API_USER3 = "test-share-user3"; + const TEST_FILES_SHARING_API_GROUP1 = "test-share-group1"; + public $stateFilesEncryption; public $filename; public $data; @@ -60,6 +62,10 @@ abstract class Test_Files_Sharing_Base extends \PHPUnit_Framework_TestCase { self::loginHelper(self::TEST_FILES_SHARING_API_USER2, true); self::loginHelper(self::TEST_FILES_SHARING_API_USER3, true); + // create group + \OC_Group::createGroup(self::TEST_FILES_SHARING_API_GROUP1); + \OC_Group::addToGroup(self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_GROUP1); + } function setUp() { @@ -94,6 +100,9 @@ abstract class Test_Files_Sharing_Base extends \PHPUnit_Framework_TestCase { \OC_User::deleteUser(self::TEST_FILES_SHARING_API_USER1); \OC_User::deleteUser(self::TEST_FILES_SHARING_API_USER2); \OC_User::deleteUser(self::TEST_FILES_SHARING_API_USER3); + + // delete group + \OC_Group::deleteGroup(self::TEST_FILES_SHARING_API_GROUP1); } /** diff --git a/apps/files_sharing/tests/share.php b/apps/files_sharing/tests/share.php index 31246c5df44..c780a9e816a 100644 --- a/apps/files_sharing/tests/share.php +++ b/apps/files_sharing/tests/share.php @@ -57,6 +57,10 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { self::$tempStorage = null; + // clear database table + $query = \OCP\DB::prepare('DELETE FROM `*PREFIX*share`'); + $query->execute(); + parent::tearDown(); } @@ -70,8 +74,6 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { $pathinfo = pathinfo($this->filename); - $duplicate = '/' . $pathinfo['filename'] . ' (2).' . $pathinfo['extension']; - $result = \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, \Test_Files_Sharing::TEST_FILES_SHARING_API_USER2, 31); @@ -84,46 +86,85 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { self::loginHelper(self::TEST_FILES_SHARING_API_USER2); $this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertTrue(\OC\Files\Filesystem::file_exists($duplicate)); self::loginHelper(self::TEST_FILES_SHARING_API_USER3); $this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertFalse(\OC\Files\Filesystem::file_exists($duplicate)); self::loginHelper(self::TEST_FILES_SHARING_API_USER2); \OC\Files\Filesystem::unlink($this->filename); self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + // both group share and user share should be gone $this->assertFalse(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertTrue(\OC\Files\Filesystem::file_exists($duplicate)); // for user3 nothing should change self::loginHelper(self::TEST_FILES_SHARING_API_USER3); $this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertFalse(\OC\Files\Filesystem::file_exists($duplicate)); + } + + /** + * if a file was shared as group share and as individual share they should be grouped + */ + function testGroupingOfShares() { + + $fileinfo = $this->view->getFileInfo($this->filename); + + $result = \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, + \Test_Files_Sharing::TEST_FILES_SHARING_API_GROUP1, \OCP\PERMISSION_READ); + + $this->assertTrue($result); + + $result = \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing::TEST_FILES_SHARING_API_USER2, \OCP\PERMISSION_UPDATE); + + $this->assertTrue($result); self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - \OC\Files\Filesystem::unlink($duplicate); - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - $this->assertFalse(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertFalse(\OC\Files\Filesystem::file_exists($duplicate)); - // for user3 nothing should change - self::loginHelper(self::TEST_FILES_SHARING_API_USER3); - $this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertFalse(\OC\Files\Filesystem::file_exists($duplicate)); + $result = \OCP\Share::getItemSharedWith('file', null); + + $this->assertTrue(is_array($result)); + + // test should return exactly one shares created from testCreateShare() + $this->assertSame(1, count($result)); + + $share = reset($result); + $this->assertSame(\OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE, $share['permissions']); + + \OC\Files\Filesystem::rename($this->filename, $this->filename . '-renamed'); + + $result = \OCP\Share::getItemSharedWith('file', null); + + $this->assertTrue(is_array($result)); + + // test should return exactly one shares created from testCreateShare() + $this->assertSame(1, count($result)); + + $share = reset($result); + $this->assertSame(\OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE, $share['permissions']); + $this->assertSame($this->filename . '-renamed', $share['file_target']); - //cleanup self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, - 'testGroup'); - \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, - self::TEST_FILES_SHARING_API_USER2); - \OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER2, 'testGroup'); - \OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER2, 'testGroup'); - \OC_Group::deleteGroup('testGroup'); + // unshare user share + $result = \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing::TEST_FILES_SHARING_API_USER2); + $this->assertTrue($result); - } + self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + + $result = \OCP\Share::getItemSharedWith('file', null); + + $this->assertTrue(is_array($result)); + + // test should return the remaining group share + $this->assertSame(1, count($result)); + + $share = reset($result); + // only the group share permissions should be available now + $this->assertSame(\OCP\PERMISSION_READ, $share['permissions']); + $this->assertSame($this->filename . '-renamed', $share['file_target']); + + } function testShareWithDifferentShareFolder() { @@ -146,12 +187,6 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { $this->assertTrue(\OC\Files\Filesystem::file_exists('/Shared/subfolder/' . $this->folder)); //cleanup - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, - self::TEST_FILES_SHARING_API_USER2); - \OCP\Share::unshare('folder', $folderinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, - self::TEST_FILES_SHARING_API_USER2); - \OCP\Config::deleteSystemValue('share_folder'); } @@ -173,18 +208,14 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { $this->assertTrue(is_array($result)); // test should return exactly one shares created from testCreateShare() - $this->assertTrue(count($result) === 1); + $this->assertSame(1, count($result), 'more then one share found'); $share = reset($result); $this->assertSame($expectedPermissions, $share['permissions']); - - \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, - \Test_Files_Sharing::TEST_FILES_SHARING_API_USER2); } function DataProviderTestFileSharePermissions() { $permission1 = \OCP\PERMISSION_ALL; - $permission2 = \OCP\PERMISSION_DELETE; $permission3 = \OCP\PERMISSION_READ; $permission4 = \OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE; $permission5 = \OCP\PERMISSION_READ | \OCP\PERMISSION_DELETE; @@ -192,7 +223,6 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { return array( array($permission1, \OCP\PERMISSION_ALL & ~\OCP\PERMISSION_DELETE), - array($permission2, 0), array($permission3, $permission3), array($permission4, $permission4), array($permission5, $permission3), diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 2864e8532aa..44051409504 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -1512,7 +1512,7 @@ class Share extends \OC\Share\Constants { * @param string $itemType * @return array of grouped items */ - private static function groupItems($items, $itemType) { + protected static function groupItems($items, $itemType) { $fileSharing = ($itemType === 'file' || $itemType === 'folder') ? true : false; diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php index bb827eece73..a0b3643837e 100644 --- a/tests/lib/share/share.php +++ b/tests/lib/share/share.php @@ -522,7 +522,7 @@ class Test_Share extends PHPUnit_Framework_TestCase { OC_User::setUserId($this->user2); $this->assertEquals(array(OCP\PERMISSION_READ | OCP\PERMISSION_UPDATE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS)); OC_User::setUserId($this->user4); - $this->assertEquals(array(), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); + $this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); // Valid share with same person - group then user OC_User::setUserId($this->user1); @@ -733,20 +733,88 @@ class Test_Share extends PHPUnit_Framework_TestCase { array(false, array('share_with' => '1234567890', 'share_type' => '3', 'id' => 101)), array(false, array('share_with' => '1234567890', 'share_type' => 3, 'id' => 101)), ); + } - /* - if (!isset($linkItem['share_with'])) { - return true; - } + /** + * @dataProvider dataProviderTestGroupItems + * @param type $ungrouped + * @param type $grouped + */ + function testGroupItems($ungrouped, $grouped) { - if ($linkItem['share_type'] != \OCP\Share::SHARE_TYPE_LINK) { - return true; - } + $result = DummyShareClass::groupItemsTest($ungrouped); - if ( \OC::$session->exists('public_link_authenticated') - && \OC::$session->get('public_link_authenticated') === $linkItem['id'] ) { - return true; + $this->compareArrays($grouped, $result); + + } + + function compareArrays($result, $expectedResult) { + foreach ($expectedResult as $key => $value) { + if (is_array($value)) { + $this->compareArrays($result[$key], $value); + } else { + $this->assertSame($value, $result[$key]); + } } - * */ + } + + function dataProviderTestGroupItems() { + return array( + // one array with one share + array( + array( // input + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_ALL, 'item_target' => 't1')), + array( // expected result + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_ALL, 'item_target' => 't1'))), + // two shares both point to the same source + array( + array( // input + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_READ, 'item_target' => 't1'), + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_UPDATE, 'item_target' => 't1'), + ), + array( // expected result + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE, 'item_target' => 't1', + 'grouped' => array( + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_READ, 'item_target' => 't1'), + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_UPDATE, 'item_target' => 't1'), + ) + ), + ) + ), + // two shares both point to the same source but with different targets + array( + array( // input + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_READ, 'item_target' => 't1'), + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_UPDATE, 'item_target' => 't2'), + ), + array( // expected result + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_READ, 'item_target' => 't1'), + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_UPDATE, 'item_target' => 't2'), + ) + ), + // three shares two point to the same source + array( + array( // input + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_READ, 'item_target' => 't1'), + array('item_source' => 2, 'permissions' => \OCP\PERMISSION_CREATE, 'item_target' => 't2'), + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_UPDATE, 'item_target' => 't1'), + ), + array( // expected result + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE, 'item_target' => 't1', + 'grouped' => array( + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_READ, 'item_target' => 't1'), + array('item_source' => 1, 'permissions' => \OCP\PERMISSION_UPDATE, 'item_target' => 't1'), + ) + ), + array('item_source' => 2, 'permissions' => \OCP\PERMISSION_CREATE, 'item_target' => 't2'), + ) + ), + ); + } +} + +class DummyShareClass extends \OC\Share\Share { + public static function groupItemsTest($items) { + return parent::groupItems($items, 'test'); } } -- 2.39.5