Skip to content

Deleting a team folder with separate storage leaves its oc_storages row behind — deleteStoragesForFolder() clears a jailed cache, which never deletes the storage row #5070

Description

@TheMorpheus407

Steps to reproduce

On an instance with separate-storage team folders and local (non-object) primary storage:

  1. occ groupfolders:create probe
  2. Note the new row in oc_storages: local::<datadir>/__groupfolders/<folder_id>/
  3. occ groupfolders:delete <folder_id> --force
  4. SELECT * FROM oc_storages WHERE id LIKE '%__groupfolders%';

Expected behaviour

The folder's oc_storages row is deleted with the folder, the way occ user:delete removes a
user's home:: row.

Actual behaviour

The folder is gone from oc_group_folders, its oc_filecache rows are gone, the directory under
<datadir>/__groupfolders/<id> is gone — and the oc_storages row is still there. One row is
orphaned per team folder the instance has ever deleted. On our instance twenty had accumulated;
occ files:scan --all and occ files:cleanup both report 0 and neither collects them, and
there is no occ verb and no API route that does.

Cause, traced at master (38ab1c1e)

lib/Mount/FolderStorageManager.php:272 deleteStoragesForFolder() fetches the folder's storage
through getBaseStorageForFolder() and calls $storage->getCache()->clear().

For a separate-storage folder that storage comes from
getBaseStorageForFolderSeparate(), which at :131-134 always returns the Local
(:137-162, rooted at <datadir>/__groupfolders/<id>) wrapped in a Jail:

return new Jail([
    'storage' => $storage,
    'root' => $type,
]);

So getCache() returns a CacheJail, whose clear() is
(server lib/private/Files/Cache/Wrapper/CacheJail.php:223):

public function clear() {
    $this->getCache()->remove($this->getRoot());
}

That removes the jailed subtree from oc_filecache and nothing else. The clear() that deletes
the storage row is the unwrapped one, server lib/private/Files/Cache/Cache.php:884:

$query->delete('storages')
    ->where($query->expr()->eq('id', $query->createNamedParameter($this->storageId)));

and the delete path never reaches it, because the storage is jailed on every way out. The
/** @var Cache $cache */ annotation on :275 says the code expects the unwrapped one.

Both entry points are affected — lib/Command/Delete.php:43 and
lib/Controller/FolderController.php:304 call the same method.

Note this is not #4155 / #4157: that fix added '' to the foreach so the folder's own root
directory is rmdir'd, and it works — the directory does go. This is the other half of the same
loop, the database row.

Suggested direction

Unwrap before clearing, so the storage's own cache is the one that gets clear()ed — e.g. take
the unjailed storage once after the loop and call getCache()->clear() on it, or resolve the
numeric storage id from the wrapper and delete the oc_storages row explicitly. A repair step
for instances that already carry orphans would be welcome too; a row is safe to take when no
oc_group_folders row names it and it holds no oc_filecache, oc_mounts or oc_previews row.

Versions

Nextcloud 34.0.3 · Team folders (groupfolders) 22.0.6 · PostgreSQL · local primary storage ·
separate-storage enabled. Re-read at master 38ab1c1e and at tag v23.0.0: the function is
unchanged in both.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions