From ee0453911a8c9397bcbb2762abd5c172da6021c7 Mon Sep 17 00:00:00 2001
From: Vincent Petry <pvince81@owncloud.com>
Date: Fri, 15 Mar 2019 10:00:20 +0100
Subject: [PATCH] Use upsert for subshare updates

To avoid race conditions happening exactly between the select and
insert/update.
---
 lib/private/Share20/DefaultShareProvider.php  | 73 +++++++------------
 .../lib/Share20/DefaultShareProviderTest.php  |  2 +-
 2 files changed, 26 insertions(+), 49 deletions(-)

diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php
index ce88747f194b..5c9ac03a52e2 100644
--- a/lib/private/Share20/DefaultShareProvider.php
+++ b/lib/private/Share20/DefaultShareProvider.php
@@ -395,59 +395,36 @@ public function updateForRecipient(\OCP\Share\IShare $share, $recipient) {
 				throw new ProviderException('Recipient not in receiving group');
 			}
 
-			// Check if there is a usergroup share
-			$qb = $this->dbConn->getQueryBuilder();
-			$stmt = $qb->select('id')
-				->from('share')
-				->where($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP)))
-				->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($recipient)))
-				->andWhere($qb->expr()->eq('parent', $qb->createNamedParameter($share->getId())))
-				->andWhere($qb->expr()->orX(
-					$qb->expr()->eq('item_type', $qb->createNamedParameter('file')),
-					$qb->expr()->eq('item_type', $qb->createNamedParameter('folder'))
-				))
-				->setMaxResults(1)
-				->execute();
-
-			$data = $stmt->fetch();
-			$stmt->closeCursor();
+			$node = $share->getNode();
 
 			$shareAttributes = $this->formatShareAttributes(
 				$share->getAttributes()
 			);
 
-			if ($data === false) {
-				// No usergroup share yet. Create one.
-				$qb = $this->dbConn->getQueryBuilder();
-				$qb->insert('share')
-					->values([
-						'share_type' => $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP),
-						'share_with' => $qb->createNamedParameter($recipient),
-						'uid_owner' => $qb->createNamedParameter($share->getShareOwner()),
-						'uid_initiator' => $qb->createNamedParameter($share->getSharedBy()),
-						'parent' => $qb->createNamedParameter($share->getId()),
-						'item_type' => $qb->createNamedParameter($share->getNode() instanceof File ? 'file' : 'folder'),
-						'item_source' => $qb->createNamedParameter($share->getNode()->getId()),
-						'file_source' => $qb->createNamedParameter($share->getNode()->getId()),
-						'file_target' => $qb->createNamedParameter($share->getTarget()),
-						'permissions' => $qb->createNamedParameter($share->getPermissions()),
-						'attributes' => $qb->createNamedParameter($shareAttributes),
-						'stime' => $qb->createNamedParameter($share->getShareTime()->getTimestamp()),
-						'accepted' => $qb->createNamedParameter($share->getState()),
-					])->execute();
-			} else {
-				// Already a usergroup share. Update it.
-				$qb = $this->dbConn->getQueryBuilder();
-				$qb->update('share')
-					->set('accepted', $qb->createNamedParameter($share->getState()))
-					->set('file_target', $qb->createNamedParameter($share->getTarget()))
-					// make sure to reset the permissions to the one of the parent share,
-					// as legacy entries with zero permissions might still exist
-					->set('permissions', $qb->createNamedParameter($share->getPermissions()))
-					->set('attributes', $qb->createNamedParameter($shareAttributes))
-					->where($qb->expr()->eq('id', $qb->createNamedParameter($data['id'])))
-					->execute();
-			}
+			// Check if there is a usergroup share
+			$this->dbConn->upsert(
+				'*PREFIX*share',
+				[
+					'share_type' => self::SHARE_TYPE_USERGROUP,
+					'share_with' => $recipient,
+					'parent' => $share->getId(),
+					'uid_owner' => $share->getShareOwner(),
+					'uid_initiator' => $share->getSharedBy(),
+					'item_type' => $node instanceof File ? 'file' : 'folder',
+					'item_source' => $node->getId(),
+					'file_source' => $node->getId(),
+					'file_target' => $share->getTarget(),
+					'attributes' => $shareAttributes,
+					'permissions' => $share->getPermissions(),
+					'stime' => $share->getShareTime()->getTimestamp(),
+					'accepted' => $share->getState(),
+				],
+				[
+					'share_type',
+					'share_with',
+					'parent'
+				]
+			);
 		} else {
 			throw new ProviderException('Can\'t update share of recipient for share type ' . $share->getShareType());
 		}
diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php
index 106d629060fa..d3afb2c57f45 100644
--- a/tests/lib/Share20/DefaultShareProviderTest.php
+++ b/tests/lib/Share20/DefaultShareProviderTest.php
@@ -2678,7 +2678,7 @@ public function testUpdateShareState($sourceState, $targetState) {
 		$folder = $this->createMock(Folder::class);
 		$folder->method('getId')->willReturn(42);
 
-		$this->rootFolder->method('getUserFolder')->with('user1')->will($this->returnSelf());
+		$this->rootFolder->method('getUserFolder')->with($this->logicalOr('user1', 'user2'))->will($this->returnSelf());
 		$this->rootFolder->method('getById')->willReturn([$folder]);
 
 		$share = $this->provider->getShareById($id, 'user0');