Skip to content

feat: ドメインイベントの監査ログ永続化#106

Merged
KinjiKawaguchi merged 1 commit intodevelopfrom
feat/domain-event-persistence-v2
Mar 24, 2026
Merged

feat: ドメインイベントの監査ログ永続化#106
KinjiKawaguchi merged 1 commit intodevelopfrom
feat/domain-event-persistence-v2

Conversation

@KinjiKawaguchi
Copy link
Copy Markdown
Member

@KinjiKawaguchi KinjiKawaguchi commented Mar 23, 2026

Replaces #101 (clean rebase on latest develop)

Why

ドメインイベントは集約のメソッドで蓄積されるが、永続化されておらずメモリ上で消失していた。
監査ログとして変更履歴を追跡可能にするため、イベントの永続化基盤を導入する。

What

  • member_domain_events / discord_account_domain_events テーブルを追加
  • Member: 共通カラム(member_id, email, event_name, occurred_at)+ 型安全なJSONBペイロード
  • DiscordAccount: 共通カラム(discord_id, member_id, event_name, occurred_at)+ ペイロード
  • リポジトリの save() 内で同一トランザクションにてイベントを書き込み
  • シリアライズ関数を別ファイルに切り出しテスト可能に
  • Affiliationはdiscriminated union(PR refactor: Affiliationの型表現をDiscriminated Unionに変更 #102)のためserialize不要、直接格納
  • ドメインイベントをクラスからplain discriminated unionに変更
  • DomainEvent基底型をintersectionで適用

How

  • Drizzleの .$type<>() でJSONBカラムにコンパイル時型安全性を付与
  • 追記専用(UPDATE/DELETE不要)の監査ログ設計
  • FKなし(エンティティ削除後もイベントを保持するため)

🤖 Generated with Claude Code

@KinjiKawaguchi KinjiKawaguchi marked this pull request as ready for review March 23, 2026 22:27
Copilot AI review requested due to automatic review settings March 23, 2026 22:27
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 potential issues.

Open in Devin Review

This comment was marked as resolved.

Comment on lines +143 to +153
if (events.length > 0) {
await tx.insert(memberDomainEvents).values(
events.map((event) => ({
memberId: event.id as string,
email: event.email.getValue(),
eventName: event.eventName,
payload: serializeMemberEventPayload(event),
occurredAt: event.occurredAt.toISOString(),
})),
);
}

This comment was marked as resolved.

Comment thread src/infrastructure/drizzle/schema.ts Outdated
Comment thread src/infrastructure/drizzle/schema.ts
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

ドメインイベントがメモリ上で消失していた問題に対し、監査ログとしてDBへ追記永続化するための基盤(テーブル/シリアライズ/リポジトリ保存)を追加するPRです。

Changes:

  • member_domain_events / discord_account_domain_events の追加と、JSONB payload 型の定義
  • リポジトリ save() をトランザクション化し、集約保存と同時にドメインイベントを追記
  • ドメインイベントを class から discriminated union object へ移行し、シリアライズ関数+テストを追加

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/infrastructure/drizzle/serializeMemberEvent.test.ts Memberイベントpayloadシリアライズのテスト追加
tests/infrastructure/drizzle/serializeDiscordAccountEvent.test.ts DiscordAccountイベントpayloadシリアライズのテスト追加
src/infrastructure/drizzle/serializeMemberEvent.ts MemberDomainEvent → JSONB payload 変換ロジック追加
src/infrastructure/drizzle/serializeDiscordAccountEvent.ts DiscordAccountDomainEvent → JSONB payload 変換ロジック追加
src/infrastructure/drizzle/schema.ts ドメインイベントpayload型+ドメインイベントテーブル定義を追加
src/infrastructure/drizzle/DrizzleMemberRepository.ts Member保存と同一Txでイベント追記を行うよう変更
src/infrastructure/drizzle/DrizzleDiscordAccountRepository.ts DiscordAccount保存と同一Txでイベント追記を行うよう変更
src/domain/base/DomainEvent.ts DomainEvent を interface → type に変更
src/domain/aggregates/member/MemberEvent.ts Memberイベント定義を class → type(discriminated union) に変更
src/domain/aggregates/member/Member.ts イベント生成を new からオブジェクトリテラルへ移行
src/domain/aggregates/discord-account/DiscordAccount.ts イベント定義/生成を class → type + オブジェクトリテラルへ移行
drizzle/meta/_journal.json Drizzleメタ更新(migration追加)
drizzle/meta/0002_snapshot.json 新テーブルを含むスナップショット追加
drizzle/0002_amazing_warlock.sql ドメインイベントテーブル/インデックス作成マイグレーション追加

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +3
CREATE TABLE "discord_account_domain_events" (
"id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
"discord_id" text NOT NULL,

This comment was marked as resolved.

Comment on lines 42 to +74
async save(account: DiscordAccount): Promise<void> {
const db = getDb();
const now = new Date().toISOString();
const events = account.getDomainEvents();

await db
.insert(discordAccounts)
.values({
discordId: account.discordId as string,
nickName: account.nickName,
memberId: account.memberId as string,
updatedAt: now,
})
.onConflictDoUpdate({
target: discordAccounts.discordId,
set: {
await db.transaction(async (tx) => {
await tx
.insert(discordAccounts)
.values({
discordId: account.discordId as string,
nickName: account.nickName,
memberId: account.memberId as string,
updatedAt: now,
},
});
})
.onConflictDoUpdate({
target: discordAccounts.discordId,
set: {
nickName: account.nickName,
updatedAt: now,
},
});

if (events.length > 0) {
await tx.insert(discordAccountDomainEvents).values(
events.map((event) => ({
discordId: event.discordId as string,
memberId: event.memberId as string,
eventName: event.eventName,
payload: serializeDiscordAccountEventPayload(event),
occurredAt: event.occurredAt.toISOString(),
})),
);
}

This comment was marked as resolved.

Comment on lines +33 to +84
describe("serializeMemberEventPayload", () => {
it("MemberRegisteredイベントをシリアライズできる", () => {
const member = createActiveMember();
const events = member.getDomainEvents();
expect(events).toHaveLength(1);

const payload = serializeMemberEventPayload(events[0]);
expect(payload.eventName).toBe("MemberRegistered");
if (payload.eventName !== "MemberRegistered") throw new Error();
expect(payload.name).toBe("テスト太郎");
expect(payload.studentId).toBe("725A0001");
expect(payload.personalEmail).toEqual({ type: "notRecorded" });
expect(payload.affiliation).toEqual({
type: "undergraduate",
value: { faculty: "情報学部", department: "情報科学科", year: 3 },
});
});

it("NameChangedイベントをシリアライズできる", () => {
const member = createActiveMember().changeName("新しい名前");
const events = member.getDomainEvents();
const event = events.find((e) => e.eventName === "NameChanged");
if (!event) throw new Error("NameChanged event not found");

const payload = serializeMemberEventPayload(event);
expect(payload.eventName).toBe("NameChanged");
if (payload.eventName !== "NameChanged") throw new Error();
expect(payload.previousName).toBe("テスト太郎");
expect(payload.newName).toBe("新しい名前");
});

it("MemberRemovedイベントをシリアライズできる", () => {
const member = createActiveMember().remove("graduation");
const events = member.getDomainEvents();
const event = events.find((e) => e.eventName === "MemberRemoved");
if (!event) throw new Error("MemberRemoved event not found");

const payload = serializeMemberEventPayload(event);
expect(payload.eventName).toBe("MemberRemoved");
if (payload.eventName !== "MemberRemoved") throw new Error();
expect(payload.reason).toBe("graduation");
});

it("InternallyAdvancedイベントをシリアライズできる", () => {
const masterAffiliation: MasterAffiliation = {
type: "master",
value: {
school: "総合科学技術研究科",
major: "情報学専攻",
course: "基盤情報学コース",
year: 1,
},

This comment was marked as resolved.

Comment on lines +309 to +318
export const discordAccountDomainEvents = pgTable(
"discord_account_domain_events",
{
id: uuid().primaryKey().defaultRandom(),
discordId: text("discord_id").notNull(),
memberId: text("member_id").notNull(),
eventName: varchar("event_name", { length: 64 }).notNull(),
payload: jsonb().notNull().$type<DiscordAccountEventPayload>(),
occurredAt: timestamp("occurred_at", { mode: "string" }).notNull(),
},

This comment was marked as resolved.

occurredAt: timestamp("occurred_at", { mode: "string" }).notNull(),
},
(table) => [
index("member_domain_events_member_id_idx").on(table.memberId),

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@KinjiKawaguchi KinjiKawaguchi force-pushed the feat/domain-event-persistence-v2 branch from 0e790a5 to 8e32ac3 Compare March 24, 2026 08:10
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Class-to-type refactoring was applied completely and consistently

Verified that all 12 member domain event classes and 1 discord account event class were converted to type aliases. Searched the entire codebase for instanceof checks and new MemberRegistered / new DiscordAccountLinked patterns — none remain. All event creation sites use plain object literals. All existing tests (e.g., tests/domain/aggregates/member/Member.test.ts) use eventName property discrimination rather than instanceof, confirming the refactoring is fully consistent.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +37 to +62
export type MemberEventPayload =
| {
name: string;
personalEmail: Recorded<string>;
studentId: string;
affiliation: CompleteAffiliation;
}
| { reason: string }
| { studentId: string; affiliation: CompleteAffiliation }
| Record<string, never>
| {
previousAffiliation: CompleteAffiliation;
newAffiliation: CompleteAffiliation;
previousStudentId: string;
newStudentId: string;
}
| {
previousAffiliation: CompleteAffiliation;
newAffiliation: CompleteAffiliation;
}
| { previousStudentId: string; newStudentId: string }
| { previousName: string; newName: string }
| {
previousPersonalEmail: Recorded<string>;
newPersonalEmail: Recorded<string>;
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: MemberEventPayload is a wide union — no discriminant for deserialization

The MemberEventPayload type at schema.ts:37-62 is a union of 9 shapes with no discriminant field. While it works fine for serialization (the serializeMemberEventPayload switch produces the correct variant), future deserialization code will need to rely on the event_name column from the parent row to determine which variant to parse into. The payload itself has overlapping shapes (e.g., { studentId, affiliation } appears in both MemberRegistered and MemberReregistered/MemberConfirmed payloads). This is fine for now but worth keeping in mind when implementing event replay/read-side.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Open in Devin Review

Comment on lines +46 to 49
if (existing) {
throw new DiscordAccountAlreadyLinkedException(
input.discordAccountId,
existing.memberId,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: ConnectDiscordAccount: intentional loss of idempotency

The old code at src/application/usecase/member/ConnectDiscordAccount.ts:46 allowed re-linking a Discord account if the same member called connect again (existing.memberId !== input.memberId). The new code rejects ALL existing links (if (existing)). This is explicitly intentional per the commit message (fix: 既にリンク済みのDiscordアカウントに対する再linkを拒否). However, this changes the public contract — any caller that previously relied on idempotent re-linking (e.g., retry logic) will now receive a DiscordAccountAlreadyLinkedException. The ChangeDiscordNickNameUseCase at src/application/usecase/member/ChangeDiscordNickName.ts handles nickname updates separately, so the use case split appears complete.

(Refers to lines 46-51)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 43 to 49
const existing = await this.discordRepo.findByDiscordId(
input.discordAccountId,
);
if (existing && existing.memberId !== input.memberId) {
if (existing) {
throw new DiscordAccountAlreadyLinkedException(
input.discordAccountId,
existing.memberId,
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: TOCTOU race in ConnectDiscordAccount could produce duplicate events

In ConnectDiscordAccount.ts:43-59, there's a read-then-write pattern: findByDiscordId checks existence, then save inserts. Two concurrent calls with the same discordAccountId could both pass the check (both see null), and both proceed to save. The onConflictDoUpdate on the discord_accounts table handles the row-level conflict gracefully, but both transactions would insert separate DiscordAccountLinked events into discord_account_domain_events. This race window is very small and unlikely in practice (it requires the same Discord user to trigger two link requests simultaneously), but it's worth noting that the event store has no deduplication mechanism.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

- ドメインイベントをclassからDiscriminated Unionに変更
- DomainEvent基底型を適用(occurredAtを共通化)
- Member/DiscordAccountイベントのシリアライズ・永続化を実装
- eventNameをpgEnumで型安全に管理(ドメイン定数から導出)
- UUIDをアプリ側で生成(DB依存を排除)
- payloadからeventNameを除外(カラムで管理)
- AffiliationをCompleteAffiliationに正確化
- NickNameChangedイベントを追加
- 既リンク済みDiscordアカウントの再link拒否
- インポートパスを#domainバレルに統一

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@KinjiKawaguchi KinjiKawaguchi force-pushed the feat/domain-event-persistence-v2 branch from 03b20e0 to ed79eb6 Compare March 24, 2026 09:01
@KinjiKawaguchi KinjiKawaguchi merged commit d1845ae into develop Mar 24, 2026
5 checks passed
@KinjiKawaguchi KinjiKawaguchi deleted the feat/domain-event-persistence-v2 branch March 24, 2026 09:03
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 new potential issues.

Open in Devin Review

Comment thread src/domain/base/index.ts
Comment on lines 1 to 2
export * from "./DomainEvent";
export * from "./NonEmptyArray";
export * from "./NonEmptyString";
export * from "./ValueObject";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 NonEmptyString and NonEmptyArray removed from public API barrel, breaking package consumers

The domain/base/index.ts barrel removed the re-exports of NonEmptyArray and NonEmptyString. These modules contain the nonEmptyString() factory function and the NonEmptyArray type, which are required by the publicly exported KarteContentProps type (src/domain/aggregates/karte/Karte.ts:54-60). Since the npm package only has a single entry point (src/index.ts), consumers cannot use subpath imports — they rely on the barrel chain. NonEmptyString is a branded type (string & { readonly __brand: unique symbol }), so without the nonEmptyString() function, it is impossible for package consumers to construct valid KarteContentProps values. This breaks CreateKarteUseCase, CorrectKarteUseCase, and ImportKarteUseCase inputs for all external consumers.

Suggested change
export * from "./DomainEvent";
export * from "./NonEmptyArray";
export * from "./NonEmptyString";
export * from "./ValueObject";
export * from "./DomainEvent";
export * from "./NonEmptyArray";
export * from "./NonEmptyString";
export * from "./ValueObject";
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +1 to +22
/**
* 記録有無を明示する型
*
* FDM原則に従い、null/undefinedに意味を持たせない。
* 過去データで記録されていない可能性があるフィールドに使用する。
*/
export type Recorded<T> =
| {
readonly type: "recorded";
readonly value: T;
}
| { readonly type: "notRecorded" };

/** 記録済みの値を生成する */
export function recorded<T>(value: T): Recorded<T> {
return { type: "recorded", value };
}

/** 未記録を生成する */
export function notRecorded<T>(): Recorded<T> {
return { type: "notRecorded" };
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Duplicate Recorded type introduced in karte aggregate

A new src/domain/aggregates/karte/Recorded.ts was created that is an exact copy of src/domain/shared/Recorded.ts. The karte-internal files (Karte.ts, Consultation.ts, Resolution.ts, SupportRecord.ts) now import from the local copy, while the rest of the codebase (member aggregate, repositories, schema) continues to use #domain/shared/Recorded. The karte barrel (karte/index.ts) does not export the local Recorded, so there's no public API conflict — TypeScript's structural typing makes them interchangeable. However, this creates a maintenance burden: changes to the Recorded type would need to be applied in two places. If this is intentional bounded-context encapsulation, it should be documented.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 1 to +4
export * from "./base";
export * from "./event";
export * from "./eventParticipation";
export * from "./karte";
export * from "./member";
export * from "./eventParticipation";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: application/usecase/index.ts removed karte re-export but application/index.ts still exports it

The application/usecase/index.ts barrel removed export * from "./karte". This was investigated and confirmed not to be a bug: the higher-level barrel application/index.ts:4 still has export * from "./usecase/karte", and all consumers (e.g., src/executable/karte.ts:7) import from #application, not #application/usecase. However, the inconsistency between application/index.ts (which exports karte) and application/usecase/index.ts (which doesn't) is confusing and should likely be reconciled.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants