diff --git a/README.md b/README.md index 7cd43b7..51c9c44 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,13 @@ flowchart TD transaction, so two people rating the same note at once cannot lose one of the votes. - **File content lives in a subcollection.** `notes/{id}/content/file` is separate from the note metadata so feed and leaderboard queries never download file data. +- **Files are base64 in Firestore, not in Storage.** Firebase Storage is the usual home + for binary data, but it requires the billed Blaze plan and this project stays on the + free Spark plan. Encoding the file into a Firestore document keeps uploads working + within that constraint, at a known cost: base64 inflates data by about a third and a + Firestore document may not exceed 1 MiB, so the practical ceiling is ~650 KB. The app + is built around that number rather than surprised by it — images are downscaled and + re-compressed to fit, and larger PDFs are rejected with a message that says why. - **Rating arithmetic is a pure function.** `RatingCalculator` has no Firebase dependency, which is what makes it unit-testable — see [Tests](#tests). - **Repositories are interfaces.** Each one has a single Firebase-backed implementation @@ -186,14 +193,6 @@ They are covered by their own test suite; see [Rules tests](#rules-tests). Honest list of things a reviewer would spot, and why they are the way they are. -**Note files are base64-encoded into Firestore, capping uploads at ~650 KB.** -The right answer is Firebase Storage, which stores binary data as-is with no such -limit. Storage requires the billed Blaze plan, and this project stays on the free -Spark plan, so files are base64-encoded into a Firestore document instead. Since -base64 inflates data by about a third and a Firestore document may not exceed 1 MiB, -the practical ceiling is ~650 KB. Images are downscaled and re-compressed to fit; -larger PDFs are rejected with a clear message. - **The points system is client-authoritative.** Rating a note updates the note's totals *and* the uploader's `points` directly from the client, so the security rules have to permit one user to modify another user's points. @@ -216,9 +215,6 @@ repeat a failed delete by tapping the button again, so the toast does not leave stuck — but the retry is still theirs to figure out. Offering it in the snackbar would mean carrying the note id through `ProfileActionState.DeleteError`. -**The report mechanism is unimplemented.** It is part of the original project brief but -there is no code for it, so the security rules deliberately deny the `reports` collection. - --- ## Getting started @@ -355,7 +351,7 @@ it down again; nothing touches the real project, and no billing account is invol | `notes` | `uploaderUid` cannot be forged on create; metadata edits are the uploader's alone; a rater may touch only `ratingSum`, `ratingCount` and `avgRating`; delete is owner-only. | | `note content` | The batched note-plus-file upload, owner-only replace and delete, and a test that deliberately asserts the *open* create rule, so the gap documented in `firestore.rules` cannot be closed by accident and go unnoticed. | | `ratings` | The `_` document id, which is what stops one user voting as another, plus the 1–5 integer range and the ban on deleting votes. | -| `unmatched paths` | A collection with no rule stays closed, including the `reports` collection mentioned in this README but never implemented. | +| `unmatched paths` | A path no `match` block covers is denied, so adding a collection to the app without adding a rule fails closed rather than open. | Fixtures are seeded with `withSecurityRulesDisabled`, so a broken rule surfaces as a failed assertion rather than a failed setup. Each suite asserts both directions — the diff --git a/firestore-tests/backfill-created-at.js b/firestore-tests/backfill-created-at.js index 40b4381..249afd9 100644 --- a/firestore-tests/backfill-created-at.js +++ b/firestore-tests/backfill-created-at.js @@ -1,7 +1,11 @@ -#!/usr/bin/env node /** * Normalises every note's `createdAt` to a Firestore Timestamp. * + * Deliberately has no `#!` line. The repo normalises to CRLF on Windows checkouts, and + * a shebang ending in `\r` is one esbuild does not strip, so the test that imports this + * file failed to parse — on Windows only, which is why CI stayed green. The script is + * always invoked through `node` or the npm script, so the shebang bought nothing. + * * 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. diff --git a/firestore-tests/rules.test.js b/firestore-tests/rules.test.js index 89d0a46..0639295 100644 --- a/firestore-tests/rules.test.js +++ b/firestore-tests/rules.test.js @@ -367,8 +367,8 @@ describe("ratings", () => { describe("unmatched paths", () => { test("a collection with no rule is closed even to signed-in users", async () => { - // README mentions a `reports` collection that was never implemented. - await assertFails(getDoc(doc(rater, "reports", "anything"))); - await assertFails(setDoc(doc(rater, "reports", "anything"), { note: NOTE })); + // Adding a collection to the app without adding a rule for it must fail closed. + await assertFails(getDoc(doc(rater, "unruled", "anything"))); + await assertFails(setDoc(doc(rater, "unruled", "anything"), { note: NOTE })); }); }); diff --git a/firestore.rules b/firestore.rules index 43149d8..144ec2e 100644 --- a/firestore.rules +++ b/firestore.rules @@ -111,7 +111,7 @@ service cloud.firestore { allow delete: if false; } - // Yukarıda eşleşmeyen her yol reddedilir. README'de geçen `reports` - // koleksiyonu henüz uygulanmadı, o yüzden kasten kural tanımlanmadı. + // Yukarıda eşleşmeyen her yol reddedilir. Yeni bir koleksiyon eklendiğinde + // kuralı da yazılana kadar kapalı kalır — açık kalmasındansa erişilemez olması. } }