From 298bc045174e3b935af0809f04151e6ae09d63ac Mon Sep 17 00:00:00 2001
From: Marshall Polaris <marshall@pol.rs>
Date: Wed, 14 Sep 2022 00:59:42 -0700
Subject: [PATCH] Make bet denormalization script fast enough to run it on prod

---
 .../src/scripts/denormalize-bet-user-data.ts  | 72 ++++++-------------
 functions/src/scripts/denormalize.ts          | 20 ++++--
 2 files changed, 38 insertions(+), 54 deletions(-)

diff --git a/functions/src/scripts/denormalize-bet-user-data.ts b/functions/src/scripts/denormalize-bet-user-data.ts
index 3bb1aac1..3c86e140 100644
--- a/functions/src/scripts/denormalize-bet-user-data.ts
+++ b/functions/src/scripts/denormalize-bet-user-data.ts
@@ -1,61 +1,35 @@
-// Filling in the contract-based fields on comments.
+// Filling in the user-based fields on bets.
 
 import * as admin from 'firebase-admin'
 import { initAdmin } from './script-init'
-import { findDiffs, describeDiff, applyDiff } from './denormalize'
-import { DocumentSnapshot, Transaction } from 'firebase-admin/firestore'
-import { log } from '../utils'
+import { findDiffs, describeDiff, getDiffUpdate } from './denormalize'
+import { log, writeAsync } from '../utils'
 
 initAdmin()
 const firestore = admin.firestore()
 
-async function getUsersById(transaction: Transaction) {
-  const users = await transaction.get(firestore.collection('users'))
-  const results = Object.fromEntries(users.docs.map((doc) => [doc.id, doc]))
-  log(`Found ${users.size} users.`)
-  return results
-}
-
-async function getBetsByUserId(transaction: Transaction) {
-  const bets = await transaction.get(firestore.collectionGroup('bets'))
-  const results = new Map<string, DocumentSnapshot[]>()
-  bets.forEach((doc) => {
-    const userId = doc.get('userId')
-    const userBets = results.get(userId) || []
-    userBets.push(doc)
-    results.set(userId, userBets)
-  })
-  log(`Found ${bets.size} bets from ${results.size} users.`)
-  return results
-}
-
+// not in a transaction for speed -- may need to be run more than once
 async function denormalize() {
-  let hasMore = true
-  while (hasMore) {
-    hasMore = await admin.firestore().runTransaction(async (transaction) => {
-      const [usersById, betsByUserId] = await Promise.all([
-        getUsersById(transaction),
-        getBetsByUserId(transaction),
-      ])
-      const mapping = Object.entries(usersById).map(([id, doc]) => {
-        return [doc, betsByUserId.get(id) || []] as const
-      })
-      const diffs = findDiffs(
-        mapping,
-        ['avatarUrl', 'userAvatarUrl'],
-        ['name', 'userName'],
-        ['username', 'userUsername']
-      )
-      log(`Found ${diffs.length} bets with mismatched user data.`)
-      diffs.slice(0, 500).forEach((d) => {
-        log(describeDiff(d))
-        applyDiff(transaction, d)
-      })
-      if (diffs.length > 500) {
-        log(`Applying first 500 because of Firestore limit...`)
-      }
-      return diffs.length > 500
+  const users = await firestore.collection('users').get()
+  log(`Found ${users.size} users.`)
+  for (const userDoc of users.docs) {
+    const userBets = await firestore
+      .collectionGroup('bets')
+      .where('userId', '==', userDoc.id)
+      .get()
+    const mapping = [[userDoc, userBets.docs] as const] as const
+    const diffs = findDiffs(
+      mapping,
+      ['avatarUrl', 'userAvatarUrl'],
+      ['name', 'userName'],
+      ['username', 'userUsername']
+    )
+    log(`Found ${diffs.length} bets with mismatched user data.`)
+    const updates = diffs.map((d) => {
+      log(describeDiff(d))
+      return getDiffUpdate(d)
     })
+    await writeAsync(firestore, updates)
   }
 }
 
diff --git a/functions/src/scripts/denormalize.ts b/functions/src/scripts/denormalize.ts
index 743ab6a8..d4feb425 100644
--- a/functions/src/scripts/denormalize.ts
+++ b/functions/src/scripts/denormalize.ts
@@ -3,6 +3,7 @@
 
 import { DocumentSnapshot, Transaction } from 'firebase-admin/firestore'
 import { isEqual, zip } from 'lodash'
+import { UpdateSpec } from '../utils'
 
 export type DocumentValue = {
   doc: DocumentSnapshot
@@ -20,7 +21,10 @@ export type DocumentDiff = {
 
 type PathPair = readonly [string, string]
 
-export function findDiffs(docs: DocumentMapping[], ...paths: PathPair[]) {
+export function findDiffs(
+  docs: readonly DocumentMapping[],
+  ...paths: PathPair[]
+) {
   const diffs: DocumentDiff[] = []
   const srcPaths = paths.map((p) => p[0])
   const destPaths = paths.map((p) => p[1])
@@ -46,8 +50,14 @@ export function describeDiff(diff: DocumentDiff) {
   return `${describeDocVal(diff.src)} -> ${describeDocVal(diff.dest)}`
 }
 
-export function applyDiff(transaction: Transaction, diff: DocumentDiff) {
-  const { src, dest } = diff
-  const updateSpec = Object.fromEntries(zip(dest.fields, src.vals))
-  transaction.update(dest.doc.ref, updateSpec)
+export function getDiffUpdate(diff: DocumentDiff) {
+  return {
+    doc: diff.dest.doc.ref,
+    fields: Object.fromEntries(zip(diff.dest.fields, diff.src.vals)),
+  } as UpdateSpec
+}
+
+export function applyDiff(transaction: Transaction, diff: DocumentDiff) {
+  const update = getDiffUpdate(diff)
+  transaction.update(update.doc, update.fields)
 }