Skip to content

Commit e9a5835

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 <[email protected]>
1 parent 16ffc29 commit e9a5835

File tree

18 files changed

+875
-811
lines changed

18 files changed

+875
-811
lines changed

apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php

Lines changed: 25 additions & 12 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,6 +75,7 @@ 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

@@ -93,7 +97,7 @@ public function getShareType() {
9397
* @throws HintException
9498
* @since 14.0.0
9599
*/
96-
public function shareReceived(ICloudFederationShare $share) {
100+
public function shareReceived(ICloudFederationShare $share): string {
97101
if (!$this->isS2SEnabled(true)) {
98102
throw new ProviderCouldNotAddShareException('Server does not support federated cloud sharing', '', Http::STATUS_SERVICE_UNAVAILABLE);
99103
}
@@ -159,9 +163,18 @@ public function shareReceived(ICloudFederationShare $share) {
159163
}
160164
}
161165

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

166179
// get DisplayName about the owner of the share
167180
$ownerDisplayName = $this->getUserDisplayName($ownerFederatedId);
@@ -184,14 +197,14 @@ public function shareReceived(ICloudFederationShare $share) {
184197
->setType('remote_share')
185198
->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_RECEIVED, [$ownerFederatedId, trim($name, '/'), $ownerDisplayName])
186199
->setAffectedUser($shareWith)
187-
->setObject('remote_share', $shareId, $name);
200+
->setObject('remote_share', $externalShare->getId(), $name);
188201
Server::get(IActivityManager::class)->publish($event);
189-
$this->notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName);
202+
$this->notifyAboutNewShare($shareWith, $externalShare->getId(), $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName);
190203

191204
// If auto-accept is enabled, accept the share
192205
if ($this->federatedShareProvider->isFederatedTrustedShareAutoAccept() && $trustedServers?->isTrustedServer($remote) === true) {
193206
/** @var IUser $userOrGroup */
194-
$this->externalShareManager->acceptShare($shareId, $userOrGroup);
207+
$this->externalShareManager->acceptShare($externalShare, $userOrGroup);
195208
}
196209
} else {
197210
/** @var IGroup $userOrGroup */
@@ -202,18 +215,18 @@ public function shareReceived(ICloudFederationShare $share) {
202215
->setType('remote_share')
203216
->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_RECEIVED, [$ownerFederatedId, trim($name, '/'), $ownerDisplayName])
204217
->setAffectedUser($user->getUID())
205-
->setObject('remote_share', $shareId, $name);
218+
->setObject('remote_share', $externalShare->getId(), $name);
206219
Server::get(IActivityManager::class)->publish($event);
207-
$this->notifyAboutNewShare($user->getUID(), $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName);
220+
$this->notifyAboutNewShare($user->getUID(), $externalShare->getId(), $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName);
208221

209222
// If auto-accept is enabled, accept the share
210223
if ($this->federatedShareProvider->isFederatedTrustedShareAutoAccept() && $trustedServers?->isTrustedServer($remote) === true) {
211-
$this->externalShareManager->acceptShare($shareId, $user);
224+
$this->externalShareManager->acceptShare($externalShare, $user);
212225
}
213226
}
214227
}
215228

216-
return $shareId;
229+
return $externalShare->getId();
217230
} catch (\Exception $e) {
218231
$this->logger->error('Server can not add remote share.', [
219232
'app' => 'files_sharing',
@@ -275,7 +288,7 @@ private function mapShareTypeToNextcloud($shareType) {
275288
return $result;
276289
}
277290

278-
private function notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $displayName): void {
291+
private function notifyAboutNewShare($shareWith, string $shareId, $ownerFederatedId, $sharedByFederatedId, string $name, string $displayName): void {
279292
$notification = $this->notificationManager->createNotification();
280293
$notification->setApp('files_sharing')
281294
->setUser($shareWith)
@@ -813,7 +826,7 @@ public function getFederationIdFromSharedSecret(
813826
return '';
814827
}
815828

816-
return $share['user'] . '@' . $share['remote'];
829+
return $share->getUser() . '@' . $share->getRemote();
817830
}
818831

819832
// 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)