feat(cli): migrate sign up documents to auto-id

Author: abraham

docs/tutorials/firebase-utils.md

@@ -76,6 +76,8 @@ Commit the change to `site.json`, check the schedule and deploy. Then delete the
 
 Also delete the `config/schedule` document. `schedule.published` in `site.json` replaces it.
 
+`4.0.0-sign-up-ids` runs when a `subscribers` or `potentialPartners` document has the email as its ID. Before v4, the site saved sign-ups with the email, without its punctuation, as the ID, so the email showed in every link to them. The migration copies each one to a random ID, as the site writes them now, and deletes the old document in the same write. The plan shows only how many move, not the emails.
+
 ### Backups and undo
 
 Before it changes a document, `--fix` saves it as it was in `.firebase/backups/<date>/documents.json`. Git ignores the folder. To undo a fix:

packages/cli/src/lib/firestore-fix.test.ts

@@ -42,6 +42,21 @@ const fakeFirestore = (stale: string[] = []) => {
       set: async (...args: unknown[]) => writes.push(['set', path, ...args]),
       delete: async (...args: unknown[]) => writes.push(['delete', path, ...args]),
     }),
+    batch: () => {
+      const operations: unknown[][] = [];
+      return {
+        create: (ref: { path: string }, data: unknown) =>
+          operations.push(['create', ref.path, data]),
+        delete: (ref: { path: string }, precondition: unknown) =>
+          operations.push(['delete', ref.path, precondition]),
+        commit: async () => {
+          if (operations.some(([, path]) => stale.includes(path as string))) {
+            throw Object.assign(new Error('stale'), { code: 9 });
+          }
+          writes.push(['batch', ...operations]);
+        },
+      };
+    },
   } as unknown as Firestore;
   return { firestore, writes };
 };
@@ -96,6 +111,45 @@ describe('planFixes', () => {
 });
 
 describe('applyFixes', () => {
+  it('moves sign-ups off email IDs in one batch each, and backs them up', async () => {
+    const repo = makeRepo();
+    const { firestore, writes } = fakeFirestore(['subscribers/gracecompanycom']);
+    const ada = { email: 'ada@example.com', firstName: '', lastName: '' };
+    const grace = { email: 'grace@company.com', firstName: '', lastName: '' };
+    const documents = [
+      doc('subscribers/adaexamplecom', ada),
+      doc('subscribers/gracecompanycom', grace),
+    ];
+    const backupFolder = join(repo, 'backup');
+    const plan = planFixes(documents, 'UTC');
+    const [toAda] = plan.migrations[0]!.plan.moves!.map(({ to }) => to);
+
+    const result = await applyFixes({
+      firestore,
+      plan,
+      documents,
+      list: async () => documents,
+      timeZone: 'UTC',
+      repoRoot: repo,
+      backupFolder,
+    });
+
+    expect(result.skipped).toEqual(['subscribers/gracecompanycom']);
+    expect(writes).toEqual([
+      [
+        'batch',
+        ['create', toAda, ada],
+        ['delete', 'subscribers/adaexamplecom', { lastUpdateTime: at }],
+      ],
+    ]);
+    const backup = JSON.parse(readFileSync(join(backupFolder, 'documents.json'), 'utf8'));
+    expect(Object.keys(backup.documents)).toEqual([
+      'subscribers/adaexamplecom',
+      'subscribers/gracecompanycom',
+    ]);
+    expect(backup.created).toHaveLength(2);
+  });
+
   it('sets a missing updatedAt to the server time', async () => {
     const repo = makeRepo();
     const { firestore, writes } = fakeFirestore();

packages/cli/src/lib/firestore-fix.ts

@@ -184,10 +184,13 @@ export const applyFixes = async ({
   const byPath = new Map(documents.map((document) => [document.path, document]));
 
   for (const { migration, plan: migrationPlan } of plan.migrations) {
+    const moves = migrationPlan.moves ?? [];
     addToBackup(
       backupFolder,
-      migrationPlan.updates.flatMap(({ path }) => byPath.get(path) ?? []),
-      migrationPlan.creates.map(({ path }) => path),
+      [...migrationPlan.updates, ...moves.map(({ from }) => ({ path: from }))].flatMap(
+        ({ path }) => byPath.get(path) ?? [],
+      ),
+      [...migrationPlan.creates.map(({ path }) => path), ...moves.map(({ to }) => to)],
     );
     const before = { written: result.written, skipped: result.skipped.length };
     for (const { path, fields } of migrationPlan.updates) {
@@ -202,6 +205,20 @@ export const applyFixes = async ({
         result.skipped.push(path);
       }
     }
+    for (const { from, to } of moves) {
+      const document = byPath.get(from)!;
+      // One batch, so a document changed since the check is neither copied nor deleted.
+      const batch = firestore.batch();
+      batch.create(firestore.doc(to), document.raw!);
+      batch.delete(firestore.doc(from), { lastUpdateTime: document.updateTime as Timestamp });
+      try {
+        await batch.commit();
+        result.written++;
+      } catch (error) {
+        if (code(error) !== FAILED_PRECONDITION && code(error) !== ALREADY_EXISTS) throw error;
+        result.skipped.push(from);
+      }
+    }
     if (migrationPlan.site) writeSite(repoRoot, migrationPlan.site);
     // A migration that skipped documents stays pending, so --fix runs it again.
     if (result.skipped.length > before.skipped) continue;

packages/cli/src/migrations/4.0.0-sign-up-ids.test.ts

@@ -0,0 +1,50 @@
+import { describe, expect, it } from 'vitest';
+import { autoId, signUpIds } from './4.0.0-sign-up-ids.js';
+
+const subscriber = { email: 'ada@example.com', firstName: 'Ada', lastName: '' };
+const documents = [
+  { path: 'subscribers/adaexamplecom', data: subscriber },
+  { path: 'subscribers/ada@example.com', data: subscriber },
+  { path: 'subscribers/Xk3PqLm9TzR2aB7cD1eF', data: subscriber },
+  {
+    path: 'potentialPartners/gracecompanycom',
+    data: { companyName: '', email: 'grace@company.com', fullName: '' },
+  },
+  { path: 'speakers/adaexamplecom', data: { email: 'ada@example.com' } },
+];
+
+describe('4.0.0-sign-up-ids', () => {
+  it('is pending while a sign-up has its email, or its email without punctuation, as its ID', () => {
+    expect(signUpIds.pending(documents)).toBe(
+      'subscribers: 2 documents, potentialPartners: 1 document with the email as the ID.',
+    );
+    expect(signUpIds.pending(documents.slice(2))).toBe(
+      'potentialPartners: 1 document with the email as the ID.',
+    );
+    expect(signUpIds.pending([documents[2]!, documents[4]!])).toBeUndefined();
+  });
+
+  it('moves them to random IDs in the same collection, without showing the emails', () => {
+    const plan = signUpIds.plan(documents);
+
+    expect(plan.moves?.map(({ from }) => from)).toEqual([
+      'subscribers/adaexamplecom',
+      'subscribers/ada@example.com',
+      'potentialPartners/gracecompanycom',
+    ]);
+    expect(plan.moves?.map(({ to }) => to)).toEqual([
+      expect.stringMatching(/^subscribers\/[A-Za-z0-9]{20}$/),
+      expect.stringMatching(/^subscribers\/[A-Za-z0-9]{20}$/),
+      expect.stringMatching(/^potentialPartners\/[A-Za-z0-9]{20}$/),
+    ]);
+    expect(plan.lines).toEqual([
+      'subscribers: 2 documents move to random IDs',
+      'potentialPartners: 1 document moves to random IDs',
+    ]);
+    expect(JSON.stringify(plan.lines)).not.toMatch(/ada|grace/);
+  });
+
+  it('makes IDs like addDoc does', () => {
+    expect(new Set(Array.from({ length: 100 }, autoId)).size).toBe(100);
+  });
+});

packages/cli/src/migrations/4.0.0-sign-up-ids.ts

@@ -0,0 +1,58 @@
+import { randomInt } from 'crypto';
+import type { FirestoreDocument, Migration } from './types.js';
+
+const COLLECTIONS = ['subscribers', 'potentialPartners'];
+
+const plural = (amount: number, noun: string) => `${amount} ${noun}${amount === 1 ? '' : 's'}`;
+
+const AUTO_ID_CHARACTERS = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
+
+/** A random ID like the ones Firestore's `addDoc` makes. */
+export const autoId = () =>
+  Array.from({ length: 20 }, () => AUTO_ID_CHARACTERS[randomInt(AUTO_ID_CHARACTERS.length)]).join(
+    '',
+  );
+
+/** Sign-ups whose ID is their email, or their email without punctuation as older sites wrote. */
+const emailIds = (documents: FirestoreDocument[]) =>
+  documents.filter(({ path, data }) => {
+    const [collection, id, ...rest] = path.split('/');
+    if (rest.length || !COLLECTIONS.includes(collection!) || typeof data?.['email'] !== 'string') {
+      return false;
+    }
+    return id === data['email'] || id === data['email'].replace(/[^\w\s]/gi, '');
+  });
+
+const counts = (documents: FirestoreDocument[], verb = '') =>
+  COLLECTIONS.flatMap((collection) => {
+    const amount = documents.filter(({ path }) => path.startsWith(`${collection}/`)).length;
+    const suffix = verb && ` ${verb}${amount === 1 ? 's' : ''}`;
+    return amount ? [`${collection}: ${plural(amount, 'document')}${suffix}`] : [];
+  });
+
+/**
+ * Before v4, the site saved sign-ups with the email as the document ID, so the email showed in
+ * every link to them. This moves them to random IDs, as the site writes them now.
+ */
+export const signUpIds: Migration = {
+  id: '4.0.0-sign-up-ids',
+  description: 'Move sign-ups whose document ID is their email to random IDs.',
+  reads: [],
+
+  pending: (documents) => {
+    const found = emailIds(documents);
+    return found.length ? `${counts(found).join(', ')} with the email as the ID.` : undefined;
+  },
+
+  plan: (documents) => {
+    const found = emailIds(documents);
+    return {
+      updates: [],
+      creates: [],
+      moves: found.map(({ path }) => ({ from: path, to: `${path.split('/')[0]}/${autoId()}` })),
+      // The IDs are emails, so only the counts are shown.
+      lines: counts(found, 'move').map((line) => `${line} to random IDs`),
+      warnings: [],
+    };
+  },
+};

packages/cli/src/migrations/index.ts

@@ -1,10 +1,11 @@
 import type { FirestoreDocument } from './types.js';
 import { scheduleOnSessions } from './4.0.0-schedule-on-sessions.js';
+import { signUpIds } from './4.0.0-sign-up-ids.js';
 
 export type { FirestoreDocument, Migration, MigrationPlan } from './types.js';
 
 /** Every data migration, oldest first. */
-export const MIGRATIONS = [scheduleOnSessions];
+export const MIGRATIONS = [scheduleOnSessions, signUpIds];
 
 /**
  * The migrations the data still needs, with what shows it. The data decides, not

packages/cli/src/migrations/types.ts

@@ -13,6 +13,8 @@ export interface MigrationPlan {
   updates: { path: string; fields: Record<string, unknown> }[];
   /** New documents, which must not exist yet. */
   creates: { path: string; data: Record<string, unknown> }[];
+  /** Documents to copy to a new path, then delete. */
+  moves?: { from: string; to: string }[];
   /** A change to packages/config/site.json. */
   site?: (site: Record<string, unknown>) => Record<string, unknown>;
   /** What the migration changes, one line each. */