Fix importGeoJson timeout and improve write performance

functions/src/common/datastore.ts

@@ -227,6 +227,18 @@ export class Datastore {
     await this.db_.doc(survey(surveyId)).collection('lois').add(loiDoc);
   }
 
+  async insertLocationsOfInterest(
+    surveyId: string,
+    loiDocs: DocumentData[]
+  ): Promise<void> {
+    const bulkWriter = this.db_.bulkWriter();
+    const collectionRef = this.db_.collection(lois(surveyId));
+    for (const loiDoc of loiDocs) {
+      bulkWriter.create(collectionRef.doc(), loiDoc);
+    }
+    await bulkWriter.close();
+  }
+
   async countSubmissionsForLoi(
     surveyId: string,
     loiId: string

functions/src/handlers.ts

@@ -16,7 +16,7 @@
 
 import cors from 'cors';
 import { DecodedIdToken } from 'firebase-admin/auth';
-import { onRequest, HttpsOptions, Request } from 'firebase-functions/v2/https';
+import { HttpsOptions, Request, onRequest } from 'firebase-functions/v2/https';
 import type { Response } from 'express';
 import { StatusCodes } from 'http-status-codes';
 import { getDecodedIdToken } from './common/auth';
@@ -159,8 +159,11 @@ function invokeCallback(
  * work is completed, but the HTTPS request will not complete until one of those two
  * callbacks are invoked.
  */
-export function onHttpsRequestAsync(callback: HttpsRequestCallback) {
-  return onRequest((req: Request, res: Response) =>
+export function onHttpsRequestAsync(
+  callback: HttpsRequestCallback,
+  options: HttpsOptions = {}
+) {
+  return onRequest(options, (req: Request, res: Response) =>
     corsMiddleware(req, res, () =>
       cookieParser()(
         req as any,

functions/src/import-geojson.spec.ts

@@ -231,22 +231,22 @@ describe('importGeoJson()', () => {
     },
   ];
 
+  let insertLocationsOfInterestSpy: jasmine.Spy;
+
   beforeEach(() => {
     mockFirestore = createMockFirestore();
     stubAdminApi(mockFirestore);
+    const db = getDatastore();
+    insertLocationsOfInterestSpy = spyOn(
+      db,
+      'insertLocationsOfInterest'
+    ).and.returnValue(Promise.resolve());
   });
 
   afterEach(() => {
     resetDatastore();
   });
 
-  async function loiData(surveyId: string) {
-    const lois = await mockFirestore
-      .collection(`surveys/${surveyId}/lois`)
-      .get();
-    return lois.docs.map(doc => doc.data());
-  }
-
   function createPostData(surveyId: string, jobId: string, geoJson: object) {
     const form = new FormData();
     form.append('survey', surveyId);
@@ -255,35 +255,76 @@ describe('importGeoJson()', () => {
     return form;
   }
 
+  async function runImport(geoJson: object) {
+    const req = await createPostRequestSpy(
+      { url: '/importGeoJson' },
+      createPostData(surveyId, jobId, geoJson)
+    );
+    const res = createResponseSpy();
+    try {
+      // Ideally we would call `importGeoJson` directly rather than via `invokeCallbackAsync`,
+      // but that would require mocking all middleware which may be overkill.
+      await invokeCallbackAsync(importGeoJsonCallback, req, res, {
+        email,
+      } as DecodedIdToken);
+    } catch (err) {
+      console.log(err);
+    }
+    return res;
+  }
+
   testCases.forEach(({ desc, input, expectedStatus, expected }) =>
     it(desc, async () => {
       // Add survey.
       mockFirestore.doc(`surveys/${surveyId}`).set(survey);
 
-      // Build mock request and response.
-      const req = await createPostRequestSpy(
-        { url: '/importGeoJson' },
-        createPostData(surveyId, jobId, input)
-      );
-      const res = createResponseSpy();
-
-      try {
-        // Run import GeoJSON function.
-        // Ideally we would call `importGeoJson` directly rather than via `invokeCallbackAsync`,
-        // but that would require mocking all middleware which may be overkill.
-        await invokeCallbackAsync(importGeoJsonCallback, req, res, {
-          email,
-        } as DecodedIdToken);
-      } catch (err) {
-        console.log(err);
-      }
+      const res = await runImport(input);
 
-      // Check post-conditions.
       expect(res.status).toHaveBeenCalledOnceWith(expectedStatus);
-      expect(await loiData(surveyId)).toEqual(expected);
+      if (expectedStatus === StatusCodes.OK) {
+        expect(insertLocationsOfInterestSpy).toHaveBeenCalledOnceWith(
+          surveyId,
+          expected
+        );
+      } else {
+        expect(insertLocationsOfInterestSpy).not.toHaveBeenCalled();
+      }
     })
   );
 
+  it('bulk-inserts all features in a single call', async () => {
+    mockFirestore.doc(`surveys/${surveyId}`).set(survey);
+    const geoJsonWithMultiplePoints = {
+      type: 'FeatureCollection',
+      features: [
+        {
+          type: 'Feature',
+          geometry: { type: 'Point', coordinates: [125.6, 10.1] },
+          properties: {},
+        },
+        {
+          type: 'Feature',
+          geometry: { type: 'Point', coordinates: [126.0, 11.0] },
+          properties: {},
+        },
+        {
+          type: 'Feature',
+          geometry: { type: 'Point', coordinates: [127.0, 12.0] },
+          properties: {},
+        },
+      ],
+    };
+
+    await runImport(geoJsonWithMultiplePoints);
+
+    // All features must be passed to a single bulk insert call, not one call per feature.
+    expect(insertLocationsOfInterestSpy).toHaveBeenCalledTimes(1);
+    const [calledSurveyId, calledDocs] =
+      insertLocationsOfInterestSpy.calls.mostRecent().args;
+    expect(calledSurveyId).toBe(surveyId);
+    expect(calledDocs.length).toBe(3);
+  });
+
   it('surfaces errors thrown during async file processing', async () => {
     // Make fetchSurvey reject to simulate an unexpected async error in the file handler.
     const db = getDatastore();

functions/src/import-geojson.ts

@@ -22,6 +22,7 @@ import Busboy from 'busboy';
 import JSONStream from 'jsonstream-ts';
 import { canImport } from './common/auth';
 import { DecodedIdToken } from 'firebase-admin/auth';
+import { DocumentData } from 'firebase-admin/firestore';
 import { GroundProtos } from '@ground/proto';
 import { isGeometryValid, toDocumentData, toGeometryPb } from '@ground/lib';
 import { Feature, GeoJsonProperties } from 'geojson';
@@ -58,9 +59,8 @@ export function importGeoJsonCallback(
   // Dictionary used to accumulate task step values, keyed by step name.
   const params: { [name: string]: string } = {};
 
-  // Accumulate Promises for insert operations, so we don't finalize the res
-  // stream before operations are complete.
-  const inserts: any[] = [];
+  // Accumulate LOI documents to be bulk-inserted once the file is fully parsed.
+  const loiDocs: DocumentData[] = [];
 
   const db = getDatastore();
 
@@ -119,8 +119,8 @@ export function importGeoJsonCallback(
   busboy.on('finish', async () => {
     if (hasError) return;
     try {
-      await Promise.all(inserts);
-      const count = inserts.length;
+      await db.insertLocationsOfInterest(params.survey, loiDocs);
+      const count = loiDocs.length;
       console.debug(`${count} LOIs imported`);
       res.send(JSON.stringify({ count }));
       done();
@@ -207,10 +207,9 @@ export function importGeoJsonCallback(
       return;
     }
     try {
-      const loi = toDocumentData(
-        toLoiPb(geoJsonFeature as Feature, jobId, ownerId)
+      loiDocs.push(
+        toDocumentData(toLoiPb(geoJsonFeature as Feature, jobId, ownerId))
       );
-      inserts.push(db.insertLocationOfInterest(surveyId, loi));
     } catch (loiErr) {
       console.debug('Skipping LOI', loiErr);
     }

functions/src/index.ts

@@ -68,7 +68,11 @@ export const onCreatePasslistEntry = onDocumentCreated(
   onCreatePasslistEntryHandler
 );
 
-export const importGeoJson = onHttpsRequestAsync(importGeoJsonCallback);
+export const importGeoJson = onHttpsRequestAsync(importGeoJsonCallback, {
+  memory: '4GiB',
+  timeoutSeconds: 3600,
+  cpu: 2,
+});
 
 export const exportCsv = onHttpsRequest(exportCsvHandler, {
   memory: '4GiB',

pnpm-workspace.yaml

@@ -34,7 +34,7 @@ catalogs:
     cors: ^2.8.5
     firebase: ^10.12.2
     firebase-admin: ^12.1.0
-    firebase-functions: ^7.2.2
+    firebase-functions: ^7.2.5
     google-auth-library: ^6.1.3
     googleapis: ^64.0.0
     http-status-codes: ^2.3.0