Skip to content
Closed
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
69 changes: 61 additions & 8 deletions apps/web/hooks/__tests__/use-optimizer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ interface CompressCall {
job: ImageJob;
options: CompressionOptions;
deferred: Deferred<ImageResult>;
signal?: AbortSignal;
}

// Populated in call order by both compress mocks below; tests resolve/reject
Expand All @@ -52,9 +53,9 @@ let compressCalls: CompressCall[] = [];
let jobCounter = 0;

const compressWithApiMock = mock(
(job: ImageJob, options: CompressionOptions) => {
(job: ImageJob, options: CompressionOptions, signal?: AbortSignal) => {
const deferred = createDeferred<ImageResult>();
compressCalls.push({ deferred, job, options });
compressCalls.push({ deferred, job, options, signal });
return deferred.promise;
}
);
Expand Down Expand Up @@ -85,12 +86,26 @@ const makeFakeJob = (file: File): ImageJob => {
};
};

const ingestFilesMock = mock((files: File[]) =>
Promise.resolve({
jobs: files.map(makeFakeJob),
messages: [],
})
);
interface IngestCall {
files: File[];
startCount: number;
deferred: Deferred<{ jobs: ImageJob[]; messages: string[] }>;
}

// Populated in call order; each entry auto-resolves immediately unless
// autoResolveIngest is turned off, letting a test hold an ingest open to
// observe what the next addFiles call sees.
let ingestCalls: IngestCall[] = [];
let autoResolveIngest = true;

const ingestFilesMock = mock((files: File[], startCount: number) => {
const deferred = createDeferred<{ jobs: ImageJob[]; messages: string[] }>();
ingestCalls.push({ deferred, files, startCount });
if (autoResolveIngest) {
deferred.resolve({ jobs: files.map(makeFakeJob), messages: [] });
}
return deferred.promise;
});

mock.module("@/lib/compress/api", () => ({
compressWithApi: compressWithApiMock,
Expand Down Expand Up @@ -137,6 +152,8 @@ const makeFile = (name: string) =>

beforeEach(() => {
compressCalls = [];
ingestCalls = [];
autoResolveIngest = true;
jobCounter = 0;
resultUrlCounter = 0;
});
Expand Down Expand Up @@ -192,6 +209,7 @@ describe("useOptimizer", () => {
result.current.actions.applyOptions();
});
expect(compressCalls).toHaveLength(2);
expect(compressCalls[0]?.signal?.aborted).toBe(true);

const staleResult = makeResult({ url: "blob:stale-result" });
await act(async () => {
Expand Down Expand Up @@ -234,6 +252,7 @@ describe("useOptimizer", () => {
result.current.actions.removeJob(jobId);
});
expect(result.current.state.jobs).toHaveLength(0);
expect(compressCalls[0]?.signal?.aborted).toBe(true);

const staleResult = makeResult({ url: "blob:removed-result" });
await act(async () => {
Expand Down Expand Up @@ -322,4 +341,38 @@ describe("useOptimizer", () => {
jest.useRealTimers();
}
});

it("counts a still-ingesting drop toward the file-count gate seen by the next one", async () => {
const { result } = renderHook(() => useOptimizer());
autoResolveIngest = false;

let firstAddFiles!: Promise<void>;
let secondAddFiles!: Promise<void>;

act(() => {
firstAddFiles = result.current.actions.addFiles([
makeFile("a.jpg"),
makeFile("b.jpg"),
]);
});
act(() => {
secondAddFiles = result.current.actions.addFiles([makeFile("c.jpg")]);
});

expect(ingestCalls).toHaveLength(2);
expect(ingestCalls[0]?.startCount).toBe(0);
expect(ingestCalls[1]?.startCount).toBe(2);

await act(async () => {
ingestCalls[0]?.deferred.resolve({
jobs: (ingestCalls[0]?.files ?? []).map(makeFakeJob),
messages: [],
});
ingestCalls[1]?.deferred.resolve({
jobs: (ingestCalls[1]?.files ?? []).map(makeFakeJob),
messages: [],
});
await Promise.all([firstAddFiles, secondAddFiles]);
});
});
});
55 changes: 49 additions & 6 deletions apps/web/hooks/use-optimizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,26 @@ import type { CompressionOptions, ImageJob } from "@/lib/image/types";

export type FilterTab = "all" | "optimized" | "errors";

const describeCompressionError = (error: unknown): string => {
if (error instanceof DOMException && error.name === "TimeoutError") {
return "The API did not respond in time.";
}
if (error instanceof Error) {
return error.message;
}
return "Compression failed";
};

export const useOptimizer = () => {
const jobsRef = useRef<ImageJob[]>([]);
const optionsRef = useRef<CompressionOptions>({
...DEFAULT_COMPRESSION_OPTIONS,
});
// addFiles reads jobsRef.current.length before an await; a second drop
// during a slow ingest would otherwise see the pre-ingest count and bypass
// the MAX_FILES ceiling. This tracks files already claimed but not yet
// committed to jobsRef.
const pendingIngestRef = useRef(0);
const qualityApplyTimerRef = useRef<ReturnType<typeof setTimeout> | null>(
null
);
Expand All @@ -32,6 +47,15 @@ export const useOptimizer = () => {
if (generationRef.current === (null as unknown as Map<string, number>)) {
generationRef.current = new Map();
}
const abortControllersRef = useRef<Map<string, AbortController>>(
null as unknown as Map<string, AbortController>
);
if (
abortControllersRef.current ===
(null as unknown as Map<string, AbortController>)
) {
abortControllersRef.current = new Map();
}
const [jobs, setJobs] = useState<ImageJob[]>([]);
const [selectedId, setSelectedId] = useState<string | null>(null);
const [options, setOptions] = useState<CompressionOptions>({
Expand Down Expand Up @@ -114,6 +138,7 @@ export const useOptimizer = () => {
);

const invalidateJob = useCallback((id: string) => {
abortControllersRef.current.get(id)?.abort();
const nextGeneration = (generationRef.current.get(id) ?? 0) + 1;
generationRef.current.set(id, nextGeneration);
return nextGeneration;
Expand Down Expand Up @@ -153,13 +178,16 @@ export const useOptimizer = () => {
status: "processing",
}));

const controller = new AbortController();
abortControllersRef.current.set(job.id, controller);

try {
const result = shouldUseBrowserEncoder(
job.inputFormat,
resolvedOptions.outputFormat
)
? await compressWithBrowser(job, resolvedOptions)
: await compressWithApi(job, resolvedOptions);
: await compressWithApi(job, resolvedOptions, controller.signal);

if (isJobRunStale(job.id, generation)) {
URL.revokeObjectURL(result.url);
Expand All @@ -177,9 +205,11 @@ export const useOptimizer = () => {
return;
}

const message = describeCompressionError(error);

updateJob(job.id, (current) => ({
...current,
error: error instanceof Error ? error.message : "Compression failed",
error: message,
status: "error",
}));
}
Expand Down Expand Up @@ -212,11 +242,18 @@ export const useOptimizer = () => {
const addFiles = useCallback(
async (fileList: File[] | FileList) => {
setNotice(null);
const { jobs: nextJobs, messages } = await ingestFiles(
[...fileList],
jobsRef.current.length
);
const claimed = fileList.length;
const startCount = jobsRef.current.length + pendingIngestRef.current;
pendingIngestRef.current += claimed;

let ingestResult: Awaited<ReturnType<typeof ingestFiles>>;
try {
ingestResult = await ingestFiles([...fileList], startCount);
} finally {
pendingIngestRef.current -= claimed;
}

const { jobs: nextJobs, messages } = ingestResult;
if (messages.length > 0) {
setNotice(messages.slice(0, 3).join(" "));
}
Expand Down Expand Up @@ -303,6 +340,8 @@ export const useOptimizer = () => {
currentSelected === id ? (remaining[0]?.id ?? null) : currentSelected
);

abortControllersRef.current.get(id)?.abort();
abortControllersRef.current.delete(id);
generationRef.current.delete(id);
}, []);

Expand All @@ -315,6 +354,10 @@ export const useOptimizer = () => {
setSelectedId(null);
setOptionsDirty(false);
setNotice(null);
for (const controller of abortControllersRef.current.values()) {
controller.abort();
}
abortControllersRef.current.clear();
generationRef.current.clear();
}, []);

Expand Down
10 changes: 9 additions & 1 deletion apps/web/lib/compress/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,14 @@ const resultFromBlob = (
};
};

// The server's write timeout is 90s (apps/api/main.go); 120s means the client
// only gives up after the server certainly has.
const API_TIMEOUT_MS = 120_000;

export const compressWithApi = async (
job: ImageJob,
options: CompressionOptions
options: CompressionOptions,
signal?: AbortSignal
): Promise<ImageResult> => {
const form = new FormData();
form.append("file", job.file, job.name);
Expand All @@ -68,6 +73,9 @@ export const compressWithApi = async (
const response = await fetch(`${apiBase}/compress`, {
body: form,
method: "POST",
signal: signal
? AbortSignal.any([signal, AbortSignal.timeout(API_TIMEOUT_MS)])
: AbortSignal.timeout(API_TIMEOUT_MS),
});

if (!response.ok) {
Expand Down
48 changes: 48 additions & 0 deletions apps/web/lib/image/__tests__/constants.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,52 @@ describe("sanitizeCompressionOptions", () => {
}).outputFormat
).toBe(DEFAULT_COMPRESSION_OPTIONS.outputFormat);
});

it("clamps resize dimensions to MAX_DIMENSION", () => {
expect(
sanitizeCompressionOptions({
maintainAspect: true,
outputFormat: "jpeg",
quality: 50,
resizeEnabled: true,
resizeHeight: 999_999,
resizeWidth: 999_999,
})
).toMatchObject({
resizeHeight: MAX_DIMENSION,
resizeWidth: MAX_DIMENSION,
});
});

it("floors negative or invalid resize dimensions to 0", () => {
expect(
sanitizeCompressionOptions({
maintainAspect: true,
outputFormat: "jpeg",
quality: 50,
resizeEnabled: true,
resizeHeight: Number.NaN,
resizeWidth: -5,
})
).toMatchObject({
resizeHeight: 0,
resizeWidth: 0,
});
});

it("rounds fractional resize dimensions", () => {
expect(
sanitizeCompressionOptions({
maintainAspect: true,
outputFormat: "jpeg",
quality: 50,
resizeEnabled: true,
resizeHeight: 800.6,
resizeWidth: 800.6,
})
).toMatchObject({
resizeHeight: 801,
resizeWidth: 801,
});
});
});
10 changes: 10 additions & 0 deletions apps/web/lib/image/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ export const normalizeQuality = (quality: number) => {
return Math.min(100, Math.max(1, rounded));
};

const normalizeDimension = (value: number) => {
const rounded = Math.round(value);
if (!Number.isFinite(rounded) || rounded <= 0) {
return 0;
}
return Math.min(MAX_DIMENSION, rounded);
};

export const sanitizeCompressionOptions = (
options: CompressionOptions
): CompressionOptions => ({
Expand All @@ -41,4 +49,6 @@ export const sanitizeCompressionOptions = (
? options.outputFormat
: DEFAULT_COMPRESSION_OPTIONS.outputFormat,
quality: normalizeQuality(options.quality),
resizeHeight: normalizeDimension(options.resizeHeight),
resizeWidth: normalizeDimension(options.resizeWidth),
});
12 changes: 10 additions & 2 deletions apps/web/lib/image/ingest.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { readPreview } from "@/lib/compress/browser";
import { MAX_CONCURRENT_JOBS, runWithConcurrency } from "@/lib/compress/pool";
import { parseJpegExif } from "@/lib/image/exif";
import { formatFromMime } from "@/lib/image/format";
import type { ImageFormat, ImageJob } from "@/lib/image/types";
Expand Down Expand Up @@ -87,8 +88,15 @@ export const ingestFiles = async (
): Promise<IngestResult> => {
const messages: string[] = [];
const accepted = acceptFiles(files, startCount, messages);
const outcomes = await Promise.all(
accepted.map(({ file, inputFormat }) => jobFromFile(file, inputFormat))
const outcomes: (ImageJob | string)[] = Array.from({
length: accepted.length,
});
await runWithConcurrency(
accepted.map((entry, index) => ({ entry, index })),
MAX_CONCURRENT_JOBS,
async ({ entry, index }) => {
outcomes[index] = await jobFromFile(entry.file, entry.inputFormat);
}
);

const jobs: ImageJob[] = [];
Expand Down