From 958c7aa11d10807f7cdcae77a20b93e90037fded Mon Sep 17 00:00:00 2001 From: Sujith H Date: Tue, 3 Apr 2018 16:24:16 +0530 Subject: [PATCH] Remove pagination from the repair step The pagination is not required as the rows are deleted during the operation. Signed-off-by: Sujith H --- lib/private/Repair/RepairOrphanedSubshare.php | 35 +- .../lib/Repair/RepairOrphanedSubshareTest.php | 313 ++++++++++++++++++ 2 files changed, 328 insertions(+), 20 deletions(-) diff --git a/lib/private/Repair/RepairOrphanedSubshare.php b/lib/private/Repair/RepairOrphanedSubshare.php index 9ce1cd7bf0bd..6d24c0c50281 100644 --- a/lib/private/Repair/RepairOrphanedSubshare.php +++ b/lib/private/Repair/RepairOrphanedSubshare.php @@ -34,25 +34,17 @@ class RepairOrphanedSubshare implements IRepairStep { /** @var IQueryBuilder */ private $missingParents; - /** @var IQueryBuilder */ - private $deleteOrphanReshares; + /** @var int */ + private $pageLimit; /** * RepairOrphanedSubshare constructor. * * @param IDBConnection $connection */ - public function __construct(IDBConnection $connection) { + public function __construct(IDBConnection $connection, $pageLimit = 1000) { $this->connection = $connection; - - //This delete query deletes orphan shares whose parents are missing - $this->deleteOrphanReshares = $this->connection->getQueryBuilder(); - $this->deleteOrphanReshares - ->delete('share') - ->where( - $this->deleteOrphanReshares->expr()->eq('parent', - $this->deleteOrphanReshares->createParameter('parentId'))); - + $this->pageLimit = $pageLimit; //Set the query to get the parent id's missing from the table. $this->missingParents = $this->connection->getQueryBuilder(); @@ -84,24 +76,27 @@ public function getName() { * @throws \Exception in case of failure */ public function run(IOutput $output) { - $pageLimit = 1000; - $paginationOffset = 0; - $deleteParents = []; /** A one time call to get missing parents from oc_share table */ do { - $this->missingParents->setMaxResults($pageLimit); - $this->missingParents->setFirstResult($paginationOffset); + $this->missingParents->setMaxResults($this->pageLimit); $results = $this->missingParents->execute(); $rows = $results->fetchAll(); $results->closeCursor(); if (count($rows) > 0) { - $paginationOffset += $pageLimit; + $this->connection->beginTransaction(); + //Set the delete command + $deleteStatement = 'DELETE FROM `*PREFIX*share` WHERE `parent` = ?'; foreach ($rows as $row) { - $this->deleteOrphanReshares->setParameter('parentId', $row['parent'])->execute(); + $this->connection->executeUpdate($deleteStatement, + [ + $row['parent'] + ] + ); } + $this->connection->commit(); } - } while (empty($rows)); + } while (!empty($rows)); } } diff --git a/tests/lib/Repair/RepairOrphanedSubshareTest.php b/tests/lib/Repair/RepairOrphanedSubshareTest.php index d1eeabbeeef5..96a1e0cfb504 100644 --- a/tests/lib/Repair/RepairOrphanedSubshareTest.php +++ b/tests/lib/Repair/RepairOrphanedSubshareTest.php @@ -286,4 +286,317 @@ public function testLargeSharesWithOrphans() { } while(count($results) > 0); $this->assertEquals($totalParents - 20, $result); } + + /** + * Positive test + * + * Rows created for test is 90 + */ + public function testOrphanSharesWithoutPagination() { + $qb = $this->connection->getQueryBuilder(); + $totalUsers[] = 'admin'; + //Create 4 users. admin, user1, user2, user3 + $user = 'user'; + for($i=1; $i <= 3; $i++) { + $this->createUser($user.$i); + $totalUsers[] = $user.$i; + } + + //Lets create 3000 entries in oc_share to share + //The idea here is 100 folders of admin are shared with + //30 other users. So each folders are getting re-shared with others + $totalParents = 1; + $parentReshareCount = 1; + $pareReshareCountRest = 0; + $rowCount = 1; + $firstIdSet = false; + foreach ($totalUsers as $user) { + for($i=1; $i <= 3000; $i++) { + $time = 1522762088 + $i * 60; + $userIndex = array_search($user, $totalUsers, true); + if (($userIndex+1) === count($totalUsers)) { + break; + } + $shareWithUser = $totalUsers[$userIndex+1]; + $uidOwner = $user; + $itemSource = $i; + if ($userIndex === 0) { + $totalParents++; + $qb->insert('share') + ->values([ + 'id' => $qb->expr()->literal((string)$rowCount), + 'share_type' => $qb->expr()->literal('0'), + 'share_with' => $qb->expr()->literal($shareWithUser), + 'uid_owner' => $qb->expr()->literal($uidOwner), + 'item_type' => $qb->expr()->literal('folder'), + 'item_source' => $qb->expr()->literal($itemSource), + 'file_source' => $qb->expr()->literal($itemSource), + 'file_target' => $qb->expr()->literal('/' . $itemSource), + 'permissions' => $qb->expr()->literal(31), + 'stime' => $qb->expr()->literal($time), + ]) + ->execute(); + } else { + if (($firstIdSet === false) && ($i === 1)) { + $getId = $this->connection->getQueryBuilder(); + $grabIds = $getId->select('id') + ->from('share') + ->execute()->fetchAll(); + $parentReshareCount = $grabIds[0]['id']; + $pareReshareCountRest = $parentReshareCount; + $firstIdSet = true; + } + + $parent = $parentReshareCount; + $itemSource = $parentReshareCount; + $parentReshareCount++; + $qb->insert('share') + ->values([ + 'id' => $qb->expr()->literal((string)$rowCount), + 'share_type' => $qb->expr()->literal('0'), + 'share_with' => $qb->expr()->literal($shareWithUser), + 'uid_owner' => $qb->expr()->literal($uidOwner), + 'parent' => $qb->expr()->literal($parent), + 'item_type' => $qb->expr()->literal('folder'), + 'item_source' => $qb->expr()->literal($itemSource), + 'file_source' => $qb->expr()->literal($itemSource), + 'file_target' => $qb->expr()->literal('/' . $itemSource), + 'permissions' => $qb->expr()->literal(31), + 'stime' => $qb->expr()->literal($time), + ]) + ->execute(); + if (($parentReshareCount - $pareReshareCountRest) >= 3000) { + //Reset the parent count + $parentReshareCount = $pareReshareCountRest; + } + } + $rowCount++; + } + } + + //Now lets randomly delete 100 folders of admin to create orphan shares + //We would remove 20 id's from oc_share + $rowId = 2500; + $randomRow = 1; + while($rowId > 0) { + $qb = $this->connection->getQueryBuilder(); + //Check if the row is there before deleting + $result = $qb->select('id') + ->from('share') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($randomRow + $pareReshareCountRest))) + ->execute()->fetchAll(); + if (count($result) === 0) { + continue; + } + $qb->delete('share') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($randomRow + $pareReshareCountRest))) + ->execute(); + $rowId--; + $randomRow++; + } + + //Now run the repair step and prove that this test fails + $this->repair = new RepairOrphanedSubshare($this->connection); + $outputMock = $this->createMock(IOutput::class); + $this->repair->run($outputMock); + + $qb = $this->connection->getQueryBuilder(); + $result = 0; + $statement = $qb->select('parent') + ->from('share') + ->groupBy('parent')->orderBy('parent'); + $results = $statement->execute()->fetchAll(); + $result += count($results); + $this->assertEquals($totalParents - 2500, $result); + } + + /** + * This test comprises of large rows i.e, 12000 rows. + * This test will have more orphan shares and will check if the repair + * test is working properly. + */ + public function testOrphanSharesFailsWithPagination() { + $qb = $this->connection->getQueryBuilder(); + $totalUsers[] = 'admin'; + //Create 4 users. admin, user1, user2, user3, user4 + $user = 'user'; + for($i=1; $i <= 4; $i++) { + $this->createUser($user.$i); + $totalUsers[] = $user.$i; + } + + //Lets create 3000 entries in oc_share to share + //The idea here is 100 folders of admin are shared with + //30 other users. So each folders are getting re-shared with others + $totalParents = 1; + $parentReshareCount = 1; + $pareReshareCountRest = 0; + $rowCount = 1; + $firstIdSet = false; + foreach ($totalUsers as $user) { + for($i=1; $i <= 3000; $i++) { + $time = 1522762088 + $i * 60; + $userIndex = array_search($user, $totalUsers, true); + if (($userIndex+1) === count($totalUsers)) { + break; + } + $shareWithUser = $totalUsers[$userIndex+1]; + $uidOwner = $user; + $itemSource = $i; + if ($userIndex === 0) { + $totalParents++; + $qb->insert('share') + ->values([ + 'id' => $qb->expr()->literal((string)$rowCount), + 'share_type' => $qb->expr()->literal('0'), + 'share_with' => $qb->expr()->literal($shareWithUser), + 'uid_owner' => $qb->expr()->literal($uidOwner), + 'item_type' => $qb->expr()->literal('folder'), + 'item_source' => $qb->expr()->literal($itemSource), + 'file_source' => $qb->expr()->literal($itemSource), + 'file_target' => $qb->expr()->literal('/' . $itemSource), + 'permissions' => $qb->expr()->literal(31), + 'stime' => $qb->expr()->literal($time), + ]) + ->execute(); + } else { + if (($firstIdSet === false) && ($i === 1)) { + $getId = $this->connection->getQueryBuilder(); + $grabIds = $getId->select('id') + ->from('share') + ->execute()->fetchAll(); + $parentReshareCount = $grabIds[0]['id']; + $pareReshareCountRest = $parentReshareCount; + $firstIdSet = true; + } + + $parent = $parentReshareCount; + $itemSource = $parentReshareCount; + $parentReshareCount++; + $qb->insert('share') + ->values([ + 'id' => $qb->expr()->literal((string)$rowCount), + 'share_type' => $qb->expr()->literal('0'), + 'share_with' => $qb->expr()->literal($shareWithUser), + 'uid_owner' => $qb->expr()->literal($uidOwner), + 'parent' => $qb->expr()->literal($parent), + 'item_type' => $qb->expr()->literal('folder'), + 'item_source' => $qb->expr()->literal($itemSource), + 'file_source' => $qb->expr()->literal($itemSource), + 'file_target' => $qb->expr()->literal('/' . $itemSource), + 'permissions' => $qb->expr()->literal(31), + 'stime' => $qb->expr()->literal($time), + ]) + ->execute(); + if (($parentReshareCount - $pareReshareCountRest) >= 3000) { + //Reset the parent count + $parentReshareCount = $pareReshareCountRest; + } + } + $rowCount++; + } + } + + //Now lets randomly delete 100 folders of admin to create orphan shares + //We would remove 20 id's from oc_share + $rowId = 2500; + $randomRow = 1; + while($rowId > 0) { + //$randomRow = (string)mt_rand($pareReshareCountRest,$pareReshareCountRest + 100); + $qb = $this->connection->getQueryBuilder(); + //Check if the row is there before deleting + $result = $qb->select('id') + ->from('share') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($randomRow + $pareReshareCountRest))) + ->execute()->fetchAll(); + if (count($result) === 0) { + continue; + } + $qb->delete('share') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($randomRow + $pareReshareCountRest))) + ->execute(); + $rowId--; + $randomRow++; + } + + //Now run the repair step and prove that this test fails + $this->mockRepairOrphanShareMethodWithPagination(); + + $qb = $this->connection->getQueryBuilder(); + $pageLimit = 1000; + $offset = 0; + $result = 0; + do { + $statement = $qb->select('parent') + ->from('share') + ->groupBy('parent')->orderBy('parent')->setMaxResults($pageLimit)->setFirstResult($offset)->execute(); + $results = $statement->fetchAll(); + $offset += $pageLimit; + $result += count($results); + } while(count($results) > 0); + $testFail = false; + try { + $this->assertEquals($totalParents - 2500, $result); + } catch (\PHPUnit_Framework_AssertionFailedError $e) { + $testFail = true; + $outputMock = $this->createMock(IOutput::class); + $this->repair->run($outputMock); + + $qb = $this->connection->getQueryBuilder(); + $result = 0; + $statement = $qb->select('parent') + ->from('share') + ->groupBy('parent')->orderBy('parent'); + $results = $statement->execute()->fetchAll(); + $result += count($results); + $this->assertEquals($totalParents - 2500, $result); + } + + //The value of $testFail is set true when exception is caught. + $this->assertTrue($testFail); + } + + public function mockRepairOrphanShareMethodWithPagination() { + + //This delete query deletes orphan shares whose parents are missing + $deleteOrphanReshares = $this->connection->getQueryBuilder(); + $deleteOrphanReshares + ->delete('share') + ->where( + $deleteOrphanReshares->expr()->eq('parent', + $deleteOrphanReshares->createParameter('parentId'))); + + + //Set the query to get the parent id's missing from the table. + $missingParents = $this->connection->getQueryBuilder(); + $qb = $this->connection->getQueryBuilder(); + + $qb->select('id') + ->from('share'); + $missingParents->select('parent') + ->from('share'); + $missingParents->where($missingParents->expr()->isNotNull('parent')); + $missingParents->andWhere($missingParents->expr()->notIn('parent', $missingParents->createFunction($qb->getSQL()))); + $missingParents->groupBy('parent'); + + + $pageLimit = 1000; + $paginationOffset = 0; + + /** A one time call to get missing parents from oc_share table */ + do { + $missingParents->setMaxResults($pageLimit); + $missingParents->setFirstResult($paginationOffset); + $results = $missingParents->execute(); + $rows = $results->fetchAll(); + $results->closeCursor(); + if (count($rows) > 0) { + $paginationOffset += $pageLimit; + foreach ($rows as $row) { + $deleteOrphanReshares->setParameter('parentId', $row['parent'])->execute(); + } + } + + } while (empty($rows)); + } }