fix: Transfer ownership with S3 as primary

[artonge]

Issue

When using S3 as primary storage, transferring ownership with the --move option fail with the following error:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '8-45b963397aa40d4a0063e0d85e4fe7a1' for key 'fs_storage_path_hash'

The --move option moves the entire home folder from one account to another.
The error means that the move failed because the destination folder already exist in oc_filecache.

Might have been introduced by: #26149

Investigation

1. With S3 as primary storage, folders only exists as entries in oc_filecache.
2. With S3 as primary storage, moveFromStorage(...) only moves the cache entry, as nothing needs to be moved on disk. This cache move does not delete potentially pre-existing destination folder.
3. With Local storage, moveFromStorage(...) calls rename(...) which delete pre-existing folder.

S3's moveFromStorage

server/lib/private/Files/ObjectStore/ObjectStoreStorage.php

Lines 593 to 599 in 373107b

	public function moveFromStorage(IStorage $sourceStorage, string $sourceInternalPath, string $targetInternalPath, ?ICacheEntry $sourceCacheEntry = null): bool {
	$sourceCache = $sourceStorage->getCache();
	if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class) && $sourceStorage->getObjectStore()->getStorageId() === $this->getObjectStore()->getStorageId()) {
	$this->getCache()->moveFromCache($sourceCache, $sourceInternalPath, $targetInternalPath);
	// Do not import any data when source and target bucket are identical.
	return true;
	}

Local's rename, called by moveFromStorage

server/lib/private/Files/Storage/Local.php

Lines 346 to 352 in 373107b

	if ($this->file_exists($target)) {
	if ($this->is_dir($target)) {
	$this->rmdir($target);
	} elseif ($this->is_file($target)) {
	$this->unlink($target);
	}
	}

Solution

Delete pre-existing folder in moveFromStorage(...)

Does it make sense to have that here? It feels off.

Todo

• Target master
• Add tests to tests/lib/Files/Storage/StoragesTest.php

[artonge]

/backport to stable31

[artonge]

/backport to stable30

[artonge]

/backport to stable29

[icewind1991]

The existing objects will have to be deleted to, not just the cache entries

[artonge]

The existing objects will have to be deleted to, not just the cache entries

Indeed, that's a concern. Not sure how to best proceed, because in case of a folder, we'll need to get all its children, no?
It leads me to wonder how are the children of a folder removed from the cache.

[artonge]

The existing objects will have to be deleted to, not just the cache entries

I added $this->unlink($targetInternalPath); and some extra assertion in the test.