Fix handling of storage that has multiple shared mounts

[christoph-bessei]

Fixes #254

Description

As described in #254 there are some edge cases where $mounts[0]->getUser()->getUID() returns a user ID that has no access to a given file. This PR fixes this by iterating through the mounts until it finds a user with access to the given file.

There might be cleaner solutions, but I wanted to reduce the changes as much as possible for an easier review. Assuming $mounts[0] is working in most cases, the performance impact should be minimal.

Notes

• This is my first PR related to Nextcloud. Please let me know if I missed something that's required before merge.

🤖 AI (if applicable)

• The content of this PR was partly or fully generated using AI

[bakiburakogun]

Confirming this on a large instance, in case another data point helps it along.

We hit exactly the symptom from #254 on Nextcloud 34.0.3 with context_chat 5.4.0 and context_chat_backend 5.4.1: roughly 1.5 M eligible files, heavy use of shares and group folders, and a steady stream of

Exception: File not found or not a file
  at apps/context_chat/lib/Controller/QueueController.php:301
  in getFileSource()

on GET /ocs/v2.php/apps/context_chat/queues/documents.

What made it costly for us is what happens next to those entries. getDocumentsQueueItems() deletes the queue row when getFileSource() throws:

if ($queueMapper->lock($document->getId())) {
    try {
        $files[$document->getId()] = $this->getFileSource(...);
    } catch (\Exception $e) {
        $this->logger->warning($e->getMessage(), ['exception' => $e]);
        $queueMapper->delete($document);
    }
}

so the queue drained quickly while almost nothing reached the vector database — from the outside it looked like indexing had stopped, when in fact files were being consumed and discarded. Because the warning goes to the Nextcloud log rather than the backend container, it is easy to spend a long time looking in the wrong place.

With this patch applied, indexing ran to 32,879 documents and 490,387 chunks before we paused it for unrelated reasons, and the remaining per-file failures are the ordinary ones — Decoded content is empty for files with no text layer, and the occasional decode error on a malformed PDF.

One note on getFileSource() as it stands after this change: if none of the mounts can resolve the file, the exception is still thrown and the entry still deleted, which is right for a file that genuinely no longer exists but also silently drops anything that fails for a transient reason. That is outside the scope here, just worth being aware of.

Thanks for writing it — happy to test further revisions on an instance of this size if that is useful.

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[marcelklehr]

If the first mount's user has an inaccessible home folder, mounts 2…N are never tried, and the caller (QueueController.php:126-129) deletes the queue row. That's the exact failure shape the PR set out to fix, just moved from "wrong user" to "broken user". It should continue, and the throw should happen after the loop when nothing resolved.

[marcelklehr]

Worth considering: getMountsForStorageId is the wrong lookup — getMountsForFileId already does this. The loop is doing in PHP, with one getUserFolder() + getFirstNodeById() round trip per mount, what one SQL query already does by path prefix. getUsersForFileId() is called ten lines further down and already calls getMountsForFileId() for the same file id — so the correct mount list is fetched anyway, just after the loop that needed it.

What we could do instead:

$mounts = $userMountCache->getMountsForFileId($document->getFileId());
if (empty($mounts)) {
    throw new \Exception('Couldn\'t find any mounts for this file');
}
$userIds = array_map(static fn ($mount) => $mount->getUser()->getUID(), $mounts);

$file = null;
foreach ($userIds as $userId) {
    try {
        $file = $rootFolder->getUserFolder($userId)->getFirstNodeById($document->getFileId());
    } catch (NotPermittedException $e) {
        continue;
    }
    if ($file instanceof File) {
        break;
    }
}
if (!($file instanceof File)) {
    throw new \Exception('File not found or not a file');
}

$userIds then feeds the Source directly and the separate $storageService->getUsersForFileId() call drops out

[christoph-bessei]

Thanks a lot for your feedback! I wanted to keep the diff as small as possible, so I didn't fix the "broken user" case (your message above) or refactored the logic.

If you prefer a switch to a cleaner solution with getMountsForFileId, then I'm happy to implement & test this in the next days. Just let me know.

[marcelklehr]

I think that would be good :)