Skip to content

Eliminate redundant project DB fetches across route handlers#478

Merged
thehabes merged 10 commits intodevelopmentfrom
copilot/fix-redundant-project-data-query-again
Mar 16, 2026
Merged

Eliminate redundant project DB fetches across route handlers#478
thehabes merged 10 commits intodevelopmentfrom
copilot/fix-redundant-project-data-query-again

Conversation

Copy link
Contributor

Copilot AI commented Mar 16, 2026

Route handlers were loading project data twice per request: once via checkUserAccess() (which internally calls #load()), and again inside utility functions like findLayerById(), findPageById(), and ProjectFactory.loadAsUser().

Changes

utilities/shared.js

  • findPageById(pageId, projectId, project = null) — accepts optional pre-loaded Project; skips Project.getById() when provided
  • findLayerById(layerId, projectId, project = null) — same pattern

Route handlers — pass pre-loaded project to utilities

  • layer/index.js PUT /:layerIdfindLayerById() + findPageById()
  • page/index.js PUT /:pageIdfindPageById()
  • line/index.js POST /, PUT /:lineId, PATCH /:lineId/text, PATCH /:lineId/boundsfindPageById()
// Before — two DB fetches for the same project
const project = new Project(projectId)
await project.checkUserAccess(...)      // fetch #1
const layer = await findLayerById(layerId, projectId)  // fetch #2 inside

// After — one fetch
const layer = await findLayerById(layerId, projectId, project)  // reuses project.data

project/customMetadataRouter.js — GET/POST/PUT /:id/custom

  • Replace database.findOne({ _id: id }, "projects") with project.data already populated by checkUserAccess()

project/projectReadRouter.js

  • GET /:id — calls ProjectFactory.loadAsUser() once (the aggregation that fetches project + group + collaborators) and derives the access check from its result via a new local userHasAccess() helper, replacing the separate checkUserAccess() + loadAsUser() pair
  • GET /:id/manifest — removes the standalone loadAsUser() existence-check call; uses project.data from checkUserAccess() and passes it to exportManifest()

classes/Project/ProjectFactory.js

  • exportManifest(projectId, preloadedProjectData = null) — skips internal loadAsUser() when project data is already available
Original prompt

This section details on the original issue you should resolve

<issue_title>Redundant Project Data Query</issue_title>
<issue_description>## Summary

Multiple route handlers load project data twice per request. The first load happens via checkUserAccess() (which internally calls #load()), and the second happens when a utility function like findLayerById(), findPageById(), or ProjectFactory.loadAsUser() internally calls Project.getById() again.

Root Cause

Utility functions always fetch the project internally with no way to accept a pre-loaded project object. When a route handler has already loaded the project for access control, the subsequent utility call duplicates that database query.

Proposed Fix

Add an optional project parameter to utility functions so callers can pass a pre-loaded project when available. Callers without a pre-loaded project continue to work unchanged by omitting the parameter.

Confirmed Double-Fetch Routes (11)

layer/index.js

Method Route Second fetch via
PUT /:layerId findLayerById()Project.getById()

page/index.js

Method Route Second fetch via
PUT /:pageId findPageById()Project.getById()

line/index.js

Method Route Second fetch via
POST / findPageById()Project.getById()
PUT /:lineId findPageById()Project.getById()
PATCH /:lineId/text findPageById()Project.getById()
PATCH /:lineId/bounds findPageById()Project.getById()

project/projectReadRouter.js

Method Route Second fetch via
GET /:id ProjectFactory.loadAsUser()
GET /:id/manifest ProjectFactory.loadAsUser()

project/customMetadataRouter.js

Method Route Second fetch via
GET /:id/custom database.findOne()
POST /:id/custom database.findOne()
PUT /:id/custom database.findOne()

Potential Double-Fetches (5, depends on ProjectFactory internals)

project/projectCopyRouter.js

  • POST /:projectId/copy — via ProjectFactory.copyProject()
  • POST /:projectId/copy-without-annotations — via ProjectFactory.cloneWithoutAnnotations()
  • POST /:projectId/copy-with-group — via ProjectFactory.cloneWithGroup()
  • POST /:projectId/copy-with-customizations — via ProjectFactory.cloneWithCustomizations()

project/projectReadRouter.js

  • GET /:id/deploymentStatus — via ProjectFactory.checkManifestUploadAndDeployment()</issue_description>

Comments on the Issue (you are @copilot in this section)


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI and others added 2 commits March 16, 2026 18:45
- Add optional `project` param to `findPageById()` and `findLayerById()` in shared.js
  so callers with a pre-loaded project can skip the redundant DB fetch
- Pass pre-loaded `project` to `findLayerById()` and `findPageById()` in layer/index.js
- Pass pre-loaded `project` to `findPageById()` in page/index.js PUT /:pageId
- Pass pre-loaded `project` to `findPageById()` in line/index.js (POST, PUT, PATCH×2)
- Use `project.data` directly in customMetadataRouter.js GET/POST/PUT /:id/custom
  instead of calling `database.findOne()` again after `checkUserAccess()` already loaded it
- Restructure projectReadRouter.js GET /:id to call `loadAsUser()` once and derive the
  access check from its result via new `userHasAccess()` helper (saves project+group load)
- Remove redundant `loadAsUser()` call in projectReadRouter.js GET /:id/manifest;
  use `project.data` from `checkUserAccess()` for the existence check instead
- Add optional `preloadedProjectData` param to `ProjectFactory.exportManifest()` so
  callers with already-loaded data skip the internal `loadAsUser()` fetch

Co-authored-by: thehabes <[email protected]>
@thehabes thehabes linked an issue Mar 16, 2026 that may be closed by this pull request
…mment on loadAsUser error pattern

Co-authored-by: thehabes <[email protected]>
Copilot AI changed the title [WIP] Fix redundant project data query in utility functions Eliminate redundant project DB fetches across route handlers Mar 16, 2026
Copilot AI requested a review from thehabes March 16, 2026 18:53

This comment was marked as outdated.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces redundant project database reads by allowing route handlers and manifest export logic to reuse project data that was already loaded during access checks, rather than fetching the same project again inside shared utilities and factory methods.

Changes:

  • Add optional project parameter to findPageById() / findLayerById() to reuse pre-loaded Project instances.
  • Update multiple layer/page/line route handlers to pass the pre-loaded project into these utilities.
  • Avoid redundant project loads in project read/manifest/custom-metadata routes by reusing project.data and by extending ProjectFactory.exportManifest() to accept preloaded project data.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
utilities/shared.js Allow page/layer lookup helpers to reuse already-loaded project data.
layer/index.js Pass preloaded project into layer/page lookup utilities during updates.
page/index.js Pass preloaded project into findPageById() on update.
line/index.js Pass preloaded project into findPageById() across create/update/patch routes.
project/customMetadataRouter.js Reuse project.data instead of a second direct DB fetch.
project/projectReadRouter.js Use loadAsUser() once and perform access checking from the returned data to avoid double fetch.
classes/Project/ProjectFactory.js Let exportManifest() accept preloaded project data to avoid an internal loadAsUser() query.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +111 to +114
return userRoleNames.some(role => {
const perms = rolePermissions[role]
if (!perms || !Array.isArray(perms)) return false
return perms.some(perm => {
@thehabes thehabes marked this pull request as ready for review March 16, 2026 22:44
@thehabes thehabes requested a review from cubap as a code owner March 16, 2026 22:44
@thehabes thehabes merged commit 8e5e4ba into development Mar 16, 2026
1 check passed
@thehabes thehabes deleted the copilot/fix-redundant-project-data-query-again branch March 16, 2026 22:44
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.

Redundant Project Data Query

3 participants