Skip to content

Commit

Permalink
Remove pagination from the repair step
Browse files Browse the repository at this point in the history 
The pagination is not required as the
rows are deleted during the operation.

Signed-off-by: Sujith H <sharidasan@owncloud.com>
  • Loading branch information
@sharidas
sharidas committed Apr 4, 2018
1 parent 6e2b8aa commit 63ab8a4
Show file tree
Hide file tree
Showing 2 changed files with 305 additions and 8 deletions.
16 changes: 8 additions & 8 deletions lib/private/Repair/RepairOrphanedSubshare.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,17 @@ class RepairOrphanedSubshare implements IRepairStep {
/** @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->pageLimit = $pageLimit;

//This delete query deletes orphan shares whose parents are missing
$this->deleteOrphanReshares = $this->connection->getQueryBuilder();
Expand Down Expand Up @@ -84,24 +88,20 @@ public function getName() {
* @throws \Exception in case of failure
*/
public function run(IOutput $output) {
$pageLimit = 1000;
$paginationOffset = 0;
$deleteParents = [];
//$pageLimit = 1000;

/** 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;
foreach ($rows as $row) {
$this->deleteOrphanReshares->setParameter('parentId', $row['parent'])->execute();
}
}

} while (empty($rows));
} while (!empty($rows));
}
}
297 changes: 297 additions & 0 deletions tests/lib/Repair/RepairOrphanedSubshareTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,4 +286,301 @@ public function testLargeSharesWithOrphans() {
} while(count($results) > 0);
$this->assertEquals($totalParents - 20, $result);
}

/**
* Positive test
*
* Rows created for test is 9000
*/
public function testOrphanSharesFailsWithoutPagination() {
$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 <= 30; $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;
echo "\nstarts from $pareReshareCountRest\n";
$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) >= 30) {
//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 = 15;
$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, 5);
$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);
echo "\nfinal results\n";
var_dump($results);
$this->assertEquals($totalParents - 15, $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);
$this->assertNotEquals($totalParents - 2500, $result);
}

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));
}
}

0 comments on commit 63ab8a4

Please sign in to comment.