Review brief

Advanced45 min

A multi-tenant GraphQL API adds request batching to remove an N+1 query. Project IDs are unique only within a tenant, authorization is per project, and the database does not promise result order.

Behavioral contract

A new batcher is created per GraphQL request for exactly one viewer and tenant; no cache or pending work crosses requests.
Project identity is the composite tenantId plus projectId, and the database query is constrained to that tenant.
Each requested project is returned only when its own ID is in viewer.allowedProjectIds; missing and unauthorized projects both resolve to null.
Batch results align one-for-one with input keys in input order, including duplicate and missing IDs, regardless of database order.
One database call contains at most 100 distinct project IDs; duplicate loads share one fetched value, and excess distinct keys continue in later batches.
A database rejection rejects every pending load in that batch and the loader remains usable for a later batch.

Review instructions

1Review the loader's lifetime, authorization, alignment, and failure path.
2Treat batching efficiency as secondary to tenant isolation.
3Prioritize no more than five findings.

Learning objectives

• Scope batching and caching to a request identity
• Preserve key-to-result alignment in unordered batches
• Apply authorization to each loaded resource

src/project-batcher-types.tsTypeScript

@@ -0,0 +1,15 @@

1+export interface Viewer {

2+ userId: string;

3+ tenantId: string;

4+ allowedProjectIds: readonly string[];

5+}

7+export interface Project {

8+ id: string;

9+ tenantId: string;

10+ name: string;

11+}

12+

13+export interface ProjectDatabase {

14+ findByTenantAndIds(tenantId: string, ids: readonly string[]): Promise<Project[]>;

15+}

Reference solution

The findings below are ordered by priority.

Findings

1. Process-wide lifetime and its cache cross viewer boundaries

Location: src/project-batcher.ts:13-29,58-63

getProjectBatcher retains the first instance for the process, including its database, pending queue, and ID-only cache. Later requests can reuse a promise resolved for another tenant or viewer, and tenant-local IDs collide in that cache, creating a cross-request data leak.

Construct one loader in each GraphQL request context, bind it to that request's single viewer and tenant, and keep its cache within that lifetime.

2. Any project permission authorizes every loaded row

Location: src/project-batcher.ts:50-54

The check asks only whether the viewer's allowed list is non-empty. A viewer allowed to see one project can receive a different project returned in the same or later batch.

Check allowedProjectIds for the specific requested ID before returning its matching row.

3. Database order is mistaken for input-key order

Location: src/project-batcher.ts:44-54

The implementation pairs pending[index] with rows[index], but database ordering is unspecified and missing IDs change the result shape. Resolvers can receive the wrong project's data even before authorization is considered.

Index rows by their tenant-qualified ID and map every original pending entry back through that index in input order.

4. A database rejection strands every pending promise

Location: src/project-batcher.ts:39-55

#flush has no rejection handling, and callers intentionally discard the flush promise. If the query rejects, none of the stored reject callbacks run, cached promises remain pending forever, and the failure becomes unhandled.

Catch the batch failure, reject every captured entry, evict affected cache entries, and leave scheduling state ready for later loads.

5. A single database call exceeds the distinct-key cap

Location: src/project-batcher.ts:41-49

The flush sends every distinct pending ID in one query without the 100-ID cap. A wide GraphQL request can therefore create an oversized database parameter list instead of continuing excess keys in later batches.

Issue chunks of at most 100 distinct IDs and resolve each pending load from the indexed fetched values. The existing per-loader promise cache already lets repeated loads for one ID share that value.

Reasonable non-findings

Scheduling a microtask is a reasonable way to collect field loads from the same turn.
Returning null for both missing and unauthorized projects matches the non-enumeration contract.
Querying a batch of IDs in one call is the intended N+1 optimization once tenant scope and alignment are correct.