Skip to content

perf(web): remove duplicate video reads and run dashboard queries concurrently - #2216

Merged
richiemcilroy merged 4 commits into
mainfrom
perf/request-dedupe-and-parallel-queries
Sep 3, 2026
Merged

perf(web): remove duplicate video reads and run dashboard queries concurrently#2216
richiemcilroy merged 4 commits into
mainfrom
perf/request-dedupe-and-parallel-queries

Conversation

@richiemcilroy

@richiemcilroy richiemcilroy commented Sep 3, 2026

Copy link
Copy Markdown
Member

Follow-up to #2213. Insights showed that after the index pass the remaining load is request shape, not the schema: select * from videos where id = ? runs ~770k times a day, and the two most-visited authenticated pages issue their queries one after another. This PR removes the duplicate reads and serialises less, with results and error ordering kept identical.

1. Policy checks reuse the row that was already loaded

  • Policy.withPolicy / withPublicPolicy run the policy before the wrapped effect, and both VideosPolicy.canView and isOwner load the video row that the effect then loads again. VideosPolicy now exposes canViewLoaded, isOwnerLoaded, getViewableById and getOwnedById, which fetch once and decide on the loaded row. buildCanView keeps its behaviour; the shared decision logic is factored into one place.
  • Switched: Videos.getByIdForViewing (share page metadata, preview, playlist), delete, duplicate, getDownloadInfo, getThumbnailURL, updateUploadProgress, and the five multipart upload handlers (presign-part is 44k requests/day). Outcomes are unchanged in every case: missing video still yields not-found, non-owner still yields 403, view rules and password prompts are evaluated on the same inputs as before.
  • The org/space membership pair inside canView was an Effect.all with no concurrency option, which runs sequentially; it now runs both queries concurrently.
  • verifyPasswordCandidates only reads video.id, so its parameter is now Pick<Video, "id">.

2. Share page

  • The page ran the full canView policy (with its own video read and sub-queries) and then a joined select of the same video. The joined select now runs first and canViewLoaded decides on that row. The tombstone filter moved from the WHERE to a check after the policy so a denied or password-gated video on a deleted org still resolves exactly as before. password and ownerId are read for the check and stripped before the row leaves the loader.
  • getDashboardData(user) was called only to read .spacesData for the share dialog, which pulled the whole dashboard dataset including a 5-queries-per-organization loop. It now calls getDashboardSpacesData, built from the same helpers, which needs the org lookup, the role lookup, the spaces query and the two org counts.
  • The owner's users row was re-fetched for the AI entitlement check even though the joined select already returns it in full.

3. Dashboard

  • getDashboardData is split into helpers shared with the share page. Independent queries now run concurrently: invites alongside the active-org work; role and notification lookup together; spaces, the "All org" entry counts and the user cap count together; and inside the per-organization loop the members, managers and owned-org reads run together, then the two count queries. The org settings row is taken from the organization row already in memory instead of a third query, and userPreferences comes from the user row that was passed in.
  • Caps page: total count, video list, folder list and org settings run together; the two shared-spaces lookups run together.

Deliberately not included

  • Memoising the Effect-side getCurrentUser per request: 17 files update the user row and read it again inside the same request, so that needs its own audit before it is safe.

Validation

  • pnpm typecheck (next typegen + tsc -b) clean.
  • Biome clean on all touched files.
  • videos-policy.test.ts extended with a parity table asserting canViewLoaded returns the same decision as canView across owner, anonymous, private, org and space membership, video and inherited passwords, and email-restriction scenarios, plus a test that the loaded variant never calls repo.getById. 52/52 pass.
  • Full web unit suite: 2,538 pass. The single failure, slack-app-manifest.test.ts, compares the checked-in slack-app-manifest.json against fixed expectations and touches nothing in this change; it is pre-existing.

Greptile Summary

This PR reduces redundant video and dashboard database reads while preserving existing authorization and response behavior.

  • Adds loaded-row video policy helpers and migrates viewing, ownership, download, thumbnail, deletion, duplication, progress, and multipart-upload paths to them.
  • Splits dashboard loading into reusable helpers and executes independent queries concurrently.
  • Reuses share-page video and owner rows for authorization and entitlement checks.
  • Adds parity tests for loaded-row viewing policies.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code regression identified.

The loaded-row policy paths preserve authorization and error outcomes, while the dashboard and caps concurrency changes retain required query dependencies and response shapes.

Important Files Changed

Filename Overview
packages/web-backend/src/Videos/VideosPolicy.ts Extracts the existing view decision into a loaded-row policy and adds single-read ownership/view helpers without changing established outcomes.
packages/web-backend/src/Videos/index.ts Migrates video operations to the single-read policy helpers and preserves not-found and authorization handling.
apps/web/app/api/upload/[...route]/multipart.ts Replaces duplicated ownership-policy reads across multipart handlers with the loaded-row ownership helper.
apps/web/app/(org)/dashboard/dashboard-data.ts Extracts dashboard query helpers, reuses already-loaded rows, and parallelizes independent reads while retaining dependency ordering.
apps/web/app/(org)/dashboard/caps/page.tsx Starts independent caps-page queries together and waits for their results before dependent shared-space processing.
apps/web/app/s/[videoId]/page.tsx Authorizes the joined video row directly, preserves tombstone and password ordering, narrows dashboard loading, and reuses the complete owner row for AI entitlement.
apps/web/tests/unit/videos-policy.test.ts Adds policy-parity coverage across ownership, visibility, membership, password, and email-restriction scenarios and verifies no duplicate repository read.
packages/web-domain/src/Video.ts Narrows the password-candidate verifier input type to the only video field it consumes.

Reviews (1): Last reviewed commit: "perf(share): load the share page with on..." | Re-trigger Greptile

@richiemcilroy
richiemcilroy merged commit ca34c3c into main Sep 3, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant