Fix sharing creation insert and get

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>

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