From b54039f9a76953608e95ffcf45b786a841431762 Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Wed, 2 Sep 2026 18:05:15 +0100 Subject: [PATCH 1/2] fix(storage-resize-images): restore the extension's default for an omitted deleteOriginal The extension mapped every DELETE_ORIGINAL_FILE value other than "true"/"false" (unset included) to delete-on-success; the kit resolved an omitted deleteOriginal to never-delete. Remove the kit-only 'case undefined' so programmatic omission resolves to on_success, matching the extension and the kit's own empty-string env path. Deploys are unaffected: the CLI writes the declared "false" default into .env. Per maintainer ruling, isAnimated keeps its programmatic default of true: the extension's unset-env false came from a shipped parser bug ('overrideIsAnimated === "true" || undefined' made the yaml-intended true unreachable), and the kit fixes the bug rather than reproducing it. The README now documents both behaviors. Fixes #3024 --- kits/storage-resize-images/CHANGELOG.md | 1 + kits/storage-resize-images/README.md | 16 ++++++ .../src/export-config.ts | 1 - .../tests/export-config.test.ts | 14 ++++- .../tests/handlers.test.ts | 55 +++++++++++++++++-- 5 files changed, 78 insertions(+), 9 deletions(-) diff --git a/kits/storage-resize-images/CHANGELOG.md b/kits/storage-resize-images/CHANGELOG.md index bddce1dca9..bab9787940 100644 --- a/kits/storage-resize-images/CHANGELOG.md +++ b/kits/storage-resize-images/CHANGELOG.md @@ -1,2 +1,3 @@ +- fix: restore the extension's default for an omitted `deleteOriginal`. The extension mapped every `DELETE_ORIGINAL_FILE` value other than `"true"`/`"false"` (unset included) to delete-on-success; the kit resolved an omitted `deleteOriginal` to never-delete. `resolveResizeImagesConfig` now resolves an omitted `deleteOriginal` to delete-on-success, matching the extension. Only library consumers who omit the field are affected; deploys are unaffected (the Firebase CLI writes the declared `"false"` default into `.env`, and an env var that is present resolves as before). Pass `deleteOriginal: "false"` to keep originals. - fix: restore the `us-central1` content-filter fallback. `checkImageContent` threw `FUNCTION_REGION is required for Vertex AI filtering.` when no region was available; the extension fell back to `us-central1`. The Vertex AI call now uses the function's region when known and `us-central1` otherwise, matching the extension. Normal CLI deploys were unaffected (the Firebase CLI sets `FUNCTION_REGION` on deployed functions); the throw was reachable for library consumers, emulator runs, and hand-rolled environments. - Initial release of kit, see README for differences between the legacy extension and this kit diff --git a/kits/storage-resize-images/README.md b/kits/storage-resize-images/README.md index ab13f0e79d..752a8df600 100644 --- a/kits/storage-resize-images/README.md +++ b/kits/storage-resize-images/README.md @@ -102,6 +102,12 @@ loads them at deploy time and prompts for any required values that are missing. | `customFilterPrompt` | `CUSTOM_FILTER_PROMPT` | no | (empty) | Custom filter prompt | | `placeholderImagePath` | `PLACEHOLDER_IMAGE_PATH` | no | (empty) | Placeholder for filtered images | +The `deleteOriginal` default above is what the CLI writes into `.env` at deploy +time. Omitting `deleteOriginal` when calling `resolveResizeImagesConfig` +directly deletes the original on a successful resize, matching how the +extension treated an unset `DELETE_ORIGINAL_FILE`; pass `"false"` to keep +originals. + ## Multiple instances To resize images from several buckets or pipelines, add one entry per instance @@ -180,6 +186,16 @@ the extension is installed. A malformed value fails the deploy with `Invalid includePathList: must be a comma-separated list of absolute path values.` rather than being rejected by an install prompt. +### An omitted `isAnimated` keeps animation + +The extension's config parser had a bug: `overrideIsAnimated === "true" || +undefined` never evaluated the intended unset check, so an unset `IS_ANIMATED` +produced first-frame-only output even though the parameter's declared default +was `true`. The kit deliberately fixes this rather than reproducing it: an +omitted `isAnimated` resolves to `true`, the default the extension intended. +Deploys are unaffected either way, since the CLI writes `IS_ANIMATED=true` +into `.env`; pass `isAnimated: false` for first-frame-only output. + ### No backfill There is no function to resize images that already exist in the bucket. The diff --git a/kits/storage-resize-images/src/export-config.ts b/kits/storage-resize-images/src/export-config.ts index 9454e020e2..98ece3f12e 100644 --- a/kits/storage-resize-images/src/export-config.ts +++ b/kits/storage-resize-images/src/export-config.ts @@ -127,7 +127,6 @@ function deleteOriginalFile( return DELETE_IMAGE.always; case false: case "false": - case undefined: return DELETE_IMAGE.never; default: return DELETE_IMAGE.onSuccess; diff --git a/kits/storage-resize-images/tests/export-config.test.ts b/kits/storage-resize-images/tests/export-config.test.ts index 2fe68054e8..f3938160bf 100644 --- a/kits/storage-resize-images/tests/export-config.test.ts +++ b/kits/storage-resize-images/tests/export-config.test.ts @@ -94,12 +94,22 @@ describe("resolveResizeImagesConfig", () => { ).toEqual(DELETE_IMAGE.never); }); - test("an unset deleteOriginal never deletes", () => { + test("an unset deleteOriginal deletes on success, matching the extension", () => { + // The extension mapped every DELETE_ORIGINAL_FILE value other than + // "true"/"false" (unset included) to onSuccess. const resolved = resolveResizeImagesConfig({ ...baseConfig, deleteOriginal: undefined, }); - expect(resolved.deleteOriginalFile).toEqual(DELETE_IMAGE.never); + expect(resolved.deleteOriginalFile).toEqual(DELETE_IMAGE.onSuccess); + }); + + test("an empty-string deleteOriginal (partial env) deletes on success", () => { + const resolved = resolveResizeImagesConfig({ + ...baseConfig, + deleteOriginal: "" as ResizeImagesConfig["deleteOriginal"], + }); + expect(resolved.deleteOriginalFile).toEqual(DELETE_IMAGE.onSuccess); }); test("splits a comma-separated sizes string", () => { diff --git a/kits/storage-resize-images/tests/handlers.test.ts b/kits/storage-resize-images/tests/handlers.test.ts index bb7681e46a..c300a06fab 100644 --- a/kits/storage-resize-images/tests/handlers.test.ts +++ b/kits/storage-resize-images/tests/handlers.test.ts @@ -73,6 +73,7 @@ import { checkImageContent } from "../src/content-filter"; import * as events from "../src/events"; import { DELETE_IMAGE, + type ResizeImagesConfig, type ResolvedResizeImagesConfig, resolveResizeImagesConfig, } from "../src/export-config"; @@ -97,16 +98,22 @@ const mock = (fn: T) => fn as unknown as ReturnType; const bucketStub = {}; +// deleteOriginal is pinned: an omitted value resolves to on_success, and +// these tests exercise handler logic, not the resolver's defaults. +const baseInput: ResizeImagesConfig = { + bucket: "demo-bucket", + sizes: "200x200", + region: "us-central1", + deleteOriginal: "false", +}; + function makeCtx( - overrides: Partial = {} + overrides: Partial = {}, + input: ResizeImagesConfig = baseInput ): HandlerContext { return { config: { - ...resolveResizeImagesConfig({ - bucket: "demo-bucket", - sizes: "200x200", - region: "us-central1", - }), + ...resolveResizeImagesConfig(input), ...overrides, }, storage: { @@ -390,6 +397,42 @@ describe("generateResizedImageHandler", () => { expect(deleteRemoteFile).not.toHaveBeenCalled(); }); + test("an omitted deleteOriginal deletes the original after a successful run", async () => { + // The extension resolved an unset DELETE_ORIGINAL_FILE to on_success. + const remoteFile = { delete: vi.fn() }; + mock(downloadOriginalFile).mockResolvedValue(["/tmp/test.jpg", remoteFile]); + const ctx = makeCtx( + {}, + { bucket: "demo-bucket", sizes: "200x200", region: "us-central1" } + ); + + await generateResizedImageHandler(mockObject, ctx, false); + + expect(deleteRemoteFile).toHaveBeenCalledWith( + remoteFile, + "images/test.jpg" + ); + expect(deleteRemoteFile).toHaveBeenCalledTimes(1); + }); + + test("an omitted deleteOriginal keeps the original on a failed run", async () => { + mock(downloadOriginalFile).mockResolvedValue([ + "/tmp/test.jpg", + { delete: vi.fn() }, + ]); + mock(resizeImages).mockResolvedValue([ + { status: "fulfilled", value: { success: false } }, + ]); + const ctx = makeCtx( + {}, + { bucket: "demo-bucket", sizes: "200x200", region: "us-central1" } + ); + + await generateResizedImageHandler(mockObject, ctx, false); + + expect(deleteRemoteFile).not.toHaveBeenCalled(); + }); + test("cleans up the temp files it created", async () => { const ctx = makeCtx(); mock(checkImageContent).mockResolvedValue(false); From f8ec361f4f931926c64eb53c6e93b1807131d06c Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Wed, 2 Sep 2026 18:13:35 +0100 Subject: [PATCH 2/2] test(storage-resize-images): pin deletion of a filter-blocked original under the omitted default --- .../tests/handlers.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/kits/storage-resize-images/tests/handlers.test.ts b/kits/storage-resize-images/tests/handlers.test.ts index c300a06fab..c1d6e03e7f 100644 --- a/kits/storage-resize-images/tests/handlers.test.ts +++ b/kits/storage-resize-images/tests/handlers.test.ts @@ -415,6 +415,26 @@ describe("generateResizedImageHandler", () => { expect(deleteRemoteFile).toHaveBeenCalledTimes(1); }); + test("an omitted deleteOriginal deletes a filter-blocked original once its placeholder resizes", async () => { + // Matches the extension: the blocked original is replaced and then removed + // under on_success, unless failedImagesPath stored a copy first. + const remoteFile = { delete: vi.fn() }; + mock(downloadOriginalFile).mockResolvedValue(["/tmp/test.jpg", remoteFile]); + mock(checkImageContent).mockResolvedValue(false); + const ctx = makeCtx( + {}, + { bucket: "demo-bucket", sizes: "200x200", region: "us-central1" } + ); + + await generateResizedImageHandler(mockObject, ctx, false); + + expect(deleteRemoteFile).toHaveBeenCalledWith( + remoteFile, + "images/test.jpg" + ); + expect(deleteRemoteFile).toHaveBeenCalledTimes(1); + }); + test("an omitted deleteOriginal keeps the original on a failed run", async () => { mock(downloadOriginalFile).mockResolvedValue([ "/tmp/test.jpg",