From 98e9ca173e07c3da2def846525cee590c7cee44f Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Wed, 2 Sep 2026 09:48:46 +0200 Subject: [PATCH 1/6] Send typed broadcast updates for survey, job, and LOI writes Replace the generic empty-payload broadcastSurveyUpdate() with a single broadcastUpdate() that tags each FCM message with a type ('survey', 'job', or 'loi'), the affected entity id, and the triggering event's commit time. LOI updates also carry a deleted flag. Clients that don't inspect the payload keep working exactly as before (full survey resync); clients that do can use the hint to fetch just what changed instead of resyncing everything. Messages still share one collapse key per survey so bursts of writes (e.g. importing LOIs) collapse into a single wake-up. --- .../src/common/broadcast-survey-update.ts | 36 ---------- functions/src/common/broadcast.ts | 69 +++++++++++++++++++ functions/src/on-create-loi.spec.ts | 4 -- functions/src/on-write-job.ts | 6 +- functions/src/on-write-loi.ts | 9 ++- functions/src/on-write-survey.ts | 4 +- 6 files changed, 80 insertions(+), 48 deletions(-) delete mode 100644 functions/src/common/broadcast-survey-update.ts create mode 100644 functions/src/common/broadcast.ts diff --git a/functions/src/common/broadcast-survey-update.ts b/functions/src/common/broadcast-survey-update.ts deleted file mode 100644 index 1f01ffea5..000000000 --- a/functions/src/common/broadcast-survey-update.ts +++ /dev/null @@ -1,36 +0,0 @@ -/** - * Copyright 2024 The Ground Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { getMessaging } from 'firebase-admin/messaging'; - -/** - * Sends an empty message to clients subscribed to the specified topic. - * Messages are sent without a payload so that they can collapsed. Collapsible - * messages are more performant, and they may be replaced by newer messages if - * necessary. This is important when importing LOIs, which may trigger - * hundred of updates in a short period of time. - * See also: https://firebase.google.com/docs/cloud-messaging/concept-options#collapsible_and_non-collapsible_messages - */ -export async function broadcastSurveyUpdate(topic: string): Promise { - if (process.env.FUNCTIONS_EMULATOR === 'true') { - console.debug(`Skipping FCM message to ${topic} (emulator mode)`); - return ''; - } - - console.debug(`Sending message to ${topic}`); - - return getMessaging().send({ topic }); -} diff --git a/functions/src/common/broadcast.ts b/functions/src/common/broadcast.ts new file mode 100644 index 000000000..8785c838b --- /dev/null +++ b/functions/src/common/broadcast.ts @@ -0,0 +1,69 @@ +/** + * Copyright 2024 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { getMessaging } from 'firebase-admin/messaging'; + +/** + * A change to announce to clients watching a survey. `type` tells them what + * changed, so they can fetch just that instead of the whole survey. Clients + * which don't recognize a message fall back to syncing everything, so new + * types can be added without breaking older ones. + */ +export type SurveyUpdate = + | { type: 'survey'; surveyId: string } + | { type: 'job'; surveyId: string; jobId: string } + | { type: 'loi'; surveyId: string; loiId: string; deleted: boolean }; + +/** + * Announces `update` to clients subscribed to its survey's topic, stamped with + * the time the triggering write was committed. + * + * Messages share one collapse key per survey so that a burst of writes - an + * import may trigger thousands - is delivered as a single wake-up. Clients must + * therefore treat the payload as a hint about one of the changes, never as the + * complete set: the ids in a collapsed message are whichever arrived last. + * See also: https://firebase.google.com/docs/cloud-messaging/concept-options#collapsible_and_non-collapsible_messages + */ +export async function broadcastUpdate( + update: SurveyUpdate, + eventTime: string +): Promise { + const { surveyId } = update; + + if (process.env.FUNCTIONS_EMULATOR === 'true') { + console.debug(`Skipping FCM message to ${surveyId} (emulator mode)`); + return ''; + } + + console.debug(`Sending ${update.type} update to ${surveyId}`); + + return getMessaging().send({ + topic: surveyId, + data: toFcmData(update, eventTime), + android: { collapseKey: surveyId, priority: 'normal' }, + }); +} + +/** FCM data payloads carry strings only, so all values are stringified. */ +function toFcmData( + update: SurveyUpdate, + eventTime: string +): { [k: string]: string } { + return Object.fromEntries([ + ...Object.entries(update).map(([k, v]) => [k, String(v)]), + ['eventTime', eventTime], + ]); +} diff --git a/functions/src/on-create-loi.spec.ts b/functions/src/on-create-loi.spec.ts index 7e89692cd..d932c9193 100644 --- a/functions/src/on-create-loi.spec.ts +++ b/functions/src/on-create-loi.spec.ts @@ -28,7 +28,6 @@ import { import { resetDatastore } from './common/context'; import { GroundProtos } from '@ground/proto'; import { onCreateLoiHandler } from './on-create-loi'; -import * as broadcastModule from './common/broadcast-survey-update'; import Pb = GroundProtos.ground.v1beta1; @@ -68,9 +67,6 @@ describe('onCreateLoiHandler()', () => { beforeEach(() => { mockFirestore = createMockFirestore(); stubAdminApi(mockFirestore); - spyOn(broadcastModule, 'broadcastSurveyUpdate').and.returnValue( - Promise.resolve('') - ); mockFirestore.doc(LOI_PATH).set(loiDoc); mockFirestore .doc('config/integrations/propertyGenerators/whisp') diff --git a/functions/src/on-write-job.ts b/functions/src/on-write-job.ts index a68bc2102..d8a9ed2fe 100644 --- a/functions/src/on-write-job.ts +++ b/functions/src/on-write-job.ts @@ -19,12 +19,12 @@ import { DocumentSnapshot, FirestoreEvent, } from 'firebase-functions/v2/firestore'; -import { broadcastSurveyUpdate } from './common/broadcast-survey-update'; +import { broadcastUpdate } from './common/broadcast'; export async function onWriteJobHandler( event: FirestoreEvent | undefined> ) { - const surveyId = event.params.surveyId; + const { surveyId, jobId } = event.params; - return broadcastSurveyUpdate(surveyId); + return broadcastUpdate({ type: 'job', surveyId, jobId }, event.time); } diff --git a/functions/src/on-write-loi.ts b/functions/src/on-write-loi.ts index 78886a84f..48a4ad8a5 100644 --- a/functions/src/on-write-loi.ts +++ b/functions/src/on-write-loi.ts @@ -19,12 +19,15 @@ import { DocumentSnapshot, FirestoreEvent, } from 'firebase-functions/v2/firestore'; -import { broadcastSurveyUpdate } from './common/broadcast-survey-update'; +import { broadcastUpdate } from './common/broadcast'; export async function onWriteLoiHandler( event: FirestoreEvent | undefined> ) { - const surveyId = event.params.surveyId; + const { surveyId, loiId } = event.params; + // Defaults to false when the change isn't available, so that clients resync + // the LOI rather than dropping it. + const deleted = event.data?.after?.exists === false; - return broadcastSurveyUpdate(surveyId); + return broadcastUpdate({ type: 'loi', surveyId, loiId, deleted }, event.time); } diff --git a/functions/src/on-write-survey.ts b/functions/src/on-write-survey.ts index 61d939407..36acc88e8 100644 --- a/functions/src/on-write-survey.ts +++ b/functions/src/on-write-survey.ts @@ -19,12 +19,12 @@ import { DocumentSnapshot, FirestoreEvent, } from 'firebase-functions/v2/firestore'; -import { broadcastSurveyUpdate } from './common/broadcast-survey-update'; +import { broadcastUpdate } from './common/broadcast'; export function onWriteSurveyHandler( event: FirestoreEvent | undefined> ): Promise { const surveyId = event.params.surveyId; - return broadcastSurveyUpdate(surveyId); + return broadcastUpdate({ type: 'survey', surveyId }, event.time); } From 9efd71a6640fba32f5222191c0a838e572a2d039 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Wed, 2 Sep 2026 10:01:38 +0200 Subject: [PATCH 2/6] fixed copyright year --- functions/src/common/broadcast.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functions/src/common/broadcast.ts b/functions/src/common/broadcast.ts index 8785c838b..d0a25702f 100644 --- a/functions/src/common/broadcast.ts +++ b/functions/src/common/broadcast.ts @@ -1,5 +1,5 @@ /** - * Copyright 2024 The Ground Authors. + * Copyright 2026 The Ground Authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 40c8a59fe43d3ec29c35b75a338748ac59a00944 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Wed, 2 Sep 2026 15:39:55 +0200 Subject: [PATCH 3/6] Moved toTimestampPb in a different file --- functions/src/common/audit-info.ts | 48 ++++++++++++++++++++++++++++++ functions/src/import-geojson.ts | 7 +---- 2 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 functions/src/common/audit-info.ts diff --git a/functions/src/common/audit-info.ts b/functions/src/common/audit-info.ts new file mode 100644 index 000000000..b7a2a4f21 --- /dev/null +++ b/functions/src/common/audit-info.ts @@ -0,0 +1,48 @@ +/** + * Copyright 2026 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { GroundProtos } from '@ground/proto'; + +import Pb = GroundProtos.ground.v1beta1; + +/** + * Returns a copy of `auditInfo` with `serverTimestamp` set from `eventTime` + * (a Firestore trigger's commit time), leaving every other field unchanged. + * Clients can only guess the server time from their own clock when they + * write an AuditInfo, so this corrects it to the time the server actually + * committed the write. + */ +export function withServerTimestamp( + auditInfo: Pb.IAuditInfo, + eventTime: string +): Pb.AuditInfo { + return new Pb.AuditInfo({ + userId: auditInfo.userId, + displayName: auditInfo.displayName, + photoUrl: auditInfo.photoUrl, + emailAddress: auditInfo.emailAddress, + clientTimestamp: auditInfo.clientTimestamp, + serverTimestamp: toTimestampPb(Date.parse(eventTime)), + }); +} + +export function toTimestampPb( + millis: number +): GroundProtos.google.protobuf.Timestamp { + return new GroundProtos.google.protobuf.Timestamp({ + seconds: Math.floor(millis / 1000), + }); +} diff --git a/functions/src/import-geojson.ts b/functions/src/import-geojson.ts index 9eb8e4e3f..91e618c14 100644 --- a/functions/src/import-geojson.ts +++ b/functions/src/import-geojson.ts @@ -18,6 +18,7 @@ import { Request } from 'firebase-functions/v2/https'; import type { Response } from 'express'; import { StatusCodes } from 'http-status-codes'; import { getDatastore } from './common/context'; +import { toTimestampPb } from './common/audit-info'; import Busboy from 'busboy'; import JSONStream from 'jsonstream-ts'; import { canImport } from './common/auth'; @@ -267,12 +268,6 @@ function toMillis(value: string | undefined): number | null { return millis; } -function toTimestampPb(millis: number): GroundProtos.google.protobuf.Timestamp { - return new GroundProtos.google.protobuf.Timestamp({ - seconds: Math.floor(millis / 1000), - }); -} - export function toLoiPbProperties(properties: GeoJsonProperties): { [k: string]: Pb.LocationOfInterest.Property; } { From 425cd98887cd946778e55dc65561f10cb94b253b Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Wed, 2 Sep 2026 15:45:06 +0200 Subject: [PATCH 4/6] Stamp a real server timestamp on newly created LOIs --- functions/src/common/datastore.ts | 6 +++- functions/src/on-create-loi.spec.ts | 46 +++++++++++++++++++++++++++++ functions/src/on-create-loi.ts | 22 +++++++++++++- 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/functions/src/common/datastore.ts b/functions/src/common/datastore.ts index 0f536bcfd..f96867fbd 100644 --- a/functions/src/common/datastore.ts +++ b/functions/src/common/datastore.ts @@ -276,7 +276,11 @@ export class Datastore { loiDoc: DocumentData ) { const loiRef = this.db_.doc(loi(surveyId, loiId)); - await loiRef.update({ [l.properties]: loiDoc[l.properties] }); + const update: DocumentData = { [l.properties]: loiDoc[l.properties] }; + if (l.created in loiDoc) update[l.created] = loiDoc[l.created]; + if (l.lastModified in loiDoc) + update[l.lastModified] = loiDoc[l.lastModified]; + await loiRef.update(update); } static toFirestoreMap(geometry: any) { diff --git a/functions/src/on-create-loi.spec.ts b/functions/src/on-create-loi.spec.ts index d932c9193..f1e9bc98f 100644 --- a/functions/src/on-create-loi.spec.ts +++ b/functions/src/on-create-loi.spec.ts @@ -38,6 +38,8 @@ const p = registry.getFieldIds(Pb.Point); const c = registry.getFieldIds(Pb.Coordinates); const j = registry.getFieldIds(Pb.Job); const intgr = registry.getFieldIds(Pb.Integration); +const ai = registry.getFieldIds(Pb.AuditInfo); +const ts = registry.getFieldIds(GroundProtos.google.protobuf.Timestamp); describe('onCreateLoiHandler()', () => { let mockFirestore: Firestore; @@ -114,4 +116,48 @@ describe('onCreateLoiHandler()', () => { [pr.numericValue]: 100, }); }); + + it('corrects created/lastModified server timestamps to the trigger event time', async () => { + mockFirestore.doc(JOB_PATH).set({}); + + const clientGuessedServerTimeMillis = Date.UTC(2020, 0, 1); + const clientTimeMillis = Date.UTC(2026, 0, 2, 3, 4, 5); + const eventTime = '2026-01-02T03:04:06.000Z'; + + const auditInfo = { + [ai.userId]: 'user1', + [ai.clientTimestamp]: { [ts.seconds]: clientTimeMillis / 1000 }, + [ai.serverTimestamp]: { + [ts.seconds]: clientGuessedServerTimeMillis / 1000, + }, + }; + const loiDocWithAuditInfo = { + ...loiDoc, + [l.created]: auditInfo, + [l.lastModified]: auditInfo, + }; + mockFirestore.doc(LOI_PATH).set(loiDocWithAuditInfo); + + await onCreateLoiHandler({ + data: newDocumentSnapshot( + loiDocWithAuditInfo + ) as unknown as QueryDocumentSnapshot, + params: { surveyId: SURVEY_ID, loiId: LOI_ID }, + time: eventTime, + } as unknown as FirestoreEvent); + + const loiData = (await mockFirestore.doc(LOI_PATH).get()).data(); + const expectedServerTimeSeconds = Math.floor(Date.parse(eventTime) / 1000); + + expect(loiData?.[l.created][ai.serverTimestamp][ts.seconds]).toEqual( + expectedServerTimeSeconds + ); + expect(loiData?.[l.created][ai.clientTimestamp][ts.seconds]).toEqual( + clientTimeMillis / 1000 + ); + expect(loiData?.[l.created][ai.userId]).toEqual('user1'); + expect(loiData?.[l.lastModified][ai.serverTimestamp][ts.seconds]).toEqual( + expectedServerTimeSeconds + ); + }); }); diff --git a/functions/src/on-create-loi.ts b/functions/src/on-create-loi.ts index 73b7f9d7f..bf20dd978 100644 --- a/functions/src/on-create-loi.ts +++ b/functions/src/on-create-loi.ts @@ -20,6 +20,7 @@ import { } from 'firebase-functions/v2/firestore'; import { Datastore } from './common/datastore'; import { getDatastore } from './common/context'; +import { withServerTimestamp } from './common/audit-info'; import { GroundProtos } from '@ground/proto'; import { toDocumentData, toGeoJsonGeometry, toMessage } from '@ground/lib'; import { toLoiPbProperties } from './import-geojson'; @@ -57,11 +58,30 @@ export async function onCreateLoiHandler( surveyId, loiId, toDocumentData( - new Pb.LocationOfInterest({ properties: toLoiPbProperties(properties) }) + new Pb.LocationOfInterest({ + properties: toLoiPbProperties(properties), + ...correctedAuditInfo(loiPb, event.time), + }) ) ); } +function correctedAuditInfo( + loiPb: Pb.LocationOfInterest, + eventTime: string +): Partial { + if (!loiPb.created) return {}; + + const created = withServerTimestamp(loiPb.created, eventTime); + + return { + created, + lastModified: loiPb.lastModified + ? withServerTimestamp(loiPb.lastModified, eventTime) + : created, + }; +} + export async function regenerateLoiProperties( db: Datastore, surveyId: string, From 22eb828c12d7da1d8f2aca81ad61fd0c4626e08f Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Fri, 4 Sep 2026 17:36:37 +0200 Subject: [PATCH 5/6] Split LOI write trigger into create, update and delete handlers --- functions/src/common/loi-properties.ts | 136 +++++++++++++++ functions/src/export-geojson.ts | 2 +- functions/src/index.ts | 18 +- functions/src/on-create-loi.ts | 160 ++++-------------- functions/src/on-delete-loi.ts | 34 ++++ .../src/{on-write-loi.ts => on-update-loi.ts} | 18 +- 6 files changed, 230 insertions(+), 138 deletions(-) create mode 100644 functions/src/common/loi-properties.ts create mode 100644 functions/src/on-delete-loi.ts rename functions/src/{on-write-loi.ts => on-update-loi.ts} (60%) diff --git a/functions/src/common/loi-properties.ts b/functions/src/common/loi-properties.ts new file mode 100644 index 000000000..853471a26 --- /dev/null +++ b/functions/src/common/loi-properties.ts @@ -0,0 +1,136 @@ +/** + * Copyright 2026 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as logger from 'firebase-functions/logger'; +import { Datastore } from './datastore'; +import { GroundProtos } from '@ground/proto'; +import { toGeoJsonGeometry, toMessage } from '@ground/lib'; +import { + Properties, + PropertyGeneratorConfig, + propertyGeneratorHandlers, +} from '../property-generators'; + +import Pb = GroundProtos.ground.v1beta1; + +/** + * Returns the properties of `loiPb` with those of every property generator + * enabled on its job merged in. Generators are called over the network, so a + * failing one is logged and skipped rather than failing the whole set. + */ +export async function regenerateLoiProperties( + db: Datastore, + surveyId: string, + loiId: string, + loiPb: Pb.LocationOfInterest +): Promise { + const geometry = toGeoJsonGeometry(loiPb.geometry!); + + let properties = propertiesPbToObject(loiPb.properties) || {}; + + const jobDoc = await db.fetchJob(surveyId, loiPb.jobId); + const jobPb = toMessage(jobDoc.data()!, Pb.Job) as Pb.Job; + const enabledIntegrationIds = new Set( + jobPb.enabledIntegrations.map(i => i.id) + ); + + const propertyGenerators = await db.fetchPropertyGenerators(); + + for (const propertyGeneratorDoc of propertyGenerators.docs) { + const generatorId = propertyGeneratorDoc.id; + const config = propertyGeneratorDoc.data() as PropertyGeneratorConfig; + const handler = propertyGeneratorHandlers[generatorId]; + + if (!handler) { + continue; + } + + if (!enabledIntegrationIds.has(generatorId)) { + continue; + } + + try { + const newProperties = await handler(config, geometry, loiId); + properties = updateProperties(properties, newProperties, config.prefix); + } catch (e) { + logger.error( + `loiId=${loiId} property generator '${generatorId}' failed:`, + e + ); + } + + Object.keys(properties) + .filter(key => typeof properties[key] === 'object') + .forEach(key => (properties[key] = JSON.stringify(properties[key]))); + } + + return properties; +} + +/** Returns whether both property maps hold the same keys and values. */ +export function propertiesEqual(a: Properties, b: Properties): boolean { + const keys = Object.keys(a); + + return ( + keys.length === Object.keys(b).length && keys.every(k => a[k] === b[k]) + ); +} + +export function propertiesPbToObject(pb: { + [k: string]: Pb.LocationOfInterest.IProperty; +}): Properties { + const properties: { [k: string]: string | number } = {}; + for (const k of Object.keys(pb).sort()) { + const v = pb[k].stringValue || pb[k].numericValue; + if (v !== null && v !== undefined) { + properties[k] = v; + } + } + return properties; +} + +function updateProperties( + properties: Properties, + newProperties: Properties, + prefix?: string +): Properties { + if (prefix) properties = removePrefixedKeys(properties, prefix); + + return { + ...properties, + ...(prefix ? prefixKeys(newProperties, prefix) : newProperties), + }; +} + +/** + * Returns a new object with all keys of the original object prefixed with the given value. + */ +function prefixKeys(obj: Properties, prefix: string): Properties { + return Object.keys(obj).reduce( + (a, k) => ((a[`${prefix}${k}`] = obj[k]), a), + {} as Properties + ); +} + +/** + * Returns a new object containing only the keys that do not start with the specified prefix. + */ +function removePrefixedKeys(obj: Properties, prefix: string): Properties { + Object.keys(obj).forEach(k => { + if (k.startsWith(prefix)) delete obj[k]; + }); + return obj; +} diff --git a/functions/src/export-geojson.ts b/functions/src/export-geojson.ts index 0f7b3fc70..a64a58369 100644 --- a/functions/src/export-geojson.ts +++ b/functions/src/export-geojson.ts @@ -24,7 +24,7 @@ import { } from './common/context'; import { getTempFilePath } from './common/temp-storage'; import { isAccessibleLoi } from './common/utils'; -import { propertiesPbToObject } from './on-create-loi'; +import { propertiesPbToObject } from './common/loi-properties'; import { DecodedIdToken } from 'firebase-admin/auth'; import { StatusCodes } from 'http-status-codes'; import { toMessage } from '@ground/lib'; diff --git a/functions/src/index.ts b/functions/src/index.ts index 58dbd612e..85be4fc8f 100644 --- a/functions/src/index.ts +++ b/functions/src/index.ts @@ -19,6 +19,7 @@ import { onSchedule } from 'firebase-functions/scheduler'; import { onDocumentCreated, onDocumentDeleted, + onDocumentUpdated, onDocumentWritten, } from 'firebase-functions/v2/firestore'; import { onHttpsRequest, onHttpsRequestAsync } from './handlers'; @@ -30,11 +31,12 @@ import { exportGeojsonHandler } from './export-geojson'; import { cleanTempHandler } from './clean-temp'; import { cleanOrphanMediaHandler } from './clean-orphan-media'; import { onCall } from 'firebase-functions/v2/https'; -import { onCreateLoiHandler } from './on-create-loi'; import { onCreatePasslistEntryHandler } from './on-create-passlist-entry'; import { onDeleteSubmissionHandler } from './on-delete-submission'; import { onWriteJobHandler } from './on-write-job'; -import { onWriteLoiHandler } from './on-write-loi'; +import { onCreateLoiHandler } from './on-create-loi'; +import { onDeleteLoiHandler } from './on-delete-loi'; +import { onUpdateLoiHandler } from './on-update-loi'; import { onWriteSubmissionHandler } from './on-write-submission'; import { onWriteSurveyHandler } from './on-write-survey'; import { @@ -91,14 +93,22 @@ export const exportGeojson = onHttpsRequest(exportGeojsonHandler, { cpu: 2, }); +export const onWriteJob = onDocumentWritten(jobPathTemplate, onWriteJobHandler); + export const onCreateLoi = onDocumentCreated( loiPathTemplate, onCreateLoiHandler ); -export const onWriteJob = onDocumentWritten(jobPathTemplate, onWriteJobHandler); +export const onUpdateLoi = onDocumentUpdated( + loiPathTemplate, + onUpdateLoiHandler +); -export const onWriteLoi = onDocumentWritten(loiPathTemplate, onWriteLoiHandler); +export const onDeleteLoi = onDocumentDeleted( + loiPathTemplate, + onDeleteLoiHandler +); export const onWriteSubmission = onDocumentWritten( submissionPathTemplate, diff --git a/functions/src/on-create-loi.ts b/functions/src/on-create-loi.ts index 617b95e37..ceda5bb51 100644 --- a/functions/src/on-create-loi.ts +++ b/functions/src/on-create-loi.ts @@ -15,55 +15,60 @@ */ import { + DocumentSnapshot, FirestoreEvent, - QueryDocumentSnapshot, } from 'firebase-functions/v2/firestore'; -import * as logger from 'firebase-functions/logger'; -import { Datastore } from './common/datastore'; import { getDatastore } from './common/context'; import { withServerTimestamp } from './common/audit-info'; +import { broadcastUpdate } from './common/broadcast'; +import { + propertiesEqual, + propertiesPbToObject, + regenerateLoiProperties, +} from './common/loi-properties'; import { GroundProtos } from '@ground/proto'; -import { toDocumentData, toGeoJsonGeometry, toMessage } from '@ground/lib'; +import { toDocumentData, toMessage } from '@ground/lib'; import { toLoiPbProperties } from './import-geojson'; -import { - Properties, - PropertyGeneratorConfig, - propertyGeneratorHandlers, -} from './property-generators'; import Pb = GroundProtos.ground.v1beta1; -/** - * Handles the creation of a Location of Interest (LOI) document in Firestore. - * This function is triggered by a Cloud Function on Firestore document creation. - * - * @param snapshot The QueryDocumentSnapshot object containing the created LOI data. - * @param context The EventContext object provided by the Cloud Functions framework. - */ export async function onCreateLoiHandler( - event: FirestoreEvent + event: FirestoreEvent ) { - const surveyId = event.params.surveyId; - const loiId = event.params.loiId; + const { surveyId, loiId } = event.params; const data = event.data?.data(); - if (!loiId || !data) return; + if (!surveyId || !loiId || !data) return; const loiPb = toMessage(data, Pb.LocationOfInterest) as Pb.LocationOfInterest; - const db = getDatastore(); const properties = await regenerateLoiProperties(db, surveyId, loiId, loiPb); + const auditInfo = correctedAuditInfo(loiPb, event.time); + const propertiesChanged = !propertiesEqual( + propertiesPbToObject(loiPb.properties), + properties + ); - await db.updateLoiProperties( - surveyId, - loiId, - toDocumentData( - new Pb.LocationOfInterest({ - properties: toLoiPbProperties(properties), - ...correctedAuditInfo(loiPb, event.time), - }) - ) + if (propertiesChanged || Object.keys(auditInfo).length) { + await db.updateLoiProperties( + surveyId, + loiId, + toDocumentData( + new Pb.LocationOfInterest({ + properties: toLoiPbProperties(properties), + ...auditInfo, + }) + ) + ); + + // onUpdateLoi announces the write just made. + return; + } + + return broadcastUpdate( + { type: 'loi', surveyId, loiId, deleted: false }, + event.time ); } @@ -82,98 +87,3 @@ function correctedAuditInfo( : created, }; } - -export async function regenerateLoiProperties( - db: Datastore, - surveyId: string, - loiId: string, - loiPb: Pb.LocationOfInterest -): Promise { - const geometry = toGeoJsonGeometry(loiPb.geometry!); - - let properties = propertiesPbToObject(loiPb.properties) || {}; - - const jobDoc = await db.fetchJob(surveyId, loiPb.jobId); - const jobPb = toMessage(jobDoc.data()!, Pb.Job) as Pb.Job; - const enabledIntegrationIds = new Set( - jobPb.enabledIntegrations.map(i => i.id) - ); - - const propertyGenerators = await db.fetchPropertyGenerators(); - - for (const propertyGeneratorDoc of propertyGenerators.docs) { - const generatorId = propertyGeneratorDoc.id; - const config = propertyGeneratorDoc.data() as PropertyGeneratorConfig; - const handler = propertyGeneratorHandlers[generatorId]; - - if (!handler) { - continue; - } - - if (!enabledIntegrationIds.has(generatorId)) { - continue; - } - - try { - const newProperties = await handler(config, geometry, loiId); - properties = updateProperties(properties, newProperties, config.prefix); - } catch (e) { - logger.error( - `onCreateLoi: loiId=${loiId} property generator '${generatorId}' failed:`, - e - ); - } - - Object.keys(properties) - .filter(key => typeof properties[key] === 'object') - .forEach(key => (properties[key] = JSON.stringify(properties[key]))); - } - - return properties; -} - -function updateProperties( - properties: Properties, - newProperties: Properties, - prefix?: string -): Properties { - if (prefix) properties = removePrefixedKeys(properties, prefix); - - return { - ...properties, - ...(prefix ? prefixKeys(newProperties, prefix) : newProperties), - }; -} - -/** - * Returns a new object with all keys of the original object prefixed with the given value. - */ -function prefixKeys(obj: Properties, prefix: string): Properties { - return Object.keys(obj).reduce( - (a, k) => ((a[`${prefix}${k}`] = obj[k]), a), - {} as Properties - ); -} - -/** - * Returns a new object containing only the keys that do not start with the specified prefix. - */ -function removePrefixedKeys(obj: Properties, prefix: string): Properties { - Object.keys(obj).forEach(k => { - if (k.startsWith(prefix)) delete obj[k]; - }); - return obj; -} - -export function propertiesPbToObject(pb: { - [k: string]: Pb.LocationOfInterest.IProperty; -}): Properties { - const properties: { [k: string]: string | number } = {}; - for (const k of Object.keys(pb).sort()) { - const v = pb[k].stringValue || pb[k].numericValue; - if (v !== null && v !== undefined) { - properties[k] = v; - } - } - return properties; -} diff --git a/functions/src/on-delete-loi.ts b/functions/src/on-delete-loi.ts new file mode 100644 index 000000000..21d9e51d3 --- /dev/null +++ b/functions/src/on-delete-loi.ts @@ -0,0 +1,34 @@ +/** + * Copyright 2026 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the 'License'); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an 'AS IS' BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + DocumentSnapshot, + FirestoreEvent, +} from 'firebase-functions/v2/firestore'; +import { broadcastUpdate } from './common/broadcast'; + +export async function onDeleteLoiHandler( + event: FirestoreEvent +) { + const { surveyId, loiId } = event.params; + + if (!surveyId || !loiId) return; + + return broadcastUpdate( + { type: 'loi', surveyId, loiId, deleted: true }, + event.time + ); +} diff --git a/functions/src/on-write-loi.ts b/functions/src/on-update-loi.ts similarity index 60% rename from functions/src/on-write-loi.ts rename to functions/src/on-update-loi.ts index 48a4ad8a5..9da85b6eb 100644 --- a/functions/src/on-write-loi.ts +++ b/functions/src/on-update-loi.ts @@ -1,14 +1,14 @@ /** - * Copyright 2024 The Ground Authors. + * Copyright 2026 The Ground Authors. * - * Licensed under the Apache License, Version 2.0 (the "License"); + * Licensed under the Apache License, Version 2.0 (the 'License'); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * https://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, + * distributed under the License is distributed on an 'AS IS' BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. @@ -21,13 +21,15 @@ import { } from 'firebase-functions/v2/firestore'; import { broadcastUpdate } from './common/broadcast'; -export async function onWriteLoiHandler( +export async function onUpdateLoiHandler( event: FirestoreEvent | undefined> ) { const { surveyId, loiId } = event.params; - // Defaults to false when the change isn't available, so that clients resync - // the LOI rather than dropping it. - const deleted = event.data?.after?.exists === false; - return broadcastUpdate({ type: 'loi', surveyId, loiId, deleted }, event.time); + if (!surveyId || !loiId) return; + + return broadcastUpdate( + { type: 'loi', surveyId, loiId, deleted: false }, + event.time + ); } From 49f00fb382e36ec72c8a024dd753733e4af7efa8 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Fri, 4 Sep 2026 17:37:35 +0200 Subject: [PATCH 6/6] update tests --- functions/src/on-create-loi.spec.ts | 80 +++++++++++++++++++---------- functions/src/on-delete-loi.spec.ts | 59 +++++++++++++++++++++ functions/src/on-update-loi.spec.ts | 60 ++++++++++++++++++++++ 3 files changed, 172 insertions(+), 27 deletions(-) create mode 100644 functions/src/on-delete-loi.spec.ts create mode 100644 functions/src/on-update-loi.spec.ts diff --git a/functions/src/on-create-loi.spec.ts b/functions/src/on-create-loi.spec.ts index 4a05ad9d9..9e08396be 100644 --- a/functions/src/on-create-loi.spec.ts +++ b/functions/src/on-create-loi.spec.ts @@ -20,14 +20,15 @@ import { stubAdminApi, } from '@ground/lib/testing/firestore'; import { registry } from '@ground/lib'; -import { Firestore } from 'firebase-admin/firestore'; +import { DocumentData, Firestore } from 'firebase-admin/firestore'; import { + DocumentSnapshot, FirestoreEvent, - QueryDocumentSnapshot, } from 'firebase-functions/v2/firestore'; import { resetDatastore } from './common/context'; import { GroundProtos } from '@ground/proto'; import { onCreateLoiHandler } from './on-create-loi'; +import * as broadcastModule from './common/broadcast'; import Pb = GroundProtos.ground.v1beta1; @@ -43,12 +44,14 @@ const ts = registry.getFieldIds(GroundProtos.google.protobuf.Timestamp); describe('onCreateLoiHandler()', () => { let mockFirestore: Firestore; + let broadcastSpy: jasmine.Spy; const SURVEY_ID = 'survey1'; const JOB_ID = 'job1'; const LOI_ID = 'loi1'; const LOI_PATH = `surveys/${SURVEY_ID}/lois/${LOI_ID}`; const JOB_PATH = `surveys/${SURVEY_ID}/jobs/${JOB_ID}`; + const EVENT_TIME = '2026-01-02T03:04:06.000Z'; const loiDoc = { [l.jobId]: JOB_ID, @@ -72,9 +75,20 @@ describe('onCreateLoiHandler()', () => { url: 'https://geoid.example.com/api', }; + function createdEvent(data: DocumentData = loiDoc) { + return { + data: newDocumentSnapshot(data), + params: { surveyId: SURVEY_ID, loiId: LOI_ID }, + time: EVENT_TIME, + } as unknown as FirestoreEvent; + } + beforeEach(() => { mockFirestore = createMockFirestore(); stubAdminApi(mockFirestore); + broadcastSpy = spyOn(broadcastModule, 'broadcastUpdate').and.returnValue( + Promise.resolve('') + ); mockFirestore.doc(LOI_PATH).set(loiDoc); mockFirestore .doc('config/integrations/propertyGenerators/whisp') @@ -92,14 +106,22 @@ describe('onCreateLoiHandler()', () => { mockFirestore.doc(JOB_PATH).set({}); const fetchSpy = spyOn(globalThis, 'fetch'); - await onCreateLoiHandler({ - data: newDocumentSnapshot(loiDoc) as unknown as QueryDocumentSnapshot, - params: { surveyId: SURVEY_ID, loiId: LOI_ID }, - } as unknown as FirestoreEvent); + await onCreateLoiHandler(createdEvent()); expect(fetchSpy).not.toHaveBeenCalled(); }); + it('broadcasts immediately when a created LOI needs no fixing up', async () => { + mockFirestore.doc(JOB_PATH).set({}); + + await onCreateLoiHandler(createdEvent()); + + expect(broadcastSpy).toHaveBeenCalledOnceWith( + { type: 'loi', surveyId: SURVEY_ID, loiId: LOI_ID, deleted: false }, + EVENT_TIME + ); + }); + it('runs property generator and updates LOI properties when integration is enabled', async () => { mockFirestore.doc(JOB_PATH).set({ [j.enabledIntegrations]: [{ [intgr.id]: 'whisp' }], @@ -115,10 +137,7 @@ describe('onCreateLoiHandler()', () => { } as Response) ); - await onCreateLoiHandler({ - data: newDocumentSnapshot(loiDoc) as unknown as QueryDocumentSnapshot, - params: { surveyId: SURVEY_ID, loiId: LOI_ID }, - } as unknown as FirestoreEvent); + await onCreateLoiHandler(createdEvent()); const loiData = (await mockFirestore.doc(LOI_PATH).get()).data(); expect(loiData?.[l.properties]?.['whisp_area']).toEqual({ @@ -126,12 +145,31 @@ describe('onCreateLoiHandler()', () => { }); }); + it('defers the broadcast to the write it makes when fixing up a created LOI', async () => { + mockFirestore.doc(JOB_PATH).set({ + [j.enabledIntegrations]: [{ [intgr.id]: 'whisp' }], + }); + spyOn(globalThis, 'fetch').and.returnValue( + Promise.resolve({ + ok: true, + json: () => + Promise.resolve({ + code: 'analysis_completed', + data: { features: [{ properties: { area: 100 } }] }, + }), + } as Response) + ); + + await onCreateLoiHandler(createdEvent()); + + expect(broadcastSpy).not.toHaveBeenCalled(); + }); + it('corrects created/lastModified server timestamps to the trigger event time', async () => { mockFirestore.doc(JOB_PATH).set({}); const clientGuessedServerTimeMillis = Date.UTC(2020, 0, 1); const clientTimeMillis = Date.UTC(2026, 0, 2, 3, 4, 5); - const eventTime = '2026-01-02T03:04:06.000Z'; const auditInfo = { [ai.userId]: 'user1', @@ -147,16 +185,10 @@ describe('onCreateLoiHandler()', () => { }; mockFirestore.doc(LOI_PATH).set(loiDocWithAuditInfo); - await onCreateLoiHandler({ - data: newDocumentSnapshot( - loiDocWithAuditInfo - ) as unknown as QueryDocumentSnapshot, - params: { surveyId: SURVEY_ID, loiId: LOI_ID }, - time: eventTime, - } as unknown as FirestoreEvent); + await onCreateLoiHandler(createdEvent(loiDocWithAuditInfo)); const loiData = (await mockFirestore.doc(LOI_PATH).get()).data(); - const expectedServerTimeSeconds = Math.floor(Date.parse(eventTime) / 1000); + const expectedServerTimeSeconds = Math.floor(Date.parse(EVENT_TIME) / 1000); expect(loiData?.[l.created][ai.serverTimestamp][ts.seconds]).toEqual( expectedServerTimeSeconds @@ -186,10 +218,7 @@ describe('onCreateLoiHandler()', () => { } as Response) ); - await onCreateLoiHandler({ - data: newDocumentSnapshot(loiDoc) as unknown as QueryDocumentSnapshot, - params: { surveyId: SURVEY_ID, loiId: LOI_ID }, - } as unknown as FirestoreEvent); + await onCreateLoiHandler(createdEvent()); const [, requestInit] = fetchSpy.calls.mostRecent().args; expect(JSON.parse(requestInit!.body as string).id).toEqual(LOI_ID); @@ -212,10 +241,7 @@ describe('onCreateLoiHandler()', () => { } as Response) ); - await onCreateLoiHandler({ - data: newDocumentSnapshot(loiDoc) as unknown as QueryDocumentSnapshot, - params: { surveyId: SURVEY_ID, loiId: LOI_ID }, - } as unknown as FirestoreEvent); + await onCreateLoiHandler(createdEvent()); const loiData = (await mockFirestore.doc(LOI_PATH).get()).data(); expect(loiData?.[l.properties]?.['geoid_geoid']).toBeUndefined(); diff --git a/functions/src/on-delete-loi.spec.ts b/functions/src/on-delete-loi.spec.ts new file mode 100644 index 000000000..1db9a4e8d --- /dev/null +++ b/functions/src/on-delete-loi.spec.ts @@ -0,0 +1,59 @@ +/** + * Copyright 2026 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + DocumentSnapshot, + FirestoreEvent, +} from 'firebase-functions/v2/firestore'; +import { onDeleteLoiHandler } from './on-delete-loi'; +import * as broadcastModule from './common/broadcast'; + +describe('onDeleteLoiHandler()', () => { + const SURVEY_ID = 'survey1'; + const LOI_ID = 'loi1'; + const EVENT_TIME = '2026-01-02T03:04:06.000Z'; + + let broadcastSpy: jasmine.Spy; + + beforeEach(() => { + broadcastSpy = spyOn(broadcastModule, 'broadcastUpdate').and.returnValue( + Promise.resolve('') + ); + }); + + function deletedEvent(params: Record = {}) { + return { + data: undefined, + params: { surveyId: SURVEY_ID, loiId: LOI_ID, ...params }, + time: EVENT_TIME, + } as unknown as FirestoreEvent; + } + + it('announces the deleted LOI', async () => { + await onDeleteLoiHandler(deletedEvent()); + + expect(broadcastSpy).toHaveBeenCalledOnceWith( + { type: 'loi', surveyId: SURVEY_ID, loiId: LOI_ID, deleted: true }, + EVENT_TIME + ); + }); + + it('does nothing when the event carries no loi id', async () => { + await onDeleteLoiHandler(deletedEvent({ loiId: '' })); + + expect(broadcastSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/functions/src/on-update-loi.spec.ts b/functions/src/on-update-loi.spec.ts new file mode 100644 index 000000000..436d12cc8 --- /dev/null +++ b/functions/src/on-update-loi.spec.ts @@ -0,0 +1,60 @@ +/** + * Copyright 2026 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + Change, + DocumentSnapshot, + FirestoreEvent, +} from 'firebase-functions/v2/firestore'; +import { onUpdateLoiHandler } from './on-update-loi'; +import * as broadcastModule from './common/broadcast'; + +describe('onUpdateLoiHandler()', () => { + const SURVEY_ID = 'survey1'; + const LOI_ID = 'loi1'; + const EVENT_TIME = '2026-01-02T03:04:06.000Z'; + + let broadcastSpy: jasmine.Spy; + + beforeEach(() => { + broadcastSpy = spyOn(broadcastModule, 'broadcastUpdate').and.returnValue( + Promise.resolve('') + ); + }); + + function updatedEvent(params: Record = {}) { + return { + data: {} as Change, + params: { surveyId: SURVEY_ID, loiId: LOI_ID, ...params }, + time: EVENT_TIME, + } as unknown as FirestoreEvent | undefined>; + } + + it('announces the updated LOI', async () => { + await onUpdateLoiHandler(updatedEvent()); + + expect(broadcastSpy).toHaveBeenCalledOnceWith( + { type: 'loi', surveyId: SURVEY_ID, loiId: LOI_ID, deleted: false }, + EVENT_TIME + ); + }); + + it('does nothing when the event carries no loi id', async () => { + await onUpdateLoiHandler(updatedEvent({ loiId: '' })); + + expect(broadcastSpy).not.toHaveBeenCalled(); + }); +});