Is your feature request related to a problem? Please describe.
I have been looking at the different solutions #6017 #5870 #5699 proposed
Describe the solution you'd like
Should there be only one source of time with all those solutions.
This service always asks the storage directly and never trusts a cached value silently. Every caller that makes a conflict/currency decision should go through a common place, so there is exactly one place this logic can drift.
Describe alternatives you've considered
Additional context
This was elaborated with claude sonnet IA. This PR is far above my pay grade so just putting this out there for you to evaluate....
Creation of
WopiMtimeResolver.php
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Richdocuments\Service;
use OCP\Files\Node;
use OCP\Files\StorageNotAvailableException;
use Psr\Log\LoggerInterface;
/**
* Single source of truth for "what is the real mtime of this node right now".
*
* Background: WOPI conflict detection (WopiController) and file_versions
* "current version" bookkeeping have historically read Node::getMTime(),
* which is served from the oc_filecache row. That row can lag behind the
* real mtime on the underlying storage during/after concurrent edits,
* producing:
* - false "document changed in storage" conflicts (#5832, #5794)
* - a stale/incorrect "current" file_version, breaking preview & download
* links until the cache catches up (#6017's motivating bug)
*
* This service always asks the storage directly and never trusts a cached
* value silently. Every caller that makes a conflict/currency decision
* should go through here instead of calling Node::getMTime() itself, so
* there is exactly one place this logic can drift.
*/
class WopiMtimeResolver {
/** Default allowed skew (seconds) before two mtimes are considered "different". */
public const DEFAULT_TOLERANCE_SECONDS = 1;
public function __construct(
private LoggerInterface $logger,
) {
}
/**
* Returns the authoritative mtime for a node, read directly from the
* storage backend rather than the (potentially stale) filecache.
*
* Falls back to the cached Node::getMTime() only if the storage refuses
* to answer (e.g. a transient backend hiccup), and logs that fallback
* so it is visible instead of silently masking drift.
*/
public function getAuthoritativeMtime(Node $node): int {
try {
$storage = $node->getStorage();
$fresh = $storage->filemtime($node->getInternalPath());
} catch (StorageNotAvailableException $e) {
$fresh = false;
}
if ($fresh === false || $fresh === null) {
$cached = $node->getMTime();
$this->logger->warning(
'WopiMtimeResolver: storage stat failed for node {fileId}, falling back to cached mtime {cached}',
['fileId' => $node->getId(), 'cached' => $cached],
);
return $cached;
}
return (int)$fresh;
}
/**
* Whether the node's real mtime has advanced past $referenceMtime by
* more than the allowed tolerance.
*
* Tolerance exists because WOPI timestamps are exchanged with
* second-level precision (Helper::toISO8601 truncates below the
* second), so a same-second write must not be flagged as a conflict.
*/
public function hasChangedSince(
Node $node,
int $referenceMtime,
int $toleranceSeconds = self::DEFAULT_TOLERANCE_SECONDS,
): bool {
$current = $this->getAuthoritativeMtime($node);
return ($current - $referenceMtime) > $toleranceSeconds;
}
/**
* Whether two mtimes should be treated as the same instant for WOPI
* purposes, given rounding/precision tolerance.
*/
public function isSameInstant(
int $mtimeA,
int $mtimeB,
int $toleranceSeconds = self::DEFAULT_TOLERANCE_SECONDS,
): bool {
return abs($mtimeA - $mtimeB) <= $toleranceSeconds;
}
}
WopiSaveGuard.php
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Richdocuments\Service;
use OCP\Files\Node;
use OCP\Lock\LockedException;
use Psr\Log\LoggerInterface;
/**
* Wraps the actual filesystem write performed by WopiController::putFile()/
* postFile() so that:
*
* - Any \Throwable escaping a write hook (files_versions listeners etc.)
* is caught, logged, and turned into a controlled WopiSaveResult instead
* of an uncaught error surfacing as an opaque 500 (see #5770/#5771:
* a TypeError from a versions hook is an \Error, not an \Exception, so
* a narrower catch(\Exception) silently let it through).
* - A LockedException is reported distinctly, so the controller can
* respond 409 (retry-able) instead of 500 (which the WOPI client can
* reinterpret as "document changed externally").
* - If the mtime advanced despite the write failing, that is flagged
* explicitly as a likely partial write, since an advanced mtime is
* exactly what makes the client assume an external change happened.
*
* This class intentionally has zero HTTP/JSONResponse knowledge, and takes
* its mtime source from WopiMtimeResolver rather than re-implementing
* mtime handling, so the two PRs this class merges (#5771's error
* hardening + #5870's "read mtime from storage") stay consistent with
* each other instead of drifting again.
*/
class WopiSaveGuard {
public function __construct(
private WopiMtimeResolver $mtimeResolver,
private LoggerInterface $logger,
) {
}
/**
* @param Node $node The node being written to.
* @param callable():void $write Performs the actual write. Any return
* value is ignored; throw on failure.
*/
public function guardedWrite(Node $node, callable $write): WopiSaveResult {
$mtimeBefore = $this->mtimeResolver->getAuthoritativeMtime($node);
try {
$write();
} catch (LockedException $e) {
$this->logger->info(
'WopiSaveGuard: save blocked by lock for node {fileId}, reporting as retryable',
['fileId' => $node->getId(), 'exception' => $e],
);
return WopiSaveResult::locked($e);
} catch (\Throwable $e) {
$partialWrite = $this->mtimeResolver->hasChangedSince($node, $mtimeBefore);
$this->logger->error(
'WopiSaveGuard: write failed for node {fileId}{partial}',
[
'fileId' => $node->getId(),
'partial' => $partialWrite ? ' (mtime advanced despite failure - partial write)' : '',
'exception' => $e,
],
);
return WopiSaveResult::failed($e, partialWriteDetected: $partialWrite);
}
$mtimeAfter = $this->mtimeResolver->getAuthoritativeMtime($node);
return WopiSaveResult::ok($mtimeAfter);
}
}
WopiSaveResult.php
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Richdocuments\Service;
/**
* Outcome of a guarded WOPI write, decoupled from any HTTP concern so it
* stays trivially testable. WopiController maps this to the appropriate
* JSONResponse / status code.
*/
final class WopiSaveResult {
public const STATUS_OK = 'ok';
public const STATUS_LOCKED = 'locked';
public const STATUS_FAILED = 'failed';
private function __construct(
public readonly string $status,
public readonly ?int $mtime = null,
public readonly ?\Throwable $error = null,
/** True if the mtime advanced despite the write failing (partial write). */
public readonly bool $partialWriteDetected = false,
) {
}
public static function ok(int $mtime): self {
return new self(self::STATUS_OK, mtime: $mtime);
}
public static function locked(\Throwable $error): self {
return new self(self::STATUS_LOCKED, error: $error);
}
public static function failed(\Throwable $error, bool $partialWriteDetected = false): self {
return new self(self::STATUS_FAILED, error: $error, partialWriteDetected: $partialWriteDetected);
}
public function isOk(): bool {
return $this->status === self::STATUS_OK;
}
}
Is your feature request related to a problem? Please describe.
I have been looking at the different solutions #6017 #5870 #5699 proposed
Describe the solution you'd like
Should there be only one source of time with all those solutions.
This service always asks the storage directly and never trusts a cached value silently. Every caller that makes a conflict/currency decision should go through a common place, so there is exactly one place this logic can drift.
Describe alternatives you've considered
Additional context
This was elaborated with claude sonnet IA. This PR is far above my pay grade so just putting this out there for you to evaluate....
Creation of
WopiMtimeResolver.php
WopiSaveGuard.php
WopiSaveResult.php