diff --git a/apps/files_sharing/lib/mountprovider.php b/apps/files_sharing/lib/mountprovider.php
index b386c6704f9b..b99cafce4c1a 100644
--- a/apps/files_sharing/lib/mountprovider.php
+++ b/apps/files_sharing/lib/mountprovider.php
@@ -50,6 +50,15 @@ public function __construct(IConfig $config, ILogger $logger) {
$this->logger = $logger;
}
+ /**
+ * Return items shared with user
+ *
+ * @internal
+ */
+ public function getItemsSharedWithUser($uid) {
+ // only here to make it mockable/testable
+ return \OCP\Share::getItemsSharedWithUser('file', $uid, \OCP\Share::FORMAT_NONE, null, -1, false, true);
+ }
/**
* Get all mountpoints applicable for the user and check for shares where we need to update the etags
@@ -59,7 +68,7 @@ public function __construct(IConfig $config, ILogger $logger) {
* @return \OCP\Files\Mount\IMountPoint[]
*/
public function getMountsForUser(IUser $user, IStorageFactory $storageFactory) {
- $shares = \OCP\Share::getItemsSharedWithUser('file', $user->getUID());
+ $shares = $this->getItemsSharedWithUser($user->getUID());
$shares = array_filter($shares, function ($share) {
return $share['permissions'] > 0;
});
diff --git a/apps/files_sharing/tests/mountprovider.php b/apps/files_sharing/tests/mountprovider.php
new file mode 100644
index 000000000000..72265552ae07
--- /dev/null
+++ b/apps/files_sharing/tests/mountprovider.php
@@ -0,0 +1,146 @@
+
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program. If not, see
+ *
+ */
+
+namespace OCA\Files_Sharing\Tests;
+
+use OCA\Files_Sharing\MountProvider;
+use OCP\Files\Storage\IStorageFactory;
+use OCP\IConfig;
+use OCP\ILogger;
+use OCP\IUser;
+use OCP\Share\IShare;
+use OCP\Share\IManager;
+use OCP\Files\Mount\IMountPoint;
+
+/**
+ * @group DB
+ */
+class MountProviderTest extends \Test\TestCase {
+
+ /** @var MountProvider */
+ private $provider;
+
+ /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
+ private $config;
+
+ /** @var IUser|\PHPUnit_Framework_MockObject_MockObject */
+ private $user;
+
+ /** @var IStorageFactory|\PHPUnit_Framework_MockObject_MockObject */
+ private $loader;
+
+ /** @var ILogger | \PHPUnit_Framework_MockObject_MockObject */
+ private $logger;
+
+ public function setUp() {
+ parent::setUp();
+
+ $this->config = $this->getMock('OCP\IConfig');
+ $this->user = $this->getMock('OCP\IUser');
+ $this->loader = $this->getMock('OCP\Files\Storage\IStorageFactory');
+ $this->loader->expects($this->any())
+ ->method('getInstance')
+ ->will($this->returnCallback(function($mountPoint, $class, $arguments) {
+ $storage = $this->getMockBuilder('OC\Files\Storage\Shared')
+ ->disableOriginalConstructor()
+ ->getMock();
+ $storage->expects($this->any())
+ ->method('getShare')
+ ->will($this->returnValue($arguments['share']));
+ return $storage;
+ }));
+ $this->logger = $this->getMock('\OCP\ILogger');
+ $this->logger->expects($this->never())
+ ->method('error');
+
+ $this->provider = $this->getMockBuilder('OCA\Files_Sharing\MountProvider')
+ ->setMethods(['getItemsSharedWithUser'])
+ ->setConstructorArgs([$this->config, $this->logger])
+ ->getMock();
+ }
+
+ private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $shareType) {
+ return [
+ 'id' => $id,
+ 'uid_owner' => $owner,
+ 'share_type' => $shareType,
+ 'item_type' => 'file',
+ 'file_target' => $target,
+ 'file_source' => $nodeId,
+ 'item_target' => null,
+ 'item_source' => $nodeId,
+ 'permissions' => $permissions,
+ 'stime' => time(),
+ 'token' => null,
+ 'expiration' => null,
+ ];
+ }
+
+ /**
+ * Tests excluding shares from the current view. This includes:
+ * - shares that were opted out of (permissions === 0)
+ * - shares with a group in which the owner is already in
+ */
+ public function testExcludeShares() {
+ $userShares = [
+ $this->makeMockShare(1, 100, 'user2', '/share2', 0, \OCP\Share::SHARE_TYPE_USER),
+ $this->makeMockShare(2, 100, 'user2', '/share2', 31, \OCP\Share::SHARE_TYPE_USER),
+ ];
+
+ $groupShares = [
+ $this->makeMockShare(3, 100, 'user2', '/share2', 0, \OCP\Share::SHARE_TYPE_GROUP),
+ $this->makeMockShare(4, 100, 'user2', '/share4', 31, \OCP\Share::SHARE_TYPE_GROUP),
+ ];
+
+ $this->user->expects($this->any())
+ ->method('getUID')
+ ->will($this->returnValue('user1'));
+
+ $allShares = array_merge($userShares, $groupShares);
+
+ $this->provider->expects($this->once())
+ ->method('getItemsSharedWithUser')
+ ->with('user1')
+ ->will($this->returnValue($allShares));
+
+ $mounts = $this->provider->getMountsForUser($this->user, $this->loader);
+
+ $this->assertCount(2, $mounts);
+ $this->assertInstanceOf('OCA\Files_Sharing\SharedMount', $mounts[0]);
+ $this->assertInstanceOf('OCA\Files_Sharing\SharedMount', $mounts[1]);
+
+ $mountedShare1 = $mounts[0]->getShare();
+
+ $this->assertEquals('2', $mountedShare1['id']);
+ $this->assertEquals('user2', $mountedShare1['uid_owner']);
+ $this->assertEquals(100, $mountedShare1['file_source']);
+ $this->assertEquals('/share2', $mountedShare1['file_target']);
+ $this->assertEquals(31, $mountedShare1['permissions']);
+
+ $mountedShare2 = $mounts[1]->getShare();
+ $this->assertEquals('4', $mountedShare2['id']);
+ $this->assertEquals('user2', $mountedShare2['uid_owner']);
+ $this->assertEquals(100, $mountedShare2['file_source']);
+ $this->assertEquals('/share4', $mountedShare2['file_target']);
+ $this->assertEquals(31, $mountedShare2['permissions']);
+ }
+}
+
diff --git a/build/integration/features/sharing-v1.feature b/build/integration/features/sharing-v1.feature
index da6ab1d71d36..40eb73cb1070 100644
--- a/build/integration/features/sharing-v1.feature
+++ b/build/integration/features/sharing-v1.feature
@@ -608,3 +608,146 @@ Feature: sharing
| /foo/ |
| /foo%20(2)/ |
+ Scenario: Merging shares for recipient when shared from outside with group and member
+ Given As an "admin"
+ And user "user0" exists
+ And user "user1" exists
+ And group "group1" exists
+ And user "user1" belongs to group "group1"
+ And user "user0" created a folder "merge-test-outside"
+ When folder "merge-test-outside" of user "user0" is shared with group "group1"
+ And folder "merge-test-outside" of user "user0" is shared with user "user1"
+ Then as "user1" the folder "merge-test-outside" exists
+ And as "user1" the folder "merge-test-outside (2)" does not exist
+
+ Scenario: Merging shares for recipient when shared from outside with group and member with different permissions
+ Given As an "admin"
+ And user "user0" exists
+ And user "user1" exists
+ And group "group1" exists
+ And user "user1" belongs to group "group1"
+ And user "user0" created a folder "merge-test-outside-perms"
+ When folder "merge-test-outside-perms" of user "user0" is shared with group "group1" with permissions 1
+ And folder "merge-test-outside-perms" of user "user0" is shared with user "user1" with permissions 31
+ Then as "user1" gets properties of folder "merge-test-outside-perms" with
+ |{http://owncloud.org/ns}permissions|
+ And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+ And as "user1" the folder "merge-test-outside-perms (2)" does not exist
+
+ Scenario: Merging shares for recipient when shared from outside with two groups
+ Given As an "admin"
+ And user "user0" exists
+ And user "user1" exists
+ And group "group1" exists
+ And group "group2" exists
+ And user "user1" belongs to group "group1"
+ And user "user1" belongs to group "group2"
+ And user "user0" created a folder "merge-test-outside-twogroups"
+ When folder "merge-test-outside-twogroups" of user "user0" is shared with group "group1"
+ And folder "merge-test-outside-twogroups" of user "user0" is shared with group "group2"
+ Then as "user1" the folder "merge-test-outside-twogroups" exists
+ And as "user1" the folder "merge-test-outside-twogroups (2)" does not exist
+
+ Scenario: Merging shares for recipient when shared from outside with two groups with different permissions
+ Given As an "admin"
+ And user "user0" exists
+ And user "user1" exists
+ And group "group1" exists
+ And group "group2" exists
+ And user "user1" belongs to group "group1"
+ And user "user1" belongs to group "group2"
+ And user "user0" created a folder "merge-test-outside-twogroups-perms"
+ When folder "merge-test-outside-twogroups-perms" of user "user0" is shared with group "group1" with permissions 1
+ And folder "merge-test-outside-twogroups-perms" of user "user0" is shared with group "group2" with permissions 31
+ Then as "user1" gets properties of folder "merge-test-outside-twogroups-perms" with
+ |{http://owncloud.org/ns}permissions|
+ And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+ And as "user1" the folder "merge-test-outside-twogroups-perms (2)" does not exist
+
+ Scenario: Merging shares for recipient when shared from outside with two groups and member
+ Given As an "admin"
+ And user "user0" exists
+ And user "user1" exists
+ And group "group1" exists
+ And group "group2" exists
+ And user "user1" belongs to group "group1"
+ And user "user1" belongs to group "group2"
+ And user "user0" created a folder "merge-test-outside-twogroups-member-perms"
+ When folder "merge-test-outside-twogroups-member-perms" of user "user0" is shared with group "group1" with permissions 1
+ And folder "merge-test-outside-twogroups-member-perms" of user "user0" is shared with group "group2" with permissions 31
+ And folder "merge-test-outside-twogroups-member-perms" of user "user0" is shared with user "user1" with permissions 1
+ Then as "user1" gets properties of folder "merge-test-outside-twogroups-member-perms" with
+ |{http://owncloud.org/ns}permissions|
+ And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+ And as "user1" the folder "merge-test-outside-twogroups-member-perms (2)" does not exist
+
+ Scenario: Merging shares for recipient when shared from inside with group
+ Given As an "admin"
+ And user "user0" exists
+ And group "group1" exists
+ And user "user0" belongs to group "group1"
+ And user "user0" created a folder "merge-test-inside-group"
+ When folder "/merge-test-inside-group" of user "user0" is shared with group "group1"
+ Then as "user0" the folder "merge-test-inside-group" exists
+ And as "user0" the folder "merge-test-inside-group (2)" does not exist
+
+ Scenario: Merging shares for recipient when shared from inside with two groups
+ Given As an "admin"
+ And user "user0" exists
+ And group "group1" exists
+ And group "group2" exists
+ And user "user0" belongs to group "group1"
+ And user "user0" belongs to group "group2"
+ And user "user0" created a folder "merge-test-inside-twogroups"
+ When folder "merge-test-inside-twogroups" of user "user0" is shared with group "group1"
+ And folder "merge-test-inside-twogroups" of user "user0" is shared with group "group2"
+ Then as "user0" the folder "merge-test-inside-twogroups" exists
+ And as "user0" the folder "merge-test-inside-twogroups (2)" does not exist
+ And as "user0" the folder "merge-test-inside-twogroups (3)" does not exist
+
+ Scenario: Merging shares for recipient when shared from inside with group with less permissions
+ Given As an "admin"
+ And user "user0" exists
+ And group "group1" exists
+ And group "group2" exists
+ And user "user0" belongs to group "group1"
+ And user "user0" belongs to group "group2"
+ And user "user0" created a folder "merge-test-inside-twogroups-perms"
+ When folder "merge-test-inside-twogroups-perms" of user "user0" is shared with group "group1"
+ And folder "merge-test-inside-twogroups-perms" of user "user0" is shared with group "group2"
+ Then as "user0" gets properties of folder "merge-test-inside-twogroups-perms" with
+ |{http://owncloud.org/ns}permissions|
+ And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "RDNVCK"
+ And as "user0" the folder "merge-test-inside-twogroups-perms (2)" does not exist
+ And as "user0" the folder "merge-test-inside-twogroups-perms (3)" does not exist
+
+ Scenario: Merging shares for recipient when shared from outside with group then user and recipient renames in between
+ Given As an "admin"
+ And user "user0" exists
+ And user "user1" exists
+ And group "group1" exists
+ And user "user1" belongs to group "group1"
+ And user "user0" created a folder "merge-test-outside-groups-renamebeforesecondshare"
+ When folder "merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with group "group1"
+ And User "user1" moved folder "/merge-test-outside-groups-renamebeforesecondshare" to "/merge-test-outside-groups-renamebeforesecondshare-renamed"
+ And folder "merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with user "user1"
+ Then as "user1" gets properties of folder "merge-test-outside-groups-renamebeforesecondshare-renamed" with
+ |{http://owncloud.org/ns}permissions|
+ And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+ And as "user1" the folder "merge-test-outside-groups-renamebeforesecondshare" does not exist
+
+ Scenario: Merging shares for recipient when shared from outside with user then group and recipient renames in between
+ Given As an "admin"
+ And user "user0" exists
+ And user "user1" exists
+ And group "group1" exists
+ And user "user1" belongs to group "group1"
+ And user "user0" created a folder "merge-test-outside-groups-renamebeforesecondshare"
+ When folder "merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with user "user1"
+ And User "user1" moved folder "/merge-test-outside-groups-renamebeforesecondshare" to "/merge-test-outside-groups-renamebeforesecondshare-renamed"
+ And folder "merge-test-outside-groups-renamebeforesecondshare" of user "user0" is shared with group "group1"
+ Then as "user1" gets properties of folder "merge-test-outside-groups-renamebeforesecondshare-renamed" with
+ |{http://owncloud.org/ns}permissions|
+ And the single response should contain a property "{http://owncloud.org/ns}permissions" with value "SRDNVCK"
+ And as "user1" the folder "merge-test-outside-groups-renamebeforesecondshare" does not exist
+
diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js
index 292230d26d55..87574a02d013 100644
--- a/core/js/shareitemmodel.js
+++ b/core/js/shareitemmodel.js
@@ -601,6 +601,33 @@
}
},
+ /**
+ * Group reshares into a single super share element.
+ * Does this by finding the most precise share and
+ * combines the permissions to be the most permissive.
+ *
+ * @param {Array} reshares
+ * @return {Object} reshare
+ */
+ _groupReshares: function(reshares) {
+ if (!reshares || !reshares.length) {
+ return false;
+ }
+
+ var superShare = reshares.shift();
+ var combinedPermissions = superShare.permissions;
+ _.each(reshares, function(reshare) {
+ // use share have higher priority than group share
+ if (reshare.share_type === OC.Share.SHARE_TYPE_USER && superShare.share_type === OC.Share.SHARE_TYPE_GROUP) {
+ superShare = reshare;
+ }
+ combinedPermissions |= reshare.permissions;
+ });
+
+ superShare.permissions = combinedPermissions;
+ return superShare;
+ },
+
fetch: function() {
var model = this;
this.trigger('request', this);
@@ -618,7 +645,7 @@
var reshare = false;
if (data2[0].ocs.data.length) {
- reshare = data2[0].ocs.data[0];
+ reshare = model._groupReshares(data2[0].ocs.data);
}
model.set(model.parse({
diff --git a/core/js/tests/specs/shareitemmodelSpec.js b/core/js/tests/specs/shareitemmodelSpec.js
index 8c9560d2646d..9d9001dc9e8f 100644
--- a/core/js/tests/specs/shareitemmodelSpec.js
+++ b/core/js/tests/specs/shareitemmodelSpec.js
@@ -181,6 +181,48 @@ describe('OC.Share.ShareItemModel', function() {
// TODO: check more attributes
});
+ it('groups reshare info into a single item', function() {
+ /* jshint camelcase: false */
+ fetchReshareDeferred.resolve(makeOcsResponse([
+ {
+ id: '1',
+ share_type: OC.Share.SHARE_TYPE_USER,
+ uid_owner: 'owner',
+ displayname_owner: 'Owner',
+ share_with: 'root',
+ permissions: 1
+ },
+ {
+ id: '2',
+ share_type: OC.Share.SHARE_TYPE_GROUP,
+ uid_owner: 'owner',
+ displayname_owner: 'Owner',
+ share_with: 'group1',
+ permissions: 15
+ },
+ {
+ id: '3',
+ share_type: OC.Share.SHARE_TYPE_GROUP,
+ uid_owner: 'owner',
+ displayname_owner: 'Owner',
+ share_with: 'group1',
+ permissions: 17
+ }
+ ]));
+ fetchSharesDeferred.resolve(makeOcsResponse([]));
+
+ OC.currentUser = 'root';
+
+ model.fetch();
+
+ var reshare = model.get('reshare');
+ // max permissions
+ expect(reshare.permissions).toEqual(31);
+ // user share has higher priority
+ expect(reshare.share_type).toEqual(OC.Share.SHARE_TYPE_USER);
+ expect(reshare.share_with).toEqual('root');
+ expect(reshare.id).toEqual('1');
+ });
it('does not parse link share when for a different file', function() {
/* jshint camelcase: false */
fetchReshareDeferred.resolve(makeOcsResponse([]));
diff --git a/lib/private/repair.php b/lib/private/repair.php
index 0cbb43293e8c..0fa6f15a8aa0 100644
--- a/lib/private/repair.php
+++ b/lib/private/repair.php
@@ -49,6 +49,7 @@
use OC\Repair\SearchLuceneTables;
use OC\Repair\UpdateOutdatedOcsIds;
use OC\Repair\RepairInvalidShares;
+use OC\Repair\RepairUnmergedShares;
class Repair extends BasicEmitter {
/**
@@ -118,6 +119,12 @@ public static function getRepairSteps() {
new RemoveGetETagEntries(\OC::$server->getDatabaseConnection()),
new UpdateOutdatedOcsIds(\OC::$server->getConfig()),
new RepairInvalidShares(\OC::$server->getConfig(), \OC::$server->getDatabaseConnection()),
+ new RepairUnmergedShares(
+ \OC::$server->getConfig(),
+ \OC::$server->getDatabaseConnection(),
+ \OC::$server->getUserManager(),
+ \OC::$server->getGroupManager()
+ ),
new AvatarPermissions(\OC::$server->getDatabaseConnection()),
new BrokenUpdaterRepair(),
];
diff --git a/lib/private/repair/repairunmergedshares.php b/lib/private/repair/repairunmergedshares.php
new file mode 100644
index 000000000000..b9ba5cf0ee52
--- /dev/null
+++ b/lib/private/repair/repairunmergedshares.php
@@ -0,0 +1,379 @@
+
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program. If not, see
+ *
+ */
+
+namespace OC\Repair;
+
+use OCP\Migration\IOutput;
+use OC\Hooks\BasicEmitter;
+use OC\Share\Constants;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IConfig;
+use OCP\IDBConnection;
+use OCP\IUserManager;
+use OCP\IUser;
+use OCP\IGroupManager;
+use OC\Share20\DefaultShareProvider;
+
+/**
+ * Repairs shares for which the received folder was not properly deduplicated.
+ *
+ * An unmerged share can for example happen when sharing a folder with the same
+ * user through multiple ways, like several groups and also directly, additionally
+ * to group shares. Since 9.0.0 these would create duplicate entries "folder (2)",
+ * one for every share. This repair step rearranges them so they only appear as a single
+ * folder.
+ */
+class RepairUnmergedShares extends BasicEmitter implements \OC\RepairStep {
+
+ /** @var \OCP\IConfig */
+ protected $config;
+
+ /** @var \OCP\IDBConnection */
+ protected $connection;
+
+ /** @var IUserManager */
+ protected $userManager;
+
+ /** @var IGroupManager */
+ protected $groupManager;
+
+ /** @var IQueryBuilder */
+ private $queryGetSharesWithUsers;
+
+ /** @var IQueryBuilder */
+ private $queryUpdateSharePermissionsAndTarget;
+
+ /** @var IQueryBuilder */
+ private $queryUpdateShareInBatch;
+
+ /**
+ * @param \OCP\IConfig $config
+ * @param \OCP\IDBConnection $connection
+ */
+ public function __construct(
+ IConfig $config,
+ IDBConnection $connection,
+ IUserManager $userManager,
+ IGroupManager $groupManager
+ ) {
+ $this->connection = $connection;
+ $this->config = $config;
+ $this->userManager = $userManager;
+ $this->groupManager = $groupManager;
+ }
+
+ public function getName() {
+ return 'Repair unmerged shares';
+ }
+
+ /**
+ * Builds prepared queries for reuse
+ */
+ private function buildPreparedQueries() {
+ /**
+ * Retrieve shares for a given user/group and share type
+ */
+ $query = $this->connection->getQueryBuilder();
+ $query
+ ->select('item_source', 'id', 'file_target', 'permissions', 'parent', 'share_type', 'stime')
+ ->from('share')
+ ->where($query->expr()->eq('share_type', $query->createParameter('shareType')))
+ ->andWhere($query->expr()->in('share_with', $query->createParameter('shareWiths')))
+ ->andWhere($query->expr()->in('item_type', $query->createParameter('itemTypes')))
+ ->orderBy('item_source', 'ASC')
+ ->addOrderBy('stime', 'ASC');
+
+ $this->queryGetSharesWithUsers = $query;
+
+ /**
+ * Updates the file_target to the given value for all given share ids.
+ *
+ * This updates several shares in bulk which is faster than individually.
+ */
+ $query = $this->connection->getQueryBuilder();
+ $query->update('share')
+ ->set('file_target', $query->createParameter('file_target'))
+ ->where($query->expr()->in('id', $query->createParameter('ids')));
+
+ $this->queryUpdateShareInBatch = $query;
+
+ /**
+ * Updates the share permissions and target path of a single share.
+ */
+ $query = $this->connection->getQueryBuilder();
+ $query->update('share')
+ ->set('permissions', $query->createParameter('permissions'))
+ ->set('file_target', $query->createParameter('file_target'))
+ ->where($query->expr()->eq('id', $query->createParameter('shareid')));
+
+ $this->queryUpdateSharePermissionsAndTarget = $query;
+
+ }
+
+ private function getSharesWithUser($shareType, $shareWiths) {
+ $groupedShares = [];
+
+ $query = $this->queryGetSharesWithUsers;
+ $query->setParameter('shareWiths', $shareWiths, IQueryBuilder::PARAM_STR_ARRAY);
+ $query->setParameter('shareType', $shareType);
+ $query->setParameter('itemTypes', ['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY);
+
+ $shares = $query->execute()->fetchAll();
+
+ // group by item_source
+ foreach ($shares as $share) {
+ if (!isset($groupedShares[$share['item_source']])) {
+ $groupedShares[$share['item_source']] = [];
+ }
+ $groupedShares[$share['item_source']][] = $share;
+ }
+ return $groupedShares;
+ }
+
+ /**
+ * Decide on the best target name based on all group shares and subshares,
+ * goal is to increase the likeliness that the chosen name matches what
+ * the user is expecting.
+ *
+ * For this, we discard the entries with parenthesis "(2)".
+ * In case the user also renamed the duplicates to a legitimate name, this logic
+ * will still pick the most recent one as it's the one the user is most likely to
+ * remember renaming.
+ *
+ * If no suitable subshare is found, use the least recent group share instead.
+ *
+ * @param array $groupShares group share entries
+ * @param array $subShares sub share entries
+ *
+ * @return string chosen target name
+ */
+ private function findBestTargetName($groupShares, $subShares) {
+ $pickedShare = null;
+ // sort by stime, this also properly sorts the direct user share if any
+ @usort($subShares, function($a, $b) {
+ if ($a['stime'] < $b['stime']) {
+ return -1;
+ } else if ($a['stime'] > $b['stime']) {
+ return 1;
+ }
+
+ return 0;
+ });
+
+ foreach ($subShares as $subShare) {
+ // skip entries that have parenthesis with numbers
+ if (preg_match('/\([0-9]*\)/', $subShare['file_target']) === 1) {
+ continue;
+ }
+ // pick any share found that would match, the last being the most recent
+ $pickedShare = $subShare;
+ }
+
+ // no suitable subshare found
+ if ($pickedShare === null) {
+ // use least recent group share target instead
+ $pickedShare = $groupShares[0];
+ }
+
+ return $pickedShare['file_target'];
+ }
+
+ /**
+ * Fix the given received share represented by the set of group shares
+ * and matching sub shares
+ *
+ * @param array $groupShares group share entries
+ * @param array $subShares sub share entries
+ *
+ * @return boolean false if the share was not repaired, true if it was
+ */
+ private function fixThisShare($groupShares, $subShares) {
+ if (empty($subShares)) {
+ return false;
+ }
+
+ $groupSharesById = [];
+ foreach ($groupShares as $groupShare) {
+ $groupSharesById[$groupShare['id']] = $groupShare;
+ }
+
+ if ($this->isThisShareValid($groupSharesById, $subShares)) {
+ return false;
+ }
+
+ $targetPath = $this->findBestTargetName($groupShares, $subShares);
+
+ // check whether the user opted out completely of all subshares
+ $optedOut = true;
+ foreach ($subShares as $subShare) {
+ if ((int)$subShare['permissions'] !== 0) {
+ $optedOut = false;
+ break;
+ }
+ }
+
+ $shareIds = [];
+ foreach ($subShares as $subShare) {
+ // only if the user deleted some subshares but not all, adjust the permissions of that subshare
+ if (!$optedOut && (int)$subShare['permissions'] === 0 && (int)$subShare['share_type'] === DefaultShareProvider::SHARE_TYPE_USERGROUP) {
+ // set permissions from parent group share
+ $permissions = $groupSharesById[$subShare['parent']]['permissions'];
+
+ // fix permissions and target directly
+ $query = $this->queryUpdateSharePermissionsAndTarget;
+ $query->setParameter('shareid', $subShare['id']);
+ $query->setParameter('file_target', $targetPath);
+ $query->setParameter('permissions', $permissions);
+ $query->execute();
+ } else {
+ // gather share ids for bulk target update
+ if ($subShare['file_target'] !== $targetPath) {
+ $shareIds[] = (int)$subShare['id'];
+ }
+ }
+ }
+
+ if (!empty($shareIds)) {
+ $query = $this->queryUpdateShareInBatch;
+ $query->setParameter('ids', $shareIds, IQueryBuilder::PARAM_INT_ARRAY);
+ $query->setParameter('file_target', $targetPath);
+ $query->execute();
+ }
+
+ return true;
+ }
+
+ /**
+ * Checks whether the number of group shares is balanced with the child subshares.
+ * If all group shares have exactly one subshare, and the target of every subshare
+ * is the same, then the share is valid.
+ * If however there is a group share entry that has no matching subshare, it means
+ * we're in the bogus situation and the whole share must be repaired
+ *
+ * @param array $groupSharesById
+ * @param array $subShares
+ *
+ * @return true if the share is valid, false if it needs repair
+ */
+ private function isThisShareValid($groupSharesById, $subShares) {
+ $foundTargets = [];
+
+ // every group share needs to have exactly one matching subshare
+ foreach ($subShares as $subShare) {
+ $foundTargets[$subShare['file_target']] = true;
+ if (count($foundTargets) > 1) {
+ // not all the same target path value => invalid
+ return false;
+ }
+ if (isset($groupSharesById[$subShare['parent']])) {
+ // remove it from the list as we found it
+ unset($groupSharesById[$subShare['parent']]);
+ }
+ }
+
+ // if we found one subshare per group entry, the set will be empty.
+ // If not empty, it means that one of the group shares did not have
+ // a matching subshare entry.
+ return empty($groupSharesById);
+ }
+
+ /**
+ * Detect unmerged received shares and merge them properly
+ */
+ private function fixUnmergedShares(IUser $user) {
+ $groups = $this->groupManager->getUserGroupIds($user);
+ if (empty($groups)) {
+ // user is in no groups, so can't have received group shares
+ return;
+ }
+
+ // get all subshares grouped by item source
+ $subSharesByItemSource = $this->getSharesWithUser(DefaultShareProvider::SHARE_TYPE_USERGROUP, [$user->getUID()]);
+
+ // because sometimes one wants to give the user more permissions than the group share
+ $userSharesByItemSource = $this->getSharesWithUser(Constants::SHARE_TYPE_USER, [$user->getUID()]);
+
+ if (empty($subSharesByItemSource) && empty($userSharesByItemSource)) {
+ // nothing to repair for this user, no need to do extra queries
+ return;
+ }
+
+ $groupSharesByItemSource = $this->getSharesWithUser(Constants::SHARE_TYPE_GROUP, $groups);
+ if (empty($groupSharesByItemSource) && empty($userSharesByItemSource)) {
+ // nothing to repair for this user
+ return;
+ }
+
+ $fixed = 0;
+ foreach ($groupSharesByItemSource as $itemSource => $groupShares) {
+ $subShares = [];
+ if (isset($subSharesByItemSource[$itemSource])) {
+ $subShares = $subSharesByItemSource[$itemSource];
+ }
+
+ if (isset($userSharesByItemSource[$itemSource])) {
+ // add it to the subshares to get a similar treatment
+ $subShares = array_merge($subShares, $userSharesByItemSource[$itemSource]);
+ }
+
+ if ($this->fixThisShare($groupShares, $subShares)) {
+ $fixed++;
+ }
+ }
+
+ if ($fixed > 0) {
+ $this->emit('\OC\Repair', 'info', array('Fixed ' . $fixed . ' share(s) that were not merged properly'));
+ }
+ }
+
+ /**
+ * Count all the users
+ *
+ * @return int
+ */
+ private function countUsers() {
+ $allCount = $this->userManager->countUsers();
+
+ $totalCount = 0;
+ foreach ($allCount as $backend => $count) {
+ $totalCount += $count;
+ }
+
+ return $totalCount;
+ }
+
+ public function run() {
+ $ocVersionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0');
+ if (version_compare($ocVersionFromBeforeUpdate, '9.0.0.0', '>=') && version_compare($ocVersionFromBeforeUpdate, '9.0.4.2', '<')) {
+ // this situation was only possible between 9.0.0 and 9.0.4 included
+
+ $function = function(IUser $user) {
+ $this->fixUnmergedShares($user);
+ };
+
+ $this->buildPreparedQueries();
+
+ $userCount = $this->countUsers();
+
+ $this->userManager->callForAllUsers($function);
+ }
+ }
+}
diff --git a/lib/private/share/share.php b/lib/private/share/share.php
index d6bcbf902d54..3804d656dbac 100644
--- a/lib/private/share/share.php
+++ b/lib/private/share/share.php
@@ -328,12 +328,13 @@ public static function getItemsSharedWith($itemType, $format = self::FORMAT_NONE
* @param mixed $parameters (optional)
* @param int $limit Number of items to return (optional) Returns all by default
* @param boolean $includeCollections (optional)
+ * @param boolean $forceGrouping (optional) force grouping
* @return mixed Return depends on format
*/
public static function getItemsSharedWithUser($itemType, $user, $format = self::FORMAT_NONE,
- $parameters = null, $limit = -1, $includeCollections = false) {
+ $parameters = null, $limit = -1, $includeCollections = false, $forceGrouping = false) {
return self::getItems($itemType, null, self::$shareTypeUserAndGroups, $user, null, $format,
- $parameters, $limit, $includeCollections);
+ $parameters, $limit, $includeCollections, false, true, $forceGrouping);
}
/**
@@ -1643,7 +1644,8 @@ public static function getSharedItemsOwners($user, $type, $includeCollections =
* @param int $limit Number of items to return, -1 to return all matches (optional)
* @param boolean $includeCollections Include collection item types (optional)
* @param boolean $itemShareWithBySource (optional)
- * @param boolean $checkExpireDate
+ * @param boolean $checkExpireDate (optional)
+ * @param boolean $forceGrouping (optional) force grouping
* @return array
*
* See public functions getItem(s)... for parameter usage
@@ -1651,7 +1653,9 @@ public static function getSharedItemsOwners($user, $type, $includeCollections =
*/
public static function getItems($itemType, $item = null, $shareType = null, $shareWith = null,
$uidOwner = null, $format = self::FORMAT_NONE, $parameters = null, $limit = -1,
- $includeCollections = false, $itemShareWithBySource = false, $checkExpireDate = true) {
+ $includeCollections = false, $itemShareWithBySource = false, $checkExpireDate = true,
+ $forceGrouping = false
+ ) {
if (!self::isEnabled()) {
return array();
}
@@ -1928,8 +1932,9 @@ public static function getItems($itemType, $item = null, $shareType = null, $sha
}
- // group items if we are looking for items shared with the current user
- if (isset($shareWith) && $shareWith === \OCP\User::getUser()) {
+ // group items if we are looking for items shared with the current user,
+ // or forceGrouping was set because the caller does always want grouped results
+ if (isset($shareWith) && ($shareWith === \OCP\User::getUser() || $forceGrouping)) {
$items = self::groupItems($items, $itemType);
}
@@ -2063,10 +2068,13 @@ protected static function groupItems($items, $itemType) {
foreach ($items as $item) {
$grouped = false;
foreach ($result as $key => $r) {
- // for file/folder shares we need to compare file_source, otherwise we compare item_source
+ // for file/folder shares we need to compare file_source
+ // since we only want a single received target for the same file_source
+ //
+ // otherwise we compare item_source
// only group shares if they already point to the same target, otherwise the file where shared
// before grouping of shares was added. In this case we don't group them toi avoid confusions
- if (( $fileSharing && $item['file_source'] === $r['file_source'] && $item['file_target'] === $r['file_target']) ||
+ if (( $fileSharing && $item['file_source'] === $r['file_source']) ||
(!$fileSharing && $item['item_source'] === $r['item_source'] && $item['item_target'] === $r['item_target'])) {
// add the first item to the list of grouped shares
if (!isset($result[$key]['grouped'])) {
@@ -2082,7 +2090,6 @@ protected static function groupItems($items, $itemType) {
if (!$grouped) {
$result[] = $item;
}
-
}
return $result;
diff --git a/lib/public/share.php b/lib/public/share.php
index e21d82bf62c5..010e5c9bf4d5 100644
--- a/lib/public/share.php
+++ b/lib/public/share.php
@@ -114,13 +114,14 @@ public static function getItemsSharedWith($itemType, $format = self::FORMAT_NONE
* @param mixed $parameters (optional)
* @param int $limit Number of items to return (optional) Returns all by default
* @param bool $includeCollections (optional)
+ * @param bool $forceGrouping (optional) force grouping received shares
* @return mixed Return depends on format
* @since 7.0.0
*/
public static function getItemsSharedWithUser($itemType, $user, $format = self::FORMAT_NONE,
- $parameters = null, $limit = -1, $includeCollections = false) {
+ $parameters = null, $limit = -1, $includeCollections = false, $forceGrouping = false) {
- return \OC\Share\Share::getItemsSharedWithUser($itemType, $user, $format, $parameters, $limit, $includeCollections);
+ return \OC\Share\Share::getItemsSharedWithUser($itemType, $user, $format, $parameters, $limit, $includeCollections, $forceGrouping);
}
/**
diff --git a/tests/lib/repair/repairunmergedsharestest.php b/tests/lib/repair/repairunmergedsharestest.php
new file mode 100644
index 000000000000..7304bfd69208
--- /dev/null
+++ b/tests/lib/repair/repairunmergedsharestest.php
@@ -0,0 +1,540 @@
+
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program. If not, see
+ *
+ */
+
+namespace Test\Repair;
+
+
+use OC\Repair\RepairUnmergedShares;
+use OC\Share\Constants;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+use Test\TestCase;
+use OC\Share20\DefaultShareProvider;
+
+/**
+ * Tests for repairing invalid shares
+ *
+ * @group DB
+ *
+ * @see \OC\Repair\RepairUnmergedShares
+ */
+class RepairUnmergedSharesTest extends TestCase {
+
+ /** @var IRepairStep */
+ private $repair;
+
+ /** @var \OCP\IDBConnection */
+ private $connection;
+
+ /** @var int */
+ private $lastShareTime;
+
+ protected function setUp() {
+ parent::setUp();
+
+ $config = $this->getMockBuilder('OCP\IConfig')
+ ->disableOriginalConstructor()
+ ->getMock();
+ $config->expects($this->any())
+ ->method('getSystemValue')
+ ->with('version')
+ ->will($this->returnValue('9.0.3.0'));
+
+ $this->connection = \OC::$server->getDatabaseConnection();
+ $this->deleteAllShares();
+
+ $user1 = $this->getMock('\OCP\IUser');
+ $user1->expects($this->any())
+ ->method('getUID')
+ ->will($this->returnValue('user1'));
+
+ $user2 = $this->getMock('\OCP\IUser');
+ $user2->expects($this->any())
+ ->method('getUID')
+ ->will($this->returnValue('user2'));
+
+ $users = [$user1, $user2];
+
+ $groupManager = $this->getMock('\OCP\IGroupManager');
+ $groupManager->expects($this->any())
+ ->method('getUserGroupIds')
+ ->will($this->returnValueMap([
+ // owner
+ [$user1, ['samegroup1', 'samegroup2']],
+ // recipient
+ [$user2, ['recipientgroup1', 'recipientgroup2']],
+ ]));
+
+ $userManager = $this->getMock('\OCP\IUserManager');
+ $userManager->expects($this->once())
+ ->method('countUsers')
+ ->will($this->returnValue([2]));
+ $userManager->expects($this->once())
+ ->method('callForAllUsers')
+ ->will($this->returnCallback(function(\Closure $closure) use ($users) {
+ foreach ($users as $user) {
+ $closure($user);
+ }
+ }));
+
+ // used to generate incremental stimes
+ $this->lastShareTime = time();
+
+ /** @var \OCP\IConfig $config */
+ $this->repair = new RepairUnmergedShares($config, $this->connection, $userManager, $groupManager);
+ }
+
+ protected function tearDown() {
+ $this->deleteAllShares();
+
+ parent::tearDown();
+ }
+
+ protected function deleteAllShares() {
+ $qb = $this->connection->getQueryBuilder();
+ $qb->delete('share')->execute();
+ }
+
+ private function createShare($type, $sourceId, $recipient, $targetName, $permissions, $parentId = null) {
+ $this->lastShareTime += 100;
+ $qb = $this->connection->getQueryBuilder();
+ $values = [
+ 'share_type' => $qb->expr()->literal($type),
+ 'share_with' => $qb->expr()->literal($recipient),
+ 'uid_owner' => $qb->expr()->literal('user1'),
+ 'item_type' => $qb->expr()->literal('folder'),
+ 'item_source' => $qb->expr()->literal($sourceId),
+ 'item_target' => $qb->expr()->literal('/' . $sourceId),
+ 'file_source' => $qb->expr()->literal($sourceId),
+ 'file_target' => $qb->expr()->literal($targetName),
+ 'permissions' => $qb->expr()->literal($permissions),
+ 'stime' => $qb->expr()->literal($this->lastShareTime),
+ ];
+ if ($parentId !== null) {
+ $values['parent'] = $qb->expr()->literal($parentId);
+ }
+ $qb->insert('share')
+ ->values($values)
+ ->execute();
+
+ return $this->connection->lastInsertId('*PREFIX*share');
+ }
+
+ private function getShareById($id) {
+ $query = $this->connection->getQueryBuilder();
+ $results = $query
+ ->select('*')
+ ->from('share')
+ ->where($query->expr()->eq('id', $query->expr()->literal($id)))
+ ->execute()
+ ->fetchAll();
+
+ if (!empty($results)) {
+ return $results[0];
+ }
+ return null;
+ }
+
+ public function sharesDataProvider() {
+ /**
+ * For all these test cases we have the following situation:
+ *
+ * - "user1" is the share owner
+ * - "user2" is the recipient, and member of "recipientgroup1" and "recipientgroup2"
+ * - "user1" is member of "samegroup1", "samegroup2" for same group tests
+ */
+ return [
+ [
+ // #0 legitimate share:
+ // - outsider shares with group1, group2
+ // - recipient renamed, resulting in subshares
+ // - one subshare for each group share
+ // - targets of subshare all match
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test renamed', 31, 0],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test renamed', 31, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 31],
+ // leave them alone
+ ['/test renamed', 31],
+ ['/test renamed', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #1 broken share:
+ // - outsider shares with group1, group2
+ // - only one subshare for two group shares
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
+ // child of the previous one
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 31, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 31],
+ ['/test', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #2 bogus share
+ // - outsider shares with group1, group2
+ // - one subshare for each group share, both with parenthesis
+ // - but the targets do not match when grouped
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 31, 0],
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (3)', 31, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 31],
+ // reset to original name as the sub-names have parenthesis
+ ['/test', 31],
+ ['/test', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #3 bogus share
+ // - outsider shares with group1, group2
+ // - one subshare for each group share, both renamed manually
+ // - but the targets do not match when grouped
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test_renamed (1 legit paren)', 31, 0],
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test_renamed (2 legit paren)', 31, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 31],
+ // reset to less recent subshare name
+ ['/test_renamed (2 legit paren)', 31],
+ ['/test_renamed (2 legit paren)', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #4 bogus share
+ // - outsider shares with group1, group2
+ // - one subshare for each group share, one with parenthesis
+ // - but the targets do not match when grouped
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 31, 0],
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test_renamed', 31, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 31],
+ // reset to less recent subshare name but without parenthesis
+ ['/test_renamed', 31],
+ ['/test_renamed', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #5 bogus share
+ // - outsider shares with group1, group2
+ // - one subshare for each group share
+ // - first subshare not renamed (as in real world scenario)
+ // - but the targets do not match when grouped
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test', 31, 0],
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 31, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 31],
+ // reset to original name
+ ['/test', 31],
+ ['/test', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #6 bogus share:
+ // - outsider shares with group1, group2
+ // - one subshare for each group share
+ // - non-matching targets
+ // - recipient deletes one duplicate (unshare from self, permissions 0)
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 15],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 0, 0],
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (3)', 15, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 15],
+ // subshares repaired and permissions restored to the max allowed
+ ['/test', 31],
+ ['/test', 15],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #7 bogus share:
+ // - outsider shares with group1, group2
+ // - one subshare for each group share
+ // - non-matching targets
+ // - recipient deletes ALL duplicates (unshare from self, permissions 0)
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 15],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 0, 0],
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (3)', 0, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 15],
+ // subshares target repaired but left "deleted" as it was the user's choice
+ ['/test', 0],
+ ['/test', 0],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #8 bogus share:
+ // - outsider shares with group1, group2 and also user2
+ // - one subshare for each group share
+ // - one extra share entry for direct share to user2
+ // - non-matching targets
+ // - user share has more permissions
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 1],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 15],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 1, 0],
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (3)', 15, 1],
+ [Constants::SHARE_TYPE_USER, 123, 'user2', '/test (4)', 31],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (5)', 31],
+ ],
+ [
+ ['/test', 1],
+ ['/test', 15],
+ // subshares repaired
+ ['/test', 1],
+ ['/test', 15],
+ ['/test', 31],
+ // leave unrelated alone
+ ['/test (5)', 31],
+ ]
+ ],
+ [
+ // #9 bogus share:
+ // - outsider shares with group1 and also user2
+ // - no subshare at all
+ // - one extra share entry for direct share to user2
+ // - non-matching targets
+ // - user share has more permissions
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 1],
+ [Constants::SHARE_TYPE_USER, 123, 'user2', '/test (2)', 31],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (5)', 31],
+ ],
+ [
+ ['/test', 1],
+ // user share repaired
+ ['/test', 31],
+ // leave unrelated alone
+ ['/test (5)', 31],
+ ]
+ ],
+ [
+ // #10 legitimate share with own group:
+ // - insider shares with both groups the user is already in
+ // - no subshares in this case
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'samegroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'samegroup2', '/test', 31],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ // leave all alone
+ ['/test', 31],
+ ['/test', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #11 legitimate shares:
+ // - group share with same group
+ // - group share with other group
+ // - user share where recipient renamed
+ // - user share where recipient did not rename
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'samegroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_USER, 123, 'user3', '/test legit rename', 31],
+ [Constants::SHARE_TYPE_USER, 123, 'user4', '/test', 31],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ // leave all alone
+ ['/test', 31],
+ ['/test', 31],
+ ['/test legit rename', 31],
+ ['/test', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #12 legitimate share:
+ // - outsider shares with group and user directly with different permissions
+ // - no subshares
+ // - same targets
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'samegroup1', '/test', 1],
+ [Constants::SHARE_TYPE_USER, 123, 'user3', '/test', 31],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ // leave all alone
+ ['/test', 1],
+ ['/test', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #13 bogus share:
+ // - outsider shares with group1, user2 and then group2
+ // - user renamed share as soon as it arrived before the next share (order)
+ // - one subshare for each group share
+ // - one extra share entry for direct share to user2
+ // - non-matching targets
+ [
+ // first share with group
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ // recipient renames
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/first', 31, 0],
+ // then direct share, user renames too
+ [Constants::SHARE_TYPE_USER, 123, 'user2', '/second', 31],
+ // another share with the second group
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
+ // use renames it
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/third', 31, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (5)', 31],
+ ],
+ [
+ // group share with group1 left alone
+ ['/test', 31],
+ // first subshare repaired
+ ['/third', 31],
+ // direct user share repaired
+ ['/third', 31],
+ // group share with group2 left alone
+ ['/test', 31],
+ // second subshare repaired
+ ['/third', 31],
+ // leave unrelated alone
+ ['/test (5)', 31],
+ ]
+ ],
+ ];
+ }
+
+ /**
+ * Test merge shares from group shares
+ *
+ * @dataProvider sharesDataProvider
+ */
+ public function testMergeGroupShares($shares, $expectedShares) {
+ $shareIds = [];
+
+ foreach ($shares as $share) {
+ // if parent
+ if (isset($share[5])) {
+ // adjust to real id
+ $share[5] = $shareIds[$share[5]];
+ } else {
+ $share[5] = null;
+ }
+ $shareIds[] = $this->createShare($share[0], $share[1], $share[2], $share[3], $share[4], $share[5]);
+ }
+
+ /** @var IOutput | \PHPUnit_Framework_MockObject_MockObject $outputMock */
+ $outputMock = $this->getMockBuilder('\OCP\Migration\IOutput')
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ $this->repair->run($outputMock);
+
+ foreach ($expectedShares as $index => $expectedShare) {
+ $share = $this->getShareById($shareIds[$index]);
+ $this->assertEquals($expectedShare[0], $share['file_target']);
+ $this->assertEquals($expectedShare[1], $share['permissions']);
+ }
+ }
+}
+
diff --git a/version.php b/version.php
index 9f7831e0cd76..5f2c604be328 100644
--- a/version.php
+++ b/version.php
@@ -26,7 +26,7 @@
// We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades
// between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel
// when updating major/minor version number.
-$OC_Version = array(9, 0, 4, 1);
+$OC_Version = array(9, 0, 4, 2);
// The human readable string
$OC_VersionString = '9.0.4';