dotCMS / dotCMS/core

Batch folder permission resolution into a single round-trip per page

Open
#36,940 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

dotCMS: Content Drive Team : Scout
Dominant language
Java
Stars
970
Forks
486
Avg merge
3d 33m
Merged PRs (30d)
170

Description

Problem Statement

FolderAPIImpl.searchFolders resolves permissions with one PermissionAPI.filterCollection call per permission type. For a single page of folders that is up to five sequential calls:

Call Scope
PERMISSION_READ the full pre-pagination collection
PERMISSION_CAN_ADD_CHILDREN the page
PERMISSION_EDIT the page (only when includePermissions=true)
PERMISSION_PUBLISH the page (only when includePermissions=true)
PERMISSION_EDIT_PERMISSIONS the page (only when includePermissions=true)

Each call independently re-resolves the requesting user's roles (loadRolesForUser) before issuing its own getPermittedIds query, and getPermittedIds chunks the id list by 500. So the work scales with number of permission types × ceil(page size / 500), when the underlying data — the permission bitmask per folder id — could be fetched once.

A single batch returning the bitmask per id would collapse all of them into one round-trip, after which the per-type sets are pure bit tests.

Raised by @fabrizzio-dotCMS while reviewing #36889 (review comment, plus an inline note on the three adjacent calls).

Why it wasn't done in #36889

That PR added the three opt-in types and was already a REST contract change; reworking PermissionAPI's batch surface is a different blast radius. It was explicitly deferred rather than folded in.

Why it's worth doing

Beyond the round-trips, this would remove the reason the perPage cap exists. #36889 had to add content.drive.folder.search.permissions.max.per.page (default 200, 400 above it) precisely because per-type resolution makes large pages expensive. With one batch the cost stops scaling with the type count, and the cap — a user-visible 400 that the frontend has to code around — could plausibly be relaxed or dropped.

Suggested approach
  1. Add a batch method to PermissionAPI returning Map<String, Integer> (permission id → bitmask) for a collection of Permissionables, resolving the user's roles once.
  2. Rewrite FolderAPIImpl.resolvePagePermissions to call it once and derive the four id sets by bit test. PermissionAPI.Type.fromBitsAsNames(int) already exists but returns the canonical names, which include WRITE instead of EDIT — see the caveat below.
  3. Re-evaluate whether the perPage cap can be raised or removed. If removed, the frontend contract in #36595 changes, so coordinate before doing it.
  4. Consider whether the pre-pagination READ filter can ride the same batch — note it runs over the whole site's folders, not the page, so it may need separate treatment (see the related item below).
Watch out for
  • PermissionAPI.Type.getCanonicalTypes() / fromBitsAsNames emit WRITE, not EDIT. PERMISSION_EDIT == PERMISSION_WRITE == 2; the canonical set holds WRITE. The Content Drive frontend checks for EDIT, so deriving names from the canonical set would silently break context-menu gating with no error and no failing test. #36889 uses explicit Type.EDIT.name() for this reason and has a test asserting WRITE is never emitted — keep that test passing.
  • filterCollection short-circuits for CMS Admin and the system user (PermissionBitAPIImpl.java:1433-1434). Any test covering this must run as a limited, non-admin user or it exercises none of the logic. FolderAPIImplFilterTest already has that setup to copy.
  • The batch path is not cache-backed while the scalar path (getPermissionIdsFromRolesgetPermissions) is, via PermissionCache. Worth measuring rather than assuming the batch is strictly faster on a warm cache.
Acceptance Criteria
  • One permission round-trip per page instead of one per type.
  • GET /api/v1/folder/search?includePermissions=true returns the same five type names, spelled identically (EDIT, never WRITE).
  • FolderAPIImplFilterTest and FolderResourceSearchTest pass unchanged.
  • A decision recorded on whether the perPage cap stays, is raised, or is removed.
Related
  • Not to be confused with the other known inefficiency in this area: the Content Drive table path (DotFolderTransformerImpl.contentDriveView) resolves permissions per folder in a loop — a genuine N+1, different code path.
  • Also separate: the pre-pagination READ filter in FolderAPIImpl runs over every folder in the site rather than the requested page, so endpoint latency scales with site size. That is the endpoint's dominant cost and predates all of this.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with FolderAPIImpl.resolvePagePermissions and the PermissionAPI permission-resolution methods, then review PermissionBitAPIImpl around the CMS Admin and system-user short circuits. Use FolderAPIImplFilterTest and FolderResourceSearchTest with a limited non-admin user, preserving EDIT rather than WRITE in the response. Done means one permission round-trip per page, unchanged endpoint results, passing tests, and a recorded decision on the perPage cap.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api, backend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.