fix(media): return the exact window for non-aligned offsets
Same B9 defect as blog: Payload paginates by page, so a non-aligned offset returned the wrong window. Fetch the straddled pages and slice the intra-page remainder. Identical window tests now run against the mock and the fake-payload stub. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,12 +1,48 @@
|
|||||||
import { describe } from "vitest";
|
import { describe, expect, it } from "vitest";
|
||||||
import { RecordingTracer } from "@repo/core-testing/instrumentation";
|
import { RecordingTracer } from "@repo/core-testing/instrumentation";
|
||||||
import { MockMediaRepository } from "@/infrastructure/repositories/media.repository.mock";
|
import { MockMediaRepository } from "@/infrastructure/repositories/media.repository.mock";
|
||||||
import { mediaRepositoryContract } from "@/__contracts__/media-repository.contract";
|
import { mediaRepositoryContract } from "@/__contracts__/media-repository.contract";
|
||||||
|
import { mediaFactory } from "@/__factories__/media.factory";
|
||||||
|
|
||||||
describe("MockMediaRepository", () => {
|
describe("MockMediaRepository", () => {
|
||||||
const tracer = new RecordingTracer();
|
const tracer = new RecordingTracer();
|
||||||
mediaRepositoryContract.run(
|
mediaRepositoryContract.run(() => new MockMediaRepository(tracer), {
|
||||||
() => new MockMediaRepository(tracer),
|
tracer: () => tracer,
|
||||||
{ tracer: () => tracer },
|
});
|
||||||
);
|
});
|
||||||
|
|
||||||
|
describe("MockMediaRepository listMedia pagination windows", () => {
|
||||||
|
// Mirrors the window tests in media.repository.test.ts — both
|
||||||
|
// implementations must return identical windows for the same offsets.
|
||||||
|
async function buildSeededRepo() {
|
||||||
|
const repo = new MockMediaRepository();
|
||||||
|
for (let i = 1; i <= 5; i++) {
|
||||||
|
await repo._store(mediaFactory.build({ id: `m-${i}` }));
|
||||||
|
}
|
||||||
|
return repo;
|
||||||
|
}
|
||||||
|
|
||||||
|
it("returns the exact window for an aligned offset", async () => {
|
||||||
|
const repo = await buildSeededRepo();
|
||||||
|
const window = await repo.listMedia({ offset: 2, limit: 2 });
|
||||||
|
expect(window.map((m) => m.id)).toEqual(["m-3", "m-4"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns the exact window for a non-aligned offset", async () => {
|
||||||
|
const repo = await buildSeededRepo();
|
||||||
|
const window = await repo.listMedia({ offset: 3, limit: 2 });
|
||||||
|
expect(window.map((m) => m.id)).toEqual(["m-4", "m-5"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("non-aligned offset near the end returns only the remaining items", async () => {
|
||||||
|
const repo = await buildSeededRepo();
|
||||||
|
const window = await repo.listMedia({ offset: 4, limit: 3 });
|
||||||
|
expect(window.map((m) => m.id)).toEqual(["m-5"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("offset past the end returns an empty array", async () => {
|
||||||
|
const repo = await buildSeededRepo();
|
||||||
|
const window = await repo.listMedia({ offset: 7, limit: 3 });
|
||||||
|
expect(window).toEqual([]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ function buildPayloadStub() {
|
|||||||
const store = new Map<string, Record<string, unknown>>();
|
const store = new Map<string, Record<string, unknown>>();
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
_store: store,
|
||||||
find: vi.fn(
|
find: vi.fn(
|
||||||
async ({
|
async ({
|
||||||
limit,
|
limit,
|
||||||
@@ -31,17 +32,31 @@ function buildPayloadStub() {
|
|||||||
},
|
},
|
||||||
),
|
),
|
||||||
findByID: vi.fn(
|
findByID: vi.fn(
|
||||||
async ({ id }: { collection: string; id: string; overrideAccess?: boolean }) => {
|
async ({
|
||||||
|
id,
|
||||||
|
}: {
|
||||||
|
collection: string;
|
||||||
|
id: string;
|
||||||
|
overrideAccess?: boolean;
|
||||||
|
}) => {
|
||||||
const doc = store.get(String(id));
|
const doc = store.get(String(id));
|
||||||
if (!doc) {
|
if (!doc) {
|
||||||
const err = Object.assign(new Error(`Not found: ${id}`), { status: 404 });
|
const err = Object.assign(new Error(`Not found: ${id}`), {
|
||||||
|
status: 404,
|
||||||
|
});
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
return doc;
|
return doc;
|
||||||
},
|
},
|
||||||
),
|
),
|
||||||
delete: vi.fn(
|
delete: vi.fn(
|
||||||
async ({ id }: { collection: string; id: string; overrideAccess?: boolean }) => {
|
async ({
|
||||||
|
id,
|
||||||
|
}: {
|
||||||
|
collection: string;
|
||||||
|
id: string;
|
||||||
|
overrideAccess?: boolean;
|
||||||
|
}) => {
|
||||||
store.delete(String(id));
|
store.delete(String(id));
|
||||||
return { id };
|
return { id };
|
||||||
},
|
},
|
||||||
@@ -79,7 +94,9 @@ describe("MediaRepository", () => {
|
|||||||
it("returns undefined when Payload throws (not found)", async () => {
|
it("returns undefined when Payload throws (not found)", async () => {
|
||||||
const { getPayload } = await import("payload");
|
const { getPayload } = await import("payload");
|
||||||
(getPayload as ReturnType<typeof vi.fn>).mockResolvedValue({
|
(getPayload as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||||
findByID: vi.fn().mockRejectedValue(
|
findByID: vi
|
||||||
|
.fn()
|
||||||
|
.mockRejectedValue(
|
||||||
Object.assign(new Error("Not found"), { status: 404 }),
|
Object.assign(new Error("Not found"), { status: 404 }),
|
||||||
),
|
),
|
||||||
});
|
});
|
||||||
@@ -113,6 +130,51 @@ describe("MediaRepository", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("listMedia pagination windows", () => {
|
||||||
|
// Mirrors the window tests in media.repository.mock.test.ts — both
|
||||||
|
// implementations must return identical windows for the same offsets.
|
||||||
|
async function buildSeededRepo() {
|
||||||
|
const stub = buildPayloadStub();
|
||||||
|
for (let i = 1; i <= 5; i++) {
|
||||||
|
stub._store.set(`m-${i}`, {
|
||||||
|
id: `m-${i}`,
|
||||||
|
alt: `Media ${i}`,
|
||||||
|
url: `https://cdn.example.com/${i}.png`,
|
||||||
|
filename: `${i}.png`,
|
||||||
|
mimeType: "image/png",
|
||||||
|
filesize: 100,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
const { getPayload } = await import("payload");
|
||||||
|
(getPayload as ReturnType<typeof vi.fn>).mockResolvedValue(stub);
|
||||||
|
return new MediaRepository(stubPayloadConfig);
|
||||||
|
}
|
||||||
|
|
||||||
|
it("returns the exact window for an aligned offset", async () => {
|
||||||
|
const repo = await buildSeededRepo();
|
||||||
|
const window = await repo.listMedia({ offset: 2, limit: 2 });
|
||||||
|
expect(window.map((m) => m.id)).toEqual(["m-3", "m-4"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns the exact window for a non-aligned offset", async () => {
|
||||||
|
const repo = await buildSeededRepo();
|
||||||
|
const window = await repo.listMedia({ offset: 3, limit: 2 });
|
||||||
|
expect(window.map((m) => m.id)).toEqual(["m-4", "m-5"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("non-aligned offset near the end returns only the remaining items", async () => {
|
||||||
|
const repo = await buildSeededRepo();
|
||||||
|
const window = await repo.listMedia({ offset: 4, limit: 3 });
|
||||||
|
expect(window.map((m) => m.id)).toEqual(["m-5"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("offset past the end returns an empty array", async () => {
|
||||||
|
const repo = await buildSeededRepo();
|
||||||
|
const window = await repo.listMedia({ offset: 7, limit: 3 });
|
||||||
|
expect(window).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("listMedia", () => {
|
describe("listMedia", () => {
|
||||||
it("returns an array of mapped Media docs", async () => {
|
it("returns an array of mapped Media docs", async () => {
|
||||||
const { getPayload } = await import("payload");
|
const { getPayload } = await import("payload");
|
||||||
|
|||||||
@@ -81,14 +81,20 @@ export class MediaRepository implements IMediaRepository {
|
|||||||
this.logger.captureException(err, {
|
this.logger.captureException(err, {
|
||||||
tags: { feature: FEATURE, repo: REPO, method: "getMedia" },
|
tags: { feature: FEATURE, repo: REPO, method: "getMedia" },
|
||||||
});
|
});
|
||||||
span.setStatus("error", err instanceof Error ? err.message : String(err));
|
span.setStatus(
|
||||||
|
"error",
|
||||||
|
err instanceof Error ? err.message : String(err),
|
||||||
|
);
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
async listMedia(opts?: { limit?: number; offset?: number }): Promise<Media[]> {
|
async listMedia(opts?: {
|
||||||
|
limit?: number;
|
||||||
|
offset?: number;
|
||||||
|
}): Promise<Media[]> {
|
||||||
return this.tracer.startSpan(
|
return this.tracer.startSpan(
|
||||||
{
|
{
|
||||||
name: "media.listMedia",
|
name: "media.listMedia",
|
||||||
@@ -101,22 +107,43 @@ export class MediaRepository implements IMediaRepository {
|
|||||||
async (span) => {
|
async (span) => {
|
||||||
try {
|
try {
|
||||||
const payload = await getPayload({ config: this.config });
|
const payload = await getPayload({ config: this.config });
|
||||||
const result = await payload.find({
|
|
||||||
|
const limit = opts?.limit ?? 50;
|
||||||
|
const offset = opts?.offset ?? 0;
|
||||||
|
// Payload paginates by page, not offset. A non-aligned offset
|
||||||
|
// (offset % limit !== 0) straddles two pages, so fetch both and
|
||||||
|
// slice out the exact [offset, offset + limit) window.
|
||||||
|
const page = limit > 0 ? Math.floor(offset / limit) + 1 : 1;
|
||||||
|
const remainder = limit > 0 ? offset % limit : 0;
|
||||||
|
|
||||||
|
const findPage = (p: number) =>
|
||||||
|
payload.find({
|
||||||
collection: "media",
|
collection: "media",
|
||||||
limit: opts?.limit ?? 50,
|
limit,
|
||||||
page: opts?.offset
|
page: p,
|
||||||
? Math.floor(opts.offset / (opts.limit ?? 50)) + 1
|
|
||||||
: 1,
|
|
||||||
overrideAccess: true,
|
overrideAccess: true,
|
||||||
});
|
});
|
||||||
const items = result.docs.map((d) => mapDoc(d as PayloadMediaDoc));
|
|
||||||
|
const first = await findPage(page);
|
||||||
|
let docs = first.docs;
|
||||||
|
if (remainder > 0) {
|
||||||
|
if (docs.length === limit) {
|
||||||
|
const second = await findPage(page + 1);
|
||||||
|
docs = [...docs, ...second.docs];
|
||||||
|
}
|
||||||
|
docs = docs.slice(remainder, remainder + limit);
|
||||||
|
}
|
||||||
|
const items = docs.map((d) => mapDoc(d as PayloadMediaDoc));
|
||||||
span.setAttribute("count", items.length);
|
span.setAttribute("count", items.length);
|
||||||
return items;
|
return items;
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
this.logger.captureException(err, {
|
this.logger.captureException(err, {
|
||||||
tags: { feature: FEATURE, repo: REPO, method: "listMedia" },
|
tags: { feature: FEATURE, repo: REPO, method: "listMedia" },
|
||||||
});
|
});
|
||||||
span.setStatus("error", err instanceof Error ? err.message : String(err));
|
span.setStatus(
|
||||||
|
"error",
|
||||||
|
err instanceof Error ? err.message : String(err),
|
||||||
|
);
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -140,7 +167,10 @@ export class MediaRepository implements IMediaRepository {
|
|||||||
this.logger.captureException(err, {
|
this.logger.captureException(err, {
|
||||||
tags: { feature: FEATURE, repo: REPO, method: "deleteMedia" },
|
tags: { feature: FEATURE, repo: REPO, method: "deleteMedia" },
|
||||||
});
|
});
|
||||||
span.setStatus("error", err instanceof Error ? err.message : String(err));
|
span.setStatus(
|
||||||
|
"error",
|
||||||
|
err instanceof Error ? err.message : String(err),
|
||||||
|
);
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user