Make share target consistent when grouping group share with user share
In some situations, a group share is created before a user share, and the recipient renamed the received share before the latter is created. In this situation, the "file_target" was already modified and the second created share must align to the already renamed share. To achieve this, the MountProvider now groups only by "item_source" value and sorts by share time. This makes it so that the least recent share is selected as super-share and its "file_target" value is then adjusted in all grouped shares. This fixes the issue where this situation would have different "file_target" values resulting in two shared folders appearing instead of one.
This commit is contained in:
parent
56e9f7cdf9
commit
e75a0a44c6
|
@ -75,7 +75,7 @@ class MountProvider implements IMountProvider {
|
|||
return $share->getPermissions() > 0 && $share->getShareOwner() !== $user->getUID();
|
||||
});
|
||||
|
||||
$superShares = $this->buildSuperShares($shares);
|
||||
$superShares = $this->buildSuperShares($shares, $user);
|
||||
|
||||
$mounts = [];
|
||||
foreach ($superShares as $share) {
|
||||
|
@ -116,17 +116,22 @@ class MountProvider implements IMountProvider {
|
|||
if (!isset($tmp[$share->getNodeId()])) {
|
||||
$tmp[$share->getNodeId()] = [];
|
||||
}
|
||||
$tmp[$share->getNodeId()][$share->getTarget()][] = $share;
|
||||
$tmp[$share->getNodeId()][] = $share;
|
||||
}
|
||||
|
||||
$result = [];
|
||||
foreach ($tmp as $tmp2) {
|
||||
foreach ($tmp2 as $item) {
|
||||
$result[] = $item;
|
||||
// sort by stime, the super share will be based on the least recent share
|
||||
foreach ($tmp as &$tmp2) {
|
||||
@usort($tmp2, function($a, $b) {
|
||||
if ($a->getShareTime() < $b->getShareTime()) {
|
||||
return -1;
|
||||
}
|
||||
return 1;
|
||||
});
|
||||
$result[] = $tmp2;
|
||||
}
|
||||
|
||||
return $result;
|
||||
return array_values($result);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -136,9 +141,10 @@ class MountProvider implements IMountProvider {
|
|||
* of all shares within the group.
|
||||
*
|
||||
* @param \OCP\Share\IShare[] $allShares
|
||||
* @param \OCP\IUser $user user
|
||||
* @return array Tuple of [superShare, groupedShares]
|
||||
*/
|
||||
private function buildSuperShares(array $allShares) {
|
||||
private function buildSuperShares(array $allShares, \OCP\IUser $user) {
|
||||
$result = [];
|
||||
|
||||
$groupedShares = $this->groupShares($allShares);
|
||||
|
@ -161,6 +167,11 @@ class MountProvider implements IMountProvider {
|
|||
$permissions = 0;
|
||||
foreach ($shares as $share) {
|
||||
$permissions |= $share->getPermissions();
|
||||
if ($share->getTarget() !== $superShare->getTarget()) {
|
||||
// adjust target, for database consistency
|
||||
$share->setTarget($superShare->getTarget());
|
||||
$this->shareManager->moveShare($share, $user->getUID());
|
||||
}
|
||||
}
|
||||
|
||||
$superShare->setPermissions($permissions);
|
||||
|
|
|
@ -85,6 +85,12 @@ class MountProviderTest extends \Test\TestCase {
|
|||
$share->expects($this->any())
|
||||
->method('getNodeId')
|
||||
->will($this->returnValue($nodeId));
|
||||
$share->expects($this->any())
|
||||
->method('getShareTime')
|
||||
->will($this->returnValue(
|
||||
// compute share time based on id, simulating share order
|
||||
new \DateTime('@' . (1469193980 + 1000 * $id))
|
||||
));
|
||||
return $share;
|
||||
}
|
||||
|
||||
|
@ -102,7 +108,7 @@ class MountProviderTest extends \Test\TestCase {
|
|||
];
|
||||
$groupShares = [
|
||||
$this->makeMockShare(3, 100, 'user2', '/share2', 0),
|
||||
$this->makeMockShare(4, 100, 'user2', '/share4', 31),
|
||||
$this->makeMockShare(4, 101, 'user2', '/share4', 31),
|
||||
$this->makeMockShare(5, 100, 'user1', '/share4', 31),
|
||||
];
|
||||
$this->user->expects($this->any())
|
||||
|
@ -134,7 +140,7 @@ class MountProviderTest extends \Test\TestCase {
|
|||
$mountedShare2 = $mounts[1]->getShare();
|
||||
$this->assertEquals('4', $mountedShare2->getId());
|
||||
$this->assertEquals('user2', $mountedShare2->getShareOwner());
|
||||
$this->assertEquals(100, $mountedShare2->getNodeId());
|
||||
$this->assertEquals(101, $mountedShare2->getNodeId());
|
||||
$this->assertEquals('/share4', $mountedShare2->getTarget());
|
||||
$this->assertEquals(31, $mountedShare2->getPermissions());
|
||||
}
|
||||
|
@ -229,6 +235,32 @@ class MountProviderTest extends \Test\TestCase {
|
|||
// no received share since "user1" opted out
|
||||
],
|
||||
],
|
||||
// #7: share as outsider with "group1" and "user1" where recipient renamed in between
|
||||
[
|
||||
[
|
||||
[1, 100, 'user2', '/share2-renamed', 31],
|
||||
],
|
||||
[
|
||||
[2, 100, 'user2', '/share2', 31],
|
||||
],
|
||||
[
|
||||
// use target of least recent share
|
||||
['1', 100, 'user2', '/share2-renamed', 31],
|
||||
],
|
||||
],
|
||||
// #8: share as outsider with "group1" and "user1" where recipient renamed in between
|
||||
[
|
||||
[
|
||||
[2, 100, 'user2', '/share2', 31],
|
||||
],
|
||||
[
|
||||
[1, 100, 'user2', '/share2-renamed', 31],
|
||||
],
|
||||
[
|
||||
// use target of least recent share
|
||||
['1', 100, 'user2', '/share2-renamed', 31],
|
||||
],
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue