Merge pull request #52045 from nextcloud/shared-target-verify-cache

fix: more optimized caching for share target verification

apps/files_sharing/lib/MountProvider.php

@@ -11,6 +11,7 @@ use OCA\Files_Sharing\Event\ShareMountedEvent;
 use OCP\Cache\CappedMemoryCache;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Config\IMountProvider;
+use OCP\Files\Mount\IMountManager;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Storage\IStorageFactory;
 use OCP\ICacheFactory;
@@ -32,6 +33,7 @@ class MountProvider implements IMountProvider {
 		protected LoggerInterface $logger,
 		protected IEventDispatcher $eventDispatcher,
 		protected ICacheFactory $cacheFactory,
+		protected IMountManager $mountManager,
 	) {
 	}
 
@@ -58,12 +60,17 @@ class MountProvider implements IMountProvider {
 
 		$superShares = $this->buildSuperShares($shares, $user);
 
-		$mounts = [];
+		$mounts = $this->mountManager->getAll();
 		$view = new View('/' . $user->getUID() . '/files');
 		$ownerViews = [];
 		$sharingDisabledForUser = $this->shareManager->sharingDisabledForUser($user->getUID());
 		/** @var CappedMemoryCache<bool> $folderExistCache */
 		$foldersExistCache = new CappedMemoryCache();
+
+		$validShareCache = $this->cacheFactory->createLocal('share-valid-mountpoint-max');
+		$maxValidatedShare = $validShareCache->get($user->getUID()) ?? 0;
+		$newMaxValidatedShare = $maxValidatedShare;
+
 		foreach ($superShares as $share) {
 			try {
 				/** @var IShare $parentShare */
@@ -80,6 +87,7 @@ class MountProvider implements IMountProvider {
 				if (!isset($ownerViews[$owner])) {
 					$ownerViews[$owner] = new View('/' . $parentShare->getShareOwner() . '/files');
 				}
+				$shareId = (int)$parentShare->getId();
 				$mount = new SharedMount(
 					'\OCA\Files_Sharing\SharedStorage',
 					$mounts,
@@ -97,9 +105,11 @@ class MountProvider implements IMountProvider {
 					$foldersExistCache,
 					$this->eventDispatcher,
 					$user,
-					$this->cacheFactory->createLocal('share-valid-mountpoint')
+					($shareId <= $maxValidatedShare)
 				);
 
+				$newMaxValidatedShare = max($shareId, $newMaxValidatedShare);
+
 				$event = new ShareMountedEvent($mount);
 				$this->eventDispatcher->dispatchTyped($event);
 
@@ -118,6 +128,8 @@ class MountProvider implements IMountProvider {
 			}
 		}
 
+		$validShareCache->set($user->getUID(), $newMaxValidatedShare, 24 * 60 * 60);
+
 		// array_filter removes the null values from the array
 		return array_values(array_filter($mounts));
 	}

apps/files_sharing/lib/SharedMount.php

@@ -16,7 +16,6 @@ use OCP\Cache\CappedMemoryCache;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Events\InvalidateMountCacheEvent;
 use OCP\Files\Storage\IStorageFactory;
-use OCP\ICache;
 use OCP\IDBConnection;
 use OCP\IUser;
 use OCP\Server;
@@ -48,13 +47,19 @@ class SharedMount extends MountPoint implements MoveableMount, ISharedMountPoint
 		CappedMemoryCache $folderExistCache,
 		private IEventDispatcher $eventDispatcher,
 		private IUser $user,
-		private ICache $cache,
+		bool $alreadyVerified,
 	) {
 		$this->superShare = $arguments['superShare'];
 		$this->groupedShares = $arguments['groupedShares'];
 
-		$newMountPoint = $this->verifyMountPoint($this->superShare, $mountpoints, $folderExistCache);
-		$absMountPoint = '/' . $user->getUID() . '/files' . $newMountPoint;
+		$absMountPoint = '/' . $user->getUID() . '/files/' . trim($this->superShare->getTarget(), '/') . '/';
+
+		// after the mountpoint is verified for the first time, only new mountpoints (e.g. groupfolders can overwrite the target)
+		if (!$alreadyVerified || isset($mountpoints[$absMountPoint])) {
+			$newMountPoint = $this->verifyMountPoint($this->superShare, $mountpoints, $folderExistCache);
+			$absMountPoint = '/' . $user->getUID() . '/files/' . trim($newMountPoint, '/') . '/';
+		}
+
 		parent::__construct($storage, $absMountPoint, $arguments, $loader, null, null, MountProvider::class);
 	}
 
@@ -71,12 +76,6 @@ class SharedMount extends MountPoint implements MoveableMount, ISharedMountPoint
 		array $mountpoints,
 		CappedMemoryCache $folderExistCache,
 	) {
-		$cacheKey = $this->user->getUID() . '/' . $share->getId() . '/' . $share->getTarget();
-		$cached = $this->cache->get($cacheKey);
-		if ($cached !== null) {
-			return $cached;
-		}
-
 		$mountPoint = basename($share->getTarget());
 		$parent = dirname($share->getTarget());
 
@@ -105,8 +104,6 @@ class SharedMount extends MountPoint implements MoveableMount, ISharedMountPoint
 			$this->updateFileTarget($newMountPoint, $share);
 		}
 
-		$this->cache->set($cacheKey, $newMountPoint, 60 * 60);
-
 		return $newMountPoint;
 	}
 

apps/files_sharing/tests/MountProviderTest.php

@@ -11,6 +11,7 @@ use OC\Share20\Share;
 use OCA\Files_Sharing\MountProvider;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\IRootFolder;
+use OCP\Files\Mount\IMountManager;
 use OCP\Files\Storage\IStorageFactory;
 use OCP\ICacheFactory;
 use OCP\IConfig;
@@ -55,8 +56,9 @@ class MountProviderTest extends \Test\TestCase {
 		$cacheFactory = $this->createMock(ICacheFactory::class);
 		$cacheFactory->method('createLocal')
 			->willReturn(new NullCache());
+		$mountManager = $this->createMock(IMountManager::class);
 
-		$this->provider = new MountProvider($this->config, $this->shareManager, $this->logger, $eventDispatcher, $cacheFactory);
+		$this->provider = new MountProvider($this->config, $this->shareManager, $this->logger, $eventDispatcher, $cacheFactory, $mountManager);
 	}
 
 	private function makeMockShareAttributes($attrs) {