Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions api/docs/photos-architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ In order of implementation:
- [x] **Race-safe quota reservation** — usage check + PENDING insert in one Serializable transaction, retried on serialization failure.
- [x] **Pending photo cleanup** — hourly sweeper deletes stale `PENDING` rows and S3 objects (default age: 24h).
- [x] **Per-user storage limit** — `User.storageLimitBytes` (default 5 GiB) replaces the global env cap, so billing can raise one account without a redeploy.
- [x] **Storage limit grants** — internal `PhotoStorageService.addStorageLimit()` for billing to raise one account. No HTTP route, no Stripe yet.
- [x] **Orphan reconciler** — daily scan deletes S3 objects under `photos/` that no `Photo` row references (§11).
- [x] **Unit tests** — service-level, mock `S3Service` and `PrismaService`.
- [x] **E2E tests** — controller-level, with auth + CASL.
Expand All @@ -234,7 +235,7 @@ In order of implementation:

- Cleanup sweeper for PENDING rows
- Idempotency keys
- Per-user paid storage upgrades (billing)
- Per-user paid storage upgrades — the internal grant method is in place (§9); the Stripe webhook and its idempotency are still future work
- Thumbnails
- CloudFront
- S3 Event-driven confirm
Expand All @@ -255,17 +256,20 @@ Over-quota uploads return **413 Payload Too Large** with message `Storage quota

### Raising a user's limit

The column is the hook for paid tiers. Billing (not built yet) will bump it per account from a payment webhook, so no HTTP endpoint exposes it:
`PhotoStorageService.addStorageLimit(userId, additionalBytes)` raises one account's ceiling and returns the new limit. It is an internal method: no HTTP route reaches it, and the service that owns the quota read owns the grant.

```ts
// Future: BillingService, driven by a payment webhook
await prisma.user.update({
where: { id: userId },
data: { storageLimitBytes: { increment: additionalBytes } },
});
const newLimitBytes = await photoStorageService.addStorageLimit(userId, purchasedBytes);
```

Until then support can do the same by hand: `UPDATE "User" SET "storageLimitBytes" = 10737418240 WHERE id = '…';`
- **Additive, never absolute.** A purchase grants capacity rather than declaring a total, so two overlapping grants accumulate. Prisma's `increment` compiles to one `UPDATE`, so there is no read-modify-write window for a grant to be lost in and no transaction is needed.
- **Rejects a non-positive or fractional increment** with 400 before touching the row. Lowering a limit is deliberately not offered; a downgrade needs its own method with its own rules about usage already above the new ceiling.
- **Unknown user** surfaces as 404: Prisma reports `P2025` for an update whose row is missing, recognised through the wrapped cause chain like the serialization failures above.
- **Every grant logs** `user.storage_limit.increased` with `audit: true` and both byte counts as strings, since bigints are not JSON-serializable.
- **Idempotency belongs to the caller.** A webhook redelivered twice grants twice; the payment layer must dedupe by event id.

Support can still do it by hand: `UPDATE "User" SET "storageLimitBytes" = 10737418240 WHERE id = '…';`

- **New accounts:** JIT provisioning in `UsersService.resolveByProviderSub()` relies on the column default; no code sets the limit.
- **Limit lowered below current usage:** new uploads fail with 413 until usage drops; existing photos stay.
Expand Down
89 changes: 88 additions & 1 deletion api/src/photos/photo-storage.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { randomUUID } from "node:crypto";
import { ConflictException, NotFoundException, PayloadTooLargeException } from "@nestjs/common";
import { BadRequestException, ConflictException, NotFoundException, PayloadTooLargeException } from "@nestjs/common";
import { Test, TestingModule } from "@nestjs/testing";
import { PhotoStatus, Prisma, PrismaClient } from "generated/prisma/client";
import { DeepMockProxy, mockDeep } from "jest-mock-extended";
Expand Down Expand Up @@ -148,6 +148,93 @@ describe("PhotoStorageService", () => {
});
});

describe("addStorageLimit", () => {
const newLimit = FREE_TIER_STORAGE_LIMIT_BYTES + TEN_GIB;

const recordNotFound = () =>
new Prisma.PrismaClientKnownRequestError("An operation failed because it depends on one or more records", {
code: "P2025",
clientVersion: "7.8.0",
});

beforeEach(() => {
prisma.user.update.mockResolvedValue({ storageLimitBytes: newLimit } as never);
});

it("increments the stored limit in one update and returns the new value", async () => {
await expect(service.addStorageLimit(userId, TEN_GIB)).resolves.toBe(newLimit);

expect(prisma.user.update).toHaveBeenCalledWith({
where: { id: userId },
data: { storageLimitBytes: { increment: TEN_GIB } },
select: { storageLimitBytes: true },
});
// The grant must not be read-modify-write: no lookup, no transaction.
expect(prisma.user.findUnique).not.toHaveBeenCalled();
expect(prisma.$transaction).not.toHaveBeenCalled();
});

it("accepts a plain number and converts it to a bigint increment", async () => {
await expect(service.addStorageLimit(userId, 1024)).resolves.toBe(newLimit);

expect(prisma.user.update).toHaveBeenCalledWith(
expect.objectContaining({ data: { storageLimitBytes: { increment: 1024n } } }),
);
});

it("logs an audit record with bigints stringified for JSON", async () => {
await service.addStorageLimit(userId, TEN_GIB);

expect(logger.info).toHaveBeenCalledWith(
{
event: "user.storage_limit.increased",
userId,
additionalBytes: TEN_GIB.toString(),
newLimitBytes: newLimit.toString(),
audit: true,
},
expect.any(String),
);
});

it.each([
["zero", 0n],
["a negative bigint", -1n],
["a negative number", -1024],
["a fractional number", 1.5],
])("rejects %s with 400 and touches no row", async (_label, value) => {
const grant = service.addStorageLimit(userId, value);

await expect(grant).rejects.toBeInstanceOf(BadRequestException);
await expect(grant).rejects.toThrow(PHOTO_SERVICE_ERRORS.INVALID_STORAGE_INCREMENT(String(value)));
expect(prisma.user.update).not.toHaveBeenCalled();
expect(logger.info).not.toHaveBeenCalled();
});

it("translates a missing user into 404", async () => {
prisma.user.update.mockRejectedValue(recordNotFound());

const grant = service.addStorageLimit(userId, TEN_GIB);

await expect(grant).rejects.toBeInstanceOf(NotFoundException);
await expect(grant).rejects.toThrow(USER_SERVICE_ERRORS.NOT_FOUND(userId));
expect(logger.info).not.toHaveBeenCalled();
});

it("recognises a P2025 wrapped by the driver adapter", async () => {
prisma.user.update.mockRejectedValue(new Error("update failed", { cause: recordNotFound() }));

await expect(service.addStorageLimit(userId, TEN_GIB)).rejects.toBeInstanceOf(NotFoundException);
});

it("rethrows unrelated database errors untouched", async () => {
const outage = new Error("connection reset");
prisma.user.update.mockRejectedValue(outage);

await expect(service.addStorageLimit(userId, TEN_GIB)).rejects.toBe(outage);
});
});

describe("reserveUploadBytes", () => {
// The transaction client is a distinct mock so the tests can prove that
// both the usage query and the insert go through it, not the root client.
Expand Down
56 changes: 54 additions & 2 deletions api/src/photos/photo-storage.service.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { ConflictException, Injectable, NotFoundException, PayloadTooLargeException } from "@nestjs/common";
import {
BadRequestException,
ConflictException,
Injectable,
NotFoundException,
PayloadTooLargeException,
} from "@nestjs/common";
import { PhotoStatus, Prisma } from "generated/prisma/client";
import { PinoLogger } from "nestjs-pino";
import { jitteredLinearBackoffMs, sleep } from "src/common/utils/async.utils";
import { isSerializationFailure } from "src/prisma/prisma.errors";
import { isRecordNotFound, isSerializationFailure } from "src/prisma/prisma.errors";
import { PrismaService } from "src/prisma/prisma.service";
import { USER_SERVICE_ERRORS } from "src/users/users.constants";
import {
Expand Down Expand Up @@ -111,6 +117,52 @@ export class PhotoStorageService {
}
}

/**
* Raises the account's ceiling by `additionalBytes` and returns the new limit.
*
* Additive on purpose: a purchase grants capacity rather than declaring a
* total, so two grants that overlap accumulate instead of overwriting each
* other. Prisma's `increment` is one UPDATE, so there is no read-modify-write
* window to lose a grant in, and no transaction is needed.
*
* Internal only. Billing calls this; nothing routes to it over HTTP, and
* lowering a limit is deliberately not offered here.
*/
async addStorageLimit(userId: string, additionalBytes: bigint | number): Promise<bigint> {
if (typeof additionalBytes === "number" && !Number.isInteger(additionalBytes)) {
throw new BadRequestException(PHOTO_SERVICE_ERRORS.INVALID_STORAGE_INCREMENT(String(additionalBytes)));
}

const increment = BigInt(additionalBytes);
if (increment <= 0n) {
throw new BadRequestException(PHOTO_SERVICE_ERRORS.INVALID_STORAGE_INCREMENT(increment.toString()));
}

try {
const { storageLimitBytes } = await this.prisma.user.update({
where: { id: userId },
data: { storageLimitBytes: { increment } },
select: { storageLimitBytes: true },
});

this.logger.info(
{
event: "user.storage_limit.increased",
userId,
additionalBytes: increment.toString(),
newLimitBytes: storageLimitBytes.toString(),
audit: true,
},
"User storage limit increased",
);

return storageLimitBytes;
} catch (error) {
if (isRecordNotFound(error)) throw new NotFoundException(USER_SERVICE_ERRORS.NOT_FOUND(userId));
throw error;
}
}

/**
* The uploader's own ceiling (`User.storageLimitBytes`). Billing raises it
* per account; there is no global override. Read through `db` so that inside
Expand Down
2 changes: 2 additions & 0 deletions api/src/photos/photos.constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,7 @@ export const PHOTO_SERVICE_ERRORS = {
READ_FORBIDDEN: (photoId: string) => `Not authorized to read photo with ID "${photoId}"`,
DELETE_FORBIDDEN: (photoId: string) => `Not authorized to delete photo with ID "${photoId}"`,
STORAGE_QUOTA_EXCEEDED: "Storage quota exceeded",
INVALID_STORAGE_INCREMENT: (value: string) =>
`Storage limit increase must be a positive whole number of bytes, received "${value}"`,
STORAGE_RESERVATION_CONFLICT: "Storage reservation conflicted with a concurrent upload, please retry",
};
24 changes: 23 additions & 1 deletion api/src/prisma/prisma.errors.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Prisma } from "generated/prisma/client";
import { isSerializationFailure } from "./prisma.errors";
import { isRecordNotFound, isSerializationFailure } from "./prisma.errors";

describe("isSerializationFailure", () => {
const knownRequestError = (code: string) =>
Expand Down Expand Up @@ -78,3 +78,25 @@ describe("isSerializationFailure", () => {
});
});
});

describe("isRecordNotFound", () => {
const knownRequestError = (code: string) =>
new Prisma.PrismaClientKnownRequestError("An operation failed", { code, clientVersion: "7.8.0" });

it("matches the code Prisma raises for a write against a missing row", () => {
expect(isRecordNotFound(knownRequestError("P2025"))).toBe(true);
});

it("matches it through a wrapping cause chain", () => {
expect(isRecordNotFound(new Error("update failed", { cause: knownRequestError("P2025") }))).toBe(true);
});

it.each([
["a serialization failure", knownRequestError("P2034")],
["a unique constraint violation", knownRequestError("P2002")],
["a plain error", new Error("connection reset")],
["null", null],
])("returns false for %s", (_label, error) => {
expect(isRecordNotFound(error)).toBe(false);
});
});
10 changes: 10 additions & 0 deletions api/src/prisma/prisma.errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const DRIVER_TRANSACTION_WRITE_CONFLICT = "TransactionWriteConflict";
// Postgres SQLSTATE 40001, the serialization failure behind both of the above.
const POSTGRES_SERIALIZATION_FAILURE = "40001";

// Prisma raises P2025 when a write targets a row that does not exist.
const PRISMA_RECORD_NOT_FOUND = "P2025";

// Wrapped errors nest a few levels deep. The bound also stops a cyclic cause
// chain from spinning forever.
const MAX_CAUSE_DEPTH = 5;
Expand Down Expand Up @@ -46,3 +49,10 @@ export const isSerializationFailure = (error: unknown): boolean =>
kind === DRIVER_TRANSACTION_WRITE_CONFLICT ||
originalCode === POSTGRES_SERIALIZATION_FAILURE,
);

/**
* True when a write targeted a row that does not exist. Callers translate this
* into their own not-found error, since only they know what the row represents.
*/
export const isRecordNotFound = (error: unknown): boolean =>
someCause(error, ({ code }) => code === PRISMA_RECORD_NOT_FOUND);
Loading