Skip to content

perf: cache shared view permission lookups#2586

Open
enjeck wants to merge 1 commit into
mainfrom
cache-permissions
Open

perf: cache shared view permission lookups#2586
enjeck wants to merge 1 commit into
mainfrom
cache-permissions

Conversation

@enjeck
Copy link
Copy Markdown
Contributor

@enjeck enjeck commented May 9, 2026

This improves /apps/tables/view performance for users with shared views by avoiding repeated permission/share resolution during one request. Added request-local caching in PermissionsService for user group IDs, user circle IDs, shared permissions per user/node and context permissions per user/node.

To see the problem:

  1. Create a table as alice.
  2. Add 17 rows.
  3. Create 8 views for that table.
  4. Share each view with a group containing bob.
  5. Log in as bob and load /index.php/apps/tables/.
  6. Check the GET /apps/tables/view XHR.

Now, the load time is improved.
Before: /apps/tables/view had slow startup, around 1.4s, with many repeated oc_tables_shares queries.

After: /apps/tables/view dropped to about 600ms. So we have a 2x speedup

Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>
@enjeck enjeck self-assigned this May 9, 2026
@enjeck enjeck requested a review from blizzz as a code owner May 9, 2026 05:37
@github-project-automation github-project-automation Bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Productivity team May 9, 2026
delete: $delete,
manage: $manage,
);
$this->sharedPermissionsByNode[$cacheKey] = clone $permissions;
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.

clone should not be necessary, or do i oversee something?

continue;
}
$sharedViews[$node['node_id']] = $this->find($node['node_id'], false, $userId);
$sharedViews[$node['node_id']] = $this->find($node['node_id'], true, $userId);
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.

Is this not needed through ViewController::indexSharedWithMe()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🧭 Planning evaluation (don't pick)

Development

Successfully merging this pull request may close these issues.

2 participants