Skip to content

[stable26] Avoid cleanup on close to not run into race conditions with other sessions on the same document #3922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
juliusknorr merged 10 commits into from
Mar 9, 2023
Merged

[stable26] Avoid cleanup on close to not run into race conditions with other sessions on the same document #3922

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fcb3ce8
feat(logging): Add debug logging for empty file saving
juliusknorr Mar 7, 2023
2dd921d
fix: Rework save locking
juliusknorr Mar 7, 2023
a5b9229
feat(logging): Add some logging to document cleanup
juliusknorr Mar 7, 2023
9754f1f
fix: Do not cleanup on close
juliusknorr Mar 7, 2023
4eac11d
fix: Cleanup attachments on itinial document cleanup
juliusknorr Mar 7, 2023
949b404
fix: Still unlock documents on close
juliusknorr Mar 8, 2023
dba594d
chore: Make resetDocument more readable
juliusknorr Mar 8, 2023
45662b5
fix: Do not reset on create
juliusknorr Mar 8, 2023
a3c6fb8
fix: Cleanup actual documents in cron
juliusknorr Mar 8, 2023
b38032e
ci: Adapt cypress tests to new cleanup behaviour
juliusknorr Mar 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat(logging): Add some logging to document cleanup 
Signed-off-by: Julius Härtl <[email protected]>
  • Loading branch information
@juliusknorr @backportbot-nextcloud
juliusknorr authored and backportbot-nextcloud[bot] committed Mar 9, 2023
commit a5b9229ceab538dacf58b47bc6f153c91231d57d
16 changes: 14 additions & 2 deletions lib/Service/ApiService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,17 @@ public function create($fileId = null, $filePath = null, $token = null, $guestNa
if ($forceRecreate || count($remainingSessions) === 0) {
$freshSession = true;
try {
$this->logger->info('Attempt to reset document during session create for ' . $file->getId());
$this->documentService->resetDocument($file->getId(), $forceRecreate);
} catch (DocumentHasUnsavedChangesException $e) {
$this->logger->debug('Failed to reset document during session create for ' . $file->getId(), [
'remainingSessions' => $remainingSessions,
]);
}
} else {
$this->logger->info('Keep previous document state of ' . $file->getId(), [
'remainingSessions' => $remainingSessions,
]);
}

$document = $this->documentService->createDocument($file);
Expand All @@ -132,20 +140,22 @@ public function create($fileId = null, $filePath = null, $token = null, $guestNa
$session = $this->sessionService->initSession($document->getId(), $guestName);

if ($freshSession) {
$this->logger->debug('Starting a fresh session');
$this->logger->debug('Starting a fresh editing session for ' . $file->getId());
$documentState = null;
$content = $this->loadContent($file);
} else {
$this->logger->debug('Loading existing session');
$this->logger->debug('Loading existing session for ' . $file->getId());
$content = null;
try {
$stateFile = $this->documentService->getStateFile($document->getId());
$documentState = $stateFile->getContent();
} catch (NotFoundException $e) {
$this->logger->debug('State file not found for ' . $file->getId());
$documentState = ''; // no state saved yet.
// If there are no steps yet we might still need the content.
$steps = $this->documentService->getSteps($document->getId(), 0);
if (empty($steps)) {
$this->logger->debug('Empty steps, loading content for ' . $file->getId());
$content = $this->loadContent($file);
}
}
Expand Down Expand Up @@ -179,7 +189,9 @@ public function close($documentId, $sessionId, $sessionToken): DataResponse {
try {
$this->documentService->resetDocument($documentId);
$this->attachmentService->cleanupAttachments($documentId);
$this->logger->info('Reset unsaved changes of ' . $documentId);
} catch (DocumentHasUnsavedChangesException $e) {
$this->logger->error('Did not reset unsaved changes during close of ' . $documentId, ['exception' => $e]);
}
}
return new DataResponse([]);
Expand Down
3 changes: 2 additions & 1 deletion lib/Service/DocumentService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public function createDocument(File $file): Document {
// This way the user can still resolve conflicts in the editor view
$stepsVersion = $this->stepMapper->getLatestVersion($document->getId());
if ($stepsVersion && ($document->getLastSavedVersion() !== $stepsVersion)) {
$this->logger->debug('Unsaved steps but collission with file, continue collaborative editing');
$this->logger->debug('Unsaved steps, continue collaborative editing');
return $document;
}
return $document;
Expand Down Expand Up @@ -258,6 +258,7 @@ private function insertSteps($documentId, $sessionId, $steps, $version): int {
}
$stepsVersion = $this->stepMapper->getLatestVersion($document->getId());
$newVersion = $stepsVersion + count($steps);
$this->logger->debug("Adding steps to $documentId: bumping version from $stepsVersion to $newVersion");
$this->cache->set('document-version-' . $document->getId(), $newVersion);
$step = new Step();
$step->setData($stepsJson);
Expand Down