Add repair step for unmerged shares (WIP)

lib/private/Repair.php

@@ -49,6 +49,7 @@ use OC\Repair\RepairMimeTypes;
 use OC\Repair\SearchLuceneTables;
 use OC\Repair\UpdateOutdatedOcsIds;
 use OC\Repair\RepairInvalidShares;
+use OC\Repair\RepairUnmergedShares;
 use OCP\AppFramework\QueryException;
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
@@ -140,6 +141,12 @@ class Repair implements IOutput{
 			new RemoveOldShares(\OC::$server->getDatabaseConnection()),
 			new AvatarPermissions(\OC::$server->getDatabaseConnection()),
 			new RemoveRootShares(\OC::$server->getDatabaseConnection(), \OC::$server->getUserManager(), \OC::$server->getLazyRootFolder()),
+			new RepairUnmergedShares(
+				\OC::$server->getConfig(),
+				\OC::$server->getDatabaseConnection(),
+				\OC::$server->getUserManager(),
+				\OC::$server->getGroupManager()
+			),
 		];
 	}
 

lib/private/Repair/RepairUnmergedShares.php

+<?php
+/**
+ * @author Vincent Petry <pvince81@owncloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\Repair;
+
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+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 implements IRepairStep {
+
+	/** @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')
+			->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;
+	}
+
+	/**
+	 * 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) {
+		$groupSharesById = [];
+		foreach ($groupShares as $groupShare) {
+			$groupSharesById[$groupShare['id']] = $groupShare;
+		}
+
+		if ($this->isThisShareValid($groupSharesById, $subShares)) {
+			return false;
+		}
+
+		$targetPath = $groupShares[0]['file_target'];
+
+		// 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(IOutput $out, IUser $user) {
+		$groups = $this->groupManager->getUserGroupIds($user);
+		if (empty($groups)) {
+			// user is in no groups, so can't have received group shares
+			return;
+		}
+
+		$subSharesByItemSource = $this->getSharesWithUser(DefaultShareProvider::SHARE_TYPE_USERGROUP, [$user->getUID()]);
+		if (empty($subSharesByItemSource)) {
+			// nothing to repair for this user
+			return;
+		}
+
+		$groupSharesByItemSource = $this->getSharesWithUser(Constants::SHARE_TYPE_GROUP, $groups);
+		if (empty($groupSharesByItemSource)) {
+			// shouldn't happen, those invalid shares must be cleant already by RepairInvalidShares
+			return;
+		}
+
+		// because sometimes one wants to give the user more permissions than the group share
+		$userSharesByItemSource = $this->getSharesWithUser(Constants::SHARE_TYPE_USER, [$user->getUID()]);
+
+		foreach ($groupSharesByItemSource as $itemSource => $groupShares) {
+			if (!isset($subSharesByItemSource[$itemSource])) {
+				// no subshares for this item source, skip it
+				continue;
+			}
+			$subShares = $subSharesByItemSource[$itemSource];
+
+			if (isset($userSharesByItemSource[$itemSource])) {
+				// add it to the subshares to get a similar treatment
+				$subShares = array_merge($subShares, $userSharesByItemSource[$itemSource]);
+			}
+
+			$this->fixThisShare($groupShares, $subShares);
+		}
+	}
+
+	/**
+	 * 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(IOutput $output) {
+		$ocVersionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0');
+		if (version_compare($ocVersionFromBeforeUpdate, '9.0.4.0', '<')) {
+			// this situation was only possible between 9.0.0 and 9.0.3 included
+
+			$function = function(IUser $user) use ($output) {
+				$this->fixUnmergedShares($output, $user);
+				$output->advance();
+			};
+
+			$this->buildPreparedQueries();
+
+			$userCount = $this->countUsers();
+			$output->startProgress($userCount);
+
+			$this->userManager->callForAllUsers($function);
+
+			$output->finishProgress();
+		}
+	}
+}

tests/lib/Repair/RepairUnmergedSharesTest.php

+<?php
+/**
+ * @author Vincent Petry <pvince81@owncloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+
+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;
+
+	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);
+				}
+			}));
+
+		/** @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) {
+		$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(time()),
+		];
+		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
+				// - 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
+					['/test', 31],
+					['/test', 31],
+					// leave unrelated alone
+					['/test (4)', 31],
+				]
+			],
+			[
+				// #3 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],
+				]
+			],
+			[
+				// #4 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],
+				]
+			],
+			[
+				// #5 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],
+				]
+			],
+			[
+				// #6 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],
+				]
+			],
+			[
+				// #7 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],
+				]
+			],
+			[
+				// #7 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],
+				]
+			],
+		];
+	}
+
+	/**
+	 * 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']);
+		}
+	}
+}
+