Merge pull request #21660 from owncloud/sharing2_update_reshares

upgrade to sharing 2.0, remove hierarchical re-shares

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
 	<licence>AGPL</licence>
 	<author>Michael Gapczynski, Bjoern Schiessle</author>
 	<default_enable/>
-	<version>0.8.1</version>
+	<version>0.9.0</version>
 	<types>
 		<filesystem/>
 	</types>

apps/files_sharing/appinfo/update.php

@@ -24,10 +24,11 @@ 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();
+	$m->updateInitiatorInfo();
 }
 
 \OC::$server->getJobList()->add('OCA\Files_sharing\Lib\DeleteOrphanedSharesJob');

apps/files_sharing/lib/migration.php

@@ -1,7 +1,6 @@
 <?php
 /**
  * @author Björn Schießle <schiessle@owncloud.com>
- * @author Morris Jobke <hey@morrisjobke.de>
  *
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  * @license AGPL-3.0
@@ -20,22 +19,246 @@
  *
  */
 
- namespace OCA\Files_Sharing;
+namespace OCA\Files_Sharing;
 
+use Doctrine\DBAL\Connection;
+use OCP\IDBConnection;
+use OC\Cache\CappedMemoryCache;
+
+/**
+ * Class Migration
+ *
+ * @package OCA\Files_Sharing
+ * @group DB
+ */
 class Migration {
 
+	/** @var IDBConnection */
+	private $connection;
+
+	/** @var  array with all shares we already saw */
+	private $shareCache;
+
+	/** @var string */
+	private $table = 'share';
+
+	public function __construct(IDBConnection $connection) {
+		$this->connection = $connection;
+
+		// We cache up to 10k share items (~20MB)
+		$this->shareCache = new CappedMemoryCache(10000);
+	}
+
+	/**
+	 * 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() {
+
+		while(true) {
+			$reShares = $this->getReShares(1000);
+
+			if (empty($reShares)) {
+				break;
+			}
+
+			// Update the cache
+			foreach($reShares as $reShare) {
+				$this->shareCache[$reShare['id']] = $reShare;
+			}
+
+			$owners = [];
+			foreach ($reShares as $share) {
+				$owners[$share['id']] = [
+					'owner' => $this->findOwner($share),
+					'initiator' => $share['uid_owner']
+				];
+			}
+			$this->updateOwners($owners);
+
+			//Clear the cache of the shares we just updated so we have more room
+			foreach($owners as $id => $owner) {
+				unset($this->shareCache[$id]);
+			}
+		}
+	}
 
 	/**
-	 * 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
+	 * update all owner information so that all shares have an owner
+	 * and an initiator for the upgrade from oC 8.2 to 9.0 with the new sharing
 	 */
-	public function addAcceptRow() {
-		$statement = 'UPDATE `*PREFIX*share_external` SET `accepted` = 1';
-		$connection = \OC::$server->getDatabaseConnection();
-		$query = $connection->prepare($statement);
-		$query->execute();
+	public function updateInitiatorInfo() {
+		while (true) {
+			$shares = $this->getMissingInitiator(1000);
+
+			if (empty($shares)) {
+				break;
+			}
+
+			$owners = [];
+			foreach ($shares as $share) {
+				$owners[$share['id']] = [
+					'owner' => $share['uid_owner'],
+					'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 $n re-shares from the database
+	 *
+	 * @param int $n The max number of shares to fetch
+	 * @return array
+	 */
+	private function getReShares($n = 1000) {
+		$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'))
+			->orderBy('id', 'asc')
+			->setMaxResults($n);
+		$result = $query->execute();
+		$shares = $result->fetchAll();
+		$result->closeCursor();
+
+		$ordered = [];
+		foreach ($shares as $share) {
+			$ordered[(int)$share['id']] = $share;
+		}
+
+		return $ordered;
+	}
+
+	/**
+	 * Get $n re-shares from the database
+	 *
+	 * @param int $n The max number of shares to fetch
+	 * @return array
+	 */
+	private function getMissingInitiator($n = 1000) {
+		$query = $this->connection->getQueryBuilder();
+		$query->select(['id', '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()->isNull('uid_initiator'))
+			->orderBy('id', 'asc')
+			->setMaxResults($n);
+		$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;
+		}
+
+	}
 
 }

apps/files_sharing/tests/migrationtest.php

@@ -32,51 +32,277 @@ 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()
+		);
+	}
 
-		$query = $this->connection->prepare('SELECT `id` FROM `*PREFIX*share_external`');
-		$query->execute();
-		$dummyEntries = $query->fetchAll();
+	public function testRemoveReShares() {
+		$this->addDummyValues();
+		$this->migration->removeReShares();
+		$this->verifyResult();
+	}
 
-		$this->assertSame(10, count($dummyEntries));
+	public function verifyResult() {
+		$query = $this->connection->getQueryBuilder();
+		$query->select('*')->from($this->table)->orderBy('id');
+		$result = $query->execute()->fetchAll();
+		$this->assertSame(8, count($result));
 
-		$m = new Migration();
-		$m->addAcceptRow();
+		// 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']);
+		}
+		// 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']);
+		}
+	}
 
-		// verify result
-		$query = $this->connection->prepare('SELECT `accepted` FROM `*PREFIX*share_external`');
-		$query->execute();
-		$results = $query->fetchAll();
-		$this->assertSame(10, count($results));
+	public function test1001DeepReshares() {
+		$parent = null;
+		for ($i = 0; $i < 1001; $i++) {
+			$query = $this->connection->getQueryBuilder();
+			$query->insert($this->table)
+				->values(
+					[
+						'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'),
+					]
+				)
+				->setParameter('share_type', \OCP\Share::SHARE_TYPE_USER)
+				->setParameter('share_with', 'user'.($i+1))
+				->setParameter('uid_owner', 'user'.($i))
+				->setParameter('uid_initiator', null)
+				->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());
 
-		foreach ($results as $r) {
-			$this->assertSame(1, (int) $r['accepted']);
+			$this->assertSame(1, $query->execute());
+			$parent = $query->getLastInsertId();
 		}
 
-		// cleanup
-		$cleanup = $this->connection->prepare('DELETE FROM `*PREFIX*share_external`');
-		$cleanup->execute();
-	}
+		$this->migration->removeReShares();
+		$this->migration->updateInitiatorInfo();
 
+		$qb = $this->connection->getQueryBuilder();
+
+		$stmt = $qb->select('id', 'share_with', 'uid_owner', 'uid_initiator', 'parent')
+			->from('share')
+			->orderBy('id', 'asc')
+			->execute();
+
+		$i = 0;
+		while($share = $stmt->fetch()) {
+			$this->assertEquals('user'.($i+1), $share['share_with']);
+			$this->assertEquals('user' . ($i), $share['uid_initiator']);
+			$this->assertEquals('user0', $share['uid_owner']);
+			$this->assertEquals(null, $share['parent']);
+			$i++;
+		}
+		$stmt->closeCursor();
+		$this->assertEquals(1001, $i);
+	}
 }