diff options
author | Julius Härtl <jus@bitgrid.net> | 2019-05-08 18:44:09 +0300 |
---|---|---|
committer | Julius Härtl <jus@bitgrid.net> | 2019-05-08 18:44:09 +0300 |
commit | d24b3fadb43810cf94bc652e8710a02019e1a801 (patch) | |
tree | 068bc6f0d14fad706db837e7d175c1dd20e98ecb | |
parent | a868aca3e6c1f8669398ac4a747fa40a496319e7 (diff) |
Proper conflict handling in the backend
Signed-off-by: Julius Härtl <jus@bitgrid.net>
-rw-r--r-- | lib/Controller/SessionController.php | 5 | ||||
-rw-r--r-- | lib/Db/Document.php | 2 | ||||
-rw-r--r-- | lib/Db/StepMapper.php | 11 | ||||
-rw-r--r-- | lib/Service/DocumentService.php | 102 | ||||
-rw-r--r-- | lib/VersionMismatchException.php | 34 |
5 files changed, 128 insertions, 26 deletions
diff --git a/lib/Controller/SessionController.php b/lib/Controller/SessionController.php index 671c47078..58a2dd7d4 100644 --- a/lib/Controller/SessionController.php +++ b/lib/Controller/SessionController.php @@ -7,6 +7,7 @@ namespace OCA\Text\Controller; use OC\Files\Node\File; use OCA\Text\Service\DocumentService; use OCA\Text\Service\SessionService; +use OCA\Text\VersionMismatchException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\FileDisplayResponse; @@ -85,8 +86,8 @@ class SessionController extends Controller { if ($this->sessionService->isValidSession($documentId, $sessionId, $token)) { try { $steps = $this->documentService->addStep($documentId, $sessionId, $steps, $version); - } catch (\Exception $e) { - return new DataResponse($e->getMessage(), 500); + } catch (VersionMismatchException $e) { + return new DataResponse($e->getMessage(), $e->getStatus()); } return new DataResponse($steps); } diff --git a/lib/Db/Document.php b/lib/Db/Document.php index 5c1e30c06..07a1e64ce 100644 --- a/lib/Db/Document.php +++ b/lib/Db/Document.php @@ -33,6 +33,8 @@ class Document extends Entity implements \JsonSerializable { protected $lastSavedVersion = 0; protected $initialVersion = 0; protected $lastSavedVersionTime = 0; + protected $lastSavedVersionEtag = ''; + protected $baseVersionEtag = ''; public function __construct() { $this->addType('id', 'integer'); diff --git a/lib/Db/StepMapper.php b/lib/Db/StepMapper.php index 847409750..6b580ee9b 100644 --- a/lib/Db/StepMapper.php +++ b/lib/Db/StepMapper.php @@ -34,13 +34,20 @@ class StepMapper extends QBMapper { parent::__construct($db, 'text_steps', Step::class); } - public function find($documentId, $fromVersion) { + public function find($documentId, $fromVersion, $lastAckedVersion = null) { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from($this->getTableName()) ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))) - ->andWhere($qb->expr()->gt('version', $qb->createNamedParameter($fromVersion))) + ->andWhere($qb->expr()->gt('version', $qb->createNamedParameter($fromVersion))); + // WIP: only return steps that were persisted completely + if ($lastAckedVersion) { + $qb->andWhere($qb->expr()->lte('version', $qb->createNamedParameter($lastAckedVersion))); + } + $qb + // TODO: limiting results currently causes the loading detection to fail + // ->setMaxResults(500) ->execute(); return $this->findEntities($qb); diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index e8e605c86..d07aa8cb2 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -23,6 +23,8 @@ namespace OCA\Text\Service; +use http\Exception\InvalidArgumentException; +use function json_encode; use OC\Files\Node\File; use OCA\Text\Db\Document; use OCA\Text\Db\DocumentMapper; @@ -30,26 +32,32 @@ use OCA\Text\Db\SessionMapper; use OCA\Text\Db\Step; use OCA\Text\Db\StepMapper; use OCA\Text\DocumentSaveConflictException; +use OCA\Text\VersionMismatchException; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\Entity; use OCP\Files\IAppData; use OCP\Files\InvalidPathException; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; +use OCP\Files\SimpleFS\ISimpleFile; use OCP\ICacheFactory; +use OCP\ILogger; +use OCP\Lock\ILockingProvider; class DocumentService { /** * Delay to wait for between autosave versions */ - const AUTOSAVE_MINIMUM_DELAY = 10; + const AUTOSAVE_MINIMUM_DELAY = 60; private $sessionMapper; private $userId; private $documentMapper; + private $logger; - public function __construct(SessionMapper $sessionMapper, DocumentMapper $documentMapper, StepMapper $stepMapper, IAppData $appData, $userId, IRootFolder $rootFolder, ICacheFactory $cacheFactory) { + public function __construct(SessionMapper $sessionMapper, DocumentMapper $documentMapper, StepMapper $stepMapper, IAppData $appData, $userId, IRootFolder $rootFolder, ICacheFactory $cacheFactory, ILogger $logger) { $this->sessionMapper = $sessionMapper; $this->documentMapper = $documentMapper; $this->stepMapper = $stepMapper; @@ -57,6 +65,7 @@ class DocumentService { $this->appData = $appData; $this->rootFolder = $rootFolder; $this->cache = $cacheFactory->createDistributed('text'); + $this->logger = $logger; try { $this->appData->getFolder('documents'); @@ -67,10 +76,10 @@ class DocumentService { /** * @param $path - * @return \OCP\AppFramework\Db\Entity + * @return Entity * @throws NotFoundException - * @throws \OCP\Files\InvalidPathException - * @throws \OCP\Files\NotPermittedException + * @throws InvalidPathException + * @throws NotPermittedException */ public function createDocumentByPath($path) { /** @var File $file */ @@ -80,10 +89,10 @@ class DocumentService { /** * @param $fileId - * @return \OCP\AppFramework\Db\Entity + * @return Entity * @throws NotFoundException - * @throws \OCP\Files\InvalidPathException - * @throws \OCP\Files\NotPermittedException + * @throws InvalidPathException + * @throws NotPermittedException */ public function getDocumentById($fileId) { return $this->createDocument($this->getFile($fileId)); @@ -94,23 +103,44 @@ class DocumentService { return $this->rootFolder->getUserFolder($this->userId)->getById($fileId)[0]; } + public function getFileForShare($token) { + + $userFolder = $this->rootFolder->getUserFolder($share->getShareOwner()); + $originalSharePath = $userFolder->getRelativePath($share->getNode()->getPath()); + + + // Single file share + if ($share->getNode() instanceof \OCP\Files\File) { + // Single file download + return $share->getNode(); + } + + return null; + } + /** * @param File $file - * @return \OCP\AppFramework\Db\Entity + * @return Entity * @throws NotFoundException - * @throws \OCP\Files\InvalidPathException - * @throws \OCP\Files\NotPermittedException + * @throws InvalidPathException + * @throws NotPermittedException */ protected function createDocument(File $file) { try { $document = $this->documentMapper->find($file->getFileInfo()->getId()); - // TODO: do not hard reset if changed from outside since this will throw away possible steps + // Do not hard reset if changed from outside since this will throw away possible steps + // This way the user can still resolve conflicts in the editor view + if ($document->getLastSavedVersion() !== $document->getCurrentVersion()) { + $this->logger->debug('Unsaved steps but collission with file, continue collaborative editing'); + return $document; + } + // TODO: Only do this when no sessions active, otherise we need to resolve the conflict differently $lastMTime = $document->getLastSavedVersionTime(); if ($file->getMTime() > $lastMTime && $lastMTime > 0) { $this->resetDocument($document->getId()); - throw new NotFoundException(); + throw new NotFoundException(); } return $document; @@ -131,28 +161,37 @@ class DocumentService { $document->setCurrentVersion(0); $document->setLastSavedVersion(0); $document->setLastSavedVersionTime($file->getFileInfo()->getMtime()); + $document->setLastSavedVersionEtag($file->getEtag()); + $document->setBaseVersionEtag($file->getEtag()); $document = $this->documentMapper->insert($document); $this->cache->set('document-version-'.$document->getId(), 0); return $document; } + /** + * @param $document + * @return ISimpleFile + * @throws NotFoundException + */ public function getBaseFile($document) { return $this->appData->getFolder('documents')->getFile($document); } /** - * @param Document $document + * @param $documentId + * @param $sessionId * @param $steps * @param $version * @return array - * @throws \Exception + * @throws DoesNotExistException + * @throws VersionMismatchException */ public function addStep($documentId, $sessionId, $steps, $version) { // TODO lock for other step adding // TODO check cache $document = $this->documentMapper->find($documentId); if ($version !== $document->getCurrentVersion()) { - throw new \Exception('Version does not match'); + throw new VersionMismatchException('Version does not match'); } $step = new Step(); $step->setData(\json_encode($steps)); @@ -172,30 +211,49 @@ class DocumentService { return $this->stepMapper->find($documentId, $lastVersion); } + /** + * @param $documentId + * @param $version + * @param $autoaveDocument + * @param bool $force + * @param bool $manualSave + * @return Document + * @throws DocumentSaveConflictException + * @throws DoesNotExistException + * @throws InvalidPathException + * @throws NotFoundException + * @throws NotPermittedException + * @throws \OCP\Files\GenericFileException + */ public function autosave($documentId, $version, $autoaveDocument, $force = false, $manualSave = false) { /** @var Document $document */ $document = $this->documentMapper->find($documentId); - if ($autoaveDocument === null) { - return $document; - } - $lastMTime = $document->getLastSavedVersionTime(); + /** @var File $file */ $file = $this->rootFolder->getUserFolder($this->userId)->getById($documentId)[0]; - if ($file->getMTime() > $lastMTime && $lastMTime > 0 && $force === false) { + + $lastMTime = $document->getLastSavedVersionTime(); + if ($lastMTime > 0 && $file->getEtag() !== $document->getLastSavedVersionEtag() && $force === false) { throw new DocumentSaveConflictException('File changed in the meantime from outside'); } + + if ($autoaveDocument === null) { + return $document; + } + // TODO: check for etag rather than mtime // Do not save if version already saved if (!$force && $version <= (string)$document->getLastSavedVersion()) { return $document; } // Only save once every AUTOSAVE_MINIMUM_DELAY seconds - if ($file->getMTime() === $lastMTime && $lastMTime > time()- self::AUTOSAVE_MINIMUM_DELAY && $manualSave === false) { + if ($file->getMTime() === $lastMTime && $lastMTime > time() - self::AUTOSAVE_MINIMUM_DELAY && $manualSave === false) { return $document; } $file->putContent($autoaveDocument); $document->setLastSavedVersion($version); $document->setLastSavedVersionTime(time()); + $document->setLastSavedVersionEtag($file->getEtag()); $this->documentMapper->update($document); return $document; } diff --git a/lib/VersionMismatchException.php b/lib/VersionMismatchException.php new file mode 100644 index 000000000..b4b1ea1ad --- /dev/null +++ b/lib/VersionMismatchException.php @@ -0,0 +1,34 @@ +<?php +/** + * @copyright Copyright (c) 2019 Julius Härtl <jus@bitgrid.net> + * + * @author Julius Härtl <jus@bitgrid.net> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +namespace OCA\Text; + + +use OCP\AppFramework\Http; + +class VersionMismatchException extends \Exception { + + public function getStatus() { + return Http::STATUS_PRECONDITION_FAILED; + } +} |