Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Aug 28, 2025
Merged

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

merged 6 commits into from
Aug 28, 2025

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Jul 11, 2025 •
edited by provokateurin
Loading

@skjnldsv skjnldsv requested review from a team as code owners July 11, 2025 15:08
@skjnldsv skjnldsv requested review from artonge, provokateurin, sorbaugh and susnux and removed request for a team July 11, 2025 15:08
@skjnldsv skjnldsv added this to the Nextcloud 32 milestone Jul 11, 2025
@skjnldsv skjnldsv mentioned this pull request Jul 11, 2025
Merged
4 tasks
@kesselb kesselb force-pushed the revert-53918-revert-53141-perf/files/setup-fs-basic-auth-request branch from bb67988 to 7762362 Compare July 11, 2025 17:00
@kesselb
Copy link
Collaborator

kesselb commented Jul 11, 2025

I've pushed a rebase and signed-off-by

@provokateurin provokateurin force-pushed the revert-53918-revert-53141-perf/files/setup-fs-basic-auth-request branch from 7762362 to 01a72cd Compare July 14, 2025 11:05
@provokateurin provokateurin marked this pull request as draft July 14, 2025 11:05
@provokateurin provokateurin added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 14, 2025
@provokateurin
Copy link
Member

Pushed the old commits for now, will check the test failure soon.

@provokateurin provokateurin mentioned this pull request Jul 14, 2025
Open
1 task
@come-nc
Copy link
Contributor

come-nc commented Aug 14, 2025

/compile rebase

come-nc
come-nc reviewed Aug 14, 2025
View reviewed changes
@nextcloud-command nextcloud-command force-pushed the revert-53918-revert-53141-perf/files/setup-fs-basic-auth-request branch from 01a72cd to 7651def Compare August 14, 2025 10:51
@come-nc
Copy link
Contributor

come-nc commented Aug 14, 2025

Also this PR deletes a file that is still used by theming integration tests:

    And logged in admin uploads theming image for "logo" from file "data/coloured-pattern-non-square.png"       # FeatureContext::loggedInAdminUploadsThemingImageForFromFile()
      Warning: fopen(data/coloured-pattern-non-square.png): Failed to open stream: No such file or directory in /home/runner/actions-runner/_work/server/server/build/integration/features/bootstrap/Theming.php line 36

@provokateurin
Copy link
Member

Also this PR deletes a file that is still used by theming integration tests:

This is only because the removed tests was the only case when I created the PR, but then another test started using the file on master, so it broke 🙈

@provokateurin provokateurin force-pushed the revert-53918-revert-53141-perf/files/setup-fs-basic-auth-request branch from 7651def to 6929f7b Compare August 19, 2025 07:45
@provokateurin provokateurin requested a review from come-nc August 19, 2025 07:45
@provokateurin provokateurin marked this pull request as ready for review August 28, 2025 08:02
@provokateurin
Copy link
Member

/compile rebase-amend

@nextcloud-command nextcloud-command force-pushed the revert-53918-revert-53141-perf/files/setup-fs-basic-auth-request branch from 1b2cfe3 to ce329da Compare August 28, 2025 08:20
@provokateurin
Copy link
Member

Nooo, there are conflicts again 😞

@provokateurin
Copy link
Member

/compile rebase-amend

@provokateurin @nextcloud-command
test(integration): Check ocs share permission for correct folder
d5c15d4 
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin @nextcloud-command
perf(base): Stop setting up the FS for every basic auth request
778c6c0 
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin @nextcloud-command
fix(core): Stop abusing the cache for avatar upload
5057d5f 
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin @nextcloud-command
refactor(Server): Deprecate \OCP\ICache service and replace it with a…
26ab3a6 
… local cache

Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin @nextcloud-command
chore: Remove unused \OC\Cache\File
eba1175 
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin @nextcloud-command
chore: Remove unused \OC\Cache\CappedMemoryCache
e38dfef 
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nextcloud-command nextcloud-command force-pushed the revert-53918-revert-53141-perf/files/setup-fs-basic-auth-request branch from ce329da to e38dfef Compare August 28, 2025 09:32
@skjnldsv skjnldsv disabled auto-merge August 28, 2025 12:12
@skjnldsv skjnldsv merged commit d785bcd into master Aug 28, 2025
234 of 247 checks passed
@skjnldsv skjnldsv deleted the revert-53918-revert-53141-perf/files/setup-fs-basic-auth-request branch August 28, 2025 12:12
@nickvergessen nickvergessen mentioned this pull request Aug 28, 2025
Closed
@nickvergessen
Copy link
Member

Reverting again as it breaks guest app users and various sharing cases with Talk.

@nickvergessen nickvergessen mentioned this pull request Aug 28, 2025
Merged
@nextcloud-bot nextcloud-bot mentioned this pull request Aug 28, 2025
Merged
@joshtrichards
Copy link
Member

Just saw this and noticed the prior two prior PRs attempted to remove the tests for OCP\Cache\CappedMemoryCache. But OCP\Cache\CappedMemoryCache is in use and not deprecated. Don't confuse it with OC\Cache\CappedMemoryCache (which was deprecated). See #33064

@provokateurin
Copy link
Member

Thanks @joshtrichards!

@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
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

@artonge artonge artonge approved these changes

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

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

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

@icewind1991 icewind1991 Awaiting requested review from icewind1991

Assignees

@provokateurin provokateurin

Labels

2. developing Work in progress performance 🚀

Projects

None yet

Milestone

Nextcloud 32

Development

Successfully merging this pull request may close these issues.

Basic auth requests always set up the filesystem

8 participants

@skjnldsv @kesselb @provokateurin @come-nc @nickvergessen @joshtrichards @susnux @artonge