From a89622c790898a39daba8a63f618c115b36347aa Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Tue, 31 May 2022 08:44:19 -0100 Subject: [PATCH] fixes Signed-off-by: Maxence Lange --- lib/CircleSharesManager.php | 8 -- lib/InternalAsync/AsyncItemUpdate.php | 141 +++++++++++++++++++++- lib/Service/FederatedSyncItemService.php | 144 +++++++---------------- 3 files changed, 179 insertions(+), 114 deletions(-) diff --git a/lib/CircleSharesManager.php b/lib/CircleSharesManager.php index d419f6a82..5adfc1098 100644 --- a/lib/CircleSharesManager.php +++ b/lib/CircleSharesManager.php @@ -237,14 +237,6 @@ public function updateItem( ); try { - -// // TODO: verify rules that apply when sharing to a circle -// $probe = new CircleProbe(); -// $probe->includeSystemCircles() -// ->mustBeMember(); -// -// $circle = $this->circleService->getCircle($circleId, $probe); -// // get valid SyncedItem based on appId, itemType, itemId $syncedItem = $this->federatedSyncItemService->initSyncedItem( $this->originAppId, diff --git a/lib/InternalAsync/AsyncItemUpdate.php b/lib/InternalAsync/AsyncItemUpdate.php index 809fd8610..a890598d1 100644 --- a/lib/InternalAsync/AsyncItemUpdate.php +++ b/lib/InternalAsync/AsyncItemUpdate.php @@ -31,22 +31,157 @@ namespace OCA\Circles\InternalAsync; +use OCA\Circles\Db\CircleRequest; +use OCA\Circles\Db\SyncedItemLockRequest; +use OCA\Circles\Db\SyncedItemRequest; +use OCA\Circles\Db\SyncedShareRequest; +use OCA\Circles\Exceptions\FederatedEventException; +use OCA\Circles\Exceptions\FederatedItemException; +use OCA\Circles\Exceptions\FederatedSyncManagerNotFoundException; +use OCA\Circles\Exceptions\InitiatorNotConfirmedException; +use OCA\Circles\Exceptions\OwnerNotFoundException; +use OCA\Circles\Exceptions\RemoteInstanceException; +use OCA\Circles\Exceptions\RemoteNotFoundException; +use OCA\Circles\Exceptions\RemoteResourceNotFoundException; +use OCA\Circles\Exceptions\RequestBuilderException; +use OCA\Circles\Exceptions\SyncedItemNotFoundException; +use OCA\Circles\Exceptions\UnknownRemoteException; +use OCA\Circles\FederatedItems\FederatedSync\ItemUpdate; use OCA\Circles\IInternalAsync; +use OCA\Circles\Model\Circle; +use OCA\Circles\Model\Federated\FederatedEvent; +use OCA\Circles\Model\SyncedItem; +use OCA\Circles\Model\SyncedItemLock; +use OCA\Circles\Model\SyncedShare; +use OCA\Circles\Service\DebugService; +use OCA\Circles\Service\FederatedEventService; +use OCA\Circles\Service\FederatedSyncService; +use OCA\Circles\Tools\Exceptions\InvalidItemException; use OCA\Circles\Tools\Model\ReferencedDataStore; class AsyncItemUpdate implements IInternalAsync { + private CircleRequest $circleRequest; + private SyncedItemRequest $syncedItemRequest; + private SyncedShareRequest $syncedShareRequest; + private SyncedItemLockRequest $syncedItemLockRequest; + private FederatedEventService $federatedEventService; + private FederatedSyncService $federatedSyncService; + private DebugService $debugService; - public function __construct() { + public function __construct( + CircleRequest $circleRequest, + SyncedItemRequest $syncedItemRequest, + SyncedShareRequest $syncedShareRequest, + SyncedItemLockRequest $syncedItemLockRequest, + FederatedEventService $federatedEventService, + FederatedSyncService $federatedSyncService, + DebugService $debugService + ) { + $this->circleRequest = $circleRequest; + $this->syncedItemRequest = $syncedItemRequest; + $this->syncedShareRequest = $syncedShareRequest; + $this->syncedItemLockRequest = $syncedItemLockRequest; + $this->federatedEventService = $federatedEventService; + $this->federatedSyncService = $federatedSyncService; + $this->debugService = $debugService; } + /** + * @param ReferencedDataStore $store + * + * @throws FederatedSyncManagerNotFoundException + * @throws SyncedItemNotFoundException + * @throws InvalidItemException + */ public function runAsynced(ReferencedDataStore $store): void { - // update Checksum + /** @var SyncedItem $syncedItem */ + $syncedItem = $store->gObj('syncedItem'); + /** @var SyncedItemLock $syncedLock */ + $syncedLock = $store->gObj('syncedItemLock'); - // broadcast update + $item = $this->federatedSyncService->initSyncManager($syncedItem) + ->serializeItem($syncedItem->getItemId()); + $syncedItem->setSerialized($item); + $this->updateChecksum($syncedItem); + $this->broadcastItemUpdate($syncedItem); + + $this->removeLock($syncedLock); } + + /** + * @param Circle $circle + * @param SyncedItem $syncedItem + * + * @throws FederatedEventException + * @throws FederatedItemException + * @throws InitiatorNotConfirmedException + * @throws OwnerNotFoundException + * @throws RemoteInstanceException + * @throws RemoteNotFoundException + * @throws RemoteResourceNotFoundException + * @throws RequestBuilderException + * @throws UnknownRemoteException + */ + private function broadcastItemUpdate(SyncedItem $item): void { + $item->setSerialized(); + + foreach ($this->getAffectedCircles($item->getSingleId()) as $circle) { + $event = new FederatedEvent(ItemUpdate::class); + $event->setCircle($circle) + ->setSyncedItem($item); + + $this->debugService->info( + 'generating {`IFederatedEvent} using {event.class}', + $circle->getSingleId(), + [ + 'event' => $event, + 'syncedItem' => $item, + 'circle' => $circle + ] + ); + + // TODO: confirm there is no re-async as we are already on a // thread (even with multiple circles) + $this->federatedEventService->newEvent($event); + } + } + + + /** + * @param string $singleId + * + * @return Circle[] + * @throws RequestBuilderException + */ + private function getAffectedCircles(string $singleId): array { + $circleIds = array_map( + function (SyncedShare $share): string { + return $share->getCircleId(); + }, $this->syncedShareRequest->getshares($singleId) + ); + + return $this->circleRequest->getCirclesByIds($circleIds); + } + + + /** + * @param SyncedItem $syncedItem + */ + private function updateChecksum(SyncedItem $syncedItem): void { + $sum = md5(json_encode($syncedItem->getSerialized())); + + $this->syncedItemRequest->updateChecksum($syncedItem->getSingleId(), $sum); + } + + + /** + * @param SyncedItemLock $syncedLock + */ + private function removeLock(SyncedItemLock $syncedLock): void { + $this->syncedItemLockRequest->remove($syncedLock); + } } diff --git a/lib/Service/FederatedSyncItemService.php b/lib/Service/FederatedSyncItemService.php index b40c1b702..3d551216a 100644 --- a/lib/Service/FederatedSyncItemService.php +++ b/lib/Service/FederatedSyncItemService.php @@ -51,15 +51,11 @@ use OCA\Circles\Exceptions\SyncedItemLockException; use OCA\Circles\Exceptions\SyncedItemNotFoundException; use OCA\Circles\Exceptions\UnknownRemoteException; -use OCA\Circles\FederatedItems\FederatedSync\ItemUpdate; use OCA\Circles\IFederatedUser; use OCA\Circles\InternalAsync\AsyncItemUpdate; -use OCA\Circles\Model\Circle; -use OCA\Circles\Model\Federated\FederatedEvent; use OCA\Circles\Model\Federated\RemoteInstance; use OCA\Circles\Model\SyncedItem; use OCA\Circles\Model\SyncedItemLock; -use OCA\Circles\Model\SyncedShare; use OCA\Circles\Model\SyncedWrapper; use OCA\Circles\Tools\ActivityPub\NCSignature; use OCA\Circles\Tools\Exceptions\InvalidItemException; @@ -309,8 +305,7 @@ public function requestSyncedItemUpdate( $federatedUser, $syncedItem, $syncedLock, - $extraData, - $initiatedRemotely + $extraData ); return; @@ -323,6 +318,7 @@ public function requestSyncedItemUpdate( throw new FederatedSyncConflictException(); } + /** * @param IFederatedUser $federatedUser * @param SyncedItem $syncedItem @@ -346,13 +342,11 @@ private function requestSyncedItemUpdateLocal( IFederatedUser $federatedUser, SyncedItem $syncedItem, SyncedItemLock $syncedLock, - array $extraData = [], - bool $initiatedRemotely = false + array $extraData = [] ): void { // item will be lock during the process, only to be unlocked when new item checksum have // been calculated (on async process) - // verify checksum and apps settings about the lock - $this->manageLock($syncedLock); + $this->manageLock($syncedItem, $syncedLock); if (!$this->isItemModifiable($federatedUser, $syncedItem, $syncedLock, $extraData)) { throw new FederatedSyncPermissionException('item modification not allowed'); @@ -372,20 +366,15 @@ private function requestSyncedItemUpdateLocal( $federatedUser ); - // - // Move all code below out of this method ? - // - // -// if ($initiatedRemotely) { - $this->asyncService->split(); // we need to confirm this is good enough -// } + // this should split the process and give back hand if request is coming from a remote instance + $this->asyncService->split(); $this->asyncService->asyncInternal( AsyncItemUpdate::class, new ReferencedDataStore( [ 'syncedItem' => $syncedItem, - 'syncedLock' => $syncedLock, + 'syncedItemLock' => $syncedLock, ] ) ); @@ -404,7 +393,6 @@ private function requestSyncedItemUpdateLocal( // $syncedItem->setSerialized($item); - $this->removeLock($syncedLock); // return $item; @@ -446,86 +434,36 @@ private function requestSyncedItemUpdateRemote( ); $syncManager = $this->federatedSyncService->initSyncManager($syncedItem); - $syncManager->syncItem( - $syncedItem->getItemId(), - $data - ); - - $this->updateChecksum($syncedItem->getSingleId(), $data); - - return $data; - } - - - private function updateChecksum(string $syncedItemId, ?array $data = null): void { - $currSum = ''; - if (is_null($data)) { - $knownItem = $this->getLocalSyncedItem($syncedItemId, true); - $data = $knownItem->getSerialized(); - $currSum = $knownItem->getChecksum(); - } - - $sum = md5(json_encode($data)); - if ($sum === $currSum) { - return; - } - - $this->syncedItemRequest->updateChecksum($syncedItemId, $sum); - } - - /** - * @param Circle $circle - * @param SyncedItem $syncedItem - * - * @throws FederatedEventException - * @throws FederatedItemException - * @throws InitiatorNotConfirmedException - * @throws OwnerNotFoundException - * @throws RemoteInstanceException - * @throws RemoteNotFoundException - * @throws RemoteResourceNotFoundException - * @throws RequestBuilderException - * @throws UnknownRemoteException - */ - private function broadcastItemUpdate(string $singleId): void { - $syncedItem = $this->syncedItemRequest->getSyncedItemFromSingleId($singleId); - - foreach ($this->getAffectedCircles($singleId) as $circle) { - $event = new FederatedEvent(ItemUpdate::class); - $event->setCircle($circle) - ->setSyncedItem($syncedItem); - - $this->debugService->info( - 'generating {`IFederatedEvent} using {event.class}', - $circle->getSingleId(), - [ - 'event' => $event, - 'syncedItem' => $syncedItem, - 'circle' => $circle - ] - ); - - // TODO: do not async for each circle as it should already on a async process! - $this->federatedEventService->newEvent($event); - } - } + $syncManager->onItemModification(); +// $syncManager->syncItem( +// $syncedItem->getItemId(), +// $data +// ); - /** - * @param string $singleId - * - * @return Circle[] - * @throws RequestBuilderException - */ - private function getAffectedCircles(string $singleId): array { - $circleIds = array_map( - function (SyncedShare $share): string { - return $share->getCircleId(); - }, $this->syncedShareRequest->getshares($singleId) - ); + // update item based on current (old) checksum to confirm item was not already updated (race condition) + // do not update checksum +// $this->updateChecksum($syncedItem->getSingleId(), $data); - return $this->circleRequest->getCirclesByIds($circleIds); + return $data; } +// +// +// private function updateChecksum(string $syncedItemId, ?array $data = null): void { +// $currSum = ''; +// if (is_null($data)) { +// $knownItem = $this->getLocalSyncedItem($syncedItemId, true); +// $data = $knownItem->getSerialized(); +// $currSum = $knownItem->getChecksum(); +// } +// +// $sum = md5(json_encode($data)); +// if ($sum === $currSum) { +// return; +// } +// +// $this->syncedItemRequest->updateChecksum($syncedItemId, $sum); +// } /** @@ -787,7 +725,7 @@ private function compareWithKnownItemId(SyncedItem $syncedItem): void { * @throws InvalidItemException * @throws SyncedItemLockException */ - private function manageLock(SyncedItemLock $syncedLock): void { + private function manageLock(SyncedItem $syncedItem, SyncedItemLock $syncedLock): void { $locked = true; for ($i = 0; $i < self::LOCK_RETRY_LIMIT; $i++) { try { @@ -800,6 +738,13 @@ private function manageLock(SyncedItemLock $syncedLock): void { } } + if ($syncedLock->isVerifyChecksum()) { + $known = $this->syncedItemRequest->getSyncedItemFromSingleId($syncedItem->getSingleId()); + if ($known->getChecksum() !== $syncedItem->getChecksum()) { + throw new SyncedItemLockException('checksum is too old'); + } + } + if ($locked) { throw new SyncedItemLockException('item is currently lock, try again later'); } @@ -807,11 +752,4 @@ private function manageLock(SyncedItemLock $syncedLock): void { $this->syncedItemLockRequest->save($syncedLock); } - - /** - * @param SyncedItemLock $syncedLock - */ - private function removeLock(SyncedItemLock $syncedLock): void { - $this->syncedItemLockRequest->remove($syncedLock); - } }