From dc8d43575f71ee19e8c4ad90c3efd8bf9192d3fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 12 Jan 2016 16:28:47 +0100 Subject: [PATCH] upgrade to sharing 2.0, remove hierarchical re-shares --- apps/files_sharing/appinfo/info.xml | 2 +- apps/files_sharing/appinfo/update.php | 8 +- apps/files_sharing/lib/migration.php | 157 ++++++++++++-- apps/files_sharing/tests/migrationtest.php | 237 ++++++++++++++++++--- 4 files changed, 352 insertions(+), 52 deletions(-) diff --git a/apps/files_sharing/appinfo/info.xml b/apps/files_sharing/appinfo/info.xml index bb16345f10..17826be47b 100644 --- a/apps/files_sharing/appinfo/info.xml +++ b/apps/files_sharing/appinfo/info.xml @@ -10,7 +10,7 @@ Turning the feature off removes shared files and folders on the server for all s AGPL Michael Gapczynski, Bjoern Schiessle - 0.8.1 + 0.9.0 diff --git a/apps/files_sharing/appinfo/update.php b/apps/files_sharing/appinfo/update.php index 549b25dae8..ee6482d00c 100644 --- a/apps/files_sharing/appinfo/update.php +++ b/apps/files_sharing/appinfo/update.php @@ -24,10 +24,10 @@ use OCA\Files_Sharing\Migration; $installedVersion = \OC::$server->getConfig()->getAppValue('files_sharing', 'installed_version'); -// Migration OC7 -> OC8 -if (version_compare($installedVersion, '0.6.0', '<')) { - $m = new Migration(); - $m->addAcceptRow(); +// Migration OC8.2 -> OC9 +if (version_compare($installedVersion, '0.9.0', '<')) { + $m = new Migration(\OC::$server->getDatabaseConnection()); + $m->removeReShares(); } \OC::$server->getJobList()->add('OCA\Files_sharing\Lib\DeleteOrphanedSharesJob'); diff --git a/apps/files_sharing/lib/migration.php b/apps/files_sharing/lib/migration.php index 0c5f46a5b3..91e0c4c099 100644 --- a/apps/files_sharing/lib/migration.php +++ b/apps/files_sharing/lib/migration.php @@ -1,7 +1,6 @@ - * @author Morris Jobke * * @copyright Copyright (c) 2016, ownCloud, Inc. * @license AGPL-3.0 @@ -20,22 +19,156 @@ * */ - namespace OCA\Files_Sharing; +namespace OCA\Files_Sharing; +use Doctrine\DBAL\Connection; +use OCP\IDBConnection; + +/** + * Class Migration + * + * @package OCA\Files_Sharing + * @group DB + */ class Migration { + /** @var IDBConnection */ + private $connection; - /** - * set accepted to 1 for all external shares. At this point in time we only - * have shares from the first version of server-to-server sharing so all should - * be accepted - */ - public function addAcceptRow() { - $statement = 'UPDATE `*PREFIX*share_external` SET `accepted` = 1'; - $connection = \OC::$server->getDatabaseConnection(); - $query = $connection->prepare($statement); - $query->execute(); + /** @var array with all shares we already saw */ + private $shareCache; + + /** @var string */ + private $table = 'share'; + + public function __construct(IDBConnection $connection) { + $this->connection = $connection; } + /** + * move all re-shares to the owner in order to have a flat list of shares + * upgrade from oC 8.2 to 9.0 with the new sharing + */ + public function removeReShares() { + $reShares = $this->getAllReShares(); + $this->shareCache = $reShares; + $owners = []; + foreach ($reShares as $share) { + $owners[$share['id']] = [ + 'owner' => $this->findOwner($share), + 'initiator' => $share['uid_owner'] + ]; + } + + $this->updateOwners($owners); + } + + /** + * find the owner of a re-shared file/folder + * + * @param array $share + * @return array + */ + private function findOwner($share) { + $currentShare = $share; + while(!is_null($currentShare['parent'])) { + if (isset($this->shareCache[$currentShare['parent']])) { + $currentShare = $this->shareCache[$currentShare['parent']]; + } else { + $currentShare = $this->getShare((int)$currentShare['parent']); + $this->shareCache[$currentShare['id']] = $currentShare; + } + } + + return $currentShare['uid_owner']; + } + + /** + * get all re-shares from the database + * + * @return array + */ + private function getAllReShares() { + $query = $this->connection->getQueryBuilder(); + $query->select(['id', 'parent', 'uid_owner']) + ->from($this->table) + ->where($query->expr()->in( + 'share_type', + $query->createNamedParameter( + [ + \OCP\Share::SHARE_TYPE_USER, + \OCP\Share::SHARE_TYPE_GROUP, + \OCP\Share::SHARE_TYPE_LINK + ], + Connection::PARAM_INT_ARRAY + ) + )) + ->andWhere($query->expr()->in( + 'item_type', + $query->createNamedParameter( + ['file', 'folder'], + Connection::PARAM_STR_ARRAY + ) + )) + ->andWhere($query->expr()->isNotNull('parent')); + $result = $query->execute(); + $shares = $result->fetchAll(); + $result->closeCursor(); + + $ordered = []; + foreach ($shares as $share) { + $ordered[(int)$share['id']] = $share; + } + + return $ordered; + } + + /** + * get a specific share + * + * @param int $id + * @return array + */ + private function getShare($id) { + $query = $this->connection->getQueryBuilder(); + $query->select(['id', 'parent', 'uid_owner']) + ->from($this->table) + ->where($query->expr()->eq('id', $query->createNamedParameter($id))); + $result = $query->execute(); + $share = $result->fetchAll(); + $result->closeCursor(); + + return $share[0]; + } + + /** + * update database with the new owners + * + * @param array $owners + * @throws \Exception + */ + private function updateOwners($owners) { + + $this->connection->beginTransaction(); + + try { + + 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(); + } + + $this->connection->commit(); + + } catch (\Exception $e) { + $this->connection->rollBack(); + throw $e; + } + + } } diff --git a/apps/files_sharing/tests/migrationtest.php b/apps/files_sharing/tests/migrationtest.php index 14df5af381..6b0e5e3ca2 100644 --- a/apps/files_sharing/tests/migrationtest.php +++ b/apps/files_sharing/tests/migrationtest.php @@ -32,51 +32,218 @@ use OCA\Files_Sharing\Migration; */ class MigrationTest extends TestCase { - /** - * @var \OCP\IDBConnection - */ + /** @var \OCP\IDBConnection */ private $connection; - function __construct() { - parent::__construct(); + /** @var Migration */ + private $migration; + + private $table = 'share'; + + public function setUp() { + parent::setUp(); $this->connection = \OC::$server->getDatabaseConnection(); + $this->migration = new Migration($this->connection); + + $this->cleanDB(); } - function testAddAccept() { + public function tearDown() { + parent::tearDown(); + $this->cleanDB(); + } - $query = $this->connection->prepare(' - INSERT INTO `*PREFIX*share_external` - (`remote`, `share_token`, `password`, `name`, `owner`, `user`, `mountpoint`, `mountpoint_hash`, `remote_id`, `accepted`) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - '); + private function cleanDB() { + $query = $this->connection->getQueryBuilder(); + $query->delete($this->table)->execute(); + } - for ($i = 0; $i < 10; $i++) { - $query->execute(array('remote', 'token', 'password', 'name', 'owner', 'user', 'mount point', $i, $i, 0)); + public function addDummyValues() { + $query = $this->connection->getQueryBuilder(); + $query->insert($this->table) + ->values( + array( + 'share_type' => $query->createParameter('share_type'), + 'share_with' => $query->createParameter('share_with'), + 'uid_owner' => $query->createParameter('uid_owner'), + 'uid_initiator' => $query->createParameter('uid_initiator'), + 'parent' => $query->createParameter('parent'), + 'item_type' => $query->createParameter('item_type'), + 'item_source' => $query->createParameter('item_source'), + 'item_target' => $query->createParameter('item_target'), + 'file_source' => $query->createParameter('file_source'), + 'file_target' => $query->createParameter('file_target'), + 'permissions' => $query->createParameter('permissions'), + 'stime' => $query->createParameter('stime'), + ) + ); + // shared contact, shouldn't be modified + $query->setParameter('share_type', \OCP\Share::SHARE_TYPE_CONTACT) + ->setParameter('share_with', 'user1') + ->setParameter('uid_owner', 'owner1') + ->setParameter('uid_initiator', '') + ->setParameter('parent', null) + ->setParameter('item_type', 'contact') + ->setParameter('item_source', '2') + ->setParameter('item_target', '/2') + ->setParameter('file_source', null) + ->setParameter('file_target', null) + ->setParameter('permissions', 31) + ->setParameter('stime', time()); + $this->assertSame(1, + $query->execute() + ); + // shared calendar, shouldn't be modified + $query->setParameter('share_type', \OCP\Share::SHARE_TYPE_USER) + ->setParameter('share_with', 'user1') + ->setParameter('uid_owner', 'owner1') + ->setParameter('uid_initiator', '') + ->setParameter('parent', null) + ->setParameter('item_type', 'calendar') + ->setParameter('item_source', '2') + ->setParameter('item_target', '/2') + ->setParameter('file_source', null) + ->setParameter('file_target', null) + ->setParameter('permissions', 31) + ->setParameter('stime', time()); + $this->assertSame(1, + $query->execute() + ); + // single user share, shouldn't be modified + $query->setParameter('share_type', \OCP\Share::SHARE_TYPE_USER) + ->setParameter('share_with', 'user1') + ->setParameter('uid_owner', 'owner1') + ->setParameter('uid_initiator', '') + ->setParameter('parent', null) + ->setParameter('item_type', 'file') + ->setParameter('item_source', '2') + ->setParameter('item_target', '/2') + ->setParameter('file_source', 2) + ->setParameter('file_target', '/foo') + ->setParameter('permissions', 31) + ->setParameter('stime', time()); + $this->assertSame(1, + $query->execute() + ); + // single group share, shouldn't be modified + $query->setParameter('share_type', \OCP\Share::SHARE_TYPE_GROUP) + ->setParameter('share_with', 'group1') + ->setParameter('uid_owner', 'owner1') + ->setParameter('uid_initiator', '') + ->setParameter('parent', null) + ->setParameter('item_type', 'file') + ->setParameter('item_source', '2') + ->setParameter('item_target', '/2') + ->setParameter('file_source', 2) + ->setParameter('file_target', '/foo') + ->setParameter('permissions', 31) + ->setParameter('stime', time()); + $this->assertSame(1, + $query->execute() + ); + $parent = $query->getLastInsertId(); + // unique target for group share, shouldn't be modified + $query->setParameter('share_type', 2) + ->setParameter('share_with', 'group1') + ->setParameter('uid_owner', 'owner1') + ->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', '/foo renamed') + ->setParameter('permissions', 31) + ->setParameter('stime', time()); + $this->assertSame(1, + $query->execute() + ); + // first user share, shouldn't be modified + $query->setParameter('share_type', \OCP\Share::SHARE_TYPE_USER) + ->setParameter('share_with', 'user1') + ->setParameter('uid_owner', 'owner2') + ->setParameter('uid_initiator', '') + ->setParameter('parent', null) + ->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() + ); + $parent = $query->getLastInsertId(); + // first re-share, should be attached to the first user share after migration + $query->setParameter('share_type', \OCP\Share::SHARE_TYPE_USER) + ->setParameter('share_with', 'user2') + ->setParameter('uid_owner', 'user1') + ->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() + ); + $parent = $query->getLastInsertId(); + // second re-share, should be attached to the first user share after migration + $query->setParameter('share_type', \OCP\Share::SHARE_TYPE_USER) + ->setParameter('share_with', 'user3') + ->setParameter('uid_owner', 'user2') + ->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() { + $this->addDummyValues(); + $this->migration->removeReShares(); + $this->verifyResult(); + } + + public function verifyResult() { + $query = $this->connection->getQueryBuilder(); + $query->select('*')->from($this->table)->orderBy('id'); + $result = $query->execute()->fetchAll(); + $this->assertSame(8, count($result)); + + // shares which shouldn't be modified + for ($i = 0; $i < 4; $i++) { + $this->assertSame('owner1', $result[$i]['uid_owner']); + $this->assertEmpty($result[$i]['uid_initiator']); + $this->assertNull($result[$i]['parent']); } - - $query = $this->connection->prepare('SELECT `id` FROM `*PREFIX*share_external`'); - $query->execute(); - $dummyEntries = $query->fetchAll(); - - $this->assertSame(10, count($dummyEntries)); - - $m = new Migration(); - $m->addAcceptRow(); - - // verify result - $query = $this->connection->prepare('SELECT `accepted` FROM `*PREFIX*share_external`'); - $query->execute(); - $results = $query->fetchAll(); - $this->assertSame(10, count($results)); - - foreach ($results as $r) { - $this->assertSame(1, (int) $r['accepted']); + // group share with unique target + $this->assertSame('owner1', $result[4]['uid_owner']); + $this->assertEmpty($result[4]['uid_initiator']); + $this->assertNotEmpty($result[4]['parent']); + // initial user share which was re-shared + $this->assertSame('owner2', $result[5]['uid_owner']); + $this->assertEmpty($result[5]['uid_initiator']); + $this->assertNull($result[5]['parent']); + // flatted re-shares + for($i = 6; $i < 8; $i++) { + $this->assertSame('owner2', $result[$i]['uid_owner']); + $user = 'user' . ($i - 5); + $this->assertSame($user, $result[$i]['uid_initiator']); + $this->assertNull($result[$i]['parent']); } - - // cleanup - $cleanup = $this->connection->prepare('DELETE FROM `*PREFIX*share_external`'); - $cleanup->execute(); } }