From ac300dd97dcbf34d414c9f7de6f19c17b3c818ef Mon Sep 17 00:00:00 2001 From: Sujith H Date: Mon, 16 Apr 2018 21:44:03 +0530 Subject: [PATCH] Update the sql query to delete duplicate sub share Removal of 'group by id' is required to achieve the purpose of the repair script. If this is not removed, then the deletion was not happening. Signed-off-by: Sujith H --- lib/private/Repair/RepairSubShares.php | 1 - tests/lib/Repair/RepairSubSharesTest.php | 125 ++++++++++++++++++++++- 2 files changed, 120 insertions(+), 6 deletions(-) diff --git a/lib/private/Repair/RepairSubShares.php b/lib/private/Repair/RepairSubShares.php index 8d77290f83ff..fbbbdc15d781 100644 --- a/lib/private/Repair/RepairSubShares.php +++ b/lib/private/Repair/RepairSubShares.php @@ -63,7 +63,6 @@ private function setRemoveAndSelectQuery() { ->from('share') ->where($builder->expr()->eq('share_type', $builder->createNamedParameter(2))) ->groupBy('parent') - ->addGroupBy('id') ->addGroupBy('share_with') ->having('count(*) > 1')->setMaxResults(1000); diff --git a/tests/lib/Repair/RepairSubSharesTest.php b/tests/lib/Repair/RepairSubSharesTest.php index 222589fc0a82..abe5096a82a6 100644 --- a/tests/lib/Repair/RepairSubSharesTest.php +++ b/tests/lib/Repair/RepairSubSharesTest.php @@ -69,6 +69,123 @@ 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; + \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']; + foreach ($usersinsharetable as $user) { + if ($user === 'admin') { + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal('1'), + 'share_with' => $qb->expr()->literal($groupName), + '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') + ]) + ->execute(); + } elseif ($user === 'user1') { + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal('2'), + 'share_with' => $qb->expr()->literal($user), + 'uid_owner' => $qb->expr()->literal('admin'), + 'uid_initiator' => $qb->expr()->literal('admin'), + 'parent' => $qb->expr()->literal('1'), + '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') + ]) + ->execute(); + } else { + for ($i = 0; $i < 2; $i++) { + $qb->insert('share') + ->values([ + 'share_type' => $qb->expr()->literal('2'), + 'share_with' => $qb->expr()->literal($user), + 'uid_owner' => $qb->expr()->literal('admin'), + 'uid_initiator' => $qb->expr()->literal('admin'), + 'parent' => $qb->expr()->literal('1'), + 'item_type' => $qb->expr()->literal('folder'), + 'item_source' => $qb->expr()->literal(23), + 'file_source' => $qb->expr()->literal(23), + 'file_target' => $qb->expr()->literal('/test_mine'), + 'permissions' => $qb->expr()->literal(31), + 'stime' => $qb->expr()->literal($time), + 'mail_send' => $qb->expr()->literal('0'), + 'accepted' => $qb->expr()->literal('0') + ]) + ->execute(); + } + } + } + + $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('share_with') + ->having('count(*) > 1')->setMaxResults(1000); + + $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'))) + ->setMaxResults(1000); + $results = $qb->execute()->fetchAll(); + $this->assertCount(1, $results); + $qb = $this->connection->getQueryBuilder(); + $qb->select('share_with') + ->from('share'); + $results = $qb->execute()->fetchAll(); + //Only 3 entries should be there + $this->assertCount(3, $results); + $userArray = [ + array('share_with' => 'group1'), + array('share_with' => 'user1'), + array('share_with' => 'user2') + ]; + foreach ($userArray as $sharewith) { + $this->assertTrue(in_array($sharewith, $results)); + } + } + /** * This is a very basic test * This test would populate DB with data @@ -83,7 +200,7 @@ public function testPopulateDBAndRemoveDuplicates() { $userName = "user"; $groupName = "group"; $folderName = "/test"; - $time = time(); + $time = 1523892; $groupCount = 1; $totalGroups = 3; $parent = 1; @@ -117,7 +234,7 @@ public function testPopulateDBAndRemoveDuplicates() { */ if (($userCount % $totalGroups) === 0) { $groupCount++; - $time = time(); + $time += 1; } /** @@ -136,7 +253,6 @@ public function testPopulateDBAndRemoveDuplicates() { ->from('share') ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(2))) ->groupBy('parent') - ->addGroupBy('id') ->addGroupBy('share_with') ->having('count(*) > 1')->setMaxResults(1000); @@ -151,7 +267,7 @@ public function testPopulateDBAndRemoveDuplicates() { public function testLargeDuplicateShareRows() { $qb = $this->connection->getQueryBuilder(); $userName = "user"; - $time = time(); + $time = 15238923; $groupCount = 0; $folderName = "/test"; $maxUsersPerGroup = 1000; @@ -190,7 +306,6 @@ public function testLargeDuplicateShareRows() { ->from('share') ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(2))) ->groupBy('parent') - ->addGroupBy('id') ->addGroupBy('share_with') ->having('count(*) > 1')->setMaxResults(1000);