summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--apps/files_sharing/lib/migration.php68
-rw-r--r--apps/files_sharing/tests/api.php120
-rw-r--r--apps/files_sharing/tests/migrationtest.php27
-rw-r--r--lib/private/share20/defaultshareprovider.php4
-rw-r--r--lib/private/share20/manager.php23
-rw-r--r--tests/lib/share20/managertest.php4
6 files changed, 214 insertions, 32 deletions
diff --git a/apps/files_sharing/lib/migration.php b/apps/files_sharing/lib/migration.php
index e7346385510..31a76687d40 100644
--- a/apps/files_sharing/lib/migration.php
+++ b/apps/files_sharing/lib/migration.php
@@ -55,32 +55,30 @@ class Migration {
*/
public function removeReShares() {
- while(true) {
- $reShares = $this->getReShares(1000);
+ $stmt = $this->getReShares();
- if (empty($reShares)) {
- break;
- }
+ $owners = [];
+ while($share = $stmt->fetch()) {
- // Update the cache
- foreach($reShares as $reShare) {
- $this->shareCache[$reShare['id']] = $reShare;
- }
+ $this->shareCache[$share['id']] = $share;
- $owners = [];
- foreach ($reShares as $share) {
- $owners[$share['id']] = [
+ $owners[$share['id']] = [
'owner' => $this->findOwner($share),
- 'initiator' => $share['uid_owner']
- ];
- }
- $this->updateOwners($owners);
+ 'initiator' => $share['uid_owner'],
+ 'type' => $share['share_type'],
+ ];
- //Clear the cache of the shares we just updated so we have more room
- foreach($owners as $id => $owner) {
- unset($this->shareCache[$id]);
+ if (count($owners) === 1000) {
+ $this->updateOwners($owners);
+ $owners = [];
}
}
+
+ $stmt->closeCursor();
+
+ if (count($owners)) {
+ $this->updateOwners($owners);
+ }
}
/**
@@ -99,7 +97,8 @@ class Migration {
foreach ($shares as $share) {
$owners[$share['id']] = [
'owner' => $share['uid_owner'],
- 'initiator' => $share['uid_owner']
+ 'initiator' => $share['uid_owner'],
+ 'type' => $share['share_type'],
];
}
$this->updateOwners($owners);
@@ -130,11 +129,11 @@ class Migration {
* Get $n re-shares from the database
*
* @param int $n The max number of shares to fetch
- * @return array
+ * @return \Doctrine\DBAL\Driver\Statement
*/
- private function getReShares($n = 1000) {
+ private function getReShares() {
$query = $this->connection->getQueryBuilder();
- $query->select(['id', 'parent', 'uid_owner'])
+ $query->select(['id', 'parent', 'uid_owner', 'share_type'])
->from($this->table)
->where($query->expr()->in(
'share_type',
@@ -156,9 +155,10 @@ class Migration {
)
))
->andWhere($query->expr()->isNotNull('parent'))
- ->orderBy('id', 'asc')
- ->setMaxResults($n);
- $result = $query->execute();
+ ->orderBy('id', 'asc');
+ return $query->execute();
+
+
$shares = $result->fetchAll();
$result->closeCursor();
@@ -178,7 +178,7 @@ class Migration {
*/
private function getMissingInitiator($n = 1000) {
$query = $this->connection->getQueryBuilder();
- $query->select(['id', 'uid_owner'])
+ $query->select(['id', 'uid_owner', 'share_type'])
->from($this->table)
->where($query->expr()->in(
'share_type',
@@ -247,11 +247,17 @@ class Migration {
foreach ($owners as $id => $owner) {
$query = $this->connection->getQueryBuilder();
$query->update($this->table)
- ->set('parent', $query->createNamedParameter(null))
->set('uid_owner', $query->createNamedParameter($owner['owner']))
- ->set('uid_initiator', $query->createNamedParameter($owner['initiator']))
- ->where($query->expr()->eq('id', $query->createNamedParameter($id)))
- ->execute();
+ ->set('uid_initiator', $query->createNamedParameter($owner['initiator']));
+
+
+ if ((int)$owner['type'] !== \OCP\Share::SHARE_TYPE_LINK) {
+ $query->set('parent', $query->createNamedParameter(null));
+ }
+
+ $query->where($query->expr()->eq('id', $query->createNamedParameter($id)));
+
+ $query->execute();
}
$this->connection->commit();
diff --git a/apps/files_sharing/tests/api.php b/apps/files_sharing/tests/api.php
index 49a08d3d0ce..3a2ae1eef37 100644
--- a/apps/files_sharing/tests/api.php
+++ b/apps/files_sharing/tests/api.php
@@ -40,6 +40,9 @@ class Test_Files_Sharing_Api extends TestCase {
private static $tempStorage;
+ /** @var \OCP\Share\IManager */
+ private $shareManager;
+
protected function setUp() {
parent::setUp();
@@ -59,6 +62,8 @@ class Test_Files_Sharing_Api extends TestCase {
$this->view->mkdir($this->folder . $this->subfolder . $this->subsubfolder);
$this->view->file_put_contents($this->folder.$this->filename, $this->data);
$this->view->file_put_contents($this->folder . $this->subfolder . $this->filename, $this->data);
+
+ $this->shareManager = \OC::$server->getShareManager();
}
protected function tearDown() {
@@ -73,6 +78,40 @@ class Test_Files_Sharing_Api extends TestCase {
}
/**
+ * @param array $data
+ * @return \OCP\IRequest
+ */
+ private function createRequest(array $data) {
+ $request = $this->getMock('\OCP\IRequest');
+ $request->method('getParam')
+ ->will($this->returnCallback(function($param, $default = null) use ($data) {
+ if (isset($data[$param])) {
+ return $data[$param];
+ }
+ return $default;
+ }));
+ return $request;
+ }
+
+ /**
+ * @param \OCP\IRequest $request
+ * @param string $userId The userId of the caller
+ * @return \OCA\Files_Sharing\API\Share20OCS
+ */
+ private function createOCS($request, $userId) {
+ $currentUser = \OC::$server->getUserManager()->get($userId);
+ return new \OCA\Files_Sharing\API\Share20OCS(
+ $this->shareManager,
+ \OC::$server->getGroupManager(),
+ \OC::$server->getUserManager(),
+ $request,
+ \OC::$server->getRootFolder(),
+ \OC::$server->getURLGenerator(),
+ $currentUser
+ );
+ }
+
+ /**
* @medium
*/
function testCreateShareUserFile() {
@@ -1725,4 +1764,85 @@ class Test_Files_Sharing_Api extends TestCase {
$config->setAppValue('core', 'shareapi_enforce_expire_date', 'no');
}
+ /**
+ * test for no invisible shares
+ * See: https://github.com/owncloud/core/issues/22295
+ */
+ public function testInvisibleSharesUser() {
+ // simulate a post request
+ $request = $this->createRequest([
+ 'path' => $this->folder,
+ 'shareWith' => self::TEST_FILES_SHARING_API_USER2,
+ 'shareType' => \OCP\Share::SHARE_TYPE_USER
+ ]);
+ $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1);
+ $result = $ocs->createShare();
+ $this->assertTrue($result->succeeded());
+ $data = $result->getData();
+
+ $topId = $data['id'];
+
+ $request = $this->createRequest([
+ 'path' => $this->folder . $this->subfolder,
+ 'shareType' => \OCP\Share::SHARE_TYPE_LINK,
+ ]);
+ $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER2);
+ $result = $ocs->createShare();
+ $this->assertTrue($result->succeeded());
+
+ $request = $this->createRequest([]);
+ $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1);
+ $result = $ocs->deleteShare($topId);
+ $this->assertTrue($result->succeeded());
+
+ $request = $this->createRequest([
+ 'reshares' => 'true',
+ ]);
+ $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1);
+ $result = $ocs->getShares();
+ $this->assertTrue($result->succeeded());
+
+ $this->assertEmpty($result->getData());
+ }
+
+ /**
+ * test for no invisible shares
+ * See: https://github.com/owncloud/core/issues/22295
+ */
+ public function testInvisibleSharesGroup() {
+ // simulate a post request
+ $request = $this->createRequest([
+ 'path' => $this->folder,
+ 'shareWith' => self::TEST_FILES_SHARING_API_GROUP1,
+ 'shareType' => \OCP\Share::SHARE_TYPE_GROUP
+ ]);
+ $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1);
+ $result = $ocs->createShare();
+ $this->assertTrue($result->succeeded());
+ $data = $result->getData();
+
+ $topId = $data['id'];
+
+ $request = $this->createRequest([
+ 'path' => $this->folder . $this->subfolder,
+ 'shareType' => \OCP\Share::SHARE_TYPE_LINK,
+ ]);
+ $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER2);
+ $result = $ocs->createShare();
+ $this->assertTrue($result->succeeded());
+
+ $request = $this->createRequest([]);
+ $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1);
+ $result = $ocs->deleteShare($topId);
+ $this->assertTrue($result->succeeded());
+
+ $request = $this->createRequest([
+ 'reshares' => 'true',
+ ]);
+ $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1);
+ $result = $ocs->getShares();
+ $this->assertTrue($result->succeeded());
+
+ $this->assertEmpty($result->getData());
+ }
}
diff --git a/apps/files_sharing/tests/migrationtest.php b/apps/files_sharing/tests/migrationtest.php
index 8a40b76a642..1bca4a419f3 100644
--- a/apps/files_sharing/tests/migrationtest.php
+++ b/apps/files_sharing/tests/migrationtest.php
@@ -226,6 +226,23 @@ class MigrationTest extends TestCase {
$this->assertSame(1,
$query->execute()
);
+
+ // Link reshare should keep its parent
+ $query->setParameter('share_type', \OCP\Share::SHARE_TYPE_LINK)
+ ->setParameter('share_with', null)
+ ->setParameter('uid_owner', 'user3')
+ ->setParameter('uid_initiator', '')
+ ->setParameter('parent', $parent)
+ ->setParameter('item_type', 'file')
+ ->setParameter('item_source', '2')
+ ->setParameter('item_target', '/2')
+ ->setParameter('file_source', 2)
+ ->setParameter('file_target', '/foobar')
+ ->setParameter('permissions', 31)
+ ->setParameter('stime', time());
+ $this->assertSame(1,
+ $query->execute()
+ );
}
public function testRemoveReShares() {
@@ -238,7 +255,7 @@ class MigrationTest extends TestCase {
$query = $this->connection->getQueryBuilder();
$query->select('*')->from($this->table)->orderBy('id');
$result = $query->execute()->fetchAll();
- $this->assertSame(9, count($result));
+ $this->assertSame(10, count($result));
// shares which shouldn't be modified
for ($i = 0; $i < 4; $i++) {
@@ -261,6 +278,14 @@ class MigrationTest extends TestCase {
$this->assertSame($user, $result[$i]['uid_initiator']);
$this->assertNull($result[$i]['parent']);
}
+
+ /*
+ * The link share is flattend but has an owner to avoid invisible shares
+ * see: https://github.com/owncloud/core/pull/22317
+ */
+ $this->assertSame('owner2', $result[9]['uid_owner']);
+ $this->assertSame('user3', $result[9]['uid_initiator']);
+ $this->assertSame($result[7]['id'], $result[9]['parent']);
}
public function test1001DeepReshares() {
diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php
index 0ab0dc81fa7..e18e306d7f6 100644
--- a/lib/private/share20/defaultshareprovider.php
+++ b/lib/private/share20/defaultshareprovider.php
@@ -118,6 +118,10 @@ class DefaultShareProvider implements IShareProvider {
if ($share->getExpirationDate() !== null) {
$qb->setValue('expiration', $qb->createNamedParameter($share->getExpirationDate(), 'datetime'));
}
+
+ if (method_exists($share, 'getParent')) {
+ $qb->setValue('parent', $qb->createNamedParameter($share->getParent()));
+ }
} else {
throw new \Exception('invalid share type!');
}
diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php
index 4345784d2e7..12893c26964 100644
--- a/lib/private/share20/manager.php
+++ b/lib/private/share20/manager.php
@@ -415,6 +415,28 @@ class Manager implements IManager {
}
/**
+ * To make sure we don't get invisible link shares we set the parent
+ * of a link if it is a reshare. This is a quick word around
+ * until we can properly display multiple link shares in the UI
+ *
+ * See: https://github.com/owncloud/core/issues/22295
+ *
+ * FIXME: Remove once multiple link shares can be properly displayed
+ *
+ * @param \OCP\Share\IShare $share
+ */
+ protected function setLinkParent(\OCP\Share\IShare $share) {
+
+ // No sense in checking if the method is not there.
+ if (method_exists($share, 'setParent')) {
+ $storage = $share->getNode()->getStorage();
+ if ($storage->instanceOfStorage('\OCA\Files_Sharing\ISharedStorage')) {
+ $share->setParent($storage->getShareId());
+ }
+ };
+ }
+
+ /**
* @param File|Folder $path
*/
protected function pathCreateChecks($path) {
@@ -470,6 +492,7 @@ class Manager implements IManager {
$this->groupCreateChecks($share);
} else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
$this->linkCreateChecks($share);
+ $this->setLinkParent($share);
/*
* For now ignore a set token.
diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php
index 73a1b0a6530..1230c191b6b 100644
--- a/tests/lib/share20/managertest.php
+++ b/tests/lib/share20/managertest.php
@@ -1549,6 +1549,7 @@ class ManagerTest extends \Test\TestCase {
'pathCreateChecks',
'validateExpirationDate',
'verifyPassword',
+ 'setLinkParent',
])
->getMock();
@@ -1589,6 +1590,9 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->once())
->method('verifyPassword')
->with('password');
+ $manager->expects($this->once())
+ ->method('setLinkParent')
+ ->with($share);
$this->hasher->expects($this->once())
->method('hash')
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-19 19:42:48 +0000