Skip to content

Commit 3bdb344

Browse files
CarlSchwanCarlSchwan
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 93b2583 commit 3bdb344

File tree

24 files changed

+941
-878
lines changed

24 files changed

+941
-878
lines changed

‎apps/cloud_federation_api/lib/Controller/RequestHandlerController.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,6 @@ private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, str
500500
*
501501
* @param IIncomingSignedRequest|null $signedRequest
502502
* @param string $resourceType
503-
* @param string $sharedSecret
504503
*
505504
* @throws IncomingRequestException
506505
* @throws BadRequestException
@@ -524,7 +523,7 @@ private function confirmNotificationIdentity(
524523
return;
525524
}
526525
} catch (\Exception $e) {
527-
throw new IncomingRequestException($e->getMessage());
526+
throw new IncomingRequestException($e->getMessage(), previous: $e);
528527
}
529528

530529
$this->confirmNotificationEntry($signedRequest, $identity);

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -752,10 +752,9 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) {
752752
* Get a share by token
753753
*
754754
* @param string $token
755-
* @return IShare
756755
* @throws ShareNotFound
757756
*/
758-
public function getShareByToken($token) {
757+
public function getShareByToken($token): IShare {
759758
$qb = $this->dbConnection->getQueryBuilder();
760759

761760
$cursor = $qb->select('*')
@@ -812,9 +811,9 @@ private function getRawShare($id) {
812811
* @throws InvalidShare
813812
* @throws ShareNotFound
814813
*/
815-
private function createShareObject($data) {
814+
private function createShareObject($data): IShare {
816815
$share = new Share($this->rootFolder, $this->userManager);
817-
$share->setId((int)$data['id'])
816+
$share->setId((string)$data['id'])
818817
->setShareType((int)$data['share_type'])
819818
->setPermissions((int)$data['permissions'])
820819
->setTarget($data['file_target'])

‎apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php‎

Lines changed: 30 additions & 20 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
}
@@ -103,7 +104,8 @@ public function shareReceived(ICloudFederationShare $share) {
103104
throw new ProviderCouldNotAddShareException('Unsupported protocol for data exchange.', '', Http::STATUS_NOT_IMPLEMENTED);
104105
}
105106

106-
[$ownerUid, $remote] = $this->addressHandler->splitUserRemote($share->getOwner());
107+
[, $remote] = $this->addressHandler->splitUserRemote($share->getOwner());
108+
107109
// for backward compatibility make sure that the remote url stored in the
108110
// database ends with a trailing slash
109111
if (!str_ends_with($remote, '/')) {
@@ -113,17 +115,15 @@ public function shareReceived(ICloudFederationShare $share) {
113115
$token = $share->getShareSecret();
114116
$name = $share->getResourceName();
115117
$owner = $share->getOwnerDisplayName() ?: $share->getOwner();
116-
$sharedBy = $share->getSharedByDisplayName();
117118
$shareWith = $share->getShareWith();
118119
$remoteId = $share->getProviderId();
119120
$sharedByFederatedId = $share->getSharedBy();
120121
$ownerFederatedId = $share->getOwner();
121122
$shareType = $this->mapShareTypeToNextcloud($share->getShareType());
122123

123-
// if no explicit information about the person who created the share was send
124+
// if no explicit information about the person who created the share was sent
124125
// we assume that the share comes from the owner
125126
if ($sharedByFederatedId === null) {
126-
$sharedBy = $owner;
127127
$sharedByFederatedId = $ownerFederatedId;
128128
}
129129

@@ -159,9 +159,19 @@ public function shareReceived(ICloudFederationShare $share) {
159159
}
160160
}
161161

162+
$externalShare = new ExternalShare();
163+
$externalShare->setId($this->snowflakeGenerator->nextId());
164+
$externalShare->setRemote($remote);
165+
$externalShare->setRemoteId($remoteId);
166+
$externalShare->setShareToken($token);
167+
$externalShare->setPassword('');
168+
$externalShare->setName($name);
169+
$externalShare->setOwner($owner);
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)