Skip to content

perf(base): Stop setting up the FS for every basic auth request#54803

Draft
provokateurin wants to merge 5 commits into
masterfrom
perf/base/basic-auth-fs-setup
Draft

perf(base): Stop setting up the FS for every basic auth request#54803
provokateurin wants to merge 5 commits into
masterfrom
perf/base/basic-auth-fs-setup

Conversation

@provokateurin

@provokateurin provokateurin commented Sep 2, 2025 •
edited by susnux
Loading

Copy link
Copy Markdown
Member

One more time, we're gonna celebrate 🎵 ✌️ (last time, I promise 🤞)

Needs nextcloud/guests#1400 to not break the Guests app.
Nothing needed to change on the server side, it's the exact same changes as with the last PR.

@provokateurin provokateurin added this to the Nextcloud 33 milestone Sep 2, 2025
@provokateurin provokateurin requested review from a team as code owners September 2, 2025 12:12
@provokateurin provokateurin requested review from nfebe, sorbaugh, susnux, szaimen and yemkareems and removed request for a team September 2, 2025 12:12
@provokateurin

provokateurin commented Sep 2, 2025

Copy link
Copy Markdown
Member Author

/compile

@provokateurin provokateurin mentioned this pull request Sep 2, 2025
Open
1 task
@nickvergessen

nickvergessen commented Sep 2, 2025 •
edited by provokateurin
Loading

Copy link
Copy Markdown
Member

@provokateurin provokateurin mentioned this pull request Sep 2, 2025
Draft
@provokateurin

provokateurin commented Sep 2, 2025

Copy link
Copy Markdown
Member Author

@nickvergessen nextcloud/spreed#15777

@provokateurin provokateurin marked this pull request as draft September 2, 2025 16:27
@szaimen szaimen requested review from come-nc and skjnldsv and removed request for szaimen September 3, 2025 09:04
@provokateurin

provokateurin commented Sep 8, 2025

Copy link
Copy Markdown
Member Author

Deck will also need a testing PR for this branch, as it relies on the same logic that is currently breaking Talk sharing tests (but I already have a solution): https://github.com/nextcloud/spreed/actions/runs/17404020365/job/49403633071?pr=15777

@provokateurin provokateurin force-pushed the perf/base/basic-auth-fs-setup branch from 6511d14 to 7500e26 Compare September 8, 2025 14:54
@provokateurin provokateurin mentioned this pull request Sep 8, 2025
Draft
@provokateurin

provokateurin commented Sep 8, 2025

Copy link
Copy Markdown
Member Author

nextcloud/deck#7219

@provokateurin

provokateurin commented Sep 9, 2025

Copy link
Copy Markdown
Member Author

I ran all the failing Talk and Deck integration tests locally, but they all pass 🙈 I double checked that I'm really using the correct branches, but it was all as intended.

This was referenced Sep 16, 2025
Merged
Merged
@provokateurin provokateurin mentioned this pull request Sep 16, 2025
Merged
@provokateurin provokateurin force-pushed the perf/base/basic-auth-fs-setup branch 4 times, most recently from aced6e3 to 0786ca8 Compare October 2, 2025 08:46
This was referenced Dec 4, 2025
Open
Open
@provokateurin provokateurin mentioned this pull request Jan 13, 2026
Merged
6 tasks
@provokateurin

provokateurin commented Jan 14, 2026

Copy link
Copy Markdown
Member Author

I think this PR could be unblocked by #57295, as I was stuck on the VerifyMountPointEvent last time 🎉

@blizzz blizzz modified the milestones: Nextcloud 33, Nextcloud 34 Feb 16, 2026
@provokateurin provokateurin force-pushed the perf/base/basic-auth-fs-setup branch from 0786ca8 to df380d0 Compare February 24, 2026 09:17
susnux
susnux reviewed May 15, 2026
View reviewed changes

@susnux susnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@provokateurin do we still need the cache mount then?

server/lib/private/Files/Mount/CacheMountProvider.php

Line 21 in 0a24ae5

class CacheMountProvider implements IMountProvider {

@provokateurin

provokateurin commented May 18, 2026

Copy link
Copy Markdown
Member Author

I'm not sure what you mean and how this is related 🤔

@susnux susnux removed this from the Nextcloud 34.0.1 milestone Jun 9, 2026
@CarlSchwan CarlSchwan force-pushed the perf/base/basic-auth-fs-setup branch from df380d0 to f73fb1f Compare June 10, 2026 15:09
CarlSchwan
CarlSchwan reviewed Jun 10, 2026
View reviewed changes
Comment thread apps/files_sharing/tests/TestCase.php
$this->userSession->completeLogin($user, ['loginName' => $uid, 'password' => $uid], false);

$this->rootFolder->newFolder('/' . $uid . '/files');
// Filesystem::initMountPoints($user);

@CarlSchwan CarlSchwan Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added in the original change, there was a lot of change in that method when rebasing so I commented it out. Not sure if this is still needed

@provokateurin provokateurin Jun 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was there to fix one explicit case where the filesystem needs to be setup for shares, in order to fix the placeholder paths from Talk and Deck.

@provokateurin @come-nc
fix(Share20\Manager): Explicitly setup the filesystem for the user
36c0b3d 
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin @come-nc
perf(base): Stop setting up the FS for every basic auth request
0374c24 
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin @come-nc
refactor(Server): Deprecate \OCP\ICache service and replace it with a…
f06a732 
… local cache

Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin @come-nc
chore: Remove unused \OC\Cache\File
dec8a7e 
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin @come-nc
fixup! fix(Share20\Manager): Explicitly setup the filesystem for the …
01ea5fa 
…user
@come-nc come-nc force-pushed the perf/base/basic-auth-fs-setup branch from f73fb1f to 01ea5fa Compare June 12, 2026 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@susnux susnux susnux approved these changes

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@come-nc come-nc come-nc approved these changes

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@nfebe nfebe Awaiting requested review from nfebe nfebe was automatically assigned from nextcloud/server-frontend

@yemkareems yemkareems Awaiting requested review from yemkareems yemkareems is a code owner automatically assigned from nextcloud/server-backend

@sorbaugh sorbaugh Awaiting requested review from sorbaugh sorbaugh is a code owner automatically assigned from nextcloud/server-backend

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

Assignees

No one assigned

Labels

3. to review Waiting for reviews performance 🚀

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Basic auth requests always set up the filesystem

8 participants

@provokateurin @nickvergessen @susnux @CarlSchwan @come-nc @joshtrichards @blizzz @nextcloud-bot