Skip to content

Commit 42ec6a5

Browse files
committed
perf(external-sharing): Port to Entity and SnowflakeId
This removes all the read after write and we don't need to queries all the time the same share in the same request anymore. Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
1 parent 16ffc29 commit 42ec6a5

File tree

19 files changed

+907
-850
lines changed

19 files changed

+907
-850
lines changed

apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414
use OCA\FederatedFileSharing\FederatedShareProvider;
1515
use OCA\Federation\TrustedServers;
1616
use OCA\Files_Sharing\Activity\Providers\RemoteShares;
17+
use OCA\Files_Sharing\External\ExternalShare;
1718
use OCA\Files_Sharing\External\Manager;
1819
use OCA\GlobalSiteSelector\Service\SlaveService;
20+
use OCA\Polls\Db\Share;
1921
use OCP\Activity\IManager as IActivityManager;
2022
use OCP\App\IAppManager;
2123
use OCP\AppFramework\QueryException;
@@ -44,6 +46,7 @@
4446
use OCP\Share\IManager;
4547
use OCP\Share\IProviderFactory;
4648
use OCP\Share\IShare;
49+
use OCP\Snowflake\IGenerator;
4750
use OCP\Util;
4851
use Psr\Log\LoggerInterface;
4952
use SensitiveParameter;
@@ -72,13 +75,11 @@ public function __construct(
7275
private IFilenameValidator $filenameValidator,
7376
private readonly IProviderFactory $shareProviderFactory,
7477
private readonly SetupManager $setupManager,
78+
private readonly IGenerator $snowflakeGenerator,
7579
) {
7680
}
7781

78-
/**
79-
* @return string
80-
*/
81-
public function getShareType() {
82+
public function getShareType(): string {
8283
return 'file';
8384
}
8485

@@ -93,7 +94,7 @@ public function getShareType() {
9394
* @throws HintException
9495
* @since 14.0.0
9596
*/
96-
public function shareReceived(ICloudFederationShare $share) {
97+
public function shareReceived(ICloudFederationShare $share): string {
9798
if (!$this->isS2SEnabled(true)) {
9899
throw new ProviderCouldNotAddShareException('Server does not support federated cloud sharing', '', Http::STATUS_SERVICE_UNAVAILABLE);
99100
}
@@ -159,9 +160,18 @@ public function shareReceived(ICloudFederationShare $share) {
159160
}
160161
}
161162

163+
$externalShare = new ExternalShare();
164+
$externalShare->setId($this->snowflakeGenerator->nextId());
165+
$externalShare->setRemote($remote);
166+
$externalShare->setRemoteId($remoteId);
167+
$externalShare->setShareToken($token);
168+
$externalShare->setPassword('');
169+
$externalShare->setName($name);
170+
$externalShare->setShareType($shareType);
171+
$externalShare->setAccepted(IShare::STATUS_PENDING);
172+
162173
try {
163-
$this->externalShareManager->addShare($remote, $token, '', $name, $owner, $shareType, false, $userOrGroup, $remoteId);
164-
$shareId = Server::get(IDBConnection::class)->lastInsertId('*PREFIX*share_external');
174+
$this->externalShareManager->addShare($externalShare, $userOrGroup);
165175

166176
// get DisplayName about the owner of the share
167177
$ownerDisplayName = $this->getUserDisplayName($ownerFederatedId);
@@ -184,14 +194,14 @@ public function shareReceived(ICloudFederationShare $share) {
184194
->setType('remote_share')
185195
->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_RECEIVED, [$ownerFederatedId, trim($name, '/'), $ownerDisplayName])
186196
->setAffectedUser($shareWith)
187-
->setObject('remote_share', $shareId, $name);
197+
->setObject('remote_share', $externalShare->getId(), $name);
188198
Server::get(IActivityManager::class)->publish($event);
189-
$this->notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName);
199+
$this->notifyAboutNewShare($shareWith, $externalShare->getId(), $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName);
190200

191201
// If auto-accept is enabled, accept the share
192202
if ($this->federatedShareProvider->isFederatedTrustedShareAutoAccept() && $trustedServers?->isTrustedServer($remote) === true) {
193203
/** @var IUser $userOrGroup */
194-
$this->externalShareManager->acceptShare($shareId, $userOrGroup);
204+
$this->externalShareManager->acceptShare($externalShare, $userOrGroup);
195205
}
196206
} else {
197207
/** @var IGroup $userOrGroup */
@@ -202,18 +212,18 @@ public function shareReceived(ICloudFederationShare $share) {
202212
->setType('remote_share')
203213
->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_RECEIVED, [$ownerFederatedId, trim($name, '/'), $ownerDisplayName])
204214
->setAffectedUser($user->getUID())
205-
->setObject('remote_share', $shareId, $name);
215+
->setObject('remote_share', $externalShare->getId(), $name);
206216
Server::get(IActivityManager::class)->publish($event);
207-
$this->notifyAboutNewShare($user->getUID(), $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName);
217+
$this->notifyAboutNewShare($user->getUID(), $externalShare->getId(), $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName);
208218

209219
// If auto-accept is enabled, accept the share
210220
if ($this->federatedShareProvider->isFederatedTrustedShareAutoAccept() && $trustedServers?->isTrustedServer($remote) === true) {
211-
$this->externalShareManager->acceptShare($shareId, $user);
221+
$this->externalShareManager->acceptShare($externalShare, $user);
212222
}
213223
}
214224
}
215225

216-
return $shareId;
226+
return $externalShare->getId();
217227
} catch (\Exception $e) {
218228
$this->logger->error('Server can not add remote share.', [
219229
'app' => 'files_sharing',
@@ -275,7 +285,7 @@ private function mapShareTypeToNextcloud($shareType) {
275285
return $result;
276286
}
277287

278-
private function notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $displayName): void {
288+
private function notifyAboutNewShare($shareWith, string $shareId, $ownerFederatedId, $sharedByFederatedId, string $name, string $displayName): void {
279289
$notification = $this->notificationManager->createNotification();
280290
$notification->setApp('files_sharing')
281291
->setUser($shareWith)
@@ -813,7 +823,7 @@ public function getFederationIdFromSharedSecret(
813823
return '';
814824
}
815825

816-
return $share['user'] . '@' . $share['remote'];
826+
return $share->getUser() . '@' . $share->getRemote();
817827
}
818828

819829
// if uid_owner is a local account, the request comes from the recipient

apps/files_sharing/composer/composer/autoload_classmap.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@
5050
'OCA\\Files_Sharing\\Exceptions\\SharingRightsException' => $baseDir . '/../lib/Exceptions/SharingRightsException.php',
5151
'OCA\\Files_Sharing\\ExpireSharesJob' => $baseDir . '/../lib/ExpireSharesJob.php',
5252
'OCA\\Files_Sharing\\External\\Cache' => $baseDir . '/../lib/External/Cache.php',
53+
'OCA\\Files_Sharing\\External\\ExternalShare' => $baseDir . '/../lib/External/ExternalShare.php',
54+
'OCA\\Files_Sharing\\External\\ExternalShareMapper' => $baseDir . '/../lib/External/ExternalShareMapper.php',
5355
'OCA\\Files_Sharing\\External\\Manager' => $baseDir . '/../lib/External/Manager.php',
5456
'OCA\\Files_Sharing\\External\\Mount' => $baseDir . '/../lib/External/Mount.php',
5557
'OCA\\Files_Sharing\\External\\MountProvider' => $baseDir . '/../lib/External/MountProvider.php',

apps/files_sharing/composer/composer/autoload_static.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ class ComposerStaticInitFiles_Sharing
6565
'OCA\\Files_Sharing\\Exceptions\\SharingRightsException' => __DIR__ . '/..' . '/../lib/Exceptions/SharingRightsException.php',
6666
'OCA\\Files_Sharing\\ExpireSharesJob' => __DIR__ . '/..' . '/../lib/ExpireSharesJob.php',
6767
'OCA\\Files_Sharing\\External\\Cache' => __DIR__ . '/..' . '/../lib/External/Cache.php',
68+
'OCA\\Files_Sharing\\External\\ExternalShare' => __DIR__ . '/..' . '/../lib/External/ExternalShare.php',
69+
'OCA\\Files_Sharing\\External\\ExternalShareMapper' => __DIR__ . '/..' . '/../lib/External/ExternalShareMapper.php',
6870
'OCA\\Files_Sharing\\External\\Manager' => __DIR__ . '/..' . '/../lib/External/Manager.php',
6971
'OCA\\Files_Sharing\\External\\Mount' => __DIR__ . '/..' . '/../lib/External/Mount.php',
7072
'OCA\\Files_Sharing\\External\\MountProvider' => __DIR__ . '/..' . '/../lib/External/MountProvider.php',

apps/files_sharing/lib/Controller/ExternalSharesController.php

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
namespace OCA\Files_Sharing\Controller;
99

10+
use OCA\Files_Sharing\External\Manager;
1011
use OCP\AppFramework\Controller;
1112
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
1213
use OCP\AppFramework\Http\JSONResponse;
@@ -21,42 +22,40 @@ class ExternalSharesController extends Controller {
2122
public function __construct(
2223
string $appName,
2324
IRequest $request,
24-
private \OCA\Files_Sharing\External\Manager $externalManager,
25+
private readonly Manager $externalManager,
2526
) {
2627
parent::__construct($appName, $request);
2728
}
2829

2930
/**
3031
* @NoOutgoingFederatedSharingRequired
31-
*
32-
* @return JSONResponse
3332
*/
3433
#[NoAdminRequired]
35-
public function index() {
34+
public function index(): JSONResponse {
3635
return new JSONResponse($this->externalManager->getOpenShares());
3736
}
3837

3938
/**
4039
* @NoOutgoingFederatedSharingRequired
41-
*
42-
* @param int $id
43-
* @return JSONResponse
4440
*/
4541
#[NoAdminRequired]
46-
public function create($id) {
47-
$this->externalManager->acceptShare($id);
42+
public function create(string $id): JSONResponse {
43+
$externalShare = $this->externalManager->getShare($id);
44+
if ($externalShare !== false) {
45+
$this->externalManager->acceptShare($externalShare);
46+
}
4847
return new JSONResponse();
4948
}
5049

5150
/**
5251
* @NoOutgoingFederatedSharingRequired
53-
*
54-
* @param integer $id
55-
* @return JSONResponse
5652
*/
5753
#[NoAdminRequired]
58-
public function destroy($id) {
59-
$this->externalManager->declineShare($id);
54+
public function destroy(string $id): JSONResponse {
55+
$externalShare = $this->externalManager->getShare($id);
56+
if ($externalShare !== false) {
57+
$this->externalManager->declineShare($externalShare);
58+
}
6059
return new JSONResponse();
6160
}
6261
}

0 commit comments

Comments
 (0)