fix(cache): invalidate root folder cache on rename

this fix https://github.com/AtalayaLabs/OxiCloud/issues/607
    which was introduced by commit 12dc648cff
    when a user rename a root folder, this invalidate the cache

    still some UX effect displaying phantom drive is grant is revoked,
    cache is 30s of TTL so this UX glitch is acceptable
This commit is contained in:
Edouard Vanbelle
2026-07-17 19:46:54 +02:00
parent dc009f053e
commit bd7b0710a8
3 changed files with 87 additions and 11 deletions
+21 -2
View File
@@ -581,7 +581,7 @@ impl FolderUseCase for FolderService {
)
.await?;
let folder = self
let renamed = self
.folder_storage
.rename_folder(id, dto.name, caller_id)
.await
@@ -592,7 +592,26 @@ impl FolderUseCase for FolderService {
)
})?;
Ok(FolderDto::from(folder))
// Root folders double as the drive's display name (see the
// `required_perm` branch above and `drive_pg_repository.rs`
// `readable_cache` + `default_drive_cache` docs).
// `drives.name` is sourced from `folders.name` of the root
// folder, so a rename affects BOTH caches — every user's
// readable-drive list AND the per-user default-drive lookup.
// Both are 30 s TTL; without the invalidation, `GET /api/drives`
// returns the stale name for up to that window after a root
// rename. Surfaced by `tests/api/drives_membership.hurl`
// Step 23. Regression from commit `12dc648c` ("perf: round 4 —
// drive-selector cache") which added the caches without
// wiring the root-rename invalidation.
if folder.parent_id().is_none()
&& let Some(drive_repo) = &self.drive_repo
{
drive_repo.invalidate_readable_all();
drive_repo.invalidate_default_drive_all();
}
Ok(FolderDto::from(renamed))
}
/// Moves a folder to a new parent. Requires `Update` on the source and
@@ -184,6 +184,29 @@ pub trait DriveRepository: Send + Sync + 'static {
/// content first so a single click can't wipe a populated drive.
async fn is_empty(&self, drive_id: Uuid) -> Result<bool, DriveRepositoryError>;
/// Drop the cached readable-drive list for one user. Called by
/// service-layer code paths that mutate state affecting a specific
/// caller's drive listing (grant writes, membership changes) but
/// don't reach through the drive-repo itself. Default no-op — the
/// no-cache stubs need no plumbing.
async fn invalidate_readable_for_user(&self, _user_id: Uuid) {}
/// Drop every cached readable-drive list. Called when the affected
/// user set is unknown at this layer — group-subject grants, drive
/// deletion, policy edits, root-folder renames (drive.name is
/// sourced from the root folder, so a rename affects the listing
/// for every user with a grant on the drive). Default no-op.
fn invalidate_readable_all(&self) {}
/// Drop every entry in the "default drive per user" cache. Called
/// from paths that mutate a drive's display name or its root
/// folder id at the concrete cache level (root-folder rename is
/// the only one today). Same class of bug as
/// `invalidate_readable_all` — the cache holds a `DriveWithRootName`
/// with `root_folder_name` baked in, so a rename would otherwise
/// stay stale for the cache TTL. Default no-op.
fn invalidate_default_drive_all(&self) {}
/// Hard-delete a drive: its `role_grants` rows, its root folder,
/// and the drive row itself, in one transaction. Caller is
/// responsible for ensuring `is_empty` first; this method does
@@ -25,10 +25,11 @@ use crate::domain::repositories::drive_repository::{
/// policy edits — all of which invalidate explicitly below), yet it is
/// re-resolved on EVERY NextCloud request (basic-auth chroot), every
/// native `/webdav` request (Mode-B scope resolution) and every WOPI
/// call. 30 s mirrors `drive_role_cache` in `pg_acl_engine.rs` and bounds
/// the one non-invalidated staleness source: a root-folder *rename*,
/// which doesn't pass through this repository. Measured in
/// `benches/CHROOT-CACHE.md`.
/// call. 30 s mirrors `drive_role_cache` in `pg_acl_engine.rs`. Root-
/// folder renames — which don't pass through this repository directly
/// — invalidate via the `DriveRepository::invalidate_default_drive_all`
/// trait hook called from `folder_service::rename_folder_with_perms`
/// when `parent_id IS NULL`. Measured in `benches/CHROOT-CACHE.md`.
const DEFAULT_DRIVE_CACHE_TTL: Duration = Duration::from_secs(30);
/// One entry per active user; entries are small (a `Drive` + a name).
@@ -56,11 +57,19 @@ pub struct DrivePgRepository {
/// through this repository or `DriveManagementService` invalidates
/// explicitly (per-user when the subject is a User, whole cache for
/// Group subjects, whose transitive membership is not resolvable
/// here). Residual staleness — a root-folder rename or a grant
/// written by a path that can't reach this cache — is bounded by
/// the same 30 s TTL the sibling caches accept; actual permission
/// enforcement is unaffected (the ACL engine re-checks per
/// operation with its own invalidation).
/// here). Root-folder renames — which update `drive.name` because it
/// reads through `folders.name` of the root row — also invalidate,
/// via the trait's `invalidate_readable_all` hook called from
/// `folder_service::rename_folder_with_perms` when
/// `parent_id IS NULL`. That path was missed by the perf commit
/// that introduced this cache (`12dc648c`) and surfaced by
/// `drives_membership.hurl` Step 23; the trait hook closes it
/// without folder_service knowing about the concrete moka cache.
///
/// Residual staleness — a grant written by a path that can't reach
/// this cache — is bounded by the same 30 s TTL the sibling caches
/// accept; actual permission enforcement is unaffected (the ACL
/// engine re-checks per operation with its own invalidation).
readable_cache: Cache<Uuid, Arc<Vec<DriveWithRootName>>>,
}
@@ -93,6 +102,15 @@ impl DrivePgRepository {
self.readable_cache.invalidate_all();
}
/// Drop every cached `default_drive_cache` entry. Exposed as a
/// `pub` sibling of the whole-cache invalidators above so trait
/// callers holding a `dyn DriveRepository` can trigger the same
/// cleanup path (e.g. `folder_service` on root-folder rename —
/// see `impl DriveRepository` below).
pub fn invalidate_default_drive_all(&self) {
self.default_drive_cache.invalidate_all();
}
fn map_sqlx_err(context: &'static str, e: sqlx::Error) -> DriveRepositoryError {
if let sqlx::Error::Database(ref dberr) = e
&& let Some(code) = dberr.code()
@@ -212,6 +230,22 @@ impl DrivePgRepository {
#[async_trait::async_trait]
impl DriveRepository for DrivePgRepository {
async fn invalidate_readable_for_user(&self, user_id: Uuid) {
// Delegate to the inherent method — the trait forwarding lets
// callers holding a `dyn DriveRepository` (e.g. `folder_service`
// on a root-folder rename) trigger invalidation without knowing
// about the concrete cache.
DrivePgRepository::invalidate_readable_for_user(self, user_id).await;
}
fn invalidate_readable_all(&self) {
DrivePgRepository::invalidate_readable_all(self);
}
fn invalidate_default_drive_all(&self) {
DrivePgRepository::invalidate_default_drive_all(self);
}
async fn create_personal_drive_atomic(
&self,
owner_id: Uuid,