From 38af28d9dcd28f7b53dca0e3ada6a0142f784d9e Mon Sep 17 00:00:00 2001 From: kojira Date: Fri, 4 Sep 2026 23:39:16 +0900 Subject: [PATCH 1/3] =?UTF-8?q?=E3=82=A4=E3=83=99=E3=83=B3=E3=83=88?= =?UTF-8?q?=E5=89=8A=E9=99=A4=E3=81=A7=E5=86=99=E7=9C=9F=E3=81=A8=E5=8B=95?= =?UTF-8?q?=E7=94=BB=E3=81=AE=E5=AE=9F=E4=BD=93=E3=82=82=E6=B6=88=E3=81=99?= =?UTF-8?q?=20(#424)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit イベントを削除しても D1 の行しか消しておらず、表紙画像・イベント写真・ 動画（本体＋ポスター）・景品画像の R2 オブジェクトが全部孤児になっていた。 後始末の契約を lib/mediaCleanup.ts の1本にまとめ、削除する経路をすべて 「D1 からキーを集める → D1 を消す → R2 をベストエフォートで消す」に揃えた。 キーの組み立て（event-images / event-photos / event-videos＋poster）も ここへ集約している。 失敗方向は孤児に倒す。R2 を先に消すと「参照はあるのに実体が無い」行が 残りうるが、これは行から復元できず、運営の対処の証跡 (#278) も失われる。 実体だけが残る側なら配信はされず、後から prefix を舐めて拾える。 掃除用の列挙は表示用の SELECT を通さない（運営が非表示にした写真を落とすと、 その実体だけが残るため）。収集は D1 削除より前でなければならず、 残る R2 側も1回の multi-delete なので deferBackground には逃がさない。 スキーマ変更は無し（マイグレーション不要）。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SLkuubF8cHzSnnsv4fN9DA --- .../src/db/repositories/eventMeetPrizes.ts | 11 + .../server/src/db/repositories/eventPhotos.ts | 19 ++ apps/server/src/lib/mediaCleanup.ts | 93 ++++++ apps/server/src/lib/purgeDeleted.ts | 32 +- apps/server/src/routes/adminModeration.ts | 2 +- apps/server/src/routes/eventCrud.ts | 11 +- apps/server/src/routes/eventMeetPrizes.ts | 35 +-- apps/server/src/routes/eventPhotos.ts | 42 ++- apps/server/src/routes/images.ts | 18 +- apps/server/test/event-media-cleanup.test.ts | 289 ++++++++++++++++++ docs/design.md | 27 ++ 11 files changed, 495 insertions(+), 84 deletions(-) create mode 100644 apps/server/src/lib/mediaCleanup.ts create mode 100644 apps/server/test/event-media-cleanup.test.ts diff --git a/apps/server/src/db/repositories/eventMeetPrizes.ts b/apps/server/src/db/repositories/eventMeetPrizes.ts index 89b3c3de..e54da4e6 100644 --- a/apps/server/src/db/repositories/eventMeetPrizes.ts +++ b/apps/server/src/db/repositories/eventMeetPrizes.ts @@ -111,6 +111,17 @@ export const eventMeetPrizesRepo = { await run("DELETE FROM event_prize WHERE id = ?", id); }, + /** イベント削除時のR2掃除用: そのイベントの景品画像の R2 キー一覧 (#424)。 + * キーは乱数入りで D1 にしか無いので、行が消えた後は誰も辿れない。 + * event 行を消すと FK CASCADE で一緒に消えるため、**削除の前に**呼ぶこと */ + async listImageKeysByEvent(eventId: string): Promise { + const rows = await many<{ image_key: string }>( + "SELECT image_key FROM event_prize WHERE event_id = ? AND image_key IS NOT NULL", + eventId, + ); + return rows.map((r) => r.image_key); + }, + /** 景品画像の R2 キーを差し替える（null で画像なしに戻す） (#434) */ async setImageKey(id: string, imageKey: string | null): Promise { await run("UPDATE event_prize SET image_key = ? WHERE id = ?", imageKey, id); diff --git a/apps/server/src/db/repositories/eventPhotos.ts b/apps/server/src/db/repositories/eventPhotos.ts index 97c73129..d2021247 100644 --- a/apps/server/src/db/repositories/eventPhotos.ts +++ b/apps/server/src/db/repositories/eventPhotos.ts @@ -219,6 +219,25 @@ export const eventPhotosRepo = { })); }, + /** イベント削除時のR2掃除用: そのイベントの写真・動画の (id, eventId, kind) 一覧 (#424)。 + * listIdsByUser と同じ理由で **共有の SELECT を使わない**。あれは運営が + * 非表示にした写真 (#278) を落とすので、通すと非表示ぶんの R2 実体だけが + * 残る（イベントが消えて誰からも辿れない孤児になる）。 + * event 行を消すと FK CASCADE で一緒に消えるため、**削除の前に**呼ぶこと */ + async listIdsByEvent( + eventId: string, + ): Promise> { + const rows = await many<{ id: string; event_id: string; kind: string }>( + "SELECT id, event_id, kind FROM event_photo WHERE event_id = ?", + eventId, + ); + return rows.map((r) => ({ + id: r.id, + eventId: r.event_id, + kind: toKind(r.kind), + })); + }, + /** 公開プロフィール用: ユーザーが公開設定イベントに投稿した写真（ページング #407）。 * 公開範囲は PUBLIC_USER_PHOTO_COND、フィルタは buildUserPhotoWhere 参照。 * コメント数は一覧と同じ COMMENT_COUNT を使う。ここだけ別に書いていたため diff --git a/apps/server/src/lib/mediaCleanup.ts b/apps/server/src/lib/mediaCleanup.ts new file mode 100644 index 00000000..2a6e2c7d --- /dev/null +++ b/apps/server/src/lib/mediaCleanup.ts @@ -0,0 +1,93 @@ +import { getBucket } from "../runtime.js"; +import { eventPhotosRepo } from "../db/repositories/eventPhotos.js"; +import { eventMeetPrizesRepo } from "../db/repositories/eventMeetPrizes.js"; + +/** + * D1 の行と R2 の実体の後始末を1本の契約にまとめる (#424)。 + * + * ■ 契約（削除する経路はすべてこの順で書く） + * 1. **D1 からキーを集める**（行が消えた後はキーを辿れない＝孤児になる） + * 2. D1 の行を消す + * 3. R2 の実体を `deleteObjects` でベストエフォートに消す + * + * ■ なぜこの失敗方向か + * どちらを先にしても失敗はしうるので、「どちらに壊れるか」を選ぶ話になる。 + * - R2 が先に成功して D1 が失敗 → **参照はあるのに実体が無い**。 + * 一覧に出るのに開けない写真が残り、モデレーションの証跡 (#278) も + * 消えている。行から辿って復元する方法が無い＝回復不能。 + * - D1 が先に成功して R2 が失敗 → **実体だけが残る（孤児）**。 + * 誰からも参照されないので配信はされず、prefix を舐める掃除で後から拾える。 + * 回復可能な方（孤児）に倒す。`purgeDeleted.ts` が退会 (#244) で既に採っている + * 順序で、イベント削除・単体削除もこれに揃えた。 + * + * ■ なぜ deferBackground に逃がさないか + * 収集は D1 削除より前でなければならない＝どのみちインラインになる。残る R2 側は + * まとめて1回の multi-delete（R2 は1回 1000 キー、イベント写真は + * EVENT_PHOTO_LIMIT=50 本＝最大 100 キー＋景品＋表紙1枚）なので、 + * サブリクエスト予算 50 に対して余裕がある。インラインなら失敗が + * テストとレスポンスから見える。 + */ + +/** R2 のキー。**イベントが持つ prefix はこの4つだけ**（bgm / deck-images / + * live-set-images / avatars / profile-cards / venue-* はユーザーか会場の持ち物）。 + * 組み立てをここに集約しているので、掃除する側が形を書き写さずに済む */ +export const eventImageR2Key = (eventId: string) => `event-images/${eventId}`; +export const photoR2Key = (eventId: string, photoId: string) => + `event-photos/${eventId}/${photoId}`; +export const videoR2Key = (eventId: string, videoId: string) => + `event-videos/${eventId}/${videoId}`; +/** ポスター（サムネイル画像）は本体の兄弟キーに置く (#408) */ +export const videoPosterR2Key = (eventId: string, videoId: string) => + `${videoR2Key(eventId, videoId)}-poster`; + +/** 1件の投稿が持つ R2 オブジェクトのキー。動画 (#408) は本体＋ポスターの2つ。 + * ポスターなしで投稿された動画でも存在しないキーの削除は無害なので分岐しない + * （分岐を増やすと「ポスターだけ残る」取りこぼしが生まれる）。 + * **写真・動画のキーを組み立てる経路は必ずここを通すこと** */ +export function photoObjectKeys(p: { + eventId: string; + id: string; + kind: "photo" | "video"; +}): string[] { + return p.kind === "video" + ? [videoR2Key(p.eventId, p.id), videoPosterR2Key(p.eventId, p.id)] + : [photoR2Key(p.eventId, p.id)]; +} + +/** イベントが持つ R2 オブジェクトのキーを D1 から列挙する (#424)。 + * **event 行を消す前に呼ぶこと**（子テーブルは FK CASCADE で一緒に消えるため、 + * 後から呼んでもキーは1つも返らない）。 + * + * 列挙元を `bucket.list` ではなく D1 にしているのは、何がこのイベントの持ち物かを + * 知っているのは D1 だから。景品画像 (#434) のキーは乱数を含み D1 にしか無い。 + * 既にある孤児（この修正より前に消したイベントの残骸）はここでは拾わない + * ＝ prefix を舐める掃除は別件。 + * + * 表紙画像は行の有無を見ずに積む。存在しないキーの削除は無害で、 + * `event_image` を引く1サブリクエストを節約できる（退会時の `avatarKey` と同じ手） */ +export async function collectEventObjects(eventId: string): Promise { + const keys = [eventImageR2Key(eventId)]; + for (const p of await eventPhotosRepo.listIdsByEvent(eventId)) { + keys.push(...photoObjectKeys(p)); + } + keys.push(...(await eventMeetPrizesRepo.listImageKeysByEvent(eventId))); + return keys; +} + +/** R2 の削除（ベストエフォート）。失敗しても throw しない＝呼び出し側の + * 削除そのものは成立させる。残骸はログの label で追える。 + * 空配列ならサブリクエストを使わずに戻る。R2 の multi-delete は1回 1000 キーまで */ +export async function deleteObjects( + keys: string[], + label: string, +): Promise { + if (keys.length === 0) return; + try { + const bucket = getBucket(); + for (let i = 0; i < keys.length; i += 1000) { + await bucket.delete(keys.slice(i, i + 1000)); + } + } catch (e) { + console.error(`${label} R2 cleanup failed (${keys.length} keys)`, e); + } +} diff --git a/apps/server/src/lib/purgeDeleted.ts b/apps/server/src/lib/purgeDeleted.ts index 635c195a..80421293 100644 --- a/apps/server/src/lib/purgeDeleted.ts +++ b/apps/server/src/lib/purgeDeleted.ts @@ -6,11 +6,7 @@ import { decksRepo } from "../db/repositories/decks.js"; import { liveSetsRepo } from "../db/repositories/liveSets.js"; import { bgmTracksRepo } from "../db/repositories/bgmTracks.js"; import { eventPhotosRepo } from "../db/repositories/eventPhotos.js"; -import { - photoR2Key, - videoPosterR2Key, - videoR2Key, -} from "../routes/eventPhotos.js"; +import { deleteObjects, photoObjectKeys } from "./mediaCleanup.js"; import { avatarKey } from "./avatarStore.js"; /** 退会猶予期間 (#250) を過ぎたアカウントの完全削除。 @@ -71,15 +67,8 @@ async function collectUserObjects( const keys = await bgmTracksRepo.listKeysByOwner(userId); const photos = await eventPhotosRepo.listIdsByUser(userId); budget.spent += 4; - // 動画 (#408) は本体＋ポスターの2キー。ポスターなし投稿でも - // 存在しないキーの削除は無害なので分岐しない - for (const p of photos) { - if (p.kind === "video") { - keys.push(videoR2Key(p.eventId, p.id), videoPosterR2Key(p.eventId, p.id)); - } else { - keys.push(photoR2Key(p.eventId, p.id)); - } - } + // 動画 (#408) は本体＋ポスターの2キー。組み立ては mediaCleanup の1か所 (#424) + for (const p of photos) keys.push(...photoObjectKeys(p)); // 自前保管のアイコン (#312) は 1ユーザー1キー固定なので list は要らない。 // 保管していなければ存在しないキーを消すだけ（削除は下でまとめて投げるので費用ゼロ） keys.push(avatarKey(userId)); @@ -158,16 +147,11 @@ export async function purgeDeletedAccounts( requestedAt: user.deletedAt, }, }); - // R2 の掃除はベストエフォート（失敗しても削除自体は成立。残骸はログで追える） - try { - const bucket = getBucket(); - for (let i = 0; i < objectKeys.length; i += 1000) { - budget.spent += 1; - await bucket.delete(objectKeys.slice(i, i + 1000)); - } - } catch (e) { - console.error(`[account-purge] R2 cleanup failed for user=${userId}`, e); - } + // R2 の掃除はベストエフォート（失敗しても削除自体は成立。残骸はログで追える）。 + // deleteObjects は 1000 キーごとに1回 delete を呼び、空配列なら + // サブリクエストを使わない。同じ数え方で予算に積む (#424) + budget.spent += Math.ceil(objectKeys.length / 1000); + await deleteObjects(objectKeys, `[account-purge] user=${userId}`); purged += 1; } catch (e) { // DB 側で失敗した場合は deleted_at が残るため、翌日の実行で再試行される diff --git a/apps/server/src/routes/adminModeration.ts b/apps/server/src/routes/adminModeration.ts index 4a3a28c2..45f30f0f 100644 --- a/apps/server/src/routes/adminModeration.ts +++ b/apps/server/src/routes/adminModeration.ts @@ -19,7 +19,7 @@ import { eventChatRepo } from "../db/repositories/eventChat.js"; import { eventQaRepo } from "../db/repositories/eventQa.js"; import { getChatRelays } from "../db/repositories/appSettings.js"; import { recordAudit } from "../db/repositories/auditLogs.js"; -import { photoR2Key, videoPosterR2Key } from "./eventPhotos.js"; +import { photoR2Key, videoPosterR2Key } from "../lib/mediaCleanup.js"; /** 運営によるイベント内コンテンツの非表示 (#278)。app admin のみ。 * diff --git a/apps/server/src/routes/eventCrud.ts b/apps/server/src/routes/eventCrud.ts index 8c0f4614..974e1c09 100644 --- a/apps/server/src/routes/eventCrud.ts +++ b/apps/server/src/routes/eventCrud.ts @@ -15,6 +15,7 @@ import { eventMembersRepo } from "../db/repositories/eventMembers.js"; import { scoringCriteriaRepo } from "../db/repositories/scoringCriteria.js"; import { communitiesRepo } from "../db/repositories/communities.js"; import { deleteEventImage, putEventImage } from "./images.js"; +import { collectEventObjects, deleteObjects } from "../lib/mediaCleanup.js"; import { notifyRequestsOnPublish } from "./eventRequests.js"; import { notifyFollowersOnPublish } from "./follows.js"; import { checkRegistrationDeadline } from "../lib/registrationDeadline.js"; @@ -144,8 +145,14 @@ eventCrudRoutes.post("/:id/publish", requireEventRole(["staff"]), async (c) => { return c.json({ event }); }); -/** イベント削除（staff のみ。関連データは FK CASCADE で削除） */ +/** イベント削除（staff のみ）。D1 の関連データは FK CASCADE で消えるが、 + * **R2 の実体は CASCADE では消えない** (#424)。表紙画像・写真・動画（本体＋ポスター）・ + * 景品画像のキーを D1 から集めてから行を消し、最後に R2 をベストエフォートで消す。 + * 順序と失敗方向の理由は lib/mediaCleanup.ts */ eventCrudRoutes.delete("/:id", requireEventRole(["staff"]), async (c) => { - await eventsRepo.delete(c.req.param("id")); + const eventId = c.req.param("id"); + const keys = await collectEventObjects(eventId); + await eventsRepo.delete(eventId); + await deleteObjects(keys, `[event-delete] event=${eventId}`); return c.json({ ok: true }); }); diff --git a/apps/server/src/routes/eventMeetPrizes.ts b/apps/server/src/routes/eventMeetPrizes.ts index 96f92b83..62cf1b19 100644 --- a/apps/server/src/routes/eventMeetPrizes.ts +++ b/apps/server/src/routes/eventMeetPrizes.ts @@ -23,6 +23,7 @@ import type { AppEnv } from "../types.js"; import { currentUser } from "../auth/session.js"; import { canManageEvent, canViewEvent, requireEventRole } from "../auth/roles.js"; import { getBucket } from "../runtime.js"; +import { deleteObjects } from "../lib/mediaCleanup.js"; import { hasImageMagicBytes, normalizeImageMime, safeServeMime } from "../lib/imageMime.js"; import { valid, zValidator } from "../lib/validator.js"; import { eventsRepo } from "../db/repositories/events.js"; @@ -253,13 +254,10 @@ meetPrizeRoutes.delete( await eventMeetPrizesRepo.delete(prize.id); // 行が消えた画像は誰にも辿れない孤児になるので、ここで R2 も消す (#434)。 // best-effort（失敗してもログで追える。参照は既に無いので配信はされない） - if (prize.imageKey) { - try { - await getBucket().delete(prize.imageKey); - } catch (e) { - console.error("[meet-prize] image cleanup failed", prize.imageKey, e); - } - } + await deleteObjects( + prize.imageKey ? [prize.imageKey] : [], + `[meet-prize] prize=${prize.id}`, + ); return c.json({ ok: true }); }, ); @@ -304,20 +302,13 @@ meetPrizeRoutes.put( await eventMeetPrizesRepo.setImageKey(prize.id, newKey); } catch (e) { // 参照の差し替えに失敗したら、置いたばかりの新キーを消して投げ直す - try { - await bucket.delete(newKey); - } catch (cleanupError) { - console.error("[meet-prize] image cleanup failed", newKey, cleanupError); - } + await deleteObjects([newKey], `[meet-prize] new prize=${prize.id}`); throw e; } - if (prize.imageKey) { - try { - await bucket.delete(prize.imageKey); - } catch (e) { - console.error("[meet-prize] old image cleanup failed", prize.imageKey, e); - } - } + await deleteObjects( + prize.imageKey ? [prize.imageKey] : [], + `[meet-prize] old prize=${prize.id}`, + ); return c.json({ prize: await eventMeetPrizesRepo.findById(prize.id) }); }, ); @@ -330,11 +321,7 @@ meetPrizeRoutes.delete( const prize = await prizeOf(c); if (!prize || !prize.imageKey) return c.json({ error: "not_found" }, 404); await eventMeetPrizesRepo.setImageKey(prize.id, null); - try { - await getBucket().delete(prize.imageKey); - } catch (e) { - console.error("[meet-prize] image cleanup failed", prize.imageKey, e); - } + await deleteObjects([prize.imageKey], `[meet-prize] prize=${prize.id}`); return c.json({ ok: true }); }, ); diff --git a/apps/server/src/routes/eventPhotos.ts b/apps/server/src/routes/eventPhotos.ts index 7e2bd40a..884ee872 100644 --- a/apps/server/src/routes/eventPhotos.ts +++ b/apps/server/src/routes/eventPhotos.ts @@ -25,17 +25,18 @@ import { eventsRepo } from "../db/repositories/events.js"; import { eventPhotosRepo } from "../db/repositories/eventPhotos.js"; import { eventPhotoCommentsRepo } from "../db/repositories/eventPhotoComments.js"; import { eventMembersRepo } from "../db/repositories/eventMembers.js"; +// R2 のキーと掃除の契約は lib/mediaCleanup.ts に集約している (#424)。 +// 管理画面 (#278)・退会時の purge (#244)・イベント削除も同じ実体を扱うので、 +// キーの組み立てを写し取らない +import { + deleteObjects, + photoObjectKeys, + photoR2Key, + videoPosterR2Key, + videoR2Key, +} from "../lib/mediaCleanup.js"; const MEMBER_ROLES = ["participant", "staff", "judge", "observer"] as const; -/** R2 のキー。管理画面 (#278) と退会時の purge (#244) も同じ実体を - * 扱うので、キーの組み立てはここに集約する */ -export const photoR2Key = (eventId: string, photoId: string) => - `event-photos/${eventId}/${photoId}`; -export const videoR2Key = (eventId: string, videoId: string) => - `event-videos/${eventId}/${videoId}`; -/** ポスター（サムネイル画像）は本体の兄弟キーに置く (#408) */ -export const videoPosterR2Key = (eventId: string, videoId: string) => - `${videoR2Key(eventId, videoId)}-poster`; /** 写真を閲覧できるか。photos_public 公開イベントは誰でも、 * それ以外はメンバー/管理者のみ */ @@ -399,14 +400,10 @@ eventPhotoRoutes.post( // ポスター put か D1 insert に失敗したら R2 を掃除する（best-effort。 // 残骸はログで追える）。行が無い動画は削除 API にも purge にも乗らないため、 // ここで消し損ねると誰にも辿れない孤児になる。put 前に消しても無害 - try { - await bucket.delete([ - videoR2Key(eventId, videoId), - videoPosterR2Key(eventId, videoId), - ]); - } catch (cleanupError) { - console.error("[event-video] R2 cleanup failed", videoId, cleanupError); - } + await deleteObjects( + photoObjectKeys({ eventId, id: videoId, kind: "video" }), + `[event-video] video=${videoId}`, + ); throw e; } return c.json({ photo: await eventPhotosRepo.findById(videoId) }, 201); @@ -437,14 +434,11 @@ eventPhotoRoutes.delete( // 非表示を落とす findById で判定すると 404 になり、投稿者には // 「なぜか消せない」としか見えない） if (photo.adminHidden) return c.json({ error: "content_hidden" }, 409); - // 動画は本体＋ポスターの2オブジェクト (#408)。ポスターなし投稿でも - // 存在しないキーの削除は無害 - await getBucket().delete( - photo.kind === "video" - ? [videoR2Key(eventId, photo.id), videoPosterR2Key(eventId, photo.id)] - : [photoR2Key(eventId, photo.id)], - ); + // **行を消してから実体を消す** (#424)。逆順だと R2 の削除に成功して D1 が + // 失敗したとき、一覧に出るのに開けない写真が残る（回復不能）。 + // 順序と失敗方向の理由は lib/mediaCleanup.ts await eventPhotosRepo.delete(photo.id); + await deleteObjects(photoObjectKeys(photo), `[event-photo] photo=${photo.id}`); return c.json({ ok: true }); }, ); diff --git a/apps/server/src/routes/images.ts b/apps/server/src/routes/images.ts index 6f71751c..4f86ed0c 100644 --- a/apps/server/src/routes/images.ts +++ b/apps/server/src/routes/images.ts @@ -3,12 +3,10 @@ import { EVENT_IMAGE } from "@eventer/shared"; import type { AppEnv } from "../types.js"; import { getBucket } from "../runtime.js"; import { normalizeImageMime, safeServeMime } from "../lib/imageMime.js"; +import { deleteObjects, eventImageR2Key } from "../lib/mediaCleanup.js"; import { eventsRepo } from "../db/repositories/events.js"; import { eventImagesRepo } from "../db/repositories/eventImages.js"; -/** R2 のオブジェクトキー（イベントごとに1枚） */ -const imageKey = (eventId: string) => `event-images/${eventId}`; - /** 公開: イベント画像の取得（認証不要。OGクローラ/表示用。本体は R2、メタは D1） */ export async function getEventImage(c: Context) { const eventId = c.req.param("id")!; @@ -19,7 +17,7 @@ export async function getEventImage(c: Context) { if (c.req.header("if-none-match") === etag) { return new Response(null, { status: 304 }); } - const obj = await getBucket().get(imageKey(eventId)); + const obj = await getBucket().get(eventImageR2Key(eventId)); if (!obj) return c.json({ error: "not_found" }, 404); return new Response(obj.body as unknown as ReadableStream, { headers: { @@ -51,7 +49,7 @@ export async function putEventImage(c: Context) { if (body.byteLength > EVENT_IMAGE.maxBytes) { return c.json({ error: "too_large", maxBytes: EVENT_IMAGE.maxBytes }, 413); } - await getBucket().put(imageKey(eventId), body, { + await getBucket().put(eventImageR2Key(eventId), body, { httpMetadata: { contentType: mime }, }); const updatedAt = await eventImagesRepo.upsert(eventId, mime); @@ -65,20 +63,22 @@ export async function copyEventImage( ): Promise { const meta = await eventImagesRepo.getMeta(srcEventId); if (!meta) return; - const obj = await getBucket().get(imageKey(srcEventId)); + const obj = await getBucket().get(eventImageR2Key(srcEventId)); if (!obj) return; // イベント画像は 1MB 以内なのでメモリに載せてコピーする const body = await obj.arrayBuffer(); - await getBucket().put(imageKey(dstEventId), body, { + await getBucket().put(eventImageR2Key(dstEventId), body, { httpMetadata: { contentType: meta.mime }, }); await eventImagesRepo.upsert(dstEventId, meta.mime); } -/** staff/admin: イベント画像の削除 */ +/** staff/admin: イベント画像の削除。**参照を外してから実体を消す** (#424)。 + * 逆順だと R2 だけ消えて行が残り、一覧に出るのに開けない画像になる + * （順序と失敗方向の理由は lib/mediaCleanup.ts） */ export async function deleteEventImage(c: Context) { const eventId = c.req.param("id")!; - await getBucket().delete(imageKey(eventId)); await eventImagesRepo.delete(eventId); + await deleteObjects([eventImageR2Key(eventId)], `[event-image] event=${eventId}`); return c.json({ ok: true }); } diff --git a/apps/server/test/event-media-cleanup.test.ts b/apps/server/test/event-media-cleanup.test.ts new file mode 100644 index 00000000..cb7e0cdf --- /dev/null +++ b/apps/server/test/event-media-cleanup.test.ts @@ -0,0 +1,289 @@ +import { SELF, env } from "cloudflare:test"; +import { describe, it, expect, afterEach, vi } from "vitest"; + +const BASE = "https://example.com"; + +/** + * イベント削除と R2 の実体の後始末 (#424)。固定したい契約: + * + * - イベントを消したら、そのイベントが持つ R2 オブジェクト（表紙画像・写真・ + * 動画の本体＋ポスター・景品画像）も消える。D1 は FK CASCADE で消えるが + * R2 は誰も消さないため、放っておくと全部が孤児になる + * - 掃除の対象は **D1 から** 列挙する。運営が非表示にした写真 (#278) も含める + * （表示用の SELECT を通すと、非表示ぶんの実体だけが残る） + * - 失敗方向は「孤児」であって「参照先が無い行」ではない。順序は + * キー収集 → D1 削除 → R2 削除（lib/mediaCleanup.ts の契約）。 + * R2 が落ちても削除自体は成立させる + */ + +/** dev-login（DevUser=staff/管理者）してセッションcookieを返す */ +async function loginDev(): Promise { + const res = await SELF.fetch(`${BASE}/api/auth/dev-login`, { method: "POST" }); + expect(res.status).toBe(200); + return res.headers.get("set-cookie")!.split(";")[0]; +} + +/** 公開イベントを作る（作成者は staff メンバー） */ +async function setupEvent(cookie: string): Promise { + const create = await SELF.fetch(`${BASE}/api/events`, { + method: "POST", + headers: { "content-type": "application/json", cookie }, + body: JSON.stringify({ + title: "掃除E2E", + venueType: "offline", + startsAt: 1, + endsAt: 99999999999999, + }), + }); + expect(create.status).toBe(201); + return ((await create.json()) as { event: { id: string } }).event.id; +} + +/** 1x1 の PNG（MIME 許可リストとマジックバイト検査を通る最小の実体） */ +const PNG = Uint8Array.from( + atob( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC", + ), + (c) => c.charCodeAt(0), +); + +/** WebM の先頭（EBML ヘッダ）を持つダミーバイト列 */ +function webmBytes(size = 64): Uint8Array { + const b = new Uint8Array(size); + for (let i = 0; i < size; i++) b[i] = i % 256; + b.set([0x1a, 0x45, 0xdf, 0xa3]); + return b; +} + +async function putCoverImage(eventId: string, cookie: string): Promise { + const res = await SELF.fetch(`${BASE}/api/events/${eventId}/image`, { + method: "PUT", + headers: { cookie, "content-type": "image/png" }, + body: PNG, + }); + expect(res.status).toBe(200); +} + +async function uploadPhoto(eventId: string, cookie: string): Promise { + const res = await SELF.fetch(`${BASE}/api/events/${eventId}/photos`, { + method: "POST", + headers: { cookie, "content-type": "image/png" }, + body: PNG, + }); + expect(res.status).toBe(201); + return ((await res.json()) as { photo: { id: string } }).photo.id; +} + +async function uploadVideo(eventId: string, cookie: string): Promise { + const form = new FormData(); + form.set("video", new File([webmBytes()], "v.webm", { type: "video/webm" })); + form.set("poster", new File([PNG], "p.png", { type: "image/png" })); + form.set("durationMs", "1000"); + const res = await SELF.fetch(`${BASE}/api/events/${eventId}/videos`, { + method: "POST", + headers: { cookie }, + body: form, + }); + expect(res.status).toBe(201); + return ((await res.json()) as { photo: { id: string } }).photo.id; +} + +/** 景品と画像を1件仕込む。景品作成 API は「出会い」機能を有効にした + * イベントでしか通らないが、ここで確かめたいのは掃除なので行と実体を直接置く */ +async function seedPrizeImage(eventId: string): Promise { + const prizeId = crypto.randomUUID(); + const key = `prize-images/${prizeId}/${crypto.randomUUID()}`; + await env.DB.prepare( + `INSERT INTO event_prize + (id, event_id, name, description, condition_type, threshold, stock, image_key, created_at) + VALUES (?, ?, '景品', '', 'meet_count', 1, 1, ?, ?)`, + ) + .bind(prizeId, eventId, key, Date.now()) + .run(); + await env.BUCKET.put(key, "prize-bytes"); + return key; +} + +/** 運営が非表示にした写真 (#278) にする */ +async function hidePhoto(photoId: string): Promise { + await env.DB.prepare( + "UPDATE event_photo SET admin_hidden_at = ? WHERE id = ?", + ) + .bind(Date.now(), photoId) + .run(); +} + +async function deleteEvent(eventId: string, cookie: string): Promise { + return SELF.fetch(`${BASE}/api/events/${eventId}`, { + method: "DELETE", + headers: { cookie }, + }); +} + +async function rowCount(sql: string, ...args: unknown[]): Promise { + const row = await env.DB.prepare(sql) + .bind(...args) + .first<{ n: number }>(); + return row?.n ?? 0; +} + +const coverKey = (eventId: string) => `event-images/${eventId}`; +const photoKey = (eventId: string, id: string) => + `event-photos/${eventId}/${id}`; +const videoKey = (eventId: string, id: string) => + `event-videos/${eventId}/${id}`; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("イベント削除で R2 の実体も消す (#424)", () => { + it("表紙画像・写真・動画（本体＋ポスター）・景品画像がすべて消える", async () => { + const cookie = await loginDev(); + const eventId = await setupEvent(cookie); + await putCoverImage(eventId, cookie); + const photoId = await uploadPhoto(eventId, cookie); + const videoId = await uploadVideo(eventId, cookie); + const prizeKey = await seedPrizeImage(eventId); + + // 前提: 実体が揃っている + expect(await env.BUCKET.head(coverKey(eventId))).not.toBeNull(); + expect(await env.BUCKET.head(photoKey(eventId, photoId))).not.toBeNull(); + expect(await env.BUCKET.head(videoKey(eventId, videoId))).not.toBeNull(); + expect( + await env.BUCKET.head(`${videoKey(eventId, videoId)}-poster`), + ).not.toBeNull(); + expect(await env.BUCKET.head(prizeKey)).not.toBeNull(); + + const res = await deleteEvent(eventId, cookie); + expect(res.status).toBe(200); + + expect(await env.BUCKET.head(coverKey(eventId))).toBeNull(); + expect(await env.BUCKET.head(photoKey(eventId, photoId))).toBeNull(); + expect(await env.BUCKET.head(videoKey(eventId, videoId))).toBeNull(); + expect( + await env.BUCKET.head(`${videoKey(eventId, videoId)}-poster`), + ).toBeNull(); + expect(await env.BUCKET.head(prizeKey)).toBeNull(); + // D1 側は FK CASCADE + expect(await rowCount("SELECT COUNT(1) AS n FROM event WHERE id = ?", eventId)).toBe(0); + expect( + await rowCount("SELECT COUNT(1) AS n FROM event_photo WHERE event_id = ?", eventId), + ).toBe(0); + }); + + it("運営が非表示にした写真・動画の実体も消える（表示用の絞り込みを通さない）", async () => { + const cookie = await loginDev(); + const eventId = await setupEvent(cookie); + const photoId = await uploadPhoto(eventId, cookie); + const videoId = await uploadVideo(eventId, cookie); + await hidePhoto(photoId); + await hidePhoto(videoId); + + expect((await deleteEvent(eventId, cookie)).status).toBe(200); + + expect(await env.BUCKET.head(photoKey(eventId, photoId))).toBeNull(); + expect(await env.BUCKET.head(videoKey(eventId, videoId))).toBeNull(); + expect( + await env.BUCKET.head(`${videoKey(eventId, videoId)}-poster`), + ).toBeNull(); + }); + + it("R2 の削除が落ちても削除は成立する（残るのは孤児であって、参照先の無い行ではない）", async () => { + const cookie = await loginDev(); + const eventId = await setupEvent(cookie); + await putCoverImage(eventId, cookie); + const photoId = await uploadPhoto(eventId, cookie); + + const spy = vi + .spyOn(env.BUCKET, "delete") + .mockRejectedValue(new Error("R2 down")); + + const res = await deleteEvent(eventId, cookie); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + // 掃除を試みた上で握り潰していること（そもそも呼んでいないなら + // 「落ちても成立する」は何も確かめていない） + expect(spy).toHaveBeenCalled(); + // D1 の行は消えている（消せなかったことにして行を残すと、実体の無い + // 参照が一覧に出続ける。孤児のほうが後から掃除できる） + expect(await rowCount("SELECT COUNT(1) AS n FROM event WHERE id = ?", eventId)).toBe(0); + expect( + await rowCount("SELECT COUNT(1) AS n FROM event_photo WHERE event_id = ?", eventId), + ).toBe(0); + // 実体は残る（孤児） + expect(await env.BUCKET.head(coverKey(eventId))).not.toBeNull(); + expect(await env.BUCKET.head(photoKey(eventId, photoId))).not.toBeNull(); + }); + + it("R2 を消しに行く時点で D1 の行は既に消えている（順序そのものを固定する）", async () => { + const cookie = await loginDev(); + const eventId = await setupEvent(cookie); + await uploadPhoto(eventId, cookie); + + // R2 の削除が呼ばれた瞬間に event 行がまだ在るかを記録する。 + // R2 が先だと「実体は消えたのに行が残っている」状態が一瞬でも生まれ、 + // そこで D1 が落ちれば参照先の無い行が確定する + const eventRowsAtDelete: number[] = []; + const original = env.BUCKET.delete.bind(env.BUCKET); + vi.spyOn(env.BUCKET, "delete").mockImplementation(async (keys) => { + eventRowsAtDelete.push( + await rowCount("SELECT COUNT(1) AS n FROM event WHERE id = ?", eventId), + ); + return original(keys as string | string[]); + }); + + expect((await deleteEvent(eventId, cookie)).status).toBe(200); + expect(eventRowsAtDelete).toEqual([0]); + }); +}); + +describe("写真1枚の削除でも実体を残さない (#424)", () => { + it("動画は本体とポスターの両方が消え、R2 を消す時点で行は既に消えている", async () => { + const cookie = await loginDev(); + const eventId = await setupEvent(cookie); + const videoId = await uploadVideo(eventId, cookie); + + // イベント削除と同じ順序（行 → 実体）であることを、削除の瞬間の行数で見る + const rowsAtDelete: number[] = []; + const original = env.BUCKET.delete.bind(env.BUCKET); + vi.spyOn(env.BUCKET, "delete").mockImplementation(async (keys) => { + rowsAtDelete.push( + await rowCount("SELECT COUNT(1) AS n FROM event_photo WHERE id = ?", videoId), + ); + return original(keys as string | string[]); + }); + + const res = await SELF.fetch( + `${BASE}/api/events/${eventId}/photos/${videoId}`, + { method: "DELETE", headers: { cookie } }, + ); + expect(res.status).toBe(200); + expect(rowsAtDelete).toEqual([0]); + expect(await env.BUCKET.head(videoKey(eventId, videoId))).toBeNull(); + expect( + await env.BUCKET.head(`${videoKey(eventId, videoId)}-poster`), + ).toBeNull(); + expect( + await rowCount("SELECT COUNT(1) AS n FROM event_photo WHERE id = ?", videoId), + ).toBe(0); + }); + + it("R2 の削除が落ちても行は消える（参照先の無い行を作らない）", async () => { + const cookie = await loginDev(); + const eventId = await setupEvent(cookie); + const videoId = await uploadVideo(eventId, cookie); + + vi.spyOn(env.BUCKET, "delete").mockRejectedValue(new Error("R2 down")); + + const res = await SELF.fetch( + `${BASE}/api/events/${eventId}/photos/${videoId}`, + { method: "DELETE", headers: { cookie } }, + ); + expect(res.status).toBe(200); + expect( + await rowCount("SELECT COUNT(1) AS n FROM event_photo WHERE id = ?", videoId), + ).toBe(0); + expect(await env.BUCKET.head(videoKey(eventId, videoId))).not.toBeNull(); + }); +}); diff --git a/docs/design.md b/docs/design.md index d0093495..c610d89b 100644 --- a/docs/design.md +++ b/docs/design.md @@ -115,6 +115,33 @@ Cloudflare Workers の `ExecutionContext.waitUntil` に載り、レスポンス 暴走防止という目的は満たす。実行文脈と違って**取りこぼしは起きない**ので、 リクエストごとに持ち直すところまではやっていない。 +### メディアの実体（R2）の後始末 (#424) + +D1 の行は FK の `ON DELETE CASCADE` で消えるが、**R2 のオブジェクトを消すものは誰もいない**。 +削除する経路がそれぞれ好きな順序で書くと必ずどこかが漏れるので、契約を1つに定める +（実装は `apps/server/src/lib/mediaCleanup.ts`。キーの組み立てもここに集約する）。 + +- **順序は「D1 からキーを集める → D1 を消す → R2 をベストエフォートで消す」。** + 行が消えた後はキーを辿れないので、収集は削除より前でなければならない。 +- **失敗方向は「孤児」に倒す。** どちらを先にしても失敗はしうるので、 + どう壊れるかを選ぶ話になる。R2 が先に成功して D1 が失敗すると + **参照はあるのに実体が無い**（一覧に出るのに開けない・運営の対処の証跡 #278 も + 消えている・行から復元できない＝回復不能）。D1 が先なら残るのは + **誰からも参照されない実体**で、配信はされず prefix を舐める掃除で後から拾える。 +- **列挙は `bucket.list` ではなく D1 から。** 何がそのイベントの持ち物かを知っているのは D1 で、 + 景品画像 (#434) のキーは乱数入りで D1 にしか無い。 + 掃除用の SELECT は**表示用の絞り込みを通さない**（運営が非表示にした写真を落とすと、 + その実体だけが残る）。 +- **`deferBackground` に逃がさない。** 収集はどのみちインラインになり、残る R2 側は + 1回の multi-delete（R2 は1回 1000 キー、イベント写真は `EVENT_PHOTO_LIMIT` 本まで）。 + サブリクエスト予算 50 に対して余裕があり、インラインなら失敗が + テストとレスポンスから見える。 +- **イベントが持つ prefix は4つだけ**: `event-images/{eventId}` / + `event-photos/{eventId}/{photoId}` / `event-videos/{eventId}/{videoId}`（＋ `-poster` #408） / + `event_prize.image_key` に入る `prize-images/{prizeId}/{uuid}`。 + `bgm/` `deck-images/` `live-set-images/` `avatars/` `profile-cards/` `venue-*` は + ユーザーか会場の持ち物で、退会時の掃除 (#244) 側が見る。 + ### リアルタイム方針の補足 - **SSE** はサーバー→クライアントの一方向通知に使う：モード切替、プレゼン対象の変更、採点提出状況の更新、表彰の段階発表。 From 2807e38b177736810889641b978b8f631924afe0 Mon Sep 17 00:00:00 2001 From: kojira Date: Sat, 5 Sep 2026 00:01:31 +0900 Subject: [PATCH 2/3] =?UTF-8?q?=E3=83=AC=E3=83=93=E3=83=A5=E3=83=BC?= =?UTF-8?q?=E5=AF=BE=E5=BF=9C:=20=E9=80=80=E4=BC=9A=E3=81=A7=E6=B6=88?= =?UTF-8?q?=E3=81=88=E3=82=8B=E4=B8=8B=E6=9B=B8=E3=81=8D=E3=82=A4=E3=83=99?= =?UTF-8?q?=E3=83=B3=E3=83=88=E3=81=AE=E5=AE=9F=E4=BD=93=E3=82=82=E6=8E=83?= =?UTF-8?q?=E9=99=A4=E3=81=99=E3=82=8B=20(#424)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 退会の完全削除は「参加者のいない下書きイベント」の行ごと消すのに、 その R2 の実体（表紙画像・写真・動画・景品画像）を誰も集めていなかった。 #424 が塞いだはずの孤児が退会の経路からそのまま出ていた。 どのイベントが消えるかの条件（WHERE 句）を accountDeletion.ts に1つだけ置き、 DELETE と、削除前にキーを集めるための SELECT の両方から引く。条件を書き写すと 片方だけずれて実体が残るため、同じ文字列を共有する。created_by に渡す値だけが 両者で違う（DELETE は ghost へ付け替えた後、収集は付け替え前なので本人）。 増える D1 の呼び出しはサブリクエスト予算に積み、MIN_COST_PER_USER も上げた。 1000キー刻みの分割も2か所に書かれていた。deleteObjects が消費した サブリクエスト数を返すようにして、呼び出し側の数え直しをやめる。 失敗しても消費済みなので、途中で落ちても同じ数を返す（予算は必ず積まれる）。 docs/design.md の事実誤りを訂正した。退会時の掃除は venue-images/ と venue-photos/ を見ておらず、community-icons/ community-banners/ は そもそも誰も消していない。契約が揃っているのはイベントが持つメディアだけで、 ユーザー・会場の経路はまだ R2 を先に消している（別件 #TBD）。 テストで固定した契約: - 退会で消える下書きイベントの表紙画像・景品画像が消えること。 同時に、残る側（公開済み・第三者が参加している下書き）の実体は消さないこと - 掃除に渡すキー配列に画像なし景品の NULL を混ぜないこと。 本番の R2 は null キーで multi-delete ごと落ちるが、テスト環境の R2 は 素通しするため「実体が消えたか」では捕まらない。渡したキー自体を見る - 表紙画像の単体削除は R2 が落ちても ok を返し、D1 の行は消えること （#424 で 500 からベストエフォートに変えた振る舞いを意図として固定する） スキーマ変更は無し（マイグレーション不要）。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SLkuubF8cHzSnnsv4fN9DA --- .../src/db/repositories/accountDeletion.ts | 40 +++++++++-- apps/server/src/lib/mediaCleanup.ts | 21 ++++-- apps/server/src/lib/purgeDeleted.ts | 45 +++++++++--- apps/server/test/account-deletion.test.ts | 68 +++++++++++++++++++ apps/server/test/event-media-cleanup.test.ts | 66 ++++++++++++++++++ docs/design.md | 19 +++++- 6 files changed, 235 insertions(+), 24 deletions(-) diff --git a/apps/server/src/db/repositories/accountDeletion.ts b/apps/server/src/db/repositories/accountDeletion.ts index ba42aa12..6294a772 100644 --- a/apps/server/src/db/repositories/accountDeletion.ts +++ b/apps/server/src/db/repositories/accountDeletion.ts @@ -15,6 +15,21 @@ import { * identity の provider_user_id からしかユーザーを解決しないため対象にならない */ const DELETED_USER_DISCORD_ID = "system:deleted-user"; +/** 「参加者のいない下書きイベント」の条件 (#244)。誰にも見えず誰も消せない + * 孤児になるため退会の完全削除で消す。 + * + * **この条件を2か所に書かない** (#424)。R2 の実体（表紙画像・写真・景品画像）は + * 行が消えると辿れなくなるので削除の**前に**キーを集める必要があり、集める側は + * 「どのイベントが消えるか」を知らなければならない。条件を書き写すと片方だけ + * ずれて実体が孤児になるので、DELETE と収集用の SELECT で同じ文字列を使う。 + * + * `?` は順に created_by と「本人以外の参加者」の user_id。created_by に何を + * 渡すかだけが両者で違う: DELETE は (1) で ghost に付け替えた**後**なので + * ghost、収集は付け替え**前**に走るので本人。 */ +const ORPHAN_DRAFT_EVENT_WHERE = `created_by = ? AND status = 'draft' + AND NOT EXISTS (SELECT 1 FROM event_member m + WHERE m.event_id = event.id AND m.user_id != ?)`; + /** 退会の一連（申請 → 猶予期間 → 完全削除）と、その手前で使う * 「利用実績があるか」の判定 (#238)。触る表の一覧は userTables.ts と共有する * （統合 accountMerge.ts と同じ定義を読む） */ @@ -93,6 +108,22 @@ export const accountDeletionRepo = { return row?.n ?? 0; }, + /** `deleteAccount` が消す下書きイベントの id (#424)。R2 の実体を消すために + * **deleteAccount を呼ぶ前に**呼ぶこと（行が消えるとキーを辿れない）。 + * 条件は DELETE と同じ ORPHAN_DRAFT_EVENT_WHERE を引く。まだ ghost へ + * 付け替える前なので created_by は本人。 + * + * 付け替え済みの ghost 名義の下書き（過去の退会の取りこぼし）はここには出ない。 + * それは prefix を舐める掃除の話で、この経路の担当ではない */ + async listDeletableDraftEventIds(userId: string): Promise { + const rows = await many<{ id: string }>( + `SELECT id FROM event WHERE ${ORPHAN_DRAFT_EVENT_WHERE}`, + userId, + userId, + ); + return rows.map((r) => r.id); + }, + /** 退会（アカウント削除） (#244)。単一トランザクション（D1 batch）で * 「共有コンテンツを『退会済みユーザー』(ghost) に付け替え → 個人データ削除 → * user 行削除（FK CASCADE で残りが消える）」を行う。 @@ -154,12 +185,11 @@ export const accountDeletionRepo = { args: [userId], }); - // (1-d) 参加者のいない下書きイベントは誰にも見えず誰も消せない孤児になるため削除 + // (1-d) 参加者のいない下書きイベントは誰にも見えず誰も消せない孤児になるため削除。 + // 条件は ORPHAN_DRAFT_EVENT_WHERE（R2 のキー収集と共有）。ここでは (1) で + // 付け替えた後なので created_by は ghost stmts.push({ - sql: `DELETE FROM event - WHERE created_by = ? AND status = 'draft' - AND NOT EXISTS (SELECT 1 FROM event_member m - WHERE m.event_id = event.id AND m.user_id != ?)`, + sql: `DELETE FROM event WHERE ${ORPHAN_DRAFT_EVENT_WHERE}`, args: [ghostId, userId], }); diff --git a/apps/server/src/lib/mediaCleanup.ts b/apps/server/src/lib/mediaCleanup.ts index 2a6e2c7d..ba32abd8 100644 --- a/apps/server/src/lib/mediaCleanup.ts +++ b/apps/server/src/lib/mediaCleanup.ts @@ -74,20 +74,31 @@ export async function collectEventObjects(eventId: string): Promise { return keys; } +/** R2 の multi-delete の上限（1回のリクエストで消せるキー数） */ +const MAX_KEYS_PER_DELETE = 1000; + /** R2 の削除（ベストエフォート）。失敗しても throw しない＝呼び出し側の * 削除そのものは成立させる。残骸はログの label で追える。 - * 空配列ならサブリクエストを使わずに戻る。R2 の multi-delete は1回 1000 キーまで */ + * 空配列ならサブリクエストを使わずに戻る。 + * + * @returns 消費したサブリクエスト数。退会の掃除 (#244) はこれを実行予算に積む。 + * 刻み幅を呼び出し側に数え直させると「1000」が2か所に散り、片方を変えたときに + * 予算だけ静かにずれる。分割した本人が数えて返す。 + * 失敗しても既に消費済みなので、途中で落ちても投げる予定だった回数を返す + * （＝予算は必ず積まれる） */ export async function deleteObjects( keys: string[], label: string, -): Promise { - if (keys.length === 0) return; +): Promise { + if (keys.length === 0) return 0; + const calls = Math.ceil(keys.length / MAX_KEYS_PER_DELETE); try { const bucket = getBucket(); - for (let i = 0; i < keys.length; i += 1000) { - await bucket.delete(keys.slice(i, i + 1000)); + for (let i = 0; i < keys.length; i += MAX_KEYS_PER_DELETE) { + await bucket.delete(keys.slice(i, i + MAX_KEYS_PER_DELETE)); } } catch (e) { console.error(`${label} R2 cleanup failed (${keys.length} keys)`, e); } + return calls; } diff --git a/apps/server/src/lib/purgeDeleted.ts b/apps/server/src/lib/purgeDeleted.ts index 80421293..5f4afd0c 100644 --- a/apps/server/src/lib/purgeDeleted.ts +++ b/apps/server/src/lib/purgeDeleted.ts @@ -6,7 +6,11 @@ import { decksRepo } from "../db/repositories/decks.js"; import { liveSetsRepo } from "../db/repositories/liveSets.js"; import { bgmTracksRepo } from "../db/repositories/bgmTracks.js"; import { eventPhotosRepo } from "../db/repositories/eventPhotos.js"; -import { deleteObjects, photoObjectKeys } from "./mediaCleanup.js"; +import { + collectEventObjects, + deleteObjects, + photoObjectKeys, +} from "./mediaCleanup.js"; import { avatarKey } from "./avatarStore.js"; /** 退会猶予期間 (#250) を過ぎたアカウントの完全削除。 @@ -16,7 +20,8 @@ import { avatarKey } from "./avatarStore.js"; * ■ サブリクエスト予算について * Workers Free のサブリクエスト上限は 1リクエストあたり 50 で、D1 / R2 への * 呼び出しもここに含まれる。1件あたりの消費数は - * 9（固定）＋ デッキ数 ＋ 配信セット数 ＋ ceil(R2キー数 / 1000) + * 10（固定）＋ デッキ数 ＋ 配信セット数 ＋ 消える下書きイベント数×2 + * ＋ ceil(R2キー数 / 1000) * とユーザーの持ちデータ量に比例するため、「1回に N 件」という件数固定では * 上限を守れない（デッキを 40 個持つ人が1人居るだけで超過する）。しかも上限を * 超えると同一リクエスト内の以降のサブリクエストが全部失敗するので、 @@ -32,15 +37,16 @@ const SUBREQUEST_BUDGET = 40; /** 1件あたりの最小消費数（データを何も持たないユーザーの場合）。 * findByIdIncludingDeleted 1 - * ＋ collectUserObjects の D1 4（decks / live_set / bgm / event_photo） + * ＋ collectUserObjects の D1 5（decks / live_set / bgm / event_photo ＋ + * 消える下書きイベントの列挙 #424。イベントがあれば +2/件） * ＋ プロフィールカードの R2 list 1 * ＋ deleteAccount の batch 1 * ＋ deleteAccount 内のスタッフチャット列挙 (#382) 1 * （部屋があれば +1/部屋。実消費は deleteAccount の戻り値で budget に積む） * ＋ recordAudit 2（INSERT と保存期間の掃除） - * ＝ 10。R2 に実体があれば delete でさらに 1 以上増えるので 11 で見積もる。 + * ＝ 11。R2 に実体があれば delete でさらに 1 以上増えるので 12 で見積もる。 * 次の1件がこれ以下の余裕しか無ければ打ち切る */ -const MIN_COST_PER_USER = 11; +const MIN_COST_PER_USER = 12; /** 1回の実行で見に行く候補の最大数。実際には予算のほうが先に効くが、 * listPurgeTargets が無制限に行を読まないための保険 */ @@ -55,8 +61,9 @@ interface Budget { /** 退会するユーザー由来の R2 オブジェクトキーを列挙する (#244)。 * 行削除後はキーを辿れなくなるため、DB 削除前に呼ぶこと。 * 対象: スライド画像・配信セット画像・BGM 音源・イベント写真・プロフィールカードPNG・ - * 自前保管のアイコン (#312)。 - * デッキ数・配信セット数だけ R2 list が増えるので、消費数を budget に積む */ + * 自前保管のアイコン (#312)・**完全削除で消える下書きイベントの持ち物** (#424)。 + * デッキ数・配信セット数だけ R2 list が、下書きイベント数だけ D1 が増えるので、 + * 消費数を budget に積む */ async function collectUserObjects( userId: string, budget: Budget, @@ -72,6 +79,19 @@ async function collectUserObjects( // 自前保管のアイコン (#312) は 1ユーザー1キー固定なので list は要らない。 // 保管していなければ存在しないキーを消すだけ（削除は下でまとめて投げるので費用ゼロ） keys.push(avatarKey(userId)); + // 完全削除では「参加者のいない下書きイベント」の行も消える (#244) ので、 + // そのイベントの持ち物（表紙画像・写真・動画・景品画像）も一緒に消さないと + // #424 が塞いだはずの孤児がここから出る。**どのイベントが消えるかの条件は + // accountDeletion.ts に1つだけ置き**、DELETE と同じ WHERE を引いた SELECT で + // 受け取る（条件を書き写すと片方だけずれて実体が残る） + budget.spent += 1; + const draftEventIds = + await accountDeletionRepo.listDeletableDraftEventIds(userId); + for (const eventId of draftEventIds) { + // collectEventObjects は D1 を2回引く（イベント写真＋景品画像） + budget.spent += 2; + keys.push(...(await collectEventObjects(eventId))); + } const prefixes = [ ...decks.map((d) => `deck-images/${d.id}/`), ...liveSets.map((s) => `live-set-images/${s.id}/`), @@ -148,10 +168,13 @@ export async function purgeDeletedAccounts( }, }); // R2 の掃除はベストエフォート（失敗しても削除自体は成立。残骸はログで追える）。 - // deleteObjects は 1000 キーごとに1回 delete を呼び、空配列なら - // サブリクエストを使わない。同じ数え方で予算に積む (#424) - budget.spent += Math.ceil(objectKeys.length / 1000); - await deleteObjects(objectKeys, `[account-purge] user=${userId}`); + // 消費したサブリクエスト数は deleteObjects が返す（刻み幅をここで数え直すと + // 「1000」が2か所に散り、片方を変えたときに予算だけ静かにずれる #424）。 + // deleteObjects は内部で握り潰す＝throw しないので、失敗しても必ず積まれる + budget.spent += await deleteObjects( + objectKeys, + `[account-purge] user=${userId}`, + ); purged += 1; } catch (e) { // DB 側で失敗した場合は deleted_at が残るため、翌日の実行で再試行される diff --git a/apps/server/test/account-deletion.test.ts b/apps/server/test/account-deletion.test.ts index 12ec6faf..d9e4cf66 100644 --- a/apps/server/test/account-deletion.test.ts +++ b/apps/server/test/account-deletion.test.ts @@ -367,6 +367,74 @@ describe("退会（アカウント削除） (#244, #250)", () => { ).toBe(1); // 本人の行だけ消える }); + it("完全削除で消える下書きイベントの R2 の実体も消える (#424)", async () => { + // 退会の完全削除は「参加者のいない下書きイベント」の行ごと消す。 + // 行が消えると表紙画像・景品画像のキーは D1 から辿れなくなるので、 + // 消す前にキーを集めていないと、まさに #424 が塞いだはずの孤児が + // この経路から出る。逆に集めすぎる（消えないイベントの実体まで消す）と + // 「行はあるのに実体が無い」になるので、残る側も同時に見張る + const a = await makeUser(); + const b = await makeUser(); + + /** 表紙画像と景品画像を持つイベントを1件作る */ + async function eventWithMedia( + status: "draft" | "published", + ): Promise<{ eventId: string; coverKey: string; prizeKey: string }> { + const eventId = crypto.randomUUID(); + await env.DB.prepare( + "INSERT INTO event (id, title, starts_at, ends_at, venue_type, status, created_by, created_at) VALUES (?, '掃除テスト', 1, 2, 'offline', ?, ?, ?)", + ) + .bind(eventId, status, a.userId, Date.now()) + .run(); + await env.DB.prepare( + "INSERT INTO event_image (event_id, mime, updated_at) VALUES (?, 'image/png', ?)", + ) + .bind(eventId, Date.now()) + .run(); + const coverKey = `event-images/${eventId}`; + await env.BUCKET.put(coverKey, "cover-bytes"); + + const prizeId = crypto.randomUUID(); + const prizeKey = `prize-images/${prizeId}/${crypto.randomUUID()}`; + await env.DB.prepare( + `INSERT INTO event_prize + (id, event_id, name, description, condition_type, threshold, stock, image_key, created_at) + VALUES (?, ?, '景品', '', 'meet_count', 1, 1, ?, ?)`, + ) + .bind(prizeId, eventId, prizeKey, Date.now()) + .run(); + await env.BUCKET.put(prizeKey, "prize-bytes"); + return { eventId, coverKey, prizeKey }; + } + + // 消える: 本人が作った、本人以外の参加者が居ない下書き + const orphan = await eventWithMedia("draft"); + // 残る(1): 公開済み＝「退会済みユーザー」名義に付け替えて残る + const published = await eventWithMedia("published"); + // 残る(2): 下書きでも第三者が参加していれば消えない + const shared = await eventWithMedia("draft"); + await joinEvent(shared.eventId, a.userId); + await joinEvent(shared.eventId, b.userId); + + expect((await deleteAndPurge(a.cookie, a.userId)).status).toBe(200); + + // 行が消えたイベントは実体も消えている（孤児を残さない） + expect( + await count("SELECT COUNT(*) AS n FROM event WHERE id = ?", orphan.eventId), + ).toBe(0); + expect(await env.BUCKET.head(orphan.coverKey)).toBeNull(); + expect(await env.BUCKET.head(orphan.prizeKey)).toBeNull(); + + // 残るイベントの実体は消さない（消すと「開けない画像」になる） + for (const e of [published, shared]) { + expect( + await count("SELECT COUNT(*) AS n FROM event WHERE id = ?", e.eventId), + ).toBe(1); + expect(await env.BUCKET.head(e.coverKey)).not.toBeNull(); + expect(await env.BUCKET.head(e.prizeKey)).not.toBeNull(); + } + }); + it("会場・オファーの連絡先は消え、未応答のオファーは辞退になる", async () => { const a = await makeUser(); const owner = await makeUser(); diff --git a/apps/server/test/event-media-cleanup.test.ts b/apps/server/test/event-media-cleanup.test.ts index cb7e0cdf..e90b9117 100644 --- a/apps/server/test/event-media-cleanup.test.ts +++ b/apps/server/test/event-media-cleanup.test.ts @@ -104,6 +104,20 @@ async function seedPrizeImage(eventId: string): Promise { return key; } +/** 画像を持たない景品（`image_key` が NULL）を1件仕込む。 + * 掃除用の SELECT が NULL を弾いていないと、キー配列に null が混ざって + * R2 の multi-delete がまるごと失敗する（`deleteObjects` が握り潰すので + * 静かに全部が孤児になる）。画像ありの景品と並べて置くことでその穴を塞ぐ */ +async function seedPrizeWithoutImage(eventId: string): Promise { + await env.DB.prepare( + `INSERT INTO event_prize + (id, event_id, name, description, condition_type, threshold, stock, image_key, created_at) + VALUES (?, ?, '画像なし景品', '', 'meet_count', 1, 1, NULL, ?)`, + ) + .bind(crypto.randomUUID(), eventId, Date.now()) + .run(); +} + /** 運営が非表示にした写真 (#278) にする */ async function hidePhoto(photoId: string): Promise { await env.DB.prepare( @@ -145,6 +159,12 @@ describe("イベント削除で R2 の実体も消す (#424)", () => { const photoId = await uploadPhoto(eventId, cookie); const videoId = await uploadVideo(eventId, cookie); const prizeKey = await seedPrizeImage(eventId); + // 画像を持たない景品 (#434) を混ぜる。列挙が NULL を弾いていないと + // キー配列に null が入る。本番の R2 は null キーを受け付けず multi-delete が + // まるごと落ちる（`deleteObjects` が握り潰すので静かに全部が孤児になる）が、 + // テスト環境の R2 は素通しするので「実体が消えたか」では捕まらない。 + // R2 に渡ったキーそのものを見る + await seedPrizeWithoutImage(eventId); // 前提: 実体が揃っている expect(await env.BUCKET.head(coverKey(eventId))).not.toBeNull(); @@ -155,9 +175,23 @@ describe("イベント削除で R2 の実体も消す (#424)", () => { ).not.toBeNull(); expect(await env.BUCKET.head(prizeKey)).not.toBeNull(); + const passedKeys: unknown[] = []; + const originalDelete = env.BUCKET.delete.bind(env.BUCKET); + vi.spyOn(env.BUCKET, "delete").mockImplementation(async (keys) => { + passedKeys.push(...(Array.isArray(keys) ? keys : [keys])); + return originalDelete(keys as string | string[]); + }); + const res = await deleteEvent(eventId, cookie); expect(res.status).toBe(200); + // R2 に渡すのは文字列のキーだけ（画像なしの景品ぶんの null を混ぜない） + expect(passedKeys.length).toBeGreaterThan(0); + expect( + passedKeys.filter((k) => typeof k !== "string"), + "R2 に文字列でないキーを渡している", + ).toEqual([]); + expect(await env.BUCKET.head(coverKey(eventId))).toBeNull(); expect(await env.BUCKET.head(photoKey(eventId, photoId))).toBeNull(); expect(await env.BUCKET.head(videoKey(eventId, videoId))).toBeNull(); @@ -238,6 +272,38 @@ describe("イベント削除で R2 の実体も消す (#424)", () => { }); }); +describe("表紙画像の単体削除でも実体を残さない (#424)", () => { + it("R2 の削除が落ちても ok を返し、D1 の行は消える", async () => { + // #424 以前は R2 の失敗がそのまま 500 になり、行だけ消えた状態で + // 呼び出し側にはエラーが返っていた。今は他の削除経路と同じ + // ベストエフォート（握り潰して孤児に倒す）に揃えてある。 + // 意図した契約なので、握り潰していること自体をここで固定する + const cookie = await loginDev(); + const eventId = await setupEvent(cookie); + await putCoverImage(eventId, cookie); + expect( + await rowCount("SELECT COUNT(1) AS n FROM event_image WHERE event_id = ?", eventId), + ).toBe(1); + + const spy = vi + .spyOn(env.BUCKET, "delete") + .mockRejectedValue(new Error("R2 down")); + + const res = await SELF.fetch(`${BASE}/api/events/${eventId}/image`, { + method: "DELETE", + headers: { cookie }, + }); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + expect(spy).toHaveBeenCalled(); + // 参照（D1）は消える。残るのは誰からも参照されない実体だけ + expect( + await rowCount("SELECT COUNT(1) AS n FROM event_image WHERE event_id = ?", eventId), + ).toBe(0); + expect(await env.BUCKET.head(coverKey(eventId))).not.toBeNull(); + }); +}); + describe("写真1枚の削除でも実体を残さない (#424)", () => { it("動画は本体とポスターの両方が消え、R2 を消す時点で行は既に消えている", async () => { const cookie = await loginDev(); diff --git a/docs/design.md b/docs/design.md index c610d89b..bfb64e42 100644 --- a/docs/design.md +++ b/docs/design.md @@ -118,8 +118,12 @@ Cloudflare Workers の `ExecutionContext.waitUntil` に載り、レスポンス ### メディアの実体（R2）の後始末 (#424) D1 の行は FK の `ON DELETE CASCADE` で消えるが、**R2 のオブジェクトを消すものは誰もいない**。 -削除する経路がそれぞれ好きな順序で書くと必ずどこかが漏れるので、契約を1つに定める +削除する経路がそれぞれ好きな順序で書くと必ずどこかが漏れるので、 +**イベントが持つメディア**については契約を1つに定める （実装は `apps/server/src/lib/mediaCleanup.ts`。キーの組み立てもここに集約する）。 +ユーザー・会場が持つメディアを消す経路（`routes/bgm.ts` の曲削除、 +`routes/venues.ts` の会場削除・会場画像・会場写真）は**まだこの順序に揃っておらず、 +R2 を先に消している**。同じ契約へ寄せるのは別件 (#483)。 - **順序は「D1 からキーを集める → D1 を消す → R2 をベストエフォートで消す」。** 行が消えた後はキーを辿れないので、収集は削除より前でなければならない。 @@ -139,8 +143,17 @@ D1 の行は FK の `ON DELETE CASCADE` で消えるが、**R2 のオブジェ - **イベントが持つ prefix は4つだけ**: `event-images/{eventId}` / `event-photos/{eventId}/{photoId}` / `event-videos/{eventId}/{videoId}`（＋ `-poster` #408） / `event_prize.image_key` に入る `prize-images/{prizeId}/{uuid}`。 - `bgm/` `deck-images/` `live-set-images/` `avatars/` `profile-cards/` `venue-*` は - ユーザーか会場の持ち物で、退会時の掃除 (#244) 側が見る。 + それ以外はユーザー・会場・コミュニティの持ち物で、掃除する主体が別々に分かれている: + - **退会時の掃除 (#244) が見る**のは `deck-images/` `live-set-images/` `bgm/` + `avatars/` `profile-cards/` と、退会で消える写真・**参加者のいない下書きイベント**の + 持ち物（上の4つを `collectEventObjects` で辿る #424）。 + - **`venue-images/` `venue-photos/` は退会時の掃除の対象外**。会場は退会者から + 「退会済みユーザー」名義へ付け替えて残るので、実体を消すのは会場側の削除経路 + (`routes/venues.ts`) の担当。 + - **`community-icons/` `community-banners/` は今のところ誰も消していない**。 + コミュニティも退会では名義を付け替えて残るので掃除の対象外だが、 + コミュニティ削除 (`routes/communities.ts`) が D1 の行しか消しておらず、 + 実体が孤児になる。これも上と同じ別件 (#483) で揃える。 ### リアルタイム方針の補足 From 6467d1defef3273fe0b50bf1fb92f69cdcee355b Mon Sep 17 00:00:00 2001 From: kojira Date: Sat, 5 Sep 2026 00:23:38 +0900 Subject: [PATCH 3/3] =?UTF-8?q?=E3=83=AC=E3=83=93=E3=83=A5=E3=83=BC?= =?UTF-8?q?=E5=AF=BE=E5=BF=9C:=20=E5=8F=8E=E9=9B=86=E3=81=A8=E5=89=8A?= =?UTF-8?q?=E9=99=A4=E3=81=AE=E9=9B=86=E5=90=88=E3=82=92=E5=BC=95=E6=95=B0?= =?UTF-8?q?=E3=81=94=E3=81=A8=E6=8F=83=E3=81=88=E3=80=814=E7=B5=8C?= =?UTF-8?q?=E8=B7=AF=E3=81=AE=E9=A0=86=E5=BA=8F=E3=82=92=E5=9B=BA=E5=AE=9A?= =?UTF-8?q?=E3=81=99=E3=82=8B=20(#424)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ■ 収集漏れ（実害あり） 下書きイベントの DELETE は付け替え後の ghost 名義を見ており、しかも 本人のイベントに限定されていない＝条件に合う ghost 名義の下書きを すべて消す。一方で収集側は本人名義しか見ていなかったため、次の3手で 行だけが消えて実体が孤児になった: 1. A の下書き（第三者 B が参加）は A の退会では消えず ghost 名義になる 2. B がイベントから抜ける → 参加者ゼロの ghost 名義の下書きになる 3. 無関係な C が退会すると、その DELETE がこの行を消す 条件の文字列を共有するだけでは足りず、**同じ引数で呼べること**が要る。 所有者の条件を `created_by IN (本人, ghost)` にして、DELETE と収集用の SELECT を同一の SQL・同一の引数にした。付け替えは status も event_member も 触らないので 「付け替え後に created_by = ghost」⇔「付け替え前に created_by ∈ {本人, ghost}」 が成り立ち、2つの集合は似ているのではなく同一になる。 「文字列を共有していれば歩調が合う」と書いていた docblock は誤りだったので、 何が集合を等しくしているのかに書き直した。 ■ 順序の固定が4経路中2つしか無かった イベント削除・写真単体削除にしか「R2 を消す時点で D1 の行は消えている」の テストが無く、表紙画像の単体削除と退会の掃除は順序を逆にしても全部通った。 同じ手法（R2 削除をスタブして、その瞬間の行数を記録する）で4経路すべてを固定。 ■ deleteObjects の分割契約が無検査だった MAX_KEYS_PER_DELETE を 1000 から 5000 にしても通る。テスト環境の R2 は 何個でも受け取るが本番は 1000 超を拒否し、例外は握り潰されるので全部が 静かに孤児になる。`return calls` を `return 0` にしても通り、これは MIN_COST_PER_USER が守っているサブリクエスト予算を過少に見積もらせる。 R2 に渡した配列の長さと返り値の回数を直接見るテストを追加した。 ミューテーション7件（4経路の順序逆転 / 1000→5000 / return 0 / 収集を本人の下書きだけに戻す）がいずれも赤になることを確認済み。 スキーマ変更は無し（マイグレーション不要）。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SLkuubF8cHzSnnsv4fN9DA --- .../src/db/repositories/accountDeletion.ts | 48 +++-- apps/server/src/lib/purgeDeleted.ts | 23 ++- apps/server/test/account-deletion.test.ts | 167 ++++++++++++++---- apps/server/test/event-media-cleanup.test.ts | 70 ++++++++ 4 files changed, 249 insertions(+), 59 deletions(-) diff --git a/apps/server/src/db/repositories/accountDeletion.ts b/apps/server/src/db/repositories/accountDeletion.ts index 6294a772..bf72d0f5 100644 --- a/apps/server/src/db/repositories/accountDeletion.ts +++ b/apps/server/src/db/repositories/accountDeletion.ts @@ -18,15 +18,26 @@ const DELETED_USER_DISCORD_ID = "system:deleted-user"; /** 「参加者のいない下書きイベント」の条件 (#244)。誰にも見えず誰も消せない * 孤児になるため退会の完全削除で消す。 * - * **この条件を2か所に書かない** (#424)。R2 の実体（表紙画像・写真・景品画像）は - * 行が消えると辿れなくなるので削除の**前に**キーを集める必要があり、集める側は - * 「どのイベントが消えるか」を知らなければならない。条件を書き写すと片方だけ - * ずれて実体が孤児になるので、DELETE と収集用の SELECT で同じ文字列を使う。 + * この条件に対する要求は **DELETE と、その前に R2 のキーを集める SELECT が + * 同じ集合を指すこと** (#424)。行が消えるとキーを辿れないので収集は削除より + * 前に走るしかなく、集合がずれた分だけ実体が孤児になる。 * - * `?` は順に created_by と「本人以外の参加者」の user_id。created_by に何を - * 渡すかだけが両者で違う: DELETE は (1) で ghost に付け替えた**後**なので - * ghost、収集は付け替え**前**に走るので本人。 */ -const ORPHAN_DRAFT_EVENT_WHERE = `created_by = ? AND status = 'draft' + * **同じ文字列を共有するだけでは足りない。同じ引数で呼べることが要る。** + * 素直に書くと所有者の指定が両者で食い違う: + * - DELETE は (1) の付け替えの**後**なので、本人の下書きは既に ghost 名義。 + * - しかも DELETE は本人のイベントに限定されておらず、条件に合う ghost 名義の + * 下書きを**すべて**消す。過去の退会で ghost に移った下書きから後になって + * 第三者の参加者が抜けると、無関係な人の退会でその行が消える。 + * - 収集は付け替えの**前**なので、本人名義のものはまだ本人名義。 + * 所有者の条件を `created_by IN (本人, ghost)` にすると、両者を**同じ引数**で + * 呼べる。付け替えは status も event_member も触らないので + * 「付け替え後に created_by = ghost」⇔「付け替え前に created_by ∈ {本人, ghost}」 + * が成り立ち、2つの集合は似ているのではなく**同一**になる。 + * （付け替え前に created_by = 本人 のものは付け替えで ghost になり、 + * 既に ghost のものはそのまま。それ以外は ghost にならない） + * + * `?` は順に 本人 / ghost / 「本人以外の参加者」の user_id。 */ +const ORPHAN_DRAFT_EVENT_WHERE = `created_by IN (?, ?) AND status = 'draft' AND NOT EXISTS (SELECT 1 FROM event_member m WHERE m.event_id = event.id AND m.user_id != ?)`; @@ -110,15 +121,17 @@ export const accountDeletionRepo = { /** `deleteAccount` が消す下書きイベントの id (#424)。R2 の実体を消すために * **deleteAccount を呼ぶ前に**呼ぶこと（行が消えるとキーを辿れない）。 - * 条件は DELETE と同じ ORPHAN_DRAFT_EVENT_WHERE を引く。まだ ghost へ - * 付け替える前なので created_by は本人。 - * - * 付け替え済みの ghost 名義の下書き（過去の退会の取りこぼし）はここには出ない。 - * それは prefix を舐める掃除の話で、この経路の担当ではない */ - async listDeletableDraftEventIds(userId: string): Promise { + * 条件も引数も DELETE 側と同一（等しくなる理由は + * ORPHAN_DRAFT_EVENT_WHERE）。ghost 名義に移っている下書きも対象に入る＝ + * DELETE が消す行を1つ残らず含む */ + async listDeletableDraftEventIds( + userId: string, + ghostId: string, + ): Promise { const rows = await many<{ id: string }>( `SELECT id FROM event WHERE ${ORPHAN_DRAFT_EVENT_WHERE}`, userId, + ghostId, userId, ); return rows.map((r) => r.id); @@ -186,11 +199,12 @@ export const accountDeletionRepo = { }); // (1-d) 参加者のいない下書きイベントは誰にも見えず誰も消せない孤児になるため削除。 - // 条件は ORPHAN_DRAFT_EVENT_WHERE（R2 のキー収集と共有）。ここでは (1) で - // 付け替えた後なので created_by は ghost + // 条件も引数も R2 のキー収集（listDeletableDraftEventIds）と同一にする。 + // (1) の付け替え後なので本人名義の分は既に ghost 名義だが、 + // `IN (本人, ghost)` なのでどちらでも同じ行に当たる stmts.push({ sql: `DELETE FROM event WHERE ${ORPHAN_DRAFT_EVENT_WHERE}`, - args: [ghostId, userId], + args: [userId, ghostId, userId], }); // (2) FK RESTRICT の個人資産 live_set は user 削除前に明示削除。 diff --git a/apps/server/src/lib/purgeDeleted.ts b/apps/server/src/lib/purgeDeleted.ts index 5f4afd0c..9ea18c65 100644 --- a/apps/server/src/lib/purgeDeleted.ts +++ b/apps/server/src/lib/purgeDeleted.ts @@ -63,9 +63,12 @@ interface Budget { * 対象: スライド画像・配信セット画像・BGM 音源・イベント写真・プロフィールカードPNG・ * 自前保管のアイコン (#312)・**完全削除で消える下書きイベントの持ち物** (#424)。 * デッキ数・配信セット数だけ R2 list が、下書きイベント数だけ D1 が増えるので、 - * 消費数を budget に積む */ + * 消費数を budget に積む。 + * `ghostId` は下書きイベントの列挙に要る（deleteAccount の DELETE が付け替え後の + * ghost 名義を見るため。詳細は accountDeletion.ts） */ async function collectUserObjects( userId: string, + ghostId: string, budget: Budget, ): Promise { const bucket = getBucket(); @@ -81,12 +84,18 @@ async function collectUserObjects( keys.push(avatarKey(userId)); // 完全削除では「参加者のいない下書きイベント」の行も消える (#244) ので、 // そのイベントの持ち物（表紙画像・写真・動画・景品画像）も一緒に消さないと - // #424 が塞いだはずの孤児がここから出る。**どのイベントが消えるかの条件は - // accountDeletion.ts に1つだけ置き**、DELETE と同じ WHERE を引いた SELECT で - // 受け取る（条件を書き写すと片方だけずれて実体が残る） + // #424 が塞いだはずの孤児がここから出る。 + // + // 消える行を**1つ残らず**受け取るのが要件で、条件の文字列を共有するだけでは + // 足りない。DELETE は付け替え後の ghost 名義を見るので、既に ghost 名義に + // なっている下書き（過去の退会で移り、その後に第三者の参加者が抜けたもの）まで + // 消す＝本人の持ち物だけ集めても足りない。条件・引数ごと揃える理由と、 + // 2つの集合が等しくなる根拠は accountDeletion.ts の ORPHAN_DRAFT_EVENT_WHERE budget.spent += 1; - const draftEventIds = - await accountDeletionRepo.listDeletableDraftEventIds(userId); + const draftEventIds = await accountDeletionRepo.listDeletableDraftEventIds( + userId, + ghostId, + ); for (const eventId of draftEventIds) { // collectEventObjects は D1 を2回引く（イベント写真＋景品画像） budget.spent += 2; @@ -149,7 +158,7 @@ export async function purgeDeletedAccounts( const user = await usersRepo.findByIdIncludingDeleted(userId); if (!user || user.deletedAt === null) continue; // 直前に復帰した attemptedAny = true; - const objectKeys = await collectUserObjects(userId, budget); + const objectKeys = await collectUserObjects(userId, ghost.id, budget); console.log( `[account-purge] user=${userId} handle=${user.username} ghost=${ghost.id} r2Objects=${objectKeys.length}`, ); diff --git a/apps/server/test/account-deletion.test.ts b/apps/server/test/account-deletion.test.ts index d9e4cf66..a55ba369 100644 --- a/apps/server/test/account-deletion.test.ts +++ b/apps/server/test/account-deletion.test.ts @@ -1,5 +1,5 @@ import { SELF, env } from "cloudflare:test"; -import { describe, it, expect } from "vitest"; +import { describe, it, expect, afterEach, vi } from "vitest"; const BASE = "https://example.com"; const GHOST_DISCORD_ID = "system:deleted-user"; @@ -91,6 +91,51 @@ async function count(sql: string, ...args: unknown[]): Promise { return row?.n ?? 0; } +/** 表紙画像と景品画像（＝R2 の実体つき）を持つイベントを1件作る (#424) */ +async function eventWithMedia( + ownerId: string, + status: "draft" | "published", +): Promise<{ eventId: string; coverKey: string; prizeKey: string }> { + const eventId = crypto.randomUUID(); + await env.DB.prepare( + "INSERT INTO event (id, title, starts_at, ends_at, venue_type, status, created_by, created_at) VALUES (?, '掃除テスト', 1, 2, 'offline', ?, ?, ?)", + ) + .bind(eventId, status, ownerId, Date.now()) + .run(); + await env.DB.prepare( + "INSERT INTO event_image (event_id, mime, updated_at) VALUES (?, 'image/png', ?)", + ) + .bind(eventId, Date.now()) + .run(); + const coverKey = `event-images/${eventId}`; + await env.BUCKET.put(coverKey, "cover-bytes"); + + const prizeId = crypto.randomUUID(); + const prizeKey = `prize-images/${prizeId}/${crypto.randomUUID()}`; + await env.DB.prepare( + `INSERT INTO event_prize + (id, event_id, name, description, condition_type, threshold, stock, image_key, created_at) + VALUES (?, ?, '景品', '', 'meet_count', 1, 1, ?, ?)`, + ) + .bind(prizeId, eventId, prizeKey, Date.now()) + .run(); + await env.BUCKET.put(prizeKey, "prize-bytes"); + return { eventId, coverKey, prizeKey }; +} + +/** イベントから抜ける（参加行を消す） */ +async function leaveEvent(eventId: string, userId: string): Promise { + await env.DB.prepare( + "DELETE FROM event_member WHERE event_id = ? AND user_id = ?", + ) + .bind(eventId, userId) + .run(); +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + async function ghostRow(): Promise<{ id: string; username: string } | null> { return env.DB.prepare( "SELECT id, username FROM user WHERE discord_id = ?", @@ -376,43 +421,12 @@ describe("退会（アカウント削除） (#244, #250)", () => { const a = await makeUser(); const b = await makeUser(); - /** 表紙画像と景品画像を持つイベントを1件作る */ - async function eventWithMedia( - status: "draft" | "published", - ): Promise<{ eventId: string; coverKey: string; prizeKey: string }> { - const eventId = crypto.randomUUID(); - await env.DB.prepare( - "INSERT INTO event (id, title, starts_at, ends_at, venue_type, status, created_by, created_at) VALUES (?, '掃除テスト', 1, 2, 'offline', ?, ?, ?)", - ) - .bind(eventId, status, a.userId, Date.now()) - .run(); - await env.DB.prepare( - "INSERT INTO event_image (event_id, mime, updated_at) VALUES (?, 'image/png', ?)", - ) - .bind(eventId, Date.now()) - .run(); - const coverKey = `event-images/${eventId}`; - await env.BUCKET.put(coverKey, "cover-bytes"); - - const prizeId = crypto.randomUUID(); - const prizeKey = `prize-images/${prizeId}/${crypto.randomUUID()}`; - await env.DB.prepare( - `INSERT INTO event_prize - (id, event_id, name, description, condition_type, threshold, stock, image_key, created_at) - VALUES (?, ?, '景品', '', 'meet_count', 1, 1, ?, ?)`, - ) - .bind(prizeId, eventId, prizeKey, Date.now()) - .run(); - await env.BUCKET.put(prizeKey, "prize-bytes"); - return { eventId, coverKey, prizeKey }; - } - // 消える: 本人が作った、本人以外の参加者が居ない下書き - const orphan = await eventWithMedia("draft"); + const orphan = await eventWithMedia(a.userId, "draft"); // 残る(1): 公開済み＝「退会済みユーザー」名義に付け替えて残る - const published = await eventWithMedia("published"); + const published = await eventWithMedia(a.userId, "published"); // 残る(2): 下書きでも第三者が参加していれば消えない - const shared = await eventWithMedia("draft"); + const shared = await eventWithMedia(a.userId, "draft"); await joinEvent(shared.eventId, a.userId); await joinEvent(shared.eventId, b.userId); @@ -435,6 +449,89 @@ describe("退会（アカウント削除） (#244, #250)", () => { } }); + it("他人の退会で消える『退会済みユーザー』名義の下書きも実体ごと消える (#424)", async () => { + // 完全削除の DELETE は本人のイベントに限定されておらず、条件に合う + // ghost 名義の下書きを**すべて**消す。そのため次の3手で、無関係な人の + // 退会が他人の残した下書きの行だけを消し、実体を孤児にできてしまう: + // 1. A の下書き（第三者 B が参加）は A の退会では消えず ghost 名義になる + // 2. B がイベントから抜ける → 参加者ゼロの ghost 名義の下書きになる + // 3. 無関係な C が退会すると、その DELETE がこの行を消す + // 収集側が「本人の持ち物」しか見ていないと 3 で実体だけが残る。 + // 収集と削除が同じ集合を指していることを、この経路で固定する + const a = await makeUser(); + const b = await makeUser(); + const c = await makeUser(); + + const carried = await eventWithMedia(a.userId, "draft"); + await joinEvent(carried.eventId, a.userId); + await joinEvent(carried.eventId, b.userId); + // 対照: 参加者が残り続ける ghost 名義の下書き。行が残る＝実体も残る + const stillJoined = await eventWithMedia(a.userId, "draft"); + await joinEvent(stillJoined.eventId, a.userId); + await joinEvent(stillJoined.eventId, b.userId); + + // 1. A の退会。B が参加しているのでどちらの下書きも消えず ghost 名義になる + expect((await deleteAndPurge(a.cookie, a.userId)).status).toBe(200); + const ghost = await ghostRow(); + for (const e of [carried, stillJoined]) { + const row = await env.DB.prepare( + "SELECT created_by FROM event WHERE id = ?", + ) + .bind(e.eventId) + .first<{ created_by: string }>(); + expect(row?.created_by).toBe(ghost!.id); + expect(await env.BUCKET.head(e.coverKey)).not.toBeNull(); + } + + // 2. B が carried から抜ける（参加者ゼロの ghost 名義の下書きになる） + await leaveEvent(carried.eventId, b.userId); + + // 3. 無関係な C が退会する + expect((await deleteAndPurge(c.cookie, c.userId)).status).toBe(200); + + // 行が消えたなら実体も消えていること + expect( + await count("SELECT COUNT(*) AS n FROM event WHERE id = ?", carried.eventId), + ).toBe(0); + expect(await env.BUCKET.head(carried.coverKey)).toBeNull(); + expect(await env.BUCKET.head(carried.prizeKey)).toBeNull(); + + // 参加者が残っている方は行も実体も無傷（集めすぎていない） + expect( + await count( + "SELECT COUNT(*) AS n FROM event WHERE id = ?", + stillJoined.eventId, + ), + ).toBe(1); + expect(await env.BUCKET.head(stillJoined.coverKey)).not.toBeNull(); + expect(await env.BUCKET.head(stillJoined.prizeKey)).not.toBeNull(); + }); + + it("R2 を消しに行く時点で D1 の行は既に消えている (#424)", async () => { + // 削除4経路のうち退会ぶん。順序が逆だと「実体は消えたのに行が残る」瞬間が + // でき、そこで D1 が落ちれば参照先の無い行が確定する + const a = await makeUser(); + const orphan = await eventWithMedia(a.userId, "draft"); + + const rowsAtDelete: Array<{ user: number; event: number }> = []; + const original = env.BUCKET.delete.bind(env.BUCKET); + vi.spyOn(env.BUCKET, "delete").mockImplementation(async (keys) => { + rowsAtDelete.push({ + user: await count("SELECT COUNT(*) AS n FROM user WHERE id = ?", a.userId), + event: await count( + "SELECT COUNT(*) AS n FROM event WHERE id = ?", + orphan.eventId, + ), + }); + return original(keys as string | string[]); + }); + + expect((await deleteAndPurge(a.cookie, a.userId)).status).toBe(200); + // 呼ばれていないなら「順序が正しい」は何も確かめていない + expect(rowsAtDelete.length).toBeGreaterThan(0); + for (const at of rowsAtDelete) expect(at).toEqual({ user: 0, event: 0 }); + }); + it("会場・オファーの連絡先は消え、未応答のオファーは辞退になる", async () => { const a = await makeUser(); const owner = await makeUser(); diff --git a/apps/server/test/event-media-cleanup.test.ts b/apps/server/test/event-media-cleanup.test.ts index e90b9117..9a7dcbd9 100644 --- a/apps/server/test/event-media-cleanup.test.ts +++ b/apps/server/test/event-media-cleanup.test.ts @@ -1,5 +1,7 @@ import { SELF, env } from "cloudflare:test"; import { describe, it, expect, afterEach, vi } from "vitest"; +import { bindEnv, type Env } from "../src/runtime.js"; +import { deleteObjects } from "../src/lib/mediaCleanup.js"; const BASE = "https://example.com"; @@ -273,6 +275,31 @@ describe("イベント削除で R2 の実体も消す (#424)", () => { }); describe("表紙画像の単体削除でも実体を残さない (#424)", () => { + it("R2 を消しに行く時点で D1 の行は既に消えている（順序そのものを固定する）", async () => { + const cookie = await loginDev(); + const eventId = await setupEvent(cookie); + await putCoverImage(eventId, cookie); + + const rowsAtDelete: number[] = []; + const original = env.BUCKET.delete.bind(env.BUCKET); + vi.spyOn(env.BUCKET, "delete").mockImplementation(async (keys) => { + rowsAtDelete.push( + await rowCount( + "SELECT COUNT(1) AS n FROM event_image WHERE event_id = ?", + eventId, + ), + ); + return original(keys as string | string[]); + }); + + const res = await SELF.fetch(`${BASE}/api/events/${eventId}/image`, { + method: "DELETE", + headers: { cookie }, + }); + expect(res.status).toBe(200); + expect(rowsAtDelete).toEqual([0]); + }); + it("R2 の削除が落ちても ok を返し、D1 の行は消える", async () => { // #424 以前は R2 の失敗がそのまま 500 になり、行だけ消えた状態で // 呼び出し側にはエラーが返っていた。今は他の削除経路と同じ @@ -353,3 +380,46 @@ describe("写真1枚の削除でも実体を残さない (#424)", () => { expect(await env.BUCKET.head(videoKey(eventId, videoId))).not.toBeNull(); }); }); + +describe("deleteObjects の分割契約 (#424)", () => { + /** 本番の R2 は multi-delete 1回につき 1000 キーまでで、超えると丸ごと拒否する。 + * テスト環境の R2 は何個でも受け取るので、「実体が消えたか」を見ても + * 上限超えには気づけない（`deleteObjects` は例外を握り潰すため、 + * 本番では全部が静かに孤児になる）。R2 に渡した配列そのものを見る。 + * + * 返り値の回数も同時に固定する。退会の掃除 (#244) はこれをサブリクエスト + * 予算に積んでおり、少なく返すと MIN_COST_PER_USER の見積もりが壊れて + * 上限 50 を超え、同じリクエスト内の以降の呼び出しが全部失敗する */ + it("1回の delete に 1000 キーを超えて渡さず、投げた回数を返す", async () => { + bindEnv(env as unknown as Env); + const keys = Array.from( + { length: 2500 }, + (_, i) => `event-photos/e/${i}`, + ); + const sizes: number[] = []; + vi.spyOn(env.BUCKET, "delete").mockImplementation(async (k) => { + sizes.push(Array.isArray(k) ? k.length : 1); + }); + + const calls = await deleteObjects(keys, "[test]"); + + expect(Math.max(...sizes)).toBeLessThanOrEqual(1000); + expect(sizes).toEqual([1000, 1000, 500]); + // 予算に積む数＝実際に投げた回数 + expect(calls).toBe(3); + expect(calls).toBe(sizes.length); + }); + + it("空配列ならサブリクエストを使わず 0 を返す", async () => { + bindEnv(env as unknown as Env); + const spy = vi.spyOn(env.BUCKET, "delete"); + expect(await deleteObjects([], "[test]")).toBe(0); + expect(spy).not.toHaveBeenCalled(); + }); + + it("失敗しても投げた回数を返す（予算は必ず積まれる）", async () => { + bindEnv(env as unknown as Env); + vi.spyOn(env.BUCKET, "delete").mockRejectedValue(new Error("R2 down")); + expect(await deleteObjects(["event-images/e"], "[test]")).toBe(1); + }); +});