Skip to content

Commit

Permalink
Merge pull request #31146 from owncloud/fix-repair-subshare-stable10
Browse files Browse the repository at this point in the history 
[stable10] Update the sql query to delete duplicate sub share
  • Loading branch information
Vincent Petry authored Jul 18, 2018
2 parents 7159f98 + b1b93a2 commit 42fa1b5
Show file tree
Hide file tree
Showing 2 changed files with 198 additions and 54 deletions.
82 changes: 51 additions & 31 deletions lib/private/Repair/RepairSubShares.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,55 +48,75 @@ public function getName() {
}

/**
* Set query to remove duplicate rows.
* i.e, except id all columns are same for oc_share
* Also set query to select rows which have duplicate rows of share.
* Get query builder to select rows which have duplicate rows of share
* @return IQueryBuilder
*/
private function setRemoveAndSelectQuery() {
private function getSelectQueryToDetectDuplicatesBuilder() {
/**
* Retrieves the duplicate rows with different id's
* of oc_share
* of oc_share. The query would look like:
* SELECT id
* FROM oc_share
* WHERE id NOT IN (
* SELECT MIN(id)
* FROM oc_share
* GROUP BY share_with, parent
* )
* AND share_type=2;
*/
$builder = $this->connection->getQueryBuilder();
$builder
->select('id', 'parent', $builder->createFunction('count(*)'))
$subQuery = $this->connection->getQueryBuilder();
$subQuery
->select($subQuery->createFunction('MIN(`id`)'))
->from('share')
->where($builder->expr()->eq('share_type', $builder->createNamedParameter(2)))
->groupBy('parent')
->addGroupBy('id')
->addGroupBy('share_with')
->having('count(*) > 1')->setMaxResults(1000);
->groupBy('share_with')
->addGroupBy('parent');

$this->getDuplicateRows = $builder;
$builder->select('id')
->from('share')
->where($builder->expr()->notIn('id', $builder->createFunction($subQuery->getSQL())))
->andWhere($builder->expr()->eq('share_type', $builder->createNamedParameter(2)));
return $builder;
}

/**
* Get query builder to delete rows which have duplicate rows of share based
* on ids
* @return IQueryBuilder
*/
private function getDeleteShareIdsBuilder() {
$builder = $this->connection->getQueryBuilder();
$builder
->delete('share')
->where($builder->expr()->eq('id', $builder->createParameter('shareId')));

$this->deleteShareId = $builder;
->where($builder->expr()->in('id', $builder->createParameter('shareIds')));
return $builder;
}

public function run(IOutput $output) {
$deletedEntries = 0;
$this->setRemoveAndSelectQuery();

/**
* Going for pagination because if there are 1 million rows
* it wont be easy to scale the data
*/
do {
$results = $this->getDuplicateRows->execute();
$rows = $results->fetchAll();
$results->closeCursor();
$lastResultCount = 0;
$selectDuplicates = $this->getSelectQueryToDetectDuplicatesBuilder();

$results = $selectDuplicates->execute();
$rows = $results->fetchAll();
$results->closeCursor();
$rowIds = [];
if (\count($rows) > 0) {
$rowIds = \array_map(
function ($value) {
return (int)$value['id'];
},
$rows
);
}

foreach ($rows as $row) {
$deletedEntries += $this->deleteShareId->setParameter('shareId', (int) $row['id'])
if (\count($rows) > 0) {
//Delete in a batch of 1000 ids
$deleteShareIds = $this->getDeleteShareIdsBuilder();
foreach (\array_chunk($rowIds, 1000) as $getIds) {
$deletedEntries += $deleteShareIds->setParameter('shareIds', $getIds, IQueryBuilder::PARAM_INT_ARRAY)
->execute();
$lastResultCount++;
}
} while($lastResultCount > 0);
}

if ($deletedEntries > 0) {
$output->info('Removed ' . $deletedEntries . ' shares where duplicate rows where found');
Expand Down
170 changes: 147 additions & 23 deletions tests/lib/Repair/RepairSubSharesTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,87 @@ public function deleteAllShares() {
$qb->delete('share')->execute();
}

/**
* A test case to verify steps repair does address the simple step mentioned
* at https://github.com/owncloud/core/issues/27990#issuecomment-357899190
*/

public function testRemoveDuplicateSubShare() {
$qb = $this->connection->getQueryBuilder();
/*
* Create 3 users: user1, user2 and user3.
*/
$userName = "user";
$groupName = "group1";
$time = 1523892;
//This array holds the id, share_with and parent per user
$getAllIdsPerUser = [];
\OC::$server->getGroupManager()->createGroup($groupName);
for ($userCount = 1; $userCount <= 3; $userCount++) {
$user = $this->createUser($userName.$userCount);
\OC::$server->getGroupManager()->get($groupName)->addUser($user);
}

$usersinsharetable = ['admin', 'user1', 'user2', 'user2'];
foreach ($usersinsharetable as $user) {
//start with a simple insert query and alter later based on users.
$inserValues = [
'uid_owner' => $qb->expr()->literal('admin'),
'uid_initiator' => $qb->expr()->literal('admin'),
'item_type' => $qb->expr()->literal('folder'),
'item_source' => $qb->expr()->literal(23),
'file_source' => $qb->expr()->literal(23),
'file_target' => $qb->expr()->literal('/test'),
'permissions' => $qb->expr()->literal(31),
'stime' => $qb->expr()->literal($time),
'mail_send' => $qb->expr()->literal('0'),
'accepted' => $qb->expr()->literal('0')
];

$userIndex = \array_search($user, $usersinsharetable, true);
if ($userIndex === 0) {
$inserValues['share_type'] = $qb->expr()->literal(1);
$inserValues['share_with'] = $qb->expr()->literal($groupName);
$user = $groupName;
} else {
$inserValues['share_type'] = $qb->expr()->literal(2);
$inserValues['share_with'] = $qb->expr()->literal($user);
$inserValues['parent'] = $qb->expr()->literal(1);
}

$qb->insert('share')
->values($inserValues)
->execute();
$getAllIdsPerUser[$user][] = ['id' => $this->getLastSharedId(), 'share_with' => $user];
}

$outputMock = $this->createMock(IOutput::class);
$this->repair->run($outputMock);

$qb = $this->invokePrivate($this->repair, 'getSelectQueryToDetectDuplicatesBuilder', []);
$results = $qb->execute()->fetchAll();
$this->assertCount(0, $results);

//There should be only one entry for share_with as 'user2'
$qb = $this->connection->getQueryBuilder();
$qb->select($qb->createFunction('count(*)'))
->from('share')
->where($qb->expr()->eq('share_with', $qb->createNamedParameter('user2')));
$results = $qb->execute()->fetchAll();
$this->assertCount(1, $results);

$qb = $this->connection->getQueryBuilder();
$qb->select('id', 'share_with')
->from('share');
$results = $qb->execute()->fetchAll();
//Only 3 entries should be there
$this->assertCount(3, $results);

foreach ($results as $id) {
$this->assertTrue(\in_array($id, $getAllIdsPerUser[$id['share_with']]));
}
}

/**
* This is a very basic test
* This test would populate DB with data
Expand All @@ -83,12 +164,14 @@ public function testPopulateDBAndRemoveDuplicates() {
$userName = "user";
$groupName = "group";
$folderName = "/test";
$time = time();
$time = 1523892;
$groupCount = 1;
$totalGroups = 3;
$parent = 1;
//This array holds the id, share_with and parent per user
$getAllIdsPerUser = [];
$multipleOf = 2;
for($userCount = 1; $userCount <= 10; $userCount++) {
for ($userCount = 1; $userCount <= 10; $userCount++) {
$user = $this->createUser($userName.$userCount);
if (\OC::$server->getGroupManager()->groupExists($groupName.$groupCount) === false) {
\OC::$server->getGroupManager()->createGroup($groupName.$groupCount);
Expand All @@ -98,7 +181,7 @@ public function testPopulateDBAndRemoveDuplicates() {
//Create a group share
$qb->insert('share')
->values([
'share_type' => $qb->expr()->literal('2'),
'share_type' => $qb->expr()->literal(2),
'share_with' => $qb->expr()->literal($userName.$groupCount),
'uid_owner' => $qb->expr()->literal('admin'),
'uid_initiator' => $qb->expr()->literal('admin'),
Expand All @@ -111,13 +194,14 @@ public function testPopulateDBAndRemoveDuplicates() {
'stime' => $qb->expr()->literal($time),
])
->execute();
$getAllIdsPerUser['ids'][] = ['id' => $this->getLastSharedId()];

/**
* Group count incremented once value of userCount reaches multiple of 3
*/
if (($userCount % $totalGroups) === 0) {
$groupCount++;
$time = time();
$time += 1;
}

/**
Expand All @@ -128,20 +212,30 @@ public function testPopulateDBAndRemoveDuplicates() {
}
}

$qb = $this->invokePrivate($this->repair, 'getSelectQueryToDetectDuplicatesBuilder', []);
$idsNotPresent = $qb->execute()->fetchAll();

$outputMock = $this->createMock(IOutput::class);
$this->repair->run($outputMock);

$qb = $this->connection->getQueryBuilder();
$qb->select('id', 'parent', $qb->createFunction('count(*)'))
->from('share')
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(2)))
->groupBy('parent')
->addGroupBy('id')
->addGroupBy('share_with')
->having('count(*) > 1')->setMaxResults(1000);

$results = $qb->execute()->fetchAll();
$this->assertCount(0, $results);

$qb = $this->connection->getQueryBuilder();
$qb->select('id')
->from('share');
$results = $qb->execute()->fetchAll();
//Only 7 entries should be there
$this->assertCount(7, $results);

foreach ($results as $id) {
$this->assertTrue(\in_array($id, $getAllIdsPerUser['ids']));
}

//Verify that these ids are not there
foreach ($results as $id) {
$this->assertFalse(\in_array($id['id'], $idsNotPresent, true));
}
}

/**
Expand All @@ -151,11 +245,13 @@ public function testPopulateDBAndRemoveDuplicates() {
public function testLargeDuplicateShareRows() {
$qb = $this->connection->getQueryBuilder();
$userName = "user";
$time = time();
$time = 15238923;
$groupCount = 0;
$folderName = "/test";
$maxUsersPerGroup = 1000;
$parent = $groupCount + 1;
//This array holds the id, share_with and parent per user
$getAllIdsPerUser = [];
for ($userCount = 0; $userCount < 5500; $userCount++) {
/**
* groupCount is incremented once userCount reaches
Expand All @@ -167,7 +263,7 @@ public function testLargeDuplicateShareRows() {
}
$qb->insert('share')
->values([
'share_type' => $qb->expr()->literal('2'),
'share_type' => $qb->expr()->literal(2),
'share_with' => $qb->expr()->literal($userName.$groupCount),
'uid_owner' => $qb->expr()->literal('admin'),
'uid_initiator' => $qb->expr()->literal('admin'),
Expand All @@ -180,21 +276,49 @@ public function testLargeDuplicateShareRows() {
'stime' => $qb->expr()->literal($time),
])
->execute();
$getAllIdsPerUser['ids'][] = [
'id' => $this->getLastSharedId()];
}

$outputMock = $this->createMock(IOutput::class);
$this->repair->run($outputMock);

$qb = $this->connection->getQueryBuilder();
$qb->select('id', 'parent', $qb->createFunction('count(*)'))
->from('share')
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(2)))
->groupBy('parent')
->addGroupBy('id')
->addGroupBy('share_with')
->having('count(*) > 1')->setMaxResults(1000);
$qb = $this->invokePrivate($this->repair, 'getSelectQueryToDetectDuplicatesBuilder', []);

$results = $qb->execute()->fetchAll();
$this->assertCount(0, $results);

$qb = $this->connection->getQueryBuilder();
$qb->select('id')
->from('share');
$results = $qb->execute()->fetchAll();

$this->assertCount(6, $results);

foreach ($results as $id) {
$this->assertTrue(\in_array($id, $getAllIdsPerUser['ids']));
if (\array_search($id, $results, true) === 5) {
for ($i = $id['id'] + 1; $i < $id['id'] + 486; $i++) {
$this->assertFalse(\in_array(['id' => $i], $results));
}
} else {
//The next 1000 ids will not be there.
for ($i = $id['id'] + 1; $i < $id['id'] + 999; $i++) {
$this->assertFalse(\in_array(['id' => $i], $results));
}
}
}
}

public function testName() {
$this->assertEquals('Repair sub shares', $this->repair->getName());
}

/**
* Returns last inserted id to the oc_share table
* @return int
*/
protected function getLastSharedId() {
return $this->connection->lastInsertId('*PREFIX*share');
}
}

0 comments on commit 42fa1b5

Please sign in to comment.