From f828dce5972ee633129b97aa267870206a8057b8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 23 Apr 2021 09:12:12 +0200 Subject: [PATCH] Fix sharing creation insert and get MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- apps/files_sharing/tests/ApiTest.php | 5 +- lib/private/Share20/DefaultShareProvider.php | 54 ++++++++----------- lib/private/Share20/ProviderFactory.php | 3 +- tests/lib/Files/ViewTest.php | 2 - .../lib/Share20/DefaultShareProviderTest.php | 33 ++++++------ 5 files changed, 42 insertions(+), 55 deletions(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index d7661297e9e..61a63ad1e19 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -959,10 +959,7 @@ class ApiTest extends TestCase { $this->assertNotNull($share1->getAttributes()); $share1 = $this->shareManager->createShare($share1); - $this->assertNull($share1->getAttributes()); $this->assertEquals(19, $share1->getPermissions()); - // attributes get cleared when empty - $this->assertNull($share1->getAttributes()); $share2 = $this->shareManager->newShare(); $share2->setNode($node1) @@ -1126,7 +1123,7 @@ class ApiTest extends TestCase { ->setSharedBy(self::TEST_FILES_SHARING_API_USER1) ->setShareType(IShare::TYPE_LINK) ->setPermissions(1); - $share2 = $this->shareManager->createShare($share1); + $share2 = $this->shareManager->createShare($share2); $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); $ocs->deleteShare($share1->getId()); diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 5201cf074b1..3f5d01618eb 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -38,12 +38,12 @@ use OC\Files\Cache\Cache; use OC\Share20\Exception\BackendError; use OC\Share20\Exception\InvalidShare; use OC\Share20\Exception\ProviderException; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Defaults; use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\Node; -use OCP\IConfig; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IURLGenerator; @@ -90,19 +90,19 @@ class DefaultShareProvider implements IShareProvider { /** @var IURLGenerator */ private $urlGenerator; - /** @var IConfig */ - private $config; + private ITimeFactory $timeFactory; public function __construct( - IDBConnection $connection, - IUserManager $userManager, - IGroupManager $groupManager, - IRootFolder $rootFolder, - IMailer $mailer, - Defaults $defaults, - IFactory $l10nFactory, - IURLGenerator $urlGenerator, - IConfig $config) { + IDBConnection $connection, + IUserManager $userManager, + IGroupManager $groupManager, + IRootFolder $rootFolder, + IMailer $mailer, + Defaults $defaults, + IFactory $l10nFactory, + IURLGenerator $urlGenerator, + ITimeFactory $timeFactory, + ) { $this->dbConn = $connection; $this->userManager = $userManager; $this->groupManager = $groupManager; @@ -111,7 +111,7 @@ class DefaultShareProvider implements IShareProvider { $this->defaults = $defaults; $this->l10nFactory = $l10nFactory; $this->urlGenerator = $urlGenerator; - $this->config = $config; + $this->timeFactory = $timeFactory; } /** @@ -216,32 +216,22 @@ class DefaultShareProvider implements IShareProvider { } // Set the time this share was created - $qb->setValue('stime', $qb->createNamedParameter(time())); + $shareTime = $this->timeFactory->now(); + $qb->setValue('stime', $qb->createNamedParameter($shareTime->getTimestamp())); // insert the data and fetch the id of the share - $this->dbConn->beginTransaction(); - $qb->execute(); - $id = $this->dbConn->lastInsertId('*PREFIX*share'); - - // Now fetch the inserted share and create a complete share object - $qb = $this->dbConn->getQueryBuilder(); - $qb->select('*') - ->from('share') - ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))); + $qb->executeStatement(); - $cursor = $qb->execute(); - $data = $cursor->fetch(); - $this->dbConn->commit(); - $cursor->closeCursor(); + // Update mandatory data + $id = $qb->getLastInsertId(); + $share->setId((string)$id); + $share->setProviderId($this->identifier()); - if ($data === false) { - throw new ShareNotFound('Newly created share could not be found'); - } + $share->setShareTime(\DateTime::createFromImmutable($shareTime)); $mailSendValue = $share->getMailSend(); - $data['mail_send'] = ($mailSendValue === null) ? true : $mailSendValue; + $share->setMailSend(($mailSendValue === null) ? true : $mailSendValue); - $share = $this->createShare($data); return $share; } diff --git a/lib/private/Share20/ProviderFactory.php b/lib/private/Share20/ProviderFactory.php index aaab5a3fd88..dbf1b21dabe 100644 --- a/lib/private/Share20/ProviderFactory.php +++ b/lib/private/Share20/ProviderFactory.php @@ -41,6 +41,7 @@ use OCA\FederatedFileSharing\TokenHandler; use OCA\ShareByMail\Settings\SettingsManager; use OCA\ShareByMail\ShareByMailProvider; use OCA\Talk\Share\RoomShareProvider; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\EventDispatcher\IEventDispatcher; use OCP\IServerContainer; @@ -104,7 +105,7 @@ class ProviderFactory implements IProviderFactory { $this->serverContainer->query(Defaults::class), $this->serverContainer->getL10NFactory(), $this->serverContainer->getURLGenerator(), - $this->serverContainer->getConfig() + $this->serverContainer->query(ITimeFactory::class), ); } diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index 2bf483df7d7..1c22541af92 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -1689,8 +1689,6 @@ class ViewTest extends \Test\TestCase { ->setSharedBy($this->user) ->setShareType(IShare::TYPE_USER) ->setPermissions(\OCP\Constants::PERMISSION_READ) - ->setId(42) - ->setProviderId('foo') ->setNode($shareDir); $shareManager->createShare($share); diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index 0a6f106a5db..0ce96102a30 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -24,12 +24,12 @@ namespace Test\Share20; use OC\Share20\DefaultShareProvider; use OC\Share20\ShareAttributes; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Defaults; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IRootFolder; -use OCP\IConfig; use OCP\IDBConnection; use OCP\IGroup; use OCP\IGroupManager; @@ -79,8 +79,8 @@ class DefaultShareProviderTest extends \Test\TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject|IURLGenerator */ protected $urlGenerator; - /** @var IConfig|MockObject */ - protected $config; + /** @var ITimeFactory|MockObject */ + protected $timeFactory; protected function setUp(): void { $this->dbConn = \OC::$server->getDatabaseConnection(); @@ -92,9 +92,10 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->l10n = $this->createMock(IL10N::class); $this->defaults = $this->getMockBuilder(Defaults::class)->disableOriginalConstructor()->getMock(); $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->config = $this->createMock(IConfig::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); $this->userManager->expects($this->any())->method('userExists')->willReturn(true); + $this->timeFactory->expects($this->any())->method('now')->willReturn(new \DateTimeImmutable("2023-05-04 00:00 Europe/Berlin")); //Empty share table $this->dbConn->getQueryBuilder()->delete('share')->execute(); @@ -108,7 +109,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->config + $this->timeFactory ); } @@ -469,7 +470,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->config + $this->timeFactory ]) ->setMethods(['getShareById']) ->getMock(); @@ -564,7 +565,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->config + $this->timeFactory ]) ->setMethods(['getShareById']) ->getMock(); @@ -724,11 +725,11 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertLessThanOrEqual(new \DateTime(), $share2->getShareTime()); $this->assertSame($path, $share2->getNode()); - // nothing from setSharedWithDisplayName/setSharedWithAvatar is saved in DB + // Data is kept after creation $this->assertSame('Displayed Name', $share->getSharedWithDisplayName()); $this->assertSame('/path/to/image.svg', $share->getSharedWithAvatar()); - $this->assertSame(null, $share2->getSharedWithDisplayName()); - $this->assertSame(null, $share2->getSharedWithAvatar()); + $this->assertSame('Displayed Name', $share2->getSharedWithDisplayName()); + $this->assertSame('/path/to/image.svg', $share2->getSharedWithAvatar()); $this->assertSame( [ @@ -794,11 +795,11 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertLessThanOrEqual(new \DateTime(), $share2->getShareTime()); $this->assertSame($path, $share2->getNode()); - // nothing from setSharedWithDisplayName/setSharedWithAvatar is saved in DB + // Data is kept after creation $this->assertSame('Displayed Name', $share->getSharedWithDisplayName()); $this->assertSame('/path/to/image.svg', $share->getSharedWithAvatar()); - $this->assertSame(null, $share2->getSharedWithDisplayName()); - $this->assertSame(null, $share2->getSharedWithAvatar()); + $this->assertSame('Displayed Name', $share2->getSharedWithDisplayName()); + $this->assertSame('/path/to/image.svg', $share2->getSharedWithAvatar()); $this->assertSame( [ @@ -2524,7 +2525,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->config + $this->timeFactory ); $password = md5(time()); @@ -2622,7 +2623,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->config + $this->timeFactory ); $u1 = $userManager->createUser('testShare1', 'test'); @@ -2718,7 +2719,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->defaults, $this->l10nFactory, $this->urlGenerator, - $this->config + $this->timeFactory ); $u1 = $userManager->createUser('testShare1', 'test'); -- 2.39.5