From 7d077b25ce491e6c6f05b823bcc0f363719a2455 Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Tue, 1 Sep 2026 18:33:19 +0100 Subject: [PATCH 1/3] chore(bigquery-firestore-export): restore the writeRunResultsToFirestore log helper The extension logged the start of the Firestore write phase via logs.writeRunResultsToFirestore(runId); the kit dropped it. Restore it in the kit's structured-log style and call it after the BigQuery results are read, before the row writes, matching the extension's call site. The extension's logs.start() and logs.invalidResourceName() are also absent from the kit, but neither has a call site upstream (they are defined and never called), and the kit already logs its own start()/init(config) pair, so they are deliberately not restored. --- kits/bigquery-firestore-export/src/helper.ts | 1 + kits/bigquery-firestore-export/src/logs.ts | 4 ++ .../tests/helper.test.ts | 67 ++++++++++++++++++- 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/kits/bigquery-firestore-export/src/helper.ts b/kits/bigquery-firestore-export/src/helper.ts index fe48a03b98..a6bdfc7de5 100644 --- a/kits/bigquery-firestore-export/src/helper.ts +++ b/kits/bigquery-firestore-export/src/helper.ts @@ -227,6 +227,7 @@ export async function writeRunResultsToFirestore( message.json.destinationDatasetId, tableName ); + logs.writeRunResultsToFirestore(runId); const collection = db.collection( `${config.firestoreCollection}/${transferConfigId}/runs/${runId}/output` ); diff --git a/kits/bigquery-firestore-export/src/logs.ts b/kits/bigquery-firestore-export/src/logs.ts index 0848691139..d538ae5f7f 100644 --- a/kits/bigquery-firestore-export/src/logs.ts +++ b/kits/bigquery-firestore-export/src/logs.ts @@ -70,6 +70,10 @@ export function bigqueryQueryFailed( }); } +export function writeRunResultsToFirestore(runId: string): void { + logger.debug("Writing BigQuery results to Firestore", { runId }); +} + export function runResultsWrittenToFirestore( runId: string, successCount: number, diff --git a/kits/bigquery-firestore-export/tests/helper.test.ts b/kits/bigquery-firestore-export/tests/helper.test.ts index 74c1ba1e16..10ece811a0 100644 --- a/kits/bigquery-firestore-export/tests/helper.test.ts +++ b/kits/bigquery-firestore-export/tests/helper.test.ts @@ -14,14 +14,21 @@ * limitations under the License. */ -import { Geography } from "@google-cloud/bigquery"; -import { Timestamp } from "firebase-admin/firestore"; -import { describe, expect, test } from "vitest"; +import { type BigQuery, Geography } from "@google-cloud/bigquery"; +import { type Firestore, Timestamp } from "firebase-admin/firestore"; +import { describe, expect, test, vi } from "vitest"; +import { resolveConfig } from "../src/export-config"; import { convertUnsupportedDataTypes, parseTransferConfigName, parseTransferRunName, + type ResultHandlerContext, + writeRunResultsToFirestore, } from "../src/helper"; +import * as logs from "../src/logs"; +import type { TransferRunMessage } from "../src/types"; + +vi.mock("../src/logs", { spy: true }); describe("transfer resource parsing", () => { test("parses config and run resource names", () => { @@ -72,3 +79,57 @@ describe("convertUnsupportedDataTypes", () => { expect(converted.nested).toEqual([{ value: true }]); }); }); + +describe("writeRunResultsToFirestore", () => { + test("logs the run id after reading results and before writing rows", async () => { + const add = vi.fn().mockResolvedValue({}); + const set = vi.fn().mockResolvedValue({}); + const db = { + collection: vi.fn(() => ({ add, doc: vi.fn(() => ({ set })) })), + runTransaction: vi.fn( + async (fn: (transaction: unknown) => Promise) => + fn({ + get: vi.fn().mockResolvedValue({ data: () => undefined }), + set: vi.fn(), + }) + ), + } as unknown as Firestore; + const bigquery = { + createQueryJob: vi.fn().mockResolvedValue([ + { + id: "job-1", + getQueryResults: vi.fn().mockResolvedValue([[{ value: 1 }]]), + }, + ]), + } as unknown as BigQuery; + const config = resolveConfig({ + bigqueryDatasetLocation: "US", + projectId: "test-project", + instanceId: "users-export", + datasetId: "analytics", + tableName: "out", + queryString: "SELECT * FROM source.users", + displayName: "Users export", + schedule: "every 24 hours", + }); + const message = { + json: { + name: "projects/test-project/locations/us/transferConfigs/config-1/runs/run-1", + runTime: "2026-08-20T10:05:39Z", + state: "SUCCEEDED", + destinationDatasetId: "analytics", + params: { destination_table_name_template: 'out_{run_time|"%H%M%S"}' }, + }, + } as TransferRunMessage; + const ctx = { db, bigquery, config } as ResultHandlerContext; + + await writeRunResultsToFirestore(ctx, message); + + const logSpy = vi.mocked(logs.writeRunResultsToFirestore); + expect(logSpy).toHaveBeenCalledTimes(1); + expect(logSpy).toHaveBeenCalledWith("run-1"); + expect(logSpy.mock.invocationCallOrder[0]).toBeLessThan( + add.mock.invocationCallOrder[0] + ); + }); +}); From 53f52a1976632ce1ca65d8f1049c8778b2cbb3ec Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Tue, 1 Sep 2026 18:33:27 +0100 Subject: [PATCH 2/3] chore(firestore-bigquery-export): restore the extension's MAX_STALENESS doc example The kit's maxStaleness doc example read 0-0 0 4:0:0 where the extension's extension.yaml documents INTERVAL "8:0:0" HOUR TO SECOND. The value is interpolated verbatim into the max_staleness view option, so the full INTERVAL expression is also the correct value shape. Restore the extension's example. Note: issue #3036 files this item under bigquery-firestore-export, but the #2974 ledger entry and the MAX_STALENESS param both belong to firestore-bigquery-export. --- kits/firestore-bigquery-export/src/export-config.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/kits/firestore-bigquery-export/src/export-config.ts b/kits/firestore-bigquery-export/src/export-config.ts index 8a11fb3f26..d523a9b41c 100644 --- a/kits/firestore-bigquery-export/src/export-config.ts +++ b/kits/firestore-bigquery-export/src/export-config.ts @@ -71,7 +71,10 @@ export interface ExportConfig { partitioning?: ChangeTrackerConfig["partitioning"]; /** Clustering columns (max 4). */ clustering?: string[] | null; - /** Materialized-view max staleness interval, e.g. `0-0 0 4:0:0`. */ + /** + * Materialized-view max staleness interval, e.g. + * `INTERVAL "8:0:0" HOUR TO SECOND`. + */ maxStaleness?: ConfigValue; /** Incremental materialized-view refresh interval in minutes. */ refreshIntervalMinutes?: ConfigValue; From af1e24b20870018cb5a6f4794959427b8d6907dc Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Tue, 1 Sep 2026 18:48:28 +0100 Subject: [PATCH 3/3] chore(bigquery-firestore-export): pin the run-results log after the BigQuery read The ordering test asserted only that the log fires before the row writes, so hoisting the call above getBigqueryResults still passed. Assert the read side too: the log's invocationCallOrder must follow getQueryResults. Verified the hoisted mutant now fails. --- kits/bigquery-firestore-export/tests/helper.test.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/kits/bigquery-firestore-export/tests/helper.test.ts b/kits/bigquery-firestore-export/tests/helper.test.ts index 10ece811a0..6a20142093 100644 --- a/kits/bigquery-firestore-export/tests/helper.test.ts +++ b/kits/bigquery-firestore-export/tests/helper.test.ts @@ -94,13 +94,11 @@ describe("writeRunResultsToFirestore", () => { }) ), } as unknown as Firestore; + const getQueryResults = vi.fn().mockResolvedValue([[{ value: 1 }]]); const bigquery = { - createQueryJob: vi.fn().mockResolvedValue([ - { - id: "job-1", - getQueryResults: vi.fn().mockResolvedValue([[{ value: 1 }]]), - }, - ]), + createQueryJob: vi + .fn() + .mockResolvedValue([{ id: "job-1", getQueryResults }]), } as unknown as BigQuery; const config = resolveConfig({ bigqueryDatasetLocation: "US", @@ -128,6 +126,9 @@ describe("writeRunResultsToFirestore", () => { const logSpy = vi.mocked(logs.writeRunResultsToFirestore); expect(logSpy).toHaveBeenCalledTimes(1); expect(logSpy).toHaveBeenCalledWith("run-1"); + expect(logSpy.mock.invocationCallOrder[0]).toBeGreaterThan( + getQueryResults.mock.invocationCallOrder[0] + ); expect(logSpy.mock.invocationCallOrder[0]).toBeLessThan( add.mock.invocationCallOrder[0] );