From a926f13f77752c2fb135771d810d0c67605444c9 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 10 Sep 2024 13:47:00 -0400 Subject: [PATCH 1/4] add a function and script for cleaning old roots --- .vscode/launch.json | 9 +++++ functions-v2/jest.config.js | 9 +++++ functions-v2/package-lock.json | 13 +++---- functions-v2/package.json | 7 ++-- functions-v2/src/at-midnight.ts | 29 +++++++++++++++ functions-v2/src/index.ts | 1 + functions-v2/test/at-midnight.test.ts | 39 ++++++++++++++++++++ functions-v2/tsconfig.json | 8 +++++ scripts/clean-firebase-roots.ts | 33 +++++++++++++++++ shared/clean-firebase-roots.ts | 51 +++++++++++++++++++++++++++ 10 files changed, 190 insertions(+), 9 deletions(-) create mode 100644 functions-v2/src/at-midnight.ts create mode 100644 functions-v2/test/at-midnight.test.ts create mode 100644 scripts/clean-firebase-roots.ts create mode 100644 shared/clean-firebase-roots.ts diff --git a/.vscode/launch.json b/.vscode/launch.json index f2df090b26..5a117a8249 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -14,6 +14,15 @@ "console": "integratedTerminal", "program": "${workspaceFolder}/node_modules/.bin/jest", "args": ["${fileBasenameNoExtension}"] + }, + { + "name": "Debug functions-v2 test", + "request": "launch", + "type": "node", + "console": "integratedTerminal", + "program": "${workspaceFolder}/functions-v2/node_modules/.bin/jest", + "args": ["${fileBasenameNoExtension}"], + "cwd": "${workspaceFolder}/functions-v2" } ] } diff --git a/functions-v2/jest.config.js b/functions-v2/jest.config.js index c712b35941..69d187da25 100644 --- a/functions-v2/jest.config.js +++ b/functions-v2/jest.config.js @@ -2,6 +2,15 @@ module.exports = { preset: 'ts-jest', testEnvironment: 'node', testPathIgnorePatterns: ['lib/', 'node_modules/'], + moduleNameMapper: { + // These are necessary so code imported from ../shared/ will use the same version of + // firebase-admin that the local code does. + // The explicit `^` and `$` are needed so this only matches what we are importing. + // Otherwise it breaks the internal firebase admin code's imports + "^firebase-admin$": "/node_modules/firebase-admin", + "^firebase-admin/firestore$": "/node_modules/firebase-admin/lib/firestore", + "^firebase-admin/app$": "/node_modules/firebase-admin/lib/app", + } }; // This is configured here because the clearFirebaseData function from diff --git a/functions-v2/package-lock.json b/functions-v2/package-lock.json index 3afbb3fb08..b7fc085d58 100644 --- a/functions-v2/package-lock.json +++ b/functions-v2/package-lock.json @@ -1,13 +1,13 @@ { - "name": "functions", + "name": "functions-v2", "lockfileVersion": 3, "requires": true, "packages": { "": { - "name": "functions", + "name": "functions-v2", "dependencies": { "firebase-admin": "^12.1.0", - "firebase-functions": "^5.0.0" + "firebase-functions": "^5.1.1" }, "devDependencies": { "@jest/globals": "^29.7.0", @@ -5823,9 +5823,10 @@ } }, "node_modules/firebase-functions": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/firebase-functions/-/firebase-functions-5.0.1.tgz", - "integrity": "sha512-1m+crtgAR8Tl36gjpM02KCY5zduAejFmDSXvih/DB93apg39f0U/WwRgT7sitGIRqyCcIpktNUbXJv7Y9JOF4A==", + "version": "5.1.1", + "resolved": "https://registry.npmjs.org/firebase-functions/-/firebase-functions-5.1.1.tgz", + "integrity": "sha512-KkyKZE98Leg/C73oRyuUYox04PQeeBThdygMfeX+7t1cmKWYKa/ZieYa89U8GHgED+0mF7m7wfNZOfbURYxIKg==", + "license": "MIT", "dependencies": { "@types/cors": "^2.8.5", "@types/express": "4.17.3", diff --git a/functions-v2/package.json b/functions-v2/package.json index 7f76e6aaab..3ccecef6e9 100644 --- a/functions-v2/package.json +++ b/functions-v2/package.json @@ -5,8 +5,9 @@ "build": "tsc", "build:watch": "tsc --watch", "emulator": "firebase emulators:start --project demo-test", + "emulator:online": "firebase emulators:start", "serve": "npm run build && firebase emulators:start --only functions", - "shell": "npm run build && firebase functions:shell", + "shell": "npm run build && firebase functions:shell --project demo-test", "start": "npm run shell", "test": "jest", "test:emulator": "firebase emulators:start --project demo-test --only firestore,database", @@ -16,10 +17,10 @@ "engines": { "node": "20" }, - "main": "lib/src/index.js", + "main": "lib/functions-v2/src/index.js", "dependencies": { "firebase-admin": "^12.1.0", - "firebase-functions": "^5.0.0" + "firebase-functions": "^5.1.1" }, "devDependencies": { "@jest/globals": "^29.7.0", diff --git a/functions-v2/src/at-midnight.ts b/functions-v2/src/at-midnight.ts new file mode 100644 index 0000000000..0c04a6ef3f --- /dev/null +++ b/functions-v2/src/at-midnight.ts @@ -0,0 +1,29 @@ +import {onSchedule} from "firebase-functions/v2/scheduler"; +import * as logger from "firebase-functions/logger"; + +// NOTE: in order for this import from shared to work it is necessary +// to alias "firebase-admin" in tsconfig.json. Otherwise Typescript will +// read the types from the parent node_modules. The parent directory +// has a different version of the firebase dependencies, which cause +// type errors. +import {cleanFirebaseRoots} from "../../shared/clean-firebase-roots"; + +export const atMidnight = onSchedule("0 0 * * *", runAtMidnight); + +// This function is split out so it can be tested by Jest. The +// firebase-functions-test library doesn't support wrapping onSchedule. +export async function runAtMidnight() { + await cleanFirebaseRoots({ + appMode: "qa", + // hoursAgo: 6, + hoursAgo: 90.7, + logger, + dryRun: true, + }); + + // When cleanFirebaseRoots is called from a NodeJS script it is + // necessary to call Firebase's deleteApp so no threads are left running. + // Inside of a firebase function according to + // https://stackoverflow.com/a/72933644/3195497 + // it isn't necessary to call deleteApp when the function is done. +} diff --git a/functions-v2/src/index.ts b/functions-v2/src/index.ts index 1d7950a314..94d32083d1 100644 --- a/functions-v2/src/index.ts +++ b/functions-v2/src/index.ts @@ -1,4 +1,5 @@ import * as admin from "firebase-admin"; export {onUserDocWritten} from "./on-user-doc-written"; +export {atMidnight} from "./at-midnight"; admin.initializeApp(); diff --git a/functions-v2/test/at-midnight.test.ts b/functions-v2/test/at-midnight.test.ts new file mode 100644 index 0000000000..7d86c4a8ba --- /dev/null +++ b/functions-v2/test/at-midnight.test.ts @@ -0,0 +1,39 @@ +import initializeFFT from "firebase-functions-test"; +import { + clearFirestoreData, +} from "firebase-functions-test/lib/providers/firestore"; +import {initializeApp} from "firebase-admin/app"; +// import {getFirestore} from "firebase-admin/firestore"; +import {runAtMidnight} from "../src/at-midnight"; + +process.env["FIRESTORE_EMULATOR_HOST"]="127.0.0.1:8088"; +const projectConfig = {projectId: "demo-test"}; +const fft = initializeFFT(projectConfig); + +// When the function is running in the cloud initializeApp is called by index.ts +// Here we are importing the function's module directly so we can call +// initializeApp ourselves. This is beneficial since initializeApp needs to +// be called after initializeFFT above. +initializeApp(); + +// type CollectionRef = admin.firestore.CollectionReference< +// admin.firestore.DocumentData, admin.firestore.DocumentData +// >; + +describe("atMidnight", () => { + beforeEach(async () => { + await clearFirestoreData(projectConfig); + }); + + test("clean up firestore roots", async () => { + // The wrapper doesn't support onSchedule. The Typescript types don't allow it + // and at run time it doesn't pass the right event: + // https://github.com/firebase/firebase-functions-test/issues/210 + // const wrapped = fft.wrap(atMidnight); + await runAtMidnight(); + }); + + afterAll(() => { + fft.cleanup(); + }); +}); diff --git a/functions-v2/tsconfig.json b/functions-v2/tsconfig.json index 4990f4585e..0c341d879f 100644 --- a/functions-v2/tsconfig.json +++ b/functions-v2/tsconfig.json @@ -11,6 +11,14 @@ // This prevents typescript from trying to include @types from the parent folders. // The types in the parent folders conflict so they break the build. "typeRoots": ["./node_modules/@types"], + "paths": { + // These are necessary so code imported from ../shared/ will use the same version of + // firebase-admin that the local code does. + "firebase-admin": ["./node_modules/firebase-admin/lib"], + "firebase-admin/firestore": ["./node_modules/firebase-admin/lib/firestore"], + "firebase-admin/app": ["./node_modules/firebase-admin/lib/app"], + }, + }, "compileOnSave": true, "include": [ diff --git a/scripts/clean-firebase-roots.ts b/scripts/clean-firebase-roots.ts new file mode 100644 index 0000000000..4965493663 --- /dev/null +++ b/scripts/clean-firebase-roots.ts @@ -0,0 +1,33 @@ +#!/usr/bin/node + +// This script finds documents without metadata in the realtime database. +// If the deleteTypes array is uncommented, it will delete these documents. + +// to run this script type the following in the terminal +// cf. https://stackoverflow.com/a/66626333/16328462 +// $ cd scripts/ai +// $ npx tsx clean-firebase-roots.ts + +import admin from "firebase-admin"; +import { deleteApp } from "firebase-admin/app"; +import {cleanFirebaseRoots} from "../shared/clean-firebase-roots.js"; +import { getScriptRootFilePath } from "./lib/script-utils.js"; + +const databaseURL = "https://collaborative-learning-ec215.firebaseio.com"; + +const serviceAccountFile = getScriptRootFilePath("serviceAccountKey.json"); +const credential = admin.credential.cert(serviceAccountFile); +// Initialize the app with a service account, granting admin privileges +const fbApp = admin.initializeApp({ + credential, + databaseURL +}); + +await cleanFirebaseRoots({ + appMode: "qa", + hoursAgo: 90.7, + logger: console, + dryRun: true +}); + +await deleteApp(fbApp); diff --git a/shared/clean-firebase-roots.ts b/shared/clean-firebase-roots.ts new file mode 100644 index 0000000000..9faa5e78bc --- /dev/null +++ b/shared/clean-firebase-roots.ts @@ -0,0 +1,51 @@ +// This requires the modern firebase-admin, so it can't be used by functions-v1 +import {Timestamp, getFirestore} from "firebase-admin/firestore"; +import {getDatabase} from "firebase-admin/database"; + +const HOUR = 1000 * 60 * 60; + +interface Logger { + info(...args: any[]): void; +} + +interface Params { + appMode: "qa" | "dev"; + hoursAgo: number; + logger: Logger; + dryRun?: boolean; +} + +export async function cleanFirebaseRoots( + { appMode, hoursAgo, logger, dryRun }: Params +) { + + // Be extra careful so we don't delete production data + if (!["qa", "dev"].includes(appMode)) { + throw new Error(`Invalid appMode ${appMode}`); + } + + // Clean up Firestore and Realtime database roots that haven't been updated in a 6 hours + const cutOffMillis = Date.now() - hoursAgo*HOUR; + const qaRootsResult = await getFirestore() + .collection(appMode) + .where("lastLaunchTime", "<", Timestamp.fromMillis(cutOffMillis)) + .get(); + + logger.info(`Found ${qaRootsResult.size} roots to delete`); + + // Need to be careful to clean up the root in the realtime database + // first. The record in Firestore is our only way to figure out which + // roots in the realtime database need to be deleted. + for (const root of qaRootsResult.docs) { + // The Realtime database root is deleted first incase it fails. + // This way the root in firestore will remain so we can find it + // and try again later. + const databasePath = `/${appMode}/${root.id}`; + logger.info(`Deleting Realtime Database root: ${databasePath} ...`); + // TODO: what if the ref doesn't exist? + if (!dryRun) await getDatabase().ref(`/${appMode}/${root.id}`).remove(); + logger.info(`Deleting Firestore root: ${root.ref.path} ...`); + if (!dryRun) await getFirestore().recursiveDelete(root.ref); + } + +} From 2fd224718edb5a10396cc2c714fafefb52c7bf0e Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Wed, 11 Sep 2024 11:15:47 -0400 Subject: [PATCH 2/4] add test for cleaning roots This also refactors out the initialization code from the 2 tests. It took a while to figure out the right configuration so Jest would not pick up two versions of firebase-admin dependency since it also exists in the parent node_modules. --- functions-v2/jest.config.js | 7 +- functions-v2/src/at-midnight.ts | 5 +- functions-v2/test/at-midnight.test.ts | 97 +++++++++++++++---- functions-v2/test/initialize.ts | 30 ++++++ functions-v2/test/on-user-doc-written.test.ts | 16 +-- functions-v2/tsconfig.json | 5 +- shared/clean-firebase-roots.ts | 1 - 7 files changed, 122 insertions(+), 39 deletions(-) create mode 100644 functions-v2/test/initialize.ts diff --git a/functions-v2/jest.config.js b/functions-v2/jest.config.js index 69d187da25..879629c095 100644 --- a/functions-v2/jest.config.js +++ b/functions-v2/jest.config.js @@ -10,7 +10,11 @@ module.exports = { "^firebase-admin$": "/node_modules/firebase-admin", "^firebase-admin/firestore$": "/node_modules/firebase-admin/lib/firestore", "^firebase-admin/app$": "/node_modules/firebase-admin/lib/app", - } + "^firebase-admin/database$": "/node_modules/firebase-admin/lib/database", + }, + // The tests can't be run in parallel because they are using a shared Firestore and + // Realtime database. + maxWorkers: 1, }; // This is configured here because the clearFirebaseData function from @@ -19,3 +23,4 @@ module.exports = { // The port here should match the port that is set in the emulators // section of firebase.json process.env["FIRESTORE_EMULATOR_HOST"]="127.0.0.1:8088"; +process.env["FIREBASE_DATABASE_EMULATOR_HOST"]="127.0.0.1:9000"; diff --git a/functions-v2/src/at-midnight.ts b/functions-v2/src/at-midnight.ts index 0c04a6ef3f..692d98b87f 100644 --- a/functions-v2/src/at-midnight.ts +++ b/functions-v2/src/at-midnight.ts @@ -15,10 +15,9 @@ export const atMidnight = onSchedule("0 0 * * *", runAtMidnight); export async function runAtMidnight() { await cleanFirebaseRoots({ appMode: "qa", - // hoursAgo: 6, - hoursAgo: 90.7, + hoursAgo: 6, logger, - dryRun: true, + dryRun: false, }); // When cleanFirebaseRoots is called from a NodeJS script it is diff --git a/functions-v2/test/at-midnight.test.ts b/functions-v2/test/at-midnight.test.ts index 7d86c4a8ba..d313b40140 100644 --- a/functions-v2/test/at-midnight.test.ts +++ b/functions-v2/test/at-midnight.test.ts @@ -1,39 +1,94 @@ -import initializeFFT from "firebase-functions-test"; import { clearFirestoreData, } from "firebase-functions-test/lib/providers/firestore"; -import {initializeApp} from "firebase-admin/app"; -// import {getFirestore} from "firebase-admin/firestore"; +import {getFirestore, Timestamp} from "firebase-admin/firestore"; +import {getDatabase} from "firebase-admin/database"; +import * as logger from "firebase-functions/logger"; +import {initialize, projectConfig} from "./initialize"; import {runAtMidnight} from "../src/at-midnight"; -process.env["FIRESTORE_EMULATOR_HOST"]="127.0.0.1:8088"; -const projectConfig = {projectId: "demo-test"}; -const fft = initializeFFT(projectConfig); +jest.mock("firebase-functions/logger"); -// When the function is running in the cloud initializeApp is called by index.ts -// Here we are importing the function's module directly so we can call -// initializeApp ourselves. This is beneficial since initializeApp needs to -// be called after initializeFFT above. -initializeApp(); +const {cleanup} = initialize(); -// type CollectionRef = admin.firestore.CollectionReference< -// admin.firestore.DocumentData, admin.firestore.DocumentData -// >; +const HOUR = 1000 * 60 * 60; + +async function writeFirestoreRoot(lastLaunchMillis = 0) { + const newRoot = getFirestore() + .collection("qa") + .doc(); + + await newRoot.set({ + lastLaunchTime: Timestamp.fromMillis(lastLaunchMillis), + }); + + // Add some sub docs to make sure they are deleted + await newRoot.collection("users").doc().set({ + uid: "test-user", + }); + + return newRoot; +} + +async function writeDatabaseRoot(rootId: string) { + getDatabase().ref("qa").child(rootId).set({someField: "firebase realtime database"}); +} + +// In other tests we use firebase-functions-test to wrap the function. +// In this case it would look like: +// const wrapped = fft.wrap(atMidnight); +// However the wrapper doesn't support onSchedule: +// - The Typescript types don't allow it +// - at run time it doesn't pass the right event: +// https://github.com/firebase/firebase-functions-test/issues/210 +// So instead the code is separated from the onSchedule and called directly. describe("atMidnight", () => { beforeEach(async () => { await clearFirestoreData(projectConfig); + await getDatabase().ref().set(null); }); - test("clean up firestore roots", async () => { - // The wrapper doesn't support onSchedule. The Typescript types don't allow it - // and at run time it doesn't pass the right event: - // https://github.com/firebase/firebase-functions-test/issues/210 - // const wrapped = fft.wrap(atMidnight); + test("clean up firestore roots with no database roots", async () => { + await writeFirestoreRoot(); await runAtMidnight(); + + const roots = await getFirestore().collection("qa").get(); + expect(roots.size).toBe(0); + expect(logger.info) + .toHaveBeenCalledWith("Found 1 roots to delete"); + }); + + test("clean up firestore root and database root", async () => { + const firestoreRoot = await writeFirestoreRoot(); + await writeDatabaseRoot(firestoreRoot.id); + + await runAtMidnight(); + + const fsRoots = await getFirestore().collection("qa").get(); + expect(fsRoots.size).toBe(0); + const dbRoots = await getDatabase().ref("qa").get(); + expect(dbRoots.val()).toEqual(null); + expect(logger.info) + .toHaveBeenCalledWith("Found 1 roots to delete"); + }); + + test("only clean up firestore roots older than 6 hours", async () => { + await writeFirestoreRoot(Date.now() - HOUR); + await writeFirestoreRoot(Date.now() - 2*HOUR); + await writeFirestoreRoot(Date.now() - 5*HOUR); + await writeFirestoreRoot(Date.now() - 8*HOUR); + await writeFirestoreRoot(Date.now() - 24*HOUR); + + await runAtMidnight(); + + const roots = await getFirestore().collection("qa").get(); + expect(roots.size).toBe(3); + expect(logger.info) + .toHaveBeenCalledWith("Found 2 roots to delete"); }); - afterAll(() => { - fft.cleanup(); + afterAll(async () => { + await cleanup(); }); }); diff --git a/functions-v2/test/initialize.ts b/functions-v2/test/initialize.ts new file mode 100644 index 0000000000..61497f4429 --- /dev/null +++ b/functions-v2/test/initialize.ts @@ -0,0 +1,30 @@ +import {deleteApp, initializeApp} from "firebase-admin/app"; +import initializeFFT from "firebase-functions-test"; + +export const projectConfig = { + projectId: "demo-test", + // This URL doesn't have to be valid, it just has to a non empty string + // The actual database host will be picked up from + // FIREBASE_DATABASE_EMULATOR_HOST + // This is defined in jest.config.js + databaseURL: "https://not-a-project.firebaseio.com", +}; + +export function initialize() { + const fft = initializeFFT(projectConfig); + + // When the function is running in the cloud initializeApp is called by index.ts + // In our tests we import the function's module directly so we can call + // initializeApp ourselves. This is beneficial since initializeApp needs to + // be called after initializeFFT above. + const fbApp = initializeApp(); + + const cleanup = async () => { + fft.cleanup(); + // Deleting the Firebase app is necessary for the Jest tests to exit when they + // are complete. FFT creates a testApp which it deletes in cleanup(), but + // we are not using this testApp. + await deleteApp(fbApp); + }; + return {fft, fbApp, cleanup}; +} diff --git a/functions-v2/test/on-user-doc-written.test.ts b/functions-v2/test/on-user-doc-written.test.ts index a6b27333e6..9bcc865cf5 100644 --- a/functions-v2/test/on-user-doc-written.test.ts +++ b/functions-v2/test/on-user-doc-written.test.ts @@ -1,22 +1,14 @@ -import initializeFFT from "firebase-functions-test"; import { clearFirestoreData, } from "firebase-functions-test/lib/providers/firestore"; import * as logger from "firebase-functions/logger"; import * as admin from "firebase-admin"; +import {initialize, projectConfig} from "./initialize"; import {onUserDocWritten} from "../src/on-user-doc-written"; jest.mock("firebase-functions/logger"); -process.env["FIRESTORE_EMULATOR_HOST"]="127.0.0.1:8088"; -const projectConfig = {projectId: "demo-test"}; -const fft = initializeFFT(projectConfig); - -// When the function is running in the cloud initializeApp is called by index.ts -// Here we are importing the function's module directly so we can call -// initializeApp ourselves. This is beneficial since initializeApp needs to -// be called after initializeFFT above. -admin.initializeApp(); +const {fft, cleanup} = initialize(); type CollectionRef = admin.firestore.CollectionReference< admin.firestore.DocumentData, admin.firestore.DocumentData @@ -288,7 +280,7 @@ describe("functions", () => { }); }); - afterAll(() => { - fft.cleanup(); + afterAll(async () => { + await cleanup(); }); }); diff --git a/functions-v2/tsconfig.json b/functions-v2/tsconfig.json index 0c341d879f..c3db751f1f 100644 --- a/functions-v2/tsconfig.json +++ b/functions-v2/tsconfig.json @@ -13,10 +13,13 @@ "typeRoots": ["./node_modules/@types"], "paths": { // These are necessary so code imported from ../shared/ will use the same version of - // firebase-admin that the local code does. + // firebase-admin that the local code does. Technically only "firebase-admin/firestore" + // seems to be currently required, but it seems safer to alias all of the admin + // libraries the shared code might be using. "firebase-admin": ["./node_modules/firebase-admin/lib"], "firebase-admin/firestore": ["./node_modules/firebase-admin/lib/firestore"], "firebase-admin/app": ["./node_modules/firebase-admin/lib/app"], + "firebase-admin/database": ["./node_modules/firebase-admin/lib/database"], }, }, diff --git a/shared/clean-firebase-roots.ts b/shared/clean-firebase-roots.ts index 9faa5e78bc..dc71bffb13 100644 --- a/shared/clean-firebase-roots.ts +++ b/shared/clean-firebase-roots.ts @@ -42,7 +42,6 @@ export async function cleanFirebaseRoots( // and try again later. const databasePath = `/${appMode}/${root.id}`; logger.info(`Deleting Realtime Database root: ${databasePath} ...`); - // TODO: what if the ref doesn't exist? if (!dryRun) await getDatabase().ref(`/${appMode}/${root.id}`).remove(); logger.info(`Deleting Firestore root: ${root.ref.path} ...`); if (!dryRun) await getFirestore().recursiveDelete(root.ref); From 3fb3b355ae165ebeb311a2670f457c91dc27da35 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Wed, 11 Sep 2024 12:23:04 -0400 Subject: [PATCH 3/4] Wait 24 hours before deleting a root. --- functions-v2/src/at-midnight.ts | 2 +- functions-v2/test/at-midnight.test.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/functions-v2/src/at-midnight.ts b/functions-v2/src/at-midnight.ts index 692d98b87f..73923325b2 100644 --- a/functions-v2/src/at-midnight.ts +++ b/functions-v2/src/at-midnight.ts @@ -15,7 +15,7 @@ export const atMidnight = onSchedule("0 0 * * *", runAtMidnight); export async function runAtMidnight() { await cleanFirebaseRoots({ appMode: "qa", - hoursAgo: 6, + hoursAgo: 24, logger, dryRun: false, }); diff --git a/functions-v2/test/at-midnight.test.ts b/functions-v2/test/at-midnight.test.ts index d313b40140..9639eadafe 100644 --- a/functions-v2/test/at-midnight.test.ts +++ b/functions-v2/test/at-midnight.test.ts @@ -73,12 +73,12 @@ describe("atMidnight", () => { .toHaveBeenCalledWith("Found 1 roots to delete"); }); - test("only clean up firestore roots older than 6 hours", async () => { + test("only clean up firestore roots older than 24 hours", async () => { await writeFirestoreRoot(Date.now() - HOUR); - await writeFirestoreRoot(Date.now() - 2*HOUR); - await writeFirestoreRoot(Date.now() - 5*HOUR); - await writeFirestoreRoot(Date.now() - 8*HOUR); - await writeFirestoreRoot(Date.now() - 24*HOUR); + await writeFirestoreRoot(Date.now() - 12*HOUR); + await writeFirestoreRoot(Date.now() - 23*HOUR); + await writeFirestoreRoot(Date.now() - 25*HOUR); + await writeFirestoreRoot(Date.now() - 48*HOUR); await runAtMidnight(); From 6c92170fa0f7ec596e61d260af4afe6644fb1d83 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Wed, 11 Sep 2024 15:10:42 -0400 Subject: [PATCH 4/4] address PR comments --- functions-v2/src/at-midnight.ts | 2 +- scripts/clean-firebase-roots.ts | 6 +++--- shared/clean-firebase-roots.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/functions-v2/src/at-midnight.ts b/functions-v2/src/at-midnight.ts index 73923325b2..e4c8cef364 100644 --- a/functions-v2/src/at-midnight.ts +++ b/functions-v2/src/at-midnight.ts @@ -8,7 +8,7 @@ import * as logger from "firebase-functions/logger"; // type errors. import {cleanFirebaseRoots} from "../../shared/clean-firebase-roots"; -export const atMidnight = onSchedule("0 0 * * *", runAtMidnight); +export const atMidnight = onSchedule("0 7 * * *", runAtMidnight); // This function is split out so it can be tested by Jest. The // firebase-functions-test library doesn't support wrapping onSchedule. diff --git a/scripts/clean-firebase-roots.ts b/scripts/clean-firebase-roots.ts index 4965493663..bd39912791 100644 --- a/scripts/clean-firebase-roots.ts +++ b/scripts/clean-firebase-roots.ts @@ -1,11 +1,11 @@ #!/usr/bin/node -// This script finds documents without metadata in the realtime database. -// If the deleteTypes array is uncommented, it will delete these documents. +// This script cleans the roots out of the QA or Dev sections of +// the Firebase Realtime database and Firestore. // to run this script type the following in the terminal // cf. https://stackoverflow.com/a/66626333/16328462 -// $ cd scripts/ai +// $ cd scripts // $ npx tsx clean-firebase-roots.ts import admin from "firebase-admin"; diff --git a/shared/clean-firebase-roots.ts b/shared/clean-firebase-roots.ts index dc71bffb13..7b300e1149 100644 --- a/shared/clean-firebase-roots.ts +++ b/shared/clean-firebase-roots.ts @@ -24,7 +24,7 @@ export async function cleanFirebaseRoots( throw new Error(`Invalid appMode ${appMode}`); } - // Clean up Firestore and Realtime database roots that haven't been updated in a 6 hours + // Clean up Firestore and Realtime database roots that haven't been launched in `hoursAgo` hours const cutOffMillis = Date.now() - hoursAgo*HOUR; const qaRootsResult = await getFirestore() .collection(appMode)