Merge pull request #243 from CodeForPhilly/chore/gitsheets-2.9

chore: gitsheets 2.9 — native renames, streamed attachments

Author: themightychris

apps/api/package.json

@@ -36,7 +36,7 @@
     "bcryptjs": "^3.0.3",
     "better-sqlite3": "^12.10.0",
     "fastify": "^5.8.5",
-    "gitsheets": "^2.7.0",
+    "gitsheets": "^2.9.0",
     "jose": "^6.2.3",
     "postmark": "^5.1.0",
     "samlify": "^2.13.0",

apps/api/src/routes/attachments.ts

@@ -11,7 +11,7 @@
  * (gitsheets auto-refresh) and after a hot reload (`store.public.refresh()`).
  *
  * The blob hash is the `ETag`, so a revalidation is answered with 304 without
- * reading the blob at all.
+ * reading the blob at all; a 200 streams the blob with its `Content-Length`.
  */
 import type { FastifyInstance, FastifyRequest } from 'fastify';
 
@@ -120,7 +120,12 @@ export async function attachmentRoutes(fastify: FastifyInstance): Promise<void>
         return reply.code(304).send();
       }
 
-      return reply.type(inferContentType(key)).send(await blob.read());
+      // Stream rather than buffer: gitsheets serves small blobs from one
+      // in-core read and pipes larger ones from git.
+      return reply
+        .type(inferContentType(key))
+        .header('Content-Length', blob.size)
+        .send(blob.stream());
     },
   );
 }

apps/api/src/services/event-series.ts

@@ -17,7 +17,7 @@ import { StateApply } from '../store/state-apply.js';
 import type { DualStoreTx } from '../store/store.js';
 import { EventService, isUpcoming } from './event.js';
 import { MARKDOWN_UPSERT } from './event.write.js';
-import { relinkAttachments, restoreAttachments, takeAttachments } from './slug-rename.js';
+import { relinkAttachments, upsertMoved } from './slug-rename.js';
 import { computeEventSeriesPermissions, type CallerSession, type EventSeriesPermissions } from './permissions.js';
 import { renderMarkdown } from './serializers/common.js';
 import type { EventSummaryResponse, SeriesRef } from './serializers/event.js';
@@ -184,10 +184,7 @@ export class EventSeriesWriteService {
       updatedAt: new Date().toISOString(),
     });
     const stateApply = new StateApply();
-    let attachments: Awaited<ReturnType<typeof takeAttachments>> = null;
     if (renamed) {
-      attachments = await takeAttachments(tx.public['event-series'], existing);
-      await tx.public['event-series'].delete(existing);
       const changedAt = new Date();
       const history = {
         id: uuidv7(),
@@ -201,8 +198,8 @@ export class EventSeriesWriteService {
       await tx.public['slug-history'].upsert(history);
       stateApply.renameEventSeriesSlug(existing.slug).upsertSlugHistory(history);
     }
-    await tx.public['event-series'].upsert(updated, MARKDOWN_UPSERT);
-    await restoreAttachments(tx.public['event-series'], updated, attachments);
+    // Moves the record and its attachments when the slug changed.
+    await upsertMoved(tx.public['event-series'], existing, updated, MARKDOWN_UPSERT);
     stateApply.upsertEventSeries(updated);
     return { series: updated, stateApply };
   }

apps/api/src/services/event.write.ts

@@ -36,7 +36,7 @@ import { StateApply } from '../store/state-apply.js';
 import type { DualStoreTx } from '../store/store.js';
 import { isRsvpOpen } from './permissions.js';
 import { cascadeRename } from './reference-cascade.js';
-import { relinkAttachments, restoreAttachments, takeAttachments } from './slug-rename.js';
+import { relinkAttachments, upsertMoved } from './slug-rename.js';
 
 function nowIso(): string {
   return new Date().toISOString();
@@ -245,15 +245,12 @@ export class EventWriteService {
     });
 
     const stateApply = new StateApply();
-    let attachments: Awaited<ReturnType<typeof takeAttachments>> = null;
 
     if (pathChanged) {
-      // Delete the old file and move every RSVP file with it in this same
+      // Move the event file (below) and every RSVP file with it in this same
       // commit (specs/api/events.md#patch-apieventsslug,
       // specs/behaviors/storage.md#sheet-layout). Slug history only for a
       // rename — the partition is invisible in URLs.
-      attachments = await takeAttachments(tx.public.events, existing);
-      await tx.public.events.delete(existing);
       if (slugChanged) {
         const changedAt = nowIso();
         const history = {
@@ -279,8 +276,8 @@ export class EventWriteService {
       });
     }
 
-    await tx.public.events.upsert(updated, MARKDOWN_UPSERT);
-    await restoreAttachments(tx.public.events, updated, attachments);
+    // Moves the record and its attachments when its path changed.
+    await upsertMoved(tx.public.events, existing, updated, MARKDOWN_UPSERT);
     stateApply.upsertEvent(updated);
     return { event: updated, previous: existing, changes, stateApply };
   }

apps/api/src/services/person.write.ts

@@ -35,7 +35,7 @@ import type { SessionContext } from '../auth/middleware.js';
 import { applyTagsForEntity, type TagAssignmentInput, type TagNamespace } from './tag.write.js';
 import type { PrivateStore } from '../store/private/index.js';
 import { cascadeRename } from './reference-cascade.js';
-import { rekeyAttachment, restoreAttachments, takeAttachments } from './slug-rename.js';
+import { rekeyAttachment, upsertMoved } from './slug-rename.js';
 
 function nowIso(): string {
   return new Date().toISOString();
@@ -117,10 +117,7 @@ export class PersonWriteService {
 
     const stateApply = new StateApply();
 
-    let attachments: Awaited<ReturnType<typeof takeAttachments>> = null;
     if (newSlug !== existing.slug) {
-      attachments = await takeAttachments(tx.public.people, existing);
-      await tx.public.people.delete(existing);
       const history = {
         id: uuidv7(),
         entityType: 'person' as const,
@@ -135,8 +132,8 @@ export class PersonWriteService {
       stateApply.upsertSlugHistory(history);
     }
 
-    await tx.public.people.upsert(updated);
-    await restoreAttachments(tx.public.people, updated, attachments);
+    // Moves the record and its attachments when the slug changed.
+    await upsertMoved(tx.public.people, existing, updated);
     stateApply.upsertPerson(updated);
 
     if (newSlug !== existing.slug) {

apps/api/src/services/project-buzz.write.ts

@@ -13,6 +13,7 @@ import { ApiNotFoundError, ApiValidationError, ConflictError } from '../lib/erro
 import { ensureUniqueSlug, isValidBuzzSlug, slugify } from '../lib/slug.js';
 import { requireAuth } from '../auth/require.js';
 import type { SessionContext } from '../auth/middleware.js';
+import { rekeyAttachment, upsertMoved } from './slug-rename.js';
 
 function nowIso(): string {
   return new Date().toISOString();
@@ -166,20 +167,19 @@ export class ProjectBuzzWriteService {
       summary: input.summary === undefined ? (existing.summary ?? null) : input.summary,
       imageKey:
         input.imageUpload === undefined
-          ? (existing.imageKey ?? null)
+          ? (rekeyAttachment(existing.imageKey, `project-buzz/${project.slug}`, existing.slug, newSlug) ??
+            null)
           : (input.imageUpload?.key ?? null),
       slug: newSlug,
       updatedAt: nowIso(),
     });
 
     const stateApply = new StateApply();
 
-    if (slugChanged) {
-      await tx.public['project-buzz'].delete(existing);
-      stateApply.removeProjectBuzz(existing);
-    }
+    if (slugChanged) stateApply.removeProjectBuzz(existing);
 
-    await tx.public['project-buzz'].upsert(updated);
+    // Moves the record and its image when the slug changed.
+    await upsertMoved(tx.public['project-buzz'], existing, updated);
     stateApply.upsertProjectBuzz(updated);
 
     return { buzz: updated, stateApply };

apps/api/src/services/project.write.ts

@@ -27,7 +27,7 @@ import { requireAuth } from '../auth/require.js';
 import type { SessionContext } from '../auth/middleware.js';
 import { applyTagsForEntity, type TagAssignmentInput } from './tag.write.js';
 import { cascadeRename } from './reference-cascade.js';
-import { rekeyAttachment, restoreAttachments, takeAttachments } from './slug-rename.js';
+import { rekeyAttachment, upsertMoved } from './slug-rename.js';
 
 export interface CreateProjectInput {
   readonly title: string;
@@ -279,12 +279,9 @@ export class ProjectWriteService {
 
     const stateApply = new StateApply();
 
-    let attachments: Awaited<ReturnType<typeof takeAttachments>> = null;
     if (slugRename) {
       // Delete the old path first, then upsert at the new path, carrying
       // the record's attachments across (delete removes them).
-      attachments = await takeAttachments(tx.public.projects, existing);
-      await tx.public.projects.delete(existing);
       // SlugHistory record
       const history = {
         id: uuidv7(),
@@ -300,8 +297,8 @@ export class ProjectWriteService {
       stateApply.upsertSlugHistory(history);
     }
 
-    await tx.public.projects.upsert(updated);
-    await restoreAttachments(tx.public.projects, updated, attachments);
+    // Moves the record and its attachments when the slug changed.
+    await upsertMoved(tx.public.projects, existing, updated);
     stateApply.upsertProject(updated);
 
     if (slugRename) {

apps/api/src/services/reference-cascade.ts

@@ -27,7 +27,7 @@ import type { InMemoryState } from '../store/memory/state.js';
 import { PUBLIC_SHEET_SCHEMAS } from '../store/public.js';
 import type { StateApply } from '../store/state-apply.js';
 import type { DualStoreTx } from '../store/store.js';
-import { restoreAttachments, takeAttachments } from './slug-rename.js';
+import { upsertMoved } from './slug-rename.js';
 
 /** How the cascade reads and re-indexes one referencing sheet's records. */
 interface ReferencingSheet<T> {
@@ -173,10 +173,8 @@ export async function cascadeRename(
       if (oldPath === newPath && Object.keys(also).every((k) => record[k] === moved[k])) continue;
       const updated = schema.parse(rekeyMovedRecord(moved, ref.sheet, oldPath, newPath));
 
-      const attachments = await takeAttachments(txSheet, record);
-      await txSheet.delete(record);
-      await txSheet.upsert(updated);
-      await restoreAttachments(txSheet, updated, attachments);
+      // Moves the record and its attachments to the new path.
+      await upsertMoved(txSheet, record, updated);
 
       adapter.remove(stateApply, record);
       adapter.upsert(stateApply, updated);

apps/api/src/services/slug-rename.ts

@@ -1,38 +1,35 @@
 /**
- * Keep a record's attachments across a slug rename.
+ * Rename helpers — specs/behaviors/slug-handles.md: a rename changes the
+ * handle and nothing else about the entity, so the record's attachments
+ * move with it and any key or link pointing under its old path is
+ * rewritten.
  *
- * A rename deletes the record at its old path and upserts it at the new one.
- * gitsheets' delete also removes the record's attachment folder, so without
- * this the attachments (avatars, featured images) are lost and the on-record
- * key points at a path that no longer exists. specs/behaviors/slug-handles.md:
- * a rename changes the handle and nothing else about the entity.
- *
- * Interim until gitsheets can move a record together with its attachments
- * (plans/slug-rename-attachments.md).
+ * The move itself is gitsheets' rename-on-upsert (≥ 2.8): upserting a
+ * record annotated with its *current* path, when its fields now render a
+ * different path, moves the record file and its attachment folder in one
+ * step.
  */
-import type { BlobHandle } from 'gitsheets';
-
-interface AttachmentSheet<T> {
-  getAttachments(record: T | string): Promise<Record<string, BlobHandle> | null>;
-  setAttachments(record: T | string, attachments: Record<string, BlobHandle>): Promise<void>;
-}
+import { RECORD_PATH_KEY } from 'gitsheets';
 
-/** Read the record's attachments before its old path is deleted. */
-export async function takeAttachments<T>(
-  sheet: AttachmentSheet<T>,
-  record: T,
-): Promise<Record<string, BlobHandle> | null> {
-  const attachments = await sheet.getAttachments(record);
-  return attachments && Object.keys(attachments).length > 0 ? attachments : null;
+interface MovableSheet<T, O> {
+  pathForRecord(record: T): Promise<string>;
+  upsert(record: T, opts?: O): Promise<unknown>;
 }
 
-/** Re-attach to the record at its new path (blobs are reused by hash). */
-export async function restoreAttachments<T>(
-  sheet: AttachmentSheet<T>,
-  record: T,
-  attachments: Record<string, BlobHandle> | null,
+/**
+ * Write `updated` in place of `existing`, moving the record (and its
+ * attachments) when its path changed. The old path is rendered from
+ * `existing` rather than read from its path annotation, which a record
+ * re-parsed through Zod no longer carries.
+ */
+export async function upsertMoved<T extends object, O = never>(
+  sheet: MovableSheet<T, O>,
+  existing: T,
+  updated: T,
+  opts?: O,
 ): Promise<void> {
-  if (attachments) await sheet.setAttachments(record, attachments);
+  const oldPath = await sheet.pathForRecord(existing);
+  await sheet.upsert(Object.assign({ ...updated }, { [RECORD_PATH_KEY]: oldPath }) as T, opts);
 }
 
 /**

apps/api/tests/attachments.test.ts

@@ -186,4 +186,17 @@ describe('GET /api/attachments/*', () => {
     const res = await app.inject({ method: 'GET', url: '/api/attachments/not-a-sheet/x/y.png' });
     expect(res.statusCode).toBe(404);
   });
+
+  it('streams a blob larger than 1 MiB intact, with Content-Length', async () => {
+    const big = Buffer.alloc(3 * 1024 * 1024 + 17);
+    for (let i = 0; i < big.length; i++) big[i] = (i * 31 + 7) & 0xff;
+    await seedRawBlob(dataRepo.path, 'people/big/scan.pdf', big, 'seed a large attachment');
+    app = await bootApp();
+
+    const res = await app.inject({ method: 'GET', url: '/api/attachments/people/big/scan.pdf' });
+    expect(res.statusCode).toBe(200);
+    expect(res.headers['content-length']).toBe(String(big.length));
+    expect(res.rawPayload.equals(big)).toBe(true);
+  });
 });
+