Fix remote group share API interactions

Accepting and declining can now be done repeatedly on both the parent
group share and sub-share with the same effects.

Added unit tests to cover these cases, and also when the same operation
is repeated.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
pull/27751/head
Vincent Petry 5 years ago
parent c1563215e6
commit 15f41a6b72
No known key found for this signature in database
GPG Key ID: E055D6A4D513575C
  1. 44
      apps/files_sharing/lib/External/Manager.php
  2. 166
      apps/files_sharing/tests/External/ManagerTest.php

@ -238,6 +238,20 @@ class Manager {
return $share;
}
private function fetchUserShare($parentId, $uid) {
$getShare = $this->connection->prepare('
SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted`, `parent`, `share_type`, `password`, `mountpoint_hash`
FROM `*PREFIX*share_external`
WHERE `parent` = ? AND `user` = ?');
$result = $getShare->execute([$parentId, $uid]);
$share = $result->fetch();
$result->closeCursor();
if ($share !== false) {
return $share;
}
return null;
}
/**
* get share
*
@ -311,9 +325,15 @@ class Manager {
} else {
$parentId = (int)$share['parent'];
if ($parentId !== -1) {
// this is the sub-share, simply update it to re-accept
// this is the sub-share
$subshare = $share;
} else {
$subshare = $this->fetchUserShare($id, $this->uid);
}
if ($subshare !== null) {
try {
$this->updateAccepted((int)$share['id'], true);
$this->updateAccepted((int)$subshare['id'], true);
$result = true;
} catch (Exception $e) {
$this->logger->logException($e);
@ -372,13 +392,17 @@ class Manager {
$this->processNotification($id);
$result = true;
} elseif ($share && (int)$share['share_type'] === IShare::TYPE_GROUP) {
$parent = (int)$share['parent'];
// can only decline an already accepted/mounted group share,
// check if this is the sub-share entry
if ($parent !== -1) {
$parentId = (int)$share['parent'];
if ($parentId !== -1) {
// this is the sub-share
$subshare = $share;
} else {
$subshare = $this->fetchUserShare($id, $this->uid);
}
if ($subshare !== null) {
try {
// this is the sub-share, simply update it to decline
$this->updateAccepted((int)$share['id'], false);
$this->updateAccepted((int)$subshare['id'], false);
$result = true;
} catch (Exception $e) {
$this->logger->logException($e);
@ -566,6 +590,10 @@ class Manager {
public function removeShare($mountPoint): bool {
$mountPointObj = $this->mountManager->find($mountPoint);
if ($mountPointObj === null) {
$this->logger->error('Mount point to remove share not found', ['mountPoint' => $mountPoint]);
return false;
}
$id = $mountPointObj->getStorage()->getCache()->getId('');
$mountPoint = $this->stripPath($mountPoint);

@ -114,6 +114,9 @@ class ManagerTest extends TestCase {
->method('search')
->willReturn([]);
$logger = $this->createMock(ILogger::class);
$logger->expects($this->never())->method('logException');
$this->manager = $this->getMockBuilder(Manager::class)
->setConstructorArgs(
[
@ -129,7 +132,7 @@ class ManagerTest extends TestCase {
$this->userManager,
$this->uid,
$this->eventDispatcher,
$this->createMock(ILogger::class),
$logger,
]
)->setMethods(['tryOCMEndPoint'])->getMock();
@ -156,6 +159,7 @@ class ManagerTest extends TestCase {
}
private function setupMounts() {
$this->mountManager->clear();
$mounts = $this->testMountProvider->getMountsForUser($this->user, new StorageFactory());
foreach ($mounts as $mount) {
$this->mountManager->addMount($mount);
@ -248,7 +252,7 @@ class ManagerTest extends TestCase {
}
// Accept the first share
$this->manager->acceptShare($openShares[0]['id']);
$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
// Check remaining shares - Accepted
$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
@ -304,7 +308,7 @@ class ManagerTest extends TestCase {
}
// Decline the third share
$this->manager->declineShare($openShares[1]['id']);
$this->assertTrue($this->manager->declineShare($openShares[1]['id']));
$this->setupMounts();
$this->assertMount($shareData1['name']);
@ -380,6 +384,162 @@ class ManagerTest extends TestCase {
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
}
private function verifyAcceptedGroupShare($shareData) {
$openShares = $this->manager->getOpenShares();
$this->assertCount(0, $openShares);
$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
$this->assertCount(1, $acceptedShares);
$shareData['accepted'] = true;
$this->assertExternalShareEntry($shareData, $acceptedShares[0], 0, $shareData['name'], $this->uid);
$this->setupMounts();
$this->assertMount($shareData['name']);
}
private function verifyDeclinedGroupShare($shareData, $tempMount = null) {
if ($tempMount === null) {
$tempMount = '{{TemporaryMountPointName#/SharedFolder}}';
}
$openShares = $this->manager->getOpenShares();
$this->assertCount(1, $openShares);
$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
$this->assertCount(0, $acceptedShares);
$this->assertExternalShareEntry($shareData, $openShares[0], 0, $tempMount, $this->uid);
$this->setupMounts();
$this->assertNotMount($shareData['name']);
$this->assertNotMount($tempMount);
}
private function createTestGroupShare() {
$shareData = [
'remote' => 'http://localhost',
'token' => 'token1',
'password' => '',
'name' => '/SharedFolder',
'owner' => 'foobar',
'shareType' => IShare::TYPE_GROUP,
'accepted' => false,
'user' => 'group1',
'remoteId' => '2342'
];
$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData));
$allShares = self::invokePrivate($this->manager, 'getShares', [null]);
$this->assertCount(1, $allShares);
// this will hold the main group entry
$groupShare = $allShares[0];
return [$shareData, $groupShare];
}
public function testAcceptOriginalGroupShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// a second time
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
}
public function testAcceptGroupShareAgainThroughGroupShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// decline again, this keeps the sub-share
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// this will return sub-entries
$openShares = $this->manager->getOpenShares();
// accept through sub-share
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData, '/SharedFolder');
// accept a second time
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData, '/SharedFolder');
}
public function testAcceptGroupShareAgainThroughSubShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// decline again, this keeps the sub-share
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// this will return sub-entries
$openShares = $this->manager->getOpenShares();
// accept through sub-share
$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
$this->verifyAcceptedGroupShare($shareData);
// accept a second time
$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
$this->verifyAcceptedGroupShare($shareData);
}
public function testDeclineOriginalGroupShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData);
// a second time
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData);
}
public function testDeclineGroupShareAgainThroughGroupShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// decline again, this keeps the sub-share
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// a second time
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
}
public function testDeclineGroupShareAgainThroughSubshare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// this will return sub-entries
$allShares = self::invokePrivate($this->manager, 'getShares', [null]);
$this->assertCount(1, $allShares);
// decline again through sub-share
$this->assertTrue($this->manager->declineShare($allShares[0]['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// a second time
$this->assertTrue($this->manager->declineShare($allShares[0]['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
}
public function testDeclineGroupShareAgainThroughMountPoint() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// decline through mount point name
$this->assertTrue($this->manager->removeShare($this->uid . '/files/' . $shareData['name']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// second time must fail as the mount point is gone
$this->assertFalse($this->manager->removeShare($this->uid . '/files/' . $shareData['name']));
}
/**
* @param array $expected
* @param array $actual

Loading…
Cancel
Save