Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,41 @@ failed assertion rather than a failed setup. Each suite asserts both directions
allowed write succeeding and the forged one being denied — because a rule that rejects
everything would otherwise pass a suite made only of `assertFails`.

---

## Data migrations

The feed and the leaderboard currently download every note in the department and sort in
the client. Paginating them means ordering in the query instead — and Firestore's
ordering has two behaviours that turn old data into damage a user can see:

- **A document that lacks the ordered field is excluded from the query.** Not sorted
last: absent. A note written before `createdAt` existed would vanish from the feed
with nothing to indicate it ever had.
- **Values sort by type before value, and numbers come before timestamps.** A note whose
`createdAt` is epoch millis would land below every timestamped note whatever its date.

`toPost()` still reads both shapes, which is the evidence that both exist. So the data is
normalised before the ordering changes, not after:

```bash
cd firestore-tests
npm run backfill:created-at -- --project <your-project-id> # reports only
npm run backfill:created-at -- --project <your-project-id> --apply # writes
```

It needs application-default credentials (`gcloud auth application-default login`) and
is a dry run unless `--apply` is passed. Notes already carrying a `Timestamp` are left
alone, so it is safe to re-run after a partial failure. A note with no date anywhere is
given the epoch rather than the current time — inventing a recent date would float
undated legacy notes to the top of every feed, which is a louder lie than showing them
last — and the report says how many were treated that way before it writes anything.

[`backfill-created-at.test.js`](firestore-tests/backfill-created-at.test.js) runs it
against the emulator with legacy-shaped documents, including the assertion that matters
most: before the migration an ordered query over two notes returns one of them, and
after it returns both.

### Continuous integration

[`.github/workflows/android.yml`](.github/workflows/android.yml) runs on every push to
Expand Down Expand Up @@ -402,7 +437,8 @@ app/src/test/java/duygu/yilmaz/campusnote/
└── ui/ one test class per ViewModel, the two adapter
diff tests, and FeedScreenTest

firestore-tests/ emulator-backed tests for firestore.rules
firestore-tests/ emulator-backed tests for firestore.rules,
plus the createdAt backfill and its own test
```

---
Expand Down
168 changes: 168 additions & 0 deletions firestore-tests/backfill-created-at.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
#!/usr/bin/env node
/**
* Normalises every note's `createdAt` to a Firestore Timestamp.
*
* Why this exists: paginating the feed means ordering on `createdAt` in the query
* instead of sorting in the client, and Firestore's ordering has two behaviours that
* turn stale data into user-visible damage.
*
* - A document that lacks the ordered field is **excluded from the query entirely**.
* Not sorted last — absent. Any note written before `createdAt` was set would
* silently disappear from the feed.
* - Values are ordered by type before value, and numbers sort before timestamps. A
* note whose `createdAt` is epoch millis (a number) would land below every
* timestamped note regardless of its actual date.
*
* Both are invisible until someone notices their note is gone, so they are fixed
* before the ordering changes rather than after.
*
* The script reports by default and only writes when told to:
*
* node backfill-created-at.js --project <id> # dry run, changes nothing
* node backfill-created-at.js --project <id> --apply # writes
*
* Against production it needs application-default credentials:
*
* gcloud auth application-default login
*
* Against the emulator, set FIRESTORE_EMULATOR_HOST instead; no credentials are used.
*/

import { resolve } from "node:path";
import { fileURLToPath } from "node:url";
import { initializeApp } from "firebase-admin/app";
import { Firestore, Timestamp } from "firebase-admin/firestore";

const NOTES = "notes";
const FIELD = "createdAt";
// Firestore accepts at most 500 writes per batch.
const BATCH_LIMIT = 450;

/**
* A note with no date at all gets the epoch rather than "now": inventing a recent
* date would push undated legacy notes to the top of every feed, which is a louder
* lie than showing them last.
*/
const UNKNOWN_DATE = Timestamp.fromMillis(0);

/**
* @returns the Timestamp this document should carry, or null if it is already correct.
*/
export function normalisedCreatedAt(data) {
const value = data?.[FIELD];

if (value instanceof Timestamp) return null;

// Epoch millis written by an older version of the app.
if (typeof value === "number" && Number.isFinite(value)) {
return Timestamp.fromMillis(value);
}

// A string date is not a shape this app ever wrote, but parsing one is cheap and
// the alternative — treating it as undated — loses real information.
if (typeof value === "string") {
const parsed = Date.parse(value);
if (!Number.isNaN(parsed)) return Timestamp.fromMillis(parsed);
}

// Missing, null, or a shape we cannot read. `updatedAt` is the best remaining
// evidence of when the note existed.
const updated = data?.updatedAt;
if (updated instanceof Timestamp) return updated;
if (typeof updated === "number" && Number.isFinite(updated)) {
return Timestamp.fromMillis(updated);
}

return UNKNOWN_DATE;
}

/** @returns a short label for the report, so the counts say *why* a note was fixed. */
export function reasonFor(data) {
const value = data?.[FIELD];
if (value instanceof Timestamp) return "ok";
if (typeof value === "number") return "number";
if (typeof value === "string") return "string";
if (value === undefined || value === null) return "missing";
return "unreadable";
}

export async function backfill(firestore, { apply = false, log = console.log } = {}) {
const snapshot = await firestore.collection(NOTES).get();

const counts = { ok: 0, number: 0, string: 0, missing: 0, unreadable: 0 };
const pending = [];

for (const doc of snapshot.docs) {
const data = doc.data();
const reason = reasonFor(data);
counts[reason] += 1;

const replacement = normalisedCreatedAt(data);
if (replacement !== null) {
pending.push({ ref: doc.ref, value: replacement, id: doc.id, reason });
}
}

log(`notes scanned: ${snapshot.size}`);
log(` already a Timestamp: ${counts.ok}`);
log(` epoch millis: ${counts.number}`);
log(` date string: ${counts.string}`);
log(` missing: ${counts.missing}`);
log(` unreadable: ${counts.unreadable}`);
log(`needs a write: ${pending.length}`);

if (pending.length > 0 && counts.missing > 0) {
log(
` ${counts.missing} note(s) have no date at all and will be dated ` +
`${UNKNOWN_DATE.toDate().toISOString()}, placing them last in the feed.`
);
}

if (!apply) {
log("dry run — nothing was written. Re-run with --apply to write.");
return { scanned: snapshot.size, counts, written: 0 };
}

let written = 0;
for (let i = 0; i < pending.length; i += BATCH_LIMIT) {
const batch = firestore.batch();
for (const item of pending.slice(i, i + BATCH_LIMIT)) {
batch.update(item.ref, { [FIELD]: item.value });
}
await batch.commit();
written += Math.min(BATCH_LIMIT, pending.length - i);
log(`written: ${written}/${pending.length}`);
}

return { scanned: snapshot.size, counts, written };
}

/** Only runs when the file is executed directly, so the tests can import it. */
const runDirectly =
process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url);

if (runDirectly) {
const args = process.argv.slice(2);
const apply = args.includes("--apply");
const projectIndex = args.indexOf("--project");
const projectId =
projectIndex >= 0 ? args[projectIndex + 1] : process.env.GOOGLE_CLOUD_PROJECT;

if (!projectId) {
console.error("usage: node backfill-created-at.js --project <id> [--apply]");
process.exit(2);
}

initializeApp({ projectId });
const firestore = new Firestore({ projectId });

backfill(firestore, { apply })
.then((result) => {
console.log(apply ? `done — ${result.written} note(s) updated.` : "done.");
process.exit(0);
})
.catch((error) => {
console.error(error);
process.exit(1);
});
}
132 changes: 132 additions & 0 deletions firestore-tests/backfill-created-at.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import { initializeApp, deleteApp } from "firebase-admin/app";
import { Firestore, Timestamp } from "firebase-admin/firestore";
import { afterAll, beforeAll, beforeEach, describe, expect, test } from "vitest";
import { backfill, normalisedCreatedAt, reasonFor } from "./backfill-created-at.js";

// The emulator is already running (npm test wraps the suite in emulators:exec), and
// the admin SDK talks to it as long as this is set before it connects.
process.env.FIRESTORE_EMULATOR_HOST ??= "127.0.0.1:8080";

const PROJECT = "campusnote-rules-test";

let app;
let firestore;

/** Swallows the script's progress output so the test run stays readable. */
const quiet = () => {};

beforeAll(() => {
app = initializeApp({ projectId: PROJECT }, "backfill-test");
firestore = new Firestore({ projectId: PROJECT });
});

afterAll(async () => {
await deleteApp(app);
});

beforeEach(async () => {
const existing = await firestore.collection("notes").get();
await Promise.all(existing.docs.map((doc) => doc.ref.delete()));
});

const note = (extra) => ({
title: "Ders notu",
department: "Bilgisayar Mühendisliği",
uploaderUid: "uid-1",
...extra,
});

describe("classifying a note", () => {
test("a proper Timestamp needs no write", () => {
const data = note({ createdAt: Timestamp.fromMillis(1_700_000_000_000) });

expect(reasonFor(data)).toBe("ok");
expect(normalisedCreatedAt(data)).toBeNull();
});

test("epoch millis keep their date", () => {
const millis = 1_700_000_000_000;

const replacement = normalisedCreatedAt(note({ createdAt: millis }));

expect(reasonFor(note({ createdAt: millis }))).toBe("number");
expect(replacement.toMillis()).toBe(millis);
});

test("a missing date falls back to updatedAt when there is one", () => {
const updated = Timestamp.fromMillis(1_600_000_000_000);

const replacement = normalisedCreatedAt(note({ updatedAt: updated }));

expect(replacement.toMillis()).toBe(updated.toMillis());
});

test("a note with no evidence at all is dated to the epoch, not to now", () => {
// Dating it "now" would float undated legacy notes to the top of every feed.
const replacement = normalisedCreatedAt(note({}));

expect(replacement.toMillis()).toBe(0);
});
});

describe("running against the emulator", () => {
test("a dry run writes nothing", async () => {
await firestore.collection("notes").doc("legacy").set(note({ createdAt: 123 }));

const result = await backfill(firestore, { apply: false, log: quiet });

expect(result.written).toBe(0);
const after = await firestore.collection("notes").doc("legacy").get();
expect(after.data().createdAt).toBe(123);
});

test("applying converts every shape and leaves correct notes alone", async () => {
const good = Timestamp.fromMillis(1_700_000_000_000);
await firestore.collection("notes").doc("ok").set(note({ createdAt: good }));
await firestore.collection("notes").doc("millis").set(note({ createdAt: 1_650_000_000_000 }));
await firestore.collection("notes").doc("absent").set(note({}));

const result = await backfill(firestore, { apply: true, log: quiet });

expect(result.scanned).toBe(3);
expect(result.written).toBe(2);
expect(result.counts).toMatchObject({ ok: 1, number: 1, missing: 1 });

const docs = await firestore.collection("notes").get();
for (const doc of docs.docs) {
expect(doc.data().createdAt).toBeInstanceOf(Timestamp);
}
const untouched = await firestore.collection("notes").doc("ok").get();
expect(untouched.data().createdAt.toMillis()).toBe(good.toMillis());
const converted = await firestore.collection("notes").doc("millis").get();
expect(converted.data().createdAt.toMillis()).toBe(1_650_000_000_000);
});

test("running it twice changes nothing the second time", async () => {
// The whole point is that this is safe to re-run after a partial failure.
await firestore.collection("notes").doc("millis").set(note({ createdAt: 1 }));
await firestore.collection("notes").doc("absent").set(note({}));

await backfill(firestore, { apply: true, log: quiet });
const second = await backfill(firestore, { apply: true, log: quiet });

expect(second.written).toBe(0);
expect(second.counts.ok).toBe(2);
});

test("after the backfill, an ordered query returns every note", async () => {
// This is the behaviour the whole migration exists for: before it, the note
// without a createdAt is missing from the ordered query entirely.
await firestore.collection("notes").doc("dated").set(note({ createdAt: Timestamp.now() }));
await firestore.collection("notes").doc("undated").set(note({}));

const before = await firestore.collection("notes").orderBy("createdAt", "desc").get();
expect(before.size).toBe(1);

await backfill(firestore, { apply: true, log: quiet });

const after = await firestore.collection("notes").orderBy("createdAt", "desc").get();
expect(after.size).toBe(2);
expect(after.docs.map((doc) => doc.id)).toEqual(["dated", "undated"]);
});
});
Loading
Loading